Merge #836

836: Replace fee rate fallbacks in settings with constant r=luckysori a=luckysori

The fallbacks were only ever used if we could never get an answer from the fee rate server. As such, it didn't make sense for them to be configurable at runtime.

We move them to reduce complexity and because there is minimal benefit in being able to configure values that will very rarely be used.

Co-authored-by: Lucas Soriano del Pino <[email protected]>

coordinator/src/node.rs

@@ -29,7 +29,6 @@ use lightning::ln::channelmanager::ChannelDetails;
 use ln_dlc_node::node::dlc_message_name;
 use ln_dlc_node::node::sub_channel_message_name;
 use ln_dlc_node::node::InMemoryStore;
-use ln_dlc_node::FeeRateFallbacks;
 use ln_dlc_node::WalletSettings;
 use rust_decimal::prelude::ToPrimitive;
 use rust_decimal::Decimal;
@@ -52,8 +51,6 @@ pub struct NodeSettings {
     // At times, we want to disallow opening new positions (e.g. before
     // scheduled upgrade)
     pub allow_opening_positions: bool,
-    pub fallback_tx_fee_rate_normal: u32,
-    pub fallback_tx_fee_rate_high_priority: u32,
     pub max_allowed_tx_fee_rate_when_opening_channel: Option<u32>,
 }
 
@@ -64,21 +61,12 @@ impl NodeSettings {
                 .max_allowed_tx_fee_rate_when_opening_channel,
         }
     }
