From 39300ae794b9fb901783bf27523c67771578cf58 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Wed, 19 Aug 2026 15:53:21 -0700 Subject: [PATCH 1/6] fix(key-wallet): correct born-wrong records when a rescan re-attributes outputs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A transaction processed before its beyond-window output's address was derived records that output as Sent (counterparty) with net_amount equal to the full input value. The gap-limit rescan (#820) re-processes the block and update_utxos heals the account's UTXO set — but confirm_transaction only mutated and re-emitted the record when its context changed, so the record and every persistence mirror built from emitted events kept the born-wrong shape forever. A reload from such a mirror is a visible fund loss (kotlin-sdk TXO-store bug, 2026-08-19). update_utxos now reports every output it recognizes as ours (including outputs skipped for insertion because they are already spent on-chain), and confirm_transaction folds that recognition back into the stored record — role flips (Sent -> Received/Change), net_amount and direction recomputed over the completed details — and returns the corrected record so the caller emits it as an updated-record event. Repro: born_wrong_record_is_corrected_by_gap_rescan drives the real filter -> block -> wallet pipeline with a self-send whose second output pays a beyond-window index, and asserts both the in-memory record and the LAST emitted record carry the corrected ownership. Co-Authored-By: Claude Fable 5 --- .../filters/coinjoin_gap_discovery_tests.rs | 175 ++++++++++++++++++ .../managed_core_funds_account.rs | 116 ++++++++++-- 2 files changed, 280 insertions(+), 11 deletions(-) diff --git a/dash-spv/src/sync/filters/coinjoin_gap_discovery_tests.rs b/dash-spv/src/sync/filters/coinjoin_gap_discovery_tests.rs index a2f54d192..0f684ca07 100644 --- a/dash-spv/src/sync/filters/coinjoin_gap_discovery_tests.rs +++ b/dash-spv/src/sync/filters/coinjoin_gap_discovery_tests.rs @@ -411,3 +411,178 @@ async fn coinjoin_gap_limit_stall_across_committed_batch() { derives scripts mid-sync." ); } + +/// Born-wrong `TransactionRecord` is never corrected by the gap rescan +/// (kotlin-sdk "TXO-store reconcile" field bug, 2026-08-19). +/// +/// One block carries a funding tx paying in-window index 0 and a self-send +/// spending it, paying index G-1 (in-window) and index G+10 (beyond the +/// initial watch window). First processing records the self-send with the +/// beyond-window output invisible: `net_amount = PAY - FUND` instead of +/// `-fee`, and no `output_details` entry for vout 1 — a record born wrong, +/// projected as-is into every persistence mirror by the emitted events. +/// Marking G-1 used extends the window past G+10, the commit-time rescan +/// (#820) re-matches the block, and re-processing runs `update_utxos` +/// unconditionally — the account's UTXO set self-heals. But +/// `confirm_transaction` only re-emits (and only mutates) the record when +/// its *context* changed, so neither the in-memory record nor any event +/// carries the correction. On-device this is the CoinJoin-funded-send shape: +/// the store keeps `netAmount` = full input value with the change TXO row +/// missing, and an engine reload from that store makes the funds vanish. +#[tokio::test] +async fn born_wrong_record_is_corrected_by_gap_rescan() { + use dashcore::ScriptBuf; + use key_wallet::managed_account::transaction_record::{OutputRole, TransactionRecord}; + use key_wallet_manager::WalletEvent; + + let (mut manager, wallet, wallet_id) = setup().await; + let addresses = coinjoin_external_addresses(&wallet, &wallet_id, (G + 11) as u32).await; + let mut events_rx = wallet.read().await.subscribe_events(); + + const FUND: u64 = 1_000_100_000; // 10.001 into in-window index 0 + const PAY: u64 = 50_000_000; // 0.5 back to in-window index G-1 + const HIDDEN: u64 = 950_000_000; // 9.5 to beyond-window index G+10 + const FEE: u64 = FUND - PAY - HIDDEN; // 100_000 + + let funding = Transaction { + version: 1, + lock_time: 0, + input: vec![TxIn { + previous_output: OutPoint::new(Txid::from([0xABu8; 32]), 0), + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: Witness::new(), + }], + output: vec![TxOut { + value: FUND, + script_pubkey: addresses[0].script_pubkey(), + }], + special_transaction_payload: None, + }; + let send = Transaction { + version: 1, + lock_time: 0, + input: vec![TxIn { + previous_output: OutPoint::new(funding.txid(), 0), + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: Witness::new(), + }], + output: vec![ + TxOut { + value: PAY, + script_pubkey: addresses[G - 1].script_pubkey(), + }, + TxOut { + value: HIDDEN, + script_pubkey: addresses[G + 10].script_pubkey(), + }, + ], + special_transaction_payload: None, + }; + let send_txid = send.txid(); + let hidden_outpoint = OutPoint::new(send_txid, 1); + + let block = Block::dummy(10, vec![funding, send]); + let filter = BlockFilter::dummy(&block); + let key = FilterMatchKey::new(10, block.block_hash()); + let blocks: HashMap = HashMap::from([(block.block_hash(), block.clone())]); + + let mut batch = FiltersBatch::new(0, 99, HashMap::from([(key, filter)])); + batch.mark_verified(); + manager.active_batches.insert(0, batch); + manager.progress.update_stored_height(99); + + let initial_events = manager.try_process_batch().await.unwrap(); + drive_to_quiescence(&mut manager, &wallet, &blocks, initial_events).await; + + { + let wm = wallet.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("wallet info present"); + let account = + info.coinjoin_managed_account_at_index(0).expect("CoinJoin account 0 present"); + + // Engine UTXO self-heal — already correct in the field. If this + // fails, the rescan re-processing itself regressed, which is a + // different bug than the one this test pins. + assert!( + account.utxos.contains_key(&hidden_outpoint), + "rescan re-processing must insert the beyond-window output into the \ + account UTXO set (update_utxos runs unconditionally in confirm_transaction)" + ); + + // The engine's own record must be corrected by the same re-processing. + // Note the born-wrong shape is not a MISSING entry: the first + // processing classified the unattributable output as `Sent` + // (counterparty), so the correction is a role flip. + let record = + account.transactions().get(&send_txid).expect("send record present in account"); + assert!( + record + .output_details + .iter() + .any(|o| o.index == 1 + && matches!(o.role, OutputRole::Received | OutputRole::Change)), + "the re-processed record must classify the beyond-window output (vout 1) as \ + ours (Received/Change); a lingering Sent role means every store projection \ + derived from this record drops the TXO. got: {:?}", + record.output_details, + ); + assert_eq!( + record.net_amount, + -(FEE as i64), + "the re-processed record's net_amount must be recomputed from the now-\ + complete ownership view (self-send nets -fee); the born-wrong value \ + PAY-FUND = {} is what makes restored balances collapse on reload", + PAY as i64 - FUND as i64, + ); + } + + // The persistence mirror is built exclusively from the emitted events: + // whatever the last record-bearing event said about this txid is what + // every store on every platform now holds. + let mut last_record: Option = None; + while let Ok(event) = events_rx.try_recv() { + match event { + WalletEvent::TransactionDetected { + record, + .. + } if record.txid == send_txid => { + last_record = Some(*record); + } + WalletEvent::BlockProcessed { + inserted, + updated, + .. + } => { + for r in inserted.into_iter().chain(updated) { + if r.txid == send_txid { + last_record = Some(r); + } + } + } + _ => {} + } + } + let projected = last_record.expect("an event must have carried the send record"); + assert!( + projected + .output_details + .iter() + .any(|o| o.index == 1 && matches!(o.role, OutputRole::Received | OutputRole::Change)), + "the LAST emitted record for the send must classify the beyond-window output \ + (vout 1) as ours — the store mirrors are built from these events only, and \ + without a corrective emission (confirm_transaction returning None when only \ + ownership knowledge changed) every mirror keeps the born-wrong Sent role. \ + got: {:?}", + projected.output_details, + ); + assert_eq!( + projected.net_amount, + -(FEE as i64), + "the LAST emitted record's net_amount must be the corrected value; the field \ + stores show the born-wrong {} shape (full input value) because no corrective \ + event ever fires", + PAY as i64 - FUND as i64, + ); +} 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 ca3210bb6..602f8598a 100644 --- a/key-wallet/src/managed_account/managed_core_funds_account.rs +++ b/key-wallet/src/managed_account/managed_core_funds_account.rs @@ -237,6 +237,14 @@ impl ManagedCoreFundsAccount { /// 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. + /// Returns an [`OutputDetail`] for every output of `tx` this call + /// recognized as paying this account — including outputs whose UTXO + /// insertion was skipped because an earlier-processed block already + /// spent them (they are still ours and still belong on the record). + /// `record_transaction` ignores the return value (its details come from + /// the same match); `confirm_transaction` uses it to detect ownership + /// learned only on re-processing — the gap-limit rescan shape — and fold + /// the correction back into the already-stored record. fn update_utxos( &mut self, tx: &Transaction, @@ -244,7 +252,8 @@ impl ManagedCoreFundsAccount { context: TransactionContext, observed_spent: &BTreeMap, external_final_parents: &BTreeSet, - ) { + ) -> Vec { + let mut recognized: Vec = Vec::new(); // Update UTXOs only for spendable account types match self.keys.managed_account_type() { ManagedAccountType::Standard { @@ -320,7 +329,9 @@ impl ManagedCoreFundsAccount { %txid, "Not crediting a transaction whose input a block already spent" ); - return; + // Nothing recognized either: a doomed transaction's + // outputs must not be folded into any record. + return recognized; } let network = self.keys.network(); @@ -334,6 +345,21 @@ impl ManagedCoreFundsAccount { vout: vout as u32, }; + // Report recognition before the spent-skips below: + // an output of ours that is already spent on-chain + // still belongs on the transaction's record and in + // its net_amount. + recognized.push(OutputDetail { + index: vout as u32, + role: if change_addrs.contains(&addr) { + OutputRole::Change + } else { + OutputRole::Received + }, + address: Some(addr.clone()), + value: output.value, + }); + // Check if this outpoint was already spent by a transaction we've seen. // This handles out-of-order block processing during rescan where a // spending transaction at a higher height may be processed before @@ -422,6 +448,7 @@ impl ManagedCoreFundsAccount { } _ => {} } + recognized } /// Drop the spent-marks that `freed` contributed, keeping every mark a @@ -767,14 +794,6 @@ impl ManagedCoreFundsAccount { } } - // Capture the (possibly updated) record before any pruning so the - // caller can still emit it in an event. - let record_after = if changed { - self.keys.transactions().get(&txid).cloned() - } else { - None - }; - // The chainlock is the trigger for dropping the full record under // the default feature configuration; an IS-lock alone is *not* // enough — we keep the record so the surrounding block @@ -782,7 +801,82 @@ 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, external_final_parents); + let recognized = + self.update_utxos(tx, account_match, context, observed_spent, external_final_parents); + + // Gap-limit rescan correction: a re-processed block can recognize + // outputs the FIRST processing could not see (their addresses were + // beyond the watch window then, derived since). `update_utxos` above + // already healed the account's UTXO set; without the block below the + // stored record — and every persistence mirror built from emitted + // records — keeps the born-wrong shape forever: `net_amount` equal to + // the full input value and no `output_details` entry for the output. + // A reload from such a mirror is exactly the field fund-loss this + // corrects (kotlin-sdk TXO-store bug, 2026-08-19). Merge the newly + // recognized outputs into the record, recompute the derived fields, + // and signal the caller so a corrective event is emitted. + if let Some(tx_record) = self.keys.transactions_mut().get_mut(&txid) { + // An output the first processing could not attribute is not + // absent from the record — `record_transaction` classified it as + // `Sent` (counterparty). Recognition therefore corrects roles in + // place as well as filling genuine gaps. + let mut corrected = false; + for ours in recognized { + match tx_record.output_details.iter_mut().find(|o| o.index == ours.index) { + Some(existing) => { + if existing.role != ours.role { + *existing = ours; + corrected = true; + } + } + None => { + tx_record.output_details.push(ours); + corrected = true; + } + } + } + if corrected { + tx_record.output_details.sort_by_key(|o| o.index); + // Same derivations as `record_transaction`, over the now- + // complete details. `input_details` was built at first + // processing while the spent parents were still live, so the + // spend side needs no recomputation. + let owned: i64 = tx_record + .output_details + .iter() + .filter(|o| matches!(o.role, OutputRole::Received | OutputRole::Change)) + .map(|o| o.value as i64) + .sum(); + let spent: i64 = + tx_record.input_details.iter().map(|i| i.value as i64).sum(); + tx_record.net_amount = owned - spent; + let has_inputs = !tx_record.input_details.is_empty(); + let has_sent = + tx_record.output_details.iter().any(|d| d.role == OutputRole::Sent); + let has_our_outputs = tx_record + .output_details + .iter() + .any(|d| matches!(d.role, OutputRole::Received | OutputRole::Change)); + if tx_record.direction != TransactionDirection::CoinJoin { + tx_record.direction = if !has_sent && has_inputs && has_our_outputs { + TransactionDirection::Internal + } else if has_inputs { + TransactionDirection::Outgoing + } else { + TransactionDirection::Incoming + }; + } + changed = true; + } + } + + // Capture the (possibly updated) record before any pruning so the + // caller can still emit it in an event. + let record_after = if changed { + self.keys.transactions().get(&txid).cloned() + } else { + None + }; #[cfg(not(feature = "keep-finalized-transactions"))] if drop_now { self.keys.drop_finalized_transaction(&txid); From 80e07b8f68314c2caa9f919fb1efff7660ca2645 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Wed, 19 Aug 2026 16:22:35 -0700 Subject: [PATCH 2/6] =?UTF-8?q?fix(dash-spv):=20durable=20pending-sweep=20?= =?UTF-8?q?set=20=E2=80=94=20replay=20interrupted=20script=20rescans=20aft?= =?UTF-8?q?er=20restart?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Scripts derived during block processing carry a rescan obligation (forward over active batches, backward over the committed range — #820/#846). That obligation lived only in memory: a process death between derivation and the cascade's COMMIT orphaned it, and nothing in a restarted session ever looks below the committed boundary again — outputs paying those scripts stay invisible to the engine forever (the interrupted-restore fund loss, 2026-08-19; on Android, LMK kills make interrupted syncs the common case). The pending-sweep set is now mirrored to metadata storage: persisted the moment scripts enter the manager (before any sweep work), re-seeded into the lowest active batch after a restart (the ordinary commit-time cascade then owns it), and cleared per batch COMMIT against the batch's retired-scripts receipt — the only point that proves the whole fixpoint completed. Opt-in via FiltersManager::with_metadata; managers without it keep the previous in-memory behavior. Repro: interrupted_sweep_is_replayed_after_restart — cross-committed-batch shape, session 1 dropped right after the scripts are derived, session 2 over the same storage recovers the committed-range outputs. Co-Authored-By: Claude Fable 5 --- dash-spv/src/client/lifecycle.rs | 6 + dash-spv/src/sync/filters/batch.rs | 21 +- .../filters/coinjoin_gap_discovery_tests.rs | 158 ++++++++++++++ dash-spv/src/sync/filters/manager.rs | 194 ++++++++++++++++++ dash-spv/src/sync/filters/sync_manager.rs | 6 + 5 files changed, 384 insertions(+), 1 deletion(-) diff --git a/dash-spv/src/client/lifecycle.rs b/dash-spv/src/client/lifecycle.rs index 46e26f71c..683df8f7d 100644 --- a/dash-spv/src/client/lifecycle.rs +++ b/dash-spv/src/client/lifecycle.rs @@ -106,6 +106,12 @@ impl DashSpvClient>, + /// Every script this batch has taken through its rescan cascade + /// (forward + backward), retained until commit. This is the receipt the + /// durable pending-sweep set is cleared against: only a COMMIT proves + /// the whole fixpoint (including blocks the sweep re-downloaded) + /// completed, so scripts must stay persisted until then — a crash + /// mid-cascade replays them on the next start. + retired_scripts: HashMap>, } impl FiltersBatch { @@ -63,6 +70,7 @@ impl FiltersBatch { scanned_wallets: BTreeMap::new(), collected_scripts: HashMap::new(), backward_scripts: HashMap::new(), + retired_scripts: HashMap::new(), } } /// Start height of this batch (inclusive). @@ -127,8 +135,14 @@ impl FiltersBatch { self.collected_scripts.entry(wallet_id).or_default().extend(scripts); } /// Take collected per-wallet scripts for rescan, leaving the map empty. + /// The taken scripts are also recorded in [`Self::retired_scripts`] so + /// the commit can clear them from the durable pending-sweep set. pub(super) fn take_collected_scripts(&mut self) -> HashMap> { - std::mem::take(&mut self.collected_scripts) + let taken = std::mem::take(&mut self.collected_scripts); + for (wallet_id, scripts) in &taken { + self.retired_scripts.entry(*wallet_id).or_default().extend(scripts.iter().cloned()); + } + taken } /// Queue already forward-rescanned scripts for the deferred backward /// sweep over the committed range. @@ -145,6 +159,11 @@ impl FiltersBatch { pub(super) fn take_backward_scripts(&mut self) -> HashMap> { std::mem::take(&mut self.backward_scripts) } + /// Every script this batch carried through its rescan cascade — see the + /// field docs; read at commit to clear the durable pending-sweep set. + pub(super) fn retired_scripts(&self) -> &HashMap> { + &self.retired_scripts + } /// Record the wallets that were behind for this batch at scan time, each /// with its `account_generation` snapshot. pub(super) fn set_scanned_wallets(&mut self, wallets: BTreeMap) { diff --git a/dash-spv/src/sync/filters/coinjoin_gap_discovery_tests.rs b/dash-spv/src/sync/filters/coinjoin_gap_discovery_tests.rs index 0f684ca07..fe7bfdec0 100644 --- a/dash-spv/src/sync/filters/coinjoin_gap_discovery_tests.rs +++ b/dash-spv/src/sync/filters/coinjoin_gap_discovery_tests.rs @@ -412,6 +412,164 @@ async fn coinjoin_gap_limit_stall_across_committed_batch() { ); } +/// Cross-committed-batch discovery survives a process death between script +/// derivation and the backward sweep's completion (the durable pending-sweep +/// set; interrupted-restore fund loss, 2026-08-19). +/// +/// Same funding shape as [`coinjoin_gap_limit_stall_across_committed_batch`]: +/// block A (height 10, batch 0..=99) pays beyond-window indices G+10..=G+21; +/// block B (height 110, batch 100..=199) pays in-window indices 0..=29. +/// Batch 0 scans clean and COMMITS. Processing block B derives the missing +/// scripts — at which point the session "crashes": the manager is dropped +/// before the backward sweep's re-downloaded block is processed. The +/// in-memory cascade is gone, batch 0 is committed, and nothing in a +/// restarted session would ever look below the committed boundary again. +/// +/// The durable pending-sweep set closes this: the scripts were persisted to +/// metadata storage the moment they entered the manager, a restarted manager +/// (same storage, `with_metadata`) reloads them, seeds them into its lowest +/// active batch, and the ordinary commit-time cascade re-tests the stored +/// filters below the committed boundary — recovering block A's outputs. +#[tokio::test] +async fn interrupted_sweep_is_replayed_after_restart() { + // Shared across the "restart": wallet (its pools/synced heights persist + // in the real system via the SDK store) and disk storage (headers, + // filters, metadata). + let mut wm = WalletManager::::new(Network::Regtest); + let wallet_id = wm + .create_wallet_from_mnemonic(TEST_MNEMONIC, 0, WalletAccountCreationOptions::Default) + .expect("create deterministic test wallet"); + let wallet = Arc::new(RwLock::new(wm)); + let storage = DiskStorageManager::with_temp_dir().await.unwrap(); + + let addresses = coinjoin_external_addresses(&wallet, &wallet_id, (G + 22) as u32).await; + let (block_a, filter_a, key_a) = block_paying(10, &addresses[(G + 10)..=(G + 21)]); + let (block_b, filter_b, key_b) = block_paying(110, &addresses[0..=29]); + let blocks: HashMap = HashMap::from([ + (block_a.block_hash(), block_a.clone()), + (block_b.block_hash(), block_b), + ]); + + // Persist headers+filters for the to-be-committed range (see the + // invariant note in the cross-committed-batch test). + { + let headers_arc = storage.block_headers(); + let filters_arc = storage.filters(); + let mut header_storage = headers_arc.write().await; + let mut filter_storage = filters_arc.write().await; + for height in 0..=99u32 { + let (header, filter_bytes) = if height == 10 { + (block_a.header, filter_a.content.clone()) + } else { + let filler = Block::dummy(height, vec![]); + let filter = BlockFilter::dummy(&filler); + (filler.header, filter.content) + }; + header_storage + .store_headers_at_height(&[header.into()], height) + .await + .expect("seed header"); + filter_storage.store_filter(height, &filter_bytes).await.expect("seed filter"); + } + } + + // ── Session 1: batch 0 commits clean, block B derives scripts, CRASH ── + { + let mut manager = FiltersManager::new( + Arc::clone(&wallet), + storage.block_headers(), + storage.filter_headers(), + storage.filters(), + ) + .await + .with_metadata(storage.metadata()) + .await; + manager.set_state(SyncState::Syncing); + + let mut batch_0 = FiltersBatch::new(0, 99, HashMap::from([(key_a, filter_a.clone())])); + batch_0.mark_verified(); + manager.active_batches.insert(0, batch_0); + let mut batch_1 = + FiltersBatch::new(100, 199, HashMap::from([(key_b.clone(), filter_b.clone())])); + batch_1.mark_verified(); + manager.active_batches.insert(100, batch_1); + manager.progress.update_stored_height(199); + + // Initial pass: batch 0 matches nothing and commits; batch 1 + // requests block B. + let initial_events = manager.try_process_batch().await.unwrap(); + let needed: Vec<(u32, BlockHash, BTreeSet)> = initial_events + .iter() + .filter_map(|e| match e { + SyncEvent::BlocksNeeded { + blocks: needed, + } => Some(needed.iter().map(|(k, w)| (k.height(), *k.hash(), w.clone()))), + _ => None, + }) + .flatten() + .collect(); + assert_eq!(needed.len(), 1, "only block B should be requested initially"); + + // Process block B once — derives the beyond-window scripts, which + // the manager must persist durably at this exact moment. + let (tx, _rx) = unbounded_channel(); + let requests = RequestSender::new(tx); + for (height, block_hash, wallets) in needed { + let block = blocks.get(&block_hash).expect("known block"); + let result = wallet + .write() + .await + .process_block_for_wallets(block, block_hash, height, &wallets) + .await; + let confirmed_txids = result.relevant_txids().cloned().collect(); + let event = SyncEvent::BlockProcessed { + block_hash, + height, + wallets, + new_scripts: result.new_scripts, + confirmed_txids, + }; + // The returned events (the sweep's own BlocksNeeded for block A + // among them) are deliberately DROPPED: the process dies here. + let _ = manager.handle_sync_event(&event, &requests).await.expect("BlockProcessed"); + } + // manager dropped — in-memory cascade gone. + } + + // ── Session 2: fresh manager over the same storage ── + let mut manager = FiltersManager::new( + Arc::clone(&wallet), + storage.block_headers(), + storage.filter_headers(), + storage.filters(), + ) + .await + .with_metadata(storage.metadata()) + .await; + manager.set_state(SyncState::Syncing); + + // Production resume recreates batches from the committed frontier up; + // batch 0 is committed (synced_height advanced), so only batch 1 exists. + let mut batch_1 = FiltersBatch::new(100, 199, HashMap::from([(key_b, filter_b)])); + batch_1.mark_verified(); + manager.active_batches.insert(100, batch_1); + manager.progress.update_stored_height(199); + + let initial_events = manager.try_process_batch().await.unwrap(); + drive_to_quiescence(&mut manager, &wallet, &blocks, initial_events).await; + + let (highest_used, highest_generated, used_count) = + coinjoin_pool_state(&wallet, &wallet_id).await; + assert_eq!( + highest_used, + Some((G + 21) as u32), + "block A's outputs sit in a batch that committed before their scripts were \ + derived, and the session died before the backward sweep finished — only the \ + durable pending-sweep set can bring them back after the restart. \ + highest_generated={highest_generated:?}, used_count={used_count}" + ); +} + /// Born-wrong `TransactionRecord` is never corrected by the gap rescan /// (kotlin-sdk "TXO-store reconcile" field bug, 2026-08-19). /// diff --git a/dash-spv/src/sync/filters/manager.rs b/dash-spv/src/sync/filters/manager.rs index 5fc18c5f3..387bc07c3 100644 --- a/dash-spv/src/sync/filters/manager.rs +++ b/dash-spv/src/sync/filters/manager.rs @@ -91,6 +91,76 @@ pub struct FiltersManager< /// `BlockProcessed` and the per-wallet record of which wallets already /// have a given processed block applied. pub(super) tracker: BlockMatchTracker, + + // === Durable pending-sweep state (interrupted-sync recovery) === + /// Scripts derived during block processing whose rescan cascade has not + /// yet COMMITTED. The in-memory cascade (`collected_scripts` → + /// `backward_scripts` → sweep → commit) evaporates on process death, and + /// nothing ever re-tests already-scanned heights against scripts derived + /// after them — outputs paying those scripts stay invisible to the + /// engine forever (the interrupted-restore fund loss, 2026-08-19). + /// Mirrored to [`Self::metadata`] when present: added when scripts enter + /// the manager, cleared per batch commit via the batch's retired-scripts + /// receipt, and re-seeded into the lowest active batch after a restart. + pending_sweep: HashMap>, + /// The batch start the current `pending_sweep` content has been seeded + /// into, so seeding is idempotent per batch. A tick-triggered rescan + /// wipes `active_batches`, after which the new lowest batch gets its own + /// seeding pass. + pending_seeded_into: Option, + /// Durable store for `pending_sweep`. `None` (tests without storage, + /// callers that never opt in) degrades to the previous in-memory-only + /// behavior. + metadata: Option>>, +} + +/// Metadata key holding the encoded pending-sweep set. +const PENDING_SWEEP_KEY: &str = "filters_pending_sweep"; + +/// Encode the pending-sweep set: `[u32 wallet_count]`, then per wallet +/// `[32-byte wallet id][u32 script_count]` followed by `[u32 len][bytes]` +/// per script. All integers little-endian. Sorted iteration so identical +/// sets encode identically. +fn encode_pending_sweep(pending: &HashMap>) -> Vec { + let mut out = Vec::new(); + let wallets: BTreeMap<&WalletId, &HashSet> = pending.iter().collect(); + out.extend((wallets.len() as u32).to_le_bytes()); + for (wallet_id, scripts) in wallets { + out.extend(*wallet_id); + let mut sorted: Vec<&ScriptBuf> = scripts.iter().collect(); + sorted.sort(); + out.extend((sorted.len() as u32).to_le_bytes()); + for script in sorted { + out.extend((script.len() as u32).to_le_bytes()); + out.extend(script.as_bytes()); + } + } + out +} + +/// Decode [`encode_pending_sweep`]'s format. Any structural inconsistency +/// yields `None` — the caller treats a corrupt blob as "no pending sweep", +/// which fails toward a missed recovery rather than a crash loop. +fn decode_pending_sweep(bytes: &[u8]) -> Option>> { + let mut cursor = 0usize; + let mut read = |n: usize| -> Option<&[u8]> { + let slice = bytes.get(cursor..cursor + n)?; + cursor += n; + Some(slice) + }; + let wallet_count = u32::from_le_bytes(read(4)?.try_into().ok()?) as usize; + let mut pending = HashMap::new(); + for _ in 0..wallet_count { + let wallet_id: WalletId = read(32)?.try_into().ok()?; + let script_count = u32::from_le_bytes(read(4)?.try_into().ok()?) as usize; + let mut scripts = HashSet::new(); + for _ in 0..script_count { + let len = u32::from_le_bytes(read(4)?.try_into().ok()?) as usize; + scripts.insert(ScriptBuf::from(read(len)?.to_vec())); + } + pending.insert(wallet_id, scripts); + } + Some(pending) } impl @@ -135,6 +205,77 @@ impl>, + ) -> Self { + use crate::storage::MetadataStorage; + match metadata.read().await.load_metadata(PENDING_SWEEP_KEY).await { + Ok(Some(bytes)) => match decode_pending_sweep(&bytes) { + Some(pending) => { + if !pending.is_empty() { + tracing::info!( + wallets = pending.len(), + scripts = pending.values().map(|s| s.len()).sum::(), + "Recovered pending script sweep from a previous session; \ + already-scanned heights will be re-tested for these scripts" + ); + } + self.pending_sweep = pending; + } + None => { + tracing::warn!( + "Corrupt pending-sweep metadata; discarding (a wallet whose \ + discovery was interrupted may need a manual rescan)" + ); + } + }, + Ok(None) => {} + Err(e) => { + tracing::warn!(error = %e, "Failed to load pending-sweep metadata"); + } + } + self.metadata = Some(metadata); + self + } + + /// Record scripts entering the rescan cascade into the durable + /// pending-sweep set. Persisted BEFORE any sweep work runs, so a crash + /// anywhere in the cascade replays them next session. + pub(super) async fn note_pending_sweep( + &mut self, + wallet_id: WalletId, + scripts: impl IntoIterator, + ) { + let entry = self.pending_sweep.entry(wallet_id).or_default(); + let before = entry.len(); + entry.extend(scripts); + if entry.len() != before { + self.persist_pending_sweep().await; + } + } + + /// Write the current pending-sweep set through to metadata storage (a + /// no-op without [`Self::metadata`]). Failures are logged, not fatal: + /// the in-memory cascade still runs; only crash durability degrades. + async fn persist_pending_sweep(&self) { + let Some(metadata) = &self.metadata else { + return; + }; + use crate::storage::MetadataStorage; + let bytes = encode_pending_sweep(&self.pending_sweep); + if let Err(e) = metadata.write().await.store_metadata(PENDING_SWEEP_KEY, &bytes).await { + tracing::warn!(error = %e, "Failed to persist pending-sweep metadata"); } } @@ -508,6 +649,34 @@ impl SyncResult> { let mut events = Vec::new(); + // Phase 0: Seed a recovered pending sweep into the lowest active + // batch. Feeding the scripts through `add_scripts_for_wallet` hands + // them to the existing commit-time cascade — forward rescan of this + // and later batches, then the backward sweep over the committed + // range — so an interrupted session's obligation is honored by the + // same fixpoint that handles freshly derived scripts. Idempotent per + // batch via `pending_seeded_into`; the durable set is only cleared + // when the seeded batch COMMITS (see `try_commit_batches`). + if !self.pending_sweep.is_empty() { + if let Some((&lowest_start, _)) = self.active_batches.first_key_value() { + if self.pending_seeded_into != Some(lowest_start) { + tracing::info!( + wallets = self.pending_sweep.len(), + scripts = self.pending_sweep.values().map(|s| s.len()).sum::(), + batch_start = lowest_start, + "Seeding recovered pending script sweep into the lowest active batch" + ); + let pending = self.pending_sweep.clone(); + if let Some(batch) = self.active_batches.get_mut(&lowest_start) { + for (wallet_id, scripts) in pending { + batch.add_scripts_for_wallet(wallet_id, scripts); + } + } + self.pending_seeded_into = Some(lowest_start); + } + } + } + // Phase 1: Commit completed batches in order events.extend(self.try_commit_batches().await?); @@ -636,6 +805,31 @@ impl self.progress.committed_height() { self.progress.update_committed_height(end); diff --git a/dash-spv/src/sync/filters/sync_manager.rs b/dash-spv/src/sync/filters/sync_manager.rs index 2dbd8ad5e..9b4855ca8 100644 --- a/dash-spv/src/sync/filters/sync_manager.rs +++ b/dash-spv/src/sync/filters/sync_manager.rs @@ -200,6 +200,12 @@ impl< if scripts.is_empty() { continue; } + // Durable first: persist the sweep obligation before + // the in-memory cascade takes it, so a crash anywhere + // between here and the batch's COMMIT replays these + // scripts next session instead of orphaning heights + // scanned before the scripts existed. + self.note_pending_sweep(*wallet_id, scripts.iter().cloned()).await; if let Some(batch) = self.active_batches.get_mut(&batch_start) { batch.add_scripts_for_wallet(*wallet_id, scripts.iter().cloned()); } From afba7afe313fa2ad68dfdbb93087b6fc21f63d14 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Wed, 19 Aug 2026 18:00:32 -0700 Subject: [PATCH 3/6] =?UTF-8?q?feat(key-wallet):=20AddressPool::ensure=5Fc?= =?UTF-8?q?ontiguous=5Fto=20=E2=80=94=20repair=20sparse=20pools=20by=20re-?= =?UTF-8?q?derivation?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A pool restored from a persistence mirror can be sparse: mirrors have been observed dropping individual address rows (2026-08-19 field wallet: BIP44-change rows missing right past the used frontier), and a restore that ingests surviving rows as-is inherits the holes while the row-derived highest_generated watermark suppresses the gap-limit maintenance that would re-derive them. An address missing from the pool makes every output paying it permanently unrecognizable — a rescan-proof fund loss. Derivation is pure key arithmetic, so holes are always repairable: derive every missing index in 0..=index, leave existing entries and used flags untouched. Co-Authored-By: Claude Fable 5 --- .../src/managed_account/address_pool.rs | 82 +++++++++++++++++++ 1 file changed, 82 insertions(+) diff --git a/key-wallet/src/managed_account/address_pool.rs b/key-wallet/src/managed_account/address_pool.rs index 57c021bed..5e496d159 100644 --- a/key-wallet/src/managed_account/address_pool.rs +++ b/key-wallet/src/managed_account/address_pool.rs @@ -448,6 +448,29 @@ impl AddressPool { self.pool_type == AddressPoolType::External } + /// Derive every missing index in `0..=index`, leaving existing entries + /// (and their used flags) untouched. Returns how many holes were filled. + /// + /// A pool restored from a persistence mirror can be SPARSE: mirrors have + /// been observed dropping individual address rows, and a restore that + /// ingests the surviving rows as-is both inherits the holes and (by + /// advancing `highest_generated` to the max persisted index) suppresses + /// the gap-limit maintenance that would otherwise re-derive them. An + /// address missing from the pool makes every output paying it + /// permanently unrecognizable — funds invisible on every rescan. Since + /// derivation is pure arithmetic over the key source, holes are always + /// repairable; restore paths call this after ingesting persisted rows. + pub fn ensure_contiguous_to(&mut self, index: u32, key_source: &KeySource) -> Result { + let mut filled = 0u32; + for idx in 0..=index { + if !self.addresses.contains_key(&idx) { + self.generate_address_at_index(idx, key_source, true)?; + filled += 1; + } + } + Ok(filled) + } + /// Generate addresses up to the specified count pub fn generate_addresses( &mut self, @@ -1367,6 +1390,65 @@ mod tests { assert_eq!(pool.addresses.len(), 10); } + /// A sparse pool (as restored from a lossy persistence mirror) must be + /// repairable to a contiguous one without disturbing surviving entries: + /// holes are re-derived, existing entries and their used flags are kept, + /// and the filled addresses match what direct derivation produces (so + /// recognition of outputs paying formerly-holed addresses works). + #[test] + fn test_ensure_contiguous_fills_holes_and_preserves_used() { + let base_path = DerivationPath::from(vec![ChildNumber::from_normal_idx(0).unwrap()]); + let key_source = test_key_source(); + + // The reference: a contiguous pool 0..=9. + let mut reference = AddressPool::new_without_generation( + base_path.clone(), + AddressPoolType::External, + 20, + Network::Testnet, + ); + reference.generate_addresses(10, &key_source, true).unwrap(); + + // The victim: same pool, then indices 3, 4, 7 dropped (mirror holes) + // with `highest_generated` still claiming full coverage — the exact + // post-restore shape. + let mut sparse = AddressPool::new_without_generation( + base_path, + AddressPoolType::External, + 20, + Network::Testnet, + ); + sparse.generate_addresses(10, &key_source, true).unwrap(); + let used_addr = sparse.addresses[&5].address.clone(); + sparse.mark_used(&used_addr); + for idx in [3u32, 4, 7] { + let info = sparse.addresses.remove(&idx).unwrap(); + sparse.address_index.remove(&info.address); + sparse.script_pubkey_index.remove(&info.script_pubkey); + } + assert_eq!(sparse.addresses.len(), 7); + assert_eq!(sparse.highest_generated, Some(9), "watermark still claims coverage"); + + let filled = sparse.ensure_contiguous_to(9, &key_source).unwrap(); + assert_eq!(filled, 3, "exactly the three holes are re-derived"); + assert_eq!(sparse.addresses.len(), 10); + for idx in 0..=9u32 { + assert_eq!( + sparse.addresses.get(&idx).map(|i| &i.address), + reference.addresses.get(&idx).map(|i| &i.address), + "index {idx} must match direct derivation" + ); + assert!( + sparse.address_index.contains_key(&reference.addresses[&idx].address), + "reverse lookup must cover index {idx}" + ); + } + assert!(sparse.used_indices.contains(&5), "used flag survives the repair"); + + // Idempotent: a second pass finds nothing to fill. + assert_eq!(sparse.ensure_contiguous_to(9, &key_source).unwrap(), 0); + } + #[test] fn test_address_usage() { let base_path = DerivationPath::from(vec![ChildNumber::from_normal_idx(0).unwrap()]); From 530c178ffa7a959fbac09c286a1b358600275752 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Thu, 20 Aug 2026 10:13:32 -0700 Subject: [PATCH 4/6] fix(key-wallet): attribute born-spent funding outputs onto their spenders' records MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Out-of-order block processing during rescan/discovery can process a SPENDER before the transaction that funded it. At spender-process time the input is unknown, so the record is born income-only (net_amount = +received) — the inflated-history shape: a 2026-08-20 field restore showed 49 such rows (47 CoinJoin mixing rounds recorded as +one-denomination income) summing to +4.63 DASH of phantom net over the true balance. The UTXO set stayed correct (#649 observed-spends / spent-outpoint skips), but nothing ever revisited the spender's RECORD, so engine history and every persistence mirror kept the one-sided net until a full rescan happened to re-process the spender. The moment the missing attribution is provable is exactly the existing born-spent skip branches in update_utxos: the funding output is recognized as ours and already spent by a previously-processed transaction. Both branches now attribute the input onto the spender's record (index, value, address), recompute net_amount/direction via the shared TransactionRecord::recompute_net_and_direction (Layer-2's output-side correction refactored onto the same helper), and stage the corrected record; wallet_checker drains it into updated_records so the event pipeline re-emits the correction to the stores. Repro: born_spent_attribution_corrects_out_of_order_spender — spender processed first pins the income-only shape, funding processed second must correct the record in-engine AND surface it in updated_records. Negative-controlled: with the hooks disabled the test fails. Co-Authored-By: Claude Fable 5 --- .../managed_account/managed_account_ref.rs | 12 ++ .../managed_core_funds_account.rs | 120 +++++++++++++----- .../src/managed_account/transaction_record.rs | 33 +++++ .../transaction_checking/wallet_checker.rs | 109 ++++++++++++++++ 4 files changed, 245 insertions(+), 29 deletions(-) diff --git a/key-wallet/src/managed_account/managed_account_ref.rs b/key-wallet/src/managed_account/managed_account_ref.rs index 172d40327..b1c7ec70c 100644 --- a/key-wallet/src/managed_account/managed_account_ref.rs +++ b/key-wallet/src/managed_account/managed_account_ref.rs @@ -415,6 +415,18 @@ impl<'a> ManagedAccountRefMut<'a> { ManagedAccountRefMut::Keys(_) => false, } } + + /// Drain spender records corrected by born-spent input attribution + /// during the last `record_transaction` / `confirm_transaction` call + /// (out-of-order funding — see + /// `ManagedCoreFundsAccount::attribute_born_spent_output`). Always + /// empty for the [`Keys`](Self::Keys) variant. + pub(crate) fn take_corrected_spender_records(&mut self) -> Vec { + match self { + ManagedAccountRefMut::Funds(a) => a.take_corrected_spender_records(), + ManagedAccountRefMut::Keys(_) => Vec::new(), + } + } } /// Owned managed core account, either funds-bearing or keys-only. 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 602f8598a..abec2357b 100644 --- a/key-wallet/src/managed_account/managed_core_funds_account.rs +++ b/key-wallet/src/managed_account/managed_core_funds_account.rs @@ -68,6 +68,18 @@ pub struct ManagedCoreFundsAccount { /// re-establish which coins are spent. #[cfg_attr(feature = "serde", serde(skip))] reservations: ReservationSet, + /// Spender records corrected as a side effect of processing a FUNDING + /// transaction after its spender (out-of-order block processing during + /// rescan/discovery): the spender was recorded with the spent input + /// unattributed (`net_amount` = income only — the inflated-history + /// shape, 2026-08-20 field wallet: 49 one-sided rows, +4.63 DASH of + /// phantom net). `update_utxos` patches the spender's record the moment + /// the funding output is seen born-spent and stages the corrected copy + /// here; `wallet_checker` drains it into `updated_records` so an event + /// re-emits the correction to every persistence mirror. Transient + /// working state, never persisted. + #[cfg_attr(feature = "serde", serde(skip))] + corrected_spender_records: Vec, } /// What [`ManagedCoreFundsAccount::apply_abandon`] removed from one account. @@ -104,6 +116,7 @@ impl ManagedCoreFundsAccount { utxos: BTreeMap::new(), spent_outpoints: HashSet::new(), reservations: ReservationSet::default(), + corrected_spender_records: Vec::new(), } } @@ -131,6 +144,7 @@ impl ManagedCoreFundsAccount { utxos: BTreeMap::new(), spent_outpoints: HashSet::new(), reservations: ReservationSet::default(), + corrected_spender_records: Vec::new(), } } @@ -372,6 +386,19 @@ impl ManagedCoreFundsAccount { outpoint = %outpoint, "Skipping UTXO already spent by previously processed transaction" ); + // Born-spent: the spender ran BEFORE this + // funding transaction, so its record was + // built with this input unattributed + // (income-only net — the inflated-history + // shape). This is the one moment the missing + // attribution is provable; patch the + // spender's record and stage it for + // re-emission. + self.attribute_born_spent_output( + &outpoint, + output.value, + addr.clone(), + ); continue; } @@ -384,6 +411,15 @@ impl ManagedCoreFundsAccount { outpoint = %outpoint, "Skipping UTXO already observed spent in an earlier-processed block (#649)" ); + // Same born-spent shape via the wallet-level + // observed-spends view: if this account also + // holds the spender's record, its spent side + // is missing this input too. + self.attribute_born_spent_output( + &outpoint, + output.value, + addr.clone(), + ); continue; } @@ -451,6 +487,55 @@ impl ManagedCoreFundsAccount { recognized } + /// Late input attribution for a spender processed before its funding + /// transaction (the born-spent hooks in [`Self::update_utxos`]). Finds + /// the recorded transaction spending `outpoint`, attributes the input + /// (index, value, address), recomputes the record's derived fields, and + /// stages the corrected copy in `corrected_spender_records` for the + /// caller to drain into an updated-records event. No-op when no record + /// spends the outpoint (spend observed from a transaction this account + /// never recorded, or a finalized record already dropped) or when the + /// input is already attributed. + fn attribute_born_spent_output(&mut self, outpoint: &OutPoint, value: u64, address: Address) { + let spender = self.keys.transactions().iter().find_map(|(txid, rec)| { + rec.transaction + .input + .iter() + .position(|i| &i.previous_output == outpoint) + .map(|pos| (*txid, pos as u32)) + }); + let Some((spender_txid, input_index)) = spender else { + return; + }; + let Some(record) = self.keys.transactions_mut().get_mut(&spender_txid) else { + return; + }; + if record.input_details.iter().any(|d| d.index == input_index) { + return; + } + record.input_details.push(InputDetail { + index: input_index, + value, + address, + }); + record.input_details.sort_by_key(|d| d.index); + record.recompute_net_and_direction(); + tracing::info!( + outpoint = %outpoint, + spender = %spender_txid, + corrected_net = record.net_amount, + "Attributed born-spent funding output onto its spender's record" + ); + let corrected = record.clone(); + self.corrected_spender_records.push(corrected); + } + + /// Drain the spender records corrected by [`Self::update_utxos`]'s + /// born-spent attribution since the last drain. + pub(crate) fn take_corrected_spender_records(&mut self) -> Vec { + std::mem::take(&mut self.corrected_spender_records) + } + /// Drop the spent-marks that `freed` contributed, keeping every mark a /// surviving record still claims. /// @@ -837,35 +922,11 @@ impl ManagedCoreFundsAccount { } if corrected { tx_record.output_details.sort_by_key(|o| o.index); - // Same derivations as `record_transaction`, over the now- - // complete details. `input_details` was built at first - // processing while the spent parents were still live, so the - // spend side needs no recomputation. - let owned: i64 = tx_record - .output_details - .iter() - .filter(|o| matches!(o.role, OutputRole::Received | OutputRole::Change)) - .map(|o| o.value as i64) - .sum(); - let spent: i64 = - tx_record.input_details.iter().map(|i| i.value as i64).sum(); - tx_record.net_amount = owned - spent; - let has_inputs = !tx_record.input_details.is_empty(); - let has_sent = - tx_record.output_details.iter().any(|d| d.role == OutputRole::Sent); - let has_our_outputs = tx_record - .output_details - .iter() - .any(|d| matches!(d.role, OutputRole::Received | OutputRole::Change)); - if tx_record.direction != TransactionDirection::CoinJoin { - tx_record.direction = if !has_sent && has_inputs && has_our_outputs { - TransactionDirection::Internal - } else if has_inputs { - TransactionDirection::Outgoing - } else { - TransactionDirection::Incoming - }; - } + // Shared derivation over the now-complete details. + // `input_details` was built at first processing while the + // spent parents were still live, so the spend side needs no + // recomputation here. + tx_record.recompute_net_and_direction(); changed = true; } } @@ -1420,6 +1481,7 @@ impl<'de> Deserialize<'de> for ManagedCoreFundsAccount { utxos: helper.utxos, spent_outpoints, reservations: ReservationSet::default(), + corrected_spender_records: Vec::new(), }) } } diff --git a/key-wallet/src/managed_account/transaction_record.rs b/key-wallet/src/managed_account/transaction_record.rs index b51aee6f1..536220ae5 100644 --- a/key-wallet/src/managed_account/transaction_record.rs +++ b/key-wallet/src/managed_account/transaction_record.rs @@ -131,6 +131,39 @@ impl TransactionRecord { } } + /// Recompute `net_amount` and `direction` from the CURRENT + /// `input_details` / `output_details` — the single derivation both + /// late-attribution paths share: output-side corrections (a rescan + /// newly recognizing an output the first processing recorded as + /// `Sent`) and input-side corrections (a funding transaction processed + /// AFTER its spender attributing the spent input onto the spender's + /// record). `net_amount` is Σ(owned outputs) − Σ(attributed inputs); + /// `direction` follows `record_transaction`'s rules, except a + /// `CoinJoin` direction is never overwritten (it is keyed to the + /// transaction type, not the flow shape). + pub fn recompute_net_and_direction(&mut self) { + let owned: i64 = self + .output_details + .iter() + .filter(|o| matches!(o.role, OutputRole::Received | OutputRole::Change)) + .map(|o| o.value as i64) + .sum(); + let spent: i64 = self.input_details.iter().map(|i| i.value as i64).sum(); + self.net_amount = owned - spent; + if self.direction != TransactionDirection::CoinJoin { + let has_inputs = !self.input_details.is_empty(); + let has_sent = self.output_details.iter().any(|d| d.role == OutputRole::Sent); + let has_our_outputs = owned > 0; + self.direction = if !has_sent && has_inputs && has_our_outputs { + TransactionDirection::Internal + } else if has_inputs { + TransactionDirection::Outgoing + } else { + TransactionDirection::Incoming + }; + } + } + /// Calculate the number of confirmations based on current chain height pub fn confirmations(&self, current_height: u32) -> u32 { match self.context.block_info() { diff --git a/key-wallet/src/transaction_checking/wallet_checker.rs b/key-wallet/src/transaction_checking/wallet_checker.rs index efea71a5f..eafde9aa8 100644 --- a/key-wallet/src/transaction_checking/wallet_checker.rs +++ b/key-wallet/src/transaction_checking/wallet_checker.rs @@ -232,6 +232,19 @@ impl WalletTransactionChecker for ManagedWalletInfo { } } + // Born-spent input attribution: processing THIS transaction may + // have corrected the records of EARLIER-processed spenders of + // its outputs (out-of-order funding during rescan/discovery). + // Surface the corrections as updated records so an event + // re-emits them to the persistence mirrors — without this the + // engine's records are right but every store keeps the + // income-only net (the inflated-history shape). + let corrected = account.take_corrected_spender_records(); + if !corrected.is_empty() { + result.state_modified = true; + result.updated_records.extend(corrected); + } + for address_info in account_match.account_type_match.all_involved_addresses() { account.mark_address_used(&address_info.address); } @@ -3592,4 +3605,100 @@ mod tests { assert_eq!(ctx.managed_wallet.balance.confirmed(), 0); assert_eq!(ctx.managed_wallet.balance.unconfirmed(), payment_value); } + + /// Out-of-order funding (rescan block-download order): the SPENDER is + /// processed before the transaction that funded it. At spender-process + /// time its input is unknown, so the record is born income-only + /// (`net_amount = +received` — the inflated-history shape: 2026-08-20 + /// field wallet showed 49 such rows summing +4.63 DASH of phantom net). + /// When the funding transaction is finally processed, its born-spent + /// output must be attributed onto the spender's record — corrected net, + /// input_details filled — and the corrected record must surface in + /// `updated_records` so an event re-emits it to persistence mirrors. + #[tokio::test] + async fn born_spent_attribution_corrects_out_of_order_spender() { + let mut ctx = TestWalletContext::new_random(); + const FUND: u64 = 1_000_000; + const BACK: u64 = 900_000; + + let funding_tx = Transaction::dummy(&ctx.receive_address, 0..1, &[FUND]); + let funded_outpoint = OutPoint { + txid: funding_tx.txid(), + vout: 0, + }; + let spender_tx = Transaction { + version: 1, + lock_time: 0, + input: vec![TxIn { + previous_output: funded_outpoint, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }], + output: vec![TxOut { + value: BACK, + script_pubkey: ctx.receive_address.script_pubkey(), + }], + special_transaction_payload: None, + }; + let spender_txid = spender_tx.txid(); + + // Spender first (height 2 processed before height 1). + let spender_context = TransactionContext::InBlock(BlockInfo::new( + 2, + BlockHash::from_slice(&[3u8; 32]).expect("block hash"), + 1_650_000_100, + )); + let result = ctx.check_transaction(&spender_tx, spender_context).await; + assert!(result.is_relevant && result.is_new_transaction); + { + let account = + ctx.managed_wallet.first_bip44_managed_account().expect("bip44 account"); + let record = account.transactions().get(&spender_txid).expect("spender recorded"); + assert_eq!( + record.net_amount, BACK as i64, + "pin the born-wrong shape: income-only net before the funding tx arrives" + ); + assert!(record.input_details.is_empty()); + } + + // Funding second — out of order. + let funding_context = TransactionContext::InBlock(BlockInfo::new( + 1, + BlockHash::from_slice(&[2u8; 32]).expect("block hash"), + 1_650_000_000, + )); + let result = ctx.check_transaction(&funding_tx, funding_context).await; + assert!(result.is_relevant); + + // The engine record is corrected... + { + let account = + ctx.managed_wallet.first_bip44_managed_account().expect("bip44 account"); + let record = account.transactions().get(&spender_txid).expect("spender record"); + assert_eq!( + record.net_amount, + BACK as i64 - FUND as i64, + "spent side attributed: net = received - spent" + ); + assert_eq!(record.input_details.len(), 1); + assert_eq!(record.input_details[0].value, FUND); + // The born-spent funding output must NOT enter the UTXO set. + assert!( + !account.utxos.contains_key(&funded_outpoint), + "a born-spent output never becomes spendable" + ); + } + + // ...AND the correction surfaces as an updated record for the event + // pipeline — without this every persistence mirror keeps the + // income-only net forever. + let corrected = result + .updated_records + .iter() + .find(|r| r.txid == spender_txid) + .expect("corrected spender record must surface in updated_records"); + assert_eq!(corrected.net_amount, BACK as i64 - FUND as i64); + assert_eq!(corrected.input_details.len(), 1); + } } From c3e8e0eddf6348d731d7719d1ad2a3d0a3647b9b Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Thu, 20 Aug 2026 11:56:26 -0700 Subject: [PATCH 5/6] feat(key-wallet): expose the account's spent-outpoint set for store reconciliation MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Read-only accessor pairing with the utxos map: together they let a persistence-mirror audit classify a store row marked unspent — in the spent set means the row lost its spend update (dashpay/platform#4425, safe to flip); in neither inventory means swept/abandoned residue (pre-rust-dashcore#971 stores). Co-Authored-By: Claude Fable 5 --- .../src/managed_account/managed_core_funds_account.rs | 11 +++++++++++ 1 file changed, 11 insertions(+) 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 abec2357b..e61eec8eb 100644 --- a/key-wallet/src/managed_account/managed_core_funds_account.rs +++ b/key-wallet/src/managed_account/managed_core_funds_account.rs @@ -203,6 +203,17 @@ impl ManagedCoreFundsAccount { self.spent_outpoints.contains(outpoint) } + /// The outpoints this account knows were spent by recorded transactions. + /// Read-only view for store reconciliation: a persistence-mirror row + /// still marked unspent whose outpoint appears here lost its spend + /// update (the dashpay/platform#4425 stale-TXO class) and can safely be + /// flipped; an unspent mirror row appearing in NEITHER this set nor + /// [`Self::utxos`] is residue of a swept/abandoned transaction + /// (pre-rust-dashcore#971 stores). + pub fn spent_outpoints(&self) -> &HashSet { + &self.spent_outpoints + } + /// Collect the outpoints among `tx`'s inputs that this account holds as a /// final UTXO — confirmed, InstantSend-locked, or trusted. /// From 9a68e6528072523eb210a5338bcb0edace9ff453 Mon Sep 17 00:00:00 2001 From: HashEngineering Date: Thu, 20 Aug 2026 19:23:54 -0700 Subject: [PATCH 6/6] =?UTF-8?q?fix(key-wallet,=20dash-spv):=20review=20rou?= =?UTF-8?q?nd=20=E2=80=94=20wallet-scope=20born-spent=20attribution,=20res?= =?UTF-8?q?can=20reseed,=20bounds?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CodeRabbit review round on #979: - Born-spent attribution now runs at WALLET scope: update_utxos stages the born-spent outputs, and wallet_checker sweeps every fund account for matching spender records — patching all of them, not the first match in the funding account. A spender recorded in a sibling account (it matched wherever its own outputs landed) was previously never corrected. New cross-account regression: born_spent_attribution_reaches_sibling_account_spenders. - reset_for_rescan clears pending_seeded_into: the discarded batches took the pending sweep's in-memory copy with them, and the recreated batch frequently starts at the same height — the stale marker made the seeding guard skip the replay for the whole session. - ensure_contiguous_to bounds the repair span (1M): a corrupt restored watermark must refuse, not stall the load deriving billions of addresses. - decode_pending_sweep uses checked cursor arithmetic — a corrupt blob must not overflow on 32-bit targets. - spent_outpoints() docs: absence proves nothing (finalized records drop, the set rebuilds from survivors) — positive membership is the only safe reconciliation signal. - born-wrong pipeline test doc rewritten to state the pinned guarantee; pool-repair test asserts state/index/script-map invariants directly. Co-Authored-By: Claude Fable 5 --- .../filters/coinjoin_gap_discovery_tests.rs | 20 +-- dash-spv/src/sync/filters/manager.rs | 12 +- .../src/managed_account/address_pool.rs | 33 ++++ .../managed_account/managed_account_ref.rs | 30 +++- .../managed_core_funds_account.rs | 160 +++++++++-------- .../transaction_checking/wallet_checker.rs | 164 ++++++++++++++++-- 6 files changed, 312 insertions(+), 107 deletions(-) diff --git a/dash-spv/src/sync/filters/coinjoin_gap_discovery_tests.rs b/dash-spv/src/sync/filters/coinjoin_gap_discovery_tests.rs index fe7bfdec0..c7e8299c9 100644 --- a/dash-spv/src/sync/filters/coinjoin_gap_discovery_tests.rs +++ b/dash-spv/src/sync/filters/coinjoin_gap_discovery_tests.rs @@ -445,10 +445,8 @@ async fn interrupted_sweep_is_replayed_after_restart() { let addresses = coinjoin_external_addresses(&wallet, &wallet_id, (G + 22) as u32).await; let (block_a, filter_a, key_a) = block_paying(10, &addresses[(G + 10)..=(G + 21)]); let (block_b, filter_b, key_b) = block_paying(110, &addresses[0..=29]); - let blocks: HashMap = HashMap::from([ - (block_a.block_hash(), block_a.clone()), - (block_b.block_hash(), block_b), - ]); + let blocks: HashMap = + HashMap::from([(block_a.block_hash(), block_a.clone()), (block_b.block_hash(), block_b)]); // Persist headers+filters for the to-be-committed range (see the // invariant note in the cross-committed-batch test). @@ -570,8 +568,10 @@ async fn interrupted_sweep_is_replayed_after_restart() { ); } -/// Born-wrong `TransactionRecord` is never corrected by the gap rescan -/// (kotlin-sdk "TXO-store reconcile" field bug, 2026-08-19). +/// A born-wrong `TransactionRecord` IS corrected by the gap rescan — both +/// in the account and in the emitted event stream. Pins the fix for the +/// kotlin-sdk "TXO-store reconcile" field bug (2026-08-19), where the +/// correction never happened: /// /// One block carries a funding tx paying in-window index 0 and a self-send /// spending it, paying index G-1 (in-window) and index G+10 (beyond the @@ -676,11 +676,9 @@ async fn born_wrong_record_is_corrected_by_gap_rescan() { let record = account.transactions().get(&send_txid).expect("send record present in account"); assert!( - record - .output_details - .iter() - .any(|o| o.index == 1 - && matches!(o.role, OutputRole::Received | OutputRole::Change)), + record.output_details.iter().any( + |o| o.index == 1 && matches!(o.role, OutputRole::Received | OutputRole::Change) + ), "the re-processed record must classify the beyond-window output (vout 1) as \ ours (Received/Change); a lingering Sent role means every store projection \ derived from this record drops the TXO. got: {:?}", diff --git a/dash-spv/src/sync/filters/manager.rs b/dash-spv/src/sync/filters/manager.rs index 387bc07c3..9587d989f 100644 --- a/dash-spv/src/sync/filters/manager.rs +++ b/dash-spv/src/sync/filters/manager.rs @@ -144,8 +144,9 @@ fn encode_pending_sweep(pending: &HashMap>) -> Vec< fn decode_pending_sweep(bytes: &[u8]) -> Option>> { let mut cursor = 0usize; let mut read = |n: usize| -> Option<&[u8]> { - let slice = bytes.get(cursor..cursor + n)?; - cursor += n; + let end = cursor.checked_add(n)?; + let slice = bytes.get(cursor..end)?; + cursor = end; Some(slice) }; let wallet_count = u32::from_le_bytes(read(4)?.try_into().ok()?) as usize; @@ -299,6 +300,13 @@ impl Result { + // A restored watermark is untrusted input: a corrupt row could name + // an index in the billions and turn the repair into an unbounded + // derivation stall during wallet load. Real pools top out in the + // low thousands (heaviest observed field wallet: 8,281); anything + // past this bound is corruption, and refusing restores the pool + // exactly as persisted — the pre-repair behavior. + const MAX_REPAIR_INDEX: u32 = 1_000_000; + if index > MAX_REPAIR_INDEX { + return Err(Error::InvalidParameter(format!( + "refusing address-pool hole repair to index {index}: exceeds the {MAX_REPAIR_INDEX} repair bound (corrupt watermark?)" + ))); + } let mut filled = 0u32; for idx in 0..=index { if !self.addresses.contains_key(&idx) { @@ -1444,6 +1456,27 @@ mod tests { ); } assert!(sparse.used_indices.contains(&5), "used flag survives the repair"); + assert_eq!( + sparse.addresses[&5].state, + AddressState::Used, + "the surviving entry's state is untouched" + ); + for idx in [3u32, 4, 7] { + let info = &sparse.addresses[&idx]; + assert_eq!( + sparse.address_index.get(&info.address), + Some(&idx), + "repaired index {idx} maps back through the address index" + ); + assert_eq!( + sparse.script_pubkey_index.get(&info.script_pubkey), + Some(&idx), + "repaired index {idx} maps back through the script index" + ); + assert_eq!(info.state, AddressState::Available, "a repaired hole starts Available"); + } + // The bound refuses implausible watermarks instead of stalling. + assert!(sparse.ensure_contiguous_to(u32::MAX, &key_source).is_err()); // Idempotent: a second pass finds nothing to fill. assert_eq!(sparse.ensure_contiguous_to(9, &key_source).unwrap(), 0); diff --git a/key-wallet/src/managed_account/managed_account_ref.rs b/key-wallet/src/managed_account/managed_account_ref.rs index b1c7ec70c..692a540f2 100644 --- a/key-wallet/src/managed_account/managed_account_ref.rs +++ b/key-wallet/src/managed_account/managed_account_ref.rs @@ -416,14 +416,28 @@ impl<'a> ManagedAccountRefMut<'a> { } } - /// Drain spender records corrected by born-spent input attribution - /// during the last `record_transaction` / `confirm_transaction` call - /// (out-of-order funding — see - /// `ManagedCoreFundsAccount::attribute_born_spent_output`). Always - /// empty for the [`Keys`](Self::Keys) variant. - pub(crate) fn take_corrected_spender_records(&mut self) -> Vec { - match self { - ManagedAccountRefMut::Funds(a) => a.take_corrected_spender_records(), + /// Drain the born-spent funding outputs staged during the last + /// `record_transaction` / `confirm_transaction` call (out-of-order + /// funding — see `ManagedCoreFundsAccount::take_born_spent_outputs`). + /// Always empty for the [`Keys`](Self::Keys) variant. + pub(crate) fn take_born_spent_outputs(&mut self) -> Vec<(OutPoint, u64, Address)> { + match self { + ManagedAccountRefMut::Funds(a) => a.take_born_spent_outputs(), + ManagedAccountRefMut::Keys(_) => Vec::new(), + } + } + + /// Attribute a born-spent funding output onto every recorded spender in + /// this account — the wallet-scope half of the out-of-order correction. + /// Always empty for the [`Keys`](Self::Keys) variant. + pub(crate) fn attribute_spent_input( + &mut self, + outpoint: &OutPoint, + value: u64, + address: &Address, + ) -> Vec { + match self { + ManagedAccountRefMut::Funds(a) => a.attribute_spent_input(outpoint, value, address), ManagedAccountRefMut::Keys(_) => Vec::new(), } } 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 e61eec8eb..611d55310 100644 --- a/key-wallet/src/managed_account/managed_core_funds_account.rs +++ b/key-wallet/src/managed_account/managed_core_funds_account.rs @@ -68,18 +68,20 @@ pub struct ManagedCoreFundsAccount { /// re-establish which coins are spent. #[cfg_attr(feature = "serde", serde(skip))] reservations: ReservationSet, - /// Spender records corrected as a side effect of processing a FUNDING - /// transaction after its spender (out-of-order block processing during - /// rescan/discovery): the spender was recorded with the spent input - /// unattributed (`net_amount` = income only — the inflated-history - /// shape, 2026-08-20 field wallet: 49 one-sided rows, +4.63 DASH of - /// phantom net). `update_utxos` patches the spender's record the moment - /// the funding output is seen born-spent and stages the corrected copy - /// here; `wallet_checker` drains it into `updated_records` so an event - /// re-emits the correction to every persistence mirror. Transient - /// working state, never persisted. + /// Born-spent funding outputs staged by [`Self::update_utxos`]: this + /// account recognized an output of a FUNDING transaction that an + /// earlier-processed transaction already spent (out-of-order block + /// processing during rescan/discovery). The spender was recorded with + /// that input unattributed (`net_amount` = income only — the + /// inflated-history shape, 2026-08-20 field wallet: 49 one-sided rows, + /// +4.63 DASH of phantom net) — and the spender's record may live in a + /// SIBLING account (it matched wherever its own outputs landed), so the + /// correction must run at wallet scope: `wallet_checker` drains this + /// staging and calls [`Self::attribute_spent_input`] on every fund + /// account, emitting each corrected record. Transient working state, + /// never persisted. #[cfg_attr(feature = "serde", serde(skip))] - corrected_spender_records: Vec, + born_spent_outputs: Vec<(OutPoint, u64, Address)>, } /// What [`ManagedCoreFundsAccount::apply_abandon`] removed from one account. @@ -116,7 +118,7 @@ impl ManagedCoreFundsAccount { utxos: BTreeMap::new(), spent_outpoints: HashSet::new(), reservations: ReservationSet::default(), - corrected_spender_records: Vec::new(), + born_spent_outputs: Vec::new(), } } @@ -144,7 +146,7 @@ impl ManagedCoreFundsAccount { utxos: BTreeMap::new(), spent_outpoints: HashSet::new(), reservations: ReservationSet::default(), - corrected_spender_records: Vec::new(), + born_spent_outputs: Vec::new(), } } @@ -204,12 +206,18 @@ impl ManagedCoreFundsAccount { } /// The outpoints this account knows were spent by recorded transactions. - /// Read-only view for store reconciliation: a persistence-mirror row - /// still marked unspent whose outpoint appears here lost its spend - /// update (the dashpay/platform#4425 stale-TXO class) and can safely be - /// flipped; an unspent mirror row appearing in NEITHER this set nor - /// [`Self::utxos`] is residue of a swept/abandoned transaction - /// (pre-rust-dashcore#971 stores). + /// Read-only view for store reconciliation. POSITIVE membership is the + /// only safe signal: a mirror row still marked unspent whose outpoint + /// appears here lost its spend update (the dashpay/platform#4425 + /// stale-TXO class) and can be flipped. + /// + /// ABSENCE proves nothing. Under the default feature configuration, + /// finalized (chain-locked) transactions drop their full records and + /// this set is rebuilt from the records that remain, so a genuinely + /// spent outpoint can be missing from BOTH this set and [`Self::utxos`] + /// after a reload. A reconciler must treat "in neither inventory" as + /// ambiguous — swept/abandoned residue (pre-rust-dashcore#971 stores) + /// OR a finalized spend whose record is gone — and never mutate on it. pub fn spent_outpoints(&self) -> &HashSet { &self.spent_outpoints } @@ -405,11 +413,11 @@ impl ManagedCoreFundsAccount { // attribution is provable; patch the // spender's record and stage it for // re-emission. - self.attribute_born_spent_output( - &outpoint, + self.born_spent_outputs.push(( + outpoint, output.value, addr.clone(), - ); + )); continue; } @@ -426,11 +434,11 @@ impl ManagedCoreFundsAccount { // observed-spends view: if this account also // holds the spender's record, its spent side // is missing this input too. - self.attribute_born_spent_output( - &outpoint, + self.born_spent_outputs.push(( + outpoint, output.value, addr.clone(), - ); + )); continue; } @@ -498,53 +506,65 @@ impl ManagedCoreFundsAccount { recognized } - /// Late input attribution for a spender processed before its funding - /// transaction (the born-spent hooks in [`Self::update_utxos`]). Finds - /// the recorded transaction spending `outpoint`, attributes the input - /// (index, value, address), recomputes the record's derived fields, and - /// stages the corrected copy in `corrected_spender_records` for the - /// caller to drain into an updated-records event. No-op when no record - /// spends the outpoint (spend observed from a transaction this account - /// never recorded, or a finalized record already dropped) or when the - /// input is already attributed. - fn attribute_born_spent_output(&mut self, outpoint: &OutPoint, value: u64, address: Address) { - let spender = self.keys.transactions().iter().find_map(|(txid, rec)| { - rec.transaction - .input - .iter() - .position(|i| &i.previous_output == outpoint) - .map(|pos| (*txid, pos as u32)) - }); - let Some((spender_txid, input_index)) = spender else { - return; - }; - let Some(record) = self.keys.transactions_mut().get_mut(&spender_txid) else { - return; - }; - if record.input_details.iter().any(|d| d.index == input_index) { - return; + /// Attribute a born-spent funding output onto EVERY recorded + /// transaction in THIS account that spends `outpoint` (a conflicting + /// double-spend can leave more than one). Called at wallet scope by + /// `wallet_checker` for each staged born-spent output, across all fund + /// accounts — the spender's record lives wherever its own outputs + /// matched, which need not be the account that owns the funding output. + /// Patches the input detail (index, value, address), recomputes the + /// record's derived fields, and returns the corrected copies for + /// re-emission. No-op for records already attributed and for accounts + /// holding no matching record (a finalized record already dropped + /// cannot be patched). + pub(crate) fn attribute_spent_input( + &mut self, + outpoint: &OutPoint, + value: u64, + address: &Address, + ) -> Vec { + let spenders: Vec<(Txid, u32)> = self + .keys + .transactions() + .iter() + .filter_map(|(txid, rec)| { + rec.transaction + .input + .iter() + .position(|i| &i.previous_output == outpoint) + .map(|pos| (*txid, pos as u32)) + }) + .collect(); + let mut corrected = Vec::new(); + for (spender_txid, input_index) in spenders { + let Some(record) = self.keys.transactions_mut().get_mut(&spender_txid) else { + continue; + }; + if record.input_details.iter().any(|d| d.index == input_index) { + continue; + } + record.input_details.push(InputDetail { + index: input_index, + value, + address: address.clone(), + }); + record.input_details.sort_by_key(|d| d.index); + record.recompute_net_and_direction(); + tracing::info!( + outpoint = %outpoint, + spender = %spender_txid, + corrected_net = record.net_amount, + "Attributed born-spent funding output onto its spender's record" + ); + corrected.push(record.clone()); } - record.input_details.push(InputDetail { - index: input_index, - value, - address, - }); - record.input_details.sort_by_key(|d| d.index); - record.recompute_net_and_direction(); - tracing::info!( - outpoint = %outpoint, - spender = %spender_txid, - corrected_net = record.net_amount, - "Attributed born-spent funding output onto its spender's record" - ); - let corrected = record.clone(); - self.corrected_spender_records.push(corrected); + corrected } - /// Drain the spender records corrected by [`Self::update_utxos`]'s - /// born-spent attribution since the last drain. - pub(crate) fn take_corrected_spender_records(&mut self) -> Vec { - std::mem::take(&mut self.corrected_spender_records) + /// Drain the born-spent outputs staged by [`Self::update_utxos`] since + /// the last drain, for the wallet-scope attribution sweep. + pub(crate) fn take_born_spent_outputs(&mut self) -> Vec<(OutPoint, u64, Address)> { + std::mem::take(&mut self.born_spent_outputs) } /// Drop the spent-marks that `freed` contributed, keeping every mark a @@ -1492,7 +1512,7 @@ impl<'de> Deserialize<'de> for ManagedCoreFundsAccount { utxos: helper.utxos, spent_outpoints, reservations: ReservationSet::default(), - corrected_spender_records: Vec::new(), + born_spent_outputs: Vec::new(), }) } } diff --git a/key-wallet/src/transaction_checking/wallet_checker.rs b/key-wallet/src/transaction_checking/wallet_checker.rs index eafde9aa8..8a8b230d3 100644 --- a/key-wallet/src/transaction_checking/wallet_checker.rs +++ b/key-wallet/src/transaction_checking/wallet_checker.rs @@ -195,6 +195,7 @@ impl WalletTransactionChecker for ManagedWalletInfo { } // Process each affected account + let mut born_spent: Vec<(dashcore::OutPoint, u64, dashcore::Address)> = Vec::new(); for account_match in result.affected_accounts.clone() { let Some(mut account) = self.accounts.get_by_account_type_match_mut(&account_match.account_type_match) @@ -232,18 +233,13 @@ impl WalletTransactionChecker for ManagedWalletInfo { } } - // Born-spent input attribution: processing THIS transaction may - // have corrected the records of EARLIER-processed spenders of - // its outputs (out-of-order funding during rescan/discovery). - // Surface the corrections as updated records so an event - // re-emits them to the persistence mirrors — without this the - // engine's records are right but every store keeps the - // income-only net (the inflated-history shape). - let corrected = account.take_corrected_spender_records(); - if !corrected.is_empty() { - result.state_modified = true; - result.updated_records.extend(corrected); - } + // Born-spent staging: processing THIS transaction may have + // revealed outputs an earlier-processed transaction already + // spent (out-of-order funding during rescan/discovery). Collect + // them here; the attribution sweep runs at wallet scope after + // this loop, because the spender's record lives wherever ITS + // outputs matched — not necessarily in this account. + born_spent.extend(account.take_born_spent_outputs()); for address_info in account_match.account_type_match.all_involved_addresses() { account.mark_address_used(&address_info.address); @@ -283,6 +279,26 @@ impl WalletTransactionChecker for ManagedWalletInfo { } } + // Wallet-scope born-spent attribution (out-of-order funding): each + // staged output's spender may be recorded in ANY fund account, and a + // conflicting double-spend can leave several records. Runs only when + // something was staged — the common path never touches the full + // account list. The corrections surface as updated records so an + // event re-emits them to the persistence mirrors; without this the + // engine's records are right but every store keeps the income-only + // net (the inflated-history shape). + if !born_spent.is_empty() { + for (outpoint, value, address) in &born_spent { + for mut account in self.accounts.all_accounts_mut() { + let corrected = account.attribute_spent_input(outpoint, *value, address); + if !corrected.is_empty() { + result.state_modified = true; + result.updated_records.extend(corrected); + } + } + } + } + if is_new { // Populate dedup sets when a tx arrives with an initial IS status if context.is_instant_send() { @@ -3606,6 +3622,124 @@ mod tests { assert_eq!(ctx.managed_wallet.balance.unconfirmed(), payment_value); } + /// Cross-account variant of the born-spent correction: the funding + /// output belongs to the BIP44 account, but the spender's record lives + /// in the COINJOIN account (it matched there via its own outputs when + /// processed first, its input unknown). The attribution must run at + /// wallet scope — an account-local lookup finds no spender and the + /// CoinJoin record keeps its income-only net forever. + #[tokio::test] + async fn born_spent_attribution_reaches_sibling_account_spenders() { + let network = Network::Testnet; + let mut wallet = + Wallet::new_random(network, WalletAccountCreationOptions::Default).expect("wallet"); + let mut managed_wallet = + ManagedWalletInfo::from_wallet_with_name(&wallet, "Test".to_string(), 0); + + // Funding pays the BIP44 receive address (account A). + let bip44_xpub = + wallet.accounts.standard_bip44_accounts.get(&0).expect("bip44").account_xpub; + let bip44_address = managed_wallet + .first_bip44_managed_account_mut() + .expect("bip44 managed") + .next_receive_address(Some(&bip44_xpub), true) + .expect("bip44 address"); + // The spender pays the CoinJoin account (account B) — so processed + // first, it is recorded in B only. The pool is pre-generated to the + // gap limit at construction; index 0 is already watched. + let cj_address = managed_wallet + .coinjoin_managed_account_at_index(0) + .expect("coinjoin managed") + .managed_account_type() + .address_pools() + .into_iter() + .find(|pool| { + pool.pool_type == crate::managed_account::address_pool::AddressPoolType::External + }) + .expect("coinjoin external pool") + .address_at_index(0) + .expect("pre-generated coinjoin address") + .clone(); + + const FUND: u64 = 1_000_000; + const BACK: u64 = 900_000; + let funding_tx = Transaction::dummy(&bip44_address, 0..1, &[FUND]); + let funded_outpoint = OutPoint { + txid: funding_tx.txid(), + vout: 0, + }; + let spender_tx = Transaction { + version: 1, + lock_time: 0, + input: vec![TxIn { + previous_output: funded_outpoint, + script_sig: ScriptBuf::new(), + sequence: 0xffffffff, + witness: dashcore::Witness::new(), + }], + output: vec![TxOut { + value: BACK, + script_pubkey: cj_address.script_pubkey(), + }], + special_transaction_payload: None, + }; + let spender_txid = spender_tx.txid(); + + // Spender first (height 2), landing in the CoinJoin account. + let result = managed_wallet + .check_core_transaction( + &spender_tx, + TransactionContext::InBlock(BlockInfo::new( + 2, + BlockHash::from_slice(&[3u8; 32]).expect("hash"), + 1_650_000_100, + )), + &mut wallet, + true, + true, + ) + .await; + assert!(result.is_relevant && result.is_new_transaction); + { + let cj = managed_wallet.coinjoin_managed_account_at_index(0).expect("cj"); + let record = cj.transactions().get(&spender_txid).expect("spender in CoinJoin acct"); + assert_eq!(record.net_amount, BACK as i64, "born income-only in the sibling account"); + } + + // Funding second (height 1) — recognized by BIP44, whose account- + // local records contain no spender. The wallet-scope sweep must + // reach the CoinJoin record. + let result = managed_wallet + .check_core_transaction( + &funding_tx, + TransactionContext::InBlock(BlockInfo::new( + 1, + BlockHash::from_slice(&[2u8; 32]).expect("hash"), + 1_650_000_000, + )), + &mut wallet, + true, + true, + ) + .await; + assert!(result.is_relevant); + + let cj = managed_wallet.coinjoin_managed_account_at_index(0).expect("cj"); + let record = cj.transactions().get(&spender_txid).expect("spender record"); + assert_eq!( + record.net_amount, + BACK as i64 - FUND as i64, + "the sibling account's record gains the spent side" + ); + assert_eq!(record.input_details.len(), 1); + let corrected = result + .updated_records + .iter() + .find(|r| r.txid == spender_txid) + .expect("cross-account correction surfaces in updated_records"); + assert_eq!(corrected.net_amount, BACK as i64 - FUND as i64); + } + /// Out-of-order funding (rescan block-download order): the SPENDER is /// processed before the transaction that funded it. At spender-process /// time its input is unknown, so the record is born income-only @@ -3652,8 +3786,7 @@ mod tests { let result = ctx.check_transaction(&spender_tx, spender_context).await; assert!(result.is_relevant && result.is_new_transaction); { - let account = - ctx.managed_wallet.first_bip44_managed_account().expect("bip44 account"); + let account = ctx.managed_wallet.first_bip44_managed_account().expect("bip44 account"); let record = account.transactions().get(&spender_txid).expect("spender recorded"); assert_eq!( record.net_amount, BACK as i64, @@ -3673,8 +3806,7 @@ mod tests { // The engine record is corrected... { - let account = - ctx.managed_wallet.first_bip44_managed_account().expect("bip44 account"); + let account = ctx.managed_wallet.first_bip44_managed_account().expect("bip44 account"); let record = account.transactions().get(&spender_txid).expect("spender record"); assert_eq!( record.net_amount,