Provide built commitment transactions to ChannelMonitor

tankyleo · tankyleo · commit 266864274c5d · 2025-03-05T19:04:40.000Z
Prior to this commit, `LatestCounterpartyCommitmentTXInfo` provided
`ChannelMonitor` with the data it needed to build counterparty
commitment transactions, instead of the fully built transaction.

This was an unnecessary optimization, as the data still included all
the htlcs. This meant that the size of
`LatestCounterpartyCommitmentTXInfo` scaled with the number of
htlcs, just like a fully built transaction.

This commit adds a new variant to `ChannelMonitorUpdateStep` called
`LatestCounterpartyCommitmentTX`, which will provide the commitment
transaction, together with the dust and non-dust htlcs for that
commitment.

We add code to handle this new variant, but refrain from
immediately setting it, as this would otherwise double the size of our
updates.
diff --git a/lightning/src/chain/channelmonitor.rs b/lightning/src/chain/channelmonitor.rs
@@ -549,6 +549,12 @@ pub(crate) enum ChannelMonitorUpdateStep {
 		to_broadcaster_value_sat: Option<u64>,
 		to_countersignatory_value_sat: Option<u64>,
 	},
+	LatestCounterpartyCommitmentTX {
+		// The dust and non-dust htlcs for that commitment
+		htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>,
+		// Contains only the non-dust htlcs
+		commitment_tx: CommitmentTransaction,
+	},
 	PaymentPreimage {
 		payment_preimage: PaymentPreimage,
 		/// If this preimage was from an inbound payment claim, information about the claim should
@@ -576,6 +582,7 @@ impl ChannelMonitorUpdateStep {
 		match self {
 			ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { .. } => "LatestHolderCommitmentTXInfo",
 			ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. } => "LatestCounterpartyCommitmentTXInfo",
+			ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { .. } => "LatestCounterpartyCommitmentTX",
 			ChannelMonitorUpdateStep::PaymentPreimage { .. } => "PaymentPreimage",
 			ChannelMonitorUpdateStep::CommitmentSecret { .. } => "CommitmentSecret",
 			ChannelMonitorUpdateStep::ChannelForceClosed { .. } => "ChannelForceClosed",
@@ -614,6 +621,10 @@ impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep,
 	(5, ShutdownScript) => {
 		(0, scriptpubkey, required),
 	},
+	(6, LatestCounterpartyCommitmentTX) => {
+		(0, htlc_outputs, required_vec),
+		(2, commitment_tx, required),
+	},
 );
 
 /// Indicates whether the balance is derived from a cooperative close, a force-close
@@ -1024,6 +1035,11 @@ pub(crate) struct ChannelMonitorImpl<Signer: EcdsaChannelSigner> {
 	/// Ordering of tuple data: (their_per_commitment_point, feerate_per_kw, to_broadcaster_sats,
 	/// to_countersignatory_sats)
 	initial_counterparty_commitment_info: Option<(PublicKey, u32, u64, u64)>,
+	/// Initial counterparty commitment transaction
+	///
+	/// We previously used the field above to re-build the counterparty commitment transaction,
+	/// we now provide the transaction outright.
+	initial_counterparty_commitment_tx: Option<CommitmentTransaction>,
 
 	/// The first block height at which we had no remaining claimable balances.
 	balances_empty_height: Option<u32>,
@@ -1247,6 +1263,7 @@ impl<Signer: EcdsaChannelSigner> Writeable for ChannelMonitorImpl<Signer> {
 			(23, self.holder_pays_commitment_tx_fee, option),
 			(25, self.payment_preimages, required),
 			(27, self.first_confirmed_funding_txo, required),
+			(29, self.initial_counterparty_commitment_tx, option),
 		});
 
 		Ok(())
@@ -1458,6 +1475,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 			best_block,
 			counterparty_node_id: Some(counterparty_node_id),
 			initial_counterparty_commitment_info: None,
+			initial_counterparty_commitment_tx: None,
 			balances_empty_height: None,
 
 			failed_back_htlc_ids: new_hash_set(),
@@ -1497,17 +1515,12 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitor<Signer> {
 	/// This is used to provide the counterparty commitment information directly to the monitor
 	/// before the initial persistence of a new channel.
 	pub(crate) fn provide_initial_counterparty_commitment_tx<L: Deref>(
-		&self, txid: Txid, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>,
-		commitment_number: u64, their_cur_per_commitment_point: PublicKey, feerate_per_kw: u32,
-		to_broadcaster_value_sat: u64, to_countersignatory_value_sat: u64, logger: &L,
-	)
-	where L::Target: Logger
+		&self, commitment_tx: CommitmentTransaction, logger: &L,
+	) where L::Target: Logger
 	{
 		let mut inner = self.inner.lock().unwrap();
 		let logger = WithChannelMonitor::from_impl(logger, &*inner, None);
-		inner.provide_initial_counterparty_commitment_tx(txid,
-			htlc_outputs, commitment_number, their_cur_per_commitment_point, feerate_per_kw,
-			to_broadcaster_value_sat, to_countersignatory_value_sat, &logger);
+		inner.provide_initial_counterparty_commitment_tx(commitment_tx, &logger);
 	}
 
 	/// Informs this monitor of the latest counterparty (ie non-broadcastable) commitment transaction.
@@ -2879,20 +2892,21 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 	}
 
 	fn provide_initial_counterparty_commitment_tx<L: Deref>(
-		&mut self, txid: Txid, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>,
-		commitment_number: u64, their_per_commitment_point: PublicKey, feerate_per_kw: u32,
-		to_broadcaster_value: u64, to_countersignatory_value: u64, logger: &WithChannelMonitor<L>,
+		&mut self, commitment_tx: CommitmentTransaction, logger: &WithChannelMonitor<L>,
 	) where L::Target: Logger {
-		self.initial_counterparty_commitment_info = Some((their_per_commitment_point.clone(),
-			feerate_per_kw, to_broadcaster_value, to_countersignatory_value));
+		// 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()));
 
 		#[cfg(debug_assertions)] {
 			let rebuilt_commitment_tx = self.initial_counterparty_commitment_tx().unwrap();
-			debug_assert_eq!(rebuilt_commitment_tx.trust().txid(), txid);
+			debug_assert_eq!(rebuilt_commitment_tx.trust().txid(), commitment_tx.trust().txid());
 		}
 
