diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index e44b9eb5bce..7659a99cfba 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -476,7 +476,7 @@ enum HTLCUpdateAwaitingACK { } macro_rules! define_state_flags { - ($flag_type_doc: expr, $flag_type: ident, [$(($flag_doc: expr, $flag: ident, $value: expr, $get: ident, $set: ident, $clear: ident)),+], $extra_flags: expr) => { + ($flag_type_doc: expr, $flag_type: ident, [$(($flag_doc: expr, $flag: ident, $value: expr, $get: ident, $set: ident, $clear: ident)),*], $extra_flags: expr) => { #[doc = $flag_type_doc] #[derive(Copy, Clone, Debug, PartialEq, PartialOrd, Eq)] struct $flag_type(u32); @@ -601,9 +601,6 @@ mod state_flags { pub const LOCAL_STFU_SENT: u32 = 1 << 15; pub const REMOTE_STFU_SENT: u32 = 1 << 16; pub const QUIESCENT: u32 = 1 << 17; - pub const INTERACTIVE_SIGNING: u32 = 1 << 18; - pub const OUR_TX_SIGNATURES_READY: u32 = 1 << 19; - pub const THEIR_TX_SIGNATURES_SENT: u32 = 1 << 20; } define_state_flags!( @@ -638,17 +635,9 @@ define_state_flags!( define_state_flags!( "Flags that only apply to [`ChannelState::FundingNegotiated`].", - FUNDED_STATE, FundingNegotiatedFlags, [ - ("Indicates we have an active interactive signing session for an interactive transaction", - INTERACTIVE_SIGNING, state_flags::INTERACTIVE_SIGNING, - is_interactive_signing, set_interactive_signing, clear_interactive_signing), - ("Indicates they sent us a `tx_signatures` message.", - THEIR_TX_SIGNATURES_SENT, state_flags::THEIR_TX_SIGNATURES_SENT, - is_their_tx_signatures_sent, set_their_tx_signatures_sent, clear_their_tx_signatures_sent), - ("Indicates we are ready to send them a `tx_signatures` message and it has been queued to send.", - OUR_TX_SIGNATURES_READY, state_flags::OUR_TX_SIGNATURES_READY, - is_our_tx_signatures_ready, set_our_tx_signatures_ready, clear_our_tx_signatures_ready) - ] + FUNDED_STATE, + FundingNegotiatedFlags, + [] ); define_state_flags!( @@ -710,9 +699,9 @@ enum ChannelState { /// `AwaitingChannelReady`. Note that this is nonsense for an inbound channel as we immediately generate /// `funding_signed` upon receipt of `funding_created`, so simply skip this state. /// - /// For inbound and outbound interactively funded channels (dual-funding/splicing), this flag indicates + /// For inbound and outbound interactively funded channels (dual-funding/splicing), this state indicates /// that interactive transaction construction has been completed and we are now interactively signing - /// the funding/splice transaction. + /// the initial funding transaction. FundingNegotiated(FundingNegotiatedFlags), /// We've received/sent `funding_created` and `funding_signed` and are thus now waiting on the /// funding transaction to confirm. @@ -799,14 +788,6 @@ impl ChannelState { } } - fn can_resume_on_reconnect(&self) -> bool { - match self { - ChannelState::NegotiatingFunding(_) => false, - ChannelState::FundingNegotiated(flags) => flags.is_interactive_signing(), - _ => true, - } - } - fn is_both_sides_shutdown(&self) -> bool { self.is_local_shutdown_sent() && self.is_remote_shutdown_sent() } @@ -863,24 +844,6 @@ impl ChannelState { clear_remote_shutdown_sent, FUNDED_STATES ); - impl_state_flag!( - is_interactive_signing, - set_interactive_signing, - clear_interactive_signing, - FundingNegotiated - ); - impl_state_flag!( - is_our_tx_signatures_ready, - set_our_tx_signatures_ready, - clear_our_tx_signatures_ready, - FundingNegotiated - ); - impl_state_flag!( - is_their_tx_signatures_sent, - set_their_tx_signatures_sent, - clear_their_tx_signatures_sent, - FundingNegotiated - ); impl_state_flag!( is_our_channel_ready, set_our_channel_ready, @@ -1950,21 +1913,27 @@ where let logger = WithChannelContext::from(logger, self.context(), None); match &mut self.phase { ChannelPhase::UnfundedV2(chan) => { - let mut signing_session = chan + debug_assert_eq!( + chan.context.channel_state, + ChannelState::NegotiatingFunding( + NegotiatingFundingFlags::OUR_INIT_SENT + | NegotiatingFundingFlags::THEIR_INIT_SENT + ), + ); + + let signing_session = chan .interactive_tx_constructor .take() .expect("PendingV2Channel::interactive_tx_constructor should be set") .into_signing_session(); let commitment_signed = chan.context.funding_tx_constructed( &mut chan.funding, - &mut signing_session, + signing_session, false, chan.unfunded_context.transaction_number(), &&logger, )?; - chan.interactive_tx_signing_session = Some(signing_session); - return Ok(commitment_signed); }, ChannelPhase::Funded(chan) => { @@ -1975,17 +1944,15 @@ where interactive_tx_constructor, ) = funding_negotiation { - let mut signing_session = - interactive_tx_constructor.into_signing_session(); + let signing_session = interactive_tx_constructor.into_signing_session(); let commitment_signed = chan.context.funding_tx_constructed( &mut funding, - &mut signing_session, + signing_session, true, chan.holder_commitment_point.next_transaction_number(), &&logger, )?; - chan.interactive_tx_signing_session = Some(signing_session); pending_splice.funding_negotiation = Some(FundingNegotiation::AwaitingSignatures(funding)); @@ -2039,7 +2006,6 @@ where funding: chan.funding, pending_funding: vec![], context: chan.context, - interactive_tx_signing_session: chan.interactive_tx_signing_session, holder_commitment_point, pending_splice: None, quiescent_action: None, @@ -2064,6 +2030,7 @@ where .map(|funding_negotiation| funding_negotiation.as_funding().is_some()) .unwrap_or(false); let session_received_commitment_signed = funded_channel + .context .interactive_tx_signing_session .as_ref() .map(|session| session.has_received_commitment_signed()) @@ -2945,6 +2912,15 @@ where /// If we can't release a [`ChannelMonitorUpdate`] until some external action completes, we /// store it here and only release it to the `ChannelManager` once it asks for it. blocked_monitor_updates: Vec, + + /// The signing session for the current interactive tx construction, if any. + /// + /// This is populated when the interactive tx construction phase completes (i.e., upon receiving + /// a consecutive `tx_complete`) and the channel enters the signing phase. + /// + /// This field is cleared once our counterparty sends a `channel_ready` or upon splice funding + /// promotion. + pub interactive_tx_signing_session: Option, } /// A channel struct implementing this trait can receive an initial counterparty commitment @@ -3519,6 +3495,8 @@ where blocked_monitor_updates: Vec::new(), is_manual_broadcast: false, + + interactive_tx_signing_session: None, }; Ok((funding, channel_context)) @@ -3755,6 +3733,8 @@ where blocked_monitor_updates: Vec::new(), local_initiated_shutdown: None, is_manual_broadcast: false, + + interactive_tx_signing_session: None, }; Ok((funding, channel_context)) @@ -4270,13 +4250,21 @@ where self.is_manual_broadcast = true; } + fn can_resume_on_reconnect(&self) -> bool { + match self.channel_state { + ChannelState::NegotiatingFunding(_) => false, + ChannelState::FundingNegotiated(_) => self.interactive_tx_signing_session.is_some(), + _ => true, + } + } + /// Returns true if this channel can be resume after a restart, implying its past the initial /// funding negotiation stages (and any assocated batch channels are similarly past initial /// funding negotiation). /// /// This is equivalent to saying the channel can be persisted to disk. pub fn can_resume_on_restart(&self) -> bool { - self.channel_state.can_resume_on_reconnect() + self.can_resume_on_reconnect() && match self.channel_state { ChannelState::AwaitingChannelReady(flags) => !flags.is_waiting_for_batch(), _ => true, @@ -4288,7 +4276,11 @@ where fn is_funding_broadcastable(&self) -> bool { match self.channel_state { ChannelState::NegotiatingFunding(_) => false, - ChannelState::FundingNegotiated(flags) => !flags.is_our_tx_signatures_ready(), + ChannelState::FundingNegotiated(_) => self + .interactive_tx_signing_session + .as_ref() + .map(|signing_session| signing_session.holder_tx_signatures().is_some()) + .unwrap_or(false), ChannelState::AwaitingChannelReady(flags) => !flags.is_waiting_for_batch(), _ => true, } @@ -4296,10 +4288,6 @@ where #[rustfmt::skip] fn unset_funding_info(&mut self, funding: &mut FundingScope) { - debug_assert!( - matches!(self.channel_state, ChannelState::FundingNegotiated(flags) if !flags.is_their_tx_signatures_sent() && !flags.is_our_tx_signatures_ready()) - || matches!(self.channel_state, ChannelState::AwaitingChannelReady(_)) - ); funding.channel_transaction_parameters.funding_outpoint = None; self.channel_id = self.temporary_channel_id.expect( "temporary_channel_id should be set since unset_funding_info is only called on funded \ @@ -6064,7 +6052,7 @@ where #[rustfmt::skip] fn funding_tx_constructed( - &mut self, funding: &mut FundingScope, signing_session: &mut InteractiveTxSigningSession, + &mut self, funding: &mut FundingScope, signing_session: InteractiveTxSigningSession, is_splice: bool, holder_commitment_transaction_number: u64, logger: &L ) -> Result where @@ -6087,9 +6075,7 @@ where }; funding .channel_transaction_parameters.funding_outpoint = Some(outpoint); - - self.channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()); - self.channel_state.set_interactive_signing(); + self.interactive_tx_signing_session = Some(signing_session); if is_splice { debug_assert_eq!( @@ -6100,6 +6086,7 @@ where return Err(AbortReason::InternalError("Splicing not yet supported")); } else { self.assert_no_commitment_advancement(holder_commitment_transaction_number, "initial commitment_signed"); + self.channel_state = ChannelState::FundingNegotiated(FundingNegotiatedFlags::new()); } let commitment_signed = self.get_initial_commitment_signed_v2(&funding, logger); @@ -6178,15 +6165,13 @@ where } fn get_initial_commitment_signed_v2( - &mut self, funding: &FundingScope, logger: &L, + &self, funding: &FundingScope, logger: &L, ) -> Option where SP::Target: SignerProvider, L::Target: Logger, { - assert!( - matches!(self.channel_state, ChannelState::FundingNegotiated(flags) if flags.is_interactive_signing()) - ); + debug_assert!(self.interactive_tx_signing_session.is_some()); let signature = self.get_initial_counterparty_commitment_signature(funding, logger); if let Some(signature) = signature { @@ -6209,19 +6194,6 @@ where } } - #[cfg(all(test))] - pub fn get_initial_counterparty_commitment_signature_for_test( - &mut self, funding: &mut FundingScope, logger: &L, - counterparty_next_commitment_point_override: PublicKey, - ) -> Option - where - SP::Target: SignerProvider, - L::Target: Logger, - { - self.counterparty_next_commitment_point = Some(counterparty_next_commitment_point_override); - self.get_initial_counterparty_commitment_signature(funding, logger) - } - fn check_funding_meets_minimum_depth(&self, funding: &FundingScope, height: u32) -> bool { let minimum_depth = self .minimum_depth(funding) @@ -6634,14 +6606,6 @@ where pub funding: FundingScope, pending_funding: Vec, pub context: ChannelContext, - /// The signing session for the current interactive tx construction, if any. - /// - /// This is populated when the interactive tx construction phase completes - /// (i.e., upon receiving a consecutive `tx_complete`) and the channel enters - /// the signing phase (`FundingNegotiated` state with the `INTERACTIVE_SIGNING` flag set). - /// - /// This field is cleared once our counterparty sends a `channel_ready`. - pub interactive_tx_signing_session: Option, holder_commitment_point: HolderCommitmentPoint, /// Info about an in-progress, pending splice (if any), on the pre-splice channel pending_splice: Option, @@ -6660,7 +6624,7 @@ macro_rules! promote_splice_funding { $self.context.historical_scids.push(scid); } core::mem::swap(&mut $self.funding, $funding); - $self.interactive_tx_signing_session = None; + $self.context.interactive_tx_signing_session = None; $self.pending_splice = None; $self.context.announcement_sigs = None; $self.context.announcement_sigs_state = AnnouncementSigsState::NotSent; @@ -7298,13 +7262,28 @@ where self.context.channel_state.clear_waiting_for_batch(); } - /// Unsets the existing funding information. + /// Unsets the existing funding information for V1 funded channels. /// /// This must only be used if the channel has not yet completed funding and has not been used. /// /// Further, the channel must be immediately shut down after this with a call to /// [`ChannelContext::force_shutdown`]. pub fn unset_funding_info(&mut self) { + let sent_or_received_tx_signatures = self + .context + .interactive_tx_signing_session + .as_ref() + .map(|signing_session| { + signing_session.holder_tx_signatures().is_some() + || signing_session.has_received_tx_signatures() + }) + .unwrap_or(false); + debug_assert!( + matches!( + self.context.channel_state, + ChannelState::FundingNegotiated(_) if !sent_or_received_tx_signatures + ) || matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(_)) + ); self.context.unset_funding_info(&mut self.funding); } @@ -7386,8 +7365,7 @@ where self.context.counterparty_current_commitment_point = self.context.counterparty_next_commitment_point; self.context.counterparty_next_commitment_point = Some(msg.next_per_commitment_point); - // Clear any interactive signing session. - self.interactive_tx_signing_session = None; + self.context.interactive_tx_signing_session = None; log_info!(logger, "Received channel_ready from peer for channel {}", &self.context.channel_id()); @@ -7535,13 +7513,17 @@ where ) -> Result::EcdsaSigner>, ChannelError> where L::Target: Logger { - if !self.context.channel_state.is_interactive_signing() - || self.context.channel_state.is_their_tx_signatures_sent() - { - let msg = "Received initial commitment_signed before funding transaction constructed or after peer's tx_signatures received!"; + if let Some(signing_session) = self.context.interactive_tx_signing_session.as_ref() { + if signing_session.has_received_tx_signatures() { + let msg = "Received initial commitment_signed after peer's tx_signatures received!"; + let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; + return Err(ChannelError::Close((msg.to_owned(), reason))); + } + } else { + let msg = "Received initial commitment_signed before funding transaction constructed!"; let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; return Err(ChannelError::Close((msg.to_owned(), reason))); - } + }; let holder_commitment_point = &mut self.holder_commitment_point.clone(); self.context.assert_no_commitment_advancement(holder_commitment_point.next_transaction_number(), "initial commitment_signed"); @@ -7553,7 +7535,7 @@ where log_info!(logger, "Received initial commitment_signed from peer for channel {}", &self.context.channel_id()); self.monitor_updating_paused(false, false, false, Vec::new(), Vec::new(), Vec::new()); - self.interactive_tx_signing_session.as_mut().expect("signing session should be present").received_commitment_signed(); + self.context.interactive_tx_signing_session.as_mut().expect("signing session should be present").received_commitment_signed(); Ok(channel_monitor) } @@ -7575,13 +7557,12 @@ where where L::Target: Logger, { - if !self.context.channel_state.is_interactive_signing() - || self.context.channel_state.is_their_tx_signatures_sent() - { - return Err(ChannelError::close( - "Received splice initial commitment_signed during invalid state".to_owned(), - )); - } + debug_assert!(self + .context + .interactive_tx_signing_session + .as_ref() + .map(|signing_session| !signing_session.has_received_tx_signatures()) + .unwrap_or(false)); let pending_splice_funding = self .pending_splice @@ -7592,6 +7573,7 @@ where }) .and_then(|funding_negotiation| funding_negotiation.as_funding()) .expect("Funding must exist for negotiated pending splice"); + let transaction_number = self.holder_commitment_point.current_transaction_number(); let commitment_point = self.holder_commitment_point.current_point().ok_or_else(|| { debug_assert!(false); @@ -7646,7 +7628,8 @@ where channel_id: Some(self.context.channel_id()), }; - self.interactive_tx_signing_session + self.context + .interactive_tx_signing_session .as_mut() .expect("Signing session must exist for negotiated pending splice") .received_commitment_signed(); @@ -8561,150 +8544,146 @@ where } } - pub fn funding_transaction_signed( - &mut self, funding_txid_signed: Txid, witnesses: Vec, - ) -> Result<(Option, Option), APIError> { - if !self.context.channel_state.is_interactive_signing() { - let err = - format!("Channel {} not expecting funding signatures", self.context.channel_id); - return Err(APIError::APIMisuseError { err }); - } - if self.context.channel_state.is_our_tx_signatures_ready() { - let err = - format!("Channel {} already received funding signatures", self.context.channel_id); - return Err(APIError::APIMisuseError { err }); - } - if let Some(pending_splice) = self.pending_splice.as_ref() { - if !pending_splice - .funding_negotiation - .as_ref() - .map(|funding_negotiation| { - matches!(funding_negotiation, FundingNegotiation::AwaitingSignatures(_)) - }) - .unwrap_or(false) + fn on_tx_signatures_exchange(&mut self, funding_tx: Transaction) { + debug_assert!(!self.context.channel_state.is_monitor_update_in_progress()); + debug_assert!(!self.context.channel_state.is_awaiting_remote_revoke()); + + if let Some(pending_splice) = self.pending_splice.as_mut() { + if let Some(FundingNegotiation::AwaitingSignatures(mut funding)) = + pending_splice.funding_negotiation.take() { + funding.funding_transaction = Some(funding_tx); + self.pending_funding.push(funding); + } else { debug_assert!(false); - let err = format!( - "Channel {} with pending splice is not expecting funding signatures yet", - self.context.channel_id - ); - return Err(APIError::APIMisuseError { err }); } + self.context.channel_state.clear_quiescent(); + } else { + self.funding.funding_transaction = Some(funding_tx); + self.context.channel_state = + ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()); } + } - let (tx_signatures_opt, funding_tx_opt) = self - .interactive_tx_signing_session - .as_mut() - .ok_or_else(|| APIError::APIMisuseError { - err: format!( - "Channel {} not expecting funding signatures", - self.context.channel_id - ), - }) - .and_then(|signing_session| { - let tx = signing_session.unsigned_tx().build_unsigned_tx(); - if funding_txid_signed != tx.compute_txid() { - return Err(APIError::APIMisuseError { - err: "Transaction was malleated prior to signing".to_owned(), - }); + pub fn funding_transaction_signed( + &mut self, funding_txid_signed: Txid, witnesses: Vec, + ) -> Result<(Option, Option), APIError> { + 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)); } - 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, - }; signing_session - .provide_holder_witnesses(tx_signatures, &self.context.secp_ctx) - .map_err(|err| APIError::APIMisuseError { err }) - })?; + } else { + let err = + format!("Channel {} not expecting funding signatures", self.context.channel_id); + return Err(APIError::APIMisuseError { err }); + }; - if tx_signatures_opt.is_some() { - self.context.channel_state.set_our_tx_signatures_ready(); + let tx = signing_session.unsigned_tx().build_unsigned_tx(); + if funding_txid_signed != tx.compute_txid() { + return Err(APIError::APIMisuseError { + err: "Transaction was malleated prior to signing".to_owned(), + }); } - if funding_tx_opt.is_some() { - self.funding.funding_transaction = funding_tx_opt.clone(); - self.context.channel_state = - ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()); + 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_opt, funding_tx_opt) = signing_session + .provide_holder_witnesses(tx_signatures, &self.context.secp_ctx) + .map_err(|err| APIError::APIMisuseError { err })?; + + if let Some(funding_tx) = funding_tx_opt.clone() { + debug_assert!(tx_signatures_opt.is_some()); + self.on_tx_signatures_exchange(funding_tx); } Ok((tx_signatures_opt, funding_tx_opt)) } - #[rustfmt::skip] - pub fn tx_signatures(&mut self, msg: &msgs::TxSignatures) -> Result<(Option, Option), ChannelError> { - if !self.context.channel_state.is_interactive_signing() - || self.context.channel_state.is_their_tx_signatures_sent() + pub fn tx_signatures( + &mut self, msg: &msgs::TxSignatures, + ) -> Result<(Option, Option), ChannelError> { + let signing_session = if let Some(signing_session) = + self.context.interactive_tx_signing_session.as_mut() { - return Err(ChannelError::Ignore("Ignoring tx_signatures received outside of interactive signing".to_owned())); - } - - if let Some(ref mut signing_session) = self.interactive_tx_signing_session { - if msg.tx_hash != signing_session.unsigned_tx().compute_txid() { - let msg = "The txid for the transaction does not match"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - return Err(ChannelError::Close((msg.to_owned(), reason))); + if signing_session.has_received_tx_signatures() { + return Err(ChannelError::Ignore("Ignoring duplicate tx_signatures".to_owned())); } - - // We need to close the channel if our peer hasn't sent their commitment signed already. - // Technically we'd wait on having an initial monitor persisted, so we shouldn't be broadcasting - // the transaction, but this may risk losing funds for a manual broadcast if we continue. if !signing_session.has_received_commitment_signed() { - let msg = "Received tx_signatures before initial commitment_signed"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - return Err(ChannelError::Close((msg.to_owned(), reason))); + return Err(ChannelError::close( + "Received tx_signatures before initial commitment_signed".to_owned(), + )); } - for witness in &msg.witnesses { - if witness.is_empty() { - let msg = "Unexpected empty witness in tx_signatures received"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - return Err(ChannelError::Close((msg.to_owned(), reason))); - } + 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)); } - let (holder_tx_signatures_opt, funding_tx_opt) = signing_session.received_tx_signatures(msg) - .map_err(|msg| ChannelError::Warn(msg))?; + signing_session + } else { + return Err(ChannelError::Ignore("Ignoring unexpected tx_signatures".to_owned())); + }; - // Set `THEIR_TX_SIGNATURES_SENT` flag after all potential errors. - self.context.channel_state.set_their_tx_signatures_sent(); + if msg.tx_hash != signing_session.unsigned_tx().compute_txid() { + let msg = "The txid for the transaction does not match"; + let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; + return Err(ChannelError::Close((msg.to_owned(), reason))); + } - if funding_tx_opt.is_some() { - // TODO(splicing): Transition back to `ChannelReady` and not `AwaitingChannelReady` - // We will also need to use the pending `FundingScope` in the splicing case. - // - // We have a finalized funding transaction, so we can set the funding transaction. - self.funding.funding_transaction = funding_tx_opt.clone(); - self.context.channel_state = ChannelState::AwaitingChannelReady(AwaitingChannelReadyFlags::new()); + for witness in &msg.witnesses { + if witness.is_empty() { + let msg = "Unexpected empty witness in tx_signatures received"; + let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; + return Err(ChannelError::Close((msg.to_owned(), reason))); } + } - Ok((holder_tx_signatures_opt, funding_tx_opt)) - } else { - let msg = "Unexpected tx_signatures. No funding transaction awaiting signatures"; - let reason = ClosureReason::ProcessingError { err: msg.to_owned() }; - return Err(ChannelError::Close((msg.to_owned(), reason))); + let (holder_tx_signatures_opt, funding_tx_opt) = + signing_session.received_tx_signatures(msg).map_err(|msg| ChannelError::Warn(msg))?; + + if let Some(funding_tx) = funding_tx_opt.clone() { + self.on_tx_signatures_exchange(funding_tx); } + + Ok((holder_tx_signatures_opt, funding_tx_opt)) } /// Queues up an outbound update fee by placing it in the holding cell. You should call @@ -8785,7 +8764,7 @@ where #[rustfmt::skip] fn remove_uncommitted_htlcs_and_mark_paused(&mut self, logger: &L) -> Result<(), ()> where L::Target: Logger { assert!(!matches!(self.context.channel_state, ChannelState::ShutdownComplete)); - if !self.context.channel_state.can_resume_on_reconnect() { + if !self.context.can_resume_on_reconnect() { return Err(()) } @@ -8932,7 +8911,7 @@ where // An active interactive signing session or an awaiting channel_ready state implies that a // commitment_signed retransmission is an initial one for funding negotiation. Thus, the // signatures should be sent before channel_ready. - let channel_ready_order = if self.interactive_tx_signing_session.is_some() { + let channel_ready_order = if self.context.interactive_tx_signing_session.is_some() { ChannelReadyOrder::SignaturesFirst } else if matches!(self.context.channel_state, ChannelState::AwaitingChannelReady(_)) { ChannelReadyOrder::SignaturesFirst @@ -9423,7 +9402,7 @@ where if let Some(next_funding) = &msg.next_funding { // - if `next_funding` matches the latest interactive funding transaction // or the current channel funding transaction: - if let Some(session) = &self.interactive_tx_signing_session { + if let Some(session) = &self.context.interactive_tx_signing_session { let our_next_funding_txid = session.unsigned_tx().compute_txid(); if our_next_funding_txid != next_funding.txid { return Err(ChannelError::close(format!( @@ -9489,7 +9468,7 @@ where // - if it has already received `tx_signatures` for that funding transaction: // - MUST send its `tx_signatures` for that funding transaction. if (session.has_received_commitment_signed() && session.holder_sends_tx_signatures_first()) - || self.context.channel_state.is_their_tx_signatures_sent() + || session.has_received_tx_signatures() { // If `holder_tx_signatures` is `None` here, the `tx_signatures` message will be sent // when the holder provides their witnesses as this will queue a `tx_signatures` if the @@ -9879,8 +9858,8 @@ where } let mut not_broadcasted = matches!(self.context.channel_state, ChannelState::NegotiatingFunding(_)); - if let ChannelState::FundingNegotiated(flags) = &self.context.channel_state { - if !flags.is_our_tx_signatures_ready() { + if let Some(signing_session) = self.context.interactive_tx_signing_session.as_ref() { + if signing_session.holder_tx_signatures().is_none() { // If we're a V1 channel or we haven't yet sent our `tx_signatures`, the funding tx // couldn't be broadcasted yet, so just short-circuit the shutdown logic. not_broadcasted = true; @@ -11250,38 +11229,27 @@ where self.sign_channel_announcement(node_signer, announcement).ok() } - #[rustfmt::skip] fn maybe_get_next_funding(&self) -> Option { // If we've sent `commtiment_signed` for an interactively constructed transaction // during a signing session, but have not received `tx_signatures` we MUST set `next_funding` // to the txid of that interactive transaction, else we MUST NOT set it. - if self.context.channel_state.is_interactive_signing() { - // Since we have a signing_session, this implies we've sent an initial `commitment_signed`... - if !self.context.channel_state.is_their_tx_signatures_sent() { - // ...but we didn't receive a `tx_signatures` from the counterparty yet. - self.interactive_tx_signing_session - .as_ref() - .map(|signing_session| { - let mut next_funding = msgs::NextFunding { - txid: signing_session.unsigned_tx().compute_txid(), - retransmit_flags: 0, - }; + self.context + .interactive_tx_signing_session + .as_ref() + .filter(|session| !session.has_received_tx_signatures()) + .map(|signing_session| { + let mut next_funding = msgs::NextFunding { + txid: signing_session.unsigned_tx().compute_txid(), + retransmit_flags: 0, + }; - // TODO(splicing): Add comment for spec requirements - if !signing_session.has_received_commitment_signed() { - next_funding.retransmit(msgs::NextFundingFlag::CommitmentSigned); - } + // TODO(splicing): Add comment for spec requirements + if !signing_session.has_received_commitment_signed() { + next_funding.retransmit(msgs::NextFundingFlag::CommitmentSigned); + } - next_funding - }) - } else { - // ...and we received a `tx_signatures` from the counterparty. - None - } - } else { - // We don't have an active signing session. - None - } + next_funding + }) } fn maybe_get_my_current_funding_locked(&self) -> Option { @@ -11794,7 +11762,7 @@ where })?; let tx_msg_opt = interactive_tx_constructor.take_initiator_first_message(); - debug_assert!(self.interactive_tx_signing_session.is_none()); + debug_assert!(self.context.interactive_tx_signing_session.is_none()); pending_splice.funding_negotiation = Some(FundingNegotiation::ConstructingTransaction( splice_funding, @@ -13066,7 +13034,6 @@ where funding: self.funding, pending_funding: vec![], context: self.context, - interactive_tx_signing_session: None, holder_commitment_point, pending_splice: None, quiescent_action: None, @@ -13110,6 +13077,10 @@ where /// The channel must be immediately shut down after this with a call to /// [`ChannelContext::force_shutdown`]. pub fn unset_funding_info(&mut self) { + debug_assert!(matches!( + self.context.channel_state, + ChannelState::FundingNegotiated(_) if self.context.interactive_tx_signing_session.is_none() + )); self.context.unset_funding_info(&mut self.funding); } } @@ -13352,7 +13323,6 @@ where funding: self.funding, pending_funding: vec![], context: self.context, - interactive_tx_signing_session: None, holder_commitment_point, pending_splice: None, quiescent_action: None, @@ -13396,8 +13366,6 @@ where pub funding_negotiation_context: FundingNegotiationContext, /// The current interactive transaction construction session under negotiation. pub interactive_tx_constructor: Option, - /// The signing session created after `tx_complete` handling - pub interactive_tx_signing_session: Option, } impl PendingV2Channel @@ -13472,7 +13440,6 @@ where unfunded_context, funding_negotiation_context, interactive_tx_constructor: None, - interactive_tx_signing_session: None, }; Ok(chan) } @@ -13660,7 +13627,6 @@ where context, funding_negotiation_context, interactive_tx_constructor, - interactive_tx_signing_session: None, unfunded_context, }) } @@ -13870,7 +13836,8 @@ where channel_state.clear_remote_stfu_sent(); channel_state.clear_quiescent(); }, - ChannelState::FundingNegotiated(flags) if flags.is_interactive_signing() => {}, + ChannelState::FundingNegotiated(_) + if self.context.interactive_tx_signing_session.is_some() => {}, _ => debug_assert!(false, "Pre-funded/shutdown channels should not be written"), } channel_state.set_peer_disconnected(); @@ -14268,7 +14235,7 @@ where (54, self.pending_funding, optional_vec), // Added in 0.2 (55, removed_htlc_attribution_data, optional_vec), // Added in 0.2 (57, holding_cell_attribution_data, optional_vec), // Added in 0.2 - (58, self.interactive_tx_signing_session, option), // Added in 0.2 + (58, self.context.interactive_tx_signing_session, option), // Added in 0.2 (59, self.funding.minimum_depth_override, option), // Added in 0.2 (60, self.context.historical_scids, optional_vec), // Added in 0.2 (61, fulfill_attribution_data, optional_vec), // Added in 0.2 @@ -15038,8 +15005,9 @@ where blocked_monitor_updates: blocked_monitor_updates.unwrap(), is_manual_broadcast: is_manual_broadcast.unwrap_or(false), + + interactive_tx_signing_session, }, - interactive_tx_signing_session, holder_commitment_point, pending_splice: None, quiescent_action, diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index 9a3a0366ae1..4628b2e2830 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/lightning/src/ln/channelmanager.rs @@ -9027,7 +9027,7 @@ 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.interactive_tx_signing_session.as_mut()) + .and_then(|_| channel.context.interactive_tx_signing_session.as_mut()) .filter(|signing_session| signing_session.holder_tx_signatures().is_none()) { if signing_session.has_local_contribution() { diff --git a/lightning/src/ln/dual_funding_tests.rs b/lightning/src/ln/dual_funding_tests.rs index e49a0f566e0..fc66ec315ca 100644 --- a/lightning/src/ln/dual_funding_tests.rs +++ b/lightning/src/ln/dual_funding_tests.rs @@ -8,243 +8,3 @@ // licenses. //! Tests that test the creation of dual-funded channels in ChannelManager. - -use { - crate::chain::chaininterface::{ConfirmationTarget, LowerBoundedFeeEstimator}, - crate::events::Event, - crate::ln::chan_utils::{ - make_funding_redeemscript, ChannelPublicKeys, ChannelTransactionParameters, - CounterpartyChannelTransactionParameters, - }, - crate::ln::channel::PendingV2Channel, - crate::ln::channel_keys::{DelayedPaymentBasepoint, HtlcBasepoint, RevocationBasepoint}, - crate::ln::functional_test_utils::*, - crate::ln::funding::FundingTxInput, - crate::ln::msgs::{BaseMessageHandler, ChannelMessageHandler, MessageSendEvent}, - crate::ln::msgs::{CommitmentSigned, TxAddInput, TxAddOutput, TxComplete, TxSignatures}, - crate::ln::types::ChannelId, - crate::prelude::*, - crate::util::test_utils, - bitcoin::Witness, -}; - -// Dual-funding: V2 Channel Establishment Tests -struct V2ChannelEstablishmentTestSession { - funding_input_sats: u64, - initiator_input_value_satoshis: u64, -} - -// TODO(dual_funding): Use real node and API for creating V2 channels as initiator when available, -// instead of manually constructing messages. -fn do_test_v2_channel_establishment(session: V2ChannelEstablishmentTestSession) { - let chanmon_cfgs = create_chanmon_cfgs(2); - let node_cfgs = create_node_cfgs(2, &chanmon_cfgs); - let mut node_1_user_config = test_default_channel_config(); - node_1_user_config.enable_dual_funded_channels = true; - let node_chanmgrs = create_node_chanmgrs(2, &node_cfgs, &[None, Some(node_1_user_config)]); - let nodes = create_network(2, &node_cfgs, &node_chanmgrs); - let logger_a = test_utils::TestLogger::with_id("node a".to_owned()); - - // Create a funding input for the new channel along with its previous transaction. - let initiator_funding_inputs: Vec<_> = create_dual_funding_utxos_with_prev_txs( - &nodes[0], - &[session.initiator_input_value_satoshis], - ); - - // Alice creates a dual-funded channel as initiator. - let funding_satoshis = session.funding_input_sats; - let mut channel = PendingV2Channel::new_outbound( - &LowerBoundedFeeEstimator(node_cfgs[0].fee_estimator), - &nodes[0].node.entropy_source, - &nodes[0].node.signer_provider, - nodes[1].node.get_our_node_id(), - &nodes[1].node.init_features(), - funding_satoshis, - initiator_funding_inputs.clone(), - 42, /* user_channel_id */ - &nodes[0].node.get_current_config(), - nodes[0].best_block_info().1, - nodes[0].node.create_and_insert_outbound_scid_alias_for_test(), - ConfirmationTarget::NonAnchorChannelFee, - &logger_a, - ) - .unwrap(); - let open_channel_v2_msg = channel.get_open_channel_v2(nodes[0].chain_source.chain_hash); - - nodes[1].node.handle_open_channel_v2(nodes[0].node.get_our_node_id(), &open_channel_v2_msg); - - let accept_channel_v2_msg = get_event_msg!( - nodes[1], - MessageSendEvent::SendAcceptChannelV2, - nodes[0].node.get_our_node_id() - ); - let channel_id = ChannelId::v2_from_revocation_basepoints( - &RevocationBasepoint::from(accept_channel_v2_msg.common_fields.revocation_basepoint), - &RevocationBasepoint::from(open_channel_v2_msg.common_fields.revocation_basepoint), - ); - - let FundingTxInput { sequence, prevtx, .. } = &initiator_funding_inputs[0]; - let tx_add_input_msg = TxAddInput { - channel_id, - serial_id: 2, // Even serial_id from initiator. - prevtx: Some(prevtx.clone()), - prevtx_out: 0, - sequence: sequence.0, - shared_input_txid: None, - }; - let input_value = tx_add_input_msg.prevtx.as_ref().unwrap().output - [tx_add_input_msg.prevtx_out as usize] - .value; - assert_eq!(input_value.to_sat(), session.initiator_input_value_satoshis); - - nodes[1].node.handle_tx_add_input(nodes[0].node.get_our_node_id(), &tx_add_input_msg); - - let _tx_complete_msg = - get_event_msg!(nodes[1], MessageSendEvent::SendTxComplete, nodes[0].node.get_our_node_id()); - - let tx_add_output_msg = TxAddOutput { - channel_id, - serial_id: 4, - sats: funding_satoshis, - script: make_funding_redeemscript( - &open_channel_v2_msg.common_fields.funding_pubkey, - &accept_channel_v2_msg.common_fields.funding_pubkey, - ) - .to_p2wsh(), - }; - nodes[1].node.handle_tx_add_output(nodes[0].node.get_our_node_id(), &tx_add_output_msg); - - let _tx_complete_msg = - get_event_msg!(nodes[1], MessageSendEvent::SendTxComplete, nodes[0].node.get_our_node_id()); - - let tx_complete_msg = TxComplete { channel_id }; - - nodes[1].node.handle_tx_complete(nodes[0].node.get_our_node_id(), &tx_complete_msg); - let msg_events = nodes[1].node.get_and_clear_pending_msg_events(); - assert_eq!(msg_events.len(), 1); - let _msg_commitment_signed_from_1 = match msg_events[0] { - MessageSendEvent::UpdateHTLCs { ref node_id, channel_id: _, ref updates } => { - assert_eq!(*node_id, nodes[0].node.get_our_node_id()); - updates.commitment_signed.clone() - }, - _ => panic!("Unexpected event"), - }; - - let (funding_outpoint, channel_type_features) = { - let per_peer_state = nodes[1].node.per_peer_state.read().unwrap(); - let peer_state = - per_peer_state.get(&nodes[0].node.get_our_node_id()).unwrap().lock().unwrap(); - let channel_funding = - peer_state.channel_by_id.get(&tx_complete_msg.channel_id).unwrap().funding(); - (channel_funding.get_funding_txo(), channel_funding.get_channel_type().clone()) - }; - - channel.funding.channel_transaction_parameters = ChannelTransactionParameters { - counterparty_parameters: Some(CounterpartyChannelTransactionParameters { - pubkeys: ChannelPublicKeys { - funding_pubkey: accept_channel_v2_msg.common_fields.funding_pubkey, - revocation_basepoint: RevocationBasepoint( - accept_channel_v2_msg.common_fields.revocation_basepoint, - ), - payment_point: accept_channel_v2_msg.common_fields.payment_basepoint, - delayed_payment_basepoint: DelayedPaymentBasepoint( - accept_channel_v2_msg.common_fields.delayed_payment_basepoint, - ), - htlc_basepoint: HtlcBasepoint(accept_channel_v2_msg.common_fields.htlc_basepoint), - }, - selected_contest_delay: accept_channel_v2_msg.common_fields.to_self_delay, - }), - holder_pubkeys: ChannelPublicKeys { - funding_pubkey: open_channel_v2_msg.common_fields.funding_pubkey, - revocation_basepoint: RevocationBasepoint( - open_channel_v2_msg.common_fields.revocation_basepoint, - ), - payment_point: open_channel_v2_msg.common_fields.payment_basepoint, - delayed_payment_basepoint: DelayedPaymentBasepoint( - open_channel_v2_msg.common_fields.delayed_payment_basepoint, - ), - htlc_basepoint: HtlcBasepoint(open_channel_v2_msg.common_fields.htlc_basepoint), - }, - holder_selected_contest_delay: open_channel_v2_msg.common_fields.to_self_delay, - is_outbound_from_holder: true, - funding_outpoint, - splice_parent_funding_txid: None, - channel_type_features, - channel_value_satoshis: funding_satoshis, - }; - - let msg_commitment_signed_from_0 = CommitmentSigned { - channel_id, - signature: channel - .context - .get_initial_counterparty_commitment_signature_for_test( - &mut channel.funding, - &&logger_a, - accept_channel_v2_msg.common_fields.first_per_commitment_point, - ) - .unwrap(), - htlc_signatures: vec![], - funding_txid: None, - #[cfg(taproot)] - partial_signature_with_nonce: None, - }; - - chanmon_cfgs[1].persister.set_update_ret(crate::chain::ChannelMonitorUpdateStatus::InProgress); - - // Handle the initial commitment_signed exchange. Order is not important here. - nodes[1] - .node - .handle_commitment_signed(nodes[0].node.get_our_node_id(), &msg_commitment_signed_from_0); - check_added_monitors(&nodes[1], 1); - - // The funding transaction should not have been broadcast before persisting initial monitor has - // been completed. - assert_eq!(nodes[1].tx_broadcaster.txn_broadcast().len(), 0); - assert_eq!(nodes[1].node.get_and_clear_pending_events().len(), 0); - - // Complete the persistence of the monitor. - let events = nodes[1].node.get_and_clear_pending_events(); - assert!(events.is_empty()); - nodes[1].chain_monitor.complete_sole_pending_chan_update(&channel_id); - - let tx_signatures_msg = get_event_msg!( - nodes[1], - MessageSendEvent::SendTxSignatures, - nodes[0].node.get_our_node_id() - ); - - assert_eq!(tx_signatures_msg.channel_id, channel_id); - - let mut witness = Witness::new(); - witness.push([0x0]); - // Receive tx_signatures from channel initiator. - nodes[1].node.handle_tx_signatures( - nodes[0].node.get_our_node_id(), - &TxSignatures { - channel_id, - tx_hash: funding_outpoint.unwrap().txid, - witnesses: vec![witness], - shared_input_signature: None, - }, - ); - - let events = nodes[1].node.get_and_clear_pending_events(); - assert_eq!(events.len(), 1); - match events[0] { - Event::ChannelPending { channel_id: chan_id, .. } => assert_eq!(chan_id, channel_id), - _ => panic!("Unexpected event"), - }; - - // For an inbound channel V2 channel the transaction should be broadcast once receiving a - // tx_signature and applying local tx_signatures: - let broadcasted_txs = nodes[1].tx_broadcaster.txn_broadcast(); - assert_eq!(broadcasted_txs.len(), 1); -} - -#[test] -fn test_v2_channel_establishment() { - do_test_v2_channel_establishment(V2ChannelEstablishmentTestSession { - funding_input_sats: 100_00, - initiator_input_value_satoshis: 150_000, - }); -} diff --git a/lightning/src/ln/interactivetxs.rs b/lightning/src/ln/interactivetxs.rs index d199b37edae..3d65996df88 100644 --- a/lightning/src/ln/interactivetxs.rs +++ b/lightning/src/ln/interactivetxs.rs @@ -439,6 +439,10 @@ impl InteractiveTxSigningSession { self.has_received_commitment_signed } + pub fn has_received_tx_signatures(&self) -> bool { + self.has_received_tx_signatures + } + pub fn holder_tx_signatures(&self) -> &Option { &self.holder_tx_signatures }