Merge #19208: test: move sync_blocks and sync_mempool functions to test_framework.py

cc84460c164bcb2a874d4f08b3a2624e5ee9ff0a test: move sync_blocks and sync_mempool functions to test_framework.py (Roy Shao)

Pull request description:

  This PR moves `sync_blocks` and `sync_mempool` out from `test_framework/util.py` to `test_framework/test_framework.py` so they can take contextual information of test framework into account.

  * Change all reference callers to call functions from `test_framework.py`
  * Remove `**kwargs` which is not used
  * Take into account of `timeout_factor` when respecting timeout in function implementations.
  * Pass all tests by running `./test/functional/test_runner.py`

  fixes #18930

ACKs for top commit:
  MarcoFalke:
    ACK cc84460c164bcb2a874d4f08b3a2624e5ee9ff0a , reviewed with --color-moved=dimmed-zebra --color-moved-ws=ignore-all-space 💫

Tree-SHA512: a79b2a3fa842fc26a7aacb834bb2aea88b3049916c0b754e60002a77ce94bb5954e0ea3b436bf268e9295efb62d721dfef263a09339a55c684ac3fda388c275e

test/functional/feature_llmq_rotation.py

@@ -18,7 +18,6 @@ from test_framework.util import (
     assert_equal,
     assert_greater_than_or_equal,
     connect_nodes,
-    sync_blocks,
     wait_until,
 )
 
@@ -126,7 +125,7 @@ class LLMQQuorumRotationTest(DashTestFramework):
 
         mninfos_online = self.mninfo.copy()
         nodes = [self.nodes[0]] + [mn.node for mn in mninfos_online]
-        sync_blocks(nodes)
+        self.sync_blocks(nodes)
         quorum_list = self.nodes[0].quorum("list", llmq_type)
         quorum_blockhash = self.nodes[0].getbestblockhash()
         fallback_blockhash = self.nodes[0].generate(1)[0]

test/functional/p2p_node_network_limited.py

@@ -11,7 +11,7 @@ and that it responds to getdata requests for blocks correctly:
 from test_framework.messages import CInv, msg_getdata, NODE_BLOOM, NODE_NETWORK_LIMITED, NODE_HEADERS_COMPRESSED, msg_verack
 from test_framework.mininode import P2PInterface, mininode_lock
 from test_framework.test_framework import BitcoinTestFramework
-from test_framework.util import assert_equal, disconnect_nodes, connect_nodes, sync_blocks, wait_until
+from test_framework.util import assert_equal, disconnect_nodes, connect_nodes, wait_until
 
 class P2PIgnoreInv(P2PInterface):
     firstAddrnServices = 0
@@ -60,7 +60,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework):
         self.log.info("Mine enough blocks to reach the NODE_NETWORK_LIMITED range.")
         connect_nodes(self.nodes[0], 1)
         blocks = self.nodes[1].generatetoaddress(292, self.nodes[1].get_deterministic_priv_key().address)
-        sync_blocks([self.nodes[0], self.nodes[1]])
+        self.sync_blocks([self.nodes[0], self.nodes[1]])
 
         self.log.info("Make sure we can max retrieve block at tip-288.")
         node.send_getdata_for_block(blocks[1])  # last block in valid range
@@ -86,7 +86,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework):
         # because node 2 is in IBD and node 0 is a NODE_NETWORK_LIMITED peer, sync must not be possible
         connect_nodes(self.nodes[0], 2)
         try:
-            sync_blocks([self.nodes[0], self.nodes[2]], timeout=5)
+            self.sync_blocks([self.nodes[0], self.nodes[2]], timeout=5)
         except:
             pass
         # node2 must remain at height 0
@@ -96,7 +96,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework):
         connect_nodes(self.nodes[1], 2)
 
         # sync must be possible
-        sync_blocks(self.nodes)
+        self.sync_blocks()
 
         # disconnect all peers
         self.disconnect_all()
@@ -108,7 +108,7 @@ class NodeNetworkLimitedTest(BitcoinTestFramework):
         connect_nodes(self.nodes[0], 1)
 
         # sync must be possible, node 1 is no longer in IBD and should therefore connect to node 0 (NODE_NETWORK_LIMITED)
-        sync_blocks([self.nodes[0], self.nodes[1]])
+        self.sync_blocks([self.nodes[0], self.nodes[1]])
         self.stop_node(0, expected_stderr='Warning: You are starting with governance validation disabled. This is expected because you are running a pruned node.')
 
 

