Merge #14631: [tests] Move deterministic address import to setup_nodes

3fd7e76f6d [tests] Move deterministic address import to setup_nodes (John Newbery)

Pull request description:

  This requires a small changes to a few tests, but means that
  deterministic addresses will always be imported (unless setup_nodes
  behaviour is explicitly overridden).

  Tidies up the way we import deterministic addresses, requested in review comment here: https://github.com/bitcoin/bitcoin/pull/14468#discussion_r225594586.

Tree-SHA512: 2b32edf500e286c463398487ab1153116a1dc90f64a53614716373311abdc83d8a251fdd8f42d1146b56e308664deaf62952113f66e98bc37f23968096d1a961

10 files changed, 21 insertions, 42 deletions

diff --git a/test/functional/feature_dbcrash.py b/test/functional/feature_dbcrash.py
index ae1eacf2d7..70d67aa53a 100755
--- a/test/functional/feature_dbcrash.py
+++ b/test/functional/feature_dbcrash.py
@@ -69,6 +69,7 @@ class ChainstateWriteCrashTest(BitcoinTestFramework):
     def setup_network(self):
         self.add_nodes(self.num_nodes, extra_args=self.extra_args)
         self.start_nodes()
+        self.import_deterministic_coinbase_privkeys()
         # Leave them unconnected, we'll use submitblock directly in this test
 
     def restart_node(self, node_index, expected_tip):
diff --git a/test/functional/feature_fee_estimation.py b/test/functional/feature_fee_estimation.py
index aaab4279b5..b68e46adbc 100755
--- a/test/functional/feature_fee_estimation.py
+++ b/test/functional/feature_fee_estimation.py
@@ -144,6 +144,9 @@ class EstimateFeeTest(BitcoinTestFramework):
         # (68k weight is room enough for 120 or so transactions)
         # Node2 is a stingy miner, that
         # produces too small blocks (room for only 55 or so transactions)
+        self.start_nodes()
+        self.import_deterministic_coinbase_privkeys()
+        self.stop_nodes()
 
     def transact_and_mine(self, numblocks, mining_node):
         min_fee = Decimal("0.00001")
@@ -171,11 +174,6 @@ class EstimateFeeTest(BitcoinTestFramework):
                     newmem.append(utx)
             self.memutxo = newmem
 
-    def import_deterministic_coinbase_privkeys(self):
-        self.start_nodes()
-        super().import_deterministic_coinbase_privkeys()
-        self.stop_nodes()
-
     def run_test(self):
         self.log.info("This test is time consuming, please be patient")
         self.log.info("Splitting inputs so we can generate tx's")
diff --git a/test/functional/feature_pruning.py b/test/functional/feature_pruning.py
index 772151dc4b..c820ca33e2 100755
--- a/test/functional/feature_pruning.py
+++ b/test/functional/feature_pruning.py
@@ -63,6 +63,8 @@ class PruneTest(BitcoinTestFramework):
     def setup_nodes(self):
         self.add_nodes(self.num_nodes, self.extra_args)
         self.start_nodes()
+        for n in self.nodes:
+            n.importprivkey(privkey=n.get_deterministic_priv_key().key, label='coinbase', rescan=False)
 
     def create_big_chain(self):
         # Start by creating some coinbases we can spend later
diff --git a/test/functional/interface_zmq.py b/test/functional/interface_zmq.py
index 48136a0108..12d484d7c2 100755
--- a/test/functional/interface_zmq.py
+++ b/test/functional/interface_zmq.py
@@ -69,6 +69,7 @@ class ZMQTest (BitcoinTestFramework):
         ]
         self.add_nodes(self.num_nodes, self.extra_args)
         self.start_nodes()
+        self.import_deterministic_coinbase_privkeys()
 
     def run_test(self):
         try:
