diff --git a/packages/rs-platform-wallet-ffi/src/dashpay.rs b/packages/rs-platform-wallet-ffi/src/dashpay.rs index f87050caec6..168c680147c 100644 --- a/packages/rs-platform-wallet-ffi/src/dashpay.rs +++ b/packages/rs-platform-wallet-ffi/src/dashpay.rs @@ -598,6 +598,20 @@ pub unsafe extern "C" fn platform_wallet_fetch_sent_contact_requests( /// The wallet seed is never made resident; every signature is produced /// inside the signer's atomic derive-and-sign step. /// +/// # Seed binding +/// +/// The send begins by draining any deferred contact-crypto build for this +/// contact, which derives and registers contact accounts from whatever seed +/// the resolver resolves. That drain runs behind the same gate as +/// `platform_wallet_drain_pending_contact_crypto`: whenever there is drainable +/// work, the resolver is checked against this wallet's persisted BIP44 +/// account-0 xpub first. A resolver mapped to a different wallet fails the +/// call with `ErrorInvalidParameter`, derives NOTHING, and leaves the queue +/// intact. Without the gate the wrong-seed contact account would be written +/// permanently (`register_contact_account` keys its existence check on the +/// contact pair, not the xpub) and the payment would then fail anyway on the +/// funding signatures — corruption first, error second. +/// /// # Safety /// - `core_signer_handle` must be a valid, non-destroyed /// `*mut MnemonicResolverHandle`. Ownership is retained by the caller — @@ -664,7 +678,22 @@ pub unsafe extern "C" fn platform_wallet_send_dashpay_payment( }) }); let result = unwrap_option_or_return!(option); - let (txid, _entry, fee_duffs) = unwrap_result_or_return!(result); + // The send opens with the SEED-VERIFIED contact-crypto drain, so a + // resolver mapped to a different wallet is refused here instead of + // registering a wrong-seed contact account and only then failing on the + // funding signatures. Reported with the same code the standalone verify + // and the drain entry point use, so a host recognizes the wrong-seed + // condition identically however it arrives. + let (txid, _entry, fee_duffs) = match result { + Ok(v) => v, + Err(e @ platform_wallet::PlatformWalletError::SeedMismatch { .. }) => { + return PlatformWalletFFIResult::err( + PlatformWalletFFIResultCode::ErrorInvalidParameter, + e.to_string(), + ); + } + Err(e) => return e.into(), + }; // Exact network fee of the broadcast transaction — Σ(selected input // values) − Σ(output values), computed by the transaction builder // itself since rust-dashcore#872, so a sub-dust change remainder @@ -839,6 +868,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 +903,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 +917,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 +928,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 dbf1ff0eb5c..6d3c68ab124 100644 --- a/packages/rs-platform-wallet-ffi/src/persistence.rs +++ b/packages/rs-platform-wallet-ffi/src/persistence.rs @@ -5035,6 +5035,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 583dfc11bb4..bb1b1840858 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 bea9b3d95aa..d36fcfdc76b 100644 --- a/packages/rs-platform-wallet/src/changeset/changeset.rs +++ b/packages/rs-platform-wallet/src/changeset/changeset.rs @@ -35,6 +35,7 @@ use key_wallet::managed_account::address_pool::{AddressPool, AddressPoolType}; use key_wallet::managed_account::transaction_record::TransactionRecord; use key_wallet::{AddressInfo, Network, PlatformP2PKHAddress, Utxo}; +use crate::changeset::identity_scan_state::IdentityScanStateEntry; use crate::wallet::platform_wallet::WalletId; use dpp::balances::credits::Credits; @@ -1884,6 +1885,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 @@ -2019,6 +2032,17 @@ impl Merge for PlatformWalletChangeSet { if let Some(meta) = other.wallet_metadata { self.wallet_metadata = Some(meta); } + // Identity-scan verdict: the later scan's verdict folded over the + // earlier one, on the rule the manager applies — see + // `IdentityScanStateEntry::superseding`. Overwriting instead would let + // a scan batched into the same persist round clear a gap it never + // probed, which is the whole reason the verdict is recorded. + if let Some(scan) = other.identity_scan_state { + self.identity_scan_state = Some(match self.identity_scan_state.take() { + Some(previous) => scan.superseding(&previous), + None => 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 @@ -2057,6 +2081,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 fbb42fa9e09..a4072b61848 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/identity_scan_state.rs b/packages/rs-platform-wallet/src/changeset/identity_scan_state.rs new file mode 100644 index 00000000000..40eca834175 --- /dev/null +++ b/packages/rs-platform-wallet/src/changeset/identity_scan_state.rs @@ -0,0 +1,148 @@ +//! Verdict of the last gap-limit identity scan — what it probed, what it +//! could not answer, and how a later scan's verdict folds over it. +//! +//! Carried on [`PlatformWalletChangeSet::identity_scan_state`] and restored +//! through [`IdentityManagerStartState::scan_states`]; the startup sequence +//! reads it to decide whether the warm-launch shortcut may skip discovery. +//! +//! [`PlatformWalletChangeSet::identity_scan_state`]: crate::changeset::PlatformWalletChangeSet::identity_scan_state +//! [`IdentityManagerStartState::scan_states`]: crate::changeset::IdentityManagerStartState::scan_states + +/// 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. A newer verdict is folded over the older one rather than +/// replacing it outright; see [`IdentityScanStateEntry::superseding`] for why +/// replacing loses gaps. +#[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, and nothing an earlier scan + /// left unanswered is still outstanding. Only a `true` here may let a + /// later launch skip discovery. + pub complete: bool, + /// The lowest index the scan probed. Together with + /// [`Self::probed_through`] this is the scan's coverage — what it is + /// entitled to have an opinion about, and the reason a suffix scan cannot + /// clear a gap below where it started. + pub probed_from: u32, + /// One past the highest index the scan probed. + pub probed_through: u32, + /// Indices whose probe never got an answer, ascending — this scan's own, + /// plus any an earlier scan left that this one did not cover. Empty for a + /// scan that was cut off before it could fail anything. + pub failed_indices: Vec, + /// A scan ended without naming where its gap was, and nothing has covered + /// that region since. + /// + /// A scan abandoned mid-await answered no index and failed none, so + /// [`Self::failed_indices`] cannot speak for it: what it never reached has + /// no name. Only a scan that starts at index 0 and answers everything it + /// probed covers a region nobody can point at, so the fact rides the state + /// until one does. + /// + /// Stored rather than read back off `failed_indices` because a fold mixes + /// the two kinds of gap. An unlocated gap followed by a suffix scan with + /// unanswered probes of its own produces a state with a non-empty failed + /// list, at which point the derived reading says "located" and a later + /// suffix scan covering those names hands the shortcut back over the + /// original gap. + pub unlocated_gap: bool, +} + +impl IdentityScanStateEntry { + /// A scan that answered every index in `probed_from..probed_through`. + pub fn completed(probed_from: u32, probed_through: u32) -> Self { + Self { + complete: true, + probed_from, + probed_through, + failed_indices: Vec::new(), + unlocated_gap: false, + } + } + + /// A scan that left at least one index unanswered, or was abandoned + /// before it could finish. + pub fn incomplete(probed_from: u32, probed_through: u32, failed_indices: Vec) -> Self { + Self { + complete: false, + // A scan that named an unanswered index located its gap; one that + // named none was cut off before it could, and its gap has no name. + unlocated_gap: failed_indices.is_empty(), + probed_from, + probed_through, + failed_indices, + } + } + + /// Fold this scan's verdict over `previous`, the one already on record. + /// + /// A scan answers the range it walked and nothing else, so an index + /// `previous` recorded as unanswered is still unanswered unless this scan + /// covered it. Replacing the verdict outright is what let a clean suffix + /// scan erase a gap it never probed: discovery resumes one past the + /// highest registered identity by default, so a wallet with identities at + /// 0 and 2 and no answer at 1 resumes at 3, answers everything from there + /// cleanly, and publishes `complete` — after which the warm-launch + /// shortcut reports a settled identity set while the identity at index 1 + /// and all of its contacts stay missing. That is the same + /// Ready-over-an-unprobed-gap failure the verdict exists to prevent, + /// reached from the other side. + /// + /// A gap this scan covered and answered is cleared; one it re-probed and + /// still could not answer is already among its own `failed_indices`. A gap + /// nobody could name is carried in [`Self::unlocated_gap`], which only a + /// clean scan starting at index 0 clears — a from-zero scan cut short + /// covered no more than the window it walked, so the unknown region above + /// it is still unknown. The folds in between may add and clear named gaps + /// freely without touching it. The result is complete only when this scan + /// was clean AND it left nothing carried over of either kind. + pub fn superseding(mut self, previous: &Self) -> Self { + let covered = self.probed_from..self.probed_through; + for index in &previous.failed_indices { + if !covered.contains(index) && !self.failed_indices.contains(index) { + self.failed_indices.push(*index); + } + } + self.failed_indices.sort_unstable(); + + // An unlocated gap is carried as a fact rather than re-derived from + // `failed_indices`, which cannot hold a gap that has no name. Only a + // scan that starts at the bottom of the index space and answers + // everything it probed can be said to have covered it: one that starts + // there and is itself cut short walked only as far as it got, and the + // region above that is the same one nobody could point at. So nothing + // narrower and nothing unfinished supersedes it — until one does it + // survives every fold in between, including the ones that put named + // gaps of their own into the list. + let supersedes_unlocated_gap = self.complete && self.probed_from == 0; + self.unlocated_gap = + self.unlocated_gap || (previous.unlocated_gap && !supersedes_unlocated_gap); + + self.complete = self.complete && self.failed_indices.is_empty() && !self.unlocated_gap; + self + } +} diff --git a/packages/rs-platform-wallet/src/changeset/mod.rs b/packages/rs-platform-wallet/src/changeset/mod.rs index e87cc14aeec..850c99b8916 100644 --- a/packages/rs-platform-wallet/src/changeset/mod.rs +++ b/packages/rs-platform-wallet/src/changeset/mod.rs @@ -14,6 +14,7 @@ pub mod client_start_state; pub mod client_wallet_start_state; pub mod core_bridge; pub mod identity_manager_start_state; +pub mod identity_scan_state; pub mod merge; pub mod persistence_capabilities; pub mod platform_address_sync_start_state; @@ -42,6 +43,7 @@ pub use client_start_state::ClientStartState; pub use client_wallet_start_state::ClientWalletStartState; pub use core_bridge::spawn_wallet_event_adapter; pub use identity_manager_start_state::IdentityManagerStartState; +pub use identity_scan_state::IdentityScanStateEntry; pub use merge::Merge; pub use persistence_capabilities::{PersistenceCapabilities, PERSISTENCE_CAPABILITIES_VERSION}; pub use platform_address_sync_start_state::PlatformAddressSyncStartState; diff --git a/packages/rs-platform-wallet/src/error.rs b/packages/rs-platform-wallet/src/error.rs index d3a53cade57..740bf64612f 100644 --- a/packages/rs-platform-wallet/src/error.rs +++ b/packages/rs-platform-wallet/src/error.rs @@ -503,6 +503,41 @@ pub enum PlatformWalletError { wallet_id: String, }, + #[error( + "Seed-binding check for wallet {wallet_id} did not answer within the \ + caller's deadline (refusing to derive through a provider that was \ + never checked)" + )] + /// The contact-crypto provider did not return the BIP44 account-0 xpub + /// before the deadline the caller supplied — a stalled host Keychain / + /// Keystore, or a budget already spent by the time the gated pass was + /// reached. Distinct from [`Self::SeedMismatch`], which is a *proven* + /// wrong seed: this one proves nothing either way, which is why it is + /// refused just as firmly. The check derives nothing and commits nothing, + /// so the queue survives for the next signer-present pass. + SeedBindingUnanswered { + /// Hex of the wallet id whose binding could not be established. + 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 1b5ac7fa031..168512f4d40 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,46 @@ 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), or reached them all and could not write what came back. + // The requests it did fetch AND persist are real, 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 whichever door they came through: + // a fetch that errored and an ingest that could not persist BOTH + // leave 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(), + unpersisted = report.unpersisted_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 +693,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 +741,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 +754,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; @@ -620,11 +840,33 @@ impl PlatformWalletManager // this call promises. let attempt_future = async { match master.as_ref() { - Some(master) => identity_wallet.discover_from_master(opts, master).await, - None => identity_wallet.discover(opts).await, + // `Some(deadline)` bounds the DPNS enrichment tail only, + // never the scan. The outer `within_budget` below still + // owns the scan's ceiling; what this adds is a way for + // enrichment to stop on its own, so a slow DPNS pass no + // longer gets the whole call cancelled out from under a + // scan that already published a complete verdict — which + // arrives here as a scan that recorded nothing, sending + // `record_identity_scan_cut_off` to overwrite it and set a + // sticky `unlocated_gap`. + Some(master) => { + identity_wallet + .discover_from_master_until(opts, master, Some(deadline)) + .await + } + None => identity_wallet.discover_until(opts, Some(deadline)).await, } }; 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 +878,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(); @@ -662,6 +929,18 @@ impl PlatformWalletManager "startup: identity discovery failed for a non-network reason" ); tally.record_discovery_failed_locally(); + // The scan folds each identity in as it finds it, so a + // fault raised after one was inserted leaves that identity + // resident and its keys in memory. The failure says + // persistence broke, NOT that the wallet owns nothing — + // reporting it as identity-less would exit at the + // no-identity guard and skip the contact sync and drain + // for an identity that is right there. Both signals are + // kept: the identity is reported and the local fault stays + // on record. + if let Some(known) = self.local_identity_id(wallet_id).await { + tally.record_local_identity(known); + } return; } } @@ -678,7 +957,45 @@ 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) { + // Coverage of nothing: the scan was dropped mid-await, so it answered + // no index and may not clear one an earlier scan left open. + let recorded = { + 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, 0, Vec::new()), + ), + None => return, + } + }; + let changeset = crate::changeset::PlatformWalletChangeSet { + identity_scan_state: Some(recorded), + ..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 +1069,7 @@ mod tests { WalletStartupStatus::NoIdentity, WalletStartupStatus::PartialAccountsPending, WalletStartupStatus::DiscoveryFailed, + WalletStartupStatus::SeedBindingUnverified, ] { assert!( !terminal.discovery_worth_retrying(), @@ -836,6 +1154,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 +1333,456 @@ 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 covered_through; + { + 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" + ); + covered_through = verdict.probed_through; + } + + 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"); + // Coverage matters: a verdict only clears the gaps it walked, so + // this stand-in for a clean rescan has to span the same indices + // the creation scan left unanswered. + info.identity_manager.record_identity_scan( + wallet_id, + IdentityScanStateEntry::completed(0, covered_through), + ); + } + + 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(); @@ -882,4 +1797,175 @@ mod tests { assert_eq!(outcome.contact_accounts_drained, 1); assert_eq!(outcome.elapsed, Duration::from_secs(3)); } + // --------------------------------------------------------------------- + // Discovery mutates identity state incrementally, so a fault raised + // part-way through does not mean the wallet owns nothing. + // --------------------------------------------------------------------- + + /// Persister handing back one already-persisted wallet, the way a restored + /// launch hydrates. Its wallet is `ExternalSignable` — the Keychain-backed + /// shape whose seed lives outside the process — which is what makes the + /// resident-key derive inside `discover` fail with a LOCAL fault rather + /// than an unreachable-Platform one. + struct RestoredWalletPersister { + wallet: key_wallet::Wallet, + managed: key_wallet::wallet::ManagedWalletInfo, + } + + impl crate::changeset::PlatformWalletPersistence for RestoredWalletPersister { + fn store( + &self, + _wallet_id: WalletId, + _changeset: crate::changeset::PlatformWalletChangeSet, + ) -> Result<(), crate::changeset::PersistenceError> { + Ok(()) + } + + fn flush(&self, _wallet_id: WalletId) -> Result<(), crate::changeset::PersistenceError> { + Ok(()) + } + + fn load( + &self, + ) -> Result + { + let mut wallets = std::collections::BTreeMap::new(); + wallets.insert( + self.wallet.compute_wallet_id(), + crate::changeset::ClientWalletStartState { + wallet: self.wallet.clone(), + wallet_info: self.managed.clone(), + identity_manager: crate::changeset::IdentityManagerStartState::default(), + unused_asset_locks: std::collections::BTreeMap::new(), + }, + ); + Ok(crate::changeset::ClientStartState { + wallets, + ..Default::default() + }) + } + } + + struct RestoreEventHandler; + impl crate::events::EventHandler for RestoreEventHandler {} + impl crate::events::PlatformEventHandler for RestoreEventHandler {} + + /// A hydrated manager holding one external-signable wallet: a scan on it + /// cannot derive from resident key material, so `discover` returns a local + /// fault instead of an unanswered-probe one. + async fn manager_whose_scan_faults_locally() -> ( + std::sync::Arc>, + WalletId, + ) { + let ctx = key_wallet::test_utils::TestWalletContext::new_random(); + let mut wallet = ctx.wallet; + wallet.downgrade_to_external_signable(); + let wallet_id = wallet.compute_wallet_id(); + + let manager = std::sync::Arc::new(crate::PlatformWalletManager::new( + std::sync::Arc::new(dash_sdk::SdkBuilder::new_mock().build().expect("mock sdk")), + std::sync::Arc::new(RestoredWalletPersister { + wallet, + managed: ctx.managed_wallet, + }), + std::sync::Arc::new(RestoreEventHandler) + as std::sync::Arc, + )); + manager + .load_from_persistor() + .await + .expect("the restored wallet must hydrate"); + + (manager, wallet_id) + } + + /// The defect: `discover` folds each identity in as it finds it — + /// `add_identity` inserts it and installs its public keys in memory — + /// before the persist that can still fail. When that failure surfaced + /// here, the branch recorded only the local-fault signal and left + /// `identity_id` unset, so the sequence exited at the no-identity guard + /// and reported a wallet with no identity while one was resident. The + /// contact sync and the contact-account drain were skipped for it. + /// + /// Reproduced by driving the retry loop directly against a wallet whose + /// scan faults locally, with the identity already folded in — the exact + /// state the failing persist leaves behind. + #[tokio::test] + async fn a_local_scan_fault_still_reports_an_identity_the_scan_had_already_folded_in() { + use crate::wallet::persister::{NoPlatformPersistence, WalletPersister}; + + let (manager, wallet_id) = manager_whose_scan_faults_locally().await; + + // What the scan had already done before its persist failed. + { + 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("fold the identity in the way discovery does"); + } + + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet handle"); + let mut tally = StartupTally::default(); + manager + .discover_identity_with_backoff( + &wallet_id, + wallet.identity(), + None, + Some(1), + Instant::now() + Duration::from_secs(30), + &mut tally, + ) + .await; + + assert!( + tally.discovery_failed_locally, + "precondition: this scan must fault locally, not go unanswered" + ); + assert_eq!( + tally.identity_id, + Some(Identifier::from([1u8; 32])), + "an identity the scan already folded in must still be reported" + ); + assert!( + tally.has_identity(), + "the no-identity guard is what skips the contact sync and the drain" + ); + assert_ne!( + tally.status(), + WalletStartupStatus::DiscoveryFailed, + "a launch that HAS an identity is not a no-identity launch" + ); + } + + /// The other direction, and the reason the branch re-reads local state + /// rather than assuming: with nothing folded in, the identical local fault + /// still settles as `DiscoveryFailed`. Without this the test above would + /// keep passing if the branch reported an identity unconditionally. + #[tokio::test] + async fn a_local_scan_fault_with_nothing_folded_in_is_still_a_failed_discovery() { + let (manager, wallet_id) = manager_whose_scan_faults_locally().await; + + let wallet = manager.get_wallet(&wallet_id).await.expect("wallet handle"); + let mut tally = StartupTally::default(); + manager + .discover_identity_with_backoff( + &wallet_id, + wallet.identity(), + None, + Some(1), + Instant::now() + Duration::from_secs(30), + &mut tally, + ) + .await; + + assert!(tally.discovery_failed_locally); + assert!( + !tally.has_identity(), + "there is no identity to report, and none may be invented" + ); + assert_eq!(tally.status(), WalletStartupStatus::DiscoveryFailed); + } } diff --git a/packages/rs-platform-wallet/src/wallet/apply.rs b/packages/rs-platform-wallet/src/wallet/apply.rs index 4390740640c..8e36af5768d 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 46fa521e4bf..e763aba8e5c 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 @@ -880,6 +880,123 @@ fn newest_sent_per_recipient( newest } +/// Ingest one identity's collapsed **received** contact requests into local +/// state, returning whether every write reached disk. +/// +/// A `false` return means a persist failed and the loop stopped there, so an +/// unknown number of the requests handed in were never ingested and their +/// account builds were never enqueued. Two things must follow from it, and +/// they are the caller's to do: leave the received high-water cursor +/// unadvanced (so the next sweep re-fetches this range) and mark the identity +/// in the [`ContactSyncReport`] (so the pass cannot report itself complete). +/// Stopping rather than continuing past the failure is deliberate — ingesting +/// later requests would let the cursor's max cover a request that never +/// persisted if the caller ever advanced it. +/// +/// Split out of the sweep so the persist-failure branches are reachable in a +/// test without standing up a Platform that answers document queries. +fn ingest_received_requests( + managed: &mut crate::wallet::identity::ManagedIdentity, + persister: &crate::wallet::persister::WalletPersister, + identity_id: Identifier, + newest_by_sender: std::collections::BTreeMap, + rotated_contacts: &mut Vec, + all_requests: &mut Vec, +) -> bool { + for (sender_id, contact_request) in newest_by_sender { + // Ignore (per-sender mute, local-only): an ignored sender's requests + // are ALL suppressed from the main pending list — including rotated + // (bumped accountReference) ones. Checked FIRST and per-sender, unlike + // the old per-(sender, accountReference) reject: if you ignored the + // person you ignored them. `unignore_sender` rewinds the cursor so + // this skip stops firing on the next sweep. + if managed.is_sender_ignored(&sender_id) { + tracing::debug!( + sender = %sender_id, + recipient = %identity_id, + account_reference = contact_request.account_reference, + "Skipping ignored sender's contact request" + ); + continue; + } + // Do NOT skip just because the sender is in `sent_contact_requests` — + // that is the reciprocal we need to let through to auto-establish. + // True dedup is (sender, accountReference): the SAME reference as the + // tracked incoming/established state is a re-ingest of a known doc; a + // DIFFERENT reference from a known sender is a rotation request + // (receive side) and must get through. + let tracked_reference = managed + .dashpay() + .incoming_contact_requests() + .get(&sender_id) + .map(|r| r.account_reference) + .or_else(|| { + managed + .dashpay() + .established_contacts() + .get(&sender_id) + .map(|c| c.incoming_request.account_reference) + }); + if tracked_reference == Some(contact_request.account_reference) { + continue; + } + + if tracked_reference.is_some() { + // Rotation: supersede the tracked request. When an established + // contact was re-keyed, queue the stale external account for + // teardown so the build sweep re-registers from the new xpub. + match managed.apply_rotated_incoming_request(contact_request.clone(), persister) { + Ok(true) => rotated_contacts.push(sender_id), + Ok(false) => {} + Err(e) => { + tracing::error!( + recipient = %identity_id, error = %e, + "received-request rotation persist failed; leaving received cursor for retry" + ); + return false; + } + } + all_requests.push(contact_request); + continue; + } + + if let Err(e) = managed.add_incoming_contact_request(contact_request.clone(), persister) { + tracing::error!( + recipient = %identity_id, error = %e, + "received-request ingest persist failed; leaving received cursor for retry" + ); + return false; + } + all_requests.push(contact_request); + } + true +} + +/// Ingest one identity's collapsed **sent** contact requests into local state, +/// returning whether every write reached disk. The sent-side counterpart of +/// [`ingest_received_requests`], with the same contract on a `false` return: +/// the sent cursor stays unadvanced and the identity is marked in the report. +/// +/// `add_sent_contact_request` carries its own duplicate / metadata-loss guard, +/// so re-ingesting the same range on the next sweep is safe. +fn ingest_sent_requests( + managed: &mut crate::wallet::identity::ManagedIdentity, + persister: &crate::wallet::persister::WalletPersister, + identity_id: Identifier, + newest_by_recipient: std::collections::BTreeMap, +) -> bool { + for (_recipient_id, contact_request) in newest_by_recipient { + if let Err(e) = managed.add_sent_contact_request(contact_request, persister) { + tracing::error!( + owner = %identity_id, error = %e, + "sent-request ingest persist failed; leaving sent cursor for retry" + ); + return false; + } + } + true +} + /// Snapshot-aware removal of drained queue entries. Removes from `queue` /// only those entries still **value-equal** to a snapshot in `drained` — /// the full entries snapshotted before the lock-free drain. An entry a @@ -942,7 +1059,10 @@ async fn bounded( /// Whether `deadline` has passed. Checked at the top of each drain iteration so /// a spent budget ends the loop between entries, never inside one. -fn budget_spent(deadline: Option) -> bool { +/// +/// Shared with the discovery scan's DPNS enrichment tail, which needs the same +/// between-items stop for the same reason. +pub(crate) fn budget_spent(deadline: Option) -> bool { deadline.is_some_and(|d| std::time::Instant::now() >= d) } @@ -1089,6 +1209,88 @@ 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 whose **received-side fetch** did not come back. Purely a + /// statement about reaching Platform — a local fault is NOT recorded here + /// (see [`Self::unpersisted_identities`]), because this list is what + /// [`Self::is_fully_degraded`] reads to call an outage, and a local + /// failure on a pass Platform answered in full is not an outage. 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. Remote, like + /// [`Self::failed_identities`]. + pub degraded_identities: Vec, + /// Identities whose fetches were answered but whose **local ingest** did + /// not land: a persister `store()` failure part-way through either + /// direction, or the wallet / managed identity being gone by the time the + /// write guard was taken. + /// + /// Kept apart from the two remote lists on purpose. The failure is real + /// and definitionally leaves the pass incomplete — the ingest loop + /// `break`s, abandoning every remaining fetched request of that direction, + /// and holds that direction's cursor back for retry — so it must stop the + /// pass being recorded as a completed sync. But it says nothing about + /// Platform's reachability, so it must not be able to turn a pass that + /// reached everybody into [`Self::is_fully_degraded`] and, through + /// [`DashPayView::sync_contact_requests`], a + /// [`PlatformWalletError::ContactSyncUnreachable`]. + pub unpersisted_identities: Vec, +} + +impl ContactSyncReport { + /// Every identity's fetches, both directions, were answered **and** + /// everything they returned reached local state. + /// + /// 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() + && self.unpersisted_identities.is_empty() + } + + /// Not one identity's contact documents could be **read from Platform**. + /// + /// 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. Neither is a wallet whose fetches all succeeded and + /// whose local writes all failed — that is a local fault, and reporting it + /// as an outage would send a host retrying the network for a disk it + /// cannot write. + 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 +1321,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 +1378,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 +1404,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 +1432,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 +1460,20 @@ 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 — + // nothing of this identity's is ingested. A LOCAL fault, + // not a remote one: Platform answered. Recorded so the + // pass cannot be called complete, and recorded in the + // local bucket so it cannot be mistaken for an outage. + report.unpersisted_identities.push(identity_id); continue; }; let managed = match info.identity_manager.managed_identity_mut(&identity_id) { Some(m) => m, - None => continue, + None => { + report.unpersisted_identities.push(identity_id); + continue; + } }; // Established contacts re-keyed by a rotation request in // this pass — their stale external accounts are torn down @@ -1232,9 +1483,10 @@ impl DashPayView<'_, B> { // failure would let the cursor advance past a request that // never persisted, so the next `$createdAt >` sweep would skip // it. On failure we stop ingesting that direction and leave its - // cursor unadvanced so the next sweep re-fetches and retries. - let mut received_persist_ok = true; - let mut sent_persist_ok = true; + // cursor unadvanced so the next sweep re-fetches and retries — + // and, since the rest of that direction is then abandoned + // un-ingested, the identity is marked below so the pass cannot + // report itself complete. // (1) Ingest received requests. // @@ -1253,84 +1505,14 @@ impl DashPayView<'_, B> { }); let newest_by_sender = newest_received_per_sender(parsed_received); - for (sender_id, contact_request) in newest_by_sender { - // Ignore (per-sender mute, local-only): an ignored - // sender's requests are ALL suppressed from the main - // pending list — including rotated (bumped - // accountReference) ones. Checked FIRST and per-sender, - // unlike the old per-(sender, accountReference) reject: - // if you ignored the person you ignored them. - // `unignore_sender` rewinds the cursor so this skip stops - // firing on the next sweep. - if managed.is_sender_ignored(&sender_id) { - tracing::debug!( - sender = %sender_id, - recipient = %identity_id, - account_reference = contact_request.account_reference, - "Skipping ignored sender's contact request" - ); - continue; - } - // Do NOT skip just because the sender is in - // `sent_contact_requests` — that is the reciprocal we - // need to let through to auto-establish. True dedup is - // (sender, accountReference): the SAME reference as the - // tracked incoming/established state is a re-ingest of a - // known doc; a DIFFERENT reference from a known sender - // is a rotation request (receive side) and must get - // through. - let tracked_reference = managed - .dashpay() - .incoming_contact_requests() - .get(&sender_id) - .map(|r| r.account_reference) - .or_else(|| { - managed - .dashpay() - .established_contacts() - .get(&sender_id) - .map(|c| c.incoming_request.account_reference) - }); - if tracked_reference == Some(contact_request.account_reference) { - continue; - } - - if tracked_reference.is_some() { - // Rotation: supersede the tracked request. When an - // established contact was re-keyed, queue the stale - // external account for teardown so the build sweep - // below re-registers it from the new xpub. - match managed.apply_rotated_incoming_request( - contact_request.clone(), - &self.persister, - ) { - Ok(true) => rotated_contacts.push(sender_id), - Ok(false) => {} - Err(e) => { - tracing::error!( - recipient = %identity_id, error = %e, - "received-request rotation persist failed; leaving received cursor for retry" - ); - received_persist_ok = false; - break; - } - } - all_requests.push(contact_request); - continue; - } - - if let Err(e) = managed - .add_incoming_contact_request(contact_request.clone(), &self.persister) - { - tracing::error!( - recipient = %identity_id, error = %e, - "received-request ingest persist failed; leaving received cursor for retry" - ); - received_persist_ok = false; - break; - } - all_requests.push(contact_request); - } + let received_persist_ok = ingest_received_requests( + managed, + &self.persister, + identity_id, + newest_by_sender, + &mut rotated_contacts, + &mut all_requests, + ); // (2) Ingest our own sent requests. `add_sent_contact_request` // guards itself against duplicates / metadata loss. @@ -1350,18 +1532,13 @@ impl DashPayView<'_, B> { Self::parse_sent_contact_request_doc(doc, identity_id, recipient_id) }); let newest_by_recipient = newest_sent_per_recipient(parsed_sent); - for (_recipient_id, contact_request) in newest_by_recipient { - if let Err(e) = - managed.add_sent_contact_request(contact_request, &self.persister) - { - tracing::error!( - owner = %identity_id, error = %e, - "sent-request ingest persist failed; leaving sent cursor for retry" - ); - sent_persist_ok = false; - break; - } - } + + let sent_persist_ok = ingest_sent_requests( + managed, + &self.persister, + identity_id, + newest_by_recipient, + ); // (2a') Rotation self-heal across restart: an external account // rebuilt from the persisted (tombstone-less) registration @@ -1442,6 +1619,20 @@ impl DashPayView<'_, B> { managed.advance_high_water_sent(hw_sent, max_sent); } + // A held-back cursor and a report that says "complete" cannot + // both be right. Either `break` above abandoned the rest of + // that direction's fetched requests un-ingested — their + // account builds never enqueued — so the pass is incomplete by + // the same rule the cursor logic already applies to itself. + // Left unrecorded, `is_complete()` stayed true, startup called + // `record_sync_ran()`, and the launch could reach `Ready` + // promising DIP-15 addresses that were never registered: the + // headline defect of this change, reached through the local + // door instead of the fetch door. + if !received_persist_ok || !sent_persist_ok { + report.unpersisted_identities.push(identity_id); + } + // (3) Collect account-building candidates: every established // contact missing a sending (external) account, skipping // contacts whose payment channel is already marked @@ -1460,7 +1651,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`], @@ -1550,7 +1742,7 @@ impl DashPayView<'_, B> { /// Enqueue a DIP-15 `AutoAccept` op for each inbound contact request to /// `identity_id` that carries a structurally-valid `autoAcceptProof` and is - /// not yet established — so the next signer-present [`drain_auto_accepts`] + /// not yet established — so the next signer-present auto-accept pass /// verifies + auto-accepts it. Signerless (the sweep has no signer): only a /// cheap structural pre-check (length + ECDSA key-type byte) runs here; the /// cryptographic verify happens in the drain. @@ -1884,9 +2076,9 @@ impl DashPayView<'_, B> { } /// Drain the persisted deferred-crypto queue using `provider` for the - /// Keychain-derived key material. Call when a signer is available (Keychain - /// unlock, or any signer-present DashPay action). Returns the number of - /// entries completed (removed from the queue). + /// Keychain-derived key material, stopping once `deadline` passes + /// (`None` is unbounded). Returns the number of entries completed + /// (removed from the queue). /// /// Per entry: run the op; on success remove it and persist the removal; on /// unavailable/transient failure leave it for the next drain. The @@ -1894,21 +2086,24 @@ impl DashPayView<'_, B> { /// fetch + ECDH/contactInfo derivation) drain in a follow-up and are left /// queued here — so calling this is always safe, it just completes what it /// can. - pub async fn drain_pending_contact_crypto( - &self, - provider: &P, - ) -> usize { - self.drain_pending_contact_crypto_until(provider, None) - .await - } - - /// [`Self::drain_pending_contact_crypto`], stopping once `deadline` passes. /// /// The drain ends between entries, so the count it returns and the queue /// removals it persists always describe work that actually completed — /// see [`bounded`] for why this cannot be an outer timeout. Entries it /// never reached stay queued for the next drain. - pub async fn drain_pending_contact_crypto_until( + /// + /// # Crate-private + /// + /// Everything this derives comes from whatever seed `provider` resolves, + /// and none of it is authenticated: a provider mapped to another wallet + /// registers contact accounts under the wrong xpub, and + /// `register_contact_account` keys existence on the contact tuple rather + /// than on the xpub, so the wrong addresses are written once and every + /// later correct-seed pass no-ops. The check that rules that out lives in + /// [`Self::drain_pending_contact_crypto_verified`], so this primitive is + /// reachable only from inside the crate — a caller outside it cannot name + /// a drain that skipped the check. + pub(crate) async fn drain_pending_contact_crypto_until( &self, provider: &P, deadline: Option, @@ -2625,31 +2820,33 @@ impl DashPayView<'_, B> { } /// Drain queued `AutoAccept` ops (DIP-15 QR auto-accept) — verify each - /// inbound request's `autoAcceptProof` and, if valid + unexpired, auto-accept - /// it (send the reciprocal). Requires the identity `signer` (the reciprocal is - /// a signed state transition) as well as the crypto `provider` (to derive our - /// auto-accept public key); the provider-only [`drain_pending_contact_crypto`] - /// deliberately skips these. Returns the number auto-accepted. + /// inbound request's `autoAcceptProof` and, if valid + unexpired, + /// auto-accept it (send the reciprocal), stopping once `deadline` passes + /// (`None` is unbounded). Requires the identity `signer` (the reciprocal + /// is a signed state transition) as well as the crypto `provider` (to + /// derive our auto-accept public key); the provider-only + /// `drain_pending_contact_crypto_until` deliberately skips these. Returns + /// the number auto-accepted. /// /// Anti-DoS: the cheap local checks (proof present, expiry, ECDSA verify /// against our own re-derived key) run **before** any network/accept, so a /// flood of junk proofs is cleared without per-entry round-trips. Verdict /// mapping: invalid / expired / malformed / bad-index ⇒ permanent (clear); /// provider-unavailable / accept-send failure ⇒ transient (leave queued). - pub async fn drain_auto_accepts(&self, signer: &S, provider: &P) -> usize - where - S: Signer + Send + Sync, - P: ContactCryptoProvider + Sync, - { - self.drain_auto_accepts_until(signer, provider, None).await - } - - /// [`Self::drain_auto_accepts`], stopping once `deadline` passes. /// /// Ends between entries, so a reciprocal that was sent is always recorded /// as accepted — see [`bounded`] for why an outer timeout would not hold /// that. Entries it never reached stay queued. - pub async fn drain_auto_accepts_until( + /// + /// # Crate-private + /// + /// A provider resolving another wallet's seed re-derives the wrong + /// auto-accept key, so a valid proof fails to verify — and the verdict + /// mapping calls a verify failure *permanent*, clearing the entry. The + /// damage is a contact request silently dropped and never offered again, + /// which is why this primitive is reachable only through + /// [`Self::drain_auto_accepts_verified`] and only from inside the crate. + pub(crate) async fn drain_auto_accepts_until( &self, signer: &S, provider: &P, @@ -3795,6 +3992,135 @@ 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" + ); + } + + /// Platform answered every identity; the disk did not take what came back. + /// The ingest `break`s, abandoning the rest of that direction's fetched + /// requests un-ingested and holding its cursor back for retry — so the + /// pass is incomplete by the same rule the cursor logic applies to itself. + /// Reported as complete, startup called `record_sync_ran()` and the launch + /// could reach `Ready` promising DIP-15 addresses that were never + /// registered. + #[test] + fn a_local_persist_failure_makes_the_pass_incomplete() { + let report = ContactSyncReport { + identities_attempted: 2, + unpersisted_identities: vec![id(1)], + ..Default::default() + }; + + assert!( + !report.is_complete(), + "a direction whose ingest did not reach disk is not a completed sync" + ); + } + + /// The other side of that coin: a local fault is NOT an outage. Every + /// fetch was answered, so nothing here says Platform is unreachable, and + /// `sync_contact_requests` must not turn a disk problem into + /// `ContactSyncUnreachable` — which reads to a host as "retry the + /// network" for a condition retrying the network cannot fix. + #[test] + fn a_local_persist_failure_is_not_an_outage() { + let report = ContactSyncReport { + identities_attempted: 2, + unpersisted_identities: vec![id(1), id(2)], + ..Default::default() + }; + + assert!( + !report.is_fully_degraded(), + "every received fetch was answered; this is a local fault, not an outage" + ); + assert!(!report.is_complete()); + } +} + #[cfg(test)] mod sweep_tests { use super::*; @@ -3816,6 +4142,38 @@ mod sweep_tests { WalletPersister::new([0u8; 32], Arc::new(NoPlatformPersistence)) } + /// A host persister whose `store()` always fails — disk full, DB error, + /// host bug. The condition the ingest `break`s on. + struct FailingPersistence; + + impl crate::changeset::PlatformWalletPersistence for FailingPersistence { + fn store( + &self, + _wallet_id: crate::wallet::platform_wallet::WalletId, + _changeset: PlatformWalletChangeSet, + ) -> Result<(), crate::changeset::PersistenceError> { + Err(crate::changeset::PersistenceError::backend("disk full")) + } + + fn flush( + &self, + _wallet_id: crate::wallet::platform_wallet::WalletId, + ) -> Result<(), crate::changeset::PersistenceError> { + Ok(()) + } + + fn load( + &self, + ) -> Result + { + Ok(crate::changeset::ClientStartState::default()) + } + } + + fn failing_persister() -> WalletPersister { + WalletPersister::new([0u8; 32], Arc::new(FailingPersistence)) + } + fn build_test_wallet() -> Wallet { Wallet::new_random(Network::Testnet, WalletAccountCreationOptions::None) .expect("test wallet") @@ -3878,6 +4236,505 @@ mod sweep_tests { (wallet, info) } + // ----------------------------------------------------------------------- + // The ingest persist-failure branches. + // + // A `false` return is the whole signal: it is what holds that direction's + // high-water cursor back AND what marks the identity in the + // `ContactSyncReport`, so the pass cannot report itself complete after + // abandoning fetched requests un-ingested. Before this change the boolean + // reached only the cursor, and the report stayed silent. + // ----------------------------------------------------------------------- + + /// A fresh identity with nothing tracked, ready for a first ingest. + fn info_with_bare_identity(our: u8) -> PlatformWalletInfo { + let wallet = build_test_wallet(); + let mut info = empty_info(&wallet); + info.identity_manager + .add_identity(test_identity(our), 0, [0u8; 32], &noop_persister()) + .expect("add identity"); + info + } + + fn one_received( + sender: u8, + recipient: u8, + reference: u32, + ) -> BTreeMap { + newest_received_per_sender([test_request(sender, recipient, reference)]) + } + + /// The control: a persister that takes the write ingests the request and + /// reports success. Without this the failure tests below would also pass + /// against a function that always returned `false`. + #[test] + fn a_received_ingest_that_persists_reports_success() { + let our = 1u8; + let our_id = Identifier::from([our; 32]); + let mut info = info_with_bare_identity(our); + let managed = info + .identity_manager + .managed_identity_mut(&our_id) + .expect("managed identity"); + + let mut rotated = Vec::new(); + let mut all_requests = Vec::new(); + let ok = ingest_received_requests( + managed, + &noop_persister(), + our_id, + one_received(2, our, 0), + &mut rotated, + &mut all_requests, + ); + + assert!( + ok, + "a persister that succeeds must report a complete ingest" + ); + assert_eq!(all_requests.len(), 1); + assert_eq!(managed.dashpay().incoming_contact_requests().len(), 1); + } + + /// The first-ingest persist failure (`add_incoming_contact_request`). + /// The request is not tracked and must not be reported as newly + /// discovered — a caller that took it as real would act on a request that + /// no longer exists anywhere after a restart. + #[test] + fn a_received_ingest_persist_failure_reports_the_pass_incomplete() { + let our = 1u8; + let our_id = Identifier::from([our; 32]); + let mut info = info_with_bare_identity(our); + let managed = info + .identity_manager + .managed_identity_mut(&our_id) + .expect("managed identity"); + + let mut rotated = Vec::new(); + let mut all_requests = Vec::new(); + let ok = ingest_received_requests( + managed, + &failing_persister(), + our_id, + one_received(2, our, 0), + &mut rotated, + &mut all_requests, + ); + + assert!( + !ok, + "a persist failure must be reported so the cursor is held AND the pass is \ + marked incomplete" + ); + assert!( + all_requests.is_empty(), + "a request that never persisted must not be reported as newly discovered" + ); + } + + /// The rotation persist failure (`apply_rotated_incoming_request`) — the + /// second of the three branches, reached only when the sender is already + /// tracked under a different `accountReference`. + #[test] + fn a_received_rotation_persist_failure_reports_the_pass_incomplete() { + let our = 1u8; + let contact = 2u8; + let our_id = Identifier::from([our; 32]); + // Established at reference 0 by the fixture; the sweep now sees the + // sender's rotated doc at reference 7. + let (_wallet, mut info) = info_with_established_contact(our, contact); + let managed = info + .identity_manager + .managed_identity_mut(&our_id) + .expect("managed identity"); + + let mut rotated = Vec::new(); + let mut all_requests = Vec::new(); + let ok = ingest_received_requests( + managed, + &failing_persister(), + our_id, + one_received(contact, our, 7), + &mut rotated, + &mut all_requests, + ); + + assert!( + !ok, + "a rotation whose persist failed leaves the pass incomplete" + ); + assert!( + rotated.is_empty(), + "an unpersisted rotation must not tear down the external account" + ); + } + + /// The sent-side persist failure (`add_sent_contact_request`) — the third + /// branch. Its own direction's cursor is the one held back, and the + /// identity is marked degraded rather than failed, but the pass is no more + /// complete than in the received case. + #[test] + fn a_sent_ingest_persist_failure_reports_the_pass_incomplete() { + let our = 1u8; + let our_id = Identifier::from([our; 32]); + let mut info = info_with_bare_identity(our); + let managed = info + .identity_manager + .managed_identity_mut(&our_id) + .expect("managed identity"); + + let newest = newest_sent_per_recipient([test_request(our, 2, 0)]); + assert!(!ingest_sent_requests( + managed, + &failing_persister(), + our_id, + newest + )); + + // Control on the same fixture: the write succeeds, so the ingest + // reports success. + let newest = newest_sent_per_recipient([test_request(our, 3, 0)]); + assert!(ingest_sent_requests( + managed, + &noop_persister(), + our_id, + newest + )); + } + + // ----------------------------------------------------------------------- + // The NEXT sweep has to actually retry the write. + // + // Holding the direction's high-water cursor makes the next sweep re-fetch + // the same range, and reporting the pass incomplete stops the launch + // claiming a sync it did not finish. Review found that neither of those + // gets the write to disk on its own: the state methods committed the + // mutation to memory BEFORE calling `persister.store`, so a failed store + // left the request sitting in `incoming_contact_requests` / + // `established_contacts` / `sent_contact_requests` anyway. The re-fetched + // range then hit the same-reference dedup — `tracked_reference == + // Some(request.account_reference)` here, the no-op guards inside + // `add_sent_contact_request`, the `already_applied` guard inside + // `apply_rotated_incoming_request` — reported success and advanced the + // cursor. The backend never received the write, a later startup called the + // sync complete and reached `Ready`, and the contact was gone after a + // restart. + // + // These three drive two sweeps over the same fetched range, the first + // against a persister that fails and the second against one that takes the + // write, and assert the second actually ingests. Each covers one of the + // three branches. + // ----------------------------------------------------------------------- + + /// Counts the writes that reached the backend, so a retry that silently + /// no-ops is distinguishable from one that re-stored. + #[derive(Default)] + struct CountingPersistence(std::sync::atomic::AtomicUsize); + + impl crate::changeset::PlatformWalletPersistence for CountingPersistence { + fn store( + &self, + _wallet_id: crate::wallet::platform_wallet::WalletId, + _changeset: crate::changeset::PlatformWalletChangeSet, + ) -> Result<(), crate::changeset::PersistenceError> { + self.0.fetch_add(1, std::sync::atomic::Ordering::SeqCst); + Ok(()) + } + + fn flush( + &self, + _wallet_id: crate::wallet::platform_wallet::WalletId, + ) -> Result<(), crate::changeset::PersistenceError> { + Ok(()) + } + + fn load( + &self, + ) -> Result + { + Ok(crate::changeset::ClientStartState::default()) + } + } + + fn counting_persister() -> (WalletPersister, Arc) { + let backend = Arc::new(CountingPersistence::default()); + (WalletPersister::new([0u8; 32], backend.clone()), backend) + } + + fn store_count(backend: &Arc) -> usize { + backend.0.load(std::sync::atomic::Ordering::SeqCst) + } + + /// Branch 1, the fresh received request (`add_incoming_contact_request`). + /// The first sweep's persist fails; the second must re-ingest the same + /// request and get it to disk, not skip it as already tracked. + #[test] + fn a_received_ingest_that_failed_to_persist_is_retried_by_the_next_sweep() { + let our = 1u8; + let our_id = Identifier::from([our; 32]); + let mut info = info_with_bare_identity(our); + let managed = info + .identity_manager + .managed_identity_mut(&our_id) + .expect("managed identity"); + + // Sweep 1: the write fails, so the cursor is held and the pass is + // reported incomplete. + let mut rotated = Vec::new(); + let mut all_requests = Vec::new(); + assert!( + !ingest_received_requests( + managed, + &failing_persister(), + our_id, + one_received(2, our, 0), + &mut rotated, + &mut all_requests, + ), + "precondition: the failed write must report the pass incomplete" + ); + assert!( + managed.dashpay().incoming_contact_requests().is_empty(), + "a request that never reached disk must not be tracked in memory, or the \ + retry below is skipped as already known" + ); + + // Sweep 2: the held-back cursor re-fetches the SAME range against a + // working persister. + let (persister, backend) = counting_persister(); + let mut rotated = Vec::new(); + let mut all_requests = Vec::new(); + assert!( + ingest_received_requests( + managed, + &persister, + our_id, + one_received(2, our, 0), + &mut rotated, + &mut all_requests, + ), + "the retry must complete the pass" + ); + + assert_eq!( + store_count(&backend), + 1, + "the retry must actually re-store — a no-op that reports success \ + advances the cursor over a write the backend never received" + ); + assert_eq!( + all_requests.len(), + 1, + "the retried request must surface as newly discovered" + ); + assert_eq!( + managed.dashpay().incoming_contact_requests().len(), + 1, + "and land in memory once it is safely on disk" + ); + } + + /// Branch 2, the rotation (`apply_rotated_incoming_request`). The retry has + /// two guards to get past: the sweep's `tracked_reference` skip and the + /// method's own `already_applied` idempotency guard. A rotation committed + /// to memory on a failed store trips both. + #[test] + fn a_received_rotation_that_failed_to_persist_is_retried_by_the_next_sweep() { + let our = 1u8; + let contact = 2u8; + let our_id = Identifier::from([our; 32]); + let (_wallet, mut info) = info_with_established_contact(our, contact); + let managed = info + .identity_manager + .managed_identity_mut(&our_id) + .expect("managed identity"); + + // Sweep 1: the sender's rotated doc at reference 7 fails to persist. + let mut rotated = Vec::new(); + let mut all_requests = Vec::new(); + assert!( + !ingest_received_requests( + managed, + &failing_persister(), + our_id, + one_received(contact, our, 7), + &mut rotated, + &mut all_requests, + ), + "precondition: the failed rotation must report the pass incomplete" + ); + assert_eq!( + managed.dashpay().established_contacts()[&Identifier::from([contact; 32])] + .incoming_request + .account_reference, + 0, + "memory must stay on the OLD reference — on the new one, both the sweep's \ + same-reference skip and `already_applied` swallow the retry" + ); + + // Sweep 2: the same rotated doc, against a working persister. + let (persister, backend) = counting_persister(); + let mut rotated = Vec::new(); + let mut all_requests = Vec::new(); + assert!( + ingest_received_requests( + managed, + &persister, + our_id, + one_received(contact, our, 7), + &mut rotated, + &mut all_requests, + ), + "the retry must complete the pass" + ); + + assert_eq!( + store_count(&backend), + 1, + "the retried rotation must actually re-store" + ); + assert_eq!( + rotated, + vec![Identifier::from([contact; 32])], + "and re-key the contact, so the caller tears down the stale external account" + ); + assert_eq!( + managed.dashpay().established_contacts()[&Identifier::from([contact; 32])] + .incoming_request + .account_reference, + 7, + "the new key material must be the tracked one once it is on disk" + ); + } + + /// Branch 3, the fresh sent request (`add_sent_contact_request`). Its + /// same-reference no-op guard returns `Ok(())`, so a memory-committed + /// failed write makes the retry report success without storing anything. + #[test] + fn a_sent_ingest_that_failed_to_persist_is_retried_by_the_next_sweep() { + let our = 1u8; + let our_id = Identifier::from([our; 32]); + let mut info = info_with_bare_identity(our); + let managed = info + .identity_manager + .managed_identity_mut(&our_id) + .expect("managed identity"); + + // Sweep 1: the write fails. + let newest = newest_sent_per_recipient([test_request(our, 2, 0)]); + assert!( + !ingest_sent_requests(managed, &failing_persister(), our_id, newest), + "precondition: the failed write must report the pass incomplete" + ); + assert!( + managed.dashpay().sent_contact_requests().is_empty(), + "an unpersisted sent request must not be tracked, or the retry hits the \ + same-reference no-op guard" + ); + + // Sweep 2: the same range, against a working persister. + let (persister, backend) = counting_persister(); + let newest = newest_sent_per_recipient([test_request(our, 2, 0)]); + assert!( + ingest_sent_requests(managed, &persister, our_id, newest), + "the retry must complete the pass" + ); + + assert_eq!( + store_count(&backend), + 1, + "the retried sent request must actually re-store" + ); + assert_eq!( + managed.dashpay().sent_contact_requests().len(), + 1, + "and land in memory once it is safely on disk" + ); + } + + /// The auto-establish shape, which loses the most on a failed store: it + /// consumes the pending entry from the opposite direction's map. Committed + /// before the store, a failure left the incoming request *removed* and the + /// established contact tracked but unpersisted — so the retry could no + /// longer reproduce the auto-establish, and a restart came back with + /// neither the pending request nor the contact. + #[test] + fn a_failed_auto_establish_leaves_both_sides_intact_for_the_retry() { + let our = 1u8; + let contact = 2u8; + let our_id = Identifier::from([our; 32]); + let contact_id = Identifier::from([contact; 32]); + let mut info = info_with_bare_identity(our); + let managed = info + .identity_manager + .managed_identity_mut(&our_id) + .expect("managed identity"); + + // We have already sent to this contact; their reciprocal now arrives. + managed + .add_sent_contact_request(test_request(our, contact, 0), &noop_persister()) + .expect("the outgoing request persists"); + assert_eq!(managed.dashpay().sent_contact_requests().len(), 1); + + // Sweep 1: the auto-establish write fails. + let mut rotated = Vec::new(); + let mut all_requests = Vec::new(); + assert!( + !ingest_received_requests( + managed, + &failing_persister(), + our_id, + one_received(contact, our, 0), + &mut rotated, + &mut all_requests, + ), + "precondition: the failed write must report the pass incomplete" + ); + assert_eq!( + managed.dashpay().sent_contact_requests().len(), + 1, + "the outgoing request must survive the failed store — without it the retry \ + cannot reproduce the auto-establish and silently downgrades the pair" + ); + assert!( + managed.dashpay().established_contacts().is_empty(), + "and nothing may be tracked as established while it is not on disk" + ); + + // Sweep 2: the same reciprocal, against a working persister. + let (persister, backend) = counting_persister(); + let mut rotated = Vec::new(); + let mut all_requests = Vec::new(); + assert!( + ingest_received_requests( + managed, + &persister, + our_id, + one_received(contact, our, 0), + &mut rotated, + &mut all_requests, + ), + "the retry must complete the pass" + ); + + assert_eq!( + store_count(&backend), + 1, + "the retried auto-establish must actually re-store" + ); + assert!( + managed + .dashpay() + .established_contacts() + .contains_key(&contact_id), + "the contact must be established on the retry" + ); + assert!( + managed.dashpay().sent_contact_requests().is_empty(), + "and the pending outgoing entry consumed, now that the establish is on disk" + ); + } + /// **Test 3 (restore-from-seed shape):** an established contact with /// zero DashPay accounts must surface as an account-build candidate so /// the sweep rebuilds BOTH the receiving and external accounts. Before 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 ba68afb0c29..93dbfe1ff5b 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/discovery.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/discovery.rs @@ -5,6 +5,7 @@ use dpp::identity::Identity; use key_wallet::bip32::ExtendedPrivKey; use crate::error::PlatformWalletError; +use crate::wallet::identity::network::contact_requests::budget_spent; use super::*; @@ -182,7 +183,31 @@ impl IdentityWallet { &self, opts: IdentityDiscoveryOptions, ) -> Result, PlatformWalletError> { - self.discover_inner(opts, KeyHashSource::ResidentWallet) + self.discover_inner(opts, KeyHashSource::ResidentWallet, None) + .await + } + + /// [`Self::discover`], bounding the best-effort DPNS enrichment tail. + /// + /// Crate-private on purpose. The deadline is a property of the one caller + /// that owns a budget — the startup sequence, which gates Core SPV — not + /// of the discovery options every client passes, so it does not belong on + /// the public [`IdentityDiscoveryOptions`]. + /// + /// It bounds the enrichment only, never the scan: enrichment runs *after* + /// the scan verdict is published, so stopping in it costs a username + /// lookup and never a verdict. A caller that instead wraps the whole call + /// in one outer timeout cannot tell "the scan was abandoned mid-walk" from + /// "the scan finished and its enrichment ran long", and has to assume the + /// former — recording an abandoned-scan verdict over a scan that answered + /// every index, which sets a sticky `unlocated_gap` and forces a from-zero + /// rescan on every launch for as long as DPNS stays slow. + pub(crate) async fn discover_until( + &self, + opts: IdentityDiscoveryOptions, + enrichment_deadline: Option, + ) -> Result, PlatformWalletError> { + self.discover_inner(opts, KeyHashSource::ResidentWallet, enrichment_deadline) .await } @@ -209,7 +234,21 @@ impl IdentityWallet { opts: IdentityDiscoveryOptions, master: &ExtendedPrivKey, ) -> Result, PlatformWalletError> { - self.discover_inner(opts, KeyHashSource::Master(master)) + self.discover_inner(opts, KeyHashSource::Master(master), None) + .await + } + + /// [`Self::discover_from_master`], bounding the DPNS enrichment tail. + /// + /// Crate-private sibling of [`Self::discover_until`]; see there for why + /// the deadline is not an option field. + pub(crate) async fn discover_from_master_until( + &self, + opts: IdentityDiscoveryOptions, + master: &ExtendedPrivKey, + enrichment_deadline: Option, + ) -> Result, PlatformWalletError> { + self.discover_inner(opts, KeyHashSource::Master(master), enrichment_deadline) .await } @@ -300,6 +339,7 @@ impl IdentityWallet { &self, opts: IdentityDiscoveryOptions, source: KeyHashSource<'_>, + enrichment_deadline: Option, ) -> Result, PlatformWalletError> { use super::identity_handle::{derive_identity_auth_key_hash_from_master, MASTER_KEY_INDEX}; use crate::wallet::identity::state::managed_identity::key_storage::DpnsNameInfo; @@ -340,124 +380,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(start_index, 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 @@ -476,7 +565,26 @@ impl IdentityWallet { } // --- DPNS lookup for all discovered identities --- - for identity in &discovered { + for (enriched, identity) in discovered.iter().enumerate() { + // Stopped between identities, the same discipline the contact + // drains use. This loop is the only unbudgeted network walk inside + // a call the startup sequence bounds from the outside, and it runs + // AFTER `publish_scan_verdict` above — so an outer timeout that + // fires in here cancels a scan whose verdict already landed, and + // the caller, seeing only a cancelled future, records it as an + // abandoned scan. Enrichment is best-effort: a username this pass + // does not fetch is picked up by the next one, while a scan + // verdict wrongly overwritten with "abandoned" costs a from-zero + // rescan on every launch until a clean scan gets all the way + // through. + if budget_spent(enrichment_deadline) { + tracing::info!( + enriched, + total = discovered.len(), + "identity discovery: budget spent; leaving DPNS enrichment for a later pass" + ); + break; + } let identity_id = identity.id(); match self .sdk @@ -526,6 +634,53 @@ 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, + ) { + // What lands in memory is the verdict folded over the one already on + // record, so that is what gets persisted too — persisting this scan's + // own verdict instead would drop the gaps the fold carried forward. + let recorded = { + 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), + 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(recorded), + ..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 +703,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 +720,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 +751,66 @@ 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, over the index range + /// `probed_from..probed_through` it walked. + /// + /// The range is carried into the verdict because a later scan may only + /// clear a recorded gap it actually covered — a suffix scan that resumes + /// past an unanswered index has said nothing about it. + /// + /// 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_from: u32, + probed_through: u32, + ) -> crate::changeset::IdentityScanStateEntry { + let unanswered = self.unanswered_indices(); + if unanswered.is_empty() { + crate::changeset::IdentityScanStateEntry::completed(probed_from, probed_through) + } else { + crate::changeset::IdentityScanStateEntry::incomplete( + probed_from, + 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 +1111,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 +1139,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(0, 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(0, 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(0, 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 +1283,193 @@ 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(0, 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(0, 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(0, 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(0, 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" + ); + } + + /// The enrichment deadline bounds the DPNS tail and nothing else. + /// + /// This is the guard for the split itself. The defect it exists to remove + /// is a scan whose identity probes all answered being reported as + /// abandoned because its enrichment ran past the caller's budget — so the + /// one way this fix could go wrong is the deadline reaching the scan. A + /// scan run with an already-spent enrichment deadline must still walk its + /// window and still leave its own verdict on record; if it ever stops + /// short, the caller sees a cancelled call and overwrites that verdict + /// with an abandoned-scan one, which is where the sticky `unlocated_gap` + /// and the per-launch from-zero rescan come from. + #[tokio::test] + async fn a_spent_enrichment_deadline_does_not_cut_the_scan() { + 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"); + + // Already in the past, so every enrichment iteration would stop on its + // first check. The scan must be untouched by it. + let spent = std::time::Instant::now() - std::time::Duration::from_secs(1); + + // The result itself is the mock's business — what matters is what the + // scan recorded on its way through. + let _ = wallet + .identity() + .discover_until( + IdentityDiscoveryOptions { + start_index: Some(0), + gap_limit: 2, + }, + Some(spent), + ) + .await; + + 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) + .expect("the scan published a verdict despite the spent enrichment deadline") + .clone(); + assert_eq!( + verdict.probed_from, 0, + "the scan started where it was told to, not where the deadline was" + ); + assert!( + verdict.probed_through > 0, + "the scan walked its window; only the enrichment tail is bounded" + ); + } } 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 80b6de019d4..fce9972f855 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/mod.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/mod.rs @@ -68,8 +68,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/payments.rs b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs index c084ea667a1..811addce4c3 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/network/payments.rs @@ -1051,10 +1051,15 @@ impl DashPayView<'_, B> { /// * `provider` - [`ContactCryptoProvider`] used to drain any /// deferred contact-crypto build for this contact before the send. The /// original sender's external-account build is enqueued by the signerless - /// sweep and completed only by a later `drain_pending_contact_crypto`; + /// sweep and completed only by a later contact-crypto drain; /// draining here (with a signer present) builds the account on demand so /// the very first send after establishing a contact succeeds instead of - /// failing the external-account lookup below. + /// failing the external-account lookup below. The drain runs behind the + /// seed-binding gate + /// ([`DashPayView::drain_pending_contact_crypto_verified`]), so a + /// provider that does not resolve this wallet's seed fails the payment + /// with [`PlatformWalletError::SeedMismatch`] before anything is + /// registered. /// /// # Returns /// @@ -1096,7 +1101,21 @@ impl DashPayView<'_, B> { // lookup below. Idempotent and a cheap no-op when the queue is empty. // Run BEFORE acquiring the wallet-manager write guard — the drain // re-acquires that (non-reentrant) lock internally. - self.drain_pending_contact_crypto(provider).await; + // + // Through the SEED-VERIFIED primitive, not the raw drain. This runs + // the same `RegisterReceiving` / `RegisterExternal` ops the unlock and + // FFI drains run, and it runs them before any funding input is signed + // — so a `provider` resolving the wrong seed (a mis-mapped + // Keychain/Keystore slot) would register a contact account derived + // from the wrong seed and only THEN fail the send on bad signatures. + // `register_contact_account` keys its existence check on `(index, us, + // them)` rather than on the xpub, so the wrong account is never + // revisited and the wallet permanently watches addresses nobody pays + // to. A payment through a wrong-seed provider cannot succeed anyway, + // so the refusal fails the send with the typed `SeedMismatch` instead + // of being allowed to write first and fail second. + self.drain_pending_contact_crypto_verified(provider, None) + .await?; let (payment_address, used_flip_changeset, tx, fee, funding_accounts) = { let mut wm = self.wallet_manager.write().await; @@ -4964,7 +4983,10 @@ mod tests { } let provider = SeedCryptoProvider::from_seed(seed, Network::Testnet); - let drained = iw.dashpay().drain_pending_contact_crypto(&provider).await; + let drained = iw + .dashpay() + .drain_pending_contact_crypto_until(&provider, None) + .await; assert_eq!(drained, 1, "the RegisterReceiving entry must be drained"); let wm = iw.wallet_manager.read().await; @@ -5101,7 +5123,10 @@ mod tests { } let provider = SeedCryptoProvider::from_seed(seed, Network::Testnet); - let drained = iw.dashpay().drain_pending_contact_crypto(&provider).await; + let drained = iw + .dashpay() + .drain_pending_contact_crypto_until(&provider, None) + .await; assert_eq!( drained, 1, "the drain must snapshot the out-of-wallet bucket and process its entry" @@ -5245,7 +5270,10 @@ mod tests { let provider = SeedCryptoProvider::from_seed(seed, Network::Testnet); for pass in 1..=2 { - let drained = iw.dashpay().drain_pending_contact_crypto(&provider).await; + let drained = iw + .dashpay() + .drain_pending_contact_crypto_until(&provider, None) + .await; assert_eq!( drained, 0, "pass {pass}: a purpose-rejected entry must stay queued, not be cleared" @@ -5346,7 +5374,10 @@ mod tests { .to_seed(""), Network::Testnet, ); - let drained = iw.dashpay().drain_pending_contact_crypto(&provider).await; + let drained = iw + .dashpay() + .drain_pending_contact_crypto_until(&provider, None) + .await; assert_eq!( drained, 1, @@ -5608,7 +5639,10 @@ mod tests { } let provider = SeedCryptoProvider::from_seed(seed, Network::Testnet); - let drained = iw.dashpay().drain_pending_contact_crypto(&provider).await; + let drained = iw + .dashpay() + .drain_pending_contact_crypto_until(&provider, None) + .await; assert_eq!( drained, 0, "a legacy-cohort decrypt failure must leave the entry queued, not clear it" @@ -5759,7 +5793,7 @@ mod tests { let drained = iw .dashpay() - .drain_pending_contact_crypto(&UnusedProvider) + .drain_pending_contact_crypto_until(&UnusedProvider, None) .await; assert_eq!( drained, 0, @@ -5851,7 +5885,7 @@ mod tests { // ----------------------------------------------------------------------- // // The original sender's external-account build is enqueued by the - // signerless sweep and completed by `drain_pending_contact_crypto`. The + // signerless sweep and completed by the contact-crypto drain. The // drain runs at the start of `send_payment` (with the signer-backed // provider) so the external account is built before the send resolves it — // otherwise the first `send_payment` after establishing a contact fails the 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 8ba700b191c..6634264f848 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,10 +9,29 @@ //! 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 [`DashPayView::drain_pending_contact_crypto_verified`] and +//! [`DashPayView::drain_auto_accepts_verified`], the two primitives every +//! provider-deriving pass goes through so no caller can forget the check, and +//! [`PlatformWallet::drain_pending_contact_crypto_verified`], the whole-wallet +//! wrapper that runs both. +//! +//! Each primitive gates itself and returns a [`ProviderBinding`] — the +//! provider it checked, tied to the wallet it checked it against — so a caller +//! sequencing two of them carries the verified provider forward instead of +//! re-deriving the conclusion from a queue probe that may since have gone +//! stale. +use dpp::identity::signer::Signer; +use dpp::identity::IdentityPublicKey; + +use crate::broadcaster::TransactionBroadcaster; use crate::error::PlatformWalletError; use crate::wallet::identity::network::contact_requests::ContactCryptoProvider; -use crate::wallet::platform_wallet::PlatformWallet; +use crate::wallet::identity::network::dashpay_view::DashPayView; +use crate::wallet::identity::network::identity_handle::IdentityWallet; +use crate::wallet::platform_wallet::{PlatformWallet, WalletId}; /// How [`PlatformWallet::verify_seed_binds_with_marker`] established the /// seed binding. @@ -30,7 +49,100 @@ pub enum SeedBindingVerification { Verified, } -impl PlatformWallet { +/// A contact-crypto provider and what the seed-binding check said about it +/// *on this drain cycle*. +/// +/// This exists because "the queue was empty a moment ago" is not the same +/// statement as "this provider owns this wallet", and the gated drain returns +/// the first while its callers were reading it as the second. Every gated +/// primitive in this module returns one, and every pass that would derive key +/// material through the provider takes one — so the fact of verification +/// travels with the value instead of being re-inferred from a queue probe that +/// has already gone stale. +/// +/// It carries the provider itself rather than a verdict about one, because a +/// verdict alone says nothing about which provider the pass that consumes it +/// will use. Both composition helpers are public on the exported +/// [`DashPayView`], so a caller could verify with provider A and hand the +/// binding to a pass running provider B: B would skip the check, derive from +/// the wrong seed, and turn a valid auto-accept proof into a permanent +/// failure. The pass now derives through the borrowed provider inside the +/// binding, so the provider that was checked and the provider that is used are +/// the same value by construction. +/// +/// The wallet is carried for the same reason and checked at the point of use: +/// the two wallets' views have the same type, so nothing but the id can tell a +/// binding earned for one from a binding earned for the other. A binding that +/// does not match re-runs the check rather than being refused, which is the +/// same fail-safe shape as an unestablished one. +/// +/// It cannot be forged: the two constructors are private to this module, so +/// only a primitive that actually ran (or skipped) the check can mint one. +pub struct ProviderBinding<'a, C> { + /// The provider the check was run against — and the one the pass this + /// binding authorizes derives through. + provider: &'a C, + /// The wallet the check was run for. + wallet_id: WalletId, + /// What the check said. + state: BindingState, +} + +// Manual: a derive would bound `C: Debug`, and the provider is a host +// Keychain handle that has no business being formatted anyway. +impl std::fmt::Debug for ProviderBinding<'_, C> { + fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result { + f.debug_struct("ProviderBinding") + .field("wallet_id", &hex::encode(self.wallet_id)) + .field("state", &self.state) + .finish_non_exhaustive() + } +} + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum BindingState { + /// `verify_seed_binds` passed for this provider. + Verified, + /// The check did not run — the gated drain found an empty queue and had + /// nothing to derive. **Not** a licence to run further provider work: the + /// queue can be refilled at any instant by the recurring contact sweep. + NotEstablished, +} + +impl<'a, C> ProviderBinding<'a, C> { + /// The provider passed the seed-binding check for `wallet_id`. + fn verified(provider: &'a C, wallet_id: WalletId) -> Self { + Self { + provider, + wallet_id, + state: BindingState::Verified, + } + } + + /// The check did not run. + fn not_established(provider: &'a C, wallet_id: WalletId) -> Self { + Self { + provider, + wallet_id, + state: BindingState::NotEstablished, + } + } + + /// Whether the provider has been shown to resolve the seed of the wallet + /// this binding was minted on. + pub fn is_verified(&self) -> bool { + matches!(self.state, BindingState::Verified) + } + + /// Whether this binding is evidence about `wallet_id` specifically. A + /// binding earned on another wallet proves nothing here, however verified + /// it is over there. + fn proves(&self, wallet_id: &WalletId) -> bool { + self.is_verified() && self.wallet_id == *wallet_id + } +} + +impl IdentityWallet { /// Verify the signer behind `crypto` resolves the seed that owns this wallet. /// /// Reads the wallet's persisted BIP44 account-0 xpub and the path it was @@ -81,8 +193,10 @@ impl PlatformWallet { // account, so the two can never drift. Drop the lock before awaiting the // signer — the guard is not held across `.await`. let (path, expected) = { - let guard = self.state().await; - let wallet = guard.wallet(); + let guard = self.wallet_manager.read().await; + let wallet = guard + .get_wallet(&self.wallet_id) + .ok_or_else(|| PlatformWalletError::WalletNotFound(hex::encode(self.wallet_id)))?; let account = wallet.get_bip44_account(0).ok_or_else(|| { PlatformWalletError::InvalidIdentityData( "wallet has no BIP44 account 0 to verify the seed against".to_string(), @@ -110,16 +224,394 @@ impl PlatformWallet { Ok((SeedBindingVerification::Verified, candidate)) } else { Err(PlatformWalletError::SeedMismatch { - wallet_id: hex::encode(self.wallet_id()), + wallet_id: hex::encode(self.wallet_id), }) } } } +impl DashPayView<'_, B> { + /// 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 behind the gate — the crate-private + /// `drain_pending_contact_crypto_until` primitive — and returns the + /// completed count. `deadline` bounds the gate and the drain alike, so + /// neither can hold a caller past its budget; `None` is unbounded. The + /// bound is on async waiting only — a host resolver callback that blocks + /// synchronously inside one poll is not interruptible, see + /// `establish_provider_binding`. + /// + /// This is the **innermost** gated primitive — the one every drain reaches, + /// whatever handle the caller is holding. The startup sequence and the FFI + /// drain entry point arrive via + /// [`PlatformWallet::drain_pending_contact_crypto_verified`], which adds + /// the DIP-15 auto-accept pass behind the same gate; the payment path + /// ([`Self::send_payment`]) calls this one directly, because a + /// `DashPayView` is what it has in hand. + /// + /// # 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. Review found the same shape a + /// third time on the payment path, which drained unverified before any + /// funding-input signing could fail on the wrong seed. Hosting the gate on + /// `DashPayView` — the handle the drain itself lives on — is what removes + /// the last place it could be omitted from: there is no way to reach the + /// drain with a provider that has not been through it. + /// + /// # 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. The + /// auto-accept pass rides the same queue, so the outer wrapper's early-out + /// on an empty queue 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, and neither has one that cannot + /// answer inside `deadline` + /// ([`PlatformWalletError::SeedBindingUnanswered`]). 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. + /// + /// # Why the raw drain cannot be reached from outside this crate + /// + /// `DashPayView` is publicly re-exported, so "the gate is the only way in" + /// has to be a property of the type rather than a convention the callers + /// keep. The two unchecked primitives — + /// `drain_pending_contact_crypto_until` and `drain_auto_accepts_until` — + /// are therefore crate-private, and a downstream crate cannot name either + /// of them: + /// + /// ```compile_fail,E0624 + /// use platform_wallet::wallet::identity::network::DashPayView; + /// use platform_wallet::ContactCryptoProvider; + /// + /// async fn bypass( + /// view: &DashPayView<'_>, + /// provider: &P, + /// ) -> usize { + /// view.drain_pending_contact_crypto_until(provider, None).await + /// } + /// ``` + /// + /// ```compile_fail,E0624 + /// use dpp::identity::signer::Signer; + /// use dpp::identity::IdentityPublicKey; + /// use platform_wallet::wallet::identity::network::DashPayView; + /// use platform_wallet::ContactCryptoProvider; + /// + /// async fn bypass(view: &DashPayView<'_>, signer: &S, provider: &P) -> usize + /// where + /// S: Signer + Send + Sync, + /// P: ContactCryptoProvider + Sync, + /// { + /// view.drain_auto_accepts_until(signer, provider, None).await + /// } + /// ``` + /// + /// The refusals above are about visibility and not about the shape of the + /// call: the same downstream crate reaches both gated boundaries — this + /// one, and the whole-wallet wrapper that adds the auto-accept pass — + /// without trouble. + /// + /// ``` + /// use platform_wallet::wallet::identity::network::DashPayView; + /// use platform_wallet::ContactCryptoProvider; + /// use platform_wallet::PlatformWalletError; + /// + /// async fn gated( + /// view: &DashPayView<'_>, + /// provider: &P, + /// ) -> Result { + /// view.drain_pending_contact_crypto_verified(provider, None).await + /// } + /// ``` + /// + /// ``` + /// use dpp::identity::signer::Signer; + /// use dpp::identity::IdentityPublicKey; + /// use platform_wallet::{ContactCryptoProvider, PlatformWallet, PlatformWalletError}; + /// + /// async fn gated( + /// wallet: &PlatformWallet, + /// signer: &S, + /// provider: &P, + /// ) -> Result + /// where + /// S: Signer + Send + Sync, + /// P: ContactCryptoProvider + Sync, + /// { + /// wallet + /// .drain_pending_contact_crypto_verified(provider, Some(signer), None) + /// .await + /// } + /// ``` + pub async fn drain_pending_contact_crypto_verified( + &self, + crypto: &C, + deadline: Option, + ) -> Result + where + C: ContactCryptoProvider + Sync, + { + self.drain_pending_contact_crypto_verified_reporting(crypto, deadline) + .await + .map(|(drained, _)| drained) + } + + /// [`Self::drain_pending_contact_crypto_verified`], additionally reporting + /// whether the provider was actually put through the check. + /// + /// The empty-queue early-out returns without verifying — correctly, since + /// there is nothing to derive — but that makes the return value ambiguous + /// to a caller that wants to run *more* provider work afterwards. Review + /// found the whole-wallet wrapper doing exactly that: it probed the queue, + /// saw it nonempty, delegated here, and this call re-probed and found the + /// queue emptied by a concurrent drain, so it returned `Ok(0)` unverified — + /// after which the wrapper ran the DIP-15 auto-accept pass anyway. That + /// pass re-snapshots the queue at its own instant, and the recurring + /// contact sweep can enqueue an `AutoAccept` inside that window, so a + /// brand-new entry could be processed through a provider nobody had + /// checked. With a wrong seed the re-derived auto-accept key does not match + /// the proof, and the mapping treats a verify failure as *permanent*: the + /// valid proof is marked failed and dropped, so the sweep's enqueue gate + /// never offers it again. + /// + /// So the fact of verification is returned rather than inferred. See + /// [`Self::drain_auto_accepts_verified`], which takes it. + pub async fn drain_pending_contact_crypto_verified_reporting<'c, C>( + &self, + crypto: &'c C, + deadline: Option, + ) -> Result<(usize, ProviderBinding<'c, C>), PlatformWalletError> + where + C: ContactCryptoProvider + Sync, + { + if self.drainable_contact_crypto_count().await == 0 { + return Ok((0, ProviderBinding::not_established(crypto, self.wallet_id))); + } + + let binding = self.establish_provider_binding(crypto, deadline).await?; + + Ok(( + self.drain_pending_contact_crypto_until(crypto, deadline) + .await, + binding, + )) + } + + /// Run the DIP-15 auto-accept pass behind the same gate, given whatever + /// binding a previous pass on this cycle established. + /// + /// The provider comes from `binding` rather than alongside it: a verdict + /// about provider A cannot authorize a pass through provider B if there is + /// no way to name a second provider. The check itself is still the gate — + /// a binding that was never established, or was established for another + /// wallet, is checked here before a single entry is touched. So the + /// auto-accept pass cannot run through an unverified provider no matter + /// how its caller is sequenced — which is the property the wrapper was + /// silently relying on the drain to provide and, on the empty-queue path, + /// not getting. + /// + /// There is deliberately **no** empty-queue early-out here. The queue this + /// would probe is re-snapshotted inside `drain_auto_accepts_until` anyway, + /// so a probe here would only re-open the same window: seeing it empty + /// and skipping the check would + /// leave the pass unverified for an entry enqueued a moment later. The + /// check is cheap next to the risk, and it only runs at all once the + /// wrapper has already seen work queued. + /// + /// `deadline` bounds the check it may have to run as well as the pass + /// itself; `None` is unbounded — with the same limit the check carries, + /// that a synchronously-blocking host resolver callback cannot be + /// interrupted (see `establish_provider_binding`). + /// + /// # Errors + /// + /// Fails closed on every verification error, exactly as the drain does — + /// including a check the provider did not answer inside `deadline`. The + /// queue is untouched, so the next signer-present pass auto-accepts + /// everything this one declined to guess at. + pub async fn drain_auto_accepts_verified( + &self, + signer: &S, + deadline: Option, + binding: ProviderBinding<'_, C>, + ) -> Result + where + S: Signer + Send + Sync, + C: ContactCryptoProvider + Sync, + { + let crypto = binding.provider; + if !binding.proves(&self.wallet_id) { + self.establish_provider_binding(crypto, deadline).await?; + } + + Ok(self + .drain_auto_accepts_until(signer, crypto, deadline) + .await) + } + + /// The check itself, with the shared refusal log. Returns the binding so + /// callers propagate evidence rather than re-deriving the conclusion. + /// + /// Bounded by the same `deadline` the pass it gates takes. The provider is + /// the host's Keychain / Keystore, so the derivation is a round trip out of + /// this process and can stall for as long as that host does — and the + /// startup sequence's whole reason for handing a deadline down is that no + /// Platform-wallet step may hold Core SPV past it. Unlike the drains, this + /// one is safe to abandon mid-await rather than between units of work: it + /// derives a public key and compares it, committing nothing on the way, so + /// dropping the future strands no work and leaves the queue exactly as it + /// found it. A deadline already spent refuses without consulting the + /// provider at all. + /// + /// # What the deadline does not bound + /// + /// It bounds the *await*, not the host. The timer only fires while the + /// wrapped future yields, and the production provider resolves the + /// mnemonic by calling out through the host's resolver — Keychain on iOS, + /// Keystore on Android — synchronously inside a single poll. A host + /// callback that blocks there blocks the whole task, timer included, so + /// the caller can be held past the budget this advertises. Bounding that + /// too would need an interruptible resolver protocol, which the current + /// pass-unretained handle cannot offer safely: detaching it to a worker + /// would open a use-after-free window. Callers that must not be held at + /// all should keep the check off their critical thread rather than trust + /// the deadline alone. + async fn establish_provider_binding<'c, C>( + &self, + crypto: &'c C, + deadline: Option, + ) -> Result, PlatformWalletError> + where + C: ContactCryptoProvider + Sync, + { + let unanswered = || PlatformWalletError::SeedBindingUnanswered { + wallet_id: hex::encode(self.wallet_id), + }; + let checked = match deadline { + Some(deadline) => { + let remaining = deadline.saturating_duration_since(std::time::Instant::now()); + if remaining.is_zero() { + Err(unanswered()) + } else { + tokio::time::timeout(remaining, self.verify_seed_binds(crypto)) + .await + .unwrap_or_else(|_elapsed| Err(unanswered())) + } + } + None => self.verify_seed_binds(crypto).await, + }; + checked.inspect_err(|e| { + tracing::error!( + wallet_id = %hex::encode(self.wallet_id), + error = %e, + "the contact-crypto provider was not shown to bind to this wallet's seed; \ + skipping the drain rather than deriving contact addresses that could never \ + be corrected" + ); + })?; + Ok(ProviderBinding::verified(crypto, self.wallet_id)) + } +} + +impl PlatformWallet { + /// Verify the signer behind `crypto` resolves the seed that owns this + /// wallet. Wallet-level entry point for + /// [`IdentityWallet::verify_seed_binds`]. + pub async fn verify_seed_binds( + &self, + crypto: &C, + ) -> Result<(), PlatformWalletError> { + self.identity().verify_seed_binds(crypto).await + } + + /// Marker-aware variant — see + /// [`IdentityWallet::verify_seed_binds_with_marker`]. + pub async fn verify_seed_binds_with_marker( + &self, + crypto: &C, + marker: Option<&str>, + keychain_stamp: Option<&str>, + ) -> Result<(SeedBindingVerification, Option), PlatformWalletError> { + self.identity() + .verify_seed_binds_with_marker(crypto, marker, keychain_stamp) + .await + } + + /// The gated drain plus the DIP-15 auto-accept pass, for callers holding a + /// whole wallet: the startup sequence and the FFI drain entry point. + /// + /// Both passes ride the same queue, so one emptiness check decides whether + /// this wrapper does anything at all — but *not* whether the provider was + /// checked. Each pass is separately gated, and the auto-accept pass is + /// handed the binding the drain established rather than inheriting the + /// assumption that it ran: the drain re-probes the queue and returns + /// unverified when a concurrent drain emptied it first, and the auto-accept + /// pass re-snapshots the queue at its own instant, so the two observations + /// can disagree. [`DashPayView::drain_auto_accepts_verified`] runs the + /// check itself in that case, so there is no path to an auto-accept + /// through an unverified provider on any interleaving. + /// + /// Returns the combined completed count; `deadline` bounds both passes and + /// the seed-binding check in front of each, `None` is unbounded. Errors + /// exactly as the inner primitives do. The bound covers async waiting + /// only: a host resolver callback that blocks synchronously within a + /// single poll cannot be interrupted, so this can still be held past the + /// budget by the Keychain / Keystore round trip behind the provider. + 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); + } + + let (drained, binding) = dashpay + .drain_pending_contact_crypto_verified_reporting(crypto, deadline) + .await?; + let accepted = match identity_signer { + Some(signer) => { + dashpay + .drain_auto_accepts_verified(signer, deadline, binding) + .await? + } + None => 0, + }; + Ok(drained + accepted) + } +} + #[cfg(test)] mod tests { use super::SeedBindingVerification; + use std::sync::atomic::{AtomicUsize, Ordering}; use std::sync::Arc; + use std::time::{Duration, Instant}; use key_wallet::mnemonic::{Language, Mnemonic}; use key_wallet::wallet::initialization::WalletAccountCreationOptions; @@ -506,4 +998,1008 @@ 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, + ) { + wallet_with_queued_contact_crypto_from(TEST_MNEMONIC).await + } + + /// [`wallet_with_queued_contact_crypto`] over an arbitrary seed, so a test + /// can hold two wallets that are genuinely different wallets. + async fn wallet_with_queued_contact_crypto_from( + phrase: &str, + ) -> ( + 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(phrase), + 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); + } + + // ----------------------------------------------------------------------- + // The payment path's pre-drain. + // + // `send_payment` drains the deferred contact-crypto queue before it takes + // the write guard and before any funding input is signed. Review found it + // doing so UNVERIFIED — the third entry point to reach these ops with no + // gate, after the FFI drain and before it the iOS-only Swift check. A + // wrong-seed provider registered the contact account first and only then + // failed the send on bad signatures, so the corruption outlived the error. + // ----------------------------------------------------------------------- + + /// Funding signer for the send path. Must never be reached: the + /// seed-binding gate fires on the pre-drain, before the write guard, coin + /// selection or any signature. + struct UnreachableCoreSigner; + + #[async_trait::async_trait] + impl key_wallet::signer::Signer for UnreachableCoreSigner { + type Error = String; + + fn supported_methods(&self) -> &[key_wallet::signer::SignerMethod] { + &[key_wallet::signer::SignerMethod::Digest] + } + + async fn sign_ecdsa( + &self, + _path: &key_wallet::DerivationPath, + _sighash: [u8; 32], + ) -> Result< + ( + dashcore::secp256k1::ecdsa::Signature, + dashcore::secp256k1::PublicKey, + ), + Self::Error, + > { + unreachable!("the seed-binding gate must fire before any funding input is signed") + } + + async fn public_key( + &self, + _path: &key_wallet::DerivationPath, + ) -> Result { + unreachable!("the seed-binding gate must fire before any key derivation") + } + } + + /// The defect: a payment made through a wrong-seed provider used to drain + /// first and fail second. The drain runs `RegisterReceiving` against + /// whatever seed the provider resolves, and `register_contact_account` + /// keys its existence check on `(index, us, them)` rather than the xpub — + /// so the wrong account is written once, never revisited, and the wallet + /// permanently watches addresses nobody pays to. The failed payment is + /// recoverable; that account is not. + #[tokio::test] + async fn send_payment_refuses_a_wrong_seed_provider_before_the_drain() { + use dpp::prelude::Identifier; + + 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 + .identity() + .dashpay() + .send_payment( + &Identifier::from([1u8; 32]), + &Identifier::from([2u8; 32]), + 10_000, + None, + &UnreachableCoreSigner, + &foreign, + ) + .await + .expect_err("a payment through a provider that does not own the wallet must fail"); + + 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 on the payment + /// path too, so the pre-drain still does its job (building the contact + /// account the send needs). Without this the test above would also pass if + /// the gate simply refused every payment. + /// + /// The send then fails on the missing DashPay *external* account — this + /// fixture has no contact xpub to build one from — which is precisely the + /// point: a failure PAST the gate, and a different one. + #[tokio::test] + async fn send_payment_lets_the_owning_seed_through_to_the_drain() { + use dpp::prelude::Identifier; + + let (manager, wallet, wallet_id) = wallet_with_queued_contact_crypto().await; + + let owning = SeedCryptoProvider::from_seed(seed_for(TEST_MNEMONIC), Network::Testnet); + let err = wallet + .identity() + .dashpay() + .send_payment( + &Identifier::from([1u8; 32]), + &Identifier::from([2u8; 32]), + 10_000, + None, + &UnreachableCoreSigner, + &owning, + ) + .await + .expect_err("this fixture has no external account, so the send cannot complete"); + + assert!( + !matches!(err, PlatformWalletError::SeedMismatch { .. }), + "the owning seed must not be refused by the gate, got: {err:?}" + ); + assert_eq!( + receiving_account_count(&manager, &wallet_id).await, + 1, + "the verified pre-drain must still build the contact receiving account" + ); + assert_eq!( + drainable(&wallet).await, + 0, + "the queued op completed rather than being skipped" + ); + } + + // ----------------------------------------------------------------------- + // The auto-accept pass and the queue-probe race. + // + // The whole-wallet wrapper probes the queue, delegates to the gated drain, + // then runs the DIP-15 auto-accept pass. Review found that the drain + // re-probes the queue and returns `Ok(0)` WITHOUT verifying when a + // concurrent drain emptied it in between — after which the wrapper ran the + // auto-accept pass anyway. That pass re-snapshots the queue at its own + // instant, so an `AutoAccept` the recurring sweep enqueued inside the + // window was processed through a provider nobody had checked. + // + // The damage is not a failed pass: `drain_auto_accepts_until` maps a proof + // that does not verify against our re-derived key to a PERMANENT verdict + // and clears it, and marks it so the sweep's enqueue gate will not offer it + // again. A wrong seed re-derives the wrong key, so a perfectly valid proof + // is destroyed. + // ----------------------------------------------------------------------- + + /// Identity signer for the auto-accept pass. Must never be reached: the + /// gate fires before a single queue entry is touched. + #[derive(Debug)] + struct UnreachableIdentitySigner; + + #[async_trait::async_trait] + impl dpp::identity::signer::Signer for UnreachableIdentitySigner { + async fn sign( + &self, + _key: &dpp::identity::IdentityPublicKey, + _data: &[u8], + ) -> Result { + unreachable!("the seed-binding gate must fire before any auto-accept is signed") + } + + async fn sign_create_witness( + &self, + _key: &dpp::identity::IdentityPublicKey, + _data: &[u8], + ) -> Result { + unreachable!("the seed-binding gate must fire before any auto-accept is signed") + } + + fn can_sign_with(&self, _key: &dpp::identity::IdentityPublicKey) -> bool { + unreachable!("the seed-binding gate must fire before the signer is consulted") + } + } + + /// Queue a DIP-15 `AutoAccept` op — what the recurring contact sweep + /// enqueues when it ingests an inbound request carrying an auto-accept + /// proof. + async fn enqueue_auto_accept( + manager: &PlatformWalletManager, + wallet_id: &WalletId, + contact: u8, + ) { + use crate::changeset::{ + upsert_pending_contact_crypto, PendingContactCrypto, PendingContactCryptoOp, + }; + use dpp::prelude::Identifier; + + 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"); + upsert_pending_contact_crypto( + managed.dashpay_pending_contact_crypto_mut(), + PendingContactCrypto { + owner_identity_id: Identifier::from([1u8; 32]), + contact_id: Identifier::from([contact; 32]), + op: PendingContactCryptoOp::AutoAccept, + enqueued_at_ms: 0, + }, + ); + } + + /// Stand-in for a concurrent drain on another task completing its work: + /// the queue this wallet's next probe will read is empty. + async fn empty_the_queue(manager: &PlatformWalletManager, wallet_id: &WalletId) { + use dpp::prelude::Identifier; + + 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"); + managed.dashpay_pending_contact_crypto_mut().clear(); + } + + /// The gated drain must say whether it actually checked the provider. Its + /// empty-queue early-out is correct on its own terms — nothing to derive, + /// nothing to protect — but a caller reading `Ok(0)` as "the provider is + /// good" is reading something the drain never said. + #[tokio::test] + async fn the_gated_drain_reports_whether_it_verified_the_provider() { + let (manager, wallet, wallet_id) = wallet_with_queued_contact_crypto().await; + let owning = SeedCryptoProvider::from_seed(seed_for(TEST_MNEMONIC), Network::Testnet); + + // Work queued → the check runs and the binding is reported. + let (drained, binding) = wallet + .identity() + .dashpay() + .drain_pending_contact_crypto_verified_reporting(&owning, None) + .await + .expect("the owning seed binds"); + assert_eq!(drained, 1); + assert!( + binding.is_verified(), + "a drain that ran must report the provider verified" + ); + + // Queue now empty → the early-out returns without checking, and must + // report exactly that rather than an unqualified success. + let (drained, binding) = wallet + .identity() + .dashpay() + .drain_pending_contact_crypto_verified_reporting(&owning, None) + .await + .expect("an empty queue is not an error"); + assert_eq!(drained, 0); + assert!( + !binding.is_verified(), + "an empty-queue early-out never consulted the provider, so it must not \ + report a binding it did not establish" + ); + let _ = (&manager, &wallet_id); + } + + /// The auto-accept pass gates itself. Handed a binding that was never + /// established, it must run the check rather than trust the caller's + /// sequencing — so no interleaving of the wrapper's two passes can reach + /// an auto-accept through an unverified provider. + #[tokio::test] + async fn the_auto_accept_pass_refuses_an_unestablished_binding() { + let (manager, wallet, wallet_id) = wallet_with_queued_contact_crypto().await; + enqueue_auto_accept(&manager, &wallet_id, 3).await; + let queued_before = drainable(&wallet).await; + + let foreign = SeedCryptoProvider::from_seed(seed_for(FOREIGN_MNEMONIC), Network::Testnet); + let err = wallet + .identity() + .dashpay() + .drain_auto_accepts_verified( + &UnreachableIdentitySigner, + None, + super::ProviderBinding::not_established(&foreign, wallet_id), + ) + .await + .expect_err("an unverified provider must not reach the auto-accept pass"); + + assert!( + matches!(err, PlatformWalletError::SeedMismatch { .. }), + "the refusal must be the typed wrong-seed error, got: {err:?}" + ); + assert_eq!( + drainable(&wallet).await, + queued_before, + "not one queue entry may be touched — a proof cleared on a wrong-seed verify \ + is destroyed permanently, the sweep's enqueue gate never re-offers it" + ); + } + + /// **The race, driven end to end.** The exact interleaving review + /// described, in the order the whole-wallet wrapper composes its two + /// passes: + /// + /// 1. the wrapper probes the queue and sees work; + /// 2. a concurrent drain empties it before the gated drain looks; + /// 3. the gated drain therefore returns without verifying; + /// 4. the recurring sweep enqueues a fresh `AutoAccept` in that window; + /// 5. the auto-accept pass runs — and must refuse. + /// + /// Step 2 is a real second task rather than a hope about scheduling: it is + /// awaited to completion, so the interleaving is the one under test on + /// every run instead of the one that happened to be scheduled. + /// + /// Before the fix this reached `drain_auto_accepts_until` with the foreign + /// provider, because `Ok(0)` from step 3 was indistinguishable from a + /// verified drain. + #[tokio::test] + async fn a_queue_emptied_between_the_probes_does_not_let_an_auto_accept_through() { + let (manager, wallet, wallet_id) = wallet_with_queued_contact_crypto().await; + let foreign = SeedCryptoProvider::from_seed(seed_for(FOREIGN_MNEMONIC), Network::Testnet); + + // 1. The wrapper's probe: nonempty, so it does not early-out. + assert_eq!( + drainable(&wallet).await, + 1, + "precondition: the wrapper must see work queued" + ); + + // 2. A concurrent drain on another task empties the queue. + { + let manager = manager.clone(); + tokio::spawn(async move { + empty_the_queue(&manager, &wallet_id).await; + }) + .await + .expect("the concurrent drain completes"); + } + + // 3. The gated drain re-probes, finds nothing, and returns UNVERIFIED. + let (drained, binding) = wallet + .identity() + .dashpay() + .drain_pending_contact_crypto_verified_reporting(&foreign, None) + .await + .expect("an empty queue is not an error, even for a foreign provider"); + assert_eq!(drained, 0); + assert!( + !binding.is_verified(), + "the empty-queue path cannot have verified anything" + ); + + // 4. The recurring sweep enqueues a new AutoAccept inside the window. + enqueue_auto_accept(&manager, &wallet_id, 4).await; + assert_eq!(drainable(&wallet).await, 1); + + // 5. The auto-accept pass must refuse the unverified provider. + let err = wallet + .identity() + .dashpay() + .drain_auto_accepts_verified(&UnreachableIdentitySigner, None, binding) + .await + .expect_err( + "an auto-accept enqueued after the queue probe must not be processed \ + through a provider that was never checked", + ); + assert!( + matches!(err, PlatformWalletError::SeedMismatch { .. }), + "the refusal must be the typed wrong-seed error, got: {err:?}" + ); + assert_eq!( + drainable(&wallet).await, + 1, + "the freshly enqueued auto-accept must survive for the next correct-seed pass" + ); + } + + /// A binding is evidence about one provider and one wallet, and may not + /// authorize a pass that is neither. + /// + /// Both composition helpers are public on the exported `DashPayView`, so + /// the pairing is the caller's to get wrong: review found that a binding + /// earned by verifying provider A could be handed to an auto-accept pass + /// running provider B, which then skipped the check and derived from the + /// wrong seed — and `drain_auto_accepts_until` treats a proof that does + /// not verify against the re-derived key as PERMANENTLY failed, so a valid + /// auto-accept is destroyed and the sweep's enqueue gate never re-offers + /// it. + /// + /// The provider half of that pairing is now unrepresentable: the pass + /// takes the binding instead of a provider and derives through the one + /// inside it. The wallet half is what this test drives, because two views + /// have the same type and only the recorded id separates them — a binding + /// earned on wallet A must send wallet B's pass back through the check, + /// which A's provider then fails. + #[tokio::test] + async fn a_binding_earned_elsewhere_cannot_authorize_this_wallets_auto_accepts() { + let (_manager_a, wallet_a, _id_a) = + wallet_with_queued_contact_crypto_from(TEST_MNEMONIC).await; + let provider_a = SeedCryptoProvider::from_seed(seed_for(TEST_MNEMONIC), Network::Testnet); + + let (manager_b, wallet_b, id_b) = + wallet_with_queued_contact_crypto_from(FOREIGN_MNEMONIC).await; + enqueue_auto_accept(&manager_b, &id_b, 3).await; + let queued_before = drainable(&wallet_b).await; + + let (_, binding) = wallet_a + .identity() + .dashpay() + .drain_pending_contact_crypto_verified_reporting(&provider_a, None) + .await + .expect("wallet A's own seed binds"); + assert!(binding.is_verified()); + + let err = wallet_b + .identity() + .dashpay() + .drain_auto_accepts_verified(&UnreachableIdentitySigner, None, binding) + .await + .expect_err("a binding earned on another wallet proves nothing here"); + assert!( + matches!(err, PlatformWalletError::SeedMismatch { .. }), + "the refusal must be the typed wrong-seed error, got: {err:?}" + ); + assert_eq!( + drainable(&wallet_b).await, + queued_before, + "not one queue entry may be touched — a proof cleared on a wrong-seed verify \ + is destroyed permanently" + ); + } + + /// The other half: a verified binding is not re-derived. Without this the + /// gate could be satisfied by refusing every auto-accept pass. + #[tokio::test] + async fn a_verified_binding_carries_into_the_auto_accept_pass() { + let (manager, wallet, wallet_id) = wallet_with_queued_contact_crypto().await; + let owning = SeedCryptoProvider::from_seed(seed_for(TEST_MNEMONIC), Network::Testnet); + + let (_, binding) = wallet + .identity() + .dashpay() + .drain_pending_contact_crypto_verified_reporting(&owning, None) + .await + .expect("the owning seed binds"); + assert!(binding.is_verified()); + + // Nothing is queued for the auto-accept pass to do, so it completes at + // zero — the point is that it is reached at all. + let accepted = wallet + .identity() + .dashpay() + .drain_auto_accepts_verified(&UnreachableIdentitySigner, None, binding) + .await + .expect("a verified binding must carry into the auto-accept pass"); + assert_eq!(accepted, 0); + let _ = (&manager, &wallet_id); + } + + // ----------------------------------------------------------------------- + // The deadline in front of the check. + // + // Both gated passes take a deadline, and the startup sequence hands one + // down precisely so no Platform-wallet step can hold Core SPV past its + // budget. The check itself used to sit outside it: the provider is the + // host's Keychain / Keystore, and a host that never answers held the whole + // launch. An already-spent deadline was worse than useless — it still paid + // for a Keychain derivation before the drain it guards would have stopped + // at its first entry. + // + // Bounding it is safe in a way bounding the drains is not: the check + // derives a public key and compares it, committing nothing on the way, so + // an abandoned check strands no work. It fails closed, which lands on the + // path the sequence already has for a refused drain — the launch reports + // the binding unverified and starts Core SPV anyway. + // ----------------------------------------------------------------------- + + /// A provider that counts every consultation and then either answers from + /// the seed it was given or never answers at all — the stalled host + /// Keychain the deadline exists to survive. + /// + /// The count is the point: the expired-deadline tests assert the provider + /// was not reached, which no error value on its own can show. + struct CountingCryptoProvider { + consulted: Arc, + /// `None` never returns. + answers_from: Option, + } + + impl CountingCryptoProvider { + fn stalled() -> (Self, Arc) { + let consulted = Arc::new(AtomicUsize::new(0)); + ( + Self { + consulted: consulted.clone(), + answers_from: None, + }, + consulted, + ) + } + + /// Answers, but from a foreign seed — so a check that runs at all + /// fails with `SeedMismatch` rather than hanging. That is what makes + /// "the deadline was already spent" distinguishable from "the check + /// ran and refused". + fn foreign() -> (Self, Arc) { + let consulted = Arc::new(AtomicUsize::new(0)); + ( + Self { + consulted: consulted.clone(), + answers_from: Some(SeedCryptoProvider::from_seed( + seed_for(FOREIGN_MNEMONIC), + Network::Testnet, + )), + }, + consulted, + ) + } + } + + #[async_trait::async_trait] + impl crate::wallet::identity::network::contact_requests::ContactCryptoProvider + for CountingCryptoProvider + { + async fn receiving_xpub( + &self, + path: &key_wallet::bip32::DerivationPath, + ) -> Result { + self.consulted.fetch_add(1, Ordering::SeqCst); + match &self.answers_from { + Some(seed) => seed.receiving_xpub(path).await, + None => std::future::pending().await, + } + } + + async fn ecdh_shared_secret( + &self, + _path: &key_wallet::bip32::DerivationPath, + _peer: &dashcore::secp256k1::PublicKey, + ) -> Result, PlatformWalletError> { + unreachable!("the gate refuses, so no queue entry is ever derived") + } + + async fn export_auto_accept_private_key( + &self, + _path: &key_wallet::bip32::DerivationPath, + ) -> Result { + unreachable!("the gate refuses, so no queue entry is ever derived") + } + + async fn account_reference( + &self, + _path: &key_wallet::bip32::DerivationPath, + _compact_xpub: &[u8], + _account_index: u32, + _version: u32, + ) -> Result { + unreachable!("the gate refuses, so no queue entry is ever derived") + } + + async fn unmask_account_reference( + &self, + _path: &key_wallet::bip32::DerivationPath, + _compact_xpub: &[u8], + _account_reference: u32, + ) -> Result<(u32, u32), PlatformWalletError> { + unreachable!("the gate refuses, so no queue entry is ever derived") + } + + async fn contact_info_seal( + &self, + _root_path: &key_wallet::bip32::DerivationPath, + _derivation_index: u32, + _contact_id: &[u8; 32], + _private_data_plaintext: &[u8], + _private_data_iv: &[u8; 16], + ) -> Result + { + unreachable!("the gate refuses, so no queue entry is ever derived") + } + + async fn contact_info_open( + &self, + _root_path: &key_wallet::bip32::DerivationPath, + _derivation_index: u32, + _enc_to_user_id: &[u8; 32], + _private_data_blob: &[u8], + ) -> Result + { + unreachable!("the gate refuses, so no queue entry is ever derived") + } + } + + /// Long enough that a machine under load cannot mistake a bounded check + /// for an unbounded one, short enough to keep the tests quick. + const STALL_BUDGET: Duration = Duration::from_millis(150); + + /// If the fix is absent the call never returns, so every stall test caps + /// itself well above `STALL_BUDGET` and fails on the cap rather than + /// hanging the suite. + const STALL_TEST_CAP: Duration = Duration::from_secs(5); + + /// A deadline already spent must refuse before the provider is touched. + /// Resolving the mnemonic is the expensive half on a host — at worst a + /// biometric prompt — and paying for it to guard a drain that will stop at + /// its first entry anyway is exactly the cost the budget exists to cap. + #[tokio::test] + async fn an_expired_deadline_refuses_the_gated_drain_without_consulting_the_provider() { + let (manager, wallet, wallet_id) = wallet_with_queued_contact_crypto().await; + let (provider, consulted) = CountingCryptoProvider::foreign(); + + let err = wallet + .drain_pending_contact_crypto_verified( + &provider, + None::<&UnusedSigner>, + Some(Instant::now()), + ) + .await + .expect_err("a spent deadline must refuse rather than derive"); + + assert!( + matches!(err, PlatformWalletError::SeedBindingUnanswered { .. }), + "the refusal must say the check never got an answer, not claim a verdict \ + it never reached; got: {err:?}" + ); + assert_eq!( + consulted.load(Ordering::SeqCst), + 0, + "a spent deadline must not pay for a Keychain derivation" + ); + assert_eq!( + receiving_account_count(&manager, &wallet_id).await, + 0, + "nothing may be registered from a provider that was never checked" + ); + assert_eq!( + drainable(&wallet).await, + 1, + "the queue must survive for the next pass with budget to spend" + ); + } + + /// The same for the auto-accept pass, which gates itself independently — + /// so it has its own spent-deadline path to get right. + #[tokio::test] + async fn an_expired_deadline_refuses_the_auto_accept_pass_without_consulting_the_provider() { + let (manager, wallet, wallet_id) = wallet_with_queued_contact_crypto().await; + enqueue_auto_accept(&manager, &wallet_id, 3).await; + let queued_before = drainable(&wallet).await; + let (provider, consulted) = CountingCryptoProvider::foreign(); + + let err = wallet + .identity() + .dashpay() + .drain_auto_accepts_verified( + &UnreachableIdentitySigner, + Some(Instant::now()), + super::ProviderBinding::not_established(&provider, wallet_id), + ) + .await + .expect_err("a spent deadline must refuse rather than derive"); + + assert!( + matches!(err, PlatformWalletError::SeedBindingUnanswered { .. }), + "the refusal must say the check never got an answer; got: {err:?}" + ); + assert_eq!( + consulted.load(Ordering::SeqCst), + 0, + "a spent deadline must not pay for a Keychain derivation" + ); + assert_eq!( + drainable(&wallet).await, + queued_before, + "not one queue entry may be touched — a proof cleared on a failed verify \ + is destroyed permanently" + ); + } + + /// **The finding, driven end to end.** A host Keychain that never answers + /// must not hold the launch: the gated drain the startup sequence calls + /// has to come back inside the deadline it was handed, having derived + /// nothing. + /// + /// Before the fix the check awaited the provider unbounded, so this call + /// never returned and the cap below is what fails. + #[tokio::test] + async fn a_stalled_provider_cannot_hold_the_gated_drain_past_its_deadline() { + let (manager, wallet, wallet_id) = wallet_with_queued_contact_crypto().await; + let (provider, consulted) = CountingCryptoProvider::stalled(); + let deadline = Instant::now() + STALL_BUDGET; + + let refused = tokio::time::timeout( + STALL_TEST_CAP, + wallet.drain_pending_contact_crypto_verified( + &provider, + None::<&UnusedSigner>, + Some(deadline), + ), + ) + .await + .expect( + "a provider that never answers must not hold the gated drain past its \ + deadline — startup hands one down so Platform-wallet work cannot delay \ + Core SPV", + ); + + let err = refused.expect_err("an unanswered check must fail closed"); + assert!( + matches!(err, PlatformWalletError::SeedBindingUnanswered { .. }), + "the refusal must say the check never got an answer; got: {err:?}" + ); + assert_eq!( + consulted.load(Ordering::SeqCst), + 1, + "the check must have been attempted — a deadline that refuses without \ + trying would pass this test for the wrong reason" + ); + assert_eq!( + receiving_account_count(&manager, &wallet_id).await, + 0, + "nothing may be registered from a provider that never answered" + ); + assert_eq!( + drainable(&wallet).await, + 1, + "the abandoned check commits nothing, so the queue survives intact" + ); + } + + /// The auto-accept pass has the same exposure through the same check. + #[tokio::test] + async fn a_stalled_provider_cannot_hold_the_auto_accept_pass_past_its_deadline() { + let (manager, wallet, wallet_id) = wallet_with_queued_contact_crypto().await; + enqueue_auto_accept(&manager, &wallet_id, 4).await; + let queued_before = drainable(&wallet).await; + let (provider, consulted) = CountingCryptoProvider::stalled(); + let deadline = Instant::now() + STALL_BUDGET; + + let refused = tokio::time::timeout( + STALL_TEST_CAP, + wallet.identity().dashpay().drain_auto_accepts_verified( + &UnreachableIdentitySigner, + Some(deadline), + super::ProviderBinding::not_established(&provider, wallet_id), + ), + ) + .await + .expect("a provider that never answers must not hold the auto-accept pass either"); + + let err = refused.expect_err("an unanswered check must fail closed"); + assert!( + matches!(err, PlatformWalletError::SeedBindingUnanswered { .. }), + "the refusal must say the check never got an answer; got: {err:?}" + ); + assert_eq!(consulted.load(Ordering::SeqCst), 1); + assert_eq!( + drainable(&wallet).await, + queued_before, + "the abandoned check commits nothing, so the queue survives intact" + ); + } + + /// The other half: a deadline with room in it changes nothing. Without + /// this the four above would all pass against a gate that refused every + /// bounded pass outright. + #[tokio::test] + async fn a_deadline_with_room_left_still_lets_the_owning_seed_through() { + 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>, + Some(Instant::now() + Duration::from_secs(30)), + ) + .await + .expect("the wallet's own seed must bind inside a budget it fits in"); + + 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" + ); + } } diff --git a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/contact_requests.rs b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/contact_requests.rs index 73fa4d24d90..1a955ce78b2 100644 --- a/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/contact_requests.rs +++ b/packages/rs-platform-wallet/src/wallet/identity/state/managed_identity/contact_requests.rs @@ -146,8 +146,23 @@ impl ManagedIdentity { let mut cs = ContactChangeSet::default(); - // Check if there's already an incoming request from this recipient - if let Some(incoming_request) = self.dashpay.incoming_contact_requests.remove(&recipient_id) + // Fresh send. Persist BEFORE committing to memory, as both rotation + // branches above already do — this branch is what the recurring + // sweep's sent-side ingest runs, and its retry is gated on these very + // maps: a store failure that had already committed leaves the + // established / sent entry in memory, so the next sweep's re-fetch of + // the held-back range hits the same-reference no-op guard at the top + // of this method, returns `Ok(())`, reports a complete pass and + // advances the cursor. The write never reaches the backend and the + // relationship disappears at the next restart. + // + // `get`, not `remove`: the incoming request has to survive a failed + // store, or the retry can no longer reproduce the auto-establish. + if let Some(incoming_request) = self + .dashpay + .incoming_contact_requests + .get(&recipient_id) + .cloned() { // Automatically establish the contact — per the ContactChangeSet // auto-establishment contract, `established` implies the matching @@ -169,6 +184,8 @@ impl ManagedIdentity { }, contact.clone(), ); + persister.store(cs.into())?; + self.dashpay.incoming_contact_requests.remove(&recipient_id); self.dashpay .established_contacts .insert(recipient_id, contact); @@ -183,11 +200,11 @@ impl ManagedIdentity { request: request.clone(), }, ); + persister.store(cs.into())?; self.dashpay .sent_contact_requests .insert(recipient_id, request); } - persister.store(cs.into())?; Ok(()) } @@ -380,6 +397,21 @@ impl ManagedIdentity { /// If there's already a sent request to the sender, the contact is /// auto-established. Persists the resulting [`ContactChangeSet`] via /// `persister` and returns `()`. + /// + /// **Persist before committing to memory** — the same order as + /// [`Self::set_contact_metadata`] and the two rotation branches of + /// [`Self::add_sent_contact_request`], and for a sharper version of the + /// same reason. On an `Err` the in-memory maps are left exactly as the + /// persisted state, because the sync sweep's retry decides what to ingest + /// by reading *these maps*: `ingest_received_requests` computes the + /// sender's tracked `accountReference` from `incoming_contact_requests` / + /// `established_contacts` and `continue`s when it equals the fetched + /// request's. A committed-but-unpersisted request therefore makes the next + /// sweep skip the sender as already known, report a complete pass and + /// advance the cursor — so holding the cursor back buys nothing, the + /// backend never receives the write, and the contact is gone after a + /// restart. Holding the cursor only retries the write if memory still + /// looks like the write never happened. pub fn add_incoming_contact_request( &mut self, request: ContactRequest, @@ -389,8 +421,12 @@ impl ManagedIdentity { let sender_id = request.sender_id; let mut cs = ContactChangeSet::default(); - // Check if there's already a sent request to this sender - if let Some(outgoing_request) = self.dashpay.sent_contact_requests.remove(&sender_id) { + // Check if there's already a sent request to this sender. `get`, not + // `remove`: the outgoing request has to survive a failed store, or the + // retry can no longer reproduce the auto-establish and silently + // downgrades the pair to a bare incoming request. + if let Some(outgoing_request) = self.dashpay.sent_contact_requests.get(&sender_id).cloned() + { // Automatically establish the contact — per the ContactChangeSet // auto-establishment contract, `established` implies the matching // pending entries are dropped, so we don't also emit a @@ -413,6 +449,8 @@ impl ManagedIdentity { }, contact.clone(), ); + persister.store(cs.into())?; + self.dashpay.sent_contact_requests.remove(&sender_id); self.dashpay.established_contacts.insert(sender_id, contact); } else { // No matching sent request, just add as incoming @@ -425,11 +463,11 @@ impl ManagedIdentity { request: request.clone(), }, ); + persister.store(cs.into())?; self.dashpay .incoming_contact_requests .insert(sender_id, request); } - persister.store(cs.into())?; Ok(()) } @@ -546,8 +584,27 @@ impl ManagedIdentity { let mut cs = ContactChangeSet::default(); + // Which map tracks this sender, settled before either is touched. The + // `entry` API is deliberately not used for the commits below: a + // fallible `persister.store` sits between the lookup and the write, and + // an occupied entry would hold a mutable borrow across it — that borrow + // is precisely what must not exist until the store has succeeded. + let tracked_pending = self + .dashpay + .incoming_contact_requests + .contains_key(&sender_id); + + // Persist BEFORE committing to memory, same order and same reason as + // `add_incoming_contact_request`: on a failed store the rotation must + // be invisible in memory. Both of the retry's gates read these maps — + // the sweep's `tracked_reference == Some(new_reference)` skip and this + // method's own `already_applied` guard above — so a rotation committed + // to memory but not to disk locks itself out of both, reports a + // complete pass, advances the cursor, and loses the new key material + // at the next restart while the caller never tears down the stale + // external account. let rekeyed_established = - if let Some(contact) = self.dashpay.established_contacts.get_mut(&sender_id) { + if let Some(contact) = self.dashpay.established_contacts.get(&sender_id) { tracing::info!( owner = %owner_id, sender = %sender_id, @@ -555,42 +612,49 @@ impl ManagedIdentity { new_reference = request.account_reference, "Contact rotated their addresses — re-keying the established contact" ); - contact.incoming_request = request; - contact.payment_channel_broken = false; + let mut updated = contact.clone(); + updated.incoming_request = request; + updated.payment_channel_broken = false; // The label belongs to the incoming request being replaced — // drop it so the rebuilt external account re-derives it from // the new request rather than showing the old label against // fresh key material. - contact.contact_account_label = None; + updated.contact_account_label = None; // The stale external account (built from the old reference) // is torn down by the caller — reset the marker so the build // sweep re-registers from the new xpub and re-stamps it. - contact.external_account_reference = None; + updated.external_account_reference = None; cs.established.insert( SentContactRequestKey { owner_id, recipient_id: sender_id, }, - contact.clone(), + updated.clone(), ); + persister.store(cs.into())?; + self.dashpay.established_contacts.insert(sender_id, updated); true - } else if let Some(slot) = self.dashpay.incoming_contact_requests.get_mut(&sender_id) { - // Pending (not-yet-accepted) incoming request — replace it - // in place so a later Accept uses the freshest key material. - *slot = request.clone(); + } else if tracked_pending { + // Pending (not-yet-accepted) incoming request — replace it so + // a later Accept uses the freshest key material. cs.incoming_requests.insert( ReceivedContactRequestKey { owner_id, sender_id, }, - ContactRequestEntry { request }, + ContactRequestEntry { + request: request.clone(), + }, ); + persister.store(cs.into())?; + self.dashpay + .incoming_contact_requests + .insert(sender_id, request); false } else { return Ok(false); }; - persister.store(cs.into())?; Ok(rekeyed_established) } 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 ea80ea19ae0..32afd60a637 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,54 @@ 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`, folded over + /// whatever verdict is already on record, and return what was stored. + /// + /// Folding rather than replacing is what stops a scan from clearing a gap + /// it never probed — see + /// [`IdentityScanStateEntry::superseding`](crate::changeset::IdentityScanStateEntry::superseding). + /// It happens here, on the one path every writer takes, rather than in + /// each of them. + /// + /// In-memory only — the caller emits the matching changeset entry, because + /// only it holds the persister, and it emits the returned value so what is + /// persisted is what is in memory. + pub fn record_identity_scan( + &mut self, + wallet_id: WalletId, + state: crate::changeset::IdentityScanStateEntry, + ) -> crate::changeset::IdentityScanStateEntry { + let recorded = match self.identity_scan_states.get(&wallet_id) { + Some(previous) => state.superseding(previous), + None => state, + }; + self.identity_scan_states + .insert(wallet_id, recorded.clone()); + recorded + } } 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 4b13ae5c4c4..7b967f669ed 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,254 @@ 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(0, 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(0, 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 rescan that covers an earlier scan's unanswered indices clears + /// them — that is what lets a clean rescan 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(0, 5, vec![1])); + assert!(manager.identity_scan_is_incomplete(&wallet)); + + // Clearing takes coverage: this rescan walked index 1 and answered it. + manager.record_identity_scan(wallet, IdentityScanStateEntry::completed(0, 6)); + assert!(!manager.identity_scan_is_incomplete(&wallet)); + } + + /// The same gap, erased from the other side: a later scan may not clear + /// an index it never probed. + /// + /// Discovery's default options resume one past the highest registered + /// identity, so a wallet with identities at 0 and 2 and no answer at index + /// 1 resumes at 3. Publishing that suffix scan's clean verdict over the + /// recorded one dropped index 1, and the next launch took the warm + /// shortcut and reported a settled identity set with the identity at 1 — + /// and every contact it owns — still missing. That is the silent, + /// permanent gap the verdict exists to record, reached through the + /// bookkeeping meant to close it. + #[test] + fn a_clean_suffix_scan_cannot_erase_an_unresolved_index() { + use crate::changeset::IdentityScanStateEntry; + + let wallet: WalletId = [10u8; 32]; + let mut manager = IdentityManager::new(); + + // Identities at 0 and 2, index 1 never answered. + manager.record_identity_scan(wallet, IdentityScanStateEntry::incomplete(0, 3, vec![1])); + assert!(manager.identity_scan_is_incomplete(&wallet)); + + // A later default scan resumes at 3 and answers 3..9 cleanly. + let recorded = + manager.record_identity_scan(wallet, IdentityScanStateEntry::completed(3, 9)); + + assert!( + manager.identity_scan_is_incomplete(&wallet), + "a scan that never probed index 1 must not clear it" + ); + assert_eq!( + recorded.failed_indices, + vec![1], + "the gap must survive by name, so a later scan knows what to cover" + ); + assert_eq!( + manager + .identity_scan_state(&wallet) + .expect("verdict on record"), + &recorded, + "what is persisted is what is in memory" + ); + + // And a scan that DOES cover it hands the shortcut back — without + // this the assertion above would pass against a verdict stuck on + // incomplete forever. + manager.record_identity_scan(wallet, IdentityScanStateEntry::completed(0, 9)); + assert!(!manager.identity_scan_is_incomplete(&wallet)); + } + + /// A scan abandoned before it answered anything records no index, so + /// nothing carries its gap by name. Only a scan that starts at the bottom + /// of the index space can be said to have covered whatever it never + /// reached; a suffix scan must not hand the shortcut back over it. + #[test] + fn an_abandoned_scan_is_not_cleared_by_a_suffix_scan() { + use crate::changeset::IdentityScanStateEntry; + + let wallet: WalletId = [10u8; 32]; + let mut manager = IdentityManager::new(); + + // What the startup budget records when discovery is dropped mid-await. + manager.record_identity_scan(wallet, IdentityScanStateEntry::incomplete(0, 0, Vec::new())); + assert!(manager.identity_scan_is_incomplete(&wallet)); + + manager.record_identity_scan(wallet, IdentityScanStateEntry::completed(3, 9)); + assert!( + manager.identity_scan_is_incomplete(&wallet), + "a scan that started at 3 says nothing about the indices below it" + ); + + manager.record_identity_scan(wallet, IdentityScanStateEntry::completed(0, 9)); + assert!(!manager.identity_scan_is_incomplete(&wallet)); + } + + /// An unlocated gap has no name, so nothing in `failed_indices` can carry + /// it — and a fold that reads the fact back off that list loses it the + /// moment an intermediate scan puts a name in there. + /// + /// The abandoned scan records no index at all. An incomplete suffix scan + /// then folds in an unanswered index of its own, and what comes out is + /// indistinguishable from an ordinary located gap: the next suffix scan + /// covers that named index, finds a previous verdict whose failed list is + /// non-empty, and hands the shortcut back — while no scan beginning at + /// index 0 ever superseded the abandoned one. The launch after that skips + /// discovery over an identity nobody has looked for, which is the gap the + /// verdict exists to keep open. + #[test] + fn an_abandoned_gap_survives_an_incomplete_suffix_scan() { + use crate::changeset::IdentityScanStateEntry; + + let wallet: WalletId = [10u8; 32]; + let mut manager = IdentityManager::new(); + + // What the startup budget records when discovery is dropped + // mid-await: no coverage, no named index, the gap could be anywhere. + manager.record_identity_scan(wallet, IdentityScanStateEntry::incomplete(0, 0, Vec::new())); + assert!(manager.identity_scan_is_incomplete(&wallet)); + + // A suffix scan resumes at 3 and leaves index 4 unanswered. It says + // nothing about the indices below it, so the abandoned gap has to ride + // along — even though this fold now has a name of its own to carry. + manager.record_identity_scan(wallet, IdentityScanStateEntry::incomplete(3, 5, vec![4])); + assert!(manager.identity_scan_is_incomplete(&wallet)); + + // The next suffix scan covers index 4 and answers it. The named gap is + // legitimately gone; the unlocated one below index 3 is not. + manager.record_identity_scan(wallet, IdentityScanStateEntry::completed(4, 6)); + assert!( + manager.identity_scan_is_incomplete(&wallet), + "nothing has started at index 0 since the abandoned scan, so this hands \ + the warm shortcut back over a region no scan ever covered" + ); + + // And a scan that DOES start at the bottom hands the shortcut back — + // without this the assertion above would pass against a verdict stuck + // on incomplete forever. + manager.record_identity_scan(wallet, IdentityScanStateEntry::completed(0, 9)); + assert!(!manager.identity_scan_is_incomplete(&wallet)); + } + + /// Starting at index 0 is not the same as having covered the region an + /// unlocated gap could be hiding in. A scan that begins at the bottom and + /// is itself cut short walked only as far as it got, and everything above + /// that is still the same unlooked-at space the abandoned scan left. + /// + /// Only the scan's own window can be credited: an incomplete from-zero + /// scan carries the gap on, and a later suffix scan that answers the names + /// it did leave behind must not read the list going empty as the unknown + /// region having been covered. + #[test] + fn an_abandoned_gap_survives_an_incomplete_scan_from_index_zero() { + use crate::changeset::IdentityScanStateEntry; + + let wallet: WalletId = [11u8; 32]; + let mut manager = IdentityManager::new(); + + // Discovery dropped mid-await: no coverage, no named index, the gap + // could be anywhere. + manager.record_identity_scan(wallet, IdentityScanStateEntry::incomplete(0, 0, Vec::new())); + assert!(manager.identity_scan_is_incomplete(&wallet)); + + // A rescan does start at the bottom, but is cut off at index 2 with + // index 1 unanswered. It covered 0..2 and nothing above, so the + // abandoned gap is exactly as unlocated as it was. + let recorded = + manager.record_identity_scan(wallet, IdentityScanStateEntry::incomplete(0, 2, vec![1])); + assert!( + recorded.unlocated_gap, + "a from-zero scan that never finished covered only the window it walked, \ + so the region above it is still the one nobody can point at" + ); + assert!(manager.identity_scan_is_incomplete(&wallet)); + + // A suffix scan answers index 1. That named gap is legitimately gone — + // the unlocated one is not, and the failed list going empty must not + // be read as it having been covered. + manager.record_identity_scan(wallet, IdentityScanStateEntry::completed(1, 3)); + assert!( + manager.identity_scan_is_incomplete(&wallet), + "no scan has both started at index 0 and finished clean, so this hands \ + the warm shortcut back over a region no scan ever covered" + ); + + // A scan that starts at the bottom AND finishes clean does supersede + // it — without this the assertion above would pass against a verdict + // stuck on incomplete forever. + manager.record_identity_scan(wallet, IdentityScanStateEntry::completed(0, 9)); + 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 9e65f891cb2..c8df1c185d5 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