delete commitment tx fee info cached

Author: tankyleo

lightning/src/ln/channel.rs

@@ -1976,14 +1976,9 @@ pub(super) struct FundingScope {
 	/// Max to_local and to_remote outputs in a remote-generated commitment transaction
 	counterparty_max_commitment_tx_output: Mutex<(u64, u64)>,
 
-	// We save these values so we can make sure `next_local_commit_tx_fee_msat` and
-	// `next_remote_commit_tx_fee_msat` properly predict what the next commitment transaction fee will
-	// be, by comparing the cached values to the fee of the transaction generated by
-	// `build_commitment_transaction`.
-	#[cfg(any(test, fuzzing))]
-	next_local_commitment_tx_fee_info_cached: Mutex<Option<CommitmentTxInfoCached>>,
-	#[cfg(any(test, fuzzing))]
-	next_remote_commitment_tx_fee_info_cached: Mutex<Option<CommitmentTxInfoCached>>,
+	// We save these values so we can make sure validation of channel updates properly predicts
+	// what the next commitment transaction fee will be, by comparing the cached values to the
+	// fee of the transaction generated by `build_commitment_transaction`.
 	#[cfg(any(test, fuzzing))]
 	next_local_fee: Mutex<PredictedNextFee>,
 	#[cfg(any(test, fuzzing))]
@@ -2061,10 +2056,6 @@ impl Readable for FundingScope {
 			short_channel_id,
 			minimum_depth_override,
 			#[cfg(any(test, fuzzing))]
-			next_local_commitment_tx_fee_info_cached: Mutex::new(None),
-			#[cfg(any(test, fuzzing))]
-			next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
-			#[cfg(any(test, fuzzing))]
 			next_local_fee: Mutex::new(PredictedNextFee::default()),
 			#[cfg(any(test, fuzzing))]
 			next_remote_fee: Mutex::new(PredictedNextFee::default()),
@@ -3210,10 +3201,6 @@ where
 			#[cfg(debug_assertions)]
 			counterparty_max_commitment_tx_output: Mutex::new((value_to_self_msat, (channel_value_satoshis * 1000 - msg_push_msat).saturating_sub(value_to_self_msat))),
 
-			#[cfg(any(test, fuzzing))]
-			next_local_commitment_tx_fee_info_cached: Mutex::new(None),
-			#[cfg(any(test, fuzzing))]
-			next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 			#[cfg(any(test, fuzzing))]
 			next_local_fee: Mutex::new(PredictedNextFee::default()),
 			#[cfg(any(test, fuzzing))]
@@ -3457,10 +3444,6 @@ where
 			#[cfg(debug_assertions)]
 			counterparty_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)),
 
-			#[cfg(any(test, fuzzing))]
-			next_local_commitment_tx_fee_info_cached: Mutex::new(None),
-			#[cfg(any(test, fuzzing))]
-			next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 			#[cfg(any(test, fuzzing))]
 			next_local_fee: Mutex::new(PredictedNextFee::default()),
 			#[cfg(any(test, fuzzing))]
@@ -4451,20 +4434,6 @@ where
 		}
 		#[cfg(any(test, fuzzing))]
 		{
-			if funding.is_outbound() {
-				let projected_commit_tx_info = funding.next_local_commitment_tx_fee_info_cached.lock().unwrap().take();
-				*funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None;
-				if let Some(info) = projected_commit_tx_info {
-					let total_pending_htlcs = self.pending_inbound_htlcs.len() + self.pending_outbound_htlcs.len()
-						+ self.holding_cell_htlc_updates.len();
-					if info.total_pending_htlcs == total_pending_htlcs
-						&& info.next_holder_htlc_id == self.next_holder_htlc_id
-						&& info.next_counterparty_htlc_id == self.next_counterparty_htlc_id
-						&& info.feerate == self.feerate_per_kw {
-							assert_eq!(commitment_data.stats.commit_tx_fee_sat, info.fee / 1000);
-						}
-				}
-			}
 			let PredictedNextFee { predicted_feerate, predicted_htlcs, predicted_fee_sat } = funding.next_local_fee.lock().unwrap().clone();
 			let mut actual_nondust_htlcs: Vec<_> = commitment_data.tx.nondust_htlcs().iter().map(|&HTLCOutputInCommitment { offered, amount_msat, .. } | HTLCAmountDirection { outbound: offered, amount_msat }).collect();
 			actual_nondust_htlcs.sort_unstable();
@@ -5349,31 +5318,7 @@ where
 		}
 
 		let num_htlcs = included_htlcs + addl_htlcs;
