From 331d1bdeecce732e997a968fd5dd7ad5d183d845 Mon Sep 17 00:00:00 2001 From: amackillop Date: Sat, 21 Mar 2026 19:21:43 -0700 Subject: [PATCH 1/3] Consolidate LSPS4 usability checks to execute time Channel usability (is_usable) was checked at four separate points: htlc_intercepted, peer_connected, process_pending_htlcs, and calculate_htlc_actions_for_peer. Each had its own deferral logic, and they had to coordinate (the timer skipped channel opens assuming peer_connected already handled them). This coordination broke: PR #9 made peer_connected call process_htlcs_for_peer during reestablish, which saw an empty capacity map because non-usable channels were filtered out, and emitted a spurious OpenChannel on every reconnect with a pending HTLC. Move the usability check to execute_htlc_actions, right before forward_intercepted_htlc. If no usable channel exists, the forward is skipped and the HTLC stays in store for the timer to retry. htlc_intercepted, peer_connected, and process_pending_htlcs now all call process_htlcs_for_peer unconditionally. calculate_htlc_actions_for_peer includes all channels in the capacity map regardless of is_usable, so it correctly sees that a reestablishing channel has sufficient capacity and does not request a spurious new channel. Change the pre-forward guard from is_peer_connected to has_usable_channel, which covers the disconnect+reconnect race where the peer is connected but the channel has not finished reestablishing. --- lightning-liquidity/src/lsps4/service.rs | 123 +++++++---------------- 1 file changed, 35 insertions(+), 88 deletions(-) diff --git a/lightning-liquidity/src/lsps4/service.rs b/lightning-liquidity/src/lsps4/service.rs index 52b23f31170..b96f814064f 100644 --- a/lightning-liquidity/src/lsps4/service.rs +++ b/lightning-liquidity/src/lsps4/service.rs @@ -193,7 +193,6 @@ where .get_cm() .list_channels_with_counterparty(&counterparty_node_id); let any_usable = channels.iter().any(|ch| ch.is_usable); - let has_channels = !channels.is_empty(); log_info!( self.logger, @@ -215,40 +214,23 @@ where ); } - if has_channels && !any_usable { - // Channels exist but none usable yet (channel_reestablish in progress). - // Defer until process_pending_htlcs picks them up. - log_info!( - self.logger, - "[LSPS4] htlc_intercepted: peer {} has {} channels but none usable, \ - deferring HTLC until channel_reestablish completes. payment_hash: {}", - counterparty_node_id, - channels.len(), - payment_hash - ); - self.htlc_store.insert(htlc).unwrap(); - } else { - // Either channels are usable, or no channels exist (need JIT open). - // calculate_htlc_actions_for_peer handles both: forward through usable - // channels, or emit OpenChannel event when no capacity exists. - let actions = self.calculate_htlc_actions_for_peer( - counterparty_node_id, - vec![htlc.clone()], - ); + let actions = self.calculate_htlc_actions_for_peer( + counterparty_node_id, + vec![htlc.clone()], + ); - if actions.new_channel_needed_msat.is_some() { - self.htlc_store.insert(htlc).unwrap(); - } + if actions.new_channel_needed_msat.is_some() { + self.htlc_store.insert(htlc).unwrap(); + } - log_debug!( - self.logger, - "[LSPS4] htlc_intercepted: calculated actions for peer {}: {:?}", - counterparty_node_id, - actions - ); + log_debug!( + self.logger, + "[LSPS4] htlc_intercepted: calculated actions for peer {}: {:?}", + counterparty_node_id, + actions + ); - self.execute_htlc_actions(actions, counterparty_node_id.clone()); - } + self.execute_htlc_actions(actions, counterparty_node_id.clone()); } } else { log_error!( @@ -385,28 +367,7 @@ where } } - if self.has_usable_channel(&counterparty_node_id) || channels.is_empty() { - // Either channels are usable (forward immediately) or no channels exist - // at all (process_htlcs_for_peer will trigger OpenChannel for JIT). - self.process_htlcs_for_peer(counterparty_node_id.clone(), htlcs); - } else { - // Channels exist but none usable yet (reestablish in progress). - // We still call process_htlcs_for_peer because calculate_htlc_actions - // skips non-usable channels. If existing capacity is insufficient, this - // will emit OpenChannel now rather than deferring to process_pending_htlcs - // (which would never open a channel). Forwards through existing channels - // will be empty since none are usable, so no premature forwarding occurs. - // The actual forwards happen later via channel_ready or process_pending_htlcs - // once reestablish completes. - log_info!( - self.logger, - "[LSPS4] peer_connected: {} has {} pending HTLCs, channels not yet usable \ - (reestablish in progress) - checking if new channel needed", - counterparty_node_id, - htlcs.len() - ); - self.process_htlcs_for_peer(counterparty_node_id.clone(), htlcs); - } + self.process_htlcs_for_peer(counterparty_node_id.clone(), htlcs); log_info!( self.logger, @@ -524,27 +485,14 @@ where } if self.has_usable_channel(&node_id) { - // Channel reestablish completed — forward the deferred HTLCs. + // Channel reestablish completed — attempt to forward the deferred HTLCs. log_info!( self.logger, "[LSPS4] process_pending_htlcs: forwarding {} HTLCs for peer {} (channel now usable)", htlcs.len(), node_id ); - let actions = self.calculate_htlc_actions_for_peer(node_id, htlcs); - if actions.new_channel_needed_msat.is_some() { - // A channel open is already in flight from htlc_intercepted or - // peer_connected. Skip — channel_ready will handle forwarding - // once the new channel is established. - log_info!( - self.logger, - "[LSPS4] process_pending_htlcs: peer {} needs a new channel, \ - skipping (channel open already in flight)", - node_id - ); - continue; - } - self.execute_htlc_actions(actions, node_id); + self.process_htlcs_for_peer(node_id, htlcs); } } } @@ -635,9 +583,6 @@ where let mut channel_capacity_map: HashMap = new_hash_map(); for channel in &channels { - if !channel.is_usable { - continue; - } channel_capacity_map.insert(channel.channel_id, channel.outbound_capacity_msat); log_info!( self.logger, @@ -750,12 +695,14 @@ where ) { // Execute forwards for forward_action in actions.forwards { - // Re-check peer liveness right before forwarding to narrow the - // TOCTOU window between the usability check and the actual forward. - if !self.is_peer_connected(&their_node_id) { + // Re-check channel usability right before forwarding to narrow the + // TOCTOU window. Covers both peer disconnect and disconnect+reconnect + // (where the peer is connected but the channel is still reestablishing). + if !self.has_usable_channel(&their_node_id) { log_info!( self.logger, - "[LSPS4] execute_htlc_actions: peer {} disconnected before forward, skipping HTLC {:?} (will retry on next timer tick). payment_hash: {}", + "[LSPS4] execute_htlc_actions: no usable channel for peer {}, \ + skipping HTLC {:?} (will retry on next timer tick). payment_hash: {}", their_node_id, forward_action.htlc.id(), forward_action.htlc.payment_hash() @@ -828,19 +775,19 @@ where // Handle new channel opening if let Some(channel_size_msat) = actions.new_channel_needed_msat { - log_info!( - self.logger, - "Need a new channel with peer {} for {}msat to forward HTLCs", - their_node_id, - channel_size_msat - ); + log_info!( + self.logger, + "Need a new channel with peer {} for {}msat to forward HTLCs", + their_node_id, + channel_size_msat + ); - let mut event_queue_notifier = self.pending_events.notifier(); - event_queue_notifier.enqueue(crate::events::LiquidityEvent::LSPS4Service(LSPS4ServiceEvent::OpenChannel { - their_network_key: their_node_id, - amt_to_forward_msat: channel_size_msat, - channel_count: actions.channel_count, - })); + let mut event_queue_notifier = self.pending_events.notifier(); + event_queue_notifier.enqueue(crate::events::LiquidityEvent::LSPS4Service(LSPS4ServiceEvent::OpenChannel { + their_network_key: their_node_id, + amt_to_forward_msat: channel_size_msat, + channel_count: actions.channel_count, + })); } } From 1eb09657ec1af8b560a54681174c8d1656e5447b Mon Sep 17 00:00:00 2001 From: amackillop Date: Sat, 21 Mar 2026 19:27:42 -0700 Subject: [PATCH 2/3] Track pending channel opens to prevent duplicates After the previous commit moved usability checks to execute time, the timer can call process_htlcs_for_peer repeatedly while a channel is still opening. calculate_htlc_actions_for_peer sees no is_channel_ready channels and requests a new one each time, producing duplicate OpenChannel events. Add a pending_channel_opens set (RwLock>). execute_htlc_actions inserts the peer when it emits OpenChannel, and channel_ready removes it. If the set already contains the peer, the OpenChannel is suppressed. calculate_htlc_actions_for_peer now filters by is_channel_ready instead of including all channels. Channels still opening (is_channel_ready=false) report outbound_capacity_msat but reject forwards with "Channel is still opening", consuming the InterceptId and losing the HTLC. These are zero-conf channels, so on-chain confirmation is not the issue; the channel simply hasn't finished its opening handshake yet. Reestablishing channels (is_channel_ready=true, is_usable=false) can forward once reestablish completes and are included, preserving the spurious-open fix from the previous commit. --- lightning-liquidity/src/lsps4/service.rs | 25 ++++++++++++++++++++++-- 1 file changed, 23 insertions(+), 2 deletions(-) diff --git a/lightning-liquidity/src/lsps4/service.rs b/lightning-liquidity/src/lsps4/service.rs index b96f814064f..ef09041fb9d 100644 --- a/lightning-liquidity/src/lsps4/service.rs +++ b/lightning-liquidity/src/lsps4/service.rs @@ -101,6 +101,7 @@ where scid_store: ScidStore, htlc_store: HTLCStore, connected_peers: RwLock>, + pending_channel_opens: RwLock>, config: LSPS4ServiceConfig, } @@ -128,6 +129,7 @@ where config, logger, connected_peers: RwLock::new(HashSet::new()), + pending_channel_opens: RwLock::new(HashSet::new()), }) } @@ -257,6 +259,7 @@ where pub fn channel_ready( &self, counterparty_node_id: &PublicKey, ) -> Result<(), APIError> { + self.pending_channel_opens.write().unwrap().remove(counterparty_node_id); let is_connected = self.is_peer_connected(counterparty_node_id); log_info!( @@ -581,8 +584,17 @@ where self.channel_manager.get_cm().list_channels_with_counterparty(&their_node_id); let channel_count = channels.len(); + // Include channels that finished opening (is_channel_ready) in capacity + // decisions, even if still reestablishing (is_usable=false). This prevents + // spurious channel opens during reestablish. Channels still opening + // (is_channel_ready=false) are excluded: they report outbound_capacity_msat + // but reject forwards, consuming the InterceptId and losing the HTLC. + // execute_htlc_actions checks usability before forwarding. let mut channel_capacity_map: HashMap = new_hash_map(); for channel in &channels { + if !channel.is_channel_ready { + continue; + } channel_capacity_map.insert(channel.channel_id, channel.outbound_capacity_msat); log_info!( self.logger, @@ -773,21 +785,30 @@ where } } - // Handle new channel opening + // Handle new channel opening — skip if one is already in flight. if let Some(channel_size_msat) = actions.new_channel_needed_msat { + if self.pending_channel_opens.read().unwrap().contains(&their_node_id) { + log_info!( + self.logger, + "[LSPS4] execute_htlc_actions: peer {} needs a new channel but one is \ + already opening, skipping duplicate OpenChannel", + their_node_id + ); + } else { log_info!( self.logger, "Need a new channel with peer {} for {}msat to forward HTLCs", their_node_id, channel_size_msat ); - + self.pending_channel_opens.write().unwrap().insert(their_node_id); let mut event_queue_notifier = self.pending_events.notifier(); event_queue_notifier.enqueue(crate::events::LiquidityEvent::LSPS4Service(LSPS4ServiceEvent::OpenChannel { their_network_key: their_node_id, amt_to_forward_msat: channel_size_msat, channel_count: actions.channel_count, })); + } } } From dc104703a07ee287f5032712db8a464d72aaca5d Mon Sep 17 00:00:00 2001 From: amackillop Date: Tue, 24 Mar 2026 04:44:01 -0700 Subject: [PATCH 3/3] Clear pending channel opens on peer disconnect If a channel open is in flight and the peer disconnects, the open is dead (LDK can't complete the funding handshake without a connected peer). Without this cleanup, the pending_channel_opens set would block future OpenChannel events for that peer permanently, since channel_ready never fires for a failed open. This does not reintroduce the duplicate-open problem from the previous commit. That bug was caused by the timer firing repeatedly while the peer stays connected and the channel is still opening. A disconnect/reconnect is a genuine restart of the channel lifecycle, so re-emitting OpenChannel is correct. --- lightning-liquidity/src/lsps4/service.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/lightning-liquidity/src/lsps4/service.rs b/lightning-liquidity/src/lsps4/service.rs index ef09041fb9d..82ca99a024e 100644 --- a/lightning-liquidity/src/lsps4/service.rs +++ b/lightning-liquidity/src/lsps4/service.rs @@ -442,6 +442,7 @@ where /// Will update the set of connected peers pub fn peer_disconnected(&self, counterparty_node_id: &PublicKey) { + self.pending_channel_opens.write().unwrap().remove(counterparty_node_id); let (was_present, remaining_count) = { let mut peers = self.connected_peers.write().unwrap(); let was = peers.remove(counterparty_node_id);