Merge #15419: qa: Always refresh cache to be out of ibd

fa2cdc9ac2 test: Simplify create_cache (MarcoFalke)
fa25210d62 qa: Fix wallet_txn_doublespend issue (MarcoFalke)
1111aecbb5 qa: Always refresh stale cache to be out of ibd (MarcoFalke)
fab0d85802 qa: Remove mocktime unless required (MarcoFalke)

Pull request description:

  When starting a test, we are always in IBD because the timestamps on cached blocks are in the past. Usually, we solve that by generating a block at the beginning of the test.

  That is clumsy and might even lead to other problems such as #15360 and https://github.com/bitcoin/bitcoin/issues/14446#issuecomment-461926598

  So fix that by getting rid of mocktime and always refreshing the last block of the cache when starting the test framework.

  Should fix #14446

Tree-SHA512: 6af09800f9c86131349a103af617a54551f5f3f3260d38e14e3f30fdd3d91a0feb0100c56cbb12eae4aeac5571ae4b530b16345cbb831d2670237b53351a22c1

test/functional/feature_proxy.py

@@ -44,6 +44,7 @@ RANGE_BEGIN = PORT_MIN + 2 * PORT_RANGE  # Start after p2p and rpc ports
 class ProxyTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 4
+        self.setup_clean_chain = True
 
     def setup_nodes(self):
         self.have_ipv6 = test_ipv6_local()
@@ -198,4 +199,3 @@ class ProxyTest(BitcoinTestFramework):
 
 if __name__ == '__main__':
     ProxyTest().main()
-

test/functional/mempool_accept.py

@@ -28,7 +28,6 @@ from test_framework.util import (
     assert_equal,
     assert_raises_rpc_error,
     hex_str_to_bytes,
-    wait_until,
 )
 
 
@@ -37,7 +36,6 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
         self.num_nodes = 1
         self.extra_args = [[
             '-txindex',
-            '-reindex',  # Need reindex for txindex
         ]] * self.num_nodes
         self.supports_cli = False
 
@@ -55,7 +53,7 @@ class MempoolAcceptanceTest(BitcoinTestFramework):
 
         self.log.info('Start with empty mempool, and 200 blocks')
         self.mempool_size = 0
-        wait_until(lambda: node.getblockcount() == 200)
+        assert_equal(node.getblockcount(), 200)
         assert_equal(node.getmempoolinfo()['size'], self.mempool_size)
         coins = node.listunspent()
 

test/functional/rpc_blockchain.py

