Produce a single monitor update per commitment number

tankyleo · tankyleo · commit 8ac035835d09 · 2025-04-16T03:01:10.000Z
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.
diff --git 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<u64>,
 		to_countersignatory_value_sat: Option<u64>,
 	},
-	LatestCounterpartyCommitmentTX {
+	LatestHolderCommitmentTXs {
 		// The dust and non-dust htlcs for that commitment
-		htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>,
+		htlc_data: Vec<(HTLCOutputData, Option<Box<HTLCSource>>)>,
+		// Contains only the non-dust htlcs
+		commitment_txs: Vec<HolderCommitmentTransaction>,
+		claimed_htlcs: Vec<(SentHTLCId, PaymentPreimage)>,
+	},
+	LatestCounterpartyCommitmentTXs {
+		// The dust and non-dust htlcs for that commitment
+		htlc_data: Vec<(HTLCOutputData, Option<Box<HTLCSource>>)>,
 		// Contains only the non-dust htlcs
-		commitment_tx: CommitmentTransaction,
+		commitment_txs: Vec<CommitmentTransaction>,
 	},
 	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<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		}
 	}
 
+	// 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<L: Deref>(
+		&mut self,
+		_htlc_data: Vec<(HTLCOutputData, Option<Box<HTLCSource>>)>,
+		_txs: &Vec<CommitmentTransaction>,
+		_logger: &WithChannelMonitor<L>,
+	) 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<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		}
 	}
 
+	// 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<Box<HTLCSource>>)>,
+		_holder_commitment_txs: Vec<HolderCommitmentTransaction>,
+		_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<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 					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<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			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<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		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<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 
 					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
@@ -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
@@ -9013,31 +9013,32 @@ impl<SP: Deref> FundedChannel<SP> 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<Box<HTLCSource>>)> =
 				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 {
