Move next_*_commitment_tx_fee_info_cached to FundingScope

Author: jkczyz

lightning/src/ln/channel.rs

@@ -1579,6 +1579,15 @@ pub(super) struct FundingScope {
 	#[cfg(debug_assertions)]
 	/// 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>>,
 }
 
 impl FundingScope {
@@ -1823,15 +1832,6 @@ pub(super) struct ChannelContext<SP: Deref> where SP::Target: SignerProvider {
 	/// This can be used to rebroadcast the channel_announcement message later.
 	announcement_sigs: Option<(Signature, Signature)>,
 
-	// 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 tranaction 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>>,
-
 	/// lnd has a long-standing bug where, upon reconnection, if the channel is not yet confirmed
 	/// they will not send a channel_reestablish until the channel locks in. Then, they will send a
 	/// channel_ready *before* sending the channel_reestablish (which is clearly a violation of
@@ -2470,6 +2470,11 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			holder_max_commitment_tx_output: Mutex::new((value_to_self_msat, (channel_value_satoshis * 1000 - msg_push_msat).saturating_sub(value_to_self_msat))),
 			#[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),
 		};
 		let channel_context = ChannelContext {
 			user_id,
@@ -2578,11 +2583,6 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 
 			announcement_sigs: None,
 
-			#[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),
-
 			workaround_lnd_bug_4006: None,
 			sent_message_awaiting_response: None,
 
@@ -2705,6 +2705,11 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			holder_max_commitment_tx_output: Mutex::new((channel_value_satoshis * 1000 - push_msat, push_msat)),
 			#[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),
 		};
 		let channel_context = Self {
 			user_id,
@@ -2810,11 +2815,6 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 
 			announcement_sigs: None,
 
-			#[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),
-
 			workaround_lnd_bug_4006: None,
 			sent_message_awaiting_response: None,
 
@@ -3868,9 +3868,17 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			}
 
 			let htlc_above_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000, HTLCInitiator::LocalOffered);
-			let mut max_reserved_commit_tx_fee_msat = context.next_local_commit_tx_fee_msat(htlc_above_dust, Some(()));
+			let mut max_reserved_commit_tx_fee_msat = context.next_local_commit_tx_fee_msat(
+				#[cfg(any(test, fuzzing))]
+				&funding,
+				htlc_above_dust, Some(()),
+			);
 			let htlc_dust = HTLCCandidate::new(real_dust_limit_timeout_sat * 1000 - 1, HTLCInitiator::LocalOffered);
-			let mut min_reserved_commit_tx_fee_msat = context.next_local_commit_tx_fee_msat(htlc_dust, Some(()));
+			let mut min_reserved_commit_tx_fee_msat = context.next_local_commit_tx_fee_msat(
+				#[cfg(any(test, fuzzing))]
+				&funding,
+				htlc_dust, Some(()),
+			);
 			if !context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
 				max_reserved_commit_tx_fee_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
 				min_reserved_commit_tx_fee_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
@@ -3899,7 +3907,11 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 			}
 
 			let htlc_above_dust = HTLCCandidate::new(real_dust_limit_success_sat * 1000, HTLCInitiator::LocalOffered);
-			let max_reserved_commit_tx_fee_msat = context.next_remote_commit_tx_fee_msat(Some(htlc_above_dust), None);
+			let max_reserved_commit_tx_fee_msat = context.next_remote_commit_tx_fee_msat(
+				#[cfg(any(test, fuzzing))]
+				&funding,
+				Some(htlc_above_dust), None,
+			);
 
 			let holder_selected_chan_reserve_msat = funding.holder_selected_channel_reserve_satoshis * 1000;
 			let remote_balance_msat = (funding.channel_value_satoshis * 1000 - funding.value_to_self_msat)
@@ -3996,7 +4008,12 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 	/// second allows for creating a buffer to ensure a further HTLC can always be accepted/added.
 	///
 	/// Dust HTLCs are excluded.
-	fn next_local_commit_tx_fee_msat(&self, htlc: HTLCCandidate, fee_spike_buffer_htlc: Option<()>) -> u64 {
+	fn next_local_commit_tx_fee_msat(
+		&self,
+		#[cfg(any(test, fuzzing))]
+		funding: &FundingScope,
+		htlc: HTLCCandidate, fee_spike_buffer_htlc: Option<()>,
+	) -> u64 {
 		let context = &self;
 		assert!(context.is_outbound());
 
@@ -4085,7 +4102,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 				},
 				feerate: context.feerate_per_kw,
 			};
-			*context.next_local_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info);
+			*funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info);
 		}
 		res
 	}
@@ -4100,7 +4117,12 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 	/// second allows for creating a buffer to ensure a further HTLC can always be accepted/added.
 	///
 	/// Dust HTLCs are excluded.
