Skip to content

Add basic fee estimation to on-chain wallet #585

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 3 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
59 changes: 59 additions & 0 deletions src/payment/onchain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -122,4 +122,63 @@ impl OnchainPayment {
let fee_rate_opt = maybe_map_fee_rate_opt!(fee_rate);
self.wallet.send_to_address(address, send_amount, fee_rate_opt)
}

/// Estimates the fee for sending an on-chain payment to the given address.
///
/// This will respect any on-chain reserve we need to keep, i.e., won't allow to cut into
/// [`BalanceDetails::total_anchor_channels_reserve_sats`].
///
/// If `fee_rate` is set it will be used for estimating the resulting transaction. Otherwise we'll retrieve
/// a reasonable estimate from the configured chain source.
///
/// [`BalanceDetails::total_anchor_channels_reserve_sats`]: crate::BalanceDetails::total_anchor_channels_reserve_sats
pub fn estimate_send_to_address(
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, as discussed elsewhere, this is kind of an odd API as it is inherently TOCTOU race-y (based on fee rate updates, but also wallet state changes), and we don't offer any API that would allow to set exactly that net. fee returned here. I think generally the 'staging' approach as described over at #578 would be preferable, but that of course also interacts with UTXO locking, etc.

&self, address: &Address, amount_sats: u64, fee_rate: Option<FeeRate>,
) -> Result<bitcoin::Amount, Error> {
let rt_lock = self.runtime.read().unwrap();
if rt_lock.is_none() {
return Err(Error::NotRunning);
}

let cur_anchor_reserve_sats =
crate::total_anchor_channels_reserve_sats(&self.channel_manager, &self.config);
let send_amount =
OnchainSendAmount::ExactRetainingReserve { amount_sats, cur_anchor_reserve_sats };
let fee_rate_opt = maybe_map_fee_rate_opt!(fee_rate);
self.wallet.estimate_fee(address, send_amount, fee_rate_opt)
}

/// Estimates the fee for sending 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 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. If `retain_reserves` is set to `true`, this
/// will try to send all spendable onchain funds, i.e.,
/// [`BalanceDetails::spendable_onchain_balance_sats`].
///
/// If `fee_rate` is set it will be used on the resulting transaction. Otherwise a reasonable
/// we'll retrieve an estimate from the configured chain source.
///
/// [`BalanceDetails::spendable_onchain_balance_sats`]: crate::balance::BalanceDetails::spendable_onchain_balance_sats
pub fn estimate_send_all_to_address(
&self, address: &Address, retain_reserves: bool, fee_rate: Option<FeeRate>,
) -> Result<bitcoin::Amount, Error> {
let rt_lock = self.runtime.read().unwrap();
if rt_lock.is_none() {
return Err(Error::NotRunning);
}

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
};
let fee_rate_opt = maybe_map_fee_rate_opt!(fee_rate);
self.wallet.estimate_fee(address, send_amount, fee_rate_opt)
}
}
159 changes: 116 additions & 43 deletions src/wallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@ use bitcoin::{

use std::ops::Deref;
use std::str::FromStr;
use std::sync::{Arc, Mutex};
use std::sync::{Arc, Mutex, MutexGuard};

#[derive(Debug, Copy, Clone)]
pub(crate) enum OnchainSendAmount {
ExactRetainingReserve { amount_sats: u64, cur_anchor_reserve_sats: u64 },
AllRetainingReserve { cur_anchor_reserve_sats: u64 },
Expand Down Expand Up @@ -352,6 +353,114 @@ where
.map_err(|_| Error::InvalidAddress)
}

pub(crate) fn estimate_fee(
&self, address: &Address, send_amount: OnchainSendAmount, fee_rate: Option<FeeRate>,
) -> Result<Amount, Error> {
let mut locked_wallet = self.inner.lock().unwrap();

// Use the set fee_rate or default to fee estimation.
let confirmation_target = ConfirmationTarget::OnchainPayment;
let fee_rate =
fee_rate.unwrap_or_else(|| self.fee_estimator.estimate_fee_rate(confirmation_target));

self.estimate_fee_internal(&mut locked_wallet, address, send_amount, fee_rate).map_err(
|e| {
log_error!(self.logger, "Failed to estimate fee: {e}");
e
},
)
}

pub(crate) fn estimate_fee_internal(
&self, locked_wallet: &mut MutexGuard<PersistedWallet<KVStoreWalletPersister>>,
address: &Address, send_amount: OnchainSendAmount, fee_rate: FeeRate,
) -> Result<Amount, Error> {
const DUST_LIMIT_SATS: u64 = 546;
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);

let psbt = match tx_builder.finish() {
Ok(psbt) => psbt,
Err(err) => {
log_error!(self.logger, "Failed to create temporary transaction: {}", err);
return Err(err.into());
},
};

// 'cancel' the transaction to free up any used change addresses
locked_wallet.cancel_tx(&psbt.unsigned_tx);
psbt.fee().map_err(|_| Error::WalletOperationFailed)
},
OnchainSendAmount::AllRetainingReserve { cur_anchor_reserve_sats }
if cur_anchor_reserve_sats > DUST_LIMIT_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 mut tx_builder = locked_wallet.build_tx();
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);

let psbt = match tx_builder.finish() {
Ok(psbt) => psbt,
Err(err) => {
log_error!(self.logger, "Failed to create temporary transaction: {}", err);
return Err(err.into());
},
};