test/functional/rpc_getblockfilter.py

@@ -7,7 +7,7 @@
 from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal, assert_is_hex_string, assert_raises_rpc_error,
-    connect_nodes, disconnect_nodes, sync_blocks
+    connect_nodes, disconnect_nodes
     )
 
 FILTER_TYPES = ["basic"]
@@ -30,7 +30,7 @@ class GetBlockFilterTest(BitcoinTestFramework):
 
         # Reorg node 0 to a new chain
         connect_nodes(self.nodes[0], 1)
-        sync_blocks(self.nodes)
+        self.sync_blocks()
 
         assert_equal(self.nodes[0].getblockcount(), 4)
         chain1_hashes = [self.nodes[0].getblockhash(block_height) for block_height in range(4)]

test/functional/rpc_preciousblock.py

@@ -8,7 +8,6 @@ from test_framework.test_framework import BitcoinTestFramework
 from test_framework.util import (
     assert_equal,
     connect_nodes,
-    sync_blocks,
 )
 
 def unidirectional_node_sync_via_rpc(node_src, node_dest):
@@ -73,7 +72,7 @@ class PreciousTest(BitcoinTestFramework):
         assert_equal(self.nodes[0].getbestblockhash(), hashC)
         self.log.info("Make Node1 prefer block C")
         self.nodes[1].preciousblock(hashC)
-        sync_blocks(self.nodes[0:2])  # wait because node 1 may not have downloaded hashC
+        self.sync_blocks(self.nodes[0:2])  # wait because node 1 may not have downloaded hashC
         assert_equal(self.nodes[1].getbestblockhash(), hashC)
         self.log.info("Make Node1 prefer block G again")
         self.nodes[1].preciousblock(hashG)

test/functional/test_framework/test_framework.py

@@ -49,8 +49,6 @@ from .util import (
     set_node_times,
     set_timeout_scale,
     satoshi_round,
-    sync_blocks,
-    sync_mempools,
     wait_until,
     get_chain_folder,
 )
@@ -527,21 +525,54 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
         connect_nodes(self.nodes[1], 2)
         self.sync_all()
 
-    def sync_blocks(self, nodes=None, **kwargs):
+    def sync_blocks(self, nodes=None, wait=1, timeout=60):
-        sync_blocks(nodes or self.nodes, **kwargs)
+        """
+        Wait until everybody has the same tip.
+        sync_blocks needs to be called with an rpc_connections set that has least
+        one node already synced to the latest, stable tip, otherwise there's a
+        chance it might return before all nodes are stably synced.
+        """
+        rpc_connections = nodes or self.nodes
+        timeout = int(timeout * self.options.timeout_factor)
+        timeout *= self.options.timeout_scale
+        stop_time = time.time() + timeout
+        while time.time() <= stop_time:
+            best_hash = [x.getbestblockhash() for x in rpc_connections]
+            if best_hash.count(best_hash[0]) == len(rpc_connections):
+                return
+            # Check that each peer has at least one connection
+            assert (all([len(x.getpeerinfo()) for x in rpc_connections]))
+            time.sleep(wait)
+        raise AssertionError("Block sync timed out:{}".format("".join("\n  {!r}".format(b) for b in best_hash)))
 
-    def sync_mempools(self, nodes=None, **kwargs):
+    def sync_mempools(self, nodes=None, wait=1, timeout=60, flush_scheduler=True, wait_func=None):
-        if self.mocktime != 0:
+        """
-            if 'wait' not in kwargs:
+        Wait until everybody has the same transactions in their memory
-                kwargs['wait'] = 0.1
+        pools
-            if 'wait_func' not in kwargs:
+        """
-                kwargs['wait_func'] = lambda: self.bump_mocktime(3, nodes=nodes)
+        rpc_connections = nodes or self.nodes
+        timeout = int(timeout * self.options.timeout_factor)
+        timeout *= self.options.timeout_scale
+        stop_time = time.time() + timeout
+        if self.mocktime != 0 and wait_func is None:
+            wait_func = lambda: self.bump_mocktime(3, nodes=nodes)
+        while time.time() <= stop_time:
+            pool = [set(r.getrawmempool()) for r in rpc_connections]
+            if pool.count(pool[0]) == len(rpc_connections):
+                if flush_scheduler:
+                    for r in rpc_connections:
+                        r.syncwithvalidationinterfacequeue()
+                return
+            # Check that each peer has at least one connection
+            assert (all([len(x.getpeerinfo()) for x in rpc_connections]))
+            if wait_func is not None:
+                wait_func()
+            time.sleep(wait)
+        raise AssertionError("Mempool sync timed out:{}".format("".join("\n  {!r}".format(m) for m in pool)))
 
