htlcswitch: add link state machine fuzz harness#10865
Conversation
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.
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.
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.
| if runtime.GOOS != "linux" { | ||
| t.Fatalf("Error: fuzzing on non-Linux OS: %s", runtime.GOOS) | ||
| } |
There was a problem hiding this comment.
Using t.Fatalf when the operating system is not Linux will cause the entire test suite to fail on non-Linux platforms (such as macOS or Windows). This breaks cross-platform development and testing for developers who run the unit tests locally. Consider using t.Skipf instead to gracefully skip these tests on unsupported platforms.
| if runtime.GOOS != "linux" { | |
| t.Fatalf("Error: fuzzing on non-Linux OS: %s", runtime.GOOS) | |
| } | |
| if runtime.GOOS != "linux" { | |
| t.Skipf("Skipping fuzz/scenario test on non-Linux OS: %s", runtime.GOOS) | |
| } |
| newFee := ((len(f.aliceLink.channel.ActiveHtlcs()))+ | ||
| int(f.htlcRef))*100 + 1000 |
There was a problem hiding this comment.
If f.htlcRef is extremely large, casting it to int can result in a negative value due to signed integer wrapping, leading to a negative or extremely large newFee. This can cause the fee update to fail immediately or terminate the link, reducing fuzzing efficiency. Bounding f.htlcRef using a modulo operator keeps the fee rate within a reasonable, positive range.
| newFee := ((len(f.aliceLink.channel.ActiveHtlcs()))+ | |
| int(f.htlcRef))*100 + 1000 | |
| newFee := (len(f.aliceLink.channel.ActiveHtlcs()) + | |
| int(f.htlcRef%100000))*100 + 1000 |
| newFee := ((len(f.bobLink.channel.ActiveHtlcs()))+ | ||
| int(f.htlcRef))*100 + 1000 |
There was a problem hiding this comment.
If f.htlcRef is extremely large, casting it to int can result in a negative value due to signed integer wrapping, leading to a negative or extremely large newFee. Bounding f.htlcRef using a modulo operator keeps the fee rate within a reasonable, positive range.
| newFee := ((len(f.bobLink.channel.ActiveHtlcs()))+ | |
| int(f.htlcRef))*100 + 1000 | |
| newFee := (len(f.bobLink.channel.ActiveHtlcs()) + | |
| int(f.htlcRef%100000))*100 + 1000 |
| bobSettlesPending map[uint64]struct{} | ||
| } | ||
|
|
||
| func newFuzzFSM(t *testing.T, channelSize, aliceShareGen, |
There was a problem hiding this comment.
According to the repository style guide, every function must be commented with its purpose and assumptions, and the comment must begin with the function name as a complete sentence. Please add a compliant comment to newFuzzFSM.
// newFuzzFSM initializes and returns a new fuzz finite state machine (FSM)
// instance with the specified channel size and configuration parameters.
func newFuzzFSM(t *testing.T, channelSize, aliceShareGen,References
- Every function must be commented with its purpose and assumptions. Function comments must begin with the function name and should be complete sentences. (link)
| } | ||
| } | ||
|
|
||
| func (f *fuzzFSM) assertInvariants() { |
There was a problem hiding this comment.
According to the repository style guide, every function must be commented with its purpose and assumptions, and the comment must begin with the function name as a complete sentence. Please add a compliant comment to assertInvariants.
| func (f *fuzzFSM) assertInvariants() { | |
| // assertInvariants verifies that the channel heights and balances are consistent | |
| // and mirrored between Alice and Bob, ensuring no silent fund misallocation. | |
| func (f *fuzzFSM) assertInvariants() { |
References
- Every function must be commented with its purpose and assumptions. Function comments must begin with the function name and should be complete sentences. (link)
| return htlc, invoice | ||
| } | ||
|
|
||
| // generateSingleHopHtlc generate a single hop htlc to send to the receiver. |
There was a problem hiding this comment.
According to the repository style guide, function comments should be complete sentences. Please correct the comment to use 'generates' instead of 'generate' to make it a complete sentence.
| // generateSingleHopHtlc generate a single hop htlc to send to the receiver. | |
| // generateSingleHopHtlc generates a single hop htlc to send to the receiver. |
References
- Function comments should be complete sentences. (link)
🔴 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 |
1 participant
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