Skip to content

Commit ece3659

Browse files
lklimekclaude
andauthored
fix: show user-friendly error for duplicate identity keys (#729)
* 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 <noreply@anthropic.com> * fix: correct error messages — duplicate keys are globally unique, not per-identity 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 <noreply@anthropic.com> * 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 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
1 parent fd0bb08 commit ece3659

3 files changed

Lines changed: 310 additions & 1 deletion

File tree

Lines changed: 177 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,177 @@
1+
# Manual Test Scenarios: Duplicate Key Error Handling (Issue #714)
2+
3+
Covers the fix for showing user-friendly error messages when adding a duplicate
4+
key to an identity, instead of raw base64-encoded CBOR.
5+
6+
---
7+
8+
## Prerequisites (all scenarios)
9+
10+
- Dash Evo Tool running and connected to **Testnet** (or Devnet).
11+
- A funded **wallet** loaded in the application.
12+
- An **identity** registered on the network with at least one existing key
13+
(ECDSA_SECP256K1 AUTHENTICATION MASTER is the default).
14+
- The identity's master private key is available (wallet unlocked or key known).
15+
16+
---
17+
18+
## Scenario 1: Adding a key with the same public key data (DuplicatedIdentityPublicKeyStateError)
19+
20+
### Goal
21+
22+
Verify the app displays a clear, actionable message when the user tries to add
23+
a key whose public key data already exists on the identity.
24+
25+
### Steps
26+
27+
1. Open **Identities** screen and select the target identity.
28+
2. Click **Keys** to view the identity's existing keys.
29+
3. Note the public key data of an existing key (e.g., copy the hex value of key
30+
ID 0).
31+
4. Click **Add Key**.
32+
5. Set **Purpose** to `AUTHENTICATION`, **Security Level** to `HIGH`,
33+
**Key Type** to `ECDSA_SECP256K1` (or match the existing key's type).
34+
6. In the **Private key (hex)** field, enter the exact same private key that
35+
corresponds to the existing public key on the identity.
36+
7. Click the **Add Key** button to submit.
37+
38+
### Expected Result
39+
40+
- An **error banner** appears at the top of the screen with the message:
41+
> This public key is already registered on the platform. Try a different key.
42+
- The message is plain text -- no base64, no CBOR encoding, no raw error dump.
43+
- The banner type is **Error** (red/error styling).
44+
- The app does **not** crash or freeze.
45+
- The **Add Key** form remains accessible so the user can correct the input and
46+
retry.
47+
48+
---
49+
50+
## Scenario 2: Adding a key with a conflicting key ID (DuplicatedIdentityPublicKeyIdStateError)
51+
52+
### Goal
53+
54+
Verify the app displays a clear message when a key ID collision occurs.
55+
56+
### Context
57+
58+
This error is less likely via the GUI because the app auto-assigns the next
59+
available key ID. It can occur if the identity state is out of sync (e.g.,
60+
another client added a key concurrently). To trigger it manually, a modified
61+
client or direct Platform interaction may be needed. This scenario validates the
62+
error display path regardless of trigger.
63+
64+
### Steps
65+
66+
1. Open **Identities** screen and select the target identity.
67+
2. Click **Keys**, then **Add Key**.
68+
3. Fill in a valid new key (different key data from existing keys).
69+
4. Arrange for the key ID to collide with an existing key ID (this may require
70+
a race condition where another client adds a key between nonce fetch and
71+
broadcast, or direct Platform API manipulation).
72+
5. Submit the **Add Key** request.
73+
74+
### Expected Result
75+
76+
- An **error banner** appears with the message:
77+
> This key hash is already registered on the platform. Try a different key.
78+
- The message is plain text -- no encoded data.
79+
- The banner type is **Error**.
80+
- The app does **not** crash.
81+
- The user can dismiss the banner and retry.
82+
83+
---
84+
85+
## Scenario 3: Adding a key that conflicts with unique contract bounds (IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError)
86+
87+
### Goal
88+
89+
Verify the app displays a clear message including the conflicting contract ID
90+
when a key violates unique contract bounds.
91+
92+
### Steps
93+
94+
1. Open **Identities** screen and select the target identity.
95+
2. Click **Keys**, then **Add Key**.
96+
3. Check the **Contract bounds** checkbox.
97+
4. Enter a valid **Contract ID** for a contract that enforces unique key bounds.
98+
5. Set the **Purpose** and **Security Level** to match an existing key that is
99+
already bound to the same contract.
100+
6. Generate or enter a new private key.
101+
7. Click the **Add Key** button to submit.
102+
103+
### Expected Result
104+
105+
- An **error banner** appears with the message:
106+
> This key conflicts with an existing key bound to contract \<contract-id\>. Use a different key or purpose.
107+
108+
where `<contract-id>` is the actual identifier of the conflicting contract
109+
(not a placeholder, not encoded).
110+
- The message is plain text -- no base64, no CBOR.
111+
- The banner type is **Error**.
112+
- The app does **not** crash.
113+
- The user can change the **Purpose**, **Contract ID**, or key data and retry.
114+
115+
---
116+
117+
## Scenario 4: Fallback for unrecognized broadcast errors
118+
119+
### Goal
120+
121+
Verify that broadcast errors not matching the three handled patterns still
122+
display a readable message (prefixed with "Broadcasting error:") rather than
123+
crashing.
124+
125+
### Steps
126+
127+
1. Open **Identities** screen and select the target identity.
128+
2. Click **Keys**, then **Add Key**.
129+
3. Trigger a broadcast failure that is NOT a duplicate key error (e.g.,
130+
disconnect from the network mid-broadcast, or submit with insufficient
131+
identity balance for the fee).
132+
4. Observe the error display.
133+
134+
### Expected Result
135+
136+
- An **error banner** appears with a message starting with:
137+
> Broadcasting error: ...
138+
- The remaining text may be technical but must not contain raw base64 CBOR
139+
blobs.
140+
- The app does **not** crash.
141+
- The user can retry.
142+
143+
---
144+
145+
## Scenario 5: Successful key addition after a duplicate key error
146+
147+
### Goal
148+
149+
Verify the app recovers gracefully and allows a successful operation after a
150+
prior duplicate key error.
151+
152+
### Steps
153+
154+
1. Reproduce **Scenario 1** to trigger the duplicate key error banner.
155+
2. Observe that the error banner is displayed.
156+
3. Change the **Private key (hex)** field to a new, unique private key.
157+
4. Click the **Add Key** button again.
158+
159+
### Expected Result
160+
161+
- The duplicate key error banner is dismissed/replaced.
162+
- A progress/info banner may appear while the transaction broadcasts.
163+
- On success, a **success banner** appears confirming the key was added,
164+
including fee information.
165+
- The new key appears in the identity's key list.
166+
- No residual error state from the previous failed attempt.
167+
168+
---
169+
170+
## Edge Cases
171+
172+
| Case | Action | Expected |
173+
|------|--------|----------|
174+
| Double-click Add Key | Click Add Key rapidly twice | Only one broadcast attempt; no duplicate submission |
175+
| Wallet locks during broadcast | Lock wallet (if possible) after clicking Add Key | Graceful error, no crash |
176+
| Network disconnect during broadcast | Disable network after clicking Add Key | Timeout or connection error displayed as banner, not raw panic |
177+
| Very long contract ID in error | Trigger Scenario 3 with a valid 256-bit identifier | Full contract ID shown in the message, no truncation |

src/backend_task/error.rs

Lines changed: 14 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,20 @@ pub enum TaskError {
6060
/// Callers should retry the failed operation.
6161
#[error("{0}")]
6262
MustRetry(String),
63+
64+
/// Duplicate identity public key — the key data already exists on the platform.
65+
#[error("This public key is already registered on the platform. Try a different key.")]
66+
DuplicateIdentityPublicKey,
67+
68+
/// Duplicate identity public key ID — the key hash is already taken platform-wide.
69+
#[error("This key hash is already registered on the platform. Try a different key.")]
70+
DuplicateIdentityPublicKeyId,
71+
72+
/// Identity public key conflicts with an existing key's unique contract bounds.
73+
#[error(
74+
"This key conflicts with an existing key bound to contract {contract_id}. Use a different key or purpose."
75+
)]
76+
IdentityPublicKeyContractBoundsConflict { contract_id: String },
6377
}
6478

