From 3e0390d6941c3ff84806e7201c363b2e4fb58042 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 23 Jul 2026 11:53:20 +0000 Subject: [PATCH 1/2] fix(wallets): move alias rename into a backend task, off the UI thread MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The wallet rename dialog persisted the new alias by reading and writing the wallet-meta sidecar synchronously from UI code, violating the UI/backend-task boundary. Worse, the HD path read metadata with `WalletMetaView::get(...) .unwrap_or_default()`: `get` collapses a storage/read failure into `None`, so a failed read defaulted the meta and the follow-up `set` clobbered every other stored field (`is_main` / `core_wallet_name` / xpub / password fields) instead of surfacing the error. Add a fallible `WalletMetaView::try_get` (on the generic `SidecarView`) that distinguishes a genuinely absent row (`Ok(None)`) from an unreadable blob (`Err(KvSidecarStorage)`). Route both wallet kinds through new typed `WalletTask::RenameHdWallet` / `RenameSingleKeyWallet` variants that persist off the UI thread and return `WalletAliasRenamed` / `SingleKeyAliasRenamed`; the screen now dispatches the rename and updates its in-memory label only from the task result. The HD task reads through `try_get`, so a read fault aborts the rename rather than dropping the other sidecar fields. No new `TaskError` variants — existing `KvSidecarStorage` / `WalletNotFound` / `SingleKeyMetaStorage` / `ImportedKeyNotFound` / `InvalidWalletAliasLength` cover every path, and the two manual callsite banners are dropped in favor of centralized `TaskError`-driven display. Co-Authored-By: Claude Opus 4.8 --- src/backend_task/mod.rs | 20 ++ src/backend_task/wallet/mod.rs | 17 ++ src/backend_task/wallet/rename_wallet.rs | 346 +++++++++++++++++++++++ src/ui/wallets/wallets_screen/mod.rs | 162 +++++------ src/wallet_backend/sidecar.rs | 11 + src/wallet_backend/wallet_meta.rs | 70 +++++ 6 files changed, 535 insertions(+), 91 deletions(-) create mode 100644 src/backend_task/wallet/rename_wallet.rs diff --git a/src/backend_task/mod.rs b/src/backend_task/mod.rs index 63a01c6d6..1b957bcf0 100644 --- a/src/backend_task/mod.rs +++ b/src/backend_task/mod.rs @@ -567,6 +567,20 @@ pub enum BackendTaskSuccessResult { seed_hash: WalletSeedHash, address: String, }, + /// An HD wallet's alias was renamed and persisted to the wallet-meta + /// sidecar. Carries the new alias so the screen updates its in-memory label + /// only after the write succeeds. + WalletAliasRenamed { + seed_hash: WalletSeedHash, + alias: String, + }, + /// An imported single-key wallet's alias was renamed and persisted to the + /// single-key sidecar. Carries the new alias so the screen updates its + /// in-memory label only after the write succeeds. + SingleKeyAliasRenamed { + address: String, + alias: String, + }, /// The wallet's tracked asset locks, read off the UI thread through the /// upstream `AssetLockManager`. Carries the `seed_hash` so screens cache /// and match the result per wallet. @@ -1266,6 +1280,12 @@ impl AppContext { ) .await } + WalletTask::RenameHdWallet { seed_hash, alias } => { + self.rename_hd_wallet(seed_hash, alias) + } + WalletTask::RenameSingleKeyWallet { address, alias } => { + self.rename_single_key_wallet(address, alias) + } }; contextualize_wallet_backend_dapi_result(result, backend.as_ref()) diff --git a/src/backend_task/wallet/mod.rs b/src/backend_task/wallet/mod.rs index a1bf6be78..8aab39e4d 100644 --- a/src/backend_task/wallet/mod.rs +++ b/src/backend_task/wallet/mod.rs @@ -5,6 +5,7 @@ mod fund_platform_address_from_asset_lock; mod fund_platform_address_from_wallet_utxos; mod generate_platform_receive_address; mod generate_receive_address; +mod rename_wallet; mod sign_message_with_identity_key; mod sign_message_with_key; mod transfer_platform_credits; @@ -258,6 +259,22 @@ pub enum WalletTask { /// If false, fees are paid from extra wallet balance (recipient receives exact amount). fee_deduct_from_output: bool, }, + /// Persist a new alias for an HD wallet to the wallet-meta sidecar, off the + /// UI thread. Reads the existing metadata fallibly so a storage/read failure + /// surfaces instead of silently clobbering the other sidecar fields + /// (`is_main` / `core_wallet_name` / xpub / password fields); a genuinely + /// absent row is seeded fresh with the alias and the wallet's xpub. An empty + /// `alias` clears the name. + RenameHdWallet { + seed_hash: WalletSeedHash, + alias: String, + }, + /// Persist a new alias for an imported single-key wallet to the single-key + /// sidecar, off the UI thread. An empty `alias` clears the name. + RenameSingleKeyWallet { + address: String, + alias: String, + }, } #[cfg(test)] diff --git a/src/backend_task/wallet/rename_wallet.rs b/src/backend_task/wallet/rename_wallet.rs new file mode 100644 index 000000000..e5b89bf22 --- /dev/null +++ b/src/backend_task/wallet/rename_wallet.rs @@ -0,0 +1,346 @@ +use crate::backend_task::BackendTaskSuccessResult; +use crate::backend_task::error::TaskError; +use crate::context::AppContext; +use crate::model::wallet::WalletSeedHash; +use std::sync::Arc; + +impl AppContext { + /// Persist a new alias for an HD wallet to the wallet-meta sidecar. + /// + /// The existing metadata is read through the FALLIBLE + /// [`WalletMetaView::try_get`](crate::wallet_backend::WalletMetaView::try_get) + /// path: a storage/read failure aborts the rename instead of defaulting and + /// clobbering the other sidecar fields (`is_main` / `core_wallet_name` / + /// xpub / password fields) on the follow-up write. Only a genuinely absent + /// row seeds a fresh entry, carrying the wallet's xpub so the cold-boot + /// picker can still render the wallet without unlocking the seed. + /// + /// # Errors + /// + /// - [`TaskError::WalletNotFound`] when `seed_hash` matches no loaded wallet. + /// - [`TaskError::KvSidecarStorage`] when the sidecar cannot be read or written. + /// - [`TaskError::InvalidWalletAliasLength`] when `alias` exceeds the limit. + pub(crate) fn rename_hd_wallet( + self: &Arc, + seed_hash: WalletSeedHash, + alias: String, + ) -> Result { + // Existence + xpub: a seed hash matching no locally-stored wallet is a + // genuine `WalletNotFound`, resolved here where the DET-side wallet + // store lives rather than collapsed into a backend transient. + let xpub_encoded = self + .wallet_arc(&seed_hash)? + .read()? + .master_bip44_ecdsa_extended_public_key + .encode() + .to_vec(); + + let backend = self.wallet_backend()?; + let meta_view = backend.wallet_meta(); + // Fallible read: a storage/read failure aborts here instead of + // defaulting and clobbering the row's other fields on the write below. + // Only a genuinely absent row (`Ok(None)`) seeds a fresh default. + let mut meta = meta_view + .try_get(self.network, &seed_hash)? + .unwrap_or_default(); + meta.alias = alias.clone(); + if meta.xpub_encoded.is_empty() { + meta.xpub_encoded = xpub_encoded; + } + meta_view.set(self.network, &seed_hash, &meta)?; + + Ok(BackendTaskSuccessResult::WalletAliasRenamed { seed_hash, alias }) + } + + /// Persist a new alias for an imported single-key wallet to the single-key + /// sidecar, delegating to the typed + /// [`SingleKeyView::set_alias`](crate::wallet_backend::single_key::SingleKeyView::set_alias) + /// chokepoint (which validates the alias and refreshes the in-memory index). + /// + /// # Errors + /// + /// - [`TaskError::ImportedKeyNotFound`] when `address` was never imported. + /// - [`TaskError::SingleKeyMetaStorage`] when the sidecar cannot be written. + /// - [`TaskError::InvalidWalletAliasLength`] when `alias` exceeds the limit. + pub(crate) fn rename_single_key_wallet( + self: &Arc, + address: String, + alias: String, + ) -> Result { + let backend = self.wallet_backend()?; + backend + .single_key() + .set_alias(&address, Some(alias.clone()))?; + Ok(BackendTaskSuccessResult::SingleKeyAliasRenamed { address, alias }) + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::app::TaskResult; + use crate::app_dir::ensure_env_file; + use crate::context::connection_status::ConnectionStatus; + use crate::database::test_helpers::create_database_at_path; + use crate::model::user_role::UserRoleCell; + use crate::model::wallet::Wallet; + use crate::model::wallet::birth_height::WalletOrigin; + use crate::model::wallet::meta::WalletMeta; + use crate::utils::egui_mpsc::SenderAsync; + use crate::utils::tasks::TaskManager; + use crate::wallet_backend::DetScope; + use dash_sdk::dpp::dashcore::secp256k1::SecretKey; + use dash_sdk::dpp::dashcore::{Network, PrivateKey}; + use tempfile::TempDir; + use tokio::sync::mpsc::Receiver; + + /// An offline testnet context with one registered HD wallet whose backend + /// is wired (so `wallet_meta()` and `single_key()` are usable). Registration + /// writes an initial wallet-meta row. The receiver and temp dir must outlive + /// the context. + struct Fixture { + ctx: Arc, + seed_hash: WalletSeedHash, + _rx: Receiver, + _dir: TempDir, + } + + async fn fixture() -> Fixture { + let dir = tempfile::tempdir().expect("tempdir"); + let data_dir = dir.path().to_path_buf(); + ensure_env_file(&data_dir); + let db = Arc::new(create_database_at_path(&data_dir.join("data.db")).expect("db")); + let app_kv = AppContext::open_app_kv(&data_dir).expect("app kv"); + let secret_store = AppContext::open_secret_store(&data_dir).expect("secret store"); + let ctx = AppContext::new( + data_dir, + Network::Testnet, + db, + Arc::new(TaskManager::new()), + Arc::new(ConnectionStatus::new()), + egui::Context::default(), + app_kv, + secret_store, + UserRoleCell::default(), + ) + .expect("offline testnet AppContext"); + + let (tx, rx) = tokio::sync::mpsc::channel::(32); + let sender = SenderAsync::new(tx, ctx.egui_ctx().clone()); + + let seed = [0x5Au8; 64]; + let wallet = + Wallet::new_from_seed(seed, Network::Testnet, None, None).expect("build wallet"); + let seed_hash = wallet.seed_hash(); + ctx.register_wallet(wallet, &seed, WalletOrigin::Fresh) + .expect("register wallet"); + ctx.ensure_wallet_backend(sender) + .await + .expect("wire wallet backend offline"); + + Fixture { + ctx, + seed_hash, + _rx: rx, + _dir: dir, + } + } + + /// A bare `u8` whose bincode string-length varint runs past the end of the + /// blob — unreadable as either the current or the legacy `WalletMeta` shape, + /// so it forces a sidecar READ failure rather than a decode-to-default. + const UNREADABLE_META_SENTINEL: u8 = 2; + + /// The headline regression: a FAILED metadata read must surface as an error + /// and leave the stored blob untouched, never silently default-and-overwrite + /// (which would drop `is_main` / `core_wallet_name` / xpub / password fields). + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn rename_hd_read_failure_surfaces_and_does_not_clobber() { + let f = fixture().await; + let key = crate::wallet_backend::wallet_meta::key_for(f.ctx.network, &f.seed_hash); + // Overwrite the registration-written meta with an unreadable blob. + f.ctx + .app_kv() + .put(DetScope::Global, &key, &UNREADABLE_META_SENTINEL) + .expect("plant unreadable blob"); + + let err = f + .ctx + .rename_hd_wallet(f.seed_hash, "renamed".into()) + .expect_err("a failed metadata read must surface, not silently overwrite"); + assert!( + matches!( + err, + TaskError::KvSidecarStorage { + sidecar: "wallet_meta", + .. + } + ), + "got {err:?}" + ); + + // The unreadable blob is untouched — the rename aborted before writing. + let raw: Option = f + .ctx + .app_kv() + .get(DetScope::Global, &key) + .expect("raw read"); + assert_eq!( + raw, + Some(UNREADABLE_META_SENTINEL), + "the unreadable blob must not be overwritten by a defaulted meta" + ); + } + + /// Renaming preserves every non-alias field of an existing meta row. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn rename_hd_preserves_other_meta_fields() { + let f = fixture().await; + let backend = f.ctx.wallet_backend().expect("backend"); + let seeded = WalletMeta { + alias: "old".into(), + is_main: true, + core_wallet_name: Some("local-dashd".into()), + xpub_encoded: vec![0xAB; 78], + uses_password: true, + password_hint: Some("granny's birthday".into()), + }; + backend + .wallet_meta() + .set(f.ctx.network, &f.seed_hash, &seeded) + .expect("seed meta"); + + let result = f + .ctx + .rename_hd_wallet(f.seed_hash, "renamed".into()) + .expect("rename"); + assert!( + matches!( + &result, + BackendTaskSuccessResult::WalletAliasRenamed { seed_hash, alias } + if *seed_hash == f.seed_hash && alias == "renamed" + ), + "got {result:?}" + ); + + let after = backend + .wallet_meta() + .get(f.ctx.network, &f.seed_hash) + .expect("meta present"); + assert_eq!(after.alias, "renamed", "alias updated"); + assert!(after.is_main, "is_main preserved"); + assert_eq!( + after.core_wallet_name.as_deref(), + Some("local-dashd"), + "core wallet name preserved" + ); + assert_eq!(after.xpub_encoded, vec![0xAB; 78], "xpub preserved"); + assert!(after.uses_password, "uses_password preserved"); + assert_eq!( + after.password_hint.as_deref(), + Some("granny's birthday"), + "password hint preserved" + ); + } + + /// A genuinely absent meta row is seeded fresh with the alias and the + /// wallet's xpub (so the cold-boot picker renders without the seed). + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn rename_hd_seeds_fresh_meta_when_absent() { + let f = fixture().await; + let backend = f.ctx.wallet_backend().expect("backend"); + backend + .wallet_meta() + .delete(f.ctx.network, &f.seed_hash) + .expect("delete registration meta"); + + f.ctx + .rename_hd_wallet(f.seed_hash, "fresh".into()) + .expect("rename"); + + let after = backend + .wallet_meta() + .get(f.ctx.network, &f.seed_hash) + .expect("meta present after rename"); + assert_eq!(after.alias, "fresh"); + let expected_xpub = f + .ctx + .wallet_arc(&f.seed_hash) + .expect("wallet") + .read() + .expect("read") + .master_bip44_ecdsa_extended_public_key + .encode() + .to_vec(); + assert_eq!( + after.xpub_encoded, expected_xpub, + "a fresh meta seeds the wallet xpub" + ); + assert!(!after.is_main, "fresh meta is not main"); + assert!( + after.core_wallet_name.is_none(), + "fresh meta has no core link" + ); + } + + /// Renaming an unknown seed hash is a genuine `WalletNotFound`, not a + /// backend transient. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn rename_hd_unknown_seed_hash_returns_wallet_not_found() { + let f = fixture().await; + let unknown: WalletSeedHash = [0xAB; 32]; + let err = f + .ctx + .rename_hd_wallet(unknown, "x".into()) + .expect_err("an unknown wallet must fail"); + assert!(matches!(err, TaskError::WalletNotFound), "got {err:?}"); + } + + /// The single-key rename persists the new alias through the typed chokepoint + /// and returns the `SingleKeyAliasRenamed` result carrying it. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn rename_single_key_persists_and_returns_typed_result() { + let f = fixture().await; + let backend = f.ctx.wallet_backend().expect("backend"); + // Mint a deterministic throwaway key rather than committing a WIF. + let sk = SecretKey::from_byte_array(&[0x11u8; 32]).expect("valid scalar"); + let wif = PrivateKey::new(sk, Network::Testnet).to_wif(); + let imported = backend + .single_key() + .import_wif(&wif, Some("old name".into())) + .expect("import"); + let address = imported.address.clone(); + + let result = f + .ctx + .rename_single_key_wallet(address.clone(), "new name".into()) + .expect("rename"); + assert!( + matches!( + &result, + BackendTaskSuccessResult::SingleKeyAliasRenamed { address: a, alias } + if *a == address && alias == "new name" + ), + "got {result:?}" + ); + + let listed = backend.single_key().list(); + let entry = listed + .iter() + .find(|e| e.address == address) + .expect("imported key present"); + assert_eq!(entry.alias.as_deref(), Some("new name"), "alias persisted"); + } + + /// Renaming an address that was never imported surfaces the typed + /// `ImportedKeyNotFound`. + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn rename_single_key_unknown_address_is_typed_not_found() { + let f = fixture().await; + let err = f + .ctx + .rename_single_key_wallet("yNeverImported".into(), "x".into()) + .expect_err("an unknown address must fail"); + assert!(matches!(err, TaskError::ImportedKeyNotFound), "got {err:?}"); + } +} diff --git a/src/ui/wallets/wallets_screen/mod.rs b/src/ui/wallets/wallets_screen/mod.rs index eff8503ed..21dbf0a06 100644 --- a/src/ui/wallets/wallets_screen/mod.rs +++ b/src/ui/wallets/wallets_screen/mod.rs @@ -9,6 +9,7 @@ use crate::app::{AppAction, DesiredAppAction}; use crate::backend_task::BackendTask; use crate::backend_task::core::CoreTask; use crate::backend_task::error::TaskError; +use crate::backend_task::wallet::WalletTask; use crate::context::AppContext; use crate::context::connection_status::spv_phase_summary; use crate::context::feature_gate::FeatureGate; @@ -167,6 +168,19 @@ fn plan_account_tabs( tabs } +/// A wallet rename confirmed in the dialog, pending dispatch as a backend task. +/// Distinguishes the two wallet kinds by the identifier each rename task needs. +enum PendingWalletRename { + Hd { + seed_hash: WalletSeedHash, + alias: String, + }, + SingleKey { + address: String, + alias: String, + }, +} + pub struct WalletsBalancesScreen { selected_wallet: Option>>, selected_single_key_wallet: Option>>, @@ -177,6 +191,11 @@ pub struct WalletsBalancesScreen { show_rename_dialog: bool, rename_dialog_opening_guard: ModalOpeningGuard, rename_input: String, + /// A confirmed rename awaiting dispatch as a backend task. Set when the user + /// confirms the rename dialog; drained into a `WalletTask` so the sidecar + /// write runs off the UI thread. The in-memory alias updates only from the + /// task's success result. + pending_rename: Option, wallet_unlock_popup: WalletUnlockPopup, show_sk_unlock_dialog: bool, sk_password_input: PasswordInput, @@ -319,6 +338,7 @@ impl WalletsBalancesScreen { show_rename_dialog: false, rename_dialog_opening_guard: ModalOpeningGuard::default(), rename_input: String::new(), + pending_rename: None, wallet_unlock_popup: WalletUnlockPopup::new(), show_sk_unlock_dialog: false, sk_password_input: PasswordInput::new().with_hint_text("Enter password"), @@ -2619,103 +2639,30 @@ impl ScreenLike for WalletsBalancesScreen { return; } - // Handle HD wallet rename + // Queue the rename for dispatch as a backend + // task; the sidecar write runs off the UI thread + // and the in-memory alias updates only from the + // task result. Persistence is identical across + // wallet kinds — only the identifier differs. + let new_alias = self.rename_input.clone(); if let Some(selected_wallet) = &self.selected_wallet { - // T-W-01: alias persistence goes - // through the wallet-meta sidecar. - // The cold-boot picker reads from - // the same key shape, so the new - // name surfaces on the next launch - // without touching the legacy - // `wallet` table. - let (seed_hash, xpub_encoded) = { - let wallet = selected_wallet.read_recover(); - ( - wallet.seed_hash(), - wallet - .master_bip44_ecdsa_extended_public_key - .encode() - .to_vec(), - ) - }; - let new_alias = self.rename_input.clone(); - let persisted = match self.app_context.wallet_backend() { - Ok(backend) => { - let meta_view = backend.wallet_meta(); - let mut meta = meta_view - .get(self.app_context.network, &seed_hash) - .unwrap_or_default(); - meta.alias = new_alias.clone(); - if meta.xpub_encoded.is_empty() { - meta.xpub_encoded = xpub_encoded; - } - meta_view.set( - self.app_context.network, - &seed_hash, - &meta, - ) - } - Err(error) => Err(error), - }; - match persisted { - Ok(()) => { - selected_wallet.write_recover().alias = Some(new_alias); - self.show_rename_dialog = false; - self.rename_input.clear(); - } - Err(error) => { - MessageBanner::set_global( - ctx, - "The wallet name could not be saved. Check available disk space and try again.", - MessageType::Error, - ) - .with_details(error); - } - } - } - // Handle single key wallet rename - else if let Some(selected_sk_wallet) = + let seed_hash = selected_wallet.read_recover().seed_hash(); + self.pending_rename = Some(PendingWalletRename::Hd { + seed_hash, + alias: new_alias, + }); + } else if let Some(selected_sk_wallet) = &self.selected_single_key_wallet { - // Persist FIRST so the in-memory display - // alias and the "renamed" outcome only - // reflect a durable change. Alias - // persistence goes through the modern - // single-key sidecar (matching the - // HD-wallet rename path above), so the - // new name survives a restart without - // touching the legacy `single_key_wallet` - // table. let address = selected_sk_wallet.read_recover().address.to_string(); - let new_alias = self.rename_input.clone(); - let persisted = match self.app_context.wallet_backend() { - Ok(backend) => backend - .single_key() - .set_alias(&address, Some(new_alias.clone())), - Err(e) => Err(e), - }; - match persisted { - Ok(()) => { - selected_sk_wallet.write_recover().alias = - Some(new_alias); - self.show_rename_dialog = false; - self.rename_input.clear(); - } - Err(e) => { - MessageBanner::set_global( - ctx, - "Could not rename the imported key. Check available disk space and try again." - .to_string(), - MessageType::Error, - ) - .with_details(&e); - } - } - } else { - self.show_rename_dialog = false; - self.rename_input.clear(); + self.pending_rename = Some(PendingWalletRename::SingleKey { + address, + alias: new_alias, + }); } + self.show_rename_dialog = false; + self.rename_input.clear(); } }); }); @@ -2733,6 +2680,20 @@ impl ScreenLike for WalletsBalancesScreen { } } + // Drain a confirmed rename into a backend task that persists the alias + // off the UI thread. The in-memory label updates from the task result. + if let Some(pending) = self.pending_rename.take() { + let task = match pending { + PendingWalletRename::Hd { seed_hash, alias } => { + WalletTask::RenameHdWallet { seed_hash, alias } + } + PendingWalletRename::SingleKey { address, alias } => { + WalletTask::RenameSingleKeyWallet { address, alias } + } + }; + action |= AppAction::BackendTask(BackendTask::WalletTask(task)); + } + // HD Wallet unlock popup if let Some(wallet_arc) = &self.selected_wallet.clone() { let result = self @@ -3013,6 +2974,25 @@ impl ScreenLike for WalletsBalancesScreen { crate::ui::BackendTaskSuccessResult::TrackedAssetLocks { seed_hash, locks } => { self.asset_lock_cache.store(seed_hash, locks); } + crate::ui::BackendTaskSuccessResult::WalletAliasRenamed { seed_hash, alias } => { + // Update the in-memory label only now that the sidecar write + // succeeded. Read the guard into a local first so the read lock + // is released before taking the write lock on the same wallet. + if let Some(wallet) = &self.selected_wallet { + let is_target = wallet.read_recover().seed_hash() == seed_hash; + if is_target { + wallet.write_recover().alias = Some(alias); + } + } + } + crate::ui::BackendTaskSuccessResult::SingleKeyAliasRenamed { address, alias } => { + if let Some(wallet) = &self.selected_single_key_wallet { + let is_target = wallet.read_recover().address.to_string() == address; + if is_target { + wallet.write_recover().alias = Some(alias); + } + } + } crate::ui::BackendTaskSuccessResult::GeneratedReceiveAddress { seed_hash, address } => { let is_selected = self .selected_wallet diff --git a/src/wallet_backend/sidecar.rs b/src/wallet_backend/sidecar.rs index 8f5efe999..f399b831d 100644 --- a/src/wallet_backend/sidecar.rs +++ b/src/wallet_backend/sidecar.rs @@ -126,6 +126,17 @@ impl<'a, V: SidecarValue> SidecarView<'a, V> { } } + /// Fetch the value for `id`, distinguishing "genuinely absent" (`Ok(None)`) + /// from "the blob is present but could not be read" (`Err`). Unlike + /// [`Self::get`], a read/decode/schema failure is surfaced through the + /// view's error envelope instead of degrading to `None`. Callers that must + /// not blindly overwrite existing fields on a failed read use this so a + /// storage fault aborts the write rather than clobbering the stored blob. + pub(crate) fn try_get(&self, network: Network, id: &SidecarId) -> Result, TaskError> { + let key = sidecar_key(network, self.infix, id); + V::read(self.kv, self.scope.det_scope(id), &key).map_err(self.map_err) + } + /// Upsert the value for `id`. Re-writing the same value is an idempotent /// overwrite (DetKv upserts by key). pub(crate) fn set(&self, network: Network, id: &SidecarId, value: &V) -> Result<(), TaskError> { diff --git a/src/wallet_backend/wallet_meta.rs b/src/wallet_backend/wallet_meta.rs index a71407cd5..a62afe93d 100644 --- a/src/wallet_backend/wallet_meta.rs +++ b/src/wallet_backend/wallet_meta.rs @@ -119,6 +119,21 @@ impl<'a> WalletMetaView<'a> { self.0.get(network, seed_hash) } + /// Fetch the metadata for a single wallet, surfacing a read failure instead + /// of degrading it to absence. `Ok(None)` means the row was never written; + /// `Err(`[`TaskError::KvSidecarStorage`]`)` means the blob is present but + /// unreadable (storage fault, schema mismatch, corrupt bytes). A rename that + /// only edits the alias must not overwrite the row on a failed read — a + /// blind `set` after a swallowed error would drop every other stored field — + /// so it reads through this fallible path. + pub fn try_get( + &self, + network: Network, + seed_hash: &WalletSeedHash, + ) -> Result, TaskError> { + self.0.try_get(network, seed_hash) + } + /// Upsert the metadata for a single wallet. Re-writing the same value is an /// idempotent overwrite (DetKv upserts by key). pub fn set( @@ -316,6 +331,61 @@ mod tests { assert_eq!(listed, vec![(seed, meta("ok", false, None))]); } + /// W-META-VIEW-008 — `try_get` distinguishes the three outcomes that + /// `get` collapses to `None`: a genuinely absent row (`Ok(None)`), a + /// present readable row (`Ok(Some)`), and a present-but-unreadable blob + /// (`Err(KvSidecarStorage)`). A rename that only edits the alias relies on + /// this to abort rather than overwrite the other stored fields on a failed + /// read. + #[test] + fn try_get_distinguishes_absent_present_and_read_failure() { + let kv = kv(); + let view = WalletMetaView::new(&kv); + + // Absent: never written. + let absent: WalletSeedHash = [0x01; 32]; + assert_eq!( + view.try_get(Network::Testnet, &absent) + .expect("absent read"), + None + ); + + // Present: round-trips as the stored value. + let present: WalletSeedHash = [0x02; 32]; + let m = meta("paycheque", true, Some("local-dashd")); + view.set(Network::Testnet, &present, &m).expect("set"); + assert_eq!( + view.try_get(Network::Testnet, &present) + .expect("present read"), + Some(m) + ); + + // Present-but-unreadable: plant a blob that decodes as neither the + // current nor the legacy shape (a bare `u8` whose string-length varint + // runs past the end). `get` swallows this to `None`; `try_get` surfaces + // it as the sidecar-storage error. + let corrupt: WalletSeedHash = [0x03; 32]; + let key = key_for(Network::Testnet, &corrupt); + kv.put(DetScope::Global, &key, &2u8) + .expect("plant corrupt blob"); + let err = view + .try_get(Network::Testnet, &corrupt) + .expect_err("a present-but-unreadable blob must surface as an error"); + assert!( + matches!( + err, + TaskError::KvSidecarStorage { + sidecar: "wallet_meta", + .. + } + ), + "got {err:?}" + ); + // The same blob still degrades to `None` through `get` — the contrast + // the rename fix depends on. + assert_eq!(view.get(Network::Testnet, &corrupt), None); + } + /// W-META-VIEW-007 — the canonical key shape uses base58 encoding /// for the 32-byte seed hash. Locks the shape so a future change /// (hex, etc.) needs an explicit migration. From cfe7d451c070240dd580cca9bf22d376ebaf02ab Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 23 Jul 2026 15:11:32 +0000 Subject: [PATCH 2/2] fix(wallets): harden alias rename against concurrent renames, removal races, and lost dialog input Follow-up to the async-rename refactor: fixes correctness regressions the sync-to-async move introduced in its own code path. - Serialize HD-wallet alias renames per wallet and hold the wallet-store guard across the sidecar read-modify-write, so a rename can no longer race a concurrent removal of the same wallet or another rename of it. - Hold the single-key index lock through sidecar persistence so the in-memory index and the stored alias stay consistent under concurrent renames. - Apply a successful rename to the wallet by its own identity instead of gating on current UI selection, so switching wallets mid-rename no longer strands a stale alias until app restart. - Keep the rename dialog open, disabled, and pre-filled while a save is in flight or after a failure, and disable the Rename entry point for the same span, so a typed alias survives a failure and a save can't be interrupted by reopening the dialog. - Add deterministic (Condvar-gated) concurrency regression tests for both rename paths and kittest coverage for the full confirm-dispatch-result cycle. - Add a CHANGELOG entry. Co-Authored-By: Claude Sonnet 5 Co-Authored-By: Codex Sol --- CHANGELOG.md | 7 + src/backend_task/mod.rs | 13 + src/backend_task/wallet/rename_wallet.rs | 208 +++++++++- src/context/mod.rs | 13 + src/ui/wallets/wallets_screen/mod.rs | 266 ++++++------ src/wallet_backend/single_key.rs | 190 ++++++++- tests/kittest/wallets_screen.rs | 505 ++++++++++++++++++++++- 7 files changed, 1066 insertions(+), 136 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 48d500acc..da60c2f06 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -46,6 +46,13 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **Wallet rename consistency**: renaming a wallet no longer overwrites other + saved wallet details when metadata cannot be read. Overlapping renames and + wallet removals also keep displayed aliases and deleted-wallet metadata + consistent. The rename dialog remains open with the entered name available + for retry when saving fails, and its controls stay disabled while a save is + in progress. + - **Shielded availability notice**: now distinguishes when the connected network does not support shielded sending from when the current interface mode does not unlock it. diff --git a/src/backend_task/mod.rs b/src/backend_task/mod.rs index 1b957bcf0..cc4431920 100644 --- a/src/backend_task/mod.rs +++ b/src/backend_task/mod.rs @@ -331,6 +331,8 @@ pub enum BackendTaskContext { ScheduledVoteSweep { network: Network }, /// Receive-address derivation for one wallet's deposit flow. GenerateReceiveAddress { seed_hash: WalletSeedHash }, + /// One HD-wallet or imported-key alias update. + WalletRename(WalletTask), /// A known backend task that needs no finer UI correlation. Other, /// An error emitted without an originating backend task. @@ -384,6 +386,13 @@ impl BackendTaskContext { _ => None, } } + + pub(crate) fn wallet_rename_task(&self) -> Option<&WalletTask> { + match self.operation() { + Self::WalletRename(task) => Some(task), + _ => None, + } + } } impl From<&BackendTask> for BackendTaskContext { @@ -425,6 +434,10 @@ impl From<&BackendTask> for BackendTaskContext { seed_hash: *seed_hash, } } + BackendTask::WalletTask( + task @ (WalletTask::RenameHdWallet { .. } + | WalletTask::RenameSingleKeyWallet { .. }), + ) => Self::WalletRename(task.clone()), _ => Self::Other, } } diff --git a/src/backend_task/wallet/rename_wallet.rs b/src/backend_task/wallet/rename_wallet.rs index e5b89bf22..74add276c 100644 --- a/src/backend_task/wallet/rename_wallet.rs +++ b/src/backend_task/wallet/rename_wallet.rs @@ -25,11 +25,16 @@ impl AppContext { seed_hash: WalletSeedHash, alias: String, ) -> Result { - // Existence + xpub: a seed hash matching no locally-stored wallet is a - // genuine `WalletNotFound`, resolved here where the DET-side wallet - // store lives rather than collapsed into a backend transient. - let xpub_encoded = self - .wallet_arc(&seed_hash)? + let rename_lock = self.hd_wallet_rename_lock(seed_hash); + let _rename_guard = rename_lock + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + + // Retaining the map guard makes existence and persistence one operation + // relative to wallet removal. The inner wallet guard is disk-I/O-free. + let wallets = self.wallets.read()?; + let wallet = wallets.get(&seed_hash).ok_or(TaskError::WalletNotFound)?; + let xpub_encoded = wallet .read()? .master_bip44_ecdsa_extended_public_key .encode() @@ -88,9 +93,13 @@ mod tests { use crate::model::wallet::meta::WalletMeta; use crate::utils::egui_mpsc::SenderAsync; use crate::utils::tasks::TaskManager; - use crate::wallet_backend::DetScope; + use crate::wallet_backend::kv_test_support::InMemoryKv; + use crate::wallet_backend::{DetKv, DetScope}; use dash_sdk::dpp::dashcore::secp256k1::SecretKey; use dash_sdk::dpp::dashcore::{Network, PrivateKey}; + use platform_wallet_storage::{KvError, KvStore, ObjectId}; + use std::sync::{Condvar, Mutex}; + use std::time::Duration; use tempfile::TempDir; use tokio::sync::mpsc::Receiver; @@ -105,12 +114,15 @@ mod tests { _dir: TempDir, } - async fn fixture() -> Fixture { + async fn fixture_with_app_kv(app_kv: Arc) -> Fixture { let dir = tempfile::tempdir().expect("tempdir"); + fixture_from_parts(dir, app_kv).await + } + + async fn fixture_from_parts(dir: TempDir, app_kv: Arc) -> Fixture { let data_dir = dir.path().to_path_buf(); ensure_env_file(&data_dir); let db = Arc::new(create_database_at_path(&data_dir.join("data.db")).expect("db")); - let app_kv = AppContext::open_app_kv(&data_dir).expect("app kv"); let secret_store = AppContext::open_secret_store(&data_dir).expect("secret store"); let ctx = AppContext::new( data_dir, @@ -146,6 +158,87 @@ mod tests { } } + async fn fixture() -> Fixture { + let dir = tempfile::tempdir().expect("tempdir"); + let app_kv = AppContext::open_app_kv(dir.path()).expect("app kv"); + fixture_from_parts(dir, app_kv).await + } + + #[derive(Default)] + struct ReadGateState { + armed: bool, + intercepted: bool, + released: bool, + } + + #[derive(Default)] + struct FirstWalletMetaReadGate { + inner: InMemoryKv, + state: Mutex, + changed: Condvar, + } + + impl FirstWalletMetaReadGate { + fn arm(&self) { + let mut state = self.state.lock().expect("gate state"); + *state = ReadGateState { + armed: true, + ..Default::default() + }; + } + + fn wait_until_intercepted(&self) { + let state = self.state.lock().expect("gate state"); + let (state, timeout) = self + .changed + .wait_timeout_while(state, Duration::from_secs(5), |state| !state.intercepted) + .expect("gate wait"); + assert!( + !timeout.timed_out() && state.intercepted, + "rename read gate" + ); + } + + fn release(&self) { + let mut state = self.state.lock().expect("gate state"); + state.released = true; + self.changed.notify_all(); + } + } + + impl KvStore for FirstWalletMetaReadGate { + fn get(&self, scope: &ObjectId, key: &str) -> Result>, KvError> { + let value = self.inner.get(scope, key)?; + let mut state = self.state.lock().expect("gate state"); + if state.armed && !state.intercepted && key.contains(":wallet_meta:") { + state.intercepted = true; + self.changed.notify_all(); + state = self + .changed + .wait_while(state, |state| !state.released) + .expect("gate release"); + } + drop(state); + Ok(value) + } + + fn put(&self, scope: &ObjectId, key: &str, value: &[u8]) -> Result<(), KvError> { + self.inner.put(scope, key, value) + } + + fn delete(&self, scope: &ObjectId, key: &str) -> Result<(), KvError> { + self.inner.delete(scope, key) + } + + fn list_keys( + &self, + scope: &ObjectId, + prefix: Option<&str>, + ) -> Result, KvError> { + self.inner.list_keys(scope, prefix) + } + } + /// A bare `u8` whose bincode string-length varint runs past the end of the /// blob — unreadable as either the current or the legacy `WalletMeta` shape, /// so it forces a sidecar READ failure rather than a decode-to-default. @@ -296,6 +389,105 @@ mod tests { assert!(matches!(err, TaskError::WalletNotFound), "got {err:?}"); } + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn overlapping_hd_renames_serialize_so_later_invocation_wins() { + let store = Arc::new(FirstWalletMetaReadGate::default()); + let f = fixture_with_app_kv(Arc::new(DetKv::from_store(store.clone()))).await; + store.arm(); + + let first_ctx = f.ctx.clone(); + let seed_hash = f.seed_hash; + let first = + std::thread::spawn(move || first_ctx.rename_hd_wallet(seed_hash, "first".into())); + store.wait_until_intercepted(); + + let later_ctx = f.ctx.clone(); + let (later_tx, later_rx) = std::sync::mpsc::channel(); + let later = std::thread::spawn(move || { + let result = later_ctx.rename_hd_wallet(seed_hash, "later".into()); + later_tx.send(result).expect("send later result"); + }); + + let later_while_first_blocked = later_rx.recv_timeout(Duration::from_secs(1)).ok(); + store.release(); + first + .join() + .expect("first rename thread") + .expect("first rename"); + let later_result = match later_while_first_blocked { + Some(result) => result, + None => later_rx + .recv_timeout(Duration::from_secs(5)) + .expect("later rename completion"), + }; + later_result.expect("later rename"); + later.join().expect("later rename thread"); + + let alias = f + .ctx + .wallet_backend() + .expect("backend") + .wallet_meta() + .try_get(f.ctx.network, &seed_hash) + .expect("read final meta") + .expect("final meta") + .alias; + assert_eq!( + alias, "later", + "the later invocation must be the final persisted alias" + ); + } + + #[tokio::test(flavor = "multi_thread", worker_threads = 2)] + async fn remove_wallet_racing_rename_does_not_resurrect_wallet_meta() { + let store = Arc::new(FirstWalletMetaReadGate::default()); + let f = fixture_with_app_kv(Arc::new(DetKv::from_store(store.clone()))).await; + store.arm(); + + let rename_ctx = f.ctx.clone(); + let seed_hash = f.seed_hash; + let rename = std::thread::spawn(move || { + rename_ctx.rename_hd_wallet(seed_hash, "rename in flight".into()) + }); + store.wait_until_intercepted(); + + let remove_ctx = f.ctx.clone(); + let runtime_handle = tokio::runtime::Handle::current(); + let (remove_tx, remove_rx) = std::sync::mpsc::channel(); + let remove = std::thread::spawn(move || { + let _runtime_guard = runtime_handle.enter(); + let result = remove_ctx.remove_wallet(&seed_hash); + remove_tx.send(result).expect("send removal result"); + }); + + let removal_while_rename_blocked = remove_rx.recv_timeout(Duration::from_secs(1)).ok(); + store.release(); + rename + .join() + .expect("rename thread") + .expect("rename completion"); + let removal_result = match removal_while_rename_blocked { + Some(result) => result, + None => remove_rx + .recv_timeout(Duration::from_secs(5)) + .expect("removal completion"), + }; + removal_result.expect("remove wallet"); + remove.join().expect("remove thread"); + + let meta = f + .ctx + .wallet_backend() + .expect("backend") + .wallet_meta() + .try_get(f.ctx.network, &seed_hash) + .expect("read final meta"); + assert!( + meta.is_none(), + "a completed removal must leave wallet metadata deleted" + ); + } + /// The single-key rename persists the new alias through the typed chokepoint /// and returns the `SingleKeyAliasRenamed` result carrying it. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] diff --git a/src/context/mod.rs b/src/context/mod.rs index 5dce7ef60..f26aa50eb 100644 --- a/src/context/mod.rs +++ b/src/context/mod.rs @@ -115,6 +115,9 @@ pub struct AppContext { /// check → fetch → insert → seal span. See [`identity_load_registry`]. identity_loads: identity_load_registry::SharedLoadRegistry, pub(crate) wallets: RwLock>>>, + /// Per-wallet guards covering the complete wallet-meta alias update. + /// Different wallets remain independent while same-wallet renames serialize. + hd_wallet_rename_locks: Mutex>>>, pub(crate) single_key_wallets: RwLock>>>, /// Hard override that keeps this context's UI still whatever the role — set by /// automated tests through [`AppState::with_animations`](crate::app::AppState::with_animations). @@ -270,6 +273,15 @@ impl std::fmt::Debug for SecretPromptSlot { } impl AppContext { + pub(crate) fn hd_wallet_rename_lock(&self, seed_hash: WalletSeedHash) -> Arc> { + self.hd_wallet_rename_locks + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .entry(seed_hash) + .or_default() + .clone() + } + pub(crate) fn try_claim_contact_request_action( &self, request_id: Identifier, @@ -425,6 +437,7 @@ impl AppContext { identity_autodiscovery_fired: AtomicBool::new(false), identity_loads: Default::default(), wallets: RwLock::new(wallets), + hd_wallet_rename_locks: Mutex::new(HashMap::new()), single_key_wallets: RwLock::new(single_key_wallets), animations_disabled: AtomicBool::new(false), cached_settings: RwLock::new(None), diff --git a/src/ui/wallets/wallets_screen/mod.rs b/src/ui/wallets/wallets_screen/mod.rs index 21dbf0a06..2a30b7df1 100644 --- a/src/ui/wallets/wallets_screen/mod.rs +++ b/src/ui/wallets/wallets_screen/mod.rs @@ -6,10 +6,10 @@ mod single_key_view; pub(crate) use single_key_view::SINGLE_KEY_SEND_UNAVAILABLE; use crate::app::{AppAction, DesiredAppAction}; -use crate::backend_task::BackendTask; use crate::backend_task::core::CoreTask; use crate::backend_task::error::TaskError; use crate::backend_task::wallet::WalletTask; +use crate::backend_task::{BackendTask, BackendTaskContext}; use crate::context::AppContext; use crate::context::connection_status::spv_phase_summary; use crate::context::feature_gate::FeatureGate; @@ -84,6 +84,29 @@ enum PendingWalletRemoval { }, } +fn rename_result_matches_task( + task: &WalletTask, + result: &crate::ui::BackendTaskSuccessResult, +) -> bool { + match (task, result) { + ( + WalletTask::RenameHdWallet { + seed_hash: task_seed_hash, + alias: task_alias, + }, + crate::ui::BackendTaskSuccessResult::WalletAliasRenamed { seed_hash, alias }, + ) => task_seed_hash == seed_hash && task_alias == alias, + ( + WalletTask::RenameSingleKeyWallet { + address: task_address, + alias: task_alias, + }, + crate::ui::BackendTaskSuccessResult::SingleKeyAliasRenamed { address, alias }, + ) => task_address == address && task_alias == alias, + _ => false, + } +} + impl Default for AccountTab { fn default() -> Self { AccountTab::Category(AccountCategory::Bip44, Some(0)) @@ -168,19 +191,6 @@ fn plan_account_tabs( tabs } -/// A wallet rename confirmed in the dialog, pending dispatch as a backend task. -/// Distinguishes the two wallet kinds by the identifier each rename task needs. -enum PendingWalletRename { - Hd { - seed_hash: WalletSeedHash, - alias: String, - }, - SingleKey { - address: String, - alias: String, - }, -} - pub struct WalletsBalancesScreen { selected_wallet: Option>>, selected_single_key_wallet: Option>>, @@ -188,14 +198,12 @@ pub struct WalletsBalancesScreen { sort_column: SortColumn, sort_order: SortOrder, refreshing: bool, - show_rename_dialog: bool, rename_dialog_opening_guard: ModalOpeningGuard, - rename_input: String, - /// A confirmed rename awaiting dispatch as a backend task. Set when the user - /// confirms the rename dialog; drained into a `WalletTask` so the sidecar - /// write runs off the UI thread. The in-memory alias updates only from the - /// task's success result. - pending_rename: Option, + /// The complete rename request shown in the dialog, including its stable + /// target identifier and editable alias. + rename_task: Option, + /// The exact in-flight dispatch whose result may close or re-enable the dialog. + pending_rename_context: Option, wallet_unlock_popup: WalletUnlockPopup, show_sk_unlock_dialog: bool, sk_password_input: PasswordInput, @@ -335,10 +343,9 @@ impl WalletsBalancesScreen { sort_column: SortColumn::Index, sort_order: SortOrder::Ascending, refreshing: false, - show_rename_dialog: false, rename_dialog_opening_guard: ModalOpeningGuard::default(), - rename_input: String::new(), - pending_rename: None, + rename_task: None, + pending_rename_context: None, wallet_unlock_popup: WalletUnlockPopup::new(), show_sk_unlock_dialog: false, sk_password_input: PasswordInput::new().with_hint_text("Enter password"), @@ -374,9 +381,21 @@ impl WalletsBalancesScreen { } fn open_rename_dialog(&mut self, alias: Option) { - self.show_rename_dialog = true; + self.rename_task = if let Some(wallet) = &self.selected_wallet { + Some(WalletTask::RenameHdWallet { + seed_hash: wallet.read_recover().seed_hash(), + alias: alias.unwrap_or_default(), + }) + } else { + self.selected_single_key_wallet.as_ref().map(|wallet| { + WalletTask::RenameSingleKeyWallet { + address: wallet.read_recover().address.to_string(), + alias: alias.unwrap_or_default(), + } + }) + }; + self.pending_rename_context = None; self.rename_dialog_opening_guard.arm(); - self.rename_input = alias.unwrap_or_default(); } fn persist_selected_single_key_hash(&self, hash: Option<[u8; 32]>) { @@ -692,6 +711,7 @@ impl WalletsBalancesScreen { // Clone wallet arcs before using to avoid borrow conflicts let hd_wallet_opt = self.selected_wallet.clone(); let single_key_wallet_opt = self.selected_single_key_wallet.clone(); + let rename_enabled = self.pending_rename_context.is_none(); // Buttons for HD wallet if let Some(wallet_arc) = hd_wallet_opt { @@ -721,7 +741,10 @@ impl WalletsBalancesScreen { self.lock_selected_wallet(); } ui.add_space(8.0); - if ui.button("Rename").clicked() { + if ui + .add_enabled(rename_enabled, egui::Button::new("Rename")) + .clicked() + { self.open_rename_dialog(alias); } } @@ -750,8 +773,10 @@ impl WalletsBalancesScreen { ui.add_space(8.0); - // Rename button - if ui.button("Rename").clicked() { + if ui + .add_enabled(rename_enabled, egui::Button::new("Rename")) + .clicked() + { self.open_rename_dialog(alias); } } @@ -938,8 +963,8 @@ impl WalletsBalancesScreen { self.set_selected_hd_wallet(next_wallet); - self.show_rename_dialog = false; - self.rename_input.clear(); + self.rename_task = None; + self.pending_rename_context = None; self.wallet_unlock_popup.close(); self.refreshing = false; @@ -2599,8 +2624,10 @@ impl ScreenLike for WalletsBalancesScreen { )); } - // Rename dialog - if self.show_rename_dialog { + if let Some(mut rename_task) = self.rename_task.take() { + let is_saving = self.pending_rename_context.is_some(); + let mut cancel = false; + let mut save = false; let window_response = egui::Window::new("Rename Wallet") .collapsible(false) .resizable(false) @@ -2611,89 +2638,68 @@ impl ScreenLike for WalletsBalancesScreen { ui.label("Enter new wallet name:"); ui.add_space(5.0); - let text_edit = egui::TextEdit::singleline(&mut self.rename_input) - .hint_text("Enter wallet name") - .desired_width(250.0); - ui.add(text_edit); + ui.add_enabled_ui(!is_saving, |ui| { + let alias = match &mut rename_task { + WalletTask::RenameHdWallet { alias, .. } + | WalletTask::RenameSingleKeyWallet { alias, .. } => alias, + _ => unreachable!("rename dialog stores only rename tasks"), + }; + let text_edit = egui::TextEdit::singleline(alias) + .hint_text("Enter wallet name") + .desired_width(250.0); + ui.add(text_edit); - ui.add_space(10.0); + ui.add_space(10.0); - ui.horizontal(|ui| { - if ComponentStyles::add_secondary_button(ui, "Cancel", dark_mode) + ui.horizontal(|ui| { + if ComponentStyles::add_secondary_button( + ui, "Cancel", dark_mode, + ) .clicked() - { - self.show_rename_dialog = false; - self.rename_input.clear(); - } - - ui.add_space(8.0); - - if ComponentStyles::add_primary_button(ui, "Save").clicked() { - if let Err(error) = validate_wallet_alias(&self.rename_input) { - MessageBanner::set_global( - ctx, - "The wallet name is too long. Use 64 characters or fewer and try again.", - MessageType::Error, - ) - .with_details(error); - return; + { + cancel = true; } - // Queue the rename for dispatch as a backend - // task; the sidecar write runs off the UI thread - // and the in-memory alias updates only from the - // task result. Persistence is identical across - // wallet kinds — only the identifier differs. - let new_alias = self.rename_input.clone(); - if let Some(selected_wallet) = &self.selected_wallet { - let seed_hash = selected_wallet.read_recover().seed_hash(); - self.pending_rename = Some(PendingWalletRename::Hd { - seed_hash, - alias: new_alias, - }); - } else if let Some(selected_sk_wallet) = - &self.selected_single_key_wallet - { - let address = - selected_sk_wallet.read_recover().address.to_string(); - self.pending_rename = Some(PendingWalletRename::SingleKey { - address, - alias: new_alias, - }); + ui.add_space(8.0); + + if ComponentStyles::add_primary_button(ui, "Save").clicked() { + if let Err(error) = validate_wallet_alias(alias) { + MessageBanner::set_global( + ctx, + "The wallet name is too long. Use 64 characters or fewer and try again.", + MessageType::Error, + ) + .with_details(error); + } else { + save = true; + } } - self.show_rename_dialog = false; - self.rename_input.clear(); - } + }); }); }); }); - if let Some(ref resp) = window_response - && clicked_outside_window_after_open( - ctx, - resp.response.rect, - &mut self.rename_dialog_opening_guard, - ) - { - self.show_rename_dialog = false; - self.rename_input.clear(); + let clicked_outside = !is_saving + && window_response.as_ref().is_some_and(|response| { + clicked_outside_window_after_open( + ctx, + response.response.rect, + &mut self.rename_dialog_opening_guard, + ) + }); + if cancel || clicked_outside { + self.pending_rename_context = None; + } else if save { + let task = BackendTask::WalletTask(rename_task.clone()); + let context = BackendTaskContext::for_dispatch(&task); + self.pending_rename_context = Some(context.clone()); + self.rename_task = Some(rename_task); + return AppAction::BackendTaskWithContext { task, context }; + } else { + self.rename_task = Some(rename_task); } } - // Drain a confirmed rename into a backend task that persists the alias - // off the UI thread. The in-memory label updates from the task result. - if let Some(pending) = self.pending_rename.take() { - let task = match pending { - PendingWalletRename::Hd { seed_hash, alias } => { - WalletTask::RenameHdWallet { seed_hash, alias } - } - PendingWalletRename::SingleKey { address, alias } => { - WalletTask::RenameSingleKeyWallet { address, alias } - } - }; - action |= AppAction::BackendTask(BackendTask::WalletTask(task)); - } - // HD Wallet unlock popup if let Some(wallet_arc) = &self.selected_wallet.clone() { let result = self @@ -2927,6 +2933,30 @@ impl ScreenLike for WalletsBalancesScreen { } } + fn display_backend_task_result( + &mut self, + context: &BackendTaskContext, + result: crate::ui::BackendTaskSuccessResult, + ) { + let rename_succeeded = self.pending_rename_context.as_ref() == Some(context) + && context + .wallet_rename_task() + .is_some_and(|task| rename_result_matches_task(task, &result)); + self.display_task_result(result); + if rename_succeeded { + self.rename_task = None; + self.pending_rename_context = None; + } + } + + fn display_backend_task_error(&mut self, context: &BackendTaskContext, _error: &TaskError) { + if self.pending_rename_context.as_ref() == Some(context) + && context.wallet_rename_task().is_some() + { + self.pending_rename_context = None; + } + } + fn display_task_result( &mut self, backend_task_success_result: crate::ui::BackendTaskSuccessResult, @@ -2975,22 +3005,26 @@ impl ScreenLike for WalletsBalancesScreen { self.asset_lock_cache.store(seed_hash, locks); } crate::ui::BackendTaskSuccessResult::WalletAliasRenamed { seed_hash, alias } => { - // Update the in-memory label only now that the sidecar write - // succeeded. Read the guard into a local first so the read lock - // is released before taking the write lock on the same wallet. - if let Some(wallet) = &self.selected_wallet { - let is_target = wallet.read_recover().seed_hash() == seed_hash; - if is_target { - wallet.write_recover().alias = Some(alias); - } + let wallet = self + .app_context + .wallets + .read_recover() + .get(&seed_hash) + .cloned(); + if let Some(wallet) = wallet { + wallet.write_recover().alias = Some(alias); } } crate::ui::BackendTaskSuccessResult::SingleKeyAliasRenamed { address, alias } => { - if let Some(wallet) = &self.selected_single_key_wallet { - let is_target = wallet.read_recover().address.to_string() == address; - if is_target { - wallet.write_recover().alias = Some(alias); - } + let wallet = self + .app_context + .single_key_wallets + .read_recover() + .values() + .find(|wallet| wallet.read_recover().address.to_string() == address) + .cloned(); + if let Some(wallet) = wallet { + wallet.write_recover().alias = Some(alias); } } crate::ui::BackendTaskSuccessResult::GeneratedReceiveAddress { seed_hash, address } => { diff --git a/src/wallet_backend/single_key.rs b/src/wallet_backend/single_key.rs index 926bc9b2f..1725cba79 100644 --- a/src/wallet_backend/single_key.rs +++ b/src/wallet_backend/single_key.rs @@ -272,19 +272,20 @@ impl<'a> SingleKeyView<'a> { /// name survives a cold boot without touching the legacy /// `single_key_wallet` table. An empty `alias` clears the nickname. /// - /// No-op success when the view has no sidecar wired (transient - /// construction path) — the in-memory index is still updated so the - /// rename is visible in-session. + /// The index write guard spans persistence so same-address renames serialize. + /// With a sidecar, the index changes only after a successful write. Without + /// one, the transient in-memory index is updated directly. pub fn set_alias(&self, address: &str, alias: Option) -> Result<(), TaskError> { if let Some(alias) = alias.as_deref() { crate::model::wallet::validate_wallet_alias(alias) .map_err(|source| TaskError::InvalidWalletAliasLength { source })?; } let mut idx = write_recover(self.index); - let entry = idx.get_mut(address).ok_or(TaskError::ImportedKeyNotFound)?; - entry.alias = alias; - let updated = entry.clone(); - drop(idx); + let mut updated = idx + .get(address) + .cloned() + .ok_or(TaskError::ImportedKeyNotFound)?; + updated.alias = alias; if let Some(kv) = self.app_kv { let key = meta_key_for(self.network, address); @@ -294,6 +295,7 @@ impl<'a> SingleKeyView<'a> { } })?; } + idx.insert(address.to_string(), updated); Ok(()) } @@ -1301,6 +1303,95 @@ mod tests { } } + #[derive(Default)] + struct AliasPutGateState { + armed: bool, + first_put_waiting: bool, + release_first_put: bool, + } + + #[derive(Default)] + struct FirstAliasPutGate { + inner: InMemoryKv, + state: std::sync::Mutex, + changed: std::sync::Condvar, + } + + impl FirstAliasPutGate { + fn arm(&self) { + let mut state = self.state.lock().expect("gate state"); + *state = AliasPutGateState { + armed: true, + ..Default::default() + }; + } + + fn wait_until_first_put(&self) { + let state = self.state.lock().expect("gate state"); + let (state, timeout) = self + .changed + .wait_timeout_while(state, std::time::Duration::from_secs(5), |state| { + !state.first_put_waiting + }) + .expect("gate wait"); + assert!( + !timeout.timed_out() && state.first_put_waiting, + "first alias put gate" + ); + } + + fn release_first_put(&self) { + let mut state = self.state.lock().expect("gate state"); + state.release_first_put = true; + self.changed.notify_all(); + } + } + + impl platform_wallet_storage::KvStore for FirstAliasPutGate { + fn get( + &self, + scope: &platform_wallet_storage::ObjectId, + key: &str, + ) -> Result>, platform_wallet_storage::KvError> { + self.inner.get(scope, key) + } + + fn put( + &self, + scope: &platform_wallet_storage::ObjectId, + key: &str, + value: &[u8], + ) -> Result<(), platform_wallet_storage::KvError> { + let mut state = self.state.lock().expect("gate state"); + if state.armed && !state.first_put_waiting && key.contains(SINGLE_KEY_META_INFIX) { + state.first_put_waiting = true; + self.changed.notify_all(); + state = self + .changed + .wait_while(state, |state| !state.release_first_put) + .expect("gate release"); + } + drop(state); + self.inner.put(scope, key, value) + } + + fn delete( + &self, + scope: &platform_wallet_storage::ObjectId, + key: &str, + ) -> Result<(), platform_wallet_storage::KvError> { + self.inner.delete(scope, key) + } + + fn list_keys( + &self, + scope: &platform_wallet_storage::ObjectId, + prefix: Option<&str>, + ) -> Result, platform_wallet_storage::KvError> { + self.inner.list_keys(scope, prefix) + } + } + /// Test fixture bundling the moving parts a [`SingleKeyView`] needs /// when wired against a fake `KvStore`. Returned as a struct to /// keep the constructor tuple-light (clippy `type_complexity`). @@ -1819,6 +1910,91 @@ mod tests { assert_eq!(view.list()[0].alias.as_deref(), Some("old name")); } + #[test] + fn overlapping_alias_updates_keep_index_and_sidecar_consistent() { + let dir = tempfile::tempdir().expect("tempdir"); + let store = + Arc::new(open_secret_store(&dir.path().join("secrets.pwsvault")).expect("open vault")); + let index = Arc::new(std::sync::RwLock::new(std::collections::BTreeMap::new())); + let gated_store = Arc::new(FirstAliasPutGate::default()); + let kv = Arc::new(DetKv::from_store(gated_store.clone())); + let view = SingleKeyView { + secret_store: &store, + index: &index, + network: Network::Testnet, + app_kv: Some(&kv), + }; + let address = view + .import_wif(known_wif(), Some("original".into())) + .expect("import") + .address; + gated_store.arm(); + + let first_store = store.clone(); + let first_index = index.clone(); + let first_kv = kv.clone(); + let first_address = address.clone(); + let first = std::thread::spawn(move || { + SingleKeyView { + secret_store: &first_store, + index: &first_index, + network: Network::Testnet, + app_kv: Some(&first_kv), + } + .set_alias(&first_address, Some("first".into())) + }); + gated_store.wait_until_first_put(); + + let later_store = store.clone(); + let later_index = index.clone(); + let later_kv = kv.clone(); + let later_address = address.clone(); + let (later_tx, later_rx) = std::sync::mpsc::channel(); + let later = std::thread::spawn(move || { + let result = SingleKeyView { + secret_store: &later_store, + index: &later_index, + network: Network::Testnet, + app_kv: Some(&later_kv), + } + .set_alias(&later_address, Some("later".into())); + later_tx.send(result).expect("send later result"); + }); + + let later_while_first_blocked = later_rx + .recv_timeout(std::time::Duration::from_secs(1)) + .ok(); + gated_store.release_first_put(); + first + .join() + .expect("first rename thread") + .expect("first rename"); + let later_result = match later_while_first_blocked { + Some(result) => result, + None => later_rx + .recv_timeout(std::time::Duration::from_secs(5)) + .expect("later rename completion"), + }; + later_result.expect("later rename"); + later.join().expect("later rename thread"); + + let indexed_alias = read_recover(&index) + .get(&address) + .and_then(|entry| entry.alias.as_deref()) + .map(str::to_owned); + let persisted_alias = kv + .get::(DetScope::Global, &meta_key_for(Network::Testnet, &address)) + .expect("read persisted alias") + .expect("persisted entry") + .alias; + assert_eq!(indexed_alias.as_deref(), Some("later")); + assert_eq!( + persisted_alias.as_deref(), + Some("later"), + "the persisted sidecar and in-memory index must agree on the later alias" + ); + } + /// Legacy 32-byte raw vault payloads (pre per-key-passphrase) /// still decode as `has_passphrase = false`, so a user who /// upgrades from a previous tag never loses their imported keys. diff --git a/tests/kittest/wallets_screen.rs b/tests/kittest/wallets_screen.rs index 380ede5b2..c1853390f 100644 --- a/tests/kittest/wallets_screen.rs +++ b/tests/kittest/wallets_screen.rs @@ -1,24 +1,30 @@ use crate::support::{fresh_app_context, with_isolated_data_dir}; #[cfg(feature = "testing")] use dash_evo_tool::app::AppAction; -#[cfg(feature = "testing")] -use dash_evo_tool::backend_task::BackendTask; use dash_evo_tool::backend_task::BackendTaskSuccessResult; #[cfg(feature = "testing")] +use dash_evo_tool::backend_task::error::TaskError; +#[cfg(feature = "testing")] use dash_evo_tool::backend_task::wallet::WalletTask; +#[cfg(feature = "testing")] +use dash_evo_tool::backend_task::{BackendTask, BackendTaskContext}; use dash_evo_tool::model::secret::Secret; use dash_evo_tool::model::wallet::Wallet; use dash_evo_tool::model::wallet::birth_height::WalletOrigin; -use dash_evo_tool::ui::ScreenLike; +#[cfg(feature = "testing")] +use dash_evo_tool::ui::components::MessageBanner; use dash_evo_tool::ui::wallets::wallets_screen::WalletsBalancesScreen; +use dash_evo_tool::ui::{MessageType, ScreenLike}; #[cfg(feature = "testing")] use dash_sdk::dashcore_rpc::dashcore::Network; #[cfg(feature = "testing")] use dash_sdk::dpp::address_funds::PlatformAddress; +#[cfg(feature = "testing")] +use dash_sdk::dpp::dashcore::PrivateKey; use egui_kittest::Harness; -use egui_kittest::kittest::Queryable; +use egui_kittest::kittest::{NodeT, Queryable}; #[cfg(feature = "testing")] -use std::cell::Cell; +use std::cell::{Cell, RefCell}; #[cfg(feature = "testing")] use std::rc::Rc; use std::sync::{Arc, RwLock}; @@ -172,6 +178,495 @@ fn platform_addresses(count: u8, network: Network) -> Vec<(String, u64)> { .collect() } +#[cfg(feature = "testing")] +#[derive(Debug, Clone)] +struct CapturedRename { + task: WalletTask, + context: BackendTaskContext, +} + +#[cfg(feature = "testing")] +fn capture_rename_action(action: AppAction) -> Option { + let (task, context) = match action { + AppAction::BackendTask(BackendTask::WalletTask(task)) => { + let backend_task = BackendTask::WalletTask(task.clone()); + (task, BackendTaskContext::from(&backend_task)) + } + AppAction::BackendTaskWithContext { + task: BackendTask::WalletTask(task), + context, + } => (task, context), + _ => return None, + }; + matches!( + task, + WalletTask::RenameHdWallet { .. } | WalletTask::RenameSingleKeyWallet { .. } + ) + .then_some(CapturedRename { task, context }) +} + +#[cfg(feature = "testing")] +fn build_rename_harness( + runtime: tokio::runtime::Runtime, + app_context: Arc, +) -> ( + Harness<'static, WalletsBalancesScreen>, + Rc>>, +) { + let dispatched = Rc::new(RefCell::new(Vec::new())); + let captured = dispatched.clone(); + let screen = WalletsBalancesScreen::new(&app_context); + let mut harness = Harness::builder() + .with_size(egui::vec2(1280.0, 800.0)) + .build_ui_state( + move |ui, screen: &mut WalletsBalancesScreen| { + let _runtime = &runtime; + if let Some(rename) = capture_rename_action(screen.ui(ui)) { + captured.borrow_mut().push(rename); + } + }, + screen, + ); + harness.run(); + (harness, dispatched) +} + +#[cfg(feature = "testing")] +fn enter_rename_alias(harness: &mut Harness<'_, WalletsBalancesScreen>, alias: &str) { + click_in_one_frame(harness, "Rename"); + let input = harness + .query_all_by_role(egui::accesskit::Role::TextInput) + .next() + .expect("rename input"); + input.focus(); + harness.event(egui::Event::Text(alias.to_string())); + harness.step(); +} + +#[cfg(feature = "testing")] +fn take_rename_dispatch(dispatched: &Rc>>) -> CapturedRename { + let mut dispatched = dispatched.borrow_mut(); + assert_eq!(dispatched.len(), 1, "exactly one rename must dispatch"); + dispatched.pop().expect("rename dispatch") +} + +#[cfg(feature = "testing")] +fn random_wif(network: Network) -> String { + loop { + let mut key_bytes: [u8; 32] = rand::random(); + if let Ok(private_key) = PrivateKey::from_byte_array(&key_bytes, network) { + key_bytes.zeroize(); + return private_key.to_wif(); + } + key_bytes.zeroize(); + } +} + +#[test] +#[cfg(feature = "testing")] +fn hd_rename_dispatches_exact_task_and_applies_success() { + with_isolated_data_dir(|| { + let (runtime, app_context) = fresh_app_context(); + let mut seed: [u8; 64] = rand::random(); + let wallet = + Wallet::new_from_seed(seed, app_context.network(), None, None).expect("wallet"); + seed.zeroize(); + let seed_hash = wallet.seed_hash(); + let wallet = Arc::new(RwLock::new(wallet)); + app_context + .wallets() + .write() + .expect("wallet map") + .insert(seed_hash, wallet.clone()); + app_context.set_selected_hd_wallet(Some(seed_hash)); + + let (mut harness, dispatched) = build_rename_harness(runtime, app_context); + enter_rename_alias(&mut harness, "Renamed HD"); + harness.get_by_label("Save").click(); + harness.run(); + + let dispatch = take_rename_dispatch(&dispatched); + assert_eq!( + dispatch.task, + WalletTask::RenameHdWallet { + seed_hash, + alias: "Renamed HD".into(), + } + ); + assert!( + harness.query_all_by_value("Renamed HD").next().is_some(), + "the attempted alias must remain visible while saving" + ); + assert!( + harness + .query_all_by_role(egui::accesskit::Role::TextInput) + .next() + .expect("rename input") + .accesskit_node() + .is_disabled(), + "the alias input must be disabled while saving" + ); + assert!( + harness.get_by_label("Save").accesskit_node().is_disabled() + && harness + .get_by_label("Cancel") + .accesskit_node() + .is_disabled(), + "Save and Cancel must be disabled while saving" + ); + let outside = egui::pos2(0.0, 0.0); + harness.input_mut().events.extend([ + egui::Event::PointerMoved(outside), + egui::Event::PointerButton { + pos: outside, + button: egui::PointerButton::Primary, + pressed: true, + modifiers: egui::Modifiers::default(), + }, + egui::Event::PointerButton { + pos: outside, + button: egui::PointerButton::Primary, + pressed: false, + modifiers: egui::Modifiers::default(), + }, + ]); + harness.step(); + assert!( + harness.query_by_label("Enter new wallet name:").is_some(), + "outside clicks must not dismiss a rename while it is saving" + ); + + harness.state_mut().display_backend_task_result( + &dispatch.context, + BackendTaskSuccessResult::WalletAliasRenamed { + seed_hash, + alias: "Renamed HD".into(), + }, + ); + harness.run(); + + assert_eq!( + wallet.read().expect("wallet").alias.as_deref(), + Some("Renamed HD") + ); + assert!( + harness.query_by_label("Enter new wallet name:").is_none(), + "success must close the rename dialog" + ); + }); +} + +#[test] +#[cfg(feature = "testing")] +fn rename_button_is_disabled_and_inert_while_save_is_pending() { + with_isolated_data_dir(|| { + let (runtime, app_context) = fresh_app_context(); + let mut seed: [u8; 64] = rand::random(); + let wallet = + Wallet::new_from_seed(seed, app_context.network(), None, None).expect("wallet"); + seed.zeroize(); + let seed_hash = wallet.seed_hash(); + app_context + .wallets() + .write() + .expect("wallet map") + .insert(seed_hash, Arc::new(RwLock::new(wallet))); + app_context.set_selected_hd_wallet(Some(seed_hash)); + + let (mut harness, dispatched) = build_rename_harness(runtime, app_context); + enter_rename_alias(&mut harness, "Pending rename"); + harness.get_by_label("Save").click(); + harness.run(); + + assert_eq!(dispatched.borrow().len(), 1, "the rename must dispatch"); + assert!( + harness + .get_by_label("Rename") + .accesskit_node() + .is_disabled(), + "the Rename entry point must be disabled while saving" + ); + + click_in_one_frame(&mut harness, "Rename"); + assert!( + harness + .query_all_by_value("Pending rename") + .next() + .is_some(), + "clicking the disabled entry point must preserve the pending alias" + ); + assert!( + harness.get_by_label("Save").accesskit_node().is_disabled(), + "clicking the disabled entry point must not reset the saving state" + ); + }); +} + +#[test] +#[cfg(feature = "testing")] +fn single_key_rename_dispatches_exact_task_and_applies_success() { + with_isolated_data_dir(|| { + let (runtime, app_context) = fresh_app_context(); + let (_imported, wallet) = app_context + .import_single_key_wif(&random_wif(app_context.network()), None, Default::default()) + .expect("import key"); + let (key_hash, address) = { + let wallet = wallet.read().expect("wallet"); + (wallet.key_hash, wallet.address.to_string()) + }; + app_context.set_selected_single_key_wallet(Some(key_hash)); + + let (mut harness, dispatched) = build_rename_harness(runtime, app_context); + enter_rename_alias(&mut harness, "Renamed key"); + harness.get_by_label("Save").click(); + harness.run(); + + let dispatch = take_rename_dispatch(&dispatched); + assert_eq!( + dispatch.task, + WalletTask::RenameSingleKeyWallet { + address: address.clone(), + alias: "Renamed key".into(), + } + ); + assert!( + harness.query_all_by_value("Renamed key").next().is_some(), + "the attempted alias must remain visible while saving" + ); + + harness.state_mut().display_backend_task_result( + &dispatch.context, + BackendTaskSuccessResult::SingleKeyAliasRenamed { + address, + alias: "Renamed key".into(), + }, + ); + harness.run(); + + assert_eq!( + wallet.read().expect("wallet").alias.as_deref(), + Some("Renamed key") + ); + assert!( + harness.query_by_label("Enter new wallet name:").is_none(), + "success must close the rename dialog" + ); + }); +} + +#[test] +#[cfg(feature = "testing")] +fn hd_rename_success_updates_original_wallet_after_selection_change() { + with_isolated_data_dir(|| { + let (runtime, app_context) = fresh_app_context(); + let mut target_seed: [u8; 64] = rand::random(); + let target = Wallet::new_from_seed( + target_seed, + app_context.network(), + Some("Target".into()), + None, + ) + .expect("target wallet"); + target_seed.zeroize(); + let target_hash = target.seed_hash(); + let target = Arc::new(RwLock::new(target)); + + let mut other_seed: [u8; 64] = rand::random(); + let other = Wallet::new_from_seed( + other_seed, + app_context.network(), + Some("Other".into()), + None, + ) + .expect("other wallet"); + other_seed.zeroize(); + let other_hash = other.seed_hash(); + let other = Arc::new(RwLock::new(other)); + + { + let mut wallets = app_context.wallets().write().expect("wallet map"); + wallets.insert(target_hash, target.clone()); + wallets.insert(other_hash, other); + } + app_context.set_selected_hd_wallet(Some(target_hash)); + + let (mut harness, dispatched) = build_rename_harness(runtime, app_context); + click_in_one_frame(&mut harness, "Rename"); + let input = harness + .query_all_by_role(egui::accesskit::Role::TextInput) + .next() + .expect("rename input"); + input.focus(); + harness.event(egui::Event::Key { + key: egui::Key::A, + physical_key: None, + pressed: true, + repeat: false, + modifiers: egui::Modifiers::COMMAND, + }); + harness.event(egui::Event::Text("Target renamed".into())); + harness.step(); + harness.get_by_label("Save").click(); + harness.run(); + let dispatch = take_rename_dispatch(&dispatched); + + harness.get_by_value("HD: Target").click(); + harness.run(); + harness.get_by_label("HD: Other (0.0000 DASH)").click(); + harness.run(); + + harness.state_mut().display_backend_task_result( + &dispatch.context, + BackendTaskSuccessResult::WalletAliasRenamed { + seed_hash: target_hash, + alias: "Target renamed".into(), + }, + ); + + assert_eq!( + target.read().expect("target").alias.as_deref(), + Some("Target renamed"), + "the canonical target wallet must update independently of selection" + ); + }); +} + +#[test] +#[cfg(feature = "testing")] +fn single_key_rename_success_updates_original_wallet_after_selection_change() { + with_isolated_data_dir(|| { + let (runtime, app_context) = fresh_app_context(); + let (_target_imported, target) = app_context + .import_single_key_wif( + &random_wif(app_context.network()), + Some("Target key".into()), + Default::default(), + ) + .expect("target key"); + let (_other_imported, other) = app_context + .import_single_key_wif( + &random_wif(app_context.network()), + Some("Other key".into()), + Default::default(), + ) + .expect("other key"); + let (target_hash, target_address) = { + let target = target.read().expect("target"); + (target.key_hash, target.address.to_string()) + }; + app_context.set_selected_single_key_wallet(Some(target_hash)); + + let (mut harness, dispatched) = build_rename_harness(runtime, app_context); + click_in_one_frame(&mut harness, "Rename"); + let input = harness + .query_all_by_role(egui::accesskit::Role::TextInput) + .next() + .expect("rename input"); + input.focus(); + harness.event(egui::Event::Key { + key: egui::Key::A, + physical_key: None, + pressed: true, + repeat: false, + modifiers: egui::Modifiers::COMMAND, + }); + harness.event(egui::Event::Text("Target key renamed".into())); + harness.step(); + harness.get_by_label("Save").click(); + harness.run(); + let dispatch = take_rename_dispatch(&dispatched); + + harness.get_by_value("SK: Target key").click(); + harness.run(); + harness.get_by_label("SK: Other key (0.0000 DASH)").click(); + harness.run(); + + harness.state_mut().display_backend_task_result( + &dispatch.context, + BackendTaskSuccessResult::SingleKeyAliasRenamed { + address: target_address, + alias: "Target key renamed".into(), + }, + ); + + assert_eq!( + target.read().expect("target").alias.as_deref(), + Some("Target key renamed"), + "the canonical target key must update independently of selection" + ); + assert_eq!( + other.read().expect("other").alias.as_deref(), + Some("Other key") + ); + }); +} + +#[test] +#[cfg(feature = "testing")] +fn failed_hd_rename_keeps_prefilled_dialog_open_and_shows_banner() { + with_isolated_data_dir(|| { + let (runtime, app_context) = fresh_app_context(); + let mut seed: [u8; 64] = rand::random(); + let wallet = + Wallet::new_from_seed(seed, app_context.network(), None, None).expect("wallet"); + seed.zeroize(); + let seed_hash = wallet.seed_hash(); + app_context + .wallets() + .write() + .expect("wallet map") + .insert(seed_hash, Arc::new(RwLock::new(wallet))); + app_context.set_selected_hd_wallet(Some(seed_hash)); + + let (mut harness, dispatched) = build_rename_harness(runtime, app_context); + enter_rename_alias(&mut harness, "Retry this alias"); + harness.get_by_label("Save").click(); + harness.run(); + let dispatch = take_rename_dispatch(&dispatched); + + let error = TaskError::WalletNotFound; + harness + .state_mut() + .display_backend_task_error(&dispatch.context, &error); + let message = error.to_string(); + MessageBanner::set_global(&harness.ctx, &message, MessageType::Error) + .disable_auto_dismiss(); + harness + .state_mut() + .display_message(&message, MessageType::Error); + harness.run(); + + assert!( + harness.query_by_label(&message).is_some(), + "the actionable typed error must appear in the global banner" + ); + assert!( + harness + .query_all_by_value("Retry this alias") + .next() + .is_some(), + "the attempted alias must remain available for retry" + ); + assert!( + !harness.get_by_label("Save").accesskit_node().is_disabled(), + "Save must be re-enabled after a matching failure" + ); + assert!( + !harness + .get_by_label("Cancel") + .accesskit_node() + .is_disabled() + && !harness + .query_all_by_role(egui::accesskit::Role::TextInput) + .next() + .expect("rename input") + .accesskit_node() + .is_disabled(), + "Cancel and alias editing must be re-enabled after failure" + ); + }); +} + #[test] #[cfg(feature = "testing")] fn fund_platform_dialog_last_popup_row_stays_open_until_fund_is_clicked() {