From 1163aec0e25a3974dd925a9161fe15e510b569ac Mon Sep 17 00:00:00 2001 From: merge-script Date: Tue, 11 Jun 2024 14:11:34 +0100 Subject: [PATCH] Merge bitcoin/bitcoin#30252: test: Remove redundant verack check 0000276b31cea5e443a59d94a98c569293ada951 test: Remove redundant verack check (MarcoFalke) Pull request description: Currently the sync in `connect_nodes` mentions the `version` and `verack` message types, but only checks the `verack`. Neither check is required, as the `pong` check implies both. In case of failure, the debug log will have to be consulted anyway, so the redundant check doesn't add value. Also clarify in the comments that the goal is to check the flag `fSuccessfullyConnected` indirectly. ACKs for top commit: furszy: utACK 0000276b31ce brunoerg: ACK 0000276b31cea5e443a59d94a98c569293ada951 tdb3: ACK 0000276b31cea5e443a59d94a98c569293ada951 Tree-SHA512: f9ddcb1436d2f70da462a8dd470ecfc90a534dd6507c23877ef7626e7c02326c077001a42ad0171a87fba5c5275d1970d8c5e5d82c56c8412de944856fdfd6db --- .../test_framework/test_framework.py | 18 +++++++----------- 1 file changed, 7 insertions(+), 11 deletions(-) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index 0bd17b707e2f..eea41dc43923 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -1,5 +1,5 @@ #!/usr/bin/env python3 -# Copyright (c) 2014-2020 The Bitcoin Core developers +# Copyright (c) 2014-present The Bitcoin Core developers # Copyright (c) 2014-2025 The Dash Core developers # Distributed under the MIT software license, see the accompanying # file COPYING or http://www.opensource.org/licenses/mit-license.php. @@ -718,11 +718,6 @@ def connect_nodes(self, a, b, *, peer_advertises_v2=None): def find_conn(node, peer_subversion, inbound): return next(filter(lambda peer: peer['subver'] == peer_subversion and peer['inbound'] == inbound, node.getpeerinfo()), None) - # poll until version handshake complete to avoid race conditions - # with transaction relaying - # See comments in net_processing: - # * Must have a version message before anything else - # * Must have a verack message before anything else self.wait_until(lambda: find_conn(from_connection, to_connection_subver, inbound=False) is not None) self.wait_until(lambda: find_conn(to_connection, from_connection_subver, inbound=True) is not None) @@ -730,11 +725,12 @@ def check_bytesrecv(peer, msg_type, min_bytes_recv): assert peer is not None, "Error: peer disconnected" return peer['bytesrecv_per_msg'].pop(msg_type, 0) >= min_bytes_recv - self.wait_until(lambda: check_bytesrecv(find_conn(from_connection, to_connection_subver, inbound=False), 'verack', 21)) - self.wait_until(lambda: check_bytesrecv(find_conn(to_connection, from_connection_subver, inbound=True), 'verack', 21)) - - # The message bytes are counted before processing the message, so make - # sure it was fully processed by waiting for a ping. + # Poll until version handshake (fSuccessfullyConnected) is complete to + # avoid race conditions, because some message types are blocked from + # being sent or received before fSuccessfullyConnected. + # + # As the flag fSuccessfullyConnected is not exposed, check it by + # waiting for a pong, which can only happen after the flag was set. self.wait_until(lambda: check_bytesrecv(find_conn(from_connection, to_connection_subver, inbound=False), 'pong', 29)) self.wait_until(lambda: check_bytesrecv(find_conn(to_connection, from_connection_subver, inbound=True), 'pong', 29))