Skip to content
Merged
Show file tree
Hide file tree
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions packages/rs-platform-wallet/src/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,27 @@ pub enum PlatformWalletError {
#[error("Identity index not set for identity {0} — register or discover the identity first")]
IdentityIndexNotSet(Identifier),

#[error(
"Identity discovery incomplete: {failed_probes} of {probed} index probe(s) from {start_index} \
could not reach Platform and no identity was found; last error: {last_error}"
)]
/// A gap-limit scan ended empty with at least one index left unanswered.
/// Distinct from an empty success: it means "we do not know", so the
/// caller must retry rather than record that the seed owns no identity.
/// Both outcomes used to arrive as `Ok(vec![])`, which is how a transient
/// DAPI failure right after restore-from-seed became a whole session
/// without an identity.
IdentityDiscoveryIncomplete {
/// First index the scan probed.
start_index: u32,
/// How many indices were probed before the gap limit stopped the scan.
probed: u32,
/// How many of those probes failed to reach Platform.
failed_probes: u32,
/// Rendered last probe failure, for the log line at the FFI boundary.
last_error: String,
Comment thread
romchornyi marked this conversation as resolved.
Outdated
},

#[error(
"DashPay receiving account already exists for identity {identity} with contact {contact} on network {network:?} (account index {account_index})"
)]
Expand Down
113 changes: 109 additions & 4 deletions packages/rs-platform-wallet/src/wallet/identity/network/discovery.rs
Original file line number Diff line number Diff line change
Expand Up @@ -339,6 +339,22 @@ impl IdentityWallet {
let mut consecutive_misses = 0u32;
let mut identity_index = start_index;
let mut discovered: Vec<Identity> = Vec::new();
// A probe that never reached Platform is not evidence that the index is
// empty, so failures are counted apart from genuine misses. They still
// advance `consecutive_misses` — the scan has to terminate when the
// network is down — but a scan that ends with nothing found and at
// least one failed probe reports itself incomplete instead of
// returning `Ok(vec![])`. That empty-Ok was indistinguishable at the
// FFI boundary from "this seed owns no identity", and dashwallet
// cached it as final for the rest of the session: a few seconds of
// trouble right after restore-from-seed hid an existing identity (and
// with it every DashPay contact) until the app was relaunched.
let mut failed_probes = 0u32;
let mut last_probe_error: Option<String> = None;
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
// Identities Platform answered with, including ones the manager
// already tracked. `discovered` holds only the NEW ones, so it is the
// wrong thing to judge reachability by — see `identities_seen += 1`.
let mut identities_seen = 0usize;

while consecutive_misses < gap_limit {
// Derive the MASTER auth pubkey hash for this identity index
Expand Down Expand Up @@ -441,6 +457,11 @@ impl IdentityWallet {
if is_new {
discovered.push(identity.clone());
}
// Counted for every sighting, not just new ones: a rescan
// that re-confirms an identity the manager already holds
// still proves Platform answered, and `discovered` stays
// empty in that case.
identities_seen += 1;
consecutive_misses = 0;
}
Ok(None) => {
Expand All @@ -452,16 +473,40 @@ impl IdentityWallet {
identity_index,
e
);
// Treat a transient fetch error as a miss so the
// scan eventually terminates; the user can rerun
// to pick up anything we skipped over.
// Advance the miss counter so the scan still terminates
// when Platform is unreachable, but remember that this
// index was never actually answered — the result below
// depends on telling the two apart.
last_probe_error = Some(e.to_string());
failed_probes += 1;
consecutive_misses += 1;
}
}

identity_index += 1;
}

