From 4d9837c21e0ee95c2f500e1b4a02d4333e3e17e8 Mon Sep 17 00:00:00 2001 From: Konstantin Akimov Date: Fri, 30 Aug 2024 14:47:15 +0700 Subject: [PATCH 1/2] refactor: add a new flag disable_mocktime to set_test_params() It's better than re-implement setup_nodes each time when you want just disable mocktime. It seems more error prune and the code is shorter --- .../feature_dip3_deterministicmns.py | 2 +- test/functional/feature_sporks.py | 2 +- test/functional/interface_zmq.py | 5 +-- test/functional/p2p_eviction.py | 5 +-- test/functional/p2p_initial_headers_sync.py | 1 + test/functional/p2p_leak.py | 5 +-- .../test_framework/test_framework.py | 41 +++++++++---------- test/functional/test_framework/test_node.py | 12 ++++-- test/functional/wallet_dump.py | 2 +- 9 files changed, 35 insertions(+), 40 deletions(-) diff --git a/test/functional/feature_dip3_deterministicmns.py b/test/functional/feature_dip3_deterministicmns.py index 85e2a91f19..66cbd48c19 100755 --- a/test/functional/feature_dip3_deterministicmns.py +++ b/test/functional/feature_dip3_deterministicmns.py @@ -22,6 +22,7 @@ class DIP3Test(BitcoinTestFramework): self.num_initial_mn = 11 # Should be >= 11 to make sure quorums are not always the same MNs self.num_nodes = 1 + self.num_initial_mn + 2 # +1 for controller, +1 for mn-qt, +1 for mn created after dip3 activation self.setup_clean_chain = True + self.disable_mocktime = True self.supports_cli = False self.extra_args = ["-deprecatedrpc=addresses"] @@ -34,7 +35,6 @@ class DIP3Test(BitcoinTestFramework): self.skip_if_no_wallet() def setup_network(self): - self.disable_mocktime() self.add_nodes(1) self.start_controller_node() self.import_deterministic_coinbase_privkeys() diff --git a/test/functional/feature_sporks.py b/test/functional/feature_sporks.py index 64194eba2f..572c2cd28c 100755 --- a/test/functional/feature_sporks.py +++ b/test/functional/feature_sporks.py @@ -12,10 +12,10 @@ class SporkTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 3 self.setup_clean_chain = True + self.disable_mocktime = True self.extra_args = [["-sporkkey=cP4EKFyJsHT39LDqgdcB43Y3YXjNyjb5Fuas1GQSeAtjnZWmZEQK"], [], []] def setup_network(self): - self.disable_mocktime() self.setup_nodes() # connect only 2 first nodes at start self.connect_nodes(0, 1) diff --git a/test/functional/interface_zmq.py b/test/functional/interface_zmq.py index aebc07f39b..7dbdf45e98 100755 --- a/test/functional/interface_zmq.py +++ b/test/functional/interface_zmq.py @@ -97,6 +97,7 @@ class ZMQTestSetupBlock: class ZMQTest (BitcoinTestFramework): def set_test_params(self): self.num_nodes = 2 + self.disable_mocktime = True if self.is_wallet_compiled(): self.requires_wallet = True # This test isn't testing txn relay/timing, so set whitelist on the @@ -109,10 +110,6 @@ class ZMQTest (BitcoinTestFramework): # TODO: drop this check after migration to MiniWallet, see bitcoin/bitcoin#24653 self.skip_if_no_bdb() - def setup_network(self): - self.disable_mocktime() - super().setup_network() - def run_test(self): self.ctx = zmq.Context() try: diff --git a/test/functional/p2p_eviction.py b/test/functional/p2p_eviction.py index 0dea0c1d38..a648d7620b 100755 --- a/test/functional/p2p_eviction.py +++ b/test/functional/p2p_eviction.py @@ -42,16 +42,13 @@ class SlowP2PInterface(P2PInterface): class P2PEvict(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True + self.disable_mocktime = True self.num_nodes = 1 # The choice of maxconnections=32 results in a maximum of 21 inbound connections # (32 - 10 outbound - 1 feeler). 20 inbound peers are protected from eviction: # 4 by netgroup, 4 that sent us blocks, 4 that sent us transactions and 8 via lowest ping time self.extra_args = [['-maxconnections=32']] - def setup_network(self): - self.disable_mocktime() - super().setup_network() - def run_test(self): protected_peers = set() # peers that we expect to be protected from eviction current_peer = -1 diff --git a/test/functional/p2p_initial_headers_sync.py b/test/functional/p2p_initial_headers_sync.py index af3dd16151..bff6d7c57e 100755 --- a/test/functional/p2p_initial_headers_sync.py +++ b/test/functional/p2p_initial_headers_sync.py @@ -28,6 +28,7 @@ import random class HeadersSyncTest(BitcoinTestFramework): def set_test_params(self): self.setup_clean_chain = True + self.disable_mocktime = True self.num_nodes = 1 def setup_chain(self): diff --git a/test/functional/p2p_leak.py b/test/functional/p2p_leak.py index e1f4965d90..16b94c6918 100755 --- a/test/functional/p2p_leak.py +++ b/test/functional/p2p_leak.py @@ -89,10 +89,7 @@ class P2PVersionStore(P2PInterface): class P2PLeakTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 - - def setup_network(self): - self.disable_mocktime() - self.setup_nodes() + self.disable_mocktime = True def run_test(self): # Another peer that never sends a version, nor any other messages. It shouldn't receive anything from the node. diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py index c40f32a1a2..c80a48ff8a 100755 --- a/test/functional/test_framework/test_framework.py +++ b/test/functional/test_framework/test_framework.py @@ -119,6 +119,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): """Sets test framework defaults. Do not override this method. Instead, override the set_test_params() method""" self.chain: str = 'regtest' self.setup_clean_chain: bool = False + self.disable_mocktime: bool = False self.nodes: List[TestNode] = [] self.network_thread = None self.mocktime = 0 @@ -409,10 +410,10 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): self.log.info("Initializing test directory " + self.options.tmpdir) if self.setup_clean_chain: self._initialize_chain_clean() - self.set_genesis_mocktime() else: self._initialize_chain() - self.set_cache_mocktime() + if not self.disable_mocktime: + self._initialize_mocktime(is_genesis=self.setup_clean_chain) def setup_network(self): """Override this method to customize test network topology""" @@ -451,8 +452,9 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): assert_equal(n.getblockchaininfo()["blocks"], 199) # To ensure that all nodes are out of IBD, the most recent block # must have a timestamp not too old (see IsInitialBlockDownload()). - self.log.debug('Generate a block with current mocktime') - self.bump_mocktime(156 * 200) + if not self.disable_mocktime: + self.log.debug('Generate a block with current mocktime') + self.bump_mocktime(156 * 200) block_hash = self.nodes[0].generate(1)[0] block = self.nodes[0].getblock(blockhash=block_hash, verbosity=0) for n in self.nodes: @@ -810,24 +812,19 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): self.sync_blocks(nodes) self.sync_mempools(nodes) - def disable_mocktime(self): - self.mocktime = 0 - for node in self.nodes: - node.mocktime = 0 - def bump_mocktime(self, t, update_nodes=True, nodes=None): - if self.mocktime != 0: - self.mocktime += t - if update_nodes: - set_node_times(nodes or self.nodes, self.mocktime) + if self.mocktime == 0: + return - def set_cache_mocktime(self): - self.mocktime = TIME_GENESIS_BLOCK + (199 * 156) - for node in self.nodes: - node.mocktime = self.mocktime + self.mocktime += t + if update_nodes: + set_node_times(nodes or self.nodes, self.mocktime) - def set_genesis_mocktime(self): - self.mocktime = TIME_GENESIS_BLOCK + def _initialize_mocktime(self, is_genesis): + if is_genesis: + self.mocktime = TIME_GENESIS_BLOCK + else: + self.mocktime = TIME_GENESIS_BLOCK + (199 * 156) for node in self.nodes: node.mocktime = self.mocktime @@ -884,7 +881,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): cache_node_dir, chain=self.chain, extra_conf=["bind=127.0.0.1"], - extra_args=['-disablewallet', "-mocktime=%d" % TIME_GENESIS_BLOCK], + extra_args=['-disablewallet', f"-mocktime={TIME_GENESIS_BLOCK}"], extra_args_from_options=self.extra_args_from_options, rpchost=None, timewait=self.rpc_timeout, @@ -911,7 +908,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): # block in the cache does not age too much (have an old tip age). # This is needed so that we are out of IBD when the test starts, # see the tip age check in IsInitialBlockDownload(). - self.set_genesis_mocktime() + self._initialize_mocktime(is_genesis=True) gen_addresses = [k.address for k in TestNode.PRIV_KEYS][:3] + [ADDRESS_BCRT1_P2SH_OP_TRUE] assert_equal(len(gen_addresses), 4) for i in range(8): @@ -926,7 +923,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass): # Shut it down, and clean up cache directories: self.stop_nodes() self.nodes = [] - self.disable_mocktime() + self.mocktime = 0 def cache_path(*paths): chain = get_chain_folder(cache_node_dir, self.chain) diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py index d92b5bc4a3..1361ae4701 100755 --- a/test/functional/test_framework/test_node.py +++ b/test/functional/test_framework/test_node.py @@ -103,9 +103,11 @@ class TestNode(): "-debug", "-debugexclude=libevent", "-debugexclude=leveldb", - "-mocktime=" + str(mocktime), - "-uacomment=testnode%d" % i + "-uacomment=testnode%d" % i, ] + if self.mocktime != 0: + self.args.append(f"-mocktime={mocktime}") + if use_valgrind: default_suppressions_file = os.path.join( os.path.dirname(os.path.realpath(__file__)), @@ -215,7 +217,7 @@ class TestNode(): all_args = self.args + self.extra_args_from_options + extra_args if self.mocktime != 0: - all_args = all_args + ["-mocktime=%d" % self.mocktime] + all_args = all_args + [f"-mocktime={self.mocktime}"] # Delete any existing cookie file -- if such a file exists (eg due to # unclean shutdown), it will get overwritten anyway by dashd, and @@ -777,3 +779,7 @@ class RPCOverloadWrapper(): for res in import_res: if not res['success']: raise JSONRPCException(res['error']) + + def setmocktime(self, mocktime): + self.mocktime = mocktime + return self.__getattr__('setmocktime')(mocktime) diff --git a/test/functional/wallet_dump.py b/test/functional/wallet_dump.py index 912dcc87ff..fe585b1f9b 100755 --- a/test/functional/wallet_dump.py +++ b/test/functional/wallet_dump.py @@ -86,6 +86,7 @@ def read_dump(file_name, addrs, script_addrs, hd_master_addr_old): class WalletDumpTest(BitcoinTestFramework): def set_test_params(self): self.num_nodes = 1 + self.disable_mocktime = True self.extra_args = [["-keypool=90", "-usehd=1"]] self.rpc_timeout = 120 @@ -93,7 +94,6 @@ class WalletDumpTest(BitcoinTestFramework): self.skip_if_no_wallet() def setup_network(self): - self.disable_mocktime() self.add_nodes(self.num_nodes, extra_args=self.extra_args) self.start_nodes() From ef4d74a669bce1d59a54f72b2ee9b593745b8ba5 Mon Sep 17 00:00:00 2001 From: UdjinM6 Date: Tue, 27 Aug 2024 21:54:05 +0300 Subject: [PATCH 2/2] test: remove dead code from `p2p_initial_headers_sync.py` to favor of disable mocktime Credits to UdjinM6 --- test/functional/p2p_initial_headers_sync.py | 14 -------------- 1 file changed, 14 deletions(-) diff --git a/test/functional/p2p_initial_headers_sync.py b/test/functional/p2p_initial_headers_sync.py index bff6d7c57e..a9636f9be8 100755 --- a/test/functional/p2p_initial_headers_sync.py +++ b/test/functional/p2p_initial_headers_sync.py @@ -31,20 +31,6 @@ class HeadersSyncTest(BitcoinTestFramework): self.disable_mocktime = True self.num_nodes = 1 - def setup_chain(self): - # This test operates under the assumption that the adjusted time is well ahead of block - # time. - # - # By default when we setup a new chain, we also adjust the mocktime (this is not done in - # Bitcoin's test suite), which violates this test's assumption and causes it to fail. We - # remedy this by ensuring the test's assumptions are met (i.e. we don't adjust mocktime) - # - self.log.info("Initializing test directory " + self.options.tmpdir) - if self.setup_clean_chain: - self._initialize_chain_clean() - else: - self._initialize_chain() - def announce_random_block(self, peers): new_block_announcement = msg_inv(inv=[CInv(MSG_BLOCK, random.randrange(1<<256))]) for p in peers: