Merge pull request #345 from tnull/2024-08-regard-reserve-spending-all

Allow honoring reserve in `send_all_to_address`

Author: tnull

bindings/ldk_node.udl

@@ -161,7 +161,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/chain/mod.rs

@@ -993,11 +993,16 @@ impl ChainSource {
 								Err(e) => match e {
 									esplora_client::Error::Reqwest(err) => {
 										if err.status() == reqwest::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!(
+												logger,
+												"Failed to broadcast due to HTTP connection error: {}",
+												err
+											);
 										} else {
 											log_error!(
 												logger,

src/error.rs

@@ -7,6 +7,7 @@
 
 use bdk_chain::bitcoin::psbt::ExtractTxError as BdkExtractTxError;
 use bdk_chain::local_chain::CannotConnectError as BdkChainConnectionError;
+use bdk_chain::tx_graph::CalculateFeeError as BdkChainCalculateFeeError;
 use bdk_wallet::error::CreateTxError as BdkCreateTxError;
 use bdk_wallet::signer::SignerError as BdkSignerError;
 
@@ -196,8 +197,11 @@ impl From<BdkSignerError> for Error {
 }
 
 impl From<BdkCreateTxError> for Error {
-	fn from(_: BdkCreateTxError) -> Self {
-		Self::OnchainTxCreationFailed
+	fn from(e: BdkCreateTxError) -> Self {
+		match e {
+			BdkCreateTxError::CoinSelection(_) => Self::InsufficientFunds,
+			_ => Self::OnchainTxCreationFailed,
+		}
 	}
 }
 
@@ -213,6 +217,12 @@ impl From<BdkChainConnectionError> for Error {
 	}
 }
 
+impl From<BdkChainCalculateFeeError> for Error {
+	fn from(_: BdkChainCalculateFeeError) -> Self {
+		Self::WalletOperationFailed
+	}
+}
+
 impl From<lightning_transaction_sync::TxSyncError> for Error {
 	fn from(_e: lightning_transaction_sync::TxSyncError) -> Self {
 		Self::TxSyncFailed

src/payment/onchain.rs

@@ -9,10 +9,11 @@
 
 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::OnchainSendAmount;
 
-use bitcoin::{Address, Amount, Txid};
+use bitcoin::{Address, Txid};
 
 use std::sync::{Arc, RwLock};
 
@@ -60,35 +61,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);
-		}
-
-		let amount = Amount::from_sat(amount_sats);
-		self.wallet.send_to_address(address, Some(amount))
+		let send_amount =
+			OnchainSendAmount::ExactRetainingReserve { 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);
+			OnchainSendAmount::AllRetainingReserve { cur_anchor_reserve_sats }
+		} else {
+			OnchainSendAmount::AllDrainingReserve
+		};
+
+		self.wallet.send_to_address(address, send_amount)
 	}
 }

src/wallet/mod.rs

@@ -28,7 +28,7 @@ use lightning_invoice::RawBolt11Invoice;
 
 use bdk_chain::spk_client::{FullScanRequest, SyncRequest};
 use bdk_chain::ChainPosition;
-use bdk_wallet::{KeychainKind, PersistedWallet, SignOptions, Update};
+use bdk_wallet::{Balance, KeychainKind, PersistedWallet, SignOptions, Update};
 
 use bitcoin::blockdata::constants::WITNESS_SCALE_FACTOR;
 use bitcoin::blockdata::locktime::absolute::LockTime;
@@ -45,6 +45,12 @@ use bitcoin::{
 use std::ops::Deref;
 use std::sync::{Arc, Mutex};
 
+pub(crate) enum OnchainSendAmount {
+	ExactRetainingReserve { amount_sats: u64, cur_anchor_reserve_sats: u64 },
+	AllRetainingReserve { cur_anchor_reserve_sats: u64 },
+	AllDrainingReserve,
+}
+
 pub(crate) mod persist;
 pub(crate) mod ser;
 
@@ -215,6 +221,12 @@ where
 			);
 		}
 
