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 01/12] 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 be30d937dd0..dbd623feaaa 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 ea9978a30ee..1e3d5ca143a 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 3ffdf2fdd1c..6f1f4340996 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 bd797eac604..d6998cf29b7 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 d9b9578a9d9..3e1cb19bd68 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 6f62c2dd616..8cb633206f5 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 d37e88a0250..c826e4cb4f1 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 f3edc95db0a..7839a458ddd 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 02/12] 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 dbd623feaaa..9a3f449bfdf 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 d6998cf29b7..f67e130a983 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 5c5e11cd808..72343fab10e 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 7839a458ddd..f34e0d1f534 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 03/12] 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 f67e130a983..68c184750f0 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 04/12] 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 9a3f449bfdf..65469d4478d 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 05/12] 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 65469d4478d..9a3a592916c 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 5d86012215d..913ea54d51b 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 1e3d5ca143a..c1dc0d79c92 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 68c184750f0..d97378166a7 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 c826e4cb4f1..49299572733 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 f34e0d1f534..cee3dc932d6 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 06/12] 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 9a3a592916c..c338c24b383 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 d97378166a7..09fd84b9f6e 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 3e1cb19bd68..1b924e9a6d6 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 8cb633206f5..bac99db93ae 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 cee3dc932d6..31f5271697b 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 07/12] 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 09fd84b9f6e..3129c6390e2 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 1b924e9a6d6..35f469da8dd 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 bac99db93ae..0d1ab3150fc 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 08/12] 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 3129c6390e2..8001b8b5430 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 35f469da8dd..fb981b0ebd4 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 0d1ab3150fc..1dc5f651ddd 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 c36d0b85ed13e0bee6fa1453760871a0786d1a34 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 7 Aug 2026 04:05:47 +0700 Subject: [PATCH 09/12] feat(platform-wallet): persist DashPay payment history through the persister callback MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DashPay payment entries were the one piece of DashPay state that bypassed the persister callback loop: FFI hosts' store() returned Ok while persisting nothing for payments, silently defeating record_dashpay_payment's rollback invariant. Durability depended on ContactDetailView.onAppear running the getter-backed refresh — a live send's Sent entry + memo were lost permanently if the app was killed first, confirm-sweep status flips didn't stick, and reconstruction re-derived its entries every relaunch. Append an on_persist_dashpay_payments_fn slot at the end of PersistenceCallbacks (established vtable-growth pattern; pin tests now 25/41 slots). FFIPersister::store flattens BOTH payment carriers — the per-identity dashpay_payments snapshots inside changeset.identities (what live writes actually emit) and any merged dashpay_payments_overlay — deduped by (owner, txid), overlay wins, and fires after the identities callback so a new owner's row is staged in the same round first. The entry payload mirrors PaymentRestoreEntryFFI so the write and restore-buffer shapes agree by construction. Swift implements the callback over the existing payment upsert core, refactored into a stage-only helper; a group whose owner identity isn't resolvable mid-round parks for one post-commit replay instead of dropping (discarded on rollback — the Rust side rolled back too). The getter/refresh path stays as a reconciler. JNI sets the slot to None: Android derives contact attribution from transaction history on reads and doesn't consume PaymentEntry. Co-Authored-By: Claude Fable 5 --- docs/sdk/sdk-parity-manifest.json | 52 +++ .../src/dashpay_payment.rs | 125 ++++++- .../src/identity_persistence.rs | 6 +- .../rs-platform-wallet-ffi/src/persistence.rs | 250 +++++++++++++- .../rs-unified-sdk-jni/src/persistence.rs | 11 +- .../PlatformWalletPersistenceHandler.swift | 308 ++++++++++++++---- .../DashPayPersistenceTests.swift | 188 +++++++++++ 7 files changed, 849 insertions(+), 91 deletions(-) diff --git a/docs/sdk/sdk-parity-manifest.json b/docs/sdk/sdk-parity-manifest.json index 08ac2ee640e..093292856c0 100644 --- a/docs/sdk/sdk-parity-manifest.json +++ b/docs/sdk/sdk-parity-manifest.json @@ -1035,6 +1035,58 @@ } ] }, + { + "id": "persistence.dashpay_payment_history", + "title": "DashPay payment history persists event-driven through the persister callback", + "area": "persistence", + "shared_apis": [ + "managed_identity_get_dashpay_payments" + ], + "required_persistence_capabilities": [ + "atomic_changesets", + "wallet_restore" + ], + "hosts": { + "swift": { + "sdk": "supported", + "example_app": "not-applicable", + "restart": "tested", + "reason": "The on_persist_dashpay_payments_fn vtable slot lands PersistentDashpayPayment rows on every Rust store() round (live sends with memos, pending-to-confirmed sweep flips, reconstruction upserts), and the identity restore buffer's payments array feeds them back at load — Sent entries and memos survive relaunch without any UI surface appearing. The getter-backed refreshDashPayPayments path remains as a reconciler." + }, + "kotlin": { + "sdk": "not-applicable", + "example_app": "not-applicable", + "restart": "not_applicable", + "reason": "Android derives contact payment attribution from transaction history on reads and does not consume PaymentEntry rows (confirmed by the Android team during the sent-payment reconstruction review), so the JNI vtable deliberately leaves the slot None and there is nothing to persist or restore on this host." + } + }, + "verification": [ + { + "host": "swift", + "kind": "unit", + "file": "packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DashPayPersistenceTests.swift", + "id": "testChangesetRoundPersistsSentEntryAndRestoreBufferRoundTripsIt", + "command": "swift test --package-path packages/swift-sdk --filter DashPayPaymentPersistenceTests", + "covers_restart": true + }, + { + "host": "swift", + "kind": "unit", + "file": "packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DashPayPersistenceTests.swift", + "id": "testChangesetRoundStatusFlipRepersistsTheSameRow", + "command": "swift test --package-path packages/swift-sdk --filter DashPayPaymentPersistenceTests", + "covers_restart": false + }, + { + "host": "shared", + "kind": "unit", + "file": "packages/rs-platform-wallet-ffi/src/persistence.rs", + "id": "store_projects_dashpay_payments_from_identities_and_overlay", + "command": "cargo test -p platform-wallet-ffi --lib store_projects_dashpay_payments_from_identities_and_overlay", + "covers_restart": false + } + ] + }, { "id": "network.masternode_discovery", "title": "Canonical masternode endpoint discovery", diff --git a/packages/rs-platform-wallet-ffi/src/dashpay_payment.rs b/packages/rs-platform-wallet-ffi/src/dashpay_payment.rs index 0121af9fac2..5cf65fb2baf 100644 --- a/packages/rs-platform-wallet-ffi/src/dashpay_payment.rs +++ b/packages/rs-platform-wallet-ffi/src/dashpay_payment.rs @@ -1,4 +1,6 @@ -//! FFI getter for per-contact DashPay payment history. +//! FFI surface for per-contact DashPay payment history: the persister +//! callback's row type ([`DashpayPaymentPersistEntryFFI`]) and an +//! on-demand getter over a live handle. //! //! Swift's `ContactDetailView` renders a payment list per contact //! (`PaymentEntry` on the managed identity's `DashPayState.payments`, keyed by @@ -8,29 +10,46 @@ //! [`crate::platform_wallet_get_managed_identity`]) as a flat array of //! POD-plus-C-string rows. //! -//! ## Why a getter, not a persister callback +//! ## Persistence: the callback is authoritative, the getter reconciles //! -//! The `dashpay_payments` map is already part of the persisted -//! `ManagedIdentity` state (it round-trips through `IdentityEntry` and -//! the `dashpay_payments_overlay` changeset field), and the FFI already -//! hands the host a live `ManagedIdentity` handle from which DashPay -//! fields are read directly (e.g. -//! [`crate::established_contact_is_payment_channel_broken`]). A -//! getter therefore lands the smaller, lower-risk diff: no new -//! persister callback, no new SwiftData rehydration path. It mirrors the -//! handle-based array-return pattern already used by -//! [`ContactRequestHandleArray`](crate::dashpay::ContactRequestHandleArray) -//! and [`IdentifierArray`](crate::IdentifierArray). +//! Payment history persists event-driven through +//! `on_persist_dashpay_payments_fn` on the persister vtable, exactly +//! like contact requests and profiles: every `store()` round whose +//! changeset carries payment rows (an `IdentityEntry.dashpay_payments` +//! snapshot from `record_dashpay_payment`, or a merged +//! `dashpay_payments_overlay`) projects them to the host. This closes +//! the write half of the durability loop whose read half — the +//! `payments` array on `IdentityRestoreEntryFFI` — already rehydrates +//! the map at load. (An earlier revision shipped only the +//! [`managed_identity_get_dashpay_payments`] getter, on the rationale +//! that the map "already persists through the changeset" — which was +//! true of the desktop SQLite persister but never of FFI hosts, whose +//! vtable had no payments slot. A host-side `store()` returned Ok while +//! dropping every Sent entry + memo unless the app happened to call the +//! getter-backed refresh path first.) +//! +//! The getter remains as (a) the on-demand read Swift's +//! `refreshDashPayPayments` uses to reconcile persisted rows against +//! live state — belt-and-suspenders over the callback — and (b) the +//! per-contact history read for UI surfaces that want current in-memory +//! state without a persistence round-trip. //! //! ## Ownership //! //! Each [`DashpayPaymentFFI`] owns its `txid` and (optional) `memo` //! C-strings. [`dashpay_payment_array_free`] releases every string //! across the array and the array backing buffer itself. +//! [`DashpayPaymentPersistEntryFFI`] rows are Rust-owned for the +//! duration of the persist callback only (the caller keeps the backing +//! `CString`s alive across the call and drops them after — no paired +//! free function, matching the other persist-direction callbacks). +use std::collections::BTreeMap; +use std::ffi::CString; use std::os::raw::c_char; -use platform_wallet::wallet::identity::{PaymentDirection, PaymentStatus}; +use dpp::prelude::Identifier; +use platform_wallet::wallet::identity::{PaymentDirection, PaymentEntry, PaymentStatus}; use crate::error::*; use crate::handle::*; @@ -78,6 +97,84 @@ impl From for DashpayPaymentStatusFFI { } } +/// One DashPay payment-history row forwarded to the host by the +/// `on_persist_dashpay_payments_fn` persister callback. +/// +/// Field set mirrors the load-side +/// [`PaymentRestoreEntryFFI`](crate::wallet_restore_types::PaymentRestoreEntryFFI) +/// — same raw `u8` direction/status discriminants, same +/// txid/memo C-string shape — plus the leading `owner_identity_id`, +/// because the persist callback is wallet-scoped while the restore +/// rows already ride inside a per-identity buffer. Keeping the write +/// and restore shapes field-for-field means a host handler and its +/// restore assembler agree by construction. +/// +/// All pointers are Rust-owned and valid only for the callback window +/// — the host must copy before returning. Persist direction needs no +/// paired free function (Rust drops the backing `CString`s after the +/// call), matching the other `on_persist_*` callbacks. +#[repr(C)] +pub struct DashpayPaymentPersistEntryFFI { + /// The identity that owns this payment-history row (the + /// `ManagedIdentity` whose `dashpay_payments` map carries it). + pub owner_identity_id: [u8; 32], + /// The other identity in this payment. Whether they are the sender + /// or the receiver is encoded in `direction_raw`. + pub counterparty_id: [u8; 32], + /// Amount in duffs. Always positive; `direction_raw` carries the sign. + pub amount_duffs: u64, + /// `PaymentDirection` discriminant: 0=Sent, 1=Received. + pub direction_raw: u8, + /// `PaymentStatus` discriminant: 0=Pending, 1=Confirmed, 2=Failed. + pub status_raw: u8, + /// NUL-terminated transaction id (hex) — the `dashpay_payments` + /// map key. Always non-null (rows whose txid cannot form a + /// C-string are dropped at build time). + pub txid: *const c_char, + /// NUL-terminated sender memo, or null when the source `Option` + /// was `None`. + pub memo: *const c_char, +} + +/// Flatten per-identity payment maps into persist-callback rows. +/// +/// Returns the row array plus the `CString` storage backing every +/// `txid` / `memo` pointer — the caller must keep the storage alive +/// until the callback returns. Rows whose txid contains an interior +/// NUL are dropped (unreachable for hex txids; defensive rather than +/// panicking); a memo with an interior NUL degrades to null, matching +/// [`cstring_or_null`]'s contract on the getter side. +pub(crate) fn build_payment_persist_entries( + payments: &BTreeMap<(Identifier, &str), &PaymentEntry>, +) -> (Vec, Vec) { + let mut storage: Vec = Vec::new(); + let mut rows: Vec = Vec::with_capacity(payments.len()); + for ((owner_id, txid), entry) in payments { + let Ok(txid_c) = CString::new(*txid) else { + continue; + }; + storage.push(txid_c); + let txid_ptr = storage.last().expect("pushed txid CString above").as_ptr(); + let memo_ptr = match entry.memo.as_deref().map(CString::new) { + Some(Ok(memo_c)) => { + storage.push(memo_c); + storage.last().expect("pushed memo CString above").as_ptr() + } + _ => std::ptr::null(), + }; + rows.push(DashpayPaymentPersistEntryFFI { + owner_identity_id: owner_id.to_buffer(), + counterparty_id: entry.counterparty_id.to_buffer(), + amount_duffs: entry.amount_duffs, + direction_raw: DashpayPaymentDirectionFFI::from(entry.direction) as u8, + status_raw: DashpayPaymentStatusFFI::from(entry.status) as u8, + txid: txid_ptr, + memo: memo_ptr, + }); + } + (rows, storage) +} + /// Flat C mirror of one [`PaymentEntry`](platform_wallet::wallet::identity::PaymentEntry) /// row on a [`ManagedIdentity`](platform_wallet::ManagedIdentity). /// diff --git a/packages/rs-platform-wallet-ffi/src/identity_persistence.rs b/packages/rs-platform-wallet-ffi/src/identity_persistence.rs index c1fc96100cf..2d0c526e034 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_persistence.rs @@ -35,8 +35,10 @@ use platform_wallet::{DashPayProfile, IdentityStatus}; /// [`IdentityKeyEntryFFI`] alongside their derivation breadcrumb via /// a separate callback. Fields that don't map onto the Swift schema /// (block times, contested DPNS names, DashPay payments) are skipped; -/// DashPay payment overlays already ride on the dedicated -/// `dashpay_payments_overlay` surface on the parent changeset. +/// DashPay payment rows travel on the dedicated +/// `on_persist_dashpay_payments_fn` callback (which flattens the +/// entry's `dashpay_payments` map together with any +/// `dashpay_payments_overlay` on the parent changeset). /// /// User-visible label is no longer carried — `ManagedIdentity` doesn't /// have one, and Swift owns the `PersistentIdentity.alias` column diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 2edf9905407..4e993a8f405 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -44,6 +44,7 @@ use crate::contact_persistence::{ }; use crate::core_address_types::{AddressPoolTypeTagFFI, CoreAddressEntryFFI, KeyTypeTagFFI}; use crate::core_wallet_types::{free_wallet_changeset_ffi, WalletChangeSetFFI}; +use crate::dashpay_payment::{build_payment_persist_entries, DashpayPaymentPersistEntryFFI}; use crate::identity_persistence::{ free_identity_entry_ffi, free_identity_key_entry_ffi, IdentityEntryFFI, IdentityKeyEntryFFI, IdentityKeyRemovalFFI, @@ -737,6 +738,33 @@ pub struct PersistenceCallbacks { count: usize, ), >, + /// Forwards DashPay payment-history rows (the + /// `IdentityEntry.dashpay_payments` snapshots and any merged + /// `dashpay_payments_overlay` on the changeset, flattened + deduped + /// per `(owner, txid)`) to the host. Appended at the END so the + /// struct layout stays stable — a host built against the previous + /// vtable keeps working, it simply never sets this slot. + /// + /// Rows are upserts only: the Rust-side map is append-only history + /// keyed by txid, so status flips (Pending → Confirmed / Failed) + /// re-emit the same `(owner, txid)` row and there is never a + /// tombstone array. Pointers inside each entry are Rust-owned for + /// the callback window; no paired free function (Rust drops the + /// backing strings after the call). + /// + /// Returns 0 on success. A non-zero return flips the round's + /// `success` flag to `false` so [`Self::on_changeset_end_fn`] + /// receives the rollback signal — load-bearing here, because a + /// dropped Sent entry + memo has no on-chain recovery (see + /// `record_dashpay_payment`'s rollback contract). + pub on_persist_dashpay_payments_fn: Option< + unsafe extern "C" fn( + context: *mut c_void, + wallet_id: *const u8, + entries: *const DashpayPaymentPersistEntryFFI, + count: usize, + ) -> i32, + >, } // SAFETY: The context pointer is managed by the FFI caller who must ensure @@ -770,6 +798,7 @@ impl Default for PersistenceCallbacks { on_get_core_tx_record_free_fn: None, on_list_wallet_core_txids_fn: None, on_list_wallet_core_txids_free_fn: None, + on_persist_dashpay_payments_fn: None, #[cfg(feature = "shielded")] on_persist_shielded_notes_fn: None, #[cfg(feature = "shielded")] @@ -1307,6 +1336,60 @@ impl PlatformWalletPersistence for FFIPersister { } } + // Send DashPay payment-history rows. Payments reach a store round + // on TWO carriers: the full-map `IdentityEntry.dashpay_payments` + // snapshot inside `changeset.identities` (what + // `record_dashpay_payment` / every scalar identity mutation + // emits) and the merged `dashpay_payments_overlay` (what + // `Merge`-combined rounds carry). Project both, deduped by + // `(owner, txid)` with the overlay winning — it is the + // later-merged delta. Fires AFTER the identities callback so a + // brand-new owner's `PersistentIdentity` row is already staged + // in the same round when the host resolves the payment's owner + // link. + if let Some(cb) = self.callbacks.on_persist_dashpay_payments_fn { + let mut merged: std::collections::BTreeMap< + (dpp::prelude::Identifier, &str), + &platform_wallet::wallet::identity::PaymentEntry, + > = std::collections::BTreeMap::new(); + if let Some(ref id_cs) = changeset.identities { + for (identity_id, entry) in &id_cs.identities { + for (txid, payment) in &entry.dashpay_payments { + merged.insert((*identity_id, txid.as_str()), payment); + } + } + } + if let Some(ref overlay) = changeset.dashpay_payments_overlay { + for (identity_id, payments) in overlay { + for (txid, payment) in payments { + merged.insert((*identity_id, txid.as_str()), payment); + } + } + } + if !merged.is_empty() { + let (entries, _string_storage) = build_payment_persist_entries(&merged); + if !entries.is_empty() { + let result = unsafe { + cb( + self.callbacks.context, + wallet_id.as_ptr(), + entries.as_ptr(), + entries.len(), + ) + }; + drop(entries); + drop(_string_storage); + if result != 0 { + eprintln!( + "DashPay payment persistence callback returned error code {}", + result + ); + round_success = false; + } + } + } + } + // Send identity-keys changeset — per-key upserts + // `(identity_id, key_id)` removals. Maps onto Swift's // `PersistentPublicKey` rows. @@ -5967,20 +6050,20 @@ mod tests { // 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). + // append (invitations, then the `release_fn` context destructor, the + // txid enumeration pair, now the DashPay payment persist slot). #[cfg(not(feature = "shielded"))] assert_eq!( std::mem::size_of::(), - 24 * std::mem::size_of::() + 25 * std::mem::size_of::() ); #[cfg(feature = "shielded")] assert_eq!( std::mem::size_of::(), - 40 * std::mem::size_of::() + 41 * std::mem::size_of::() ); assert_eq!( - std::mem::offset_of!(PersistenceCallbacks, on_list_wallet_core_txids_free_fn) + std::mem::offset_of!(PersistenceCallbacks, on_persist_dashpay_payments_fn) + std::mem::size_of::(), std::mem::size_of::() ); @@ -6030,6 +6113,163 @@ mod tests { ); } + /// A store round whose changeset carries payment rows on either + /// carrier — the full-map `IdentityEntry.dashpay_payments` snapshot + /// (what `record_dashpay_payment` emits inside + /// `changeset.identities`) or a merged `dashpay_payments_overlay` + /// — must flatten BOTH through `on_persist_dashpay_payments_fn`, + /// deduped by `(owner, txid)` with the overlay winning. This is the + /// write half of the relaunch-durability loop: without it a live + /// send's Sent entry + memo exist only in memory and `store()` + /// returns Ok having persisted nothing for payments, silently + /// defeating `record_dashpay_payment`'s rollback invariant. + #[test] + fn store_projects_dashpay_payments_from_identities_and_overlay() { + use platform_wallet::changeset::IdentityChangeSet; + use platform_wallet::wallet::identity::{PaymentEntry, PaymentStatus}; + use std::sync::atomic::{AtomicUsize, Ordering}; + + type CollectedRow = ([u8; 32], String, u64, u8, u8, Option); + + #[derive(Default)] + struct PaymentSink { + rows: std::sync::Mutex>, + calls: AtomicUsize, + } + unsafe extern "C" fn collect_payments( + ctx: *mut c_void, + _wallet_id: *const u8, + entries: *const DashpayPaymentPersistEntryFFI, + count: usize, + ) -> i32 { + let sink = &*(ctx as *const PaymentSink); + sink.calls.fetch_add(1, Ordering::SeqCst); + let slice = std::slice::from_raw_parts(entries, count); + let mut rows = sink.rows.lock().expect("sink lock"); + for e in slice { + let txid = std::ffi::CStr::from_ptr(e.txid) + .to_str() + .expect("txid utf8") + .to_string(); + let memo = if e.memo.is_null() { + None + } else { + Some( + std::ffi::CStr::from_ptr(e.memo) + .to_str() + .expect("memo utf8") + .to_string(), + ) + }; + rows.push(( + e.owner_identity_id, + txid, + e.amount_duffs, + e.direction_raw, + e.status_raw, + memo, + )); + } + 0 + } + + // Live-send shape: a managed identity carrying one Pending Sent + // payment with a memo, snapshotted the same way + // `record_dashpay_payment` does. + let identity = dpp::identity::Identity::V0(dpp::identity::v0::IdentityV0::default()); + let mut managed = platform_wallet::ManagedIdentity::new(identity, 0); + let sent_txid = "aa".repeat(32); + managed.dashpay_payments_mut().insert( + sent_txid.clone(), + PaymentEntry::new_sent( + dpp::prelude::Identifier::from([7u8; 32]), + 12_000, + Some("lunch".into()), + ), + ); + let owner_id = managed.id(); + let mut id_cs = IdentityChangeSet::default(); + id_cs.identities.insert( + owner_id, + platform_wallet::changeset::IdentityEntry::from_managed(&managed), + ); + + // Overlay carriers: (a) the SAME (owner, txid) flipped to + // Confirmed — must win over the snapshot's Pending row — and + // (b) a second owner's Received row that exists only on the + // overlay. + let mut confirmed = PaymentEntry::new_sent( + dpp::prelude::Identifier::from([7u8; 32]), + 12_000, + Some("lunch".into()), + ); + confirmed.status = PaymentStatus::Confirmed; + let other_owner = dpp::prelude::Identifier::from([9u8; 32]); + let received_txid = "bb".repeat(32); + let mut overlay: std::collections::BTreeMap< + dpp::prelude::Identifier, + std::collections::BTreeMap, + > = Default::default(); + overlay + .entry(owner_id) + .or_default() + .insert(sent_txid.clone(), confirmed); + overlay.entry(other_owner).or_default().insert( + received_txid.clone(), + PaymentEntry::new_received(dpp::prelude::Identifier::from([8u8; 32]), 7_500, None), + ); + + let changeset = PlatformWalletChangeSet { + identities: Some(id_cs), + dashpay_payments_overlay: Some(overlay), + ..Default::default() + }; + + let sink = std::sync::Arc::new(PaymentSink::default()); + let callbacks = PersistenceCallbacks { + context: std::sync::Arc::as_ptr(&sink) as *mut c_void, + on_persist_dashpay_payments_fn: Some(collect_payments), + ..PersistenceCallbacks::default() + }; + let persister = FFIPersister::new(callbacks); + persister + .store([1u8; 32], changeset) + .expect("payment round must succeed"); + + assert_eq!( + sink.calls.load(Ordering::SeqCst), + 1, + "both carriers must flatten into a single callback fire" + ); + let mut rows = sink.rows.lock().expect("sink lock").clone(); + rows.sort(); + assert_eq!(rows.len(), 2, "one deduped row per (owner, txid)"); + // BTreeMap order: default-id owner ([0; 32]) before [9; 32]. + let (owner, txid, amount, direction, status, memo) = &rows[0]; + assert_eq!(*owner, owner_id.to_buffer()); + assert_eq!(*txid, sent_txid); + assert_eq!(*amount, 12_000); + assert_eq!(*direction, 0, "Sent discriminant"); + assert_eq!( + *status, 1, + "the overlay's Confirmed flip must win over the snapshot's Pending row" + ); + assert_eq!(memo.as_deref(), Some("lunch")); + let (owner, txid, amount, direction, status, memo) = &rows[1]; + assert_eq!(*owner, [9u8; 32]); + assert_eq!(*txid, received_txid); + assert_eq!(*amount, 7_500); + assert_eq!(*direction, 1, "Received discriminant"); + assert_eq!(*status, 1, "Received entries record as Confirmed"); + assert!(memo.is_none()); + + // A payments-free round must not fire the callback at all. + persister + .store([1u8; 32], PlatformWalletChangeSet::default()) + .expect("empty round must succeed"); + assert_eq!(sink.calls.load(Ordering::SeqCst), 1); + } + #[cfg(feature = "shielded")] #[test] fn shielded_viewing_key_capability_requires_complete_callback_triplet() { diff --git a/packages/rs-unified-sdk-jni/src/persistence.rs b/packages/rs-unified-sdk-jni/src/persistence.rs index 6c3ec086df4..638217d6db4 100644 --- a/packages/rs-unified-sdk-jni/src/persistence.rs +++ b/packages/rs-unified-sdk-jni/src/persistence.rs @@ -116,8 +116,9 @@ unsafe impl Sync for KotlinPersistenceCtx {} // ShieldedActivityData org/dashfoundation/dashsdk/ffi/ShieldedActivityData // CoreTxRecordData org/dashfoundation/dashsdk/ffi/CoreTxRecordData -/// Assemble the full 32-slot vtable. `context` is the boxed -/// [`KotlinPersistenceCtx`] pointer. +/// Assemble the full persistence vtable (every slot named, wired or an +/// explicit `None`). `context` is the boxed [`KotlinPersistenceCtx`] +/// pointer. pub(crate) fn build_vtable(context: *mut c_void) -> PersistenceCallbacks { PersistenceCallbacks { context, @@ -180,6 +181,12 @@ pub(crate) fn build_vtable(context: *mut c_void) -> PersistenceCallbacks { // pre-restore contact payments) rather than misreporting. on_list_wallet_core_txids_fn: None, on_list_wallet_core_txids_free_fn: None, + // Android derives contact attribution from transaction history on + // reads and doesn't consume `PaymentEntry` rows, so there is + // nothing to land these in. `None` keeps the Rust-side payment + // recording in-memory-only on Android — same behaviour as before + // the slot existed. + on_persist_dashpay_payments_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 31f5271697b..5fb539815f3 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -121,6 +121,21 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { /// Confined to `serialQueue` like all other mutable handler state. private var deferredBackfills: [(walletId: Data, items: [KeychainManager.IdentityPrivateKeyMetadata])] = [] + /// DashPay payment rows the persister callback could not stage + /// because the owner `PersistentIdentity` row wasn't resolvable + /// mid-round. Normally the owner is visible — the identities + /// callback fires before the payments callback in the same Rust + /// `store()` round, and `FetchDescriptor` sees the round's pending + /// inserts — so this only holds rows whose owner is in neither the + /// current round nor the store. Replayed once by `endChangeset` + /// after a successful commit (one more chance for a late owner + /// row), then dropped with a log — the `refreshDashPayPayments` + /// reconciler re-upserts anything dropped here. Discarded on + /// rollback: a failed round also rolls the entries back out of the + /// Rust in-memory map, so persisting them would fabricate history. + /// Confined to `serialQueue` like all other mutable handler state. + private var deferredPaymentUpserts: [(ownerIdentityId: Data, payments: [DashPayPayment])] = [] + public init(modelContainer: ModelContainer, network: Network? = nil) { self.modelContainer = modelContainer self.network = network @@ -1307,6 +1322,7 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { 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 + cb.on_persist_dashpay_payments_fn = persistDashpayPaymentsCallback return cb } @@ -1360,6 +1376,11 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { if success { do { try backgroundContext.save() + // With the round durably committed, give payment rows + // parked on a missing owner identity one replay — a + // successful commit is the only point a late owner row + // can have become visible. + replayDeferredPaymentUpserts() return true } catch { // The context still has the pending changes on @@ -1370,10 +1391,15 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { // round did NOT commit, so report failure upward. print("⚠️ endChangeset: save failed: \(error.localizedDescription)") backgroundContext.rollback() + // The failed round's Rust-side rollback also removed + // these entries from the in-memory map — persisting + // them would fabricate history. + deferredPaymentUpserts.removeAll() return false } } else { backgroundContext.rollback() + deferredPaymentUpserts.removeAll() return false } } @@ -2399,19 +2425,18 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { // MARK: - DashPay payment-history persistence - /// Upsert DashPay payment-history rows for one owner identity. - /// - /// NOT a persister-callback path — the Rust persister doesn't - /// project payment history. Called by + /// Upsert DashPay payment-history rows for one owner identity — + /// the reconciler half of the payment durability loop. Called by /// `PlatformWalletManager.refreshDashPayPayments` after reading /// the `managed_identity_get_dashpay_payments` getter, so the UI - /// can `@Query` `PersistentDashpayPayment` rows reactively. + /// can `@Query` `PersistentDashpayPayment` rows reactively. The + /// authoritative event-driven half is the + /// `on_persist_dashpay_payments_fn` persister callback + /// (`persistDashpayPayments(walletId:entriesByOwner:)` below); + /// this refresh path reconciles anything the callback era predates + /// or a parked-row drop lost. /// - /// Upsert-only: the Rust `dashpay_payments` map is append-only - /// history (keyed by txid), so a refresh never has to delete - /// rows; cascade from the owner identity handles wallet wipes. - /// Rows are keyed `(networkRaw, ownerIdentityId, txid)`. Skips - /// silently when the owner identity row doesn't exist yet — + /// Skips silently when the owner identity row doesn't exist yet — /// the next refresh after the identity flush replays it. /// /// Saves immediately when no changeset round is open — same @@ -2422,64 +2447,10 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { payments: [DashPayPayment] ) { onQueue { - let ownerId = ownerIdentityId - let ownerDescriptor = FetchDescriptor( - predicate: #Predicate { $0.identityId == ownerId } - ) - guard let owner = try? backgroundContext.fetch(ownerDescriptor).first else { - return - } - let networkRaw = owner.networkRaw - - for payment in payments { - guard !payment.txid.isEmpty else { continue } - let txid = payment.txid - let descriptor = FetchDescriptor( - predicate: #Predicate { - $0.networkRaw == networkRaw - && $0.ownerIdentityId == ownerId - && $0.txid == txid - } - ) - if let existing = try? backgroundContext.fetch(descriptor).first { - // Refresh in place only when a field actually changed. - // The FFI snapshot is authoritative, and `status` is the - // field that moves (Pending → Confirmed / Failed). A - // no-op rewrite would still dirty the row and re-fire - // every `@Query` observer on each refresh pass — and the - // recurring DashPay-sync falling edge calls this even on - // a quiescent channel, so skipping unchanged rows keeps - // an open payment list from re-rendering every sync. - let changed = existing.counterpartyIdentityId != payment.counterpartyId - || existing.amountDuffs != payment.amountDuffs - || existing.directionRaw != payment.direction.rawValue - || existing.statusRaw != payment.status.rawValue - || existing.memo != payment.memo - || existing.owner !== owner - if changed { - existing.counterpartyIdentityId = payment.counterpartyId - existing.amountDuffs = payment.amountDuffs - existing.directionRaw = payment.direction.rawValue - existing.statusRaw = payment.status.rawValue - existing.memo = payment.memo - if existing.owner !== owner { - existing.owner = owner - } - existing.lastUpdated = Date() - } - } else { - let row = PersistentDashpayPayment( - owner: owner, - counterpartyIdentityId: payment.counterpartyId, - amountDuffs: payment.amountDuffs, - direction: payment.direction, - status: payment.status, - txid: payment.txid, - memo: payment.memo - ) - backgroundContext.insert(row) - } - } + guard stageDashpayPaymentUpserts( + ownerIdentityId: ownerIdentityId, + payments: payments + ) else { return } // Same guard as the other app-facing writers // (`setWalletName`, …): a refresh landing while a Rust // persister round is open must ride that round's @@ -2500,6 +2471,152 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { } } + /// Apply one `on_persist_dashpay_payments_fn` persister-callback + /// batch — payment rows flattened out of a Rust `store()` round, + /// grouped by owner identity. This is the event-driven write half + /// of the payment durability loop: it fires on every round whose + /// changeset carries payment rows (a live `send_payment`, a + /// pending→confirmed sweep flip, a reconstruction upsert), so + /// Sent entries + memos are durable without any UI surface ever + /// appearing. + /// + /// Runs mid-round: rows are staged on `backgroundContext` and ride + /// the round's `endChangeset` commit/rollback. A group whose owner + /// `PersistentIdentity` row isn't resolvable — not even as a + /// pending insert staged by this round's identities callback, + /// which fires first — is parked on `deferredPaymentUpserts` for + /// one post-commit replay rather than dropped (see that field's + /// doc). + func persistDashpayPayments( + walletId: Data, + entriesByOwner: [Data: [DashPayPayment]] + ) { + onQueue { + _ = walletId + for (ownerId, payments) in entriesByOwner { + if !stageDashpayPaymentUpserts(ownerIdentityId: ownerId, payments: payments) { + deferredPaymentUpserts.append((ownerId, payments)) + } + } + // No save here even outside a round: the Rust store() round + // that invoked this callback brackets it with begin/end, so + // `inChangeset` is set in practice; if a host ever fires it + // without a bracket, autosave/next round flushes the stage. + } + } + + /// Replay payment groups parked on a missing owner identity. Must + /// run on `serialQueue`, immediately after a successful round + /// commit (the only point a late owner row becomes visible). One + /// attempt: still-missing owners are dropped with a log — the + /// `refreshDashPayPayments` reconciler re-upserts them later — + /// so the parked list can never grow across rounds. + private func replayDeferredPaymentUpserts() { + guard !deferredPaymentUpserts.isEmpty else { return } + let parked = deferredPaymentUpserts + deferredPaymentUpserts.removeAll() + var stagedAny = false + for entry in parked { + if stageDashpayPaymentUpserts( + ownerIdentityId: entry.ownerIdentityId, + payments: entry.payments + ) { + stagedAny = true + } else { + print( + "⚠️ persistDashpayPayments: owner identity " + + "\(entry.ownerIdentityId.toHexString()) never appeared; dropping " + + "\(entry.payments.count) parked payment row(s) — the refresh " + + "reconciler will restore them on the next read" + ) + } + } + if stagedAny { + do { + try backgroundContext.save() + } catch { + print("⚠️ replayDeferredPaymentUpserts: SwiftData save failed — payment history may be incomplete: \(error)") + } + } + } + + /// Stage upserts for one owner's payment rows on + /// `backgroundContext` — shared core of the persister callback, + /// the post-commit replay, and the refresh reconciler. No + /// `save()`; each caller owns its own commit point. Returns + /// `false` (nothing staged) when the owner `PersistentIdentity` + /// row doesn't exist — not even as a pending insert in the open + /// round. Must run on `serialQueue`. + /// + /// Upsert-only: the Rust `dashpay_payments` map is append-only + /// history (keyed by txid), so this never has to delete rows; + /// cascade from the owner identity handles wallet wipes. Rows are + /// keyed `(networkRaw, ownerIdentityId, txid)`. + private func stageDashpayPaymentUpserts( + ownerIdentityId: Data, + payments: [DashPayPayment] + ) -> Bool { + let ownerId = ownerIdentityId + let ownerDescriptor = FetchDescriptor( + predicate: #Predicate { $0.identityId == ownerId } + ) + guard let owner = try? backgroundContext.fetch(ownerDescriptor).first else { + return false + } + let networkRaw = owner.networkRaw + + for payment in payments { + guard !payment.txid.isEmpty else { continue } + let txid = payment.txid + let descriptor = FetchDescriptor( + predicate: #Predicate { + $0.networkRaw == networkRaw + && $0.ownerIdentityId == ownerId + && $0.txid == txid + } + ) + if let existing = try? backgroundContext.fetch(descriptor).first { + // Refresh in place only when a field actually changed. + // The FFI snapshot is authoritative, and `status` is the + // field that moves (Pending → Confirmed / Failed). A + // no-op rewrite would still dirty the row and re-fire + // every `@Query` observer on each refresh pass — and the + // recurring DashPay-sync falling edge calls this even on + // a quiescent channel, so skipping unchanged rows keeps + // an open payment list from re-rendering every sync. + let changed = existing.counterpartyIdentityId != payment.counterpartyId + || existing.amountDuffs != payment.amountDuffs + || existing.directionRaw != payment.direction.rawValue + || existing.statusRaw != payment.status.rawValue + || existing.memo != payment.memo + || existing.owner !== owner + if changed { + existing.counterpartyIdentityId = payment.counterpartyId + existing.amountDuffs = payment.amountDuffs + existing.directionRaw = payment.direction.rawValue + existing.statusRaw = payment.status.rawValue + existing.memo = payment.memo + if existing.owner !== owner { + existing.owner = owner + } + existing.lastUpdated = Date() + } + } else { + let row = PersistentDashpayPayment( + owner: owner, + counterpartyIdentityId: payment.counterpartyId, + amountDuffs: payment.amountDuffs, + direction: payment.direction, + status: payment.status, + txid: payment.txid, + memo: payment.memo + ) + backgroundContext.insert(row) + } + } + return true + } + // MARK: - Identity key derivation-path helpers /// Resolve the wallet's network and format the DIP-9 identity-auth path @@ -7667,3 +7784,58 @@ private func listWalletCoreTxidsFreeCallback( } _ = context } + +/// C shim for `on_persist_dashpay_payments_fn`. Copies every +/// `DashpayPaymentPersistEntryFFI` row into a Swift-owned +/// `DashPayPayment` (grouped by owner identity) before invoking the +/// handler, so the Rust side can drop its backing strings the moment +/// we return. Rows without a txid pointer are skipped defensively — +/// the Rust builder documents `txid` as always non-null. +/// +/// Always returns 0: a missing owner identity parks the group for a +/// post-commit replay rather than failing (see +/// `deferredPaymentUpserts`), and a commit failure is reported through +/// the round's `on_changeset_end_fn` return instead. +private func persistDashpayPaymentsCallback( + context: UnsafeMutableRawPointer?, + walletIdPtr: UnsafePointer?, + entriesPtr: UnsafePointer?, + count: UInt +) -> Int32 { + guard let context = context, + let walletIdPtr = walletIdPtr else { + return 0 + } + let handler = Unmanaged + .fromOpaque(context) + .takeUnretainedValue() + let walletId = Data(bytes: walletIdPtr, count: 32) + + var entriesByOwner: [Data: [DashPayPayment]] = [:] + if count > 0, let entriesPtr = entriesPtr { + for i in 0..( + predicate: #Predicate { $0.identityId == target } + ) + let owner = try XCTUnwrap(try context.fetch(ownerDescriptor).first) + owner.wallet = wallet + try context.save() + + // The persister round: begin → payments batch → end. + handler.beginChangeset(walletId: walletId) + handler.persistDashpayPayments( + walletId: walletId, + entriesByOwner: [ownerId: [makePayment(status: .pending, memo: "rent + utilities")]] + ) + // Mid-round: staged, not committed. + XCTAssertEqual( + try fetchPaymentRows().count, 0, + "the callback must ride the round's atomic commit, not flush early" + ) + handler.endChangeset(walletId: walletId, success: true) + + let rows = try fetchPaymentRows() + XCTAssertEqual(rows.count, 1) + let row = try XCTUnwrap(rows.first) + XCTAssertEqual(row.memo, "rent + utilities") + XCTAssertEqual(row.status, .pending) + XCTAssertEqual(row.direction, .sent) + XCTAssertEqual(row.ownerIdentityId, ownerId) + XCTAssertEqual(row.txid, txid) + + // Cold-start restore: the row must ride the identity restore + // buffer's payments array back into Rust. + let (entries, count, errored) = handler.loadWalletList() + XCTAssertFalse(errored) + XCTAssertEqual(count, 1) + let entriesPtr = try XCTUnwrap(entries) + defer { handler.loadWalletListFree(entries: UnsafeRawPointer(entriesPtr)) } + + var restored: [(txid: String, memo: String?)] = [] + let walletEntry = entriesPtr[0] + for iIdx in 0.. 0 else { + continue + } + for pIdx in 0.. Date: Fri, 7 Aug 2026 04:23:04 +0700 Subject: [PATCH 10/12] fix(platform-wallet): address payment-persistence review nits MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Register on_persist_dashpay_payments_fn in the parity manifest's shared_symbols and the capability's shared_apis; downgrade the Swift host to partial with restart required — the loadWalletList() round-trip test exercises the write-then-restore loop in-process against an in-memory container, which does not validate a real process death. Truncate the parked-row drop log to the owner id's first eight bytes, matching the file's identifier-logging convention. Co-Authored-By: Claude Fable 5 --- docs/sdk/sdk-parity-manifest.json | 12 +++++++----- .../PlatformWalletPersistenceHandler.swift | 2 +- 2 files changed, 8 insertions(+), 6 deletions(-) diff --git a/docs/sdk/sdk-parity-manifest.json b/docs/sdk/sdk-parity-manifest.json index 093292856c0..21e8f603f51 100644 --- a/docs/sdk/sdk-parity-manifest.json +++ b/docs/sdk/sdk-parity-manifest.json @@ -30,6 +30,7 @@ "on_load_shielded_viewing_keys_fn": "packages/rs-platform-wallet-ffi/src/persistence.rs", "on_load_shielded_viewing_keys_free_fn": "packages/rs-platform-wallet-ffi/src/persistence.rs", "on_persist_address_balances_fn": "packages/rs-platform-wallet-ffi/src/persistence.rs", + "on_persist_dashpay_payments_fn": "packages/rs-platform-wallet-ffi/src/persistence.rs", "on_persist_shielded_viewing_keys_fn": "packages/rs-platform-wallet-ffi/src/persistence.rs", "platform_address_wallet_addresses_with_balances": "packages/rs-platform-wallet-ffi/src/platform_addresses/wallet.rs", "platform_wallet_claim_invitation": "packages/rs-platform-wallet-ffi/src/invitation.rs", @@ -1040,7 +1041,8 @@ "title": "DashPay payment history persists event-driven through the persister callback", "area": "persistence", "shared_apis": [ - "managed_identity_get_dashpay_payments" + "managed_identity_get_dashpay_payments", + "on_persist_dashpay_payments_fn" ], "required_persistence_capabilities": [ "atomic_changesets", @@ -1048,10 +1050,10 @@ ], "hosts": { "swift": { - "sdk": "supported", + "sdk": "partial", "example_app": "not-applicable", - "restart": "tested", - "reason": "The on_persist_dashpay_payments_fn vtable slot lands PersistentDashpayPayment rows on every Rust store() round (live sends with memos, pending-to-confirmed sweep flips, reconstruction upserts), and the identity restore buffer's payments array feeds them back at load — Sent entries and memos survive relaunch without any UI surface appearing. The getter-backed refreshDashPayPayments path remains as a reconciler." + "restart": "required", + "reason": "The on_persist_dashpay_payments_fn vtable slot lands PersistentDashpayPayment rows on every Rust store() round (live sends with memos, pending-to-confirmed sweep flips, reconstruction upserts), and the identity restore buffer's payments array feeds them back at load; the getter-backed refreshDashPayPayments path remains as a reconciler. Remains partial pending a process-death restart gate: the loadWalletList() round-trip test exercises the write-then-restore loop in-process against an in-memory container, which does not validate survival across a real app kill and relaunch." }, "kotlin": { "sdk": "not-applicable", @@ -1067,7 +1069,7 @@ "file": "packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DashPayPersistenceTests.swift", "id": "testChangesetRoundPersistsSentEntryAndRestoreBufferRoundTripsIt", "command": "swift test --package-path packages/swift-sdk --filter DashPayPaymentPersistenceTests", - "covers_restart": true + "covers_restart": false }, { "host": "swift", diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index 5fb539815f3..9479cbfb6b3 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -2525,7 +2525,7 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { } else { print( "⚠️ persistDashpayPayments: owner identity " - + "\(entry.ownerIdentityId.toHexString()) never appeared; dropping " + + "\(entry.ownerIdentityId.prefix(8).toHexString())… never appeared; dropping " + "\(entry.payments.count) parked payment row(s) — the refresh " + "reconciler will restore them on the next read" ) From 3fe84b382bf2e5f39f449436bfccd8a67b170ea4 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 7 Aug 2026 04:54:25 +0700 Subject: [PATCH 11/12] fix(platform-wallet): bound the payment projection and keep deferred rows inside the atomic round MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Address the two thepastaclaw findings: Blocker — deferred payments persisted after the round committed: endChangeset now stages parked payment groups BEFORE the round's single save, so they commit in the same atomic transaction as the owner identity; a group whose owner is still unresolvable fails the whole round (rollback + failure to Rust) instead of committing a lossy persist behind a success report. The post-commit replay helper and its second save are gone. Bounded work — every snapshot replayed the full history: record_dashpay_payment (the single writer for every payment mutation) now rides exactly the changed (owner, txid) row on dashpay_payments_overlay, and FFIPersister::store projects ONLY the overlay — never the full-map IdentityEntry snapshots — so per-round work is bounded by the delta while the snapshot keeps serving blob-style persisters unchanged. New platform-wallet test pins the single-row overlay emission; the FFI test now also pins that a snapshot-only round does not fire the callback. Co-Authored-By: Claude Fable 5 --- docs/sdk/sdk-parity-manifest.json | 12 +- .../src/dashpay_payment.rs | 65 ++++---- .../rs-platform-wallet-ffi/src/persistence.rs | 145 +++++++++--------- .../state/managed_identity/identity_ops.rs | 108 ++++++++++++- .../PlatformWalletPersistenceHandler.swift | 106 ++++++------- .../DashPayPersistenceTests.swift | 48 +++++- 6 files changed, 310 insertions(+), 174 deletions(-) diff --git a/docs/sdk/sdk-parity-manifest.json b/docs/sdk/sdk-parity-manifest.json index 25dec963791..0e76ac58a27 100644 --- a/docs/sdk/sdk-parity-manifest.json +++ b/docs/sdk/sdk-parity-manifest.json @@ -1083,8 +1083,16 @@ "host": "shared", "kind": "unit", "file": "packages/rs-platform-wallet-ffi/src/persistence.rs", - "id": "store_projects_dashpay_payments_from_identities_and_overlay", - "command": "cargo test -p platform-wallet-ffi --lib store_projects_dashpay_payments_from_identities_and_overlay", + "id": "store_projects_dashpay_payments_overlay_only", + "command": "cargo test -p platform-wallet-ffi --lib store_projects_dashpay_payments_overlay_only", + "covers_restart": false + }, + { + "host": "shared", + "kind": "unit", + "file": "packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/identity_ops.rs", + "id": "record_dashpay_payment_emits_single_row_overlay", + "command": "cargo test -p platform-wallet record_dashpay_payment_emits_single_row_overlay", "covers_restart": false } ] diff --git a/packages/rs-platform-wallet-ffi/src/dashpay_payment.rs b/packages/rs-platform-wallet-ffi/src/dashpay_payment.rs index 5cf65fb2baf..60a31b340b8 100644 --- a/packages/rs-platform-wallet-ffi/src/dashpay_payment.rs +++ b/packages/rs-platform-wallet-ffi/src/dashpay_payment.rs @@ -14,11 +14,14 @@ //! //! Payment history persists event-driven through //! `on_persist_dashpay_payments_fn` on the persister vtable, exactly -//! like contact requests and profiles: every `store()` round whose -//! changeset carries payment rows (an `IdentityEntry.dashpay_payments` -//! snapshot from `record_dashpay_payment`, or a merged -//! `dashpay_payments_overlay`) projects them to the host. This closes -//! the write half of the durability loop whose read half — the +//! like contact requests and profiles: `record_dashpay_payment` — the +//! single writer for every payment mutation — rides the changed +//! `(owner, txid)` row on `dashpay_payments_overlay`, and every +//! `store()` round carrying that overlay projects it to the host. +//! Only the overlay is projected (never the full-map +//! `IdentityEntry.dashpay_payments` snapshots), so per-round work is +//! bounded by the delta rather than the accumulated history. This +//! closes the write half of the durability loop whose read half — the //! `payments` array on `IdentityRestoreEntryFFI` — already rehydrates //! the map at load. (An earlier revision shipped only the //! [`managed_identity_get_dashpay_payments`] getter, on the rationale @@ -136,7 +139,7 @@ pub struct DashpayPaymentPersistEntryFFI { pub memo: *const c_char, } -/// Flatten per-identity payment maps into persist-callback rows. +/// Flatten a `dashpay_payments_overlay` into persist-callback rows. /// /// Returns the row array plus the `CString` storage backing every /// `txid` / `memo` pointer — the caller must keep the storage alive @@ -145,32 +148,34 @@ pub struct DashpayPaymentPersistEntryFFI { /// panicking); a memo with an interior NUL degrades to null, matching /// [`cstring_or_null`]'s contract on the getter side. pub(crate) fn build_payment_persist_entries( - payments: &BTreeMap<(Identifier, &str), &PaymentEntry>, + overlay: &BTreeMap>, ) -> (Vec, Vec) { let mut storage: Vec = Vec::new(); - let mut rows: Vec = Vec::with_capacity(payments.len()); - for ((owner_id, txid), entry) in payments { - let Ok(txid_c) = CString::new(*txid) else { - continue; - }; - storage.push(txid_c); - let txid_ptr = storage.last().expect("pushed txid CString above").as_ptr(); - let memo_ptr = match entry.memo.as_deref().map(CString::new) { - Some(Ok(memo_c)) => { - storage.push(memo_c); - storage.last().expect("pushed memo CString above").as_ptr() - } - _ => std::ptr::null(), - }; - rows.push(DashpayPaymentPersistEntryFFI { - owner_identity_id: owner_id.to_buffer(), - counterparty_id: entry.counterparty_id.to_buffer(), - amount_duffs: entry.amount_duffs, - direction_raw: DashpayPaymentDirectionFFI::from(entry.direction) as u8, - status_raw: DashpayPaymentStatusFFI::from(entry.status) as u8, - txid: txid_ptr, - memo: memo_ptr, - }); + let mut rows: Vec = Vec::new(); + for (owner_id, payments) in overlay { + for (txid, entry) in payments { + let Ok(txid_c) = CString::new(txid.as_str()) else { + continue; + }; + storage.push(txid_c); + let txid_ptr = storage.last().expect("pushed txid CString above").as_ptr(); + let memo_ptr = match entry.memo.as_deref().map(CString::new) { + Some(Ok(memo_c)) => { + storage.push(memo_c); + storage.last().expect("pushed memo CString above").as_ptr() + } + _ => std::ptr::null(), + }; + rows.push(DashpayPaymentPersistEntryFFI { + owner_identity_id: owner_id.to_buffer(), + counterparty_id: entry.counterparty_id.to_buffer(), + amount_duffs: entry.amount_duffs, + direction_raw: DashpayPaymentDirectionFFI::from(entry.direction) as u8, + status_raw: DashpayPaymentStatusFFI::from(entry.status) as u8, + txid: txid_ptr, + memo: memo_ptr, + }); + } } (rows, storage) } diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index d5261737c19..4bed9e757b4 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -738,12 +738,16 @@ pub struct PersistenceCallbacks { count: usize, ), >, - /// Forwards DashPay payment-history rows (the - /// `IdentityEntry.dashpay_payments` snapshots and any merged - /// `dashpay_payments_overlay` on the changeset, flattened + deduped - /// per `(owner, txid)`) to the host. Appended at the END so the - /// struct layout stays stable — a host built against the previous - /// vtable keeps working, it simply never sets this slot. + /// Forwards DashPay payment-history rows — the changeset's + /// `dashpay_payments_overlay`, which `record_dashpay_payment` (the + /// single writer for every payment mutation) populates with exactly + /// the changed `(owner, txid)` row(s) — to the host. Per-call work + /// is therefore bounded by the delta, never the identity's + /// accumulated history; the full-map snapshots riding + /// `changeset.identities` are deliberately not projected. Appended + /// at the END so the struct layout stays stable — a host built + /// against the previous vtable keeps working, it simply never sets + /// this slot. /// /// Rows are upserts only: the Rust-side map is append-only history /// keyed by txid, so status flips (Pending → Confirmed / Failed) @@ -1336,38 +1340,24 @@ impl PlatformWalletPersistence for FFIPersister { } } - // Send DashPay payment-history rows. Payments reach a store round - // on TWO carriers: the full-map `IdentityEntry.dashpay_payments` - // snapshot inside `changeset.identities` (what - // `record_dashpay_payment` / every scalar identity mutation - // emits) and the merged `dashpay_payments_overlay` (what - // `Merge`-combined rounds carry). Project both, deduped by - // `(owner, txid)` with the overlay winning — it is the - // later-merged delta. Fires AFTER the identities callback so a - // brand-new owner's `PersistentIdentity` row is already staged - // in the same round when the host resolves the payment's owner - // link. - if let Some(cb) = self.callbacks.on_persist_dashpay_payments_fn { - let mut merged: std::collections::BTreeMap< - (dpp::prelude::Identifier, &str), - &platform_wallet::wallet::identity::PaymentEntry, - > = std::collections::BTreeMap::new(); - if let Some(ref id_cs) = changeset.identities { - for (identity_id, entry) in &id_cs.identities { - for (txid, payment) in &entry.dashpay_payments { - merged.insert((*identity_id, txid.as_str()), payment); - } - } - } - if let Some(ref overlay) = changeset.dashpay_payments_overlay { - for (identity_id, payments) in overlay { - for (txid, payment) in payments { - merged.insert((*identity_id, txid.as_str()), payment); - } - } - } - if !merged.is_empty() { - let (entries, _string_storage) = build_payment_persist_entries(&merged); + // Send DashPay payment-history rows — the `dashpay_payments_overlay` + // ONLY. `record_dashpay_payment`, the single writer for every + // payment mutation (live sends, confirm-sweep flips, reconstruction + // upserts), emits exactly the changed `(owner, txid)` row on the + // overlay, so per-round work here is bounded by the delta. The + // full-map `IdentityEntry.dashpay_payments` snapshots riding + // `changeset.identities` are deliberately NOT projected: replaying + // an identity's complete history on every snapshot (including + // unrelated scalar mutations) is unbounded per-call work as history + // grows, against the persistence trait's bounded-work guidance — + // history bootstrap is the host restore buffer's job, and the + // getter-backed reconciler covers gaps. Fires AFTER the identities + // callback so a brand-new owner's `PersistentIdentity` row is + // already staged in the same round when the host resolves the + // payment's owner link. + if let Some(ref overlay) = changeset.dashpay_payments_overlay { + if let Some(cb) = self.callbacks.on_persist_dashpay_payments_fn { + let (entries, _string_storage) = build_payment_persist_entries(overlay); if !entries.is_empty() { let result = unsafe { cb( @@ -6117,18 +6107,19 @@ mod tests { ); } - /// A store round whose changeset carries payment rows on either - /// carrier — the full-map `IdentityEntry.dashpay_payments` snapshot - /// (what `record_dashpay_payment` emits inside - /// `changeset.identities`) or a merged `dashpay_payments_overlay` - /// — must flatten BOTH through `on_persist_dashpay_payments_fn`, - /// deduped by `(owner, txid)` with the overlay winning. This is the - /// write half of the relaunch-durability loop: without it a live - /// send's Sent entry + memo exist only in memory and `store()` - /// returns Ok having persisted nothing for payments, silently - /// defeating `record_dashpay_payment`'s rollback invariant. + /// A store round carrying a `dashpay_payments_overlay` — the + /// single-row delta `record_dashpay_payment` emits for every + /// payment mutation — must project exactly those rows through + /// `on_persist_dashpay_payments_fn`, and a round carrying only the + /// full-map `IdentityEntry.dashpay_payments` snapshot must NOT + /// fire the callback at all. The first half is the write half of + /// the relaunch-durability loop (without it a live send's Sent + /// entry + memo exist only in memory while `store()` returns Ok); + /// the second half pins the bounded-work contract — replaying an + /// identity's complete history on every snapshot is unbounded + /// per-call work as history grows. #[test] - fn store_projects_dashpay_payments_from_identities_and_overlay() { + fn store_projects_dashpay_payments_overlay_only() { use platform_wallet::changeset::IdentityChangeSet; use platform_wallet::wallet::identity::{PaymentEntry, PaymentStatus}; use std::sync::atomic::{AtomicUsize, Ordering}; @@ -6177,19 +6168,15 @@ mod tests { 0 } - // Live-send shape: a managed identity carrying one Pending Sent - // payment with a memo, snapshotted the same way - // `record_dashpay_payment` does. + // An identity snapshot whose payments map carries a row that is + // NOT in the overlay — the projection must ignore the snapshot + // entirely. let identity = dpp::identity::Identity::V0(dpp::identity::v0::IdentityV0::default()); let mut managed = platform_wallet::ManagedIdentity::new(identity, 0); - let sent_txid = "aa".repeat(32); + let historical_txid = "cc".repeat(32); managed.dashpay_payments_mut().insert( - sent_txid.clone(), - PaymentEntry::new_sent( - dpp::prelude::Identifier::from([7u8; 32]), - 12_000, - Some("lunch".into()), - ), + historical_txid.clone(), + PaymentEntry::new_sent(dpp::prelude::Identifier::from([6u8; 32]), 999, None), ); let owner_id = managed.id(); let mut id_cs = IdentityChangeSet::default(); @@ -6198,10 +6185,9 @@ mod tests { platform_wallet::changeset::IdentityEntry::from_managed(&managed), ); - // Overlay carriers: (a) the SAME (owner, txid) flipped to - // Confirmed — must win over the snapshot's Pending row — and - // (b) a second owner's Received row that exists only on the - // overlay. + // The overlay delta: one Confirmed Sent row with a memo for the + // snapshot's owner, one Received row for a second owner. + let sent_txid = "aa".repeat(32); let mut confirmed = PaymentEntry::new_sent( dpp::prelude::Identifier::from([7u8; 32]), 12_000, @@ -6224,7 +6210,7 @@ mod tests { ); let changeset = PlatformWalletChangeSet { - identities: Some(id_cs), + identities: Some(id_cs.clone()), dashpay_payments_overlay: Some(overlay), ..Default::default() }; @@ -6240,24 +6226,21 @@ mod tests { .store([1u8; 32], changeset) .expect("payment round must succeed"); - assert_eq!( - sink.calls.load(Ordering::SeqCst), - 1, - "both carriers must flatten into a single callback fire" - ); + assert_eq!(sink.calls.load(Ordering::SeqCst), 1); let mut rows = sink.rows.lock().expect("sink lock").clone(); rows.sort(); - assert_eq!(rows.len(), 2, "one deduped row per (owner, txid)"); + assert_eq!( + rows.len(), + 2, + "exactly the overlay's delta rows — the snapshot's historical row must not ride along" + ); // BTreeMap order: default-id owner ([0; 32]) before [9; 32]. let (owner, txid, amount, direction, status, memo) = &rows[0]; assert_eq!(*owner, owner_id.to_buffer()); assert_eq!(*txid, sent_txid); assert_eq!(*amount, 12_000); assert_eq!(*direction, 0, "Sent discriminant"); - assert_eq!( - *status, 1, - "the overlay's Confirmed flip must win over the snapshot's Pending row" - ); + assert_eq!(*status, 1, "Confirmed discriminant"); assert_eq!(memo.as_deref(), Some("lunch")); let (owner, txid, amount, direction, status, memo) = &rows[1]; assert_eq!(*owner, [9u8; 32]); @@ -6267,6 +6250,20 @@ mod tests { assert_eq!(*status, 1, "Received entries record as Confirmed"); assert!(memo.is_none()); + // A snapshot-only round (payments in the identity map, no + // overlay) must NOT fire the callback — the bounded-work pin: + // identity snapshots must not replay payment history. + persister + .store( + [1u8; 32], + PlatformWalletChangeSet { + identities: Some(id_cs), + ..Default::default() + }, + ) + .expect("snapshot-only round must succeed"); + assert_eq!(sink.calls.load(Ordering::SeqCst), 1); + // A payments-free round must not fire the callback at all. persister .store([1u8; 32], PlatformWalletChangeSet::default()) diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/identity_ops.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/identity_ops.rs index 50a615aa7eb..933f3aad5c6 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/identity_ops.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/identity_ops.rs @@ -157,9 +157,23 @@ impl ManagedIdentity { // A failed overwrite restores the previous entry rather than deleting // it. Either way the persist result is returned, not swallowed: the // user-initiated send path (`send_payment`) surfaces it in the UI. - let previous = self.dashpay.payments.insert(tx_id.clone(), entry); - let cs = self.snapshot_changeset(); - if let Err(e) = persister.store(cs.into()) { + let previous = self.dashpay.payments.insert(tx_id.clone(), entry.clone()); + let mut cs: crate::changeset::PlatformWalletChangeSet = self.snapshot_changeset().into(); + // Ride the changed row on `dashpay_payments_overlay` as well: the + // identity snapshot above carries the FULL payments map (blob-style + // persisters overwrite the whole entry), so a delta-style persister + // projecting the snapshot would replay the identity's complete + // history on every recorded payment — unbounded per-call work as + // history grows, against the `PlatformWalletPersistence` bounded- + // work guidance. The single-row overlay is the bounded carrier + // those persisters (the FFI vtable) project instead; every payment + // mutation funnels through this method, so the overlay sees every + // write. + cs.dashpay_payments_overlay = Some(BTreeMap::from([( + self.id(), + BTreeMap::from([(tx_id.clone(), entry)]), + )])); + if let Err(e) = persister.store(cs) { match previous { Some(prev) => { self.dashpay.payments.insert(tx_id, prev); @@ -640,6 +654,94 @@ mod tests { ); } + /// Recording a payment emits BOTH carriers with the right granularity: + /// the identity snapshot keeps the full payments map (blob-style + /// persisters overwrite the whole entry) while + /// `dashpay_payments_overlay` carries EXACTLY the one changed + /// `(owner, txid)` row — never the accumulated history. Delta-style + /// persisters (the FFI vtable) project only the overlay, so this + /// single-row shape is what keeps per-store work bounded as history + /// grows: N recorded payments must emit N overlay rows total, not + /// 1 + 2 + … + N. + #[test] + fn record_dashpay_payment_emits_single_row_overlay() { + let owner_id = Identifier::from([1u8; 32]); + let identity = Identity::V0(IdentityV0 { + id: owner_id, + public_keys: BTreeMap::new(), + balance: 0, + revision: 0, + }); + let mut managed = ManagedIdentity::new(identity, 0); + let capturing = std::sync::Arc::new(CapturingPersister::default()); + let p = WalletPersister::new([0xAB; 32], capturing.clone() as _); + let alice = Identifier::from([0xAA; 32]); + + managed + .record_dashpay_payment( + "tx1".into(), + PaymentEntry::new_sent(alice, 100, Some("rent".into())), + &p, + ) + .expect("record tx1"); + managed + .record_dashpay_payment( + "tx2".into(), + PaymentEntry::new_received(alice, 250, None), + &p, + ) + .expect("record tx2"); + + let stores = capturing.stores.lock().unwrap(); + assert_eq!(stores.len(), 2); + + // Second store: the snapshot map has both rows, the overlay only + // the newly recorded one. + let cs = &stores[1]; + let snapshot = cs + .identities + .as_ref() + .expect("identity snapshot rides along") + .identities + .get(&owner_id) + .expect("owner entry"); + assert_eq!( + snapshot.dashpay_payments.len(), + 2, + "full map on the blob carrier" + ); + let overlay = cs + .dashpay_payments_overlay + .as_ref() + .expect("overlay must carry the delta") + .get(&owner_id) + .expect("owner overlay"); + assert_eq!( + overlay.len(), + 1, + "the overlay must carry only the changed row, not the history" + ); + let row = overlay.get("tx2").expect("the just-recorded txid"); + assert_eq!(row.amount_duffs, 250); + + // A status flip re-emits the same single (owner, txid) row. + let mut confirmed = PaymentEntry::new_sent(alice, 100, Some("rent".into())); + confirmed.status = crate::wallet::identity::PaymentStatus::Confirmed; + drop(stores); + managed + .record_dashpay_payment("tx1".into(), confirmed.clone(), &p) + .expect("flip tx1"); + let stores = capturing.stores.lock().unwrap(); + let overlay = stores[2] + .dashpay_payments_overlay + .as_ref() + .expect("overlay on the flip") + .get(&owner_id) + .expect("owner overlay"); + assert_eq!(overlay.len(), 1); + assert_eq!(overlay.get("tx1"), Some(&confirmed)); + } + /// A failed payment persist must NOT strand the entry in memory — else a /// caller's `contains_key` retry guard skips the next sweep's re-attempt /// and the Sent entry + memo (no on-chain recovery) is permanently lost. diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift index 9479cbfb6b3..a665f6d1e9f 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletPersistenceHandler.swift @@ -127,13 +127,16 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { /// callback fires before the payments callback in the same Rust /// `store()` round, and `FetchDescriptor` sees the round's pending /// inserts — so this only holds rows whose owner is in neither the - /// current round nor the store. Replayed once by `endChangeset` - /// after a successful commit (one more chance for a late owner - /// row), then dropped with a log — the `refreshDashPayPayments` - /// reconciler re-upserts anything dropped here. Discarded on - /// rollback: a failed round also rolls the entries back out of the - /// Rust in-memory map, so persisting them would fabricate history. - /// Confined to `serialQueue` like all other mutable handler state. + /// current round (yet) nor the store. Drained by `endChangeset` + /// BEFORE the round's single `save()`, so parked rows commit + /// atomically with everything else; a group whose owner is still + /// unresolvable at that point fails the whole round (rollback + + /// failure reported to Rust) rather than committing a lossy + /// persist. Cleared without staging on a failed round: the Rust + /// side rolled its in-memory entries back too, so persisting them + /// later would fabricate history. Never survives a round either + /// way, so it cannot grow across rounds. Confined to `serialQueue` + /// like all other mutable handler state. private var deferredPaymentUpserts: [(ownerIdentityId: Data, payments: [DashPayPayment])] = [] public init(modelContainer: ModelContainer, network: Network? = nil) { @@ -1374,13 +1377,34 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { self.drainDeferredBackfills() } if success { + // Stage payment groups parked on a mid-round missing owner + // BEFORE the round's single save — by now every identity + // insert the round staged is visible to the fetch. A group + // whose owner is STILL unresolvable fails the whole round: + // committing the rest while dropping payment rows would + // report success for a lossy persist, and Rust would keep + // in-memory payment state that never reached disk — the + // exact invariant `record_dashpay_payment`'s rollback + // protects. No second save after the commit: the round + // stays one atomic transaction. + let parked = deferredPaymentUpserts + deferredPaymentUpserts.removeAll() + for entry in parked where !stageDashpayPaymentUpserts( + ownerIdentityId: entry.ownerIdentityId, + payments: entry.payments + ) { + print( + "⚠️ endChangeset: no PersistentIdentity for owner " + + "\(entry.ownerIdentityId.prefix(8).toHexString())… after the " + + "round's identity applies; failing the round so Rust rolls " + + "back \(entry.payments.count) payment row(s) instead of " + + "losing them" + ) + backgroundContext.rollback() + return false + } do { try backgroundContext.save() - // With the round durably committed, give payment rows - // parked on a missing owner identity one replay — a - // successful commit is the only point a late owner row - // can have become visible. - replayDeferredPaymentUpserts() return true } catch { // The context still has the pending changes on @@ -1391,14 +1415,13 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { // round did NOT commit, so report failure upward. print("⚠️ endChangeset: save failed: \(error.localizedDescription)") backgroundContext.rollback() - // The failed round's Rust-side rollback also removed - // these entries from the in-memory map — persisting - // them would fabricate history. - deferredPaymentUpserts.removeAll() return false } } else { backgroundContext.rollback() + // Parked rows from the failed round die with it: the Rust + // side rolled its in-memory entries back too, so persisting + // them later would fabricate history. deferredPaymentUpserts.removeAll() return false } @@ -2484,9 +2507,10 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { /// the round's `endChangeset` commit/rollback. A group whose owner /// `PersistentIdentity` row isn't resolvable — not even as a /// pending insert staged by this round's identities callback, - /// which fires first — is parked on `deferredPaymentUpserts` for - /// one post-commit replay rather than dropped (see that field's - /// doc). + /// which fires first — is parked on `deferredPaymentUpserts`; + /// `endChangeset` stages parked groups before the round's single + /// save and fails the round if an owner is still unresolvable (see + /// that field's doc). func persistDashpayPayments( walletId: Data, entriesByOwner: [Data: [DashPayPayment]] @@ -2505,41 +2529,6 @@ public final class PlatformWalletPersistenceHandler: @unchecked Sendable { } } - /// Replay payment groups parked on a missing owner identity. Must - /// run on `serialQueue`, immediately after a successful round - /// commit (the only point a late owner row becomes visible). One - /// attempt: still-missing owners are dropped with a log — the - /// `refreshDashPayPayments` reconciler re-upserts them later — - /// so the parked list can never grow across rounds. - private func replayDeferredPaymentUpserts() { - guard !deferredPaymentUpserts.isEmpty else { return } - let parked = deferredPaymentUpserts - deferredPaymentUpserts.removeAll() - var stagedAny = false - for entry in parked { - if stageDashpayPaymentUpserts( - ownerIdentityId: entry.ownerIdentityId, - payments: entry.payments - ) { - stagedAny = true - } else { - print( - "⚠️ persistDashpayPayments: owner identity " - + "\(entry.ownerIdentityId.prefix(8).toHexString())… never appeared; dropping " - + "\(entry.payments.count) parked payment row(s) — the refresh " - + "reconciler will restore them on the next read" - ) - } - } - if stagedAny { - do { - try backgroundContext.save() - } catch { - print("⚠️ replayDeferredPaymentUpserts: SwiftData save failed — payment history may be incomplete: \(error)") - } - } - } - /// Stage upserts for one owner's payment rows on /// `backgroundContext` — shared core of the persister callback, /// the post-commit replay, and the refresh reconciler. No @@ -7792,10 +7781,11 @@ private func listWalletCoreTxidsFreeCallback( /// we return. Rows without a txid pointer are skipped defensively — /// the Rust builder documents `txid` as always non-null. /// -/// Always returns 0: a missing owner identity parks the group for a -/// post-commit replay rather than failing (see -/// `deferredPaymentUpserts`), and a commit failure is reported through -/// the round's `on_changeset_end_fn` return instead. +/// Always returns 0: a missing owner identity parks the group on +/// `deferredPaymentUpserts` — staged before the round's single save, +/// with a still-unresolvable owner failing the round — and a commit +/// failure is reported through the round's `on_changeset_end_fn` +/// return, so per-batch failure signaling here would be redundant. private func persistDashpayPaymentsCallback( context: UnsafeMutableRawPointer?, walletIdPtr: UnsafePointer?, diff --git a/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DashPayPersistenceTests.swift b/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DashPayPersistenceTests.swift index 01e8eecd965..5b509c904e1 100644 --- a/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DashPayPersistenceTests.swift +++ b/packages/swift-sdk/SwiftTests/SwiftDashSDKTests/DashPayPersistenceTests.swift @@ -962,11 +962,12 @@ final class DashPayPaymentPersistenceTests: XCTestCase { } /// A payments batch whose owner identity is staged LATER in the - /// same round (or is otherwise unresolvable mid-round) parks and - /// replays once the round commits — preserving the replay - /// semantics the refresh path has, instead of silently dropping - /// the rows. - func testRowsForAnOwnerStagedLaterInTheRoundReplayAfterCommit() throws { + /// same round parks mid-round and is staged again by `endChangeset` + /// BEFORE the round's single save — so the payment rows commit in + /// the same atomic transaction as the owner identity, never in a + /// second post-commit save a process kill could separate from the + /// round. + func testRowsForAnOwnerStagedLaterInTheRoundCommitAtomically() throws { let walletId = Data(repeating: 0xAA, count: 32) let lateOwner = Data(repeating: 0x33, count: 32) // Wallet row so `persistIdentities` can resolve the network @@ -999,14 +1000,47 @@ final class DashPayPaymentPersistenceTests: XCTestCase { ], removed: [] ) - handler.endChangeset(walletId: walletId, success: true) + let committed = handler.endChangeset(walletId: walletId, success: true) + XCTAssertTrue(committed, "a resolvable parked owner must not fail the round") let rows = try fetchPaymentRows() - XCTAssertEqual(rows.count, 1, "parked rows must replay once the owner commits") + XCTAssertEqual( + rows.count, 1, + "parked rows must commit with the round's single save" + ) XCTAssertEqual(rows.first?.ownerIdentityId, lateOwner) XCTAssertEqual(rows.first?.memo, "parked") } + /// A payments batch whose owner identity never appears — neither + /// pre-existing nor staged by the round — must FAIL the round: + /// `endChangeset` rolls back and reports failure to Rust (which + /// then rolls its in-memory entry back), instead of committing the + /// rest of the round while silently dropping payment rows. + func testUnresolvableOwnerFailsTheRoundInsteadOfDroppingRows() throws { + let walletId = Data(repeating: 0xAA, count: 32) + let ghostOwner = Data(repeating: 0x55, count: 32) + + handler.beginChangeset(walletId: walletId) + handler.persistDashpayPayments( + walletId: walletId, + entriesByOwner: [ + ownerId: [makePayment()], + ghostOwner: [makePayment(memo: "orphaned")], + ] + ) + let committed = handler.endChangeset(walletId: walletId, success: true) + + XCTAssertFalse( + committed, + "an unresolvable payment owner must fail the round, not drop the rows" + ) + XCTAssertEqual( + try fetchPaymentRows().count, 0, + "the failed round must roll back everything it staged" + ) + } + /// A failed round discards BOTH staged and parked payment rows — /// the Rust side rolled the entries back out of its in-memory map, /// so persisting them later would fabricate history. From 1c70311612530ed2bc095e872c0d28d407e34905 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Fri, 7 Aug 2026 04:59:00 +0700 Subject: [PATCH 12/12] fix(platform-wallet): declare the payments getter in the parity manifest's shared_symbols managed_identity_get_dashpay_payments rode the capability's shared_apis without a shared_symbols declaration, which the manifest validator rejects. Co-Authored-By: Claude Fable 5 --- docs/sdk/sdk-parity-manifest.json | 1 + 1 file changed, 1 insertion(+) diff --git a/docs/sdk/sdk-parity-manifest.json b/docs/sdk/sdk-parity-manifest.json index 0e76ac58a27..d1aa778b140 100644 --- a/docs/sdk/sdk-parity-manifest.json +++ b/docs/sdk/sdk-parity-manifest.json @@ -27,6 +27,7 @@ "dash_sdk_sign_async_completion": "packages/rs-sdk-ffi/src/signer.rs", "dpns_name_array_free": "packages/rs-platform-wallet-ffi/src/dpns.rs", "managed_identity_get_contested_dpns_names": "packages/rs-platform-wallet-ffi/src/dpns.rs", + "managed_identity_get_dashpay_payments": "packages/rs-platform-wallet-ffi/src/dashpay_payment.rs", "on_load_shielded_viewing_keys_fn": "packages/rs-platform-wallet-ffi/src/persistence.rs", "on_load_shielded_viewing_keys_free_fn": "packages/rs-platform-wallet-ffi/src/persistence.rs", "on_persist_address_balances_fn": "packages/rs-platform-wallet-ffi/src/persistence.rs",