-
-    fn as_fee_rate_fallbacks(&self) -> FeeRateFallbacks {
-        FeeRateFallbacks {
-            normal_priority: self.fallback_tx_fee_rate_normal,
-            high_priority: self.fallback_tx_fee_rate_high_priority,
-        }
-    }
 }
 
 impl Default for NodeSettings {
     fn default() -> Self {
         Self {
             allow_opening_positions: true,
-            fallback_tx_fee_rate_normal: 2000,
-            fallback_tx_fee_rate_high_priority: 5000,
             max_allowed_tx_fee_rate_when_opening_channel: None,
         }
     }
@@ -110,11 +98,6 @@ impl Node {
         // Forward relevant settings down to the wallet
         let wallet_settings = settings.as_wallet_settings();
         self.inner.wallet().update_settings(wallet_settings).await;
-
-        let fee_rate_fallbacks = settings.as_fee_rate_fallbacks();
-        self.inner
-            .fee_rate_estimator
-            .update_fallbacks(fee_rate_fallbacks);
     }
 
     /// Returns true or false, whether we can find an usable channel with the provided trader.

coordinator/src/settings.rs

@@ -89,8 +89,6 @@ impl Settings {
     pub fn as_node_settings(&self) -> NodeSettings {
         NodeSettings {
             allow_opening_positions: self.new_positions_enabled,
-            fallback_tx_fee_rate_normal: self.fallback_tx_fee_rate_normal,
-            fallback_tx_fee_rate_high_priority: self.fallback_tx_fee_rate_high_priority,
             max_allowed_tx_fee_rate_when_opening_channel: self
                 .max_allowed_tx_fee_rate_when_opening_channel,
         }

crates/ln-dlc-node/src/fee_rate_estimator.rs

@@ -17,53 +17,60 @@ const CONFIRMATION_TARGETS: [(ConfirmationTarget, usize); 3] = [
     (ConfirmationTarget::HighPriority, 3),
 ];
 
+/// Default values used when constructing the [`FeeRateEstimator`] if the fee rate sever cannot give
+/// us up-to-date values.
+///
+/// In sats/kwu.
+const FEE_RATE_DEFAULTS: [(ConfirmationTarget, u32); 3] = [
+    (ConfirmationTarget::Background, FEERATE_FLOOR_SATS_PER_KW),
+    (ConfirmationTarget::Normal, 2000),
+    (ConfirmationTarget::HighPriority, 5000),
+];
+
 pub struct FeeRateEstimator {
     client: esplora_client::BlockingClient,
     fee_rate_cache: RwLock<HashMap<ConfirmationTarget, FeeRate>>,
-    fallbacks: RwLock<FeeRateFallbacks>,
-}
-
-#[derive(Debug)]
-pub struct FeeRateFallbacks {
-    pub normal_priority: u32,
-    pub high_priority: u32,
-}
-
-impl Default for FeeRateFallbacks {
-    fn default() -> Self {
-        Self {
-            normal_priority: 2000,
-            high_priority: 5000,
-        }
-    }
 }
 
 impl FeeRateEstimator {
+    /// Constructor for the [`FeeRateEstimator`].
     pub fn new(esplora_url: String) -> Self {
         let client = esplora_client::BlockingClient::from_agent(esplora_url, ureq::agent());
 
-        let fee_rate_cache = RwLock::new(HashMap::default());
-        let fallbacks = RwLock::new(FeeRateFallbacks::default());
+        let initial_fee_rates = match client.get_fee_estimates() {
+            Ok(estimates) => {
+                HashMap::from_iter(CONFIRMATION_TARGETS.into_iter().map(|(target, n_blocks)| {
+                    let fee_rate = esplora_client::convert_fee_rate(n_blocks, estimates.clone())
+                        .expect("fee rates for our confirmation targets");
+                    let fee_rate = FeeRate::from_sat_per_vb(fee_rate);
+
+                    (target, fee_rate)
+                }))
+            }
+            Err(e) => {
+                tracing::warn!(defaults = ?FEE_RATE_DEFAULTS, "Initializing fee rate cache with default values: {e:#}");
+
+                HashMap::from_iter(
+                    FEE_RATE_DEFAULTS.into_iter().map(|(target, fee_rate)| {
+                        (target, FeeRate::from_sat_per_kwu(fee_rate as f32))
+                    }),
+                )
+            }
+        };
+
+        let fee_rate_cache = RwLock::new(initial_fee_rates);
 
         Self {
             client,
             fee_rate_cache,
-            fallbacks,
         }
     }
 
     pub(crate) fn get(&self, target: ConfirmationTarget) -> FeeRate {
         self.cache_read_lock()
             .get(&target)
             .copied()
-            .unwrap_or_else(|| {
-                let fee_rate = match target {
-                    ConfirmationTarget::Background => FEERATE_FLOOR_SATS_PER_KW,
-                    ConfirmationTarget::Normal => self.fallbacks_read_lock().normal_priority,
-                    ConfirmationTarget::HighPriority => self.fallbacks_read_lock().high_priority,
-                };
-                FeeRate::from_sat_per_kwu(fee_rate as f32)
-            })
+            .expect("to have entries for all confirmation targets")
     }
 
     #[autometrics]
@@ -87,19 +94,6 @@ impl FeeRateEstimator {
         Ok(())
     }
 
-    pub fn update_fallbacks(&self, fallbacks: FeeRateFallbacks) {
-        tracing::info!(?fallbacks, "Updating fee rate fallbacks");
-        *self.fallbacks_write_lock() = fallbacks;
-    }
-
-    fn fallbacks_read_lock(&self) -> RwLockReadGuard<FeeRateFallbacks> {
-        self.fallbacks.read().expect("RwLock to not be poisoned")
-    }
-
-    fn fallbacks_write_lock(&self) -> RwLockWriteGuard<FeeRateFallbacks> {
-        self.fallbacks.write().expect("RwLock to not be poisoned")
-    }
-
     fn cache_read_lock(&self) -> RwLockReadGuard<HashMap<ConfirmationTarget, FeeRate>> {
         self.fee_rate_cache
             .read()

crates/ln-dlc-node/src/lib.rs

@@ -38,7 +38,6 @@ pub mod seed;
 #[cfg(test)]
 mod tests;
 
-pub use fee_rate_estimator::FeeRateFallbacks;
 pub use ldk_node_wallet::WalletSettings;
 pub use ln::ChannelDetails;
 pub use ln::DlcChannelDetails;