fuzz: several improvements to scriptpubkeyman harness #34969

brunoerg commented at 7:10 PM on March 31, 2026: contributor

The scriptpubkeyman harness is slow - I'm getting under 40 exec/s on my personal server. This PR solves the following issues that may be affecting the performance:

There are redundant IsMine calls
ConsumeCoins always create up to 10'000 coins which may cause slowness.
There are some expensive ops in a loop - e.g. One of the slow units I got on my server is about a repeated sequence of GetNewDestination calls (see flamegraph below).
We're always calling GetDescriptorString unconditionally. It looks like an inofensive function but this is extremely costly due to calls to ToNormalizedString/ToPrivateString functions that touch key operations.
Not related to slowness, but there is a bug on harness because we are aborting the execution when we cannot update the wallet descriptor, we can just skip and continue.

spkm-updated

DrahtBot added the label Fuzzing on Mar 31, 2026

DrahtBot commented at 7:11 PM on March 31, 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/34969.

Reviews

See the guideline for information on the review process. A summary of reviews will appear here.

Conflicts

No conflicts as of last run.

andrewtoth commented at 9:04 PM on March 31, 2026: contributor

Getting decent throughput when running on my laptop:

$ FUZZ=scriptpubkeyman ./build_fuzz_nosan/bin/fuzz ../qa-assets/fuzz_corpora/scriptpubkeyman/
...
[#3326974](/bitcoin-bitcoin/3326974/)	REDUCE cov: 14175 ft: 84867 corp: 5289/37Mb lim: 998620 exec/s: 1319 rss: 262Mb L: 15776/882699 MS: 1 EraseBytes-
[#3327186](/bitcoin-bitcoin/3327186/)	REDUCE cov: 14175 ft: 84867 corp: 5289/37Mb lim: 998620 exec/s: 1319 rss: 262Mb L: 961/882699 MS: 2 ChangeBit-EraseBytes-
[#3328687](/bitcoin-bitcoin/3328687/)	REDUCE cov: 14175 ft: 84867 corp: 5289/37Mb lim: 998620 exec/s: 1319 rss: 262Mb L: 1249/882699 MS: 1 EraseBytes-
...

How is your server configured? Do you see a big speedup after applying this patch?

brunoerg commented at 1:11 AM on April 1, 2026: contributor

Getting decent throughput when running on my laptop:

What are the specs? How did you build it? Building with --preset=libfuzzer on my Ubuntu machine (AMD Ryzen 9 7900 / 32GB RAM), I get around 20 exec/s. With this PR, on same machine, I could reach similar coverage from master with an avg of 150 exec/s.

andrewtoth commented at 3:07 AM on April 1, 2026: contributor

I'm on a i9-14900 with 96 GB RAM.

I was running with only fuzzer sanitizer. When --preset=libfuzzer that uses undefined,address,fuzzer. With those I get a lot slower. It started at at 39 but after a few minutes was up to 128 exec/s. I checked out this PR and then it started at 128 and after a few minutes was up to 385 exec/s. Not bad!

Running with only fuzzer sanitizer on this branch however didn't seem to speed up it was around 1100 exec/s.

brunoerg commented at 12:13 PM on April 1, 2026: contributor

I'm on a i9-14900 with 96 GB RAM.

I was running with only fuzzer sanitizer. When --preset=libfuzzer that uses undefined,address,fuzzer. With those I get a lot slower. It started at at 39 but after a few minutes was up to 128 exec/s. I checked out this PR and then it started at 128 and after a few minutes was up to 385 exec/s. Not bad!

Running with only fuzzer sanitizer on this branch however didn't seem to speed up it was around 1100 exec/s.

Ah, got it. This is expected, because ASan instruments most memory accesses in instrumented code and tracks heap allocations, while UBSan adds runtime checks for selected undefined behaviors. So operations like GetScriptPubKeys(), ConsumeCoins(), and some string operations, such as GetDescriptorString() can become more expensive under these sanitizers.

in src/wallet/test/fuzz/scriptpubkeyman.cpp:131 in 1d826e5b8c outdated

 126 | -                }
 127 | -            },
 128 |              [&] {
 129 |                  auto spks{spk_manager->GetScriptPubKeys()};
 130 |                  for (const CScript& spk : spks) {
 131 | -                    assert(spk_manager->IsMine(spk));

maflcko commented at 10:53 AM on April 10, 2026:

1d826e5b8c49f8ebb8acd84c50861ea65c4129f8: Can you explain this a bit better? Sure, it may be internally covered in MarkUnusedAddresses, but the goal of fuzz tests is not only to cover code, but also to assert on results or assumptions

brunoerg commented at 1:48 PM on April 13, 2026:

I've changed this commit to say that it's removing excessive IsMine calls (e.g. calling it for every single spk from GetScriptPubKeys. In a later commit, I add the assert back but in a smoothly way.

brunoerg force-pushed on Apr 13, 2026

DrahtBot added the label Needs rebase on Apr 29, 2026

brunoerg force-pushed on Apr 30, 2026

DrahtBot removed the label Needs rebase on Apr 30, 2026

DrahtBot added the label CI failed on Apr 30, 2026

DrahtBot commented at 7:58 PM on April 30, 2026: contributor

🚧 At least one of the CI tasks failed. <sub>Task iwyu: https://github.com/bitcoin/bitcoin/actions/runs/25183113515/job/73833297823</sub> <sub>LLM reason (✨ experimental): CI failed because the IWYU include/format check for src/util/feefrac.h generated a failure (^^^ ⚠️ Failure generated from IWYU).</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>

brunoerg force-pushed on Apr 30, 2026

DrahtBot removed the label CI failed on Apr 30, 2026

DrahtBot added the label Needs rebase on May 5, 2026

fuzz: spkm: remove excessive IsMine calls

Avoid calling IsMine for every single spkm from
GetScriptPubKeys(). Also, expands `MarkUnusedAddresses`
which covers it. In a later commit, we introduce the
assertion back in a better way.

71fda91d28

fuzz: add a num_coins parameter to ConsumeCoins

This new parameter is used to control the max number of coins
this function might create. It allows us to create less than
10'000 coins and then reduce the cost.

5af76bc3d1

fuzz: spkm: reduce number of created coins fbb3b77fc6

fuzz: spkm: move expensive ops out of the loop f760b6fd33

fuzz: spkm: do not call GetDescriptorString unconditionally

GetDescriptorString calls ToNormalizedString or ToPrivateString,
both of which rebuild the full descriptor string. This is expensive.

2a2327755f

fuzz: spkm: fix early return on updating the wallet descriptor 66bce7c354

fuzz: spkm: assert a spkm from GetScriptPubKeys() is mine 40786bb2f8

brunoerg force-pushed on May 5, 2026

brunoerg commented at 2:42 PM on May 5, 2026: contributor

Rebased

DrahtBot removed the label Needs rebase on May 5, 2026