6579
impl From<String> for TaskError {

src/backend_task/identity/add_key_to_identity.rs

Lines changed: 119 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,16 @@
11
use super::BackendTaskSuccessResult;
22
use crate::backend_task::FeeResult;
3+
use crate::backend_task::error::TaskError;
34
use crate::context::AppContext;
45
use crate::model::fee_estimation::PlatformFeeEstimator;
56
use crate::model::qualified_identity::PrivateKeyTarget::PrivateKeyOnMainIdentity;
67
use crate::model::qualified_identity::QualifiedIdentity;
78
use crate::model::qualified_identity::qualified_identity_public_key::QualifiedIdentityPublicKey;
9+
use dash_sdk::Error as SdkError;
810
use dash_sdk::Sdk;
11+
use dash_sdk::dpp::ProtocolError;
12+
use dash_sdk::dpp::consensus::ConsensusError;
13+
use dash_sdk::dpp::consensus::state::state_error::StateError;
914
use dash_sdk::dpp::identity::accessors::{IdentityGettersV0, IdentitySettersV0};
1015
use dash_sdk::dpp::identity::identity_public_key::accessors::v0::{
1116
IdentityPublicKeyGettersV0, IdentityPublicKeySettersV0,
@@ -17,6 +22,36 @@ use dash_sdk::dpp::state_transition::proof_result::StateTransitionProofResult;
1722
use dash_sdk::platform::transition::broadcast::BroadcastStateTransition;
1823
use dash_sdk::platform::{Fetch, Identity};
1924

25+
fn broadcast_error_message(error: &SdkError) -> String {
26+
let consensus_error = match error {
27+
SdkError::StateTransitionBroadcastError(broadcast_err) => broadcast_err.cause.as_ref(),
28+
SdkError::Protocol(ProtocolError::ConsensusError(ce)) => Some(ce.as_ref()),
29+
_ => None,
30+
};
31+
32+
if let Some(ce) = consensus_error {
33+
match ce {
34+
ConsensusError::StateError(StateError::DuplicatedIdentityPublicKeyStateError(_)) => {
35+
return TaskError::DuplicateIdentityPublicKey.to_string();
36+
}
37+
ConsensusError::StateError(StateError::DuplicatedIdentityPublicKeyIdStateError(_)) => {
38+
return TaskError::DuplicateIdentityPublicKeyId.to_string();
39+
}
40+
ConsensusError::StateError(
41+
StateError::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError(e),
42+
) => {
43+
return TaskError::IdentityPublicKeyContractBoundsConflict {
44+
contract_id: format!("{}", e.contract_id()),
45+
}
46+
.to_string();
47+
}
48+
_ => {}
49+
}
50+
}
51+
52+
format!("Broadcasting error: {}", error)
53+
}
54+
2055
impl AppContext {
2156
pub(super) async fn add_key_to_identity(
2257
&self,
@@ -69,7 +104,7 @@ impl AppContext {
69104
let result = state_transition
70105
.broadcast_and_wait(sdk, None)
71106
.await
72-
.map_err(|e| format!("Broadcasting error: {}", e))?;
107+
.map_err(|ref e| broadcast_error_message(e))?;
73108

74109
// Log and handle the proof result
75110
tracing::info!("AddKeyToIdentity proof result: {}", result);
@@ -126,3 +161,86 @@ impl AppContext {
126161
.map_err(|e| format!("Database error: {}", e))
127162
}
128163
}
164+
165+
#[cfg(test)]
166+
mod tests {
167+
use super::*;
168+
use dash_sdk::dpp::consensus::state::identity::duplicated_identity_public_key_id_state_error::DuplicatedIdentityPublicKeyIdStateError;
169+
use dash_sdk::dpp::consensus::state::identity::duplicated_identity_public_key_state_error::DuplicatedIdentityPublicKeyStateError;
170+
use dash_sdk::dpp::consensus::state::identity::identity_public_key_already_exists_for_unique_contract_bounds_error::IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError;
171+
use dash_sdk::dpp::identity::Purpose;
172+
use dash_sdk::dpp::identifier::Identifier;
173+
174+
#[test]
175+
fn test_duplicate_public_key_error() {
176+
let consensus =
177+
ConsensusError::from(DuplicatedIdentityPublicKeyStateError::new(vec![1, 2]));
178+
let sdk_err = SdkError::from(consensus);
179+
let msg = broadcast_error_message(&sdk_err);
180+
assert_eq!(
181+
msg,
182+
"This public key is already registered on the platform. Try a different key."
183+
);
184+
}
185+
186+
#[test]
187+
fn test_duplicate_public_key_id_error() {
188+
let consensus = ConsensusError::from(DuplicatedIdentityPublicKeyIdStateError::new(vec![3]));
189+
let sdk_err = SdkError::from(consensus);
190+
let msg = broadcast_error_message(&sdk_err);
191+
assert_eq!(
192+
msg,
193+
"This key hash is already registered on the platform. Try a different key."
194+
);
195+
}
196+
197+
#[test]
198+
fn test_contract_bounds_conflict_error() {
199+
let contract_id = Identifier::random();
200+
let identity_id = Identifier::random();
201+
let consensus = ConsensusError::from(
202+
IdentityPublicKeyAlreadyExistsForUniqueContractBoundsError::new(
203+
identity_id,
204+
contract_id,
205+
Purpose::AUTHENTICATION,
206+
2,
207+
1,
208+
),
209+
);
210+
let sdk_err = SdkError::from(consensus);
211+
let msg = broadcast_error_message(&sdk_err);
212+
assert_eq!(
213+
msg,
214+
format!(
215+
"This key conflicts with an existing key bound to contract {}. Use a different key or purpose.",
216+
contract_id
217+
)
218+
);
219+
}
220+
221+
#[test]
222+
fn test_broadcast_error_with_cause_duplicate_key() {
223+
// Test the StateTransitionBroadcastError path (the actual production path
224+
// when Platform returns a structured broadcast error with a consensus cause).
225+
let consensus = ConsensusError::from(DuplicatedIdentityPublicKeyStateError::new(vec![1]));
226+
let broadcast_err = dash_sdk::error::StateTransitionBroadcastError {
227+
code: 40206,
228+
message: "duplicate key".to_string(),
229+
cause: Some(consensus),
230+
};
231+
let sdk_err = SdkError::StateTransitionBroadcastError(broadcast_err);
232+
let msg = broadcast_error_message(&sdk_err);
233+
assert_eq!(
234+
msg,
235+
"This public key is already registered on the platform. Try a different key."
236+
);
237+
}
238+
239+
#[test]
240+
fn test_unknown_sdk_error_falls_back() {
241+
let sdk_err = SdkError::Generic("connection timeout".to_string());
242+
let msg = broadcast_error_message(&sdk_err);
243+
assert!(msg.starts_with("Broadcasting error:"));
244+
assert!(msg.contains("connection timeout"));
245+
}
246+
}

0 commit comments

Comments
 (0)