0
0
Fork 0
mirror of https://github.com/bitcoin/bitcoin.git synced 2025-02-21 12:22:50 -05:00

Merge bitcoin/bitcoin#31760: test: make sure we are on sync with a peer before checking if they have sent a message

3f4b104b1b test: make sure we are on sync with a peer before checking if they have sent a message (Sergi Delgado Segura)

Pull request description:

  p2p_orphan_handling checks whether a message has not been requested slightly too soon, making the check always succeed. This passes unnoticed since the expected result is for the message to not have been received, but it will make the test not catch a relevant change that should make it fail.

  An easy way to check this is the case is to modify one of the test cases to force a request within the expected time, and check how the request is not seen. After the change, the test would crash as expected:

  ```diff
  index 963d92485c..30ab5f2035 100755
  --- a/test/functional/p2p_orphan_handling.py
  +++ b/test/functional/p2p_orphan_handling.py
  @@ -186,9 +185,12 @@ class OrphanHandlingTest(BitcoinTestFramework):
           parent_inv = CInv(t=MSG_WTX, h=int(tx_parent_arrives["tx"].getwtxid(), 16))
           assert_equal(len(peer_spy.get_invs()), 0)
           peer_spy.assert_no_immediate_response(msg_getdata([parent_inv]))
  +        txid = 0xdeadbeef
  +        peer_spy.send_and_ping(msg_inv([CInv(t=MSG_WTX, h=txid)]))

           # Request would be scheduled with this delay because it is not a preferred relay peer.
           self.nodes[0].bumpmocktime(NONPREF_PEER_TX_DELAY)
  +        peer_spy.assert_never_requested(int(txid))
           peer_spy.assert_never_requested(int(tx_parent_arrives["txid"], 16))
           peer_spy.assert_never_requested(int(tx_parent_doesnt_arrive["txid"], 16))
           # Request would be scheduled with this delay because it is by txid.
  ```

  It is worth noting that this is not seen in the cases where the message is expected to be received, because in such cases `assert_never_requested` is always after a `wait_....` method, which is already waiting for the node to sync on their end.

ACKs for top commit:
  i-am-yuvi:
    ACK 3f4b104b1b
  instagibbs:
    ACK 3f4b104b1b
  glozow:
    ACK 3f4b104b1b

Tree-SHA512: 321a6605d630bed2217b6374e999dbb84da14138263dd8adf65fe3a6cd7981a50c873beced9cf05cb6d747a912e91017c58e7d4323d25449c87d83095ff4cba9
This commit is contained in:
glozow 2025-02-06 00:01:41 -05:00
commit ae9eaa063b
No known key found for this signature in database
GPG key ID: BA03F4DBE0C63FB4

View file

@ -113,6 +113,7 @@ class PeerTxRelayer(P2PTxInvStore):
def assert_never_requested(self, txhash):
"""Check that the node has never sent us a getdata for this hash (int type)"""
self.sync_with_ping()
for getdata in self.getdata_received:
for request in getdata.inv:
assert request.hash != txhash