[v0.20.x-branch] Backport #10895: htlcswitch: separate onchain and offchain intercpeted HTLCs cleanly#10933
Merged
Conversation
Author
|
Please cherry-pick the changes locally and resolve any conflicts. git fetch origin backport-10895-to-v0.20.x-branch
git worktree add --checkout .worktree/backport-10895-to-v0.20.x-branch backport-10895-to-v0.20.x-branch
cd .worktree/backport-10895-to-v0.20.x-branch
git reset --hard HEAD^
git cherry-pick -x 9b31ba83ef52e2d2800f53564f44b9efd4a75ca3 eb1193f80bbe5f0ec2f6618657f1deada5f6561c 98da7b4a56a75ba2dbf47391d43229fd9696e98a 8909c2fbf5535b81ee18c4f4e7fe0160ca43e725 9c5f32a2ec8eab0760a46c3d0357d4127794425f
git push --force-with-lease |
Add coverage for held forwards that move on chain after the incoming channel force closes. The restart case exercises the path where Bob loses the in-memory held set and contractcourt re-offers the HTLC through the witness beacon. The no-restart case keeps the original off-chain hold and proves that settlement must still reach the on-chain resolver. (cherry picked from commit 9b31ba8)
Store held forwards as off-chain or on-chain entries instead of a raw InterceptedForward map. Off-chain entries keep the existing resume, fail, settle and auto-fail behavior. On-chain entries are settle-only and expire by pruning local interceptor state. When contractcourt re-offers a circuit that is already held off-chain, replace the stored entry with the on-chain forward so a later SETTLE reaches the witness beacon instead of the old link mailbox path. Also set the on-chain interceptor deadline to the HTLC refund timeout. This keeps the public interceptor deadline populated while ensuring only off-chain held entries use that value to fail back. Only off-chain held HTLCs can be released when an optional interceptor disconnects, because they can resume into the link forwarding flow. On-chain held HTLCs have no link flow to resume. Keep them in the held set so a reconnecting interceptor can replay and settle them while contractcourt waits for the preimage or on-chain expiry. Use distinct internal deadline types for off-chain auto-fail heights and on-chain settlement deadlines instead of overloading the intercepted packet field. Project both variants back into the existing router RPC auto_fail_height field to preserve wire compatibility. Reject mismatched held HTLC deadline types in tests. On-chain intercepted HTLCs can only be settled. Resume and fail actions already return concrete errors through the on-chain intercepted forward, so let those errors propagate to the interceptor client instead of converting them to success. Keep the held entry tracked on these errors so the client can reconnect and settle the HTLC later. (cherry picked from commit eb1193f)
Release the preimage beacon lock before invoking the on-chain interceptor. The interceptor path can block on the htlcswitch event loop, while resolution of another held on-chain HTLC can call back into the beacon to add a preimage. If interceptor delivery fails after the subscriber was registered, cancel the subscription before returning the error. On-chain held entries are replay handles for the interceptor while contractcourt waits for a preimage or on-chain expiry. Once the resolver tears down, keeping the handle until the refund timeout can replay a stale HTLC to a reconnecting interceptor. Thread a dedicated cleanup signal from the witness subscription cancel path back through the interceptable switch event loop. The held set only removes on-chain entries for that signal, leaving off-chain entries under the link flow lifecycle. (cherry picked from commit 98da7b4)
routerrpc: document on-chain interceptor responses (cherry picked from commit 8909c2f)
(cherry picked from commit 9c5f32a)
ziggie1984
force-pushed
the
backport-10895-to-v0.20.x-branch
branch
from
fa4aa7a to
cf20de9
Compare
Collaborator
|
github-actions
Bot
added
the
severity-critical
Author
🔴 PR Severity: CRITICAL
🔴 Critical (4 files)
🟠 High (1 file)
🟡 Medium (3 files)
🟢 Low (1 file)
⚪ Excluded from count (5 files)
AnalysisThis PR modifies core htlcswitch package files — specifically The PR also exceeds 500 non-test lines changed (695 lines across 9 non-test, non-generated files), which triggers a severity bump — though the classification was already CRITICAL due to the htlcswitch changes. Expert review is recommended, with particular attention to state transitions in the held HTLC set and the interceptable switch logic. To override, add a |
Collaborator
|
Flake fix is not related to this change but still tracked here: #10938 |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #10895
Change Description
Fixes #10892
Look at the first commit which shows via itests the failure cases of the old Interceptor Implementation
The second commit introduces a new interface and distingishes between onchain and offchain HTLC for the interceptor. It does not change any public interface.