aboutsummaryrefslogtreecommitdiff
path: root/test
diff options
context:
space:
mode:
authorMacroFake <falke.marco@gmail.com>2022-11-04 16:50:22 +0100
committerMacroFake <falke.marco@gmail.com>2022-11-04 16:50:42 +0100
commite42ba134f4a06980cb2d8a134a50ada5562063d8 (patch)
tree47f8cd1b9d8c75365dde90e5bf4d9a7df4c97d96 /test
parent83cf055bef8032bc4dea76807eae6318f1e83aea (diff)
parent74d975318a1443aebfbcee36e331df4e54ec1fbe (diff)
Merge bitcoin/bitcoin#26448: test: fix intermittent failure in p2p_sendtxrcncl.py
74d975318a1443aebfbcee36e331df4e54ec1fbe test: fix intermittent failure in p2p_sendtxrcncl.py (Martin Zumsande) Pull request description: `p2p_sendtxrcncl.py` currently fails intermittently in the CI, see e.g. https://cirrus-ci.com/task/5511952184115200?logs=ci#L4024 I believe that this is related to the reuse of the parameter `p2p_idx=2` of `add_outbound_p2p_connection` in this test: When we call `peer_disconnect`, we don't wait until the node has completed the disconnection. So there is a race between setting up the next connection (next `addconnection` RPC), and if the old one hasn't been removed and has an identical port like the new one (because we didn't increment `p2p_idx`), `CConnman::OpenNetworkConnection` just [returns](https://github.com/bitcoin/bitcoin/blob/5274f324375fd31cf8507531fbc612765d03092f/src/net.cpp#L1976) without establishing a connection, and the test fails. Fix this by using distinct `disconnect_p2ps` instead of `peer_disconnect`, which waits for the disconnect to complete. We can then use the same value for `p2p_idx` everywhere. ACKs for top commit: MarcoFalke: review ACK 74d975318a1443aebfbcee36e331df4e54ec1fbe Tree-SHA512: f99f2550b6b320c0a2416a475c1cf189c009fce3a5abf1d4462486e1bfe309e2c3fd4228a4009b0ca38cb77465ce85e3d22298719eb07302fa0a72fbab0e0668
Diffstat (limited to 'test')
-rwxr-xr-xtest/functional/p2p_sendtxrcncl.py30
-rwxr-xr-xtest/functional/test_framework/test_node.py4
2 files changed, 19 insertions, 15 deletions
diff --git a/test/functional/p2p_sendtxrcncl.py b/test/functional/p2p_sendtxrcncl.py
index 50a5127f25..fed9832a7d 100755
--- a/test/functional/p2p_sendtxrcncl.py
+++ b/test/functional/p2p_sendtxrcncl.py
@@ -74,7 +74,7 @@ class SendTxRcnclTest(BitcoinTestFramework):
assert not peer.sendtxrcncl_msg_received.initiator
assert peer.sendtxrcncl_msg_received.responder
assert_equal(peer.sendtxrcncl_msg_received.version, 1)
- peer.peer_disconnect()
+ self.nodes[0].disconnect_p2ps()
self.log.info('SENDTXRCNCL should be sent before VERACK')
peer = self.nodes[0].add_p2p_connection(PeerTrackMsgOrder(), send_version=True, wait_for_verack=True)
@@ -82,7 +82,7 @@ class SendTxRcnclTest(BitcoinTestFramework):
verack_index = [i for i, msg in enumerate(peer.messages) if msg.msgtype == b'verack'][0]
sendtxrcncl_index = [i for i, msg in enumerate(peer.messages) if msg.msgtype == b'sendtxrcncl'][0]
assert(sendtxrcncl_index < verack_index)
- peer.peer_disconnect()
+ self.nodes[0].disconnect_p2ps()
self.log.info('SENDTXRCNCL on pre-WTXID version should not be sent')
peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=False, wait_for_verack=False)
@@ -94,7 +94,7 @@ class SendTxRcnclTest(BitcoinTestFramework):
peer.send_message(pre_wtxid_version_msg)
peer.wait_for_verack()
assert not peer.sendtxrcncl_msg_received
- peer.peer_disconnect()
+ self.nodes[0].disconnect_p2ps()
self.log.info('SENDTXRCNCL for fRelay=false should not be sent')
peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=False, wait_for_verack=False)
@@ -106,7 +106,7 @@ class SendTxRcnclTest(BitcoinTestFramework):
peer.send_message(no_txrelay_version_msg)
peer.wait_for_verack()
assert not peer.sendtxrcncl_msg_received
- peer.peer_disconnect()
+ self.nodes[0].disconnect_p2ps()
self.log.info('valid SENDTXRCNCL received')
peer = self.nodes[0].add_p2p_connection(PeerNoVerack(), send_version=True, wait_for_verack=False)
@@ -152,31 +152,31 @@ class SendTxRcnclTest(BitcoinTestFramework):
with self.nodes[0].assert_debug_log(['Forget txreconciliation state of peer']):
peer.send_message(create_sendtxrcncl_msg())
peer.send_message(msg_verack())
- peer.peer_disconnect()
+ self.nodes[0].disconnect_p2ps()
self.log.info('SENDTXRCNCL sent to an outbound')
peer = self.nodes[0].add_outbound_p2p_connection(
- SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=1, connection_type="outbound-full-relay")
+ SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=0, connection_type="outbound-full-relay")
assert peer.sendtxrcncl_msg_received
assert peer.sendtxrcncl_msg_received.initiator
assert not peer.sendtxrcncl_msg_received.responder
assert_equal(peer.sendtxrcncl_msg_received.version, 1)
- peer.peer_disconnect()
+ self.nodes[0].disconnect_p2ps()
self.log.info('SENDTXRCNCL should not be sent if block-relay-only')
peer = self.nodes[0].add_outbound_p2p_connection(
- SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=2, connection_type="block-relay-only")
+ SendTxrcnclReceiver(), wait_for_verack=True, p2p_idx=0, connection_type="block-relay-only")
assert not peer.sendtxrcncl_msg_received
- peer.peer_disconnect()
+ self.nodes[0].disconnect_p2ps()
self.log.info("SENDTXRCNCL should not be sent if feeler")
- peer = self.nodes[0].add_outbound_p2p_connection(P2PFeelerReceiver(), p2p_idx=2, connection_type="feeler")
+ peer = self.nodes[0].add_outbound_p2p_connection(P2PFeelerReceiver(), p2p_idx=0, connection_type="feeler")
assert not peer.sendtxrcncl_msg_received
- peer.peer_disconnect()
+ self.nodes[0].disconnect_p2ps()
self.log.info('SENDTXRCNCL if block-relay-only triggers a disconnect')
peer = self.nodes[0].add_outbound_p2p_connection(
- PeerNoVerack(), wait_for_verack=False, p2p_idx=3, connection_type="block-relay-only")
+ PeerNoVerack(), wait_for_verack=False, p2p_idx=0, connection_type="block-relay-only")
with self.nodes[0].assert_debug_log(["we indicated no tx relay; disconnecting"]):
peer.send_message(create_sendtxrcncl_msg(initiator=False))
peer.wait_for_disconnect()
@@ -184,7 +184,7 @@ class SendTxRcnclTest(BitcoinTestFramework):
self.log.info('SENDTXRCNCL with initiator=1 and responder=0 from outbound triggers a disconnect')
sendtxrcncl_wrong_role = create_sendtxrcncl_msg(initiator=True)
peer = self.nodes[0].add_outbound_p2p_connection(
- PeerNoVerack(), wait_for_verack=False, p2p_idx=4, connection_type="outbound-full-relay")
+ PeerNoVerack(), wait_for_verack=False, p2p_idx=0, connection_type="outbound-full-relay")
with self.nodes[0].assert_debug_log(["txreconciliation protocol violation"]):
peer.send_message(sendtxrcncl_wrong_role)
peer.wait_for_disconnect()
@@ -193,13 +193,13 @@ class SendTxRcnclTest(BitcoinTestFramework):
self.restart_node(0, [])
peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=True, wait_for_verack=True)
assert not peer.sendtxrcncl_msg_received
- peer.peer_disconnect()
+ self.nodes[0].disconnect_p2ps()
self.log.info('SENDTXRCNCL not sent if blocksonly is set')
self.restart_node(0, ["-txreconciliation", "-blocksonly"])
peer = self.nodes[0].add_p2p_connection(SendTxrcnclReceiver(), send_version=True, wait_for_verack=True)
assert not peer.sendtxrcncl_msg_received
- peer.peer_disconnect()
+ self.nodes[0].disconnect_p2ps()
if __name__ == '__main__':
diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
index 83f9f253fd..0075fa0996 100755
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -625,6 +625,10 @@ class TestNode():
This method adds the p2p connection to the self.p2ps list and returns
the connection to the caller.
+
+ p2p_idx must be different for simultaneously connected peers. When reusing it for the next peer
+ after disconnecting the previous one, it is necessary to wait for the disconnect to finish to avoid
+ a race condition.
"""
def addconnection_callback(address, port):