test: add coverage for invalid parameter in `keypoolrefill` #26731

pull brunoerg wants to merge 1 commits into bitcoin:master from brunoerg:2022-12-keypool-invalid-parameter changing 1 files +1 −0

brunoerg commented at 7:44 PM on December 20, 2022: contributor

This PR adds test coverage for the following error: https://github.com/bitcoin/bitcoin/blob/dcdfd72861c09a7945b9facc3726177a2d06fa64/src/wallet/rpc/addresses.cpp#L339-L344
test: add coverage for invalid parameter in `keypoolrefill` f6511b9cac
DrahtBot commented at 7:44 PM on December 20, 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. A summary of reviews will appear here.
DrahtBot added the label Tests on Dec 20, 2022
in test/functional/wallet_keypool.py:136 in f6511b9cac
```
 132 | @@ -133,6 +133,7 @@ def run_test(self):
 133 |  
 134 |          nodes[0].walletpassphrase('test', 100)
 135 |          nodes[0].keypoolrefill(100)
 136 | +        assert_raises_rpc_error(-8, "Invalid parameter, expected valid size.", nodes[0].keypoolrefill, -1)
```
yusufsahinhamza commented at 9:43 PM on December 20, 2022:

@brunoerg I don't think this makes sense as keypoolrefill is clearly set to 100 above. There are also more assert_equal checks below that making sure keypoolrefill has valid size.

brunoerg commented at 10:28 PM on December 20, 2022:

Why not? I just calling keypoolrefill with an invalid parameter to test how it deals with it, this call doesn't interfere on the previous one (keypoolrefill(100)), itkeeps valid and working, that's why the test doesn't fail. Regardless where I put it, it doesn't interfere on the test. Am I missing anything?

yusufsahinhamza commented at 10:52 PM on December 20, 2022:

@brunoerg Then this assert doesn't actually "expects valid size", there are lot of places to put such calls as well, it just doesn't seem necessary for me and that's just my opinion.
fanquake commented at 9:59 AM on February 16, 2023: member

Doesn't seem to be agreement on doing this. Closing for now.
fanquake closed this on Feb 16, 2023
bitcoin locked this on Feb 16, 2024

Contributors

Labels

test: add coverage for invalid parameter in `keypoolrefill` #26731

Reviews