fuzz: Extend `scriptpubkeyman` coverage

Chand-ra commented at 12:04 pm on December 29, 2025: none

Add fuzzing coverage for several missing DescriptorScriptPubKeyMan methods.

DrahtBot added the label Fuzzing on Dec 29, 2025

DrahtBot commented at 12:04 pm on December 29, 2025: 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/34170.

Reviews

See the guideline for information on the review process.

Type	Reviewers
Stale ACK	bensig

If your review is incorrectly listed, please copy-paste <!–meta-tag:bot-skip–> into the comment that the bot should ignore.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#32876 (refactor: use options struct for signing and PSBT operations by Sjors)
#32857 (wallet: allow skipping script paths by Sjors)

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.

LLM Linter (✨ experimental)

Possible typos and grammar issues:

setup -> set up [Used as a verb in “cannot setup a new SPKM”; the correct verb form is “set up”]

Possible places where named args for integral literals may be used (e.g. func(x, /*named_arg=*/0) in C++, and func(x, named_arg=0) in Python):

FillPSBT(psbt, txdata, sighash_type, sign, bip32derivs, nullptr, finalize) in src/wallet/test/fuzz/scriptpubkeyman.cpp

2026-01-16

DrahtBot added the label CI failed on Dec 29, 2025

DrahtBot commented at 1:20 pm on December 29, 2025: contributor

🚧 At least one of the CI tasks failed. Task macOS native, fuzz: https://github.com/bitcoin/bitcoin/actions/runs/20572426083/job/59082228214 LLM reason (✨ experimental): Uncaught std::runtime_error “Unable to expand descriptor” in fuzz target scriptpubkeyman caused the CI failure.

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.

in src/wallet/test/fuzz/scriptpubkeyman.cpp:202 in a9af94b211

