test: fuzz wallet_rpc target #35342

pull kevkevinpal wants to merge 2 commits into bitcoin:master from kevkevinpal:fuzzwalletrpc changing 6 files +530 −176

kevkevinpal commented at 3:35 AM on May 21, 2026: contributor

Summary

This is part of #29901 and a resurrection of #30570, which I had left up for grabs earlier.

This change adds a new fuzz target wallet_rpc that fuzz tests the wallet RPC

This PR also moves and modifies ConsumeScalarRPCArgument to be used for the wallet_rpc target

Right now, I see zero fuzz coverage for the wallet rpc, this new target will add some coverage there. https://storage.googleapis.com/oss-fuzz-coverage/bitcoin-core/reports/20260519/linux/src/bitcoin-core/src/wallet/rpc/report.html
test/refactor: move RPC fuzz helpers to util/rpc
Moved ConsumeScalarRPCArgument to its own fuzz util file
cf5829f16d
DrahtBot added the label Tests on May 21, 2026
DrahtBot commented at 3:35 AM on May 21, 2026: contributor


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



Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35342.



Reviews

See the guideline for information on the review process.

Type Reviewers

Approach NACK brunoerg

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.



Conflicts

Reviewers, this pull request conflicts with the following ones:
- #32993 (fuzz: wallet: add target for tx scanning by brunoerg)
If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.
kevkevinpal force-pushed on May 21, 2026
DrahtBot added the label CI failed on May 21, 2026
DrahtBot commented at 3:52 AM on May 21, 2026: contributor


🚧 At least one of the CI tasks failed. <sub>Task macOS native, fuzz: https://github.com/bitcoin/bitcoin/actions/runs/26203867808/job/77099459891</sub> <sub>LLM reason (✨ experimental): CI failed because the wallet_rpc fuzz target aborted with an error that RPC command "addhdkey" is missing from wallet_commands_safe_for_fuzzing/wallet_commands_not_safe_for_fuzzing (update wallet/test/fuzz/rpc.cpp).</sub>

<details><summary>Hints</summary>

Try to run the tests locally, according to the documentation. However, a CI failure may still happen due to a number of reasons, for example:
- Possibly due to a silent merge conflict (the changes in this pull request being incompatible with the current code in the target branch). If so, make sure to rebase on the latest commit of the target branch.
- A sanitizer issue, which can only be found by compiling with the sanitizer and running the affected test.
- An intermittent issue.
Leave a comment here, if you need help tracking down a confusing failure.

</details>

Type	Reviewers
Approach NACK	brunoerg

test: add wallet_rpc fuzz target

By reusing some of the logic for the rpc fuzz target this new fuzz
target wallet_rpc adds coverage to the wallet rpc calls.

9d2b44b752

kevkevinpal force-pushed on May 21, 2026
kevkevinpal commented at 4:25 AM on May 21, 2026: contributor

This is the coverage I was able to generate in the short time I had it running.

<img width="1069" height="178" alt="Screenshot 2026-05-20 at 11 23 11 PM" src="https://github.com/user-attachments/assets/da3b69cf-e2c7-45b7-8efc-2f735141cb1c" />
DrahtBot removed the label CI failed on May 21, 2026
brunoerg commented at 11:23 PM on May 21, 2026: contributor

I'm tending to approach NACK, will explain:

I understand the need of fuzzing the RPCs but I have been wondering about the efficacy of the approach like this. This is basically fuzzing the RPC without any other context that might affect the RPCs. I mean, what would be the efficacy of fuzzing the RPC listsinceblock if this harness is not producing blocks? Or even RPCs that need spendable coins.

In general, I think running it will not produce results as expected.
in src/wallet/test/fuzz/rpc.cpp:169 in 9d2b44b752
```
 164 | +}
 165 | +
 166 | +void SetupMinimalFuzzWallet(CWallet& wallet)
 167 | +{
 168 | +    LOCK(wallet.cs_wallet);
 169 | +    wallet.m_keypool_size = 1;
```
brunoerg commented at 11:25 PM on May 21, 2026:

We usually need to set the keypool size to 1 in wallet targets to avoid performance issues, and this is one more reason that fuzzing some of these RPCs would not have much effect (e.g. getnewaddress).
maflcko commented at 5:54 AM on May 22, 2026: member

Maybe the fuzz target can be allowed to call several wallet RPCs, but I am not sure what the performance of this would be?
kevkevinpal commented at 12:20 PM on May 22, 2026: contributor

Maybe the fuzz target can be allowed to call several wallet RPCs, but I am not sure what the performance of this would be?

Maybe I'm not understanding the comment, but in this change, there is a list of wallet RPCs to call, some marked as. safe and others marked as unsafe. https://github.com/bitcoin/bitcoin/pull/35342/changes#diff-8ae84e14f2db08a67e421373290af839a5490843b50423f7ab8256509888acd3R86-R150
kevkevinpal commented at 12:29 PM on May 22, 2026: contributor

This is basically fuzzing the RPC without any other context that might affect the RPCs. I mean, what would be the efficacy of fuzzing the RPC listsinceblock if this harness is not producing blocks? Or even RPCs that need spendable coins.

Gotcha, and thanks for the review. I do think that makes sense as well. It does make little sense to fuzz listsinceblock if no blocks are being made, and the same for some of the other RPCs.

Do you think it would make more sense to create individual harnesses for each RPC? So we can build a fuzz test suite that serves it better? I have a feeling that would be cumbersome on the fuzz test suite.

Do you think this same logic also applies to the existing rpc harness? I've tried to replicate it closely to that one.
brunoerg commented at 12:50 PM on May 22, 2026: contributor

Maybe the fuzz target can be allowed to call several wallet RPCs, but I am not sure what the performance of this would be?

Pretty sure the performance would be terrible.

Contributors

Labels

Linked (view graph)

#29901 Wallet fuzzing tracking issue #30570 [tests] New fuzz target wallet_rpc

test: fuzz wallet_rpc target #35342

Summary

Code Coverage & Benchmarks

Reviews

Conflicts