Merge bitcoin/bitcoin#30920: test: Remove 0.16.3 test from wallet_bac…

…kwards_compatibility.py

fae44c8 test: Remove 0.16.3 test from wallet_backwards_compatibility.py (MarcoFalke)

Pull request description:

  The test checks that any wallet created with current master can not be loaded with `v0.16.3`. This is interesting documentation, however it is probably not something to keep as a test, because:

  * It seems like an extremely unlikely (and unsupported) edge case that someone creates a wallet with master and then goes ahead to open it with a long EOL software version.
  * A better test would be the inverse: Create a wallet with `v0.16.3` and open it with current master. This is already tested in `wallet_upgradewallet.py`, where I've added an additional balance check before upgrading the `v0.16.3` wallet.
  * The test is intermittently failing when shutting down the `v0.16.3` node, for example in bitcoin/bitcoin#30875 (comment). The exact cause is unclear, but given that the test isn't worthy to keep, removing it will ensure that the error disappears.

ACKs for top commit:
  Sjors:
    utACK fae44c8
  fanquake:
    ACK fae44c8 - I agree that test seems to have past it's usefulness, and the fact that it otherwise causes intemittent issues is further reason to remove it.

Tree-SHA512: 85bf428e616e0880198c1a7529936520505d7fa87c2eeb87a0457f13b50a163accaf5f80f9364dea978f6bd14b0b5350cda88f49aa7584682c8b5e0b0b117724

test/functional/wallet_backwards_compatibility.py

@@ -33,7 +33,7 @@ def add_options(self, parser):
 
     def set_test_params(self):
         self.setup_clean_chain = True
-        self.num_nodes = 12
+        self.num_nodes = 11
         # Add new version after each release:
         self.extra_args = [
             ["-addresstype=bech32", "[email protected]"], # Pre-release: use to mine blocks. noban for immediate tx relay
@@ -47,7 +47,6 @@ def set_test_params(self):
             ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "[email protected]"], # v0.19.1
             ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=127.0.0.1"], # v0.18.1
             ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=127.0.0.1"], # v0.17.2
-            ["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=127.0.0.1", "-wallet=wallet.dat"], # v0.16.3
         ]
         self.wallet_names = [self.default_wallet_name]
 
@@ -68,7 +67,6 @@ def setup_nodes(self):
             190100,
             180100,
             170200,
-            160300,
         ])
 
         self.start_nodes()
@@ -133,18 +131,17 @@ def test_v19_addmultisigaddress(self):
     def run_test(self):
         node_miner = self.nodes[0]
         node_master = self.nodes[1]
-        node_v21 = self.nodes[self.num_nodes - 6]
-        node_v17 = self.nodes[self.num_nodes - 2]
-        node_v16 = self.nodes[self.num_nodes - 1]
+        node_v21 = self.nodes[self.num_nodes - 5]
+        node_v17 = self.nodes[self.num_nodes - 1]
 
         legacy_nodes = self.nodes[2:] # Nodes that support legacy wallets
-        legacy_only_nodes = self.nodes[-5:] # Nodes that only support legacy wallets
-        descriptors_nodes = self.nodes[2:-5] # Nodes that support descriptor wallets
+        legacy_only_nodes = self.nodes[-4:] # Nodes that only support legacy wallets
+        descriptors_nodes = self.nodes[2:-4] # Nodes that support descriptor wallets
 
         self.generatetoaddress(node_miner, COINBASE_MATURITY + 1, node_miner.getnewaddress())
 
         # Sanity check the test framework:
-        res = node_v16.getblockchaininfo()
+        res = node_v17.getblockchaininfo()
         assert_equal(res['blocks'], COINBASE_MATURITY + 1)
 
         self.log.info("Test wallet backwards compatibility...")
@@ -215,9 +212,6 @@ def run_test(self):
         # In descriptors wallet mode, run this test on the nodes that support descriptor wallets
         # In legacy wallets mode, run this test on the nodes that support legacy wallets
         for node in descriptors_nodes if self.options.descriptors else legacy_nodes:
-            if self.major_version_less_than(node, 17):
-                # loadwallet was introduced in v0.17.0
-                continue
             self.log.info(f"- {node.version}")
             for wallet_name in ["w1", "w2", "w3"]:
                 if self.major_version_less_than(node, 18) and wallet_name == "w3":
@@ -290,15 +284,6 @@ def run_test(self):
             node_v17.assert_start_raises_init_error(["-wallet=w3"], "Error: Error loading w3: Wallet requires newer version of Bitcoin Core")
         self.start_node(node_v17.index)
 
-        # No wallet created in master can be opened in 0.16
-        self.log.info("Test that wallets created in master are too new for 0.16")
-        self.stop_node(node_v16.index)
-        for wallet_name in ["w1", "w2", "w3"]:
-            if self.options.descriptors:
-                node_v16.assert_start_raises_init_error([f"-wallet={wallet_name}"], f"Error: {wallet_name} corrupt, salvage failed")
-            else:
-                node_v16.assert_start_raises_init_error([f"-wallet={wallet_name}"], f"Error: Error loading {wallet_name}: Wallet requires newer version of Bitcoin Core")
-
         # When descriptors are enabled, w1 cannot be opened by 0.21 since it contains a taproot descriptor
         if self.options.descriptors:
             self.log.info("Test that 0.21 cannot open wallet containing tr() descriptors")

test/functional/wallet_upgradewallet.py

@@ -185,6 +185,7 @@ def copy_split_hd():
         self.restart_node(0)
         copy_v16()
         wallet = node_master.get_wallet_rpc(self.default_wallet_name)
+        assert_equal(wallet.getbalance(), v16_3_balance)
         self.log.info("Test upgradewallet without a version argument")
         self.test_upgradewallet(wallet, previous_version=159900, expected_version=169900)
         # wallet should still contain the same balance