From 7d3feafc7f50e42b0ab66577e35b9330b6c54dbd Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 22 Jul 2026 11:57:59 +0000 Subject: [PATCH 1/6] fix: centralize validation and fee hygiene - PROJ-001: enforce shared model validators in UI and backend paths - PROJ-002: reserve transfer and withdrawal fees with integer estimator results - PROJ-003: keep technical errors in banner details - PROJ-005: use named placeholders and complete translation units in scope - PROJ-006: route local identity removal through IdentityTask - RUST-001: replace fragile offsets and annotate upstream string matching Co-Authored-By: Codex Sol --- src/backend_task/dashpay/contact_requests.rs | 10 + src/backend_task/dashpay/payments.rs | 5 + src/backend_task/error.rs | 50 +++- src/backend_task/identity/mod.rs | 6 + .../identity/register_dpns_name.rs | 15 +- src/backend_task/identity/remove_identity.rs | 44 ++++ src/backend_task/migration/finish_unwire.rs | 2 +- src/backend_task/mod.rs | 4 + src/backend_task/tokens/mod.rs | 5 + src/model/dashpay.rs | 25 ++ src/model/dpns.rs | 79 ++++++ src/model/fee_estimation.rs | 12 + src/model/mod.rs | 2 + src/model/token.rs | 33 +++ src/model/validation.rs | 24 ++ src/model/wallet/mod.rs | 15 ++ .../contracts_documents_screen.rs | 7 +- .../document_action_screen.rs | 14 +- .../group_actions_screen.rs | 7 +- src/ui/dashpay/add_contact_screen.rs | 7 +- src/ui/dashpay/contact_profile_viewer.rs | 7 +- src/ui/dashpay/contacts_list.rs | 9 +- src/ui/dashpay/qr_scanner.rs | 7 +- src/ui/dashpay/send_payment.rs | 99 +++++--- src/ui/dpns/dpns_contested_names_screen.rs | 13 +- src/ui/helpers.rs | 80 +++--- src/ui/identities/identities_screen.rs | 132 +++++----- src/ui/identities/keys/add_key_screen.rs | 35 +-- src/ui/identities/keys/key_info_screen.rs | 29 ++- .../identities/register_dpns_name_screen.rs | 70 +----- src/ui/identities/transfer_screen.rs | 71 +++--- src/ui/identities/withdraw_screen.rs | 46 ++-- src/ui/masternodes/detail_screen.rs | 20 +- src/ui/tokens/tokens_screen/mod.rs | 7 +- src/ui/tokens/tokens_screen/my_tokens.rs | 16 +- src/ui/tokens/tokens_screen/token_creator.rs | 124 ++++----- src/ui/tools/grovestark_screen.rs | 7 +- src/ui/tools/transition_visualizer_screen.rs | 38 ++- src/ui/wallets/import_mnemonic_screen.rs | 55 ++-- src/ui/wallets/single_key_send_screen.rs | 72 +++--- src/ui/wallets/wallets_screen/mod.rs | 236 ++++++++++-------- src/wallet_backend/single_key.rs | 54 ++++ src/wallet_backend/wallet_meta.rs | 43 ++++ 43 files changed, 1061 insertions(+), 575 deletions(-) create mode 100644 src/backend_task/identity/remove_identity.rs create mode 100644 src/model/token.rs create mode 100644 src/model/validation.rs diff --git a/src/backend_task/dashpay/contact_requests.rs b/src/backend_task/dashpay/contact_requests.rs index 5e51bbf0d..59820909b 100644 --- a/src/backend_task/dashpay/contact_requests.rs +++ b/src/backend_task/dashpay/contact_requests.rs @@ -240,6 +240,16 @@ pub async fn send_contact_request_with_proof( account_label: Option, qr_auto_accept: Option, ) -> Result { + if let Some(label) = account_label.as_deref() + && let Err(error) = crate::model::dashpay::validate_account_label(label) + { + return Err(DashPayError::AccountLabelTooLong { + length: error.actual, + max: error.max, + } + .into()); + } + // Step 1: Resolve the recipient identity let to_username_or_id = to_username_or_id.trim().to_string(); diff --git a/src/backend_task/dashpay/payments.rs b/src/backend_task/dashpay/payments.rs index de0adca4c..c31213575 100644 --- a/src/backend_task/dashpay/payments.rs +++ b/src/backend_task/dashpay/payments.rs @@ -4,6 +4,7 @@ use crate::backend_task::error::TaskError; use crate::context::AppContext; use crate::model::dashpay::{ PaymentDirection as StoredPaymentDirection, PaymentStatus as StoredPaymentStatus, + validate_payment_memo, }; use crate::model::dashpay_derivation::derive_payment_address; use crate::model::qualified_identity::QualifiedIdentity; @@ -232,6 +233,10 @@ pub async fn send_payment_to_contact( use crate::backend_task::core::{CoreTask, PaymentRecipient, WalletPaymentRequest}; use dash_sdk::dpp::identity::accessors::IdentityGettersV0; + if let Some(memo) = memo.as_deref() { + validate_payment_memo(memo).map_err(|source| TaskError::DashPayMemoTooLong { source })?; + } + // Get a wallet from the identity's associated wallets let wallet = from_identity .associated_wallets diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs index 55b7ede98..3e58968ca 100644 --- a/src/backend_task/error.rs +++ b/src/backend_task/error.rs @@ -1187,6 +1187,35 @@ pub enum TaskError { source_error: Box, }, + /// A DPNS label failed the shared registration-format validator. + #[error( + "The DPNS name format is not valid. Use 3 to 63 letters, numbers, or hyphens, without a hyphen at either end." + )] + InvalidDpnsName { + validation: crate::model::dpns::DpnsNameValidationResult, + }, + + /// A DashPay memo exceeded the shared character limit. + #[error("The payment memo is too long. Use 100 characters or fewer and try again.")] + DashPayMemoTooLong { + #[source] + source: crate::model::validation::TextLengthError, + }, + + /// A searchable contract keyword fell outside the shared character range. + #[error("A contract keyword has an invalid length. Use 3 to 50 characters and try again.")] + InvalidContractKeywordLength { + #[source] + source: crate::model::validation::TextLengthError, + }, + + /// A wallet alias exceeded the shared character limit. + #[error("The wallet name is too long. Use 64 characters or fewer and try again.")] + InvalidWalletAliasLength { + #[source] + source: crate::model::validation::TextLengthError, + }, + /// A document's unique values conflict with an existing entry. #[error( "This request conflicts with an existing entry. Please use different values and try again." @@ -2614,9 +2643,8 @@ pub fn is_empty_tree_proof(error: &SdkError) -> bool { leaf.is_some_and(|s| s.to_lowercase().contains(EMPTY_TREE_PROOF_MARKER)) } -// TODO: Replace string parsing with a pre-check on amount + fee > spendable -// before calling the SDK builder, or wait for upstream to add a typed -// ProtocolError variant (currently ProtocolError::ShieldedBuildError(String)). +// TODO: workaround — replace with a typed shielded-build error or a local +// amount-plus-fee pre-check when the SDK exposes one (see issue #714). /// Parse the "amount + fee exceeds spendable" pattern from DPP builder errors. /// @@ -2627,18 +2655,22 @@ pub fn is_empty_tree_proof(error: &SdkError) -> bool { /// /// Returns `(amount, fee, spendable)` on match. fn parse_fee_exceeds_spendable(detail: &str) -> Option<(u64, u64, u64)> { + const AMOUNT_MARKER: &str = "amount "; + const FEE_MARKER: &str = "fee "; + const SPENDABLE_MARKER: &str = "exceeds total spendable value "; + // Pattern: "{type} amount {A} + fee {F} = {sum} exceeds total spendable value {S}" - let amount_start = detail.find("amount ")? + 7; + let amount_start = detail.find(AMOUNT_MARKER)? + AMOUNT_MARKER.len(); let amount_end = detail[amount_start..].find(' ')? + amount_start; let amount: u64 = detail[amount_start..amount_end].parse().ok()?; - let fee_marker = detail.find("fee ")?; - let fee_start = fee_marker + 4; + let fee_marker = detail.find(FEE_MARKER)?; + let fee_start = fee_marker + FEE_MARKER.len(); let fee_end = detail[fee_start..].find(' ')? + fee_start; let fee: u64 = detail[fee_start..fee_end].parse().ok()?; - let spendable_marker = detail.find("exceeds total spendable value ")?; - let spendable_start = spendable_marker + 30; + let spendable_marker = detail.find(SPENDABLE_MARKER)?; + let spendable_start = spendable_marker + SPENDABLE_MARKER.len(); let spendable: u64 = detail[spendable_start..].trim().parse().ok()?; Some((amount, fee, spendable)) @@ -2657,6 +2689,8 @@ pub fn shielded_build_error(detail: String) -> TaskError { fee, spendable, } + // TODO: workaround — replace this upstream wording match with a typed + // shielded anchor error when the SDK exposes one (see issue #714). } else if detail.contains("AnchorMismatch") { TaskError::ShieldedAnchorMismatch { detail } } else { diff --git a/src/backend_task/identity/mod.rs b/src/backend_task/identity/mod.rs index 69399bced..41b9912d3 100644 --- a/src/backend_task/identity/mod.rs +++ b/src/backend_task/identity/mod.rs @@ -9,6 +9,7 @@ mod refresh_identity; mod refresh_loaded_identities_dpns_names; mod register_dpns_name; mod register_identity; +mod remove_identity; mod top_up_identity; mod transfer; mod withdraw_from_identity; @@ -508,6 +509,10 @@ pub enum IdentityTask { key_id: Option, }, RegisterDpnsName(RegisterDpnsNameInput), + /// Remove a local identity and its associated voter identity, if present. + RemoveIdentity { + identity_id: Identifier, + }, RefreshIdentity(QualifiedIdentity), RefreshLoadedIdentitiesOwnedDPNSNames, } @@ -851,6 +856,7 @@ impl AppContext { IdentityTask::RegisterDpnsName(input) => { Ok(self.register_dpns_name(sdk, input).await?) } + IdentityTask::RemoveIdentity { identity_id } => self.remove_identity(identity_id), IdentityTask::RefreshIdentity(qualified_identity) => { self.refresh_identity(sdk, qualified_identity, sender).await } diff --git a/src/backend_task/identity/register_dpns_name.rs b/src/backend_task/identity/register_dpns_name.rs index d313af6a1..dff3cb2e5 100644 --- a/src/backend_task/identity/register_dpns_name.rs +++ b/src/backend_task/identity/register_dpns_name.rs @@ -2,7 +2,13 @@ use std::collections::BTreeMap; use crate::backend_task::FeeResult; use crate::backend_task::error::TaskError; -use crate::{context::AppContext, model::qualified_identity::DPNSNameInfo}; +use crate::{ + context::AppContext, + model::{ + dpns::{DpnsNameValidationResult, validate_dpns_name}, + qualified_identity::DPNSNameInfo, + }, +}; use bip39::rand::{Rng, SeedableRng, rngs::StdRng}; use dash_sdk::{ Sdk, @@ -37,6 +43,11 @@ impl AppContext { sdk: &Sdk, input: RegisterDpnsNameInput, ) -> Result { + let validation = validate_dpns_name(&input.name_input); + if validation != DpnsNameValidationResult::Valid { + return Err(TaskError::InvalidDpnsName { validation }); + } + let mut rng = StdRng::from_entropy(); let dpns_contract = self.dpns_contract.clone(); @@ -217,7 +228,7 @@ impl AppContext { qualified_identity.dpns_names = owned_dpns_names; if qualified_identity.alias.is_none() { - qualified_identity.alias = Some(format!("{}.dash", input.name_input)); + qualified_identity.alias = Some(format!("{name}.dash", name = input.name_input)); } let refreshed_identity = dash_sdk::platform::Identity::fetch_by_identifier( diff --git a/src/backend_task/identity/remove_identity.rs b/src/backend_task/identity/remove_identity.rs new file mode 100644 index 000000000..111d57512 --- /dev/null +++ b/src/backend_task/identity/remove_identity.rs @@ -0,0 +1,44 @@ +use crate::backend_task::{BackendTaskSuccessResult, TaskError}; +use crate::context::AppContext; +use dash_sdk::dpp::identity::accessors::IdentityGettersV0; +use dash_sdk::platform::Identifier; + +impl AppContext { + pub(super) fn remove_identity( + &self, + identity_id: Identifier, + ) -> Result { + let associated_voter_identity_id = self + .load_local_qualified_identities()? + .into_iter() + .find(|identity| identity.identity.id() == identity_id) + .and_then(|identity| { + identity + .associated_voter_identity + .map(|(voter_identity, _)| voter_identity.id()) + }); + + self.delete_local_qualified_identity(&identity_id)?; + + let mut removed_identity_ids = vec![identity_id]; + let mut associated_cleanup_failed = false; + if let Some(voter_id) = associated_voter_identity_id.filter(|id| *id != identity_id) { + match self.delete_local_qualified_identity(&voter_id) { + Ok(()) => removed_identity_ids.push(voter_id), + Err(error) => { + associated_cleanup_failed = true; + tracing::warn!( + ?error, + voter_identity_id = %voter_id, + "Associated voter identity cleanup failed" + ); + } + } + } + + Ok(BackendTaskSuccessResult::RemovedIdentities { + identity_ids: removed_identity_ids, + associated_cleanup_failed, + }) + } +} diff --git a/src/backend_task/migration/finish_unwire.rs b/src/backend_task/migration/finish_unwire.rs index 1a9ef72a9..4f219544d 100644 --- a/src/backend_task/migration/finish_unwire.rs +++ b/src/backend_task/migration/finish_unwire.rs @@ -2195,7 +2195,7 @@ fn migrate_wallet_meta_rows(app_context: &Arc) -> Result<(), TaskErr let view = backend.wallet_meta(); let outcome = migrate_wallet_meta_rows_from_conn( &conn, - |seed_hash, meta| view.set(app_context.network, &seed_hash, &meta), + |seed_hash, meta| view.set_migrated(app_context.network, &seed_hash, &meta), app_context.network, )?; tracing::info!( diff --git a/src/backend_task/mod.rs b/src/backend_task/mod.rs index 6c5b99592..872eb0639 100644 --- a/src/backend_task/mod.rs +++ b/src/backend_task/mod.rs @@ -669,6 +669,10 @@ pub enum BackendTaskSuccessResult { TransferredCredits(FeeResult), WithdrewFromIdentity(FeeResult), RegisteredDpnsName(FeeResult), + RemovedIdentities { + identity_ids: Vec, + associated_cleanup_failed: bool, + }, RefreshedIdentity(QualifiedIdentity), LoadedIdentity(QualifiedIdentity), /// This identity's keys were sealed under a password (opt-in). diff --git a/src/backend_task/tokens/mod.rs b/src/backend_task/tokens/mod.rs index 943d62f7f..9d3b3304c 100644 --- a/src/backend_task/tokens/mod.rs +++ b/src/backend_task/tokens/mod.rs @@ -258,6 +258,11 @@ impl AppContext { signing_key, params, } => { + params + .contract_keywords + .iter() + .try_for_each(|keyword| crate::model::token::validate_contract_keyword(keyword)) + .map_err(|source| TaskError::InvalidContractKeywordLength { source })?; let alias = params.token_names[0].0.clone(); let data_contract = self .build_data_contract_v1_with_one_token(identity.identity.id(), *params) diff --git a/src/model/dashpay.rs b/src/model/dashpay.rs index 3af3f9b50..e0466d809 100644 --- a/src/model/dashpay.rs +++ b/src/model/dashpay.rs @@ -5,6 +5,23 @@ use dash_sdk::dpp::document::DocumentV0Getters; use dash_sdk::platform::{Document, Identifier}; use serde::{Deserialize, Serialize}; +use super::validation::{TextLengthError, validate_char_count}; + +/// Maximum number of characters stored in a DashPay payment memo. +pub const MAX_PAYMENT_MEMO_CHARS: usize = 100; +/// Maximum number of characters stored in a contact-request account label. +pub const MAX_ACCOUNT_LABEL_CHARS: usize = 100; + +/// Validate an optional DashPay payment memo. +pub fn validate_payment_memo(memo: &str) -> Result<(), TextLengthError> { + validate_char_count(memo, 0, MAX_PAYMENT_MEMO_CHARS) +} + +/// Validate a DashPay contact-request account label. +pub fn validate_account_label(label: &str) -> Result<(), TextLengthError> { + validate_char_count(label, 0, MAX_ACCOUNT_LABEL_CHARS) +} + /// The recipient (`toUserId`) of a DashPay `contactRequest` document. /// /// Returns `None` when the field is absent or does not hold a readable @@ -543,4 +560,12 @@ mod tests { fn avatar_url_scheme_check_ignores_surrounding_whitespace() { assert!(validate_profile_fields("", "", " https://example.com/a.png ").is_empty()); } + + #[test] + fn dashpay_text_limits_count_characters() { + assert!(validate_payment_memo(&"é".repeat(100)).is_ok()); + assert!(validate_payment_memo(&"m".repeat(101)).is_err()); + assert!(validate_account_label(&"é".repeat(100)).is_ok()); + assert!(validate_account_label(&"l".repeat(101)).is_err()); + } } diff --git a/src/model/dpns.rs b/src/model/dpns.rs index 06f1cd9b5..d8e4b167a 100644 --- a/src/model/dpns.rs +++ b/src/model/dpns.rs @@ -12,6 +12,55 @@ use dash_sdk::platform::{Document, Identifier}; /// The `.dash` parent domain suffix (case-insensitive match target). const DASH_SUFFIX: &str = ".dash"; +/// Result of validating a bare label for DPNS registration. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum DpnsNameValidationResult { + Valid, + TooShort, + TooLong, + InvalidCharacter(char), + StartsWithHyphen, + EndsWithHyphen, +} + +/// Validate the format of a bare DPNS label before registration. +pub fn validate_dpns_name(name: &str) -> DpnsNameValidationResult { + if name.len() < 3 { + return DpnsNameValidationResult::TooShort; + } + if name.len() > 63 { + return DpnsNameValidationResult::TooLong; + } + if name.starts_with('-') { + return DpnsNameValidationResult::StartsWithHyphen; + } + if name.ends_with('-') { + return DpnsNameValidationResult::EndsWithHyphen; + } + for character in name.chars() { + if !character.is_ascii_alphanumeric() && character != '-' { + return DpnsNameValidationResult::InvalidCharacter(character); + } + } + DpnsNameValidationResult::Valid +} + +impl DpnsNameValidationResult { + /// Return user guidance for an invalid label. + pub fn error_message(self) -> Option { + match self { + Self::Valid => None, + Self::TooShort => Some("Name must be at least 3 characters long.".to_string()), + Self::TooLong => Some("Name must be no more than 63 characters long.".to_string()), + Self::InvalidCharacter(character) => Some(format!( + "The character '{character}' is not allowed. Use only letters, numbers, and hyphens." + )), + Self::StartsWithHyphen => Some("Name cannot start with a hyphen.".to_string()), + Self::EndsWithHyphen => Some("Name cannot end with a hyphen.".to_string()), + } + } +} + /// Extract the bare label from a DPNS input and apply homograph-safe normalization. /// /// Handles all common user inputs: @@ -179,4 +228,34 @@ mod tests { Err(NonDashDomainError) ); } + + #[test] + fn dpns_registration_name_accepts_boundary_lengths() { + assert_eq!(validate_dpns_name("abc"), DpnsNameValidationResult::Valid); + assert_eq!( + validate_dpns_name(&"a".repeat(63)), + DpnsNameValidationResult::Valid + ); + } + + #[test] + fn dpns_registration_name_rejects_invalid_formats() { + assert_eq!(validate_dpns_name("ab"), DpnsNameValidationResult::TooShort); + assert_eq!( + validate_dpns_name(&"a".repeat(64)), + DpnsNameValidationResult::TooLong + ); + assert_eq!( + validate_dpns_name("-alice"), + DpnsNameValidationResult::StartsWithHyphen + ); + assert_eq!( + validate_dpns_name("alice-"), + DpnsNameValidationResult::EndsWithHyphen + ); + assert_eq!( + validate_dpns_name("ali_ce"), + DpnsNameValidationResult::InvalidCharacter('_') + ); + } } diff --git a/src/model/fee_estimation.rs b/src/model/fee_estimation.rs index ba536ee4c..03495219e 100644 --- a/src/model/fee_estimation.rs +++ b/src/model/fee_estimation.rs @@ -23,6 +23,11 @@ use dash_sdk::dpp::state_transition::address_credit_withdrawal_transition::Addre use dash_sdk::dpp::state_transition::address_credit_withdrawal_transition::v0::AddressCreditWithdrawalTransitionV0; use dash_sdk::dpp::state_transition::address_funding_from_asset_lock_transition::AddressFundingFromAssetLockTransition; use dash_sdk::dpp::state_transition::address_funding_from_asset_lock_transition::v0::AddressFundingFromAssetLockTransitionV0; + +/// Subtract an estimated fee from a credit balance without floating-point conversion. +pub const fn max_spendable_credits(balance: u64, estimated_fee: u64) -> u64 { + balance.saturating_sub(estimated_fee) +} use dash_sdk::dpp::version::PlatformVersion; use dash_sdk::dpp::withdrawal::Pooling; use std::collections::BTreeMap; @@ -1102,6 +1107,13 @@ mod tests { assert_eq!(estimator.estimate_credit_transfer(), 100_000); } + #[test] + fn max_spendable_credits_is_exact_above_f64_integer_precision() { + let balance = (1_u64 << 53) + 17; + assert_eq!(max_spendable_credits(balance, 11), balance - 11); + assert_eq!(max_spendable_credits(10, 11), 0); + } + #[test] fn test_identity_create_estimate() { let estimator = PlatformFeeEstimator::new(); diff --git a/src/model/mod.rs b/src/model/mod.rs index b6feb0ec1..394de1fce 100644 --- a/src/model/mod.rs +++ b/src/model/mod.rs @@ -24,6 +24,8 @@ pub mod selected_wallet; pub mod settings; pub mod single_key; pub mod spv_status; +pub mod token; pub mod user_role; +pub mod validation; pub mod wallet; pub mod wallet_association; diff --git a/src/model/token.rs b/src/model/token.rs new file mode 100644 index 000000000..de3669c7d --- /dev/null +++ b/src/model/token.rs @@ -0,0 +1,33 @@ +use super::validation::{TextLengthError, validate_char_count}; + +/// Minimum number of characters in a searchable contract keyword. +pub const MIN_CONTRACT_KEYWORD_CHARS: usize = 3; +/// Maximum number of characters in a searchable contract keyword. +pub const MAX_CONTRACT_KEYWORD_CHARS: usize = 50; + +/// Validate a searchable data-contract keyword. +pub fn validate_contract_keyword(keyword: &str) -> Result<(), TextLengthError> { + validate_char_count( + keyword, + MIN_CONTRACT_KEYWORD_CHARS, + MAX_CONTRACT_KEYWORD_CHARS, + ) +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn contract_keyword_accepts_boundary_lengths() { + assert!(validate_contract_keyword("abc").is_ok()); + assert!(validate_contract_keyword(&"k".repeat(50)).is_ok()); + } + + #[test] + fn contract_keyword_rejects_outside_character_limits() { + assert!(validate_contract_keyword("ab").is_err()); + assert!(validate_contract_keyword(&"k".repeat(51)).is_err()); + assert!(validate_contract_keyword(&"é".repeat(50)).is_ok()); + } +} diff --git a/src/model/validation.rs b/src/model/validation.rs new file mode 100644 index 000000000..8ce37dce7 --- /dev/null +++ b/src/model/validation.rs @@ -0,0 +1,24 @@ +/// A text field is outside its permitted character-count range. +#[derive(Debug, Clone, Copy, PartialEq, Eq, thiserror::Error)] +#[error("text contains {actual} characters; expected between {min} and {max}")] +pub struct TextLengthError { + /// Actual character count. + pub actual: usize, + /// Minimum permitted character count. + pub min: usize, + /// Maximum permitted character count. + pub max: usize, +} + +pub(crate) fn validate_char_count( + value: &str, + min: usize, + max: usize, +) -> Result<(), TextLengthError> { + let actual = value.chars().count(); + if (min..=max).contains(&actual) { + Ok(()) + } else { + Err(TextLengthError { actual, min, max }) + } +} diff --git a/src/model/wallet/mod.rs b/src/model/wallet/mod.rs index f3ab6dec7..761b53608 100644 --- a/src/model/wallet/mod.rs +++ b/src/model/wallet/mod.rs @@ -8,6 +8,7 @@ pub mod single_key; use crate::database::WalletError; use crate::model::secret::Secret; +use crate::model::validation::{TextLengthError, validate_char_count}; use crate::model::wallet::auth_pubkey_cache::AuthPubkeyCache; use crate::wallet_backend::poison::RwLockRecover; use dash_sdk::dpp::address_funds::PlatformAddress; @@ -30,6 +31,14 @@ use std::ops::Range; use std::sync::{Arc, RwLock}; use thiserror::Error; +/// Maximum number of characters in a wallet alias. +pub const MAX_WALLET_ALIAS_CHARS: usize = 64; + +/// Validate a wallet alias before it is persisted. +pub fn validate_wallet_alias(alias: &str) -> Result<(), TextLengthError> { + validate_char_count(alias, 0, MAX_WALLET_ALIAS_CHARS) +} + /// Why a set of payment recipients was rejected. /// /// Model-local so this pure validator carries no dependency on the @@ -3823,4 +3832,10 @@ mod tests { assert!(!wallet.reconcile_platform_address(&foreign, network)); assert!(!wallet.known_addresses.contains_key(&foreign)); } + + #[test] + fn wallet_alias_limit_counts_characters() { + assert!(validate_wallet_alias(&"é".repeat(64)).is_ok()); + assert!(validate_wallet_alias(&"w".repeat(65)).is_err()); + } } diff --git a/src/ui/contracts_documents/contracts_documents_screen.rs b/src/ui/contracts_documents/contracts_documents_screen.rs index 3074521f4..bb1ce83c5 100644 --- a/src/ui/contracts_documents/contracts_documents_screen.rs +++ b/src/ui/contracts_documents/contracts_documents_screen.rs @@ -251,14 +251,15 @@ impl DocumentQueryScreen { FetchDocumentsPage(parsed_query), ))); } - Err(e) => { + Err(error) => { self.query_banner.take_and_clear(); self.document_query_status = DocumentQueryStatus::Error; MessageBanner::set_global( ui.ctx(), - format!("Failed to parse query properly: {}", e), + "The document query is not valid. Check its fields and try again.", MessageType::Error, - ); + ) + .with_details(error); } } } diff --git a/src/ui/contracts_documents/document_action_screen.rs b/src/ui/contracts_documents/document_action_screen.rs index 802428320..d96656b35 100644 --- a/src/ui/contracts_documents/document_action_screen.rs +++ b/src/ui/contracts_documents/document_action_screen.rs @@ -1027,12 +1027,13 @@ impl DocumentActionScreen { identity_key: key.clone(), })) } - Err(e) => { + Err(error) => { MessageBanner::set_global( self.app_context.egui_ctx(), - format!("Failed to build document: {}", e), + "The document could not be built. Check its fields and try again.", crate::ui::MessageType::Error, - ); + ) + .with_details(error); BackendTask::None } } @@ -1133,12 +1134,13 @@ impl DocumentActionScreen { token_payment_info, })) } - Err(e) => { + Err(error) => { MessageBanner::set_global( self.app_context.egui_ctx(), - format!("Failed to build updated document: {}", e), + "The updated document could not be built. Check its fields and try again.", crate::ui::MessageType::Error, - ); + ) + .with_details(error); BackendTask::None } } diff --git a/src/ui/contracts_documents/group_actions_screen.rs b/src/ui/contracts_documents/group_actions_screen.rs index 835c6306b..9f5ffcad4 100644 --- a/src/ui/contracts_documents/group_actions_screen.rs +++ b/src/ui/contracts_documents/group_actions_screen.rs @@ -267,14 +267,15 @@ impl GroupActionsScreen { }; let identity_token_info = match IdentityTokenInfo::try_from_identity_token_balance_with_lookup(&identity_token_balance, &self.app_context) { Ok(identity_token_info) => identity_token_info, - Err(e) => { + Err(error) => { self.fetch_group_actions_status = FetchGroupActionsStatus::Error; MessageBanner::set_global( ui.ctx(), - format!("Failed to get identity token info: {}", e), + "Token information for this identity could not be loaded. Refresh and try again.", MessageType::Error, - ); + ) + .with_details(error); return; } }; diff --git a/src/ui/dashpay/add_contact_screen.rs b/src/ui/dashpay/add_contact_screen.rs index c9811b934..2635703aa 100644 --- a/src/ui/dashpay/add_contact_screen.rs +++ b/src/ui/dashpay/add_contact_screen.rs @@ -4,6 +4,7 @@ use crate::backend_task::dashpay::errors::DashPayError; use crate::backend_task::error::TaskError; use crate::backend_task::{BackendTask, BackendTaskSuccessResult}; use crate::context::AppContext; +use crate::model::dashpay::validate_account_label; use crate::model::qualified_identity::QualifiedIdentity; use crate::model::wallet::Wallet; use crate::ui::components::ResultBannerExt; @@ -143,10 +144,10 @@ impl AddContactScreen { } // Validate account label length - if self.account_label.len() > 100 { + if let Err(error) = validate_account_label(&self.account_label) { let error = DashPayError::AccountLabelTooLong { - length: self.account_label.len(), - max: 100, + length: error.actual, + max: error.max, }; self.status = ContactRequestStatus::Error(error); return AppAction::None; diff --git a/src/ui/dashpay/contact_profile_viewer.rs b/src/ui/dashpay/contact_profile_viewer.rs index 0779b0449..78cc65838 100644 --- a/src/ui/dashpay/contact_profile_viewer.rs +++ b/src/ui/dashpay/contact_profile_viewer.rs @@ -375,12 +375,13 @@ impl ContactProfileViewerScreen { MessageType::Success, ); } - Err(e) => { + Err(error) => { crate::ui::components::MessageBanner::set_global( ui.ctx(), - format!("Failed to save: {}", e), + "The private contact information could not be saved. Check available disk space and try again.", MessageType::Error, - ); + ) + .with_details(error); } } } diff --git a/src/ui/dashpay/contacts_list.rs b/src/ui/dashpay/contacts_list.rs index b118a2c53..e66ae69ce 100644 --- a/src/ui/dashpay/contacts_list.rs +++ b/src/ui/dashpay/contacts_list.rs @@ -795,9 +795,14 @@ impl ContactsList { existing.notes, new_hidden, ); - if let Err(e) = sidecar_result { + if let Err(error) = sidecar_result { + tracing::warn!( + ?error, + "Failed to update private contact information" + ); self.message = Some(( - format!("Failed to update contact: {}", e), + "The contact could not be updated. Check available disk space and try again." + .to_string(), MessageType::Error, )); } else { diff --git a/src/ui/dashpay/qr_scanner.rs b/src/ui/dashpay/qr_scanner.rs index 1dcc56a81..1c4e5d255 100644 --- a/src/ui/dashpay/qr_scanner.rs +++ b/src/ui/dashpay/qr_scanner.rs @@ -89,13 +89,14 @@ impl QRScannerScreen { MessageType::Success, ); } - Err(e) => { + Err(error) => { self.parsed_qr_data = None; MessageBanner::set_global( self.app_context.egui_ctx(), - format!("Invalid QR code: {}", e), + "The QR code is not valid for DashPay. Scan a DashPay code and try again.", MessageType::Error, - ); + ) + .with_details(error); } } } diff --git a/src/ui/dashpay/send_payment.rs b/src/ui/dashpay/send_payment.rs index 0928e5088..b36d98370 100644 --- a/src/ui/dashpay/send_payment.rs +++ b/src/ui/dashpay/send_payment.rs @@ -3,6 +3,7 @@ use crate::backend_task::dashpay::DashPayTask; use crate::backend_task::{BackendTask, BackendTaskSuccessResult}; use crate::context::AppContext; use crate::model::amount::Amount; +use crate::model::dashpay::{MAX_PAYMENT_MEMO_CHARS, validate_payment_memo}; use crate::model::fee_estimation::format_duffs_as_dash; use crate::model::qualified_identity::QualifiedIdentity; use crate::model::wallet::Wallet; @@ -99,36 +100,46 @@ impl SendPaymentScreen { } // Check wallet is available and unlocked - let wallet_check = if let Some(wallet) = &self.selected_wallet { - match wallet.read() { - Ok(guard) => { - if guard.is_open() { - Ok(()) - } else { - Err("Wallet must be unlocked to send a payment".to_string()) - } - } - Err(e) => Err(format!("Failed to access wallet: {}", e)), - } - } else { - Err("No wallet associated with this identity".to_string()) - }; - - if let Err(e) = wallet_check { - MessageBanner::set_global(self.app_context.egui_ctx(), &e, MessageType::Error); + let Some(wallet) = &self.selected_wallet else { + MessageBanner::set_global( + self.app_context.egui_ctx(), + "No wallet is associated with this identity. Load its wallet and try again.", + MessageType::Error, + ); return AppAction::None; + }; + match wallet.read() { + Ok(guard) if guard.is_open() => {} + Ok(_) => { + MessageBanner::set_global( + self.app_context.egui_ctx(), + "Unlock the wallet before sending this payment, then try again.", + MessageType::Error, + ); + return AppAction::None; + } + Err(error) => { + MessageBanner::set_global( + self.app_context.egui_ctx(), + "The wallet could not be opened for this payment. Wait a moment and try again.", + MessageType::Error, + ) + .with_details(error); + return AppAction::None; + } } // Resolve the amount in duffs at the UI edge — no floating-point value // crosses into the backend. let amount_duffs = match self.amount.dash_to_duffs() { Ok(duffs) => duffs, - Err(e) => { + Err(error) => { MessageBanner::set_global( self.app_context.egui_ctx(), - format!("Invalid amount: {}", e), + "The payment amount is not valid. Check the amount and try again.", MessageType::Error, - ); + ) + .with_details(error); return AppAction::None; } }; @@ -151,17 +162,20 @@ impl SendPaymentScreen { } fn show_success(&self, ui: &mut Ui) -> AppAction { + let message = if let Some(tx_id) = &self.tx_id { + format!( + "Payment of {amount} sent successfully!\n\nTransaction ID: {tx_id}", + amount = self.amount + ) + } else { + format!( + "Payment of {amount} sent successfully!", + amount = self.amount + ) + }; crate::ui::helpers::show_success_screen( ui, - format!( - "Payment of {} sent successfully!{}", - self.amount, - if let Some(tx_id) = &self.tx_id { - format!("\n\nTransaction ID: {}", tx_id) - } else { - String::new() - } - ), + message, vec![ ("Back to DashPay".to_string(), AppAction::GoToMainScreen), ("Send Another Payment".to_string(), AppAction::PopScreen), @@ -257,7 +271,7 @@ impl SendPaymentScreen { 0.0 }; ui.label( - RichText::new(format!("{:.8} DASH", balance_dash)) + RichText::new(format!("{balance_dash:.8} DASH")) .color(DashColors::text_primary(dark_mode)), ); }); @@ -278,7 +292,7 @@ impl SendPaymentScreen { } else { let dark_mode = ui.style().visuals.dark_mode; ui.label( - RichText::new(format!("{}", self.to_contact_id)) + RichText::new(self.to_contact_id.to_string(Encoding::Base58)) .color(DashColors::text_primary(dark_mode)), ); } @@ -334,7 +348,11 @@ impl SendPaymentScreen { ); let dark_mode = ui.style().visuals.dark_mode; ui.label( - RichText::new(format!("{}/100 characters", self.memo.len())) + RichText::new(format!( + "{count}/{max} characters", + count = self.memo.chars().count(), + max = MAX_PAYMENT_MEMO_CHARS + )) .small() .color(DashColors::text_secondary(dark_mode)), ); @@ -358,12 +376,13 @@ impl SendPaymentScreen { }); if ui.add_enabled(send_enabled, send_button).clicked() { - if self.memo.len() > 100 { + if let Err(error) = validate_payment_memo(&self.memo) { MessageBanner::set_global( ui.ctx(), - "Memo must be 100 characters or less", + "The memo is too long. Use 100 characters or fewer and try again.", MessageType::Error, - ); + ) + .with_details(error); } else { action = self.send_payment(); } @@ -457,7 +476,7 @@ impl ScreenLike for SendPaymentScreen { self.sending = false; if let BackendTaskSuccessResult::DashPayPaymentSent(_recipient, address, _amount) = result { self.payment_success = true; - self.tx_id = Some(format!("Sent to {}", address)); + self.tx_id = Some(format!("Sent to {address}")); } } } @@ -693,12 +712,12 @@ impl PaymentHistory { let amount_str = format_duffs_as_dash(payment.amount); if payment.is_incoming { ui.label( - RichText::new(format!("+{}", amount_str)) + RichText::new(format!("+{amount_str}")) .color(egui::Color32::DARK_GREEN), ); } else { ui.label( - RichText::new(format!("-{}", amount_str)) + RichText::new(format!("-{amount_str}")) .color(egui::Color32::DARK_RED), ); } @@ -707,7 +726,7 @@ impl PaymentHistory { // Memo if let Some(memo) = &payment.memo { ui.label( - RichText::new(format!("\"{}\"", memo)) + RichText::new(format!("\"{memo}\"")) .italics() .color(DashColors::text_secondary(dark_mode)), ); @@ -723,7 +742,7 @@ impl PaymentHistory { // Timestamp let payment_time_text = format_relative_time(payment.timestamp) - .map(|t| format!("• {}", t)) + .map(|transaction| format!("• {transaction}")) .unwrap_or_default(); if !payment_time_text.is_empty() { ui.label( diff --git a/src/ui/dpns/dpns_contested_names_screen.rs b/src/ui/dpns/dpns_contested_names_screen.rs index 3db22d103..ddbe86efd 100644 --- a/src/ui/dpns/dpns_contested_names_screen.rs +++ b/src/ui/dpns/dpns_contested_names_screen.rs @@ -965,16 +965,17 @@ impl DPNSScreen { { MessageBanner::set_global( ui.ctx(), - format!("Failed to set alias: {}", e), - MessageType::Error, - ); + "The alias could not be saved. Check available disk space and try again.", + MessageType::Error, + ) + .with_details(e); } else { MessageBanner::set_global( ui.ctx(), format!( - "Alias set to '{}' for identity {}", - alias_with_suffix, - identifier.to_string(Encoding::Base58) + "Alias set to '{alias}' for identity {identity_id}", + alias = alias_with_suffix, + identity_id = identifier.to_string(Encoding::Base58) ), MessageType::Success, ); diff --git a/src/ui/helpers.rs b/src/ui/helpers.rs index 2e07b044e..1a8628da1 100644 --- a/src/ui/helpers.rs +++ b/src/ui/helpers.rs @@ -109,22 +109,22 @@ use super::tokens::tokens_screen::IdentityTokenInfo; /// Returns a string like "Key 0 | AUTHENTICATION | CRITICAL | ECDSA_SECP256K1" pub fn format_key_label(key: &IdentityPublicKey) -> String { format!( - "Key {} | {} | {} | {}", - key.id(), - key.purpose(), - key.security_level(), - key.key_type() + "Key {key_id} | {purpose} | {security_level} | {key_type}", + key_id = key.id(), + purpose = key.purpose(), + security_level = key.security_level(), + key_type = key.key_type() ) } /// Formats a key label with a [DEV] suffix for dev mode display. pub fn format_key_label_dev(key: &IdentityPublicKey) -> String { format!( - "Key {} | {} | {} | {} [DEV]", - key.id(), - key.purpose(), - key.security_level(), - key.key_type() + "Key {key_id} | {purpose} | {security_level} | {key_type} [DEV]", + key_id = key.id(), + purpose = key.purpose(), + security_level = key.security_level(), + key_type = key.key_type() ) } @@ -195,7 +195,7 @@ pub fn check_token_authorization( let group = match action_takers { AuthorizedActionTakers::NoOne => { - error_message = Some(format!("{} is not allowed on this token", action_name)); + error_message = Some(format!("{action_name} is not allowed on this token")); None } AuthorizedActionTakers::ContractOwner => { @@ -203,8 +203,8 @@ pub fn check_token_authorization( != identity_token_info.identity.identity.id() { error_message = Some(format!( - "You are not allowed to {} this token. Only the contract owner is.", - action_name.to_lowercase() + "You are not allowed to {action} this token. Only the contract owner is.", + action = action_name.to_lowercase() )); } None @@ -212,8 +212,8 @@ pub fn check_token_authorization( AuthorizedActionTakers::Identity(identifier) => { if identifier != &identity_token_info.identity.identity.id() { error_message = Some(format!( - "You are not allowed to {} this token", - action_name.to_lowercase() + "You are not allowed to {action} this token", + action = action_name.to_lowercase() )); } None @@ -234,8 +234,12 @@ pub fn check_token_authorization( .expected_group(group_pos) { Ok(group) => Some((group_pos, group.clone())), - Err(e) => { - error_message = Some(format!("Invalid contract: {}", e)); + Err(error) => { + tracing::debug!(?error, "Main token control group lookup failed"); + error_message = Some( + "The token contract does not contain its main control group. Refresh the token and try again." + .to_string(), + ); None } } @@ -249,8 +253,12 @@ pub fn check_token_authorization( .expected_group(*group_pos) { Ok(group) => Some((*group_pos, group.clone())), - Err(e) => { - error_message = Some(format!("Invalid contract: {}", e)); + Err(error) => { + tracing::debug!(?error, "Token control group lookup failed"); + error_message = Some( + "The token contract does not contain the required control group. Refresh the token and try again." + .to_string(), + ); None } } @@ -442,7 +450,7 @@ fn render_no_eligible_key_group( ui.set_min_width(220.0); ui.vertical(|ui| { ui.label("No eligible key. This transaction type requires:"); - ui.label(format!("{} key", transaction_type.label())); + ui.label(format!("{transaction_type} key", transaction_type = transaction_type.label())); if has_eligible_public_keys_without_private { ui.label( @@ -906,11 +914,11 @@ pub fn render_group_action_text( ui.add_space(10.0); ui.label(format!( - "You are signing an active {} group action (Action ID {})", - group_action_type_str, - group_action_id.to_string(Encoding::Base58) + "You are signing an active {action_type} group action (Action ID {action_id})", + action_type = group_action_type_str, + action_id = group_action_id.to_string(Encoding::Base58) )); - format!("Sign {}", group_action_type_str) + format!("Sign {group_action_type_str}") } else if let Some((_, group)) = group.as_ref() { let your_power = group .members() @@ -928,11 +936,11 @@ pub fn render_group_action_text( ui.colored_label( Color32::DARK_RED, format!( - "You are not a valid group member for {} on this token", - group_action_type_str + "You are not a valid group member for {action_type} on this token", + action_type = group_action_type_str ), ); - return format!("Test {} (Should fail)", group_action_type_str); + return format!("Test {group_action_type_str} (Should fail)"); } ui.add_space(10.0); @@ -941,17 +949,17 @@ pub fn render_group_action_text( ui.label("You are a unilateral group member.\nYou do not need other group members to sign off on this action for it to process.".to_string()); group_action_type_str.to_string() } else { - ui.label(format!("You are not a unilateral group member.\nYou can initiate the {group_action_type_str} action but will need other group members to sign off on it for it to process.\nThis action requires a total power of {}.\nYour power is {your_power}.", group.required_power())); + ui.label(format!("You are not a unilateral group member.\nYou can initiate the {group_action_type_str} action but will need other group members to sign off on it for it to process.\nThis action requires a total power of {required_power}.\nYour power is {your_power}.", required_power = group.required_power())); ui.add_space(10.0); ui.label(format!( - "Other group members are : \n{}", - group + "Other group members are: \n{members}", + members = group .members() .iter() .filter_map(|(member, power)| { if member != &identity_token_info.identity.identity.id() { - Some(format!(" - {} with power {}", member, power)) + Some(format!(" - {member} with power {power}")) } else { None } @@ -959,10 +967,10 @@ pub fn render_group_action_text( .collect::>() .join(", \n") )); - format!("Initiate Group {}", group_action_type_str) + format!("Initiate Group {group_action_type_str}") } } else { - format!("Test {} (It should fail)", group_action_type_str) + format!("Test {group_action_type_str} (It should fail)") } } else { group_action_type_str.to_string() @@ -1055,11 +1063,11 @@ pub fn show_group_token_success_screen_with_fee( // Determine the success message based on the action type if is_group_action_signing { - ui.heading(format!("Group {} Signing Successful.", action_name)); + ui.heading(format!("Group {action_name} Signing Successful.")); } else if !is_unilateral_group_member && has_group { - ui.heading(format!("Group {} Initiated.", action_name)); + ui.heading(format!("Group {action_name} Initiated.")); } else { - ui.heading(format!("{} Successful.", action_name)); + ui.heading(format!("{action_name} Successful.")); } // Optional fee info section diff --git a/src/ui/identities/identities_screen.rs b/src/ui/identities/identities_screen.rs index 343b43b9b..ec4506cd3 100644 --- a/src/ui/identities/identities_screen.rs +++ b/src/ui/identities/identities_screen.rs @@ -355,14 +355,14 @@ impl IdentitiesScreen { }; ui.add(egui::Label::new(message).sense(egui::Sense::hover())) - .info_tooltip(format!("{}", qualified_identity.identity.balance())); + .info_tooltip(qualified_identity.identity.balance().to_string()); } fn show_balance(ui: &mut Ui, qualified_identity: &QualifiedIdentity) { let balance_in_dash = qualified_identity.identity.balance() as f64 * 1e-11; - let formatted_balance = format!("{:.4} DASH", balance_in_dash); + let formatted_balance = format!("{balance_in_dash:.4} DASH"); ui.add(egui::Label::new(formatted_balance).sense(egui::Sense::hover())) - .info_tooltip(format!("{}", qualified_identity.identity.balance())); + .info_tooltip(qualified_identity.identity.balance().to_string()); } fn format_key_name(&self, key: &IdentityPublicKey) -> String { @@ -381,7 +381,10 @@ impl IdentitiesScreen { SecurityLevel::HIGH => "High", SecurityLevel::MEDIUM => "Medium", }; - format!("{} - {} - {}", key.id(), purpose_letter, security_level) + format!( + "{key_id} - {purpose_letter} - {security_level}", + key_id = key.id() + ) } fn render_no_identities_view(&self, ui: &mut Ui) { @@ -583,7 +586,7 @@ impl IdentitiesScreen { ui.vertical_centered(|ui| { ui.horizontal_centered(|ui| { // Show identity type and status - let type_text = format!("{}", qualified_identity.identity_type); + let type_text = qualified_identity.identity_type.to_string(); let status = qualified_identity.status; // Always show status information (don't disable this column) ui.add_enabled_ui(true, |ui|{ @@ -617,7 +620,10 @@ impl IdentitiesScreen { .clickable_tooltip("Manage identity credits") .disabled_tooltip("Identity actions are unavailable until this identity becomes active"); - let actions_popup_id = ui.make_persistent_id(format!("actions_popup_{}", qualified_identity.identity.id().to_string(Encoding::Base58))); + let actions_popup_id = ui.make_persistent_id(format!( + "actions_popup_{identity_id}", + identity_id = qualified_identity.identity.id().to_string(Encoding::Base58) + )); egui::Popup::from_toggle_button_response(&actions_response).id(actions_popup_id) .close_behavior(egui::PopupCloseBehavior::CloseOnClickOutside) .frame(egui::Frame::popup(ui.style()).fill(DashColors::popup_fill(ui.style().visuals.dark_mode))) @@ -728,7 +734,10 @@ impl IdentitiesScreen { let button_response = ui.add(button).clickable_tooltip("View and manage keys for this identity"); - let popup_id = ui.make_persistent_id(format!("keys_popup_{}", qualified_identity.identity.id().to_string(Encoding::Base58))); + let popup_id = ui.make_persistent_id(format!( + "keys_popup_{identity_id}", + identity_id = qualified_identity.identity.id().to_string(Encoding::Base58) + )); egui::Popup::from_toggle_button_response(&button_response).id(popup_id) .close_behavior(egui::PopupCloseBehavior::CloseOnClickOutside) .frame(egui::Frame::popup(ui.style()).fill(DashColors::popup_fill(ui.style().visuals.dark_mode))) @@ -812,9 +821,9 @@ impl IdentitiesScreen { // Remove if ui.button("Remove").clickable_tooltip("Remove this identity from Dash Evo Tool (it'll still exist on Dash Platform)").clicked() { let message = format!( - "Are you sure you want to no longer track this {} identity?\n\nIdentity ID: {}", - qualified_identity.identity_type, - qualified_identity.identity.id().to_string( + "Are you sure you want to no longer track this {identity_type} identity?\n\nIdentity ID: {identity_id}", + identity_type = qualified_identity.identity_type, + identity_id = qualified_identity.identity.id().to_string( qualified_identity.identity_type.default_encoding() ) ); @@ -884,43 +893,9 @@ impl IdentitiesScreen { } if let Some(identity_to_remove) = self.identity_to_remove.take() { let identity_id = identity_to_remove.identity.id(); - - match self - .app_context - .delete_local_qualified_identity(&identity_id) - { - Ok(_) => { - let mut lock = self.identities.lock_recover(); - lock.shift_remove(&identity_id); - } - Err(e) => { - tracing::warn!( - "Failed to delete identity from database: {}", - e - ); - MessageBanner::set_global( - self.app_context.egui_ctx(), - format!("Failed to remove identity: {}", e), - MessageType::Error, - ) - .disable_auto_dismiss(); - } - } - - if let Some((voter_identity, _)) = - &identity_to_remove.associated_voter_identity - { - let voter_identity_id = voter_identity.id(); - if let Err(e) = self - .app_context - .delete_local_qualified_identity(&voter_identity_id) - { - tracing::warn!( - "Failed to delete voter identity from database: {}", - e - ); - } - } + return AppAction::BackendTask(BackendTask::IdentityTask( + IdentityTask::RemoveIdentity { identity_id }, + )); } } ConfirmationStatus::Canceled => { @@ -1018,7 +993,7 @@ impl IdentitiesScreen { Err(e) => { MessageBanner::set_global( ctx, - "Failed to save alias", + "The alias could not be saved. Check available disk space and try again.", MessageType::Error, ) .with_details(e); @@ -1073,26 +1048,51 @@ impl ScreenLike for IdentitiesScreen { &mut self, backend_task_success_result: crate::ui::BackendTaskSuccessResult, ) { - if let crate::ui::BackendTaskSuccessResult::RefreshedIdentity(_) = - backend_task_success_result - { - self.pending_refresh_count = self.pending_refresh_count.saturating_sub(1); - if self.pending_refresh_count == 0 { - self.refresh_banner.take_and_clear(); - let message = if self.total_refresh_count == 1 { - "Successfully refreshed identity".to_string() - } else { - format!( - "Successfully refreshed {} identities", - self.total_refresh_count + match backend_task_success_result { + crate::ui::BackendTaskSuccessResult::RefreshedIdentity(_) => { + self.pending_refresh_count = self.pending_refresh_count.saturating_sub(1); + if self.pending_refresh_count == 0 { + self.refresh_banner.take_and_clear(); + let message = if self.total_refresh_count == 1 { + "Successfully refreshed identity".to_string() + } else { + format!( + "Successfully refreshed {count} identities", + count = self.total_refresh_count + ) + }; + MessageBanner::set_global( + self.app_context.egui_ctx(), + &message, + MessageType::Success, + ); + } + } + crate::ui::BackendTaskSuccessResult::RemovedIdentities { + identity_ids, + associated_cleanup_failed, + } => { + let mut identities = self.identities.lock_recover(); + for identity_id in identity_ids { + identities.shift_remove(&identity_id); + } + drop(identities); + if associated_cleanup_failed { + MessageBanner::set_global( + self.app_context.egui_ctx(), + "The identity was removed, but its associated voter identity could not be removed. Retry after restarting the app.", + MessageType::Warning, ) - }; - MessageBanner::set_global( - self.app_context.egui_ctx(), - &message, - MessageType::Success, - ); + .disable_auto_dismiss(); + } else { + MessageBanner::set_global( + self.app_context.egui_ctx(), + "The identity was removed from this device.", + MessageType::Success, + ); + } } + _ => {} } } diff --git a/src/ui/identities/keys/add_key_screen.rs b/src/ui/identities/keys/add_key_screen.rs index 3ba219bec..76e1e809f 100644 --- a/src/ui/identities/keys/add_key_screen.rs +++ b/src/ui/identities/keys/add_key_screen.rs @@ -193,13 +193,14 @@ impl AddKeyScreen { &private_key_bytes, self.app_context.network, ); - if let Err(err) = public_key_data_result { + if let Err(error) = public_key_data_result { self.add_key_status = AddKeyStatus::Error; MessageBanner::set_global( self.app_context.egui_ctx(), - format!("Issue verifying private key: {}", err), + "The private key could not be verified. Check the key and try again.", MessageType::Error, - ); + ) + .with_details(error); } else { // Handle contract bounds if enabled let contract_bounds = if self.enable_contract_bounds @@ -216,13 +217,14 @@ impl AddKeyScreen { }) } } - Err(e) => { + Err(error) => { self.add_key_status = AddKeyStatus::Error; MessageBanner::set_global( self.app_context.egui_ctx(), - format!("Invalid contract ID: {}", e), + "The contract ID is not valid. Check the ID and try again.", MessageType::Error, - ); + ) + .with_details(error); return app_action; } } @@ -246,13 +248,14 @@ impl AddKeyScreen { // Validate the private key against the public key let validation_result = new_key .validate_private_key_bytes(&private_key_bytes, self.app_context.network); - if let Err(err) = validation_result { + if let Err(error) = validation_result { self.add_key_status = AddKeyStatus::Error; MessageBanner::set_global( self.app_context.egui_ctx(), - format!("Issue verifying private key: {}", err), + "The private key could not be verified. Check the key and try again.", MessageType::Error, - ); + ) + .with_details(error); } else if validation_result .expect("invariant: Err handled in the preceding branch") { @@ -455,7 +458,7 @@ impl ScreenLike for AddKeyScreen { ui.label("Purpose:"); let prev_purpose = self.purpose; egui::ComboBox::from_id_salt("purpose_selector") - .selected_text(format!("{:?}", self.purpose)) + .selected_text(format!("{purpose:?}", purpose = self.purpose)) .show_ui(ui, |ui| { if self.enable_contract_bounds { // When contract bounds are enabled, only allow ENCRYPTION and DECRYPTION @@ -524,7 +527,10 @@ impl ScreenLike for AddKeyScreen { let has_multiple_security_levels = self.purpose == Purpose::AUTHENTICATION; let inner_response = ui.add_enabled_ui(has_multiple_security_levels, |ui| { egui::ComboBox::from_id_salt("security_level_selector") - .selected_text(format!("{:?}", self.security_level)) + .selected_text(format!( + "{security_level:?}", + security_level = self.security_level + )) .show_ui(ui, |ui| { if self.enable_contract_bounds { // When contract bounds are enabled, only allow MEDIUM @@ -576,8 +582,9 @@ impl ScreenLike for AddKeyScreen { egui::Sense::hover(), ); hover_response.info_tooltip(format!( - "{:?} purpose requires {:?} security level", - self.purpose, self.security_level + "{purpose:?} purpose requires {security_level:?} security level", + purpose = self.purpose, + security_level = self.security_level )); } ui.end_row(); @@ -585,7 +592,7 @@ impl ScreenLike for AddKeyScreen { // Key Type ui.label("Key Type:"); egui::ComboBox::from_id_salt("key_type_selector") - .selected_text(format!("{:?}", self.key_type)) + .selected_text(format!("{key_type:?}", key_type = self.key_type)) .show_ui(ui, |ui| { ui.selectable_value( &mut self.key_type, diff --git a/src/ui/identities/keys/key_info_screen.rs b/src/ui/identities/keys/key_info_screen.rs index 1d308c48a..3df001fda 100644 --- a/src/ui/identities/keys/key_info_screen.rs +++ b/src/ui/identities/keys/key_info_screen.rs @@ -848,12 +848,13 @@ impl KeyInfoScreen { let validation_result = self .key .validate_private_key_bytes(&private_key_bytes, self.app_context.network); - if let Err(err) = validation_result { + if let Err(error) = validation_result { MessageBanner::set_global( self.app_context.egui_ctx(), - format!("Issue verifying private key {}", err), + "The private key could not be verified. Check the key and try again.", MessageType::Error, - ); + ) + .with_details(error); } else if validation_result.expect("invariant: Err handled in the preceding branch") { // If valid, store the private key in the context and reset the input field self.private_key_data = Some((PrivateKeyData::Clear(private_key_bytes), None)); @@ -861,16 +862,17 @@ impl KeyInfoScreen { (self.key.purpose().into(), self.key.id()), (self.key.clone().into(), private_key_bytes), ); - if let Err(e) = self + if let Err(error) = self .app_context .update_local_qualified_identity(&self.identity) { - MessageBanner::set_global( + let handle = MessageBanner::set_global( self.app_context.egui_ctx(), - format!("Issue saving: {}", e), + "The private key could not be saved. Check available disk space and try again.", MessageType::Error, - ) - .disable_auto_dismiss(); + ); + handle.with_details(error); + handle.disable_auto_dismiss(); } } else { MessageBanner::set_global( @@ -1062,16 +1064,17 @@ impl KeyInfoScreen { .private_keys .private_keys .remove(&(self.key.purpose().into(), self.key.id())); - if let Err(e) = self + if let Err(error) = self .app_context .update_local_qualified_identity(&self.identity) { - MessageBanner::set_global( + let handle = MessageBanner::set_global( ui.ctx(), - format!("Issue saving: {}", e), + "The private-key change could not be saved. Check available disk space and try again.", MessageType::Error, - ) - .disable_auto_dismiss(); + ); + handle.with_details(error); + handle.disable_auto_dismiss(); } } } diff --git a/src/ui/identities/register_dpns_name_screen.rs b/src/ui/identities/register_dpns_name_screen.rs index ca46f8181..bd4d86902 100644 --- a/src/ui/identities/register_dpns_name_screen.rs +++ b/src/ui/identities/register_dpns_name_screen.rs @@ -2,6 +2,7 @@ use crate::app::AppAction; use crate::backend_task::identity::{IdentityTask, RegisterDpnsNameInput}; use crate::backend_task::{BackendTask, BackendTaskSuccessResult, FeeResult}; use crate::context::AppContext; +use crate::model::dpns::{DpnsNameValidationResult, validate_dpns_name}; use crate::model::fee_estimation::format_credits_as_dash; use crate::model::qualified_identity::QualifiedIdentity; use crate::model::wallet::Wallet; @@ -453,7 +454,10 @@ impl ScreenLike for RegisterDpnsNameScreen { inner_action |= self.render_identity_id_selection(ui); ui.add_space(5.0); if let Some(identity) = &self.selected_qualified_identity { - ui.label(format!("Identity balance: {:.6}", identity.identity.balance() as f64 * 1e-11)); + ui.label(format!( + "Identity balance: {balance:.6}", + balance = identity.identity.balance() as f64 * 1e-11 + )); } ui.add_space(10.0); @@ -581,8 +585,8 @@ impl ScreenLike for RegisterDpnsNameScreen { let hover_text = if !has_enough_balance { format!( - "Insufficient identity balance for fee (need at least {})", - format_credits_as_dash(estimated_fee) + "Insufficient identity balance for fee (need at least {fee})", + fee = format_credits_as_dash(estimated_fee) ) } else if !name_is_valid { "Please enter a valid name".to_string() @@ -665,63 +669,3 @@ pub fn is_contested_name(name: &str) -> bool { } true } - -#[derive(Debug, PartialEq)] -pub enum DpnsNameValidationResult { - Valid, - TooShort, - TooLong, - InvalidCharacter(char), - StartsWithHyphen, - EndsWithHyphen, -} - -pub fn validate_dpns_name(name: &str) -> DpnsNameValidationResult { - if name.len() < 3 { - return DpnsNameValidationResult::TooShort; - } - - if name.len() > 63 { - return DpnsNameValidationResult::TooLong; - } - - if name.starts_with('-') { - return DpnsNameValidationResult::StartsWithHyphen; - } - - if name.ends_with('-') { - return DpnsNameValidationResult::EndsWithHyphen; - } - - for c in name.chars() { - if !c.is_ascii_alphanumeric() && c != '-' { - return DpnsNameValidationResult::InvalidCharacter(c); - } - } - - DpnsNameValidationResult::Valid -} - -impl DpnsNameValidationResult { - pub fn error_message(&self) -> Option { - match self { - DpnsNameValidationResult::Valid => None, - DpnsNameValidationResult::TooShort => { - Some("Name must be at least 3 characters long".to_string()) - } - DpnsNameValidationResult::TooLong => { - Some("Name must be no more than 63 characters long".to_string()) - } - DpnsNameValidationResult::InvalidCharacter(c) => Some(format!( - "Invalid character '{}'. Only letters, numbers, and hyphens are allowed", - c - )), - DpnsNameValidationResult::StartsWithHyphen => { - Some("Name cannot start with a hyphen".to_string()) - } - DpnsNameValidationResult::EndsWithHyphen => { - Some("Name cannot end with a hyphen".to_string()) - } - } - } -} diff --git a/src/ui/identities/transfer_screen.rs b/src/ui/identities/transfer_screen.rs index f8a8695d7..582c5550c 100644 --- a/src/ui/identities/transfer_screen.rs +++ b/src/ui/identities/transfer_screen.rs @@ -3,7 +3,7 @@ use crate::backend_task::identity::IdentityTask; use crate::backend_task::{BackendTask, BackendTaskSuccessResult, FeeResult}; use crate::context::AppContext; use crate::model::amount::Amount; -use crate::model::fee_estimation::format_credits_as_dash; +use crate::model::fee_estimation::{format_credits_as_dash, max_spendable_credits}; use crate::model::qualified_identity::QualifiedIdentity; use crate::model::user_role::UserRole; use crate::model::wallet::Wallet; @@ -144,12 +144,17 @@ impl TransferScreen { fn render_amount_input(&mut self, ui: &mut Ui) { // Show available balance let balance_in_dash = self.max_amount as f64 / 100_000_000_000.0; - ui.label(format!("Available balance: {:.8} DASH", balance_in_dash)); + ui.label(format!("Available balance: {balance_in_dash:.8} DASH")); ui.add_space(5.0); - // Calculate max amount minus fee for the "Max" button - let max_amount_minus_fee = (self.max_amount as f64 / 100_000_000_000.0 - 0.0002).max(0.0); - let max_amount_credits = (max_amount_minus_fee * 100_000_000_000.0) as u64; + let fee_estimator = self.app_context.fee_estimator(); + let estimated_fee = match self.destination_type { + TransferDestinationType::Identity => fee_estimator.estimate_credit_transfer(), + TransferDestinationType::PlatformAddress => { + fee_estimator.estimate_credit_transfer_to_addresses(1) + } + }; + let max_amount_credits = max_spendable_credits(self.max_amount, estimated_fee); let amount_input = self.amount_input.get_or_insert_with(|| { AmountInput::new(Amount::new_dash(0.0)) @@ -282,21 +287,22 @@ impl TransferScreen { // Try to parse as Bech32m Platform address first (dash1.../tdash1... per DIP-18) if crate::ui::helpers::is_platform_address_string(input) { let addr = PlatformAddress::from_bech32m_string(input) - .map_err(|e| format!("Invalid Bech32m address: {}", e))?; + .map_err(|error| format!("Invalid Bech32m address: {error}"))?; return Ok(addr); } // Fall back to base58 parsing for backwards compatibility let unchecked_addr: Address = input .parse() - .map_err(|e| format!("Invalid address format: {}", e))?; + .map_err(|error| format!("Invalid address format: {error}"))?; // Platform addresses use the same version byte (0x5a / prefix 'd') for // testnet, devnet, and regtest per DIP-18. We use assume_checked() here // because require_network() would fail on regtest (address parses as testnet). let address = unchecked_addr.assume_checked(); - PlatformAddress::try_from(address).map_err(|e| format!("Invalid Platform address: {}", e)) + PlatformAddress::try_from(address) + .map_err(|error| format!("Invalid Platform address: {error}")) } /// Handle the confirmation action for Platform address transfer @@ -307,8 +313,14 @@ impl TransferScreen { // Validate Platform address let platform_address = match self.validate_platform_address() { Ok(addr) => addr, - Err(error) => { - self.set_error_state(error); + Err(details) => { + self.transfer_credits_status = TransferCreditsStatus::Error; + MessageBanner::set_global( + self.app_context.egui_ctx(), + "The Platform address is not valid. Check the address and try again.", + MessageType::Error, + ) + .with_details(details); return AppAction::None; } }; @@ -343,8 +355,8 @@ impl TransferScreen { (self.identity.identity.balance() as u128).saturating_sub(estimated_fee as u128); if credits > max_transferable { self.set_error_state(format!( - "Amount plus estimated fee exceeds available balance (max transferable: {})", - format_credits_as_dash(max_transferable as u64) + "Amount plus estimated fee exceeds available balance (max transferable: {max_amount})", + max_amount = format_credits_as_dash(max_transferable as u64) )); return AppAction::None; } @@ -414,8 +426,8 @@ impl TransferScreen { (self.identity.identity.balance() as u128).saturating_sub(estimated_fee as u128); if credits > max_transferable { self.set_error_state(format!( - "Amount plus estimated fee exceeds available balance (max transferable: {})", - format_credits_as_dash(max_transferable as u64) + "Amount plus estimated fee exceeds available balance (max transferable: {max_amount})", + max_amount = format_credits_as_dash(max_transferable as u64) )); self.confirmation_popup = false; return AppAction::None; @@ -474,10 +486,7 @@ impl TransferScreen { let receiver_id = self.receiver_identity_id.clone(); - let msg = format!( - "Are you sure you want to transfer {} to {}?", - amount, receiver_id - ); + let msg = format!("Are you sure you want to transfer {amount} to {receiver_id}?"); // Lazy initialization of the confirmation dialog let confirmation_dialog = self.confirmation_dialog.get_or_insert_with(|| { @@ -506,8 +515,7 @@ impl TransferScreen { let platform_address = self.platform_address_input.clone(); let msg = format!( - "Are you sure you want to transfer {} to Platform address {}?", - amount, platform_address + "Are you sure you want to transfer {amount} to Platform address {platform_address}?" ); // Lazy initialization of the confirmation dialog @@ -564,13 +572,14 @@ impl ScreenLike for TransferScreen { let identities = self .app_context .load_local_qualified_identities() - .unwrap_or_else(|e| { - MessageBanner::set_global( + .unwrap_or_else(|error| { + let handle = MessageBanner::set_global( self.app_context.egui_ctx(), - format!("Failed to load local identities: {e}"), + "Saved identities could not be loaded. Refresh the screen and try again.", MessageType::Error, - ) - .disable_auto_dismiss(); + ); + handle.with_details(error); + handle.disable_auto_dismiss(); vec![] }); if let Some(refreshed) = identities @@ -632,8 +641,8 @@ impl ScreenLike for TransferScreen { ui.colored_label( egui::Color32::DARK_RED, format!( - "You do not have any transfer keys loaded for this {} identity.", - self.identity.identity_type + "You do not have any transfer keys loaded for this {identity_type} identity.", + identity_type = self.identity.identity_type ), ); ui.add_space(10.0); @@ -698,9 +707,9 @@ impl ScreenLike for TransferScreen { // Show identity info let identity_id_string = self.identity.identity.id().to_string(Encoding::Base58); let identity_label = if let Some(alias) = &self.identity.alias { - format!("From: {} ({})", alias, identity_id_string) + format!("From: {alias} ({identity_id_string})") } else { - format!("From: {}", identity_id_string) + format!("From: {identity_id_string}") }; ui.label(identity_label); ui.add_space(5.0); @@ -806,8 +815,8 @@ impl ScreenLike for TransferScreen { let hover_text = if !has_enough_balance { format!( - "Insufficient balance for transfer fee (need at least {})", - format_credits_as_dash(estimated_fee) + "Insufficient balance for transfer fee (need at least {fee})", + fee = format_credits_as_dash(estimated_fee) ) } else if ready { "Transfer credits to another identity or Platform address".to_string() diff --git a/src/ui/identities/withdraw_screen.rs b/src/ui/identities/withdraw_screen.rs index d7629e236..b1ddb6889 100644 --- a/src/ui/identities/withdraw_screen.rs +++ b/src/ui/identities/withdraw_screen.rs @@ -3,7 +3,7 @@ use crate::backend_task::identity::IdentityTask; use crate::backend_task::{BackendTask, BackendTaskSuccessResult, FeeResult}; use crate::context::AppContext; use crate::model::amount::Amount; -use crate::model::fee_estimation::format_credits_as_dash; +use crate::model::fee_estimation::{format_credits_as_dash, max_spendable_credits}; use crate::model::qualified_identity::encrypted_key_storage::PrivateKeyData; use crate::model::qualified_identity::{IdentityType, PrivateKeyTarget, QualifiedIdentity}; use crate::model::user_role::UserRole; @@ -133,8 +133,11 @@ impl WithdrawalScreen { } fn render_amount_input(&mut self, ui: &mut Ui) { - let max_amount_minus_fee = (self.max_amount as f64 / 100_000_000_000.0 - 0.005).max(0.0); - let max_amount_credits = (max_amount_minus_fee * 100_000_000_000.0) as u64; + let estimated_fee = self + .app_context + .fee_estimator() + .estimate_credit_withdrawal(); + let max_amount_credits = max_spendable_credits(self.max_amount, estimated_fee); // Lazy initialization with basic configuration let amount_input = self.withdrawal_amount_input.get_or_insert_with(|| { @@ -263,7 +266,7 @@ impl WithdrawalScreen { .identity .masternode_payout_address(self.app_context.network) { - format!("masternode payout address {}", payout_address) + format!("masternode payout address {payout_address}") } else if !self.app_context.user_role().at_least(UserRole::Power) { self.withdraw_from_identity_status = WithdrawFromIdentityStatus::Error; MessageBanner::set_global( @@ -292,11 +295,12 @@ impl WithdrawalScreen { ConfirmationDialog::new( "Confirm Withdrawal".to_string(), format!( - "Are you sure you want to withdraw {} to {}", - self.withdrawal_amount + "Are you sure you want to withdraw {amount} to {address}", + amount = self + .withdrawal_amount .as_ref() .expect("Withdrawal amount should be present"), - message_address + address = message_address ), ) .danger_mode(true) // Withdrawal is a destructive operation @@ -375,13 +379,14 @@ impl ScreenLike for WithdrawalScreen { if let Some(refreshed) = self .app_context .load_local_qualified_identities() - .unwrap_or_else(|e| { - MessageBanner::set_global( + .unwrap_or_else(|error| { + let handle = MessageBanner::set_global( self.app_context.egui_ctx(), - format!("Failed to load local identities: {e}"), + "Saved identities could not be loaded. Refresh the screen and try again.", MessageType::Error, - ) - .disable_auto_dismiss(); + ); + handle.with_details(error); + handle.disable_auto_dismiss(); vec![] }) .into_iter() @@ -439,7 +444,10 @@ impl ScreenLike for WithdrawalScreen { if !has_keys { ui.colored_label( egui::Color32::DARK_RED, - format!("You do not have any withdrawal keys loaded for this {} identity. Note that TRANSFER or OWNER keys are used for withdrawals.", self.identity.identity_type)); + format!( + "You do not have any withdrawal keys loaded for this {identity_type} identity. Note that TRANSFER or OWNER keys are used for withdrawals.", + identity_type = self.identity.identity_type + )); ui.add_space(10.0); if self.identity.identity_type != IdentityType::User { @@ -481,7 +489,7 @@ impl ScreenLike for WithdrawalScreen { IdentityType::Evonode => "Payout", }; if ui - .button(format!("Check {} Address Key", key_type_name)) + .button(format!("Check {key_type_name} Address Key")) .clicked() { inner_action |= @@ -563,9 +571,9 @@ impl ScreenLike for WithdrawalScreen { // Show identity info let identity_id_string = self.identity.identity.id().to_string(Encoding::Base58); let identity_label = if let Some(alias) = &self.identity.alias { - format!("From: {} ({})", alias, identity_id_string) + format!("From: {alias} ({identity_id_string})") } else { - format!("From: {}", identity_id_string) + format!("From: {identity_id_string}") }; ui.label(identity_label); @@ -573,7 +581,7 @@ impl ScreenLike for WithdrawalScreen { let balance_dash = self.max_amount as f64 / 100_000_000_000.0; ui.horizontal(|ui| { ui.label("Available Balance:"); - ui.label(RichText::new(format!("{:.4} Dash", balance_dash))); + ui.label(RichText::new(format!("{balance_dash:.4} Dash"))); }); ui.add_space(5.0); @@ -645,8 +653,8 @@ impl ScreenLike for WithdrawalScreen { "Please enter a valid withdrawal address".to_string() } else if !has_enough_balance { format!( - "Insufficient balance for withdrawal fee (need at least {})", - format_credits_as_dash(estimated_fee) + "Insufficient balance for withdrawal fee (need at least {fee})", + fee = format_credits_as_dash(estimated_fee) ) } else { String::new() diff --git a/src/ui/masternodes/detail_screen.rs b/src/ui/masternodes/detail_screen.rs index 311b34535..0dd5d09e2 100644 --- a/src/ui/masternodes/detail_screen.rs +++ b/src/ui/masternodes/detail_screen.rs @@ -169,10 +169,11 @@ fn manage_keys_labels( .iter() .map(|(target, key)| { let (role, tip) = key_role_label(target, key); - let mut label = format!("{role} key"); - if key.is_disabled() { - label.push_str(" (disabled)"); - } + let label = if key.is_disabled() { + format!("{role} key (disabled)") + } else { + format!("{role} key") + }; (label, tip) }) .collect(); @@ -186,7 +187,7 @@ fn manage_keys_labels( .zip(keys.iter()) .map(|((label, tip), (_, key))| { if counts.get(label.as_str()).copied().unwrap_or(0) > 1 { - (format!("{label} #{}", key.id()), *tip) + (format!("{label} #{key_id}", key_id = key.id()), *tip) } else { (label.clone(), *tip) } @@ -613,8 +614,11 @@ impl MasternodeDetailView { let voter_full = voter.id().to_string(Encoding::Base58); ui.horizontal(|ui| { ui.label( - RichText::new(format!("Voter identity: {}", shorten_id(&voter_full))) - .color(DashColors::text_secondary(dark_mode)), + RichText::new(format!( + "Voter identity: {voter}", + voter = shorten_id(&voter_full) + )) + .color(DashColors::text_secondary(dark_mode)), ); // `small_button` keeps the copy affordance text-height and // vertically centered with the voter-identity label. @@ -1167,7 +1171,7 @@ mod tests { // An unmapped purpose keeps its name and carries no tooltip. assert_eq!( role_label_and_tip(false, Purpose::ENCRYPTION), - (format!("{:?}", Purpose::ENCRYPTION), None) + (format!("{purpose:?}", purpose = Purpose::ENCRYPTION), None,) ); } diff --git a/src/ui/tokens/tokens_screen/mod.rs b/src/ui/tokens/tokens_screen/mod.rs index bdfa97e74..161925fcc 100644 --- a/src/ui/tokens/tokens_screen/mod.rs +++ b/src/ui/tokens/tokens_screen/mod.rs @@ -2577,12 +2577,13 @@ impl TokensScreen { if let Some(status) = response.dialog_response { match status { ConfirmationStatus::Confirmed => { - if let Err(e) = self.app_context.remove_token(&token_to_remove) { + if let Err(error) = self.app_context.remove_token(&token_to_remove) { MessageBanner::set_global( self.app_context.egui_ctx(), - format!("Error removing token balance: {}", e), + "The token balance could not be removed. Refresh and try again.", MessageType::Error, - ); + ) + .with_details(error); } else { self.refresh(); } diff --git a/src/ui/tokens/tokens_screen/my_tokens.rs b/src/ui/tokens/tokens_screen/my_tokens.rs index ae39c62cb..337ccd1ba 100644 --- a/src/ui/tokens/tokens_screen/my_tokens.rs +++ b/src/ui/tokens/tokens_screen/my_tokens.rs @@ -166,8 +166,13 @@ impl TokensScreen { // Otherwise, show the list of all tokens match self.render_token_list(ui) { Ok(list_action) => action |= list_action, - Err(e) => { - MessageBanner::set_global(ui.ctx(), &e, MessageType::Error); + Err(error) => { + MessageBanner::set_global( + ui.ctx(), + "The token list could not be displayed. Refresh and try again.", + MessageType::Error, + ) + .with_details(error); } } } @@ -734,12 +739,13 @@ impl TokensScreen { MessageType::Error, ); } - Err(e) => { + Err(error) => { MessageBanner::set_global( ui.ctx(), - format!("Error fetching token contract: {e}"), + "The token contract could not be loaded. Refresh and try again.", MessageType::Error, - ); + ) + .with_details(error); } } } diff --git a/src/ui/tokens/tokens_screen/token_creator.rs b/src/ui/tokens/tokens_screen/token_creator.rs index d74606478..314ce6acb 100644 --- a/src/ui/tokens/tokens_screen/token_creator.rs +++ b/src/ui/tokens/tokens_screen/token_creator.rs @@ -17,6 +17,7 @@ use crate::ui::ScreenType; use crate::app::{AppAction, BackendTasksExecutionMode}; use crate::backend_task::BackendTask; use crate::backend_task::tokens::TokenTask; +use crate::model::token::validate_contract_keyword; use crate::ui::components::styled::{StyledCheckbox}; use crate::ui::components::confirmation_dialog::{ConfirmationDialog, ConfirmationStatus}; use crate::ui::components::Component; @@ -74,9 +75,12 @@ impl TokensScreen { ui.add_space(10.0); let all_identities = match self.app_context.load_local_user_identities() { Ok(identities) => identities.into_iter().filter(|qi| !qi.private_keys.private_keys.is_empty()).collect::>(), - Err(e) => { - tracing::error!(err=?e, "Error loading identities from local DB."); - ui.colored_label(Color32::DARK_RED,format!("Error loading identities from local DB: {}", e)); + Err(error) => { + tracing::error!(?error, "Token-creator identity loading failed"); + ui.colored_label( + Color32::DARK_RED, + "Saved identities could not be loaded. Refresh the screen and try again.", + ); return; } }; @@ -401,7 +405,7 @@ impl TokensScreen { if ui.selectable_value( &mut self.selected_token_preset, Some(variant), - format!("{} - {}", text, description), + format!("{text} - {description}"), ).clicked() { let preset = TokenConfigurationPreset { features: variant, @@ -438,7 +442,7 @@ impl TokensScreen { // Auto-set plural name if empty (singular + "s") let singular = self.token_names_input[0].0.trim().to_string(); if self.token_names_input[0].1.trim().is_empty() { - self.token_names_input[0].1 = format!("{}s", singular); + self.token_names_input[0].1 = format!("{singular}s"); } // Trigger the token creation confirmation @@ -464,7 +468,7 @@ impl TokensScreen { "token preset" }; ui.label( - RichText::new(format!("Please select a {}", missing)) + RichText::new(format!("Please select a {missing}")) .color(egui::Color32::GRAY) .italics(), ); @@ -491,8 +495,8 @@ impl TokensScreen { ui.text_edit_singleline(&mut self.token_names_input[i].0); ui.horizontal(|ui| { let allow_all_languages = i != 0; - ui.push_id(format!("combo_{}", i), |ui| { - let combo_id = format!("token_name_language_selector_{}", i); + ui.push_id(format!("combo_{i}"), |ui| { + let combo_id = format!("token_name_language_selector_{i}"); Self::render_token_name_language_selector( ui, &mut self.token_names_input[i].2, @@ -565,7 +569,7 @@ impl TokensScreen { seen_keywords.insert(name.0.clone()); for keyword in contract_keywords.iter() { if seen_keywords.contains(*keyword) { - ui.colored_label(Color32::DARK_RED, format!("Duplicate contract keyword: {}", keyword)); + ui.colored_label(Color32::DARK_RED, format!("Duplicate contract keyword: {keyword}")); } seen_keywords.insert(keyword.to_string()); } @@ -649,9 +653,9 @@ impl TokensScreen { .unwrap_or(""); let message = if self.decimals_input == "0" { - format!("Non Fractional Token (i.e. 0, 1, 2 or 10 {})", token_name) + format!("Non Fractional Token (i.e. 0, 1, 2 or 10 {token_name})") } else { - format!("Fractional Token (i.e. 0.2 {})", token_name) + format!("Fractional Token (i.e. 0.2 {token_name})") }; ui.label(RichText::new(message).color(Color32::GRAY)); @@ -810,10 +814,9 @@ impl TokensScreen { ui.horizontal(|ui| { ui.label("Allow main control group change:"); ComboBox::from_id_salt("main_control_group_change_selector") - .selected_text(format!( - "{}", - self.authorized_main_control_group_change - )) + .selected_text( + self.authorized_main_control_group_change.to_string(), + ) .show_ui(ui, |ui| { ui.selectable_value( &mut self.authorized_main_control_group_change, @@ -915,8 +918,13 @@ impl TokensScreen { args.into_contract_params(), ) { Ok(dc) => dc, - Err(e) => { - MessageBanner::set_global(context, format!("Error building contract V1: {e}"), MessageType::Error); + Err(error) => { + MessageBanner::set_global( + context, + "The token contract could not be prepared. Review its settings and try again.", + MessageType::Error, + ) + .with_details(error); return; } }; @@ -1032,7 +1040,7 @@ impl TokensScreen { .size = 12.0; ComboBox::from_id_salt(id_salt) - .selected_text(format!("{}", current_language)) + .selected_text(current_language.to_string()) .width(100.0) .show_ui(ui, |ui| { ui.style_mut() @@ -1201,10 +1209,12 @@ impl TokensScreen { .split(',') .map(|s| { let trimmed = s.trim().to_string(); - if trimmed.len() < 3 || trimmed.len() > 50 { + if let Err(error) = validate_contract_keyword(&trimmed) { Err(format!( - "Invalid contract keyword {}, keyword must be between 3 and 50 characters", - trimmed + "Contract keyword '{keyword}' must contain between {min} and {max} characters.", + keyword = trimmed, + min = error.min, + max = error.max )) } else { Ok(trimmed) @@ -1225,8 +1235,8 @@ impl TokensScreen { for name_with_language in &self.token_names_input { if seen_languages.contains(&name_with_language.2) { return Err(format!( - "Duplicate token name language: {:?}", - name_with_language.1 + "Duplicate token name language: {language:?}", + language = name_with_language.1 )); } seen_languages.insert(name_with_language.2); @@ -1236,15 +1246,15 @@ impl TokensScreen { for name_with_language in &self.token_names_input { if name_with_language.0.len() < 3 || name_with_language.0.len() > 50 { return Err(format!( - "The name in {:?} must be between 3 and 50 characters", - name_with_language.2 + "The name in {language:?} must be between 3 and 50 characters", + language = name_with_language.2 )); } if name_with_language.1.len() < 3 || name_with_language.1.len() > 50 { return Err(format!( - "The plural form in {:?} must be between 3 and 50 characters", - name_with_language.2 + "The plural form in {language:?} must be between 3 and 50 characters", + language = name_with_language.2 )); } @@ -1393,10 +1403,6 @@ impl TokensScreen { fn render_token_creator_confirmation_popup(&mut self, ui: &mut Ui) -> AppAction { let mut action = AppAction::None; - // Prepare the confirmation message - let mut confirmation_message = - "Are you sure you want to register a new token contract with these settings?\n\n" - .to_string(); let base_supply_display = self .base_supply_amount .as_ref() @@ -1409,33 +1415,32 @@ impl TokensScreen { .map(|amount| amount.to_string_opts(true, false)) .unwrap_or_else(|| "None".to_string()); - confirmation_message.push_str(&format!( - "Name: {}\nBase Supply: {}\nMax Supply: {}\n\n", - self.token_names_input[0].0, base_supply_display, max_supply_display, - )); - - confirmation_message.push_str(&format!( - "Estimated cost to register this token is {} Dash", - self.estimate_registration_cost() as f64 / 100_000_000_000.0 - )); - // Tokens are always created NotTradeable; warn if that can never change. - let mut is_danger_mode = false; - if let Some(args) = &self.cached_build_args { - let marketplace_rules_locked = matches!( + let is_danger_mode = self.cached_build_args.as_ref().is_some_and(|args| { + matches!( args.marketplace_rules, ChangeControlRules::V0(ChangeControlRulesV0 { authorized_to_make_change: AuthorizedActionTakers::NoOne, admin_action_takers: AuthorizedActionTakers::NoOne, .. }) - ); - - if marketplace_rules_locked { - confirmation_message.push_str("\n\nWARNING: This token will be permanently set to NotTradeable and can NEVER be made tradeable in the future!"); - is_danger_mode = true; - } - } + ) + }); + let name = &self.token_names_input[0].0; + let cost = self.estimate_registration_cost() as f64 / 100_000_000_000.0; + let confirmation_message = if is_danger_mode { + format!( + "Are you sure you want to register a new token contract with these settings?\n\nName: {name}\nBase Supply: {base_supply}\nMax Supply: {max_supply}\n\nEstimated cost to register this token is {cost} Dash\n\nWARNING: This token will be permanently set to NotTradeable and can NEVER be made tradeable in the future!", + base_supply = base_supply_display, + max_supply = max_supply_display, + ) + } else { + format!( + "Are you sure you want to register a new token contract with these settings?\n\nName: {name}\nBase Supply: {base_supply}\nMax Supply: {max_supply}\n\nEstimated cost to register this token is {cost} Dash", + base_supply = base_supply_display, + max_supply = max_supply_display, + ) + }; let confirmation_dialog = self .token_creator_confirmation_dialog @@ -1568,14 +1573,14 @@ impl TokensScreen { if let Some(ref error) = self.document_schemas_error { ui.colored_label( Color32::DARK_RED, - format!("Schema validation error: {}", error), + format!("Schema validation error: {error}"), ); } else if let Some(parsed_document_schemas) = self.parsed_document_schemas.as_ref() { let schema_count = parsed_document_schemas.len(); if schema_count > 0 { ui.colored_label( Color32::DARK_GREEN, - format!("✓ {} valid document schema(s) parsed", schema_count), + format!("✓ {schema_count} valid document schema(s) parsed"), ); } } @@ -1605,14 +1610,13 @@ impl TokensScreen { schemas.insert(key.clone(), value.clone()); } else { self.document_schemas_error = Some(format!( - "Document schema '{}' missing required 'type' field", - key + "Document schema '{key}' is missing the required 'type' field" )); return; } } else { self.document_schemas_error = - Some(format!("Document schema '{}' must be an object", key)); + Some(format!("Document schema '{key}' must be an object")); return; } } @@ -1625,8 +1629,12 @@ impl TokensScreen { } } } - Err(e) => { - self.document_schemas_error = Some(format!("Invalid JSON: {}", e)); + Err(error) => { + tracing::debug!(?error, "Token document-schema JSON parsing failed"); + self.document_schemas_error = Some( + "The document schema JSON is not valid. Check its syntax and try again." + .to_string(), + ); } } } diff --git a/src/ui/tools/grovestark_screen.rs b/src/ui/tools/grovestark_screen.rs index 417547b0c..08710aad8 100644 --- a/src/ui/tools/grovestark_screen.rs +++ b/src/ui/tools/grovestark_screen.rs @@ -463,12 +463,13 @@ impl GroveSTARKScreen { let task = BackendTask::GroveSTARKTask(GroveSTARKTask::VerifyProof { proof_data }); AppAction::BackendTask(task) } - Err(e) => { + Err(error) => { MessageBanner::set_global( app_context.egui_ctx(), - format!("Failed to parse proof: {}", e), + "The proof could not be parsed. Check its encoding and try again.", MessageType::Error, - ); + ) + .with_details(error); self.is_verifying = false; AppAction::None } diff --git a/src/ui/tools/transition_visualizer_screen.rs b/src/ui/tools/transition_visualizer_screen.rs index f1cd9b1fd..d51f446bb 100644 --- a/src/ui/tools/transition_visualizer_screen.rs +++ b/src/ui/tools/transition_visualizer_screen.rs @@ -102,13 +102,19 @@ impl TransitionVisualizerScreen { .filter(|s| !s.trim().is_empty()) // Skip empty segments .map(|s| s.trim().parse::()) .collect::, _>>() - .map_err(|e| format!("Failed to parse comma-separated integers: {}", e)) + .map_err(|error| { + tracing::debug!(?error, "Transition byte-list parsing failed"); + "The comma-separated values are not valid bytes. Use numbers from 0 to 255." + .to_string() + }) } else { // Try to decode the input as hex first hex::decode(self.input_data.trim()).or_else(|_| { - STANDARD - .decode(self.input_data.trim()) - .map_err(|e| format!("Base64 decode error: {}", e)) + STANDARD.decode(self.input_data.trim()).map_err(|error| { + tracing::debug!(?error, "Transition base64 decoding failed"); + "The input is not valid hexadecimal or base64 data. Check it and try again." + .to_string() + }) }) }; @@ -130,17 +136,23 @@ impl TransitionVisualizerScreen { ); } } - Err(e) => { + Err(error) => { + tracing::debug!(?error, "Transition JSON serialization failed"); self.parse_error = Some(( - format!("Failed to serialize to JSON: {}", e), + "The transition could not be displayed as JSON. Check the input and try again." + .to_string(), Instant::now(), )); } } } - Err(e) => { - self.parse_error = - Some((format!("Failed to parse: {}", e), Instant::now())); + Err(error) => { + tracing::debug!(?error, "State-transition deserialization failed"); + self.parse_error = Some(( + "The state transition could not be read. Check the input format and try again." + .to_string(), + Instant::now(), + )); } } } @@ -270,7 +282,7 @@ impl TransitionVisualizerScreen { }; ui.colored_label( Color32::from_rgba_premultiplied(139, 0, 0, alpha), // Dark red - format!("Error: {}", msg), + msg, ); ui.ctx().request_repaint_after(Duration::from_millis(100)); } else { @@ -401,14 +413,14 @@ impl ScreenLike for TransitionVisualizerScreen { crate::ui::BackendTaskSuccessResult::FetchedContract(contract) => { let contract_id = contract.id().to_string(Encoding::Base58); self.contract_fetch_message = Some(( - format!("✅ Contract {} fetched successfully", contract_id), + format!("✅ Contract {contract_id} fetched successfully"), Instant::now(), )); } crate::ui::BackendTaskSuccessResult::FetchedContracts(contracts) => { let count = contracts.iter().filter(|c| c.is_some()).count(); self.contract_fetch_message = Some(( - format!("✅ {} contract(s) fetched successfully", count), + format!("✅ {count} contract(s) fetched successfully"), Instant::now(), )); } @@ -458,7 +470,7 @@ impl ScreenLike for TransitionVisualizerScreen { ui.add_space(10.0); if let Some(ref contract_id) = self.selected_contract_id { - ui.label(format!("Contract ID: {}", contract_id)); + ui.label(format!("Contract ID: {contract_id}")); ui.add_space(10.0); // Check if contract already exists diff --git a/src/ui/wallets/import_mnemonic_screen.rs b/src/ui/wallets/import_mnemonic_screen.rs index bad96b5de..24605a5c7 100644 --- a/src/ui/wallets/import_mnemonic_screen.rs +++ b/src/ui/wallets/import_mnemonic_screen.rs @@ -95,9 +95,12 @@ impl ImportMnemonicScreen { self.parsed_single_key_wallet = Some(wallet); self.error = None; } - Err(e) => { + Err(error) => { + tracing::debug!(?error, "Imported private-key preview parsing failed"); self.parsed_single_key_wallet = None; - self.error = Some(format!("Invalid private key: {}", e)); + self.error = Some( + "The private key is not valid. Check the WIF or hexadecimal value.".to_string(), + ); } } } @@ -130,7 +133,7 @@ impl ImportMnemonicScreen { .read() .map(|w| w.len()) .unwrap_or(0); - Some(format!("Key {}", existing_wallet_count + 1)) + Some(format!("Key {number}", number = existing_wallet_count + 1)) } else { Some(self.alias_input.clone()) }; @@ -147,15 +150,22 @@ impl ImportMnemonicScreen { })?; if bytes.len() != 32 { return Err(format!( - "Hex private keys must be exactly 32 bytes; got {} bytes.", - bytes.len() + "Hex private keys must be exactly 32 bytes; got {byte_count} bytes.", + byte_count = bytes.len() )); } let mut buf = [0u8; 32]; buf.copy_from_slice(&bytes); - PrivateKey::from_byte_array(&buf, self.app_context.network) - .map_err(|e| format!("Invalid private key: {e}"))? - .to_wif() + match PrivateKey::from_byte_array(&buf, self.app_context.network) { + Ok(private_key) => private_key.to_wif(), + Err(error) => { + tracing::debug!(?error, "Imported hexadecimal private key was rejected"); + return Err( + "The private key is not valid. Check the hexadecimal value and try again." + .to_string(), + ); + } + } } }; @@ -192,7 +202,7 @@ impl ImportMnemonicScreen { .read() .map(|w| w.len()) .unwrap_or(0); - format!("Wallet {}", existing_wallet_count + 1) + format!("Wallet {number}", number = existing_wallet_count + 1) } else { self.alias_input.clone() }; @@ -305,14 +315,14 @@ impl ImportMnemonicScreen { ui.label("Seed Phrase Length:"); ComboBox::from_label("") - .selected_text(format!("{}", self.selected_seed_phrase_length)) + .selected_text(self.selected_seed_phrase_length.to_string()) .width(100.0) .show_ui(ui, |ui| { for &length in &[12, 15, 18, 21, 24] { ui.selectable_value( &mut self.selected_seed_phrase_length, length, - format!("{}", length), + length.to_string(), ); } }); @@ -335,7 +345,7 @@ impl ImportMnemonicScreen { .show(ui, |ui| { for i in 0..self.selected_seed_phrase_length { ui.horizontal(|ui| { - ui.label(format!("{:2}:", i + 1)); + ui.label(format!("{word_number:2}:", word_number = i + 1)); let mut word = self.seed_phrase_words[i].clone(); @@ -382,8 +392,7 @@ impl ImportMnemonicScreen { fn render_private_key_input(&mut self, ui: &mut Ui, step: u32) { ui.heading(format!( - "{}. Enter your private key (WIF or 64-character hex format)", - step + "{step}. Enter your private key (WIF or 64-character hex format)" )); ui.add_space(8.0); @@ -485,7 +494,7 @@ impl ScreenLike for ImportMnemonicScreen { // Import type selection (only show when advanced options is checked) if self.show_advanced_options { - ui.heading(format!("{}. Select what you want to import.", step)); + ui.heading(format!("{step}. Select what you want to import.")); ui.add_space(10.0); self.render_import_type_selection(ui); ui.add_space(10.0); @@ -495,7 +504,7 @@ impl ScreenLike for ImportMnemonicScreen { // Identity scan count option (only for mnemonic/HD wallets) if self.import_type == ImportType::Mnemonic { - ui.heading(format!("{}. Configure identity auto-discovery.", step)); + ui.heading(format!("{step}. Configure identity auto-discovery.")); ui.add_space(10.0); ui.horizontal(|ui| { ui.label("Identity indices to scan:"); @@ -517,7 +526,7 @@ impl ScreenLike for ImportMnemonicScreen { // Different UI based on import type match self.import_type { ImportType::Mnemonic => { - ui.heading(format!("{}. Select the seed phrase length and enter all words.", step)); + ui.heading(format!("{step}. Select the seed phrase length and enter all words.")); self.render_seed_phrase_input(ui); // Check seed phrase validity whenever all words are filled @@ -570,7 +579,7 @@ impl ScreenLike for ImportMnemonicScreen { ui.separator(); ui.add_space(10.0); - ui.heading(format!("{}. Enter a name to remember it by. (This will not go on the blockchain)", step)); + ui.heading(format!("{step}. Enter a name to remember it by. (This will not go on the blockchain)")); ui.add_space(8.0); @@ -585,7 +594,7 @@ impl ScreenLike for ImportMnemonicScreen { ui.separator(); ui.add_space(10.0); - ui.heading(format!("{}. Add a password to encrypt. (Optional but recommended)", step)); + ui.heading(format!("{step}. Add a password to encrypt. (Optional but recommended)")); ui.add_space(8.0); @@ -643,8 +652,8 @@ impl ScreenLike for ImportMnemonicScreen { ui.add_space(10.0); ui.label(format!( - "Estimated time to crack: {}", - self.estimated_time_to_crack + "Estimated time to crack: {duration}", + duration = self.estimated_time_to_crack )); step += 1; @@ -654,8 +663,8 @@ impl ScreenLike for ImportMnemonicScreen { ui.add_space(10.0); let button_text = match self.import_type { - ImportType::Mnemonic => format!("{}. Save the wallet.", step), - ImportType::PrivateKey => format!("{}. Import the key.", step), + ImportType::Mnemonic => format!("{step}. Save the wallet."), + ImportType::PrivateKey => format!("{step}. Import the key."), }; ui.heading(button_text); ui.add_space(10.0); diff --git a/src/ui/wallets/single_key_send_screen.rs b/src/ui/wallets/single_key_send_screen.rs index ae3710836..b56e5581f 100644 --- a/src/ui/wallets/single_key_send_screen.rs +++ b/src/ui/wallets/single_key_send_screen.rs @@ -226,13 +226,14 @@ impl SingleKeyWalletSendScreen { let mut total_amount: u64 = 0; for (index, recipient) in self.recipients.iter().enumerate() { + let recipient_number = index + 1; if recipient.address.trim().is_empty() { - return Err(format!("Recipient {} has an empty address", index + 1)); + return Err(format!("Recipient {recipient_number} has an empty address")); } let amount = Self::parse_amount_to_duffs(&recipient.amount) - .map_err(|e| format!("Recipient {}: {}", index + 1, e))?; + .map_err(|error| format!("Recipient {recipient_number}: {error}"))?; if amount == 0 { - return Err(format!("Recipient {} has zero amount", index + 1)); + return Err(format!("Recipient {recipient_number} has zero amount")); } total_amount = total_amount.saturating_add(amount); @@ -247,9 +248,9 @@ impl SingleKeyWalletSendScreen { let wallet_guard = wallet.read().map_err(|e| e.to_string())?; if total_amount > wallet_guard.total_balance { return Err(format!( - "Insufficient balance. Need {} but only have {}", - format_duffs_as_dash(total_amount), - format_duffs_as_dash(wallet_guard.total_balance) + "Insufficient balance. Need {needed} but only have {available}", + needed = format_duffs_as_dash(total_amount), + available = format_duffs_as_dash(wallet_guard.total_balance) )); } } @@ -320,11 +321,12 @@ impl SingleKeyWalletSendScreen { .show(ui, |ui| { for i in 0..recipient_count { let recipient_id = self.recipients[i].id; + let recipient_number = i + 1; // Address field ui.horizontal(|ui| { ui.label( - RichText::new(format!("Address {}:", i + 1)) + RichText::new(format!("Address {recipient_number}:")) .color(DashColors::text_secondary(dark_mode)) .size(14.0), ); @@ -342,7 +344,7 @@ impl SingleKeyWalletSendScreen { // Amount field ui.label( - RichText::new(format!("Amount {} (DASH):", i + 1)) + RichText::new(format!("Amount {recipient_number} (DASH):")) .color(DashColors::text_secondary(dark_mode)) .size(14.0), ); @@ -418,9 +420,8 @@ impl SingleKeyWalletSendScreen { ); ui.label( RichText::new(format!( - "{} ({:.8} DASH)", - estimated_fee, - estimated_fee as f64 * 1e-8 + "{estimated_fee} ({fee_dash:.8} DASH)", + fee_dash = estimated_fee as f64 * 1e-8 )) .color(DashColors::text_primary(dark_mode)) .size(14.0), @@ -434,7 +435,7 @@ impl SingleKeyWalletSendScreen { .size(12.0), ); ui.label( - RichText::new(format!("{} inputs, ~{} bytes", utxo_count, tx_size)) + RichText::new(format!("{utxo_count} inputs, ~{tx_size} bytes")) .color(DashColors::text_secondary(dark_mode)) .size(12.0), ); @@ -512,9 +513,12 @@ impl SingleKeyWalletSendScreen { .size(14.0), ); ui.label( - RichText::new(format!("~{:.8} DASH", estimated_fee as f64 * 1e-8)) - .color(DashColors::text_primary(dark_mode)) - .size(14.0), + RichText::new(format!( + "~{fee_dash:.8} DASH", + fee_dash = estimated_fee as f64 * 1e-8 + )) + .color(DashColors::text_primary(dark_mode)) + .size(14.0), ); }); } @@ -557,9 +561,9 @@ impl SingleKeyWalletSendScreen { ); ui.label( RichText::new(format!( - "{} duffs ({:.8} DASH)", - self.fee_dialog.estimated_fee, - self.fee_dialog.estimated_fee as f64 * 1e-8 + "{estimated_fee} duffs ({fee_dash:.8} DASH)", + estimated_fee = self.fee_dialog.estimated_fee, + fee_dash = self.fee_dialog.estimated_fee as f64 * 1e-8 )) .color(DashColors::text_primary(dark_mode)), ); @@ -572,9 +576,9 @@ impl SingleKeyWalletSendScreen { ); ui.label( RichText::new(format!( - "{} duffs ({:.8} DASH)", - self.fee_dialog.required_fee, - self.fee_dialog.required_fee as f64 * 1e-8 + "{required_fee} duffs ({fee_dash:.8} DASH)", + required_fee = self.fee_dialog.required_fee, + fee_dash = self.fee_dialog.required_fee as f64 * 1e-8 )) .color(DashColors::WARNING) .strong(), @@ -592,9 +596,8 @@ impl SingleKeyWalletSendScreen { ); ui.label( RichText::new(format!( - "+{} duffs ({:.8} DASH)", - fee_diff, - fee_diff as f64 * 1e-8 + "+{fee_diff} duffs ({fee_dash:.8} DASH)", + fee_dash = fee_diff as f64 * 1e-8 )) .color(DashColors::text_primary(dark_mode)), ); @@ -930,23 +933,24 @@ impl ScreenLike for SingleKeyWalletSendScreen { let msg = if recipients.len() == 1 { let (address, amount) = &recipients[0]; format!( - "Sent {} to {}\nTxID: {}", - format_duffs_as_dash(*amount), - address, - txid + "Sent {amount} to {address}\nTxID: {txid}", + amount = format_duffs_as_dash(*amount) ) } else { let recipient_list: String = recipients .iter() - .map(|(addr, amt)| format!(" {} to {}", format_duffs_as_dash(*amt), addr)) + .map(|(address, amount)| { + format!( + " {amount} to {address}", + amount = format_duffs_as_dash(*amount) + ) + }) .collect::>() .join("\n"); format!( - "Sent {} total to {} recipients:\n{}\nTxID: {}", - format_duffs_as_dash(total_amount), - recipients.len(), - recipient_list, - txid + "Sent {total_amount} total to {recipient_count} recipients:\n{recipient_list}\nTxID: {txid}", + total_amount = format_duffs_as_dash(total_amount), + recipient_count = recipients.len() ) }; MessageBanner::set_global(self.app_context.egui_ctx(), &msg, MessageType::Success); diff --git a/src/ui/wallets/wallets_screen/mod.rs b/src/ui/wallets/wallets_screen/mod.rs index ab3cb0d83..85be52593 100644 --- a/src/ui/wallets/wallets_screen/mod.rs +++ b/src/ui/wallets/wallets_screen/mod.rs @@ -15,7 +15,9 @@ use crate::context::feature_gate::FeatureGate; use crate::model::fee_estimation::format_duffs_as_dash; use crate::model::spv_status::SpvStatus; use crate::model::user_role::UserRole; -use crate::model::wallet::{TransactionStatus, Wallet, WalletSeedHash, WalletTransaction}; +use crate::model::wallet::{ + TransactionStatus, Wallet, WalletSeedHash, WalletTransaction, validate_wallet_alias, +}; use crate::ui::components::MessageBanner; use crate::ui::components::component_trait::Component; use crate::ui::components::confirmation_dialog::{ConfirmationDialog, ConfirmationStatus}; @@ -589,9 +591,8 @@ impl WalletsBalancesScreen { let balance_dash = (core_balance + platform_balance + shielded_balance) as f64 * 1e-8; let label = format!( - "HD: {} ({:.4} DASH)", - guard.alias.clone().unwrap_or_else(|| "Unnamed".to_string()), - balance_dash + "HD: {alias} ({balance_dash:.4} DASH)", + alias = guard.alias.clone().unwrap_or_else(|| "Unnamed".to_string()) ); items.push((label, WalletItem::Hd(wallet.clone()))); } @@ -603,9 +604,8 @@ impl WalletsBalancesScreen { let guard = wallet.read_recover(); let balance_dash = guard.total_balance_duffs() as f64 * 1e-8; let label = format!( - "SK: {} ({:.4} DASH)", - guard.alias.clone().unwrap_or_else(|| "Unnamed".to_string()), - balance_dash + "SK: {alias} ({balance_dash:.4} DASH)", + alias = guard.alias.clone().unwrap_or_else(|| "Unnamed".to_string()) ); items.push((label, WalletItem::SingleKey(wallet.clone()))); } @@ -623,8 +623,8 @@ impl WalletsBalancesScreen { .ok() .map(|guard| { format!( - "HD: {}", - guard.alias.clone().unwrap_or_else(|| "Unnamed".to_string()) + "HD: {alias}", + alias = guard.alias.clone().unwrap_or_else(|| "Unnamed".to_string()) ) }) .unwrap_or_else(|| "Select a wallet".to_string()) @@ -634,8 +634,8 @@ impl WalletsBalancesScreen { .ok() .map(|guard| { format!( - "SK: {}", - guard.alias.clone().unwrap_or_else(|| "Unnamed".to_string()) + "SK: {alias}", + alias = guard.alias.clone().unwrap_or_else(|| "Unnamed".to_string()) ) }) .unwrap_or_else(|| "Select a wallet".to_string()) @@ -699,7 +699,10 @@ impl WalletsBalancesScreen { ui.colored_label( DashColors::text_primary(ui.style().visuals.dark_mode), - format!(" Balance: {}", format_duffs_as_dash(current_balance)), + format!( + " Balance: {balance}", + balance = format_duffs_as_dash(current_balance) + ), ); }); @@ -876,8 +879,7 @@ impl WalletsBalancesScreen { .clone() .unwrap_or_else(|| "Unnamed Wallet".to_string()); let message = format!( - "Removing wallet \"{}\" clears the data used by this version, including its addresses, balances, and asset locks. Identities linked to it will remain, but keys derived from this wallet will not work unless the wallet is imported again. If this wallet came from an earlier version, that version's read-only recovery database stays on this device. Continue?", - alias + "Removing wallet \"{alias}\" clears the data used by this version, including its addresses, balances, and asset locks. Identities linked to it will remain, but keys derived from this wallet will not work unless the wallet is imported again. If this wallet came from an earlier version, that version's read-only recovery database stays on this device. Continue?" ); ( PendingWalletRemoval::Hd { @@ -893,8 +895,7 @@ impl WalletsBalancesScreen { .clone() .unwrap_or_else(|| "Unnamed Wallet".to_string()); let message = format!( - "Removing wallet \"{}\" will delete its imported private key and local wallet data from this device. Make sure you have a backup of the private key before continuing. Continue?", - alias + "Removing wallet \"{alias}\" will delete its imported private key and local wallet data from this device. Make sure you have a backup of the private key before continuing. Continue?" ); ( PendingWalletRemoval::SingleKey { @@ -944,7 +945,7 @@ impl WalletsBalancesScreen { self.persist_selected_single_key_hash(None); MessageBanner::set_global( self.app_context.egui_ctx(), - format!("Removed wallet \"{}\" successfully.", alias), + format!("Removed wallet \"{alias}\" successfully."), MessageType::Success, ); } @@ -968,16 +969,17 @@ impl WalletsBalancesScreen { MessageBanner::set_global( self.app_context.egui_ctx(), - format!("Removed wallet \"{}\" successfully", alias), + format!("Removed wallet \"{alias}\" successfully"), MessageType::Success, ); } - Err(err) => { + Err(error) => { MessageBanner::set_global( self.app_context.egui_ctx(), - format!("Failed to remove wallet: {}", err), + "The wallet could not be removed. Close any open wallet actions and try again.", MessageType::Error, - ); + ) + .with_details(error); } } } @@ -1076,13 +1078,13 @@ impl WalletsBalancesScreen { fn format_duration_ago(duration: std::time::Duration) -> String { let secs = duration.as_secs(); if secs < 60 { - format!("{}s ago", secs) + format!("{secs}s ago") } else if secs < 3600 { - format!("{}m ago", secs / 60) + format!("{minutes}m ago", minutes = secs / 60) } else if secs < 86400 { - format!("{}h ago", secs / 3600) + format!("{hours}h ago", hours = secs / 3600) } else { - format!("{}d ago", secs / 86400) + format!("{days}d ago", days = secs / 86400) } } @@ -1099,9 +1101,9 @@ impl WalletsBalancesScreen { fn transaction_amount_display(tx: &WalletTransaction, dark_mode: bool) -> (String, Color32) { let amount = format_duffs_as_dash(tx.amount_abs()); if tx.is_incoming() { - (format!("+{}", amount), DashColors::SUCCESS) + (format!("+{amount}"), DashColors::SUCCESS) } else if tx.is_outgoing() { - (format!("-{}", amount), DashColors::ERROR) + (format!("-{amount}"), DashColors::ERROR) } else { (amount, DashColors::text_primary(dark_mode)) } @@ -1113,11 +1115,11 @@ impl WalletsBalancesScreen { TransactionStatus::InstantSendLocked => "⚡ InstantSend".to_string(), TransactionStatus::Confirmed => tx .height - .map(|h| format!("Confirmed @{}", h)) + .map(|height| format!("Confirmed @{height}")) .unwrap_or_else(|| "Confirmed".to_string()), TransactionStatus::ChainLocked => tx .height - .map(|h| format!("🔒 ChainLocked @{}", h)) + .map(|height| format!("🔒 ChainLocked @{height}")) .unwrap_or_else(|| "🔒 ChainLocked".to_string()), } } @@ -1288,8 +1290,8 @@ impl WalletsBalancesScreen { if ui .button( RichText::new(format!( - "Refresh mode: {}", - self.refresh_mode.label() + "Refresh mode: {mode}", + mode = self.refresh_mode.label() )) .color(DashColors::text_primary(dark_mode)) .strong(), @@ -1410,9 +1412,9 @@ impl WalletsBalancesScreen { fn format_tab_balance(duffs: u64) -> String { let dash = duffs as f64 / 100_000_000.0; // Format with 4 decimal places, then trim trailing zeros - let formatted = format!("{:.4}", dash); + let formatted = format!("{dash:.4}"); let trimmed = formatted.trim_end_matches('0').trim_end_matches('.'); - format!("{} DASH", trimmed) + format!("{trimmed} DASH") } /// Render the Accounts & Addresses tab bar and content. @@ -1476,12 +1478,11 @@ impl WalletsBalancesScreen { } }; let label = if balance_duffs == 0 { - format!("{} (empty)", base_label) + format!("{base_label} (empty)") } else { format!( - "{} ({})", - base_label, - Self::format_tab_balance(balance_duffs) + "{base_label} ({balance})", + balance = Self::format_tab_balance(balance_duffs) ) }; let is_selected = &self.selected_account_tab == tab; @@ -1561,13 +1562,16 @@ impl WalletsBalancesScreen { self.selected_account = Some((cat.clone(), idx)); // Addresses (collapsible) - let addresses_heading = format!("Addresses ({})", cat.label(idx)); + let addresses_heading = format!("Addresses ({label})", label = cat.label(idx)); let addr_header = egui::CollapsingHeader::new( RichText::new(addresses_heading) .size(16.0) .color(DashColors::text_primary(dark_mode)), ) - .id_salt(format!("addresses_{}_{:?}", cat.tab_label(idx), idx)) + .id_salt(format!( + "addresses_{label}_{idx:?}", + label = cat.tab_label(idx) + )) .default_open(true); addr_header.show(ui, |ui| { ui.horizontal(|ui| { @@ -1670,17 +1674,15 @@ impl WalletsBalancesScreen { Self::format_tab_balance(*balance) }; let heading = format!( - "{} ({} addresses, {})", - cat.label(*idx), - addr_count, - balance_text + "{category} ({addr_count} addresses, {balance_text})", + category = cat.label(*idx) ); let header = egui::CollapsingHeader::new( RichText::new(heading) .size(14.0) .color(DashColors::text_primary(dark_mode)), ) - .id_salt(format!("system_section_{:?}_{:?}", cat, idx)) + .id_salt(format!("system_section_{cat:?}_{idx:?}")) .default_open(false); header.show(ui, |ui| { if let Some(description) = cat.description() { @@ -1907,8 +1909,7 @@ impl WalletsBalancesScreen { .clicked() { ui.ctx().open_url(egui::OpenUrl::new_tab(format!( - "{}{}", - base_url, full_txid + "{base_url}{full_txid}" ))); } }); @@ -1969,8 +1970,8 @@ impl WalletsBalancesScreen { ui.colored_label( Color32::DARK_GREEN, RichText::new(format!( - "Synced — {} peers", - snapshot.connected_peers + "Synced — {peer_count} peers", + peer_count = snapshot.connected_peers )) .size(sz), ); @@ -2040,8 +2041,10 @@ impl WalletsBalancesScreen { ui.label(RichText::new("•").size(sz).color(secondary)); let shielded_text = match shielded_seed_hash { Some(hash) => format!( - "Shielded: {}", - format_duffs_as_dash(self.app_context.shielded_balance_duffs(&hash)) + "Shielded: {balance}", + balance = format_duffs_as_dash( + self.app_context.shielded_balance_duffs(&hash) + ) ), None => "Shielded: unavailable".to_string(), }; @@ -2061,10 +2064,13 @@ impl WalletsBalancesScreen { let total = core_balance + platform_balance + shielded_balance; ui.label( - RichText::new(format!("Balance: {}", format_duffs_as_dash(total))) - .color(DashColors::text_primary(dark_mode)) - .size(20.0) - .strong(), + RichText::new(format!( + "Balance: {balance}", + balance = format_duffs_as_dash(total) + )) + .color(DashColors::text_primary(dark_mode)) + .size(20.0) + .strong(), ); } @@ -2086,16 +2092,19 @@ impl WalletsBalancesScreen { header.show(ui, |ui| { ui.horizontal(|ui| { - ui.label(format!("Core: {}", format_duffs_as_dash(core_balance))); + ui.label(format!( + "Core: {balance}", + balance = format_duffs_as_dash(core_balance) + )); ui.label(" | "); ui.label(format!( - "Platform: {}", - format_duffs_as_dash(platform_balance) + "Platform: {balance}", + balance = format_duffs_as_dash(platform_balance) )); ui.label(" | "); ui.label(format!( - "Shielded: {}", - format_duffs_as_dash(shielded_balance) + "Shielded: {balance}", + balance = format_duffs_as_dash(shielded_balance) )); }); }); @@ -2230,12 +2239,13 @@ impl WalletsBalancesScreen { let locked = { let mut wallet = match wallet_arc.write() { Ok(guard) => guard, - Err(err) => { + Err(error) => { MessageBanner::set_global( self.app_context.egui_ctx(), - format!("Failed to lock wallet: {}", err), + "The wallet could not be locked. Finish any active wallet action and try again.", MessageType::Error, - ); + ) + .with_details(error); return; } }; @@ -2357,7 +2367,10 @@ impl WalletsBalancesScreen { Ok(_) => { MessageBanner::set_global( ctx, - format!("Imported key added for {}.", request.address_preview), + format!( + "Imported key added for {address}.", + address = request.address_preview + ), MessageType::Success, ); self.import_single_key_dialog.open = false; @@ -2680,16 +2693,18 @@ impl ScreenLike for WalletsBalancesScreen { ui.add_space(8.0); if ComponentStyles::add_primary_button(ui, "Save").clicked() { - // Limit the alias length to 64 characters - if self.rename_input.len() > 64 { - self.rename_input.truncate(64); + if let Err(error) = validate_wallet_alias(&self.rename_input) { + MessageBanner::set_global( + ctx, + "The wallet name is too long. Use 64 characters or fewer and try again.", + MessageType::Error, + ) + .with_details(error); + return; } // Handle HD wallet rename if let Some(selected_wallet) = &self.selected_wallet { - let mut wallet = selected_wallet.write_recover(); - wallet.alias = Some(self.rename_input.clone()); - // T-W-01: alias persistence goes // through the wallet-meta sidecar. // The cold-boot picker reads from @@ -2697,37 +2712,50 @@ impl ScreenLike for WalletsBalancesScreen { // name surfaces on the next launch // without touching the legacy // `wallet` table. - let seed_hash = wallet.seed_hash(); - if let Ok(backend) = self.app_context.wallet_backend() { - let meta_view = backend.wallet_meta(); - let mut meta = meta_view - .get(self.app_context.network, &seed_hash) - .unwrap_or_default(); - meta.alias = self.rename_input.clone(); - // Backfill the xpub on first - // rename after migration so old - // entries written before T-W-00.5 - // get a non-empty picker hint. - if meta.xpub_encoded.is_empty() { - meta.xpub_encoded = wallet + let (seed_hash, xpub_encoded) = { + let wallet = selected_wallet.read_recover(); + ( + wallet.seed_hash(), + wallet .master_bip44_ecdsa_extended_public_key .encode() - .to_vec(); + .to_vec(), + ) + }; + let new_alias = self.rename_input.clone(); + let persisted = match self.app_context.wallet_backend() { + Ok(backend) => { + let meta_view = backend.wallet_meta(); + let mut meta = meta_view + .get(self.app_context.network, &seed_hash) + .unwrap_or_default(); + meta.alias = new_alias.clone(); + if meta.xpub_encoded.is_empty() { + meta.xpub_encoded = xpub_encoded; + } + meta_view.set( + self.app_context.network, + &seed_hash, + &meta, + ) } - if let Err(e) = meta_view.set( - self.app_context.network, - &seed_hash, - &meta, - ) { - tracing::warn!( - wallet = %hex::encode(seed_hash), - error = ?e, - "Failed to persist wallet alias to sidecar", - ); + Err(error) => Err(error), + }; + match persisted { + Ok(()) => { + selected_wallet.write_recover().alias = Some(new_alias); + self.show_rename_dialog = false; + self.rename_input.clear(); + } + Err(error) => { + MessageBanner::set_global( + ctx, + "The wallet name could not be saved. Check available disk space and try again.", + MessageType::Error, + ) + .with_details(error); } } - self.show_rename_dialog = false; - self.rename_input.clear(); } // Handle single key wallet rename else if let Some(selected_sk_wallet) = @@ -2848,8 +2876,7 @@ impl ScreenLike for WalletsBalancesScreen { && let Ok(wallet) = wallet_arc.read() { if let Some(alias) = &wallet.alias { ui.label(format!( - "Wallet \"{}\" is locked. Please enter the password to unlock it:", - alias + "Wallet \"{alias}\" is locked. Please enter the password to unlock it:" )); } else { ui.label("This wallet is locked. Please enter the password to unlock it:"); @@ -3056,17 +3083,14 @@ impl ScreenLike for WalletsBalancesScreen { let msg = if recipients.len() == 1 { let (address, amount) = &recipients[0]; format!( - "Sent {} to {}\nTxID: {}", - format_duffs_as_dash(*amount), - address, - txid + "Sent {amount} to {address}\nTxID: {txid}", + amount = format_duffs_as_dash(*amount) ) } else { format!( - "Sent {} total to {} recipients\nTxID: {}", - format_duffs_as_dash(total_amount), - recipients.len(), - txid + "Sent {total_amount} total to {recipient_count} recipients\nTxID: {txid}", + total_amount = format_duffs_as_dash(total_amount), + recipient_count = recipients.len() ) }; MessageBanner::set_global(self.app_context.egui_ctx(), &msg, MessageType::Success); @@ -3186,7 +3210,7 @@ impl ScreenLike for WalletsBalancesScreen { self.refreshing = false; MessageBanner::set_global( self.app_context.egui_ctx(), - format!("Mined {} block(s)", count), + format!("Mined {count} block(s)"), MessageType::Success, ); } diff --git a/src/wallet_backend/single_key.rs b/src/wallet_backend/single_key.rs index 87742b78c..926bc9b2f 100644 --- a/src/wallet_backend/single_key.rs +++ b/src/wallet_backend/single_key.rs @@ -177,6 +177,10 @@ impl<'a> SingleKeyView<'a> { alias: Option, passphrase: ImportPassphrase, ) -> Result { + if let Some(alias) = alias.as_deref() { + crate::model::wallet::validate_wallet_alias(alias) + .map_err(|source| TaskError::InvalidWalletAliasLength { source })?; + } let priv_key = PrivateKey::from_wif(wif).map_err(|source| TaskError::InvalidWif { source: Box::new(source), })?; @@ -272,6 +276,10 @@ impl<'a> SingleKeyView<'a> { /// construction path) — the in-memory index is still updated so the /// rename is visible in-session. pub fn set_alias(&self, address: &str, alias: Option) -> Result<(), TaskError> { + if let Some(alias) = alias.as_deref() { + crate::model::wallet::validate_wallet_alias(alias) + .map_err(|source| TaskError::InvalidWalletAliasLength { source })?; + } let mut idx = write_recover(self.index); let entry = idx.get_mut(address).ok_or(TaskError::ImportedKeyNotFound)?; entry.alias = alias; @@ -1018,6 +1026,25 @@ mod tests { "cMahea7zqjxrtgAbB7LSGbcQUr1uX1ojuat9jZodMN8rFTv2sfUK" } + #[test] + fn import_wif_rejects_overlong_alias_before_writes() { + let dir = tempfile::tempdir().expect("tempdir"); + let (store, index, network) = fresh_view(dir.path(), Network::Testnet); + let view = SingleKeyView { + secret_store: &store, + index: &index, + network, + app_kv: None, + }; + + let error = view + .import_wif(known_wif(), Some("w".repeat(65))) + .expect_err("overlong alias must fail"); + + assert!(matches!(error, TaskError::InvalidWalletAliasLength { .. })); + assert!(view.list().is_empty()); + } + /// TC-SK-003: importing a WIF writes exactly one entry whose label /// matches `^single_key_priv\.[1-9A-HJ-NP-Za-km-z]{26,35}$` and is /// scoped to the per-backend single-key `WalletId` namespace. @@ -1765,6 +1792,33 @@ mod tests { assert!(matches!(err, TaskError::ImportedKeyNotFound), "got {err:?}"); } + #[test] + fn set_alias_rejects_overlong_alias_before_persisting() { + let dir = tempfile::tempdir().expect("tempdir"); + let ViewFixture { + store, + index, + kv, + network, + } = fresh_view_with_kv(dir.path(), Network::Testnet); + let view = SingleKeyView { + secret_store: &store, + index: &index, + network, + app_kv: Some(&kv), + }; + let imported = view + .import_wif(known_wif(), Some("old name".into())) + .expect("import"); + + let error = view + .set_alias(&imported.address, Some("w".repeat(65))) + .expect_err("overlong alias must fail"); + + assert!(matches!(error, TaskError::InvalidWalletAliasLength { .. })); + assert_eq!(view.list()[0].alias.as_deref(), Some("old name")); + } + /// Legacy 32-byte raw vault payloads (pre per-key-passphrase) /// still decode as `has_passphrase = false`, so a user who /// upgrades from a previous tag never loses their imported keys. diff --git a/src/wallet_backend/wallet_meta.rs b/src/wallet_backend/wallet_meta.rs index 5ae1ee631..a71407cd5 100644 --- a/src/wallet_backend/wallet_meta.rs +++ b/src/wallet_backend/wallet_meta.rs @@ -127,6 +127,26 @@ impl<'a> WalletMetaView<'a> { seed_hash: &WalletSeedHash, meta: &WalletMeta, ) -> Result<(), TaskError> { + crate::model::wallet::validate_wallet_alias(&meta.alias) + .map_err(|source| TaskError::InvalidWalletAliasLength { source })?; + self.0.set(network, seed_hash, meta) + } + + /// Preserve metadata imported from legacy storage, including aliases that + /// predate the current length limit. New writes must use [`Self::set`]. + pub(crate) fn set_migrated( + &self, + network: Network, + seed_hash: &WalletSeedHash, + meta: &WalletMeta, + ) -> Result<(), TaskError> { + if let Err(error) = crate::model::wallet::validate_wallet_alias(&meta.alias) { + tracing::warn!( + alias_chars = error.actual, + max_alias_chars = error.max, + "Preserving an overlong legacy wallet alias during migration" + ); + } self.0.set(network, seed_hash, meta) } @@ -205,6 +225,29 @@ mod tests { ); } + #[test] + fn set_rejects_overlong_alias() { + let kv = kv(); + let view = WalletMetaView::new(&kv); + let seed: WalletSeedHash = [0x23; 32]; + let error = view + .set(Network::Mainnet, &seed, &meta(&"w".repeat(65), false, None)) + .expect_err("overlong alias must fail"); + assert!(matches!(error, TaskError::InvalidWalletAliasLength { .. })); + assert_eq!(view.get(Network::Mainnet, &seed), None); + } + + #[test] + fn migration_preserves_legacy_overlong_alias() { + let kv = kv(); + let view = WalletMetaView::new(&kv); + let seed: WalletSeedHash = [0x24; 32]; + let legacy_meta = meta(&"w".repeat(65), false, None); + view.set_migrated(Network::Mainnet, &seed, &legacy_meta) + .expect("legacy alias must be preserved"); + assert_eq!(view.get(Network::Mainnet, &seed), Some(legacy_meta)); + } + /// W-META-VIEW-003 — `list` does not leak entries from other /// networks (the `:` prefix is the partition). Mirrors /// the per-network isolation contract from `kv.rs::list`. From 05cb1b35ca1bf3100ca4b53853f6a95e1d9a4769 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:14:09 +0000 Subject: [PATCH 2/6] fix(wallets): validate HD alias before writing the seed envelope register_wallet ran validate_wallet_alias inside write_wallet_meta, which fires AFTER write_seed_envelope. An overlong HD alias therefore failed the meta write only after the encrypted seed was already resident in the vault, leaving an orphaned seed with no meta row (never hydrated, no cleanup path). Hoist the alias validation to the top of register_wallet, before any secret-critical write, mirroring the single-key import path. Adds a regression test asserting no raw-seed entry survives a rejected HD registration (RED against the buggy write order). Co-Authored-By: Claude Opus --- src/context/wallet_lifecycle/registration.rs | 10 +++++ src/context/wallet_lifecycle/tests.rs | 42 ++++++++++++++++++++ 2 files changed, 52 insertions(+) diff --git a/src/context/wallet_lifecycle/registration.rs b/src/context/wallet_lifecycle/registration.rs index 0ab68eaeb..97732b7c7 100644 --- a/src/context/wallet_lifecycle/registration.rs +++ b/src/context/wallet_lifecycle/registration.rs @@ -107,6 +107,16 @@ impl AppContext { let seed_hash = wallet.seed_hash(); let uses_password = wallet.uses_password; + // 0. Reject an invalid alias FIRST — this is pure input validation and + // must fail before any secret-critical write. A rejection at the + // `write_wallet_meta` layer would land AFTER `write_seed_envelope`, + // orphaning the encrypted seed (no meta row → never hydrated, no + // cleanup path). Mirrors the single-key import path. + if let Some(alias) = wallet.alias.as_deref() { + crate::model::wallet::validate_wallet_alias(alias) + .map_err(|source| TaskError::InvalidWalletAliasLength { source })?; + } + // 1. Reject a duplicate import. The upstream `platform-wallet.sqlite` // persistor is the system of record now; DET no longer writes the // legacy `data.db.wallet` row (the fresh-install schema gates that diff --git a/src/context/wallet_lifecycle/tests.rs b/src/context/wallet_lifecycle/tests.rs index 235e7efcf..2894989eb 100644 --- a/src/context/wallet_lifecycle/tests.rs +++ b/src/context/wallet_lifecycle/tests.rs @@ -2428,6 +2428,48 @@ async fn register_wallet_fails_closed_when_wallet_meta_write_fails() { ); } +/// An overlong alias is pure input validation and MUST be rejected BEFORE any +/// secret-critical write. Otherwise a meta-write-time rejection lands AFTER +/// `write_seed_envelope`, orphaning the encrypted seed (no meta row → never +/// hydrated, no cleanup path). Mirrors the single-key import path, which +/// already validates before writing. +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn register_wallet_rejects_overlong_alias_before_seed_write() { + let temp_dir = tempfile::tempdir().expect("tempdir"); + let (ctx, _sender) = offline_testnet_context_at(temp_dir.path()); + + let seed = [0x5Au8; 64]; + let wallet = crate::model::wallet::Wallet::new_from_seed( + seed, + Network::Testnet, + Some("w".repeat(65)), + None, + ) + .expect("build wallet"); + let seed_hash = wallet.seed_hash(); + + let result = ctx.register_wallet(wallet, &seed, WalletOrigin::Fresh); + assert!( + matches!(result, Err(TaskError::InvalidWalletAliasLength { .. })), + "an overlong alias must be rejected before any seed write" + ); + assert!( + WalletSeedView::new(&ctx.secret_store()) + .get_raw(&seed_hash) + .expect("read raw seed") + .is_none(), + "no seed material must survive a rejected HD registration (orphaned secret)" + ); + assert!( + !ctx.wallets.read_recover().contains_key(&seed_hash), + "a rejected wallet must not be kept in memory" + ); + assert!( + !ctx.has_wallet.load(Ordering::Relaxed), + "has_wallet must not flip true when registration is rejected" + ); +} + /// Build a valid BIP44 account-0 master xpub (testnet) for a legacy wallet row. fn legacy_master_epk_bytes(seed: &[u8; 64]) -> Vec { crate::database::test_helpers::legacy_master_epk_bytes(seed, Network::Testnet) From 4a93449362ec384333e873b68eca508b2b637bbd Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:14:15 +0000 Subject: [PATCH 3/6] fix(tokens): name the duplicated language, not the plural form The duplicate-token-name check keys on name_with_language.2 (the TokenNameLanguage) but the error message interpolated name_with_language.1 (the plural-name String), so users saw a nonsensical value and could not tell which language was duplicated. Interpolate .2 to match the check and the sibling name-length messages. Co-Authored-By: Claude Opus --- src/ui/tokens/tokens_screen/token_creator.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ui/tokens/tokens_screen/token_creator.rs b/src/ui/tokens/tokens_screen/token_creator.rs index 314ce6acb..5ca7bd11f 100644 --- a/src/ui/tokens/tokens_screen/token_creator.rs +++ b/src/ui/tokens/tokens_screen/token_creator.rs @@ -1236,7 +1236,7 @@ impl TokensScreen { if seen_languages.contains(&name_with_language.2) { return Err(format!( "Duplicate token name language: {language:?}", - language = name_with_language.1 + language = name_with_language.2 )); } seen_languages.insert(name_with_language.2); From 60528816dfd9fd02fc89d87d45fd58de2e41b6d1 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:14:25 +0000 Subject: [PATCH 4/6] fix(tools): surface concrete parse errors in the Transition Visualizer MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The Transition Visualizer is a Platform-Developer tool whose purpose is diagnosing why a state transition fails to parse. The unified error-message policy (aimed at the Everyday-User banner flows) had genericized these messages and routed the concrete error only to tracing::debug! (off by default) — and the inline parse_error label has no details panel, so the diagnostic was unreachable in the UI at default verbosity. Append the concrete error to each inline message (byte-list, hex/base64 decode, state-transition deserialize, JSON serialize) so the developer sees the actual failure without restarting under RUST_LOG=debug. GroveSTARK already preserves its error via .with_details() and is left unchanged. Co-Authored-By: Claude Opus --- src/ui/tools/transition_visualizer_screen.rs | 20 ++++++++++++-------- 1 file changed, 12 insertions(+), 8 deletions(-) diff --git a/src/ui/tools/transition_visualizer_screen.rs b/src/ui/tools/transition_visualizer_screen.rs index d51f446bb..128939bad 100644 --- a/src/ui/tools/transition_visualizer_screen.rs +++ b/src/ui/tools/transition_visualizer_screen.rs @@ -104,16 +104,18 @@ impl TransitionVisualizerScreen { .collect::, _>>() .map_err(|error| { tracing::debug!(?error, "Transition byte-list parsing failed"); - "The comma-separated values are not valid bytes. Use numbers from 0 to 255." - .to_string() + format!( + "The comma-separated values are not valid bytes. Use numbers from 0 to 255. ({error})" + ) }) } else { // Try to decode the input as hex first hex::decode(self.input_data.trim()).or_else(|_| { STANDARD.decode(self.input_data.trim()).map_err(|error| { tracing::debug!(?error, "Transition base64 decoding failed"); - "The input is not valid hexadecimal or base64 data. Check it and try again." - .to_string() + format!( + "The input is not valid hexadecimal or base64 data. Check it and try again. ({error})" + ) }) }) }; @@ -139,8 +141,9 @@ impl TransitionVisualizerScreen { Err(error) => { tracing::debug!(?error, "Transition JSON serialization failed"); self.parse_error = Some(( - "The transition could not be displayed as JSON. Check the input and try again." - .to_string(), + format!( + "The transition could not be displayed as JSON. Check the input and try again. ({error})" + ), Instant::now(), )); } @@ -149,8 +152,9 @@ impl TransitionVisualizerScreen { Err(error) => { tracing::debug!(?error, "State-transition deserialization failed"); self.parse_error = Some(( - "The state transition could not be read. Check the input format and try again." - .to_string(), + format!( + "The state transition could not be read. Check the input format and try again. ({error})" + ), Instant::now(), )); } From 4acdd53d122fad75072f1ba143c23aced8f25bac Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 22 Jul 2026 16:14:32 +0000 Subject: [PATCH 5/6] perf(identity): look up one identity directly in remove_identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit remove_identity loaded and deserialized every local qualified identity just to find the one being removed and read its associated voter id. Use the direct get_local_qualified_identity(&id) lookup instead — same extraction, no full-collection scan. Co-Authored-By: Claude Opus --- src/backend_task/identity/remove_identity.rs | 4 +--- 1 file changed, 1 insertion(+), 3 deletions(-) diff --git a/src/backend_task/identity/remove_identity.rs b/src/backend_task/identity/remove_identity.rs index 111d57512..6a43a5678 100644 --- a/src/backend_task/identity/remove_identity.rs +++ b/src/backend_task/identity/remove_identity.rs @@ -9,9 +9,7 @@ impl AppContext { identity_id: Identifier, ) -> Result { let associated_voter_identity_id = self - .load_local_qualified_identities()? - .into_iter() - .find(|identity| identity.identity.id() == identity_id) + .get_local_qualified_identity(&identity_id)? .and_then(|identity| { identity .associated_voter_identity From 2d6abbd34b6c63271d3bcee00ceb13c20e72c72f Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 23 Jul 2026 09:56:05 +0000 Subject: [PATCH 6/6] fix(ui): use fee estimator for identity-list withdraw/transfer thresholds Ports the remaining piece of #622 (fix/integer-max-amount by thepastaclaw) that this branch's PROJ-002 hadn't covered yet: the Withdraw/Transfer button enable/disable thresholds in the identities list's action popup still used hardcoded f64-derived credit constants. Switches them to self.app_context.fee_estimator(), matching the pattern already used in transfer_screen.rs and withdraw_screen.rs on this branch. Also drops the amount from the disabled-button tooltip text since it no longer matches a single hardcoded figure. With this, PR #927 fully supersedes #622, which is being closed. Co-Authored-By: Claude Opus 4.5 --- src/ui/identities/identities_screen.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/src/ui/identities/identities_screen.rs b/src/ui/identities/identities_screen.rs index 8d7583d10..be812f664 100644 --- a/src/ui/identities/identities_screen.rs +++ b/src/ui/identities/identities_screen.rs @@ -676,14 +676,16 @@ impl IdentitiesScreen { .show(|ui| { ui.set_min_width(150.0); - // Minimum balance needed for withdrawal (0.005 DASH fee in credits) - let min_withdrawal_balance: u64 = 500_000_000; // 0.005 DASH in credits + let min_withdrawal_balance = self + .app_context + .fee_estimator() + .estimate_credit_withdrawal(); let can_withdraw = qualified_identity.identity.balance() > min_withdrawal_balance; let withdraw_hover = if can_withdraw { "Withdraw credits from this identity to a Dash Core address" } else { - "Insufficient balance for withdrawal (need at least 0.005 DASH for fees)" + "Insufficient balance for withdrawal fees" }; let width = ui.available_width(); ui.scope(|ui| { @@ -713,14 +715,16 @@ impl IdentitiesScreen { ); } - // Minimum balance needed for transfer (0.0002 DASH fee in credits) - let min_transfer_balance: u64 = 20_000_000; + let min_transfer_balance = self + .app_context + .fee_estimator() + .estimate_credit_transfer(); let can_transfer = qualified_identity.identity.balance() > min_transfer_balance; let transfer_hover = if can_transfer { "Transfer credits from this identity to another identity" } else { - "Insufficient balance for transfer (need at least 0.0002 DASH for fees)" + "Insufficient balance for transfer fees" }; let width = ui.available_width(); ui.scope(|ui| {