fix(ui-spv): address PR #837 review feedback

src/backend_task/error.rs

@@ -573,8 +573,12 @@ pub enum TaskError {
     },
 
     /// The requested operation requires Dash Core (RPC) and cannot run in light-wallet (SPV) mode.
+    ///
+    /// The `operation` field is kept for debug/log context only — the user-facing
+    /// `Display` message is a static, complete sentence so it remains translatable
+    /// and cannot pick up grammar bugs from the callsite-provided fragment.
     #[error(
-        "{operation} is only available when connected to Dash Core. Switch to Dash Core in Settings and retry."
+        "This action is only available when connected to Dash Core. Switch to Dash Core in Settings and retry."
     )]
     OperationRequiresDashCore { operation: &'static str },
 

src/backend_task/shielded/bundle.rs

@@ -499,11 +499,21 @@ pub async fn shield_from_asset_lock(
         .broadcast_raw_transaction(&asset_lock_transaction)
         .await
     {
-        match app_context.transactions_waiting_for_finality.lock() {
+        // Use `try_lock` on the error return path so a contended mutex never
+        // blocks us here. Matches the timeout-path cleanup below and the
+        // existing pattern in `src/context/transaction_processing.rs`.
+        match app_context.transactions_waiting_for_finality.try_lock() {
             Ok(mut proofs) => {
                 proofs.remove(&tx_id);
             }
-            Err(poisoned) => {
+            Err(std::sync::TryLockError::WouldBlock) => {
+                tracing::debug!(
+                    %tx_id,
+                    "transactions_waiting_for_finality lock is contended after broadcast failure; \
+                     finality tracking entry not cleared"
+                );
+            }
+            Err(std::sync::TryLockError::Poisoned(poisoned)) => {
                 tracing::warn!(
                     %tx_id,
                     "transactions_waiting_for_finality lock is poisoned after broadcast failure; \
@@ -553,26 +563,26 @@ pub async fn shield_from_asset_lock(
     loop {
         tokio::select! {
             _ = &mut timeout => {
-                match app_context.transactions_waiting_for_finality.try_lock() {
-                    Ok(mut proofs) => {
-                        proofs.remove(&tx_id);
-                    }
-                    Err(std::sync::TryLockError::WouldBlock) => {
-                        tracing::debug!(
-                            %tx_id,
-                            "transactions_waiting_for_finality lock is contended on timeout; \
-                             finality tracking entry not cleared"
-                        );
-                    }
-                    Err(std::sync::TryLockError::Poisoned(poisoned)) => {
-                        tracing::warn!(
-                            %tx_id,
-                            "transactions_waiting_for_finality lock is poisoned on timeout; \
-                             recovering through poisoned guard so the entry is cleared"
-                        );
-                        let mut proofs = poisoned.into_inner();
-                        proofs.remove(&tx_id);
-                    }
+                // Guarantee the finality-tracking entry is removed before we
+                // return `ShieldedAssetLockTimeout`. Use a blocking `lock()`
+                // with poison recovery (not `try_lock`) so a contended mutex
+                // can't cause the entry to leak across retries.
+                {
+                    let mut proofs = match app_context
+                        .transactions_waiting_for_finality
+                        .lock()
+                    {
+                        Ok(guard) => guard,
+                        Err(poisoned) => {
+                            tracing::warn!(
+                                %tx_id,
+                                "transactions_waiting_for_finality lock is poisoned on timeout; \
+                                 recovering through poisoned guard so the entry is cleared"
+                            );
+                            poisoned.into_inner()
+                        }
+                    };
+                    proofs.remove(&tx_id);
                 }
 
                 if app_context.core_backend_mode() == crate::spv::CoreBackendMode::Rpc

src/ui/wallets/mod.rs

@@ -10,3 +10,35 @@ pub mod shielded_tab;
 pub mod single_key_send_screen;
 pub mod unshield_credits_screen;
 pub mod wallets_screen;
+
+use crate::ui::MessageType;
+use crate::ui::components::MessageBanner;
+use crate::ui::components::component_trait::Component;
+use eframe::egui::Ui;
+
+/// Shared user-facing copy shown in every surface (tooltips, inline banners)
+/// where a single-key wallet action that needs Core (sending, refreshing
+/// balances) is unavailable because the app is running on the built-in SPV
+/// backend. Centralised so the wording stays consistent and a single string
+/// needs updating when translations land.
+///
+/// Intentionally action-specific: receiving and viewing an already-loaded
+/// balance/UTXO list still work in SPV mode, so the copy must not imply the
+/// whole wallet is unusable.
+pub const SINGLE_KEY_REQUIRES_CORE_MESSAGE: &str = "Sending and refreshing balances for single-key wallets require a local Dash Core node. Open Settings, switch to Expert mode, and select Local Dash Core node to enable these actions. Receiving still works in SPV mode.";
+
+/// Renders the persistent "single-key wallets require Dash Core" notice as a
+/// [`MessageBanner`] anchored to the current surface. Unlike the global
+/// banner, this is not dismissed automatically — the underlying app state
+/// (SPV backend mode) is what drives its visibility.
+///
+/// Constructed fresh each frame on purpose: this is a persistent state notice
+/// (bound to the SPV backend mode), not a transient task result, so we want
+/// it visible the whole time the mode is active. A fresh instance every frame
+/// means the auto-dismiss timer never fires and the banner is shown
+/// consistently; rendering is cheap and egui handles the repainting.
+pub fn render_single_key_requires_core_banner(ui: &mut Ui) {
+    let mut banner = MessageBanner::new();
+    banner.set_message(SINGLE_KEY_REQUIRES_CORE_MESSAGE, MessageType::Warning);
+    banner.show(ui);
+}

src/ui/wallets/single_key_send_screen.rs

@@ -8,13 +8,14 @@ use crate::model::amount::{Amount, DASH_DECIMAL_PLACES};
 use crate::model::wallet::single_key::SingleKeyWallet;
 use crate::spv::CoreBackendMode;
 use crate::ui::components::MessageBanner;
-use crate::ui::components::component_trait::Component;
 use crate::ui::components::left_panel::add_left_panel;
 use crate::ui::components::password_input::PasswordInput;
 use crate::ui::components::styled::island_central_panel;
 use crate::ui::components::top_panel::add_top_panel;
 use crate::ui::theme::{ComponentStyles, DashColors};
-use crate::ui::wallets::wallets_screen::single_key_view::SINGLE_KEY_REQUIRES_CORE;
+use crate::ui::wallets::{
+    SINGLE_KEY_REQUIRES_CORE_MESSAGE, render_single_key_requires_core_banner,
+};
 use crate::ui::{MessageType, RootScreenType, ScreenLike};
 use dash_sdk::dpp::key_wallet::wallet::managed_wallet_info::fee::FeeRate;
 use eframe::egui::{self, Context, RichText, Ui};
@@ -834,7 +835,7 @@ impl SingleKeyWalletSendScreen {
 
             let mut response = ui.add_enabled(button_enabled, send_button);
             if !is_rpc_mode {
-                response = response.on_disabled_hover_text(SINGLE_KEY_REQUIRES_CORE);
+                response = response.on_disabled_hover_text(SINGLE_KEY_REQUIRES_CORE_MESSAGE);
             }
             if response.clicked() {
                 match self.validate_and_send() {
@@ -878,13 +879,11 @@ impl ScreenLike for SingleKeyWalletSendScreen {
             egui::ScrollArea::vertical()
                 .auto_shrink([true; 2])
                 .show(ui, |ui| {
-                    // Persistent warning banner for the SPV backend. Constructed
-                    // fresh each frame on purpose (see `single_key_view.rs` for
-                    // the rationale): it is a state notice, not a task result.
+                    // Persistent warning banner for the SPV backend. See
+                    // `ui::wallets::render_single_key_requires_core_banner`
+                    // for the rationale on constructing it fresh each frame.
                     if !is_rpc_mode {
-                        let mut banner = MessageBanner::new();
-                        banner.set_message(SINGLE_KEY_REQUIRES_CORE, MessageType::Warning);
-                        banner.show(ui);
+                        render_single_key_requires_core_banner(ui);
                         ui.add_space(10.0);
                     }
 

src/ui/wallets/wallets_screen/mod.rs

@@ -1,7 +1,7 @@
 mod address_table;
 mod asset_locks;
 mod dialogs;
-pub(crate) mod single_key_view;
+mod single_key_view;
 
 use crate::app::{AppAction, BackendTasksExecutionMode, DesiredAppAction};
 use crate::backend_task::BackendTask;

src/ui/wallets/wallets_screen/single_key_view.rs

@@ -1,19 +1,15 @@
 use crate::app::AppAction;
 use crate::spv::CoreBackendMode;
-use crate::ui::components::MessageBanner;
-use crate::ui::components::component_trait::Component;
+use crate::ui::ScreenType;
 use crate::ui::theme::DashColors;
-use crate::ui::{MessageType, ScreenType};
+use crate::ui::wallets::{
+    SINGLE_KEY_REQUIRES_CORE_MESSAGE, render_single_key_requires_core_banner,
+};
 use eframe::egui;
 use egui::{Frame, Margin, RichText, Ui};
 
 use super::WalletsBalancesScreen;
 
-/// Shown as a disabled-button tooltip and in the in-screen warning banner for
-/// any single-key-wallet action that depends on Dash Core RPC. Exported so the
-/// dedicated send screen can reuse the same copy.
-pub(crate) const SINGLE_KEY_REQUIRES_CORE: &str = "Single-key wallets do not yet support SPV. Open Settings, switch to Expert mode, and select Local Dash Core node to use this wallet.";
-
 impl WalletsBalancesScreen {
     /// Render the detail view for a selected single key wallet
     pub(super) fn render_single_key_wallet_view(
@@ -60,19 +56,8 @@ impl WalletsBalancesScreen {
                     // are unavailable. The actions themselves are greyed out
                     // below; this banner is the "why" users otherwise wouldn't
                     // see from a silent disable.
-                    //
-                    // We construct the `MessageBanner` as a fresh local each
-                    // frame on purpose: this is a persistent state notice
-                    // (bound to the SPV backend mode), not a transient task
-                    // result, so we want it visible the whole time the mode
-                    // is active. A fresh instance every frame means the
-                    // auto-dismiss timer never fires and the banner is shown
-                    // consistently; rendering is cheap and egui handles the
-                    // repainting.
                     if !is_rpc_mode {
-                        let mut banner = MessageBanner::new();
-                        banner.set_message(SINGLE_KEY_REQUIRES_CORE, MessageType::Warning);
-                        banner.show(ui);
+                        render_single_key_requires_core_banner(ui);
                         ui.add_space(10.0);
                     }
 
@@ -92,7 +77,7 @@ impl WalletsBalancesScreen {
                         let send_response = if is_rpc_mode {
                             send_response
                         } else {
-                            send_response.on_disabled_hover_text(SINGLE_KEY_REQUIRES_CORE)
+                            send_response.on_disabled_hover_text(SINGLE_KEY_REQUIRES_CORE_MESSAGE)
                         };
                         if send_response.clicked() {
                             action = AppAction::AddScreen(

tests/backend-e2e/core_tasks.rs

@@ -68,10 +68,10 @@ async fn test_tc002_refresh_wallet_info_core_and_platform() {
 
 // TC-003: RefreshSingleKeyWalletInfo
 //
-// Single-key wallets require Dash Core (RPC) for UTXO discovery — SPV tracks
-// HD wallet-derived addresses only. The backend now returns a typed
-// `OperationRequiresDashCore` error in SPV mode; the test asserts that
-// mode-specific outcome rather than an unconditional success.
+// The backend-e2e harness always runs in SPV mode (see tests/backend-e2e/README.md).
+// Single-key wallets depend on Dash Core for UTXO discovery — SPV only tracks
+// HD wallet-derived addresses — so the backend returns a typed
+// `OperationRequiresDashCore` error here. This test asserts that outcome.
 #[ignore]
 #[tokio_shared_rt::test(shared, flavor = "multi_thread", worker_threads = 12)]
 async fn test_tc003_refresh_single_key_wallet_info() {
@@ -95,35 +95,18 @@ async fn test_tc003_refresh_single_key_wallet_info() {
 
     let skw_arc = Arc::new(RwLock::new(skw));
 
-    let task = BackendTask::CoreTask(CoreTask::RefreshSingleKeyWalletInfo(skw_arc.clone()));
-    let result = run_task(app_context, task).await;
-
-    // Single-key wallets require Dash Core for UTXO discovery. In SPV mode
-    // the backend returns a typed `OperationRequiresDashCore` error; in RPC
-    // mode the refresh should succeed.
-    match app_context.core_backend_mode() {
-        dash_evo_tool::spv::CoreBackendMode::Spv => {
-            let err = result
-                .expect_err("RefreshSingleKeyWalletInfo must fail in SPV mode with a typed error");
-            assert!(
-                matches!(
-                    err,
-                    dash_evo_tool::backend_task::error::TaskError::OperationRequiresDashCore { .. }
-                ),
-                "Expected OperationRequiresDashCore in SPV mode, got: {:?}",
-                err
-            );
-        }
-        dash_evo_tool::spv::CoreBackendMode::Rpc => {
-            let result = result.expect("RefreshSingleKeyWalletInfo should succeed in RPC mode");
-            match result {
-                BackendTaskSuccessResult::RefreshedWallet { .. } => {}
-                other => panic!("Expected RefreshedWallet, got: {:?}", other),
-            }
-            // Balance may be 0 for a fresh key — just verify the read succeeds
-            let _balance = skw_arc.read().expect("skw lock").total_balance_duffs();
-        }
-    }
+    let task = BackendTask::CoreTask(CoreTask::RefreshSingleKeyWalletInfo(skw_arc));
+    let err = run_task(app_context, task)
+        .await
+        .expect_err("RefreshSingleKeyWalletInfo must fail in SPV mode with a typed error");
+    assert!(
+        matches!(
+            err,
+            dash_evo_tool::backend_task::error::TaskError::OperationRequiresDashCore { .. }
+        ),
+        "Expected OperationRequiresDashCore in SPV mode, got: {:?}",
+        err
+    );
 }
 
 // TC-004: CreateRegistrationAssetLock