Skip to content

[v0.21.x-branch] Backport #11163: htlcswitch: reconcile resumed forward replays - #11188

Merged
ziggie1984 merged 3 commits into
v0.21.x-branchfrom
backport-11163-to-v0.21.x-branch
Sep 16, 2026
Merged

ziggie1984 merged 3 commits into
v0.21.x-branchfrom
backport-11163-to-v0.21.x-branch

Conversation

@github-actions

Copy link
Copy Markdown

Backport of #11163


Change Description

In this PR, we make resumed forward-interceptor HTLCs idempotent across
incoming-link replays.

Once an interceptor resumes a forward, the switch commits its incoming circuit
and removes the packet from the held set. If the incoming link restarts before
the return packet is committed, it can replay the add. Treating that replay as a
new intercepted forward can fail the incoming HTLC while the original outgoing
HTLC remains live.

For replayed adds that already have a circuit, we now pass the add back through
the switch and wait for the circuit map to classify it before releasing the
incoming link. Fresh adds and unresolved replays still use the asynchronous
interceptor path, so a blocked interceptor callback doesn't stall the link.

We also check the held set before applying the current-height expiry cutoff. A
held replay keeps the original resolution handle and auto-fail height instead of
making a second expiry decision.

Steps to Test

Run the focused forward-interceptor tests:

go test ./htlcswitch \
  -run '^(TestInterceptableSwitch|TestSwitchHoldForward)' -count=1

Run the package tests and vet with the development build tag:

go test -tags dev ./htlcswitch -count=1
go vet -tags dev ./htlcswitch

Run the two replay regressions under the race detector:

go test -race -tags dev ./htlcswitch \
  -run '^TestInterceptableSwitchReplayAfterResume -count=10
go test -race -tags dev ./htlcswitch \
  -run '^TestInterceptableSwitchHeldReplayNearExpiry -count=10

Pull Request Checklist

Testing

  • The public PR passes all CI checks.
  • Tests covering the positive and negative paths are included.
  • The bug fix includes tests that trigger both replay regressions.

Code Style and Documentation

  • The change is substantial.
  • The change follows the code documentation and commenting guidelines.
  • The commits follow the ideal Git commit structure.
  • New logging uses the existing HTLC switch subsystem and error level.
  • The change adds no lncli commands.
  • Add a release note or mark the public PR to skip the release-note check.

@github-actions github-actions Bot added this to the v0.21.4 milestone Sep 10, 2026
@github-actions

Copy link
Copy Markdown
Author

Please cherry-pick the changes locally and resolve any conflicts.

git fetch origin backport-11163-to-v0.21.x-branch
git worktree add --checkout .worktree/backport-11163-to-v0.21.x-branch backport-11163-to-v0.21.x-branch
cd .worktree/backport-11163-to-v0.21.x-branch
git reset --hard HEAD^
git cherry-pick -x bb916e5f9ddd855802fee998e94d47c8218946ff
git push --force-with-lease

In this commit, we let the switch reconcile replayed adds whose incoming
circuit has already been committed.

Once the interceptor resumes a forward, the switch commits its incoming
circuit and removes the packet from the held set. If the incoming link then
replays that add before the return packet is committed, the packet can be
offered to the interceptor a second time.

The circuit map still distinguishes in-memory duplicates from circuits
restored during recovery. Fresh packets continue through the normal
interception path.
In this commit, we recognize held duplicates before applying the current-height
expiry check.

The original held forward keeps its resolution handle and auto-fail deadline.
This prevents a near-expiry replay from failing upstream while the interceptor
can still resume the outgoing HTLC. The regression test verifies that the
original handle opens and settles exactly one circuit.
In this commit, we add the public PR to the 0.20.5 and 0.21.4
release notes.

The note calls out that incoming-link replays are reconciled after the
interceptor resumes a forward, so operators know the duplicate interception
behavior is fixed.
@ziggie1984
ziggie1984 force-pushed the backport-11163-to-v0.21.x-branch branch from 06cd342 to faa5163 Compare September 16, 2026 02:19
@ziggie1984

Copy link
Copy Markdown
Collaborator

Re-applied the backport cleanly

The branch previously ended in a BACKPORT-CONFLICT commit. All three commits
from #11163 are now cherry-picked individually onto v0.21.x-branch, with
authorship and commit structure preserved.

