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 48aa18d90..5c772f550 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, + }, + /// 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( + "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, + 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/identity/mod.rs b/src/backend_task/identity/mod.rs index 2b8b3d339..ecbe2acca 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 { @@ -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/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 { + 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::(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::(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 4871d7ed3..e6cd2466c 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; @@ -77,23 +77,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 `det-.sqlite` @@ -3786,6 +3769,82 @@ 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( + wallet_database_path(cold_dir.path(), Network::Testnet), + 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)] @@ -4077,6 +4136,171 @@ 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 +} + +/// 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. +/// +/// 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( @@ -4313,10 +4537,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 da3f2639e..8b2e47fd4 100644 --- a/src/wallet_backend/identity_ops.rs +++ b/src/wallet_backend/identity_ops.rs @@ -34,6 +34,30 @@ enum Funding { TopUpNotBound, } +/// 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 { + 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 @@ -50,6 +74,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, @@ -59,6 +89,15 @@ impl WalletBackend { identity_signer: &crate::model::qualified_identity::QualifiedIdentity, settings: Option, ) -> Result { + // 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 @@ -110,8 +149,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, @@ -144,6 +185,14 @@ impl WalletBackend { if info.identity_manager.identity(&id).is_some() { return Ok(false); } + if let Some(occupant_id) = + index_occupant(&info.identity_manager, &wallet_id, identity_index) + { + return Err(TaskError::IdentityIndexAlreadyTaken { + occupant_id, + index: identity_index, + }); + } match info.identity_manager.add_identity( identity.clone(), identity_index, @@ -158,6 +207,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 { + 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) + } + /// Test-only: the identity id this wallet's upstream manager resolves for /// `identity_id`. Upstream files managed identities by `(wallet, index)` and /// looks them up through a side index, so a foreign identity written into an @@ -431,12 +548,73 @@ 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(®istration_index); + info.core_wallet + .accounts + .identity_topup + .remove(®istration_index); + } + Funding::TopUpNotBound => { + kw.accounts.identity_topup_not_bound = None; + info.core_wallet.accounts.identity_topup_not_bound = None; + } + } + }) + } + + /// 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-