From 0d132c223a01b993f662c2bf1a39944d24049fe3 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Wed, 5 Aug 2026 16:43:52 +0300 Subject: [PATCH 1/9] feat(platform-wallet): reconstruct sent DashPay payments from tx history MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Received DashPay payments already recover after a restore-from-seed: `reconcile_incoming_payments` walks `dashpay_receival_accounts`, which persist with their UTXOs. The sending direction had no equivalent, so a restored wallet showed "No payments with this contact yet" for every contact it had paid — the transactions were on chain and in local history, just not attributed. `reconcile_sent_payments_from_tx_history` closes that gap. It walks the wallet's persisted core transactions, matches outputs against the addresses derived from each contact's `DashpayExternalAccount`, and records one `Sent` entry per (owner, contact, txid). Local-only, no network round-trips, idempotent — an existing entry for a txid is never overwritten, so the live send path and the incoming reconcile both keep priority. It runs as a step of `dashpay_sync()` after `reconcile_incoming_payments`. Matching reads `record.transaction.output` and compares script pubkeys. It deliberately does not read `record.output_details`: records handed back by `get_core_tx_record` are rebuilt from the host's raw transaction bytes, so only `transaction`, `txid` and `context` carry real data and the details vec is always empty. Comparing scripts rather than rendered addresses also sidesteps address-encoding differences. Contacts are skipped once they have a `Sent` entry, or once swept this launch. The direction matters: the incoming reconcile runs first, so a "has any payment with this contact" test would have hidden the outgoing history of every contact we had also received from. The per-launch marker is in-memory only and is not set when a persister read or write failed, so a transient error cannot permanently strand a contact. Enumerating the wallet's transactions needs a new persistence hook, `list_wallet_core_txids`, defaulting to an empty list so existing persisters keep compiling. --- .../rs-platform-wallet-ffi/src/persistence.rs | 98 +++ .../src/changeset/traits.rs | 12 + .../src/manager/dashpay_sync.rs | 16 + .../src/wallet/identity/network/payments.rs | 809 +++++++++++++++++- .../state/managed_identity/dashpay.rs | 15 + .../identity/state/managed_identity/mod.rs | 11 + .../src/wallet/persister.rs | 7 + .../PlatformWalletPersistenceHandler.swift | 91 ++ 8 files changed, 1058 insertions(+), 1 deletion(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index be30d937dd..dbd623feaa 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -627,6 +627,34 @@ pub struct PersistenceCallbacks { pub on_get_core_tx_record_free_fn: Option< unsafe extern "C" fn(context: *mut c_void, tx_bytes: *const u8, tx_bytes_len: usize), >, + /// Enumerate the persisted Core txids that belong to `wallet_id`. + /// + /// Used by DashPay sent-payment reconstruction to walk the local + /// transaction history without requiring the optional in-memory + /// `transactions()` map to retain finalized records. + /// + /// Output contract: + /// - Set `*out_txids` to a contiguous buffer of `32 * *out_count` + /// bytes, one raw-wire txid per 32-byte chunk, and `*out_count` + /// to the number of txids returned. + /// - Set `*out_txids = null` and `*out_count = 0` when no rows + /// exist for the wallet. + /// - Return `0` on success; non-zero values are treated as backend + /// failures by the Rust side. + pub on_list_wallet_core_txids_fn: Option< + unsafe extern "C" fn( + context: *mut c_void, + wallet_id: *const u8, + out_txids: *mut *const u8, + out_count: *mut usize, + ) -> i32, + >, + /// Paired free callback for the txid buffer returned by + /// [`Self::on_list_wallet_core_txids_fn`]. Rust invokes this with + /// the same pointer and txid count, exactly once per successful + /// hit. + pub on_list_wallet_core_txids_free_fn: + Option, /// Called with an `AssetLockChangeSet` slice — upserts on the /// tracked-asset-lock store and outpoint tombstones. Swift maps /// upserts onto `PersistentAssetLock` rows keyed by the 36-byte @@ -711,6 +739,8 @@ impl Default for PersistenceCallbacks { on_persist_contacts_fn: None, on_get_core_tx_record_fn: None, on_get_core_tx_record_free_fn: None, + on_list_wallet_core_txids_fn: None, + on_list_wallet_core_txids_free_fn: None, #[cfg(feature = "shielded")] on_persist_shielded_notes_fn: None, #[cfg(feature = "shielded")] @@ -2730,6 +2760,74 @@ impl PlatformWalletPersistence for FFIPersister { label: String::new(), })) } + + fn list_wallet_core_txids( + &self, + wallet_id: WalletId, + ) -> Result, PersistenceError> { + use dashcore::hashes::Hash; + + let Some(list_cb) = self.callbacks.on_list_wallet_core_txids_fn else { + return Ok(Vec::new()); + }; + + let mut txids_ptr: *const u8 = std::ptr::null(); + let mut count: usize = 0; + + let rc = unsafe { + list_cb( + self.callbacks.context, + wallet_id.as_ptr(), + &mut txids_ptr, + &mut count, + ) + }; + + struct TxidBytesGuard { + ptr: *const u8, + count: usize, + free_fn: + Option, + ctx: *mut c_void, + } + impl Drop for TxidBytesGuard { + fn drop(&mut self) { + if let (Some(free), false) = (self.free_fn, self.ptr.is_null()) { + unsafe { free(self.ctx, self.ptr, self.count) }; + } + } + } + let _txid_guard = TxidBytesGuard { + ptr: txids_ptr, + count, + free_fn: self.callbacks.on_list_wallet_core_txids_free_fn, + ctx: self.callbacks.context, + }; + + if rc != 0 { + return Err(PersistenceError::backend(format!( + "on_list_wallet_core_txids_fn returned non-zero status {rc}" + ))); + } + if txids_ptr.is_null() || count == 0 { + return Ok(Vec::new()); + } + + let raw = unsafe { slice::from_raw_parts(txids_ptr, count.saturating_mul(32)) }; + if raw.len() != count.saturating_mul(32) { + return Err(PersistenceError::backend( + "on_list_wallet_core_txids_fn returned an inconsistent txid buffer", + )); + } + + let mut out = Vec::with_capacity(count); + for chunk in raw.chunks_exact(32) { + let mut bytes = [0u8; 32]; + bytes.copy_from_slice(chunk); + out.push(dashcore::Txid::from_byte_array(bytes)); + } + Ok(out) + } } /// Decode `count` contiguous 32-byte commitments / nullifiers from a diff --git a/packages/rs-platform-wallet/src/changeset/traits.rs b/packages/rs-platform-wallet/src/changeset/traits.rs index ea9978a30e..1e3d5ca143 100644 --- a/packages/rs-platform-wallet/src/changeset/traits.rs +++ b/packages/rs-platform-wallet/src/changeset/traits.rs @@ -344,6 +344,18 @@ pub trait PlatformWalletPersistence: Send + Sync { Ok(None) } + /// Enumerate the persisted Core transaction ids that belong to + /// `wallet_id`. + /// + /// Used by DashPay sent-payment reconstruction to walk the + /// wallet's locally persisted transaction history without relying + /// on the optional in-memory `transactions()` map. The default + /// implementation returns an empty set for backwards compatibility + /// with backends that don't index wallet-scoped tx history. + fn list_wallet_core_txids(&self, _wallet_id: WalletId) -> Result, PersistenceError> { + Ok(Vec::new()) + } + // TODO: `list_wallets` and `delete_wallet` are deferred contract // candidates. They live as inherent methods on the SQLite backend // today; they may return to this trait once a cross-backend contract diff --git a/packages/rs-platform-wallet/src/manager/dashpay_sync.rs b/packages/rs-platform-wallet/src/manager/dashpay_sync.rs index 3ffdf2fdd1..6f1f434099 100644 --- a/packages/rs-platform-wallet/src/manager/dashpay_sync.rs +++ b/packages/rs-platform-wallet/src/manager/dashpay_sync.rs @@ -464,6 +464,22 @@ impl DashPaySyncManager { ); } + // Local-only: rebuild missing `Sent` entries from persisted + // wallet transaction history + the contact external-account + // address pools. Runs after the incoming reconcile so an + // existing received entry under the txid wins the dedup guard. + if let Err(e) = identity + .dashpay() + .reconcile_sent_payments_from_tx_history() + .await + { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + error = %e, + "DashPay sent-payment reconstruction failed" + ); + } + // Local-only: DIP-15 §12.6 coreHeight backfill — lower SPV synced_height // to re-scan for incoming payments that landed on a contact's receival // address before it was watched (restore-from-seed / 2nd device / diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs index bd797eac60..d6998cf29b 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs @@ -195,6 +195,207 @@ impl DashPayView<'_, B> { Ok(floor) } + /// Rebuild missing `Sent` [`PaymentEntry`]s by matching persisted + /// wallet transaction outputs against the wallet's registered + /// `DashpayExternalAccount` address pools. + /// + /// Recovery path for sent-payment history after restore-from-seed: + /// the wallet's transaction records survive in persistence but the + /// local DashPay payment cache may be empty. Unlike the + /// receival-side UTXO walk this scans persisted tx records, sums + /// every output that pays a contact's external-account address, and + /// records a `Sent` entry per `(owner, contact, txid)`. + /// + /// Only contacts whose local payment history is still empty are + /// eligible, and each eligible contact is swept at most once per + /// launch. That keeps the recovery path cheap in steady state: + /// after the first "nothing to reconstruct" answer, recurring + /// `dashpay_sync()` passes stop full-scanning persisted tx history + /// every 15 seconds. + /// + /// Local-only and idempotent: an existing payment entry under the + /// txid is never overwritten. + pub async fn reconcile_sent_payments_from_tx_history( + &self, + ) -> Result { + use crate::wallet::identity::types::dashpay::payment::{PaymentDirection, PaymentStatus}; + use dashcore::ScriptBuf; + use std::collections::{BTreeMap, BTreeSet}; + + // Keyed by script pubkey, not by rendered address: the outputs this + // matches against come from a consensus-decoded `Transaction`, which + // carries scripts. Comparing scripts also sidesteps address-encoding + // pitfalls (network prefix, P2PKH vs P2SH rendering). + let (address_matches, eligible_contacts): ( + BTreeMap, + Vec<(Identifier, Identifier)>, + ) = { + let wm = self.wallet_manager.read().await; + let info = match wm.get_wallet_info(&self.wallet_id) { + Some(info) => info, + None => return Ok(0), + }; + let mut out = BTreeMap::new(); + let mut eligible = Vec::new(); + for (key, account) in &info.core_wallet.accounts.dashpay_external_accounts { + let owner = Identifier::from(key.user_identity_id); + let contact = Identifier::from(key.friend_identity_id); + let Some(managed) = info.identity_manager.managed_identity(&owner) else { + continue; + }; + if managed + .dashpay() + .sent_payment_reconcile_attempted + .contains(&contact) + { + continue; + } + // Only a `Sent` entry proves this contact's outgoing history is + // already present. `reconcile_incoming_payments` runs first and + // records `Received` entries, so testing for "any payment with + // this contact" hid every send to a contact we had also + // received from — the incoming pass filled the map, this guard + // read it as done, and the sends were never reconstructed. + if managed.dashpay().payments.values().any(|payment| { + payment.counterparty_id == contact + && payment.direction == PaymentDirection::Sent + }) { + continue; + } + let pools = account.managed_account_type().address_pools(); + let Some(pool) = pools.first() else { + continue; + }; + eligible.push((owner, contact)); + for address_info in pool.addresses.values() { + out.entry(address_info.script_pubkey.clone()) + .or_insert((owner, contact)); + } + } + (out, eligible) + }; + if address_matches.is_empty() { + // Steady state — every contact is either reconstructed or already + // swept this launch. Silent on purpose: this runs on every + // `dashpay_sync` pass, and logging it would emit a line every + // 15 seconds for the life of the process. + return Ok(0); + } + tracing::info!( + eligible_contacts = eligible_contacts.len(), + candidate_addresses = address_matches.len(), + "reconcile_sent_payments_from_tx_history: candidate set built" + ); + + let txids = self.persister.list_wallet_core_txids().map_err(|e| { + PlatformWalletError::Persistence(format!("failed to enumerate wallet txids: {e}")) + })?; + + let mut totals: BTreeMap<(Identifier, Identifier, String), (u64, PaymentStatus)> = + BTreeMap::new(); + let mut had_read_error = false; + let txid_count = txids.len(); + let mut records_read = 0usize; + let mut outputs_scanned = 0usize; + for txid in txids { + let record = match self.persister.get_core_tx_record(&txid) { + Ok(Some(record)) => record, + Ok(None) => continue, + Err(e) => { + had_read_error = true; + tracing::warn!( + error = %e, + %txid, + "reconcile_sent_payments_from_tx_history: tx-record read failed; will retry next sweep" + ); + continue; + } + }; + records_read += 1; + let status = sent_payment_status_for_record(&record); + let txid_str = txid.to_string(); + // Walk the decoded transaction's outputs, NOT `record.output_details`. + // Records handed back by `get_core_tx_record` are rebuilt from the + // host's raw transaction bytes: `transaction`, `txid` and `context` + // are real, every other field is a placeholder — `output_details` is + // always an empty vec. Matching on it silently found nothing. + for out in &record.transaction.output { + outputs_scanned += 1; + let Some(&(owner, contact)) = address_matches.get(&out.script_pubkey) else { + continue; + }; + let entry = totals + .entry((owner, contact, txid_str.clone())) + .or_insert((0u64, status)); + entry.0 += out.value; + entry.1 = status; + } + } + + tracing::info!( + txids = txid_count, + records_read, + outputs_scanned, + matched_txids = totals.len(), + "reconcile_sent_payments_from_tx_history: scan complete" + ); + + let mut wm = self.wallet_manager.write().await; + let Some(info) = wm.get_wallet_info_mut(&self.wallet_id) else { + return Ok(0); + }; + + let mut recorded = 0usize; + let mut write_failed_for: BTreeSet<(Identifier, Identifier)> = BTreeSet::new(); + for ((owner, contact, txid), (amount_duffs, status)) in totals { + let Some(managed) = info.identity_manager.managed_identity_mut(&owner) else { + continue; + }; + if managed.dashpay().payments.contains_key(&txid) { + continue; + } + let mut entry = + crate::wallet::identity::types::dashpay::payment::PaymentEntry::new_sent( + contact, + amount_duffs, + None, + ); + entry.status = status; + tracing::info!( + owner = %owner, + contact = %contact, + %txid, + amount_duffs, + ?status, + "Recording reconstructed sent DashPay payment" + ); + if let Err(e) = managed.record_dashpay_payment(txid, entry, &self.persister) { + tracing::warn!( + error = %e, + "Failed to persist reconstructed sent payment; will retry next sweep" + ); + write_failed_for.insert((owner, contact)); + continue; + } + recorded += 1; + } + + if !had_read_error { + for (owner, contact) in eligible_contacts { + if write_failed_for.contains(&(owner, contact)) { + continue; + } + let Some(managed) = info.identity_manager.managed_identity_mut(&owner) else { + continue; + }; + managed + .dashpay_sent_payment_reconcile_attempted_mut() + .insert(contact); + } + } + Ok(recorded) + } + /// Flip `Pending` `Sent` [`PaymentEntry`]s to `Confirmed` when the /// persisted core transaction record reports the transaction final. /// @@ -396,6 +597,19 @@ fn record_received_payment_totals( recorded } +fn sent_payment_status_for_record( + record: &key_wallet::managed_account::transaction_record::TransactionRecord, +) -> crate::wallet::identity::types::dashpay::payment::PaymentStatus { + use crate::wallet::identity::types::dashpay::payment::PaymentStatus; + use key_wallet::transaction_checking::TransactionContext; + + if record.is_confirmed() || matches!(record.context, TransactionContext::InstantSend(_)) { + PaymentStatus::Confirmed + } else { + PaymentStatus::Pending + } +} + /// Advance a sender's `Sent` [`PaymentEntry`] from `Pending` to /// `Confirmed` once its broadcast transaction reaches finality. /// @@ -825,6 +1039,7 @@ mod tests { use dpp::identity::Identity; use dpp::prelude::Identifier; use key_wallet::account::account_collection::DashpayAccountKey; + use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; use key_wallet::mnemonic::{Language, Mnemonic}; use key_wallet::wallet::initialization::WalletAccountCreationOptions; use key_wallet::Network; @@ -874,11 +1089,13 @@ mod tests { #[derive(Default)] struct RecordStorePersister { records: Mutex< - std::collections::HashMap< + std::collections::BTreeMap< dashcore::Txid, key_wallet::managed_account::transaction_record::TransactionRecord, >, >, + list_wallet_core_txids_calls: Mutex, + get_core_tx_record_calls: Mutex, } impl PlatformWalletPersistence for RecordStorePersister { @@ -903,8 +1120,17 @@ mod tests { Option, PersistenceError, > { + *self.get_core_tx_record_calls.lock().unwrap() += 1; Ok(self.records.lock().unwrap().get(txid).cloned()) } + + fn list_wallet_core_txids( + &self, + _wallet_id: WalletId, + ) -> Result, PersistenceError> { + *self.list_wallet_core_txids_calls.lock().unwrap() += 1; + Ok(self.records.lock().unwrap().keys().copied().collect()) + } } struct NoopEventHandler; @@ -1194,6 +1420,155 @@ mod tests { txid.to_string() } + async fn install_external_account( + manager: &Arc>, + wallet_id: WalletId, + owner: Identifier, + contact: Identifier, + ) -> Vec { + use key_wallet::account::AccountType; + use key_wallet::managed_account::ManagedCoreFundsAccount; + + let wallet = manager + .get_wallet(&wallet_id) + .await + .expect("wallet registered"); + let iw = wallet.identity(); + let mut wm = iw.wallet_manager.write().await; + let (wallet, info) = wm + .get_wallet_mut_and_info_mut(&wallet_id) + .expect("wallet and info"); + + let account_type = AccountType::DashpayExternalAccount { + index: 0, + user_identity_id: owner.to_buffer(), + friend_identity_id: contact.to_buffer(), + }; + let account_xpub = test_receiving_xpub(&owner, &contact); + let account = key_wallet::Account { + parent_wallet_id: Some(wallet_id), + account_type, + network: Network::Testnet, + account_xpub, + is_watch_only: true, + }; + let managed = ManagedCoreFundsAccount::from_account(&account); + + wallet + .add_account(account_type, Some(account_xpub)) + .expect("add immutable external account"); + info.core_wallet + .accounts + .insert_funds_bearing_account(managed) + .expect("add managed external account"); + + let key = DashpayAccountKey { + index: 0, + user_identity_id: owner.to_buffer(), + friend_identity_id: contact.to_buffer(), + }; + let account = info + .core_wallet + .accounts + .dashpay_external_accounts + .get(&key) + .expect("external account present"); + account + .managed_account_type() + .address_pools() + .first() + .expect("external account has a pool") + .addresses + .values() + .take(2) + .map(|info| info.address.clone()) + .collect() + } + + async fn first_standard_wallet_address( + manager: &Arc>, + wallet_id: WalletId, + ) -> dashcore::Address { + let wallet = manager + .get_wallet(&wallet_id) + .await + .expect("wallet registered"); + let iw = wallet.identity(); + let wm = iw.wallet_manager.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("wallet info"); + info.core_wallet + .accounts + .standard_bip44_accounts + .get(&0) + .expect("bip44 account 0") + .managed_account_type() + .address_pools() + .first() + .expect("standard external pool") + .addresses + .values() + .next() + .expect("at least one standard address") + .address + .clone() + } + + fn tx_record_with_outputs( + context: key_wallet::transaction_checking::TransactionContext, + outputs: Vec<( + dashcore::Address, + u64, + key_wallet::managed_account::transaction_record::OutputRole, + )>, + ) -> key_wallet::managed_account::transaction_record::TransactionRecord { + use dashcore::{OutPoint, Transaction, TxIn, TxOut, Txid}; + use key_wallet::account::{AccountType, StandardAccountType}; + use key_wallet::managed_account::transaction_record::{ + OutputDetail, TransactionDirection, TransactionRecord, + }; + use key_wallet::transaction_checking::TransactionType; + + let tx = Transaction { + version: 2, + lock_time: 0, + input: vec![TxIn { + previous_output: OutPoint::new(Txid::from([0x91; 32]), 0), + ..Default::default() + }], + output: outputs + .iter() + .map(|(address, value, _)| TxOut { + value: *value, + script_pubkey: address.script_pubkey(), + }) + .collect(), + special_transaction_payload: None, + }; + let output_details = outputs + .into_iter() + .enumerate() + .map(|(index, (address, value, role))| OutputDetail { + index: index as u32, + role, + address: Some(address), + value, + }) + .collect(); + TransactionRecord::new( + tx, + AccountType::Standard { + index: 0, + standard_account_type: StandardAccountType::BIP44Account, + }, + context, + TransactionType::Standard, + TransactionDirection::Outgoing, + Vec::new(), + output_details, + 0, + ) + } + /// 1. Registering a contact receival account must persist an /// `AccountRegistrationEntry` — otherwise the account (and every /// UTXO routed to it) silently vanishes on the next app launch @@ -2523,6 +2898,438 @@ mod tests { ); } + #[tokio::test] + async fn reconcile_sent_payments_from_tx_history_rebuilds_and_is_idempotent() { + use dashcore::hashes::Hash; + use dashcore::BlockHash; + use key_wallet::managed_account::transaction_record::OutputRole; + use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; + + use crate::wallet::identity::types::dashpay::payment::PaymentStatus; + + let persister = Arc::new(RecordStorePersister::default()); + let (manager, wallet_id) = make_wallet_with(Arc::clone(&persister)).await; + let owner = Identifier::from([0xAA; 32]); + let contact = Identifier::from([0xBB; 32]); + + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet.identity(); + let p = WalletPersister::new(wallet_id, Arc::clone(&persister) as _); + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .add_identity(bare_identity([0xAA; 32]), 0, wallet_id, &p) + .expect("add owner"); + } + + let contact_addresses = install_external_account(&manager, wallet_id, owner, contact).await; + assert!( + contact_addresses.len() >= 2, + "external account must pre-derive at least two addresses" + ); + let change_address = first_standard_wallet_address(&manager, wallet_id).await; + + let record = tx_record_with_outputs( + TransactionContext::InBlock(BlockInfo::new(123, BlockHash::all_zeros(), 0)), + vec![ + (contact_addresses[0].clone(), 25_000, OutputRole::Sent), + (change_address, 90_000, OutputRole::Change), + (contact_addresses[1].clone(), 10_000, OutputRole::Sent), + ], + ); + let txid = record.txid; + persister.records.lock().unwrap().insert(txid, record); + + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("reconcile"), + 1, + "one reconstructed payment should be recorded" + ); + + { + let wm = iw.wallet_manager.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("info"); + let entry = info + .identity_manager + .managed_identity(&owner) + .expect("managed") + .dashpay() + .payments + .get(&txid.to_string()) + .cloned() + .expect("reconstructed sent payment"); + assert_eq!(entry.amount_duffs, 35_000, "sum all contact outputs only"); + assert_eq!(entry.status, PaymentStatus::Confirmed); + } + + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("second pass"), + 0, + "reconstruction must be idempotent" + ); + } + + #[tokio::test] + async fn reconcile_sent_payments_from_tx_history_does_not_overwrite_existing_entry() { + use dashcore::hashes::Hash; + use dashcore::BlockHash; + use key_wallet::managed_account::transaction_record::OutputRole; + use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; + + use crate::wallet::identity::types::dashpay::payment::{PaymentEntry, PaymentStatus}; + + let persister = Arc::new(RecordStorePersister::default()); + let (manager, wallet_id) = make_wallet_with(Arc::clone(&persister)).await; + let owner = Identifier::from([0xAA; 32]); + let contact = Identifier::from([0xBB; 32]); + + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet.identity(); + let p = WalletPersister::new(wallet_id, Arc::clone(&persister) as _); + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .add_identity(bare_identity([0xAA; 32]), 0, wallet_id, &p) + .expect("add owner"); + } + + let contact_address = install_external_account(&manager, wallet_id, owner, contact) + .await + .remove(0); + let record = tx_record_with_outputs( + TransactionContext::InBlock(BlockInfo::new(55, BlockHash::all_zeros(), 0)), + vec![(contact_address, 50_000, OutputRole::Sent)], + ); + let txid = record.txid; + persister.records.lock().unwrap().insert(txid, record); + + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .managed_identity_mut(&owner) + .expect("managed") + .record_dashpay_payment( + txid.to_string(), + PaymentEntry::new_received(contact, 7_500, Some("keep me".into())), + &p, + ) + .expect("preexisting received entry"); + } + + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("reconcile"), + 0, + "an existing txid entry must win the dedup guard" + ); + + { + let wm = iw.wallet_manager.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("info"); + let entry = info + .identity_manager + .managed_identity(&owner) + .expect("managed") + .dashpay() + .payments + .get(&txid.to_string()) + .cloned() + .expect("entry still present"); + assert_eq!(entry.amount_duffs, 7_500); + assert_eq!(entry.status, PaymentStatus::Confirmed); + assert_eq!(entry.memo.as_deref(), Some("keep me")); + } + } + + /// Reconstruction must work on the record shape the FFI actually hands + /// back. `PlatformWalletPersistence::get_core_tx_record` rebuilds a record + /// from the host's raw transaction bytes and fills only `transaction`, + /// `txid` and `context` — `output_details` is always empty. The test + /// helper populates both, which is why a version of this sweep that read + /// `output_details` passed every unit test and matched nothing on device + /// (`outputs_scanned=0`, `matched_txids=0` against 49 records read). + #[tokio::test] + async fn reconcile_sent_payments_from_tx_history_matches_without_output_details() { + use dashcore::hashes::Hash; + use dashcore::BlockHash; + use key_wallet::managed_account::transaction_record::OutputRole; + use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; + + use crate::wallet::identity::types::dashpay::payment::PaymentDirection; + + let persister = Arc::new(RecordStorePersister::default()); + let (manager, wallet_id) = make_wallet_with(Arc::clone(&persister)).await; + let owner = Identifier::from([0xAA; 32]); + let contact = Identifier::from([0xBB; 32]); + + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet.identity(); + let p = WalletPersister::new(wallet_id, Arc::clone(&persister) as _); + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .add_identity(bare_identity([0xAA; 32]), 0, wallet_id, &p) + .expect("add owner"); + } + + let contact_address = install_external_account(&manager, wallet_id, owner, contact) + .await + .remove(0); + let mut record = tx_record_with_outputs( + TransactionContext::InBlock(BlockInfo::new(77, BlockHash::all_zeros(), 0)), + vec![(contact_address, 250_000, OutputRole::Sent)], + ); + // Exactly what the FFI returns: scripts on the decoded transaction, + // nothing in the details vec. + record.output_details.clear(); + let txid = record.txid; + persister.records.lock().unwrap().insert(txid, record); + + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("reconcile"), + 1, + "matching must not depend on `output_details`, which the FFI leaves empty" + ); + + { + let wm = iw.wallet_manager.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("info"); + let entry = info + .identity_manager + .managed_identity(&owner) + .expect("managed") + .dashpay() + .payments + .get(&txid.to_string()) + .cloned() + .expect("reconstructed entry"); + assert_eq!(entry.direction, PaymentDirection::Sent); + assert_eq!(entry.amount_duffs, 250_000); + assert_eq!(entry.counterparty_id, contact); + } + } + + /// A contact we have also *received* from must still get its sends + /// reconstructed. `reconcile_incoming_payments` runs first and fills the + /// payments map with `Received` entries; a skip-guard that only asked + /// "any payment with this contact?" read that as "already reconstructed" + /// and permanently hid the outgoing history for every two-way contact. + #[tokio::test] + async fn reconcile_sent_payments_from_tx_history_reconstructs_for_contact_with_received_history() + { + use dashcore::hashes::Hash; + use dashcore::BlockHash; + use key_wallet::managed_account::transaction_record::OutputRole; + use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; + + use crate::wallet::identity::types::dashpay::payment::{ + PaymentDirection, PaymentEntry, PaymentStatus, + }; + + let persister = Arc::new(RecordStorePersister::default()); + let (manager, wallet_id) = make_wallet_with(Arc::clone(&persister)).await; + let owner = Identifier::from([0xAA; 32]); + let contact = Identifier::from([0xBB; 32]); + + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet.identity(); + let p = WalletPersister::new(wallet_id, Arc::clone(&persister) as _); + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .add_identity(bare_identity([0xAA; 32]), 0, wallet_id, &p) + .expect("add owner"); + } + + let contact_address = install_external_account(&manager, wallet_id, owner, contact) + .await + .remove(0); + let record = tx_record_with_outputs( + TransactionContext::InBlock(BlockInfo::new(55, BlockHash::all_zeros(), 0)), + vec![(contact_address, 50_000, OutputRole::Sent)], + ); + let sent_txid = record.txid; + persister.records.lock().unwrap().insert(sent_txid, record); + + // An unrelated incoming payment from the same contact, as the incoming + // reconcile would have left it — a different txid, so the per-txid + // dedup guard is not what is under test here. + let received_txid = "11".repeat(32); + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .managed_identity_mut(&owner) + .expect("managed") + .record_dashpay_payment( + received_txid.clone(), + PaymentEntry::new_received(contact, 7_500, None), + &p, + ) + .expect("preexisting received entry"); + } + + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("reconcile"), + 1, + "received history with a contact must not suppress the sent sweep" + ); + + { + let wm = iw.wallet_manager.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("info"); + let payments = &info + .identity_manager + .managed_identity(&owner) + .expect("managed") + .dashpay() + .payments; + let entry = payments.get(&sent_txid.to_string()).expect("sent entry"); + assert_eq!(entry.direction, PaymentDirection::Sent); + assert_eq!(entry.amount_duffs, 50_000); + assert_eq!(entry.status, PaymentStatus::Confirmed); + // The incoming entry is untouched. + let received = payments.get(&received_txid).expect("received entry"); + assert_eq!(received.direction, PaymentDirection::Received); + assert_eq!(received.amount_duffs, 7_500); + } + } + + #[tokio::test] + async fn reconcile_sent_payments_from_tx_history_keeps_mempool_entries_pending() { + use key_wallet::managed_account::transaction_record::OutputRole; + use key_wallet::transaction_checking::TransactionContext; + + use crate::wallet::identity::types::dashpay::payment::PaymentStatus; + + let persister = Arc::new(RecordStorePersister::default()); + let (manager, wallet_id) = make_wallet_with(Arc::clone(&persister)).await; + let owner = Identifier::from([0xAA; 32]); + let contact = Identifier::from([0xBB; 32]); + + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet.identity(); + let p = WalletPersister::new(wallet_id, Arc::clone(&persister) as _); + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .add_identity(bare_identity([0xAA; 32]), 0, wallet_id, &p) + .expect("add owner"); + } + + let contact_address = install_external_account(&manager, wallet_id, owner, contact) + .await + .remove(0); + let record = tx_record_with_outputs( + TransactionContext::Mempool, + vec![(contact_address, 11_000, OutputRole::Sent)], + ); + let txid = record.txid; + persister.records.lock().unwrap().insert(txid, record); + + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("reconcile"), + 1 + ); + + let wm = iw.wallet_manager.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("info"); + assert_eq!( + info.identity_manager + .managed_identity(&owner) + .expect("managed") + .dashpay() + .payments + .get(&txid.to_string()) + .expect("entry") + .status, + PaymentStatus::Pending, + "a mempool tx must reconstruct as Pending until the confirm sweep flips it" + ); + } + + #[tokio::test] + async fn reconcile_sent_payments_from_tx_history_skips_repeat_empty_sweeps() { + let persister = Arc::new(RecordStorePersister::default()); + let (manager, wallet_id) = make_wallet_with(Arc::clone(&persister)).await; + let owner = Identifier::from([0xAA; 32]); + let contact = Identifier::from([0xBB; 32]); + + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet.identity(); + let p = WalletPersister::new(wallet_id, Arc::clone(&persister) as _); + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .add_identity(bare_identity([0xAA; 32]), 0, wallet_id, &p) + .expect("add owner"); + } + + let _ = install_external_account(&manager, wallet_id, owner, contact).await; + + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("first reconcile"), + 0, + "an empty tx history should produce no reconstructed payments" + ); + assert_eq!( + *persister.list_wallet_core_txids_calls.lock().unwrap(), + 1, + "the first pass must enumerate txids once" + ); + assert_eq!( + *persister.get_core_tx_record_calls.lock().unwrap(), + 0, + "with no txids there should be no per-record reads" + ); + + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("second reconcile"), + 0, + "the second pass should early-exit before touching persistence" + ); + assert_eq!( + *persister.list_wallet_core_txids_calls.lock().unwrap(), + 1, + "steady state must not keep re-enumerating txids every sweep" + ); + assert_eq!( + *persister.get_core_tx_record_calls.lock().unwrap(), + 0, + "steady-state early exit should avoid tx-record fetches entirely" + ); + } + /// The seedless drain path: `register_external_contact_account` with a /// **precomputed** ECDH shared secret (the Keychain signer computed it; the /// scalar never entered this crate) decrypts the contact's xpub and builds diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/dashpay.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/dashpay.rs index d9b9578a9d..3e1cb19bd6 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/dashpay.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/dashpay.rs @@ -89,6 +89,21 @@ pub struct DashPayState { /// identity, with direction, amount, memo, and status. pub payments: BTreeMap, + /// Contacts whose historical sent-payment reconstruction sweep has + /// already run in this process lifetime. + /// + /// `reconcile_sent_payments_from_tx_history` is a restore-time + /// recovery path for contacts whose local payment cache is still + /// empty. Once a contact has either been reconstructed or proven to + /// have nothing to reconstruct, re-running the full persisted-tx + /// scan every recurring sync pass is pure overhead. This guard + /// suppresses that steady-state rescan. + /// + /// In-memory only (never persisted): a relaunch retries the sweep + /// once for still-empty contacts, which is safe and far cheaper than + /// re-scanning every sync pass forever. + pub sent_payment_reconcile_attempted: BTreeSet, + /// Cached **contact** profiles keyed by the contact's identity id — /// established contacts, pending incoming-request senders, and (later) /// ignored senders, independent of relationship state. Populated by diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs index 6f62c2dd61..8cb633206f 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs @@ -123,6 +123,17 @@ impl ManagedIdentity { &mut self.dashpay.payments } + /// Mutable access to the per-session sent-payment reconcile guard. + /// + /// In-memory only — never persisted; see the field docs on + /// [`DashPayState::sent_payment_reconcile_attempted`] for the + /// relaunch contract. + pub fn dashpay_sent_payment_reconcile_attempted_mut( + &mut self, + ) -> &mut std::collections::BTreeSet { + &mut self.dashpay.sent_payment_reconcile_attempted + } + /// Mutable access to the cached contact profiles. /// /// Replay/restore surface: bypasses persistence on purpose (the diff --git a/packages/rs-platform-wallet/src/wallet/persister.rs b/packages/rs-platform-wallet/src/wallet/persister.rs index d37e88a025..c826e4cb4f 100644 --- a/packages/rs-platform-wallet/src/wallet/persister.rs +++ b/packages/rs-platform-wallet/src/wallet/persister.rs @@ -58,6 +58,13 @@ impl WalletPersister { ) -> Result, PersistenceError> { self.inner.get_core_tx_record(self.wallet_id, txid) } + + /// Enumerate the persisted Core transaction ids scoped to this + /// wallet. Used by DashPay sent-payment reconstruction to fetch + /// the full records via [`Self::get_core_tx_record`]. + pub(crate) fn list_wallet_core_txids(&self) -> Result, PersistenceError> { + self.inner.list_wallet_core_txids(self.wallet_id) + } } /// No-op platform persistence for standalone wallets. diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index f3edc95db0..7839a458dd 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -27,6 +27,22 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { } } + static func walletOwnsTransaction( + walletId: Data, + transaction: PersistentTransaction + ) -> Bool { + if transaction.involvedAccounts.contains(where: { $0.wallet.walletId == walletId }) { + return true + } + if transaction.outputs.contains(where: { $0.walletId == walletId }) { + return true + } + if transaction.inputs.contains(where: { $0.walletId == walletId }) { + return true + } + return transaction.pendingInputs.contains(where: { $0.walletId == walletId }) + } + let modelContainer: ModelContainer /// Network this handler's owning `PlatformWalletManager` is bound @@ -1260,6 +1276,8 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { cb.on_persist_invitations_fn = persistInvitationsCallback cb.on_get_core_tx_record_fn = getCoreTxRecordCallback cb.on_get_core_tx_record_free_fn = getCoreTxRecordFreeCallback + cb.on_list_wallet_core_txids_fn = listWalletCoreTxidsCallback + cb.on_list_wallet_core_txids_free_fn = listWalletCoreTxidsFreeCallback return cb } @@ -5755,6 +5773,23 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { } } + /// Enumerate the persisted txids scoped to `walletId`. + /// + /// Scope is the union of wallet-owned TXOs (`outputs`, `inputs`, + /// `pendingInputs`) and payload-only account involvement + /// (`involvedAccounts`). + func walletCoreTxids(walletId: Data) -> [Data] { + onQueue { + let descriptor = FetchDescriptor() + guard let rows = try? backgroundContext.fetch(descriptor), !rows.isEmpty else { + return [] + } + return rows.compactMap { tx in + Self.walletOwnsTransaction(walletId: walletId, transaction: tx) ? tx.txid : nil + } + } + } + /// Look up the network for a wallet id by reading the owning /// `PersistentWallet` row. Returns `nil` if the wallet row /// doesn't exist or its network hasn't been resolved yet. @@ -7465,3 +7500,59 @@ private func getCoreTxRecordFreeCallback( _ = context _ = txBytesLen } + +/// C shim for `on_list_wallet_core_txids_fn`. Returns a contiguous +/// `count * 32` byte buffer of raw txids in wire order. +private func listWalletCoreTxidsCallback( + context: UnsafeMutableRawPointer?, + walletIdPtr: UnsafePointer?, + outTxids: UnsafeMutablePointer?>?, + outCount: UnsafeMutablePointer? +) -> Int32 { + guard let context = context, + let walletIdPtr = walletIdPtr, + let outTxids = outTxids, + let outCount = outCount else { + return 0 + } + + outTxids.pointee = nil + outCount.pointee = 0 + + let handler = Unmanaged + .fromOpaque(context) + .takeUnretainedValue() + let walletId = Data(bytes: walletIdPtr, count: 32) + let txids = handler.walletCoreTxids(walletId: walletId) + guard !txids.isEmpty else { + return 0 + } + + let buffer = UnsafeMutablePointer.allocate(capacity: txids.count * 32) + // Pack only well-formed txids and report how many were packed. Skipping a + // malformed one while still reporting `txids.count` would leave its slot + // uninitialized and hand Rust 32 bytes of garbage as a txid. + var packed = 0 + for txid in txids where txid.count == 32 { + txid.copyBytes(to: buffer.advanced(by: packed * 32), count: 32) + packed += 1 + } + guard packed > 0 else { + buffer.deallocate() + return 0 + } + outTxids.pointee = UnsafePointer(buffer) + outCount.pointee = UInt(packed) + return 0 +} + +/// Paired free callback for `on_list_wallet_core_txids_free_fn`. +private func listWalletCoreTxidsFreeCallback( + context: UnsafeMutableRawPointer?, + txids: UnsafePointer?, + _ count: UInt +) { + guard let txids = txids else { return } + UnsafeMutablePointer(mutating: txids).deallocate() + _ = context +} From 188b48618efe2bb83178ba43fda956f051c7e6fe Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Wed, 5 Aug 2026 17:44:13 +0300 Subject: [PATCH 2/9] fix(platform-wallet): address review on sent-payment reconstruction MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit CI: - `cargo fmt` on the long test signature. - Android's JNI vtable builds `PersistenceCallbacks` by literal, so the two new slots have to be named there. Left `None`, matching the existing `on_persist_invitations_fn` precedent: Android keeps today's behaviour rather than reporting a reconstruction it cannot perform. Review: - Append the txid callbacks after `release_fn`. The struct is a C-bound vtable whose trailing slots are documented as end-safe; inserting before them would shift the layout for hosts built against the previous header. - Replace the tautological buffer-length check with the check that matters: `count * 32` must not overflow and must fit in `isize::MAX` before `from_raw_parts` sees it. Adds the missing SAFETY note. - Do not stamp the per-launch guard when the enumeration came back empty. After a restore the recurring sweep can fire before the host has repopulated its transaction table, and a zero-txid answer is indistinguishable from "nothing to reconstruct" — stamping there ended recovery for the rest of the process, the exact symptom this pass exists to fix. `..._skips_repeat_empty_sweeps` pinned that behaviour and is replaced by two tests: one that an empty enumeration is retried, one that a conclusive scan is not repeated. - Guard the fault-loaded `account.wallet` access in `walletOwnsTransaction` the way `loadWalletList` already does; this predicate runs over every persisted transaction row, so the exposure is wider. - Report failures from the txid callback. A nil argument or a failed fetch returned 0 with an empty list, which Rust could not tell from an empty wallet. --- .../rs-platform-wallet-ffi/src/persistence.rs | 80 ++++++++----- .../src/wallet/identity/network/payments.rs | 113 ++++++++++++++---- .../rs-unified-sdk-jni/src/persistence.rs | 7 ++ .../PlatformWalletPersistenceHandler.swift | 39 ++++-- 4 files changed, 178 insertions(+), 61 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index dbd623feaa..9a3f449bfd 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -627,34 +627,6 @@ pub struct PersistenceCallbacks { pub on_get_core_tx_record_free_fn: Option< unsafe extern "C" fn(context: *mut c_void, tx_bytes: *const u8, tx_bytes_len: usize), >, - /// Enumerate the persisted Core txids that belong to `wallet_id`. - /// - /// Used by DashPay sent-payment reconstruction to walk the local - /// transaction history without requiring the optional in-memory - /// `transactions()` map to retain finalized records. - /// - /// Output contract: - /// - Set `*out_txids` to a contiguous buffer of `32 * *out_count` - /// bytes, one raw-wire txid per 32-byte chunk, and `*out_count` - /// to the number of txids returned. - /// - Set `*out_txids = null` and `*out_count = 0` when no rows - /// exist for the wallet. - /// - Return `0` on success; non-zero values are treated as backend - /// failures by the Rust side. - pub on_list_wallet_core_txids_fn: Option< - unsafe extern "C" fn( - context: *mut c_void, - wallet_id: *const u8, - out_txids: *mut *const u8, - out_count: *mut usize, - ) -> i32, - >, - /// Paired free callback for the txid buffer returned by - /// [`Self::on_list_wallet_core_txids_fn`]. Rust invokes this with - /// the same pointer and txid count, exactly once per successful - /// hit. - pub on_list_wallet_core_txids_free_fn: - Option, /// Called with an `AssetLockChangeSet` slice — upserts on the /// tracked-asset-lock store and outpoint tombstones. Swift maps /// upserts onto `PersistentAssetLock` rows keyed by the 36-byte @@ -708,6 +680,38 @@ pub struct PersistenceCallbacks { /// callbacks memory-safe. A context needing no cleanup takes a no-op /// `release_fn`; `None` is valid only alongside a null `context`. pub release_fn: Option, + /// Enumerate the persisted Core txids that belong to `wallet_id`. + /// + /// Appended at the END so the struct layout stays stable — a host + /// built against the previous vtable keeps working, it simply never + /// sets these two slots. + /// + /// Used by DashPay sent-payment reconstruction to walk the local + /// transaction history without requiring the optional in-memory + /// `transactions()` map to retain finalized records. + /// + /// Output contract: + /// - Set `*out_txids` to a contiguous buffer of `32 * *out_count` + /// bytes, one raw-wire txid per 32-byte chunk, and `*out_count` + /// to the number of txids returned. + /// - Set `*out_txids = null` and `*out_count = 0` when no rows + /// exist for the wallet. + /// - Return `0` on success; non-zero values are treated as backend + /// failures by the Rust side. + pub on_list_wallet_core_txids_fn: Option< + unsafe extern "C" fn( + context: *mut c_void, + wallet_id: *const u8, + out_txids: *mut *const u8, + out_count: *mut usize, + ) -> i32, + >, + /// Paired free callback for the txid buffer returned by + /// [`Self::on_list_wallet_core_txids_fn`]. Rust invokes this with + /// the same pointer and txid count, exactly once per successful + /// hit. + pub on_list_wallet_core_txids_free_fn: + Option, } // SAFETY: The context pointer is managed by the FFI caller who must ensure @@ -2813,13 +2817,27 @@ impl PlatformWalletPersistence for FFIPersister { return Ok(Vec::new()); } - let raw = unsafe { slice::from_raw_parts(txids_ptr, count.saturating_mul(32)) }; - if raw.len() != count.saturating_mul(32) { + // Validate the byte length BEFORE building the slice: `from_raw_parts` + // requires it to fit in `isize::MAX`, and an implausible `count` from + // the host would otherwise be silently clamped into a slice that + // outruns the allocation. + let Some(byte_len) = count.checked_mul(32) else { return Err(PersistenceError::backend( - "on_list_wallet_core_txids_fn returned an inconsistent txid buffer", + "on_list_wallet_core_txids_fn reported a txid count whose byte length overflows", + )); + }; + if byte_len > isize::MAX as usize { + return Err(PersistenceError::backend( + "on_list_wallet_core_txids_fn reported a txid buffer larger than isize::MAX", )); } + // SAFETY: the host guarantees `txids_ptr` points to `byte_len` valid + // bytes for the duration of the callback window — `_txid_guard` keeps + // that window open until this function returns — and `byte_len` is + // checked above to be a valid slice length. + let raw = unsafe { slice::from_raw_parts(txids_ptr, byte_len) }; + let mut out = Vec::with_capacity(count); for chunk in raw.chunks_exact(32) { let mut bytes = [0u8; 32]; diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs index d6998cf29b..f67e130a98 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs @@ -380,7 +380,15 @@ impl DashPayView<'_, B> { recorded += 1; } - if !had_read_error { + // An enumeration that came back empty proves nothing: after a restore + // the recurring `dashpay_sync()` can fire before the host has finished + // repopulating its transaction table, and a zero-txid sweep is + // indistinguishable from a wallet that genuinely has nothing to + // reconstruct. Stamping the guard there would end recovery for the + // rest of the process — the exact symptom this pass exists to fix. + // Retrying costs one enumeration per sweep, without the per-record + // reads, until the wallet actually has transactions. + if !had_read_error && txid_count > 0 { for (owner, contact) in eligible_contacts { if write_failed_for.contains(&(owner, contact)) { continue; @@ -3130,8 +3138,8 @@ mod tests { /// "any payment with this contact?" read that as "already reconstructed" /// and permanently hid the outgoing history for every two-way contact. #[tokio::test] - async fn reconcile_sent_payments_from_tx_history_reconstructs_for_contact_with_received_history() - { + async fn reconcile_sent_payments_from_tx_history_reconstructs_for_contact_with_received_history( + ) { use dashcore::hashes::Hash; use dashcore::BlockHash; use key_wallet::managed_account::transaction_record::OutputRole; @@ -3271,8 +3279,14 @@ mod tests { ); } + /// An empty enumeration is inconclusive, so the sweep must keep retrying. + /// + /// After a restore the recurring `dashpay_sync()` can fire before the host + /// has repopulated its transaction table. Treating that zero-txid answer as + /// "nothing to reconstruct" would stamp the per-launch guard and end + /// recovery for the rest of the process. #[tokio::test] - async fn reconcile_sent_payments_from_tx_history_skips_repeat_empty_sweeps() { + async fn reconcile_sent_payments_from_tx_history_retries_after_empty_enumeration() { let persister = Arc::new(RecordStorePersister::default()); let (manager, wallet_id) = make_wallet_with(Arc::clone(&persister)).await; let owner = Identifier::from([0xAA; 32]); @@ -3291,33 +3305,86 @@ mod tests { let _ = install_external_account(&manager, wallet_id, owner, contact).await; - assert_eq!( - iw.dashpay() - .reconcile_sent_payments_from_tx_history() - .await - .expect("first reconcile"), - 0, - "an empty tx history should produce no reconstructed payments" - ); + for pass in 1..=2 { + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("reconcile"), + 0, + "an empty tx history should produce no reconstructed payments (pass {pass})" + ); + } assert_eq!( *persister.list_wallet_core_txids_calls.lock().unwrap(), - 1, - "the first pass must enumerate txids once" + 2, + "an empty enumeration must not be taken as conclusive" ); assert_eq!( *persister.get_core_tx_record_calls.lock().unwrap(), 0, "with no txids there should be no per-record reads" ); + } - assert_eq!( - iw.dashpay() - .reconcile_sent_payments_from_tx_history() - .await - .expect("second reconcile"), - 0, - "the second pass should early-exit before touching persistence" + /// Once a sweep has actually scanned transactions, repeating it is pure + /// overhead — the guard stops the full walk on every later pass. + #[tokio::test] + async fn reconcile_sent_payments_from_tx_history_skips_repeat_sweeps_after_a_real_scan() { + use dashcore::hashes::Hash; + use dashcore::BlockHash; + use key_wallet::managed_account::transaction_record::OutputRole; + use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; + + let persister = Arc::new(RecordStorePersister::default()); + let (manager, wallet_id) = make_wallet_with(Arc::clone(&persister)).await; + let owner = Identifier::from([0xAA; 32]); + let contact = Identifier::from([0xBB; 32]); + + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet.identity(); + let p = WalletPersister::new(wallet_id, Arc::clone(&persister) as _); + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .add_identity(bare_identity([0xAA; 32]), 0, wallet_id, &p) + .expect("add owner"); + } + + let _ = install_external_account(&manager, wallet_id, owner, contact).await; + + // A transaction that pays someone else: the enumeration is non-empty, + // so the scan is conclusive even though it reconstructs nothing. + let unrelated = dashcore::Address::p2pkh( + &dashcore::PublicKey::from_slice(&[ + 0x02, 0x79, 0xBE, 0x66, 0x7E, 0xF9, 0xDC, 0xBB, 0xAC, 0x55, 0xA0, 0x62, 0x95, 0xCE, + 0x87, 0x0B, 0x07, 0x02, 0x9B, 0xFC, 0xDB, 0x2D, 0xCE, 0x28, 0xD9, 0x59, 0xF2, 0x81, + 0x5B, 0x16, 0xF8, 0x17, 0x98, + ]) + .expect("valid compressed pubkey"), + Network::Testnet, ); + let record = tx_record_with_outputs( + TransactionContext::InBlock(BlockInfo::new(11, BlockHash::all_zeros(), 0)), + vec![(unrelated, 1_000, OutputRole::Sent)], + ); + persister + .records + .lock() + .unwrap() + .insert(record.txid, record); + + for pass in 1..=2 { + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("reconcile"), + 0, + "nothing pays this contact (pass {pass})" + ); + } assert_eq!( *persister.list_wallet_core_txids_calls.lock().unwrap(), 1, @@ -3325,8 +3392,8 @@ mod tests { ); assert_eq!( *persister.get_core_tx_record_calls.lock().unwrap(), - 0, - "steady-state early exit should avoid tx-record fetches entirely" + 1, + "the second pass must early-exit before any tx-record fetch" ); } diff --git a/packages/rs-unified-sdk-jni/src/persistence.rs b/packages/rs-unified-sdk-jni/src/persistence.rs index 5c5e11cd80..72343fab10 100644 --- a/packages/rs-unified-sdk-jni/src/persistence.rs +++ b/packages/rs-unified-sdk-jni/src/persistence.rs @@ -176,6 +176,13 @@ pub(crate) fn build_vtable(context: *mut c_void) -> PersistenceCallbacks { // invitation flow refuses to run on Android rather than create a // non-durable voucher whose one-time key could be reused on restart. on_persist_invitations_fn: None, + // Android hasn't wired transaction enumeration yet. `None` makes + // `list_wallet_core_txids` return an empty list, so the sent-payment + // reconstruction sweep finds nothing to match and records nothing — + // Android keeps today's behaviour (a restored wallet shows no + // pre-restore contact payments) rather than misreporting. + on_list_wallet_core_txids_fn: None, + on_list_wallet_core_txids_free_fn: None, release_fn: Some(release_persistence_ctx), } } diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index 7839a458dd..f34e0d1f53 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -31,7 +31,14 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { walletId: Data, transaction: PersistentTransaction ) -> Bool { - if transaction.involvedAccounts.contains(where: { $0.wallet.walletId == walletId }) { + // `account.wallet` is non-optional on the model but is a fault-loaded + // relationship; a relationship-store inconsistency would crash here, + // so guard via Optional cast (same treatment as the UTXO bucketing in + // `loadWalletList`). + if transaction.involvedAccounts.contains(where: { + let wallet: PersistentWallet? = $0.wallet + return wallet?.walletId == walletId + }) { return true } if transaction.outputs.contains(where: { $0.walletId == walletId }) { @@ -5778,15 +5785,27 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { /// Scope is the union of wallet-owned TXOs (`outputs`, `inputs`, /// `pendingInputs`) and payload-only account involvement /// (`involvedAccounts`). - func walletCoreTxids(walletId: Data) -> [Data] { + /// Returns `errored: true` when the fetch itself failed, so the shim can + /// report a non-zero status. Collapsing a database fault to an empty list + /// would be indistinguishable from a wallet with no transactions, and the + /// Rust side treats those two very differently. + func walletCoreTxids(walletId: Data) -> (txids: [Data], errored: Bool) { onQueue { let descriptor = FetchDescriptor() - guard let rows = try? backgroundContext.fetch(descriptor), !rows.isEmpty else { - return [] + let rows: [PersistentTransaction] + do { + rows = try backgroundContext.fetch(descriptor) + } catch { + NSLog( + "[persistor-txids:swift] PersistentTransaction fetch failed: %@", + String(describing: error) + ) + return ([], true) } - return rows.compactMap { tx in + let txids = rows.compactMap { tx in Self.walletOwnsTransaction(walletId: walletId, transaction: tx) ? tx.txid : nil } + return (txids, false) } } @@ -7509,11 +7528,14 @@ private func listWalletCoreTxidsCallback( outTxids: UnsafeMutablePointer?>?, outCount: UnsafeMutablePointer? ) -> Int32 { + // Non-zero on a missing argument: reporting success here would hand Rust + // an empty enumeration that it cannot tell apart from a wallet with no + // transactions. guard let context = context, let walletIdPtr = walletIdPtr, let outTxids = outTxids, let outCount = outCount else { - return 0 + return -1 } outTxids.pointee = nil @@ -7523,7 +7545,10 @@ private func listWalletCoreTxidsCallback( .fromOpaque(context) .takeUnretainedValue() let walletId = Data(bytes: walletIdPtr, count: 32) - let txids = handler.walletCoreTxids(walletId: walletId) + let (txids, errored) = handler.walletCoreTxids(walletId: walletId) + guard !errored else { + return -1 + } guard !txids.isEmpty else { return 0 } From ef9b0df7202a45492bc99ac1e3258203da8b1a63 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Wed, 5 Aug 2026 18:20:36 +0300 Subject: [PATCH 3/9] fix(platform-wallet): name the reconstruction's index types for clippy `clippy::type_complexity` is denied workspace-wide and the inline tuple annotation tripped it. Extracting `OwnerContact` and `ContactScriptIndex` also gives the script-pubkey keying an obvious place to be explained. --- .../src/wallet/identity/network/payments.rs | 21 ++++++++++++------- 1 file changed, 13 insertions(+), 8 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs index f67e130a98..68c184750f 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs @@ -222,14 +222,19 @@ impl DashPayView<'_, B> { use dashcore::ScriptBuf; use std::collections::{BTreeMap, BTreeSet}; - // Keyed by script pubkey, not by rendered address: the outputs this - // matches against come from a consensus-decoded `Transaction`, which - // carries scripts. Comparing scripts also sidesteps address-encoding - // pitfalls (network prefix, P2PKH vs P2SH rendering). - let (address_matches, eligible_contacts): ( - BTreeMap, - Vec<(Identifier, Identifier)>, - ) = { + /// The `(owner identity, contact identity)` pair every reconstructed + /// entry is attributed to. + type OwnerContact = (Identifier, Identifier); + /// Script pubkeys derived from the eligible contacts' external + /// accounts, mapped back to the pair that owns each one. + /// + /// Keyed by script pubkey, not by rendered address: the outputs this + /// matches against come from a consensus-decoded `Transaction`, which + /// carries scripts. Comparing scripts also sidesteps address-encoding + /// pitfalls (network prefix, P2PKH vs P2SH rendering). + type ContactScriptIndex = BTreeMap; + + let (address_matches, eligible_contacts): (ContactScriptIndex, Vec) = { let wm = self.wallet_manager.read().await; let info = match wm.get_wallet_info(&self.wallet_id) { Some(info) => info, From 4dfc45452bf3683f5c01401924ae95e4d64d5e95 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Wed, 5 Aug 2026 18:39:06 +0300 Subject: [PATCH 4/9] fix(platform-wallet-ffi): re-pin the vtable layout after the txid append MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `ffi_capability_projection_has_stable_v1_layout_values` pins the callback vtable's size and asserts the last-appended field is terminal. Both move when a slot is added, exactly as they did for invitations and then for `release_fn`. The two txid callbacks sit after `release_fn`, so no previously-defined slot changes offset — which is the property that actually matters for hosts built against an older header, and the reason growth is only ever safe at the end. --- .../rs-platform-wallet-ffi/src/persistence.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 9a3f449bfd..65469d4478 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -5874,21 +5874,25 @@ mod tests { assert_eq!(ffi.bits, 0x81); assert_eq!(std::mem::size_of::(), 16); // Capability negotiation is deliberately NOT appended to the legacy - // callback vtable. Pin the vtable size (invitations + the appended - // `release_fn` context destructor) and prove `release_fn` is the - // terminal field so old clients are never over-read past it. + // callback vtable. Pin the vtable size so a new slot has to be a + // deliberate, reviewed act, and prove the last-appended field really is + // terminal — growth is only safe while it happens at the end, where no + // previously-defined slot changes offset. The count moves with each + // append (invitations, then the `release_fn` context destructor, now + // the txid enumeration pair). #[cfg(not(feature = "shielded"))] assert_eq!( std::mem::size_of::(), - 22 * std::mem::size_of::() + 24 * std::mem::size_of::() ); #[cfg(feature = "shielded")] assert_eq!( std::mem::size_of::(), - 38 * std::mem::size_of::() + 40 * std::mem::size_of::() ); assert_eq!( - std::mem::offset_of!(PersistenceCallbacks, release_fn) + std::mem::size_of::(), + std::mem::offset_of!(PersistenceCallbacks, on_list_wallet_core_txids_free_fn) + + std::mem::size_of::(), std::mem::size_of::() ); assert_eq!( From e3b193c5792c49bc5881c838b1a1d9a8ef5c2685 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Thu, 6 Aug 2026 02:35:57 +0700 Subject: [PATCH 5/9] fix(platform-wallet): harden sent-payment reconstruction against partial scans MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Addresses the four reconstruction blockers from review: - Contact eligibility no longer treats one existing Sent entry as proof of completion. A write that fails after a sibling write succeeded is retried on the next sweep; per-txid dedup already makes re-sweeping recorded entries a no-op. - A listed txid whose record resolves to Ok(None) now marks the scan incomplete. The FFI collapses backend failures, missing/undecodable tx bytes and pending InstantSend rows into a miss, so a miss on a txid the host itself enumerated means "not available yet" — the completion guard stays unstamped and the sweep retries. - Candidate scripts are derived from the contact xpub over the historical range (matched index + gap limit, iterated to a fixed point) on a pool clone, instead of matching only the addresses the restored pool materialized. Payments past the initial gap window are now found after restore-from-seed. - The txid enumeration carries a per-txid spends-wallet-input flag computed by the host from persisted TXO rows (inputs tracked under a watch-only DashPay external account do not count). Transactions the wallet did not fund — a third party paying the watched contact address, or incoming payments — are skipped without a record read and can no longer fabricate Sent history. The get_core_tx_record field contract now requires the real decoded transaction (or None); reconcile_sent_payments reuses the shared finality helper. Co-Authored-By: Claude Fable 5 --- .../rs-platform-wallet-ffi/src/persistence.rs | 80 +- .../rs-platform-wallet/src/changeset/mod.rs | 4 +- .../src/changeset/traits.rs | 61 +- .../src/wallet/identity/network/payments.rs | 718 ++++++++++++++++-- .../src/wallet/persister.rs | 9 +- .../PlatformWalletPersistenceHandler.swift | 77 +- 6 files changed, 821 insertions(+), 128 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 65469d4478..9a3a592916 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -23,7 +23,7 @@ use std::str::FromStr; use crate::types::{FFINetwork, Network}; use platform_wallet::changeset::{ AccountAddressPoolEntry, AccountRegistrationEntry, ClientStartState, ClientWalletStartState, - Merge, PersistenceCapabilities, PersistenceError, PlatformWalletChangeSet, + ListedCoreTxid, Merge, PersistenceCapabilities, PersistenceError, PlatformWalletChangeSet, PlatformWalletPersistence, ProviderKeyAccountEntry, ProviderKeyExtendedPubKey, PERSISTENCE_CAPABILITIES_VERSION, }; @@ -680,7 +680,8 @@ pub struct PersistenceCallbacks { /// callbacks memory-safe. A context needing no cleanup takes a no-op /// `release_fn`; `None` is valid only alongside a null `context`. pub release_fn: Option, - /// Enumerate the persisted Core txids that belong to `wallet_id`. + /// Enumerate the persisted Core txids that belong to `wallet_id`, + /// each tagged with whether the wallet funded the transaction. /// /// Appended at the END so the struct layout stays stable — a host /// built against the previous vtable keeps working, it simply never @@ -694,8 +695,16 @@ pub struct PersistenceCallbacks { /// - Set `*out_txids` to a contiguous buffer of `32 * *out_count` /// bytes, one raw-wire txid per 32-byte chunk, and `*out_count` /// to the number of txids returned. - /// - Set `*out_txids = null` and `*out_count = 0` when no rows - /// exist for the wallet. + /// - Set `*out_flags` to a buffer of `*out_count` bytes, one per + /// txid in the same order. Bit `0x01` means the transaction + /// spends at least one input funded by this wallet's own + /// spendable accounts. Inputs tracked only through a watch-only + /// DashPay external (contact) account do NOT count — those are + /// the contact's coins, and flagging them fabricates `Sent` + /// history for third-party transactions. Remaining bits are + /// reserved and must be zero. + /// - Set `*out_txids = null`, `*out_flags = null` and + /// `*out_count = 0` when no rows exist for the wallet. /// - Return `0` on success; non-zero values are treated as backend /// failures by the Rust side. pub on_list_wallet_core_txids_fn: Option< @@ -703,15 +712,22 @@ pub struct PersistenceCallbacks { context: *mut c_void, wallet_id: *const u8, out_txids: *mut *const u8, + out_flags: *mut *const u8, out_count: *mut usize, ) -> i32, >, - /// Paired free callback for the txid buffer returned by + /// Paired free callback for the txid + flags buffers returned by /// [`Self::on_list_wallet_core_txids_fn`]. Rust invokes this with - /// the same pointer and txid count, exactly once per successful + /// the same pointers and txid count, exactly once per successful /// hit. - pub on_list_wallet_core_txids_free_fn: - Option, + pub on_list_wallet_core_txids_free_fn: Option< + unsafe extern "C" fn( + context: *mut c_void, + txids: *const u8, + flags: *const u8, + count: usize, + ), + >, } // SAFETY: The context pointer is managed by the FFI caller who must ensure @@ -2768,7 +2784,7 @@ impl PlatformWalletPersistence for FFIPersister { fn list_wallet_core_txids( &self, wallet_id: WalletId, - ) -> Result, PersistenceError> { + ) -> Result, PersistenceError> { use dashcore::hashes::Hash; let Some(list_cb) = self.callbacks.on_list_wallet_core_txids_fn else { @@ -2776,6 +2792,7 @@ impl PlatformWalletPersistence for FFIPersister { }; let mut txids_ptr: *const u8 = std::ptr::null(); + let mut flags_ptr: *const u8 = std::ptr::null(); let mut count: usize = 0; let rc = unsafe { @@ -2783,26 +2800,35 @@ impl PlatformWalletPersistence for FFIPersister { self.callbacks.context, wallet_id.as_ptr(), &mut txids_ptr, + &mut flags_ptr, &mut count, ) }; struct TxidBytesGuard { - ptr: *const u8, + txids: *const u8, + flags: *const u8, count: usize, - free_fn: - Option, + free_fn: Option< + unsafe extern "C" fn( + context: *mut c_void, + txids: *const u8, + flags: *const u8, + count: usize, + ), + >, ctx: *mut c_void, } impl Drop for TxidBytesGuard { fn drop(&mut self) { - if let (Some(free), false) = (self.free_fn, self.ptr.is_null()) { - unsafe { free(self.ctx, self.ptr, self.count) }; + if let (Some(free), false) = (self.free_fn, self.txids.is_null()) { + unsafe { free(self.ctx, self.txids, self.flags, self.count) }; } } } let _txid_guard = TxidBytesGuard { - ptr: txids_ptr, + txids: txids_ptr, + flags: flags_ptr, count, free_fn: self.callbacks.on_list_wallet_core_txids_free_fn, ctx: self.callbacks.context, @@ -2816,6 +2842,15 @@ impl PlatformWalletPersistence for FFIPersister { if txids_ptr.is_null() || count == 0 { return Ok(Vec::new()); } + // The flags buffer is not optional once rows exist: without the + // per-txid ownership verdict the reconstruction sweep cannot tell a + // wallet-funded send from a third-party transaction that pays a + // watched contact address. Failing loud beats guessing either way. + if flags_ptr.is_null() { + return Err(PersistenceError::backend( + "on_list_wallet_core_txids_fn returned txids without a flags buffer", + )); + } // Validate the byte length BEFORE building the slice: `from_raw_parts` // requires it to fit in `isize::MAX`, and an implausible `count` from @@ -2833,16 +2868,21 @@ impl PlatformWalletPersistence for FFIPersister { } // SAFETY: the host guarantees `txids_ptr` points to `byte_len` valid - // bytes for the duration of the callback window — `_txid_guard` keeps - // that window open until this function returns — and `byte_len` is - // checked above to be a valid slice length. + // bytes and `flags_ptr` to `count` valid bytes for the duration of + // the callback window — `_txid_guard` keeps that window open until + // this function returns — and both lengths are checked above to be + // valid slice lengths (`count <= byte_len`). let raw = unsafe { slice::from_raw_parts(txids_ptr, byte_len) }; + let flags = unsafe { slice::from_raw_parts(flags_ptr, count) }; let mut out = Vec::with_capacity(count); - for chunk in raw.chunks_exact(32) { + for (chunk, flag) in raw.chunks_exact(32).zip(flags) { let mut bytes = [0u8; 32]; bytes.copy_from_slice(chunk); - out.push(dashcore::Txid::from_byte_array(bytes)); + out.push(ListedCoreTxid { + txid: dashcore::Txid::from_byte_array(bytes), + spends_wallet_input: flag & 0x01 != 0, + }); } Ok(out) } diff --git a/packages/rs-platform-wallet/src/changeset/mod.rs b/packages/rs-platform-wallet/src/changeset/mod.rs index 5d86012215..913ea54d51 100644 --- a/packages/rs-platform-wallet/src/changeset/mod.rs +++ b/packages/rs-platform-wallet/src/changeset/mod.rs @@ -48,4 +48,6 @@ pub use platform_address_sync_start_state::PlatformAddressSyncStartState; pub use shielded_changeset::ShieldedChangeSet; #[cfg(feature = "shielded")] pub use shielded_sync_start_state::{ShieldedSubwalletStartState, ShieldedSyncStartState}; -pub use traits::{PersistenceError, PersistenceErrorKind, PlatformWalletPersistence}; +pub use traits::{ + ListedCoreTxid, PersistenceError, PersistenceErrorKind, PlatformWalletPersistence, +}; diff --git a/packages/rs-platform-wallet/src/changeset/traits.rs b/packages/rs-platform-wallet/src/changeset/traits.rs index 1e3d5ca143..c1dc0d79c9 100644 --- a/packages/rs-platform-wallet/src/changeset/traits.rs +++ b/packages/rs-platform-wallet/src/changeset/traits.rs @@ -12,6 +12,22 @@ use crate::wallet::platform_wallet::WalletId; use dashcore::Txid; use key_wallet::managed_account::transaction_record::TransactionRecord; +/// One row of [`PlatformWalletPersistence::list_wallet_core_txids`]: +/// a persisted Core transaction id plus the host's per-wallet +/// ownership verdict for it. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub struct ListedCoreTxid { + /// The persisted transaction's id. + pub txid: Txid, + /// `true` when the transaction spends at least one input funded by + /// this wallet's own spendable accounts — i.e. the wallet actually + /// paid out in this transaction. `false` for transactions the host + /// persisted for other reasons: incoming payments, and third-party + /// transactions that merely pay an address on a watch-only DashPay + /// external (contact) account. + pub spends_wallet_input: bool, +} + /// Retry classification for [`PersistenceError::Backend`]. /// /// The kind carries the persister's `is_transient()` contract across @@ -322,20 +338,23 @@ pub trait PlatformWalletPersistence: Send + Sync { /// (`SqliteWalletPersister`, the SwiftData iOS persister) should /// override. /// - /// **Field contract.** Implementations are only required to - /// populate `txid` and `context` (with the `BlockInfo` inside - /// `InChainLockedBlock` / `InBlock` carrying real height + block - /// hash + timestamp). Other fields (`transaction`, `input_details`, + /// **Field contract.** Implementations must populate `txid`, + /// `context` (with the `BlockInfo` inside `InChainLockedBlock` / + /// `InBlock` carrying real height + block hash + timestamp) and + /// `transaction` — the real consensus-decoded transaction, never a + /// synthetic body. A backend that cannot produce the real + /// transaction for a txid must return `Ok(None)` instead; + /// DashPay sent-payment reconstruction walks + /// `record.transaction.output` and treats a miss on a txid the + /// backend itself enumerated (via + /// [`Self::list_wallet_core_txids`]) as "not available yet", so a + /// placeholder body would silently corrupt reconstruction where a + /// miss is retried safely. The remaining fields (`input_details`, /// `output_details`, `account_type`, `transaction_type`, /// `direction`, `net_amount`, `fee`, `label`) MAY be returned as - /// best-effort placeholders and MUST NOT be relied upon by callers. - /// The current consumer — the asset-lock proof flow — only reads - /// `context` and `height()` (which is - /// `context.block_info().map(|b| b.height)`). FFI-backed - /// implementations (e.g. the SwiftData iOS persister) take - /// advantage of this contract by emitting a synthetic record with a - /// placeholder transaction body, since reconstructing the full - /// `Transaction` over the C ABI is not free and isn't needed. + /// best-effort placeholders and MUST NOT be relied upon by callers + /// — the asset-lock proof flow reads only `context` and `height()` + /// (which is `context.block_info().map(|b| b.height)`). fn get_core_tx_record( &self, _wallet_id: WalletId, @@ -345,14 +364,28 @@ pub trait PlatformWalletPersistence: Send + Sync { } /// Enumerate the persisted Core transaction ids that belong to - /// `wallet_id`. + /// `wallet_id`, each tagged with whether the transaction spends an + /// input this wallet funded. /// /// Used by DashPay sent-payment reconstruction to walk the /// wallet's locally persisted transaction history without relying /// on the optional in-memory `transactions()` map. The default /// implementation returns an empty set for backwards compatibility /// with backends that don't index wallet-scoped tx history. - fn list_wallet_core_txids(&self, _wallet_id: WalletId) -> Result, PersistenceError> { + /// + /// `spends_wallet_input` must be `true` only when at least one of + /// the transaction's inputs spends an output owned by one of this + /// wallet's own spendable accounts. Watch-only mirrors — a DashPay + /// external account tracking a *contact's* addresses — do NOT + /// count: a third party paying that contact produces a transaction + /// the host persists as wallet-involved, but nothing in it was + /// funded by this wallet. Reconstruction only considers + /// wallet-funded transactions, so an over-broad `true` here turns + /// other people's payments into fabricated `Sent` history. + fn list_wallet_core_txids( + &self, + _wallet_id: WalletId, + ) -> Result, PersistenceError> { Ok(Vec::new()) } diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs index 68c184750f..d97378166a 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs @@ -206,20 +206,25 @@ impl DashPayView<'_, B> { /// every output that pays a contact's external-account address, and /// records a `Sent` entry per `(owner, contact, txid)`. /// - /// Only contacts whose local payment history is still empty are - /// eligible, and each eligible contact is swept at most once per - /// launch. That keeps the recovery path cheap in steady state: - /// after the first "nothing to reconstruct" answer, recurring - /// `dashpay_sync()` passes stop full-scanning persisted tx history - /// every 15 seconds. + /// Each contact is swept at most once per launch. That keeps the + /// recovery path cheap in steady state: after the first sweep, + /// recurring `dashpay_sync()` passes stop full-scanning persisted + /// tx history every 15 seconds. Eligibility deliberately does NOT + /// consult the existing payment map: "the contact already has a + /// `Sent` entry" proves one write landed, not that the contact's + /// history is complete — using it as a completion marker + /// permanently stranded any sibling entry whose write failed after + /// the first one succeeded. The per-txid dedup guard below already + /// makes re-sweeping recorded entries a no-op. /// /// Local-only and idempotent: an existing payment entry under the /// txid is never overwritten. pub async fn reconcile_sent_payments_from_tx_history( &self, ) -> Result { - use crate::wallet::identity::types::dashpay::payment::{PaymentDirection, PaymentStatus}; + use crate::wallet::identity::types::dashpay::payment::PaymentStatus; use dashcore::ScriptBuf; + use key_wallet::managed_account::address_pool::{AddressPool, KeySource}; use std::collections::{BTreeMap, BTreeSet}; /// The `(owner identity, contact identity)` pair every reconstructed @@ -234,14 +239,31 @@ impl DashPayView<'_, B> { /// pitfalls (network prefix, P2PKH vs P2SH rendering). type ContactScriptIndex = BTreeMap; - let (address_matches, eligible_contacts): (ContactScriptIndex, Vec) = { + /// Per-contact derivation context: a private clone of the contact's + /// external address pool plus the key source to extend it with, so + /// the historical range walk below never mutates resident wallet + /// state and runs outside the wallet-manager lock. + struct ContactWindow { + owner: Identifier, + contact: Identifier, + pool: AddressPool, + key_source: KeySource, + } + + let mut windows: Vec = { let wm = self.wallet_manager.read().await; let info = match wm.get_wallet_info(&self.wallet_id) { Some(info) => info, None => return Ok(0), }; - let mut out = BTreeMap::new(); - let mut eligible = Vec::new(); + // The contact xpubs live on the immutable `Account`s in + // `wallet.accounts`; the managed collection only holds pool + // state. Both reads sit under the same read guard. + let wallet = match wm.get_wallet(&self.wallet_id) { + Some(wallet) => wallet, + None => return Ok(0), + }; + let mut out = Vec::new(); for (key, account) in &info.core_wallet.accounts.dashpay_external_accounts { let owner = Identifier::from(key.user_identity_id); let contact = Identifier::from(key.friend_identity_id); @@ -255,91 +277,212 @@ impl DashPayView<'_, B> { { continue; } - // Only a `Sent` entry proves this contact's outgoing history is - // already present. `reconcile_incoming_payments` runs first and - // records `Received` entries, so testing for "any payment with - // this contact" hid every send to a contact we had also - // received from — the incoming pass filled the map, this guard - // read it as done, and the sends were never reconstructed. - if managed.dashpay().payments.values().any(|payment| { - payment.counterparty_id == contact - && payment.direction == PaymentDirection::Sent - }) { - continue; - } let pools = account.managed_account_type().address_pools(); let Some(pool) = pools.first() else { continue; }; - eligible.push((owner, contact)); - for address_info in pool.addresses.values() { - out.entry(address_info.script_pubkey.clone()) - .or_insert((owner, contact)); - } + let key_source = wallet + .accounts + .dashpay_external_accounts + .get(key) + .map(|a| KeySource::Public(a.account_xpub)) + .unwrap_or(KeySource::NoKeySource); + out.push(ContactWindow { + owner, + contact, + pool: (*pool).clone(), + key_source, + }); } - (out, eligible) + out }; - if address_matches.is_empty() { - // Steady state — every contact is either reconstructed or already - // swept this launch. Silent on purpose: this runs on every - // `dashpay_sync` pass, and logging it would emit a line every - // 15 seconds for the life of the process. + if windows.is_empty() { + // Steady state — every contact was already swept this launch. + // Silent on purpose: this runs on every `dashpay_sync` pass, and + // logging it would emit a line every 15 seconds for the life of + // the process. return Ok(0); } tracing::info!( - eligible_contacts = eligible_contacts.len(), - candidate_addresses = address_matches.len(), - "reconcile_sent_payments_from_tx_history: candidate set built" + eligible_contacts = windows.len(), + "reconcile_sent_payments_from_tx_history: candidate contacts selected" ); - let txids = self.persister.list_wallet_core_txids().map_err(|e| { + let listed = self.persister.list_wallet_core_txids().map_err(|e| { PlatformWalletError::Persistence(format!("failed to enumerate wallet txids: {e}")) })?; - let mut totals: BTreeMap<(Identifier, Identifier, String), (u64, PaymentStatus)> = - BTreeMap::new(); - let mut had_read_error = false; - let txid_count = txids.len(); - let mut records_read = 0usize; - let mut outputs_scanned = 0usize; - for txid in txids { - let record = match self.persister.get_core_tx_record(&txid) { - Ok(Some(record)) => record, - Ok(None) => continue, + // Read every wallet-funded record up front. Transactions the wallet + // did not fund (`spends_wallet_input == false`) can never be sent + // payments — the host persists them for incoming detection and for + // third-party transactions that pay a watched contact address, and + // treating those as ours would fabricate `Sent` history. + // + // `incomplete_scan` tracks whether this pass saw the wallet's full + // funded history. A listed txid that resolves to `Ok(None)` counts as + // NOT seen: the FFI record path collapses backend failures, missing + // or undecodable tx bytes, and not-yet-minable InstantSend rows into + // `None`, so a miss on a txid the host itself enumerated means "not + // available yet", never "does not exist". Stamping the per-contact + // completion guard on such a pass would end recovery with records + // still unread, so the guard stays unstamped and the next sweep + // retries. + struct FundedTx { + txid: String, + status: PaymentStatus, + outputs: Vec<(ScriptBuf, u64)>, + } + let mut incomplete_scan = false; + let txid_count = listed.len(); + let mut funded: Vec = Vec::new(); + for entry in listed { + if !entry.spends_wallet_input { + continue; + } + let txid = entry.txid; + match self.persister.get_core_tx_record(&txid) { + Ok(Some(record)) => { + // Walk the decoded transaction's outputs, NOT + // `record.output_details`. Records handed back by + // `get_core_tx_record` are rebuilt from the host's raw + // transaction bytes: `transaction`, `txid` and `context` + // are real, every other field is a placeholder — + // `output_details` is always an empty vec. Matching on it + // silently found nothing. + funded.push(FundedTx { + txid: txid.to_string(), + status: sent_payment_status_for_record(&record), + outputs: record + .transaction + .output + .iter() + .map(|out| (out.script_pubkey.clone(), out.value)) + .collect(), + }); + } + Ok(None) => { + incomplete_scan = true; + tracing::debug!( + %txid, + "reconcile_sent_payments_from_tx_history: listed tx record unavailable; will retry next sweep" + ); + } Err(e) => { - had_read_error = true; + incomplete_scan = true; tracing::warn!( error = %e, %txid, "reconcile_sent_payments_from_tx_history: tx-record read failed; will retry next sweep" ); - continue; } - }; - records_read += 1; - let status = sent_payment_status_for_record(&record); - let txid_str = txid.to_string(); - // Walk the decoded transaction's outputs, NOT `record.output_details`. - // Records handed back by `get_core_tx_record` are rebuilt from the - // host's raw transaction bytes: `transaction`, `txid` and `context` - // are real, every other field is a placeholder — `output_details` is - // always an empty vec. Matching on it silently found nothing. - for out in &record.transaction.output { + } + } + + // Extend each contact's pool clone over the historical range before + // matching. After a restore-from-seed the resident pool holds only + // the initial gap window (20 addresses at index 0..), while the + // persisted history can pay indices past it — live sends only derive + // index N once earlier addresses were marked used, so historical + // usage always chains within the gap limit. Standard BIP44 recovery: + // whenever an observed output matches a derived script, keep the + // window generated through `matched index + gap limit` and rescan + // until no match lands near the frontier. + let observed_scripts: BTreeSet<&ScriptBuf> = funded + .iter() + .flat_map(|tx| tx.outputs.iter().map(|(script, _)| script)) + .collect(); + let mut address_matches: ContactScriptIndex = BTreeMap::new(); + for window in &mut windows { + // A pool restored without any materialized addresses can't seed + // the range walk — generate the initial gap window first. + if window.pool.highest_generated.is_none() && window.key_source.can_derive() { + let initial = window.pool.gap_limit; + if let Err(e) = window + .pool + .generate_addresses(initial, &window.key_source, true) + { + incomplete_scan = true; + tracing::warn!( + error = %e, + owner = %window.owner, + contact = %window.contact, + "reconcile_sent_payments_from_tx_history: initial address derivation failed; will retry next sweep" + ); + } + } + loop { + let highest_matched = window + .pool + .addresses + .values() + .filter(|info| observed_scripts.contains(&info.script_pubkey)) + .map(|info| info.index) + .max(); + let Some(highest_matched) = highest_matched else { + break; + }; + let target = highest_matched.saturating_add(window.pool.gap_limit); + let generated_through = match window.pool.highest_generated { + Some(index) if index >= target => break, + Some(index) => index, + None => break, + }; + if !window.key_source.can_derive() { + // No xpub to extend with — match what the resident pool + // already materialized, but do NOT certify completion: + // history past the materialized window is unreachable + // this pass. + incomplete_scan = true; + tracing::warn!( + owner = %window.owner, + contact = %window.contact, + "reconcile_sent_payments_from_tx_history: external account has no xpub; historical range walk skipped" + ); + break; + } + if let Err(e) = window.pool.generate_addresses( + target - generated_through, + &window.key_source, + true, + ) { + incomplete_scan = true; + tracing::warn!( + error = %e, + owner = %window.owner, + contact = %window.contact, + "reconcile_sent_payments_from_tx_history: address derivation failed; will retry next sweep" + ); + break; + } + } + for address_info in window.pool.addresses.values() { + address_matches + .entry(address_info.script_pubkey.clone()) + .or_insert((window.owner, window.contact)); + } + } + + let mut totals: BTreeMap<(Identifier, Identifier, String), (u64, PaymentStatus)> = + BTreeMap::new(); + let mut outputs_scanned = 0usize; + for tx in &funded { + for (script, value) in &tx.outputs { outputs_scanned += 1; - let Some(&(owner, contact)) = address_matches.get(&out.script_pubkey) else { + let Some(&(owner, contact)) = address_matches.get(script) else { continue; }; let entry = totals - .entry((owner, contact, txid_str.clone())) - .or_insert((0u64, status)); - entry.0 += out.value; - entry.1 = status; + .entry((owner, contact, tx.txid.clone())) + .or_insert((0u64, tx.status)); + entry.0 += value; + entry.1 = tx.status; } } tracing::info!( txids = txid_count, - records_read, + funded_records_read = funded.len(), + candidate_addresses = address_matches.len(), outputs_scanned, matched_txids = totals.len(), "reconcile_sent_payments_from_tx_history: scan complete" @@ -392,18 +535,22 @@ impl DashPayView<'_, B> { // reconstruct. Stamping the guard there would end recovery for the // rest of the process — the exact symptom this pass exists to fix. // Retrying costs one enumeration per sweep, without the per-record - // reads, until the wallet actually has transactions. - if !had_read_error && txid_count > 0 { - for (owner, contact) in eligible_contacts { - if write_failed_for.contains(&(owner, contact)) { + // reads, until the wallet actually has transactions. The same logic + // gates on `incomplete_scan`: a pass that could not read every + // wallet-funded record (or could not derive a contact's historical + // address range) has not proven anything about the records it missed. + if !incomplete_scan && txid_count > 0 { + for window in &windows { + if write_failed_for.contains(&(window.owner, window.contact)) { continue; } - let Some(managed) = info.identity_manager.managed_identity_mut(&owner) else { + let Some(managed) = info.identity_manager.managed_identity_mut(&window.owner) + else { continue; }; managed .dashpay_sent_payment_reconcile_attempted_mut() - .insert(contact); + .insert(window.contact); } } Ok(recorded) @@ -431,7 +578,6 @@ impl DashPayView<'_, B> { /// Returns the number of entries confirmed this pass. pub async fn reconcile_sent_payments(&self) -> Result { use crate::wallet::identity::types::dashpay::payment::{PaymentDirection, PaymentStatus}; - use key_wallet::transaction_checking::TransactionContext; // Snapshot the pending sent (owner, txid) pairs under a read lock so // the persister reads below don't hold the wallet lock across I/O. @@ -474,10 +620,9 @@ impl DashPayView<'_, B> { } }; // An InstantSend lock is final for DashPay display, same as a - // mined block. - let is_final = record.is_confirmed() - || matches!(record.context, TransactionContext::InstantSend(_)); - if !is_final { + // mined block — one definition of "final", shared with the + // reconstruction sweep. + if sent_payment_status_for_record(&record) != PaymentStatus::Confirmed { continue; } // Flip in place via the shared confirm path (re-checks the @@ -1097,8 +1242,9 @@ mod tests { /// Persister that answers `get_core_tx_record` from a configurable /// in-memory map, so a test can stage the persisted core transaction - /// state the sent-payment reconcile reads. `store`/`flush` are no-ops; - /// `load` returns the default state. + /// state the sent-payment reconcile reads. `store`/`flush` are no-ops + /// (unless a store-failure budget is armed); `load` returns the + /// default state. #[derive(Default)] struct RecordStorePersister { records: Mutex< @@ -1107,6 +1253,16 @@ mod tests { key_wallet::managed_account::transaction_record::TransactionRecord, >, >, + /// Txids the enumeration lists but `get_core_tx_record` answers + /// `Ok(None)` for — the FFI shape for "row exists, record not + /// available yet" (missing bytes, undecodable, pending InstantSend). + listed_but_unavailable: Mutex>, + /// Txids the enumeration reports as NOT wallet-funded + /// (`spends_wallet_input == false`). + not_wallet_funded: Mutex>, + /// `Some(n)` lets the next `n` `store` calls succeed and fails every + /// later one until the budget is disarmed (`None` = always succeed). + allow_stores_then_fail: Mutex>, list_wallet_core_txids_calls: Mutex, get_core_tx_record_calls: Mutex, } @@ -1117,7 +1273,15 @@ mod tests { _wallet_id: WalletId, _changeset: PlatformWalletChangeSet, ) -> Result<(), PersistenceError> { - Ok(()) + let mut budget = self.allow_stores_then_fail.lock().unwrap(); + match budget.as_mut() { + Some(0) => Err(PersistenceError::backend("injected store failure")), + Some(remaining) => { + *remaining -= 1; + Ok(()) + } + None => Ok(()), + } } fn flush(&self, _wallet_id: WalletId) -> Result<(), PersistenceError> { Ok(()) @@ -1134,15 +1298,34 @@ mod tests { PersistenceError, > { *self.get_core_tx_record_calls.lock().unwrap() += 1; + if self.listed_but_unavailable.lock().unwrap().contains(txid) { + return Ok(None); + } Ok(self.records.lock().unwrap().get(txid).cloned()) } fn list_wallet_core_txids( &self, _wallet_id: WalletId, - ) -> Result, PersistenceError> { + ) -> Result, PersistenceError> { *self.list_wallet_core_txids_calls.lock().unwrap() += 1; - Ok(self.records.lock().unwrap().keys().copied().collect()) + let not_funded = self.not_wallet_funded.lock().unwrap(); + let unavailable = self.listed_but_unavailable.lock().unwrap(); + let listed: std::collections::BTreeSet = self + .records + .lock() + .unwrap() + .keys() + .copied() + .chain(unavailable.iter().copied()) + .collect(); + Ok(listed + .into_iter() + .map(|txid| crate::changeset::traits::ListedCoreTxid { + txid, + spends_wallet_input: !not_funded.contains(&txid), + }) + .collect()) } } @@ -3402,6 +3585,379 @@ mod tests { ); } + /// A failed persist for one payment must be retried even when a sibling + /// payment to the same contact was written successfully — "the contact + /// has a `Sent` entry" is not a completion marker. + #[tokio::test] + async fn reconcile_sent_payments_from_tx_history_retries_failed_write_after_sibling_success() { + use dashcore::hashes::Hash; + use dashcore::BlockHash; + use key_wallet::managed_account::transaction_record::OutputRole; + use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; + + let persister = Arc::new(RecordStorePersister::default()); + let (manager, wallet_id) = make_wallet_with(Arc::clone(&persister)).await; + let owner = Identifier::from([0xAA; 32]); + let contact = Identifier::from([0xBB; 32]); + + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet.identity(); + let p = WalletPersister::new(wallet_id, Arc::clone(&persister) as _); + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .add_identity(bare_identity([0xAA; 32]), 0, wallet_id, &p) + .expect("add owner"); + } + + let contact_addresses = install_external_account(&manager, wallet_id, owner, contact).await; + let record_a = tx_record_with_outputs( + TransactionContext::InBlock(BlockInfo::new(101, BlockHash::all_zeros(), 0)), + vec![(contact_addresses[0].clone(), 10_000, OutputRole::Sent)], + ); + let record_b = tx_record_with_outputs( + TransactionContext::InBlock(BlockInfo::new(102, BlockHash::all_zeros(), 0)), + vec![(contact_addresses[1].clone(), 20_000, OutputRole::Sent)], + ); + let txid_a = record_a.txid; + let txid_b = record_b.txid; + { + let mut recs = persister.records.lock().unwrap(); + recs.insert(txid_a, record_a); + recs.insert(txid_b, record_b); + } + + // First sweep: one write lands, the second fails. + *persister.allow_stores_then_fail.lock().unwrap() = Some(1); + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("first sweep"), + 1, + "exactly one write should survive the injected failure" + ); + + // Second sweep, persistence healthy again: the stranded payment must + // be recorded even though the contact already has a `Sent` entry. + *persister.allow_stores_then_fail.lock().unwrap() = None; + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("second sweep"), + 1, + "the failed sibling write must be retried on the next sweep" + ); + + { + let wm = iw.wallet_manager.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("info"); + let payments = &info + .identity_manager + .managed_identity(&owner) + .expect("managed") + .dashpay() + .payments; + assert!(payments.contains_key(&txid_a.to_string())); + assert!(payments.contains_key(&txid_b.to_string())); + } + + // Both recorded → the guard is stamped; steady state stops sweeping. + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("third sweep"), + 0 + ); + assert_eq!( + *persister.list_wallet_core_txids_calls.lock().unwrap(), + 2, + "the third sweep must early-exit on the stamped guard" + ); + } + + /// A listed txid whose record comes back `Ok(None)` means "not available + /// yet" (the FFI collapses backend failures, missing bytes and pending + /// InstantSend rows into a miss), so the sweep must not certify the + /// contact as complete until every listed wallet-funded record was read. + #[tokio::test] + async fn reconcile_sent_payments_from_tx_history_retries_when_listed_record_is_unavailable() { + use dashcore::hashes::Hash; + use dashcore::BlockHash; + use key_wallet::managed_account::transaction_record::OutputRole; + use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; + + let persister = Arc::new(RecordStorePersister::default()); + let (manager, wallet_id) = make_wallet_with(Arc::clone(&persister)).await; + let owner = Identifier::from([0xAA; 32]); + let contact = Identifier::from([0xBB; 32]); + + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet.identity(); + let p = WalletPersister::new(wallet_id, Arc::clone(&persister) as _); + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .add_identity(bare_identity([0xAA; 32]), 0, wallet_id, &p) + .expect("add owner"); + } + + let contact_addresses = install_external_account(&manager, wallet_id, owner, contact).await; + let available = tx_record_with_outputs( + TransactionContext::InBlock(BlockInfo::new(77, BlockHash::all_zeros(), 0)), + vec![(contact_addresses[0].clone(), 25_000, OutputRole::Sent)], + ); + let available_txid = available.txid; + persister + .records + .lock() + .unwrap() + .insert(available_txid, available); + let ghost_txid = dashcore::Txid::from([0x42; 32]); + persister + .listed_but_unavailable + .lock() + .unwrap() + .insert(ghost_txid); + + // First sweep records what it can read but must NOT stamp the guard: + // one listed wallet-funded record was unavailable. + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("first sweep"), + 1 + ); + + // The record becomes readable (e.g. the InstantSend row mined) and + // turns out to pay the contact too — the retry must pick it up. + persister + .listed_but_unavailable + .lock() + .unwrap() + .remove(&ghost_txid); + let late = { + let mut record = tx_record_with_outputs( + TransactionContext::InBlock(BlockInfo::new(78, BlockHash::all_zeros(), 0)), + vec![(contact_addresses[1].clone(), 30_000, OutputRole::Sent)], + ); + record.txid = ghost_txid; + record + }; + persister.records.lock().unwrap().insert(ghost_txid, late); + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("second sweep"), + 1, + "an unavailable listed record must keep the sweep retrying" + ); + + // Now conclusive: the guard is stamped and sweeping stops. + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("third sweep"), + 0 + ); + assert_eq!( + *persister.list_wallet_core_txids_calls.lock().unwrap(), + 2, + "the third sweep must early-exit on the stamped guard" + ); + } + + /// Historical sends land past the initial gap window after a + /// restore-from-seed: the resident pool materializes only the first + /// `gap_limit` addresses, while real usage chained further. The sweep + /// must extend its derivation window (matched index + gap limit) instead + /// of matching only what the pool already holds. + #[tokio::test] + async fn reconcile_sent_payments_from_tx_history_finds_payments_past_initial_gap_window() { + use dashcore::hashes::Hash; + use dashcore::BlockHash; + use key_wallet::managed_account::address_pool::KeySource; + use key_wallet::managed_account::transaction_record::OutputRole; + use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; + + let persister = Arc::new(RecordStorePersister::default()); + let (manager, wallet_id) = make_wallet_with(Arc::clone(&persister)).await; + let owner = Identifier::from([0xAA; 32]); + let contact = Identifier::from([0xBB; 32]); + + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet.identity(); + let p = WalletPersister::new(wallet_id, Arc::clone(&persister) as _); + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .add_identity(bare_identity([0xAA; 32]), 0, wallet_id, &p) + .expect("add owner"); + } + + let _ = install_external_account(&manager, wallet_id, owner, contact).await; + + // Derive addresses past the resident window from the same xpub the + // account was installed with, exactly like a live wallet whose sends + // consumed the early indices before the restore. + let (near_address, far_address, far_index) = { + let wm = iw.wallet_manager.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("info"); + let key = DashpayAccountKey { + index: 0, + user_identity_id: owner.to_buffer(), + friend_identity_id: contact.to_buffer(), + }; + let pools = info + .core_wallet + .accounts + .dashpay_external_accounts + .get(&key) + .expect("external account") + .managed_account_type() + .address_pools(); + let pool = (*pools.first().expect("pool")).clone(); + let near_index = pool + .highest_generated + .expect("resident pool has a generated window"); + let far_index = near_index + pool.gap_limit; + let mut extended = pool; + let key_source = KeySource::Public(test_receiving_xpub(&owner, &contact)); + extended + .generate_addresses(extended.gap_limit + 1, &key_source, true) + .expect("extend clone past the resident window"); + ( + extended.addresses[&near_index].address.clone(), + extended.addresses[&far_index].address.clone(), + far_index, + ) + }; + + let near_record = tx_record_with_outputs( + TransactionContext::InBlock(BlockInfo::new(201, BlockHash::all_zeros(), 0)), + vec![(near_address, 10_000, OutputRole::Sent)], + ); + let far_record = tx_record_with_outputs( + TransactionContext::InBlock(BlockInfo::new(202, BlockHash::all_zeros(), 0)), + vec![(far_address, 20_000, OutputRole::Sent)], + ); + let far_txid = far_record.txid; + { + let mut recs = persister.records.lock().unwrap(); + recs.insert(near_record.txid, near_record); + recs.insert(far_txid, far_record); + } + + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("reconcile"), + 2, + "the payment at derivation index {far_index} must be found by the range walk" + ); + let wm = iw.wallet_manager.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("info"); + assert_eq!( + info.identity_manager + .managed_identity(&owner) + .expect("managed") + .dashpay() + .payments + .get(&far_txid.to_string()) + .expect("payment past the initial window") + .amount_duffs, + 20_000 + ); + } + + /// A transaction the wallet did not fund — a third party paying the + /// watched contact address — must never be recorded as `Sent`, and + /// skipping it is conclusive (the guard still stamps). + #[tokio::test] + async fn reconcile_sent_payments_from_tx_history_skips_transactions_wallet_did_not_fund() { + use dashcore::hashes::Hash; + use dashcore::BlockHash; + use key_wallet::managed_account::transaction_record::OutputRole; + use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; + + let persister = Arc::new(RecordStorePersister::default()); + let (manager, wallet_id) = make_wallet_with(Arc::clone(&persister)).await; + let owner = Identifier::from([0xAA; 32]); + let contact = Identifier::from([0xBB; 32]); + + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet.identity(); + let p = WalletPersister::new(wallet_id, Arc::clone(&persister) as _); + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .add_identity(bare_identity([0xAA; 32]), 0, wallet_id, &p) + .expect("add owner"); + } + + let contact_address = install_external_account(&manager, wallet_id, owner, contact) + .await + .remove(0); + let record = tx_record_with_outputs( + TransactionContext::InBlock(BlockInfo::new(300, BlockHash::all_zeros(), 0)), + vec![(contact_address, 40_000, OutputRole::Sent)], + ); + let txid = record.txid; + persister.records.lock().unwrap().insert(txid, record); + persister.not_wallet_funded.lock().unwrap().insert(txid); + + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("reconcile"), + 0, + "a third-party payment to the contact must not become our Sent history" + ); + assert_eq!( + *persister.get_core_tx_record_calls.lock().unwrap(), + 0, + "a non-wallet-funded tx must be skipped without a record read" + ); + { + let wm = iw.wallet_manager.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("info"); + assert!( + info.identity_manager + .managed_identity(&owner) + .expect("managed") + .dashpay() + .payments + .is_empty(), + "no payment entry may be fabricated" + ); + } + + // Skipping unfunded transactions is conclusive — steady state holds. + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("second sweep"), + 0 + ); + assert_eq!( + *persister.list_wallet_core_txids_calls.lock().unwrap(), + 1, + "the second sweep must early-exit on the stamped guard" + ); + } + /// The seedless drain path: `register_external_contact_account` with a /// **precomputed** ECDH shared secret (the Keychain signer computed it; the /// scalar never entered this crate) decrypts the contact's xpub and builds diff --git a/packages/rs-platform-wallet/src/wallet/persister.rs b/packages/rs-platform-wallet/src/wallet/persister.rs index c826e4cb4f..4929957273 100644 --- a/packages/rs-platform-wallet/src/wallet/persister.rs +++ b/packages/rs-platform-wallet/src/wallet/persister.rs @@ -60,9 +60,12 @@ impl WalletPersister { } /// Enumerate the persisted Core transaction ids scoped to this - /// wallet. Used by DashPay sent-payment reconstruction to fetch - /// the full records via [`Self::get_core_tx_record`]. - pub(crate) fn list_wallet_core_txids(&self) -> Result, PersistenceError> { + /// wallet, tagged with the host's wallet-funded verdict. Used by + /// DashPay sent-payment reconstruction to fetch the full records + /// via [`Self::get_core_tx_record`]. + pub(crate) fn list_wallet_core_txids( + &self, + ) -> Result, PersistenceError> { self.inner.list_wallet_core_txids(self.wallet_id) } } diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index f34e0d1f53..cee3dc932d 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -5780,7 +5780,41 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { } } - /// Enumerate the persisted txids scoped to `walletId`. + /// `AccountTypeTagFFI` discriminant for a watch-only DashPay external + /// (contact) account. TXOs tracked under it are the *contact's* coins, + /// mirrored locally so sends to the contact can be detected — they are + /// not spendable by this wallet. + static let dashpayExternalAccountTypeTag: UInt32 = 13 + + /// `true` when `transaction` spends at least one input funded by one of + /// this wallet's own spendable accounts. + /// + /// Pure row data: each entry in `transaction.inputs` is a `PersistentTxo` + /// this transaction spent, carrying the owning wallet denorm and the + /// account it was tracked under. A TXO tracked only by the watch-only + /// DashPay external account does NOT count — those are the contact's + /// coins, and counting them would tag a third party's transaction (the + /// contact spending their own money) as wallet-funded. A TXO whose + /// account link faulted to `nil` counts as owned: spendable-account rows + /// always carry the link, so `nil` is a relationship-store anomaly and + /// under-reporting would silently erase real sent history. + /// `pendingInputs` are deliberately ignored: a spend of our own coins + /// always has its funding TXO persisted (the wallet had to know the + /// output to spend it), while a pending row proves nothing about + /// ownership. + static func walletFundedTransaction( + walletId: Data, + transaction: PersistentTransaction + ) -> Bool { + transaction.inputs.contains { txo in + txo.walletId == walletId + && txo.account.map { $0.accountType != dashpayExternalAccountTypeTag } ?? true + } + } + + /// Enumerate the persisted txids scoped to `walletId`, each paired with + /// whether this wallet funded the transaction (see + /// [`walletFundedTransaction`]). /// /// Scope is the union of wallet-owned TXOs (`outputs`, `inputs`, /// `pendingInputs`) and payload-only account involvement @@ -5789,7 +5823,9 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { /// report a non-zero status. Collapsing a database fault to an empty list /// would be indistinguishable from a wallet with no transactions, and the /// Rust side treats those two very differently. - func walletCoreTxids(walletId: Data) -> (txids: [Data], errored: Bool) { + func walletCoreTxids( + walletId: Data + ) -> (txids: [(txid: Data, spendsWalletInput: Bool)], errored: Bool) { onQueue { let descriptor = FetchDescriptor() let rows: [PersistentTransaction] @@ -5802,8 +5838,17 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { ) return ([], true) } - let txids = rows.compactMap { tx in - Self.walletOwnsTransaction(walletId: walletId, transaction: tx) ? tx.txid : nil + let txids = rows.compactMap { tx -> (txid: Data, spendsWalletInput: Bool)? in + guard Self.walletOwnsTransaction(walletId: walletId, transaction: tx) else { + return nil + } + return ( + txid: tx.txid, + spendsWalletInput: Self.walletFundedTransaction( + walletId: walletId, + transaction: tx + ) + ) } return (txids, false) } @@ -7521,11 +7566,14 @@ private func getCoreTxRecordFreeCallback( } /// C shim for `on_list_wallet_core_txids_fn`. Returns a contiguous -/// `count * 32` byte buffer of raw txids in wire order. +/// `count * 32` byte buffer of raw txids in wire order plus a parallel +/// `count`-byte flags buffer (bit `0x01` = the wallet funded the +/// transaction). private func listWalletCoreTxidsCallback( context: UnsafeMutableRawPointer?, walletIdPtr: UnsafePointer?, outTxids: UnsafeMutablePointer?>?, + outFlags: UnsafeMutablePointer?>?, outCount: UnsafeMutablePointer? ) -> Int32 { // Non-zero on a missing argument: reporting success here would hand Rust @@ -7534,11 +7582,13 @@ private func listWalletCoreTxidsCallback( guard let context = context, let walletIdPtr = walletIdPtr, let outTxids = outTxids, + let outFlags = outFlags, let outCount = outCount else { return -1 } outTxids.pointee = nil + outFlags.pointee = nil outCount.pointee = 0 let handler = Unmanaged @@ -7554,19 +7604,23 @@ private func listWalletCoreTxidsCallback( } let buffer = UnsafeMutablePointer.allocate(capacity: txids.count * 32) + let flags = UnsafeMutablePointer.allocate(capacity: txids.count) // Pack only well-formed txids and report how many were packed. Skipping a // malformed one while still reporting `txids.count` would leave its slot // uninitialized and hand Rust 32 bytes of garbage as a txid. var packed = 0 - for txid in txids where txid.count == 32 { - txid.copyBytes(to: buffer.advanced(by: packed * 32), count: 32) + for row in txids where row.txid.count == 32 { + row.txid.copyBytes(to: buffer.advanced(by: packed * 32), count: 32) + flags.advanced(by: packed).pointee = row.spendsWalletInput ? 0x01 : 0x00 packed += 1 } guard packed > 0 else { buffer.deallocate() + flags.deallocate() return 0 } outTxids.pointee = UnsafePointer(buffer) + outFlags.pointee = UnsafePointer(flags) outCount.pointee = UInt(packed) return 0 } @@ -7575,9 +7629,14 @@ private func listWalletCoreTxidsCallback( private func listWalletCoreTxidsFreeCallback( context: UnsafeMutableRawPointer?, txids: UnsafePointer?, + flags: UnsafePointer?, _ count: UInt ) { - guard let txids = txids else { return } - UnsafeMutablePointer(mutating: txids).deallocate() + if let txids = txids { + UnsafeMutablePointer(mutating: txids).deallocate() + } + if let flags = flags { + UnsafeMutablePointer(mutating: flags).deallocate() + } _ = context } From 54758c2b75bc77a717a6251330836534067973b9 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Wed, 5 Aug 2026 23:41:43 +0300 Subject: [PATCH 6/9] fix(platform-wallet): certify a scan only when history is settled MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from the review of e3b193c. **A partially populated transaction table was treated as complete.** Distinguishing an empty enumeration from a completed one was not enough: any non-empty prefix still certified the snapshot. The sweep runs on a timer, so it can read the table while a DashPay rescan is still delivering rows into it — one early row makes `txid_count > 0`, the per-launch guard gets stamped, and every row arriving afterwards is ignored for the rest of the process. `reconcile_dashpay_rescan` now records the tip it rewound from, and the sweep refuses to certify a contact while `synced_height` is still below that mark. In-memory like `rescan_triggered`, and self-healing for the same reason: a relaunch restores `synced_height` at its high-water and re-arms the mark with the re-triggered backfill. **Legacy TXOs were classified as unfunded.** `PersistentTxo.walletId` is empty on rows written before the denormalized field existed, and `loadWalletList` already resolves those through the owning account's wallet. Both `walletOwnsTransaction` and `walletFundedTransaction` compared the raw field, so a real spend of an untouched legacy TXO read as "not ours" — the transaction was skipped and the contact could still be stamped. Both now share one resolver that prefers the populated field and falls back to the account's wallet, while still excluding the watch-only DashPay external account from funded inputs. **Txid-buffer ownership contradicted its own contract.** The free callback is documented as transferring ownership on success only, but the guard was installed before the status check, so a buffer returned alongside a failure was freed too — a double free for any host that cleans up its own failed allocation. The guard now goes up after the check and fires when either output pointer is non-null, so a flags-only allocation from a malformed success can't leak. --- .../rs-platform-wallet-ffi/src/persistence.rs | 33 ++++- .../src/wallet/identity/network/payments.rs | 128 ++++++++++++++++++ .../state/managed_identity/dashpay.rs | 17 +++ .../identity/state/managed_identity/mod.rs | 8 ++ .../PlatformWalletPersistenceHandler.swift | 41 +++++- 5 files changed, 214 insertions(+), 13 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 9a3a592916..c338c24b38 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -720,6 +720,13 @@ pub struct PersistenceCallbacks { /// [`Self::on_list_wallet_core_txids_fn`]. Rust invokes this with /// the same pointers and txid count, exactly once per successful /// hit. + /// + /// Ownership transfers on success ONLY: when the enumeration callback + /// returns non-zero, Rust does not call this and the host keeps + /// whatever it allocated (same contract as + /// [`Self::on_load_wallet_list_free_fn`]). On success it is called + /// whenever either output pointer is non-null, so a host that emits + /// only one of the two buffers still gets it released. pub on_list_wallet_core_txids_free_fn: Option< unsafe extern "C" fn( context: *mut c_void, @@ -2821,11 +2828,30 @@ impl PlatformWalletPersistence for FFIPersister { } impl Drop for TxidBytesGuard { fn drop(&mut self) { - if let (Some(free), false) = (self.free_fn, self.txids.is_null()) { + // Either output pointer being non-null means the host handed + // over an allocation. Gating on `txids` alone would leak a + // flags-only buffer from a malformed but successful callback. + if let (Some(free), true) = + (self.free_fn, !self.txids.is_null() || !self.flags.is_null()) + { unsafe { free(self.ctx, self.txids, self.flags, self.count) }; } } } + + // Ownership transfers on success only — the same contract + // `on_load_wallet_list_fn` documents and `load` implements by building + // its guard after the status check. Installing the guard first would + // free a buffer the host still owns on the failure path, which is a + // double free for any host that cleans up its own failed allocation. + if rc != 0 { + return Err(PersistenceError::backend(format!( + "on_list_wallet_core_txids_fn returned non-zero status {rc}" + ))); + } + + // Success: ownership is ours now, and every return below must release + // it — including the error paths that reject a malformed buffer. let _txid_guard = TxidBytesGuard { txids: txids_ptr, flags: flags_ptr, @@ -2834,11 +2860,6 @@ impl PlatformWalletPersistence for FFIPersister { ctx: self.callbacks.context, }; - if rc != 0 { - return Err(PersistenceError::backend(format!( - "on_list_wallet_core_txids_fn returned non-zero status {rc}" - ))); - } if txids_ptr.is_null() || count == 0 { return Ok(Vec::new()); } diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs index d97378166a..09fd84b9f6 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs @@ -182,6 +182,16 @@ impl DashPayView<'_, B> { for (owner, contact) in to_mark { if let Some(managed) = info.identity_manager.managed_identity_mut(&owner) { managed.dashpay_rescan_triggered_mut().insert(contact); + // Arm the completeness mark whenever the tip was actually + // rewound: until the backfill climbs back to `synced_height` + // as it stood a moment ago, the wallet's transaction table is + // still being filled and no snapshot of it is conclusive. + // Keep the highest target if several rescans overlap — the + // last one to finish is the one that matters. + if floor.is_some() { + let target = managed.dashpay_rescan_backfill_target_mut(); + *target = Some(target.map_or(synced_height, |cur| cur.max(synced_height))); + } } } if let Some(floor) = floor { @@ -225,6 +235,7 @@ impl DashPayView<'_, B> { use crate::wallet::identity::types::dashpay::payment::PaymentStatus; use dashcore::ScriptBuf; use key_wallet::managed_account::address_pool::{AddressPool, KeySource}; + use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use std::collections::{BTreeMap, BTreeSet}; /// The `(owner identity, contact identity)` pair every reconstructed @@ -539,6 +550,15 @@ impl DashPayView<'_, B> { // gates on `incomplete_scan`: a pass that could not read every // wallet-funded record (or could not derive a contact's historical // address range) has not proven anything about the records it missed. + // A non-empty enumeration is not proof that history is complete. This + // sweep runs on a timer and can snapshot the transaction table while a + // DashPay rescan is still delivering rows into it — one early row is + // enough to make `txid_count > 0` true. Certifying that snapshot would + // stamp the guard and ignore every row that lands afterwards, which is + // the same permanent-miss this pass exists to prevent, just narrower. + // So refuse to certify while the backfill has not climbed back to the + // tip it rewound from. + let synced_height = info.core_wallet.synced_height(); if !incomplete_scan && txid_count > 0 { for window in &windows { if write_failed_for.contains(&(window.owner, window.contact)) { @@ -548,6 +568,13 @@ impl DashPayView<'_, B> { else { continue; }; + if managed + .dashpay() + .rescan_backfill_target + .is_some_and(|target| synced_height < target) + { + continue; + } managed .dashpay_sent_payment_reconcile_attempted_mut() .insert(window.contact); @@ -3517,6 +3544,107 @@ mod tests { /// Once a sweep has actually scanned transactions, repeating it is pure /// overhead — the guard stops the full walk on every later pass. + /// A rescan still delivering rows must not let the sweep certify the scan. + /// + /// `txid_count > 0` only says the snapshot was non-empty. The sweep runs + /// on a timer, so it can read the transaction table while the DashPay + /// backfill is mid-flight: one early row makes the count positive, the + /// guard gets stamped, and every row that arrives afterwards is ignored + /// for the rest of the process. + #[tokio::test] + async fn reconcile_sent_payments_from_tx_history_waits_for_the_rescan_backfill() { + use dashcore::hashes::Hash; + use dashcore::BlockHash; + use key_wallet::managed_account::transaction_record::OutputRole; + use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; + use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; + + let persister = Arc::new(RecordStorePersister::default()); + let (manager, wallet_id) = make_wallet_with(Arc::clone(&persister)).await; + let owner = Identifier::from([0xAA; 32]); + let contact = Identifier::from([0xBB; 32]); + + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet.identity(); + let p = WalletPersister::new(wallet_id, Arc::clone(&persister) as _); + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .add_identity(bare_identity([0xAA; 32]), 0, wallet_id, &p) + .expect("add owner"); + } + + let contact_address = install_external_account(&manager, wallet_id, owner, contact) + .await + .remove(0); + let record = tx_record_with_outputs( + TransactionContext::InBlock(BlockInfo::new(40, BlockHash::all_zeros(), 0)), + vec![(contact_address, 30_000, OutputRole::Sent)], + ); + persister + .records + .lock() + .unwrap() + .insert(record.txid, record); + + // Backfill in flight: the tip was rewound from 1000 and has only + // climbed back to 500. + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.core_wallet.update_synced_height(500); + *info + .identity_manager + .managed_identity_mut(&owner) + .expect("managed") + .dashpay_rescan_backfill_target_mut() = Some(1000); + } + + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("reconcile"); + { + let wm = iw.wallet_manager.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("info"); + assert!( + !info + .identity_manager + .managed_identity(&owner) + .expect("managed") + .dashpay() + .sent_payment_reconcile_attempted + .contains(&contact), + "a snapshot taken mid-backfill must not certify the contact" + ); + } + + // Backfill complete: the tip is back at the mark. + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.core_wallet.update_synced_height(1000); + } + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("reconcile"); + { + let wm = iw.wallet_manager.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("info"); + assert!( + info.identity_manager + .managed_identity(&owner) + .expect("managed") + .dashpay() + .sent_payment_reconcile_attempted + .contains(&contact), + "once the backfill reaches its mark the scan is conclusive" + ); + } + } + #[tokio::test] async fn reconcile_sent_payments_from_tx_history_skips_repeat_sweeps_after_a_real_scan() { use dashcore::hashes::Hash; diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/dashpay.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/dashpay.rs index 3e1cb19bd6..1b924e9a6d 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/dashpay.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/dashpay.rs @@ -127,6 +127,23 @@ pub struct DashPayState { /// backfill durable across a crash if that ever becomes necessary. pub rescan_triggered: BTreeSet, + /// SPV scan tip captured immediately before a DashPay rescan lowered it — + /// the height the backfill has to climb back to before the wallet's + /// transaction history covers the rewound range again. + /// + /// Sent-payment reconstruction reads this as its completeness signal. A + /// non-empty enumeration is NOT proof that history is complete: the sweep + /// runs on a timer and can snapshot the transaction table while the + /// backfill is still delivering rows into it. Certifying that snapshot + /// would stamp the per-launch guard and ignore every row that arrives + /// afterwards. While `synced_height` is below this mark the scan is + /// treated as incomplete instead. + /// + /// In-memory only, same contract as [`Self::rescan_triggered`]: a relaunch + /// clears it, and `synced_height` is restored at its monotonic high-water, + /// so an interrupted backfill re-triggers and re-arms this mark. + pub rescan_backfill_target: Option, + /// DashPay contact-crypto ops the unattended background sweep enqueued for /// THIS identity but could not perform because key material was unavailable /// (watch-only / signer locked). Drained when a signer is available diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs index 8cb633206f..bac99db93a 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs @@ -158,6 +158,14 @@ impl ManagedIdentity { &mut self.dashpay.rescan_triggered } + /// Mutable access to the rescan backfill high-water mark. + /// + /// In-memory only — see [`DashPayState::rescan_backfill_target`] for why + /// sent-payment reconstruction cannot certify a scan below it. + pub fn dashpay_rescan_backfill_target_mut(&mut self) -> &mut Option { + &mut self.dashpay.rescan_backfill_target + } + /// Mutable access to the deferred contact-crypto queue. /// /// The queue's dedup invariant (≤ 1 entry per diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index cee3dc932d..31f5271697 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -27,26 +27,48 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { } } + /// Wallet a TXO belongs to, resolved the way `loadWalletList` already + /// resolves it. + /// + /// `PersistentTxo.walletId` is a denormalized convenience field and is + /// **empty on rows written before it existed**. Comparing it raw makes + /// every legacy TXO look like it belongs to no wallet — which, for + /// sent-payment reconstruction, silently reclassifies a real spend as + /// "not ours" and drops the payment. Fall back to the owning account's + /// wallet for those rows. + /// + /// `account.wallet` is non-optional on the model but is a fault-loaded + /// relationship, so it is read through an Optional cast: a + /// relationship-store inconsistency would otherwise crash here. + static func resolvedWalletId(of txo: PersistentTxo) -> Data? { + if !txo.walletId.isEmpty { + return txo.walletId + } + let account: PersistentAccount? = txo.account + guard let account else { return nil } + let wallet: PersistentWallet? = account.wallet + return wallet?.walletId + } + static func walletOwnsTransaction( walletId: Data, transaction: PersistentTransaction ) -> Bool { - // `account.wallet` is non-optional on the model but is a fault-loaded - // relationship; a relationship-store inconsistency would crash here, - // so guard via Optional cast (same treatment as the UTXO bucketing in - // `loadWalletList`). if transaction.involvedAccounts.contains(where: { let wallet: PersistentWallet? = $0.wallet return wallet?.walletId == walletId }) { return true } - if transaction.outputs.contains(where: { $0.walletId == walletId }) { + if transaction.outputs.contains(where: { resolvedWalletId(of: $0) == walletId }) { return true } - if transaction.inputs.contains(where: { $0.walletId == walletId }) { + if transaction.inputs.contains(where: { resolvedWalletId(of: $0) == walletId }) { return true } + // `PersistentPendingInput` carries no account relationship, so its + // denormalized `walletId` is the only thing to compare — it is also a + // newer row type, written only by the current send path. return transaction.pendingInputs.contains(where: { $0.walletId == walletId }) } @@ -5807,7 +5829,12 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { transaction: PersistentTransaction ) -> Bool { transaction.inputs.contains { txo in - txo.walletId == walletId + // Resolved, not raw: a legacy TXO with an empty denormalized + // `walletId` is still our coin, and reading it as "not ours" turns + // a real spend into an unfunded transaction — the sweep then skips + // it and can still stamp the contact, losing the payment for the + // process lifetime. + Self.resolvedWalletId(of: txo) == walletId && txo.account.map { $0.accountType != dashpayExternalAccountTypeTag } ?? true } } From af67c4ec55d01172ac83d6b44a0618f41c95896f Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Thu, 6 Aug 2026 22:34:39 +0300 Subject: [PATCH 7/9] fix(platform-wallet): certify the sweep against a scan height, not a flag MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two blockers from the review of de9ea9b. **A partial transaction snapshot could be certified as complete.** The previous guard was a boolean armed by `reconcile_dashpay_rescan`, but `sync_wallet_dashpay` runs the reconstruction sweep *before* that reconcile — so the first post-restore pass saw no target, took whatever prefix of the transaction table was visible as the whole of history, and stamped the contact. The same gap remained on an initial or forward-only scan, where `reconcile_dashpay_rescan` arms nothing at all because there is no rewind, yet rows keep arriving with each new block. The marker is now the `synced_height` a sweep certified against, not a flag. A pass is conclusive for the table as it stood at that height and says nothing about later blocks, so a contact becomes eligible again the moment the height advances — which is exactly when new rows can appear. That needs no "history is settled" signal (nothing in the persistence callbacks reports one) and removes the ordering dependency entirely: a rewind lowers the height, so the stamp stops matching whichever reconcile ran first. In steady state the height stops moving and so does the sweep. `rescan_backfill_target` existed only to paper over the ordering and is deleted. **The derivation walk could not cross a full unused gap.** It extended the pool only past an address it had already seen paid, so a stretch of unused indices stopped it. Those stretches are reachable: `send_payment` marks the chosen contact address used before `build_signed` and does not roll that back when the build fails, so after enough failures a real payment sits past a hole no on-chain output bridges — and a restored pool stops short of it. The seed window is now five gap limits wide and is derived whether or not the pool already materialized addresses, so the walk no longer depends on an earlier match. Both regression tests were verified to fail without their fix. --- .../src/wallet/identity/network/payments.rs | 317 +++++++++++++----- .../state/managed_identity/dashpay.rs | 47 +-- .../identity/state/managed_identity/mod.rs | 20 +- 3 files changed, 258 insertions(+), 126 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs index 09fd84b9f6..3129c6390e 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs @@ -182,16 +182,6 @@ impl DashPayView<'_, B> { for (owner, contact) in to_mark { if let Some(managed) = info.identity_manager.managed_identity_mut(&owner) { managed.dashpay_rescan_triggered_mut().insert(contact); - // Arm the completeness mark whenever the tip was actually - // rewound: until the backfill climbs back to `synced_height` - // as it stood a moment ago, the wallet's transaction table is - // still being filled and no snapshot of it is conclusive. - // Keep the highest target if several rescans overlap — the - // last one to finish is the one that matters. - if floor.is_some() { - let target = managed.dashpay_rescan_backfill_target_mut(); - *target = Some(target.map_or(synced_height, |cur| cur.max(synced_height))); - } } } if let Some(floor) = floor { @@ -238,6 +228,16 @@ impl DashPayView<'_, B> { use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use std::collections::{BTreeMap, BTreeSet}; + /// How many gap limits wide to derive before the match-driven walk + /// starts, so a stretch of unused indices cannot stop it. + /// + /// Unused stretches come from sends that consumed an address and then + /// failed to build; five gap limits (100 addresses at the DIP-15 gap of + /// 20) covers far more consecutive failures than a contact realistically + /// accumulates, and the walk still extends past it whenever a match + /// lands near the frontier. + const HISTORICAL_SEED_GAP_MULTIPLE: u32 = 5; + /// The `(owner identity, contact identity)` pair every reconstructed /// entry is attributed to. type OwnerContact = (Identifier, Identifier); @@ -274,6 +274,11 @@ impl DashPayView<'_, B> { Some(wallet) => wallet, None => return Ok(0), }; + // The scan height this pass will certify against. Read once so + // eligibility and the stamp below agree even if a concurrent SPV + // pass advances it mid-sweep — a later height simply makes the + // contact eligible again next time. + let synced_height = info.core_wallet.synced_height(); let mut out = Vec::new(); for (key, account) in &info.core_wallet.accounts.dashpay_external_accounts { let owner = Identifier::from(key.user_identity_id); @@ -281,10 +286,14 @@ impl DashPayView<'_, B> { let Some(managed) = info.identity_manager.managed_identity(&owner) else { continue; }; + // Swept already — but only for the history that existed at + // that height. Newly scanned blocks are exactly when new rows + // can appear, so any advance makes the contact eligible again. if managed .dashpay() - .sent_payment_reconcile_attempted - .contains(&contact) + .sent_payment_reconcile_swept_at + .get(&contact) + .is_some_and(|swept_at| *swept_at >= synced_height) { continue; } @@ -404,21 +413,40 @@ impl DashPayView<'_, B> { .collect(); let mut address_matches: ContactScriptIndex = BTreeMap::new(); for window in &mut windows { - // A pool restored without any materialized addresses can't seed - // the range walk — generate the initial gap window first. - if window.pool.highest_generated.is_none() && window.key_source.can_derive() { - let initial = window.pool.gap_limit; - if let Err(e) = window + // Seed the walk with a window WIDER than one gap limit, and do it + // whether or not the pool already materialized addresses. + // + // The match-driven loop below only extends past an address it has + // already seen paid, so it cannot cross a stretch of unused + // indices. Those stretches are reachable in practice: `send_payment` + // marks the chosen contact address used before `build_signed`, and a + // failed build never rolls that back. After enough failures a later + // successful payment lands past a hole no on-chain output bridges, + // and on restore the recreated pool stops short of it — the sweep + // then finds nothing and stamps the contact as swept. + // + // Deriving a fixed bounded window first removes the dependency on an + // earlier match. Cost is one derivation per contact per launch. + if window.key_source.can_derive() { + let want = window .pool - .generate_addresses(initial, &window.key_source, true) - { - incomplete_scan = true; - tracing::warn!( - error = %e, - owner = %window.owner, - contact = %window.contact, - "reconcile_sent_payments_from_tx_history: initial address derivation failed; will retry next sweep" - ); + .gap_limit + .saturating_mul(HISTORICAL_SEED_GAP_MULTIPLE); + let have = window.pool.highest_generated.map_or(0, |i| i + 1); + if have < want { + if let Err(e) = + window + .pool + .generate_addresses(want - have, &window.key_source, true) + { + incomplete_scan = true; + tracing::warn!( + error = %e, + owner = %window.owner, + contact = %window.contact, + "reconcile_sent_payments_from_tx_history: seed address derivation failed; will retry next sweep" + ); + } } } loop { @@ -557,7 +585,18 @@ impl DashPayView<'_, B> { // stamp the guard and ignore every row that lands afterwards, which is // the same permanent-miss this pass exists to prevent, just narrower. // So refuse to certify while the backfill has not climbed back to the - // tip it rewound from. + // Record the height this pass certified, not a bare "done". The scan + // is conclusive for the transaction table as it stood at + // `synced_height`; it says nothing about rows that arrive with later + // blocks. Stamping a flag instead would end recovery on whatever + // prefix of history happened to be visible — and nothing in the + // persistence callbacks reports whether the host has finished + // delivering it. + // + // A height also removes any ordering requirement between this sweep + // and `reconcile_dashpay_rescan`: a rewind lowers `synced_height`, so + // the stamp stops matching and the contact is swept again on the way + // back up, whichever ran first. let synced_height = info.core_wallet.synced_height(); if !incomplete_scan && txid_count > 0 { for window in &windows { @@ -568,16 +607,9 @@ impl DashPayView<'_, B> { else { continue; }; - if managed - .dashpay() - .rescan_backfill_target - .is_some_and(|target| synced_height < target) - { - continue; - } managed - .dashpay_sent_payment_reconcile_attempted_mut() - .insert(window.contact); + .dashpay_sent_payment_reconcile_swept_at_mut() + .insert(window.contact, synced_height); } } Ok(recorded) @@ -3542,22 +3574,20 @@ mod tests { ); } - /// Once a sweep has actually scanned transactions, repeating it is pure - /// overhead — the guard stops the full walk on every later pass. - /// A rescan still delivering rows must not let the sweep certify the scan. + /// A stretch of unused indices must not stop the walk. /// - /// `txid_count > 0` only says the snapshot was non-empty. The sweep runs - /// on a timer, so it can read the transaction table while the DashPay - /// backfill is mid-flight: one early row makes the count positive, the - /// guard gets stamped, and every row that arrives afterwards is ignored - /// for the rest of the process. + /// `send_payment` marks the chosen contact address used before + /// `build_signed` and never rolls that back when the build fails, so a + /// contact's real payment can sit past a hole no on-chain output bridges. + /// The match-driven extension alone cannot cross that hole — it only + /// extends past an address it has already seen paid — so the seed window + /// has to be wider than one gap limit. #[tokio::test] - async fn reconcile_sent_payments_from_tx_history_waits_for_the_rescan_backfill() { + async fn reconcile_sent_payments_from_tx_history_crosses_a_full_unused_gap() { use dashcore::hashes::Hash; use dashcore::BlockHash; use key_wallet::managed_account::transaction_record::OutputRole; use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; - use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; let persister = Arc::new(RecordStorePersister::default()); let (manager, wallet_id) = make_wallet_with(Arc::clone(&persister)).await; @@ -3575,72 +3605,193 @@ mod tests { .expect("add owner"); } - let contact_address = install_external_account(&manager, wallet_id, owner, contact) - .await - .remove(0); + let _ = install_external_account(&manager, wallet_id, owner, contact).await; + + // An address a full gap limit past the materialized frontier, with + // NOTHING paid in between — the hole a run of failed builds leaves. + let (beyond_gap, materialized_max) = { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + let key = DashpayAccountKey { + index: 0, + user_identity_id: owner.to_buffer(), + friend_identity_id: contact.to_buffer(), + }; + let account = info + .core_wallet + .accounts + .dashpay_external_accounts + .get(&key) + .expect("external account"); + let pools = account.managed_account_type().address_pools(); + let pool = *pools.first().expect("pool"); + let materialized_max = pool.addresses.keys().copied().max().unwrap_or(0); + let target = materialized_max + pool.gap_limit + 1; + let mut scan = pool.clone(); + let key_source = key_wallet::KeySource::Public(test_receiving_xpub(&owner, &contact)); + scan.generate_addresses(target + 1, &key_source, true) + .expect("derive past the hole"); + ( + scan.addresses + .get(&target) + .expect("target derived") + .address + .clone(), + materialized_max, + ) + }; + let record = tx_record_with_outputs( - TransactionContext::InBlock(BlockInfo::new(40, BlockHash::all_zeros(), 0)), - vec![(contact_address, 30_000, OutputRole::Sent)], + TransactionContext::InBlock(BlockInfo::new(88, BlockHash::all_zeros(), 0)), + vec![(beyond_gap, 60_000, OutputRole::Sent)], ); - persister - .records - .lock() - .unwrap() - .insert(record.txid, record); + let txid = record.txid; + persister.records.lock().unwrap().insert(txid, record); + + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("reconcile"), + 1, + "a payment past a full unused gap (materialized up to {materialized_max}) must still be found" + ); + { + let wm = iw.wallet_manager.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("info"); + assert_eq!( + info.identity_manager + .managed_identity(&owner) + .expect("managed") + .dashpay() + .payments + .get(&txid.to_string()) + .expect("reconstructed entry") + .amount_duffs, + 60_000 + ); + } + } + + /// A sweep certifies only the history that existed at the height it ran + /// against, so the contact becomes eligible again as soon as the scan + /// advances. + /// + /// The alternative — a bare "already swept" flag — ends recovery on + /// whatever prefix of the transaction table happened to be visible. That + /// prefix is not under our control: `dashpay_sync` runs this sweep before + /// `reconcile_dashpay_rescan`, and on an initial or forward-only scan rows + /// keep arriving with every new block. + #[tokio::test] + async fn reconcile_sent_payments_from_tx_history_resweeps_when_the_scan_advances() { + use dashcore::hashes::Hash; + use dashcore::BlockHash; + use key_wallet::managed_account::transaction_record::OutputRole; + use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; + use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; - // Backfill in flight: the tip was rewound from 1000 and has only - // climbed back to 500. + let persister = Arc::new(RecordStorePersister::default()); + let (manager, wallet_id) = make_wallet_with(Arc::clone(&persister)).await; + let owner = Identifier::from([0xAA; 32]); + let contact = Identifier::from([0xBB; 32]); + + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet.identity(); + let p = WalletPersister::new(wallet_id, Arc::clone(&persister) as _); { let mut wm = iw.wallet_manager.write().await; let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .add_identity(bare_identity([0xAA; 32]), 0, wallet_id, &p) + .expect("add owner"); info.core_wallet.update_synced_height(500); - *info - .identity_manager - .managed_identity_mut(&owner) - .expect("managed") - .dashpay_rescan_backfill_target_mut() = Some(1000); } + let addresses = install_external_account(&manager, wallet_id, owner, contact).await; + let own_address = first_standard_wallet_address(&manager, wallet_id).await; + + // First pass at height 500: one unrelated transaction exists, so the + // scan is conclusive for what it saw and stamps that height. + let unrelated = tx_record_with_outputs( + TransactionContext::InBlock(BlockInfo::new(10, BlockHash::all_zeros(), 0)), + vec![(own_address, 1_000, OutputRole::Sent)], + ); + persister + .records + .lock() + .unwrap() + .insert(unrelated.txid, unrelated); iw.dashpay() .reconcile_sent_payments_from_tx_history() .await - .expect("reconcile"); + .expect("first sweep"); { let wm = iw.wallet_manager.read().await; let info = wm.get_wallet_info(&wallet_id).expect("info"); - assert!( - !info - .identity_manager + assert_eq!( + info.identity_manager .managed_identity(&owner) .expect("managed") .dashpay() - .sent_payment_reconcile_attempted - .contains(&contact), - "a snapshot taken mid-backfill must not certify the contact" + .sent_payment_reconcile_swept_at + .get(&contact), + Some(&500), + "the sweep records the height it certified" ); } - // Backfill complete: the tip is back at the mark. + // A payment arrives with a later block — a row the first pass could + // not have seen. + let late = tx_record_with_outputs( + TransactionContext::InBlock(BlockInfo::new(700, BlockHash::all_zeros(), 0)), + vec![(addresses[0].clone(), 40_000, OutputRole::Sent)], + ); + let late_txid = late.txid; + persister.records.lock().unwrap().insert(late_txid, late); + + // Still at 500: the stamp holds, nothing re-runs. + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("sweep at the same height"), + 0, + "an unchanged scan height must not re-run the walk" + ); + + // The scan advances past it — eligibility returns and the payment is + // recovered. A flag-based guard would have lost it permanently. { let mut wm = iw.wallet_manager.write().await; let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); - info.core_wallet.update_synced_height(1000); + info.core_wallet.update_synced_height(800); } - iw.dashpay() - .reconcile_sent_payments_from_tx_history() - .await - .expect("reconcile"); + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("sweep after the scan advanced"), + 1, + "a higher scan height must re-open the contact" + ); { let wm = iw.wallet_manager.read().await; let info = wm.get_wallet_info(&wallet_id).expect("info"); - assert!( - info.identity_manager - .managed_identity(&owner) - .expect("managed") + let managed = info + .identity_manager + .managed_identity(&owner) + .expect("managed"); + assert!(managed + .dashpay() + .payments + .contains_key(&late_txid.to_string())); + assert_eq!( + managed .dashpay() - .sent_payment_reconcile_attempted - .contains(&contact), - "once the backfill reaches its mark the scan is conclusive" + .sent_payment_reconcile_swept_at + .get(&contact), + Some(&800), + "the stamp moves to the newly certified height" ); } } diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/dashpay.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/dashpay.rs index 1b924e9a6d..35f469da8d 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/dashpay.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/dashpay.rs @@ -89,20 +89,26 @@ pub struct DashPayState { /// identity, with direction, amount, memo, and status. pub payments: BTreeMap, - /// Contacts whose historical sent-payment reconstruction sweep has - /// already run in this process lifetime. + /// SPV scan height at which each contact's historical sent-payment + /// reconstruction sweep last completed. /// - /// `reconcile_sent_payments_from_tx_history` is a restore-time - /// recovery path for contacts whose local payment cache is still - /// empty. Once a contact has either been reconstructed or proven to - /// have nothing to reconstruct, re-running the full persisted-tx - /// scan every recurring sync pass is pure overhead. This guard - /// suppresses that steady-state rescan. + /// `reconcile_sent_payments_from_tx_history` is a restore-time recovery + /// path. Re-running its full persisted-tx scan every recurring sync pass + /// is pure overhead once a contact has been reconstructed — but "already + /// swept" is only a safe answer for the history that existed at the time. /// - /// In-memory only (never persisted): a relaunch retries the sweep - /// once for still-empty contacts, which is safe and far cheaper than - /// re-scanning every sync pass forever. - pub sent_payment_reconcile_attempted: BTreeSet, + /// Hence a height, not a flag. A sweep certifies the transaction table *as + /// of* `synced_height`; the contact becomes eligible again the moment that + /// height advances, because newly scanned blocks are exactly when new rows + /// can appear. Nothing has to know whether the host has "finished" + /// delivering history — an answer no callback provides — and no ordering + /// between this sweep and the rescan reconcile has to hold. + /// + /// In steady state the height stops moving and the sweep stops running. + /// + /// In-memory only (never persisted): a relaunch re-sweeps once per contact, + /// which is safe and far cheaper than re-scanning every pass forever. + pub sent_payment_reconcile_swept_at: BTreeMap, /// Cached **contact** profiles keyed by the contact's identity id — /// established contacts, pending incoming-request senders, and (later) @@ -127,23 +133,6 @@ pub struct DashPayState { /// backfill durable across a crash if that ever becomes necessary. pub rescan_triggered: BTreeSet, - /// SPV scan tip captured immediately before a DashPay rescan lowered it — - /// the height the backfill has to climb back to before the wallet's - /// transaction history covers the rewound range again. - /// - /// Sent-payment reconstruction reads this as its completeness signal. A - /// non-empty enumeration is NOT proof that history is complete: the sweep - /// runs on a timer and can snapshot the transaction table while the - /// backfill is still delivering rows into it. Certifying that snapshot - /// would stamp the per-launch guard and ignore every row that arrives - /// afterwards. While `synced_height` is below this mark the scan is - /// treated as incomplete instead. - /// - /// In-memory only, same contract as [`Self::rescan_triggered`]: a relaunch - /// clears it, and `synced_height` is restored at its monotonic high-water, - /// so an interrupted backfill re-triggers and re-arms this mark. - pub rescan_backfill_target: Option, - /// DashPay contact-crypto ops the unattended background sweep enqueued for /// THIS identity but could not perform because key material was unavailable /// (watch-only / signer locked). Drained when a signer is available diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs index bac99db93a..0d1ab3150f 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs @@ -123,15 +123,15 @@ impl ManagedIdentity { &mut self.dashpay.payments } - /// Mutable access to the per-session sent-payment reconcile guard. + /// Mutable access to the per-contact sent-payment sweep heights. /// /// In-memory only — never persisted; see the field docs on - /// [`DashPayState::sent_payment_reconcile_attempted`] for the - /// relaunch contract. - pub fn dashpay_sent_payment_reconcile_attempted_mut( + /// [`DashPayState::sent_payment_reconcile_swept_at`] for why the guard is + /// a height rather than a flag. + pub fn dashpay_sent_payment_reconcile_swept_at_mut( &mut self, - ) -> &mut std::collections::BTreeSet { - &mut self.dashpay.sent_payment_reconcile_attempted + ) -> &mut std::collections::BTreeMap { + &mut self.dashpay.sent_payment_reconcile_swept_at } /// Mutable access to the cached contact profiles. @@ -158,14 +158,6 @@ impl ManagedIdentity { &mut self.dashpay.rescan_triggered } - /// Mutable access to the rescan backfill high-water mark. - /// - /// In-memory only — see [`DashPayState::rescan_backfill_target`] for why - /// sent-payment reconstruction cannot certify a scan below it. - pub fn dashpay_rescan_backfill_target_mut(&mut self) -> &mut Option { - &mut self.dashpay.rescan_backfill_target - } - /// Mutable access to the deferred contact-crypto queue. /// /// The queue's dedup invariant (≤ 1 entry per From b55002f74eeb310771b0cf71378786327c92ebed Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 7 Aug 2026 03:34:04 +0700 Subject: [PATCH 8/9] fix(platform-wallet): certify reconstruction against the scanned table itself MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Replaces the scan-height guard with a digest of the enumerated (txid, wallet-funded) rows, and stops failed sends from consuming payment addresses. A chain height was never a completeness signal for the persisted transaction table: the sweep could stamp tip H before the rescan coordinator rewound below it (excluding the whole backfill until H+1), the wallet-event adapter commits rows asynchronously behind the in-memory height, mempool rows arrive with no height advance at all, and a second height read after the persistence I/O could certify a height the pass never inspected. Stamping the digest of exactly the enumeration the pass scanned closes all four: any row that lands afterwards changes the next enumeration's digest and re-opens the affected contacts, whichever order the reconciles ran in, and there is no second read to race. It also ends the scan-per-block regression — an unchanged table costs later sweeps one txid enumeration and zero record reads, and a block with no new wallet rows re-triggers nothing. The five-gap seed window guarded against failed sends marching the next derivation index past any recoverable range, but nothing bounded the march. Bound it at the source instead: a failed build_signed now returns the consumed address to the pool (state -> Available) before anything was persisted or broadcast, restoring the BIP44 invariant that used indices chain within the gap limit. The seed window stays as tolerance for histories written before this fix. Co-Authored-By: Claude Fable 5 --- .../src/wallet/identity/network/payments.rs | 458 +++++++++++++----- .../state/managed_identity/dashpay.rs | 32 +- .../identity/state/managed_identity/mod.rs | 12 +- 3 files changed, 354 insertions(+), 148 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs index 3129c6390e..8001b8b543 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs @@ -206,16 +206,21 @@ impl DashPayView<'_, B> { /// every output that pays a contact's external-account address, and /// records a `Sent` entry per `(owner, contact, txid)`. /// - /// Each contact is swept at most once per launch. That keeps the - /// recovery path cheap in steady state: after the first sweep, - /// recurring `dashpay_sync()` passes stop full-scanning persisted - /// tx history every 15 seconds. Eligibility deliberately does NOT - /// consult the existing payment map: "the contact already has a - /// `Sent` entry" proves one write landed, not that the contact's - /// history is complete — using it as a completion marker - /// permanently stranded any sibling entry whose write failed after - /// the first one succeeded. The per-txid dedup guard below already - /// makes re-sweeping recorded entries a no-op. + /// Each contact is swept once per distinct state of the persisted + /// transaction table (see + /// [`DashPayState::sent_payment_reconcile_swept_table`](crate::wallet::identity::state::managed_identity::dashpay::DashPayState::sent_payment_reconcile_swept_table)): + /// a full scan certifies the exact enumeration it inspected, and + /// re-runs only when the table's digest changes. That keeps the + /// recovery path cheap in steady state — one txid enumeration per + /// recurring `dashpay_sync()` pass, no record reads — while any new + /// row (rescan backfill, asynchronous persistence, mempool) makes + /// the affected wallet's contacts eligible again. Eligibility + /// deliberately does NOT consult the existing payment map: "the + /// contact already has a `Sent` entry" proves one write landed, not + /// that the contact's history is complete — using it as a + /// completion marker permanently stranded any sibling entry whose + /// write failed after the first one succeeded. The per-txid dedup + /// guard below already makes re-sweeping recorded entries a no-op. /// /// Local-only and idempotent: an existing payment entry under the /// txid is never overwritten. @@ -225,7 +230,6 @@ impl DashPayView<'_, B> { use crate::wallet::identity::types::dashpay::payment::PaymentStatus; use dashcore::ScriptBuf; use key_wallet::managed_account::address_pool::{AddressPool, KeySource}; - use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use std::collections::{BTreeMap, BTreeSet}; /// How many gap limits wide to derive before the match-driven walk @@ -261,6 +265,65 @@ impl DashPayView<'_, B> { key_source: KeySource, } + // Pass 1 (cheap, read lock): every external-account contact and the + // table digest it was last certified against. No pool clones yet — + // in steady state this pass plus one txid enumeration is the whole + // sweep. + let contact_digests: Vec<(Identifier, Identifier, Option<[u8; 32]>)> = { + let wm = self.wallet_manager.read().await; + let info = match wm.get_wallet_info(&self.wallet_id) { + Some(info) => info, + None => return Ok(0), + }; + let mut out = Vec::new(); + for key in info.core_wallet.accounts.dashpay_external_accounts.keys() { + let owner = Identifier::from(key.user_identity_id); + let contact = Identifier::from(key.friend_identity_id); + let Some(managed) = info.identity_manager.managed_identity(&owner) else { + continue; + }; + let stored = managed + .dashpay() + .sent_payment_reconcile_swept_table + .get(&contact) + .copied(); + out.push((owner, contact, stored)); + } + out + }; + if contact_digests.is_empty() { + return Ok(0); + } + + let listed = self.persister.list_wallet_core_txids().map_err(|e| { + PlatformWalletError::Persistence(format!("failed to enumerate wallet txids: {e}")) + })?; + // The digest this pass will certify. Computed from exactly the rows + // enumerated here, so the stamp below can never claim more than this + // pass inspected: rows that land after this enumeration — a rescan + // backfill filling the table back in, the wallet-event adapter + // committing asynchronously behind the in-memory chain height, a + // mempool transaction with no height advance at all — change the next + // enumeration's digest and make every stamped contact eligible again. + let table_digest = wallet_tx_table_digest(&listed); + + let stale: BTreeSet<(Identifier, Identifier)> = contact_digests + .iter() + .filter(|(_, _, stored)| *stored != Some(table_digest)) + .map(|(owner, contact, _)| (*owner, *contact)) + .collect(); + if stale.is_empty() { + // Steady state — every contact was certified against exactly this + // table. Silent on purpose: this runs on every `dashpay_sync` + // pass, and logging it would emit a line every 15 seconds for the + // life of the process. + return Ok(0); + } + + // Pass 2 (read lock): derivation context for the stale contacts only — + // a private clone of each contact's external address pool plus its + // xpub, so the historical range walk below never mutates resident + // wallet state and runs outside the wallet-manager lock. let mut windows: Vec = { let wm = self.wallet_manager.read().await; let info = match wm.get_wallet_info(&self.wallet_id) { @@ -274,27 +337,11 @@ impl DashPayView<'_, B> { Some(wallet) => wallet, None => return Ok(0), }; - // The scan height this pass will certify against. Read once so - // eligibility and the stamp below agree even if a concurrent SPV - // pass advances it mid-sweep — a later height simply makes the - // contact eligible again next time. - let synced_height = info.core_wallet.synced_height(); let mut out = Vec::new(); for (key, account) in &info.core_wallet.accounts.dashpay_external_accounts { let owner = Identifier::from(key.user_identity_id); let contact = Identifier::from(key.friend_identity_id); - let Some(managed) = info.identity_manager.managed_identity(&owner) else { - continue; - }; - // Swept already — but only for the history that existed at - // that height. Newly scanned blocks are exactly when new rows - // can appear, so any advance makes the contact eligible again. - if managed - .dashpay() - .sent_payment_reconcile_swept_at - .get(&contact) - .is_some_and(|swept_at| *swept_at >= synced_height) - { + if !stale.contains(&(owner, contact)) { continue; } let pools = account.managed_account_type().address_pools(); @@ -317,10 +364,6 @@ impl DashPayView<'_, B> { out }; if windows.is_empty() { - // Steady state — every contact was already swept this launch. - // Silent on purpose: this runs on every `dashpay_sync` pass, and - // logging it would emit a line every 15 seconds for the life of - // the process. return Ok(0); } tracing::info!( @@ -328,10 +371,6 @@ impl DashPayView<'_, B> { "reconcile_sent_payments_from_tx_history: candidate contacts selected" ); - let listed = self.persister.list_wallet_core_txids().map_err(|e| { - PlatformWalletError::Persistence(format!("failed to enumerate wallet txids: {e}")) - })?; - // Read every wallet-funded record up front. Transactions the wallet // did not fund (`spends_wallet_input == false`) can never be sent // payments — the host persists them for incoming detection and for @@ -567,37 +606,25 @@ impl DashPayView<'_, B> { recorded += 1; } - // An enumeration that came back empty proves nothing: after a restore - // the recurring `dashpay_sync()` can fire before the host has finished - // repopulating its transaction table, and a zero-txid sweep is - // indistinguishable from a wallet that genuinely has nothing to - // reconstruct. Stamping the guard there would end recovery for the - // rest of the process — the exact symptom this pass exists to fix. - // Retrying costs one enumeration per sweep, without the per-record - // reads, until the wallet actually has transactions. The same logic - // gates on `incomplete_scan`: a pass that could not read every - // wallet-funded record (or could not derive a contact's historical - // address range) has not proven anything about the records it missed. - // A non-empty enumeration is not proof that history is complete. This - // sweep runs on a timer and can snapshot the transaction table while a - // DashPay rescan is still delivering rows into it — one early row is - // enough to make `txid_count > 0` true. Certifying that snapshot would - // stamp the guard and ignore every row that lands afterwards, which is - // the same permanent-miss this pass exists to prevent, just narrower. - // So refuse to certify while the backfill has not climbed back to the - // Record the height this pass certified, not a bare "done". The scan - // is conclusive for the transaction table as it stood at - // `synced_height`; it says nothing about rows that arrive with later - // blocks. Stamping a flag instead would end recovery on whatever - // prefix of history happened to be visible — and nothing in the - // persistence callbacks reports whether the host has finished - // delivering it. + // Stamp the digest of exactly the enumeration this pass scanned — + // never a bare "done". A pass is conclusive only for that snapshot of + // the table; any row that lands afterwards (rescan backfill, + // asynchronous wallet-event persistence at an unchanged height, a + // mempool transaction) changes the next enumeration's digest, so the + // contact is swept again whichever order this sweep and + // `reconcile_dashpay_rescan` ran in. A table that has not changed + // costs later passes one enumeration and no record reads. // - // A height also removes any ordering requirement between this sweep - // and `reconcile_dashpay_rescan`: a rewind lowers `synced_height`, so - // the stamp stops matching and the contact is swept again on the way - // back up, whichever ran first. - let synced_height = info.core_wallet.synced_height(); + // An enumeration that came back empty still proves nothing: after a + // restore the recurring `dashpay_sync()` can fire before the host has + // repopulated any of its transaction table, and a zero-txid sweep is + // indistinguishable from a wallet that genuinely has nothing to + // reconstruct. Stamping there would end recovery until the digest + // next changes, so leave the guard unstamped until at least one row + // exists. The same logic gates on `incomplete_scan`: a pass that + // could not read every wallet-funded record (or could not derive a + // contact's historical address range) has not proven anything about + // the records it missed. if !incomplete_scan && txid_count > 0 { for window in &windows { if write_failed_for.contains(&(window.owner, window.contact)) { @@ -608,8 +635,8 @@ impl DashPayView<'_, B> { continue; }; managed - .dashpay_sent_payment_reconcile_swept_at_mut() - .insert(window.contact, synced_height); + .dashpay_sent_payment_reconcile_swept_table_mut() + .insert(window.contact, table_digest); } } Ok(recorded) @@ -814,6 +841,31 @@ fn record_received_payment_totals( recorded } +/// Order-independent digest of an enumerated wallet transaction table: +/// SHA-256 over the sorted `(txid, spends_wallet_input)` rows. +/// +/// This is what the sent-payment reconstruction sweep stamps per contact — +/// the pass certifies exactly the rows it enumerated, nothing beyond them. +/// The funded flag is part of the digest on purpose: a host correcting a +/// row's wallet-funded attribution changes the table's meaning for the +/// sweep without adding or removing a txid, and must re-trigger it. +/// In-memory only, never persisted — no cross-version stability required. +fn wallet_tx_table_digest(listed: &[crate::changeset::traits::ListedCoreTxid]) -> [u8; 32] { + use dashcore::hashes::{sha256, Hash, HashEngine}; + + let mut rows: Vec<([u8; 32], bool)> = listed + .iter() + .map(|entry| (*entry.txid.as_byte_array(), entry.spends_wallet_input)) + .collect(); + rows.sort_unstable(); + let mut engine = sha256::Hash::engine(); + for (txid, funded) in rows { + engine.input(&txid); + engine.input(&[funded as u8]); + } + sha256::Hash::from_engine(engine).to_byte_array() +} + fn sent_payment_status_for_record( record: &key_wallet::managed_account::transaction_record::TransactionRecord, ) -> crate::wallet::identity::types::dashpay::payment::PaymentStatus { @@ -1145,12 +1197,53 @@ impl DashPayView<'_, B> { // dropped sub-dust change remainder included — since // rust-dashcore#872 (pinned above). No caller-side // recomputation needed. - let (tx, fee) = builder + let (tx, fee) = match builder .build_signed(signer, |addr| { managed_account.address_derivation_path(&addr) }) .await - .map_err(|e| PlatformWalletError::TransactionBuild(e.to_string()))?; + { + Ok(built) => built, + Err(e) => { + // Return the consumed address to the pool. Nothing was + // signed to completion, persisted (the used-flip store + // below is unreachable from here) or broadcast, so the + // mark exists only in this process's memory and the + // address was never exposed on-chain — un-marking it + // cannot break DIP-15 rotation. Leaving it consumed + // would let every failed build (insufficient funds + // retried by the user, a signer refusing) advance the + // next index by one with no bound; enough failures + // before one successful payment would put that payment + // beyond any gap-limit walk a restore-from-seed can + // perform, permanently hiding it from sent-payment + // reconstruction. Used indices must chain within the + // gap limit, so consumption is committed only once a + // fully signed transaction exists. + if let Some(external_account) = info + .core_wallet + .accounts + .dashpay_external_accounts + .get_mut(&key) + { + for pool in external_account + .managed_account_type_mut() + .address_pools_mut() + { + let Some(&index) = pool.address_index.get(&payment_address) else { + continue; + }; + pool.used_indices.remove(&index); + if let Some(address_info) = pool.addresses.get_mut(&index) { + address_info.state = + key_wallet::managed_account::address_pool::AddressState::Available; + } + pool.highest_used = pool.used_indices.iter().max().copied(); + } + } + return Err(PlatformWalletError::TransactionBuild(e.to_string())); + } + }; (payment_address, used_flip_changeset, tx, fee) }; @@ -1164,10 +1257,8 @@ impl DashPayView<'_, B> { // payments on-chain. A store failure aborts the send pre-broadcast // (nothing has hit the network); the consumed in-memory address only // leaves a one-address gap that the pool's gap window absorbs on - // retry. A funding-build failure above returns before this point, so - // an address consumed for a send that never broadcasts is likewise - // left only in memory — safe to re-hand, since it was never exposed - // on-chain. + // retry — bounded, because a signed transaction exists here, unlike + // the unbounded build-failure case rolled back above. self.persister.store(used_flip_changeset).map_err(|e| { PlatformWalletError::Persistence(format!( "failed to persist payment-address used flip: {e}" @@ -3683,12 +3774,11 @@ mod tests { /// `reconcile_dashpay_rescan`, and on an initial or forward-only scan rows /// keep arriving with every new block. #[tokio::test] - async fn reconcile_sent_payments_from_tx_history_resweeps_when_the_scan_advances() { + async fn reconcile_sent_payments_from_tx_history_resweeps_when_the_table_changes() { use dashcore::hashes::Hash; use dashcore::BlockHash; use key_wallet::managed_account::transaction_record::OutputRole; use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; - use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; let persister = Arc::new(RecordStorePersister::default()); let (manager, wallet_id) = make_wallet_with(Arc::clone(&persister)).await; @@ -3704,14 +3794,13 @@ mod tests { info.identity_manager .add_identity(bare_identity([0xAA; 32]), 0, wallet_id, &p) .expect("add owner"); - info.core_wallet.update_synced_height(500); } let addresses = install_external_account(&manager, wallet_id, owner, contact).await; let own_address = first_standard_wallet_address(&manager, wallet_id).await; - // First pass at height 500: one unrelated transaction exists, so the - // scan is conclusive for what it saw and stamps that height. + // First pass: one unrelated transaction exists, so the scan is + // conclusive for the table it enumerated and stamps its digest. let unrelated = tx_record_with_outputs( TransactionContext::InBlock(BlockInfo::new(10, BlockHash::all_zeros(), 0)), vec![(own_address, 1_000, OutputRole::Sent)], @@ -3728,72 +3817,65 @@ mod tests { { let wm = iw.wallet_manager.read().await; let info = wm.get_wallet_info(&wallet_id).expect("info"); - assert_eq!( + assert!( info.identity_manager .managed_identity(&owner) .expect("managed") .dashpay() - .sent_payment_reconcile_swept_at - .get(&contact), - Some(&500), - "the sweep records the height it certified" + .sent_payment_reconcile_swept_table + .contains_key(&contact), + "the sweep records the table digest it certified" ); } - // A payment arrives with a later block — a row the first pass could - // not have seen. + // A payment row lands AFTER the certified pass — a rescan backfill + // delivering history, or the wallet-event adapter committing + // asynchronously. No chain-height advance is involved: the row's + // arrival alone changes the table digest, so the very next sweep + // recovers it. (The prior height-stamped guard ignored rows like + // this until another block happened to arrive.) let late = tx_record_with_outputs( TransactionContext::InBlock(BlockInfo::new(700, BlockHash::all_zeros(), 0)), vec![(addresses[0].clone(), 40_000, OutputRole::Sent)], ); let late_txid = late.txid; persister.records.lock().unwrap().insert(late_txid, late); - - // Still at 500: the stamp holds, nothing re-runs. assert_eq!( iw.dashpay() .reconcile_sent_payments_from_tx_history() .await - .expect("sweep at the same height"), - 0, - "an unchanged scan height must not re-run the walk" - ); - - // The scan advances past it — eligibility returns and the payment is - // recovered. A flag-based guard would have lost it permanently. - { - let mut wm = iw.wallet_manager.write().await; - let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); - info.core_wallet.update_synced_height(800); - } - assert_eq!( - iw.dashpay() - .reconcile_sent_payments_from_tx_history() - .await - .expect("sweep after the scan advanced"), + .expect("sweep after a new row"), 1, - "a higher scan height must re-open the contact" + "a changed table must re-open the contact immediately" ); { let wm = iw.wallet_manager.read().await; let info = wm.get_wallet_info(&wallet_id).expect("info"); - let managed = info + assert!(info .identity_manager .managed_identity(&owner) - .expect("managed"); - assert!(managed + .expect("managed") .dashpay() .payments .contains_key(&late_txid.to_string())); - assert_eq!( - managed - .dashpay() - .sent_payment_reconcile_swept_at - .get(&contact), - Some(&800), - "the stamp moves to the newly certified height" - ); } + + // Unchanged table: the new stamp holds — one enumeration, no record + // reads, nothing recorded. + let reads_before = *persister.get_core_tx_record_calls.lock().unwrap(); + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("sweep on the unchanged table"), + 0, + "an unchanged table must not re-run the walk" + ); + assert_eq!( + *persister.get_core_tx_record_calls.lock().unwrap(), + reads_before, + "an unchanged table must not re-read any records" + ); } #[tokio::test] @@ -3854,13 +3936,13 @@ mod tests { } assert_eq!( *persister.list_wallet_core_txids_calls.lock().unwrap(), - 1, - "steady state must not keep re-enumerating txids every sweep" + 2, + "every sweep pays exactly one txid enumeration to detect table changes" ); assert_eq!( *persister.get_core_tx_record_calls.lock().unwrap(), 1, - "the second pass must early-exit before any tx-record fetch" + "the second pass must early-exit on the digest before any tx-record fetch" ); } @@ -3943,7 +4025,9 @@ mod tests { assert!(payments.contains_key(&txid_b.to_string())); } - // Both recorded → the guard is stamped; steady state stops sweeping. + // Both recorded → the digest is stamped; an unchanged table costs + // later sweeps one enumeration and zero record reads. + let reads_after_success = *persister.get_core_tx_record_calls.lock().unwrap(); assert_eq!( iw.dashpay() .reconcile_sent_payments_from_tx_history() @@ -3952,9 +4036,9 @@ mod tests { 0 ); assert_eq!( - *persister.list_wallet_core_txids_calls.lock().unwrap(), - 2, - "the third sweep must early-exit on the stamped guard" + *persister.get_core_tx_record_calls.lock().unwrap(), + reads_after_success, + "the third sweep must early-exit on the digest before any tx-record fetch" ); } @@ -4038,7 +4122,9 @@ mod tests { "an unavailable listed record must keep the sweep retrying" ); - // Now conclusive: the guard is stamped and sweeping stops. + // Now conclusive: the digest is stamped and record reads stop while + // the table stays unchanged. + let reads_after_success = *persister.get_core_tx_record_calls.lock().unwrap(); assert_eq!( iw.dashpay() .reconcile_sent_payments_from_tx_history() @@ -4047,9 +4133,9 @@ mod tests { 0 ); assert_eq!( - *persister.list_wallet_core_txids_calls.lock().unwrap(), - 2, - "the third sweep must early-exit on the stamped guard" + *persister.get_core_tx_record_calls.lock().unwrap(), + reads_after_success, + "the third sweep must early-exit on the digest before any tx-record fetch" ); } @@ -4231,9 +4317,9 @@ mod tests { 0 ); assert_eq!( - *persister.list_wallet_core_txids_calls.lock().unwrap(), - 1, - "the second sweep must early-exit on the stamped guard" + *persister.get_core_tx_record_calls.lock().unwrap(), + 0, + "the second sweep must early-exit on the digest before any tx-record fetch" ); } @@ -5192,6 +5278,116 @@ mod tests { } } + /// A failed `build_signed` must return the consumed payment address to + /// the pool. Without the rollback every failed build (insufficient + /// funds, a refusing signer) permanently advances the next index by one: + /// enough failures before one successful payment put that payment past + /// any gap-limit walk a restore-from-seed can perform, and sent-payment + /// reconstruction never finds it. + #[tokio::test] + async fn send_payment_failed_build_returns_the_address_to_the_pool() { + use crate::wallet::identity::network::contact_requests::SeedCryptoProvider; + + let (manager, persister, wallet_id) = make_wallet().await; + let wallet_arc = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet_arc.identity(); + + let owner_id = Identifier::from([0x11; 32]); + let contact_id = Identifier::from([0x22; 32]); + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .add_identity( + bare_identity([0x11; 32]), + 0, + wallet_id, + &WalletPersister::new(wallet_id, Arc::clone(&persister) as _), + ) + .expect("add owner"); + } + + let shared_key = [0x55u8; 32]; + let iv = [0x11u8; 16]; + let compact = { + let seed = Mnemonic::from_phrase(TEST_MNEMONIC, Language::English) + .expect("mnemonic") + .to_seed(""); + let w = key_wallet::wallet::Wallet::from_seed_bytes( + seed, + Network::Testnet, + WalletAccountCreationOptions::None, + ) + .expect("seed wallet"); + crate::wallet::identity::crypto::dip14::derive_contact_xpub( + &w, + Network::Testnet, + 0, + &owner_id, + &contact_id, + ) + .expect("derive a valid compact xpub") + .compact + .to_bytes() + }; + let encrypted = + platform_encryption::encrypt_extended_public_key(&shared_key, &iv, &compact); + let contact = bare_identity([0x22; 32]); + iw.dashpay() + .register_external_contact_account( + &owner_id, + &contact, + &encrypted, + zeroize::Zeroizing::new(shared_key), + ) + .await + .expect("register external account"); + + let seed = Mnemonic::from_phrase(TEST_MNEMONIC, Language::English) + .expect("valid mnemonic") + .to_seed(""); + let provider = SeedCryptoProvider::from_seed(seed, Network::Testnet); + let signer = SeedSigner::new(seed, Network::Testnet); + + // Two failed builds in a row: without rollback each one consumes an + // index and the pool's used range marches forward off-chain. + for attempt in 1..=2 { + iw.dashpay() + .send_payment(&owner_id, &contact_id, 10_000, None, &signer, &provider) + .await + .expect_err( + "seedless test wallet has no UTXOs, so the build must fail \ + (attempt {attempt})", + ); + let _ = attempt; + } + + let wm = iw.wallet_manager.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("info"); + let key = DashpayAccountKey { + index: 0, + user_identity_id: owner_id.to_buffer(), + friend_identity_id: contact_id.to_buffer(), + }; + let account = info + .core_wallet + .accounts + .dashpay_external_accounts + .get(&key) + .expect("external account present"); + let pools = account.managed_account_type().address_pools(); + let pool = pools.first().expect("external pool"); + assert!( + pool.used_indices.is_empty(), + "a failed build must not leave any address consumed, found {:?}", + pool.used_indices + ); + assert_eq!( + pool.highest_used, None, + "no on-chain use happened, so the pool's used high-water must stay unset" + ); + } + /// The `send_payment` used-flag flip persist must run only AFTER the /// wallet-manager write guard is released (and before the broadcast). /// diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/dashpay.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/dashpay.rs index 35f469da8d..fb981b0ebd 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/dashpay.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/dashpay.rs @@ -89,26 +89,36 @@ pub struct DashPayState { /// identity, with direction, amount, memo, and status. pub payments: BTreeMap, - /// SPV scan height at which each contact's historical sent-payment - /// reconstruction sweep last completed. + /// Digest of the persisted transaction table against which each + /// contact's historical sent-payment reconstruction sweep last completed. /// /// `reconcile_sent_payments_from_tx_history` is a restore-time recovery /// path. Re-running its full persisted-tx scan every recurring sync pass /// is pure overhead once a contact has been reconstructed — but "already - /// swept" is only a safe answer for the history that existed at the time. + /// swept" is only a safe answer for the table contents the sweep actually + /// inspected. /// - /// Hence a height, not a flag. A sweep certifies the transaction table *as - /// of* `synced_height`; the contact becomes eligible again the moment that - /// height advances, because newly scanned blocks are exactly when new rows - /// can appear. Nothing has to know whether the host has "finished" - /// delivering history — an answer no callback provides — and no ordering - /// between this sweep and the rescan reconcile has to hold. + /// Hence a digest of the enumerated `(txid, wallet-funded)` rows, not a + /// flag and not a chain height. The sweep stamps exactly the snapshot it + /// scanned, so any change to the table — a rescan backfill delivering + /// rows, the wallet-event adapter committing rows asynchronously behind + /// the in-memory height, a mempool transaction with no height advance, a + /// host fixing a row's funded attribution — changes the digest and makes + /// the contact eligible again. Nothing has to know whether the host has + /// "finished" delivering history (an answer no callback provides), no + /// ordering between this sweep and the rescan reconcile has to hold, and + /// a chain-height advance with no new wallet rows does NOT re-trigger the + /// scan. A height stamp had both failure modes: it certified rows the + /// pass never saw (committed later at the same height, or delivered by a + /// backfill running below an already-stamped height) and re-ran the full + /// scan on every block. /// - /// In steady state the height stops moving and the sweep stops running. + /// In steady state the table stops changing and the sweep stops at one + /// cheap txid enumeration per pass — no record reads, no derivations. /// /// In-memory only (never persisted): a relaunch re-sweeps once per contact, /// which is safe and far cheaper than re-scanning every pass forever. - pub sent_payment_reconcile_swept_at: BTreeMap, + pub sent_payment_reconcile_swept_table: BTreeMap, /// Cached **contact** profiles keyed by the contact's identity id — /// established contacts, pending incoming-request senders, and (later) diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs index 0d1ab3150f..1dc5f651dd 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/mod.rs @@ -123,15 +123,15 @@ impl ManagedIdentity { &mut self.dashpay.payments } - /// Mutable access to the per-contact sent-payment sweep heights. + /// Mutable access to the per-contact sent-payment sweep table digests. /// /// In-memory only — never persisted; see the field docs on - /// [`DashPayState::sent_payment_reconcile_swept_at`] for why the guard is - /// a height rather than a flag. - pub fn dashpay_sent_payment_reconcile_swept_at_mut( + /// [`DashPayState::sent_payment_reconcile_swept_table`] for why the guard + /// is a digest of the scanned table rather than a flag or a height. + pub fn dashpay_sent_payment_reconcile_swept_table_mut( &mut self, - ) -> &mut std::collections::BTreeMap { - &mut self.dashpay.sent_payment_reconcile_swept_at + ) -> &mut std::collections::BTreeMap { + &mut self.dashpay.sent_payment_reconcile_swept_table } /// Mutable access to the cached contact profiles. From c9bb8d594f6146d63c1d1a6161397265ff9d36ed Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 7 Aug 2026 04:11:44 +0700 Subject: [PATCH 9/9] fix(platform-wallet): revert address consumption on rejected broadcast MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two review findings on b55002f: - A definitively rejected broadcast (BroadcastError::Rejected — the transaction provably never reached the network) now returns the consumed contact payment address to the pool AND persists the revert. Unlike the build-failure rollback, the used flip was already durable by broadcast time, so an in-memory revert alone would be undone at the next relaunch; left consumed, every definitive rejection widens the off-chain gap in the used range by one with no bound — the same unrecoverable-gap class as an unrolled-back build failure. An indeterminate failure (MaybeSent) keeps the consumption: the transaction may have propagated. The un-mark logic is shared between both paths (return_contact_payment_address_to_pool). - list_wallet_core_txids now returns Option, separating "backend does not index wallet-scoped tx history" (None — the default, and the FFI answer when the enumeration callbacks are unset, i.e. Android) from "supported, no rows yet" (Some(vec![])). The sweep skips outright on None instead of treating the backend as a perpetually incomplete empty table and re-deriving per-contact candidate windows on every recurring sync pass forever. Co-Authored-By: Claude Fable 5 --- .../rs-platform-wallet-ffi/src/persistence.rs | 12 +- .../src/changeset/traits.rs | 18 +- .../src/wallet/identity/network/payments.rs | 368 ++++++++++++++++-- .../src/wallet/persister.rs | 5 +- 4 files changed, 363 insertions(+), 40 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 2edf990540..1a19ea3b32 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -2793,11 +2793,15 @@ impl PlatformWalletPersistence for FFIPersister { fn list_wallet_core_txids( &self, wallet_id: WalletId, - ) -> Result, PersistenceError> { + ) -> Result>, PersistenceError> { use dashcore::hashes::Hash; + // An unset callback means this host never wired wallet-scoped + // transaction enumeration (the Android vtable leaves both slots + // `None`). Report the capability as absent — NOT an empty table — + // so sent-payment reconstruction skips instead of retrying forever. let Some(list_cb) = self.callbacks.on_list_wallet_core_txids_fn else { - return Ok(Vec::new()); + return Ok(None); }; let mut txids_ptr: *const u8 = std::ptr::null(); @@ -2863,7 +2867,7 @@ impl PlatformWalletPersistence for FFIPersister { }; if txids_ptr.is_null() || count == 0 { - return Ok(Vec::new()); + return Ok(Some(Vec::new())); } // The flags buffer is not optional once rows exist: without the // per-txid ownership verdict the reconstruction sweep cannot tell a @@ -2907,7 +2911,7 @@ impl PlatformWalletPersistence for FFIPersister { spends_wallet_input: flag & 0x01 != 0, }); } - Ok(out) + Ok(Some(out)) } } diff --git a/packages/rs-platform-wallet/src/changeset/traits.rs b/packages/rs-platform-wallet/src/changeset/traits.rs index c1dc0d79c9..21653019b0 100644 --- a/packages/rs-platform-wallet/src/changeset/traits.rs +++ b/packages/rs-platform-wallet/src/changeset/traits.rs @@ -369,9 +369,17 @@ pub trait PlatformWalletPersistence: Send + Sync { /// /// Used by DashPay sent-payment reconstruction to walk the /// wallet's locally persisted transaction history without relying - /// on the optional in-memory `transactions()` map. The default - /// implementation returns an empty set for backwards compatibility - /// with backends that don't index wallet-scoped tx history. + /// on the optional in-memory `transactions()` map. + /// + /// Returns `Ok(None)` when the backend does not index wallet-scoped + /// transaction history at all — the default, kept by backends that + /// never wire the capability (e.g. the Android vtable leaves the + /// enumeration callbacks unset). `None` is NOT an empty table: an + /// empty table (`Some(vec![])`) means "supported, nothing persisted + /// yet" and reconstruction keeps retrying until rows appear, while + /// `None` tells the caller to skip reconstruction entirely instead + /// of re-deriving candidate windows against a table that will never + /// materialize. /// /// `spends_wallet_input` must be `true` only when at least one of /// the transaction's inputs spends an output owned by one of this @@ -385,8 +393,8 @@ pub trait PlatformWalletPersistence: Send + Sync { fn list_wallet_core_txids( &self, _wallet_id: WalletId, - ) -> Result, PersistenceError> { - Ok(Vec::new()) + ) -> Result>, PersistenceError> { + Ok(None) } // TODO: `list_wallets` and `delete_wallet` are deferred contract diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs index 8001b8b543..f2f65ace31 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs @@ -295,9 +295,17 @@ impl DashPayView<'_, B> { return Ok(0); } - let listed = self.persister.list_wallet_core_txids().map_err(|e| { + let Some(listed) = self.persister.list_wallet_core_txids().map_err(|e| { PlatformWalletError::Persistence(format!("failed to enumerate wallet txids: {e}")) - })?; + })? + else { + // The backend does not index wallet-scoped transaction history + // (e.g. the Android vtable leaves the enumeration callbacks + // unset). Reconstruction has nothing it could ever read — skip, + // instead of treating the backend as a perpetually incomplete + // empty table and re-deriving candidate windows every sweep. + return Ok(0); + }; // The digest this pass will certify. Computed from exactly the rows // enumerated here, so the stamp below can never claim more than this // pass inspected: rows that land after this enumeration — a rescan @@ -841,6 +849,35 @@ fn record_received_payment_totals( recorded } +/// Return a consumed contact payment address to its pool: clear the used +/// mark and index, and recompute the used high-water. +/// +/// Sound ONLY while the transaction that consumed the address never reached +/// the network — a failed `build_signed`, or a broadcast the network +/// definitively rejected pre-send. The address was never exposed on-chain in +/// either case, so re-handing it later cannot break DIP-15 per-payment +/// rotation, and clearing the mark is what preserves the invariant +/// sent-payment reconstruction depends on: used indices chain within the gap +/// limit. Extra lookahead addresses the selection may have generated are +/// left in place — generated-but-available entries are harmless. +fn return_contact_payment_address_to_pool( + account: &mut key_wallet::managed_account::ManagedCoreFundsAccount, + payment_address: &dashcore::Address, +) { + use key_wallet::managed_account::address_pool::AddressState; + + for pool in account.managed_account_type_mut().address_pools_mut() { + let Some(&index) = pool.address_index.get(payment_address) else { + continue; + }; + pool.used_indices.remove(&index); + if let Some(address_info) = pool.addresses.get_mut(&index) { + address_info.state = AddressState::Available; + } + pool.highest_used = pool.used_indices.iter().max().copied(); + } +} + /// Order-independent digest of an enumerated wallet transaction table: /// SHA-256 over the sorted `(txid, spends_wallet_input)` rows. /// @@ -1226,20 +1263,7 @@ impl DashPayView<'_, B> { .dashpay_external_accounts .get_mut(&key) { - for pool in external_account - .managed_account_type_mut() - .address_pools_mut() - { - let Some(&index) = pool.address_index.get(&payment_address) else { - continue; - }; - pool.used_indices.remove(&index); - if let Some(address_info) = pool.addresses.get_mut(&index) { - address_info.state = - key_wallet::managed_account::address_pool::AddressState::Available; - } - pool.highest_used = pool.used_indices.iter().max().copied(); - } + return_contact_payment_address_to_pool(external_account, &payment_address); } return Err(PlatformWalletError::TransactionBuild(e.to_string())); } @@ -1267,7 +1291,7 @@ impl DashPayView<'_, B> { // --- 3. Broadcast the transaction, releasing the build's UTXO // reservation if the broadcast is definitively rejected pre-send. --- - let txid = crate::wallet::reservations::broadcast_releasing_on_rejection( + let txid = match crate::wallet::reservations::broadcast_releasing_on_rejection( self.broadcaster.as_ref(), &self.wallet_manager, &self.wallet_id, @@ -1275,7 +1299,77 @@ impl DashPayView<'_, B> { 0, &tx, ) - .await?; + .await + { + Ok(txid) => txid, + Err(e) => { + // A definitive rejection means the transaction never reached + // the network, so the payment address was never exposed + // on-chain — but unlike the build-failure rollback above, its + // used flip WAS persisted (durability precedes broadcast). + // Return the address to the pool and persist the revert: + // leaving it consumed lets every definitively rejected send + // widen the off-chain gap in the used range by one, with no + // bound, until a later successful payment lands beyond any + // recovery walk — the same failure class as an unrolled-back + // build failure, one step later. An indeterminate broadcast + // failure keeps the consumption: the transaction may still + // have propagated, so the address must never be re-handed. + if matches!(e, crate::broadcaster::BroadcastError::Rejected { .. }) { + let revert_changeset = { + let mut wm = self.wallet_manager.write().await; + wm.get_wallet_info_mut(&self.wallet_id).and_then(|info| { + info.core_wallet + .accounts + .dashpay_external_accounts + .get_mut(&DashpayAccountKey { + index: account_index, + user_identity_id: from_identity_id.to_buffer(), + friend_identity_id: to_contact_id.to_buffer(), + }) + .map(|external_account| { + return_contact_payment_address_to_pool( + external_account, + &payment_address, + ); + crate::changeset::PlatformWalletChangeSet { + account_address_pools: + crate::changeset::account_address_pool_entries( + key_wallet::account::AccountType::DashpayExternalAccount { + index: account_index, + user_identity_id: from_identity_id.to_buffer(), + friend_identity_id: to_contact_id.to_buffer(), + }, + external_account + .managed_account_type() + .address_pools(), + ), + ..Default::default() + } + }) + }) + }; + match revert_changeset { + // Persisted outside the write guard, same as the flip + // itself. A failed revert store is logged, not fatal: + // the address stays consumed and the one-address gap + // is absorbed by the pool's gap window. + Some(changeset) => { + if let Err(persist_err) = self.persister.store(changeset) { + tracing::warn!( + error = %persist_err, + "failed to persist payment-address revert after rejected broadcast" + ); + } + } + None => tracing::warn!( + "external account not found while reverting payment address after rejected broadcast" + ), + } + } + return Err(e.into()); + } + }; tracing::info!( from_identity = %from_identity_id, @@ -1413,6 +1507,9 @@ mod tests { /// `Some(n)` lets the next `n` `store` calls succeed and fails every /// later one until the budget is disarmed (`None` = always succeed). allow_stores_then_fail: Mutex>, + /// `true` makes the enumeration answer `Ok(None)` — the shape of a + /// backend that never wired wallet-scoped tx enumeration (Android). + enumeration_unsupported: Mutex, list_wallet_core_txids_calls: Mutex, get_core_tx_record_calls: Mutex, } @@ -1457,8 +1554,12 @@ mod tests { fn list_wallet_core_txids( &self, _wallet_id: WalletId, - ) -> Result, PersistenceError> { + ) -> Result>, PersistenceError> + { *self.list_wallet_core_txids_calls.lock().unwrap() += 1; + if *self.enumeration_unsupported.lock().unwrap() { + return Ok(None); + } let not_funded = self.not_wallet_funded.lock().unwrap(); let unavailable = self.listed_but_unavailable.lock().unwrap(); let listed: std::collections::BTreeSet = self @@ -1469,13 +1570,15 @@ mod tests { .copied() .chain(unavailable.iter().copied()) .collect(); - Ok(listed - .into_iter() - .map(|txid| crate::changeset::traits::ListedCoreTxid { - txid, - spends_wallet_input: !not_funded.contains(&txid), - }) - .collect()) + Ok(Some( + listed + .into_iter() + .map(|txid| crate::changeset::traits::ListedCoreTxid { + txid, + spends_wallet_input: !not_funded.contains(&txid), + }) + .collect(), + )) } } @@ -4323,6 +4426,80 @@ mod tests { ); } + /// A backend that does not support wallet-scoped tx enumeration + /// (`list_wallet_core_txids` → `Ok(None)`, the Android vtable shape) + /// must make the sweep skip outright — not treat the backend as a + /// perpetually incomplete empty table and re-derive candidate windows + /// every recurring sync pass. + #[tokio::test] + async fn reconcile_sent_payments_from_tx_history_skips_backends_without_enumeration() { + use dashcore::hashes::Hash; + use dashcore::BlockHash; + use key_wallet::managed_account::transaction_record::OutputRole; + use key_wallet::transaction_checking::{BlockInfo, TransactionContext}; + + let persister = Arc::new(RecordStorePersister::default()); + *persister.enumeration_unsupported.lock().unwrap() = true; + let (manager, wallet_id) = make_wallet_with(Arc::clone(&persister)).await; + let owner = Identifier::from([0xAA; 32]); + let contact = Identifier::from([0xBB; 32]); + + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet.identity(); + let p = WalletPersister::new(wallet_id, Arc::clone(&persister) as _); + { + let mut wm = iw.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("info"); + info.identity_manager + .add_identity(bare_identity([0xAA; 32]), 0, wallet_id, &p) + .expect("add owner"); + } + + // A record exists and even pays the contact — but the backend cannot + // enumerate, so reconstruction must not fabricate work (or entries). + let contact_address = install_external_account(&manager, wallet_id, owner, contact) + .await + .remove(0); + let record = tx_record_with_outputs( + TransactionContext::InBlock(BlockInfo::new(42, BlockHash::all_zeros(), 0)), + vec![(contact_address, 15_000, OutputRole::Sent)], + ); + persister + .records + .lock() + .unwrap() + .insert(record.txid, record); + + for pass in 1..=2 { + assert_eq!( + iw.dashpay() + .reconcile_sent_payments_from_tx_history() + .await + .expect("reconcile"), + 0, + "an enumeration-less backend must reconstruct nothing (pass {pass})" + ); + } + assert_eq!( + *persister.get_core_tx_record_calls.lock().unwrap(), + 0, + "no record may be fetched when enumeration is unsupported" + ); + { + let wm = iw.wallet_manager.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("info"); + assert!( + info.identity_manager + .managed_identity(&owner) + .expect("managed") + .dashpay() + .payments + .is_empty(), + "no payment entry may be recorded without an enumeration" + ); + } + } + /// The seedless drain path: `register_external_contact_account` with a /// **precomputed** ECDH shared secret (the Keychain signer computed it; the /// scalar never entered this crate) decrypts the contact's xpub and builds @@ -5388,6 +5565,105 @@ mod tests { ); } + /// A definitively rejected broadcast must return the consumed payment + /// address to the pool AND persist the revert — unlike a failed build, + /// the used flip was already persisted before the broadcast attempt, so + /// an in-memory revert alone would be undone by the next relaunch. + #[tokio::test] + async fn send_payment_rejected_broadcast_returns_the_address_to_the_pool() { + use crate::wallet::identity::network::contact_requests::SeedCryptoProvider; + use key_wallet::account::AccountType; + + let (manager, persister, wallet_id, owner_id, contact_id) = + register_sender_and_external_account().await; + let wallet_arc = manager.get_wallet(&wallet_id).await.expect("wallet"); + let iw = wallet_arc.identity(); + + // Fund the wallet so build + sign succeed and the send reaches the + // broadcast (and its preceding used-flip persist). + fund_bip44_account_0(&manager, wallet_id, 0xB7, 120_000).await; + + let seed = Mnemonic::from_phrase(TEST_MNEMONIC, Language::English) + .expect("valid mnemonic") + .to_seed(""); + let provider = SeedCryptoProvider::from_seed(seed, Network::Testnet); + let signer = SeedSigner::new(seed, Network::Testnet); + + // Clear the store log so the assertions below see only the send's + // own writes. + persister.stores.lock().unwrap().clear(); + + let iw_send = with_rejecting_broadcaster(iw); + let err = iw_send + .dashpay() + .send_payment(&owner_id, &contact_id, 50_000, None, &signer, &provider) + .await + .expect_err("the rejecting broadcaster must fail the send"); + assert!( + matches!(err, PlatformWalletError::TransactionBroadcast(_)), + "expected the definitive-rejection error, got: {err:?}" + ); + + // In-memory: the address is back in the pool. + { + let wm = iw.wallet_manager.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("info"); + let key = DashpayAccountKey { + index: 0, + user_identity_id: owner_id.to_buffer(), + friend_identity_id: contact_id.to_buffer(), + }; + let account = info + .core_wallet + .accounts + .dashpay_external_accounts + .get(&key) + .expect("external account present"); + let pools = account.managed_account_type().address_pools(); + let pool = pools.first().expect("external pool"); + assert!( + pool.used_indices.is_empty(), + "a rejected broadcast must not leave any address consumed, found {:?}", + pool.used_indices + ); + } + + // Persisted: the flip went out before the broadcast, so the revert + // must have been stored after it — the LAST persisted snapshot of + // the external account's pool shows no used address. + let stores = persister.stores.lock().unwrap(); + let last_external_pool_snapshot = stores + .iter() + .rev() + .flat_map(|(_, changeset)| changeset.account_address_pools.iter()) + .find(|entry| { + matches!( + entry.account_type, + AccountType::DashpayExternalAccount { .. } + ) + }) + .expect("the send must have persisted external-account pool snapshots"); + assert!( + last_external_pool_snapshot + .addresses + .iter() + .all(|address_info| !address_info.is_used()), + "the persisted revert must show the address returned to the pool" + ); + assert!( + stores + .iter() + .flat_map(|(_, changeset)| changeset.account_address_pools.iter()) + .filter(|entry| matches!( + entry.account_type, + AccountType::DashpayExternalAccount { .. } + )) + .count() + >= 2, + "both the pre-broadcast flip and the post-rejection revert must persist" + ); + } + /// The `send_payment` used-flag flip persist must run only AFTER the /// wallet-manager write guard is released (and before the broadcast). /// @@ -5589,6 +5865,39 @@ mod tests { } } + /// Broadcaster stub that definitively rejects every transaction, for the + /// rejected-broadcast cleanup paths. Build + sign run for real; only the + /// network says no. + struct RejectingBroadcaster; + + #[async_trait::async_trait] + impl crate::broadcaster::TransactionBroadcaster for RejectingBroadcaster { + async fn broadcast( + &self, + _transaction: &dashcore::Transaction, + ) -> Result { + Err(crate::broadcaster::BroadcastError::Rejected { + reason: "test rejection".to_string(), + }) + } + } + + /// [`with_accepting_broadcaster`], but the transport definitively + /// rejects. + fn with_rejecting_broadcaster( + real: &crate::wallet::identity::IdentityWallet, + ) -> crate::wallet::identity::IdentityWallet { + crate::wallet::identity::IdentityWallet { + sdk: Arc::clone(&real.sdk), + wallet_manager: Arc::clone(&real.wallet_manager), + wallet_id: real.wallet_id, + asset_locks: Arc::clone(&real.asset_locks), + persister: real.persister.clone(), + broadcaster: Arc::new(RejectingBroadcaster), + sdk_writer: Arc::clone(&real.sdk_writer), + } + } + /// Plant a single spendable UTXO of `value_duffs` on BIP-44 account 0's /// first pool address (a real derived address, so its derivation path is /// resolvable and [`SeedSigner`] can sign the funding input). @@ -5651,6 +5960,7 @@ mod tests { /// `send_payment_passes_external_lookup_once_account_built` up to the send). async fn register_sender_and_external_account() -> ( Arc>, + Arc, WalletId, Identifier, Identifier, @@ -5710,7 +6020,7 @@ mod tests { .await .expect("register external account"); - (manager, wallet_id, owner_id, contact_id) + (manager, persister, wallet_id, owner_id, contact_id) } /// A fully-successful `send_payment` whose exact change would be dust @@ -5724,7 +6034,7 @@ mod tests { async fn send_payment_reports_exact_fee_folding_dropped_dust_change() { use crate::wallet::identity::network::contact_requests::SeedCryptoProvider; - let (manager, wallet_id, owner_id, contact_id) = + let (manager, _persister, wallet_id, owner_id, contact_id) = register_sender_and_external_account().await; // One UTXO of V = A + 526. The size-based fee for 1 input + 1 output @@ -5780,7 +6090,7 @@ mod tests { async fn send_payment_reports_size_fee_when_change_is_emitted() { use crate::wallet::identity::network::contact_requests::SeedCryptoProvider; - let (manager, wallet_id, owner_id, contact_id) = + let (manager, _persister, wallet_id, owner_id, contact_id) = register_sender_and_external_account().await; // One UTXO of V = A + 1226. Change = V − A − size_fee = 1226 − 226 = diff --git a/packages/rs-platform-wallet/src/wallet/persister.rs b/packages/rs-platform-wallet/src/wallet/persister.rs index 4929957273..e2eafb8a87 100644 --- a/packages/rs-platform-wallet/src/wallet/persister.rs +++ b/packages/rs-platform-wallet/src/wallet/persister.rs @@ -62,10 +62,11 @@ impl WalletPersister { /// Enumerate the persisted Core transaction ids scoped to this /// wallet, tagged with the host's wallet-funded verdict. Used by /// DashPay sent-payment reconstruction to fetch the full records - /// via [`Self::get_core_tx_record`]. + /// via [`Self::get_core_tx_record`]. `None` means the backend does + /// not support wallet-scoped enumeration (never "empty table"). pub(crate) fn list_wallet_core_txids( &self, - ) -> Result, PersistenceError> { + ) -> Result>, PersistenceError> { self.inner.list_wallet_core_txids(self.wallet_id) } }