From 75ce7f0ade7cf2c47262dc82d230bc83ebfcc2d1 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Tue, 20 Oct 2026 17:18:35 +0200 Subject: [PATCH 1/5] Upgrade BDK Wallet to 3.2 Our wallet needs per-input finalization results to distinguish unfinished foreign inputs from failures to finalize its own inputs. Raise the minimum BDK Wallet version to 3.2, which provides these results alongside fixes for TRUC input selection and changeset merging. Co-Authored-By: HAL 9000 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 448daa3803..ba98aebdd6 100755 --- a/Cargo.toml +++ b/Cargo.toml @@ -102,7 +102,7 @@ lightning-dns-resolver = { git = "https://github.com/lightningdevkit/rust-lightn bdk_chain = { version = "0.23.3", default-features = false, features = ["std"] } bdk_esplora = { version = "0.22.2", default-features = false, features = ["async-https-rustls", "tokio"], optional = true } bdk_electrum = { version = "0.24.0", default-features = false, features = ["use-rustls-ring"], optional = true } -bdk_wallet = { version = "3.1.0", default-features = false, features = ["std", "keys-bip39"]} +bdk_wallet = { version = "3.2.0", default-features = false, features = ["std", "keys-bip39"]} bitreq = { version = "0.3", default-features = false, features = ["async-https", "json-using-serde"] } rustls = { version = "0.23", default-features = false } From db2ed565e5ba99687fa68445854af76e7419d779 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Tue, 20 Oct 2026 17:24:07 +0200 Subject: [PATCH 2/5] Move to caller-owned wallet signers BDK's wallet-owned signing APIs are deprecated, but our signing paths still need their witness-UTXO policy, sighash checks, low-R signatures, and input finalization. Use caller-owned signers for both keychains to preserve these signing options while dropping the deprecated APIs. Keep signing available after reload, and cover witness-only signing from both keychains. Refs #654 Co-Authored-By: HAL 9000 --- src/builder.rs | 24 +++++- src/error.rs | 2 - src/wallet/mod.rs | 135 ++++++++++++++++++++++++++------ tests/integration_tests_rust.rs | 50 ++++++++++-- 4 files changed, 176 insertions(+), 35 deletions(-) diff --git a/src/builder.rs b/src/builder.rs index ddf9104cc2..21f15508b4 100644 --- a/src/builder.rs +++ b/src/builder.rs @@ -17,6 +17,8 @@ use std::path::PathBuf; use std::sync::{Arc, Mutex, Once, RwLock}; use std::time::SystemTime; +use bdk_wallet::descriptor::IntoWalletDescriptor; +use bdk_wallet::signer::SignersContainer; use bdk_wallet::template::Bip84; use bdk_wallet::{KeychainKind, Wallet as BdkWallet}; use bitcoin::bip32::{ChildNumber, Xpriv}; @@ -1758,17 +1760,30 @@ fn build_with_store_internal( BuildError::WalletSetupFailed })?; - let descriptor = Bip84(xprv, KeychainKind::External); - let change_descriptor = Bip84(xprv, KeychainKind::Internal); + let secp_ctx = Secp256k1::new(); + let (descriptor, keymap) = Bip84(xprv, KeychainKind::External) + .into_wallet_descriptor(&secp_ctx, config.network.into()) + .map_err(|e| { + log_error!(logger, "Failed to derive external wallet descriptor: {}", e); + BuildError::WalletSetupFailed + })?; + let (change_descriptor, change_keymap) = Bip84(xprv, KeychainKind::Internal) + .into_wallet_descriptor(&secp_ctx, config.network.into()) + .map_err(|e| { + log_error!(logger, "Failed to derive internal wallet descriptor: {}", e); + BuildError::WalletSetupFailed + })?; + let signers = [ + SignersContainer::build(keymap, &descriptor, &secp_ctx), + SignersContainer::build(change_keymap, &change_descriptor, &secp_ctx), + ]; let mut wallet_persister = KVStoreWalletPersister::new(Arc::clone(&kv_store), Arc::clone(&logger)); - #[allow(deprecated)] let wallet_opt = runtime .block_on(async { BdkWallet::load() .descriptor(KeychainKind::External, Some(descriptor.clone())) .descriptor(KeychainKind::Internal, Some(change_descriptor.clone())) - .extract_keys() .check_network(config.network) .load_wallet_async(&mut wallet_persister) .await @@ -1957,6 +1972,7 @@ fn build_with_store_internal( let wallet = Arc::new(Wallet::new( bdk_wallet, + signers, wallet_persister, persisted_pool_indices, Arc::clone(&tx_broadcaster), diff --git a/src/error.rs b/src/error.rs index d187c68200..202cb38878 100644 --- a/src/error.rs +++ b/src/error.rs @@ -11,7 +11,6 @@ use bdk_chain::bitcoin::psbt::ExtractTxError as BdkExtractTxError; use bdk_chain::local_chain::CannotConnectError as BdkChainConnectionError; use bdk_chain::tx_graph::CalculateFeeError as BdkChainCalculateFeeError; use bdk_wallet::error::CreateTxError as BdkCreateTxError; -#[allow(deprecated)] use bdk_wallet::signer::SignerError as BdkSignerError; #[derive(Copy, Clone, Debug, PartialEq, Eq)] @@ -255,7 +254,6 @@ impl fmt::Display for Error { impl std::error::Error for Error {} -#[allow(deprecated)] impl From for Error { fn from(_: BdkSignerError) -> Self { Self::OnchainTxSigningFailed diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index b883ea918a..808b229ffe 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -15,9 +15,10 @@ use bdk_chain::spk_client::{FullScanRequest, SyncRequest}; use bdk_chain::ChainPosition; use bdk_wallet::descriptor::ExtendedDescriptor; use bdk_wallet::error::{BuildFeeBumpError, CreateTxError}; -#[allow(deprecated)] -use bdk_wallet::SignOptions; -use bdk_wallet::{Balance, KeychainKind, LocalOutput, PersistedWallet, Update, WalletEvent}; +use bdk_wallet::signer::SignersContainer; +use bdk_wallet::{ + Balance, KeychainKind, LocalOutput, PersistedWallet, SignOptions, Update, WalletEvent, +}; use bitcoin::address::NetworkUnchecked; use bitcoin::blockdata::constants::WITNESS_SCALE_FACTOR; use bitcoin::blockdata::locktime::absolute::LockTime; @@ -160,6 +161,8 @@ impl AddressPool { pub(crate) struct Wallet { // A BDK on-chain wallet. inner: Mutex>, + // External and internal signing keys are kept outside the BDK wallet. + signers: [SignersContainer; 2], persister: tokio::sync::Mutex, address_pool: Mutex, // Serializes refill runs so concurrent pops never over-reveal. @@ -186,10 +189,11 @@ pub(crate) struct Wallet { impl Wallet { pub(crate) fn new( wallet: bdk_wallet::PersistedWallet, - wallet_persister: KVStoreWalletPersister, persisted_pool_indices: Vec, - broadcaster: Arc, fee_estimator: Arc, - chain_source: Arc, payment_store: Arc, runtime: Arc, - config: Arc, logger: Arc, pending_payment_store: Arc, + signers: [SignersContainer; 2], wallet_persister: KVStoreWalletPersister, + persisted_pool_indices: Vec, broadcaster: Arc, + fee_estimator: Arc, chain_source: Arc, + payment_store: Arc, runtime: Arc, config: Arc, + logger: Arc, pending_payment_store: Arc, channel_tx_facts_store: Arc, ) -> Self { let address_pool = Mutex::new(AddressPool::new(persisted_pool_indices, &wallet, &logger)); @@ -198,6 +202,7 @@ impl Wallet { let address_pool_refill_lock = tokio::sync::Mutex::new(()); Self { inner, + signers, persister, address_pool, address_pool_refill_lock, @@ -1700,7 +1705,6 @@ impl Wallet { Ok(()) } - #[allow(deprecated)] pub(crate) async fn create_funding_transaction( &self, output_script: ScriptBuf, amount: Amount, confirmation_target: ConfirmationTarget, locktime: LockTime, fee_rate: Option, @@ -1725,7 +1729,11 @@ impl Wallet { }, }; - match locked_wallet.sign(&mut psbt, SignOptions::default()) { + match locked_wallet.sign_with_signers( + &mut psbt, + &self.signers.each_ref(), + SignOptions::default(), + ) { Ok(finalized) => { if !finalized { return Err(Error::OnchainTxCreationFailed); @@ -2166,7 +2174,6 @@ impl Wallet { .map_err(|_| Error::InvalidAddress) } - #[allow(deprecated)] pub(crate) async fn send_to_address( &self, address: &bitcoin::Address, send_amount: OnchainSendAmount, fee_rate: Option, @@ -2288,7 +2295,11 @@ impl Wallet { _ => {}, } - match locked_wallet.sign(&mut psbt, SignOptions::default()) { + match locked_wallet.sign_with_signers( + &mut psbt, + &self.signers.each_ref(), + SignOptions::default(), + ) { Ok(finalized) => { if !finalized { return Err(Error::OnchainTxCreationFailed); @@ -2527,7 +2538,6 @@ impl Wallet { Ok(utxos) } - #[allow(deprecated)] async fn get_change_script_inner(&self) -> Result { let mut locked_persister = self.persister.lock().await; let (address_info, change_set) = { @@ -2541,7 +2551,6 @@ impl Wallet { Ok(address_info.address.script_pubkey()) } - #[allow(deprecated)] pub(crate) fn sign_owned_inputs(&self, unsigned_tx: Transaction) -> Result { let locked_wallet = self.inner.lock().expect("lock"); @@ -2564,7 +2573,7 @@ impl Wallet { let mut sign_options = SignOptions::default(); sign_options.trust_witness_utxo = true; - match locked_wallet.sign(&mut psbt, sign_options) { + match locked_wallet.sign_with_signers(&mut psbt, &self.signers.each_ref(), sign_options) { Ok(finalized) => debug_assert!(!finalized), Err(e) => { log_error!(self.logger, "Failed to sign owned inputs: {}", e); @@ -2582,7 +2591,6 @@ impl Wallet { } } - #[allow(deprecated)] fn sign_psbt_inner(&self, mut psbt: Psbt) -> Result { let locked_wallet = self.inner.lock().expect("lock"); @@ -2592,7 +2600,7 @@ impl Wallet { let mut sign_options = SignOptions::default(); sign_options.trust_witness_utxo = true; - match locked_wallet.sign(&mut psbt, sign_options) { + match locked_wallet.sign_with_signers(&mut psbt, &self.signers.each_ref(), sign_options) { Ok(_finalized) => { // BDK will fail to finalize for all LDK-provided inputs of the PSBT. Unfortunately // we can't check more fine grained if it succeeded for all the other inputs here, @@ -3367,7 +3375,6 @@ impl Wallet { Ok(FundingStatusUpdate::Applied) } - #[allow(deprecated)] pub(crate) async fn bump_fee_rbf( &self, payment_id: PaymentId, fee_rate: Option, cur_anchor_reserve_sats: u64, ) -> Result { @@ -3618,7 +3625,11 @@ impl Wallet { return Err(Error::InsufficientFunds); } - match locked_wallet.sign(&mut psbt, SignOptions::default()) { + match locked_wallet.sign_with_signers( + &mut psbt, + &self.signers.each_ref(), + SignOptions::default(), + ) { Ok(finalized) => { if !finalized { log_error!(self.logger, "Failed to finalize signing for fee bump of {}", txid); @@ -4176,6 +4187,7 @@ mod tests { use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; use bdk_chain::{BlockId, CheckPoint, ConfirmationBlockTime, TxUpdate}; + use bdk_wallet::descriptor::IntoWalletDescriptor; use bdk_wallet::Wallet as BdkWallet; use bitcoin::hashes::Hash; use bitcoin::Network; @@ -4384,19 +4396,26 @@ mod tests { let mut wallet_persister = KVStoreWalletPersister::new(Arc::clone(&store), Arc::clone(&logger)); - #[allow(deprecated)] + let secp_ctx = Secp256k1::new(); + let (descriptor, keymap) = + EXTERNAL_DESCRIPTOR.into_wallet_descriptor(&secp_ctx, config.network.into()).unwrap(); + let (change_descriptor, change_keymap) = + INTERNAL_DESCRIPTOR.into_wallet_descriptor(&secp_ctx, config.network.into()).unwrap(); + let signers = [ + SignersContainer::build(keymap, &descriptor, &secp_ctx), + SignersContainer::build(change_keymap, &change_descriptor, &secp_ctx), + ]; let bdk_wallet = if load_existing { BdkWallet::load() - .descriptor(KeychainKind::External, Some(EXTERNAL_DESCRIPTOR)) - .descriptor(KeychainKind::Internal, Some(INTERNAL_DESCRIPTOR)) - .extract_keys() + .descriptor(KeychainKind::External, Some(descriptor)) + .descriptor(KeychainKind::Internal, Some(change_descriptor)) .check_network(Network::Regtest) .load_wallet_async(&mut wallet_persister) .await .unwrap() .unwrap() } else { - BdkWallet::create(EXTERNAL_DESCRIPTOR, INTERNAL_DESCRIPTOR) + BdkWallet::create(descriptor, change_descriptor) .network(Network::Regtest) .create_wallet_async(&mut wallet_persister) .await @@ -4460,6 +4479,7 @@ mod tests { Arc::new(Wallet::new( bdk_wallet, + signers, wallet_persister, persisted_pool_indices, broadcaster, @@ -4493,6 +4513,75 @@ mod tests { ) } + #[tokio::test] + async fn sign_both_keychains_before_and_after_reload() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let mut wallet = new_test_wallet(Arc::clone(&store), false).await; + let secp_ctx = Secp256k1::new(); + + for reload in [false, true] { + if reload { + wallet = new_test_wallet(Arc::clone(&store), true).await; + } + + let prevouts = { + let locked_wallet = wallet.inner.lock().unwrap(); + [KeychainKind::External, KeychainKind::Internal].map(|keychain| TxOut { + value: Amount::from_sat(100_000), + script_pubkey: locked_wallet.peek_address(keychain, 0).address.script_pubkey(), + }) + }; + let unsigned_tx = Transaction { + version: bitcoin::transaction::Version::TWO, + lock_time: LockTime::ZERO, + input: (0..2) + .map(|vout| bitcoin::TxIn { + previous_output: OutPoint { txid: Txid::from_byte_array([42; 32]), vout }, + ..Default::default() + }) + .collect(), + output: vec![TxOut { + value: Amount::from_sat(199_000), + script_pubkey: prevouts[0].script_pubkey.clone(), + }], + }; + let mut psbt = Psbt::from_unsigned_tx(unsigned_tx).unwrap(); + for (input, prevout) in psbt.inputs.iter_mut().zip(&prevouts) { + // LDK supplies witness UTXOs without the full previous transactions. + input.witness_utxo = Some(prevout.clone()); + } + + let tx = wallet.sign_psbt_inner(psbt).unwrap(); + let mut sighash_cache = bitcoin::sighash::SighashCache::new(&tx); + for (index, (input, prevout)) in tx.input.iter().zip(&prevouts).enumerate() { + assert_eq!(input.witness.len(), 2, "both keychains must be finalized"); + let signature = bitcoin::ecdsa::Signature::from_slice(&input.witness[0]).unwrap(); + let public_key = bitcoin::PublicKey::from_slice(&input.witness[1]).unwrap(); + assert_eq!( + ScriptBuf::new_p2wpkh(&public_key.wpubkey_hash().unwrap()), + prevout.script_pubkey + ); + assert_eq!(signature.sighash_type, bitcoin::sighash::EcdsaSighashType::All); + assert!(input.witness[0].len() <= 71, "ECDSA signatures must use low R"); + let sighash = sighash_cache + .p2wpkh_signature_hash( + index, + &prevout.script_pubkey, + prevout.value, + signature.sighash_type, + ) + .unwrap(); + secp_ctx + .verify_ecdsa( + &bitcoin::secp256k1::Message::from_digest(sighash.to_byte_array()), + &signature.signature, + &public_key.inner, + ) + .unwrap(); + } + } + } + #[tokio::test] async fn refill_publishes_addresses_only_after_their_reveal_is_persisted() { let fail_store = FailSwitchStore::new(); diff --git a/tests/integration_tests_rust.rs b/tests/integration_tests_rust.rs index a48f38cc04..499fa55d2d 100644 --- a/tests/integration_tests_rust.rs +++ b/tests/integration_tests_rust.rs @@ -6639,7 +6639,7 @@ enum OldLdkVersion { async fn build_0_6_2_node( bitcoind: &BitcoinD, electrsd: &ElectrsD, storage_path: String, esplora_url: String, seed_bytes: [u8; 64], -) -> (u64, bitcoin::secp256k1::PublicKey) { +) -> (u64, bitcoin::secp256k1::PublicKey, Txid) { let mut builder_old = ldk_node_062::Builder::new(); builder_old.set_network(bitcoin::Network::Regtest); builder_old.set_storage_dir_path(storage_path); @@ -6658,6 +6658,14 @@ async fn build_0_6_2_node( .await; node_old.sync_wallets().unwrap(); + // Pay ourselves so the upgraded node holds funds on both the external and internal keychains. + let self_addr = node_old.onchain_payment().new_address().unwrap(); + let self_send_txid = + node_old.onchain_payment().send_to_address(&self_addr, 50_000, None).unwrap(); + wait_for_tx(&electrsd.client, self_send_txid).await; + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_old.sync_wallets().unwrap(); + let balance = node_old.list_balances().spendable_onchain_balance_sats; assert!(balance > 0); let node_id = node_old.node_id(); @@ -6668,13 +6676,13 @@ async fn build_0_6_2_node( drop(node_old); }); - (balance, node_id) + (balance, node_id, self_send_txid) } async fn build_0_7_0_node( bitcoind: &BitcoinD, electrsd: &ElectrsD, esplora_url: String, seed_bytes: [u8; 64], config: &TestConfig, -) -> (u64, bitcoin::secp256k1::PublicKey) { +) -> (u64, bitcoin::secp256k1::PublicKey, Txid) { let mut builder_old = ldk_node_070::Builder::new(); builder_old.set_network(bitcoin::Network::Regtest); builder_old.set_storage_dir_path(config.node_config.storage_dir_path.clone()); @@ -6699,13 +6707,21 @@ async fn build_0_7_0_node( .await; node_old.sync_wallets().unwrap(); + // Pay ourselves so the upgraded node holds funds on both the external and internal keychains. + let self_addr = node_old.onchain_payment().new_address().unwrap(); + let self_send_txid = + node_old.onchain_payment().send_to_address(&self_addr, 50_000, None).unwrap(); + wait_for_tx(&electrsd.client, self_send_txid).await; + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_old.sync_wallets().unwrap(); + let balance = node_old.list_balances().spendable_onchain_balance_sats; assert!(balance > 0); let node_id = node_old.node_id(); node_old.stop().unwrap(); - (balance, node_id) + (balance, node_id, self_send_txid) } async fn do_persistence_backwards_compatibility(version: OldLdkVersion) { @@ -6715,7 +6731,7 @@ async fn do_persistence_backwards_compatibility(version: OldLdkVersion) { let storage_path = common::random_storage_path().to_str().unwrap().to_owned(); let seed_bytes = [42u8; 64]; - let (old_balance, old_node_id) = match version { + let (old_balance, old_node_id, self_send_txid) = match version { OldLdkVersion::V0_6_2 => { build_0_6_2_node( &bitcoind, @@ -6758,6 +6774,28 @@ async fn do_persistence_backwards_compatibility(version: OldLdkVersion) { assert_eq!(old_node_id, new_node_id); assert_eq!(old_balance, new_balance); + // Spending both outputs of the self-payment requires signing for both keychains with the + // keys derived by the upgraded node. + let sweep_addr = bitcoind.client.new_address().unwrap(); + let sweep_txid = + node_new.onchain_payment().send_all_to_address(&sweep_addr, false, None).unwrap(); + wait_for_tx(&electrsd.client, sweep_txid).await; + let sweep_tx = electrsd.client.transaction_get(&sweep_txid).unwrap(); + let mut spent_vouts: Vec = sweep_tx + .input + .iter() + .map(|input| { + assert_eq!(input.previous_output.txid, self_send_txid); + input.previous_output.vout + }) + .collect(); + spent_vouts.sort(); + assert_eq!(spent_vouts, vec![0, 1]); + + generate_blocks_and_wait(&bitcoind.client, &electrsd.client, 6).await; + node_new.sync_wallets().unwrap(); + assert_eq!(node_new.list_balances().total_onchain_balance_sats, 0); + node_new.stop().unwrap(); } @@ -6780,7 +6818,7 @@ async fn fs_store_persistence_backwards_compatibility() { let mut config = TestConfig::default(); config.node_config.storage_dir_path = storage_path.clone(); config.store_type = TestStoreType::FilesystemStore; - let (old_balance, old_node_id) = + let (old_balance, old_node_id, _) = build_0_7_0_node(&bitcoind, &electrsd, esplora_url.clone(), seed_bytes, &config).await; // Now reopen with current code's build_with_fs_store, which should From 6a175442ccc2e9471e7da2d65a7f9562f1b3fa5c Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Tue, 20 Oct 2026 17:26:23 +0200 Subject: [PATCH 3/5] Test that a clean Electrum EOF wakes concurrent requests Concurrent requests sharing an Electrum connection can remain blocked when the active reader sees a clean EOF. This can keep blocking Tokio workers alive through node shutdown. electrum-client 0.25.1 fixed this, and we already require a release that includes the fix. Cover the failure at the LDK Node boundary so dependency updates cannot reintroduce the hang. Fixes #1007 Refs #1055 (cherry picked from commit aff6ce0d1148c0648fd4ea4f6412608211c93d9a) Co-Authored-By: HAL 9000 --- src/chain/electrum.rs | 88 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 87 insertions(+), 1 deletion(-) diff --git a/src/chain/electrum.rs b/src/chain/electrum.rs index 2841ec70b0..00155771a9 100644 --- a/src/chain/electrum.rs +++ b/src/chain/electrum.rs @@ -934,14 +934,100 @@ impl Filter for ElectrumRuntimeClient { #[cfg(test)] mod tests { + use std::io::{BufRead, BufReader, Write}; + use std::net::{Shutdown, TcpListener}; use std::sync::atomic::{AtomicUsize, Ordering}; - use std::sync::mpsc; + use std::sync::{mpsc, Barrier}; use std::thread; use bitcoin::blockdata::constants::genesis_block; use super::*; + #[test] + fn clean_eof_wakes_concurrent_electrum_requests() { + const REQUEST_COUNT: usize = 8; + let listener = TcpListener::bind("127.0.0.1:0").unwrap(); + let server_address = listener.local_addr().unwrap(); + let (client_sender, client_receiver) = mpsc::channel::>(); + let server = thread::spawn(move || { + let (stream, _) = listener.accept().unwrap(); + let mut reader = BufReader::new(stream); + let mut version_request = String::new(); + assert!(reader.read_line(&mut version_request).unwrap() > 0); + let version_request: serde_json::Value = + serde_json::from_str(&version_request).unwrap(); + let version_response = serde_json::json!({ + "jsonrpc": "2.0", + "id": version_request["id"], + "result": ["test-server", "1.4"], + }); + writeln!(reader.get_mut(), "{version_response}").unwrap(); + reader.get_mut().flush().unwrap(); + let client = client_receiver.recv().unwrap(); + + // The client's reader thread holds the shared stream while it blocks on a read, so + // other requests can't be written meanwhile. Send notifications to repeatedly + // unblock it until every request was fully written and flushed (excluding the + // version handshake). + let nudge = serde_json::json!({"jsonrpc": "2.0", "method": "test.nudge", "params": []}); + let deadline = std::time::Instant::now() + Duration::from_secs(30); + while client.calls_made().unwrap() - 1 < REQUEST_COUNT { + assert!(std::time::Instant::now() < deadline, "requests weren't all sent"); + writeln!(reader.get_mut(), "{nudge}").unwrap(); + thread::sleep(Duration::from_millis(10)); + } + drop(client); + // Give the requests time to start waiting for their responses. + thread::sleep(Duration::from_millis(50)); + + // Only shut down our sending side so the client observes a clean EOF. Closing the + // entire socket with unread requests could send a TCP reset instead, which older + // clients already handled. + reader.get_ref().shutdown(Shutdown::Write).unwrap(); + + // Keep reading until the client hangs up so unread requests don't trigger a reset. + reader.get_mut().set_read_timeout(Some(Duration::from_secs(10))).unwrap(); + let mut remaining = String::new(); + while matches!(reader.read_line(&mut remaining), Ok(n) if n > 0) { + remaining.clear(); + } + }); + + let config = ElectrumConfigBuilder::new().retry(0).build(); + let client = Arc::new( + ElectrumClient::from_config(&format!("tcp://{server_address}"), config).unwrap(), + ); + client_sender.send(Arc::clone(&client)).unwrap(); + let start = Arc::new(Barrier::new(REQUEST_COUNT + 1)); + let (finished_sender, finished_receiver) = mpsc::channel(); + let mut requests = Vec::new(); + for _ in 0..REQUEST_COUNT { + let client = Arc::clone(&client); + let start = Arc::clone(&start); + let finished_sender = finished_sender.clone(); + requests.push(thread::spawn(move || { + start.wait(); + let result = client.raw_call("server.ping", Vec::::new()); + finished_sender.send(result.is_err()).unwrap(); + })); + } + start.wait(); + + for _ in 0..REQUEST_COUNT { + let failed_as_expected = finished_receiver + .recv_timeout(Duration::from_secs(60)) + .expect("clean EOF must wake every concurrent Electrum request"); + assert!(failed_as_expected); + } + for request in requests { + request.join().unwrap(); + } + // Hang up so the server stops reading. + drop(client); + server.join().unwrap(); + } + struct RecordingConfirm { calls: AtomicUsize, relevant_txid: Txid, From e55d7594df5b180746e6e372eab19cec707b8be9 Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Tue, 20 Oct 2026 18:20:24 +0200 Subject: [PATCH 4/5] Require wallet inputs to be finalized Mixed wallet and LDK transactions could be returned with unsigned wallet inputs because the aggregate signing result also accounts for unfinished foreign inputs. Require BDK to finalize every recognized wallet input while allowing inputs without a wallet descriptor to remain unfinished. Cover both signing paths with tests that exercise missing wallet signers. Co-Authored-By: HAL 9000 --- src/wallet/mod.rs | 120 +++++++++++++++++++++++++++++++++++++++++----- 1 file changed, 109 insertions(+), 11 deletions(-) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index 808b229ffe..d679843e77 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -17,7 +17,8 @@ use bdk_wallet::descriptor::ExtendedDescriptor; use bdk_wallet::error::{BuildFeeBumpError, CreateTxError}; use bdk_wallet::signer::SignersContainer; use bdk_wallet::{ - Balance, KeychainKind, LocalOutput, PersistedWallet, SignOptions, Update, WalletEvent, + Balance, FinalizeInputOutcome, KeychainKind, LocalOutput, PersistedWallet, SignOptions, Update, + WalletEvent, }; use bitcoin::address::NetworkUnchecked; use bitcoin::blockdata::constants::WITNESS_SCALE_FACTOR; @@ -2580,6 +2581,7 @@ impl Wallet { return Err(()); }, } + self.finalize_wallet_inputs(&locked_wallet, &mut psbt)?; match psbt.extract_tx() { Ok(tx) => Ok(tx), @@ -2600,17 +2602,12 @@ impl Wallet { let mut sign_options = SignOptions::default(); sign_options.trust_witness_utxo = true; - match locked_wallet.sign_with_signers(&mut psbt, &self.signers.each_ref(), sign_options) { - Ok(_finalized) => { - // BDK will fail to finalize for all LDK-provided inputs of the PSBT. Unfortunately - // we can't check more fine grained if it succeeded for all the other inputs here, - // so we just ignore the returned `finalized` bool. - }, - Err(err) => { + locked_wallet + .sign_with_signers(&mut psbt, &self.signers.each_ref(), sign_options) + .map_err(|err| { log_error!(self.logger, "Failed to sign transaction: {}", err); - return Err(()); - }, - } + })?; + self.finalize_wallet_inputs(&locked_wallet, &mut psbt)?; let tx = psbt.extract_tx().map_err(|e| { log_error!(self.logger, "Failed to extract transaction: {}", e); @@ -2620,6 +2617,28 @@ impl Wallet { Ok(tx) } + fn finalize_wallet_inputs( + &self, wallet: &bdk_wallet::Wallet, psbt: &mut Psbt, + ) -> Result<(), ()> { + let outcome = wallet.try_finalize_psbt(psbt).map_err(|e| { + log_error!(self.logger, "Failed to finalize transaction: {}", e); + })?; + for (index, input_outcome) in outcome.into_outcomes() { + match input_outcome { + FinalizeInputOutcome::AlreadyFinalized | FinalizeInputOutcome::Finalized => {}, + // Expected for LDK's inputs, which BDK cannot finalize. + FinalizeInputOutcome::MissingDescriptor => {}, + // A descriptor that BDK recognizes must be satisfied before we return the + // transaction. + FinalizeInputOutcome::CouldNotSatisfy(e) => { + log_error!(self.logger, "Failed to finalize wallet input {}: {}", index, e); + return Err(()); + }, + } + } + Ok(()) + } + /// Builds this node's share of the `active` round of an interactive funding whose negotiated /// history is `candidates`, and the per-candidate figures of that history, for recording the /// round under `payment_id`. Returns `None` when there is nothing to record: no local @@ -4582,6 +4601,85 @@ mod tests { } } + #[tokio::test] + async fn sign_owned_inputs_requires_wallet_finalization() { + assert_signing_requires_every_wallet_signer(|wallet, psbt| { + wallet.sign_owned_inputs(psbt.unsigned_tx) + }) + .await; + } + + #[tokio::test] + async fn sign_psbt_requires_wallet_finalization() { + assert_signing_requires_every_wallet_signer(|wallet, psbt| wallet.sign_psbt_inner(psbt)) + .await; + } + + /// Checks that `sign` finalizes the inputs of both keychains while leaving foreign inputs + /// unsigned, and that it fails if the signer of either keychain is missing. + async fn assert_signing_requires_every_wallet_signer( + sign: impl Fn(&Wallet, Psbt) -> Result, + ) { + let (wallet, psbt) = mixed_input_psbt().await; + let tx = sign(&wallet, psbt).unwrap(); + assert_eq!(tx.input[0].witness.len(), 2, "external input must be finalized"); + assert_eq!(tx.input[1].witness.len(), 2, "internal input must be finalized"); + assert!(tx.input[2].witness.is_empty(), "foreign inputs may remain unsigned"); + + // Signers are indexed by keychain: external first, internal second. + for missing_signers in [&[1][..], &[0], &[0, 1]] { + let (mut wallet, psbt) = mixed_input_psbt().await; + for &index in missing_signers { + Arc::get_mut(&mut wallet).unwrap().signers[index] = SignersContainer::new(); + } + assert!( + sign(&wallet, psbt).is_err(), + "an unfinished wallet input must fail signing (missing signers: {missing_signers:?})" + ); + } + } + + /// A PSBT spending an external and an internal wallet output, and a foreign output awaiting + /// its owner's signature. + async fn mixed_input_psbt() -> (Arc, Psbt) { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let wallet = new_test_wallet(store, false).await; + let external_funding_tx = wallet_paying_tx(&wallet, 42); + insert_confirmed_tx(&wallet, external_funding_tx.clone(), 1); + let mut internal_funding_tx = wallet_paying_tx(&wallet, 44); + internal_funding_tx.output[0].script_pubkey = wallet + .inner + .lock() + .unwrap() + .reveal_next_address(KeychainKind::Internal) + .address + .script_pubkey(); + insert_confirmed_tx(&wallet, internal_funding_tx.clone(), 1); + let foreign_output = + TxOut { value: Amount::from_sat(90_000), script_pubkey: ScriptBuf::new().to_p2wsh() }; + let unsigned_tx = Transaction { + version: bitcoin::transaction::Version::TWO, + lock_time: LockTime::ZERO, + input: [ + OutPoint { txid: external_funding_tx.compute_txid(), vout: 0 }, + OutPoint { txid: internal_funding_tx.compute_txid(), vout: 0 }, + OutPoint { txid: Txid::from_byte_array([43; 32]), vout: 0 }, + ] + .into_iter() + .map(|previous_output| bitcoin::TxIn { previous_output, ..Default::default() }) + .collect(), + output: vec![TxOut { + value: Amount::from_sat(269_000), + script_pubkey: foreign_output.script_pubkey.clone(), + }], + }; + let mut psbt = Psbt::from_unsigned_tx(unsigned_tx).unwrap(); + psbt.inputs[0].witness_utxo = Some(external_funding_tx.output[0].clone()); + psbt.inputs[1].witness_utxo = Some(internal_funding_tx.output[0].clone()); + psbt.inputs[2].witness_utxo = Some(foreign_output); + (wallet, psbt) + } + #[tokio::test] async fn refill_publishes_addresses_only_after_their_reveal_is_persisted() { let fail_store = FailSwitchStore::new(); From 4611c67bfad6d2ab716d9bff842696d12a33c7aa Mon Sep 17 00:00:00 2001 From: Elias Rohrer Date: Tue, 20 Oct 2026 18:22:20 +0200 Subject: [PATCH 5/5] Test TRUC eligibility in funding estimates The maximum funding estimate relies on BDK's input selection. The BDK 3.2 TRUC filtering change needs coverage at this boundary. Check that unconfirmed v2 change remains usable, while v3 outputs only increase the available funding amount after confirmation. Co-Authored-By: HAL 9000 --- src/wallet/mod.rs | 38 ++++++++++++++++++++++++++++++++++++++ 1 file changed, 38 insertions(+) diff --git a/src/wallet/mod.rs b/src/wallet/mod.rs index d679843e77..edc0004d58 100644 --- a/src/wallet/mod.rs +++ b/src/wallet/mod.rs @@ -8170,6 +8170,44 @@ mod tests { .is_none()); } + #[tokio::test] + async fn max_funding_waits_for_truc_output_confirmation() { + let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new())); + let wallet = new_test_wallet(store, false).await; + let change_script = wallet + .inner + .lock() + .unwrap() + .reveal_next_address(KeychainKind::Internal) + .address + .script_pubkey(); + let fee_rate = FeeRate::from_sat_per_kwu(250); + + // Both outputs are trusted change, but our v2 funding transactions can only spend + // the v2 output while the v3 (TRUC) parent remains unconfirmed. + let mut regular_tx = wallet_paying_tx(&wallet, 42); + regular_tx.output[0].script_pubkey = change_script.clone(); + insert_unconfirmed_tx(&wallet, regular_tx); + let max_without_truc = wallet.get_max_funding_amount(0, fee_rate).unwrap(); + assert!(max_without_truc > 0); + + let mut truc_tx = wallet_paying_tx(&wallet, 43); + truc_tx.version = bitcoin::transaction::Version(3); + truc_tx.output[0].script_pubkey = change_script; + insert_unconfirmed_tx(&wallet, truc_tx.clone()); + assert_eq!( + wallet.get_max_funding_amount(0, fee_rate).unwrap(), + max_without_truc, + "unconfirmed TRUC outputs must not increase the v2 funding amount" + ); + + insert_confirmed_tx(&wallet, truc_tx, 1); + assert!( + wallet.get_max_funding_amount(0, fee_rate).unwrap() > max_without_truc, + "confirmed TRUC outputs must become available for funding" + ); + } + #[tokio::test] async fn max_funding_estimate_keeps_reserved_change_address_used() { let store: Arc = Arc::new(DynStoreWrapper(InMemoryStore::new()));