fix: include identity name in auth key missing error messages#745
fix: include identity name in auth key missing error messages#745thepastaclaw wants to merge 2 commits into
Conversation
When an identity lacks an authentication key for signing document transitions, the error message now includes the identity name or base58 ID so users with multiple identities can identify which one has the problem. Closes dashpay#743
|
Important Review skippedDraft detected. Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
📝 WalkthroughWalkthroughTwo files are updated to enhance error messaging when an identity lacks an authentication key for signing document transitions. Error messages now include the identity identifier, enabling users to pinpoint which identity is affected. Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~8 minutes Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
|
@coderabbitai review |
✅ Actions performedReview triggered.
|
There was a problem hiding this comment.
🧹 Nitpick comments (1)
src/backend_task/identity/register_dpns_name.rs (1)
128-131: Make this error construction lazy withok_or_else.
ok_or(format!(...))builds the message on every call, including success cases.ok_or_else(|| format!(...))avoids unnecessary allocation anddisplay_string()work.Proposed diff
- let public_key = qualified_identity - .document_signing_key(&preorder_document_type) - .ok_or(format!( - "Identity {} doesn't have an authentication key for signing document transitions", - qualified_identity.display_string() - ))?; + let public_key = qualified_identity + .document_signing_key(&preorder_document_type) + .ok_or_else(|| { + format!( + "Identity {} doesn't have an authentication key for signing document transitions", + qualified_identity.display_string() + ) + })?;🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend_task/identity/register_dpns_name.rs` around lines 128 - 131, Replace the eager error construction using ok_or(...) with a lazy closure ok_or_else(|| ...) so the formatted message (including qualified_identity.display_string()) is only built on the error path; locate the call that currently uses .ok_or(format!("Identity {} doesn't have an authentication key for signing document transitions", qualified_identity.display_string())) and change it to use .ok_or_else(|| format!(...)) to avoid unnecessary allocation and work on success.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Nitpick comments:
In `@src/backend_task/identity/register_dpns_name.rs`:
- Around line 128-131: Replace the eager error construction using ok_or(...)
with a lazy closure ok_or_else(|| ...) so the formatted message (including
qualified_identity.display_string()) is only built on the error path; locate the
call that currently uses .ok_or(format!("Identity {} doesn't have an
authentication key for signing document transitions",
qualified_identity.display_string())) and change it to use .ok_or_else(||
format!(...)) to avoid unnecessary allocation and work on success.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 4d3f6e63-d83c-4197-900f-9a0abd7bd270
📒 Files selected for processing (2)
src/backend_task/identity/register_dpns_name.rssrc/ui/identities/mod.rs
|
Superseded by #750 which includes this fix plus suppresses the error on startup auto-select. |
Summary
When an identity lacks an authentication key for signing document transitions,
the error message now includes the identity name (or base58 ID if no alias is
set). This helps users with multiple identities identify which one has the
problem.
Closes #743
Changes
src/ui/identities/mod.rs— includequalified_identity.display_string()in the error from
get_selected_walletsrc/backend_task/identity/register_dpns_name.rs— same fix in the DPNSname registration path
Validation
cargo clippy— no warningscargo fmt --all -- --check— cleancargo check— compiles successfullySummary by CodeRabbit