From e409c292dc7a6e92703243ec42eb707f71b93797 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 17:33:48 +0100 Subject: [PATCH 1/6] fix(spv): transition to error status when sync manager fails The SpvManager's progress watcher and sync event handler did not detect fatal sync errors, leaving the UI stuck in "Syncing" status indefinitely. Two detection paths are now in place: 1. spawn_sync_event_handler handles SyncEvent::ManagerError to transition SpvStatus to Error and store the error message. This is the primary path since dash-spv always emits this event on manager failure. 2. spawn_progress_watcher checks SyncState::Error from the progress channel as defense-in-depth (currently blocked by upstream bug dashpay/rust-dashcore#469 where try_emit_progress is not called on error paths). Once SpvStatus::Error is set, the existing ConnectionStatus logic maps it to OverallConnectionState::Disconnected, showing a red indicator icon with "SPV: Error" in the tooltip. Related upstream issues: - dashpay/rust-dashcore#469 (missing progress emit on error) - dashpay/rust-dashcore#470 (QRInfo chain lock retry robustness) Co-Authored-By: Claude Opus 4.6 --- .../manual-test-scenarios.md | 83 +++++++++++++++++++ src/spv/manager.rs | 25 ++++++ 2 files changed, 108 insertions(+) create mode 100644 docs/ai-design/2026-02-24-spv-sync-error-status/manual-test-scenarios.md diff --git a/docs/ai-design/2026-02-24-spv-sync-error-status/manual-test-scenarios.md b/docs/ai-design/2026-02-24-spv-sync-error-status/manual-test-scenarios.md new file mode 100644 index 000000000..2630068a5 --- /dev/null +++ b/docs/ai-design/2026-02-24-spv-sync-error-status/manual-test-scenarios.md @@ -0,0 +1,83 @@ +# Manual Test Scenarios: SPV Sync Error Status + +## Context + +When SPV sync encounters a fatal error (e.g., masternode sync failure), the app +should transition from "Syncing" to an error state, with the connectivity icon +turning red and the tooltip/status panel reflecting the failure. + +## Prerequisites + +- Dash Evo Tool built with the fix applied +- Access to Testnet (or a network where SPV sync can be triggered) +- SPV backend mode enabled (not RPC mode) + +## Scenario 1: Verify error state on sync failure + +**Goal:** Confirm the connectivity icon transitions to red (Disconnected) when +SPV sync fails. + +### Steps + +1. Launch Dash Evo Tool and connect to Testnet in SPV mode. +2. Observe the top-left connectivity icon during sync — it should pulse orange + (Syncing state). +3. If sync completes successfully, the icon should turn green (Running state). +4. If sync fails (e.g., masternode QRInfo failure visible in logs), observe: + - The connectivity icon turns **red** (static, no pulsation). + - Hovering over the icon shows tooltip: **"Disconnected"** with + **"SPV: Error"** detail. +5. Open the Network Chooser screen and check the SPV status detail — it should + display the error message (e.g., "Sync manager Masternode failed: ..."). + +### Expected Result + +- Icon transitions from orange (Syncing) to red (Disconnected) on error. +- Tooltip shows "Disconnected / SPV: Error". +- Error message is visible in the status detail panel. + +## Scenario 2: Verify normal sync still works + +**Goal:** Confirm the fix doesn't break the happy path. + +### Steps + +1. Launch Dash Evo Tool and connect to Testnet in SPV mode. +2. Wait for sync to complete (may take several minutes on first sync). +3. Observe the connectivity icon transitions: + - Orange (Syncing) during sync. + - Green (Running) after sync completes. +4. Hover over the icon — tooltip should show "Ready" with "SPV: Running". + +### Expected Result + +- Sync completes normally, icon turns green. +- No false error transitions during normal sync. + +## Scenario 3: Verify error message content + +**Goal:** Confirm the error message stored in `last_error` contains useful +diagnostic information. + +### Steps + +1. Trigger an SPV sync that fails (e.g., by connecting to a network with + known chain lock propagation issues). +2. Check application logs for the error: + - Look for `SPV manager ... reported error: ...` log line. +3. On the Network Chooser screen, verify the status detail shows the same + error message (not a generic "Sync failed" without context). + +### Expected Result + +- Log contains `SPV manager "Masternode" reported error: Masternode sync failed: ...`. +- UI status detail shows the specific error from the sync manager, including + the block hash reference. + +## Notes + +- The actual QRInfo chain lock error is an upstream issue + (dashpay/rust-dashcore#470). This fix ensures the app **reports** the error + correctly rather than silently staying stuck in "Syncing". +- A separate upstream issue (dashpay/rust-dashcore#469) tracks the missing + `try_emit_progress()` call on error paths in dash-spv. diff --git a/src/spv/manager.rs b/src/spv/manager.rs index a2e9ddcd3..27357ef30 100644 --- a/src/spv/manager.rs +++ b/src/spv/manager.rs @@ -9,6 +9,7 @@ use dash_sdk::dash_spv::network::PeerNetworkManager; use dash_sdk::dash_spv::storage::DiskStorageManager; use dash_sdk::dash_spv::sync::SyncEvent; use dash_sdk::dash_spv::sync::SyncProgress as SpvSyncProgress; +use dash_sdk::dash_spv::sync::SyncState; use dash_sdk::dash_spv::types::ValidationMode; use dash_sdk::dash_spv::{ClientConfig, DashSpvClient, Hash, LLMQType, QuorumHash}; use dash_sdk::dpp::dashcore::{Address, InstantLock, Network, Transaction, Txid}; @@ -1049,6 +1050,7 @@ impl SpvManager { mut progress_rx: tokio::sync::watch::Receiver, ) { let status = Arc::clone(&self.status); + let last_error = Arc::clone(&self.last_error); let sync_progress_state = Arc::clone(&self.sync_progress_state); let progress_updated_at = Arc::clone(&self.progress_updated_at); let cancel = self.subtasks.cancellation_token.clone(); @@ -1063,6 +1065,7 @@ impl SpvManager { } let watch_progress = progress_rx.borrow().clone(); let is_synced = watch_progress.is_synced(); + let is_error = watch_progress.state() == SyncState::Error; // Update sync progress state if let Ok(mut stored_sync) = sync_progress_state.write() { @@ -1076,6 +1079,12 @@ impl SpvManager { if let Ok(mut status_guard) = status.write() { if is_synced { *status_guard = SpvStatus::Running; + } else if is_error { + *status_guard = SpvStatus::Error; + if let Ok(mut err_guard) = last_error.write() { + *err_guard = + Some("Sync failed (reported by SPV library)".into()); + } } else if !matches!(*status_guard, SpvStatus::Stopping | SpvStatus::Stopped | SpvStatus::Error) { *status_guard = SpvStatus::Syncing; } @@ -1091,6 +1100,7 @@ impl SpvManager { let reconcile_tx = self.reconcile_tx.lock().ok().and_then(|g| g.clone()); let finality_tx = self.finality_tx.lock().ok().and_then(|g| g.clone()); let status = Arc::clone(&self.status); + let last_error = Arc::clone(&self.last_error); let cancel = self.subtasks.cancellation_token.clone(); self.subtasks.spawn_sync("spv_sync_event_handler", async move { @@ -1135,6 +1145,21 @@ impl SpvManager { { *guard = SpvStatus::Running; } + + // Transition to Error when a sync manager reports a + // fatal failure. The dash-spv library emits this event + // but does NOT update the progress channel on the error + // path, so we must react to the event directly. + if let SyncEvent::ManagerError { ref manager, ref error } = event { + tracing::error!("SPV manager {:?} reported error: {}", manager, error); + if let Ok(mut guard) = status.write() { + *guard = SpvStatus::Error; + } + if let Ok(mut err_guard) = last_error.write() { + *err_guard = Some(format!("Sync manager {} failed: {}", manager, error)); + } + } + if should_signal && let Some(ref tx) = reconcile_tx { From 3b6e3a24b8ee7277605fecedb813344ea0516b3d Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 17:42:21 +0100 Subject: [PATCH 2/6] fix(spv): preserve detailed error message in progress watcher Only set the generic "Sync failed" message in last_error when no detailed message has already been stored by the sync event handler. Co-Authored-By: Claude Opus 4.6 --- src/spv/manager.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/spv/manager.rs b/src/spv/manager.rs index 27357ef30..09d8ef74c 100644 --- a/src/spv/manager.rs +++ b/src/spv/manager.rs @@ -1081,7 +1081,9 @@ impl SpvManager { *status_guard = SpvStatus::Running; } else if is_error { *status_guard = SpvStatus::Error; - if let Ok(mut err_guard) = last_error.write() { + if let Ok(mut err_guard) = last_error.write() + && err_guard.is_none() + { *err_guard = Some("Sync failed (reported by SPV library)".into()); } From a2a74780aa567d289369980c14719e98ffb79082 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 17:52:51 +0100 Subject: [PATCH 3/6] feat(ui): add Error state to connection indicator Distinguish "connected but sync failed" from "never connected" by adding an `OverallConnectionState::Error` variant. SPV sync errors now show a magenta pulsating circle with "!" glyph instead of the same red static circle used for disconnected state. Tooltip shows the specific SPV error message for easier debugging. Co-Authored-By: Claude Opus 4.6 --- src/context/connection_status.rs | 40 +++++++++++++++++++++++++------- src/ui/components/top_panel.rs | 21 ++++++++++++++--- src/ui/theme.rs | 9 +++++++ 3 files changed, 59 insertions(+), 11 deletions(-) diff --git a/src/context/connection_status.rs b/src/context/connection_status.rs index e4bd8cca3..9577cfc58 100644 --- a/src/context/connection_status.rs +++ b/src/context/connection_status.rs @@ -13,7 +13,7 @@ use std::time::{Duration, Instant}; const REFRESH_CONNECTED: Duration = Duration::from_secs(10); const REFRESH_DISCONNECTED: Duration = Duration::from_secs(2); -/// Three-state connection indicator matching the UI's red/orange/green circle. +/// Connection indicator matching the UI's colored circle. #[derive(Debug, Clone, Copy, PartialEq, Eq)] #[repr(u8)] pub enum OverallConnectionState { @@ -23,6 +23,8 @@ pub enum OverallConnectionState { Syncing = 1, /// Fully connected and operational — green indicator. Synced = 2, + /// Connected but sync failed — magenta indicator with "!" glyph. + Error = 3, } impl From for OverallConnectionState { @@ -30,6 +32,7 @@ impl From for OverallConnectionState { match v { 1 => Self::Syncing, 2 => Self::Synced, + 3 => Self::Error, _ => Self::Disconnected, } } @@ -47,6 +50,7 @@ pub struct ConnectionStatus { backend_mode: AtomicU8, disable_zmq: AtomicBool, overall_state: AtomicU8, + spv_last_error: Mutex>, last_update: Mutex, dapi_total_endpoints: AtomicU16, dapi_available_endpoints: AtomicU16, @@ -61,6 +65,7 @@ impl ConnectionStatus { backend_mode: AtomicU8::new(CoreBackendMode::Rpc.as_u8()), disable_zmq: AtomicBool::new(false), overall_state: AtomicU8::new(OverallConnectionState::Disconnected as u8), + spv_last_error: Mutex::new(None), last_update: Mutex::new(Instant::now()), dapi_total_endpoints: AtomicU16::new(0), dapi_available_endpoints: AtomicU16::new(0), @@ -86,6 +91,9 @@ impl ConnectionStatus { OverallConnectionState::Disconnected as u8, Ordering::Relaxed, ); + if let Ok(mut err) = self.spv_last_error.lock() { + *err = None; + } // Set last_update to epoch so the next trigger_refresh fires immediately if let Ok(mut last) = self.last_update.lock() { *last = Instant::now() - REFRESH_CONNECTED; @@ -207,6 +215,7 @@ impl ConnectionStatus { SpvStatus::Starting | SpvStatus::Syncing | SpvStatus::Stopping => { OverallConnectionState::Syncing } + SpvStatus::Error => OverallConnectionState::Error, _ => OverallConnectionState::Disconnected, } } @@ -240,6 +249,7 @@ impl ConnectionStatus { OverallConnectionState::Synced => "Connected to Dash Core Wallet", // RPC mode doesn't currently produce Syncing, but kept for forward-compat. OverallConnectionState::Syncing => "Syncing to Dash Core Wallet", + OverallConnectionState::Error => "Connection error", OverallConnectionState::Disconnected if self.rpc_online() => { "Dash Core connection incomplete" } @@ -252,9 +262,18 @@ impl ConnectionStatus { CoreBackendMode::Spv => { let spv_label = format!("SPV: {:?}", spv_status); let header = match overall { - OverallConnectionState::Synced => "SPV synced", - OverallConnectionState::Syncing => "SPV syncing", - OverallConnectionState::Disconnected => "SPV disconnected", + OverallConnectionState::Synced => "SPV synced".to_string(), + OverallConnectionState::Syncing => "SPV syncing".to_string(), + OverallConnectionState::Error => { + let detail = self + .spv_last_error + .lock() + .ok() + .and_then(|g| g.clone()) + .unwrap_or_else(|| "unknown error".to_string()); + format!("SPV sync error: {detail}") + } + OverallConnectionState::Disconnected => "SPV disconnected".to_string(), }; format!("{header}\n{spv_label}\n{dapi_status}") } @@ -353,10 +372,15 @@ impl ConnectionStatus { match backend_mode { CoreBackendMode::Spv => { - // SPV status is updated elsewhere - let spv_status = app_context.spv_manager().status().status; - tracing::trace!("ConnectionStatus: polled SPV status = {:?}", spv_status); - self.set_spv_status(spv_status); + let snapshot = app_context.spv_manager().status(); + tracing::trace!( + "ConnectionStatus: polled SPV status = {:?}", + snapshot.status + ); + self.set_spv_status(snapshot.status); + if let Ok(mut err) = self.spv_last_error.lock() { + *err = snapshot.last_error; + } } CoreBackendMode::Rpc => { // Update ZMQ status if there's a new event diff --git a/src/ui/components/top_panel.rs b/src/ui/components/top_panel.rs index b2f46add3..9f2000959 100644 --- a/src/ui/components/top_panel.rs +++ b/src/ui/components/top_panel.rs @@ -6,7 +6,9 @@ use crate::context::connection_status::OverallConnectionState; use crate::spv::CoreBackendMode; use crate::ui::ScreenType; use crate::ui::theme::{DashColors, Shadow, Shape}; -use egui::{Context, Frame, Margin, RichText, Stroke, TextureHandle, TopBottomPanel, Ui}; +use egui::{ + Align2, Context, FontId, Frame, Margin, RichText, Stroke, TextureHandle, TopBottomPanel, Ui, +}; use rust_embed::RustEmbed; use std::sync::Arc; use tracing::error; @@ -104,19 +106,21 @@ fn add_connection_indicator(ui: &mut Ui, app_context: &Arc) -> AppAc let dark_mode = ui.ctx().style().visuals.dark_mode; let circle_size = 14.0; - // Three-state color: green (synced), orange (syncing), red (disconnected) + // Color per state: green (synced), orange (syncing), magenta (error), red (disconnected) let color = match overall { OverallConnectionState::Synced => DashColors::success_color(dark_mode), OverallConnectionState::Syncing => DashColors::warning_color(dark_mode), + OverallConnectionState::Error => DashColors::sync_error_color(dark_mode), OverallConnectionState::Disconnected => DashColors::error_color(dark_mode), }; - // Pulsation: synced = normal pulse, syncing = slower pulse, disconnected = none + // Pulsation per state let time = ui.ctx().input(|i| i.time as f32); let pulse_scale = match overall { OverallConnectionState::Synced => 1.0 + 0.2 * (time * 2.0).sin(), OverallConnectionState::Syncing => 1.0 + 0.15 * (time * 1.2).sin(), + OverallConnectionState::Error => 1.0 + 0.25 * (time * 0.8).sin(), OverallConnectionState::Disconnected => 1.0, // No pulse when disconnected }; @@ -147,6 +151,17 @@ fn add_connection_indicator(ui: &mut Ui, app_context: &Arc) -> AppAc // Draw the main circle ui.painter().circle_filled(center, circle_size / 2.0, color); + // Draw "!" glyph on error state + if overall == OverallConnectionState::Error { + ui.painter().text( + center, + Align2::CENTER_CENTER, + "!", + FontId::proportional(10.0), + egui::Color32::WHITE, + ); + } + // Request repaint for animation (only when not disconnected) if overall != OverallConnectionState::Disconnected { app_context.repaint_animation(ui.ctx()); diff --git a/src/ui/theme.rs b/src/ui/theme.rs index dd186acfd..6f9a38f3a 100644 --- a/src/ui/theme.rs +++ b/src/ui/theme.rs @@ -324,6 +324,15 @@ impl DashColors { } } + /// Magenta-red for sync/connection error state (distinct from disconnected red). + pub fn sync_error_color(dark_mode: bool) -> Color32 { + if dark_mode { + Color32::from_rgb(230, 80, 180) + } else { + Color32::from_rgb(200, 50, 150) + } + } + pub fn info_color(dark_mode: bool) -> Color32 { if dark_mode { Color32::from_rgb(100, 180, 255) // Lighter blue for dark mode From c7f66d4f6c58190cdaad2fb5dd2db9c3531d8682 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 18:12:39 +0100 Subject: [PATCH 4/6] =?UTF-8?q?fix:=20address=20review=20findings=20?= =?UTF-8?q?=E2=80=94=20nested=20lock=20and=20stale=20test=20doc?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Move last_error write outside status write guard in spawn_progress_watcher to maintain consistent lock ordering (status → release → last_error), eliminating latent deadlock risk. - Update manual test scenarios to match actual implementation: magenta pulsating "!" indicator (not red/static/Disconnected). - Add Scenario 4 to explicitly verify Error vs Disconnected distinction. Co-Authored-By: Claude Opus 4.6 --- .../manual-test-scenarios.md | 48 ++++++++++++++----- src/spv/manager.rs | 15 +++--- 2 files changed, 44 insertions(+), 19 deletions(-) diff --git a/docs/ai-design/2026-02-24-spv-sync-error-status/manual-test-scenarios.md b/docs/ai-design/2026-02-24-spv-sync-error-status/manual-test-scenarios.md index 2630068a5..c61838808 100644 --- a/docs/ai-design/2026-02-24-spv-sync-error-status/manual-test-scenarios.md +++ b/docs/ai-design/2026-02-24-spv-sync-error-status/manual-test-scenarios.md @@ -3,8 +3,9 @@ ## Context When SPV sync encounters a fatal error (e.g., masternode sync failure), the app -should transition from "Syncing" to an error state, with the connectivity icon -turning red and the tooltip/status panel reflecting the failure. +should transition from "Syncing" to a distinct Error state, with the connectivity +icon turning **magenta** (with "!" glyph and slow pulse) and the tooltip showing +the specific error message. ## Prerequisites @@ -14,8 +15,8 @@ turning red and the tooltip/status panel reflecting the failure. ## Scenario 1: Verify error state on sync failure -**Goal:** Confirm the connectivity icon transitions to red (Disconnected) when -SPV sync fails. +**Goal:** Confirm the connectivity icon transitions to Error (magenta) when +SPV sync fails, distinct from Disconnected (red). ### Steps @@ -24,16 +25,20 @@ SPV sync fails. (Syncing state). 3. If sync completes successfully, the icon should turn green (Running state). 4. If sync fails (e.g., masternode QRInfo failure visible in logs), observe: - - The connectivity icon turns **red** (static, no pulsation). - - Hovering over the icon shows tooltip: **"Disconnected"** with - **"SPV: Error"** detail. + - The connectivity icon turns **magenta** with a slow pulsation and a white + **"!"** glyph in the center. + - Hovering over the icon shows tooltip: **"SPV sync error: {detail}"** with + the specific error message (e.g., "Sync manager Masternode failed: ..."). + - Below that: **"SPV: Error"** detail line. 5. Open the Network Chooser screen and check the SPV status detail — it should - display the error message (e.g., "Sync manager Masternode failed: ..."). + display the error message. ### Expected Result -- Icon transitions from orange (Syncing) to red (Disconnected) on error. -- Tooltip shows "Disconnected / SPV: Error". +- Icon transitions from orange (Syncing) to magenta (Error) on sync failure. +- Error icon is visually distinct from red (Disconnected) — magenta color, + slow pulse, "!" glyph. +- Tooltip shows "SPV sync error: ..." with the specific error message. - Error message is visible in the status detail panel. ## Scenario 2: Verify normal sync still works @@ -47,7 +52,7 @@ SPV sync fails. 3. Observe the connectivity icon transitions: - Orange (Syncing) during sync. - Green (Running) after sync completes. -4. Hover over the icon — tooltip should show "Ready" with "SPV: Running". +4. Hover over the icon — tooltip should show "SPV synced" with "SPV: Running". ### Expected Result @@ -65,15 +70,32 @@ diagnostic information. known chain lock propagation issues). 2. Check application logs for the error: - Look for `SPV manager ... reported error: ...` log line. -3. On the Network Chooser screen, verify the status detail shows the same +3. Hover over the connectivity icon and verify the tooltip shows the same error message (not a generic "Sync failed" without context). ### Expected Result - Log contains `SPV manager "Masternode" reported error: Masternode sync failed: ...`. -- UI status detail shows the specific error from the sync manager, including +- Tooltip shows the specific error from the sync manager, including the block hash reference. +## Scenario 4: Verify Error state is distinct from Disconnected + +**Goal:** Confirm the user can visually distinguish Error from Disconnected. + +### Steps + +1. With the app in SPV mode, trigger a sync error (Scenario 1). +2. Note the icon appearance: magenta, pulsating, "!" glyph. +3. Switch to a network with no connectivity (e.g., disconnect network). +4. Note the icon appearance: red, static, no glyph. + +### Expected Result + +- Error state: magenta circle, slow pulse, white "!" glyph. +- Disconnected state: red circle, static (no pulse), no glyph. +- The two states are clearly visually distinguishable. + ## Notes - The actual QRInfo chain lock error is an upstream issue diff --git a/src/spv/manager.rs b/src/spv/manager.rs index 09d8ef74c..cf64c39bb 100644 --- a/src/spv/manager.rs +++ b/src/spv/manager.rs @@ -1081,16 +1081,19 @@ impl SpvManager { *status_guard = SpvStatus::Running; } else if is_error { *status_guard = SpvStatus::Error; - if let Ok(mut err_guard) = last_error.write() - && err_guard.is_none() - { - *err_guard = - Some("Sync failed (reported by SPV library)".into()); - } } else if !matches!(*status_guard, SpvStatus::Stopping | SpvStatus::Stopped | SpvStatus::Error) { *status_guard = SpvStatus::Syncing; } } + // Write last_error outside status lock to maintain + // consistent lock ordering (status → release → last_error). + if is_error + && let Ok(mut err_guard) = last_error.write() + && err_guard.is_none() + { + *err_guard = + Some("Sync failed (reported by SPV library)".into()); + } } } } From a93b6d6d21b4b77a38eb96f4df7cb49b99e0525a Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 26 Feb 2026 14:02:08 +0100 Subject: [PATCH 5/6] =?UTF-8?q?fix(spv):=20address=20review=20findings=20?= =?UTF-8?q?=E2=80=94=20lock=20ordering,=20error=20semantics,=20allocations?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add explicit drop(guard) in spawn_sync_event_handler for lock ordering - Use first-error-wins policy consistently, log subsequent errors - Add failed_manager_name() helper for detailed progress-channel errors - Add TODO for error string truncation (CWE-400) - Use Cow in SPV tooltip to avoid unnecessary allocations - Document Mutex choice for spv_last_error - Update animation repaint comment to list all pulsating states Co-Authored-By: Claude Opus 4.6 --- src/context/connection_status.rs | 12 +++++---- src/spv/manager.rs | 45 +++++++++++++++++++++++++++++--- src/ui/components/top_panel.rs | 2 +- 3 files changed, 50 insertions(+), 9 deletions(-) diff --git a/src/context/connection_status.rs b/src/context/connection_status.rs index f9251ede0..d1428d7c3 100644 --- a/src/context/connection_status.rs +++ b/src/context/connection_status.rs @@ -51,6 +51,8 @@ pub struct ConnectionStatus { backend_mode: AtomicU8, disable_zmq: AtomicBool, overall_state: AtomicU8, + // NOTE: Mutex (not RwLock) is intentional — single reader (tooltip hover), + // single writer (poll cycle), minimal contention. RwLock overhead not justified. spv_last_error: Mutex>, last_update: Mutex, dapi_total_endpoints: AtomicU16, @@ -266,9 +268,9 @@ impl ConnectionStatus { format!("{header}\n{rpc_status}\n{zmq_status}\n{dapi_status}") } CoreBackendMode::Spv => { - let header = match overall { - OverallConnectionState::Synced => "Ready".to_string(), - OverallConnectionState::Syncing => "Syncing".to_string(), + let header: std::borrow::Cow<'_, str> = match overall { + OverallConnectionState::Synced => "Ready".into(), + OverallConnectionState::Syncing => "Syncing".into(), OverallConnectionState::Error => { let detail = self .spv_last_error @@ -276,9 +278,9 @@ impl ConnectionStatus { .ok() .and_then(|g| g.clone()) .unwrap_or_else(|| "unknown error".to_string()); - format!("SPV sync error: {detail}") + format!("SPV sync error: {detail}").into() } - OverallConnectionState::Disconnected => "Disconnected".to_string(), + OverallConnectionState::Disconnected => "Disconnected".into(), }; let spv_label = if spv_status == SpvStatus::Running { "SPV: Synced".to_string() diff --git a/src/spv/manager.rs b/src/spv/manager.rs index cf64c39bb..667535f3c 100644 --- a/src/spv/manager.rs +++ b/src/spv/manager.rs @@ -1045,6 +1045,26 @@ impl SpvManager { }); } + /// Identify which sync manager phase is in Error state, if any. + fn failed_manager_name(progress: &SpvSyncProgress) -> &'static str { + if progress.masternodes().is_ok_and(|p| p.state() == SyncState::Error) { + return "Masternodes"; + } + if progress.headers().is_ok_and(|p| p.state() == SyncState::Error) { + return "Headers"; + } + if progress.filter_headers().is_ok_and(|p| p.state() == SyncState::Error) { + return "Filter headers"; + } + if progress.filters().is_ok_and(|p| p.state() == SyncState::Error) { + return "Filters"; + } + if progress.blocks().is_ok_and(|p| p.state() == SyncState::Error) { + return "Blocks"; + } + "unknown phase" + } + fn spawn_progress_watcher( &self, mut progress_rx: tokio::sync::watch::Receiver, @@ -1066,6 +1086,11 @@ impl SpvManager { let watch_progress = progress_rx.borrow().clone(); let is_synced = watch_progress.is_synced(); let is_error = watch_progress.state() == SyncState::Error; + let failed_phase = if is_error { + Some(Self::failed_manager_name(&watch_progress)) + } else { + None + }; // Update sync progress state if let Ok(mut stored_sync) = sync_progress_state.write() { @@ -1091,8 +1116,14 @@ impl SpvManager { && let Ok(mut err_guard) = last_error.write() && err_guard.is_none() { - *err_guard = - Some("Sync failed (reported by SPV library)".into()); + // Note: this path is currently unreachable due to upstream + // bug dashpay/rust-dashcore#469 (progress channel never + // receives SyncState::Error). Once fixed, this will fire. + let phase = failed_phase.unwrap_or("unknown phase"); + *err_guard = Some(format!( + "Sync failed: {} (reported by SPV progress channel)", + phase + )); } } } @@ -1159,9 +1190,17 @@ impl SpvManager { tracing::error!("SPV manager {:?} reported error: {}", manager, error); if let Ok(mut guard) = status.write() { *guard = SpvStatus::Error; + drop(guard); // Maintain lock ordering: status → release → last_error } + // TODO: truncate error string to ~512 chars to prevent + // unbounded memory from adversarial peer errors (CWE-400). + let msg = format!("Sync manager {} failed: {}", manager, error); if let Ok(mut err_guard) = last_error.write() { - *err_guard = Some(format!("Sync manager {} failed: {}", manager, error)); + if err_guard.is_none() { + *err_guard = Some(msg); + } else { + tracing::warn!("SPV last_error already set, ignoring subsequent: {}", msg); + } } } diff --git a/src/ui/components/top_panel.rs b/src/ui/components/top_panel.rs index a423e3332..6fcc32ad8 100644 --- a/src/ui/components/top_panel.rs +++ b/src/ui/components/top_panel.rs @@ -162,7 +162,7 @@ fn add_connection_indicator(ui: &mut Ui, app_context: &Arc) -> AppAc ); } - // Request repaint for animation (only when not disconnected) + // Request repaint for animation (Synced, Syncing, and Error states pulse) if overall != OverallConnectionState::Disconnected { app_context.repaint_animation(ui.ctx()); } From acfe2c7ab6d9d96ed5f48e223660dd063d4e81cc Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 26 Feb 2026 14:17:01 +0100 Subject: [PATCH 6/6] =?UTF-8?q?fix(spv):=20address=20round-2=20review=20?= =?UTF-8?q?=E2=80=94=20tooltip=20detail,=20formatting,=20docs?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add explicit SpvStatus::Error branch in spv_label to show "SPV: Error" instead of falling through to spv_phase_summary() which shows "syncing..." - Use Display ({}) instead of Debug ({:?}) for ManagerIdentifier in tracing - Document masternodes-first order in failed_manager_name() Co-Authored-By: Claude Opus 4.6 --- src/context/connection_status.rs | 2 ++ src/spv/manager.rs | 29 +++++++++++++++++++++++------ 2 files changed, 25 insertions(+), 6 deletions(-) diff --git a/src/context/connection_status.rs b/src/context/connection_status.rs index d1428d7c3..148d30e72 100644 --- a/src/context/connection_status.rs +++ b/src/context/connection_status.rs @@ -284,6 +284,8 @@ impl ConnectionStatus { }; let spv_label = if spv_status == SpvStatus::Running { "SPV: Synced".to_string() + } else if spv_status == SpvStatus::Error { + "SPV: Error".to_string() } else { app_context .spv_manager() diff --git a/src/spv/manager.rs b/src/spv/manager.rs index 667535f3c..374d1ad2a 100644 --- a/src/spv/manager.rs +++ b/src/spv/manager.rs @@ -1046,20 +1046,37 @@ impl SpvManager { } /// Identify which sync manager phase is in Error state, if any. + /// Checks masternodes first as the most common failure point, + /// rather than pipeline execution order used by `spv_phase_summary()`. fn failed_manager_name(progress: &SpvSyncProgress) -> &'static str { - if progress.masternodes().is_ok_and(|p| p.state() == SyncState::Error) { + if progress + .masternodes() + .is_ok_and(|p| p.state() == SyncState::Error) + { return "Masternodes"; } - if progress.headers().is_ok_and(|p| p.state() == SyncState::Error) { + if progress + .headers() + .is_ok_and(|p| p.state() == SyncState::Error) + { return "Headers"; } - if progress.filter_headers().is_ok_and(|p| p.state() == SyncState::Error) { + if progress + .filter_headers() + .is_ok_and(|p| p.state() == SyncState::Error) + { return "Filter headers"; } - if progress.filters().is_ok_and(|p| p.state() == SyncState::Error) { + if progress + .filters() + .is_ok_and(|p| p.state() == SyncState::Error) + { return "Filters"; } - if progress.blocks().is_ok_and(|p| p.state() == SyncState::Error) { + if progress + .blocks() + .is_ok_and(|p| p.state() == SyncState::Error) + { return "Blocks"; } "unknown phase" @@ -1187,7 +1204,7 @@ impl SpvManager { // but does NOT update the progress channel on the error // path, so we must react to the event directly. if let SyncEvent::ManagerError { ref manager, ref error } = event { - tracing::error!("SPV manager {:?} reported error: {}", manager, error); + tracing::error!("SPV manager {} reported error: {}", manager, error); if let Ok(mut guard) = status.write() { *guard = SpvStatus::Error; drop(guard); // Maintain lock ordering: status → release → last_error