Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 1 commit into from Apr 27, 2023

Conversation

brunoerg
Copy link
Contributor

This PR adds test coverage for the following rpc error:

// We can't rescan beyond non-pruned blocks, stop and throw an error
if (!pwallet->chain().hasBlocks(pwallet->GetLastBlockHash(), start_height, stop_height)) {
throw JSONRPCError(RPC_MISC_ERROR, "Can't rescan beyond pruned data. Use RPC call getblockchaininfo to determine your pruned height.");
}

@DrahtBot DrahtBot added the Tests label Aug 26, 2022
@jonatack
Copy link
Contributor

jonatack commented Sep 1, 2022

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.

Copy link
Contributor

@jonatack jonatack left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

@@ -143,6 +143,10 @@ def test_invalid_command_line_options(self):
extra_args=['-prune=550', '-reindex-chainstate'],
)

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

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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)
results (two runs)

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

@brunoerg
Copy link
Contributor Author

brunoerg commented Sep 5, 2022

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

Copy link
Member

@aureleoules aureleoules left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACK cca4f82

@aureleoules
Copy link
Member

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

@DrahtBot
Copy link
Contributor

DrahtBot commented Sep 23, 2022

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
Copy link
Contributor Author

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

@aureleoules, I think so.

@fanquake
Copy link
Member

@aureleoules, I think so.

Does that mean you are working on / implementing this?

@achow101 achow101 requested a review from maflcko April 25, 2023 16:22
@maflcko
Copy link
Member

maflcko commented Apr 26, 2023

@DrahtBot DrahtBot removed the request for review from maflcko April 26, 2023 14:09
@fanquake fanquake merged commit ba4076d into bitcoin:master Apr 27, 2023
sidhujag pushed a commit to syscoin/syscoin that referenced this pull request Apr 28, 2023
…rescan beyond pruned data

cca4f82 test: add coverage for rpc error when trying to rescan beyond pruned data (brunoerg)

Pull request description:

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

ACKs for top commit:
  MarcoFalke:
    lgtm ACK cca4f82
  aureleoules:
    ACK cca4f82

Tree-SHA512: 724a055e9f6cddf1935699e8769015115f24f6485a0bd87e8660072ee44a15c1bddfdda848acc101ea7184b7e65a33b5b0d80b563d2ba3ecdab7a631378d6476
@bitcoin bitcoin locked and limited conversation to collaborators Apr 26, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants