test: backwards compatibility: misc fixes

This cleanup may slightly impact test coverage.

Reduce code repition by looping over the list of nodes.

Reduce brittleness by refering to nodes by name rather than index.

Add helper method nodes_wallet_dir.

1 files changed, 80 insertions, 183 deletions

diff --git a/test/functional/feature_backwards_compatibility.py b/test/functional/feature_backwards_compatibility.py
index 476a6a0c14..df18efb6d2 100755
--- a/test/functional/feature_backwards_compatibility.py
+++ b/test/functional/feature_backwards_compatibility.py
@@ -63,19 +63,27 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
         self.start_nodes()
         self.import_deterministic_coinbase_privkeys()
 
-    def run_test(self):
-        self.generatetoaddress(self.nodes[0], COINBASE_MATURITY + 1, self.nodes[0].getnewaddress())
-
-        # Sanity check the test framework:
-        res = self.nodes[self.num_nodes - 1].getblockchaininfo()
-        assert_equal(res['blocks'], COINBASE_MATURITY + 1)
+    def nodes_wallet_dir(self, node):
+        if node.version < 170000:
+            return os.path.join(node.datadir, "regtest")
+        return os.path.join(node.datadir, "regtest/wallets")
 
-        node_master = self.nodes[self.num_nodes - 5]
+    def run_test(self):
+        node_miner = self.nodes[0]
+        node_master = self.nodes[1]
         node_v19 = self.nodes[self.num_nodes - 4]
         node_v18 = self.nodes[self.num_nodes - 3]
         node_v17 = self.nodes[self.num_nodes - 2]
         node_v16 = self.nodes[self.num_nodes - 1]
 
+        legacy_nodes = self.nodes[2:]
+
+        self.generatetoaddress(node_miner, COINBASE_MATURITY + 1, node_miner.getnewaddress())
+
+        # Sanity check the test framework:
+        res = node_v16.getblockchaininfo()
+        assert_equal(res['blocks'], COINBASE_MATURITY + 1)
+
         self.log.info("Test wallet backwards compatibility...")
         # Create a number of wallets and open them in older versions:
 
@@ -88,21 +96,21 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
         assert info['keypoolsize'] > 0
         # Create a confirmed transaction, receiving coins
         address = wallet.getnewaddress()
-        self.nodes[0].sendtoaddress(address, 10)
+        node_miner.sendtoaddress(address, 10)
         self.sync_mempools()
-        self.generate(self.nodes[0], 1)
+        self.generate(node_miner, 1)
         # Create a conflicting transaction using RBF
-        return_address = self.nodes[0].getnewaddress()
-        tx1_id = self.nodes[1].sendtoaddress(return_address, 1)
-        tx2_id = self.nodes[1].bumpfee(tx1_id)["txid"]
+        return_address = node_miner.getnewaddress()
+        tx1_id = node_master.sendtoaddress(return_address, 1)
+        tx2_id = node_master.bumpfee(tx1_id)["txid"]
         # Confirm the transaction
         self.sync_mempools()
-        self.generate(self.nodes[0], 1)
+        self.generate(node_miner, 1)
         # Create another conflicting transaction using RBF
-        tx3_id = self.nodes[1].sendtoaddress(return_address, 1)
-        tx4_id = self.nodes[1].bumpfee(tx3_id)["txid"]
+        tx3_id = node_master.sendtoaddress(return_address, 1)
+        tx4_id = node_master.bumpfee(tx3_id)["txid"]
         # Abandon transaction, but don't confirm
-        self.nodes[1].abandontransaction(tx3_id)
+        node_master.abandontransaction(tx3_id)
 
         # w1_v19: regular wallet, created with v0.19
         node_v19.rpc.createwallet(wallet_name="w1_v19")
@@ -113,6 +121,7 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
         # Use addmultisigaddress (see #18075)
         address_18075 = wallet.rpc.addmultisigaddress(1, ["0296b538e853519c726a2c91e61ec11600ae1390813a627c66fb8be7947be63c52", "037211a824f55b505228e4c3d5194c1fcfaa15a456abdf37f9b9d97a4040afc073"], "", "legacy")["address"]
         assert wallet.getaddressinfo(address_18075)["solvable"]
+        node_v19.unloadwallet("w1_v19")
 
         # w1_v18: regular wallet, created with v0.18
         node_v18.rpc.createwallet(wallet_name="w1_v18")
@@ -130,20 +139,6 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
         assert info['private_keys_enabled'] == False
         assert info['keypoolsize'] == 0
 
