diff --git a/CHANGELOG.md b/CHANGELOG.md index 1eddbec28..7753f2037 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -186,6 +186,48 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **Token balance refresh status**: requesting a refresh while token balances + are already updating now shows a brief informational note instead of a red + error banner that must be dismissed. + +- **Topping up an identity from more than one funding wallet no longer gets + stuck loading**: requesting a new deposit address at the same time as + refreshing your asset-lock transactions could silently drop the refresh, + leaving those wallets stuck showing "Loading" for the rest of the session. + Both requests are now sent together. + +- **A single damaged transaction record no longer hides an entire wallet**: + previously, one unreadable entry in a wallet's transaction history could + make the wallet and its balance disappear from every screen until the + underlying data was manually repaired. The app now skips the damaged entry + and keeps the wallet visible, with the rest of its history intact. + +- **Scheduled DPNS vote sweeps no longer get stuck after an unexpected error**: + an internal failure while casting due votes could permanently block future + vote sweeps for that network until the app was restarted. It now recovers + on its own. + +- **Send confirmations now show what the recipient actually receives**: when + the network fee is deducted from the amount you entered, the confirmation + dialog now says so and shows the reduced amount the recipient will get, + instead of implying they receive the full entered amount. + +- **A missed deposit no longer leaves the funding screen waiting forever**: + if the app missed the notification that your deposit arrived (for example + because you were on another screen), it now also checks your wallet + balance directly, so the funding step advances even if the one-time + notification was missed. + +- **Deposit-address screens no longer strand you after an address error**: if + generating a new receive address failed, the screen used to reset to a bare + view with no way to retry. It now keeps the retry button available. + +- **A network preference that can't be restored no longer defaults you to + Mainnet**: if the app can't confirm your previous network selection during + an upgrade, it now asks you to choose a network explicitly instead of + silently starting the session on Mainnet — important if you were previously + using Testnet. + - **Submitted Platform actions are no longer reported as rejected when only confirmation failed**: if a state transition was broadcast but its result could not be confirmed, the app now tells you to check whether it completed diff --git a/docs/gui-testing/README.md b/docs/gui-testing/README.md index eae35399d..7da93a2b9 100644 --- a/docs/gui-testing/README.md +++ b/docs/gui-testing/README.md @@ -25,12 +25,106 @@ network timing, or a flow `kittest` can't simulate. ## How to run a scenario -The mechanical launch recipe (X display setup, accessibility tree, gotchas -like stderr redirection) lives in the global `desktop-gui` Claude Code skill -(`~/.claude/skills/desktop-gui/SKILL.md`) — read that first, it's not repeated -here. This directory covers what's specific to *this project*: which -scenarios exist, what credentials they need, and the safety rules for running -them against a live network. +Use a graphical session whose `DISPLAY` already points at the desktop you want +to observe. Do not assume a display number: local desktops, SSH forwarding, +CI, and headless X servers all use different values. Verify the selected +display before launching: + +```bash +: "${DISPLAY:?Set DISPLAY to the desktop used for GUI testing}" +xdpyinfo >/dev/null +``` + +On a minimal Ubuntu host, egui/eframe also needs an X keyboard library and a +wgpu backend. Install missing packages only after the launch log identifies +the corresponding failure: + +```bash +sudo apt-get install -y libxkbcommon-x11-0 mesa-vulkan-drivers xdotool +``` + +Build in the checkout under test, then ask Cargo for the effective target +directory. This honors `CARGO_TARGET_DIR` and any `target-dir` configured in +Cargo's configuration files. + +```bash +cargo build +TARGET_DIR=$(cargo metadata --format-version 1 --no-deps | \ + python3 -c 'import json, sys; print(json.load(sys.stdin)["target_directory"])') +BIN="$TARGET_DIR/debug/dash-evo-tool" +test -x "$BIN" +``` + +Prepare isolated state and launch the binary as a detached process. The app +redirects some diagnostics into its data directory, so inspect both the launch +log and `det-stderr.log` / `det.log` after a crash or panic. + +```bash +DATADIR=$(mktemp -d) +cp .env.example "$DATADIR/.env" +LOG="$DATADIR/gui-test-launch.log" + +pgrep -af dash-evo-tool +DASH_EVO_DATA_DIR="$DATADIR" nohup "$BIN" >"$LOG" 2>&1 & +APP_PID=$! + +pgrep -af "$BIN" +WID=$(xdotool search --pid "$APP_PID" | head -1) +xdotool getwindowgeometry "$WID" +xdotool windowsize "$WID" 1260 780 +xdotool windowactivate "$WID" +``` + +### Accessibility tree + +Dash Evo Tool publishes its AccessKit tree over AT-SPI2 when accessibility is +enabled. A headless Ubuntu session needs the accessibility bus and Python +bindings: + +```bash +sudo apt-get install -y at-spi2-core python3-pyatspi +export DBUS_SESSION_BUS_ADDRESS="unix:path=/run/user/$(id -u)/bus" + +dbus-send --session --print-reply \ + --dest=org.a11y.Bus /org/a11y/bus org.a11y.Bus.GetAddress +gdbus call --session --dest org.a11y.Bus --object-path /org/a11y/bus \ + --method org.freedesktop.DBus.Properties.Set \ + org.a11y.Status ScreenReaderEnabled '' +gdbus call --session --dest org.a11y.Bus --object-path /org/a11y/bus \ + --method org.freedesktop.DBus.Properties.Set \ + org.a11y.Status IsEnabled '' +``` + +Add `DASH_EVO_TOOL_ACCESSIBILITY=1` to the launch command, focus the window +with `xdotool windowactivate "$WID"`, and inspect the tree with any AT-SPI +client. This self-contained Python example prints roles and labels: + +```bash +python3 - <<'PY' +import pyatspi + + +def walk(node, depth=0): + print(f"{' ' * depth}{node.getRoleName()}: {node.name}") + for child in node: + walk(child, depth + 1) + + +for app in pyatspi.Registry.getDesktop(0): + if app.name == "dash-evo-tool": + walk(app) +PY +``` + +An empty application list normally means the AT-SPI status flags are still +disabled or the app window is not focused. The tree can lag during screen +transitions and omits purely decorative visuals, so use it for semantic labels +and structure while using screenshots for pixels, colors, and final visual +confirmation. Capture durable screenshots with `scrot -o .png`. + +The complete recipe is versioned here because an installed `desktop-gui` +automation skill is not available to every contributor. When present, that +skill may still provide convenient screenshot and input tooling. ## Non-negotiable safety rules @@ -138,8 +232,8 @@ one scenario silently invalidating another: ## Known UI/environment quirks - **Default window is small (800×600) and clips controls** (sidebar items, - settings sections below the fold). Resize immediately after launch — see the - `desktop-gui` skill's launch recipe. Some settings sections are collapsible + settings sections below the fold). Resize immediately after launch with the + `xdotool windowsize` command above. Some settings sections are collapsible *and* below the fold even after resizing: expect to expand a section, then scroll, before a control becomes visible — don't conclude a control doesn't exist from the first screenshot after expanding. @@ -151,12 +245,11 @@ one scenario silently invalidating another: and dismiss it immediately. If a dialog flashes shut the instant it opens, suspect this pattern before assuming a mis-click — take a screenshot a frame later and retry with the click and the opening action clearly separated. -- **The shared `/data/target` build output is not campaign-exclusive.** If - other worktrees/sessions on the same box can rebuild concurrently, the - binary under test can be silently overwritten mid-campaign by an unrelated - build. For any run spanning hours, build to a private path and hash-verify - (`sha256sum`) before each relaunch rather than trusting the shared path - throughout. +- **A shared Cargo target directory is not campaign-exclusive.** If other + worktrees/sessions on the same box can rebuild concurrently, the binary under + test can be silently overwritten mid-campaign by an unrelated build. For any + run spanning hours, set `CARGO_TARGET_DIR` to a private path before building + and hash-verify (`sha256sum`) before each relaunch. ## Scenario index diff --git a/docs/gui-testing/scenarios/TEMPLATE.md b/docs/gui-testing/scenarios/TEMPLATE.md index 025b6a62b..8c9cac324 100644 --- a/docs/gui-testing/scenarios/TEMPLATE.md +++ b/docs/gui-testing/scenarios/TEMPLATE.md @@ -24,7 +24,12 @@ cp .env.example "$DATADIR/.env" # Confirm no conflicting instance is already using this display/data dir pgrep -af dash-evo-tool -DISPLAY=:99 DASH_EVO_DATA_DIR="$DATADIR" nohup /data/target/debug/dash-evo-tool >/tmp/.log 2>&1 & +TARGET_DIR=$(cargo metadata --format-version 1 --no-deps | \ + python3 -c 'import json, sys; print(json.load(sys.stdin)["target_directory"])') +BIN="$TARGET_DIR/debug/dash-evo-tool" +test -x "$BIN" +LOG="$DATADIR/.log" +DASH_EVO_DATA_DIR="$DATADIR" nohup "$BIN" >"$LOG" 2>&1 & ``` ## Procedure diff --git a/src/app.rs b/src/app.rs index 61fe225cd..21f874d64 100644 --- a/src/app.rs +++ b/src/app.rs @@ -100,16 +100,75 @@ pub(crate) fn scheduled_vote_sweep_is_quiet(error: &TaskError) -> bool { const LEGACY_SETTINGS_IMPORT_WARNING: &str = "The app could not confirm that your network preference was restored from the previous version. Check the selected network before using the application."; -fn show_legacy_settings_import_warning( - ctx: &egui::Context, - error: &crate::backend_task::migration::legacy_settings::SettingsImportError, -) { +fn show_legacy_settings_import_warning(ctx: &egui::Context, error: &impl std::fmt::Debug) { let handle = MessageBanner::set_global(ctx, LEGACY_SETTINGS_IMPORT_WARNING, MessageType::Warning); handle.disable_auto_dismiss(); handle.with_details(error); } +fn legacy_settings_import_requires_network_selection( + _error: &crate::backend_task::migration::legacy_settings::SettingsImportError, +) -> bool { + true +} + +fn initial_root_screen( + persisted: RootScreenType, + persisted_is_registered: bool, + network_selection_required: bool, +) -> RootScreenType { + if network_selection_required { + RootScreenType::RootScreenNetworkChooser + } else if persisted_is_registered { + persisted + } else { + FALLBACK_ROOT_SCREEN + } +} + +fn show_welcome_screen(onboarding_completed: bool, network_selection_required: bool) -> bool { + !onboarding_completed && !network_selection_required +} + +fn network_selection_allows_root( + network_selection_required: bool, + root_screen: RootScreenType, +) -> bool { + !network_selection_required || root_screen == RootScreenType::RootScreenNetworkChooser +} + +fn network_selection_allows_action(network_selection_required: bool, action: &AppAction) -> bool { + !network_selection_required || matches!(action, AppAction::None | AppAction::SwitchNetwork(_)) +} + +fn boot_auto_start_spv( + onboarding_completed: bool, + auto_start_spv: bool, + network_selection_required: bool, +) -> bool { + onboarding_completed && auto_start_spv && !network_selection_required +} + +fn clear_scheduled_vote_sweep_guard_on_error( + in_progress: &mut BTreeSet, + context: &BackendTaskContext, + error: &TaskError, +) { + let network = match (context, error) { + (_, TaskError::ScheduledVoteSweepFailed { network, .. }) => Some(*network), + (_, TaskError::ScheduledVoteSweepAllAddressesExhausted { network, .. }) => Some(*network), + ( + BackendTaskContext::ScheduledVoteSweep { network }, + TaskError::BackendTaskFailed { .. }, + ) => Some(*network), + _ => None, + }; + if let Some(network) = network { + in_progress.remove(&network); + } +} + fn unix_time_ms() -> u64 { SystemTime::now() .duration_since(UNIX_EPOCH) @@ -265,6 +324,44 @@ mod backend_task_join_tests { ); } + #[tokio::test] + async fn panicking_scheduled_vote_sweep_clears_in_progress_guard() { + let network = Network::Testnet; + let unrelated_network = Network::Regtest; + let mut in_progress = BTreeSet::from([network, unrelated_network]); + let (tx, mut rx) = tokio::sync::mpsc::channel(1); + let sender = SenderAsync::new(tx, egui::Context::default()); + let join_handle = tokio::task::spawn_blocking(|| panic!("scheduled sweep panic")); + + forward_backend_task_join_error( + join_handle, + sender, + None, + BackendTaskContext::ScheduledVoteSweep { network }, + ) + .await; + + let TaskResult::Error { context, error } = rx + .recv() + .await + .expect("join failure result must be forwarded") + else { + panic!("expected a scheduled-vote sweep error"); + }; + assert!(matches!(error, TaskError::BackendTaskFailed { .. })); + + clear_scheduled_vote_sweep_guard_on_error(&mut in_progress, &context, &error); + + assert!( + !in_progress.contains(&network), + "a terminal panic must allow the next scheduled-vote sweep" + ); + assert!( + in_progress.contains(&unrelated_network), + "a terminal panic must not release another network's sweep guard" + ); + } + #[tokio::test] async fn panicking_paid_contact_action_keeps_its_request_correlation() { let (tx, mut rx) = tokio::sync::mpsc::channel(1); @@ -655,6 +752,8 @@ pub struct AppState { network_switch_pending: Option, /// Progress banner displayed while a network switch is in progress. network_switch_banner: Option, + /// Whether boot must remain on the network chooser until the user confirms a network. + network_selection_required: bool, pub task_result_sender: egui_mpsc::SenderAsync, // Channel sender for sending task results pub task_result_receiver: tokiompsc::Receiver, // Channel receiver for receiving task results theme: ThemeState, @@ -693,6 +792,9 @@ pub struct AppState { /// Shared MCP context -- follows network switches via `ArcSwap`. #[cfg(feature = "mcp")] pub mcp_app_context: Option>>, + /// MCP configuration held until a required boot-time network selection succeeds. + #[cfg(feature = "mcp")] + mcp_server_pending_config: Option, /// The egui secret prompt host, kept so newly-created (on-demand) network /// contexts can have it installed before their backend is wired. secret_prompt_host: Arc, @@ -963,20 +1065,26 @@ impl AppState { // out of legacy `data.db` before they are read below. This has to run // here, ahead of the read: the active network is chosen from the blob // a few lines down, and booting a testnet user onto mainnet is a - // safety hazard. A failure is not fatal — the boot continues on - // defaults and the (unwritten) sentinel makes the next launch retry. - match crate::backend_task::migration::legacy_settings::import_legacy_settings(&app_kv, &db) - { - Ok(outcome) => tracing::debug!(?outcome, "Legacy settings import"), - Err(e) => { - tracing::warn!( - error = ?e, - "Could not import preferences from the previous version — using defaults; \ - the next launch retries", - ); - show_legacy_settings_import_warning(&ctx, &e); - } - } + // safety hazard. Read/write failures force explicit network selection; + // the (unwritten) sentinel makes the next launch retry. + let mut network_selection_required = + match crate::backend_task::migration::legacy_settings::import_legacy_settings( + &app_kv, &db, + ) { + Ok(outcome) => { + tracing::debug!(?outcome, "Legacy settings import"); + false + } + Err(e) => { + tracing::warn!( + error = ?e, + "Could not import preferences from the previous version — using defaults; \ + the next launch retries", + ); + show_legacy_settings_import_warning(&ctx, &e); + legacy_settings_import_requires_network_selection(&e) + } + }; let settings = match app_kv.get::(DetScope::Global, AppSettings::KV_KEY) { Ok(Some(s)) => s, @@ -986,6 +1094,8 @@ impl AppState { error = ?e, "Failed to read AppSettings at boot — using defaults" ); + show_legacy_settings_import_warning(&ctx, &e); + network_selection_required = true; AppSettings::default() } }; @@ -1157,7 +1267,11 @@ impl AppState { // task that wires the backend goes on to start SPV. Folding the start // into the spawned init closes the boot race where a synchronous // `start_spv()` fired before the fire-and-forget wiring could finish. - let boot_auto_start_spv = onboarding_completed && settings.auto_start_spv; + let boot_auto_start_spv = boot_auto_start_spv( + onboarding_completed, + settings.auto_start_spv, + network_selection_required, + ); for (&net, app_ctx) in network_contexts.iter() { let auto_start = boot_auto_start_spv && net == chosen_network; Self::spawn_backend_init( @@ -1171,28 +1285,25 @@ impl AppState { // MCP server (feature-gated, opt-in via MCP_API_KEY env var) #[cfg(feature = "mcp")] - let mcp_app_context = { + let (mcp_app_context, mcp_server_pending_config) = { if let Some(mcp_config) = crate::mcp::McpConfig::from_env() { let initial_ctx = active_context.clone(); let mcp_ctx = Arc::new(arc_swap::ArcSwap::new(initial_ctx)); - let ctx_for_server = mcp_ctx.clone(); - let cancel = subtasks.cancellation_token.clone(); - subtasks.spawn_sync("mcp-server", async move { - if let Err(e) = - crate::mcp::start_http_server(ctx_for_server, mcp_config, cancel).await - { - tracing::error!("MCP server failed: {e}"); - } - }); - tracing::debug!("MCP server enabled"); - Some(mcp_ctx) + let pending_config = if !network_selection_required { + Self::spawn_mcp_server(&subtasks, mcp_ctx.clone(), mcp_config); + None + } else { + tracing::debug!("MCP server deferred until network selection"); + Some(mcp_config) + }; + (Some(mcp_ctx), pending_config) } else { let reason = match std::env::var("MCP_API_KEY") { Ok(ref k) if !k.is_empty() => "MCP_API_KEY is set but invalid (too short)", _ => "MCP_API_KEY not set", }; tracing::debug!("MCP server disabled ({reason})"); - None + (None, None) } }; @@ -1314,11 +1425,11 @@ impl AppState { // Resolve the effective selected root screen. If the persisted value is // no longer registered, fall back to `FALLBACK_ROOT_SCREEN` so // `active_root_screen_mut()` does not panic on first frame. - let selected_main_screen = if main_screens.contains_key(&persisted_main_screen) { - persisted_main_screen - } else { - FALLBACK_ROOT_SCREEN - }; + let selected_main_screen = initial_root_screen( + persisted_main_screen, + main_screens.contains_key(&persisted_main_screen), + network_selection_required, + ); let mut app_state = Self { main_screens, @@ -1329,6 +1440,7 @@ impl AppState { network_contexts, network_switch_pending: None, network_switch_banner: None, + network_selection_required, task_result_sender, task_result_receiver, theme: ThemeState::new(theme_preference), @@ -1338,7 +1450,10 @@ impl AppState { scheduled_vote_recovery_last_attempt: BTreeMap::new(), last_repaint_request: Instant::now(), subtasks, - show_welcome_screen: !onboarding_completed, + show_welcome_screen: show_welcome_screen( + onboarding_completed, + network_selection_required, + ), welcome_screen: None, connection_banner: ConnectionBanner::new(), // Arm the block for the boot SPV sync when it auto-starts (F-SPV-A: @@ -1350,6 +1465,8 @@ impl AppState { accessibility: AccessibilityActivator::new(accessibility_enforced), #[cfg(feature = "mcp")] mcp_app_context, + #[cfg(feature = "mcp")] + mcp_server_pending_config, secret_prompt_host, secret_prompt_receiver, active_secret_prompt: None, @@ -1449,6 +1566,21 @@ impl AppState { }); } + #[cfg(feature = "mcp")] + fn spawn_mcp_server( + subtasks: &Arc, + app_context: Arc>, + config: crate::mcp::McpConfig, + ) { + let cancel = subtasks.cancellation_token.clone(); + subtasks.spawn_sync("mcp-server", async move { + if let Err(error) = crate::mcp::start_http_server(app_context, config, cancel).await { + tracing::error!(%error, "MCP server failed"); + } + }); + tracing::debug!("MCP server enabled"); + } + // Handle the backend task and send the result through the channel. // // Uses spawn_blocking + block_on to avoid Send bound issues with platform @@ -1595,6 +1727,7 @@ impl AppState { /// Complete the network switch after the context is available. fn finalize_network_switch(&mut self, network: Network) { + let was_network_selection_required = self.network_selection_required; // Forget any session-cached secrets on the outgoing context before we // leave it. The outgoing per-network context stays cached in // `network_contexts` (its `WalletBackend` is NOT dropped on switch), so @@ -1605,9 +1738,15 @@ impl AppState { } self.chosen_network = network; + self.network_selection_required = false; let app_context = self.current_app_context().clone(); + if was_network_selection_required && !app_context.get_app_settings().onboarding_completed { + self.show_welcome_screen = true; + self.welcome_screen = Some(WelcomeScreen::new(app_context.clone())); + } + // Same eager wallet-backend init as at app start (Case B): chain- // only SDK lookups must work pre-unlock on the freshly-switched // context too, otherwise the SDK tight-loops on WalletBackendNotYetWired. @@ -1632,6 +1771,13 @@ impl AppState { mcp_ctx.store(app_context.clone()); tracing::debug!("MCP context switched to {:?}", network); } + #[cfg(feature = "mcp")] + if let (Some(mcp_ctx), Some(config)) = ( + self.mcp_app_context.clone(), + self.mcp_server_pending_config.take(), + ) { + Self::spawn_mcp_server(&self.subtasks, mcp_ctx, config); + } // Deliberately clear stale banners from the previous network context. // A backend task completing after the switch could set a new banner in the new @@ -1660,9 +1806,18 @@ impl AppState { self.migration.reset_for_switch(); // Persist the network choice. - app_context - .update_settings(RootScreenType::RootScreenNetworkChooser) - .ok(); + match app_context.update_settings(RootScreenType::RootScreenNetworkChooser) { + Ok(()) if was_network_selection_required => { + if let Err(error) = crate::backend_task::migration::legacy_settings::finish_after_explicit_network_selection(app_context.app_kv().as_ref()) { + show_legacy_settings_import_warning(app_context.egui_ctx(), &error); + } + } + Ok(()) => {} + Err(error) => { + tracing::warn!(error = ?error, "Could not persist the selected network"); + show_legacy_settings_import_warning(app_context.egui_ctx(), &error); + } + } } /// Whether a passphrase prompt owns the frame's full interaction surface. @@ -1797,6 +1952,9 @@ impl AppState { } fn set_main_screen(&mut self, root_screen_type: RootScreenType) { + if !network_selection_allows_root(self.network_selection_required, root_screen_type) { + return; + } self.select_main_screen(root_screen_type); self.active_root_screen_mut().refresh_on_arrival(); self.current_app_context() @@ -2083,6 +2241,15 @@ impl App for AppState { // without a manual Refresh. No banner — this fires every 15 s. active_context.apply_platform_address_push(updates); } + BackendTaskSuccessResult::TokenBalanceRefreshAlreadyInFlight => { + MessageBanner::set_global( + ctx, + "Token balances are already refreshing. Wait a moment before refreshing again.", + MessageType::Info, + ); + self.visible_screen_mut() + .display_backend_task_result(&context, unboxed_message); + } _ => { // For all other success results, let the screen decide how to display // the outcome without showing a generic global success banner. @@ -2107,6 +2274,13 @@ impl App for AppState { } => { self.network_switch_pending = None; self.network_switch_banner.take_and_clear(); + let current_context = self.current_app_context().clone(); + if let Some(screen) = self + .main_screens + .get_mut(&RootScreenType::RootScreenNetworkChooser) + { + screen.change_context(current_context); + } MessageBanner::set_global(ctx, err.to_string(), MessageType::Error) .disable_auto_dismiss(); } @@ -2124,10 +2298,14 @@ impl App for AppState { TaskResult::Error { context, error: - err @ (TaskError::ScheduledVoteSweepFailed { network, .. } - | TaskError::ScheduledVoteSweepAllAddressesExhausted { network, .. }), + err @ (TaskError::ScheduledVoteSweepFailed { .. } + | TaskError::ScheduledVoteSweepAllAddressesExhausted { .. }), } => { - self.scheduled_vote_sweeps_in_progress.remove(&network); + clear_scheduled_vote_sweep_guard_on_error( + &mut self.scheduled_vote_sweeps_in_progress, + &context, + &err, + ); self.visible_screen_mut() .display_backend_task_error(&context, &err); let handled = self.visible_screen_mut().display_task_error(&err); @@ -2144,6 +2322,11 @@ impl App for AppState { context, error: err, } => { + clear_scheduled_vote_sweep_guard_on_error( + &mut self.scheduled_vote_sweeps_in_progress, + &context, + &err, + ); self.route_contact_request_error_to_hidden_hub(&err); let is_database_clear = context == BackendTaskContext::ClearNetworkDatabase; let suppress_stale_error = !is_database_clear @@ -2210,7 +2393,9 @@ impl App for AppState { self.scheduled_vote_sweep_deferred_since_ms .entry(network) .or_insert_with(unix_time_ms); - } else if !self.scheduled_vote_sweeps_in_progress.contains(&network) { + } else if !self.network_selection_required + && !self.scheduled_vote_sweeps_in_progress.contains(&network) + { let preserve_eligibility_since_ms = self .scheduled_vote_sweep_deferred_since_ms .get(&network) @@ -2229,11 +2414,14 @@ impl App for AppState { .insert(network, now); } self.scheduled_vote_sweeps_in_progress.insert(network); - self.handle_backend_task(BackendTask::ContestedResourceTask( - ContestedResourceTask::CastDueScheduledVotes { - preserve_eligibility_since_ms, - }, - )); + self.handle_backend_task_with_context( + BackendTask::ContestedResourceTask( + ContestedResourceTask::CastDueScheduledVotes { + preserve_eligibility_since_ms, + }, + ), + BackendTaskContext::ScheduledVoteSweep { network }, + ); } } @@ -2307,18 +2495,31 @@ impl App for AppState { { self.handle_backend_task(task); } - if let Some(task) = self.migration.dispatch_cold_start(&active_context) { + if !self.network_selection_required + && let Some(task) = self.migration.dispatch_cold_start(&active_context) + { self.handle_backend_task(task); } - self.migration - .update_banner(ctx, &active_context, migration_state.as_ref()); - self.migration.handle_esc(ctx); - if let Some(task) = self.migration.drain_actions(ctx, self.chosen_network) { - self.handle_backend_task(task); + if !self.network_selection_required { + self.migration + .update_banner(ctx, &active_context, migration_state.as_ref()); + self.migration.handle_esc(ctx); + if let Some(task) = self.migration.drain_actions(ctx, self.chosen_network) { + self.handle_backend_task(task); + } } self.drain_overlay_actions(ctx); for action in actions { + if !network_selection_allows_action(self.network_selection_required, &action) { + tracing::debug!("Blocked an action until the user confirms a network"); + MessageBanner::set_global( + ctx, + "Choose a network before using this control.", + MessageType::Info, + ); + continue; + } match action { AppAction::None => {} AppAction::AddScreen(screen) => self.screen_stack.push(screen), @@ -2524,6 +2725,69 @@ mod migration_banner_tests { MessageBanner::clear_global_message(&ctx, LEGACY_SETTINGS_IMPORT_WARNING); } + #[test] + fn legacy_settings_io_failure_requires_explicit_network_selection() { + use crate::backend_task::migration::legacy_settings::SettingsImportError; + use crate::wallet_backend::KvAdapterError; + + let read_error = SettingsImportError::LegacyRead { + source: rusqlite::Error::InvalidQuery, + }; + let write_error = SettingsImportError::Write { + source: KvAdapterError::Truncated, + }; + + for error in [&read_error, &write_error] { + let selection_required = legacy_settings_import_requires_network_selection(error); + assert!(selection_required); + assert_eq!( + initial_root_screen( + RootScreenType::RootScreenWalletsBalances, + true, + selection_required, + ), + RootScreenType::RootScreenNetworkChooser, + ); + assert!(!show_welcome_screen(false, selection_required)); + assert!(!boot_auto_start_spv(true, true, selection_required)); + assert!(!network_selection_allows_root( + selection_required, + RootScreenType::RootScreenWalletsBalances, + )); + assert!(network_selection_allows_root( + selection_required, + RootScreenType::RootScreenNetworkChooser, + )); + assert!(!network_selection_allows_action( + selection_required, + &AppAction::StartSpv, + )); + assert!(!network_selection_allows_action( + selection_required, + &AppAction::BackendTask(BackendTask::None), + )); + assert!(network_selection_allows_action( + selection_required, + &AppAction::SwitchNetwork(Network::Mainnet), + )); + } + + let version_error = SettingsImportError::LegacyDataTooOld { + found: 1, + minimum_supported: 11, + }; + assert!(legacy_settings_import_requires_network_selection( + &version_error + )); + } + + #[test] + fn onboarding_resumes_after_required_network_selection() { + assert!(!show_welcome_screen(false, true)); + assert!(show_welcome_screen(false, false)); + assert!(!show_welcome_screen(true, false)); + } + #[test] fn deferred_vote_cutoff_clears_only_after_matching_success() { let mut deferred = BTreeMap::from([(Network::Testnet, 42)]); diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs index fa5ec69f1..50c343d57 100644 --- a/src/backend_task/error.rs +++ b/src/backend_task/error.rs @@ -66,6 +66,9 @@ pub enum WalletTransactionHistoryError { /// A record was removed between key enumeration and the record lookup. #[error("transaction record {txid} disappeared during hydration")] RecordMissing { txid: dash_sdk::dpp::dashcore::Txid }, + /// One or more persisted rows could not be decoded during hydration. + #[error("{skipped_rows} transaction history rows could not be loaded")] + RowsSkipped { skipped_rows: usize }, } /// Redacted diagnostic for a backend task that panicked or was cancelled. @@ -312,13 +315,23 @@ pub enum TaskError { /// Persisted Core transaction rows could not be read through the upstream /// wallet persistence API during wallet registration. #[error( - "Could not load this wallet's transaction history. Restart the application and try again." + "Could not load this wallet's transaction history. Your balance is unaffected. Restart the application and try again." )] WalletTransactionHistoryLoad { #[source] source: WalletTransactionHistoryError, }, + /// Some persisted transaction rows were unreadable, but wallet + /// registration and balance hydration completed. + #[error( + "Some of this wallet's transaction history could not be loaded. Your balance is unaffected. Restart the application and try again." + )] + WalletTransactionHistoryPartial { + #[source] + source: WalletTransactionHistoryError, + }, + /// The on-disk wallet database was written by a newer build of the app /// than the one running, so this build cannot open it. Distinct from /// [`Self::WalletStorage`] because restarting or freeing disk space never @@ -1147,12 +1160,6 @@ pub enum TaskError { )] TokenBalanceRefreshInProgress, - /// Upstream skipped a token-balance refresh because another pass was still running. - #[error( - "Token balances are still being refreshed. Wait a moment and refresh the Tokens screen again." - )] - TokenBalanceRefreshSkipped, - /// Connected server is behind (SdkError::StaleNode). #[error("The server you connected to is behind. Please retry.")] DapiStaleNode { diff --git a/src/backend_task/migration/legacy_settings.rs b/src/backend_task/migration/legacy_settings.rs index f1ce9ba58..992de8830 100644 --- a/src/backend_task/migration/legacy_settings.rs +++ b/src/backend_task/migration/legacy_settings.rs @@ -28,6 +28,7 @@ use crate::wallet_backend::{DetKv, DetScope, KvAdapterError}; /// [`finish_unwire`](super::finish_unwire) sentinel this one is not /// per-network. Versioned so a future format change bumps the key. const SENTINEL_KEY: &str = "det:migration:legacy_settings:v1"; +const IMPORT_GUARD_KEY: &str = "det:migration:legacy_settings:guard:v1"; /// What the import did on this launch. Returned so the caller can log it and /// tests can assert the branch taken. @@ -44,8 +45,9 @@ pub enum SettingsImport { Imported { network: Network }, } -/// Failure of the settings import. The caller boots with defaults on error -/// and leaves the sentinel unwritten, so the next launch retries. +/// Failure of the settings import. The caller boots with current settings or +/// defaults on error. Once importing begins, the durable guard makes recovery +/// finish without rereading stale legacy data. #[derive(Debug, thiserror::Error)] pub enum SettingsImportError { /// The legacy database predates the oldest layout this import supports. @@ -67,6 +69,20 @@ pub enum SettingsImportError { source: rusqlite::Error, }, + /// The current settings or a migration marker could not be read. + #[error("could not read saved settings")] + Read { + #[source] + source: KvAdapterError, + }, + + /// The previous settings could not be encoded for guard comparison. + #[error("could not prepare saved settings for migration")] + Encode { + #[source] + source: bincode::error::EncodeError, + }, + /// The imported settings, or the sentinel, could not be written to the /// app k/v store. #[error("could not write imported settings")] @@ -88,7 +104,8 @@ pub enum SettingsImportError { /// /// Returns [`SettingsImportError`] when the saved data is outside the supported /// direct-update range, `data.db` cannot be read, or the k/v store cannot be -/// written. The sentinel stays unwritten on failure. +/// written. The durable guard prevents a partial import from overwriting newer +/// settings on a later launch. pub fn import_legacy_settings( app_kv: &DetKv, db: &Database, @@ -96,6 +113,9 @@ pub fn import_legacy_settings( if sentinel_present(app_kv)? { return Ok(SettingsImport::AlreadyDone); } + if let Some(guard) = read_import_guard(app_kv)? { + return finish_guarded_import(app_kv, guard); + } let version = db .stored_data_version() @@ -133,10 +153,15 @@ pub fn import_legacy_settings( }; let network = settings.network; + let mut guard = prepare_import_guard(app_kv, &settings)?; + write_import_guard(app_kv, &guard)?; app_kv .put(DetScope::Global, AppSettings::KV_KEY, &settings) .map_err(|source| SettingsImportError::Write { source })?; + guard.settings_written = true; + write_import_guard(app_kv, &guard)?; write_sentinel(app_kv)?; + clear_import_guard_best_effort(app_kv); tracing::info!( target = "migration::legacy_settings", @@ -156,13 +181,84 @@ struct SettingsImportSentinel { sha: String, } +/// Durable recovery record written before imported settings are made visible. +#[derive(Debug, Serialize, Deserialize)] +struct SettingsImportGuard { + settings: AppSettings, + previous_settings_bytes: Option>, + settings_written: bool, + version: String, +} + fn sentinel_present(app_kv: &DetKv) -> Result { app_kv .get::(DetScope::Global, SENTINEL_KEY) .map(|v| v.is_some()) + .map_err(|source| SettingsImportError::Read { source }) +} + +fn read_import_guard(app_kv: &DetKv) -> Result, SettingsImportError> { + app_kv + .get(DetScope::Global, IMPORT_GUARD_KEY) + .map_err(|source| SettingsImportError::Read { source }) +} + +fn finish_guarded_import( + app_kv: &DetKv, + mut guard: SettingsImportGuard, +) -> Result { + let existing = app_kv + .get::(DetScope::Global, AppSettings::KV_KEY) + .map_err(|source| SettingsImportError::Read { source })?; + let existing_bytes = existing.as_ref().map(encode_settings).transpose()?; + let outcome = if !guard.settings_written && existing_bytes == guard.previous_settings_bytes { + let network = guard.settings.network; + app_kv + .put(DetScope::Global, AppSettings::KV_KEY, &guard.settings) + .map_err(|source| SettingsImportError::Write { source })?; + SettingsImport::Imported { network } + } else { + SettingsImport::AlreadyDone + }; + guard.settings_written = true; + write_import_guard(app_kv, &guard)?; + write_sentinel(app_kv)?; + clear_import_guard_best_effort(app_kv); + Ok(outcome) +} + +fn prepare_import_guard( + app_kv: &DetKv, + settings: &AppSettings, +) -> Result { + let previous_settings_bytes = app_kv + .get::(DetScope::Global, AppSettings::KV_KEY) + .map_err(|source| SettingsImportError::Read { source })? + .as_ref() + .map(encode_settings) + .transpose()?; + Ok(SettingsImportGuard { + settings: settings.clone(), + previous_settings_bytes, + settings_written: false, + version: env!("CARGO_PKG_VERSION").to_string(), + }) +} + +fn write_import_guard( + app_kv: &DetKv, + guard: &SettingsImportGuard, +) -> Result<(), SettingsImportError> { + app_kv + .put(DetScope::Global, IMPORT_GUARD_KEY, guard) .map_err(|source| SettingsImportError::Write { source }) } +fn encode_settings(settings: &AppSettings) -> Result, SettingsImportError> { + bincode::serde::encode_to_vec(settings, bincode::config::standard()) + .map_err(|source| SettingsImportError::Encode { source }) +} + fn write_sentinel(app_kv: &DetKv) -> Result<(), SettingsImportError> { let sentinel = SettingsImportSentinel { sha: env!("CARGO_PKG_VERSION").to_string(), @@ -172,13 +268,78 @@ fn write_sentinel(app_kv: &DetKv) -> Result<(), SettingsImportError> { .map_err(|source| SettingsImportError::Write { source }) } +/// Retire a pending legacy import after the user explicitly confirms a +/// network and that choice has been persisted. +pub fn finish_after_explicit_network_selection(app_kv: &DetKv) -> Result<(), SettingsImportError> { + write_sentinel(app_kv)?; + clear_import_guard_best_effort(app_kv); + Ok(()) +} + +fn clear_import_guard_best_effort(app_kv: &DetKv) { + if let Err(error) = app_kv.delete(DetScope::Global, IMPORT_GUARD_KEY) { + tracing::warn!( + target = "migration::legacy_settings", + error = ?error, + "Completed settings import left its recovery guard in storage", + ); + } +} + #[cfg(test)] mod tests { use super::*; use crate::model::settings::{RootScreenType, ThemeMode}; use crate::wallet_backend::kv_test_support::InMemoryKv; + use platform_wallet_storage::{KvError, KvStore, ObjectId}; use std::sync::Arc; + #[derive(Default)] + struct FailKeyOnceKv { + inner: InMemoryKv, + fail_key: std::sync::Mutex>, + } + + impl FailKeyOnceKv { + fn fail_next_put(&self, key: &'static str) { + *self.fail_key.lock().unwrap() = Some(key); + } + } + + impl KvStore for FailKeyOnceKv { + fn get(&self, scope: &ObjectId, key: &str) -> Result>, KvError> { + self.inner.get(scope, key) + } + + fn put(&self, scope: &ObjectId, key: &str, value: &[u8]) -> Result<(), KvError> { + let should_fail = { + let mut fail_key = self.fail_key.lock().unwrap(); + if fail_key.as_deref() == Some(key) { + fail_key.take(); + true + } else { + false + } + }; + if should_fail { + return Err(KvError::LockPoisoned); + } + 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) + } + } + fn kv() -> DetKv { DetKv::from_store(Arc::new(InMemoryKv::default())) } @@ -359,6 +520,123 @@ mod tests { ); } + #[test] + fn sentinel_failure_does_not_reimport_over_newer_settings_on_next_launch() { + let dir = tempfile::tempdir().unwrap(); + let db = legacy_db(dir.path()); + let store = Arc::new(FailKeyOnceKv::default()); + let app_kv = DetKv::from_store(store.clone()); + app_kv + .put( + DetScope::Global, + AppSettings::KV_KEY, + &AppSettings::default(), + ) + .expect("seed stale default blob"); + store.fail_next_put(SENTINEL_KEY); + + assert!(matches!( + import_legacy_settings(&app_kv, &db), + Err(SettingsImportError::Write { .. }) + )); + assert_eq!(stored(&app_kv).unwrap().network, Network::Testnet); + + let chosen = AppSettings::default(); + app_kv + .put(DetScope::Global, AppSettings::KV_KEY, &chosen) + .expect("user restores the pre-import settings"); + + assert_eq!( + import_legacy_settings(&app_kv, &db).expect("next launch"), + SettingsImport::AlreadyDone + ); + assert_eq!( + stored(&app_kv).unwrap().network, + Network::Mainnet, + "a retry must not overwrite settings changed after the import", + ); + } + + #[test] + fn settings_write_failure_recovers_legacy_settings_on_next_launch() { + let dir = tempfile::tempdir().unwrap(); + let db = legacy_db(dir.path()); + let store = Arc::new(FailKeyOnceKv::default()); + let app_kv = DetKv::from_store(store.clone()); + app_kv + .put( + DetScope::Global, + AppSettings::KV_KEY, + &AppSettings::default(), + ) + .expect("seed stale default blob"); + store.fail_next_put(AppSettings::KV_KEY); + + assert!(matches!( + import_legacy_settings(&app_kv, &db), + Err(SettingsImportError::Write { .. }) + )); + assert_eq!(stored(&app_kv).unwrap().network, Network::Mainnet); + + assert_eq!( + import_legacy_settings(&app_kv, &db).expect("next launch"), + SettingsImport::Imported { + network: Network::Testnet + } + ); + assert_eq!( + stored(&app_kv).unwrap().network, + Network::Testnet, + "an unchanged stale blob must not suppress recovery", + ); + } + + #[test] + fn explicit_choice_retires_import_after_initial_guard_write_failure() { + let dir = tempfile::tempdir().unwrap(); + let db = legacy_db(dir.path()); + let store = Arc::new(FailKeyOnceKv::default()); + let app_kv = DetKv::from_store(store.clone()); + store.fail_next_put(IMPORT_GUARD_KEY); + + assert!(matches!( + import_legacy_settings(&app_kv, &db), + Err(SettingsImportError::Write { .. }) + )); + + let chosen = AppSettings { + network: Network::Regtest, + ..AppSettings::default() + }; + app_kv + .put(DetScope::Global, AppSettings::KV_KEY, &chosen) + .expect("persist explicit network choice"); + finish_after_explicit_network_selection(&app_kv).expect("retire stale legacy import"); + + assert_eq!( + import_legacy_settings(&app_kv, &db).expect("next launch"), + SettingsImport::AlreadyDone, + ); + assert_eq!(stored(&app_kv).unwrap().network, Network::Regtest); + } + + #[test] + fn successful_import_removes_the_recovery_guard() { + let dir = tempfile::tempdir().unwrap(); + let db = legacy_db(dir.path()); + let app_kv = kv(); + + import_legacy_settings(&app_kv, &db).expect("import"); + + assert!( + app_kv + .get::(DetScope::Global, IMPORT_GUARD_KEY) + .expect("read guard") + .is_none(), + "completed imports must not retain duplicate settings data", + ); + } + /// A fresh install has no legacy row. The import writes the sentinel so /// the probe does not repeat, and leaves the settings blob alone. #[test] diff --git a/src/backend_task/mod.rs b/src/backend_task/mod.rs index bd62d51dc..e096c9b73 100644 --- a/src/backend_task/mod.rs +++ b/src/backend_task/mod.rs @@ -300,6 +300,10 @@ pub enum BackendTaskContext { TokenRewardEstimate(IdentityTokenIdentifier), /// The destructive per-network database clear. ClearNetworkDatabase, + /// A scheduled-vote sweep for one network. + ScheduledVoteSweep { network: Network }, + /// Receive-address derivation for one wallet's deposit flow. + GenerateReceiveAddress { seed_hash: WalletSeedHash }, /// A known backend task that needs no finer UI correlation. Other, /// An error emitted without an originating backend task. @@ -339,6 +343,13 @@ impl BackendTaskContext { ) ) } + + pub(crate) fn generated_receive_address_wallet(&self) -> Option { + match self.operation() { + Self::GenerateReceiveAddress { seed_hash } => Some(*seed_hash), + _ => None, + } + } } impl From<&BackendTask> for BackendTaskContext { @@ -369,6 +380,11 @@ impl From<&BackendTask> for BackendTaskContext { _ => Self::Other, }, BackendTask::SystemTask(SystemTask::ClearNetworkDatabase) => Self::ClearNetworkDatabase, + BackendTask::WalletTask(WalletTask::GenerateReceiveAddress { seed_hash }) => { + Self::GenerateReceiveAddress { + seed_hash: *seed_hash, + } + } _ => Self::Other, } } @@ -619,6 +635,9 @@ pub enum BackendTaskSuccessResult { ClaimedTokens(FeeResult), UpdatedTokenConfig(String, FeeResult), // The config item that was updated FetchedTokenBalances, + /// A requested foreground refresh found the upstream balance sync already + /// running, so no second pass was started. + TokenBalanceRefreshAlreadyInFlight, SavedToken, // Identity operation results (replacing string messages) @@ -1767,4 +1786,20 @@ mod tests { &TaskError::WalletStorageNotReady )); } + + #[test] + fn receive_address_context_retains_the_wallet_identity() { + let seed_hash = [7; 32]; + let task = BackendTask::WalletTask(WalletTask::GenerateReceiveAddress { seed_hash }); + let context = BackendTaskContext::from(&task); + + assert_eq!(context.generated_receive_address_wallet(), Some(seed_hash)); + assert_eq!( + BackendTaskContext::from(&BackendTask::WalletTask( + WalletTask::ListTrackedAssetLocks { seed_hash }, + )) + .generated_receive_address_wallet(), + None, + ); + } } diff --git a/src/backend_task/tokens/query_my_token_balances.rs b/src/backend_task/tokens/query_my_token_balances.rs index 57393bce6..93feb7da3 100644 --- a/src/backend_task/tokens/query_my_token_balances.rs +++ b/src/backend_task/tokens/query_my_token_balances.rs @@ -73,7 +73,7 @@ impl AppContext { let refresh_guard = self.begin_token_balance_refresh()?; let context = Arc::clone(self); - await_managed_network_request_with_timeout( + let outcome = await_managed_network_request_with_timeout( self.subtasks.clone(), "token_balance_refresh_reaper", NETWORK_REQUEST_TIMEOUT, @@ -84,12 +84,14 @@ impl AppContext { |source| TaskError::TokenBalanceRefreshTimeout { source }, ) .await??; - sender - .send(TaskResult::Refresh) - .await - .map_err(|_| TaskError::InternalSendError)?; - - Ok(BackendTaskSuccessResult::FetchedTokenBalances) + let result = Self::token_balance_sync_result(outcome); + if matches!(result, BackendTaskSuccessResult::FetchedTokenBalances) { + sender + .send(TaskResult::Refresh) + .await + .map_err(|_| TaskError::InternalSendError)?; + } + Ok(result) } pub async fn query_token_balance( @@ -108,7 +110,7 @@ impl AppContext { let watch_sets = self.token_watch_sets(vec![pair.identity_id])?; let refresh_guard = self.begin_token_balance_refresh()?; let context = Arc::clone(self); - await_managed_network_request_with_timeout( + let outcome = await_managed_network_request_with_timeout( self.subtasks.clone(), "token_balance_refresh_reaper", NETWORK_REQUEST_TIMEOUT, @@ -119,12 +121,14 @@ impl AppContext { |source| TaskError::TokenBalanceRefreshTimeout { source }, ) .await??; - sender - .send(TaskResult::Refresh) - .await - .map_err(|_| TaskError::InternalSendError)?; - - Ok(BackendTaskSuccessResult::FetchedTokenBalances) + let result = Self::token_balance_sync_result(outcome); + if matches!(result, BackendTaskSuccessResult::FetchedTokenBalances) { + sender + .send(TaskResult::Refresh) + .await + .map_err(|_| TaskError::InternalSendError)?; + } + Ok(result) } /// Stop tracking one identity-token balance. Un-watches the pair in the @@ -226,22 +230,22 @@ impl AppContext { async fn refresh_upstream_token_balances( &self, watch_sets: Vec<(Identifier, Vec)>, - ) -> Result<(), TaskError> { + ) -> Result { let backend = self.wallet_backend()?; for (identity_id, token_ids) in watch_sets { backend .register_identity_tokens(identity_id, token_ids) .await; } - Self::require_confirmed_token_balance_sync(backend.sync_token_balances_now().await) + Ok(backend.sync_token_balances_now().await) } - fn require_confirmed_token_balance_sync( - outcome: TokenBalanceSyncOutcome, - ) -> Result<(), TaskError> { + fn token_balance_sync_result(outcome: TokenBalanceSyncOutcome) -> BackendTaskSuccessResult { match outcome { - TokenBalanceSyncOutcome::Performed => Ok(()), - TokenBalanceSyncOutcome::AlreadyInFlight => Err(TaskError::TokenBalanceRefreshSkipped), + TokenBalanceSyncOutcome::Performed => BackendTaskSuccessResult::FetchedTokenBalances, + TokenBalanceSyncOutcome::AlreadyInFlight => { + BackendTaskSuccessResult::TokenBalanceRefreshAlreadyInFlight + } } } } @@ -437,11 +441,13 @@ mod tests { .await .expect("the expired lease must allow a retry attempt"); - let result = AppContext::require_confirmed_token_balance_sync( - TokenBalanceSyncOutcome::AlreadyInFlight, - ); + let result = + AppContext::token_balance_sync_result(TokenBalanceSyncOutcome::AlreadyInFlight); - assert!(matches!(result, Err(TaskError::TokenBalanceRefreshSkipped))); + assert!(matches!( + result, + BackendTaskSuccessResult::TokenBalanceRefreshAlreadyInFlight + )); drop(retry_guard); } diff --git a/src/context/wallet_lifecycle/tests.rs b/src/context/wallet_lifecycle/tests.rs index 516bf7127..84be2fc0e 100644 --- a/src/context/wallet_lifecycle/tests.rs +++ b/src/context/wallet_lifecycle/tests.rs @@ -3571,10 +3571,10 @@ async fn ensure_identity_funding_accounts_succeeds_on_cold_booted_watch_only_wal backend2.shutdown().await; } -/// Persisted Core transactions must populate DET's display snapshot during a -/// seedless cold boot, before any live wallet event can replay them. +/// A corrupt persisted Core transaction must not hide the wallet or prevent +/// valid history from populating its first seedless cold-boot snapshot. #[tokio::test(flavor = "multi_thread", worker_threads = 2)] -async fn cold_boot_hydrates_persisted_transaction_history_without_live_events() { +async fn cold_boot_keeps_wallet_visible_when_persisted_transaction_txid_is_corrupt() { use dash_sdk::dpp::dashcore::hashes::Hash; use dash_sdk::dpp::dashcore::{BlockHash, Transaction}; use dash_sdk::dpp::key_wallet::account::{AccountType, StandardAccountType}; @@ -3669,17 +3669,49 @@ async fn cold_boot_hydrates_persisted_transaction_history_without_live_events() .expect("flush transaction record"); drop(persister); + let connection = rusqlite::Connection::open(&persister_path) + .expect("open upstream persister for corruption fixture"); + connection + .execute( + "INSERT INTO core_transactions \ + (wallet_id, txid, height, block_hash, block_time, finalized, record_blob) \ + SELECT wallet_id, X'A1', height, block_hash, block_time, finalized, record_blob \ + FROM core_transactions WHERE wallet_id = ?1 LIMIT 1", + [wallet_id.as_slice()], + ) + .expect("insert invalid-width transaction id"); + drop(connection); + let (ctx, sender) = offline_testnet_context_at(cold_dir.path()); ctx.ensure_wallet_backend(sender) .await - .expect("wire cold-boot backend"); + .expect("corrupt history must not prevent cold-boot registration"); let backend = ctx.wallet_backend().expect("cold-boot backend"); let history = backend.transaction_history(&seed_hash); - assert_eq!(history.len(), 1, "persisted history must load at cold boot"); + assert!( + backend.is_wallet_registered(&seed_hash), + "the wallet must remain registered when one history row is corrupt" + ); + assert!( + backend.has_snapshot(&seed_hash), + "the wallet must remain visible when one history row is corrupt" + ); + assert_eq!( + history.len(), + 1, + "the corrupt row must be skipped without dropping valid history" + ); assert_eq!(history[0].txid, expected_txid); assert_eq!(history[0].timestamp, u64::from(timestamp)); assert_eq!(history[0].net_amount, 250_000); + assert!(matches!( + backend.transaction_history_status(&seed_hash), + crate::wallet_backend::TransactionHistoryStatus::Partial { + skipped_rows: 1, + .. + } + )); backend.shutdown().await; } diff --git a/src/model/settings.rs b/src/model/settings.rs index 1b004f6b5..f0369df35 100644 --- a/src/model/settings.rs +++ b/src/model/settings.rs @@ -417,6 +417,10 @@ pub(crate) fn detect_dash_qt_path() -> Option { mod tests { use super::*; + /// Captured from the pre-change wire struct with live `core_backend_mode = 0`. + const PRE_CHANGE_APP_SETTINGS_WIRE: &[u8] = + b"\x07testnet\x03\x01\x0c/opt/dash-qt\x00\x01\x04Dark\x00\x01\x01\x08Beginner\x00\x01"; + /// S1: verify the `AppSettings` defaults for a fresh install (no blob in /// k/v yet). `auto_start_spv` intentionally differs from the old DB column /// default (0/false): new installs sync without a manual step; existing @@ -518,25 +522,11 @@ mod tests { /// bincode format and corrupt already-stored `det:settings:v1` blobs. #[test] fn reserved_core_backend_mode_byte_preserves_wire_layout() { - let wire = AppSettingsWire { - network: "testnet".to_string(), - root_screen_type: 3, - dash_qt_path: Some("/opt/dash-qt".to_string()), - overwrite_dash_conf: false, - disable_zmq: true, - theme_mode: "Dark".to_string(), - _reserved_core_backend_mode: 0, // legacy "RPC" value from an old blob - onboarding_completed: true, - show_evonode_tools: true, - user_mode: "Beginner".to_string(), - close_dash_qt_on_exit: false, - auto_start_spv: true, - }; - let encoded = - bincode::serde::encode_to_vec(&wire, bincode::config::standard()).expect("encode"); - let (decoded, _): (AppSettings, _) = - bincode::serde::decode_from_slice(&encoded, bincode::config::standard()) - .expect("decode"); + let (decoded, _) = bincode::serde::decode_from_slice::( + PRE_CHANGE_APP_SETTINGS_WIRE, + bincode::config::standard(), + ) + .expect("the complete legacy settings blob must decode"); // Fields after the reserved byte must be read from the correct // offset — a shifted layout would scramble these. assert!(decoded.onboarding_completed); @@ -550,6 +540,20 @@ mod tests { assert!(matches!(decoded.theme_mode, ThemeMode::Dark)); } + #[test] + fn truncated_settings_blob_is_rejected() { + let truncated = &PRE_CHANGE_APP_SETTINGS_WIRE[..PRE_CHANGE_APP_SETTINGS_WIRE.len() - 1]; + + assert!( + bincode::serde::decode_from_slice::( + truncated, + bincode::config::standard(), + ) + .is_err(), + "boot must distinguish a corrupt saved preference from a fresh install", + ); + } + /// S3: legacy "dash" network value (used by databases predating the /// `Network::Dash` → `Network::Mainnet` rename) decodes to Mainnet /// instead of failing or coercing to a different network. diff --git a/src/ui/components/message_banner.rs b/src/ui/components/message_banner.rs index ed8bcf2e8..99c21443f 100644 --- a/src/ui/components/message_banner.rs +++ b/src/ui/components/message_banner.rs @@ -360,6 +360,14 @@ impl MessageBanner { self } + /// Attach structured diagnostic details to the current per-instance banner. + pub fn set_details(&mut self, details: impl fmt::Debug) -> &mut Self { + if let Some(state) = &mut self.state { + state.details = Some(format!("{details:?}")); + } + self + } + /// Clears the current message immediately. pub fn clear(&mut self) { self.state = None; diff --git a/src/ui/identities/add_new_identity_screen/by_receive_deposit.rs b/src/ui/identities/add_new_identity_screen/by_receive_deposit.rs index 02a451d9a..d8fcba71f 100644 --- a/src/ui/identities/add_new_identity_screen/by_receive_deposit.rs +++ b/src/ui/identities/add_new_identity_screen/by_receive_deposit.rs @@ -5,6 +5,7 @@ use crate::ui::components::MessageBanner; use crate::ui::identities::add_new_identity_screen::AddNewIdentityScreen; use crate::ui::identities::funding_common::{ FundingMethod, WalletFundedScreenStep, generate_qr_code_image, round_up_dash_4dp, + should_queue_funding_address, snapshot_deposit_outcome, }; use crate::ui::theme::DashColors; use crate::wallet_backend::poison::RwLockRecover; @@ -25,10 +26,12 @@ impl AddNewIdentityScreen { /// flight, or a prior derivation failed. Idempotent, so it is safe to call /// every frame from the QR view. fn queue_funding_address_request(&mut self) { - if self.funding_address.is_some() - || self.pending_funding_address_request.is_some() - || self.funding_address_request_failed - { + if !should_queue_funding_address( + self.funding_address.is_some(), + self.pending_funding_address_request.is_some(), + self.funding_address_request_in_flight, + self.funding_address_request_failed, + ) { return; } if let Some(wallet) = &self.selected_wallet @@ -88,13 +91,12 @@ impl AddNewIdentityScreen { }); ui.add_space(8.0); - // Show what has arrived at THIS address specifically (accumulated per - // deposit), never whole-wallet balance — leftover change elsewhere must - // not read as progress toward this deposit. - let received = self.received_at_funding_address_duffs; + // Show only funds currently available at this address. Unrelated wallet + // funds must never read as progress toward this deposit. + let received = self.funding_address_balance_duffs; if received > 0 { ui.label(format!( - "Received {received_amount} at this address so far. Waiting for at least \ + "This address has {received_amount} available. Waiting for at least \ {minimum_amount}.", received_amount = Amount::dash_from_duffs(received), )); @@ -106,12 +108,42 @@ impl AddNewIdentityScreen { } } + fn reconcile_funding_deposit(&mut self) { + let Some(address) = self.funding_address.as_ref() else { + return; + }; + let Some(seed_hash) = self + .selected_wallet + .as_ref() + .and_then(|wallet| wallet.read().ok().map(|wallet| wallet.seed_hash())) + else { + return; + }; + let address_balance_duffs = self + .app_context + .snapshot_address_balances(&seed_hash) + .get(address) + .copied() + .unwrap_or(0); + self.funding_address_balance_duffs = address_balance_duffs; + + let minimum_credits = self.deposit_minimum_credits(); + let current_step = *self.step.read_recover(); + let (next_step, prefill) = + snapshot_deposit_outcome(current_step, address_balance_duffs, minimum_credits); + if prefill.is_some() { + self.prefill_funding_amount = true; + *self.step.write_recover() = next_step; + } + } + /// Render the "Receive a new deposit" funding method: a scannable deposit /// address while waiting, then an editable amount and Create button once the /// deposit arrives. A "Choose a different funding method" affordance is /// present throughout so the user is never trapped. pub fn render_ui_by_receive_deposit(&mut self, ui: &mut Ui, step_number: u32) -> AppAction { let mut action = AppAction::None; + self.reconcile_funding_deposit(); let step = *self.step.read_recover(); if step == WalletFundedScreenStep::WaitingOnFunds { diff --git a/src/ui/identities/add_new_identity_screen/mod.rs b/src/ui/identities/add_new_identity_screen/mod.rs index 774cc0cdc..2d6948ee3 100644 --- a/src/ui/identities/add_new_identity_screen/mod.rs +++ b/src/ui/identities/add_new_identity_screen/mod.rs @@ -12,7 +12,7 @@ use crate::backend_task::identity::{ RegisterIdentityFundingMethod, default_identity_key_specs, }; use crate::backend_task::wallet::WalletTask; -use crate::backend_task::{BackendTask, BackendTaskSuccessResult, FeeResult}; +use crate::backend_task::{BackendTask, BackendTaskContext, BackendTaskSuccessResult, FeeResult}; use crate::context::AppContext; use crate::model::fee_estimation::format_credits_as_dash; use crate::model::secret::Secret; @@ -27,8 +27,8 @@ use crate::ui::components::wallet_unlock_popup::{ }; use crate::ui::identities::funding_common::{ FundingMethod, WalletFundedScreenStep, default_funding_state, deposit_event_outcome, - deposit_matches, funding_method_after_switch, max_amount_after_fee_reserve, - spendable_covers_minimum, wallet_selection_combo, + funding_method_after_switch, max_amount_after_fee_reserve, spendable_covers_minimum, + step_after_task_failure, wallet_selection_combo, }; use crate::ui::state::TrackedAssetLockCache; use crate::ui::theme::DashColors; @@ -36,6 +36,7 @@ use crate::ui::{MessageType, ScreenLike}; use crate::wallet_backend::poison::RwLockRecover; use dash_sdk::dashcore_rpc::dashcore::Address; use dash_sdk::dashcore_rpc::dashcore::transaction::special_transaction::TransactionPayload; +use dash_sdk::dpp::balances::credits::CREDITS_PER_DUFF; use dash_sdk::dpp::dashcore::OutPoint; use dash_sdk::dpp::data_contract::accessors::v0::DataContractV0Getters; use dash_sdk::dpp::identity::accessors::IdentityGettersV0; @@ -80,13 +81,14 @@ pub struct AddNewIdentityScreen { /// method. Set when the QR view needs an address; drained at the end of /// `ui()` into a [`WalletTask::GenerateReceiveAddress`] task. pending_funding_address_request: Option, - /// Set when a derived deposit address could not be parsed, so the QR view - /// stops auto-retrying and offers a manual retry instead of spinning forever. + /// True after the queued receive-address request is dispatched and until + /// its correlated success or failure result returns. + funding_address_request_in_flight: bool, + /// Set when deposit-address generation or parsing fails, so the QR view + /// offers a manual retry instead of spinning forever. funding_address_request_failed: bool, - /// Duffs received so far at `funding_address` (accumulated per deposit - /// event), for the "received so far" line — a per-address running total, not - /// whole-wallet balance. - received_at_funding_address_duffs: u64, + /// Spendable duffs currently held at the address shown by the deposit flow. + funding_address_balance_duffs: u64, /// Set on the transition to `FundsReceived` so the amount field pre-fills /// the fee-reserve-capped received balance on the next render. prefill_funding_amount: bool, @@ -181,8 +183,9 @@ impl AddNewIdentityScreen { selected_wallet: None, // updated later funding_address: None, pending_funding_address_request: None, + funding_address_request_in_flight: false, funding_address_request_failed: false, - received_at_funding_address_duffs: 0, + funding_address_balance_duffs: 0, prefill_funding_amount: false, funding_method: Arc::new(RwLock::new(FundingMethod::NoSelection)), user_chose_funding_method: false, @@ -445,8 +448,9 @@ impl AddNewIdentityScreen { // wallet; `update_wallet` re-derives the funding method/step. self.funding_address = None; self.pending_funding_address_request = None; + self.funding_address_request_in_flight = false; self.funding_address_request_failed = false; - self.received_at_funding_address_duffs = 0; + self.funding_address_balance_duffs = 0; self.prefill_funding_amount = false; self.funding_asset_lock = None; self.copied_to_clipboard = None; @@ -656,8 +660,9 @@ impl AddNewIdentityScreen { } self.funding_address = None; self.pending_funding_address_request = None; + self.funding_address_request_in_flight = false; self.funding_address_request_failed = false; - self.received_at_funding_address_duffs = 0; + self.funding_address_balance_duffs = 0; self.prefill_funding_amount = false; self.funding_amount = None; self.funding_amount_input = None; @@ -679,8 +684,9 @@ impl AddNewIdentityScreen { self.user_chose_funding_method = false; self.funding_address = None; self.pending_funding_address_request = None; + self.funding_address_request_in_flight = false; self.funding_address_request_failed = false; - self.received_at_funding_address_duffs = 0; + self.funding_address_balance_duffs = 0; self.prefill_funding_amount = false; self.funding_amount = None; self.funding_amount_input = None; @@ -1048,6 +1054,25 @@ impl AddNewIdentityScreen { if amount == 0 { return AppAction::None; } + if funding_method == FundingMethod::ReceiveDeposit { + let key_count = self.identity_keys.others.len() + 1; + let fee_credits = self + .app_context + .fee_estimator() + .estimate_identity_create(key_count); + let available_credits = max_amount_after_fee_reserve( + self.funding_address_balance_duffs, + fee_credits, + ); + if amount.saturating_mul(CREDITS_PER_DUFF) > available_credits { + MessageBanner::set_global( + self.app_context.egui_ctx(), + "That deposit cannot cover this amount. Wait for more funds or choose a smaller amount.", + MessageType::Warning, + ); + return AppAction::None; + } + } let wallet_seed_hash = hex::encode(selected_wallet.read_recover().seed_hash()); tracing::debug!(wallet_seed_hash, "funding with wallet balance"); @@ -1130,16 +1155,19 @@ impl AddNewIdentityScreen { funding_method, FundingMethod::UseWalletBalance | FundingMethod::ReceiveDeposit ) { - let spendable_duffs = self - .selected_wallet - .as_ref() - .and_then(|wallet| wallet.read().ok()) - .map(|wallet| { - self.app_context - .snapshot_balance(&wallet.seed_hash()) - .spendable() - }) - .unwrap_or(0); + let spendable_duffs = if funding_method == FundingMethod::ReceiveDeposit { + self.funding_address_balance_duffs + } else { + self.selected_wallet + .as_ref() + .and_then(|wallet| wallet.read().ok()) + .map(|wallet| { + self.app_context + .snapshot_balance(&wallet.seed_hash()) + .spendable() + }) + .unwrap_or(0) + }; let key_count = self.identity_keys.others.len() + 1; // +1 for master key let estimated_fee = self .app_context @@ -1151,8 +1179,8 @@ impl AddNewIdentityScreen { Some(max_with_fee_reserved), true, Some(format!( - "~{} reserved for fees", - format_credits_as_dash(estimated_fee) + "The estimated fee reserves about {}.", + format_credits_as_dash(estimated_fee), )), ) } else { @@ -1300,10 +1328,21 @@ impl AddNewIdentityScreen { impl ScreenLike for AddNewIdentityScreen { fn display_message(&mut self, _message: &str, message_type: MessageType) { if matches!(message_type, MessageType::Error | MessageType::Warning) { - // Reset step so we stop showing "Waiting for Platform acknowledgement". - // The error itself is displayed by the global MessageBanner. let mut step = self.step.write_recover(); - *step = WalletFundedScreenStep::ReadyToCreate; + *step = step_after_task_failure(*step); + } + } + + fn display_backend_task_error(&mut self, context: &BackendTaskContext, _error: &TaskError) { + let selected_seed_hash = self + .selected_wallet + .as_ref() + .and_then(|wallet| wallet.read().ok().map(|wallet| wallet.seed_hash())); + if self.funding_address_request_in_flight + && context.generated_receive_address_wallet() == selected_seed_hash + { + self.funding_address_request_in_flight = false; + self.funding_address_request_failed = true; } } fn display_task_result(&mut self, backend_task_success_result: BackendTaskSuccessResult) { @@ -1339,6 +1378,7 @@ impl ScreenLike for AddNewIdentityScreen { .map(|w| w.seed_hash() == *seed_hash) .unwrap_or(false); if is_ours { + self.funding_address_request_in_flight = false; match address.parse::>() { Ok(addr) => { self.funding_address = Some(addr.assume_checked()); @@ -1380,23 +1420,6 @@ impl ScreenLike for AddNewIdentityScreen { CoreItem::ReceivedAvailableUTXOTransaction(_, outputs), ) = &backend_task_success_result { - // Accumulate what this deposit added at the shown address, so - // the "received so far" line tracks the deposit itself, not - // whole-wallet balance. - self.received_at_funding_address_duffs = self - .received_at_funding_address_duffs - .saturating_add(deposit_matches(self.funding_address.as_ref(), outputs)); - - let spendable_duffs = self - .selected_wallet - .as_ref() - .and_then(|w| w.read().ok()) - .map(|w| { - self.app_context - .snapshot_balance(&w.seed_hash()) - .spendable() - }) - .unwrap_or(0); let key_count = self.identity_keys.others.len() + 1; // +1 for master key let minimum_credits = self .app_context @@ -1406,7 +1429,6 @@ impl ScreenLike for AddNewIdentityScreen { current_step, self.funding_address.as_ref(), outputs, - spendable_duffs, minimum_credits, ); // Pre-fill the amount with the fee-reserve-capped balance when @@ -1733,15 +1755,17 @@ impl ScreenLike for AddNewIdentityScreen { // Derive the "Receive a new deposit" address off the UI thread; the QR // view queues this when it has no address yet. if let Some(seed_hash) = self.pending_funding_address_request.take() { + self.funding_address_request_in_flight = true; pending_tasks.push(BackendTask::WalletTask( WalletTask::GenerateReceiveAddress { seed_hash }, )); } - match pending_tasks.len() { - 0 => {} - 1 => action |= AppAction::BackendTask(pending_tasks.pop().expect("len == 1")), - _ => { + match pending_tasks.pop() { + None => {} + Some(task) if pending_tasks.is_empty() => action |= AppAction::BackendTask(task), + Some(task) => { + pending_tasks.push(task); action |= AppAction::BackendTasks(pending_tasks, BackendTasksExecutionMode::Concurrent) } diff --git a/src/ui/identities/funding_common.rs b/src/ui/identities/funding_common.rs index b888c1bae..bc80a9117 100644 --- a/src/ui/identities/funding_common.rs +++ b/src/ui/identities/funding_common.rs @@ -105,6 +105,48 @@ pub fn spendable_covers_minimum(spendable_duffs: u64, minimum_credits: u64) -> b spendable_duffs.saturating_mul(CREDITS_PER_DUFF) >= minimum_credits } +/// Resolve a polling snapshot for the address shown by the deposit flow. +/// Advancement and prefill are both bounded by funds at that address, never by +/// unrelated spendable funds elsewhere in the wallet. +pub fn snapshot_deposit_outcome( + current_step: WalletFundedScreenStep, + address_balance_duffs: u64, + minimum_credits: u64, +) -> (WalletFundedScreenStep, Option) { + let advance = current_step == WalletFundedScreenStep::WaitingOnFunds + && spendable_covers_minimum(address_balance_duffs, minimum_credits); + let next_step = if advance { + WalletFundedScreenStep::FundsReceived + } else { + current_step + }; + let prefill = + advance.then(|| max_amount_after_fee_reserve(address_balance_duffs, minimum_credits)); + (next_step, prefill) +} + +/// Whether the QR flow should dispatch a new receive-address request. +pub fn should_queue_funding_address( + has_address: bool, + request_pending: bool, + request_in_flight: bool, + request_failed: bool, +) -> bool { + !has_address && !request_pending && !request_in_flight && !request_failed +} + +/// Restore an amount-entry step after a failed funding task without moving a +/// deposit-address failure away from its retry view. +pub fn step_after_task_failure(current_step: WalletFundedScreenStep) -> WalletFundedScreenStep { + match current_step { + WalletFundedScreenStep::WaitingForAssetLock + | WalletFundedScreenStep::WaitingForPlatformAcceptance => { + WalletFundedScreenStep::ReadyToCreate + } + _ => current_step, + } +} + /// The largest amount, in credits, a "Max" button can safely offer from a /// wallet holding `spendable_duffs`, after reserving `fee_credits` for the /// platform fee. Built on `spendable_duffs` (not the wallet's `total`, which @@ -129,8 +171,7 @@ pub fn round_up_dash_4dp(dash: f64) -> f64 { /// address contributes nothing. Returns `0` when no address is shown yet. /// /// This decides only whether *this* event touched the shown address; the -/// cumulative "received so far" figure comes from the wallet's spendable -/// snapshot, since deposits across separate events accumulate there. +/// cumulative available amount comes from that address's UTXO snapshot. pub fn deposit_matches( funding_address: Option<&Address>, outputs: &[(OutPoint, TxOut, Address)], @@ -145,23 +186,20 @@ pub fn deposit_matches( /// Next funding step after a received-UTXO event arrives while awaiting a /// deposit. Advances to [`WalletFundedScreenStep::FundsReceived`] only when the -/// deposit landed on the shown `funding_address` AND the wallet's cumulative -/// `spendable_duffs` now covers `minimum_credits`; otherwise the step is left -/// unchanged. The step guard means a matching deposit seen while another method -/// is active never forces an advance. +/// outputs in this event paid enough to the shown `funding_address` to cover +/// `minimum_credits`; otherwise the step is left unchanged. The per-frame +/// snapshot reconciler handles totals accumulated across multiple events. The +/// step guard prevents another funding method from advancing spuriously. pub fn deposit_step_after_utxo( current_step: WalletFundedScreenStep, funding_address: Option<&Address>, outputs: &[(OutPoint, TxOut, Address)], - spendable_duffs: u64, minimum_credits: u64, ) -> WalletFundedScreenStep { if current_step != WalletFundedScreenStep::WaitingOnFunds { return current_step; } - if deposit_matches(funding_address, outputs) > 0 - && spendable_covers_minimum(spendable_duffs, minimum_credits) - { + if spendable_covers_minimum(deposit_matches(funding_address, outputs), minimum_credits) { WalletFundedScreenStep::FundsReceived } else { current_step @@ -178,18 +216,12 @@ pub fn deposit_event_outcome( current_step: WalletFundedScreenStep, funding_address: Option<&Address>, outputs: &[(OutPoint, TxOut, Address)], - spendable_duffs: u64, fee_credits: u64, ) -> (WalletFundedScreenStep, Option) { - let next_step = deposit_step_after_utxo( - current_step, - funding_address, - outputs, - spendable_duffs, - fee_credits, - ); + let deposited_duffs = deposit_matches(funding_address, outputs); + let next_step = deposit_step_after_utxo(current_step, funding_address, outputs, fee_credits); let prefill_credits = (next_step == WalletFundedScreenStep::FundsReceived) - .then(|| max_amount_after_fee_reserve(spendable_duffs, fee_credits)); + .then(|| max_amount_after_fee_reserve(deposited_duffs, fee_credits)); (next_step, prefill_credits) } @@ -633,7 +665,7 @@ mod tests { assert_eq!(deposit_matches(None, &outputs), 0); } - /// TC-QRFUND-04: a matching deposit that lifts spendable to the minimum + /// TC-QRFUND-04: a matching deposit that covers the minimum /// advances the wizard to the amount step. #[test] fn deposit_step_advances_when_matched_and_minimum_covered() { @@ -645,7 +677,6 @@ mod tests { WalletFundedScreenStep::WaitingOnFunds, Some(&shown), &outputs, - 100, // spendable duffs minimum_credits, ), WalletFundedScreenStep::FundsReceived @@ -657,14 +688,13 @@ mod tests { #[test] fn deposit_step_stays_waiting_below_minimum() { let shown = addr(1); - let outputs = [output(40_000, &shown)]; + let outputs = [output(40, &shown)]; let minimum_credits = 100 * CREDITS_PER_DUFF; assert_eq!( deposit_step_after_utxo( WalletFundedScreenStep::WaitingOnFunds, Some(&shown), &outputs, - 40, // spendable duffs, below the 100-duff minimum minimum_credits, ), WalletFundedScreenStep::WaitingOnFunds @@ -672,7 +702,7 @@ mod tests { } /// TC-QRFUND-06: a deposit to a different address never advances the wizard, - /// even when spendable happens to cover the minimum. + /// even when unrelated wallet funds happen to cover the minimum. #[test] fn deposit_step_stays_waiting_for_other_address() { let shown = addr(1); @@ -684,7 +714,6 @@ mod tests { WalletFundedScreenStep::WaitingOnFunds, Some(&shown), &outputs, - 100, minimum_credits, ), WalletFundedScreenStep::WaitingOnFunds @@ -706,7 +735,7 @@ mod tests { WalletFundedScreenStep::WaitingForAssetLock, ] { assert_eq!( - deposit_step_after_utxo(step, Some(&shown), &outputs, 100, minimum_credits), + deposit_step_after_utxo(step, Some(&shown), &outputs, minimum_credits), step, "guard must not change step {step:?}" ); @@ -726,7 +755,6 @@ mod tests { WalletFundedScreenStep::WaitingOnFunds, Some(&shown), &outputs, - 100_000, // spendable duffs, well above the fee fee_credits, ); assert_eq!(next, WalletFundedScreenStep::FundsReceived); @@ -745,19 +773,53 @@ mod tests { #[test] fn below_minimum_deposit_yields_no_prefill() { let shown = addr(1); - let outputs = [output(40_000, &shown)]; + let outputs = [output(40, &shown)]; let fee_credits = 100 * CREDITS_PER_DUFF; let (next, prefill) = deposit_event_outcome( WalletFundedScreenStep::WaitingOnFunds, Some(&shown), &outputs, - 40, // below the 100-duff minimum fee_credits, ); assert_eq!(next, WalletFundedScreenStep::WaitingOnFunds); assert_eq!(prefill, None); } + #[test] + fn unrelated_wallet_balance_does_not_complete_a_partial_deposit() { + let shown = addr(1); + let outputs = [output(1, &shown)]; + let minimum_credits = 50_000_000; + + assert_eq!( + snapshot_deposit_outcome(WalletFundedScreenStep::WaitingOnFunds, 1, minimum_credits,), + (WalletFundedScreenStep::WaitingOnFunds, None), + ); + assert_eq!( + deposit_event_outcome( + WalletFundedScreenStep::WaitingOnFunds, + Some(&shown), + &outputs, + minimum_credits, + ), + (WalletFundedScreenStep::WaitingOnFunds, None), + ); + } + + #[test] + fn shared_address_request_and_failure_helpers_cover_both_identity_flows() { + assert!(!should_queue_funding_address(false, false, true, false)); + assert!(should_queue_funding_address(false, false, false, false)); + assert_eq!( + step_after_task_failure(WalletFundedScreenStep::WaitingOnFunds), + WalletFundedScreenStep::WaitingOnFunds, + ); + assert_eq!( + step_after_task_failure(WalletFundedScreenStep::WaitingForAssetLock), + WalletFundedScreenStep::ReadyToCreate, + ); + } + /// A deposit to a different address never advances, so it never pre-fills. #[test] fn deposit_to_other_address_yields_no_prefill() { @@ -769,7 +831,6 @@ mod tests { WalletFundedScreenStep::WaitingOnFunds, Some(&shown), &outputs, - 100_000, fee_credits, ); assert_eq!(next, WalletFundedScreenStep::WaitingOnFunds); diff --git a/src/ui/identities/top_up_identity_screen/by_receive_deposit.rs b/src/ui/identities/top_up_identity_screen/by_receive_deposit.rs index c20bbc38b..5624baca4 100644 --- a/src/ui/identities/top_up_identity_screen/by_receive_deposit.rs +++ b/src/ui/identities/top_up_identity_screen/by_receive_deposit.rs @@ -4,6 +4,7 @@ use crate::ui::MessageType; use crate::ui::components::MessageBanner; use crate::ui::identities::funding_common::{ FundingMethod, WalletFundedScreenStep, generate_qr_code_image, round_up_dash_4dp, + should_queue_funding_address, snapshot_deposit_outcome, }; use crate::ui::identities::top_up_identity_screen::TopUpIdentityScreen; use crate::ui::theme::DashColors; @@ -15,10 +16,12 @@ impl TopUpIdentityScreen { /// flight, or a prior derivation failed. Idempotent, so it is safe to call /// every frame from the QR view. fn queue_funding_address_request(&mut self) { - if self.funding_address.is_some() - || self.pending_funding_address_request.is_some() - || self.funding_address_request_failed - { + if !should_queue_funding_address( + self.funding_address.is_some(), + self.pending_funding_address_request.is_some(), + self.funding_address_request_in_flight, + self.funding_address_request_failed, + ) { return; } if let Some(wallet) = &self.wallet @@ -78,13 +81,12 @@ impl TopUpIdentityScreen { }); ui.add_space(8.0); - // Show what has arrived at THIS address specifically (accumulated per - // deposit), never whole-wallet balance — leftover change elsewhere must - // not read as progress toward this deposit. - let received = self.received_at_funding_address_duffs; + // Show only funds currently available at this address. Unrelated wallet + // funds must never read as progress toward this deposit. + let received = self.funding_address_balance_duffs; if received > 0 { ui.label(format!( - "Received {received_amount} at this address so far. Waiting for at least \ + "This address has {received_amount} available. Waiting for at least \ {minimum_amount}.", received_amount = Amount::dash_from_duffs(received), )); @@ -96,12 +98,42 @@ impl TopUpIdentityScreen { } } + fn reconcile_funding_deposit(&mut self) { + let Some(address) = self.funding_address.as_ref() else { + return; + }; + let Some(seed_hash) = self + .wallet + .as_ref() + .and_then(|wallet| wallet.read().ok().map(|wallet| wallet.seed_hash())) + else { + return; + }; + let address_balance_duffs = self + .app_context + .snapshot_address_balances(&seed_hash) + .get(address) + .copied() + .unwrap_or(0); + self.funding_address_balance_duffs = address_balance_duffs; + + let minimum_credits = self.app_context.fee_estimator().estimate_identity_topup(); + let current_step = self.current_step(); + let (next_step, prefill) = + snapshot_deposit_outcome(current_step, address_balance_duffs, minimum_credits); + if prefill.is_some() { + self.prefill_funding_amount = true; + self.set_step(next_step); + } + } + /// Render the "Receive a new deposit" funding method: a scannable deposit /// address while waiting, then an editable amount and Top Up button once the /// deposit arrives. A "Choose a different funding method" affordance is /// present throughout so the user is never trapped. pub fn render_ui_by_receive_deposit(&mut self, ui: &mut Ui, step_number: u32) -> AppAction { let mut action = AppAction::None; + self.reconcile_funding_deposit(); let step = self.current_step(); if step == WalletFundedScreenStep::WaitingOnFunds { diff --git a/src/ui/identities/top_up_identity_screen/mod.rs b/src/ui/identities/top_up_identity_screen/mod.rs index 500383180..8227cc52a 100644 --- a/src/ui/identities/top_up_identity_screen/mod.rs +++ b/src/ui/identities/top_up_identity_screen/mod.rs @@ -9,7 +9,7 @@ use crate::backend_task::core::CoreItem; use crate::backend_task::error::TaskError; use crate::backend_task::identity::{IdentityTask, IdentityTopUpInfo, TopUpIdentityFundingMethod}; use crate::backend_task::wallet::WalletTask; -use crate::backend_task::{BackendTask, BackendTaskSuccessResult, FeeResult}; +use crate::backend_task::{BackendTask, BackendTaskContext, BackendTaskSuccessResult, FeeResult}; use crate::context::AppContext; use crate::model::amount::Amount; use crate::model::fee_estimation::format_credits_as_dash; @@ -27,7 +27,7 @@ use crate::ui::components::wallet_unlock_popup::{ }; use crate::ui::identities::funding_common::{ FundingMethod, WalletFundedScreenStep, default_funding_state, deposit_event_outcome, - deposit_matches, max_amount_after_fee_reserve, spendable_covers_minimum, + max_amount_after_fee_reserve, spendable_covers_minimum, step_after_task_failure, wallet_selection_combo, }; use crate::ui::state::TrackedAssetLockCache; @@ -35,7 +35,7 @@ use crate::ui::{MessageType, ScreenLike}; use dash_sdk::dashcore_rpc::dashcore::Address; use dash_sdk::dashcore_rpc::dashcore::transaction::special_transaction::TransactionPayload; use dash_sdk::dpp::address_funds::PlatformAddress; -use dash_sdk::dpp::balances::credits::{Credits, Duffs}; +use dash_sdk::dpp::balances::credits::{CREDITS_PER_DUFF, Credits, Duffs}; use dash_sdk::dpp::dashcore::OutPoint; use dash_sdk::dpp::identity::accessors::IdentityGettersV0; use dash_sdk::dpp::platform_value::string_encoding::Encoding; @@ -46,6 +46,23 @@ use std::sync::{Arc, RwLock}; const WALLET_SELECTION_TOOLTIP: &str = "This wallet will provide the address for receiving funds \ and create the asset lock transaction to top up your identity."; +fn pending_backend_tasks_action( + mut lock_fetches: Vec, + funding_address_request: Option, +) -> AppAction { + if let Some(task) = funding_address_request { + lock_fetches.push(task); + } + match lock_fetches.pop() { + None => AppAction::None, + Some(task) if lock_fetches.is_empty() => AppAction::BackendTask(task), + Some(task) => { + lock_fetches.push(task); + AppAction::BackendTasks(lock_fetches, BackendTasksExecutionMode::Concurrent) + } + } +} + pub struct TopUpIdentityScreen { pub identity: QualifiedIdentity, step: Arc>, @@ -59,13 +76,14 @@ pub struct TopUpIdentityScreen { /// method. Set when the QR view needs an address; drained at the end of /// `ui()` into a [`WalletTask::GenerateReceiveAddress`] task. pending_funding_address_request: Option, - /// Set when a derived deposit address could not be parsed, so the QR view - /// stops auto-retrying and offers a manual retry instead of spinning forever. + /// True after the queued receive-address request is dispatched and until + /// its correlated success or failure result returns. + funding_address_request_in_flight: bool, + /// Set when deposit-address generation or parsing fails, so the QR view + /// offers a manual retry instead of spinning forever. funding_address_request_failed: bool, - /// Duffs received so far at `funding_address` (accumulated per deposit - /// event), for the "received so far" line — a per-address running total, not - /// whole-wallet balance. - received_at_funding_address_duffs: u64, + /// Spendable duffs currently held at the address shown by the deposit flow. + funding_address_balance_duffs: u64, /// Set on the transition to `FundsReceived` so the amount field pre-fills /// the fee-reserve-capped received balance on the next render. prefill_funding_amount: bool, @@ -99,8 +117,9 @@ impl TopUpIdentityScreen { wallet: None, funding_address: None, pending_funding_address_request: None, + funding_address_request_in_flight: false, funding_address_request_failed: false, - received_at_funding_address_duffs: 0, + funding_address_balance_duffs: 0, prefill_funding_amount: false, funding_method: Arc::new(RwLock::new(FundingMethod::NoSelection)), funding_amount: "".to_string(), @@ -247,8 +266,9 @@ impl TopUpIdentityScreen { self.wallet_open_attempted = false; self.funding_address = None; self.pending_funding_address_request = None; + self.funding_address_request_in_flight = false; self.funding_address_request_failed = false; - self.received_at_funding_address_duffs = 0; + self.funding_address_balance_duffs = 0; self.prefill_funding_amount = false; self.funding_asset_lock = None; self.funding_amount_input = None; @@ -286,8 +306,9 @@ impl TopUpIdentityScreen { self.set_step(step); self.funding_address = None; self.pending_funding_address_request = None; + self.funding_address_request_in_flight = false; self.funding_address_request_failed = false; - self.received_at_funding_address_duffs = 0; + self.funding_address_balance_duffs = 0; self.prefill_funding_amount = false; self.funding_amount_input = None; self.funding_amount_exact = None; @@ -396,8 +417,9 @@ impl TopUpIdentityScreen { self.set_step(WalletFundedScreenStep::WaitingOnFunds); self.funding_address = None; self.pending_funding_address_request = None; + self.funding_address_request_in_flight = false; self.funding_address_request_failed = false; - self.received_at_funding_address_duffs = 0; + self.funding_address_balance_duffs = 0; self.prefill_funding_amount = false; self.funding_amount_input = None; self.funding_amount_exact = None; @@ -452,6 +474,21 @@ impl TopUpIdentityScreen { if amount == 0 { return AppAction::None; } + if funding_method == FundingMethod::ReceiveDeposit { + let fee_credits = self.app_context.fee_estimator().estimate_identity_topup(); + let available_credits = max_amount_after_fee_reserve( + self.funding_address_balance_duffs, + fee_credits, + ); + if amount.saturating_mul(CREDITS_PER_DUFF) > available_credits { + MessageBanner::set_global( + self.app_context.egui_ctx(), + "That deposit cannot cover this amount. Wait for more funds or choose a smaller amount.", + MessageType::Warning, + ); + return AppAction::None; + } + } let identity_input = IdentityTopUpInfo { qualified_identity: self.identity.clone(), wallet: Arc::clone(selected_wallet), // Clone the Arc reference @@ -488,16 +525,19 @@ impl TopUpIdentityScreen { funding_method, FundingMethod::UseWalletBalance | FundingMethod::ReceiveDeposit ) { - let max_spendable_duffs = self - .wallet - .as_ref() - .and_then(|w| w.read().ok()) - .map(|w| { - self.app_context - .snapshot_balance(&w.seed_hash()) - .spendable() - }) - .unwrap_or(0); + let max_spendable_duffs = if funding_method == FundingMethod::ReceiveDeposit { + self.funding_address_balance_duffs + } else { + self.wallet + .as_ref() + .and_then(|w| w.read().ok()) + .map(|w| { + self.app_context + .snapshot_balance(&w.seed_hash()) + .spendable() + }) + .unwrap_or(0) + }; let fee_estimator = self.app_context.fee_estimator(); let estimated_fee = fee_estimator.estimate_identity_topup(); let max_with_fee_reserved = @@ -506,8 +546,8 @@ impl TopUpIdentityScreen { Some(max_with_fee_reserved), true, Some(format!( - "~{} reserved for fees", - format_credits_as_dash(estimated_fee) + "The estimated fee reserves about {}.", + format_credits_as_dash(estimated_fee), )), ) } else { @@ -558,13 +598,20 @@ impl ScreenLike for TopUpIdentityScreen { fn display_message(&mut self, _message: &str, message_type: MessageType) { // Banner display is handled globally by AppState; this is only for side-effects. if matches!(message_type, MessageType::Error | MessageType::Warning) { - // Reset step so UI is not stuck on waiting messages - let step = self.current_step(); - if step == WalletFundedScreenStep::WaitingForPlatformAcceptance - || step == WalletFundedScreenStep::WaitingForAssetLock - { - self.set_step(WalletFundedScreenStep::ReadyToCreate); - } + self.set_step(step_after_task_failure(self.current_step())); + } + } + + fn display_backend_task_error(&mut self, context: &BackendTaskContext, _error: &TaskError) { + let selected_seed_hash = self + .wallet + .as_ref() + .and_then(|wallet| wallet.read().ok().map(|wallet| wallet.seed_hash())); + if self.funding_address_request_in_flight + && context.generated_receive_address_wallet() == selected_seed_hash + { + self.funding_address_request_in_flight = false; + self.funding_address_request_failed = true; } } fn display_task_result(&mut self, backend_task_success_result: BackendTaskSuccessResult) { @@ -586,6 +633,7 @@ impl ScreenLike for TopUpIdentityScreen { .map(|w| w.seed_hash() == *seed_hash) .unwrap_or(false); if is_ours { + self.funding_address_request_in_flight = false; match address.parse::>() { Ok(addr) => { self.funding_address = Some(addr.assume_checked()); @@ -612,29 +660,11 @@ impl ScreenLike for TopUpIdentityScreen { outputs, )) = &backend_task_success_result { - // Accumulate what this deposit added at the shown address, so the - // "received so far" line tracks the deposit itself, not whole-wallet - // balance. - self.received_at_funding_address_duffs = self - .received_at_funding_address_duffs - .saturating_add(deposit_matches(self.funding_address.as_ref(), outputs)); - - let spendable_duffs = self - .wallet - .as_ref() - .and_then(|w| w.read().ok()) - .map(|w| { - self.app_context - .snapshot_balance(&w.seed_hash()) - .spendable() - }) - .unwrap_or(0); let minimum_credits = self.app_context.fee_estimator().estimate_identity_topup(); let (next, prefill) = deposit_event_outcome( WalletFundedScreenStep::WaitingOnFunds, self.funding_address.as_ref(), outputs, - spendable_duffs, minimum_credits, ); // Pre-fill the amount with the fee-reserve-capped balance when the @@ -897,19 +927,68 @@ impl ScreenLike for TopUpIdentityScreen { .collect() }) .unwrap_or_default(); - let tasks = self.asset_lock_cache.ensure_requested_many(seed_hashes); - if !tasks.is_empty() { - action |= AppAction::BackendTasks(tasks, BackendTasksExecutionMode::Concurrent); - } + let lock_fetches = self.asset_lock_cache.ensure_requested_many(seed_hashes); // Derive the "Receive a new deposit" address off the UI thread; the QR // view queues this when it has no address yet. - if let Some(seed_hash) = self.pending_funding_address_request.take() { - action |= AppAction::BackendTask(BackendTask::WalletTask( - WalletTask::GenerateReceiveAddress { seed_hash }, - )); + let funding_address_request = + self.pending_funding_address_request + .take() + .map(|seed_hash| { + BackendTask::WalletTask(WalletTask::GenerateReceiveAddress { seed_hash }) + }); + if funding_address_request.is_some() { + self.funding_address_request_in_flight = true; } + action |= pending_backend_tasks_action(lock_fetches, funding_address_request); action } } + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn same_frame_dispatch_keeps_lock_fetches_and_receive_address_request() { + let lock_seed_a = [1u8; 32]; + let lock_seed_b = [2u8; 32]; + let receive_seed = [3u8; 32]; + let action = pending_backend_tasks_action( + vec![ + BackendTask::WalletTask(WalletTask::ListTrackedAssetLocks { + seed_hash: lock_seed_a, + }), + BackendTask::WalletTask(WalletTask::ListTrackedAssetLocks { + seed_hash: lock_seed_b, + }), + ], + Some(BackendTask::WalletTask( + WalletTask::GenerateReceiveAddress { + seed_hash: receive_seed, + }, + )), + ); + + let AppAction::BackendTasks(tasks, BackendTasksExecutionMode::Concurrent) = action else { + panic!("same-frame tasks must be dispatched as one concurrent batch"); + }; + assert_eq!(tasks.len(), 3); + assert!(tasks.iter().any(|task| matches!( + task, + BackendTask::WalletTask(WalletTask::ListTrackedAssetLocks { seed_hash }) + if *seed_hash == lock_seed_a + ))); + assert!(tasks.iter().any(|task| matches!( + task, + BackendTask::WalletTask(WalletTask::ListTrackedAssetLocks { seed_hash }) + if *seed_hash == lock_seed_b + ))); + assert!(tasks.iter().any(|task| matches!( + task, + BackendTask::WalletTask(WalletTask::GenerateReceiveAddress { seed_hash }) + if *seed_hash == receive_seed + ))); + } +} diff --git a/src/ui/tokens/tokens_screen/mod.rs b/src/ui/tokens/tokens_screen/mod.rs index b6e20a101..bdfa97e74 100644 --- a/src/ui/tokens/tokens_screen/mod.rs +++ b/src/ui/tokens/tokens_screen/mod.rs @@ -232,7 +232,8 @@ fn completes_pending_operation( } match result { - BackendTaskSuccessResult::FetchedTokenBalances => { + BackendTaskSuccessResult::FetchedTokenBalances + | BackendTaskSuccessResult::TokenBalanceRefreshAlreadyInFlight => { *result_context == BackendTaskContext::TokenBalanceRefresh } BackendTaskSuccessResult::TokenEstimatedNonClaimedPerpetualDistributionAmountWithExplanation( @@ -3065,7 +3066,8 @@ impl ScreenLike for TokensScreen { &self.token_pricing_data, ); } - BackendTaskSuccessResult::FetchedTokenBalances => { + BackendTaskSuccessResult::FetchedTokenBalances + | BackendTaskSuccessResult::TokenBalanceRefreshAlreadyInFlight => { if completes_pending { self.refreshing_status = RefreshingStatus::NotRefreshing; self.pending_operation_context = None; diff --git a/src/ui/wallets/send_screen.rs b/src/ui/wallets/send_screen.rs index 5e31ecfb9..351a2d0a9 100644 --- a/src/ui/wallets/send_screen.rs +++ b/src/ui/wallets/send_screen.rs @@ -2985,10 +2985,17 @@ impl WalletSendScreen { .as_ref() .map(|amount| format_credits_as_dash(amount.value())) .unwrap_or_else(|| "0 DASH".to_string()); - let fee = Self::confirmation_fee_sentence( - self.current_fee_preview() - .map(|preview| preview.fee_credits), - ); + let fee_preview = self.current_fee_preview(); + let fee = Self::confirmation_fee_sentence(fee_preview.map(|preview| preview.fee_credits)); + + if let Some(recipient_receives_credits) = + fee_preview.and_then(|preview| preview.recipient_receives_credits) + { + let recipient_amount = format_credits_as_dash(recipient_receives_credits); + return format!( + "You are about to send an entered amount of {amount} to {destination}. The network fee will be deducted from the output amount, so the recipient will receive {recipient_amount}. {fee} Confirm this transaction only if the destination, amount, and fee are correct." + ); + } format!( "You are about to send {amount} to {destination}. {fee} Confirm this transaction only if the destination, amount, and fee are correct." @@ -4434,6 +4441,69 @@ mod tests { ); } + #[test] + fn simple_confirmation_reports_recipient_amount_when_fee_is_deducted() { + let (mut screen, _temp_dir) = send_screen(); + let destination_address = + PlatformAddress::try_from(testnet_core_address(8)).expect("platform destination"); + let destination = destination_address.to_bech32m_string(Network::Testnet); + screen.selected_source = Some(SourceSelection::CoreWallet); + screen.validated_destination = Some(ValidatedAddress::Platform { + address: destination_address, + bech32m: destination.clone(), + }); + screen.amount = Some(Amount::new_dash(1.0)); + + let preview = screen.current_fee_preview().expect("fee preview"); + let amount = format_credits_as_dash(screen.amount.as_ref().expect("amount").value()); + let recipient_amount = format_credits_as_dash( + preview + .recipient_receives_credits + .expect("deducted recipient amount"), + ); + let estimated_fee = format_credits_as_dash(preview.fee_credits); + + assert_eq!( + screen.simple_send_confirmation_message(), + format!( + "You are about to send an entered amount of {amount} to {destination}. The network fee will be deducted from the output amount, so the recipient will receive {recipient_amount}. The estimated network fee is approximately {estimated_fee}. Confirm this transaction only if the destination, amount, and fee are correct." + ) + ); + } + + #[test] + fn simple_confirmation_reports_entered_amount_when_fee_is_added_on_top() { + let (mut screen, _temp_dir) = send_screen(); + let source_core = testnet_core_address(9); + let source_platform = + PlatformAddress::try_from(source_core.clone()).expect("platform source"); + let destination_address = + PlatformAddress::try_from(testnet_core_address(10)).expect("platform destination"); + let destination = destination_address.to_bech32m_string(Network::Testnet); + screen.selected_source = Some(SourceSelection::PlatformAddresses(vec![( + source_platform, + source_core, + 2 * CREDITS_PER_DUFF * 100_000_000, + )])); + screen.validated_destination = Some(ValidatedAddress::Platform { + address: destination_address, + bech32m: destination.clone(), + }); + screen.amount = Some(Amount::new_dash(1.0)); + + let preview = screen.current_fee_preview().expect("fee preview"); + assert_eq!(preview.recipient_receives_credits, None); + let amount = format_credits_as_dash(screen.amount.as_ref().expect("amount").value()); + let estimated_fee = format_credits_as_dash(preview.fee_credits); + + assert_eq!( + screen.simple_send_confirmation_message(), + format!( + "You are about to send {amount} to {destination}. The estimated network fee is approximately {estimated_fee}. Confirm this transaction only if the destination, amount, and fee are correct." + ) + ); + } + #[test] fn advanced_send_click_opens_confirmation_without_dispatching() { let (mut screen, _temp_dir) = send_screen(); diff --git a/src/ui/wallets/wallets_screen/mod.rs b/src/ui/wallets/wallets_screen/mod.rs index 6ec1647a0..28588d61b 100644 --- a/src/ui/wallets/wallets_screen/mod.rs +++ b/src/ui/wallets/wallets_screen/mod.rs @@ -33,6 +33,7 @@ use crate::ui::state::account_summary::{ }; use crate::ui::theme::{ComponentStyles, DashColors, ResponseExt}; use crate::ui::{MessageType, RootScreenType, ScreenLike, ScreenType}; +use crate::wallet_backend::TransactionHistoryStatus; use crate::wallet_backend::poison::RwLockRecover; use chrono::{DateTime, Utc}; use dash_sdk::dashcore_rpc::dashcore::Address; @@ -239,6 +240,10 @@ pub struct WalletsBalancesScreen { /// Transaction count at the time `cached_tx_indices` was last built. /// Used to detect list growth that doesn't make existing indices OOB. cached_tx_source_len: Option, + /// Last hydration notice applied to the transaction-history banner. This + /// prevents a dismissed persistent notice from being recreated every frame. + transaction_history_notice: Option<(WalletSeedHash, TransactionHistoryStatus)>, + transaction_history_banner: MessageBanner, /// Persistent warning banner rendered on the single-key wallet detail /// view when the app is running on the SPV backend. Stored on the screen /// (rather than constructed fresh each frame) so the underlying tracing @@ -369,6 +374,8 @@ impl WalletsBalancesScreen { pending_wallet_refresh_on_switch: false, cached_tx_indices: None, cached_tx_source_len: None, + transaction_history_notice: None, + transaction_history_banner: MessageBanner::new(), sk_spv_warning_banner: crate::ui::components::MessageBanner::new(), import_single_key_dialog: ImportSingleKeyDialog::new(app_context.network), restore_single_key_dialog: RestoreSingleKeyDialog::new(), @@ -1730,6 +1737,26 @@ impl WalletsBalancesScreen { .unwrap_or(false); let transactions = self.snapshot_transactions(&selected_seed_hash); + let history_status = self + .app_context + .wallet_backend() + .map(|backend| backend.transaction_history_status(&selected_seed_hash)) + .unwrap_or_default(); + let notice = (selected_seed_hash, history_status.clone()); + if self.transaction_history_notice.as_ref() != Some(¬ice) { + self.transaction_history_notice = Some(notice); + match history_status.error() { + Some(error) => { + self.transaction_history_banner + .set_message(error, MessageType::Warning) + .set_details(error) + .disable_auto_dismiss(); + } + None => self.transaction_history_banner.clear(), + } + } + self.transaction_history_banner.show(ui); + if !backend_ready { ui.label("Syncing transactions from the network…"); return; diff --git a/src/wallet_backend/mod.rs b/src/wallet_backend/mod.rs index 358ea8a87..546967902 100644 --- a/src/wallet_backend/mod.rs +++ b/src/wallet_backend/mod.rs @@ -100,7 +100,7 @@ pub use kv::{DetKv, DetScope, KvAdapterError, SCHEMA_VERSION as KV_SCHEMA_VERSIO pub use loader::LoadedWallets; pub use single_key::SingleKeyView; use snapshot::SnapshotStore; -pub use snapshot::{DetUtxo, DetWalletBalance, WalletSnapshot}; +pub use snapshot::{DetUtxo, DetWalletBalance, TransactionHistoryStatus, WalletSnapshot}; use token_balance::TokenBalanceStore; pub use token_balance::UpstreamTokenBalances; pub use wallet_meta::WalletMetaView; @@ -121,6 +121,8 @@ where SyncNow: FnOnce() -> SyncFuture, SyncFuture: std::future::Future, { + // TODO: Upstream `IdentitySyncManager::sync_now()` should return a performed/skipped + // outcome or completion generation; before/after `is_syncing()` samples are racy. if is_syncing() { return TokenBalanceSyncOutcome::AlreadyInFlight; } @@ -148,7 +150,7 @@ use platform_wallet_storage::{SqlitePersister, SqlitePersisterConfig}; use crate::app::TaskResult; use crate::backend_task::BackendTaskSuccessResult; -use crate::backend_task::error::TaskError; +use crate::backend_task::error::{TaskError, WalletTransactionHistoryError}; use crate::context::AppContext; use crate::context::connection_status::ConnectionStatus; use crate::model::selected_identity::SelectedIdentity; @@ -277,6 +279,24 @@ const REGISTRATION_RESOLVE_BACKOFF: std::time::Duration = std::time::Duration::f /// from DET's `WalletSeedHash` = `SHA256(seed_bytes)`. The map is the bridge: /// populated once per wallet at registration, read by every DET-keyed call. type WalletId = [u8; 32]; +const MAX_PERSISTED_TRANSACTION_SKIP_CONTEXTS: usize = 5; + +#[derive(Debug, Default)] +struct PersistedTransactionHydration { + hydrated_rows: usize, + skipped_rows: usize, +} + +fn record_persisted_transaction_skip( + skipped_rows: &mut usize, + sampled_contexts: &mut Vec, + context: impl FnOnce() -> String, +) { + *skipped_rows = skipped_rows.saturating_add(1); + if sampled_contexts.len() < MAX_PERSISTED_TRANSACTION_SKIP_CONTEXTS { + sampled_contexts.push(context()); + } +} /// Per-wallet platform-address warm-start seed: `(seed_hash, owned /// [`PlatformAddressEntry`] list, optional (timestamp, height) cursor)`. @@ -708,7 +728,6 @@ impl WalletBackend { continue; }; - self.hydrate_persisted_transactions(&wallet_id)?; self.inner.id_map.write()?.insert(seed_hash, wallet_id); self.inner .wallets @@ -717,6 +736,7 @@ impl WalletBackend { self.inner .snapshots .register_wallet(seed_hash, wallet_id, pw); + self.hydrate_persisted_transactions_nonfatal(&wallet_id); self.inner.snapshots.recompute(&wallet_id); tracing::debug!( wallet = %hex::encode(seed_hash), @@ -1065,7 +1085,6 @@ impl WalletBackend { return Err(TaskError::WalletRegistrationXpubMismatch); } - self.hydrate_persisted_transactions(&wallet_id)?; self.inner.id_map.write()?.insert(seed_hash, wallet_id); self.inner .wallets @@ -1074,13 +1093,59 @@ impl WalletBackend { self.inner .snapshots .register_wallet(seed_hash, wallet_id, pw); + self.hydrate_persisted_transactions_nonfatal(&wallet_id); self.inner.snapshots.recompute(&wallet_id); Ok(()) } + fn hydrate_persisted_transactions_nonfatal(&self, wallet_id: &WalletId) { + match self.hydrate_persisted_transactions(wallet_id) { + Ok(outcome) => { + let status = if outcome.skipped_rows == 0 { + TransactionHistoryStatus::Complete + } else { + let error = Arc::new(TaskError::WalletTransactionHistoryPartial { + source: WalletTransactionHistoryError::RowsSkipped { + skipped_rows: outcome.skipped_rows, + }, + }); + TransactionHistoryStatus::Partial { + skipped_rows: outcome.skipped_rows, + error, + } + }; + self.inner + .snapshots + .set_transaction_history_status(*wallet_id, status); + tracing::debug!( + wallet_id = %hex::encode(wallet_id), + hydrated_rows = outcome.hydrated_rows, + skipped_rows = outcome.skipped_rows, + "Persisted transaction history hydration complete" + ); + } + Err(error) => { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + error = ?error, + "Persisted transaction history could not be loaded; continuing wallet registration" + ); + self.inner.snapshots.set_transaction_history_status( + *wallet_id, + TransactionHistoryStatus::Unavailable { + error: Arc::new(error), + }, + ); + } + } + } + /// Restore persisted public transaction records before publishing a /// wallet's first display snapshot. This path never touches wallet secrets. - fn hydrate_persisted_transactions(&self, wallet_id: &WalletId) -> Result<(), TaskError> { + fn hydrate_persisted_transactions( + &self, + wallet_id: &WalletId, + ) -> Result { use crate::backend_task::error::WalletTransactionHistoryError; use dash_sdk::dpp::dashcore::hashes::Hash; use platform_wallet::changeset::PlatformWalletPersistence; @@ -1097,6 +1162,8 @@ impl WalletBackend { rusqlite::OpenFlags::SQLITE_OPEN_READ_ONLY, ) .map_err(&storage_error)?; + let mut skipped_rows = 0; + let mut skipped_contexts = Vec::new(); let txid_bytes = { // Upstream's public API decodes full records one txid at a time. // Enumerate only its keys here, then delegate every record read. @@ -1111,37 +1178,71 @@ impl WalletBackend { let rows = statement .query_map([wallet_id.as_slice()], |row| row.get::<_, Vec>(0)) .map_err(&storage_error)?; - rows.collect::, _>>() - .map_err(&storage_error)? + let mut txid_bytes = Vec::new(); + for (row_index, row) in rows.enumerate() { + match row { + Ok(bytes) => txid_bytes.push(bytes), + Err(source) => { + record_persisted_transaction_skip( + &mut skipped_rows, + &mut skipped_contexts, + || format!("row {row_index} read failed: {source:?}"), + ); + } + } + } + txid_bytes }; drop(connection); let mut records = Vec::with_capacity(txid_bytes.len()); for bytes in txid_bytes { - let txid = dash_sdk::dpp::dashcore::Txid::from_slice(&bytes).map_err(|source| { - TaskError::WalletTransactionHistoryLoad { - source: WalletTransactionHistoryError::Persistence { - source: WalletStorageError::HashDecode { source }.into(), - }, + let txid = match dash_sdk::dpp::dashcore::Txid::from_slice(&bytes) { + Ok(txid) => txid, + Err(source) => { + record_persisted_transaction_skip( + &mut skipped_rows, + &mut skipped_contexts, + || format!("invalid transaction id length {}: {source:?}", bytes.len()), + ); + continue; } - })?; - let record = self - .inner - .persister - .get_core_tx_record(*wallet_id, &txid) - .map_err(|source| TaskError::WalletTransactionHistoryLoad { - source: WalletTransactionHistoryError::Persistence { source }, - })? - .ok_or(TaskError::WalletTransactionHistoryLoad { - source: WalletTransactionHistoryError::RecordMissing { txid }, - })?; - records.push(record); + }; + match self.inner.persister.get_core_tx_record(*wallet_id, &txid) { + Ok(Some(record)) => records.push(record), + Ok(None) => { + record_persisted_transaction_skip( + &mut skipped_rows, + &mut skipped_contexts, + || format!("record disappeared for transaction id {txid}"), + ); + } + Err(source) => { + record_persisted_transaction_skip( + &mut skipped_rows, + &mut skipped_contexts, + || format!("record read failed for transaction id {txid}: {source:?}"), + ); + } + } } + let hydrated_rows = records.len(); self.inner .snapshots .hydrate_transactions(wallet_id, records.iter()); - Ok(()) + if skipped_rows > 0 { + tracing::warn!( + wallet_id = %hex::encode(wallet_id), + skipped_rows, + sampled_contexts = ?skipped_contexts, + "Persisted transaction history loaded with skipped rows" + ); + } + Ok(PersistedTransactionHydration { + hydrated_rows, + skipped_rows, + }) } /// Wipe every piece of DET-local state for a forgotten wallet — the @@ -2305,6 +2406,18 @@ impl WalletBackend { .clone() } + /// Startup hydration status for the display-only transaction history. + pub fn transaction_history_status( + &self, + seed_hash: &WalletSeedHash, + ) -> TransactionHistoryStatus { + self.inner + .snapshots + .snapshot(seed_hash) + .transaction_history_status + .clone() + } + /// Current unspent outputs for the wallet. DISPLAY-ONLY — never feed /// these into coin selection (A04 fund-safety gate). pub fn utxos(&self, seed_hash: &WalletSeedHash) -> Vec { diff --git a/src/wallet_backend/snapshot.rs b/src/wallet_backend/snapshot.rs index d6a494f45..3ea0af0ba 100644 --- a/src/wallet_backend/snapshot.rs +++ b/src/wallet_backend/snapshot.rs @@ -44,6 +44,7 @@ use dash_sdk::dpp::key_wallet::transaction_checking::TransactionContext; use dash_sdk::dpp::key_wallet::wallet::managed_wallet_info::wallet_info_interface::WalletInfoInterface; use platform_wallet::PlatformWallet; +use crate::backend_task::error::TaskError; use crate::model::dashpay::DetectedIncomingOutput; use crate::model::wallet::{TransactionStatus, WalletSeedHash, WalletTransaction}; @@ -105,6 +106,52 @@ pub struct WalletSnapshot { /// bookkeeping has not indexed yet, so no funded address is dropped from the /// per-category tab totals. pub address_paths: BTreeMap, + /// Whether persisted transaction history was fully restored at startup. + pub transaction_history_status: TransactionHistoryStatus, +} + +/// Startup restoration state for the display-only transaction history. +#[derive(Debug, Clone, Default)] +pub enum TransactionHistoryStatus { + #[default] + Complete, + Partial { + skipped_rows: usize, + error: Arc, + }, + Unavailable { + error: Arc, + }, +} + +impl PartialEq for TransactionHistoryStatus { + fn eq(&self, other: &Self) -> bool { + match (self, other) { + (Self::Complete, Self::Complete) => true, + ( + Self::Partial { + skipped_rows: left, .. + }, + Self::Partial { + skipped_rows: right, + .. + }, + ) => left == right, + (Self::Unavailable { .. }, Self::Unavailable { .. }) => true, + _ => false, + } + } +} + +impl Eq for TransactionHistoryStatus {} + +impl TransactionHistoryStatus { + pub fn error(&self) -> Option<&Arc> { + match self { + Self::Complete => None, + Self::Partial { error, .. } | Self::Unavailable { error } => Some(error), + } + } } /// Chain-derived parts of a published snapshot — everything except the @@ -354,6 +401,7 @@ pub(super) struct SnapshotStore { /// `Txid`. A `BTreeMap` per wallet so re-seen records (mempool → block → /// chainlock) upsert in place and iteration is deterministic. tx_log: Mutex>>, + transaction_history_status: Mutex>, /// Per-wallet registration: upstream `WalletId` → (DET `WalletSeedHash`, /// cheap shared `PlatformWallet` handle). The handle gives lock-free /// balance (`balance()`) and non-blocking UTXO (`try_state()`) reads, so @@ -377,6 +425,7 @@ impl SnapshotStore { Self { snapshots: ArcSwap::from_pointee(HashMap::new()), tx_log: Mutex::new(HashMap::new()), + transaction_history_status: Mutex::new(HashMap::new()), registered: Mutex::new(HashMap::new()), } } @@ -411,6 +460,9 @@ impl SnapshotStore { if let Ok(mut log) = self.tx_log.lock() { log.remove(wallet_id); } + if let Ok(mut statuses) = self.transaction_history_status.lock() { + statuses.remove(wallet_id); + } } /// Resolve an upstream `WalletId` to DET's `WalletSeedHash`, if the wallet @@ -474,6 +526,16 @@ impl SnapshotStore { } } + pub(super) fn set_transaction_history_status( + &self, + wallet_id: WalletId, + status: TransactionHistoryStatus, + ) { + if let Ok(mut statuses) = self.transaction_history_status.lock() { + statuses.insert(wallet_id, status); + } + } + /// Upgrade a previously-accumulated record's status to /// `InstantSendLocked`. /// @@ -624,6 +686,12 @@ impl SnapshotStore { address_balances: state.address_balances, monitored_receive_addresses: state.monitored_receive_addresses, address_paths: state.address_paths, + transaction_history_status: self + .transaction_history_status + .lock() + .ok() + .and_then(|statuses| statuses.get(wallet_id).cloned()) + .unwrap_or_default(), }); self.snapshots.rcu(|current| { @@ -1001,6 +1069,29 @@ mod tests { assert!(store.snapshot(&seed(9)).transactions.is_empty()); } + #[test] + fn unavailable_history_status_is_published_with_the_wallet_snapshot() { + let store = SnapshotStore::new(); + store.set_transaction_history_status( + wid(8), + TransactionHistoryStatus::Unavailable { + error: Arc::new(TaskError::WalletTransactionHistoryPartial { + source: + crate::backend_task::error::WalletTransactionHistoryError::RowsSkipped { + skipped_rows: 1, + }, + }), + }, + ); + + publish_tx_only(&store, seed(8), wid(8)); + + assert!(matches!( + store.snapshot(&seed(8)).transaction_history_status, + TransactionHistoryStatus::Unavailable { .. } + )); + } + /// A published snapshot whose header total equals the sum of its /// per-address breakdown — the consistency invariant the wallets screen /// relies on (`core_balance_duffs` reads `.balance.total`; the Core tab sums @@ -1030,6 +1121,7 @@ mod tests { address_balances, monitored_receive_addresses: vec!["yWatched".to_string()], address_paths, + transaction_history_status: TransactionHistoryStatus::Complete, } }