fix: LSPS4 HTLC forwarding race condition and retry mechanism#4
Closed
martinsaposnic wants to merge 1 commit into
Closed
fix: LSPS4 HTLC forwarding race condition and retry mechanism#4martinsaposnic wants to merge 1 commit into
martinsaposnic wants to merge 1 commit into
Conversation
forward_intercepted_htlc was returning Ok when the channel was not live (peer_disconnected flag set), causing LSPS4 to remove the HTLC from the store. LDK's send_htlc would then reject with "Cannot send while disconnected", silently losing the HTLC. Additionally, peer_connected fires before ChannelReestablish completes, so channels are never is_usable at that point. Combined with a 10s HTLC expiry, payments arriving while the peer was offline had almost no chance of being forwarded in time. Changes: - forward_intercepted_htlc: return Err when channel is not live - execute_htlc_actions: only remove HTLC from store on Ok; on Err, keep/re-insert for retry - peer_connected: skip forwarding when no channels are is_usable - New try_forward_pending_htlcs(): periodic retry for stored HTLCs, called from the 5s poll loop - New get_all_peer_node_ids() on HTLCStore - Bump HTLC_EXPIRY_THRESHOLD_SECS from 10 to 30
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
1 participant
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.
Summary
Fixes two bugs causing ~6.7% LSPS4 payment failures on staging (2/30 L402 payments failed):
forward_intercepted_htlclies about success: ReturnsOkwhen channel is not live (peer_disconnectedflag set). LSPS4 removes HTLC from store, then LDK'ssend_htlcrejects with "Cannot send while disconnected". HTLC silently lost. (22 occurrences in staging logs.)peer_connectedrace + short expiry:peer_connectedfires beforeChannelReestablishcompletes, sois_usableis alwaysfalse. HTLCs forwarded at this point always fail silently. Combined with 10s expiry, payments arriving while peer was offline couldn't survive the webhook round-trip (webhook -> merchant wake -> TCP -> noise -> reestablish).Changes
channelmanager.rs:forward_intercepted_htlcreturnsErr(ChannelUnavailable)when!is_liveservice.rs:execute_htlc_actionsonly removes HTLC from store onOk; onErr, keeps/re-inserts for retryservice.rs:peer_connectedskips forwarding when no channels areis_usableservice.rs: Newtry_forward_pending_htlcs()- periodic retry for stored HTLCs (called from 5s poll loop in ldk-node)htlc_store.rs: Newget_all_peer_node_ids()helperservice.rs:HTLC_EXPIRY_THRESHOLD_SECSbumped from 10 to 30How it works now
peer_connectedskips forwarding (channels not usable yet)ChannelReestablishcompletes -> channels becomeis_usabletry_forward_pending_htlcsfinds stored HTLCs, sees usable channel, forwardsTest plan
cargo check -p lightningpassescargo check -p lightning-liquiditypasseslsps4_client_service_integrationtest passeslsps4_htlc_retry_after_reconnectintegration test in ldk-node (disconnect client, send payment, reconnect, verify retry forwards successfully)