@@ -31,10 +31,12 @@ from test_framework.util import (
     assert_raises_rpc_error,
     assert_is_hex_string,
     assert_is_hash_string,
+    set_node_times,
 )
 from test_framework.blocktools import (
     create_block,
     create_coinbase,
+    TIME_GENESIS_BLOCK,
 )
 from test_framework.messages import (
     CBlockHeader,
@@ -48,10 +50,12 @@ from test_framework.mininode import (
 
 class BlockchainTest(BitcoinTestFramework):
     def set_test_params(self):
+        self.setup_clean_chain = True
         self.num_nodes = 1
         self.supports_cli = False
 
     def run_test(self):
+        self.mine_chain()
         self.restart_node(0, extra_args=['-stopatheight=207', '-prune=1', '-txindex=0'])  # Set extra args with pruning after rescan is complete
 
         # Actual tests
@@ -65,6 +69,15 @@ class BlockchainTest(BitcoinTestFramework):
         self._test_waitforblockheight()
         assert self.nodes[0].verifychain(4, 0)
 
+    def mine_chain(self):
+        self.log.info('Create some old blocks')
+        address = self.nodes[0].get_deterministic_priv_key().address
+        for t in range(TIME_GENESIS_BLOCK, TIME_GENESIS_BLOCK + 200 * 156, 156):
+            # 156 sec steps from genesis block time
+            set_node_times(self.nodes, t)
+            self.nodes[0].generatetoaddress(1, address)
+        assert_equal(self.nodes[0].getblockchaininfo()['blocks'], 200)
+
     def _test_getblockchaininfo(self):
         self.log.info("Test getblockchaininfo")
 

test/functional/test_framework/test_framework.py

@@ -55,6 +55,7 @@ from .util import (
     get_chain_folder,
 )
 
+
 class TestStatus(Enum):
     PASSED = 1
     FAILED = 2
@@ -353,6 +354,20 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
         self.add_nodes(self.num_nodes, extra_args)
         self.start_nodes()
         self.import_deterministic_coinbase_privkeys()
+        if not self.setup_clean_chain:
+            for n in self.nodes:
+                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)
+            block_hash = self.nodes[0].generate(1)[0]
+            block = self.nodes[0].getblock(blockhash=block_hash, verbosity=0)
+            for n in self.nodes:
+                n.submitblock(block)
+                chain_info = n.getblockchaininfo()
+                assert_equal(chain_info["blocks"], 200)
+                assert_equal(chain_info["initialblockdownload"], False)
 
     def import_deterministic_coinbase_privkeys(self):
         for n in self.nodes:
@@ -499,15 +514,13 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
             node.mocktime = 0
 
     def bump_mocktime(self, t, update_nodes=True, nodes=None):
-        self.mocktime += t
-        if update_nodes:
-            set_node_times(nodes or self.nodes, self.mocktime)
+        if self.mocktime != 0:
+            self.mocktime += t
+            if update_nodes:
+                set_node_times(nodes or self.nodes, self.mocktime)
 
     def set_cache_mocktime(self):
-        # For backwared compatibility of the python scripts
-        # with previous versions of the cache, set MOCKTIME
-        # to regtest genesis time + (201 * 156)
-        self.mocktime = TIME_GENESIS_BLOCK + (201 * 156)
+        self.mocktime = TIME_GENESIS_BLOCK + (199 * 156)
         for node in self.nodes:
             node.mocktime = self.mocktime
 
@@ -549,7 +562,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
     def _initialize_chain(self, extra_args=None):
         """Initialize a pre-mined blockchain for use by the test.
 
-        Create a cache of a 200-block-long chain (with wallet) for MAX_NODES
+        Create a cache of a 199-block-long chain (with wallet) for MAX_NODES
         Afterward, create num_nodes copies from the cache."""
 
         assert self.num_nodes <= MAX_NODES
@@ -591,22 +604,19 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
             for node in self.nodes:
                 node.wait_for_rpc_connection()
 
-            # Create a 200-block-long chain; each of the 4 first nodes
+            # Create a 199-block-long chain; each of the 4 first nodes
             # gets 25 mature blocks and 25 immature.
-            # Note: To preserve compatibility with older versions of
-            # initialize_chain, only 4 nodes will generate coins.
-            #
-            # blocks are created with timestamps 10 minutes apart
-            # starting from 2010 minutes in the past
-            block_time = TIME_GENESIS_BLOCK
-            for i in range(2):
-                for peer in range(4):
-                    for j in range(25):
-                        set_node_times(self.nodes, block_time)
-                        self.nodes[peer].generatetoaddress(1, self.nodes[peer].get_deterministic_priv_key().address)
-                        block_time += 156
-                    # Must sync before next peer starts generating blocks
-                    self.sync_blocks()
+            # The 4th node gets only 24 immature blocks so that the very last
+            # 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()
+            for i in range(8):
+                self.bump_mocktime((25 if i != 7 else 24) * 156)
+                self.nodes[0].generatetoaddress(25 if i != 7 else 24, self.nodes[i % 4].get_deterministic_priv_key().address)
+            sync_blocks(self.nodes)
+            for n in self.nodes:
+                assert_equal(n.getblockchaininfo()["blocks"], 199)
 
             # Shut them down, and clean up cache directories:
             self.stop_nodes()

test/functional/wallet_listtransactions.py

@@ -11,7 +11,6 @@ from test_framework.util import assert_array_result, assert_equal
 class ListTransactionsTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
-        self.set_cache_mocktime()
 
     def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()

test/functional/wallet_txn_doublespend.py

@@ -34,6 +34,14 @@ class TxnMallTest(BitcoinTestFramework):
     def run_test(self):
         # All nodes should start with 12,500 DASH:
         starting_balance = 12500
+
+        # All nodes should be out of IBD.
+        # If the nodes are not all out of IBD, that can interfere with
+        # blockchain sync later in the test when nodes are connected, due to
+        # timing issues.
+        for n in self.nodes:
+            assert n.getblockchaininfo()["initialblockdownload"] == False
+
         for i in range(4):
             assert_equal(self.nodes[i].getbalance(), starting_balance)
             self.nodes[i].getnewaddress("")  # bug workaround, coins generated assigned to first getnewaddress!