mirror of
https://github.com/bitcoin/bitcoin.git
synced 2026-07-05 11:40:07 +02:00
Merge bitcoin/bitcoin#35550: net_processing: fix BIP152 first integer interpretation
abc33ff043test: announce field must be 0 or 1 in sendcmpct (brunoerg)2d0dce0af5net_processing: fix BIP152 first integer interpretation (brunoerg) Pull request description: Fixes #35542 According to the BIP152, the first integer in `sendcmpct` message shall be interpreted as a boolean (and MUST have a value of either 1 or 0). We currently correctly interpret it as boolean, however, we accept any value >=1 and treat it as `true`, deviating from the specification. This PR fixes it. ACKs for top commit: edilmedeiros: utACKabc33ff043davidgumberg: crACKabc33ff043Seems reasonable to comply with BIP152 strictly, test looks good as well. Sjors: ACKabc33ff043jonatack: re-ACKabc33ff043achow101: ACKabc33ff043w0xlt: ACKabc33ff043Tree-SHA512: 77fed86d4de81f7c35ff002b6e1b2a90882ea55f159075da4d34a619d1075f625fca34f929cdd981f1b2eb06f76b64f42cda46502dcd1b4a634c690b0882ec7c
This commit is contained in:
@@ -3935,10 +3935,17 @@ void PeerManagerImpl::ProcessMessage(Peer& peer, CNode& pfrom, const std::string
|
||||
}
|
||||
|
||||
if (msg_type == NetMsgType::SENDCMPCT) {
|
||||
bool sendcmpct_hb{false};
|
||||
uint8_t sendcmpct_hb{0};
|
||||
uint64_t sendcmpct_version{0};
|
||||
vRecv >> sendcmpct_hb >> sendcmpct_version;
|
||||
|
||||
// BIP152: the first integer is interpreted as a boolean and MUST have a
|
||||
// value of either 1 or 0.
|
||||
if (sendcmpct_hb > 1) {
|
||||
Misbehaving(peer, "invalid sendcmpct announce field");
|
||||
return;
|
||||
}
|
||||
|
||||
// Only support compact block relay with witnesses
|
||||
if (sendcmpct_version != CMPCTBLOCKS_VERSION) return;
|
||||
|
||||
|
||||
@@ -263,6 +263,16 @@ class CompactBlocksTest(BitcoinTestFramework):
|
||||
test_node.send_and_ping(msg_sendcmpct(announce=False, version=2))
|
||||
check_announcement_of_new_block(node, test_node, lambda p: "cmpctblock" not in p.last_message and "headers" in p.last_message)
|
||||
|
||||
# BIP152 mandates that the announce field of a sendcmpct message is a
|
||||
# boolean and MUST have a value of either 1 or 0. Sending any other value
|
||||
# should be treated as misbehavior and lead to a disconnect.
|
||||
def test_invalid_sendcmpct_announce(self):
|
||||
node = self.nodes[0]
|
||||
bad_peer = node.add_p2p_connection(TestP2PConn())
|
||||
msg = msg_sendcmpct(announce=2, version=2)
|
||||
with node.assert_debug_log(['invalid sendcmpct announce field']):
|
||||
bad_peer.send_await_disconnect(msg)
|
||||
|
||||
# This test actually causes bitcoind to (reasonably!) disconnect us, so do this last.
|
||||
def test_invalid_cmpctblock_message(self):
|
||||
self.generate(self.nodes[0], COINBASE_MATURITY + 1)
|
||||
@@ -1017,6 +1027,9 @@ class CompactBlocksTest(BitcoinTestFramework):
|
||||
# The previous test will lead to a disconnection. Reconnect before continuing.
|
||||
self.segwit_node = self.nodes[0].add_p2p_connection(TestP2PConn())
|
||||
|
||||
self.log.info("Testing invalid announce field in sendcmpct message...")
|
||||
self.test_invalid_sendcmpct_announce()
|
||||
|
||||
self.log.info("Testing invalid index in cmpctblock message...")
|
||||
self.test_invalid_cmpctblock_message()
|
||||
|
||||
|
||||
Reference in New Issue
Block a user