From 1086b4f9e94d2a1f4a180f0538d672afc9240b6f Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 10 Aug 2026 18:40:39 +0300 Subject: [PATCH 01/12] feat(platform-wallet): add the wallet bring-up status model MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit First piece of moving the DashPay startup ordering out of the clients. A contact's DIP-15 addresses come from its contact account, and an address that is not watched when the filter scan passes its funding height yields no transaction — so the bring-up order decides whether a restored wallet has contact payment history, and the client has to be able to hold Core SPV back until the addresses exist. That is a policy decision, so it belongs here rather than reimplemented per client. iOS had it in Swift; Android would have had to write it again. This commit is the classification alone — no I/O, no SDK — so the rule that already regressed once in a client is pinned by tests before the async body exists: a scan that came back definitively empty settles as `NoIdentity` and is never retried, while a scan that never reached Platform reports `PartialNoIdentity` and is. platform#4352 is what made those two distinguishable at all; this is the first consumer of that distinction. `StartupTally` mirrors `ScanTally` in discovery.rs deliberately: the counters live in a type the tests can drive through the same methods production uses, so a later miswiring fails a test instead of shipping. --- .../rs-platform-wallet/src/manager/mod.rs | 1 + .../rs-platform-wallet/src/manager/startup.rs | 309 ++++++++++++++++++ 2 files changed, 310 insertions(+) create mode 100644 packages/rs-platform-wallet/src/manager/startup.rs diff --git a/packages/rs-platform-wallet/src/manager/mod.rs b/packages/rs-platform-wallet/src/manager/mod.rs index fc4e47b15f..1e64401db2 100644 --- a/packages/rs-platform-wallet/src/manager/mod.rs +++ b/packages/rs-platform-wallet/src/manager/mod.rs @@ -8,6 +8,7 @@ mod load; pub mod platform_address_sync; #[cfg(feature = "shielded")] pub mod shielded_sync; +pub mod startup; mod wallet_lifecycle; use std::sync::Arc; diff --git a/packages/rs-platform-wallet/src/manager/startup.rs b/packages/rs-platform-wallet/src/manager/startup.rs new file mode 100644 index 0000000000..f6c61c5263 --- /dev/null +++ b/packages/rs-platform-wallet/src/manager/startup.rs @@ -0,0 +1,309 @@ +//! Ordered wallet bring-up: identity → contacts → contact accounts. +//! +//! A DashPay contact's DIP-15 payment addresses are derived from its contact +//! account, and an address the wallet is not watching when the compact-filter +//! scan passes its funding height produces no transaction at all. So the order +//! in which these come up decides whether a restored wallet has contact +//! payment history, and the client must be able to hold Core SPV back until +//! the addresses exist. +//! +//! That sequencing is a policy decision, which is why it lives here rather +//! than in each client. The iOS app had it in Swift first (a hand-rolled step +//! sequencer with its own budget and poll loops); Android would have had to +//! reproduce it, and the identity-retry half of it regressed once already +//! while it lived there. +//! +//! # The step that is easy to miss +//! +//! After a DashPay sync pass the contact accounts still do not exist. The +//! recurring sweep runs unattended and holds no signer, so it cannot derive +//! the receiving xpub or run the ECDH for a contact's external account — it +//! only *enqueues* the work (see `enqueue_deferred_contact_crypto` in +//! [`crate::wallet::identity::network::contact_requests`]). The accounts come +//! into being when a signer-present drain runs. A sequence that stopped after +//! the sync pass would be correctly ordered and still start SPV with nothing +//! extra to watch. +//! +//! # Key material +//! +//! Per-call, never resident. The caller resolves the master xpriv and the +//! contact-crypto provider for exactly this call and drops them after — the +//! same contract [`crate::wallet::identity::IdentityWallet::discover_from_master`] +//! and the drain already use. Making the unattended sweep self-sufficient +//! instead would turn a narrowly-scoped Keychain capability into a standing +//! one, which is a security-posture change and deliberately not what this is. + +use std::time::Duration; + +use dpp::prelude::Identifier; + +/// Why a bring-up stopped where it did. +/// +/// Every variant is a normal return. Budget exhaustion and an unreachable +/// Platform are outcomes the caller acts on, not errors to unwind — Core sync +/// is the wallet's primary function and must never be held hostage to +/// Platform being slow. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum WalletStartupStatus { + /// Identity resolved, the sync pass ran, and no contact-account builds are + /// left queued. Everything a contact payment needs is in place. + Ready, + /// Platform answered that this seed owns no identity. Terminal, and not a + /// failure: there is nothing to sync and nothing to drain. + NoIdentity, + /// The scan never reached Platform within the budget. The wallet may well + /// own an identity; we do not know yet. + PartialNoIdentity, + /// Identity resolved and synced, but contact-account builds are still + /// queued — the drain did not finish inside the budget. + PartialAccountsPending, +} + +impl WalletStartupStatus { + /// Whether Platform gave a definitive answer about this seed's identity. + /// + /// `false` only for [`Self::PartialNoIdentity`] — the one outcome worth + /// repeating. This is the distinction that platform#4352 made expressible: + /// before it, "no identity exists" and "we never got through" both arrived + /// as an empty success, so clients either retried a proven-empty scan + /// forever or cached a network failure as fact. + pub fn identity_is_settled(self) -> bool { + !matches!(self, Self::PartialNoIdentity) + } +} + +/// What a bring-up did, for the client to log and act on. +#[derive(Debug, Clone, PartialEq, Eq)] +pub struct WalletStartupOutcome { + pub status: WalletStartupStatus, + /// The wallet's identity, when one is known by the time this returns. + pub identity_id: Option, + /// Discovery scans performed. `0` when a local identity was already known + /// and no network scan was needed. + pub discovery_attempts: u32, + /// Whether the inline DashPay sync pass ran (skipped when there is no + /// identity to sync for). + pub dashpay_sync_ran: bool, + /// Contact-crypto entries completed by the drain. + pub contact_accounts_drained: usize, + /// Contact-account builds still queued when this returned. + pub contact_accounts_pending: usize, + pub elapsed: Duration, +} + +/// Running state of one bring-up, and the verdict it produces. +/// +/// Split out from the async body so the classification is testable without an +/// SDK, a wallet, or a network — the same reason [`ScanTally`] exists in +/// `discovery.rs`. The empty-vs-unreachable rule below is the one that has +/// already regressed once in a client implementation, so it is pinned by tests +/// rather than left inline. +/// +/// [`ScanTally`]: crate::wallet::identity::network::discovery +#[derive(Debug, Default)] +pub(crate) struct StartupTally { + /// Identity known locally or found by a scan. + pub identity_id: Option, + /// Platform definitively answered "this seed owns no identity". + pub proven_no_identity: bool, + /// The budget ran out while discovery was still unreachable. + pub discovery_unreachable: bool, + pub discovery_attempts: u32, + pub dashpay_sync_ran: bool, + pub contact_accounts_drained: usize, + pub contact_accounts_pending: usize, +} + +impl StartupTally { + /// A local identity was already on file — no scan needed. + pub(crate) fn record_local_identity(&mut self, identity_id: Identifier) { + self.identity_id = Some(identity_id); + } + + /// A scan reached Platform and found an identity. + pub(crate) fn record_discovered(&mut self, identity_id: Identifier) { + self.identity_id = Some(identity_id); + self.discovery_attempts += 1; + } + + /// A scan reached Platform and proved there is no identity for this seed. + /// + /// Terminal. Rescanning cannot change a proof of absence, and treating it + /// as retryable is exactly the bug this type exists to prevent: it costs a + /// wallet that legitimately owns no identity a full backoff schedule of + /// pointless round trips on every launch. + pub(crate) fn record_proven_absent(&mut self) { + self.proven_no_identity = true; + self.discovery_attempts += 1; + } + + /// A scan failed to reach Platform. Retryable while budget remains. + pub(crate) fn record_unreachable(&mut self) { + self.discovery_attempts += 1; + } + + /// The budget expired with discovery still unresolved. + pub(crate) fn record_discovery_gave_up(&mut self) { + self.discovery_unreachable = true; + } + + /// Whether there is an identity to sync and drain for. + pub(crate) fn has_identity(&self) -> bool { + self.identity_id.is_some() + } + + pub(crate) fn record_sync_ran(&mut self) { + self.dashpay_sync_ran = true; + } + + pub(crate) fn record_drain(&mut self, drained: usize, pending: usize) { + self.contact_accounts_drained = drained; + self.contact_accounts_pending = pending; + } + + /// Classify the run. + /// + /// Order matters: an unreachable Platform outranks everything, because + /// every later step was skipped or ran against incomplete state. A proven + /// absence outranks the drain counters for the same reason — with no + /// identity there is nothing to have drained. + pub(crate) fn status(&self) -> WalletStartupStatus { + if self.discovery_unreachable { + return WalletStartupStatus::PartialNoIdentity; + } + if self.proven_no_identity && self.identity_id.is_none() { + return WalletStartupStatus::NoIdentity; + } + if self.contact_accounts_pending > 0 { + return WalletStartupStatus::PartialAccountsPending; + } + WalletStartupStatus::Ready + } + + pub(crate) fn into_outcome(self, elapsed: Duration) -> WalletStartupOutcome { + WalletStartupOutcome { + status: self.status(), + identity_id: self.identity_id, + discovery_attempts: self.discovery_attempts, + dashpay_sync_ran: self.dashpay_sync_ran, + contact_accounts_drained: self.contact_accounts_drained, + contact_accounts_pending: self.contact_accounts_pending, + elapsed, + } + } +} + +#[cfg(test)] +mod tests { + use super::*; + + fn identity() -> Identifier { + Identifier::from([7u8; 32]) + } + + /// The regression this type exists to prevent, and the one that already + /// shipped once in a client: a scan that came back definitively empty must + /// settle as `NoIdentity`, never as something to retry. + #[test] + fn proven_absence_settles_and_is_not_retryable() { + let mut tally = StartupTally::default(); + tally.record_proven_absent(); + + assert_eq!(tally.status(), WalletStartupStatus::NoIdentity); + assert!( + tally.status().identity_is_settled(), + "a proof of absence is an answer; retrying it cannot change it" + ); + } + + /// The opposite case, and the reason the distinction is expressible at all + /// (platform#4352): never reaching Platform is not evidence of absence. + #[test] + fn unreachable_discovery_is_not_settled() { + let mut tally = StartupTally::default(); + tally.record_unreachable(); + tally.record_unreachable(); + tally.record_discovery_gave_up(); + + assert_eq!(tally.status(), WalletStartupStatus::PartialNoIdentity); + assert!(!tally.status().identity_is_settled()); + assert_eq!(tally.discovery_attempts, 2); + } + + /// An unreachable Platform outranks a clean drain: the later steps ran + /// against state we know to be incomplete. + #[test] + fn unreachable_discovery_outranks_a_finished_drain() { + let mut tally = StartupTally::default(); + tally.record_unreachable(); + tally.record_discovery_gave_up(); + tally.record_drain(0, 0); + + assert_eq!(tally.status(), WalletStartupStatus::PartialNoIdentity); + } + + #[test] + fn identity_found_and_drained_is_ready() { + let mut tally = StartupTally::default(); + tally.record_discovered(identity()); + tally.record_sync_ran(); + tally.record_drain(4, 0); + + assert_eq!(tally.status(), WalletStartupStatus::Ready); + assert_eq!(tally.discovery_attempts, 1); + } + + /// A warm launch: the identity was already on file, so no scan ran at all. + #[test] + fn local_identity_needs_no_discovery_attempt() { + let mut tally = StartupTally::default(); + tally.record_local_identity(identity()); + tally.record_sync_ran(); + tally.record_drain(0, 0); + + assert_eq!(tally.status(), WalletStartupStatus::Ready); + assert_eq!( + tally.discovery_attempts, 0, + "a known identity must not cost a network scan" + ); + } + + #[test] + fn queued_builds_report_as_pending() { + let mut tally = StartupTally::default(); + tally.record_discovered(identity()); + tally.record_sync_ran(); + tally.record_drain(2, 3); + + assert_eq!(tally.status(), WalletStartupStatus::PartialAccountsPending); + assert!( + tally.status().identity_is_settled(), + "the identity question is answered even though the drain is not done" + ); + } + + /// `has_identity` gates the sync and drain steps, so it must not be fooled + /// by a proven absence. + #[test] + fn proven_absence_has_no_identity_to_sync_for() { + let mut tally = StartupTally::default(); + tally.record_proven_absent(); + + assert!(!tally.has_identity()); + } + + #[test] + fn outcome_carries_the_tally_through() { + let mut tally = StartupTally::default(); + tally.record_discovered(identity()); + tally.record_sync_ran(); + tally.record_drain(1, 0); + + let outcome = tally.into_outcome(Duration::from_secs(3)); + assert_eq!(outcome.status, WalletStartupStatus::Ready); + assert_eq!(outcome.identity_id, Some(identity())); + assert!(outcome.dashpay_sync_ran); + assert_eq!(outcome.contact_accounts_drained, 1); + assert_eq!(outcome.elapsed, Duration::from_secs(3)); + } +} From c4694593a5b563decc1bcf1c00ea3c707a94d9b7 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 10 Aug 2026 18:46:46 +0300 Subject: [PATCH 02/12] feat(platform-wallet): sequence identity, contacts and contact accounts MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `start_wallet_subsystems` brings one wallet's DashPay state up in dependency order so the caller can start Core SPV against a complete contact-address set, instead of scanning first and repairing after. Three steps, each a precondition of the next: 1. Identity. Skipped entirely when one is already known locally, so a warm launch costs no network round trip. Otherwise a scan, retried ONLY on `IdentityDiscoveryIncomplete` — a scan that returned has an answer from Platform, and an empty answer is a proof of absence that rescanning cannot overturn. 2. One contact-request pass, so the deferred builds exist to drain. Log-and-continue: a prior session may have queued work this call can still finish. 3. The drain. This is the step whose absence made the ordering pointless elsewhere — after a sync pass the contact accounts still do not exist, because the unattended sweep holds no signer and can only enqueue. Without it SPV would start with nothing extra to watch. Never returns an error except `WalletNotFound`. An unreachable Platform, a failed sync and an unfinished drain are all reported in the outcome, because failing loudly would trade a data gap for a wallet with no balance — the worse of the two. The budget is a parameter defaulting to 20s. Key material stays per-call: the master xpriv and contact-crypto provider are borrowed for this call only, matching what `discover_from_master` and the drain already require. Making the recurring sweep resolve its own signer would remove the need for any of this, but it would turn a narrowly-scoped Keychain capability into a standing one, and the auto-accept half needs a full identity signer — a security-posture change, not a refactor. --- .../rs-platform-wallet/src/manager/startup.rs | 243 +++++++++++++++++- 1 file changed, 242 insertions(+), 1 deletion(-) diff --git a/packages/rs-platform-wallet/src/manager/startup.rs b/packages/rs-platform-wallet/src/manager/startup.rs index f6c61c5263..8cb8448226 100644 --- a/packages/rs-platform-wallet/src/manager/startup.rs +++ b/packages/rs-platform-wallet/src/manager/startup.rs @@ -33,9 +33,54 @@ //! instead would turn a narrowly-scoped Keychain capability into a standing //! one, which is a security-posture change and deliberately not what this is. -use std::time::Duration; +use std::time::{Duration, Instant}; +use dpp::identity::accessors::IdentityGettersV0; +use dpp::identity::signer::Signer; +use dpp::identity::IdentityPublicKey; use dpp::prelude::Identifier; +use key_wallet::bip32::ExtendedPrivKey; + +use crate::changeset::PlatformWalletPersistence; +use crate::error::PlatformWalletError; +use crate::manager::PlatformWalletManager; +use crate::wallet::identity::network::{ContactCryptoProvider, IdentityDiscoveryOptions}; +use crate::wallet::platform_wallet::WalletId; + +/// Whole-sequence budget when the caller does not name one. +/// +/// Core sync is the wallet's primary function and Platform is not: a Platform +/// outage must never be able to leave the user without a balance. Twenty +/// seconds is roughly the window in which a cold quorum endpoint either +/// answers or has already failed. +pub const DEFAULT_STARTUP_BUDGET: Duration = Duration::from_secs(20); + +/// Backoff between discovery attempts when Platform could not be reached. +/// +/// Retrying the scan is what helps here, not retrying inside DAPI: every probe +/// verifies its proof against quorum keys fetched from a single endpoint whose +/// cache is cold at process start, so a scan that begins before that endpoint +/// answers fails whole rather than per-node. A wallet that owns no identity +/// never reaches this schedule — a proof of absence settles on the first pass. +const DISCOVERY_BACKOFF: [Duration; 2] = [Duration::from_secs(3), Duration::from_secs(8)]; + +/// Knobs for [`PlatformWalletManager::start_wallet_subsystems`]. +#[derive(Debug, Clone, Copy)] +pub struct WalletStartupOptions { + /// Ceiling for the whole sequence. Expiry is reported, never an error. + pub budget: Duration, + /// Gap limit for identity discovery; `None` uses the crate default. + pub gap_limit: Option, +} + +impl Default for WalletStartupOptions { + fn default() -> Self { + Self { + budget: DEFAULT_STARTUP_BUDGET, + gap_limit: None, + } + } +} /// Why a bring-up stopped where it did. /// @@ -193,6 +238,202 @@ impl StartupTally { } } +impl PlatformWalletManager

