lnd+htlcswitch: keep on-chain intercepts held until the htlc expires#10893
lnd+htlcswitch: keep on-chain intercepts held until the htlc expires#10893kovin-muun wants to merge 3 commits into
Conversation
The auto-fail sweep treated an unset AutoFailHeight (zero value) as a deadline that has already been reached, popping the forward on the first block after it was held. Skip forwards with a non-positive auto-fail height instead, so that a forward without a deadline stays available to the interceptor until it is resolved. This protects on-chain intercepted forwards, which are created without an auto-fail height and cannot be failed back at all: FailWithCode returns ErrCannotFail, but the sweep deleted the entry from the held set regardless, leaving any later settle from the interceptor to fail with "fwd not found".
Set AutoFailHeight on the packets that the witness beacon offers to the htlc interceptor. An on-chain intercept can only be settled, so the right deadline is the htlc's on-chain expiry: past RefundTimeout a settle can no longer reliably win the race against the remote's timeout claim, while before it the forward must remain available so that a settle at any time hands the preimage to the witness beacon for the on-chain claim. Previously the field was left at zero and the auto-fail sweep evicted every on-chain intercept from the held set on the first new block after it was offered (~10 minutes). A settle arriving after that failed with "fwd not found" - additionally tearing down the interceptor stream - and the htlc was eventually claimed by the counterparty via the timeout path, losing funds that the interceptor held the preimage for. Fixes lightningnetwork#10892
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 addresses a critical issue where on-chain intercepted HTLCs were being prematurely evicted from the held set. By incorrectly treating a zero-value auto-fail height as an expired deadline, the system would tear down interceptor streams on the first block following an on-chain offer. The changes ensure that these intercepts are maintained until their actual on-chain expiry, allowing the interceptor sufficient time to provide a settlement preimage. 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 addresses a bug where HTLCs offered to the interceptor via the on-chain resolution flow were prematurely evicted from the held set on the first new block because their auto-fail height was not set. The fix ensures that forwards with a zero or negative auto-fail height are not auto-failed in popAutoFails, and sets the AutoFailHeight to the HTLC's refund timeout in witness_beacon.go. A corresponding unit test has been added to prevent regressions, and the release notes have been updated. No review comments were provided, and the implementation aligns with the repository's style guidelines.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
|
PR Severity: CRITICAL. Automated classification. 3 files (excl. tests), 25 lines changed. Critical: htlcswitch/held_htlc_set.go (HTLC forwarding state machine). Medium: witness_beacon.go (root-level Go). Low: docs/release-notes/release-notes-0.22.0.md. No bump applied. <!-- pr-severity-bot --> |
1 participant
Change Description
Bug: HTLCs offered to the interceptor through the on-chain resolution flow (
witness_beacon.go) are created without anAutoFailHeight, and the auto-fail sweep inhtlcswitchtreats the zero value as a deadline that has already passed.Consequence: therefore, every on-chain intercept is evicted from the held set on the first new block after being offered. Any later
Settlefrom the interceptor fails withfwd not found, the preimage never reaches the witnessbeacon, and the htlc is eventually claimed by the counterparty via the timeout path even though the interceptor held the preimage.
This PR fixes the witness beacon so that now sets
AutoFailHeightto the htlc's on-chain expiry (RefundTimeout), (past it a settle can no longer reliably win the race against the remote's timeout claim).This PR also changes the default behavior of the auto-fail sweep to skips held forwards with a non-positive
AutoFailHeightinstead of treating them as already expired. On-chain intercepts cannot be failed back anyway(
FailWithCodereturnsErrCannotFail), but the sweep deleted them from the held set regardless.Fixes #10892
Steps to Test
Regression test triggering the bug (fails on master, passes with this
change):
End-to-end on regtest (recipe from the issue):
requireinterceptor=trueand an interceptor client connected.Cannot fail packet: cannot fail in the on-chain flowand the held entry is evicted; with this change the forward stays held.Settlewith the correct preimage from the interceptor. On master it fails withfwd not found; with this change the preimage reaches the witness beacon and B claims the HTLC on-chain.