Wallet fuzzing tracking issue #29901

The wallet has poor fuzz coverage. Hopefully, some work is being done to improve it. The goal of this issue is to actively track current work and work that needs to be done to improve fuzz coverage for the wallet.

Open PRs / Ready to review

• #34969
• #33916
• #32993
• #34026
• #32624
• #34230
• #31870
• #31806
• #29694
• #29936
• #30134
• #28074
• #32772
• #33210

Current wallet targets

See src/wallet/test/fuzz to see the existing wallet targets and https://maflcko.github.io/b-c-cov/fuzz.coverage/src/wallet/index.html for coverage.

Won't/Can't cover/Known issues

Legacy wallet removal

The goal is to remove legacy wallets and migrate them to descriptor ones. There is an open issue with a proposed timeline for Legacy Wallet and BDB removal (https://github.com/bitcoin/bitcoin/issues/20160). For this reason, we do not aim to increase fuzz coverage for legacy stuff. See that the scriptpubkeyman target only uses descriptor ones.

edit: legacy wallet has been removed.

External signer

I do believe we can't have fuzz coverage for external signer code.

Performance and determinism

Unfortunately, some aspects of the wallet may affect the fuzzing performance. E.g.:

• SetupDescriptorScriptPubKeyMans (it might also be non-deterministic due to key generation)
• Wallet encryption
• Keypool (we usually set it to 1 to avoid any issue)
• DB interactions

Wallet RPC

There were some PRs attempting to create a fuzz target for wallet RPCs. However, it's not straightforward to do it since many RPCs depend on other contexts (e.g. fuzzing the listsinceblock RPC without producing any blocks does not seem too efficient). Also, the performance is another issue to take in consideration.

Any ideas about it or PRs to add let me know.

Coverage is nice, but if the fuzz test never finds any real user-facing issues, it will be useless. I don't think any wallet fuzz target found a real issue yet? Obviously coverage is the first step toward adding regression fuzz tests, so here are some ideas:

• #23444 tried to add a regression fuzz test, but it was picked up as part of #28933, and I couldn't reproduce the regression crash by re-introducing it.
• #27271 would be another idea to check
• Something else?

SetupDescriptorScriptPubKeyMans (it might also be non-deterministic due to key generation)

I wrote the ImportDescriptors helper, which should be deterministic.

Edit: Also links to the current coverage:

• https://maflcko.github.io/b-c-cov/fuzz.coverage/src/wallet/index-sort-l.html
• https://storage.googleapis.com/oss-fuzz-coverage/bitcoin-core/reports/20240416/linux/src/bitcoin-core/src/wallet/report.html

Coverage is nice, but if the fuzz test never finds any real user-facing issues, it will be useless. I don't think any wallet fuzz target found a real issue yet?

I think we found a real issue with the coin selection target after the rework. But yes, it would be the only one.

Coverage is nice, but if the fuzz test never finds any real user-facing issues, it will be useless. I don't think any wallet fuzz target found a real issue yet? Obviously coverage is the first step toward adding regression fuzz tests, so here are some ideas:

• fuzz: Add regression test for wallet crash [#23444](/bitcoin-bitcoin/23444/) tried to add a regression fuzz test, but it was picked up as part of fuzz: Faster wallet_notifications target [#28933](/bitcoin-bitcoin/28933/), and I couldn't reproduce the regression crash by re-introducing it.
• RPC: Fix fund transaction crash when at 0-value, 0-fee [#27271](/bitcoin-bitcoin/27271/) would be another idea to check
• Something else?

I'm working on a target to cover and reproduce #27271.

Just added #29936 to the list. It's a regression to #27271.

Added #30134 to the list for review. It adds more coverage for ScriptPubKeyMan.

edit: merged

I started to take a look at the Wallet RPC issue, I'm still fairly new to fuzz testing so it might take longer but I am working off of this branch if you would like to track my progress.

I was able to get the coverage to

Lines: 32%
Functions 50%

I think I just need to run the fuzzer for longer to get better coverage but I can do more testing and also cleanup my branch. If you would like the qa-assets for wallet-rpc please let me know.

Given that the first segment of your seeds need to correspond to an RPC command, a dictionary might help with bootstrapping. I’m also not particularly knowledgeable about building fuzz tests, but it seems to me that it might be better to build a separate harness for each RPC?

Most fuzz engines will create the dictionary themselves, by extracting the string literals. If not, -use_value_profile=1 may be useful to "recover" the RPC name.

If you look at the coverage line-by-line, you will be able to see where the fuzz engine returns early. The right fix will then usually be obvious.

Removed #30570 since it's closed.

Not sure about performance issues, but a fuzz target to cover #31474 would be great?

Not sure about performance issues, but a fuzz target to cover #31474 would be great?

I'm working on it.

wallet_create_transaction and scriptpubkeyman are two targets that I'm taking a look to try to speed them up. I'm getting less than 10 exec/s for both of them on my personal server which is bad. Just added to the "Nice to have" section the "Speed up current targets".

Just added #33916 to the review list. This PR adds a fuzz target for the TransactionCanBeBumped function from feebumper.