From c30d9113d5a419df6d80bcd9bb9cae9cb4199ede Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Fri, 13 Mar 2026 10:50:26 -0700 Subject: [PATCH 1/2] Allow cancellation of pending splice funding negotiations A user may wish to cancel an in-flight funding negotiation for whatever reason (e.g., mempool feerates have gone down, inability to sign, etc.), so we should make it possible for them to do so. Note that this can only be done for splice funding negotiations for which the user has made a contribution to. --- lightning/src/events/mod.rs | 8 +- lightning/src/ln/channel.rs | 96 +++++++-- lightning/src/ln/channelmanager.rs | 159 +++++++-------- lightning/src/ln/interactivetxs.rs | 3 + lightning/src/ln/splicing_tests.rs | 317 ++++++++++++++++++++++++++++- 5 files changed, 473 insertions(+), 110 deletions(-) diff --git a/lightning/src/events/mod.rs b/lightning/src/events/mod.rs index 73c4a39c76f..0f873fab07b 100644 --- a/lightning/src/events/mod.rs +++ b/lightning/src/events/mod.rs @@ -1833,7 +1833,7 @@ pub enum Event { invoice_request: InvoiceRequest, }, /// Indicates that a channel funding transaction constructed interactively is ready to be - /// signed. This event will only be triggered if at least one input was contributed. + /// signed. This event will only be triggered if a contribution was made to the transaction. /// /// The transaction contains all inputs and outputs provided by both parties including the /// channel's funding output and a change output if applicable. @@ -1844,8 +1844,9 @@ pub enum Event { /// Each signature MUST use the `SIGHASH_ALL` flag to avoid invalidation of the initial commitment and /// hence possible loss of funds. /// - /// After signing, call [`ChannelManager::funding_transaction_signed`] with the (partially) signed - /// funding transaction. + /// After signing, call [`ChannelManager::funding_transaction_signed`] with the (partially) + /// signed funding transaction. For splices where you contributed inputs or outputs, call + /// [`ChannelManager::cancel_funding_contributed`] instead if you no longer wish to proceed. /// /// Generated in [`ChannelManager`] message handling. /// @@ -1854,6 +1855,7 @@ pub enum Event { /// returning `Err(ReplayEvent ())`), but will only be regenerated as needed after restarts. /// /// [`ChannelManager`]: crate::ln::channelmanager::ChannelManager + /// [`ChannelManager::cancel_funding_contributed`]: crate::ln::channelmanager::ChannelManager::cancel_funding_contributed /// [`ChannelManager::funding_transaction_signed`]: crate::ln::channelmanager::ChannelManager::funding_transaction_signed FundingTransactionReadyForSigning { /// The `channel_id` of the channel which you'll need to pass back into diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e6397aefbcb..7d8bed5b0f9 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -12713,30 +12713,88 @@ where } } - #[cfg(test)] - pub fn abandon_splice( - &mut self, - ) -> Result<(msgs::TxAbort, Option), APIError> { - if self.should_reset_pending_splice_state(false) { - let tx_abort = - msgs::TxAbort { channel_id: self.context.channel_id(), data: Vec::new() }; - let splice_funding_failed = self.reset_pending_splice_state(); - Ok((tx_abort, splice_funding_failed)) - } else if self.has_pending_splice_awaiting_signatures() { - Err(APIError::APIMisuseError { + pub fn cancel_funding_contributed(&mut self) -> Result { + if matches!(self.quiescent_action, Some(QuiescentAction::Splice { .. })) { + let splice_funding_failed = self.abandon_quiescent_action(); + let err = if self.context.channel_state.is_local_stfu_sent() + || self.context.channel_state.is_remote_stfu_sent() + || self.context.channel_state.is_quiescent() + { + ChannelError::WarnAndDisconnect("Manually canceled funding contribution".into()) + } else { + ChannelError::Abort(AbortReason::ManualIntervention) + }; + return Ok(InteractiveTxMsgError { err, splice_funding_failed }); + } + + let funding_negotiation = self + .pending_splice + .as_ref() + .and_then(|pending_splice| pending_splice.funding_negotiation.as_ref()); + let Some(funding_negotiation) = funding_negotiation else { + return Err(APIError::APIMisuseError { err: format!( - "Channel {} splice cannot be abandoned; already awaiting signatures", - self.context.channel_id(), + "Channel {} does not have a pending splice negotiation", + self.context.channel_id() ), - }) - } else { - Err(APIError::APIMisuseError { + }); + }; + + let made_contribution = match funding_negotiation { + FundingNegotiation::AwaitingAck { context, .. } => { + context.contributed_inputs().next().is_some() + || context.contributed_outputs().next().is_some() + }, + FundingNegotiation::ConstructingTransaction { interactive_tx_constructor, .. } => { + interactive_tx_constructor.contributed_inputs().next().is_some() + || interactive_tx_constructor.contributed_outputs().next().is_some() + }, + FundingNegotiation::AwaitingSignatures { .. } => self + .context + .interactive_tx_signing_session + .as_ref() + .expect("We have a pending splice awaiting signatures") + .has_local_contribution(), + }; + if !made_contribution { + return Err(APIError::APIMisuseError { err: format!( - "Channel {} splice cannot be abandoned; no pending splice", - self.context.channel_id(), + "Channel {} has a pending splice negotiation with no contribution made", + self.context.channel_id() ), - }) + }); } + + // We typically don't reset the pending funding negotiation when we're in + // [`FundingNegotiation::AwaitingSignatures`] since we're able to resume it on + // re-establishment, so we still need to handle this case separately if the user wishes to + // cancel. If they've yet to call [`Channel::funding_transaction_signed`], then we can + // guarantee to never have sent any signatures to the counterparty, or have processed any + // signatures from them. + if matches!(funding_negotiation, FundingNegotiation::AwaitingSignatures { .. }) { + let already_signed = self + .context + .interactive_tx_signing_session + .as_ref() + .expect("We have a pending splice awaiting signatures") + .has_holder_tx_signatures(); + if already_signed { + return Err(APIError::APIMisuseError { + err: format!( + "Channel {} has pending splice negotiation that was already signed", + self.context.channel_id(), + ), + }); + } + } + + debug_assert!(self.context.channel_state.is_quiescent()); + let splice_funding_failed = self.reset_pending_splice_state(); + debug_assert!(splice_funding_failed.is_some()); + Ok(InteractiveTxMsgError { + err: ChannelError::Abort(AbortReason::ManualIntervention), + splice_funding_failed, + }) } /// Checks during handling splice_init diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 1f32423507f..709a3b3cd69 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -4920,96 +4920,85 @@ impl< } } - #[cfg(test)] - pub(crate) fn abandon_splice( - &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, - ) -> Result<(), APIError> { - let mut res = Ok(()); - PersistenceNotifierGuard::optionally_notify(self, || { - let result = self.internal_abandon_splice(channel_id, counterparty_node_id); - res = result; - match res { - Ok(_) => NotifyOption::SkipPersistHandleEvents, - Err(_) => NotifyOption::SkipPersistNoEvents, - } - }); - res - } - - #[cfg(test)] - fn internal_abandon_splice( + /// Cancels an in-flight [`FundingContribution`]. + /// + /// This is primarily useful after receiving an [`Event::FundingTransactionReadyForSigning`] for + /// a [`FundingContribution`] you no longer wish to proceed with. This may be called for any + /// pending [`FundingContribution`] after its corresponding + /// [`ChannelManager::funding_contributed`] call up until + /// [`ChannelManager::funding_transaction_signed`]. + /// + /// Returns [`ChannelUnavailable`] when a channel is not found or an incorrect + /// `counterparty_node_id` is provided, or [`APIMisuseError`] otherwise with the error details. + /// + /// [`Event::FundingTransactionReadyForSigning`]: events::Event::FundingTransactionReadyForSigning + /// [`ChannelUnavailable`]: APIError::ChannelUnavailable + /// [`APIMisuseError`]: APIError::APIMisuseError + pub fn cancel_funding_contributed( &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, ) -> Result<(), APIError> { - let per_peer_state = self.per_peer_state.read().unwrap(); - - let peer_state_mutex = match per_peer_state - .get(counterparty_node_id) - .ok_or_else(|| APIError::no_such_peer(counterparty_node_id)) - { - Ok(p) => p, - Err(e) => return Err(e), - }; - - let mut peer_state_lock = peer_state_mutex.lock().unwrap(); - let peer_state = &mut *peer_state_lock; - - // Look for the channel - match peer_state.channel_by_id.entry(*channel_id) { - hash_map::Entry::Occupied(mut chan_phase_entry) => { - if !chan_phase_entry.get().context().is_connected() { - // TODO: We should probably support this, but right now `splice_channel` refuses when - // the peer is disconnected, so we just check it here. - return Err(APIError::ChannelUnavailable { - err: "Cannot abandon splice while peer is disconnected".to_owned(), - }); - } - - if let Some(chan) = chan_phase_entry.get_mut().as_funded_mut() { - let (tx_abort, splice_funding_failed) = chan.abandon_splice()?; - - peer_state.pending_msg_events.push(MessageSendEvent::SendTxAbort { - node_id: *counterparty_node_id, - msg: tx_abort, - }); + let mut result = Ok(()); + PersistenceNotifierGuard::manually_notify(self, || { + let per_peer_state = self.per_peer_state.read().unwrap(); + let peer_state_mutex = match per_peer_state + .get(counterparty_node_id) + .ok_or_else(|| APIError::no_such_peer(counterparty_node_id)) + { + Ok(p) => p, + Err(e) => { + result = Err(e); + return; + }, + }; + let mut peer_state_lock = peer_state_mutex.lock().unwrap(); + let peer_state = &mut *peer_state_lock; - 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: *channel_id, - counterparty_node_id: *counterparty_node_id, - user_channel_id: chan.context.get_user_id(), - abandoned_funding_txo: splice_funding_failed.funding_txo, - channel_type: splice_funding_failed.channel_type, - }, - None, - )); - pending_events.push_back(( - events::Event::DiscardFunding { - channel_id: *channel_id, - funding_info: FundingInfo::Contribution { - inputs: splice_funding_failed.contributed_inputs, - outputs: splice_funding_failed.contributed_outputs, - }, + match peer_state.channel_by_id.entry(*channel_id) { + hash_map::Entry::Occupied(mut chan_entry) => { + if let Some(channel) = chan_entry.get_mut().as_funded_mut() { + let err = match channel.cancel_funding_contributed() { + Ok(v) => v, + Err(e) => { + result = Err(e); + return; }, - None, - )); - } + }; + let user_channel_id = channel.context().get_user_id(); + peer_state.pending_msg_events.retain(|event| { + !matches!(event, + MessageSendEvent::SendStfu { node_id, msg } + if *node_id == *counterparty_node_id && msg.channel_id == *channel_id + ) + }); + mem::drop(peer_state_lock); + mem::drop(per_peer_state); - Ok(()) - } else { - Err(APIError::ChannelUnavailable { - err: format!( - "Channel with id {} is not funded, cannot abandon splice", - channel_id - ), - }) - } - }, - hash_map::Entry::Vacant(_) => { - Err(APIError::no_such_channel_for_peer(channel_id, counterparty_node_id)) - }, - } + let err = self.handle_interactive_tx_msg_err( + err, + *channel_id, + counterparty_node_id, + user_channel_id, + ); + let _ = self.handle_error(Err::<(), _>(err), *counterparty_node_id); + self.event_persist_notifier.notify(); + } else { + result = Err(APIError::ChannelUnavailable { + err: format!( + "Channel with id {} is not funded, cannot cancel splice", + channel_id + ), + }); + return; + } + }, + hash_map::Entry::Vacant(_) => { + result = + Err(APIError::no_such_channel_for_peer(channel_id, counterparty_node_id)); + return; + }, + } + }); + result } fn forward_needs_intercept_to_known_chan( diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index ca8c4450012..71a22d84540 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -143,6 +143,8 @@ pub(crate) enum AbortReason { NegotiationInProgress, /// The initiator's feerate exceeds our maximum. FeeRateTooHigh, + /// The user manually intervened to abort the funding negotiation. + ManualIntervention, /// Internal error InternalError(&'static str), } @@ -209,6 +211,7 @@ impl Display for AbortReason { AbortReason::FeeRateTooHigh => { f.write_str("The initiator's feerate exceeds our maximum") }, + AbortReason::ManualIntervention => f.write_str("Manually aborted funding negotiation"), AbortReason::InternalError(text) => { f.write_fmt(format_args!("Internal error: {}", text)) }, diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index b2cb1eda375..b6c860ae4e9 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -2740,8 +2740,8 @@ fn fail_splice_on_tx_abort() { let _tx_complete = get_event_msg!(acceptor, MessageSendEvent::SendTxComplete, node_id_initiator); - acceptor.node.abandon_splice(&channel_id, &node_id_initiator).unwrap(); - let tx_abort = get_event_msg!(acceptor, MessageSendEvent::SendTxAbort, node_id_initiator); + // Inject a fake `tx_abort` to the initiator to trigger the splice to be aborted. + let tx_abort = msgs::TxAbort { channel_id, data: Vec::new() }; initiator.node.handle_tx_abort(node_id_acceptor, &tx_abort); expect_splice_failed_events(initiator, &channel_id, funding_contribution); @@ -2753,6 +2753,9 @@ fn fail_splice_on_tx_abort() { check_added_monitors(initiator, 1); if let MessageSendEvent::SendTxAbort { msg, .. } = &msg_events[0] { acceptor.node.handle_tx_abort(node_id_initiator, msg); + // The acceptor still tries to ack the abort by sending its own back to the initiator since + // a fake one was originally sent to it. + let _ = get_event_msg!(acceptor, MessageSendEvent::SendTxAbort, node_id_initiator); } else { panic!("Unexpected event {:?}", msg_events[0]); }; @@ -2764,6 +2767,314 @@ fn fail_splice_on_tx_abort() { }; } +#[test] +fn acceptor_with_local_contribution_can_cancel_funding_contributed_before_funding_transaction_signed( +) { + 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 initiator = &nodes[0]; + let acceptor = &nodes[1]; + + let node_id_initiator = initiator.node.get_our_node_id(); + let node_id_acceptor = acceptor.node.get_our_node_id(); + + let initial_channel_capacity = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_capacity, 0); + + provide_utxo_reserves(&nodes, 2, Amount::ONE_BTC); + + let outputs = vec![TxOut { + value: Amount::from_sat(1_000), + script_pubkey: initiator.wallet_source.get_change_script().unwrap(), + }]; + let initiator_contribution = + initiate_splice_out(initiator, acceptor, channel_id, outputs).unwrap(); + let acceptor_contribution = initiate_splice_in( + acceptor, + initiator, + channel_id, + Amount::from_sat(initial_channel_capacity / 2), + ); + + let stfu_initiator = get_event_msg!(initiator, MessageSendEvent::SendStfu, node_id_acceptor); + let stfu_acceptor = get_event_msg!(acceptor, MessageSendEvent::SendStfu, node_id_initiator); + + acceptor.node.handle_stfu(node_id_initiator, &stfu_initiator); + assert!(acceptor.node.get_and_clear_pending_msg_events().is_empty()); + + initiator.node.handle_stfu(node_id_acceptor, &stfu_acceptor); + + let splice_init = get_event_msg!(initiator, MessageSendEvent::SendSpliceInit, node_id_acceptor); + acceptor.node.handle_splice_init(node_id_initiator, &splice_init); + let splice_ack = get_event_msg!(acceptor, MessageSendEvent::SendSpliceAck, node_id_initiator); + assert_ne!(splice_ack.funding_contribution_satoshis, 0); + initiator.node.handle_splice_ack(node_id_acceptor, &splice_ack); + + let new_funding_script = chan_utils::make_funding_redeemscript( + &splice_init.funding_pubkey, + &splice_ack.funding_pubkey, + ) + .to_p2wsh(); + complete_interactive_funding_negotiation_for_both( + initiator, + acceptor, + channel_id, + initiator_contribution.clone(), + Some(acceptor_contribution.clone()), + splice_ack.funding_contribution_satoshis, + new_funding_script, + ); + + let event = get_event!(initiator, Event::FundingTransactionReadyForSigning); + if let Event::FundingTransactionReadyForSigning { + channel_id, + counterparty_node_id, + unsigned_transaction, + .. + } = event + { + let partially_signed_tx = initiator.wallet_source.sign_tx(unsigned_transaction).unwrap(); + initiator + .node + .funding_transaction_signed(&channel_id, &counterparty_node_id, partially_signed_tx) + .unwrap(); + } else { + unreachable!(); + } + + let msg_events = initiator.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); + let initial_commit_sig = if let MessageSendEvent::UpdateHTLCs { updates, .. } = &msg_events[0] { + updates.commitment_signed[0].clone() + } else { + panic!("Unexpected event {:?}", msg_events[0]); + }; + acceptor.node.handle_commitment_signed(node_id_initiator, &initial_commit_sig); + assert!(acceptor.node.get_and_clear_pending_msg_events().is_empty()); + + let _signing_event = get_event!(acceptor, Event::FundingTransactionReadyForSigning); + + acceptor.node.cancel_funding_contributed(&channel_id, &node_id_initiator).unwrap(); + let events = acceptor.node.get_and_clear_pending_events(); + assert_eq!(events.len(), 2); + assert!(matches!(events[0], Event::SpliceFailed { .. })); + assert!(matches!(events[1], Event::DiscardFunding { .. })); + let tx_abort = get_event_msg!(acceptor, MessageSendEvent::SendTxAbort, node_id_initiator); + + initiator.node.handle_tx_abort(node_id_acceptor, &tx_abort); + expect_splice_failed_events(initiator, &channel_id, initiator_contribution); + let tx_abort = get_event_msg!(initiator, MessageSendEvent::SendTxAbort, node_id_acceptor); + acceptor.node.handle_tx_abort(node_id_initiator, &tx_abort); +} + +#[test] +fn cancel_funding_contributed_before_funding_transaction_signed() { + do_cancel_funding_contributed_before_funding_transaction_signed(0); // AwaitingQuiescence + do_cancel_funding_contributed_before_funding_transaction_signed(1); // AwaitingAck + do_cancel_funding_contributed_before_funding_transaction_signed(2); // ConstructingTransaction + do_cancel_funding_contributed_before_funding_transaction_signed(3); // AwaitingSignatures +} + +#[cfg(test)] +fn do_cancel_funding_contributed_before_funding_transaction_signed(state: u8) { + 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 initiator = &nodes[0]; + let acceptor = &nodes[1]; + + let node_id_initiator = initiator.node.get_our_node_id(); + let node_id_acceptor = acceptor.node.get_our_node_id(); + + let initial_channel_capacity = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_capacity, 0); + + let outputs = vec![TxOut { + value: Amount::from_sat(1_000), + script_pubkey: initiator.wallet_source.get_change_script().unwrap(), + }]; + let funding_contribution = + initiate_splice_out(initiator, acceptor, channel_id, outputs).unwrap(); + + match state { + 0 => { + // Cancel after funding_contributed queues `stfu`, but before the quiescence attempt is + // delivered to the peer. + }, + 1 => { + // Deliver splice_init, but keep splice_ack queued so the initiator remains in + // FundingNegotiation::AwaitingAck while the acceptor tracks the pending splice. + let stfu_init = get_event_msg!(initiator, MessageSendEvent::SendStfu, node_id_acceptor); + acceptor.node.handle_stfu(node_id_initiator, &stfu_init); + let stfu_ack = get_event_msg!(acceptor, MessageSendEvent::SendStfu, node_id_initiator); + initiator.node.handle_stfu(node_id_acceptor, &stfu_ack); + + let splice_init = + get_event_msg!(initiator, MessageSendEvent::SendSpliceInit, node_id_acceptor); + acceptor.node.handle_splice_init(node_id_initiator, &splice_init); + assert!(initiator.node.get_and_clear_pending_msg_events().is_empty()); + + let msg_events = acceptor.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); + assert!(matches!(msg_events[0], MessageSendEvent::SendSpliceAck { .. })); + }, + 2 => { + // Complete the splice handshake so the initiator is constructing the interactive tx. + let _new_funding_script = complete_splice_handshake(initiator, acceptor); + + let msg_events = initiator.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); + assert!(matches!(msg_events[0], MessageSendEvent::SendTxAddInput { .. })); + assert!(acceptor.node.get_and_clear_pending_msg_events().is_empty()); + }, + 3 => { + // Complete interactive tx negotiation so the initiator is awaiting funding signatures. + let new_funding_script = complete_splice_handshake(initiator, acceptor); + complete_interactive_funding_negotiation( + initiator, + acceptor, + channel_id, + funding_contribution.clone(), + new_funding_script, + ); + + // The initiator should have a signing event to handle, while the acceptor immediately + // sends their initial commitment_signed. Deliver it before canceling to ensure it gets + // discarded with the splice. + let _signing_event = get_event!(initiator, Event::FundingTransactionReadyForSigning); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); + let acceptor_commit_sig = get_htlc_update_msgs(acceptor, &node_id_initiator); + initiator.node.handle_commitment_signed( + node_id_acceptor, + &acceptor_commit_sig.commitment_signed[0], + ); + check_added_monitors(initiator, 0); + assert!(initiator.node.get_and_clear_pending_msg_events().is_empty()); + }, + _ => panic!("unexpected state {state}"), + } + assert!(initiator.node.get_and_clear_pending_events().is_empty()); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); + + // Queue an outgoing HTLC to the holding cell. It should be freed once we cancel the splice and + // exit quiescence. + if state != 0 { + let (route, payment_hash, _payment_preimage, payment_secret) = + get_route_and_payment_hash!(initiator, acceptor, 1_000_000); + let onion = RecipientOnionFields::secret_only(payment_secret, 1_000_000); + let payment_id = PaymentId(payment_hash.0); + initiator.node.send_payment_with_route(route, payment_hash, onion, payment_id).unwrap(); + assert!(initiator.node.get_and_clear_pending_msg_events().is_empty()); + } + + initiator.node.cancel_funding_contributed(&channel_id, &node_id_acceptor).unwrap(); + expect_splice_failed_events(initiator, &channel_id, funding_contribution); + + // We exit or terminate the quiescence attempt upon canceling the splice, so we should see a + // tx_abort followed by the holding cell HTLC being released immediately. + let msg_events = initiator.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), if state == 0 { 1 } else { 2 }, "{msg_events:?}"); + if state == 0 { + if let MessageSendEvent::HandleError { action, .. } = &msg_events[0] { + assert!(matches!(action, msgs::ErrorAction::DisconnectPeerWithWarning { .. })); + } else { + panic!("Unexpected event {:?}", msg_events[0]); + } + return; + } + + let tx_abort = if let MessageSendEvent::SendTxAbort { msg, .. } = &msg_events[0] { + msg + } else { + panic!("Unexpected event {:?}", msg_events[0]); + }; + let update = if let MessageSendEvent::UpdateHTLCs { updates, .. } = &msg_events[1] { + updates + } else { + panic!("Unexpected event {:?}", msg_events[1]); + }; + check_added_monitors(initiator, 1); + + acceptor.node.handle_tx_abort(node_id_initiator, tx_abort); + let tx_abort = get_event_msg!(acceptor, MessageSendEvent::SendTxAbort, node_id_initiator); + initiator.node.handle_tx_abort(node_id_acceptor, &tx_abort); + + acceptor.node.handle_update_add_htlc(node_id_initiator, &update.update_add_htlcs[0]); + do_commitment_signed_dance(acceptor, initiator, &update.commitment_signed, false, false); +} + +#[test] +fn cannot_cancel_funding_contributed_after_funding_transaction_signed() { + 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 initiator = &nodes[0]; + let acceptor = &nodes[1]; + + let node_id_initiator = initiator.node.get_our_node_id(); + let node_id_acceptor = acceptor.node.get_our_node_id(); + + let initial_channel_capacity = 100_000; + let (_, _, channel_id, _) = + create_announced_chan_between_nodes_with_value(&nodes, 0, 1, initial_channel_capacity, 0); + + let outputs = vec![TxOut { + value: Amount::from_sat(1_000), + script_pubkey: initiator.wallet_source.get_change_script().unwrap(), + }]; + let funding_contribution = + initiate_splice_out(initiator, acceptor, channel_id, outputs).unwrap(); + let new_funding_script = complete_splice_handshake(initiator, acceptor); + complete_interactive_funding_negotiation( + initiator, + acceptor, + channel_id, + funding_contribution, + new_funding_script, + ); + assert!(acceptor.node.get_and_clear_pending_events().is_empty()); + let _acceptor_commit_sig = get_htlc_update_msgs(acceptor, &node_id_initiator); + + let event = get_event!(initiator, Event::FundingTransactionReadyForSigning); + if let Event::FundingTransactionReadyForSigning { + channel_id, + counterparty_node_id, + unsigned_transaction, + .. + } = event + { + let partially_signed_tx = initiator.wallet_source.sign_tx(unsigned_transaction).unwrap(); + initiator + .node + .funding_transaction_signed(&channel_id, &counterparty_node_id, partially_signed_tx) + .unwrap(); + } else { + unreachable!(); + } + + let res = initiator.node.cancel_funding_contributed(&channel_id, &node_id_acceptor); + match res { + Err(APIError::APIMisuseError { err }) => assert!(err.contains("already signed")), + _ => panic!("Unexpected result {res:?}"), + } + + assert!(initiator.node.get_and_clear_pending_events().is_empty()); + let msg_events = initiator.node.get_and_clear_pending_msg_events(); + assert!( + msg_events.iter().all(|event| !matches!(event, MessageSendEvent::SendTxAbort { .. })), + "{msg_events:?}" + ); +} + #[test] fn fail_splice_on_tx_complete_error() { let chanmon_cfgs = create_chanmon_cfgs(2); @@ -7376,7 +7687,7 @@ fn test_no_disconnect_after_splice_aborted() { 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(); + nodes[0].node.cancel_funding_contributed(&channel_id, &node_id_1).unwrap(); expect_splice_failed_events(&nodes[0], &channel_id, funding_contribution); From aac57cc6dfa13f2640791661f7a95b6c1b776917 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 19 Mar 2026 11:57:20 -0700 Subject: [PATCH 2/2] Rename should_reset_pending_splice_state argument There's a case in `should_reset_pending_splice_state` where we are awaiting signatures, but still want to preserve the pending negotiation upon a disconnection. We previously used `counterparty_aborted` as a way to toggle this behavior. Now that we support the user manually canceling an ongoing negotiation, we interpret the argument a bit more generically in terms of whether we wish to resume the negotiation or not when we are found in such a state. --- lightning/src/ln/channel.rs | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 7d8bed5b0f9..f09f8fcee69 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1709,7 +1709,7 @@ where if matches!(chan.context.channel_state, ChannelState::ChannelReady(_)) { chan.context.channel_state.clear_local_stfu_sent(); chan.context.channel_state.clear_remote_stfu_sent(); - if chan.should_reset_pending_splice_state(false) { + if chan.should_reset_pending_splice_state(true) { // If there was a pending splice negotiation that failed due to disconnecting, we // also take the opportunity to clean up our state. let splice_funding_failed = chan.reset_pending_splice_state(); @@ -1826,7 +1826,7 @@ where None }, ChannelPhase::Funded(funded_channel) => { - if funded_channel.should_reset_pending_splice_state(false) { + if funded_channel.should_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"); @@ -2009,7 +2009,7 @@ where "Received tx_abort while awaiting tx_signatures exchange".to_owned(), )); } - if funded_channel.should_reset_pending_splice_state(true) { + if funded_channel.should_reset_pending_splice_state(false) { let has_funding_negotiation = funded_channel .pending_splice .as_ref() @@ -7144,7 +7144,7 @@ where fn maybe_fail_splice_negotiation(&mut self) -> Option { if matches!(self.context.channel_state, ChannelState::ChannelReady(_)) { - if self.should_reset_pending_splice_state(false) { + if self.should_reset_pending_splice_state(true) { self.reset_pending_splice_state() } else { self.abandon_quiescent_action() @@ -7201,7 +7201,7 @@ where /// Returns a boolean indicating whether we should reset the splice's /// [`PendingFunding::funding_negotiation`]. - fn should_reset_pending_splice_state(&self, counterparty_aborted: bool) -> bool { + fn should_reset_pending_splice_state(&self, allow_resumption: bool) -> bool { self.pending_splice .as_ref() .map(|pending_splice| { @@ -7213,7 +7213,11 @@ where funding_negotiation, FundingNegotiation::AwaitingSignatures { .. } ); - if counterparty_aborted { + if allow_resumption { + // If we want to resume the negotiation after reconnecting, we must be + // in [`FundingNegotiation::AwaitingSignatures`] to not reset our state. + !is_awaiting_signatures + } else { !is_awaiting_signatures || !self .context() @@ -7221,8 +7225,6 @@ where .as_ref() .expect("We have a pending splice awaiting signatures") .has_received_commitment_signed() - } else { - !is_awaiting_signatures } }) .unwrap_or_else(|| { @@ -7236,7 +7238,7 @@ where } fn reset_pending_splice_state(&mut self) -> Option { - debug_assert!(self.should_reset_pending_splice_state(true)); + debug_assert!(self.should_reset_pending_splice_state(false)); // Only clear the signing session if the current round is mid-signing. When an earlier // round completed signing and a later RBF round is in AwaitingAck or @@ -7296,7 +7298,7 @@ where } pub(super) fn maybe_splice_funding_failed(&self) -> Option { - if !self.should_reset_pending_splice_state(false) { + if !self.should_reset_pending_splice_state(true) { return None; } @@ -15494,7 +15496,7 @@ impl Writeable for FundedChannel { ChannelState::ChannelReady(_) => { channel_state.clear_local_stfu_sent(); channel_state.clear_remote_stfu_sent(); - if self.should_reset_pending_splice_state(false) + if self.should_reset_pending_splice_state(true) || !self.has_pending_splice_awaiting_signatures() { // We shouldn't be quiescent anymore upon reconnecting if: @@ -15884,7 +15886,7 @@ impl Writeable for FundedChannel { // We don't have to worry about resetting the pending `FundingNegotiation` because we // can only read `FundingNegotiation::AwaitingSignatures` variants anyway. let pending_splice = - self.pending_splice.as_ref().filter(|_| !self.should_reset_pending_splice_state(false)); + self.pending_splice.as_ref().filter(|_| !self.should_reset_pending_splice_state(true)); let monitor_pending_tx_signatures = self.context.monitor_pending_tx_signatures.then_some(());