chaininterface: Add LowerBoundedFeeEstimator wrapper

The LowerBoundedFeeEstimator wrapper is used internally to wrap a
user-provided FeeEstimator to ensure that we clip fee estimates to no
less than 253 sats/KW.

lightning/src/chain/chaininterface.rs

@@ -13,6 +13,8 @@
 //! Includes traits for monitoring and receiving notifications of new blocks and block
 //! disconnections, transaction broadcasting, and feerate information requests.
 
+use core::{cmp, ops::Deref};
+
 use bitcoin::blockdata::transaction::Transaction;
 
 /// An interface to send a transaction to the Bitcoin network.
@@ -41,14 +43,75 @@ pub enum ConfirmationTarget {
 pub trait FeeEstimator {
 	/// Gets estimated satoshis of fee required per 1000 Weight-Units.
 	///
-	/// Must return a value no smaller than 253 (ie 1 satoshi-per-byte rounded up to ensure later
-	/// round-downs don't put us below 1 satoshi-per-byte).
-	///
-	/// This method can be implemented with the following unit conversions:
-	///  * max(satoshis-per-byte * 250, 253)
-	///  * max(satoshis-per-kbyte / 4, 253)
+	/// LDK will wrap this method and ensure that the value returned is no smaller than 253
+	/// (ie 1 satoshi-per-byte rounded up to ensure later round-downs don't put us below 1 satoshi-per-byte).
 	fn get_est_sat_per_1000_weight(&self, confirmation_target: ConfirmationTarget) -> u32;
 }
 
 /// Minimum relay fee as required by bitcoin network mempool policy.
 pub const MIN_RELAY_FEE_SAT_PER_1000_WEIGHT: u64 = 4000;
+/// Minimum feerate that takes a sane approach to rounding
+pub const FEERATE_FLOOR_SATS_PER_KW: u32 = 253;
+
+/// Wraps a `FeeEstimator` so that any fee estimations provided by it
+/// are bounded below by `FEERATE_FLOOR_SATS_PER_KW` (253 sats/KW)
+pub(crate) struct LowerBoundedFeeEstimator<'a, F: Deref>(pub(crate) &'a F)
+where
+	F::Target: FeeEstimator;
+
+impl<'a, F: Deref> LowerBoundedFeeEstimator<'a, F>
+where
+	F::Target: FeeEstimator,
+{
+	pub(crate) fn new(fee_estimator: &'a F) -> Self
+	where
+		F::Target: FeeEstimator,
+	{
+		LowerBoundedFeeEstimator(fee_estimator)
+	}
+}
+
+impl<F: Deref> FeeEstimator for LowerBoundedFeeEstimator<'_, F>
+where
+	F::Target: FeeEstimator,
+{
+	fn get_est_sat_per_1000_weight(&self, confirmation_target: ConfirmationTarget) -> u32 {
+		cmp::max(
+			self.0.get_est_sat_per_1000_weight(confirmation_target),
+			FEERATE_FLOOR_SATS_PER_KW,
+		)
+	}
+}
+
+#[cfg(test)]
+mod tests {
+	use super::{FEERATE_FLOOR_SATS_PER_KW, LowerBoundedFeeEstimator, ConfirmationTarget, FeeEstimator};
+
+	struct TestFeeEstimator {
+		sat_per_kw: u32,
+	}
+
+	impl FeeEstimator for TestFeeEstimator {
+		fn get_est_sat_per_1000_weight(&self, _: ConfirmationTarget) -> u32 {
+			self.sat_per_kw
+		}
+	}
+
+	#[test]
+	fn test_fee_estimator_less_than_floor() {
+		let sat_per_kw = FEERATE_FLOOR_SATS_PER_KW - 1;
+		let test_fee_estimator = &TestFeeEstimator { sat_per_kw };
+		let fee_estimator = LowerBoundedFeeEstimator::new(&test_fee_estimator);
+
+		assert_eq!(fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background), FEERATE_FLOOR_SATS_PER_KW);
+	}
+
+	#[test]
+	fn test_fee_estimator_greater_than_floor() {
+		let sat_per_kw = FEERATE_FLOOR_SATS_PER_KW + 1;
+		let test_fee_estimator = &TestFeeEstimator { sat_per_kw };
+		let fee_estimator = LowerBoundedFeeEstimator::new(&test_fee_estimator);
+
+		assert_eq!(fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Background), sat_per_kw);
+	}
+}

lightning/src/chain/package.rs

@@ -38,6 +38,8 @@ use core::mem;
 use core::ops::Deref;
 use bitcoin::Witness;
 
+use super::chaininterface::LowerBoundedFeeEstimator;
+
 const MAX_ALLOC_SIZE: usize = 64*1024;
 
 
