More flexible fee rate estimates

Author: benthecarman

fuzz/src/chanmon_consistency.rs

@@ -80,9 +80,10 @@ impl FeeEstimator for FuzzEstimator {
 		// always return a HighPriority feerate here which is >= the maximum Normal feerate and a
 		// Background feerate which is <= the minimum Normal feerate.
 		match conf_target {
-			ConfirmationTarget::HighPriority => MAX_FEE,
-			ConfirmationTarget::Background|ConfirmationTarget::MempoolMinimum => 253,
-			ConfirmationTarget::Normal => cmp::min(self.ret_val.load(atomic::Ordering::Acquire), MAX_FEE),
+			ConfirmationTarget::MaxAllowedNonAnchorChannelRemoteFee => MAX_FEE * 10,
+			ConfirmationTarget::HtlcSweep => MAX_FEE,
+			ConfirmationTarget::ChannelCloseMinimum|ConfirmationTarget::AnchorChannelFee|ConfirmationTarget::MinAllowedAnchorChannelRemoteFee|ConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee => 253,
+			ConfirmationTarget::NonAnchorChannelFee => cmp::min(self.ret_val.load(atomic::Ordering::Acquire), MAX_FEE),
 		}
 	}
 }

lightning/src/chain/chaininterface.rs

@@ -48,21 +48,47 @@ pub trait BroadcasterInterface {
 /// estimation.
 #[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)]
 pub enum ConfirmationTarget {
-	/// We'd like a transaction to confirm in the future, but don't want to commit most of the fees
-	/// required to do so yet. The remaining fees will come via a Child-Pays-For-Parent (CPFP) fee
-	/// bump of the transaction.
+	/// We need to sweep an HTLC after a force close transaction. This should be a high priority
+	/// transaction.
+	HtlcSweep,
+	/// The highest fee rate we will allow our channel counterparty to have in a non-anchor channel.
+	/// Having this be a low value can cause force closures. Using 10x your [`HtlcSweep`] fee rate
+	/// can be a good default.
 	///
-	/// The feerate returned should be the absolute minimum feerate required to enter most node
-	/// mempools across the network. Note that if you are not able to obtain this feerate estimate,
-	/// you should likely use the furthest-out estimate allowed by your fee estimator.
-	MempoolMinimum,
-	/// We are happy with a transaction confirming slowly, at least within a day or so worth of
-	/// blocks.
-	Background,
-	/// We'd like a transaction to confirm without major delayed, i.e., within the next 12-24 blocks.
-	Normal,
-	/// We'd like a transaction to confirm in the next few blocks.
-	HighPriority,
+	/// This maximum is needed to prevent our counterparty from creating a channel which we cannot
+	/// afford to force close because our HTLC outputs would become dust.
+	MaxAllowedNonAnchorChannelRemoteFee,
+	/// This needs to be sufficient to get into the mempool/get confirmed when the channel needs to
+	/// be force-closed. Setting too low may result in force-closures. Because this is for anchor
+	/// channels, it can be a low value as we can always bump the fee rate later.
+	///
+	/// A good estimate is the expected mempool minimum at the time of force-closure.
+	MinAllowedAnchorChannelRemoteFee,
+	/// The lowest fee rate we will allow our channel counterparty to have in a non-anchor channel.
+	/// This needs to be sufficient to get into the mempool/get confirmed when the channel needs to
+	/// be force-closed. Setting too low may result in force-closures.
+	///
+	/// A good estimate is at least within a day (144) or so worth of blocks.
+	MinAllowedNonAnchorChannelRemoteFee,
+	/// Fee rate to use for anchor channels. This is safe to be the mempool minimum when the channel
+	/// needs to be force-closed.
+	///
+	/// This is the feerate for the pre-signed transactions which we will broadcast in the event of
+	/// a force-closure, and needs to be sufficient at that time.
+	AnchorChannelFee,
+	/// Fee rate to use for non-anchor channels. A good estimate is within the next 12-24 blocks.
+	///
+	/// This is the feerate for the pre-signed transactions which we will broadcast in the event of
+	/// a force-closure, and needs to be sufficient at that time.
+	NonAnchorChannelFee,
+	/// When cooperative closing channel, the minimum fee rate we will accept. Recommended at least
+	/// within a day or so worth of blocks.
+	///
+	/// This will also be used when initiating a cooperative close of a channel. When closing a channel
+	/// you can override this fee by using [`ChannelManager::close_channel_with_feerate_and_script`].
+	///
+	/// [`ChannelManager::close_channel_with_feerate_and_script`]: crate::ln::channelmanager::ChannelManager::close_channel_with_feerate_and_script
+	ChannelCloseMinimum,
 }
 
 /// A trait which should be implemented to provide feerate information on a number of time
@@ -135,7 +161,7 @@ mod tests {
 		let test_fee_estimator = &TestFeeEstimator { sat_per_kw };
 		let fee_estimator = LowerBoundedFeeEstimator::new(test_fee_estimator);
 
-		assert_eq!(fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Background), FEERATE_FLOOR_SATS_PER_KW);
+		assert_eq!(fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::AnchorChannelFee), FEERATE_FLOOR_SATS_PER_KW);
 	}
 
 	#[test]
