#36073 fuzz: Add musig harness

ReviewabilityReviewsAgreementSize
#36073 nervana21 fuzz: Add musig harnessReady0Strong0 + 227 tests
  • Add dedicated fuzz testing coverage for MuSig module wrapper APIs
  • Exercises key aggregation, nonce generation, and signing fail paths against malformed inputs
  • Ready for review
  • Nothing known that would invalidate a review now
  • Substantive support with no open objections
  • Concept approval because the signing functions currently have no unit tests (jeanpablojp)
  • Concept approval without stated reasons (brunoerg)
  • Resolved an abort on random secnonces that broke oss-fuzz runs (jeanpablojp)
  • 0 lines added or modified outside tests
  • 227 lines added or modified in tests
  • 0 lines removed in total
  • 2 files, 1 commits
Test infrastructure#23 of 54
P3 · test coverage
  • P3 because it adds fuzz coverage for MuSig signing and aggregation APIs that lacked unit tests
  • Exercises input validation and edge cases against malformed inputs in cryptographic wrappers
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

Adds a dedicated fuzz test harness for the `src/musig` module. The new target exercises public key aggregation (with and without cache/expected pubkeys), synthetic xpub creation, session ID generation, nonce generation, partial signature creation, and signature aggregation.

Problem: The wrapper APIs in `src/musig` lacked dedicated fuzz testing coverage, leaving error handling, input validation, and edge cases underexercised against malformed inputs.

Evidence of importance:

  • PR description: 'Previously, the module APIs had no dedicated harness, so fail paths were underexercised.'

Risk of a mistake: low (tests). Only introduces a new test harness, where a flaw would at most cause false-positive fuzzing failures in test environments.

Changed since the previous assessment: No structural changes in approach; the target continues to exercise MuSig2 key aggregation, session and synthetic xpub utilities, and partial-to-aggregate signing pipelines.

Reviewability

Ready

Nothing known that would invalidate a review now.

Waiting on (from the thread read):

  • reviewer: brunoerg to re-review after author addressed feedback and pushed changes 2026-10-26

Agreement

Strong

substantive support, no open objection (jeanpablojp)

Thread notes: The author rebased, implemented brunoerg's requested checks on secnonces and pubkey aggregation, and answered brunoerg's question on length control flags.

Objections, suggestions, and questions:

ReviewerTypeHarm, request, or questionStatusMust beAuthor repliedQuote
jeanpablojp (contributor)objection, correctnessThe fuzz harness aborts on random input due to libsecp256k1 illegal argument assertions when passing never-generated secnonces, breaking oss-fuzz runsfixed · fix pushed · call dropped from the harnesschangedyes2026-09-08: 'The target aborts on 8 of 1000 files, with `[libsecp256k1] illegal argument: secp256k1_memcmp_var(&secnonce->data[0], secp256k1_musig_secnonce_magic, 4) == 0`.'
Status from: 2026-09-08: nervana21: 'Yes, I think this is a better approach. I've dropped that call from this harness.'
jeanpablojp (contributor)suggestion, approachGenerate participant keys and nonces via CreateMuSig2Nonce and feed partial signatures into aggregation so multi-participant paths are exercised, keeping raw bytes as a separate casefixed · fix pushed · author implemented the suggestion in the pushansweredyes2026-09-08: 'Worth going that way and keeping the raw bytes as a separate case, so the parse failure stays reachable?'
Status from: 2026-09-08: nervana21: 'Ahh, makes sense, thanks! I think I've understood and taken your suggestions.'
jeanpablojp (contributor)suggestion, maintenanceDrop the ser length check as ser is always 33 bytes when expected has a valuefixed · fix pushed · check removed as requestedansweredyes2026-09-08: 'This `ser` only exists when `expected` has a value, and then it comes from the aggregate pubkey, which is always 33 bytes. Could the check go?'
Status from: 2026-09-08: nervana21: 'Done.'
brunoerg (member)suggestion, correctnessAssert that CreateMuSig2AggregateSig succeeds once every partial signature has succeededfixed · fix pushed · implemented in pushansweredyes2026-10-21: 'I think this target could be improved to check that once every partial signature has succeeded, the aggregate then succeeds.'
Status from: 2026-10-26: nervana21: 'Addressed @brunoerg's feedback'
brunoerg (member)suggestion, correctnessCheck that the secnonce is valid after a successful CreateMuSig2PartialSigfixed · fix pushed · implemented in pushansweredyes2026-10-21: 'I think the target could check that the `secnonce` is valid after a successful `CreateMuSig2PartialSig`.'
Status from: 2026-10-26: nervana21: 'Addressed @brunoerg's feedback'
brunoerg (member)suggestion, correctnessCheck that MuSig2AggregatePubkeys is rejected when given a wrong expected aggregatefixed · fix pushed · implemented in pushansweredyes2026-10-21: 'It could check the call is rejected when it creates a wrong expected aggregate?'
Status from: 2026-10-26: nervana21: 'Addressed @brunoerg's feedback'
brunoerg (member)question, approachHas the author tested the target with -max_len=4096 -len_control=0 to ensure tweaks are reachable?answered · author confirmed testing with those flags and explained why the current gate was keptansweredyes2026-10-21: 'Have you tried with `-max_len=4096 -len_control=0`?'
Status from: 2026-10-26: nervana21: 'When running from scratch with `-max_len=4096` and `-len_control=0`, I was able to reach the tweaks faster than without those flags.'

Support:

  • jeanpablojp (contributor) (Concept ACK, 2026-09-08) [read]: The four signing functions in the module have no unit test today, only the `wallet_musig.py` functional test, so the harness covers ground that was empty.
  • brunoerg (member) (Concept ACK, 2026-10-21): (no reason given) [not substantive]

Participants:

  • jeanpablojp (contributor, objection, 4 statements 2026-09-08 to 2026-09-08): Concept ACK, but objected that ungenerated secnonces cause assertion aborts and suggested improving multi-participant paths
  • brunoerg (member, support, 6 statements 2026-10-21 to 2026-10-21): Concept ACK; suggested additional assertions on signature and pubkey aggregation, and asked about length control

Files

227 lines under test/bench/ci.

  • src/test/fuzz/musig.cpp +226/-0
  • src/test/fuzz/CMakeLists.txt +1/-0

Card

The short text the ranking pass compares across PRs.

Adds a dedicated fuzz harness for the `src/musig` module. The harness exercises public key aggregation, synthetic xpub generation, session IDs, and multi-party signing flows under both valid and fuzzed inputs. Previously, the wrapper APIs lacked fuzz coverage, leaving error paths and input validation underexercised.

Data

dossier JSON · extract JSON · model ledger, generated 2026-10-28T01:21, confidence high, input hash 63fa867d9211fe64
thread read through 2026-10-26 (22 statements), head efc4246167
labels: Fuzzing

Categories

Test infrastructure

P3 · test coverage

Adds fuzz testing coverage for previously untested MuSig module signing and aggregation functions. As noted in review, 'the four signing functions in the module have no unit test today... so the harness covers ground that was empty', exercising edge cases and malformed inputs against new cryptographic wrapper APIs.

Membership: Adds a new fuzz harness in src/test/fuzz/musig.cpp and registers it in src/test/fuzz/CMakeLists.txt under the Fuzzing label.

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