funding: add coverage-guided fuzz harness for the channel-opening state machine

[MPins]

Overview

This PR adds FuzzFundingManagerFSM, a native Go coverage-guided fuzz harness that exercises the funding manager's channel-opening state machine end to end. It decodes the corpus byte stream into a sequence of
events driving one or more concurrent funding flows against the manager.

Design

• Single shared SUT, per-flow counterparty. Alice (the system under test) is one shared *Manager; each flow gets its own counterparty (Bob) manager. The SUT is what we're testing, so all adversarial or inconsistent values are imposed by the counterparty and the SUT must validate/reject them — as originator the SUT only ever emits valid values.
• FSM over a byte stream. Events (start-as-funder, start-as-fundee, switch-flow, peer-interaction, confirm-funding-tx, …) and their parameters are read from the fuzz input, so the fuzzer explores interleavings of concurrent flows.
• Deterministic oracles. Reservation counts are asserted at fixed handshake barriers; adversarial injections must never advance the live handshake; a premature channel_ready must be parked, not opened.

BOLT-2 channel_type conformance

One family of oracles checks what the SUT emits against BOLT 2's channel_type requirements, rather than against what the funding manager on the other end happens to accept. The distinction matters here: both ends of the harness are real funding.Managers, so an lnd-to-lnd flow agrees with itself even where both sides diverge from the spec, and the divergence stays invisible until a peer running the current spec shows up.

Since the counterparty is a real manager, it never emits a non-conforming message on its own. The harness therefore rewrites the captured message before delivering it, one corpus-selectable variant per rule:

• open_channel with channel_type stripped, option_channel_type negotiated
• open_channel naming a type neither side advertised the feature for
• open_channel naming a type the SUT's features cannot support
• accept_channel with the echo stripped
• accept_channel echoing a type the SUT never proposed

Two assertions then run on every message the SUT sends: an open_channel must carry a channel_type, and an accept_channel must echo the received one exactly.

Against master this reaches three divergences, all of them fixed by #11064 :

1. Peer omits channel_type while option_channel_type is negotiated. BOLT 2 says the receiver must fail the channel. lnd instead falls back to its default selection and puts that invented type in accept_channel — which a conforming funder must reject, the echo having to match what it sent.

2. Peer advertises neither bit 44 nor 45 but sends channel_type, as current BOLT 2 requires of it, the field no longer being feature-gated. lnd accepts the channel and omits the echo entirely.

3. Same peer, lnd funding. lnd's own open_channel goes out with no channel_type.

Until #11064 lands these three are logged rather than fatal, so the fuzzer can keep running — it otherwise stops within seconds of starting and reaches nothing else. The must-reject check for case 1 is commented out for the same reason; the case still surfaces, through the echo oracle, as the invented type in accept_channel. Each spot carries a TODO to restore the hard assertion. The remaining channel_type oracles stay armed, master already enforcing the funder-side checks on accept_channel.

[github-actions]

🟢 PR Severity: LOW

Classified from file diff | 1 file | 2034 lines changed

🟢 Low (1 file)

• funding/fuzz_test.go - New fuzz test file (*_test.go); test-only change, no production code touched

Analysis