if scan_result_is_trustworthy(identities_seen, failed_probes) {
// Found something despite a failed probe: the discovered
// identities are already persisted, so return them rather than
// discarding the work. The gap is still worth a line — an identity
// at the failed index would be missed until the next scan.
if failed_probes > 0 {
tracing::warn!(
"Identity discovery completed with {} unanswered probe(s); an identity at \
a failed index may be missing until the next scan",
failed_probes
);
}
} else {
return Err(PlatformWalletError::IdentityDiscoveryIncomplete {
start_index,
probed: identity_index.saturating_sub(start_index),
failed_probes,
last_error: last_probe_error.unwrap_or_else(|| "unknown probe failure".to_string()),
});
}

// --- DPNS lookup for all discovered identities ---
for identity in &discovered {
let identity_id = identity.id();
Expand Down Expand Up @@ -515,10 +560,35 @@ impl IdentityWallet {
}
}

/// Whether a finished gap-limit scan may be reported as its literal result.
///
/// A scan answers "which identities does this seed own", and there are three
/// endings, not two: it found some, it confirmed there are none, or it never
/// got an answer. The third used to be reported as the second — a failed probe
/// incremented the same miss counter as an empty index, so a scan that reached
/// no one at all returned "this seed owns no identity". Callers cannot retry
/// what they were told is a definitive answer, so a few seconds of network
/// trouble after restore-from-seed cost a whole session's DashPay state.
///
/// Emptiness is only trustworthy when every probe was actually answered.
/// A scan that saw an identity is trustworthy either way: it reached Platform
/// and the seed demonstrably owns one, and the caller re-scans later for
/// anything a failed index hid.
///
/// `identities_seen` counts every index Platform answered with an identity —
/// NOT the length of the returned `discovered` list, which holds only the ones
/// the manager did not already track. A rescan from index 0 (what the app's
/// "Find identities" command does) re-confirms known identities without adding
/// to `discovered`, so judging by that list would report a reachable scan as
/// incomplete the moment any later index failed.
fn scan_result_is_trustworthy(identities_seen: usize, failed_probes: u32) -> bool {
identities_seen > 0 || failed_probes == 0
}

#[cfg(test)]
mod tests {
use super::super::identity_handle::derive_ecdsa_identity_auth_keypair_from_master;
use super::breadcrumb_decisions;
use super::{breadcrumb_decisions, scan_result_is_trustworthy};
use dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0;
use dpp::identity::identity_public_key::v0::IdentityPublicKeyV0;
use dpp::identity::v0::IdentityV0;
Expand Down Expand Up @@ -772,4 +842,39 @@ mod tests {
);
}
}

/// The regression this whole distinction exists for: a scan that found
/// nothing because it could not reach Platform must not be reported as a
/// scan that found nothing because there is nothing.
#[test]
fn empty_scan_with_unanswered_probes_is_not_trustworthy() {
assert!(!scan_result_is_trustworthy(0, 1));
assert!(!scan_result_is_trustworthy(0, 5));
}

#[test]
fn empty_scan_with_every_probe_answered_is_trustworthy() {
assert!(scan_result_is_trustworthy(0, 0));
}

/// Discovered identities are already persisted, so a partial failure does
/// not discard them — it only means a later scan may find more.
#[test]
fn scan_that_found_something_is_trustworthy_despite_failures() {
assert!(scan_result_is_trustworthy(1, 0));
assert!(scan_result_is_trustworthy(1, 3));
}

/// A rescan from index 0 re-confirms identities the manager already holds,
/// which never reach the returned `discovered` list. Judging by that list
/// would call a scan that plainly reached Platform "incomplete" as soon as
/// a later index failed — turning the app's "Find identities" command into
/// an error for a wallet whose identity was found successfully.
#[test]
fn rescan_of_a_known_identity_is_trustworthy_despite_a_later_failure() {
let identities_seen = 1; // answered at index 0, but is_new == false
let discovered_len = 0; // …so nothing was added to `discovered`
assert!(scan_result_is_trustworthy(identities_seen, 4));
assert!(!scan_result_is_trustworthy(discovered_len, 4));
Comment thread
romchornyi marked this conversation as resolved.
Outdated
}
}
Loading