{ + /// Bring one wallet's DashPay state up in dependency order, so the caller + /// can start Core SPV against a complete contact-address set. + /// + /// Runs identity discovery (only when no identity is known locally), then + /// one DashPay sync pass, then the signer-present drain that turns queued + /// contact-crypto into real accounts. See the module docs for why the + /// drain is not optional. + /// + /// # Errors + /// + /// Only [`PlatformWalletError::WalletNotFound`]. Every other outcome — + /// an unreachable Platform, a failed sync pass, a drain that did not + /// finish inside the budget — is reported in [`WalletStartupOutcome`], so + /// a client can start Core SPV regardless and let the DIP-15 rescan repair + /// whatever is missing. Failing loudly here would trade a data gap for a + /// wallet with no balance, which is the worse of the two. + /// + /// # Key material + /// + /// `master` and `contact_crypto` are borrowed for this call only and must + /// not be retained by the caller afterwards. `master` is `None` for a + /// wallet holding resident keys; `identity_signer` is `None` to skip the + /// DIP-15 auto-accept pass. + pub async fn start_wallet_subsystems( + &self, + wallet_id: &WalletId, + master: Option<&ExtendedPrivKey>, + contact_crypto: &C, + identity_signer: Option<&S>, + opts: WalletStartupOptions, + ) -> Result + where + C: ContactCryptoProvider + Sync, + S: Signer + Send + Sync, + { + let started = Instant::now(); + let deadline = started + opts.budget; + let mut tally = StartupTally::default(); + + let wallet = self + .get_wallet(wallet_id) + .await + .ok_or_else(|| PlatformWalletError::WalletNotFound(hex::encode(wallet_id)))?; + let identity_wallet = wallet.identity(); + + // 1. Local identities first. A warm launch must not pay for a network + // scan it does not need. + if let Some(known) = self.local_identity_id(wallet_id).await { + tally.record_local_identity(known); + } else { + self.discover_identity_with_backoff( + identity_wallet, + master, + opts.gap_limit, + deadline, + &mut tally, + ) + .await; + } + + // With no identity there is nothing to sync and nothing to drain, and + // that is true whether Platform proved absence or never answered. + if !tally.has_identity() { + return Ok(tally.into_outcome(started.elapsed())); + } + + // 2. One contact-request pass, so the deferred builds exist to drain. + // Log-and-continue: a prior session may already have queued work + // that this call can still complete. + if Instant::now() < deadline { + match identity_wallet.dashpay().sync_contact_requests().await { + Ok(requests) => { + tally.record_sync_ran(); + tracing::debug!( + wallet_id = %hex::encode(wallet_id), + requests = requests.len(), + "startup: contact-request pass complete" + ); + } + Err(e) => { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + error = %e, + "startup: contact-request pass failed; continuing to the drain" + ); + } + } + } + + // 3. The step that actually creates the addresses. + let drained = identity_wallet + .dashpay() + .drain_pending_contact_crypto(contact_crypto) + .await; + let accepted = match identity_signer { + Some(signer) => { + identity_wallet + .dashpay() + .drain_auto_accepts(signer, contact_crypto) + .await + } + None => 0, + }; + let pending = identity_wallet + .dashpay() + .pending_contact_crypto_count() + .await; + tally.record_drain(drained + accepted, pending); + + if pending > 0 { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + pending, + "startup: contact-account builds still queued; the DIP-15 rescan will backfill" + ); + } + + Ok(tally.into_outcome(started.elapsed())) + } + + /// The first identity this wallet already owns locally, if any. + async fn local_identity_id(&self, wallet_id: &WalletId) -> Option { + let wm = self.wallet_manager.read().await; + let info = wm.get_wallet_info(wallet_id)?; + info.identity_manager + .wallet_identity_ids(wallet_id) + .into_iter() + .next() + } + + /// Scan for an identity, retrying only while Platform stays unreachable. + /// + /// An `Ok` result ends the loop whether or not it found anything: Platform + /// answered, and an empty answer is a proof of absence that rescanning + /// cannot overturn. Only [`PlatformWalletError::IdentityDiscoveryIncomplete`] + /// — the error platform#4352 introduced for a scan that never got through + /// — is worth another attempt. + async fn discover_identity_with_backoff( + &self, + identity_wallet: &crate::wallet::identity::IdentityWallet, + master: Option<&ExtendedPrivKey>, + gap_limit: Option, + deadline: Instant, + tally: &mut StartupTally, + ) { + let opts = IdentityDiscoveryOptions { + start_index: Some(0), + gap_limit: gap_limit.unwrap_or(IdentityDiscoveryOptions::default().gap_limit), + }; + + for (attempt, backoff) in DISCOVERY_BACKOFF.iter().map(Some).chain([None]).enumerate() { + let result = match master { + Some(master) => identity_wallet.discover_from_master(opts, master).await, + None => identity_wallet.discover(opts).await, + }; + + match result { + Ok(found) => { + match found.first() { + Some(identity) => tally.record_discovered(identity.id()), + None => tally.record_proven_absent(), + } + return; + } + Err(PlatformWalletError::IdentityDiscoveryIncomplete { .. }) => { + tally.record_unreachable(); + } + Err(e) => { + // Not a reachability question — a wallet/persistence + // failure will not fix itself on the next attempt. + tracing::warn!( + error = %e, + "startup: identity discovery failed for a non-network reason" + ); + tally.record_discovery_gave_up(); + return; + } + } + + let Some(backoff) = backoff else { break }; + let remaining = deadline.saturating_duration_since(Instant::now()); + if remaining.is_zero() { + break; + } + tracing::info!( + attempt = attempt + 1, + "startup: identity discovery could not reach Platform; retrying" + ); + tokio::time::sleep((*backoff).min(remaining)).await; + } + + tally.record_discovery_gave_up(); + } +} + #[cfg(test)] mod tests { use super::*; From 6ae473065bc1b99b3dd5d0081a7a8f11aa344181 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 10 Aug 2026 18:55:53 +0300 Subject: [PATCH 03/12] feat(platform-wallet-ffi): expose the ordered wallet bring-up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit One call a host makes at wallet load, immediately before starting Core SPV, replacing the step sequencer each client would otherwise hand-roll. `run_on_big_stack_thread` rather than `block_on_worker`: the manager is only reachable as a `&PlatformWalletManager` borrowed from the handle store, so the future cannot satisfy `block_on_worker`'s `'static` bound. The scoped thread also supplies the 8 MB stack the GroveDB proof verification inside discovery needs — the same reason discovery's own FFI avoids the calling thread's stack. The master xpriv is resolved once up front and erased with `non_secure_erase` before every return path; `ExtendedPrivKey` has no `Drop`, so this is the same explicit hygiene `platform_wallet_discover_identities` already performs. Both handles follow the established per-call contract — borrowed for the duration, never retained — and the identity signer stays nullable, where null skips the DIP-15 auto-accept pass exactly as it does for `platform_wallet_drain_pending_contact_crypto`. `budget_secs = 0` means the crate default, deliberately not "unbounded": this call gates Core SPV, so it must always terminate. Only an invalid handle or unknown wallet id return an error — every partial outcome is reported in the out-struct so the host can start Core SPV regardless. Adds `wallet_network_blocking` on the manager: the network is needed to build the key material before the sequence can run, and a caller holding a manager handle has no wallet handle to read it from. --- packages/rs-platform-wallet-ffi/src/lib.rs | 1 + .../src/wallet_startup.rs | 213 ++++++++++++++++++ .../src/manager/accessors.rs | 13 ++ 3 files changed, 227 insertions(+) create mode 100644 packages/rs-platform-wallet-ffi/src/wallet_startup.rs diff --git a/packages/rs-platform-wallet-ffi/src/lib.rs b/packages/rs-platform-wallet-ffi/src/lib.rs index a6df9e830d..19aa69700c 100644 --- a/packages/rs-platform-wallet-ffi/src/lib.rs +++ b/packages/rs-platform-wallet-ffi/src/lib.rs @@ -82,6 +82,7 @@ pub mod utils; pub mod wallet; pub mod wallet_registration_persistence; pub mod wallet_restore_types; +pub mod wallet_startup; pub mod xpub_render; // Re-exports diff --git a/packages/rs-platform-wallet-ffi/src/wallet_startup.rs b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs new file mode 100644 index 0000000000..d24cc8aee0 --- /dev/null +++ b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs @@ -0,0 +1,213 @@ +//! FFI for the ordered wallet bring-up. +//! +//! One call that runs identity discovery → contact-request sync → the +//! signer-present contact-crypto drain, so the host can start Core SPV knowing +//! the DIP-15 contact addresses exist and will be in the first filter set. The +//! ordering policy itself lives in +//! [`platform_wallet::manager::startup`]; this is the marshalling shell. + +use platform_wallet::manager::startup::{ + WalletStartupOptions, WalletStartupOutcome, WalletStartupStatus, +}; +use rs_sdk_ffi::{MnemonicResolverHandle, SignerHandle, VTableSigner}; +use std::time::Duration; + +use crate::check_ptr; +use crate::dashpay::resolver_contact_crypto_provider; +use crate::error::{PlatformWalletFFIResult, PlatformWalletFFIResultCode}; +use crate::handle::{Handle, PLATFORM_WALLET_MANAGER_STORAGE}; +use crate::identity_keys_from_mnemonic::resolve_master_from_resolver; +use crate::runtime::run_on_big_stack_thread; + +/// Discriminants for [`WalletStartupStatus`] across the boundary. +/// +/// Values are part of the ABI — append, never renumber. +#[repr(u8)] +pub enum WalletStartupStatusFFI { + Ready = 0, + NoIdentity = 1, + PartialNoIdentity = 2, + PartialAccountsPending = 3, +} + +impl From for WalletStartupStatusFFI { + fn from(status: WalletStartupStatus) -> Self { + match status { + WalletStartupStatus::Ready => Self::Ready, + WalletStartupStatus::NoIdentity => Self::NoIdentity, + WalletStartupStatus::PartialNoIdentity => Self::PartialNoIdentity, + WalletStartupStatus::PartialAccountsPending => Self::PartialAccountsPending, + } + } +} + +/// Flat result of [`platform_wallet_manager_start_wallet_subsystems`]. +#[repr(C)] +pub struct WalletStartupOutcomeFFI { + /// A [`WalletStartupStatusFFI`] discriminant. + pub status: u8, + /// Whether `identity_id` carries a value. + pub has_identity_id: bool, + /// The wallet's identity, valid only when `has_identity_id`. + pub identity_id: [u8; 32], + /// Discovery scans performed; `0` when a local identity was already known. + pub discovery_attempts: u32, + /// Whether the inline contact-request pass ran. + pub dashpay_sync_ran: bool, + /// Contact-crypto entries the drain completed. + pub contact_accounts_drained: u32, + /// Contact-account builds still queued on return. + pub contact_accounts_pending: u32, + /// Wall-clock duration of the whole sequence. + pub elapsed_ms: u64, +} + +impl From for WalletStartupOutcomeFFI { + fn from(outcome: WalletStartupOutcome) -> Self { + let (has_identity_id, identity_id) = match outcome.identity_id { + Some(id) => (true, id.to_buffer()), + None => (false, [0u8; 32]), + }; + Self { + status: WalletStartupStatusFFI::from(outcome.status) as u8, + has_identity_id, + identity_id, + discovery_attempts: outcome.discovery_attempts, + dashpay_sync_ran: outcome.dashpay_sync_ran, + contact_accounts_drained: outcome.contact_accounts_drained as u32, + contact_accounts_pending: outcome.contact_accounts_pending as u32, + elapsed_ms: outcome.elapsed.as_millis() as u64, + } + } +} + +/// Bring one wallet's DashPay state up in dependency order. +/// +/// Blocks until the sequence finishes or its budget expires, then writes the +/// outcome to `out_outcome`. Intended to be called once per wallet load, +/// immediately before starting Core SPV. +/// +/// Only an invalid manager handle or an unknown `wallet_id` produce an error; +/// an unreachable Platform, a failed sync pass and an unfinished drain are all +/// reported through `out_outcome.status`, because a host must be able to start +/// Core SPV regardless. See [`WalletStartupStatusFFI`]. +/// +/// # Arguments +/// +/// * `wallet_id` — 32 bytes. +/// * `mnemonic_resolver_handle` — nullable. Required for a Keychain-backed +/// external-signable wallet, whose seed does not live in the wallet manager; +/// null means the wallet holds resident keys and derives in-process. +/// * `identity_signer_handle` — nullable; null skips the DIP-15 auto-accept +/// pass, matching `platform_wallet_drain_pending_contact_crypto`. +/// * `budget_secs` — `0` means the crate default (20s). Deliberately not +/// "unbounded": this call gates Core SPV, so it must always terminate. +/// * `gap_limit` — `0` means the crate default. +/// +/// # Safety +/// +/// `wallet_id` must point to 32 readable bytes and `out_outcome` to a writable +/// [`WalletStartupOutcomeFFI`]. Both handles, when non-null, must be valid for +/// the duration of this call — the callee borrows and never retains them. +#[no_mangle] +pub unsafe extern "C" fn platform_wallet_manager_start_wallet_subsystems( + manager_handle: Handle, + wallet_id: *const u8, + mnemonic_resolver_handle: *mut MnemonicResolverHandle, + identity_signer_handle: *mut SignerHandle, + budget_secs: u64, + gap_limit: u32, + out_outcome: *mut WalletStartupOutcomeFFI, +) -> PlatformWalletFFIResult { + check_ptr!(wallet_id); + check_ptr!(out_outcome); + + let wid: [u8; 32] = std::ptr::read(wallet_id as *const [u8; 32]); + + // The network is needed before any key material can be built, and the + // caller gave us a manager handle rather than a wallet handle. + let Some(network_opt) = PLATFORM_WALLET_MANAGER_STORAGE + .with_item(manager_handle, |m| m.wallet_network_blocking(&wid)) + else { + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorInvalidHandle, + "Manager handle invalid".to_string(), + ); + }; + let Some(network) = network_opt else { + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::NotFound, + "Wallet not found".to_string(), + ); + }; + + // Resolve the master xpriv once, up front. The helper holds the mnemonic + // and seed in `Zeroizing` buffers and scrubs them before returning; the + // master itself has no `Drop`, so it is erased explicitly below. + let mut master = if mnemonic_resolver_handle.is_null() { + None + } else { + match resolve_master_from_resolver(mnemonic_resolver_handle, &wid, network) { + Ok(master) => Some(master), + Err(e) => return e, + } + }; + + let signer_addr = if identity_signer_handle.is_null() { + 0usize + } else { + identity_signer_handle as usize + }; + let provider = resolver_contact_crypto_provider(mnemonic_resolver_handle, wid, network); + + let opts = WalletStartupOptions { + budget: if budget_secs == 0 { + WalletStartupOptions::default().budget + } else { + Duration::from_secs(budget_secs) + }, + gap_limit: (gap_limit > 0).then_some(gap_limit), + }; + + // `run_on_big_stack_thread`, not `block_on_worker`: the manager is only + // reachable as a `&PlatformWalletManager` borrowed from the handle store, + // so the future cannot satisfy `block_on_worker`'s `'static` bound. The + // scoped thread also supplies the 8 MB stack the GroveDB proof + // verification inside discovery needs. + let result = run_on_big_stack_thread(|| { + PLATFORM_WALLET_MANAGER_STORAGE.with_item(manager_handle, |manager| { + crate::runtime::runtime().block_on(async { + let signer = + (signer_addr != 0).then(|| unsafe { &*(signer_addr as *const VTableSigner) }); + manager + .start_wallet_subsystems(&wid, master.as_ref(), &provider, signer, opts) + .await + }) + }) + }); + + // Erase the master before any early return below. + if let Some(master) = master.as_mut() { + master.private_key.non_secure_erase(); + } + + let outcome = match result { + Ok(Some(Ok(outcome))) => outcome, + Ok(Some(Err(e))) => return PlatformWalletFFIResult::from(e), + Ok(None) => { + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorInvalidHandle, + "Manager handle invalid".to_string(), + ) + } + Err(e) => { + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorWalletOperation, + format!("failed to spawn the startup thread: {e}"), + ) + } + }; + + *out_outcome = WalletStartupOutcomeFFI::from(outcome); + PlatformWalletFFIResult::ok() +} diff --git a/packages/rs-platform-wallet/src/manager/accessors.rs b/packages/rs-platform-wallet/src/manager/accessors.rs index 4905ba3b37..7b9c642282 100644 --- a/packages/rs-platform-wallet/src/manager/accessors.rs +++ b/packages/rs-platform-wallet/src/manager/accessors.rs @@ -445,6 +445,19 @@ impl PlatformWalletManager

{ wallets.keys().copied().collect() } + /// Network a registered wallet belongs to, or `None` when the id is + /// unknown. + /// + /// Exists for FFI callers that hold a manager handle and a wallet id but + /// no wallet handle, and need the network before they can build the + /// per-call key material a wallet operation requires (resolving a master + /// xpriv, constructing a contact-crypto provider). Blocking and cheap: one + /// `RwLock` read, no I/O. + pub fn wallet_network_blocking(&self, wallet_id: &WalletId) -> Option { + let wm = self.wallet_manager.blocking_read(); + Some(wm.get_wallet_info(wallet_id)?.core_wallet.network()) + } + /// Snapshot of [`PlatformAddressSyncManager`] tunables and last- /// pass timestamp. `watch_list_size` is `wallets.len()` — every /// registered wallet participates in each pass since the sync From 151d20253345faa6bec7c6aaf0e67bfe54b6656b Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 10 Aug 2026 19:00:50 +0300 Subject: [PATCH 04/12] feat(swift-sdk): wrap the ordered wallet bring-up MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `startWalletSubsystems(wallet:budget:gapLimit:)` — one awaitable call a host makes before `startSpv`, replacing the step sequencer each client would otherwise hand-roll. The ordering, the retry policy and the budget all stay Rust-side; this marshals arguments and maps the outcome. `WalletStartupStatus` carries `identityIsSettled`, false only for `partialNoIdentity`, so a caller never has to re-derive which outcomes are worth repeating. An unrecognised discriminant decodes to that same case: the conservative reading, since it is the one status that does not claim the identity question is answered. The resolver and identity signer are built per call and pinned with `withExtendedLifetime` — Rust borrows and never retains them, the same contract `unlockWalletFromKeychain` and `discoverIdentities` use. `ensureConfigured`, `modelContainer` and `signerNetwork` go from private to internal so this extension reuses them instead of duplicating the gate and the signer construction in a second file. --- .../PlatformWalletManager.swift | 11 +- .../PlatformWalletManagerStartup.swift | 158 ++++++++++++++++++ 2 files changed, 166 insertions(+), 3 deletions(-) create mode 100644 packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift index fa9b787c1a..90a2a55581 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManager.swift @@ -250,8 +250,11 @@ public class PlatformWalletManager: ObservableObject { /// `KeychainSigner` (the identity document signer) for the unlock-time /// auto-accept drain. Nil when configured without persistence (no Keychain /// signing possible → the drain runs provider-only). - private var modelContainer: ModelContainer? - private var signerNetwork: Network? + /// `internal`, not `private`: the ordered bring-up in + /// `PlatformWalletManagerStartup` builds the same signer for the same + /// drain, and reusing these beats duplicating the construction there. + var modelContainer: ModelContainer? + var signerNetwork: Network? /// Convenience reference; the FFI callback context's lifetime is /// owned by Rust (retained reference transferred at `configure`, @@ -1342,7 +1345,9 @@ public class PlatformWalletManager: ObservableObject { // MARK: - Internals - private func ensureConfigured() throws { + /// `internal` so the extensions in sibling files gate on the same check + /// rather than re-implementing it. + func ensureConfigured() throws { if !isConfigured || handle == NULL_HANDLE { throw PlatformWalletError.invalidHandle( "PlatformWalletManager not configured" diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift new file mode 100644 index 0000000000..2ccbe96662 --- /dev/null +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift @@ -0,0 +1,158 @@ +// +// PlatformWalletManagerStartup.swift +// SwiftDashSDK +// +// Ordered wallet bring-up: identity → contacts → contact accounts, run as one +// bounded call before the host starts Core SPV. +// + +import Foundation + +/// Why a bring-up stopped where it did. +/// +/// Every case is a normal result. A host starts Core SPV on all of them — the +/// partial cases just mean the DIP-15 rescan has work left to do. +public enum WalletStartupStatus: UInt8, Sendable { + /// Identity resolved, contacts synced, no contact-account builds left + /// queued. Everything a contact payment needs is in place. + case ready = 0 + /// Platform answered that this seed owns no identity. Terminal, and not a + /// failure — there is nothing to sync and nothing to drain. + case noIdentity = 1 + /// The identity scan never reached Platform inside the budget. The wallet + /// may well own one; we do not know yet. + case partialNoIdentity = 2 + /// Identity resolved and synced, but contact-account builds are still + /// queued — the drain did not finish inside the budget. + case partialAccountsPending = 3 + + /// Whether Platform gave a definitive answer about this seed's identity. + /// + /// `false` only for ``partialNoIdentity``, the one outcome worth repeating. + public var identityIsSettled: Bool { self != .partialNoIdentity } +} + +/// What a bring-up did. +public struct WalletStartupOutcome: Sendable, Equatable { + public let status: WalletStartupStatus + /// The wallet's identity, when one is known by the time this returned. + public let identityId: Data? + /// Discovery scans performed. `0` when a local identity was already known + /// and no network scan was needed. + public let discoveryAttempts: UInt32 + /// Whether the inline contact-request pass ran. + public let dashPaySyncRan: Bool + /// Contact-crypto entries the drain completed. + public let contactAccountsDrained: UInt32 + /// Contact-account builds still queued on return. Non-zero means the + /// DIP-15 rescan will have to backfill those contacts' payments. + public let contactAccountsPending: UInt32 + public let elapsed: TimeInterval +} + +extension PlatformWalletManager { + /// Bring one wallet's DashPay state up in dependency order, then return so + /// the caller can start Core SPV. + /// + /// A contact's DIP-15 payment addresses are derived from its contact + /// account, and an address the wallet is not watching when the + /// compact-filter scan passes its funding height produces no transaction at + /// all. This runs identity discovery, one contact-request pass, and the + /// signer-present drain that turns queued contact-crypto into real accounts + /// — so the first filter set already covers them. + /// + /// The ordering, the retry policy and the budget all live Rust-side; this + /// is a thin bridge. Call it once per wallet load, immediately before + /// `startSpv`. + /// + /// - Parameters: + /// - wallet: the wallet to bring up. + /// - budget: ceiling for the whole sequence. `nil` uses the SDK default + /// (20s). Expiry is reported in the outcome, never thrown: Core sync is + /// the wallet's primary function and must not be held hostage to + /// Platform being slow. + /// - gapLimit: identity-discovery gap limit; `nil` uses the default. + /// - storage: `WalletStorage` used by the resolver callback to read the + /// BIP-39 mnemonic from the Keychain. Overridable for tests. + /// + /// - Returns: what the sequence achieved. Start Core SPV regardless of the + /// status; inspect `contactAccountsPending` for diagnostics. + /// + /// - Throws: only for an unconfigured manager, a malformed wallet id, or an + /// unknown wallet. An unreachable Platform, a failed sync pass and an + /// unfinished drain are all reported through the returned status. + /// + /// # Key material + /// + /// The mnemonic resolver and identity signer are built for this call and + /// pinned across it with `withExtendedLifetime`; Rust borrows and never + /// retains them. That per-call contract is deliberate — a standing + /// Keychain capability driven by an unattended loop would be a different + /// security posture, not a convenience. + @discardableResult + public func startWalletSubsystems( + wallet: ManagedPlatformWallet, + budget: TimeInterval? = nil, + gapLimit: UInt32? = nil, + storage: WalletStorage = WalletStorage() + ) async throws -> WalletStartupOutcome { + try ensureConfigured() + + let walletId = wallet.walletId + guard walletId.count == 32 else { + throw PlatformWalletError.invalidParameter( + "walletId must be 32 bytes, got \(walletId.count)" + ) + } + + let handle = self.handle + // A genuine watch-only wallet has no Keychain mnemonic; Rust then + // derives in-process and the resolver is never consulted. + let coreSigner: MnemonicResolver? = + storage.hasMnemonic(for: walletId) ? MnemonicResolver(storage: storage) : nil + // Identity signer for the DIP-15 auto-accept pass. Nil without a + // SwiftData container → the drain runs provider-only, matching + // `unlockWalletFromKeychain`. + let identitySigner: KeychainSigner? = self.modelContainer.map { + KeychainSigner(modelContainer: $0, network: self.signerNetwork ?? .testnet) + } + let budgetSecs = UInt64((budget ?? 0).rounded()) + + return try await Task.detached(priority: .userInitiated) { () -> WalletStartupOutcome in + try withExtendedLifetime((coreSigner, identitySigner)) { + var out = WalletStartupOutcomeFFI() + try walletId.withUnsafeBytes { raw in + try platform_wallet_manager_start_wallet_subsystems( + handle, + raw.bindMemory(to: UInt8.self).baseAddress, + coreSigner?.handle, + identitySigner?.handle, + budgetSecs, + gapLimit ?? 0, + &out + ).check() + } + return WalletStartupOutcome(out) + } + }.value + } +} + +extension WalletStartupOutcome { + /// Map the flat FFI struct. An unrecognised status discriminant is read as + /// ``WalletStartupStatus/partialNoIdentity`` — the conservative choice, + /// since it is the one status that does not claim the identity question is + /// settled. + init(_ ffi: WalletStartupOutcomeFFI) { + self.status = WalletStartupStatus(rawValue: ffi.status) ?? .partialNoIdentity + self.identityId = + ffi.has_identity_id + ? Swift.withUnsafeBytes(of: ffi.identity_id) { Data($0) } + : nil + self.discoveryAttempts = ffi.discovery_attempts + self.dashPaySyncRan = ffi.dashpay_sync_ran + self.contactAccountsDrained = ffi.contact_accounts_drained + self.contactAccountsPending = ffi.contact_accounts_pending + self.elapsed = TimeInterval(ffi.elapsed_ms) / 1000 + } +} From d402a186c3149b6fcc352bb6db963562dadfe41e Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 10 Aug 2026 20:49:01 +0300 Subject: [PATCH 05/12] fix(platform-wallet): bound every network step, and skip a drain with no provider MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three findings from review on #4359. **The budget bounded only discovery.** `sync_contact_requests` and both drains ran without a deadline, so a stalled DAPI endpoint could hold the call — and therefore Core SPV, which this gates — well past `budget`, the exact outcome the module docs promise to avoid. Every network step now runs through `within_budget`. Abandoning one is safe by construction: work a drain did not finish stays queued and the next signer-present action retries it. The queue-length read stays unbounded on purpose — it is local, and its answer matters most precisely when the steps above ran out of time. **A resident-key wallet drained into a provider that could not work.** The provider is resolver-backed, so with a null resolver every crypto operation fails with `NullHandle`: the drain reported zero while leaving the queue untouched, which reads as "nothing to do" rather than "could not try". `contact_crypto` is now `Option`, the FFI passes `None` when there is no resolver, and the sequence skips the drain and reports the real pending count. **The Swift budget conversion could trap.** `UInt64(x.rounded())` traps on a negative, NaN or infinite value, reachable whenever the budget comes out of arithmetic — now rejected with a typed error. A positive budget under half a second also rounded to `0`, which the FFI reads as "use the default", handing the caller the longest budget where they asked for the shortest; clamped to one second instead. Also corrects the `PartialAccountsPending` doc, which claimed the budget had expired. A non-zero pending count no longer implies that — the drain may have failed on some entries, or been skipped for want of a provider. Three `within_budget` tests on a paused clock cover the gap that let the first finding through. --- .../src/wallet_startup.rs | 9 +- .../rs-platform-wallet/src/manager/startup.rs | 141 ++++++++++++++---- .../PlatformWalletManagerStartup.swift | 17 ++- 3 files changed, 135 insertions(+), 32 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/wallet_startup.rs b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs index d24cc8aee0..3b90d80c72 100644 --- a/packages/rs-platform-wallet-ffi/src/wallet_startup.rs +++ b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs @@ -158,7 +158,12 @@ pub unsafe extern "C" fn platform_wallet_manager_start_wallet_subsystems( } else { identity_signer_handle as usize }; - let provider = resolver_contact_crypto_provider(mnemonic_resolver_handle, wid, network); + // The provider is resolver-backed, so without a resolver every crypto + // operation would fail with `NullHandle` and the drain would report zero + // while leaving the queue untouched. Pass `None` instead: the sequence then + // skips the drain and reports the real pending count. + let provider = (!mnemonic_resolver_handle.is_null()) + .then(|| resolver_contact_crypto_provider(mnemonic_resolver_handle, wid, network)); let opts = WalletStartupOptions { budget: if budget_secs == 0 { @@ -180,7 +185,7 @@ pub unsafe extern "C" fn platform_wallet_manager_start_wallet_subsystems( let signer = (signer_addr != 0).then(|| unsafe { &*(signer_addr as *const VTableSigner) }); manager - .start_wallet_subsystems(&wid, master.as_ref(), &provider, signer, opts) + .start_wallet_subsystems(&wid, master.as_ref(), provider.as_ref(), signer, opts) .await }) }) diff --git a/packages/rs-platform-wallet/src/manager/startup.rs b/packages/rs-platform-wallet/src/manager/startup.rs index 8cb8448226..abf0b84e74 100644 --- a/packages/rs-platform-wallet/src/manager/startup.rs +++ b/packages/rs-platform-wallet/src/manager/startup.rs @@ -64,6 +64,20 @@ pub const DEFAULT_STARTUP_BUDGET: Duration = Duration::from_secs(20); /// never reaches this schedule — a proof of absence settles on the first pass. const DISCOVERY_BACKOFF: [Duration; 2] = [Duration::from_secs(3), Duration::from_secs(8)]; +/// Run `future` with whatever is left of the budget, or not at all. +/// +/// `None` means the deadline passed — either before the step started or while +/// it ran. Every step in the sequence is abandonable: the work it did not +/// finish stays queued for the next attempt, and the caller needs Core SPV to +/// start far more than it needs any one of them to complete. +async fn within_budget(deadline: Instant, future: F) -> Option { + let remaining = deadline.saturating_duration_since(Instant::now()); + if remaining.is_zero() { + return None; + } + tokio::time::timeout(remaining, future).await.ok() +} + /// Knobs for [`PlatformWalletManager::start_wallet_subsystems`]. #[derive(Debug, Clone, Copy)] pub struct WalletStartupOptions { @@ -100,7 +114,10 @@ pub enum WalletStartupStatus { /// own an identity; we do not know yet. PartialNoIdentity, /// Identity resolved and synced, but contact-account builds are still - /// queued — the drain did not finish inside the budget. + /// queued. The budget may have run out, the drain may have failed on some + /// entries, or no contact-crypto provider was supplied — the count is what + /// is certain, not the reason. Either way those contacts' payments wait on + /// the DIP-15 rescan. PartialAccountsPending, } @@ -266,7 +283,7 @@ impl PlatformWalletManager &self, wallet_id: &WalletId, master: Option<&ExtendedPrivKey>, - contact_crypto: &C, + contact_crypto: Option<&C>, identity_signer: Option<&S>, opts: WalletStartupOptions, ) -> Result @@ -308,40 +325,74 @@ impl PlatformWalletManager // 2. One contact-request pass, so the deferred builds exist to drain. // Log-and-continue: a prior session may already have queued work // that this call can still complete. - if Instant::now() < deadline { - match identity_wallet.dashpay().sync_contact_requests().await { - Ok(requests) => { - tally.record_sync_ran(); - tracing::debug!( - wallet_id = %hex::encode(wallet_id), - requests = requests.len(), - "startup: contact-request pass complete" - ); - } - Err(e) => { - tracing::warn!( - wallet_id = %hex::encode(wallet_id), - error = %e, - "startup: contact-request pass failed; continuing to the drain" - ); - } + match within_budget(deadline, identity_wallet.dashpay().sync_contact_requests()).await { + Some(Ok(requests)) => { + tally.record_sync_ran(); + tracing::debug!( + wallet_id = %hex::encode(wallet_id), + requests = requests.len(), + "startup: contact-request pass complete" + ); + } + Some(Err(e)) => { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + error = %e, + "startup: contact-request pass failed; continuing to the drain" + ); + } + None => { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + "startup: budget spent before the contact-request pass finished" + ); } } // 3. The step that actually creates the addresses. - let drained = identity_wallet - .dashpay() - .drain_pending_contact_crypto(contact_crypto) - .await; - let accepted = match identity_signer { - Some(signer) => { - identity_wallet - .dashpay() - .drain_auto_accepts(signer, contact_crypto) + // + // Both drains hit the network, so both are bounded. An abandoned drain + // is safe to walk away from: entries it did not complete stay queued + // and the next signer-present action retries them. + // + // Without a provider there is nothing to drain WITH. A caller that + // passes `None` gets the sequence's other steps and an honest + // `contact_accounts_pending`, rather than a drain that reports zero + // because every crypto operation failed. + let (drained, accepted) = match contact_crypto { + Some(contact_crypto) => { + let drained = within_budget( + deadline, + identity_wallet + .dashpay() + .drain_pending_contact_crypto(contact_crypto), + ) + .await + .unwrap_or(0); + let accepted = match identity_signer { + Some(signer) => within_budget( + deadline, + identity_wallet + .dashpay() + .drain_auto_accepts(signer, contact_crypto), + ) .await + .unwrap_or(0), + None => 0, + }; + (drained, accepted) + } + None => { + tracing::info!( + wallet_id = %hex::encode(wallet_id), + "startup: no contact-crypto provider; skipping the drain" + ); + (0, 0) } - None => 0, }; + // Not budgeted: a local queue-length read with no I/O. Leaving it + // unbounded keeps the reported `pending` truthful even when the steps + // above ran out of time — which is exactly when it matters most. let pending = identity_wallet .dashpay() .pending_contact_crypto_count() @@ -533,6 +584,38 @@ mod tests { assert!(!tally.has_identity()); } + /// Every network step is abandonable, so `within_budget` must return + /// `None` rather than run a future past the deadline. This is the guard for + /// the gap review found: bounding only the discovery retries let a stalled + /// sync or drain hold Core SPV well past `budget`. + #[tokio::test(start_paused = true)] + async fn within_budget_abandons_a_step_that_outlasts_the_deadline() { + let deadline = Instant::now() + Duration::from_secs(2); + let slow = async { + tokio::time::sleep(Duration::from_secs(30)).await; + "finished" + }; + assert_eq!(within_budget(deadline, slow).await, None); + } + + #[tokio::test(start_paused = true)] + async fn within_budget_returns_a_step_that_fits() { + let deadline = Instant::now() + Duration::from_secs(10); + let quick = async { + tokio::time::sleep(Duration::from_secs(1)).await; + "finished" + }; + assert_eq!(within_budget(deadline, quick).await, Some("finished")); + } + + /// A deadline already in the past must not start the step at all. + #[tokio::test(start_paused = true)] + async fn within_budget_skips_once_the_deadline_has_passed() { + let deadline = Instant::now(); + tokio::time::sleep(Duration::from_secs(1)).await; + assert_eq!(within_budget(deadline, async { "ran" }).await, None); + } + #[test] fn outcome_carries_the_tally_through() { let mut tally = StartupTally::default(); diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift index 2ccbe96662..b9aea7f1aa 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift @@ -116,7 +116,22 @@ extension PlatformWalletManager { let identitySigner: KeychainSigner? = self.modelContainer.map { KeychainSigner(modelContainer: $0, network: self.signerNetwork ?? .testnet) } - let budgetSecs = UInt64((budget ?? 0).rounded()) + // `0` means "SDK default" across the boundary, so a caller asking for a + // deliberately short budget must not round down into it — clamp to one + // second instead. Non-finite and negative values are rejected outright + // rather than trapping in the `UInt64` initializer, which is reachable + // whenever the budget comes out of arithmetic. + let budgetSecs: UInt64 + switch budget { + case .none: + budgetSecs = 0 + case .some(let requested) where !requested.isFinite || requested < 0: + throw PlatformWalletError.invalidParameter( + "budget must be a finite, non-negative number of seconds, got \(requested)" + ) + case .some(let requested): + budgetSecs = max(1, UInt64(requested.rounded())) + } return try await Task.detached(priority: .userInitiated) { () -> WalletStartupOutcome in try withExtendedLifetime((coreSigner, identitySigner)) { From 235620bb642e4f1a8aa51a47366997a3812ad8d5 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 10 Aug 2026 21:00:40 +0300 Subject: [PATCH 06/12] fix(platform-wallet): tell a local discovery failure apart from an unreachable Platform MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Review follow-up on #4359. The non-network `Err` arm called `record_discovery_gave_up()`, which maps to `PartialNoIdentity` — the one status that asks the client to scan again. Its own comment says the opposite: a wallet-manager or persistence fault "will not fix itself on the next attempt". Clients were being sent on a rescan guaranteed to hit the same fault. New terminal `DiscoveryFailed` status for that case. It outranks unreachability in the classification, since both leave the identity question open but only this one settles whether asking again is worth it. That distinction did not fit the existing predicate, so there are now two, and they are deliberately not inverses: - `discovery_worth_retrying()` — true only for `PartialNoIdentity`. Answers "should I scan again?" - `identity_is_settled()` — false for `PartialNoIdentity` AND `DiscoveryFailed`. Answers "do we know?" `DiscoveryFailed` is the case that needs both: unanswered, yet not worth retrying. Collapsing them into one predicate is what produced the bug. FFI discriminant 4, appended so existing values are untouched. --- .../src/wallet_startup.rs | 2 + .../rs-platform-wallet/src/manager/startup.rs | 99 ++++++++++++++++--- .../PlatformWalletManagerStartup.swift | 27 ++++- 3 files changed, 112 insertions(+), 16 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/wallet_startup.rs b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs index 3b90d80c72..c218636aec 100644 --- a/packages/rs-platform-wallet-ffi/src/wallet_startup.rs +++ b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs @@ -28,6 +28,7 @@ pub enum WalletStartupStatusFFI { NoIdentity = 1, PartialNoIdentity = 2, PartialAccountsPending = 3, + DiscoveryFailed = 4, } impl From for WalletStartupStatusFFI { @@ -37,6 +38,7 @@ impl From for WalletStartupStatusFFI { WalletStartupStatus::NoIdentity => Self::NoIdentity, WalletStartupStatus::PartialNoIdentity => Self::PartialNoIdentity, WalletStartupStatus::PartialAccountsPending => Self::PartialAccountsPending, + WalletStartupStatus::DiscoveryFailed => Self::DiscoveryFailed, } } } diff --git a/packages/rs-platform-wallet/src/manager/startup.rs b/packages/rs-platform-wallet/src/manager/startup.rs index abf0b84e74..d6bf78c396 100644 --- a/packages/rs-platform-wallet/src/manager/startup.rs +++ b/packages/rs-platform-wallet/src/manager/startup.rs @@ -111,8 +111,13 @@ pub enum WalletStartupStatus { /// failure: there is nothing to sync and nothing to drain. NoIdentity, /// The scan never reached Platform within the budget. The wallet may well - /// own an identity; we do not know yet. + /// own an identity; we do not know yet, and asking again may answer it. PartialNoIdentity, + /// Discovery failed locally — a wallet-manager or persistence error, not a + /// reachability problem. Like [`Self::PartialNoIdentity`] the identity + /// question is unanswered, but unlike it another scan will not answer it: + /// the same local fault is still there. Terminal for this launch. + DiscoveryFailed, /// Identity resolved and synced, but contact-account builds are still /// queued. The budget may have run out, the drain may have failed on some /// entries, or no contact-crypto provider was supplied — the count is what @@ -122,15 +127,29 @@ pub enum WalletStartupStatus { } impl WalletStartupStatus { - /// Whether Platform gave a definitive answer about this seed's identity. + /// Whether another discovery scan could change the answer. /// - /// `false` only for [`Self::PartialNoIdentity`] — the one outcome worth - /// repeating. This is the distinction that platform#4352 made expressible: - /// before it, "no identity exists" and "we never got through" both arrived - /// as an empty success, so clients either retried a proven-empty scan - /// forever or cached a network failure as fact. + /// True only for [`Self::PartialNoIdentity`]. The other three are terminal + /// for different reasons — an identity was found, absence was proven, or + /// the failure is local and will still be there next time — and only an + /// unreachable Platform is worth asking again. + /// + /// This is the distinction platform#4352 made expressible: before it, "no + /// identity exists" and "we never got through" both arrived as an empty + /// success, so clients either retried a proven-empty scan forever or cached + /// a network failure as fact. + pub fn discovery_worth_retrying(self) -> bool { + matches!(self, Self::PartialNoIdentity) + } + + /// Whether the identity question has an answer. + /// + /// Note this is NOT the inverse of [`Self::discovery_worth_retrying`]: + /// [`Self::DiscoveryFailed`] leaves the question open *and* is not worth + /// retrying. Use this to decide what to display, and + /// `discovery_worth_retrying` to decide whether to scan again. pub fn identity_is_settled(self) -> bool { - !matches!(self, Self::PartialNoIdentity) + !matches!(self, Self::PartialNoIdentity | Self::DiscoveryFailed) } } @@ -170,6 +189,10 @@ pub(crate) struct StartupTally { pub proven_no_identity: bool, /// The budget ran out while discovery was still unreachable. pub discovery_unreachable: bool, + /// Discovery hit a local fault (wallet manager, persistence) rather than a + /// reachability problem. Tracked apart from `discovery_unreachable` + /// because retrying cannot clear it. + pub discovery_failed_locally: bool, pub discovery_attempts: u32, pub dashpay_sync_ran: bool, pub contact_accounts_drained: usize, @@ -204,11 +227,18 @@ impl StartupTally { self.discovery_attempts += 1; } - /// The budget expired with discovery still unresolved. + /// The budget expired with discovery still unresolved. Retryable. pub(crate) fn record_discovery_gave_up(&mut self) { self.discovery_unreachable = true; } + /// Discovery failed on a local fault. Not retryable — the wallet-manager + /// or persistence problem behind it is still there on the next attempt, so + /// telling the client to rescan would only waste a round trip. + pub(crate) fn record_discovery_failed_locally(&mut self) { + self.discovery_failed_locally = true; + } + /// Whether there is an identity to sync and drain for. pub(crate) fn has_identity(&self) -> bool { self.identity_id.is_some() @@ -230,6 +260,11 @@ impl StartupTally { /// absence outranks the drain counters for the same reason — with no /// identity there is nothing to have drained. pub(crate) fn status(&self) -> WalletStartupStatus { + // A local fault outranks unreachability: both leave the question open, + // but only this one tells the client not to bother asking again. + if self.discovery_failed_locally { + return WalletStartupStatus::DiscoveryFailed; + } if self.discovery_unreachable { return WalletStartupStatus::PartialNoIdentity; } @@ -459,12 +494,13 @@ impl PlatformWalletManager } Err(e) => { // Not a reachability question — a wallet/persistence - // failure will not fix itself on the next attempt. + // failure will not fix itself on the next attempt, so this + // is recorded as terminal rather than as "try again". tracing::warn!( error = %e, "startup: identity discovery failed for a non-network reason" ); - tally.record_discovery_gave_up(); + tally.record_discovery_failed_locally(); return; } } @@ -522,6 +558,47 @@ mod tests { assert_eq!(tally.discovery_attempts, 2); } + /// A local discovery fault is terminal, unlike an unreachable Platform. + /// The branch that produces it says the failure will not fix itself, so + /// reporting it as retryable would send clients on a futile rescan. + #[test] + fn a_local_discovery_failure_is_terminal() { + let mut tally = StartupTally::default(); + tally.record_discovery_failed_locally(); + + assert_eq!(tally.status(), WalletStartupStatus::DiscoveryFailed); + assert!( + !tally.status().discovery_worth_retrying(), + "the same local fault will still be there next time" + ); + assert!( + !tally.status().identity_is_settled(), + "terminal is not the same as answered — we still do not know" + ); + } + + /// Both leave the identity question open, but only one is worth asking + /// again. Keeping that asymmetry visible is the point of the two methods. + #[test] + fn only_an_unreachable_platform_is_worth_retrying() { + let mut unreachable = StartupTally::default(); + unreachable.record_unreachable(); + unreachable.record_discovery_gave_up(); + assert!(unreachable.status().discovery_worth_retrying()); + + for terminal in [ + WalletStartupStatus::Ready, + WalletStartupStatus::NoIdentity, + WalletStartupStatus::PartialAccountsPending, + WalletStartupStatus::DiscoveryFailed, + ] { + assert!( + !terminal.discovery_worth_retrying(), + "{terminal:?} must not ask the client to rescan" + ); + } + } + /// An unreachable Platform outranks a clean drain: the later steps ran /// against state we know to be incomplete. #[test] diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift index b9aea7f1aa..db2b57bffc 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift @@ -20,16 +20,33 @@ public enum WalletStartupStatus: UInt8, Sendable { /// failure — there is nothing to sync and nothing to drain. case noIdentity = 1 /// The identity scan never reached Platform inside the budget. The wallet - /// may well own one; we do not know yet. + /// may well own one; we do not know yet, and asking again may answer it. case partialNoIdentity = 2 /// Identity resolved and synced, but contact-account builds are still - /// queued — the drain did not finish inside the budget. + /// queued. The budget may have run out, the drain may have failed on some + /// entries, or no contact-crypto provider was available. case partialAccountsPending = 3 + /// Discovery failed locally — a wallet or persistence fault, not a + /// reachability problem. The identity question is unanswered, and another + /// scan will not answer it: the same fault is still there. + case discoveryFailed = 4 - /// Whether Platform gave a definitive answer about this seed's identity. + /// Whether another discovery scan could change the answer. /// - /// `false` only for ``partialNoIdentity``, the one outcome worth repeating. - public var identityIsSettled: Bool { self != .partialNoIdentity } + /// True only for ``partialNoIdentity``. The others are terminal for this + /// launch — an identity was found, absence was proven, or the failure is + /// local and will still be there next time. + public var discoveryWorthRetrying: Bool { self == .partialNoIdentity } + + /// Whether the identity question has an answer. + /// + /// Not the inverse of ``discoveryWorthRetrying``: ``discoveryFailed`` + /// leaves the question open *and* is not worth retrying. Use this to decide + /// what to show, and ``discoveryWorthRetrying`` to decide whether to scan + /// again. + public var identityIsSettled: Bool { + self != .partialNoIdentity && self != .discoveryFailed + } } /// What a bring-up did. From 4f04fd3fd4240dd629d3fb55810dbe46a49e77b4 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 10 Aug 2026 21:59:34 +0300 Subject: [PATCH 07/12] fix(platform-wallet): close six holes in the bring-up contract MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit All from review on #4359. **Discovery ran outside the budget.** The previous round bounded the sync pass and both drains but left the scans themselves unbounded — and one scan walks up to `gap_limit` indices, each a Platform fetch plus a DPNS lookup. A Platform outage could hold Core SPV well past the ceiling this call advertises. Each attempt now goes through `within_budget`. **An absurd budget could abort the host.** `Instant + Duration` panics when the sum is unrepresentable, and the FFI thread wrapper re-raises a panic as an abort, so a large `budget_secs` took the process down instead of returning. Built with `checked_add`, `InvalidParameter` on overflow. Swift had the mirror problem one layer up: a finite `TimeInterval` can still be outside `UInt64`, so the conversion now uses `UInt64(exactly:)` — the two validations are independent because plenty of representable `UInt64` seconds still cannot be added to an `Instant`. **An empty scan is not always proof of absence.** `discover` reports only identities THAT call inserted, so two concurrent bring-ups can race: the second sees the identity on Platform, finds it already managed, returns empty, and would have recorded proven absence for a wallet that demonstrably owns one. Local state is now consulted before classifying an empty return, and again after an abandoned scan, since sightings persist incrementally. **A failed contact pass could still report `Ready`.** An empty queue only means "nothing left to build" if a pass actually completed; without one there may be undiscovered requests whose builds were never enqueued. `Ready` now requires `dashpay_sync_ran`. **Eager mnemonic resolution broke the error contract.** The FFI resolved the master xpriv before Rust checked for a local identity, so a Keychain hiccup failed a warm launch that needed no scan at all — while the docs promise only handle and wallet-id problems throw. Resolution failure is now a warning; if a scan does turn out to be needed it fails without key material and surfaces as `DiscoveryFailed`, which is the structured outcome for precisely this. Partial on the first point: `sync_contact_requests` swallows per-identity fetch failures internally and returns `Ok` regardless, so a pass where every identity failed still counts as ran. Fixing that means changing its signature to report completion, which is a wider change than this PR should carry — noted on the thread. --- .../src/wallet_startup.rs | 16 ++++- .../rs-platform-wallet/src/manager/startup.rs | 65 +++++++++++++++++-- .../PlatformWalletManagerStartup.swift | 13 +++- 3 files changed, 87 insertions(+), 7 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/wallet_startup.rs b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs index c218636aec..1168c3c488 100644 --- a/packages/rs-platform-wallet-ffi/src/wallet_startup.rs +++ b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs @@ -146,12 +146,26 @@ pub unsafe extern "C" fn platform_wallet_manager_start_wallet_subsystems( // Resolve the master xpriv once, up front. The helper holds the mnemonic // and seed in `Zeroizing` buffers and scrubs them before returning; the // master itself has no `Drop`, so it is erased explicitly below. + // + // A failure here is NOT returned as an FFI error. This call documents that + // only handle and wallet-id problems throw, and the resolver is needed only + // when a scan actually runs — a warm launch whose identity is already known + // must not be failed by a Keychain hiccup it never needed. If a scan does + // turn out to be required, it fails without key material and surfaces as + // `DiscoveryFailed`, which is the structured outcome for exactly this. let mut master = if mnemonic_resolver_handle.is_null() { None } else { match resolve_master_from_resolver(mnemonic_resolver_handle, &wid, network) { Ok(master) => Some(master), - Err(e) => return e, + Err(_) => { + tracing::warn!( + wallet_id = %hex::encode(wid), + "startup: could not resolve the wallet mnemonic; continuing without \ + scan key material" + ); + None + } } }; diff --git a/packages/rs-platform-wallet/src/manager/startup.rs b/packages/rs-platform-wallet/src/manager/startup.rs index d6bf78c396..0e32b90c4f 100644 --- a/packages/rs-platform-wallet/src/manager/startup.rs +++ b/packages/rs-platform-wallet/src/manager/startup.rs @@ -274,6 +274,13 @@ impl StartupTally { if self.contact_accounts_pending > 0 { return WalletStartupStatus::PartialAccountsPending; } + // An empty queue only means "nothing left to build" if a contact pass + // actually completed. Without one there may be undiscovered contact + // requests whose account builds were never enqueued, and calling that + // `Ready` would promise addresses this call never prepared. + if !self.dashpay_sync_ran { + return WalletStartupStatus::PartialAccountsPending; + } WalletStartupStatus::Ready } @@ -327,7 +334,16 @@ impl PlatformWalletManager S: Signer + Send + Sync, { let started = Instant::now(); - let deadline = started + opts.budget; + // `Instant + Duration` panics when the sum is not representable, and + // this runs under an `extern "C"` caller whose thread wrapper re-raises + // a panic as an abort — so an absurd budget would take the host process + // down rather than return an error. + let deadline = started.checked_add(opts.budget).ok_or_else(|| { + PlatformWalletError::InvalidParameter(format!( + "startup budget {:?} exceeds the supported duration range", + opts.budget + )) + })?; let mut tally = StartupTally::default(); let wallet = self @@ -342,6 +358,7 @@ impl PlatformWalletManager tally.record_local_identity(known); } else { self.discover_identity_with_backoff( + wallet_id, identity_wallet, master, opts.gap_limit, @@ -464,6 +481,7 @@ impl PlatformWalletManager /// — is worth another attempt. async fn discover_identity_with_backoff( &self, + wallet_id: &WalletId, identity_wallet: &crate::wallet::identity::IdentityWallet, master: Option<&ExtendedPrivKey>, gap_limit: Option, @@ -476,16 +494,39 @@ impl PlatformWalletManager }; for (attempt, backoff) in DISCOVERY_BACKOFF.iter().map(Some).chain([None]).enumerate() { - let result = match master { - Some(master) => identity_wallet.discover_from_master(opts, master).await, - None => identity_wallet.discover(opts).await, + // A single scan walks up to `gap_limit` indices, each a Platform + // fetch plus a DPNS lookup, so it needs the same ceiling the other + // steps have — otherwise one attempt can outlast the whole budget + // this call promises. + let attempt_future = async { + match master { + Some(master) => identity_wallet.discover_from_master(opts, master).await, + None => identity_wallet.discover(opts).await, + } + }; + let Some(result) = within_budget(deadline, attempt_future).await else { + // Sightings persist incrementally, so an abandoned scan may + // still have folded an identity in before it was cut off. + if let Some(known) = self.local_identity_id(wallet_id).await { + tally.record_local_identity(known); + return; + } + break; }; match result { Ok(found) => { match found.first() { Some(identity) => tally.record_discovered(identity.id()), - None => tally.record_proven_absent(), + // An empty return is not proof on its own: `discover` + // reports only identities THIS call inserted, so a + // concurrent startup that inserted one first leaves us + // seeing it as already-managed and returning nothing. + // Consult local state before calling it absence. + None => match self.local_identity_id(wallet_id).await { + Some(known) => tally.record_discovered(known), + None => tally.record_proven_absent(), + }, } return; } @@ -637,6 +678,20 @@ mod tests { ); } + /// An empty drain queue is not evidence of readiness on its own. Without a + /// completed contact pass there may be requests nobody has looked at, whose + /// account builds were therefore never enqueued — reporting `Ready` would + /// promise addresses this call never prepared. + #[test] + fn an_empty_queue_without_a_contact_pass_is_not_ready() { + let mut tally = StartupTally::default(); + tally.record_discovered(identity()); + tally.record_drain(0, 0); + + assert!(!tally.dashpay_sync_ran); + assert_eq!(tally.status(), WalletStartupStatus::PartialAccountsPending); + } + #[test] fn queued_builds_report_as_pending() { let mut tally = StartupTally::default(); diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift index db2b57bffc..af1ac6b72b 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift @@ -147,7 +147,18 @@ extension PlatformWalletManager { "budget must be a finite, non-negative number of seconds, got \(requested)" ) case .some(let requested): - budgetSecs = max(1, UInt64(requested.rounded())) + // `UInt64(_:)` traps on anything outside its range, which a finite + // `TimeInterval` can still be — `.greatestFiniteMagnitude` among + // them. The failable initializer turns that into an error instead + // of a crash. Rust validates the deadline independently: plenty of + // representable `UInt64` seconds still cannot be added to an + // `Instant`. + guard let converted = UInt64(exactly: requested.rounded()) else { + throw PlatformWalletError.invalidParameter( + "budget is outside the supported range of whole seconds: \(requested)" + ) + } + budgetSecs = max(1, converted) } return try await Task.detached(priority: .userInitiated) { () -> WalletStartupOutcome in From 1af08c92868c4d7c0677dbbea876df4f9fa66656 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Mon, 10 Aug 2026 22:08:56 +0300 Subject: [PATCH 08/12] fix(platform-wallet): skip the Keychain round trip a warm launch cannot use MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Two follow-ups on #4359. The previous commit stopped a resolver failure from failing the call, but left the resolution itself happening unconditionally — so a warm launch still paid for a Keychain round trip whose result it would never use, and spent that time outside the budget it was about to be measured against. Key material is only ever needed for a scan, and a wallet with an identity on file does not run one. `has_local_identity_blocking` answers that from a single lock read, and the resolve is skipped when the answer is yes. Also documents the second error this call can now return. `checked_add` made `InvalidParameter` reachable for an unrepresentable budget, while the Rust, FFI and Swift docs all still promised that only missing-wallet and configuration problems throw. All three now say the same thing: errors are about the request, outcomes are about the run. --- .../src/wallet_startup.rs | 29 ++++++++++++------- .../src/manager/accessors.rs | 18 ++++++++++++ .../rs-platform-wallet/src/manager/startup.rs | 17 +++++++---- .../PlatformWalletManagerStartup.swift | 5 ++-- 4 files changed, 50 insertions(+), 19 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/wallet_startup.rs b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs index 1168c3c488..11e2bc5a44 100644 --- a/packages/rs-platform-wallet-ffi/src/wallet_startup.rs +++ b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs @@ -89,10 +89,12 @@ impl From for WalletStartupOutcomeFFI { /// outcome to `out_outcome`. Intended to be called once per wallet load, /// immediately before starting Core SPV. /// -/// Only an invalid manager handle or an unknown `wallet_id` produce an error; -/// an unreachable Platform, a failed sync pass and an unfinished drain are all -/// reported through `out_outcome.status`, because a host must be able to start -/// Core SPV regardless. See [`WalletStartupStatusFFI`]. +/// Errors are limited to the request itself: an invalid manager handle, an +/// unknown `wallet_id`, or a `budget_secs` so large its deadline is not +/// representable. Everything about the run — an unreachable Platform, a failed +/// sync pass, an unfinished drain — is reported through `out_outcome.status`, +/// because a host must be able to start Core SPV regardless. See +/// [`WalletStartupStatusFFI`]. /// /// # Arguments /// @@ -143,17 +145,22 @@ pub unsafe extern "C" fn platform_wallet_manager_start_wallet_subsystems( ); }; + // Key material is only needed for a scan, and a wallet whose identity is + // already on file will not run one — so a warm launch must not pay for a + // Keychain round trip, nor be failed by one that goes wrong. Ask first. + let needs_scan_key = !PLATFORM_WALLET_MANAGER_STORAGE + .with_item(manager_handle, |m| m.has_local_identity_blocking(&wid)) + .unwrap_or(false); + // Resolve the master xpriv once, up front. The helper holds the mnemonic // and seed in `Zeroizing` buffers and scrubs them before returning; the // master itself has no `Drop`, so it is erased explicitly below. // - // A failure here is NOT returned as an FFI error. This call documents that - // only handle and wallet-id problems throw, and the resolver is needed only - // when a scan actually runs — a warm launch whose identity is already known - // must not be failed by a Keychain hiccup it never needed. If a scan does - // turn out to be required, it fails without key material and surfaces as - // `DiscoveryFailed`, which is the structured outcome for exactly this. - let mut master = if mnemonic_resolver_handle.is_null() { + // A failure here is NOT returned as an FFI error: this call documents that + // only handle and wallet-id problems throw. Without key material the scan + // fails and surfaces as `DiscoveryFailed`, which is the structured outcome + // for exactly this. + let mut master = if mnemonic_resolver_handle.is_null() || !needs_scan_key { None } else { match resolve_master_from_resolver(mnemonic_resolver_handle, &wid, network) { diff --git a/packages/rs-platform-wallet/src/manager/accessors.rs b/packages/rs-platform-wallet/src/manager/accessors.rs index 7b9c642282..c766449fb1 100644 --- a/packages/rs-platform-wallet/src/manager/accessors.rs +++ b/packages/rs-platform-wallet/src/manager/accessors.rs @@ -458,6 +458,24 @@ impl PlatformWalletManager

{ Some(wm.get_wallet_info(wallet_id)?.core_wallet.network()) } + /// Whether this wallet already has an identity on file locally. + /// + /// Lets a caller skip work that only an identity scan would need — most + /// importantly resolving key material, which on iOS means a Keychain round + /// trip that a warm launch should not pay for. Blocking and cheap: one + /// `RwLock` read, no I/O. + pub fn has_local_identity_blocking(&self, wallet_id: &WalletId) -> bool { + let wm = self.wallet_manager.blocking_read(); + wm.get_wallet_info(wallet_id) + .map(|info| { + !info + .identity_manager + .wallet_identity_ids(wallet_id) + .is_empty() + }) + .unwrap_or(false) + } + /// Snapshot of [`PlatformAddressSyncManager`] tunables and last- /// pass timestamp. `watch_list_size` is `wallets.len()` — every /// registered wallet participates in each pass since the sync diff --git a/packages/rs-platform-wallet/src/manager/startup.rs b/packages/rs-platform-wallet/src/manager/startup.rs index 0e32b90c4f..42056a8687 100644 --- a/packages/rs-platform-wallet/src/manager/startup.rs +++ b/packages/rs-platform-wallet/src/manager/startup.rs @@ -308,12 +308,17 @@ impl PlatformWalletManager /// /// # Errors /// - /// Only [`PlatformWalletError::WalletNotFound`]. Every other outcome — - /// an unreachable Platform, a failed sync pass, a drain that did not - /// finish inside the budget — is reported in [`WalletStartupOutcome`], so - /// a client can start Core SPV regardless and let the DIP-15 rescan repair - /// whatever is missing. Failing loudly here would trade a data gap for a - /// wallet with no balance, which is the worse of the two. + /// Two, both about the request rather than its execution: + /// [`PlatformWalletError::WalletNotFound`] for an unknown `wallet_id`, and + /// [`PlatformWalletError::InvalidParameter`] for a budget whose deadline + /// would not be representable. + /// + /// Everything about the run itself — an unreachable Platform, a failed + /// sync pass, a drain that did not finish inside the budget — is reported + /// in [`WalletStartupOutcome`], so a client can start Core SPV regardless + /// and let the DIP-15 rescan repair whatever is missing. Failing loudly + /// there would trade a data gap for a wallet with no balance, which is the + /// worse of the two. /// /// # Key material /// diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift index af1ac6b72b..6c866a5b8a 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift @@ -95,8 +95,9 @@ extension PlatformWalletManager { /// - Returns: what the sequence achieved. Start Core SPV regardless of the /// status; inspect `contactAccountsPending` for diagnostics. /// - /// - Throws: only for an unconfigured manager, a malformed wallet id, or an - /// unknown wallet. An unreachable Platform, a failed sync pass and an + /// - Throws: only for a malformed request — an unconfigured manager, a + /// malformed wallet id, an unknown wallet, or a `budget` outside the + /// supported range. An unreachable Platform, a failed sync pass and an /// unfinished drain are all reported through the returned status. /// /// # Key material From 482997e3c84e37d5cf5f8234157431711f152189 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 11 Aug 2026 00:06:38 +0300 Subject: [PATCH 09/12] fix(platform-wallet): bound the drains from the inside, not with a timeout MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both from review on #4359. **The drains are not cancel-safe, and this call was their first caller that cancels them.** Both commit per-entry side effects as they go — `register_contact_account`, `mark_contact_channel_broken`, the reciprocal send — while accumulating the dequeue list in a local vec applied once after the loop. `within_budget` is `tokio::time::timeout`, which drops the future, so a budget expiry at entry 14 of 14 left 13 accounts that really exist with all 14 entries still queued, and reported `drained=0, pending=14`. The queue is at-least-once, so nothing was corrupted; what was wrong is the outcome struct this call exists to return, wrong exactly when the budget fires. Fixed by threading the deadline into the loop rather than wrapping it: `drain_pending_contact_crypto_until` / `drain_auto_accepts_until`, with the existing methods as `None`-deadline wrappers so the six unbounded call sites are untouched. The loop stops between entries, and within an entry only the reads that precede its first commit are bounded — the xpub derive, the contact fetch, the ECDH, the proof-key derive. Every commit still runs to completion, `cleared` always describes work that landed, and the `.unwrap_or(0)` pair at the call site is gone rather than corrected. Clearing each entry as it lands was the other option, and costs more than it looks: `FlushMode::Immediate` means one sqlite transaction per `store()`, so a 350-entry queue would pay hundreds of blocking commits inside the pre-SPV path this whole sequence exists to keep short. The value-aware `retain_drained_by_snapshot` apply block is also left untouched this way. `register_external_contact_account` now documents why it must never be wrapped in a timeout: unlike its sibling it persists *before* acquiring the write lock, so a future dropped in that window leaves an account on disk that no in-memory collection knows about until the next drain re-registers it. Not reachable from this change — a landmine for the next one that wraps by analogy. **A key that could not be read was reported as a key that does not exist.** A resolver failure became `master = None`, the library took the resident-wallet derive, and an external-signable wallet failed it with "no private key" — classified terminal `DiscoveryFailed`. But the realistic causes at launch are a locked device or a denied Keychain read, all transient, and `DiscoveryFailed` tells the client not to try again. `ScanKey::{Resident, Master, Unavailable}` spells the third case out, and Swift stops collapsing it: `WalletStorage.mnemonicAvailability` returns `present` / `absent` / `unavailable(OSStatus)`, and only a definitive `absent` means watch-only. cargo test -p platform-wallet --lib # 635 passed (4 new) cargo test -p platform-wallet-ffi --lib # 261 passed cargo clippy + cargo fmt --check # clean --- .../src/wallet_startup.rs | 53 +++-- .../rs-platform-wallet/src/manager/startup.rs | 94 +++++--- .../identity/network/contact_requests.rs | 208 +++++++++++++++++- .../src/wallet/identity/network/contacts.rs | 11 + .../Core/Wallet/WalletStorage.swift | 36 +++ .../PlatformWalletManagerStartup.swift | 22 +- 6 files changed, 366 insertions(+), 58 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/wallet_startup.rs b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs index 11e2bc5a44..dd3e4e71d4 100644 --- a/packages/rs-platform-wallet-ffi/src/wallet_startup.rs +++ b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs @@ -7,7 +7,7 @@ //! [`platform_wallet::manager::startup`]; this is the marshalling shell. use platform_wallet::manager::startup::{ - WalletStartupOptions, WalletStartupOutcome, WalletStartupStatus, + ScanKey, WalletStartupOptions, WalletStartupOutcome, WalletStartupStatus, }; use rs_sdk_ffi::{MnemonicResolverHandle, SignerHandle, VTableSigner}; use std::time::Duration; @@ -156,24 +156,33 @@ pub unsafe extern "C" fn platform_wallet_manager_start_wallet_subsystems( // and seed in `Zeroizing` buffers and scrubs them before returning; the // master itself has no `Drop`, so it is erased explicitly below. // - // A failure here is NOT returned as an FFI error: this call documents that - // only handle and wallet-id problems throw. Without key material the scan - // fails and surfaces as `DiscoveryFailed`, which is the structured outcome - // for exactly this. - let mut master = if mnemonic_resolver_handle.is_null() || !needs_scan_key { + // A failure here is NOT returned as an FFI error — this call documents that + // only handle and wallet-id problems throw. It becomes `ScanKey::Unavailable` + // instead, so a locked device defers the scan rather than failing it + // terminally. + // + // `None` = no key needed (warm launch) or none exists (resident-key + // wallet); `Some(Err(()))` = a scan needs one and we could not get it. + // Both no-key cases collapse to `None`: nothing to scan for, or a host that + // deliberately supplied no resolver (watch-only / resident-key wallet, where + // discovery derives in-process). Neither is a failure to obtain a key. + let mut master: Option> = if !needs_scan_key || mnemonic_resolver_handle.is_null() + { None } else { - match resolve_master_from_resolver(mnemonic_resolver_handle, &wid, network) { - Ok(master) => Some(master), - Err(_) => { - tracing::warn!( - wallet_id = %hex::encode(wid), - "startup: could not resolve the wallet mnemonic; continuing without \ - scan key material" - ); - None - } - } + Some( + match resolve_master_from_resolver(mnemonic_resolver_handle, &wid, network) { + Ok(master) => Ok(master), + Err(_) => { + tracing::warn!( + wallet_id = %hex::encode(wid), + "startup: could not resolve the wallet mnemonic; deferring the \ + identity scan to a later start" + ); + Err(()) + } + }, + ) }; let signer_addr = if identity_signer_handle.is_null() { @@ -202,20 +211,26 @@ pub unsafe extern "C" fn platform_wallet_manager_start_wallet_subsystems( // so the future cannot satisfy `block_on_worker`'s `'static` bound. The // scoped thread also supplies the 8 MB stack the GroveDB proof // verification inside discovery needs. + let scan_key = match master.as_ref() { + Some(Ok(master)) => ScanKey::Master(master), + Some(Err(())) => ScanKey::Unavailable, + None => ScanKey::Resident, + }; + let result = run_on_big_stack_thread(|| { PLATFORM_WALLET_MANAGER_STORAGE.with_item(manager_handle, |manager| { crate::runtime::runtime().block_on(async { let signer = (signer_addr != 0).then(|| unsafe { &*(signer_addr as *const VTableSigner) }); manager - .start_wallet_subsystems(&wid, master.as_ref(), provider.as_ref(), signer, opts) + .start_wallet_subsystems(&wid, scan_key, provider.as_ref(), signer, opts) .await }) }) }); // Erase the master before any early return below. - if let Some(master) = master.as_mut() { + if let Some(Ok(master)) = master.as_mut() { master.private_key.non_secure_erase(); } diff --git a/packages/rs-platform-wallet/src/manager/startup.rs b/packages/rs-platform-wallet/src/manager/startup.rs index 42056a8687..85d17ed22b 100644 --- a/packages/rs-platform-wallet/src/manager/startup.rs +++ b/packages/rs-platform-wallet/src/manager/startup.rs @@ -70,6 +70,12 @@ const DISCOVERY_BACKOFF: [Duration; 2] = [Duration::from_secs(3), Duration::from /// it ran. Every step in the sequence is abandonable: the work it did not /// finish stays queued for the next attempt, and the caller needs Core SPV to /// start far more than it needs any one of them to complete. +/// +/// Only for steps that are **safe to drop mid-await**. This drops the future, +/// so any step that commits side effects before recording that it did must +/// take the deadline as a parameter and end itself between units of work +/// instead — that is why the two drains have `_until` variants rather than +/// being wrapped here. async fn within_budget(deadline: Instant, future: F) -> Option { let remaining = deadline.saturating_duration_since(Instant::now()); if remaining.is_zero() { @@ -78,6 +84,25 @@ async fn within_budget(deadline: Instant, future: F) -> tokio::time::timeout(remaining, future).await.ok() } +/// Where an identity scan gets its key material — and, importantly, the +/// difference between not needing any and not being able to get it. +/// +/// Collapsing the last case into "no master supplied" makes a temporary +/// problem look permanent: a Keychain-backed wallet would fall through to the +/// resident-key derive, fail because it deliberately holds no private key, and +/// be classified [`WalletStartupStatus::DiscoveryFailed`] — which tells the +/// client never to try again over a device that was merely locked. +pub enum ScanKey<'a> { + /// The wallet holds resident keys; discovery derives in-process. + Resident, + /// The caller resolved a master xpriv for this call. + Master(&'a ExtendedPrivKey), + /// Key material was needed but could not be obtained right now — a locked + /// device, a denied Keychain read, a transient failure. Discovery is + /// skipped and reported as retryable rather than attempted and failed. + Unavailable, +} + /// Knobs for [`PlatformWalletManager::start_wallet_subsystems`]. #[derive(Debug, Clone, Copy)] pub struct WalletStartupOptions { @@ -322,14 +347,14 @@ impl PlatformWalletManager /// /// # Key material /// - /// `master` and `contact_crypto` are borrowed for this call only and must - /// not be retained by the caller afterwards. `master` is `None` for a - /// wallet holding resident keys; `identity_signer` is `None` to skip the - /// DIP-15 auto-accept pass. + /// `scan_key` and `contact_crypto` are borrowed for this call only and must + /// not be retained by the caller afterwards. See [`ScanKey`] for why + /// "unavailable" is spelled out rather than folded into "none". + /// `identity_signer` is `None` to skip the DIP-15 auto-accept pass. pub async fn start_wallet_subsystems( &self, wallet_id: &WalletId, - master: Option<&ExtendedPrivKey>, + scan_key: ScanKey<'_>, contact_crypto: Option<&C>, identity_signer: Option<&S>, opts: WalletStartupOptions, @@ -365,7 +390,7 @@ impl PlatformWalletManager self.discover_identity_with_backoff( wallet_id, identity_wallet, - master, + scan_key, opts.gap_limit, deadline, &mut tally, @@ -408,9 +433,14 @@ impl PlatformWalletManager // 3. The step that actually creates the addresses. // - // Both drains hit the network, so both are bounded. An abandoned drain - // is safe to walk away from: entries it did not complete stay queued - // and the next signer-present action retries them. + // Both drains hit the network, so both are bounded — but by the + // deadline they take as a parameter, NOT by `within_budget`. They + // commit per-entry side effects as they go and apply the dequeue list + // once at the end, so dropping one mid-loop would strand work that + // really happened and report it as zero. Bounding them from the inside + // stops the loop between entries instead: what they return and what + // they dequeue always describe work that completed, and entries they + // never reached stay queued for the next signer-present action. // // Without a provider there is nothing to drain WITH. A caller that // passes `None` gets the sequence's other steps and an honest @@ -418,23 +448,17 @@ impl PlatformWalletManager // because every crypto operation failed. let (drained, accepted) = match contact_crypto { Some(contact_crypto) => { - let drained = within_budget( - deadline, - identity_wallet - .dashpay() - .drain_pending_contact_crypto(contact_crypto), - ) - .await - .unwrap_or(0); + let drained = identity_wallet + .dashpay() + .drain_pending_contact_crypto_until(contact_crypto, Some(deadline)) + .await; let accepted = match identity_signer { - Some(signer) => within_budget( - deadline, + Some(signer) => { identity_wallet .dashpay() - .drain_auto_accepts(signer, contact_crypto), - ) - .await - .unwrap_or(0), + .drain_auto_accepts_until(signer, contact_crypto, Some(deadline)) + .await + } None => 0, }; (drained, accepted) @@ -488,11 +512,23 @@ impl PlatformWalletManager &self, wallet_id: &WalletId, identity_wallet: &crate::wallet::identity::IdentityWallet, - master: Option<&ExtendedPrivKey>, + scan_key: ScanKey<'_>, gap_limit: Option, deadline: Instant, tally: &mut StartupTally, ) { + // No key, no scan — but this is a "come back later", not a failure. + // Attempting it anyway would derive against a wallet that holds no + // private key, and the resulting error would be classified terminal. + if matches!(scan_key, ScanKey::Unavailable) { + tracing::info!( + "startup: scan key material unavailable; deferring discovery to a later start" + ); + tally.record_unreachable(); + tally.record_discovery_gave_up(); + return; + } + let opts = IdentityDiscoveryOptions { start_index: Some(0), gap_limit: gap_limit.unwrap_or(IdentityDiscoveryOptions::default().gap_limit), @@ -504,9 +540,13 @@ impl PlatformWalletManager // steps have — otherwise one attempt can outlast the whole budget // this call promises. let attempt_future = async { - match master { - Some(master) => identity_wallet.discover_from_master(opts, master).await, - None => identity_wallet.discover(opts).await, + match scan_key { + ScanKey::Master(master) => { + identity_wallet.discover_from_master(opts, master).await + } + ScanKey::Resident => identity_wallet.discover(opts).await, + // Unreachable: guarded before the loop. + ScanKey::Unavailable => unreachable!(), } }; let Some(result) = within_budget(deadline, attempt_future).await else { diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs b/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs index ac821c2e3b..3c2dbe6549 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs @@ -910,6 +910,42 @@ fn retain_drained_by_snapshot( removed } +/// Await `future`, giving up once `deadline` has passed. `None` deadline is +/// the unbounded behaviour; a `None` return means the budget is spent. +/// +/// **Why the drains bound themselves rather than being wrapped in a +/// `tokio::time::timeout` by their caller.** Both drains commit per-entry side +/// effects as they go (`register_contact_account`, `mark_contact_channel_broken`, +/// the reciprocal send) while accumulating the dequeue list in a local `cleared` +/// vec applied once at the end. Dropping the drain future mid-loop — which is +/// exactly what an outer `timeout` does — discards that vec, so entries whose +/// work really landed stay queued and the returned count reports zero for work +/// that happened. The queue is at-least-once, so nothing is corrupted, but the +/// count is a lie precisely when a caller has a budget to report against. +/// +/// Threading the deadline inside keeps the loop the only thing that ever ends +/// early: it stops **between** entries, and within an entry only the reads that +/// precede its first commit are bounded. Every commit still runs to completion. +async fn bounded( + deadline: Option, + future: F, +) -> Option { + let Some(deadline) = deadline else { + return Some(future.await); + }; + let remaining = deadline.saturating_duration_since(std::time::Instant::now()); + if remaining.is_zero() { + return None; + } + tokio::time::timeout(remaining, future).await.ok() +} + +/// Whether `deadline` has passed. Checked at the top of each drain iteration so +/// a spent budget ends the loop between entries, never inside one. +fn budget_spent(deadline: Option) -> bool { + deadline.is_some_and(|d| std::time::Instant::now() >= d) +} + /// Whether a registered outbound `DashpayExternalAccount` for `contact` /// must be torn down + rebuilt because it was NOT built from the contact's /// current `incoming_request.account_reference`. @@ -1851,6 +1887,21 @@ impl DashPayView<'_, B> { pub async fn drain_pending_contact_crypto( &self, provider: &P, + ) -> usize { + self.drain_pending_contact_crypto_until(provider, None) + .await + } + + /// [`Self::drain_pending_contact_crypto`], stopping once `deadline` passes. + /// + /// The drain ends between entries, so the count it returns and the queue + /// removals it persists always describe work that actually completed — + /// see [`bounded`] for why this cannot be an outer timeout. Entries it + /// never reached stay queued for the next drain. + pub async fn drain_pending_contact_crypto_until( + &self, + provider: &P, + deadline: Option, ) -> usize { use crate::changeset::{PendingContactCryptoKey, PendingContactCryptoOp}; @@ -1874,6 +1925,17 @@ impl DashPayView<'_, B> { let mut cleared: Vec = Vec::new(); for entry in &entries { + // Stop between entries, never inside one: an entry that has already + // committed its side effect must reach the `cleared.push` that + // records it. + if budget_spent(deadline) { + tracing::info!( + processed = cleared.len(), + total = entries.len(), + "drain: budget spent; leaving the rest queued" + ); + break; + } match &entry.op { PendingContactCryptoOp::RegisterReceiving => { // Build the friendship path in Rust; the provider derives @@ -1893,7 +1955,16 @@ impl DashPayView<'_, B> { continue; } }; - match provider.receiving_xpub(&path).await { + // Bounded: the derive is the last step before this entry + // commits anything, so abandoning it changes no state. + let Some(xpub) = bounded(deadline, provider.receiving_xpub(&path)).await else { + tracing::info!( + owner = %entry.owner_identity_id, contact = %entry.contact_id, + "drain: budget spent deriving the receiving xpub; leaving queued" + ); + continue; + }; + match xpub { Ok(xpub) => match self .register_contact_account( &entry.owner_identity_id, @@ -1957,9 +2028,20 @@ impl DashPayView<'_, B> { }; // Fetch the contact identity (transient on failure → leave). + // Bounded: a Platform round trip, and nothing in this entry + // has committed yet. let contact_identity = { use dash_sdk::platform::Fetch; - match Identity::fetch(&self.sdk, entry.contact_id).await { + let fetched = + bounded(deadline, Identity::fetch(&self.sdk, entry.contact_id)).await; + let Some(fetched) = fetched else { + tracing::info!( + owner = %entry.owner_identity_id, contact = %entry.contact_id, + "drain: budget spent fetching the contact identity; leaving queued" + ); + continue; + }; + match fetched { Ok(Some(id)) => id, Ok(None) => { tracing::warn!( @@ -2073,7 +2155,18 @@ impl DashPayView<'_, B> { // ECDH via the Keychain-backed provider (scalar stays in the // signer; we only get the shared secret). - let shared = match provider.ecdh_shared_secret(&path, &peer).await { + // Bounded: the last step before the external-account + // registration commits. + let Some(shared) = + bounded(deadline, provider.ecdh_shared_secret(&path, &peer)).await + else { + tracing::info!( + owner = %entry.owner_identity_id, contact = %entry.contact_id, + "drain: budget spent on ECDH; leaving queued" + ); + continue; + }; + let shared = match shared { Ok(s) => s, Err(e) => { tracing::warn!( @@ -2147,10 +2240,24 @@ impl DashPayView<'_, B> { // the signer (the op carries no payload, so the latest // published version always wins). The owner-ownership / // confused-deputy guard lives in `drain_contact_info_decrypt`. - match self - .drain_contact_info_decrypt(&entry.owner_identity_id, provider) - .await - { + // Bounded as a whole: its apply runs under a single write + // lock with no await between persist and the in-memory + // mutation, so abandoning it either leaves nothing applied + // or leaves it fully applied and still queued — and a + // re-run re-fetches the latest published version anyway. + let decrypted = bounded( + deadline, + self.drain_contact_info_decrypt(&entry.owner_identity_id, provider), + ) + .await; + let Some(decrypted) = decrypted else { + tracing::info!( + owner = %entry.owner_identity_id, + "drain: budget spent decrypting contactInfo; leaving queued" + ); + continue; + }; + match decrypted { Ok(applied) => { tracing::debug!( owner = %entry.owner_identity_id, applied, @@ -2254,6 +2361,24 @@ impl DashPayView<'_, B> { /// mapping: invalid / expired / malformed / bad-index ⇒ permanent (clear); /// provider-unavailable / accept-send failure ⇒ transient (leave queued). pub async fn drain_auto_accepts(&self, signer: &S, provider: &P) -> usize + where + S: Signer + Send + Sync, + P: ContactCryptoProvider + Sync, + { + self.drain_auto_accepts_until(signer, provider, None).await + } + + /// [`Self::drain_auto_accepts`], stopping once `deadline` passes. + /// + /// Ends between entries, so a reciprocal that was sent is always recorded + /// as accepted — see [`bounded`] for why an outer timeout would not hold + /// that. Entries it never reached stay queued. + pub async fn drain_auto_accepts_until( + &self, + signer: &S, + provider: &P, + deadline: Option, + ) -> usize where S: Signer + Send + Sync, P: ContactCryptoProvider + Sync, @@ -2298,6 +2423,16 @@ impl DashPayView<'_, B> { let mut accepted: usize = 0; for entry in &entries { + // Stop between entries: an accept whose reciprocal already went out + // must reach its `cleared.push` / `accepted += 1`. + if budget_spent(deadline) { + tracing::info!( + processed = cleared.len(), + total = entries.len(), + "auto-accept: budget spent; leaving the rest queued" + ); + break; + } let owner = entry.owner_identity_id; // us (the QR owner / recipient) let sender = entry.contact_id; // the scanner (request $ownerId) @@ -2354,7 +2489,14 @@ impl DashPayView<'_, B> { continue; } }; - let pubkey = match provider.receiving_xpub(&path).await { + // Bounded: the local verify and the accept both come after this, + // so abandoning the derive commits nothing. + let Some(derived) = bounded(deadline, provider.receiving_xpub(&path)).await else { + tracing::info!(owner = %owner, sender = %sender, + "auto-accept: budget spent deriving our proof key; leaving queued"); + continue; + }; + let pubkey = match derived { Ok(xpub) => xpub.public_key, Err(e) => { tracing::warn!(owner = %owner, sender = %sender, error = %e, @@ -4450,3 +4592,53 @@ mod stamp_race_tests { ); } } + +#[cfg(test)] +mod drain_budget_tests { + use super::{bounded, budget_spent}; + use std::time::{Duration, Instant}; + + #[tokio::test] + async fn no_deadline_always_runs_the_future_to_completion() { + // The shape every pre-existing caller relies on: an unbounded drain + // must not acquire an early-exit path just because one caller wanted + // a budget. + assert!(!budget_spent(None)); + assert_eq!(bounded(None, async { 7 }).await, Some(7)); + } + + #[tokio::test] + async fn a_spent_deadline_refuses_before_polling_the_future() { + // Not merely "returns None": the future must never start, because in + // the drains it is the step that precedes a commit. + let past = Instant::now() - Duration::from_secs(1); + assert!(budget_spent(Some(past))); + + let mut polled = false; + let result = bounded(Some(past), async { + polled = true; + 7 + }) + .await; + assert_eq!(result, None); + assert!(!polled, "a spent budget must not start the work"); + } + + #[tokio::test] + async fn a_live_deadline_lets_a_finished_future_through() { + let future = Instant::now() + Duration::from_secs(30); + assert!(!budget_spent(Some(future))); + assert_eq!(bounded(Some(future), async { 7 }).await, Some(7)); + } + + #[tokio::test(start_paused = true)] + async fn a_deadline_that_passes_mid_await_abandons_the_step() { + let deadline = Instant::now() + Duration::from_millis(50); + let result = bounded(deadline.into(), async { + tokio::time::sleep(Duration::from_secs(5)).await; + 7 + }) + .await; + assert_eq!(result, None, "the step outlasted the budget"); + } +} diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs b/packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs index 0b5679fd48..2cfdd8320f 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/contacts.rs @@ -402,6 +402,17 @@ impl DashPayView<'_, B> { /// On success returns [`ExternalAccountRegistration`] so the caller can /// tell a real (re)build from an already-existed no-op — only the former /// may stamp the rotation self-heal marker (see the enum docs). + /// + /// **Never wrap this call in a timeout.** Unlike its sibling + /// [`Self::register_contact_account`], which persists inside the write lock + /// it already holds, this one persists *before* acquiring the lock — so + /// there is a genuine `.await` between the durable write and the in-memory + /// inserts. A future dropped in that window leaves an account on disk that + /// no in-memory collection knows about for the rest of the process's life; + /// a real crash reloads it from disk, an in-process cancellation does not. + /// (The next drain does re-register it idempotently, so this is a landmine + /// rather than data loss — but the asymmetry with the sibling is not + /// something to rediscover by analogy.) pub async fn register_external_contact_account( &self, our_identity_id: &Identifier, diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/WalletStorage.swift b/packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/WalletStorage.swift index 1315f49f93..2cafc50491 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/WalletStorage.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/WalletStorage.swift @@ -139,6 +139,42 @@ public class WalletStorage { /// /// Unlike `retrieveMnemonic(...)`, this does not materialize the /// mnemonic bytes into Swift heap objects. + /// Three-way answer to "can this wallet's mnemonic be read right now?". + /// + /// [`hasMnemonic(for:)`] collapses the last two cases into `false`, which + /// is fine where the question is "is this watch-only?" but wrong wherever + /// the caller has to decide between giving up and trying again later. + public enum MnemonicAvailability: Sendable, Equatable { + /// The item exists and its attributes were readable. + case present + /// The Keychain answered definitively that there is no such item — + /// a genuine watch-only wallet. + case absent + /// The lookup failed for another reason: the device is locked, access + /// was denied, the daemon was unavailable. Says nothing about whether + /// a mnemonic exists, so callers should retry rather than conclude. + case unavailable(OSStatus) + } + + /// Whether the wallet's mnemonic is readable, keeping "no such item" apart + /// from "could not tell". Attribute-only; no secret is materialized. + public func mnemonicAvailability(for walletId: Data) -> MnemonicAvailability { + let account = perWalletMnemonicAccount(for: walletId) + let query: [String: Any] = [ + kSecClass as String: kSecClassGenericPassword, + kSecAttrService as String: keychainService, + kSecAttrAccount as String: account, + kSecMatchLimit as String: kSecMatchLimitOne, + kSecReturnAttributes as String: true + ] + var result: AnyObject? + switch SecItemCopyMatching(query as CFDictionary, &result) { + case errSecSuccess: return .present + case errSecItemNotFound: return .absent + case let status: return .unavailable(status) + } + } + public func hasMnemonic(for walletId: Data) -> Bool { let account = perWalletMnemonicAccount(for: walletId) let query: [String: Any] = [ diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift index 6c866a5b8a..6bc7ef94aa 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift @@ -124,10 +124,24 @@ extension PlatformWalletManager { } let handle = self.handle - // A genuine watch-only wallet has no Keychain mnemonic; Rust then - // derives in-process and the resolver is never consulted. - let coreSigner: MnemonicResolver? = - storage.hasMnemonic(for: walletId) ? MnemonicResolver(storage: storage) : nil + // Only a definitive "no such item" means watch-only. A lookup that + // failed for another reason — locked device, denied access — must still + // hand Rust a resolver: it can then report the scan as deferred rather + // than being told the wallet has no seed at all, which would classify a + // temporary condition as terminal. + let coreSigner: MnemonicResolver? + switch storage.mnemonicAvailability(for: walletId) { + case .absent: + coreSigner = nil + case .present: + coreSigner = MnemonicResolver(storage: storage) + case .unavailable(let status): + SDKLogger.log( + "startup: mnemonic availability unknown (OSStatus \(status)); " + + "passing a resolver so the scan can defer rather than fail", + minimumLevel: .medium) + coreSigner = MnemonicResolver(storage: storage) + } // Identity signer for the DIP-15 auto-accept pass. Nil without a // SwiftData container → the drain runs provider-only, matching // `unlockWalletFromKeychain`. From eadd43b968b6b53707840f810cd6d40ce551adb2 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 11 Aug 2026 00:25:25 +0300 Subject: [PATCH 10/12] fix(platform-wallet): clear each drained entry as it lands, resolve the scan key lazily MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both as review asked for them on #4359. **The dequeue now lands with the work.** Each drain applied its whole dequeue list after the loop while committing per-entry side effects as it went, so any stop — the budget, or a caller that drops the future — left completed work still queued and reported as zero. Both drains now flush one entry's removal before starting the next, through `flush_drained_contact_crypto` (value-aware, unchanged semantics) and `flush_cleared_auto_accepts` (key-retain plus the verify-failure marks, which have to land with their entry or the sweep re-queues a proof that was already rejected). At most one entry's bookkeeping is ever in flight, and that is inherent — there is no transaction spanning a Platform round trip and a sqlite write. The deadline-driven early exit stays alongside it. Per-entry clearing alone would leave `drained` unknowable whenever an outer timeout fires, because the return value dies with the dropped future; ending the loop between entries is what keeps the returned count meaningful. Together the queue is right even if a future caller wraps a drain in a timeout anyway, which is exactly the mistake this call made. **The FFI stopped predicting when a scan happens.** It asked `has_local_identity_blocking` and resolved the master xpriv up front, which is the library's rule reimplemented in a second crate, with the JNI bridge due to be the third — and a resolver failure there became `master = None`, then a resident-key derive, then terminal `DiscoveryFailed` for what is usually a locked device. `start_wallet_subsystems` now takes `Option` — a closure invoked at most once, only on the branch that scans. The FFI drops to marshalling (handle carried as a `usize` to keep the closure `Send + Sync`), `has_local_identity_blocking` is deleted, and the library erases the key it resolved. A resolver error is classified retryable rather than terminal, and a warm launch touches no Keychain at all rather than relying on the caller to have predicted that. `wallet_network_blocking` stays: the contact-crypto provider is still built caller-side and genuinely needs the network, so it is not a prediction of library policy the way the identity gate was. cargo test -p platform-wallet --lib # 635 passed cargo test -p platform-wallet-ffi --lib # 261 passed cargo clippy --all-targets + cargo fmt --check # clean --- .../src/wallet_startup.rs | 78 +++--- .../src/manager/accessors.rs | 18 -- .../rs-platform-wallet/src/manager/startup.rs | 223 ++++++++++-------- .../identity/network/contact_requests.rs | 74 +++++- 4 files changed, 223 insertions(+), 170 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/wallet_startup.rs b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs index dd3e4e71d4..3e2e453c34 100644 --- a/packages/rs-platform-wallet-ffi/src/wallet_startup.rs +++ b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs @@ -7,7 +7,7 @@ //! [`platform_wallet::manager::startup`]; this is the marshalling shell. use platform_wallet::manager::startup::{ - ScanKey, WalletStartupOptions, WalletStartupOutcome, WalletStartupStatus, + WalletStartupOptions, WalletStartupOutcome, WalletStartupStatus, }; use rs_sdk_ffi::{MnemonicResolverHandle, SignerHandle, VTableSigner}; use std::time::Duration; @@ -145,45 +145,36 @@ pub unsafe extern "C" fn platform_wallet_manager_start_wallet_subsystems( ); }; - // Key material is only needed for a scan, and a wallet whose identity is - // already on file will not run one — so a warm launch must not pay for a - // Keychain round trip, nor be failed by one that goes wrong. Ask first. - let needs_scan_key = !PLATFORM_WALLET_MANAGER_STORAGE - .with_item(manager_handle, |m| m.has_local_identity_blocking(&wid)) - .unwrap_or(false); - - // Resolve the master xpriv once, up front. The helper holds the mnemonic - // and seed in `Zeroizing` buffers and scrubs them before returning; the - // master itself has no `Drop`, so it is erased explicitly below. - // - // A failure here is NOT returned as an FFI error — this call documents that - // only handle and wallet-id problems throw. It becomes `ScanKey::Unavailable` - // instead, so a locked device defers the scan rather than failing it - // terminally. + // The master xpriv is resolved lazily, by the library, on the branch that + // actually scans — this crate no longer predicts when that is. Erasure is + // the library's too: it resolved the key, so it clears it. // - // `None` = no key needed (warm launch) or none exists (resident-key - // wallet); `Some(Err(()))` = a scan needs one and we could not get it. - // Both no-key cases collapse to `None`: nothing to scan for, or a host that - // deliberately supplied no resolver (watch-only / resident-key wallet, where - // discovery derives in-process). Neither is a failure to obtain a key. - let mut master: Option> = if !needs_scan_key || mnemonic_resolver_handle.is_null() - { - None - } else { - Some( - match resolve_master_from_resolver(mnemonic_resolver_handle, &wid, network) { - Ok(master) => Ok(master), - Err(_) => { - tracing::warn!( - wallet_id = %hex::encode(wid), - "startup: could not resolve the wallet mnemonic; deferring the \ - identity scan to a later start" - ); - Err(()) - } - }, - ) + // The handle is carried as a `usize` so the closure stays `Send + Sync`; + // it is valid for the duration of this call, which is the only time the + // closure can run. A null resolver means the host is telling us the wallet + // holds resident keys (watch-only / in-process derive), so no resolver is + // supplied at all — as opposed to one that fails, which the library treats + // as a retryable "come back later". + let resolver_addr = mnemonic_resolver_handle as usize; + let resolve_scan_key = move || { + resolve_master_from_resolver(resolver_addr as *mut MnemonicResolverHandle, &wid, network) + .map_err(|e| { + tracing::warn!( + wallet_id = %hex::encode(wid), + code = ?e.code, + "startup: could not resolve the wallet mnemonic for the identity scan" + ); + // Carried as an error the library can classify, not as an FFI + // throw: the documented contract is that only handle and + // wallet-id problems throw, and a locked device is neither. + platform_wallet::error::PlatformWalletError::KeyDerivation(format!( + "mnemonic resolver failed ({:?})", + e.code + )) + }) }; + let scan_key = (!mnemonic_resolver_handle.is_null()) + .then_some(&resolve_scan_key as &(dyn Fn() -> Result<_, _> + Send + Sync)); let signer_addr = if identity_signer_handle.is_null() { 0usize @@ -211,12 +202,6 @@ pub unsafe extern "C" fn platform_wallet_manager_start_wallet_subsystems( // so the future cannot satisfy `block_on_worker`'s `'static` bound. The // scoped thread also supplies the 8 MB stack the GroveDB proof // verification inside discovery needs. - let scan_key = match master.as_ref() { - Some(Ok(master)) => ScanKey::Master(master), - Some(Err(())) => ScanKey::Unavailable, - None => ScanKey::Resident, - }; - let result = run_on_big_stack_thread(|| { PLATFORM_WALLET_MANAGER_STORAGE.with_item(manager_handle, |manager| { crate::runtime::runtime().block_on(async { @@ -229,11 +214,6 @@ pub unsafe extern "C" fn platform_wallet_manager_start_wallet_subsystems( }) }); - // Erase the master before any early return below. - if let Some(Ok(master)) = master.as_mut() { - master.private_key.non_secure_erase(); - } - let outcome = match result { Ok(Some(Ok(outcome))) => outcome, Ok(Some(Err(e))) => return PlatformWalletFFIResult::from(e), diff --git a/packages/rs-platform-wallet/src/manager/accessors.rs b/packages/rs-platform-wallet/src/manager/accessors.rs index c766449fb1..7b9c642282 100644 --- a/packages/rs-platform-wallet/src/manager/accessors.rs +++ b/packages/rs-platform-wallet/src/manager/accessors.rs @@ -458,24 +458,6 @@ impl PlatformWalletManager

{ Some(wm.get_wallet_info(wallet_id)?.core_wallet.network()) } - /// Whether this wallet already has an identity on file locally. - /// - /// Lets a caller skip work that only an identity scan would need — most - /// importantly resolving key material, which on iOS means a Keychain round - /// trip that a warm launch should not pay for. Blocking and cheap: one - /// `RwLock` read, no I/O. - pub fn has_local_identity_blocking(&self, wallet_id: &WalletId) -> bool { - let wm = self.wallet_manager.blocking_read(); - wm.get_wallet_info(wallet_id) - .map(|info| { - !info - .identity_manager - .wallet_identity_ids(wallet_id) - .is_empty() - }) - .unwrap_or(false) - } - /// Snapshot of [`PlatformAddressSyncManager`] tunables and last- /// pass timestamp. `watch_list_size` is `wallets.len()` — every /// registered wallet participates in each pass since the sync diff --git a/packages/rs-platform-wallet/src/manager/startup.rs b/packages/rs-platform-wallet/src/manager/startup.rs index 85d17ed22b..3524da3838 100644 --- a/packages/rs-platform-wallet/src/manager/startup.rs +++ b/packages/rs-platform-wallet/src/manager/startup.rs @@ -26,10 +26,12 @@ //! //! # Key material //! -//! Per-call, never resident. The caller resolves the master xpriv and the -//! contact-crypto provider for exactly this call and drops them after — the -//! same contract [`crate::wallet::identity::IdentityWallet::discover_from_master`] -//! and the drain already use. Making the unattended sweep self-sufficient +//! Per-call, never resident. The contact-crypto provider is built for exactly +//! this call and dropped after — the same contract the drain already uses. The +//! master xpriv is not even that: the caller hands over a [`ScanKeyResolver`] +//! and this module invokes it only on the branch that scans, so a launch with +//! nothing to discover never touches the Keychain at all. Making the unattended +//! sweep self-sufficient //! instead would turn a narrowly-scoped Keychain capability into a standing //! one, which is a security-posture change and deliberately not what this is. @@ -84,24 +86,25 @@ async fn within_budget(deadline: Instant, future: F) -> tokio::time::timeout(remaining, future).await.ok() } -/// Where an identity scan gets its key material — and, importantly, the -/// difference between not needing any and not being able to get it. +/// Produces the master xpriv an identity scan needs, on demand. /// -/// Collapsing the last case into "no master supplied" makes a temporary -/// problem look permanent: a Keychain-backed wallet would fall through to the -/// resident-key derive, fail because it deliberately holds no private key, and -/// be classified [`WalletStartupStatus::DiscoveryFailed`] — which tells the -/// client never to try again over a device that was merely locked. -pub enum ScanKey<'a> { - /// The wallet holds resident keys; discovery derives in-process. - Resident, - /// The caller resolved a master xpriv for this call. - Master(&'a ExtendedPrivKey), - /// Key material was needed but could not be obtained right now — a locked - /// device, a denied Keychain read, a transient failure. Discovery is - /// skipped and reported as retryable rather than attempted and failed. - Unavailable, -} +/// **Lazy is the point.** Which branches scan is this module's decision — a +/// wallet with an identity on file does not — and a caller that resolves up +/// front has to predict that decision to avoid paying for it. On iOS resolving +/// means a Keychain round trip, at worst behind a biometric prompt, so a warm +/// launch would pay for a key it never uses, and pay outside the budget it is +/// about to be measured against. Handing the sequence a closure instead keeps +/// the rule in one crate: the FFI marshals, the JNI bridge inherits the +/// behaviour rather than reimplementing it, and a future change to when a scan +/// happens cannot silently starve one of them of key material. +/// +/// `None` means the wallet holds resident keys and discovery derives +/// in-process. An `Err` means a scan needed a key and could not get one right +/// now — a locked device, a denied Keychain read — which is reported as +/// retryable, never as the terminal [`WalletStartupStatus::DiscoveryFailed`] +/// that an attempted-and-failed derive would produce. +pub type ScanKeyResolver<'a> = + &'a (dyn Fn() -> Result + Send + Sync); /// Knobs for [`PlatformWalletManager::start_wallet_subsystems`]. #[derive(Debug, Clone, Copy)] @@ -348,13 +351,17 @@ impl PlatformWalletManager /// # Key material /// /// `scan_key` and `contact_crypto` are borrowed for this call only and must - /// not be retained by the caller afterwards. See [`ScanKey`] for why - /// "unavailable" is spelled out rather than folded into "none". + /// not be retained by the caller afterwards. `scan_key` is invoked at most + /// once, and only on the branch that actually scans — see + /// [`ScanKeyResolver`] for why the caller must not resolve it up front. /// `identity_signer` is `None` to skip the DIP-15 auto-accept pass. + /// + /// The resolved key never outlives this call: it is erased before + /// discovery returns. pub async fn start_wallet_subsystems( &self, wallet_id: &WalletId, - scan_key: ScanKey<'_>, + scan_key: Option>, contact_crypto: Option<&C>, identity_signer: Option<&S>, opts: WalletStartupOptions, @@ -512,98 +519,118 @@ impl PlatformWalletManager &self, wallet_id: &WalletId, identity_wallet: &crate::wallet::identity::IdentityWallet, - scan_key: ScanKey<'_>, + scan_key: Option>, gap_limit: Option, deadline: Instant, tally: &mut StartupTally, ) { - // No key, no scan — but this is a "come back later", not a failure. - // Attempting it anyway would derive against a wallet that holds no - // private key, and the resulting error would be classified terminal. - if matches!(scan_key, ScanKey::Unavailable) { - tracing::info!( - "startup: scan key material unavailable; deferring discovery to a later start" - ); - tally.record_unreachable(); - tally.record_discovery_gave_up(); - return; - } + // This is the branch that scans, so this is where the key gets + // resolved — not one moment earlier. A failure here is "come back + // later", never a verdict: the realistic causes are a locked device or + // a denied Keychain read, and deriving anyway against a wallet that + // holds no private key would fail locally and be classified terminal, + // telling the client to stop trying over a condition that clears + // itself on the next unlock. + let mut master = match scan_key { + None => None, + Some(resolve) => match resolve() { + Ok(master) => Some(master), + Err(e) => { + tracing::warn!( + error = %e, + "startup: scan key material unavailable; deferring discovery to a \ + later start" + ); + tally.record_unreachable(); + tally.record_discovery_gave_up(); + return; + } + }, + }; let opts = IdentityDiscoveryOptions { start_index: Some(0), gap_limit: gap_limit.unwrap_or(IdentityDiscoveryOptions::default().gap_limit), }; - for (attempt, backoff) in DISCOVERY_BACKOFF.iter().map(Some).chain([None]).enumerate() { - // A single scan walks up to `gap_limit` indices, each a Platform - // fetch plus a DPNS lookup, so it needs the same ceiling the other - // steps have — otherwise one attempt can outlast the whole budget - // this call promises. - let attempt_future = async { - match scan_key { - ScanKey::Master(master) => { - identity_wallet.discover_from_master(opts, master).await + // Scoped so every exit path — including the early returns inside the + // loop — lands on the erase below. `return` leaves this block, not the + // function, so the tally bookkeeping keeps its existing shape. + async { + for (attempt, backoff) in DISCOVERY_BACKOFF.iter().map(Some).chain([None]).enumerate() { + // A single scan walks up to `gap_limit` indices, each a Platform + // fetch plus a DPNS lookup, so it needs the same ceiling the other + // steps have — otherwise one attempt can outlast the whole budget + // this call promises. + let attempt_future = async { + match master.as_ref() { + Some(master) => identity_wallet.discover_from_master(opts, master).await, + None => identity_wallet.discover(opts).await, } - ScanKey::Resident => identity_wallet.discover(opts).await, - // Unreachable: guarded before the loop. - ScanKey::Unavailable => unreachable!(), - } - }; - let Some(result) = within_budget(deadline, attempt_future).await else { - // Sightings persist incrementally, so an abandoned scan may - // still have folded an identity in before it was cut off. - if let Some(known) = self.local_identity_id(wallet_id).await { - tally.record_local_identity(known); - return; - } - break; - }; - - match result { - Ok(found) => { - match found.first() { - Some(identity) => tally.record_discovered(identity.id()), - // An empty return is not proof on its own: `discover` - // reports only identities THIS call inserted, so a - // concurrent startup that inserted one first leaves us - // seeing it as already-managed and returning nothing. - // Consult local state before calling it absence. - None => match self.local_identity_id(wallet_id).await { - Some(known) => tally.record_discovered(known), - None => tally.record_proven_absent(), - }, + }; + let Some(result) = within_budget(deadline, attempt_future).await else { + // Sightings persist incrementally, so an abandoned scan may + // still have folded an identity in before it was cut off. + if let Some(known) = self.local_identity_id(wallet_id).await { + tally.record_local_identity(known); + return; + } + break; + }; + + match result { + Ok(found) => { + match found.first() { + Some(identity) => tally.record_discovered(identity.id()), + // An empty return is not proof on its own: `discover` + // reports only identities THIS call inserted, so a + // concurrent startup that inserted one first leaves us + // seeing it as already-managed and returning nothing. + // Consult local state before calling it absence. + None => match self.local_identity_id(wallet_id).await { + Some(known) => tally.record_discovered(known), + None => tally.record_proven_absent(), + }, + } + return; + } + Err(PlatformWalletError::IdentityDiscoveryIncomplete { .. }) => { + tally.record_unreachable(); + } + Err(e) => { + // Not a reachability question — a wallet/persistence + // failure will not fix itself on the next attempt, so this + // is recorded as terminal rather than as "try again". + tracing::warn!( + error = %e, + "startup: identity discovery failed for a non-network reason" + ); + tally.record_discovery_failed_locally(); + return; } - return; - } - Err(PlatformWalletError::IdentityDiscoveryIncomplete { .. }) => { - tally.record_unreachable(); } - Err(e) => { - // Not a reachability question — a wallet/persistence - // failure will not fix itself on the next attempt, so this - // is recorded as terminal rather than as "try again". - tracing::warn!( - error = %e, - "startup: identity discovery failed for a non-network reason" - ); - tally.record_discovery_failed_locally(); - return; + + let Some(backoff) = backoff else { break }; + let remaining = deadline.saturating_duration_since(Instant::now()); + if remaining.is_zero() { + break; } + tracing::info!( + attempt = attempt + 1, + "startup: identity discovery could not reach Platform; retrying" + ); + tokio::time::sleep((*backoff).min(remaining)).await; } - let Some(backoff) = backoff else { break }; - let remaining = deadline.saturating_duration_since(Instant::now()); - if remaining.is_zero() { - break; - } - tracing::info!( - attempt = attempt + 1, - "startup: identity discovery could not reach Platform; retrying" - ); - tokio::time::sleep((*backoff).min(remaining)).await; + tally.record_discovery_gave_up(); } + .await; - tally.record_discovery_gave_up(); + // The key lives no longer than the scan that needed it. `ExtendedPrivKey` + // has no `Drop`, so this is the only thing that clears it. + if let Some(master) = master.as_mut() { + master.private_key.non_secure_erase(); + } } } diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs b/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs index 3c2dbe6549..ad74f80ebd 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs @@ -1924,7 +1924,19 @@ impl DashPayView<'_, B> { } let mut cleared: Vec = Vec::new(); + // How much of `cleared` is already dequeued + persisted, and the running + // total actually removed. Bookkeeping lands per entry, so at most one + // entry's worth can ever be in flight. + let mut flushed: usize = 0; + let mut drained_total: usize = 0; for entry in &entries { + // Land the previous entry's dequeue before starting any new work. + if cleared.len() > flushed { + drained_total += self + .flush_drained_contact_crypto(&entries, &cleared[flushed..]) + .await; + flushed = cleared.len(); + } // Stop between entries, never inside one: an entry that has already // committed its side effect must reach the `cleared.push` that // records it. @@ -2283,6 +2295,29 @@ impl DashPayView<'_, B> { } } + // Land whatever the last entry completed. + drained_total += self + .flush_drained_contact_crypto(&entries, &cleared[flushed..]) + .await; + + drained_total + } + + /// Apply the dequeue for entries a drain just completed: remove them from + /// their owners' in-memory queues and persist the removal. Returns how many + /// were actually removed. + /// + /// Called after **each** completed entry rather than once per drain. The + /// per-entry side effects (`register_contact_account`, the reciprocal send) + /// commit as the loop runs, so batching the bookkeeping to the end means a + /// drain that stops — its budget, or a caller that drops the future — + /// leaves work it really finished still queued and reported as zero. One + /// entry's work and one entry's dequeue now land together. + async fn flush_drained_contact_crypto( + &self, + entries: &[crate::changeset::PendingContactCrypto], + cleared: &[crate::changeset::PendingContactCryptoKey], + ) -> usize { if cleared.is_empty() { return 0; } @@ -2303,7 +2338,7 @@ impl DashPayView<'_, B> { // entry still value-equal to the snapshot's — a mid-drain upsert // (changed payload) is left queued for the next drain rather than // clobbered by this stale snapshot. - let removed: Vec = { + let removed: Vec = { let mut wm = self.wallet_manager.write().await; match wm.get_wallet_info_mut(&self.wallet_id) { Some(info) => { @@ -2421,8 +2456,22 @@ impl DashPayView<'_, B> { // failure) are NOT pushed here so they stay retryable. let mut verify_failed: Vec<(Identifier, Identifier, Vec)> = Vec::new(); let mut accepted: usize = 0; + // How much of each has already been applied — see the loop head. + let mut flushed: usize = 0; + let mut verify_flushed: usize = 0; for entry in &entries { + // Land the previous entry's dequeue + verify-failure marks before + // starting new work, so a stop can strand at most one entry's. + if cleared.len() > flushed || verify_failed.len() > verify_flushed { + self.flush_cleared_auto_accepts( + &cleared[flushed..], + &verify_failed[verify_flushed..], + ) + .await; + flushed = cleared.len(); + verify_flushed = verify_failed.len(); + } // Stop between entries: an accept whose reciprocal already went out // must reach its `cleared.push` / `accepted += 1`. if budget_spent(deadline) { @@ -2535,6 +2584,23 @@ impl DashPayView<'_, B> { } } + // Land whatever the last entry resolved. + self.flush_cleared_auto_accepts(&cleared[flushed..], &verify_failed[verify_flushed..]) + .await; + + accepted + } + + /// Dequeue the `AutoAccept` entries a drain just resolved and record the + /// permanent verify failures among them. Called after each resolved entry, + /// for the same reason as [`Self::flush_drained_contact_crypto`]: a + /// reciprocal that has already been broadcast must not be able to end up + /// still queued because the drain stopped before its bookkeeping ran. + async fn flush_cleared_auto_accepts( + &self, + cleared: &[crate::changeset::PendingContactCryptoKey], + verify_failed: &[(Identifier, Identifier, Vec)], + ) { if !cleared.is_empty() { { let mut wm = self.wallet_manager.write().await; @@ -2562,7 +2628,7 @@ impl DashPayView<'_, B> { // enqueue gate skips the same bad proof (in-memory only — // retried once per launch; the request stays manually // acceptable). - for (owner, sender, proof) in &verify_failed { + for (owner, sender, proof) in verify_failed { if let Some(managed) = info.identity_manager.managed_identity_mut(owner) { managed.mark_auto_accept_verify_failed(sender, proof); } @@ -2570,7 +2636,7 @@ impl DashPayView<'_, B> { } } let changeset = crate::changeset::PlatformWalletChangeSet { - pending_contact_crypto_cleared: cleared, + pending_contact_crypto_cleared: cleared.to_vec(), ..Default::default() }; if let Err(e) = self.persister.store(changeset) { @@ -2578,8 +2644,6 @@ impl DashPayView<'_, B> { "auto-accept: failed to persist cleared entries (in-memory already updated)"); } } - - accepted } /// Build a DIP-15 auto-accept QR URI (`dash:?du=&dapk=`), From cd91af1e9346672e6dcfbba2c849b057693d38c1 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 11 Aug 2026 00:44:46 +0300 Subject: [PATCH 11/12] fix(platform-wallet): apply the two auto-accept lists independently MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit From CodeRabbit on #4359. Splitting the end-of-drain apply block into a per-entry flush carried its outer `if !cleared.is_empty()` guard over the verify-failure marking too. The caller advances both cursors after the helper returns, so a call with marks and no dequeue would drop those marks for good — the next sweep would then re-queue a proof already proven bad. Unreachable today, since every `verify_failed` push is paired with a `cleared` push, but it is a trap the previous shape did not have: there the two always arrived together by construction. Each list is now applied on its own, and only the dequeue is persisted — the marks are in-memory by design, retried once per launch. Also on the Swift side: `mnemonicAvailability` was inserted between `hasMnemonic`'s doc comment and `hasMnemonic` itself, leaving the "cheap existence check" paragraph documenting the enum. Reattached, and `hasMnemonic` now delegates rather than repeating the query, so the two answers cannot drift apart. --- .../identity/network/contact_requests.rs | 55 ++++++++++++------- .../Core/Wallet/WalletStorage.swift | 24 +++----- 2 files changed, 44 insertions(+), 35 deletions(-) diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs b/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs index ad74f80ebd..ec6eda0072 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/contact_requests.rs @@ -2601,10 +2601,19 @@ impl DashPayView<'_, B> { cleared: &[crate::changeset::PendingContactCryptoKey], verify_failed: &[(Identifier, Identifier, Vec)], ) { - if !cleared.is_empty() { - { - let mut wm = self.wallet_manager.write().await; - if let Some(info) = wm.get_wallet_info_mut(&self.wallet_id) { + // Each list is applied on its own. The caller advances both cursors + // after this returns, so gating one on the other would discard the + // ungated one for good. Not reachable today — every `verify_failed` + // push is paired with a `cleared` push — but a future "leave queued, + // remember the bad proof" case would lose its marks silently. + if cleared.is_empty() && verify_failed.is_empty() { + return; + } + + { + let mut wm = self.wallet_manager.write().await; + if let Some(info) = wm.get_wallet_info_mut(&self.wallet_id) { + if !cleared.is_empty() { // Remove the cleared entries from their owners' queues. Each // cleared key names its owner, and only that owner's queue can // hold it, so retain each affected owner's queue against the @@ -2624,25 +2633,31 @@ impl DashPayView<'_, B> { .retain(|e| !cleared.iter().any(|k| *k == e.key())); } } - // Record permanent verify failures so the next sweep's - // enqueue gate skips the same bad proof (in-memory only — - // retried once per launch; the request stays manually - // acceptable). - for (owner, sender, proof) in verify_failed { - if let Some(managed) = info.identity_manager.managed_identity_mut(owner) { - managed.mark_auto_accept_verify_failed(sender, proof); - } + } + // Record permanent verify failures so the next sweep's + // enqueue gate skips the same bad proof (in-memory only — + // retried once per launch; the request stays manually + // acceptable). + for (owner, sender, proof) in verify_failed { + if let Some(managed) = info.identity_manager.managed_identity_mut(owner) { + managed.mark_auto_accept_verify_failed(sender, proof); } } } - let changeset = crate::changeset::PlatformWalletChangeSet { - pending_contact_crypto_cleared: cleared.to_vec(), - ..Default::default() - }; - if let Err(e) = self.persister.store(changeset) { - tracing::warn!(error = %e, - "auto-accept: failed to persist cleared entries (in-memory already updated)"); - } + } + + // Only the dequeue is persisted; the verify-failure marks are in-memory + // by design. + if cleared.is_empty() { + return; + } + let changeset = crate::changeset::PlatformWalletChangeSet { + pending_contact_crypto_cleared: cleared.to_vec(), + ..Default::default() + }; + if let Err(e) = self.persister.store(changeset) { + tracing::warn!(error = %e, + "auto-accept: failed to persist cleared entries (in-memory already updated)"); } } diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/WalletStorage.swift b/packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/WalletStorage.swift index 2cafc50491..e468cc4440 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/WalletStorage.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/Core/Wallet/WalletStorage.swift @@ -135,10 +135,6 @@ public class WalletStorage { return mnemonic } - /// Cheap existence check used by signer preflight paths. - /// - /// Unlike `retrieveMnemonic(...)`, this does not materialize the - /// mnemonic bytes into Swift heap objects. /// Three-way answer to "can this wallet's mnemonic be read right now?". /// /// [`hasMnemonic(for:)`] collapses the last two cases into `false`, which @@ -175,18 +171,16 @@ public class WalletStorage { } } + /// Cheap existence check used by signer preflight paths. + /// + /// Unlike `retrieveMnemonic(...)`, this does not materialize the + /// mnemonic bytes into Swift heap objects. + /// + /// Answers `false` both for "no such item" and for "could not tell", + /// which is what a preflight wants. A caller that has to choose between + /// giving up and retrying needs `mnemonicAvailability(for:)` instead. public func hasMnemonic(for walletId: Data) -> Bool { - let account = perWalletMnemonicAccount(for: walletId) - let query: [String: Any] = [ - kSecClass as String: kSecClassGenericPassword, - kSecAttrService as String: keychainService, - kSecAttrAccount as String: account, - kSecMatchLimit as String: kSecMatchLimitOne, - kSecReturnAttributes as String: true - ] - var result: AnyObject? - let status = SecItemCopyMatching(query as CFDictionary, &result) - return status == errSecSuccess + mnemonicAvailability(for: walletId) == .present } /// Attribute-only identity stamp of the wallet's mnemonic Keychain From 58a43aaf4721cfa3a688ca46c0266bd2988f1696 Mon Sep 17 00:00:00 2001 From: Roman <51091564+jeanpierreroma@users.noreply.github.com> Date: Tue, 11 Aug 2026 01:47:04 +0300 Subject: [PATCH 12/12] fix(platform-wallet): erase the scan key by lifetime, and keep its failure's permanence MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both from review on #4359. **The erase was on a code path, and this call invites cancellation.** Moving erasure from the FFI into the library left it as an explicit `non_secure_erase` after the discovery loop — reached by every return the loop makes, and by none at all if the caller drops the whole `start_wallet_subsystems` future while a scan is awaiting Platform. `ExtendedPrivKey` has no erasing `Drop`, so the scalar would stay in the freed future. It is now owned by a `ScanKeyGuard` that erases on drop, which also lets the scan loop go back to its plain shape: the `async` block existed only to funnel returns onto the explicit call. **A corrupt phrase was reported as something the next launch might fix.** Every resolver failure became one `KeyDerivation`, and startup maps any resolver error to retryable `PartialNoIdentity`. Right for a locked device; wrong for invalid UTF-8, a phrase in no BIP-39 wordlist, an impossible buffer length, or a seed that yields no master key — none of which a retry edits. The FFI result codes cannot carry this: every one of those causes lands on `ErrorWalletOperation`. So the classification is captured where it is produced — `ResolveFailureKind::{Unavailable, Permanent}` on a `ResolveFailure`, returned by `_classified` variants of the two resolver helpers. The existing helpers become thin wrappers that drop the kind, so the ~10 other call sites are untouched. The library side is `ScanKeyError::{Unavailable, Invalid}`: unavailable keeps the retryable path, invalid records a local discovery failure and settles as terminal `DiscoveryFailed`. `NOT_FOUND` is deliberately retryable rather than permanent: the host filters watch-only wallets before calling, so reaching it means the item was expected and was not there — a wipe/restore race, not proof the wallet has no seed. 4 tests drive the classification through a fake resolver vtable, one per cause. Getting the mapping backwards is exactly the kind of thing that reads fine and behaves wrong, in opposite directions. cargo test -p platform-wallet --lib # 635 passed cargo test -p platform-wallet-ffi --lib # 265 passed (4 new) cargo clippy --all-targets + cargo fmt --check # clean --- .../src/identity_keys_from_mnemonic.rs | 218 ++++++++++++++++-- .../src/wallet_startup.rs | 48 ++-- .../rs-platform-wallet/src/manager/startup.rs | 218 +++++++++++------- 3 files changed, 365 insertions(+), 119 deletions(-) diff --git a/packages/rs-platform-wallet-ffi/src/identity_keys_from_mnemonic.rs b/packages/rs-platform-wallet-ffi/src/identity_keys_from_mnemonic.rs index 47d3e36608..ef4ad2bf93 100644 --- a/packages/rs-platform-wallet-ffi/src/identity_keys_from_mnemonic.rs +++ b/packages/rs-platform-wallet-ffi/src/identity_keys_from_mnemonic.rs @@ -104,12 +104,68 @@ pub(crate) unsafe fn resolve_master_from_resolver( wallet_id: &[u8; 32], network: Network, ) -> Result { - let seed = resolve_seed_from_resolver(mnemonic_resolver_handle, wallet_id)?; + resolve_master_from_resolver_classified(mnemonic_resolver_handle, wallet_id, network) + .map_err(|failure| failure.result) +} + +/// Whether a resolve failure is worth asking about again. +/// +/// The FFI result codes cannot carry this: every cause below the callback — +/// a locked Keychain, a phrase that is not BIP-39, bytes that are not UTF-8 — +/// lands on `ErrorWalletOperation`. The distinction exists at the point the +/// failure is produced and is lost immediately after, so it is captured here +/// rather than re-derived from a message downstream. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub(crate) enum ResolveFailureKind { + /// The mnemonic is presumed to exist; this attempt could not read it. + /// A locked device, a denied or cancelled Keychain read, a resolver that + /// was not ready. The next launch may get a different answer. + Unavailable, + /// What is stored cannot produce a key, however many times it is read: + /// invalid UTF-8, a phrase in no supported wordlist, a length the buffer + /// contract forbids, a seed no master key can be built from. + Permanent, +} + +/// A resolve failure with its retry classification attached. +pub(crate) struct ResolveFailure { + pub kind: ResolveFailureKind, + pub result: PlatformWalletFFIResult, +} + +impl ResolveFailure { + fn unavailable(result: PlatformWalletFFIResult) -> Self { + Self { + kind: ResolveFailureKind::Unavailable, + result, + } + } + + fn permanent(result: PlatformWalletFFIResult) -> Self { + Self { + kind: ResolveFailureKind::Permanent, + result, + } + } +} + +/// [`resolve_master_from_resolver`], keeping the retry classification. +/// +/// # Safety +/// Same contract as [`resolve_master_from_resolver`]. +pub(crate) unsafe fn resolve_master_from_resolver_classified( + mnemonic_resolver_handle: *mut rs_sdk_ffi::MnemonicResolverHandle, + wallet_id: &[u8; 32], + network: Network, +) -> Result { + let seed = resolve_seed_from_resolver_classified(mnemonic_resolver_handle, wallet_id)?; + // A seed that cannot yield a master key is the stored material being + // wrong, not the moment being wrong. ExtendedPrivKey::new_master(network, seed.as_ref()).map_err(|e| { - PlatformWalletFFIResult::err( + ResolveFailure::permanent(PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorWalletOperation, format!("failed to build master xpriv from resolved mnemonic: {e}"), - ) + )) }) } @@ -133,6 +189,19 @@ pub(crate) unsafe fn resolve_seed_from_resolver( mnemonic_resolver_handle: *mut rs_sdk_ffi::MnemonicResolverHandle, wallet_id: &[u8; 32], ) -> Result, PlatformWalletFFIResult> { + resolve_seed_from_resolver_classified(mnemonic_resolver_handle, wallet_id) + .map_err(|failure| failure.result) +} + +/// [`resolve_seed_from_resolver`], keeping the retry classification. See +/// [`ResolveFailureKind`] for why the codes alone cannot express it. +/// +/// # Safety +/// Same contract as [`resolve_seed_from_resolver`]. +pub(crate) unsafe fn resolve_seed_from_resolver_classified( + mnemonic_resolver_handle: *mut rs_sdk_ffi::MnemonicResolverHandle, + wallet_id: &[u8; 32], +) -> Result, ResolveFailure> { use rs_sdk_ffi::{mnemonic_resolver_result, MNEMONIC_RESOLVER_BUFFER_CAPACITY}; use std::ffi::c_void; @@ -152,44 +221,49 @@ pub(crate) unsafe fn resolve_seed_from_resolver( match rc { x if x == mnemonic_resolver_result::SUCCESS => {} x if x == mnemonic_resolver_result::NOT_FOUND => { - return Err(PlatformWalletFFIResult::err( + // Not permanent: the host filters watch-only wallets before + // calling, so reaching this means the item was expected and was + // not readable — including the wipe/restore race. + return Err(ResolveFailure::unavailable(PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorWalletOperation, "mnemonic resolver: no mnemonic stored for the supplied wallet_id", - )); + ))); } x if x == mnemonic_resolver_result::BUFFER_TOO_SMALL => { - return Err(PlatformWalletFFIResult::err( + return Err(ResolveFailure::permanent(PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorWalletOperation, "mnemonic resolver: mnemonic exceeded the FFI buffer capacity", - )); + ))); } _ => { - return Err(PlatformWalletFFIResult::err( + // The Keychain bucket: locked device, denied or cancelled prompt, + // daemon unavailable. Retryable by nature. + return Err(ResolveFailure::unavailable(PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorWalletOperation, "mnemonic resolver: failed (other / Keychain access error)", - )); + ))); } } if mnemonic_len == 0 || mnemonic_len > MNEMONIC_RESOLVER_BUFFER_CAPACITY { - return Err(PlatformWalletFFIResult::err( + return Err(ResolveFailure::permanent(PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorWalletOperation, "mnemonic resolver: returned invalid length", - )); + ))); } // Validate UTF-8 over the resolver-claimed prefix only — never // build a `String` (Swift's can't be zeroized; ours can). let mnemonic_str = std::str::from_utf8(&mnemonic_buf[..mnemonic_len]).map_err(|e| { - PlatformWalletFFIResult::err( + ResolveFailure::permanent(PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorUtf8Conversion, format!("mnemonic resolver: returned invalid UTF-8: {e}"), - ) + )) })?; let mnemonic = parse_mnemonic_any_language(mnemonic_str).map_err(|e| { - PlatformWalletFFIResult::err( + ResolveFailure::permanent(PlatformWalletFFIResult::err( PlatformWalletFFIResultCode::ErrorWalletOperation, format!("mnemonic resolver: returned an invalid mnemonic: {e}"), - ) + )) })?; let seed: Zeroizing<[u8; 64]> = Zeroizing::new(mnemonic.to_seed("")); @@ -520,3 +594,117 @@ mod tests { assert_eq!(out.count, 0); } } + +/// The classification the startup path depends on: a resolver that could not +/// read must not look like one that read something unusable, and vice versa. +/// Both arrive as the same FFI result code, so only [`ResolveFailureKind`] +/// carries the difference — and getting it backwards either tells a client to +/// stop over a locked device, or to retry forever over a corrupt phrase. +#[cfg(test)] +mod resolve_classification_tests { + use super::*; + use rs_sdk_ffi::{ + mnemonic_resolver_result, MnemonicResolverHandle, MnemonicResolverVTable, + MNEMONIC_RESOLVER_BUFFER_CAPACITY, + }; + use std::os::raw::{c_char, c_void}; + + /// Resolver that always reports "no mnemonic for this wallet". + unsafe extern "C" fn resolve_not_found( + _ctx: *const c_void, + _wallet_id: *const u8, + _out: *mut c_char, + _cap: usize, + _out_len: *mut usize, + ) -> i32 { + mnemonic_resolver_result::NOT_FOUND + } + + /// Resolver standing in for a locked device / denied Keychain read. + unsafe extern "C" fn resolve_keychain_error( + _ctx: *const c_void, + _wallet_id: *const u8, + _out: *mut c_char, + _cap: usize, + _out_len: *mut usize, + ) -> i32 { + mnemonic_resolver_result::OTHER + } + + /// Resolver that answers successfully with a phrase in no wordlist. + unsafe extern "C" fn resolve_garbage_phrase( + _ctx: *const c_void, + _wallet_id: *const u8, + out: *mut c_char, + cap: usize, + out_len: *mut usize, + ) -> i32 { + let phrase = b"not a bip39 phrase at all"; + assert!(cap >= phrase.len()); + std::ptr::copy_nonoverlapping(phrase.as_ptr(), out as *mut u8, phrase.len()); + *out_len = phrase.len(); + mnemonic_resolver_result::SUCCESS + } + + /// Resolver that claims a length the buffer contract forbids. + unsafe extern "C" fn resolve_absurd_length( + _ctx: *const c_void, + _wallet_id: *const u8, + _out: *mut c_char, + _cap: usize, + out_len: *mut usize, + ) -> i32 { + *out_len = MNEMONIC_RESOLVER_BUFFER_CAPACITY + 1; + mnemonic_resolver_result::SUCCESS + } + + unsafe extern "C" fn destroy_noop(_ctx: *mut c_void) {} + + fn classify(resolve: rs_sdk_ffi::MnemonicResolveCallback) -> ResolveFailureKind { + let mut vtable = MnemonicResolverVTable { + resolve, + destroy: destroy_noop, + }; + let mut handle = MnemonicResolverHandle { + ctx: std::ptr::null_mut(), + vtable: &mut vtable, + }; + let failure = unsafe { resolve_seed_from_resolver_classified(&mut handle, &[9u8; 32]) } + .expect_err("this resolver never succeeds"); + failure.kind + } + + #[test] + fn a_keychain_that_would_not_answer_is_retryable() { + assert_eq!( + classify(resolve_keychain_error), + ResolveFailureKind::Unavailable, + "a locked or denied Keychain read says nothing about what is stored" + ); + } + + #[test] + fn a_missing_item_is_retryable_not_a_verdict() { + // The host filters watch-only wallets before calling, so reaching + // NOT_FOUND means the item was expected — a wipe/restore race, not + // proof that this wallet has no seed. + assert_eq!(classify(resolve_not_found), ResolveFailureKind::Unavailable); + } + + #[test] + fn a_phrase_in_no_wordlist_is_permanent() { + assert_eq!( + classify(resolve_garbage_phrase), + ResolveFailureKind::Permanent, + "re-reading the same bytes cannot make them a valid mnemonic" + ); + } + + #[test] + fn an_impossible_length_is_permanent() { + assert_eq!( + classify(resolve_absurd_length), + ResolveFailureKind::Permanent + ); + } +} diff --git a/packages/rs-platform-wallet-ffi/src/wallet_startup.rs b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs index 3e2e453c34..583dfc11bb 100644 --- a/packages/rs-platform-wallet-ffi/src/wallet_startup.rs +++ b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs @@ -7,7 +7,7 @@ //! [`platform_wallet::manager::startup`]; this is the marshalling shell. use platform_wallet::manager::startup::{ - WalletStartupOptions, WalletStartupOutcome, WalletStartupStatus, + ScanKeyError, WalletStartupOptions, WalletStartupOutcome, WalletStartupStatus, }; use rs_sdk_ffi::{MnemonicResolverHandle, SignerHandle, VTableSigner}; use std::time::Duration; @@ -16,7 +16,9 @@ use crate::check_ptr; use crate::dashpay::resolver_contact_crypto_provider; use crate::error::{PlatformWalletFFIResult, PlatformWalletFFIResultCode}; use crate::handle::{Handle, PLATFORM_WALLET_MANAGER_STORAGE}; -use crate::identity_keys_from_mnemonic::resolve_master_from_resolver; +use crate::identity_keys_from_mnemonic::{ + resolve_master_from_resolver_classified, ResolveFailureKind, +}; use crate::runtime::run_on_big_stack_thread; /// Discriminants for [`WalletStartupStatus`] across the boundary. @@ -153,25 +155,33 @@ pub unsafe extern "C" fn platform_wallet_manager_start_wallet_subsystems( // it is valid for the duration of this call, which is the only time the // closure can run. A null resolver means the host is telling us the wallet // holds resident keys (watch-only / in-process derive), so no resolver is - // supplied at all — as opposed to one that fails, which the library treats - // as a retryable "come back later". + // supplied at all — distinct from one that is supplied and fails, which + // the library classifies as retryable or terminal per `ScanKeyError`. let resolver_addr = mnemonic_resolver_handle as usize; let resolve_scan_key = move || { - resolve_master_from_resolver(resolver_addr as *mut MnemonicResolverHandle, &wid, network) - .map_err(|e| { - tracing::warn!( - wallet_id = %hex::encode(wid), - code = ?e.code, - "startup: could not resolve the wallet mnemonic for the identity scan" - ); - // Carried as an error the library can classify, not as an FFI - // throw: the documented contract is that only handle and - // wallet-id problems throw, and a locked device is neither. - platform_wallet::error::PlatformWalletError::KeyDerivation(format!( - "mnemonic resolver failed ({:?})", - e.code - )) - }) + resolve_master_from_resolver_classified( + resolver_addr as *mut MnemonicResolverHandle, + &wid, + network, + ) + .map_err(|failure| { + tracing::warn!( + wallet_id = %hex::encode(wid), + code = ?failure.result.code, + kind = ?failure.kind, + "startup: could not resolve the wallet mnemonic for the identity scan" + ); + // Carried as an outcome the library classifies, not as an FFI + // throw: the documented contract is that only handle and wallet-id + // problems throw, and a locked device is neither. The permanence + // travels with it — a phrase that is not BIP-39 must not be + // reported as something the next launch might fix. + let detail = format!("mnemonic resolver failed ({:?})", failure.result.code); + match failure.kind { + ResolveFailureKind::Unavailable => ScanKeyError::Unavailable(detail), + ResolveFailureKind::Permanent => ScanKeyError::Invalid(detail), + } + }) }; let scan_key = (!mnemonic_resolver_handle.is_null()) .then_some(&resolve_scan_key as &(dyn Fn() -> Result<_, _> + Send + Sync)); diff --git a/packages/rs-platform-wallet/src/manager/startup.rs b/packages/rs-platform-wallet/src/manager/startup.rs index 3524da3838..1b5ac7fa03 100644 --- a/packages/rs-platform-wallet/src/manager/startup.rs +++ b/packages/rs-platform-wallet/src/manager/startup.rs @@ -30,10 +30,11 @@ //! this call and dropped after — the same contract the drain already uses. The //! master xpriv is not even that: the caller hands over a [`ScanKeyResolver`] //! and this module invokes it only on the branch that scans, so a launch with -//! nothing to discover never touches the Keychain at all. Making the unattended -//! sweep self-sufficient -//! instead would turn a narrowly-scoped Keychain capability into a standing -//! one, which is a security-posture change and deliberately not what this is. +//! nothing to discover never touches the Keychain at all, and what is resolved +//! is erased when its guard drops rather than on the paths that happen to +//! reach an explicit call. Making the unattended sweep self-sufficient instead +//! would turn a narrowly-scoped Keychain capability into a standing one, which +//! is a security-posture change and deliberately not what this is. use std::time::{Duration, Instant}; @@ -99,12 +100,61 @@ async fn within_budget(deadline: Instant, future: F) -> /// happens cannot silently starve one of them of key material. /// /// `None` means the wallet holds resident keys and discovery derives -/// in-process. An `Err` means a scan needed a key and could not get one right -/// now — a locked device, a denied Keychain read — which is reported as -/// retryable, never as the terminal [`WalletStartupStatus::DiscoveryFailed`] -/// that an attempted-and-failed derive would produce. +/// in-process. An `Err` is classified by [`ScanKeyError`] rather than assumed. pub type ScanKeyResolver<'a> = - &'a (dyn Fn() -> Result + Send + Sync); + &'a (dyn Fn() -> Result + Send + Sync); + +/// Why a [`ScanKeyResolver`] could not produce a key — and, the part that +/// matters, whether asking again could ever change the answer. +/// +/// Flattening these two would misreport whichever one it did not pick. Both +/// have a plausible-looking home in the status vocabulary, and they are +/// opposites: one says "try on the next launch", the other says "stop". +#[derive(Debug)] +pub enum ScanKeyError { + /// The key exists but could not be read right now — a locked device, a + /// denied Keychain read, a resolver that was not ready yet. Reported as + /// retryable, and the next launch may well succeed. + Unavailable(String), + /// The key material itself is wrong: a mnemonic that is not valid BIP-39, + /// bytes that are not UTF-8, a master key that cannot be built from the + /// seed. Reported as terminal — no number of retries edits what is stored. + Invalid(String), +} + +impl std::fmt::Display for ScanKeyError { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + match self { + Self::Unavailable(detail) => write!(f, "scan key unavailable: {detail}"), + Self::Invalid(detail) => write!(f, "scan key invalid: {detail}"), + } + } +} + +/// Owns a resolved master xpriv and erases it when it drops. +/// +/// `ExtendedPrivKey` has no erasing `Drop`, so an explicit +/// `non_secure_erase` at the end of the scan only covers the paths that reach +/// it. A caller that drops the whole `start_wallet_subsystems` future while +/// discovery is awaiting Platform reaches none of them, and the scalar stays +/// in the freed future. Tying the erase to the value's lifetime instead of to +/// a code path makes cancellation — which this call now invites, since it is +/// budget-bounded and abandonable by design — a non-event. +struct ScanKeyGuard(ExtendedPrivKey); + +impl Drop for ScanKeyGuard { + fn drop(&mut self) { + self.0.private_key.non_secure_erase(); + } +} + +impl std::ops::Deref for ScanKeyGuard { + type Target = ExtendedPrivKey; + + fn deref(&self) -> &Self::Target { + &self.0 + } +} /// Knobs for [`PlatformWalletManager::start_wallet_subsystems`]. #[derive(Debug, Clone, Copy)] @@ -531,20 +581,30 @@ impl PlatformWalletManager // holds no private key would fail locally and be classified terminal, // telling the client to stop trying over a condition that clears // itself on the next unlock. - let mut master = match scan_key { + let master = match scan_key { None => None, Some(resolve) => match resolve() { - Ok(master) => Some(master), - Err(e) => { + Ok(master) => Some(ScanKeyGuard(master)), + // Retryable: the key is there, this moment was wrong. + Err(e @ ScanKeyError::Unavailable(_)) => { tracing::warn!( error = %e, - "startup: scan key material unavailable; deferring discovery to a \ - later start" + "startup: scan key unavailable; deferring discovery to a later start" ); tally.record_unreachable(); tally.record_discovery_gave_up(); return; } + // Terminal: retrying re-reads the same bad material. Reported + // like any other local fault, because that is what it is. + Err(e @ ScanKeyError::Invalid(_)) => { + tracing::warn!( + error = %e, + "startup: scan key material is not usable; no later scan can fix it" + ); + tally.record_discovery_failed_locally(); + return; + } }, }; @@ -553,84 +613,72 @@ impl PlatformWalletManager gap_limit: gap_limit.unwrap_or(IdentityDiscoveryOptions::default().gap_limit), }; - // Scoped so every exit path — including the early returns inside the - // loop — lands on the erase below. `return` leaves this block, not the - // function, so the tally bookkeeping keeps its existing shape. - async { - for (attempt, backoff) in DISCOVERY_BACKOFF.iter().map(Some).chain([None]).enumerate() { - // A single scan walks up to `gap_limit` indices, each a Platform - // fetch plus a DPNS lookup, so it needs the same ceiling the other - // steps have — otherwise one attempt can outlast the whole budget - // this call promises. - let attempt_future = async { - match master.as_ref() { - Some(master) => identity_wallet.discover_from_master(opts, master).await, - None => identity_wallet.discover(opts).await, - } - }; - let Some(result) = within_budget(deadline, attempt_future).await else { - // Sightings persist incrementally, so an abandoned scan may - // still have folded an identity in before it was cut off. - if let Some(known) = self.local_identity_id(wallet_id).await { - tally.record_local_identity(known); - return; - } - break; - }; - - match result { - Ok(found) => { - match found.first() { - Some(identity) => tally.record_discovered(identity.id()), - // An empty return is not proof on its own: `discover` - // reports only identities THIS call inserted, so a - // concurrent startup that inserted one first leaves us - // seeing it as already-managed and returning nothing. - // Consult local state before calling it absence. - None => match self.local_identity_id(wallet_id).await { - Some(known) => tally.record_discovered(known), - None => tally.record_proven_absent(), - }, - } - return; - } - Err(PlatformWalletError::IdentityDiscoveryIncomplete { .. }) => { - tally.record_unreachable(); - } - Err(e) => { - // Not a reachability question — a wallet/persistence - // failure will not fix itself on the next attempt, so this - // is recorded as terminal rather than as "try again". - tracing::warn!( - error = %e, - "startup: identity discovery failed for a non-network reason" - ); - tally.record_discovery_failed_locally(); - return; + for (attempt, backoff) in DISCOVERY_BACKOFF.iter().map(Some).chain([None]).enumerate() { + // A single scan walks up to `gap_limit` indices, each a Platform + // fetch plus a DPNS lookup, so it needs the same ceiling the other + // steps have — otherwise one attempt can outlast the whole budget + // this call promises. + let attempt_future = async { + match master.as_ref() { + Some(master) => identity_wallet.discover_from_master(opts, master).await, + None => identity_wallet.discover(opts).await, + } + }; + let Some(result) = within_budget(deadline, attempt_future).await else { + // Sightings persist incrementally, so an abandoned scan may + // still have folded an identity in before it was cut off. + if let Some(known) = self.local_identity_id(wallet_id).await { + tally.record_local_identity(known); + return; + } + break; + }; + + match result { + Ok(found) => { + match found.first() { + Some(identity) => tally.record_discovered(identity.id()), + // An empty return is not proof on its own: `discover` + // reports only identities THIS call inserted, so a + // concurrent startup that inserted one first leaves us + // seeing it as already-managed and returning nothing. + // Consult local state before calling it absence. + None => match self.local_identity_id(wallet_id).await { + Some(known) => tally.record_discovered(known), + None => tally.record_proven_absent(), + }, } + return; } - - let Some(backoff) = backoff else { break }; - let remaining = deadline.saturating_duration_since(Instant::now()); - if remaining.is_zero() { - break; + Err(PlatformWalletError::IdentityDiscoveryIncomplete { .. }) => { + tally.record_unreachable(); + } + Err(e) => { + // Not a reachability question — a wallet/persistence + // failure will not fix itself on the next attempt, so this + // is recorded as terminal rather than as "try again". + tracing::warn!( + error = %e, + "startup: identity discovery failed for a non-network reason" + ); + tally.record_discovery_failed_locally(); + return; } - tracing::info!( - attempt = attempt + 1, - "startup: identity discovery could not reach Platform; retrying" - ); - tokio::time::sleep((*backoff).min(remaining)).await; } - tally.record_discovery_gave_up(); + let Some(backoff) = backoff else { break }; + let remaining = deadline.saturating_duration_since(Instant::now()); + if remaining.is_zero() { + break; + } + tracing::info!( + attempt = attempt + 1, + "startup: identity discovery could not reach Platform; retrying" + ); + tokio::time::sleep((*backoff).min(remaining)).await; } - .await; - // The key lives no longer than the scan that needed it. `ExtendedPrivKey` - // has no `Drop`, so this is the only thing that clears it. - if let Some(master) = master.as_mut() { - master.private_key.non_secure_erase(); - } + tally.record_discovery_gave_up(); } }