-        sync_mempools(nodes or self.nodes, **kwargs)
+    def sync_all(self, nodes=None):
-
+        self.sync_blocks(nodes)
-    def sync_all(self, nodes=None, **kwargs):
+        self.sync_mempools(nodes)
-        self.sync_blocks(nodes, **kwargs)
-        self.sync_mempools(nodes, **kwargs)
 
     def disable_mocktime(self):
         self.mocktime = 0
@@ -1257,7 +1288,7 @@ class DashTestFramework(BitcoinTestFramework):
                 return True
             self.bump_mocktime(sleep, nodes=nodes)
             self.nodes[0].generate(1)
-            sync_blocks(nodes)
+            self.sync_blocks(nodes)
             return False
         wait_until(wait_func, timeout=timeout, sleep=sleep)
 
@@ -1269,7 +1300,7 @@ class DashTestFramework(BitcoinTestFramework):
                     return True
             self.bump_mocktime(sleep, nodes=nodes)
             self.nodes[0].generate(1)
-            sync_blocks(nodes)
+            self.sync_blocks(nodes)
             return False
         wait_until(wait_func, timeout=timeout, sleep=sleep)
 
@@ -1277,7 +1308,7 @@ class DashTestFramework(BitcoinTestFramework):
         time.sleep(1)
         self.bump_mocktime(1, nodes=nodes)
         self.nodes[0].generate(num_blocks)
-        sync_blocks(nodes)
+        self.sync_blocks(nodes)
 
     def mine_quorum(self, llmq_type_name="llmq_test", llmq_type=100, expected_connections=None, expected_members=None, expected_contributions=None, expected_complaints=0, expected_justifications=0, expected_commitments=None, mninfos_online=None, mninfos_valid=None):
         spork21_active = self.nodes[0].spork('show')['SPORK_21_QUORUM_ALL_CONNECTED'] <= 1
@@ -1307,7 +1338,7 @@ class DashTestFramework(BitcoinTestFramework):
         if skip_count != 0:
             self.bump_mocktime(1, nodes=nodes)
             self.nodes[0].generate(skip_count)
-        sync_blocks(nodes)
+        self.sync_blocks(nodes)
 
         q = self.nodes[0].getbestblockhash()
         self.log.info("Expected quorum_hash:"+str(q))
@@ -1349,7 +1380,7 @@ class DashTestFramework(BitcoinTestFramework):
         self.bump_mocktime(1, nodes=nodes)
         self.nodes[0].getblocktemplate() # this calls CreateNewBlock
         self.nodes[0].generate(1)
-        sync_blocks(nodes)
+        self.sync_blocks(nodes)
 
         self.log.info("Waiting for quorum to appear in the list")
         self.wait_for_quorum_list(q, nodes, llmq_type_name=llmq_type_name)
@@ -1361,7 +1392,7 @@ class DashTestFramework(BitcoinTestFramework):
         # Mine 8 (SIGN_HEIGHT_OFFSET) more blocks to make sure that the new quorum gets eligible for signing sessions
         self.nodes[0].generate(8)
 
-        sync_blocks(nodes)
+        self.sync_blocks(nodes)
 
         self.log.info("New quorum: height=%d, quorumHash=%s, quorumIndex=%d, minedBlock=%s" % (quorum_info["height"], new_quorum, quorum_info["quorumIndex"], quorum_info["minedBlock"]))
 
@@ -1397,7 +1428,7 @@ class DashTestFramework(BitcoinTestFramework):
         #     self.bump_mocktime(1, nodes=nodes)
         #     self.nodes[0].generate(skip_count)
         #     time.sleep(4)
-        # sync_blocks(nodes)
+        # self.sync_blocks(nodes)
 
         self.move_blocks(nodes, skip_count)
 
@@ -1479,7 +1510,7 @@ class DashTestFramework(BitcoinTestFramework):
         self.bump_mocktime(1, nodes=nodes)
         self.nodes[0].getblocktemplate() # this calls CreateNewBlock
         self.nodes[0].generate(1)
-        sync_blocks(nodes)
+        self.sync_blocks(nodes)
 
         time.sleep(6)
         self.log.info("Waiting for quorum(s) to appear in the list")
@@ -1490,7 +1521,7 @@ class DashTestFramework(BitcoinTestFramework):
         # Mine 8 (SIGN_HEIGHT_OFFSET) more blocks to make sure that the new quorum gets eligible for signing sessions
         self.nodes[0].generate(8)
 
