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.

Author: tnull

src/wallet/mod.rs

@@ -363,61 +363,73 @@ 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 {
+						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());
-							},
-						}
-					};
-
-					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()),
 						);
-						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);
-					}
+						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
+						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();

tests/integration_tests_rust.rs

@@ -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))
+		.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();