+		self.get_balances_inner(balance, total_anchor_channels_reserve_sats)
+	}
+
+	fn get_balances_inner(
+		&self, balance: Balance, total_anchor_channels_reserve_sats: u64,
+	) -> Result<(u64, u64), Error> {
 		let (total, spendable) = (
 			balance.total().to_sat(),
 			balance.trusted_spendable().to_sat().saturating_sub(total_anchor_channels_reserve_sats),
@@ -229,32 +241,95 @@ where
 		self.get_balances(total_anchor_channels_reserve_sats).map(|(_, s)| s)
 	}
 
-	/// Send funds to the given address.
-	///
-	/// If `amount_msat_or_drain` is `None` the wallet will be drained, i.e., all available funds will be
-	/// spent.
 	pub(crate) fn send_to_address(
-		&self, address: &bitcoin::Address, amount_or_drain: Option<Amount>,
+		&self, address: &bitcoin::Address, send_amount: OnchainSendAmount,
 	) -> Result<Txid, Error> {
 		let confirmation_target = ConfirmationTarget::OnchainPayment;
 		let fee_rate = self.fee_estimator.estimate_fee_rate(confirmation_target);
 
 		let tx = {
 			let mut locked_wallet = self.inner.lock().unwrap();
-			let mut tx_builder = locked_wallet.build_tx();
-
-			if let Some(amount) = amount_or_drain {
-				tx_builder
-					.add_recipient(address.script_pubkey(), amount)
-					.fee_rate(fee_rate)
-					.enable_rbf();
-			} else {
-				tx_builder
-					.drain_wallet()
-					.drain_to(address.script_pubkey())
-					.fee_rate(fee_rate)
-					.enable_rbf();
-			}
+
+			// Prepare the tx_builder. We properly check the reserve requirements (again) further down.
+			let tx_builder = match send_amount {
+				OnchainSendAmount::ExactRetainingReserve { amount_sats, .. } => {
+					let mut tx_builder = locked_wallet.build_tx();
+					let amount = Amount::from_sat(amount_sats);
+					tx_builder
+						.add_recipient(address.script_pubkey(), amount)
+						.fee_rate(fee_rate)
+						.enable_rbf();
+					tx_builder
+				},
+				OnchainSendAmount::AllRetainingReserve { cur_anchor_reserve_sats } => {
+					let change_address_info = locked_wallet.peek_address(KeychainKind::Internal, 0);
+					let balance = locked_wallet.balance();
+					let spendable_amount_sats = self
+						.get_balances_inner(balance, cur_anchor_reserve_sats)
+						.map(|(_, s)| s)
+						.unwrap_or(0);
+					let tmp_tx = {
+						let mut tmp_tx_builder = locked_wallet.build_tx();
+						tmp_tx_builder
+							.drain_wallet()
+							.drain_to(address.script_pubkey())
+							.add_recipient(
+								change_address_info.address.script_pubkey(),
+								Amount::from_sat(cur_anchor_reserve_sats),
+							)
+							.fee_rate(fee_rate)
+							.enable_rbf();
+						match tmp_tx_builder.finish() {
+							Ok(psbt) => psbt.unsigned_tx,
+							Err(err) => {
+								log_error!(
+									self.logger,
+									"Failed to create temporary transaction: {}",
+									err
+								);
+								return Err(err.into());
+							},
+						}
+					};
+
+					let estimated_tx_fee = locked_wallet.calculate_fee(&tmp_tx).map_err(|e| {
+						log_error!(
+							self.logger,
+							"Failed to calculate fee of temporary transaction: {}",
+							e
+						);
+						e
+					})?;
+					let estimated_spendable_amount = Amount::from_sat(
+						spendable_amount_sats.saturating_sub(estimated_tx_fee.to_sat()),
+					);
+
+					if estimated_spendable_amount == Amount::ZERO {
+						log_error!(self.logger,
+							"Unable to send payment without infringing on Anchor reserves. Available: {}sats, estimated fee required: {}sats.",
+							spendable_amount_sats,
+							estimated_tx_fee,
+						);
+						return Err(Error::InsufficientFunds);
+					}
+
+					let mut tx_builder = locked_wallet.build_tx();
+					tx_builder
+						.add_recipient(address.script_pubkey(), estimated_spendable_amount)
+						.fee_absolute(estimated_tx_fee)
+						.enable_rbf();
+					tx_builder
+				},
+				OnchainSendAmount::AllDrainingReserve => {
+					let mut tx_builder = locked_wallet.build_tx();
+					tx_builder
+						.drain_wallet()
+						.drain_to(address.script_pubkey())
+						.fee_rate(fee_rate)
+						.enable_rbf();
+					tx_builder
+				},
+			};
 
 			let mut psbt = match tx_builder.finish() {
 				Ok(psbt) => {
@@ -267,6 +342,58 @@ where
 				},
 			};
 
+			// Check the reserve requirements (again) and return an error if they aren't met.
+			match send_amount {
+				OnchainSendAmount::ExactRetainingReserve {
+					amount_sats,
+					cur_anchor_reserve_sats,
+				} => {
+					let balance = locked_wallet.balance();
+					let spendable_amount_sats = self
+						.get_balances_inner(balance, cur_anchor_reserve_sats)
+						.map(|(_, s)| s)
+						.unwrap_or(0);
+					let tx_fee_sats = locked_wallet
+						.calculate_fee(&psbt.unsigned_tx)
+						.map_err(|e| {
+							log_error!(
+								self.logger,
+								"Failed to calculate fee of candidate transaction: {}",
+								e
+							);
+							e
+						})?
+						.to_sat();
+					if spendable_amount_sats < amount_sats.saturating_add(tx_fee_sats) {
+						log_error!(self.logger,
+							"Unable to send payment due to insufficient funds. Available: {}sats, Required: {}sats + {}sats fee",
+							spendable_amount_sats,
+							amount_sats,
+							tx_fee_sats,
+						);
+						return Err(Error::InsufficientFunds);
+					}
+				},
+				OnchainSendAmount::AllRetainingReserve { cur_anchor_reserve_sats } => {
+					let balance = locked_wallet.balance();
+					let spendable_amount_sats = self
+						.get_balances_inner(balance, cur_anchor_reserve_sats)
+						.map(|(_, s)| s)
+						.unwrap_or(0);
+					let (sent, received) = locked_wallet.sent_and_received(&psbt.unsigned_tx);
+					let drain_amount = sent - received;
+					if spendable_amount_sats < drain_amount.to_sat() {
+						log_error!(self.logger,
+							"Unable to send payment due to insufficient funds. Available: {}sats, Required: {}",
+							spendable_amount_sats,
+							drain_amount,
+						);
+						return Err(Error::InsufficientFunds);
+					}
+				},
+				_ => {},
+			}
+
 			match locked_wallet.sign(&mut psbt, SignOptions::default()) {
 				Ok(finalized) => {
 					if !finalized {
@@ -295,21 +422,33 @@ where
 
 		let txid = tx.compute_txid();
 
-		if let Some(amount) = amount_or_drain {
-			log_info!(
-				self.logger,
-				"Created new transaction {} sending {}sats on-chain to address {}",
-				txid,
-				amount.to_sat(),
-				address
-			);
-		} else {
-			log_info!(
-				self.logger,
-				"Created new transaction {} sending all available on-chain funds to address {}",
-				txid,
-				address
-			);
+		match send_amount {
+			OnchainSendAmount::ExactRetainingReserve { amount_sats, .. } => {
+				log_info!(
+					self.logger,
+					"Created new transaction {} sending {}sats on-chain to address {}",
+					txid,
+					amount_sats,
+					address
+				);
+			},
+			OnchainSendAmount::AllRetainingReserve { cur_anchor_reserve_sats } => {
+				log_info!(
+					self.logger,
+					"Created new transaction {} sending available on-chain funds retaining a reserve of {}sats to address {}",
+					txid,
+					cur_anchor_reserve_sats,
+					address,
+				);
+			},
+			OnchainSendAmount::AllDrainingReserve => {
+				log_info!(
+					self.logger,
+					"Created new transaction {} sending all available on-chain funds to address {}",
+					txid,
+					address
+				);
+			},
 		}
 
 		Ok(txid)