-	fn next_remote_commit_tx_fee_msat(&self, htlc: Option<HTLCCandidate>, fee_spike_buffer_htlc: Option<()>) -> u64 {
+	fn next_remote_commit_tx_fee_msat(
+		&self,
+		#[cfg(any(test, fuzzing))]
+		funding: &FundingScope,
+		htlc: Option<HTLCCandidate>, fee_spike_buffer_htlc: Option<()>,
+	) -> u64 {
 		debug_assert!(htlc.is_some() || fee_spike_buffer_htlc.is_some(), "At least one of the options must be set");
 
 		let context = &self;
@@ -4179,7 +4201,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider {
 				},
 				feerate: context.feerate_per_kw,
 			};
-			*context.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info);
+			*funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = Some(commitment_tx_info);
 		}
 		res
 	}
@@ -5199,7 +5221,11 @@ impl<SP: Deref> FundedChannel<SP> where
 		{
 			let remote_commit_tx_fee_msat = if self.context.is_outbound() { 0 } else {
 				let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered);
-				self.context.next_remote_commit_tx_fee_msat(Some(htlc_candidate), None) // Don't include the extra fee spike buffer HTLC in calculations
+				self.context.next_remote_commit_tx_fee_msat(
+					#[cfg(any(test, fuzzing))]
+					&self.funding,
+					Some(htlc_candidate), None, // Don't include the extra fee spike buffer HTLC in calculations
+				)
 			};
 			let anchor_outputs_value_msat = if !self.context.is_outbound() && self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
 				ANCHOR_OUTPUT_VALUE_SATOSHI * 2 * 1000