-        # w2_v19: wallet with private keys disabled, created with v0.19
-        node_v19.rpc.createwallet(wallet_name="w2_v19", disable_private_keys=True)
-        wallet = node_v19.get_wallet_rpc("w2_v19")
-        info = wallet.getwalletinfo()
-        assert info['private_keys_enabled'] == False
-        assert info['keypoolsize'] == 0
-
-        # w2_v18: wallet with private keys disabled, created with v0.18
-        node_v18.rpc.createwallet(wallet_name="w2_v18", disable_private_keys=True)
-        wallet = node_v18.get_wallet_rpc("w2_v18")
-        info = wallet.getwalletinfo()
-        assert info['private_keys_enabled'] == False
-        assert info['keypoolsize'] == 0
-
         # w3: blank wallet, created on master: update this
         #     test when default blank wallets can no longer be opened by older versions.
         node_master.createwallet(wallet_name="w3", blank=True)
@@ -152,170 +147,72 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
         assert info['private_keys_enabled']
         assert info['keypoolsize'] == 0
 
-        # w3_v19: blank wallet, created with v0.19
-        node_v19.rpc.createwallet(wallet_name="w3_v19", blank=True)
-        wallet = node_v19.get_wallet_rpc("w3_v19")
-        info = wallet.getwalletinfo()
-        assert info['private_keys_enabled']
-        assert info['keypoolsize'] == 0
-
-        # w3_v18: blank wallet, created with v0.18
-        node_v18.rpc.createwallet(wallet_name="w3_v18", blank=True)
-        wallet = node_v18.get_wallet_rpc("w3_v18")
-        info = wallet.getwalletinfo()
-        assert info['private_keys_enabled']
-        assert info['keypoolsize'] == 0
-
-        # Copy the wallets to older nodes:
+        # Unload wallets and copy to older nodes:
         node_master_wallets_dir = os.path.join(node_master.datadir, "regtest/wallets")
         node_v19_wallets_dir = os.path.join(node_v19.datadir, "regtest/wallets")
-        node_v18_wallets_dir = os.path.join(node_v18.datadir, "regtest/wallets")
         node_v17_wallets_dir = os.path.join(node_v17.datadir, "regtest/wallets")
         node_v16_wallets_dir = os.path.join(node_v16.datadir, "regtest")
         node_master.unloadwallet("w1")
         node_master.unloadwallet("w2")
-        node_v19.unloadwallet("w1_v19")
-        node_v19.unloadwallet("w2_v19")
-        node_v18.unloadwallet("w1_v18")
-        node_v18.unloadwallet("w2_v18")
-
-        # Copy wallets to v0.16
-        for wallet in os.listdir(node_master_wallets_dir):
-            shutil.copytree(
-                os.path.join(node_master_wallets_dir, wallet),
-                os.path.join(node_v16_wallets_dir, wallet)
-            )
+        node_master.unloadwallet("w3")
 
-        # Copy wallets to v0.17
-        for wallet in os.listdir(node_master_wallets_dir):
-            shutil.copytree(
-                os.path.join(node_master_wallets_dir, wallet),
-                os.path.join(node_v17_wallets_dir, wallet)
-            )
-        for wallet in os.listdir(node_v18_wallets_dir):
-            shutil.copytree(
-                os.path.join(node_v18_wallets_dir, wallet),
-                os.path.join(node_v17_wallets_dir, wallet)
-            )
-
-        # Copy wallets to v0.18
-        for wallet in os.listdir(node_master_wallets_dir):
-            shutil.copytree(
-                os.path.join(node_master_wallets_dir, wallet),
-                os.path.join(node_v18_wallets_dir, wallet)
-            )
-
-        # Copy wallets to v0.19
-        for wallet in os.listdir(node_master_wallets_dir):
-            shutil.copytree(
-                os.path.join(node_master_wallets_dir, wallet),
-                os.path.join(node_v19_wallets_dir, wallet)
-            )
+        for node in legacy_nodes:
+            # Copy wallets to previous version
+            for wallet in os.listdir(node_master_wallets_dir):
+                shutil.copytree(
+                    os.path.join(node_master_wallets_dir, wallet),
+                    os.path.join(self.nodes_wallet_dir(node), wallet)
+                )
 
         if not self.options.descriptors:
             # Descriptor wallets break compatibility, only run this test for legacy wallet