-		let commit_tx_fee_msat = SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000;
-		#[cfg(any(test, fuzzing))]
-		{
-			let mut fee = commit_tx_fee_msat;
-			if fee_spike_buffer_htlc.is_some() {
-				fee = SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs - 1, funding.get_channel_type()) * 1000;
-			}
-			let total_pending_htlcs = context.pending_inbound_htlcs.len() + context.pending_outbound_htlcs.len()
-				+ context.holding_cell_htlc_updates.len();
-			let commitment_tx_info = CommitmentTxInfoCached {
-				fee,
-				total_pending_htlcs,
-				next_holder_htlc_id: match htlc.origin {
-					HTLCInitiator::LocalOffered => context.next_holder_htlc_id + 1,
-					HTLCInitiator::RemoteOffered => context.next_holder_htlc_id,
-				},
-				next_counterparty_htlc_id: match htlc.origin {
-					HTLCInitiator::LocalOffered => context.next_counterparty_htlc_id,
-					HTLCInitiator::RemoteOffered => context.next_counterparty_htlc_id + 1,
-				},
-				feerate: context.feerate_per_kw,
-			};
-			*funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info);
-		}
-		commit_tx_fee_msat
+		SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000
 	}
 
 	/// Get the commitment tx fee for the remote's next commitment transaction based on the number of
@@ -5450,30 +5395,7 @@ where
 		}
 
 		let num_htlcs = included_htlcs + addl_htlcs;
-		let commit_tx_fee_msat = SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000;
-		#[cfg(any(test, fuzzing))]
-		if let Some(htlc) = &htlc {
-			let mut fee = commit_tx_fee_msat;
-			if fee_spike_buffer_htlc.is_some() {
-				fee = SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs - 1, funding.get_channel_type()) * 1000;
-			}
-			let total_pending_htlcs = context.pending_inbound_htlcs.len() + context.pending_outbound_htlcs.len();
-			let commitment_tx_info = CommitmentTxInfoCached {
-				fee,
-				total_pending_htlcs,
-				next_holder_htlc_id: match htlc.origin {
-					HTLCInitiator::LocalOffered => context.next_holder_htlc_id + 1,
-					HTLCInitiator::RemoteOffered => context.next_holder_htlc_id,
-				},
-				next_counterparty_htlc_id: match htlc.origin {
-					HTLCInitiator::LocalOffered => context.next_counterparty_htlc_id,
-					HTLCInitiator::RemoteOffered => context.next_counterparty_htlc_id + 1,
-				},
-				feerate: context.feerate_per_kw,
-			};
-			*funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info);
-		}
-		commit_tx_fee_msat
+		SpecTxBuilder {}.commit_tx_fee_sat(context.feerate_per_kw, num_htlcs, funding.get_channel_type()) * 1000
 	}
 
 	#[rustfmt::skip]
@@ -6128,15 +6050,6 @@ macro_rules! promote_splice_funding {
 	};
 }
 
-#[cfg(any(test, fuzzing))]
-struct CommitmentTxInfoCached {
-	fee: u64,
-	total_pending_htlcs: usize,
-	next_holder_htlc_id: u64,
-	next_counterparty_htlc_id: u64,
-	feerate: u32,
-}
-
 #[cfg(any(test, fuzzing))]
 #[derive(Clone, Default)]
 struct PredictedNextFee {
@@ -7645,16 +7558,6 @@ where
 			return Err(ChannelError::close("Received an unexpected revoke_and_ack".to_owned()));
 		}
 
-		#[cfg(any(test, fuzzing))]
-		{
-			for funding in
-				core::iter::once(&mut self.funding).chain(self.pending_funding.iter_mut())
-			{
-				*funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None;
-				*funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None;
-			}
-		}
-
 		match &self.context.holder_signer {
 			ChannelSignerType::Ecdsa(ecdsa) => {
 				ecdsa
@@ -11096,19 +10999,6 @@ where
 
 		#[cfg(any(test, fuzzing))]
 		{
-			if !funding.is_outbound() {
-				let projected_commit_tx_info = funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap().take();
-				*funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None;
-				if let Some(info) = projected_commit_tx_info {
-					let total_pending_htlcs = self.context.pending_inbound_htlcs.len() + self.context.pending_outbound_htlcs.len();
-					if info.total_pending_htlcs == total_pending_htlcs
-						&& info.next_holder_htlc_id == self.context.next_holder_htlc_id
-						&& info.next_counterparty_htlc_id == self.context.next_counterparty_htlc_id
-						&& info.feerate == self.context.feerate_per_kw {
-							assert_eq!(commitment_data.stats.commit_tx_fee_sat, info.fee);
-						}
-				}
-			}
 			let PredictedNextFee { predicted_feerate, predicted_htlcs, predicted_fee_sat } = funding.next_remote_fee.lock().unwrap().clone();
 			let mut actual_nondust_htlcs: Vec<_> = counterparty_commitment_tx.nondust_htlcs().iter().map(|&HTLCOutputInCommitment { offered, amount_msat, .. }| HTLCAmountDirection { outbound: !offered, amount_msat }).collect();
 			actual_nondust_htlcs.sort_unstable();
@@ -13736,10 +13626,6 @@ where
 				#[cfg(debug_assertions)]
 				counterparty_max_commitment_tx_output: Mutex::new((0, 0)),
 
-				#[cfg(any(test, fuzzing))]
-				next_local_commitment_tx_fee_info_cached: Mutex::new(None),
-				#[cfg(any(test, fuzzing))]
-				next_remote_commitment_tx_fee_info_cached: Mutex::new(None),
 				#[cfg(any(test, fuzzing))]
 				next_local_fee: Mutex::new(PredictedNextFee::default()),
 				#[cfg(any(test, fuzzing))]