Skip to content

Commit ff5b7b0

Browse files
committed
Merge bitcoin/bitcoin#32214: test: Remove fragile and ancient release 0.17 wallet test
fac978f test: Remove fragile and ancient release 0.17 wallet test (MarcoFalke) Pull request description: The test checks that the 0.17 wallet rejects wallet files created in "the future". This is nice, and good to know. However, * The 0.17 release is ancient and should be unused outside of tests, especially to load future wallets. * The test intermittently fails, due to ancient RPC server bugs, that were fixed in the meantime. [1] * Albeit they are not identical, the 0.18 release is still checked in this test, so any theoretical bug that would be caught by 0.17 is hopefully still caught by 0.18 as well. So fix all issues by removing the test case. [1] For example from https://api.cirrus-ci.com/v1/task/6161588714995712/logs/ci.log: ``` 190/321 - [1mwallet_backwards_compatibility.py --descriptors[0m failed, Duration: 23 s [17:21:40.700] [17:21:40.700] [1mstdout: [17:21:40.700] [0m2025-04-02T21:21:16.575000Z TestFramework (INFO): PRNG seed is: 5772716217847090743 [17:21:40.700] 2025-04-02T21:21:16.580000Z TestFramework (INFO): Initializing test directory /ci_container_base/ci/scratch/test_runner/test_runner_₿_🏃_20250402_210134/wallet_backwards_compatibility_134 [17:21:40.700] 2025-04-02T21:21:26.378000Z TestFramework (INFO): Test wallet backwards compatibility... [17:21:40.700] 2025-04-02T21:21:33.191000Z TestFramework (INFO): Testing 0.19 addmultisigaddress case (#18075) [17:21:40.700] 2025-04-02T21:21:33.637000Z TestFramework (INFO): Test that a wallet made on master can be opened on: [17:21:40.700] 2025-04-02T21:21:33.637000Z TestFramework (INFO): - 250000 [17:21:40.700] 2025-04-02T21:21:34.055000Z TestFramework (INFO): - 240001 [17:21:40.700] 2025-04-02T21:21:34.435000Z TestFramework (INFO): - 230000 [17:21:40.700] 2025-04-02T21:21:34.858000Z TestFramework (INFO): - 220000 [17:21:40.700] 2025-04-02T21:21:35.614000Z TestFramework (INFO): - 210000 [17:21:40.700] 2025-04-02T21:21:35.707000Z TestFramework (INFO): Test descriptor wallet incompatibility on: [17:21:40.700] 2025-04-02T21:21:35.707000Z TestFramework (INFO): - 200100 [17:21:40.700] 2025-04-02T21:21:35.878000Z TestFramework (INFO): - 190100 [17:21:40.700] 2025-04-02T21:21:36.021000Z TestFramework (INFO): - 180100 [17:21:40.700] 2025-04-02T21:21:36.319000Z TestFramework (INFO): Test descriptor wallet incompatibility with 0.17 [17:21:40.700] 2025-04-02T21:21:37.328000Z TestFramework (INFO): Test that 0.21 cannot open wallet containing tr() descriptors [17:21:40.700] 2025-04-02T21:21:37.356000Z TestFramework (INFO): Test that a wallet can upgrade to and downgrade from master, from: [17:21:40.700] 2025-04-02T21:21:37.361000Z TestFramework (INFO): - 250000 [17:21:40.700] 2025-04-02T21:21:37.665000Z TestFramework (INFO): - 240001 [17:21:40.700] 2025-04-02T21:21:37.970000Z TestFramework (INFO): - 230000 [17:21:40.700] 2025-04-02T21:21:38.439000Z TestFramework (INFO): - 220000 [17:21:40.700] 2025-04-02T21:21:38.793000Z TestFramework (INFO): - 210000 [17:21:40.700] 2025-04-02T21:21:39.470000Z TestFramework (INFO): Stopping nodes [17:21:40.700] [17:21:40.700] [17:21:40.700] [1mstderr: [17:21:40.700] [0mTraceback (most recent call last): [17:21:40.700] File "/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/test/functional/wallet_backwards_compatibility.py", line 389, in <module> [17:21:40.700] BackwardsCompatibilityTest(__file__).main() [17:21:40.700] File "/ci_container_base/test/functional/test_framework/test_framework.py", line 206, in main [17:21:40.700] exit_code = self.shutdown() [17:21:40.700] File "/ci_container_base/test/functional/test_framework/test_framework.py", line 379, in shutdown [17:21:40.700] self.stop_nodes() [17:21:40.700] File "/ci_container_base/test/functional/test_framework/test_framework.py", line 643, in stop_nodes [17:21:40.700] node.stop_node(wait=wait, wait_until_stopped=False) [17:21:40.700] File "/ci_container_base/test/functional/test_framework/test_node.py", line 397, in stop_node [17:21:40.700] self.stop() [17:21:40.700] File "/ci_container_base/test/functional/test_framework/coverage.py", line 50, in __call__ [17:21:40.700] return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs) [17:21:40.700] File "/ci_container_base/test/functional/test_framework/authproxy.py", line 132, in __call__ [17:21:40.700] response, status = self._request('POST', self.__url.path, postdata.encode('utf-8')) [17:21:40.700] File "/ci_container_base/test/functional/test_framework/authproxy.py", line 106, in _request [17:21:40.700] return self._get_response() [17:21:40.700] File "/ci_container_base/test/functional/test_framework/authproxy.py", line 169, in _get_response [17:21:40.700] http_response = self.__conn.getresponse() [17:21:40.700] File "/usr/lib/python3.10/http/client.py", line 1375, in getresponse [17:21:40.700] response.begin() [17:21:40.700] File "/usr/lib/python3.10/http/client.py", line 318, in begin [17:21:40.700] version, status, reason = self._read_status() [17:21:40.700] File "/usr/lib/python3.10/http/client.py", line 287, in _read_status [17:21:40.700] raise RemoteDisconnected("Remote end closed connection without" [17:21:40.700] http.client.RemoteDisconnected: Remote end closed connection without response [17:21:40.700] [node 10] Cleaning up leftover process [17:21:40.700] [node 9] Cleaning up leftover process [17:21:40.700] [node 8] Cleaning up leftover process [17:21:40.700] [node 7] Cleaning up leftover process [17:21:40.700] [node 6] Cleaning up leftover process [17:21:40.700] [node 5] Cleaning up leftover process [17:21:40.700] [node 4] Cleaning up leftover process [17:21:40.700] [node 3] Cleaning up leftover process [17:21:40.700] [node 2] Cleaning up leftover process [17:21:40.700] [node 1] Cleaning up leftover process [17:21:40.700] [node 0] Cleaning up leftover process ACKs for top commit: laanwj: Code review ACK fac978f janb84: Re ACK [fac978f](bitcoin/bitcoin@fac978f) pablomartin4btc: re ACK fac978f BrandonOdiwuor: Code Review ACK fac978f Tree-SHA512: 13acdfc6be4293a0ff45ae20b26ba60636e130097da380b7b51716faaa950320462399bef55e74b3cedc82944586dcc1bfd078babb96edb03c4efdb8f40af5a4
2 parents 873a45f + fac978f commit ff5b7b0