// 'cancel' the transaction to free up any used change addresses
locked_wallet.cancel_tx(&psbt.unsigned_tx);

let estimated_tx_fee = psbt.fee().map_err(|_| Error::WalletOperationFailed)?;

// enforce the reserve requirements to make sure we can actually afford the tx + fee
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);
}

Ok(estimated_tx_fee)
},
OnchainSendAmount::AllDrainingReserve
| OnchainSendAmount::AllRetainingReserve { cur_anchor_reserve_sats: _ } => {
let mut tx_builder = locked_wallet.build_tx();
tx_builder.drain_wallet().drain_to(address.script_pubkey()).fee_rate(fee_rate);
let psbt = match tx_builder.finish() {
Ok(psbt) => psbt,
Err(err) => {
log_error!(self.logger, "Failed to create temporary transaction: {}", err);
return Err(err.into());
},
};

// 'cancel' the transaction to free up any used change addresses
locked_wallet.cancel_tx(&psbt.unsigned_tx);
psbt.fee().map_err(|_| Error::WalletOperationFailed)
},
}
}

pub(crate) fn send_to_address(
&self, address: &bitcoin::Address, send_amount: OnchainSendAmount,
fee_rate: Option<FeeRate>,
Expand All @@ -378,60 +487,24 @@ where
OnchainSendAmount::AllRetainingReserve { cur_anchor_reserve_sats }
if cur_anchor_reserve_sats > DUST_LIMIT_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);
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: {}",
// estimate_fee_internal will enforce that we are retaining the reserve limits
let estimated_tx_fee = self
.estimate_fee_internal(&mut locked_wallet, address, send_amount, fee_rate)
.map_err(|e| {
log_error!(self.logger, "Failed to estimate fee: {e}");
e
);
e
})?;

// 'cancel' the transaction to free up any used change addresses
locked_wallet.cancel_tx(&tmp_tx);
})?;

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)
Expand Down
37 changes: 36 additions & 1 deletion tests/integration_tests_rust.rs
Original file line number Diff line number Diff line change
Expand Up @@ -383,9 +383,19 @@ fn onchain_send_receive() {
);

let amount_to_send_sats = 54321;
let fee_estimate = node_a
.onchain_payment()
.estimate_send_to_address(&addr_a, amount_to_send_sats, None)
.unwrap();
let txid =
node_b.onchain_payment().send_to_address(&addr_a, amount_to_send_sats, None).unwrap();
wait_for_tx(&electrsd.client, txid);

// verify we estimated the fee correctly
let entry = bitcoind.client.get_mempool_entry(txid).unwrap();
let actual_fee = Amount::from_btc(entry.0.fees.base).unwrap();
assert_eq!(fee_estimate, actual_fee);

node_a.sync_wallets().unwrap();
node_b.sync_wallets().unwrap();

Expand Down Expand Up @@ -449,10 +459,19 @@ fn onchain_send_receive() {
}

let addr_b = node_b.onchain_payment().new_address().unwrap();
let fee_estimate =
node_a.onchain_payment().estimate_send_all_to_address(&addr_b, true, None).unwrap();
let txid = node_a.onchain_payment().send_all_to_address(&addr_b, true, None).unwrap();
generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6);

wait_for_tx(&electrsd.client, txid);

// verify we estimated the fee correctly
let entry = bitcoind.client.get_mempool_entry(txid).unwrap();
let actual_fee = Amount::from_btc(entry.0.fees.base).unwrap();
assert_eq!(fee_estimate, actual_fee);

generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6);

node_a.sync_wallets().unwrap();
node_b.sync_wallets().unwrap();

Expand Down Expand Up @@ -522,9 +541,17 @@ fn onchain_send_all_retains_reserve() {
assert_eq!(node_b.list_balances().spendable_onchain_balance_sats, premine_amount_sat);

// Send all over, with 0 reserve as we don't have any channels open.
let fee_estimate =
node_a.onchain_payment().estimate_send_all_to_address(&addr_b, true, None).unwrap();
let txid = node_a.onchain_payment().send_all_to_address(&addr_b, true, None).unwrap();

wait_for_tx(&electrsd.client, txid);

// verify we estimated the fee correctly
let entry = bitcoind.client.get_mempool_entry(txid).unwrap();
let actual_fee = Amount::from_btc(entry.0.fees.base).unwrap();
assert_eq!(fee_estimate, actual_fee);

generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6);

node_a.sync_wallets().unwrap();
Expand Down Expand Up @@ -563,9 +590,17 @@ fn onchain_send_all_retains_reserve() {
.contains(&node_b.list_balances().spendable_onchain_balance_sats));

// Send all over again, this time ensuring the reserve is accounted for
let fee_estimate =
node_b.onchain_payment().estimate_send_all_to_address(&addr_a, true, None).unwrap();
let txid = node_b.onchain_payment().send_all_to_address(&addr_a, true, None).unwrap();

wait_for_tx(&electrsd.client, txid);

// verify we estimated the fee correctly
let entry = bitcoind.client.get_mempool_entry(txid).unwrap();
let actual_fee = Amount::from_btc(entry.0.fees.base).unwrap();
assert_eq!(fee_estimate, actual_fee);

generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6);

node_a.sync_wallets().unwrap();
Expand Down
Loading