fix(wallet): stop a duplicate identity index from making a wallet unloadable

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)]

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(
@@ -303,6 +315,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

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

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,

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`
@@ -3802,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)]
@@ -4105,6 +4168,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 +4511,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"