Skip to content

Commit

Permalink
Merge bitcoin/bitcoin#31658: test: p2p: fix sending of manual INVs in…
Browse files Browse the repository at this point in the history
… tx download test

8996fef test: p2p: check that INV messages not matching wtxidrelay are ignored (Sebastian Falbesoner)
e0b3336 test: p2p: fix sending of manual INVs in tx download test (Sebastian Falbesoner)

Pull request description:

  The `test_inv_block` sub-test in p2p_tx_download.py has a subtle bug: the manual msg_inv announcements from peers currently have no effect, since they don't match the wtxidrelay setting (=true by default for `P2PInterface` instances) and are hence ignored by the nodes (since 2d282e0 / PR #18044):

  https://github.com/bitcoin/bitcoin/blob/e7c479495509c068215b73f6df070af2d406ae15/src/net_processing.cpp#L3904-L3911

  Though the test still passes on master, it does so without the intended scenario of asking an additional peer (triggering the GETDATA_TX_INTERVAL delay). Fix this by sending the INV message with MSG_WTX instead of MSG_TX. This increases the test run time by about one minute intentionally.

  It might be good to avoid issues like this in the future, happy to add test framework improvements if someone has a concrete idea.

  (Got into the topic of tx/wtx announcements via the discussion bitcoin/bitcoin#31397 (comment))

ACKs for top commit:
  maflcko:
    ACK 8996fef 😸
  danielabrozzoni:
    ACK 8996fef
  mzumsande:
    Code Review ACK 8996fef

Tree-SHA512: 3da26f9539c89d64c3b0d0579d9af2a6a4577615eed192506e1fb4318421b235f99a6672a497dea3050fba85dad32678f37fd2cda9ecb70cbf52982db37982e8
  • Loading branch information
fanquake committed Jan 24, 2025
2 parents 796e1a4 + 8996fef commit 2d07384
Showing 1 changed file with 33 additions and 2 deletions.
35 changes: 33 additions & 2 deletions test/functional/p2p_tx_download.py
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,11 @@ def getdata_found(peer_index):
def test_inv_block(self):
self.log.info("Generate a transaction on node 0")
tx = self.wallet.create_self_transfer()
txid = int(tx['txid'], 16)
wtxid = int(tx['wtxid'], 16)

self.log.info(
"Announce the transaction to all nodes from all {} incoming peers, but never send it".format(NUM_INBOUND))
msg = msg_inv([CInv(t=MSG_TX, h=txid)])
msg = msg_inv([CInv(t=MSG_WTX, h=wtxid)])
for p in self.peers:
p.send_and_ping(msg)

Expand Down Expand Up @@ -270,6 +270,36 @@ def test_rejects_filter_reset(self):
node.bumpmocktime(MAX_GETDATA_INBOUND_WAIT)
peer.wait_for_getdata([int(low_fee_tx['wtxid'], 16)])

def test_inv_wtxidrelay_mismatch(self):
self.log.info("Check that INV messages that don't match the wtxidrelay setting are ignored")
node = self.nodes[0]
wtxidrelay_on_peer = node.add_p2p_connection(TestP2PConn(wtxidrelay=True))
wtxidrelay_off_peer = node.add_p2p_connection(TestP2PConn(wtxidrelay=False))
random_tx = self.wallet.create_self_transfer()

# MSG_TX INV from wtxidrelay=True peer -> mismatch, ignored
wtxidrelay_on_peer.send_and_ping(msg_inv([CInv(t=MSG_TX, h=int(random_tx['txid'], 16))]))
node.setmocktime(int(time.time()))
node.bumpmocktime(MAX_GETDATA_INBOUND_WAIT)
wtxidrelay_on_peer.sync_with_ping()
assert_equal(wtxidrelay_on_peer.tx_getdata_count, 0)

# MSG_WTX INV from wtxidrelay=False peer -> mismatch, ignored
wtxidrelay_off_peer.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=int(random_tx['wtxid'], 16))]))
node.bumpmocktime(MAX_GETDATA_INBOUND_WAIT)
wtxidrelay_off_peer.sync_with_ping()
assert_equal(wtxidrelay_off_peer.tx_getdata_count, 0)

# MSG_TX INV from wtxidrelay=False peer works
wtxidrelay_off_peer.send_and_ping(msg_inv([CInv(t=MSG_TX, h=int(random_tx['txid'], 16))]))
node.bumpmocktime(MAX_GETDATA_INBOUND_WAIT)
wtxidrelay_off_peer.wait_for_getdata([int(random_tx['txid'], 16)])

# MSG_WTX INV from wtxidrelay=True peer works
wtxidrelay_on_peer.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=int(random_tx['wtxid'], 16))]))
node.bumpmocktime(MAX_GETDATA_INBOUND_WAIT)
wtxidrelay_on_peer.wait_for_getdata([int(random_tx['wtxid'], 16)])

def run_test(self):
self.wallet = MiniWallet(self.nodes[0])

Expand All @@ -291,6 +321,7 @@ def run_test(self):
(self.test_inv_block, True),
(self.test_tx_requests, True),
(self.test_rejects_filter_reset, False),
(self.test_inv_wtxidrelay_mismatch, False),
]:
self.stop_nodes()
self.start_nodes()
Expand Down

0 comments on commit 2d07384

Please sign in to comment.