From 8ac035835d092add139e799fb9f05efeb6351354 Mon Sep 17 00:00:00 2001 From: Leo Nash Date: Fri, 11 Apr 2025 03:04:42 +0000 Subject: [PATCH] Produce a single monitor update per commitment number Such updates will contain a single HTLC-source table for that commitment number, together with a vector of commitment transactions created at that commitment number. This vector will grow with the number of pending splices for a channel. The HTLC-source table produced by channel will stop storing information on which HTLCs were assigned which output index. Instead, this information will be stored in each commitment transaction in the monitor update. This commit deduplicates the `HTLCSource` collection for each commitment number, but `HTLCOutputData` would store information already stored in each `CommitmentTransaction` in a monitor update. This is a partial rework of b8a03cd. That commit has not been shipped in a release yet. --- lightning/src/chain/channelmonitor.rs | 85 ++++++++++++++++++++------- lightning/src/ln/chan_utils.rs | 19 ++++++ lightning/src/ln/channel.rs | 43 +++++++------- 3 files changed, 106 insertions(+), 41 deletions(-) diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs index a74cb7d7685..6b1278e24eb 100644 --- a/lightning/src/chain/channelmonitor.rs +++ b/lightning/src/chain/channelmonitor.rs @@ -38,7 +38,7 @@ use crate::types::features::ChannelTypeFeatures; use crate::types::payment::{PaymentHash, PaymentPreimage}; use crate::ln::msgs::DecodeError; use crate::ln::channel_keys::{DelayedPaymentKey, DelayedPaymentBasepoint, HtlcBasepoint, HtlcKey, RevocationKey, RevocationBasepoint}; -use crate::ln::chan_utils::{self,CommitmentTransaction, CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLCClaim, ChannelTransactionParameters, HolderCommitmentTransaction}; +use crate::ln::chan_utils::{self,CommitmentTransaction, CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HTLCClaim, ChannelTransactionParameters, HolderCommitmentTransaction, HTLCOutputData}; use crate::ln::channelmanager::{HTLCSource, SentHTLCId, PaymentClaimDetails}; use crate::chain; use crate::chain::{BestBlock, WatchedOutput}; @@ -591,11 +591,18 @@ pub(crate) enum ChannelMonitorUpdateStep { to_broadcaster_value_sat: Option, to_countersignatory_value_sat: Option, }, - LatestCounterpartyCommitmentTX { + LatestHolderCommitmentTXs { // The dust and non-dust htlcs for that commitment - htlc_outputs: Vec<(HTLCOutputInCommitment, Option>)>, + htlc_data: Vec<(HTLCOutputData, Option>)>, + // Contains only the non-dust htlcs + commitment_txs: Vec, + claimed_htlcs: Vec<(SentHTLCId, PaymentPreimage)>, + }, + LatestCounterpartyCommitmentTXs { + // The dust and non-dust htlcs for that commitment + htlc_data: Vec<(HTLCOutputData, Option>)>, // Contains only the non-dust htlcs - commitment_tx: CommitmentTransaction, + commitment_txs: Vec, }, PaymentPreimage { payment_preimage: PaymentPreimage, @@ -624,7 +631,8 @@ impl ChannelMonitorUpdateStep { match self { ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { .. } => "LatestHolderCommitmentTXInfo", ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. } => "LatestCounterpartyCommitmentTXInfo", - ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { .. } => "LatestCounterpartyCommitmentTX", + ChannelMonitorUpdateStep::LatestHolderCommitmentTXs { .. } => "LatestHolderCommitmentTXs", + ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXs { .. } => "LatestCounterpartyCommitmentTXs", ChannelMonitorUpdateStep::PaymentPreimage { .. } => "PaymentPreimage", ChannelMonitorUpdateStep::CommitmentSecret { .. } => "CommitmentSecret", ChannelMonitorUpdateStep::ChannelForceClosed { .. } => "ChannelForceClosed", @@ -663,9 +671,14 @@ impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep, (5, ShutdownScript) => { (0, scriptpubkey, required), }, - (6, LatestCounterpartyCommitmentTX) => { - (0, htlc_outputs, required_vec), - (2, commitment_tx, required), + (6, LatestHolderCommitmentTXs) => { // Added in 0.2 + (1, htlc_data, required_vec), + (3, commitment_txs, required_vec), + (5, claimed_htlcs, required_vec), + }, + (8, LatestCounterpartyCommitmentTXs) => { // Added in 0.2 + (1, htlc_data, required_vec), + (3, commitment_txs, required_vec), }, ); @@ -3084,6 +3097,19 @@ impl ChannelMonitorImpl { } } + // This is the sibling of `provide_latest_counterparty_commitment_tx`, but updated for a world + // in which the HTLC-source table passed from channel does NOT store dust-vs-nondust and + // index HTLC data. + fn provide_latest_counterparty_commitment_data( + &mut self, + _htlc_data: Vec<(HTLCOutputData, Option>)>, + _txs: &Vec, + _logger: &WithChannelMonitor, + ) where L::Target: Logger { + // TODO(splicing, 0.2): Populate this monitor's data structures + todo!(); + } + /// 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 @@ -3175,6 +3201,19 @@ impl ChannelMonitorImpl { } } + // This is the sibling of `provide_latest_holder_commitment_tx`, but updated for a world + // in which the HTLC-source table passed from channel does NOT store dust-vs-nondust and + // index HTLC data. + fn provide_latest_holder_commitment_data( + &mut self, + _htlc_data: Vec<(HTLCOutputData, Option>)>, + _holder_commitment_txs: Vec, + _claimed_htlcs: &[(SentHTLCId, PaymentPreimage)], + ) { + // TODO(splicing, 0.2): Populate this monitor's data structures + todo!(); + } + /// Provides a payment_hash->payment_preimage mapping. Will be automatically pruned when all /// commitment_tx_infos which contain the payment hash have been revoked. /// @@ -3392,16 +3431,21 @@ impl ChannelMonitorImpl { if self.lockdown_from_offchain { panic!(); } self.provide_latest_holder_commitment_tx(commitment_tx.clone(), htlc_outputs.clone(), &claimed_htlcs, nondust_htlc_sources.clone()); } - // Soon we will drop the `LatestCounterpartyCommitmentTXInfo` variant in favor of `LatestCounterpartyCommitmentTX`. + // Soon we will drop the `LatestCounterpartyCommitmentTXInfo` variant in favor of `LatestCounterpartyCommitmentTXs`. // 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 `LatestCounterpartyCommitmentTXs` 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) }, - 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::LatestHolderCommitmentTXs { htlc_data, commitment_txs, claimed_htlcs } => { + log_trace!(logger, "Updating ChannelMonitor with latest holder commitment transaction(s)"); + if self.lockdown_from_offchain { panic!(); } + self.provide_latest_holder_commitment_data(htlc_data.clone(), commitment_txs.clone(), claimed_htlcs) + } + ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXs { htlc_data, commitment_txs } => { + log_trace!(logger, "Updating ChannelMonitor with latest counterparty commitment transaction(s)"); + self.provide_latest_counterparty_commitment_data(htlc_data.clone(), commitment_txs, logger) }, ChannelMonitorUpdateStep::PaymentPreimage { payment_preimage, payment_info } => { log_trace!(logger, "Updating ChannelMonitor with payment preimage"); @@ -3465,7 +3509,8 @@ impl ChannelMonitorImpl { match update { ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { .. } |ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. } - |ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { .. } + |ChannelMonitorUpdateStep::LatestHolderCommitmentTXs { .. } + |ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXs { .. } |ChannelMonitorUpdateStep::ShutdownScript { .. } |ChannelMonitorUpdateStep::CommitmentSecret { .. } => is_pre_close_update = true, @@ -3620,7 +3665,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 `LatestCounterpartyCommitmentTXs` variant. match update { &ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid, ref htlc_outputs, commitment_number, their_per_commitment_point, @@ -3638,16 +3683,16 @@ 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, + &ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXs { + htlc_data: _, ref commitment_txs, } => { - Some(commitment_tx.clone()) + Some(commitment_txs.clone()) }, _ => None, } - }).collect() + }).flatten().collect() } fn sign_to_local_justice_tx( diff --git a/lightning/src/ln/chan_utils.rs b/lightning/src/ln/chan_utils.rs index c978680a620..227536e3f8c 100644 --- a/lightning/src/ln/chan_utils.rs +++ b/lightning/src/ln/chan_utils.rs @@ -586,6 +586,25 @@ pub fn get_counterparty_payment_script(channel_type_features: &ChannelTypeFeatur } } +/// The subset of the fields in `HTLCOutputInCommitment` that is constant across multiple +/// commitment transactions for a commitment number (ie. splices). Therefore, this struct does not +/// store information on whether the HTLC is dust or non-dust, and if non-dust, which index in the +/// commitment transaction the HTLC got assigned to. +#[derive(Clone, Debug, PartialEq, Eq)] +pub(crate) struct HTLCOutputData { + pub(crate) offered: bool, + pub(crate) amount_msat: u64, + pub(crate) cltv_expiry: u32, + pub(crate) payment_hash: PaymentHash, +} + +impl_writeable_tlv_based!(HTLCOutputData, { + (1, offered, required), + (3, amount_msat, required), + (5, cltv_expiry, required), + (7, payment_hash, required), +}); + /// Information about an HTLC as it appears in a commitment transaction #[derive(Clone, Debug, PartialEq, Eq)] pub struct HTLCOutputInCommitment { diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 4b60b83aaf1..bd14313030b 100644 --- a/lightning/src/ln/channel.rs +++ b/lightning/src/ln/channel.rs @@ -9013,31 +9013,32 @@ 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()) { + // Even in the pending splices case, we will send a single update for each commitment number. + // In that case, the update will contain multiple commitment transactions. + let mut updates = vec![]; + if self.pending_funding.is_empty() { let (htlcs_ref, counterparty_commitment_tx) = - self.build_commitment_no_state_update(funding, logger); + self.build_commitment_no_state_update(&self.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, - }); - } + // Soon, we will switch this to `LatestCounterpartyCommitmentTXs`, + // 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 { + // TODO(splicing): Build the HTLC-source table once per commitment number, and then + // build all the commitment transactions for that commitment number against that same + // table. Use `LatestCounterpartyCommitmentTXs` and `LatestHolderCommitmentTXs` to + // communicate these to `ChannelMonitor`. + todo!(); } if self.context.announcement_sigs_state == AnnouncementSigsState::MessageSent {