Merge pull request #352 from tnull/2024-08-fee-estimator-refactor

Author: tnull

src/event.rs

@@ -6,6 +6,7 @@ use crate::{
 };
 
 use crate::connection::ConnectionManager;
+use crate::fee_estimator::ConfirmationTarget;
 
 use crate::payment::store::{
 	PaymentDetails, PaymentDetailsUpdate, PaymentDirection, PaymentKind, PaymentStatus,
@@ -18,7 +19,6 @@ use crate::io::{
 };
 use crate::logger::{log_debug, log_error, log_info, Logger};
 
-use lightning::chain::chaininterface::ConfirmationTarget;
 use lightning::events::bump_transaction::BumpTransactionEvent;
 use lightning::events::{ClosureReason, PaymentPurpose};
 use lightning::events::{Event as LdkEvent, PaymentFailureReason};
@@ -398,7 +398,7 @@ where
 			} => {
 				// Construct the raw transaction with the output that is paid the amount of the
 				// channel.
-				let confirmation_target = ConfirmationTarget::NonAnchorChannelFee;
+				let confirmation_target = ConfirmationTarget::ChannelFunding;
 
 				// We set nLockTime to the current height to discourage fee sniping.
 				let cur_height = self.channel_manager.current_best_block().height;

src/fee_estimator.rs

@@ -2,9 +2,9 @@ use crate::config::FEE_RATE_CACHE_UPDATE_TIMEOUT_SECS;
 use crate::logger::{log_error, log_trace, Logger};
 use crate::{Config, Error};
 
-use lightning::chain::chaininterface::{
-	ConfirmationTarget, FeeEstimator, FEERATE_FLOOR_SATS_PER_KW,
-};
+use lightning::chain::chaininterface::ConfirmationTarget as LdkConfirmationTarget;
+use lightning::chain::chaininterface::FeeEstimator as LdkFeeEstimator;
+use lightning::chain::chaininterface::FEERATE_FLOOR_SATS_PER_KW;
 
 use bdk::FeeRate;
 use esplora_client::AsyncClient as EsploraClient;
@@ -17,6 +17,26 @@ use std::ops::Deref;
 use std::sync::{Arc, RwLock};
 use std::time::Duration;
 
+#[derive(Clone, Copy, Debug, Hash, PartialEq, Eq)]
+pub(crate) enum ConfirmationTarget {
+	/// The default target for onchain payments.
+	OnchainPayment,
+	/// The target used for funding transactions.
+	ChannelFunding,
+	/// Targets used by LDK.
+	Lightning(LdkConfirmationTarget),
+}
+
+pub(crate) trait FeeEstimator {
+	fn estimate_fee_rate(&self, confirmation_target: ConfirmationTarget) -> FeeRate;
+}
+
+impl From<LdkConfirmationTarget> for ConfirmationTarget {
+	fn from(value: LdkConfirmationTarget) -> Self {
+		Self::Lightning(value)
+	}
+}
+
 pub(crate) struct OnchainFeeEstimator<L: Deref>
 where
 	L::Target: Logger,
@@ -61,23 +81,30 @@ where
 		}
 
 		let confirmation_targets = vec![
-			ConfirmationTarget::OnChainSweep,
-			ConfirmationTarget::MinAllowedAnchorChannelRemoteFee,
-			ConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee,
-			ConfirmationTarget::AnchorChannelFee,
-			ConfirmationTarget::NonAnchorChannelFee,
-			ConfirmationTarget::ChannelCloseMinimum,
-			ConfirmationTarget::OutputSpendingFee,
+			ConfirmationTarget::OnchainPayment,
+			ConfirmationTarget::ChannelFunding,
+			LdkConfirmationTarget::OnChainSweep.into(),
+			LdkConfirmationTarget::MinAllowedAnchorChannelRemoteFee.into(),
+			LdkConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee.into(),
+			LdkConfirmationTarget::AnchorChannelFee.into(),
+			LdkConfirmationTarget::NonAnchorChannelFee.into(),
+			LdkConfirmationTarget::ChannelCloseMinimum.into(),
+			LdkConfirmationTarget::OutputSpendingFee.into(),
 		];
