From dfcb906591321a2341d108141e19eb0121a2acf3 Mon Sep 17 00:00:00 2001
From: Lukasz Klimek <842586+lklimek@users.noreply.github.com>
Date: Fri, 31 Jul 2026 14:24:17 +0000
Subject: [PATCH 1/3] fix(wallet): stop a duplicate identity index from making
 a wallet unloadable
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A wallet whose saved state was migrated from an older install could come
back on the next launch as "Saved wallet data appears damaged and cannot
be loaded", with recovery-phrase restore as the only way forward.

The wallet backend buckets a wallet's identities on
`(wallet_id, identity_index)`. DET's identity index is user-entered (the
"add existing identity" screen) and carries no uniqueness, so one wallet
can hold two identities at the same index — the ordinary shape of an
install upgraded from a build that never used the index as a key.
Registering the second identity displaced the first from the active set
while both kept their saved records. The next launch replayed the same
collapse, could not reattach the displaced identity's saved keys, and
failed the whole wallet's load with `OrphanedIdentityEntry` — surfaced as
`TaskError::WalletLocalDataLoadFailed`.

`ensure_identity_managed` is the single DET path into that registration,
so it now refuses a second identity at a taken index with the dedicated
`TaskError::IdentityIndexAlreadyTaken`, naming the identity that holds it.
Nothing is written, the wallet stays loadable, and the identity reconciler
logs the permanent condition as a warning instead of a retryable defer.

Tests: `a_second_identity_at_a_taken_index_is_refused_and_the_wallet_still_reloads`
(RED before the guard with the exact user-facing failure) and
`a_migrated_install_relaunches_without_damaged_wallet_data`. The existing
`reconcile_managed_identities_registers_only_wallet_owned` probed a taken
index to prove an unrelated filter; it now probes a free one.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 src/app_dir.rs                             |  18 +++
 src/backend_task/error.rs                  |  15 +++
 src/backend_task/migration/v093_upgrade.rs |  60 ++++++++++
 src/context/wallet_lifecycle/bootstrap.rs  |   7 ++
 src/context/wallet_lifecycle/tests.rs      | 132 +++++++++++++++++----
 src/wallet_backend/identity_ops.rs         |  20 +++-
 6 files changed, 230 insertions(+), 22 deletions(-)

diff --git a/src/app_dir.rs b/src/app_dir.rs
index 414ef778d..ba920f135 100644
--- a/src/app_dir.rs
+++ b/src/app_dir.rs
@@ -76,6 +76,24 @@ pub fn create_app_user_data_directory_if_not_exists() -> Result<(), std::io::Err
     ensure_data_dir_exists(&app_data_dir)
 }
 
+/// Copy a data directory tree, permissions included. Cold-boot tests reopen
+/// wallet state over a fresh path with identical bytes, sidestepping the
+/// persister's single-open advisory lock a lingering subtask may still hold.
+#[cfg(test)]
+pub(crate) fn copy_dir_recursive(src: &Path, dst: &Path) {
+    ensure_data_dir_exists(dst).expect("create destination directory");
+    for entry in fs::read_dir(src).expect("read_dir") {
+        let entry = entry.expect("dir entry");
+        let from = entry.path();
+        let to = dst.join(entry.file_name());
+        if from.is_dir() {
+            copy_dir_recursive(&from, &to);
+        } else {
+            fs::copy(&from, &to).expect("copy file");
+        }
+    }
+}
+
 /// Creates the given data directory if it does not exist and verifies it is a directory.
 pub fn ensure_data_dir_exists(data_dir: &Path) -> Result<(), std::io::Error> {
     #[cfg(unix)]
diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs
index 3881e8666..b91be584a 100644
--- a/src/backend_task/error.rs
+++ b/src/backend_task/error.rs
@@ -303,6 +303,21 @@ pub enum TaskError {
         wallet_index: u32,
     },
 
+    /// Two identities on one wallet claim the same identity index. The wallet
+    /// backend keys its active set on that index, so admitting the second
+    /// would displace the first from memory while both keep their saved
+    /// records — and the next launch would reject the whole wallet's saved
+    /// data as damaged. Stopped before anything is written.
+    ///
+    /// `index` is a numeric diagnostic for logs and the `Debug` view only.
+    #[error(
+        "This identity uses the same identity index as identity {occupant_id}, and every identity on a wallet needs its own. Reload this identity with an index no other identity on this wallet uses, then try again."
+    )]
+    IdentityIndexAlreadyTaken {
+        occupant_id: dash_sdk::platform::Identifier,
+        index: u32,
+    },
+
     /// A wallet-funded top-up targeted an identity this wallet does not own
     /// (it has no HD funding slot here). Funding it from this wallet would
     /// derive an unrelated asset-lock account, so the op is stopped before any
diff --git a/src/backend_task/migration/v093_upgrade.rs b/src/backend_task/migration/v093_upgrade.rs
index 0413b05cc..0c25beccf 100644
--- a/src/backend_task/migration/v093_upgrade.rs
+++ b/src/backend_task/migration/v093_upgrade.rs
@@ -1577,6 +1577,66 @@ async fn a_second_launch_after_an_unreadable_identity_preserves_user_edits_and_d
     backend.shutdown().await;
 }
 