@@ -144,6 +170,6 @@ mod tests {
 		let test_fee_estimator = &TestFeeEstimator { sat_per_kw };
 		let fee_estimator = LowerBoundedFeeEstimator::new(test_fee_estimator);
 
-		assert_eq!(fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Background), sat_per_kw);
+		assert_eq!(fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::AnchorChannelFee), sat_per_kw);
 	}
 }

lightning/src/chain/onchaintx.rs

@@ -580,7 +580,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
 		if cached_request.is_malleable() {
 			if cached_request.requires_external_funding() {
 				let target_feerate_sat_per_1000_weight = cached_request.compute_package_feerate(
-					fee_estimator, ConfirmationTarget::HighPriority, force_feerate_bump
+					fee_estimator, ConfirmationTarget::HtlcSweep, force_feerate_bump
 				);
 				if let Some(htlcs) = cached_request.construct_malleable_package_with_external_funding(self) {
 					return Some((
@@ -631,7 +631,7 @@ impl<ChannelSigner: WriteableEcdsaChannelSigner> OnchainTxHandler<ChannelSigner>
 					debug_assert_eq!(tx.txid(), self.holder_commitment.trust().txid(),
 						"Holder commitment transaction mismatch");
 
-					let conf_target = ConfirmationTarget::HighPriority;
+					let conf_target = ConfirmationTarget::HtlcSweep;
 					let package_target_feerate_sat_per_1000_weight = cached_request
 						.compute_package_feerate(fee_estimator, conf_target, force_feerate_bump);
 					if let Some(input_amount_sat) = output.funding_amount {

lightning/src/chain/package.rs

@@ -24,7 +24,7 @@ use crate::ln::PaymentPreimage;
 use crate::ln::chan_utils::{TxCreationKeys, HTLCOutputInCommitment};
 use crate::ln::chan_utils;
 use crate::ln::msgs::DecodeError;
-use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT};
+use crate::chain::chaininterface::{FeeEstimator, ConfirmationTarget, MIN_RELAY_FEE_SAT_PER_1000_WEIGHT, compute_feerate_sat_per_1000_weight};
 use crate::sign::WriteableEcdsaChannelSigner;
 use crate::chain::onchaintx::{ExternalHTLCClaim, OnchainTxHandler};
 use crate::util::logger::Logger;
@@ -1111,30 +1111,15 @@ fn compute_fee_from_spent_amounts<F: Deref, L: Deref>(input_amounts: u64, predic
 	where F::Target: FeeEstimator,
 	      L::Target: Logger,
 {
-	let mut updated_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64;
-	let mut fee = updated_feerate * (predicted_weight as u64) / 1000;
+	let sweep_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::HtlcSweep);
+	let fee_rate = cmp::min(sweep_feerate, compute_feerate_sat_per_1000_weight(input_amounts / 2, predicted_weight as u64)) as u64;
+	let fee = fee_rate * (predicted_weight as u64) / 1000;
 	if input_amounts <= fee {
-		updated_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal) as u64;
-		fee = updated_feerate * (predicted_weight as u64) / 1000;
-		if input_amounts <= fee {
-			updated_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Background) as u64;
-			fee = updated_feerate * (predicted_weight as u64) / 1000;
-			if input_amounts <= fee {
-				log_error!(logger, "Failed to generate an on-chain punishment tx as even low priority fee ({} sat) was more than the entire claim balance ({} sat)",
+		log_error!(logger, "Failed to generate an on-chain punishment tx as even low priority fee ({} sat) was more than the entire claim balance ({} sat)",
 					fee, input_amounts);
-				None
-			} else {
-				log_warn!(logger, "Used low priority fee for on-chain punishment tx as high priority fee was more than the entire claim balance ({} sat)",
-					input_amounts);
-				Some((fee, updated_feerate))
-			}
-		} else {
-			log_warn!(logger, "Used medium priority fee for on-chain punishment tx as high priority fee was more than the entire claim balance ({} sat)",
-				input_amounts);
-			Some((fee, updated_feerate))
-		}
+		None
 	} else {
-		Some((fee, updated_feerate))
+		Some((fee, fee_rate))
 	}
 }
 

lightning/src/ln/channel.rs

@@ -1168,7 +1168,7 @@ impl<SP: Deref> ChannelContext<SP> where SP::Target: SignerProvider  {
 		match self.config.options.max_dust_htlc_exposure {
 			MaxDustHTLCExposure::FeeRateMultiplier(multiplier) => {
 				let feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight(
-					ConfirmationTarget::HighPriority);
+					ConfirmationTarget::HtlcSweep);
 				feerate_per_kw as u64 * multiplier
 			},
 			MaxDustHTLCExposure::FixedLimitMsat(limit) => limit,
@@ -2155,21 +2155,17 @@ impl<SP: Deref> Channel<SP> where
 		// apply to channels supporting anchor outputs since HTLC transactions are pre-signed with a
 		// zero fee, so their fee is no longer considered to determine dust limits.
 		if !channel_type.supports_anchors_zero_fee_htlc_tx() {
-			let upper_limit = cmp::max(250 * 25,
-				fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64 * 10);
+			let upper_limit =
+				fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::MaxAllowedNonAnchorChannelRemoteFee) as u64;
 			if feerate_per_kw as u64 > upper_limit {
 				return Err(ChannelError::Close(format!("Peer's feerate much too high. Actual: {}. Our expected upper limit: {}", feerate_per_kw, upper_limit)));
 			}
 		}
 
-		// We can afford to use a lower bound with anchors than previously since we can now bump
-		// fees when broadcasting our commitment. However, we must still make sure we meet the
-		// minimum mempool feerate, until package relay is deployed, such that we can ensure the
-		// commitment transaction propagates throughout node mempools on its own.
 		let lower_limit_conf_target = if channel_type.supports_anchors_zero_fee_htlc_tx() {
-			ConfirmationTarget::MempoolMinimum
+			ConfirmationTarget::MinAllowedAnchorChannelRemoteFee
 		} else {
-			ConfirmationTarget::Background
+			ConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee
 		};
 		let lower_limit = fee_estimator.bounded_sat_per_1000_weight(lower_limit_conf_target);
 		// Some fee estimators round up to the next full sat/vbyte (ie 250 sats per kw), causing
@@ -4155,8 +4151,10 @@ impl<SP: Deref> Channel<SP> where
 		// Propose a range from our current Background feerate to our Normal feerate plus our
 		// force_close_avoidance_max_fee_satoshis.
 		// If we fail to come to consensus, we'll have to force-close.
-		let mut proposed_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Background);
-		let normal_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal);
+		let mut proposed_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::ChannelCloseMinimum);
+		// Use NonAnchorChannelFee because this should be an estimate for a channel close
+		// that we don't expect to be need fee bumping
+		let normal_feerate = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee);
 		let mut proposed_max_feerate = if self.context.is_outbound() { normal_feerate } else { u32::max_value() };
 
 		// The spec requires that (when the channel does not have anchors) we only send absolute
