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

[github-actions]

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]

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

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

[ziggie1984]

Re-applied the backport cleanly

The previous state of this branch was a single squashed BACKPORT-CONFLICT
commit that had two problems:

1. It contained live conflict markers at
   itest/lnd_forward_interceptor_test.go:483 and :533, so the branch did
   not compile.
2. It only carried the first of the three commits from htlcswitch: reconcile resumed forward replays #11163. Commits 2
   (htlcswitch: retain held replay expiry decisions) and 3 (release notes)
   were dropped entirely — that is the 112-line switch_test.go addition, the
   held_htlc_set expiry changes, and the changelog entry.

I force-pushed a re-applied version: all three upstream commits cherry-picked
individually onto v0.20.x-branch, preserving authorship and commit structure.

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	conflict
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.20.5.md	clean
3		docs/release-notes/release-notes-0.21.4.md	conflict (modify/delete)

8 of 10 file-changes applied cleanly. The two conflicts are detailed below so
they can be verified without re-running the cherry-pick.

Conflict 1 — itest/lnd_forward_interceptor_test.go

A single hunk, and it is purely a naming difference, not a logic difference.
master has completed the endorsement -> accountability rename;
v0.20.x-branch has not.

Upstream asserts unconditionally:

require.Len(ht, packet.InWireCustomRecords, 2)
require.Equal(
        ht,
        lntest.CustomRecordsWithUnaccountable(customRecords),
        packet.InWireCustomRecords,
)

On v0.20.x-branch only CustomRecordsWithUnendorsed and
ExperimentalEndorsementActive() exist (lntest/utils.go:290 and :311),
and the expected record count is gated on whether the experiment is still
active.

Resolution: keep the upstream control flow verbatim — the
if !resumeBeforeRestart { ... } block and the carolPacket handoff — and
restore the v0.20.x expectedLen / expectedCarolLen gating inside it. No
behavioural change relative to upstream; the assertions cover the same states.

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

Master-only file; it does not exist on v0.20.x-branch. Dropped. The
release-notes-0.20.5.md entry applied unchanged.

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.166s

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]

🔴 PR Severity: CRITICAL

gh pr | 7 files | 585 lines changed

🔴 Critical (1 file)

• htlcswitch/interceptable_switch.go - HTLC forwarding/interception logic in the payment routing state machine (+82/-8)

🟢 Low (6 files)

• docs/release-notes/release-notes-0.20.5.md - release notes entry
• htlcswitch/held_htlc_set_test.go - test-only change
• htlcswitch/switch_test.go - test-only change (new coverage for held replay expiry)
• itest/list_exclude_test.go - itest registration, test-only
• itest/list_on_test.go - itest registration, test-only
• itest/lnd_forward_interceptor_test.go - itest, test-only

Analysis

This is a backport of a fix to htlcswitch/interceptable_switch.go, which sits
in the HTLC forwarding/interception state machine (htlcswitch/*) and is
classified CRITICAL regardless of the surrounding test/doc changes. The rest
of the diff is test scaffolding (itest registration, unit test coverage) and a
release-notes entry, which are LOW on their own but don't lower the overall
PR severity. File count and non-test/non-doc line count are well under the
bump thresholds, so no additional escalation applies.

Given this touches the interceptor's replay/resume handling directly, it
warrants review from someone familiar with htlcswitch HTLC forwarding
semantics, per the PR author's own notes about conflict resolution during the
backport (endorsement/accountability rename mismatch, dropped commits in the
prior squashed attempt).

---

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

[ziggie1984]

LGTM