diff options
author | fanquake <fanquake@gmail.com> | 2023-11-08 09:49:27 +0000 |
---|---|---|
committer | fanquake <fanquake@gmail.com> | 2023-11-08 09:55:53 +0000 |
commit | 1162d046ecf23dd7047811a2769ff62262d73369 (patch) | |
tree | 9165ab769e8cb2e22112e5f2f8c9b7f5140a101d /test | |
parent | b5d8f001a83878d323ea53a3a7eb8d1957c9949a (diff) | |
parent | fa025984698270d8c9a621aef80f1d788ea2ab2e (diff) |
Merge bitcoin/bitcoin#28782: test: Add missing sync on send_version in peer_connect
fa025984698270d8c9a621aef80f1d788ea2ab2e test: Add missing sync on send_version in peer_connect (MarcoFalke)
Pull request description:
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:
```py
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
ACKs for top commit:
mzumsande:
ACK fa025984698270d8c9a621aef80f1d788ea2ab2e
Tree-SHA512: 78871f603d387e2df8c0acbdfa95441fa186f80e94593021bb219bbf1bc9dc7efc4e266bd254b5cc41114c38227ff3b7f6172335d9bb828427f0a2acffde752d
Diffstat (limited to 'test')
-rwxr-xr-x | test/functional/test_framework/p2p.py | 4 | ||||
-rwxr-xr-x | test/functional/test_framework/test_node.py | 6 |
2 files changed, 6 insertions, 4 deletions
diff --git a/test/functional/test_framework/p2p.py b/test/functional/test_framework/p2p.py index 352becb367..b1ed97b794 100755 --- a/test/functional/test_framework/p2p.py +++ b/test/functional/test_framework/p2p.py @@ -364,8 +364,8 @@ class P2PInterface(P2PConnection): vt.addrFrom.port = 0 self.on_connection_send_msg = vt # Will be sent in connection_made callback - def peer_connect(self, *args, services=P2P_SERVICES, send_version=True, **kwargs): - create_conn = super().peer_connect(*args, **kwargs) + def peer_connect(self, *, services=P2P_SERVICES, send_version, **kwargs): + create_conn = super().peer_connect(**kwargs) if send_version: self.peer_connect_send_version(services) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index b6af71d85c..f440ac4b27 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -634,7 +634,7 @@ class TestNode(): assert_msg += "with expected error " + expected_msg self._raise_assertion_error(assert_msg) - def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, **kwargs): + def add_p2p_connection(self, p2p_conn, *, wait_for_verack=True, send_version=True, **kwargs): """Add an inbound p2p connection to the node. This method adds the p2p connection to the self.p2ps list and also @@ -645,9 +645,11 @@ class TestNode(): kwargs['dstaddr'] = '127.0.0.1' p2p_conn.p2p_connected_to_node = True - p2p_conn.peer_connect(**kwargs, net=self.chain, timeout_factor=self.timeout_factor)() + p2p_conn.peer_connect(**kwargs, send_version=send_version, net=self.chain, timeout_factor=self.timeout_factor)() self.p2ps.append(p2p_conn) p2p_conn.wait_until(lambda: p2p_conn.is_connected, check_connected=False) + if send_version: + p2p_conn.wait_until(lambda: not p2p_conn.on_connection_send_msg) if wait_for_verack: # Wait for the node to send us the version and verack p2p_conn.wait_for_verack() |