-        sync_blocks(nodes)
+        self.sync_blocks(nodes)
         self.log.info("New quorum: height=%d, quorumHash=%s, quorumIndex=%d, minedBlock=%s" % (quorum_info_0["height"], q_0, quorum_info_0["quorumIndex"], quorum_info_0["minedBlock"]))
         self.log.info("New quorum: height=%d, quorumHash=%s, quorumIndex=%d, minedBlock=%s" % (quorum_info_1["height"], q_1, quorum_info_1["quorumIndex"], quorum_info_1["minedBlock"]))
 
@@ -1515,7 +1546,7 @@ class DashTestFramework(BitcoinTestFramework):
         if skip_count != 0:
             self.bump_mocktime(1, nodes=nodes)
             self.nodes[0].generate(skip_count)
-        sync_blocks(nodes)
+        self.sync_blocks(nodes)
         time.sleep(1)
         self.log.info('Moved from block %d to %d' % (cur_block, self.nodes[0].getblockcount()))
 

test/functional/test_framework/util.py

@@ -474,48 +474,6 @@ def reconnect_isolated_node(node, node_num):
     node.setnetworkactive(True)
     connect_nodes(node, node_num)
 
-def sync_blocks(rpc_connections, *, wait=1, timeout=60):
-    """
-    Wait until everybody has the same tip.
-
-    sync_blocks needs to be called with an rpc_connections set that has least
-    one node already synced to the latest, stable tip, otherwise there's a
-    chance it might return before all nodes are stably synced.
-    """
-    timeout *= Options.timeout_scale
-
-    stop_time = time.time() + timeout
-    while time.time() <= stop_time:
-        best_hash = [x.getbestblockhash() for x in rpc_connections]
-        if best_hash.count(best_hash[0]) == len(rpc_connections):
-            return
-        # Check that each peer has at least one connection
-        assert (all([len(x.getpeerinfo()) for x in rpc_connections]))
-        time.sleep(wait)
-    raise AssertionError("Block sync timed out:{}".format("".join("\n  {!r}".format(b) for b in best_hash)))
-
-
-def sync_mempools(rpc_connections, *, wait=1, timeout=60, flush_scheduler=True, wait_func=None):
-    """
-    Wait until everybody has the same transactions in their memory
-    pools
-    """
-    timeout *= Options.timeout_scale
-    stop_time = time.time() + timeout
-    while time.time() <= stop_time:
-        pool = [set(r.getrawmempool()) for r in rpc_connections]
-        if pool.count(pool[0]) == len(rpc_connections):
-            if flush_scheduler:
-                for r in rpc_connections:
-                    r.syncwithvalidationinterfacequeue()
-            return
-        # Check that each peer has at least one connection
-        assert (all([len(x.getpeerinfo()) for x in rpc_connections]))
-        if wait_func is not None:
-            wait_func()
-        time.sleep(wait)
-    raise AssertionError("Mempool sync timed out:{}".format("".join("\n  {!r}".format(m) for m in pool)))
-
 def force_finish_mnsync(node):
     """
     Masternodes won't accept incoming connections while IsSynced is false.

test/functional/wallet_balance.py

@@ -12,7 +12,6 @@ from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
     connect_nodes,
-    sync_blocks,
 )
 
 
@@ -203,7 +202,7 @@ class WalletTest(BitcoinTestFramework):
         self.restart_node(1, ['-persistmempool=0', '-checklevel=0'])
         connect_nodes(self.nodes[0], 1)
         connect_nodes(self.nodes[1], 0)
-        sync_blocks(self.nodes)
+        self.sync_blocks()
         self.nodes[1].sendrawtransaction(tx_orig)
         self.nodes[1].generatetoaddress(1, ADDRESS_WATCHONLY)
         self.sync_all()

test/functional/wallet_listreceivedby.py

@@ -10,7 +10,6 @@ from test_framework.util import (
     assert_array_result,
     assert_equal,
     assert_raises_rpc_error,
-    sync_blocks,
 )
 
 
@@ -25,7 +24,7 @@ class ReceivedByTest(BitcoinTestFramework):
     def run_test(self):
         # Generate block to get out of IBD
         self.nodes[0].generate(1)
-        sync_blocks(self.nodes)
+        self.sync_blocks()
 
         # save the number of coinbase reward addresses so far
         num_cb_reward_addresses = len(self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True, include_watchonly=True))