From 12998cee963810098921c9ad8a6a47eedacf98bf Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 17 Mar 2026 15:25:55 -0500 Subject: [PATCH 1/7] Exit quiescence when tx_init_rbf is rejected with Abort When tx_init_rbf is rejected with ChannelError::Abort (e.g., insufficient RBF feerate, negotiation in progress, feerate too high), the error is converted to a tx_abort message but quiescence is never exited and holding cells are never freed. This leaves the channel stuck in a quiescent state. Fix this by intercepting ChannelError::Abort before try_channel_entry! in internal_tx_init_rbf, calling exit_quiescence on the channel, and returning the error with exited_quiescence set so that handle_error frees holding cells. Also make exit_quiescence available in non-test builds by removing its cfg gate. Update tests to use the proper RBF initiation flow (with tampered feerates) so that handle_tx_abort correctly echoes the abort and exits quiescence, rather than manually crafting tx_init_rbf messages that leave node 0 without proper negotiation state. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/channel.rs | 2 - lightning/src/ln/channelmanager.rs | 9 ++++ lightning/src/ln/splicing_tests.rs | 72 ++++++++++++++++++++++-------- 3 files changed, 63 insertions(+), 20 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 32c0e94bdc8..61e61a3c9aa 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -14237,8 +14237,6 @@ where Some(msgs::Stfu { channel_id: self.context.channel_id, initiator }) } - #[cfg(any(test, fuzzing, feature = "_test_utils"))] - #[rustfmt::skip] pub fn exit_quiescence(&mut self) -> bool { // Make sure we either finished the quiescence handshake and are quiescent, or we never // attempted to initiate quiescence at all. diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 2e782701e47..202d191a6df 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -13309,6 +13309,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self.fee_estimator, &self.logger, ); + if let Err(ChannelError::Abort(_)) = &init_res { + funded_channel.exit_quiescence(); + let chan_id = funded_channel.context.channel_id(); + let res = MsgHandleErrInternal::from_chan_no_close( + init_res.unwrap_err(), + chan_id, + ); + return Err(res.with_exited_quiescence(true)); + } let tx_ack_rbf_msg = try_channel_entry!(self, peer_state, init_res, chan_entry); peer_state.pending_msg_events.push(MessageSendEvent::SendTxAckRbf { node_id: *counterparty_node_id, diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 9adccd17627..b61caa4ea63 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -4586,33 +4586,54 @@ fn test_splice_rbf_insufficient_feerate() { .is_ok()); // Acceptor-side: tx_init_rbf with an insufficient feerate is also rejected. - reenter_quiescence(&nodes[0], &nodes[1], &channel_id); + // Node 0 initiates a proper RBF but we tamper the feerate to be insufficient. + provide_utxo_reserves(&nodes, 2, added_value * 2); + let _funding_contribution = + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, min_rbf_feerate); - let tx_init_rbf = msgs::TxInitRbf { - channel_id, - locktime: 0, - feerate_sat_per_1000_weight: FEERATE_FLOOR_SATS_PER_KW, - funding_output_contribution: Some(added_value.to_sat() as i64), - }; + let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + nodes[1].node.handle_stfu(node_id_0, &stfu_0); + let stfu_1 = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); + nodes[0].node.handle_stfu(node_id_1, &stfu_1); + let mut tx_init_rbf = get_event_msg!(nodes[0], MessageSendEvent::SendTxInitRbf, node_id_1); + tx_init_rbf.feerate_sat_per_1000_weight = FEERATE_FLOOR_SATS_PER_KW; nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); let tx_abort = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); assert_eq!(tx_abort.channel_id, channel_id); + // Node 0 echoes tx_abort and exits quiescence. + nodes[0].node.handle_tx_abort(node_id_1, &tx_abort); + let tx_abort_echo = get_event_msg!(nodes[0], MessageSendEvent::SendTxAbort, node_id_1); + + let events = nodes[0].node.get_and_clear_pending_events(); + assert_eq!(events.len(), 2); + assert!( + matches!(&events[0], Event::SpliceFailed { channel_id: cid, .. } if *cid == channel_id) + ); + assert!( + matches!(&events[1], Event::DiscardFunding { channel_id: cid, .. } if *cid == channel_id) + ); + + // Node 1 handles the echo (no-op since it already aborted). + nodes[1].node.handle_tx_abort(node_id_0, &tx_abort_echo); + // Acceptor-side: a counterparty feerate that satisfies the spec's 25/24 rule (264) is // accepted, even though our own RBF floor (+25 sat/kwu = 278) is higher. - // After tx_abort the channel remains quiescent, so no need to re-enter quiescence. - nodes[0].node.handle_tx_abort(node_id_1, &tx_abort); + // Node 0 initiates another proper RBF but we tamper the feerate to the 25/24 value. + provide_utxo_reserves(&nodes, 2, added_value * 2); + let _funding_contribution = + do_initiate_rbf_splice_in(&nodes[0], &nodes[1], channel_id, added_value, min_rbf_feerate); - let rbf_feerate_25_24 = ((FEERATE_FLOOR_SATS_PER_KW as u64) * 25).div_ceil(24) as u32; - let tx_init_rbf = msgs::TxInitRbf { - channel_id, - locktime: 0, - feerate_sat_per_1000_weight: rbf_feerate_25_24, - funding_output_contribution: Some(added_value.to_sat() as i64), - }; + let stfu_0 = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + nodes[1].node.handle_stfu(node_id_0, &stfu_0); + let stfu_1 = get_event_msg!(nodes[1], MessageSendEvent::SendStfu, node_id_0); + nodes[0].node.handle_stfu(node_id_1, &stfu_1); + let mut tx_init_rbf = get_event_msg!(nodes[0], MessageSendEvent::SendTxInitRbf, node_id_1); + let rbf_feerate_25_24 = ((FEERATE_FLOOR_SATS_PER_KW as u64) * 25).div_ceil(24) as u32; + tx_init_rbf.feerate_sat_per_1000_weight = rbf_feerate_25_24; nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); let _tx_ack_rbf = get_event_msg!(nodes[1], MessageSendEvent::SendTxAckRbf, node_id_0); } @@ -5300,10 +5321,25 @@ fn test_splice_rbf_tiebreak_feerate_too_high_rejected() { assert_eq!(tx_init_rbf.feerate_sat_per_1000_weight, high_feerate.to_sat_per_kwu() as u32); // Node 1 handles tx_init_rbf — TooHigh: target (100k) >> max (3k) and fair fee > budget. + // Node 1 exits quiescence upon rejecting with tx_abort, and since it has a pending + // QuiescentAction (from its own splice RBF attempt), it immediately re-proposes quiescence. nodes[1].node.handle_tx_init_rbf(node_id_0, &tx_init_rbf); - let tx_abort = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); - assert_eq!(tx_abort.channel_id, channel_id); + let msg_events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 2); + match &msg_events[0] { + MessageSendEvent::SendTxAbort { node_id, msg } => { + assert_eq!(*node_id, node_id_0); + assert_eq!(msg.channel_id, channel_id); + }, + _ => panic!("Expected SendTxAbort, got {:?}", msg_events[0]), + }; + match &msg_events[1] { + MessageSendEvent::SendStfu { node_id, .. } => { + assert_eq!(*node_id, node_id_0); + }, + _ => panic!("Expected SendStfu, got {:?}", msg_events[1]), + }; } #[test] From 3c4652930457b65d8704b5f324ecfa3c455e6fec Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Fri, 3 Apr 2026 16:34:06 -0500 Subject: [PATCH 2/7] f - Verify holding cell is freed after tx_abort exits quiescence Add a payment during quiescence in test_splice_rbf_insufficient_feerate to verify that outbound HTLCs queued in the holding cell are freed when quiescence is exited by the tx_abort exchange. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/splicing_tests.rs | 37 +++++++++++++++++++++++++++--- 1 file changed, 34 insertions(+), 3 deletions(-) diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index b61caa4ea63..78000e154e3 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -4603,12 +4603,20 @@ fn test_splice_rbf_insufficient_feerate() { let tx_abort = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); assert_eq!(tx_abort.channel_id, channel_id); - // Node 0 echoes tx_abort and exits quiescence. + // Queue a payment while quiescent. It should go to the holding cell and be freed once + // quiescence is exited by the tx_abort exchange. + let (route, payment_hash, _payment_preimage, payment_secret) = + get_route_and_payment_hash!(nodes[0], nodes[1], 1_000_000); + let onion = RecipientOnionFields::secret_only(payment_secret, 1_000_000); + let payment_id = PaymentId(payment_hash.0); + nodes[0].node.send_payment_with_route(route, payment_hash, onion, payment_id).unwrap(); + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + + // Node 0 echoes tx_abort and exits quiescence, freeing the holding cell. nodes[0].node.handle_tx_abort(node_id_1, &tx_abort); - let tx_abort_echo = get_event_msg!(nodes[0], MessageSendEvent::SendTxAbort, node_id_1); let events = nodes[0].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 2); + assert_eq!(events.len(), 2, "{events:?}"); assert!( matches!(&events[0], Event::SpliceFailed { channel_id: cid, .. } if *cid == channel_id) ); @@ -4616,6 +4624,29 @@ fn test_splice_rbf_insufficient_feerate() { matches!(&events[1], Event::DiscardFunding { channel_id: cid, .. } if *cid == channel_id) ); + let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 2, "{msg_events:?}"); + let tx_abort_echo = match &msg_events[0] { + MessageSendEvent::SendTxAbort { msg, .. } => msg.clone(), + other => panic!("Expected SendTxAbort, got {:?}", other), + }; + match &msg_events[1] { + MessageSendEvent::UpdateHTLCs { updates, .. } => { + assert_eq!(updates.update_add_htlcs.len(), 1); + }, + other => panic!("Expected UpdateHTLCs, got {:?}", other), + } + + // Complete the HTLC commitment exchange so the channel is ready for the next RBF attempt. + // The holding cell free generated a monitor update for the outgoing HTLC. + check_added_monitors(&nodes[0], 1); + if let MessageSendEvent::UpdateHTLCs { updates, .. } = &msg_events[1] { + nodes[1].node.handle_update_add_htlc(node_id_0, &updates.update_add_htlcs[0]); + do_commitment_signed_dance(&nodes[1], &nodes[0], &updates.commitment_signed, false, false); + } else { + unreachable!(); + } + // Node 1 handles the echo (no-op since it already aborted). nodes[1].node.handle_tx_abort(node_id_0, &tx_abort_echo); From 0d4c84fcade033221acdd876abe492389267eadd Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 17 Mar 2026 15:43:46 -0500 Subject: [PATCH 3/7] Exit quiescence when splice_init is rejected with Abort The same bug fixed in the prior commit for tx_init_rbf also exists in internal_splice_init: when splice_init triggers FeeRateTooHigh in resolve_queued_contribution, the ChannelError::Abort goes through try_channel_entry! without exiting quiescence. Apply the same fix: intercept ChannelError::Abort before try_channel_entry!, call exit_quiescence, and return the error with exited_quiescence set. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/channelmanager.rs | 9 +++++++++ lightning/src/ln/splicing_tests.rs | 19 +++++++++++++++++-- 2 files changed, 26 insertions(+), 2 deletions(-) diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 202d191a6df..79f2e7933d7 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -13264,6 +13264,15 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ &self.get_our_node_id(), &self.logger, ); + if let Err(ChannelError::Abort(_)) = &init_res { + funded_channel.exit_quiescence(); + let chan_id = funded_channel.context.channel_id(); + let res = MsgHandleErrInternal::from_chan_no_close( + init_res.unwrap_err(), + chan_id, + ); + return Err(res.with_exited_quiescence(true)); + } let splice_ack_msg = try_channel_entry!(self, peer_state, init_res, chan_entry); peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceAck { node_id: *counterparty_node_id, diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index 78000e154e3..c4069e4f0f8 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -1816,10 +1816,25 @@ fn test_splice_tiebreak_feerate_too_high_rejected() { let splice_init = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceInit, node_id_1); // Node 1 handles SpliceInit — TooHigh: target (100k) >> max (3k) and fair fee > budget. + // Node 1 exits quiescence upon rejecting with tx_abort, and since it has a pending + // QuiescentAction (from its own splice attempt), it immediately re-proposes quiescence. nodes[1].node.handle_splice_init(node_id_0, &splice_init); - let tx_abort = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); - assert_eq!(tx_abort.channel_id, channel_id); + let msg_events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 2); + match &msg_events[0] { + MessageSendEvent::SendTxAbort { node_id, msg } => { + assert_eq!(*node_id, node_id_0); + assert_eq!(msg.channel_id, channel_id); + }, + _ => panic!("Expected SendTxAbort, got {:?}", msg_events[0]), + }; + match &msg_events[1] { + MessageSendEvent::SendStfu { node_id, .. } => { + assert_eq!(*node_id, node_id_0); + }, + _ => panic!("Expected SendStfu, got {:?}", msg_events[1]), + }; } #[cfg(test)] From 23796864db295676b7f279bcbae133ceee39714e Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 17 Mar 2026 16:31:10 -0500 Subject: [PATCH 4/7] Return InteractiveTxMsgError from splice_init and tx_init_rbf The prior two commits manually intercepted ChannelError::Abort in the channelmanager handlers for splice_init and tx_init_rbf to exit quiescence before returning, since the channel methods didn't signal this themselves. The interactive TX message handlers already solved this by returning InteractiveTxMsgError which bundles exited_quiescence into the error type. Apply the same pattern: change splice_init and tx_init_rbf to return InteractiveTxMsgError, adding a quiescent_negotiation_err helper on FundedChannel that exits quiescence for Abort errors and passes through other variants unchanged. Extract handle_interactive_tx_msg_err in channelmanager to deduplicate the error handling across internal_tx_msg, internal_splice_init, and internal_tx_init_rbf. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/channel.rs | 40 +++++--- lightning/src/ln/channelmanager.rs | 143 ++++++++++++++++------------- 2 files changed, 105 insertions(+), 78 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 61e61a3c9aa..8105042d4ce 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -12890,13 +12890,15 @@ where pub(crate) fn splice_init( &mut self, msg: &msgs::SpliceInit, entropy_source: &ES, holder_node_id: &PublicKey, logger: &L, - ) -> Result { + ) -> Result { let feerate = FeeRate::from_sat_per_kwu(msg.funding_feerate_per_kw as u64); - let (our_funding_contribution, holder_balance) = - self.resolve_queued_contribution(feerate, logger)?; + let (our_funding_contribution, holder_balance) = self + .resolve_queued_contribution(feerate, logger) + .map_err(|e| self.quiescent_negotiation_err(e))?; - let splice_funding = - self.validate_splice_init(msg, our_funding_contribution.unwrap_or(SignedAmount::ZERO))?; + let splice_funding = self + .validate_splice_init(msg, our_funding_contribution.unwrap_or(SignedAmount::ZERO)) + .map_err(|e| self.quiescent_negotiation_err(e))?; // Adjust for the feerate and clone so we can store it for future RBF re-use. let (adjusted_contribution, our_funding_inputs, our_funding_outputs) = @@ -13048,10 +13050,11 @@ where pub(crate) fn tx_init_rbf( &mut self, msg: &msgs::TxInitRbf, entropy_source: &ES, holder_node_id: &PublicKey, fee_estimator: &LowerBoundedFeeEstimator, logger: &L, - ) -> Result { + ) -> Result { let feerate = FeeRate::from_sat_per_kwu(msg.feerate_sat_per_1000_weight as u64); - let (queued_net_value, holder_balance) = - self.resolve_queued_contribution(feerate, logger)?; + let (queued_net_value, holder_balance) = self + .resolve_queued_contribution(feerate, logger) + .map_err(|e| self.quiescent_negotiation_err(e))?; // If no queued contribution, try prior contribution from previous negotiation. // Failing here means the RBF would erase our splice — reject it. @@ -13068,7 +13071,8 @@ where prior .net_value_for_acceptor_at_feerate(feerate, holder_balance) .map_err(|_| ChannelError::Abort(AbortReason::InsufficientRbfFeerate)) - })?; + }) + .map_err(|e| self.quiescent_negotiation_err(e))?; Some(net_value) } else { None @@ -13076,11 +13080,13 @@ where let our_funding_contribution = queued_net_value.or(prior_net_value); - let rbf_funding = self.validate_tx_init_rbf( - msg, - our_funding_contribution.unwrap_or(SignedAmount::ZERO), - fee_estimator, - )?; + let rbf_funding = self + .validate_tx_init_rbf( + msg, + our_funding_contribution.unwrap_or(SignedAmount::ZERO), + fee_estimator, + ) + .map_err(|e| self.quiescent_negotiation_err(e))?; // Consume the appropriate contribution source. let (our_funding_inputs, our_funding_outputs) = if queued_net_value.is_some() { @@ -14249,6 +14255,12 @@ where was_quiescent } + fn quiescent_negotiation_err(&mut self, err: ChannelError) -> InteractiveTxMsgError { + let exited_quiescence = + if matches!(err, ChannelError::Abort(_)) { self.exit_quiescence() } else { false }; + InteractiveTxMsgError { err, splice_funding_failed: None, exited_quiescence } + } + pub fn remove_legacy_scids_before_block(&mut self, height: u32) -> alloc::vec::Drain<'_, u64> { let end = self .funding diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 79f2e7933d7..b67d8dd7e6f 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -11823,6 +11823,39 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } } + fn handle_interactive_tx_msg_err( + &self, err: InteractiveTxMsgError, channel_id: ChannelId, counterparty_node_id: &PublicKey, + user_channel_id: u128, + ) -> MsgHandleErrInternal { + if let Some(splice_funding_failed) = err.splice_funding_failed { + let pending_events = &mut self.pending_events.lock().unwrap(); + pending_events.push_back(( + events::Event::SpliceFailed { + channel_id, + counterparty_node_id: *counterparty_node_id, + user_channel_id, + abandoned_funding_txo: splice_funding_failed.funding_txo, + channel_type: splice_funding_failed.channel_type.clone(), + }, + None, + )); + pending_events.push_back(( + events::Event::DiscardFunding { + channel_id, + funding_info: FundingInfo::Contribution { + inputs: splice_funding_failed.contributed_inputs, + outputs: splice_funding_failed.contributed_outputs, + }, + }, + None, + )); + } + debug_assert!(!err.exited_quiescence || matches!(err.err, ChannelError::Abort(_))); + + MsgHandleErrInternal::from_chan_no_close(err.err, channel_id) + .with_exited_quiescence(err.exited_quiescence) + } + fn internal_tx_msg< HandleTxMsgFn: Fn(&mut Channel) -> Result, >( @@ -11844,38 +11877,14 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ peer_state.pending_msg_events.push(msg_send_event); Ok(()) }, - Err(InteractiveTxMsgError { - err, - splice_funding_failed, - exited_quiescence, - }) => { - if let Some(splice_funding_failed) = splice_funding_failed { - let pending_events = &mut self.pending_events.lock().unwrap(); - pending_events.push_back(( - events::Event::SpliceFailed { - channel_id, - counterparty_node_id: *counterparty_node_id, - user_channel_id: channel.context().get_user_id(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type.clone(), - }, - None, - )); - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id, - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, - }, - }, - None, - )); - } - debug_assert!(!exited_quiescence || matches!(err, ChannelError::Abort(_))); - - Err(MsgHandleErrInternal::from_chan_no_close(err, channel_id) - .with_exited_quiescence(exited_quiescence)) + Err(err) => { + let user_channel_id = channel.context().get_user_id(); + Err(self.handle_interactive_tx_msg_err( + err, + channel_id, + counterparty_node_id, + user_channel_id, + )) }, } }, @@ -13258,27 +13267,30 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } if let Some(ref mut funded_channel) = chan_entry.get_mut().as_funded_mut() { - let init_res = funded_channel.splice_init( + let user_channel_id = funded_channel.context.get_user_id(); + match funded_channel.splice_init( msg, &self.entropy_source, &self.get_our_node_id(), &self.logger, - ); - if let Err(ChannelError::Abort(_)) = &init_res { - funded_channel.exit_quiescence(); - let chan_id = funded_channel.context.channel_id(); - let res = MsgHandleErrInternal::from_chan_no_close( - init_res.unwrap_err(), - chan_id, - ); - return Err(res.with_exited_quiescence(true)); + ) { + Ok(splice_ack_msg) => { + peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceAck { + node_id: *counterparty_node_id, + msg: splice_ack_msg, + }); + Ok(()) + }, + Err(err) => { + debug_assert!(err.splice_funding_failed.is_none()); + Err(self.handle_interactive_tx_msg_err( + err, + msg.channel_id, + counterparty_node_id, + user_channel_id, + )) + }, } - let splice_ack_msg = try_channel_entry!(self, peer_state, init_res, chan_entry); - peer_state.pending_msg_events.push(MessageSendEvent::SendSpliceAck { - node_id: *counterparty_node_id, - msg: splice_ack_msg, - }); - Ok(()) } else { try_channel_entry!( self, @@ -13311,28 +13323,31 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ }, hash_map::Entry::Occupied(mut chan_entry) => { if let Some(ref mut funded_channel) = chan_entry.get_mut().as_funded_mut() { - let init_res = funded_channel.tx_init_rbf( + let user_channel_id = funded_channel.context.get_user_id(); + match funded_channel.tx_init_rbf( msg, &self.entropy_source, &self.get_our_node_id(), &self.fee_estimator, &self.logger, - ); - if let Err(ChannelError::Abort(_)) = &init_res { - funded_channel.exit_quiescence(); - let chan_id = funded_channel.context.channel_id(); - let res = MsgHandleErrInternal::from_chan_no_close( - init_res.unwrap_err(), - chan_id, - ); - return Err(res.with_exited_quiescence(true)); + ) { + Ok(tx_ack_rbf_msg) => { + peer_state.pending_msg_events.push(MessageSendEvent::SendTxAckRbf { + node_id: *counterparty_node_id, + msg: tx_ack_rbf_msg, + }); + Ok(()) + }, + Err(err) => { + debug_assert!(err.splice_funding_failed.is_none()); + Err(self.handle_interactive_tx_msg_err( + err, + msg.channel_id, + counterparty_node_id, + user_channel_id, + )) + }, } - let tx_ack_rbf_msg = try_channel_entry!(self, peer_state, init_res, chan_entry); - peer_state.pending_msg_events.push(MessageSendEvent::SendTxAckRbf { - node_id: *counterparty_node_id, - msg: tx_ack_rbf_msg, - }); - Ok(()) } else { try_channel_entry!( self, From e4b1c6bb7922d8ba126b6be4d3e64164e4232177 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 2 Apr 2026 17:41:17 -0500 Subject: [PATCH 5/7] f - Defer resolve_queued_contribution until after validation validate_splice_init and validate_tx_init_rbf check preconditions including quiescence. Move them before resolve_queued_contribution so that a misbehaving peer sending splice_init or tx_init_rbf before quiescence is rejected early. This also moves validate_splice_contributions and FundingScope construction into the callers since they depend on the resolved contribution. Have validate_tx_init_rbf return the last candidate's funding pubkeys so the caller can construct FundingScope without re-accessing pending_splice. Add a debug_assert in quiescent_negotiation_err that Abort errors only occur when the channel is quiescent, since exit_quiescence requires it. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/channel.rs | 132 ++++++++++++++--------------- lightning/src/ln/splicing_tests.rs | 90 ++++++++++++++++++++ 2 files changed, 155 insertions(+), 67 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 8105042d4ce..9ded88af61e 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -12686,9 +12686,7 @@ where } /// Checks during handling splice_init - pub fn validate_splice_init( - &self, msg: &msgs::SpliceInit, our_funding_contribution: SignedAmount, - ) -> Result { + pub fn validate_splice_init(&self, msg: &msgs::SpliceInit) -> Result<(), ChannelError> { if self.holder_commitment_point.current_point().is_none() { return Err(ChannelError::WarnAndDisconnect(format!( "Channel {} commitment point needs to be advanced once before spliced", @@ -12725,32 +12723,7 @@ where ))); } - self.validate_splice_contributions(our_funding_contribution, their_funding_contribution) - .map_err(|e| ChannelError::WarnAndDisconnect(e))?; - - // Rotate the pubkeys using the prev_funding_txid as a tweak - let prev_funding_txid = self.funding.get_funding_txid(); - let funding_pubkey = match prev_funding_txid { - None => { - debug_assert!(false); - self.funding.get_holder_pubkeys().funding_pubkey - }, - Some(prev_funding_txid) => self - .context - .holder_signer - .new_funding_pubkey(prev_funding_txid, &self.context.secp_ctx), - }; - let mut new_keys = self.funding.get_holder_pubkeys().clone(); - new_keys.funding_pubkey = funding_pubkey; - - Ok(FundingScope::for_splice( - &self.funding, - &self.context, - our_funding_contribution, - their_funding_contribution, - msg.funding_pubkey, - new_keys, - )) + Ok(()) } fn validate_splice_contributions( @@ -12891,18 +12864,45 @@ where &mut self, msg: &msgs::SpliceInit, entropy_source: &ES, holder_node_id: &PublicKey, logger: &L, ) -> Result { + self.validate_splice_init(msg).map_err(|e| self.quiescent_negotiation_err(e))?; + let feerate = FeeRate::from_sat_per_kwu(msg.funding_feerate_per_kw as u64); - let (our_funding_contribution, holder_balance) = self + let (queued_net_value, holder_balance) = self .resolve_queued_contribution(feerate, logger) .map_err(|e| self.quiescent_negotiation_err(e))?; - let splice_funding = self - .validate_splice_init(msg, our_funding_contribution.unwrap_or(SignedAmount::ZERO)) - .map_err(|e| self.quiescent_negotiation_err(e))?; + let our_funding_contribution = queued_net_value.unwrap_or(SignedAmount::ZERO); + let their_funding_contribution = SignedAmount::from_sat(msg.funding_contribution_satoshis); + self.validate_splice_contributions(our_funding_contribution, their_funding_contribution) + .map_err(|e| self.quiescent_negotiation_err(ChannelError::WarnAndDisconnect(e)))?; + + // Rotate the pubkeys using the prev_funding_txid as a tweak + let prev_funding_txid = self.funding.get_funding_txid(); + let funding_pubkey = match prev_funding_txid { + None => { + debug_assert!(false); + self.funding.get_holder_pubkeys().funding_pubkey + }, + Some(prev_funding_txid) => self + .context + .holder_signer + .new_funding_pubkey(prev_funding_txid, &self.context.secp_ctx), + }; + let mut holder_pubkeys = self.funding.get_holder_pubkeys().clone(); + holder_pubkeys.funding_pubkey = funding_pubkey; + + let splice_funding = FundingScope::for_splice( + &self.funding, + &self.context, + our_funding_contribution, + their_funding_contribution, + msg.funding_pubkey, + holder_pubkeys, + ); // Adjust for the feerate and clone so we can store it for future RBF re-use. let (adjusted_contribution, our_funding_inputs, our_funding_outputs) = - if our_funding_contribution.is_some() { + if queued_net_value.is_some() { let adjusted_contribution = self .take_queued_funding_contribution() .expect("queued_funding_contribution was Some") @@ -12913,7 +12913,6 @@ where } else { (None, Default::default(), Default::default()) }; - let our_funding_contribution = our_funding_contribution.unwrap_or(SignedAmount::ZERO); log_info!( logger, @@ -12956,9 +12955,8 @@ where /// Checks during handling tx_init_rbf for an existing splice fn validate_tx_init_rbf( - &self, msg: &msgs::TxInitRbf, our_funding_contribution: SignedAmount, - fee_estimator: &LowerBoundedFeeEstimator, - ) -> Result { + &self, msg: &msgs::TxInitRbf, fee_estimator: &LowerBoundedFeeEstimator, + ) -> Result<(ChannelPublicKeys, PublicKey), ChannelError> { if self.holder_commitment_point.current_point().is_none() { return Err(ChannelError::WarnAndDisconnect(format!( "Channel {} commitment point needs to be advanced once before RBF", @@ -13024,26 +13022,11 @@ where return Err(ChannelError::Abort(AbortReason::InsufficientRbfFeerate)); } - let their_funding_contribution = match msg.funding_output_contribution { - Some(value) => SignedAmount::from_sat(value), - None => SignedAmount::ZERO, - }; - - self.validate_splice_contributions(our_funding_contribution, their_funding_contribution) - .map_err(|e| ChannelError::WarnAndDisconnect(e))?; - // Reuse funding pubkeys from the last negotiated candidate since all RBF candidates // for the same splice share the same funding output script. - let holder_pubkeys = last_candidate.get_holder_pubkeys().clone(); - let counterparty_funding_pubkey = *last_candidate.counterparty_funding_pubkey(); - - Ok(FundingScope::for_splice( - &self.funding, - &self.context, - our_funding_contribution, - their_funding_contribution, - counterparty_funding_pubkey, - holder_pubkeys, + Ok(( + last_candidate.get_holder_pubkeys().clone(), + *last_candidate.counterparty_funding_pubkey(), )) } @@ -13051,6 +13034,10 @@ where &mut self, msg: &msgs::TxInitRbf, entropy_source: &ES, holder_node_id: &PublicKey, fee_estimator: &LowerBoundedFeeEstimator, logger: &L, ) -> Result { + let (holder_pubkeys, counterparty_funding_pubkey) = self + .validate_tx_init_rbf(msg, fee_estimator) + .map_err(|e| self.quiescent_negotiation_err(e))?; + let feerate = FeeRate::from_sat_per_kwu(msg.feerate_sat_per_1000_weight as u64); let (queued_net_value, holder_balance) = self .resolve_queued_contribution(feerate, logger) @@ -13079,14 +13066,23 @@ where }; let our_funding_contribution = queued_net_value.or(prior_net_value); + let our_funding_contribution = our_funding_contribution.unwrap_or(SignedAmount::ZERO); - let rbf_funding = self - .validate_tx_init_rbf( - msg, - our_funding_contribution.unwrap_or(SignedAmount::ZERO), - fee_estimator, - ) - .map_err(|e| self.quiescent_negotiation_err(e))?; + let their_funding_contribution = match msg.funding_output_contribution { + Some(value) => SignedAmount::from_sat(value), + None => SignedAmount::ZERO, + }; + self.validate_splice_contributions(our_funding_contribution, their_funding_contribution) + .map_err(|e| self.quiescent_negotiation_err(ChannelError::WarnAndDisconnect(e)))?; + + let rbf_funding = FundingScope::for_splice( + &self.funding, + &self.context, + our_funding_contribution, + their_funding_contribution, + counterparty_funding_pubkey, + holder_pubkeys, + ); // Consume the appropriate contribution source. let (our_funding_inputs, our_funding_outputs) = if queued_net_value.is_some() { @@ -13123,8 +13119,6 @@ where Default::default() }; - let our_funding_contribution = our_funding_contribution.unwrap_or(SignedAmount::ZERO); - log_info!( logger, "Starting RBF funding negotiation for channel {} after receiving tx_init_rbf; channel value: {} sats", @@ -14256,8 +14250,12 @@ where } fn quiescent_negotiation_err(&mut self, err: ChannelError) -> InteractiveTxMsgError { - let exited_quiescence = - if matches!(err, ChannelError::Abort(_)) { self.exit_quiescence() } else { false }; + let exited_quiescence = if matches!(err, ChannelError::Abort(_)) { + debug_assert!(self.context.channel_state.is_quiescent()); + self.exit_quiescence() + } else { + false + }; InteractiveTxMsgError { err, splice_funding_failed: None, exited_quiescence } } diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index c4069e4f0f8..dcc786b9224 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -6583,6 +6583,96 @@ fn test_splice_revalidation_at_quiescence() { expect_splice_failed_events(&nodes[0], &channel_id, contribution); } +#[test] +fn test_splice_init_before_quiescence_sends_warning() { + // A misbehaving peer sends splice_init before quiescence is established. The receiver + // should send a warning and disconnect. + 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_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); + + // Node 0 initiates quiescence. + nodes[0].node.maybe_propose_quiescence(&node_id_1, &channel_id).unwrap(); + let _stfu = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + + // Misbehaving node 1 sends splice_init before completing the STFU handshake. + let funding_pubkey = + PublicKey::from_secret_key(&Secp256k1::new(), &SecretKey::from_slice(&[42; 32]).unwrap()); + let splice_init = msgs::SpliceInit { + channel_id, + funding_contribution_satoshis: 50_000, + funding_feerate_per_kw: FEERATE_FLOOR_SATS_PER_KW, + locktime: 0, + funding_pubkey, + require_confirmed_inputs: None, + }; + nodes[0].node.handle_splice_init(node_id_1, &splice_init); + + // Node 0 should send a warning and disconnect. + let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1); + match &msg_events[0] { + MessageSendEvent::HandleError { node_id, .. } => assert_eq!(*node_id, node_id_1), + other => panic!("Expected HandleError, got {:?}", other), + } +} + +#[test] +fn test_tx_init_rbf_before_quiescence_sends_warning() { + // A misbehaving peer sends tx_init_rbf before quiescence is established. The receiver + // should send a warning and disconnect. + 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_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 added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + + // Complete a splice-in so there's a pending splice to RBF. + let funding_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value); + let (_splice_tx, _new_funding_script) = + splice_channel(&nodes[0], &nodes[1], channel_id, funding_contribution); + + // Node 0 initiates quiescence. + nodes[0].node.maybe_propose_quiescence(&node_id_1, &channel_id).unwrap(); + let _stfu = get_event_msg!(nodes[0], MessageSendEvent::SendStfu, node_id_1); + + // Misbehaving node 1 sends tx_init_rbf before completing the STFU handshake. + let tx_init_rbf = msgs::TxInitRbf { + channel_id, + locktime: 0, + feerate_sat_per_1000_weight: FEERATE_FLOOR_SATS_PER_KW + 25, + funding_output_contribution: Some(added_value.to_sat() as i64), + }; + nodes[0].node.handle_tx_init_rbf(node_id_1, &tx_init_rbf); + + // Node 0 should send a warning and disconnect. + let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1); + match &msg_events[0] { + MessageSendEvent::HandleError { node_id, .. } => assert_eq!(*node_id, node_id_1), + other => panic!("Expected HandleError, got {:?}", other), + } + + // Clean up events from the splice setup. + nodes[0].node.get_and_clear_pending_events(); + nodes[1].node.get_and_clear_pending_events(); +} + #[test] fn test_splice_rbf_rejects_low_feerate_after_several_attempts() { // After several RBF attempts, the counterparty's RBF feerate must be high enough to From c1594a067c5b31e875a90785feb91457ba71c86c Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 7 Apr 2026 19:30:33 -0500 Subject: [PATCH 6/7] Remove exited_quiescence from error handling The `exited_quiescence` field on `MsgHandleErrInternal` and `InteractiveTxMsgError` is a leaky abstraction -- the channelmanager error handling shouldn't know about quiescence, only whether the holding cell needs to be released. Infer this from the presence of a `tx_abort` instead, since exiting quiescence via an error always produces one. Remove `exited_quiescence` from `InteractiveTxMsgError`, `MsgHandleErrInternal`, and the return type of `Channel::tx_abort`, along with the `with_exited_quiescence` builder. For unfunded v2 channels, `tx_abort` may be present without quiescence having been exited, but the holding cell release is a no-op since an unfunded channel won't have any HTLCs. Similarly, the unreachable `debug_assert!(false)` branch in `fail_interactive_tx_negotiation` for funded channels produces a `tx_abort` without exiting quiescence, but the holding cell release is a no-op since the channel is still quiescent. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/channel.rs | 48 +++++++++---------------- lightning/src/ln/channelmanager.rs | 57 ++++++++++-------------------- 2 files changed, 35 insertions(+), 70 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 9ded88af61e..5c25250ed0e 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1172,9 +1172,6 @@ pub(super) struct InteractiveTxMsgError { /// If a splice was in progress when processing the message, this contains the splice funding /// information for emitting a `SpliceFailed` event. pub(super) splice_funding_failed: Option, - /// Whether we were quiescent when we received the message, and are no longer due to aborting - /// the session. - pub(super) exited_quiescence: bool, } /// The return value of `monitor_updating_restored` @@ -1818,30 +1815,24 @@ where let logger = WithChannelContext::from(logger, &self.context(), None); log_info!(logger, "Failed interactive transaction negotiation: {reason}"); - let (splice_funding_failed, exited_quiescence) = match &mut self.phase { + let splice_funding_failed = match &mut self.phase { ChannelPhase::Undefined => unreachable!(), - ChannelPhase::UnfundedOutboundV1(_) | ChannelPhase::UnfundedInboundV1(_) => { - (None, false) - }, + ChannelPhase::UnfundedOutboundV1(_) | ChannelPhase::UnfundedInboundV1(_) => None, ChannelPhase::UnfundedV2(pending_v2_channel) => { pending_v2_channel.interactive_tx_constructor.take(); - (None, false) + None }, ChannelPhase::Funded(funded_channel) => { if funded_channel.should_reset_pending_splice_state(false) { - (funded_channel.reset_pending_splice_state(), true) + funded_channel.reset_pending_splice_state() } else { debug_assert!(false, "We should never fail an interactive funding negotiation once we're exchanging tx_signatures"); - (None, false) + None } }, }; - InteractiveTxMsgError { - err: ChannelError::Abort(reason), - splice_funding_failed, - exited_quiescence, - } + InteractiveTxMsgError { err: ChannelError::Abort(reason), splice_funding_failed } } pub fn tx_add_input( @@ -1856,7 +1847,6 @@ where "Received unexpected interactive transaction negotiation message".to_owned(), ), splice_funding_failed: None, - exited_quiescence: false, }), } } @@ -1873,7 +1863,6 @@ where "Received unexpected interactive transaction negotiation message".to_owned(), ), splice_funding_failed: None, - exited_quiescence: false, }), } } @@ -1890,7 +1879,6 @@ where "Received unexpected interactive transaction negotiation message".to_owned(), ), splice_funding_failed: None, - exited_quiescence: false, }), } } @@ -1907,7 +1895,6 @@ where "Received unexpected interactive transaction negotiation message".to_owned(), ), splice_funding_failed: None, - exited_quiescence: false, }), } } @@ -1924,7 +1911,6 @@ where return Err(InteractiveTxMsgError { err: ChannelError::WarnAndDisconnect(err.to_owned()), splice_funding_failed: None, - exited_quiescence: false, }); }, }; @@ -1985,13 +1971,13 @@ where pub fn tx_abort( &mut self, msg: &msgs::TxAbort, logger: &L, - ) -> Result<(Option, Option, bool), ChannelError> { + ) -> Result<(Option, Option), ChannelError> { // If we have not sent a `tx_abort` message for this negotiation previously, we need to echo // back a tx_abort message according to the spec: // https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L560-L561 // For rationale why we echo back `tx_abort`: // https://github.com/lightning/bolts/blob/247e83d/02-peer-protocol.md?plain=1#L578-L580 - let (should_ack, splice_funding_failed, exited_quiescence) = match &mut self.phase { + let (should_ack, splice_funding_failed) = match &mut self.phase { ChannelPhase::Undefined => unreachable!(), ChannelPhase::UnfundedOutboundV1(_) | ChannelPhase::UnfundedInboundV1(_) => { let err = "Got an unexpected tx_abort message: This is an unfunded channel created with V1 channel establishment"; @@ -2000,7 +1986,7 @@ where ChannelPhase::UnfundedV2(pending_v2_channel) => { let had_constructor = pending_v2_channel.interactive_tx_constructor.take().is_some(); - (had_constructor, None, false) + (had_constructor, None) }, ChannelPhase::Funded(funded_channel) => { if funded_channel.has_pending_splice_awaiting_signatures() @@ -2028,11 +2014,11 @@ where .unwrap_or(false); debug_assert!(has_funding_negotiation); let splice_funding_failed = funded_channel.reset_pending_splice_state(); - (true, splice_funding_failed, true) + (true, splice_funding_failed) } else { // We were not tracking the pending funding negotiation state anymore, likely // due to a disconnection or already having sent our own `tx_abort`. - (false, None, false) + (false, None) } }, }; @@ -2048,7 +2034,7 @@ where } }); - Ok((tx_abort, splice_funding_failed, exited_quiescence)) + Ok((tx_abort, splice_funding_failed)) } #[rustfmt::skip] @@ -14250,13 +14236,11 @@ where } fn quiescent_negotiation_err(&mut self, err: ChannelError) -> InteractiveTxMsgError { - let exited_quiescence = if matches!(err, ChannelError::Abort(_)) { + if matches!(err, ChannelError::Abort(_)) { debug_assert!(self.context.channel_state.is_quiescent()); - self.exit_quiescence() - } else { - false - }; - InteractiveTxMsgError { err, splice_funding_failed: None, exited_quiescence } + self.exit_quiescence(); + } + InteractiveTxMsgError { err, splice_funding_failed: None } } pub fn remove_legacy_scids_before_block(&mut self, height: u32) -> alloc::vec::Drain<'_, u64> { diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index b67d8dd7e6f..f2bd2675136 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -1027,7 +1027,6 @@ struct MsgHandleErrInternal { closes_channel: bool, shutdown_finish: Option<(ShutdownResult, Option<(msgs::ChannelUpdate, NodeId, NodeId)>)>, tx_abort: Option, - exited_quiescence: bool, } impl MsgHandleErrInternal { @@ -1042,7 +1041,6 @@ impl MsgHandleErrInternal { closes_channel: false, shutdown_finish: None, tx_abort: None, - exited_quiescence: false, } } @@ -1062,13 +1060,7 @@ impl MsgHandleErrInternal { } fn from_no_close(err: msgs::LightningError) -> Self { - Self { - err, - closes_channel: false, - shutdown_finish: None, - tx_abort: None, - exited_quiescence: false, - } + Self { err, closes_channel: false, shutdown_finish: None, tx_abort: None } } fn from_finish_shutdown( @@ -1089,7 +1081,6 @@ impl MsgHandleErrInternal { closes_channel: true, shutdown_finish: Some((shutdown_res, channel_update)), tx_abort: None, - exited_quiescence: false, } } @@ -1125,13 +1116,7 @@ impl MsgHandleErrInternal { }, }, }; - Self { - err, - closes_channel: false, - shutdown_finish: None, - tx_abort, - exited_quiescence: false, - } + Self { err, closes_channel: false, shutdown_finish: None, tx_abort } } fn dont_send_error_message(&mut self) { @@ -1148,9 +1133,11 @@ impl MsgHandleErrInternal { self.closes_channel } - fn with_exited_quiescence(mut self, exited_quiescence: bool) -> Self { - self.exited_quiescence = exited_quiescence; - self + /// Whether the holding cell should be released after handling this error. This is inferred + /// from the presence of a `tx_abort`, which is sent when aborting an interactive transaction + /// negotiation that was conducted during quiescence. + fn needs_holding_cell_release(&self) -> bool { + self.tx_abort.is_some() } } @@ -4569,6 +4556,7 @@ impl< internal.map_err(|err_internal| { let mut msg_event = None; + let needs_holding_cell_release = err_internal.needs_holding_cell_release(); if let Some((shutdown_res, update_option)) = err_internal.shutdown_finish { let counterparty_node_id = shutdown_res.counterparty_node_id; @@ -4610,7 +4598,7 @@ impl< } let mut holding_cell_res = None; - if msg_event.is_some() || err_internal.exited_quiescence { + if msg_event.is_some() || needs_holding_cell_release { let per_peer_state = self.per_peer_state.read().unwrap(); if let Some(peer_state_mutex) = per_peer_state.get(&counterparty_node_id) { let mut peer_state = peer_state_mutex.lock().unwrap(); @@ -4621,8 +4609,7 @@ impl< } // We need to enqueue the `tx_abort` in `pending_msg_events` above before we // enqueue any commitment updates generated by freeing holding cell HTLCs. - holding_cell_res = err_internal - .exited_quiescence + holding_cell_res = needs_holding_cell_release .then(|| self.check_free_peer_holding_cells(&mut peer_state)); } } @@ -11850,10 +11837,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ None, )); } - debug_assert!(!err.exited_quiescence || matches!(err.err, ChannelError::Abort(_))); - MsgHandleErrInternal::from_chan_no_close(err.err, channel_id) - .with_exited_quiescence(err.exited_quiescence) } fn internal_tx_msg< @@ -12012,11 +11996,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ Ok(()) }, - Err(InteractiveTxMsgError { - err, - splice_funding_failed, - exited_quiescence, - }) => { + Err(InteractiveTxMsgError { err, splice_funding_failed }) => { if let Some(splice_funding_failed) = splice_funding_failed { let pending_events = &mut self.pending_events.lock().unwrap(); pending_events.push_back(( @@ -12040,10 +12020,8 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ None, )); } - debug_assert!(!exited_quiescence || matches!(err, ChannelError::Abort(_))); - Err(MsgHandleErrInternal::from_chan_no_close(err, msg.channel_id) - .with_exited_quiescence(exited_quiescence)) + Err(MsgHandleErrInternal::from_chan_no_close(err, msg.channel_id)) }, } }, @@ -12114,7 +12092,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } // We consider a splice negotiated when we exchange `tx_signatures`, // which also terminates quiescence. - let exited_quiescence = splice_negotiated.is_some(); + let needs_holding_cell_release = splice_negotiated.is_some(); if let Some(splice_negotiated) = splice_negotiated { self.pending_events.lock().unwrap().push_back(( events::Event::SplicePending { @@ -12129,7 +12107,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ None, )); } - let holding_cell_res = if exited_quiescence { + let holding_cell_res = if needs_holding_cell_release { self.check_free_peer_holding_cells(peer_state) } else { Vec::new() @@ -12171,7 +12149,7 @@ 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) => { let res = chan_entry.get_mut().tx_abort(msg, &self.logger); - let (tx_abort, splice_failed, exited_quiescence) = + let (tx_abort, splice_failed) = try_channel_entry!(self, peer_state, res, chan_entry); let persist = if tx_abort.is_some() || splice_failed.is_some() { @@ -12180,6 +12158,9 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ NotifyOption::SkipPersistNoEvents }; + // Release any HTLCs held during quiescence now that we're + // exiting via tx_abort. + let needs_holding_cell_release = tx_abort.is_some(); if let Some(tx_abort_msg) = tx_abort { peer_state.pending_msg_events.push(MessageSendEvent::SendTxAbort { node_id: *counterparty_node_id, @@ -12211,7 +12192,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ )); } - let holding_cell_res = if exited_quiescence { + let holding_cell_res = if needs_holding_cell_release { self.check_free_peer_holding_cells(peer_state) } else { Vec::new() From bdc17dc095003941881436df6ac68c6fb928214c Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 7 Apr 2026 17:57:07 -0500 Subject: [PATCH 7/7] Clear disconnect timer when exiting quiescence Several code paths exit quiescence by calling `clear_quiescent()` directly without also clearing the disconnect timer via `mark_response_received()`. This causes the timer to fire after the splice completes or is aborted, spuriously disconnecting the peer. Replace `clear_quiescent()` with `exit_quiescence()` in `on_tx_signatures_exchange`, `reset_pending_splice_state`, and `peer_connected_get_handshake`, which clears both the quiescent state and the disconnect timer. Co-Authored-By: Claude Opus 4.6 (1M context) --- lightning/src/ln/channel.rs | 10 +- lightning/src/ln/splicing_tests.rs | 194 ++++++++++++++++++++++++++++- 2 files changed, 200 insertions(+), 4 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 5c25250ed0e..c44690f6035 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1719,7 +1719,10 @@ where // We shouldn't be quiescent anymore upon reconnecting if: // - We were in quiescence but a splice/RBF was never negotiated or // - We were in quiescence but the splice negotiation failed due to disconnecting - chan.context.channel_state.clear_quiescent(); + // + // NOTE: While `exit_quiescence` clears the disconnect timer, it should already + // have been cleared by `remove_uncommitted_htlcs_and_mark_paused`. + chan.exit_quiescence(); None } else { None @@ -7285,7 +7288,7 @@ where self.pending_splice.take(); } - self.context.channel_state.clear_quiescent(); + self.exit_quiescence(); if current_is_awaiting_signatures { self.context.interactive_tx_signing_session.take(); } @@ -9305,7 +9308,6 @@ where debug_assert!(!self.context.channel_state.is_awaiting_remote_revoke()); if let Some(pending_splice) = self.pending_splice.as_mut() { - self.context.channel_state.clear_quiescent(); if let Some(FundingNegotiation::AwaitingSignatures { mut funding, funding_feerate_sat_per_1000_weight, @@ -9354,6 +9356,8 @@ where } else { debug_assert!(false); } + + self.exit_quiescence(); } else { self.funding.funding_transaction = Some(funding_tx.clone()); self.context.channel_state = diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index dcc786b9224..df8f9d0b690 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -16,7 +16,8 @@ use crate::chain::ChannelMonitorUpdateStatus; use crate::events::{ClosureReason, Event, FundingInfo, HTLCHandlingFailureType}; use crate::ln::chan_utils; use crate::ln::channel::{ - CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY, FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, + CHANNEL_ANNOUNCEMENT_PROPAGATION_DELAY, DISCONNECT_PEER_AWAITING_RESPONSE_TICKS, + FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE, }; use crate::ln::channelmanager::{provided_init_features, PaymentId, BREAKDOWN_TIMEOUT}; use crate::ln::functional_test_utils::*; @@ -6819,3 +6820,194 @@ fn test_splice_rbf_rejects_own_low_feerate_after_several_attempts() { other => panic!("Expected SpliceFailed, got {:?}", other), } } + +#[test] +fn test_no_disconnect_after_splice_completes() { + // Test that the disconnect timer is cleared when exiting quiescence after a successful splice + // negotiation. Previously, `on_tx_signatures_exchange` cleared the quiescent state but not the + // disconnect timer, causing a spurious disconnect after the splice completed. + 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 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 added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + + let funding_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value); + let new_funding_script = complete_splice_handshake(&nodes[0], &nodes[1]); + + // Fire a tick while quiescent to arm the disconnect timer. + nodes[0].node.timer_tick_occurred(); + nodes[1].node.timer_tick_occurred(); + + // Complete the splice negotiation, which should clear the timer when exiting quiescence. + complete_interactive_funding_negotiation( + &nodes[0], + &nodes[1], + channel_id, + funding_contribution, + new_funding_script, + ); + let (_, splice_locked) = sign_interactive_funding_tx(&nodes[0], &nodes[1], false); + assert!(splice_locked.is_none()); + + let node_id_0 = nodes[0].node.get_our_node_id(); + let node_id_1 = nodes[1].node.get_our_node_id(); + expect_splice_pending_event(&nodes[0], &node_id_1); + expect_splice_pending_event(&nodes[1], &node_id_0); + + // Fire enough ticks to trigger a disconnect if the timer wasn't properly cleared. + for _ in 0..DISCONNECT_PEER_AWAITING_RESPONSE_TICKS { + nodes[0].node.timer_tick_occurred(); + nodes[1].node.timer_tick_occurred(); + } + + let has_disconnect = |events: &[MessageSendEvent]| { + events.iter().any(|event| { + matches!( + event, + MessageSendEvent::HandleError { + action: msgs::ErrorAction::DisconnectPeerWithWarning { .. }, + .. + } + ) + }) + }; + assert!(!has_disconnect(&nodes[0].node.get_and_clear_pending_msg_events())); + assert!(!has_disconnect(&nodes[1].node.get_and_clear_pending_msg_events())); +} + +#[test] +fn test_no_disconnect_after_splice_aborted() { + // Test that the disconnect timer is cleared when exiting quiescence after a splice negotiation + // is aborted via tx_abort. Previously, `reset_pending_splice_state` cleared the quiescent + // state but not the disconnect timer, causing a spurious disconnect after the abort. + 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 added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + + let funding_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value); + complete_splice_handshake(&nodes[0], &nodes[1]); + + // Fire a tick while quiescent to arm the disconnect timer. + nodes[0].node.timer_tick_occurred(); + nodes[1].node.timer_tick_occurred(); + + // Abort the splice, which should clear the timer when exiting quiescence. + nodes[0].node.abandon_splice(&channel_id, &node_id_1).unwrap(); + + expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution); + + let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); + let tx_abort = msg_events + .iter() + .find_map(|event| { + if let MessageSendEvent::SendTxAbort { msg, .. } = event { + Some(msg.clone()) + } else { + None + } + }) + .expect("Expected SendTxAbort"); + + nodes[1].node.handle_tx_abort(node_id_0, &tx_abort); + let tx_abort_echo = get_event_msg!(nodes[1], MessageSendEvent::SendTxAbort, node_id_0); + nodes[1].node.get_and_clear_pending_events(); + + nodes[0].node.handle_tx_abort(node_id_1, &tx_abort_echo); + + // Fire enough ticks to trigger a disconnect if the timer wasn't properly cleared. + for _ in 0..DISCONNECT_PEER_AWAITING_RESPONSE_TICKS { + nodes[0].node.timer_tick_occurred(); + nodes[1].node.timer_tick_occurred(); + } + + let has_disconnect = |events: &[MessageSendEvent]| { + events.iter().any(|event| { + matches!( + event, + MessageSendEvent::HandleError { + action: msgs::ErrorAction::DisconnectPeerWithWarning { .. }, + .. + } + ) + }) + }; + assert!(!has_disconnect(&nodes[0].node.get_and_clear_pending_msg_events())); + assert!(!has_disconnect(&nodes[1].node.get_and_clear_pending_msg_events())); +} + +#[test] +fn test_no_disconnect_after_quiescence_on_reconnect() { + // Test that there is no spurious disconnect after reconnecting from a quiescent state. The + // disconnect timer is cleared by `remove_uncommitted_htlcs_and_mark_paused` during + // disconnection and by `exit_quiescence` during reconnection. + 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 added_value = Amount::from_sat(50_000); + provide_utxo_reserves(&nodes, 2, added_value * 2); + + let funding_contribution = do_initiate_splice_in(&nodes[0], &nodes[1], channel_id, added_value); + complete_splice_handshake(&nodes[0], &nodes[1]); + + // Fire a tick while quiescent to arm the disconnect timer. + nodes[0].node.timer_tick_occurred(); + nodes[1].node.timer_tick_occurred(); + + // Disconnect and reconnect. + nodes[0].node.peer_disconnected(node_id_1); + nodes[1].node.peer_disconnected(node_id_0); + + expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution); + + let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + reconnect_args.send_channel_ready = (true, true); + reconnect_args.send_announcement_sigs = (true, true); + reconnect_nodes(reconnect_args); + + // Fire enough ticks to trigger a disconnect if the timer wasn't properly cleared. + for _ in 0..DISCONNECT_PEER_AWAITING_RESPONSE_TICKS { + nodes[0].node.timer_tick_occurred(); + nodes[1].node.timer_tick_occurred(); + } + + let has_disconnect = |events: &[MessageSendEvent]| { + events.iter().any(|event| { + matches!( + event, + MessageSendEvent::HandleError { + action: msgs::ErrorAction::DisconnectPeerWithWarning { .. }, + .. + } + ) + }) + }; + assert!(!has_disconnect(&nodes[0].node.get_and_clear_pending_msg_events())); + assert!(!has_disconnect(&nodes[1].node.get_and_clear_pending_msg_events())); +}