Tweak return values of ChannelContext::build_commitment_transaction

We choose to include in `CommitmentStats` only fields that will be
calculated or constructed solely by custom transaction builders.

The other fields are created by channel, and placed in a new struct
we call `CommitmentData`.

Author: tankyleo

lightning/src/ln/channel.rs

@@ -875,15 +875,20 @@ struct HTLCStats {
 	on_holder_tx_outbound_holding_cell_htlcs_count: u32, // dust HTLCs *non*-included
 }
 
+/// An enum gathering data on a commitment, either local or remote.
+struct CommitmentData<'a> {
+	stats: CommitmentStats,
+	htlcs_included: Vec<(HTLCOutputInCommitment, Option<&'a HTLCSource>)>, // the list of HTLCs (dust HTLCs *included*) which were not ignored when building the transaction
+	outbound_htlc_preimages: Vec<PaymentPreimage>, // preimages for successful offered HTLCs since last commitment
+	inbound_htlc_preimages: Vec<PaymentPreimage>, // preimages for successful received HTLCs since last commitment
+}
+
 /// An enum gathering stats on commitment transaction, either local or remote.
-struct CommitmentStats<'a> {
+struct CommitmentStats {
 	tx: CommitmentTransaction, // the transaction info
 	total_fee_sat: u64, // the total fee included in the transaction
-	htlcs_included: Vec<(HTLCOutputInCommitment, Option<&'a HTLCSource>)>, // the list of HTLCs (dust HTLCs *included*) which were not ignored when building the transaction
 	local_balance_msat: u64, // local balance before fees *not* considering dust limits
 	remote_balance_msat: u64, // remote balance before fees *not* considering dust limits
-	outbound_htlc_preimages: Vec<PaymentPreimage>, // preimages for successful offered HTLCs since last commitment
-	inbound_htlc_preimages: Vec<PaymentPreimage>, // preimages for successful received HTLCs since last commitment
 }
 
 /// Used when calculating whether we or the remote can afford an additional HTLC.
@@ -1966,7 +1971,8 @@ trait InitialRemoteCommitmentReceiver<SP: Deref> where SP::Target: SignerProvide
 	) -> Result<CommitmentTransaction, ChannelError> where L::Target: Logger {
 		let funding_script = self.context().get_funding_redeemscript();
 
-		let initial_commitment_tx = self.context().build_commitment_transaction(self.funding(), holder_commitment_point.transaction_number(), &holder_commitment_point.current_point(), true, false, logger).tx;
+		let commitment_data = self.context().build_commitment_transaction(self.funding(), holder_commitment_point.transaction_number(), &holder_commitment_point.current_point(), true, false, logger);
+		let initial_commitment_tx = commitment_data.stats.tx;
 		let trusted_tx = initial_commitment_tx.trust();
 		let initial_commitment_bitcoin_tx = trusted_tx.built_transaction();
 		let sighash = initial_commitment_bitcoin_tx.get_sighash_all(&funding_script, self.funding().channel_value_satoshis);
@@ -2003,7 +2009,8 @@ trait InitialRemoteCommitmentReceiver<SP: Deref> where SP::Target: SignerProvide
 			}
 		};
 		let context = self.context();
-		let counterparty_initial_commitment_tx = context.build_commitment_transaction(self.funding(), context.cur_counterparty_commitment_transaction_number, &context.counterparty_cur_commitment_point.unwrap(), false, false, logger).tx;
+		let commitment_data = context.build_commitment_transaction(self.funding(), context.cur_counterparty_commitment_transaction_number, &context.counterparty_cur_commitment_point.unwrap(), false, false, logger);
+		let counterparty_initial_commitment_tx = commitment_data.stats.tx;
 		let counterparty_trusted_tx = counterparty_initial_commitment_tx.trust();
 		let counterparty_initial_bitcoin_tx = counterparty_trusted_tx.built_transaction();
 
@@ -3408,7 +3415,8 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 	/// generated by the peer which proposed adding the HTLCs, and thus we need to understand both
 	/// which peer generated this transaction and "to whom" this transaction flows.
 	#[inline]
