Skip to content

Commit fa3f9a0

Browse files
author
MarcoFalke
committed
test: Fix intermittent sync_blocks failures
1 parent e727c2b commit fa3f9a0

File tree

6 files changed

+42
-26
lines changed

6 files changed

+42
-26
lines changed

‎test/functional/feature_loadblock.py‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -16,10 +16,8 @@
1616
import tempfile
1717
import urllib
1818

19-
from test_framework.test_framework import (
20-
BitcoinTestFramework,
21-
)
22-
from test_framework.util import assert_equal, wait_until
19+
from test_framework.test_framework import BitcoinTestFramework
20+
from test_framework.util import assert_equal
2321

2422

2523
class LoadblockTest(BitcoinTestFramework):
@@ -75,7 +73,7 @@ def run_test(self):
7573
self.log.info("Restart second, unsynced node with bootstrap file")
7674
self.stop_node(1)
7775
self.start_node(1, ["-loadblock=" + bootstrap_file])
78-
wait_until(lambda: self.nodes[1].getblockcount() == 100)
76+
assert_equal(self.nodes[1].getblockcount(), 100) # start_node is blocking on all block files being imported
7977

8078
assert_equal(self.nodes[1].getblockchaininfo()['blocks'], 100)
8179
assert_equal(self.nodes[0].getbestblockhash(), self.nodes[1].getbestblockhash())

