test: Add missing sync on send_version in peer_connect #28782

pull maflcko wants to merge 1 commits into bitcoin:master from maflcko:2311-test-less-p2p-race- changing 2 files +6 −4
  1. maflcko commented at 12:30 PM on November 3, 2023: member

    Without the sync, the logic will be racy. For example, p2p_sendtxrcncl.py is failing locally (and on CI occasionally), because non-version messages will be sent before the version message:

            self.log.info('SENDTXRCNCL with version=0 triggers a disconnect')
            sendtxrcncl_low_version = create_sendtxrcncl_msg()
            sendtxrcncl_low_version.version = 0
            peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False)
            with self.nodes[0].assert_debug_log(["txreconciliation protocol violation"]):
                peer.send_message(sendtxrcncl_low_version)
                peer.wait_for_disconnect()
    
     test  2023-11-02T08:15:19.620000Z TestFramework (INFO): SENDTXRCNCL with version=0 triggers a disconnect 
     test  2023-11-02T08:15:19.621000Z TestFramework.p2p (DEBUG): Connecting to Bitcoin Node: 127.0.0.1:11312 
     test  2023-11-02T08:15:19.624000Z TestFramework.p2p (DEBUG): Connected & Listening: 127.0.0.1:11312 
     test  2023-11-02T08:15:19.798000Z TestFramework.p2p (DEBUG): Send message to 127.0.0.1:11312: msg_sendtxrcncl(version=0, salt=2) 
     test  2023-11-02T08:15:19.799000Z TestFramework.p2p (DEBUG): Send message to 127.0.0.1:11312: msg_version(nVersion=70016 nServices=9 nTime=Thu Nov  2 08:15:19 2023 addrTo=CAddress(nServices=1 net=IPv4 addr=127.0.0.1 port=11312) addrFrom=CAddress(nServices=1 net=IPv4 addr=0.0.0.0 port=0) nNonce=0x369AC031CDA96022 strSubVer=/python-p2p-tester:0.0.3/ nStartingHeight=-1 relay=1) 
     node0 2023-11-02T08:15:19.804409Z [net] [net.cpp:3676] [CNode] [net] Added connection peer=0 
     node0 2023-11-02T08:15:19.805256Z [net] [net.cpp:1825] [CreateNodeFromAcceptedSocket] [net] connection from 127.0.0.1:55964 accepted 
     node0 2023-11-02T08:15:19.809861Z [msghand] [net_processing.cpp:3356] [ProcessMessage] [net] received: sendtxrcncl (12 bytes) peer=0 
     node0 2023-11-02T08:15:19.810297Z [msghand] [net_processing.cpp:3582] [ProcessMessage] [net] non-version message before version handshake. Message "sendtxrcncl" from peer=0 
     node0 2023-11-02T08:15:19.810928Z [msghand] [net_processing.cpp:3356] [ProcessMessage] [net] received: version (111 bytes) peer=0 
    ...
     test  2023-11-02T09:35:20.166000Z TestFramework.utils (ERROR): wait_until() failed. Predicate: '''' 
                                               def test_function():
                                                   if check_connected:
                                                       assert self.is_connected
                                                   return test_function_in()
                                       '''
     test  2023-11-02T09:35:20.187000Z TestFramework (ERROR): Assertion failed 
                                       Traceback (most recent call last):
                                         File "/ci_container_base/ci/scratch/build/bitcoin-s390x-linux-gnu/test/functional/test_framework/test_framework.py", line 132, in main
                                           self.run_test()
                                         File "/ci_container_base/ci/scratch/build/bitcoin-s390x-linux-gnu/test/functional/p2p_sendtxrcncl.py", line 188, in run_test
                                           peer.wait_for_disconnect()
                                         File "/ci_container_base/ci/scratch/build/bitcoin-s390x-linux-gnu/test/functional/test_framework/p2p.py", line 478, in wait_for_disconnect
                                           self.wait_until(test_function, timeout=timeout, check_connected=False)
                                         File "/ci_container_base/ci/scratch/build/bitcoin-s390x-linux-gnu/test/functional/test_framework/p2p.py", line 470, in wait_until
                                           wait_until_helper_internal(test_function, timeout=timeout, lock=p2p_lock, timeout_factor=self.timeout_factor)
                                         File "/ci_container_base/ci/scratch/build/bitcoin-s390x-linux-gnu/test/functional/test_framework/util.py", line 275, in wait_until_helper_internal
                                           raise AssertionError("Predicate {} not true after {} seconds".format(predicate_source, timeout))
                                       AssertionError: Predicate ''''
                                               def test_function():
                                                   if check_connected:
                                                       assert self.is_connected
                                                   return test_function_in()
                                       ''' not true after 4800.0 seconds
    
  2. test: Add missing sync on send_version in peer_connect fa02598469
  3. DrahtBot commented at 12:30 PM on November 3, 2023: contributor

    <!--e57a25ab6845829454e8d69fc972939a-->

    The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

    <!--006a51241073e994b41acfe9ec718e94-->

    Code Coverage

    For detailed information about the code coverage, see the test coverage report.

    <!--021abf342d371248e50ceaed478a90ca-->

    Reviews

    See the guideline for information on the review process.

    Type Reviewers
    ACK mzumsande

    If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

    <!--174a7506f384e20aa4161008e828411d-->

    Conflicts

    Reviewers, this pull request conflicts with the following ones:

    • #28463 (p2p: Increase inbound capacity for block-relay only connections by mzumsande)
    • #24748 (test/BIP324: functional tests for v2 P2P encryption by stratospher)

    If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

  4. DrahtBot added the label Tests on Nov 3, 2023
  5. fanquake requested review from mzumsande on Nov 3, 2023
  6. mzumsande commented at 7:29 PM on November 7, 2023: contributor

    ACK fa025984698270d8c9a621aef80f1d788ea2ab2e

    I tested this by adding a sleep to connection_made before sending the message. However p2p_sendtxrcncl.py will then fail later in the SENDTXRCNCL if block-relay-only triggers a disconnect part because add_outbound_p2p_connection has the same problem. Do you want to fix that too here, or in another PR?

  7. fanquake merged this on Nov 8, 2023
  8. fanquake closed this on Nov 8, 2023

  9. maflcko deleted the branch on Nov 8, 2023
  10. fanquake referenced this in commit c2da8c583f on Nov 9, 2023
  11. kwvg referenced this in commit e6d5ecdda4 on Oct 15, 2024
  12. kwvg referenced this in commit c0b3062215 on Oct 16, 2024
  13. PastaPastaPasta referenced this in commit dd629cf0eb on Oct 22, 2024
  14. bitcoin locked this on Nov 7, 2024

github-metadata-mirror

This is a metadata mirror of the GitHub repository bitcoin/bitcoin. This site is not affiliated with GitHub. Content is generated from a GitHub metadata backup.
generated: 2026-04-24 09:14 UTC

This site is hosted by @0xB10C
More mirrored repositories can be found on mirror.b10c.me