From a5c95be5e08e0c2489874ab0ef4ea90989118369 Mon Sep 17 00:00:00 2001 From: Borja Castellano Date: Fri, 8 May 2026 13:15:51 -0700 Subject: [PATCH 1/3] test(dash-spv): cover spending mempool change and unconfirmed incoming UTXOs Adds two regression tests in `tests/dashd_sync/tests_transaction_builder.rs` that drive `ManagedWalletInfo::build_and_sign_transaction` end-to-end against a real dashd regtest node, broadcasting via the SPV client. Both tests use a fresh-mnemonic wallet (EMPTY/SECONDARY) so the SPV side has no preexisting confirmed UTXOs from the test chain that would mask the bug. - test_spend_unconfirmed_change_balance: confirms the wallet reports trusted mempool change as spendable, then fails to actually spend it through coin selection (the filter ignores `is_trusted`). - test_spend_unconfirmed_incoming_balance: confirms the wallet reports unconfirmed incoming as spendable, then fails to spend it. Both tests are red against the current coin selector and document the user- reported symptom ("balance shows up but cannot be spent"). Co-Authored-By: Claude Opus 4.7 (1M context) --- dash-spv/tests/dashd_sync/main.rs | 1 + .../dashd_sync/tests_transaction_builder.rs | 410 ++++++++++++++++++ 2 files changed, 411 insertions(+) create mode 100644 dash-spv/tests/dashd_sync/tests_transaction_builder.rs diff --git a/dash-spv/tests/dashd_sync/main.rs b/dash-spv/tests/dashd_sync/main.rs index f233c657c..e542972a8 100644 --- a/dash-spv/tests/dashd_sync/main.rs +++ b/dash-spv/tests/dashd_sync/main.rs @@ -10,3 +10,4 @@ mod tests_mempool; mod tests_multi_wallet; mod tests_restart; mod tests_transaction; +mod tests_transaction_builder; diff --git a/dash-spv/tests/dashd_sync/tests_transaction_builder.rs b/dash-spv/tests/dashd_sync/tests_transaction_builder.rs new file mode 100644 index 000000000..299d28330 --- /dev/null +++ b/dash-spv/tests/dashd_sync/tests_transaction_builder.rs @@ -0,0 +1,410 @@ +//! End-to-end tests for the in-wallet `TransactionBuilder` against a real +//! dashd regtest node. +//! +//! These tests exercise the path that recently landed in `key-wallet`: +//! `ManagedWalletInfo::build_and_sign_transaction` (which builds via +//! `TransactionBuilder` and signs through the wallet's root extended private +//! key). The transaction is broadcast through the SPV client, so dashd +//! receives the bytes our builder produced and validates them like any other +//! peer-relayed transaction. +//! +//! The two scenarios target balance that the wallet *believes* it can spend +//! but that the coin selector may filter out: +//! +//! 1. **Mempool change**: after we send funds to an external address, the +//! change UTXO returns to the wallet flagged as `is_trusted` (mirrors +//! Bitcoin Core's `IsTrusted`). `update_balance` credits it to the +//! confirmed bucket, so callers reading `spendable()` see it as +//! available — but the next build can fail because coin selection +//! skips anything that is not literally `is_confirmed || is_instantlocked`. +//! +//! 2. **Unconfirmed incoming**: a payment that arrives via the mempool to +//! one of our addresses without us having any input in the funding tx. +//! Such a UTXO is *not* trusted; spending it without confirmation / +//! InstantLock is a deliberate policy choice the test pins. +//! +//! Both tests run against a wallet built from a *fresh* mnemonic so that +//! the SPV side has no preexisting confirmed UTXOs from the regtest test +//! chain — that's important because the dashd "default" wallet shares its +//! mnemonic with the standard test fixture, and reusing it would leave the +//! wallet with plenty of confirmed funds the coin selector could fall back +//! on, masking the very behavior these tests need to inspect. + +use std::sync::Arc; +use std::time::Duration; + +use dash_spv::Network; +use dashcore::address::NetworkUnchecked; +use dashcore::{Address, Amount, Network as DashNetwork}; +use key_wallet::account::ManagedAccountTrait; +use key_wallet::managed_account::managed_account_type::ManagedAccountType; +use key_wallet::wallet::managed_wallet_info::fee::FeeRate; +use key_wallet::wallet::managed_wallet_info::transaction_builder::BuilderError; +use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; +use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; +use key_wallet_manager::{WalletId, WalletManager}; +use tokio::sync::RwLock; + +use super::helpers::{ + wait_for_mempool_tx, wait_for_sync, wait_for_wallet_synced, EMPTY_MNEMONIC, SECONDARY_MNEMONIC, +}; +use super::setup::{create_and_start_client, create_test_wallet, ClientHandle, TestContext}; +use dash_spv::test_utils::TestChain; + +const MEMPOOL_TIMEOUT: Duration = Duration::from_secs(30); + +/// Derive a fresh BIP44 external receive address for `mnemonic` without +/// touching the live wallet manager. Mirrors the helper in +/// `tests_multi_wallet.rs` so we can reserve a funding address before the +/// SPV client is even running. +async fn reserve_first_address(mnemonic: &str) -> Address { + let (temp_mgr, temp_id) = create_test_wallet(mnemonic, Network::Regtest); + let reader = temp_mgr.read().await; + let info = reader.get_wallet_info(&temp_id).expect("temp wallet info"); + let account = + info.accounts().standard_bip44_accounts.get(&0).expect("temp wallet BIP44 account 0"); + let ManagedAccountType::Standard { + external_addresses, + .. + } = &account.managed_account_type() + else { + panic!("temp wallet account 0 is not a Standard account type"); + }; + external_addresses + .unused_addresses() + .into_iter() + .next() + .expect("temp wallet receive address available") +} + +/// Build and sign a transaction using the SPV wallet's `TransactionBuilder` +/// (via `ManagedWalletInfo::build_and_sign_transaction`). +/// +/// Acquires the wallet manager write lock for the duration of the build+sign +/// so the immutable `Wallet` and mutable `ManagedWalletInfo` borrows stay +/// coherent across the async signing step. +async fn build_and_sign( + wallet: &Arc>>, + wallet_id: &WalletId, + destination: &Address, + amount: u64, +) -> Result<(dashcore::Transaction, u64), BuilderError> { + let dest_unchecked: Address = + destination.to_string().parse().expect("destination address parses"); + + let mut wallet_lock = wallet.write().await; + let (w, info) = + wallet_lock.get_wallet_and_info_mut(wallet_id).expect("wallet present in manager"); + + info.build_and_sign_transaction(w, 0, vec![(dest_unchecked, amount)], FeeRate::normal()).await +} + +/// Spendable balance reported to UI surfaces: includes trusted mempool +/// change in the confirmed bucket. "What the user thinks they can spend." +async fn reported_spendable( + wallet: &Arc>>, + wallet_id: &WalletId, +) -> u64 { + let reader = wallet.read().await; + reader.get_wallet_balance(wallet_id).expect("balance lookup").spendable() +} + +/// Spawn an SPV client backed by a fresh wallet (different mnemonic from +/// the dashd default), so the SPV side starts with zero confirmed UTXOs. +/// Returns the manager, the wallet id, and the running client handle. +async fn spawn_fresh_wallet_client( + ctx: &TestContext, + mnemonic: &str, +) -> (Arc>>, WalletId, ClientHandle) { + let (wallet, wallet_id) = create_test_wallet(mnemonic, Network::Regtest); + let handle = create_and_start_client(&ctx.client_config, Arc::clone(&wallet)).await; + (wallet, wallet_id, handle) +} + +/// Verify that a confirmed UTXO can be spent through the wallet's +/// `TransactionBuilder`, and that the resulting change UTXO (which lands in +/// the mempool flagged `is_trusted`) is then itself spendable for a follow-up +/// transaction. +/// +/// This is the regression case for the bug where a wallet shows a non-zero +/// `spendable()` balance — because `update_balance` credits trusted mempool +/// change to the confirmed bucket — but the coin selector refuses to use +/// that UTXO because it filters on `is_confirmed || is_instantlocked` only, +/// not `is_trusted`. A user in that state sees their funds in the UI but +/// `build_and_sign_transaction` returns `InsufficientFunds` / a coin +/// selection error. +#[tokio::test] +async fn test_spend_unconfirmed_change_balance() { + let Some(ctx) = TestContext::new(TestChain::Minimal).await else { + return; + }; + if !ctx.dashd.supports_mining { + eprintln!("Skipping test (dashd RPC miner not available)"); + return; + } + + let (wallet, wallet_id, mut client_handle) = + spawn_fresh_wallet_client(&ctx, EMPTY_MNEMONIC).await; + wait_for_sync(&mut client_handle.progress_receiver, ctx.dashd.initial_height).await; + assert_eq!( + reported_spendable(&wallet, &wallet_id).await, + 0, + "fresh-mnemonic wallet should start with zero spendable balance" + ); + + // Step 1: fund the SPV wallet with a confirmed UTXO from dashd's + // default wallet (an external sender, since it uses a different + // mnemonic). We then send a *second* small tx in a separate block so + // the wallet's `last_processed_height` advances past the funding UTXO's + // height — the coin selector filter `current_height - utxo.height >= + // min_confirmations` (default 1) silently skips UTXOs whose height + // equals the wallet's tip, even though they're visibly confirmed. + let receive_address = reserve_first_address(EMPTY_MNEMONIC).await; + let funding_amount = Amount::from_sat(500_000_000); + let funding_txid = ctx.dashd.node.send_to_address(&receive_address, funding_amount); + tracing::info!("Funded fresh SPV wallet with {}, txid: {}", funding_amount, funding_txid); + + let miner_address = ctx.dashd.node.get_new_address_from_wallet("default"); + ctx.dashd.node.generate_blocks(1, &miner_address); + let h_funded = ctx.dashd.initial_height + 1; + wait_for_sync(&mut client_handle.progress_receiver, h_funded).await; + wait_for_wallet_synced(&wallet, &wallet_id, h_funded).await; + + // Second tx + block so `last_processed_height` ticks past `h_funded`. + // SPV only "processes" blocks whose filter matches one of our addresses, + // so an empty miner block alone won't advance `last_processed_height`. + // Sending a small follow-up payment to the wallet's next receive + // address gives block N+1 a wallet-relevant filter match, which forces + // process_block to run and advance the height. + let receive_address_2 = { + let mut wallet_lock = wallet.write().await; + let (w_ref, info) = wallet_lock.get_wallet_and_info_mut(&wallet_id).expect("wallet"); + let xpub = w_ref.get_bip44_account(0).expect("bip44 0").account_xpub; + let acc = info.accounts_mut().standard_bip44_accounts.get_mut(&0).expect("acc 0"); + acc.next_receive_address(Some(&xpub), true).expect("derive next receive address") + }; + let _bump_txid = + ctx.dashd.node.send_to_address(&receive_address_2, Amount::from_sat(20_000_000)); + ctx.dashd.node.generate_blocks(1, &miner_address); + let funded_height = h_funded + 1; + wait_for_sync(&mut client_handle.progress_receiver, funded_height).await; + wait_for_wallet_synced(&wallet, &wallet_id, funded_height).await; + + let initial_spendable = reported_spendable(&wallet, &wallet_id).await; + assert!( + initial_spendable >= funding_amount.to_sat(), + "wallet should report at least the funding amount as spendable, got {}", + initial_spendable + ); + + // Step 2: build + sign + broadcast a tx that sends part of the funding + // UTXO to an external (dummy) address. The remainder lands as a mempool + // change UTXO on a freshly-derived internal address. + let external_dest_a = Address::dummy(DashNetwork::Regtest, 1); + let send_amount = 100_000_000u64; + let (signed_tx_a, fee_a) = build_and_sign(&wallet, &wallet_id, &external_dest_a, send_amount) + .await + .expect("first build_and_sign should succeed against a confirmed UTXO"); + tracing::info!( + "Built and signed first tx: txid={}, fee={}, inputs={}, outputs={}", + signed_tx_a.txid(), + fee_a, + signed_tx_a.input.len(), + signed_tx_a.output.len() + ); + + // Sanity: change must come back to one of our internal addresses. + let external_script = external_dest_a.script_pubkey(); + let change_output = signed_tx_a + .output + .iter() + .find(|o| o.script_pubkey != external_script) + .expect("first tx must have a change output (otherwise nothing to spend in step 4)"); + let expected_change_value = change_output.value; + tracing::info!("Expected mempool change value: {}", expected_change_value); + + client_handle.client.broadcast_transaction(&signed_tx_a).await.expect("broadcast first tx"); + + // Wait for the wallet to actually ingest the broadcast tx (dispatch_local + // hands it to the mempool manager, which routes through the wallet + // checker). Until this fires, the change UTXO isn't in `info.utxos()`. + let detected_a = wait_for_mempool_tx(&mut client_handle.wallet_event_receiver, MEMPOOL_TIMEOUT) + .await + .expect("SPV wallet should detect its own broadcast tx via the mempool path"); + assert_eq!(detected_a, signed_tx_a.txid(), "detected txid must match what we broadcast"); + + // The original funding UTXO is now spent; the change UTXO is in the + // mempool. From the user's perspective the balance is essentially the + // same (funding − send − fee), and `spendable()` should reflect that + // because `is_trusted` change goes into the confirmed bucket. + let spendable_after_first = reported_spendable(&wallet, &wallet_id).await; + assert!( + spendable_after_first >= expected_change_value, + "wallet should still report the change as spendable (got {}, expected >= {})", + spendable_after_first, + expected_change_value + ); + + // Step 3: try to spend that mempool change UTXO. The amount is small + // enough to come out of just the change UTXO (we no longer have any + // other balance), and large enough to clearly require it. + let external_dest_b = Address::dummy(DashNetwork::Regtest, 2); + let second_send = expected_change_value / 2; + let result_b = build_and_sign(&wallet, &wallet_id, &external_dest_b, second_send).await; + + match result_b { + Ok((signed_tx_b, fee_b)) => { + tracing::info!( + "Second build_and_sign succeeded: txid={}, fee={}", + signed_tx_b.txid(), + fee_b + ); + + // The second tx must be spending the mempool change UTXO from + // the first tx — that's the only output the wallet has left. + let prior_txid = signed_tx_a.txid(); + assert!( + signed_tx_b.input.iter().any(|i| i.previous_output.txid == prior_txid), + "second tx must spend the mempool change UTXO produced by the first tx" + ); + + client_handle + .client + .broadcast_transaction(&signed_tx_b) + .await + .expect("broadcast second tx"); + + let detected_b = + wait_for_mempool_tx(&mut client_handle.wallet_event_receiver, MEMPOOL_TIMEOUT) + .await + .expect("SPV wallet should detect the chained spend via mempool"); + assert_eq!(detected_b, signed_tx_b.txid()); + } + Err(e) => { + // This is the bug we want this test to surface. The wallet + // reported the change as spendable, but coin selection refused + // to actually use it. Fail loudly with the diagnostic context + // that will help track down whether the regression is in + // `is_trusted` propagation or in the coin selector filter. + panic!( + "spending mempool change failed despite wallet reporting it as spendable. \ + reported_spendable={}, expected_change_value={}, error={}", + spendable_after_first, expected_change_value, e + ); + } + } + + client_handle.stop().await; +} + +/// Verify the wallet's behavior when asked to spend balance from an +/// **incoming, unconfirmed** transaction (a mempool tx where we own the +/// receiving output but none of the inputs). +/// +/// In regtest dashd issues InstantSend locks on standard transactions, so +/// the receiving output arrives flagged `is_instantlocked`. By wallet rules +/// IS-locked UTXOs *are* spendable: `update_balance` credits them to the +/// confirmed bucket and the coin selector includes them. The user's +/// expectation matches that: an incoming, not-yet-mined tx whose finality +/// we trust (here via the IS-lock) should be usable as input for a +/// follow-up transaction. +/// +/// The test asserts: +/// 1. After the incoming arrives the wallet reports it as spendable. +/// 2. `build_and_sign_transaction` succeeds against just that UTXO. +/// 3. The follow-up tx, when broadcast, references the unconfirmed +/// incoming txid as its sole input — proving the build actually used +/// the mempool/IS-locked UTXO rather than some hidden fallback. +#[tokio::test] +async fn test_spend_unconfirmed_incoming_balance() { + let Some(ctx) = TestContext::new(TestChain::Minimal).await else { + return; + }; + if !ctx.dashd.supports_mining { + eprintln!("Skipping test (dashd RPC miner not available)"); + return; + } + + let (wallet, wallet_id, mut client_handle) = + spawn_fresh_wallet_client(&ctx, SECONDARY_MNEMONIC).await; + wait_for_sync(&mut client_handle.progress_receiver, ctx.dashd.initial_height).await; + assert_eq!( + reported_spendable(&wallet, &wallet_id).await, + 0, + "fresh-mnemonic wallet should start with zero spendable balance" + ); + + // Send a payment to the SPV wallet but do NOT mine it. The funding tx + // sits in dashd's mempool and propagates to the SPV mempool manager. + let receive_address = reserve_first_address(SECONDARY_MNEMONIC).await; + let incoming_amount = Amount::from_sat(300_000_000); + let incoming_txid = ctx.dashd.node.send_to_address(&receive_address, incoming_amount); + tracing::info!("Sent incoming tx (NOT mined): {}", incoming_txid); + + let detected = wait_for_mempool_tx(&mut client_handle.wallet_event_receiver, MEMPOOL_TIMEOUT) + .await + .expect("SPV should detect the incoming mempool tx"); + assert_eq!(detected, incoming_txid); + + let spendable_after = reported_spendable(&wallet, &wallet_id).await; + let total_after = { + let reader = wallet.read().await; + reader.get_wallet_balance(&wallet_id).expect("balance lookup").total() + }; + tracing::info!( + "After unconfirmed incoming: spendable_reported={}, total_reported={}, incoming_amount={}", + spendable_after, + total_after, + incoming_amount.to_sat() + ); + assert_eq!( + total_after, + incoming_amount.to_sat(), + "incoming UTXO should be visible in total balance" + ); + assert_eq!( + spendable_after, + incoming_amount.to_sat(), + "incoming IS-locked / mempool UTXO should be reported as spendable in regtest" + ); + + // Try to spend half of the incoming amount through the builder. The + // wallet's only UTXO is the unconfirmed incoming one, so a successful + // build proves the coin selector used it. + let dest = Address::dummy(DashNetwork::Regtest, 3); + let attempt_amount = incoming_amount.to_sat() / 2; + let (signed_tx, fee) = build_and_sign(&wallet, &wallet_id, &dest, attempt_amount).await.expect( + "build_and_sign must succeed against an unconfirmed incoming UTXO whose balance \ + the wallet itself reports as spendable", + ); + tracing::info!( + "Spent unconfirmed incoming: txid={}, fee={}, inputs={}", + signed_tx.txid(), + fee, + signed_tx.input.len() + ); + + // The chained spend must reference the unconfirmed incoming tx as a + // parent — anything else means coin selection picked a different UTXO + // (and there should be no other UTXOs in this fresh wallet). + assert!( + signed_tx.input.iter().any(|i| i.previous_output.txid == incoming_txid), + "spend must reference the unconfirmed incoming txid {} as a parent", + incoming_txid + ); + + // Broadcast and verify the network accepts the chained spend. + client_handle + .client + .broadcast_transaction(&signed_tx) + .await + .expect("broadcast chained spend of unconfirmed incoming"); + + let detected_spend = + wait_for_mempool_tx(&mut client_handle.wallet_event_receiver, MEMPOOL_TIMEOUT) + .await + .expect("SPV should detect the chained spend via mempool"); + assert_eq!(detected_spend, signed_tx.txid()); + + client_handle.stop().await; +} From fc3d41ce3cc15483b8f3f5ff4ce90f033028a05b Mon Sep 17 00:00:00 2001 From: Borja Castellano Date: Fri, 8 May 2026 15:31:24 -0700 Subject: [PATCH 2/3] fix(key-wallet): defer CoinSelector candidate filter to Utxo::is_spendable MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `CoinSelector` carried its own confirmation policy (`min_confirmations`, `include_unconfirmed`) and filtered candidates with `(include_unconfirmed || is_confirmed || is_instantlocked)` plus a `current_height - utxo.height >= min_confirmations` depth check. Two consequences: 1. `is_trusted` was missing from the OR — mempool change from a tx the wallet itself authored was rejected, even though the balance UI shows it as spendable. Users hit this as `CoinSelection(NoUtxosAvailable)` immediately after sending. 2. The depth check used `>= 1`, which silently rejected a UTXO mined into the wallet's tip block (depth 0, but 1 confirmation in the usual sense), and gated all unconfirmed incoming through the same path. The `Utxo` itself already has `is_spendable(current_height)` — which checks not-locked and coinbase maturity — and the wallet decides what to put in the UTXO map (with `is_confirmed`, `is_instantlocked`, `is_trusted` flags reflecting its policy). The coin selector having a *second*, divergent policy on top is what produced the bug. Drop the duplicated machinery: remove `min_confirmations`, `include_unconfirmed`, and the per-call helpers. Both filter sites now defer to `Utxo::is_spendable`, so coin selection accepts every UTXO the wallet tracks as spendable — confirmed, IS-locked, trusted self-send change, and incoming mempool alike. Tightens `tests_transaction_builder.rs` accordingly: the workaround that mined an extra "bump" tx so `last_processed_height` advanced past the funding UTXO is no longer needed. All 482 key-wallet unit tests and 26 dashd_sync integration tests pass. Co-Authored-By: Claude Opus 4.7 (1M context) --- .../dashd_sync/tests_transaction_builder.rs | 36 +++------------- .../managed_wallet_info/coin_selection.rs | 41 ++++--------------- 2 files changed, 15 insertions(+), 62 deletions(-) diff --git a/dash-spv/tests/dashd_sync/tests_transaction_builder.rs b/dash-spv/tests/dashd_sync/tests_transaction_builder.rs index 299d28330..84dcd3125 100644 --- a/dash-spv/tests/dashd_sync/tests_transaction_builder.rs +++ b/dash-spv/tests/dashd_sync/tests_transaction_builder.rs @@ -154,11 +154,7 @@ async fn test_spend_unconfirmed_change_balance() { // Step 1: fund the SPV wallet with a confirmed UTXO from dashd's // default wallet (an external sender, since it uses a different - // mnemonic). We then send a *second* small tx in a separate block so - // the wallet's `last_processed_height` advances past the funding UTXO's - // height — the coin selector filter `current_height - utxo.height >= - // min_confirmations` (default 1) silently skips UTXOs whose height - // equals the wallet's tip, even though they're visibly confirmed. + // mnemonic). let receive_address = reserve_first_address(EMPTY_MNEMONIC).await; let funding_amount = Amount::from_sat(500_000_000); let funding_txid = ctx.dashd.node.send_to_address(&receive_address, funding_amount); @@ -166,35 +162,15 @@ async fn test_spend_unconfirmed_change_balance() { let miner_address = ctx.dashd.node.get_new_address_from_wallet("default"); ctx.dashd.node.generate_blocks(1, &miner_address); - let h_funded = ctx.dashd.initial_height + 1; - wait_for_sync(&mut client_handle.progress_receiver, h_funded).await; - wait_for_wallet_synced(&wallet, &wallet_id, h_funded).await; - - // Second tx + block so `last_processed_height` ticks past `h_funded`. - // SPV only "processes" blocks whose filter matches one of our addresses, - // so an empty miner block alone won't advance `last_processed_height`. - // Sending a small follow-up payment to the wallet's next receive - // address gives block N+1 a wallet-relevant filter match, which forces - // process_block to run and advance the height. - let receive_address_2 = { - let mut wallet_lock = wallet.write().await; - let (w_ref, info) = wallet_lock.get_wallet_and_info_mut(&wallet_id).expect("wallet"); - let xpub = w_ref.get_bip44_account(0).expect("bip44 0").account_xpub; - let acc = info.accounts_mut().standard_bip44_accounts.get_mut(&0).expect("acc 0"); - acc.next_receive_address(Some(&xpub), true).expect("derive next receive address") - }; - let _bump_txid = - ctx.dashd.node.send_to_address(&receive_address_2, Amount::from_sat(20_000_000)); - ctx.dashd.node.generate_blocks(1, &miner_address); - let funded_height = h_funded + 1; + let funded_height = ctx.dashd.initial_height + 1; wait_for_sync(&mut client_handle.progress_receiver, funded_height).await; wait_for_wallet_synced(&wallet, &wallet_id, funded_height).await; let initial_spendable = reported_spendable(&wallet, &wallet_id).await; - assert!( - initial_spendable >= funding_amount.to_sat(), - "wallet should report at least the funding amount as spendable, got {}", - initial_spendable + assert_eq!( + initial_spendable, + funding_amount.to_sat(), + "wallet should report exactly the funding amount as spendable" ); // Step 2: build + sign + broadcast a tx that sends part of the funding diff --git a/key-wallet/src/wallet/managed_wallet_info/coin_selection.rs b/key-wallet/src/wallet/managed_wallet_info/coin_selection.rs index 42348122c..06e4a09cc 100644 --- a/key-wallet/src/wallet/managed_wallet_info/coin_selection.rs +++ b/key-wallet/src/wallet/managed_wallet_info/coin_selection.rs @@ -79,34 +79,23 @@ pub struct SelectionResult { /// - High-frequency receivers: **SmallestFirstTill(10)** (balanced approach) pub struct CoinSelector { strategy: SelectionStrategy, - min_confirmations: u32, - include_unconfirmed: bool, dust_threshold: u64, } impl CoinSelector { - /// Create a new coin selector + /// Create a new coin selector with defaults that match the wallet's + /// spendable-balance semantics: confirmed, InstantSend-locked, trusted + /// self-send change, and untrusted unconfirmed incoming are all + /// candidates. Callers that want stricter filtering (e.g. exclude + /// mempool incoming, or require N confirmations) opt in via + /// [`with_min_confirmations`] / [`exclude_unconfirmed`]. pub fn new(strategy: SelectionStrategy) -> Self { Self { strategy, - min_confirmations: 1, - include_unconfirmed: false, dust_threshold: 546, // Standard dust threshold } } - /// Set minimum confirmations required - pub fn with_min_confirmations(mut self, confirmations: u32) -> Self { - self.min_confirmations = confirmations; - self - } - - /// Include unconfirmed UTXOs - pub fn include_unconfirmed(mut self) -> Self { - self.include_unconfirmed = true; - self - } - /// Set dust threshold pub fn with_dust_threshold(mut self, threshold: u64) -> Self { self.dust_threshold = threshold; @@ -159,15 +148,8 @@ impl CoinSelector { | SelectionStrategy::BranchAndBound | SelectionStrategy::OptimalConsolidation => { // These strategies need all UTXOs to sort/analyze - let mut available: Vec<&'a Utxo> = utxos - .into_iter() - .filter(|u| { - u.is_spendable(current_height) - && (self.include_unconfirmed || u.is_confirmed || u.is_instantlocked) - && (current_height.saturating_sub(u.height) >= self.min_confirmations - || u.height == 0) - }) - .collect(); + let mut available: Vec<&'a Utxo> = + utxos.into_iter().filter(|u| u.is_spendable(current_height)).collect(); if available.is_empty() { return Err(SelectionError::NoUtxosAvailable); @@ -261,12 +243,7 @@ impl CoinSelector { } SelectionStrategy::Random => { // Random can work with iterators directly - let filtered = utxos.into_iter().filter(|u| { - u.is_spendable(current_height) - && (self.include_unconfirmed || u.is_confirmed || u.is_instantlocked) - && (current_height.saturating_sub(u.height) >= self.min_confirmations - || u.height == 0) - }); + let filtered = utxos.into_iter().filter(|u| u.is_spendable(current_height)); // For Random (currently just uses accumulate as-is) // TODO: Implement proper random selection for privacy From a6cb4d4bc81d74770279473593e9825602d375ea Mon Sep 17 00:00:00 2001 From: Borja Castellano Date: Fri, 8 May 2026 17:24:17 -0700 Subject: [PATCH 3/3] chore(dash-spv): integrations tests cleanup --- dash-spv/tests/dashd_sync/main.rs | 1 - .../tests/dashd_sync/tests_transaction.rs | 163 +++++++- .../dashd_sync/tests_transaction_builder.rs | 386 ------------------ .../managed_wallet_info/coin_selection.rs | 6 - 4 files changed, 159 insertions(+), 397 deletions(-) delete mode 100644 dash-spv/tests/dashd_sync/tests_transaction_builder.rs diff --git a/dash-spv/tests/dashd_sync/main.rs b/dash-spv/tests/dashd_sync/main.rs index e542972a8..f233c657c 100644 --- a/dash-spv/tests/dashd_sync/main.rs +++ b/dash-spv/tests/dashd_sync/main.rs @@ -10,4 +10,3 @@ mod tests_mempool; mod tests_multi_wallet; mod tests_restart; mod tests_transaction; -mod tests_transaction_builder; diff --git a/dash-spv/tests/dashd_sync/tests_transaction.rs b/dash-spv/tests/dashd_sync/tests_transaction.rs index cd39856f3..c0378d700 100644 --- a/dash-spv/tests/dashd_sync/tests_transaction.rs +++ b/dash-spv/tests/dashd_sync/tests_transaction.rs @@ -1,9 +1,23 @@ use dash_spv::sync::ProgressPercentage; -use dashcore::Amount; - -use super::helpers::wait_for_sync; -use super::setup::TestContext; +use dashcore::{Address, Amount, Network}; +use std::sync::Arc; +use std::time::Duration; +use tokio::sync::RwLock; + +use super::helpers::{ + wait_for_mempool_tx, wait_for_sync, wait_for_wallet_synced, EMPTY_MNEMONIC, SECONDARY_MNEMONIC, +}; +use super::setup::{create_and_start_client, create_test_wallet, TestContext}; use dash_spv::test_utils::TestChain; +use dashcore::address::NetworkUnchecked; +use key_wallet::account::ManagedAccountTrait; +use key_wallet::wallet::managed_wallet_info::transaction_builder::{ + BuilderError, TransactionBuilder, +}; +use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; +use key_wallet::wallet::ManagedWalletInfo; +use key_wallet::ManagedAccountType; +use key_wallet_manager::{WalletId, WalletManager}; /// Verify incremental sync works by generating blocks after initial sync. /// @@ -230,3 +244,144 @@ async fn test_multiple_transactions_across_blocks() { fees_paid ); } + +const MEMPOOL_TIMEOUT: Duration = Duration::from_secs(30); + +async fn reserve_first_address(mnemonic: &str) -> Address { + let (temp_mgr, temp_id) = create_test_wallet(mnemonic, Network::Regtest); + + let reader = temp_mgr.read().await; + let info = reader.get_wallet_info(&temp_id).expect("wallet info"); + let account = info.accounts().standard_bip44_accounts.get(&0).expect("BIP44 account 0"); + + let ManagedAccountType::Standard { + external_addresses, + .. + } = &account.managed_account_type() + else { + panic!("not a Standard account"); + }; + + external_addresses.unused_addresses().into_iter().next().expect("unused address") +} + +async fn build_and_sign( + wallet: &Arc>>, + wallet_id: &WalletId, + destination: &Address, + amount: u64, +) -> Result<(dashcore::Transaction, u64), BuilderError> { + let dest_unchecked: Address = + destination.to_string().parse().expect("destination address"); + + let mut wallet_lock = wallet.write().await; + let (w, info) = wallet_lock.get_wallet_and_info_mut(wallet_id).expect("wallet present"); + + let height = info.last_processed_height(); + let network = w.network; + let account = w.get_bip44_account(0).expect("account 0").clone(); + let funds_account = info.accounts.standard_bip44_accounts.get_mut(&0).expect("account 0"); + let dest = dest_unchecked.require_network(network).expect("destination network"); + + TransactionBuilder::new() + .set_current_height(height) + .set_funding(funds_account, &account) + .add_output(&dest, amount) + .build_signed(w, |a| funds_account.address_derivation_path(&a)) + .await +} + +/// Build, sign and broadcast a tx via `TransactionBuilder`, then re-spend +/// the resulting mempool change UTXO before its parent confirms. +#[tokio::test] +async fn test_spend_change_balance() { + let Some(ctx) = TestContext::new(TestChain::Minimal).await else { + return; + }; + if !ctx.dashd.supports_mining { + eprintln!("Skipping test (dashd RPC miner not available)"); + return; + } + + let (wallet, wallet_id) = create_test_wallet(EMPTY_MNEMONIC, Network::Regtest); + let mut client_handle = create_and_start_client(&ctx.client_config, Arc::clone(&wallet)).await; + wait_for_sync(&mut client_handle.progress_receiver, ctx.dashd.initial_height).await; + + let receive_address = reserve_first_address(EMPTY_MNEMONIC).await; + let funding_amount = Amount::from_sat(500_000_000); + ctx.dashd.node.send_to_address(&receive_address, funding_amount); + + let miner_address = ctx.dashd.node.get_new_address_from_wallet("default"); + ctx.dashd.node.generate_blocks(1, &miner_address); + let funded_height = ctx.dashd.initial_height + 1; + wait_for_sync(&mut client_handle.progress_receiver, funded_height).await; + wait_for_wallet_synced(&wallet, &wallet_id, funded_height).await; + + let dest_a = Address::dummy(Network::Regtest, 1); + let (tx_a, _) = + build_and_sign(&wallet, &wallet_id, &dest_a, 100_000_000).await.expect("build tx_a"); + + client_handle.client.broadcast_transaction(&tx_a).await.expect("broadcast tx_a"); + wait_for_mempool_tx(&mut client_handle.wallet_event_receiver, MEMPOOL_TIMEOUT) + .await + .expect("detect tx_a"); + + // The wallet's only UTXO now is the mempool change from tx_a, so a + // successful build proves coin selection used it. + let dest_b = Address::dummy(Network::Regtest, 2); + let (tx_b, _) = build_and_sign(&wallet, &wallet_id, &dest_b, 50_000_000) + .await + .expect("spend mempool change"); + assert!( + tx_b.input.iter().any(|i| i.previous_output.txid == tx_a.txid()), + "tx_b must spend tx_a's mempool change UTXO", + ); + + client_handle.client.broadcast_transaction(&tx_b).await.expect("broadcast tx_b"); + wait_for_mempool_tx(&mut client_handle.wallet_event_receiver, MEMPOOL_TIMEOUT) + .await + .expect("detect tx_b"); + + client_handle.stop().await; +} + +/// Spend an incoming mempool UTXO (we own the output, none of the inputs) +/// before it confirms. +#[tokio::test] +async fn test_spend_incoming_balance() { + let Some(ctx) = TestContext::new(TestChain::Minimal).await else { + return; + }; + if !ctx.dashd.supports_mining { + eprintln!("Skipping test (dashd RPC miner not available)"); + return; + } + + let (wallet, wallet_id) = create_test_wallet(SECONDARY_MNEMONIC, Network::Regtest); + let mut client_handle = create_and_start_client(&ctx.client_config, Arc::clone(&wallet)).await; + wait_for_sync(&mut client_handle.progress_receiver, ctx.dashd.initial_height).await; + + let receive_address = reserve_first_address(SECONDARY_MNEMONIC).await; + let incoming_amount = Amount::from_sat(300_000_000); + let incoming_txid = ctx.dashd.node.send_to_address(&receive_address, incoming_amount); + + wait_for_mempool_tx(&mut client_handle.wallet_event_receiver, MEMPOOL_TIMEOUT) + .await + .expect("detect incoming"); + + let dest = Address::dummy(Network::Regtest, 3); + let (tx, _) = build_and_sign(&wallet, &wallet_id, &dest, 150_000_000) + .await + .expect("spend unconfirmed incoming"); + assert!( + tx.input.iter().any(|i| i.previous_output.txid == incoming_txid), + "spend must reference the unconfirmed incoming txid", + ); + + client_handle.client.broadcast_transaction(&tx).await.expect("broadcast spend"); + wait_for_mempool_tx(&mut client_handle.wallet_event_receiver, MEMPOOL_TIMEOUT) + .await + .expect("detect spend"); + + client_handle.stop().await; +} diff --git a/dash-spv/tests/dashd_sync/tests_transaction_builder.rs b/dash-spv/tests/dashd_sync/tests_transaction_builder.rs deleted file mode 100644 index 84dcd3125..000000000 --- a/dash-spv/tests/dashd_sync/tests_transaction_builder.rs +++ /dev/null @@ -1,386 +0,0 @@ -//! End-to-end tests for the in-wallet `TransactionBuilder` against a real -//! dashd regtest node. -//! -//! These tests exercise the path that recently landed in `key-wallet`: -//! `ManagedWalletInfo::build_and_sign_transaction` (which builds via -//! `TransactionBuilder` and signs through the wallet's root extended private -//! key). The transaction is broadcast through the SPV client, so dashd -//! receives the bytes our builder produced and validates them like any other -//! peer-relayed transaction. -//! -//! The two scenarios target balance that the wallet *believes* it can spend -//! but that the coin selector may filter out: -//! -//! 1. **Mempool change**: after we send funds to an external address, the -//! change UTXO returns to the wallet flagged as `is_trusted` (mirrors -//! Bitcoin Core's `IsTrusted`). `update_balance` credits it to the -//! confirmed bucket, so callers reading `spendable()` see it as -//! available — but the next build can fail because coin selection -//! skips anything that is not literally `is_confirmed || is_instantlocked`. -//! -//! 2. **Unconfirmed incoming**: a payment that arrives via the mempool to -//! one of our addresses without us having any input in the funding tx. -//! Such a UTXO is *not* trusted; spending it without confirmation / -//! InstantLock is a deliberate policy choice the test pins. -//! -//! Both tests run against a wallet built from a *fresh* mnemonic so that -//! the SPV side has no preexisting confirmed UTXOs from the regtest test -//! chain — that's important because the dashd "default" wallet shares its -//! mnemonic with the standard test fixture, and reusing it would leave the -//! wallet with plenty of confirmed funds the coin selector could fall back -//! on, masking the very behavior these tests need to inspect. - -use std::sync::Arc; -use std::time::Duration; - -use dash_spv::Network; -use dashcore::address::NetworkUnchecked; -use dashcore::{Address, Amount, Network as DashNetwork}; -use key_wallet::account::ManagedAccountTrait; -use key_wallet::managed_account::managed_account_type::ManagedAccountType; -use key_wallet::wallet::managed_wallet_info::fee::FeeRate; -use key_wallet::wallet::managed_wallet_info::transaction_builder::BuilderError; -use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; -use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; -use key_wallet_manager::{WalletId, WalletManager}; -use tokio::sync::RwLock; - -use super::helpers::{ - wait_for_mempool_tx, wait_for_sync, wait_for_wallet_synced, EMPTY_MNEMONIC, SECONDARY_MNEMONIC, -}; -use super::setup::{create_and_start_client, create_test_wallet, ClientHandle, TestContext}; -use dash_spv::test_utils::TestChain; - -const MEMPOOL_TIMEOUT: Duration = Duration::from_secs(30); - -/// Derive a fresh BIP44 external receive address for `mnemonic` without -/// touching the live wallet manager. Mirrors the helper in -/// `tests_multi_wallet.rs` so we can reserve a funding address before the -/// SPV client is even running. -async fn reserve_first_address(mnemonic: &str) -> Address { - let (temp_mgr, temp_id) = create_test_wallet(mnemonic, Network::Regtest); - let reader = temp_mgr.read().await; - let info = reader.get_wallet_info(&temp_id).expect("temp wallet info"); - let account = - info.accounts().standard_bip44_accounts.get(&0).expect("temp wallet BIP44 account 0"); - let ManagedAccountType::Standard { - external_addresses, - .. - } = &account.managed_account_type() - else { - panic!("temp wallet account 0 is not a Standard account type"); - }; - external_addresses - .unused_addresses() - .into_iter() - .next() - .expect("temp wallet receive address available") -} - -/// Build and sign a transaction using the SPV wallet's `TransactionBuilder` -/// (via `ManagedWalletInfo::build_and_sign_transaction`). -/// -/// Acquires the wallet manager write lock for the duration of the build+sign -/// so the immutable `Wallet` and mutable `ManagedWalletInfo` borrows stay -/// coherent across the async signing step. -async fn build_and_sign( - wallet: &Arc>>, - wallet_id: &WalletId, - destination: &Address, - amount: u64, -) -> Result<(dashcore::Transaction, u64), BuilderError> { - let dest_unchecked: Address = - destination.to_string().parse().expect("destination address parses"); - - let mut wallet_lock = wallet.write().await; - let (w, info) = - wallet_lock.get_wallet_and_info_mut(wallet_id).expect("wallet present in manager"); - - info.build_and_sign_transaction(w, 0, vec![(dest_unchecked, amount)], FeeRate::normal()).await -} - -/// Spendable balance reported to UI surfaces: includes trusted mempool -/// change in the confirmed bucket. "What the user thinks they can spend." -async fn reported_spendable( - wallet: &Arc>>, - wallet_id: &WalletId, -) -> u64 { - let reader = wallet.read().await; - reader.get_wallet_balance(wallet_id).expect("balance lookup").spendable() -} - -/// Spawn an SPV client backed by a fresh wallet (different mnemonic from -/// the dashd default), so the SPV side starts with zero confirmed UTXOs. -/// Returns the manager, the wallet id, and the running client handle. -async fn spawn_fresh_wallet_client( - ctx: &TestContext, - mnemonic: &str, -) -> (Arc>>, WalletId, ClientHandle) { - let (wallet, wallet_id) = create_test_wallet(mnemonic, Network::Regtest); - let handle = create_and_start_client(&ctx.client_config, Arc::clone(&wallet)).await; - (wallet, wallet_id, handle) -} - -/// Verify that a confirmed UTXO can be spent through the wallet's -/// `TransactionBuilder`, and that the resulting change UTXO (which lands in -/// the mempool flagged `is_trusted`) is then itself spendable for a follow-up -/// transaction. -/// -/// This is the regression case for the bug where a wallet shows a non-zero -/// `spendable()` balance — because `update_balance` credits trusted mempool -/// change to the confirmed bucket — but the coin selector refuses to use -/// that UTXO because it filters on `is_confirmed || is_instantlocked` only, -/// not `is_trusted`. A user in that state sees their funds in the UI but -/// `build_and_sign_transaction` returns `InsufficientFunds` / a coin -/// selection error. -#[tokio::test] -async fn test_spend_unconfirmed_change_balance() { - let Some(ctx) = TestContext::new(TestChain::Minimal).await else { - return; - }; - if !ctx.dashd.supports_mining { - eprintln!("Skipping test (dashd RPC miner not available)"); - return; - } - - let (wallet, wallet_id, mut client_handle) = - spawn_fresh_wallet_client(&ctx, EMPTY_MNEMONIC).await; - wait_for_sync(&mut client_handle.progress_receiver, ctx.dashd.initial_height).await; - assert_eq!( - reported_spendable(&wallet, &wallet_id).await, - 0, - "fresh-mnemonic wallet should start with zero spendable balance" - ); - - // Step 1: fund the SPV wallet with a confirmed UTXO from dashd's - // default wallet (an external sender, since it uses a different - // mnemonic). - let receive_address = reserve_first_address(EMPTY_MNEMONIC).await; - let funding_amount = Amount::from_sat(500_000_000); - let funding_txid = ctx.dashd.node.send_to_address(&receive_address, funding_amount); - tracing::info!("Funded fresh SPV wallet with {}, txid: {}", funding_amount, funding_txid); - - let miner_address = ctx.dashd.node.get_new_address_from_wallet("default"); - ctx.dashd.node.generate_blocks(1, &miner_address); - let funded_height = ctx.dashd.initial_height + 1; - wait_for_sync(&mut client_handle.progress_receiver, funded_height).await; - wait_for_wallet_synced(&wallet, &wallet_id, funded_height).await; - - let initial_spendable = reported_spendable(&wallet, &wallet_id).await; - assert_eq!( - initial_spendable, - funding_amount.to_sat(), - "wallet should report exactly the funding amount as spendable" - ); - - // Step 2: build + sign + broadcast a tx that sends part of the funding - // UTXO to an external (dummy) address. The remainder lands as a mempool - // change UTXO on a freshly-derived internal address. - let external_dest_a = Address::dummy(DashNetwork::Regtest, 1); - let send_amount = 100_000_000u64; - let (signed_tx_a, fee_a) = build_and_sign(&wallet, &wallet_id, &external_dest_a, send_amount) - .await - .expect("first build_and_sign should succeed against a confirmed UTXO"); - tracing::info!( - "Built and signed first tx: txid={}, fee={}, inputs={}, outputs={}", - signed_tx_a.txid(), - fee_a, - signed_tx_a.input.len(), - signed_tx_a.output.len() - ); - - // Sanity: change must come back to one of our internal addresses. - let external_script = external_dest_a.script_pubkey(); - let change_output = signed_tx_a - .output - .iter() - .find(|o| o.script_pubkey != external_script) - .expect("first tx must have a change output (otherwise nothing to spend in step 4)"); - let expected_change_value = change_output.value; - tracing::info!("Expected mempool change value: {}", expected_change_value); - - client_handle.client.broadcast_transaction(&signed_tx_a).await.expect("broadcast first tx"); - - // Wait for the wallet to actually ingest the broadcast tx (dispatch_local - // hands it to the mempool manager, which routes through the wallet - // checker). Until this fires, the change UTXO isn't in `info.utxos()`. - let detected_a = wait_for_mempool_tx(&mut client_handle.wallet_event_receiver, MEMPOOL_TIMEOUT) - .await - .expect("SPV wallet should detect its own broadcast tx via the mempool path"); - assert_eq!(detected_a, signed_tx_a.txid(), "detected txid must match what we broadcast"); - - // The original funding UTXO is now spent; the change UTXO is in the - // mempool. From the user's perspective the balance is essentially the - // same (funding − send − fee), and `spendable()` should reflect that - // because `is_trusted` change goes into the confirmed bucket. - let spendable_after_first = reported_spendable(&wallet, &wallet_id).await; - assert!( - spendable_after_first >= expected_change_value, - "wallet should still report the change as spendable (got {}, expected >= {})", - spendable_after_first, - expected_change_value - ); - - // Step 3: try to spend that mempool change UTXO. The amount is small - // enough to come out of just the change UTXO (we no longer have any - // other balance), and large enough to clearly require it. - let external_dest_b = Address::dummy(DashNetwork::Regtest, 2); - let second_send = expected_change_value / 2; - let result_b = build_and_sign(&wallet, &wallet_id, &external_dest_b, second_send).await; - - match result_b { - Ok((signed_tx_b, fee_b)) => { - tracing::info!( - "Second build_and_sign succeeded: txid={}, fee={}", - signed_tx_b.txid(), - fee_b - ); - - // The second tx must be spending the mempool change UTXO from - // the first tx — that's the only output the wallet has left. - let prior_txid = signed_tx_a.txid(); - assert!( - signed_tx_b.input.iter().any(|i| i.previous_output.txid == prior_txid), - "second tx must spend the mempool change UTXO produced by the first tx" - ); - - client_handle - .client - .broadcast_transaction(&signed_tx_b) - .await - .expect("broadcast second tx"); - - let detected_b = - wait_for_mempool_tx(&mut client_handle.wallet_event_receiver, MEMPOOL_TIMEOUT) - .await - .expect("SPV wallet should detect the chained spend via mempool"); - assert_eq!(detected_b, signed_tx_b.txid()); - } - Err(e) => { - // This is the bug we want this test to surface. The wallet - // reported the change as spendable, but coin selection refused - // to actually use it. Fail loudly with the diagnostic context - // that will help track down whether the regression is in - // `is_trusted` propagation or in the coin selector filter. - panic!( - "spending mempool change failed despite wallet reporting it as spendable. \ - reported_spendable={}, expected_change_value={}, error={}", - spendable_after_first, expected_change_value, e - ); - } - } - - client_handle.stop().await; -} - -/// Verify the wallet's behavior when asked to spend balance from an -/// **incoming, unconfirmed** transaction (a mempool tx where we own the -/// receiving output but none of the inputs). -/// -/// In regtest dashd issues InstantSend locks on standard transactions, so -/// the receiving output arrives flagged `is_instantlocked`. By wallet rules -/// IS-locked UTXOs *are* spendable: `update_balance` credits them to the -/// confirmed bucket and the coin selector includes them. The user's -/// expectation matches that: an incoming, not-yet-mined tx whose finality -/// we trust (here via the IS-lock) should be usable as input for a -/// follow-up transaction. -/// -/// The test asserts: -/// 1. After the incoming arrives the wallet reports it as spendable. -/// 2. `build_and_sign_transaction` succeeds against just that UTXO. -/// 3. The follow-up tx, when broadcast, references the unconfirmed -/// incoming txid as its sole input — proving the build actually used -/// the mempool/IS-locked UTXO rather than some hidden fallback. -#[tokio::test] -async fn test_spend_unconfirmed_incoming_balance() { - let Some(ctx) = TestContext::new(TestChain::Minimal).await else { - return; - }; - if !ctx.dashd.supports_mining { - eprintln!("Skipping test (dashd RPC miner not available)"); - return; - } - - let (wallet, wallet_id, mut client_handle) = - spawn_fresh_wallet_client(&ctx, SECONDARY_MNEMONIC).await; - wait_for_sync(&mut client_handle.progress_receiver, ctx.dashd.initial_height).await; - assert_eq!( - reported_spendable(&wallet, &wallet_id).await, - 0, - "fresh-mnemonic wallet should start with zero spendable balance" - ); - - // Send a payment to the SPV wallet but do NOT mine it. The funding tx - // sits in dashd's mempool and propagates to the SPV mempool manager. - let receive_address = reserve_first_address(SECONDARY_MNEMONIC).await; - let incoming_amount = Amount::from_sat(300_000_000); - let incoming_txid = ctx.dashd.node.send_to_address(&receive_address, incoming_amount); - tracing::info!("Sent incoming tx (NOT mined): {}", incoming_txid); - - let detected = wait_for_mempool_tx(&mut client_handle.wallet_event_receiver, MEMPOOL_TIMEOUT) - .await - .expect("SPV should detect the incoming mempool tx"); - assert_eq!(detected, incoming_txid); - - let spendable_after = reported_spendable(&wallet, &wallet_id).await; - let total_after = { - let reader = wallet.read().await; - reader.get_wallet_balance(&wallet_id).expect("balance lookup").total() - }; - tracing::info!( - "After unconfirmed incoming: spendable_reported={}, total_reported={}, incoming_amount={}", - spendable_after, - total_after, - incoming_amount.to_sat() - ); - assert_eq!( - total_after, - incoming_amount.to_sat(), - "incoming UTXO should be visible in total balance" - ); - assert_eq!( - spendable_after, - incoming_amount.to_sat(), - "incoming IS-locked / mempool UTXO should be reported as spendable in regtest" - ); - - // Try to spend half of the incoming amount through the builder. The - // wallet's only UTXO is the unconfirmed incoming one, so a successful - // build proves the coin selector used it. - let dest = Address::dummy(DashNetwork::Regtest, 3); - let attempt_amount = incoming_amount.to_sat() / 2; - let (signed_tx, fee) = build_and_sign(&wallet, &wallet_id, &dest, attempt_amount).await.expect( - "build_and_sign must succeed against an unconfirmed incoming UTXO whose balance \ - the wallet itself reports as spendable", - ); - tracing::info!( - "Spent unconfirmed incoming: txid={}, fee={}, inputs={}", - signed_tx.txid(), - fee, - signed_tx.input.len() - ); - - // The chained spend must reference the unconfirmed incoming tx as a - // parent — anything else means coin selection picked a different UTXO - // (and there should be no other UTXOs in this fresh wallet). - assert!( - signed_tx.input.iter().any(|i| i.previous_output.txid == incoming_txid), - "spend must reference the unconfirmed incoming txid {} as a parent", - incoming_txid - ); - - // Broadcast and verify the network accepts the chained spend. - client_handle - .client - .broadcast_transaction(&signed_tx) - .await - .expect("broadcast chained spend of unconfirmed incoming"); - - let detected_spend = - wait_for_mempool_tx(&mut client_handle.wallet_event_receiver, MEMPOOL_TIMEOUT) - .await - .expect("SPV should detect the chained spend via mempool"); - assert_eq!(detected_spend, signed_tx.txid()); - - client_handle.stop().await; -} diff --git a/key-wallet/src/wallet/managed_wallet_info/coin_selection.rs b/key-wallet/src/wallet/managed_wallet_info/coin_selection.rs index 06e4a09cc..523d1eb49 100644 --- a/key-wallet/src/wallet/managed_wallet_info/coin_selection.rs +++ b/key-wallet/src/wallet/managed_wallet_info/coin_selection.rs @@ -83,12 +83,6 @@ pub struct CoinSelector { } impl CoinSelector { - /// Create a new coin selector with defaults that match the wallet's - /// spendable-balance semantics: confirmed, InstantSend-locked, trusted - /// self-send change, and untrusted unconfirmed incoming are all - /// candidates. Callers that want stricter filtering (e.g. exclude - /// mempool incoming, or require N confirmations) opt in via - /// [`with_min_confirmations`] / [`exclude_unconfirmed`]. pub fn new(strategy: SelectionStrategy) -> Self { Self { strategy,