From 4e11fe921072902993d82942aa4d41d94c8a01ae Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 11 Mar 2026 11:40:43 +0100 Subject: [PATCH 01/17] fix: show user-friendly error for duplicate identity keys (#714) Replace raw base64-encoded CBOR error with actionable messages when adding a duplicate key to an identity. Match structured SDK error variants (ConsensusError::StateError) instead of string parsing. - Add TaskError variants: DuplicateIdentityPublicKey, DuplicateIdentityPublicKeyId, IdentityPublicKeyContractBoundsConflict - Add broadcast_error_message() helper matching both SDK error paths (StateTransitionBroadcastError and Protocol/ConsensusError) - 5 unit tests covering all variants + fallback - Manual test scenarios document Co-Authored-By: Claude Opus 4.6 --- .../manual-test-scenarios.md | 177 ++++++++++++++++++ src/backend_task/error.rs | 14 ++ .../identity/add_key_to_identity.rs | 120 +++++++++++- 3 files changed, 310 insertions(+), 1 deletion(-) create mode 100644 docs/ai-design/2026-03-11-duplicate-key-error/manual-test-scenarios.md diff --git a/docs/ai-design/2026-03-11-duplicate-key-error/manual-test-scenarios.md b/docs/ai-design/2026-03-11-duplicate-key-error/manual-test-scenarios.md new file mode 100644 index 000000000..3a7d60e8e --- /dev/null +++ b/docs/ai-design/2026-03-11-duplicate-key-error/manual-test-scenarios.md @@ -0,0 +1,177 @@ +# Manual Test Scenarios: Duplicate Key Error Handling (Issue #714) + +Covers the fix for showing user-friendly error messages when adding a duplicate +key to an identity, instead of raw base64-encoded CBOR. + +--- + +## Prerequisites (all scenarios) + +- Dash Evo Tool running and connected to **Testnet** (or Devnet). +- A funded **wallet** loaded in the application. +- An **identity** registered on the network with at least one existing key + (ECDSA_SECP256K1 AUTHENTICATION MASTER is the default). +- The identity's master private key is available (wallet unlocked or key known). + +--- + +## Scenario 1: Adding a key with the same public key data (DuplicatedIdentityPublicKeyStateError) + +### Goal + +Verify the app displays a clear, actionable message when the user tries to add +a key whose public key data already exists on the identity. + +### Steps + +1. Open **Identities** screen and select the target identity. +2. Click **Keys** to view the identity's existing keys. +3. Note the public key data of an existing key (e.g., copy the hex value of key + ID 0). +4. Click **Add Key**. +5. Set **Purpose** to `AUTHENTICATION`, **Security Level** to `HIGH`, + **Key Type** to `ECDSA_SECP256K1` (or match the existing key's type). +6. In the **Private key (hex)** field, enter the exact same private key that + corresponds to the existing public key on the identity. +7. Click the **Add Key** button to submit. + +### Expected Result + +- An **error banner** appears at the top of the screen with the message: + > This key already exists on this identity. Try adding a different key. +- The message is plain text -- no base64, no CBOR encoding, no raw error dump. +- The banner type is **Error** (red/error styling). +- The app does **not** crash or freeze. +- The **Add Key** form remains accessible so the user can correct the input and + retry. + +--- + +## Scenario 2: Adding a key with a conflicting key ID (DuplicatedIdentityPublicKeyIdStateError) + +### Goal + +Verify the app displays a clear message when a key ID collision occurs. + +### Context + +This error is less likely via the GUI because the app auto-assigns the next +available key ID. It can occur if the identity state is out of sync (e.g., +another client added a key concurrently). To trigger it manually, a modified +client or direct Platform interaction may be needed. This scenario validates the +error display path regardless of trigger. + +### Steps + +1. Open **Identities** screen and select the target identity. +2. Click **Keys**, then **Add Key**. +3. Fill in a valid new key (different key data from existing keys). +4. Arrange for the key ID to collide with an existing key ID (this may require + a race condition where another client adds a key between nonce fetch and + broadcast, or direct Platform API manipulation). +5. Submit the **Add Key** request. + +### Expected Result + +- An **error banner** appears with the message: + > A key with this ID already exists on this identity. Try adding a different key. +- The message is plain text -- no encoded data. +- The banner type is **Error**. +- The app does **not** crash. +- The user can dismiss the banner and retry. + +--- + +## Scenario 3: Adding a key that conflicts with unique contract bounds (IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError) + +### Goal + +Verify the app displays a clear message including the conflicting contract ID +when a key violates unique contract bounds. + +### Steps + +1. Open **Identities** screen and select the target identity. +2. Click **Keys**, then **Add Key**. +3. Check the **Contract bounds** checkbox. +4. Enter a valid **Contract ID** for a contract that enforces unique key bounds. +5. Set the **Purpose** and **Security Level** to match an existing key that is + already bound to the same contract. +6. Generate or enter a new private key. +7. Click the **Add Key** button to submit. + +### Expected Result + +- An **error banner** appears with the message: + > This key conflicts with an existing key bound to contract \. Use a different key or purpose. + + where `` is the actual identifier of the conflicting contract + (not a placeholder, not encoded). +- The message is plain text -- no base64, no CBOR. +- The banner type is **Error**. +- The app does **not** crash. +- The user can change the **Purpose**, **Contract ID**, or key data and retry. + +--- + +## Scenario 4: Fallback for unrecognized broadcast errors + +### Goal + +Verify that broadcast errors not matching the three handled patterns still +display a readable message (prefixed with "Broadcasting error:") rather than +crashing. + +### Steps + +1. Open **Identities** screen and select the target identity. +2. Click **Keys**, then **Add Key**. +3. Trigger a broadcast failure that is NOT a duplicate key error (e.g., + disconnect from the network mid-broadcast, or submit with insufficient + identity balance for the fee). +4. Observe the error display. + +### Expected Result + +- An **error banner** appears with a message starting with: + > Broadcasting error: ... +- The remaining text may be technical but must not contain raw base64 CBOR + blobs. +- The app does **not** crash. +- The user can retry. + +--- + +## Scenario 5: Successful key addition after a duplicate key error + +### Goal + +Verify the app recovers gracefully and allows a successful operation after a +prior duplicate key error. + +### Steps + +1. Reproduce **Scenario 1** to trigger the duplicate key error banner. +2. Observe that the error banner is displayed. +3. Change the **Private key (hex)** field to a new, unique private key. +4. Click the **Add Key** button again. + +### Expected Result + +- The duplicate key error banner is dismissed/replaced. +- A progress/info banner may appear while the transaction broadcasts. +- On success, a **success banner** appears confirming the key was added, + including fee information. +- The new key appears in the identity's key list. +- No residual error state from the previous failed attempt. + +--- + +## Edge Cases + +| Case | Action | Expected | +|------|--------|----------| +| Double-click Add Key | Click Add Key rapidly twice | Only one broadcast attempt; no duplicate submission | +| Wallet locks during broadcast | Lock wallet (if possible) after clicking Add Key | Graceful error, no crash | +| Network disconnect during broadcast | Disable network after clicking Add Key | Timeout or connection error displayed as banner, not raw panic | +| Very long contract ID in error | Trigger Scenario 3 with a valid 256-bit identifier | Full contract ID shown in the message, no truncation | diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs index 435a1d97c..05d89efb1 100644 --- a/src/backend_task/error.rs +++ b/src/backend_task/error.rs @@ -60,6 +60,20 @@ pub enum TaskError { /// Callers should retry the failed operation. #[error("{0}")] MustRetry(String), + + /// Duplicate identity public key -- the key data already exists on this identity. + #[error("This key already exists on this identity. Try adding a different key.")] + DuplicateIdentityPublicKey, + + /// Duplicate identity public key ID -- the key ID is already taken. + #[error("A key with this ID already exists on this identity. Try adding a different key.")] + DuplicateIdentityPublicKeyId, + + /// 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 }, } impl From for TaskError { diff --git a/src/backend_task/identity/add_key_to_identity.rs b/src/backend_task/identity/add_key_to_identity.rs index 117353b94..5351b082a 100644 --- a/src/backend_task/identity/add_key_to_identity.rs +++ b/src/backend_task/identity/add_key_to_identity.rs @@ -1,11 +1,16 @@ use super::BackendTaskSuccessResult; use crate::backend_task::FeeResult; +use crate::backend_task::error::TaskError; use crate::context::AppContext; use crate::model::fee_estimation::PlatformFeeEstimator; use crate::model::qualified_identity::PrivateKeyTarget::PrivateKeyOnMainIdentity; use crate::model::qualified_identity::QualifiedIdentity; use crate::model::qualified_identity::qualified_identity_public_key::QualifiedIdentityPublicKey; +use dash_sdk::Error as SdkError; use dash_sdk::Sdk; +use dash_sdk::dpp::ProtocolError; +use dash_sdk::dpp::consensus::ConsensusError; +use dash_sdk::dpp::consensus::state::state_error::StateError; use dash_sdk::dpp::identity::accessors::{IdentityGettersV0, IdentitySettersV0}; use dash_sdk::dpp::identity::identity_public_key::accessors::v0::{ IdentityPublicKeyGettersV0, IdentityPublicKeySettersV0, @@ -17,6 +22,36 @@ use dash_sdk::dpp::state_transition::proof_result::StateTransitionProofResult; use dash_sdk::platform::transition::broadcast::BroadcastStateTransition; use dash_sdk::platform::{Fetch, Identity}; +fn broadcast_error_message(error: &SdkError) -> String { + let consensus_error = match error { + SdkError::StateTransitionBroadcastError(broadcast_err) => broadcast_err.cause.as_ref(), + SdkError::Protocol(ProtocolError::ConsensusError(ce)) => Some(ce.as_ref()), + _ => None, + }; + + if let Some(ce) = consensus_error { + match ce { + ConsensusError::StateError(StateError::DuplicatedIdentityPublicKeyStateError(_)) => { + return TaskError::DuplicateIdentityPublicKey.to_string(); + } + ConsensusError::StateError(StateError::DuplicatedIdentityPublicKeyIdStateError(_)) => { + return TaskError::DuplicateIdentityPublicKeyId.to_string(); + } + ConsensusError::StateError( + StateError::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError(e), + ) => { + return TaskError::IdentityPublicKeyContractBoundsConflict { + contract_id: format!("{}", e.contract_id()), + } + .to_string(); + } + _ => {} + } + } + + format!("Broadcasting error: {}", error) +} + impl AppContext { pub(super) async fn add_key_to_identity( &self, @@ -69,7 +104,7 @@ impl AppContext { let result = state_transition .broadcast_and_wait(sdk, None) .await - .map_err(|e| format!("Broadcasting error: {}", e))?; + .map_err(|ref e| broadcast_error_message(e))?; // Log and handle the proof result tracing::info!("AddKeyToIdentity proof result: {}", result); @@ -126,3 +161,86 @@ impl AppContext { .map_err(|e| format!("Database error: {}", e)) } } + +#[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::dpp::identifier::Identifier; + + #[test] + fn test_duplicate_public_key_error() { + let consensus = + ConsensusError::from(DuplicatedIdentityPublicKeyStateError::new(vec![1, 2])); + let sdk_err = SdkError::from(consensus); + let msg = broadcast_error_message(&sdk_err); + assert_eq!( + msg, + "This key already exists on this identity. Try adding a different key." + ); + } + + #[test] + fn test_duplicate_public_key_id_error() { + let consensus = ConsensusError::from(DuplicatedIdentityPublicKeyIdStateError::new(vec![3])); + let sdk_err = SdkError::from(consensus); + let msg = broadcast_error_message(&sdk_err); + assert_eq!( + msg, + "A key with this ID already exists on this identity. Try adding a different key." + ); + } + + #[test] + fn test_contract_bounds_conflict_error() { + 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 msg = broadcast_error_message(&sdk_err); + assert_eq!( + msg, + format!( + "This key conflicts with an existing key bound to contract {}. Use a different key or purpose.", + contract_id + ) + ); + } + + #[test] + fn test_broadcast_error_with_cause_duplicate_key() { + // Test the StateTransitionBroadcastError path (the actual production path + // when Platform returns a structured broadcast error with a consensus cause). + 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 msg = broadcast_error_message(&sdk_err); + assert_eq!( + msg, + "This key already exists on this identity. Try adding a different key." + ); + } + + #[test] + fn test_unknown_sdk_error_falls_back() { + let sdk_err = SdkError::Generic("connection timeout".to_string()); + let msg = broadcast_error_message(&sdk_err); + assert!(msg.starts_with("Broadcasting error:")); + assert!(msg.contains("connection timeout")); + } +} From 5a43c1a17c4acf07ecf09401b4db7f43643a5d5b Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 11 Mar 2026 11:48:50 +0100 Subject: [PATCH 02/17] =?UTF-8?q?fix:=20correct=20error=20messages=20?= =?UTF-8?q?=E2=80=94=20duplicate=20keys=20are=20globally=20unique,=20not?= =?UTF-8?q?=20per-identity?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit DuplicatedIdentityPublicKeyStateError and DuplicatedIdentityPublicKeyIdStateError are platform-wide uniqueness constraints, not per-identity. Updated error messages to reflect that keys must be globally unique across all identities. Co-Authored-By: Claude Opus 4.6 --- .../manual-test-scenarios.md | 4 ++-- src/backend_task/error.rs | 12 ++++++++---- src/backend_task/identity/add_key_to_identity.rs | 6 +++--- 3 files changed, 13 insertions(+), 9 deletions(-) diff --git a/docs/ai-design/2026-03-11-duplicate-key-error/manual-test-scenarios.md b/docs/ai-design/2026-03-11-duplicate-key-error/manual-test-scenarios.md index 3a7d60e8e..9bcb5f36c 100644 --- a/docs/ai-design/2026-03-11-duplicate-key-error/manual-test-scenarios.md +++ b/docs/ai-design/2026-03-11-duplicate-key-error/manual-test-scenarios.md @@ -38,7 +38,7 @@ a key whose public key data already exists on the identity. ### Expected Result - An **error banner** appears at the top of the screen with the message: - > This key already exists on this identity. Try adding a different key. + > This public key is already registered on the platform. Each key must be globally unique — try a different key. - The message is plain text -- no base64, no CBOR encoding, no raw error dump. - The banner type is **Error** (red/error styling). - The app does **not** crash or freeze. @@ -74,7 +74,7 @@ error display path regardless of trigger. ### Expected Result - An **error banner** appears with the message: - > A key with this ID already exists on this identity. Try adding a different key. + > This key hash is already registered on the platform. Each key must be globally unique — try a different key. - The message is plain text -- no encoded data. - The banner type is **Error**. - The app does **not** crash. diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs index 05d89efb1..1b52ca93b 100644 --- a/src/backend_task/error.rs +++ b/src/backend_task/error.rs @@ -61,12 +61,16 @@ pub enum TaskError { #[error("{0}")] MustRetry(String), - /// Duplicate identity public key -- the key data already exists on this identity. - #[error("This key already exists on this identity. Try adding a different key.")] + /// Duplicate identity public key — the key data already exists on the platform. + #[error( + "This public key is already registered on the platform. Each key must be globally unique — try a different key." + )] DuplicateIdentityPublicKey, - /// Duplicate identity public key ID -- the key ID is already taken. - #[error("A key with this ID already exists on this identity. Try adding a different key.")] + /// Duplicate identity public key ID — the key hash is already taken platform-wide. + #[error( + "This key hash is already registered on the platform. Each key must be globally unique — try a different key." + )] DuplicateIdentityPublicKeyId, /// Identity public key conflicts with an existing key's unique contract bounds. diff --git a/src/backend_task/identity/add_key_to_identity.rs b/src/backend_task/identity/add_key_to_identity.rs index 5351b082a..2679d404d 100644 --- a/src/backend_task/identity/add_key_to_identity.rs +++ b/src/backend_task/identity/add_key_to_identity.rs @@ -179,7 +179,7 @@ mod tests { let msg = broadcast_error_message(&sdk_err); assert_eq!( msg, - "This key already exists on this identity. Try adding a different key." + "This public key is already registered on the platform. Each key must be globally unique — try a different key." ); } @@ -190,7 +190,7 @@ mod tests { let msg = broadcast_error_message(&sdk_err); assert_eq!( msg, - "A key with this ID already exists on this identity. Try adding a different key." + "This key hash is already registered on the platform. Each key must be globally unique — try a different key." ); } @@ -232,7 +232,7 @@ mod tests { let msg = broadcast_error_message(&sdk_err); assert_eq!( msg, - "This key already exists on this identity. Try adding a different key." + "This public key is already registered on the platform. Each key must be globally unique — try a different key." ); } From b9cc1b721ff9874136f81cade4e17492f590843d Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 11 Mar 2026 11:55:54 +0100 Subject: [PATCH 03/17] fix: remove incorrect "globally unique" claim from error messages Only unique key types (ECDSA_SECP256K1, BLS12_381) require platform-wide uniqueness. Non-unique types (ECDSA_HASH160, BIP13_SCRIPT_HASH, EDDSA_25519_HASH160) can be shared across identities. Simplified messages to avoid misleading users about key uniqueness rules. Co-Authored-By: Claude Opus 4.6 --- .../manual-test-scenarios.md | 4 ++-- src/backend_task/error.rs | 8 ++------ src/backend_task/identity/add_key_to_identity.rs | 6 +++--- 3 files changed, 7 insertions(+), 11 deletions(-) diff --git a/docs/ai-design/2026-03-11-duplicate-key-error/manual-test-scenarios.md b/docs/ai-design/2026-03-11-duplicate-key-error/manual-test-scenarios.md index 9bcb5f36c..639d6f7e0 100644 --- a/docs/ai-design/2026-03-11-duplicate-key-error/manual-test-scenarios.md +++ b/docs/ai-design/2026-03-11-duplicate-key-error/manual-test-scenarios.md @@ -38,7 +38,7 @@ a key whose public key data already exists on the identity. ### Expected Result - An **error banner** appears at the top of the screen with the message: - > This public key is already registered on the platform. Each key must be globally unique — try a different key. + > This public key is already registered on the platform. Try a different key. - The message is plain text -- no base64, no CBOR encoding, no raw error dump. - The banner type is **Error** (red/error styling). - The app does **not** crash or freeze. @@ -74,7 +74,7 @@ error display path regardless of trigger. ### Expected Result - An **error banner** appears with the message: - > This key hash is already registered on the platform. Each key must be globally unique — try a different key. + > This key hash is already registered on the platform. Try a different key. - The message is plain text -- no encoded data. - The banner type is **Error**. - The app does **not** crash. diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs index 1b52ca93b..b05392723 100644 --- a/src/backend_task/error.rs +++ b/src/backend_task/error.rs @@ -62,15 +62,11 @@ pub enum TaskError { MustRetry(String), /// Duplicate identity public key — the key data already exists on the platform. - #[error( - "This public key is already registered on the platform. Each key must be globally unique — try a different key." - )] + #[error("This public key is already registered on the platform. Try a different key.")] DuplicateIdentityPublicKey, /// Duplicate identity public key ID — the key hash is already taken platform-wide. - #[error( - "This key hash is already registered on the platform. Each key must be globally unique — try a different key." - )] + #[error("This key hash is already registered on the platform. Try a different key.")] DuplicateIdentityPublicKeyId, /// Identity public key conflicts with an existing key's unique contract bounds. diff --git a/src/backend_task/identity/add_key_to_identity.rs b/src/backend_task/identity/add_key_to_identity.rs index 2679d404d..59f756c3b 100644 --- a/src/backend_task/identity/add_key_to_identity.rs +++ b/src/backend_task/identity/add_key_to_identity.rs @@ -179,7 +179,7 @@ mod tests { let msg = broadcast_error_message(&sdk_err); assert_eq!( msg, - "This public key is already registered on the platform. Each key must be globally unique — try a different key." + "This public key is already registered on the platform. Try a different key." ); } @@ -190,7 +190,7 @@ mod tests { let msg = broadcast_error_message(&sdk_err); assert_eq!( msg, - "This key hash is already registered on the platform. Each key must be globally unique — try a different key." + "This key hash is already registered on the platform. Try a different key." ); } @@ -232,7 +232,7 @@ mod tests { let msg = broadcast_error_message(&sdk_err); assert_eq!( msg, - "This public key is already registered on the platform. Each key must be globally unique — try a different key." + "This public key is already registered on the platform. Try a different key." ); } From dd88a5d8db893ee9cc861d19430f84fff74a3510 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 11 Mar 2026 14:02:48 +0100 Subject: [PATCH 04/17] refactor: return typed TaskError from broadcast_error and add_key_to_identity MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - broadcast_error() now returns TaskError directly instead of String - add_key_to_identity() returns Result<..., TaskError> — typed variants flow through without String round-trip - run_identity_task() returns Result<..., TaskError> — other arms auto-convert via From Co-Authored-By: Claude Opus 4.6 --- .../identity/add_key_to_identity.rs | 64 +++++++---------- src/backend_task/identity/mod.rs | 72 +++++++++---------- 2 files changed, 60 insertions(+), 76 deletions(-) diff --git a/src/backend_task/identity/add_key_to_identity.rs b/src/backend_task/identity/add_key_to_identity.rs index 59f756c3b..2bc01331e 100644 --- a/src/backend_task/identity/add_key_to_identity.rs +++ b/src/backend_task/identity/add_key_to_identity.rs @@ -22,7 +22,12 @@ use dash_sdk::dpp::state_transition::proof_result::StateTransitionProofResult; use dash_sdk::platform::transition::broadcast::BroadcastStateTransition; use dash_sdk::platform::{Fetch, Identity}; -fn broadcast_error_message(error: &SdkError) -> String { +/// Converts a broadcast SDK error into a typed `TaskError`. +/// +/// Matches known consensus error variants from two SDK error paths +/// (`StateTransitionBroadcastError` and `Protocol/ConsensusError`), +/// falling back to `TaskError::Generic` for unrecognised errors. +fn broadcast_error(error: &SdkError) -> TaskError { let consensus_error = match error { SdkError::StateTransitionBroadcastError(broadcast_err) => broadcast_err.cause.as_ref(), SdkError::Protocol(ProtocolError::ConsensusError(ce)) => Some(ce.as_ref()), @@ -32,24 +37,23 @@ fn broadcast_error_message(error: &SdkError) -> String { if let Some(ce) = consensus_error { match ce { ConsensusError::StateError(StateError::DuplicatedIdentityPublicKeyStateError(_)) => { - return TaskError::DuplicateIdentityPublicKey.to_string(); + return TaskError::DuplicateIdentityPublicKey; } ConsensusError::StateError(StateError::DuplicatedIdentityPublicKeyIdStateError(_)) => { - return TaskError::DuplicateIdentityPublicKeyId.to_string(); + return TaskError::DuplicateIdentityPublicKeyId; } ConsensusError::StateError( StateError::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError(e), ) => { return TaskError::IdentityPublicKeyContractBoundsConflict { contract_id: format!("{}", e.contract_id()), - } - .to_string(); + }; } _ => {} } } - format!("Broadcasting error: {}", error) + TaskError::Generic(format!("Broadcasting error: {}", error)) } impl AppContext { @@ -59,19 +63,19 @@ impl AppContext { mut qualified_identity: QualifiedIdentity, mut public_key_to_add: QualifiedIdentityPublicKey, private_key: [u8; 32], - ) -> Result { + ) -> Result { let new_identity_nonce = sdk .get_identity_nonce(qualified_identity.identity.id(), true, None) .await .map_err(|e| format!("Fetch nonce error: {}", e))?; let Some(master_key) = qualified_identity.can_sign_with_master_key() else { - return Err("Master key not found".to_string()); + return Err("Master key not found".to_string().into()); }; let master_key_id = master_key.identity_public_key.id(); let identity = Identity::fetch_by_identifier(sdk, qualified_identity.identity.id()) .await - .map_err(|e| format!("Fetch nonce error: {}", e))? - .ok_or("Identity not found".to_string())?; + .map_err(|e| format!("Fetch identity error: {}", e))? + .ok_or_else(|| TaskError::Generic("Identity not found".into()))?; qualified_identity.identity = identity; qualified_identity.identity.bump_revision(); public_key_to_add @@ -104,7 +108,7 @@ impl AppContext { let result = state_transition .broadcast_and_wait(sdk, None) .await - .map_err(|ref e| broadcast_error_message(e))?; + .map_err(|ref e| broadcast_error(e))?; // Log and handle the proof result tracing::info!("AddKeyToIdentity proof result: {}", result); @@ -158,7 +162,7 @@ impl AppContext { self.update_local_qualified_identity(&qualified_identity) .map(|_| BackendTaskSuccessResult::AddedKeyToIdentity(fee_result)) - .map_err(|e| format!("Database error: {}", e)) + .map_err(|e| TaskError::Generic(format!("Database error: {}", e))) } } @@ -176,22 +180,16 @@ mod tests { let consensus = ConsensusError::from(DuplicatedIdentityPublicKeyStateError::new(vec![1, 2])); let sdk_err = SdkError::from(consensus); - let msg = broadcast_error_message(&sdk_err); - assert_eq!( - msg, - "This public key is already registered on the platform. Try a different key." - ); + let err = broadcast_error(&sdk_err); + assert!(matches!(err, TaskError::DuplicateIdentityPublicKey)); } #[test] fn test_duplicate_public_key_id_error() { let consensus = ConsensusError::from(DuplicatedIdentityPublicKeyIdStateError::new(vec![3])); let sdk_err = SdkError::from(consensus); - let msg = broadcast_error_message(&sdk_err); - assert_eq!( - msg, - "This key hash is already registered on the platform. Try a different key." - ); + let err = broadcast_error(&sdk_err); + assert!(matches!(err, TaskError::DuplicateIdentityPublicKeyId)); } #[test] @@ -208,13 +206,9 @@ mod tests { ), ); let sdk_err = SdkError::from(consensus); - let msg = broadcast_error_message(&sdk_err); - assert_eq!( - msg, - format!( - "This key conflicts with an existing key bound to contract {}. Use a different key or purpose.", - contract_id - ) + let err = broadcast_error(&sdk_err); + assert!( + matches!(err, TaskError::IdentityPublicKeyContractBoundsConflict { ref contract_id } if !contract_id.is_empty()) ); } @@ -229,18 +223,14 @@ mod tests { cause: Some(consensus), }; let sdk_err = SdkError::StateTransitionBroadcastError(broadcast_err); - let msg = broadcast_error_message(&sdk_err); - assert_eq!( - msg, - "This public key is already registered on the platform. Try a different key." - ); + let err = broadcast_error(&sdk_err); + assert!(matches!(err, TaskError::DuplicateIdentityPublicKey)); } #[test] fn test_unknown_sdk_error_falls_back() { let sdk_err = SdkError::Generic("connection timeout".to_string()); - let msg = broadcast_error_message(&sdk_err); - assert!(msg.starts_with("Broadcasting error:")); - assert!(msg.contains("connection timeout")); + let err = broadcast_error(&sdk_err); + assert!(matches!(err, TaskError::Generic(ref s) if s.contains("connection timeout"))); } } diff --git a/src/backend_task/identity/mod.rs b/src/backend_task/identity/mod.rs index 4fc440a22..4c505198f 100644 --- a/src/backend_task/identity/mod.rs +++ b/src/backend_task/identity/mod.rs @@ -11,7 +11,7 @@ mod top_up_identity; mod transfer; mod withdraw_from_identity; -use super::{BackendTaskSuccessResult, FeeResult}; +use super::{BackendTaskSuccessResult, FeeResult, TaskError}; use crate::app::TaskResult; use crate::context::AppContext; use crate::model::qualified_identity::encrypted_key_storage::{KeyStorage, WalletDerivationPath}; @@ -530,65 +530,59 @@ impl AppContext { task: IdentityTask, sdk: &Sdk, sender: crate::utils::egui_mpsc::SenderAsync, - ) -> Result { + ) -> Result { match task { - IdentityTask::LoadIdentity(input) => self.load_identity(sdk, input).await, + IdentityTask::LoadIdentity(input) => Ok(self.load_identity(sdk, input).await?), IdentityTask::WithdrawFromIdentity(qualified_identity, to_address, credits, id) => { - self.withdraw_from_identity(qualified_identity, to_address, credits, id) - .await + Ok(self + .withdraw_from_identity(qualified_identity, to_address, credits, id) + .await?) } IdentityTask::AddKeyToIdentity(qualified_identity, public_key_to_add, private_key) => { self.add_key_to_identity(sdk, qualified_identity, public_key_to_add, private_key) .await } IdentityTask::RegisterIdentity(registration_info) => { - self.register_identity(registration_info).await + Ok(self.register_identity(registration_info).await?) } - IdentityTask::RegisterDpnsName(input) => self.register_dpns_name(sdk, input).await, - IdentityTask::RefreshIdentity(qualified_identity) => self + IdentityTask::RegisterDpnsName(input) => { + Ok(self.register_dpns_name(sdk, input).await?) + } + IdentityTask::RefreshIdentity(qualified_identity) => Ok(self .refresh_identity(sdk, qualified_identity, sender) .await - .map_err(|e| format!("Error refreshing identity: {}", e)), - IdentityTask::Transfer(qualified_identity, to_identifier, credits, id) => { - self.transfer_to_identity(qualified_identity, to_identifier, credits, id) - .await - } - IdentityTask::SearchIdentityFromWallet(wallet, identity_index) => { - self.load_user_identity_from_wallet(sdk, wallet, identity_index, sender) - .await - } - IdentityTask::SearchIdentitiesUpToIndex(wallet, max_identity_index) => { - self.load_user_identities_up_to_index(sdk, wallet, max_identity_index, sender) - .await + .map_err(|e| format!("Error refreshing identity: {}", e))?), + IdentityTask::Transfer(qualified_identity, to_identifier, credits, id) => Ok(self + .transfer_to_identity(qualified_identity, to_identifier, credits, id) + .await?), + IdentityTask::SearchIdentityFromWallet(wallet, identity_index) => Ok(self + .load_user_identity_from_wallet(sdk, wallet, identity_index, sender) + .await?), + IdentityTask::SearchIdentitiesUpToIndex(wallet, max_identity_index) => Ok(self + .load_user_identities_up_to_index(sdk, wallet, max_identity_index, sender) + .await?), + IdentityTask::SearchIdentityByDpnsName(dpns_name, wallet_seed_hash) => Ok(self + .load_identity_by_dpns_name(sdk, dpns_name, wallet_seed_hash) + .await?), + IdentityTask::TopUpIdentity(top_up_info) => { + Ok(self.top_up_identity(top_up_info).await?) } - IdentityTask::SearchIdentityByDpnsName(dpns_name, wallet_seed_hash) => { - self.load_identity_by_dpns_name(sdk, dpns_name, wallet_seed_hash) - .await - } - IdentityTask::TopUpIdentity(top_up_info) => self.top_up_identity(top_up_info).await, IdentityTask::TopUpIdentityFromPlatformAddresses { identity, inputs, wallet_seed_hash, - } => { - self.top_up_identity_from_platform_addresses( - sdk, - identity, - inputs, - wallet_seed_hash, - ) - .await - } + } => Ok(self + .top_up_identity_from_platform_addresses(sdk, identity, inputs, wallet_seed_hash) + .await?), IdentityTask::TransferToAddresses { identity, outputs, key_id, - } => { - self.transfer_to_addresses(sdk, identity, outputs, key_id) - .await - } + } => Ok(self + .transfer_to_addresses(sdk, identity, outputs, key_id) + .await?), IdentityTask::RefreshLoadedIdentitiesOwnedDPNSNames => { - self.refresh_loaded_identities_dpns_names(sender).await + Ok(self.refresh_loaded_identities_dpns_names(sender).await?) } } } From 6da02fc924915c766e258d07371cd631cb4d22b6 Mon Sep 17 00:00:00 2001 From: lklimek <842586+lklimek@users.noreply.github.com> Date: Wed, 11 Mar 2026 15:45:26 +0100 Subject: [PATCH 05/17] Update src/backend_task/identity/add_key_to_identity.rs Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com> --- src/backend_task/identity/add_key_to_identity.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/src/backend_task/identity/add_key_to_identity.rs b/src/backend_task/identity/add_key_to_identity.rs index 2bc01331e..bdee8ca12 100644 --- a/src/backend_task/identity/add_key_to_identity.rs +++ b/src/backend_task/identity/add_key_to_identity.rs @@ -207,8 +207,9 @@ mod tests { ); let sdk_err = SdkError::from(consensus); let err = broadcast_error(&sdk_err); + let expected_contract_id = contract_id.to_string(); assert!( - matches!(err, TaskError::IdentityPublicKeyContractBoundsConflict { ref contract_id } if !contract_id.is_empty()) + matches!(err, TaskError::IdentityPublicKeyContractBoundsConflict { ref contract_id } if *contract_id == expected_contract_id) ); } From ec5e11d32bbdbab7d56637e5f31fc92a68d5bef1 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Wed, 11 Mar 2026 16:15:26 +0100 Subject: [PATCH 06/17] fix: use explicit Encoding::Base58 for Identifier::to_string() calls The Platform SDK changed Identifier::to_string() to require an explicit Encoding argument. Updated both production and test code to use to_string(Encoding::Base58) instead of the no-arg variant. Co-Authored-By: Claude Opus 4.6 --- src/backend_task/identity/add_key_to_identity.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/backend_task/identity/add_key_to_identity.rs b/src/backend_task/identity/add_key_to_identity.rs index bdee8ca12..b231e67e8 100644 --- a/src/backend_task/identity/add_key_to_identity.rs +++ b/src/backend_task/identity/add_key_to_identity.rs @@ -15,6 +15,7 @@ use dash_sdk::dpp::identity::accessors::{IdentityGettersV0, IdentitySettersV0}; use dash_sdk::dpp::identity::identity_public_key::accessors::v0::{ IdentityPublicKeyGettersV0, IdentityPublicKeySettersV0, }; +use dash_sdk::dpp::platform_value::string_encoding::Encoding; use dash_sdk::dpp::prelude::UserFeeIncrease; use dash_sdk::dpp::state_transition::identity_update_transition::IdentityUpdateTransition; use dash_sdk::dpp::state_transition::identity_update_transition::methods::IdentityUpdateTransitionMethodsV0; @@ -46,7 +47,7 @@ fn broadcast_error(error: &SdkError) -> TaskError { StateError::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError(e), ) => { return TaskError::IdentityPublicKeyContractBoundsConflict { - contract_id: format!("{}", e.contract_id()), + contract_id: e.contract_id().to_string(Encoding::Base58), }; } _ => {} @@ -207,7 +208,7 @@ mod tests { ); let sdk_err = SdkError::from(consensus); let err = broadcast_error(&sdk_err); - let expected_contract_id = contract_id.to_string(); + let expected_contract_id = contract_id.to_string(Encoding::Base58); assert!( matches!(err, TaskError::IdentityPublicKeyContractBoundsConflict { ref contract_id } if *contract_id == expected_contract_id) ); From 05e781abc78c872e8c4bb2095f125b717a9312ae Mon Sep 17 00:00:00 2001 From: Copilot <198982749+Copilot@users.noreply.github.com> Date: Wed, 11 Mar 2026 22:58:45 +0100 Subject: [PATCH 07/17] refactor: preserve typed SdkError in TaskError variants for duplicate-key errors (#731) * Initial plan * fix: log raw SdkError in broadcast_error to preserve technical context Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: embed raw SdkError as source_error field in typed TaskError variants Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * refactor: store Box as typed source in TaskError variants instead of debug string Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> * style: fix rustfmt formatting issues in error.rs and add_key_to_identity.rs Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> --------- Co-authored-by: copilot-swe-agent[bot] <198982749+Copilot@users.noreply.github.com> Co-authored-by: lklimek <842586+lklimek@users.noreply.github.com> --- src/backend_task/error.rs | 20 ++++- .../identity/add_key_to_identity.rs | 79 ++++++++++++------- 2 files changed, 69 insertions(+), 30 deletions(-) diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs index b05392723..5a35815a0 100644 --- a/src/backend_task/error.rs +++ b/src/backend_task/error.rs @@ -5,6 +5,7 @@ //! `From` → backwards compatible with existing `Result` code. //! Parses known error patterns into typed variants automatically. +use dash_sdk::Error as SdkError; use dash_sdk::dashcore_rpc; use thiserror::Error; @@ -63,17 +64,30 @@ 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, + }, /// 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, + DuplicateIdentityPublicKeyId { + /// The original SDK error returned by the broadcast API. + #[source] + source_error: Box, + }, /// 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, + }, } impl From for TaskError { diff --git a/src/backend_task/identity/add_key_to_identity.rs b/src/backend_task/identity/add_key_to_identity.rs index b231e67e8..27ce719c6 100644 --- a/src/backend_task/identity/add_key_to_identity.rs +++ b/src/backend_task/identity/add_key_to_identity.rs @@ -28,33 +28,55 @@ use dash_sdk::platform::{Fetch, Identity}; /// Matches known consensus error variants from two SDK error paths /// (`StateTransitionBroadcastError` and `Protocol/ConsensusError`), /// falling back to `TaskError::Generic` for unrecognised errors. -fn broadcast_error(error: &SdkError) -> TaskError { - let consensus_error = match error { - SdkError::StateTransitionBroadcastError(broadcast_err) => broadcast_err.cause.as_ref(), - SdkError::Protocol(ProtocolError::ConsensusError(ce)) => Some(ce.as_ref()), - _ => None, - }; +/// The original `SdkError` is preserved as a typed field on each variant. +fn broadcast_error(error: SdkError) -> TaskError { + // Classify the error while borrowing; the borrow is dropped before `error` + // is moved into the returned variant. + enum ConsensusKind { + DuplicateKey, + DuplicateKeyId, + ContractBoundsConflict(String), + } + + let kind: Option = { + let consensus_error = match &error { + SdkError::StateTransitionBroadcastError(broadcast_err) => broadcast_err.cause.as_ref(), + SdkError::Protocol(ProtocolError::ConsensusError(ce)) => Some(ce.as_ref()), + _ => None, + }; - if let Some(ce) = consensus_error { - match ce { + consensus_error.and_then(|ce| match ce { ConsensusError::StateError(StateError::DuplicatedIdentityPublicKeyStateError(_)) => { - return TaskError::DuplicateIdentityPublicKey; + Some(ConsensusKind::DuplicateKey) } ConsensusError::StateError(StateError::DuplicatedIdentityPublicKeyIdStateError(_)) => { - return TaskError::DuplicateIdentityPublicKeyId; + Some(ConsensusKind::DuplicateKeyId) } ConsensusError::StateError( StateError::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError(e), - ) => { - return TaskError::IdentityPublicKeyContractBoundsConflict { - contract_id: e.contract_id().to_string(Encoding::Base58), - }; + ) => Some(ConsensusKind::ContractBoundsConflict( + e.contract_id().to_string(Encoding::Base58), + )), + _ => 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::Generic(format!("Broadcasting error: {boxed}")), } - - TaskError::Generic(format!("Broadcasting error: {}", error)) } impl AppContext { @@ -109,7 +131,7 @@ impl AppContext { let result = state_transition .broadcast_and_wait(sdk, None) .await - .map_err(|ref e| broadcast_error(e))?; + .map_err(broadcast_error)?; // Log and handle the proof result tracing::info!("AddKeyToIdentity proof result: {}", result); @@ -181,16 +203,19 @@ mod tests { let consensus = ConsensusError::from(DuplicatedIdentityPublicKeyStateError::new(vec![1, 2])); let sdk_err = SdkError::from(consensus); - let err = broadcast_error(&sdk_err); - assert!(matches!(err, TaskError::DuplicateIdentityPublicKey)); + let err = broadcast_error(sdk_err); + assert!(matches!(err, TaskError::DuplicateIdentityPublicKey { .. })); } #[test] fn test_duplicate_public_key_id_error() { let consensus = ConsensusError::from(DuplicatedIdentityPublicKeyIdStateError::new(vec![3])); let sdk_err = SdkError::from(consensus); - let err = broadcast_error(&sdk_err); - assert!(matches!(err, TaskError::DuplicateIdentityPublicKeyId)); + let err = broadcast_error(sdk_err); + assert!(matches!( + err, + TaskError::DuplicateIdentityPublicKeyId { .. } + )); } #[test] @@ -207,10 +232,10 @@ mod tests { ), ); let sdk_err = SdkError::from(consensus); - let err = broadcast_error(&sdk_err); + let err = broadcast_error(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) + matches!(err, TaskError::IdentityPublicKeyContractBoundsConflict { ref contract_id, .. } if *contract_id == expected_contract_id) ); } @@ -225,14 +250,14 @@ mod tests { cause: Some(consensus), }; let sdk_err = SdkError::StateTransitionBroadcastError(broadcast_err); - let err = broadcast_error(&sdk_err); - assert!(matches!(err, TaskError::DuplicateIdentityPublicKey)); + let err = broadcast_error(sdk_err); + assert!(matches!(err, TaskError::DuplicateIdentityPublicKey { .. })); } #[test] fn test_unknown_sdk_error_falls_back() { let sdk_err = SdkError::Generic("connection timeout".to_string()); - let err = broadcast_error(&sdk_err); + let err = broadcast_error(sdk_err); assert!(matches!(err, TaskError::Generic(ref s) if s.contains("connection timeout"))); } } From 32c703e83bcc8ff9f1244cc6e16f4b7c5663f81d Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 12 Mar 2026 08:10:54 +0100 Subject: [PATCH 08/17] refactor: address PR review comments for typed TaskError migration MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Convert standalone `broadcast_error()` fn to `impl From for TaskError` (per @lklimek review), moving logic and tests to `error.rs` - Add `BroadcastError` variant with user-friendly Display message instead of leaking raw SDK text via `TaskError::Generic` (per CodeRabbit review) - Change `refresh_identity`, `top_up_identity_from_platform_addresses`, and `transfer_to_addresses` to return `Result<_, TaskError>` instead of `Result<_, String>` (per CodeRabbit review) - Simplify callers in `run_identity_task` — direct `.await` without unnecessary `Ok(self.foo().await?)` wrapping Co-Authored-By: Claude Opus 4.6 --- src/backend_task/error.rs | 134 +++++++++++++++++ .../identity/add_key_to_identity.rs | 139 +----------------- src/backend_task/identity/mod.rs | 58 +++++--- src/backend_task/identity/refresh_identity.rs | 19 +-- 4 files changed, 180 insertions(+), 170 deletions(-) diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs index 5a35815a0..516051d72 100644 --- a/src/backend_task/error.rs +++ b/src/backend_task/error.rs @@ -7,6 +7,10 @@ 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). @@ -88,6 +92,13 @@ pub enum TaskError { #[source] source_error: Box, }, + + /// Unclassified broadcast error — the state transition failed for an unknown reason. + #[error("Failed to broadcast identity update. Please try again later.")] + BroadcastError { + #[source] + source_error: Box, + }, } impl From for TaskError { @@ -112,9 +123,68 @@ impl From for TaskError { } } +impl From for TaskError { + fn from(error: SdkError) -> Self { + enum ConsensusKind { + DuplicateKey, + DuplicateKeyId, + ContractBoundsConflict(String), + } + + let kind: Option = { + 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, + }) + }; + + 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::BroadcastError { + source_error: boxed, + }, + } + } +} + #[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() { @@ -176,4 +246,68 @@ 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 { .. })); + } + + #[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_unknown_falls_back_to_broadcast_error() { + let sdk_err = SdkError::Generic("connection timeout".to_string()); + let err = TaskError::from(sdk_err); + assert!( + matches!(err, TaskError::BroadcastError { .. }), + "Expected BroadcastError, got: {err:?}" + ); + } } diff --git a/src/backend_task/identity/add_key_to_identity.rs b/src/backend_task/identity/add_key_to_identity.rs index 27ce719c6..2100efa79 100644 --- a/src/backend_task/identity/add_key_to_identity.rs +++ b/src/backend_task/identity/add_key_to_identity.rs @@ -6,16 +6,11 @@ use crate::model::fee_estimation::PlatformFeeEstimator; use crate::model::qualified_identity::PrivateKeyTarget::PrivateKeyOnMainIdentity; use crate::model::qualified_identity::QualifiedIdentity; use crate::model::qualified_identity::qualified_identity_public_key::QualifiedIdentityPublicKey; -use dash_sdk::Error as SdkError; use dash_sdk::Sdk; -use dash_sdk::dpp::ProtocolError; -use dash_sdk::dpp::consensus::ConsensusError; -use dash_sdk::dpp::consensus::state::state_error::StateError; use dash_sdk::dpp::identity::accessors::{IdentityGettersV0, IdentitySettersV0}; use dash_sdk::dpp::identity::identity_public_key::accessors::v0::{ IdentityPublicKeyGettersV0, IdentityPublicKeySettersV0, }; -use dash_sdk::dpp::platform_value::string_encoding::Encoding; use dash_sdk::dpp::prelude::UserFeeIncrease; use dash_sdk::dpp::state_transition::identity_update_transition::IdentityUpdateTransition; use dash_sdk::dpp::state_transition::identity_update_transition::methods::IdentityUpdateTransitionMethodsV0; @@ -23,62 +18,6 @@ use dash_sdk::dpp::state_transition::proof_result::StateTransitionProofResult; use dash_sdk::platform::transition::broadcast::BroadcastStateTransition; use dash_sdk::platform::{Fetch, Identity}; -/// Converts a broadcast SDK error into a typed `TaskError`. -/// -/// Matches known consensus error variants from two SDK error paths -/// (`StateTransitionBroadcastError` and `Protocol/ConsensusError`), -/// falling back to `TaskError::Generic` for unrecognised errors. -/// The original `SdkError` is preserved as a typed field on each variant. -fn broadcast_error(error: SdkError) -> TaskError { - // Classify the error while borrowing; the borrow is dropped before `error` - // is moved into the returned variant. - enum ConsensusKind { - DuplicateKey, - DuplicateKeyId, - ContractBoundsConflict(String), - } - - let kind: Option = { - 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, - }) - }; - - 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::Generic(format!("Broadcasting error: {boxed}")), - } -} - impl AppContext { pub(super) async fn add_key_to_identity( &self, @@ -128,10 +67,7 @@ impl AppContext { ) .map_err(|e| format!("IdentityUpdateTransition error: {}", e))?; - let result = state_transition - .broadcast_and_wait(sdk, None) - .await - .map_err(broadcast_error)?; + let result = state_transition.broadcast_and_wait(sdk, None).await?; // Log and handle the proof result tracing::info!("AddKeyToIdentity proof result: {}", result); @@ -188,76 +124,3 @@ impl AppContext { .map_err(|e| TaskError::Generic(format!("Database error: {}", e))) } } - -#[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::dpp::identifier::Identifier; - - #[test] - fn test_duplicate_public_key_error() { - let consensus = - ConsensusError::from(DuplicatedIdentityPublicKeyStateError::new(vec![1, 2])); - let sdk_err = SdkError::from(consensus); - let err = broadcast_error(sdk_err); - assert!(matches!(err, TaskError::DuplicateIdentityPublicKey { .. })); - } - - #[test] - fn test_duplicate_public_key_id_error() { - let consensus = ConsensusError::from(DuplicatedIdentityPublicKeyIdStateError::new(vec![3])); - let sdk_err = SdkError::from(consensus); - let err = broadcast_error(sdk_err); - assert!(matches!( - err, - TaskError::DuplicateIdentityPublicKeyId { .. } - )); - } - - #[test] - fn test_contract_bounds_conflict_error() { - 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 = broadcast_error(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 test_broadcast_error_with_cause_duplicate_key() { - // Test the StateTransitionBroadcastError path (the actual production path - // when Platform returns a structured broadcast error with a consensus cause). - 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 = broadcast_error(sdk_err); - assert!(matches!(err, TaskError::DuplicateIdentityPublicKey { .. })); - } - - #[test] - fn test_unknown_sdk_error_falls_back() { - let sdk_err = SdkError::Generic("connection timeout".to_string()); - let err = broadcast_error(sdk_err); - assert!(matches!(err, TaskError::Generic(ref s) if s.contains("connection timeout"))); - } -} diff --git a/src/backend_task/identity/mod.rs b/src/backend_task/identity/mod.rs index 4c505198f..327c3fa46 100644 --- a/src/backend_task/identity/mod.rs +++ b/src/backend_task/identity/mod.rs @@ -548,10 +548,9 @@ impl AppContext { IdentityTask::RegisterDpnsName(input) => { Ok(self.register_dpns_name(sdk, input).await?) } - IdentityTask::RefreshIdentity(qualified_identity) => Ok(self - .refresh_identity(sdk, qualified_identity, sender) - .await - .map_err(|e| format!("Error refreshing identity: {}", e))?), + IdentityTask::RefreshIdentity(qualified_identity) => { + self.refresh_identity(sdk, qualified_identity, sender).await + } IdentityTask::Transfer(qualified_identity, to_identifier, credits, id) => Ok(self .transfer_to_identity(qualified_identity, to_identifier, credits, id) .await?), @@ -571,16 +570,23 @@ impl AppContext { identity, inputs, wallet_seed_hash, - } => Ok(self - .top_up_identity_from_platform_addresses(sdk, identity, inputs, wallet_seed_hash) - .await?), + } => { + self.top_up_identity_from_platform_addresses( + sdk, + identity, + inputs, + wallet_seed_hash, + ) + .await + } IdentityTask::TransferToAddresses { identity, outputs, key_id, - } => Ok(self - .transfer_to_addresses(sdk, identity, outputs, key_id) - .await?), + } => { + self.transfer_to_addresses(sdk, identity, outputs, key_id) + .await + } IdentityTask::RefreshLoadedIdentitiesOwnedDPNSNames => { Ok(self.refresh_loaded_identities_dpns_names(sender).await?) } @@ -594,7 +600,7 @@ impl AppContext { qualified_identity: QualifiedIdentity, inputs: BTreeMap, wallet_seed_hash: WalletSeedHash, - ) -> Result { + ) -> Result { use crate::model::fee_estimation::PlatformFeeEstimator; use dash_sdk::platform::transition::top_up_identity_from_addresses::TopUpIdentityFromAddresses; @@ -614,14 +620,18 @@ impl AppContext { wallets .get(&wallet_seed_hash) .cloned() - .ok_or_else(|| "Wallet not found".to_string())? + .ok_or_else(|| TaskError::Generic("Wallet not found".into()))? }; - let wallet_guard = wallet.read().map_err(|e| e.to_string())?; + let wallet_guard = wallet + .read() + .map_err(|e| TaskError::Generic(e.to_string()))?; // Ensure wallet is open if !wallet_guard.is_open() { - return Err("Wallet must be unlocked to sign Platform transactions".to_string()); + return Err(TaskError::Generic( + "Wallet must be unlocked to sign Platform transactions".into(), + )); } wallet_guard.clone() @@ -638,7 +648,10 @@ impl AppContext { .await .map_err(|e| { tracing::error!("top_up_from_addresses failed: {}", e); - format!("Failed to top up identity from Platform addresses: {}", e) + TaskError::Generic(format!( + "Failed to top up identity from Platform addresses: {}", + e + )) })?; tracing::info!( @@ -658,8 +671,7 @@ impl AppContext { updated_identity.identity.set_balance(new_balance); // Store the updated identity (use update to preserve wallet association) - self.update_local_qualified_identity(&updated_identity) - .map_err(|e| format!("Failed to store updated identity: {}", e))?; + self.update_local_qualified_identity(&updated_identity)?; let fee_result = FeeResult::new(estimated_fee, estimated_fee); Ok(BackendTaskSuccessResult::ToppedUpIdentity( @@ -675,7 +687,7 @@ impl AppContext { qualified_identity: QualifiedIdentity, outputs: BTreeMap, key_id: Option, - ) -> Result { + ) -> Result { use crate::model::fee_estimation::PlatformFeeEstimator; use dash_sdk::platform::transition::transfer_to_addresses::TransferToAddresses; @@ -700,7 +712,12 @@ impl AppContext { None, ) .await - .map_err(|e| format!("Failed to transfer credits to Platform addresses: {}", e))?; + .map_err(|e| { + TaskError::Generic(format!( + "Failed to transfer credits to Platform addresses: {}", + e + )) + })?; // Update destination address balances in any wallets that contain them // (using proof-verified data from the SDK response) @@ -742,8 +759,7 @@ impl AppContext { } // Store the updated identity (use update to preserve wallet association) - self.update_local_qualified_identity(&updated_identity) - .map_err(|e| format!("Failed to store updated identity: {}", e))?; + self.update_local_qualified_identity(&updated_identity)?; let fee_result = FeeResult::new(estimated_fee, actual_fee); Ok(BackendTaskSuccessResult::TransferredCredits(fee_result)) diff --git a/src/backend_task/identity/refresh_identity.rs b/src/backend_task/identity/refresh_identity.rs index ab52f2482..eb057a1d1 100644 --- a/src/backend_task/identity/refresh_identity.rs +++ b/src/backend_task/identity/refresh_identity.rs @@ -1,4 +1,5 @@ use crate::app::TaskResult; +use crate::backend_task::error::TaskError; use crate::context::AppContext; use crate::model::qualified_identity::{IdentityStatus, QualifiedIdentity}; use dash_sdk::Sdk; @@ -14,27 +15,25 @@ impl AppContext { sdk: &Sdk, qualified_identity: QualifiedIdentity, sender: crate::utils::egui_mpsc::SenderAsync, - ) -> Result { + ) -> Result { let refreshed_identity_id = qualified_identity.identity.id(); // Fetch the latest state of the identity from Platform let maybe_refreshed_identity = Identity::fetch_by_identifier(sdk, refreshed_identity_id) .await - .map_err(|e| e.to_string())?; + .map_err(|e| TaskError::Generic(format!("Failed to fetch identity: {}", e)))?; // Get local identities - let mut local_qualified_identities = self - .load_local_qualified_identities() - .map_err(|e| e.to_string())?; + let mut local_qualified_identities = self.load_local_qualified_identities()?; // Find the local identity to update let outdated_identity_index = local_qualified_identities .iter() .position(|qi| qi.identity.id() == refreshed_identity_id) .ok_or_else(|| { - format!( + TaskError::Generic(format!( "Identity with id {} not found in local identities", refreshed_identity_id.to_string(Encoding::Base58) - ) + )) })?; // Remove the outdated identity from local state @@ -50,7 +49,6 @@ impl AppContext { .update(IdentityStatus::Active); } None => { - // it is not found and the status allows refresh, update status to NotFound qualified_identity_to_update .status .update(IdentityStatus::NotFound); @@ -58,14 +56,13 @@ impl AppContext { } // Insert the updated identity into local state - self.update_local_qualified_identity(&qualified_identity_to_update) - .map_err(|e| e.to_string())?; + self.update_local_qualified_identity(&qualified_identity_to_update)?; // Send refresh message to refresh the Identities Screen sender .send(TaskResult::Refresh) .await - .map_err(|e| e.to_string())?; + .map_err(|e| TaskError::Generic(e.to_string()))?; Ok(BackendTaskSuccessResult::RefreshedIdentity( qualified_identity, From 3d07287b0a279f6e7c12f95a04ae6c6e415f648a Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 12 Mar 2026 09:48:42 +0100 Subject: [PATCH 09/17] refactor: replace BroadcastError with SdkError variant and user-friendly messages Rename TaskError::BroadcastError to TaskError::SdkError and add sdk_error_user_message() helper that inspects the SDK error variant to produce actionable, user-friendly Display text for MessageBanner. Covers: broadcast rejections, timeouts, stale nodes, DAPI client errors, unavailable servers, cancellation, already-exists, nonce overflow/not-found. Fallback includes the SDK's own Display text for unmatched variants. Co-Authored-By: Claude Opus 4.6 --- src/backend_task/error.rs | 69 +++++++++++++++++++++++++++++++++++---- 1 file changed, 63 insertions(+), 6 deletions(-) diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs index 516051d72..2187893d3 100644 --- a/src/backend_task/error.rs +++ b/src/backend_task/error.rs @@ -93,14 +93,71 @@ pub enum TaskError { source_error: Box, }, - /// Unclassified broadcast error — the state transition failed for an unknown reason. - #[error("Failed to broadcast identity update. Please try again later.")] - BroadcastError { + /// 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, }, } +/// 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(e) => { + // Known broadcast rejection that didn't match a typed consensus variant + // above (DuplicateKey, DuplicateKeyId, ContractBoundsConflict). + // The platform message is often the most specific info we have. + // TODO: classify more consensus causes into dedicated TaskError variants + // so fewer errors reach this fallback. + format!( + "The platform rejected this operation: {}. Try a different approach.", + e.message + ) + } + 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(detail) => { + format!("This already exists on the platform: {detail}. No action needed.") + } + 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. + _ => { + // Fallback — the technical cause is in the #[source] chain / details panel. + format!("Unexpected error: {}. Please try again later.", error) + } + } +} + impl From for TaskError { fn from(s: String) -> Self { if s.contains("Wallet file not specified") { @@ -170,7 +227,7 @@ impl From for TaskError { source_error: boxed, } } - None => TaskError::BroadcastError { + None => TaskError::SdkError { source_error: boxed, }, } @@ -306,8 +363,8 @@ mod tests { let sdk_err = SdkError::Generic("connection timeout".to_string()); let err = TaskError::from(sdk_err); assert!( - matches!(err, TaskError::BroadcastError { .. }), - "Expected BroadcastError, got: {err:?}" + matches!(err, TaskError::SdkError { .. }), + "Expected SdkError, got: {err:?}" ); } } From adfacac3606c1b89696a28664998f437e322a401 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 12 Mar 2026 09:49:05 +0100 Subject: [PATCH 10/17] docs: add error message tone and form guidelines to CLAUDE.md Define rules for user-facing error text: audience (Everyday User persona), structure (what happened + what to do), tone (calm, direct, no jargon), and where technical details belong (details panel, not the message). Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 3f4cb4a2f..d11777637 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -62,6 +62,16 @@ 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: retry, wait, try a different approach, or contact support. +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. Attach via `BannerHandle::with_details(e)` — the `Debug` repr goes to the collapsible details panel and logs. +5. **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. + ## 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. From f89550f37943914bd9894e08119814e7b1ed7685 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 12 Mar 2026 09:52:53 +0100 Subject: [PATCH 11/17] =?UTF-8?q?docs:=20refine=20error=20message=20guidel?= =?UTF-8?q?ines=20=E2=80=94=20no=20support=20redirects,=20i18n-ready?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Remove "contact support" as an acceptable action — users must self-resolve - Add Fluent i18n readiness rule: simple sentences, no fragment concatenation, placeholders only for dynamic values Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index d11777637..e2866fd7b 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -67,10 +67,11 @@ scripts/safe-cargo.sh +nightly fmt --all 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: retry, wait, try a different approach, or contact support. +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. Attach via `BannerHandle::with_details(e)` — the `Debug` repr goes to the collapsible details panel and logs. -5. **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. +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. ## Architecture Overview From 4e775bfd9253740f77ac55cf6332cfddafc208d7 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 12 Mar 2026 10:04:34 +0100 Subject: [PATCH 12/17] refactor: make error messages i18n-ready and self-contained - Remove "key hash" jargon from DuplicateIdentityPublicKeyId message - Include platform/SDK error text as a Fluent-friendly { $error } placeholder in broadcast rejection and fallback messages - Remove "review the details" references (not visible in basic mode) - Never refer users to "contact support" or "details panel" - Update CLAUDE.md error guidelines accordingly Co-Authored-By: Claude Opus 4.6 --- CLAUDE.md | 2 +- src/backend_task/error.rs | 16 +++++++++------- 2 files changed, 10 insertions(+), 8 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index e2866fd7b..61c0ab78e 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -69,7 +69,7 @@ User-facing error messages (shown in `MessageBanner` via `Display`) must follow 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. Attach via `BannerHandle::with_details(e)` — the `Debug` repr goes to the collapsible details panel and logs. +4. **Technical details**: Never in the message itself. 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. 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. diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs index 2187893d3..7014b825e 100644 --- a/src/backend_task/error.rs +++ b/src/backend_task/error.rs @@ -75,7 +75,7 @@ pub enum TaskError { }, /// 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.")] + #[error("This key is already registered on the platform. Try a different key.")] DuplicateIdentityPublicKeyId { /// The original SDK error returned by the broadcast API. #[source] @@ -115,11 +115,10 @@ fn sdk_error_user_message(error: &SdkError) -> String { SdkError::StateTransitionBroadcastError(e) => { // Known broadcast rejection that didn't match a typed consensus variant // above (DuplicateKey, DuplicateKeyId, ContractBoundsConflict). - // The platform message is often the most specific info we have. // TODO: classify more consensus causes into dedicated TaskError variants // so fewer errors reach this fallback. format!( - "The platform rejected this operation: {}. Try a different approach.", + "The platform rejected this operation: {}. Please try a different approach.", e.message ) } @@ -140,8 +139,8 @@ fn sdk_error_user_message(error: &SdkError) -> String { "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(detail) => { - format!("This already exists on the platform: {detail}. No action needed.") + 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() @@ -152,8 +151,11 @@ fn sdk_error_user_message(error: &SdkError) -> String { // TODO: add arms for Protocol (consensus sub-errors), InvalidCreditTransfer, // MissingDependency, Config, etc. _ => { - // Fallback — the technical cause is in the #[source] chain / details panel. - format!("Unexpected error: {}. Please try again later.", error) + // Fallback — include the SDK's Display text so the user has *something* + // to act on. Maps to Fluent `{ $error }` placeholder. + // TODO: add dedicated arms for remaining variants (Protocol, + // InvalidCreditTransfer, MissingDependency, Config, etc.). + format!("An unexpected error occurred: {error}. Please try again later.") } } } From bdeba53981d27cbac1e2d84205f26b2e75f23caa Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 12 Mar 2026 10:45:30 +0100 Subject: [PATCH 13/17] =?UTF-8?q?docs:=20refine=20error=20message=20guidel?= =?UTF-8?q?ines=20=E2=80=94=20Base58=20IDs=20allowed,=20prefer=20typed=20v?= =?UTF-8?q?ariants?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Clarify rule 4: "technical details" means raw error strings/codes/SDK internals, not user-meaningful identifiers; add exception note pointing to new rule 7. - Add rule 7: Base58 IDs (contract, identity, document) are allowed in user-facing messages — they are opaque-but-copyable handles, not jargon. - Add rule 8: prefer granular TaskError variants with #[source] over TaskError::Generic(format!(...)); Generic is a last resort. - Fix "generic message" wording in Error banners section to avoid confusion with TaskError::Generic. - Add soft guideline: consider moving repeated banner messages into TaskError variants for centralised wording and testability. Co-Authored-By: Claude Sonnet 4.5 --- CLAUDE.md | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 61c0ab78e..216c68b73 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -69,9 +69,11 @@ User-facing error messages (shown in `MessageBanner` via `Display`) must follow 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. 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. +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 @@ -192,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 From c226c563418ff9fe2723dd8e56cf21ef954410e2 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 12 Mar 2026 10:47:11 +0100 Subject: [PATCH 14/17] =?UTF-8?q?refactor:=20address=20PR=20review=20comme?= =?UTF-8?q?nts=20=E2=80=94=20typed=20errors=20and=20clean=20user=20message?= =?UTF-8?q?s?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Add TaskError::IdentityNotFoundLocally variant with actionable Display - Fix sdk_error_user_message: remove raw e.message from StateTransitionBroadcastError arm; use fixed, jargon-free string - refresh_identity: bare ? on SDK fetch (From classifies); use IdentityNotFoundLocally; fix channel error to fixed string - add_key_to_identity: bare ? on nonce/identity fetches; use IdentityNotFoundLocally for None identity; fixed string for IdentityUpdateTransition build failure; bare ? + Ok() on DB update - identity/mod.rs: bare ? on top_up_from_addresses and transfer_credits_to_addresses (From handles classification) Addresses review threads 11, 12, 13, 14, 16, 20, 21. Co-Authored-By: Claude Sonnet 4.5 --- src/backend_task/error.rs | 14 ++++++++----- .../identity/add_key_to_identity.rs | 19 +++++++++--------- src/backend_task/identity/mod.rs | 17 ++-------------- src/backend_task/identity/refresh_identity.rs | 20 ++++++------------- 4 files changed, 27 insertions(+), 43 deletions(-) diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs index 7014b825e..9ecd30df0 100644 --- a/src/backend_task/error.rs +++ b/src/backend_task/error.rs @@ -93,6 +93,12 @@ pub enum TaskError { source_error: Box, }, + /// 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. @@ -112,15 +118,13 @@ pub enum TaskError { /// Each arm should explain *what happened* and *what the user can do*. fn sdk_error_user_message(error: &SdkError) -> String { match error { - SdkError::StateTransitionBroadcastError(e) => { + 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. - format!( - "The platform rejected this operation: {}. Please try a different approach.", - e.message - ) + "The platform rejected this request. Please check your input and try again." + .to_string() } SdkError::TimeoutReached(duration, _) => { format!( diff --git a/src/backend_task/identity/add_key_to_identity.rs b/src/backend_task/identity/add_key_to_identity.rs index 2100efa79..05fcd5538 100644 --- a/src/backend_task/identity/add_key_to_identity.rs +++ b/src/backend_task/identity/add_key_to_identity.rs @@ -28,16 +28,14 @@ impl AppContext { ) -> Result { let new_identity_nonce = sdk .get_identity_nonce(qualified_identity.identity.id(), true, None) - .await - .map_err(|e| format!("Fetch nonce error: {}", e))?; + .await?; let Some(master_key) = qualified_identity.can_sign_with_master_key() else { return Err("Master key not found".to_string().into()); }; let master_key_id = master_key.identity_public_key.id(); let identity = Identity::fetch_by_identifier(sdk, qualified_identity.identity.id()) - .await - .map_err(|e| format!("Fetch identity error: {}", e))? - .ok_or_else(|| TaskError::Generic("Identity not found".into()))?; + .await? + .ok_or(TaskError::IdentityNotFoundLocally)?; qualified_identity.identity = identity; qualified_identity.identity.bump_revision(); public_key_to_add @@ -65,7 +63,11 @@ impl AppContext { sdk.version(), None, ) - .map_err(|e| format!("IdentityUpdateTransition error: {}", e))?; + .map_err(|_| { + TaskError::Generic( + "Could not build the key update transaction. Please retry.".to_string(), + ) + })?; let result = state_transition.broadcast_and_wait(sdk, None).await?; @@ -119,8 +121,7 @@ impl AppContext { let fee_result = FeeResult::new(estimated_fee, actual_fee); - self.update_local_qualified_identity(&qualified_identity) - .map(|_| BackendTaskSuccessResult::AddedKeyToIdentity(fee_result)) - .map_err(|e| TaskError::Generic(format!("Database error: {}", e))) + self.update_local_qualified_identity(&qualified_identity)?; + Ok(BackendTaskSuccessResult::AddedKeyToIdentity(fee_result)) } } diff --git a/src/backend_task/identity/mod.rs b/src/backend_task/identity/mod.rs index 327c3fa46..09f126727 100644 --- a/src/backend_task/identity/mod.rs +++ b/src/backend_task/identity/mod.rs @@ -645,14 +645,7 @@ impl AppContext { // Execute the top-up let (address_infos, new_balance) = identity .top_up_from_addresses(sdk, inputs, &wallet_clone, None) - .await - .map_err(|e| { - tracing::error!("top_up_from_addresses failed: {}", e); - TaskError::Generic(format!( - "Failed to top up identity from Platform addresses: {}", - e - )) - })?; + .await?; tracing::info!( "top_up_from_addresses succeeded, new_balance={}", @@ -711,13 +704,7 @@ impl AppContext { &qualified_identity, None, ) - .await - .map_err(|e| { - TaskError::Generic(format!( - "Failed to transfer credits to Platform addresses: {}", - e - )) - })?; + .await?; // Update destination address balances in any wallets that contain them // (using proof-verified data from the SDK response) diff --git a/src/backend_task/identity/refresh_identity.rs b/src/backend_task/identity/refresh_identity.rs index eb057a1d1..1045566af 100644 --- a/src/backend_task/identity/refresh_identity.rs +++ b/src/backend_task/identity/refresh_identity.rs @@ -4,7 +4,6 @@ use crate::context::AppContext; use crate::model::qualified_identity::{IdentityStatus, QualifiedIdentity}; use dash_sdk::Sdk; use dash_sdk::dpp::identity::accessors::IdentityGettersV0; -use dash_sdk::dpp::platform_value::string_encoding::Encoding; use dash_sdk::platform::{Fetch, Identity}; use super::BackendTaskSuccessResult; @@ -18,9 +17,8 @@ impl AppContext { ) -> Result { let refreshed_identity_id = qualified_identity.identity.id(); // Fetch the latest state of the identity from Platform - let maybe_refreshed_identity = Identity::fetch_by_identifier(sdk, refreshed_identity_id) - .await - .map_err(|e| TaskError::Generic(format!("Failed to fetch identity: {}", e)))?; + let maybe_refreshed_identity = + Identity::fetch_by_identifier(sdk, refreshed_identity_id).await?; // Get local identities let mut local_qualified_identities = self.load_local_qualified_identities()?; @@ -29,12 +27,7 @@ impl AppContext { let outdated_identity_index = local_qualified_identities .iter() .position(|qi| qi.identity.id() == refreshed_identity_id) - .ok_or_else(|| { - TaskError::Generic(format!( - "Identity with id {} not found in local identities", - refreshed_identity_id.to_string(Encoding::Base58) - )) - })?; + .ok_or(TaskError::IdentityNotFoundLocally)?; // Remove the outdated identity from local state let mut qualified_identity_to_update = @@ -59,10 +52,9 @@ impl AppContext { self.update_local_qualified_identity(&qualified_identity_to_update)?; // Send refresh message to refresh the Identities Screen - sender - .send(TaskResult::Refresh) - .await - .map_err(|e| TaskError::Generic(e.to_string()))?; + sender.send(TaskResult::Refresh).await.map_err(|_| { + TaskError::Generic("Internal update failed. Please retry the operation.".to_string()) + })?; Ok(BackendTaskSuccessResult::RefreshedIdentity( qualified_identity, From 072d1b435463e67ecb94c66b6e816dcee923face Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 12 Mar 2026 11:10:43 +0100 Subject: [PATCH 15/17] fix: add message-based duplicate-key fallback and IdentitySaveError variant MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - From: add .or_else() message-based fallback for StateTransitionBroadcastError with cause=None; if message contains "duplicate" (case-insensitive) map to DuplicateIdentityPublicKey — guards against #714 regression when DAPI omits structured cause - Add regression test for cause=None message-only duplicate-key path - Rename misleading test from_sdk_error_unknown_falls_back_to_broadcast_error → from_sdk_error_generic_variant_falls_back_to_sdk_error - Add TaskError::IdentitySaveError { #[source] source: rusqlite::Error } with user-friendly Display; use at both update_local_qualified_identity callsites in top_up and transfer paths (threads 7, 8) - Add TODO(i18n/ux) comment to sdk_error_user_message fallback arm (thread 17) Addresses review threads 7, 8, 9, 17, 19. Co-Authored-By: Claude Sonnet 4.5 --- src/backend_task/error.rs | 77 ++++++++++++++++++++++++-------- src/backend_task/identity/mod.rs | 6 ++- 2 files changed, 62 insertions(+), 21 deletions(-) diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs index 9ecd30df0..9f2259bbf 100644 --- a/src/backend_task/error.rs +++ b/src/backend_task/error.rs @@ -51,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), @@ -155,10 +162,11 @@ fn sdk_error_user_message(error: &SdkError) -> String { // TODO: add arms for Protocol (consensus sub-errors), InvalidCreditTransfer, // MissingDependency, Config, etc. _ => { - // Fallback — include the SDK's Display text so the user has *something* - // to act on. Maps to Fluent `{ $error }` placeholder. - // TODO: add dedicated arms for remaining variants (Protocol, - // 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.") } } @@ -203,20 +211,36 @@ impl From for TaskError { _ => 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, - }) + 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); @@ -365,7 +389,7 @@ mod tests { } #[test] - fn from_sdk_error_unknown_falls_back_to_broadcast_error() { + 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!( @@ -373,4 +397,19 @@ mod tests { "Expected SdkError, got: {err:?}" ); } + + #[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:?}" + ); + } } diff --git a/src/backend_task/identity/mod.rs b/src/backend_task/identity/mod.rs index 09f126727..74bdb74e7 100644 --- a/src/backend_task/identity/mod.rs +++ b/src/backend_task/identity/mod.rs @@ -664,7 +664,8 @@ impl AppContext { updated_identity.identity.set_balance(new_balance); // Store the updated identity (use update to preserve wallet association) - self.update_local_qualified_identity(&updated_identity)?; + self.update_local_qualified_identity(&updated_identity) + .map_err(|e| TaskError::IdentitySaveError { source: e })?; let fee_result = FeeResult::new(estimated_fee, estimated_fee); Ok(BackendTaskSuccessResult::ToppedUpIdentity( @@ -746,7 +747,8 @@ impl AppContext { } // Store the updated identity (use update to preserve wallet association) - self.update_local_qualified_identity(&updated_identity)?; + self.update_local_qualified_identity(&updated_identity) + .map_err(|e| TaskError::IdentitySaveError { source: e })?; let fee_result = FeeResult::new(estimated_fee, actual_fee); Ok(BackendTaskSuccessResult::TransferredCredits(fee_result)) From b5c0f8c2eccfd5877129980e92636019591a92c1 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 12 Mar 2026 12:11:16 +0100 Subject: [PATCH 16/17] refactor: preserve source errors in typed TaskError variants - Add IdentityUpdateTransitionError variant to preserve ProtocolError from try_from_identity_with_signer (was discarded via |_| Generic) - Add InternalSendError variant to preserve SendError from channel send (was discarded via |_| Generic in refresh_identity) - Wire IdentitySaveError in add_key_to_identity (bare ? produced TaskError::Sqlite with raw rusqlite text instead of friendly message) - Add TODO for LockPoisoned variant in mod.rs wallet.read() path Co-Authored-By: Claude Sonnet 4.6 --- src/backend_task/error.rs | 14 ++++++++++++++ src/backend_task/identity/add_key_to_identity.rs | 9 ++++----- src/backend_task/identity/mod.rs | 2 ++ src/backend_task/identity/refresh_identity.rs | 9 ++++++--- 4 files changed, 26 insertions(+), 8 deletions(-) diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs index 9f2259bbf..4883903c5 100644 --- a/src/backend_task/error.rs +++ b/src/backend_task/error.rs @@ -106,6 +106,20 @@ pub enum TaskError { )] IdentityNotFoundLocally, + /// Failed to build the identity update state transition. + #[error("Could not build the key update transaction. Please retry.")] + IdentityUpdateTransitionError { + #[source] + source: Box, + }, + + /// Failed to send a result back to the UI — the receiver was dropped. + #[error("Internal update failed. Please retry the operation.")] + InternalSendError { + #[source] + source: Box, + }, + /// 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. diff --git a/src/backend_task/identity/add_key_to_identity.rs b/src/backend_task/identity/add_key_to_identity.rs index 05fcd5538..304865017 100644 --- a/src/backend_task/identity/add_key_to_identity.rs +++ b/src/backend_task/identity/add_key_to_identity.rs @@ -63,10 +63,8 @@ impl AppContext { sdk.version(), None, ) - .map_err(|_| { - TaskError::Generic( - "Could not build the key update transaction. Please retry.".to_string(), - ) + .map_err(|e| TaskError::IdentityUpdateTransitionError { + source: Box::new(e), })?; let result = state_transition.broadcast_and_wait(sdk, None).await?; @@ -121,7 +119,8 @@ impl AppContext { let fee_result = FeeResult::new(estimated_fee, actual_fee); - self.update_local_qualified_identity(&qualified_identity)?; + self.update_local_qualified_identity(&qualified_identity) + .map_err(|e| TaskError::IdentitySaveError { source: e })?; Ok(BackendTaskSuccessResult::AddedKeyToIdentity(fee_result)) } } diff --git a/src/backend_task/identity/mod.rs b/src/backend_task/identity/mod.rs index 74bdb74e7..fb47170aa 100644 --- a/src/backend_task/identity/mod.rs +++ b/src/backend_task/identity/mod.rs @@ -623,6 +623,8 @@ impl AppContext { .ok_or_else(|| TaskError::Generic("Wallet not found".into()))? }; + // TODO: Replace Generic with a dedicated TaskError::LockPoisoned variant + // that preserves the PoisonError as #[source] with a user-friendly Display. let wallet_guard = wallet .read() .map_err(|e| TaskError::Generic(e.to_string()))?; diff --git a/src/backend_task/identity/refresh_identity.rs b/src/backend_task/identity/refresh_identity.rs index 1045566af..7fccfd788 100644 --- a/src/backend_task/identity/refresh_identity.rs +++ b/src/backend_task/identity/refresh_identity.rs @@ -52,9 +52,12 @@ impl AppContext { self.update_local_qualified_identity(&qualified_identity_to_update)?; // Send refresh message to refresh the Identities Screen - sender.send(TaskResult::Refresh).await.map_err(|_| { - TaskError::Generic("Internal update failed. Please retry the operation.".to_string()) - })?; + sender + .send(TaskResult::Refresh) + .await + .map_err(|e| TaskError::InternalSendError { + source: Box::new(e), + })?; Ok(BackendTaskSuccessResult::RefreshedIdentity( qualified_identity, From b5c8884a0d1ce06d756153e8196956134105e9a4 Mon Sep 17 00:00:00 2001 From: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Date: Thu, 12 Mar 2026 12:20:56 +0100 Subject: [PATCH 17/17] refactor: use concrete SdkError type in error variants; drop dyn Error MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - IdentityUpdateTransitionError.source_error: Box → Box (ProtocolError converts via SdkError::Protocol(e)) - InternalSendError: struct variant → unit variant (SendError carries no diagnostic information worth preserving in the chain) - CLAUDE.md rule 8: document source field type conventions — Box for SDK-originated errors, concrete domain type for non-SDK errors, omit #[source] when upstream carries no diagnostic value Co-Authored-By: Claude Sonnet 4.6 --- CLAUDE.md | 2 +- src/backend_task/error.rs | 7 ++----- src/backend_task/identity/add_key_to_identity.rs | 3 ++- src/backend_task/identity/refresh_identity.rs | 4 +--- 4 files changed, 6 insertions(+), 10 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 216c68b73..01763d36c 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -73,7 +73,7 @@ User-facing error messages (shown in `MessageBanner` via `Display`) must follow 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. +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. For `#[source]` fields in SDK-originated error variants, use `Box` — convert upstream types (e.g. `ProtocolError`) via `SdkError::Protocol(e)`. Use the concrete domain type directly for non-SDK errors (e.g. `rusqlite::Error`). Omit `#[source]` entirely when the upstream error carries no useful diagnostic information (e.g. a channel `SendError`). ## Architecture Overview diff --git a/src/backend_task/error.rs b/src/backend_task/error.rs index 4883903c5..359ec570b 100644 --- a/src/backend_task/error.rs +++ b/src/backend_task/error.rs @@ -110,15 +110,12 @@ pub enum TaskError { #[error("Could not build the key update transaction. Please retry.")] IdentityUpdateTransitionError { #[source] - source: Box, + source_error: Box, }, /// Failed to send a result back to the UI — the receiver was dropped. #[error("Internal update failed. Please retry the operation.")] - InternalSendError { - #[source] - source: Box, - }, + InternalSendError, /// Unclassified SDK error — the operation failed for an unrecognised reason. /// Display is implemented manually via [`sdk_error_user_message`] to inspect diff --git a/src/backend_task/identity/add_key_to_identity.rs b/src/backend_task/identity/add_key_to_identity.rs index 304865017..4f5a44446 100644 --- a/src/backend_task/identity/add_key_to_identity.rs +++ b/src/backend_task/identity/add_key_to_identity.rs @@ -6,6 +6,7 @@ use crate::model::fee_estimation::PlatformFeeEstimator; use crate::model::qualified_identity::PrivateKeyTarget::PrivateKeyOnMainIdentity; use crate::model::qualified_identity::QualifiedIdentity; use crate::model::qualified_identity::qualified_identity_public_key::QualifiedIdentityPublicKey; +use dash_sdk::Error as SdkError; use dash_sdk::Sdk; use dash_sdk::dpp::identity::accessors::{IdentityGettersV0, IdentitySettersV0}; use dash_sdk::dpp::identity::identity_public_key::accessors::v0::{ @@ -64,7 +65,7 @@ impl AppContext { None, ) .map_err(|e| TaskError::IdentityUpdateTransitionError { - source: Box::new(e), + source_error: Box::new(SdkError::Protocol(e)), })?; let result = state_transition.broadcast_and_wait(sdk, None).await?; diff --git a/src/backend_task/identity/refresh_identity.rs b/src/backend_task/identity/refresh_identity.rs index 7fccfd788..5b7d8701c 100644 --- a/src/backend_task/identity/refresh_identity.rs +++ b/src/backend_task/identity/refresh_identity.rs @@ -55,9 +55,7 @@ impl AppContext { sender .send(TaskResult::Refresh) .await - .map_err(|e| TaskError::InternalSendError { - source: Box::new(e), - })?; + .map_err(|_| TaskError::InternalSendError)?; Ok(BackendTaskSuccessResult::RefreshedIdentity( qualified_identity,