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();