test: (Un)solicited invalid cb -> get disconnected.
What the developer wrote
Authored by David Gumberg
test: (Un)solicited invalid cb -> get disconnected.
Modifies the invalid_cmpctblock_message test to check that both HB peers
sending unsolicited and non-HB peers sending solicited invalid
cmpctblock's get the boot from us. Also refactors the test to make it
less stateful.
What changed, and why it matters
This commit only changes a test file. It expands an existing test to verify that Bitcoin Core correctly disconnects peers that send badly formed compact block messages, whether those peers are high-bandwidth or low-bandwidth. It does not change any production code, so it cannot directly introduce or fix a live security vulnerability.
No security action required. Review as normal test improvement.
Security signals we found
Test-only change
Adds coverage for peer disconnection on malformed compact block messages
Comment explicitly states messages are not consensus-invalid
Evidence from the diff
The diff modifies test/functional/p2p_compactblocks.py. It refactors test_invalid_cmpctblock_message to create separate high-bandwidth and low-bandwidth peers, sends each an improperly constructed cmpctblock message (with a prefilled transaction index that is too high), and asserts that the node disconnects both and does not accept the block. The commit is purely test coverage and includes a clarifying comment that these are not consensus-invalid blocks but malformed compact block messages.
Changed components
test/functional/p2p_compactblocks.pyInspect captured patch +24 / −7
diff --git a/test/functional/p2p_compactblocks.py b/test/functional/p2p_compactblocks.py
index 82c1a944..135e96db 100755
--- a/test/functional/p2p_compactblocks.py
+++ b/test/functional/p2p_compactblocks.py
@@ -291,20 +291,37 @@ class CompactBlocksTest(BitcoinTestFramework):
check_announcement_of_new_block(node, test_node, lambda p: "cmpctblock" not in p.last_message and "headers" in p.last_message)
# This test actually causes bitcoind to (reasonably!) disconnect us, so do this last.
+ # Note, these are not consensus-invalid blocks, but improperly constructed cmpctblock messages.
def test_invalid_cmpctblock_message(self):
+ # Make a high-bandwidth peer
+ hb_peer = self.nodes[0].add_p2p_connection(TestP2PConn())
+ self.request_cb_announcements(hb_peer)
+ self.make_peer_hb_to_candidate(self.nodes[0], hb_peer)
+ self.assert_highbandwidth_states(self.nodes[0], idx=-1, hb_to=True, hb_from=True)
+
+ # Make a low-bandwidth peer
+ lb_peer = self.nodes[0].add_p2p_connection(TestP2PConn())
+ self.request_cb_announcements(lb_peer)
+ self.assert_highbandwidth_states(self.nodes[0], idx=-1, hb_to=False, hb_from=True)
+
+ # Construct an invalid cmpctblock message
self.generate(self.nodes[0], COINBASE_MATURITY + 1)
block = self.build_block_on_tip(self.nodes[0])
-
- self.segwit_node.send_header_for_blocks([block])
- self.segwit_node.wait_for_getdata([block.hash_int], timeout=30)
-
cmpct_block = P2PHeaderAndShortIDs()
cmpct_block.header = CBlockHeader(block)
cmpct_block.prefilled_txn_length = 1
- # This index will be too high
- prefilled_txn = PrefilledTransaction(1, block.vtx[0])
+ too_high_prefill_idx = 1
+ prefilled_txn = PrefilledTransaction(too_high_prefill_idx, block.vtx[0])
cmpct_block.prefilled_txn = [prefilled_txn]
- self.segwit_node.send_await_disconnect(msg_cmpctblock(cmpct_block))
+
+ # Test an unsolicited invalid cmpctblock from an HB peer.
+ hb_peer.send_await_disconnect(msg_cmpctblock(cmpct_block))
+ assert_equal(int(self.nodes[0].getbestblockhash(), 16), block.hashPrevBlock)
+
+ # Test a solicited invalid cmpctblock from a non-HB peer.
+ lb_peer.send_header_for_blocks([block])
+ lb_peer.wait_for_getdata([block.hash_int], timeout=30)
+ lb_peer.send_await_disconnect(msg_cmpctblock(cmpct_block))
assert_equal(int(self.nodes[0].getbestblockhash(), 16), block.hashPrevBlock)
# Compare the generated shortids to what we expect based on BIP 152, given
Why this scored 12/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.