-		self.provide_latest_counterparty_commitment_tx(txid, htlc_outputs, commitment_number,
-				their_per_commitment_point, logger);
+		self.provide_latest_counterparty_commitment_tx(commitment_tx.trust().txid(), Vec::new(), commitment_tx.commitment_number(),
+				commitment_tx.per_commitment_point(), logger);
+		// Soon, we will only populate this field
+		self.initial_counterparty_commitment_tx = Some(commitment_tx);
 	}
 
 	fn provide_latest_counterparty_commitment_tx<L: Deref>(
@@ -3238,10 +3252,17 @@ 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`.
+				// 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, .. } => {
 					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::PaymentPreimage { payment_preimage, payment_info } => {
 					log_trace!(logger, "Updating ChannelMonitor with payment preimage");
 					self.provide_payment_preimage(&PaymentHash(Sha256::hash(&payment_preimage.0[..]).to_byte_array()), &payment_preimage, payment_info, broadcaster, &bounded_fee_estimator, logger)
@@ -3304,6 +3325,7 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 			match update {
 				ChannelMonitorUpdateStep::LatestHolderCommitmentTXInfo { .. }
 					|ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { .. }
+					|ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { .. }
 					|ChannelMonitorUpdateStep::ShutdownScript { .. }
 					|ChannelMonitorUpdateStep::CommitmentSecret { .. } =>
 						is_pre_close_update = true,
@@ -3440,14 +3462,21 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 	}
 
 	fn initial_counterparty_commitment_tx(&mut self) -> Option<CommitmentTransaction> {
-		let (their_per_commitment_point, feerate_per_kw, to_broadcaster_value,
-			to_countersignatory_value) = self.initial_counterparty_commitment_info?;
-		let htlc_outputs = vec![];
-
-		let commitment_tx = self.build_counterparty_commitment_tx(INITIAL_COMMITMENT_NUMBER,
-			&their_per_commitment_point, to_broadcaster_value, to_countersignatory_value,
-			feerate_per_kw, htlc_outputs);
-		Some(commitment_tx)
+		self.initial_counterparty_commitment_tx.clone().or_else(|| {
+			// This provides forward compatibility; an old monitor will not contain the full
+			// transaction; only enough information to rebuild it
+			self.initial_counterparty_commitment_info
+				.map(|(their_per_commitment_point, feerate_per_kw, to_broadcaster_value, to_countersignatory_value)| {
+				let htlc_outputs = vec![];
+
+				let commitment_tx = self.build_counterparty_commitment_tx(INITIAL_COMMITMENT_NUMBER,
+					&their_per_commitment_point, to_broadcaster_value, to_countersignatory_value,
+					feerate_per_kw, htlc_outputs);
+				// Take the opportunity to populate this recently introduced field
+				self.initial_counterparty_commitment_tx = Some(commitment_tx.clone());
+				commitment_tx
+			})
+		})
 	}
 
 	fn build_counterparty_commitment_tx(
@@ -3475,12 +3504,16 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 
 	fn counterparty_commitment_txs_from_update(&self, update: &ChannelMonitorUpdate) -> Vec<CommitmentTransaction> {
 		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.
 			match update {
 				&ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid,
 					ref htlc_outputs, commitment_number, their_per_commitment_point,
 					feerate_per_kw: Some(feerate_per_kw),
 					to_broadcaster_value_sat: Some(to_broadcaster_value),
-					to_countersignatory_value_sat: Some(to_countersignatory_value) } => {
+					to_countersignatory_value_sat: Some(to_countersignatory_value)
+				} => {
 
 					let nondust_htlcs = htlc_outputs.iter().filter_map(|(htlc, _)| {
 						htlc.transaction_output_index.map(|_| (htlc.clone(), None))
@@ -3494,6 +3527,11 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 
 					Some(commitment_tx)
 				},
+				&ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTX { htlc_outputs: _,
+					ref commitment_tx,
+				} => {
+					Some(commitment_tx.clone())
+				},
 				_ => None,
 			}
 		}).collect()
@@ -5076,6 +5114,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 		let mut spendable_txids_confirmed = Some(Vec::new());
 		let mut counterparty_fulfilled_htlcs = Some(new_hash_map());
 		let mut initial_counterparty_commitment_info = None;
+		let mut initial_counterparty_commitment_tx = None;
 		let mut balances_empty_height = None;
 		let mut channel_id = None;
 		let mut holder_pays_commitment_tx_fee = None;
@@ -5096,6 +5135,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 			(23, holder_pays_commitment_tx_fee, option),
 			(25, payment_preimages_with_info, option),
 			(27, first_confirmed_funding_txo, (default_value, funding_info.0)),
+			(29, initial_counterparty_commitment_tx, option),
 		});
 		if let Some(payment_preimages_with_info) = payment_preimages_with_info {
 			if payment_preimages_with_info.len() != payment_preimages.len() {
@@ -5192,6 +5232,7 @@ impl<'a, 'b, ES: EntropySource, SP: SignerProvider> ReadableArgs<(&'a ES, &'b SP
 			best_block,
 			counterparty_node_id,
 			initial_counterparty_commitment_info,
+			initial_counterparty_commitment_tx,
 			balances_empty_height,
 			failed_back_htlc_ids: new_hash_set(),
 		})))
diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs
@@ -2053,7 +2053,7 @@ trait InitialRemoteCommitmentReceiver<SP: Deref> where SP::Target: SignerProvide
 
 	fn initial_commitment_signed<L: Deref>(
 		&mut self, channel_id: ChannelId, counterparty_signature: Signature, holder_commitment_point: &mut HolderCommitmentPoint,
-		counterparty_commitment_number: u64, best_block: BestBlock, signer_provider: &SP, logger: &L,
+		_counterparty_commitment_number: u64, best_block: BestBlock, signer_provider: &SP, logger: &L,
 	) -> Result<(ChannelMonitor<<SP::Target as SignerProvider>::EcdsaSigner>, CommitmentTransaction), ChannelError>
 	where
 		L::Target: Logger
@@ -2131,14 +2131,7 @@ trait InitialRemoteCommitmentReceiver<SP: Deref> where SP::Target: SignerProvide
 		                                          funding_redeemscript.clone(), funding.get_value_satoshis(),
 		                                          obscure_factor,
 		                                          holder_commitment_tx, best_block, context.counterparty_node_id, context.channel_id());
-		channel_monitor.provide_initial_counterparty_commitment_tx(
-			counterparty_initial_bitcoin_tx.txid, Vec::new(),
-			counterparty_commitment_number,
-			context.counterparty_cur_commitment_point.unwrap(),
-			counterparty_initial_commitment_tx.feerate_per_kw(),
-			counterparty_initial_commitment_tx.to_broadcaster_value_sat(),
-			counterparty_initial_commitment_tx.to_countersignatory_value_sat(),
-			logger);
+		channel_monitor.provide_initial_counterparty_commitment_tx(counterparty_initial_commitment_tx.clone(), logger);
 
 		self.context_mut().cur_counterparty_commitment_transaction_number -= 1;
 
@@ -8507,6 +8500,8 @@ impl<SP: Deref> FundedChannel<SP> where
 		let monitor_update = ChannelMonitorUpdate {
 			update_id: self.context.latest_monitor_update_id,
 			counterparty_node_id: Some(self.context.counterparty_node_id),
+			// Soon, we will switch this to `LatestCounterpartyCommitmentTX`,
+			// and provide the full commit tx instead of the information needed to rebuild it.
 			updates: vec![ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo {
 				commitment_txid: counterparty_commitment_txid,
 				htlc_outputs: htlcs.clone(),