+/// A relaunch of an already-migrated install: a second `AppContext` over the
+/// same on-disk state, reading the settings the first boot wrote.
+fn reopen(dir: &std::path::Path) -> Arc<AppContext> {
+    let db = Arc::new(
+        Database::open_legacy_read_only(dir.join("data.db")).expect("open data.db read-only"),
+    );
+    let app_kv = AppContext::open_app_kv(dir).expect("open app k/v");
+    let settings = app_kv
+        .get::<AppSettings>(DetScope::Global, AppSettings::KV_KEY)
+        .expect("read settings blob")
+        .expect("the first boot must have written a settings blob");
+    let secret_store = AppContext::open_secret_store(dir).expect("open secret store");
+    AppContext::new(
+        dir.to_path_buf(),
+        settings.network,
+        db,
+        Default::default(),
+        Default::default(),
+        egui::Context::default(),
+        app_kv,
+        secret_store,
+        crate::model::user_role::UserRoleCell::default(),
+    )
+    .expect("AppContext")
+}
+
+/// The user's reproduction, minus the network: migrate a v0.9.3 install that
+/// holds a password-protected wallet, close the app, open it again. The
+/// relaunch must load the saved wallet state — a fatal persister load surfaces
+/// as `WalletLocalDataLoadFailed` ("Saved wallet data appears damaged").
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn a_migrated_install_relaunches_without_damaged_wallet_data() {
+    let tmp = tempfile::tempdir().expect("tempdir");
+    write_v093_database(tmp.path());
+
+    let (ctx, _settings) = boot(tmp.path());
+    let backend = wire_backend(&ctx).await;
+    assert!(
+        run_migration_with_wallet_passwords(&ctx)
+            .await
+            .expect("migration"),
+        "precondition: the fixture has data to move",
+    );
+    backend.shutdown().await;
+    drop(backend);
+    drop(ctx);
+
+    let cold = tempfile::tempdir().expect("cold tempdir");
+    crate::app_dir::copy_dir_recursive(tmp.path(), cold.path());
+
+    let ctx2 = reopen(cold.path());
+    let (tx, _rx) = tokio::sync::mpsc::channel::<crate::app::TaskResult>(32);
+    let sender = crate::utils::egui_mpsc::SenderAsync::new(tx, ctx2.egui_ctx().clone());
+    ctx2.ensure_wallet_backend(sender)
+        .await
+        .expect("a relaunch after the storage update must load the saved wallet data");
+    let backend2 = ctx2.wallet_backend().expect("backend wired");
+    backend2.shutdown().await;
+}
+
 /// The identity import carries its own sentinel. Reusing the wallet drain's
 /// would silently skip the import for every install that already drained its
 /// wallets under a build that had no identity importer — i.e. exactly the
