Merge #6242: refactor: add a new flag disable_mocktime to set_test_params()

ef4d74a669 test: remove dead code from `p2p_initial_headers_sync.py` to favor of disable mocktime (UdjinM6)
4d9837c21e refactor: add a new flag disable_mocktime to set_test_params() (Konstantin Akimov)

Pull request description:

  ## Issue being fixed or feature implemented
  To disable mocktime you should re-implement setup_nodes(). It seems as bug-friendly solution

  ## What was done?
  This PR introduce a new flag "disable_mocktime" which can be set in `set_test_params`.
  It seems more error prune and the code is shorter

  ## How Has This Been Tested?
  Run unit/functional tests including future changes from https://github.com/dashpay/dash/pull/6235

  ## Breaking Changes
  N/A

  ## Checklist:
  - [x] I have performed a self-review of my own code
  - [x] I have commented my code, particularly in hard-to-understand areas
  - [x] I have added or updated relevant unit/integration/functional/e2e tests
  - [x] I have made corresponding changes to the documentation
  - [x] I have assigned this pull request to a milestone

ACKs for top commit:
  UdjinM6:
    utACK ef4d74a669
  kwvg:
    utACK ef4d74a669
  PastaPastaPasta:
    utACK ef4d74a669

Tree-SHA512: c6be8002cae4d7824e150938957464c156931d0b6f7fc41c430a83d662865431f1b56cb11df73f56db54a140f36b0addd68b2917e25c5c941fae52e8d322bc25

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()

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)

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:

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

test/functional/p2p_initial_headers_sync.py

@@ -28,22 +28,9 @@ 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):
-        # 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:

test/functional/p2p_leak.py

@@ -89,10 +89,7 @@ class P2PVersionStore(P2PInterface):
 class P2PLeakTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 1
-
+        self.disable_mocktime = True
-    def setup_network(self):
-        self.disable_mocktime()
-        self.setup_nodes()
 
     def run_test(self):
         # Another peer that never sends a version, nor any other messages. It shouldn't receive anything from the node.

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,6 +452,7 @@ 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()).
+            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]
@@ -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:
+        if self.mocktime == 0:
+            return
+
         self.mocktime += t
         if update_nodes:
             set_node_times(nodes or self.nodes, self.mocktime)
 
-    def set_cache_mocktime(self):
+    def _initialize_mocktime(self, is_genesis):
-        self.mocktime = TIME_GENESIS_BLOCK + (199 * 156)
+        if is_genesis:
-        for node in self.nodes:
-            node.mocktime = self.mocktime
-
-    def set_genesis_mocktime(self):
             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)

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)

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()