Skip to content

Commit

Permalink
Merge bitcoin#31406: test: fix test_invalid_tx_in_compactblock in `…
Browse files Browse the repository at this point in the history
…p2p_compactblocks`

7239ddb test: make sure node has all transactions (brunoerg)
ee1b9be test: replace `is not` to `!=` when comparing block hash (brunoerg)

Pull request description:

  `test_invalid_tx_in_compactblock` tests that we don't get disconnected if we relay a compact block with valid header, but invalid transactions.

  In this test, after sending the block with invalid transactions, this test checks two things: the tip in the receiver node did not advance and the sender did not get disconnected. However, even if the block contains only valid transactions, the tip would not advance because the receiver does not have all transactions to reconstruct the valid and would request them back. This PR fixes it by sending all the transactions.

  Also, comparing block hash (int) using `is not` can lead to subtle bugs, this PR fixes it by replacing it to `!=`.

  --------------

  Can be tested by applying:
  ```diff
  diff --git a/test/functional/p2p_compactblocks.py b/test/functional/p2p_compactblocks.py
  index 274ef9532c..419153a32f 100755
  --- a/test/functional/p2p_compactblocks.py
  +++ b/test/functional/p2p_compactblocks.py
  @@ -723,11 +723,8 @@ class CompactBlocksTest(BitcoinTestFramework):
           utxo = self.utxos[0]

           block = self.build_block_with_transactions(node, utxo, 5)
  -        del block.vtx[3]
           block.hashMerkleRoot = block.calc_merkle_root()
           # Drop the coinbase witness but include the witness commitment.
  -        add_witness_commitment(block)
  -        block.vtx[0].wit.vtxinwit = []
           block.solve()

           # Make sure node has the transactions to reconstruct the block
  ```

ACKs for top commit:
  instagibbs:
    ACK 7239ddb
  glozow:
    ACK 7239ddb
  lucasbalieiro:
    Tested ACK for commit [7239ddb](bitcoin@7239ddb)

Tree-SHA512: 6d04fb7c50b5e635c83ede75c12130cbd8e1b229887a86a2e1bfe747e4208731faecc7265cae063c1ace187b20c5f37080d5116760766fa2948f38971e5f6fbf
  • Loading branch information
glozow committed Dec 6, 2024
2 parents 1a35447 + 7239ddb commit b1f0f3c
Showing 1 changed file with 2 additions and 2 deletions.
4 changes: 2 additions & 2 deletions test/functional/p2p_compactblocks.py
Original file line number Diff line number Diff line change
Expand Up @@ -733,12 +733,12 @@ def test_invalid_tx_in_compactblock(self, test_node):
# Now send the compact block with all transactions prefilled, and
# verify that we don't get disconnected.
comp_block = HeaderAndShortIDs()
comp_block.initialize_from_block(block, prefill_list=[0, 1, 2, 3, 4], use_witness=True)
comp_block.initialize_from_block(block, prefill_list=list(range(len(block.vtx))), use_witness=True)
msg = msg_cmpctblock(comp_block.to_p2p())
test_node.send_and_ping(msg)

# Check that the tip didn't advance
assert int(node.getbestblockhash(), 16) is not block.sha256
assert int(node.getbestblockhash(), 16) != block.sha256
test_node.sync_with_ping()

# Helper for enabling cb announcements
Expand Down

0 comments on commit b1f0f3c

Please sign in to comment.