-            # Open the wallets in v0.19
-            node_v19.loadwallet("w1")
-            wallet = node_v19.get_wallet_rpc("w1")
-            info = wallet.getwalletinfo()
-            assert info['private_keys_enabled']
-            assert info['keypoolsize'] > 0
-            txs = wallet.listtransactions()
-            assert_equal(len(txs), 5)
-            assert_equal(txs[1]["txid"], tx1_id)
-            assert_equal(txs[2]["walletconflicts"], [tx1_id])
-            assert_equal(txs[1]["replaced_by_txid"], tx2_id)
-            assert not(txs[1]["abandoned"])
-            assert_equal(txs[1]["confirmations"], -1)
-            assert_equal(txs[2]["blockindex"], 1)
-            assert txs[3]["abandoned"]
-            assert_equal(txs[4]["walletconflicts"], [tx3_id])
-            assert_equal(txs[3]["replaced_by_txid"], tx4_id)
-            assert not(hasattr(txs[3], "blockindex"))
-
-            node_v19.loadwallet("w2")
-            wallet = node_v19.get_wallet_rpc("w2")
-            info = wallet.getwalletinfo()
-            assert info['private_keys_enabled'] == False
-            assert info['keypoolsize'] == 0
-
-            node_v19.loadwallet("w3")
-            wallet = node_v19.get_wallet_rpc("w3")
-            info = wallet.getwalletinfo()
-            assert info['private_keys_enabled']
-            assert info['keypoolsize'] == 0
-
-            # Open the wallets in v0.18
-            node_v18.loadwallet("w1")
-            wallet = node_v18.get_wallet_rpc("w1")
-            info = wallet.getwalletinfo()
-            assert info['private_keys_enabled']
-            assert info['keypoolsize'] > 0
-            txs = wallet.listtransactions()
-            assert_equal(len(txs), 5)
-            assert_equal(txs[1]["txid"], tx1_id)
-            assert_equal(txs[2]["walletconflicts"], [tx1_id])
-            assert_equal(txs[1]["replaced_by_txid"], tx2_id)
-            assert not(txs[1]["abandoned"])
-            assert_equal(txs[1]["confirmations"], -1)
-            assert_equal(txs[2]["blockindex"], 1)
-            assert txs[3]["abandoned"]
-            assert_equal(txs[4]["walletconflicts"], [tx3_id])
-            assert_equal(txs[3]["replaced_by_txid"], tx4_id)
-            assert not(hasattr(txs[3], "blockindex"))
-
-            node_v18.loadwallet("w2")
-            wallet = node_v18.get_wallet_rpc("w2")
-            info = wallet.getwalletinfo()
-            assert info['private_keys_enabled'] == False
-            assert info['keypoolsize'] == 0
-
-            node_v18.loadwallet("w3")
-            wallet = node_v18.get_wallet_rpc("w3")
-            info = wallet.getwalletinfo()
-            assert info['private_keys_enabled']
-            assert info['keypoolsize'] == 0
-
-            node_v17.loadwallet("w1")
-            wallet = node_v17.get_wallet_rpc("w1")
-            info = wallet.getwalletinfo()
-            assert info['private_keys_enabled']
-            assert info['keypoolsize'] > 0
-
-            node_v17.loadwallet("w2")
-            wallet = node_v17.get_wallet_rpc("w2")
-            info = wallet.getwalletinfo()
-            assert info['private_keys_enabled'] == False
-            assert info['keypoolsize'] == 0
+            # Load modern wallet with older nodes
+            for node in legacy_nodes:
+                for wallet_name in ["w1", "w2", "w3"]:
+                    if node.version < 170000:
+                        # loadwallet was introduced in v0.17.0
+                        continue
+                    if node.version < 180000 and wallet_name == "w3":
+                        # Blank wallets were introduced in v0.18.0. We test the loading error below.
+                        continue
+                    node.loadwallet(wallet_name)
+                    wallet = node.get_wallet_rpc(wallet_name)
+                    info = wallet.getwalletinfo()
+                    if wallet_name == "w1":
+                        assert info['private_keys_enabled'] == True
+                        assert info['keypoolsize'] > 0
+                        txs = wallet.listtransactions()
+                        assert_equal(len(txs), 5)
+                        assert_equal(txs[1]["txid"], tx1_id)
+                        assert_equal(txs[2]["walletconflicts"], [tx1_id])
+                        assert_equal(txs[1]["replaced_by_txid"], tx2_id)
+                        assert not(txs[1]["abandoned"])
+                        assert_equal(txs[1]["confirmations"], -1)
+                        assert_equal(txs[2]["blockindex"], 1)
+                        assert txs[3]["abandoned"]
+                        assert_equal(txs[4]["walletconflicts"], [tx3_id])
+                        assert_equal(txs[3]["replaced_by_txid"], tx4_id)
+                        assert not(hasattr(txs[3], "blockindex"))
+                    elif wallet_name == "w2":
+                        assert(info['private_keys_enabled'] == False)
+                        assert info['keypoolsize'] == 0
+                    else:
+                        assert(info['private_keys_enabled'] == True)
+                        assert info['keypoolsize'] == 0
         else:
