Fixes #34175
In feature_pruning, thewallet_test doesn't require any specific wallet functionality and this test is important for one of next ones (test_scanblocks_pruned). The reason is that it synchronizes the node 5 and, without this sync, test_scanblocks_pruned will fail since we expect scanblocks to fail due to Block not available (pruned data) and it doesn't happen without this sync.
<!--e57a25ab6845829454e8d69fc972939a-->
The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.
<!--006a51241073e994b41acfe9ec718e94-->
For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/34185.
<!--021abf342d371248e50ceaed478a90ca-->
See the guideline for information on the review process.
| Type | Reviewers |
|---|---|
| ACK | furszy, musaHaruna, w0xlt, achow101 |
| Stale ACK | bensig |
If your review is incorrectly listed, please copy-paste <code><!--meta-tag:bot-skip--></code> into the comment that the bot should ignore.
<!--5faf32d7da4f0f540f40219e4f7537a3-->
the
wallet_testdoesn't require any specific wallet functionality...
Perhaps consider giving this function a less confusing name?
if the function was meant to test wallet restarts during pruning, what about actually checking the wallet?
if the function was meant to test wallet restarts during pruning, what about actually checking the wallet?
Sounds good, I'm going to address it.
Force-pushed:
getwalletinfo).wallet_test to test_wallet_rescan.test_wallet_rescan entirely if there is no wallet compiled.
ACK fa49789
Tested on macOS with ENABLE_WALLET=OFF - feature_pruning.py passes.
nit: "benefit of synchronization" --> "benefit from the synchronization"
Rather than applying the first and third commits, I think it is better to isolate the tests and add the missing sync where it's actually needed. Making other tests depend on the wallet test internal code isn’t really a good idea. It makes it more complicate to reason about. We should be able to fix this by adding another self.sync_blocks([self.nodes[0], self.nodes[5]], wait=5, timeout=300) call in the right place.
We should be able to fix this by adding another self.sync_blocks([self.nodes[0], self.nodes[5]], wait=5, timeout=300) call in the right place.
Just a single sync_blocks call would not solve it, but anyway, I've changed the approach to a simpler one.
wallet_test to be done before calling test_wallet and the next tests that also need it (It avoids duplicated code and actions).wallet_test to test_wallet_rescan.353 | self.restart_node(2, extra_args=["-prune=550"]) 354 | - self.log.info("Success") 355 | + 356 | + wallet_info = self.nodes[2].getwalletinfo() 357 | + assert_equal(wallet_info["scanning"], False) 358 | + assert_equal(wallet_info["lastprocessedblock"]["height"], 1553)
assert_equal(wallet_info["lastprocessedblock"]["height"], self.nodes[2].getblockcount())
I will leave as is for now. Going to address it.
366 | self.restart_node(5, extra_args=["-prune=550", "-blockfilterindex=1"]) # restart to trigger rescan 367 | - self.log.info("Success") 368 | + 369 | + wallet_info = self.nodes[5].getwalletinfo() 370 | + assert_equal(wallet_info["scanning"], False) 371 | + assert_equal(wallet_info["lastprocessedblock"]["height"], 1553)
assert_equal(wallet_info["lastprocessedblock"]["height"], self.nodes[5].getblockcount())
Force-pushed addressing #34185 (review).
362 | self.restart_node(5, extra_args=["-prune=550", "-blockfilterindex=1"]) # restart to trigger rescan 363 | - self.log.info("Success") 364 | + 365 | + wallet_info = self.nodes[5].getwalletinfo() 366 | + assert_equal(wallet_info["scanning"], False) 367 | + assert_equal(wallet_info["lastprocessedblock"]["height"], self.nodes[2].getblockcount())
As node[5] syncs from node[0], I guess this should have been self.nodes[0}.getblockcount()
Yes, just addressed it.
utACK f892ee88ec2aedcb8bd03460739b3a05124287d8
Force-pushed addressing #34185 (review)
utACK 8fb5e5f41ddf550a78b1253184d79a107097815a
Tested ACK 8fb5e5f I was able to reproduce the issue on macOS 26.0 before the changes in this PR
% cmake -B build -DENABLE_WALLET=OFF
% cmake --build build -j 10
% ./build/test/functional/feature_pruning.py --tmpdir $(pwd)/tmp
2026-01-14T06:17:09.449618Z TestFramework (ERROR): Unexpected exception
Traceback (most recent call last):
File "/Users/musaharuna/bitcoin/test/functional/test_framework/test_framework.py", line 142, in main
self.run_test()
~~~~~~~~~~~~~^^
File "/Users/musaharuna/bitcoin/./build/test/functional/feature_pruning.py", line 481, in run_test
self.test_scanblocks_pruned()
~~~~~~~~~~~~~~~~~~~~~~~~~~~^^
File "/Users/musaharuna/bitcoin/./build/test/functional/feature_pruning.py", line 496, in test_scanblocks_pruned
assert_raises_rpc_error(-1, "Block not available (pruned data)", node.scanblocks,
~~~~~~~~~~~~~~~~~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
"start", [{"desc": f"raw({false_positive_spk.hex()})"}], 0, 0, "basic", {"filter_false_positives": True})
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
File "/Users/musaharuna/bitcoin/test/functional/test_framework/util.py", line 157, in assert_raises_rpc_error
assert try_rpc(code, message, fun, *args, **kwds), "No exception raised"
~~~~~~~^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
AssertionError: No exception raised
2026-01-14T06:17:09.504754Z TestFramework (INFO): Not stopping nodes as test failed. The dangling processes will be cleaned up later.
Test passed successfully after the changes in the PR.
Also tested when cmake -B build -DENABLE_WALLET=ON test passed successfully as well.
ACK 8fb5e5f41ddf550a78b1253184d79a107097815a
