Skip to content
Merged
Show file tree
Hide file tree
Changes from 17 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
4e11fe9
fix: show user-friendly error for duplicate identity keys (#714)
lklimek Mar 11, 2026
5a43c1a
fix: correct error messages — duplicate keys are globally unique, not…
lklimek Mar 11, 2026
b9cc1b7
fix: remove incorrect "globally unique" claim from error messages
lklimek Mar 11, 2026
dd88a5d
refactor: return typed TaskError from broadcast_error and add_key_to_…
lklimek Mar 11, 2026
4780643
Merge remote-tracking branch 'origin/v1.0-dev' into fix/714-duplicate…
lklimek Mar 11, 2026
6da02fc
Update src/backend_task/identity/add_key_to_identity.rs
lklimek Mar 11, 2026
d33490e
Merge branch 'v1.0-dev' into fix/714-duplicate-key-error
lklimek Mar 11, 2026
ec5e11d
fix: use explicit Encoding::Base58 for Identifier::to_string() calls
lklimek Mar 11, 2026
05e781a
refactor: preserve typed SdkError in TaskError variants for duplicate…
Copilot Mar 11, 2026
32c703e
refactor: address PR review comments for typed TaskError migration
lklimek Mar 12, 2026
3d07287
refactor: replace BroadcastError with SdkError variant and user-frien…
lklimek Mar 12, 2026
adfacac
docs: add error message tone and form guidelines to CLAUDE.md
lklimek Mar 12, 2026
f89550f
docs: refine error message guidelines — no support redirects, i18n-ready
lklimek Mar 12, 2026
4e775bf
refactor: make error messages i18n-ready and self-contained
lklimek Mar 12, 2026
bdeba53
docs: refine error message guidelines — Base58 IDs allowed, prefer ty…
lklimek Mar 12, 2026
c226c56
refactor: address PR review comments — typed errors and clean user me…
lklimek Mar 12, 2026
072d1b4
fix: add message-based duplicate-key fallback and IdentitySaveError v…
lklimek Mar 12, 2026
b5c0f8c
refactor: preserve source errors in typed TaskError variants
lklimek Mar 12, 2026
b5c8884
refactor: use concrete SdkError type in error variants; drop dyn Error
lklimek Mar 12, 2026
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
16 changes: 15 additions & 1 deletion CLAUDE.md
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,19 @@ scripts/safe-cargo.sh +nightly fmt --all
* When a method takes `&AppContext` (or `Option<&AppContext>`), place it as the first parameter after `self`.
* Screen constructors handle errors internally via `MessageBanner` and return `Self` with degraded state. Keep `create_screen()` clean — no error handling at callsites.

### Error messages

User-facing error messages (shown in `MessageBanner` via `Display`) must follow these rules:

1. **Audience**: Write for the Everyday User persona (`docs/personas/everyday-user.md`). No jargon — no "consensus error", "nonce", "state transition", "SDK", "RPC", or error codes.
2. **Structure**: *What happened* + *what to do*. Every message must include a concrete action the user can take themselves: retry, wait, try a different approach. Never redirect to "contact support" — users must be able to self-resolve.
3. **Tone**: Calm, direct, brief. Not apologetic ("Sorry!"), not alarming ("Something went wrong!"), not vague ("An error occurred").
4. **Technical details**: Never in the message itself — no raw error strings, stack traces, SDK internals, or error codes. Attach via `BannerHandle::with_details(e)` — the `Debug` repr goes to the collapsible details panel and logs. Never refer users to "details" or "details panel" — these are not visible in basic mode. Exception: Base58 identifiers (see rule 7) are not technical details — they are user-meaningful handles.
5. **i18n-ready**: Write messages as simple, complete sentences without interpolation tricks. Avoid concatenating fragments, positional assumptions, or grammar that breaks in other languages. Messages should be straightforward to extract into [Fluent](https://projectfluent.org/) `.ftl` files later — one message ID per string, placeholders only for dynamic values (`{ $seconds }`, `{ $name }`), no logic in the text itself.
6. **Reference implementation**: `sdk_error_user_message()` in `src/backend_task/error.rs` demonstrates the pattern for SDK errors. New `TaskError` variants should follow the same style.
7. **Base58 IDs are allowed in messages**: Contract IDs, identity IDs, document IDs, and similar Base58-encoded identifiers may appear in user-facing messages when they help the user identify which object is involved (e.g., *"This key conflicts with an existing key bound to contract `Abc123…`."*). They are not jargon — they are opaque-but-copyable handles the user can look up.
8. **Prefer granular `TaskError` variants over `Generic`**: When mapping errors to add context, add a dedicated `TaskError` variant with a `#[source]` field rather than converting to `TaskError::Generic(format!(...))`. Granular variants preserve the error chain, enable structural matching by callers, and make `Display` / `Debug` separation explicit. `TaskError::Generic` is a last resort for one-off strings with no upstream error to preserve.

## Architecture Overview

**Dash Evo Tool** is a cross-platform GUI application (Rust + egui) for interacting with Dash Evolution. It enables DPNS username registration, contest voting, state transition viewing, wallet management, and identity operations across Mainnet/Testnet/Devnet.
Expand Down Expand Up @@ -181,11 +194,12 @@ User-facing messages (errors, warnings, success, infos) use `MessageBanner` (`sr

**Logging**: MessageBanner logs all displayed messages (with details) automatically. Additional logging is unnecessary.

**Error banners**: Never expose raw backend/database errors to users. Use a generic user-friendly message in the banner and attach technical details via `BannerHandle::with_details()`. When the error implements `Display` and its text is user-appropriate, pass it directly to `set_global`; otherwise use a descriptive generic message:
**Error banners**: Never expose raw backend/database errors to users. Use a user-friendly message in the banner and attach technical details via `BannerHandle::with_details()`. When the error implements `Display` and its text is user-appropriate, pass it directly to `set_global`; otherwise write a descriptive, actionable message:
```rust
MessageBanner::set_global(ctx, "Failed to load token balances", MessageType::Error)
.with_details(e);
```
Consider whether a repeated or reused message belongs in a dedicated `TaskError` variant instead of being written as a string literal at the callsite. A variant centralises the wording, keeps `Display` / `Debug` separation clean, and makes the error testable. This is a soft guideline — a one-off screen-level message that wraps no upstream error is fine as a literal; errors that originate in backend tasks should generally live in `TaskError`.

## Database

Expand Down
258 changes: 254 additions & 4 deletions src/backend_task/error.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,12 @@
//! `From<String>` → backwards compatible with existing `Result<T, String>` code.
//! Parses known error patterns into typed variants automatically.

use dash_sdk::Error as SdkError;
use dash_sdk::dashcore_rpc;
use dash_sdk::dpp::ProtocolError;
use dash_sdk::dpp::consensus::ConsensusError;
use dash_sdk::dpp::consensus::state::state_error::StateError;
use dash_sdk::dpp::platform_value::string_encoding::Encoding;
use thiserror::Error;

/// Dash Core RPC error code: wallet file not specified (multi-wallet node).
Expand Down Expand Up @@ -46,6 +51,13 @@ pub enum TaskError {
#[error(transparent)]
Sqlite(#[from] rusqlite::Error),

/// Failed to persist an identity update to the local database.
#[error("Could not save identity changes. Check available disk space and retry.")]
IdentitySaveError {
#[source]
source: rusqlite::Error,
},

/// Tokio task join errors.
#[error(transparent)]
JoinError(#[from] tokio::task::JoinError),
Expand All @@ -63,17 +75,101 @@ pub enum TaskError {

/// Duplicate identity public key — the key data already exists on the platform.
#[error("This public key is already registered on the platform. Try a different key.")]
DuplicateIdentityPublicKey,
DuplicateIdentityPublicKey {
/// The original SDK error returned by the broadcast API.
#[source]
source_error: Box<SdkError>,
},

/// Duplicate identity public key ID — the key hash is already taken platform-wide.
#[error("This key hash is already registered on the platform. Try a different key.")]
DuplicateIdentityPublicKeyId,
#[error("This key is already registered on the platform. Try a different key.")]
DuplicateIdentityPublicKeyId {
/// The original SDK error returned by the broadcast API.
#[source]
source_error: Box<SdkError>,
},

/// Identity public key conflicts with an existing key's unique contract bounds.
#[error(
"This key conflicts with an existing key bound to contract {contract_id}. Use a different key or purpose."
)]
IdentityPublicKeyContractBoundsConflict { contract_id: String },
IdentityPublicKeyContractBoundsConflict {
contract_id: String,
/// The original SDK error returned by the broadcast API.
#[source]
source_error: Box<SdkError>,
},
Comment thread
lklimek marked this conversation as resolved.

/// The identity could not be found in the local wallet database.
#[error(
"This identity could not be found in your local wallet. Try refreshing your identities list."
)]
IdentityNotFoundLocally,

/// Unclassified SDK error — the operation failed for an unrecognised reason.
/// Display is implemented manually via [`sdk_error_user_message`] to inspect
/// the source error and produce an actionable, user-friendly message.
#[error("{}", sdk_error_user_message(source_error))]
SdkError {
#[source]
source_error: Box<SdkError>,
},
}

/// Produce a user-friendly message by inspecting the SDK error variant.
///
/// The returned text is shown in `MessageBanner` via `Display`.
/// Technical details remain available through the `#[source]` chain / `Debug`.
///
/// TODO: Expand match arms as we encounter more SDK error variants in the wild.
/// Each arm should explain *what happened* and *what the user can do*.
fn sdk_error_user_message(error: &SdkError) -> String {
match error {
SdkError::StateTransitionBroadcastError(_) => {
// Known broadcast rejection that didn't match a typed consensus variant
// above (DuplicateKey, DuplicateKeyId, ContractBoundsConflict).
// TODO: classify more consensus causes into dedicated TaskError variants
// so fewer errors reach this fallback.
"The platform rejected this request. Please check your input and try again."
.to_string()
}
SdkError::TimeoutReached(duration, _) => {
format!(
"The operation did not complete within {} seconds. Please retry — it often succeeds on the second attempt.",
duration.as_secs()
)
}
SdkError::StaleNode(_) => {
"The server you connected to is behind. Please retry — the app will pick a different server automatically.".to_string()
}
SdkError::DapiClientError(_) => {
// TODO: inspect inner DapiClientError for connection refused vs TLS vs DNS.
"Could not connect to the Dash network. Please retry in a few moments.".to_string()
}
SdkError::NoAvailableAddressesToRetry(_) => {
"All Dash network servers are temporarily unreachable. Please wait a minute and retry.".to_string()
}
SdkError::Cancelled(_) => "The operation was cancelled.".to_string(),
SdkError::AlreadyExists(_) => {
"This object already exists on the platform.".to_string()
}
SdkError::NonceOverflow(_) => {
"This identity has reached its maximum number of operations. Please try again later.".to_string()
}
SdkError::IdentityNonceNotFound(_) => {
"The platform has not indexed this identity yet. Please retry in a few moments.".to_string()
}
// TODO: add arms for Protocol (consensus sub-errors), InvalidCreditTransfer,
// MissingDependency, Config, etc.
_ => {
// TODO(i18n/ux): This fallback embeds the raw SDK error Display string,
// which may contain jargon or technical details. Add dedicated arms for
// remaining SdkError variants (Protocol, InvalidCreditTransfer,
// MissingDependency, Config, etc.) and replace {error} with a fixed,
// user-friendly message once each variant's typical causes are understood.
format!("An unexpected error occurred: {error}. Please try again later.")
Comment thread
lklimek marked this conversation as resolved.
}
}
}

impl From<String> for TaskError {
Expand All @@ -98,9 +194,84 @@ impl From<dashcore_rpc::Error> for TaskError {
}
}

impl From<SdkError> for TaskError {
fn from(error: SdkError) -> Self {
enum ConsensusKind {
DuplicateKey,
DuplicateKeyId,
ContractBoundsConflict(String),
}

let kind: Option<ConsensusKind> = {
let consensus_error = match &error {
SdkError::StateTransitionBroadcastError(broadcast_err) => {
broadcast_err.cause.as_ref()
}
SdkError::Protocol(ProtocolError::ConsensusError(ce)) => Some(ce.as_ref()),
_ => None,
};

consensus_error
.and_then(|ce| match ce {
ConsensusError::StateError(
StateError::DuplicatedIdentityPublicKeyStateError(_),
) => Some(ConsensusKind::DuplicateKey),
ConsensusError::StateError(
StateError::DuplicatedIdentityPublicKeyIdStateError(_),
) => Some(ConsensusKind::DuplicateKeyId),
ConsensusError::StateError(
StateError::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError(e),
) => Some(ConsensusKind::ContractBoundsConflict(
e.contract_id().to_string(Encoding::Base58),
)),
_ => None,
})
.or_else(|| {
// Message-based fallback: when the SDK doesn't provide a structured
// consensus cause, inspect the broadcast message text. This handles
// DAPI responses where cause=None but message carries the
// duplicate-key signal — the exact regression guarded by issue #714.
if let SdkError::StateTransitionBroadcastError(broadcast_err) = &error
&& broadcast_err.cause.is_none()
{
let msg = broadcast_err.message.to_lowercase();
if msg.contains("duplicate") {
return Some(ConsensusKind::DuplicateKey);
}
}
None
})
};

let boxed = Box::new(error);
match kind {
Some(ConsensusKind::DuplicateKey) => TaskError::DuplicateIdentityPublicKey {
source_error: boxed,
},
Some(ConsensusKind::DuplicateKeyId) => TaskError::DuplicateIdentityPublicKeyId {
source_error: boxed,
},
Some(ConsensusKind::ContractBoundsConflict(contract_id)) => {
TaskError::IdentityPublicKeyContractBoundsConflict {
contract_id,
source_error: boxed,
}
}
None => TaskError::SdkError {
source_error: boxed,
},
}
Comment thread
lklimek marked this conversation as resolved.
}
}

#[cfg(test)]
mod tests {
use super::*;
use dash_sdk::dpp::consensus::state::identity::duplicated_identity_public_key_id_state_error::DuplicatedIdentityPublicKeyIdStateError;
use dash_sdk::dpp::consensus::state::identity::duplicated_identity_public_key_state_error::DuplicatedIdentityPublicKeyStateError;
use dash_sdk::dpp::consensus::state::identity::identity_public_key_already_exists_for_unique_contract_bounds_error::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError;
use dash_sdk::dpp::identity::Purpose;
use dash_sdk::platform::Identifier;

#[test]
fn from_string_detects_rpc_error_minus_19() {
Expand Down Expand Up @@ -162,4 +333,83 @@ mod tests {
"Expected Generic, got: {err:?}"
);
}

#[test]
fn from_sdk_error_duplicate_public_key() {
let consensus =
ConsensusError::from(DuplicatedIdentityPublicKeyStateError::new(vec![1, 2]));
let sdk_err = SdkError::from(consensus);
let err = TaskError::from(sdk_err);
assert!(matches!(err, TaskError::DuplicateIdentityPublicKey { .. }));
}
Comment thread
lklimek marked this conversation as resolved.

#[test]
fn from_sdk_error_duplicate_public_key_id() {
let consensus = ConsensusError::from(DuplicatedIdentityPublicKeyIdStateError::new(vec![3]));
let sdk_err = SdkError::from(consensus);
let err = TaskError::from(sdk_err);
assert!(matches!(
err,
TaskError::DuplicateIdentityPublicKeyId { .. }
));
}

#[test]
fn from_sdk_error_contract_bounds_conflict() {
let contract_id = Identifier::random();
let identity_id = Identifier::random();
let consensus = ConsensusError::from(
IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError::new(
identity_id,
contract_id,
Purpose::AUTHENTICATION,
2,
1,
),
);
let sdk_err = SdkError::from(consensus);
let err = TaskError::from(sdk_err);
let expected_contract_id = contract_id.to_string(Encoding::Base58);
assert!(
matches!(err, TaskError::IdentityPublicKeyContractBoundsConflict { ref contract_id, .. } if *contract_id == expected_contract_id)
);
}

#[test]
fn from_sdk_error_broadcast_cause_duplicate_key() {
let consensus = ConsensusError::from(DuplicatedIdentityPublicKeyStateError::new(vec![1]));
let broadcast_err = dash_sdk::error::StateTransitionBroadcastError {
code: 40206,
message: "duplicate key".to_string(),
cause: Some(consensus),
};
let sdk_err = SdkError::StateTransitionBroadcastError(broadcast_err);
let err = TaskError::from(sdk_err);
assert!(matches!(err, TaskError::DuplicateIdentityPublicKey { .. }));
}

#[test]
fn from_sdk_error_generic_variant_falls_back_to_sdk_error() {
let sdk_err = SdkError::Generic("connection timeout".to_string());
let err = TaskError::from(sdk_err);
assert!(
matches!(err, TaskError::SdkError { .. }),
"Expected SdkError, got: {err:?}"
);
Comment thread
lklimek marked this conversation as resolved.
}

#[test]
fn from_sdk_error_broadcast_cause_none_message_duplicate_falls_back_to_duplicate_key() {
let broadcast_err = dash_sdk::error::StateTransitionBroadcastError {
code: 40206,
message: "DuplicateIdentityPublicKeyStateError".to_string(),
cause: None,
};
let sdk_err = SdkError::StateTransitionBroadcastError(broadcast_err);
let err = TaskError::from(sdk_err);
assert!(
matches!(err, TaskError::DuplicateIdentityPublicKey { .. }),
"Expected DuplicateIdentityPublicKey, got: {err:?}"
);
}
}
Loading
Loading