193@@ -194,6 +194,76 @@ FUZZ_TARGET(scriptpubkeyman, .init = initialize_spkm)
194                 auto bip32derivs = fuzzed_data_provider.ConsumeBool();
195                 auto finalize = fuzzed_data_provider.ConsumeBool();
196                 (void)spk_manager->FillPSBT(psbt, txdata, sighash_type, sign, bip32derivs, nullptr, finalize);
197+            },
198+            [&] {
199+                std::vector<unsigned char> key_bytes = ConsumeFixedLengthByteVector(fuzzed_data_provider, 32);
200+                CKeyingMaterial master_key(key_bytes.begin(), key_bytes.end());
201+                WalletBatch batch{wallet.GetDatabase()};
202+                (void)spk_manager->Encrypt(master_key, &batch);

brunoerg commented at 1:43 pm on December 29, 2025:

As far as I remember, we were avoiding to cover any function related to encryption/decryption because it makes the target even slower.

Chand-ra force-pushed on Dec 30, 2025

in src/wallet/test/fuzz/scriptpubkeyman.cpp:154 in 09f0cf628d

148@@ -149,7 +149,11 @@ FUZZ_TARGET(scriptpubkeyman, .init = initialize_spkm)
149                 auto spks{spk_manager->GetScriptPubKeys()};
150                 if (!spks.empty()) {
151                     auto& spk{PickValue(fuzzed_data_provider, spks)};
152-                    (void)spk_manager->MarkUnusedAddresses(spk);
153+                    try  {
154+                        (void)spk_manager->MarkUnusedAddresses(spk);
155+                    } catch (const std::runtime_error& e) {

brunoerg commented at 11:18 pm on December 30, 2025:

From CI (https://github.com/bitcoin/bitcoin/actions/runs/20591844481/job/59138283200?pr=34170):

0D:\a\bitcoin\bitcoin\src\wallet\test\fuzz\scriptpubkeyman.cpp(154,56): error C2220: the following warning is treated as an error [D:\a\bitcoin\bitcoin\build\src\test\fuzz\fuzz.vcxproj]
1D:\a\bitcoin\bitcoin\src\wallet\test\fuzz\scriptpubkeyman.cpp(154,56): warning C4101: 'e': unreferenced local variable [D:\a\bitcoin\bitcoin\build\src\test\fuzz\fuzz.vcxproj]
2D:\a\bitcoin\bitcoin\src\wallet\test\fuzz\scriptpubkeyman.cpp(259,52): warning C4101: 'e': unreferenced local variable [D:\a\bitcoin\bitcoin\build\src\test\fuzz\fuzz.vcxproj]
3D:\a\bitcoin\bitcoin\src\wallet\test\fuzz\scriptpubkeyman.cpp(284,52): warning C4101: 'e': unreferenced local variable [D:\a\bitcoin\bitcoin\build\src\test\fuzz\fuzz.vcxproj]
4D:\a\bitcoin\bitcoin\src\wallet\test\fuzz\scriptpubkeyman.cpp(291,52): warning C4101: 'e': unreferenced local variable [D:\a\bitcoin\bitcoin\build\src\test\fuzz\fuzz.vcxproj]

Chand-ra force-pushed on Dec 31, 2025

DrahtBot removed the label CI failed on Dec 31, 2025

Chand-ra requested review from brunoerg on Jan 6, 2026

bensig commented at 11:36 pm on January 7, 2026: contributor

ACK 9efc7963132bb82cbaac05f3cfdd0fbe6940ffb1

Tests ran.

in src/wallet/test/fuzz/scriptpubkeyman.cpp:155 in 9efc796313 outdated

148@@ -149,7 +149,11 @@ FUZZ_TARGET(scriptpubkeyman, .init = initialize_spkm)
149                 auto spks{spk_manager->GetScriptPubKeys()};
150                 if (!spks.empty()) {
151                     auto& spk{PickValue(fuzzed_data_provider, spks)};
152-                    (void)spk_manager->MarkUnusedAddresses(spk);
153+                    try  {
154+                        (void)spk_manager->MarkUnusedAddresses(spk);
155+                    } catch (const std::runtime_error&) {
156+                        // Expected failure when cache is inconsistent with map

brunoerg commented at 7:12 pm on January 15, 2026:

9efc7963132bb82cbaac05f3cfdd0fbe6940ffb1: Not sure if I agree with this approach. As far as I remember, the target is done in a way it will never reach that runtime_error. Can you explain this change?

brunoerg commented at 7:14 pm on January 15, 2026:

I’m tending to Approach NACK with this kind of change.

Chand-ra commented at 4:54 pm on January 16, 2026:

This try-catch (and the other ones added in this PR) is necessary is because this PR adds a new test case for SetCache(), which is supposed to simulate a scenario where a wallet loads a corrupted or partial cache. Since this is a runtime error thrown by the wallet logic itself (not a crash or assertion failure), catching it allows the fuzzer to verify that the wallet handles this state safely.

If that does not sound worthwhile, I can get rid of the SetCache() test case which should remove the need for try-catch blocks added in this PR.

in src/wallet/test/fuzz/scriptpubkeyman.cpp:243 in 9efc796313

238+                    // the active descriptor range.
239+                    uint32_t key_exp_index = fuzzed_data_provider.ConsumeIntegral<uint32_t>();
240+                    if (fuzzed_data_provider.ConsumeBool()) {
241+                        key_exp_index %= 100;
242+                    }
243+                    switch (fuzzed_data_provider.ConsumeIntegralInRange<int>(0, 2)) {

brunoerg commented at 7:12 pm on January 15, 2026:

9efc7963132bb82cbaac05f3cfdd0fbe6940ffb1: nit: You could use a CallOneOf.

Chand-ra force-pushed on Jan 16, 2026

fuzz: extend scriptpubkeyman coverage

Add fuzzing coverage for several missing `DescriptorScriptPubKeyMan`
methods.

0139fba131

Chand-ra force-pushed on Jan 16, 2026

DrahtBot added the label CI failed on Jan 16, 2026

DrahtBot removed the label CI failed on Jan 16, 2026

fuzz: Extend scriptpubkeyman coverage #34170

Code Coverage & Benchmarks

Reviews

Conflicts

LLM Linter (✨ experimental)

fuzz: Extend `scriptpubkeyman` coverage #34170