@@ -5727,9 +5725,9 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
 		debug_assert!(channel_type.is_subset(&channelmanager::provided_channel_type_features(&config)));
 
 		let commitment_conf_target = if channel_type.supports_anchors_zero_fee_htlc_tx() {
-			ConfirmationTarget::MempoolMinimum
+			ConfirmationTarget::AnchorChannelFee
 		} else {
-			ConfirmationTarget::Normal
+			ConfirmationTarget::NonAnchorChannelFee
 		};
 		let commitment_feerate = fee_estimator.bounded_sat_per_1000_weight(commitment_conf_target);
 
@@ -6019,7 +6017,7 @@ impl<SP: Deref> OutboundV1Channel<SP> where SP::Target: SignerProvider {
 		// whatever reason.
 		if self.context.channel_type.supports_anchors_zero_fee_htlc_tx() {
 			self.context.channel_type.clear_anchors_zero_fee_htlc_tx();
-			self.context.feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal);
+			self.context.feerate_per_kw = fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee);
 			assert!(!self.context.channel_transaction_parameters.channel_type_features.supports_anchors_nonzero_fee_htlc_tx());
 		} else if self.context.channel_type.supports_scid_privacy() {
 			self.context.channel_type.clear_scid_privacy();

lightning/src/ln/channelmanager.rs

@@ -2640,11 +2640,11 @@ where
 	/// will be accepted on the given channel, and after additional timeout/the closing of all
 	/// pending HTLCs, the channel will be closed on chain.
 	///
-	///  * If we are the channel initiator, we will pay between our [`Background`] and
-	///    [`ChannelConfig::force_close_avoidance_max_fee_satoshis`] plus our [`Normal`] fee
-	///    estimate.
+	///  * If we are the channel initiator, we will pay between our [`ChannelCloseMinimum`] and
+	///    [`ChannelConfig::force_close_avoidance_max_fee_satoshis`] plus our [`NonAnchorChannelFee`]
+	///    fee estimate.
 	///  * If our counterparty is the channel initiator, we will require a channel closing
-	///    transaction feerate of at least our [`Background`] feerate or the feerate which
+	///    transaction feerate of at least our [`ChannelCloseMinimum`] feerate or the feerate which
 	///    would appear on a force-closure transaction, whichever is lower. We will allow our
 	///    counterparty to pay as much fee as they'd like, however.
 	///
@@ -2656,8 +2656,8 @@ where
 	/// channel.
 	///
 	/// [`ChannelConfig::force_close_avoidance_max_fee_satoshis`]: crate::util::config::ChannelConfig::force_close_avoidance_max_fee_satoshis
-	/// [`Background`]: crate::chain::chaininterface::ConfirmationTarget::Background
-	/// [`Normal`]: crate::chain::chaininterface::ConfirmationTarget::Normal
+	/// [`ChannelCloseMinimum`]: crate::chain::chaininterface::ConfirmationTarget::ChannelCloseMinimum
+	/// [`NonAnchorChannelFee`]: crate::chain::chaininterface::ConfirmationTarget::NonAnchorChannelFee
 	/// [`SendShutdown`]: crate::events::MessageSendEvent::SendShutdown
 	pub fn close_channel(&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey) -> Result<(), APIError> {
 		self.close_channel_internal(channel_id, counterparty_node_id, None, None)
@@ -2671,8 +2671,8 @@ where
 	/// the channel being closed or not:
 	///  * If we are the channel initiator, we will pay at least this feerate on the closing
 	///    transaction. The upper-bound is set by
-	///    [`ChannelConfig::force_close_avoidance_max_fee_satoshis`] plus our [`Normal`] fee
-	///    estimate (or `target_feerate_sat_per_1000_weight`, if it is greater).
+	///    [`ChannelConfig::force_close_avoidance_max_fee_satoshis`] plus our [`NonAnchorChannelFee`]
+	///    fee estimate (or `target_feerate_sat_per_1000_weight`, if it is greater).
 	///  * If our counterparty is the channel initiator, we will refuse to accept a channel closure
 	///    transaction feerate below `target_feerate_sat_per_1000_weight` (or the feerate which
 	///    will appear on a force-closure transaction, whichever is lower).
@@ -2690,8 +2690,7 @@ where
 	/// channel.
 	///
 	/// [`ChannelConfig::force_close_avoidance_max_fee_satoshis`]: crate::util::config::ChannelConfig::force_close_avoidance_max_fee_satoshis
-	/// [`Background`]: crate::chain::chaininterface::ConfirmationTarget::Background
-	/// [`Normal`]: crate::chain::chaininterface::ConfirmationTarget::Normal
+	/// [`NonAnchorChannelFee`]: crate::chain::chaininterface::ConfirmationTarget::NonAnchorChannelFee
 	/// [`SendShutdown`]: crate::events::MessageSendEvent::SendShutdown
 	pub fn close_channel_with_feerate_and_script(&self, channel_id: &ChannelId, counterparty_node_id: &PublicKey, target_feerate_sats_per_1000_weight: Option<u32>, shutdown_script: Option<ShutdownScript>) -> Result<(), APIError> {
 		self.close_channel_internal(channel_id, counterparty_node_id, target_feerate_sats_per_1000_weight, shutdown_script)
@@ -4754,8 +4753,8 @@ where
 		PersistenceNotifierGuard::optionally_notify(self, || {
 			let mut should_persist = NotifyOption::SkipPersistNoEvents;
 
-			let normal_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal);
-			let min_mempool_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::MempoolMinimum);
+			let non_anchor_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee);
+			let anchor_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::AnchorChannelFee);
 
 			let per_peer_state = self.per_peer_state.read().unwrap();
 			for (_cp_id, peer_state_mutex) in per_peer_state.iter() {
@@ -4765,9 +4764,9 @@ where
 					|(chan_id, phase)| if let ChannelPhase::Funded(chan) = phase { Some((chan_id, chan)) } else { None }
 				) {
 					let new_feerate = if chan.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
-						min_mempool_feerate
+						anchor_feerate
 					} else {
-						normal_feerate
+						non_anchor_feerate
 					};
 					let chan_needs_persist = self.update_channel_fee(chan_id, chan, new_feerate);
 					if chan_needs_persist == NotifyOption::DoPersist { should_persist = NotifyOption::DoPersist; }
@@ -4799,8 +4798,8 @@ where
 		PersistenceNotifierGuard::optionally_notify(self, || {
 			let mut should_persist = NotifyOption::SkipPersistNoEvents;
 
-			let normal_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::Normal);
-			let min_mempool_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::MempoolMinimum);
+			let non_anchor_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::NonAnchorChannelFee);
+			let anchor_feerate = self.fee_estimator.bounded_sat_per_1000_weight(ConfirmationTarget::AnchorChannelFee);
 
 			let mut handle_errors: Vec<(Result<(), _>, _)> = Vec::new();
 			let mut timed_out_mpp_htlcs = Vec::new();
@@ -4847,9 +4846,9 @@ where
 						match phase {
 							ChannelPhase::Funded(chan) => {
 								let new_feerate = if chan.context.get_channel_type().supports_anchors_zero_fee_htlc_tx() {
-									min_mempool_feerate
+									anchor_feerate
 								} else {
-									normal_feerate
+									non_anchor_feerate
 								};
 								let chan_needs_persist = self.update_channel_fee(chan_id, chan, new_feerate);
 								if chan_needs_persist == NotifyOption::DoPersist { should_persist = NotifyOption::DoPersist; }