From 9175c641025299393cde582c8841defadf4d2d23 Mon Sep 17 00:00:00 2001 From: amackillop Date: Fri, 5 Jun 2026 11:00:38 -0700 Subject: [PATCH] Reserve funding tx inputs at build time create_funding_transaction built and signed the funding tx but never applied it back to the BDK wallet graph, so the selected UTXOs stayed selectable until the next chain or mempool sync. Two opens between syncs would build funding transactions that spend the same inputs. The network rejects the loser, but its 0-conf channel is already live, leaving an unbacked phantom channel. Apply the extracted tx with apply_unconfirmed_txs before releasing the wallet lock so its inputs are marked spent immediately, with no window for a second build to reselect them. A forced wallet sync was the obvious alternative but cannot close the hole: broadcast is async so the tx is not in any mempool when we would sync, and the rejected loser never enters a mempool at all, so a sync could never reserve its inputs. Holding the inputs at build time means a failed open now has to release them again, otherwise the UTXOs are locked out of all future coin selection. The FundingGenerationReady handler rolls back via cancel_tx on any error before logging. The regression test gives node A a single UTXO, opens one 0-conf channel, and checks that the spendable balance drops without a sync and that a second open is rejected with InsufficientFunds. Before the fix that second open silently produced a phantom channel. --- src/event.rs | 53 ++++++++++++-------- src/wallet/mod.rs | 21 ++++++-- tests/integration_tests_rust.rs | 89 +++++++++++++++++++++++++++++++++ 3 files changed, 138 insertions(+), 25 deletions(-) diff --git a/src/event.rs b/src/event.rs index 6089b32354..03ae5e2a8e 100644 --- a/src/event.rs +++ b/src/event.rs @@ -585,6 +585,8 @@ where ) }); + let final_tx_for_rollback = final_tx.clone(); + let result = if needs_manual_broadcast { self.liquidity_source.as_ref().map(|ls| { ls.lsps2_store_funding_transaction( @@ -608,27 +610,38 @@ where match result { Ok(()) => {}, - Err(APIError::APIMisuseError { err }) => { - log_error!( - self.logger, - "Encountered APIMisuseError, this should never happen: {}", - err - ); - debug_assert!(false, "APIMisuseError: {}", err); - }, - Err(APIError::ChannelUnavailable { err }) => { - log_error!( - self.logger, - "Failed to process funding transaction as channel went away before we could fund it: {}", - err - ) - }, Err(err) => { - log_error!( - self.logger, - "Failed to process funding transaction: {:?}", - err - ) + if let Err(e) = self.wallet.cancel_tx(&final_tx_for_rollback) { + log_error!( + self.logger, + "Failed to cancel funding transaction after open failure: {:?}", + e + ); + } + match err { + APIError::APIMisuseError { err } => { + log_error!( + self.logger, + "Encountered APIMisuseError, this should never happen: {}", + err + ); + debug_assert!(false, "APIMisuseError: {}", err); + }, + APIError::ChannelUnavailable { err } => { + log_error!( + self.logger, + "Failed to process funding transaction as channel went away before we could fund it: {}", + err + ) + }, + err => { + log_error!( + self.logger, + "Failed to process funding transaction: {:?}", + err + ) + }, + } }, } }, diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 34422c7d35..9fb1aabb84 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -10,6 +10,7 @@ use std::ops::Deref; use std::pin::Pin; use std::str::FromStr; use std::sync::{Arc, Mutex}; +use std::time::{Duration, SystemTime, UNIX_EPOCH}; use bdk_chain::spk_client::{FullScanRequest, SyncRequest}; use bdk_wallet::descriptor::ExtendedDescriptor; @@ -266,17 +267,27 @@ impl Wallet { }, } + let tx = psbt.extract_tx().map_err(|e| { + log_error!(self.logger, "Failed to extract transaction: {}", e); + e + })?; + + // Record the funding tx in the wallet graph as unconfirmed *before* releasing the + // lock, so its inputs are immediately marked spent and a subsequent funding build + // cannot reselect the same UTXOs. Without this, two funding txs built between two + // chain syncs double-spend each other; the loser is rejected by the network while + // its 0-conf channel is already live, leaving an unbacked "phantom" channel. If the + // open later fails, `cancel_tx` rolls this back and frees the inputs again. + let last_seen = + SystemTime::now().duration_since(UNIX_EPOCH).unwrap_or(Duration::ZERO).as_secs(); + locked_wallet.apply_unconfirmed_txs([(tx.clone(), last_seen)]); + let mut locked_persister = self.persister.lock().unwrap(); locked_wallet.persist(&mut locked_persister).map_err(|e| { log_error!(self.logger, "Failed to persist wallet: {}", e); Error::PersistenceFailed })?; - let tx = psbt.extract_tx().map_err(|e| { - log_error!(self.logger, "Failed to extract transaction: {}", e); - e - })?; - Ok(tx) } diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index 954dce8385..008b3db006 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -118,6 +118,95 @@ async fn channel_full_cycle_legacy_staticremotekey() { .await; } +// Regression test for phantom 0-conf channels (MDK-968). `create_funding_transaction` now +// applies the built-and-signed funding tx back to the BDK wallet before returning, so its +// inputs are reserved immediately rather than only after the next chain/mempool sync. This +// prevents two opens between syncs from building funding transactions that double-spend the +// same UTXO(s) — the root cause of unbacked 0-conf channels in production. +// +// Pre-fix, the second open below silently succeeded and produced a phantom channel. Post-fix, +// node A's spendable balance reflects the spent UTXO without any sync, and the second open is +// rejected up front with `InsufficientFunds`. +#[tokio::test(flavor = "multi_thread", worker_threads = 1)] +async fn zero_conf_channel_funding_tx_no_double_spend() { + let (bitcoind, electrsd) = setup_bitcoind_and_electrsd(); + let chain_source = TestChainSource::Esplora(&electrsd); + + // Node A is the funder; B and C trust A for 0-conf channels. + println!("== Node A =="); + let config_a = random_config(false); + let node_a = setup_node(&chain_source, config_a, None); + + println!("\n== Node B =="); + let mut config_b = random_config(false); + config_b.node_config.trusted_peers_0conf.push(node_a.node_id()); + let node_b = setup_node(&chain_source, config_b, None); + + println!("\n== Node C =="); + let mut config_c = random_config(false); + config_c.node_config.trusted_peers_0conf.push(node_a.node_id()); + let node_c = setup_node(&chain_source, config_c, None); + + // Give node A exactly ONE confirmed UTXO. Any two funding txs built from this state must + // spend it, so they are double-spends of each other by construction. + let premine_amount_sat = 100_000; + let addr_a = node_a.onchain_payment().new_address().unwrap(); + premine_and_distribute_funds( + &bitcoind.client, + &electrsd.client, + vec![addr_a], + Amount::from_sat(premine_amount_sat), + ) + .await; + node_a.sync_wallets().unwrap(); + assert_eq!(node_a.list_balances().spendable_onchain_balance_sats, premine_amount_sat); + + // Two channels of 70k each need 140k, but node A only owns 100k. + let funding_amount_sat = 70_000; + + // First 0-conf channel A -> B. After ChannelPending the funding tx has been built, so the + // fix must already have reserved its inputs. + println!("\nA -- open_channel -> B"); + node_a + .open_channel( + node_b.node_id(), + node_b.listening_addresses().unwrap().first().unwrap().clone(), + funding_amount_sat, + None, + None, + ) + .unwrap(); + expect_channel_pending_event!(node_a, node_b.node_id()); + expect_channel_pending_event!(node_b, node_a.node_id()); + expect_channel_ready_event!(node_a, node_b.node_id()); + expect_channel_ready_event!(node_b, node_a.node_id()); + + // Crucially WITHOUT a sync: the spendable balance must already reflect the spent UTXO, + // leaving only the unconfirmed change (< funding_amount). Pre-fix this still read 100k. + assert!( + node_a.list_balances().spendable_onchain_balance_sats < funding_amount_sat, + "funding inputs were not reserved at build time" + ); + + // Second 0-conf channel A -> C would double-spend the same UTXO. With inputs reserved the + // open must be rejected here instead of silently producing a phantom channel. + println!("\nA -- open_channel -> C"); + assert_eq!( + Err(NodeError::InsufficientFunds), + node_a.open_channel( + node_c.node_id(), + node_c.listening_addresses().unwrap().first().unwrap().clone(), + funding_amount_sat, + None, + None, + ) + ); + + node_a.stop().unwrap(); + node_b.stop().unwrap(); + node_c.stop().unwrap(); +} + #[tokio::test(flavor = "multi_thread", worker_threads = 1)] async fn channel_open_fails_when_funds_insufficient() { let (bitcoind, electrsd) = setup_bitcoind_and_electrsd();