Handle inferred splice_locked on reestablish first prior to updates
What changed, and why it matters
This commit fixes an ordering bug in LDK's Lightning channel reconnection logic. When two peers reconnect after a channel splice (a way to resize a Lightning channel), LDK can infer that the splice has finalized even if the explicit 'splice_locked' message was missed. Previously, LDK would release queued payment updates before applying that inferred splice, which could cause it to generate commitment transactions based on the old channel state. The fix applies the inferred splice first, then releases queued updates. The bug was found by an internal fuzz test, not reported as an external security issue.
Review and merge the patch, then run the splicing and channel monitor consistency test suites. Operators running LDK nodes with splicing enabled should upgrade to a release containing this fix to avoid potential commitment state inconsistency after reconnections.
Security signals we found
State ordering bug: pending commitment updates could be generated against pre-splice channel state
Inferred splice_locked not applied before freeing holding cells
Potential for stale commitment signatures or inconsistent channel state after reconnection
Regression test demonstrates claim held until monitor update completes
Found by internal chanmon_consistency fuzz target, not an external report
Evidence from the diff
In internal_channel_reestablish, LDK previously captured responses.inferred_splice_locked and need_lnd_workaround, then called check_free_peer_holding_cells before applying either. The patch restructures the code so that internal_channel_ready_with_funded_channel (for the LND workaround) and internal_splice_locked_with_funded_channel (for the inferred splice) are executed inside the peer-state lock before freeing holding cells. It also extracts the splice_locked handling into internal_splice_locked_with_funded_channel so it can be called both from the normal splice_locked path and from reestablish. A new regression test, test_holding_cell_claim_freed_after_inferred_splice_locked, verifies that a payment claim queued while disconnected is only released after the splice promotion monitor update completes, using an in-progress monitor update to force asynchrony.
Changed components
lightning/src/ln/channelmanager.rslightning/src/ln/splicing_tests.rsLDK channel reestablishment handlingLDK splice locking and holding cell logicInspect captured patch +257 / −116
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 2126caf..4f90b00 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -12368,43 +12368,13 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
match peer_state.channel_by_id.entry(msg.channel_id) {
hash_map::Entry::Occupied(mut chan_entry) => {
if let Some(chan) = chan_entry.get_mut().as_funded_mut() {
- let logger = WithChannelContext::from(&self.logger, &chan.context, None);
- let res = chan.channel_ready(
- &msg,
- &self.node_signer,
- self.chain_hash,
- &self.config.read().unwrap(),
- &self.best_block.read().unwrap(),
- &&logger
+ let res = self.internal_channel_ready_with_funded_channel(
+ counterparty_node_id,
+ msg,
+ chan,
+ &mut peer_state.pending_msg_events,
);
- let announcement_sigs_opt =
- try_channel_entry!(self, peer_state, res, chan_entry);
- if let Some(announcement_sigs) = announcement_sigs_opt {
- log_trace!(logger, "Sending announcement_signatures");
- peer_state.pending_msg_events.push(MessageSendEvent::SendAnnouncementSignatures {
- node_id: counterparty_node_id.clone(),
- msg: announcement_sigs,
- });
- } else if chan.context.is_usable() {
- // If we're sending an announcement_signatures, we'll send the (public)
- // channel_update after sending a channel_announcement when we receive our
- // counterparty's announcement_signatures. Thus, we only bother to send a
- // channel_update here if the channel is not public, i.e. we're not sending an
- // announcement_signatures.
- log_trace!(logger, "Sending private initial channel_update for our counterparty");
- if let Ok((msg, _, _)) = self.get_channel_update_for_unicast(chan) {
- peer_state.pending_msg_events.push(MessageSendEvent::SendChannelUpdate {
- node_id: counterparty_node_id.clone(),
- msg,
- });
- }
- }
-
- {
- let mut pending_events = self.pending_events.lock().unwrap();
- emit_initial_channel_ready_event!(pending_events, chan);
- }
-
+ try_channel_entry!(self, peer_state, res, chan_entry);
Ok(())
} else {
try_channel_entry!(self, peer_state, Err(ChannelError::close(
@@ -12417,6 +12387,49 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
}
}
+ #[rustfmt::skip]
+ fn internal_channel_ready_with_funded_channel(
+ &self, counterparty_node_id: &PublicKey, msg: &msgs::ChannelReady,
+ chan: &mut FundedChannel<SP>, pending_msg_events: &mut Vec<MessageSendEvent>,
+ ) -> Result<(), ChannelError> {
+ let logger = WithChannelContext::from(&self.logger, &chan.context, None);
+ let announcement_sigs_opt = chan.channel_ready(
+ &msg,
+ &self.node_signer,
+ self.chain_hash,
+ &self.config.read().unwrap(),
+ &self.best_block.read().unwrap(),
+ &&logger
+ )?;
+ if let Some(announcement_sigs) = announcement_sigs_opt {
+ log_trace!(logger, "Sending announcement_signatures");
+ pending_msg_events.push(MessageSendEvent::SendAnnouncementSignatures {
+ node_id: counterparty_node_id.clone(),
+ msg: announcement_sigs,
+ });
+ } else if chan.context.is_usable() {
+ // If we're sending an announcement_signatures, we'll send the (public)
+ // channel_update after sending a channel_announcement when we receive our
+ // counterparty's announcement_signatures. Thus, we only bother to send a
+ // channel_update here if the channel is not public, i.e. we're not sending an
+ // announcement_signatures.
+ log_trace!(logger, "Sending private initial channel_update for our counterparty");
+ if let Ok((msg, _, _)) = self.get_channel_update_for_unicast(chan) {
+ pending_msg_events.push(MessageSendEvent::SendChannelUpdate {
+ node_id: counterparty_node_id.clone(),
+ msg,
+ });
+ }
+ }
+
+ {
+ let mut pending_events = self.pending_events.lock().unwrap();
+ emit_initial_channel_ready_event!(pending_events, chan);
+ }
+
+ Ok(())
+ }
+
fn internal_shutdown(
&self, counterparty_node_id: &PublicKey, msg: &msgs::Shutdown,
) -> Result<(), MsgHandleErrInternal> {
@@ -13240,7 +13253,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
#[rustfmt::skip]
fn internal_channel_reestablish(&self, counterparty_node_id: &PublicKey, msg: &msgs::ChannelReestablish) -> Result<(), MsgHandleErrInternal> {
- let (inferred_splice_locked, need_lnd_workaround, holding_cell_res) = {
+ let (post_splice_locked_update, holding_cell_res) = {
let per_peer_state = self.per_peer_state.read().unwrap();
let peer_state_mutex = per_peer_state.get(counterparty_node_id).ok_or_else(|| {
@@ -13249,7 +13262,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
let logger = WithContext::from(&self.logger, Some(*counterparty_node_id), Some(msg.channel_id), None);
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
let peer_state = &mut *peer_state_lock;
- match peer_state.channel_by_id.entry(msg.channel_id) {
+ let post_splice_locked_update = match peer_state.channel_by_id.entry(msg.channel_id) {
hash_map::Entry::Occupied(mut chan_entry) => {
if let Some(chan) = chan_entry.get_mut().as_funded_mut() {
// Currently, we expect all holding cell update_adds to be dropped on peer
@@ -13285,6 +13298,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
}
}
let need_lnd_workaround = chan.context.workaround_lnd_bug_4006.take();
+ let inferred_splice_locked = responses.inferred_splice_locked;
let funding_tx_signed = if responses.tx_signatures.is_some() || responses.splice_locked.is_some() {
Some(FundingTxSigned {
tx_signatures: responses.tx_signatures,
@@ -13305,8 +13319,33 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
peer_state.pending_msg_events.push(upd);
}
- let holding_cell_res = self.check_free_peer_holding_cells(peer_state);
- (responses.inferred_splice_locked, need_lnd_workaround, holding_cell_res)
+ if let Some(channel_ready_msg) = need_lnd_workaround {
+ let res = self.internal_channel_ready_with_funded_channel(
+ counterparty_node_id,
+ &channel_ready_msg,
+ chan,
+ &mut peer_state.pending_msg_events,
+ );
+ try_channel_entry!(self, peer_state, res, chan_entry);
+ }
+
+ // A reestablish may infer a missed `splice_locked`; apply it before freeing
+ // holding cells so we don't generate commitment updates against stale splice
+ // state.
+ if let Some(splice_locked) = inferred_splice_locked {
+ let result = self.internal_splice_locked_with_funded_channel(
+ counterparty_node_id,
+ &splice_locked,
+ chan,
+ &mut peer_state.in_flight_monitor_updates,
+ &mut peer_state.monitor_update_blocked_actions,
+ &mut peer_state.pending_msg_events,
+ peer_state.is_connected,
+ );
+ try_channel_entry!(self, peer_state, result, chan_entry)
+ } else {
+ None
+ }
} else {
return try_channel_entry!(self, peer_state, Err(ChannelError::close(
"Got a channel_reestablish message for an unfunded channel!".into())), chan_entry);
@@ -13344,18 +13383,16 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
return Err(MsgHandleErrInternal::no_such_channel_for_peer(counterparty_node_id, msg.channel_id)
)
}
- }
- };
-
- self.handle_holding_cell_free_result(holding_cell_res);
+ };
- if let Some(channel_ready_msg) = need_lnd_workaround {
- self.internal_channel_ready(counterparty_node_id, &channel_ready_msg)?;
- }
+ let holding_cell_res = self.check_free_peer_holding_cells(peer_state);
+ (post_splice_locked_update, holding_cell_res)
+ };
- if let Some(splice_locked) = inferred_splice_locked {
- self.internal_splice_locked(counterparty_node_id, &splice_locked)?;
+ if let Some(data) = post_splice_locked_update {
+ self.handle_post_monitor_update_chan_resume(data);
}
+ self.handle_holding_cell_free_result(holding_cell_res);
Ok(())
}
@@ -13585,9 +13622,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
})?;
let mut peer_state_lock = peer_state_mutex.lock().unwrap();
let peer_state = &mut *peer_state_lock;
-
// Look for the channel
- match peer_state.channel_by_id.entry(msg.channel_id) {
+ let post_update_data = match peer_state.channel_by_id.entry(msg.channel_id) {
hash_map::Entry::Vacant(_) => {
return Err(MsgHandleErrInternal::no_such_channel_for_peer(
counterparty_node_id,
@@ -13596,73 +13632,16 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
},
hash_map::Entry::Occupied(mut chan_entry) => {
if let Some(chan) = chan_entry.get_mut().as_funded_mut() {
- let logger = WithChannelContext::from(&self.logger, &chan.context, None);
- let result = chan.splice_locked(
+ let result = self.internal_splice_locked_with_funded_channel(
+ counterparty_node_id,
msg,
- &self.node_signer,
- self.chain_hash,
- &self.config.read().unwrap(),
- self.best_block.read().unwrap().height,
- &&logger,
+ chan,
+ &mut peer_state.in_flight_monitor_updates,
+ &mut peer_state.monitor_update_blocked_actions,
+ &mut peer_state.pending_msg_events,
+ peer_state.is_connected,
);
- let splice_promotion = try_channel_entry!(self, peer_state, result, chan_entry);
- if let Some(splice_promotion) = splice_promotion {
- {
- let mut short_to_chan_info = self.short_to_chan_info.write().unwrap();
- insert_short_channel_id!(short_to_chan_info, chan);
- }
-
- {
- let mut pending_events = self.pending_events.lock().unwrap();
- pending_events.push_back((
- events::Event::ChannelReady {
- channel_id: chan.context.channel_id(),
- user_channel_id: chan.context.get_user_id(),
- counterparty_node_id: chan.context.get_counterparty_node_id(),
- funding_txo: Some(
- splice_promotion.funding_txo.into_bitcoin_outpoint(),
- ),
- channel_type: chan.funding.get_channel_type().clone(),
- },
- None,
- ));
- splice_promotion.discarded_funding.into_iter().for_each(
- |funding_info| {
- let event = Event::DiscardFunding {
- channel_id: chan.context.channel_id(),
- funding_info,
- };
- pending_events.push_back((event, None));
- },
- );
- }
-
- if let Some(announcement_sigs) = splice_promotion.announcement_sigs {
- log_trace!(logger, "Sending announcement_signatures",);
- peer_state.pending_msg_events.push(
- MessageSendEvent::SendAnnouncementSignatures {
- node_id: counterparty_node_id.clone(),
- msg: announcement_sigs,
- },
- );
- }
-
- if let Some(monitor_update) = splice_promotion.monitor_update {
- if let Some(data) = self.handle_new_monitor_update(
- &mut peer_state.in_flight_monitor_updates,
- &mut peer_state.monitor_update_blocked_actions,
- &mut peer_state.pending_msg_events,
- peer_state.is_connected,
- chan,
- splice_promotion.funding_txo,
- monitor_update,
- ) {
- mem::drop(peer_state_lock);
- mem::drop(per_peer_state);
- self.handle_post_monitor_update_chan_resume(data);
- }
- }
- }
+ try_channel_entry!(self, peer_state, result, chan_entry)
} else {
return Err(MsgHandleErrInternal::send_err_msg_no_close(
"Channel is not funded, cannot splice".to_owned(),
@@ -13671,10 +13650,87 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
}
},
};
+ mem::drop(peer_state_lock);
+ mem::drop(per_peer_state);
+
+ if let Some(data) = post_update_data {
+ self.handle_post_monitor_update_chan_resume(data);
+ }
Ok(())
}
+ fn internal_splice_locked_with_funded_channel(
+ &self, counterparty_node_id: &PublicKey, msg: &msgs::SpliceLocked,
+ chan: &mut FundedChannel<SP>,
+ in_flight_monitor_updates: &mut BTreeMap<ChannelId, (OutPoint, Vec<ChannelMonitorUpdate>)>,
+ monitor_update_blocked_actions: &mut BTreeMap<
+ ChannelId,
+ Vec<MonitorUpdateCompletionAction>,
+ >,
+ pending_msg_events: &mut Vec<MessageSendEvent>, is_connected: bool,
+ ) -> Result<Option<PostMonitorUpdateChanResume>, ChannelError> {
+ let logger = WithChannelContext::from(&self.logger, &chan.context, None);
+ let splice_promotion = chan.splice_locked(
+ msg,
+ &self.node_signer,
+ self.chain_hash,
+ &self.config.read().unwrap(),
+ self.best_block.read().unwrap().height,
+ &&logger,
+ )?;
+ let mut post_update_data = None;
+ if let Some(splice_promotion) = splice_promotion {
+ {
+ let mut short_to_chan_info = self.short_to_chan_info.write().unwrap();
+ insert_short_channel_id!(short_to_chan_info, chan);
+ }
+
+ {
+ let mut pending_events = self.pending_events.lock().unwrap();
+ pending_events.push_back((
+ events::Event::ChannelReady {
+ channel_id: chan.context.channel_id(),
+ user_channel_id: chan.context.get_user_id(),
+ counterparty_node_id: chan.context.get_counterparty_node_id(),
+ funding_txo: Some(splice_promotion.funding_txo.into_bitcoin_outpoint()),
+ channel_type: chan.funding.get_channel_type().clone(),
+ },
+ None,
+ ));
+ splice_promotion.discarded_funding.into_iter().for_each(|funding_info| {
+ let event = Event::DiscardFunding {
+ channel_id: chan.context.channel_id(),
+ funding_info,
+ };
+ pending_events.push_back((event, None));
+ });
+ }
+
+ if let Some(announcement_sigs) = splice_promotion.announcement_sigs {
+ log_trace!(logger, "Sending announcement_signatures",);
+ pending_msg_events.push(MessageSendEvent::SendAnnouncementSignatures {
+ node_id: counterparty_node_id.clone(),
+ msg: announcement_sigs,
+ });
+ }
+
+ if let Some(monitor_update) = splice_promotion.monitor_update {
+ post_update_data = self.handle_new_monitor_update(
+ in_flight_monitor_updates,
+ monitor_update_blocked_actions,
+ pending_msg_events,
+ is_connected,
+ chan,
+ splice_promotion.funding_txo,
+ monitor_update,
+ );
+ }
+ }
+
+ Ok(post_update_data)
+ }
+
/// Process pending events from the [`chain::Watch`], returning whether any events were processed.
fn process_pending_monitor_events(&self) -> bool {
debug_assert!(self.total_consistency_lock.try_write().is_err()); // Caller holds read lock
diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs
index ca45a39..6e6af60 100644
--- a/lightning/src/ln/splicing_tests.rs
+++ b/lightning/src/ln/splicing_tests.rs
@@ -2794,6 +2794,91 @@ fn test_splice_confirms_on_both_sides_while_disconnected() {
.remove_watched_txn_and_outputs(prev_funding_outpoint, prev_funding_script);
}
+#[test]
+fn test_holding_cell_claim_freed_after_inferred_splice_locked() {
+ // If `channel_reestablish` infers a missed `splice_locked`, it must promote the splice before
+ // freeing holding-cell updates. If the promotion monitor update is asynchronous, holding-cell
+ // updates must remain held until that monitor update completes.
+ let chanmon_cfgs = create_chanmon_cfgs(2);
+ let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+ let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, None]);
+ let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+ let node_id_0 = nodes[0].node.get_our_node_id();
+ let node_id_1 = nodes[1].node.get_our_node_id();
+
+ let initial_channel_value_sat = 100_000;
+ let (_, _, channel_id, _) =
+ create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_value_sat, 0);
+ let prev_funding_outpoint = get_monitor!(nodes[0], channel_id).get_funding_txo();
+ let prev_funding_script = get_monitor!(nodes[0], channel_id).get_funding_script();
+ let prev_scid = nodes[0].node.list_channels()[0].short_channel_id;
+
+ let (payment_preimage, payment_hash, ..) = route_payment(&nodes[0], &[&nodes[1]], 1_000_000);
+
+ let outputs = vec![
+ TxOut {
+ value: Amount::from_sat(initial_channel_value_sat / 4),
+ script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(),
+ },
+ TxOut {
+ value: Amount::from_sat(initial_channel_value_sat / 4),
+ script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(),
+ },
+ ];
+ let funding_contribution =
+ initiate_splice_out(&nodes[0], &nodes[1], channel_id, outputs).unwrap();
+ let (splice_tx, _) = splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution);
+
+ nodes[0].node.peer_disconnected(node_id_1);
+ nodes[1].node.peer_disconnected(node_id_0);
+
+ nodes[1].node.claim_funds(payment_preimage);
+ check_added_monitors(&nodes[1], 1);
+ expect_payment_claimed!(nodes[1], payment_hash, 1_000_000);
+
+ confirm_transaction(&nodes[0], &splice_tx);
+ confirm_transaction(&nodes[1], &splice_tx);
+ assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty());
+ assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty());
+
+ chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::InProgress);
+
+ let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]);
+ reconnect_args.expect_renegotiated_funding_locked_monitor_update = (true, true);
+ reconnect_args.send_announcement_sigs = (true, true);
+ reconnect_nodes(reconnect_args);
+
+ expect_channel_ready_event(&nodes[0], &node_id_1);
+ expect_channel_ready_event(&nodes[1], &node_id_0);
+ assert_ne!(prev_scid, nodes[0].node.list_channels()[0].short_channel_id);
+
+ nodes[1].chain_monitor.complete_sole_pending_chan_update(&channel_id);
+ chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed);
+
+ let mut commitment_update = get_htlc_update_msgs(&nodes[1], &node_id_0);
+ check_added_monitors(&nodes[1], 1);
+ nodes[0]
+ .node
+ .handle_update_fulfill_htlc(node_id_1, commitment_update.update_fulfill_htlcs.remove(0));
+ do_commitment_signed_dance(
+ &nodes[0],
+ &nodes[1],
+ &commitment_update.commitment_signed,
+ false,
+ false,
+ );
+
+ expect_payment_sent!(nodes[0], payment_preimage);
+
+ nodes[0]
+ .chain_source
+ .remove_watched_txn_and_outputs(prev_funding_outpoint, prev_funding_script.clone());
+ nodes[1]
+ .chain_source
+ .remove_watched_txn_and_outputs(prev_funding_outpoint, prev_funding_script);
+}
+
#[test]
fn test_stale_announcement_signatures_ignored_after_splice_lock() {
// Regression test: a peer may transmit `announcement_signatures` signed over a pre-splice
Why this scored 59/100
Community notes
Notes can correct, qualify, or add evidence to the AI analysis. Every note shown here has been validated by a human moderator.
The AI analysis stands alone for now. Submit a note if you can add evidence or important context.