htlcswitch: add link state machine fuzz harness#10865
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request adds a high-performance, coverage-guided fuzz harness for the Lightning Network channel link state machine. By replacing expensive cryptographic operations and disk I/O with in-memory mocks, the harness achieves the high iteration rates required for effective fuzzing. Additionally, the PR includes documentation for the link state machine and refactors core wallet components to support these testing hooks. Highlights
New Features🧠 You can now enable Memory (public preview) to help Gemini Code Assist learn from your team's feedback. This makes future code reviews more consistent and personalized to your project's style. Click here to enable Memory in your admin console. Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
github-actions
Bot
added
the
severity-critical
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a coverage-guided fuzz test and deterministic scenario tests for the two-party commitment protocol, along with state machine documentation and testing hooks to bypass expensive cryptographic operations. The review feedback is highly constructive, pointing out opportunities to prevent test failures on non-Linux platforms, avoid integer wrapping issues when calculating fees from fuzz inputs, and ensure strict compliance with the repository's style guide regarding function comments.
🔴 PR Severity: CRITICAL
🔴 Critical (6 files)
🟢 Low (1 file)
⚪ Excluded from counting (2 test files)
AnalysisThis PR touches two distinct CRITICAL package families:
Severity bump check (non-test/non-generated files):
The bulk of the additions (1,820 lines) is a new fuzz test ( To override, add a |
|
@MPins, remember to re-request review from reviewers when ready |
Expose the `invoiceRegistry` field in `singleLinkTestHarness` so tests can register and look up invoices directly. Add `generateSingleHopHtlc`, a test helper that builds a single-hop `UpdateAddHTLC` with a random preimage, intended for use in unit and fuzz tests.
Add a no-op MailBox implementation and a no-op ticker for use in the channelLink FSM fuzz harness.
Replace createChannelLinkWithPeer (which required a Switch and spawned the htlcManager goroutine) with newFuzzLink, a minimal link factory that: - accepts dependencies directly (registry, preimage cache, circuit map, bestHeight) instead of a mockServer, so no Switch or background goroutines are created at all - sets link.upstream directly to a buffered channel controlled by the caller, bypassing the mailbox entirely - attaches a mockMailBox so mailBox.ResetPackets() in resumeLink succeeds
Add a failReason string field to channelLink that is populated by failf alongside the existing failed flag. This gives fuzz and unit tests direct access to the human-readable failure reason without requiring a dedicated OnChannelFailure callback or log scraping.
Introduce a one-shot nextOnionFailMode flag on mockIteratorDecoder
and matching payloadFail / extractFail fields on mockHopIterator so
that fuzz and unit tests can deterministically exercise the three
error branches of channelLink.processRemoteAdds:
- onionFailDecode → DecodeHopIterator returns a non-CodeNone
failcode (CodeTemporaryChannelFailure).
- onionFailPayload → HopPayload returns hop.ErrInvalidPayload.
- onionFailExtract → ExtractErrorEncrypter returns a non-CodeNone
failcode (CodeInvalidOnionVersion).
The flag is consumed and cleared on each DecodeHopIterator call so
it affects exactly one HTLC. Default behaviour is unchanged when no
mode is armed, so existing callers see no difference.
newMockHopIterator now returns *mockHopIterator (instead of
hop.Iterator) so the decoder can set the per-iterator failure flags
after construction; the concrete type still satisfies hop.Iterator
and the only external caller in test_utils.go is unaffected.
MPins
force-pushed
the
link_fsm_fuzz
branch
3 times, most recently
from
328ed3b to
ad4f1d5
Compare
Introduce `fuzz_link_test.go` with a model-based fuzzer that drives the Alice-Bob channel link through arbitrary sequences of protocol events and checks key invariants after each step.
Introduce fuzzSigner and fuzzSigVerifier in the fuzz harness, along with the SigVerifier hook in LightningChannel (WithSigVerifier, verifySig) and a matching SigPool extension (VerifyFunc field) so the harness can bypass secp256k1 verification end-to-end. Also refactors createTestChannel to accept functional options (testChannelOpt) so the signer and channel options can be injected from tests.
Introduce CommitKeyDeriverFunc and WithCommitKeyDeriver to allow LightningChannel to bypass the secp256k1-based DeriveCommitmentKeys on every commit round. All internal call sites are migrated to lc.deriveCommitmentKeys. The fuzz harness injects fuzzCommitKeyDeriver, a trivial identity deriver that avoids scalar-multiplication overhead.
createTestChannel started alicePool and bobPool but never stopped them. During fuzzing this caused goroutines to leak per. Register t.Cleanup handlers to call Stop() on both pools so all workers are torn down when the test ends.
newMockRegistry started an InvoiceRegistry but never stopped it. InvoiceRegistry internally starts two background goroutines — invoiceEventLoop and the InvoiceExpiryWatcher mainLoop — that run for the lifetime of the registry. Without a matching Stop() call both goroutines leaked for every test that called newMockRegistry, accumulating thousands of goroutines during fuzzing. Register a t.Cleanup to call registry.Stop() so both loops are torn down when the test ends.
2 participants
This PR adds a coverage-guided fuzz harness that exercises the link state machine by randomly interleaving HTLC additions, commits, revocations, settlements, failures, link restarts, and transitions into and out of quiescence mode from both Alice and Bob.
Making each iteration cheap enough to fuzz
A coverage-guided fuzzer typically needs to execute hundreds of iterations per second to be able to find interesting states. The dominant per-iteration costs in the normal link path are secp256k1 arithmetic and disk I/O, neither of which are the focus of this harness. To keep the state machine the bottleneck rather than crypto and the filesystem, the harness introduces three substitutions:
*channeldb is backed by bbolt files created under t.TempDir(). To avoid the disk I/O bottleneck, the harness redirects TMPDIR to /dev/shm (tmpfs) so the bbolt files live in RAM, because tmpfs is Linux-specific, the harness fails fast on non-Linux hosts.
This work was initially developed and reviewed in my fork MPins#1 before being submitted upstream.
go test ./htlcswitch/ -run TestChannelLinkFSMScenarios -v
go test ./htlcswitch -run=^$ -fuzz=FuzzChannelLinkFSM -fuzztime=1m
Adding the coverage report for the current state of the PR.
coverage.html