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

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
68 changes: 30 additions & 38 deletions src/backend_task/identity/add_key_to_identity.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,20 @@ 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;
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 {
Comment thread
lklimek marked this conversation as resolved.
Outdated
let consensus_error = match error {
SdkError::StateTransitionBroadcastError(broadcast_err) => broadcast_err.cause.as_ref(),
SdkError::Protocol(ProtocolError::ConsensusError(ce)) => Some(ce.as_ref()),
Expand All @@ -32,24 +38,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();
contract_id: e.contract_id().to_string(Encoding::Base58),
};
}
_ => {}
}
}

format!("Broadcasting error: {}", error)
TaskError::Generic(format!("Broadcasting error: {}", error))
}

impl AppContext {
Expand All @@ -59,19 +64,19 @@ impl AppContext {
mut qualified_identity: QualifiedIdentity,
mut public_key_to_add: QualifiedIdentityPublicKey,
private_key: [u8; 32],
) -> Result<BackendTaskSuccessResult, String> {
) -> Result<BackendTaskSuccessResult, TaskError> {
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()))?;
Comment thread
lklimek marked this conversation as resolved.
Outdated
qualified_identity.identity = identity;
qualified_identity.identity.bump_revision();
public_key_to_add
Expand Down Expand Up @@ -104,7 +109,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);
Expand Down Expand Up @@ -158,7 +163,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)))
}
Comment thread
coderabbitai[bot] marked this conversation as resolved.
}

Expand All @@ -176,22 +181,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]
Expand All @@ -208,13 +207,10 @@ 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);
let expected_contract_id = contract_id.to_string(Encoding::Base58);
assert!(
matches!(err, TaskError::IdentityPublicKeyContractBoundsConflict { ref contract_id } if *contract_id == expected_contract_id)
);
}

Expand All @@ -229,18 +225,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")));
}
}
72 changes: 33 additions & 39 deletions src/backend_task/identity/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ mod top_up_identity;
mod transfer;
mod withdraw_from_identity;

use super::{BackendTaskSuccessResult, FeeResult};
use super::{BackendTaskSuccessResult, FeeResult, TaskError};
Comment thread
lklimek marked this conversation as resolved.
use crate::app::TaskResult;
use crate::context::AppContext;
use crate::model::qualified_identity::encrypted_key_storage::{KeyStorage, WalletDerivationPath};
Expand Down Expand Up @@ -530,65 +530,59 @@ impl AppContext {
task: IdentityTask,
sdk: &Sdk,
sender: crate::utils::egui_mpsc::SenderAsync<TaskResult>,
) -> Result<BackendTaskSuccessResult, String> {
) -> Result<BackendTaskSuccessResult, TaskError> {
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?),
Comment thread
coderabbitai[bot] marked this conversation as resolved.
Outdated
IdentityTask::RefreshLoadedIdentitiesOwnedDPNSNames => {
self.refresh_loaded_identities_dpns_names(sender).await
Ok(self.refresh_loaded_identities_dpns_names(sender).await?)
}
}
}
Expand Down
Loading