File tree

1 file changed

+28
-76
lines changed

1 file changed

+28
-76
lines changed

test/functional/wallet_backwards_compatibility.py

Lines changed: 28 additions & 76 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,5 @@
11
#!/usr/bin/env python3
2-
# Copyright (c) 2018-2022 The Bitcoin Core developers
2+
# Copyright (c) 2018-present The Bitcoin Core developers
33
# Distributed under the MIT software license, see the accompanying
44
# file COPYING or http://www.opensource.org/licenses/mit-license.php.
55
"""Backwards compatibility functional test
@@ -33,7 +33,7 @@ def add_options(self, parser):
3333

3434
def set_test_params(self):
3535
self.setup_clean_chain = True
36-
self.num_nodes = 11
36+
self.num_nodes = 10
3737
# Add new version after each release:
3838
self.extra_args = [
3939
["-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # Pre-release: use to mine blocks. noban for immediate tx relay
@@ -46,7 +46,6 @@ def set_test_params(self):
4646
["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v0.20.1
4747
["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=noban@127.0.0.1"], # v0.19.1
4848
["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=127.0.0.1"], # v0.18.1
49-
["-nowallet", "-walletrbf=1", "-addresstype=bech32", "-whitelist=127.0.0.1"], # v0.17.2
5049
]
5150
self.wallet_names = [self.default_wallet_name]
5251

@@ -66,7 +65,6 @@ def setup_nodes(self):
6665
200100,
6766
190100,
6867
180100,
69-
170200,
7068
])
7169

7270
self.start_nodes()
@@ -97,7 +95,7 @@ def test_v19_addmultisigaddress(self):
9795
# See #18075
9896
self.log.info("Testing 0.19 addmultisigaddress case (#18075)")
9997
node_master = self.nodes[1]
100-
node_v19 = self.nodes[self.num_nodes - 4]
98+
node_v19 = self.nodes[self.num_nodes - 3]
10199
node_v19.rpc.createwallet(wallet_name="w1_v19")
102100
wallet = node_v19.get_wallet_rpc("w1_v19")
103101
info = wallet.getwalletinfo()
@@ -131,17 +129,17 @@ def test_v19_addmultisigaddress(self):
131129
def run_test(self):
132130
node_miner = self.nodes[0]
133131
node_master = self.nodes[1]
134-
node_v21 = self.nodes[self.num_nodes - 5]
135-
node_v17 = self.nodes[self.num_nodes - 1]
132+
node_v21 = self.nodes[self.num_nodes - 4]
133+
node_v18 = self.nodes[self.num_nodes - 1]
136134

137135
legacy_nodes = self.nodes[2:] # Nodes that support legacy wallets
138-
legacy_only_nodes = self.nodes[-4:] # Nodes that only support legacy wallets
139-
descriptors_nodes = self.nodes[2:-4] # Nodes that support descriptor wallets
136+
legacy_only_nodes = self.nodes[-3:] # Nodes that only support legacy wallets
137+
descriptors_nodes = self.nodes[2:-3] # Nodes that support descriptor wallets
140138

141139
self.generatetoaddress(node_miner, COINBASE_MATURITY + 1, node_miner.getnewaddress())
142140

143141
# Sanity check the test framework:
144-
res = node_v17.getblockchaininfo()
142+
res = node_v18.getblockchaininfo()
145143
assert_equal(res['blocks'], COINBASE_MATURITY + 1)
146144

147145
self.log.info("Test wallet backwards compatibility...")
@@ -201,11 +199,7 @@ def run_test(self):
201199
for wallet in os.listdir(node_master_wallets_dir):
202200
dest = node.wallets_path / wallet
203201
source = node_master_wallets_dir / wallet
204-
if self.major_version_equals(node, 16):
205-
# 0.16 node expect the wallet to be in the wallet dir but as a plain file rather than in directories
206-
shutil.copyfile(source / "wallet.dat", dest)
207-
else:
208-
shutil.copytree(source, dest)
202+
shutil.copytree(source, dest)
209203

210204
self.test_v19_addmultisigaddress()
211205

@@ -215,9 +209,6 @@ def run_test(self):
215209
for node in descriptors_nodes if self.options.descriptors else legacy_nodes:
216210
self.log.info(f"- {node.version}")
217211
for wallet_name in ["w1", "w2", "w3"]:
218-
if self.major_version_less_than(node, 18) and wallet_name == "w3":
219-
# Blank wallets were introduced in v0.18.0. We test the loading error below.
220-
continue
221212
if self.major_version_less_than(node, 22) and wallet_name == "w1" and self.options.descriptors:
222213
# Descriptor wallets created after 0.21 have taproot descriptors which 0.21 does not support, tested below
223214
continue
@@ -261,30 +252,12 @@ def run_test(self):
261252
if self.options.descriptors:
262253
self.log.info("Test descriptor wallet incompatibility on:")
263254
for node in legacy_only_nodes:
264-
# RPC loadwallet failure causes bitcoind to exit in <= 0.17, in addition to the RPC
265-
# call failure, so the following test won't work:
266-
# assert_raises_rpc_error(-4, "Wallet loading failed.", node_v17.loadwallet, 'w3')
267-
if self.major_version_less_than(node, 18):
268-
continue
269255
self.log.info(f"- {node.version}")
270256
# Descriptor wallets appear to be corrupted wallets to old software
271-
assert self.major_version_at_least(node, 18) and self.major_version_less_than(node, 21)
257+
assert self.major_version_less_than(node, 21)
272258
for wallet_name in ["w1", "w2", "w3"]:
273259
assert_raises_rpc_error(-4, "Wallet file verification failed: wallet.dat corrupt, salvage failed", node.loadwallet, wallet_name)
274260

275-
# Instead, we stop node and try to launch it with the wallet:
276-
self.stop_node(node_v17.index)
277-
if self.options.descriptors:
278-
self.log.info("Test descriptor wallet incompatibility with 0.17")
279-
# Descriptor wallets appear to be corrupted wallets to old software
280-
node_v17.assert_start_raises_init_error(["-wallet=w1"], "Error: wallet.dat corrupt, salvage failed")
281-
node_v17.assert_start_raises_init_error(["-wallet=w2"], "Error: wallet.dat corrupt, salvage failed")
282-
node_v17.assert_start_raises_init_error(["-wallet=w3"], "Error: wallet.dat corrupt, salvage failed")
283-
else:
284-
self.log.info("Test blank wallet incompatibility with v17")
285-
node_v17.assert_start_raises_init_error(["-wallet=w3"], "Error: Error loading w3: Wallet requires newer version of Bitcoin Core")
286-
self.start_node(node_v17.index)
287-
288261
# When descriptors are enabled, w1 cannot be opened by 0.21 since it contains a taproot descriptor
289262
if self.options.descriptors:
290263
self.log.info("Test that 0.21 cannot open wallet containing tr() descriptors")
@@ -294,20 +267,13 @@ def run_test(self):
294267
for node in descriptors_nodes if self.options.descriptors else legacy_nodes:
295268
self.log.info(f"- {node.version}")
296269
wallet_name = f"up_{node.version}"
297-
if self.major_version_less_than(node, 17):
298-
# createwallet is only available in 0.17+
299-
self.restart_node(node.index, extra_args=[f"-wallet={wallet_name}"])
300-
wallet_prev = node.get_wallet_rpc(wallet_name)
301-
address = wallet_prev.getnewaddress('', "bech32")
302-
addr_info = wallet_prev.validateaddress(address)
270+
if self.major_version_at_least(node, 21):
271+
node.rpc.createwallet(wallet_name=wallet_name, descriptors=self.options.descriptors)
303272
else:
304-
if self.major_version_at_least(node, 21):
305-
node.rpc.createwallet(wallet_name=wallet_name, descriptors=self.options.descriptors)
306-
else:
307-
node.rpc.createwallet(wallet_name=wallet_name)
308-
wallet_prev = node.get_wallet_rpc(wallet_name)
309-
address = wallet_prev.getnewaddress('', "bech32")
310-
addr_info = wallet_prev.getaddressinfo(address)
273+
node.rpc.createwallet(wallet_name=wallet_name)
274+
wallet_prev = node.get_wallet_rpc(wallet_name)
275+
address = wallet_prev.getnewaddress('', "bech32")
276+
addr_info = wallet_prev.getaddressinfo(address)
311277

312278
hdkeypath = addr_info["hdkeypath"].replace("'", "h")
313279
pubkey = addr_info["pubkey"]
@@ -317,10 +283,7 @@ def run_test(self):
317283
wallet_prev.backupwallet(backup_path)
318284

319285
# Remove the wallet from old node
320-
if self.major_version_at_least(node, 17):
321-
wallet_prev.unloadwallet()
322-
else:
323-
self.stop_node(node.index)
286+
wallet_prev.unloadwallet()
324287

325288
# Restore the wallet to master
326289
load_res = node_master.restorewallet(wallet_name, backup_path)
@@ -362,28 +325,17 @@ def run_test(self):
362325

363326
wallet.unloadwallet()
364327

365-
# Check that no automatic upgrade broke the downgrading the wallet
366-
if self.major_version_less_than(node, 17):
367-
# loadwallet is only available in 0.17+
368-
shutil.copyfile(
369-
down_backup_path,
370-
node.wallets_path / down_wallet_name
371-
)
372-
self.start_node(node.index, extra_args=[f"-wallet={down_wallet_name}"])
373-
wallet_res = node.get_wallet_rpc(down_wallet_name)
374-
info = wallet_res.validateaddress(address)
375-
assert_equal(info, addr_info)
376-
else:
377-
target_dir = node.wallets_path / down_wallet_name
378-
os.makedirs(target_dir, exist_ok=True)
379-
shutil.copyfile(
380-
down_backup_path,
381-
target_dir / "wallet.dat"
382-
)
383-
node.loadwallet(down_wallet_name)
384-
wallet_res = node.get_wallet_rpc(down_wallet_name)
385-
info = wallet_res.getaddressinfo(address)
386-
assert_equal(info, addr_info)
328+
# Check that no automatic upgrade broke downgrading the wallet
329+
target_dir = node.wallets_path / down_wallet_name
330+
os.makedirs(target_dir, exist_ok=True)
331+
shutil.copyfile(
332+
down_backup_path,
333+
target_dir / "wallet.dat"
334+
)
335+
node.loadwallet(down_wallet_name)
336+
wallet_res = node.get_wallet_rpc(down_wallet_name)
337+
info = wallet_res.getaddressinfo(address)
338+
assert_equal(info, addr_info)
387339

388340
if __name__ == '__main__':
389341
BackwardsCompatibilityTest(__file__).main()

0 commit comments

Comments
 (0)