Merge pull request #3938 from carlaKC/3789-offchain-followups

#3884 followups

Author: TheBlueMatt

lightning/src/ln/chan_utils.rs

@@ -260,8 +260,9 @@ pub(crate) fn htlc_tx_fees_sat(feerate_per_kw: u32, num_accepted_htlcs: usize, n
 	num_accepted_htlcs as u64 * htlc_success_tx_fee_sat + num_offered_htlcs as u64 * htlc_timeout_tx_fee_sat
 }
 
-/// Returns a fee estimate for the commitment transaction depending on channel type.
-pub(super) fn commitment_sat_per_1000_weight_for_type<F: Deref>(
+/// Returns a fee estimate for the commitment transaction that we would ideally like to set,
+/// depending on channel type.
+pub(super) fn selected_commitment_sat_per_1000_weight<F: Deref>(
 	fee_estimator: &LowerBoundedFeeEstimator<F>, channel_type: &ChannelTypeFeatures,
 ) -> u32
 where

lightning/src/ln/channel.rs

@@ -41,8 +41,8 @@ use crate::ln::chan_utils;
 #[cfg(splicing)]
 use crate::ln::chan_utils::FUNDING_TRANSACTION_WITNESS_WEIGHT;
 use crate::ln::chan_utils::{
-	commitment_sat_per_1000_weight_for_type, get_commitment_transaction_number_obscure_factor,
-	max_htlcs, second_stage_tx_fees_sat, ChannelPublicKeys, ChannelTransactionParameters,
+	get_commitment_transaction_number_obscure_factor, max_htlcs, second_stage_tx_fees_sat,
+	selected_commitment_sat_per_1000_weight, ChannelPublicKeys, ChannelTransactionParameters,
 	ClosingTransaction, CommitmentTransaction, CounterpartyChannelTransactionParameters,
 	CounterpartyCommitmentSecrets, HTLCOutputInCommitment, HolderCommitmentTransaction,
 };
@@ -3392,7 +3392,9 @@ where
 		debug_assert!(!channel_type.supports_any_optional_bits());
 		debug_assert!(!channel_type.requires_unknown_bits_from(&channelmanager::provided_channel_type_features(&config)));
 
-		let commitment_feerate = commitment_sat_per_1000_weight_for_type(&fee_estimator, &channel_type);
+		let commitment_feerate = selected_commitment_sat_per_1000_weight(
+			&fee_estimator, &channel_type,
+		);
 
 		let value_to_self_msat = channel_value_satoshis * 1000 - push_msat;
 		let commit_tx_fee_sat = SpecTxBuilder {}.commit_tx_fee_sat(commitment_feerate, MIN_AFFORDABLE_HTLC_COUNT, &channel_type);
@@ -5042,7 +5044,7 @@ where
 	/// Get the commitment tx fee for the local's (i.e. our) next commitment transaction based on the
 	/// number of pending HTLCs that are on track to be in our next commitment tx.
 	///
-	/// Optionally includes the `HTLCCandidate` given by `htlc` and an additional non-dust HTLC if
+	/// Includes the `HTLCCandidate` given by `htlc` and an additional non-dust HTLC if
 	/// `fee_spike_buffer_htlc` is `Some`.
 	///
 	/// The first extra HTLC is useful for determining whether we can accept a further HTLC, the
@@ -5460,7 +5462,9 @@ where
 
 		let next_channel_type = get_initial_channel_type(user_config, &eligible_features);
 
-		self.feerate_per_kw = commitment_sat_per_1000_weight_for_type(&fee_estimator, &next_channel_type);
+		self.feerate_per_kw = selected_commitment_sat_per_1000_weight(
+			&fee_estimator, &next_channel_type,
+		);
 	 	funding.channel_transaction_parameters.channel_type_features = next_channel_type;
 
 		Ok(())

lightning/src/ln/channelmanager.rs

@@ -56,7 +56,7 @@ use crate::events::{
 	InboundChannelFunds, PaymentFailureReason, ReplayEvent,
 };
 use crate::events::{FundingInfo, PaidBolt12Invoice};
-use crate::ln::chan_utils::commitment_sat_per_1000_weight_for_type;
+use crate::ln::chan_utils::selected_commitment_sat_per_1000_weight;
 // Since this struct is returned in `list_channels` methods, expose it here in case users want to
 // construct one themselves.
 use crate::ln::channel::PendingV2Channel;
@@ -7166,6 +7166,7 @@ where
 	pub fn maybe_update_chan_fees(&self) {
 		PersistenceNotifierGuard::optionally_notify(self, || {
 			let mut should_persist = NotifyOption::SkipPersistNoEvents;
+			let mut feerate_cache = new_hash_map();
 
 			let per_peer_state = self.per_peer_state.read().unwrap();
 			for (_cp_id, peer_state_mutex) in per_peer_state.iter() {
@@ -7174,7 +7175,12 @@ where
 				for (chan_id, chan) in peer_state.channel_by_id.iter_mut()
 					.filter_map(|(chan_id, chan)| chan.as_funded_mut().map(|chan| (chan_id, chan)))
 				{
-					let new_feerate = commitment_sat_per_1000_weight_for_type(&self.fee_estimator, chan.funding.get_channel_type());
+					let channel_type = chan.funding.get_channel_type();
+					let new_feerate = feerate_cache.get(channel_type).copied().or_else(|| {
+						let feerate = selected_commitment_sat_per_1000_weight(&self.fee_estimator, &channel_type);
+						feerate_cache.insert(channel_type.clone(), feerate);
+						Some(feerate)
+					}).unwrap();
 					let chan_needs_persist = self.update_channel_fee(chan_id, chan, new_feerate);
 					if chan_needs_persist == NotifyOption::DoPersist { should_persist = NotifyOption::DoPersist; }
 				}
@@ -7212,6 +7218,7 @@ where
 			let mut handle_errors: Vec<(Result<(), _>, _)> = Vec::new();
 			let mut timed_out_mpp_htlcs = Vec::new();
 			let mut pending_peers_awaiting_removal = Vec::new();
+			let mut feerate_cache = new_hash_map();
 
 			{
 				let per_peer_state = self.per_peer_state.read().unwrap();
@@ -7223,7 +7230,12 @@ where
 					peer_state.channel_by_id.retain(|chan_id, chan| {
 						match chan.as_funded_mut() {
 							Some(funded_chan) => {
-								let new_feerate = commitment_sat_per_1000_weight_for_type(&self.fee_estimator, funded_chan.funding.get_channel_type());
+								let channel_type = funded_chan.funding.get_channel_type();
+								let new_feerate = feerate_cache.get(channel_type).copied().or_else(|| {
+									let feerate = selected_commitment_sat_per_1000_weight(&self.fee_estimator, &channel_type);
+									feerate_cache.insert(channel_type.clone(), feerate);
+									Some(feerate)
+								}).unwrap();
 								let chan_needs_persist = self.update_channel_fee(chan_id, funded_chan, new_feerate);
 								if chan_needs_persist == NotifyOption::DoPersist { should_persist = NotifyOption::DoPersist; }