diff --git a/src/error.rs b/src/error.rs index 7506b013b..2d463df44 100644 --- a/src/error.rs +++ b/src/error.rs @@ -178,6 +178,7 @@ impl From for Error { fn from(e: bdk::Error) -> Self { match e { bdk::Error::Signer(_) => Self::OnchainTxSigningFailed, + bdk::Error::InsufficientFunds { .. } => Self::InsufficientFunds, _ => Self::WalletOperationFailed, } } diff --git a/src/payment/onchain.rs b/src/payment/onchain.rs index 5c1365de3..ca09c85b8 100644 --- a/src/payment/onchain.rs +++ b/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 { + /// 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 { 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) } } diff --git a/src/wallet.rs b/src/wallet.rs index 0da3f6db8..9e0d0a633 100644 --- a/src/wallet.rs +++ b/src/wallet.rs @@ -43,6 +43,12 @@ enum WalletSyncStatus { InProgress { subscribers: tokio::sync::broadcast::Sender> }, } +pub(crate) enum OnchainSendType { + SendRetainingReserve { amount_sats: u64, cur_anchor_reserve_sats: u64 }, + SendAllRetainingReserve { cur_anchor_reserve_sats: u64 }, + SendAllDrainingReserve, +} + pub struct Wallet 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, + &self, address: &bitcoin::Address, send_amount: OnchainSendType, ) -> Result { let confirmation_target = ConfirmationTarget::OutputSpendingFee; let fee_rate = FeeRate::from_sat_per_kwu( @@ -249,23 +251,69 @@ 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); @@ -273,6 +321,38 @@ where }, }; + // 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) diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 5a918762a..e29676e8b 100644 --- a/tests/integration_tests_rust.rs +++ b/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);