This PR consists solely of a new file, funding/fuzz_test.go, adding 2034 lines with no deletions. Although funding/* is normally a CRITICAL package (channel funding workflow coordination), the file matches the *_test.go pattern, which the classification rules explicitly designate as test-only content regardless of package path — placing it in the LOW severity tier ("best-effort review").

Since this is the only file in the diff and it's a test file, it's also excluded from the file-count/line-count bump calculation (0 non-test files, 0 non-test lines changed), so no severity bump applies.

No production code in funding/* is modified — this PR only adds fuzzing coverage for that package.

---

_{To override, add a severity-override-{critical,high,medium,low} label.}

[Bartok9]

Review

Thanks for investing in a real coverage-guided funding-state fuzz harness — this is high-value (channel open is a classic adversarial surface) and matches the spirit of growing fuzz coverage rather than only happy-path unit tests.

Concept ACK on the direction: event enum (funder/fundee, peer adversarial modes, conf/reorg, disconnect/reconnect) is the right shape for a state-machine fuzzer.

Please clarify before a full ACK

1. Default CI / developer cost — Is the fuzz target skipped in plain go test ./funding (build tags, short mode, or fuzz-only entrypoints)? A ~3kLOC harness is worth it if day-to-day tests stay fast; please document the intended go test -fuzz=... -fuzztime= invocation in the PR body or funding package comment.
2. Failure oracle — Beyond panics, which invariants are checked (e.g. consistent chan IDs after confirm, no progress after fatal remote errors, funding tx / short chan id agreement across roles)? A short bullet list would help reviewers trust the harness.
3. Entropy — Confirm peer/crypto/timing inputs are derived from fuzz data (no unintended wall-clock flakiness except explicit timeout paths).
4. go.mod alignment — On a quick skim the new file imports look sensitive to btcd module paths; CI on this branch is the source of truth — green module build is enough.

Happy to re-review after the CI/skip + oracle notes land. Not requesting changes on concept — just wanting the operational story explicit for maintainers.

[litbot-9000]

@MPins, remember to re-request review from reviewers when ready

[Bartok9]

Thanks for the fresh push (@MPins) — saw the two commits land (funding: add coverage-guided fuzz harness… + funding: derive test funding manager state from the node's own key).

Quick re-check against the open questions from the earlier concept-ACK review:

1. chanIDSeed / multi-manager isolation — The manager_test.go change (seed pending-channel-ID stream from privKey, wire RootKey/SingleSigner/SecretKeyRing off the passed key) directly answers the concurrent Alice/Bob isolation concern. Looks correct on skim.
2. Oracles — PR body now states the deterministic checks (reservation barriers, adversarial inject must not advance handshake, premature channel_ready parked). That covers the oracle ask at the summary level; still happy if a short comment above FuzzFundingManagerFSM mirrors those bullets for in-tree reviewers.
3. Still open for a full ACK
   • Default CI / developer cost: please spell the intended invocation (go test -fuzz=FuzzFundingManagerFSM -fuzztime=…) and confirm plain go test ./funding (and unit CI) does not burn wall time on long fuzzing — t.Skip on non-Linux is good; note whether -short / fuzz-only is expected.
   • CI green on this tip — checks just restarted and are still pending; will re-review once unit/lint are green.
   • Entropy/flakiness: still assuming peer/crypto/timing come from the corpus (no wall-clock except explicit timeout paths) — a one-liner in the harness header would close that.

No blocker on concept. Will look again after CI settles and the run-cost note lands (PR body or package comment is enough).

[Bartok9]

Thanks for the additional force-pushes on this tip (045eb233 — still the two-commit series: harness + createTestFundingManager key wiring).

Re-skimmed the current head against the open items from the concept-ACK / last note:

Addressed / looking good

• Multi-manager isolation: manager_test.go still seeds chanIDSeed from privKey.Serialize() and wires RootKey / SingleSigner / SecretKeyRing off the passed key — correct for concurrent Alice/Bob flows.
• Oracles in-tree: harness has real checks (assertReservations, adversarial assertNoAdvance, channel_type echo/funder checks, premature/NotifyWhenOnline park paths for channel_ready). PR body bullets match the code.
• Default CI cost: FuzzFundingManagerFSM is a native testing.F target (f.Fuzz(...) only). Plain go test ./funding will compile/seed corpus entries but will not run long coverage-guided fuzzing unless -fuzz=FuzzFundingManagerFSM is passed. Non-Linux is t.Skip in newFuzzFSM (tmpfs//dev/shm path). That closes the “don’t burn unit CI” concern for me.

Still nice-to-have (non-blocking for concept)

• One line in the PR body (or above FuzzFundingManagerFSM) with the intended invocation, e.g.
  go test ./funding -fuzz=FuzzFundingManagerFSM -fuzztime=30s
  so maintainers don’t have to reverse-engineer it.
• Entropy note: peer/crypto choices are corpus-driven; remaining wall-clock is only explicit wait/poll timeouts (managerTimeout / handoffPollInterval) — a header one-liner would lock that in for reviewers.

CI: checks on this tip just (re)queued after the latest push — still pending here. Will give a fuller pass once unit/lint are green on 045eb233.

No change on concept: still a concept ACK. Happy to move toward a full ACK after green CI + the short run-cost blurb.

[Bartok9]

Thanks for the further force-push (@MPins) — tip is now 7b75250fbe (still the two-commit series: harness + createTestFundingManager key wiring; harness grown ~3.5kLOC).

Quick delta vs the last note:

• Still looking good: multi-manager isolation via key-derived chanIDSeed / signer wiring; native testing.F target so plain go test ./funding does not burn long fuzz time; non-Linux skip path retained.
• Still open (non-blocking for concept): PR body / harness header still missing the one-liner invocation (go test ./funding -fuzz=FuzzFundingManagerFSM -fuzztime=30s) and a short entropy note (corpus-driven peer/crypto; wall-clock only on explicit wait/poll timeouts).
• CI: fresh checks are in flight on this tip; one early failure already showed on Backwards compatibility test while most jobs are still pending — will re-check once the suite settles before any full-ACK move.

Concept ACK unchanged. Happy to look again after green unit/lint + the short run-cost blurb.

[MPins]

@saubyk what is your opinion on the comments above? Personally, I think it's noise.