I also re-based onto the current v0.21.x-branch tip (727277cec) rather
than the stale base the bot used (6f870b17b). Two backports landed in
between — #11179 and #11125 — and one of them touches itest/list_on_test.go,
which this change also modifies.

What applied cleanly vs. what needed resolution

# Commit File Result
1 504078087 htlcswitch: reconcile resumed forward replays htlcswitch/interceptable_switch.go clean
1 htlcswitch/switch_test.go clean (auto-merge)
1 itest/list_exclude_test.go clean
1 itest/list_on_test.go clean (auto-merge)
1 itest/lnd_forward_interceptor_test.go clean (auto-merge)
2 a8662b586 htlcswitch: retain held replay expiry decisions htlcswitch/held_htlc_set_test.go clean
2 htlcswitch/interceptable_switch.go clean
2 htlcswitch/switch_test.go clean (auto-merge)
3 bb916e5f9 docs: note replay reconciliation fix docs/release-notes/release-notes-0.21.4.md conflict
3 docs/release-notes/release-notes-0.20.5.md conflict (modify/delete)

8 of 10 file-changes applied cleanly, and both conflicts are confined to
release notes. No Go code needed manual resolution — unlike the v0.20.x
backport (#11187), v0.21.x-branch already carries the endorsement ->
accountability rename, so lnd_forward_interceptor_test.go merged as-is.

Conflict 1 — docs/release-notes/release-notes-0.21.4.md

Purely additive. The # Bug Fixes section on v0.21.x-branch now contains the
#11179 SQL graph migration entry, which landed after the bot's base commit.
Resolution: keep both entries, #11179 first, then the #11163 entry appended.

Conflict 2 — docs/release-notes/release-notes-0.20.5.md

Does not exist on v0.21.x-branch. Dropped.

Worth noting: the previous BACKPORT-CONFLICT commit resolved this the other
way and added release-notes-0.20.5.md to v0.21.x-branch — a 64-line
v0.20.5 release notes file that does not belong on this branch. That is now
gone.

Verification

go build ./...                                                  # OK
go vet -tags="integration dev" ./itest/... ./htlcswitch/...     # OK
go test -tags dev ./htlcswitch -count=1 \
  -run '^(TestInterceptableSwitch|TestSwitchHoldForward|TestHeldHtlcSet)'
                                                                # ok  15.248s

The itests were not run locally (they need the full harness).
testForwardInterceptorRestartBeforeResume and
testForwardInterceptorRestartAfterResume are registered in list_on_test.go
and compile under the integration tag; CI covers the rest.

@github-actions github-actions Bot added the severity-critical Requires expert review - security/consensus critical label Sep 16, 2026
@github-actions

Copy link
Copy Markdown
Author

🔴 PR Severity: CRITICAL

backport | 7 files | 579 lines changed

🔴 Critical (1 file)
  • htlcswitch/interceptable_switch.go - reconciles resumed forward replays and held-HTLC replay expiry decisions in the HTLC forwarding/interception path
🟢 Low (6 files)
  • docs/release-notes/release-notes-0.21.4.md - release notes entry
  • htlcswitch/held_htlc_set_test.go - test-only change
  • htlcswitch/switch_test.go - test-only change
  • itest/list_exclude_test.go - test-only change
  • itest/list_on_test.go - test-only change
  • itest/lnd_forward_interceptor_test.go - test-only change

Analysis

This is a backport of #‌11163 to v0.21.x-branch. The only non-test, non-docs
file touched is htlcswitch/interceptable_switch.go, which falls under
htlcswitch/* (HTLC forwarding / payment routing state machine) — a CRITICAL
package per policy. The remaining changes are test coverage and a release
notes entry. No bump conditions apply (file count, line count, and
multi-package spread are all well under thresholds once tests/docs are
excluded).


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

@ziggie1984
ziggie1984 marked this pull request as ready for review September 16, 2026 12:35

@ziggie1984 ziggie1984 left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM (just release note conflicts to resolve)

@ziggie1984
ziggie1984 merged commit c339078 into v0.21.x-branch Sep 16, 2026
38 of 40 checks passed
@ziggie1984
ziggie1984 deleted the backport-11163-to-v0.21.x-branch branch September 16, 2026 13:03
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

no-changelog severity-critical Requires expert review - security/consensus critical

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants