fix(key-wallet): let CoinSelector spend mempool UTXOs (#748)

* 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) <noreply@anthropic.com>

* fix(key-wallet): defer CoinSelector candidate filter to Utxo::is_spendable

`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) <noreply@anthropic.com>

* chore(dash-spv): integrations tests cleanup

---------

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: ZocoLini

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<RwLock<WalletManager<ManagedWalletInfo>>>,
+    wallet_id: &WalletId,
+    destination: &Address,
+    amount: u64,
+) -> Result<(dashcore::Transaction, u64), BuilderError> {
+    let dest_unchecked: Address<NetworkUnchecked> =
+        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;
+}

key-wallet/src/wallet/managed_wallet_info/coin_selection.rs

@@ -79,34 +79,17 @@ 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
     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 +142,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 +237,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