From 689d0c1d612397b852c173aa3b2aa10c071c834b Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 29 May 2025 16:20:46 -0700 Subject: [PATCH 1/3] Introduce RenegotiatedFunding monitor update variant This is a new `ChannelMonitorUpdateStep` variant intended to be used whenever a new funding transaction for the channel has been negotiated via the `InteractiveTxConstructor`. This commit primarily focuses on its use for splices, but future work will expand where needed to support RBFs (both for the initial dual funding transaction, and splice transactions). To draw a parallel to channel open, we generally want to have the commitment transactions negotiated for the funding transaction and committed to the respective `ChannelMonitor` before attempting to sign the funding transaction itself. This monitor update fulfills this need for a newly negotiated splice; it includes both the new holder and counterparty commitment transactions, and the new set of applicable `ChannelTransactionParameters`. Once the monitor update has been applied to the monitor and persisted, we allow the release of our `tx_signatures` for the splice transaction to wait for its confirmation. --- lightning/src/chain/chainmonitor.rs | 31 ++++- lightning/src/chain/channelmonitor.rs | 191 +++++++++++++++++++++++++- lightning/src/ln/channel.rs | 95 ++++++++++++- 3 files changed, 310 insertions(+), 7 deletions(-) diff --git a/lightning/src/chain/chainmonitor.rs b/lightning/src/chain/chainmonitor.rs index 09d87b775be..f14a270b39f 100644 --- a/lightning/src/chain/chainmonitor.rs +++ b/lightning/src/chain/chainmonitor.rs @@ -31,7 +31,7 @@ use bitcoin::hash_types::{Txid, BlockHash}; use crate::chain; use crate::chain::{ChannelMonitorUpdateStatus, Filter, WatchedOutput}; use crate::chain::chaininterface::{BroadcasterInterface, FeeEstimator}; -use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, Balance, MonitorEvent, TransactionOutputs, WithChannelMonitor}; +use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, Balance, MonitorEvent, TransactionOutputs, WithChannelMonitor}; use crate::chain::transaction::{OutPoint, TransactionData}; use crate::ln::types::ChannelId; use crate::sign::ecdsa::EcdsaChannelSigner; @@ -867,6 +867,35 @@ where C::Target: chain::Filter, panic!("{}", err_str); }, } + + // We may need to start monitoring for any alternative funding transactions. + if let Some(ref chain_source) = self.chain_source { + for update in &update.updates { + match update { + ChannelMonitorUpdateStep::RenegotiatedFunding { + channel_parameters, .. + } => { + let funding_outpoint = channel_parameters.funding_outpoint + .expect("Renegotiated funding must always have known outpoint"); + let funding_script = + channel_parameters.make_funding_redeemscript().to_p2wsh(); + log_trace!( + &logger, + "Registering renegotiated funding outpoint {} with the filter to monitor confirmations and spends", + &funding_outpoint + ); + chain_source.register_tx(&funding_outpoint.txid, &funding_script); + chain_source.register_output(WatchedOutput { + block_hash: None, + outpoint: funding_outpoint, + script_pubkey: funding_script, + }); + }, + _ => {}, + } + } + } + if update_res.is_err() { ChannelMonitorUpdateStatus::InProgress } else { diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 41f4751fbd3..f6fcb3c3cef 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -621,6 +621,11 @@ pub(crate) enum ChannelMonitorUpdateStep { ShutdownScript { scriptpubkey: ScriptBuf, }, + RenegotiatedFunding { + channel_parameters: ChannelTransactionParameters, + holder_commitment_tx: HolderCommitmentTransaction, + counterparty_commitment_tx: CommitmentTransaction, + }, } impl ChannelMonitorUpdateStep { @@ -633,6 +638,7 @@ impl ChannelMonitorUpdateStep { ChannelMonitorUpdateStep::CommitmentSecret { .. } => "CommitmentSecret", ChannelMonitorUpdateStep::ChannelForceClosed { .. } => "ChannelForceClosed", ChannelMonitorUpdateStep::ShutdownScript { .. } => "ShutdownScript", + ChannelMonitorUpdateStep::RenegotiatedFunding { .. } => "RenegotiatedFunding", } } } @@ -671,6 +677,11 @@ impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep, (0, htlc_outputs, required_vec), (2, commitment_tx, required), }, + (10, RenegotiatedFunding) => { + (1, channel_parameters, (required: ReadableArgs, None)), + (3, holder_commitment_tx, required), + (5, counterparty_commitment_tx, required), + }, ); /// Indicates whether the balance is derived from a cooperative close, a force-close @@ -997,9 +1008,69 @@ struct FundingScope { prev_holder_commitment_tx: Option, } +impl FundingScope { + fn funding_outpoint(&self) -> OutPoint { + *self.channel_parameters.funding_outpoint.as_ref() + .expect("Funding outpoint must be set for active monitor") + } + + fn funding_txid(&self) -> Txid { + self.funding_outpoint().txid + } +} + +impl Writeable for FundingScope { + fn write(&self, w: &mut W) -> Result<(), io::Error> { + write_tlv_fields!(w, { + (1, self.channel_parameters, (required: ReadableArgs, None)), + (3, self.current_counterparty_commitment_txid, required), + (5, self.prev_counterparty_commitment_txid, option), + (7, self.current_holder_commitment_tx, required), + (9, self.prev_holder_commitment_tx, option), + (11, self.counterparty_claimable_outpoints, required), + }); + Ok(()) + } +} + +impl Readable for FundingScope { + fn read(r: &mut R) -> Result { + let mut channel_parameters = RequiredWrapper(None); + let mut current_counterparty_commitment_txid = RequiredWrapper(None); + let mut prev_counterparty_commitment_txid = None; + let mut current_holder_commitment_tx = RequiredWrapper(None); + let mut prev_holder_commitment_tx = None; + let mut counterparty_claimable_outpoints = RequiredWrapper(None); + + read_tlv_fields!(r, { + (1, channel_parameters, (required: ReadableArgs, None)), + (3, current_counterparty_commitment_txid, required), + (5, prev_counterparty_commitment_txid, option), + (7, current_holder_commitment_tx, required), + (9, prev_holder_commitment_tx, option), + (11, counterparty_claimable_outpoints, required), + }); + + let channel_parameters: ChannelTransactionParameters = channel_parameters.0.unwrap(); + let redeem_script = channel_parameters.make_funding_redeemscript(); + + Ok(Self { + script_pubkey: redeem_script.to_p2wsh(), + redeem_script, + channel_parameters, + current_counterparty_commitment_txid: current_counterparty_commitment_txid.0.unwrap(), + prev_counterparty_commitment_txid, + current_holder_commitment_tx: current_holder_commitment_tx.0.unwrap(), + prev_holder_commitment_tx, + counterparty_claimable_outpoints: counterparty_claimable_outpoints.0.unwrap(), + }) + } +} + #[derive(Clone, PartialEq)] pub(crate) struct ChannelMonitorImpl { funding: FundingScope, + pending_funding: Vec, latest_update_id: u64, commitment_transaction_number_obscure_factor: u64, @@ -1433,6 +1504,7 @@ impl Writeable for ChannelMonitorImpl { (27, self.first_confirmed_funding_txo, required), (29, self.initial_counterparty_commitment_tx, option), (31, self.funding.channel_parameters, required), + (32, self.pending_funding, optional_vec), }); Ok(()) @@ -1588,6 +1660,7 @@ impl ChannelMonitor { current_holder_commitment_tx: initial_holder_commitment_tx, prev_holder_commitment_tx: None, }, + pending_funding: vec![], latest_update_id: 0, commitment_transaction_number_obscure_factor, @@ -1812,14 +1885,16 @@ impl ChannelMonitor { { let lock = self.inner.lock().unwrap(); let logger = WithChannelMonitor::from_impl(logger, &*lock, None); - log_trace!(&logger, "Registering funding outpoint {}", &lock.get_funding_txo()); - let funding_outpoint = lock.get_funding_txo(); - filter.register_tx(&funding_outpoint.txid, &lock.funding.script_pubkey); + for funding in core::iter::once(&lock.funding).chain(&lock.pending_funding) { + let funding_outpoint = funding.funding_outpoint(); + log_trace!(&logger, "Registering funding outpoint {} with the filter to monitor confirmations", &funding_outpoint); + filter.register_tx(&funding_outpoint.txid, &funding.script_pubkey); + } for (txid, outputs) in lock.get_outputs_to_watch().iter() { for (index, script_pubkey) in outputs.iter() { assert!(*index <= u16::MAX as u32); let outpoint = OutPoint { txid: *txid, index: *index as u16 }; - log_trace!(logger, "Registering outpoint {} with the filter for monitoring spends", outpoint); + log_trace!(logger, "Registering outpoint {} with the filter to monitor spend", outpoint); filter.register_output(WatchedOutput { block_hash: None, outpoint, @@ -3364,6 +3439,94 @@ impl ChannelMonitorImpl { ); } + fn renegotiated_funding( + &mut self, logger: &WithChannelMonitor, + channel_parameters: &ChannelTransactionParameters, + alternative_holder_commitment_tx: &HolderCommitmentTransaction, + alternative_counterparty_commitment_tx: &CommitmentTransaction, + ) -> Result<(), ()> + where + L::Target: Logger, + { + let redeem_script = channel_parameters.make_funding_redeemscript(); + let script_pubkey = redeem_script.to_p2wsh(); + let alternative_counterparty_commitment_txid = + alternative_counterparty_commitment_tx.trust().txid(); + + // Both the current counterparty commitment and the alternative one share the same set of + // non-dust and dust HTLCs in the same order, though the index of each non-dust HTLC may be + // different. + // + // We clone all HTLCs and their sources to use in the alternative funding scope, and update + // each non-dust HTLC with their corresponding index in the alternative counterparty + // commitment. + let current_counterparty_commitment_htlcs = + if let Some(txid) = &self.funding.current_counterparty_commitment_txid { + self.funding.counterparty_claimable_outpoints.get(txid).unwrap() + } else { + debug_assert!(false); + log_error!(logger, "Received funding renegotiation while initial funding negotiation is still pending"); + return Err(()); + }; + let mut htlcs_with_sources = current_counterparty_commitment_htlcs.clone(); + let alternative_htlcs = alternative_counterparty_commitment_tx.nondust_htlcs(); + // The left side only tracks non-dust, while the right side tracks dust as well. + debug_assert!(alternative_htlcs.len() <= current_counterparty_commitment_htlcs.len()); + for (alternative_htlc, (htlc, _)) in alternative_htlcs.iter().zip(htlcs_with_sources.iter_mut()) { + debug_assert!(htlc.transaction_output_index.is_some()); + debug_assert!(alternative_htlc.transaction_output_index.is_some()); + debug_assert!(alternative_htlc.is_data_equal(htlc)); + htlc.transaction_output_index = alternative_htlc.transaction_output_index; + } + + let mut counterparty_claimable_outpoints = new_hash_map(); + counterparty_claimable_outpoints.insert( + alternative_counterparty_commitment_txid, htlcs_with_sources, + ); + + // TODO(splicing): Enforce any necessary RBF validity checks. + let alternative_funding = FundingScope { + script_pubkey: script_pubkey.clone(), + redeem_script, + channel_parameters: channel_parameters.clone(), + current_counterparty_commitment_txid: Some(alternative_counterparty_commitment_txid), + prev_counterparty_commitment_txid: None, + counterparty_claimable_outpoints, + current_holder_commitment_tx: alternative_holder_commitment_tx.clone(), + prev_holder_commitment_tx: None, + }; + let alternative_funding_outpoint = alternative_funding.funding_outpoint(); + + if self.pending_funding.iter() + .any(|funding| funding.funding_txid() == alternative_funding_outpoint.txid) + { + log_error!(logger, "Renegotiated funding transaction with a duplicate funding txid {}", + alternative_funding_outpoint.txid); + return Err(()); + } + + if let Some(parent_funding_txid) = channel_parameters.splice_parent_funding_txid.as_ref() { + // Only one splice can be negotiated at a time after we've exchanged `channel_ready` + // (implying our funding is confirmed) that spends our currently locked funding. + if !self.pending_funding.is_empty() { + log_error!(logger, "Negotiated splice while channel is pending channel_ready/splice_locked"); + return Err(()); + } + if *parent_funding_txid != self.funding.funding_txid() { + log_error!(logger, "Negotiated splice that does not spend currently locked funding transaction"); + return Err(()); + } + } + + self.outputs_to_watch.insert( + alternative_funding_outpoint.txid, + vec![(alternative_funding_outpoint.index as u32, script_pubkey)], + ); + self.pending_funding.push(alternative_funding); + + Ok(()) + } + fn update_monitor( &mut self, updates: &ChannelMonitorUpdate, broadcaster: &B, fee_estimator: &F, logger: &WithChannelMonitor ) -> Result<(), ()> @@ -3442,6 +3605,17 @@ impl ChannelMonitorImpl { ret = Err(()); } }, + ChannelMonitorUpdateStep::RenegotiatedFunding { + channel_parameters, holder_commitment_tx, counterparty_commitment_tx, + } => { + log_trace!(logger, "Updating ChannelMonitor with alternative holder and counterparty commitment transactions for funding txid {}", + channel_parameters.funding_outpoint.unwrap().txid); + if let Err(_) = self.renegotiated_funding( + logger, channel_parameters, holder_commitment_tx, counterparty_commitment_tx, + ) { + ret = Err(()); + } + }, ChannelMonitorUpdateStep::ChannelForceClosed { should_broadcast } => { log_trace!(logger, "Updating ChannelMonitor: channel force closed, should broadcast: {}", should_broadcast); self.lockdown_from_offchain = true; @@ -3493,7 +3667,8 @@ impl ChannelMonitorImpl { |ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. } |ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { .. } |ChannelMonitorUpdateStep::ShutdownScript { .. } - |ChannelMonitorUpdateStep::CommitmentSecret { .. } => + |ChannelMonitorUpdateStep::CommitmentSecret { .. } + |ChannelMonitorUpdateStep::RenegotiatedFunding { .. } => is_pre_close_update = true, // After a channel is closed, we don't communicate with our peer about it, so the // only things we will update is getting a new preimage (from a different channel) @@ -3671,6 +3846,9 @@ impl ChannelMonitorImpl { } => { Some(commitment_tx.clone()) }, + &ChannelMonitorUpdateStep::RenegotiatedFunding { ref counterparty_commitment_tx, .. } => { + Some(counterparty_commitment_tx.clone()) + }, _ => None, } }).collect() @@ -5303,6 +5481,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP let mut payment_preimages_with_info: Option> = None; let mut first_confirmed_funding_txo = RequiredWrapper(None); let mut channel_parameters = None; + let mut pending_funding = None; read_tlv_fields!(reader, { (1, funding_spend_confirmed, option), (3, htlcs_resolved_on_chain, optional_vec), @@ -5320,6 +5499,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP (27, first_confirmed_funding_txo, (default_value, outpoint)), (29, initial_counterparty_commitment_tx, option), (31, channel_parameters, (option: ReadableArgs, None)), + (32, pending_funding, optional_vec), }); if let Some(payment_preimages_with_info) = payment_preimages_with_info { if payment_preimages_with_info.len() != payment_preimages.len() { @@ -5435,6 +5615,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP current_holder_commitment_tx, prev_holder_commitment_tx, }, + pending_funding: pending_funding.unwrap_or(vec![]), latest_update_id, commitment_transaction_number_obscure_factor, diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index a9e463a99eb..93c42231e03 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -1677,7 +1677,33 @@ impl Channel where res }, ChannelPhase::Funded(mut funded_channel) => { - let res = funded_channel.commitment_signed(msg, logger).map(|monitor_update_opt| (None, monitor_update_opt)); + #[cfg(splicing)] + let has_negotiated_pending_splice = funded_channel.pending_splice.as_ref() + .map(|pending_splice| pending_splice.funding.is_some()) + .unwrap_or(false); + #[cfg(splicing)] + let session_received_commitment_signed = funded_channel + .interactive_tx_signing_session + .as_ref() + .map(|session| session.has_received_commitment_signed()); + #[cfg(splicing)] + let res = if has_negotiated_pending_splice + // Not having a signing session implies they've already sent `splice_locked`, + // which must always come after the initial commitment signed is sent. + && !session_received_commitment_signed.unwrap_or(true) + { + funded_channel + .splice_initial_commitment_signed(msg, logger) + .map(|monitor_update_opt| (None, monitor_update_opt)) + } else { + funded_channel.commitment_signed(msg, logger) + .map(|monitor_update_opt| (None, monitor_update_opt)) + }; + + #[cfg(not(splicing))] + let res = funded_channel.commitment_signed(msg, logger) + .map(|monitor_update_opt| (None, monitor_update_opt)); + self.phase = ChannelPhase::Funded(funded_channel); res }, @@ -1941,6 +1967,7 @@ impl FundingScope { #[cfg(splicing)] struct PendingSplice { pub our_funding_contribution: i64, + funding: Option, } /// Contains everything about the channel including state, and various flags. @@ -6013,6 +6040,71 @@ impl FundedChannel where Ok(channel_monitor) } + #[cfg(splicing)] + pub fn splice_initial_commitment_signed( + &mut self, msg: &msgs::CommitmentSigned, logger: &L, + ) -> Result, ChannelError> + 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(), + )); + } + + let pending_splice_funding = self.pending_splice.as_ref() + .and_then(|pending_splice| pending_splice.funding.as_ref()) + .expect("Funding must exist for negotiated pending splice"); + + let holder_commitment_data = self.context.validate_commitment_signed( + pending_splice_funding, &self.holder_commitment_point, msg, logger, + )?; + + let counterparty_commitment_tx = self.context.build_commitment_transaction( + pending_splice_funding, self.context.cur_counterparty_commitment_transaction_number, + &self.context.counterparty_cur_commitment_point.unwrap(), false, false, logger, + ).tx; + + { + let counterparty_trusted_tx = counterparty_commitment_tx.trust(); + let counterparty_bitcoin_tx = counterparty_trusted_tx.built_transaction(); + log_trace!(logger, "Splice initial counterparty tx for channel {} is: txid {} tx {}", + &self.context.channel_id(), counterparty_bitcoin_tx.txid, + encode::serialize_hex(&counterparty_bitcoin_tx.transaction)); + } + + log_info!(logger, "Received splice initial commitment_signed from peer for channel {} with funding txid {}", + &self.context.channel_id(), pending_splice_funding.get_funding_txo().unwrap().txid); + + self.context.latest_monitor_update_id += 1; + let monitor_update = ChannelMonitorUpdate { + update_id: self.context.latest_monitor_update_id, + updates: vec![ChannelMonitorUpdateStep::RenegotiatedFunding { + channel_parameters: pending_splice_funding.channel_transaction_parameters.clone(), + holder_commitment_tx: holder_commitment_data.commitment_tx, + counterparty_commitment_tx, + }], + channel_id: Some(self.context.channel_id()), + }; + + let tx_signatures = self.interactive_tx_signing_session.as_mut() + .expect("Signing session must exist for negotiated pending splice") + .received_commitment_signed(); + self.context.monitor_pending_tx_signatures = tx_signatures; + + if self.context.channel_state.is_monitor_update_in_progress() { + // There may be a pending monitor update for a `revoke_and_ack` or preimage we have to + // handle first. + } else { + self.monitor_updating_paused(false, false, false, Vec::new(), Vec::new(), Vec::new()); + } + + Ok(self.push_ret_blockable_mon_update(monitor_update)) + } + pub fn commitment_signed(&mut self, msg: &msgs::CommitmentSigned, logger: &L) -> Result, ChannelError> where L::Target: Logger { @@ -8984,6 +9076,7 @@ impl FundedChannel where self.pending_splice = Some(PendingSplice { our_funding_contribution: our_funding_contribution_satoshis, + funding: None, }); let msg = self.get_splice_init(our_funding_contribution_satoshis, funding_feerate_per_kw, locktime); From 6b350f512fd3246943a32831b07a97fe6634f9d7 Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 12 Jun 2025 13:58:46 -0700 Subject: [PATCH 2/3] Introduce LatestHolderCommitment monitor update variant This new variant is a backwards-incompatible successor to `LatestHolderCommitmentTXInfo` that is capable of handling holder commitment updates while a splice is pending. Since all holder commitment candidates share the same set of non-dust and dust HTLCs (though each non-dust HTLC can have differing output indices), we can share the same set of HTLC source data across all candidates. --- lightning/src/chain/channelmonitor.rs | 111 +++++++++++++++++---- lightning/src/ln/channel.rs | 137 +++++++++++++++----------- 2 files changed, 167 insertions(+), 81 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index f6fcb3c3cef..420d4bbf6c5 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -586,6 +586,11 @@ pub(crate) enum ChannelMonitorUpdateStep { claimed_htlcs: Vec<(SentHTLCId, PaymentPreimage)>, nondust_htlc_sources: Vec, }, + LatestHolderCommitment { + commitment_txs: Vec, + htlc_data: CommitmentHTLCData, + claimed_htlcs: Vec<(SentHTLCId, PaymentPreimage)>, + }, LatestCounterpartyCommitmentTXInfo { commitment_txid: Txid, htlc_outputs: Vec<(HTLCOutputInCommitment, Option>)>, @@ -632,6 +637,7 @@ impl ChannelMonitorUpdateStep { fn variant_name(&self) -> &'static str { match self { ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { .. } => "LatestHolderCommitmentTXInfo", + ChannelMonitorUpdateStep::LatestHolderCommitment { .. } => "LatestHolderCommitment", ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. } => "LatestCounterpartyCommitmentTXInfo", ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { .. } => "LatestCounterpartyCommitmentTX", ChannelMonitorUpdateStep::PaymentPreimage { .. } => "PaymentPreimage", @@ -676,6 +682,10 @@ impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep, (6, LatestCounterpartyCommitmentTX) => { (0, htlc_outputs, required_vec), (2, commitment_tx, required), + (8, LatestHolderCommitment) => { + (1, commitment_txs, required_vec), + (3, htlc_data, required), + (5, claimed_htlcs, required_vec), }, (10, RenegotiatedFunding) => { (1, channel_parameters, (required: ReadableArgs, None)), @@ -932,12 +942,12 @@ impl Clone for ChannelMonitor where Signer: } } -#[derive(Clone, PartialEq)] -struct CommitmentHTLCData { +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct CommitmentHTLCData { // These must be sorted in increasing output index order to match the expected order of the // HTLCs in the `CommitmentTransaction`. - nondust_htlc_sources: Vec, - dust_htlcs: Vec<(HTLCOutputInCommitment, Option)>, + pub nondust_htlc_sources: Vec, + pub dust_htlcs: Vec<(HTLCOutputInCommitment, Option)>, } impl CommitmentHTLCData { @@ -946,6 +956,11 @@ impl CommitmentHTLCData { } } +impl_writeable_tlv_based!(CommitmentHTLCData, { + (1, nondust_htlc_sources, required_vec), + (3, dust_htlcs, required_vec), +}); + impl TryFrom for CommitmentHTLCData { type Error = (); fn try_from(value: HolderSignedTx) -> Result { @@ -3192,11 +3207,11 @@ impl ChannelMonitorImpl { /// up-to-date as our holder commitment transaction is updated. /// Panics if set_on_holder_tx_csv has never been called. fn provide_latest_holder_commitment_tx( - &mut self, mut holder_commitment_tx: HolderCommitmentTransaction, + &mut self, holder_commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option, Option)>, claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], mut nondust_htlc_sources: Vec, - ) { - let dust_htlcs: Vec<_> = if htlc_outputs.iter().any(|(_, s, _)| s.is_some()) { + ) -> Result<(), &'static str> { + let htlc_data = if htlc_outputs.iter().any(|(_, s, _)| s.is_some()) { // If we have non-dust HTLCs in htlc_outputs, ensure they match the HTLCs in the // `holder_commitment_tx`. In the future, we'll no longer provide the redundant data // and just pass in source data via `nondust_htlc_sources`. @@ -3224,7 +3239,7 @@ impl ChannelMonitorImpl { None }).collect(); - dust_htlcs + CommitmentHTLCData { nondust_htlc_sources, dust_htlcs } } else { // If we don't have any non-dust HTLCs in htlc_outputs, assume they were all passed via // `nondust_htlc_sources`, building up the final htlc_outputs by combining @@ -3251,30 +3266,67 @@ impl ChannelMonitorImpl { assert!(sources.next().is_none(), "All HTLC sources should have been exhausted"); // This only includes dust HTLCs as checked above. - htlc_outputs.into_iter().map(|(htlc, _, source)| (htlc, source)).collect() + let dust_htlcs = htlc_outputs.into_iter().map(|(htlc, _, source)| (htlc, source)).collect(); + + CommitmentHTLCData { nondust_htlc_sources, dust_htlcs } }; - self.current_holder_commitment_number = holder_commitment_tx.trust().commitment_number(); - self.onchain_tx_handler.provide_latest_holder_tx(holder_commitment_tx.clone()); + self.update_holder_commitment_data( + vec![holder_commitment_tx], htlc_data, claimed_htlcs.to_vec(), + ) + } - mem::swap(&mut holder_commitment_tx, &mut self.funding.current_holder_commitment_tx); - self.funding.prev_holder_commitment_tx = Some(holder_commitment_tx); - let mut holder_htlc_data = CommitmentHTLCData { nondust_htlc_sources, dust_htlcs }; - mem::swap(&mut holder_htlc_data, &mut self.current_holder_htlc_data); - self.prev_holder_htlc_data = Some(holder_htlc_data); + fn update_holder_commitment_data( + &mut self, mut commitment_txs: Vec, + mut htlc_data: CommitmentHTLCData, claimed_htlcs: Vec<(SentHTLCId, PaymentPreimage)>, + ) -> Result<(), &'static str> { + if self.pending_funding.len() + 1 != commitment_txs.len() { + return Err("Commitment transaction(s) mismatch"); + } + + let mut current_funding_commitment = commitment_txs.remove(0); + let holder_commitment_number = current_funding_commitment.commitment_number(); + for (pending_funding, mut commitment_tx) in self.pending_funding.iter_mut().zip(commitment_txs.into_iter()) { + let trusted_tx = commitment_tx.trust(); + if trusted_tx.commitment_number() != holder_commitment_number { + return Err("Commitment number mismatch"); + } + + let funding_outpoint_spent = + trusted_tx.built_transaction().transaction.tx_in(0).map(|input| input.previous_output).ok(); + let expected_funding_outpoint_spent = + pending_funding.channel_parameters.funding_outpoint.map(|op| op.into_bitcoin_outpoint()); + if funding_outpoint_spent != expected_funding_outpoint_spent { + return Err("Funding outpoint mismatch"); + } + + mem::swap(&mut commitment_tx, &mut pending_funding.current_holder_commitment_tx); + pending_funding.prev_holder_commitment_tx = Some(commitment_tx); + } + + self.current_holder_commitment_number = holder_commitment_number; + self.onchain_tx_handler.provide_latest_holder_tx(current_funding_commitment.clone()); + mem::swap(&mut current_funding_commitment, &mut self.funding.current_holder_commitment_tx); + self.funding.prev_holder_commitment_tx = Some(current_funding_commitment); + + mem::swap(&mut htlc_data, &mut self.current_holder_htlc_data); + self.prev_holder_htlc_data = Some(htlc_data); for (claimed_htlc_id, claimed_preimage) in claimed_htlcs { #[cfg(debug_assertions)] { let cur_counterparty_htlcs = self.funding.counterparty_claimable_outpoints.get( - &self.funding.current_counterparty_commitment_txid.unwrap()).unwrap(); + &self.funding.current_counterparty_commitment_txid.unwrap() + ).unwrap(); assert!(cur_counterparty_htlcs.iter().any(|(_, source_opt)| { if let Some(source) = source_opt { - SentHTLCId::from_source(source) == *claimed_htlc_id + SentHTLCId::from_source(source) == claimed_htlc_id } else { false } })); } - self.counterparty_fulfilled_htlcs.insert(*claimed_htlc_id, *claimed_preimage); + self.counterparty_fulfilled_htlcs.insert(claimed_htlc_id, claimed_preimage); } + + Ok(()) } /// Provides a payment_hash->payment_preimage mapping. Will be automatically pruned when all @@ -3579,9 +3631,25 @@ impl ChannelMonitorImpl { ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs, claimed_htlcs, nondust_htlc_sources } => { log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction info"); if self.lockdown_from_offchain { panic!(); } - self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone()); + if let Err(e) = self.provide_latest_holder_commitment_tx( + commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, + nondust_htlc_sources.clone() + ) { + log_error!(logger, "Failed updating latest holder commitment transaction info: {}", e); + } } - // Soon we will drop the `LatestCounterpartyCommitmentTXInfo` variant in favor of `LatestCounterpartyCommitmentTX`. + ChannelMonitorUpdateStep::LatestHolderCommitment { + commitment_txs, htlc_data, claimed_htlcs, + } => { + log_trace!(logger, "Updating ChannelMonitor with latest holder commitment"); + assert!(!self.lockdown_from_offchain); + if let Err(e) = self.update_holder_commitment_data( + commitment_txs.clone(), htlc_data.clone(), claimed_htlcs.clone(), + ) { + log_error!(logger, "Failed updating latest holder commitment state: {}", e); + } + }, + // Soon we will drop the `LatestCounterpartyCommitmentTXInfo` variant in favor of `LatestCounterpartyCommitment`. // For now we just add the code to handle the new updates. // Next step: in channel, switch channel monitor updates to use the `LatestCounterpartyCommitmentTX` variant. ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_per_commitment_point, .. } => { @@ -3664,6 +3732,7 @@ impl ChannelMonitorImpl { for update in updates.updates.iter() { match update { ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { .. } + |ChannelMonitorUpdateStep::LatestHolderCommitment { .. } |ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. } |ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { .. } |ChannelMonitorUpdateStep::ShutdownScript { .. } diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 93c42231e03..5fc9277e7db 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -55,7 +55,10 @@ use crate::ln::chan_utils; use crate::ln::onion_utils::{HTLCFailReason, LocalHTLCFailureReason, AttributionData}; use crate::chain::BestBlock; use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBoundedFeeEstimator, fee_for_weight}; -use crate::chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS}; +use crate::chain::channelmonitor::{ + ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, CommitmentHTLCData, + LATENCY_GRACE_PERIOD_BLOCKS, +}; use crate::chain::transaction::{OutPoint, TransactionData}; use crate::sign::ecdsa::EcdsaChannelSigner; use crate::sign::{EntropySource, ChannelSigner, SignerProvider, NodeSigner, Recipient}; @@ -3816,7 +3819,7 @@ impl ChannelContext where SP::Target: SignerProvider { fn validate_commitment_signed( &self, funding: &FundingScope, holder_commitment_point: &HolderCommitmentPoint, msg: &msgs::CommitmentSigned, logger: &L, - ) -> Result + ) -> Result<(HolderCommitmentTransaction, Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>), ChannelError> where L::Target: Logger, { @@ -3875,9 +3878,7 @@ impl ChannelContext where SP::Target: SignerProvider { } let holder_keys = commitment_data.tx.trust().keys(); - let mut nondust_htlc_sources = Vec::with_capacity(commitment_data.tx.nondust_htlcs().len()); - let mut dust_htlcs = Vec::with_capacity(commitment_data.htlcs_included.len() - commitment_data.tx.nondust_htlcs().len()); - for (idx, (htlc, mut source_opt)) in commitment_data.htlcs_included.into_iter().enumerate() { + for (idx, (htlc, _)) in commitment_data.htlcs_included.iter().enumerate() { if let Some(_) = htlc.transaction_output_index { let htlc_tx = chan_utils::build_htlc_transaction(&commitment_txid, commitment_data.tx.feerate_per_kw(), funding.get_counterparty_selected_contest_delay().unwrap(), &htlc, funding.get_channel_type(), @@ -3892,17 +3893,7 @@ impl ChannelContext where SP::Target: SignerProvider { if let Err(_) = self.secp_ctx.verify_ecdsa(&htlc_sighash, &msg.htlc_signatures[idx], &holder_keys.countersignatory_htlc_key.to_public_key()) { return Err(ChannelError::close("Invalid HTLC tx signature from peer".to_owned())); } - if htlc.offered { - if let Some(source) = source_opt.take() { - nondust_htlc_sources.push(source.clone()); - } else { - panic!("Missing outbound HTLC source"); - } - } - } else { - dust_htlcs.push((htlc, None, source_opt.take().cloned())); } - debug_assert!(source_opt.is_none(), "HTLCSource should have been put somewhere"); } let holder_commitment_tx = HolderCommitmentTransaction::new( @@ -3916,11 +3907,7 @@ impl ChannelContext where SP::Target: SignerProvider { self.holder_signer.as_ref().validate_holder_commitment(&holder_commitment_tx, commitment_data.outbound_htlc_preimages) .map_err(|_| ChannelError::close("Failed to validate our commitment".to_owned()))?; - Ok(LatestHolderCommitmentTXInfo { - commitment_tx: holder_commitment_tx, - htlc_outputs: dust_htlcs, - nondust_htlc_sources, - }) + Ok((holder_commitment_tx, commitment_data.htlcs_included)) } /// Builds stats on a potential commitment transaction build, without actually building the @@ -5250,14 +5237,6 @@ struct CommitmentTxInfoCached { feerate: u32, } -/// Partial data from ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo used to simplify the -/// return type of `ChannelContext::validate_commitment_signed`. -struct LatestHolderCommitmentTXInfo { - pub commitment_tx: HolderCommitmentTransaction, - pub htlc_outputs: Vec<(HTLCOutputInCommitment, Option, Option)>, - pub nondust_htlc_sources: Vec, -} - /// Contents of a wire message that fails an HTLC backwards. Useful for [`FundedChannel::fail_htlc`] to /// fail with either [`msgs::UpdateFailMalformedHTLC`] or [`msgs::UpdateFailHTLC`] as needed. trait FailHTLCContents { @@ -6105,21 +6084,50 @@ impl FundedChannel where Ok(self.push_ret_blockable_mon_update(monitor_update)) } + fn get_commitment_htlc_data( + htlcs_included: Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>, num_nondust: usize, + ) -> CommitmentHTLCData { + let mut nondust_htlc_sources = Vec::with_capacity(num_nondust); + let mut dust_htlcs = Vec::with_capacity(htlcs_included.len() - num_nondust); + for (htlc, mut source_opt) in htlcs_included.into_iter() { + if htlc.transaction_output_index.is_some() { + if htlc.offered { + if let Some(source) = source_opt.take() { + nondust_htlc_sources.push(source.clone()); + } else { + panic!("Missing outbound HTLC source"); + } + } + } else { + dust_htlcs.push((htlc, source_opt.take().cloned())); + } + debug_assert!(source_opt.is_none(), "HTLCSource should have been put somewhere"); + } + CommitmentHTLCData { nondust_htlc_sources, dust_htlcs } + } + pub fn commitment_signed(&mut self, msg: &msgs::CommitmentSigned, logger: &L) -> Result, ChannelError> where L::Target: Logger { self.commitment_signed_check_state()?; - let updates = self + let update = self .context .validate_commitment_signed(&self.funding, &self.holder_commitment_point, msg, logger) - .map(|LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs, nondust_htlc_sources }| - vec![ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { + .map(|(commitment_tx, htlcs_included)| { + let htlc_data = Self::get_commitment_htlc_data( + htlcs_included, commitment_tx.nondust_htlcs().len(), + ); + let htlc_outputs = htlc_data.dust_htlcs.into_iter() + .map(|(htlc, source)| (htlc, None, source)) + .collect(); + let nondust_htlc_sources = htlc_data.nondust_htlc_sources; + ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs, claimed_htlcs: vec![], nondust_htlc_sources, - }] - )?; + } + })?; - self.commitment_signed_update_monitor(updates, logger) + self.commitment_signed_update_monitor(update, logger) } pub fn commitment_signed_batch(&mut self, batch: Vec, logger: &L) -> Result, ChannelError> @@ -6146,25 +6154,29 @@ impl FundedChannel where // Any commitment_signed not associated with a FundingScope is ignored below if a // pending splice transaction has confirmed since receiving the batch. - let updates = core::iter::once(&self.funding) - .chain(self.pending_funding.iter()) - .map(|funding| { - let funding_txid = funding.get_funding_txo().unwrap().txid; - let msg = messages - .get(&funding_txid) - .ok_or_else(|| ChannelError::close(format!("Peer did not send a commitment_signed for pending splice transaction: {}", funding_txid)))?; - self.context - .validate_commitment_signed(funding, &self.holder_commitment_point, msg, logger) - .map(|LatestHolderCommitmentTXInfo { commitment_tx, htlc_outputs, nondust_htlc_sources }| - ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { - commitment_tx, htlc_outputs, claimed_htlcs: vec![], nondust_htlc_sources, - } - ) - } - ) - .collect::, ChannelError>>()?; + let mut commitment_txs = Vec::with_capacity(self.pending_funding.len() + 1); + let mut htlc_data = None; + for funding in core::iter::once(&self.funding).chain(self.pending_funding.iter()) { + let funding_txid = funding.get_funding_txo().unwrap().txid; + let msg = messages + .get(&funding_txid) + .ok_or_else(|| ChannelError::close(format!("Peer did not send a commitment_signed for pending splice transaction: {}", funding_txid)))?; + let (commitment_tx, htlcs_included) = self.context.validate_commitment_signed( + funding, &self.holder_commitment_point, msg, logger, + )?; + let num_nondust = commitment_tx.nondust_htlcs().len(); + commitment_txs.push(commitment_tx); + if htlc_data.is_none() { + htlc_data = Some(Self::get_commitment_htlc_data(htlcs_included, num_nondust)); + } + } - self.commitment_signed_update_monitor(updates, logger) + let update = ChannelMonitorUpdateStep::LatestHolderCommitment { + commitment_txs, + htlc_data: htlc_data.unwrap(), + claimed_htlcs: Vec::new(), + }; + self.commitment_signed_update_monitor(update, logger) } fn commitment_signed_check_state(&self) -> Result<(), ChannelError> { @@ -6184,7 +6196,7 @@ impl FundedChannel where Ok(()) } - fn commitment_signed_update_monitor(&mut self, mut updates: Vec, logger: &L) -> Result, ChannelError> + fn commitment_signed_update_monitor(&mut self, mut update: ChannelMonitorUpdateStep, logger: &L) -> Result, ChannelError> where L::Target: Logger { if self.holder_commitment_point.advance(&self.context.holder_signer, &self.context.secp_ctx, logger).is_err() { @@ -6240,21 +6252,26 @@ impl FundedChannel where } } - for mut update in updates.iter_mut() { - if let ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { + match &mut update { + ChannelMonitorUpdateStep::LatestHolderCommitment { claimed_htlcs: ref mut update_claimed_htlcs, .. - } = &mut update { + } => { debug_assert!(update_claimed_htlcs.is_empty()); *update_claimed_htlcs = claimed_htlcs.clone(); - } else { - debug_assert!(false); - } + }, + ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { + claimed_htlcs: ref mut update_claimed_htlcs, .. + } => { + debug_assert!(update_claimed_htlcs.is_empty()); + *update_claimed_htlcs = claimed_htlcs.clone(); + }, + _ => debug_assert!(false), } self.context.latest_monitor_update_id += 1; let mut monitor_update = ChannelMonitorUpdate { update_id: self.context.latest_monitor_update_id, - updates, + updates: vec![update], channel_id: Some(self.context.channel_id()), }; From 7f942f48c9076c0ad69d5d6296f424f2547298aa Mon Sep 17 00:00:00 2001 From: Wilmer Paulino Date: Thu, 12 Jun 2025 14:01:19 -0700 Subject: [PATCH 3/3] Introduce LatestCounterpartyCommitment monitor update variant This new variant is a backwards-incompatible successor to `LatestCounterpartyCommitmentTXInfo` that is capable of handling counterparty commitment updates while a splice is pending. Since all counterparty commitment candidates share the same set of non-dust and dust HTLCs (though each non-dust HTLC can have differing output indices), we can share the same set of HTLC source data across all candidates. Unfortunately, each `FundingScope` tracks its own set of `counterparty_claimable_outpoints`, which includes the HTLC source data (though only for the current and previous counterparty commitments), so we end up duplicating it (both in memory and on disk) temporarily. --- lightning/src/chain/channelmonitor.rs | 164 ++++++++++++++++++-------- lightning/src/ln/channel.rs | 74 ++++++++---- 2 files changed, 163 insertions(+), 75 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index 420d4bbf6c5..15d3dfaa30d 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -600,11 +600,9 @@ pub(crate) enum ChannelMonitorUpdateStep { to_broadcaster_value_sat: Option, to_countersignatory_value_sat: Option, }, - LatestCounterpartyCommitmentTX { - // The dust and non-dust htlcs for that commitment - htlc_outputs: Vec<(HTLCOutputInCommitment, Option>)>, - // Contains only the non-dust htlcs - commitment_tx: CommitmentTransaction, + LatestCounterpartyCommitment { + commitment_txs: Vec, + htlc_data: CommitmentHTLCData, }, PaymentPreimage { payment_preimage: PaymentPreimage, @@ -639,7 +637,7 @@ impl ChannelMonitorUpdateStep { ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { .. } => "LatestHolderCommitmentTXInfo", ChannelMonitorUpdateStep::LatestHolderCommitment { .. } => "LatestHolderCommitment", ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. } => "LatestCounterpartyCommitmentTXInfo", - ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { .. } => "LatestCounterpartyCommitmentTX", + ChannelMonitorUpdateStep::LatestCounterpartyCommitment { .. } => "LatestCounterpartyCommitment", ChannelMonitorUpdateStep::PaymentPreimage { .. } => "PaymentPreimage", ChannelMonitorUpdateStep::CommitmentSecret { .. } => "CommitmentSecret", ChannelMonitorUpdateStep::ChannelForceClosed { .. } => "ChannelForceClosed", @@ -679,9 +677,10 @@ impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep, (5, ShutdownScript) => { (0, scriptpubkey, required), }, - (6, LatestCounterpartyCommitmentTX) => { - (0, htlc_outputs, required_vec), - (2, commitment_tx, required), + (6, LatestCounterpartyCommitment) => { + (1, commitment_txs, required_vec), + (3, htlc_data, required), + }, (8, LatestHolderCommitment) => { (1, commitment_txs, required_vec), (3, htlc_data, required), @@ -1768,13 +1767,11 @@ impl ChannelMonitor { /// /// This is used to provide the counterparty commitment transaction directly to the monitor /// before the initial persistence of a new channel. - pub(crate) fn provide_initial_counterparty_commitment_tx( - &self, commitment_tx: CommitmentTransaction, logger: &L, - ) where L::Target: Logger - { + pub(crate) fn provide_initial_counterparty_commitment_tx( + &self, commitment_tx: CommitmentTransaction, + ) { let mut inner = self.inner.lock().unwrap(); - let logger = WithChannelMonitor::from_impl(logger, &*inner, None); - inner.provide_initial_counterparty_commitment_tx(commitment_tx, &logger); + inner.provide_initial_counterparty_commitment_tx(commitment_tx); } /// Informs this monitor of the latest counterparty (ie non-broadcastable) commitment transaction. @@ -1782,18 +1779,16 @@ impl ChannelMonitor { /// possibly future revocation/preimage information) to claim outputs where possible. /// We cache also the mapping hash:commitment number to lighten pruning of old preimages by watchtowers. #[cfg(test)] - fn provide_latest_counterparty_commitment_tx( + fn provide_latest_counterparty_commitment_tx( &self, txid: Txid, htlc_outputs: Vec<(HTLCOutputInCommitment, Option>)>, commitment_number: u64, their_per_commitment_point: PublicKey, - logger: &L, - ) where L::Target: Logger { + ) { let mut inner = self.inner.lock().unwrap(); - let logger = WithChannelMonitor::from_impl(logger, &*inner, None); inner.provide_latest_counterparty_commitment_tx( - txid, htlc_outputs, commitment_number, their_per_commitment_point, &logger) + txid, htlc_outputs, commitment_number, their_per_commitment_point) } #[cfg(test)] @@ -1801,7 +1796,9 @@ impl ChannelMonitor { &self, holder_commitment_tx: HolderCommitmentTransaction, htlc_outputs: Vec<(HTLCOutputInCommitment, Option, Option)>, ) { - self.inner.lock().unwrap().provide_latest_holder_commitment_tx(holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new()) + self.inner.lock().unwrap().provide_latest_holder_commitment_tx( + holder_commitment_tx, htlc_outputs, &Vec::new(), Vec::new() + ).unwrap() } /// This is used to provide payment preimage(s) out-of-band during startup without updating the @@ -3144,9 +3141,9 @@ impl ChannelMonitorImpl { Ok(()) } - fn provide_initial_counterparty_commitment_tx( - &mut self, commitment_tx: CommitmentTransaction, logger: &WithChannelMonitor, - ) where L::Target: Logger { + fn provide_initial_counterparty_commitment_tx( + &mut self, commitment_tx: CommitmentTransaction, + ) { // We populate this field for downgrades self.initial_counterparty_commitment_info = Some((commitment_tx.per_commitment_point(), commitment_tx.feerate_per_kw(), commitment_tx.to_broadcaster_value_sat(), commitment_tx.to_countersignatory_value_sat())); @@ -3157,16 +3154,15 @@ impl ChannelMonitorImpl { } self.provide_latest_counterparty_commitment_tx(commitment_tx.trust().txid(), Vec::new(), commitment_tx.commitment_number(), - commitment_tx.per_commitment_point(), logger); + commitment_tx.per_commitment_point()); // Soon, we will only populate this field self.initial_counterparty_commitment_tx = Some(commitment_tx); } - fn provide_latest_counterparty_commitment_tx( - &mut self, txid: Txid, - htlc_outputs: Vec<(HTLCOutputInCommitment, Option>)>, - commitment_number: u64, their_per_commitment_point: PublicKey, logger: &WithChannelMonitor, - ) where L::Target: Logger { + fn provide_latest_counterparty_commitment_tx( + &mut self, txid: Txid, htlc_outputs: Vec<(HTLCOutputInCommitment, Option>)>, + commitment_number: u64, their_per_commitment_point: PublicKey, + ) { // TODO: Encrypt the htlc_outputs data with the single-hash of the commitment transaction // so that a remote monitor doesn't learn anything unless there is a malicious close. // (only maybe, sadly we cant do the same for local info, as we need to be aware of @@ -3175,11 +3171,11 @@ impl ChannelMonitorImpl { self.counterparty_hash_commitment_number.insert(htlc.payment_hash, commitment_number); } - log_trace!(logger, "Tracking new counterparty commitment transaction with txid {} at commitment number {} with {} HTLC outputs", txid, commitment_number, htlc_outputs.len()); self.funding.prev_counterparty_commitment_txid = self.funding.current_counterparty_commitment_txid.take(); self.funding.current_counterparty_commitment_txid = Some(txid); self.funding.counterparty_claimable_outpoints.insert(txid, htlc_outputs.clone()); self.current_counterparty_commitment_number = commitment_number; + //TODO: Merge this into the other per-counterparty-transaction output storage stuff match self.their_cur_per_commitment_points { Some(old_points) => { @@ -3201,6 +3197,74 @@ impl ChannelMonitorImpl { } } + fn update_counterparty_commitment_data( + &mut self, commitment_txs: &[CommitmentTransaction], htlc_data: &CommitmentHTLCData, + ) -> Result<(), &'static str> { + if self.pending_funding.len() + 1 != commitment_txs.len() { + return Err("Commitment transaction(s) mismatch"); + } + + let htlcs_for_commitment = |commitment: &CommitmentTransaction| { + let mut nondust_htlcs = commitment.nondust_htlcs().clone().into_iter(); + let mut sources = htlc_data.nondust_htlc_sources.clone().into_iter(); + let nondust_htlcs = core::iter::from_fn(move || { + let htlc = if let Some(htlc) = nondust_htlcs.next() { + htlc + } else { + debug_assert!(sources.next().is_none()); + return None; + }; + + let mut source = None; + if htlc.offered { + source = Some(Box::new(sources.next().expect( + "Every offered non-dust HTLC should have a corresponding source" + ))); + } + + Some((htlc, source)) + }); + + let dust_htlcs = htlc_data.dust_htlcs.clone().into_iter().map(|(htlc, source)| { + (htlc, source.map(|source| Box::new(source))) + }); + + nondust_htlcs.chain(dust_htlcs).collect::>() + }; + + let current_funding_commitment = commitment_txs.first().unwrap(); + for (pending_funding, commitment_tx) in self.pending_funding.iter_mut().zip(commitment_txs.iter()) { + let trusted_tx = commitment_tx.trust(); + if trusted_tx.commitment_number() != current_funding_commitment.commitment_number() { + return Err("Commitment number mismatch"); + } + + let funding_outpoint_spent = + trusted_tx.built_transaction().transaction.tx_in(0).map(|input| input.previous_output).ok(); + let expected_funding_outpoint_spent = + pending_funding.channel_parameters.funding_outpoint.map(|op| op.into_bitcoin_outpoint()); + if funding_outpoint_spent != expected_funding_outpoint_spent { + return Err("Funding outpoint mismatch"); + } + + pending_funding.prev_counterparty_commitment_txid = + pending_funding.current_counterparty_commitment_txid.take(); + pending_funding.current_counterparty_commitment_txid = Some(trusted_tx.txid()); + pending_funding.counterparty_claimable_outpoints.insert( + trusted_tx.txid(), htlcs_for_commitment(commitment_tx), + ); + } + + self.provide_latest_counterparty_commitment_tx( + current_funding_commitment.trust().txid(), + htlcs_for_commitment(current_funding_commitment), + current_funding_commitment.commitment_number(), + current_funding_commitment.per_commitment_point(), + ); + + Ok(()) + } + /// Informs this monitor of the latest holder (ie broadcastable) commitment transaction. The /// monitor watches for timeouts and may broadcast it if we approach such a timeout. Thus, it /// is important that any clones of this channel monitor (including remote clones) by kept @@ -3651,14 +3715,18 @@ impl ChannelMonitorImpl { }, // Soon we will drop the `LatestCounterpartyCommitmentTXInfo` variant in favor of `LatestCounterpartyCommitment`. // For now we just add the code to handle the new updates. - // Next step: in channel, switch channel monitor updates to use the `LatestCounterpartyCommitmentTX` variant. + // Next step: in channel, switch channel monitor updates to use the `LatestCounterpartyCommitment` variant. ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, htlc_outputs, commitment_number, their_per_commitment_point, .. } => { log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info"); - self.provide_latest_counterparty_commitment_tx(*commitment_txid, htlc_outputs.clone(), *commitment_number, *their_per_commitment_point, logger) + self.provide_latest_counterparty_commitment_tx(*commitment_txid, htlc_outputs.clone(), *commitment_number, *their_per_commitment_point) }, - ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { htlc_outputs, commitment_tx } => { - log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction info"); - self.provide_latest_counterparty_commitment_tx(commitment_tx.trust().txid(), htlc_outputs.clone(), commitment_tx.commitment_number(), commitment_tx.per_commitment_point(), logger) + ChannelMonitorUpdateStep::LatestCounterpartyCommitment { + commitment_txs, htlc_data, + } => { + log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment"); + if let Err(e) = self.update_counterparty_commitment_data(commitment_txs, htlc_data) { + log_error!(logger, "Failed updating latest counterparty commitment state: {}", e); + } }, ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage, payment_info } => { log_trace!(logger, "Updating ChannelMonitor with payment preimage"); @@ -3734,7 +3802,7 @@ impl ChannelMonitorImpl { ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { .. } |ChannelMonitorUpdateStep::LatestHolderCommitment { .. } |ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. } - |ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { .. } + |ChannelMonitorUpdateStep::LatestCounterpartyCommitment { .. } |ChannelMonitorUpdateStep::ShutdownScript { .. } |ChannelMonitorUpdateStep::CommitmentSecret { .. } |ChannelMonitorUpdateStep::RenegotiatedFunding { .. } => @@ -3890,7 +3958,7 @@ impl ChannelMonitorImpl { update.updates.iter().filter_map(|update| { // Soon we will drop the first branch here in favor of the second. // In preparation, we just add the second branch without deleting the first. - // Next step: in channel, switch channel monitor updates to use the `LatestCounterpartyCommitmentTX` variant. + // Next step: in channel, switch channel monitor updates to use the `LatestCounterpartyCommitment` variant. match update { &ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, ref htlc_outputs, commitment_number, their_per_commitment_point, @@ -3908,19 +3976,17 @@ impl ChannelMonitorImpl { debug_assert_eq!(commitment_tx.trust().txid(), commitment_txid); - Some(commitment_tx) + Some(vec![commitment_tx]) }, - &ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { - htlc_outputs: _, ref commitment_tx, - } => { - Some(commitment_tx.clone()) + &ChannelMonitorUpdateStep::LatestCounterpartyCommitment { ref commitment_txs, .. } => { + Some(commitment_txs.clone()) }, &ChannelMonitorUpdateStep::RenegotiatedFunding { ref counterparty_commitment_tx, .. } => { - Some(counterparty_commitment_tx.clone()) + Some(vec![counterparty_commitment_tx.clone()]) }, _ => None, } - }).collect() + }).flatten().collect() } fn sign_to_local_justice_tx( @@ -5988,9 +6054,9 @@ mod tests { monitor.provide_latest_holder_commitment_tx(dummy_commitment_tx.clone(), nondust_htlcs.into_iter().map(|htlc| (htlc.clone(), Some(dummy_sig), Some(dummy_source.clone()))).collect()); monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"1").to_byte_array()), - preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key, &logger); + preimages_slice_to_htlc_outputs!(preimages[5..15]), 281474976710655, dummy_key); monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"2").to_byte_array()), - preimages_slice_to_htlc_outputs!(preimages[15..20]), 281474976710654, dummy_key, &logger); + preimages_slice_to_htlc_outputs!(preimages[15..20]), 281474976710654, dummy_key); for &(ref preimage, ref hash) in preimages.iter() { let bounded_fee_estimator = LowerBoundedFeeEstimator::new(&fee_estimator); monitor.provide_payment_preimage_unsafe_legacy( @@ -6007,7 +6073,7 @@ mod tests { test_preimages_exist!(&preimages[15..20], monitor); monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"3").to_byte_array()), - preimages_slice_to_htlc_outputs!(preimages[17..20]), 281474976710653, dummy_key, &logger); + preimages_slice_to_htlc_outputs!(preimages[17..20]), 281474976710653, dummy_key); // Now provide a further secret, pruning preimages 15-17 secret[0..32].clone_from_slice(&>::from_hex("c7518c8ae4660ed02894df8976fa1a3659c1a8b4b5bec0c4b872abeba4cb8964").unwrap()); @@ -6017,7 +6083,7 @@ mod tests { test_preimages_exist!(&preimages[17..20], monitor); monitor.provide_latest_counterparty_commitment_tx(Txid::from_byte_array(Sha256::hash(b"4").to_byte_array()), - preimages_slice_to_htlc_outputs!(preimages[18..20]), 281474976710652, dummy_key, &logger); + preimages_slice_to_htlc_outputs!(preimages[18..20]), 281474976710652, dummy_key); // Now update holder commitment tx info, pruning only element 18 as we still care about the // previous commitment tx's preimages too diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 5fc9277e7db..2d65268923a 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -2370,7 +2370,7 @@ trait InitialRemoteCommitmentReceiver where SP::Target: SignerProvide holder_commitment_tx, best_block, context.counterparty_node_id, context.channel_id(), ); channel_monitor.provide_initial_counterparty_commitment_tx( - counterparty_initial_commitment_tx.clone(), logger, + counterparty_initial_commitment_tx.clone(), ); self.context_mut().cur_counterparty_commitment_transaction_number -= 1; @@ -9379,32 +9379,54 @@ impl FundedChannel where } self.context.resend_order = RAACommitmentOrder::RevokeAndACKFirst; - let mut updates = Vec::with_capacity(self.pending_funding.len() + 1); - for funding in core::iter::once(&self.funding).chain(self.pending_funding.iter()) { + let update = if self.pending_funding.is_empty() { let (htlcs_ref, counterparty_commitment_tx) = - self.build_commitment_no_state_update(funding, logger); - let htlc_outputs: Vec<(HTLCOutputInCommitment, Option>)> = - htlcs_ref.into_iter().map(|(htlc, htlc_source)| (htlc, htlc_source.map(|source_ref| Box::new(source_ref.clone())))).collect(); - - if self.pending_funding.is_empty() { - // Soon, we will switch this to `LatestCounterpartyCommitmentTX`, - // and provide the full commit tx instead of the information needed to rebuild it. - updates.push(ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { - commitment_txid: counterparty_commitment_tx.trust().txid(), - htlc_outputs, - commitment_number: self.context.cur_counterparty_commitment_transaction_number, - their_per_commitment_point: self.context.counterparty_cur_commitment_point.unwrap(), - feerate_per_kw: Some(counterparty_commitment_tx.feerate_per_kw()), - to_broadcaster_value_sat: Some(counterparty_commitment_tx.to_broadcaster_value_sat()), - to_countersignatory_value_sat: Some(counterparty_commitment_tx.to_countersignatory_value_sat()), - }); - } else { - updates.push(ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { - htlc_outputs, - commitment_tx: counterparty_commitment_tx, - }); + self.build_commitment_no_state_update(&self.funding, logger); + let htlc_outputs = htlcs_ref.into_iter() + .map(|(htlc, htlc_source)| ( + htlc, htlc_source.map(|source_ref| Box::new(source_ref.clone())) + )) + .collect(); + + // Soon, we will switch this to `LatestCounterpartyCommitment`, + // and provide the full commit tx instead of the information needed to rebuild it. + ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { + commitment_txid: counterparty_commitment_tx.trust().txid(), + htlc_outputs, + commitment_number: self.context.cur_counterparty_commitment_transaction_number, + their_per_commitment_point: self.context.counterparty_cur_commitment_point.unwrap(), + feerate_per_kw: Some(counterparty_commitment_tx.feerate_per_kw()), + to_broadcaster_value_sat: Some(counterparty_commitment_tx.to_broadcaster_value_sat()), + to_countersignatory_value_sat: Some(counterparty_commitment_tx.to_countersignatory_value_sat()), } - } + } else { + let mut htlc_data = None; + let commitment_txs = core::iter::once(&self.funding) + .chain(self.pending_funding.iter()) + .map(|funding| { + let (htlcs_ref, counterparty_commitment_tx) = + self.build_commitment_no_state_update(funding, logger); + if htlc_data.is_none() { + let nondust_htlc_sources = htlcs_ref.iter() + // We check !offered as this is the HTLC from the counterparty's point of view. + .filter(|(htlc, _)| !htlc.offered && htlc.transaction_output_index.is_some()) + .map(|(_, source)| source.expect("Outbound HTLC must have a source").clone()) + .collect(); + let dust_htlcs = htlcs_ref.into_iter() + .filter(|(htlc, _)| htlc.transaction_output_index.is_none()) + .map(|(htlc, source)| (htlc, source.cloned())) + .collect(); + htlc_data = Some(CommitmentHTLCData { + nondust_htlc_sources, + dust_htlcs, + }); + } + counterparty_commitment_tx + }) + .collect(); + let htlc_data = htlc_data.unwrap(); + ChannelMonitorUpdateStep::LatestCounterpartyCommitment { commitment_txs, htlc_data } + }; if self.context.announcement_sigs_state == AnnouncementSigsState::MessageSent { self.context.announcement_sigs_state = AnnouncementSigsState::Committed; @@ -9413,7 +9435,7 @@ impl FundedChannel where self.context.latest_monitor_update_id += 1; let monitor_update = ChannelMonitorUpdate { update_id: self.context.latest_monitor_update_id, - updates, + updates: vec![update], channel_id: Some(self.context.channel_id()), }; self.context.channel_state.set_awaiting_remote_revoke();