Produce a single monitor update per commitment number

tankyleo · tankyleo · commit 2adce747a523 · 2025-04-15T16:31:25.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,16 @@ pub(crate) enum ChannelMonitorUpdateStep {
 		to_broadcaster_value_sat: Option<u64>,
 		to_countersignatory_value_sat: Option<u64>,
 	},
-	LatestCounterpartyCommitmentTX {
+	LatestHolderCommitmentTXs {
+		htlc_data: Vec<(HTLCOutputData, Option<HTLCSource>)>,
+		commitment_txs: Vec<HolderCommitmentTransaction>,
+		claimed_htlcs: Vec<(SentHTLCId, PaymentPreimage)>,
+	},
+	LatestCounterpartyCommitmentTXs {
 		// 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_tx: CommitmentTransaction,
+		commitment_txs: Vec<CommitmentTransaction>,
 	},
 	PaymentPreimage {
 		payment_preimage: PaymentPreimage,
@@ -624,7 +629,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 +669,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) => {
+		(0, htlc_data, required_vec),
+		(2, commitment_txs, required_vec),
+		(4, claimed_htlcs, required_vec),
+	},
+	(7, LatestCounterpartyCommitmentTXs) => {
+		(0, htlc_data, required_vec),
+		(2, commitment_txs, required_vec),
 	},
 );
 
@@ -3084,6 +3095,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 +3199,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<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.
 	///
@@ -3399,9 +3436,14 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 					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 +3507,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,
@@ -3638,16 +3681,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,32 @@ pub fn get_counterparty_payment_script(channel_type_features: &ChannelTypeFeatur
 	}
 }
 
+/// Information about an HTLC that is generic over multiple commitment transactions
+/// for a commitment number
+#[derive(Clone, Debug, PartialEq, Eq)]
+pub struct HTLCOutputData {
+	/// Whether the HTLC was "offered" (ie outbound in relation to this commitment transaction).
+	/// Note that this is not the same as whether it is ountbound *from us*. To determine that you
+	/// need to compare this value to whether the commitment transaction in question is that of
+	/// the counterparty or our own.
+	pub offered: bool,
+	/// The value, in msat, of the HTLC. The value as it appears in the commitment transaction is
+	/// this divided by 1000.
+	pub amount_msat: u64,
+	/// The CLTV lock-time at which this HTLC expires.
+	pub cltv_expiry: u32,
+	/// The hash of the preimage which unlocks this HTLC.
+	pub payment_hash: PaymentHash,
+	// Do we want anything else ? the HTLC's uni-directional ID ?
+}
+
+impl_writeable_tlv_based!(HTLCOutputData, {
+	(0, offered, required),
+	(2, amount_msat, required),
+	(4, cltv_expiry, required),
+	(6, 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,29 @@ 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 `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 {
+			// TODO(splicing): The pending splices case will be addressed later
+			todo!();
 		}
 
 		if self.context.announcement_sigs_state == AnnouncementSigsState::MessageSent {
