Free holding cell upon tx_signatures exchange
What changed, and why it matters
This patch fixes a bug in the Lightning Dev Kit where, after a special quiet period called 'quiescence' used during splicing (modifying a channel's funds), some queued payment updates were not released when the quiet period ended via the 'tx_signatures' message exchange. The fix ensures those queued updates are freed and processed, preventing payment delays or stalls during splice operations.
Apply the patch and run the new regression test. Review other quiescence exit paths to ensure no similar omissions remain. Monitor for any related stuck-payment reports in splicing workflows.
Security signals we found
Denial-of-service-like stall: queued HTLCs could remain stuck during splice, affecting channel liveness
Logic bug in state transition: quiescence exit path not accounted for
Regression test added demonstrating stuck-then-freed holding cell behavior
Evidence from the diff
The commit modifies internal_tx_signatures in channelmanager.rs to detect when a splice negotiation completes (splice_negotiated is Some), which terminates quiescence, and then calls check_free_peer_holding_cells to release any updates that were queued in the holding cell during quiescence. Previously, this exit path was missed, causing HTLCs and other updates to remain stuck. A regression test in splicing_tests.rs verifies that an outgoing HTLC queued during quiescence is freed and sent after tx_signatures exchange.
Changed components
lightning/src/ln/channelmanager.rslightning/src/ln/splicing_tests.rsLDK channel state machine / splicing flowHolding cell / quiescence managementInspect captured patch +188 / −82
diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs
index 36fbd32..930f5fe 100644
--- a/lightning/src/ln/channelmanager.rs
+++ b/lightning/src/ln/channelmanager.rs
@@ -11543,90 +11543,106 @@ This indicates a bug inside LDK. Please report this error at https://github.com/
fn internal_tx_signatures(
&self, counterparty_node_id: &PublicKey, msg: &msgs::TxSignatures,
) -> Result<(), MsgHandleErrInternal> {
- let per_peer_state = self.per_peer_state.read().unwrap();
- let peer_state_mutex = per_peer_state.get(counterparty_node_id).ok_or_else(|| {
- debug_assert!(false);
- MsgHandleErrInternal::no_such_peer(counterparty_node_id, msg.channel_id)
- })?;
- 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) {
- hash_map::Entry::Occupied(mut chan_entry) => {
- match chan_entry.get_mut().as_funded_mut() {
- Some(chan) => {
- let best_block_height = self.best_block.read().unwrap().height;
- let FundingTxSigned {
- commitment_signed,
- counterparty_initial_commitment_signed_result,
- tx_signatures,
- funding_tx,
- splice_negotiated,
- splice_locked,
- } = try_channel_entry!(
- self,
- peer_state,
- chan.tx_signatures(msg, best_block_height, &self.logger),
- chan_entry
- );
-
- // We should never be sending a `commitment_signed` in response to their
- // `tx_signatures`.
- debug_assert!(commitment_signed.is_none());
- debug_assert!(counterparty_initial_commitment_signed_result.is_none());
-
- if let Some(tx_signatures) = tx_signatures {
- peer_state.pending_msg_events.push(
- MessageSendEvent::SendTxSignatures {
- node_id: *counterparty_node_id,
- msg: tx_signatures,
- },
- );
- }
- if let Some(splice_locked) = splice_locked {
- peer_state.pending_msg_events.push(
- MessageSendEvent::SendSpliceLocked {
- node_id: *counterparty_node_id,
- msg: splice_locked,
- },
- );
- }
- if let Some((ref funding_tx, ref tx_type)) = funding_tx {
- self.broadcast_interactive_funding(
- chan,
+ let (result, 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(|| {
+ debug_assert!(false);
+ MsgHandleErrInternal::no_such_peer(counterparty_node_id, msg.channel_id)
+ })?;
+ 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) {
+ hash_map::Entry::Occupied(mut chan_entry) => {
+ match chan_entry.get_mut().as_funded_mut() {
+ Some(chan) => {
+ let best_block_height = self.best_block.read().unwrap().height;
+ let FundingTxSigned {
+ commitment_signed,
+ counterparty_initial_commitment_signed_result,
+ tx_signatures,
funding_tx,
- Some(tx_type.clone()),
- &self.logger,
+ splice_negotiated,
+ splice_locked,
+ } = try_channel_entry!(
+ self,
+ peer_state,
+ chan.tx_signatures(msg, best_block_height, &self.logger),
+ chan_entry
);
- }
- if let Some(splice_negotiated) = splice_negotiated {
- self.pending_events.lock().unwrap().push_back((
- events::Event::SplicePending {
- channel_id: msg.channel_id,
- counterparty_node_id: *counterparty_node_id,
- user_channel_id: chan.context.get_user_id(),
- new_funding_txo: splice_negotiated.funding_txo,
- channel_type: splice_negotiated.channel_type,
- new_funding_redeem_script: splice_negotiated
- .funding_redeem_script,
- },
- None,
- ));
- }
- },
- None => {
- let msg = "Got an unexpected tx_signatures message";
- let reason = ClosureReason::ProcessingError { err: msg.to_owned() };
- let err = ChannelError::Close((msg.to_owned(), reason));
- try_channel_entry!(self, peer_state, Err(err), chan_entry)
- },
- }
- Ok(())
- },
- hash_map::Entry::Vacant(_) => Err(MsgHandleErrInternal::no_such_channel_for_peer(
- counterparty_node_id,
- msg.channel_id,
- )),
- }
+
+ // We should never be sending a `commitment_signed` in response to their
+ // `tx_signatures`.
+ debug_assert!(commitment_signed.is_none());
+ debug_assert!(counterparty_initial_commitment_signed_result.is_none());
+
+ if let Some(tx_signatures) = tx_signatures {
+ peer_state.pending_msg_events.push(
+ MessageSendEvent::SendTxSignatures {
+ node_id: *counterparty_node_id,
+ msg: tx_signatures,
+ },
+ );
+ }
+ if let Some(splice_locked) = splice_locked {
+ peer_state.pending_msg_events.push(
+ MessageSendEvent::SendSpliceLocked {
+ node_id: *counterparty_node_id,
+ msg: splice_locked,
+ },
+ );
+ }
+ if let Some((ref funding_tx, ref tx_type)) = funding_tx {
+ self.broadcast_interactive_funding(
+ chan,
+ funding_tx,
+ Some(tx_type.clone()),
+ &self.logger,
+ );
+ }
+ // We consider a splice negotiated when we exchange `tx_signatures`,
+ // which also terminates quiescence.
+ let exited_quiescence = splice_negotiated.is_some();
+ if let Some(splice_negotiated) = splice_negotiated {
+ self.pending_events.lock().unwrap().push_back((
+ events::Event::SplicePending {
+ channel_id: msg.channel_id,
+ counterparty_node_id: *counterparty_node_id,
+ user_channel_id: chan.context.get_user_id(),
+ new_funding_txo: splice_negotiated.funding_txo,
+ channel_type: splice_negotiated.channel_type,
+ new_funding_redeem_script: splice_negotiated
+ .funding_redeem_script,
+ },
+ None,
+ ));
+ }
+ let holding_cell_res = if exited_quiescence {
+ self.check_free_peer_holding_cells(peer_state)
+ } else {
+ Vec::new()
+ };
+ (Ok(()), holding_cell_res)
+ },
+ None => {
+ let msg = "Got an unexpected tx_signatures message";
+ let reason = ClosureReason::ProcessingError { err: msg.to_owned() };
+ let err = ChannelError::Close((msg.to_owned(), reason));
+ try_channel_entry!(self, peer_state, Err(err), chan_entry)
+ },
+ }
+ },
+ hash_map::Entry::Vacant(_) => (
+ Err(MsgHandleErrInternal::no_such_channel_for_peer(
+ counterparty_node_id,
+ msg.channel_id,
+ )),
+ Vec::new(),
+ ),
+ }
+ };
+
+ self.handle_holding_cell_free_result(holding_cell_res);
+ result
}
fn internal_tx_abort(
diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs
index fc5c181..6727437 100644
--- a/lightning/src/ln/splicing_tests.rs
+++ b/lightning/src/ln/splicing_tests.rs
@@ -2165,6 +2165,96 @@ fn fail_splice_on_tx_complete_error() {
do_commitment_signed_dance(initiator, acceptor, &update.commitment_signed, false, false);
}
+#[test]
+fn free_holding_cell_on_tx_signatures_quiescence_exit() {
+ // Test that if there's an update in the holding cell while we're quiescent, that it gets freed
+ // upon exiting quiescence via the `tx_signatures` exchange.
+ let chanmon_cfgs = create_chanmon_cfgs(2);
+ let node_cfgs = create_node_cfgs(2, &chanmon_cfgs);
+ let config = test_default_channel_config();
+ let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[Some(config.clone()), Some(config)]);
+ let nodes = create_network(2, &node_cfgs, &node_chanmgrs);
+
+ let initiator = &nodes[0];
+ let acceptor = &nodes[1];
+ let node_id_initiator = initiator.node.get_our_node_id();
+ let node_id_acceptor = acceptor.node.get_our_node_id();
+
+ let (_, _, channel_id, _) =
+ create_announced_chan_between_nodes_with_value(&nodes, 0, 1, 100_000, 0);
+
+ let outputs = vec![TxOut {
+ value: Amount::from_sat(1_000),
+ script_pubkey: initiator.wallet_source.get_change_script().unwrap(),
+ }];
+ let contribution = initiate_splice_out(initiator, acceptor, channel_id, outputs);
+ negotiate_splice_tx(initiator, acceptor, channel_id, contribution);
+
+ // Queue an outgoing HTLC to the holding cell. It should be freed once we exit quiescence.
+ let (route, payment_hash, _payment_preimage, payment_secret) =
+ get_route_and_payment_hash!(initiator, acceptor, 1_000_000);
+ let onion = RecipientOnionFields::secret_only(payment_secret);
+ let payment_id = PaymentId(payment_hash.0);
+ initiator.node.send_payment_with_route(route, payment_hash, onion, payment_id).unwrap();
+ assert!(initiator.node.get_and_clear_pending_msg_events().is_empty());
+
+ let event = get_event!(initiator, Event::FundingTransactionReadyForSigning);
+ if let Event::FundingTransactionReadyForSigning {
+ channel_id,
+ counterparty_node_id,
+ unsigned_transaction,
+ ..
+ } = event
+ {
+ let partially_signed_tx = initiator.wallet_source.sign_tx(unsigned_transaction).unwrap();
+ initiator
+ .node
+ .funding_transaction_signed(&channel_id, &counterparty_node_id, partially_signed_tx)
+ .unwrap();
+ } else {
+ unreachable!();
+ }
+
+ let update = get_htlc_update_msgs(initiator, &node_id_acceptor);
+ acceptor.node.handle_commitment_signed(node_id_initiator, &update.commitment_signed[0]);
+ check_added_monitors(&acceptor, 1);
+
+ let msg_events = acceptor.node.get_and_clear_pending_msg_events();
+ assert_eq!(msg_events.len(), 2, "{msg_events:?}");
+ if let MessageSendEvent::UpdateHTLCs { ref updates, .. } = &msg_events[0] {
+ let commitment_signed = &updates.commitment_signed[0];
+ initiator.node.handle_commitment_signed(node_id_acceptor, commitment_signed);
+ check_added_monitors(&initiator, 1);
+ } else {
+ panic!("Unexpected event {:?}", &msg_events[0]);
+ }
+ if let MessageSendEvent::SendTxSignatures { ref msg, .. } = &msg_events[1] {
+ initiator.node.handle_tx_signatures(node_id_acceptor, msg);
+ } else {
+ panic!("Unexpected event {:?}", &msg_events[1]);
+ }
+
+ // With `tx_signatures` exchanged, we've exited quiescence and should now see the outgoing HTLC
+ // update be sent.
+ let msg_events = initiator.node.get_and_clear_pending_msg_events();
+ assert_eq!(msg_events.len(), 2, "{msg_events:?}");
+ check_added_monitors(initiator, 1); // Outgoing HTLC monitor update
+ if let MessageSendEvent::SendTxSignatures { ref msg, .. } = &msg_events[0] {
+ acceptor.node.handle_tx_signatures(node_id_initiator, msg);
+ } else {
+ panic!("Unexpected event {:?}", &msg_events[0]);
+ }
+ if let MessageSendEvent::UpdateHTLCs { updates, .. } = &msg_events[1] {
+ acceptor.node.handle_update_add_htlc(node_id_initiator, &updates.update_add_htlcs[0]);
+ do_commitment_signed_dance(acceptor, initiator, &updates.commitment_signed, false, false);
+ } else {
+ panic!("Unexpected event {:?}", &msg_events[1]);
+ }
+
+ expect_splice_pending_event(initiator, &node_id_acceptor);
+ expect_splice_pending_event(acceptor, &node_id_initiator);
+}
+
#[test]
fn fail_splice_on_channel_close() {
let chanmon_cfgs = create_chanmon_cfgs(2);
Why this scored 44/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.