-	fn build_commitment_transaction<L: Deref>(&self, funding: &FundingScope, commitment_number: u64, per_commitment_point: &PublicKey, local: bool, generated_by_local: bool, logger: &L) -> CommitmentStats
+	fn build_commitment_transaction<L: Deref>(&self, funding: &FundingScope, commitment_number: u64, per_commitment_point: &PublicKey, local: bool, generated_by_local: bool, logger: &L)
+		-> CommitmentData
 		where L::Target: Logger
 	{
 		let num_htlcs = self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len();
@@ -3637,12 +3645,16 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			}
 		});
 
-		CommitmentStats {
+		let stats = CommitmentStats {
 			tx,
 			total_fee_sat,
-			htlcs_included: htlcs_in_tx,
 			local_balance_msat: value_to_self_msat,
 			remote_balance_msat: value_to_remote_msat,
+		};
+
+		CommitmentData {
+			stats,
+			htlcs_included: htlcs_in_tx,
 			inbound_htlc_preimages,
 			outbound_htlc_preimages,
 		}
@@ -4441,8 +4453,9 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 		SP::Target: SignerProvider,
 		L::Target: Logger
 	{
-		let counterparty_initial_commitment_tx = self.build_commitment_transaction(
-			funding, self.cur_counterparty_commitment_transaction_number, &self.counterparty_cur_commitment_point.unwrap(), false, false, logger).tx;
+		let commitment_data = self.build_commitment_transaction(
+			funding, self.cur_counterparty_commitment_transaction_number, &self.counterparty_cur_commitment_point.unwrap(), false, false, logger);
+		let counterparty_initial_commitment_tx = commitment_data.stats.tx;
 		match self.holder_signer {
 			// TODO (taproot|arik): move match into calling method for Taproot
 			ChannelSignerType::Ecdsa(ref ecdsa) => {
@@ -5435,7 +5448,8 @@ impl<SP: Deref> FundedChannel<SP> where
 
 		let funding_script = self.context.get_funding_redeemscript();
 
-		let commitment_stats = self.context.build_commitment_transaction(&self.funding, self.holder_commitment_point.transaction_number(), &self.holder_commitment_point.current_point(), true, false, logger);
+		let commitment_data = self.context.build_commitment_transaction(&self.funding, self.holder_commitment_point.transaction_number(), &self.holder_commitment_point.current_point(), true, false, logger);
+		let commitment_stats = commitment_data.stats;
 		let commitment_txid = {
 			let trusted_tx = commitment_stats.tx.trust();
 			let bitcoin_tx = trusted_tx.built_transaction();
@@ -5450,7 +5464,7 @@ impl<SP: Deref> FundedChannel<SP> where
 			}
 			bitcoin_tx.txid
 		};
-		let mut htlcs_cloned: Vec<_> = commitment_stats.htlcs_included.iter().map(|htlc| (htlc.0.clone(), htlc.1.map(|h| h.clone()))).collect();
+		let mut htlcs_cloned: Vec<_> = commitment_data.htlcs_included.iter().map(|htlc| (htlc.0.clone(), htlc.1.map(|h| h.clone()))).collect();
 
 		// If our counterparty updated the channel fee in this commitment transaction, check that
 		// they can actually afford the new fee now.
@@ -5541,7 +5555,7 @@ impl<SP: Deref> FundedChannel<SP> where
 			self.context.counterparty_funding_pubkey()
 		);
 
-		self.context.holder_signer.as_ref().validate_holder_commitment(&holder_commitment_tx, commitment_stats.outbound_htlc_preimages)
+		self.context.holder_signer.as_ref().validate_holder_commitment(&holder_commitment_tx, commitment_data.outbound_htlc_preimages)
 			.map_err(|_| ChannelError::close("Failed to validate our commitment".to_owned()))?;
 
 		// Update state now that we've passed all the can-fail calls...
@@ -6198,9 +6212,9 @@ impl<SP: Deref> FundedChannel<SP> where
 		// Before proposing a feerate update, check that we can actually afford the new fee.
 		let dust_exposure_limiting_feerate = self.context.get_dust_exposure_limiting_feerate(&fee_estimator);
 		let htlc_stats = self.context.get_pending_htlc_stats(Some(feerate_per_kw), dust_exposure_limiting_feerate);
-		let commitment_stats = self.context.build_commitment_transaction(&self.funding, self.holder_commitment_point.transaction_number(), &self.holder_commitment_point.current_point(), true, true, logger);
-		let buffer_fee_msat = commit_tx_fee_sat(feerate_per_kw, commitment_stats.tx.htlcs().len() + htlc_stats.on_holder_tx_outbound_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, self.context.get_channel_type()) * 1000;
-		let holder_balance_msat = commitment_stats.local_balance_msat - htlc_stats.outbound_holding_cell_msat;
+		let commitment_data = self.context.build_commitment_transaction(&self.funding, self.holder_commitment_point.transaction_number(), &self.holder_commitment_point.current_point(), true, true, logger);
+		let buffer_fee_msat = commit_tx_fee_sat(feerate_per_kw, commitment_data.stats.tx.htlcs().len() + htlc_stats.on_holder_tx_outbound_holding_cell_htlcs_count as usize + CONCURRENT_INBOUND_HTLC_FEE_BUFFER as usize, self.context.get_channel_type()) * 1000;
+		let holder_balance_msat = commitment_data.stats.local_balance_msat - htlc_stats.outbound_holding_cell_msat;
 		if holder_balance_msat < buffer_fee_msat  + self.funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 {
 			//TODO: auto-close after a number of failures?
 			log_debug!(logger, "Cannot afford to send new feerate at {}", feerate_per_kw);
@@ -6511,7 +6525,8 @@ impl<SP: Deref> FundedChannel<SP> where
 			self.holder_commitment_point.try_resolve_pending(&self.context.holder_signer, &self.context.secp_ctx, logger);
 		}
 		let funding_signed = if self.context.signer_pending_funding && !self.context.is_outbound() {
-			let counterparty_initial_commitment_tx = self.context.build_commitment_transaction(&self.funding, self.context.cur_counterparty_commitment_transaction_number + 1, &self.context.counterparty_cur_commitment_point.unwrap(), false, false, logger).tx;
+			let commitment_data = self.context.build_commitment_transaction(&self.funding, self.context.cur_counterparty_commitment_transaction_number + 1, &self.context.counterparty_cur_commitment_point.unwrap(), false, false, logger);
+			let counterparty_initial_commitment_tx = commitment_data.stats.tx;
 			self.context.get_funding_signed_msg(logger, counterparty_initial_commitment_tx)
 		} else { None };
 		// Provide a `channel_ready` message if we need to, but only if we're _not_ still pending
@@ -8447,8 +8462,8 @@ impl<SP: Deref> FundedChannel<SP> where
 	-> (Vec<(HTLCOutputInCommitment, Option<&HTLCSource>)>, CommitmentTransaction)
 	where L::Target: Logger
 	{
-		let commitment_stats = self.context.build_commitment_transaction(&self.funding, self.context.cur_counterparty_commitment_transaction_number, &self.context.counterparty_cur_commitment_point.unwrap(), false, true, logger);
-		let counterparty_commitment_tx = commitment_stats.tx;
+		let commitment_data = self.context.build_commitment_transaction(&self.funding, self.context.cur_counterparty_commitment_transaction_number, &self.context.counterparty_cur_commitment_point.unwrap(), false, true, logger);
+		let counterparty_commitment_tx = commitment_data.stats.tx;
 
 		#[cfg(any(test, fuzzing))]
 		{
@@ -8468,7 +8483,7 @@ impl<SP: Deref> FundedChannel<SP> where
 			}
 		}
 
-		(commitment_stats.htlcs_included, counterparty_commitment_tx)
+		(commitment_data.htlcs_included, counterparty_commitment_tx)
 	}
 
 	/// Only fails in case of signer rejection. Used for channel_reestablish commitment_signed
@@ -8478,23 +8493,25 @@ impl<SP: Deref> FundedChannel<SP> where
 		#[cfg(any(test, fuzzing))]
 		self.build_commitment_no_state_update(logger);
 
-		let commitment_stats = self.context.build_commitment_transaction(&self.funding, self.context.cur_counterparty_commitment_transaction_number, &self.context.counterparty_cur_commitment_point.unwrap(), false, true, logger);
+		let commitment_data = self.context.build_commitment_transaction(&self.funding, self.context.cur_counterparty_commitment_transaction_number, &self.context.counterparty_cur_commitment_point.unwrap(), false, true, logger);
+		let commitment_stats = commitment_data.stats;
+		let htlcs_included = commitment_data.htlcs_included;
 		let counterparty_commitment_txid = commitment_stats.tx.trust().txid();
 
 		match &self.context.holder_signer {
 			ChannelSignerType::Ecdsa(ecdsa) => {
 				let (signature, htlc_signatures);
 
 				{
-					let mut htlcs = Vec::with_capacity(commitment_stats.htlcs_included.len());
-					for &(ref htlc, _) in commitment_stats.htlcs_included.iter() {
+					let mut htlcs = Vec::with_capacity(htlcs_included.len());
+					for &(ref htlc, _) in htlcs_included.iter() {
 						htlcs.push(htlc);
 					}
 
 					let res = ecdsa.sign_counterparty_commitment(
 							&commitment_stats.tx,
-							commitment_stats.inbound_htlc_preimages,
-							commitment_stats.outbound_htlc_preimages,
+							commitment_data.inbound_htlc_preimages,
+							commitment_data.outbound_htlc_preimages,
 							&self.context.secp_ctx,
 						).map_err(|_| ChannelError::Ignore("Failed to get signatures for new commitment_signed".to_owned()))?;
 					signature = res.0;
@@ -8522,7 +8539,7 @@ impl<SP: Deref> FundedChannel<SP> where
 					batch: None,
 					#[cfg(taproot)]
 					partial_signature_with_nonce: None,
-				}, (counterparty_commitment_txid, commitment_stats.htlcs_included)))
+				}, (counterparty_commitment_txid, htlcs_included)))
 			},
 			// TODO (taproot|arik)
 			#[cfg(taproot)]