diff --git a/test/functional/p2p_node_network_limited.py b/test/functional/p2p_node_network_limited.py
index ec3d336dc1..359880506e 100755
--- a/test/functional/p2p_node_network_limited.py
+++ b/test/functional/p2p_node_network_limited.py
@@ -43,8 +43,8 @@ class NodeNetworkLimitedTest(BitcoinTestFramework):
         disconnect_nodes(self.nodes[1], 2)
 
     def setup_network(self):
-        super(NodeNetworkLimitedTest, self).setup_network()
-        self.disconnect_all()
+        self.add_nodes(self.num_nodes, self.extra_args)
+        self.start_nodes()
 
     def run_test(self):
         node = self.nodes[0].add_p2p_connection(P2PIgnoreInv())
diff --git a/test/functional/test_framework/test_framework.py b/test/functional/test_framework/test_framework.py
index 7e2ec673df..44fc185e6d 100755
--- a/test/functional/test_framework/test_framework.py
+++ b/test/functional/test_framework/test_framework.py
@@ -168,7 +168,6 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
             self.skip_test_if_missing_module()
             self.setup_chain()
             self.setup_network()
-            self.import_deterministic_coinbase_privkeys()
             self.run_test()
             success = TestStatus.PASSED
         except JSONRPCException as e:
@@ -261,11 +260,9 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
             extra_args = self.extra_args
         self.add_nodes(self.num_nodes, extra_args)
         self.start_nodes()
+        self.import_deterministic_coinbase_privkeys()
 
     def import_deterministic_coinbase_privkeys(self):
-        if self.setup_clean_chain:
-            return
-
         for n in self.nodes:
             try:
                 n.getwalletinfo()
@@ -273,7 +270,7 @@ class BitcoinTestFramework(metaclass=BitcoinTestMetaClass):
                 assert str(e).startswith('Method not found')
                 continue
 
-            n.importprivkey(n.get_deterministic_priv_key().key)
+            n.importprivkey(privkey=n.get_deterministic_priv_key().key, label='coinbase')
 
     def run_test(self):
         """Tests must override this method to define test logic"""
diff --git a/test/functional/test_framework/test_node.py b/test/functional/test_framework/test_node.py
index cc1bfabbfa..3a6107bb37 100755
--- a/test/functional/test_framework/test_node.py
+++ b/test/functional/test_framework/test_node.py
@@ -199,21 +199,6 @@ class TestNode():
 
     def generate(self, nblocks, maxtries=1000000):
         self.log.debug("TestNode.generate() dispatches `generate` call to `generatetoaddress`")
-        # Try to import the node's deterministic private key. This is a no-op if the private key
-        # has already been imported.
-        try:
-            self.rpc.importprivkey(privkey=self.get_deterministic_priv_key().key, label='coinbase', rescan=False)
-        except JSONRPCException as e:
-            # This may fail if:
-            # - wallet is disabled ('Method not found')
-            # - there are multiple wallets to import to ('Wallet file not specified')
-            # - wallet is locked ('Error: Please enter the wallet passphrase with walletpassphrase first')
-            # Just ignore those errors. We can make this tidier by importing the privkey during TestFramework.setup_nodes
-            # TODO: tidy up deterministic privkey import.
-            assert str(e).startswith('Method not found') or \
-                str(e).startswith('Wallet file not specified') or \
-                str(e).startswith('Error: Please enter the wallet passphrase with walletpassphrase first')
-
         return self.generatetoaddress(nblocks=nblocks, address=self.get_deterministic_priv_key().address, maxtries=maxtries)
 
     def get_wallet_rpc(self, wallet_name):
diff --git a/test/functional/wallet_basic.py b/test/functional/wallet_basic.py
index 4de3356d79..6116103fc6 100755
--- a/test/functional/wallet_basic.py
+++ b/test/functional/wallet_basic.py
@@ -28,10 +28,9 @@ class WalletTest(BitcoinTestFramework):
         self.skip_if_no_wallet()
 
     def setup_network(self):
