test: add coverage for rpc error when trying to rescan beyond pruned data #25937

brunoerg commented at 6:40 PM on August 26, 2022: contributor

This PR adds test coverage for the following rpc error: https://github.com/bitcoin/bitcoin/blob/15692e2641592394bdd4da0a7c2d371de8e576dd/src/wallet/rpc/transactions.cpp#L896-L899

DrahtBot added the label Tests on Aug 26, 2022

jonatack commented at 5:47 PM on September 1, 2022: contributor

Concept ACK. The pruning tests are in the EXTENDED_SCRIPTS and take a long time to run on my 2-core toaster, so it would be easier/more practical to test this if the new test could be reached sooner (if only it could be in, say, wallet_transactiontime_rescan.py or test/functional/wallet_hd.py instead). I'll retry running this a bit later tonight or overnight.

in test/functional/feature_pruning.py:148 in 7852e8e44e outdated

 142 | @@ -143,6 +143,10 @@ def test_invalid_command_line_options(self):
 143 |              extra_args=['-prune=550', '-reindex-chainstate'],
 144 |          )
 145 |  
 146 | +    def test_rescan_blockchain(self):
 147 | +        self.restart_node(0, ["-prune=550"])
 148 | +        assert_raises_rpc_error(-1, "Can't rescan beyond pruned data. Use RPC call getblockchaininfo to determine your pruned height.", self.nodes[0].rescanblockchain)

maflcko commented at 7:45 AM on September 2, 2022:

This fails for me to run without wallet

test_framework.authproxy.JSONRPCException: Method not found (-32601)

jonatack commented at 9:40 AM on September 2, 2022:

It might be good to explain in a docstring or comment why this assert will fail here.

If helpful, here is the result of printing getblockchaininfo at this point.

    def test_rescan_blockchain(self):
        self.restart_node(0, ["-prune=550"])
+        import pprint
+        pprint.pprint(self.nodes[0].getblockchaininfo())
         assert_raises_rpc_error(-1, "Can't rescan beyond pruned data. Use RPC call getblockchaininfo to determine your pruned height.", self.nodes[0].rescanblockchain)

<details><summary>results (two runs)</summary><p>

2022-09-01T22:12:35.086000Z TestFramework (INFO): Test it's not possible to rescan beyond pruned data
{'automatic_pruning': True,
 'bestblockhash': '5e4cd87c756bfa94ee47f7ad64e7ccc2db3ca77ba214ac64ca121159388acb2c',
 'blocks': 1553,
 'chain': 'regtest',
 'chainwork': '0000000000000000000000000000000000000000000000000000000000000c24',
 'difficulty': Decimal('4.656542373906925E-10'),
 'headers': 1553,
 'initialblockdownload': False,
 'mediantime': 1662069944,
 'prune_target_size': 576716800,
 'pruned': True,
 'pruneheight': 1056,
 'size_on_disk': 461006996,
 'time': 1662069945,
 'verificationprogress': 1,
 'warnings': 'This is a pre-release test build - use at your own risk - do not '
             'use for mining or merchant applications'}
2022-09-01T22:12:35.847000Z TestFramework (INFO): Done

2022-09-01T23:00:43.821000Z TestFramework (INFO): Test it's not possible to rescan beyond pruned data
{'automatic_pruning': True,
 'bestblockhash': '7075780a24234273a3d53f0a73e0be233c0748fd3fe74c8beb6c6a7ea8ee9686',
 'blocks': 1553,
 'chain': 'regtest',
 'chainwork': '0000000000000000000000000000000000000000000000000000000000000c24',
 'difficulty': Decimal('4.656542373906925E-10'),
 'headers': 1553,
 'initialblockdownload': False,
 'mediantime': 1662072650,
 'prune_target_size': 576716800,
 'pruned': True,
 'pruneheight': 1056,
 'size_on_disk': 461006996,
 'time': 1662072651,
 'verificationprogress': 1,
 'warnings': 'This is a pre-release test build - use at your own risk - do not '
             'use for mining or merchant applications'}
2022-09-01T23:00:44.840000Z TestFramework (INFO): Done

</p></details>

brunoerg commented at 1:22 PM on September 5, 2022:

Yes, I missed that. test_rescan_blockchain should be called if wallet is compiled. Done.

jonatack commented at 9:43 AM on September 2, 2022: contributor

The test passed four times for me. Edit: did not try without the wallet.

test: add coverage for rpc error when trying to rescan beyond pruned data cca4f82b82

brunoerg force-pushed on Sep 5, 2022

brunoerg commented at 1:23 PM on September 5, 2022: contributor

Force-pushed: call test_rescan_blockchain only if the wallet is compiled.

aureleoules commented at 9:43 AM on September 13, 2022: member

ACK cca4f82b828669ae23f6ac64fb83e068b81ae189

aureleoules commented at 9:45 AM on September 13, 2022: member

is it possible to check that rescanblockchain starting from the pruned height works though?

DrahtBot commented at 4:51 AM on September 23, 2022: contributor

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

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	aureleoules, MarcoFalke
Concept ACK	jonatack

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.

brunoerg commented at 5:55 PM on September 23, 2022: contributor

is it possible to check that rescanblockchain starting from the pruned height works though? @aureleoules, I think so.

fanquake commented at 2:53 PM on February 17, 2023: member

@aureleoules, I think so.

Does that mean you are working on / implementing this?

achow101 requested review from maflcko on Apr 25, 2023

maflcko commented at 2:09 PM on April 26, 2023: member

lgtm ACK cca4f82b828669ae23f6ac64fb83e068b81ae189

DrahtBot removed review request from maflcko on Apr 26, 2023

fanquake merged this on Apr 27, 2023

fanquake closed this on Apr 27, 2023

sidhujag referenced this in commit 037622c95c on Apr 28, 2023

bitcoin locked this on Apr 26, 2024