Allow honoring reserve in send_all_to_address

bindings/ldk_node.udl

@@ -146,7 +146,7 @@ interface OnchainPayment {
 	[Throws=NodeError]
 	Txid send_to_address([ByRef]Address address, u64 amount_sats);
 	[Throws=NodeError]
-	Txid send_all_to_address([ByRef]Address address);
+	Txid send_all_to_address([ByRef]Address address, boolean retain_reserve);
 };
 
 interface UnifiedQrPayment {

src/error.rs

@@ -181,6 +181,7 @@ impl From<bdk::Error> for Error {
 	fn from(e: bdk::Error) -> Self {
 		match e {
 			bdk::Error::Signer(_) => Self::OnchainTxSigningFailed,
+			bdk::Error::InsufficientFunds { .. } => Self::InsufficientFunds,
 			_ => Self::WalletOperationFailed,
 		}
 	}

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/payment/onchain.rs

@@ -2,8 +2,9 @@
 
 use crate::config::Config;
 use crate::error::Error;
-use crate::logger::{log_error, log_info, FilesystemLogger, Logger};
+use crate::logger::{log_info, FilesystemLogger, Logger};
 use crate::types::{ChannelManager, Wallet};
+use crate::wallet::OnchainSendType;
 
 use bitcoin::{Address, Txid};
 
@@ -53,33 +54,39 @@ impl OnchainPayment {
 
 		let cur_anchor_reserve_sats =
 			crate::total_anchor_channels_reserve_sats(&self.channel_manager, &self.config);
-		let spendable_amount_sats =
-			self.wallet.get_spendable_amount_sats(cur_anchor_reserve_sats).unwrap_or(0);
-
-		if spendable_amount_sats < amount_sats {
-			log_error!(self.logger,
-				"Unable to send payment due to insufficient funds. Available: {}sats, Required: {}sats",
-				spendable_amount_sats, amount_sats
-			);
-			return Err(Error::InsufficientFunds);
-		}
-		self.wallet.send_to_address(address, Some(amount_sats))
+		let send_amount =
+			OnchainSendType::SendRetainingReserve { amount_sats, cur_anchor_reserve_sats };
+		self.wallet.send_to_address(address, send_amount)
 	}
 
-	/// Send an on-chain payment to the given address, draining all the available funds.
+	/// Send an on-chain payment to the given address, draining the available funds.
 	///
 	/// This is useful if you have closed all channels and want to migrate funds to another
 	/// on-chain wallet.
 	///
-	/// Please note that this will **not** retain any on-chain reserves, which might be potentially
+	/// Please note that if `retain_reserves` is set to `false` this will **not** retain any on-chain reserves, which might be potentially
 	/// dangerous if you have open Anchor channels for which you can't trust the counterparty to
-	/// spend the Anchor output after channel closure.
-	pub fn send_all_to_address(&self, address: &bitcoin::Address) -> Result<Txid, Error> {
+	/// spend the Anchor output after channel closure. If `retain_reserves` is set to `true`, this
+	/// will try to send all spendable onchain funds, i.e.,
+	/// [`BalanceDetails::spendable_onchain_balance_sats`].
+	///
+	/// [`BalanceDetails::spendable_onchain_balance_sats`]: crate::balance::BalanceDetails::spendable_onchain_balance_sats
+	pub fn send_all_to_address(
+		&self, address: &bitcoin::Address, retain_reserves: bool,
+	) -> Result<Txid, Error> {
 		let rt_lock = self.runtime.read().unwrap();
 		if rt_lock.is_none() {
 			return Err(Error::NotRunning);
 		}
 
-		self.wallet.send_to_address(address, None)
+		let send_amount = if retain_reserves {
+			let cur_anchor_reserve_sats =
+				crate::total_anchor_channels_reserve_sats(&self.channel_manager, &self.config);
+			OnchainSendType::SendAllRetainingReserve { cur_anchor_reserve_sats }
+		} else {
+			OnchainSendType::SendAllDrainingReserve
+		};
+
+		self.wallet.send_to_address(address, send_amount)
 	}
 }

src/tx_broadcaster.rs

@@ -56,11 +56,16 @@ where
 						Err(e) => match e {
 							esplora_client::Error::Reqwest(err) => {
 								if err.status() == StatusCode::from_u16(400).ok() {
-									// Ignore 400, as this just means bitcoind already knows the
+									// Log 400 at lesser level, as this often just means bitcoind already knows the
 									// transaction.
 									// FIXME: We can further differentiate here based on the error
 									// message which will be available with rust-esplora-client 0.7 and
 									// later.
+									log_trace!(
+										self.logger,
+										"Failed to broadcast due to HTTP connection error: {}",
+										err
+									);
 								} else {
 									log_error!(
 										self.logger,