From 20742f4c21d921f92b1c0b1ee88b00946a0da9cf Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Sat, 10 Feb 2024 19:13:03 +0100 Subject: [PATCH] Disallow empty fee estimates on Mainnet We generally want to properly be able to detect whenever a fee estimation would fail, as we need to fail startup if we can't retrieve the latest fee rates. However, currently the `convert_fee_estimates` function would fallback to a default of 1sat/vbyte if the retrieved estimate map is empty. This is fine/potentially needed for, e.g., Signet where Esplora's `fee-estimates` endpoint would return an empty dictionary. However, we need to detect the empty map and fail our fee estimation if we encounter such a case, rather than using the 1 sat/vbyte fallback on mainnet, where differences in fee estimation could lead to costly force-closures. Here we do this and just return a `FeerateEstimationFailed` if we hit such a case. --- src/builder.rs | 14 ++++++++++---- src/fee_estimator.rs | 20 ++++++++++++++++---- 2 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index 072b6e85e..d8519815e 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -508,8 +508,11 @@ fn build_with_store_internal( tx_sync.client().clone(), Arc::clone(&logger), )); - let fee_estimator = - Arc::new(OnchainFeeEstimator::new(tx_sync.client().clone(), Arc::clone(&logger))); + let fee_estimator = Arc::new(OnchainFeeEstimator::new( + tx_sync.client().clone(), + Arc::clone(&config), + Arc::clone(&logger), + )); (blockchain, tx_sync, tx_broadcaster, fee_estimator) } None => { @@ -523,8 +526,11 @@ fn build_with_store_internal( tx_sync.client().clone(), Arc::clone(&logger), )); - let fee_estimator = - Arc::new(OnchainFeeEstimator::new(tx_sync.client().clone(), Arc::clone(&logger))); + let fee_estimator = Arc::new(OnchainFeeEstimator::new( + tx_sync.client().clone(), + Arc::clone(&config), + Arc::clone(&logger), + )); (blockchain, tx_sync, tx_broadcaster, fee_estimator) } }; diff --git a/src/fee_estimator.rs b/src/fee_estimator.rs index c12242784..b94fa7a7b 100644 --- a/src/fee_estimator.rs +++ b/src/fee_estimator.rs @@ -1,5 +1,5 @@ use crate::logger::{log_error, log_trace, Logger}; -use crate::Error; +use crate::{Config, Error}; use lightning::chain::chaininterface::{ ConfirmationTarget, FeeEstimator, FEERATE_FLOOR_SATS_PER_KW, @@ -8,11 +8,12 @@ use lightning::chain::chaininterface::{ use bdk::FeeRate; use esplora_client::AsyncClient as EsploraClient; +use bitcoin::Network; use bitcoin::blockdata::weight::Weight; use std::collections::HashMap; use std::ops::Deref; -use std::sync::RwLock; +use std::sync::{Arc, RwLock}; pub(crate) struct OnchainFeeEstimator where @@ -20,6 +21,7 @@ where { fee_rate_cache: RwLock>, esplora_client: EsploraClient, + config: Arc, logger: L, } @@ -27,9 +29,9 @@ impl OnchainFeeEstimator where L::Target: Logger, { - pub(crate) fn new(esplora_client: EsploraClient, logger: L) -> Self { + pub(crate) fn new(esplora_client: EsploraClient, config: Arc, logger: L) -> Self { let fee_rate_cache = RwLock::new(HashMap::new()); - Self { fee_rate_cache, esplora_client, logger } + Self { fee_rate_cache, esplora_client, config, logger } } pub(crate) async fn update_fee_estimates(&self) -> Result<(), Error> { @@ -61,6 +63,16 @@ where Error::FeerateEstimationUpdateFailed })?; + if estimates.is_empty() && self.config.network == Network::Bitcoin { + // Ensure we fail if we didn't receive any estimates. + log_error!( + self.logger, + "Failed to retrieve fee rate estimates for {:?}: empty fee estimates are dissallowed on Mainnet.", + target, + ); + return Err(Error::FeerateEstimationUpdateFailed); + } + let converted_estimates = esplora_client::convert_fee_rate(num_blocks, estimates) .map_err(|e| { log_error!(