diff --git a/bindings/ldk_node.udl b/bindings/ldk_node.udl index 57eeacdf3..1ded616d9 100644 --- a/bindings/ldk_node.udl +++ b/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 { diff --git a/src/chain/mod.rs b/src/chain/mod.rs index a7906fc0f..3c5ffc27b 100644 --- a/src/chain/mod.rs +++ b/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, diff --git a/src/error.rs b/src/error.rs index 8caaaabdd..d1fd848b1 100644 --- a/src/error.rs +++ b/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 for Error { } impl From 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 for Error { } } +impl From for Error { + fn from(_: BdkChainCalculateFeeError) -> Self { + Self::WalletOperationFailed + } +} + impl From for Error { fn from(_e: lightning_transaction_sync::TxSyncError) -> Self { Self::TxSyncFailed diff --git a/src/payment/onchain.rs b/src/payment/onchain.rs index b43765a97..d46eba2b5 100644 --- a/src/payment/onchain.rs +++ b/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 { + /// 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); + OnchainSendAmount::AllRetainingReserve { cur_anchor_reserve_sats } + } else { + OnchainSendAmount::AllDrainingReserve + }; + + self.wallet.send_to_address(address, send_amount) } } diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index d237e8c6a..a4d4b066e 100644 --- a/src/wallet/mod.rs +++ b/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, + &self, address: &bitcoin::Address, send_amount: OnchainSendAmount, ) -> Result { 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) diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index b198b6caa..31990440e 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -277,44 +277,91 @@ fn onchain_spend_receive() { let addr_a = node_a.onchain_payment().new_address().unwrap(); let addr_b = node_b.onchain_payment().new_address().unwrap(); + let premine_amount_sat = 1_100_000; premine_and_distribute_funds( &bitcoind.client, &electrsd.client, - vec![addr_b.clone()], - Amount::from_sat(100000), + vec![addr_a.clone(), addr_b.clone()], + Amount::from_sat(premine_amount_sat), ); node_a.sync_wallets().unwrap(); node_b.sync_wallets().unwrap(); - assert_eq!(node_b.list_balances().spendable_onchain_balance_sats, 100000); + assert_eq!(node_a.list_balances().spendable_onchain_balance_sats, premine_amount_sat); + assert_eq!(node_b.list_balances().spendable_onchain_balance_sats, premine_amount_sat); + + let channel_amount_sat = 1_000_000; + let reserve_amount_sat = 25_000; + open_channel(&node_b, &node_a, channel_amount_sat, true, &electrsd); + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6); + + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + let onchain_fee_buffer_sat = 1000; + let expected_node_a_balance = premine_amount_sat - reserve_amount_sat; + let expected_node_b_balance_lower = + premine_amount_sat - channel_amount_sat - reserve_amount_sat - onchain_fee_buffer_sat; + let expected_node_b_balance_upper = + premine_amount_sat - channel_amount_sat - reserve_amount_sat; + assert_eq!(node_a.list_balances().spendable_onchain_balance_sats, expected_node_a_balance); + assert!(node_b.list_balances().spendable_onchain_balance_sats > expected_node_b_balance_lower); + assert!(node_b.list_balances().spendable_onchain_balance_sats < expected_node_b_balance_upper); assert_eq!( Err(NodeError::InsufficientFunds), - node_a.onchain_payment().send_to_address(&addr_b, 1000) + node_a.onchain_payment().send_to_address(&addr_b, expected_node_a_balance + 1) ); - let txid = node_b.onchain_payment().send_to_address(&addr_a, 1000).unwrap(); + let amount_to_send_sats = 1000; + let txid = node_b.onchain_payment().send_to_address(&addr_a, amount_to_send_sats).unwrap(); + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6); + wait_for_tx(&electrsd.client, txid); + + node_a.sync_wallets().unwrap(); + node_b.sync_wallets().unwrap(); + + let expected_node_a_balance = expected_node_a_balance + amount_to_send_sats; + let expected_node_b_balance_lower = expected_node_b_balance_lower - amount_to_send_sats; + let expected_node_b_balance_upper = expected_node_b_balance_upper - amount_to_send_sats; + assert_eq!(node_a.list_balances().spendable_onchain_balance_sats, expected_node_a_balance); + assert!(node_b.list_balances().spendable_onchain_balance_sats > expected_node_b_balance_lower); + assert!(node_b.list_balances().spendable_onchain_balance_sats < expected_node_b_balance_upper); + + let addr_b = node_b.onchain_payment().new_address().unwrap(); + let txid = node_a.onchain_payment().send_all_to_address(&addr_b, true).unwrap(); generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6); wait_for_tx(&electrsd.client, txid); node_a.sync_wallets().unwrap(); node_b.sync_wallets().unwrap(); - assert_eq!(node_a.list_balances().spendable_onchain_balance_sats, 1000); - assert!(node_b.list_balances().spendable_onchain_balance_sats > 98000); - assert!(node_b.list_balances().spendable_onchain_balance_sats < 100000); + let expected_node_b_balance_lower = expected_node_b_balance_lower + expected_node_a_balance; + let expected_node_b_balance_upper = expected_node_b_balance_upper + expected_node_a_balance; + let expected_node_a_balance = 0; + assert_eq!(node_a.list_balances().spendable_onchain_balance_sats, expected_node_a_balance); + assert_eq!(node_a.list_balances().total_onchain_balance_sats, reserve_amount_sat); + assert!(node_b.list_balances().spendable_onchain_balance_sats > expected_node_b_balance_lower); + assert!(node_b.list_balances().spendable_onchain_balance_sats < expected_node_b_balance_upper); 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); node_a.sync_wallets().unwrap(); node_b.sync_wallets().unwrap(); - assert_eq!(node_a.list_balances().total_onchain_balance_sats, 0); - assert!(node_b.list_balances().spendable_onchain_balance_sats > 99000); - assert!(node_b.list_balances().spendable_onchain_balance_sats < 100000); + let expected_node_b_balance_lower = expected_node_b_balance_lower + reserve_amount_sat; + let expected_node_b_balance_upper = expected_node_b_balance_upper + reserve_amount_sat; + let expected_node_a_balance = 0; + + assert_eq!(node_a.list_balances().spendable_onchain_balance_sats, expected_node_a_balance); + assert_eq!(node_a.list_balances().total_onchain_balance_sats, expected_node_a_balance); + assert!(node_b.list_balances().spendable_onchain_balance_sats > expected_node_b_balance_lower); + assert!(node_b.list_balances().spendable_onchain_balance_sats < expected_node_b_balance_upper); } #[test]