#34969 fuzz: several improvements to scriptpubkeyman harness

ReviewabilityReviewsAgreementSize
#34969 brunoerg fuzz: several improvements to scriptpubkeyman harnessAuthor silent 74d0Strong0 + 71 tests
  • Speed up the scriptpubkeyman fuzz harness to improve test throughput and coverage
  • Prevent fuzz runs from aborting prematurely when wallet descriptor updates fail
  • Ready for review
  • Strong support with verified benchmark speedups and no remaining objections
  • Verified benchmark speedup from roughly 39 to 385 executions per second (andrewtoth)
  • Concept approval for fixing the early return and moving heavy work out of hot paths (pablomartin4btc)
  • Resolved concerns about weakened assertions by restoring checks outside the loop (maflcko, pablomartin4btc)
  • 0 lines added or modified outside tests
  • 71 lines added or modified in tests
  • 86 lines removed in total
  • 3 files, 7 commits
Test infrastructure#15 of 54
P3 · speedup
  • P3 because it delivers a three to ten times speedup for an existing slow fuzz harness
  • Fixes premature execution termination to enable deeper fuzz exploration
Ranking notes
Reviewability: ReadyStalePaused
Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets
Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs.
Size: lines added or modified outside tests, then in tests; greener = smaller.

Goal

This pull request optimizes the `scriptpubkeyman` fuzz harness and adds a `num_coins` limit parameter to the `ConsumeCoins` fuzz utility. It removes redundant `IsMine` calls, moves costly operations such as `GetNewDestination` and `GetDescriptorString` out of inner loops, reduces the coin generation ceiling from 10,000 to 5 for this harness, and fixes an early return that aborted execution when descriptor updates failed.

Problem: The `scriptpubkeyman` fuzz target ran very slowly under libfuzzer sanitizers (under 40 exec/s on standard development hardware) due to repetitive expensive descriptor string conversions and massive coin generations in loops. Additionally, an early return bug prematurely terminated executions when failing to update wallet descriptors, reducing fuzz exploration depth.

Reviewability

Stale · Author silent 74d

No author activity for 74 days; a review may wait a long time for a response.

Agreement

Strong

substantive support, no open objection (andrewtoth, pablomartin4btc)

Thread notes: Initial record. maflcko and pablomartin4btc raised nonblocking concerns regarding weakened IsMine assertions, both resolved after brunoerg restored the full check outside the loop.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
maflcko (member)objection, correctnessremoving assertions weakens fuzz test verification by not asserting on results or assumptionsfixed · fix pushed · v1, not yet re-checkedansweredyes2026-04-10: 'the goal of fuzz tests is not only to cover code, but also to assert on results or assumptions'
Status from: 2026-04-13: brunoerg: 'In a later commit, I add the assert back but in a smoothly way.'
pablomartin4btc (member)objection, correctnesschecking only one randomly picked script weakens invariant verification and drops the inverse direction checkfixed · fix pushed · v1, not yet re-checkedansweredyes2026-08-11: 'The original code asserted `IsMine` for *every* script in the set — a stronger invariant that would catch a bug where some (but not all) returned scripts fail `IsMine`'
Status from: 2026-08-19: brunoerg: 'Just changed it to full assertion since it's cheap.'

Support:

  • andrewtoth (member) (2026-04-01): confirmed benchmark speedup from ~39-128 exec/s to ~128-385 exec/s under libfuzzer presets
  • pablomartin4btc (member) (Concept ACK, 2026-08-11): A real improvement — the early return bug fix and moving expensive ops out of the hot path are the right calls.

Participants:

  • andrewtoth (member, support, 2 statements 2026-03-31 to 2026-04-01): benchmarked speedup under libfuzzer presets
  • maflcko (member, objection, 1 statements 2026-04-10 to 2026-04-10): concerned that removing IsMine calls dropped invariant assertions
  • ekzyis (contributor, question, 2 statements 2026-06-13 to 2026-06-26): asked about throughput discrepancies when running without qa-assets corpus
  • pablomartin4btc (member, objection, 3 statements 2026-08-10 to 2026-08-11): Concept ACK, but noted that weakening IsMine assertions lost invariant coverage

Files

157 lines under test/bench/ci.

  • src/wallet/test/fuzz/scriptpubkeyman.cpp +68/-83
  • src/test/fuzz/util.cpp +2/-2
  • src/test/fuzz/util.h +1/-1

Card

The short text the ranking pass compares across PRs.

This PR optimizes the scriptpubkeyman fuzz harness by removing redundant IsMine calls, limiting the number of generated coins, moving heavy descriptor operations out of the loop, and fixing an early harness exit on descriptor update failure. Fuzzing throughput improves by 3x to 10x under sanitizer builds (from ~40 exec/s up to 150-385 exec/s). The PR has a Concept ACK, verified benchmarks, and no open reviewer objections.

Data

dossier JSON · extract JSON · model ledger, generated 2026-09-18T03:26, confidence high, input hash 2ff5fc150192ee8f
thread read through 2026-08-19 (16 statements), head 0b0df80ba0
Discussion read under schema 1 rules: suggestions and questions may be missing, and an objection may combine several harms (schema history).
labels: Fuzzing

Categories

Test infrastructure

P3 · speedup

The PR optimizes a slow fuzz harness by moving heavy operations out of loops, achieving a verified 3x to 10x exec/s speedup, and fixes an early return bug that prematurely halted execution paths.

Membership: Carries the Fuzzing label and modifies the fuzz harness `src/wallet/test/fuzz/scriptpubkeyman.cpp` alongside common fuzz utility code in `src/test/fuzz/util.{cpp,h}`.

Factors: security/stability 0, bug 0, performance 1, user value 0, leverage 0