diff --git a/packages/rs-platform-wallet-ffi/src/dashpay.rs b/packages/rs-platform-wallet-ffi/src/dashpay.rs index f87050caec..f9b749c51b 100644 --- a/packages/rs-platform-wallet-ffi/src/dashpay.rs +++ b/packages/rs-platform-wallet-ffi/src/dashpay.rs @@ -839,6 +839,24 @@ impl platform_wallet::ContactCryptoProvider for ResolverContactCryptoProvider { /// identity `signer_handle` to send the reciprocal). Writes the total number of /// completed entries (drained + auto-accepted) to `out_drained`. /// +/// # Seed binding +/// +/// Whenever there is drainable work, the resolver behind `core_signer_handle` +/// is first checked against this wallet's persisted BIP44 account-0 xpub +/// (`PlatformWallet::drain_pending_contact_crypto_verified`, the same gate the +/// startup sequence drains through). A resolver mapped to a different wallet +/// fails the call with `ErrorInvalidParameter` and derives NOTHING — the queue +/// is left intact for the next correct-seed drain. This is not advisory: a +/// wrong-seed drain writes contact receiving accounts that no later +/// correct-seed pass revisits (`register_contact_account` keys its existence +/// check on the contact pair, not on the xpub), so the corruption would be +/// permanent and its only symptom payments that never arrive. Any other +/// verification failure — a resolver that simply cannot answer — fails closed +/// the same way, with `ErrorWalletOperation`. +/// +/// An empty queue skips the check entirely, so a poll with nothing to do still +/// costs no key material. +/// /// # Safety /// - `signer_handle` (the identity document signer) is **optional**: pass null to /// run only the provider-derived ops (account build / contactInfo decrypt) and @@ -856,6 +874,10 @@ pub unsafe extern "C" fn platform_wallet_drain_pending_contact_crypto( ) -> PlatformWalletFFIResult { check_ptr!(core_signer_handle); check_ptr!(out_drained); + // Zero-init before any fallible work so a refused drain leaves a truthful + // count rather than whatever the caller's stack held — same discipline as + // the cached seed-binding verify. + unsafe { *out_drained = 0 }; // The identity signer is optional — null means "provider-only drain". let signer_addr = if signer_handle.is_null() { @@ -866,7 +888,6 @@ pub unsafe extern "C" fn platform_wallet_drain_pending_contact_crypto( let core_signer_addr = core_signer_handle as usize; let option = PLATFORM_WALLET_STORAGE.with_item(wallet_handle, |wallet| { - let identity = wallet.identity().clone(); let wallet_id = wallet.wallet_id(); let network = wallet.network(); // SAFETY: same lifetime contract as platform_wallet_send_dashpay_payment — @@ -878,30 +899,45 @@ pub unsafe extern "C" fn platform_wallet_drain_pending_contact_crypto( network, ) }; + let wallet = wallet.clone(); block_on_worker(async move { - let drained = identity - .dashpay() - .drain_pending_contact_crypto(&provider) - .await; // The auto-accept pass needs the identity signer for the reciprocal; - // skip it when no identity signer was supplied. - let accepted = if signer_addr != 0 { - let signer: &VTableSigner = &*(signer_addr as *const VTableSigner); - identity - .dashpay() - .drain_auto_accepts(signer, &provider) - .await + // `None` skips it, matching a null `signer_handle`. + let signer: Option<&VTableSigner> = if signer_addr != 0 { + Some(&*(signer_addr as *const VTableSigner)) } else { - 0 + None }; - drained + accepted + // Unbounded, as this entry point has always been: it is called off + // the main thread by a host that decided the work is worth waiting + // for, not from the Core-SPV-gating startup path that owns a budget. + wallet + .drain_pending_contact_crypto_verified(&provider, signer, None) + .await }) }); - let total = unwrap_option_or_return!(option); - unsafe { - *out_drained = total as u32; + let result = unwrap_option_or_return!(option); + match result { + Ok(total) => { + unsafe { + *out_drained = total as u32; + } + PlatformWalletFFIResult::ok() + } + // Same code the standalone verify reports for a mis-mapped resolver, so + // a host recognizes the wrong-seed condition identically whether it + // checked up front or was refused at the drain. + Err(e @ platform_wallet::PlatformWalletError::SeedMismatch { .. }) => { + PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorInvalidParameter, + e.to_string(), + ) + } + Err(e) => PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorWalletOperation, + e.to_string(), + ), } - PlatformWalletFFIResult::ok() } /// Number of deferred **account-build** contact-crypto ops queued for this diff --git a/packages/rs-platform-wallet-ffi/src/persistence.rs b/packages/rs-platform-wallet-ffi/src/persistence.rs index 04a4e29ea1..459c1bf314 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -4786,6 +4786,11 @@ fn build_wallet_start_state( let identity_manager = IdentityManagerStartState { out_of_wallet_identities: BTreeMap::new(), wallet_identities, + // No vtable slot carries the identity-scan verdict yet, so nothing is + // restored here. Empty reads as "unknown", which preserves the + // warm-launch shortcut rather than forcing a scan every launch — see + // `IdentityManagerStartState::scan_states`. + scan_states: BTreeMap::new(), }; // Rehydrate tracked asset-locks (built / broadcast / IS-locked diff --git a/packages/rs-platform-wallet-ffi/src/wallet_startup.rs b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs index 583dfc11bb..bb1b184085 100644 --- a/packages/rs-platform-wallet-ffi/src/wallet_startup.rs +++ b/packages/rs-platform-wallet-ffi/src/wallet_startup.rs @@ -31,6 +31,8 @@ pub enum WalletStartupStatusFFI { PartialNoIdentity = 2, PartialAccountsPending = 3, DiscoveryFailed = 4, + SeedBindingUnverified = 5, + IdentityScanIncomplete = 6, } impl From for WalletStartupStatusFFI { @@ -41,6 +43,8 @@ impl From for WalletStartupStatusFFI { WalletStartupStatus::PartialNoIdentity => Self::PartialNoIdentity, WalletStartupStatus::PartialAccountsPending => Self::PartialAccountsPending, WalletStartupStatus::DiscoveryFailed => Self::DiscoveryFailed, + WalletStartupStatus::SeedBindingUnverified => Self::SeedBindingUnverified, + WalletStartupStatus::IdentityScanIncomplete => Self::IdentityScanIncomplete, } } } @@ -56,8 +60,17 @@ pub struct WalletStartupOutcomeFFI { 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. + /// Whether the inline contact-request pass ran **to completion**. `false` + /// when it came back degraded — some identities' contact documents could + /// not be read, so their account builds were never enqueued. pub dashpay_sync_ran: bool, + /// The drain was skipped because the supplied contact-crypto provider does + /// not resolve this wallet's seed. Nothing was derived or written. + pub seed_binding_unverified: bool, + /// The wallet's identity scan is on record as having left indices + /// unanswered and this launch did not close the gap. Any identity reported + /// here is real; it may not be the only one. + pub identity_scan_incomplete: bool, /// Contact-crypto entries the drain completed. pub contact_accounts_drained: u32, /// Contact-account builds still queued on return. @@ -78,6 +91,8 @@ impl From for WalletStartupOutcomeFFI { identity_id, discovery_attempts: outcome.discovery_attempts, dashpay_sync_ran: outcome.dashpay_sync_ran, + seed_binding_unverified: outcome.seed_binding_unverified, + identity_scan_incomplete: outcome.identity_scan_incomplete, 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, diff --git a/packages/rs-platform-wallet/src/changeset/changeset.rs b/packages/rs-platform-wallet/src/changeset/changeset.rs index fa425fbde5..a100837083 100644 --- a/packages/rs-platform-wallet/src/changeset/changeset.rs +++ b/packages/rs-platform-wallet/src/changeset/changeset.rs @@ -1255,6 +1255,67 @@ pub struct WalletMetadataEntry { pub birth_height: u32, } +/// Whether the last gap-limit identity scan for this wallet answered every +/// index it probed. +/// +/// A scan has three endings, and only two of them are visible in what it +/// returns. It can find identities, it can prove there are none, or it can +/// find *some* while one of its probes goes unanswered — and that third +/// ending returns `Ok` with the identities it did find, because discarding +/// them would be worse. `ScanTally::is_trustworthy` is +/// `identities_seen > 0 || failed_probes == 0`, so a scan that saw index 0 +/// and got no answer at index 1 is reported as a success. +/// +/// That is survivable only if something scans again. Nothing did: the +/// warm-launch shortcut skips discovery whenever any identity is on file, and +/// the fact that the scan behind that identity was partial existed nowhere +/// once the process exited. An identity at the unanswered index then stayed +/// invisible for the life of the installation, along with all of its contacts +/// — a silent, permanent gap whose only symptom is a missing identity and +/// DPNS name after a restore. See dashpay/platform#4365. +/// +/// This is that missing fact. `complete` is stored rather than derived from +/// `failed_indices` because the two ways a scan can end early are different: +/// unanswered probes leave indices behind, while a scan abandoned at the +/// startup budget leaves none and is no more complete for it. +/// +/// Carried as `Option` — at most one scan verdict per +/// persist round, last-write-wins, which is correct because a later scan's +/// verdict wholly supersedes an earlier one's. +#[derive(Debug, Clone, PartialEq, Eq, Default)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct IdentityScanStateEntry { + /// Every index the scan probed was answered. Only a `true` here may let a + /// later launch skip discovery. + pub complete: bool, + /// One past the highest index the scan probed. + pub probed_through: u32, + /// Indices whose probe never got an answer, ascending. Empty for a scan + /// that was cut off before it could fail anything. + pub failed_indices: Vec, +} + +impl IdentityScanStateEntry { + /// A scan that answered every index it probed. + pub fn completed(probed_through: u32) -> Self { + Self { + complete: true, + probed_through, + failed_indices: Vec::new(), + } + } + + /// A scan that left at least one index unanswered, or was abandoned + /// before it could finish. + pub fn incomplete(probed_through: u32, failed_indices: Vec) -> Self { + Self { + complete: false, + probed_through, + failed_indices, + } + } +} + /// One entry per registered account. Captures the per-account xpub /// + type so a future load path can rebuild the wallet watch-only /// via `Account::from_xpub`. Hardened derivation at the account @@ -1606,6 +1667,18 @@ pub struct PlatformWalletChangeSet { /// Per-wallet metadata emitted once at registration. See /// [`WalletMetadataEntry`] for the merge policy. pub wallet_metadata: Option, + /// Verdict of the most recent gap-limit identity scan. Emitted by + /// discovery and by the startup sequence when it abandons a scan; read on + /// the next launch to decide whether the warm-launch shortcut may skip + /// discovery. See [`IdentityScanStateEntry`]. + /// + /// Durability caveat, the same one [`Self::pending_contact_crypto_added`] + /// carries: no persister vtable has a slot for this field yet, so on + /// hosts that have not adopted it the verdict is process-lifetime only. + /// Within a process it still redirects a second bring-up, and a partial + /// scan is now retried inside its own launch — but closing + /// dashpay/platform#4365 across launches needs the host slot. + pub identity_scan_state: Option, /// Per-account registration entries emitted at registration / on /// later `add_account` calls. See [`AccountRegistrationEntry`] for /// the merge policy (plain `Vec::extend`, dedup is the apply-side @@ -1741,6 +1814,13 @@ impl Merge for PlatformWalletChangeSet { if let Some(meta) = other.wallet_metadata { self.wallet_metadata = Some(meta); } + // Identity-scan verdict: last-write-wins. A later scan's verdict + // wholly supersedes an earlier one's — merging two would have to + // invent a rule for combining a complete scan with an incomplete one, + // and either answer would be wrong for one of them. + if let Some(scan) = other.identity_scan_state { + self.identity_scan_state = Some(scan); + } // Per-account specs and address-pool snapshots: append-only. // See the type docstrings for the rationale (registration // round emits each key once; snapshots are whole-pool, so @@ -1779,6 +1859,7 @@ impl Merge for PlatformWalletChangeSet { .as_ref() .is_none_or(|m| m.is_empty()) && self.wallet_metadata.is_none() + && self.identity_scan_state.is_none() && self.account_registrations.is_empty() && self.provider_key_account_registrations.is_empty() && self.account_address_pools.is_empty() diff --git a/packages/rs-platform-wallet/src/changeset/identity_manager_start_state.rs b/packages/rs-platform-wallet/src/changeset/identity_manager_start_state.rs index fbb42fa9e0..a4072b6184 100644 --- a/packages/rs-platform-wallet/src/changeset/identity_manager_start_state.rs +++ b/packages/rs-platform-wallet/src/changeset/identity_manager_start_state.rs @@ -8,6 +8,7 @@ use std::collections::BTreeMap; use dpp::prelude::Identifier; +use crate::changeset::IdentityScanStateEntry; use crate::wallet::identity::ManagedIdentity; use crate::wallet::identity::RegistrationIndex; use crate::wallet::platform_wallet::WalletId; @@ -26,4 +27,13 @@ pub struct IdentityManagerStartState { /// Wallet-owned identities, outer-keyed by wallet id and /// inner-keyed by BIP-9 registration index. pub wallet_identities: BTreeMap>, + /// Per-wallet verdict of the last gap-limit identity scan. + /// + /// An absent entry means "no verdict was restored", which is NOT the same + /// as a complete scan — a host that does not persist the verdict yet, and + /// a wallet whose first scan has not run, both land here. Absence + /// therefore preserves the existing warm-launch behaviour rather than + /// claiming a guarantee nobody made; only a restored `complete: false` + /// forces a rescan. See [`IdentityScanStateEntry`]. + pub scan_states: BTreeMap, } diff --git a/packages/rs-platform-wallet/src/changeset/mod.rs b/packages/rs-platform-wallet/src/changeset/mod.rs index e87cc14aee..4ecb036fae 100644 --- a/packages/rs-platform-wallet/src/changeset/mod.rs +++ b/packages/rs-platform-wallet/src/changeset/mod.rs @@ -31,12 +31,12 @@ pub use changeset::{ AssetLockChangeSet, AssetLockEntry, ContactChangeSet, ContactRequestEntry, CoreChangeSet, DpnsNameSaleStatus, DpnsNameStateChangeSet, DpnsNameStateEntry, HighestUsedIndexes, IdentityChangeSet, IdentityEntry, IdentityKeyDerivationIndices, IdentityKeyEntry, - IdentityKeysChangeSet, InvitationChangeSet, InvitationEntry, InvitationStatus, - KeyDerivationBreadcrumb, KeyWithBreadcrumb, PendingContactCrypto, PendingContactCryptoKey, - PendingContactCryptoKind, PendingContactCryptoOp, PlatformAddressBalanceEntry, - PlatformAddressChangeSet, PlatformWalletChangeSet, ProviderKeyAccountEntry, - ProviderKeyExtendedPubKey, ProviderPlatformNodePubKey, ReceivedContactRequestKey, - SentContactRequestKey, TokenBalanceChangeSet, WalletMetadataEntry, + IdentityKeysChangeSet, IdentityScanStateEntry, InvitationChangeSet, InvitationEntry, + InvitationStatus, KeyDerivationBreadcrumb, KeyWithBreadcrumb, PendingContactCrypto, + PendingContactCryptoKey, PendingContactCryptoKind, PendingContactCryptoOp, + PlatformAddressBalanceEntry, PlatformAddressChangeSet, PlatformWalletChangeSet, + ProviderKeyAccountEntry, ProviderKeyExtendedPubKey, ProviderPlatformNodePubKey, + ReceivedContactRequestKey, SentContactRequestKey, TokenBalanceChangeSet, WalletMetadataEntry, }; pub use client_start_state::ClientStartState; pub use client_wallet_start_state::ClientWalletStartState; diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index 8349eb1df2..6451048d4f 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -453,6 +453,24 @@ pub enum PlatformWalletError { wallet_id: String, }, + #[error( + "Contact-request sync reached none of the wallet's {identities} identities \ + (Platform unreachable) — the pass did not complete" + )] + /// A contact-request pass had identities to fetch for and could not read a + /// single one of them. Distinct from an empty success, which means + /// "Platform answered, and there is nothing new": this one means we do not + /// know, so the caller must not record the pass as completed. + /// + /// The sweep's per-identity log-and-continue collapsed the two, so a DAPI + /// outage returned `Ok(vec![])` and a startup sequence recorded a + /// successful contact sync — then reported `Ready`, promising that every + /// contact's DIP-15 addresses existed before Core SPV started. + ContactSyncUnreachable { + /// Identities the pass tried, and failed, to fetch for. + identities: usize, + }, + #[error("SPV is already running — stop it before starting again")] SpvAlreadyRunning, diff --git a/packages/rs-platform-wallet/src/manager/startup.rs b/packages/rs-platform-wallet/src/manager/startup.rs index 1b5ac7fa03..e1170f0099 100644 --- a/packages/rs-platform-wallet/src/manager/startup.rs +++ b/packages/rs-platform-wallet/src/manager/startup.rs @@ -202,29 +202,61 @@ pub enum WalletStartupStatus { /// is certain, not the reason. Either way those contacts' payments wait on /// the DIP-15 rescan. PartialAccountsPending, + /// The contact-crypto provider does not resolve the seed that owns this + /// wallet, so the drain was skipped without deriving anything. + /// + /// Not a slow-Platform outcome like the other partials — it says the host + /// handed this call a signer for a different wallet, and the only safe + /// response was to do nothing. Deriving anyway would write contact + /// receiving xpubs from the wrong seed, and because + /// `register_contact_account` keys its existence check on the contact pair + /// rather than on the xpub, those wrong addresses would be written once + /// and never revisited by a later correct-seed pass. The wallet would then + /// watch addresses nobody pays to, with no symptom but payments that never + /// arrive. + SeedBindingUnverified, + /// An identity is known and every later step ran, but the wallet's + /// gap-limit identity scan is still on record as having left indices + /// unanswered — the rescan this launch forced did not close the gap. + /// + /// The distinction from [`Self::Ready`] is the whole point: an identity + /// hiding at an unanswered index is invisible to everything that consults + /// local state, so calling this launch `Ready` promises an identity set + /// that was never established. That is #4365's exact shape, one level up — + /// the wallet has *an* identity, so the warm-launch shortcut and every + /// tally signal read clean while a second identity stays lost. + /// + /// Not terminal: the verdict stays on record, so the next launch re-opens + /// the question instead of taking the shortcut. Nothing about the contact + /// state is in doubt here — the sync and the drain both ran for the + /// identity that *is* known. + IdentityScanIncomplete, } impl WalletStartupStatus { /// Whether another discovery scan could change the answer. /// - /// 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. + /// True for [`Self::PartialNoIdentity`] (Platform was never reached) and + /// [`Self::IdentityScanIncomplete`] (it was reached, but some indices were + /// not). The rest are terminal for different reasons — absence was proven, + /// the failure is local and will still be there next time, or the scan + /// answered everything it probed. /// /// 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) + matches!(self, Self::PartialNoIdentity | Self::IdentityScanIncomplete) } /// 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 + /// retrying, while [`Self::IdentityScanIncomplete`] has an answer that is + /// merely known to be partial — an identity was found, so there is + /// something to display. 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 | Self::DiscoveryFailed) @@ -240,9 +272,24 @@ pub struct WalletStartupOutcome { /// 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). + /// Whether the inline DashPay sync pass ran **to completion**. `false` + /// when it was skipped, failed, ran out of budget, or came back degraded — + /// a pass that could not read some identities' contact documents leaves + /// their account builds unenqueued, so it is not a pass the caller may + /// rely on. pub dashpay_sync_ran: bool, + /// The contact-account drain was skipped because the supplied + /// contact-crypto provider does not resolve this wallet's seed. Nothing + /// was derived and nothing was written; the queue is intact. + pub seed_binding_unverified: bool, + /// The wallet's gap-limit identity scan is on record as having left + /// indices unanswered, and this launch's scan did not close the gap. The + /// identities reported here are real; they may not be all of them. + /// + /// Carried separately from `status` because the status can only report one + /// thing and a pending contact queue outranks this — a client that wants + /// to surface "still looking for your other identities" reads the flag. + pub identity_scan_incomplete: bool, /// Contact-crypto entries completed by the drain. pub contact_accounts_drained: usize, /// Contact-account builds still queued when this returned. @@ -273,6 +320,13 @@ pub(crate) struct StartupTally { pub discovery_failed_locally: bool, pub discovery_attempts: u32, pub dashpay_sync_ran: bool, + /// The drain was skipped because the contact-crypto provider could not be + /// shown to resolve this wallet's seed. + pub seed_binding_unverified: bool, + /// The recorded identity-scan verdict still says indices were left + /// unanswered once discovery was done for this launch. Independent of + /// `identity_id`: the gap is about the identities that were NOT found. + pub identity_scan_incomplete: bool, pub contact_accounts_drained: usize, pub contact_accounts_pending: usize, } @@ -326,6 +380,25 @@ impl StartupTally { self.dashpay_sync_ran = true; } + /// The seed behind the contact-crypto provider could not be shown to own + /// this wallet, so the drain never ran. + pub(crate) fn record_seed_binding_unverified(&mut self) { + self.seed_binding_unverified = true; + } + + /// Discovery is done for this launch and the recorded scan verdict still + /// says indices went unanswered. + /// + /// Read from the persisted verdict rather than inferred from the + /// discovery counters, because the two are not the same question. The + /// counters describe what *this* call did; the verdict describes what the + /// wallet's identity set is known to be missing, and it survives a launch + /// that never scanned at all. Only positive evidence sets it — an absent + /// verdict is "unknown", never "incomplete". + pub(crate) fn record_identity_scan_incomplete(&mut self) { + self.identity_scan_incomplete = true; + } + pub(crate) fn record_drain(&mut self, drained: usize, pending: usize) { self.contact_accounts_drained = drained; self.contact_accounts_pending = pending; @@ -338,17 +411,33 @@ 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 { + // Both of these say "the identity question is still open", so neither + // may decide the verdict once an identity is known. That used to be + // structurally impossible — discovery ran only when nothing was on + // file, and every branch that found something returned early — but a + // rescan forced by an incomplete prior scan reaches them with an + // identity already recorded, and reporting *that* launch as + // `DiscoveryFailed` would hide a sync and drain that both ran. + // // 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 { + if self.discovery_failed_locally && self.identity_id.is_none() { return WalletStartupStatus::DiscoveryFailed; } - if self.discovery_unreachable { + if self.discovery_unreachable && self.identity_id.is_none() { return WalletStartupStatus::PartialNoIdentity; } if self.proven_no_identity && self.identity_id.is_none() { return WalletStartupStatus::NoIdentity; } + // Outranks the queue counters, and must: a wrong-seed provider is why + // the queue was not drained, and it is the one ending here that points + // at a host misconfiguration rather than at Platform being slow. It + // also has to outrank `Ready` — with an empty queue every other signal + // would read as a clean run. + if self.seed_binding_unverified { + return WalletStartupStatus::SeedBindingUnverified; + } if self.contact_accounts_pending > 0 { return WalletStartupStatus::PartialAccountsPending; } @@ -359,6 +448,22 @@ impl StartupTally { if !self.dashpay_sync_ran { return WalletStartupStatus::PartialAccountsPending; } + // Last, and deliberately so: every check above describes work this + // launch did, while this one describes an identity set the wallet is + // on record as not having fully established. Ranking it here is what + // makes the fix additive — the only run whose status changes is the + // one that used to come back `Ready`, which is precisely the run that + // was lying. Everything else keeps the status a client already + // handles, and reads `identity_scan_incomplete` on the outcome if it + // cares. + // + // `Ready` is the promise that a contact payment has everything it + // needs. An unanswered index can hide a whole identity from every + // consumer of local state, so a launch that knows its scan was partial + // has not earned that word. + if self.identity_scan_incomplete { + return WalletStartupStatus::IdentityScanIncomplete; + } WalletStartupStatus::Ready } @@ -368,6 +473,8 @@ impl StartupTally { identity_id: self.identity_id, discovery_attempts: self.discovery_attempts, dashpay_sync_ran: self.dashpay_sync_ran, + seed_binding_unverified: self.seed_binding_unverified, + identity_scan_incomplete: self.identity_scan_incomplete, contact_accounts_drained: self.contact_accounts_drained, contact_accounts_pending: self.contact_accounts_pending, elapsed, @@ -440,19 +547,71 @@ impl PlatformWalletManager 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( - wallet_id, - identity_wallet, - scan_key, - opts.gap_limit, - deadline, - &mut tally, - ) - .await; + // scan it does not need — unless the scan that produced those + // identities is on record as having left indices unanswered, in + // which case "we already have one" is not evidence that we have + // them all. A wallet whose second identity was hidden by a failed + // probe used to stay that way for the life of the installation, + // because this shortcut is the only thing that would have looked + // again (dashpay/platform#4365). + // + // Only a recorded incomplete scan re-opens the question. An absent + // verdict keeps the shortcut, so hosts that do not persist it are + // exactly where they were rather than paying for a scan every + // launch. + let scan_incomplete = self.identity_scan_is_incomplete(wallet_id).await; + match self.local_identity_id(wallet_id).await { + Some(known) if !scan_incomplete => tally.record_local_identity(known), + Some(known) => { + tracing::info!( + wallet_id = %hex::encode(wallet_id), + "startup: the last identity scan left indices unanswered; rescanning \ + rather than trusting the identities already on file" + ); + tally.record_local_identity(known); + self.discover_identity_with_backoff( + wallet_id, + identity_wallet, + scan_key, + opts.gap_limit, + deadline, + &mut tally, + ) + .await; + } + None => { + self.discover_identity_with_backoff( + wallet_id, + identity_wallet, + scan_key, + opts.gap_limit, + deadline, + &mut tally, + ) + .await; + } + } + + // Discovery is done for this launch; re-read the verdict it leaves + // behind. Re-reading rather than inferring from the branch above is + // what makes this correct in every ending: a rescan that closed the + // gap publishes a complete verdict and this reads `false`, a rescan + // that could not publishes (or leaves) an incomplete one, and a fresh + // scan that came back partial without ever having a prior verdict is + // caught too — it is the same defect, reached from the other side. + // + // Without this the tally has no way to express "an identity is known + // and the set it belongs to is not", so a launch whose rescan was + // unreachable arrived at `Ready`: the guard in `status()` requires + // `identity_id.is_none()` before a discovery signal may decide the + // verdict, and here an identity IS on file. + if self.identity_scan_is_incomplete(wallet_id).await { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + "startup: the identity scan is still on record as incomplete; this launch \ + cannot report a settled identity set" + ); + tally.record_identity_scan_incomplete(); } // With no identity there is nothing to sync and nothing to drain, and @@ -464,15 +623,44 @@ 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. - match within_budget(deadline, identity_wallet.dashpay().sync_contact_requests()).await { - Some(Ok(requests)) => { + match within_budget( + deadline, + identity_wallet.dashpay().sync_contact_requests_reporting(), + ) + .await + { + Some(Ok(report)) if report.is_complete() => { tally.record_sync_ran(); tracing::debug!( wallet_id = %hex::encode(wallet_id), - requests = requests.len(), + requests = report.requests.len(), + identities = report.identities_attempted, "startup: contact-request pass complete" ); } + // Reached Platform for some identities and not others (or for none + // at all). The requests it did fetch are real and already + // persisted, but the identities it missed have contact requests + // nobody has looked at, whose account builds were therefore never + // enqueued — so the queue being empty below proves nothing. Not + // recording the pass keeps `status()` off `Ready`, which is the + // promise that every contact's DIP-15 addresses exist before Core + // SPV starts. + // + // The failures retry themselves: a fetch that errored leaves that + // direction's high-water cursor unadvanced, so the next sweep + // re-requests exactly the range this pass missed. + Some(Ok(report)) => { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + requests = report.requests.len(), + identities = report.identities_attempted, + failed = report.failed_identities.len(), + degraded = report.degraded_identities.len(), + "startup: contact-request pass was degraded; not recording it as a \ + completed sync" + ); + } Some(Err(e)) => { tracing::warn!( wallet_id = %hex::encode(wallet_id), @@ -503,29 +691,45 @@ impl PlatformWalletManager // 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 = identity_wallet - .dashpay() - .drain_pending_contact_crypto_until(contact_crypto, Some(deadline)) - .await; - let accepted = match identity_signer { - Some(signer) => { - identity_wallet - .dashpay() - .drain_auto_accepts_until(signer, contact_crypto, Some(deadline)) - .await - } - None => 0, - }; - (drained, accepted) - } + // + // The seed-binding gate in front of both drains is NOT applied here: + // it lives inside + // [`PlatformWallet::drain_pending_contact_crypto_verified`], the one + // primitive this sequence and the FFI drain entry point share. Keeping + // it there rather than in each caller is the whole point — a client + // that has to remember to gate the call is a client that will + // eventually forget, which is exactly how the FFI entry point came to + // have no gate while iOS enforced one in its Swift wrapper. The only + // error it can return is a failed verification (the drains themselves + // report counts, never errors), so an `Err` here means precisely "the + // provider was not shown to own this wallet". + let drained = match contact_crypto { + Some(contact_crypto) => match wallet + .drain_pending_contact_crypto_verified( + contact_crypto, + identity_signer, + Some(deadline), + ) + .await + { + Ok(drained) => drained, + Err(e) => { + tally.record_seed_binding_unverified(); + tracing::error!( + wallet_id = %hex::encode(wallet_id), + error = %e, + "startup: the contact-crypto drain was refused; the supplied provider \ + does not bind to this wallet's seed" + ); + 0 + } + }, None => { tracing::info!( wallet_id = %hex::encode(wallet_id), "startup: no contact-crypto provider; skipping the drain" ); - (0, 0) + 0 } }; // Not budgeted: a local queue-length read with no I/O. Leaving it @@ -535,7 +739,7 @@ impl PlatformWalletManager .dashpay() .pending_contact_crypto_count() .await; - tally.record_drain(drained + accepted, pending); + tally.record_drain(drained, pending); if pending > 0 { tracing::warn!( @@ -548,6 +752,20 @@ impl PlatformWalletManager Ok(tally.into_outcome(started.elapsed())) } + /// Whether this wallet's last gap-limit scan is on record as having left + /// indices unanswered. + /// + /// `false` when no verdict is known — see + /// [`IdentityManager::identity_scan_is_incomplete`] for why "unknown" must + /// not read as "incomplete". + /// + /// [`IdentityManager::identity_scan_is_incomplete`]: crate::wallet::identity::IdentityManager::identity_scan_is_incomplete + async fn identity_scan_is_incomplete(&self, wallet_id: &WalletId) -> bool { + let wm = self.wallet_manager.read().await; + wm.get_wallet_info(wallet_id) + .is_some_and(|info| info.identity_manager.identity_scan_is_incomplete(wallet_id)) + } + /// 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; @@ -625,6 +843,15 @@ impl PlatformWalletManager } }; let Some(result) = within_budget(deadline, attempt_future).await else { + // Dropped mid-await, so the scan recorded no verdict of its + // own. Record one here: an abandoned scan probed an unknown + // prefix of the index space and answered the rest of it not at + // all, which is exactly the state a later launch must not + // mistake for a settled identity set. Without this the + // budget-expiry path reproduces #4365 in its own right — it + // consults local state, finds the sighting that was persisted + // before cancellation, and records a warm launch. + self.record_identity_scan_cut_off(wallet_id).await; // 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 { @@ -636,19 +863,44 @@ impl PlatformWalletManager match result { Ok(found) => { - match found.first() { - Some(identity) => tally.record_discovered(identity.id()), + let identity = match found.first() { + Some(identity) => Some(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(), - }, + None => self.local_identity_id(wallet_id).await, + }; + let Some(identity) = identity else { + tally.record_proven_absent(); + return; + }; + tally.record_discovered(identity); + // `Ok` does not mean "every index was answered": a scan + // that saw an identity is reported as trustworthy even + // when a later probe went unanswered, and an identity + // hiding at that index is invisible until something scans + // again. Retry it here, inside the budget the caller + // already granted and with the scan key already resolved, + // rather than leaving it to a launch that may never come. + if !self.identity_scan_is_incomplete(wallet_id).await { + return; } - return; + if backoff.is_none() { + // Out of attempts. The verdict stays on record, so the + // next launch re-opens the question instead of taking + // the warm shortcut. + tracing::warn!( + "startup: identity discovery still has unanswered indices after \ + every attempt; the recorded verdict will force a rescan" + ); + return; + } + tracing::info!( + attempt = attempt + 1, + "startup: identity discovery left indices unanswered; rescanning" + ); } Err(PlatformWalletError::IdentityDiscoveryIncomplete { .. }) => { tally.record_unreachable(); @@ -678,7 +930,46 @@ impl PlatformWalletManager tokio::time::sleep((*backoff).min(remaining)).await; } - tally.record_discovery_gave_up(); + // Only meaningful while the identity question is still open. The + // partial-scan retry above can exhaust the loop with an identity + // already recorded, and that launch is not an unreachable-Platform + // launch — it found something, it just could not prove it found + // everything. + if !tally.has_identity() { + tally.record_discovery_gave_up(); + } + } + + /// Record that a scan was abandoned before it could answer every index. + /// + /// Mirrors what `discover` publishes for itself; needed separately because + /// a scan dropped mid-await never reaches its own bookkeeping. + async fn record_identity_scan_cut_off(&self, wallet_id: &WalletId) { + { + let mut wm = self.wallet_manager.write().await; + match wm.get_wallet_info_mut(wallet_id) { + Some(info) => info.identity_manager.record_identity_scan( + *wallet_id, + crate::changeset::IdentityScanStateEntry::incomplete(0, Vec::new()), + ), + None => return, + } + } + let changeset = crate::changeset::PlatformWalletChangeSet { + identity_scan_state: Some(crate::changeset::IdentityScanStateEntry::incomplete( + 0, + Vec::new(), + )), + ..Default::default() + }; + if let Err(e) = self.persister.store(*wallet_id, changeset) { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + error = %e, + "failed to persist an abandoned scan's verdict; the next launch may take the \ + warm shortcut over an incomplete identity set" + ); + } } } @@ -752,6 +1043,7 @@ mod tests { WalletStartupStatus::NoIdentity, WalletStartupStatus::PartialAccountsPending, WalletStartupStatus::DiscoveryFailed, + WalletStartupStatus::SeedBindingUnverified, ] { assert!( !terminal.discovery_worth_retrying(), @@ -836,6 +1128,153 @@ mod tests { assert!(!tally.has_identity()); } + /// A contact pass that could not read some identities' documents is not a + /// completed pass, and the whole point of tracking that is to keep it off + /// `Ready`. `Ready` promises the DIP-15 addresses exist before Core SPV + /// starts; a degraded pass never enqueued the account builds for the + /// identities it missed, so the queue being empty proves nothing. + #[test] + fn a_degraded_contact_pass_is_not_ready_even_with_an_empty_queue() { + let mut tally = StartupTally::default(); + tally.record_discovered(identity()); + // Deliberately no `record_sync_ran` — this is what startup does when + // the report comes back degraded. + tally.record_drain(0, 0); + + assert_eq!(tally.status(), WalletStartupStatus::PartialAccountsPending); + assert!(!tally.dashpay_sync_ran); + } + + /// The wrong-seed ending outranks every other non-discovery verdict, + /// including a clean-looking drain. With an empty queue the run is + /// otherwise indistinguishable from a healthy one, and reporting it as + /// `Ready` would hide the single condition here that points at a host + /// misconfiguration rather than at Platform being slow. + #[test] + fn an_unverified_seed_binding_outranks_a_clean_drain() { + let mut tally = StartupTally::default(); + tally.record_discovered(identity()); + tally.record_sync_ran(); + tally.record_seed_binding_unverified(); + tally.record_drain(0, 0); + + assert_eq!(tally.status(), WalletStartupStatus::SeedBindingUnverified); + assert!( + tally.status().identity_is_settled(), + "the identity was found; it is the drain that did not run" + ); + assert!(!tally.status().discovery_worth_retrying()); + } + + /// The outcome carries the flag so a client can tell "nothing was queued" + /// from "we refused to derive". + #[test] + fn an_unverified_seed_binding_reaches_the_outcome() { + let mut tally = StartupTally::default(); + tally.record_discovered(identity()); + tally.record_sync_ran(); + tally.record_seed_binding_unverified(); + + let outcome = tally.into_outcome(Duration::from_secs(1)); + assert!(outcome.seed_binding_unverified); + assert_eq!(outcome.status, WalletStartupStatus::SeedBindingUnverified); + } + + /// A rescan forced by an incomplete prior scan can reach the + /// discovery-failure branches with an identity already on file. Those + /// statuses say "the identity question is still open", which would be a + /// lie here — and it would also hide a sync and drain that both ran. But + /// the rescan failing is not nothing either: it means the scan gap that + /// forced it is still there. + /// + /// This test previously asserted `Ready` for the unreachable half, pinning + /// the very defect the `identity_scan_incomplete` signal exists to close — + /// a launch that knows its identity set is partial reporting the status + /// that promises it is complete. Both halves keep their real subject (the + /// identity must not be re-opened) and now assert the gap is reported. + #[test] + fn a_failed_rescan_reports_the_scan_gap_without_reopening_the_identity() { + // The scenario the name describes: the prior verdict said incomplete, + // which is the only reason a rescan ran at all, and it is still + // incomplete afterwards. + let mut unreachable = StartupTally::default(); + unreachable.record_local_identity(identity()); + unreachable.record_unreachable(); + unreachable.record_discovery_gave_up(); + unreachable.record_identity_scan_incomplete(); + unreachable.record_sync_ran(); + unreachable.record_drain(1, 0); + assert_eq!( + unreachable.status(), + WalletStartupStatus::IdentityScanIncomplete, + "a launch whose rescan never closed the gap has not established the identity set" + ); + assert!( + unreachable.status().identity_is_settled(), + "the identity that WAS found is real; only the set around it is open" + ); + assert!( + unreachable.status().discovery_worth_retrying(), + "the unanswered indices are exactly what another scan could answer" + ); + + let mut local_fault = StartupTally::default(); + local_fault.record_local_identity(identity()); + local_fault.record_discovery_failed_locally(); + local_fault.record_identity_scan_incomplete(); + local_fault.record_sync_ran(); + local_fault.record_drain(0, 2); + assert_eq!( + local_fault.status(), + WalletStartupStatus::PartialAccountsPending, + "a pending contact queue still outranks the scan gap in the status" + ); + assert!( + local_fault.status().identity_is_settled(), + "a local discovery fault must not re-open an identity that is on file" + ); + } + + /// The corrected verdict, isolated: an otherwise perfectly clean run — an + /// identity, a completed contact pass, an empty queue — is still not + /// `Ready` while the scan that produced that identity is on record as + /// having left indices unanswered. `Ready` promises a settled identity + /// set, and this run cannot promise one. + #[test] + fn an_incomplete_scan_keeps_an_otherwise_clean_run_off_ready() { + let mut tally = StartupTally::default(); + tally.record_local_identity(identity()); + tally.record_sync_ran(); + tally.record_identity_scan_incomplete(); + tally.record_drain(1, 0); + + assert_eq!(tally.status(), WalletStartupStatus::IdentityScanIncomplete); + + let outcome = tally.into_outcome(Duration::from_secs(1)); + assert!( + outcome.identity_scan_incomplete, + "the flag must reach the client even where the status is outranked" + ); + assert_eq!(outcome.identity_id, Some(identity())); + } + + /// The other direction, and the reason the check reads the recorded + /// verdict rather than the discovery counters: the identical run with a + /// scan that answered every index it probed IS `Ready`. Without this the + /// test above would keep passing if the signal were stuck on. + #[test] + fn a_complete_scan_reaches_ready() { + let mut tally = StartupTally::default(); + tally.record_local_identity(identity()); + tally.record_sync_ran(); + tally.record_drain(1, 0); + + assert_eq!(tally.status(), WalletStartupStatus::Ready); + + let outcome = tally.into_outcome(Duration::from_secs(1)); + assert!(!outcome.identity_scan_incomplete); + } + /// 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 @@ -868,6 +1307,449 @@ mod tests { assert_eq!(within_budget(deadline, async { "ran" }).await, None); } + // --------------------------------------------------------------------- + // End-to-end: the seed-binding gate in front of the drain. + // + // Driven through the real `start_wallet_subsystems` over a mock SDK, so + // what is asserted is the sequence's actual behaviour rather than a + // restatement of the tally rules above. + // --------------------------------------------------------------------- + + /// Canonical all-`abandon` BIP-39 vector — the seed + /// `test_platform_wallet_manager` builds its wallet from. + const OWNING_MNEMONIC: &str = "abandon abandon abandon abandon abandon abandon \ + abandon abandon abandon abandon abandon about"; + /// A different valid BIP-39 vector: the mis-mapped Keychain slot. + const FOREIGN_MNEMONIC: &str = + "legal winner thank year wave sausage worth useful legal winner thank yellow"; + + /// Only ever passed as `None`, so the sequence skips the DIP-15 + /// auto-accept pass — but the generic still has to be named. + #[derive(Debug)] + struct UnusedSigner; + + #[async_trait::async_trait] + impl Signer for UnusedSigner { + async fn sign( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + unreachable!("the auto-accept pass is never reached with a None signer") + } + + async fn sign_create_witness( + &self, + _key: &IdentityPublicKey, + _data: &[u8], + ) -> Result { + unreachable!("the auto-accept pass is never reached with a None signer") + } + + fn can_sign_with(&self, _key: &IdentityPublicKey) -> bool { + false + } + } + + fn seed_for(phrase: &str) -> [u8; 64] { + use key_wallet::mnemonic::{Language, Mnemonic}; + Mnemonic::from_phrase(phrase, Language::English) + .expect("valid test mnemonic") + .to_seed("") + } + + fn test_identity(id_byte: u8) -> dpp::identity::Identity { + use dpp::identity::v0::IdentityV0; + dpp::identity::Identity::V0(IdentityV0 { + id: Identifier::from([id_byte; 32]), + public_keys: std::collections::BTreeMap::new(), + balance: 0, + revision: 0, + }) + } + + /// A manager holding one wallet that owns one identity with a single + /// queued `RegisterReceiving` op — the smallest state in which the drain + /// has real work, and the op that derives a contact receiving xpub + /// straight from the provider with no network round trip. + async fn manager_with_queued_contact_crypto() -> ( + std::sync::Arc>, + WalletId, + ) { + use crate::changeset::{ + upsert_pending_contact_crypto, PendingContactCrypto, PendingContactCryptoOp, + }; + use crate::wallet::persister::{NoPlatformPersistence, WalletPersister}; + + let (manager, wallet_id) = crate::test_support::test_platform_wallet_manager().await; + let persister = WalletPersister::new(wallet_id, std::sync::Arc::new(NoPlatformPersistence)); + + let mut wm = manager.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("wallet info"); + info.identity_manager + .add_identity(test_identity(1), 0, wallet_id, &persister) + .expect("add identity"); + let managed = info + .identity_manager + .managed_identity_mut(&Identifier::from([1u8; 32])) + .expect("managed identity"); + upsert_pending_contact_crypto( + managed.dashpay_pending_contact_crypto_mut(), + PendingContactCrypto { + owner_identity_id: Identifier::from([1u8; 32]), + contact_id: Identifier::from([2u8; 32]), + op: PendingContactCryptoOp::RegisterReceiving, + enqueued_at_ms: 0, + }, + ); + drop(wm); + + (manager, wallet_id) + } + + /// Count the DashPay receiving accounts the wallet is watching. The thing + /// a wrong-seed drain would corrupt: `register_contact_account` keys its + /// existence check on `(index, us, them)` and NOT on the xpub, so an + /// account written from the wrong seed is never revisited. + async fn receiving_account_count( + manager: &crate::PlatformWalletManager, + wallet_id: &WalletId, + ) -> usize { + let wm = manager.wallet_manager.read().await; + wm.get_wallet_info(wallet_id) + .map(|info| info.core_wallet.accounts.dashpay_receival_accounts.len()) + .unwrap_or(0) + } + + async fn drainable( + manager: &crate::PlatformWalletManager, + wallet_id: &WalletId, + ) -> usize { + let wallet = manager.get_wallet(wallet_id).await.expect("wallet"); + wallet + .identity() + .dashpay() + .drainable_contact_crypto_count() + .await + } + + /// The defect this gate closes: a provider resolving someone else's seed + /// derives contact receiving xpubs that are written once and never + /// corrected, so the wallet watches addresses nobody pays to. The drain + /// must not run at all, and the queue must survive intact for the next + /// signer-present attempt. + #[tokio::test] + async fn a_wrong_seed_provider_never_reaches_the_drain() { + use crate::wallet::identity::network::SeedCryptoProvider; + + let (manager, wallet_id) = manager_with_queued_contact_crypto().await; + assert_eq!(receiving_account_count(&manager, &wallet_id).await, 0); + assert_eq!(drainable(&manager, &wallet_id).await, 1); + + let foreign = + SeedCryptoProvider::from_seed(seed_for(FOREIGN_MNEMONIC), key_wallet::Network::Testnet); + let outcome = manager + .start_wallet_subsystems( + &wallet_id, + None, + Some(&foreign), + None::<&UnusedSigner>, + WalletStartupOptions::default(), + ) + .await + .expect("a wrong seed is reported, not raised"); + + assert_eq!(outcome.status, WalletStartupStatus::SeedBindingUnverified); + assert!(outcome.seed_binding_unverified); + assert_eq!( + outcome.contact_accounts_drained, 0, + "nothing may be drained with a provider that does not own the wallet" + ); + assert_eq!( + receiving_account_count(&manager, &wallet_id).await, + 0, + "not one contact account may be registered from the wrong seed" + ); + assert_eq!( + drainable(&manager, &wallet_id).await, + 1, + "the queue must survive so the next signer-present drain can do the work" + ); + } + + /// The other half: the wallet's own seed passes the gate and the drain + /// runs. Without this the test above would also pass if the gate simply + /// refused everything. + #[tokio::test] + async fn the_owning_seed_passes_the_gate_and_the_drain_runs() { + use crate::wallet::identity::network::SeedCryptoProvider; + + let (manager, wallet_id) = manager_with_queued_contact_crypto().await; + let owning = + SeedCryptoProvider::from_seed(seed_for(OWNING_MNEMONIC), key_wallet::Network::Testnet); + + let outcome = manager + .start_wallet_subsystems( + &wallet_id, + None, + Some(&owning), + None::<&UnusedSigner>, + WalletStartupOptions::default(), + ) + .await + .expect("bring-up reports rather than raises"); + + assert!( + !outcome.seed_binding_unverified, + "the wallet's own seed must bind" + ); + assert_ne!(outcome.status, WalletStartupStatus::SeedBindingUnverified); + assert_eq!( + outcome.contact_accounts_drained, 1, + "the queued RegisterReceiving op must have been completed" + ); + assert_eq!( + receiving_account_count(&manager, &wallet_id).await, + 1, + "the contact receiving account must exist after a verified drain" + ); + } + + /// The gate is paid for only when there is something to protect. An empty + /// queue means the drain would derive nothing, so no key material is + /// resolved — which is what keeps this affordable on a warm launch. + /// Proven with a provider that would FAIL the check: reaching a status + /// other than `SeedBindingUnverified` shows it was never consulted. + #[tokio::test] + async fn an_empty_queue_skips_the_gate_entirely() { + use crate::changeset::{PendingContactCryptoKey, PendingContactCryptoKind}; + use crate::wallet::identity::network::SeedCryptoProvider; + + let (manager, wallet_id) = manager_with_queued_contact_crypto().await; + // Empty the queue so the drain has nothing to do. + { + let mut wm = manager.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("wallet info"); + let managed = info + .identity_manager + .managed_identity_mut(&Identifier::from([1u8; 32])) + .expect("managed identity"); + let key = PendingContactCryptoKey { + owner_identity_id: Identifier::from([1u8; 32]), + contact_id: Identifier::from([2u8; 32]), + kind: PendingContactCryptoKind::RegisterReceiving, + }; + managed + .dashpay_pending_contact_crypto_mut() + .retain(|e| e.key() != key); + } + assert_eq!(drainable(&manager, &wallet_id).await, 0); + + let foreign = + SeedCryptoProvider::from_seed(seed_for(FOREIGN_MNEMONIC), key_wallet::Network::Testnet); + let outcome = manager + .start_wallet_subsystems( + &wallet_id, + None, + Some(&foreign), + None::<&UnusedSigner>, + WalletStartupOptions::default(), + ) + .await + .expect("bring-up reports rather than raises"); + + assert!( + !outcome.seed_binding_unverified, + "with nothing to drain the binding check must not run at all" + ); + } + + /// The F1 regression, end to end and against a Platform that answers + /// nothing (the mock SDK fails every contact fetch, which is exactly the + /// DAPI-unreachable shape). + /// + /// Before the fix this pass returned `Ok(vec![])`, startup called + /// `record_sync_ran`, and a wallet whose contacts had never been read + /// reported `Ready` — the status that promises every contact's DIP-15 + /// addresses exist before Core SPV starts. + #[tokio::test] + async fn a_contact_pass_that_reached_nobody_is_not_a_completed_sync() { + use crate::wallet::identity::network::SeedCryptoProvider; + + let (manager, wallet_id) = manager_with_queued_contact_crypto().await; + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet"); + + // The pass itself: one identity attempted, none reached. + let report = wallet + .identity() + .dashpay() + .sync_contact_requests_reporting() + .await + .expect("the pass returns a report"); + assert_eq!(report.identities_attempted, 1); + assert_eq!(report.failed_identities.len(), 1); + assert!(!report.is_complete()); + assert!(report.is_fully_degraded()); + + // The back-compat return shape can no longer render this as success. + let err = wallet + .identity() + .dashpay() + .sync_contact_requests() + .await + .expect_err("reaching nobody must not look like an empty result"); + assert!( + matches!( + err, + PlatformWalletError::ContactSyncUnreachable { identities: 1 } + ), + "expected ContactSyncUnreachable, got: {err:?}" + ); + + // The cursors stayed put, so the next sweep re-requests the same + // range — this is what makes the failure retried rather than buried. + { + let wm = manager.wallet_manager.read().await; + let managed = wm + .get_wallet_info(&wallet_id) + .expect("wallet info") + .identity_manager + .managed_identity(&Identifier::from([1u8; 32])) + .expect("managed identity"); + assert_eq!( + managed.dashpay().high_water_received_ms(), + None, + "a failed fetch must not advance the cursor past requests it never read" + ); + assert_eq!(managed.dashpay().high_water_sent_ms(), None); + } + + // And the sequence must not record it as a sync that ran. + let owning = + SeedCryptoProvider::from_seed(seed_for(OWNING_MNEMONIC), key_wallet::Network::Testnet); + let outcome = manager + .start_wallet_subsystems( + &wallet_id, + None, + Some(&owning), + None::<&UnusedSigner>, + WalletStartupOptions::default(), + ) + .await + .expect("bring-up reports rather than raises"); + + assert!( + !outcome.dashpay_sync_ran, + "a pass that read none of the wallet's identities is not a completed sync" + ); + assert_ne!( + outcome.status, + WalletStartupStatus::Ready, + "Ready promises contact addresses this call never prepared" + ); + assert_eq!(outcome.status, WalletStartupStatus::PartialAccountsPending); + } + + /// The wire-up, end to end: the sequence reads the wallet's RECORDED scan + /// verdict once discovery is done and carries it out on the outcome. + /// + /// Reading the verdict rather than inferring from this call's discovery + /// counters is the point — a launch that took the warm shortcut, or whose + /// rescan was abandoned before it started, still has to report the gap the + /// wallet is on record as having, and neither of those launches has a + /// discovery counter to infer it from. + /// + /// Driven with a zero budget so no branch depends on network timing: every + /// step is abandoned at its deadline and what is asserted is purely which + /// verdict came out. ("No verdict at all" is not reachable here — the + /// harness's mock SDK answers no probe, so creating the wallet already + /// leaves one — and it is the accessor's own documented contract that an + /// absent verdict reads as unknown rather than incomplete.) + #[tokio::test] + async fn a_recorded_incomplete_scan_reaches_the_outcome() { + use crate::changeset::IdentityScanStateEntry; + use crate::wallet::identity::network::SeedCryptoProvider; + + let no_time = WalletStartupOptions { + budget: Duration::ZERO, + gap_limit: None, + }; + + let (manager, wallet_id) = manager_with_queued_contact_crypto().await; + let owning = + SeedCryptoProvider::from_seed(seed_for(OWNING_MNEMONIC), key_wallet::Network::Testnet); + + // The wallet arrives with an unanswered index on record — a real + // incomplete scan, produced by the mock SDK refusing every probe + // during wallet creation, not a hand-planted flag. + { + let wm = manager.wallet_manager.read().await; + let verdict = wm + .get_wallet_info(&wallet_id) + .expect("wallet info") + .identity_manager + .identity_scan_state(&wallet_id) + .cloned() + .expect("precondition: the creation scan recorded a verdict"); + assert!( + !verdict.complete, + "precondition: that verdict must be the incomplete one" + ); + } + + let outcome = manager + .start_wallet_subsystems( + &wallet_id, + None, + Some(&owning), + None::<&UnusedSigner>, + no_time, + ) + .await + .expect("bring-up reports rather than raises"); + + assert!( + outcome.identity_scan_incomplete, + "the recorded gap must reach the client: {outcome:?}" + ); + assert_ne!( + outcome.status, + WalletStartupStatus::Ready, + "Ready promises an identity set this launch did not establish" + ); + assert!( + outcome.identity_id.is_some(), + "the identity that IS known must still be reported" + ); + + // The other direction, through the same sequence: once the scan is on + // record as having answered everything it probed, the signal clears. + // Without this the assertion above would keep passing if the flag were + // simply stuck on. + { + let mut wm = manager.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("wallet info"); + info.identity_manager + .record_identity_scan(wallet_id, IdentityScanStateEntry::completed(4)); + } + + let outcome = manager + .start_wallet_subsystems( + &wallet_id, + None, + Some(&owning), + None::<&UnusedSigner>, + no_time, + ) + .await + .expect("bring-up reports rather than raises"); + + assert!( + !outcome.identity_scan_incomplete, + "a complete verdict leaves nothing to report: {outcome:?}" + ); + } + #[test] fn outcome_carries_the_tally_through() { let mut tally = StartupTally::default(); diff --git a/packages/rs-platform-wallet/src/wallet/apply.rs b/packages/rs-platform-wallet/src/wallet/apply.rs index 4390740640..8e36af5768 100644 --- a/packages/rs-platform-wallet/src/wallet/apply.rs +++ b/packages/rs-platform-wallet/src/wallet/apply.rs @@ -106,6 +106,7 @@ impl PlatformWalletInfo { // replay hook. invitations: _, dpns_name_states, + identity_scan_state, // Registration-round metadata / per-account specs / // per-pool snapshots are persistence-only — the // canonical in-memory wallet state is built up at @@ -162,6 +163,18 @@ impl PlatformWalletInfo { } } + // 2a'. Identity-scan verdict. Replayed rather than dropped: unlike the + // registration metadata below it, this one has live in-memory + // state on the identity manager, and it is read on the next + // bring-up to decide whether the identity set may be treated as + // settled. A verdict that survived to persistence and then got + // dropped on the way back in would leave a partial scan looking + // complete — the exact failure the verdict exists to prevent. + if let Some(scan) = identity_scan_state { + self.identity_manager + .record_identity_scan(wallet.wallet_id, scan); + } + // 2a. DPNS name states (username marketplace): upserts land // first, then tombstones, into the in-memory working set — // same LWW-then-remove discipline as the rest of this 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 c99018792c..b8f6d7bbd0 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 @@ -1089,6 +1089,63 @@ fn count_account_build_ops(queue: &[crate::changeset::PendingContactCrypto]) -> .count() } +/// What one contact-request pass actually reached, as opposed to what it +/// returned. +/// +/// The sweep is deliberately log-and-continue per identity: one identity's +/// transient DAPI error must not stall DashPay sync for every other identity +/// on the wallet. That is right for a recurring background sweep and wrong for +/// anything that treats the pass as a precondition, because the two endings it +/// collapses are opposites — "Platform answered, and there is nothing new" and +/// "Platform answered nobody, so we do not know". Both used to arrive as +/// `Ok(vec![])`. +/// +/// The distinction matters most at startup, where a completed pass is the +/// promise that a contact's DIP-15 addresses exist before the compact-filter +/// scan passes their funding height. An address the wallet is not watching by +/// then produces no transaction at all, so recording an unreachable pass as a +/// successful one does not merely mislabel a status — it starts Core SPV +/// against an address set that is silently short. +#[derive(Debug, Default, Clone)] +pub struct ContactSyncReport { + /// Newly discovered incoming contact requests. Real whatever else failed: + /// they were fetched, ingested and persisted. + pub requests: Vec, + /// Identities the pass tried to fetch for. + pub identities_attempted: usize, + /// Identities nothing was ingested for — the received-side fetch failed, + /// or their local state was gone when the write guard was taken. Their + /// high-water cursors are deliberately left unadvanced, so the next sweep + /// re-fetches exactly the range this one missed. + pub failed_identities: Vec, + /// Identities whose received side ingested but whose **sent**-side fetch + /// failed. Their incoming requests are real; what is missing is the + /// reciprocal reconciliation that establishes contacts, and the sent + /// cursor stays unadvanced so the next sweep retries it. + pub degraded_identities: Vec, +} + +impl ContactSyncReport { + /// Every identity's fetches, both directions, were answered. + /// + /// The only state in which the pass may be recorded as a completed one. A + /// wallet with no identities is complete by this rule — there was nothing + /// to fetch, which is an answer rather than a degradation. + pub fn is_complete(&self) -> bool { + self.failed_identities.is_empty() && self.degraded_identities.is_empty() + } + + /// Not one identity's contact documents could be read. + /// + /// The signature of an unreachable Platform rather than of an empty + /// wallet, and the ending that must never be mistaken for a clean pass. A + /// wallet with no identities is NOT fully degraded: nothing was attempted, + /// so nothing failed. + pub fn is_fully_degraded(&self) -> bool { + self.identities_attempted > 0 && self.failed_identities.len() == self.identities_attempted + } +} + impl DashPayView<'_, B> { /// Fetch and process contact requests from the platform for all local identities. /// @@ -1119,7 +1176,36 @@ impl DashPayView<'_, B> { /// them inline under the guard would deadlock on first execution. /// /// Returns all newly discovered incoming contact requests. + /// + /// # Errors + /// + /// [`PlatformWalletError::ContactSyncUnreachable`] when the pass had + /// identities to fetch for and not one of them could be read. That ending + /// is indistinguishable from a clean empty result in the return value + /// alone, and reporting it as success is what let a startup sequence + /// record an unreachable Platform as a completed contact pass. Callers + /// that need to tell a partial pass from a complete one — rather than only + /// a total failure from everything else — should call + /// [`Self::sync_contact_requests_reporting`] instead. pub async fn sync_contact_requests(&self) -> Result, PlatformWalletError> { + let report = self.sync_contact_requests_reporting().await?; + if report.is_fully_degraded() { + return Err(PlatformWalletError::ContactSyncUnreachable { + identities: report.identities_attempted, + }); + } + Ok(report.requests) + } + + /// [`Self::sync_contact_requests`], reporting what the pass reached. + /// + /// Same work, same side effects; the difference is only that the caller + /// gets the failure set rather than a `Vec` that cannot express it. Use + /// this wherever a *complete* pass is a precondition — a partial one is + /// still `Ok`, and still leaves some contacts' account builds unenqueued. + pub async fn sync_contact_requests_reporting( + &self, + ) -> Result { // Snapshot each identity's high-water cursors up front so the // incremental query bound is read before any mutation this sweep. let identities: Vec<(Identifier, Option, Option)> = { @@ -1147,6 +1233,10 @@ impl DashPayView<'_, B> { .collect() }; + let mut report = ContactSyncReport { + identities_attempted: identities.len(), + ..Default::default() + }; let mut all_requests = Vec::new(); for (identity_id, hw_received, hw_sent) in identities { @@ -1169,6 +1259,12 @@ impl DashPayView<'_, B> { error = %e, "Failed to fetch received contact requests; skipping this identity" ); + // Nothing of this identity's is ingested this pass, and its + // cursors stay where they were. Recorded rather than only + // logged so a caller that treats the pass as a precondition + // can tell this from a clean empty result — see + // `ContactSyncReport`. + report.failed_identities.push(identity_id); continue; } }; @@ -1191,6 +1287,7 @@ impl DashPayView<'_, B> { "Failed to fetch sent contact requests; reconciling received side only" ); sent_ok = false; + report.degraded_identities.push(identity_id); Default::default() } }; @@ -1218,11 +1315,18 @@ impl DashPayView<'_, B> { let candidates = { let mut wm = self.wallet_manager.write().await; let Some((wallet, info)) = wm.get_wallet_mut_and_info_mut(&self.wallet_id) else { + // Fetched, but there is no longer anywhere to put it. Same + // outcome for this identity as a failed fetch — nothing + // ingested — so it is reported the same way. + report.failed_identities.push(identity_id); continue; }; let managed = match info.identity_manager.managed_identity_mut(&identity_id) { Some(m) => m, - None => continue, + None => { + report.failed_identities.push(identity_id); + continue; + } }; // Established contacts re-keyed by a rotation request in // this pass — their stale external accounts are torn down @@ -1460,7 +1564,8 @@ impl DashPayView<'_, B> { self.enqueue_pending_auto_accepts(&identity_id).await; } - Ok(all_requests) + report.requests = all_requests; + Ok(report) } /// Parse a received `contactRequest` document into a [`ContactRequest`], @@ -3795,6 +3900,94 @@ mod cursor_tests { } } +#[cfg(test)] +mod contact_sync_report_tests { + use super::ContactSyncReport; + use dpp::prelude::Identifier; + + fn id(b: u8) -> Identifier { + Identifier::from([b; 32]) + } + + /// The clean pass: every identity answered, nothing new to report. This + /// must stay distinguishable from the unreachable case below, because it + /// is the only one that entitles a caller to say the contact set is + /// current. + #[test] + fn an_answered_pass_with_no_new_requests_is_complete() { + let report = ContactSyncReport { + identities_attempted: 2, + ..Default::default() + }; + + assert!(report.is_complete()); + assert!(!report.is_fully_degraded()); + } + + /// A wallet with no identities had nothing to fetch. That is an answer, + /// not a degradation — and specifically not a *total* one, or an empty + /// wallet would report the same thing as a total outage. + #[test] + fn a_wallet_with_no_identities_is_complete_and_not_degraded() { + let report = ContactSyncReport::default(); + + assert!(report.is_complete()); + assert!( + !report.is_fully_degraded(), + "nothing was attempted, so nothing failed" + ); + } + + /// Not one identity could be read: the DAPI-unreachable ending that used + /// to arrive as `Ok(vec![])`. + #[test] + fn a_pass_that_read_no_identity_is_fully_degraded() { + let report = ContactSyncReport { + identities_attempted: 2, + failed_identities: vec![id(1), id(2)], + ..Default::default() + }; + + assert!(!report.is_complete()); + assert!(report.is_fully_degraded()); + } + + /// The partial pass. What it fetched is real, so it is not a total + /// failure — and it is still not complete, because the identities it + /// missed have contact requests nobody looked at and account builds + /// nobody enqueued. Treating this as a completed sync is the same bug as + /// the total case, one identity at a time. + #[test] + fn a_partial_pass_is_neither_complete_nor_fully_degraded() { + let report = ContactSyncReport { + identities_attempted: 3, + failed_identities: vec![id(1)], + ..Default::default() + }; + + assert!(!report.is_complete()); + assert!(!report.is_fully_degraded()); + } + + /// A sent-side failure ingests the received side, so nothing is lost — + /// but the reciprocal reconciliation that establishes contacts did not + /// happen, so the pass still may not be recorded as complete. + #[test] + fn a_sent_side_failure_alone_still_degrades_the_pass() { + let report = ContactSyncReport { + identities_attempted: 1, + degraded_identities: vec![id(1)], + ..Default::default() + }; + + assert!(!report.is_complete()); + assert!( + !report.is_fully_degraded(), + "the received side was read; this is not a total failure" + ); + } +} + #[cfg(test)] mod sweep_tests { use super::*; diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/discovery.rs b/packages/rs-platform-wallet/src/wallet/identity/network/discovery.rs index ba68afb0c2..4de41bf3f7 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/discovery.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/discovery.rs @@ -340,124 +340,173 @@ impl IdentityWallet { let mut discovered: Vec = Vec::new(); let mut tally = ScanTally::default(); - while tally.should_continue(gap_limit) { - // Derive the MASTER auth pubkey hash for this identity index - // from whichever source the caller picked. The per-index read - // lock is only needed for the wallet-internal derive (it reads - // the resident key material); the master derive is a pure, - // lock-free secp256k1 pass. - let key_hash_array = match source { - KeyHashSource::ResidentWallet => { - let wm = self.wallet_manager.read().await; - let wallet = wm.get_wallet(&self.wallet_id).ok_or_else(|| { - crate::error::PlatformWalletError::WalletNotFound( - "Wallet not found in wallet manager".to_string(), - ) - })?; - derive_identity_auth_key_hash( - wallet, + // The scan runs inside its own block so its early returns cannot skip + // the verdict below. Every `?` in here is a LOCAL fault — a wallet that + // left the manager, a persistence write that failed — not a probe that + // went unanswered, and each of them abandons the scan part-way through + // the index space. Returning straight out left no verdict at all, and + // "unknown" is what keeps the warm-launch shortcut: the next launch saw + // the identities this scan had already folded in, took the shortcut, + // and never looked at the indices it never reached. That is #4365's + // shape on the local-fault path, so the error is carried out to the + // publish below rather than thrown from the middle of the walk. + let scan_outcome: Result<(), PlatformWalletError> = async { + while tally.should_continue(gap_limit) { + // Derive the MASTER auth pubkey hash for this identity index + // from whichever source the caller picked. The per-index read + // lock is only needed for the wallet-internal derive (it reads + // the resident key material); the master derive is a pure, + // lock-free secp256k1 pass. + let key_hash_array = match source { + KeyHashSource::ResidentWallet => { + let wm = self.wallet_manager.read().await; + let wallet = wm.get_wallet(&self.wallet_id).ok_or_else(|| { + crate::error::PlatformWalletError::WalletNotFound( + "Wallet not found in wallet manager".to_string(), + ) + })?; + derive_identity_auth_key_hash( + wallet, + network, + identity_index, + MASTER_KEY_INDEX, + )? + } + KeyHashSource::Master(master) => derive_identity_auth_key_hash_from_master( + master, network, identity_index, MASTER_KEY_INDEX, - )? - } - KeyHashSource::Master(master) => derive_identity_auth_key_hash_from_master( - master, - network, - identity_index, - MASTER_KEY_INDEX, - )?, - }; + )?, + }; + + // Query Platform for an identity registered with this key + // hash. No locks are held during this network call. + let fetch_result = Identity::fetch(&self.sdk, PublicKeyHash(key_hash_array)).await; + + match fetch_result { + Ok(Some(identity)) => { + let identity_id = identity.id(); + + // Derive + verify a candidate for every on-chain key + // (shared with the index-load path) BEFORE taking the write + // lock — candidate derivation borrows the resident wallet / + // master xpriv, while breadcrumb emission needs `&mut info`. + let key_decisions = self + .derive_key_breadcrumbs( + &identity, + identity_index, + network, + match source { + KeyHashSource::Master(master) => Some(master), + KeyHashSource::ResidentWallet => None, + }, + ) + .await?; + + // Acquire write lock to add/enrich the identity, then emit + // every per-key breadcrumb in one batched changeset. + let mut wm_guard = self.wallet_manager.write().await; + let info_guard = + wm_guard + .get_wallet_info_mut(&self.wallet_id) + .ok_or_else(|| { + crate::error::PlatformWalletError::WalletNotFound( + "Wallet info not found in wallet manager".to_string(), + ) + })?; + let is_new = info_guard.identity_manager.identity(&identity_id).is_none(); + if is_new { + info_guard.identity_manager.add_identity( + identity.clone(), + identity_index, + wallet_id, + &self.persister, + )?; + } - // Query Platform for an identity registered with this key - // hash. No locks are held during this network call. - let fetch_result = Identity::fetch(&self.sdk, PublicKeyHash(key_hash_array)).await; - - match fetch_result { - Ok(Some(identity)) => { - let identity_id = identity.id(); - - // Derive + verify a candidate for every on-chain key - // (shared with the index-load path) BEFORE taking the write - // lock — candidate derivation borrows the resident wallet / - // master xpriv, while breadcrumb emission needs `&mut info`. - let key_decisions = self - .derive_key_breadcrumbs( - &identity, - identity_index, - network, - match source { - KeyHashSource::Master(master) => Some(master), - KeyHashSource::ResidentWallet => None, - }, - ) - .await?; - - // Acquire write lock to add/enrich the identity, then emit - // every per-key breadcrumb in one batched changeset. - let mut wm_guard = self.wallet_manager.write().await; - let info_guard = - wm_guard - .get_wallet_info_mut(&self.wallet_id) - .ok_or_else(|| { - crate::error::PlatformWalletError::WalletNotFound( - "Wallet info not found in wallet manager".to_string(), - ) - })?; - let is_new = info_guard.identity_manager.identity(&identity_id).is_none(); - if is_new { - info_guard.identity_manager.add_identity( - identity.clone(), - identity_index, - wallet_id, - &self.persister, - )?; - } + if let Some(managed) = info_guard + .identity_manager + .managed_identity_mut(&identity_id) + { + managed.set_status(IdentityStatus::Active, &self.persister); + managed.wallet_id = Some(wallet_id); + // Breadcrumbs for every re-derivable key (not just the + // MASTER key) so the client (iOS Keychain) can + // re-derive each signing key's private key — without + // this only the master key is materialized and the + // imported identity cannot sign with its HIGH / + // CRITICAL authentication keys. A failed persist here + // would silently leave the identity watch-only after + // restart, so surface it (matching `add_identity` above). + managed + .add_keys(key_decisions, &self.persister) + .map_err(|e| { + PlatformWalletError::Persistence(format!( + "identity keys not persisted during discovery: {e}" + )) + })?; + } + drop(wm_guard); - if let Some(managed) = info_guard - .identity_manager - .managed_identity_mut(&identity_id) - { - managed.set_status(IdentityStatus::Active, &self.persister); - managed.wallet_id = Some(wallet_id); - // Breadcrumbs for every re-derivable key (not just the - // MASTER key) so the client (iOS Keychain) can - // re-derive each signing key's private key — without - // this only the master key is materialized and the - // imported identity cannot sign with its HIGH / - // CRITICAL authentication keys. A failed persist here - // would silently leave the identity watch-only after - // restart, so surface it (matching `add_identity` above). - managed - .add_keys(key_decisions, &self.persister) - .map_err(|e| { - PlatformWalletError::Persistence(format!( - "identity keys not persisted during discovery: {e}" - )) - })?; + if is_new { + discovered.push(identity.clone()); + } + tally.record_sighting(); } - drop(wm_guard); - - if is_new { - discovered.push(identity.clone()); + Ok(None) => { + tally.record_miss(); + } + Err(e) => { + tracing::warn!( + "Failed to query identity at index {}: {}", + identity_index, + e + ); + tally.record_failure(identity_index, e); } - tally.record_sighting(); - } - Ok(None) => { - tally.record_miss(); - } - Err(e) => { - tracing::warn!( - "Failed to query identity at index {}: {}", - identity_index, - e - ); - tally.record_failure(e); } - } - identity_index += 1; + identity_index += 1; + } + Ok(()) } + .await; + + // A local fault stopped the walk at `identity_index`, so that index and + // everything above it went unanswered. Record the index it died on + // before the verdict is built: without it `verdict` sees an empty + // failed-index list and would publish this abandoned scan as COMPLETE — + // strictly worse than the missing verdict this fixes, since a complete + // verdict actively re-arms the shortcut. + let probed_through = match &scan_outcome { + Ok(()) => identity_index, + Err(e) => { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + index = identity_index, + error = %e, + "identity discovery hit a local fault mid-scan; recording the index it \ + stopped at as unanswered so a later launch rescans instead of trusting \ + a walk that never finished" + ); + tally.record_local_fault(identity_index); + identity_index.saturating_add(1) + } + }; + + // Record what this scan could and could not answer, before the verdict + // on whether its *result* is usable. The two are independent: a scan + // that found an identity despite an unanswered probe returns `Ok` and + // is still not a scan anybody may build a "nothing left to find" + // conclusion on. Published on every ending — an unreachable Platform is + // the strongest possible reason to scan again, and so is a scan that + // was cut short by a fault on this device. + self.publish_scan_verdict(wallet_id, tally.verdict(probed_through)) + .await; + + // Only now, with the verdict on record either way. + scan_outcome?; if tally.is_trustworthy() { // Found something despite a failed probe: the discovered @@ -526,6 +575,50 @@ impl IdentityWallet { Ok(discovered) } + + /// Record and persist what a gap-limit scan managed to probe. + /// + /// Best-effort by design, and on the persist half only: the in-memory + /// record always lands, so a second bring-up in this process already sees + /// an incomplete scan and rescans. A failed persist costs the verdict its + /// survival across a restart, which is the same exposure a host that has + /// no slot for the field already has — it must not be allowed to fail the + /// scan that just succeeded. + async fn publish_scan_verdict( + &self, + wallet_id: crate::wallet::platform_wallet::WalletId, + verdict: crate::changeset::IdentityScanStateEntry, + ) { + { + let mut wm = self.wallet_manager.write().await; + match wm.get_wallet_info_mut(&wallet_id) { + Some(info) => info + .identity_manager + .record_identity_scan(wallet_id, verdict.clone()), + None => { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + "identity scan finished for a wallet that is no longer managed; \ + dropping its verdict" + ); + return; + } + } + } + + let changeset = crate::changeset::PlatformWalletChangeSet { + identity_scan_state: Some(verdict), + ..Default::default() + }; + if let Err(e) = self.persister.store(changeset) { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + error = %e, + "failed to persist the identity-scan verdict; a partial scan may not be \ + retried after a restart" + ); + } + } } /// Running bookkeeping for one gap-limit scan, and the verdict it produces. @@ -548,6 +641,13 @@ struct ScanTally { consecutive_misses: u32, /// Probes that never reached Platform. failed_probes: u32, + /// The indices behind [`Self::failed_probes`], ascending. + /// + /// The count alone says a scan was partial; the indices say *where*, which + /// is what makes the verdict actionable — a later launch knows exactly + /// which slots were never answered, and a reader of the persisted verdict + /// can tell an unanswered probe from a scan that was simply cut short. + failed_indices: Vec, /// Every index Platform answered with an identity — including ones the /// manager already tracked, which never reach the returned `discovered` /// list. A rescan from index 0 (what the app's "Find identities" command @@ -558,6 +658,15 @@ struct ScanTally { /// Last probe failure, kept typed so callers can inspect the variant /// rather than parse a rendered string. last_probe_error: Option, + /// The index a LOCAL fault stopped the scan at, if one did. + /// + /// Held apart from [`Self::failed_indices`] so [`Self::failed_probes`] and + /// the incomplete-scan error keep meaning exactly "probes Platform never + /// answered" — a persistence write that failed is not a network condition + /// and must not be reported as one. [`Self::verdict`] folds the two + /// together, because to a later launch they are the same fact: an index + /// nobody answered. + aborted_at_index: Option, } impl ScanTally { @@ -580,12 +689,53 @@ impl ScanTally { /// The probe never got an answer. It still advances the miss counter — the /// scan has to terminate when the network is down — but it is remembered /// separately, because the verdict depends on telling the two apart. - fn record_failure(&mut self, error: dash_sdk::Error) { + fn record_failure(&mut self, index: u32, error: dash_sdk::Error) { self.last_probe_error = Some(error); self.failed_probes += 1; + self.failed_indices.push(index); self.consecutive_misses += 1; } + /// A local fault abandoned the scan at `index` — the walk stopped there, + /// so that index and every one above it went unanswered. + /// + /// Recorded so [`Self::verdict`] cannot call an abandoned scan complete. + /// It does not touch the probe counters: the scan did not fail to REACH + /// Platform, it failed on this device, and conflating the two would make + /// the incomplete-scan error claim a network cause it has no evidence for. + fn record_local_fault(&mut self, index: u32) { + self.aborted_at_index = Some(index); + } + + /// Every index this scan did not answer, ascending — unanswered probes + /// plus the index a local fault abandoned it at. + fn unanswered_indices(&self) -> Vec { + let mut indices = self.failed_indices.clone(); + if let Some(index) = self.aborted_at_index { + if !indices.contains(&index) { + indices.push(index); + indices.sort_unstable(); + } + } + indices + } + + /// The verdict to persist for this scan. + /// + /// Separate from [`Self::is_trustworthy`] and not its mirror: a scan that + /// found an identity despite an unanswered probe IS trustworthy — its + /// findings are real and worth keeping — and is still not complete. That + /// gap is precisely where an identity goes missing for the life of an + /// installation, so the two questions get two methods. + fn verdict(&self, probed_through: u32) -> crate::changeset::IdentityScanStateEntry { + let unanswered = self.unanswered_indices(); + if unanswered.is_empty() { + crate::changeset::IdentityScanStateEntry::completed(probed_through) + } else { + crate::changeset::IdentityScanStateEntry::incomplete(probed_through, unanswered) + } + } + /// Whether the scan's literal result may be reported as-is. /// /// Emptiness is only trustworthy when every probe was answered. A scan @@ -886,14 +1036,18 @@ mod tests { outcomes: impl IntoIterator, ()>>, ) -> ScanTally { let mut tally = ScanTally::default(); - for outcome in outcomes { + // Index-carrying like the production loop, which probes from + // `start_index` upward — the harness scans from 0, so the element + // position IS the index. + for (index, outcome) in outcomes.into_iter().enumerate() { if !tally.should_continue(gap_limit) { break; } + let index = index as u32; match outcome { Ok(Some(())) => tally.record_sighting(), Ok(None) => tally.record_miss(), - Err(()) => tally.record_failure(probe_failure()), + Err(()) => tally.record_failure(index, probe_failure()), } } tally @@ -910,6 +1064,64 @@ mod tests { assert!(!tally.is_trustworthy()); } + /// The #4365 shape: an identity at index 0, no answer at index 1. The + /// scan is trustworthy — its findings are real — and it is NOT complete, + /// and those are different questions. Reporting only the first is what let + /// an identity at the unanswered index stay hidden for the life of an + /// installation. + #[test] + fn a_scan_that_found_something_despite_a_failed_probe_is_trustworthy_but_incomplete() { + let tally = run_scan(5, [Ok(Some(())), Err(()), Ok(None), Ok(None), Ok(None)]); + + assert!( + tally.is_trustworthy(), + "the identity it found is real and must not be discarded" + ); + let verdict = tally.verdict(5); + assert!( + !verdict.complete, + "an unanswered index means the identity set is not settled" + ); + assert_eq!( + verdict.failed_indices, + vec![1], + "the verdict names which index went unanswered" + ); + assert_eq!(verdict.probed_through, 5); + } + + /// A scan that answered everything is the only one that may let a later + /// launch skip discovery. + #[test] + fn a_fully_answered_scan_produces_a_complete_verdict() { + let tally = run_scan( + 5, + [ + Ok(Some(())), + Ok(None), + Ok(None), + Ok(None), + Ok(None), + Ok(None), + ], + ); + + let verdict = tally.verdict(6); + assert!(verdict.complete); + assert!(verdict.failed_indices.is_empty()); + } + + /// Every probe unanswered: the verdict records all of them, so a rescan + /// knows the whole range is open. + #[test] + fn a_scan_that_reached_nobody_records_every_failed_index() { + let tally = run_scan(3, [Err(()), Err(()), Err(())]); + + let verdict = tally.verdict(3); + assert!(!verdict.complete); + assert_eq!(verdict.failed_indices, vec![0, 1, 2]); + } + /// The genuinely-empty wallet: every probe answered, all of them "none". #[test] fn scan_with_every_probe_answered_empty_is_trustworthy() { @@ -996,4 +1208,140 @@ mod tests { assert!(error.source().is_some(), "source must survive for callers"); assert!(error.to_string().contains("dapi unreachable")); } + + // ----------------------------------------------------------------------- + // Local faults: the scan aborted by something on THIS device rather than + // by an unanswered probe. + // ----------------------------------------------------------------------- + + /// A scan abandoned part-way through must never be published as complete. + /// + /// This is the trap in the fix: a local fault records no failed *probe*, + /// so a verdict built from the probe bookkeeping alone sees an empty + /// failed-index list and calls the abandoned walk clean. That is worse + /// than the missing verdict it replaces — a complete verdict actively + /// re-arms the warm-launch shortcut over an index space nobody finished. + #[test] + fn a_locally_aborted_scan_is_never_complete() { + let mut tally = run_scan(5, [Ok(Some(())), Ok(None)]); + // Fault on the index the walk stopped at, exactly as `discover_inner` + // records it. + tally.record_local_fault(2); + + let verdict = tally.verdict(3); + assert!( + !verdict.complete, + "a scan that stopped early cannot claim it answered everything" + ); + assert_eq!( + verdict.failed_indices, + vec![2], + "the verdict names the index the walk died on" + ); + assert_eq!(verdict.probed_through, 3); + } + + /// The two kinds of gap are merged, ascending, for the reader: to a later + /// launch an unanswered probe and an abandoned index are the same fact. + #[test] + fn an_abort_is_merged_with_the_unanswered_probes() { + let mut tally = run_scan(5, [Ok(Some(())), Err(()), Ok(None)]); + tally.record_local_fault(3); + + let verdict = tally.verdict(4); + assert!(!verdict.complete); + assert_eq!(verdict.failed_indices, vec![1, 3]); + assert_eq!( + tally.failed_probes, 1, + "a device-side fault is not a probe Platform failed to answer" + ); + } + + /// A local fault at an index that ALSO went unanswered is recorded once. + #[test] + fn an_abort_at_an_already_unanswered_index_is_not_duplicated() { + let mut tally = run_scan(5, [Err(())]); + tally.record_local_fault(0); + + assert_eq!(tally.verdict(1).failed_indices, vec![0]); + } + + /// End to end, with a real local fault injected mid-scan: a stale + /// **complete** verdict must not survive it. + /// + /// The fault is genuine rather than mocked — `discover()` derives each + /// probe hash from resident key material, and this wallet is + /// external-signable (its seed lives outside the manager), so the derive + /// fails on the first index. That is one of the `?` early returns above + /// `publish_scan_verdict`, and before this fix every one of them returned + /// without publishing anything at all: the previous verdict stood, and a + /// verdict that says "complete" is exactly what keeps the warm-launch + /// shortcut armed. The wallet would then trust an index space this scan + /// abandoned — #4365's shape reached from the local-fault side. + #[tokio::test] + async fn a_local_fault_mid_scan_replaces_a_stale_complete_verdict() { + use crate::changeset::IdentityScanStateEntry; + use crate::wallet::identity::network::IdentityDiscoveryOptions; + + let (manager, wallet_id) = crate::test_support::test_platform_wallet_manager().await; + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet"); + + assert!( + !wallet.state().await.wallet().has_seed(), + "precondition: the resident derive must be the thing that faults" + ); + + // The state the defect preserves: a wallet whose last scan answered + // everything, so the next launch takes the shortcut. + { + let mut wm = manager.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("wallet info"); + info.identity_manager + .record_identity_scan(wallet_id, IdentityScanStateEntry::completed(9)); + } + { + let wm = manager.wallet_manager.read().await; + assert!( + !wm.get_wallet_info(&wallet_id) + .expect("wallet info") + .identity_manager + .identity_scan_is_incomplete(&wallet_id), + "precondition: the warm shortcut is armed" + ); + } + + let err = wallet + .identity() + .discover(IdentityDiscoveryOptions { + start_index: Some(0), + gap_limit: 5, + }) + .await + .expect_err("the resident derive cannot work for a seedless wallet"); + assert!( + !matches!(err, PlatformWalletError::IdentityDiscoveryIncomplete { .. }), + "precondition: this must be a LOCAL fault, not an unanswered probe: {err:?}" + ); + + let wm = manager.wallet_manager.read().await; + let info = wm.get_wallet_info(&wallet_id).expect("wallet info"); + let verdict = info + .identity_manager + .identity_scan_state(&wallet_id) + .expect("a verdict must have been published on the fault path"); + assert!( + !verdict.complete, + "the stale complete verdict must not have survived an abandoned scan" + ); + assert_eq!( + verdict.failed_indices, + vec![0], + "the index the walk died on is on record as unanswered" + ); + assert!( + info.identity_manager + .identity_scan_is_incomplete(&wallet_id), + "the next launch must re-scan instead of taking the warm shortcut" + ); + } } diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/mod.rs b/packages/rs-platform-wallet/src/wallet/identity/network/mod.rs index 752fee202c..3eb1753449 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/mod.rs @@ -67,8 +67,14 @@ pub use seed_binding::SeedBindingVerification; mod tokens; pub use contact_info::ContactInfoPublishOutcome; +/// Seed-backed [`ContactCryptoProvider`] for tests. Lives behind the private +/// `contact_requests` module, so sibling modules reach it directly and the +/// manager's tests reach it through here. +#[cfg(test)] +pub(crate) use contact_requests::SeedCryptoProvider; pub use contact_requests::{ AutoAcceptProofSource, ContactCryptoProvider, ContactInfoOpened, ContactInfoSealed, + ContactSyncReport, }; pub use dashpay_view::DashPayView; pub use discovery::IdentityDiscoveryOptions; diff --git a/packages/rs-platform-wallet/src/wallet/identity/network/seed_binding.rs b/packages/rs-platform-wallet/src/wallet/identity/network/seed_binding.rs index 8ba700b191..8d1b94795a 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/seed_binding.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/seed_binding.rs @@ -9,6 +9,13 @@ //! Keychain slot — the signer resolving some other wallet's mnemonic — derives //! a different xpub and is refused, so it can never sign for the wrong wallet. //! This is the wrong-seed detection without ever holding a resident seed. +//! +//! The check is also the gate in front of the deferred contact-crypto drain — +//! see [`PlatformWallet::drain_pending_contact_crypto_verified`], the primitive +//! every client drains through so none of them can forget it. + +use dpp::identity::signer::Signer; +use dpp::identity::IdentityPublicKey; use crate::error::PlatformWalletError; use crate::wallet::identity::network::contact_requests::ContactCryptoProvider; @@ -114,6 +121,88 @@ impl PlatformWallet { }) } } + + /// Drain the deferred contact-crypto queue, but only through a provider + /// that has been shown to resolve this wallet's seed. + /// + /// Runs the provider-only ops + /// ([`drain_pending_contact_crypto_until`]) and, when an identity signer is + /// supplied, the DIP-15 auto-accept pass + /// ([`drain_auto_accepts_until`]) — the same pair every drain entry point + /// runs — and returns their combined completed count. `deadline` bounds + /// both from the inside; `None` is unbounded. + /// + /// # Why the gate lives here + /// + /// Everything the drain derives comes from whatever seed the provider + /// resolves, and none of it is authenticated. A provider mapped to the + /// wrong wallet derives contact receiving xpubs from the wrong seed, and + /// `register_contact_account` keys its existence check on `(index, us, + /// them)` rather than on the xpub — so the wrong addresses are written + /// once and every later correct-seed pass no-ops. The corruption is + /// permanent and its only symptom is payments that never arrive. + /// + /// Putting the check in each client is what lets a client forget it: iOS + /// enforced it in its Swift wrapper while the FFI drain entry point had no + /// gate at all, so a JNI binding written against that entry point + /// inherited the bug rather than the rule. This is the one primitive both + /// the startup sequence and the FFI drain call, so there is a single place + /// the gate can be removed from and none where it can be omitted. + /// + /// # Cost + /// + /// Proportional to the risk: an empty queue would derive nothing, so there + /// is no wrong-seed write to prevent and the check is skipped entirely — + /// a warm launch with nothing queued resolves no key material at all. Both + /// drains ride the same queue, so one count covers both. + /// + /// # Errors + /// + /// Fails closed on **every** verification error, not only on + /// [`PlatformWalletError::SeedMismatch`]: a provider that cannot answer has + /// not been shown to own this wallet. Skipping costs nothing that is not + /// recoverable — the queue is untouched, so the next signer-present drain + /// completes exactly the work this one declined to guess at. + /// + /// [`drain_pending_contact_crypto_until`]: crate::wallet::identity::network::DashPayView::drain_pending_contact_crypto_until + /// [`drain_auto_accepts_until`]: crate::wallet::identity::network::DashPayView::drain_auto_accepts_until + pub async fn drain_pending_contact_crypto_verified( + &self, + crypto: &C, + identity_signer: Option<&S>, + deadline: Option, + ) -> Result + where + C: ContactCryptoProvider + Sync, + S: Signer + Send + Sync, + { + let dashpay = self.identity().dashpay(); + if dashpay.drainable_contact_crypto_count().await == 0 { + return Ok(0); + } + + self.verify_seed_binds(crypto).await.inspect_err(|e| { + tracing::error!( + wallet_id = %hex::encode(self.wallet_id()), + error = %e, + "the contact-crypto provider does not bind to this wallet's seed; skipping \ + the drain rather than deriving contact addresses that could never be corrected" + ); + })?; + + let drained = dashpay + .drain_pending_contact_crypto_until(crypto, deadline) + .await; + let accepted = match identity_signer { + Some(signer) => { + dashpay + .drain_auto_accepts_until(signer, crypto, deadline) + .await + } + None => 0, + }; + Ok(drained + accepted) + } } #[cfg(test)] @@ -506,4 +595,208 @@ mod tests { "expected InvalidIdentityData, got: {err:?}" ); } + + // ----------------------------------------------------------------------- + // The gate in front of the drain. + // + // `verify_seed_binds` above proves the check itself is right. These prove + // the drain cannot run without it — the property that matters, because the + // FFI entry point every JNI client binds to used to call the drains + // directly and skip the check entirely. + // ----------------------------------------------------------------------- + + /// A different valid BIP-39 vector: the mis-mapped Keychain slot. + const FOREIGN_MNEMONIC: &str = + "legal winner thank year wave sausage worth useful legal winner thank yellow"; + + /// Only ever passed as `None`, so the auto-accept pass is skipped — but the + /// generic still has to be named. + #[derive(Debug)] + struct UnusedSigner; + + #[async_trait::async_trait] + impl dpp::identity::signer::Signer for UnusedSigner { + async fn sign( + &self, + _key: &dpp::identity::IdentityPublicKey, + _data: &[u8], + ) -> Result { + unreachable!("the auto-accept pass is never reached with a None signer") + } + + async fn sign_create_witness( + &self, + _key: &dpp::identity::IdentityPublicKey, + _data: &[u8], + ) -> Result { + unreachable!("the auto-accept pass is never reached with a None signer") + } + + fn can_sign_with(&self, _key: &dpp::identity::IdentityPublicKey) -> bool { + false + } + } + + /// A wallet owning one identity with a single queued `RegisterReceiving` + /// op — the smallest state in which the drain has real work, and the op + /// that derives a contact receiving xpub straight from the provider with + /// no network round trip. + async fn wallet_with_queued_contact_crypto() -> ( + Arc>, + Arc, + WalletId, + ) { + use crate::changeset::{ + upsert_pending_contact_crypto, PendingContactCrypto, PendingContactCryptoOp, + }; + use crate::wallet::persister::{NoPlatformPersistence, WalletPersister}; + use dpp::identity::v0::IdentityV0; + use dpp::prelude::Identifier; + + let manager = make_manager(); + let wallet = manager + .create_wallet_from_seed_bytes( + Network::Testnet, + &seed_for(TEST_MNEMONIC), + WalletAccountCreationOptions::Default, + Some(0), + ) + .await + .expect("wallet creation"); + let wallet_id = wallet.wallet_id(); + let persister = WalletPersister::new(wallet_id, Arc::new(NoPlatformPersistence)); + + let mut wm = manager.wallet_manager.write().await; + let info = wm.get_wallet_info_mut(&wallet_id).expect("wallet info"); + info.identity_manager + .add_identity( + dpp::identity::Identity::V0(IdentityV0 { + id: Identifier::from([1u8; 32]), + public_keys: std::collections::BTreeMap::new(), + balance: 0, + revision: 0, + }), + 0, + wallet_id, + &persister, + ) + .expect("add identity"); + let managed = info + .identity_manager + .managed_identity_mut(&Identifier::from([1u8; 32])) + .expect("managed identity"); + upsert_pending_contact_crypto( + managed.dashpay_pending_contact_crypto_mut(), + PendingContactCrypto { + owner_identity_id: Identifier::from([1u8; 32]), + contact_id: Identifier::from([2u8; 32]), + op: PendingContactCryptoOp::RegisterReceiving, + enqueued_at_ms: 0, + }, + ); + drop(wm); + + (manager, wallet, wallet_id) + } + + /// The DashPay receiving accounts the wallet is watching — the thing a + /// wrong-seed drain corrupts. `register_contact_account` keys its + /// existence check on `(index, us, them)` and NOT on the xpub, so an + /// account written from the wrong seed is never revisited. + async fn receiving_account_count( + manager: &PlatformWalletManager, + wallet_id: &WalletId, + ) -> usize { + let wm = manager.wallet_manager.read().await; + wm.get_wallet_info(wallet_id) + .map(|info| info.core_wallet.accounts.dashpay_receival_accounts.len()) + .unwrap_or(0) + } + + async fn drainable(wallet: &crate::PlatformWallet) -> usize { + wallet + .identity() + .dashpay() + .drainable_contact_crypto_count() + .await + } + + /// The defect: a provider resolving someone else's seed derives contact + /// receiving xpubs that are written once and never corrected, so the + /// wallet watches addresses nobody pays to. The drain must not run at all, + /// and the queue must survive intact for the next signer-present attempt. + #[tokio::test] + async fn a_wrong_seed_provider_is_refused_before_the_drain() { + let (manager, wallet, wallet_id) = wallet_with_queued_contact_crypto().await; + assert_eq!(receiving_account_count(&manager, &wallet_id).await, 0); + assert_eq!(drainable(&wallet).await, 1); + + let foreign = SeedCryptoProvider::from_seed(seed_for(FOREIGN_MNEMONIC), Network::Testnet); + let err = wallet + .drain_pending_contact_crypto_verified(&foreign, None::<&UnusedSigner>, None) + .await + .expect_err("a provider that does not own the wallet must be refused"); + + assert!( + matches!(err, PlatformWalletError::SeedMismatch { .. }), + "the refusal must be the typed wrong-seed error, got: {err:?}" + ); + assert_eq!( + receiving_account_count(&manager, &wallet_id).await, + 0, + "not one contact account may be registered from the wrong seed" + ); + assert_eq!( + drainable(&wallet).await, + 1, + "the queue must survive so the next correct-seed drain can do the work" + ); + } + + /// The other half: the wallet's own seed passes the gate and the drain + /// runs. Without this the test above would also pass if the gate simply + /// refused everything. + #[tokio::test] + async fn the_owning_seed_passes_the_gate_and_the_drain_runs() { + let (manager, wallet, wallet_id) = wallet_with_queued_contact_crypto().await; + + let owning = SeedCryptoProvider::from_seed(seed_for(TEST_MNEMONIC), Network::Testnet); + let drained = wallet + .drain_pending_contact_crypto_verified(&owning, None::<&UnusedSigner>, None) + .await + .expect("the wallet's own seed must bind"); + + assert_eq!(drained, 1, "the queued RegisterReceiving op must complete"); + assert_eq!( + receiving_account_count(&manager, &wallet_id).await, + 1, + "the contact receiving account must exist after a verified drain" + ); + } + + /// The gate is paid for only when there is something to protect. An empty + /// queue would derive nothing, so no key material is resolved — which is + /// what keeps this affordable on a warm launch. Proven with a provider + /// that would FAIL the check: an `Ok` shows it was never consulted. + #[tokio::test] + async fn an_empty_queue_never_consults_the_provider() { + let manager = make_manager(); + let wallet = manager + .create_wallet_from_seed_bytes( + Network::Testnet, + &seed_for(TEST_MNEMONIC), + WalletAccountCreationOptions::Default, + Some(0), + ) + .await + .expect("wallet creation"); + assert_eq!(drainable(&wallet).await, 0); + + let foreign = SeedCryptoProvider::from_seed(seed_for(FOREIGN_MNEMONIC), Network::Testnet); + let drained = wallet + .drain_pending_contact_crypto_verified(&foreign, None::<&UnusedSigner>, None) + .await + .expect("with nothing to drain the binding check must not run at all"); + assert_eq!(drained, 0); + } } diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/manager/accessors.rs b/packages/rs-platform-wallet/src/wallet/identity/state/manager/accessors.rs index ea80ea19ae..fcabcc3fdc 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/manager/accessors.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/manager/accessors.rs @@ -212,4 +212,40 @@ impl IdentityManager { .get(wallet_id) .and_then(|m| m.keys().last().copied()) } + + /// Verdict of the last gap-limit identity scan for `wallet_id`, if one is + /// known. + pub fn identity_scan_state( + &self, + wallet_id: &WalletId, + ) -> Option<&crate::changeset::IdentityScanStateEntry> { + self.identity_scan_states.get(wallet_id) + } + + /// Whether a scan is known to have left indices unanswered. + /// + /// The question the warm-launch shortcut asks, phrased so that only + /// positive evidence of an incomplete scan can force a rescan. Deliberately + /// **not** `!is_complete()`: an absent verdict means nobody recorded one — + /// a host that does not persist it, or a wallet whose identities predate + /// this bookkeeping — and treating "unknown" as "incomplete" would make + /// every launch on such a host pay for a full scan plus its Keychain round + /// trip, which is the cost the warm-launch shortcut exists to avoid. + pub fn identity_scan_is_incomplete(&self, wallet_id: &WalletId) -> bool { + self.identity_scan_states + .get(wallet_id) + .is_some_and(|state| !state.complete) + } + + /// Record the verdict of a gap-limit scan for `wallet_id`. + /// + /// In-memory only — the caller emits the matching changeset entry, because + /// only it holds the persister. + pub fn record_identity_scan( + &mut self, + wallet_id: WalletId, + state: crate::changeset::IdentityScanStateEntry, + ) { + self.identity_scan_states.insert(wallet_id, state); + } } diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/manager/mod.rs b/packages/rs-platform-wallet/src/wallet/identity/state/manager/mod.rs index 4b13ae5c4c..4c20f791dc 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/manager/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/manager/mod.rs @@ -31,7 +31,7 @@ mod apply; mod lifecycle; use super::managed_identity::ManagedIdentity; -use crate::changeset::IdentityManagerStartState; +use crate::changeset::{IdentityManagerStartState, IdentityScanStateEntry}; use crate::wallet::platform_wallet::WalletId; use dpp::identity::accessors::IdentityGettersV0; use dpp::prelude::Identifier; @@ -95,6 +95,18 @@ pub struct IdentityManager { /// callers that need to drop an identity reach the buckets through /// `remove_for_apply` so the index stays in sync. location_index: BTreeMap, + + /// Per-wallet verdict of the last gap-limit identity scan, keyed by wallet + /// id because a scan is a wallet-scoped act even though its result is a + /// set of identities. + /// + /// Consulted by the startup sequence before it takes the warm-launch + /// shortcut: a scan that could not answer every index must not let a + /// later launch conclude the identity set is settled. An absent entry + /// means no verdict is known — see + /// [`IdentityManagerStartState::scan_states`] for why that is deliberately + /// not read as "complete". + identity_scan_states: BTreeMap, } impl From for IdentityManager { @@ -102,6 +114,7 @@ impl From for IdentityManager { let IdentityManagerStartState { out_of_wallet_identities, wallet_identities, + scan_states, } = state; // Rebuild the side-index from the two buckets — `IdentityManagerStartState` @@ -127,6 +140,7 @@ impl From for IdentityManager { out_of_wallet_identities, wallet_identities, location_index, + identity_scan_states: scan_states, } } } @@ -405,6 +419,83 @@ mod tests { assert!(manager.location_index().is_empty()); } + /// The cross-launch half of dashpay/platform#4365: an incomplete scan + /// verdict restored from the start state must still say "incomplete", or + /// the next launch takes the warm shortcut over an identity set that was + /// never fully probed. + #[test] + fn an_incomplete_scan_verdict_survives_a_restore() { + use crate::changeset::{IdentityManagerStartState, IdentityScanStateEntry}; + + let wallet: WalletId = [10u8; 32]; + let mut state = IdentityManagerStartState::default(); + state + .scan_states + .insert(wallet, IdentityScanStateEntry::incomplete(5, vec![1])); + + let manager = IdentityManager::from(state); + + assert!( + manager.identity_scan_is_incomplete(&wallet), + "a restored partial scan must still force a rescan" + ); + assert_eq!( + manager + .identity_scan_state(&wallet) + .expect("verdict restored") + .failed_indices, + vec![1] + ); + } + + /// The other side of it: a scan that answered everything restores as + /// complete, so the warm-launch shortcut keeps working and a healthy + /// wallet pays for no probes. + #[test] + fn a_complete_scan_verdict_permits_the_warm_shortcut() { + use crate::changeset::{IdentityManagerStartState, IdentityScanStateEntry}; + + let wallet: WalletId = [10u8; 32]; + let mut state = IdentityManagerStartState::default(); + state + .scan_states + .insert(wallet, IdentityScanStateEntry::completed(6)); + + let manager = IdentityManager::from(state); + + assert!(!manager.identity_scan_is_incomplete(&wallet)); + } + + /// "No verdict" is not "incomplete". Every wallet that predates this + /// bookkeeping, and every host that does not persist the verdict yet, + /// lands here — and forcing them all to rescan on every launch would cost + /// a full gap-limit scan plus a Keychain round trip before every Core SPV + /// start, which is the cost the warm shortcut exists to avoid. + #[test] + fn an_unknown_scan_verdict_does_not_force_a_rescan() { + let manager = IdentityManager::new(); + + assert!(!manager.identity_scan_is_incomplete(&[42u8; 32])); + assert!(manager.identity_scan_state(&[42u8; 32]).is_none()); + } + + /// A later scan's verdict wholly supersedes an earlier one's — that is + /// what lets a clean rescan clear a prior partial scan and hand the + /// shortcut back. + #[test] + fn a_clean_rescan_clears_an_earlier_partial_verdict() { + use crate::changeset::IdentityScanStateEntry; + + let wallet: WalletId = [10u8; 32]; + let mut manager = IdentityManager::new(); + + manager.record_identity_scan(wallet, IdentityScanStateEntry::incomplete(5, vec![1])); + assert!(manager.identity_scan_is_incomplete(&wallet)); + + manager.record_identity_scan(wallet, IdentityScanStateEntry::completed(6)); + assert!(!manager.identity_scan_is_incomplete(&wallet)); + } + #[test] fn from_start_state_rebuilds_location_index() { use crate::changeset::IdentityManagerStartState; diff --git a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift index 9e65f891cb..c8df1c185d 100644 --- a/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift +++ b/packages/swift-sdk/Sources/SwiftDashSDK/PlatformWallet/PlatformWalletManagerStartup.swift @@ -30,20 +30,49 @@ public enum WalletStartupStatus: UInt8, Sendable { /// reachability problem. The identity question is unanswered, and another /// scan will not answer it: the same fault is still there. case discoveryFailed = 4 + /// The contact-crypto provider does not resolve the seed that owns this + /// wallet, so the contact-account drain was skipped without deriving + /// anything. + /// + /// Unlike the other partial cases this one is not about Platform being + /// slow — it says the signer handed to the call belongs to a different + /// wallet. Deriving anyway would write contact receiving addresses from + /// the wrong seed that no later correct-seed pass would ever revisit, so + /// doing nothing is the only safe response. Check the Keychain mapping for + /// this wallet; a rerun with the right signer completes the work, which is + /// still queued. + case seedBindingUnverified = 5 + /// An identity is known and every later step ran, but the gap-limit + /// identity scan is still on record as having left indices unanswered. + /// + /// Not a failure of this launch's work — the contact sync and the drain + /// both ran for the identity that *is* known. It says the identity SET is + /// not established: an identity sitting at an unanswered index is + /// invisible to everything that reads local state, and reporting ``ready`` + /// would promise a set this launch never proved. The verdict stays on + /// record, so the next launch re-scans instead of taking the warm + /// shortcut. + case identityScanIncomplete = 6 /// Whether another discovery scan could change the answer. /// - /// 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 } + /// True for ``partialNoIdentity`` (Platform was never reached) and + /// ``identityScanIncomplete`` (it was reached, but not for every index). + /// The others are terminal for this launch — absence was proven, the + /// failure is local and will still be there next time, or the scan + /// answered everything it probed. + public var discoveryWorthRetrying: Bool { + self == .partialNoIdentity || self == .identityScanIncomplete + } /// 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. + /// leaves the question open *and* is not worth retrying, while + /// ``identityScanIncomplete`` has an answer that is merely known to be + /// partial — an identity was found, so there is something to show. Use + /// this to decide what to show, and ``discoveryWorthRetrying`` to decide + /// whether to scan again. public var identityIsSettled: Bool { self != .partialNoIdentity && self != .discoveryFailed } @@ -57,8 +86,21 @@ public struct WalletStartupOutcome: Sendable, Equatable { /// 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. + /// Whether the inline contact-request pass ran **to completion**. `false` + /// when it was skipped, failed, ran out of budget, or came back degraded: + /// a pass that could not read some identities' contact documents left + /// their account builds unenqueued, so an empty pending count does not + /// mean their addresses are ready. public let dashPaySyncRan: Bool + /// The contact-account drain was skipped because the contact-crypto + /// provider does not resolve this wallet's seed. Nothing was derived and + /// nothing was written; the queued work is intact. + public let seedBindingUnverified: Bool + /// The wallet's identity scan is on record as having left indices + /// unanswered and this launch did not close the gap. Any identity reported + /// here is real; it may not be the only one. Carried separately from + /// ``status`` because a pending contact queue outranks it there. + public let identityScanIncomplete: Bool /// Contact-crypto entries the drain completed. public let contactAccountsDrained: UInt32 /// Contact-account builds still queued on return. Non-zero means the @@ -142,6 +184,15 @@ extension PlatformWalletManager { // races this call. The verify is marker-cached, so the common path // costs a string comparison. // + // The shared Rust sequence now runs the same check of its own, just + // before the drain and only when something is actually queued, so a + // future JNI client inherits the gate instead of the bug. The two are + // not redundant: this one throws, refusing the call outright, while + // the Rust one fails closed and reports `seedBindingUnverified` — it + // has to let Core SPV start regardless. Keeping this here is what + // turns a wrong-seed pairing into a loud error on iOS rather than a + // silently degraded launch. + // // Against `storage`, not a default one: the resolver below reads that // store, and verifying a different Keychain than the work will use // would approve one mnemonic while another derives the accounts. @@ -233,6 +284,8 @@ extension WalletStartupOutcome { : nil self.discoveryAttempts = ffi.discovery_attempts self.dashPaySyncRan = ffi.dashpay_sync_ran + self.seedBindingUnverified = ffi.seed_binding_unverified + self.identityScanIncomplete = ffi.identity_scan_incomplete self.contactAccountsDrained = ffi.contact_accounts_drained self.contactAccountsPending = ffi.contact_accounts_pending self.elapsed = TimeInterval(ffi.elapsed_ms) / 1000