Skip to content

Don't fail send_all retaining reserves for 0 channels #540

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.

Sign up for GitHub

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

Don't fail send_all retaining reserves for 0 channels #540

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
55e6bae
Don't fail `send_all` retaining reserves for 0 channels
tnull May 16, 2025
c28cce2
Log how much we got when rejecting Anchor channels due to reserves
tnull May 16, 2025
0c027cf
Call `Wallet::cancel_tx` for temporary transactions
tnull May 16, 2025
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
4 changes: 3 additions & 1 deletion src/event.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1123,8 +1123,10 @@ where
if spendable_amount_sats < required_amount_sats {
log_error!(
self.logger,
"Rejecting inbound Anchor channel from peer {} due to insufficient available on-chain reserves.",
"Rejecting inbound Anchor channel from peer {} due to insufficient available on-chain reserves. Available: {}/{}sats",
counterparty_node_id,
spendable_amount_sats,
required_amount_sats,
);
self.channel_manager
.force_close_without_broadcasting_txn(
Expand Down
112 changes: 64 additions & 48 deletions src/wallet/mod.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,61 +363,77 @@ where
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);
match tmp_tx_builder.finish() {
Ok(psbt) => psbt.unsigned_tx,
Err(err) => {
const DUST_LIMIT_SATS: u64 = 546;
if cur_anchor_reserve_sats > DUST_LIMIT_SATS {
Comment on lines 365 to +367
Copy link
Contributor

Choose a reason for hiding this comment

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

We can avoid the large diff by moving the conditional check into the match arm and having the else fall through to OnchainSendAmount::AllRetainingReserve { .. } | OnchainSendAmount::AllDrainingReserve.

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 create temporary transaction: {}",
err
"Failed to calculate fee of temporary transaction: {}",
e
);
return Err(err.into());
},
}
};
e
})?;

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()),
);
// 'cancel' the transaction to free up any used change addresses
locked_wallet.cancel_tx(&tmp_tx);

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,
let estimated_spendable_amount = Amount::from_sat(
spendable_amount_sats.saturating_sub(estimated_tx_fee.to_sat()),
);
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);
tx_builder
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);
tx_builder
} else {
let mut tx_builder = locked_wallet.build_tx();
tx_builder
.drain_wallet()
.drain_to(address.script_pubkey())
.fee_rate(fee_rate);
tx_builder
}
},
OnchainSendAmount::AllDrainingReserve => {
let mut tx_builder = locked_wallet.build_tx();
Expand Down
80 changes: 80 additions & 0 deletions tests/integration_tests_rust.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -496,6 +496,86 @@ fn onchain_send_receive() {
assert_eq!(node_b_payments.len(), 5);
}

#[test]
fn onchain_send_all_retains_reserve() {
let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();
let chain_source = TestChainSource::Esplora(&electrsd);
let (node_a, node_b) = setup_two_nodes(&chain_source, false, true, false);

// Setup nodes
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_000_000;
let reserve_amount_sat = 25_000;
let onchain_fee_buffer_sat = 1000;
premine_and_distribute_funds(
&bitcoind.client,
&electrsd.client,
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_a.list_balances().spendable_onchain_balance_sats, premine_amount_sat);
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 txid = node_a.onchain_payment().send_all_to_address(&addr_b, true, None).unwrap();

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

node_a.sync_wallets().unwrap();
node_b.sync_wallets().unwrap();
assert_eq!(node_a.list_balances().spendable_onchain_balance_sats, 0);
assert!(((premine_amount_sat * 2 - onchain_fee_buffer_sat)..(premine_amount_sat * 2))
Copy link
Contributor

Choose a reason for hiding this comment

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

Should the ranges use ..= here and below?

.contains(&node_b.list_balances().spendable_onchain_balance_sats));

// Refill to make sure we have enough reserve for the channel open.
let txid = bitcoind
.client
.send_to_address(&addr_a, Amount::from_sat(reserve_amount_sat))
.unwrap()
.0
.parse()
.unwrap();
wait_for_tx(&electrsd.client, txid);
generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6);
node_a.sync_wallets().unwrap();
node_b.sync_wallets().unwrap();
assert_eq!(node_a.list_balances().spendable_onchain_balance_sats, reserve_amount_sat);

// Open a channel.
open_channel(&node_b, &node_a, premine_amount_sat, false, &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());

assert_eq!(node_a.list_balances().spendable_onchain_balance_sats, 0);
assert!(((premine_amount_sat - reserve_amount_sat - onchain_fee_buffer_sat)
..premine_amount_sat)
.contains(&node_b.list_balances().spendable_onchain_balance_sats));

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

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

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

assert_eq!(node_b.list_balances().total_onchain_balance_sats, reserve_amount_sat);
assert_eq!(node_b.list_balances().spendable_onchain_balance_sats, 0);
assert!(((premine_amount_sat - reserve_amount_sat - onchain_fee_buffer_sat)
..premine_amount_sat)
.contains(&node_a.list_balances().spendable_onchain_balance_sats));
}

#[test]
fn onchain_wallet_recovery() {
let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();
Expand Down
Loading