diff --git a/lightning/src/chain/chaininterface.rs b/lightning/src/chain/chaininterface.rs index 9a45a88ffe4..ceb004da2f4 100644 --- a/lightning/src/chain/chaininterface.rs +++ b/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 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); + } +} \ No newline at end of file diff --git a/lightning/src/chain/package.rs b/lightning/src/chain/package.rs index b0961293c07..10a25b243c9 100644 --- a/lightning/src/chain/package.rs +++ b/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(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 { diff --git a/lightning/src/ln/channel.rs b/lightning/src/ln/channel.rs index 9b47770f120..fd79528ffab 100644 --- a/lightning/src/ln/channel.rs +++ b/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 Channel { 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 Channel { fn check_remote_fee(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 Channel { 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. diff --git a/lightning/src/ln/channelmanager.rs b/lightning/src/ln/channelmanager.rs index c277a7438da..c45a90ebf0d 100644 --- a/lightning/src/ln/channelmanager.rs +++ b/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 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 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();