Allow honoring reserve in send_all_to_address

Previously, `OnchainPayment::send_all_to_address` could only be used to
fully drain the onchain wallet, i.e., would not retain any reserves.

Here, we try to introduce a `retain_reserves` bool that allows users to
send all funds while honoring the configured on-chain reserves. While
we're at it, we move the reserve checks for `send_to_address` also to
the internal wallet's method, which makes the checks more accurate as
they now are checked against the final transaction value, including
transaction fees.

Author: tnull

src/error.rs

@@ -178,6 +178,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/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/wallet.rs

@@ -43,6 +43,12 @@ enum WalletSyncStatus {
 	InProgress { subscribers: tokio::sync::broadcast::Sender<Result<(), Error>> },
 }
 
+pub(crate) enum OnchainSendType {
+	SendRetainingReserve { amount_sats: u64, cur_anchor_reserve_sats: u64 },
+	SendAllRetainingReserve { cur_anchor_reserve_sats: u64 },
+	SendAllDrainingReserve,
+}
+
 pub struct Wallet<D, B: Deref, E: Deref, L: Deref>
 where
 	D: BatchDatabase,
@@ -233,12 +239,8 @@ 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_msat_or_drain: Option<u64>,
+		&self, address: &bitcoin::Address, send_amount: OnchainSendType,
 	) -> Result<Txid, Error> {
 		let confirmation_target = ConfirmationTarget::OutputSpendingFee;
 		let fee_rate = FeeRate::from_sat_per_kwu(
@@ -249,30 +251,108 @@ where
 			let locked_wallet = self.inner.lock().unwrap();
 			let mut tx_builder = locked_wallet.build_tx();
 
-			if let Some(amount_sats) = amount_msat_or_drain {
-				tx_builder
-					.add_recipient(address.script_pubkey(), amount_sats)
-					.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.
+			match send_amount {
+				OnchainSendType::SendRetainingReserve { amount_sats, .. } => {
+					tx_builder
+						.add_recipient(address.script_pubkey(), amount_sats)
+						.fee_rate(fee_rate)
+						.enable_rbf();
+				},
+				OnchainSendType::SendAllRetainingReserve { cur_anchor_reserve_sats } => {
+					let spendable_amount_sats =
+						self.get_spendable_amount_sats(cur_anchor_reserve_sats).unwrap_or(0);
+					// TODO: can we make this closer resemble the actual transaction?
+					// As draining the wallet always will only add one output, this method likely
+					// under-estimates the fee rate a bit.
+					let mut tmp_tx_builder = locked_wallet.build_tx();
+					tmp_tx_builder
+						.drain_wallet()
+						.drain_to(address.script_pubkey())
+						.fee_rate(fee_rate)
+						.enable_rbf();
+					let tmp_tx_details = match tmp_tx_builder.finish() {
+						Ok((_, tmp_tx_details)) => tmp_tx_details,
+						Err(err) => {
+							log_error!(
+								self.logger,
+								"Failed to create temporary transaction: {}",
+								err
+							);
+							return Err(err.into());
+						},
+					};
+
+					let estimated_tx_fee_sats = tmp_tx_details.fee.unwrap_or(0);
+					let estimated_spendable_amount_sats =
+						spendable_amount_sats.saturating_sub(estimated_tx_fee_sats);
+
+					if estimated_spendable_amount_sats == 0 {
+						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_sats,
+						);
+						return Err(Error::InsufficientFunds);
+					}
+
+					tx_builder
+						.add_recipient(address.script_pubkey(), estimated_spendable_amount_sats)
+						.fee_absolute(estimated_tx_fee_sats)
+						.enable_rbf();
+				},
+				OnchainSendType::SendAllDrainingReserve => {
+					tx_builder
+						.drain_wallet()
+						.drain_to(address.script_pubkey())
+						.fee_rate(fee_rate)
+						.enable_rbf();
+				},
 			}
 
-			let mut psbt = match tx_builder.finish() {
-				Ok((psbt, _)) => {
+			let (mut psbt, tx_details) = match tx_builder.finish() {
+				Ok((psbt, tx_details)) => {
 					log_trace!(self.logger, "Created PSBT: {:?}", psbt);
-					psbt
+					(psbt, tx_details)
 				},
 				Err(err) => {
 					log_error!(self.logger, "Failed to create transaction: {}", err);
 					return Err(err.into());
 				},
 			};
 
+			// Check the reserve requirements (again) and return an error if they aren't met.
+			match send_amount {
+				OnchainSendType::SendRetainingReserve { amount_sats, cur_anchor_reserve_sats } => {
+					let spendable_amount_sats =
+						self.get_spendable_amount_sats(cur_anchor_reserve_sats).unwrap_or(0);
+					let tx_fee_sats = tx_details.fee.unwrap_or(0);
+					if spendable_amount_sats < amount_sats + 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);
+					}
+				},
+				OnchainSendType::SendAllRetainingReserve { cur_anchor_reserve_sats } => {
+					let spendable_amount_sats =
+						self.get_spendable_amount_sats(cur_anchor_reserve_sats).unwrap_or(0);
+					let drain_amount_sats = tx_details.sent - tx_details.received;
+					if spendable_amount_sats < drain_amount_sats {
+						log_error!(self.logger,
+							"Unable to send payment due to insufficient funds. Available: {}sats, Required: {}sats",
+							spendable_amount_sats,
+							drain_amount_sats,
+						);
+						return Err(Error::InsufficientFunds);
+					}
+				},
+				_ => {},
+			}
+
 			match locked_wallet.sign(&mut psbt, SignOptions::default()) {
 				Ok(finalized) => {
 					if !finalized {
@@ -291,21 +371,33 @@ where
 
 		let txid = tx.txid();
 
-		if let Some(amount_sats) = amount_msat_or_drain {
-			log_info!(
-				self.logger,
-				"Created new transaction {} sending {}sats on-chain to address {}",
-				txid,
-				amount_sats,
-				address
-			);
-		} else {
-			log_info!(
-				self.logger,
-				"Created new transaction {} sending all available on-chain funds to address {}",
-				txid,
-				address
-			);
+		match send_amount {
+			OnchainSendType::SendRetainingReserve { amount_sats, .. } => {
+				log_info!(
+					self.logger,
+					"Created new transaction {} sending {}sats on-chain to address {}",
+					txid,
+					amount_sats,
+					address
+				);
+			},
+			OnchainSendType::SendAllRetainingReserve { cur_anchor_reserve_sats } => {
+				log_info!(
+					self.logger,
+					"Created new transaction {} sending available on-chain funds retaining a reserve of {}sats to address {}",
+					txid,
+					address,
+					cur_anchor_reserve_sats,
+				);
+			},
+			OnchainSendType::SendAllDrainingReserve => {
+				log_info!(
+					self.logger,
+					"Created new transaction {} sending all available on-chain funds to address {}",
+					txid,
+					address
+				);
+			},
 		}
 
 		Ok(txid)

tests/integration_tests_rust.rs

@@ -279,7 +279,7 @@ fn onchain_spend_receive() {
 	assert!(node_b.list_balances().spendable_onchain_balance_sats < 100000);
 
 	let addr_b = node_b.onchain_payment().new_address().unwrap();
-	let txid = node_a.onchain_payment().send_all_to_address(&addr_b).unwrap();
+	let txid = node_a.onchain_payment().send_all_to_address(&addr_b, false).unwrap();
 	generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6);
 	wait_for_tx(&electrsd.client, txid);