From 5945bf8287877c475ceb68328d6a4892dbb3ac43 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 10:25:33 +0100 Subject: [PATCH 01/17] fix(wallet): calculate asset lock tx fee dynamically based on input count Replace hardcoded 3000 duff fee with dynamic fee calculation that accounts for actual number of inputs. Estimates tx size using standard component sizes (P2PKH input ~148B, output ~34B, header ~10B, payload ~60B) and uses max(3000, estimated_size) to always meet the min relay fee. Properly handles fee shortfall when allow_take_fee_from_amount is set, and returns clear error messages for insufficient funds. Co-Authored-By: Claude Opus 4.6 --- src/model/wallet/asset_lock_transaction.rs | 43 +++++++++++++++++----- 1 file changed, 34 insertions(+), 9 deletions(-) diff --git a/src/model/wallet/asset_lock_transaction.rs b/src/model/wallet/asset_lock_transaction.rs index fbc73f1fb..a189e5011 100644 --- a/src/model/wallet/asset_lock_transaction.rs +++ b/src/model/wallet/asset_lock_transaction.rs @@ -145,19 +145,44 @@ impl Wallet { let asset_lock_public_key = private_key.public_key(&secp); let one_time_key_hash = asset_lock_public_key.pubkey_hash(); - let fee = 3_000; - let (utxos, change_option) = self - .take_unspent_utxos_for(amount, fee, allow_take_fee_from_amount) + // Use a small initial estimate to select UTXOs; we recalculate the fee + // below based on the actual number of inputs. + let initial_fee_estimate = 3_000u64; + + let (utxos, initial_change_option) = self + .take_unspent_utxos_for(amount, initial_fee_estimate, allow_take_fee_from_amount) .ok_or("take_unspent_utxos_for() returned None".to_string())?; - let actual_amount = if change_option.is_none() && allow_take_fee_from_amount { - // The amount has been adjusted by taking the fee from the amount - // Calculate the adjusted amount based on the total value of the UTXOs minus the fee - let total_input_value: u64 = utxos.iter().map(|(_, (tx_out, _))| tx_out.value).sum(); - total_input_value - fee + // Calculate fee based on actual transaction size so we always meet the + // min relay fee (1 duff/byte = 1000 duffs/kB). + // Sizes: P2PKH input ~148 B, output ~34 B, header ~10 B, payload ~60 B. + let num_inputs = utxos.len(); + let has_initial_change = initial_change_option.is_some(); + let num_outputs = 1 + if has_initial_change { 1 } else { 0 }; + let estimated_size = 10 + (num_inputs * 148) + (num_outputs * 34) + 60; + let fee = std::cmp::max(initial_fee_estimate, estimated_size as u64); + + // Recalculate amount and change with the real fee + let total_input_value: u64 = utxos.iter().map(|(_, (tx_out, _))| tx_out.value).sum(); + + let (actual_amount, change_option) = if total_input_value < amount + fee { + if allow_take_fee_from_amount { + // Deduct the fee shortfall from the output amount + let adjusted = total_input_value.saturating_sub(fee); + if adjusted == 0 { + return Err("Insufficient funds for transaction fee".to_string()); + } + (adjusted, None) + } else { + return Err(format!( + "Insufficient funds: need {} + {} fee, have {}", + amount, fee, total_input_value + )); + } } else { - amount + let change = total_input_value - amount - fee; + (amount, if change > 0 { Some(change) } else { None }) }; let payload_output = TxOut { From e06b6acdfcdcf5d3505f77a3f5318a0862a2fa0c Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 10:32:55 +0100 Subject: [PATCH 02/17] feat(ui): add sync status panel to wallet screen Co-Authored-By: Claude Opus 4.6 --- src/ui/wallets/wallets_screen/mod.rs | 247 ++++++++++++++++++++++++++- 1 file changed, 246 insertions(+), 1 deletion(-) diff --git a/src/ui/wallets/wallets_screen/mod.rs b/src/ui/wallets/wallets_screen/mod.rs index d6bf5a002..9600a0676 100644 --- a/src/ui/wallets/wallets_screen/mod.rs +++ b/src/ui/wallets/wallets_screen/mod.rs @@ -9,7 +9,7 @@ use crate::backend_task::core::CoreTask; use crate::context::AppContext; use crate::model::amount::Amount; use crate::model::wallet::{Wallet, WalletSeedHash, WalletTransaction}; -use crate::spv::CoreBackendMode; +use crate::spv::{CoreBackendMode, SpvStatus}; use crate::ui::components::component_trait::Component; use crate::ui::components::confirmation_dialog::{ConfirmationDialog, ConfirmationStatus}; use crate::ui::components::left_panel::add_left_panel; @@ -23,6 +23,7 @@ use crate::ui::wallets::account_summary::{ }; use crate::ui::{MessageType, RootScreenType, ScreenLike, ScreenType}; use chrono::{DateTime, Utc}; +use dash_sdk::dash_spv::sync::{ProgressPercentage, SyncProgress as SpvSyncProgress, SyncState}; use dash_sdk::dashcore_rpc::dashcore::Address; use dash_sdk::dpp::balances::credits::CREDITS_PER_DUFF; use eframe::egui::{self, ComboBox, Context, Ui}; @@ -114,6 +115,8 @@ pub struct WalletsBalancesScreen { utxo_page: usize, /// Selected refresh mode (only shown in dev mode) refresh_mode: RefreshMode, + /// Cached platform sync info: (last_sync_timestamp, last_sync_height) + platform_sync_info: Option<(u64, u64)>, } impl WalletsBalancesScreen { @@ -171,6 +174,13 @@ impl WalletsBalancesScreen { selected_wallet: Option>>, selected_single_key_wallet: Option>>, ) -> Self { + let platform_sync_info = selected_wallet + .as_ref() + .and_then(|w| w.read().ok().map(|g| g.seed_hash())) + .and_then(|hash| app_context.db.get_platform_sync_info(&hash).ok()) + .map(|(ts, checkpoint, _terminal)| (ts, checkpoint)) + .filter(|(ts, _)| *ts > 0); + Self { selected_wallet, selected_single_key_wallet, @@ -201,6 +211,7 @@ impl WalletsBalancesScreen { pending_asset_lock_search_after_unlock: false, utxo_page: 0, refresh_mode: RefreshMode::default(), + platform_sync_info, } } @@ -224,6 +235,17 @@ impl WalletsBalancesScreen { .update_selected_single_key_hash(hash.as_ref()); } + /// Refresh the cached platform sync info from the database. + fn refresh_platform_sync_info_cache(&mut self, seed_hash: &WalletSeedHash) { + self.platform_sync_info = self + .app_context + .db + .get_platform_sync_info(seed_hash) + .ok() + .map(|(ts, checkpoint, _terminal)| (ts, checkpoint)) + .filter(|(ts, _)| *ts > 0); + } + fn select_hd_wallet(&mut self, wallet: Arc>) { self.selected_wallet = Some(wallet.clone()); self.selected_single_key_wallet = None; @@ -231,6 +253,7 @@ impl WalletsBalancesScreen { if let Ok(hash) = wallet.read().map(|g| g.seed_hash()) { self.persist_selected_wallet_hash(Some(hash)); + self.refresh_platform_sync_info_cache(&hash); } self.persist_selected_single_key_hash(None); } @@ -794,6 +817,29 @@ impl WalletsBalancesScreen { Amount::dash_from_duffs(amount_duffs).to_string() } + /// Format a Unix timestamp (seconds since epoch) as a relative "time ago" string. + fn format_unix_time_ago(unix_ts: u64) -> String { + let now = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|d| d.as_secs()) + .unwrap_or(0); + let elapsed_secs = now.saturating_sub(unix_ts); + Self::format_duration_ago(std::time::Duration::from_secs(elapsed_secs)) + } + + fn format_duration_ago(duration: std::time::Duration) -> String { + let secs = duration.as_secs(); + if secs < 60 { + format!("{}s ago", secs) + } else if secs < 3600 { + format!("{}m ago", secs / 60) + } else if secs < 86400 { + format!("{}h ago", secs / 3600) + } else { + format!("{}d ago", secs / 86400) + } + } + fn transaction_direction_label(tx: &WalletTransaction) -> &'static str { if tx.is_incoming() { "Received" @@ -1085,6 +1131,199 @@ impl WalletsBalancesScreen { }); } + fn render_sync_status(&self, ui: &mut Ui) { + let dark_mode = ui.ctx().style().visuals.dark_mode; + + Frame::group(ui.style()) + .fill(DashColors::surface(dark_mode)) + .inner_margin(Margin::symmetric(16, 8)) + .show(ui, |ui| { + // Line 1 -- Core sync status + ui.horizontal(|ui| { + ui.label( + RichText::new("Core:") + .size(12.0) + .strong() + .color(DashColors::text_primary(dark_mode)), + ); + + match self.app_context.core_backend_mode() { + CoreBackendMode::Rpc => { + if self.app_context.connection_status().rpc_online() { + ui.colored_label( + Color32::DARK_GREEN, + RichText::new("Connected").size(12.0), + ); + } else { + ui.colored_label( + DashColors::ERROR, + RichText::new("Disconnected").size(12.0), + ); + } + } + CoreBackendMode::Spv => { + let snapshot = self.app_context.spv_manager().status(); + match snapshot.status { + SpvStatus::Idle | SpvStatus::Stopped => { + ui.label( + RichText::new("Disconnected") + .size(12.0) + .color(DashColors::text_secondary(dark_mode)), + ); + } + SpvStatus::Starting => { + ui.add( + egui::Spinner::new() + .size(12.0) + .color(DashColors::DASH_BLUE), + ); + ui.label( + RichText::new("Connecting...") + .size(12.0) + .color(DashColors::DASH_BLUE), + ); + } + SpvStatus::Syncing => { + ui.add( + egui::Spinner::new() + .size(12.0) + .color(DashColors::DASH_BLUE), + ); + let phase_text = + Self::spv_active_phase_text(&snapshot.sync_progress); + ui.label( + RichText::new(format!("Syncing — {phase_text}")) + .size(12.0) + .color(DashColors::DASH_BLUE), + ); + } + SpvStatus::Running => { + ui.colored_label( + Color32::DARK_GREEN, + RichText::new(format!( + "Synced — {} peers", + snapshot.connected_peers + )) + .size(12.0), + ); + } + SpvStatus::Stopping => { + ui.add( + egui::Spinner::new() + .size(12.0) + .color(DashColors::DASH_BLUE), + ); + ui.label( + RichText::new("Disconnecting...") + .size(12.0) + .color(DashColors::DASH_BLUE), + ); + } + SpvStatus::Error => { + ui.colored_label( + DashColors::ERROR, + RichText::new("Error").size(12.0), + ); + } + } + } + } + }); + + // Line 2 -- Platform sync status + ui.horizontal(|ui| { + ui.label( + RichText::new("Platform:") + .size(12.0) + .strong() + .color(DashColors::text_primary(dark_mode)), + ); + + // Addresses + let addr_count = self + .selected_wallet + .as_ref() + .and_then(|w| w.read().ok()) + .map(|w| w.platform_address_info.len()) + .unwrap_or(0); + if self.refreshing { + ui.add(egui::Spinner::new().size(12.0).color(DashColors::DASH_BLUE)); + } + let addr_text = + if let Some((last_sync_ts, sync_height)) = self.platform_sync_info { + let ago = Self::format_unix_time_ago(last_sync_ts); + format!( + "Addresses: {} synced (blk {}, {})", + addr_count, sync_height, ago + ) + } else { + "Addresses: never synced".to_string() + }; + ui.label( + RichText::new(addr_text) + .size(12.0) + .color(if self.refreshing { + DashColors::DASH_BLUE + } else { + DashColors::text_secondary(dark_mode) + }), + ); + }); + }); + } + + /// Get a text summary of the active SPV sync phase. + fn spv_active_phase_text(sync_progress: &Option) -> String { + let Some(progress) = sync_progress else { + return "starting...".to_string(); + }; + + // Grab target height from headers (blocks doesn't expose target_height directly) + let target_height = progress + .headers() + .ok() + .map(|h| h.target_height()) + .unwrap_or(0); + + // Check phases in order of execution + if let Ok(headers) = progress.headers() + && headers.state() == SyncState::Syncing + { + let pct = Self::simple_progress_pct(headers.current_height(), headers.target_height()); + return format!("Headers {pct}%"); + } + + if let Ok(fh) = progress.filter_headers() + && fh.state() == SyncState::Syncing + { + let pct = Self::simple_progress_pct(fh.current_height(), fh.target_height()); + return format!("Filter Headers {pct}%"); + } + + if let Ok(filters) = progress.filters() + && filters.state() == SyncState::Syncing + { + let pct = Self::simple_progress_pct(filters.current_height(), filters.target_height()); + return format!("Filters {pct}%"); + } + + if let Ok(blocks) = progress.blocks() + && blocks.state() == SyncState::Syncing + { + let pct = Self::simple_progress_pct(blocks.last_processed(), target_height); + return format!("Blocks {pct}%"); + } + + "syncing...".to_string() + } + + fn simple_progress_pct(current: u32, target: u32) -> u32 { + if target == 0 { + return 0; + } + ((current as f64 / target as f64) * 100.0).clamp(0.0, 100.0) as u32 + } + fn render_wallet_detail_panel(&mut self, ui: &mut Ui, ctx: &Context) -> AppAction { let Some(wallet_arc) = self.selected_wallet.clone() else { self.render_no_wallets_view(ui); @@ -1439,6 +1678,12 @@ impl ScreenLike for WalletsBalancesScreen { ui.add_space(10.0); + // Sync status panel (only for HD wallets) + if self.selected_wallet.is_some() { + self.render_sync_status(ui); + ui.add_space(6.0); + } + // Render the appropriate detail view based on selection if self.selected_wallet.is_some() { inner_action |= self.render_wallet_detail_panel(ui, ctx); From 87bd1b2efc47662dea9ddd65dc3a4a1e8995e1b2 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 11:27:35 +0100 Subject: [PATCH 03/17] docs: add manual test scenarios for asset lock fee fix Co-Authored-By: Claude Opus 4.6 --- .../manual-test-scenarios.md | 277 ++++++++++++++++++ 1 file changed, 277 insertions(+) create mode 100644 docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scenarios.md diff --git a/docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scenarios.md b/docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scenarios.md new file mode 100644 index 000000000..c41b822b9 --- /dev/null +++ b/docs/ai-design/2026-02-24-asset-lock-fee-fix/manual-test-scenarios.md @@ -0,0 +1,277 @@ +# Manual Test Scenarios: Dynamic Asset Lock Fee Calculation + +**PR:** #636 (`zk-extract/asset-lock-fee-fix`) +**Date:** 2026-02-24 +**Component:** `src/model/wallet/asset_lock_transaction.rs` + +## Background + +This PR replaces the hardcoded 3000 duff fee in `asset_lock_transaction_from_private_key()` with a +dynamic fee calculated from the estimated transaction size: +- 10 bytes header + (inputs x 148 bytes) + (outputs x 34 bytes) + 60 bytes payload +- Minimum fee remains 3000 duffs (via `std::cmp::max`) +- Change output is recalculated after the real fee is determined +- Fee shortfall is handled gracefully when `allow_take_fee_from_amount` is set + +The change affects three public entry points: +- `registration_asset_lock_transaction()` -- identity registration +- `top_up_asset_lock_transaction()` -- identity top-up +- `generic_asset_lock_transaction()` -- platform address funding + +**Note:** `asset_lock_transaction_for_utxo_from_private_key()` (single-UTXO variant) is NOT changed +by this PR and still uses a hardcoded 3000 duff fee. + +--- + +## Preconditions (all scenarios) + +1. Dash Evo Tool is running and connected to **Testnet** (or a Devnet). +2. A wallet is loaded with known UTXO distribution (check wallet screen for balances). +3. The wallet has been synced recently (UTXOs are up to date). +4. Dash Core node is reachable and accepting transactions. + +--- + +## Scenario 1: Single-Input Asset Lock (fee equals minimum) + +**Purpose:** Verify that a transaction with 1 input still works correctly and the fee is at least +3000 duffs. With 1 input and 2 outputs: `10 + 148 + 68 + 60 = 286 bytes`, which is below 3000, so +the minimum fee of 3000 applies. + +### Preconditions +- Wallet has a single UTXO of at least 50,000 duffs (0.0005 DASH). + +### Steps +1. Navigate to the Identity screen. +2. Initiate a new identity registration with an amount of 10,000 duffs. +3. Confirm the transaction. + +### Expected Results +- The asset lock transaction is created and broadcast successfully. +- The transaction is accepted by the network (no "min relay fee not met" error). +- The wallet balance decreases by approximately 10,000 + 3,000 = 13,000 duffs. +- If the UTXO was larger than the required amount, a change output is returned to the wallet. +- The identity registration proceeds to completion (or awaits proof confirmation). + +--- + +## Scenario 2: Multiple-Input Asset Lock (fee scales with inputs) + +**Purpose:** Verify that when many UTXOs are consumed, the fee scales upward with the number of +inputs, exceeding the 3000 duff minimum. + +### Preconditions +- Wallet has many small UTXOs (e.g., 30 UTXOs of 5,000 duffs each = 150,000 duffs total). +- No single UTXO is large enough to cover the requested amount alone. + +### Steps +1. Navigate to the Identity screen. +2. Initiate a new identity registration with an amount of 100,000 duffs (0.001 DASH). +3. Confirm the transaction. + +### Expected Results +- The transaction consumes multiple UTXOs (approximately 21+ inputs needed for 100,000 + fee). +- With ~21 inputs: estimated size = `10 + (21 * 148) + (2 * 34) + 60 = 3246 bytes`, so fee should + be 3246 duffs (above the 3000 minimum). +- The transaction is accepted by the network without "min relay fee not met" errors. +- The wallet balance decreases by the requested amount plus the dynamically calculated fee. +- Previous behavior with a hardcoded 3000 fee would have been rejected by the network for large + input counts, so acceptance confirms the fix works. + +--- + +## Scenario 3: Tight Balance with Fee Deduction from Amount + +**Purpose:** Verify that when `allow_take_fee_from_amount` is true and the wallet balance barely +covers the amount, the fee is correctly deducted from the output amount. + +### Preconditions +- Wallet has exactly one UTXO of 10,000 duffs. + +### Steps +1. Navigate to the platform address funding screen (or identity top-up). +2. Initiate a funding/top-up for 10,000 duffs with the "deduct fee from amount" option enabled. +3. Confirm the transaction. + +### Expected Results +- The transaction is created successfully. +- The actual funded amount is 10,000 - 3,000 = 7,000 duffs (fee deducted from amount). +- No change output is present in the transaction. +- The transaction is accepted by the network. +- The wallet balance drops to 0. + +--- + +## Scenario 4: Insufficient Funds (no fee deduction allowed) + +**Purpose:** Verify that a clear error message is shown when the wallet cannot cover the amount +plus fee and fee deduction is not allowed. + +### Preconditions +- Wallet has exactly one UTXO of 10,000 duffs. + +### Steps +1. Navigate to the platform address funding screen. +2. Initiate a funding for 10,000 duffs with the "deduct fee from amount" option DISABLED. +3. Confirm the transaction. + +### Expected Results +- The operation fails with an error message similar to: + `"Insufficient funds: need 10000 + 3000 fee, have 10000"` +- The error message includes the specific amounts (requested, fee, available). +- No transaction is broadcast to the network. +- The wallet UTXOs remain unchanged. + +--- + +## Scenario 5: Insufficient Funds (fee deduction allowed but amount too small) + +**Purpose:** Verify that when the entire balance would be consumed by the fee alone, a clear error +is returned even when fee deduction is allowed. + +### Preconditions +- Wallet has exactly one UTXO of 2,500 duffs (less than the minimum 3000 fee). + +### Steps +1. Navigate to the identity registration screen. +2. Initiate a registration with an amount of 2,500 duffs, with fee deduction from amount enabled. +3. Confirm the transaction. + +### Expected Results +- The operation fails with `"Insufficient funds for transaction fee"` or the UTXO selection + returns None (displayed as an appropriate error in the UI). +- No transaction is broadcast. +- Wallet UTXOs remain unchanged. + +--- + +## Scenario 6: Change Output Presence and Correctness + +**Purpose:** Verify that the change output is correctly calculated after the dynamic fee is applied, +not the initial estimate. + +### Preconditions +- Wallet has a single UTXO of 100,000 duffs. + +### Steps +1. Navigate to the Identity screen. +2. Initiate a new identity registration with an amount of 50,000 duffs. +3. Confirm the transaction. + +### Expected Results +- The transaction has exactly 2 outputs: the burn output (50,000 duffs) and a change output. +- Change = 100,000 - 50,000 - fee. With 1 input, 2 outputs: fee = max(3000, 10+148+68+60) = 3000. +- Change should be 100,000 - 50,000 - 3,000 = 47,000 duffs. +- The change output goes to a wallet-controlled change address. +- After the transaction confirms, the wallet shows approximately 47,000 duffs remaining. + +--- + +## Scenario 7: No Change Output (exact amount consumed) + +**Purpose:** Verify that when inputs exactly equal amount + fee, no change output is created. + +### Preconditions +- Wallet has a single UTXO of exactly 53,000 duffs. + +### Steps +1. Navigate to the Identity screen. +2. Initiate a new identity registration with an amount of 50,000 duffs. +3. Confirm the transaction. + +### Expected Results +- The transaction has exactly 1 output (the burn output for 50,000 duffs). No change output. +- Fee = 3,000 duffs (1 input, 1 output: `10 + 148 + 34 + 60 = 252`, min 3000 applies). +- 53,000 - 50,000 - 3,000 = 0, so no change. +- The wallet balance drops to 0 after confirmation. + +--- + +## Scenario 8: Transaction Acceptance by Network (regression) + +**Purpose:** This is the core regression test -- confirm that transactions which previously failed +with "min relay fee not met" now succeed. + +### Preconditions +- Wallet has 50+ small UTXOs (e.g., from prior dust consolidation or faucet drips). + +### Steps +1. Attempt to register an identity or top up an identity using an amount that requires consuming + many UTXOs (e.g., 200,000 duffs spread across 50+ UTXOs of 4,000 duffs each). +2. Confirm the transaction. + +### Expected Results +- The transaction is broadcast successfully (no RPC error). +- The transaction is accepted into the mempool (no rejection). +- With 50 inputs: estimated size = `10 + (50 * 148) + (2 * 34) + 60 = 7538 bytes`. +- Fee should be 7,538 duffs (well above the old hardcoded 3,000). +- The identity registration or top-up completes normally after proof confirmation. + +--- + +## Scenario 9: Identity Top-Up with Dynamic Fee + +**Purpose:** Verify the fix works for identity top-up flows (not just registration). + +### Preconditions +- An identity already exists in the wallet. +- Wallet has sufficient balance (e.g., 100,000 duffs). + +### Steps +1. Navigate to the identity detail screen. +2. Initiate a top-up of 20,000 duffs. +3. Confirm the transaction. + +### Expected Results +- The asset lock transaction is created with a dynamically calculated fee. +- The transaction is accepted by the network. +- The identity balance increases by approximately 20,000 duffs (in credits). +- Wallet balance decreases by 20,000 + fee. + +--- + +## Scenario 10: Platform Address Funding with Dynamic Fee + +**Purpose:** Verify the fix works for the generic platform address funding flow. + +### Preconditions +- Wallet has sufficient balance (e.g., 100,000 duffs). +- A valid platform address is available to fund. + +### Steps +1. Navigate to the platform address funding screen. +2. Enter a destination platform address and amount of 30,000 duffs. +3. Choose "deduct fee from amount" = disabled. +4. Confirm the transaction. + +### Expected Results +- The asset lock transaction is created with a dynamically calculated fee. +- The transaction is accepted by the network. +- The destination platform address receives exactly 30,000 duffs worth of credits (minus platform + fees, but not minus Core tx fees). +- Wallet balance decreases by more than 30,000 duffs (amount + estimated platform fee + Core fee). + +--- + +## Edge Cases + +### E1: Very Large Number of Inputs +- If a wallet has 100+ tiny UTXOs and attempts to spend them all, the fee could exceed 15,000 duffs. + Verify the transaction is still accepted and the fee does not consume an unreasonable portion of + the amount. + +### E2: UTXO Selection Crosses Initial Estimate Threshold +- The initial estimate of 3000 duffs is used for UTXO selection. If the real fee is higher and + causes a shortfall (total inputs < amount + real fee), the code should handle this via the + `allow_take_fee_from_amount` path or return an error. Verify no panic or silent data loss occurs. + +### E3: Database Connection Change (secondary change in PR) +- The PR also removes `Arc` wrapping from the `Database.conn` field and removes the + `shared_connection()` method. Verify that all database operations (wallet persistence, UTXO + tracking, identity storage) continue to work correctly after this change. This is a structural + refactor and should not affect behavior. + +### E4: Concurrent Asset Lock Creation +- If two asset lock transactions are created in quick succession (e.g., rapid identity registration + attempts), verify that UTXO double-spend is prevented and the second attempt either uses different + UTXOs or fails gracefully. From 76c85017cd8266b96497644b586ea3cb3809b324 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 11:27:57 +0100 Subject: [PATCH 04/17] docs: add manual test scenarios for sync status panel Co-Authored-By: Claude Opus 4.6 --- .../manual-test-scenarios.md | 436 ++++++++++++++++++ 1 file changed, 436 insertions(+) create mode 100644 docs/ai-design/2026-02-24-sync-status-panel/manual-test-scenarios.md diff --git a/docs/ai-design/2026-02-24-sync-status-panel/manual-test-scenarios.md b/docs/ai-design/2026-02-24-sync-status-panel/manual-test-scenarios.md new file mode 100644 index 000000000..5660685c2 --- /dev/null +++ b/docs/ai-design/2026-02-24-sync-status-panel/manual-test-scenarios.md @@ -0,0 +1,436 @@ +# Manual Test Scenarios: Sync Status Panel (PR #642) + +**Feature:** Compact sync status panel on the Wallets screen showing Core and Platform sync status. +**Branch:** `zk-extract/sync-status-panel` +**Date:** 2026-02-24 + +--- + +## TS-01: Panel visibility -- no wallet selected + +### Preconditions +- Application launched and on the Wallets screen. +- No HD wallet is selected (either no wallets exist or the user has not yet clicked one). + +### Steps +1. Navigate to the Wallets screen. +2. Observe the area between the wallet selector and the wallet detail panel. + +### Expected Results +- The sync status panel is **not visible**. +- No "Core:" or "Platform:" labels appear. + +--- + +## TS-02: Panel visibility -- HD wallet selected + +### Preconditions +- At least one HD wallet is loaded in the application. + +### Steps +1. Navigate to the Wallets screen. +2. Select an HD wallet from the left panel. + +### Expected Results +- A compact panel appears between the wallet selector and the wallet detail panel. +- The panel contains two lines: + - **Line 1:** Starts with bold "Core:" label. + - **Line 2:** Starts with bold "Platform:" label. + +--- + +## TS-03: Core status -- RPC mode, connected + +### Preconditions +- Application configured in RPC mode (Dash Core wallet running and reachable). +- RPC connection is healthy. +- An HD wallet is selected. + +### Steps +1. Observe the Core line in the sync status panel. + +### Expected Results +- Core line displays: **Core: Connected** +- "Connected" text is in dark green. +- No spinner is shown. + +--- + +## TS-04: Core status -- RPC mode, disconnected + +### Preconditions +- Application configured in RPC mode. +- Dash Core wallet is stopped or unreachable. +- An HD wallet is selected. + +### Steps +1. Observe the Core line in the sync status panel. + +### Expected Results +- Core line displays: **Core: Disconnected** +- "Disconnected" text is in the error/red color. +- No spinner is shown. + +--- + +## TS-05: Core status -- SPV mode, idle/stopped + +### Preconditions +- Application configured in SPV mode. +- SPV service has not started yet or has been stopped. +- An HD wallet is selected. + +### Steps +1. Observe the Core line in the sync status panel. + +### Expected Results +- Core line displays: **Core: Disconnected** +- Text uses the secondary/muted color. + +--- + +## TS-06: Core status -- SPV mode, starting + +### Preconditions +- Application configured in SPV mode. +- SPV service is in the process of connecting (Starting state). +- An HD wallet is selected. + +### Steps +1. Watch the Core line as the SPV service initializes. + +### Expected Results +- A blue spinner appears next to the "Core:" label. +- Text reads: **Core: Connecting...** +- Text is in Dash blue color. + +--- + +## TS-07: Core status -- SPV mode, syncing (Headers phase) + +### Preconditions +- Application configured in SPV mode. +- SPV sync is actively downloading block headers (earliest sync phase). +- An HD wallet is selected. + +### Steps +1. Observe the Core line during initial header sync. + +### Expected Results +- A blue spinner is visible. +- Text reads: **Core: Syncing -- Headers NN%** where NN is a whole number 0-100. +- The percentage increases over time as headers are downloaded. + +--- + +## TS-08: Core status -- SPV mode, syncing (Filter Headers phase) + +### Preconditions +- Application configured in SPV mode. +- SPV sync has completed headers and is downloading filter headers. +- An HD wallet is selected. + +### Steps +1. Observe the Core line during filter header sync. + +### Expected Results +- A blue spinner is visible. +- Text reads: **Core: Syncing -- Filter Headers NN%** +- Percentage reflects progress of filter header download. + +--- + +## TS-09: Core status -- SPV mode, syncing (Filters phase) + +### Preconditions +- Application configured in SPV mode. +- SPV sync is downloading compact block filters. +- An HD wallet is selected. + +### Steps +1. Observe the Core line during filter sync. + +### Expected Results +- A blue spinner is visible. +- Text reads: **Core: Syncing -- Filters NN%** + +--- + +## TS-10: Core status -- SPV mode, syncing (Blocks phase) + +### Preconditions +- Application configured in SPV mode. +- SPV sync is downloading relevant blocks. +- An HD wallet is selected. + +### Steps +1. Observe the Core line during block sync. + +### Expected Results +- A blue spinner is visible. +- Text reads: **Core: Syncing -- Blocks NN%** + +--- + +## TS-11: Core status -- SPV mode, fully synced (Running) + +### Preconditions +- Application configured in SPV mode. +- SPV sync has completed and the node is running normally. +- An HD wallet is selected. + +### Steps +1. Observe the Core line after sync completes. + +### Expected Results +- No spinner is shown. +- Text reads: **Core: Synced -- N peers** where N is the number of connected peers. +- Text is in dark green. + +--- + +## TS-12: Core status -- SPV mode, stopping + +### Preconditions +- Application configured in SPV mode. +- SPV service is shutting down. +- An HD wallet is selected. + +### Steps +1. Trigger an action that stops SPV (e.g., switching networks). +2. Observe the Core line during shutdown. + +### Expected Results +- A blue spinner is visible. +- Text reads: **Core: Disconnecting...** +- Text is in Dash blue color. + +--- + +## TS-13: Core status -- SPV mode, error + +### Preconditions +- Application configured in SPV mode. +- SPV service has encountered an error. +- An HD wallet is selected. + +### Steps +1. Observe the Core line when SPV is in error state. + +### Expected Results +- Text reads: **Core: Error** +- Text is in the error/red color. +- No spinner is shown. + +--- + +## TS-14: Platform status -- wallet never synced + +### Preconditions +- An HD wallet is selected. +- The wallet has never performed a platform sync (no sync record in database, or timestamp is 0). + +### Steps +1. Observe the Platform line in the sync status panel. + +### Expected Results +- Platform line displays: **Platform: Addresses: never synced** +- Text uses the secondary/muted color. + +--- + +## TS-15: Platform status -- wallet previously synced + +### Preconditions +- An HD wallet is selected. +- The wallet has been synced with the platform at least once (database contains a non-zero sync timestamp and block height). + +### Steps +1. Observe the Platform line in the sync status panel. + +### Expected Results +- Platform line displays: **Platform: Addresses: N synced (blk H, T ago)** + - N = number of platform addresses in the wallet. + - H = the block height at which the last sync occurred. + - T = relative time since last sync (e.g., "30s ago", "5m ago", "2h ago", "1d ago"). +- Text uses the secondary/muted color. + +--- + +## TS-16: Platform status -- active refresh in progress + +### Preconditions +- An HD wallet is selected. +- A platform address balance refresh is currently in progress (the `refreshing` flag is true). + +### Steps +1. Trigger a platform refresh (e.g., click the refresh button). +2. Observe the Platform line while the refresh is running. + +### Expected Results +- A blue spinner appears on the Platform line. +- The address text (count, block height, time ago) is displayed in Dash blue instead of the secondary color. +- Once the refresh completes, the spinner disappears and text returns to secondary color. + +--- + +## TS-17: Time-ago formatting -- seconds + +### Preconditions +- A wallet has been synced very recently (less than 60 seconds ago). + +### Steps +1. Trigger a platform sync. +2. Immediately observe the Platform line after sync completes. + +### Expected Results +- The time-ago portion reads something like "5s ago", "12s ago", etc. +- The number is between 0 and 59 (inclusive). + +--- + +## TS-18: Time-ago formatting -- minutes + +### Preconditions +- A wallet was last synced between 1 and 59 minutes ago. + +### Steps +1. Observe the Platform line. + +### Expected Results +- The time-ago portion reads something like "3m ago", "45m ago". +- Uses integer division (e.g., 90 seconds shows "1m ago", not "1.5m ago"). + +--- + +## TS-19: Time-ago formatting -- hours + +### Preconditions +- A wallet was last synced between 1 and 23 hours ago. + +### Steps +1. Observe the Platform line. + +### Expected Results +- The time-ago portion reads something like "2h ago", "18h ago". + +--- + +## TS-20: Time-ago formatting -- days + +### Preconditions +- A wallet was last synced 24 hours or more ago. + +### Steps +1. Observe the Platform line. + +### Expected Results +- The time-ago portion reads something like "1d ago", "7d ago". + +--- + +## TS-21: Wallet switching updates the panel + +### Preconditions +- Two or more HD wallets are loaded. +- Wallet A has been synced recently (has platform sync info). +- Wallet B has never been synced (no platform sync info). + +### Steps +1. Select Wallet A from the left panel. +2. Observe the sync status panel -- note the platform sync info (address count, block height, time ago). +3. Switch to Wallet B by clicking it in the left panel. +4. Observe the sync status panel. + +### Expected Results +- After step 2: Platform line shows address count, block height, and time-ago for Wallet A. +- After step 4: Platform line updates to show "Addresses: never synced" for Wallet B. +- The Core line remains unchanged (it reflects the global core connection, not per-wallet state). + +--- + +## TS-22: Panel respects light and dark mode + +### Preconditions +- An HD wallet is selected. +- The application supports theme switching. + +### Steps +1. Switch the application to dark mode. +2. Observe the sync status panel colors and contrast. +3. Switch the application to light mode. +4. Observe the sync status panel colors and contrast. + +### Expected Results +- In dark mode: panel background uses the dark surface color; "Core:" and "Platform:" labels use the dark-mode primary text color; secondary text is readable against the dark background. +- In light mode: panel background uses the light surface color; labels and secondary text are readable against the light background. +- Status colors (dark green for connected/synced, red for error/disconnected, blue for syncing) remain visually distinct in both modes. + +--- + +## TS-23: SPV sync phase progression + +### Preconditions +- Application configured in SPV mode. +- Starting from a fresh state (no previously synced data) so that all four sync phases are traversed. +- An HD wallet is selected. + +### Steps +1. Start the application and let SPV sync begin. +2. Continuously monitor the Core line throughout the entire sync process. + +### Expected Results +- The Core line progresses through these states in order: + 1. "Connecting..." + 2. "Syncing -- Headers NN%" (percentage climbs from low to 100) + 3. "Syncing -- Filter Headers NN%" + 4. "Syncing -- Filters NN%" + 5. "Syncing -- Blocks NN%" + 6. "Synced -- N peers" +- Each phase percentage starts low and progresses upward. +- A spinner is visible during all syncing phases and disappears when fully synced. + +--- + +## TS-24: SPV sync progress -- target height zero + +### Preconditions +- Application configured in SPV mode. +- SPV sync is in early startup where the target height may not yet be known (target = 0). + +### Steps +1. Observe the Core line during the very first moments of sync. + +### Expected Results +- If a phase has target_height = 0, the progress displays "0%" (not a division-by-zero crash or NaN). +- The panel remains stable and does not flicker or show garbled text. + +--- + +## TS-25: Database refactor -- shared_connection removal + +### Preconditions +- This is a code-level verification, not a UI test. + +### Steps +1. Verify the application compiles without errors (`cargo build`). +2. Run the full test suite (`cargo test --all-features --workspace`). +3. Search the codebase for any remaining calls to `Database::shared_connection()`. + +### Expected Results +- The application compiles successfully. +- All tests pass. +- No remaining references to `shared_connection()` exist in the codebase (the method was removed and `Database.conn` changed from `Arc>` to `Mutex`). + +--- + +## Edge Cases + +| # | Scenario | Expected Behavior | +|---|----------|-------------------| +| E1 | System clock is set to the past (before the last sync timestamp) | `format_unix_time_ago` uses `saturating_sub`, so it shows "0s ago" rather than panicking or showing negative time. | +| E2 | SPV sync progress is `None` while status is `Syncing` | Phase text falls back to "starting..." rather than crashing. | +| E3 | Wallet read lock fails (poisoned RwLock) | Address count defaults to 0; platform sync info shows as "never synced". The panel does not crash. | +| E4 | Very large block heights or peer counts | Numbers render as plain integers without overflow; the panel layout adjusts to accommodate wider text. | +| E5 | Rapidly switching between wallets | The `platform_sync_info` cache is refreshed on each selection; no stale data from the previous wallet is shown. | +| E6 | Single-key wallet selected (not HD) | The sync status panel is hidden (it only renders when `selected_wallet` is `Some`, which is for HD wallets only). | From b1a6c01efe0a1b0d0325f654ba7620494a9f0fd8 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 11:24:09 +0000 Subject: [PATCH 05/17] Initial plan From 2740af19b85f87adf9cae0b113d27880aec2a016 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Tue, 24 Feb 2026 11:49:45 +0000 Subject: [PATCH 06/17] fix(wallet): defer UTXO removal until asset lock tx is fully built and signed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, `asset_lock_transaction_from_private_key` called `take_unspent_utxos_for` which immediately removed selected UTXOs from `Wallet.utxos`. Since fee recalculation and signing happen afterward, any failure at those steps (fee shortfall, missing private key, change address derivation error) would permanently drop UTXOs — especially dangerous in SPV mode where there is no Core RPC reload fallback. Fix: - Add `select_unspent_utxos_for` (`&self`, non-mutating) that performs the same UTXO selection logic without removing anything. - Add `remove_selected_utxos` (`&mut self`) for explicit removal. - Refactor `take_unspent_utxos_for` to delegate to these two methods (no behavior change for existing callers). - In `asset_lock_transaction_from_private_key`, use `select_unspent_utxos_for` for selection and only call `remove_selected_utxos` after the full tx is built and signed. Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> --- src/model/wallet/asset_lock_transaction.rs | 12 ++- src/model/wallet/utxos.rs | 87 ++++++++++------------ 2 files changed, 51 insertions(+), 48 deletions(-) diff --git a/src/model/wallet/asset_lock_transaction.rs b/src/model/wallet/asset_lock_transaction.rs index a189e5011..f149e3aa3 100644 --- a/src/model/wallet/asset_lock_transaction.rs +++ b/src/model/wallet/asset_lock_transaction.rs @@ -150,9 +150,14 @@ impl Wallet { // below based on the actual number of inputs. let initial_fee_estimate = 3_000u64; + // Select UTXOs without committing the removal yet. UTXOs are only removed + // from the wallet after the transaction is fully built and signed, so that a + // failure at any later step (fee shortfall, missing private key, …) cannot + // permanently drop UTXOs from the wallet — especially important in SPV mode + // where there is no Core RPC reload fallback. let (utxos, initial_change_option) = self - .take_unspent_utxos_for(amount, initial_fee_estimate, allow_take_fee_from_amount) - .ok_or("take_unspent_utxos_for() returned None".to_string())?; + .select_unspent_utxos_for(amount, initial_fee_estimate, allow_take_fee_from_amount) + .ok_or("Insufficient funds to cover amount and fee".to_string())?; // Calculate fee based on actual transaction size so we always meet the // min relay fee (1 duff/byte = 1000 duffs/kB). @@ -302,6 +307,9 @@ impl Wallet { Ok::<(), String>(()) })?; + // Transaction is fully built and signed; commit the UTXO removals now. + self.remove_selected_utxos(&utxos); + Ok((tx, private_key, change_address, utxos)) } diff --git a/src/model/wallet/utxos.rs b/src/model/wallet/utxos.rs index 55f07f0b8..3453edac1 100644 --- a/src/model/wallet/utxos.rs +++ b/src/model/wallet/utxos.rs @@ -6,89 +6,84 @@ use dash_sdk::dpp::dashcore::{Address, Network, OutPoint, TxOut}; use std::collections::{BTreeMap, HashMap, HashSet}; impl Wallet { + /// Selects UTXOs sufficient to cover `amount + fee` without removing them from the wallet. + /// + /// Returns the selected UTXOs and an optional change amount, or `None` if there are + /// insufficient funds. Use this when you need to inspect or validate the selection before + /// committing; call [`Self::take_unspent_utxos_for`] (or remove the UTXOs manually) only + /// once the operation is guaranteed to succeed. #[allow(clippy::type_complexity)] - pub fn take_unspent_utxos_for( - &mut self, + pub fn select_unspent_utxos_for( + &self, amount: u64, fee: u64, allow_take_fee_from_amount: bool, ) -> Option<(BTreeMap, Option)> { - // Ensure UTXOs exist - let utxos = &mut self.utxos; - let mut required: i64 = (amount + fee) as i64; - let mut taken_utxos = BTreeMap::new(); - let mut utxos_to_remove = Vec::new(); + let mut selected_utxos = BTreeMap::new(); - // Iterate over the UTXOs to collect enough to cover the required amount - for (address, outpoints) in utxos.iter_mut() { + for (address, outpoints) in self.utxos.iter() { for (outpoint, tx_out) in outpoints.iter() { if required <= 0 { break; } - - // Add the UTXO to the result - taken_utxos.insert(*outpoint, (tx_out.clone(), address.clone())); - + selected_utxos.insert(*outpoint, (tx_out.clone(), address.clone())); required -= tx_out.value as i64; - utxos_to_remove.push((address.clone(), *outpoint)); } } - // If not enough UTXOs were found, try to adjust if allowed if required > 0 { if allow_take_fee_from_amount { let total_collected = (amount + fee) as i64 - required; if total_collected >= amount as i64 { - // We have enough to cover the amount, but not the fee - // So we can reduce the amount by the missing fee let missing_fee = required; // required > 0 let adjusted_amount = amount as i64 - missing_fee; if adjusted_amount <= 0 { - // Cannot adjust amount to cover missing fee return None; } - // Remove UTXOs from wallet - for (address, outpoint) in utxos_to_remove { - if let Some(outpoints) = utxos.get_mut(&address) { - outpoints.remove(&outpoint); - if outpoints.is_empty() { - utxos.remove(&address); - } - } - } - // Return collected UTXOs and None for change - Some((taken_utxos, None)) + Some((selected_utxos, None)) } else { - // Not enough to cover amount even after adjusting None } } else { - // Not enough UTXOs and not allowed to take fee from amount None } } else { - // Remove the collected UTXOs from the wallet's UTXO map - for (address, outpoint) in utxos_to_remove { - if let Some(outpoints) = utxos.get_mut(&address) { - outpoints.remove(&outpoint); - if outpoints.is_empty() { - utxos.remove(&address); - } - } - } - // Calculate change amount - let total_input = (amount + fee) as i64 - required; // total input collected + let total_input = (amount + fee) as i64 - required; let change = total_input as u64 - amount - fee; - - // If change is zero, return None let change_option = if change > 0 { Some(change) } else { None }; + Some((selected_utxos, change_option)) + } + } - // Return the collected UTXOs and the change amount - Some((taken_utxos, change_option)) + /// Removes UTXOs previously returned by [`Self::select_unspent_utxos_for`] from the wallet. + pub fn remove_selected_utxos( + &mut self, + selected: &BTreeMap, + ) { + for (outpoint, (_, address)) in selected { + if let Some(outpoints) = self.utxos.get_mut(address) { + outpoints.remove(outpoint); + if outpoints.is_empty() { + self.utxos.remove(address); + } + } } } + #[allow(clippy::type_complexity)] + pub fn take_unspent_utxos_for( + &mut self, + amount: u64, + fee: u64, + allow_take_fee_from_amount: bool, + ) -> Option<(BTreeMap, Option)> { + let (selected, change_option) = + self.select_unspent_utxos_for(amount, fee, allow_take_fee_from_amount)?; + self.remove_selected_utxos(&selected); + Some((selected, change_option)) + } + pub fn reload_utxos( &mut self, core_client: &Client, From 497bf21373da9df56f5b4ad98d079973173485b6 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 15:17:12 +0100 Subject: [PATCH 07/17] refactor(wallet): consolidate UTXO removal, DB persistence, and balance recalc into remove_selected_utxos MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, every backend task caller had to manually: (1) remove UTXOs from the in-memory map, (2) drop them from the database, and (3) recalculate affected address balances. This was error-prone — the payment transaction builders were missing the balance recalculation entirely. Now `remove_selected_utxos` accepts an optional `&AppContext` and handles all three steps atomically. The redundant cleanup blocks in 5 backend task callers are removed. Also applies the safe select-then-commit UTXO pattern to `build_standard_payment_transaction` and `build_multi_recipient_payment_transaction`, fixing the same UTXO-loss-on-signing-failure bug that was previously fixed only for asset lock transactions. Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 10 ++++ src/backend_task/core/create_asset_lock.rs | 40 +------------- .../identity/register_identity.rs | 22 +------- src/backend_task/identity/top_up_identity.rs | 23 +------- ...fund_platform_address_from_wallet_utxos.rs | 29 +---------- src/model/wallet/asset_lock_transaction.rs | 2 +- src/model/wallet/mod.rs | 16 +++++- src/model/wallet/utxos.rs | 52 ++++++++++++++++--- 8 files changed, 77 insertions(+), 117 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index b8b7b30eb..3fe2179bb 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -56,6 +56,16 @@ scripts/safe-cargo.sh +nightly fmt --all ``` +## Coding Conventions + +### Parameter ordering + +When a method takes `&AppContext` (or `Option<&AppContext>`), place it as the first parameter after `self`. Example: + +```rust +fn remove_selected_utxos(&mut self, context: Option<&AppContext>, selected: &BTreeMap<...>) -> Result<(), String> +``` + ## Architecture Overview **Dash Evo Tool** is a cross-platform GUI application (Rust + egui) for interacting with Dash Evolution. It enables DPNS username registration, contest voting, state transition viewing, wallet management, and identity operations across Mainnet/Testnet/Devnet. diff --git a/src/backend_task/core/create_asset_lock.rs b/src/backend_task/core/create_asset_lock.rs index 5111d83dd..4b4b65f11 100644 --- a/src/backend_task/core/create_asset_lock.rs +++ b/src/backend_task/core/create_asset_lock.rs @@ -17,7 +17,7 @@ impl AppContext { let amount_duffs = amount / CREDITS_PER_DUFF; // Create the asset lock transaction - let (asset_lock_transaction, _private_key, _change_address, used_utxos) = { + let (asset_lock_transaction, _private_key, _change_address, _used_utxos) = { let mut wallet_guard = wallet.write().map_err(|e| e.to_string())?; wallet_guard.registration_asset_lock_transaction( @@ -42,24 +42,6 @@ impl AppContext { .await .map_err(|e| format!("Failed to broadcast asset lock transaction: {}", e))?; - // Update wallet UTXOs - { - let mut wallet_guard = wallet.write().map_err(|e| e.to_string())?; - wallet_guard.utxos.retain(|_, utxo_map| { - utxo_map.retain(|outpoint, _| !used_utxos.contains_key(outpoint)); - !utxo_map.is_empty() // Keep addresses that still have UTXOs - }); - - // Drop used UTXOs from database - for utxo in used_utxos.keys() { - self.db - .drop_utxo(utxo, &self.network.to_string()) - .map_err(|e| e.to_string())?; - } - - wallet_guard.recalculate_affected_address_balances(&used_utxos, self)?; - } - Ok(BackendTaskSuccessResult::Message(format!( "Asset lock transaction broadcast successfully. TX ID: {}", tx_id @@ -78,7 +60,7 @@ impl AppContext { let amount_duffs = amount / CREDITS_PER_DUFF; // Create the asset lock transaction - let (asset_lock_transaction, _private_key, _change_address, used_utxos) = { + let (asset_lock_transaction, _private_key, _change_address, _used_utxos) = { let mut wallet_guard = wallet.write().map_err(|e| e.to_string())?; wallet_guard.top_up_asset_lock_transaction( @@ -104,24 +86,6 @@ impl AppContext { .await .map_err(|e| format!("Failed to broadcast asset lock transaction: {}", e))?; - // Update wallet UTXOs - { - let mut wallet_guard = wallet.write().map_err(|e| e.to_string())?; - wallet_guard.utxos.retain(|_, utxo_map| { - utxo_map.retain(|outpoint, _| !used_utxos.contains_key(outpoint)); - !utxo_map.is_empty() // Keep addresses that still have UTXOs - }); - - // Drop used UTXOs from database - for utxo in used_utxos.keys() { - self.db - .drop_utxo(utxo, &self.network.to_string()) - .map_err(|e| e.to_string())?; - } - - wallet_guard.recalculate_affected_address_balances(&used_utxos, self)?; - } - Ok(BackendTaskSuccessResult::Message(format!( "Asset lock transaction broadcast successfully. TX ID: {}", tx_id diff --git a/src/backend_task/identity/register_identity.rs b/src/backend_task/identity/register_identity.rs index 47ca59b7d..37b88c7ab 100644 --- a/src/backend_task/identity/register_identity.rs +++ b/src/backend_task/identity/register_identity.rs @@ -91,7 +91,7 @@ impl AppContext { } RegisterIdentityFundingMethod::FundWithWallet(amount, identity_index) => { // Scope the write lock to avoid holding it across an await. - let (asset_lock_transaction, asset_lock_proof_private_key, _, used_utxos) = { + let (asset_lock_transaction, asset_lock_proof_private_key, _, _used_utxos) = { let mut wallet = wallet.write().unwrap(); wallet_id = wallet.seed_hash(); match wallet.registration_asset_lock_transaction( @@ -142,26 +142,6 @@ impl AppContext { ) .map_err(|e| format!("Failed to store asset lock transaction: {}", e))?; - // TODO: UTXO removal timing issue - UTXOs are removed here BEFORE the asset - // lock proof is confirmed below. If the transaction fails or times out after - // this point, the UTXOs will be "lost" from wallet tracking even though they - // weren't actually spent. This should be refactored to remove UTXOs only AFTER - // successful proof confirmation. See Phase 2.2 in PR review plan. - { - let mut wallet = wallet.write().unwrap(); - wallet.utxos.retain(|_, utxo_map| { - utxo_map.retain(|outpoint, _| !used_utxos.contains_key(outpoint)); - !utxo_map.is_empty() // Keep addresses that still have UTXOs - }); - for utxo in used_utxos.keys() { - self.db - .drop_utxo(utxo, &self.network.to_string()) - .map_err(|e| e.to_string())?; - } - - wallet.recalculate_affected_address_balances(&used_utxos, self)?; - } - let asset_lock_proof = self.wait_for_asset_lock_proof(tx_id).await?; (asset_lock_proof, asset_lock_proof_private_key, tx_id) diff --git a/src/backend_task/identity/top_up_identity.rs b/src/backend_task/identity/top_up_identity.rs index d01fe3076..3ea349e0b 100644 --- a/src/backend_task/identity/top_up_identity.rs +++ b/src/backend_task/identity/top_up_identity.rs @@ -96,7 +96,7 @@ impl AppContext { asset_lock_transaction, asset_lock_proof_private_key, _, - used_utxos, + _used_utxos, wallet_seed_hash, ) = { let mut wallet = wallet.write().unwrap(); @@ -136,12 +136,6 @@ impl AppContext { }; let tx_id = asset_lock_transaction.txid(); - // todo: maybe one day we will want to use platform again, but for right now we use - // the local core as it is more stable - // let asset_lock_proof = self - // .broadcast_and_retrieve_asset_lock(&asset_lock_transaction, &change_address) - // .await - // .map_err(|e| e.to_string())?; { let mut proofs = self.transactions_waiting_for_finality.lock().unwrap(); @@ -164,21 +158,6 @@ impl AppContext { ) .map_err(|e| format!("Failed to store asset lock transaction: {}", e))?; - { - let mut wallet = wallet.write().unwrap(); - wallet.utxos.retain(|_, utxo_map| { - utxo_map.retain(|outpoint, _| !used_utxos.contains_key(outpoint)); - !utxo_map.is_empty() // Keep addresses that still have UTXOs - }); - for utxo in used_utxos.keys() { - self.db - .drop_utxo(utxo, &self.network.to_string()) - .map_err(|e| e.to_string())?; - } - - wallet.recalculate_affected_address_balances(&used_utxos, self)?; - } - let asset_lock_proof = self.wait_for_asset_lock_proof(tx_id).await?; ( diff --git a/src/backend_task/wallet/fund_platform_address_from_wallet_utxos.rs b/src/backend_task/wallet/fund_platform_address_from_wallet_utxos.rs index ed646881b..a4406a2a5 100644 --- a/src/backend_task/wallet/fund_platform_address_from_wallet_utxos.rs +++ b/src/backend_task/wallet/fund_platform_address_from_wallet_utxos.rs @@ -40,7 +40,7 @@ impl AppContext { }; // Step 1: Create the asset lock transaction - let (asset_lock_transaction, asset_lock_private_key, _asset_lock_address, used_utxos) = { + let (asset_lock_transaction, asset_lock_private_key, _asset_lock_address, _used_utxos) = { let wallet_arc = { let wallets = self.wallets.read().unwrap(); wallets @@ -117,32 +117,7 @@ impl AppContext { return Err(format!("Failed to broadcast asset lock transaction: {}", e)); } - // Step 5: Remove used UTXOs from wallet - { - let wallet_arc = { - let wallets = self.wallets.read().unwrap(); - wallets - .get(&seed_hash) - .cloned() - .ok_or_else(|| "Wallet not found".to_string())? - }; - - let mut wallet = wallet_arc.write().map_err(|e| e.to_string())?; - wallet.utxos.retain(|_, utxo_map| { - utxo_map.retain(|outpoint, _| !used_utxos.contains_key(outpoint)); - !utxo_map.is_empty() - }); - - for utxo in used_utxos.keys() { - self.db - .drop_utxo(utxo, &self.network.to_string()) - .map_err(|e| e.to_string())?; - } - - wallet.recalculate_affected_address_balances(&used_utxos, self)?; - } - - // Step 6: Wait for asset lock proof (InstantLock or ChainLock) via shared helper. + // Step 5: Wait for asset lock proof (InstantLock or ChainLock) via shared helper. // On timeout the helper cleans up the finality tracking entry. // Post-timeout recovery is mode-dependent: // RPC — fire-and-forget refresh_wallet_info to reconcile spent UTXOs diff --git a/src/model/wallet/asset_lock_transaction.rs b/src/model/wallet/asset_lock_transaction.rs index 095d760bf..99986c048 100644 --- a/src/model/wallet/asset_lock_transaction.rs +++ b/src/model/wallet/asset_lock_transaction.rs @@ -395,7 +395,7 @@ impl Wallet { })?; // Transaction is fully built and signed; commit the UTXO removals now. - self.remove_selected_utxos(&utxos); + self.remove_selected_utxos(register_addresses, &utxos)?; Ok((tx, private_key, change_address, utxos)) } diff --git a/src/model/wallet/mod.rs b/src/model/wallet/mod.rs index fe6cf613f..33928d3a9 100644 --- a/src/model/wallet/mod.rs +++ b/src/model/wallet/mod.rs @@ -1557,8 +1557,11 @@ impl Wallet { )); } + // Select UTXOs without removing them yet — UTXOs are only removed after + // the transaction is fully built and signed, so that a failure at any later + // step cannot permanently drop UTXOs from the wallet. let (utxos, change_option) = self - .take_unspent_utxos_for(amount, fee, subtract_fee_from_amount) + .select_unspent_utxos_for(amount, fee, subtract_fee_from_amount) .ok_or_else(|| "Insufficient funds".to_string())?; let send_value = if change_option.is_none() && subtract_fee_from_amount { @@ -1649,6 +1652,9 @@ impl Wallet { Ok::<(), String>(()) })?; + // Transaction is fully built and signed; commit the UTXO removals now. + self.remove_selected_utxos(register_addresses, &utxos)?; + Ok(tx) } @@ -1679,8 +1685,11 @@ impl Wallet { // Calculate total amount needed let total_amount: u64 = recipients.iter().map(|(_, amount)| *amount).sum(); + // Select UTXOs without removing them yet — UTXOs are only removed after + // the transaction is fully built and signed, so that a failure at any later + // step cannot permanently drop UTXOs from the wallet. let (utxos, change_option) = self - .take_unspent_utxos_for(total_amount, fee, subtract_fee_from_amount) + .select_unspent_utxos_for(total_amount, fee, subtract_fee_from_amount) .ok_or_else(|| "Insufficient funds".to_string())?; // Build outputs for each recipient @@ -1790,6 +1799,9 @@ impl Wallet { Ok::<(), String>(()) })?; + // Transaction is fully built and signed; commit the UTXO removals now. + self.remove_selected_utxos(register_addresses, &utxos)?; + Ok(tx) } diff --git a/src/model/wallet/utxos.rs b/src/model/wallet/utxos.rs index d727d2b3d..eab164038 100644 --- a/src/model/wallet/utxos.rs +++ b/src/model/wallet/utxos.rs @@ -10,9 +10,13 @@ impl Wallet { /// Selects UTXOs sufficient to cover `amount + fee` without removing them from the wallet. /// /// Returns the selected UTXOs and an optional change amount, or `None` if there are - /// insufficient funds. Use this when you need to inspect or validate the selection before - /// committing; call [`Self::take_unspent_utxos_for`] (or remove the UTXOs manually) only - /// once the operation is guaranteed to succeed. + /// insufficient funds. The returned change value is computed from the caller-supplied + /// `fee` parameter; callers that recalculate fees afterward (e.g., based on the actual + /// number of inputs) should ignore the change value and recompute it. + /// + /// Use this when you need to inspect or validate the selection before + /// committing; call [`Self::remove_selected_utxos`] only once the operation + /// is guaranteed to succeed. #[allow(clippy::type_complexity)] pub fn select_unspent_utxos_for( &self, @@ -57,21 +61,55 @@ impl Wallet { } } - /// Removes UTXOs previously returned by [`Self::select_unspent_utxos_for`] from the wallet. + /// Removes the given UTXOs from the wallet's in-memory UTXO set, persists + /// the removal to the database, and recalculates affected address balances. + /// + /// Typically called with the result of [`Self::select_unspent_utxos_for`] + /// after the transaction has been fully built and signed. + /// + /// When `context` is `None`, only the in-memory state is updated (useful + /// for tests or when no database persistence is needed). pub fn remove_selected_utxos( &mut self, + context: Option<&AppContext>, selected: &BTreeMap, - ) { + ) -> Result<(), String> { for (outpoint, (_, address)) in selected { if let Some(outpoints) = self.utxos.get_mut(address) { outpoints.remove(outpoint); if outpoints.is_empty() { self.utxos.remove(address); } + } else { + tracing::debug!( + ?outpoint, + %address, + "remove_selected_utxos: outpoint not found in wallet, skipping" + ); } } + + if let Some(context) = context { + // Drop used UTXOs from database + for outpoint in selected.keys() { + context + .db + .drop_utxo(outpoint, &context.network.to_string()) + .map_err(|e| e.to_string())?; + } + + // Recalculate balances for affected addresses + self.recalculate_affected_address_balances(selected, context)?; + } + + Ok(()) } + /// Convenience wrapper: selects and immediately removes UTXOs (in-memory only). + /// + /// Used in tests and contexts without database persistence. Production code + /// should prefer [`Self::select_unspent_utxos_for`] followed by + /// [`Self::remove_selected_utxos`] with a context. #[allow(clippy::type_complexity)] pub fn take_unspent_utxos_for( &mut self, @@ -81,7 +119,9 @@ impl Wallet { ) -> Option<(BTreeMap, Option)> { let (selected, change_option) = self.select_unspent_utxos_for(amount, fee, allow_take_fee_from_amount)?; - self.remove_selected_utxos(&selected); + // In-memory only — no context for DB persistence. + self.remove_selected_utxos(None, &selected) + .expect("remove_selected_utxos without context cannot fail"); Some((selected, change_option)) } From 333cb82f134847c817c2f87e3593e3bc495d8f84 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 15:34:56 +0100 Subject: [PATCH 08/17] fix(wallet): address audit findings from PR #645 review - Add checked arithmetic to UTXO selection (amount + fee overflow safety) - Replace hardcoded fee in single-UTXO path with calculate_asset_lock_fee - Add UTXO selection retry when real fee exceeds initial estimate - Document write-lock invariant on select_unspent_utxos_for - Replace .unwrap() with .map_err() on wallet write locks - Restrict Database::shared_connection visibility to pub(crate) Co-Authored-By: Claude Opus 4.6 --- .../identity/register_identity.rs | 2 +- src/backend_task/identity/top_up_identity.rs | 2 +- src/database/mod.rs | 3 +- src/model/wallet/asset_lock_transaction.rs | 96 ++++++++++++++----- src/model/wallet/utxos.rs | 12 ++- 5 files changed, 83 insertions(+), 32 deletions(-) diff --git a/src/backend_task/identity/register_identity.rs b/src/backend_task/identity/register_identity.rs index 37b88c7ab..8e9221802 100644 --- a/src/backend_task/identity/register_identity.rs +++ b/src/backend_task/identity/register_identity.rs @@ -92,7 +92,7 @@ impl AppContext { RegisterIdentityFundingMethod::FundWithWallet(amount, identity_index) => { // Scope the write lock to avoid holding it across an await. let (asset_lock_transaction, asset_lock_proof_private_key, _, _used_utxos) = { - let mut wallet = wallet.write().unwrap(); + let mut wallet = wallet.write().map_err(|e| e.to_string())?; wallet_id = wallet.seed_hash(); match wallet.registration_asset_lock_transaction( sdk.network, diff --git a/src/backend_task/identity/top_up_identity.rs b/src/backend_task/identity/top_up_identity.rs index 3ea349e0b..48ef5621b 100644 --- a/src/backend_task/identity/top_up_identity.rs +++ b/src/backend_task/identity/top_up_identity.rs @@ -99,7 +99,7 @@ impl AppContext { _used_utxos, wallet_seed_hash, ) = { - let mut wallet = wallet.write().unwrap(); + let mut wallet = wallet.write().map_err(|e| e.to_string())?; let seed_hash = wallet.seed_hash(); let tx_result = match wallet.top_up_asset_lock_transaction( sdk.network, diff --git a/src/database/mod.rs b/src/database/mod.rs index 13d459641..0205ec489 100644 --- a/src/database/mod.rs +++ b/src/database/mod.rs @@ -56,7 +56,8 @@ impl Database { /// /// Used by `ClientPersistentCommitmentTree` to share the same SQLite /// connection for the shielded commitment tree tables. - pub fn shared_connection(&self) -> Arc> { + #[allow(dead_code)] // Prepared for ClientPersistentCommitmentTree + pub(crate) fn shared_connection(&self) -> Arc> { self.conn.clone() } diff --git a/src/model/wallet/asset_lock_transaction.rs b/src/model/wallet/asset_lock_transaction.rs index 99986c048..9836419fb 100644 --- a/src/model/wallet/asset_lock_transaction.rs +++ b/src/model/wallet/asset_lock_transaction.rs @@ -110,6 +110,56 @@ pub(crate) fn calculate_asset_lock_fee( } impl Wallet { + /// Select UTXOs and compute fee, retrying with the real fee if the initial + /// estimate was too low and additional UTXOs are available. + #[allow(clippy::type_complexity)] + fn select_utxos_with_fee_retry( + &self, + amount: u64, + allow_take_fee_from_amount: bool, + ) -> Result<(BTreeMap, AssetLockFeeResult), String> { + let mut fee_estimate = MIN_ASSET_LOCK_FEE; + + for _ in 0..2 { + let (utxos, _) = self + .select_unspent_utxos_for(amount, fee_estimate, allow_take_fee_from_amount) + .ok_or_else(|| { + format!( + "Not enough spendable funds to create asset lock transaction: \ + requested amount {} plus estimated fee {}", + amount, fee_estimate + ) + })?; + + let total_input_value: u64 = utxos.iter().map(|(_, (tx_out, _))| tx_out.value).sum(); + let num_inputs = utxos.len(); + + match calculate_asset_lock_fee( + total_input_value, + amount, + num_inputs, + allow_take_fee_from_amount, + ) { + Ok(fee_result) => return Ok((utxos, fee_result)), + Err(_) if fee_estimate == MIN_ASSET_LOCK_FEE => { + // The real fee may exceed our initial estimate. Recompute + // with a 2-output size estimate and retry UTXO selection so + // we can pick up any additional marginal UTXOs. + fee_estimate = + std::cmp::max(MIN_ASSET_LOCK_FEE, estimate_tx_size(num_inputs, 2) as u64); + continue; + } + Err(e) => return Err(e), + } + } + + Err(format!( + "Not enough spendable funds to create asset lock transaction: \ + requested amount {} plus fee {}", + amount, fee_estimate + )) + } + #[allow(clippy::type_complexity)] pub fn registration_asset_lock_transaction( &mut self, @@ -244,35 +294,21 @@ impl Wallet { let one_time_key_hash = asset_lock_public_key.pubkey_hash(); - // Use a small initial estimate to select UTXOs; we recalculate the fee - // below based on the actual number of inputs. - let initial_fee_estimate = MIN_ASSET_LOCK_FEE; - // Select UTXOs without committing the removal yet. UTXOs are only removed // from the wallet after the transaction is fully built and signed, so that a // failure at any later step (fee shortfall, missing private key, …) cannot // permanently drop UTXOs from the wallet — especially important in SPV mode // where there is no Core RPC reload fallback. - let (utxos, _initial_change_option) = self - .select_unspent_utxos_for(amount, initial_fee_estimate, allow_take_fee_from_amount) - .ok_or_else(|| { - format!( - "Not enough spendable funds to create asset lock transaction: \ - requested amount {} plus estimated fee {}", - amount, initial_fee_estimate - ) - })?; - - // Calculate fee, amount, and change using the extracted pure function. - let total_input_value: u64 = utxos.iter().map(|(_, (tx_out, _))| tx_out.value).sum(); - let num_inputs = utxos.len(); - - let fee_result = calculate_asset_lock_fee( - total_input_value, - amount, - num_inputs, - allow_take_fee_from_amount, - )?; + // + // Note: `&mut self` ensures exclusive access during the entire select → build + // → sign → remove sequence, preventing concurrent UTXO double-selection. + // + // We use an initial fee estimate for UTXO selection, then recalculate the + // real fee based on input count. If the real fee exceeds the estimate and + // the selected UTXOs are insufficient, we retry once with the computed fee + // so that marginal UTXOs are not missed. + let (utxos, fee_result) = + self.select_utxos_with_fee_retry(amount, allow_take_fee_from_amount)?; let actual_amount = fee_result.actual_amount; let change_option = fee_result.change; @@ -461,8 +497,16 @@ impl Wallet { let asset_lock_public_key = private_key.public_key(&secp); let one_time_key_hash = asset_lock_public_key.pubkey_hash(); - let fee = 3_000; - let output_amount = previous_tx_output.value - fee; + + // Single-UTXO path: use calculate_asset_lock_fee for consistency with + // the multi-input path, ensuring dust check and overflow safety. + let fee_result = calculate_asset_lock_fee( + previous_tx_output.value, + previous_tx_output.value.saturating_sub(MIN_ASSET_LOCK_FEE), + 1, // single input + true, // take fee from amount since the UTXO *is* the total + )?; + let output_amount = fee_result.actual_amount; let payload_output = TxOut { value: output_amount, diff --git a/src/model/wallet/utxos.rs b/src/model/wallet/utxos.rs index eab164038..6de622abc 100644 --- a/src/model/wallet/utxos.rs +++ b/src/model/wallet/utxos.rs @@ -17,6 +17,11 @@ impl Wallet { /// Use this when you need to inspect or validate the selection before /// committing; call [`Self::remove_selected_utxos`] only once the operation /// is guaranteed to succeed. + /// + /// **Important:** The caller must hold the wallet write lock (`&mut self` on `Wallet`) + /// continuously from this call through the corresponding [`Self::remove_selected_utxos`] + /// call. Dropping the lock between selection and removal would allow a concurrent + /// caller to select the same UTXOs, creating a double-spend. #[allow(clippy::type_complexity)] pub fn select_unspent_utxos_for( &self, @@ -24,7 +29,8 @@ impl Wallet { fee: u64, allow_take_fee_from_amount: bool, ) -> Option<(BTreeMap, Option)> { - let mut required: i64 = (amount + fee) as i64; + let target = amount.checked_add(fee)?; + let mut required: i64 = i64::try_from(target).ok()?; let mut selected_utxos = BTreeMap::new(); for (address, outpoints) in self.utxos.iter() { @@ -39,7 +45,7 @@ impl Wallet { if required > 0 { if allow_take_fee_from_amount { - let total_collected = (amount + fee) as i64 - required; + let total_collected = target as i64 - required; if total_collected >= amount as i64 { let missing_fee = required; // required > 0 let adjusted_amount = amount as i64 - missing_fee; @@ -54,7 +60,7 @@ impl Wallet { None } } else { - let total_input = (amount + fee) as i64 - required; + let total_input = target as i64 - required; let change = total_input as u64 - amount - fee; let change_option = if change > 0 { Some(change) } else { None }; Some((selected_utxos, change_option)) From 6159ef9d65e244280756147ed359ea85e66d78c0 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 15:44:24 +0100 Subject: [PATCH 09/17] chore: minimal improvement in conn status tooltip --- src/context/connection_status.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/src/context/connection_status.rs b/src/context/connection_status.rs index e4bd8cca3..2cb4f175f 100644 --- a/src/context/connection_status.rs +++ b/src/context/connection_status.rs @@ -169,7 +169,7 @@ impl ConnectionStatus { if total == 0 { "No endpoints configured".to_string() } else if available > 0 { - format!("Available ({available}/{total} endpoints)") + format!("Available ({available} unbanned / {total} total endpoints)") } else { format!("All {total} endpoints banned") } @@ -250,12 +250,12 @@ impl ConnectionStatus { format!("{header}\n{rpc_status}\n{zmq_status}\n{dapi_status}") } CoreBackendMode::Spv => { - let spv_label = format!("SPV: {:?}", spv_status); let header = match overall { - OverallConnectionState::Synced => "SPV synced", - OverallConnectionState::Syncing => "SPV syncing", - OverallConnectionState::Disconnected => "SPV disconnected", + OverallConnectionState::Synced => "Ready", + OverallConnectionState::Syncing => "Syncing", + OverallConnectionState::Disconnected => "Disconnected", }; + let spv_label = format!("SPV: {:?}", spv_status); format!("{header}\n{spv_label}\n{dapi_status}") } } From 0b7d7ab3e61f7335a6763ef6ff0d27dd5abf63ce Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 16:30:24 +0100 Subject: [PATCH 10/17] refactor(wallet): simplify remove_selected_utxos to take &Database + Network directly Replace Option<&AppContext> with concrete dependencies (&Database, Network), removing the need for take_unspent_utxos_for. Extract balance recalculation into a private helper reused by both remove_selected_utxos and the existing AppContext-based wrapper. Co-Authored-By: Claude Opus 4.6 --- src/model/wallet/asset_lock_transaction.rs | 4 +- src/model/wallet/mod.rs | 167 ++++++++++++++------- src/model/wallet/utxos.rs | 49 ++---- 3 files changed, 128 insertions(+), 92 deletions(-) diff --git a/src/model/wallet/asset_lock_transaction.rs b/src/model/wallet/asset_lock_transaction.rs index 9836419fb..2ad56b61f 100644 --- a/src/model/wallet/asset_lock_transaction.rs +++ b/src/model/wallet/asset_lock_transaction.rs @@ -431,7 +431,9 @@ impl Wallet { })?; // Transaction is fully built and signed; commit the UTXO removals now. - self.remove_selected_utxos(register_addresses, &utxos)?; + if let Some(context) = register_addresses { + self.remove_selected_utxos(&utxos, &context.db, network)?; + } Ok((tx, private_key, change_address, utxos)) } diff --git a/src/model/wallet/mod.rs b/src/model/wallet/mod.rs index 33928d3a9..8128fda22 100644 --- a/src/model/wallet/mod.rs +++ b/src/model/wallet/mod.rs @@ -3,6 +3,7 @@ pub mod encryption; pub mod single_key; mod utxos; +use crate::database::Database; use dash_sdk::dpp::ProtocolError; use dash_sdk::dpp::address_funds::{AddressWitness, PlatformAddress}; use dash_sdk::dpp::identity::signer::Signer; @@ -1653,7 +1654,9 @@ impl Wallet { })?; // Transaction is fully built and signed; commit the UTXO removals now. - self.remove_selected_utxos(register_addresses, &utxos)?; + if let Some(context) = register_addresses { + self.remove_selected_utxos(&utxos, &context.db, network)?; + } Ok(tx) } @@ -1800,7 +1803,9 @@ impl Wallet { })?; // Transaction is fully built and signed; commit the UTXO removals now. - self.remove_selected_utxos(register_addresses, &utxos)?; + if let Some(context) = register_addresses { + self.remove_selected_utxos(&utxos, &context.db, network)?; + } Ok(tx) } @@ -1838,10 +1843,37 @@ impl Wallet { used_utxos: &BTreeMap, context: &AppContext, ) -> Result<(), String> { + self.recalculate_affected_address_balances_with_db(used_utxos, &context.db) + } + + /// Core implementation: recalculate and persist balances for addresses affected + /// by spent UTXOs, using the database directly. + /// + /// Prefer [`Self::recalculate_affected_address_balances`] when an `AppContext` + /// is available. This variant is used by [`Self::remove_selected_utxos`] which + /// already receives `&Database` directly. + fn recalculate_affected_address_balances_with_db( + &mut self, + used_utxos: &BTreeMap, + db: &Database, + ) -> Result<(), String> { + let seed_hash = self.seed_hash(); let affected_addresses: BTreeSet<_> = used_utxos.values().map(|(_, addr)| addr.clone()).collect(); for address in affected_addresses { - self.recalculate_address_balance(&address, context)?; + let new_balance: u64 = self + .utxos + .get(&address) + .map(|utxo_map| utxo_map.values().map(|tx_out| tx_out.value).sum()) + .unwrap_or(0); + if let Some(current) = self.address_balances.get(&address) + && *current == new_balance + { + continue; + } + self.address_balances.insert(address.clone(), new_balance); + db.update_address_balance(&seed_hash, &address, new_balance) + .map_err(|e| e.to_string())?; } Ok(()) } @@ -2528,6 +2560,14 @@ mod tests { OutPoint::new(Txid::from_slice(&txid_bytes).unwrap(), vout) } + /// Helper: create a test wallet pre-loaded with a single UTXO of the given value. + fn test_wallet_with_utxo(value: u64) -> Wallet { + let mut wallet = test_wallet(); + let addr = test_address(1); + add_utxo(&mut wallet, &addr, 1, 0, value); + wallet + } + /// Helper: add a UTXO to a wallet fn add_utxo(wallet: &mut Wallet, address: &Address, tx_index: u8, vout: u32, value: u64) { let outpoint = test_outpoint(tx_index, vout); @@ -2641,52 +2681,43 @@ mod tests { } // ======================================================================== - // take_unspent_utxos_for tests + // select_unspent_utxos_for / remove_selected_utxos tests // ======================================================================== #[test] - fn test_take_utxos_exact_amount() { - let mut wallet = test_wallet(); - let addr = test_address(1); - add_utxo(&mut wallet, &addr, 1, 0, 100_000); + fn test_select_utxos_exact_amount() { + let wallet = test_wallet_with_utxo(100_000); - let result = wallet.take_unspent_utxos_for(90_000, 10_000, false); + let result = wallet.select_unspent_utxos_for(90_000, 10_000, false); assert!(result.is_some()); let (utxos, change) = result.unwrap(); assert_eq!(utxos.len(), 1); assert!(change.is_none()); // exact amount, no change - // UTXO should be removed from wallet - assert!(wallet.utxos.is_empty()); + // Selection is non-mutating — wallet UTXOs unchanged + assert!(!wallet.utxos.is_empty()); } #[test] - fn test_take_utxos_with_change() { - let mut wallet = test_wallet(); - let addr = test_address(1); - add_utxo(&mut wallet, &addr, 1, 0, 200_000); + fn test_select_utxos_with_change() { + let wallet = test_wallet_with_utxo(200_000); - let result = wallet.take_unspent_utxos_for(90_000, 10_000, false); + let result = wallet.select_unspent_utxos_for(90_000, 10_000, false); assert!(result.is_some()); let (utxos, change) = result.unwrap(); assert_eq!(utxos.len(), 1); assert_eq!(change, Some(100_000)); // 200k - 90k - 10k = 100k change - assert!(wallet.utxos.is_empty()); } #[test] - fn test_take_utxos_insufficient_funds() { - let mut wallet = test_wallet(); - let addr = test_address(1); - add_utxo(&mut wallet, &addr, 1, 0, 50_000); + fn test_select_utxos_insufficient_funds() { + let wallet = test_wallet_with_utxo(50_000); - let result = wallet.take_unspent_utxos_for(90_000, 10_000, false); + let result = wallet.select_unspent_utxos_for(90_000, 10_000, false); assert!(result.is_none()); - // UTXOs should NOT be removed on failure - assert!(!wallet.utxos.is_empty()); } #[test] - fn test_take_utxos_multiple_utxos_needed() { + fn test_select_utxos_multiple_utxos_needed() { let mut wallet = test_wallet(); let addr1 = test_address(1); let addr2 = test_address(2); @@ -2694,10 +2725,9 @@ mod tests { add_utxo(&mut wallet, &addr2, 2, 0, 40_000); add_utxo(&mut wallet, &addr1, 3, 0, 50_000); - let result = wallet.take_unspent_utxos_for(100_000, 10_000, false); + let result = wallet.select_unspent_utxos_for(100_000, 10_000, false); assert!(result.is_some()); let (utxos, change) = result.unwrap(); - // Should have collected enough UTXOs to cover 110_000 let total_collected: u64 = utxos.values().map(|(tx_out, _)| tx_out.value).sum(); assert!(total_collected >= 110_000); if let Some(change_amount) = change { @@ -2706,73 +2736,100 @@ mod tests { } #[test] - fn test_take_utxos_allow_take_fee_from_amount() { - let mut wallet = test_wallet(); - let addr = test_address(1); - // Wallet has exactly enough for the amount but not the fee - add_utxo(&mut wallet, &addr, 1, 0, 100_000); + fn test_select_utxos_allow_take_fee_from_amount() { + let wallet = test_wallet_with_utxo(100_000); // Request 100k amount + 10k fee = 110k total, but only 100k available // With allow_take_fee_from_amount=true, should still succeed since total >= amount - let result = wallet.take_unspent_utxos_for(100_000, 10_000, true); + let result = wallet.select_unspent_utxos_for(100_000, 10_000, true); assert!(result.is_some()); let (_utxos, change) = result.unwrap(); assert!(change.is_none()); } #[test] - fn test_take_utxos_allow_take_fee_but_not_enough_for_amount() { - let mut wallet = test_wallet(); - let addr = test_address(1); - add_utxo(&mut wallet, &addr, 1, 0, 50_000); + fn test_select_utxos_allow_take_fee_but_not_enough_for_amount() { + let wallet = test_wallet_with_utxo(50_000); // Request 100k amount + 10k fee = 110k, only 50k available // Even with take_fee_from_amount, 50k < 100k amount, so should fail - let result = wallet.take_unspent_utxos_for(100_000, 10_000, true); + let result = wallet.select_unspent_utxos_for(100_000, 10_000, true); assert!(result.is_none()); } #[test] - fn test_take_utxos_zero_amount() { - let mut wallet = test_wallet(); - let addr = test_address(1); - add_utxo(&mut wallet, &addr, 1, 0, 50_000); + fn test_select_utxos_zero_amount() { + let wallet = test_wallet_with_utxo(50_000); - let result = wallet.take_unspent_utxos_for(0, 0, false); + let result = wallet.select_unspent_utxos_for(0, 0, false); assert!(result.is_some()); let (utxos, change) = result.unwrap(); assert!(utxos.is_empty()); assert!(change.is_none()); } + /// Helper: register a wallet address in the test database so that + /// `update_address_balance` can find the row. + fn register_test_address(db: &Database, wallet: &Wallet, address: &Address) { + let seed_hash = wallet.seed_hash(); + let path = DerivationPath::from(vec![ + ChildNumber::Hardened { index: 44 }, + ChildNumber::Hardened { index: 1 }, + ChildNumber::Hardened { index: 0 }, + ChildNumber::Normal { index: 0 }, + ChildNumber::Normal { index: 0 }, + ]); + db.add_address_if_not_exists( + &seed_hash, + address, + &Network::Testnet, + &path, + DerivationPathReference::BIP44, + DerivationPathType::CLEAR_FUNDS, + Some(0), + ) + .expect("register test address"); + } + #[test] - fn test_take_utxos_removes_from_wallet() { + fn test_remove_utxos_removes_from_wallet() { + use crate::database::test_helpers::create_test_database; + let mut wallet = test_wallet(); let addr = test_address(1); add_utxo(&mut wallet, &addr, 1, 0, 100_000); add_utxo(&mut wallet, &addr, 2, 0, 200_000); - assert_eq!(wallet.max_balance(), 300_000); - // Take only enough for 100k - let result = wallet.take_unspent_utxos_for(90_000, 10_000, false); - assert!(result.is_some()); + let db = create_test_database().expect("test db"); + register_test_address(&db, &wallet, &addr); + let (selected, _) = wallet + .select_unspent_utxos_for(90_000, 10_000, false) + .unwrap(); + wallet + .remove_selected_utxos(&selected, &db, Network::Testnet) + .unwrap(); - // Remaining wallet should have reduced UTXOs - let remaining_balance = wallet.max_balance(); - assert!(remaining_balance < 300_000); + assert!(wallet.max_balance() < 300_000); } #[test] - fn test_take_utxos_cleans_empty_address_entries() { + fn test_remove_utxos_cleans_empty_address_entries() { + use crate::database::test_helpers::create_test_database; + let mut wallet = test_wallet(); let addr = test_address(1); add_utxo(&mut wallet, &addr, 1, 0, 100_000); - let result = wallet.take_unspent_utxos_for(90_000, 10_000, false); - assert!(result.is_some()); + let db = create_test_database().expect("test db"); + register_test_address(&db, &wallet, &addr); + let (selected, _) = wallet + .select_unspent_utxos_for(90_000, 10_000, false) + .unwrap(); + wallet + .remove_selected_utxos(&selected, &db, Network::Testnet) + .unwrap(); - // The address entry should be removed since it has no more UTXOs assert!(!wallet.utxos.contains_key(&addr)); } diff --git a/src/model/wallet/utxos.rs b/src/model/wallet/utxos.rs index 6de622abc..e80655435 100644 --- a/src/model/wallet/utxos.rs +++ b/src/model/wallet/utxos.rs @@ -1,9 +1,10 @@ use crate::context::AppContext; +use crate::database::Database; use crate::model::wallet::{DerivationPathHelpers, Wallet}; use crate::spv::CoreBackendMode; use dash_sdk::dashcore_rpc::RpcApi; use dash_sdk::dashcore_rpc::json::ListUnspentResultEntry; -use dash_sdk::dpp::dashcore::{Address, OutPoint, TxOut}; +use dash_sdk::dpp::dashcore::{Address, Network, OutPoint, TxOut}; use std::collections::{BTreeMap, HashMap, HashSet}; impl Wallet { @@ -72,14 +73,13 @@ impl Wallet { /// /// Typically called with the result of [`Self::select_unspent_utxos_for`] /// after the transaction has been fully built and signed. - /// - /// When `context` is `None`, only the in-memory state is updated (useful - /// for tests or when no database persistence is needed). pub fn remove_selected_utxos( &mut self, - context: Option<&AppContext>, selected: &BTreeMap, + db: &Database, + network: Network, ) -> Result<(), String> { + // Update in-memory UTXO map for (outpoint, (_, address)) in selected { if let Some(outpoints) = self.utxos.get_mut(address) { outpoints.remove(outpoint); @@ -95,40 +95,17 @@ impl Wallet { } } - if let Some(context) = context { - // Drop used UTXOs from database - for outpoint in selected.keys() { - context - .db - .drop_utxo(outpoint, &context.network.to_string()) - .map_err(|e| e.to_string())?; - } - - // Recalculate balances for affected addresses - self.recalculate_affected_address_balances(selected, context)?; + // Persist UTXO removals to the database + let network_str = network.to_string(); + for outpoint in selected.keys() { + db.drop_utxo(outpoint, &network_str) + .map_err(|e| e.to_string())?; } - Ok(()) - } + // Recalculate and persist balances for affected addresses + self.recalculate_affected_address_balances_with_db(selected, db)?; - /// Convenience wrapper: selects and immediately removes UTXOs (in-memory only). - /// - /// Used in tests and contexts without database persistence. Production code - /// should prefer [`Self::select_unspent_utxos_for`] followed by - /// [`Self::remove_selected_utxos`] with a context. - #[allow(clippy::type_complexity)] - pub fn take_unspent_utxos_for( - &mut self, - amount: u64, - fee: u64, - allow_take_fee_from_amount: bool, - ) -> Option<(BTreeMap, Option)> { - let (selected, change_option) = - self.select_unspent_utxos_for(amount, fee, allow_take_fee_from_amount)?; - // In-memory only — no context for DB persistence. - self.remove_selected_utxos(None, &selected) - .expect("remove_selected_utxos without context cannot fail"); - Some((selected, change_option)) + Ok(()) } /// Reload UTXOs from Core RPC, updating both in-memory state and database. From 4c7320e8856a7977f84a336d4f3980b4f0fa58ef Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Tue, 24 Feb 2026 17:06:36 +0100 Subject: [PATCH 11/17] fix(wallet): address remaining audit findings from code review - Refresh platform sync info cache after wallet refresh completes - Add broadcast failure cleanup in create_asset_lock (remove stale finality tracking entries, replace mutex .unwrap() with .map_err()) - Replace .expect() with proper error propagation in signing loops - Use i128 for fee logging subtraction to prevent overflow - Renumber step comments sequentially after refactoring Co-Authored-By: Claude Opus 4.6 --- src/backend_task/core/create_asset_lock.rs | 38 +++++++++++++++---- src/backend_task/identity/top_up_identity.rs | 2 +- ...fund_platform_address_from_wallet_utxos.rs | 6 +-- src/model/wallet/asset_lock_transaction.rs | 27 +++++++++---- src/ui/wallets/wallets_screen/mod.rs | 9 +++++ 5 files changed, 62 insertions(+), 20 deletions(-) diff --git a/src/backend_task/core/create_asset_lock.rs b/src/backend_task/core/create_asset_lock.rs index 4b4b65f11..c29907edf 100644 --- a/src/backend_task/core/create_asset_lock.rs +++ b/src/backend_task/core/create_asset_lock.rs @@ -33,14 +33,27 @@ impl AppContext { // Insert the transaction into waiting for finality { - let mut proofs = self.transactions_waiting_for_finality.lock().unwrap(); + let mut proofs = self + .transactions_waiting_for_finality + .lock() + .map_err(|e| e.to_string())?; proofs.insert(tx_id, None); } - // Broadcast the transaction - self.broadcast_raw_transaction(&asset_lock_transaction) + // Broadcast the transaction. If broadcast fails, the UTXOs have already + // been removed from the wallet (inside the transaction builder) but were + // never actually spent on-chain. We trigger a wallet refresh so the next + // UTXO reload reconciles the in-memory state with the chain. + if let Err(e) = self + .broadcast_raw_transaction(&asset_lock_transaction) .await - .map_err(|e| format!("Failed to broadcast asset lock transaction: {}", e))?; + { + // Clean up the finality tracking entry + if let Ok(mut proofs) = self.transactions_waiting_for_finality.lock() { + proofs.remove(&tx_id); + } + return Err(format!("Failed to broadcast asset lock transaction: {}", e)); + } Ok(BackendTaskSuccessResult::Message(format!( "Asset lock transaction broadcast successfully. TX ID: {}", @@ -77,14 +90,23 @@ impl AppContext { // Insert the transaction into waiting for finality { - let mut proofs = self.transactions_waiting_for_finality.lock().unwrap(); + let mut proofs = self + .transactions_waiting_for_finality + .lock() + .map_err(|e| e.to_string())?; proofs.insert(tx_id, None); } - // Broadcast the transaction - self.broadcast_raw_transaction(&asset_lock_transaction) + // Broadcast the transaction (see registration path above for cleanup rationale) + if let Err(e) = self + .broadcast_raw_transaction(&asset_lock_transaction) .await - .map_err(|e| format!("Failed to broadcast asset lock transaction: {}", e))?; + { + if let Ok(mut proofs) = self.transactions_waiting_for_finality.lock() { + proofs.remove(&tx_id); + } + return Err(format!("Failed to broadcast asset lock transaction: {}", e)); + } Ok(BackendTaskSuccessResult::Message(format!( "Asset lock transaction broadcast successfully. TX ID: {}", diff --git a/src/backend_task/identity/top_up_identity.rs b/src/backend_task/identity/top_up_identity.rs index 48ef5621b..bc2175555 100644 --- a/src/backend_task/identity/top_up_identity.rs +++ b/src/backend_task/identity/top_up_identity.rs @@ -448,7 +448,7 @@ impl AppContext { "Top-up fee mismatch: estimated {} vs actual {} (diff: {})", estimated_fee, actual_fee, - actual_fee as i64 - estimated_fee as i64 + actual_fee as i128 - estimated_fee as i128 ); } } else { diff --git a/src/backend_task/wallet/fund_platform_address_from_wallet_utxos.rs b/src/backend_task/wallet/fund_platform_address_from_wallet_utxos.rs index a4406a2a5..39d34fa01 100644 --- a/src/backend_task/wallet/fund_platform_address_from_wallet_utxos.rs +++ b/src/backend_task/wallet/fund_platform_address_from_wallet_utxos.rs @@ -161,7 +161,7 @@ impl AppContext { } }; - // Step 7: Get wallet, SDK, and derive a fresh change address if needed + // Step 6: Get wallet, SDK, and derive a fresh change address if needed let (wallet, sdk, change_platform_address) = { let wallet_arc = { let wallets = self.wallets.read().unwrap(); @@ -191,7 +191,7 @@ impl AppContext { (wallet, sdk, change_platform_address) }; - // Step 8: Fund the destination platform address + // Step 7: Fund the destination platform address let mut outputs = std::collections::BTreeMap::new(); let fee_strategy = if fee_deduct_from_output { @@ -239,7 +239,7 @@ impl AppContext { .await .map_err(|e| format!("Failed to fund platform address: {}", e))?; - // Step 9: Refresh platform address balances + // Step 8: Refresh platform address balances self.fetch_platform_address_balances(seed_hash, PlatformSyncMode::Auto) .await?; diff --git a/src/model/wallet/asset_lock_transaction.rs b/src/model/wallet/asset_lock_transaction.rs index 2ad56b61f..0a1ea493f 100644 --- a/src/model/wallet/asset_lock_transaction.rs +++ b/src/model/wallet/asset_lock_transaction.rs @@ -369,22 +369,28 @@ impl Wallet { // Next, collect the sighashes for each input since that's what we need from the // cache - let sighashes: Vec<_> = tx + let sighashes: Result, String> = tx .input .iter() .enumerate() .map(|(i, input)| { let script_pubkey = utxos .get(&input.previous_output) - .expect("expected a txout") + .ok_or_else(|| { + format!( + "UTXO not found in selected set for input {}", + input.previous_output + ) + })? .0 .script_pubkey .clone(); cache .legacy_signature_hash(i, &script_pubkey, sighash_u32) - .expect("expected sighash") + .map_err(|e| format!("Failed to compute sighash for input {}: {}", i, e)) }) .collect(); + let sighashes = sighashes?; // Now we can drop the cache to end the immutable borrow #[allow(clippy::drop_non_drop)] @@ -397,9 +403,13 @@ impl Wallet { .zip(sighashes.into_iter()) .try_for_each(|(input, sighash)| { // You need to provide the actual script_pubkey of the UTXO being spent - let (_, input_address) = check_utxos - .remove(&input.previous_output) - .expect("expected a txout"); + let (_, input_address) = + check_utxos.remove(&input.previous_output).ok_or_else(|| { + format!( + "UTXO not found in selected set for input {}", + input.previous_output + ) + })?; let message = Message::from_digest(sighash.into()); let private_key = self @@ -545,16 +555,17 @@ impl Wallet { // Next, collect the sighashes for each input since that's what we need from the // cache - let sighashes: Vec<_> = tx + let sighashes: Result, String> = tx .input .iter() .enumerate() .map(|(i, _)| { cache .legacy_signature_hash(i, &previous_tx_output.script_pubkey, sighash_u32) - .expect("expected sighash") + .map_err(|e| format!("Failed to compute sighash for input {}: {}", i, e)) }) .collect(); + let sighashes = sighashes?; // Now we can drop the cache to end the immutable borrow #[allow(clippy::drop_non_drop)] diff --git a/src/ui/wallets/wallets_screen/mod.rs b/src/ui/wallets/wallets_screen/mod.rs index 9600a0676..9c087a3b4 100644 --- a/src/ui/wallets/wallets_screen/mod.rs +++ b/src/ui/wallets/wallets_screen/mod.rs @@ -2045,6 +2045,15 @@ impl ScreenLike for WalletsBalancesScreen { match backend_task_success_result { crate::ui::BackendTaskSuccessResult::RefreshedWallet { warning } => { self.refreshing = false; + // Refresh the cached platform sync info so the panel shows + // updated timestamps and block heights after a wallet sync. + let seed_hash = self + .selected_wallet + .as_ref() + .and_then(|w| w.read().ok().map(|g| g.seed_hash())); + if let Some(hash) = seed_hash { + self.refresh_platform_sync_info_cache(&hash); + } if let Some(warn_msg) = warning { self.set_message( format!("Wallet refreshed with warning: {}", warn_msg), From 9c8cb31a73a5410eeae71de16218b1885cca9731 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 25 Feb 2026 09:55:40 +0100 Subject: [PATCH 12/17] fix(ui): refresh sync info cache after platform balance fetch The PlatformAddressBalances task result handler updated wallet balances but did not refresh the platform_sync_info cache, causing the UI to display "never synced" until the wallet was reselected. Co-Authored-By: Claude Opus 4.6 --- src/ui/wallets/wallets_screen/mod.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/src/ui/wallets/wallets_screen/mod.rs b/src/ui/wallets/wallets_screen/mod.rs index 57b66f2e8..eb84709ef 100644 --- a/src/ui/wallets/wallets_screen/mod.rs +++ b/src/ui/wallets/wallets_screen/mod.rs @@ -2180,6 +2180,7 @@ impl ScreenLike for WalletsBalancesScreen { wallet.set_platform_address_info(addr, balance, nonce); } } + self.refresh_platform_sync_info_cache(&seed_hash); self.set_message( "Successfully synced Platform balances".to_string(), MessageType::Success, From 0406d47c975bbbc42585d6fb2a2c7ddcc12e1a5e Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 25 Feb 2026 10:00:15 +0100 Subject: [PATCH 13/17] feat(ui): make sync status panel collapsible and dev-mode only The Core/Platform sync status panel is now hidden by default and only visible when developer mode is enabled. It uses a collapsible header so developers can expand/collapse it as needed. Co-Authored-By: Claude Opus 4.6 --- src/ui/wallets/wallets_screen/mod.rs | 271 ++++++++++++++------------- 1 file changed, 140 insertions(+), 131 deletions(-) diff --git a/src/ui/wallets/wallets_screen/mod.rs b/src/ui/wallets/wallets_screen/mod.rs index eb84709ef..33715ea08 100644 --- a/src/ui/wallets/wallets_screen/mod.rs +++ b/src/ui/wallets/wallets_screen/mod.rs @@ -1154,142 +1154,151 @@ impl WalletsBalancesScreen { fn render_sync_status(&self, ui: &mut Ui) { let dark_mode = ui.ctx().style().visuals.dark_mode; - Frame::group(ui.style()) - .fill(DashColors::surface(dark_mode)) - .inner_margin(Margin::symmetric(16, 8)) - .show(ui, |ui| { - // Line 1 -- Core sync status - ui.horizontal(|ui| { - ui.label( - RichText::new("Core:") - .size(12.0) - .strong() - .color(DashColors::text_primary(dark_mode)), - ); + ui.collapsing( + RichText::new("Sync Status") + .size(12.0) + .color(DashColors::text_secondary(dark_mode)), + |ui| { + Frame::group(ui.style()) + .fill(DashColors::surface(dark_mode)) + .inner_margin(Margin::symmetric(16, 8)) + .show(ui, |ui| { + // Line 1 -- Core sync status + ui.horizontal(|ui| { + ui.label( + RichText::new("Core:") + .size(12.0) + .strong() + .color(DashColors::text_primary(dark_mode)), + ); - match self.app_context.core_backend_mode() { - CoreBackendMode::Rpc => { - if self.app_context.connection_status().rpc_online() { - ui.colored_label( - Color32::DARK_GREEN, - RichText::new("Connected").size(12.0), - ); - } else { - ui.colored_label( - DashColors::ERROR, - RichText::new("Disconnected").size(12.0), - ); - } - } - CoreBackendMode::Spv => { - let snapshot = self.app_context.spv_manager().status(); - match snapshot.status { - SpvStatus::Idle | SpvStatus::Stopped => { - ui.label( - RichText::new("Disconnected") - .size(12.0) - .color(DashColors::text_secondary(dark_mode)), - ); - } - SpvStatus::Starting => { - ui.add( - egui::Spinner::new() - .size(12.0) - .color(DashColors::DASH_BLUE), - ); - ui.label( - RichText::new("Connecting...") - .size(12.0) - .color(DashColors::DASH_BLUE), - ); - } - SpvStatus::Syncing => { - ui.add( - egui::Spinner::new() - .size(12.0) - .color(DashColors::DASH_BLUE), - ); - let phase_text = - Self::spv_active_phase_text(&snapshot.sync_progress); - ui.label( - RichText::new(format!("Syncing — {phase_text}")) - .size(12.0) - .color(DashColors::DASH_BLUE), - ); - } - SpvStatus::Running => { - ui.colored_label( - Color32::DARK_GREEN, - RichText::new(format!( - "Synced — {} peers", - snapshot.connected_peers - )) - .size(12.0), - ); - } - SpvStatus::Stopping => { - ui.add( - egui::Spinner::new() - .size(12.0) - .color(DashColors::DASH_BLUE), - ); - ui.label( - RichText::new("Disconnecting...") - .size(12.0) - .color(DashColors::DASH_BLUE), - ); + match self.app_context.core_backend_mode() { + CoreBackendMode::Rpc => { + if self.app_context.connection_status().rpc_online() { + ui.colored_label( + Color32::DARK_GREEN, + RichText::new("Connected").size(12.0), + ); + } else { + ui.colored_label( + DashColors::ERROR, + RichText::new("Disconnected").size(12.0), + ); + } } - SpvStatus::Error => { - ui.colored_label( - DashColors::ERROR, - RichText::new("Error").size(12.0), - ); + CoreBackendMode::Spv => { + let snapshot = self.app_context.spv_manager().status(); + match snapshot.status { + SpvStatus::Idle | SpvStatus::Stopped => { + ui.label( + RichText::new("Disconnected") + .size(12.0) + .color(DashColors::text_secondary(dark_mode)), + ); + } + SpvStatus::Starting => { + ui.add( + egui::Spinner::new() + .size(12.0) + .color(DashColors::DASH_BLUE), + ); + ui.label( + RichText::new("Connecting...") + .size(12.0) + .color(DashColors::DASH_BLUE), + ); + } + SpvStatus::Syncing => { + ui.add( + egui::Spinner::new() + .size(12.0) + .color(DashColors::DASH_BLUE), + ); + let phase_text = Self::spv_active_phase_text( + &snapshot.sync_progress, + ); + ui.label( + RichText::new(format!("Syncing — {phase_text}")) + .size(12.0) + .color(DashColors::DASH_BLUE), + ); + } + SpvStatus::Running => { + ui.colored_label( + Color32::DARK_GREEN, + RichText::new(format!( + "Synced — {} peers", + snapshot.connected_peers + )) + .size(12.0), + ); + } + SpvStatus::Stopping => { + ui.add( + egui::Spinner::new() + .size(12.0) + .color(DashColors::DASH_BLUE), + ); + ui.label( + RichText::new("Disconnecting...") + .size(12.0) + .color(DashColors::DASH_BLUE), + ); + } + SpvStatus::Error => { + ui.colored_label( + DashColors::ERROR, + RichText::new("Error").size(12.0), + ); + } + } } } - } - } - }); + }); - // Line 2 -- Platform sync status - ui.horizontal(|ui| { - ui.label( - RichText::new("Platform:") - .size(12.0) - .strong() - .color(DashColors::text_primary(dark_mode)), - ); + // Line 2 -- Platform sync status + ui.horizontal(|ui| { + ui.label( + RichText::new("Platform:") + .size(12.0) + .strong() + .color(DashColors::text_primary(dark_mode)), + ); - // Addresses - let addr_count = self - .selected_wallet - .as_ref() - .and_then(|w| w.read().ok()) - .map(|w| w.platform_address_info.len()) - .unwrap_or(0); - if self.refreshing { - ui.add(egui::Spinner::new().size(12.0).color(DashColors::DASH_BLUE)); - } - let addr_text = - if let Some((last_sync_ts, sync_height)) = self.platform_sync_info { - let ago = Self::format_unix_time_ago(last_sync_ts); - format!( - "Addresses: {} synced (blk {}, {})", - addr_count, sync_height, ago - ) - } else { - "Addresses: never synced".to_string() - }; - ui.label( - RichText::new(addr_text) - .size(12.0) - .color(if self.refreshing { - DashColors::DASH_BLUE + // Addresses + let addr_count = self + .selected_wallet + .as_ref() + .and_then(|w| w.read().ok()) + .map(|w| w.platform_address_info.len()) + .unwrap_or(0); + if self.refreshing { + ui.add( + egui::Spinner::new().size(12.0).color(DashColors::DASH_BLUE), + ); + } + let addr_text = if let Some((last_sync_ts, sync_height)) = + self.platform_sync_info + { + let ago = Self::format_unix_time_ago(last_sync_ts); + format!( + "Addresses: {} synced (blk {}, {})", + addr_count, sync_height, ago + ) } else { - DashColors::text_secondary(dark_mode) - }), - ); - }); - }); + "Addresses: never synced".to_string() + }; + ui.label(RichText::new(addr_text).size(12.0).color( + if self.refreshing { + DashColors::DASH_BLUE + } else { + DashColors::text_secondary(dark_mode) + }, + )); + }); + }); + }, + ); } /// Get a text summary of the active SPV sync phase. @@ -1698,8 +1707,8 @@ impl ScreenLike for WalletsBalancesScreen { ui.add_space(10.0); - // Sync status panel (only for HD wallets) - if self.selected_wallet.is_some() { + // Sync status panel (only for HD wallets, dev mode only) + if self.selected_wallet.is_some() && self.app_context.is_developer_mode() { self.render_sync_status(ui); ui.add_space(6.0); } From dafff5711bd687b823f2d2c3f8b31e3a659cdb24 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 25 Feb 2026 11:14:08 +0100 Subject: [PATCH 14/17] fix(ui): address PR #642 review findings - Centralize wallet selection via set_selected_hd_wallet() helper to keep platform_sync_info cache consistent across all code paths - Add tracing::warn! for Mutex poisoning in asset lock cleanup paths - Fix misleading comment about wallet refresh on broadcast failure - Remove TS-25 from manual test scenarios (not part of this PR) Refs: #657 Co-Authored-By: Claude Opus 4.6 --- .../manual-test-scenarios.md | 17 ------ src/backend_task/core/create_asset_lock.rs | 9 ++- src/ui/wallets/wallets_screen/mod.rs | 59 ++++++++++++------- 3 files changed, 45 insertions(+), 40 deletions(-) diff --git a/docs/ai-design/2026-02-24-sync-status-panel/manual-test-scenarios.md b/docs/ai-design/2026-02-24-sync-status-panel/manual-test-scenarios.md index 5660685c2..675bb77fd 100644 --- a/docs/ai-design/2026-02-24-sync-status-panel/manual-test-scenarios.md +++ b/docs/ai-design/2026-02-24-sync-status-panel/manual-test-scenarios.md @@ -407,23 +407,6 @@ --- -## TS-25: Database refactor -- shared_connection removal - -### Preconditions -- This is a code-level verification, not a UI test. - -### Steps -1. Verify the application compiles without errors (`cargo build`). -2. Run the full test suite (`cargo test --all-features --workspace`). -3. Search the codebase for any remaining calls to `Database::shared_connection()`. - -### Expected Results -- The application compiles successfully. -- All tests pass. -- No remaining references to `shared_connection()` exist in the codebase (the method was removed and `Database.conn` changed from `Arc>` to `Mutex`). - ---- - ## Edge Cases | # | Scenario | Expected Behavior | diff --git a/src/backend_task/core/create_asset_lock.rs b/src/backend_task/core/create_asset_lock.rs index b656a8579..4a90ef97c 100644 --- a/src/backend_task/core/create_asset_lock.rs +++ b/src/backend_task/core/create_asset_lock.rs @@ -42,8 +42,9 @@ impl AppContext { // Broadcast the transaction. If broadcast fails, the UTXOs have already // been removed from the wallet (inside the transaction builder) but were - // never actually spent on-chain. We trigger a wallet refresh so the next - // UTXO reload reconciles the in-memory state with the chain. + // never actually spent on-chain. The caller should handle refreshing + // the wallet so the next UTXO reload reconciles in-memory state with + // the chain. See also: https://github.com/dashpay/dash-evo-tool/issues/657 if let Err(e) = self .broadcast_raw_transaction(&asset_lock_transaction) .await @@ -51,6 +52,8 @@ impl AppContext { // Clean up the finality tracking entry if let Ok(mut proofs) = self.transactions_waiting_for_finality.lock() { proofs.remove(&tx_id); + } else { + tracing::warn!("Failed to clean up finality tracking for tx {tx_id}: Mutex poisoned"); } return Err(format!("Failed to broadcast asset lock transaction: {}", e)); } @@ -104,6 +107,8 @@ impl AppContext { { if let Ok(mut proofs) = self.transactions_waiting_for_finality.lock() { proofs.remove(&tx_id); + } else { + tracing::warn!("Failed to clean up finality tracking for tx {tx_id}: Mutex poisoned"); } return Err(format!("Failed to broadcast asset lock transaction: {}", e)); } diff --git a/src/ui/wallets/wallets_screen/mod.rs b/src/ui/wallets/wallets_screen/mod.rs index 33715ea08..7479a967b 100644 --- a/src/ui/wallets/wallets_screen/mod.rs +++ b/src/ui/wallets/wallets_screen/mod.rs @@ -249,15 +249,29 @@ impl WalletsBalancesScreen { .filter(|(ts, _)| *ts > 0); } - fn select_hd_wallet(&mut self, wallet: Arc>) { - self.selected_wallet = Some(wallet.clone()); + /// Set the selected HD wallet and update all associated state (persisted + /// hash, platform sync info cache). All code paths that change + /// `selected_wallet` should go through this helper to keep the sync + /// status panel consistent. + fn set_selected_hd_wallet(&mut self, wallet: Option>>) { + let seed_hash = wallet + .as_ref() + .and_then(|w| w.read().ok().map(|g| g.seed_hash())); + self.selected_wallet = wallet; self.selected_single_key_wallet = None; self.selected_account = None; - if let Ok(hash) = wallet.read().map(|g| g.seed_hash()) { + if let Some(hash) = seed_hash { self.persist_selected_wallet_hash(Some(hash)); self.refresh_platform_sync_info_cache(&hash); + } else { + self.persist_selected_wallet_hash(None); + self.platform_sync_info = None; } + } + + fn select_hd_wallet(&mut self, wallet: Arc>) { + self.set_selected_hd_wallet(Some(wallet)); self.persist_selected_single_key_hash(None); } @@ -265,6 +279,7 @@ impl WalletsBalancesScreen { self.selected_single_key_wallet = Some(wallet.clone()); self.selected_wallet = None; self.selected_account = None; + self.platform_sync_info = None; self.utxo_page = 0; if let Ok(hash) = wallet.read().map(|g| g.key_hash) { @@ -285,7 +300,7 @@ impl WalletsBalancesScreen { return; } // HD wallet no longer valid - self.selected_wallet = None; + self.set_selected_hd_wallet(None); } // Check if single key wallet selection is still valid @@ -303,12 +318,14 @@ impl WalletsBalancesScreen { } // No valid selection, pick a new one (HD wallet first, then single key) - if let Ok(wallets) = self.app_context.wallets.read() - && let Some(wallet) = wallets.values().next().cloned() - { - self.selected_wallet = Some(wallet); - self.selected_single_key_wallet = None; - self.selected_account = None; + let next_hd = self + .app_context + .wallets + .read() + .ok() + .and_then(|w| w.values().next().cloned()); + if let Some(wallet) = next_hd { + self.set_selected_hd_wallet(Some(wallet)); return; } @@ -318,10 +335,12 @@ impl WalletsBalancesScreen { self.selected_single_key_wallet = Some(wallet); self.selected_wallet = None; self.selected_account = None; + self.platform_sync_info = None; return; } self.selected_account = None; + self.platform_sync_info = None; } fn add_receiving_address(&mut self) { @@ -713,13 +732,7 @@ impl WalletsBalancesScreen { .ok() .and_then(|wallets| wallets.values().next().cloned()); - self.selected_wallet = next_wallet.clone(); - - // Update persisted selection in AppContext and database - let new_hash = next_wallet - .as_ref() - .and_then(|w| w.read().ok().map(|g| g.seed_hash())); - self.persist_selected_wallet_hash(new_hash); + self.set_selected_hd_wallet(next_wallet); self.show_rename_dialog = false; self.rename_input.clear(); @@ -2233,10 +2246,14 @@ impl ScreenLike for WalletsBalancesScreen { // If no wallet of either type is selected but wallets exist, select the first HD wallet if self.selected_wallet.is_none() && self.selected_single_key_wallet.is_none() { - if let Ok(wallets) = self.app_context.wallets.read() - && let Some(wallet) = wallets.values().next().cloned() - { - self.selected_wallet = Some(wallet); + let next_hd = self + .app_context + .wallets + .read() + .ok() + .and_then(|w| w.values().next().cloned()); + if let Some(wallet) = next_hd { + self.set_selected_hd_wallet(Some(wallet)); return; } // If no HD wallets, try single key wallets From cb8ceafbf6e5173de69791b1f3ff4915838251de Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 25 Feb 2026 11:56:04 +0100 Subject: [PATCH 15/17] refactor(ui): extract shared SPV phase summary and enrich tooltip Consolidate duplicated SPV sync phase formatting into a shared `spv_phase_summary()` function in `connection_status.rs`. The wallet screen now uses this shared function instead of its own copy. The network screen retains its richer operator-facing formatter. The connection indicator tooltip now shows detailed sync progress (e.g. "SPV: Headers: 12345 / 27000 (45%)") instead of bare "SPV: Syncing" when in SPV mode. Also adjust refresh polling rates: 4s when connected, 1s when disconnected (was 10s/2s). Co-Authored-By: Claude Opus 4.6 --- src/context/connection_status.rs | 83 ++++++++++++++++++++++++++-- src/ui/components/top_panel.rs | 2 +- src/ui/wallets/wallets_screen/mod.rs | 62 ++------------------- 3 files changed, 86 insertions(+), 61 deletions(-) diff --git a/src/context/connection_status.rs b/src/context/connection_status.rs index 2cb4f175f..d2170663f 100644 --- a/src/context/connection_status.rs +++ b/src/context/connection_status.rs @@ -5,13 +5,14 @@ use crate::backend_task::BackendTaskSuccessResult; use crate::backend_task::core::{CoreItem, CoreTask}; use crate::components::core_zmq_listener::ZMQConnectionEvent; use crate::spv::{CoreBackendMode, SpvStatus}; +use dash_sdk::dash_spv::sync::{ProgressPercentage, SyncProgress as SpvSyncProgress, SyncState}; use dash_sdk::dpp::dashcore::{ChainLock, Network}; use std::sync::Mutex; use std::sync::atomic::{AtomicBool, AtomicU8, AtomicU16, Ordering}; use std::time::{Duration, Instant}; -const REFRESH_CONNECTED: Duration = Duration::from_secs(10); -const REFRESH_DISCONNECTED: Duration = Duration::from_secs(2); +const REFRESH_CONNECTED: Duration = Duration::from_secs(4); +const REFRESH_DISCONNECTED: Duration = Duration::from_secs(1); /// Three-state connection indicator matching the UI's red/orange/green circle. #[derive(Debug, Clone, Copy, PartialEq, Eq)] @@ -215,7 +216,12 @@ impl ConnectionStatus { self.overall_state.store(state as u8, Ordering::Relaxed); } - pub fn tooltip_text(&self) -> String { + /// Build the tooltip string for the connection indicator. + /// + /// In SPV mode, fetches sync progress from the [`SpvManager`] to display + /// a detailed phase summary (e.g. `"SPV: Headers: 12345 / 27000 (45%)"`) + /// instead of the bare `"SPV: Syncing"`. + pub fn tooltip_text(&self, app_context: &crate::context::AppContext) -> String { let backend_mode = self.backend_mode(); let disable_zmq = self.disable_zmq(); let spv_status = self.spv_status(); @@ -255,7 +261,13 @@ impl ConnectionStatus { OverallConnectionState::Syncing => "Syncing", OverallConnectionState::Disconnected => "Disconnected", }; - let spv_label = format!("SPV: {:?}", spv_status); + let spv_label = app_context + .spv_manager() + .status() + .sync_progress + .as_ref() + .map(|p| format!("SPV: {}", spv_phase_summary(p))) + .unwrap_or_else(|| format!("SPV: {:?}", spv_status)); format!("{header}\n{spv_label}\n{dapi_status}") } } @@ -387,6 +399,69 @@ impl ConnectionStatus { } } +/// Compact text summary of the active SPV sync phase. +/// +/// Returns e.g. `"Headers: 12345 / 27000 (45%)"`, `"Masternodes: 800 / 2000 (40%)"`, +/// or `"syncing..."` if no phase is actively syncing. +/// +/// Phases are checked in pipeline execution order (early → late) so the user +/// sees progression from headers through to blocks. +pub fn spv_phase_summary(progress: &SpvSyncProgress) -> String { + // Check phases in order of execution + if let Ok(headers) = progress.headers() + && headers.state() == SyncState::Syncing + { + let (cur, tgt) = (headers.current_height(), headers.target_height()); + return format!("Headers: {} / {} ({}%)", cur, tgt, pct(cur, tgt)); + } + + if let Ok(mn) = progress.masternodes() + && mn.state() == SyncState::Syncing + { + let (cur, tgt) = (mn.current_height(), mn.target_height()); + return format!("Masternodes: {} / {} ({}%)", cur, tgt, pct(cur, tgt)); + } + + if let Ok(fh) = progress.filter_headers() + && fh.state() == SyncState::Syncing + { + let (cur, tgt) = (fh.current_height(), fh.target_height()); + return format!("Filter Headers: {} / {} ({}%)", cur, tgt, pct(cur, tgt)); + } + + if let Ok(filters) = progress.filters() + && filters.state() == SyncState::Syncing + { + let (cur, tgt) = (filters.current_height(), filters.target_height()); + return format!("Filters: {} / {} ({}%)", cur, tgt, pct(cur, tgt)); + } + + if let Ok(blocks) = progress.blocks() + && blocks.state() == SyncState::Syncing + { + // Blocks doesn't expose its own target_height; use the best available + // approximation: max of headers target and blocks last_processed. + let target = progress + .headers() + .ok() + .map(|h| h.target_height()) + .unwrap_or(0) + .max(blocks.last_processed()); + let cur = blocks.last_processed(); + return format!("Blocks: {} / {} ({}%)", cur, target, pct(cur, target)); + } + + "syncing...".to_string() +} + +fn pct(current: u32, target: u32) -> u32 { + if target == 0 { + 0 + } else { + ((current as f64 / target as f64) * 100.0).clamp(0.0, 100.0) as u32 + } +} + impl Default for ConnectionStatus { fn default() -> Self { Self::new() diff --git a/src/ui/components/top_panel.rs b/src/ui/components/top_panel.rs index b2f46add3..e022f1a59 100644 --- a/src/ui/components/top_panel.rs +++ b/src/ui/components/top_panel.rs @@ -151,7 +151,7 @@ fn add_connection_indicator(ui: &mut Ui, app_context: &Arc) -> AppAc if overall != OverallConnectionState::Disconnected { app_context.repaint_animation(ui.ctx()); } - let tip = status.tooltip_text(); + let tip = status.tooltip_text(app_context); let resp = resp.on_hover_text(tip); if resp.clicked() diff --git a/src/ui/wallets/wallets_screen/mod.rs b/src/ui/wallets/wallets_screen/mod.rs index 7479a967b..7591ec4ae 100644 --- a/src/ui/wallets/wallets_screen/mod.rs +++ b/src/ui/wallets/wallets_screen/mod.rs @@ -23,7 +23,7 @@ use crate::ui::wallets::account_summary::{ }; use crate::ui::{MessageType, RootScreenType, ScreenLike, ScreenType}; use chrono::{DateTime, Utc}; -use dash_sdk::dash_spv::sync::{ProgressPercentage, SyncProgress as SpvSyncProgress, SyncState}; +use crate::context::connection_status::spv_phase_summary; use dash_sdk::dashcore_rpc::dashcore::Address; use dash_sdk::dpp::balances::credits::CREDITS_PER_DUFF; use eframe::egui::{self, ComboBox, Context, Ui}; @@ -1227,9 +1227,11 @@ impl WalletsBalancesScreen { .size(12.0) .color(DashColors::DASH_BLUE), ); - let phase_text = Self::spv_active_phase_text( - &snapshot.sync_progress, - ); + let phase_text = snapshot + .sync_progress + .as_ref() + .map(spv_phase_summary) + .unwrap_or_else(|| "starting...".to_string()); ui.label( RichText::new(format!("Syncing — {phase_text}")) .size(12.0) @@ -1314,58 +1316,6 @@ impl WalletsBalancesScreen { ); } - /// Get a text summary of the active SPV sync phase. - fn spv_active_phase_text(sync_progress: &Option) -> String { - let Some(progress) = sync_progress else { - return "starting...".to_string(); - }; - - // Grab target height from headers (blocks doesn't expose target_height directly) - let target_height = progress - .headers() - .ok() - .map(|h| h.target_height()) - .unwrap_or(0); - - // Check phases in order of execution - if let Ok(headers) = progress.headers() - && headers.state() == SyncState::Syncing - { - let pct = Self::simple_progress_pct(headers.current_height(), headers.target_height()); - return format!("Headers {pct}%"); - } - - if let Ok(fh) = progress.filter_headers() - && fh.state() == SyncState::Syncing - { - let pct = Self::simple_progress_pct(fh.current_height(), fh.target_height()); - return format!("Filter Headers {pct}%"); - } - - if let Ok(filters) = progress.filters() - && filters.state() == SyncState::Syncing - { - let pct = Self::simple_progress_pct(filters.current_height(), filters.target_height()); - return format!("Filters {pct}%"); - } - - if let Ok(blocks) = progress.blocks() - && blocks.state() == SyncState::Syncing - { - let pct = Self::simple_progress_pct(blocks.last_processed(), target_height); - return format!("Blocks {pct}%"); - } - - "syncing...".to_string() - } - - fn simple_progress_pct(current: u32, target: u32) -> u32 { - if target == 0 { - return 0; - } - ((current as f64 / target as f64) * 100.0).clamp(0.0, 100.0) as u32 - } - fn render_wallet_detail_panel(&mut self, ui: &mut Ui, ctx: &Context) -> AppAction { let Some(wallet_arc) = self.selected_wallet.clone() else { self.render_no_wallets_view(ui); From 13a989115f6ea6884bb29d245b04b9d13e0925c4 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 25 Feb 2026 12:22:12 +0100 Subject: [PATCH 16/17] style: apply nightly rustfmt formatting Co-Authored-By: Claude Opus 4.6 --- src/backend_task/core/create_asset_lock.rs | 8 ++++++-- src/ui/wallets/wallets_screen/mod.rs | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/src/backend_task/core/create_asset_lock.rs b/src/backend_task/core/create_asset_lock.rs index 4a90ef97c..5023f1531 100644 --- a/src/backend_task/core/create_asset_lock.rs +++ b/src/backend_task/core/create_asset_lock.rs @@ -53,7 +53,9 @@ impl AppContext { if let Ok(mut proofs) = self.transactions_waiting_for_finality.lock() { proofs.remove(&tx_id); } else { - tracing::warn!("Failed to clean up finality tracking for tx {tx_id}: Mutex poisoned"); + tracing::warn!( + "Failed to clean up finality tracking for tx {tx_id}: Mutex poisoned" + ); } return Err(format!("Failed to broadcast asset lock transaction: {}", e)); } @@ -108,7 +110,9 @@ impl AppContext { if let Ok(mut proofs) = self.transactions_waiting_for_finality.lock() { proofs.remove(&tx_id); } else { - tracing::warn!("Failed to clean up finality tracking for tx {tx_id}: Mutex poisoned"); + tracing::warn!( + "Failed to clean up finality tracking for tx {tx_id}: Mutex poisoned" + ); } return Err(format!("Failed to broadcast asset lock transaction: {}", e)); } diff --git a/src/ui/wallets/wallets_screen/mod.rs b/src/ui/wallets/wallets_screen/mod.rs index 7591ec4ae..92e4d8fa8 100644 --- a/src/ui/wallets/wallets_screen/mod.rs +++ b/src/ui/wallets/wallets_screen/mod.rs @@ -7,6 +7,7 @@ use crate::app::{AppAction, DesiredAppAction}; use crate::backend_task::BackendTask; use crate::backend_task::core::CoreTask; use crate::context::AppContext; +use crate::context::connection_status::spv_phase_summary; use crate::model::amount::Amount; use crate::model::wallet::{Wallet, WalletSeedHash, WalletTransaction}; use crate::spv::{CoreBackendMode, SpvStatus}; @@ -23,7 +24,6 @@ use crate::ui::wallets::account_summary::{ }; use crate::ui::{MessageType, RootScreenType, ScreenLike, ScreenType}; use chrono::{DateTime, Utc}; -use crate::context::connection_status::spv_phase_summary; use dash_sdk::dashcore_rpc::dashcore::Address; use dash_sdk::dpp::balances::credits::CREDITS_PER_DUFF; use eframe::egui::{self, ComboBox, Context, Ui}; From 0e7fe20c515ea6367dce1ea80afc901daae60a41 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 25 Feb 2026 12:50:20 +0100 Subject: [PATCH 17/17] fix(ui): address second round of PR review comments - Update test scenarios TS-07 through TS-11 and TS-23 to reflect new SPV phase format: "Headers: C / T (NN%)" instead of "Headers NN%" - Add Masternodes phase to TS-23 progression - Add developer mode precondition to test scenarios - Fix tooltip showing "syncing..." when SPV is fully synced (Running) - Update stale throttle comment to reflect new refresh rates Co-Authored-By: Claude Opus 4.6 --- .../manual-test-scenarios.md | 30 +++++++++++-------- src/context/connection_status.rs | 20 ++++++++----- 2 files changed, 29 insertions(+), 21 deletions(-) diff --git a/docs/ai-design/2026-02-24-sync-status-panel/manual-test-scenarios.md b/docs/ai-design/2026-02-24-sync-status-panel/manual-test-scenarios.md index 675bb77fd..7dcb2b409 100644 --- a/docs/ai-design/2026-02-24-sync-status-panel/manual-test-scenarios.md +++ b/docs/ai-design/2026-02-24-sync-status-panel/manual-test-scenarios.md @@ -10,6 +10,7 @@ ### Preconditions - Application launched and on the Wallets screen. +- Developer mode is enabled. - No HD wallet is selected (either no wallets exist or the user has not yet clicked one). ### Steps @@ -26,6 +27,7 @@ ### Preconditions - At least one HD wallet is loaded in the application. +- Developer mode is enabled. ### Steps 1. Navigate to the Wallets screen. @@ -118,8 +120,8 @@ ### Expected Results - A blue spinner is visible. -- Text reads: **Core: Syncing -- Headers NN%** where NN is a whole number 0-100. -- The percentage increases over time as headers are downloaded. +- Text reads: **Core: Syncing — Headers: C / T (NN%)** where C is the current height, T is the target height, and NN is a whole number 0-100. +- The numbers increase over time as headers are downloaded. --- @@ -135,8 +137,8 @@ ### Expected Results - A blue spinner is visible. -- Text reads: **Core: Syncing -- Filter Headers NN%** -- Percentage reflects progress of filter header download. +- Text reads: **Core: Syncing — Filter Headers: C / T (NN%)** +- Numbers reflect progress of filter header download. --- @@ -152,7 +154,7 @@ ### Expected Results - A blue spinner is visible. -- Text reads: **Core: Syncing -- Filters NN%** +- Text reads: **Core: Syncing — Filters: C / T (NN%)** --- @@ -168,7 +170,7 @@ ### Expected Results - A blue spinner is visible. -- Text reads: **Core: Syncing -- Blocks NN%** +- Text reads: **Core: Syncing — Blocks: C / T (NN%)** --- @@ -372,8 +374,9 @@ ### Preconditions - Application configured in SPV mode. -- Starting from a fresh state (no previously synced data) so that all four sync phases are traversed. +- Starting from a fresh state (no previously synced data) so that all five sync phases are traversed. - An HD wallet is selected. +- Developer mode is enabled. ### Steps 1. Start the application and let SPV sync begin. @@ -382,12 +385,13 @@ ### Expected Results - The Core line progresses through these states in order: 1. "Connecting..." - 2. "Syncing -- Headers NN%" (percentage climbs from low to 100) - 3. "Syncing -- Filter Headers NN%" - 4. "Syncing -- Filters NN%" - 5. "Syncing -- Blocks NN%" - 6. "Synced -- N peers" -- Each phase percentage starts low and progresses upward. + 2. "Syncing — Headers: C / T (NN%)" (numbers climb toward target) + 3. "Syncing — Masternodes: C / T (NN%)" + 4. "Syncing — Filter Headers: C / T (NN%)" + 5. "Syncing — Filters: C / T (NN%)" + 6. "Syncing — Blocks: C / T (NN%)" + 7. "Synced -- N peers" +- Each phase shows current/target heights and a percentage that progresses upward. - A spinner is visible during all syncing phases and disappears when fully synced. --- diff --git a/src/context/connection_status.rs b/src/context/connection_status.rs index d2170663f..e7e685247 100644 --- a/src/context/connection_status.rs +++ b/src/context/connection_status.rs @@ -261,13 +261,17 @@ impl ConnectionStatus { OverallConnectionState::Syncing => "Syncing", OverallConnectionState::Disconnected => "Disconnected", }; - let spv_label = app_context - .spv_manager() - .status() - .sync_progress - .as_ref() - .map(|p| format!("SPV: {}", spv_phase_summary(p))) - .unwrap_or_else(|| format!("SPV: {:?}", spv_status)); + let spv_label = if spv_status == SpvStatus::Running { + "SPV: Synced".to_string() + } else { + app_context + .spv_manager() + .status() + .sync_progress + .as_ref() + .map(|p| format!("SPV: {}", spv_phase_summary(p))) + .unwrap_or_else(|| format!("SPV: {:?}", spv_status)) + }; format!("{header}\n{spv_label}\n{dapi_status}") } } @@ -330,7 +334,7 @@ impl ConnectionStatus { } pub fn trigger_refresh(&self, app_context: &crate::context::AppContext) -> AppAction { - // throttle updates to once every 2 seconds + // throttle updates based on connection state (1s disconnected, 4s connected) let mut last_update = match self.last_update.lock() { Ok(guard) => guard, Err(poisoned) => poisoned.into_inner(),