test: Remove 0.16.3 test from wallet_backwards

maflcko commented at 6:55 am on September 18, 2024: member

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 #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.

DrahtBot commented at 6:55 am on September 18, 2024: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	Sjors, fanquake

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

No conflicts as of last run.

DrahtBot added the label Tests on Sep 18, 2024

maflcko force-pushed on Sep 18, 2024

DrahtBot added the label CI failed on Sep 18, 2024

maflcko force-pushed on Sep 18, 2024

DrahtBot removed the label CI failed on Sep 18, 2024

test: Remove 0.16.3 test from wallet_backwards_compatibility.py fae44c83da

maflcko force-pushed on Sep 18, 2024

kevkevinpal commented at 4:26 pm on September 19, 2024: contributor

was there a seed value for #30875 (comment)?

I’m trying to look for it to try and reproduce but am having trouble finding it

[edit] never mind I was able to get the full logs to open up seed:8381474046628750392

maflcko commented at 4:43 pm on September 19, 2024: member

I’m trying to look for it to try and reproduce but am having trouble finding it

I think the logs were nuked in the meantime, which is a Cirrus bug :(

IIRC it may have been a shutdown error of the v0.16.3 software. However, I don’t think looking at the CI failure matters, because the test doesn’t make much sense and a more realistic test already exists.

maflcko commented at 4:46 pm on September 19, 2024: member

cc @Sjors from 89a28e02fa46f3d5eb07ab02aa34aa95c6fcee11

Sjors commented at 5:10 pm on September 19, 2024: member

Since we can longer create a legacy (bdb4) wallets and since older Bitcoin Core versions can’t load descriptor (sqlite3) wallets, it should be safe to drop certain older nodes from the downgrade tests.

Will study this PR later.

maflcko requested review from Sjors on Oct 1, 2024

maflcko commented at 3:18 pm on October 6, 2024: member

I keep seeing the shutdown bug, also locally:

 0 test  2024-10-03T15:31:32.242000Z TestFramework (INFO): Test that wallets created in master are too new for 0.16 
 1 test  2024-10-03T15:31:32.242000Z TestFramework.node11 (DEBUG): Stopping node 
 2 test  2024-10-03T15:31:32.340000Z TestFramework (ERROR): Unexpected exception caught during testing 
 3                                   Traceback (most recent call last):
 4                                     File "/ci_container_base/test/functional/test_framework/test_framework.py", line 132, in main
 5                                       self.run_test()
 6                                     File "/ci_container_base/ci/scratch/build-x86_64-pc-linux-gnu/test/functional/wallet_backwards_compatibility.py", line 295, in run_test
 7                                       self.stop_node(node_v16.index)
 8                                     File "/ci_container_base/test/functional/test_framework/test_framework.py", line 581, in stop_node
 9                                       self.nodes[i].stop_node(expected_stderr, wait=wait)
10                                     File "/ci_container_base/test/functional/test_framework/test_node.py", line 397, in stop_node
11                                       self.stop()
12                                     File "/ci_container_base/test/functional/test_framework/coverage.py", line 50, in __call__
13                                       return_val = self.auth_service_proxy_instance.__call__(*args, **kwargs)
14                                     File "/ci_container_base/test/functional/test_framework/authproxy.py", line 127, in __call__
15                                       response, status = self._request('POST', self.__url.path, postdata.encode('utf-8'))
16                                     File "/ci_container_base/test/functional/test_framework/authproxy.py", line 106, in _request
17                                       return self._get_response()
18                                     File "/ci_container_base/test/functional/test_framework/authproxy.py", line 164, in _get_response
19                                       http_response = self.__conn.getresponse()
20                                     File "/usr/lib/python3.10/http/client.py", line 1375, in getresponse
21                                       response.begin()
22                                     File "/usr/lib/python3.10/http/client.py", line 318, in begin
23                                       version, status, reason = self._read_status()
24                                     File "/usr/lib/python3.10/http/client.py", line 287, in _read_status
25                                       raise RemoteDisconnected("Remote end closed connection without"
26                                   http.client.RemoteDisconnected: Remote end closed connection without response

with the node log:

 0                                   2024-10-03 15:31:32.250748 Stopping RPC
 1                                   2024-10-03 15:31:32.250913 RPC stopped.
 2                                   2024-10-03 15:31:32.250936 Stopping HTTP server
 3                                   2024-10-03 15:31:32.250948 Waiting for HTTP worker threads to exit
 4                                   2024-10-03 15:31:32.251430 addcon thread exit
 5                                   2024-10-03 15:31:32.251520 Waiting for HTTP event thread to exit
 6                                   2024-10-03 15:31:32.259012 msghand thread exit
 7                                   2024-10-03 15:31:32.262571 Exited http event loop
 8                                   2024-10-03 15:31:32.337592 Stopped HTTP server
 9                                   2024-10-03 15:31:32.337684 CDBEnv::Flush: Flush(false)
10                                   2024-10-03 15:31:32.337703 CDBEnv::Flush: Flush(false) took               0ms
11                                   2024-10-03 15:31:32.337716 CDBEnv::Flush: Flush(false)
12                                   2024-10-03 15:31:32.337728 CDBEnv::Flush: Flush(false) took               0ms
13                                   2024-10-03 15:31:32.338399 net thread exit
14                                   2024-10-03 15:31:32.342600 Flushed 0 addresses to peers.dat  1ms
15                                   2024-10-03 15:31:32.356145 scheduler thread interrupt
16                                   2024-10-03 15:31:32.356809 Dumped mempool: 5e-06s to copy, 0.000187s to dump
17                                   2024-10-03 15:31:32.356856 Recorded 1 unconfirmed txs from mempool in 1.6e-05s
18                                   2024-10-03 15:31:32.358456 Writing final batch of 0.01 MiB
19                                   2024-10-03 15:31:32.358594 Committed 106 changed transaction outputs (out of 106) to coin database...
20                                   2024-10-03 15:31:32.359266 Writing final batch of 0.00 MiB
21                                   2024-10-03 15:31:32.359301 Committed 0 changed transaction outputs (out of 0) to coin database...
22                                   2024-10-03 15:31:32.359404 CDBEnv::Flush: Flush(true)
23                                   2024-10-03 15:31:32.359425 CDBEnv::Flush: Flushing 0 (refcount = 0)...
24                                   2024-10-03 15:31:32.359457 CDBEnv::Flush: 0 checkpoint
25                                   2024-10-03 15:31:32.359545 CDBEnv::Flush: 0 detach
26                                   2024-10-03 15:31:32.359675 CDBEnv::Flush: 0 closed
27                                   2024-10-03 15:31:32.359697 CDBEnv::Flush: Flushing wallet.dat (refcount = 0)...
28                                   2024-10-03 15:31:32.359721 CDBEnv::Flush: wallet.dat checkpoint
29                                   2024-10-03 15:31:32.359781 CDBEnv::Flush: wallet.dat detach
30                                   2024-10-03 15:31:32.359884 CDBEnv::Flush: wallet.dat closed
31                                   2024-10-03 15:31:32.359901 CDBEnv::Flush: Flush(true) took               0ms
32                                   2024-10-03 15:31:32.542072 CDBEnv::Flush: Flush(true) database not started
33                                   2024-10-03 15:31:32.542401 Shutdown: done

Sjors approved

Sjors commented at 7:55 am on October 7, 2024: member

utACK fae44c83da982095661b034bdd01afe8ac2fb0a6

fanquake approved

fanquake commented at 3:05 pm on October 8, 2024: member

ACK fae44c83da982095661b034bdd01afe8ac2fb0a6 - 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.

fanquake merged this on Oct 8, 2024

fanquake closed this on Oct 8, 2024

maflcko deleted the branch on Oct 8, 2024

test: Remove 0.16.3 test from wallet_backwards_compatibility.py #30920

Code Coverage

Reviews

Conflicts