-        self.add_nodes(4)
-        self.start_node(0)
-        self.start_node(1)
-        self.start_node(2)
+        self.setup_nodes()
+        # Only need nodes 0-2 running at start of test
+        self.stop_node(3)
         connect_nodes_bi(self.nodes, 0, 1)
         connect_nodes_bi(self.nodes, 1, 2)
         connect_nodes_bi(self.nodes, 0, 2)
diff --git a/test/functional/wallet_import_rescan.py b/test/functional/wallet_import_rescan.py
index f043a544fc..08809a688a 100755
--- a/test/functional/wallet_import_rescan.py
+++ b/test/functional/wallet_import_rescan.py
@@ -122,16 +122,14 @@ class ImportRescanTest(BitcoinTestFramework):
 
         # Import keys with pruning disabled
         self.start_nodes(extra_args=[[]] * self.num_nodes)
-        super().import_deterministic_coinbase_privkeys()
+        for n in self.nodes:
+            n.importprivkey(privkey=n.get_deterministic_priv_key().key, label='coinbase')
         self.stop_nodes()
 
         self.start_nodes()
         for i in range(1, self.num_nodes):
             connect_nodes(self.nodes[i], 0)
 
-    def import_deterministic_coinbase_privkeys(self):
-        pass
-
     def run_test(self):
         # Create one transaction on node 0 with a unique amount for
         # each possible type of wallet import RPC.
diff --git a/test/functional/wallet_listreceivedby.py b/test/functional/wallet_listreceivedby.py
index 9e8667c600..011975e371 100755
--- a/test/functional/wallet_listreceivedby.py
+++ b/test/functional/wallet_listreceivedby.py
@@ -18,11 +18,6 @@ class ReceivedByTest(BitcoinTestFramework):
     def set_test_params(self):
         self.num_nodes = 2
 
-    def import_deterministic_coinbase_privkeys(self):
-        assert_equal(0, len(self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True, include_watchonly=True)))
-        super().import_deterministic_coinbase_privkeys()
-        self.num_cb_reward_addresses = len(self.nodes[1].listreceivedbyaddress(minconf=0, include_empty=True, include_watchonly=True))
-
     def skip_test_if_missing_module(self):
         self.skip_if_no_wallet()
 
@@ -31,6 +26,9 @@ class ReceivedByTest(BitcoinTestFramework):
         self.nodes[0].generate(1)
         sync_blocks(self.nodes)
 
+        # 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))
+
         self.log.info("listreceivedbyaddress Test")
 
         # Send from node 0 to 1
@@ -76,7 +74,7 @@ class ReceivedByTest(BitcoinTestFramework):
         assert_raises_rpc_error(-4, "address_filter parameter was invalid", self.nodes[1].listreceivedbyaddress, minconf=0, include_empty=True, include_watchonly=True, address_filter="bamboozling")
         # Another address receive money
         res = self.nodes[1].listreceivedbyaddress(0, True, True)
-        assert_equal(len(res), 2 + self.num_cb_reward_addresses)  # Right now 2 entries
+        assert_equal(len(res), 2 + num_cb_reward_addresses)  # Right now 2 entries
         other_addr = self.nodes[1].getnewaddress()
         txid2 = self.nodes[0].sendtoaddress(other_addr, 0.1)
         self.nodes[0].generate(1)
@@ -93,7 +91,7 @@ class ReceivedByTest(BitcoinTestFramework):
         assert_equal(len(res), 1)
         # Should be two entries though without filter
         res = self.nodes[1].listreceivedbyaddress(0, True, True)
-        assert_equal(len(res), 3 + self.num_cb_reward_addresses)  # Became 3 entries
+        assert_equal(len(res), 3 + num_cb_reward_addresses)  # Became 3 entries
 
         # Not on random addr
         other_addr = self.nodes[0].getnewaddress()  # note on node[0]! just a random addr