+
 		for target in confirmation_targets {
 			let num_blocks = match target {
-				ConfirmationTarget::OnChainSweep => 6,
-				ConfirmationTarget::MinAllowedAnchorChannelRemoteFee => 1008,
-				ConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee => 144,
-				ConfirmationTarget::AnchorChannelFee => 1008,
-				ConfirmationTarget::NonAnchorChannelFee => 12,
-				ConfirmationTarget::ChannelCloseMinimum => 144,
-				ConfirmationTarget::OutputSpendingFee => 12,
+				ConfirmationTarget::OnchainPayment => 6,
+				ConfirmationTarget::ChannelFunding => 12,
+				ConfirmationTarget::Lightning(ldk_target) => match ldk_target {
+					LdkConfirmationTarget::OnChainSweep => 6,
+					LdkConfirmationTarget::MinAllowedAnchorChannelRemoteFee => 1008,
+					LdkConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee => 144,
+					LdkConfirmationTarget::AnchorChannelFee => 1008,
+					LdkConfirmationTarget::NonAnchorChannelFee => 12,
+					LdkConfirmationTarget::ChannelCloseMinimum => 144,
+					LdkConfirmationTarget::OutputSpendingFee => 12,
+				},
 			};
 
 			let converted_estimates =
@@ -96,7 +123,9 @@ where
 			// LDK 0.0.118 introduced changes to the `ConfirmationTarget` semantics that
 			// require some post-estimation adjustments to the fee rates, which we do here.
 			let adjusted_fee_rate = match target {
-				ConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee => {
+				ConfirmationTarget::Lightning(
+					LdkConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee,
+				) => {
 					let slightly_less_than_background =
 						fee_rate.fee_wu(Weight::from_wu(1000)) - 250;
 					FeeRate::from_sat_per_kwu(slightly_less_than_background as f32)
@@ -115,33 +144,53 @@ where
 		}
 		Ok(())
 	}
+}
 
-	pub(crate) fn estimate_fee_rate(&self, confirmation_target: ConfirmationTarget) -> FeeRate {
+impl<L: Deref> FeeEstimator for OnchainFeeEstimator<L>
+where
+	L::Target: Logger,
+{
+	fn estimate_fee_rate(&self, confirmation_target: ConfirmationTarget) -> FeeRate {
 		let locked_fee_rate_cache = self.fee_rate_cache.read().unwrap();
 
 		let fallback_sats_kwu = match confirmation_target {
-			ConfirmationTarget::OnChainSweep => 5000,
-			ConfirmationTarget::MinAllowedAnchorChannelRemoteFee => FEERATE_FLOOR_SATS_PER_KW,
-			ConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee => FEERATE_FLOOR_SATS_PER_KW,
-			ConfirmationTarget::AnchorChannelFee => 500,
-			ConfirmationTarget::NonAnchorChannelFee => 1000,
-			ConfirmationTarget::ChannelCloseMinimum => 500,
-			ConfirmationTarget::OutputSpendingFee => 1000,
+			ConfirmationTarget::OnchainPayment => 5000,
+			ConfirmationTarget::ChannelFunding => 1000,
+			ConfirmationTarget::Lightning(ldk_target) => match ldk_target {
+				LdkConfirmationTarget::OnChainSweep => 5000,
+				LdkConfirmationTarget::MinAllowedAnchorChannelRemoteFee => {
+					FEERATE_FLOOR_SATS_PER_KW
+				},
+				LdkConfirmationTarget::MinAllowedNonAnchorChannelRemoteFee => {
+					FEERATE_FLOOR_SATS_PER_KW
+				},
+				LdkConfirmationTarget::AnchorChannelFee => 500,
+				LdkConfirmationTarget::NonAnchorChannelFee => 1000,
+				LdkConfirmationTarget::ChannelCloseMinimum => 500,
+				LdkConfirmationTarget::OutputSpendingFee => 1000,
+			},
 		};
 
 		// We'll fall back on this, if we really don't have any other information.
 		let fallback_rate = FeeRate::from_sat_per_kwu(fallback_sats_kwu as f32);
 
-		*locked_fee_rate_cache.get(&confirmation_target).unwrap_or(&fallback_rate)
+		let estimate = *locked_fee_rate_cache.get(&confirmation_target).unwrap_or(&fallback_rate);
+
+		// Currently we assume every transaction needs to at least be relayable, which is why we
+		// enforce a lower bound of `FEERATE_FLOOR_SATS_PER_KW`.
+		let weight_units = Weight::from_wu(1000);
+		FeeRate::from_wu(
+			estimate.fee_wu(weight_units).max(FEERATE_FLOOR_SATS_PER_KW as u64),
+			weight_units,
+		)
 	}
 }
 
-impl<L: Deref> FeeEstimator for OnchainFeeEstimator<L>
+impl<L: Deref> LdkFeeEstimator for OnchainFeeEstimator<L>
 where
 	L::Target: Logger,
 {
-	fn get_est_sat_per_1000_weight(&self, confirmation_target: ConfirmationTarget) -> u32 {
-		(self.estimate_fee_rate(confirmation_target).fee_wu(Weight::from_wu(1000)) as u32)
-			.max(FEERATE_FLOOR_SATS_PER_KW)
+	fn get_est_sat_per_1000_weight(&self, confirmation_target: LdkConfirmationTarget) -> u32 {
+		self.estimate_fee_rate(confirmation_target.into()).fee_wu(Weight::from_wu(1000)) as u32
 	}
 }

src/wallet.rs

@@ -1,9 +1,10 @@
 use crate::logger::{log_error, log_info, log_trace, Logger};
 
 use crate::config::BDK_WALLET_SYNC_TIMEOUT_SECS;
+use crate::fee_estimator::{ConfirmationTarget, FeeEstimator};
 use crate::Error;
 
-use lightning::chain::chaininterface::{BroadcasterInterface, ConfirmationTarget, FeeEstimator};
+use lightning::chain::chaininterface::BroadcasterInterface;
 
 use lightning::events::bump_transaction::{Utxo, WalletSource};
 use lightning::ln::msgs::{DecodeError, UnsignedGossipMessage};
@@ -18,8 +19,7 @@ use lightning::util::message_signing;
 use bdk::blockchain::EsploraBlockchain;
 use bdk::database::BatchDatabase;
 use bdk::wallet::AddressIndex;
-use bdk::{Balance, FeeRate};
-use bdk::{SignOptions, SyncOptions};
+use bdk::{Balance, SignOptions, SyncOptions};
 
 use bitcoin::address::{Payload, WitnessVersion};
 use bitcoin::bech32::u5;
@@ -43,7 +43,7 @@ enum WalletSyncStatus {
 	InProgress { subscribers: tokio::sync::broadcast::Sender<Result<(), Error>> },
 }
 
-pub struct Wallet<D, B: Deref, E: Deref, L: Deref>
+pub(crate) struct Wallet<D, B: Deref, E: Deref, L: Deref>
 where
 	D: BatchDatabase,
 	B::Target: BroadcasterInterface,
@@ -153,9 +153,7 @@ where
 		&self, output_script: ScriptBuf, value_sats: u64, confirmation_target: ConfirmationTarget,
 		locktime: LockTime,
 	) -> Result<Transaction, Error> {
-		let fee_rate = FeeRate::from_sat_per_kwu(
-			self.fee_estimator.get_est_sat_per_1000_weight(confirmation_target) as f32,
-		);
+		let fee_rate = self.fee_estimator.estimate_fee_rate(confirmation_target);
 
 		let locked_wallet = self.inner.lock().unwrap();
 		let mut tx_builder = locked_wallet.build_tx();
@@ -240,10 +238,8 @@ where
 	pub(crate) fn send_to_address(
 		&self, address: &bitcoin::Address, amount_msat_or_drain: Option<u64>,
 	) -> Result<Txid, Error> {
-		let confirmation_target = ConfirmationTarget::OutputSpendingFee;
-		let fee_rate = FeeRate::from_sat_per_kwu(
-			self.fee_estimator.get_est_sat_per_1000_weight(confirmation_target) as f32,
-		);
+		let confirmation_target = ConfirmationTarget::OnchainPayment;
+		let fee_rate = self.fee_estimator.estimate_fee_rate(confirmation_target);
 
 		let tx = {
 			let locked_wallet = self.inner.lock().unwrap();
@@ -485,7 +481,7 @@ where
 
 /// Similar to [`KeysManager`], but overrides the destination and shutdown scripts so they are
 /// directly spendable by the BDK wallet.
-pub struct WalletKeysManager<D, B: Deref, E: Deref, L: Deref>
+pub(crate) struct WalletKeysManager<D, B: Deref, E: Deref, L: Deref>
 where
 	D: BatchDatabase,
 	B::Target: BroadcasterInterface,