From e66fbf557510aaa17d2f83ffbc97170dbe4addc5 Mon Sep 17 00:00:00 2001 From: Quantum Explorer Date: Thu, 2 Jul 2026 18:12:48 +0700 Subject: [PATCH 1/2] fix(platform-wallet): preserve persisted wallet state through seedless rehydration Review follow-ups for the watch-only rehydration PR: - Carry the FFI persister's fully-restored ManagedWalletInfo across load() as ClientWalletStartState::core_wallet_info (still keyless: no Wallet, no seed). The manager consumes the snapshot directly, so per-account UTXO/tx-record attribution, exact pool contents (derived-but-unused deep addresses stay in the SPV watch set), and per-index used flags survive a restart verbatim - and the second eager gap-window derivation on the launch path disappears. The core_state/used_core_addresses projection remains as the fallback for persisters that cannot reconstruct a snapshot (SQLite until dashpay/platform#3968). - Run the rehydration mark<->refill loop to a fixpoint so a previously used address in the wedge zone (past the discovery horizon, within gap-refill reach) is re-marked used instead of coming back pool-visible as fresh (address-reuse leak; regression test added). - Hoist the load_from_persistor idempotency check ahead of per-wallet reconstruction so repeat loads skip the rebuild instead of discarding it at the WalletExists arm; validate a carried snapshot's wallet_id/network against its row and skip mismatches as corrupt. - Consume LoadOutcomeFFI in Swift loadFromPersistor: skipped wallets are logged, surfaced via lastLoadSkippedWallets, excluded from the get_wallet loop (no more spurious NotFound in lastError), and the outcome is freed via platform_wallet_load_outcome_free. - Drop RehydrateRowError (variant-for-variant mirror of CorruptKind); build_watch_only_wallet returns CorruptKind directly. - Reuse vec_to_ptr for the FFI skipped array. Co-Authored-By: Claude Fable 5 --- .../src/core_wallet_types.rs | 6 +- .../rs-platform-wallet-ffi/src/manager.rs | 7 +- .../rs-platform-wallet-ffi/src/persistence.rs | 73 ++---- .../changeset/client_wallet_start_state.rs | 24 +- packages/rs-platform-wallet/src/error.rs | 29 --- .../rs-platform-wallet/src/manager/load.rs | 113 +++++++-- .../src/manager/rehydrate.rs | 184 ++++++++++++--- .../tests/rehydration_load.rs | 216 ++++++++++++++++++ .../PlatformWalletManager.swift | 73 +++++- 9 files changed, 582 insertions(+), 143 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/core_wallet_types.rs b/packages/rs-platform-wallet-ffi/src/core_wallet_types.rs index c8ebc1348f..1d3dc910e0 100644 --- a/packages/rs-platform-wallet-ffi/src/core_wallet_types.rs +++ b/packages/rs-platform-wallet-ffi/src/core_wallet_types.rs @@ -964,7 +964,11 @@ fn tx_record_to_ffi( } } -fn vec_to_ptr(v: Vec) -> *mut T { +/// Convert a `Vec` into a raw heap pointer for a C out-array: null for +/// empty, `Box::into_raw(boxed_slice)` otherwise. The caller owns the +/// allocation and must free it by reconstructing the boxed slice with +/// the ORIGINAL length. +pub(crate) fn vec_to_ptr(v: Vec) -> *mut T { if v.is_empty() { std::ptr::null_mut() } else { diff --git a/packages/rs-platform-wallet-ffi/src/manager.rs b/packages/rs-platform-wallet-ffi/src/manager.rs index dd6430e8cd..438f543a11 100644 --- a/packages/rs-platform-wallet-ffi/src/manager.rs +++ b/packages/rs-platform-wallet-ffi/src/manager.rs @@ -438,12 +438,7 @@ pub unsafe extern "C" fn platform_wallet_manager_load_from_persistor( }) .collect(); let skipped_count = skipped_vec.len(); - let skipped_ptr = if skipped_count == 0 { - std::ptr::null_mut() - } else { - let boxed = skipped_vec.into_boxed_slice(); - Box::into_raw(boxed) as *mut SkippedWalletFFI - }; + let skipped_ptr = crate::core_wallet_types::vec_to_ptr(skipped_vec); std::ptr::write( out_outcome, LoadOutcomeFFI { diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 50eed9ade5..ccd9875a05 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -3451,16 +3451,20 @@ fn build_wallet_start_state( // status without rebroadcasting. let unused_asset_locks = build_unused_asset_locks(entry)?; - // Project the reconstructed `wallet` + `wallet_info` into the - // keyless `ClientWalletStartState` the persister contract requires - // (SECRETS.md: no `Wallet`/seed crosses `load()`). The manager - // rebuilds a watch-only wallet from this manifest via - // `Wallet::new_watch_only` and applies this `core_state` projection. - // Signing happens later via the on-demand - // `sign_with_mnemonic_resolver` path, which fail-closed gates the - // resolver-supplied seed against the loaded `wallet_id`. The - // locally-built `wallet` is dropped — it was only needed to shape - // the account collection / UTXO routing above. + // Hand the fully-restored `wallet_info` across as the keyless + // snapshot (SECRETS.md: no `Wallet`/seed crosses `load()` — + // `ManagedWalletInfo` carries balances / pools / UTXOs, never key + // material). The manager rebuilds a watch-only wallet from the + // manifest via `Wallet::new_watch_only` and consumes this snapshot + // directly, so everything the decode blocks above restored survives + // verbatim: per-account UTXO and tx-record attribution (including + // the unresolved asset-lock funding records), exact pool contents + // with per-index `used` flags (the address-reuse guard and the SPV + // watch set), and the sync metadata / chainlock. Signing happens + // later via the on-demand `sign_with_mnemonic_resolver` path, which + // fail-closed gates the resolver-supplied seed against the loaded + // `wallet_id`. The locally-built `wallet` is dropped — it was only + // needed to shape the account collection / UTXO routing above. let account_manifest: Vec = wallet .accounts .all_accounts() @@ -3470,23 +3474,6 @@ fn build_wallet_start_state( account_xpub: a.account_xpub, }) .collect(); - let new_utxos: Vec = wallet_info - .accounts - .all_funding_accounts() - .into_iter() - .flat_map(|acct| acct.utxos.values().cloned()) - .collect(); - let core_state = platform_wallet::changeset::CoreChangeSet { - new_utxos, - last_processed_height: (wallet_info.metadata.last_processed_height > 0) - .then_some(wallet_info.metadata.last_processed_height), - synced_height: (wallet_info.metadata.synced_height > 0) - .then_some(wallet_info.metadata.synced_height), - // Carry the decoded chainlock through the keyless projection; - // `apply_persisted_core_state` re-applies it onto the rebuilt wallet. - last_applied_chain_lock: wallet_info.metadata.last_applied_chain_lock.clone(), - ..Default::default() - }; // `contacts` / `identity_keys` are the PR-3 keyless feed the // manager layers onto the managed identities via @@ -3498,38 +3485,22 @@ fn build_wallet_start_state( // would need a new cross-boundary struct field + Swift wiring, // tracked as a follow-up. Empty slots make `apply_contacts_and_keys` // a no-op for this path, preserving the established iOS behaviour. - // Carry the persisted pool used-state through the keyless projection. - // The pool-decode block above already merged the persisted `used` - // flags into `wallet_info`; project the used addresses out so - // `apply_persisted_core_state` can re-mark them used on rehydrate. - // Without this a previously-used address whose funds were since spent - // comes back marked unused and could be handed out again as a fresh - // receive address — an address-reuse privacy leak. - let used_core_addresses: Vec = { - use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; - let mut used = Vec::new(); - for acct in wallet_info.accounts.all_funding_accounts() { - for pool in acct.managed_account_type().address_pools() { - for info in pool.addresses.values() { - if info.used { - used.push(info.address.clone()); - } - } - } - } - used - }; - + // + // `core_state` / `used_core_addresses` stay empty: they are the + // projection fallback for persisters that cannot reconstruct a full + // snapshot (the SQLite path until dashpay/platform#3968), and the + // manager ignores them when `core_wallet_info` is `Some`. let wallet_state = ClientWalletStartState { network, birth_height: entry.birth_height, account_manifest, - core_state, + core_wallet_info: Some(Box::new(wallet_info)), + core_state: Default::default(), identity_manager, unused_asset_locks, contacts: Default::default(), identity_keys: Default::default(), - used_core_addresses, + used_core_addresses: Vec::new(), }; let platform_address_state = if per_account.is_empty() diff --git a/packages/rs-platform-wallet/src/changeset/client_wallet_start_state.rs b/packages/rs-platform-wallet/src/changeset/client_wallet_start_state.rs index 37328335c3..4c1e3876b8 100644 --- a/packages/rs-platform-wallet/src/changeset/client_wallet_start_state.rs +++ b/packages/rs-platform-wallet/src/changeset/client_wallet_start_state.rs @@ -2,7 +2,8 @@ //! //! **Keyless by type.** This carries everything needed to *reconstruct* //! a watch-only wallet — network, birth height, the account manifest, -//! the rebuilt core-state projection, identities, filtered asset locks — +//! the managed-state snapshot (or its keyless projection), identities, +//! filtered asset locks — //! but **no** [`Wallet`](key_wallet::Wallet) and no seed. The persister //! can never mint a `Wallet`; the manager rebuilds a watch-only one via //! [`Wallet::new_watch_only`](key_wallet::wallet::Wallet::new_watch_only) @@ -18,6 +19,7 @@ use crate::changeset::{ }; use crate::wallet::asset_lock::tracked::TrackedAssetLock; use dashcore::OutPoint; +use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; use key_wallet::{Address, Network}; /// Keyless per-wallet slice of the startup snapshot. @@ -36,6 +38,23 @@ pub struct ClientWalletStartState { /// Keyless account manifest — the account-set oracle for building the /// watch-only wallet (one watch-only account per entry's xpub). pub account_manifest: Vec, + /// Full keyless managed-wallet snapshot for persisters that can + /// reconstruct one — pools with exact derivation indices and `used` + /// flags, per-account UTXO and tx-record attribution, IS-lock set, + /// and sync metadata. [`ManagedWalletInfo`] carries **no key + /// material** (see its docs: balances, account metadata, UTXO set), + /// so the SECRETS.md boundary holds: still no `Wallet`, no seed. + /// + /// When `Some`, the manager consumes it directly (after validating + /// its `wallet_id`/`network` against the row) instead of minting a + /// `ManagedWalletInfo::from_wallet` skeleton and replaying the + /// projection below — preserving per-account attribution, the full + /// SPV watch set, and pool used-state verbatim, without re-deriving + /// anything. The FFI/iOS persister populates this. When `None` (the + /// native/SQLite persister until dashpay/platform#3968), the manager + /// falls back to the skeleton + [`core_state`](Self::core_state) / + /// [`used_core_addresses`](Self::used_core_addresses) replay. + pub core_wallet_info: Option>, /// Keyless projection of the persisted core rows (UTXOs, tx /// records, IS-locks, sync watermarks, `last_applied_chain_lock`). /// The manager applies this onto a fresh @@ -43,6 +62,9 @@ pub struct ClientWalletStartState { /// watch-only wallet. Populated by the persister's /// [`PlatformWalletPersistence::load`](crate::changeset::PlatformWalletPersistence::load) /// implementation reading the persisted core rows. + /// + /// Ignored when [`core_wallet_info`](Self::core_wallet_info) is + /// `Some` — the full snapshot supersedes the projection. pub core_state: CoreChangeSet, /// Lean snapshot of this wallet's /// [`IdentityManager`](crate::wallet::identity::IdentityManager). diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index b6de18a670..48b4938127 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -5,35 +5,6 @@ use key_wallet::account::StandardAccountType; use key_wallet::managed_account::address_pool::AddressPoolType; use key_wallet::Network; -use crate::manager::load_outcome::CorruptKind; - -/// Per-row failure surfacing during watch-only rehydration of a single -/// persisted wallet. Maps 1:1 to [`CorruptKind`] for the -/// [`SkipReason`](crate::manager::load_outcome::SkipReason) the load loop -/// records. -#[derive(Debug)] -pub(crate) enum RehydrateRowError { - /// Manifest was empty — no account to rebuild the wallet around. - MissingManifest, - /// Building a watch-only [`Account`](key_wallet::account::Account) from a - /// manifest entry failed (xpub structurally malformed for its - /// [`AccountType`](key_wallet::account::AccountType)). - MalformedXpub, - /// `AccountCollection::insert` rejected an account (typically a - /// duplicate `account_type` within the manifest). - DecodeError(String), -} - -impl From for CorruptKind { - fn from(e: RehydrateRowError) -> Self { - match e { - RehydrateRowError::MissingManifest => CorruptKind::MissingManifest, - RehydrateRowError::MalformedXpub => CorruptKind::MalformedXpub, - RehydrateRowError::DecodeError(s) => CorruptKind::DecodeError(s), - } - } -} - /// Errors that can occur in platform wallet operations #[derive(Debug, thiserror::Error)] pub enum PlatformWalletError { diff --git a/packages/rs-platform-wallet/src/manager/load.rs b/packages/rs-platform-wallet/src/manager/load.rs index c824319860..73cf040c4a 100644 --- a/packages/rs-platform-wallet/src/manager/load.rs +++ b/packages/rs-platform-wallet/src/manager/load.rs @@ -7,7 +7,7 @@ use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; use crate::changeset::{ClientStartState, ClientWalletStartState, PlatformWalletPersistence}; use crate::error::PlatformWalletError; -use crate::manager::load_outcome::{LoadOutcome, SkipReason}; +use crate::manager::load_outcome::{CorruptKind, LoadOutcome, SkipReason}; use crate::wallet::core::WalletBalance; use crate::wallet::identity::IdentityManager; use crate::wallet::platform_wallet::{PlatformWalletInfo, WalletId}; @@ -21,8 +21,20 @@ impl PlatformWalletManager

{ /// keyless reconstruction snapshot; each wallet is rebuilt via /// [`Wallet::new_watch_only`](key_wallet::wallet::Wallet::new_watch_only) /// from its [`AccountRegistrationEntry`](crate::changeset::AccountRegistrationEntry) - /// manifest, the keyless core-state projection is applied, and the - /// result is registered into the manager. + /// manifest, the managed core state is restored, and the result is + /// registered into the manager. + /// + /// Core state comes in one of two shapes, per wallet: + /// - a full keyless snapshot + /// ([`ClientWalletStartState::core_wallet_info`]) — consumed + /// directly, preserving per-account UTXO/record attribution and + /// exact pool contents (the FFI/iOS persister); or + /// - the keyless projection + /// ([`core_state`](ClientWalletStartState::core_state) + + /// [`used_core_addresses`](ClientWalletStartState::used_core_addresses)), + /// replayed onto a fresh skeleton via + /// [`apply_persisted_core_state`](super::rehydrate::apply_persisted_core_state) + /// (persisters that cannot reconstruct the snapshot). /// /// The load path never touches the seed, so it performs no wrong-seed /// check. Signing happens later, on demand, via the configured @@ -105,6 +117,7 @@ impl PlatformWalletManager

{ network, birth_height, account_manifest, + core_wallet_info, core_state, identity_manager, unused_asset_locks, @@ -113,6 +126,20 @@ impl PlatformWalletManager

{ used_core_addresses, } = wallet_state; + // Idempotency, checked FIRST: a wallet already registered (a + // prior load pass, or a runtime create) is already-satisfied. + // Checking before any reconstruction work matters — the + // rebuild below derives eager gap windows (and possibly a + // deep discovery scan), all of which the `WalletExists` arm + // at insert time would only throw away. + { + let wm = self.wallet_manager.read().await; + if wm.get_wallet(&expected_wallet_id).is_some() { + outcome.loaded.push(expected_wallet_id); + continue 'load; + } + } + // Build the watch-only wallet from the keyless manifest. A // structural decode failure skips this row (per-row // resilience) — it never aborts the batch and never inserts @@ -123,10 +150,8 @@ impl PlatformWalletManager

{ &account_manifest, ) { Ok(w) => w, - Err(row_err) => { - let reason = SkipReason::CorruptPersistedRow { - kind: row_err.into(), - }; + Err(kind) => { + let reason = SkipReason::CorruptPersistedRow { kind }; outcome.skipped.push((expected_wallet_id, reason.clone())); self.event_manager .on_wallet_skipped_on_load(expected_wallet_id, &reason); @@ -134,21 +159,65 @@ impl PlatformWalletManager

{ } }; - // Mint the managed-info skeleton from the watch-only wallet, - // then apply the keyless persisted core state (UTXOs, sync - // watermarks, per-account balances). A wallet with persisted - // UTXOs but no funds account hard-fails here rather than - // reconstructing a silent zero balance. - let mut wallet_info = ManagedWalletInfo::from_wallet(&wallet, birth_height); - if let Err(e) = super::rehydrate::apply_persisted_core_state( - &mut wallet_info, - &account_manifest, - &core_state, - &used_core_addresses, - ) { - load_error = Some(e); - break 'load; - } + let wallet_info = match core_wallet_info { + // Full keyless snapshot carried by the persister (the + // FFI/iOS path): consume it directly. This preserves + // per-account UTXO/record attribution, the exact pool + // contents (derived-but-unused addresses stay in the SPV + // watch set), and per-index used flags — none of which + // the projection replay below can reconstruct — and + // skips a second eager gap-window derivation. + Some(info) => { + let mut info = *info; + // The snapshot must describe this row's wallet; a + // mismatch is a corrupt row, skipped like any other + // structural failure. + if info.wallet_id != expected_wallet_id || info.network != network { + let reason = SkipReason::CorruptPersistedRow { + kind: CorruptKind::DecodeError(format!( + "managed-info snapshot (wallet {}, network {:?}) does not \ + match its row (wallet {}, network {:?})", + hex::encode(info.wallet_id), + info.network, + hex::encode(expected_wallet_id), + network, + )), + }; + outcome.skipped.push((expected_wallet_id, reason.clone())); + self.event_manager + .on_wallet_skipped_on_load(expected_wallet_id, &reason); + continue 'load; + } + // Recompute totals from the carried UTXO set so the + // lock-free balance mirrored below can never drift + // from it (no-silent-zero holds by recomputation). + { + use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; + info.update_balance(); + } + info + } + // No snapshot (native/SQLite persister until + // dashpay/platform#3968): mint the managed-info skeleton + // from the watch-only wallet, then replay the keyless + // projection (UTXOs, sync watermarks, used addresses). A + // wallet with persisted UTXOs but no funds account + // hard-fails here rather than reconstructing a silent + // zero balance. + None => { + let mut wallet_info = ManagedWalletInfo::from_wallet(&wallet, birth_height); + if let Err(e) = super::rehydrate::apply_persisted_core_state( + &mut wallet_info, + &account_manifest, + &core_state, + &used_core_addresses, + ) { + load_error = Some(e); + break 'load; + } + wallet_info + } + }; // Flatten the (account → outpoint → lock) map. let mut tracked_asset_locks = BTreeMap::new(); diff --git a/packages/rs-platform-wallet/src/manager/rehydrate.rs b/packages/rs-platform-wallet/src/manager/rehydrate.rs index 613cfc3abc..2b7238e1ce 100644 --- a/packages/rs-platform-wallet/src/manager/rehydrate.rs +++ b/packages/rs-platform-wallet/src/manager/rehydrate.rs @@ -20,7 +20,8 @@ use key_wallet::wallet::Wallet; use key_wallet::Network; use crate::changeset::AccountRegistrationEntry; -use crate::error::{PlatformWalletError, RehydrateRowError}; +use crate::error::PlatformWalletError; +use crate::manager::load_outcome::CorruptKind; /// Build a watch-only [`Wallet`] from the keyless account manifest. /// @@ -29,8 +30,10 @@ use crate::error::{PlatformWalletError, RehydrateRowError}; /// [`AccountCollection`] is handed to [`Wallet::new_watch_only`] under /// the same id. No key material crosses this function. /// -/// Returns [`RehydrateRowError`] when the row is structurally unusable -/// (caller maps it onto a per-row [`SkipReason`]). +/// Returns [`CorruptKind`] when the row is structurally unusable +/// (caller wraps it in a per-row [`SkipReason`]). +/// +/// [`SkipReason`]: crate::manager::load_outcome::SkipReason /// /// # Trust boundary /// @@ -48,9 +51,9 @@ pub(super) fn build_watch_only_wallet( network: Network, expected_wallet_id: [u8; 32], manifest: &[AccountRegistrationEntry], -) -> Result { +) -> Result { if manifest.is_empty() { - return Err(RehydrateRowError::MissingManifest); + return Err(CorruptKind::MissingManifest); } let mut accounts = AccountCollection::new(); for entry in manifest { @@ -63,10 +66,10 @@ pub(super) fn build_watch_only_wallet( entry.account_xpub, network, ) - .map_err(|_| RehydrateRowError::MalformedXpub)?; + .map_err(|_| CorruptKind::MalformedXpub)?; accounts .insert(account) - .map_err(|e| RehydrateRowError::DecodeError(e.to_string()))?; + .map_err(|e| CorruptKind::DecodeError(e.to_string()))?; } Ok(Wallet::new_watch_only( network, @@ -497,27 +500,43 @@ fn extend_pools_for_restored_addresses( // funded address keeps `used = false` and could be handed out as a fresh // receive address. `mark_used` is a no-op for addresses not in this // pool, so an underived (foreign / sparse) index is never marked. - let mut marked_any = false; - for addr in restored_addresses { - if pool.mark_used(addr) { - marked_any = true; - } - } - - // Refill the gap window past the deepest used index (needs the xpub). - if marked_any { - if let Some(key_source) = key_source.as_ref() { - if let Err(e) = pool.maintain_gap_limit(key_source) { - tracing::warn!( - wallet_id = %hex::encode(wallet_id), - account_type = ?account_type, - pool_type = ?pool.pool_type, - error = %e, - "rehydration: gap-limit maintenance failed; pool window \ - may be short until the next sync" - ); + // + // Mark ↔ refill runs to a FIXPOINT: marking raises `highest_used`, + // whose gap refill can derive a deeper previously-used address that + // the discovery walk missed (e.g. used idx 45 with in-window used + // idx 20 and gap 30 — the walk's horizon stops at 30, but the refill + // reaches 50 and derives idx 45). A single mark-then-refill pass + // would leave that address in the pool with `used = false`, handing + // a previously-used address back out as fresh. Terminates: each + // round marks at least one new address from the finite restored set + // (`mark_used` returns `true` only on an unused→used flip). + loop { + let mut marked_any = false; + for addr in restored_addresses { + if pool.mark_used(addr) { + marked_any = true; } } + if !marked_any { + break; + } + // Refill the gap window past the deepest used index (needs the + // xpub); without one no deeper address can be derived, so a + // single mark pass is all that's possible. + let Some(key_source) = key_source.as_ref() else { + break; + }; + if let Err(e) = pool.maintain_gap_limit(key_source) { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + account_type = ?account_type, + pool_type = ?pool.pool_type, + error = %e, + "rehydration: gap-limit maintenance failed; pool window \ + may be short until the next sync" + ); + break; + } } } Ok(()) @@ -592,7 +611,7 @@ mod tests { fn empty_manifest_is_missing_manifest() { let err = build_watch_only_wallet(Network::Testnet, [0u8; 32], &[]) .expect_err("empty manifest must be MissingManifest"); - assert!(matches!(err, RehydrateRowError::MissingManifest)); + assert!(matches!(err, CorruptKind::MissingManifest)); } /// Regression: after restart-in-place the watch-only pools eagerly @@ -1229,6 +1248,7 @@ mod tests { network: Network::Testnet, birth_height: 1, account_manifest: manifest.clone(), + core_wallet_info: None, core_state: core, identity_manager: Default::default(), unused_asset_locks: Default::default(), @@ -1315,6 +1335,116 @@ mod tests { ); } + /// Regression (mark↔refill fixpoint): a previously-used address in the + /// "wedge zone" — past the discovery horizon but within reach of the + /// gap refill — must come back `used`. With used addresses at idx 20 + /// (in the eager window) and idx 45 (gap 30): the discovery walk + /// excludes in-window addresses from `unresolved`, so nothing anchors + /// the horizon past 30 and idx 45 is never scanned; marking idx 20 then + /// makes `maintain_gap_limit` derive out to 20+30=50, which brings the + /// idx-45 address into the pool. A single mark-then-refill pass left it + /// there with `used = false` — pool-visible as a FRESH address, handed + /// out again, and its stale `used = false` persisted back over the + /// store's `is_used = true` on the next pool snapshot. The fixpoint + /// re-marks after every refill until nothing new resolves. + #[test] + fn rehydration_wedge_zone_used_address_marked_after_refill() { + use key_wallet::bip32::DerivationPath; + use key_wallet::gap_limit::DEFAULT_EXTERNAL_GAP_LIMIT; + use key_wallet::managed_account::address_pool::{AddressPool, AddressPoolType, KeySource}; + use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; + use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; + use key_wallet::Address; + + let wallet = Wallet::from_seed_bytes( + [61u8; 64], + Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + let manifest = manifest_for(&wallet); + let mut wallet_info = ManagedWalletInfo::from_wallet(&wallet, 1); + + let funds_type = wallet_info + .accounts + .all_funding_accounts() + .first() + .unwrap() + .managed_account_type() + .to_account_type(); + let xpub = manifest + .iter() + .find(|e| e.account_type == funds_type) + .map(|e| e.account_xpub) + .expect("funds account xpub"); + + let derive = |index: u32| -> Address { + let mut p = AddressPool::new_without_generation( + DerivationPath::master(), + AddressPoolType::External, + DEFAULT_EXTERNAL_GAP_LIMIT, + Network::Testnet, + ); + p.generate_addresses(index + 1, &KeySource::Public(xpub), true) + .unwrap(); + p.address_at_index(index).unwrap() + }; + // Reachable multi-device state: this device saw idx 20 used; + // another device (same mnemonic) handed out and used idx 45. + let in_window_used = derive(20); + let wedge_used = derive(45); + + // No UTXOs at all — only the persisted pool used-state. + let core = crate::changeset::CoreChangeSet { + last_processed_height: Some(1), + synced_height: Some(1), + ..Default::default() + }; + apply_persisted_core_state( + &mut wallet_info, + &manifest, + &core, + &[in_window_used.clone(), wedge_used.clone()], + ) + .unwrap(); + + let funds = wallet_info + .accounts + .all_funding_accounts() + .into_iter() + .next() + .unwrap(); + let pools = funds.managed_account_type().address_pools(); + let external = pools.iter().find(|p| p.is_external()).unwrap(); + assert!( + external + .address_info(&in_window_used) + .expect("in-window used address present") + .used, + "in-window used address must be restored as used" + ); + let wedge_info = external + .address_info(&wedge_used) + .expect("wedge-zone address must be derived into the pool by the refill"); + assert!( + wedge_info.used, + "wedge-zone previously-used address must be re-marked used, \ + not left pool-visible as fresh" + ); + assert!(external.used_indices.contains(&45), "idx 45 recorded used"); + assert_eq!( + external.highest_used, + Some(45), + "highest_used must reflect the wedge-zone slot" + ); + // And the window is refilled past the re-marked slot. + assert!( + external.highest_generated >= Some(45 + DEFAULT_EXTERNAL_GAP_LIMIT), + "gap window must extend past the re-marked wedge slot (got {:?})", + external.highest_generated, + ); + } + /// Documented limitation (solution b): a legitimately-owned but /// deep-and-sparse UTXO — external idx 45 with nothing unspent at idx /// <= 30 — is left unresolved because the discovery horizon (gap_limit diff --git a/packages/rs-platform-wallet/tests/rehydration_load.rs b/packages/rs-platform-wallet/tests/rehydration_load.rs index 96a97fff48..37a3d18e8d 100644 --- a/packages/rs-platform-wallet/tests/rehydration_load.rs +++ b/packages/rs-platform-wallet/tests/rehydration_load.rs @@ -15,6 +15,10 @@ //! fires on the registered handler, `load` returns `Ok`. //! - RT-Z: no key/seed material in any `LoadOutcome` / `SkipReason` //! surface (the structural-only contract). +//! - RT-Snapshot: a carried `core_wallet_info` snapshot is consumed +//! verbatim — per-account UTXO attribution and derived-but-unused +//! deep pool addresses survive the reload; a snapshot whose +//! `wallet_id` mismatches its row is skipped as corrupt. use std::sync::{Arc, Mutex}; @@ -69,6 +73,7 @@ impl PlatformWalletPersistence for FixedLoadPersister { network: w.network, birth_height: w.birth_height, account_manifest: w.account_manifest.clone(), + core_wallet_info: w.core_wallet_info.clone(), core_state: w.core_state.clone(), identity_manager: Default::default(), unused_asset_locks: Default::default(), @@ -129,6 +134,7 @@ fn slice(seed: [u8; 64]) -> (WalletId, ClientWalletStartState) { network: key_wallet::Network::Testnet, birth_height: 1, account_manifest: manifest, + core_wallet_info: None, core_state: CoreChangeSet::default(), identity_manager: Default::default(), unused_asset_locks: Default::default(), @@ -280,6 +286,7 @@ async fn rt_corrupt_row_skipped_and_other_loads() { network: key_wallet::Network::Testnet, birth_height: 1, account_manifest: Vec::new(), + core_wallet_info: None, core_state: CoreChangeSet::default(), identity_manager: Default::default(), unused_asset_locks: Default::default(), @@ -347,6 +354,7 @@ async fn rt_z_secret_hygiene_surfaces() { network: key_wallet::Network::Testnet, birth_height: 1, account_manifest: Vec::new(), + core_wallet_info: None, core_state: CoreChangeSet::default(), identity_manager: Default::default(), unused_asset_locks: Default::default(), @@ -369,3 +377,211 @@ async fn rt_z_secret_hygiene_surfaces() { assert!(!rendered.to_lowercase().contains(&"ab".repeat(10))); } } + +/// RT-Snapshot: a carried `core_wallet_info` snapshot is consumed +/// verbatim. Two properties the projection replay could NOT provide: +/// - per-account UTXO attribution — a CoinJoin-account UTXO stays on the +/// CoinJoin account (the fallback path routed every UTXO to the first +/// funds account, zeroing non-first-account balances); +/// - derived-but-unused deep pool addresses (idx 40, past the eager gap +/// window) stay in the pool, so the SPV watch set still covers a +/// handed-out-but-unpaid receive address after restart. +#[tokio::test] +async fn rt_snapshot_preserves_attribution_and_pools() { + use key_wallet::account::AccountType; + use key_wallet::managed_account::address_pool::KeySource; + use key_wallet::managed_account::managed_account_trait::ManagedAccountTrait; + use key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; + use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; + + let seed = [0x66; 64]; + let wallet = Wallet::from_seed_bytes( + seed, + key_wallet::Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + let id = wallet.compute_wallet_id(); + let manifest: Vec = wallet + .accounts + .all_accounts() + .into_iter() + .map(|a| AccountRegistrationEntry { + account_type: a.account_type, + account_xpub: a.account_xpub, + }) + .collect(); + + let mut info = ManagedWalletInfo::from_wallet(&wallet, 1); + + // A UTXO on the CoinJoin account's own idx-0 address, inserted where + // the persisted rows put it: on the CoinJoin account. + let cj_value = 250_000u64; + let (cj_type, cj_addr) = { + let cj = info + .accounts + .all_funding_accounts() + .into_iter() + .find(|a| { + matches!( + a.managed_account_type().to_account_type(), + AccountType::CoinJoin { .. } + ) + }) + .expect("Default creation includes a CoinJoin account"); + let addr = cj + .managed_account_type() + .address_pools() + .first() + .expect("CoinJoin account has a pool") + .address_at_index(0) + .expect("eager window covers idx 0"); + (cj.managed_account_type().to_account_type(), addr) + }; + { + let cj = info + .accounts + .all_funding_accounts_mut() + .into_iter() + .find(|a| a.managed_account_type().to_account_type() == cj_type) + .unwrap(); + cj.utxos.insert( + dashcore::OutPoint { + txid: dashcore::Txid::from([0x42u8; 32]), + vout: 0, + }, + key_wallet::Utxo { + outpoint: dashcore::OutPoint { + txid: dashcore::Txid::from([0x42u8; 32]), + vout: 0, + }, + txout: dashcore::TxOut { + value: cj_value, + script_pubkey: cj_addr.script_pubkey(), + }, + address: cj_addr, + height: 1, + is_coinbase: false, + is_confirmed: true, + is_instantlocked: false, + is_locked: false, + is_trusted: false, + }, + ); + } + + // Extend the FIRST funds account's first pool to idx 40 — a + // derived-but-UNUSED deep address (handed out, not yet paid). + let (first_type, deep_keys_total) = { + let first = info + .accounts + .all_funding_accounts_mut() + .into_iter() + .next() + .expect("a first funds account exists"); + let first_type = first.managed_account_type().to_account_type(); + let xpub = manifest + .iter() + .find(|e| e.account_type == first_type) + .map(|e| e.account_xpub) + .expect("first funds account xpub in manifest"); + let pools = first.managed_account_type_mut().address_pools_mut(); + let pool = pools.into_iter().next().expect("first pool"); + let highest = pool.highest_generated.expect("eager window derived"); + assert!( + highest < 40, + "fixture: idx 40 must be past the eager window" + ); + pool.generate_addresses(40 - highest, &KeySource::Public(xpub), true) + .unwrap(); + assert!( + pool.address_at_index(40).is_some(), + "fixture: idx 40 derived" + ); + (first_type, pool.addresses.len() as u32) + }; + info.update_balance(); + + let (_, mut s) = slice(seed); + s.core_wallet_info = Some(Box::new(info)); + let p = Arc::new(FixedLoadPersister::new()); + let h = Arc::new(RecordingHandler::default()); + let mut st = ClientStartState::default(); + st.wallets.insert(id, s); + p.set(st); + + let mgr = manager(Arc::clone(&p), Arc::clone(&h)).await; + let outcome = mgr.load_from_persistor().await.expect("Ok"); + assert_eq!(outcome.loaded, vec![id]); + assert!(outcome.skipped.is_empty()); + + let rows = { + let mgr = Arc::clone(&mgr); + tokio::task::spawn_blocking(move || mgr.account_balances_blocking(&id)) + .await + .unwrap() + }; + let cj_row = rows + .iter() + .find(|r| r.account_type == cj_type) + .expect("CoinJoin account row"); + assert_eq!( + cj_row.balance.total(), + cj_value, + "CoinJoin UTXO must stay attributed to the CoinJoin account" + ); + let first_row = rows + .iter() + .find(|r| r.account_type == first_type) + .expect("first funds account row"); + assert!( + first_row.keys_total >= deep_keys_total, + "derived-but-unused deep addresses must survive the reload \ + (watch-set coverage): got {} keys, snapshot had {}", + first_row.keys_total, + deep_keys_total, + ); +} + +/// RT-Snapshot-Mismatch: a snapshot whose `wallet_id` does not match its +/// row key is a corrupt row — skipped with `DecodeError`, never +/// registered, and the batch continues. +#[tokio::test] +async fn rt_snapshot_wallet_id_mismatch_is_skipped() { + use key_wallet::wallet::managed_wallet_info::ManagedWalletInfo; + + let seed = [0x77; 64]; + let other_seed = [0x78; 64]; + let p = Arc::new(FixedLoadPersister::new()); + let h = Arc::new(RecordingHandler::default()); + + // Row keyed by wallet A, snapshot built from wallet B. + let (id_a, mut s) = slice(seed); + let wallet_b = Wallet::from_seed_bytes( + other_seed, + key_wallet::Network::Testnet, + WalletAccountCreationOptions::Default, + ) + .unwrap(); + s.core_wallet_info = Some(Box::new(ManagedWalletInfo::from_wallet(&wallet_b, 1))); + + let mut st = ClientStartState::default(); + st.wallets.insert(id_a, s); + p.set(st); + + let mgr = manager(Arc::clone(&p), Arc::clone(&h)).await; + let outcome = mgr.load_from_persistor().await.expect("Ok"); + + assert!(outcome.loaded.is_empty(), "mismatched row must not load"); + assert_eq!(outcome.skipped.len(), 1); + let (skipped_id, reason) = &outcome.skipped[0]; + assert_eq!(*skipped_id, id_a); + assert!(matches!( + reason, + SkipReason::CorruptPersistedRow { + kind: CorruptKind::DecodeError(_) + } + )); + assert!(mgr.get_wallet(&id_a).await.is_none()); + assert_eq!(h.skipped.lock().unwrap().len(), 1); +} diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift index 2ead77cdbd..08b77bb62d 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift @@ -122,6 +122,13 @@ public class PlatformWalletManager: ObservableObject { /// Last error from a wallet operation, if any. Cleared on successful op. @Published public private(set) var lastError: Error? + /// Wallets Rust skipped during the most recent [`loadFromPersistor`] + /// because their persisted row was structurally corrupt. Empty after + /// a clean load. Rust treats these as non-fatal — the load still + /// succeeds — so they are surfaced here rather than through + /// [`lastError`], letting UI offer to inspect / clear the bad rows. + @Published public private(set) var lastLoadSkippedWallets: [SkippedWalletOnLoad] = [] + // MARK: - Internals /// FFI handle; `NULL_HANDLE` until [`configure`] is called. @@ -319,6 +326,29 @@ public class PlatformWalletManager: ObservableObject { // MARK: - Watch-only restore from persister + /// One wallet Rust skipped during `load_from_persistor` because its + /// persisted row was structurally corrupt. `reasonCode` is one of the + /// Rust-side `LOAD_SKIP_REASON_*` constants (100 missing manifest, + /// 101 malformed xpub, 102 decode error, 199 other corrupt row, + /// 200 other skip); [`reasonDescription`] renders it for display. + public struct SkippedWalletOnLoad { + public let walletId: Data + public let reasonCode: UInt32 + + /// Human-readable rendering of `reasonCode`, mirroring the Rust + /// `LOAD_SKIP_REASON_*` constants. + public var reasonDescription: String { + switch reasonCode { + case 100: return "missing account manifest" + case 101: return "malformed account xpub" + case 102: return "decode error" + case 199: return "other corrupt row" + case 200: return "other skip" + default: return "unknown skip reason (\(reasonCode))" + } + } + } + /// Rehydrate wallets from SwiftData on app launch. /// /// Calls `platform_wallet_manager_load_from_persistor` which fires @@ -342,12 +372,40 @@ public class PlatformWalletManager: ObservableObject { public func loadFromPersistor() throws -> [ManagedPlatformWallet] { try ensureConfigured() - // Pass nil for `out_outcome` — Swift doesn't currently consume - // the per-wallet skip summary (corrupt persisted rows are - // logged by Rust at warn level). When Swift starts surfacing - // skipped wallets to the UI, pass a `LoadOutcomeFFI` here and - // free it with `platform_wallet_load_outcome_free`. - try platform_wallet_manager_load_from_persistor(handle, nil).check() + // Consume the load outcome so Rust's per-wallet skip summary + // isn't discarded. Rust writes `out_outcome` on every path + // (including early errors), so freeing it is safe even if the + // `.check()` below throws — defer the free before that throwing + // call. + var outcome = LoadOutcomeFFI(loaded_count: 0, skipped_count: 0, skipped: nil) + let loadResult = platform_wallet_manager_load_from_persistor(handle, &outcome) + defer { platform_wallet_load_outcome_free(&outcome) } + try loadResult.check() + + // Collect the ids Rust skipped as structurally corrupt. These + // are non-fatal on the Rust side, so they must not reach + // `lastError`: they are surfaced through `lastLoadSkippedWallets` + // and their ids are excluded from the per-id restore loop below + // (a skipped id is still in SwiftData, so `get_wallet` would + // return NotFound for it). + var skippedIds = Set() + var skippedWallets: [SkippedWalletOnLoad] = [] + if let skipped = outcome.skipped { + skippedWallets.reserveCapacity(Int(outcome.skipped_count)) + for i in 0.. Date: Thu, 2 Jul 2026 19:24:10 +0700 Subject: [PATCH 2/2] docs(platform-wallet): add normative persistence architecture to README Records the agreed model: platform-wallet is the single writer of wallet-state tables and the authority for state transitions; the persisted store is the durable history and the client's read model (display never blocks on load/unlock); the store schema is a versioned public contract; load() consumes the store verbatim (no re-derivation); persist errors are hard errors because local-only state (manifest, used-flags, metadata) is unrecoverable by rescan; load is seedless. Co-Authored-By: Claude Fable 5 --- packages/rs-platform-wallet/README.md | 93 +++++++++++++++++++++++++++ 1 file changed, 93 insertions(+) diff --git a/packages/rs-platform-wallet/README.md b/packages/rs-platform-wallet/README.md index d91bafa525..21b6c414ee 100644 --- a/packages/rs-platform-wallet/README.md +++ b/packages/rs-platform-wallet/README.md @@ -85,6 +85,99 @@ The package is structured as follows: - Active/inactive status - Note: Credit balance and revision are accessed from the Identity itself +## Persistence architecture + +This section is normative: it records the agreed model for how wallet +state, the persister, and clients relate. Changes that violate these +invariants need an explicit architecture discussion first, not just a +code review. + +``` + commands (send, register, sync, …) + client ──────────────────────────────────────▶ platform-wallet + │ │ + │ reads (display) changesets │ (single writer) + ▼ ▼ + ┌─────────────────────── persisted store ──────────────────────┐ + │ wallet-state tables: written ONLY by platform-wallet │ + │ client-owned tables (UI prefs etc.): written by client │ + └───────────────────────────────────────────────────────────────┘ + ▲ + │ load(persister) at launch — verbatim + platform-wallet +``` + +### Roles + +- **platform-wallet** is the authority for state *transitions*. Every + mutation of wallet state happens here and is emitted as a changeset + to the persister. Its in-memory state is volatile — a cache that is + empty at process start. +- **The persisted store** is the authority for state *history*: it is + the only copy of the wallet that survives a restart, and it doubles + as the client's **read model** — UIs render persisted rows directly + and reactively. Display therefore never blocks on platform-wallet + being loaded, unlocked, or synced. +- **Clients** (dash-evo-tool, the iOS SDK app, …) issue commands to + platform-wallet and read the store freely. They never write + wallet-state rows. + +### Invariants + +1. **Single writer.** Only platform-wallet's changesets mutate + wallet-state tables. Clients may keep their own tables (UI + preferences, view state) in the same database; ownership is per + table family, never shared. +2. **The store schema is a versioned public contract.** Two parties + depend on it — the persister's writes and every client's reads — so + schema changes are breaking changes for clients, not private + refactors. +3. **Reads never feed back into writes** except through platform-wallet + commands. A client that computes something from persisted rows and + wants it stored must go through a platform-wallet API. +4. **`load()` is verbatim.** At launch, platform-wallet reconstructs + itself from the store through + [`PlatformWalletPersistence::load`]; the store contains exactly what + platform-wallet wrote, so the load path must consume it as-is. + Re-deriving, re-inferring, or "repairing" state during load is + forbidden — a lossy round-trip here silently diverges the wallet + from its own history (per-account attribution, address-pool + `used` flags, and SPV watch-set coverage are the historical + casualties). Anything genuinely missing from the store re-warms on + the next sync, never inside `load()`. +5. **Persist errors are hard errors.** The store is the only durable + copy, and part of it — the account manifest, address used-flags, + birth heights, identity/contact associations — is *local-only*: no + chain rescan can ever reconstruct it. A swallowed persister write + error is silent, permanent data loss discovered at the next launch. +6. **Load is seedless.** The store never carries a seed or a + `Wallet`; restore produces watch-only wallets + (`Wallet::new_watch_only`) and signing keys are derived on demand + via the resolver-backed sign paths. See the trust-boundary notes on + [`PlatformWalletPersistence::load`] for what is (and is not) + authenticated on this path. + +### What restore is for + +Because the store is the read model, restoring platform-wallet at +launch is **not** about showing balances or history — the client +already renders those from the store. It exists to refill the +operational state that only lives in platform-wallet's memory: + +- **Detection** — the SPV watch set is the address-pool contents; + without it, incoming payments to existing addresses are not seen. +- **Spending** — coin/input selection runs against the in-memory UTXO + set. +- **Resume** — sync watermarks, tracked asset locks mid-registration, + and fresh-receive-address (`used`) state. + +Persisters that can reconstruct the full keyless snapshot hand it back +as `ClientWalletStartState::core_wallet_info` (consumed verbatim, per +invariant 4). The flattened projection fields +(`core_state`/`used_core_addresses`) are a transitional fallback for +persisters that cannot build a snapshot yet, and are slated for +removal once every in-tree persister produces snapshots. + ## Key Features ### Wallet Operations (via ManagedWalletInfo)