@@ -8957,7 +8974,8 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
 
 	/// Only allowed after [`ChannelContext::channel_transaction_parameters`] is set.
 	fn get_funding_created_msg<L: Deref>(&mut self, logger: &L) -> Option<msgs::FundingCreated> where L::Target: Logger {
-		let counterparty_initial_commitment_tx = self.context.build_commitment_transaction(&self.funding, self.context.cur_counterparty_commitment_transaction_number, &self.context.counterparty_cur_commitment_point.unwrap(), false, false, logger).tx;
+		let commitment_data = self.context.build_commitment_transaction(&self.funding, self.context.cur_counterparty_commitment_transaction_number, &self.context.counterparty_cur_commitment_point.unwrap(), false, false, logger);
+		let counterparty_initial_commitment_tx = commitment_data.stats.tx;
 		let signature = match &self.context.holder_signer {
 			// TODO (taproot|arik): move match into calling method for Taproot
 			ChannelSignerType::Ecdsa(ecdsa) => {
@@ -11609,12 +11627,12 @@ mod tests {
 				$( { $htlc_idx: expr, $counterparty_htlc_sig_hex: expr, $htlc_sig_hex: expr, $htlc_tx_hex: expr } ), *
 			} ) => { {
 				let (commitment_tx, htlcs): (_, Vec<HTLCOutputInCommitment>) = {
-					let mut commitment_stats = chan.context.build_commitment_transaction(&chan.funding, 0xffffffffffff - 42, &per_commitment_point, true, false, &logger);
+					let mut commitment_data = chan.context.build_commitment_transaction(&chan.funding, 0xffffffffffff - 42, &per_commitment_point, true, false, &logger);
 
-					let htlcs = commitment_stats.htlcs_included.drain(..)
+					let htlcs = commitment_data.htlcs_included.drain(..)
 						.filter_map(|(htlc, _)| if htlc.transaction_output_index.is_some() { Some(htlc) } else { None })
 						.collect();
-					(commitment_stats.tx, htlcs)
+					(commitment_data.stats.tx, htlcs)
 				};
 				let trusted_tx = commitment_tx.trust();
 				let unsigned_tx = trusted_tx.built_transaction();