diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index fd780da8d91..c163b81a1cd 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1902,13 +1902,11 @@ where } } - pub fn tx_complete( - &mut self, msg: &msgs::TxComplete, logger: &L, - ) -> Result< - (Option, Option), - (ChannelError, Option), - > + pub fn tx_complete( + &mut self, msg: &msgs::TxComplete, fee_estimator: &LowerBoundedFeeEstimator, logger: &L, + ) -> Result)> where + F::Target: FeeEstimator, L::Target: Logger, { let tx_complete_action = match self.interactive_tx_constructor_mut() { @@ -1934,13 +1932,46 @@ 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(); + Some( + self.funding_transaction_signed(funding_txid, vec![], 0, fee_estimator, 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 + }; + + Ok(TxCompleteResult { interactive_tx_msg_send, event_unsigned_tx, funding_tx_signed }) } pub fn tx_abort( @@ -2046,14 +2077,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,77 +2097,35 @@ 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() { - pending_splice - .funding_negotiation - .take() - .and_then(|funding_negotiation| { - if let FundingNegotiation::ConstructingTransaction { + let funding_negotiation = pending_splice.funding_negotiation.take(); + if let Some(FundingNegotiation::ConstructingTransaction { + mut funding, + interactive_tx_constructor, + }) = funding_negotiation + { + let is_initiator = interactive_tx_constructor.is_initiator(); + funding.channel_transaction_parameters.funding_outpoint = + Some(funding_outpoint); + pending_splice.funding_negotiation = + Some(FundingNegotiation::AwaitingSignatures { + is_initiator, funding, - interactive_tx_constructor, - } = funding_negotiation - { - let is_initiator = interactive_tx_constructor.is_initiator(); - Some((is_initiator, funding, interactive_tx_constructor)) - } else { - // Replace the taken state for later error handling - pending_splice.funding_negotiation = Some(funding_negotiation); - None - } - }) - .ok_or_else(|| { - 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", - )) - }, - } - })? + initial_commitment_signed_from_counterparty: None, + }); + interactive_tx_constructor + } else { + // Replace the taken state for later error handling + pending_splice.funding_negotiation = funding_negotiation; + return Err(AbortReason::InternalError( + "Got a tx_complete message in an invalid state", + )); + } } else { return Err(AbortReason::InternalError( "Got a tx_complete message in an invalid state", @@ -2159,7 +2142,181 @@ 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 funding_transaction_signed( + &mut self, funding_txid_signed: Txid, witnesses: Vec, best_block_height: u32, + fee_estimator: &LowerBoundedFeeEstimator, logger: &L, + ) -> Result + where + F::Target: FeeEstimator, + 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, + counterparty_initial_commitment_signed_result: 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, + counterparty_initial_commitment_signed_result: 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) + }; + + // If we have a pending splice with a buffered initial commitment signed from our + // counterparty, process it now that we have provided our signatures. + let counterparty_initial_commitment_signed_result = + if let Some(funded_channel) = self.as_funded_mut() { + if let Some(commit_sig) = funded_channel + .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 + } + }) { + Some(funded_channel.splice_initial_commitment_signed( + &commit_sig, + fee_estimator, + &&logger, + )) + } else { + None + } + } else { + None + }; + + Ok(FundingTxSigned { + commitment_signed, + counterparty_initial_commitment_signed_result, + tx_signatures, + funding_tx, + splice_negotiated, + splice_locked, + }) } pub fn force_shutdown(&mut self, closure_reason: ClosureReason) -> ShutdownResult { @@ -2223,9 +2380,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 + // [`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 { + 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)) @@ -2709,6 +2890,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 + /// [`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. + /// + /// 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, }, } @@ -2716,6 +2908,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 ); @@ -2911,6 +3104,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 +3836,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 +4076,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 +7056,30 @@ type BestBlockUpdatedRes = ( Option, ); +/// The result of handling a `tx_complete` message during interactive transaction construction. +pub(super) 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 { +pub(super) struct FundingTxSigned { + /// The initial `commitment_signed` message to send to the counterparty, if necessary. + pub commitment_signed: Option, + + /// The result of processing a buffered initial commitment signed from our counterparty, + /// if any. + pub counterparty_initial_commitment_signed_result: + Option, ChannelError>>, + /// Signatures that should be sent to the counterparty, if necessary. pub tx_signatures: Option, @@ -8051,6 +8269,7 @@ where Vec::new(), logger, ); + self.context.monitor_pending_tx_signatures = true; Ok(self.push_ret_blockable_mon_update(monitor_update)) } @@ -9080,107 +9299,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 { - 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 { - 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) - }; - - Ok(FundingTxSigned { tx_signatures, funding_tx, splice_negotiated, splice_locked }) - } - pub fn tx_signatures( &mut self, msg: &msgs::TxSignatures, best_block_height: u32, logger: &L, ) -> Result @@ -9246,6 +9364,8 @@ where }; Ok(FundingTxSigned { + commitment_signed: None, + counterparty_initial_commitment_signed_result: None, tx_signatures: holder_tx_signatures, funding_tx, splice_negotiated, @@ -9451,6 +9571,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 +9678,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 +15213,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 +15235,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 +15667,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 +15682,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 +16095,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..23e94e76f1c 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -6424,69 +6424,101 @@ 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 { - tx_signatures: Some(tx_signatures), - funding_tx, - splice_negotiated, - splice_locked, - }) => { - if let Some(funding_tx) = funding_tx { - self.broadcast_interactive_funding( - chan, - &funding_tx, - &self.logger, + match peer_state.channel_by_id.entry(*channel_id) { + hash_map::Entry::Occupied(mut chan_entry) => { + 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.fee_estimator, + &self.logger, + ) { + Ok(FundingTxSigned { + commitment_signed, + counterparty_initial_commitment_signed_result, + 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, + )); + } + + 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(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, + if let Some(tx_signatures) = tx_signatures { + peer_state.pending_msg_events.push( + MessageSendEvent::SendTxSignatures { + node_id: *counterparty_node_id, + msg: tx_signatures, }, - 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 { @@ -6495,37 +6527,52 @@ where }, ); } - return NotifyOption::DoPersist; - }, - Err(err) => { - 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 => { - result = Err(APIError::APIMisuseError { - err: format!( - "Channel with id {} not expecting funding signatures", - channel_id - ), - }); - return NotifyOption::SkipPersistNoEvents; - }, + } + + if let Some(funded_chan) = chan.as_funded_mut() { + match counterparty_initial_commitment_signed_result { + Some(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)); + } + }, + Some(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)); + }, + Some(Ok(None)) | None => {}, + } + } + + funding_tx_signed_result = Ok(()); + }, + Err(err) => { + funding_tx_signed_result = Err(err); + 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 @@ -6534,9 +6581,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( @@ -10151,79 +10214,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); @@ -11205,31 +11195,74 @@ This indicates a bug inside LDK. Please report this error at https://github.com/ match peer_state.channel_by_id.entry(msg.channel_id) { hash_map::Entry::Occupied(mut chan_entry) => { let 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 { + match chan.tx_complete(msg, &self.fee_estimator, &self.logger) { + 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, + counterparty_initial_commitment_signed_result, + 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. Similarly, we + // shouldn't have a result for the counterparty's initial commitment + // signed as they haven't sent it yet. + debug_assert!(funding_tx.is_none()); + debug_assert!(splice_negotiated.is_none()); + debug_assert!(splice_locked.is_none()); + debug_assert!(counterparty_initial_commitment_signed_result.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 +11307,8 @@ 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, + counterparty_initial_commitment_signed_result, tx_signatures, funding_tx, splice_negotiated, @@ -11284,6 +11319,12 @@ 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()); + debug_assert!(counterparty_initial_commitment_signed_result.is_none()); + if let Some(tx_signatures) = tx_signatures { peer_state.pending_msg_events.push(MessageSendEvent::SendTxSignatures { node_id: *counterparty_node_id, @@ -19013,6 +19054,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..ef524db6be3 100644 --- a/lightning/src/ln/splicing_tests.rs +++ b/lightning/src/ln/splicing_tests.rs @@ -27,9 +27,9 @@ 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::ecdsa::Signature; use bitcoin::secp256k1::PublicKey; use bitcoin::{Amount, OutPoint as BitcoinOutPoint, ScriptBuf, Transaction, TxOut, WPubkeyHash}; @@ -132,7 +132,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 +141,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 +184,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 +240,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 +264,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 +312,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 +352,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 +645,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 +672,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 +1293,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 +1323,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 +1338,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 +1358,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 +1382,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 +1393,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 +1643,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 +1782,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 +1827,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 +1844,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 +1907,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] @@ -2277,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); +}