From df5db06fc05a702e317451b2df8c0b3e55fdb6cb Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 22 Jan 2026 09:49:53 -0800 Subject: [PATCH 1/3] Rework ChannelManager::funding_transaction_signed Previously, we'd emit a FundingTransactionReadyForSigning event once the initial commitment_signed is exchanged for a splicing/dual-funding attempt and require users to call back with their signed inputs using ChannelManager::funding_transaction_signed. While this approach worked in practice, it prevents us from abandoning a splice if we cannot or no longer wish to sign as the splice has already been committed to by this point. This commit reworks the API such that this is now possible. After exchanging tx_complete, we will no longer immediately send our initial commitment_signed. We will now emit the FundingTransactionReadyForSigning event and wait for the user to call back before releasing both our initial commitment_signed and our tx_signatures. As a result, the event is now persisted, as there is only one possible path in which it is generated. Note that we continue to only emit the event if a local contribution to negotiated transaction was made. Future work will expose a cancellation API such that we can abandon splice attempts safely (we can just force close the channel with dual-funding). --- lightning/src/ln/channel.rs | 177 +++++++++----- lightning/src/ln/channelmanager.rs | 242 ++++++++++--------- lightning/src/ln/functional_test_utils.rs | 2 +- lightning/src/ln/interactivetxs.rs | 5 +- lightning/src/ln/splicing_tests.rs | 278 +++++++++------------- 5 files changed, 350 insertions(+), 354 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index fd780da8d91..c6197423a5a 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1904,10 +1904,7 @@ where pub fn tx_complete( &mut self, msg: &msgs::TxComplete, logger: &L, - ) -> Result< - (Option, Option), - (ChannelError, Option), - > + ) -> Result)> where L::Target: Logger, { @@ -1934,13 +1931,38 @@ where let funding_outpoint = if let Some(funding_outpoint) = negotiation_complete { funding_outpoint } else { - return Ok((interactive_tx_msg_send, None)); + return Ok(TxCompleteResult { + interactive_tx_msg_send, + event_unsigned_tx: None, + funding_tx_signed: None, + }); }; - let commitment_signed = self - .funding_tx_constructed(funding_outpoint, logger) + self.funding_tx_constructed(funding_outpoint) .map_err(|abort_reason| self.fail_interactive_tx_negotiation(abort_reason, logger))?; - Ok((interactive_tx_msg_send, Some(commitment_signed))) + + let signing_session = self + .context() + .interactive_tx_signing_session + .as_ref() + .expect("The signing session must have been initialized in funding_tx_constructed"); + let has_local_contribution = signing_session.has_local_contribution(); + + let event_unsigned_tx = + has_local_contribution.then(|| signing_session.unsigned_tx().tx().clone()); + + let funding_tx_signed = if !has_local_contribution { + let funding_txid = signing_session.unsigned_tx().tx().compute_txid(); + if let ChannelPhase::Funded(chan) = &mut self.phase { + chan.funding_transaction_signed(funding_txid, vec![], 0, logger).ok() + } else { + None + } + } else { + None + }; + + Ok(TxCompleteResult { interactive_tx_msg_send, event_unsigned_tx, funding_tx_signed }) } pub fn tx_abort( @@ -2046,14 +2068,8 @@ where result.map(|monitor| (self.as_funded_mut().expect("Channel should be funded"), monitor)) } - fn funding_tx_constructed( - &mut self, funding_outpoint: OutPoint, logger: &L, - ) -> Result - where - L::Target: Logger, - { - let logger = WithChannelContext::from(logger, self.context(), None); - let (interactive_tx_constructor, commitment_signed) = match &mut self.phase { + fn funding_tx_constructed(&mut self, funding_outpoint: OutPoint) -> Result<(), AbortReason> { + let interactive_tx_constructor = match &mut self.phase { ChannelPhase::UnfundedV2(chan) => { debug_assert_eq!( chan.context.channel_state, @@ -2072,24 +2088,9 @@ where chan.funding.channel_transaction_parameters.funding_outpoint = Some(funding_outpoint); - let interactive_tx_constructor = chan - .interactive_tx_constructor + chan.interactive_tx_constructor .take() - .expect("PendingV2Channel::interactive_tx_constructor should be set"); - - let commitment_signed = - chan.context.get_initial_commitment_signed_v2(&chan.funding, &&logger); - let commitment_signed = match commitment_signed { - Some(commitment_signed) => commitment_signed, - // TODO(dual_funding): Support async signing - None => { - return Err(AbortReason::InternalError( - "Failed to compute commitment_signed signatures", - )); - }, - }; - - (interactive_tx_constructor, commitment_signed) + .expect("PendingV2Channel::interactive_tx_constructor should be set") }, ChannelPhase::Funded(chan) => { if let Some(pending_splice) = chan.pending_splice.as_mut() { @@ -2098,12 +2099,19 @@ where .take() .and_then(|funding_negotiation| { if let FundingNegotiation::ConstructingTransaction { - funding, + mut funding, interactive_tx_constructor, } = funding_negotiation { let is_initiator = interactive_tx_constructor.is_initiator(); - Some((is_initiator, funding, interactive_tx_constructor)) + funding.channel_transaction_parameters.funding_outpoint = + Some(funding_outpoint); + pending_splice.funding_negotiation = + Some(FundingNegotiation::AwaitingSignatures { + is_initiator, + funding, + }); + Some(interactive_tx_constructor) } else { // Replace the taken state for later error handling pending_splice.funding_negotiation = Some(funding_negotiation); @@ -2114,34 +2122,6 @@ where AbortReason::InternalError( "Got a tx_complete message in an invalid state", ) - }) - .and_then(|(is_initiator, mut funding, interactive_tx_constructor)| { - funding.channel_transaction_parameters.funding_outpoint = - Some(funding_outpoint); - match chan.context.get_initial_commitment_signed_v2(&funding, &&logger) - { - Some(commitment_signed) => { - // Advance the state - pending_splice.funding_negotiation = - Some(FundingNegotiation::AwaitingSignatures { - is_initiator, - funding, - }); - Ok((interactive_tx_constructor, commitment_signed)) - }, - // TODO(splicing): Support async signing - None => { - // Restore the taken state for later error handling - pending_splice.funding_negotiation = - Some(FundingNegotiation::ConstructingTransaction { - funding, - interactive_tx_constructor, - }); - Err(AbortReason::InternalError( - "Failed to compute commitment_signed signatures", - )) - }, - } })? } else { return Err(AbortReason::InternalError( @@ -2159,7 +2139,7 @@ where let signing_session = interactive_tx_constructor.into_signing_session(); self.context_mut().interactive_tx_signing_session = Some(signing_session); - Ok(commitment_signed) + Ok(()) } pub fn force_shutdown(&mut self, closure_reason: ClosureReason) -> ShutdownResult { @@ -2911,6 +2891,7 @@ where /// send it first. resend_order: RAACommitmentOrder, + monitor_pending_tx_signatures: bool, monitor_pending_channel_ready: bool, monitor_pending_revoke_and_ack: bool, monitor_pending_commitment_signed: bool, @@ -3642,6 +3623,7 @@ where resend_order: RAACommitmentOrder::CommitmentFirst, + monitor_pending_tx_signatures: false, monitor_pending_channel_ready: false, monitor_pending_revoke_and_ack: false, monitor_pending_commitment_signed: false, @@ -3881,6 +3863,7 @@ where resend_order: RAACommitmentOrder::CommitmentFirst, + monitor_pending_tx_signatures: false, monitor_pending_channel_ready: false, monitor_pending_revoke_and_ack: false, monitor_pending_commitment_signed: false, @@ -6860,8 +6843,25 @@ type BestBlockUpdatedRes = ( Option, ); +/// The result of handling a `tx_complete` message during interactive transaction construction. +pub(crate) struct TxCompleteResult { + /// The message to send to the counterparty, if any. + pub interactive_tx_msg_send: Option, + + /// If the negotiation completed and the holder has local contributions, this contains the + /// unsigned funding transaction for the `FundingTransactionReadyForSigning` event. + pub event_unsigned_tx: Option, + + /// If the negotiation completed and the holder has no local contributions, this contains + /// the result of automatically calling `funding_transaction_signed` with empty witnesses. + pub funding_tx_signed: Option, +} + /// The result of signing a funding transaction negotiated using the interactive-tx protocol. pub struct FundingTxSigned { + /// The initial `commitment_signed` message to send to the counterparty, if necessary. + pub commitment_signed: Option, + /// Signatures that should be sent to the counterparty, if necessary. pub tx_signatures: Option, @@ -8051,6 +8051,7 @@ where Vec::new(), logger, ); + self.context.monitor_pending_tx_signatures = true; Ok(self.push_ret_blockable_mon_update(monitor_update)) } @@ -9104,6 +9105,7 @@ where // Our `tx_signatures` either should've been the first time we processed them, // or we're waiting for our counterparty to send theirs first. return Ok(FundingTxSigned { + commitment_signed: None, tx_signatures: None, funding_tx: None, splice_negotiated: None, @@ -9117,6 +9119,7 @@ where // We may be handling a duplicate call and the funding was already locked so we // no longer have the signing session present. return Ok(FundingTxSigned { + commitment_signed: None, tx_signatures: None, funding_tx: None, splice_negotiated: None, @@ -9178,7 +9181,21 @@ where (None, None) }; - Ok(FundingTxSigned { tx_signatures, funding_tx, splice_negotiated, splice_locked }) + let funding = self + .pending_splice + .as_ref() + .and_then(|pending_splice| pending_splice.funding_negotiation.as_ref()) + .and_then(|funding_negotiation| funding_negotiation.as_funding()) + .unwrap_or(&self.funding); + let commitment_signed = self.context.get_initial_commitment_signed_v2(funding, &&logger); + + Ok(FundingTxSigned { + commitment_signed, + tx_signatures, + funding_tx, + splice_negotiated, + splice_locked, + }) } pub fn tx_signatures( @@ -9246,6 +9263,7 @@ where }; Ok(FundingTxSigned { + commitment_signed: None, tx_signatures: holder_tx_signatures, funding_tx, splice_negotiated, @@ -9451,6 +9469,25 @@ where self.context.channel_state.clear_monitor_update_in_progress(); assert_eq!(self.blocked_monitor_updates_pending(), 0); + let mut tx_signatures = self + .context + .monitor_pending_tx_signatures + .then(|| ()) + .and_then(|_| self.context.interactive_tx_signing_session.as_ref()) + .and_then(|signing_session| signing_session.holder_tx_signatures().clone()); + if tx_signatures.is_some() { + // We want to clear that the monitor update for our `tx_signatures` has completed, but + // we may still need to hold back the message until it's ready to be sent. + self.context.monitor_pending_tx_signatures = false; + let signing_session = self.context.interactive_tx_signing_session.as_ref() + .expect("We have a tx_signatures message so we must have a valid signing session"); + if !signing_session.holder_sends_tx_signatures_first() + && !signing_session.has_received_tx_signatures() + { + tx_signatures.take(); + } + } + // If we're past (or at) the AwaitingChannelReady stage on an outbound (or V2-established) channel, // try to (re-)broadcast the funding transaction as we may have declined to broadcast it when we // first received the funding_signed. @@ -9539,7 +9576,7 @@ where match commitment_order { RAACommitmentOrder::CommitmentFirst => "commitment", RAACommitmentOrder::RevokeAndACKFirst => "RAA"}); MonitorRestoreUpdates { raa, commitment_update, commitment_order, accepted_htlcs, failed_htlcs, finalized_claimed_htlcs, - pending_update_adds, funding_broadcastable, channel_ready, announcement_sigs, tx_signatures: None, + pending_update_adds, funding_broadcastable, channel_ready, announcement_sigs, tx_signatures, channel_ready_order, } } @@ -15074,6 +15111,9 @@ where let pending_splice = self.pending_splice.as_ref().filter(|_| !self.should_reset_pending_splice_state(false)); + let monitor_pending_tx_signatures = + self.context.monitor_pending_tx_signatures.then_some(()); + write_tlv_fields!(writer, { (0, self.context.announcement_sigs, option), // minimum_depth and counterparty_selected_channel_reserve_satoshis used to have a @@ -15093,6 +15133,7 @@ where (9, self.context.target_closing_feerate_sats_per_kw, option), (10, monitor_pending_update_adds, option), // Added in 0.0.122 (11, self.context.monitor_pending_finalized_fulfills, required_vec), + (12, monitor_pending_tx_signatures, option), // Added in 0.3 (13, self.context.channel_creation_height, required), (15, preimages, required_vec), (17, self.context.announcement_sigs_state, required), @@ -15524,6 +15565,8 @@ where let mut holding_cell_accountable: Option> = None; let mut pending_outbound_accountable: Option> = None; + let mut monitor_pending_tx_signatures: Option<()> = None; + read_tlv_fields!(reader, { (0, announcement_sigs, option), (1, minimum_depth, option), @@ -15537,6 +15580,7 @@ where (9, target_closing_feerate_sats_per_kw, option), (10, monitor_pending_update_adds, option), // Added in 0.0.122 (11, monitor_pending_finalized_fulfills, optional_vec), + (12, monitor_pending_tx_signatures, option), // Added in 0.3 (13, channel_creation_height, required), (15, preimages, required_vec), // The preimages transitioned from optional to required in 0.2 (17, announcement_sigs_state, required), @@ -15949,6 +15993,7 @@ where resend_order, + monitor_pending_tx_signatures: monitor_pending_tx_signatures.is_some(), monitor_pending_channel_ready, monitor_pending_revoke_and_ack, monitor_pending_commitment_signed, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index dafeffe98bf..598e1d3b554 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -6455,7 +6455,8 @@ where &self.logger, ) { Ok(FundingTxSigned { - tx_signatures: Some(tx_signatures), + commitment_signed, + tx_signatures, funding_tx, splice_negotiated, splice_locked, @@ -6481,19 +6482,39 @@ where None, )); } - peer_state.pending_msg_events.push( - MessageSendEvent::SendTxSignatures { - node_id: *counterparty_node_id, - msg: tx_signatures, - }, - ); - if let Some(splice_locked) = splice_locked { - peer_state.pending_msg_events.push( - MessageSendEvent::SendSpliceLocked { - node_id: *counterparty_node_id, - msg: splice_locked, - }, - ); + if chan.context.is_connected() { + if let Some(commitment_signed) = commitment_signed { + peer_state.pending_msg_events.push( + MessageSendEvent::UpdateHTLCs { + node_id: *counterparty_node_id, + channel_id: *channel_id, + updates: CommitmentUpdate { + commitment_signed: vec![commitment_signed], + update_add_htlcs: vec![], + update_fulfill_htlcs: vec![], + update_fail_htlcs: vec![], + update_fail_malformed_htlcs: vec![], + update_fee: None, + }, + }, + ); + } + if let Some(tx_signatures) = tx_signatures { + peer_state.pending_msg_events.push( + MessageSendEvent::SendTxSignatures { + node_id: *counterparty_node_id, + msg: tx_signatures, + }, + ); + } + if let Some(splice_locked) = splice_locked { + peer_state.pending_msg_events.push( + MessageSendEvent::SendSpliceLocked { + node_id: *counterparty_node_id, + msg: splice_locked, + }, + ); + } } return NotifyOption::DoPersist; }, @@ -6501,17 +6522,6 @@ where result = Err(err); return NotifyOption::SkipPersistNoEvents; }, - Ok(FundingTxSigned { - tx_signatures: None, - funding_tx, - splice_negotiated, - splice_locked, - }) => { - debug_assert!(funding_tx.is_none()); - debug_assert!(splice_negotiated.is_none()); - debug_assert!(splice_locked.is_none()); - return NotifyOption::SkipPersistNoEvents; - }, } }, None => { @@ -10151,79 +10161,6 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ } } - if let Some(signing_session) = (!channel.is_awaiting_monitor_update()) - .then(|| ()) - .and_then(|_| channel.context.interactive_tx_signing_session.as_mut()) - .filter(|signing_session| signing_session.has_received_commitment_signed()) - .filter(|signing_session| signing_session.holder_tx_signatures().is_none()) - { - if signing_session.has_local_contribution() { - let mut pending_events = self.pending_events.lock().unwrap(); - let unsigned_transaction = signing_session.unsigned_tx().tx().clone(); - let event_action = ( - Event::FundingTransactionReadyForSigning { - unsigned_transaction, - counterparty_node_id, - channel_id: channel.context.channel_id(), - user_channel_id: channel.context.get_user_id(), - }, - None, - ); - - if !pending_events.contains(&event_action) { - pending_events.push_back(event_action); - } - } else { - let txid = signing_session.unsigned_tx().compute_txid(); - let best_block_height = self.best_block.read().unwrap().height; - match channel.funding_transaction_signed(txid, vec![], best_block_height, &self.logger) { - Ok(FundingTxSigned { - tx_signatures: Some(tx_signatures), - funding_tx, - splice_negotiated, - splice_locked, - }) => { - if let Some(funding_tx) = funding_tx { - self.broadcast_interactive_funding(channel, &funding_tx, &self.logger); - } - - if let Some(splice_negotiated) = splice_negotiated { - self.pending_events.lock().unwrap().push_back(( - events::Event::SplicePending { - channel_id: channel.context.channel_id(), - counterparty_node_id, - user_channel_id: channel.context.get_user_id(), - new_funding_txo: splice_negotiated.funding_txo, - channel_type: splice_negotiated.channel_type, - new_funding_redeem_script: splice_negotiated.funding_redeem_script, - }, - None, - )); - } - - if channel.context.is_connected() { - pending_msg_events.push(MessageSendEvent::SendTxSignatures { - node_id: counterparty_node_id, - msg: tx_signatures, - }); - if let Some(splice_locked) = splice_locked { - pending_msg_events.push(MessageSendEvent::SendSpliceLocked { - node_id: counterparty_node_id, - msg: splice_locked, - }); - } - } - }, - Ok(FundingTxSigned { tx_signatures: None, .. }) => { - debug_assert!(false, "If our tx_signatures is empty, then we should send it first!"); - }, - Err(err) => { - log_warn!(logger, "Failed signing interactive funding transaction: {err:?}"); - }, - } - } - } - { let mut pending_events = self.pending_events.lock().unwrap(); emit_channel_pending_event!(pending_events, channel); @@ -11206,30 +11143,69 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ hash_map::Entry::Occupied(mut chan_entry) => { let chan = chan_entry.get_mut(); match chan.tx_complete(msg, &self.logger) { - Ok((interactive_tx_msg_send, commitment_signed)) => { - let persist = if interactive_tx_msg_send.is_some() || commitment_signed.is_some() { - NotifyOption::SkipPersistHandleEvents - } else { - NotifyOption::SkipPersistNoEvents - }; - if let Some(interactive_tx_msg_send) = interactive_tx_msg_send { + Ok(tx_complete_result) => { + let mut persist = NotifyOption::SkipPersistNoEvents; + + if let Some(interactive_tx_msg_send) = tx_complete_result.interactive_tx_msg_send { let msg_send_event = interactive_tx_msg_send.into_msg_send_event(counterparty_node_id); peer_state.pending_msg_events.push(msg_send_event); + persist = NotifyOption::SkipPersistHandleEvents; }; - if let Some(commitment_signed) = commitment_signed { - peer_state.pending_msg_events.push(MessageSendEvent::UpdateHTLCs { - node_id: counterparty_node_id, - channel_id: msg.channel_id, - updates: CommitmentUpdate { - commitment_signed: vec![commitment_signed], - update_add_htlcs: vec![], - update_fulfill_htlcs: vec![], - update_fail_htlcs: vec![], - update_fail_malformed_htlcs: vec![], - update_fee: None, + + if let Some(unsigned_transaction) = tx_complete_result.event_unsigned_tx { + self.pending_events.lock().unwrap().push_back(( + events::Event::FundingTransactionReadyForSigning { + unsigned_transaction, + counterparty_node_id, + channel_id: msg.channel_id, + user_channel_id: chan.context().get_user_id(), }, - }); + None, + )); + // + // We have a successful signing session that we need to persist. + persist = NotifyOption::DoPersist; + } + + if let Some(FundingTxSigned { + commitment_signed, + tx_signatures, + funding_tx, + splice_negotiated, + splice_locked, + }) = tx_complete_result.funding_tx_signed + { + // We shouldn't expect to see the splice negotiated or locked yet as we + // haven't exchanged `tx_signatures` at this point. + debug_assert!(funding_tx.is_none()); + debug_assert!(splice_negotiated.is_none()); + debug_assert!(splice_locked.is_none()); + + if let Some(commitment_signed) = commitment_signed { + peer_state.pending_msg_events.push(MessageSendEvent::UpdateHTLCs { + node_id: counterparty_node_id, + channel_id: msg.channel_id, + updates: CommitmentUpdate { + commitment_signed: vec![commitment_signed], + update_add_htlcs: vec![], + update_fulfill_htlcs: vec![], + update_fail_htlcs: vec![], + update_fail_malformed_htlcs: vec![], + update_fee: None, + }, + }); + } + if let Some(tx_signatures) = tx_signatures { + peer_state.pending_msg_events.push(MessageSendEvent::SendTxSignatures { + node_id: counterparty_node_id, + msg: tx_signatures, + }); + } + + // We have a successful signing session that we need to persist. + persist = NotifyOption::DoPersist; } + Ok(persist) }, Err((error, splice_funding_failed)) => { @@ -11274,6 +11250,7 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ Some(chan) => { let best_block_height = self.best_block.read().unwrap().height; let FundingTxSigned { + commitment_signed, tx_signatures, funding_tx, splice_negotiated, @@ -11284,6 +11261,11 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ chan.tx_signatures(msg, best_block_height, &self.logger), chan_entry ); + + // We should never be sending a `commitment_signed` in response to their + // `tx_signatures`. + debug_assert!(commitment_signed.is_none()); + if let Some(tx_signatures) = tx_signatures { peer_state.pending_msg_events.push(MessageSendEvent::SendTxSignatures { node_id: *counterparty_node_id, @@ -19013,6 +18995,32 @@ where } } + // We may need to regenerate [`Event::FundingTransactionReadyForSigning`] for channels that + // still need their holder `tx_signatures`. + for (counterparty_node_id, peer_state_mutex) in per_peer_state.iter() { + let peer_state = peer_state_mutex.lock().unwrap(); + for (channel_id, chan) in peer_state.channel_by_id.iter() { + if let Some(signing_session) = + chan.context().interactive_tx_signing_session.as_ref() + { + if signing_session.holder_tx_signatures().is_none() + && signing_session.has_local_contribution() + { + let unsigned_transaction = signing_session.unsigned_tx().tx().clone(); + pending_events_read.push_back(( + Event::FundingTransactionReadyForSigning { + unsigned_transaction, + counterparty_node_id: *counterparty_node_id, + channel_id: *channel_id, + user_channel_id: chan.context().get_user_id(), + }, + None, + )); + } + } + } + } + let best_block = BestBlock::new(best_block_hash, best_block_height); let flow = OffersMessageFlow::new( chain_hash, diff --git a/lightning/src/ln/functional_test_utils.rs b/lightning/src/ln/functional_test_utils.rs index 2cf5ea96acb..a70879718e2 100644 --- a/lightning/src/ln/functional_test_utils.rs +++ b/lightning/src/ln/functional_test_utils.rs @@ -1086,7 +1086,7 @@ macro_rules! get_event { let ev = events.pop().unwrap(); match ev { $event_type { .. } => ev, - _ => panic!("Unexpected event"), + _ => panic!("Unexpected event {ev:?}"), } }}; } diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index 7ed829886c6..f402ac5efa6 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -668,10 +668,9 @@ impl InteractiveTxSigningSession { self.holder_tx_signatures = Some(tx_signatures); let funding_tx_opt = self.maybe_finalize_funding_tx(); - let holder_tx_signatures = (self.holder_sends_tx_signatures_first - || self.has_received_tx_signatures()) + let holder_tx_signatures = (self.has_received_commitment_signed + && (self.holder_sends_tx_signatures_first || self.has_received_tx_signatures())) .then(|| { - debug_assert!(self.has_received_commitment_signed); self.holder_tx_signatures.clone().expect("Holder tx_signatures were just provided") }); diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index db6680d963c..c0ba401cfae 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -27,7 +27,6 @@ use crate::ln::types::ChannelId; use crate::routing::router::{PaymentParameters, RouteParameters}; use crate::util::errors::APIError; use crate::util::ser::Writeable; -use crate::util::test_channel_signer::SignerOp; use bitcoin::hashes::Hash; use bitcoin::secp256k1::PublicKey; @@ -132,7 +131,7 @@ fn test_v1_splice_in_negative_insufficient_inputs() { pub fn negotiate_splice_tx<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, initiator_contribution: SpliceContribution, -) -> msgs::CommitmentSigned { +) { let new_funding_script = complete_splice_handshake(initiator, acceptor, channel_id, initiator_contribution.clone()); complete_interactive_funding_negotiation( @@ -141,7 +140,7 @@ pub fn negotiate_splice_tx<'a, 'b, 'c, 'd>( channel_id, initiator_contribution, new_funding_script, - ) + ); } pub fn complete_splice_handshake<'a, 'b, 'c, 'd>( @@ -184,7 +183,7 @@ pub fn complete_splice_handshake<'a, 'b, 'c, 'd>( pub fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>( initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, channel_id: ChannelId, initiator_contribution: SpliceContribution, new_funding_script: ScriptBuf, -) -> msgs::CommitmentSigned { +) { let node_id_initiator = initiator.node.get_our_node_id(); let node_id_acceptor = acceptor.node.get_our_node_id(); @@ -240,22 +239,15 @@ pub fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>( ); acceptor.node.handle_tx_add_output(node_id_initiator, &tx_add_output); } else { - let mut msg_events = initiator.node.get_and_clear_pending_msg_events(); - assert_eq!( - msg_events.len(), - if acceptor_sent_tx_complete { 2 } else { 1 }, - "{msg_events:?}" - ); - if let MessageSendEvent::SendTxComplete { ref msg, .. } = msg_events.remove(0) { + let msg_events = initiator.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); + if let MessageSendEvent::SendTxComplete { ref msg, .. } = &msg_events[0] { acceptor.node.handle_tx_complete(node_id_initiator, msg); } else { panic!(); } if acceptor_sent_tx_complete { - if let MessageSendEvent::UpdateHTLCs { mut updates, .. } = msg_events.remove(0) { - return updates.commitment_signed.remove(0); - } - panic!(); + break; } } @@ -271,13 +263,38 @@ pub fn complete_interactive_funding_negotiation<'a, 'b, 'c, 'd>( } pub fn sign_interactive_funding_tx<'a, 'b, 'c, 'd>( - initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, - initial_commit_sig_for_acceptor: msgs::CommitmentSigned, is_0conf: bool, + initiator: &'a Node<'b, 'c, 'd>, acceptor: &'a Node<'b, 'c, 'd>, is_0conf: bool, ) -> (Transaction, Option<(msgs::SpliceLocked, PublicKey)>) { let node_id_initiator = initiator.node.get_our_node_id(); let node_id_acceptor = acceptor.node.get_our_node_id(); assert!(initiator.node.get_and_clear_pending_msg_events().is_empty()); + + let event = get_event!(initiator, Event::FundingTransactionReadyForSigning); + if let Event::FundingTransactionReadyForSigning { + channel_id, + counterparty_node_id, + unsigned_transaction, + .. + } = event + { + let partially_signed_tx = initiator.wallet_source.sign_tx(unsigned_transaction).unwrap(); + initiator + .node + .funding_transaction_signed(&channel_id, &counterparty_node_id, partially_signed_tx) + .unwrap(); + } else { + panic!(); + } + + let msg_events = initiator.node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); + let initial_commit_sig_for_acceptor = + if let MessageSendEvent::UpdateHTLCs { ref updates, .. } = &msg_events[0] { + updates.commitment_signed[0].clone() + } else { + panic!(); + }; acceptor.node.handle_commitment_signed(node_id_initiator, &initial_commit_sig_for_acceptor); let msg_events = acceptor.node.get_and_clear_pending_msg_events(); @@ -294,20 +311,6 @@ pub fn sign_interactive_funding_tx<'a, 'b, 'c, 'd>( panic!(); } - 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(); - } let mut msg_events = initiator.node.get_and_clear_pending_msg_events(); assert_eq!(msg_events.len(), if is_0conf { 2 } else { 1 }, "{msg_events:?}"); if let MessageSendEvent::SendTxSignatures { ref msg, .. } = &msg_events[0] { @@ -348,15 +351,14 @@ pub fn splice_channel<'a, 'b, 'c, 'd>( let new_funding_script = complete_splice_handshake(initiator, acceptor, channel_id, initiator_contribution.clone()); - let initial_commit_sig_for_acceptor = complete_interactive_funding_negotiation( + complete_interactive_funding_negotiation( initiator, acceptor, channel_id, initiator_contribution, new_funding_script, ); - let (splice_tx, splice_locked) = - sign_interactive_funding_tx(initiator, acceptor, initial_commit_sig_for_acceptor, false); + let (splice_tx, splice_locked) = sign_interactive_funding_tx(initiator, acceptor, false); assert!(splice_locked.is_none()); expect_splice_pending_event(initiator, &node_id_acceptor); @@ -642,15 +644,13 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { nodes[0].node.handle_tx_complete(node_id_1, &tx_complete); let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 2); + assert_eq!(msg_events.len(), 1); if let MessageSendEvent::SendTxComplete { .. } = &msg_events[0] { } else { panic!("Unexpected event"); } - if let MessageSendEvent::UpdateHTLCs { .. } = &msg_events[1] { - } else { - panic!("Unexpected event"); - } + + let _event = get_event!(nodes[0], Event::FundingTransactionReadyForSigning); if reload { let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode(); @@ -671,6 +671,8 @@ fn do_test_splice_state_reset_on_disconnect(reload: bool) { chain_monitor_1c, node_1c ); + // We should have another signing event generated upon reload as they're not persisted. + let _ = get_event!(nodes[0], Event::FundingTransactionReadyForSigning); } else { nodes[0].node.peer_disconnected(node_id_1); nodes[1].node.peer_disconnected(node_id_0); @@ -1290,25 +1292,17 @@ fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) { script_pubkey: nodes[1].wallet_source.get_change_script().unwrap(), }, ]); - let initial_commit_sig_for_acceptor = - negotiate_splice_tx(&nodes[0], &nodes[1], channel_id, initiator_contribution); - assert_eq!(initial_commit_sig_for_acceptor.htlc_signatures.len(), 1); - let initial_commit_sig_for_initiator = get_htlc_update_msgs(&nodes[1], &node_id_0); - assert_eq!(initial_commit_sig_for_initiator.commitment_signed.len(), 1); - assert_eq!(initial_commit_sig_for_initiator.commitment_signed[0].htlc_signatures.len(), 1); + negotiate_splice_tx(&nodes[0], &nodes[1], channel_id, initiator_contribution); - macro_rules! reconnect_nodes { - ($f: expr) => { - nodes[0].node.peer_disconnected(node_id_1); - nodes[1].node.peer_disconnected(node_id_0); - let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); - $f(&mut reconnect_args); - reconnect_nodes(reconnect_args); - }; - } + // Node 0 should have a signing event to handle since they had a contribution in the splice. + // Node 1 won't and will immediately try to send their initial `commitment_signed`. + let signing_event = get_event!(nodes[0], Event::FundingTransactionReadyForSigning); + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); - // Reestablishing now should force both nodes to retransmit their initial `commitment_signed` - // message as they were never delivered. + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + let _ = get_htlc_update_msgs(&nodes[1], &node_id_0); + + // Disconnect them, and handle the signing event on the initiator side. if reload { let encoded_monitor_0 = get_monitor!(nodes[0], channel_id).encode(); reload_node!( @@ -1328,6 +1322,8 @@ fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) { chain_monitor_1a, node_1a ); + // We should have another signing event generated upon reload as they're not persisted. + let _ = get_event!(nodes[0], Event::FundingTransactionReadyForSigning); if async_monitor_update { persister_0a.set_update_ret(ChannelMonitorUpdateStatus::InProgress); persister_1a.set_update_ret(ChannelMonitorUpdateStatus::InProgress); @@ -1341,6 +1337,17 @@ fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) { } } + if let Event::FundingTransactionReadyForSigning { unsigned_transaction, .. } = signing_event { + let tx = nodes[0].wallet_source.sign_tx(unsigned_transaction).unwrap(); + nodes[0].node.funding_transaction_signed(&channel_id, &node_id_1, tx).unwrap(); + } + + // Since they're not connected, no messages should be sent. + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + assert!(nodes[1].node.get_and_clear_pending_msg_events().is_empty()); + + // Reestablishing now should force both nodes to retransmit their initial `commitment_signed` + // message as they were never delivered. let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); reconnect_args.send_interactive_tx_commit_sig = (true, true); reconnect_nodes(reconnect_args); @@ -1350,6 +1357,16 @@ fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) { check_added_monitors(&nodes[0], 1); check_added_monitors(&nodes[1], 1); + macro_rules! reconnect_nodes { + ($f: expr) => { + nodes[0].node.peer_disconnected(node_id_1); + nodes[1].node.peer_disconnected(node_id_0); + let mut reconnect_args = ReconnectArgs::new(&nodes[0], &nodes[1]); + $f(&mut reconnect_args); + reconnect_nodes(reconnect_args); + }; + } + if async_monitor_update { // Reconnecting again should result in no messages/events being generated as the monitor // update is pending. @@ -1364,11 +1381,9 @@ fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) { chanmon_cfgs[1].persister.set_update_ret(ChannelMonitorUpdateStatus::Completed); } - // Node 0 should have a signing event to handle since they had a contribution in the splice. - // Node 1 won't and will immediately send `tx_signatures`. - let _ = get_event!(nodes[0], Event::FundingTransactionReadyForSigning); + // Both nodes should have their `tx_signatures` ready after completing the monitor update, but + // node 0 has to wait for node 1 to send theirs first. assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); - assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); let _ = get_event_msg!(nodes[1], MessageSendEvent::SendTxSignatures, node_id_0); // Reconnecting now should force node 1 to retransmit their `tx_signatures` since it was never @@ -1377,18 +1392,6 @@ fn do_test_splice_reestablish(reload: bool, async_monitor_update: bool) { reconnect_nodes!(|reconnect_args: &mut ReconnectArgs| { reconnect_args.send_interactive_tx_sigs = (true, false); }); - let _ = get_event!(nodes[0], Event::FundingTransactionReadyForSigning); - - // Reconnect again to make sure node 1 doesn't retransmit `tx_signatures` unnecessarily as it - // was delivered in the previous reestablishment. - reconnect_nodes!(|_| {}); - - // Have node 0 sign, we should see its `tx_signatures` go out. - let event = get_event!(nodes[0], Event::FundingTransactionReadyForSigning); - if let Event::FundingTransactionReadyForSigning { unsigned_transaction, .. } = event { - let tx = nodes[0].wallet_source.sign_tx(unsigned_transaction).unwrap(); - nodes[0].node.funding_transaction_signed(&channel_id, &node_id_1, tx).unwrap(); - } let _ = get_event_msg!(nodes[0], MessageSendEvent::SendTxSignatures, node_id_1); expect_splice_pending_event(&nodes[0], &node_id_1); @@ -1639,25 +1642,22 @@ fn do_test_propose_splice_while_disconnected(reload: bool, use_0conf: bool) { .unwrap(); // Negotiate the first splice to completion. - let initial_commit_sig = { - nodes[1].node.handle_splice_init(node_id_0, &splice_init); - let splice_ack = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceAck, node_id_0); - nodes[0].node.handle_splice_ack(node_id_1, &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( - &nodes[0], - &nodes[1], - channel_id, - node_0_contribution, - new_funding_script, - ) - }; - let (splice_tx, splice_locked) = - sign_interactive_funding_tx(&nodes[0], &nodes[1], initial_commit_sig, use_0conf); + nodes[1].node.handle_splice_init(node_id_0, &splice_init); + let splice_ack = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceAck, node_id_0); + nodes[0].node.handle_splice_ack(node_id_1, &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( + &nodes[0], + &nodes[1], + channel_id, + node_0_contribution, + new_funding_script, + ); + let (splice_tx, splice_locked) = sign_interactive_funding_tx(&nodes[0], &nodes[1], use_0conf); expect_splice_pending_event(&nodes[0], &node_id_1); expect_splice_pending_event(&nodes[1], &node_id_0); @@ -1781,25 +1781,22 @@ fn do_test_propose_splice_while_disconnected(reload: bool, use_0conf: bool) { } let splice_init = get_event_msg!(nodes[1], MessageSendEvent::SendSpliceInit, node_id_0); - let initial_commit_sig = { - nodes[0].node.handle_splice_init(node_id_1, &splice_init); - let splice_ack = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceAck, node_id_1); - nodes[1].node.handle_splice_ack(node_id_0, &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( - &nodes[1], - &nodes[0], - channel_id, - node_1_contribution, - new_funding_script, - ) - }; - let (splice_tx, splice_locked) = - sign_interactive_funding_tx(&nodes[1], &nodes[0], initial_commit_sig, use_0conf); + nodes[0].node.handle_splice_init(node_id_1, &splice_init); + let splice_ack = get_event_msg!(nodes[0], MessageSendEvent::SendSpliceAck, node_id_1); + nodes[1].node.handle_splice_ack(node_id_0, &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( + &nodes[1], + &nodes[0], + channel_id, + node_1_contribution, + new_funding_script, + ); + let (splice_tx, splice_locked) = sign_interactive_funding_tx(&nodes[1], &nodes[0], use_0conf); expect_splice_pending_event(&nodes[0], &node_id_1); expect_splice_pending_event(&nodes[1], &node_id_0); @@ -1829,7 +1826,7 @@ fn disconnect_on_unexpected_interactive_tx_message() { let initiator = &nodes[0]; let acceptor = &nodes[1]; - let _node_id_initiator = initiator.node.get_our_node_id(); + 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; @@ -1846,11 +1843,10 @@ fn disconnect_on_unexpected_interactive_tx_message() { // Complete interactive-tx construction, but fail by having the acceptor send a duplicate // tx_complete instead of commitment_signed. - let _ = negotiate_splice_tx(initiator, acceptor, channel_id, contribution.clone()); + negotiate_splice_tx(initiator, acceptor, channel_id, contribution.clone()); - let mut msg_events = acceptor.node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1); - assert!(matches!(msg_events.remove(0), MessageSendEvent::UpdateHTLCs { .. })); + let _ = get_event!(initiator, Event::FundingTransactionReadyForSigning); + let _ = get_htlc_update_msgs(acceptor, &node_id_initiator); let tx_complete = msgs::TxComplete { channel_id }; initiator.node.handle_tx_complete(node_id_acceptor, &tx_complete); @@ -1910,58 +1906,6 @@ fn fail_splice_on_interactive_tx_error() { let tx_abort = get_event_msg!(acceptor, MessageSendEvent::SendTxAbort, node_id_initiator); initiator.node.handle_tx_abort(node_id_acceptor, &tx_abort); - - // Fail signing the commitment transaction, which prevents the initiator from sending - // tx_complete. - initiator.disable_channel_signer_op( - &node_id_acceptor, - &channel_id, - SignerOp::SignCounterpartyCommitment, - ); - let _ = complete_splice_handshake(initiator, acceptor, channel_id, contribution.clone()); - - let tx_add_input = - get_event_msg!(initiator, MessageSendEvent::SendTxAddInput, node_id_acceptor); - acceptor.node.handle_tx_add_input(node_id_initiator, &tx_add_input); - - let tx_complete = get_event_msg!(acceptor, MessageSendEvent::SendTxComplete, node_id_initiator); - initiator.node.handle_tx_complete(node_id_acceptor, &tx_complete); - - let tx_add_input = - get_event_msg!(initiator, MessageSendEvent::SendTxAddInput, node_id_acceptor); - acceptor.node.handle_tx_add_input(node_id_initiator, &tx_add_input); - - let tx_complete = get_event_msg!(acceptor, MessageSendEvent::SendTxComplete, node_id_initiator); - initiator.node.handle_tx_complete(node_id_acceptor, &tx_complete); - - let tx_add_output = - get_event_msg!(initiator, MessageSendEvent::SendTxAddOutput, node_id_acceptor); - acceptor.node.handle_tx_add_output(node_id_initiator, &tx_add_output); - - let tx_complete = get_event_msg!(acceptor, MessageSendEvent::SendTxComplete, node_id_initiator); - initiator.node.handle_tx_complete(node_id_acceptor, &tx_complete); - - let tx_add_output = - get_event_msg!(initiator, MessageSendEvent::SendTxAddOutput, node_id_acceptor); - acceptor.node.handle_tx_add_output(node_id_initiator, &tx_add_output); - - let tx_complete = get_event_msg!(acceptor, MessageSendEvent::SendTxComplete, node_id_initiator); - initiator.node.handle_tx_complete(node_id_acceptor, &tx_complete); - - let event = get_event!(initiator, Event::SpliceFailed); - match event { - Event::SpliceFailed { contributed_inputs, .. } => { - assert_eq!(contributed_inputs.len(), 1); - assert_eq!(contributed_inputs[0], contribution.inputs()[0].outpoint()); - }, - _ => panic!("Expected Event::SpliceFailed"), - } - - let tx_abort = get_event_msg!(initiator, MessageSendEvent::SendTxAbort, node_id_acceptor); - 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); } #[test] From ffeb8617c1ffbcb6fff66bc2d12f32a152548f8b Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 22 Jan 2026 09:49:59 -0800 Subject: [PATCH 2/3] Buffer interactive-tx initial commitment signed from counterparty This is crucial to enable the splice cancellation use case. When we process the initial commitment signed from our counterparty, we queue a monitor update that cannot be undone. To give the user a chance to abort the splice negotiation before it's committed to, we buffer the message until a successful call to `Channel::funding_transaction_signed` and process it then. Note that this is currently only done for splice and RBF attempts, as if we want to abort a dual funding negotiation, we can just force close the channel as it hasn't been funded yet. --- lightning/src/ln/channel.rs | 88 +++++++++- lightning/src/ln/channelmanager.rs | 254 ++++++++++++++++++----------- lightning/src/ln/splicing_tests.rs | 231 ++++++++++++++++++++++++++ 3 files changed, 473 insertions(+), 100 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index c6197423a5a..f0caba66fd5 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2110,6 +2110,7 @@ where Some(FundingNegotiation::AwaitingSignatures { is_initiator, funding, + initial_commitment_signed_from_counterparty: None, }); Some(interactive_tx_constructor) } else { @@ -2203,9 +2204,33 @@ where // which must always come after the initial commitment signed is sent. .unwrap_or(true); let res = if has_negotiated_pending_splice && !session_received_commitment_signed { - funded_channel - .splice_initial_commitment_signed(msg, fee_estimator, logger) - .map(|monitor_update_opt| (None, monitor_update_opt)) + let has_holder_tx_signatures = funded_channel + .context + .interactive_tx_signing_session + .as_ref() + .map(|session| session.holder_tx_signatures().is_some()) + .unwrap_or(false); + + // We delay processing this until the user manually approves the splice via + // [`FundedChannel::funding_transaction_signed`], as otherwise, there would be a + // [`ChannelMonitorUpdateStep::RenegotiatedFunding`] committed that we would + // need to undo if they no longer wish to proceed. + if has_holder_tx_signatures { + funded_channel + .splice_initial_commitment_signed(msg, fee_estimator, logger) + .map(|monitor_update_opt| (None, monitor_update_opt)) + } else { + let pending_splice = funded_channel.pending_splice.as_mut() + .expect("We have a pending splice negotiated"); + let funding_negotiation = pending_splice.funding_negotiation.as_mut() + .expect("We have a pending splice negotiated"); + if let FundingNegotiation::AwaitingSignatures { + ref mut initial_commitment_signed_from_counterparty, .. + } = funding_negotiation { + *initial_commitment_signed_from_counterparty = Some(msg.clone()); + } + Ok((None, None)) + } } else { funded_channel.commitment_signed(msg, fee_estimator, logger) .map(|monitor_update_opt| (None, monitor_update_opt)) @@ -2689,6 +2714,17 @@ enum FundingNegotiation { AwaitingSignatures { funding: FundingScope, is_initiator: bool, + /// The initial [`msgs::CommitmentSigned`] message received for the [`FundingScope`] above. + /// We delay processing this until the user manually approves the splice via + /// [`FundedChannel::funding_transaction_signed`], as otherwise, there would be a + /// [`ChannelMonitorUpdateStep::RenegotiatedFunding`] committed that we would need to undo + /// if they no longer wish to proceed. + /// + /// Note that this doesn't need to be done with dual-funded channels as there is no + /// equivalent monitor update for them, and we can just force close the channel. + /// + /// This field is not persisted as the message should be resent on reconnections. + initial_commitment_signed_from_counterparty: Option, }, } @@ -2696,6 +2732,7 @@ impl_writeable_tlv_based_enum_upgradable!(FundingNegotiation, (0, AwaitingSignatures) => { (1, funding, required), (3, is_initiator, required), + (_unused, initial_commitment_signed_from_counterparty, (static_value, None)), }, unread_variants: AwaitingAck, ConstructingTransaction ); @@ -9023,6 +9060,50 @@ where } } + /// If a counterparty's initial `msgs::CommitmentSigned` is currently pending to be processed + /// due to the holder signatures not being avialable at the time of receipt, attempt to process + /// it now if they are available. + pub fn maybe_process_pending_counterparty_initial_commitment_signed( + &mut self, fee_estimator: &LowerBoundedFeeEstimator, logger: &L, + ) -> Result, ChannelError> + where + F::Target: FeeEstimator, + L::Target: Logger, + { + if self + .context + .interactive_tx_signing_session + .as_ref() + .map(|signing_session| { + !signing_session.has_local_contribution() + && signing_session.holder_tx_signatures().is_none() + }) + .unwrap_or(true) + { + return Ok(None); + } + + if let Some(commit_sig) = self + .pending_splice + .as_mut() + .and_then(|pending_splice| pending_splice.funding_negotiation.as_mut()) + .and_then(|funding_negotiation| { + if let FundingNegotiation::AwaitingSignatures { + ref mut initial_commitment_signed_from_counterparty, + .. + } = funding_negotiation + { + initial_commitment_signed_from_counterparty.take() + } else { + None + } + }) { + self.splice_initial_commitment_signed(&commit_sig, fee_estimator, logger) + } else { + Ok(None) + } + } + fn on_tx_signatures_exchange<'a, L: Deref>( &mut self, funding_tx: Transaction, best_block_height: u32, logger: &WithChannelContext<'a, L>, @@ -9106,6 +9187,7 @@ where // or we're waiting for our counterparty to send theirs first. return Ok(FundingTxSigned { commitment_signed: None, + tx_signatures: None, funding_tx: None, splice_negotiated: None, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 598e1d3b554..a10e0fedbe9 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -6424,118 +6424,162 @@ where pub fn funding_transaction_signed( &self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, transaction: Transaction, ) -> Result<(), APIError> { - let mut result = Ok(()); + let mut funding_tx_signed_result = Ok(()); + let mut monitor_update_result: Option< + Result, + > = None; + PersistenceNotifierGuard::optionally_notify(self, || { let per_peer_state = self.per_peer_state.read().unwrap(); let peer_state_mutex_opt = per_peer_state.get(counterparty_node_id); if peer_state_mutex_opt.is_none() { - result = Err(APIError::ChannelUnavailable { + funding_tx_signed_result = Err(APIError::ChannelUnavailable { err: format!("Can't find a peer matching the passed counterparty node_id {counterparty_node_id}") }); return NotifyOption::SkipPersistNoEvents; } - let mut peer_state = peer_state_mutex_opt.unwrap().lock().unwrap(); + let mut peer_state_lock = peer_state_mutex_opt.unwrap().lock().unwrap(); + let peer_state = &mut *peer_state_lock; - match peer_state.channel_by_id.get_mut(channel_id) { - Some(channel) => match channel.as_funded_mut() { - Some(chan) => { - let txid = transaction.compute_txid(); - let witnesses: Vec<_> = transaction - .input - .into_iter() - .map(|input| input.witness) - .filter(|witness| !witness.is_empty()) - .collect(); - let best_block_height = self.best_block.read().unwrap().height; - match chan.funding_transaction_signed( - txid, - witnesses, - best_block_height, - &self.logger, - ) { - Ok(FundingTxSigned { - commitment_signed, - tx_signatures, - funding_tx, - splice_negotiated, - splice_locked, - }) => { - if let Some(funding_tx) = funding_tx { - self.broadcast_interactive_funding( - chan, - &funding_tx, - &self.logger, - ); - } - if let Some(splice_negotiated) = splice_negotiated { - self.pending_events.lock().unwrap().push_back(( - events::Event::SplicePending { - channel_id: *channel_id, - counterparty_node_id: *counterparty_node_id, - user_channel_id: chan.context.get_user_id(), - new_funding_txo: splice_negotiated.funding_txo, - channel_type: splice_negotiated.channel_type, - new_funding_redeem_script: splice_negotiated - .funding_redeem_script, - }, - None, - )); - } - if chan.context.is_connected() { - if let Some(commitment_signed) = commitment_signed { - peer_state.pending_msg_events.push( - MessageSendEvent::UpdateHTLCs { - node_id: *counterparty_node_id, - channel_id: *channel_id, - updates: CommitmentUpdate { - commitment_signed: vec![commitment_signed], - update_add_htlcs: vec![], - update_fulfill_htlcs: vec![], - update_fail_htlcs: vec![], - update_fail_malformed_htlcs: vec![], - update_fee: None, - }, - }, + match peer_state.channel_by_id.entry(*channel_id) { + hash_map::Entry::Occupied(mut chan_entry) => { + match chan_entry.get_mut().as_funded_mut() { + Some(chan) => { + let txid = transaction.compute_txid(); + let witnesses: Vec<_> = transaction + .input + .into_iter() + .map(|input| input.witness) + .filter(|witness| !witness.is_empty()) + .collect(); + let best_block_height = self.best_block.read().unwrap().height; + + match chan.funding_transaction_signed( + txid, + witnesses, + best_block_height, + &self.logger, + ) { + Ok(FundingTxSigned { + commitment_signed, + tx_signatures, + funding_tx, + splice_negotiated, + splice_locked, + }) => { + if let Some(funding_tx) = funding_tx { + self.broadcast_interactive_funding( + chan, + &funding_tx, + &self.logger, ); } - if let Some(tx_signatures) = tx_signatures { - peer_state.pending_msg_events.push( - MessageSendEvent::SendTxSignatures { - node_id: *counterparty_node_id, - msg: tx_signatures, + if let Some(splice_negotiated) = splice_negotiated { + self.pending_events.lock().unwrap().push_back(( + events::Event::SplicePending { + channel_id: *channel_id, + counterparty_node_id: *counterparty_node_id, + user_channel_id: chan.context.get_user_id(), + new_funding_txo: splice_negotiated.funding_txo, + channel_type: splice_negotiated.channel_type, + new_funding_redeem_script: splice_negotiated + .funding_redeem_script, }, - ); + None, + )); } - if let Some(splice_locked) = splice_locked { - peer_state.pending_msg_events.push( - MessageSendEvent::SendSpliceLocked { - node_id: *counterparty_node_id, - msg: splice_locked, - }, - ); + + if chan.context.is_connected() { + if let Some(commitment_signed) = commitment_signed { + peer_state.pending_msg_events.push( + MessageSendEvent::UpdateHTLCs { + node_id: *counterparty_node_id, + channel_id: *channel_id, + updates: CommitmentUpdate { + commitment_signed: vec![commitment_signed], + update_add_htlcs: vec![], + update_fulfill_htlcs: vec![], + update_fail_htlcs: vec![], + update_fail_malformed_htlcs: vec![], + update_fee: None, + }, + }, + ); + } + if let Some(tx_signatures) = tx_signatures { + peer_state.pending_msg_events.push( + MessageSendEvent::SendTxSignatures { + node_id: *counterparty_node_id, + msg: tx_signatures, + }, + ); + } + if let Some(splice_locked) = splice_locked { + peer_state.pending_msg_events.push( + MessageSendEvent::SendSpliceLocked { + node_id: *counterparty_node_id, + msg: splice_locked, + }, + ); + } } - } - return NotifyOption::DoPersist; - }, - Err(err) => { - result = Err(err); - return NotifyOption::SkipPersistNoEvents; - }, - } - }, - None => { - result = Err(APIError::APIMisuseError { - err: format!( - "Channel with id {} not expecting funding signatures", - channel_id - ), - }); - return NotifyOption::SkipPersistNoEvents; - }, + + funding_tx_signed_result = Ok(()); + }, + Err(err) => { + funding_tx_signed_result = Err(err); + return NotifyOption::SkipPersistNoEvents; + }, + } + + match chan.maybe_process_pending_counterparty_initial_commitment_signed( + &self.fee_estimator, + &self.logger, + ) { + Ok(None) => {}, + Ok(Some(monitor_update)) => { + let funding_txo = chan.funding.get_funding_txo(); + if let Some(post_update_data) = self.handle_new_monitor_update( + &mut peer_state.in_flight_monitor_updates, + &mut peer_state.monitor_update_blocked_actions, + &mut peer_state.pending_msg_events, + peer_state.is_connected, + chan, + funding_txo.unwrap(), + monitor_update, + ) { + monitor_update_result = Some(Ok(post_update_data)); + } + }, + Err(err) => { + let (drop, err) = self.locked_handle_funded_force_close( + &mut peer_state.closed_channel_monitor_update_ids, + &mut peer_state.in_flight_monitor_updates, + err, + chan, + ); + if drop { + chan_entry.remove_entry(); + } + + monitor_update_result = Some(Err(err)); + }, + } + }, + None => { + funding_tx_signed_result = Err(APIError::APIMisuseError { + err: format!( + "Channel with id {} not expecting funding signatures", + channel_id + ), + }); + return NotifyOption::SkipPersistNoEvents; + }, + } }, - None => { - result = Err(APIError::ChannelUnavailable { + hash_map::Entry::Vacant(_) => { + funding_tx_signed_result = Err(APIError::ChannelUnavailable { err: format!( "Channel with id {} not found for the passed counterparty node_id {}", channel_id, counterparty_node_id @@ -6544,9 +6588,25 @@ where return NotifyOption::SkipPersistNoEvents; }, } + + mem::drop(peer_state_lock); + mem::drop(per_peer_state); + + if let Some(monitor_update_result) = monitor_update_result { + match monitor_update_result { + Ok(post_update_data) => { + self.handle_post_monitor_update_chan_resume(post_update_data); + }, + Err(_) => { + let _ = self.handle_error(monitor_update_result, *counterparty_node_id); + }, + } + } + + NotifyOption::DoPersist }); - result + funding_tx_signed_result } fn broadcast_interactive_funding( diff --git a/lightning/src/ln/splicing_tests.rs b/lightning/src/ln/splicing_tests.rs index c0ba401cfae..ef524db6be3 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -29,6 +29,7 @@ use crate::util::errors::APIError; use crate::util::ser::Writeable; use bitcoin::hashes::Hash; +use bitcoin::secp256k1::ecdsa::Signature; use bitcoin::secp256k1::PublicKey; use bitcoin::{Amount, OutPoint as BitcoinOutPoint, ScriptBuf, Transaction, TxOut, WPubkeyHash}; @@ -2221,3 +2222,233 @@ fn test_splice_with_inflight_htlc_forward_and_resolution() { do_test_splice_with_inflight_htlc_forward_and_resolution(true); do_test_splice_with_inflight_htlc_forward_and_resolution(false); } + +#[test] +fn test_splice_buffer_commitment_signed_until_funding_tx_signed() { + // Test that when the counterparty sends their initial `commitment_signed` before the user has + // called `funding_transaction_signed`, we buffer the message and process it at the end of + // `funding_transaction_signed`. This allows the user to cancel the splice negotiation if + // desired without having queued an irreversible monitor update. + 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); + + // Negotiate a splice-out where only the initiator (node 0) has a contribution. + // This means node 1 will send their commitment_signed immediately after tx_complete. + let initiator_contribution = SpliceContribution::splice_out(vec![TxOut { + value: Amount::from_sat(1_000), + script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), + }]); + negotiate_splice_tx(&nodes[0], &nodes[1], channel_id, initiator_contribution); + + // Node 0 (initiator with contribution) should have a signing event to handle. + let signing_event = get_event!(nodes[0], Event::FundingTransactionReadyForSigning); + + // Node 1 (acceptor with no contribution) won't have a signing event and will immediately + // send their initial commitment_signed. + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + let acceptor_commit_sig = get_htlc_update_msgs(&nodes[1], &node_id_0); + + // Deliver the acceptor's commitment_signed to the initiator BEFORE the initiator has called + // funding_transaction_signed. The message should be buffered, not processed. + nodes[0].node.handle_commitment_signed(node_id_1, &acceptor_commit_sig.commitment_signed[0]); + + // No monitor update should have happened since the message is buffered. + check_added_monitors(&nodes[0], 0); + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + + // Now handle the signing event and call `funding_transaction_signed`. + if let Event::FundingTransactionReadyForSigning { + channel_id: event_channel_id, + counterparty_node_id, + unsigned_transaction, + .. + } = signing_event + { + assert_eq!(event_channel_id, channel_id); + assert_eq!(counterparty_node_id, node_id_1); + + let partially_signed_tx = nodes[0].wallet_source.sign_tx(unsigned_transaction).unwrap(); + nodes[0] + .node + .funding_transaction_signed(&channel_id, &node_id_1, partially_signed_tx) + .unwrap(); + } else { + panic!("Expected FundingTransactionReadyForSigning event"); + } + + // After funding_transaction_signed: + // 1. The initiator should send their commitment_signed + // 2. The buffered commitment_signed from the acceptor should be processed (monitor update) + let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); + let initiator_commit_sig = + if let MessageSendEvent::UpdateHTLCs { ref updates, .. } = &msg_events[0] { + updates.commitment_signed[0].clone() + } else { + panic!("Expected UpdateHTLCs message"); + }; + + // The buffered commitment_signed should have been processed, resulting in a monitor update. + check_added_monitors(&nodes[0], 1); + + // Complete the rest of the flow normally. + nodes[1].node.handle_commitment_signed(node_id_0, &initiator_commit_sig); + let msg_events = nodes[1].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); + if let MessageSendEvent::SendTxSignatures { ref msg, .. } = &msg_events[0] { + nodes[0].node.handle_tx_signatures(node_id_1, msg); + } else { + panic!("Expected SendTxSignatures message"); + } + check_added_monitors(&nodes[1], 1); + + let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 1, "{msg_events:?}"); + if let MessageSendEvent::SendTxSignatures { ref msg, .. } = &msg_events[0] { + nodes[1].node.handle_tx_signatures(node_id_0, msg); + } else { + panic!("Expected SendTxSignatures message"); + } + + expect_splice_pending_event(&nodes[0], &node_id_1); + expect_splice_pending_event(&nodes[1], &node_id_0); + + // Both nodes should broadcast the splice transaction. + let splice_tx = { + let mut txn_0 = nodes[0].tx_broadcaster.txn_broadcast(); + assert_eq!(txn_0.len(), 1); + let txn_1 = nodes[1].tx_broadcaster.txn_broadcast(); + assert_eq!(txn_0, txn_1); + txn_0.remove(0) + }; + + // Verify the channel is operational by sending a payment. + send_payment(&nodes[0], &[&nodes[1]], 1_000_000); + + // Lock the splice by confirming the transaction. + mine_transaction(&nodes[0], &splice_tx); + mine_transaction(&nodes[1], &splice_tx); + lock_splice_after_blocks(&nodes[0], &nodes[1], ANTI_REORG_DELAY - 1); + + // Verify the channel is still operational by sending another payment. + send_payment(&nodes[0], &[&nodes[1]], 1_000_000); +} + +#[test] +fn test_splice_buffer_invalid_commitment_signed_closes_channel() { + // Test that when the counterparty sends an invalid `commitment_signed` (with a bad signature) + // before the user has called `funding_transaction_signed`, the channel is closed with an error + // when `ChannelManager::funding_transaction_signed` processes the buffered message. + 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); + + // Negotiate a splice-out where only the initiator (node 0) has a contribution. + // This means node 1 will send their commitment_signed immediately after tx_complete. + let initiator_contribution = SpliceContribution::splice_out(vec![TxOut { + value: Amount::from_sat(1_000), + script_pubkey: nodes[0].wallet_source.get_change_script().unwrap(), + }]); + negotiate_splice_tx(&nodes[0], &nodes[1], channel_id, initiator_contribution); + + // Node 0 (initiator with contribution) should have a signing event to handle. + let signing_event = get_event!(nodes[0], Event::FundingTransactionReadyForSigning); + + // Node 1 (acceptor with no contribution) won't have a signing event and will immediately + // send their initial commitment_signed. + assert!(nodes[1].node.get_and_clear_pending_events().is_empty()); + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + let mut acceptor_commit_sig = get_htlc_update_msgs(&nodes[1], &node_id_0); + + // Invalidate the signature by modifying one byte. This will cause signature verification + // to fail when the buffered message is processed. + let original_sig = acceptor_commit_sig.commitment_signed[0].signature; + let mut sig_bytes = original_sig.serialize_compact(); + sig_bytes[0] ^= 0x01; // Flip a bit to corrupt the signature + acceptor_commit_sig.commitment_signed[0].signature = + Signature::from_compact(&sig_bytes).unwrap(); + + // Deliver the acceptor's invalid commitment_signed to the initiator BEFORE the initiator has + // called funding_transaction_signed. The message should be buffered, not processed. + nodes[0].node.handle_commitment_signed(node_id_1, &acceptor_commit_sig.commitment_signed[0]); + + // No monitor update should have happened since the message is buffered. + check_added_monitors(&nodes[0], 0); + assert!(nodes[0].node.get_and_clear_pending_msg_events().is_empty()); + + // Now handle the signing event and call `funding_transaction_signed`. + // This should process the buffered invalid commitment_signed and close the channel. + if let Event::FundingTransactionReadyForSigning { + channel_id: event_channel_id, + counterparty_node_id, + unsigned_transaction, + .. + } = signing_event + { + assert_eq!(event_channel_id, channel_id); + assert_eq!(counterparty_node_id, node_id_1); + + let partially_signed_tx = nodes[0].wallet_source.sign_tx(unsigned_transaction).unwrap(); + nodes[0] + .node + .funding_transaction_signed(&channel_id, &node_id_1, partially_signed_tx) + .unwrap(); + } else { + panic!("Expected FundingTransactionReadyForSigning event"); + } + + // After funding_transaction_signed: + // 1. The initiator sends its commitment_signed (UpdateHTLCs message). + // 2. The buffered invalid commitment_signed from the acceptor is processed, causing the + // channel to close due to the invalid signature. + // We expect 3 message events: UpdateHTLCs, BroadcastChannelUpdate, and HandleError. + let msg_events = nodes[0].node.get_and_clear_pending_msg_events(); + assert_eq!(msg_events.len(), 3, "{msg_events:?}"); + match &msg_events[0] { + MessageSendEvent::UpdateHTLCs { ref updates, .. } => { + assert!(!updates.commitment_signed.is_empty()); + }, + _ => panic!("Expected UpdateHTLCs message, got {:?}", msg_events[0]), + } + match &msg_events[1] { + MessageSendEvent::HandleError { + action: msgs::ErrorAction::SendErrorMessage { ref msg }, + .. + } => { + assert!(msg.data.contains("Invalid commitment tx signature from peer")); + }, + _ => panic!("Expected HandleError with SendErrorMessage, got {:?}", msg_events[1]), + } + match &msg_events[2] { + MessageSendEvent::BroadcastChannelUpdate { ref msg, .. } => { + assert_eq!(msg.contents.channel_flags & 2, 2); + }, + _ => panic!("Expected BroadcastChannelUpdate, got {:?}", msg_events[2]), + } + + let err = "Invalid commitment tx signature from peer".to_owned(); + let reason = ClosureReason::ProcessingError { err }; + check_closed_events( + &nodes[0], + &[ExpectedCloseEvent::from_id_reason(channel_id, false, reason)], + ); + check_added_monitors(&nodes[0], 1); +} From 0fc923c7554c11ec7b1093345ee8b86a316189cb Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 22 Jan 2026 11:52:42 -0800 Subject: [PATCH 3/3] Support funding_transaction_signed for unfunded dual-funded channels Now that we require users to first call `ChannelManager::funding_transaction_signed` before releasing any signatures, it's possible that it is called before we receive the initial commitment signed from our counterparty, which would transition the channel to funded. Because of this, we need to support the API call while the channel is still in the unfunded phase. Note that this commit is mostly a code move of `FundedChannel::funding_transaction_signed` to `Channel::funding_transaction_signed` that doesn't alter the signing logic. --- lightning/src/ln/channel.rs | 279 ++++++++++++++++------------- lightning/src/ln/channelmanager.rs | 244 ++++++++++++------------- 2 files changed, 273 insertions(+), 250 deletions(-) diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index f0caba66fd5..69e91094dc9 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1953,11 +1953,18 @@ where let funding_tx_signed = if !has_local_contribution { let funding_txid = signing_session.unsigned_tx().tx().compute_txid(); - if let ChannelPhase::Funded(chan) = &mut self.phase { - chan.funding_transaction_signed(funding_txid, vec![], 0, logger).ok() - } else { - None - } + Some(self.funding_transaction_signed(funding_txid, vec![], 0, logger).map_err( + |err| { + log_error!( + logger, + "Failed signing funding transaction without local contribution: {err:?}" + ); + self.fail_interactive_tx_negotiation( + AbortReason::InternalError("Signing failed"), + logger, + ) + }, + )?) } else { None }; @@ -2143,6 +2150,146 @@ where Ok(()) } + pub fn funding_transaction_signed( + &mut self, funding_txid_signed: Txid, witnesses: Vec, best_block_height: u32, + logger: &L, + ) -> Result + where + L::Target: Logger, + { + let (context, funding, pending_splice) = match &mut self.phase { + ChannelPhase::Undefined => unreachable!(), + ChannelPhase::UnfundedV2(channel) => (&mut channel.context, &channel.funding, None), + ChannelPhase::Funded(channel) => { + (&mut channel.context, &channel.funding, channel.pending_splice.as_ref()) + }, + _ => { + return Err(APIError::APIMisuseError { + err: format!( + "Channel with id {} not expecting funding signatures", + self.context().channel_id + ), + }); + }, + }; + + let signing_session = if let Some(signing_session) = + context.interactive_tx_signing_session.as_mut() + { + if let Some(pending_splice) = pending_splice.as_ref() { + debug_assert!(pending_splice + .funding_negotiation + .as_ref() + .map(|funding_negotiation| matches!( + funding_negotiation, + FundingNegotiation::AwaitingSignatures { .. } + )) + .unwrap_or(false)); + } + + if signing_session.holder_tx_signatures().is_some() { + // Our `tx_signatures` either should've been the first time we processed them, + // or we're waiting for our counterparty to send theirs first. + return Ok(FundingTxSigned { + commitment_signed: None, + + tx_signatures: None, + funding_tx: None, + splice_negotiated: None, + splice_locked: None, + }); + } + + signing_session + } else { + if Some(funding_txid_signed) == funding.get_funding_txid() { + // We may be handling a duplicate call and the funding was already locked so we + // no longer have the signing session present. + return Ok(FundingTxSigned { + commitment_signed: None, + tx_signatures: None, + funding_tx: None, + splice_negotiated: None, + splice_locked: None, + }); + } + let err = format!("Channel {} not expecting funding signatures", context.channel_id); + return Err(APIError::APIMisuseError { err }); + }; + + let tx = signing_session.unsigned_tx().tx(); + if funding_txid_signed != tx.compute_txid() { + return Err(APIError::APIMisuseError { + err: "Transaction was malleated prior to signing".to_owned(), + }); + } + + let shared_input_signature = + if let Some(splice_input_index) = signing_session.unsigned_tx().shared_input_index() { + let sig = match &context.holder_signer { + ChannelSignerType::Ecdsa(signer) => signer.sign_splice_shared_input( + &funding.channel_transaction_parameters, + tx, + splice_input_index as usize, + &context.secp_ctx, + ), + #[cfg(taproot)] + ChannelSignerType::Taproot(_) => todo!(), + }; + Some(sig) + } else { + None + }; + debug_assert_eq!(pending_splice.is_some(), shared_input_signature.is_some()); + + let tx_signatures = msgs::TxSignatures { + channel_id: context.channel_id, + tx_hash: funding_txid_signed, + witnesses, + shared_input_signature, + }; + let (tx_signatures, funding_tx) = signing_session + .provide_holder_witnesses(tx_signatures, &context.secp_ctx) + .map_err(|err| APIError::APIMisuseError { err })?; + + let logger = WithChannelContext::from(logger, &context, None); + if tx_signatures.is_some() { + log_info!( + logger, + "Sending tx_signatures for interactive funding transaction {funding_txid_signed}" + ); + } + + let funding = pending_splice + .as_ref() + .and_then(|pending_splice| pending_splice.funding_negotiation.as_ref()) + .and_then(|funding_negotiation| funding_negotiation.as_funding()) + .unwrap_or(funding); + let commitment_signed = context.get_initial_commitment_signed_v2(funding, &&logger); + + // In the common case for zero conf channels, we don't expect the funding transaction to be + // ready for broadcast yet as our counterparty shouldn't have sent their `tx_signatures` + // without us having sent our initial commitment signed to them first. However, in the event + // they do, we choose to handle it anyway. + let (splice_negotiated, splice_locked) = if let Some(funding_tx) = funding_tx.clone() { + debug_assert!(tx_signatures.is_some()); + let funded_channel = self.as_funded_mut().expect( + "Funding transactions ready for broadcast can only exist for funded channels", + ); + funded_channel.on_tx_signatures_exchange(funding_tx, best_block_height, &logger) + } else { + (None, None) + }; + + Ok(FundingTxSigned { + commitment_signed, + tx_signatures, + funding_tx, + splice_negotiated, + splice_locked, + }) + } + pub fn force_shutdown(&mut self, closure_reason: ClosureReason) -> ShutdownResult { let (funding, context) = self.funding_and_context_mut(); context.force_shutdown(funding, closure_reason) @@ -2212,7 +2359,7 @@ where .unwrap_or(false); // We delay processing this until the user manually approves the splice via - // [`FundedChannel::funding_transaction_signed`], as otherwise, there would be a + // [`Channel::funding_transaction_signed`], as otherwise, there would be a // [`ChannelMonitorUpdateStep::RenegotiatedFunding`] committed that we would // need to undo if they no longer wish to proceed. if has_holder_tx_signatures { @@ -2716,7 +2863,7 @@ enum FundingNegotiation { is_initiator: bool, /// The initial [`msgs::CommitmentSigned`] message received for the [`FundingScope`] above. /// We delay processing this until the user manually approves the splice via - /// [`FundedChannel::funding_transaction_signed`], as otherwise, there would be a + /// [`Channel::funding_transaction_signed`], as otherwise, there would be a /// [`ChannelMonitorUpdateStep::RenegotiatedFunding`] committed that we would need to undo /// if they no longer wish to proceed. /// @@ -9162,124 +9309,6 @@ where } } - pub fn funding_transaction_signed( - &mut self, funding_txid_signed: Txid, witnesses: Vec, best_block_height: u32, - logger: &L, - ) -> Result - where - L::Target: Logger, - { - let signing_session = - if let Some(signing_session) = self.context.interactive_tx_signing_session.as_mut() { - if let Some(pending_splice) = self.pending_splice.as_ref() { - debug_assert!(pending_splice - .funding_negotiation - .as_ref() - .map(|funding_negotiation| matches!( - funding_negotiation, - FundingNegotiation::AwaitingSignatures { .. } - )) - .unwrap_or(false)); - } - - if signing_session.holder_tx_signatures().is_some() { - // Our `tx_signatures` either should've been the first time we processed them, - // or we're waiting for our counterparty to send theirs first. - return Ok(FundingTxSigned { - commitment_signed: None, - - tx_signatures: None, - funding_tx: None, - splice_negotiated: None, - splice_locked: None, - }); - } - - signing_session - } else { - if Some(funding_txid_signed) == self.funding.get_funding_txid() { - // We may be handling a duplicate call and the funding was already locked so we - // no longer have the signing session present. - return Ok(FundingTxSigned { - commitment_signed: None, - tx_signatures: None, - funding_tx: None, - splice_negotiated: None, - splice_locked: None, - }); - } - let err = - format!("Channel {} not expecting funding signatures", self.context.channel_id); - return Err(APIError::APIMisuseError { err }); - }; - - let tx = signing_session.unsigned_tx().tx(); - if funding_txid_signed != tx.compute_txid() { - return Err(APIError::APIMisuseError { - err: "Transaction was malleated prior to signing".to_owned(), - }); - } - - let shared_input_signature = - if let Some(splice_input_index) = signing_session.unsigned_tx().shared_input_index() { - let sig = match &self.context.holder_signer { - ChannelSignerType::Ecdsa(signer) => signer.sign_splice_shared_input( - &self.funding.channel_transaction_parameters, - tx, - splice_input_index as usize, - &self.context.secp_ctx, - ), - #[cfg(taproot)] - ChannelSignerType::Taproot(_) => todo!(), - }; - Some(sig) - } else { - None - }; - debug_assert_eq!(self.pending_splice.is_some(), shared_input_signature.is_some()); - - let tx_signatures = msgs::TxSignatures { - channel_id: self.context.channel_id, - tx_hash: funding_txid_signed, - witnesses, - shared_input_signature, - }; - let (tx_signatures, funding_tx) = signing_session - .provide_holder_witnesses(tx_signatures, &self.context.secp_ctx) - .map_err(|err| APIError::APIMisuseError { err })?; - - let logger = WithChannelContext::from(logger, &self.context, None); - if tx_signatures.is_some() { - log_info!( - logger, - "Sending tx_signatures for interactive funding transaction {funding_txid_signed}" - ); - } - - let (splice_negotiated, splice_locked) = if let Some(funding_tx) = funding_tx.clone() { - debug_assert!(tx_signatures.is_some()); - self.on_tx_signatures_exchange(funding_tx, best_block_height, &logger) - } else { - (None, None) - }; - - let funding = self - .pending_splice - .as_ref() - .and_then(|pending_splice| pending_splice.funding_negotiation.as_ref()) - .and_then(|funding_negotiation| funding_negotiation.as_funding()) - .unwrap_or(&self.funding); - let commitment_signed = self.context.get_initial_commitment_signed_v2(funding, &&logger); - - Ok(FundingTxSigned { - commitment_signed, - tx_signatures, - funding_tx, - splice_negotiated, - splice_locked, - }) - } - pub fn tx_signatures( &mut self, msg: &msgs::TxSignatures, best_block_height: u32, logger: &L, ) -> Result diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index a10e0fedbe9..d1d0d286c60 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -6444,139 +6444,133 @@ where match peer_state.channel_by_id.entry(*channel_id) { hash_map::Entry::Occupied(mut chan_entry) => { - match chan_entry.get_mut().as_funded_mut() { - Some(chan) => { - let txid = transaction.compute_txid(); - let witnesses: Vec<_> = transaction - .input - .into_iter() - .map(|input| input.witness) - .filter(|witness| !witness.is_empty()) - .collect(); - let best_block_height = self.best_block.read().unwrap().height; - - match chan.funding_transaction_signed( - txid, - witnesses, - best_block_height, - &self.logger, - ) { - Ok(FundingTxSigned { - commitment_signed, - tx_signatures, - funding_tx, - splice_negotiated, - splice_locked, - }) => { - if let Some(funding_tx) = funding_tx { - self.broadcast_interactive_funding( - chan, - &funding_tx, - &self.logger, - ); - } - if let Some(splice_negotiated) = splice_negotiated { - self.pending_events.lock().unwrap().push_back(( - events::Event::SplicePending { - channel_id: *channel_id, - counterparty_node_id: *counterparty_node_id, - user_channel_id: chan.context.get_user_id(), - new_funding_txo: splice_negotiated.funding_txo, - channel_type: splice_negotiated.channel_type, - new_funding_redeem_script: splice_negotiated - .funding_redeem_script, - }, - None, - )); - } - - if chan.context.is_connected() { - if let Some(commitment_signed) = commitment_signed { - peer_state.pending_msg_events.push( - MessageSendEvent::UpdateHTLCs { - node_id: *counterparty_node_id, - channel_id: *channel_id, - updates: CommitmentUpdate { - commitment_signed: vec![commitment_signed], - update_add_htlcs: vec![], - update_fulfill_htlcs: vec![], - update_fail_htlcs: vec![], - update_fail_malformed_htlcs: vec![], - update_fee: None, - }, - }, - ); - } - if let Some(tx_signatures) = tx_signatures { - peer_state.pending_msg_events.push( - MessageSendEvent::SendTxSignatures { - node_id: *counterparty_node_id, - msg: tx_signatures, - }, - ); - } - if let Some(splice_locked) = splice_locked { - peer_state.pending_msg_events.push( - MessageSendEvent::SendSpliceLocked { - node_id: *counterparty_node_id, - msg: splice_locked, - }, - ); - } - } - - funding_tx_signed_result = Ok(()); - }, - Err(err) => { - funding_tx_signed_result = Err(err); - return NotifyOption::SkipPersistNoEvents; - }, + let txid = transaction.compute_txid(); + let witnesses: Vec<_> = transaction + .input + .into_iter() + .map(|input| input.witness) + .filter(|witness| !witness.is_empty()) + .collect(); + let best_block_height = self.best_block.read().unwrap().height; + + let chan = chan_entry.get_mut(); + match chan.funding_transaction_signed( + txid, + witnesses, + best_block_height, + &self.logger, + ) { + Ok(FundingTxSigned { + commitment_signed, + tx_signatures, + funding_tx, + splice_negotiated, + splice_locked, + }) => { + if let Some(funding_tx) = funding_tx { + let funded_chan = chan.as_funded_mut().expect( + "Funding transactions ready for broadcast can only exist for funded channels", + ); + self.broadcast_interactive_funding( + funded_chan, + &funding_tx, + &self.logger, + ); + } + if let Some(splice_negotiated) = splice_negotiated { + self.pending_events.lock().unwrap().push_back(( + events::Event::SplicePending { + channel_id: *channel_id, + counterparty_node_id: *counterparty_node_id, + user_channel_id: chan.context().get_user_id(), + new_funding_txo: splice_negotiated.funding_txo, + channel_type: splice_negotiated.channel_type, + new_funding_redeem_script: splice_negotiated + .funding_redeem_script, + }, + None, + )); } - match chan.maybe_process_pending_counterparty_initial_commitment_signed( - &self.fee_estimator, - &self.logger, - ) { - Ok(None) => {}, - Ok(Some(monitor_update)) => { - let funding_txo = chan.funding.get_funding_txo(); - if let Some(post_update_data) = self.handle_new_monitor_update( - &mut peer_state.in_flight_monitor_updates, - &mut peer_state.monitor_update_blocked_actions, - &mut peer_state.pending_msg_events, - peer_state.is_connected, - chan, - funding_txo.unwrap(), - monitor_update, - ) { - monitor_update_result = Some(Ok(post_update_data)); - } - }, - Err(err) => { - let (drop, err) = self.locked_handle_funded_force_close( - &mut peer_state.closed_channel_monitor_update_ids, - &mut peer_state.in_flight_monitor_updates, - err, - chan, + if chan.context().is_connected() { + if let Some(commitment_signed) = commitment_signed { + peer_state.pending_msg_events.push( + MessageSendEvent::UpdateHTLCs { + node_id: *counterparty_node_id, + channel_id: *channel_id, + updates: CommitmentUpdate { + commitment_signed: vec![commitment_signed], + update_add_htlcs: vec![], + update_fulfill_htlcs: vec![], + update_fail_htlcs: vec![], + update_fail_malformed_htlcs: vec![], + update_fee: None, + }, + }, ); - if drop { - chan_entry.remove_entry(); - } - - monitor_update_result = Some(Err(err)); - }, + } + if let Some(tx_signatures) = tx_signatures { + peer_state.pending_msg_events.push( + MessageSendEvent::SendTxSignatures { + node_id: *counterparty_node_id, + msg: tx_signatures, + }, + ); + } + if let Some(splice_locked) = splice_locked { + peer_state.pending_msg_events.push( + MessageSendEvent::SendSpliceLocked { + node_id: *counterparty_node_id, + msg: splice_locked, + }, + ); + } } + + funding_tx_signed_result = Ok(()); }, - None => { - funding_tx_signed_result = Err(APIError::APIMisuseError { - err: format!( - "Channel with id {} not expecting funding signatures", - channel_id - ), - }); + Err(err) => { + funding_tx_signed_result = Err(err); return NotifyOption::SkipPersistNoEvents; }, } + + if let Some(funded_chan) = chan.as_funded_mut() { + match funded_chan + .maybe_process_pending_counterparty_initial_commitment_signed( + &self.fee_estimator, + &self.logger, + ) { + Ok(None) => {}, + Ok(Some(monitor_update)) => { + let funding_txo = funded_chan.funding.get_funding_txo(); + if let Some(post_update_data) = self.handle_new_monitor_update( + &mut peer_state.in_flight_monitor_updates, + &mut peer_state.monitor_update_blocked_actions, + &mut peer_state.pending_msg_events, + peer_state.is_connected, + funded_chan, + funding_txo.unwrap(), + monitor_update, + ) { + monitor_update_result = Some(Ok(post_update_data)); + } + }, + Err(err) => { + let (drop, err) = self.locked_handle_funded_force_close( + &mut peer_state.closed_channel_monitor_update_ids, + &mut peer_state.in_flight_monitor_updates, + err, + funded_chan, + ); + if drop { + chan_entry.remove_entry(); + } + + monitor_update_result = Some(Err(err)); + }, + } + } }, hash_map::Entry::Vacant(_) => { funding_tx_signed_result = Err(APIError::ChannelUnavailable {