@@ -5222,7 +5248,11 @@ impl<SP: Deref> FundedChannel<SP> where
 		if self.context.is_outbound() {
 			// Check that they won't violate our local required channel reserve by adding this HTLC.
 			let htlc_candidate = HTLCCandidate::new(msg.amount_msat, HTLCInitiator::RemoteOffered);
-			let local_commit_tx_fee_msat = self.context.next_local_commit_tx_fee_msat(htlc_candidate, None);
+			let local_commit_tx_fee_msat = self.context.next_local_commit_tx_fee_msat(
+				#[cfg(any(test, fuzzing))]
+				&self.funding,
+				htlc_candidate, None,
+			);
 			if self.funding.value_to_self_msat < self.funding.counterparty_selected_channel_reserve_satoshis.unwrap() * 1000 + local_commit_tx_fee_msat + anchor_outputs_value_msat {
 				return Err(ChannelError::close("Cannot accept HTLC that would put our balance under counterparty-announced channel reserve value".to_owned()));
 			}
@@ -5400,8 +5430,8 @@ impl<SP: Deref> FundedChannel<SP> where
 		#[cfg(any(test, fuzzing))]
 		{
 			if self.context.is_outbound() {
-				let projected_commit_tx_info = self.context.next_local_commitment_tx_fee_info_cached.lock().unwrap().take();
-				*self.context.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None;
+				let projected_commit_tx_info = self.funding.next_local_commitment_tx_fee_info_cached.lock().unwrap().take();
+				*self.funding.next_remote_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()
 						+ self.context.holding_cell_htlc_updates.len();
@@ -5770,8 +5800,8 @@ impl<SP: Deref> FundedChannel<SP> where
 
 		#[cfg(any(test, fuzzing))]
 		{
-			*self.context.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None;
-			*self.context.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None;
+			*self.funding.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None;
+			*self.funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap() = None;
 		}
 
 		match &self.context.holder_signer {
@@ -7411,7 +7441,11 @@ impl<SP: Deref> FundedChannel<SP> where
 			//
 			// A `None` `HTLCCandidate` is used as in this case because we're already accounting for
 			// the incoming HTLC as it has been fully committed by both sides.
-			let mut remote_fee_cost_incl_stuck_buffer_msat = self.context.next_remote_commit_tx_fee_msat(None, Some(()));
+			let mut remote_fee_cost_incl_stuck_buffer_msat = self.context.next_remote_commit_tx_fee_msat(
+				#[cfg(any(test, fuzzing))]
+				&self.funding,
+				None, Some(()),
+			);
 			if !self.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
 				remote_fee_cost_incl_stuck_buffer_msat *= FEE_SPIKE_BUFFER_FEE_INCREASE_MULTIPLE;
 			}
@@ -8353,8 +8387,8 @@ impl<SP: Deref> FundedChannel<SP> where
 		#[cfg(any(test, fuzzing))]
 		{
 			if !self.context.is_outbound() {
-				let projected_commit_tx_info = self.context.next_remote_commitment_tx_fee_info_cached.lock().unwrap().take();
-				*self.context.next_local_commitment_tx_fee_info_cached.lock().unwrap() = None;
+				let projected_commit_tx_info = self.funding.next_remote_commitment_tx_fee_info_cached.lock().unwrap().take();
+				*self.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
@@ -10375,6 +10409,11 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 				holder_max_commitment_tx_output: Mutex::new((0, 0)),
 				#[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),
 			},
 			context: ChannelContext {
 				user_id,
@@ -10470,11 +10509,6 @@ impl<'a, 'b, 'c, ES: Deref, SP: Deref> ReadableArgs<(&'a ES, &'b SP, u32, &'c Ch
 
 				announcement_sigs,
 
-				#[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),
-
 				workaround_lnd_bug_4006: None,
 				sent_message_awaiting_response: None,
 
@@ -10744,7 +10778,7 @@ mod tests {
 		// Make sure when Node A calculates their local commitment transaction, none of the HTLCs pass
 		// the dust limit check.
 		let htlc_candidate = HTLCCandidate::new(htlc_amount_msat, HTLCInitiator::LocalOffered);
-		let local_commit_tx_fee = node_a_chan.context.next_local_commit_tx_fee_msat(htlc_candidate, None);
+		let local_commit_tx_fee = node_a_chan.context.next_local_commit_tx_fee_msat(&node_a_chan.funding, htlc_candidate, None);
 		let local_commit_fee_0_htlcs = commit_tx_fee_sat(node_a_chan.context.feerate_per_kw, 0, node_a_chan.context.get_channel_type()) * 1000;
 		assert_eq!(local_commit_tx_fee, local_commit_fee_0_htlcs);
 
@@ -10753,7 +10787,7 @@ mod tests {
 		node_a_chan.context.channel_transaction_parameters.is_outbound_from_holder = false;
 		let remote_commit_fee_3_htlcs = commit_tx_fee_sat(node_a_chan.context.feerate_per_kw, 3, node_a_chan.context.get_channel_type()) * 1000;
 		let htlc_candidate = HTLCCandidate::new(htlc_amount_msat, HTLCInitiator::LocalOffered);
-		let remote_commit_tx_fee = node_a_chan.context.next_remote_commit_tx_fee_msat(Some(htlc_candidate), None);
+		let remote_commit_tx_fee = node_a_chan.context.next_remote_commit_tx_fee_msat(&node_a_chan.funding, Some(htlc_candidate), None);
 		assert_eq!(remote_commit_tx_fee, remote_commit_fee_3_htlcs);
 	}
 
@@ -10781,27 +10815,27 @@ mod tests {
 		// counted as dust when it shouldn't be.
 		let htlc_amt_above_timeout = ((253 * htlc_timeout_tx_weight(chan.context.get_channel_type()) / 1000) + chan.context.holder_dust_limit_satoshis + 1) * 1000;
 		let htlc_candidate = HTLCCandidate::new(htlc_amt_above_timeout, HTLCInitiator::LocalOffered);
-		let commitment_tx_fee = chan.context.next_local_commit_tx_fee_msat(htlc_candidate, None);
+		let commitment_tx_fee = chan.context.next_local_commit_tx_fee_msat(&chan.funding, htlc_candidate, None);
 		assert_eq!(commitment_tx_fee, commitment_tx_fee_1_htlc);
 
 		// If swapped: this HTLC would be counted as non-dust when it shouldn't be.
 		let dust_htlc_amt_below_success = ((253 * htlc_success_tx_weight(chan.context.get_channel_type()) / 1000) + chan.context.holder_dust_limit_satoshis - 1) * 1000;
 		let htlc_candidate = HTLCCandidate::new(dust_htlc_amt_below_success, HTLCInitiator::RemoteOffered);
-		let commitment_tx_fee = chan.context.next_local_commit_tx_fee_msat(htlc_candidate, None);
+		let commitment_tx_fee = chan.context.next_local_commit_tx_fee_msat(&chan.funding, htlc_candidate, None);
 		assert_eq!(commitment_tx_fee, commitment_tx_fee_0_htlcs);
 
 		chan.context.channel_transaction_parameters.is_outbound_from_holder = false;
 
 		// If swapped: this HTLC would be counted as non-dust when it shouldn't be.
 		let dust_htlc_amt_above_timeout = ((253 * htlc_timeout_tx_weight(chan.context.get_channel_type()) / 1000) + chan.context.counterparty_dust_limit_satoshis + 1) * 1000;
 		let htlc_candidate = HTLCCandidate::new(dust_htlc_amt_above_timeout, HTLCInitiator::LocalOffered);
-		let commitment_tx_fee = chan.context.next_remote_commit_tx_fee_msat(Some(htlc_candidate), None);
+		let commitment_tx_fee = chan.context.next_remote_commit_tx_fee_msat(&chan.funding, Some(htlc_candidate), None);
 		assert_eq!(commitment_tx_fee, commitment_tx_fee_0_htlcs);
 
 		// If swapped: this HTLC would be counted as dust when it shouldn't be.
 		let htlc_amt_below_success = ((253 * htlc_success_tx_weight(chan.context.get_channel_type()) / 1000) + chan.context.counterparty_dust_limit_satoshis - 1) * 1000;
 		let htlc_candidate = HTLCCandidate::new(htlc_amt_below_success, HTLCInitiator::RemoteOffered);
-		let commitment_tx_fee = chan.context.next_remote_commit_tx_fee_msat(Some(htlc_candidate), None);
+		let commitment_tx_fee = chan.context.next_remote_commit_tx_fee_msat(&chan.funding, Some(htlc_candidate), None);
 		assert_eq!(commitment_tx_fee, commitment_tx_fee_1_htlc);
 	}