From b4e06d514ebb9cd1d2169d8e3c8d5d552d1bd7b1 Mon Sep 17 00:00:00 2001 From: Lucas Soriano del Pino Date: Wed, 28 Jun 2023 12:43:07 +1000 Subject: [PATCH] fix: Replace fee rate fallbacks in settings with constant 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. --- coordinator/src/node.rs | 17 ----- coordinator/src/settings.rs | 2 - crates/ln-dlc-node/src/fee_rate_estimator.rs | 74 +++++++++----------- crates/ln-dlc-node/src/lib.rs | 1 - 4 files changed, 34 insertions(+), 60 deletions(-) diff --git a/coordinator/src/node.rs b/coordinator/src/node.rs index 376c03883..6edd0cf0c 100644 --- a/coordinator/src/node.rs +++ b/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, } @@ -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. diff --git a/coordinator/src/settings.rs b/coordinator/src/settings.rs index 1a0cdc560..54e2dbd57 100644 --- a/coordinator/src/settings.rs +++ b/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, } diff --git a/crates/ln-dlc-node/src/fee_rate_estimator.rs b/crates/ln-dlc-node/src/fee_rate_estimator.rs index 42d32a29a..e5ee54869 100644 --- a/crates/ln-dlc-node/src/fee_rate_estimator.rs +++ b/crates/ln-dlc-node/src/fee_rate_estimator.rs @@ -17,38 +17,52 @@ 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>, - fallbacks: RwLock, -} - -#[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, } } @@ -56,14 +70,7 @@ impl FeeRateEstimator { 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 { - self.fallbacks.read().expect("RwLock to not be poisoned") - } - - fn fallbacks_write_lock(&self) -> RwLockWriteGuard { - self.fallbacks.write().expect("RwLock to not be poisoned") - } - fn cache_read_lock(&self) -> RwLockReadGuard> { self.fee_rate_cache .read() diff --git a/crates/ln-dlc-node/src/lib.rs b/crates/ln-dlc-node/src/lib.rs index 6ece8983a..bab2eb7c1 100644 --- a/crates/ln-dlc-node/src/lib.rs +++ b/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;