diff --git a/src/context/wallet_lifecycle/bootstrap.rs b/src/context/wallet_lifecycle/bootstrap.rs
index 90e1bc1ac..7e14737d9 100644
--- a/src/context/wallet_lifecycle/bootstrap.rs
+++ b/src/context/wallet_lifecycle/bootstrap.rs
@@ -259,6 +259,13 @@ impl AppContext {
             {
                 Ok(true) => added += 1,
                 Ok(false) => {}
+                // A taken index stays taken until the user re-indexes the
+                // identity, so this one is not deferred and never retries.
+                Err(error @ TaskError::IdentityIndexAlreadyTaken { .. }) => tracing::warn!(
+                    identity = %qi.identity.id(),
+                    %error,
+                    "Identity shares its identity index with another on this wallet; left unregistered"
+                ),
                 Err(error) => tracing::debug!(
                     identity = %qi.identity.id(),
                     %error,
diff --git a/src/context/wallet_lifecycle/tests.rs b/src/context/wallet_lifecycle/tests.rs
index d881461e1..adb55ed1e 100644
--- a/src/context/wallet_lifecycle/tests.rs
+++ b/src/context/wallet_lifecycle/tests.rs
@@ -1,6 +1,6 @@
 use super::*;
 use crate::app::TaskResult;
-use crate::app_dir::{ensure_data_dir_exists, ensure_env_file};
+use crate::app_dir::{copy_dir_recursive, ensure_env_file};
 use crate::context::AppContext;
 use crate::context::connection_status::ConnectionStatus;
 use crate::context::migration_status::MigrationState;
@@ -76,23 +76,6 @@ fn offline_testnet_context_with_db(
     (ctx, sender)
 }
 
-/// Recursively copy a directory tree. Cold-boot tests reopen wallet state
-/// over a fresh path (identical on-disk bytes) to sidestep the persister's
-/// single-open advisory lock a lingering subtask may still hold.
-fn copy_dir_recursive(src: &std::path::Path, dst: &std::path::Path) {
-    ensure_data_dir_exists(dst).expect("create secure destination directory");
-    for entry in std::fs::read_dir(src).expect("read_dir") {
-        let entry = entry.expect("dir entry");
-        let from = entry.path();
-        let to = dst.join(entry.file_name());
-        if from.is_dir() {
-            copy_dir_recursive(&from, &to);
-        } else {
-            std::fs::copy(&from, &to).expect("copy file");
-        }
-    }
-}
-
 /// Process-global serialization lock for tests that tear a wallet backend
 /// down and immediately rebuild it over the *same* on-disk path. The
 /// upstream persister enforces a single open per `platform-wallet.sqlite`
@@ -4105,6 +4088,113 @@ fn basic_test_identity() -> dash_sdk::dpp::identity::Identity {
         .expect("basic identity")
 }
 
+/// A published identity carrying one authentication key, so registering it
+/// upstream writes `identity_keys` rows — the rows the rehydration merge
+/// re-attaches to their owner.
+fn test_identity_with_key() -> dash_sdk::dpp::identity::Identity {
+    use dash_sdk::dpp::identity::accessors::IdentityGettersV0;
+    use dash_sdk::dpp::identity::identity_public_key::v0::IdentityPublicKeyV0;
+    use dash_sdk::dpp::identity::{IdentityPublicKey, KeyType, Purpose, SecurityLevel};
+    use dash_sdk::dpp::platform_value::BinaryData;
+
+    let mut identity = basic_test_identity();
+    let key = IdentityPublicKey::V0(IdentityPublicKeyV0 {
+        id: 0,
+        purpose: Purpose::AUTHENTICATION,
+        security_level: SecurityLevel::HIGH,
+        contract_bounds: None,
+        key_type: KeyType::ECDSA_SECP256K1,
+        read_only: false,
+        data: BinaryData::new(vec![0x02; 33]),
+        disabled_at: None,
+    });
+    identity.public_keys_mut().insert(0, key);
+    identity
+}
+
+/// A second identity claiming an identity index another identity on the same
+/// wallet already holds must be refused — and must not cost the user the whole
+/// wallet on the next launch.
+///
+/// The index is user-entered (the "add existing identity" screen) and DET puts
+/// no uniqueness on it, so one wallet can hold two identities at the same
+/// index. Upstream keys a wallet's identities on `(wallet_id, identity_index)`,
+/// so admitting the second displaces the first in memory while both keep their
+/// rows on disk (`identities` is keyed on `identity_id`). The next launch
+/// replays that collapse, leaves the displaced identity's `identity_keys` rows
+/// without an owner, and rejects the wallet's whole saved state with a fatal
+/// `OrphanedIdentityEntry` — the user-visible "Saved wallet data appears
+/// damaged" banner (`TaskError::WalletLocalDataLoadFailed`).
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn a_second_identity_at_a_taken_index_is_refused_and_the_wallet_still_reloads() {
+    use dash_sdk::dpp::identity::accessors::IdentityGettersV0;
+
+    let _guard = backend_reopen_lock().await;
+    let source_dir = tempfile::tempdir().expect("source tempdir");
+    let seed = [0xE1u8; 64];
+    let resident = test_identity_with_key();
+    let colliding = test_identity_with_key();
+
+    {
+        let wallet =
+            crate::model::wallet::Wallet::new_from_seed(seed, Network::Testnet, None, None)
+                .expect("build wallet");
+        let seed_hash = wallet.seed_hash();
+
+        let (ctx, sender) = offline_testnet_context_at(source_dir.path());
+        ctx.register_wallet(wallet, &seed, WalletOrigin::Fresh)
+            .expect("register wallet");
+        ctx.ensure_wallet_backend(sender)
+            .await
+            .expect("wire backend offline");
+        let backend = ctx.wallet_backend().expect("backend");
+        backend
+            .register_wallet_from_seed(&seed_hash, &seed, Some(0))
+            .await
+            .expect("upstream register");
+
+        assert!(
+            backend
+                .ensure_identity_managed(&seed_hash, &resident, 0)
+                .await
+                .expect("the first identity at a free index must register"),
+        );
+
+        let error = backend
+            .ensure_identity_managed(&seed_hash, &colliding, 0)
+            .await
+            .expect_err("a second identity at a taken index must be refused");
+        assert!(
+            matches!(
+                error,
+                TaskError::IdentityIndexAlreadyTaken { occupant_id, index }
+                    if occupant_id == resident.id() && index == 0
+            ),
+            "the refusal must name the identity holding the index, got: {error:?}",
+        );
+
+        backend.shutdown().await;
+    }
+
+    let cold_dir = tempfile::tempdir().expect("cold tempdir");
+    copy_dir_recursive(source_dir.path(), cold_dir.path());
+
+    let (ctx2, sender2) = offline_testnet_context_at(cold_dir.path());
+    ctx2.ensure_wallet_backend(sender2)
+        .await
+        .expect("the next launch must load the saved wallet data");
+    let backend2 = ctx2.wallet_backend().expect("backend");
+    assert!(
+        backend2.is_wallet_registered(&{
+            crate::model::wallet::Wallet::new_from_seed(seed, Network::Testnet, None, None)
+                .expect("build wallet")
+                .seed_hash()
+        }),
+        "the wallet must come back registered, not rejected as damaged",
+    );
+    backend2.shutdown().await;
+}
+
 /// Wrap a basic identity in a minimal wallet-owned `QualifiedIdentity` for
 /// sidecar-reconcile tests.
 fn wallet_owned_qualified_identity(
@@ -4341,10 +4431,12 @@ async fn reconcile_managed_identities_registers_only_wallet_owned() {
             .expect("owned_b"),
         "wallet-owned identity B must already be managed after reconcile"
     );
-    // The index-less identity was skipped → ensure newly registers it.
+    // The index-less identity was skipped → ensure newly registers it. Probed
+    // at a free index: index 0 belongs to identity A, and one identity per
+    // index per wallet is the invariant that keeps the wallet loadable.
     assert!(
         backend
-            .ensure_identity_managed(&seed_hash, &detached.identity, 0)
+            .ensure_identity_managed(&seed_hash, &detached.identity, 2)
             .await
             .expect("detached"),
         "index-less identity must have been skipped by the reconcile filter"
diff --git a/src/wallet_backend/identity_ops.rs b/src/wallet_backend/identity_ops.rs
index 1fd06c39c..8b44490bd 100644
--- a/src/wallet_backend/identity_ops.rs
+++ b/src/wallet_backend/identity_ops.rs
@@ -101,8 +101,10 @@ impl WalletBackend {
     /// # Errors
     /// [`TaskError::WalletNotLoaded`] if the wallet is not yet upstream
     /// registered; [`TaskError::WalletStateInconsistent`] if the resolved wallet
-    /// has no manager entry; [`TaskError::WalletBackend`] on an upstream add
-    /// failure other than the swallowed `IdentityAlreadyExists`.
+    /// has no manager entry; [`TaskError::IdentityIndexAlreadyTaken`] if another
+    /// identity on this wallet already holds `identity_index`;
+    /// [`TaskError::WalletBackend`] on an upstream add failure other than the
+    /// swallowed `IdentityAlreadyExists`.
     pub(crate) async fn ensure_identity_managed(
         &self,
         seed_hash: &WalletSeedHash,
@@ -135,6 +137,20 @@ impl WalletBackend {
         if info.identity_manager.identity(&id).is_some() {
             return Ok(false);
         }
+        // Upstream buckets a wallet's identities on `(wallet_id,
+        // identity_index)`: a second identity at a taken index displaces the
+        // resident one in memory while both keep their rows, and the next
+        // launch cannot reattach the displaced one's keys — it rejects the
+        // whole wallet's saved data as damaged. DET's index is user-entered
+        // and carries no uniqueness, so this is the last gate before the write.
+        if let Some(occupant) = info.identity_manager.managed_identities().find(|managed| {
+            managed.wallet_id == Some(wallet_id) && managed.identity_index == Some(identity_index)
+        }) {
+            return Err(TaskError::IdentityIndexAlreadyTaken {
+                occupant_id: occupant.identity.id(),
+                index: identity_index,
+            });
+        }
         match info.identity_manager.add_identity(
             identity.clone(),
             identity_index,

From 72de5aeb981c21eddb1bef6a92d7cf7311ae3bdc Mon Sep 17 00:00:00 2001
From: Lukasz Klimek <842586+lklimek@users.noreply.github.com>
Date: Fri, 31 Jul 2026 14:40:25 +0000
Subject: [PATCH 2/3] fix(wallet): persist the identity funding account DET
 provisions
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

A restart between an asset-lock broadcast and its consumption could strand the
lock and the funds in it: `resume_asset_lock` fails to re-derive the credit
output with "Funding account IdentityTopUp not found for re-derivation".

`load()` rebuilds `Wallet.accounts` from `account_registrations` alone.
`provision_identity_funding_account` created the account in both upstream
in-memory collections and persisted nothing, so upstream's own creator — the
only writer of that row — then hit its `contains_*` guards, concluded both
sides already existed, and took its early return, skipping the registration and
address-pool store its docs call load-bearing for crash recovery. The account
was live in memory and absent from disk on every launch.

DET's provisioning now writes the `AccountRegistrationEntry` through the
persister and flushes it, and rolls both in-memory inserts back on a store
failure so a retry re-creates and re-persists rather than short-circuiting on
the presence guards. Failures surface as the dedicated
`TaskError::IdentityFundingAccountPersistFailed`.

Residual: the paired address-pool snapshot upstream writes alongside the
registration is not reachable from here (`account_address_pool_entries` is
crate-private), so pool depth for that account re-warms on the next sync
instead of restoring. The account itself — what re-derivation needs — is
restored.

Test: `a_provisioned_identity_topup_account_survives_a_restart`, confirmed RED
before the fix (0 persisted rows, expected 1).

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 src/backend_task/error.rs             | 12 ++++
 src/context/wallet_lifecycle/tests.rs | 80 +++++++++++++++++++++++++++
 src/wallet_backend/identity_ops.rs    | 59 +++++++++++++++++++-
 3 files changed, 150 insertions(+), 1 deletion(-)

diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs
index b91be584a..6766d7c78 100644
--- a/src/backend_task/error.rs
+++ b/src/backend_task/error.rs
@@ -185,6 +185,18 @@ pub enum TaskError {
         source: dash_sdk::dpp::key_wallet::Error,
     },
 
+    /// An identity-funding account was derived but could not be saved. Saving
+    /// it is what lets a restart find the account again, so the operation is
+    /// stopped rather than left able to strand a funding lock the app could no
+    /// longer spend. The technical cause lives in `Debug` and the logs.
+    #[error(
+        "Your wallet could not save the account this payment needs. Check that your disk is not full, then try again."
+    )]
+    IdentityFundingAccountPersistFailed {
+        #[source]
+        source: Box<platform_wallet::changeset::PersistenceError>,
+    },
+
     /// Single-key wallets are not supported in this version. Their data is
     /// preserved; HD (recovery-phrase) wallets remain fully functional.
     #[error(
diff --git a/src/context/wallet_lifecycle/tests.rs b/src/context/wallet_lifecycle/tests.rs
index adb55ed1e..496445d99 100644
--- a/src/context/wallet_lifecycle/tests.rs
+++ b/src/context/wallet_lifecycle/tests.rs
@@ -3785,6 +3785,86 @@ async fn ensure_identity_funding_accounts_succeeds_on_cold_booted_watch_only_wal
     backend2.shutdown().await;
 }
 
+/// A provisioned identity top-up account must survive a restart.
+///
+/// `load()` rebuilds `Wallet.accounts` from `account_registrations` alone, and
+/// the upstream creator that would otherwise write that row skips it once both
+/// in-memory collections already hold the account — which DET's own
+/// provisioning puts there first. A memory-only account leaves a restart
+/// between an asset-lock broadcast and its consumption unable to re-derive the
+/// credit-output path, stranding the lock and the funds in it.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn a_provisioned_identity_topup_account_survives_a_restart() {
+    let _guard = backend_reopen_lock().await;
+    let source_dir = tempfile::tempdir().expect("source tempdir");
+    let seed = [0xF3u8; 64];
+    let registration_index = 7u32;
+
+    let seed_hash = {
+        let wallet =
+            crate::model::wallet::Wallet::new_from_seed(seed, Network::Testnet, None, None)
+                .expect("build wallet");
+        let seed_hash = wallet.seed_hash();
+
+        let (ctx, sender) = offline_testnet_context_at(source_dir.path());
+        ctx.register_wallet(wallet, &seed, WalletOrigin::Fresh)
+            .expect("register wallet");
+        ctx.ensure_wallet_backend(sender)
+            .await
+            .expect("wire backend offline");
+        let backend = ctx.wallet_backend().expect("backend");
+        backend
+            .register_wallet_from_seed(&seed_hash, &seed, Some(0))
+            .await
+            .expect("upstream register");
+        backend
+            .ensure_identity_funding_accounts(&seed_hash, &seed, registration_index)
+            .await
+            .expect("provision identity funding accounts");
+        backend.shutdown().await;
+        seed_hash
+    };
+
+    let cold_dir = tempfile::tempdir().expect("cold tempdir");
+    copy_dir_recursive(source_dir.path(), cold_dir.path());
+
+    // The manifest is the only thing `load()` rebuilds `Wallet.accounts` from,
+    // so assert the row itself rather than a downstream in-memory effect.
+    let persisted_topup_rows: i64 = rusqlite::Connection::open_with_flags(
+        cold_dir
+            .path()
+            .join("spv")
+            .join("testnet")
+            .join("platform-wallet.sqlite"),
+        rusqlite::OpenFlags::SQLITE_OPEN_READ_ONLY,
+    )
+    .expect("open persisted store")
+    .query_row(
+        "SELECT COUNT(*) FROM account_registrations \
+         WHERE account_type = 'identity_topup' AND account_index = ?1",
+        [registration_index],
+        |row| row.get(0),
+    )
+    .expect("count persisted top-up registrations");
+    assert_eq!(
+        persisted_topup_rows, 1,
+        "the provisioned top-up account must be in the persisted manifest, or a \
+         broadcast asset lock cannot be resumed after a restart",
+    );
+
+    let (ctx2, sender2) = offline_testnet_context_at(cold_dir.path());
+    ctx2.ensure_wallet_backend(sender2)
+        .await
+        .expect("cold boot must load the persisted wallet");
+    let backend2 = ctx2.wallet_backend().expect("backend");
+    assert!(
+        backend2.is_wallet_registered(&seed_hash),
+        "the wallet must still come back registered with the extra account row",
+    );
+
+    backend2.shutdown().await;
+}
+
 /// A malformed Orchard viewing key must be isolated to its own wallet during
 /// the real seedless cold-boot load.
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
diff --git a/src/wallet_backend/identity_ops.rs b/src/wallet_backend/identity_ops.rs
index 8b44490bd..da240b40f 100644
--- a/src/wallet_backend/identity_ops.rs
+++ b/src/wallet_backend/identity_ops.rs
@@ -403,12 +403,69 @@ impl WalletBackend {
         }
         .ok_or(TaskError::WalletStateInconsistent)?;
 
+        let account_type = derived.account_type;
+        let account_xpub = derived.account_xpub;
+
         let managed = ManagedCoreKeysAccount::from_account(derived);
         info.core_wallet
             .accounts
             .insert_keys_bearing_account(managed)
             .map_err(|source| TaskError::IdentityFundingAccountProvisionFailed { source })?;
-        Ok(())
+
+        // Persist the registration. `load()` rebuilds `Wallet.accounts` from
+        // `account_registrations` alone, and the upstream creator that would
+        // normally write this row skips it once both in-memory collections hold
+        // the account — which they do by the time it runs, because of the
+        // inserts above. Without this the account is memory-only: a restart
+        // between an asset-lock broadcast and its consumption cannot re-derive
+        // the credit-output path, stranding the lock and its funds.
+        self.persist_account_registration(&wallet_id, account_type, account_xpub)
+            .inspect_err(|_| {
+                // Roll back both sides so a retry re-creates and re-persists,
+                // rather than the `in_wallet && in_managed` guard above short-
+                // circuiting a persist that never happened.
+                match funding {
+                    Funding::Registration => {
+                        kw.accounts.identity_registration = None;
+                        info.core_wallet.accounts.identity_registration = None;
+                    }
+                    Funding::TopUp(registration_index) => {
+                        kw.accounts.identity_topup.remove(&registration_index);
+                        info.core_wallet
+                            .accounts
+                            .identity_topup
+                            .remove(&registration_index);
+                    }
+                }
+            })
+    }
+
+    /// Write one account registration through the upstream persister and flush
+    /// it, so a cold boot rebuilds the account from the manifest.
+    fn persist_account_registration(
+        &self,
+        wallet_id: &platform_wallet::wallet::platform_wallet::WalletId,
+        account_type: dash_sdk::dpp::key_wallet::AccountType,
+        account_xpub: dash_sdk::dpp::key_wallet::bip32::ExtendedPubKey,
+    ) -> Result<(), TaskError> {
+        use platform_wallet::changeset::{
+            AccountRegistrationEntry, PlatformWalletChangeSet, PlatformWalletPersistence,
+        };
+
+        let changeset = PlatformWalletChangeSet {
+            account_registrations: vec![AccountRegistrationEntry {
+                account_type,
+                account_xpub,
+            }],
+            ..Default::default()
+        };
+        self.inner
+            .wallet_persister
+            .store(*wallet_id, changeset)
+            .and_then(|()| self.inner.wallet_persister.flush(*wallet_id))
+            .map_err(|source| TaskError::IdentityFundingAccountPersistFailed {
+                source: Box::new(source),
+            })
     }
 
     /// Provision the identity-registration funding account and the per-

From dc547a141bf7fc5eda94f2834c03e0e1aeacf4a4 Mon Sep 17 00:00:00 2001
From: Lukasz Klimek <842586+lklimek@users.noreply.github.com>
Date: Fri, 31 Jul 2026 15:01:55 +0000
Subject: [PATCH 3/3] fix(identity): close the registration and removal routes
 into a lost wallet
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

The occupancy guard added in dfcb9065 covered only the reconcile and top-up
paths. Two routes to the same fatal corruption stayed open, both found by
independent review.

Registration: `WalletBackend::register_identity` handed off to upstream
`register_identity_with_funding`, whose Step 4 calls `add_identity` with no
occupancy check and then deliberately swallows that add's failure so the spent
asset lock is still consumed — a collision there is persisted, never reported.
The UI gate that was assumed to cover this is inert: `wallet.identities`
hydrates empty on every cold boot, so after a restart the index picker marks
nothing as used and recommends index 0, the very index a first identity
usually holds. A pre-flight check now refuses before the asset lock is built,
so a collision costs the user nothing. Per CLAUDE.md the backend task is the
authoritative enforcement layer; a combo box was never enough.

Removal: `AppContext::remove_identity` only cleared DET's own k/v store and
vault, so the wallet backend kept the identity and its index. The slot stayed
occupied by an identity the user believes is gone — blocking re-use of that
index with a phantom occupant they cannot act on, and leaving the index looking
free to the picker while a registration there would still collide. Removal now
releases the index upstream, which records a tombstone rather than deleting:
the rows stay on disk and any orphaned key rows become safe to skip at load,
which is what keeps a removal from becoming the damaged-data failure itself.

Both call sites share one `index_occupant` helper so the invariant cannot drift
between them, and the error message now names controls that exist ("pick an
identity index that is not marked as used, or remove identity X first").

Test `removing_an_identity_frees_its_index_for_reuse` drives the real
`BackendTask::RemoveIdentity` dispatch and was confirmed RED before the fix,
failing with the exact phantom-occupant `IdentityIndexAlreadyTaken`. The
registration pre-flight is not unit-testable offline — it needs funds and a
live Platform — so it is verified by code trace against the pinned upstream
revision, not by a run.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
---
 src/backend_task/error.rs                    |   2 +-
 src/backend_task/identity/mod.rs             |   2 +-
 src/backend_task/identity/remove_identity.rs |  27 ++++-
 src/context/wallet_lifecycle/tests.rs        |  58 +++++++++
 src/wallet_backend/identity_ops.rs           | 121 +++++++++++++++++--
 5 files changed, 197 insertions(+), 13 deletions(-)

diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs
index 6766d7c78..c8e623e7b 100644
--- a/src/backend_task/error.rs
+++ b/src/backend_task/error.rs
@@ -323,7 +323,7 @@ pub enum TaskError {
     ///
     /// `index` is a numeric diagnostic for logs and the `Debug` view only.
     #[error(
-        "This identity uses the same identity index as identity {occupant_id}, and every identity on a wallet needs its own. Reload this identity with an index no other identity on this wallet uses, then try again."
+        "Identity {occupant_id} on this wallet already uses this identity index, and every identity on a wallet needs its own. Pick an identity index that is not marked as used, or remove identity {occupant_id} first, then try again."
     )]
     IdentityIndexAlreadyTaken {
         occupant_id: dash_sdk::platform::Identifier,
diff --git a/src/backend_task/identity/mod.rs b/src/backend_task/identity/mod.rs
index 70eb06181..d763bdc3e 100644
--- a/src/backend_task/identity/mod.rs
+++ b/src/backend_task/identity/mod.rs
@@ -878,7 +878,7 @@ impl AppContext {
             IdentityTask::RegisterDpnsName(input) => {
                 Ok(self.register_dpns_name(sdk, input).await?)
             }
-            IdentityTask::RemoveIdentity { identity_id } => self.remove_identity(identity_id),
+            IdentityTask::RemoveIdentity { identity_id } => self.remove_identity(identity_id).await,
             IdentityTask::RefreshIdentity(qualified_identity) => {
                 self.refresh_identity(sdk, qualified_identity, sender).await
             }
diff --git a/src/backend_task/identity/remove_identity.rs b/src/backend_task/identity/remove_identity.rs
index 6a43a5678..13d3a54a9 100644
--- a/src/backend_task/identity/remove_identity.rs
+++ b/src/backend_task/identity/remove_identity.rs
@@ -4,7 +4,7 @@ use dash_sdk::dpp::identity::accessors::IdentityGettersV0;
 use dash_sdk::platform::Identifier;
 
 impl AppContext {
-    pub(super) fn remove_identity(
+    pub(super) async fn remove_identity(
         &self,
         identity_id: Identifier,
     ) -> Result<BackendTaskSuccessResult, TaskError> {
@@ -16,11 +16,13 @@ impl AppContext {
                     .map(|(voter_identity, _)| voter_identity.id())
             });
 
+        self.release_identity_index(&identity_id).await;
         self.delete_local_qualified_identity(&identity_id)?;
 
         let mut removed_identity_ids = vec![identity_id];
         let mut associated_cleanup_failed = false;
         if let Some(voter_id) = associated_voter_identity_id.filter(|id| *id != identity_id) {
+            self.release_identity_index(&voter_id).await;
             match self.delete_local_qualified_identity(&voter_id) {
                 Ok(()) => removed_identity_ids.push(voter_id),
                 Err(error) => {
@@ -39,4 +41,27 @@ impl AppContext {
             associated_cleanup_failed,
         })
     }
+
+    /// Release the identity index this identity holds in the wallet backend.
+    ///
+    /// DET's own records are not the only ones holding it: an entry left behind
+    /// upstream keeps the index occupied by an identity the user believes is
+    /// gone, which blocks re-adding one there and lets a later registration
+    /// collide into the damaged-saved-data failure.
+    ///
+    /// Best-effort — a wallet not registered this session has nothing to
+    /// release, and the user's removal must complete either way.
+    async fn release_identity_index(&self, identity_id: &Identifier) {
+        let Ok(backend) = self.wallet_backend() else {
+            return;
+        };
+        if let Err(error) = backend.forget_identity(identity_id).await {
+            tracing::warn!(
+                %identity_id,
+                %error,
+                "Identity index could not be released in the wallet backend; \
+                 adding an identity at that index may be refused"
+            );
+        }
+    }
 }
diff --git a/src/context/wallet_lifecycle/tests.rs b/src/context/wallet_lifecycle/tests.rs
index 496445d99..14d0a1ac2 100644
--- a/src/context/wallet_lifecycle/tests.rs
+++ b/src/context/wallet_lifecycle/tests.rs
@@ -4192,6 +4192,64 @@ fn test_identity_with_key() -> dash_sdk::dpp::identity::Identity {
     identity
 }
 
+/// Removing an identity must release the identity index it held, so the slot is
+/// usable again.
+///
+/// DET's own removal only touched its k/v store and vault, leaving the wallet
+/// backend's entry in place. The index then stayed occupied by an identity the
+/// user believes is gone: re-using it was refused, naming a phantom occupant the
+/// user cannot act on, and — before the occupancy guard existed — a registration
+/// there would silently collide and cost the whole wallet on the next launch.
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn removing_an_identity_frees_its_index_for_reuse() {
+    use crate::backend_task::BackendTask;
+    use crate::backend_task::identity::IdentityTask;
+    use dash_sdk::dpp::identity::accessors::IdentityGettersV0;
+
+    let (ctx, sender, _tmp) = offline_testnet_context();
+    ctx.ensure_wallet_backend(sender.clone())
+        .await
+        .expect("wire backend offline");
+    let backend = ctx.wallet_backend().expect("backend");
+
+    let seed = [0xB2u8; 64];
+    let wallet = crate::model::wallet::Wallet::new_from_seed(seed, Network::Testnet, None, None)
+        .expect("build wallet");
+    let seed_hash = wallet.seed_hash();
+    backend
+        .register_wallet_from_seed(&seed_hash, &seed, None)
+        .await
+        .expect("upstream register");
+
+    let removed = test_identity_with_key();
+    let replacement = test_identity_with_key();
+    assert!(
+        backend
+            .ensure_identity_managed(&seed_hash, &removed, 0)
+            .await
+            .expect("first identity registers at a free index"),
+    );
+
+    ctx.run_backend_task(
+        BackendTask::IdentityTask(IdentityTask::RemoveIdentity {
+            identity_id: removed.id(),
+        }),
+        sender,
+    )
+    .await
+    .expect("removing an identity must succeed");
+
+    backend
+        .ensure_identity_managed(&seed_hash, &replacement, 0)
+        .await
+        .expect(
+            "the index of a removed identity must be free again; a phantom occupant \
+             means the removal did not reach the wallet backend",
+        );
+
+    backend.shutdown().await;
+}
+
 /// A second identity claiming an identity index another identity on the same
 /// wallet already holds must be refused — and must not cost the user the whole
 /// wallet on the next launch.
diff --git a/src/wallet_backend/identity_ops.rs b/src/wallet_backend/identity_ops.rs
index da240b40f..912bcea3a 100644
--- a/src/wallet_backend/identity_ops.rs
+++ b/src/wallet_backend/identity_ops.rs
@@ -30,6 +30,30 @@ enum Funding {
     TopUp(u32),
 }
 
+/// The identity, if any, already holding `identity_index` on `wallet_id`.
+///
+/// Upstream buckets a wallet's identities on `(wallet_id, identity_index)` and
+/// admits a second one at a taken index without complaint: it displaces the
+/// resident identity in memory while both keep their persisted rows, and the
+/// next launch cannot reattach the displaced one's keys — it rejects the whole
+/// wallet's saved data as damaged. DET's index is user-entered and carries no
+/// uniqueness, so every DET path into upstream's identity registration checks
+/// this first. One implementation so the two call sites cannot drift.
+fn index_occupant(
+    manager: &platform_wallet::wallet::identity::IdentityManager,
+    wallet_id: &platform_wallet::wallet::platform_wallet::WalletId,
+    identity_index: u32,
+) -> Option<dash_sdk::platform::Identifier> {
+    use dash_sdk::dpp::identity::accessors::IdentityGettersV0;
+    manager
+        .managed_identities()
+        .find(|managed| {
+            managed.wallet_id.as_ref() == Some(wallet_id)
+                && managed.identity_index == Some(identity_index)
+        })
+        .map(|managed| managed.identity.id())
+}
+
 impl WalletBackend {
     /// Register a new identity on Platform funded by an asset lock built and
     /// tracked-to-finality by the upstream `AssetLockManager`. Returns the
@@ -46,6 +70,12 @@ impl WalletBackend {
     /// builds a fresh asset lock, `FromExistingAssetLock` resumes from a
     /// tracked outpoint (the wallet-backend tracker is the single source of
     /// asset-lock state).
+    ///
+    /// # Errors
+    /// [`TaskError::IdentityIndexAlreadyTaken`] if another identity on this
+    /// wallet already holds `identity_index` — checked before any funds move,
+    /// because upstream's registration would otherwise displace the resident
+    /// identity and leave the wallet unloadable on the next launch.
     pub async fn register_identity(
         &self,
         seed_hash: &WalletSeedHash,
@@ -55,6 +85,15 @@ impl WalletBackend {
         identity_signer: &crate::model::qualified_identity::QualifiedIdentity,
         settings: Option<dash_sdk::platform::transition::put_settings::PutSettings>,
     ) -> Result<dash_sdk::platform::Identity, TaskError> {
+        // Refuse before the asset lock is built, so a collision costs nothing:
+        // upstream registers the identity at `identity_index` without an
+        // occupancy check and deliberately swallows that add's failure to
+        // guarantee the spent lock is consumed, so a collision there is
+        // persisted rather than reported. A pre-flight, not a mutex — DET
+        // cannot hold the manager lock across the network round-trip.
+        self.reject_taken_identity_index(seed_hash, identity_index)
+            .await?;
+
         let scope = Self::hd_scope(seed_hash);
         self.inner
             .secret_access
@@ -137,17 +176,11 @@ impl WalletBackend {
         if info.identity_manager.identity(&id).is_some() {
             return Ok(false);
         }
-        // Upstream buckets a wallet's identities on `(wallet_id,
-        // identity_index)`: a second identity at a taken index displaces the
-        // resident one in memory while both keep their rows, and the next
-        // launch cannot reattach the displaced one's keys — it rejects the
-        // whole wallet's saved data as damaged. DET's index is user-entered
-        // and carries no uniqueness, so this is the last gate before the write.
-        if let Some(occupant) = info.identity_manager.managed_identities().find(|managed| {
-            managed.wallet_id == Some(wallet_id) && managed.identity_index == Some(identity_index)
-        }) {
+        if let Some(occupant_id) =
+            index_occupant(&info.identity_manager, &wallet_id, identity_index)
+        {
             return Err(TaskError::IdentityIndexAlreadyTaken {
-                occupant_id: occupant.identity.id(),
+                occupant_id,
                 index: identity_index,
             });
         }
@@ -165,6 +198,74 @@ impl WalletBackend {
         }
     }
 
+    /// Fail with [`TaskError::IdentityIndexAlreadyTaken`] when another identity
+    /// on this wallet already holds `identity_index`.
+    ///
+    /// Read-lock only, so it is safe to call before a seed session is opened
+    /// and before any funds move.
+    async fn reject_taken_identity_index(
+        &self,
+        seed_hash: &WalletSeedHash,
+        identity_index: u32,
+    ) -> Result<(), TaskError> {
+        let wallet = self.resolve_wallet(seed_hash).await?;
+        let wallet_id = wallet.wallet_id();
+        let wm = wallet.wallet_manager().read().await;
+        let info = wm
+            .get_wallet_info(&wallet_id)
+            .ok_or(TaskError::WalletStateInconsistent)?;
+        match index_occupant(&info.identity_manager, &wallet_id, identity_index) {
+            Some(occupant_id) => Err(TaskError::IdentityIndexAlreadyTaken {
+                occupant_id,
+                index: identity_index,
+            }),
+            None => Ok(()),
+        }
+    }
+
+    /// Drop `identity_id` from the upstream `IdentityManager` of whichever
+    /// registered wallet holds it, so the identity index it occupied is
+    /// genuinely free again.
+    ///
+    /// Upstream records a tombstone rather than deleting: the identity's rows
+    /// stay on disk and its orphaned key rows become safe to skip at load,
+    /// which is exactly what keeps a removal from turning into the damaged-data
+    /// failure. Without this the index stays occupied by an identity the user
+    /// believes is gone — invisible to DET's own picker, but still able to
+    /// collide.
+    ///
+    /// Best-effort and idempotent: an identity no registered wallet manages is
+    /// `Ok(false)`. Touches only public identity data, never the seed.
+    pub(crate) async fn forget_identity(
+        &self,
+        identity_id: &dash_sdk::platform::Identifier,
+    ) -> Result<bool, TaskError> {
+        for wallet_id in self.inner.pwm.wallet_ids().await {
+            let Some(wallet) = self.inner.pwm.get_wallet(&wallet_id).await else {
+                continue;
+            };
+            let persister = wallet.persister().clone();
+            let mut wm = wallet.wallet_manager().write().await;
+            let Some(info) = wm.get_wallet_info_mut(&wallet_id) else {
+                continue;
+            };
+            if info.identity_manager.identity(identity_id).is_none() {
+                continue;
+            }
+            return match info
+                .identity_manager
+                .remove_identity(identity_id, &persister)
+            {
+                Ok(_) => Ok(true),
+                Err(platform_wallet::error::PlatformWalletError::IdentityNotFound(_)) => Ok(false),
+                Err(e) => Err(TaskError::WalletBackend {
+                    source: Arc::new(e),
+                }),
+            };
+        }
+        Ok(false)
+    }
+
     /// Top up an existing identity's credit balance from this wallet's
     /// UTXOs. Returns the post-top-up identity balance (credits).
     ///