-
Notifications
You must be signed in to change notification settings - Fork 12
fix(ui): improve auth key error message and suppress on startup auto-select #761
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: v1.0-dev
Are you sure you want to change the base?
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -77,11 +77,10 @@ impl QRCodeGeneratorScreen { | |
| new_self.selected_identity_string = | ||
| identities[0].identity.id().to_string(Encoding::Base58); | ||
|
|
||
| // Get wallet for the selected identity | ||
| // Get wallet for the selected identity (don't show error on auto-select; | ||
| // some identities like evonodes may lack document signing keys) | ||
| new_self.selected_wallet = | ||
| get_selected_wallet(&identities[0], Some(&app_context), None) | ||
| .or_show_error(app_context.egui_ctx()) | ||
| .unwrap_or(None); | ||
| get_selected_wallet(&identities[0], Some(&app_context), None).unwrap_or(None); | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fair point in theory, but in practice the only other error path here is DPNS preorder document type not found — which means the DPNS contract is broken/missing, and the app would fail in many other places too. Selectively filtering error types here adds complexity for a scenario that's already catastrophic. The current approach keeps the auto-select path clean.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in a6bd8f3 — startup auto-select now suppresses only the expected missing document-signing-key error and surfaces other get_selected_wallet errors with a banner. |
||
| } | ||
|
|
||
| new_self | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -78,8 +78,17 @@ pub fn get_selected_wallet( | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| qualified_identity | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .document_signing_key(&preorder_document_type) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .ok_or_else(|| { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Identity doesn't have an authentication key for signing document transitions" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .to_string() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| use dash_sdk::dpp::identity::accessors::IdentityGettersV0; | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| let identity_label = qualified_identity | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .alias | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .as_deref() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .or_else(|| qualified_identity.dpns_names.first().map(|n| n.name.as_str())) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| .unwrap_or("unknown"); | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| format!( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| "Identity '{}' ({}) doesn't have an authentication key for signing document transitions", | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. claw remove quotes, and when no name found, put empty string. We will just have a few spaces when identity label is not availble.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done — removed quotes and switched the fallback to empty string. 83f602c
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in 83f602c — removed quotes and switched to empty string when no name is found.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done in a6bd8f3 — preserved the no-quotes formatting and empty-string fallback after the amended update. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| identity_label, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| qualified_identity.identity.id() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+95
to
+112
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The Display impl for Identifier outputs Base58 — same encoding used elsewhere in the UI. No mismatch here.
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed in a6bd8f3 — the identity id is now formatted explicitly with Encoding::Base58. |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| })? | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+97
to
113
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 💬 Nitpick: Empty identity label produces a double space in the rendered message When 💡 Suggested change
Suggested change
source: ['claude'] |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } else { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| // Fallback: directly use the provided selected key. | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same rationale as the QRCodeGenerator comment above — the only other error is a missing DPNS contract, which is a global failure, not something worth special-casing here.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fixed in a6bd8f3 — same selective handling as QRCodeGenerator: only the missing signing-key case is suppressed, other errors are shown.