From 705d55f7ba20b6998bb39dc9b05ab8d8e785e71e Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Fri, 16 May 2025 15:12:13 +0200 Subject: [PATCH 1/3] Don't fail `send_all` retaining reserves for 0 channels Previouly, `OnchainPayment::send_all_to_address` would fail in the `retain_reserves` mode if the maintained reserves were below the dust limit. Most notably this would happen if we had no channels open at all. Here, we fix this by simply falling back to the draining case (not considering reserves) if the anchor reserves are below dust. We also add a unit test that would have caught this regression in the first place. --- src/wallet/mod.rs | 8 +++- tests/integration_tests_rust.rs | 83 +++++++++++++++++++++++++++++++++ 2 files changed, 89 insertions(+), 2 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index c4ef15731..c104a2346 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -355,6 +355,7 @@ where let mut locked_wallet = self.inner.lock().unwrap(); // Prepare the tx_builder. We properly check the reserve requirements (again) further down. + const DUST_LIMIT_SATS: u64 = 546; let tx_builder = match send_amount { OnchainSendAmount::ExactRetainingReserve { amount_sats, .. } => { let mut tx_builder = locked_wallet.build_tx(); @@ -362,7 +363,9 @@ where tx_builder.add_recipient(address.script_pubkey(), amount).fee_rate(fee_rate); tx_builder }, - OnchainSendAmount::AllRetainingReserve { cur_anchor_reserve_sats } => { + 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 @@ -419,7 +422,8 @@ where .fee_absolute(estimated_tx_fee); tx_builder }, - OnchainSendAmount::AllDrainingReserve => { + 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); tx_builder diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index ded88d35c..fde566202 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -496,6 +496,89 @@ 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(); + // Check node a sent all and node b received it + assert_eq!(node_a.list_balances().spendable_onchain_balance_sats, 0); + assert!(((premine_amount_sat * 2 - onchain_fee_buffer_sat)..=(premine_amount_sat * 2)) + .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()); + + // Check node a sent all and node b received it + 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(); + + // Check node b sent all and node a received it + 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(); From a8aab2bdf6df73a403838c3420f62715f612d14d Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Fri, 16 May 2025 15:17:39 +0200 Subject: [PATCH 2/3] Log how much we got when rejecting Anchor channels due to reserves .. as this is useful information. --- src/event.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/event.rs b/src/event.rs index 00d8441e5..28923675e 100644 --- a/src/event.rs +++ b/src/event.rs @@ -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( From 79eebfebe6bfacecacc023ec81dbc8b298e52508 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Tue, 20 May 2025 10:51:31 +0200 Subject: [PATCH 3/3] Call `Wallet::cancel_tx` for temporary transactions We create temporary transactions for fee estimation purposes. Doing this might advance the descriptor state though. So here we call `cancel_tx` to tell the wallet we no longer intend to broadcast the temporary transactions. --- src/wallet/mod.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index c104a2346..c4f37427e 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -403,6 +403,10 @@ where ); 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()), );