diff --git a/dash-spv-ffi/src/bin/ffi_cli.rs b/dash-spv-ffi/src/bin/ffi_cli.rs index 38aeb8098..198fc8975 100644 --- a/dash-spv-ffi/src/bin/ffi_cli.rs +++ b/dash-spv-ffi/src/bin/ffi_cli.rs @@ -226,6 +226,34 @@ extern "C" fn on_transaction_detected( ); } +extern "C" fn on_transactions_swept( + wallet_id: *const c_char, + txids: *const [u8; 32], + txids_count: usize, + superseded_by: *const [u8; 32], + balance: *const FFIBalance, + _account_balances: *const dash_spv_ffi::FFIAccountBalance, + _account_balances_count: u32, + _user_data: *mut c_void, +) { + let wallet_short = short_wallet(wallet_id); + if txids.is_null() || superseded_by.is_null() { + println!("[Wallet] TXs swept: wallet={}..., null payload", wallet_short); + return; + } + let list = unsafe { std::slice::from_raw_parts(txids, txids_count) }; + let winner = unsafe { &*superseded_by }; + let b = read_balance(balance); + println!( + "[Wallet] TXs swept: wallet={}..., removed=[{}], superseded_by={}, balance[confirmed={}, unconfirmed={}]", + wallet_short, + list.iter().map(hex::encode).collect::>().join(","), + hex::encode(winner), + b.confirmed, + b.unconfirmed, + ); +} + extern "C" fn on_transaction_instant_locked( wallet_id: *const c_char, txid: *const [u8; 32], @@ -548,6 +576,7 @@ fn main() { wallet: FFIWalletEventCallbacks { on_transaction_detected: Some(on_transaction_detected), on_transaction_instant_locked: Some(on_transaction_instant_locked), + on_transactions_swept: Some(on_transactions_swept), on_block_processed: Some(on_wallet_block_processed), on_sync_height_advanced: Some(on_sync_height_advanced), on_chain_lock_processed: Some(on_wallet_chain_lock_processed), diff --git a/dash-spv-ffi/src/callbacks.rs b/dash-spv-ffi/src/callbacks.rs index c9fc45ff5..921f99041 100644 --- a/dash-spv-ffi/src/callbacks.rs +++ b/dash-spv-ffi/src/callbacks.rs @@ -752,6 +752,37 @@ pub type OnTransactionDetectedCallback = Option< ), >; +/// Callback for `WalletEvent::TransactionsSwept`. +/// +/// Fires when the wallet removes transactions that a later, final transaction +/// provably beat to one of their inputs: they can never confirm, so their +/// outputs are gone from the UTXO set and their records deleted. +/// +/// **The only removal-shaped wallet callback.** Every other one is additive, +/// so a consumer mirroring wallet state to disk must act on this — delete the +/// named transactions and any UTXO they created. Ignoring it leaves the dead +/// rows in the mirror, which replays them on the next load and re-creates a +/// balance the wallet has already corrected. +/// +/// `txids` points to `txids_count` consecutive 32-byte txids. +/// `superseded_by` is the transaction whose arrival settled the inputs. +/// All pointer parameters are borrowed and only valid for the duration of the +/// callback. `balance` is the wallet's balance *after* the removal; +/// `account_balances` follows the same contract as on +/// [`OnTransactionDetectedCallback`]. +pub type OnTransactionsSweptCallback = Option< + extern "C" fn( + wallet_id: *const c_char, + txids: *const [u8; 32], + txids_count: usize, + superseded_by: *const [u8; 32], + balance: *const FFIBalance, + account_balances: *const FFIAccountBalance, + account_balances_count: u32, + user_data: *mut c_void, + ), +>; + /// Callback for `WalletEvent::TransactionInstantLocked`. /// /// Fires when an InstantSend lock is applied to a previously-seen off-chain @@ -909,6 +940,7 @@ pub type OnWalletChainLockProcessedCallback = Option< pub struct FFIWalletEventCallbacks { pub on_transaction_detected: OnTransactionDetectedCallback, pub on_transaction_instant_locked: OnTransactionInstantLockedCallback, + pub on_transactions_swept: OnTransactionsSweptCallback, pub on_block_processed: OnWalletBlockProcessedCallback, pub on_sync_height_advanced: OnSyncHeightAdvancedCallback, pub on_chain_lock_processed: OnWalletChainLockProcessedCallback, @@ -924,6 +956,7 @@ impl Default for FFIWalletEventCallbacks { Self { on_transaction_detected: None, on_transaction_instant_locked: None, + on_transactions_swept: None, on_block_processed: None, on_sync_height_advanced: None, on_chain_lock_processed: None, @@ -1021,6 +1054,52 @@ impl FFIWalletEventCallbacks { /// Dispatch a WalletEvent to the appropriate callback. pub fn dispatch(&self, event: &WalletEvent) { match event { + WalletEvent::TransactionsSwept { + wallet_id, + txids, + superseded_by, + balance, + account_balances, + } => { + if let Some(cb) = self.on_transactions_swept { + let wallet_id_hex = hex::encode(wallet_id); + let c_wallet_id = CString::new(wallet_id_hex).unwrap_or_default(); + let raw_txids: Vec<[u8; 32]> = + txids.iter().map(|t| t.to_byte_array()).collect(); + let raw_superseded_by = superseded_by.to_byte_array(); + let ffi_balance = FFIBalance::from(*balance); + let ffi_account_balances = FFIAccountBalance::from_map(account_balances); + let account_balances_ptr = if ffi_account_balances.is_empty() { + ptr::null() + } else { + ffi_account_balances.as_ptr() + }; + + cb( + c_wallet_id.as_ptr(), + raw_txids.as_ptr(), + raw_txids.len(), + &raw_superseded_by as *const [u8; 32], + &ffi_balance as *const FFIBalance, + account_balances_ptr, + ffi_account_balances.len() as u32, + self.user_data, + ); + + drop(ffi_account_balances); + } else { + // Deliberately loud: every other wallet callback is + // additive, so a consumer that leaves this one unset keeps + // transactions the wallet has already dropped. + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + swept = txids.len(), + %superseded_by, + "no on_transactions_swept callback set; the consumer will keep \ + mirroring transactions the wallet removed" + ); + } + } WalletEvent::TransactionDetected { wallet_id, record, diff --git a/dash-spv-ffi/tests/dashd_sync/callbacks.rs b/dash-spv-ffi/tests/dashd_sync/callbacks.rs index c5335dead..c9e58b108 100644 --- a/dash-spv-ffi/tests/dashd_sync/callbacks.rs +++ b/dash-spv-ffi/tests/dashd_sync/callbacks.rs @@ -622,6 +622,8 @@ pub(super) fn create_wallet_callbacks(tracker: &Arc) -> FFIWall on_block_processed: Some(on_wallet_block_processed), on_sync_height_advanced: Some(on_sync_height_advanced), on_chain_lock_processed: None, + // Not exercised by these tests: they never build a conflicting spend. + on_transactions_swept: None, user_data: Arc::as_ptr(tracker) as *mut c_void, } } diff --git a/key-wallet-manager/src/events.rs b/key-wallet-manager/src/events.rs index 77a5af1e7..803f3984e 100644 --- a/key-wallet-manager/src/events.rs +++ b/key-wallet-manager/src/events.rs @@ -222,6 +222,29 @@ pub enum WalletEvent { /// full balance after the change — not a delta. account_balances: BTreeMap, }, + /// Transactions were removed from the wallet: each was a recorded spend + /// that a later, final transaction provably beat to one of its inputs, so + /// it can never confirm. Their outputs are gone from the UTXO set and + /// their records deleted. + /// + /// The only removal-shaped event on this bus. A consumer mirroring wallet + /// state to disk must act on it — every other variant is additive, so + /// without this the mirror keeps the dead rows and replays them on the + /// next load, re-creating a balance the wallet has already corrected. + TransactionsSwept { + /// ID of the affected wallet. + wallet_id: WalletId, + /// Transactions removed. Delete these rows and any UTXO they created. + txids: Vec, + /// The transaction whose arrival settled the inputs, for provenance. + superseded_by: Txid, + /// Wallet balance after the removal. + balance: WalletCoreBalance, + /// Post-event balance **snapshots** for accounts whose balance + /// changed as a result of this event. Each value is the account's + /// full balance after the change — not a delta. + account_balances: BTreeMap, + }, /// A block was processed for a wallet. Carries records bucketed by what /// happened to them in this block, plus the post-block balance. /// `inserted` is records first stored in this block, `updated` is @@ -332,6 +355,10 @@ impl WalletEvent { wallet_id, .. } + | WalletEvent::TransactionsSwept { + wallet_id, + .. + } | WalletEvent::TransactionInstantLocked { wallet_id, .. @@ -382,6 +409,20 @@ impl fmt::Display for WalletEvent { balance, format_account_balances(account_balances), ), + WalletEvent::TransactionsSwept { + txids, + superseded_by, + balance, + account_balances, + .. + } => write!( + f, + "TransactionsSwept(count={}, superseded_by={}, balance={}, account_balances={})", + txids.len(), + superseded_by, + balance, + format_account_balances(account_balances), + ), WalletEvent::BlockProcessed { height, chain_lock, diff --git a/key-wallet-manager/src/lib.rs b/key-wallet-manager/src/lib.rs index 89002d0ac..13e7b1c2b 100644 --- a/key-wallet-manager/src/lib.rs +++ b/key-wallet-manager/src/lib.rs @@ -24,14 +24,16 @@ pub use events::{DerivedAddress, WalletEvent}; pub use matching::{check_compact_filters_for_elements, FilterMatchKey}; pub use wallet_interface::{BlockProcessingResult, MempoolTransactionResult, WalletInterface}; -use dashcore::blockdata::transaction::Transaction; +use dashcore::blockdata::transaction::{OutPoint, Transaction}; use dashcore::prelude::CoreBlockHeight; +use dashcore::Txid; use key_wallet::account::AccountCollection; use key_wallet::managed_account::transaction_record::TransactionRecord; use key_wallet::transaction_checking::{DerivedAddressInfo, TransactionContext}; use key_wallet::wallet::managed_wallet_info::coin_selection::SelectionStrategy; use key_wallet::wallet::managed_wallet_info::transaction_building::AccountTypePreference; use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; +use key_wallet::wallet::managed_wallet_info::AbandonOutcome; use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; use key_wallet::{AccountType, Address, ExtendedPrivKey, Mnemonic, Network, Wallet}; use key_wallet::{ExtendedPubKey, WalletCoreBalance}; @@ -94,6 +96,12 @@ pub struct CheckTransactionsResult { /// Records whose state was updated by this check (confirmation or /// InstantSend lock on a previously stored record), grouped by wallet. pub per_wallet_updated_records: BTreeMap>, + /// Transactions this check *removed*, grouped by wallet: recorded spends + /// the arriving transaction provably beat to one of its inputs. See + /// [`crate::events::WalletEvent::TransactionsSwept`] + /// — a consumer mirroring wallet state must delete these rows, since no + /// other signal on the bus reports a removal. + pub per_wallet_swept: BTreeMap>, } impl CheckTransactionsResult { @@ -628,6 +636,19 @@ impl WalletManager { } } + // Gathered outside the relevance branch above: a sweep can + // fire for a transaction this wallet finds irrelevant — the + // shared input is gone from `utxos` and the winner may pay + // only external addresses — and the removal still has to + // reach the consumer. + if !check_result.swept_transactions.is_empty() { + result + .per_wallet_swept + .entry(*wallet_id) + .or_default() + .extend(check_result.swept_transactions); + } + if !check_result.new_addresses.is_empty() { result .new_addresses @@ -662,6 +683,51 @@ impl WalletManager { } impl WalletManager { + /// Abandon `root` in `wallet_id`, and every recorded transaction + /// descending from it, then recompute the balance. + /// + /// The manager-level entry point for + /// [`ManagedWalletInfo::abandon_transaction_with_spends`] — the only + /// production path that clears a transaction the network never accepted. + /// The conflict sweep cannot reach that case: it needs a competing final + /// spend to prove the loser dead, and a transaction nobody ever saw has + /// no competitor. Its outputs would otherwise be credited forever, and as + /// trusted self-send change they are counted confirmed and are spendable. + /// + /// `external_spends` maps an outpoint to the transaction a caller's + /// persistence mirror recorded as spending it, for descendants whose own + /// records the load path never restored. Pass an empty map to walk only + /// the recorded transactions. + /// + /// **This asserts the root is dead; it does not establish it.** There is + /// no negative signal on the p2p network — Dash Core removed BIP61 + /// `reject` — so silence is not proof, and abandoning a transaction that + /// is merely quiet re-exposes its inputs to coin selection. Settled roots + /// are refused, but the judgement otherwise belongs to the caller that + /// owns broadcast policy. + /// + /// Returns `None` when the wallet is unknown. + pub fn abandon_transaction( + &mut self, + wallet_id: &WalletId, + root: Txid, + external_spends: &BTreeMap, + ) -> Option { + let info = self.get_wallet_info_mut(wallet_id)?; + let outcome = info.abandon_transaction_with_spends(root, external_spends); + if !outcome.is_empty() { + info.update_balance(); + tracing::info!( + %root, + abandoned = outcome.abandoned.len(), + records_removed = outcome.records_removed, + utxos_removed = outcome.utxos_removed, + "Abandoned a dead transaction and everything built on it" + ); + } + Some(outcome) + } + /// Get receive address from a specific wallet and account pub fn next_receive_address( &mut self, diff --git a/key-wallet-manager/src/process_block.rs b/key-wallet-manager/src/process_block.rs index 28e0f986f..f889083d7 100644 --- a/key-wallet-manager/src/process_block.rs +++ b/key-wallet-manager/src/process_block.rs @@ -84,6 +84,26 @@ impl WalletInterface for WalletM for (wallet_id, records) in check_result.per_wallet_updated_records { per_wallet_updated.entry(wallet_id).or_default().extend(records); } + // Emitted per transaction rather than batched into the block + // event: a sweep names the transaction that superseded the + // removed ones, and that attribution is lost once the block's + // transactions are folded together. + for (wallet_id, txids) in check_result.per_wallet_swept { + if txids.is_empty() { + continue; + } + let Some(info) = self.wallet_infos.get(&wallet_id) else { + continue; + }; + let event = WalletEvent::TransactionsSwept { + wallet_id, + txids, + superseded_by: tx.txid(), + balance: info.balance(), + account_balances: BTreeMap::new(), + }; + self.emit_event(event); + } } self.finalize_block_advance( @@ -185,6 +205,29 @@ impl WalletInterface for WalletM } } + // Removals, before the additive events: a consumer applying these in + // order sees the dead rows deleted first, so a replacement paying the + // same address cannot be clobbered by the delete that follows it. + for (wallet_id, txids) in std::mem::take(&mut check_result.per_wallet_swept) { + if txids.is_empty() { + continue; + } + let Some(info) = self.wallet_infos.get(&wallet_id) else { + continue; + }; + let event = WalletEvent::TransactionsSwept { + wallet_id, + txids, + superseded_by: tx.txid(), + balance: info.balance(), + account_balances: per_wallet_account_diff + .get(&wallet_id) + .cloned() + .unwrap_or_default(), + }; + self.emit_event(event); + } + if let Some(lock) = instant_lock { for (wallet_id, records) in per_wallet_updated_records { if records.is_empty() { diff --git a/key-wallet/src/managed_account/managed_account_collection.rs b/key-wallet/src/managed_account/managed_account_collection.rs index cf60f086b..d5ef903b4 100644 --- a/key-wallet/src/managed_account/managed_account_collection.rs +++ b/key-wallet/src/managed_account/managed_account_collection.rs @@ -3,7 +3,10 @@ //! This module provides a structure for managing multiple accounts //! across different networks in a hierarchical manner. -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; + +use dashcore::blockdata::transaction::OutPoint; +use dashcore::Transaction; use crate::account::account_collection::{DashpayAccountKey, PlatformPaymentAccountKey}; use crate::gap_limit::DIP17_GAP_LIMIT; @@ -934,6 +937,25 @@ impl ManagedAccountCollection { accounts } + /// Union, across every funds-bearing account, of the outpoints among + /// `tx`'s inputs that the wallet holds as final UTXOs. + /// + /// A single account can only answer this for the coins it owns, but + /// pooled funding (asset locks draw from BIP44 + BIP32 + the DashPay + /// contact-receiving accounts) routinely spreads one transaction's inputs + /// across several. The union is what makes the trusted-self-send check in + /// [`ManagedCoreFundsAccount::record_transaction`] see the whole wallet. + /// + /// Must be taken before any account processes `tx` — `update_utxos` + /// removes spent parents as it goes. + pub(crate) fn final_parents_of(&self, tx: &Transaction) -> BTreeSet { + let mut parents = BTreeSet::new(); + for funds in self.all_funding_accounts() { + funds.collect_final_parents(tx, &mut parents); + } + parents + } + /// Get all accounts in the collection as mutable /// [`ManagedAccountRefMut`] values. pub fn all_accounts_mut(&mut self) -> Vec> { diff --git a/key-wallet/src/managed_account/managed_account_ref.rs b/key-wallet/src/managed_account/managed_account_ref.rs index b94895b79..172d40327 100644 --- a/key-wallet/src/managed_account/managed_account_ref.rs +++ b/key-wallet/src/managed_account/managed_account_ref.rs @@ -23,7 +23,7 @@ use crate::Network; use dashcore::blockdata::transaction::OutPoint; use dashcore::prelude::CoreBlockHeight; use dashcore::{Address, ScriptBuf, Transaction, Txid}; -use std::collections::BTreeMap; +use std::collections::{BTreeMap, BTreeSet}; /// Immutable reference to a managed core account, either funds-bearing or /// keys-only. @@ -314,6 +314,7 @@ impl<'a> ManagedAccountRefMut<'a> { context, transaction_type, &BTreeMap::new(), + &BTreeSet::new(), ) } @@ -321,6 +322,9 @@ impl<'a> ManagedAccountRefMut<'a> { /// the wallet-level `observed_spent_outpoints` view /// (dashpay/rust-dashcore#649); only the funds variant consults it (keys /// accounts track no UTXOs/output details). + /// + /// `external_final_parents` is the wallet-level view of input parents held + /// by sibling accounts, used for the trusted-self-send determination. pub(crate) fn record_transaction_with_observed_spends( &mut self, tx: &Transaction, @@ -328,11 +332,17 @@ impl<'a> ManagedAccountRefMut<'a> { context: TransactionContext, transaction_type: TransactionType, observed_spent: &BTreeMap, + external_final_parents: &BTreeSet, ) -> TransactionRecord { match self { - ManagedAccountRefMut::Funds(a) => { - a.record_transaction(tx, account_match, context, transaction_type, observed_spent) - } + ManagedAccountRefMut::Funds(a) => a.record_transaction( + tx, + account_match, + context, + transaction_type, + observed_spent, + external_final_parents, + ), ManagedAccountRefMut::Keys(a) => { a.record_transaction(tx, account_match, context, transaction_type) } @@ -361,12 +371,16 @@ impl<'a> ManagedAccountRefMut<'a> { context, transaction_type, &BTreeMap::new(), + &BTreeSet::new(), ) } /// Re-process an existing transaction, reconciling refreshed UTXO state /// against `observed_spent` — the wallet-level `observed_spent_outpoints` /// view (dashpay/rust-dashcore#649); only the funds variant consults it. + /// + /// `external_final_parents` is the wallet-level view of input parents held + /// by sibling accounts, used for the trusted-self-send determination. pub(crate) fn confirm_transaction_with_observed_spends( &mut self, tx: &Transaction, @@ -374,11 +388,17 @@ impl<'a> ManagedAccountRefMut<'a> { context: TransactionContext, transaction_type: TransactionType, observed_spent: &BTreeMap, + external_final_parents: &BTreeSet, ) -> Option { match self { - ManagedAccountRefMut::Funds(a) => { - a.confirm_transaction(tx, account_match, context, transaction_type, observed_spent) - } + ManagedAccountRefMut::Funds(a) => a.confirm_transaction( + tx, + account_match, + context, + transaction_type, + observed_spent, + external_final_parents, + ), ManagedAccountRefMut::Keys(a) => { a.confirm_transaction(tx, account_match, context, transaction_type) } diff --git a/key-wallet/src/managed_account/managed_core_funds_account.rs b/key-wallet/src/managed_account/managed_core_funds_account.rs index 663e24dc6..91eac9d5c 100644 --- a/key-wallet/src/managed_account/managed_core_funds_account.rs +++ b/key-wallet/src/managed_account/managed_core_funds_account.rs @@ -70,6 +70,16 @@ pub struct ManagedCoreFundsAccount { reservations: ReservationSet, } +/// What [`ManagedCoreFundsAccount::apply_abandon`] removed from one account. +#[derive(Debug, Clone, Copy, Default, PartialEq, Eq)] +pub(crate) struct AbandonRemoval { + /// UTXOs the abandoned transactions had contributed. + pub utxos: usize, + /// Transaction records actually dropped — a txid the account never held + /// removes nothing. + pub records: usize, +} + impl ManagedCoreFundsAccount { /// Create a new managed funds account pub fn new(managed_account_type: ManagedAccountType, network: Network) -> Self { @@ -164,6 +174,23 @@ impl ManagedCoreFundsAccount { self.spent_outpoints.contains(outpoint) } + /// Collect the outpoints among `tx`'s inputs that this account holds as a + /// final UTXO — confirmed, InstantSend-locked, or trusted. + /// + /// Used at the wallet level to assemble the cross-account parent view that + /// [`Self::record_transaction`] needs: one account cannot tell whether a + /// pooled transaction's other inputs are ours, but the wallet can ask every + /// account and union the answers. + pub(crate) fn collect_final_parents(&self, tx: &Transaction, into: &mut BTreeSet) { + for input in &tx.input { + if self.utxos.get(&input.previous_output).is_some_and(|parent| { + parent.is_confirmed || parent.is_instantlocked || parent.is_trusted + }) { + into.insert(input.previous_output); + } + } + } + /// Cached scriptPubKeys for every address that could still receive or hold /// funds under a single-use address discipline: addresses not yet used /// (the gap-limit lookahead, including reserved ones) plus used addresses @@ -190,12 +217,18 @@ impl ManagedCoreFundsAccount { /// /// Skips any output whose outpoint is already in `observed_spent` — it is /// spent on-chain (dashpay/rust-dashcore#649), so the record stays consistent. + /// + /// `external_final_parents` carries the wallet-level view of the inputs: + /// outpoints that a *sibling* account of the same wallet holds as final. + /// See [`Self::record_transaction`] for why a per-account view is not + /// enough. An empty set degrades this to the account-local check. fn update_utxos( &mut self, tx: &Transaction, account_match: &AccountMatch, context: TransactionContext, observed_spent: &BTreeMap, + external_final_parents: &BTreeSet, ) { // Update UTXOs only for spendable account types match self.keys.managed_account_type() { @@ -233,15 +266,48 @@ impl ManagedCoreFundsAccount { // they are only removed after the insert loop below. An unknown // or non-final parent denies trust, so funds that the network // may still drop never surface as confirmed. + // + // "Ours" is a wallet-level question, not an account-level one: + // pooled funding (asset locks draw from BIP44 + BIP32 + the + // DashPay contact-receiving accounts) routinely puts inputs + // from a sibling account into a transaction whose change lands + // here. Consulting only `self.utxos` would deny trust to our + // own transfer and file its change under `unconfirmed`, so the + // caller's wallet-wide view fills in the parents this account + // cannot see. let all_inputs_final_and_ours = tx.input.iter().all(|input| { self.utxos.get(&input.previous_output).is_some_and(|parent| { parent.is_confirmed || parent.is_instantlocked || parent.is_trusted - }) + }) || external_final_parents.contains(&input.previous_output) }); let txid = tx.txid(); let mut utxos_changed = false; + // A transaction whose input a *block* already spent can never + // confirm — unless this arrival is that block delivery itself. + // The conflict sweep below only fires when the arriving + // transaction is final, so without this the reverse order + // (winner confirms, loser arrives afterwards as mempool) + // credits the loser's outputs with nothing left to remove + // them, and `is_spendable` would hand them to coin selection. + let doomed_by_a_settled_spend = !context.confirmed() + && !matches!(context, TransactionContext::InstantSend(_)) + && tx + .input + .iter() + .any(|input| observed_spent.contains_key(&input.previous_output)); + if doomed_by_a_settled_spend { + // Deliberately before any mutation: the record built by + // the caller stands, so history still shows the attempt, + // but nothing it created enters the UTXO set. + tracing::info!( + %txid, + "Not crediting a transaction whose input a block already spent" + ); + return; + } + let network = self.keys.network(); // Insert UTXOs for outputs paying to our addresses @@ -343,6 +409,244 @@ impl ManagedCoreFundsAccount { } } + /// Drop the spent-marks that `freed` contributed, keeping every mark a + /// surviving record still claims. + /// + /// Deliberately *not* a wholesale rebuild from the live records. Under the + /// default `keep-finalized-transactions = off` a chainlocked spend's + /// record is reduced to its txid, so its inputs survive only as marks + /// already in this set — reassigning from the record map would silently + /// drop them and let a later backfill re-credit coins that are spent on + /// chain. Only outpoints the removed records actually contributed are + /// considered, and a removed record's input stays marked when a survivor + /// spends it too (a loser spending A+B against a winner spending only A + /// must leave A marked and free B). + fn release_spent_marks(&mut self, freed: &HashSet) { + if freed.is_empty() { + return; + } + let still_spent = rebuild_spent_outpoints(&self.keys); + self.spent_outpoints + .retain(|outpoint| !freed.contains(outpoint) || still_spent.contains(outpoint)); + } + + /// Remove every trace of `abandoned` from this account. + /// + /// Drops the outputs those transactions contributed and their records, and + /// releases the outpoints they spent from `spent_outpoints` so the coins + /// become eligible for rediscovery. + /// + /// The released parents are deliberately **not** re-inserted into `utxos`. + /// `update_utxos` discards the `Utxo` when it removes a spent parent, and + /// `InputDetail` keeps only index/value/address, so the flags that decide + /// which balance bucket a restored coin belongs in are not retained + /// anywhere. Inventing them would be a guess. What these coins genuinely + /// are is unspent on chain — the abandoned transaction never reached the + /// network — so the correct source of truth is a rescan, which releasing + /// them from `spent_outpoints` now permits. + /// + /// Reservations are deliberately left alone. A recorded transaction has + /// already handed its inputs from the ephemeral set to `spent_outpoints` + /// (see `update_utxos`), so there is nothing of this build's left to + /// release — while an unconditional release here could free a reservation + /// a *newer* build has since taken over the same outpoints, which + /// `ReservationSet::release` documents as forbidden for exactly this + /// caller shape. + /// + /// Returns what was actually removed. + pub(crate) fn apply_abandon(&mut self, abandoned: &BTreeSet) -> AbandonRemoval { + let doomed: Vec = self + .utxos + .keys() + .filter(|outpoint| abandoned.contains(&outpoint.txid)) + .copied() + .collect(); + let utxos = doomed.len(); + for outpoint in doomed { + self.utxos.remove(&outpoint); + } + + let mut records = 0; + let mut freed: HashSet = HashSet::new(); + for txid in abandoned { + if let Some(record) = self.keys.transactions_mut().remove(txid) { + records += 1; + freed.extend(record.transaction.input.iter().map(|input| input.previous_output)); + } + } + if records > 0 { + self.release_spent_marks(&freed); + } + + if utxos > 0 { + self.keys.bump_monitor_revision(); + } + AbandonRemoval { + utxos, + records, + } + } + + /// Drop the outputs of any recorded unconfirmed transaction that `tx` + /// provably beat to one of its inputs. + /// + /// When `tx` arrives with a final context — in a block, or InstantSend + /// locked — every input it spends is settled under Dash consensus. Any + /// *other* transaction we recorded that spends the same outpoint can + /// therefore never confirm, and the UTXOs it contributed (its change) are + /// money that does not exist. Nothing else removes them: the loser is not + /// in a block, so no block processing revisits it, and mempool expiry in + /// dash-spv only drops its own tracking without telling the wallet. Left + /// alone they are counted permanently — and as *confirmed*, not merely + /// unconfirmed, whenever the trusted-self-send rule applies to them, + /// which also makes them selectable by coin selection. + /// + /// This is deliberately narrow. It fires only on proof — a conflicting + /// spend that is itself final — never on a timeout: the p2p network has no + /// negative signal (modern Dash Core removed BIP61 `reject`), so a + /// transaction that merely went quiet may still be alive in a miner's + /// mempool, and un-applying it would re-expose its inputs to coin + /// selection and invite a double-spend. + /// + /// Only the loser's *outputs* are reverted. Where the winner spends every + /// input the loser did — the ordinary resend — that is complete: those + /// inputs are correctly accounted for by `tx`, the transaction that + /// actually spent them. + /// + /// A loser may also spend inputs the winner does not. Those coins are + /// freed from `spent_outpoints` below, but they cannot be re-credited + /// here: `update_utxos` discarded their `Utxo` — and its flags — when the + /// loser was recorded, and `InputDetail` keeps only index/value/address. + /// The release is what makes them recoverable: a rescan re-delivering the + /// funding transaction inserts them again. Until that rescan they are + /// absent from the balance. + /// + /// That recovery has a boundary worth knowing. A funding transaction that + /// was chainlock-finalized keeps only its txid, so `has_transaction` stays + /// true and re-delivery is not a new sighting — `confirm_transaction` + /// returns before `update_utxos`, the only production insert site, and the + /// coin does not come back. Recovering it needs a rescan deep enough to + /// re-fetch the block, which is above this layer. Since Dash chainlocks + /// within a block or two, that is the normal posture for older coins. + /// + /// Scope: account-local. A loser recorded here has its outputs dropped + /// here; a loser whose change landed in a *different* account is not + /// reached, because both the transaction records and the UTXO set are + /// per-account. That covers the ordinary shape — a resend keeps the same + /// funding account and so the same change account — but not every one. + /// + /// Returns the txids it removed. + pub(crate) fn drop_conflicted_transactions( + &mut self, + tx: &Transaction, + context: &TransactionContext, + ) -> Vec { + if !(context.confirmed() || matches!(context, TransactionContext::InstantSend(_))) { + return Vec::new(); + } + + let winner = tx.txid(); + let spent: BTreeSet = + tx.input.iter().map(|input| input.previous_output).collect(); + + // A finalized transaction keeps only its txid, so a chainlocked record + // can never be a loser here — and must not be, since it is settled. + let mut losers: BTreeSet = self + .keys + .transactions() + .iter() + .filter(|(txid, record)| { + // Precedence, per DIP-10: a chainlock is final over + // everything, an InstantSend lock is final against a double + // spend, and a plain block is provisional until its own + // chainlock lands. So an IS-locked record may only be evicted + // by a chainlocked arrival — a plain `InBlock` winner cannot + // overrule a lock the network already signed, and the block + // it arrived in can still reorg away. + let loser_is_locked = record.context.is_instant_send(); + **txid != winner + && !record.is_confirmed() + && (!loser_is_locked || context.is_chain_locked()) + && record + .transaction + .input + .iter() + .any(|input| spent.contains(&input.previous_output)) + }) + .map(|(txid, _)| *txid) + .collect(); + + if losers.is_empty() { + return Vec::new(); + } + + // A loser's change may already have funded further unconfirmed + // transactions. Those can never exist either — their parent cannot — + // so leaving their outputs credited would preserve the very + // phantom-balance class this sweep exists to remove. Walk the + // unconfirmed descendant closure; confirmed records are never + // followed, since a transaction in a block spent something real. + loop { + let mut found = BTreeSet::new(); + for (txid, record) in self.keys.transactions() { + if record.is_confirmed() + || record.context.is_instant_send() + || losers.contains(txid) + || *txid == winner + { + continue; + } + if record + .transaction + .input + .iter() + .any(|input| losers.contains(&input.previous_output.txid)) + { + found.insert(*txid); + } + } + let before = losers.len(); + losers.extend(found); + if losers.len() == before { + break; + } + } + + let mut freed: HashSet = HashSet::new(); + let mut changed = false; + for loser in &losers { + let removed: Vec = + self.utxos.keys().filter(|outpoint| outpoint.txid == *loser).copied().collect(); + for outpoint in removed { + self.utxos.remove(&outpoint); + changed = true; + } + if let Some(record) = self.keys.transactions_mut().remove(loser) { + freed.extend(record.transaction.input.iter().map(|input| input.previous_output)); + } + tracing::info!( + conflicted_txid = %loser, + winning_txid = %winner, + "Dropped a conflicted transaction: its input was spent by a final transaction" + ); + } + // Never free an outpoint the winner itself spends. `freed` collects + // every input of every removed loser, and the shared one is exactly + // what the winner consumed — releasing it would let a later rescan + // re-insert a coin that is spent on chain, and coin selection would + // then build a guaranteed double spend. `release_spent_marks` cannot + // catch this on its own: on the checker path the sweep runs before + // the winner is recorded, so no live record claims the outpoint yet. + // Only the loser's *extra* inputs are genuinely released. + freed.retain(|outpoint| !spent.contains(outpoint)); + self.release_spent_marks(&freed); + if changed { + self.keys.bump_monitor_revision(); + } + + losers.into_iter().collect() + } + /// Re-process an existing transaction with updated context (e.g., /// mempool→block confirmation) and potentially new address matches /// from gap limit rescans. @@ -366,6 +670,7 @@ impl ManagedCoreFundsAccount { context: TransactionContext, transaction_type: TransactionType, observed_spent: &BTreeMap, + external_final_parents: &BTreeSet, ) -> Option { let txid = tx.txid(); @@ -384,6 +689,7 @@ impl ManagedCoreFundsAccount { context, transaction_type, observed_spent, + external_final_parents, ); return Some(record); } @@ -423,7 +729,7 @@ impl ManagedCoreFundsAccount { // chainlock catches up. #[cfg(not(feature = "keep-finalized-transactions"))] let drop_now = context.is_chain_locked(); - self.update_utxos(tx, account_match, context, observed_spent); + self.update_utxos(tx, account_match, context, observed_spent, external_final_parents); #[cfg(not(feature = "keep-finalized-transactions"))] if drop_now { self.keys.drop_finalized_transaction(&txid); @@ -439,6 +745,14 @@ impl ManagedCoreFundsAccount { /// for any output whose outpoint is already observed spent on-chain, so a /// coin whose spend was seen in an earlier-processed block is never /// (re-)tracked as spendable. + /// + /// `external_final_parents` is the wallet-level answer to "are these + /// inputs ours and final" for parents this account does not hold. A + /// transaction funded from several accounts — the normal shape for asset + /// locks — is still our own self-send, and its change must not be filed + /// under `unconfirmed` merely because the sibling account's UTXOs are + /// invisible from here. Callers driving a single account directly pass an + /// empty set and get the account-local behavior. pub(crate) fn record_transaction( &mut self, tx: &Transaction, @@ -446,6 +760,7 @@ impl ManagedCoreFundsAccount { context: TransactionContext, transaction_type: TransactionType, observed_spent: &BTreeMap, + external_final_parents: &BTreeSet, ) -> TransactionRecord { let net_amount = account_match.received as i64 - account_match.sent as i64; @@ -553,7 +868,7 @@ impl ManagedCoreFundsAccount { // feature is on (we want to keep the full record). #[cfg(not(feature = "keep-finalized-transactions"))] let drop_now = context.is_chain_locked(); - self.update_utxos(tx, account_match, context, observed_spent); + self.update_utxos(tx, account_match, context, observed_spent, external_final_parents); #[cfg(not(feature = "keep-finalized-transactions"))] if drop_now { self.keys.drop_finalized_transaction(&txid); @@ -919,9 +1234,14 @@ impl ManagedAccountTrait for ManagedCoreFundsAccount { /// /// Every input of every recorded transaction is a spend this account has seen, /// so its `previous_output` belongs in the derived set. The field is not -/// persisted (`#[serde(skip)]`), so both [`Deserialize`] and the test reload -/// simulation reconstruct it through here to stay in lockstep. -#[cfg(any(feature = "serde", test))] +/// serialized (`#[serde(skip_serializing)]`), so [`Deserialize`] and the test +/// reload simulation reconstruct it through here to stay in lockstep. +/// +/// **Derives only from live records.** Under the default +/// `keep-finalized-transactions = off`, a chainlocked record is dropped to +/// just its txid, so its inputs survive only as entries already in the set — +/// which a wholesale rebuild would discard. Callers pruning a subset of +/// records must retain the rest rather than reassigning from this. fn rebuild_spent_outpoints(keys: &ManagedCoreKeysAccount) -> HashSet { keys.transactions() .values() diff --git a/key-wallet/src/transaction_checking/account_checker.rs b/key-wallet/src/transaction_checking/account_checker.rs index e1a907d8b..c138ce994 100644 --- a/key-wallet/src/transaction_checking/account_checker.rs +++ b/key-wallet/src/transaction_checking/account_checker.rs @@ -17,6 +17,7 @@ use dashcore::blockdata::transaction::Transaction; use dashcore::hashes::Hash as _; use dashcore::transaction::TransactionPayload; use dashcore::ScriptBuf; +use dashcore::Txid; /// Classification of an address within an account #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -76,6 +77,16 @@ pub struct TransactionCheckResult { /// applied to a previously stored record). Each record carries its owning /// `AccountType` on `record.account_type`. pub updated_records: Vec, + /// Transactions this check *removed*: a recorded spend that the arriving + /// transaction provably beat to one of its inputs, plus anything built on + /// its outputs. They can never confirm, so their outputs were dropped + /// from the UTXO set and their records deleted. + /// + /// The only non-additive field here, and it exists because a consumer + /// mirroring wallet state cannot otherwise learn a row is gone — it would + /// replay the dead transaction on the next load and re-create the phantom + /// balance this removal just cleared. + pub swept_transactions: Vec, } /// Enum representing the type of Core account that matched with embedded data @@ -405,6 +416,7 @@ impl ManagedAccountCollection { new_addresses: Vec::new(), new_records: Vec::new(), updated_records: Vec::new(), + swept_transactions: Vec::new(), }; for account_type in account_types { diff --git a/key-wallet/src/transaction_checking/wallet_checker.rs b/key-wallet/src/transaction_checking/wallet_checker.rs index cb0272f2a..f16bfdb5d 100644 --- a/key-wallet/src/transaction_checking/wallet_checker.rs +++ b/key-wallet/src/transaction_checking/wallet_checker.rs @@ -81,10 +81,34 @@ impl WalletTransactionChecker for ManagedWalletInfo { } } + // A final arrival settles its inputs whether or not this transaction + // looks relevant to us. Relevance is computed from matching outputs + // and from inputs still present in `utxos` — but a recorded loser + // already removed the shared input, so a winner that spends our coin + // and pays only external addresses matches nothing and would return + // below with the loser still credited. Sweep first, wallet-wide, next + // to `record_observed_spends` above for the same reason it is + // unconditional. + if update_state && (context.confirmed() || context.is_instant_send()) { + result.swept_transactions = self.sweep_conflicts(tx, &context); + if !result.swept_transactions.is_empty() { + result.state_modified = true; + } + } + if !update_state || !result.is_relevant { return result; } + // Wallet-wide view of this transaction's input parents, taken while + // every account is still readable and before any `update_utxos` call + // starts removing spent parents. Without it a pooled self-send — the + // normal shape for asset locks, which fund from BIP44 + BIP32 + the + // DashPay contact-receiving accounts — is not recognised as ours by + // the account holding the change, and that change lands in the + // `unconfirmed` bucket. + let external_final_parents = self.accounts.final_parents_of(tx); + // Check if this transaction already exists in any affected account let txid = tx.txid(); let mut is_new = true; @@ -150,6 +174,7 @@ impl WalletTransactionChecker for ManagedWalletInfo { context.clone(), tx_type, &self.observed_spent_outpoints, + &external_final_parents, ); account.mark_utxos_instant_send(&txid); result.new_records.push(record); @@ -182,6 +207,7 @@ impl WalletTransactionChecker for ManagedWalletInfo { context.clone(), tx_type, &self.observed_spent_outpoints, + &external_final_parents, ); result.new_records.push(record); result.state_modified = true; @@ -193,6 +219,7 @@ impl WalletTransactionChecker for ManagedWalletInfo { context.clone(), tx_type, &self.observed_spent_outpoints, + &external_final_parents, ) { result.state_modified = true; if existed_before { @@ -286,7 +313,7 @@ mod tests { use dashcore::TxOut; use dashcore::{Address, BlockHash, TxIn, Txid}; use dashcore_hashes::Hash; - use std::collections::BTreeMap; + use std::collections::{BTreeMap, BTreeSet}; /// Test wallet checker with unrelated transaction #[tokio::test] @@ -1412,6 +1439,7 @@ mod tests { block_context, tx_type, &BTreeMap::new(), + &BTreeSet::new(), ); assert!(backfilled.is_some(), "Should return Some when backfilling a missing record"); @@ -1469,6 +1497,7 @@ mod tests { block_context, tx_type, &BTreeMap::new(), + &BTreeSet::new(), ); assert!(confirmed.is_some(), "Should return Some when confirming unconfirmed tx"); @@ -2038,6 +2067,1157 @@ mod tests { assert_eq!(ctx.managed_wallet.balance.spendable(), change_amount); } + /// The rescan recovery above has a boundary: a funding transaction that + /// was chainlock-finalized keeps only its txid, so re-delivering it is not + /// a new sighting and never reaches the only production UTXO insert site. + /// The coin stays absent. Documented rather than fixed — recovering it + /// needs a rescan deep enough to re-fetch the block, which is above this + /// layer. + #[tokio::test] + async fn test_rescan_recovery_does_not_reach_a_finalized_funding_transaction() { + let mut ctx = TestWalletContext::new_random(); + let external_address = Address::p2pkh( + &dashcore::PublicKey::from_slice(&[0x02; 33]).expect("pubkey"), + Network::Testnet, + ); + + let funding_tx = Transaction::dummy(&ctx.receive_address, 0..1, &[1_000_000]); + let finalized = TransactionContext::InChainLockedBlock(BlockInfo::new( + 100, + BlockHash::from_slice(&[5u8; 32]).expect("hash"), + 1_700_000_000, + )); + ctx.check_transaction(&funding_tx, finalized.clone()).await; + + let change_address = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .next_change_address(Some(&ctx.xpub), true) + .expect("change address"); + let spend = Transaction { + version: 2, + lock_time: 0, + input: vec![TxIn { + previous_output: OutPoint { + txid: funding_tx.txid(), + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }], + output: vec![ + TxOut { + value: 900_000, + script_pubkey: external_address.script_pubkey(), + }, + TxOut { + value: 99_000, + script_pubkey: change_address.script_pubkey(), + }, + ], + special_transaction_payload: None, + }; + ctx.check_transaction(&spend, TransactionContext::Mempool).await; + + ctx.managed_wallet.abandon_transaction(spend.txid()); + ctx.managed_wallet.update_balance(); + + // Re-delivering the funding block does not bring the coin back. + ctx.check_transaction(&funding_tx, finalized).await; + assert_eq!( + ctx.managed_wallet.balance.confirmed(), + 0, + "a finalized funding record blocks the redelivery path this \ + recovery depends on" + ); + } + + /// A winner that spends our coin but pays only external addresses matches + /// nothing: its outputs are not ours, and the input it shares with the + /// loser was already removed from `utxos` when the loser was recorded. It + /// is therefore classified irrelevant — and the sweep still has to run, + /// or the loser's change stays credited with nothing left to clear it. + #[tokio::test] + async fn test_an_irrelevant_winner_still_sweeps_its_loser() { + let mut ctx = TestWalletContext::new_random(); + let external_address = Address::p2pkh( + &dashcore::PublicKey::from_slice(&[0x02; 33]).expect("pubkey"), + Network::Testnet, + ); + + let funding_value = 1_000_000u64; + let funding_tx = Transaction::dummy(&ctx.receive_address, 0..1, &[funding_value]); + ctx.check_transaction( + &funding_tx, + TransactionContext::InBlock(BlockInfo::new( + 100, + BlockHash::from_slice(&[1u8; 32]).expect("hash"), + 1_700_000_000, + )), + ) + .await; + + let funding_outpoint = OutPoint { + txid: funding_tx.txid(), + vout: 0, + }; + let input = || TxIn { + previous_output: funding_outpoint, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }; + + // The loser pays us change, so it is relevant and gets recorded. + let loser_change = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .next_change_address(Some(&ctx.xpub), true) + .expect("change address"); + let loser = Transaction { + version: 2, + lock_time: 0, + input: vec![input()], + output: vec![ + TxOut { + value: 600_000, + script_pubkey: external_address.script_pubkey(), + }, + TxOut { + value: 399_000, + script_pubkey: loser_change.script_pubkey(), + }, + ], + special_transaction_payload: None, + }; + ctx.check_transaction(&loser, TransactionContext::Mempool).await; + assert_eq!( + ctx.managed_wallet.balance.confirmed(), + 399_000, + "trusted self-send change counts as confirmed" + ); + + // The winner spends the same coin and pays only outside the wallet. + let winner = Transaction { + version: 2, + lock_time: 0, + input: vec![input()], + output: vec![TxOut { + value: 999_000, + script_pubkey: external_address.script_pubkey(), + }], + special_transaction_payload: None, + }; + let result = ctx + .check_transaction( + &winner, + TransactionContext::InBlock(BlockInfo::new( + 101, + BlockHash::from_slice(&[2u8; 32]).expect("hash"), + 1_700_000_100, + )), + ) + .await; + assert!( + !result.is_relevant, + "the precondition: nothing about this winner matches the wallet" + ); + + assert!( + !ctx.bip44_account().transactions().contains_key(&loser.txid()), + "the loser must be swept even though the winner is irrelevant" + ); + assert_eq!( + ctx.managed_wallet.balance.confirmed(), + 0, + "and its change must stop counting as confirmed money" + ); + } + + /// Pooled funding puts a loser's change in an account the winner never + /// touches. Sweeping only the winner's matched accounts leaves it behind. + #[tokio::test] + async fn test_a_loser_in_a_sibling_account_is_swept() { + let mut ctx = TestWalletContext::new_random(); + let external_address = Address::p2pkh( + &dashcore::PublicKey::from_slice(&[0x02; 33]).expect("pubkey"), + Network::Testnet, + ); + + // Fund the BIP32 account. + let bip32_xpub = ctx + .wallet + .accounts + .standard_bip32_accounts + .get(&0) + .expect("BIP32 account") + .account_xpub; + let bip32_address = ctx + .managed_wallet + .first_bip32_managed_account_mut() + .expect("BIP32 managed account") + .next_receive_address(Some(&bip32_xpub), true) + .expect("BIP32 receive address"); + let funding_tx = Transaction::dummy(&bip32_address, 0..1, &[1_000_000]); + ctx.check_transaction( + &funding_tx, + TransactionContext::InBlock(BlockInfo::new( + 100, + BlockHash::from_slice(&[3u8; 32]).expect("hash"), + 1_700_000_000, + )), + ) + .await; + + let funding_outpoint = OutPoint { + txid: funding_tx.txid(), + vout: 0, + }; + let spend = |change: &Address, change_amount: u64, sent: u64| Transaction { + version: 2, + lock_time: 0, + input: vec![TxIn { + previous_output: funding_outpoint, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }], + output: vec![ + TxOut { + value: sent, + script_pubkey: external_address.script_pubkey(), + }, + TxOut { + value: change_amount, + script_pubkey: change.script_pubkey(), + }, + ], + special_transaction_payload: None, + }; + + // The loser's change lands on BIP44 — an account the winner, whose + // change goes back to BIP32, never matches. + let loser_change = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .next_change_address(Some(&ctx.xpub), true) + .expect("change address"); + let loser = spend(&loser_change, 399_000, 600_000); + ctx.check_transaction(&loser, TransactionContext::Mempool).await; + + let winner_change = ctx + .managed_wallet + .first_bip32_managed_account_mut() + .expect("BIP32 managed account") + .next_change_address(Some(&bip32_xpub), true) + .expect("BIP32 change address"); + let winner = spend(&winner_change, 299_000, 700_000); + ctx.check_transaction( + &winner, + TransactionContext::InBlock(BlockInfo::new( + 101, + BlockHash::from_slice(&[4u8; 32]).expect("hash"), + 1_700_000_100, + )), + ) + .await; + + assert!( + !ctx.bip44_account().transactions().contains_key(&loser.txid()), + "a loser in a sibling account must be swept too" + ); + assert_eq!( + ctx.managed_wallet.balance.confirmed(), + 299_000, + "only the winner's change survives" + ); + } + + /// The reverse arrival order: the winner confirms first, and the loser + /// turns up afterwards from the mempool. No sweep can help — the sweep + /// fires on the *arriving* transaction being final, and here the arrival + /// is the loser. The refusal has to happen at record time. + #[tokio::test] + async fn test_a_loser_arriving_after_its_winner_is_never_credited() { + let mut ctx = TestWalletContext::new_random(); + let external_address = Address::p2pkh( + &dashcore::PublicKey::from_slice(&[0x02; 33]).expect("pubkey"), + Network::Testnet, + ); + + let funding_tx = Transaction::dummy(&ctx.receive_address, 0..1, &[1_000_000]); + ctx.check_transaction( + &funding_tx, + TransactionContext::InBlock(BlockInfo::new( + 100, + BlockHash::from_slice(&[8u8; 32]).expect("hash"), + 1_700_000_000, + )), + ) + .await; + + let shared = OutPoint { + txid: funding_tx.txid(), + vout: 0, + }; + let spend = |change: &Address, change_amount: u64, sent: u64| Transaction { + version: 2, + lock_time: 0, + input: vec![TxIn { + previous_output: shared, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }], + output: vec![ + TxOut { + value: sent, + script_pubkey: external_address.script_pubkey(), + }, + TxOut { + value: change_amount, + script_pubkey: change.script_pubkey(), + }, + ], + special_transaction_payload: None, + }; + + // The winner confirms first. + let winner_change = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .next_change_address(Some(&ctx.xpub), true) + .expect("change address"); + let winner = spend(&winner_change, 299_000, 700_000); + ctx.check_transaction( + &winner, + TransactionContext::InBlock(BlockInfo::new( + 101, + BlockHash::from_slice(&[9u8; 32]).expect("hash"), + 1_700_000_100, + )), + ) + .await; + assert_eq!(ctx.managed_wallet.balance.confirmed(), 299_000); + + // Then the loser turns up. Its input is provably spent, so its + // outputs must never be credited. + let loser_change = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .next_change_address(Some(&ctx.xpub), true) + .expect("change address"); + let loser = spend(&loser_change, 399_000, 600_000); + ctx.check_transaction(&loser, TransactionContext::Mempool).await; + + assert!( + !ctx.bip44_account().utxos.keys().any(|o| o.txid == loser.txid()), + "a transaction whose input a block already spent must not be credited" + ); + assert_eq!( + ctx.managed_wallet.balance.confirmed(), + 299_000, + "only the winner's change counts" + ); + } + + /// `abandon_transaction_with_spends`' external view: a descendant whose + /// own record the load path never restored, a stale row naming a settled + /// transaction that must not be followed, and a settled root refused + /// outright. None of these are reachable through the no-argument form. + #[tokio::test] + async fn test_abandon_honours_the_external_spend_view_and_refuses_settled() { + let mut ctx = TestWalletContext::new_random(); + let external_address = Address::p2pkh( + &dashcore::PublicKey::from_slice(&[0x02; 33]).expect("pubkey"), + Network::Testnet, + ); + + let funding_tx = Transaction::dummy(&ctx.receive_address, 0..1, &[1_000_000]); + let block = TransactionContext::InBlock(BlockInfo::new( + 100, + BlockHash::from_slice(&[7u8; 32]).expect("hash"), + 1_700_000_000, + )); + ctx.check_transaction(&funding_tx, block.clone()).await; + + let spend_of = + |parent: OutPoint, change: &Address, change_amount: u64, sent: u64| Transaction { + version: 2, + lock_time: 0, + input: vec![TxIn { + previous_output: parent, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }], + output: vec![ + TxOut { + value: sent, + script_pubkey: external_address.script_pubkey(), + }, + TxOut { + value: change_amount, + script_pubkey: change.script_pubkey(), + }, + ], + special_transaction_payload: None, + }; + let next_change = |ctx: &mut TestWalletContext| { + ctx.managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .next_change_address(Some(&ctx.xpub), true) + .expect("change address") + }; + + // Root, then a child spending its change. + let root_change = next_change(&mut ctx); + let root = spend_of( + OutPoint { + txid: funding_tx.txid(), + vout: 0, + }, + &root_change, + 399_000, + 600_000, + ); + ctx.check_transaction(&root, TransactionContext::Mempool).await; + let child_change = next_change(&mut ctx); + let child = spend_of( + OutPoint { + txid: root.txid(), + vout: 1, + }, + &child_change, + 298_000, + 100_000, + ); + ctx.check_transaction(&child, TransactionContext::Mempool).await; + + // A settled root is refused outright, whatever the map says. + ctx.check_transaction(&funding_tx, block).await; + let refused = ctx.managed_wallet.abandon_transaction(funding_tx.txid()); + assert!(refused.is_empty(), "a settled root must be refused: {refused:?}"); + + // Simulate the restore: the child's record is absent, so the plain + // walk cannot reach it — only the mirror's linkage can. + ctx.managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .keys_mut() + .transactions_mut() + .remove(&child.txid()); + + let plain = ctx.managed_wallet.abandon_transaction(root.txid()); + assert_eq!(plain.abandoned.len(), 1, "without the map the walk stops at the root"); + assert!( + ctx.bip44_account().utxos.keys().any(|o| o.txid == child.txid()), + "the child's output is still credited" + ); + + // Now with the map, plus a stale row naming the settled funding tx — + // which must not be followed. + let mut external = BTreeMap::new(); + external.insert( + OutPoint { + txid: root.txid(), + vout: 1, + }, + child.txid(), + ); + external.insert( + OutPoint { + txid: child.txid(), + vout: 1, + }, + funding_tx.txid(), + ); + let outcome = ctx.managed_wallet.abandon_transaction_with_spends(root.txid(), &external); + ctx.managed_wallet.update_balance(); + + assert!( + outcome.abandoned.contains(&child.txid()), + "the map must reach a descendant whose record is gone" + ); + assert!( + !outcome.abandoned.contains(&funding_tx.txid()), + "a settled spender named by a stale row must not be followed" + ); + assert!( + !ctx.bip44_account().utxos.keys().any(|o| o.txid == child.txid()), + "the child's outputs must be gone" + ); + assert!( + ctx.bip44_account().transactions().contains_key(&funding_tx.txid()), + "the settled funding record must survive" + ); + } + + /// The sweep must never free the outpoint the winner itself spends. + /// + /// Two of the three arrival paths hide this: a block winner is already in + /// `observed_spent_outpoints`, and a relevant winner re-marks the outpoint + /// when it is recorded. An InstantSend winner has neither — the context + /// carries no block info, so no observed spend is recorded, and an + /// irrelevant one is never recorded at all. Releasing the shared coin + /// there lets a rescan re-insert money that is spent on chain. + #[tokio::test] + async fn test_the_sweep_never_frees_the_winners_own_input() { + let mut ctx = TestWalletContext::new_random(); + let external_address = Address::p2pkh( + &dashcore::PublicKey::from_slice(&[0x02; 33]).expect("pubkey"), + Network::Testnet, + ); + + let funding_tx = Transaction::dummy(&ctx.receive_address, 0..1, &[1_000_000]); + let funding_context = TransactionContext::InBlock(BlockInfo::new( + 100, + BlockHash::from_slice(&[6u8; 32]).expect("hash"), + 1_700_000_000, + )); + ctx.check_transaction(&funding_tx, funding_context.clone()).await; + + let shared_input = OutPoint { + txid: funding_tx.txid(), + vout: 0, + }; + let input = || TxIn { + previous_output: shared_input, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }; + + // Loser first: recorded, so the coin leaves `utxos`. + let loser_change = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .next_change_address(Some(&ctx.xpub), true) + .expect("change address"); + let loser = Transaction { + version: 2, + lock_time: 0, + input: vec![input()], + output: vec![ + TxOut { + value: 600_000, + script_pubkey: external_address.script_pubkey(), + }, + TxOut { + value: 399_000, + script_pubkey: loser_change.script_pubkey(), + }, + ], + special_transaction_payload: None, + }; + ctx.check_transaction(&loser, TransactionContext::Mempool).await; + + // The winner arrives InstantSend-locked and pays only outside the + // wallet: no block info to record an observed spend, and nothing + // about it matches, so it is never recorded either. + let winner = Transaction { + version: 2, + lock_time: 0, + input: vec![input()], + output: vec![TxOut { + value: 999_000, + script_pubkey: external_address.script_pubkey(), + }], + special_transaction_payload: None, + }; + let is_lock = InstantLock { + txid: winner.txid(), + ..InstantLock::default() + }; + let result = ctx.check_transaction(&winner, TransactionContext::InstantSend(is_lock)).await; + assert!(!result.is_relevant, "the precondition: the winner matches nothing"); + assert!( + !ctx.bip44_account().transactions().contains_key(&loser.txid()), + "the loser is still swept" + ); + + // The shared coin is spent on chain by the winner. Re-delivering the + // funding block must not bring it back. + ctx.check_transaction(&funding_tx, funding_context).await; + assert!( + !ctx.bip44_account().utxos.contains_key(&shared_input), + "a rescan must not resurrect a coin the winner consumed" + ); + assert_eq!(ctx.managed_wallet.balance.confirmed(), 0); + } + + /// A loser can spend inputs the winner does not. Sweeping it frees those + /// coins from the spent set, but their `Utxo` values were discarded when + /// the loser was recorded — so the sweep alone cannot put them back, and + /// the coins must be recoverable by the rescan the release enables. + #[tokio::test] + async fn test_a_swept_losers_extra_input_is_recoverable_by_rescan() { + let mut ctx = TestWalletContext::new_random(); + let external_address = Address::p2pkh( + &dashcore::PublicKey::from_slice(&[0x02; 33]).expect("pubkey"), + Network::Testnet, + ); + + // One funding transaction pays us twice: A and B. + let funding_tx = Transaction::dummy(&ctx.receive_address, 0..2, &[500_000, 400_000]); + let funding_context = TransactionContext::InBlock(BlockInfo::new( + 100, + BlockHash::from_slice(&[1u8; 32]).expect("hash"), + 1_700_000_000, + )); + ctx.check_transaction(&funding_tx, funding_context.clone()).await; + assert_eq!(ctx.managed_wallet.balance.confirmed(), 900_000); + + let coin_a = OutPoint { + txid: funding_tx.txid(), + vout: 0, + }; + let coin_b = OutPoint { + txid: funding_tx.txid(), + vout: 1, + }; + let spend = + |inputs: Vec, change: &Address, change_amount: u64, sent: u64| Transaction { + version: 2, + lock_time: 0, + input: inputs + .into_iter() + .map(|previous_output| TxIn { + previous_output, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }) + .collect(), + output: vec![ + TxOut { + value: sent, + script_pubkey: external_address.script_pubkey(), + }, + TxOut { + value: change_amount, + script_pubkey: change.script_pubkey(), + }, + ], + special_transaction_payload: None, + }; + + // The loser spends A and B; the winner spends only A, and confirms. + let loser_change = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .next_change_address(Some(&ctx.xpub), true) + .expect("change address"); + let loser = spend(vec![coin_a, coin_b], &loser_change, 99_000, 800_000); + ctx.check_transaction(&loser, TransactionContext::Mempool).await; + + let winner_change = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .next_change_address(Some(&ctx.xpub), true) + .expect("change address"); + let winner = spend(vec![coin_a], &winner_change, 99_000, 400_000); + ctx.check_transaction( + &winner, + TransactionContext::InBlock(BlockInfo::new( + 101, + BlockHash::from_slice(&[2u8; 32]).expect("hash"), + 1_700_000_100, + )), + ) + .await; + + // The loser is gone, and B is not credited — its `Utxo` was + // discarded when the loser was recorded and cannot be invented. + assert!(!ctx.bip44_account().transactions().contains_key(&loser.txid())); + assert!(!ctx.bip44_account().utxos.contains_key(&coin_b)); + + // But B was freed from the spent set, so re-delivering the funding + // block restores it. That is what makes the loss recoverable rather + // than permanent. + ctx.check_transaction(&funding_tx, funding_context).await; + assert!( + ctx.bip44_account().utxos.contains_key(&coin_b), + "a rescan must be able to rediscover the loser's extra input" + ); + assert_eq!(ctx.managed_wallet.balance.confirmed(), 499_000, "B plus the winner's change"); + } + + /// An InstantSend lock is final, so it settles the winner's inputs just as + /// a block would — including when the winner was already sitting in the + /// mempool alongside its loser, which is the transition that skips + /// `record_transaction` and so skips the sweep it carries. + #[tokio::test] + async fn test_instant_send_on_an_existing_mempool_tx_drops_its_conflict() { + let mut ctx = TestWalletContext::new_random(); + let external_address = Address::p2pkh( + &dashcore::PublicKey::from_slice(&[0x02; 33]).expect("pubkey"), + Network::Testnet, + ); + + let funding_value = 1_000_000u64; + let funding_tx = Transaction::dummy(&ctx.receive_address, 0..1, &[funding_value]); + ctx.check_transaction( + &funding_tx, + TransactionContext::InBlock(BlockInfo::new( + 100, + BlockHash::from_slice(&[1u8; 32]).expect("hash"), + 1_700_000_000, + )), + ) + .await; + + let funding_outpoint = OutPoint { + txid: funding_tx.txid(), + vout: 0, + }; + let spend_of = |change: &Address, change_amount: u64, sent: u64| Transaction { + version: 2, + lock_time: 0, + input: vec![TxIn { + previous_output: funding_outpoint, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }], + output: vec![ + TxOut { + value: sent, + script_pubkey: external_address.script_pubkey(), + }, + TxOut { + value: change_amount, + script_pubkey: change.script_pubkey(), + }, + ], + special_transaction_payload: None, + }; + + // Both competing spends land in the mempool, loser first. + let loser_change = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .next_change_address(Some(&ctx.xpub), true) + .expect("change address"); + let loser = spend_of(&loser_change, 399_000, 600_000); + ctx.check_transaction(&loser, TransactionContext::Mempool).await; + + let winner_change = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .next_change_address(Some(&ctx.xpub), true) + .expect("change address"); + let winner = spend_of(&winner_change, 299_000, 700_000); + ctx.check_transaction(&winner, TransactionContext::Mempool).await; + + let loser_change_outpoint = OutPoint { + txid: loser.txid(), + vout: 1, + }; + assert!( + ctx.bip44_account().utxos.contains_key(&loser_change_outpoint), + "both are live while neither is final" + ); + + // The winner is InstantSend-locked. It is already recorded, so this + // takes the update-in-place branch rather than recording afresh. + let is_lock = InstantLock { + txid: winner.txid(), + ..InstantLock::default() + }; + ctx.check_transaction(&winner, TransactionContext::InstantSend(is_lock)).await; + + assert!( + !ctx.bip44_account().utxos.contains_key(&loser_change_outpoint), + "an IS lock settles the input, so the loser's change must not survive" + ); + assert!( + !ctx.bip44_account().transactions().contains_key(&loser.txid()), + "the loser's record must be dropped too" + ); + } + + /// Abandoning a transaction that never reached the network must take the + /// transactions built on its change with it. This reproduces the shape + /// seen on a testnet device: an asset-lock funding transaction stuck at + /// `Built` whose broadcast never happened, then two further self-sends + /// chained onto its phantom change. Five UTXOs from three transactions, + /// none of which the network ever saw, and the whole chain has to go. + #[tokio::test] + async fn test_abandoning_an_unbroadcast_root_cascades_to_its_descendants() { + let mut ctx = TestWalletContext::new_random(); + let external_address = Address::p2pkh( + &dashcore::PublicKey::from_slice(&[0x02; 33]).expect("pubkey"), + Network::Testnet, + ); + + // A real, confirmed coin funds the chain. + let funding_value = 100_000_000u64; + let funding_tx = Transaction::dummy(&ctx.receive_address, 0..1, &[funding_value]); + ctx.check_transaction( + &funding_tx, + TransactionContext::InBlock(BlockInfo::new( + 100, + BlockHash::from_slice(&[1u8; 32]).expect("hash"), + 1_700_000_000, + )), + ) + .await; + assert_eq!(ctx.managed_wallet.balance.confirmed(), funding_value); + + // Build a three-link chain, each link spending its parent's change. + // Every one of them stays in the mempool: nothing was ever broadcast. + let mut parent = OutPoint { + txid: funding_tx.txid(), + vout: 0, + }; + let mut change_left = funding_value; + let mut chain = Vec::new(); + for (link, sent) in [40_000_000u64, 20_000_000, 5_000_000].into_iter().enumerate() { + let fee = 226u64; + let change = change_left - sent - fee; + let change_address = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .next_change_address(Some(&ctx.xpub), true) + .expect("change address"); + // The tip pays us twice, and nothing spends it onward, so both + // outputs are still live when the cascade runs — exercising the + // per-txid removal loops against a transaction contributing more + // than one UTXO. A filter that dropped only the first would + // otherwise pass every test here. + let split = link == 2; + let (change_a, change_b) = if split { + (change / 2, change - change / 2) + } else { + (change, 0) + }; + let mut outputs = vec![ + TxOut { + value: sent, + script_pubkey: external_address.script_pubkey(), + }, + TxOut { + value: change_a, + script_pubkey: change_address.script_pubkey(), + }, + ]; + if split { + let second = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .next_change_address(Some(&ctx.xpub), true) + .expect("change address"); + outputs.push(TxOut { + value: change_b, + script_pubkey: second.script_pubkey(), + }); + } + let tx = Transaction { + version: 2, + lock_time: 0, + input: vec![TxIn { + previous_output: parent, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }], + output: outputs, + special_transaction_payload: None, + }; + ctx.check_transaction(&tx, TransactionContext::Mempool).await; + parent = OutPoint { + txid: tx.txid(), + vout: 1, + }; + change_left = change; + chain.push(tx); + } + + let root = chain[0].txid(); + // Both live UTXOs belong to the tip, which pays us twice and is spent + // onward by nothing — so the cascade has to drop two UTXOs for that + // one txid rather than assuming one each. The earlier links' change + // is consumed by the next link. + assert_eq!(ctx.bip44_account().utxos.len(), 2, "live change outputs"); + + let outcome = ctx.managed_wallet.abandon_transaction(root); + ctx.managed_wallet.update_balance(); + + assert_eq!( + outcome.abandoned.len(), + 3, + "the root and both descendants must be abandoned, got {:?}", + outcome.abandoned + ); + for tx in &chain { + assert!( + outcome.abandoned.contains(&tx.txid()), + "chain member {} must be abandoned", + tx.txid() + ); + assert!( + !ctx.bip44_account().transactions().contains_key(&tx.txid()), + "chain member {} must lose its record", + tx.txid() + ); + } + assert!(ctx.bip44_account().utxos.is_empty(), "no phantom output may survive the cascade"); + // The load-bearing assertion. Trusted self-send change is bucketed as + // *confirmed*, so `unconfirmed() == 0` holds before the abandon too + // and proves nothing on its own. + assert_eq!( + ctx.managed_wallet.balance.confirmed(), + 0, + "the phantom counts as confirmed, so that is where its absence must show" + ); + assert_eq!(ctx.managed_wallet.balance.unconfirmed(), 0); + + // The real coin the chain consumed is released from the spent set, so + // a rescan can rediscover it — but only while its funding record is + // still live. A chainlock-finalized funding transaction keeps just its + // txid, so `has_transaction` stays true, `is_new` stays false, and + // `confirm_transaction` returns before reaching `update_utxos` — the + // only production insert site. See the sibling test below. + let rediscovered = ctx + .check_transaction( + &funding_tx, + TransactionContext::InBlock(BlockInfo::new( + 100, + BlockHash::from_slice(&[1u8; 32]).expect("hash"), + 1_700_000_000, + )), + ) + .await; + assert!(rediscovered.is_relevant); + assert_eq!( + ctx.managed_wallet.balance.confirmed(), + funding_value, + "the funding coin comes back on rescan — the chain never spent it on chain" + ); + } + + /// A transaction that loses a race for its inputs can never confirm, so + /// the change it contributed is money that does not exist. Nothing else + /// removes it — the loser is in no block, so no block processing revisits + /// it — and it would otherwise sit in the `unconfirmed` bucket forever. + #[tokio::test] + async fn test_conflicting_confirmed_spend_drops_the_losing_transactions_outputs() { + let mut ctx = TestWalletContext::new_random(); + let external_address = Address::p2pkh( + &dashcore::PublicKey::from_slice(&[0x02; 33]).expect("pubkey"), + Network::Testnet, + ); + + // Confirmed funding UTXO. + let funding_value = 1_000_000u64; + let funding_tx = Transaction::dummy(&ctx.receive_address, 0..1, &[funding_value]); + ctx.check_transaction( + &funding_tx, + TransactionContext::InBlock(BlockInfo::new( + 100, + BlockHash::from_slice(&[1u8; 32]).expect("hash"), + 1_700_000_000, + )), + ) + .await; + + let funding_outpoint = OutPoint { + txid: funding_tx.txid(), + vout: 0, + }; + let spend_of = |change_address: &Address, change_amount: u64, sent: u64| Transaction { + version: 2, + lock_time: 0, + input: vec![TxIn { + previous_output: funding_outpoint, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }], + output: vec![ + TxOut { + value: sent, + script_pubkey: external_address.script_pubkey(), + }, + TxOut { + value: change_amount, + script_pubkey: change_address.script_pubkey(), + }, + ], + special_transaction_payload: None, + }; + + // First attempt: broadcast into the mempool, change comes back to us. + let first_change = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .next_change_address(Some(&ctx.xpub), true) + .expect("change address"); + let loser = spend_of(&first_change, 399_000, 600_000); + ctx.check_transaction(&loser, TransactionContext::Mempool).await; + + let loser_change = OutPoint { + txid: loser.txid(), + vout: 1, + }; + assert!( + ctx.bip44_account().utxos.contains_key(&loser_change), + "the first attempt's change is tracked while it is still live" + ); + + // Second attempt spends the same input and confirms in a block. The + // first attempt can now never confirm. + let second_change = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .next_change_address(Some(&ctx.xpub), true) + .expect("change address"); + let winner = spend_of(&second_change, 299_000, 700_000); + ctx.check_transaction( + &winner, + TransactionContext::InBlock(BlockInfo::new( + 101, + BlockHash::from_slice(&[2u8; 32]).expect("hash"), + 1_700_000_100, + )), + ) + .await; + + assert!( + !ctx.bip44_account().utxos.contains_key(&loser_change), + "the losing transaction's change must not survive as spendable money" + ); + assert!( + !ctx.bip44_account().transactions().contains_key(&loser.txid()), + "the losing transaction's record must be dropped too" + ); + + // Only the winner's change remains, and it is the whole balance. + let winner_change = OutPoint { + txid: winner.txid(), + vout: 1, + }; + assert!(ctx.bip44_account().utxos.contains_key(&winner_change)); + // Without the sweep this is 698_000 — the loser's change is trusted + // self-send change and lands in the confirmed bucket, so only the + // exact total catches a regression. + assert_eq!(ctx.managed_wallet.balance.confirmed(), 299_000); + assert_eq!(ctx.managed_wallet.balance.unconfirmed(), 0); + } + + /// Pooled funding spans account families — an asset lock draws from BIP44, + /// BIP32 and the DashPay contact-receiving accounts at once — so the + /// trusted-self-send check must be answered by the whole wallet, not by + /// the single account that happens to hold the change. Here the only input + /// belongs to the BIP32 account while the change lands on BIP44: the + /// transaction is still entirely ours, and its change must be trusted. + #[tokio::test] + async fn test_self_send_change_is_trusted_when_parent_is_in_a_sibling_account() { + let mut ctx = TestWalletContext::new_random(); + let external_address = Address::p2pkh( + &dashcore::PublicKey::from_slice(&[0x02; 33]).expect("pubkey"), + Network::Testnet, + ); + + // Fund the *BIP32* account, confirmed in a block. + let bip32_xpub = ctx + .wallet + .accounts + .standard_bip32_accounts + .get(&0) + .expect("BIP32 account") + .account_xpub; + let bip32_address = ctx + .managed_wallet + .first_bip32_managed_account_mut() + .expect("BIP32 managed account") + .next_receive_address(Some(&bip32_xpub), true) + .expect("BIP32 receive address"); + + let funding_value = 1_000_000u64; + let funding_tx = Transaction::dummy(&bip32_address, 0..1, &[funding_value]); + let block_context = TransactionContext::InBlock(BlockInfo::new( + 100, + BlockHash::from_slice(&[7u8; 32]).expect("hash"), + 1_700_000_000, + )); + ctx.check_transaction(&funding_tx, block_context).await; + assert_eq!(ctx.managed_wallet.balance.confirmed(), funding_value); + + // Change goes to the BIP44 account, which holds none of the inputs. + let change_address = ctx + .managed_wallet + .first_bip44_managed_account_mut() + .expect("account") + .next_change_address(Some(&ctx.xpub), true) + .expect("change address"); + + let send_amount = 600_000u64; + let fee = 1_000u64; + let change_amount = funding_value - send_amount - fee; + let spend_tx = Transaction { + version: 2, + lock_time: 0, + input: vec![TxIn { + previous_output: OutPoint { + txid: funding_tx.txid(), + vout: 0, + }, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }], + output: vec![ + TxOut { + value: send_amount, + script_pubkey: external_address.script_pubkey(), + }, + TxOut { + value: change_amount, + script_pubkey: change_address.script_pubkey(), + }, + ], + special_transaction_payload: None, + }; + + let result = ctx.check_transaction(&spend_tx, TransactionContext::Mempool).await; + assert!(result.is_relevant); + + let change_outpoint = OutPoint { + txid: spend_tx.txid(), + vout: 1, + }; + let change_utxo = + ctx.bip44_account().utxos.get(&change_outpoint).expect("change UTXO recorded"); + assert!(!change_utxo.is_confirmed); + assert!( + change_utxo.is_trusted, + "change of a wallet-owned transfer must be trusted even when the spent \ + parent lives in a sibling account" + ); + + // And therefore it is confirmed, not unconfirmed, in the balance split. + assert_eq!(ctx.managed_wallet.balance.unconfirmed(), 0); + assert_eq!(ctx.managed_wallet.balance.confirmed(), change_amount); + } + /// Sibling of `test_self_send_change_in_mempool_lands_in_confirmed_balance`: /// a self-send change output is only trusted when the spent parent is /// itself final. `Utxo::is_trusted` mirrors Bitcoin Core's diff --git a/key-wallet/src/wallet/managed_wallet_info/helpers.rs b/key-wallet/src/wallet/managed_wallet_info/helpers.rs index 5237f1c5f..8e3842588 100644 --- a/key-wallet/src/wallet/managed_wallet_info/helpers.rs +++ b/key-wallet/src/wallet/managed_wallet_info/helpers.rs @@ -3,10 +3,251 @@ use super::ManagedWalletInfo; use crate::account::account_collection::PlatformPaymentAccountKey; use crate::account::ManagedCoreFundsAccount; +use crate::account::TransactionRecord; +use crate::managed_account::managed_account_ref::ManagedAccountRefMut; +use crate::managed_account::managed_account_trait::ManagedAccountTrait; use crate::managed_account::managed_platform_account::ManagedPlatformAccount; use crate::managed_account::ManagedCoreKeysAccount; +use crate::transaction_checking::TransactionContext; +use crate::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; +use dashcore::{OutPoint, Transaction, Txid}; +use std::collections::{BTreeMap, BTreeSet}; + +/// What [`ManagedWalletInfo::abandon_transaction`] removed. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct AbandonOutcome { + /// Every transaction dropped: the root and its recorded descendants. + pub abandoned: BTreeSet, + /// How many UTXOs those transactions had contributed. + pub utxos_removed: usize, + /// How many transaction records were actually dropped. Distinct from + /// `abandoned.len()`, which counts what was *asked* for. + pub records_removed: usize, +} + +impl AbandonOutcome { + /// Whether anything was actually removed. + /// + /// `abandoned` always contains the root, whether or not the wallet held + /// anything for it, so it cannot answer this on its own — a root the + /// wallet never recorded removes nothing. + pub fn is_empty(&self) -> bool { + self.records_removed == 0 && self.utxos_removed == 0 + } +} + +/// Txids in `records` that spend an output of anything in `abandoned`. +/// +/// Settled records are never followed — what they spent was real. An +/// InstantSend lock settles a transaction against a double spend just as a +/// block does, and `is_confirmed()` does not cover it. +fn collect_spenders_of_records( + records: &std::collections::BTreeMap, + abandoned: &BTreeSet, + into: &mut BTreeSet, +) { + for (txid, record) in records { + if record.is_confirmed() || record.context.is_instant_send() || abandoned.contains(txid) { + continue; + } + if record + .transaction + .input + .iter() + .any(|input| abandoned.contains(&input.previous_output.txid)) + { + into.insert(*txid); + } + } +} impl ManagedWalletInfo { + /// Drop the outputs of every recorded transaction that `tx` provably beat + /// to one of its inputs, across the whole wallet. + /// + /// Wallet-wide on purpose, and deliberately not gated on relevance. Two + /// separate gaps make an account-local, relevance-gated sweep miss the + /// cases that matter: + /// + /// * Pooled funding puts a loser's change in an account the winner never + /// touches, so sweeping only the winner's accounts leaves it credited — + /// and as *trusted* change it is counted confirmed and is spendable. + /// * Relevance is computed from matching outputs and from inputs still + /// present in `utxos`, but the loser already removed the shared input. + /// A winner that spends our coin and pays only external addresses is + /// therefore classified irrelevant, and no account is visited at all. + /// + /// Returns the txids removed, so a caller mirroring wallet state can + /// learn those rows are gone — nothing else in the event surface reports + /// a removal, and a mirror that misses it replays the dead transaction. + pub fn sweep_conflicts(&mut self, tx: &Transaction, context: &TransactionContext) -> Vec { + let mut swept = Vec::new(); + for account in self.accounts.all_accounts_mut() { + if let ManagedAccountRefMut::Funds(funds) = account { + swept.extend(funds.drop_conflicted_transactions(tx, context)); + } + } + if !swept.is_empty() { + self.update_balance(); + // One transaction can be recorded in several accounts, so the + // per-account results overlap. + swept.sort_unstable(); + swept.dedup(); + } + swept + } + + /// Whether any account holds `txid` as settled by the network. + /// + /// Settled means chainlock-finalized, in a block, **or InstantSend-locked** + /// — an IS lock is final against a double spend under DIP-10, so the coins + /// it moved are as irreversibly gone as a block's. `is_confirmed()` covers + /// only the first two, which is why the lock is checked explicitly. + /// + /// A finalized transaction may keep only its txid, so both the retained + /// set and the live record have to be consulted. Keys-only accounts are + /// included: they hold records too, and a settled record there is just as + /// authoritative. + fn transaction_is_settled(&self, txid: &Txid) -> bool { + self.accounts.all_accounts().into_iter().any(|account| { + account.transaction_is_finalized(txid) + || account + .transactions() + .get(txid) + .is_some_and(|r| r.is_confirmed() || r.context.is_instant_send()) + }) + } + + /// Abandon `root` and every recorded transaction descending from it. + /// + /// A transaction the network never accepted still mutated this wallet: + /// its outputs were credited and its inputs marked spent. Nothing reverses + /// that on its own — the transaction is in no block, so no block + /// processing revisits it — and further transactions can be built on its + /// change, each inheriting the same fiction. Left alone the whole chain + /// sits in the `unconfirmed` bucket permanently, as money the wallet + /// displays and does not have. + /// + /// The walk is transitive and wallet-wide — every account that holds + /// records, funds-bearing or keys-only. Pooled funding spreads a + /// transaction's inputs across account families, so a descendant's change + /// can land in an account holding none of the root; and an asset-lock + /// funding transaction is recorded in both its funding account and the + /// identity account it pays. Confirmed and + /// finalized transactions are never followed — they are settled on chain, + /// so whatever they spent was real. + /// + /// **This call asserts that the root is dead; it does not establish it.** + /// The p2p network has no negative signal — modern Dash Core removed BIP61 + /// `reject` — so silence is not proof, and a transaction that merely went + /// quiet may still be live in a miner's mempool. Abandoning such a + /// transaction re-exposes its inputs to coin selection and invites a + /// double-spend. Only call this where the death is known: a build that + /// provably never reached the network, or an explicit user decision. The + /// judgement belongs to the layer that owns broadcast policy. + /// + /// The coins the abandoned transactions consumed are released from the + /// spent set so a rescan can rediscover them, rather than being + /// re-credited directly: the `Utxo` removed for a spent parent is + /// discarded by `update_utxos` and `InputDetail` keeps only + /// index/value/address, so the flags that decide a restored coin's + /// balance bucket are not retained anywhere. + /// + /// Does not recompute the balance — callers batching several abandons + /// should run `update_balance` + /// (from [`WalletInfoInterface`]) + /// once at the end. + pub fn abandon_transaction(&mut self, root: Txid) -> AbandonOutcome { + self.abandon_transaction_with_spends(root, &BTreeMap::new()) + } + + /// [`abandon_transaction`](Self::abandon_transaction), with an external + /// view of who spent what. + /// + /// The descendant walk normally reads recorded transactions, but a caller + /// restoring a wallet may hold UTXOs whose creating transactions were + /// never put back into the in-memory map — leaving the walk unable to see + /// that one abandoned output funded the next transaction along. Callers + /// with a persistence mirror can supply `external_spends`, mapping an + /// outpoint to the transaction that spent it, and the walk follows both. + /// + /// An external spender the wallet holds as confirmed or finalized is + /// **not** followed: the mirror carries no confirmation state of its own, + /// so without this check a stale row could name a settled transaction and + /// have its record and UTXOs deleted. The same guard rejects a confirmed + /// root outright — a transaction in a block spent something real, and + /// nothing built on it is fiction. + pub fn abandon_transaction_with_spends( + &mut self, + root: Txid, + external_spends: &BTreeMap, + ) -> AbandonOutcome { + if self.transaction_is_settled(&root) { + tracing::warn!( + txid = %root, + "refusing to abandon a transaction the wallet holds as settled" + ); + return AbandonOutcome { + abandoned: BTreeSet::new(), + utxos_removed: 0, + records_removed: 0, + }; + } + let mut abandoned = BTreeSet::from([root]); + + // Transitive closure over recorded spenders. Each pass can only add + // txids, and the set is bounded by the recorded transactions, so this + // terminates; a spend cycle is impossible anyway. + loop { + let mut found = BTreeSet::new(); + for account in self.accounts.all_accounts() { + collect_spenders_of_records(account.transactions(), &abandoned, &mut found); + } + // Same step over the external view: anything spending an output of + // an abandoned transaction is itself abandoned. + for (outpoint, spender) in external_spends { + if abandoned.contains(&outpoint.txid) && !self.transaction_is_settled(spender) { + found.insert(*spender); + } + } + let before = abandoned.len(); + abandoned.extend(found); + if abandoned.len() == before { + break; + } + } + + let mut utxos_removed = 0; + let mut records_removed = 0; + for account in self.accounts.all_accounts_mut() { + match account { + ManagedAccountRefMut::Funds(funds) => { + let removed = funds.apply_abandon(&abandoned); + utxos_removed += removed.utxos; + records_removed += removed.records; + } + // Keys-only accounts hold no UTXOs, but they do hold records + // — an asset-lock funding transaction is recorded in both its + // funding account and the identity account it pays. Leaving + // the record here makes `is_new` false on a later re-sighting, + // so the funds account never re-records the transaction and + // never re-marks its input spent. + ManagedAccountRefMut::Keys(keys) => { + for txid in &abandoned { + if keys.transactions_mut().remove(txid).is_some() { + records_removed += 1; + } + } + } + } + } + + AbandonOutcome { + abandoned, + utxos_removed, + records_removed, + } + } // BIP44 Account Helpers /// Get the first BIP44 managed account diff --git a/key-wallet/src/wallet/managed_wallet_info/mod.rs b/key-wallet/src/wallet/managed_wallet_info/mod.rs index 698e3327b..6bfece722 100644 --- a/key-wallet/src/wallet/managed_wallet_info/mod.rs +++ b/key-wallet/src/wallet/managed_wallet_info/mod.rs @@ -7,6 +7,7 @@ pub mod asset_lock_builder; pub mod coin_selection; pub mod fee; pub mod helpers; +pub use helpers::AbandonOutcome; pub mod managed_account_operations; pub mod managed_accounts; pub mod transaction_builder; diff --git a/key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs b/key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs index c2f298d5d..b66293ce0 100644 --- a/key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs +++ b/key-wallet/src/wallet/managed_wallet_info/wallet_info_interface.rs @@ -270,6 +270,14 @@ pub trait WalletInfoInterface: Sized + WalletTransactionChecker + ManagedAccount /// Mark UTXOs for a transaction as InstantSend-locked across all accounts /// and update the corresponding transaction record context. /// Returns `true` if any UTXO was newly marked. + /// Apply an InstantSend lock: mark the transaction's UTXOs, rewrite its + /// record context, and drop any competing spend the lock now settles. + /// + /// Returns whether wallet state changed in any of those ways — callers + /// use it to refresh balances and to decide whether to emit + /// `TransactionInstantLocked`. An outgoing transaction can own no UTXOs + /// of ours and still change state by rewriting its context or by the + /// sweep removing a loser, so this is broader than "a UTXO was marked". fn mark_instant_send_utxos(&mut self, txid: &Txid, lock: &InstantLock) -> bool; /// Return the aggregated monitor revision across all accounts. @@ -580,18 +588,36 @@ impl WalletInfoInterface for ManagedWalletInfo { return false; } let mut any_changed = false; + // Kept for the sweep below: it needs the locked transaction's inputs, + // and this signature carries only its txid. + let mut locked_transaction = None; for mut account in self.accounts.all_accounts_mut() { if account.mark_utxos_instant_send(txid) { any_changed = true; } if let Some(record) = account.transactions_mut().get_mut(txid) { record.update_context(TransactionContext::InstantSend(lock.clone())); + any_changed = true; + if locked_transaction.is_none() { + locked_transaction = Some(record.transaction.clone()); + } } } - if any_changed { + // An IS lock settles this transaction's inputs, so any recorded + // competing spend can never confirm. This is the path the live + // dash-spv pipeline takes for a lock arriving after the transaction is + // already tracked (`process_instant_send_lock`), and it had no sweep — + // the one in `check_core_transaction` is only reachable on a first + // sighting that already carries the lock. + let swept = locked_transaction.is_some_and(|tx| { + !self.sweep_conflicts(&tx, &TransactionContext::InstantSend(lock.clone())).is_empty() + }); + if any_changed && !swept { + // `sweep_conflicts` recomputes on its own when it removes + // something, so this only covers the marking-only case. self.update_balance(); } - any_changed + any_changed || swept } fn monitor_revision(&self) -> u64 {