Provide built commitment transactions to ChannelMonitor

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 switches to providing the fully built transaction. We take
care to make this change both forward and backward compatible, so we
still provide the former fields of `LatestCounterpartyCommitmentTXInfo`.

Author: tankyleo

lightning/src/chain/channelmonitor.rs

@@ -548,6 +548,7 @@ pub(crate) enum ChannelMonitorUpdateStep {
 		feerate_per_kw: Option<u32>,
 		to_broadcaster_value_sat: Option<u64>,
 		to_countersignatory_value_sat: Option<u64>,
+		commitment_tx: Option<CommitmentTransaction>,
 	},
 	PaymentPreimage {
 		payment_preimage: PaymentPreimage,
@@ -599,6 +600,7 @@ impl_writeable_tlv_based_enum_upgradable!(ChannelMonitorUpdateStep,
 		(4, their_per_commitment_point, required),
 		(5, to_countersignatory_value_sat, option),
 		(6, htlc_outputs, required_vec),
+		(7, commitment_tx, option),
 	},
 	(2, PaymentPreimage) => {
 		(0, payment_preimage, required),
@@ -1027,6 +1029,10 @@ 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 transaction, we now provide it outright.
+	initial_counterparty_commitment_tx: Option<CommitmentTransaction>,
 
 	/// The first block height at which we had no remaining claimable balances.
 	balances_empty_height: Option<u32>,
@@ -1250,6 +1256,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(())
@@ -1461,6 +1468,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(),
@@ -1500,17 +1508,14 @@ 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,
+		&self, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>,
+		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(htlc_outputs, commitment_tx, &logger);
 	}
 
 	/// Informs this monitor of the latest counterparty (ie non-broadcastable) commitment transaction.
@@ -2878,20 +2883,22 @@ 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, htlc_outputs: Vec<(HTLCOutputInCommitment, Option<Box<HTLCSource>>)>,
+		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(), htlc_outputs, 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>(
@@ -3442,15 +3449,20 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 		ret
 	}
 
-	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![];
+	fn initial_counterparty_commitment_tx(&self) -> Option<CommitmentTransaction> {
+		// This provides forward compatibility; an old monitor will not contain the full transaction; only enough
+		// information to rebuild it
+		if let Some((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)
+			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)
+		} else {
+			self.initial_counterparty_commitment_tx.clone()
+		}
 	}
 
 	fn build_counterparty_commitment_tx(
@@ -3479,11 +3491,13 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 	fn counterparty_commitment_txs_from_update(&self, update: &ChannelMonitorUpdate) -> Vec<CommitmentTransaction> {
 		update.updates.iter().filter_map(|update| {
 			match update {
+				// Provided for forward compatibility; old updates won't contain the full transaction
 				&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),
+					commitment_tx: None } => {
 
 					let nondust_htlcs = htlc_outputs.iter().filter_map(|(htlc, _)| {
 						htlc.transaction_output_index.map(|_| (htlc.clone(), None))
@@ -3497,6 +3511,15 @@ impl<Signer: EcdsaChannelSigner> ChannelMonitorImpl<Signer> {
 
 					Some(commitment_tx)
 				},
+				&ChannelMonitorUpdateStep::LatestCounterpartyCommitmentTXInfo { commitment_txid: _,
+					htlc_outputs: _, commitment_number: _, their_per_commitment_point: _,
+					feerate_per_kw: _,
+					to_broadcaster_value_sat: _,
+					to_countersignatory_value_sat: _,
+					commitment_tx: Some(ref commitment_tx) } => {
+
+					Some(commitment_tx.clone())
+				},
 				_ => None,
 			}
 		}).collect()
@@ -5079,6 +5102,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;
@@ -5099,6 +5123,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() {
@@ -5195,6 +5220,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(),
 		})))

lightning/src/ln/channel.rs

@@ -1985,7 +1985,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
@@ -2063,14 +2063,7 @@ trait InitialRemoteCommitmentReceiver<SP: Deref> where SP::Target: SignerProvide
 		                                          funding_redeemscript.clone(), self.funding().channel_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(Vec::new(), counterparty_initial_commitment_tx.clone(), logger);
 
 		self.context_mut().cur_counterparty_commitment_transaction_number -= 1;
 
@@ -8469,6 +8462,8 @@ impl<SP: Deref> FundedChannel<SP> where
 				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()),
+				// This is everything we need, but we still provide all the above fields for downgrades
+				commitment_tx: Some(counterparty_commitment_tx),
 			}],
 			channel_id: Some(self.context.channel_id()),
 		};