Skip to content

Commit

Permalink
Allow honoring reserve in send_all_to_address
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
tnull committed Aug 16, 2024
1 parent 3b645b3 commit b62484a
Show file tree
Hide file tree
Showing 5 changed files with 153 additions and 53 deletions.
2 changes: 1 addition & 1 deletion bindings/ldk_node.udl
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
1 change: 1 addition & 0 deletions src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
}
}
Expand Down
41 changes: 24 additions & 17 deletions src/payment/onchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand Down Expand Up @@ -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)
}
}
160 changes: 126 additions & 34 deletions src/wallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down Expand Up @@ -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(
Expand All @@ -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 {
Expand All @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion tests/integration_tests_rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);

Expand Down

0 comments on commit b62484a

Please sign in to comment.