diff --git a/CHANGELOG.md b/CHANGELOG.md index d89df5e75..a22bf2ef6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -25,6 +25,19 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Added +- **Unload an identity from this device**: Identity Hub → Settings now has a + working "Unload this identity from this device" action. It removes the + identity from the app on this device and clears the local keys, profile + information, DashPay details, and queued scheduled DPNS votes that the app can + identify for it, while leaving your other identities untouched. The + confirmation names how many scheduled votes will be cancelled. If cleanup + cannot finish, the app reports that clearly so you can retry. The identity + itself is unaffected on the network. Wallet-derived private keys can be + restored from the wallet. To load the identity again, you need recovery + information for any keys stored only on this device. Unloaded identities now + stay unloaded after automatic wallet discovery, and delayed profile results + are ignored after an unload. + - **Automatic Platform node refresh during upgrades**: migrating a pre-1.0 installation now triggers a best-effort Mainnet or Testnet node refresh. Failed attempts retry on later launches until fresh addresses are saved and @@ -63,6 +76,84 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **Identity unload and reload follow-ups for #889 / PR #925**: node load forms + now reject regular identities, wallet discovery keeps deliberately unloaded + identities unloaded until the user explicitly loads them again even when + discovery overlaps an unload, successful loads are not reported as failed + when marker cleanup leaves recoverable residue, associated voter removal + outcomes are reported accurately, partial cleanup failures still reconcile + the app's active identity state, and late DashPay profile responses can no + longer restore stale profile data. Reloading a masternode or evonode + identity that was left in a recoverable state by an earlier cleanup failure + now finishes that cleanup and completes the load, instead of being rejected + as already loaded; "delete all local data" now reaches those same + recoverable identities too. That same recovery now runs consistently across + every way an identity can be reloaded — overwriting, merging keys into an + existing record, loading from a wallet, loading by DPNS name, and automatic + wallet discovery — so a leftover key from an earlier interrupted unload is + always cleared before a fresh load replaces it, not just on one path. + +- **Identity unload/removal confirmations, feedback, and discovery guarantees, made consistent**: + the three places you can unload or remove an identity (Identity Hub → + Settings, the Identities screen, and a masternode's detail view) now show + the same specific-verb, input-blocking confirmation everywhere, instead of + one of them using a generic "Yes"/"No". Removing a masternode or evonode + from the Masternodes tab now reports the same success/warning outcome as + removing it anywhere else, instead of showing no feedback at all when + cleanup left residue behind. A wallet-wide identity search can no longer + bring back an identity you unloaded under any circumstance — previously + only the automatic background scan respected that — and a user-started + search now reports how many identities it left unloaded and how many it + failed to save, instead of looking identical to finding nothing new. The + unload confirmation's own wording was corrected to match: it no longer + implies a manual search can undo the unload, and it now names an actual, + reachable way back (the identity's own ID or username) instead of one + hidden behind an advanced-options toggle. + +- **Contest-decision ETA tooltip no longer under-reports the wait**: the + pending-username hover tooltip could round a decision estimate down by a + whole hour or day (e.g. showing "about 2 hours" for a decision actually + about 3 hours away), depending on exactly when it happened to be read. It + now rounds to the nearest hour/day instead of always rounding down. + +- **"Clear Database" no longer reports a clean wipe it did not finish**: + the wipe now keeps every identity reserved until the last step is done, so an + identity being loaded in the background cannot be written back to disk after + the wipe has already passed it and still be reported as erased. A failure + while removing retired shielded files no longer aborts the wipe partway or + makes identities that were erased successfully look as though they failed. + This closes the race against a concurrent load of the same identity in the + common case; one narrow exception remains (an identity that is both marked + unloaded and still on the device briefly reopens its slot between two + internal steps), and a wipe that loses that narrow race still reports itself + incomplete rather than succeeding silently. Other identity operations in + flight during a wipe (refreshing, adding a key, sending funds, and similar) + are not guarded yet and are tracked as follow-up work. + +- **A removal interrupted by a cleanup failure stays findable**: if the app + cannot finish clearing an identity's local data, it now records the identity + as unloaded, so loading it again finds the leftovers and finishes the job. + Previously such an identity could disappear from every list while its private + keys remained on the device with nothing able to reach them. The message + shown when this happens names that recovery — load the identity again, then + unload or remove it — instead of suggesting a retry that was impossible. + +- **Removing an identity now says what it really does**: the "Remove" action on + the Identities list and "Remove masternode" on the masternode page show the + same confirmation as Identity Hub → Settings, rather than wording that + suggested the identity was merely untracked. That confirmation is now + accurate too: it names the private keys and app entry that are deleted, + points to the "Clear Database" action in Settings for synced data such as + contacts and payment history that this action does not remove, and discloses + that the app remembers the unload so automatic discovery does not bring the + identity back. Removing a masternode still states that its voting identity + goes with it, and a masternode or evonode confirmation now describes how that + node is really restored — loaded again with its ProTxHash, with its private + keys entered again by hand if it held any — instead of promising wallet + recovery it never had. An identity with a name you chose is now also identified by its + full identifier, so two identities sharing a name cannot be confused on an + action that deletes keys. + - **Wallet rename consistency**: renaming a wallet no longer overwrites other saved wallet details when metadata cannot be read. Overlapping renames and wallet removals also keep displayed aliases and deleted-wallet metadata @@ -74,6 +165,11 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). does not support shielded sending from when the current interface mode does not unlock it. +- **Loading a masternode or evonode identity that was already loaded as a + regular identity no longer gets stuck**: the load screen now recognizes a + matching identity that hasn't finished loading and completes it correctly + instead of endlessly reporting it as already present. + - **Identity Home actions simplified**: the action row previously had six buttons — several of which opened the same screen (`Send`/`Send to another identity`, `Receive`/`Add funds`). It's now one row of four: **Add funds**, diff --git a/CLAUDE.md b/CLAUDE.md index e421bf6b6..0d204e8f1 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -107,7 +107,7 @@ Code lives by responsibility, not convenience: - **`model/`** — stateless data types and pure validation (format/length/charset). The single source of truth for validation. No `AppContext`, `Sdk`, DB, or `BackendTask`. All fee estimation goes in `model/fee_estimation.rs` — never inlined elsewhere. - **`backend_task/`** — async business logic, one submodule per domain; the authoritative enforcement layer. `TaskError` and its typed variants live in `backend_task/error.rs`. -- **`database/`** — SQLite persistence, one module per domain. +- **`database/`** — **Frozen legacy `data.db`, read-only in production.** Production opens an existing `data.db` with `SQLITE_OPEN_READ_ONLY` (`Database::open_legacy_read_only`, `src/app.rs`); the schema ladder in `database/initialization.rs` runs only on a fresh install that has no `data.db` yet, never on an existing one. Never add a table, column, or write path here — it becomes permanently unwritable after the install's first boot. All current durable state is a `DetKv` key (see `docs/kv-keys.md`) or a `SecretStore` entry (`wallet_backend/secret_seam.rs`); `database/` exists solely as a v0.9.3→v1.0 migration-read source and recovery artifact. - **`context/`** — `AppContext` submodules (`*_db.rs`, lifecycle, settings, status). - **`wallet_backend/`** — the wallet orchestration seam: adapters, views, backend-side live caches, signers, the secret chokepoint, the event bridge. All wallet secret bytes (HD seed, imported single key, identity private key) enter/leave the vault through ONE chokepoint, `wallet_backend/secret_seam.rs` (raw `SecretBytes`, no DET-side serialization). Per-secret at-rest encryption is implemented via `put_secret_protected`/`get_secret_protected` (Argon2id + XChaCha20-Poly1305, per-secret object-password envelope, AAD bound to `wallet_id ‖ label`); unprotected secrets use `put_secret`/`get_secret` (raw, keyless vault). Identity keys (imported/loaded, including masternode voting/owner/payout) enter unprotected (Tier-1 keyless) at load/creation time — the load flow has no password field — but can be sealed to Tier-2 per-identity afterward via `IdentityTask::ProtectIdentityKeys` (Key Info screen → "Add password protection…"; gated by vault-key scheme, not identity type). The keyless-vault residual is only no-password secrets and keys the user has not opted to protect. Design + migration: `docs/ai-design/2026-06-19-secret-storage-seam/`. - **`ui//`** — screens (`ScreenLike`). UI may *call* `model/` validators for instant feedback but never implements its own validation. @@ -151,7 +151,7 @@ User-facing error messages (shown in `MessageBanner` via `Display`) must follow - **Wallet Backend (`wallet_backend/`)** — Wallet orchestration seam: adapters, views, backend-side live caches, signers, the secret chokepoint (`secret_seam.rs`), and the event bridge. A thin adapter over the upstream `platform-wallet` crate. - **Context (`context/`)** — `AppContext`: shared state — network config, SDK client, database, wallets, settings cache, connection health (`ConnectionStatus` / `SpvManager`), split into submodules (`identity_db.rs`, `wallet_lifecycle.rs`, `settings_db.rs`, etc.). Glue between layers. - **Model (`model/`)** — Pure data types and stateless validation (amounts, fees, settings, wallet/identity models). No side effects, no IO. All fee estimation lives in `model/fee_estimation.rs` — never inline fee math elsewhere. -- **Database (`database/`)** — SQLite persistence (rusqlite), one module per domain. Typed CRUD, no business decisions. +- **Database (`database/`)** — Frozen legacy `data.db`. Read-only in production (migration-source reads and recovery only); no new tables, columns, or writes. Current persistence is `DetKv` over `det-app.sqlite` / `platform-wallet.sqlite` (`docs/kv-keys.md`) and `SecretStore`. - **Platform Integration** — Chain sync, address derivation, asset-lock/identity handling, and the shielded coordinator come from the upstream **`platform-wallet`** crate (git dep, dashpay/platform); DET is a thin adapter over it via `wallet_backend/`. SPV health is surfaced through `SpvManager` → `ConnectionStatus`. (DET's bespoke `src/spv/` stack and the `core_zmq_listener` module were removed in the platform-wallet migration.) ### Layer Rules @@ -354,7 +354,9 @@ Consider whether a repeated or reused message belongs in a dedicated `TaskError` ## Database -Single SQLite connection wrapped in `Mutex`. Schema initialized in `database/initialization.rs`. Domain modules provide typed CRUD methods. Backend task errors use `TaskError` (`src/backend_task/error.rs`) — see App Task System section above. +`AppContext.db` is the **legacy** `data.db` — a frozen migration-read source and recovery artifact, not a general persistence layer. Production opens it with `SQLITE_OPEN_READ_ONLY` whenever the file already exists, and only initializes (runs `database/initialization.rs`'s schema ladder) on a fresh install that has none yet; consequently that ladder never executes against an existing production install, and any write attempted on one fails at the SQLite layer. **Never add a table, column, or write path to `database/`.** + +All current durable state lives in `DetKv` (wraps the upstream `platform_wallet_storage::KvStore`; two backing SQLite files, `det-app.sqlite` and `spv//platform-wallet.sqlite` — see `docs/kv-keys.md` for the full key registry) or `SecretStore` (`wallet_backend/secret_seam.rs`). New persistent state is a new `DetKv` key registered in `docs/kv-keys.md`, never a new SQL table. Backend task errors use `TaskError` (`src/backend_task/error.rs`) — see App Task System section above. ## Platform Targets diff --git a/docs/kv-keys.md b/docs/kv-keys.md index 7c36c167f..f7b1754b2 100644 --- a/docs/kv-keys.md +++ b/docs/kv-keys.md @@ -10,6 +10,11 @@ Three backing stores exist: | `platform-wallet.sqlite` | `/spv//platform-wallet.sqlite` | Per-network identities, tokens, contracts, DashPay overlays, platform addresses, selected wallet | | `SecretStore` | `/secrets/det-secrets.*` | Encrypted HD-wallet seed envelopes and imported single-key private bytes | +Deliberately absent from this table: the legacy `data.db` behind `src/database/`. It is a +frozen v0.9.3→v1.0 migration-read source, opened read-only in production whenever it already +exists — never a target for new state. New persistent state is always a new `DetKv` key +registered below, never a new SQL table. + In the per-domain tables below, a `Scope` of `None` denotes `DetScope::Global`. --- @@ -82,11 +87,16 @@ Source: `src/model/selected_wallet.rs`, `src/wallet_backend/mod.rs` The identity blob and top-up history are **identity-scoped** (`DetScope::Identity(&id)`) so the upstream soft-cascade reaps them when the identity row is deleted. `DetScope::Identity` has no cross-identity listing, so a Global `det:identity_index:v1` slot holds the complete id roster the load-all paths iterate. `det:identity_order:v1` is a separate user-ordering view (may lag the full set) and stays Global. +`det:forgotten_identity:` is Global for the opposite reason to the blob: the marker's whole purpose is to outlive the identity it names, so automatic discovery cannot resurrect a deliberate unload. An identity-scoped slot would be reaped by that same soft-cascade at exactly the moment the marker becomes load-bearing. Like the other Global identity keys it is per-network by virtue of the per-network store, not by anything in the key or the value. + +The markers are **one key per identity**, enumerated by prefix scan, rather than a single slot holding the whole set. `DetKv` takes the persister lock per call, so a shared collection would make every marker write an unguarded read-modify-write: two identities unloading concurrently would clobber each other, either dropping a marker (the unload silently forgotten) or resurrecting a cleared one. Independent keys collide no more than the per-row SQL table this replaced, and confine a damaged marker to the one identity it names. + | Key | Scope | Store | Value type | Notes | |-----|-------|-------|------------|-------| | `det:identity:v1` | `DetScope::Identity(&id)` | `platform-wallet.sqlite` | `StoredQualifiedIdentity` | Fields: `qi_bytes` (inner bincode, redacted in `Debug`), `status: u8`, `identity_type: String`, `wallet_hash: Option<[u8;32]>`, `wallet_index: Option` | | `det:identity_index:v1` | `None` | `platform-wallet.sqlite` | `Vec<[u8;32]>` | Complete enumeration index of stored identity ids | | `det:identity_order:v1` | `None` | `platform-wallet.sqlite` | `Vec<[u8;32]>` | User-chosen display ordering of identity ID raw bytes | +| `det:forgotten_identity:` | `None` | `platform-wallet.sqlite` | `()` | Presence-only flag: the user deliberately unloaded this identity and discovery must not restore it. One key per identity | | `det:top_ups:v1` | `DetScope::Identity(&id)` | `platform-wallet.sqlite` | `BTreeMap` | Top-up history: account index → credits | Source: `src/context/identity_db.rs` @@ -204,8 +214,8 @@ Source: `src/wallet_backend/single_key.rs` (`SINGLE_KEY_PRIV_LABEL_PREFIX`, `SIN | Store | Key count | |-------|-----------| | `det-app.sqlite` | 4 (settings, wallet-meta sidecar, single-key-meta sidecar, migration sentinel) | -| `platform-wallet.sqlite` | 21 (across 8 domains) | +| `platform-wallet.sqlite` | 22 (across 8 domains) | | `SecretStore` | 2 label patterns (seed envelopes, imported-key private bytes) | -| **Total** | **27** | +| **Total** | **28** | Prefixed/templated keys (e.g. `det:identity:`) are counted once per prefix, not per instance. `SecretStore` entries are counted as label-pattern families, not per-wallet instances. diff --git a/docs/user-stories.md b/docs/user-stories.md index 72be13402..72963fd97 100644 --- a/docs/user-stories.md +++ b/docs/user-stories.md @@ -652,6 +652,7 @@ As a user, I want my wallet's identities to be found and loaded automatically on - The search uses a rolling five-index lookahead, going deeper each time an identity is found, so identities at non-contiguous indices are discovered. - Already-loaded identities are refreshed (new keys, new DPNS names) while any alias the user assigned is preserved. - Locked, password-protected wallets are skipped without prompting; they are searched after the user unlocks them. +- No search of a whole wallet — automatic or user-started — brings back an identity the user unloaded. The automatic sweep leaves them alone silently; the user-started "Load Identity → From my wallet" search reports how many it left alone, so a smaller count is not mistaken for a failure. That search also reports identities it found but could not save on this device. ### IDN-016: Identities and their keys preserved across an app upgrade [Implemented] **Persona:** Alex, Priya @@ -665,6 +666,31 @@ As a user, I want the identities I loaded before an upgrade — and the keys the - When identities and scheduled votes are both unreadable on the same launch, one banner names both remedies, and acknowledging it retires both reports — neither report can bury the other. - An identity the user deletes after the upgrade stays deleted. The import runs once, so a later launch never restores a removed identity, its alias, or its keys. +### IDN-020: Unload one identity from this device [Implemented] +**Persona:** Alex, Priya + +As a user, I want to unload one identity from this device so that I can recover from an incorrect import or stop keeping its private keys locally without removing a shared wallet. + +- The Identity Hub, the Identities list, and the masternode detail view all ask + for the same confirmation before unloading. If the identity is + wallet-derived, it explains that the identity can be loaded again from the + wallet's recovery seed. Otherwise, it warns that keys stored only on this + device are permanently deleted and require separate recovery information. If + scheduled votes are queued, the confirmation states how many will be + cancelled. The confirmation also names the synced data that only a full + database clear removes, and discloses that the app records the unload so no + search brings the identity back. All three + confirmations use the same danger-styled, input-blocking dialog with buttons + that name the action instead of a generic Yes/No pair. +- The recorded unload also survives a user-started search of the whole wallet. + Only a load aimed at that one identity brings it back: its own wallet index, + its identity ID, or its username. +- Removal reports its outcome, whichever screen it was started from, and says + what to retry when local data or the node's voting identity could not be + cleaned up. +- Unloading removes only the selected identity's local keys, metadata, its own DashPay overlay records, queued scheduled votes, and device record, while leaving the Platform identity unchanged. DashPay data held by another loaded identity that lists this one as a contact is not touched. +- Other identities on the same wallet and the wallet's recovery seed remain available. + --- ## DPNS (DPN) diff --git a/src/app.rs b/src/app.rs index c79924cd6..9e479f227 100644 --- a/src/app.rs +++ b/src/app.rs @@ -23,7 +23,9 @@ use crate::ui::components::{BannerHandle, MessageBanner, OptionBannerExt, Progre use crate::ui::contracts_documents::contracts_documents_screen::DocumentQueryScreen; use crate::ui::dashpay::{DashPayScreen, DashPaySubscreen, ProfileSearchScreen}; use crate::ui::dpns::dpns_contested_names_screen::{DPNSScreen, DPNSSubscreen}; +use crate::ui::identities::add_existing_identity_screen::wallet_identity_search_message; use crate::ui::identities::identities_screen::IdentitiesScreen; +use crate::ui::identity::settings::identity_removal_message; use crate::ui::network_chooser_screen::{NetworkChooserScreen, chooser_network_label}; use crate::ui::theme::ThemeMode; use crate::ui::tokens::tokens_screen::{TokensScreen, TokensSubscreen}; @@ -2579,13 +2581,14 @@ impl App for AppState { self.visible_screen_mut() .display_backend_task_result(&context, unboxed_message); } - BackendTaskSuccessResult::IdentitiesLoaded { count } => { - let msg = if count == 1 { - "Successfully loaded 1 identity from your wallet.".to_string() - } else { - format!("Successfully loaded {count} identities from your wallet.") - }; - MessageBanner::set_global(ctx, &msg, MessageType::Success); + BackendTaskSuccessResult::IdentitiesLoaded { + count, + skipped_forgotten, + failed, + } => { + let (msg, message_type) = + wallet_identity_search_message(count, skipped_forgotten, failed); + MessageBanner::set_global(ctx, &msg, message_type); self.visible_screen_mut() .display_backend_task_result(&context, unboxed_message); } @@ -2668,6 +2671,34 @@ impl App for AppState { // without a manual Refresh. No banner — this fires every 15 s. active_context.apply_platform_address_push(updates); } + BackendTaskSuccessResult::RemovedIdentities { + primary_cleanup_failed, + associated_cleanup_failed, + associated_removal_failed, + .. + } => { + // Here rather than in a screen: the card or row + // disappearing looks the same whether cleanup + // succeeded or left keys on disk, so every screen + // that removes an identity — present and future — + // reports the outcome by construction. + let (message, message_type) = identity_removal_message( + primary_cleanup_failed, + associated_cleanup_failed, + associated_removal_failed, + ); + let banner = MessageBanner::set_global(ctx, message, message_type); + if primary_cleanup_failed + || associated_cleanup_failed + || associated_removal_failed + { + // The user has a retry to perform; a warning + // that fades out is a warning they may miss. + banner.disable_auto_dismiss(); + } + self.visible_screen_mut() + .display_backend_task_result(&context, unboxed_message); + } BackendTaskSuccessResult::TokenBalanceRefreshAlreadyInFlight => { MessageBanner::set_global( ctx, diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs index 812a80426..9efd15613 100644 --- a/src/backend_task/error.rs +++ b/src/backend_task/error.rs @@ -680,6 +680,40 @@ pub enum TaskError { source: std::sync::Arc, }, + /// Owner-attributable local state could not be fully removed while unloading + /// an identity. Cleanup continues after failures, but only the first failure + /// is preserved in the nested typed error for logs. + /// + /// The identity is already out of the local index when this is raised, so it + /// is gone from every screen: the only recovery is to load it again, which + /// finishes the deferred cleanup, and then unload or remove it a second + /// time. Both entry points reach this, so the text names neither button's + /// verb alone. + #[error( + "Some local data for identity {identity_id} could not be fully removed. Load this identity again, then unload or remove it to finish clearing it." + )] + IdentityUnloadCleanupFailed { + identity_id: Identifier, + #[source] + source: Box, + }, + + /// Another load currently owns this identity's exclusive claim. + #[error( + "The identity {identity_id} is being updated right now. Wait a moment and try unloading it again." + )] + IdentityBusyWithLoad { identity_id: Identifier }, + + /// A user's choice to keep an unloaded identity off this device could not + /// be read or saved in the per-network wallet k/v store. + #[error( + "This identity could not be kept unloaded. Check available disk space and try again." + )] + ForgottenIdentityStorage { + #[source] + source: crate::wallet_backend::KvAdapterError, + }, + /// An identity top-up history record could not be persisted to the /// per-network wallet k/v store. #[error("Could not save your top-up history. Check available disk space and try again.")] @@ -1492,6 +1526,22 @@ pub enum TaskError { )] MasternodeNotFound { identity_id: Identifier }, + /// A regular identity load fetched a registered masternode or evonode. + /// Carries the resolved identity id for structured matching. + #[error( + "This identifier belongs to a registered masternode or evonode, not a regular identity. \ + Load it from the Masternodes page instead, where you can enter its owner, voting, and \ + payout keys." + )] + IdentityIsMasternode { identity_id: Identifier }, + + /// A masternode or evonode load fetched a regular identity. + /// Carries the resolved identity id for structured matching. + #[error( + "This identifier belongs to a regular identity, not a masternode or evonode. Load it from the Identities page instead." + )] + IdentityIsNotMasternode { identity_id: Identifier }, + /// The identity could not be constructed from the given parameters. #[error("Could not create the identity. Please check your input and try again.")] IdentityCreationError { @@ -2403,6 +2453,11 @@ pub enum TaskError { } impl TaskError { + /// Whether identity storage was already removed before this error occurred. + pub(crate) fn identity_was_removed(&self) -> bool { + matches!(self, Self::IdentityUnloadCleanupFailed { .. }) + } + /// Reclassifies SDK reachability failures when every configured DAPI address is exhausted. pub(crate) fn contextualize_dapi_availability( self, diff --git a/src/backend_task/identity/discover_identities.rs b/src/backend_task/identity/discover_identities.rs index bbfb96dc2..f7f642ae8 100644 --- a/src/backend_task/identity/discover_identities.rs +++ b/src/backend_task/identity/discover_identities.rs @@ -2,6 +2,7 @@ use crate::app::TaskResult; use crate::backend_task::BackendTaskSuccessResult; use crate::backend_task::error::TaskError; use crate::context::AppContext; +use crate::context::identity_load_registry::IdentityLoadGuard; use crate::model::identity_discovery::{ DiscoverySummary, IDENTITY_GAP_LIMIT, IDENTITY_SCAN_HARD_CAP, should_continue_scan, }; @@ -15,6 +16,39 @@ use std::sync::{Arc, RwLock}; /// concluding no identity is registered there. const AUTH_KEY_LOOKUP_WINDOW: u32 = 12; +/// Whether discovery is automatic, follows unlock, or is user-requested. +/// +/// The mode decides only whether a cold secret-cache miss may prompt. No mode +/// restores an identity the user unloaded: a scan sweeps a whole wallet and +/// names no identity, so it can never carry consent for one. Restoring is the +/// job of a load aimed at a single identity — the By-Wallet search's specific +/// index, or a load by identity id. +#[derive(Clone, Copy)] +pub(crate) enum IdentityDiscoveryMode { + /// Automatic startup discovery; never prompts. + Background, + /// Post-unlock discovery; may use the unlocked seed. + WalletUnlock, + /// User-started wallet-wide search; may prompt for the seed. + ExplicitSearch, +} + +impl IdentityDiscoveryMode { + fn allow_prompt(self) -> bool { + !matches!(self, Self::Background) + } +} + +/// What one discovered identity contributed to the pass. +enum DiscoveredIdentityOutcome { + /// Newly inserted or refreshed in the local database. + Stored, + /// Left unloaded because the user had unloaded it. + SkippedForgotten, + /// Not stored for any other reason (e.g. the network changed mid-scan). + Skipped, +} + impl AppContext { /// Discover and load identities derived from a wallet by checking the /// network, with a rolling gap-limited lookahead. @@ -27,10 +61,9 @@ impl AppContext { /// prior-session high index is never missed even if the early indices are /// empty. /// - /// `allow_prompt` controls the secret path: with `true` (the interactive - /// search) a cold auth-key cache miss prompts for the passphrase; with - /// `false` (the background sweep) a locked, protected wallet is skipped - /// instead of prompting. + /// `mode` controls whether a cold secret-cache miss may prompt. Whatever the + /// mode, an identity the user unloaded is left unloaded and counted in + /// [`DiscoverySummary::skipped_forgotten`]. /// /// When `progress` is `Some`, a [`BackendTaskSuccessResult::Progress`] event /// is sent before each probed index. @@ -38,7 +71,7 @@ impl AppContext { self: &Arc, wallet: &Arc>, seed_from_index: u32, - allow_prompt: bool, + mode: IdentityDiscoveryMode, progress: Option<&SenderAsync>, ) -> Result { use dash_sdk::platform::Fetch; @@ -64,7 +97,7 @@ impl AppContext { tracing::info!( seed = %hex::encode(seed_hash), seed_window = ?seed_window, - allow_prompt, + allow_prompt = mode.allow_prompt(), "Starting gap-limited identity discovery for wallet" ); @@ -101,7 +134,12 @@ impl AppContext { for key_index in 0..AUTH_KEY_LOOKUP_WINDOW { let public_key = match self - .resolve_identity_auth_pubkey(wallet, allow_prompt, current_index, key_index) + .resolve_identity_auth_pubkey( + wallet, + mode.allow_prompt(), + current_index, + key_index, + ) .await { Ok(key) => key, @@ -168,17 +206,26 @@ impl AppContext { identity, wallet, scan_network, - allow_prompt, + mode, current_index, ) .await { - Ok(()) => summary.stored = summary.stored.saturating_add(1), - Err(e) => tracing::warn!( - identity_id = %identity_id, - error = %e, - "Failed to store discovered identity" - ), + Ok(DiscoveredIdentityOutcome::Stored) => { + summary.stored = summary.stored.saturating_add(1) + } + Ok(DiscoveredIdentityOutcome::SkippedForgotten) => { + summary.skipped_forgotten = summary.skipped_forgotten.saturating_add(1) + } + Ok(DiscoveredIdentityOutcome::Skipped) => {} + Err(e) => { + summary.failed = summary.failed.saturating_add(1); + tracing::warn!( + identity_id = %identity_id, + error = %e, + "Failed to store discovered identity" + ); + } } } @@ -189,6 +236,8 @@ impl AppContext { seed = %hex::encode(seed_hash), found = summary.found, stored = summary.stored, + skipped_forgotten = summary.skipped_forgotten, + failed = summary.failed, "Gap-limited identity discovery complete" ); @@ -205,8 +254,13 @@ impl AppContext { wallet: &Arc>, max_identity_index: u32, ) -> Result<(), TaskError> { - self.discover_identities_gap_limited(wallet, max_identity_index, true, None) - .await?; + self.discover_identities_gap_limited( + wallet, + max_identity_index, + IdentityDiscoveryMode::WalletUnlock, + None, + ) + .await?; Ok(()) } @@ -229,27 +283,77 @@ impl AppContext { identity: dash_sdk::platform::Identity, wallet: &Arc>, scan_network: dash_sdk::dpp::dashcore::Network, - allow_prompt: bool, + mode: IdentityDiscoveryMode, identity_index: u32, - ) -> Result<(), TaskError> { + ) -> Result { if self.network != scan_network { tracing::debug!("Network changed mid-scan; skipping store of discovered identity"); - return Ok(()); + return Ok(DiscoveredIdentityOutcome::Skipped); } let identity_id = identity.id(); let seed_hash = wallet.read()?.seed_hash(); - let mut qualified_identity = self + let qualified_identity = self .build_qualified_identity_from_wallet( sdk, identity, wallet, - allow_prompt, + mode.allow_prompt(), identity_index, ) .await?; + let Some(load_guard) = self.persist_discovered_identity( + qualified_identity.clone(), + seed_hash, + identity_index, + )? + else { + tracing::debug!( + identity_id = %identity_id, + "Skipped a discovered identity that the user unloaded" + ); + return Ok(DiscoveredIdentityOutcome::SkippedForgotten); + }; + + if let Ok(mut wallet_guard) = wallet.write() { + wallet_guard + .identities + .insert(identity_index, qualified_identity.identity.clone()); + } + load_guard.loaded(); + tracing::info!( + identity_id = %identity_id, + "Successfully loaded discovered identity" + ); + Ok(DiscoveredIdentityOutcome::Stored) + } + + /// Persist one discovery result, or `Ok(None)` when the user unloaded it. + /// + /// A persisted result returns its exclusive load claim so the caller can + /// keep it through the corresponding wallet-cache update. Discovery never + /// overrides an unload: it derives identities from a wallet, so it cannot + /// distinguish the one identity a user wants back from every other one it + /// re-derives along the way. + pub(crate) fn persist_discovered_identity( + &self, + mut qualified_identity: crate::model::qualified_identity::QualifiedIdentity, + seed_hash: crate::model::wallet::WalletSeedHash, + identity_index: u32, + ) -> Result, TaskError> { + let identity_id = qualified_identity.identity.id(); + let load_guard = self.begin_identity_load(identity_id, None)?; + if self.is_identity_forgotten(&identity_id)? { + load_guard.loaded(); + return Ok(None); + } + + // No stuck-unload repair here: an interrupted unload leaves the marker + // set, so this path has already returned above. Repair belongs to the + // targeted load paths, which name an identity, and to + // `clear_network_database`, which sweeps every marker on a full wipe. match self.get_identity_by_id(&identity_id)? { Some(existing) => { // Carry DET-only metadata onto the refreshed identity, then @@ -266,17 +370,7 @@ impl AppContext { )?; } } - - if let Ok(mut wallet_guard) = wallet.write() { - wallet_guard - .identities - .insert(identity_index, qualified_identity.identity.clone()); - } - tracing::info!( - identity_id = %identity_id, - "Successfully loaded discovered identity" - ); - Ok(()) + Ok(Some(load_guard)) } /// Build a QualifiedIdentity from a fetched Identity with wallet key derivation paths. @@ -437,3 +531,183 @@ impl AppContext { }) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::app::TaskResult; + use crate::context::identity_load_registry::IdentityLoadPhase; + use crate::context::test_support::test_app_context; + use crate::model::qualified_identity::{IdentityStatus, IdentityType, QualifiedIdentity}; + use crate::utils::egui_mpsc::SenderAsync; + use dash_sdk::dpp::dashcore::Network; + use dash_sdk::dpp::version::PlatformVersion; + use dash_sdk::platform::{Identifier, Identity}; + use std::collections::BTreeMap; + + fn wallet_derived_identity( + id: Identifier, + wallet: &Arc>, + identity_index: u32, + ) -> QualifiedIdentity { + let wallet_seed_hash = wallet.read().expect("read wallet").seed_hash(); + QualifiedIdentity { + identity: Identity::create_basic_identity(id, PlatformVersion::latest()) + .expect("create identity"), + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::User, + alias: None, + private_keys: Default::default(), + dpns_names: Vec::new(), + associated_wallets: BTreeMap::from([(wallet_seed_hash, Arc::clone(wallet))]), + secret_access: None, + wallet_index: Some(identity_index), + top_ups: BTreeMap::new(), + status: IdentityStatus::Active, + network: Network::Testnet, + } + } + + /// No discovery pass restores an identity the user unloaded — not the + /// startup sweep, and not a user-started wallet-wide search, which targets a + /// search depth rather than any particular identity. Only a load aimed at + /// one identity may retire its marker. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn no_discovery_pass_resurrects_a_forgotten_identity() { + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + let wallet = Arc::new(RwLock::new( + Wallet::new_from_seed([0x69; 64], Network::Testnet, None, None).expect("build wallet"), + )); + let wallet_seed_hash = wallet.read().expect("read wallet").seed_hash(); + ctx.wallets() + .write() + .expect("write wallets") + .insert(wallet_seed_hash, Arc::clone(&wallet)); + let identity_id = Identifier::from([0x6A; 32]); + let identity = wallet_derived_identity(identity_id, &wallet, 4); + ctx.insert_local_qualified_identity(&identity, &None) + .expect("insert identity without a stored wallet association"); + + ctx.unload_identity(identity_id).expect("unload identity"); + + let stored = ctx + .persist_discovered_identity(identity.clone(), wallet_seed_hash, 4) + .expect("simulate a discovery pass over the wallet"); + assert!( + stored.is_none(), + "no discovery pass may restore an identity the user unloaded" + ); + assert_eq!( + ctx.latest_identity_load_phase(&identity_id), + Some(IdentityLoadPhase::Loaded), + "intentionally skipping a forgotten identity is a successful load no-op" + ); + assert!( + ctx.is_identity_forgotten(&identity_id) + .expect("read forgotten marker"), + "a discovery pass must leave the forgotten marker in place" + ); + assert!( + ctx.get_identity_by_id(&identity_id) + .expect("read identity") + .is_none(), + "the unloaded identity must stay unloaded" + ); + + // A load aimed at this one identity retires its marker — the only way + // back. Discovery then treats the identity like any other again. + ctx.clear_forgotten_identity_after_explicit_load(&identity_id) + .expect("a targeted load retires the marker"); + let load_guard = ctx + .persist_discovered_identity(identity, wallet_seed_hash, 4) + .expect("simulate discovery after a targeted load") + .expect("discovery must work normally once the marker is retired"); + load_guard.loaded(); + assert!( + ctx.get_identity_by_id(&identity_id) + .expect("read rediscovered identity") + .is_some() + ); + + backend.shutdown().await; + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn unload_during_discovery_persist_does_not_resurrect_wallet_cache() { + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + let wallet = Arc::new(RwLock::new( + Wallet::new_from_seed([0x64; 64], Network::Testnet, None, None).expect("build wallet"), + )); + let wallet_seed_hash = wallet.read().expect("read wallet").seed_hash(); + ctx.wallets() + .write() + .expect("write wallets") + .insert(wallet_seed_hash, Arc::clone(&wallet)); + let identity_id = Identifier::from([0x65; 32]); + let identity = wallet_derived_identity(identity_id, &wallet, 5); + + let load_guard = ctx + .persist_discovered_identity(identity.clone(), wallet_seed_hash, 5) + .expect("persist discovery") + .expect("discovery must persist a new identity"); + + let unload_ctx = Arc::clone(&ctx); + let (attempted_tx, attempted_rx) = tokio::sync::oneshot::channel(); + let unload_task = tokio::spawn(async move { + let mut attempted_tx = Some(attempted_tx); + loop { + match unload_ctx.unload_identity(identity_id) { + Err(TaskError::IdentityBusyWithLoad { .. }) => { + if let Some(attempted_tx) = attempted_tx.take() { + let _ = attempted_tx.send(()); + } + tokio::task::yield_now().await; + } + result => return result, + } + } + }); + attempted_rx + .await + .expect("unload must overlap the held discovery claim"); + + wallet + .write() + .expect("write wallet") + .identities + .insert(5, identity.identity); + load_guard.loaded(); + unload_task + .await + .expect("join unload") + .expect("unload after discovery cache insertion"); + + assert!( + wallet + .read() + .expect("read wallet") + .identities + .values() + .all(|identity| identity.id() != identity_id), + "an unload that completes during discovery must not be overwritten by a late cache insert" + ); + + backend.shutdown().await; + } +} diff --git a/src/backend_task/identity/load_guard.rs b/src/backend_task/identity/load_guard.rs new file mode 100644 index 000000000..47ae5e61b --- /dev/null +++ b/src/backend_task/identity/load_guard.rs @@ -0,0 +1,136 @@ +use crate::backend_task::error::TaskError; +use crate::context::AppContext; +use crate::context::identity_load_registry::{IdentityLoadGuard, IdentityLoadToken}; +use crate::model::qualified_identity::IdentityType; +use dash_sdk::dpp::identity::accessors::IdentityGettersV0; +use dash_sdk::platform::{Identifier, Identity}; + +impl AppContext { + pub(super) fn begin_identity_load_and_validate_type( + &self, + identity_type: IdentityType, + identity: &Identity, + load_token: Option, + ) -> Result { + let load_guard = self.begin_identity_load(identity.id(), load_token)?; + super::load_identity::validate_loaded_identity_type(identity_type, identity)?; + Ok(load_guard) + } + + pub(super) fn finish_identity_load_after_persist( + &self, + identity_id: &Identifier, + load_guard: IdentityLoadGuard, + ) { + if let Err(error) = self.clear_forgotten_identity_after_explicit_load(identity_id) { + tracing::warn!( + ?error, + identity_id = %identity_id, + "Persisted identity but could not clear its forgotten marker" + ); + } + load_guard.loaded(); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::context::identity_load_registry::IdentityLoadPhase; + use crate::context::test_support::{open_persister_fault_connection, test_app_context}; + use dash_sdk::dpp::identity::Purpose; + use dash_sdk::dpp::identity::identity_public_key::accessors::v0::{ + IdentityPublicKeyGettersV0, IdentityPublicKeySettersV0, + }; + use dash_sdk::dpp::version::PlatformVersion; + use dash_sdk::platform::IdentityPublicKey; + use std::collections::BTreeMap; + + #[test] + fn rejected_identity_type_reports_failed_load() { + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let platform_version = PlatformVersion::latest(); + let mut owner_key = IdentityPublicKey::random_key(1, Some(1), platform_version); + owner_key.set_purpose(Purpose::OWNER); + let identity = Identity::new_with_id_and_keys( + Identifier::from([0x71; 32]), + BTreeMap::from([(owner_key.id(), owner_key)]), + platform_version, + ) + .expect("identity"); + let identity_id = identity.id(); + let token = ctx + .mark_identity_load_submitted(identity_id) + .expect("submit load"); + + let error = ctx + .begin_identity_load_and_validate_type(IdentityType::User, &identity, Some(token)) + .expect_err("a user load must reject an identity with an owner key"); + + assert!(matches!( + error, + TaskError::IdentityIsMasternode { + identity_id: rejected_id + } if rejected_id == identity_id + )); + assert_eq!( + ctx.identity_load_phase(&identity_id, token), + Some(IdentityLoadPhase::Failed), + "type validation must happen after the load becomes reportable" + ); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn cleanup_failure_after_persist_still_reports_loaded() { + use crate::app::TaskResult; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + let identity_id = Identifier::from([0x72; 32]); + ctx.record_forgotten_identity(&identity_id) + .expect("record forgotten marker"); + let fault_connection = open_persister_fault_connection(&backend); + fault_connection + .execute_batch( + "CREATE TRIGGER fail_forgotten_marker_cleanup + BEFORE DELETE ON meta_global + WHEN OLD.key LIKE 'det:forgotten_identity:%' + BEGIN + SELECT RAISE(FAIL, 'injected forgotten marker cleanup failure'); + END;", + ) + .expect("install cleanup failure trigger"); + let token = ctx + .mark_identity_load_submitted(identity_id) + .expect("submit load"); + let load_guard = ctx + .begin_identity_load(identity_id, Some(token)) + .expect("claim load"); + + ctx.finish_identity_load_after_persist(&identity_id, load_guard); + + assert_eq!( + ctx.identity_load_phase(&identity_id, token), + Some(IdentityLoadPhase::Loaded), + "marker cleanup is non-essential after durable persistence" + ); + assert!( + ctx.is_identity_forgotten(&identity_id) + .expect("read retained marker"), + "the injected cleanup fault must leave the marker in place" + ); + + fault_connection + .execute_batch("DROP TRIGGER fail_forgotten_marker_cleanup;") + .expect("remove cleanup failure trigger"); + backend.shutdown().await; + } +} diff --git a/src/backend_task/identity/load_identity.rs b/src/backend_task/identity/load_identity.rs index cd62304f1..a3f0f47c7 100644 --- a/src/backend_task/identity/load_identity.rs +++ b/src/backend_task/identity/load_identity.rs @@ -14,7 +14,7 @@ use crate::model::qualified_identity::encrypted_key_storage::{ }; use crate::model::qualified_identity::qualified_identity_public_key::QualifiedIdentityPublicKey; use crate::model::qualified_identity::{ - DPNSNameInfo, IdentityStatus, IdentityType, QualifiedIdentity, + DPNSNameInfo, IdentityStatus, IdentityType, QualifiedIdentity, identity_carries_owner_key, }; use crate::model::wallet::{Wallet, WalletSeedHash}; use crate::ui::identities::add_new_identity_screen::MAX_IDENTITY_INDEX; @@ -40,6 +40,15 @@ use std::sync::{Arc, RwLock}; type WalletKeyMap = BTreeMap<(PrivateKeyTarget, u32), (QualifiedIdentityPublicKey, PrivateKeyData)>; type WalletMatchResult = Option<(WalletSeedHash, u32, WalletKeyMap)>; +fn is_bare_placeholder(qualified_identity: &QualifiedIdentity) -> bool { + qualified_identity.private_keys.private_keys.is_empty() + && qualified_identity.alias.is_none() + && qualified_identity.associated_voter_identity.is_none() + && qualified_identity.associated_operator_identity.is_none() + && qualified_identity.associated_owner_key_id.is_none() + && qualified_identity.status == IdentityStatus::Active +} + /// Merge an already-stored identity's keys and associations into a freshly /// built one, preserving anything the new (partial) load did not resupply /// (§10.8, the "Add voting key" in-place update). Keys the new load provides @@ -69,6 +78,28 @@ fn merge_existing_keys_into(new: &mut QualifiedIdentity, existing: QualifiedIden } } +pub(super) fn validate_loaded_identity_type( + identity_type: IdentityType, + identity: &Identity, +) -> Result<(), TaskError> { + if identity_type == IdentityType::User && identity_carries_owner_key(identity) { + return Err(TaskError::IdentityIsMasternode { + identity_id: identity.id(), + }); + } + if matches!( + identity_type, + IdentityType::Masternode | IdentityType::Evonode + ) && !identity_carries_owner_key(identity) + { + return Err(TaskError::IdentityIsNotMasternode { + identity_id: identity.id(), + }); + } + + Ok(()) +} + impl AppContext { pub(super) async fn load_identity( &self, @@ -149,12 +180,16 @@ impl AppContext { // before any network fetch — so the existing node's alias/keys/protection // tier are never silently overwritten. Checked here, at the storage // layer, so every `RejectIfExists` caller is guarded uniformly. - let existing_stored = self.get_local_qualified_identity(&identity_id)?; - match load_mode { - IdentityLoadMode::RejectIfExists if existing_stored.is_some() => { - return Err(TaskError::DuplicateProTxHash { identity_id }); - } - _ => {} + let mut existing_stored = self.get_local_qualified_identity(&identity_id)?; + if self.prepare_stuck_unload_cleanup_for_reload(&identity_id.to_buffer())? { + existing_stored = None; + } + if load_mode == IdentityLoadMode::RejectIfExists + && existing_stored + .as_ref() + .is_some_and(|identity| !is_bare_placeholder(identity)) + { + return Err(TaskError::DuplicateProTxHash { identity_id }); } // An in-place merge into a password-protected (Tier-2) node must @@ -198,6 +233,8 @@ impl AppContext { Err(e) => return Err(TaskError::from(e)), }; + validate_loaded_identity_type(identity_type, &identity)?; + let mut encrypted_private_keys = BTreeMap::new(); let wallets = self.wallets.read().map_err(TaskError::from)?.clone(); @@ -486,7 +523,9 @@ impl AppContext { // newly-supplied keys into the already-stored identity's keys instead of // clobbering them — the new voting key is added while the existing // Owner/Payout keys (which the update leaves blank) survive. - if load_mode == IdentityLoadMode::MergeIntoExisting + if (load_mode == IdentityLoadMode::MergeIntoExisting + || (load_mode == IdentityLoadMode::RejectIfExists + && existing_stored.as_ref().is_some_and(is_bare_placeholder))) && let Some(existing) = existing_stored { merge_existing_keys_into(&mut qualified_identity, existing); @@ -526,11 +565,9 @@ impl AppContext { if let Some(password) = encryption_password { self.protect_identity_keys(qualified_identity.identity.id(), password, None)?; } - - // Past the last fallible step: the node is stored with its keys as - // requested. Anything that failed before this — including a key seal that - // left the insert behind — reported `Failed` when the guard dropped. - load_guard.loaded(); + // The identity is durably stored with its keys as requested. Clearing + // the discovery marker is best-effort and must not mask that success. + self.finish_identity_load_after_persist(&identity_id, load_guard); Ok(BackendTaskSuccessResult::LoadedIdentity(qualified_identity)) } @@ -790,6 +827,8 @@ mod tests { use dash_sdk::dpp::dashcore::Network; use dash_sdk::dpp::identity::Identity; use dash_sdk::dpp::identity::KeyID; + use dash_sdk::dpp::identity::Purpose; + use dash_sdk::dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeySettersV0; use dash_sdk::dpp::platform_value::string_encoding::Encoding; use dash_sdk::dpp::version::PlatformVersion; use dash_sdk::platform::IdentityPublicKey; @@ -798,6 +837,77 @@ mod tests { const M: PrivateKeyTarget = PrivateKeyTarget::PrivateKeyOnMainIdentity; const V: PrivateKeyTarget = PrivateKeyTarget::PrivateKeyOnVoterIdentity; + fn identity_with_key_purpose(purpose: Purpose) -> Identity { + let platform_version = PlatformVersion::latest(); + let mut key = IdentityPublicKey::random_key(1, Some(1), platform_version); + key.set_purpose(purpose); + Identity::new_with_id_and_keys( + Identifier::random(), + BTreeMap::from([(key.id(), key)]), + platform_version, + ) + .expect("identity") + } + + /// Exercises the post-fetch guard directly because this module has no fetch stub. + #[test] + fn user_load_rejects_identity_with_owner_key() { + let identity = identity_with_key_purpose(Purpose::OWNER); + let expected_id = identity.id(); + + let error = validate_loaded_identity_type(IdentityType::User, &identity) + .expect_err("a User load must reject a masternode-owned identity"); + + assert!(matches!( + error, + TaskError::IdentityIsMasternode { identity_id } if identity_id == expected_id + )); + } + + #[test] + fn user_load_accepts_identity_with_only_authentication_key() { + let identity = identity_with_key_purpose(Purpose::AUTHENTICATION); + + assert!(validate_loaded_identity_type(IdentityType::User, &identity).is_ok()); + } + + #[test] + fn masternode_load_rejects_regular_identity() { + let identity = identity_with_key_purpose(Purpose::AUTHENTICATION); + let expected_id = identity.id(); + + let error = validate_loaded_identity_type(IdentityType::Masternode, &identity) + .expect_err("a Masternode load must reject a regular identity"); + + assert!(matches!( + error, + TaskError::IdentityIsNotMasternode { identity_id } if identity_id == expected_id + )); + } + + #[test] + fn evonode_load_rejects_regular_identity() { + let identity = identity_with_key_purpose(Purpose::AUTHENTICATION); + let expected_id = identity.id(); + + let error = validate_loaded_identity_type(IdentityType::Evonode, &identity) + .expect_err("an Evonode load must reject a regular identity"); + + assert!(matches!( + error, + TaskError::IdentityIsNotMasternode { identity_id } if identity_id == expected_id + )); + } + + #[test] + fn node_load_accepts_identity_with_owner_key() { + let identity = identity_with_key_purpose(Purpose::OWNER); + + for identity_type in [IdentityType::Masternode, IdentityType::Evonode] { + assert!(validate_loaded_identity_type(identity_type, &identity).is_ok()); + } + } + #[tokio::test] async fn identity_network_timeout_is_typed_and_actionable() { let error = crate::backend_task::await_network_request_with_timeout( @@ -994,6 +1104,60 @@ mod tests { ); } + #[test] + fn failed_creation_status_is_not_a_bare_placeholder() { + let pv = PlatformVersion::latest(); + let identity = + Identity::create_basic_identity(Identifier::random(), pv).expect("basic identity"); + let failed_creation_marker = QualifiedIdentity { + identity, + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::User, + alias: None, + private_keys: KeyStorage::default(), + dpns_names: vec![], + associated_wallets: BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::FailedCreation, + network: Network::Testnet, + }; + assert!( + !is_bare_placeholder(&failed_creation_marker), + "a failed-creation marker must make RejectIfExists report a duplicate", + ); + } + + #[test] + fn reject_if_exists_does_not_take_over_failed_creation_status() { + let pv = PlatformVersion::latest(); + let identity = + Identity::create_basic_identity(Identifier::random(), pv).expect("basic identity"); + let existing_failed_creation = QualifiedIdentity { + identity: identity.clone(), + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::User, + alias: None, + private_keys: KeyStorage::default(), + dpns_names: vec![], + associated_wallets: BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::FailedCreation, + network: Network::Testnet, + }; + assert!( + !is_bare_placeholder(&existing_failed_creation), + "RejectIfExists must stop before the bare-placeholder merge can replace the status", + ); + } + /// §10.9 / TC-EDGE-07 — a fresh load (`RejectIfExists`) of a /// ProTxHash already stored is rejected with [`TaskError::DuplicateProTxHash`] /// BEFORE any network fetch, and the already-stored node is left untouched. @@ -1028,6 +1192,8 @@ mod tests { let identity_id = qi.identity.id(); ctx.insert_local_qualified_identity(&qi, &None) .expect("insert first masternode identity"); + ctx.record_forgotten_identity(&identity_id) + .expect("record stale forgotten marker"); let input = IdentityInputToLoad { identity_id_input: identity_id.to_string(Encoding::Hex), @@ -1071,6 +1237,71 @@ mod tests { ctx.wallet_backend().expect("backend").shutdown().await; } + /// Issue #889: a bare, keyless `User` record from the generic load screen + /// must not permanently block a `RejectIfExists` load of the same identifier + /// under the correct Masternode type. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn reject_if_exists_does_not_dead_end_on_bare_user_typed_pro_tx_hash() { + let temp_dir = tempfile::tempdir().expect("tempdir"); + let data_dir = temp_dir.path().to_path_buf(); + ensure_env_file(&data_dir); + let db = Arc::new(create_database_at_path(&data_dir.join("data.db")).expect("db")); + let app_kv = AppContext::open_app_kv(&data_dir).expect("app kv"); + let secret_store = AppContext::open_secret_store(&data_dir).expect("secret store"); + let ctx = AppContext::new( + data_dir, + Network::Testnet, + db, + Arc::new(TaskManager::new()), + Arc::new(ConnectionStatus::new()), + egui::Context::default(), + app_kv, + secret_store, + crate::model::user_role::UserRoleCell::default(), + ) + .expect("offline testnet AppContext::new"); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + + let (mut qi, _) = masternode_shaped_qi(); + qi.identity_type = IdentityType::User; + qi.private_keys = KeyStorage::default(); + qi.associated_voter_identity = None; + qi.associated_operator_identity = None; + qi.associated_owner_key_id = None; + qi.associated_wallets = BTreeMap::new(); + let identity_id = qi.identity.id(); + ctx.insert_local_qualified_identity(&qi, &None) + .expect("insert bare User-typed identity"); + + let input = IdentityInputToLoad { + identity_id_input: identity_id.to_string(Encoding::Hex), + identity_type: IdentityType::Masternode, + alias_input: String::new(), + voting_private_key_input: Secret::new(""), + owner_private_key_input: Secret::new(""), + payout_address_private_key_input: Secret::new(""), + keys_input: vec![], + derive_keys_from_wallets: false, + selected_wallet_seed_hash: None, + encryption_password: None, + load_mode: IdentityLoadMode::RejectIfExists, + load_token: None, + }; + + let sdk = ctx.sdk(); + let result = ctx.load_identity(&sdk, input).await; + assert!( + !matches!(result, Err(TaskError::DuplicateProTxHash { .. })), + "bare User-typed record must not be treated as a conflicting duplicate: {result:?}" + ); + + ctx.wallet_backend().expect("backend").shutdown().await; + } + /// Merge×Tier-2 (success path) — merging a new key into a password-protected /// (Tier-2) node seals the new key Tier-2 *before* the at-rest insert, so /// the fail-closed guard (`encode_identity_blob_vault_first`) never rejects @@ -1395,4 +1626,196 @@ mod tests { ctx.wallet_backend().expect("backend").shutdown().await; } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn persisted_load_with_marker_cleanup_failure_still_reports_success() { + use crate::context::identity_load_registry::IdentityLoadPhase; + use crate::context::test_support::test_app_context; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + let (mut qualified_identity, _) = masternode_shaped_qi(); + qualified_identity.identity_type = IdentityType::User; + qualified_identity.private_keys = KeyStorage::default(); + let identity_id = qualified_identity.identity.id(); + ctx.insert_local_qualified_identity(&qualified_identity, &None) + .expect("durably persist loaded identity"); + ctx.record_forgotten_identity(&identity_id) + .expect("record forgotten marker"); + let fault_connection = + crate::context::test_support::open_persister_fault_connection(&backend); + fault_connection + .execute_batch( + "CREATE TRIGGER fail_load_marker_cleanup + BEFORE DELETE ON meta_global + WHEN OLD.key LIKE 'det:forgotten_identity:%' + BEGIN + SELECT RAISE(FAIL, 'injected load marker cleanup failure'); + END;", + ) + .expect("install marker cleanup failure trigger"); + let token = ctx + .mark_identity_load_submitted(identity_id) + .expect("submit load"); + let load_guard = ctx + .begin_identity_load(identity_id, Some(token)) + .expect("claim load"); + + ctx.finish_identity_load_after_persist(&identity_id, load_guard); + + assert!( + ctx.get_local_qualified_identity(&identity_id) + .expect("read persisted identity") + .is_some(), + "the identity must remain durably persisted" + ); + assert_eq!( + ctx.identity_load_phase(&identity_id, token), + Some(IdentityLoadPhase::Loaded), + "cleanup residue must not turn a committed load into a reported failure" + ); + + fault_connection + .execute_batch("DROP TRIGGER fail_load_marker_cleanup;") + .expect("remove marker cleanup failure trigger"); + backend.shutdown().await; + } + + async fn assert_generic_reload_clears_repaired_unload_ghost( + load_mode: IdentityLoadMode, + id_byte: u8, + ) { + use crate::context::test_support::test_app_context; + use dash_sdk::SdkBuilder; + use dash_sdk::dpp::platform_value::Value; + use dash_sdk::drive::query::{SelectProjection, WhereClause, WhereOperator}; + use dash_sdk::platform::DocumentQuery; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + + let identity_id = Identifier::from([id_byte; 32]); + let identity = Identity::create_basic_identity(identity_id, PlatformVersion::latest()) + .expect("identity"); + let old_key_id = + ctx.install_repaired_unload_ghost_for_test(identity.clone(), [id_byte + 1; 32]); + let view = IdentityKeyView::new(backend.secret_store(), identity_id.to_buffer()); + assert!( + view.get(&M, old_key_id) + .expect("read old vault key before reload") + .is_some(), + "precondition: the interrupted unload retains its old vault key", + ); + + let mut sdk = SdkBuilder::new_mock() + .with_version(PlatformVersion::latest()) + .build() + .expect("build pinned mock SDK"); + sdk.mock() + .expect_fetch(identity_id, Some(identity)) + .await + .expect("mock identity fetch"); + let dpns_query = DocumentQuery { + select: SelectProjection::documents(), + data_contract: ctx.dpns_contract.clone(), + document_type_name: "domain".to_string(), + where_clauses: vec![WhereClause { + field: "records.identity".to_string(), + operator: WhereOperator::Equal, + value: Value::Identifier(identity_id.into()), + }], + group_by: Vec::new(), + having: Vec::new(), + order_by_clauses: vec![], + limit: 100, + start: None, + }; + sdk.mock() + .expect_fetch_many( + dpns_query, + Some(dash_sdk::query_types::Documents::default()), + ) + .await + .expect("mock DPNS fetch"); + let input = IdentityInputToLoad { + identity_id_input: identity_id.to_string(Encoding::Hex), + identity_type: IdentityType::User, + alias_input: String::new(), + voting_private_key_input: Secret::new(""), + owner_private_key_input: Secret::new(""), + payout_address_private_key_input: Secret::new(""), + keys_input: vec![], + derive_keys_from_wallets: false, + selected_wallet_seed_hash: None, + encryption_password: None, + load_mode, + load_token: None, + }; + + let result = ctx.load_identity(&sdk, input).await; + assert!( + matches!( + result, + Ok(BackendTaskSuccessResult::LoadedIdentity(ref loaded)) + if loaded.identity.id() == identity_id + ), + "the repaired ghost must reload successfully: {result:?}", + ); + let Ok(BackendTaskSuccessResult::LoadedIdentity(loaded)) = &result else { + unreachable!("the successful load shape was asserted above"); + }; + assert!( + !loaded + .private_keys + .private_keys + .contains_key(&(M, old_key_id)), + "the replacement identity must not retain the ghost's stale key metadata", + ); + let persisted = ctx + .get_local_qualified_identity(&identity_id) + .expect("read replacement identity") + .expect("replacement identity is stored"); + assert!( + !persisted + .private_keys + .private_keys + .contains_key(&(M, old_key_id)), + "the replacement blob must not retain the ghost's stale key metadata", + ); + assert!( + view.get(&M, old_key_id) + .expect("read old vault key after reload") + .is_none(), + "the old vault key must be cleared before the replacement blob is written", + ); + assert!( + !ctx.is_identity_forgotten(&identity_id) + .expect("read marker after reload"), + "a successful replacement load must retire the forgotten marker", + ); + + backend.shutdown().await; + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn overwrite_and_merge_clear_repaired_unload_ghost_keys() { + assert_generic_reload_clears_repaired_unload_ghost(IdentityLoadMode::Overwrite, 0xB1).await; + assert_generic_reload_clears_repaired_unload_ghost( + IdentityLoadMode::MergeIntoExisting, + 0xB3, + ) + .await; + } } diff --git a/src/backend_task/identity/load_identity_by_dpns_name.rs b/src/backend_task/identity/load_identity_by_dpns_name.rs index 669a778cd..dd60ed7ff 100644 --- a/src/backend_task/identity/load_identity_by_dpns_name.rs +++ b/src/backend_task/identity/load_identity_by_dpns_name.rs @@ -7,6 +7,7 @@ use crate::model::qualified_identity::{ use crate::model::wallet::WalletSeedHash; use dash_sdk::Sdk; use dash_sdk::dpp::document::DocumentV0Getters; +use dash_sdk::dpp::identity::accessors::IdentityGettersV0; use dash_sdk::dpp::platform_value::Value; use dash_sdk::drive::query::{SelectProjection, WhereClause, WhereOperator}; use dash_sdk::platform::{Document, DocumentQuery, Fetch, FetchMany, Identity}; @@ -66,6 +67,8 @@ impl AppContext { Ok(None) => return Err(TaskError::IdentityNotFound), Err(e) => return Err(TaskError::from(e)), }; + let load_guard = + self.begin_identity_load_and_validate_type(IdentityType::User, &identity, None)?; // Get the label from the document for display let label = domain_doc @@ -152,13 +155,102 @@ impl AppContext { status: IdentityStatus::Active, network: self.network, }; + self.persist_identity_loaded_by_dpns_name(qualified_identity, load_guard) + } + + fn persist_identity_loaded_by_dpns_name( + &self, + qualified_identity: QualifiedIdentity, + load_guard: crate::context::identity_load_registry::IdentityLoadGuard, + ) -> Result { + let identity_id = qualified_identity.identity.id(); let wallet_info = qualified_identity .determine_wallet_info() .map_err(|e| TaskError::WalletInfoDeterminationFailed { detail: e })?; // Insert qualified identity into the database + self.prepare_stuck_unload_cleanup_for_reload(&identity_id.to_buffer())?; self.insert_local_qualified_identity(&qualified_identity, &wallet_info)?; + self.finish_identity_load_after_persist(&identity_id, load_guard); Ok(BackendTaskSuccessResult::LoadedIdentity(qualified_identity)) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::context::test_support::test_app_context; + use crate::model::qualified_identity::PrivateKeyTarget; + use crate::utils::egui_mpsc::SenderAsync; + use crate::wallet_backend::IdentityKeyView; + use dash_sdk::dpp::version::PlatformVersion; + use dash_sdk::platform::Identifier; + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn dpns_load_clears_repaired_unload_ghost_key() { + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + + let platform_version = PlatformVersion::latest(); + let identity_id = Identifier::from([0xD1; 32]); + let identity = + Identity::create_basic_identity(identity_id, platform_version).expect("identity"); + let old_key_id = ctx.install_repaired_unload_ghost_for_test(identity.clone(), [0xD2; 32]); + let view = IdentityKeyView::new(backend.secret_store(), identity_id.to_buffer()); + assert!( + view.get(&PrivateKeyTarget::PrivateKeyOnMainIdentity, old_key_id,) + .expect("read old vault key before reload") + .is_some(), + "precondition: the interrupted unload retains its old vault key", + ); + + let load_guard = ctx + .begin_identity_load_and_validate_type(IdentityType::User, &identity, None) + .expect("claim replacement load"); + let qualified_identity = QualifiedIdentity { + identity, + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::User, + alias: Some("alice.dash".to_string()), + private_keys: Default::default(), + dpns_names: Vec::new(), + associated_wallets: Default::default(), + secret_access: Some(backend.secret_access()), + wallet_index: None, + top_ups: Default::default(), + status: IdentityStatus::Active, + network: ctx.network(), + }; + let result = ctx.persist_identity_loaded_by_dpns_name(qualified_identity, load_guard); + assert!( + matches!( + result, + Ok(BackendTaskSuccessResult::LoadedIdentity(ref loaded)) + if loaded.identity.id() == identity_id + ), + "the repaired ghost must reload by DPNS name: {result:?}", + ); + assert!( + view.get(&PrivateKeyTarget::PrivateKeyOnMainIdentity, old_key_id,) + .expect("read old vault key after reload") + .is_none(), + "the old vault key must be cleared before the DPNS load writes its blob", + ); + assert!( + !ctx.is_identity_forgotten(&identity_id) + .expect("read marker after reload"), + "a successful DPNS load must retire the forgotten marker", + ); + + backend.shutdown().await; + } +} diff --git a/src/backend_task/identity/load_identity_from_wallet.rs b/src/backend_task/identity/load_identity_from_wallet.rs index 484d8bdfd..f884ec0f5 100644 --- a/src/backend_task/identity/load_identity_from_wallet.rs +++ b/src/backend_task/identity/load_identity_from_wallet.rs @@ -1,6 +1,7 @@ use super::{BackendTaskSuccessResult, IdentityIndex}; use crate::app::TaskResult; use crate::backend_task::error::TaskError; +use crate::backend_task::identity::IdentityDiscoveryMode; use crate::context::AppContext; use crate::model::qualified_identity::encrypted_key_storage::{ PrivateKeyData, WalletDerivationPath, @@ -98,6 +99,8 @@ impl AppContext { let matching_identity_key_id = matching_identity_key.id(); let identity_id = identity.id(); + let load_guard = + self.begin_identity_load_and_validate_type(IdentityType::User, &identity, None)?; let dpns_names_document_query = DocumentQuery { select: SelectProjection::documents(), @@ -249,6 +252,7 @@ impl AppContext { // Carry the user-assigned alias from any existing record so a re-load // refreshes keys/DPNS without wiping DET-only metadata. + self.prepare_stuck_unload_cleanup_for_reload(&identity_id.to_buffer())?; if let Some(existing) = self.get_identity_by_id(&identity_id)? { qualified_identity.alias = existing.alias; self.update_local_qualified_identity(&qualified_identity)?; @@ -265,10 +269,20 @@ impl AppContext { .identities .insert(identity_index, qualified_identity.identity.clone()); } + self.finish_identity_load_after_persist(&identity_id, load_guard); - Ok(BackendTaskSuccessResult::IdentitiesLoaded { count: 1 }) + Ok(BackendTaskSuccessResult::IdentitiesLoaded { + count: 1, + skipped_forgotten: 0, + failed: 0, + }) } + /// Search a whole wallet for identities, seeded from a user-supplied index. + /// + /// The search names no identity, so it never restores one the user + /// unloaded; those are reported back as `skipped_forgotten` and stay + /// unloaded until the user loads one by its own index. pub(super) async fn load_user_identities_up_to_index( self: &Arc, wallet_arc_ref: WalletArcRef, @@ -281,7 +295,7 @@ impl AppContext { .discover_identities_gap_limited( &wallet_arc_ref.wallet, seed_identity_index, - true, + IdentityDiscoveryMode::ExplicitSearch, Some(&sender), ) .await?; @@ -293,7 +307,156 @@ impl AppContext { } Ok(BackendTaskSuccessResult::IdentitiesLoaded { - count: summary.found, + count: summary.stored, + skipped_forgotten: summary.skipped_forgotten, + failed: summary.failed, }) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::context::test_support::test_app_context; + use crate::model::wallet::Wallet; + use crate::model::wallet::birth_height::WalletOrigin; + use crate::utils::egui_mpsc::SenderAsync; + use crate::wallet_backend::IdentityKeyView; + use dash_sdk::SdkBuilder; + use dash_sdk::dpp::dashcore::Network; + use dash_sdk::dpp::identity::identity_public_key::accessors::v0::{ + IdentityPublicKeyGettersV0, IdentityPublicKeySettersV0, + }; + use dash_sdk::dpp::identity::{Purpose, SecurityLevel}; + use dash_sdk::dpp::platform_value::BinaryData; + use dash_sdk::dpp::version::PlatformVersion; + use dash_sdk::platform::{Identifier, IdentityPublicKey}; + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn wallet_load_clears_repaired_unload_ghost_key() { + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender.clone()) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + + let seed = [0xC1; 64]; + let identity_index = 4; + let wallet = Wallet::new_from_seed( + seed, + Network::Testnet, + Some("Ghost reload".to_string()), + None, + ) + .expect("build wallet"); + let derived_public_key = wallet + .identity_authentication_ecdsa_public_key_from_seed( + &seed, + Network::Testnet, + identity_index, + 0, + ) + .expect("derive identity authentication key"); + let (wallet_seed_hash, wallet) = ctx + .register_wallet(wallet, &seed, WalletOrigin::Fresh) + .expect("register wallet"); + + let platform_version = PlatformVersion::latest(); + let identity_id = Identifier::from([0xC2; 32]); + let mut identity_key = IdentityPublicKey::random_key(1, Some(1), platform_version); + identity_key.set_purpose(Purpose::AUTHENTICATION); + identity_key.set_security_level(SecurityLevel::MASTER); + identity_key.set_key_type(KeyType::ECDSA_SECP256K1); + identity_key.set_data(BinaryData::new( + derived_public_key.inner.serialize().to_vec(), + )); + let identity = Identity::new_with_id_and_keys( + identity_id, + BTreeMap::from([(identity_key.id(), identity_key)]), + platform_version, + ) + .expect("build wallet-backed identity"); + let old_key_id = ctx.install_repaired_unload_ghost_for_test(identity.clone(), [0xC3; 32]); + let view = IdentityKeyView::new(backend.secret_store(), identity_id.to_buffer()); + assert!( + view.get(&PrivateKeyTarget::PrivateKeyOnMainIdentity, old_key_id,) + .expect("read old vault key before reload") + .is_some(), + "precondition: the interrupted unload retains its old vault key", + ); + + let mut sdk = SdkBuilder::new_mock() + .with_version(platform_version) + .build() + .expect("build pinned mock SDK"); + let identity_query = NonUniquePublicKeyHashQuery { + key_hash: derived_public_key.pubkey_hash().into(), + after: None, + }; + sdk.mock() + .expect_fetch(identity_query, Some(identity)) + .await + .expect("mock wallet identity fetch"); + let dpns_query = DocumentQuery { + select: SelectProjection::documents(), + data_contract: ctx.dpns_contract.clone(), + document_type_name: "domain".to_string(), + where_clauses: vec![WhereClause { + field: "records.identity".to_string(), + operator: WhereOperator::Equal, + value: Value::Identifier(identity_id.into()), + }], + group_by: Vec::new(), + having: Vec::new(), + order_by_clauses: vec![], + limit: 100, + start: None, + }; + sdk.mock() + .expect_fetch_many( + dpns_query, + Some(dash_sdk::query_types::Documents::default()), + ) + .await + .expect("mock DPNS fetch"); + + let result = ctx + .load_user_identity_from_wallet( + &sdk, + WalletArcRef { + wallet, + seed_hash: wallet_seed_hash, + }, + identity_index, + sender, + ) + .await; + assert!( + matches!( + result, + Ok(BackendTaskSuccessResult::IdentitiesLoaded { + count: 1, + skipped_forgotten: 0, + failed: 0 + }) + ), + "the repaired ghost must reload from its wallet: {result:?}", + ); + assert!( + view.get(&PrivateKeyTarget::PrivateKeyOnMainIdentity, old_key_id,) + .expect("read old vault key after reload") + .is_none(), + "the old vault key must be cleared before the wallet load writes its blob", + ); + assert!( + !ctx.is_identity_forgotten(&identity_id) + .expect("read marker after reload"), + "a successful wallet load must retire the forgotten marker", + ); + + backend.shutdown().await; + } +} diff --git a/src/backend_task/identity/mod.rs b/src/backend_task/identity/mod.rs index 41b9912d3..77134df30 100644 --- a/src/backend_task/identity/mod.rs +++ b/src/backend_task/identity/mod.rs @@ -1,6 +1,7 @@ mod add_key_to_identity; mod auth_pubkey_resolve; mod discover_identities; +mod load_guard; mod load_identity; mod load_identity_by_dpns_name; mod load_identity_from_wallet; @@ -12,8 +13,11 @@ mod register_identity; mod remove_identity; mod top_up_identity; mod transfer; +mod unload_identity; mod withdraw_from_identity; +pub(crate) use discover_identities::IdentityDiscoveryMode; + use super::{BackendTaskSuccessResult, FeeResult, TaskError}; use crate::app::TaskResult; use crate::context::AppContext; @@ -498,6 +502,19 @@ pub enum IdentityTask { /// The current per-identity password, verified before downgrading. password: Secret, }, + /// Permanently remove one identity's keys and local device state while + /// leaving the Platform identity itself unchanged. + /// + /// Unload deliberately requires no per-identity password, even for Tier-2 + /// protected identities. Deletion exposes no key material, and a password + /// gate is not a security boundary for someone with device and app access + /// who can already destroy the local files. [`Self::UnprotectIdentityKeys`] + /// verifies the password because it retains the keys while removing their + /// protection; deletion needs no equivalent proof. + UnloadIdentity { + /// The identity to unload from this device. + identity_id: Identifier, + }, WithdrawFromIdentity(QualifiedIdentity, Option
, Credits, Option), Transfer(QualifiedIdentity, Identifier, Credits, Option), /// Transfer credits from identity to Platform addresses @@ -908,6 +925,7 @@ impl AppContext { identity_id, password, } => self.unprotect_identity_keys(identity_id, password), + IdentityTask::UnloadIdentity { identity_id } => self.unload_identity(identity_id), } } diff --git a/src/backend_task/identity/remove_identity.rs b/src/backend_task/identity/remove_identity.rs index 6a43a5678..91b1aa788 100644 --- a/src/backend_task/identity/remove_identity.rs +++ b/src/backend_task/identity/remove_identity.rs @@ -16,27 +16,466 @@ impl AppContext { .map(|(voter_identity, _)| voter_identity.id()) }); - self.delete_local_qualified_identity(&identity_id)?; + // `unload_local_qualified_identity` (not the bare `delete_...`) so a + // wallet-derived identity's forgotten-marker is recorded — otherwise + // discovery would silently resurrect it after this "Remove" action, + // same as any other unload. Discriminate a committed-but-cleanup-only + // failure (`identity_was_removed()`) from a genuine removal failure so + // in-memory state still gets reconciled in the former case, matching + // `unload_identity()`'s contract instead of aborting via a bare `?`. + // Each residue below is logged where it is detected: the user is told to + // retry, and that report is only diagnosable against a log line naming + // the identity and the underlying error. + let cleanup_error = match self.unload_local_qualified_identity(&identity_id) { + Ok(()) => None, + Err(error) if error.identity_was_removed() => { + tracing::warn!( + ?error, + identity = %identity_id, + "Removed identity but left some of its local data behind" + ); + Some(error) + } + Err(error) => return Err(error), + }; + self.reconcile_unloaded_identity_memory(&identity_id); let mut removed_identity_ids = vec![identity_id]; + let primary_cleanup_failed = cleanup_error.is_some(); let mut associated_cleanup_failed = false; + let mut associated_removal_failed = false; if let Some(voter_id) = associated_voter_identity_id.filter(|id| *id != identity_id) { - match self.delete_local_qualified_identity(&voter_id) { - Ok(()) => removed_identity_ids.push(voter_id), - Err(error) => { + match self.unload_local_qualified_identity(&voter_id) { + Ok(()) => { + self.reconcile_unloaded_identity_memory(&voter_id); + removed_identity_ids.push(voter_id); + } + Err(error) if error.identity_was_removed() => { + self.reconcile_unloaded_identity_memory(&voter_id); + removed_identity_ids.push(voter_id); associated_cleanup_failed = true; tracing::warn!( ?error, voter_identity_id = %voter_id, - "Associated voter identity cleanup failed" + "Removed the associated voter identity but left some of its local data behind" + ); + } + Err(error) => { + associated_removal_failed = true; + tracing::warn!( + ?error, + voter_identity_id = %voter_id, + "Associated voter identity could not be removed" ); } } } + debug_assert!( + !(associated_cleanup_failed && associated_removal_failed), + "one associated voter cannot be both removed-with-residue and retained" + ); Ok(BackendTaskSuccessResult::RemovedIdentities { identity_ids: removed_identity_ids, + primary_cleanup_failed, associated_cleanup_failed, + associated_removal_failed, }) } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::context::test_support::test_app_context; + use crate::model::dashpay::ContactPrivateInfo; + use crate::model::qualified_identity::{IdentityStatus, IdentityType, QualifiedIdentity}; + use crate::model::wallet::Wallet; + use dash_sdk::dpp::dashcore::Network; + use dash_sdk::dpp::identity::Identity; + use dash_sdk::dpp::version::PlatformVersion; + use dash_sdk::platform::IdentityPublicKey; + use std::collections::BTreeMap; + use std::sync::{Arc, RwLock}; + + fn qualified_identity( + identity: Identity, + associated_voter_identity: Option<(Identity, IdentityPublicKey)>, + secret_access: crate::wallet_backend::SecretAccess, + ) -> QualifiedIdentity { + QualifiedIdentity { + identity, + associated_voter_identity, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::Masternode, + alias: None, + private_keys: Default::default(), + dpns_names: Vec::new(), + associated_wallets: BTreeMap::new(), + secret_access: Some(secret_access), + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::Active, + network: Network::Testnet, + } + } + + async fn removal_result_with_cleanup_failure( + fail_associated_cleanup: bool, + ) -> BackendTaskSuccessResult { + use crate::app::TaskResult; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + let platform_version = PlatformVersion::latest(); + let target_id = Identifier::from([0x81; 32]); + let voter_id = Identifier::from([0x82; 32]); + let target = Identity::create_basic_identity(target_id, platform_version) + .expect("create target identity"); + let voter = Identity::create_basic_identity(voter_id, platform_version) + .expect("create voter identity"); + let voter_key = IdentityPublicKey::random_key(1, Some(1), platform_version); + let associated_voter_identity = fail_associated_cleanup.then(|| (voter.clone(), voter_key)); + let target = qualified_identity(target, associated_voter_identity, backend.secret_access()); + ctx.insert_local_qualified_identity(&target, &None) + .expect("insert target identity"); + if fail_associated_cleanup { + let voter = qualified_identity(voter, None, backend.secret_access()); + ctx.insert_local_qualified_identity(&voter, &None) + .expect("insert voter identity"); + } + + let fault_id = if fail_associated_cleanup { + voter_id + } else { + target_id + }; + let contact_id = Identifier::from([0x83; 32]); + backend + .dashpay_set_private_info( + &fault_id, + &contact_id, + &ContactPrivateInfo { + nickname: "cleanup fault".into(), + notes: "cleanup fault".into(), + is_hidden: false, + }, + ) + .expect("seed owner overlay"); + let fault_buf = fault_id.to_buffer(); + let overlay_key = backend + .kv() + .list( + crate::wallet_backend::DetScope::Identity(&fault_buf), + Some("det:dashpay:private:"), + ) + .expect("list owner overlays") + .into_iter() + .next() + .expect("owner overlay key"); + let persister_path = backend.spv_storage_dir().join("platform-wallet.sqlite"); + let fault_connection = + rusqlite::Connection::open(&persister_path).expect("open persister second handle"); + fault_connection + .execute_batch(&format!( + "CREATE TRIGGER fail_remove_identity_overlay_delete + BEFORE DELETE ON meta_identity + WHEN OLD.identity_id = X'{}' AND OLD.key = '{}' + BEGIN + SELECT RAISE(FAIL, 'injected owner overlay delete failure'); + END;", + hex::encode(fault_buf), + overlay_key.replace('\'', "''"), + )) + .expect("install owner-overlay delete trigger"); + + let result = ctx + .remove_identity(target_id) + .expect("committed cleanup faults remain a successful removal result"); + + fault_connection + .execute_batch("DROP TRIGGER fail_remove_identity_overlay_delete;") + .expect("remove owner-overlay delete trigger"); + backend.shutdown().await; + result + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn remove_identity_reports_primary_and_associated_cleanup_failures_independently() { + let primary_failure = removal_result_with_cleanup_failure(false).await; + assert!(matches!( + primary_failure, + BackendTaskSuccessResult::RemovedIdentities { + primary_cleanup_failed: true, + associated_cleanup_failed: false, + associated_removal_failed: false, + .. + } + )); + + let associated_failure = removal_result_with_cleanup_failure(true).await; + assert!(matches!( + associated_failure, + BackendTaskSuccessResult::RemovedIdentities { + primary_cleanup_failed: false, + associated_cleanup_failed: true, + associated_removal_failed: false, + .. + } + )); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn remove_identity_never_combines_associated_cleanup_and_removal_failures() { + let result = removal_result_with_cleanup_failure(true).await; + assert!(matches!( + result, + BackendTaskSuccessResult::RemovedIdentities { + associated_cleanup_failed: true, + associated_removal_failed: false, + .. + } + )); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn remove_identity_distinguishes_associated_removal_failure_from_cleanup_residue() { + use crate::app::TaskResult; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + let platform_version = PlatformVersion::latest(); + let target_id = Identifier::from([0x84; 32]); + let voter_id = Identifier::from([0x85; 32]); + let target = Identity::create_basic_identity(target_id, platform_version) + .expect("create target identity"); + let voter = Identity::create_basic_identity(voter_id, platform_version) + .expect("create voter identity"); + let voter_key = IdentityPublicKey::random_key(1, Some(1), platform_version); + let target = qualified_identity( + target, + Some((voter.clone(), voter_key)), + backend.secret_access(), + ); + let voter = qualified_identity(voter, None, backend.secret_access()); + ctx.insert_local_qualified_identity(&target, &None) + .expect("insert target identity"); + ctx.insert_local_qualified_identity(&voter, &None) + .expect("insert voter identity"); + let voter_load_guard = ctx + .begin_identity_load(voter_id, None) + .expect("hold voter load claim"); + + let result = ctx + .remove_identity(target_id) + .expect("primary removal remains successful"); + + assert!(matches!( + result, + BackendTaskSuccessResult::RemovedIdentities { + identity_ids, + primary_cleanup_failed: false, + associated_cleanup_failed: false, + associated_removal_failed: true, + } if identity_ids == vec![target_id] + )); + assert!( + ctx.get_local_qualified_identity(&voter_id) + .expect("read voter identity") + .is_some(), + "a genuine associated removal failure must leave the voter present" + ); + + drop(voter_load_guard); + backend.shutdown().await; + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn remove_identity_reconciles_wallet_cache_and_selection() { + use crate::app::TaskResult; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + let platform_version = PlatformVersion::latest(); + let target_id = Identifier::from([0x61; 32]); + let sibling_id = Identifier::from([0x62; 32]); + let target = Identity::create_basic_identity(target_id, platform_version) + .expect("create target identity"); + let sibling = Identity::create_basic_identity(sibling_id, platform_version) + .expect("create sibling identity"); + let mut wallet = Wallet::new_from_seed([0x63; 64], Network::Testnet, None, None) + .expect("build test wallet"); + wallet.identities.insert(3, target); + wallet.identities.insert(7, sibling); + let wallet_seed_hash = wallet.seed_hash(); + ctx.wallets() + .write() + .expect("write wallets") + .insert(wallet_seed_hash, Arc::new(RwLock::new(wallet))); + ctx.set_selected_identity(Some(target_id)); + ctx.set_pending_identity_selection(target_id); + + let result = ctx + .remove_identity(target_id) + .expect("remove identity through the real handler"); + + assert!(matches!( + &result, + BackendTaskSuccessResult::RemovedIdentities { + identity_ids, + primary_cleanup_failed, + associated_cleanup_failed, + associated_removal_failed, + } if identity_ids == &vec![target_id] + && !primary_cleanup_failed + && !associated_cleanup_failed + && !associated_removal_failed + )); + let (target_evicted, cached_sibling) = { + let wallets = ctx.wallets().read().expect("read wallets"); + let wallet = wallets + .get(&wallet_seed_hash) + .expect("test wallet remains") + .read() + .expect("read test wallet"); + ( + wallet + .identities + .values() + .all(|identity| identity.id() != target_id), + wallet.identities.get(&7).map(IdentityGettersV0::id), + ) + }; + assert!( + target_evicted, + "the removed identity must be evicted from the wallet cache" + ); + assert_eq!( + cached_sibling, + Some(sibling_id), + "the sibling identity must remain cached" + ); + assert_eq!( + ctx.selected_identity_id(), + None, + "the removed identity must no longer be selected" + ); + assert_eq!( + ctx.take_pending_identity_selection(), + None, + "a pending selection for the removed identity must be cleared" + ); + backend.shutdown().await; + } + + /// QA (issue #889 review): the two `associated_voter_identity_id` shapes + /// the associated-voter branch has to tell apart — absent, and + /// self-referential (the voter identity is the identity being removed). + /// The `.filter(|id| *id != identity_id)` guard exists precisely to skip + /// the second identity_id==voter_id case; unlike the ordinary + /// distinct-voter path exercised elsewhere in this file, no test drove a + /// self-referential voter through the real entry point before this one. + /// A regression here would either double-process the same identity or + /// wrongly flag `associated_removal_failed`/`associated_cleanup_failed` + /// for an identity that was, in fact, fully removed by the primary step. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn remove_identity_handles_absent_and_self_referential_voter() { + use crate::app::TaskResult; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + let platform_version = PlatformVersion::latest(); + + // No associated voter at all. + let no_voter_id = Identifier::from([0x86; 32]); + let no_voter_identity = Identity::create_basic_identity(no_voter_id, platform_version) + .expect("create no-voter identity"); + let no_voter = qualified_identity(no_voter_identity, None, backend.secret_access()); + ctx.insert_local_qualified_identity(&no_voter, &None) + .expect("insert no-voter identity"); + + let result = ctx + .remove_identity(no_voter_id) + .expect("remove an identity with no associated voter"); + assert!( + matches!( + &result, + BackendTaskSuccessResult::RemovedIdentities { + identity_ids, + primary_cleanup_failed: false, + associated_cleanup_failed: false, + associated_removal_failed: false, + } if identity_ids == &vec![no_voter_id] + ), + "an identity with no associated voter must report exactly itself and no \ + associated-voter failure flags, got {result:?}" + ); + + // Self-referential voter: associated_voter_identity_id == identity_id. + let self_voter_id = Identifier::from([0x87; 32]); + let self_voter_identity = Identity::create_basic_identity(self_voter_id, platform_version) + .expect("create self-referential identity"); + let self_voter_key = IdentityPublicKey::random_key(1, Some(1), platform_version); + let self_voter = qualified_identity( + self_voter_identity.clone(), + Some((self_voter_identity, self_voter_key)), + backend.secret_access(), + ); + ctx.insert_local_qualified_identity(&self_voter, &None) + .expect("insert self-referential identity"); + + let result = ctx + .remove_identity(self_voter_id) + .expect("remove a self-referentially-voting identity"); + assert!( + matches!( + &result, + BackendTaskSuccessResult::RemovedIdentities { + identity_ids, + primary_cleanup_failed: false, + associated_cleanup_failed: false, + associated_removal_failed: false, + } if identity_ids == &vec![self_voter_id] + ), + "a self-referential voter must not be double-processed or reported as an \ + associated-removal failure, got {result:?}" + ); + assert!( + ctx.get_local_qualified_identity(&self_voter_id) + .expect("read removed self-referential identity") + .is_none(), + "the self-referential identity must actually be gone, not just reported as such" + ); + + backend.shutdown().await; + } +} diff --git a/src/backend_task/identity/unload_identity.rs b/src/backend_task/identity/unload_identity.rs new file mode 100644 index 000000000..95679f796 --- /dev/null +++ b/src/backend_task/identity/unload_identity.rs @@ -0,0 +1,424 @@ +use std::collections::HashMap; + +use dash_sdk::dpp::identity::Identity; +use dash_sdk::dpp::identity::accessors::IdentityGettersV0; +use dash_sdk::platform::Identifier; + +use super::BackendTaskSuccessResult; +use crate::backend_task::error::TaskError; +use crate::context::AppContext; +use crate::wallet_backend::poison::{MutexRecover, RwLockRecover}; + +fn retain_other_identities(identities: &mut HashMap, identity_id: &Identifier) { + identities.retain(|_, identity| identity.id() != *identity_id); +} + +impl AppContext { + pub(crate) fn reconcile_unloaded_identity_memory(&self, identity_id: &Identifier) { + let wallets = self.wallets.read_recover(); + for wallet in wallets.values() { + retain_other_identities(&mut wallet.write_recover().identities, identity_id); + } + drop(wallets); + + if self.selected_identity_id() == Some(*identity_id) { + self.set_selected_identity(None); + } + let mut pending = self.pending_identity_selection.lock_recover(); + if *pending == Some(*identity_id) { + *pending = None; + } + } + + pub(super) fn unload_identity( + &self, + identity_id: Identifier, + ) -> Result { + let cleanup_error = match self.unload_local_qualified_identity(&identity_id) { + Ok(()) => None, + Err(error) if error.identity_was_removed() => Some(error), + Err(error) => return Err(error), + }; + self.reconcile_unloaded_identity_memory(&identity_id); + + tracing::info!( + target = "backend_task::identity::unload_identity", + identity = %identity_id, + "Unloaded identity and its local device state", + ); + if let Some(error) = cleanup_error { + return Err(error); + } + Ok(BackendTaskSuccessResult::UnloadedIdentity(identity_id)) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::context::test_support::test_app_context; + use crate::model::dashpay::ContactPrivateInfo; + use crate::model::qualified_identity::{IdentityStatus, IdentityType, QualifiedIdentity}; + use crate::model::wallet::Wallet; + use dash_sdk::dpp::dashcore::Network; + use dash_sdk::dpp::version::PlatformVersion; + use std::collections::BTreeMap; + use std::sync::{Arc, RwLock}; + + #[test] + fn identity_unload_evicts_only_target_from_wallet_cache() { + let platform_version = PlatformVersion::latest(); + let target_id = Identifier::from([0x11; 32]); + let sibling_id = Identifier::from([0x22; 32]); + let target = Identity::create_basic_identity(target_id, platform_version) + .expect("create target identity"); + let sibling = Identity::create_basic_identity(sibling_id, platform_version) + .expect("create sibling identity"); + let mut identities = HashMap::from([(3, target), (7, sibling)]); + + retain_other_identities(&mut identities, &target_id); + + assert_eq!(identities.len(), 1, "only the target must be evicted"); + assert_eq!( + identities.get(&7).map(IdentityGettersV0::id), + Some(sibling_id), + "the sibling identity must remain cached" + ); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn identity_unload_handler_clears_wallet_cache_and_identity_selection() { + use crate::app::TaskResult; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + let platform_version = PlatformVersion::latest(); + let target_id = Identifier::from([0x31; 32]); + let sibling_id = Identifier::from([0x32; 32]); + let target = Identity::create_basic_identity(target_id, platform_version) + .expect("create target identity"); + let sibling = Identity::create_basic_identity(sibling_id, platform_version) + .expect("create sibling identity"); + let mut wallet = Wallet::new_from_seed([0x33; 64], Network::Testnet, None, None) + .expect("build test wallet"); + wallet.identities.insert(3, target); + wallet.identities.insert(7, sibling); + let wallet_seed_hash = wallet.seed_hash(); + ctx.wallets() + .write() + .expect("write wallets") + .insert(wallet_seed_hash, Arc::new(RwLock::new(wallet))); + ctx.set_selected_identity(Some(target_id)); + ctx.set_pending_identity_selection(target_id); + + let result = ctx + .unload_identity(target_id) + .expect("unload identity through the real handler"); + + assert!(matches!( + result, + BackendTaskSuccessResult::UnloadedIdentity(identity_id) if identity_id == target_id + )); + // Snapshot the cache state under the wallet guards, releasing them at the + // block's end so no guard is live across `backend.shutdown().await` below + // (clippy::await_holding_lock). + let (target_evicted, cached_sibling) = { + let wallets = ctx.wallets().read().expect("read wallets"); + let wallet = wallets + .get(&wallet_seed_hash) + .expect("test wallet remains") + .read() + .expect("read test wallet"); + ( + wallet + .identities + .values() + .all(|identity| identity.id() != target_id), + wallet.identities.get(&7).map(IdentityGettersV0::id), + ) + }; + assert!( + target_evicted, + "the target identity must be evicted from the wallet cache" + ); + assert_eq!( + cached_sibling, + Some(sibling_id), + "the sibling identity must remain cached" + ); + assert_eq!( + ctx.selected_identity_id(), + None, + "the unloaded identity must no longer be selected" + ); + assert_eq!( + ctx.take_pending_identity_selection(), + None, + "a pending selection for the unloaded identity must be cleared" + ); + backend.shutdown().await; + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn identity_unload_reconciles_memory_after_committed_cleanup_failure() { + use crate::app::TaskResult; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + let platform_version = PlatformVersion::latest(); + let target_id = Identifier::from([0x51; 32]); + let contact_id = Identifier::from([0x52; 32]); + let target = Identity::create_basic_identity(target_id, platform_version) + .expect("create target identity"); + let mut wallet = Wallet::new_from_seed([0x53; 64], Network::Testnet, None, None) + .expect("build test wallet"); + wallet.identities.insert(3, target.clone()); + let wallet_seed_hash = wallet.seed_hash(); + let qualified_identity = QualifiedIdentity { + identity: target, + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::User, + alias: None, + private_keys: Default::default(), + dpns_names: Vec::new(), + associated_wallets: BTreeMap::from([( + wallet_seed_hash, + Arc::new(RwLock::new(wallet.clone())), + )]), + secret_access: Some(backend.secret_access()), + wallet_index: Some(3), + top_ups: BTreeMap::new(), + status: IdentityStatus::Active, + network: Network::Testnet, + }; + ctx.insert_local_qualified_identity(&qualified_identity, &Some((wallet_seed_hash, 3))) + .expect("insert target identity"); + ctx.wallets() + .write() + .expect("write wallets") + .insert(wallet_seed_hash, Arc::new(RwLock::new(wallet))); + ctx.set_selected_identity(Some(target_id)); + ctx.set_pending_identity_selection(target_id); + + backend + .dashpay_set_private_info( + &target_id, + &contact_id, + &ContactPrivateInfo { + nickname: "target contact".into(), + notes: "target note".into(), + is_hidden: false, + }, + ) + .expect("seed target owner overlay"); + let target_buf = target_id.to_buffer(); + let overlay_key = backend + .kv() + .list( + crate::wallet_backend::DetScope::Identity(&target_buf), + Some("det:dashpay:private:"), + ) + .expect("list target overlays") + .into_iter() + .next() + .expect("target overlay key"); + let persister_path = backend.spv_storage_dir().join("platform-wallet.sqlite"); + let fault_connection = + rusqlite::Connection::open(&persister_path).expect("open persister second handle"); + fault_connection + .execute_batch(&format!( + "CREATE TRIGGER fail_unload_overlay_delete + BEFORE DELETE ON meta_identity + WHEN OLD.identity_id = X'{}' AND OLD.key = '{}' + BEGIN + SELECT RAISE(FAIL, 'injected owner overlay delete failure'); + END;", + hex::encode(target_buf), + overlay_key.replace('\'', "''"), + )) + .expect("install owner-overlay delete trigger"); + + let error = ctx + .unload_identity(target_id) + .expect_err("cleanup failure must still reach the caller"); + assert!(matches!( + error, + TaskError::IdentityUnloadCleanupFailed { identity_id, .. } + if identity_id == target_id + )); + + let target_is_cached = { + let wallets = ctx.wallets().read().expect("read wallets"); + let wallet = wallets + .get(&wallet_seed_hash) + .expect("wallet remains") + .read() + .expect("read wallet"); + wallet + .identities + .values() + .any(|identity| identity.id() == target_id) + }; + assert!( + !target_is_cached, + "post-commit cleanup errors must still evict the wallet cache" + ); + assert_eq!(ctx.selected_identity_id(), None); + assert_eq!(ctx.take_pending_identity_selection(), None); + + fault_connection + .execute_batch("DROP TRIGGER fail_unload_overlay_delete;") + .expect("remove owner-overlay delete trigger"); + backend.shutdown().await; + } + + /// QA (issue #889 review): a second unload of the identity a first unload + /// already claims must be rejected outright through the real + /// `unload_identity()` task handler (not just the lower-level + /// `delete_local_qualified_identity_inner`), and — because it never got + /// past `begin_identity_load` — must leave in-memory state completely + /// untouched: `error.identity_was_removed()` is false for + /// `IdentityBusyWithLoad`, so `unload_identity()` returns before ever + /// calling `reconcile_unloaded_identity_memory`. A rejected concurrent + /// unload evicting the wallet cache or clearing the selection anyway + /// would be a real bug: it would desync the UI from storage, which still + /// holds the identity untouched. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn second_concurrent_unload_of_the_same_identity_is_rejected_without_side_effects() { + use crate::app::TaskResult; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + let platform_version = PlatformVersion::latest(); + let target_id = Identifier::from([0x91; 32]); + let target = Identity::create_basic_identity(target_id, platform_version) + .expect("create target identity"); + let mut wallet = Wallet::new_from_seed([0x93; 64], Network::Testnet, None, None) + .expect("build test wallet"); + wallet.identities.insert(3, target.clone()); + let wallet_seed_hash = wallet.seed_hash(); + let qualified_identity = QualifiedIdentity { + identity: target, + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::User, + alias: None, + private_keys: Default::default(), + dpns_names: Vec::new(), + associated_wallets: BTreeMap::from([( + wallet_seed_hash, + Arc::new(RwLock::new(wallet.clone())), + )]), + secret_access: Some(backend.secret_access()), + wallet_index: Some(3), + top_ups: BTreeMap::new(), + status: IdentityStatus::Active, + network: Network::Testnet, + }; + ctx.insert_local_qualified_identity(&qualified_identity, &Some((wallet_seed_hash, 3))) + .expect("insert target identity"); + ctx.wallets() + .write() + .expect("write wallets") + .insert(wallet_seed_hash, Arc::new(RwLock::new(wallet))); + ctx.set_selected_identity(Some(target_id)); + ctx.set_pending_identity_selection(target_id); + + // Simulate a first unload already in flight by holding its exclusive + // claim directly, the same claim `unload_local_qualified_identity` + // takes internally. + let first_unload_guard = ctx + .begin_identity_load(target_id, None) + .expect("claim the identity for the first, in-flight unload"); + + let second_unload_error = ctx + .unload_identity(target_id) + .expect_err("a second unload of the same identity must be rejected, not raced"); + assert!( + matches!( + second_unload_error, + TaskError::IdentityBusyWithLoad { identity_id } if identity_id == target_id + ), + "the rejection must be IdentityBusyWithLoad, got {second_unload_error:?}" + ); + + // Nothing the rejected call touched: storage, wallet cache, and + // selection must all still reflect the identity as loaded. + assert!( + ctx.get_local_qualified_identity(&target_id) + .expect("read identity") + .is_some(), + "a rejected concurrent unload must not remove the identity from storage" + ); + let target_is_cached = { + let wallets = ctx.wallets().read().expect("read wallets"); + let wallet = wallets + .get(&wallet_seed_hash) + .expect("wallet remains") + .read() + .expect("read wallet"); + wallet + .identities + .values() + .any(|identity| identity.id() == target_id) + }; + assert!( + target_is_cached, + "a rejected concurrent unload must not evict the wallet cache" + ); + assert_eq!( + ctx.selected_identity_id(), + Some(target_id), + "a rejected concurrent unload must not clear the selection" + ); + assert_eq!( + ctx.take_pending_identity_selection(), + Some(target_id), + "a rejected concurrent unload must not clear the pending selection" + ); + ctx.set_pending_identity_selection(target_id); + + // Once the first unload's claim is released, a genuine second attempt + // must succeed and actually perform the unload this time. + drop(first_unload_guard); + let result = ctx + .unload_identity(target_id) + .expect("unload succeeds once the in-flight claim is released"); + assert!(matches!( + result, + BackendTaskSuccessResult::UnloadedIdentity(identity_id) if identity_id == target_id + )); + assert!( + ctx.get_local_qualified_identity(&target_id) + .expect("read identity") + .is_none(), + "the retried unload must actually remove the identity" + ); + + backend.shutdown().await; + } +} diff --git a/src/backend_task/mod.rs b/src/backend_task/mod.rs index cc4431920..83e866d37 100644 --- a/src/backend_task/mod.rs +++ b/src/backend_task/mod.rs @@ -717,7 +717,11 @@ pub enum BackendTaskSuccessResult { }, RemovedIdentities { identity_ids: Vec, + primary_cleanup_failed: bool, + /// Mutually exclusive with `associated_removal_failed`. associated_cleanup_failed: bool, + /// Mutually exclusive with `associated_cleanup_failed`. + associated_removal_failed: bool, }, RefreshedIdentity(QualifiedIdentity), LoadedIdentity(QualifiedIdentity), @@ -736,6 +740,8 @@ pub enum BackendTaskSuccessResult { /// The identity whose key protection was removed. identity_id: Identifier, }, + /// One identity and its owner-attributable local state were removed. + UnloadedIdentity(Identifier), // Document operation results (replacing string messages) DeletedDocument(Identifier, FeeResult), @@ -838,6 +844,14 @@ pub enum BackendTaskSuccessResult { /// Identities were discovered and loaded from a wallet by index search. IdentitiesLoaded { count: u32, + /// Identities the search found but left unloaded because the user had + /// unloaded them. Reported so a search that loads fewer identities than + /// it found is not mistaken for a failure. + skipped_forgotten: u32, + /// Identities the search found but could not store on this device. + /// Reported so a search whose writes all failed is not mistaken for one + /// that found nothing new. + failed: u32, }, } diff --git a/src/context/identity_db.rs b/src/context/identity_db.rs index fc7e6a96b..c019401d2 100644 --- a/src/context/identity_db.rs +++ b/src/context/identity_db.rs @@ -33,6 +33,23 @@ const IDENTITY_ORDER_KEY: &str = "det:identity_order:v1"; /// full set. const IDENTITY_INDEX_KEY: &str = "det:identity_index:v1"; +/// "Deliberately unloaded" marker, one Global key per identity. The full key +/// is `det:forgotten_identity:`; presence is the whole signal, so +/// the value is empty. Enumerated by prefix scan. +/// +/// One key per identity rather than one shared set: the adapter takes the +/// persister lock per call, so a shared collection would turn every marker +/// write into an unguarded read-modify-write and let two identities' unloads +/// clobber each other. Independent keys collide no more than the per-row SQL +/// table this replaced. +/// +/// [`DetScope::Global`] rather than [`DetScope::Identity`] because the marker +/// has to outlive the identity it names: an identity-scoped slot is reaped by +/// the upstream soft-cascade at exactly the moment the marker becomes +/// load-bearing. Per-network by virtue of the store, like +/// [`IDENTITY_INDEX_KEY`]. +const FORGOTTEN_IDENTITY_KEY_PREFIX: &str = "det:forgotten_identity:"; + /// Scheduled-vote slot key, scoped to [`DetScope::Identity`] of the /// voter. The full key is `det:scheduled_vote:` — the /// voter id is carried by the scope. @@ -69,6 +86,32 @@ fn top_up_err(source: KvAdapterError) -> TaskError { TaskError::TopUpHistoryStorage { source } } +/// Map a k/v adapter failure to the forgotten-identity marker storage error. +fn forgotten_err(source: KvAdapterError) -> TaskError { + TaskError::ForgottenIdentityStorage { source } +} + +fn keep_first_unload_cleanup_error( + cleanup_error: &mut Option, + identity_id: Identifier, + result: std::result::Result<(), TaskError>, +) { + if let Err(source) = result { + if cleanup_error.is_none() { + *cleanup_error = Some(TaskError::IdentityUnloadCleanupFailed { + identity_id, + source: Box::new(source), + }); + } else { + tracing::warn!( + identity_id = %identity_id, + error = ?source, + "Additional identity unload cleanup step failed" + ); + } + } +} + /// Merge `top_ups` into the stored history of `identity_id` (read-merge-write). /// /// Callers hold a partial view of the history — the top-up flow carries the @@ -253,6 +296,56 @@ fn index_remove_identity(kv: &DetKv, identity_id: &[u8; 32]) -> std::result::Res .map_err(identity_err) } +/// The marker key naming `identity_id`. +fn forgotten_identity_key(identity_id: &Identifier) -> String { + use dash_sdk::dpp::platform_value::string_encoding::Encoding; + format!( + "{FORGOTTEN_IDENTITY_KEY_PREFIX}{}", + identity_id.to_string(Encoding::Base58) + ) +} + +/// Recover the identity a marker key names. `None` when the suffix is not a +/// valid identifier — a damaged or foreign key sharing the prefix, which the +/// enumeration skips rather than failing the whole listing over. +fn forgotten_identity_from_key(key: &str) -> Option { + use dash_sdk::dpp::platform_value::string_encoding::Encoding; + let suffix = key.strip_prefix(FORGOTTEN_IDENTITY_KEY_PREFIX)?; + Identifier::from_string(suffix, Encoding::Base58).ok() +} + +/// Mark `identity_id` as deliberately unloaded. Idempotent — the write is a +/// blind upsert of a presence-only key, so it neither reads nor rewrites any +/// other identity's marker. +fn forgotten_marker_put( + kv: &DetKv, + identity_id: &Identifier, +) -> std::result::Result<(), TaskError> { + kv.put::<()>(DetScope::Global, &forgotten_identity_key(identity_id), &()) + .map_err(forgotten_err) +} + +/// Drop `identity_id`'s marker. Idempotent — deleting an absent key is `Ok`. +fn forgotten_marker_delete( + kv: &DetKv, + identity_id: &Identifier, +) -> std::result::Result<(), TaskError> { + kv.delete(DetScope::Global, &forgotten_identity_key(identity_id)) + .map_err(forgotten_err) +} + +/// Whether `identity_id` carries a marker. A damaged marker fails only this +/// identity's check; every other identity's guard is unaffected. +fn forgotten_marker_exists( + kv: &DetKv, + identity_id: &Identifier, +) -> std::result::Result { + Ok(kv + .get::<()>(DetScope::Global, &forgotten_identity_key(identity_id)) + .map_err(forgotten_err)? + .is_some()) +} + /// Delete every Identity-scoped child of `id` (blob, top-up history, all /// scheduled votes) and prune the scheduled-vote voter index. Does not /// touch the Global identity index — callers decide whether to drop the @@ -422,9 +515,12 @@ fn encode_identity_blob_vault_first( fn purge_identity_scope(kv: &DetKv, id: &[u8; 32]) -> std::result::Result<(), TaskError> { let scope = DetScope::Identity(id); - kv.delete(scope, IDENTITY_KEY).map_err(identity_err)?; + // The identity blob is the recovery inventory for its vault keys. Delete + // auxiliary records first and the blob last so a partial purge remains + // discoverable and retryable. kv.delete(scope, TOP_UPS_KEY).map_err(top_up_err)?; - delete_scheduled_votes_for_voter(kv, id) + delete_scheduled_votes_for_voter(kv, id)?; + kv.delete(scope, IDENTITY_KEY).map_err(identity_err) } /// Read the Global scheduled-vote voter index. Returns an empty vector @@ -909,8 +1005,8 @@ impl AppContext { .collect()) } - /// Remove a locally-stored identity and all of its Identity-scoped - /// children. Returns `Ok(())` even when the identity is unknown — + /// Remove a locally-stored identity and its owner-attributable local state. + /// Returns `Ok(())` even when the identity is unknown — /// mirrors the pre-C7 `DELETE` which silently no-ops on missing rows. /// /// Cleanup verdict: explicit. DET never deletes the upstream @@ -918,12 +1014,224 @@ impl AppContext { /// DET stores the qualified-identity blob in the `meta_identity` k/v /// scope only), so the upstream `cascade_meta_identity_on_identity_delete` /// trigger never fires for this path. This method therefore drains the - /// Identity scope itself — the blob, the top-up history, and every - /// scheduled vote queued for this identity — and removes the Global - /// index entries that the trigger would not touch. + /// identity blob, keys, top-up history, scheduled votes, DashPay overlays, + /// entity timestamp, reverse address mappings, metadata, and Global identity + /// index entry itself. + /// Global payment timestamp entries keyed only by transaction id cannot be + /// attributed to one owner and remain until full-wallet teardown. Upstream + /// `platform-wallet`'s `IdentitySyncManager`, exposed through + /// `identity_sync()`, also has no per-identity forget operation, so token sync + /// continues tracking a removed identity until upstream adds one. pub fn delete_local_qualified_identity( &self, identifier: &Identifier, + ) -> std::result::Result<(), TaskError> { + self.delete_local_qualified_identity_inner(identifier, false) + } + + /// Delete an identity and remember a wallet-derived user's unload choice. + pub(crate) fn unload_local_qualified_identity( + &self, + identifier: &Identifier, + ) -> std::result::Result<(), TaskError> { + self.delete_local_qualified_identity_inner(identifier, true) + } + + /// Record that automatic discovery must not restore an unloaded identity. + pub(crate) fn record_forgotten_identity( + &self, + identifier: &Identifier, + ) -> std::result::Result<(), TaskError> { + forgotten_marker_put(&self.det_kv()?, identifier) + } + + /// Whether automatic discovery must leave this identity unloaded. + pub(crate) fn is_identity_forgotten( + &self, + identifier: &Identifier, + ) -> std::result::Result { + forgotten_marker_exists(&self.det_kv()?, identifier) + } + + /// Every identity deliberately unloaded on this network, read by prefix + /// scan. A marker key whose suffix does not decode is skipped with a + /// warning rather than failing the sweep that consumes this list. + pub(crate) fn list_forgotten_identities( + &self, + ) -> std::result::Result, TaskError> { + let kv = self.det_kv()?; + let keys = kv + .list(DetScope::Global, Some(FORGOTTEN_IDENTITY_KEY_PREFIX)) + .map_err(forgotten_err)?; + Ok(keys + .into_iter() + .filter_map(|key| { + let identity_id = forgotten_identity_from_key(&key); + if identity_id.is_none() { + tracing::warn!(key = %key, "Skipping unreadable forgotten-identity marker"); + } + identity_id + }) + .collect()) + } + + /// Clear the discovery block after a user-requested load succeeds. + pub(crate) fn clear_forgotten_identity_after_explicit_load( + &self, + identifier: &Identifier, + ) -> std::result::Result<(), TaskError> { + forgotten_marker_delete(&self.det_kv()?, identifier) + } + + /// Finish cleanup for a forgotten, unindexed identity whose blob remains. + /// + /// Returns `Ok(true)` after cleanup, `Ok(false)` for a non-ghost state, and + /// preserves the blob and marker when vault-key clearing or purging fails. + pub(crate) fn retry_stuck_unload_cleanup( + &self, + id: &[u8; 32], + ) -> std::result::Result { + self.retry_stuck_unload_cleanup_inner(id, true) + } + + /// Load-path form of [`Self::retry_stuck_unload_cleanup`]. The forgotten + /// marker remains in place across the later network fetch and persistence; + /// [`Self::finish_identity_load_after_persist`] clears it only after the + /// replacement identity is durably stored. + pub(crate) fn prepare_stuck_unload_cleanup_for_reload( + &self, + id: &[u8; 32], + ) -> std::result::Result { + let identifier = Identifier::from(*id); + if !self.is_identity_forgotten(&identifier)? + || self.local_identity_ids()?.contains(&identifier) + { + return Ok(false); + } + + // This also handles a marker-only unload residue whose vault/blob tail + // succeeded after an earlier sidecar failure. Cleanup is idempotent, and + // the marker deliberately remains until the fresh load is persisted. + self.cleanup_identity_after_index_removal(&identifier)?; + Ok(true) + } + + fn retry_stuck_unload_cleanup_inner( + &self, + id: &[u8; 32], + clear_forgotten_marker: bool, + ) -> std::result::Result { + let identifier = Identifier::from(*id); + if !self.is_identity_forgotten(&identifier)? + || !self.has_local_qualified_identity(&identifier)? + || self.local_identity_ids()?.contains(&identifier) + { + return Ok(false); + } + + self.cleanup_identity_after_index_removal(&identifier)?; + if clear_forgotten_marker { + forgotten_marker_delete(&self.det_kv()?, &identifier)?; + } + Ok(true) + } + + /// Remove identity-scoped residue for a forgotten identity whose recovery + /// blob is already gone. Vault cleanup already completed before that blob + /// could be purged; this finishes best-effort sidecar and metadata cleanup. + pub(crate) fn purge_forgotten_identity_residue( + &self, + identifier: &Identifier, + ) -> std::result::Result<(), TaskError> { + self.cleanup_identity_after_index_removal(identifier) + } + + /// Idempotent cleanup tail shared by normal deletion, ghost recovery, and + /// marker-only full-wipe recovery. The blob is retained whenever its vault + /// inventory could not be cleared. + fn cleanup_identity_after_index_removal( + &self, + identifier: &Identifier, + ) -> std::result::Result<(), TaskError> { + let kv = self.det_kv()?; + let backend = self.wallet_backend()?; + let id = identifier.to_buffer(); + let mut cleanup_error = None; + keep_first_unload_cleanup_error( + &mut cleanup_error, + *identifier, + backend.dashpay_clear_owner_overlays(identifier), + ); + keep_first_unload_cleanup_error( + &mut cleanup_error, + *identifier, + backend.dashpay_clear_identity_timestamps(identifier), + ); + keep_first_unload_cleanup_error( + &mut cleanup_error, + *identifier, + backend.dashpay_clear_identity_addr_map(identifier), + ); + keep_first_unload_cleanup_error( + &mut cleanup_error, + *identifier, + backend.identity_meta().delete(self.network, &id), + ); + match self.clear_identity_vault_keys(&kv, &id) { + Ok(()) => keep_first_unload_cleanup_error( + &mut cleanup_error, + *identifier, + purge_identity_scope(&kv, &id), + ), + Err(error) => { + keep_first_unload_cleanup_error(&mut cleanup_error, *identifier, Err(error)) + } + } + cleanup_error.map_or(Ok(()), Err) + } + + fn delete_local_qualified_identity_inner( + &self, + identifier: &Identifier, + remember_unload: bool, + ) -> std::result::Result<(), TaskError> { + let load_guard = self.begin_identity_cleanup_claim(identifier)?; + self.delete_local_qualified_identity_with_claim(identifier, remember_unload)?; + load_guard.loaded(); + Ok(()) + } + + /// Full-wipe deletion that returns its exclusive identity claim so the + /// caller can retain it through marker retirement and wipe completion. + pub(crate) fn delete_local_qualified_identity_retaining_claim( + &self, + identifier: &Identifier, + ) -> std::result::Result + { + let load_guard = self.begin_identity_cleanup_claim(identifier)?; + self.delete_local_qualified_identity_with_claim(identifier, false)?; + Ok(load_guard) + } + + fn begin_identity_cleanup_claim( + &self, + identifier: &Identifier, + ) -> std::result::Result + { + // The load registry provides the existing per-identity exclusive claim. + self.begin_identity_load(*identifier, None) + .map_err(|error| match error { + TaskError::IdentityLoadInProgress { identity_id } => { + TaskError::IdentityBusyWithLoad { identity_id } + } + other => other, + }) + } + + fn delete_local_qualified_identity_with_claim( + &self, + identifier: &Identifier, + remember_unload: bool, ) -> std::result::Result<(), TaskError> { let _migration_guard = self .migration_run @@ -939,9 +1247,45 @@ impl AppContext { source: Arc::new(source), }, )?; - self.clear_identity_vault_keys(&kv, &id)?; - purge_identity_scope(&kv, &id)?; - index_remove_identity(&kv, &id) + if remember_unload { + self.record_forgotten_identity(identifier)?; + } + // Drop the identity from the index BEFORE the irreversible vault-key + // clear: a fault in either of the next two steps must never leave a + // "zombie" identity that is still visible but already missing its + // keys. `clear_identity_vault_keys` must still run before + // `purge_identity_scope`, since it reads the identity blob that + // `purge_identity_scope` deletes. + if let Err(error) = index_remove_identity(&kv, &id) { + if remember_unload && let Err(rollback_error) = forgotten_marker_delete(&kv, identifier) + { + tracing::warn!( + identity_id = %identifier, + original_error = ?error, + rollback_error = ?rollback_error, + "Identity unload marker rollback failed after the removal commit failed" + ); + } + return Err(error); + } + if let Err(error) = self.cleanup_identity_after_index_removal(identifier) { + // The index entry is already gone, so without a marker this identity + // is reachable by no recovery path while its vault keys survive. A + // remembered unload wrote its marker above; every other caller gets + // one here. Never mask the cleanup error with a marker-write failure. + if !remember_unload + && let Err(marker_error) = self.record_forgotten_identity(identifier) + { + tracing::warn!( + identity_id = %identifier, + original_error = ?error, + marker_error = ?marker_error, + "Failed to record safety-net forgotten marker after cleanup failure" + ); + } + return Err(error); + } + Ok(()) } /// EAGER identity-key migration (dialog-free): move any plaintext @@ -1097,6 +1441,15 @@ impl AppContext { Ok(()) } + /// Count the scheduled votes queued for one identity on this network. + pub fn scheduled_vote_count_for_identity( + &self, + voter: &Identifier, + ) -> std::result::Result { + let kv = self.det_kv()?; + Ok(scheduled_vote_keys(&kv, &voter.to_buffer())?.len()) + } + /// Fetch every scheduled vote queued for this network from the /// wallet k/v store, across all voters in the Global voter index. pub fn get_scheduled_votes(&self) -> std::result::Result, TaskError> { @@ -1217,12 +1570,105 @@ impl AppContext { } } +#[cfg(test)] +impl AppContext { + /// Install and repair the retained blob for an interrupted unload. + pub(crate) fn install_repaired_unload_ghost_for_test( + &self, + identity: dash_sdk::platform::Identity, + secret: [u8; 32], + ) -> dash_sdk::dpp::identity::KeyID { + use crate::model::qualified_identity::PrivateKeyTarget; + use crate::model::qualified_identity::encrypted_key_storage::{KeyStorage, PrivateKeyData}; + use crate::model::qualified_identity::qualified_identity_public_key::QualifiedIdentityPublicKey; + use dash_sdk::dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; + use dash_sdk::dpp::version::PlatformVersion; + use dash_sdk::platform::IdentityPublicKey; + + let identity_id = identity.id(); + let identity_buf = identity_id.to_buffer(); + let key = IdentityPublicKey::random_key(97, Some(97), PlatformVersion::latest()); + let key_id = key.id(); + let mut private_keys = KeyStorage::default(); + private_keys.private_keys.insert( + (PrivateKeyTarget::PrivateKeyOnMainIdentity, key_id), + ( + QualifiedIdentityPublicKey::from(key), + PrivateKeyData::Clear(secret), + ), + ); + let qualified_identity = QualifiedIdentity { + identity, + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::User, + alias: Some("Interrupted unload".to_string()), + private_keys, + dpns_names: Vec::new(), + associated_wallets: BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::Active, + network: self.network, + }; + self.insert_local_qualified_identity(&qualified_identity, &None) + .expect("insert identity before faulted unload"); + + let kv = self.det_kv().expect("open identity k/v"); + let stored_before_fault = kv + .get::(DetScope::Identity(&identity_buf), IDENTITY_KEY) + .expect("read stored identity") + .expect("stored identity exists"); + kv.put( + DetScope::Identity(&identity_buf), + IDENTITY_KEY, + &StoredQualifiedIdentity { + qi_bytes: vec![0xAB; 16], + status: IdentityStatus::Active.as_u8(), + identity_type: IdentityType::User.as_tag().to_string(), + wallet_hash: None, + wallet_index: None, + }, + ) + .expect("corrupt retained identity inventory"); + + assert!(matches!( + self.unload_local_qualified_identity(&identity_id), + Err(TaskError::IdentityUnloadCleanupFailed { .. }) + )); + kv.put( + DetScope::Identity(&identity_buf), + IDENTITY_KEY, + &stored_before_fault, + ) + .expect("repair retained identity inventory"); + assert!( + !self + .local_identity_ids() + .expect("read identity index") + .contains(&identity_id), + "precondition: the interrupted unload is not indexed", + ); + assert!( + self.is_identity_forgotten(&identity_id) + .expect("read forgotten marker"), + "precondition: the interrupted unload remains marked", + ); + + key_id + } +} + #[cfg(test)] mod tests { use super::*; + use crate::backend_task::BackendTaskSuccessResult; use crate::wallet_backend::kv_test_support::InMemoryKv; use DetKv; - use std::sync::Arc; + use std::io::Write; + use std::sync::{Arc, Mutex}; fn empty_kv() -> DetKv { DetKv::from_store(Arc::new(InMemoryKv::default())) @@ -1252,6 +1698,87 @@ mod tests { index_add_identity(kv, id).unwrap(); } + #[derive(Clone, Default)] + struct SharedLog(Arc>>); + + struct SharedLogWriter(Arc>>); + + impl Write for SharedLogWriter { + fn write(&mut self, buf: &[u8]) -> std::io::Result { + self.0 + .lock() + .expect("lock captured log") + .extend_from_slice(buf); + Ok(buf.len()) + } + + fn flush(&mut self) -> std::io::Result<()> { + Ok(()) + } + } + + impl<'a> tracing_subscriber::fmt::MakeWriter<'a> for SharedLog { + type Writer = SharedLogWriter; + + fn make_writer(&'a self) -> Self::Writer { + SharedLogWriter(Arc::clone(&self.0)) + } + } + + #[test] + fn keep_first_unload_cleanup_error_logs_every_later_failure() { + let identity_id = Identifier::from([0x01; 32]); + let mut cleanup_error = None; + let captured = SharedLog::default(); + let subscriber = tracing_subscriber::fmt() + .without_time() + .with_ansi(false) + .with_max_level(tracing::Level::WARN) + .with_writer(captured.clone()) + .finish(); + + tracing::subscriber::with_default(subscriber, || { + keep_first_unload_cleanup_error( + &mut cleanup_error, + identity_id, + Err(TaskError::IdentityNotFound), + ); + keep_first_unload_cleanup_error( + &mut cleanup_error, + identity_id, + Err(TaskError::InternalSendError), + ); + keep_first_unload_cleanup_error( + &mut cleanup_error, + identity_id, + Err(TaskError::InternalSendError), + ); + }); + + match cleanup_error.expect("a cleanup error must be recorded") { + TaskError::IdentityUnloadCleanupFailed { + identity_id: id, + source, + } => { + assert_eq!(id, identity_id); + assert!( + matches!(*source, TaskError::IdentityNotFound), + "the first failure remains the primary source, got {source:?}" + ); + } + other => panic!("expected IdentityUnloadCleanupFailed, got {other:?}"), + } + let output = + String::from_utf8(captured.0.lock().expect("lock captured log").clone()).unwrap(); + assert_eq!( + output + .matches("Additional identity unload cleanup step failed") + .count(), + 2, + "the second and third failures must each be visible in logs: {output}" + ); + } + // --------------------------------------------------------------- // SEC: the redacting Debug must never print the private-key blob. // --------------------------------------------------------------- @@ -1344,82 +1871,316 @@ mod tests { } // --------------------------------------------------------------- - // Identity-type tag: writer and filter share one stable mapping. + // Forgotten-identity markers: the record that must outlive its + // identity, so automatic discovery cannot resurrect a deliberate + // unload. // --------------------------------------------------------------- - #[test] - fn identity_type_tag_round_trips_writer_to_filter() { - // The writer stores `as_tag()`; the load filters classify via - // `from_tag()`. They must agree for every variant, independent of the - // derived `Debug` representation. - for ty in [ - IdentityType::User, - IdentityType::Masternode, - IdentityType::Evonode, - ] { - assert_eq!(IdentityType::from_tag(ty.as_tag()), Some(ty)); - } - // Tags are fixed string constants, not the `Debug` output. - assert_eq!(IdentityType::User.as_tag(), "User"); - assert_eq!(IdentityType::Masternode.as_tag(), "Masternode"); - assert_eq!(IdentityType::Evonode.as_tag(), "Evonode"); - // The user / non-user split the load filters depend on. An unknown tag - // decodes to `None`, which the filters treat as non-user (voting) — - // preserving the pre-tag string-compare behaviour. - assert_eq!(IdentityType::from_tag("Bogus"), None); + fn identifier(b: u8) -> Identifier { + Identifier::from(id(b)) } - // --------------------------------------------------------------- - // Top-ups: Identity-scoped round-trip. - // --------------------------------------------------------------- - #[test] - fn top_ups_round_trip_in_identity_scope() { + fn forgotten_marker_round_trips_and_is_per_identity() { let kv = empty_kv(); - let a = id(1); - let mut map = std::collections::BTreeMap::new(); - map.insert(0u32, 100u64); - map.insert(1u32, 250u64); - kv.put(DetScope::Identity(&a), TOP_UPS_KEY, &map).unwrap(); - let got: std::collections::BTreeMap = kv - .get(DetScope::Identity(&a), TOP_UPS_KEY) - .unwrap() - .unwrap(); - assert_eq!(got, map); - } + assert!(!forgotten_marker_exists(&kv, &identifier(1)).unwrap()); - /// A legacy import carries only the entries it found in `data.db`. It must - /// union them into whatever the user has recorded since — a top-up made - /// between two migration passes is real money moved, and a replacing write - /// would erase its record. - #[test] - fn save_top_ups_merges_into_the_stored_history() { - let kv = empty_kv(); - let a = id(1); + forgotten_marker_put(&kv, &identifier(1)).unwrap(); + forgotten_marker_put(&kv, &identifier(2)).unwrap(); - // The user tops up in the new build; the entry lands at index 1. - save_top_ups_in(&kv, &a, &std::collections::BTreeMap::from([(1u32, 250u64)])).unwrap(); - // A late migration pass replays the legacy history, which knows only index 0. - save_top_ups_in(&kv, &a, &std::collections::BTreeMap::from([(0u32, 100u64)])).unwrap(); + assert!(forgotten_marker_exists(&kv, &identifier(1)).unwrap()); + assert!(forgotten_marker_exists(&kv, &identifier(2)).unwrap()); + assert!(!forgotten_marker_exists(&kv, &identifier(3)).unwrap()); - let got: std::collections::BTreeMap = kv - .get(DetScope::Identity(&a), TOP_UPS_KEY) - .unwrap() - .unwrap(); - assert_eq!( - got, - std::collections::BTreeMap::from([(0u32, 100u64), (1u32, 250u64)]), - "the entry recorded between the passes must survive the import", + forgotten_marker_delete(&kv, &identifier(1)).unwrap(); + assert!(!forgotten_marker_exists(&kv, &identifier(1)).unwrap()); + assert!( + forgotten_marker_exists(&kv, &identifier(2)).unwrap(), + "clearing one marker keeps the rest" ); } - /// On a colliding index the caller's value wins: the top-up flow writes the - /// amount it just confirmed on-chain, which is fresher than any stored copy. #[test] - fn save_top_ups_incoming_value_wins_on_a_colliding_index() { + fn forgotten_marker_writes_are_idempotent() { let kv = empty_kv(); - let a = id(1); - + forgotten_marker_put(&kv, &identifier(1)).unwrap(); + forgotten_marker_put(&kv, &identifier(1)).unwrap(); + assert_eq!( + kv.list(DetScope::Global, Some(FORGOTTEN_IDENTITY_KEY_PREFIX)) + .unwrap() + .len(), + 1 + ); + + forgotten_marker_delete(&kv, &identifier(1)).unwrap(); + forgotten_marker_delete(&kv, &identifier(1)).unwrap(); + forgotten_marker_delete(&kv, &identifier(9)).unwrap(); + assert!( + kv.list(DetScope::Global, Some(FORGOTTEN_IDENTITY_KEY_PREFIX)) + .unwrap() + .is_empty() + ); + } + + /// Each identity owns a private key, so no marker write ever reads or + /// rewrites another identity's marker. This is what makes concurrent + /// unloads safe; a shared collection would reintroduce the lost update. + #[test] + fn each_forgotten_marker_is_an_independent_key() { + let kv = empty_kv(); + forgotten_marker_put(&kv, &identifier(1)).unwrap(); + forgotten_marker_put(&kv, &identifier(2)).unwrap(); + + let keys = kv + .list(DetScope::Global, Some(FORGOTTEN_IDENTITY_KEY_PREFIX)) + .unwrap(); + assert_eq!(keys.len(), 2, "two identities must occupy two keys"); + assert!(keys.contains(&forgotten_identity_key(&identifier(1)))); + assert!(keys.contains(&forgotten_identity_key(&identifier(2)))); + + // A marker key round-trips through its own encoding. + assert_eq!( + forgotten_identity_from_key(&forgotten_identity_key(&identifier(1))), + Some(identifier(1)) + ); + // A key that merely shares the prefix is skipped, not fatal. + assert_eq!( + forgotten_identity_from_key(&format!("{FORGOTTEN_IDENTITY_KEY_PREFIX}not-base58!")), + None + ); + } + + /// The marker exists to outlive the identity it names. Storing it in the + /// identity's own scope would hand it to the upstream soft-cascade, which + /// reaps that scope exactly when the identity goes away. + #[test] + fn forgotten_marker_outlives_its_identity_scope() { + let kv = empty_kv(); + put_identity(&kv, &id(1), "User"); + forgotten_marker_put(&kv, &identifier(1)).unwrap(); + + purge_identity_scope(&kv, &id(1)).unwrap(); + index_remove_identity(&kv, &id(1)).unwrap(); + + assert!( + forgotten_marker_exists(&kv, &identifier(1)).unwrap(), + "purging every identity-scoped record must not touch the marker" + ); + assert!( + kv.list(DetScope::Identity(&id(1)), None) + .unwrap() + .is_empty(), + "the marker must not be one of the identity's own reapable slots" + ); + } + + /// Markers are partitioned by network through the store they live in, so + /// they must land in the per-network wallet k/v — a marker written to the + /// cross-network app k/v would keep the identity unloaded on every network. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn forgotten_markers_live_in_the_per_network_store() { + use crate::app::TaskResult; + use crate::context::test_support::test_app_context; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + let identity_id = Identifier::from([0x2A; 32]); + + ctx.record_forgotten_identity(&identity_id) + .expect("record marker"); + + assert_eq!( + ctx.list_forgotten_identities().expect("list markers"), + vec![identity_id] + ); + let marker_key = forgotten_identity_key(&identity_id); + assert!( + ctx.det_kv() + .expect("per-network k/v") + .get::<()>(DetScope::Global, &marker_key) + .expect("read per-network marker slot") + .is_some(), + "the marker belongs to the per-network wallet store" + ); + assert!( + ctx.app_kv() + .get::<()>(DetScope::Global, &marker_key) + .expect("read cross-network marker slot") + .is_none(), + "the cross-network app store must hold no marker" + ); + + ctx.clear_forgotten_identity_after_explicit_load(&identity_id) + .expect("clear marker"); + assert!( + ctx.list_forgotten_identities() + .expect("list cleared markers") + .is_empty() + ); + + backend.shutdown().await; + } + + /// Concurrent marker work on *different* identities must not lose an + /// update. Records and clears run interleaved against the real persister, + /// so any read-modify-write over a shared blob would drop a marker one way + /// (an unload silently forgotten, #889 back) or resurrect one the other + /// (a cleared identity kept unloaded). + #[tokio::test(flavor = "multi_thread", worker_threads = 4)] + async fn concurrent_marker_writes_for_distinct_identities_do_not_clobber() { + use crate::app::TaskResult; + use crate::context::test_support::test_app_context; + use crate::utils::egui_mpsc::SenderAsync; + + const IDENTITIES: u8 = 12; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + + // Half start marked and get cleared; half start clear and get marked. + // Both directions run at once so a lost update in either direction + // shows up as a wrong final state. + let to_record: Vec = (0..IDENTITIES).map(identifier).collect(); + let to_clear: Vec = (100..100 + IDENTITIES).map(identifier).collect(); + for identity_id in &to_clear { + ctx.record_forgotten_identity(identity_id) + .expect("seed a marker the race will clear"); + } + + // `(identity, is_clear)` work items, owned so each task can take one. + let mut work: Vec<(Identifier, bool)> = to_record.iter().map(|id| (*id, false)).collect(); + work.extend(to_clear.iter().map(|id| (*id, true))); + + let mut tasks = tokio::task::JoinSet::new(); + for (identity_id, clearing) in work { + let ctx = Arc::clone(&ctx); + tasks.spawn(async move { + if clearing { + ctx.clear_forgotten_identity_after_explicit_load(&identity_id) + .expect("concurrent marker clear"); + } else { + ctx.record_forgotten_identity(&identity_id) + .expect("concurrent marker record"); + } + }); + } + while let Some(joined) = tasks.join_next().await { + joined.expect("no marker task may panic"); + } + + for identity_id in &to_record { + assert!( + ctx.is_identity_forgotten(identity_id) + .expect("read recorded marker"), + "a concurrent record was lost for {identity_id}" + ); + } + for identity_id in &to_clear { + assert!( + !ctx.is_identity_forgotten(identity_id) + .expect("read cleared marker"), + "a concurrent clear was resurrected for {identity_id}" + ); + } + let mut listed = ctx.list_forgotten_identities().expect("list markers"); + listed.sort_unstable(); + let mut expected = to_record; + expected.sort_unstable(); + assert_eq!(listed, expected, "the surviving marker set must be exact"); + + backend.shutdown().await; + } + + // --------------------------------------------------------------- + // Identity-type tag: writer and filter share one stable mapping. + // --------------------------------------------------------------- + + #[test] + fn identity_type_tag_round_trips_writer_to_filter() { + // The writer stores `as_tag()`; the load filters classify via + // `from_tag()`. They must agree for every variant, independent of the + // derived `Debug` representation. + for ty in [ + IdentityType::User, + IdentityType::Masternode, + IdentityType::Evonode, + ] { + assert_eq!(IdentityType::from_tag(ty.as_tag()), Some(ty)); + } + // Tags are fixed string constants, not the `Debug` output. + assert_eq!(IdentityType::User.as_tag(), "User"); + assert_eq!(IdentityType::Masternode.as_tag(), "Masternode"); + assert_eq!(IdentityType::Evonode.as_tag(), "Evonode"); + // The user / non-user split the load filters depend on. An unknown tag + // decodes to `None`, which the filters treat as non-user (voting) — + // preserving the pre-tag string-compare behaviour. + assert_eq!(IdentityType::from_tag("Bogus"), None); + } + + // --------------------------------------------------------------- + // Top-ups: Identity-scoped round-trip. + // --------------------------------------------------------------- + + #[test] + fn top_ups_round_trip_in_identity_scope() { + let kv = empty_kv(); + let a = id(1); + let mut map = std::collections::BTreeMap::new(); + map.insert(0u32, 100u64); + map.insert(1u32, 250u64); + kv.put(DetScope::Identity(&a), TOP_UPS_KEY, &map).unwrap(); + let got: std::collections::BTreeMap = kv + .get(DetScope::Identity(&a), TOP_UPS_KEY) + .unwrap() + .unwrap(); + assert_eq!(got, map); + } + + /// A legacy import carries only the entries it found in `data.db`. It must + /// union them into whatever the user has recorded since — a top-up made + /// between two migration passes is real money moved, and a replacing write + /// would erase its record. + #[test] + fn save_top_ups_merges_into_the_stored_history() { + let kv = empty_kv(); + let a = id(1); + + // The user tops up in the new build; the entry lands at index 1. + save_top_ups_in(&kv, &a, &std::collections::BTreeMap::from([(1u32, 250u64)])).unwrap(); + // A late migration pass replays the legacy history, which knows only index 0. + save_top_ups_in(&kv, &a, &std::collections::BTreeMap::from([(0u32, 100u64)])).unwrap(); + + let got: std::collections::BTreeMap = kv + .get(DetScope::Identity(&a), TOP_UPS_KEY) + .unwrap() + .unwrap(); + assert_eq!( + got, + std::collections::BTreeMap::from([(0u32, 100u64), (1u32, 250u64)]), + "the entry recorded between the passes must survive the import", + ); + } + + /// On a colliding index the caller's value wins: the top-up flow writes the + /// amount it just confirmed on-chain, which is fresher than any stored copy. + #[test] + fn save_top_ups_incoming_value_wins_on_a_colliding_index() { + let kv = empty_kv(); + let a = id(1); + save_top_ups_in(&kv, &a, &std::collections::BTreeMap::from([(0u32, 100u64)])).unwrap(); save_top_ups_in(&kv, &a, &std::collections::BTreeMap::from([(0u32, 999u64)])).unwrap(); @@ -1540,6 +2301,54 @@ mod tests { assert_eq!(voters, vec![v1, v2]); } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn scheduled_vote_count_for_identity_counts_only_that_voters_queue() { + use crate::app::TaskResult; + use crate::context::test_support::test_app_context; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let target = id(0x61); + let other = id(0x62); + let kv = ctx.det_kv().expect("det kv"); + for (voter, name) in [(target, "alpha"), (target, "beta"), (other, "gamma")] { + kv.put( + DetScope::Identity(&voter), + &scheduled_vote_key(name), + &StoredScheduledVote { + voter_id: voter, + contested_name: name.to_string(), + choice: StoredVoteChoice::Lock, + unix_timestamp: 0, + executed_successfully: false, + }, + ) + .expect("store scheduled vote"); + } + + assert_eq!( + ctx.scheduled_vote_count_for_identity(&Identifier::from(target)) + .expect("count target votes"), + 2 + ); + assert_eq!( + ctx.scheduled_vote_count_for_identity(&Identifier::from(id(0x63))) + .expect("count absent voter"), + 0 + ); + + ctx.wallet_backend() + .expect("backend wired") + .shutdown() + .await; + } + #[test] fn delete_scheduled_votes_for_voter_drains_scope_and_prunes_index() { let kv = empty_kv(); @@ -1688,7 +2497,8 @@ mod tests { /// A `QualifiedIdentity` carrying one `Clear` (HIGH), one `AlwaysClear` /// (MEDIUM), and one `AtWalletDerivationPath` key. Returns the QI plus the /// `(target, key_id)` of each plaintext key for assertions. - fn qi_with_plaintext_and_derived( + fn qi_with_id_plaintext_and_derived( + identity_id: Identifier, secret_high: [u8; 32], secret_medium: [u8; 32], ) -> QualifiedIdentity { @@ -1721,8 +2531,7 @@ mod tests { }), ), ); - let identity = - Identity::create_basic_identity(Identifier::default(), pv).expect("basic identity"); + let identity = Identity::create_basic_identity(identity_id, pv).expect("basic identity"); QualifiedIdentity { identity, associated_voter_identity: None, @@ -1741,6 +2550,1192 @@ mod tests { } } + fn qi_with_plaintext_and_derived( + secret_high: [u8; 32], + secret_medium: [u8; 32], + ) -> QualifiedIdentity { + qi_with_id_plaintext_and_derived(Identifier::default(), secret_high, secret_medium) + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn identity_unload_removes_all_owned_state_and_preserves_siblings() { + use crate::app::TaskResult; + use crate::context::test_support::test_app_context; + use crate::model::dashpay::{ContactAddressIndex, ContactPrivateInfo}; + use crate::model::qualified_identity::identity_meta::IdentityMeta; + use crate::utils::egui_mpsc::SenderAsync; + use crate::wallet_backend::WalletSeedView; + use dash_sdk::dpp::platform_value::string_encoding::Encoding; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + + let target_id = Identifier::from([0x11; 32]); + let sibling_id = Identifier::from([0x22; 32]); + let contact_id = Identifier::from([0x33; 32]); + let target = qi_with_id_plaintext_and_derived(target_id, [0x41; 32], [0x42; 32]); + let sibling = qi_with_id_plaintext_and_derived(sibling_id, [0x51; 32], [0x52; 32]); + ctx.insert_local_qualified_identity(&target, &None) + .expect("insert target identity"); + ctx.insert_local_qualified_identity(&sibling, &None) + .expect("insert sibling identity"); + + let target_buf = target_id.to_buffer(); + let sibling_buf = sibling_id.to_buffer(); + let target_vault = IdentityKeyView::new(backend.secret_store(), target_buf); + let sibling_vault = IdentityKeyView::new(backend.secret_store(), sibling_buf); + assert!( + target_vault + .get(&PrivateKeyTarget::PrivateKeyOnMainIdentity, 1) + .expect("read target key") + .is_some(), + "target key must exist before removal" + ); + + backend + .dashpay_set_private_info( + &target_id, + &contact_id, + &ContactPrivateInfo { + nickname: "target contact".into(), + notes: "target note".into(), + is_hidden: false, + }, + ) + .expect("seed target private memo"); + backend + .dashpay_set_address_index( + &target_id, + &contact_id, + &ContactAddressIndex { + owner_identity_id: target_buf.to_vec(), + contact_identity_id: contact_id.to_buffer().to_vec(), + next_send_index: 3, + highest_receive_index: 2, + bloom_registered_count: 1, + }, + ) + .expect("seed target address index"); + backend + .dashpay_mark_blocked(&target_id, &contact_id) + .expect("seed target blocked marker"); + backend + .dashpay_mark_declined(&target_id, &contact_id) + .expect("seed target declined marker"); + backend + .dashpay_mark_withdrawn(&target_id, &contact_id) + .expect("seed target withdrawn marker"); + let contact_b58 = contact_id.to_string(Encoding::Base58); + backend + .kv() + .put::<()>( + DetScope::Identity(&target_buf), + &format!("det:dashpay:request_action:decline:{contact_b58}"), + &(), + ) + .expect("seed target request-action journal"); + + backend + .dashpay_set_private_info( + &sibling_id, + &contact_id, + &ContactPrivateInfo { + nickname: "sibling contact".into(), + notes: "sibling note".into(), + is_hidden: true, + }, + ) + .expect("seed sibling private memo"); + backend + .dashpay_mark_blocked(&sibling_id, &contact_id) + .expect("seed sibling blocked marker"); + + backend + .dashpay_set_timestamps(&target_id, 11, 12) + .expect("seed target timestamps"); + backend + .dashpay_set_timestamps(&sibling_id, 21, 22) + .expect("seed sibling timestamps"); + backend + .dashpay_set_address_mapping(&target_id, "target-address-1", &contact_id, 1) + .expect("seed first target address mapping"); + backend + .dashpay_set_address_mapping(&target_id, "target-address-2", &contact_id, 2) + .expect("seed second target address mapping"); + backend + .dashpay_set_address_mapping(&sibling_id, "sibling-address", &contact_id, 3) + .expect("seed sibling address mapping"); + backend + .identity_meta() + .set( + ctx.network(), + &target_buf, + &IdentityMeta { + password_hint: Some("target hint".into()), + }, + ) + .expect("seed target identity metadata"); + + let wallet_seed_hash = [0x77; 32]; + let wallet_seed = [0x88; 64]; + WalletSeedView::new(backend.secret_store()) + .set_raw(&wallet_seed_hash, &wallet_seed) + .expect("seed wallet secret"); + + let kv = backend.kv(); + assert_eq!( + kv.list(DetScope::Identity(&target_buf), Some("det:dashpay:")) + .expect("list target overlays") + .len(), + 6, + "all six owner-overlay families must be seeded" + ); + + ctx.delete_local_qualified_identity(&target_id) + .expect("remove target identity"); + + assert!( + ctx.get_local_qualified_identity(&target_id) + .expect("read removed identity") + .is_none(), + "target identity blob must be removed" + ); + assert_eq!( + ctx.local_identity_ids().expect("read identity index"), + vec![sibling_id], + "identity index must retain only the sibling" + ); + assert!( + target_vault + .get(&PrivateKeyTarget::PrivateKeyOnMainIdentity, 1) + .expect("read removed target key") + .is_none(), + "target vault keys must be removed" + ); + assert!( + sibling_vault + .get(&PrivateKeyTarget::PrivateKeyOnMainIdentity, 1) + .expect("read sibling key") + .is_some(), + "sibling vault keys must survive" + ); + assert!( + kv.list(DetScope::Identity(&target_buf), Some("det:dashpay:")) + .expect("list removed target overlays") + .is_empty(), + "all target owner overlays must be removed" + ); + assert_eq!( + kv.list(DetScope::Identity(&sibling_buf), Some("det:dashpay:")) + .expect("list sibling overlays") + .len(), + 2, + "sibling owner overlays must survive" + ); + + let target_timestamps = format!( + "det:dashpay:timestamps:{}", + target_id.to_string(Encoding::Base58) + ); + let sibling_timestamps = format!( + "det:dashpay:timestamps:{}", + sibling_id.to_string(Encoding::Base58) + ); + assert!( + kv.get::<(i64, i64)>(DetScope::Global, &target_timestamps) + .expect("read target timestamps") + .is_none(), + "target entity timestamps must be removed" + ); + assert_eq!( + kv.get::<(i64, i64)>(DetScope::Global, &sibling_timestamps) + .expect("read sibling timestamps"), + Some((21, 22)), + "sibling entity timestamps must survive" + ); + assert!( + backend + .dashpay_get_address_mapping(&target_id, "target-address-1") + .expect("read first removed target address mapping") + .is_none(), + "the first target address mapping must be removed" + ); + assert!( + backend + .dashpay_get_address_mapping(&target_id, "target-address-2") + .expect("read second removed target address mapping") + .is_none(), + "the second target address mapping must be removed" + ); + assert_eq!( + backend + .dashpay_get_address_mapping(&sibling_id, "sibling-address") + .expect("read sibling address mapping"), + Some((contact_id, 3)), + "the sibling address mapping must survive" + ); + assert!( + backend + .identity_meta() + .get(ctx.network(), &target_buf) + .is_none(), + "target identity metadata must be removed" + ); + assert_eq!( + WalletSeedView::new(backend.secret_store()) + .get_raw(&wallet_seed_hash) + .expect("read wallet seed") + .as_deref(), + Some(&wallet_seed), + "wallet seed.raw.v1 must survive identity removal" + ); + + backend.shutdown().await; + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn identity_unload_reports_failure_when_wallet_backend_is_unwired() { + use crate::app::TaskResult; + use crate::context::test_support::test_app_context; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + let target_id = Identifier::from([0xB1; 32]); + let target = qi_with_id_plaintext_and_derived(target_id, [0xB2; 32], [0xB3; 32]); + ctx.insert_local_qualified_identity(&target, &None) + .expect("insert target identity"); + + ctx.wallet_backend.store(None); + + assert!( + matches!( + ctx.unload_local_qualified_identity(&target_id), + Err(TaskError::WalletBackendNotYetWired) + ), + "unload must not report success when its cleanup backend is unavailable" + ); + assert!( + backend + .kv() + .get::( + DetScope::Identity(&target_id.to_buffer()), + IDENTITY_KEY, + ) + .expect("read retained identity") + .is_some(), + "an unload rejected before its commit point must retain the identity" + ); + + backend.shutdown().await; + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn failed_unload_before_commit_preserves_dashpay_overlays() { + use crate::app::TaskResult; + use crate::context::test_support::test_app_context; + use crate::model::dashpay::ContactPrivateInfo; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + let target_id = Identifier::from([0xB4; 32]); + let contact_id = Identifier::from([0xB5; 32]); + let target = qi_with_id_plaintext_and_derived(target_id, [0xB6; 32], [0xB7; 32]); + ctx.insert_local_qualified_identity(&target, &Some(([0xB8; 32], 1))) + .expect("insert target identity"); + backend + .dashpay_set_private_info( + &target_id, + &contact_id, + &ContactPrivateInfo { + nickname: "retained contact".into(), + notes: "retained note".into(), + is_hidden: false, + }, + ) + .expect("seed owner overlay"); + + let persister_path = backend.spv_storage_dir().join("platform-wallet.sqlite"); + let fault_connection = + rusqlite::Connection::open(&persister_path).expect("open persister second handle"); + fault_connection + .execute_batch( + "CREATE TRIGGER fail_identity_index_commit + BEFORE INSERT ON meta_global + WHEN NEW.key = 'det:identity_index:v1' + BEGIN + SELECT RAISE(FAIL, 'injected identity-index failure'); + END;", + ) + .expect("install identity-index trigger"); + + assert!( + matches!( + ctx.unload_local_qualified_identity(&target_id), + Err(TaskError::IdentityStorage { .. }) + ), + "the failed commit must surface its identity-storage error" + ); + assert!( + backend + .dashpay_get_private_info(&target_id, &contact_id) + .expect("read retained owner overlay") + .is_some(), + "cleanup must not destroy DashPay overlays before the unload commits" + ); + + fault_connection + .execute_batch("DROP TRIGGER fail_identity_index_commit;") + .expect("remove identity-index trigger"); + backend.shutdown().await; + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn rollback_failure_does_not_replace_original_unload_error() { + use crate::app::TaskResult; + use crate::context::test_support::test_app_context; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + let target_id = Identifier::from([0xB9; 32]); + let target = qi_with_id_plaintext_and_derived(target_id, [0xBA; 32], [0xBB; 32]); + ctx.insert_local_qualified_identity(&target, &Some(([0xBC; 32], 2))) + .expect("insert target identity"); + + let fault_connection = + crate::context::test_support::open_persister_fault_connection(&backend); + fault_connection + .execute_batch( + "CREATE TRIGGER fail_identity_index_commit + BEFORE INSERT ON meta_global + WHEN NEW.key = 'det:identity_index:v1' + BEGIN + SELECT RAISE(FAIL, 'injected identity-index failure'); + END;", + ) + .expect("install identity-index trigger"); + // Recording the marker is a write and rolling it back a delete, so the + // two halves of the unload fault independently: the marker still gets + // recorded, only its rollback fails. + fault_connection + .execute_batch( + "CREATE TRIGGER fail_forgotten_marker_rollback + BEFORE DELETE ON meta_global + WHEN OLD.key LIKE 'det:forgotten_identity:%' + BEGIN + SELECT RAISE(FAIL, 'injected marker-rollback failure'); + END;", + ) + .expect("install marker-rollback trigger"); + + let error = ctx + .unload_local_qualified_identity(&target_id) + .expect_err("the unload commit must fail"); + assert!( + matches!(error, TaskError::IdentityStorage { .. }), + "the original commit failure must remain primary, got {error:?}" + ); + assert!( + ctx.is_identity_forgotten(&target_id) + .expect("read marker after the failed rollback"), + "a failed rollback leaves the marker the unload had already recorded" + ); + + fault_connection + .execute_batch( + "DROP TRIGGER fail_identity_index_commit; + DROP TRIGGER fail_forgotten_marker_rollback;", + ) + .expect("remove injected triggers"); + backend.shutdown().await; + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn identity_unload_continues_cleanup_and_wipes_vault_after_overlay_failure() { + use crate::app::TaskResult; + use crate::context::test_support::test_app_context; + use crate::model::dashpay::ContactPrivateInfo; + use crate::model::qualified_identity::identity_meta::IdentityMeta; + use crate::utils::egui_mpsc::SenderAsync; + use dash_sdk::dpp::platform_value::string_encoding::Encoding; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + + let target_id = Identifier::from([0xA1; 32]); + let contact_id = Identifier::from([0xA2; 32]); + let target = qi_with_id_plaintext_and_derived(target_id, [0xA3; 32], [0xA4; 32]); + ctx.insert_local_qualified_identity(&target, &None) + .expect("insert target identity"); + + let target_buf = target_id.to_buffer(); + let target_vault = IdentityKeyView::new(backend.secret_store(), target_buf); + assert!( + target_vault + .get(&PrivateKeyTarget::PrivateKeyOnMainIdentity, 1) + .expect("read target key") + .is_some(), + "target key must exist before removal" + ); + + backend + .dashpay_set_private_info( + &target_id, + &contact_id, + &ContactPrivateInfo { + nickname: "target contact".into(), + notes: "target note".into(), + is_hidden: false, + }, + ) + .expect("seed target owner overlay"); + backend + .dashpay_set_timestamps(&target_id, 11, 12) + .expect("seed target timestamps"); + backend + .dashpay_set_address_mapping(&target_id, "target-address", &contact_id, 1) + .expect("seed target address mapping"); + backend + .identity_meta() + .set( + ctx.network(), + &target_buf, + &IdentityMeta { + password_hint: Some("target hint".into()), + }, + ) + .expect("seed target identity metadata"); + + let kv = backend.kv(); + let overlay_keys = kv + .list( + DetScope::Identity(&target_buf), + Some("det:dashpay:private:"), + ) + .expect("list target owner overlays"); + let [overlay_key] = overlay_keys.as_slice() else { + panic!("exactly one target owner overlay must be seeded"); + }; + let timestamps_key = format!( + "det:dashpay:timestamps:{}", + target_id.to_string(Encoding::Base58) + ); + + let persister_path = backend.spv_storage_dir().join("platform-wallet.sqlite"); + let fault_connection = + rusqlite::Connection::open(&persister_path).expect("open persister second handle"); + let trigger_name = "fail_target_owner_overlay_delete"; + let trigger_sql = format!( + "CREATE TRIGGER {trigger_name} + BEFORE DELETE ON meta_identity + WHEN OLD.identity_id = X'{}' AND OLD.key = '{}' + BEGIN + SELECT RAISE(FAIL, 'injected owner overlay delete failure'); + END;", + hex::encode(target_buf), + overlay_key.replace('\'', "''"), + ); + fault_connection + .execute_batch(&trigger_sql) + .expect("install owner-overlay delete trigger"); + + match ctx.unload_local_qualified_identity(&target_id) { + Err(TaskError::IdentityUnloadCleanupFailed { + identity_id, + source, + }) => { + assert_eq!(identity_id, target_id); + assert!( + matches!(*source, TaskError::DashpaySidecarStorage { .. }), + "the first cleanup failure must preserve its DashPay source" + ); + } + other => panic!("expected identity-unload cleanup failure, got {other:?}"), + } + + assert!( + kv.get::(DetScope::Identity(&target_buf), overlay_key) + .expect("read retained owner overlay") + .is_some(), + "the targeted owner overlay must survive its injected delete failure" + ); + assert!( + kv.get::<(i64, i64)>(DetScope::Global, ×tamps_key) + .expect("read target timestamps") + .is_none(), + "timestamp cleanup must continue after the owner-overlay failure" + ); + assert!( + backend + .dashpay_get_address_mapping(&target_id, "target-address") + .expect("read target address mapping") + .is_none(), + "address-map cleanup must continue after the owner-overlay failure" + ); + assert!( + backend + .identity_meta() + .get(ctx.network(), &target_buf) + .is_none(), + "identity-metadata cleanup must continue after the owner-overlay failure" + ); + assert!( + target_vault + .get(&PrivateKeyTarget::PrivateKeyOnMainIdentity, 1) + .expect("read removed target key") + .is_none(), + "target vault keys must be removed despite the cleanup failure" + ); + assert!( + ctx.is_identity_forgotten(&target_id) + .expect("read marker after partial cleanup"), + "the partial unload must remain discoverable after its blob is purged" + ); + assert!( + !ctx.has_local_qualified_identity(&target_id) + .expect("read blob after partial cleanup"), + "vault cleanup success allows the recovery blob to be purged" + ); + + fault_connection + .execute_batch(&format!("DROP TRIGGER {trigger_name};")) + .expect("remove owner-overlay delete trigger"); + ctx.clear_network_database() + .await + .expect("full wipe must retry marker-only identity residue"); + assert!( + kv.get::(DetScope::Identity(&target_buf), overlay_key) + .expect("read owner overlay after full wipe") + .is_none(), + "the full wipe must remove owner residue even when no blob remains" + ); + assert!( + !ctx.is_identity_forgotten(&target_id) + .expect("read marker after full wipe"), + "the marker must clear only after its residue cleanup succeeds" + ); + backend.shutdown().await; + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn identity_unload_respects_an_in_flight_load_claim() { + use crate::app::TaskResult; + use crate::context::test_support::test_app_context; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + let target_id = Identifier::from([0x71; 32]); + let target = qi_with_id_plaintext_and_derived(target_id, [0x72; 32], [0x73; 32]); + ctx.insert_local_qualified_identity(&target, &None) + .expect("insert target identity"); + let load_guard = ctx + .begin_identity_load(target_id, None) + .expect("claim target for an in-flight load"); + + assert!( + matches!( + ctx.delete_local_qualified_identity(&target_id), + Err(TaskError::IdentityBusyWithLoad { identity_id }) if identity_id == target_id + ), + "deletion must surface an unload-specific error without racing the load" + ); + assert!( + ctx.get_local_qualified_identity(&target_id) + .expect("read retained identity") + .is_some(), + "a rejected deletion must not mutate the identity" + ); + + drop(load_guard); + backend.shutdown().await; + } + + /// A failure partway through deletion must never leave a "zombie" identity: + /// still indexed (so still visible in the UI) with its vault keys already + /// gone. `index_remove_identity` must run before the irreversible vault-key + /// clear, so a fault anywhere from that point on still leaves the identity + /// hidden rather than visibly broken. + /// + /// The fault is a corrupted stored blob (bad `qi_bytes`) written directly + /// after a real insert — a real, naturally-occurring failure of + /// `clear_identity_vault_keys`'s `decode_stored_identity` call, not a + /// simulated one — driving the actual `delete_local_qualified_identity` + /// entry point end to end. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn deletion_fault_after_index_removal_leaves_no_visible_zombie() { + use crate::app::TaskResult; + use crate::context::test_support::test_app_context; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + + let target_id = Identifier::from([0x91; 32]); + let target = qi_with_id_plaintext_and_derived(target_id, [0x92; 32], [0x93; 32]); + ctx.insert_local_qualified_identity(&target, &None) + .expect("insert target identity"); + assert!( + ctx.local_identity_ids() + .expect("read index") + .contains(&target_id), + "the identity must be indexed before deletion" + ); + + // Corrupt the stored blob in place: the index and wallet association + // are untouched, but `clear_identity_vault_keys`'s decode will fail. + let id_buf = target_id.to_buffer(); + let kv = ctx.det_kv().expect("det kv"); + kv.put(DetScope::Identity(&id_buf), IDENTITY_KEY, &stored("User")) + .expect("corrupt the stored blob"); + + let result = ctx.delete_local_qualified_identity(&target_id); + assert!( + result.is_err(), + "the corrupted blob must surface as an error" + ); + + assert!( + !ctx.local_identity_ids() + .expect("read index") + .contains(&target_id), + "the identity must already be hidden from the index even though a \ + later cleanup step failed — never a visible entry with its keys \ + already gone" + ); + + backend.shutdown().await; + } + + /// A failed vault-key clear must retain the identity blob that inventories + /// its labels, allowing a later retry to finish without orphaning the key. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn deletion_fault_after_index_removal_must_not_permanently_orphan_vault_key() { + use crate::app::TaskResult; + use crate::context::test_support::test_app_context; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + + let target_id = Identifier::from([0xA1; 32]); + let target = qi_with_id_plaintext_and_derived(target_id, [0xA2; 32], [0xA3; 32]); + ctx.insert_local_qualified_identity(&target, &None) + .expect("insert target identity"); + + let target_buf = target_id.to_buffer(); + let target_vault = IdentityKeyView::new(backend.secret_store(), target_buf); + assert!( + target_vault + .get(&PrivateKeyTarget::PrivateKeyOnMainIdentity, 1) + .expect("read target key before deletion") + .is_some(), + "the vault key must exist before the faulted deletion" + ); + + let id_buf = target_id.to_buffer(); + let kv = ctx.det_kv().expect("det kv"); + let stored_before_fault = kv + .get::(DetScope::Identity(&id_buf), IDENTITY_KEY) + .expect("read stored identity") + .expect("stored identity exists"); + // Corrupt the stored blob in place so the first clear cannot decode + // the inventory of vault labels. + kv.put(DetScope::Identity(&id_buf), IDENTITY_KEY, &stored("User")) + .expect("corrupt the stored blob"); + + assert!(matches!( + ctx.unload_local_qualified_identity(&target_id), + Err(TaskError::IdentityUnloadCleanupFailed { .. }) + )); + assert!( + !ctx.local_identity_ids() + .expect("read index") + .contains(&target_id), + "precondition: the identity is already hidden from the index" + ); + assert!( + kv.get::(DetScope::Identity(&id_buf), IDENTITY_KEY) + .expect("read retained inventory") + .is_some(), + "a vault-clear failure must retain the only on-disk inventory of key labels" + ); + assert!( + ctx.is_identity_forgotten(&target_id) + .expect("read forgotten marker"), + "the interrupted unload must remain discoverable for retry" + ); + assert!(matches!( + ctx.retry_stuck_unload_cleanup(&id_buf), + Err(TaskError::IdentityUnloadCleanupFailed { .. }) + )); + assert!( + ctx.has_local_qualified_identity(&target_id) + .expect("read retained identity after failed retry"), + "a failed retry must preserve the identity blob" + ); + assert!( + ctx.is_identity_forgotten(&target_id) + .expect("read retained marker after failed retry"), + "a failed retry must preserve the forgotten marker" + ); + + kv.put( + DetScope::Identity(&id_buf), + IDENTITY_KEY, + &stored_before_fault, + ) + .expect("restore stored identity"); + assert!( + ctx.retry_stuck_unload_cleanup(&id_buf) + .expect("retry repaired cleanup"), + "repairing the inventory must let the retry finish" + ); + + assert!( + target_vault + .get(&PrivateKeyTarget::PrivateKeyOnMainIdentity, 1) + .expect("read target key after retry") + .is_none(), + "the retained inventory must let a retry delete the vault key" + ); + assert!( + !ctx.has_local_qualified_identity(&target_id) + .expect("read identity after successful retry"), + "a successful retry must purge the retained blob" + ); + assert!( + !ctx.is_identity_forgotten(&target_id) + .expect("read marker after successful retry"), + "a successful retry must clear the forgotten marker" + ); + + backend.shutdown().await; + } + + /// A plain delete records no forgotten marker up front, so a cleanup fault + /// after the index removal would leave the identity reachable by no recovery + /// path — not the index, not the marker sweeps — while its vault keys + /// survive on disk. The delete must write a safety-net marker on that path. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn delete_without_remembered_unload_records_a_safety_net_marker_on_cleanup_failure() { + use crate::app::TaskResult; + use crate::context::test_support::test_app_context; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + + let target_id = Identifier::from([0xB1; 32]); + let target = qi_with_id_plaintext_and_derived(target_id, [0xB2; 32], [0xB3; 32]); + ctx.insert_local_qualified_identity(&target, &None) + .expect("insert target identity"); + + let id_buf = target_id.to_buffer(); + let target_vault = IdentityKeyView::new(backend.secret_store(), id_buf); + let kv = ctx.det_kv().expect("det kv"); + let stored_before_fault = kv + .get::(DetScope::Identity(&id_buf), IDENTITY_KEY) + .expect("read stored identity") + .expect("stored identity exists"); + // Corrupt the stored blob so the vault-key clear cannot decode the + // inventory of labels and the cleanup tail fails. + kv.put(DetScope::Identity(&id_buf), IDENTITY_KEY, &stored("User")) + .expect("corrupt the stored blob"); + + assert!(matches!( + ctx.delete_local_qualified_identity(&target_id), + Err(TaskError::IdentityUnloadCleanupFailed { .. }) + )); + assert!( + !ctx.local_identity_ids() + .expect("read index") + .contains(&target_id), + "precondition: the identity is already out of the index" + ); + assert!( + ctx.is_identity_forgotten(&target_id) + .expect("read forgotten marker"), + "a faulted delete must leave a marker, or no recovery path can find \ + the identity again" + ); + + kv.put( + DetScope::Identity(&id_buf), + IDENTITY_KEY, + &stored_before_fault, + ) + .expect("restore stored identity"); + assert!( + ctx.retry_stuck_unload_cleanup(&id_buf) + .expect("retry repaired cleanup"), + "the safety-net marker must make the residue recoverable" + ); + assert!( + target_vault + .get(&PrivateKeyTarget::PrivateKeyOnMainIdentity, 1) + .expect("read target key after retry") + .is_none(), + "the recovered retry must delete the vault key the fault left behind" + ); + assert!( + !ctx.is_identity_forgotten(&target_id) + .expect("read marker after successful retry"), + "a successful retry must clear the safety-net marker" + ); + + backend.shutdown().await; + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn stuck_unload_retry_ignores_non_ghost_states() { + use crate::app::TaskResult; + use crate::context::test_support::test_app_context; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + + let stored_id = Identifier::from([0xA4; 32]); + let stored_identity = qi_with_id_plaintext_and_derived(stored_id, [0xA5; 32], [0xA6; 32]); + ctx.insert_local_qualified_identity(&stored_identity, &None) + .expect("insert non-forgotten identity"); + assert!( + !ctx.retry_stuck_unload_cleanup(&stored_id.to_buffer()) + .expect("check non-forgotten identity"), + "an identity that was never forgotten is not a cleanup ghost" + ); + assert!( + ctx.has_local_qualified_identity(&stored_id) + .expect("read non-forgotten identity"), + "the non-forgotten identity must remain stored" + ); + + let purged_id = Identifier::from([0xA7; 32]); + ctx.record_forgotten_identity(&purged_id) + .expect("record marker without a blob"); + assert!( + !ctx.retry_stuck_unload_cleanup(&purged_id.to_buffer()) + .expect("check marker without blob"), + "a forgotten identity with no residual blob needs no retry" + ); + assert!( + ctx.is_identity_forgotten(&purged_id) + .expect("read marker without blob"), + "normal marker lifecycle remains the caller's responsibility" + ); + + backend.shutdown().await; + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn reject_if_exists_retries_a_repaired_unload_ghost() { + use crate::app::TaskResult; + use crate::backend_task::identity::{IdentityInputToLoad, IdentityLoadMode, IdentityTask}; + use crate::context::test_support::test_app_context; + use crate::model::secret::Secret; + use crate::utils::egui_mpsc::SenderAsync; + use dash_sdk::SdkBuilder; + use dash_sdk::dpp::platform_value::Value; + use dash_sdk::dpp::platform_value::string_encoding::Encoding; + use dash_sdk::dpp::version::PlatformVersion; + use dash_sdk::drive::query::{SelectProjection, WhereClause, WhereOperator}; + use dash_sdk::platform::{DocumentQuery, Identifier, Identity}; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + + let target_id = Identifier::from([0xA8; 32]); + let target = qi_with_id_plaintext_and_derived(target_id, [0xA9; 32], [0xAA; 32]); + ctx.insert_local_qualified_identity(&target, &None) + .expect("insert target identity"); + let id_buf = target_id.to_buffer(); + let target_vault = IdentityKeyView::new(backend.secret_store(), id_buf); + assert!( + target_vault + .get(&PrivateKeyTarget::PrivateKeyOnMainIdentity, 1) + .expect("read old key before ghost recovery") + .is_some(), + "precondition: the interrupted unload retains its old vault key", + ); + let kv = ctx.det_kv().expect("det kv"); + let stored_before_fault = kv + .get::(DetScope::Identity(&id_buf), IDENTITY_KEY) + .expect("read stored identity") + .expect("stored identity exists"); + kv.put(DetScope::Identity(&id_buf), IDENTITY_KEY, &stored("User")) + .expect("corrupt the stored blob"); + assert!(matches!( + ctx.unload_local_qualified_identity(&target_id), + Err(TaskError::IdentityUnloadCleanupFailed { .. }) + )); + let failed_wipe = ctx.clear_network_database().await; + assert!( + matches!( + failed_wipe, + Err(TaskError::WalletDataClearIncomplete { .. }) + ), + "an unreadable ghost must make the full wipe report incomplete" + ); + assert!( + ctx.has_local_qualified_identity(&target_id) + .expect("read retained ghost after failed wipe"), + "a failed full-wipe retry must preserve the ghost blob" + ); + assert!( + ctx.is_identity_forgotten(&target_id) + .expect("read retained marker after failed wipe"), + "a failed full-wipe retry must preserve the forgotten marker" + ); + kv.put( + DetScope::Identity(&id_buf), + IDENTITY_KEY, + &stored_before_fault, + ) + .expect("restore stored identity"); + + let input = IdentityInputToLoad { + identity_id_input: target_id.to_string(Encoding::Hex), + identity_type: IdentityType::User, + alias_input: String::new(), + voting_private_key_input: Secret::new(""), + owner_private_key_input: Secret::new(""), + payout_address_private_key_input: Secret::new(""), + keys_input: vec![], + derive_keys_from_wallets: false, + selected_wallet_seed_hash: None, + encryption_password: None, + load_mode: IdentityLoadMode::RejectIfExists, + load_token: None, + }; + let (task_tx, _task_rx) = tokio::sync::mpsc::channel::(32); + let task_sender = SenderAsync::new(task_tx, ctx.egui_ctx().clone()); + let mut missing_sdk = SdkBuilder::new_mock() + .with_version(PlatformVersion::latest()) + .build() + .expect("build pinned mock SDK"); + missing_sdk + .mock() + .expect_fetch(target_id, None::) + .await + .expect("mock missing identity fetch"); + let failed_result = ctx + .run_identity_task( + IdentityTask::LoadIdentity(input.clone()), + &missing_sdk, + task_sender.clone(), + ) + .await; + assert!( + matches!(failed_result, Err(TaskError::IdentityNotFound)), + "a missing network identity must fail the reload: {failed_result:?}" + ); + assert!( + ctx.is_identity_forgotten(&target_id) + .expect("read marker after failed reload"), + "a failed reload must preserve the user's unload marker" + ); + assert!( + !ctx.has_local_qualified_identity(&target_id) + .expect("read slot after failed reload"), + "the repaired ghost blob should remain purged" + ); + assert!( + target_vault + .get(&PrivateKeyTarget::PrivateKeyOnMainIdentity, 1) + .expect("read old key after ghost recovery") + .is_none(), + "ghost recovery must clear the old vault key before any replacement is written", + ); + + let mut sdk = SdkBuilder::new_mock() + .with_version(PlatformVersion::latest()) + .build() + .expect("build pinned mock SDK"); + sdk.mock() + .expect_fetch(target_id, Some(target.identity.clone())) + .await + .expect("mock identity fetch"); + let dpns_query = DocumentQuery { + select: SelectProjection::documents(), + data_contract: ctx.dpns_contract.clone(), + document_type_name: "domain".to_string(), + where_clauses: vec![WhereClause { + field: "records.identity".to_string(), + operator: WhereOperator::Equal, + value: Value::Identifier(target_id.into()), + }], + group_by: Vec::new(), + having: Vec::new(), + order_by_clauses: vec![], + limit: 100, + start: None, + }; + sdk.mock() + .expect_fetch_many( + dpns_query, + Some(dash_sdk::query_types::Documents::default()), + ) + .await + .expect("mock DPNS fetch"); + let result = ctx + .run_identity_task(IdentityTask::LoadIdentity(input), &sdk, task_sender) + .await; + + assert!( + matches!( + result, + Ok(BackendTaskSuccessResult::LoadedIdentity(ref identity)) + if identity.identity.id() == target_id + ), + "a repaired ghost must load successfully: {result:?}" + ); + assert!( + ctx.has_local_qualified_identity(&target_id) + .expect("read identity after retry"), + "the fresh identity must be stored after the ghost is purged" + ); + assert!( + !ctx.is_identity_forgotten(&target_id) + .expect("read marker after retry"), + "the fresh load must clear the recovered ghost marker" + ); + + backend.shutdown().await; + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn clear_network_database_purges_a_repaired_unload_ghost() { + use crate::app::TaskResult; + use crate::context::test_support::test_app_context; + use crate::utils::egui_mpsc::SenderAsync; + + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let (tx, _rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + let backend = ctx.wallet_backend().expect("wallet backend"); + + let target_id = Identifier::from([0xAB; 32]); + let target = qi_with_id_plaintext_and_derived(target_id, [0xAC; 32], [0xAD; 32]); + ctx.insert_local_qualified_identity(&target, &None) + .expect("insert target identity"); + let id_buf = target_id.to_buffer(); + let target_vault = IdentityKeyView::new(backend.secret_store(), id_buf); + let kv = ctx.det_kv().expect("det kv"); + let stored_before_fault = kv + .get::(DetScope::Identity(&id_buf), IDENTITY_KEY) + .expect("read stored identity") + .expect("stored identity exists"); + kv.put(DetScope::Identity(&id_buf), IDENTITY_KEY, &stored("User")) + .expect("corrupt the stored blob"); + assert!(matches!( + ctx.unload_local_qualified_identity(&target_id), + Err(TaskError::IdentityUnloadCleanupFailed { .. }) + )); + kv.put( + DetScope::Identity(&id_buf), + IDENTITY_KEY, + &stored_before_fault, + ) + .expect("restore stored identity"); + + ctx.clear_network_database() + .await + .expect("full wipe must recover and purge the ghost"); + + assert!( + !ctx.has_local_qualified_identity(&target_id) + .expect("read identity after full wipe"), + "the full wipe must purge the ghost blob" + ); + assert!( + !ctx.is_identity_forgotten(&target_id) + .expect("read marker after full wipe"), + "the full wipe must clear the recovered ghost marker" + ); + assert!( + target_vault + .get(&PrivateKeyTarget::PrivateKeyOnMainIdentity, 1) + .expect("read vault after full wipe") + .is_none(), + "the full wipe must remove the ghost's vault key" + ); + + backend.shutdown().await; + } + /// Load-path migration — `migrate_keystore_to_vault` content-detects Clear/AlwaysClear, /// stores them in the vault FIRST, then rewrites the blob to InVault. /// Asserts: vault-first (the raw bytes are present), the wallet-derived key diff --git a/src/context/identity_load_registry.rs b/src/context/identity_load_registry.rs index d3d0a5e72..f784bb5c3 100644 --- a/src/context/identity_load_registry.rs +++ b/src/context/identity_load_registry.rs @@ -14,12 +14,14 @@ //! persisted by a load that errored. //! //! So each load reports its own phase here, and the registry is the single source -//! of truth for it. Loads are dispatched from several places — the Masternodes -//! form, Add Existing, the detail screen, MCP tools — so a record is stamped with -//! a [`IdentityLoadToken`] identifying the one load it belongs to, and every write -//! checks that stamp first. Without it a later submission could erase a running -//! load's record, and the two loads would then race each other's storage writes, -//! or publish each other's outcome. +//! of truth for it. Identity deletion and deliberate discovery skips also finish +//! through the same phase while holding the identity's exclusive claim. Loads are +//! dispatched from several places — the Masternodes form, Add Existing, the +//! detail screen, MCP tools — so a record is stamped with a [`IdentityLoadToken`] +//! identifying the one load it belongs to, and every write checks that stamp +//! first. Without it a later submission could erase a running load's record, and +//! the two loads would then race each other's storage writes, or publish each +//! other's outcome. //! //! The records also give a load exclusive use of its identity for its whole //! check → fetch → insert → seal span, which is what makes @@ -45,7 +47,7 @@ pub enum IdentityLoadPhase { Submitted, /// Running: the task holds this identity's exclusive claim. Running, - /// Finished, fully applied — the node is stored, with its keys as requested. + /// Finished and fully applied: stored, deliberately skipped, or deleted. Loaded, /// Finished with an error. The node may still have been persisted: the insert /// precedes the key seal, and a failed seal leaves the insert behind. Anything @@ -288,6 +290,19 @@ impl AppContext { .filter(|record| record.token == token) .map(|record| record.phase) } + + #[cfg(test)] + pub(crate) fn latest_identity_load_phase( + &self, + identity_id: &Identifier, + ) -> Option { + self.identity_loads + .lock() + .unwrap_or_else(|e| e.into_inner()) + .records + .get(identity_id) + .map(|record| record.phase) + } } #[cfg(test)] diff --git a/src/context/test_support.rs b/src/context/test_support.rs index fb6cd0bf3..504120527 100644 --- a/src/context/test_support.rs +++ b/src/context/test_support.rs @@ -40,3 +40,26 @@ pub(crate) fn test_app_context_with_kv(dir: &Path, app_kv: Arc) -> Arc rusqlite::Connection { + let connection = + rusqlite::Connection::open(backend.spv_storage_dir().join("platform-wallet.sqlite")) + .expect("open persister second handle"); + connection + .busy_timeout(PERSISTER_BUSY_TIMEOUT) + .expect("match the persister's busy timeout"); + connection +} diff --git a/src/context/wallet_lifecycle/bootstrap.rs b/src/context/wallet_lifecycle/bootstrap.rs index 90e1bc1ac..afaf8b9bd 100644 --- a/src/context/wallet_lifecycle/bootstrap.rs +++ b/src/context/wallet_lifecycle/bootstrap.rs @@ -2,6 +2,7 @@ //! managed identities, warming auth-key caches, and queuing identity discovery. use super::*; +use crate::backend_task::identity::IdentityDiscoveryMode; impl AppContext { /// Whether `wallet` still needs its bootstrap address set derived. @@ -446,7 +447,12 @@ impl AppContext { .subtasks .spawn_sync("all_wallets_identity_discovery", async move { if let Err(error) = ctx - .discover_identities_gap_limited(&wallet, 0, false, None) + .discover_identities_gap_limited( + &wallet, + 0, + IdentityDiscoveryMode::Background, + None, + ) .await { tracing::warn!( @@ -496,7 +502,7 @@ impl AppContext { } if let Err(error) = self - .discover_identities_gap_limited(wallet, 0, true, None) + .discover_identities_gap_limited(wallet, 0, IdentityDiscoveryMode::WalletUnlock, None) .await { tracing::warn!( diff --git a/src/context/wallet_lifecycle/spv.rs b/src/context/wallet_lifecycle/spv.rs index 3d1e76d09..105156ffd 100644 --- a/src/context/wallet_lifecycle/spv.rs +++ b/src/context/wallet_lifecycle/spv.rs @@ -3,6 +3,9 @@ use super::*; +const IDENTITY_WIPE_ATTEMPTS: usize = 5; +const IDENTITY_WIPE_RETRY_DELAY: std::time::Duration = std::time::Duration::from_millis(25); + impl AppContext { /// Delete the cached chain-sync data (headers, filters, blocks, masternode /// state, peers) for this network so the next connection re-syncs from @@ -71,25 +74,183 @@ impl AppContext { failures.push(TaskError::DashpaySidecarStorage { source }); } } + // Hold every successful cleanup claim — ghost recovery and ordinary + // identity wipe alike — until the whole wipe finishes. Otherwise a + // concurrent explicit load could repopulate the just-purged slot after + // the index sweep and make a reported-success wipe incomplete. + let mut successful_identity_cleanup_guards = Vec::new(); + let mut failed_forgotten_cleanup_guards = Vec::new(); + let mut forgotten_marker_clear_candidates = Vec::new(); + let mut forgotten_indexed_identities = Vec::new(); + match self.list_forgotten_identities() { + Ok(forgotten_identities) => { + for identity_id in forgotten_identities { + let load_guard = self + .begin_identity_load(identity_id, None) + .map_err(|error| match error { + TaskError::IdentityLoadInProgress { identity_id } => { + TaskError::IdentityBusyWithLoad { identity_id } + } + other => other, + }); + let (retry_result, load_guard) = match load_guard { + Ok(load_guard) => { + let retry_result = match self.migration_run.try_lock() { + Ok(_migration_guard) + if self.migration_status().state().is_in_progress() => + { + Err(TaskError::WalletStorageNotReady) + } + Ok(_migration_guard) => { + self.retry_stuck_unload_cleanup(&identity_id.to_buffer()) + } + Err(_) => Err(TaskError::WalletStorageNotReady), + }; + (retry_result, Some(load_guard)) + } + Err(error) => (Err(error), None), + }; + match retry_result { + Ok(true) => { + if let Some(load_guard) = load_guard { + successful_identity_cleanup_guards.push(load_guard); + } + } + Ok(false) => { + let Some(load_guard) = load_guard else { + // Only a claimed identity can reach a retry + // verdict, so this is a broken invariant rather + // than a runtime condition. Degrade instead of + // panicking: a panic here would abort a + // destructive wipe partway through. + tracing::error!( + identity_id = %identity_id, + "Forgotten identity reported a cleanup verdict without owning its load claim" + ); + failures.push(TaskError::WalletStorageNotReady); + continue; + }; + match self.local_identity_ids() { + Ok(indexed) if indexed.contains(&identity_id) => { + // The normal indexed wipe below owns this + // identity and acquires its own claim. This + // arm's claim drops here, so a load can win + // the gap before that reacquisition — the + // wipe then fails to claim it and reports + // incomplete rather than a clean sweep. + forgotten_indexed_identities.push(identity_id); + } + Ok(_) => { + let residue_result = match self.migration_run.try_lock() { + Ok(_migration_guard) + if self.migration_status().state().is_in_progress() => + { + Err(TaskError::WalletStorageNotReady) + } + Ok(_migration_guard) => { + self.purge_forgotten_identity_residue(&identity_id) + } + Err(_) => Err(TaskError::WalletStorageNotReady), + }; + match residue_result { + Ok(()) => { + forgotten_marker_clear_candidates.push(identity_id); + successful_identity_cleanup_guards.push(load_guard); + } + Err(error) => { + failed_forgotten_cleanup_guards.push(load_guard); + tracing::warn!( + identity_id = %identity_id, + error = ?error, + "Forgotten identity residue cleanup failed during full wipe" + ); + failures.push(error); + } + } + } + Err(error) => { + failed_forgotten_cleanup_guards.push(load_guard); + tracing::warn!( + identity_id = %identity_id, + error = ?error, + "Identity index check failed during full wipe" + ); + failures.push(error); + } + } + } + Err(error) => { + if let Some(load_guard) = load_guard { + failed_forgotten_cleanup_guards.push(load_guard); + } + // Keep the marker so a later load or full wipe can retry + // cleanup using the residual blob's vault-key inventory. + tracing::warn!( + identity_id = %identity_id, + error = ?error, + "Forgotten identity cleanup retry failed during full wipe" + ); + failures.push(error); + } + } + } + } + Err(error) => { + tracing::warn!(error = ?error, "Forgotten identity listing failed during full wipe"); + failures.push(error); + } + } match self.local_identity_ids() { Ok(owners) => { - for owner in owners { - if let Err(e) = backend.dashpay_clear_owner_overlays(&owner) { + for identity_id in &forgotten_indexed_identities { + if !owners.contains(identity_id) { + // The identity changed between the guarded forgotten + // classification and this indexed snapshot. Fail closed + // and keep its marker; the next wipe can classify the + // resulting state without a handoff gap. tracing::warn!( - owner = %owner, - "DashPay per-owner overlay clear failed: {e:?}" + identity_id = %identity_id, + "Forgotten identity changed during full-wipe handoff" ); - failures.push(e); + failures.push(TaskError::WalletStorageNotReady); } + } + for owner in owners { // Wipe each identity's vault keys and det:identity:* records too — // Tier-1 keyless identity keys (incl. masternode voting/owner/payout) // are plaintext-recoverable, so a full wipe must remove them as well. - if let Err(e) = self.delete_local_qualified_identity(&owner) { - tracing::warn!( - owner = %owner, - "Identity private-key wipe failed during clear: {e:?}" - ); - failures.push(e); + // Every identity is deleted through the claim-retaining form so no + // slot reopens to a concurrent load while the sweep is still running. + let mut attempts_remaining = IDENTITY_WIPE_ATTEMPTS; + let deletion_result = loop { + match self.delete_local_qualified_identity_retaining_claim(&owner) { + Err(TaskError::IdentityBusyWithLoad { .. }) + if attempts_remaining > 1 => + { + attempts_remaining -= 1; + tokio::time::sleep(IDENTITY_WIPE_RETRY_DELAY).await; + } + result => break result, + } + }; + match deletion_result { + Ok(load_guard) => { + if forgotten_indexed_identities.contains(&owner) { + forgotten_marker_clear_candidates.push(owner); + } + successful_identity_cleanup_guards.push(load_guard); + } + Err(error) => { + tracing::warn!( + owner = %owner, + "Identity private-key wipe failed during clear: {error:?}" + ); + let underlying_error = match error { + TaskError::IdentityUnloadCleanupFailed { source, .. } => *source, + other => other, + }; + failures.push(underlying_error); + } } } } @@ -101,12 +262,30 @@ impl AppContext { failures.push(e); } } + // Retire only markers captured by this sweep whose attributable cleanup + // completed. A blanket clear could erase a recovery marker created by a + // concurrent unload after the listing snapshot. + for identity_id in forgotten_marker_clear_candidates { + if let Err(error) = self.clear_forgotten_identity_after_explicit_load(&identity_id) { + tracing::warn!( + identity_id = %identity_id, + error = ?error, + "Forgotten identity marker clear failed during full wipe" + ); + failures.push(error); + } + } // Reset the upstream shielded coordinator (quiesces its sync loop and // empties the per-network store) and unlink DET's two retired legacy // shielded files. The legacy-file unlinks are synchronous and scoped - // strictly to THIS network's spv directory. - cleanup_legacy_shielded_files(backend.spv_storage_dir())?; + // strictly to THIS network's spv directory. Neither may early-return: + // the identity claims below are still held, and a `?` here would drop + // them unresolved, reporting durably-cleaned identities as `Failed`. + if let Err(error) = cleanup_legacy_shielded_files(backend.spv_storage_dir()) { + tracing::warn!(%error, "Legacy shielded file cleanup failed during clear"); + failures.push(error); + } if let Err(error) = backend.clear_shielded().await { tracing::warn!(%error, "Shielded coordinator reset failed during clear"); @@ -123,6 +302,22 @@ impl AppContext { self.has_wallet.store(false, Ordering::Relaxed); + // Resolve the claims collected above only now, once every step that can + // restore or touch per-identity state is done: marker retirement, the + // shielded and legacy-file cleanup, and the in-memory wallet teardown. + // Holding them this long is what makes a reported-clean wipe true — a + // claim released earlier reopens that identity's slot to a concurrent + // load, which could persist a fresh blob after this function's only + // index sweep. Nothing between guard capture and here early-returns, so + // an identity whose cleanup durably succeeded still records `Loaded`. + // The forgotten-and-still-indexed classification above is the one claim + // not collected here: it hands off to the owners loop by reacquiring, + // and a load winning that gap makes the wipe report incomplete. + for load_guard in successful_identity_cleanup_guards { + load_guard.loaded(); + } + drop(failed_forgotten_cleanup_guards); + // Any secret-bearing delete that failed above means data may survive on // disk, so never report a clean wipe. The in-memory maps are still // cleared; the typed error tells the user to restart and retry. diff --git a/src/context/wallet_lifecycle/tests.rs b/src/context/wallet_lifecycle/tests.rs index de0af38f5..56bb1b585 100644 --- a/src/context/wallet_lifecycle/tests.rs +++ b/src/context/wallet_lifecycle/tests.rs @@ -2300,6 +2300,110 @@ async fn clear_network_database_wipes_local_identity_private_keys() { .await; } +/// A network clear retries when a background load briefly owns the identity. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn clear_network_database_retries_until_a_load_claim_clears() { + use crate::model::qualified_identity::encrypted_key_storage::{KeyStorage, PrivateKeyData}; + use crate::model::qualified_identity::qualified_identity_public_key::QualifiedIdentityPublicKey; + use crate::model::qualified_identity::{ + IdentityStatus, IdentityType, PrivateKeyTarget, QualifiedIdentity, + }; + use crate::wallet_backend::IdentityKeyView; + use dash_sdk::dpp::identity::Identity; + use dash_sdk::dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; + use dash_sdk::dpp::version::PlatformVersion; + use dash_sdk::platform::{Identifier, IdentityPublicKey}; + use std::collections::BTreeMap; + + let (ctx, sender, _tmp) = offline_testnet_context(); + ctx.ensure_wallet_backend(sender) + .await + .expect("ensure_wallet_backend should succeed offline"); + + let pv = PlatformVersion::latest(); + let key = IdentityPublicKey::random_key(1, Some(1), pv); + let key_id = key.id(); + let mut private_keys = KeyStorage::default(); + private_keys.private_keys.insert( + (PrivateKeyTarget::PrivateKeyOnMainIdentity, key_id), + ( + QualifiedIdentityPublicKey::from(key), + PrivateKeyData::Clear([0x5Bu8; 32]), + ), + ); + let identity_id = Identifier::from([0x34u8; 32]); + let identity = Identity::create_basic_identity(identity_id, pv).expect("basic identity"); + let qi = QualifiedIdentity { + identity, + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::User, + alias: None, + private_keys, + dpns_names: vec![], + associated_wallets: BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::Active, + network: Network::Testnet, + }; + ctx.insert_local_qualified_identity(&qi, &None) + .expect("persist local identity"); + + let store = ctx.secret_store(); + let view = IdentityKeyView::new(&store, identity_id.to_buffer()); + assert!( + view.get(&PrivateKeyTarget::PrivateKeyOnMainIdentity, key_id) + .expect("vault read before clear") + .is_some(), + "precondition: the identity private key is in the vault before clear" + ); + + // Hold the exclusive claim long enough for the first wipe attempt to fail, + // then release it within the bounded retry window. + let discovery_claim = ctx + .begin_identity_load(identity_id, None) + .expect("simulate an outstanding discovery claim on this identity"); + let release_claim = tokio::spawn(async move { + tokio::time::sleep(std::time::Duration::from_millis(40)).await; + drop(discovery_claim); + }); + + let result = ctx.clear_network_database().await; + release_claim.await.expect("release discovery claim"); + + assert!( + result.is_ok(), + "a claim released within the retry window must not make the clear incomplete: {result:?}" + ); + + assert!( + ctx.local_identity_ids() + .expect("list ids after clear") + .is_empty(), + "the retried identity must be removed from local storage" + ); + assert!( + view.get(&PrivateKeyTarget::PrivateKeyOnMainIdentity, key_id) + .expect("vault read after clear") + .is_none(), + "the retried wipe must delete the identity's private key" + ); + + // The in-memory wallet maps are torn down after the successful retry. + assert!( + ctx.wallets().read().expect("read wallets").is_empty(), + "in-memory wallets must be cleared after the wipe" + ); + + ctx.wallet_backend() + .expect("backend wired") + .shutdown() + .await; +} + /// A masternode removal must report an incomplete clear when its voting, /// owner, or payout key cannot be deleted from the vault. #[cfg(unix)] @@ -2430,6 +2534,334 @@ async fn clear_network_database_reports_incomplete_when_shielded_clear_fails() { } } +/// "Delete all local data" clears forgotten-identity markers so re-importing a +/// wallet can rediscover identities that were unloaded before the wipe. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn clear_network_database_clears_forgotten_identity_markers() { + use dash_sdk::platform::Identifier; + + let (ctx, sender, _tmp) = offline_testnet_context(); + ctx.ensure_wallet_backend(sender) + .await + .expect("ensure_wallet_backend should succeed offline"); + + let identity_id = Identifier::from([0x35u8; 32]); + ctx.record_forgotten_identity(&identity_id) + .expect("record forgotten marker before the wipe"); + assert!( + ctx.is_identity_forgotten(&identity_id) + .expect("read marker before wipe"), + "precondition: the identity is marked forgotten before the wipe" + ); + + ctx.clear_network_database() + .await + .expect("clear_network_database should succeed with nothing else to wipe"); + + assert!( + !ctx.is_identity_forgotten(&identity_id) + .expect("read marker after wipe"), + "the full wipe must clear forgotten-identity markers", + ); + + ctx.wallet_backend() + .expect("backend wired") + .shutdown() + .await; +} + +/// A minimal local identity carrying one plaintext key, so wiping it has real +/// vault state to remove. +fn keyed_qualified_identity( + identity_id: dash_sdk::platform::Identifier, + secret: [u8; 32], +) -> crate::model::qualified_identity::QualifiedIdentity { + use crate::model::qualified_identity::encrypted_key_storage::{KeyStorage, PrivateKeyData}; + use crate::model::qualified_identity::qualified_identity_public_key::QualifiedIdentityPublicKey; + use crate::model::qualified_identity::{ + IdentityStatus, IdentityType, PrivateKeyTarget, QualifiedIdentity, + }; + use dash_sdk::dpp::identity::Identity; + use dash_sdk::dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; + use dash_sdk::dpp::version::PlatformVersion; + use dash_sdk::platform::IdentityPublicKey; + + let pv = PlatformVersion::latest(); + let key = IdentityPublicKey::random_key(1, Some(1), pv); + let mut private_keys = KeyStorage::default(); + private_keys.private_keys.insert( + (PrivateKeyTarget::PrivateKeyOnMainIdentity, key.id()), + ( + QualifiedIdentityPublicKey::from(key), + PrivateKeyData::Clear(secret), + ), + ); + QualifiedIdentity { + identity: Identity::create_basic_identity(identity_id, pv).expect("basic identity"), + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::User, + alias: None, + private_keys, + dpns_names: vec![], + associated_wallets: std::collections::BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: std::collections::BTreeMap::new(), + status: IdentityStatus::Active, + network: Network::Testnet, + } +} + +/// An ordinary (never-unloaded) identity's exclusive claim must outlive the +/// whole wipe, not just its own deletion. Releasing it mid-sweep reopens that +/// identity's slot: a concurrent load could persist a fresh blob after the +/// wipe's only index sweep, and the wipe would still report success. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn clear_network_database_holds_ordinary_identity_claim_until_the_wipe_ends() { + use crate::context::identity_load_registry::IdentityLoadPhase; + use dash_sdk::platform::Identifier; + + let (ctx, sender, _tmp) = offline_testnet_context(); + ctx.ensure_wallet_backend(sender) + .await + .expect("ensure_wallet_backend should succeed offline"); + + // The enumeration index preserves insertion order, so the wipe reaches the + // ordinary identity first and the blocked ones after it. Their bounded + // retries are what keep the wipe running while the probe below happens. + let ordinary_id = Identifier::from([0x36u8; 32]); + ctx.insert_local_qualified_identity( + &keyed_qualified_identity(ordinary_id, [0x5Cu8; 32]), + &None, + ) + .expect("persist the ordinary identity"); + let blocked_ids: Vec = (0..5).map(|i| Identifier::from([0x40u8 + i; 32])).collect(); + for (i, blocked_id) in blocked_ids.iter().enumerate() { + ctx.insert_local_qualified_identity( + &keyed_qualified_identity(*blocked_id, [0x60u8 + i as u8; 32]), + &None, + ) + .expect("persist a blocked identity"); + } + let blocking_claims: Vec<_> = blocked_ids + .iter() + .map(|blocked_id| { + ctx.begin_identity_load(*blocked_id, None) + .expect("hold a claim the wipe has to retry against") + }) + .collect(); + + let wipe = tokio::spawn({ + let ctx = Arc::clone(&ctx); + async move { ctx.clear_network_database().await } + }); + + // Wait for the ordinary identity's blob purge — the last step of its + // deletion — then let the deletion call itself return. A claim released per + // deletion is gone microseconds after that point; a claim held to the end of + // the wipe survives the ~500ms the blocked identities spend retrying. + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + while ctx + .has_local_qualified_identity(&ordinary_id) + .expect("read the ordinary identity") + { + assert!( + std::time::Instant::now() < deadline, + "the wipe never reached the ordinary identity" + ); + tokio::time::sleep(std::time::Duration::from_millis(1)).await; + } + tokio::time::sleep(std::time::Duration::from_millis(50)).await; + + let concurrent_load = ctx.begin_identity_load(ordinary_id, None); + let phase_during_wipe = ctx.latest_identity_load_phase(&ordinary_id); + + drop(blocking_claims); + let _ = wipe.await.expect("the wipe task must not panic"); + + assert!( + matches!( + concurrent_load, + Err(TaskError::IdentityLoadInProgress { identity_id }) if identity_id == ordinary_id + ), + "a concurrent load must be excluded until the wipe finishes, got: {concurrent_load:?}" + ); + assert_eq!( + phase_during_wipe, + Some(IdentityLoadPhase::Running), + "the wiped identity's claim must still be unresolved mid-wipe" + ); + assert!( + !ctx.local_identity_ids() + .expect("read the identity index after the wipe") + .contains(&ordinary_id), + "the ordinary identity must still be wiped" + ); + + ctx.wallet_backend() + .expect("backend wired") + .shutdown() + .await; +} + +/// An identity that is both forgotten-marked and still indexed is classified in +/// the forgotten pass, which releases its claim and leaves the owners loop to +/// reacquire one, so a load can win that hand-off gap. However the race lands, +/// the wipe must never report success while leaving that identity on disk: it +/// either owns the identity and removes it, or fails to claim it and reports the +/// clear incomplete. +#[tokio::test(flavor = "multi_thread", worker_threads = 4)] +async fn clear_network_database_never_reports_success_for_a_reclaimed_forgotten_identity() { + use crate::context::identity_load_registry::IdentityLoadPhase; + use dash_sdk::platform::Identifier; + + let (ctx, sender, _tmp) = offline_testnet_context(); + ctx.ensure_wallet_backend(sender) + .await + .expect("ensure_wallet_backend should succeed offline"); + + // Inserted first, so the owners loop stalls on their held claims long enough + // for the probe below to reach the target's hand-off window. + let blocked_ids: Vec = (0..5).map(|i| Identifier::from([0x50u8 + i; 32])).collect(); + for (i, blocked_id) in blocked_ids.iter().enumerate() { + ctx.insert_local_qualified_identity( + &keyed_qualified_identity(*blocked_id, [0x70u8 + i as u8; 32]), + &None, + ) + .expect("persist a blocked identity"); + } + + let target_id = Identifier::from([0x99u8; 32]); + ctx.insert_local_qualified_identity(&keyed_qualified_identity(target_id, [0x98u8; 32]), &None) + .expect("persist target identity"); + ctx.record_forgotten_identity(&target_id) + .expect("mark the target forgotten while it is still indexed"); + + let blocking_claims: Vec<_> = blocked_ids + .iter() + .map(|blocked_id| { + ctx.begin_identity_load(*blocked_id, None) + .expect("hold a claim the wipe has to retry against") + }) + .collect(); + + let wipe = tokio::spawn({ + let ctx = Arc::clone(&ctx); + async move { ctx.clear_network_database().await } + }); + + // Wait for the classification pass to release the target's claim, then race + // the owners loop for it exactly as a background load would. + let deadline = std::time::Instant::now() + std::time::Duration::from_secs(5); + while !wipe.is_finished() + && !matches!( + ctx.latest_identity_load_phase(&target_id), + Some(IdentityLoadPhase::Failed) + ) + { + assert!( + std::time::Instant::now() < deadline, + "the target's classification claim was never observed being released" + ); + tokio::time::sleep(std::time::Duration::from_millis(1)).await; + } + let concurrent_load = ctx.begin_identity_load(target_id, None); + let load_won_the_gap = concurrent_load.is_ok(); + + drop(blocking_claims); + let result = wipe.await.expect("the wipe task must not panic"); + let target_survived = ctx + .has_local_qualified_identity(&target_id) + .expect("read the target after the wipe"); + // Only now — the claim has to outlive the wipe to contest it at all. + drop(concurrent_load); + + if load_won_the_gap { + assert!( + matches!(result, Err(TaskError::WalletDataClearIncomplete { .. })), + "a wipe that could not claim the identity must report incomplete: {result:?}" + ); + assert!( + target_survived, + "an incomplete report must correspond to an identity actually left on disk" + ); + } else { + assert!( + result.is_ok(), + "an uncontested wipe must succeed: {result:?}" + ); + assert!( + !target_survived, + "a successful wipe must leave no trace of the identity" + ); + } + + ctx.wallet_backend() + .expect("backend wired") + .shutdown() + .await; +} + +/// Guards are resolved after the legacy shielded-file cleanup, so that cleanup +/// must never early-return: a `?` there would drop every held claim unresolved +/// and report durably-wiped identities as failed loads. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn clear_network_database_resolves_identity_claims_after_a_legacy_file_cleanup_failure() { + use crate::context::identity_load_registry::IdentityLoadPhase; + use dash_sdk::platform::Identifier; + + let (ctx, sender, _tmp) = offline_testnet_context(); + ctx.ensure_wallet_backend(sender) + .await + .expect("ensure_wallet_backend should succeed offline"); + let backend = ctx.wallet_backend().expect("backend wired"); + + let identity_id = Identifier::from([0x37u8; 32]); + ctx.insert_local_qualified_identity( + &keyed_qualified_identity(identity_id, [0x5Du8; 32]), + &None, + ) + .expect("persist local identity"); + + // A directory where a legacy shielded file is expected: the unlink fails + // with a non-NotFound error, leaving the wallet database in the same + // directory untouched. + std::fs::create_dir(backend.spv_storage_dir().join("det-shielded.sqlite")) + .expect("plant the legacy shielded cleanup fault"); + + let result = ctx.clear_network_database().await; + + backend.shutdown().await; + + match result { + Err(TaskError::WalletDataClearIncomplete { + failed, + first_error, + }) => { + assert_eq!(failed, 1, "the legacy file unlink is the only failure"); + assert!( + matches!(*first_error, TaskError::FileSystem { .. }), + "the aggregate must preserve the legacy file cleanup error" + ); + } + other => panic!("a legacy file cleanup failure must make clear incomplete: {other:?}"), + } + assert_eq!( + ctx.latest_identity_load_phase(&identity_id), + Some(IdentityLoadPhase::Loaded), + "an identity wiped durably must record a successful claim regardless of \ + unrelated cleanup failures" + ); + assert!( + ctx.local_identity_ids() + .expect("read the identity index after the wipe") + .is_empty(), + "the identity must still be wiped" + ); +} + /// Clear-all must fail before changing any state when the wallet backend is /// unavailable, because persisted secrets from an earlier run may still exist. #[tokio::test] diff --git a/src/database/initialization.rs b/src/database/initialization.rs index b9da5686d..1082a2b0d 100644 --- a/src/database/initialization.rs +++ b/src/database/initialization.rs @@ -742,8 +742,8 @@ impl Database { /// are created. Truly-fresh DET installs pass `false` so these dormant /// schemas never appear in `data.db`; legacy installs and the migration /// ladder still pass `true` so upgrade arms keep working. Always-present - /// tables (`settings`, `identity`, `platform_address_balances`) are - /// created regardless. + /// tables (`settings`, `platform_address_balances`) are created + /// regardless. pub(crate) fn create_tables(&self, include_legacy: bool) -> rusqlite::Result<()> { let conn = self.locked_conn(); // Create the settings table. diff --git a/src/database/mod.rs b/src/database/mod.rs index ffa200211..0c42f457f 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -1,3 +1,11 @@ +//! Frozen legacy `data.db` — a migration-read source and recovery artifact, +//! not a general persistence layer. Production opens an existing file +//! read-only (`Database::open_legacy_read_only`) and never runs the schema +//! ladder in `initialization.rs` against it; only a fresh install with no +//! `data.db` yet initializes and writes. Never add a table, column, or write +//! path here. Current durable state is a `DetKv` key (`docs/kv-keys.md`) or a +//! `SecretStore` entry (`wallet_backend/secret_seam.rs`). + mod initialization; #[cfg(test)] pub(crate) use initialization::DEFAULT_DB_VERSION; diff --git a/src/model/contested_name.rs b/src/model/contested_name.rs index 893dcde65..fd049a864 100644 --- a/src/model/contested_name.rs +++ b/src/model/contested_name.rs @@ -180,7 +180,10 @@ pub fn approximate_time_until(decided_at_ms: TimestampMillis, now_ms: u64) -> Op "Dash masternodes vote on who receives this username. A decision is expected in about 1 hour." .to_string() } else if secs < DAY { - let hours = secs / HOUR; + // Round rather than floor: `secs` is measured from an independent + // clock read than the caller's target, so a few elapsed + // milliseconds must not drop the displayed hour count by one. + let hours = (secs + HOUR / 2) / HOUR; format!( "Dash masternodes vote on who receives this username. A decision is expected in about {hours} hours." ) @@ -188,7 +191,7 @@ pub fn approximate_time_until(decided_at_ms: TimestampMillis, now_ms: u64) -> Op "Dash masternodes vote on who receives this username. A decision is expected in about 1 day." .to_string() } else { - let days = secs / DAY; + let days = (secs + DAY / 2) / DAY; format!( "Dash masternodes vote on who receives this username. A decision is expected in about {days} days." ) @@ -458,6 +461,28 @@ mod tests { ); } + #[test] + fn approximate_time_until_rounds_instead_of_flooring_near_an_hour_or_day_boundary() { + // Callers compute the target from one clock read and pass `now_ms` from + // a second, later read (see `pill::pending_username_tooltip`) — a few + // elapsed milliseconds must not drop the displayed count by a whole + // bucket. + let now = 1_000_000_000_000u64; + let ms = |secs: u64| now + secs * 1_000; + assert_eq!( + approximate_time_until(ms(3 * 3_600 - 1), now).as_deref(), + Some( + "Dash masternodes vote on who receives this username. A decision is expected in about 3 hours." + ) + ); + assert_eq!( + approximate_time_until(ms(3 * 86_400 - 1), now).as_deref(), + Some( + "Dash masternodes vote on who receives this username. A decision is expected in about 3 days." + ) + ); + } + #[test] fn approximate_time_until_is_none_when_deadline_passed_or_now() { let now = 1_000_000_000_000u64; diff --git a/src/model/identity_discovery.rs b/src/model/identity_discovery.rs index 4828f8b2b..b75931a0c 100644 --- a/src/model/identity_discovery.rs +++ b/src/model/identity_discovery.rs @@ -85,6 +85,15 @@ pub struct DiscoverySummary { pub found: u32, /// Identities newly stored or refreshed in the local database. pub stored: u32, + /// Identities left unloaded because the user had unloaded them. Counted + /// separately from the rest of `found - stored` so the user can be told a + /// deliberate unload — not a failure — is why the scan loaded fewer + /// identities than it found. + pub skipped_forgotten: u32, + /// Identities found on the network that could not be stored on this device. + /// Without this count a scan whose every write failed is indistinguishable + /// from one that simply found nothing new. + pub failed: u32, } #[cfg(test)] diff --git a/src/model/qualified_identity/encrypted_key_storage.rs b/src/model/qualified_identity/encrypted_key_storage.rs index 25fe42699..9d61c697b 100644 --- a/src/model/qualified_identity/encrypted_key_storage.rs +++ b/src/model/qualified_identity/encrypted_key_storage.rs @@ -425,6 +425,23 @@ impl KeyStorage { .collect() } + /// Whether any stored private key lacks an available HD wallet. + pub(crate) fn has_keys_without_available_wallet( + &self, + wallet_is_available: impl Fn(&WalletSeedHash) -> bool, + ) -> bool { + self.private_keys.values().any(|(public_key, private_key)| { + let wallet_seed_hash = match private_key { + PrivateKeyData::AtWalletDerivationPath(path) => Some(&path.wallet_seed_hash), + _ => public_key + .in_wallet_at_derivation_path + .as_ref() + .map(|path| &path.wallet_seed_hash), + }; + wallet_seed_hash.is_none_or(|seed_hash| !wallet_is_available(seed_hash)) + }) + } + /// Inserts an unencrypted key into `ClearKeyStorage`. Returns an error if the storage is closed. pub fn insert_non_encrypted( &mut self, @@ -663,6 +680,80 @@ mod tests { ks } + #[test] + fn wallet_derived_keys_do_not_need_separate_recovery_information() { + let storage = { + let pv = PlatformVersion::latest(); + let key = IdentityPublicKey::random_key(4, Some(4), pv); + KeyStorage::from(BTreeMap::from([( + (PrivateKeyTarget::PrivateKeyOnMainIdentity, key.id()), + ( + QualifiedIdentityPublicKey::from(key), + WalletDerivationPath { + wallet_seed_hash: [0x04; 32], + derivation_path: DerivationPath::from(vec![]), + }, + ), + )])) + }; + + assert!( + !storage.has_keys_without_available_wallet(|seed_hash| { *seed_hash == [0x04; 32] }) + ); + } + + #[test] + fn wallet_derived_key_needs_recovery_when_its_wallet_is_unavailable() { + let pv = PlatformVersion::latest(); + let key = IdentityPublicKey::random_key(6, Some(6), pv); + let storage = KeyStorage::from(BTreeMap::from([( + (PrivateKeyTarget::PrivateKeyOnMainIdentity, key.id()), + ( + QualifiedIdentityPublicKey::from(key), + WalletDerivationPath { + wallet_seed_hash: [0x06; 32], + derivation_path: DerivationPath::from(vec![]), + }, + ), + )])); + + assert!(storage.has_keys_without_available_wallet(|_| false)); + } + + #[test] + fn mixed_wallet_and_local_keys_need_separate_recovery_information() { + let local_high = distinctive_secret(); + let mut local_medium = local_high; + local_medium[0] ^= 0xFF; + let storage = storage_with_plaintext_and_derived(local_high, local_medium); + + assert!(storage.has_keys_without_available_wallet(|_| true)); + } + + #[test] + fn vault_key_with_wallet_metadata_does_not_need_separate_recovery_information() { + let pv = PlatformVersion::latest(); + let key = IdentityPublicKey::random_key(5, Some(5), pv); + let wallet_path = WalletDerivationPath { + wallet_seed_hash: [0x05; 32], + derivation_path: DerivationPath::from(vec![]), + }; + let storage = KeyStorage::from(BTreeMap::from([( + (PrivateKeyTarget::PrivateKeyOnMainIdentity, key.id()), + ( + QualifiedIdentityPublicKey::from_identity_public_key_in_wallet( + key, + Some(wallet_path), + ), + PrivateKeyData::InVault, + ), + )])); + + assert!( + !storage.has_keys_without_available_wallet(|seed_hash| { *seed_hash == [0x05; 32] }) + ); + } + /// TS-RESID-02 — a bincode blob written BEFORE `InVault` was appended /// (discriminants 0–3 only) still decodes into the extended enum, and the /// new highest-index variant round-trips. Guards the bincode-discriminant diff --git a/src/model/qualified_identity/mod.rs b/src/model/qualified_identity/mod.rs index 5a3d2a2b6..02b3ffd1a 100644 --- a/src/model/qualified_identity/mod.rs +++ b/src/model/qualified_identity/mod.rs @@ -544,6 +544,14 @@ fn identity_blob_decode_config() -> impl bincode::config::Config { bincode::config::standard().with_limit::<{ IDENTITY_BLOB_DECODE_LIMIT }>() } +/// Returns whether an identity has a masternode or evonode owner key. +pub fn identity_carries_owner_key(identity: &Identity) -> bool { + identity + .public_keys() + .values() + .any(|key| key.purpose() == Purpose::OWNER) +} + impl QualifiedIdentity { /// Serializes the QualifiedIdentity to a vector of bytes. pub fn to_bytes(&self) -> Vec { @@ -593,6 +601,14 @@ impl QualifiedIdentity { presence } + /// Whether unloading would remove a key that no loaded HD wallet can restore. + pub fn requires_recovery_information_after_unload(&self) -> bool { + self.private_keys + .has_keys_without_available_wallet(|seed_hash| { + self.associated_wallets.contains_key(seed_hash) + }) + } + /// Resolve the 32-byte private key for `(target, key_id)` without ever /// reading a wallet's parked seed. /// @@ -1103,6 +1119,40 @@ impl QualifiedIdentity { } } +#[cfg(test)] +mod identity_owner_key_tests { + use super::*; + use dash_sdk::dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeySettersV0; + use dash_sdk::dpp::version::PlatformVersion; + use dash_sdk::platform::Identifier; + + fn identity_with_key_purpose(purpose: Purpose) -> Identity { + let platform_version = PlatformVersion::latest(); + let mut key = IdentityPublicKey::random_key(1, Some(1), platform_version); + key.set_purpose(purpose); + Identity::new_with_id_and_keys( + Identifier::random(), + BTreeMap::from([(key.id(), key)]), + platform_version, + ) + .expect("identity") + } + + #[test] + fn identity_with_owner_key_is_detected() { + let identity = identity_with_key_purpose(Purpose::OWNER); + + assert!(identity_carries_owner_key(&identity)); + } + + #[test] + fn identity_with_only_authentication_key_is_not_detected() { + let identity = identity_with_key_purpose(Purpose::AUTHENTICATION); + + assert!(!identity_carries_owner_key(&identity)); + } +} + #[cfg(test)] mod masternode_key_presence_tests { use super::*; diff --git a/src/ui/dashpay/dashpay_screen.rs b/src/ui/dashpay/dashpay_screen.rs index fd60c954b..d7c21713f 100644 --- a/src/ui/dashpay/dashpay_screen.rs +++ b/src/ui/dashpay/dashpay_screen.rs @@ -210,7 +210,8 @@ impl ScreenLike for DashPayScreen { .contacts_list .contact_requests .display_task_error(error), - DashPaySubscreen::Profile | DashPaySubscreen::Payments => false, + DashPaySubscreen::Profile => self.profile_screen.display_task_error(error), + DashPaySubscreen::Payments => false, DashPaySubscreen::ProfileSearch => false, } } diff --git a/src/ui/dashpay/profile_screen.rs b/src/ui/dashpay/profile_screen.rs index b75802837..cf8bdbb0d 100644 --- a/src/ui/dashpay/profile_screen.rs +++ b/src/ui/dashpay/profile_screen.rs @@ -1,5 +1,6 @@ use crate::app::AppAction; use crate::backend_task::dashpay::DashPayTask; +use crate::backend_task::error::TaskError; use crate::backend_task::{BackendTask, BackendTaskSuccessResult}; use crate::context::AppContext; use crate::model::dashpay::{MAX_AVATAR_URL_CHARS, ProfileFieldError}; @@ -59,6 +60,8 @@ pub struct ProfileScreen { loading: bool, saving: bool, // Track if we're saving vs loading profile_load_attempted: bool, + in_flight_profile_load: Option<(dash_sdk::platform::Identifier, u64)>, + profile_load_generation: u64, validation_errors: Vec, has_unsaved_changes: bool, original_display_name: String, @@ -92,6 +95,8 @@ impl ProfileScreen { loading: false, saving: false, profile_load_attempted: false, + in_flight_profile_load: None, + profile_load_generation: 0, validation_errors: Vec::new(), has_unsaved_changes: false, original_display_name: String::new(), @@ -168,7 +173,12 @@ impl ProfileScreen { } pub fn trigger_load_profile(&mut self) -> AppAction { + if self.in_flight_profile_load.is_some() { + return AppAction::None; + } if let Some(identity) = self.selected_identity.clone() { + self.in_flight_profile_load = + Some((identity.identity.id(), self.profile_load_generation)); self.loading = true; self.profile_load_attempted = true; AppAction::BackendTask(BackendTask::DashPayTask(Box::new( @@ -381,6 +391,7 @@ impl ProfileScreen { if response.changed() { // Reset state when identity changes + self.profile_load_generation = self.profile_load_generation.wrapping_add(1); self.profile = None; self.profile_load_attempted = false; self.loading = false; @@ -1046,9 +1057,35 @@ impl ProfileScreen { if matches!(message_type, MessageType::Error | MessageType::Warning) { self.loading = false; self.saving = false; + self.in_flight_profile_load = None; } } + fn invalidate_unloaded_identity(&mut self, identity_id: &dash_sdk::platform::Identifier) { + if self + .selected_identity + .as_ref() + .is_some_and(|identity| identity.identity.id() == *identity_id) + { + self.profile_load_generation = self.profile_load_generation.wrapping_add(1); + self.selected_identity = None; + self.selected_identity_string.clear(); + self.profile = None; + self.loading = false; + self.saving = false; + self.profile_load_attempted = false; + self.editing = false; + self.has_unsaved_changes = false; + } + } + + pub fn display_task_error(&mut self, error: &TaskError) -> bool { + if let TaskError::IdentityUnloadCleanupFailed { identity_id, .. } = error { + self.invalidate_unloaded_identity(identity_id); + } + false + } + pub fn display_task_result(&mut self, result: BackendTaskSuccessResult) { // Avatar results arrive independently of profile load/save; route them // without disturbing those loading states. @@ -1057,13 +1094,35 @@ impl ProfileScreen { return; } - // Always clear loading and saving states first - self.loading = false; - self.saving = false; - self.profile_load_attempted = true; + match &result { + BackendTaskSuccessResult::UnloadedIdentity(identity_id) => { + self.invalidate_unloaded_identity(identity_id); + return; + } + BackendTaskSuccessResult::RemovedIdentities { identity_ids, .. } => { + for identity_id in identity_ids { + self.invalidate_unloaded_identity(identity_id); + } + return; + } + _ => {} + } match result { BackendTaskSuccessResult::DashPayProfile(profile_data) => { + let Some((owner_id, generation)) = self.in_flight_profile_load.take() else { + return; + }; + self.loading = false; + let selected_id = self + .selected_identity + .as_ref() + .map(|identity| identity.identity.id()); + if generation != self.profile_load_generation || selected_id != Some(owner_id) { + self.profile_load_attempted = false; + return; + } + self.profile_load_attempted = true; if let Some((display_name, bio, avatar_url)) = profile_data { // Check if avatar URL changed - if so, we need to re-fetch the avatar let old_avatar_url = self.profile.as_ref().map(|p| p.avatar_url.clone()); @@ -1101,7 +1160,15 @@ impl ProfileScreen { // Don't show a message - let the UI show "Create Profile" button } } - BackendTaskSuccessResult::DashPayProfileUpdated(_identity_id) => { + BackendTaskSuccessResult::DashPayProfileUpdated(updated_identity_id) => { + self.saving = false; + if self + .selected_identity + .as_ref() + .is_none_or(|identity| identity.identity.id() != updated_identity_id) + { + return; + } // Profile was successfully created/updated; the upstream // mirror (`update_profile` → `dashpay_set_profile`) is the // authoritative write, so we only refresh local in-memory @@ -1144,3 +1211,63 @@ impl ProfileScreen { } } } + +#[cfg(test)] +mod tests { + use super::*; + use crate::context::test_support::test_app_context; + use crate::model::qualified_identity::{IdentityStatus, IdentityType}; + use dash_sdk::dpp::dashcore::Network; + use dash_sdk::dpp::version::PlatformVersion; + use dash_sdk::platform::{Identifier, Identity}; + use std::collections::BTreeMap; + + fn qualified_identity(identity_id: Identifier) -> QualifiedIdentity { + QualifiedIdentity { + identity: Identity::create_basic_identity(identity_id, PlatformVersion::latest()) + .expect("create identity"), + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::User, + alias: None, + private_keys: Default::default(), + dpns_names: Vec::new(), + associated_wallets: BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::Active, + network: Network::Testnet, + } + } + + #[test] + fn late_profile_result_for_unloaded_identity_is_discarded() { + let temp_dir = tempfile::tempdir().expect("tempdir"); + let ctx = test_app_context(temp_dir.path()); + let identity = qualified_identity(Identifier::from([0xD1; 32])); + let mut screen = ProfileScreen::new(ctx); + screen.selected_identity = Some(identity); + assert!(matches!( + screen.trigger_load_profile(), + AppAction::BackendTask(BackendTask::DashPayTask(_)) + )); + + let error = TaskError::IdentityUnloadCleanupFailed { + identity_id: Identifier::from([0xD1; 32]), + source: Box::new(TaskError::IdentityNotFound), + }; + assert!(!screen.display_task_error(&error)); + screen.display_task_result(BackendTaskSuccessResult::DashPayProfile(Some(( + "Stale name".to_string(), + "Stale bio".to_string(), + "https://example.invalid/stale.png".to_string(), + )))); + + assert!( + screen.profile.is_none(), + "a result started for an identity that is no longer displayed must be discarded" + ); + } +} diff --git a/src/ui/identities/add_existing_identity_screen.rs b/src/ui/identities/add_existing_identity_screen.rs index 9ab5f5f0e..87b3dd705 100644 --- a/src/ui/identities/add_existing_identity_screen.rs +++ b/src/ui/identities/add_existing_identity_screen.rs @@ -24,6 +24,58 @@ use egui::{Color32, ComboBox, RichText, Ui}; use std::sync::atomic::Ordering; use std::sync::{Arc, RwLock}; +/// Outcome of a wallet identity search, as one message for the user and the +/// banner severity to show it at. +/// +/// Shared by the screen and the global banner so both report the same thing. +/// Each case is one whole template rather than sentences spliced at runtime, so +/// a translator sees the complete message and can reorder it freely. Counts stay +/// plural-neutral ("identity(ies)"), matching the scheduled-vote wording in the +/// unload confirmation. +/// +/// A search never restores an identity the user unloaded, and it can find an +/// identity it then fails to store; both are called out, because either one +/// silently loads fewer identities than the search found. The way back names +/// the identity-ID and username routes: their mode buttons are always on +/// screen, and in default mode both derive the keys from the loaded wallet, so +/// neither asks the user for a key this device no longer holds. +pub(crate) fn wallet_identity_search_message( + count: u32, + skipped_forgotten: u32, + failed: u32, +) -> (String, MessageType) { + match (skipped_forgotten > 0, failed > 0) { + (false, false) => ( + format!("Loaded {count} identity(ies) from your wallet."), + MessageType::Success, + ), + (true, false) => ( + format!( + "Loaded {count} identity(ies) from your wallet. {skipped_forgotten} identity(ies) \ + you unloaded were left alone. To load one again, use its identity ID or its \ + username." + ), + MessageType::Success, + ), + (false, true) => ( + format!( + "Loaded {count} identity(ies) from your wallet. {failed} identity(ies) could not \ + be saved on this device. Search again in a moment to load them." + ), + MessageType::Warning, + ), + (true, true) => ( + format!( + "Loaded {count} identity(ies) from your wallet. {failed} identity(ies) could not \ + be saved on this device. Search again in a moment to load them. \ + {skipped_forgotten} identity(ies) you unloaded were left alone. To load one \ + again, use its identity ID or its username." + ), + MessageType::Warning, + ), + } +} + #[derive(Clone, Copy, PartialEq, Eq)] enum LoadIdentityMode { IdentityId, @@ -970,14 +1022,23 @@ impl ScreenLike for AddExistingIdentityScreen { self.success_message = Some("Successfully loaded identity.".to_string()); self.add_identity_status = AddIdentityStatus::Complete; } - BackendTaskSuccessResult::IdentitiesLoaded { count } => { + BackendTaskSuccessResult::IdentitiesLoaded { + count, + skipped_forgotten, + failed, + } => { self.refresh_banner.take_and_clear(); - self.success_message = Some(if count == 1 { - "Successfully loaded 1 identity from your wallet.".to_string() - } else { - format!("Successfully loaded {count} identities from your wallet.") - }); - self.add_identity_status = AddIdentityStatus::Complete; + let (message, message_type) = + wallet_identity_search_message(count, skipped_forgotten, failed); + match message_type { + MessageType::Success => { + self.success_message = Some(message); + self.add_identity_status = AddIdentityStatus::Complete; + } + // A search that could not store what it found keeps the form + // up, so the retry the banner asks for is one click away. + _ => self.add_identity_status = AddIdentityStatus::NotStarted, + } } BackendTaskSuccessResult::Message(msg) => { // Check if this is a final success message or a progress update @@ -1143,7 +1204,7 @@ impl ScreenLike for AddExistingIdentityScreen { #[cfg(test)] mod load_identity_mode_tests { - use super::LoadIdentityMode; + use super::{LoadIdentityMode, MessageType, wallet_identity_search_message}; const ALL_MODES: [LoadIdentityMode; 3] = [ LoadIdentityMode::IdentityId, @@ -1171,4 +1232,65 @@ mod load_identity_mode_tests { } } } + + /// A wallet search that leaves an unloaded identity alone must say so, and + /// name an action that gets it back — otherwise the count silently disagrees + /// with what the user sees in the list. + #[test] + fn search_message_reports_identities_left_unloaded() { + let (clean, message_type) = wallet_identity_search_message(2, 0, 0); + assert_eq!(clean, "Loaded 2 identity(ies) from your wallet."); + assert_eq!(message_type, MessageType::Success); + + let (skipped, message_type) = wallet_identity_search_message(1, 1, 0); + assert!(skipped.contains("1 identity(ies) you unloaded were left alone.")); + assert!( + skipped.contains("use its identity ID or its username"), + "the way back must be a route the default mode offers: {skipped}" + ); + assert_eq!( + message_type, + MessageType::Success, + "a deliberate unload the search honoured is not a failure", + ); + } + + /// A search that found identities it could not store must not render as a + /// plain success — that is indistinguishable from finding nothing new. + #[test] + fn search_message_warns_when_identities_could_not_be_saved() { + let (failed_only, message_type) = wallet_identity_search_message(0, 0, 2); + assert!(failed_only.contains("2 identity(ies) could not be saved on this device.")); + assert!(failed_only.contains("Search again in a moment")); + assert_eq!(message_type, MessageType::Warning); + + let (both, message_type) = wallet_identity_search_message(1, 3, 2); + assert!(both.contains("2 identity(ies) could not be saved on this device.")); + assert!(both.contains("3 identity(ies) you unloaded were left alone.")); + assert_eq!(message_type, MessageType::Warning); + } + + /// Every case is one whole template: the message must never be assembled + /// from sentences chosen at runtime, which no translator can reorder. + #[test] + fn search_message_names_an_action_and_stays_one_template() { + for (count, skipped, failed) in [(2, 0, 0), (2, 1, 0), (2, 0, 1), (2, 1, 1)] { + let (message, _) = wallet_identity_search_message(count, skipped, failed); + assert!( + message.starts_with("Loaded 2 identity(ies) from your wallet."), + "every case opens with the same complete sentence: {message}" + ); + assert!( + message.ends_with('.'), + "every case is a set of complete sentences: {message}" + ); + if skipped > 0 || failed > 0 { + assert!( + message.contains("use its identity ID or its username") + || message.contains("Search again"), + "a shortfall must name what the user can do about it: {message}" + ); + } + } + } } diff --git a/src/ui/identities/identities_screen.rs b/src/ui/identities/identities_screen.rs index d4a17f94b..95184958e 100644 --- a/src/ui/identities/identities_screen.rs +++ b/src/ui/identities/identities_screen.rs @@ -22,6 +22,9 @@ use crate::ui::identities::register_dpns_name_screen::{ }; use crate::ui::identities::top_up_identity_screen::TopUpIdentityScreen; use crate::ui::identities::transfer_screen::TransferScreen; +use crate::ui::identity::settings::{ + UNLOAD_DETAILS_LOAD_FAILED, identity_removal_confirmation_dialog, identity_unload_tip, +}; use crate::ui::theme::{ComponentStyles, DashColors, ResponseExt}; use crate::ui::{MessageType, RootScreenType, Screen, ScreenLike, ScreenType}; use crate::wallet_backend::poison::MutexRecover; @@ -879,22 +882,32 @@ impl IdentitiesScreen { } // Remove - if ui.button("Remove").clickable_tooltip("Remove this identity from Dash Evo Tool (it'll still exist on Dash Platform)").clicked() { - let message = format!( - "Are you sure you want to no longer track this {identity_type} identity?\n\nIdentity ID: {identity_id}", - identity_type = qualified_identity.identity_type, - identity_id = qualified_identity.identity.id().to_string( - qualified_identity.identity_type.default_encoding() - ) - ); - self.identity_to_remove = - Some(qualified_identity.clone()); - self.remove_confirmation_dialog = Some( - ConfirmationDialog::new("Confirm Removal", message) - .confirm_text(Some("Yes")) - .cancel_text(Some("No")) - .danger_mode(true), - ); + if ui.button("Remove").clickable_tooltip(identity_unload_tip(qualified_identity)).clicked() { + // Same disclosure as Identity Hub → Settings: + // this permanently unloads the identity and + // deletes its private keys on this device. + // This list also removes nodes, so it needs + // the voting-identity variant. + match self.app_context.scheduled_vote_count_for_identity(&qualified_identity.identity.id()) { + Ok(scheduled_vote_count) => { + self.identity_to_remove = + Some(qualified_identity.clone()); + self.remove_confirmation_dialog = Some( + identity_removal_confirmation_dialog( + qualified_identity, + scheduled_vote_count, + ), + ); + } + Err(error) => { + MessageBanner::set_global( + ui.ctx(), + UNLOAD_DETAILS_LOAD_FAILED, + MessageType::Error, + ) + .with_details(&error); + } + } } // Up arrow @@ -1128,29 +1141,12 @@ impl ScreenLike for IdentitiesScreen { ); } } - crate::ui::BackendTaskSuccessResult::RemovedIdentities { - identity_ids, - associated_cleanup_failed, - } => { + crate::ui::BackendTaskSuccessResult::RemovedIdentities { identity_ids, .. } => { + // `AppState` owns the outcome banner, for every dispatcher. let mut identities = self.identities.lock_recover(); for identity_id in identity_ids { identities.shift_remove(&identity_id); } - drop(identities); - if associated_cleanup_failed { - MessageBanner::set_global( - self.app_context.egui_ctx(), - "The identity was removed, but its associated voter identity could not be removed. Retry after restarting the app.", - MessageType::Warning, - ) - .disable_auto_dismiss(); - } else { - MessageBanner::set_global( - self.app_context.egui_ctx(), - "The identity was removed from this device.", - MessageType::Success, - ); - } } _ => {} } @@ -1262,9 +1258,52 @@ mod tests { use super::render_identity_name_cell; use crate::model::contested_name::PendingUsername; use crate::ui::components::pill::PENDING_USERNAME_PILL_LABEL; + use crate::ui::identity::settings::identity_removal_confirmation_message; use egui_kittest::Harness; use egui_kittest::kittest::Queryable; + /// This list removes masternodes and evonodes too, so its Remove dialog must + /// carry the same voting-identity disclosure as the masternode detail view. + #[test] + fn remove_dialog_discloses_the_voting_identity_for_a_node() { + use crate::model::qualified_identity::encrypted_key_storage::KeyStorage; + use crate::model::qualified_identity::{IdentityStatus, IdentityType, QualifiedIdentity}; + use dash_sdk::dpp::dashcore::Network; + use dash_sdk::dpp::identity::Identity; + use dash_sdk::dpp::version::PlatformVersion; + use dash_sdk::platform::{Identifier, IdentityPublicKey}; + use std::collections::BTreeMap; + + let pv = PlatformVersion::latest(); + let node = QualifiedIdentity { + identity: Identity::create_basic_identity(Identifier::from([0xE1; 32]), pv) + .expect("basic identity"), + associated_voter_identity: Some(( + Identity::create_basic_identity(Identifier::from([0xE2; 32]), pv) + .expect("voter identity"), + IdentityPublicKey::random_key(1, Some(1), pv), + )), + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::Masternode, + alias: None, + private_keys: KeyStorage::default(), + dpns_names: vec![], + associated_wallets: BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::Active, + network: Network::Testnet, + }; + + let message = identity_removal_confirmation_message(&node, 0); + assert!( + message.ends_with("This also removes the node's voting identity from this device."), + "removing a node from the Identities list must disclose the voting identity: {message}" + ); + } + /// The Identities list Name cell shows the identity's name and, when a DPNS /// registration is pending, a "Pending" pill beside it. #[test] diff --git a/src/ui/identity/hub_screen.rs b/src/ui/identity/hub_screen.rs index 0a1e12728..03773826d 100644 --- a/src/ui/identity/hub_screen.rs +++ b/src/ui/identity/hub_screen.rs @@ -534,6 +534,19 @@ impl ScreenLike for IdentityHubScreen { *saved_id, ); } + BackendTaskSuccessResult::UnloadedIdentity(identity_id) => { + self.profile_cache.remove_identity(identity_id); + MessageBanner::set_global( + self.app_context.egui_ctx(), + "This identity was unloaded from this device.", + MessageType::Success, + ); + } + BackendTaskSuccessResult::RemovedIdentities { identity_ids, .. } => { + for identity_id in identity_ids { + self.profile_cache.remove_identity(identity_id); + } + } // Populate the Received/Sent request caches so the Contacts tab // can render real RequestCard rows instead of hardcoded empties. // The result arrives from LoadContactRequests, @@ -587,6 +600,9 @@ impl ScreenLike for IdentityHubScreen { } fn display_task_error(&mut self, error: &TaskError) -> bool { + if let TaskError::IdentityUnloadCleanupFailed { identity_id, .. } = error { + self.profile_cache.remove_identity(identity_id); + } if self.handle_contact_request_error(error) { return matches!( contact_info_read_error_key(error), @@ -930,6 +946,81 @@ mod tests { ); } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn unload_cleanup_error_invalidates_in_flight_profile_result() { + let (_temp_dir, context) = wired_test_context().await; + let identity_id = seed_user_identity(&context, 3); + let identity = context + .get_local_qualified_identity(&identity_id) + .expect("read identity") + .expect("identity exists"); + let mut screen = IdentityHubScreen::new(&context); + + assert!(screen.profile_cache.get_or_request(&identity).is_none()); + assert!(matches!( + screen.profile_cache.dispatch_pending(), + AppAction::BackendTask(_) + )); + + let error = TaskError::IdentityUnloadCleanupFailed { + identity_id, + source: Box::new(TaskError::DocumentNotFound), + }; + assert!( + !screen.display_task_error(&error), + "AppState must still render the cleanup warning" + ); + assert!( + screen + .profile_cache + .record_result(&BackendTaskSuccessResult::DashPayProfile(Some(( + "Stale name".to_string(), + "Stale bio".to_string(), + "https://example.invalid/stale.png".to_string(), + )))) + ); + assert!( + screen.profile_cache.get_or_request(&identity).is_none(), + "a profile request started before the committed unload must be discarded" + ); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn removed_identities_invalidate_every_profile_cache_entry() { + let (_temp_dir, context) = wired_test_context().await; + let first_id = seed_user_identity(&context, 4); + let second_id = seed_user_identity(&context, 5); + let first = context + .get_local_qualified_identity(&first_id) + .expect("read first identity") + .expect("first identity exists"); + let second = context + .get_local_qualified_identity(&second_id) + .expect("read second identity") + .expect("second identity exists"); + let mut screen = IdentityHubScreen::new(&context); + + for identity_id in [first_id, second_id] { + screen.profile_cache.record_saved( + identity_id, + crate::ui::identity::profile_cache::ProfileFields { + display_name: "Cached name".to_string(), + ..Default::default() + }, + ); + } + + screen.display_task_result(BackendTaskSuccessResult::RemovedIdentities { + identity_ids: vec![first_id, second_id], + primary_cleanup_failed: false, + associated_cleanup_failed: false, + associated_removal_failed: false, + }); + + assert!(screen.profile_cache.get_or_request(&first).is_none()); + assert!(screen.profile_cache.get_or_request(&second).is_none()); + } + #[test] fn a_result_for_the_selected_identity_applies() { assert!(applies_to_selected_identity(Some(id(1)), &id(1))); diff --git a/src/ui/identity/profile_cache.rs b/src/ui/identity/profile_cache.rs index fb147a07d..af6d57028 100644 --- a/src/ui/identity/profile_cache.rs +++ b/src/ui/identity/profile_cache.rs @@ -42,9 +42,13 @@ pub struct ProfileCache { requested: HashSet, /// Identity of the in-flight load. The result variant carries no owner id, /// so it is associated with this id on arrival. - in_flight: Option, + in_flight: Option<(Identifier, u64, u64)>, /// Identities a tab asked for this frame that still need a load dispatched. wanted: Vec, + /// Invalidates every request dispatched before a full reset. + cache_generation: u64, + /// Invalidates requests dispatched before one identity was unloaded. + identity_generations: HashMap, } impl ProfileCache { @@ -77,7 +81,8 @@ impl ProfileCache { }; let id = identity.identity.id(); self.requested.insert(id); - self.in_flight = Some(id); + let identity_generation = self.identity_generations.get(&id).copied().unwrap_or(0); + self.in_flight = Some((id, self.cache_generation, identity_generation)); AppAction::BackendTask(BackendTask::DashPayTask(Box::new( DashPayTask::LoadProfile { identity }, ))) @@ -89,9 +94,15 @@ impl ProfileCache { let BackendTaskSuccessResult::DashPayProfile(data) = result else { return false; }; - let Some(id) = self.in_flight.take() else { + let Some((id, cache_generation, identity_generation)) = self.in_flight.take() else { return false; }; + self.requested.remove(&id); + let is_current = cache_generation == self.cache_generation + && identity_generation == self.identity_generations.get(&id).copied().unwrap_or(0); + if !is_current { + return true; + } let fields = data .clone() .map(|(display_name, bio, avatar_url)| ProfileFields { @@ -103,6 +114,19 @@ impl ProfileCache { true } + /// Remove one identity's cached and queued profile state. + /// + /// An already-dispatched request remains the single in-flight operation, + /// but its generation is invalidated so a late response is discarded. + pub fn remove_identity(&mut self, identity_id: &Identifier) { + self.loaded.remove(identity_id); + self.requested.remove(identity_id); + self.wanted + .retain(|identity| identity.identity.id() != *identity_id); + let generation = self.identity_generations.entry(*identity_id).or_default(); + *generation = generation.wrapping_add(1); + } + /// Optimistically record a just-saved profile so every tab reflects it /// immediately, without waiting for a re-fetch. /// @@ -114,7 +138,7 @@ impl ProfileCache { pub fn record_saved(&mut self, id: Identifier, fields: ProfileFields) { self.loaded.insert(id, Some(fields)); self.requested.remove(&id); - if self.in_flight == Some(id) { + if matches!(self.in_flight, Some((in_flight_id, _, _)) if in_flight_id == id) { self.in_flight = None; } self.wanted.retain(|q| q.identity.id() != id); @@ -124,7 +148,118 @@ impl ProfileCache { pub fn reset(&mut self) { self.loaded.clear(); self.requested.clear(); - self.in_flight = None; self.wanted.clear(); + self.in_flight = None; + self.cache_generation = self.cache_generation.wrapping_add(1); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::model::qualified_identity::{IdentityStatus, IdentityType}; + use dash_sdk::dpp::dashcore::Network; + use dash_sdk::dpp::version::PlatformVersion; + use dash_sdk::platform::Identity; + use std::collections::BTreeMap; + + fn qualified_identity(id: Identifier) -> QualifiedIdentity { + QualifiedIdentity { + identity: Identity::create_basic_identity(id, PlatformVersion::latest()) + .expect("create identity"), + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::User, + alias: None, + private_keys: Default::default(), + dpns_names: Vec::new(), + associated_wallets: BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::Active, + network: Network::Testnet, + } + } + + #[test] + fn late_profile_result_after_unload_does_not_repopulate_cache() { + let id = Identifier::from([0x41; 32]); + let identity = qualified_identity(id); + let mut cache = ProfileCache::default(); + + assert!(cache.get_or_request(&identity).is_none()); + assert!(matches!( + cache.dispatch_pending(), + AppAction::BackendTask(BackendTask::DashPayTask(_)) + )); + cache.loaded.insert( + id, + Some(ProfileFields { + display_name: "Previously cached".to_string(), + ..Default::default() + }), + ); + + cache.remove_identity(&id); + assert!( + cache.record_result(&BackendTaskSuccessResult::DashPayProfile(Some(( + "Stale name".to_string(), + "Stale bio".to_string(), + "https://example.invalid/stale.png".to_string(), + )))) + ); + + assert!( + !cache.loaded.contains_key(&id), + "a response started before unload must stay discarded" + ); + + assert!(cache.get_or_request(&identity).is_none()); + assert!(matches!( + cache.dispatch_pending(), + AppAction::BackendTask(BackendTask::DashPayTask(_)) + )); + assert!( + cache.record_result(&BackendTaskSuccessResult::DashPayProfile(Some(( + "Fresh name".to_string(), + "Fresh bio".to_string(), + "https://example.invalid/fresh.png".to_string(), + )))) + ); + assert_eq!( + cache + .loaded + .get(&id) + .and_then(Option::as_ref) + .map(|fields| fields.display_name.as_str()), + Some("Fresh name"), + "the next request after reload must populate normally" + ); + } + + #[test] + fn reset_unblocks_dispatch_after_profile_load_error() { + let failed_identity = qualified_identity(Identifier::from([0x42; 32])); + let next_identity = qualified_identity(Identifier::from([0x43; 32])); + let mut cache = ProfileCache::default(); + + assert!(cache.get_or_request(&failed_identity).is_none()); + assert!(matches!( + cache.dispatch_pending(), + AppAction::BackendTask(BackendTask::DashPayTask(_)) + )); + + cache.reset(); + + assert!(cache.get_or_request(&next_identity).is_none()); + assert!( + matches!( + cache.dispatch_pending(), + AppAction::BackendTask(BackendTask::DashPayTask(_)) + ), + "reset must abandon an unresolved request so another identity can load" + ); } } diff --git a/src/ui/identity/settings.rs b/src/ui/identity/settings.rs index 320f3c9b5..ccfd264b8 100644 --- a/src/ui/identity/settings.rs +++ b/src/ui/identity/settings.rs @@ -6,19 +6,14 @@ //! //! ## Backend integration //! -//! This tab is **additive** with respect to the backend: it dispatches only -//! backend tasks that already exist and never introduces new variants. As of -//! 2026-04-23 the following controls cannot be wired to a backend task and are +//! This tab dispatches backend tasks for supported settings actions. The +//! following controls do not have a backend task and are //! therefore feature-gated — rendered as non-interactive affordances with a //! `disabled_tooltip` explaining that the action is coming in a follow-up: //! //! - **Delete social profile** — no `DashPayTask::DeleteProfile` variant. //! - **Add / remove alias** and **Make primary** — no `IdentityTask::AddAlias` //! / `RemoveAlias` / `MakePrimaryAlias` variants. -//! - **Unload this identity from this device** — no identity-unload task; the -//! existing `wallet_lifecycle` unload path is wallet-scoped, not identity- -//! scoped, and wiring it here would bypass the dashpay / DPNS state cleanup -//! the operation implies. //! //! These appear as `Gated(missing_task)` non-interactive rows with the copy //! from design-spec §D (tooltip catalog entries #49 and #59). A TODO comment @@ -86,8 +81,12 @@ const TIP_ADD_KEY: &str = const TIP_MANAGE_KEYS: &str = "View this identity's keys and their security settings."; const TIP_VIEW_USERNAMES: &str = "Open the complete list of your registered usernames."; const TIP_REFRESH: &str = "Fetch the latest state of this identity from the network."; -const TIP_UNLOAD: &str = "Remove this identity from this device. It remains on Dash Platform — you can load it \ - again later."; +const TIP_UNLOAD_WALLET_DERIVED: &str = "Remove this identity, its private keys, and its entry in this app from this \ + device. It remains on Dash Platform, and its wallet-derived private keys can be restored when you load it again."; +const TIP_UNLOAD_RECOVERY_REQUIRED: &str = "Remove this identity, its private keys, and its entry in this app from \ + this device. It remains on Dash Platform, but you will need its recovery information to load it again."; +const TIP_UNLOAD_NODE: &str = "Remove this node, its private keys, and its entry in this app from this device. It \ + remains on Dash Platform, and you can load it again using its ProTxHash."; const TIP_SAVE_ALIAS: &str = "Save this name on this device."; const TIP_ID_COPY: &str = "Copy the full identity ID to your clipboard."; @@ -100,6 +99,34 @@ const ALIAS_HINT: &str = "For example: My main identity"; const ALIAS_SAVED: &str = "Name saved on this device."; const ALIAS_SAVE_FAILED: &str = "This name could not be saved on your device. Try again in a moment."; +pub(crate) const UNLOAD_DETAILS_LOAD_FAILED: &str = + "The unload details could not be loaded. Try again in a moment."; +/// The removal consequence carried only by an identity that has a voting +/// identity — masternodes and evonodes, on every screen that removes them. +const REMOVE_VOTING_IDENTITY_DISCLOSURE: &str = + "This also removes the node's voting identity from this device."; +// Dialog title and button labels per identity kind. Each names the action it +// performs, because a destructive confirmation answered with "Yes" tells the +// user nothing about what they are agreeing to (docs/ux-design-patterns.md §4). +const UNLOAD_IDENTITY_TITLE: &str = "Unload this identity"; +const UNLOAD_IDENTITY_CONFIRM: &str = "Permanently unload"; +const UNLOAD_IDENTITY_CANCEL: &str = "Keep identity"; +const REMOVE_MASTERNODE: &str = "Remove masternode"; +const KEEP_MASTERNODE: &str = "Keep masternode"; +const REMOVE_EVONODE: &str = "Remove evonode"; +const KEEP_EVONODE: &str = "Keep evonode"; +// How the identity is restored after the unload — one complete statement per +// identity kind. Nodes are loaded by ProTxHash and are never wallet-derived, so +// they never carry the wallet or recovery-information wording; the ProTxHash +// restores the entry, never the keys this action deletes. +const USER_RESTORATION_RECOVERY_REQUIRED: &str = + "It remains on Dash Platform, but you will need its recovery information to load it again."; +const USER_RESTORATION_WALLET_DERIVED: &str = "It remains on Dash Platform, and its wallet-derived \ + private keys can be restored when you load it again."; +const NODE_RESTORATION_WITH_KEYS: &str = "It remains on Dash Platform, and you can load it again \ + using its ProTxHash, but the private keys deleted here must be entered again by hand."; +const NODE_RESTORATION_WATCH_ONLY: &str = + "It remains on Dash Platform, and you can load it again using its ProTxHash."; const TIP_PROTX_COPY: &str = "Copy the masternode ID to your clipboard."; // Marker strings for controls without a matching backend task. Surfaced in // disabled_tooltip and as a prefix on the row so users know it is a coming @@ -118,6 +145,11 @@ use crate::model::dashpay::{ // Stateful tab component // --------------------------------------------------------------------------- +struct PendingIdentityUnload { + dialog: ConfirmationDialog, + target_id: Identifier, +} + /// Settings tab state. Holds the currently-selected identity (picked on /// construction) plus per-field edit state. Follows the project's stateful-UI /// pattern used by `ProfileScreen`: form fields, dirty tracking, confirmation @@ -154,8 +186,8 @@ pub struct SettingsTab { advanced_open: bool, /// Confirmation dialog for the (gated) "Delete social profile" action. confirm_delete_profile: Option, - /// Confirmation dialog for the (gated) "Unload this identity" action. - confirm_unload: Option, + /// Confirmation dialog for the destructive "Unload this identity" action. + confirm_unload: Option, /// Track whether we have loaded the cached profile for the current /// identity. Reset on identity change. profile_loaded: bool, @@ -232,7 +264,7 @@ impl SettingsTab { }); // Dialogs on top. - action |= self.show_gated_dialogs(ui); + action |= self.show_confirmation_dialogs(ui); action } @@ -746,26 +778,30 @@ impl SettingsTab { .color(DashColors::text_secondary(dark_mode)), ); ui.add_space(6.0); - // TODO(identity-hub): wire once an identity-scoped unload task - // exists. Wallet-scoped unload (wallet_lifecycle) is too broad - // — it would silently drop sibling identities on the same wallet. - let unload = ui - .add_enabled( - false, - ComponentStyles::danger_button("Unload this identity from this device"), - ) - .disabled_tooltip(format!("{TIP_UNLOAD} {GATED_COMING_SOON}")); + let unload = + ComponentStyles::add_danger_button(ui, "Unload this identity from this device") + .clickable_tooltip(identity_unload_tip(identity)); if unload.clicked() { - self.confirm_unload = Some( - ConfirmationDialog::new( - "Unload this identity", - "This removes the identity from this device. It remains on Dash \ - Platform — you can load it again later.", - ) - .confirm_text(Some("Unload")) - .cancel_text(Some("Keep")) - .danger_mode(true), - ); + let target_id = identity.identity.id(); + match app_context.scheduled_vote_count_for_identity(&target_id) { + Ok(scheduled_vote_count) => { + self.confirm_unload = Some(PendingIdentityUnload { + dialog: identity_unload_confirmation_dialog( + identity, + scheduled_vote_count, + ), + target_id, + }); + } + Err(error) => { + MessageBanner::set_global( + ui.ctx(), + UNLOAD_DETAILS_LOAD_FAILED, + MessageType::Error, + ) + .with_details(&error); + } + } } }); @@ -776,7 +812,7 @@ impl SettingsTab { // Dialog handling // ----------------------------------------------------------------- - fn show_gated_dialogs(&mut self, ui: &mut Ui) -> AppAction { + fn show_confirmation_dialogs(&mut self, ui: &mut Ui) -> AppAction { if let Some(dialog) = self.confirm_delete_profile.as_mut() { match dialog.show(ui).inner.dialog_response { Some(ConfirmationStatus::Confirmed) | Some(ConfirmationStatus::Canceled) => { @@ -786,9 +822,14 @@ impl SettingsTab { } } - if let Some(dialog) = self.confirm_unload.as_mut() { - match dialog.show(ui).inner.dialog_response { - Some(ConfirmationStatus::Confirmed) | Some(ConfirmationStatus::Canceled) => { + if let Some(pending) = self.confirm_unload.as_mut() { + match pending.dialog.show(ui).inner.dialog_response { + Some(ConfirmationStatus::Confirmed) => { + let target_id = pending.target_id; + self.confirm_unload = None; + return confirmed_unload_action(target_id); + } + Some(ConfirmationStatus::Canceled) => { self.confirm_unload = None; } None => {} @@ -854,6 +895,8 @@ impl SettingsTab { if !changed { self.selected_identity = incoming.clone(); + } else { + self.confirm_unload = None; } changed @@ -969,6 +1012,246 @@ fn keys_screen_type(identity: &QualifiedIdentity) -> ScreenType { ScreenType::Keys(identity.identity.clone()) } +fn identity_unload_label(identity: &QualifiedIdentity) -> String { + identity + .alias + .as_deref() + .map(str::trim) + .filter(|alias| !alias.is_empty()) + .map(str::to_owned) + .unwrap_or_else(|| identity.identity.id().to_string(Encoding::Base58)) +} + +/// Hover text for the control that unloads or removes `identity`. Shared so no +/// screen invents a wording that contradicts the dialog it opens, and branched +/// by identity kind for the same reason the dialog is: a node has no wallet to +/// restore keys from, so the wallet-derived wording never applies to it. +pub(crate) fn identity_unload_tip(identity: &QualifiedIdentity) -> &'static str { + match identity.identity_type { + IdentityType::Masternode | IdentityType::Evonode => TIP_UNLOAD_NODE, + IdentityType::User => { + identity_unload_tip_for(identity.requires_recovery_information_after_unload()) + } + } +} + +fn identity_unload_tip_for(recovery_information_required: bool) -> &'static str { + if recovery_information_required { + TIP_UNLOAD_RECOVERY_REQUIRED + } else { + TIP_UNLOAD_WALLET_DERIVED + } +} + +/// Confirmation body for unloading `identity`, naming what is deleted, what is +/// kept, and how many scheduled votes the unload cancels. Shared by every screen +/// that unloads or removes an identity so the disclosure cannot drift between +/// them. +pub(crate) fn identity_unload_confirmation_message( + identity: &QualifiedIdentity, + scheduled_vote_count: usize, +) -> String { + let identity_label = identity_unload_label(identity); + let base58_id = identity.identity.id().to_string(Encoding::Base58); + // Aliases are user-set and not unique, so a confirmation for an irreversible + // action must carry the id too. A label that already is the id says it once. + let identity_id = (identity_label != base58_id).then_some(base58_id.as_str()); + identity_unload_confirmation_message_for( + &identity_label, + identity_id, + identity_restoration_clause(identity), + scheduled_vote_count, + ) +} + +/// How `identity` is restored after the unload. Chosen by identity kind and +/// composed into the message once — a correction appended after a clause written +/// for a different kind of identity reads as a contradiction, not a correction. +fn identity_restoration_clause(identity: &QualifiedIdentity) -> &'static str { + let holds_unrecoverable_keys = identity.requires_recovery_information_after_unload(); + match identity.identity_type { + IdentityType::Masternode | IdentityType::Evonode if holds_unrecoverable_keys => { + NODE_RESTORATION_WITH_KEYS + } + IdentityType::Masternode | IdentityType::Evonode => NODE_RESTORATION_WATCH_ONLY, + IdentityType::User if holds_unrecoverable_keys => USER_RESTORATION_RECOVERY_REQUIRED, + IdentityType::User => USER_RESTORATION_WALLET_DERIVED, + } +} + +/// Removal confirmation: the shared unload disclosure, plus the voting-identity +/// consequence for an identity that carries one (masternodes and evonodes). +/// Safe for any identity type — an identity without a voter identity gets the +/// plain unload disclosure. +pub(crate) fn identity_removal_confirmation_message( + identity: &QualifiedIdentity, + scheduled_vote_count: usize, +) -> String { + let message = identity_unload_confirmation_message(identity, scheduled_vote_count); + match identity.associated_voter_identity { + Some(_) => format!("{message}\n\n{REMOVE_VOTING_IDENTITY_DISCLOSURE}"), + None => message, + } +} + +/// Outcome of a completed identity removal, as text for the user and the banner +/// severity to show it at. `AppState` calls this for whichever screen dispatched +/// the removal, so the wording is the same wherever the removal was started. +/// +/// Each arm states what was removed, what may remain, and the retry the user can +/// perform themselves. The three cleanup flags are the only difference between a +/// clean removal and one that left keys on disk. +pub(crate) fn identity_removal_message( + primary_cleanup_failed: bool, + associated_cleanup_failed: bool, + associated_removal_failed: bool, +) -> (&'static str, MessageType) { + match ( + primary_cleanup_failed, + associated_cleanup_failed, + associated_removal_failed, + ) { + (false, false, false) => ( + "The identity was removed from this device.", + MessageType::Success, + ), + (true, false, false) => ( + "The identity was removed, but some local data could not be cleaned up. Load and remove it again to retry.", + MessageType::Warning, + ), + (false, true, false) => ( + "The identity and its associated voter identity were removed, but some local voter data could not be cleaned up. Load and remove the identity again to retry the cleanup.", + MessageType::Warning, + ), + (true, true, false) => ( + "The identity and its associated voter identity were removed, but some local data could not be cleaned up. Load and remove both identities again to retry.", + MessageType::Warning, + ), + (false, false, true) => ( + "The identity was removed, but its associated voter identity is still on this device. Wait a moment, then load and remove the identity again to retry.", + MessageType::Warning, + ), + (true, false, true) => ( + "The identity was removed, but some local data could not be cleaned up and its associated voter identity is still on this device. Restart the app, then load and remove the identity again to retry.", + MessageType::Warning, + ), + (_, true, true) => ( + "The identity was removed, but the associated voter identity may still have local data on this device. Restart the app, then load and remove the identity again to retry.", + MessageType::Warning, + ), + } +} + +/// Title and button labels of an unload or removal confirmation. `confirm` also +/// labels the control that opens the confirmation, so the button a user clicks +/// and the button they confirm with cannot name different actions. +pub(crate) struct UnloadDialogLabels { + pub(crate) title: &'static str, + pub(crate) confirm: &'static str, + pub(crate) cancel: &'static str, +} + +/// The wording every unload and removal control uses for `identity_type`. +/// Derived from the identity kind rather than supplied by the screen, so no +/// call site can invent labels for an action that deletes private keys. +pub(crate) fn unload_dialog_labels(identity_type: IdentityType) -> UnloadDialogLabels { + match identity_type { + IdentityType::Masternode => UnloadDialogLabels { + title: REMOVE_MASTERNODE, + confirm: REMOVE_MASTERNODE, + cancel: KEEP_MASTERNODE, + }, + IdentityType::Evonode => UnloadDialogLabels { + title: REMOVE_EVONODE, + confirm: REMOVE_EVONODE, + cancel: KEEP_EVONODE, + }, + IdentityType::User => UnloadDialogLabels { + title: UNLOAD_IDENTITY_TITLE, + confirm: UNLOAD_IDENTITY_CONFIRM, + cancel: UNLOAD_IDENTITY_CANCEL, + }, + } +} + +/// Confirmation dialog for unloading `identity` from this device. +/// +/// Danger-styled and input-blocking: the action deletes private keys and cannot +/// be undone, so it must not be answerable by a stray click on the screen +/// behind it. Use [`identity_removal_confirmation_dialog`] where the action also +/// removes the identity's voting identity. +pub(crate) fn identity_unload_confirmation_dialog( + identity: &QualifiedIdentity, + scheduled_vote_count: usize, +) -> ConfirmationDialog { + unload_dialog( + identity, + identity_unload_confirmation_message(identity, scheduled_vote_count), + ) +} + +/// Confirmation dialog for removing `identity` and the voting identity it +/// carries. Same treatment and labels as [`identity_unload_confirmation_dialog`], +/// with the voting-identity consequence added to the disclosure. +pub(crate) fn identity_removal_confirmation_dialog( + identity: &QualifiedIdentity, + scheduled_vote_count: usize, +) -> ConfirmationDialog { + unload_dialog( + identity, + identity_removal_confirmation_message(identity, scheduled_vote_count), + ) +} + +fn unload_dialog(identity: &QualifiedIdentity, message: String) -> ConfirmationDialog { + let labels = unload_dialog_labels(identity.identity_type); + ConfirmationDialog::new(labels.title, message) + .confirm_text(Some(labels.confirm)) + .cancel_text(Some(labels.cancel)) + .danger_mode(true) + .blocks_input(true) +} + +fn identity_unload_confirmation_message_for( + identity_label: &str, + identity_id: Option<&str>, + restoration: &str, + scheduled_vote_count: usize, +) -> String { + // Complete sentences of their own, so each stays one translation unit rather + // than a fragment glued into a neighbouring sentence. The identification is + // empty when the identity is already named by its id. + let identity_identification = match identity_id { + Some(identity_id) => format!(" Its full identifier is {identity_id}."), + None => String::new(), + }; + match scheduled_vote_count > 0 { + true => format!( + "Identity \"{identity_label}\" will be permanently unloaded from this device, \ + deleting its private keys and its entry in this app.{identity_identification} Some \ + synced network data, such \ + as contacts and payment history, is removed only by the \"Clear Database\" action in \ + Settings. This app remembers that you unloaded this identity, so no search brings it \ + back. {restoration} This also cancels {scheduled_vote_count} \ + scheduled vote(s)." + ), + false => format!( + "Identity \"{identity_label}\" will be permanently unloaded from this device, \ + deleting its private keys and its entry in this app.{identity_identification} Some \ + synced network data, such \ + as contacts and payment history, is removed only by the \"Clear Database\" action in \ + Settings. This app remembers that you unloaded this identity, so no search brings it \ + back. {restoration}" + ), + } +} + +fn confirmed_unload_action(target_id: Identifier) -> AppAction { + AppAction::BackendTask(BackendTask::IdentityTask(IdentityTask::UnloadIdentity { + identity_id: target_id, + })) +} + fn usernames_screen_action() -> AppAction { AppAction::SetMainScreenThenGoToMainScreen(RootScreenType::RootScreenDPNSOwnedNames) } @@ -1056,16 +1339,23 @@ fn string_if_set(s: &str) -> Option { #[cfg(test)] mod tests { use super::*; + use crate::model::qualified_identity::PrivateKeyTarget; + use crate::model::qualified_identity::encrypted_key_storage::{ + PrivateKeyData, WalletDerivationPath, + }; + use crate::model::qualified_identity::qualified_identity_public_key::QualifiedIdentityPublicKey; use crate::model::qualified_identity::{IdentityStatus, IdentityType}; use dash_sdk::dpp::dashcore::Network; use dash_sdk::dpp::identity::Identity; + use dash_sdk::dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; + use dash_sdk::dpp::key_wallet::bip32::DerivationPath; use dash_sdk::dpp::version::PlatformVersion; use dash_sdk::platform::{Identifier, IdentityPublicKey}; use std::collections::BTreeMap; - fn qualified_identity() -> QualifiedIdentity { + fn qualified_identity_with(byte: u8, alias: Option<&str>) -> QualifiedIdentity { let identity = Identity::create_basic_identity( - Identifier::from_bytes(&[7; 32]).expect("32-byte identifier"), + Identifier::from_bytes(&[byte; 32]).expect("32-byte identifier"), PlatformVersion::latest(), ) .expect("basic identity"); @@ -1075,7 +1365,7 @@ mod tests { associated_operator_identity: None, associated_owner_key_id: None, identity_type: IdentityType::User, - alias: None, + alias: alias.map(str::to_owned), private_keys: Default::default(), dpns_names: vec![], associated_wallets: BTreeMap::new(), @@ -1087,6 +1377,10 @@ mod tests { } } + fn qualified_identity() -> QualifiedIdentity { + qualified_identity_with(7, None) + } + #[test] fn default_has_no_identity_selected() { let tab = SettingsTab::new(); @@ -1136,6 +1430,448 @@ mod tests { ); } + #[test] + fn identity_change_clears_an_open_unload_confirmation() { + let mut tab = SettingsTab::new(); + tab.selected_identity = Some(qualified_identity_with(7, Some("Primary"))); + tab.confirm_unload = Some(PendingIdentityUnload { + dialog: ConfirmationDialog::new("Unload", "Confirm unload"), + target_id: tab + .selected_identity + .as_ref() + .expect("selected identity") + .identity + .id(), + }); + + let changed = tab.reconcile_selected_identity(&Some(qualified_identity_with(8, None))); + + assert!(changed, "the selected identity changed"); + assert!( + tab.confirm_unload.is_none(), + "the old identity's unload confirmation must be dismissed" + ); + } + + #[test] + fn confirmed_unload_action_uses_the_captured_target() { + let captured_target = Identifier::from([0x41; 32]); + let newly_selected = Identifier::from([0x42; 32]); + + let action = confirmed_unload_action(captured_target); + + assert!(matches!( + action, + AppAction::BackendTask(BackendTask::IdentityTask(IdentityTask::UnloadIdentity { + identity_id, + })) if identity_id == captured_target && identity_id != newly_selected + )); + } + + #[test] + fn unload_dialog_label_prefers_alias_and_falls_back_to_base58_id() { + let aliased = qualified_identity_with(9, Some(" Daily identity ")); + let unaliased = qualified_identity_with(10, None); + + assert_eq!(identity_unload_label(&aliased), "Daily identity"); + assert_eq!( + identity_unload_label(&unaliased), + unaliased.identity.id().to_string(Encoding::Base58) + ); + } + + #[test] + fn unload_dialog_omits_recovery_warning_for_wallet_derived_keys() { + assert_eq!( + identity_unload_confirmation_message_for( + "Wallet identity", + None, + USER_RESTORATION_WALLET_DERIVED, + 0, + ), + "Identity \"Wallet identity\" will be permanently unloaded from this device, \ + deleting its private keys and its entry in this app. Some synced network data, such \ + as contacts and payment history, is removed only by the \"Clear Database\" action in \ + Settings. This app remembers that you unloaded this identity, so no search brings it \ + back. It remains on Dash Platform, and its wallet-derived private \ + keys can be restored when you load it again." + ); + assert_eq!(identity_unload_tip_for(false), TIP_UNLOAD_WALLET_DERIVED); + } + + /// Unloading is the action a user takes to sever a device↔identity link, so + /// the dialog must disclose what it does NOT remove and that the app keeps a + /// record of the unload — on every variant, not just one. + #[test] + fn unload_dialog_discloses_retained_data_and_the_remembered_unload() { + for restoration in [ + USER_RESTORATION_RECOVERY_REQUIRED, + USER_RESTORATION_WALLET_DERIVED, + NODE_RESTORATION_WITH_KEYS, + NODE_RESTORATION_WATCH_ONLY, + ] { + for scheduled_vote_count in [0, 2] { + let message = identity_unload_confirmation_message_for( + "Disclosure identity", + None, + restoration, + scheduled_vote_count, + ); + assert!( + message.contains("deleting its private keys and its entry in this app"), + "the dialog must name what is actually deleted: {message}" + ); + assert!( + message.contains( + "Some synced network data, such as contacts and payment history, is \ + removed only by the \"Clear Database\" action in Settings." + ), + "the dialog must name the data it leaves behind and where to remove it: \ + {message}" + ); + assert!( + message.contains( + "This app remembers that you unloaded this identity, so no search \ + brings it back." + ), + "the dialog must disclose the durable record of the unload: {message}" + ); + assert!( + !message.contains("deleting its local data"), + "the dialog must not overstate the removal as all local data: {message}" + ); + // No discovery pass restores a forgotten identity, so a + // disclosure that qualifies the promise — "automatic discovery", + // "background sync" — invites the user to expect a manual search + // to undo what they are about to authorize. + assert!( + !message.contains("automatic"), + "the record of the unload holds against every search, not just automatic \ + ones: {message}" + ); + } + } + } + + #[test] + fn unload_dialog_mentions_scheduled_votes_only_when_queued() { + assert!( + identity_unload_confirmation_message_for( + "Voting identity", + None, + USER_RESTORATION_RECOVERY_REQUIRED, + 3, + ) + .contains("This also cancels 3 scheduled vote(s).") + ); + assert!( + !identity_unload_confirmation_message_for( + "Voting identity", + None, + USER_RESTORATION_RECOVERY_REQUIRED, + 0, + ) + .contains("scheduled vote") + ); + } + + /// Aliases are user-set and not unique, so a confirmation that named only + /// the alias could not tell two identically-aliased identities apart — on an + /// action that permanently deletes keys. + #[test] + fn unload_dialog_names_the_id_alongside_an_alias() { + let aliased = qualified_identity_with(13, Some("Shared alias")); + let aliased_id = aliased.identity.id().to_string(Encoding::Base58); + let message = identity_unload_confirmation_message(&aliased, 0); + assert!( + message.contains("Identity \"Shared alias\" will be"), + "an aliased identity must still be named by its alias: {message}" + ); + assert!( + message.contains(&format!("Its full identifier is {aliased_id}.")), + "an aliased identity must also be named by its id, as its own sentence: {message}" + ); + + let unaliased = qualified_identity_with(14, None); + let unaliased_id = unaliased.identity.id().to_string(Encoding::Base58); + let message = identity_unload_confirmation_message(&unaliased, 0); + assert!( + message.starts_with(&format!("Identity \"{unaliased_id}\" will be")), + "an unaliased identity is named by its id: {message}" + ); + assert!( + !message.contains("Its full identifier is"), + "an identity already named by its id must not repeat it: {message}" + ); + } + + /// A node is never wallet-derived and is loaded by ProTxHash, so neither the + /// wallet-derived nor the recovery-information wording may reach it — in + /// either of its two real states. Asserting only on an appended sentence + /// misses a contradictory claim made earlier in the same message, so these + /// assert the whole composed string. + #[test] + fn unload_dialog_gives_nodes_a_restoration_clause_that_fits_them() { + for identity_type in [IdentityType::Masternode, IdentityType::Evonode] { + // Watch-only: no keys imported, so there is nothing to recover and + // no key-restoration claim to make. + let mut watch_only = qualified_identity_with(17, Some("Node")); + watch_only.identity_type = identity_type; + assert!( + !watch_only.requires_recovery_information_after_unload(), + "fixture check: a keyless node needs no recovery information" + ); + let message = identity_unload_confirmation_message(&watch_only, 0); + assert!( + message.ends_with(NODE_RESTORATION_WATCH_ONLY), + "a watch-only {identity_type:?} must simply be loadable again: {message}" + ); + + // Keys present: they are deleted here and are not derivable from the + // ProTxHash, so the message must say they are re-entered by hand. + let mut keyed = watch_only.clone(); + let key = IdentityPublicKey::random_key(1, Some(1), PlatformVersion::latest()); + keyed.private_keys.private_keys.insert( + (PrivateKeyTarget::PrivateKeyOnMainIdentity, key.id()), + ( + QualifiedIdentityPublicKey::from(key), + PrivateKeyData::InVault, + ), + ); + assert!( + keyed.requires_recovery_information_after_unload(), + "fixture check: a node holding its own keys has no wallet to restore them" + ); + let message = identity_unload_confirmation_message(&keyed, 0); + assert!( + message.ends_with(NODE_RESTORATION_WITH_KEYS), + "a keyed {identity_type:?} must be told its keys are re-entered: {message}" + ); + + for message in [ + identity_unload_confirmation_message(&watch_only, 0), + identity_unload_confirmation_message(&keyed, 0), + ] { + assert!( + !message.contains("wallet-derived"), + "a {identity_type:?} is never wallet-derived: {message}" + ); + assert!( + !message.contains("recovery information"), + "a {identity_type:?} has no recovery information to ask for: {message}" + ); + } + assert_eq!( + identity_unload_tip(&keyed), + TIP_UNLOAD_NODE, + "the tooltip must not promise a node's keys come back from a wallet" + ); + } + + let user = identity_unload_confirmation_message( + &qualified_identity_with(18, Some("User identity")), + 0, + ); + assert!( + !user.contains("ProTxHash"), + "an ordinary identity must not be given node guidance: {user}" + ); + } + + /// The removal variant is used for every identity type, so it must add the + /// voting-identity consequence only for an identity that has one. + #[test] + fn removal_dialog_names_the_voting_identity_only_when_one_exists() { + let without_voter = identity_removal_confirmation_message( + &qualified_identity_with(15, Some("Plain identity")), + 0, + ); + assert!( + without_voter.contains("will be permanently unloaded from this device"), + "removal must reuse the shared unload disclosure: {without_voter}" + ); + assert!( + !without_voter.contains(REMOVE_VOTING_IDENTITY_DISCLOSURE), + "an identity without a voting identity must not claim one is removed: {without_voter}" + ); + + let mut node = qualified_identity_with(16, Some("Node")); + node.identity_type = crate::model::qualified_identity::IdentityType::Masternode; + node.associated_voter_identity = Some(( + Identity::create_basic_identity( + Identifier::from([0xD4; 32]), + PlatformVersion::latest(), + ) + .expect("voter identity"), + IdentityPublicKey::random_key(1, Some(1), PlatformVersion::latest()), + )); + let with_voter = identity_removal_confirmation_message(&node, 2); + assert!( + with_voter.ends_with(REMOVE_VOTING_IDENTITY_DISCLOSURE), + "a node with a voting identity must disclose its removal: {with_voter}" + ); + assert!( + with_voter.contains("This also cancels 2 scheduled vote(s)."), + "the shared scheduled-vote clause must survive the addition: {with_voter}" + ); + } + + #[test] + fn identity_removal_messages_distinguish_cleanup_outcomes() { + let (message, message_type) = identity_removal_message(false, false, false); + assert_eq!(message, "The identity was removed from this device."); + assert_eq!(message_type, MessageType::Success); + + let (message, message_type) = identity_removal_message(true, false, false); + assert!(message.contains("some local data could not be cleaned up")); + assert!(!message.contains("associated voter identity")); + assert_eq!(message_type, MessageType::Warning); + + let (message, message_type) = identity_removal_message(false, true, false); + assert!(message.contains("associated voter identity were removed")); + assert!(message.contains("could not be cleaned up")); + assert_eq!(message_type, MessageType::Warning); + + let (message, message_type) = identity_removal_message(false, false, true); + assert!(message.contains("associated voter identity is still on this device")); + assert!(!message.contains("local data could not be cleaned up")); + assert!(message.contains("Wait a moment")); + assert!(!message.contains("Restart the app")); + assert_eq!(message_type, MessageType::Warning); + + let (message, message_type) = identity_removal_message(true, false, true); + assert!(message.contains("some local data could not be cleaned up")); + assert!(message.contains("associated voter identity is still on this device")); + assert_eq!(message_type, MessageType::Warning); + + // Both cleanup flags set (primary and associated cleanup left residue, + // but nothing was left un-removed), then the `(_, true, true)` catch-all + // arm: `remove_identity` never sets `associated_cleanup_failed` and + // `associated_removal_failed` together, but the match covers all 8 + // combinations regardless. + let (message, message_type) = identity_removal_message(true, true, false); + assert!(message.contains("associated voter identity were removed")); + assert!(message.contains("some local data could not be cleaned up")); + assert!(message.contains("both identities")); + assert_eq!(message_type, MessageType::Warning); + + let (message, message_type) = identity_removal_message(false, true, true); + assert!(message.contains("may still have local data")); + assert_eq!(message_type, MessageType::Warning); + } + + /// A removal that left residue must never auto-dismiss its banner: the user + /// has a retry to perform and cannot act on a warning they did not see. + #[test] + fn only_a_clean_removal_reports_success() { + for (primary, associated_cleanup, associated_removal) in [ + (true, false, false), + (false, true, false), + (false, false, true), + (true, true, true), + ] { + let (_, message_type) = + identity_removal_message(primary, associated_cleanup, associated_removal); + assert_eq!( + message_type, + MessageType::Warning, + "a removal with residue must not report success", + ); + } + } + + /// Every unload and removal confirmation names the action on its buttons. + /// A generic Yes/No pair is forbidden for a destructive action, and the node + /// verbs are fixed copy the masternode detail view is specified against. + #[test] + fn unload_dialog_labels_name_the_action_for_every_identity_kind() { + for identity_type in [ + IdentityType::User, + IdentityType::Masternode, + IdentityType::Evonode, + ] { + let labels = unload_dialog_labels(identity_type); + for label in [labels.title, labels.confirm, labels.cancel] { + assert!( + !["Yes", "No", "OK", "Confirm", "Cancel"].contains(&label), + "{identity_type:?} must not answer a destructive action with {label:?}" + ); + } + } + + let node = unload_dialog_labels(IdentityType::Masternode); + assert_eq!(node.title, REMOVE_MASTERNODE); + assert_eq!(node.confirm, REMOVE_MASTERNODE); + assert_eq!( + unload_dialog_labels(IdentityType::Evonode).confirm, + REMOVE_EVONODE + ); + } + + /// Both dialog flavours are irreversible, so both register egui's modal + /// layer: a click on the screen behind must never reach the app while the + /// confirmation is open. + #[test] + fn unload_dialogs_block_input_behind_them() { + let identity = qualified_identity_with(19, Some("Blocking identity")); + for mut dialog in [ + identity_unload_confirmation_dialog(&identity, 0), + identity_removal_confirmation_dialog(&identity, 0), + ] { + let ctx = egui::Context::default(); + // Two passes: `set_modal_layer` is consumed at the end of the pass + // that registers it, so it is observable from the next one. + for _ in 0..2 { + let _ = ctx.run_ui(egui::RawInput::default(), |ui| { + dialog.show(ui); + }); + } + assert!( + ctx.memory(|memory| memory.top_modal_layer().is_some()), + "an unload confirmation must block input to the screen behind it" + ); + } + } + + #[test] + fn unload_dialog_warns_about_recovery_information_for_mixed_keys() { + let mut identity = qualified_identity_with(12, Some("Mixed identity")); + let derived_key = IdentityPublicKey::random_key(1, Some(1), PlatformVersion::latest()); + identity.private_keys.private_keys.insert( + (PrivateKeyTarget::PrivateKeyOnMainIdentity, derived_key.id()), + ( + QualifiedIdentityPublicKey::from(derived_key), + PrivateKeyData::AtWalletDerivationPath(WalletDerivationPath { + wallet_seed_hash: [0x12; 32], + derivation_path: DerivationPath::from(vec![]), + }), + ), + ); + let local_key = IdentityPublicKey::random_key(2, Some(2), PlatformVersion::latest()); + identity.private_keys.private_keys.insert( + (PrivateKeyTarget::PrivateKeyOnMainIdentity, local_key.id()), + ( + QualifiedIdentityPublicKey::from(local_key), + PrivateKeyData::InVault, + ), + ); + + let identity_id = identity.identity.id().to_string(Encoding::Base58); + assert_eq!( + identity_unload_confirmation_message(&identity, 0), + format!( + "Identity \"Mixed identity\" will be permanently unloaded from this device, \ + deleting its private keys and its entry in this app. Its full identifier is \ + {identity_id}. Some synced network data, such as contacts and payment history, \ + is removed only by the \"Clear Database\" action in Settings. This app remembers \ + that you unloaded this identity, so no search brings it back. \ + It remains on Dash Platform, but you will need its recovery information to load \ + it again." + ) + ); + assert_eq!(identity_unload_tip(&identity), TIP_UNLOAD_RECOVERY_REQUIRED); + } + #[test] fn has_changes_tracks_baseline() { let mut tab = SettingsTab::new(); diff --git a/src/ui/masternodes/detail_screen.rs b/src/ui/masternodes/detail_screen.rs index 0dd5d09e2..fe21d2ca6 100644 --- a/src/ui/masternodes/detail_screen.rs +++ b/src/ui/masternodes/detail_screen.rs @@ -35,6 +35,9 @@ use crate::ui::components::password_input::PasswordInput; use crate::ui::identities::keys::key_info_screen::KeyInfoScreen; use crate::ui::identity::identity_picker_card::draw_type_badge; use crate::ui::identity::identity_pill::shorten_id; +use crate::ui::identity::settings::{ + UNLOAD_DETAILS_LOAD_FAILED, identity_removal_confirmation_dialog, unload_dialog_labels, +}; use crate::ui::masternodes::card::{ PLATFORM_IDENTITY_STATUS_TOOLTIP, platform_identity_status_label, }; @@ -231,8 +234,6 @@ pub enum DetailOutcome { None, /// Return to the card list (`‹ All masternodes`). Back, - /// The node was removed — return to the list and reload. - Removed, /// Push a reused screen / navigate. Boxed because `AppAction` is large. Forward(Box), } @@ -441,8 +442,8 @@ impl MasternodeDetailView { outcome = DetailOutcome::Forward(Box::new(action)); } ui.add_space(12.0); - if self.render_remove_section(ui, dark_mode) { - outcome = DetailOutcome::Removed; + if let Some(action) = self.render_remove_section(ui, dark_mode) { + outcome = DetailOutcome::Forward(Box::new(action)); } }); @@ -953,69 +954,61 @@ impl MasternodeDetailView { action } - /// Returns `true` once the node has been removed. - fn render_remove_section(&mut self, ui: &mut Ui, _dark_mode: bool) -> bool { + /// Dispatch the shared removal task after confirmation. + fn render_remove_section(&mut self, ui: &mut Ui, _dark_mode: bool) -> Option { let migration_in_progress = self.app_context.migration_status().state().is_in_progress(); + // The trigger takes the same kind-derived verb as the confirmation it + // opens, so an evonode is never offered a masternode's wording. + let remove_verb = unload_dialog_labels(self.identity.identity_type).confirm; if ui - .add_enabled( - !migration_in_progress, - egui::Button::new("Remove masternode"), - ) + .add_enabled(!migration_in_progress, egui::Button::new(remove_verb)) .on_disabled_hover_text( - "Wait for the storage update to finish before removing this masternode.", + "Wait for the storage update to finish before removing this node.", ) .clicked() { - self.remove_dialog = Some( - ConfirmationDialog::new( - "Remove masternode", - "This removes the node and its voting identity from this device. \ - You can load it again later with its ProTxHash.", - ) - .danger_mode(true) - // §7 confirm verb (TC-US4-02). - .confirm_text(Some("Remove masternode")), - ); + // Same disclosure as Identity Hub → Settings, plus the voting + // identity this screen removes alongside the node. + let identity_id = self.identity.identity.id(); + match self + .app_context + .scheduled_vote_count_for_identity(&identity_id) + { + Ok(scheduled_vote_count) => { + self.remove_dialog = Some(identity_removal_confirmation_dialog( + &self.identity, + scheduled_vote_count, + )); + } + Err(error) => { + MessageBanner::set_global( + ui.ctx(), + UNLOAD_DETAILS_LOAD_FAILED, + MessageType::Error, + ) + .with_details(&error); + } + } } - let mut removed = false; + let mut action = None; if let Some(dialog) = self.remove_dialog.as_mut() { use crate::ui::components::component_trait::Component; let response = dialog.show(ui); if let Some(status) = response.inner.dialog_response { self.remove_dialog = None; if status == ConfirmationStatus::Confirmed { - removed = self.remove_node(ui.ctx()); + action = Some(Self::remove_node(self.identity.identity.id())); } } } - removed + action } - /// Delete the node and its associated voter identity from local storage. - /// On the primary delete failing, surface an actionable error banner rather - /// than failing silently, and keep the detail view open so the user can - /// retry. The secondary voter-identity delete failing is non-fatal (the node - /// is already gone) and only logged. - fn remove_node(&self, ctx: &egui::Context) -> bool { - let node_id = self.identity.identity.id(); - if let Err(e) = self.app_context.delete_local_qualified_identity(&node_id) { - MessageBanner::set_global( - ctx, - "This masternode couldn't be removed from this device. Try again in a moment.", - MessageType::Error, - ) - .with_details(e); - return false; - } - if let Some((voter, _)) = self.identity.associated_voter_identity.as_ref() - && let Err(e) = self - .app_context - .delete_local_qualified_identity(&voter.id()) - { - tracing::warn!("Failed to remove voter identity: {e}"); - } - true + fn remove_node(node_id: dash_sdk::platform::Identifier) -> AppAction { + AppAction::BackendTask(BackendTask::IdentityTask(IdentityTask::RemoveIdentity { + identity_id: node_id, + })) } } @@ -1041,6 +1034,17 @@ mod tests { ); } + #[test] + fn remove_node_dispatches_the_shared_identity_removal_task() { + let node_id = dash_sdk::platform::Identifier::from([0xD2; 32]); + assert!(matches!( + MasternodeDetailView::remove_node(node_id), + AppAction::BackendTask(BackendTask::IdentityTask( + IdentityTask::RemoveIdentity { identity_id } + )) if identity_id == node_id + )); + } + /// Build a masternode key with a chosen id / purpose / disabled state. fn mn_key( id: dash_sdk::dpp::identity::KeyID, diff --git a/src/ui/masternodes/list_screen.rs b/src/ui/masternodes/list_screen.rs index bae36f9a6..79effc0fb 100644 --- a/src/ui/masternodes/list_screen.rs +++ b/src/ui/masternodes/list_screen.rs @@ -389,11 +389,6 @@ impl MasternodesScreen { self.view = MasternodesView::List; AppAction::None } - DetailOutcome::Removed => { - self.view = MasternodesView::List; - self.reload(); - AppAction::None - } DetailOutcome::Forward(action) => *action, } } @@ -551,7 +546,17 @@ impl ScreenLike for MasternodesScreen { } } - fn display_task_result(&mut self, _result: crate::backend_task::BackendTaskSuccessResult) { + fn display_task_result(&mut self, result: crate::backend_task::BackendTaskSuccessResult) { + let removed_open_node = matches!( + (&self.view, &result), + ( + MasternodesView::Detail(detail), + crate::backend_task::BackendTaskSuccessResult::RemovedIdentities { + identity_ids, + .. + } + ) if identity_ids.contains(&detail.node_id()) + ); self.reload(); // Settle the submitted load against the phase its task reported. This // screen also receives detail-view results (voting, RefreshIdentity) and @@ -562,7 +567,9 @@ impl ScreenLike for MasternodesScreen { // Add-voting-key merge, a RefreshIdentity) just updated the store. // Re-open the detail view for that node so the on-screen view reflects // the fresh data instead of the stale clone captured at open time. - if let MasternodesView::Detail(detail) = &self.view { + if removed_open_node { + self.view = MasternodesView::List; + } else if let MasternodesView::Detail(detail) = &self.view { let node_id = detail.node_id(); self.open_detail(node_id); } @@ -672,6 +679,37 @@ mod tests { .expect("seed masternode"); } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn removal_result_closes_detail_while_failure_keeps_it_open() { + use crate::backend_task::BackendTaskSuccessResult; + use crate::backend_task::error::TaskError; + + let (ctx, _tmp) = offline_ctx().await; + let node_id = Identifier::from([0xD3; 32]); + seed_masternode(&ctx, 0xD3, None); + let mut screen = MasternodesScreen::new(&ctx); + screen.open_detail(node_id); + + assert!(!screen.display_task_error(&TaskError::WalletBackendNotYetWired)); + assert!( + matches!(screen.view, MasternodesView::Detail(_)), + "a failed removal must keep the detail view open" + ); + + screen.display_task_result(BackendTaskSuccessResult::RemovedIdentities { + identity_ids: vec![node_id], + primary_cleanup_failed: false, + associated_cleanup_failed: false, + associated_removal_failed: false, + }); + assert!( + matches!(screen.view, MasternodesView::List), + "a successful shared removal result must close the detail view" + ); + + ctx.wallet_backend().expect("backend").shutdown().await; + } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn nodes_are_sorted_case_insensitively_by_display_heading() { let (ctx, _tmp) = offline_ctx().await; diff --git a/src/wallet_backend/dashpay.rs b/src/wallet_backend/dashpay.rs index 80b255322..524074c92 100644 --- a/src/wallet_backend/dashpay.rs +++ b/src/wallet_backend/dashpay.rs @@ -1126,6 +1126,42 @@ impl WalletBackend { .map_err(|e| TaskError::DashpaySidecarStorage { source: e }) } + /// Delete the Global entity-timestamp entry keyed by `identity_id`. + /// + /// Payment timestamps and timestamps for other entities are not safely + /// owner-attributable, so only full-wallet teardown reclaims those entries. + pub fn dashpay_clear_identity_timestamps( + &self, + identity_id: &Identifier, + ) -> Result<(), TaskError> { + let key = sidecar_key(KV_PREFIX_TIMESTAMPS, identity_id); + self.kv() + .delete(DetScope::Global, &key) + .map_err(|e| TaskError::DashpaySidecarStorage { source: e }) + } + + /// Delete every Global reverse-address mapping owned by `identity_id`. + pub fn dashpay_clear_identity_addr_map( + &self, + identity_id: &Identifier, + ) -> Result<(), TaskError> { + use dash_sdk::dpp::platform_value::string_encoding::Encoding; + + let owner_prefix = format!( + "{KV_PREFIX_ADDR_MAP}{}:", + identity_id.to_string(Encoding::Base58) + ); + let kv = self.kv(); + let keys = kv + .list(DetScope::Global, Some(&owner_prefix)) + .map_err(|e| TaskError::DashpaySidecarStorage { source: e })?; + for key in keys { + kv.delete(DetScope::Global, &key) + .map_err(|e| TaskError::DashpaySidecarStorage { source: e })?; + } + Ok(()) + } + /// Write DET-local `(created_at_ms, confirmed_at_ms)` timestamps for a /// payment in the k/v sidecar, keyed by transaction id. Upstream /// `PaymentEntry` carries no timestamps of its own, so this is the @@ -1274,11 +1310,8 @@ impl WalletBackend { /// per-contact private memos, address-index cursors, the blocked / declined / /// withdrawn markers, and paid-action recovery journals. /// - /// The remaining Global-scoped overlays (timestamps, reverse address map) - /// are not owner-scoped and are swept by the `det:dashpay:` Global prefix in - /// [`crate::context::AppContext::clear_network_database`]; this method - /// covers the overlays that live under [`DetScope::Identity`] of the owner, - /// which that Global sweep can no longer reach. + /// Global-scoped overlays are cleared separately because this method covers + /// only the overlays under [`DetScope::Identity`] of the owner. pub fn dashpay_clear_owner_overlays(&self, owner: &Identifier) -> Result<(), TaskError> { let owner_buf = owner.to_buffer(); let scope = DetScope::Identity(&owner_buf); diff --git a/src/wallet_backend/kv.rs b/src/wallet_backend/kv.rs index 43f1cd062..d26a0a04d 100644 --- a/src/wallet_backend/kv.rs +++ b/src/wallet_backend/kv.rs @@ -18,10 +18,14 @@ //! `private` / `address_index` overlays are all identity-scoped. //! //! All keys carried by this adapter follow a colon-separated namespace -//! convention, with a mandatory `:` prefix for global slots so -//! mainnet / testnet / devnet entries cannot collide inside the same -//! upstream database file. See the documentation on the consumer -//! callers (e.g. settings storage) for the canonical key schema. +//! convention. Whether a key needs a `:` prefix depends on the +//! store behind it, not on the scope: entries in the cross-network +//! `det-app.sqlite` (wallet-meta and single-key sidecars, migration +//! sentinels) share one file across every network and must carry the +//! prefix so mainnet / testnet / devnet cannot collide. Entries in the +//! per-network `spv//platform-wallet.sqlite` get one file per +//! network already, so they omit it — see `det:identity_index:v1` and +//! its siblings. `docs/kv-keys.md` catalogues every key and its store. //! //! ## Encoding //! diff --git a/tests/kittest/identities_screen.rs b/tests/kittest/identities_screen.rs index ca37cdc1a..bd303a4f2 100644 --- a/tests/kittest/identities_screen.rs +++ b/tests/kittest/identities_screen.rs @@ -1,5 +1,15 @@ -use crate::support::with_isolated_data_dir; +use crate::support::{mount_app, with_isolated_data_dir}; +use dash_evo_tool::context::AppContext; +use dash_evo_tool::model::qualified_identity::encrypted_key_storage::KeyStorage; +use dash_evo_tool::model::qualified_identity::{IdentityStatus, IdentityType, QualifiedIdentity}; +use dash_evo_tool::ui::{RootScreenType, ScreenLike}; +use dash_sdk::dpp::identity::Identity; +use dash_sdk::dpp::version::PlatformVersion; +use dash_sdk::platform::Identifier; use egui_kittest::Harness; +use egui_kittest::kittest::Queryable; +use std::collections::BTreeMap; +use std::sync::Arc; /// Test that the identities screen can be rendered #[test] @@ -88,3 +98,70 @@ fn test_frame_batch_processing() { } }); } + +/// Seed one keyless user identity into the live per-network identity DB so the +/// list renders a row with its action buttons. +fn seed_user_identity(app_context: &Arc, byte: u8, alias: &str) { + let identity = + Identity::create_basic_identity(Identifier::from([byte; 32]), PlatformVersion::latest()) + .expect("basic identity"); + let qualified_identity = QualifiedIdentity { + identity, + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::User, + alias: Some(alias.to_string()), + private_keys: KeyStorage::default(), + dpns_names: vec![], + associated_wallets: BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::Active, + network: app_context.network(), + }; + app_context + .insert_local_qualified_identity(&qualified_identity, &None) + .expect("seed user identity"); +} + +/// The list's Remove confirmation is destructive and irreversible, so it must +/// carry the same specific verbs as the Identity Hub's unload confirmation — +/// generic Yes/No labels are forbidden for destructive actions. +#[test] +fn remove_confirmation_uses_specific_verbs() { + with_isolated_data_dir(|| { + let rt = tokio::runtime::Runtime::new().expect("Failed to create tokio runtime"); + let _guard = rt.enter(); + + let mut harness = mount_app(RootScreenType::RootScreenIdentities); + let app_context = harness.state().current_app_context().clone(); + seed_user_identity(&app_context, 0xB1, "list-remove-verbs"); + harness + .state_mut() + .active_root_screen_mut() + .refresh_on_arrival(); + harness.run_steps(3); + + harness.get_by_label("Remove").click(); + harness.run_steps(3); + + assert!( + harness.query_by_label("Permanently unload").is_some(), + "the confirm button must name the action it performs" + ); + assert!( + harness.query_by_label("Keep identity").is_some(), + "the cancel button must name the outcome of cancelling" + ); + assert!( + harness.query_by_label("Yes").is_none(), + "a destructive confirmation must not offer a generic Yes" + ); + assert!( + harness.query_by_label("No").is_none(), + "a destructive confirmation must not offer a generic No" + ); + }); +} diff --git a/tests/kittest/masternode_tab.rs b/tests/kittest/masternode_tab.rs index 393285c74..8351dbbe8 100644 --- a/tests/kittest/masternode_tab.rs +++ b/tests/kittest/masternode_tab.rs @@ -781,6 +781,142 @@ fn remove_flow_deletes_associated_voter_identity() { }); } +/// The card disappearing is not feedback: a removal that left owner/voter key +/// residue on disk looks identical to a clean one. Every screen that removes an +/// identity must report the outcome, this tab included. +#[test] +fn remove_flow_reports_the_removal_outcome() { + with_isolated_data_dir(|| { + let rt = tokio::runtime::Runtime::new().expect("Failed to create tokio runtime"); + let _guard = rt.enter(); + + let mut harness = mount_app(RootScreenType::RootScreenIdentities); + let app_context = harness.state().current_app_context().clone(); + seed_node( + &app_context, + 0x99, + "mn-report-outcome", + IdentityType::Masternode, + ); + activate_masternodes_tab(&mut harness, &app_context); + + harness.get_by_label("Open mn-report-outcome").click(); + harness.run_steps(3); + harness.get_by_label("Remove masternode").click(); + harness.run_steps(3); + harness + .query_all_by_label("Remove masternode") + .last() + .expect("confirm button present") + .click(); + harness.run_steps(5); + + assert!( + harness + .query_by_label("The identity was removed from this device.") + .is_some(), + "removing a node from this tab must report the cleanup outcome" + ); + }); +} + +/// An evonode is not a masternode: the control that opens the removal +/// confirmation must carry the same kind-derived verb as the confirmation +/// itself, or the two disagree about what the click does. +#[test] +fn remove_trigger_names_the_node_kind() { + with_isolated_data_dir(|| { + let rt = tokio::runtime::Runtime::new().expect("Failed to create tokio runtime"); + let _guard = rt.enter(); + + let mut harness = mount_app(RootScreenType::RootScreenIdentities); + let app_context = harness.state().current_app_context().clone(); + seed_node(&app_context, 0x9A, "evo-remove-verb", IdentityType::Evonode); + activate_masternodes_tab(&mut harness, &app_context); + + harness.get_by_label("Open evo-remove-verb").click(); + harness.run_steps(3); + + assert!( + harness.query_by_label("Remove evonode").is_some(), + "an evonode's remove button must name the evonode" + ); + assert!( + harness.query_by_label("Remove masternode").is_none(), + "an evonode must never be offered a masternode's wording" + ); + + // The confirmation it opens agrees with the trigger. + harness.get_by_label("Remove evonode").click(); + harness.run_steps(3); + assert!( + harness.query_by_label("Keep evonode").is_some(), + "the confirmation must be the kind-derived one" + ); + }); +} + +/// The failure path of the removal banner: cleanup residue must reach the user +/// as a warning that stays put, not a success that fades. Drives the result +/// through the app's own task channel, which is the wiring `AppState` owns — +/// production of the flags themselves is covered by the fault-injection tests in +/// `backend_task::identity::remove_identity`. +#[test] +fn removal_with_cleanup_residue_warns_and_stays_on_screen() { + use dash_evo_tool::app::TaskResult; + use dash_evo_tool::backend_task::{BackendTaskContext, BackendTaskSuccessResult}; + + with_isolated_data_dir(|| { + let rt = tokio::runtime::Runtime::new().expect("Failed to create tokio runtime"); + let _guard = rt.enter(); + + let mut harness = mount_app(RootScreenType::RootScreenIdentities); + let app_context = harness.state().current_app_context().clone(); + let node_id = Identifier::from([0x9B; 32]); + seed_node(&app_context, 0x9B, "mn-residue", IdentityType::Masternode); + activate_masternodes_tab(&mut harness, &app_context); + + harness + .state() + .task_result_sender + .try_send(TaskResult::Success { + context: BackendTaskContext::Unknown, + result: Box::new(BackendTaskSuccessResult::RemovedIdentities { + identity_ids: vec![node_id], + primary_cleanup_failed: true, + associated_cleanup_failed: false, + associated_removal_failed: false, + }), + }) + .expect("queue the removal result"); + harness.run_steps(5); + + assert!( + harness + .query_by_label( + "The identity was removed, but some local data could not be cleaned up. Load \ + and remove it again to retry." + ) + .is_some(), + "cleanup residue must be reported, not swallowed" + ); + assert!( + harness.query_by_label("\u{26A0}").is_some(), + "residue is a warning, not a success" + ); + // An auto-dismissing banner renders a countdown beside its dismiss + // control; a warning's window is 9s, so only these two can appear in the + // frames this test runs. Their absence is the durable signal that + // `disable_auto_dismiss` was applied. + for countdown in ["(9s)", "(10s)"] { + assert!( + harness.query_by_label(countdown).is_none(), + "a banner the user must act on cannot fade away on its own" + ); + } + }); +} + /// Execution-level: clicking a per-key "Manage keys" button in the masternode /// detail view opens the interactive `KeyInfoScreen` (not the static read-only /// `KeysScreen`). Seeds a node whose voter identity carries one key so