-            # Descriptor wallets appear to be corrupted wallets to old software
-            assert_raises_rpc_error(-4, "Wallet file verification failed: wallet.dat corrupt, salvage failed", node_v19.loadwallet, "w1")
-            assert_raises_rpc_error(-4, "Wallet file verification failed: wallet.dat corrupt, salvage failed", node_v19.loadwallet, "w2")
-            assert_raises_rpc_error(-4, "Wallet file verification failed: wallet.dat corrupt, salvage failed", node_v19.loadwallet, "w3")
-            assert_raises_rpc_error(-4, "Wallet file verification failed: wallet.dat corrupt, salvage failed", node_v18.loadwallet, "w1")
-            assert_raises_rpc_error(-4, "Wallet file verification failed: wallet.dat corrupt, salvage failed", node_v18.loadwallet, "w2")
-            assert_raises_rpc_error(-4, "Wallet file verification failed: wallet.dat corrupt, salvage failed", node_v18.loadwallet, "w3")
-
-        # Open the wallets in v0.17
-        node_v17.loadwallet("w1_v18")
-        wallet = node_v17.get_wallet_rpc("w1_v18")
-        info = wallet.getwalletinfo()
-        assert info['private_keys_enabled']
-        assert info['keypoolsize'] > 0
-
-        node_v17.loadwallet("w2_v18")
-        wallet = node_v17.get_wallet_rpc("w2_v18")
-        info = wallet.getwalletinfo()
-        assert info['private_keys_enabled'] == False
-        assert info['keypoolsize'] == 0
+            for node in legacy_nodes:
+                # Descriptor wallets appear to be corrupted wallets to old software
+                # and loadwallet is introduced in v0.17.0
+                if node.version >= 170000 and node.version < 210000:
+                    for wallet_name in ["w1", "w2", "w3"]:
+                        assert_raises_rpc_error(-4, "Wallet file verification failed: wallet.dat corrupt, salvage failed", node.loadwallet, wallet_name)
 
         # RPC loadwallet failure causes bitcoind to exit, in addition to the RPC
         # call failure, so the following test won't work:
-        # assert_raises_rpc_error(-4, "Wallet loading failed.", node_v17.loadwallet, 'w3_v18')
+        # assert_raises_rpc_error(-4, "Wallet loading failed.", node_v17.loadwallet, 'w3')
 
         # Instead, we stop node and try to launch it with the wallet:
-        self.stop_node(4)
-        node_v17.assert_start_raises_init_error(["-wallet=w3_v18"], "Error: Error loading w3_v18: Wallet requires newer version of Bitcoin Core")
+        self.stop_node(node_v17.index)
         if self.options.descriptors:
             # Descriptor wallets appear to be corrupted wallets to old software
             node_v17.assert_start_raises_init_error(["-wallet=w1"], "Error: wallet.dat corrupt, salvage failed")
@@ -323,23 +220,23 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
             node_v17.assert_start_raises_init_error(["-wallet=w3"], "Error: wallet.dat corrupt, salvage failed")
         else:
             node_v17.assert_start_raises_init_error(["-wallet=w3"], "Error: Error loading w3: Wallet requires newer version of Bitcoin Core")
-        self.start_node(4)
+        self.start_node(node_v17.index)
 
         if not self.options.descriptors:
             # Descriptor wallets break compatibility, only run this test for legacy wallets
             # Open most recent wallet in v0.16 (no loadwallet RPC)
-            self.restart_node(5, extra_args=["-wallet=w2"])
+            self.restart_node(node_v16.index, extra_args=["-wallet=w2"])
             wallet = node_v16.get_wallet_rpc("w2")
             info = wallet.getwalletinfo()
             assert info['keypoolsize'] == 1
 
         # Create upgrade wallet in v0.16
-        self.restart_node(-1, extra_args=["-wallet=u1_v16"])
+        self.restart_node(node_v16.index, extra_args=["-wallet=u1_v16"])
         wallet = node_v16.get_wallet_rpc("u1_v16")
         v16_addr = wallet.getnewaddress('', "bech32")
         v16_info = wallet.validateaddress(v16_addr)
         v16_pubkey = v16_info['pubkey']
-        self.stop_node(-1)
+        self.stop_node(node_v16.index)
 
         self.log.info("Test wallet upgrade path...")
         # u1: regular wallet, created with v0.17
@@ -371,7 +268,7 @@ class BackwardsCompatibilityTest(BitcoinTestFramework):
                 os.path.join(node_master_wallets_dir, "u1_v16"),
                 os.path.join(node_v16_wallets_dir, "wallets/u1_v16")
             )
-            self.start_node(-1, extra_args=["-wallet=u1_v16"])
+            self.start_node(node_v16.index, extra_args=["-wallet=u1_v16"])
             wallet = node_v16.get_wallet_rpc("u1_v16")
             info = wallet.validateaddress(v16_addr)
             assert_equal(info, v16_info)