‎test/functional/feature_reindex.py‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -10,10 +10,10 @@
1010
"""
1111

1212
from test_framework.test_framework import BitcoinTestFramework
13-
from test_framework.util import wait_until
13+
from test_framework.util import assert_equal
1414

15-
class ReindexTest(BitcoinTestFramework):
1615

16+
class ReindexTest(BitcoinTestFramework):
1717
def set_test_params(self):
1818
self.setup_clean_chain = True
1919
self.num_nodes = 1
@@ -24,7 +24,7 @@ def reindex(self, justchainstate=False):
2424
self.stop_nodes()
2525
extra_args = [["-reindex-chainstate" if justchainstate else "-reindex"]]
2626
self.start_nodes(extra_args)
27-
wait_until(lambda: self.nodes[0].getblockcount() == blockcount)
27+
assert_equal(self.nodes[0].getblockcount(), blockcount) # start_node is blocking on reindex
2828
self.log.info("Success")
2929

3030
def run_test(self):

‎test/functional/mempool_persist.py‎

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -95,8 +95,8 @@ def run_test(self):
9595
self.start_node(1, extra_args=["-persistmempool=0"])
9696
self.start_node(0)
9797
self.start_node(2)
98-
wait_until(lambda: self.nodes[0].getmempoolinfo()["loaded"], timeout=1)
99-
wait_until(lambda: self.nodes[2].getmempoolinfo()["loaded"], timeout=1)
98+
assert self.nodes[0].getmempoolinfo()["loaded"] # start_node is blocking on the mempool being loaded
99+
assert self.nodes[2].getmempoolinfo()["loaded"]
100100
assert_equal(len(self.nodes[0].getrawmempool()), 6)
101101
assert_equal(len(self.nodes[2].getrawmempool()), 5)
102102
# The others have loaded their mempool. If node_1 loaded anything, we'd probably notice by now:
@@ -117,13 +117,13 @@ def run_test(self):
117117
self.log.debug("Stop-start node0 with -persistmempool=0. Verify that it doesn't load its mempool.dat file.")
118118
self.stop_nodes()
119119
self.start_node(0, extra_args=["-persistmempool=0", "-disablewallet"])
120-
wait_until(lambda: self.nodes[0].getmempoolinfo()["loaded"])
120+
assert self.nodes[0].getmempoolinfo()["loaded"]
121121
assert_equal(len(self.nodes[0].getrawmempool()), 0)
122122

123123
self.log.debug("Stop-start node0. Verify that it has the transactions in its mempool.")
124124
self.stop_nodes()
125125
self.start_node(0)
126-
wait_until(lambda: self.nodes[0].getmempoolinfo()["loaded"])
126+
assert self.nodes[0].getmempoolinfo()["loaded"]
127127
assert_equal(len(self.nodes[0].getrawmempool()), 6)
128128

129129
mempooldat0 = os.path.join(self.nodes[0].datadir, self.chain, 'mempool.dat')
@@ -137,7 +137,7 @@ def run_test(self):
137137
os.rename(mempooldat0, mempooldat1)
138138
self.stop_nodes()
139139
self.start_node(1, extra_args=[])
140-
wait_until(lambda: self.nodes[1].getmempoolinfo()["loaded"])
140+
assert self.nodes[1].getmempoolinfo()["loaded"]
141141
assert_equal(len(self.nodes[1].getrawmempool()), 6)
142142

143143
self.log.debug("Prevent bitcoind from writing mempool.dat to disk. Verify that `savemempool` fails")

‎test/functional/test_framework/test_node.py‎

Lines changed: 26 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,7 @@ def __init__(self, i, datadir, *, chain, rpchost, timewait, factor, bitcoind, bi
110110
"--gen-suppressions=all", "--exit-on-first-error=yes",
111111
"--error-exitcode=1", "--quiet"] + self.args
112112

113-
if self.version is None or self.version >= 190000:
113+
if self.version_is_at_least(190000):
114114
self.args.append("-logthreadnames")
115115

116116
self.cli = TestNodeCLI(bitcoin_cli, self.datadir)
@@ -222,6 +222,27 @@ def wait_for_rpc_connection(self):
222222
rpc = get_rpc_proxy(rpc_url(self.datadir, self.index, self.chain, self.rpchost), self.index, timeout=self.rpc_timeout, coveragedir=self.coverage_dir)
223223
rpc.getblockcount()
224224
# If the call to getblockcount() succeeds then the RPC connection is up
225+
if self.version_is_at_least(190000):
226+
# getmempoolinfo.loaded is available since commit
227+
# bb8ae2c (version 0.19.0)
228+
wait_until(lambda: rpc.getmempoolinfo()['loaded'])
229+
# Wait for the node to finish reindex, block import, and
230+
# loading the mempool. Usually importing happens fast or
231+
# even "immediate" when the node is started. However, there
232+
# is no guarantee and sometimes ThreadImport might finish
233+
# later. This is going to cause intermittent test failures,
234+
# because generally the tests assume the node is fully
235+
# ready after being started.
236+
#
237+
# For example, the node will reject block messages from p2p
238+
# when it is still importing with the error "Unexpected
239+
# block message received"
240+
#
241+
# The wait is done here to make tests as robust as possible
242+
# and prevent racy tests and intermittent failures as much
243+
# as possible. Some tests might not need this, but the
244+
# overhead is trivial, and the added gurantees are worth
245+
# the minimal performance cost.
225246
self.log.debug("RPC successfully started")
226247
if self.use_cli:
227248
return
@@ -274,14 +295,17 @@ def get_wallet_rpc(self, wallet_name):
274295
wallet_path = "wallet/{}".format(urllib.parse.quote(wallet_name))
275296
return RPCOverloadWrapper(self.rpc / wallet_path, descriptors=self.descriptors)
276297

298+
def version_is_at_least(self, ver):
299+
return self.version is None or self.version >= ver
300+
277301
def stop_node(self, expected_stderr='', wait=0):
278302
"""Stop the node."""
279303
if not self.running:
280304
return
281305
self.log.debug("Stopping node")
282306
try:
283307
# Do not use wait argument when testing older nodes, e.g. in feature_backwards_compatibility.py
284-
if self.version is None or self.version >= 180000:
308+
if self.version_is_at_least(180000):
285309
self.stop(wait=wait)
286310
else:
287311
self.stop()

‎test/functional/wallet_abandonconflict.py‎

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,6 @@
1818
assert_raises_rpc_error,
1919
connect_nodes,
2020
disconnect_nodes,
21-
wait_until,
2221
)
2322

2423

@@ -98,7 +97,7 @@ def run_test(self):
9897
# TODO: redo with eviction
9998
self.stop_node(0)
10099
self.start_node(0, extra_args=["-minrelaytxfee=0.0001"])
101-
wait_until(lambda: self.nodes[0].getmempoolinfo()['loaded'])
100+
assert self.nodes[0].getmempoolinfo()['loaded']
102101

103102
# Verify txs no longer in either node's mempool
104103
assert_equal(len(self.nodes[0].getrawmempool()), 0)
@@ -126,7 +125,7 @@ def run_test(self):
126125
# Verify that even with a low min relay fee, the tx is not reaccepted from wallet on startup once abandoned
127126
self.stop_node(0)
128127
self.start_node(0, extra_args=["-minrelaytxfee=0.00001"])
129-
wait_until(lambda: self.nodes[0].getmempoolinfo()['loaded'])
128+
assert self.nodes[0].getmempoolinfo()['loaded']
130129

131130
assert_equal(len(self.nodes[0].getrawmempool()), 0)
132131
assert_equal(self.nodes[0].getbalance(), balance)
@@ -148,7 +147,7 @@ def run_test(self):
148147
# Remove using high relay fee again
149148
self.stop_node(0)
150149
self.start_node(0, extra_args=["-minrelaytxfee=0.0001"])
151-
wait_until(lambda: self.nodes[0].getmempoolinfo()['loaded'])
150+
assert self.nodes[0].getmempoolinfo()['loaded']
152151
assert_equal(len(self.nodes[0].getrawmempool()), 0)
153152
newbalance = self.nodes[0].getbalance()
154153
assert_equal(newbalance, balance - Decimal("24.9996"))

‎test/functional/wallet_basic.py‎

Lines changed: 2 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
55
"""Test the wallet."""
66
from decimal import Decimal
7-
import time
87

98
from test_framework.test_framework import BitcoinTestFramework
109
from test_framework.util import (
@@ -466,12 +465,8 @@ def run_test(self):
466465
extra_args = ["-walletrejectlongchains", "-limitancestorcount=" + str(2 * chainlimit)]
467466
self.start_node(0, extra_args=extra_args)
468467

469-
# wait for loadmempool
470-
timeout = 10
471-
while (timeout > 0 and len(self.nodes[0].getrawmempool()) < chainlimit * 2):
472-
time.sleep(0.5)
473-
timeout -= 0.5
474-
assert_equal(len(self.nodes[0].getrawmempool()), chainlimit * 2)
468+
# wait until the wallet has submitted all transactions to the mempool
469+
wait_until(lambda: len(self.nodes[0].getrawmempool()) == chainlimit * 2)
475470

476471
node0_balance = self.nodes[0].getbalance()
477472
# With walletrejectlongchains we will not create the tx and store it in our wallet.

0 commit comments

Comments
 (0)