From 12998cee963810098921c9ad8a6a47eedacf98bf Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 17 Mar 2026 15:25:55 -0500 Subject: [PATCH 1/4] 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 67e30768e8f3b2b18a5ca5306fdf14710f4094c7 Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 17 Mar 2026 15:43:46 -0500 Subject: [PATCH 2/4] 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 b61caa4ea63..bbbe31f68a5 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 f5bfedd32a9a34fa73950150954fb74e30e6fbfd Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Tue, 17 Mar 2026 16:31:10 -0500 Subject: [PATCH 3/4] 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 3c55d3c791e6deda2f5545193fa6806b59bb996a Mon Sep 17 00:00:00 2001 From: Jeffrey Czyz Date: Thu, 2 Apr 2026 17:41:17 -0500 Subject: [PATCH 4/4] 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 bbbe31f68a5..6d69196918a 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -6552,6 +6552,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