@@ -776,6 +778,7 @@ fn compute_fee_from_spent_amounts<F: Deref, L: Deref>(input_amounts: u64, predic
 	where F::Target: FeeEstimator,
 	      L::Target: Logger,
 {
+	let fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
 	let mut updated_feerate = fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::HighPriority) as u64;
 	let mut fee = updated_feerate * (predicted_weight as u64) / 1000;
 	if input_amounts <= fee {

lightning/src/ln/channel.rs

@@ -31,7 +31,7 @@ use ln::channelmanager::{CounterpartyForwardingInfo, PendingHTLCStatus, HTLCSour
 use ln::chan_utils::{CounterpartyCommitmentSecrets, TxCreationKeys, HTLCOutputInCommitment, htlc_success_tx_weight, htlc_timeout_tx_weight, make_funding_redeemscript, ChannelPublicKeys, CommitmentTransaction, HolderCommitmentTransaction, ChannelTransactionParameters, CounterpartyChannelTransactionParameters, MAX_HTLCS, get_commitment_transaction_number_obscure_factor, ClosingTransaction};
 use ln::chan_utils;
 use chain::BestBlock;
-use chain::chaininterface::{FeeEstimator,ConfirmationTarget};
+use chain::chaininterface::{FeeEstimator, ConfirmationTarget, LowerBoundedFeeEstimator};
 use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, LATENCY_GRACE_PERIOD_BLOCKS};
 use chain::transaction::{OutPoint, TransactionData};
 use chain::keysinterface::{Sign, KeysInterface};
@@ -888,6 +888,7 @@ impl<Signer: Sign> Channel<Signer> {
 		let holder_selected_contest_delay = config.channel_handshake_config.our_to_self_delay;
 		let holder_signer = keys_provider.get_channel_signer(false, channel_value_satoshis);
 		let pubkeys = holder_signer.pubkeys().clone();
+		let fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
 
 		if !their_features.supports_wumbo() && channel_value_satoshis > MAX_FUNDING_SATOSHIS_NO_WUMBO {
 			return Err(APIError::APIMisuseError{err: format!("funding_value must not exceed {}, it was {}", MAX_FUNDING_SATOSHIS_NO_WUMBO, channel_value_satoshis)});
@@ -1047,6 +1048,7 @@ impl<Signer: Sign> Channel<Signer> {
 	fn check_remote_fee<F: Deref>(fee_estimator: &F, feerate_per_kw: u32) -> Result<(), ChannelError>
 		where F::Target: FeeEstimator
 	{
+		let fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
 		// We only bound the fee updates on the upper side to prevent completely absurd feerates,
 		// always accepting up to 25 sat/vByte or 10x our fee estimator's "High Priority" fee.
 		// We generally don't care too much if they set the feerate to something very high, but it
@@ -3987,6 +3989,7 @@ impl<Signer: Sign> Channel<Signer> {
 		where F::Target: FeeEstimator
 	{
 		if let Some((min, max)) = self.closing_fee_limits { return (min, max); }
+		let fee_estimator = LowerBoundedFeeEstimator::new(fee_estimator);
 
 		// Propose a range from our current Background feerate to our Normal feerate plus our
 		// force_close_avoidance_max_fee_satoshis.

lightning/src/ln/channelmanager.rs

@@ -36,7 +36,7 @@ use bitcoin::secp256k1;
 
 use chain;
 use chain::{Confirm, ChannelMonitorUpdateErr, Watch, BestBlock};
-use chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator};
+use chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator, LowerBoundedFeeEstimator};
 use chain::channelmonitor::{ChannelMonitor, ChannelMonitorUpdate, ChannelMonitorUpdateStep, HTLC_FAIL_BACK_BUFFER, CLTV_CLAIM_BUFFER, LATENCY_GRACE_PERIOD_BLOCKS, ANTI_REORG_DELAY, MonitorEvent, CLOSED_CHANNEL_UPDATE_ID};
 use chain::transaction::{OutPoint, TransactionData};
 // Since this struct is returned in `list_channels` methods, expose it here in case users want to
@@ -3436,7 +3436,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 		PersistenceNotifierGuard::optionally_notify(&self.total_consistency_lock, &self.persistence_notifier, || {
 			let mut should_persist = NotifyOption::SkipPersist;
 
-			let new_feerate = self.fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal);
+			let new_feerate = LowerBoundedFeeEstimator::new(&self.fee_estimator).get_est_sat_per_1000_weight(ConfirmationTarget::Normal);
 
 			let mut handle_errors = Vec::new();
 			{
@@ -3473,7 +3473,7 @@ impl<Signer: Sign, M: Deref, T: Deref, K: Deref, F: Deref, L: Deref> ChannelMana
 			let mut should_persist = NotifyOption::SkipPersist;
 			if self.process_background_events() { should_persist = NotifyOption::DoPersist; }
 
-			let new_feerate = self.fee_estimator.get_est_sat_per_1000_weight(ConfirmationTarget::Normal);
+			let new_feerate = LowerBoundedFeeEstimator::new(&self.fee_estimator).get_est_sat_per_1000_weight(ConfirmationTarget::Normal);
 
 			let mut handle_errors = Vec::new();
 			let mut timed_out_mpp_htlcs = Vec::new();