From 7005a01c19001ab5821731597656f8bc5e8c11e3 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Wed, 4 Oct 2023 11:05:03 -0400 Subject: [PATCH 1/3] test: add wait_for_connect to BitcoinTestFramework.connect_nodes --- test/functional/test_framework/test_framework.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index ab7fed335c6..a34c34713e9 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -586,7 +586,14 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): def wait_for_node_exit(self, i, timeout): self.nodes[i].process.wait(timeout) - def connect_nodes(self, a, b, *, peer_advertises_v2=None): + def connect_nodes(self, a, b, *, peer_advertises_v2=None, wait_for_connect: bool = True): + """ + Kwargs: + wait_for_connect: if True, block until the nodes are verified as connected. You might + want to disable this when using -stopatheight with one of the connected nodes, + since there will be a race between the actual connection and performing + the assertions before one node shuts down. + """ from_connection = self.nodes[a] to_connection = self.nodes[b] from_num_peers = 1 + len(from_connection.getpeerinfo()) @@ -603,6 +610,9 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): # compatibility with older clients from_connection.addnode(ip_port, "onetry") + if not wait_for_connect: + return + # poll until version handshake complete to avoid race conditions # with transaction relaying # See comments in net_processing: From 5bd2010f024b5bcccf1d57bae6fc36c53f5facc5 Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Wed, 4 Oct 2023 11:05:27 -0400 Subject: [PATCH 2/3] test: assumeutxo: avoid race in functional test --- test/functional/feature_assumeutxo.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index be1aa189938..4d0552f3321 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -142,7 +142,10 @@ class AssumeutxoTest(BitcoinTestFramework): f"-stopatheight={PAUSE_HEIGHT}", *self.extra_args[1]]) # Finally connect the nodes and let them sync. - self.connect_nodes(0, 1) + # + # Set `wait_for_connect=False` to avoid a race between performing connection + # assertions and the -stopatheight tripping. + self.connect_nodes(0, 1, wait_for_connect=False) n1.wait_until_stopped(timeout=5) From 7e4003226030a04a19c718a4b1b83b4ca40ca33f Mon Sep 17 00:00:00 2001 From: James O'Beirne Date: Wed, 4 Oct 2023 11:18:14 -0400 Subject: [PATCH 3/3] tests: assumeutxo: accept final height from either chainstate --- test/functional/feature_assumeutxo.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/test/functional/feature_assumeutxo.py b/test/functional/feature_assumeutxo.py index 4d0552f3321..be0715df32c 100755 --- a/test/functional/feature_assumeutxo.py +++ b/test/functional/feature_assumeutxo.py @@ -159,7 +159,15 @@ class AssumeutxoTest(BitcoinTestFramework): self.connect_nodes(0, 1) self.log.info(f"Ensuring snapshot chain syncs to tip. ({FINAL_HEIGHT})") - wait_until_helper(lambda: n1.getchainstates()['snapshot']['blocks'] == FINAL_HEIGHT) + + def check_for_final_height(): + chainstates = n1.getchainstates() + # The background validation may have completed before we run our first + # check, so accept a final blockheight from either chainstate type. + cs = chainstates.get('snapshot') or chainstates.get('normal') + return cs['blocks'] == FINAL_HEIGHT + + wait_until_helper(check_for_final_height) self.sync_blocks(nodes=(n0, n1)) self.log.info("Ensuring background validation completes")