Skip to content

LSPS2 service: Treat replayed HTLCIntercepted events idempotently#4656

Open
tnull wants to merge 3 commits into
lightningdevkit:mainfrom
tnull:2026-06-lsps2-replayed-htlc-queue
Open

LSPS2 service: Treat replayed HTLCIntercepted events idempotently#4656
tnull wants to merge 3 commits into
lightningdevkit:mainfrom
tnull:2026-06-lsps2-replayed-htlc-queue

Conversation

@tnull
Copy link
Copy Markdown
Contributor

@tnull tnull commented Jun 2, 2026

Fixes #4637.

Replayed intercepted HTLC events should not duplicate queued payments or
panic after restart. Ignore already-queued intercept IDs so persisted
queues remain stable across event replay.

Also:

Terminal JIT channel state is only useful while the forwarded channel
still exists. Drop completed LSPS2 mappings once the channel is gone so
persisted service state does not retain stale entries indefinitely.

tnull added 2 commits June 2, 2026 14:20
Persisting LSPS2 service state can race with replayed intercepted HTLC events after restart. Cover replaying the same intercepted HTLC after restoring peer state so duplicate queueing is caught.

Co-Authored-By: HAL 9000
Replayed intercepted HTLC events should not duplicate queued payments or panic after restart. Ignore already-queued intercept IDs so persisted queues remain stable across event replay.

Co-Authored-By: HAL 9000
@ldk-reviews-bot
Copy link
Copy Markdown

ldk-reviews-bot commented Jun 2, 2026

I've assigned @jkczyz as a reviewer!
I'll wait for their review and will help manage the review process.
Once they submit their review, I'll check if a second reviewer would be helpful.

@tnull tnull force-pushed the 2026-06-lsps2-replayed-htlc-queue branch from 686c27d to 190b6af Compare June 2, 2026 12:48
Comment thread lightning-liquidity/src/lsps2/service.rs Outdated
@ldk-claude-review-bot
Copy link
Copy Markdown
Collaborator

ldk-claude-review-bot commented Jun 2, 2026

I've completed a thorough review of every hunk in this PR diff, examining:

  1. PaymentQueue idempotency (payment_queue.rs): The duplicate intercept_id check correctly prevents re-adding replayed HTLCs across all state machine arms that call add_htlc. The debug_assert_eq on payment_hash is appropriate validation.

  2. remove_terminal_channel_state (service.rs): Correctly guards removal behind PaymentForwarded state check via get_channel_id() and channel_id match. Cleans up all three inner maps consistently.

  3. channel_closed (service.rs): Properly cleans up both outer maps (peer_by_intercept_scid, peer_by_channel_id) and inner state. Uses self.persist() rather than persist_peer_state(), which is the right choice since it also handles pruning empty peer state.

  4. Sync wrapper (service.rs): Follows the established pattern used by all other sync wrappers in LSPS2ServiceHandler.

  5. Documentation (manager.rs): Correctly adds ChannelClosed to the event forwarding list with proper doc links.

  6. Tests: Both new tests (replayed_intercepted_htlc_after_persist_is_idempotent and removes_terminal_state_for_closed_channel) correctly exercise the key scenarios including serialize/deserialize round-trip and idempotent second removal.

No new issues found. The previously flagged Vec vs HashSet performance concern (inline comment on service.rs:1841) remains the only finding.

@ldk-reviews-bot ldk-reviews-bot requested a review from jkczyz June 2, 2026 12:57
Terminal JIT channel state is only useful while the forwarded channel still exists. Drop completed LSPS2 mappings once the channel is gone so persisted service state does not retain stale entries indefinitely.

Co-Authored-By: HAL 9000
@tnull tnull force-pushed the 2026-06-lsps2-replayed-htlc-queue branch from 190b6af to 444c009 Compare June 2, 2026 13:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

LSP panics on restart due to stale LSPS2 payment queue entries (duplicate intercept_id assertion)

3 participants