diff --git a/CHANGELOG.md b/CHANGELOG.md index 1a5cda0ff..7cada13c2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,6 +94,54 @@ The format is based on [Keep a Changelog](https://keepachangelog.com/en/1.1.0/). ### Fixed +- **An identity's keys are reachable again**: the keys list under an identity's + Settings → Advanced now opens each key's own page, so keys can be inspected + and restored — and, once a key is on this device, signed with or + password-protected — without changing the interface mode and without starting + a payment. Previously that list was a read-only table with no way onward, and + every route to a key's page ran through an action screen — sending, + withdrawing, a token operation — each of which offers it only when the + identity already holds a key of the kind that action needs. So an identity + missing its keys, the one case where this matters most, could not get to them + at all. The offer to restore keys left behind by an earlier version now also + appears on the keys list itself, above the keys, rather than only inside a + key's page. Each key is named by its role and states whether it is saved on + this device. Keys are named for the identity they belong to: a user identity's + keys are described in plain language rather than in masternode registration + terms, which previously appeared on every identity. Leaving a key returns to + the list with both its keys and the restore offer brought up to date, so a + restore made from a key's page is reflected immediately instead of being + offered again. + + A key opened from a masternode's page keeps its name too. A voting key is the + node's voting key however it is recorded, and its own page now says so instead + of describing it as another kind of key, which also means the page no longer + reports such a key as missing while the list it was opened from shows it as + saved on this device. + + One known limitation, for a voting key stored on the identity itself rather + than on a separate voting identity: the keys list and the key's page now agree + on whether such a key is saved here, but saving or removing one by hand can + affect a voting key of the same number on a linked voting identity, and + removing it may leave the original in place. So until then, after saving or + removing a voting key on an identity like that, open the keys list and check + that each key still reads as you expect, and re-enter any key that should be + saved but no longer is. This will be closed by the in-progress key-placement + resolution fix. + +- **A key's page now catches up on changes made while it was open**: previously, + if something else updated your identity while a key's page was open — most + relevantly, a restore that finished from a different screen — the next key + edit made on that page could silently overwrite the change. The page now + picks up such changes as they arrive. + +- **Key role names are complete, consistent phrases everywhere**: a key's role + (owner, voting, payout, and so on) now reads the same complete phrase across + the keys list, a masternode's page, and the key's own page, instead of a + partly-assembled label that could vary by screen. The on-chain purpose value + itself remains available as its own line in Expert view for anyone who wants + it verbatim. + - **Wallet rename consistency**: renaming a wallet no longer overwrites other saved wallet details when metadata cannot be read. Overlapping renames and wallet removals also keep displayed aliases and deleted-wallet metadata diff --git a/docs/ai-design/2026-07-28-legacy-identity-recovery/design.md b/docs/ai-design/2026-07-28-legacy-identity-recovery/design.md index 379ac58d0..e61ef15fa 100644 --- a/docs/ai-design/2026-07-28-legacy-identity-recovery/design.md +++ b/docs/ai-design/2026-07-28-legacy-identity-recovery/design.md @@ -792,3 +792,33 @@ the eager vault migration inside the read path — a caller already holding the guard reaches it, so taking it there would self-deadlock. That write is idempotent (it replaces plaintext with vault placeholders in the blob it just read) and carries a TODO to fold it into the guarded path. + +### 10.14 The Key Info offer needed a route before it was reachable at all + +§2.1 named the Key Info screen as reachable "from the Identities screen for +`User` identities and via 'Manage keys' for nodes", and §7 row 1 called the +entry point discoverable. Both were false for `User` identities as shipped. + +The legacy `IdentitiesScreen` per-key popup is the route §2.1 means, and +`ui/components/left_panel.rs` deliberately drops `RootScreenIdentities` from the +nav, so nothing navigates to it. The identity hub's replacement surface — +Settings → Advanced → "Manage keys" — opened a read-only key table with no way +into `KeyInfoScreen`. Every remaining route (transfer, withdraw, the token +screens) is gated on the identity already holding a key of the kind that action +needs, which is exactly false for the identities this flow exists to help. A +`User` identity with stranded keys could therefore reach the offer only in +Developer view, through a send-money screen. + +§2.1 and §7 row 1 are both left as written, per this document's model — the +proposal stands as proposed and this section is the correction. A third surface +now carries the offer that §2.1 names two for, and the mechanism §7 row 1 calls +an "on-arrival" detection task is the `ensure_checked()` render-loop latch of +§10.1. + +Shipped: `KeysScreen` carries the `QualifiedIdentity`, renders one row per key +in the §10.8 vocabulary with its held state in words, and opens +`KeyInfoScreen` for any key regardless of what the device holds. The offer +itself renders on that list, above the rows, because it is identity-scoped — +requiring a user to pick an arbitrary key to discover an identity-level offer +repeats the defect one level up. The offer stays on `KeyInfoScreen` too: it +self-extinguishes, and the masternode path lands there. diff --git a/docs/user-stories.md b/docs/user-stories.md index 911f350a0..eace4f72c 100644 --- a/docs/user-stories.md +++ b/docs/user-stories.md @@ -570,12 +570,16 @@ As a power user, I want to add a new key to my identity so that I can authorize - Key is added via state transition. ### IDN-008: View identity keys and details [Implemented] -**Persona:** Priya, Jordan +**Persona:** Alex, Priya, Jordan As a user, I want to view all keys associated with my identity so that I can audit access and verify key configuration. -- Lists all keys with type, purpose, and status. -- View individual key details. +- The list is reachable from the identity's own Settings tab, under the Advanced section, without changing the interface mode and without starting a payment. +- Every key on the identity gets a row, and every row opens that key's own page — whether or not this device holds the key's private half. A key the device is missing is exactly the one a user comes here about, so it is never hidden or unopenable. +- Each row names the key by its role in words that suit the identity — a user identity's keys in plain language, a masternode's in its registration terms — and says whether the key is saved on this device, in words rather than by colour alone. A key the network has retired says so in its name. +- The on-chain specifics — key id, Platform purpose, security level, key type, read-only — are Expert-view detail, on both the list and the key's own page. The Everyday view gets the role and the held state, which is what it can act on. +- A key's own page shows its public key, hash and address, and offers to view the private half, sign a message with it, or add and remove it. +- The offer to restore keys an upgrade left behind also appears here; its criteria are IDN-020's. ### IDN-013: Password-protect an identity's signing keys (SEC-001) [Implemented] **Persona:** Priya, Jordan @@ -670,8 +674,12 @@ As a user, I want the identities I loaded before an upgrade — and the keys the As a user whose identity was already in the app before the upgrade — a masternode loaded from its ProTxHash, or an identity holding only some of its keys — I want to bring across the keys that stayed behind in the previous version's data, so that I do not have to re-enter private keys I no longer have on hand. -- The offer appears on the identity's own page — the node detail page and the Key Info screen — only when the previous version's data actually holds keys this identity does not, and it disappears once there is nothing left to restore. -- The list names each key by its role and is exactly what gets restored. Nothing is restored without pressing Restore, and nothing happens automatically at launch or during the upgrade. +- The offer appears on the identity's own page — the node detail page, the identity's keys list, and the Key Info screen — only when the previous version's data actually holds keys this identity does not, and it disappears once there is nothing left to restore. +- The keys list is reachable without changing the interface mode and without starting a payment: an identity's keys are listed under Settings, and every key opens its own page whether or not this device holds that key. Keys that are missing are exactly the ones a user comes here about, so a key the device does not hold is never hidden or unopenable. +- The offer sits above the key list, so a user whose keys are missing finds it without opening a key first. +- Each key is named by its role and says whether it is saved on this device, in words rather than by colour alone. The role words match the identity: a user identity's keys are named in plain language, not in masternode registration terms, and the restore offer above the list uses the same words as the list itself. +- Leaving a key returns to the keys list with both the keys and the restore offer up to date, so a restore made from a key's page is never offered a second time. +- The restore offer lists each key by its role, and that list is exactly what gets restored. Nothing is restored without pressing Restore, and nothing happens automatically at launch or during the upgrade. - Keys already saved for the identity are never replaced or removed — only missing ones are added. An identity the user deleted is never brought back. - On a password-protected identity the identity password is asked for first; cancelling, or getting it wrong, leaves everything exactly as it was. - An owner or payout key that is genuinely missing comes back this way. A voting key held on a separate voting identity does not, unless the identity's own record already links to that voting identity: otherwise nothing but the previous version's data says the node still uses that key, so it is listed as one that cannot be brought back and entering it by hand stays the remedy. Checking such a key against the chain, which would let it be restored safely, is tracked as issue #942. diff --git a/src/ui/components/legacy_recovery_section.rs b/src/ui/components/legacy_recovery_section.rs index fc93c857c..1b0286746 100644 --- a/src/ui/components/legacy_recovery_section.rs +++ b/src/ui/components/legacy_recovery_section.rs @@ -14,7 +14,8 @@ use crate::model::legacy_recovery::{ ExclusionReason, RecoveryItem, RecoveryItemDescriptor, RecoveryPlan, }; use crate::ui::components::component_trait::{Component, ComponentResponse}; -use crate::ui::masternodes::{disambiguate_role_labels, role_label_and_tip}; +use crate::ui::masternodes::{KeyVocabulary, disambiguate_role_labels, role_label_and_tip}; +use crate::ui::state::legacy_recovery::LegacyRecoveryState; use crate::ui::theme::DashColors; /// Section lead-in. Avoids "migration", "blob" and "vault" — the user knows @@ -95,17 +96,22 @@ pub fn exclusion_explanation(reason: ExclusionReason) -> &'static str { /// item is named here exactly as the "Manage keys" list names the same key, and /// two rows that would otherwise read alike are told apart by their key id. pub fn recovery_item_labels( + vocabulary: KeyVocabulary, items: &[&RecoveryItemDescriptor], ) -> Vec<(String, Option<&'static str>)> { let mut labels: Vec<(String, Option<&'static str>)> = items .iter() .map(|item| match &item.item { RecoveryItem::Key { .. } => { - let (role, tip) = role_label_and_tip( + let (label, tip) = role_label_and_tip( + vocabulary, item.is_on_voter_identity(), item.purpose.unwrap_or(Purpose::AUTHENTICATION), + // The previous version's data records no retired state, and + // a key still restorable is one the identity still uses. + false, ); - (format!("{role} key"), tip) + (label.to_string(), tip) } RecoveryItem::VoterAssociation => ("Voting identity link".to_string(), None), RecoveryItem::OperatorAssociation => ("Operator identity link".to_string(), None), @@ -155,14 +161,23 @@ impl ComponentResponse for LegacyRecoverySectionResponse { pub struct LegacyRecoverySection<'a> { plan: &'a RecoveryPlan, restoring: bool, + vocabulary: KeyVocabulary, } impl<'a> LegacyRecoverySection<'a> { - /// The offer for `plan`. - pub fn new(plan: &'a RecoveryPlan) -> Self { + /// The offer for `plan`, naming its keys in `vocabulary`. + /// + /// `vocabulary` is an argument rather than a builder option on purpose. This + /// offer is hosted by three separate screens, and a default would let a + /// fourth inherit the wrong wording silently: naming a user identity's + /// transfer key a "payout address key" asserts it owns a masternode, and + /// disagrees with the keys list rendered right below the offer. Required + /// here, every host has to answer for the identity it is showing. + pub fn new(plan: &'a RecoveryPlan, vocabulary: KeyVocabulary) -> Self { Self { plan, restoring: false, + vocabulary, } } @@ -174,6 +189,39 @@ impl<'a> LegacyRecoverySection<'a> { } } +/// Render `state`'s offer for a hosting screen, returning the items the user +/// approved this frame. Renders nothing at all when detection found nothing, so +/// the offer appears only where it has something to say and retires itself once +/// a restore lands. +/// +/// The whole of what a host owes the offer's *rendering*, so no host has to +/// restate it. Three screens host this one offer, and each restatement is a +/// place the next one can drift: naming a key differently, forgetting the +/// in-flight guard that stops a restore being dispatched twice, or showing an +/// empty section. A host supplies only the identity's `vocabulary` and whatever +/// separators frame it — see [`LegacyRecoveryState::has_offer`] for deciding +/// whether to draw those. +/// +/// `vocabulary` stays an argument for the reason [`LegacyRecoverySection::new`] +/// gives: naming a user identity's transfer key a "payout address key" asserts +/// it owns a masternode. +pub fn host_offer( + state: &LegacyRecoveryState, + vocabulary: KeyVocabulary, + ui: &mut Ui, +) -> Option> { + let restoring = state.is_restoring(); + let plan = state.plan().filter(|plan| !plan.is_empty())?; + // `changed_value` hands back a reference into the response, so the approved + // set has to be cloned out before the response is dropped. + LegacyRecoverySection::new(plan, vocabulary) + .restoring(restoring) + .show(ui) + .inner + .changed_value() + .clone() +} + impl Component for LegacyRecoverySection<'_> { type DomainType = Vec; type Response = LegacyRecoverySectionResponse; @@ -196,7 +244,7 @@ impl Component for LegacyRecoverySection<'_> { ui.label(RichText::new(intro).color(DashColors::warning_color(dark_mode))); let previewed = self.plan.preview_items(); - for (label, tip) in recovery_item_labels(&previewed) { + for (label, tip) in recovery_item_labels(self.vocabulary, &previewed) { ui.horizontal(|ui| { ui.add_space(12.0); let row = ui @@ -238,8 +286,9 @@ impl Component for LegacyRecoverySection<'_> { let excluded: Vec<&RecoveryItemDescriptor> = self.plan.excluded.iter().map(|(item, _)| item).collect(); let reasons = self.plan.excluded.iter().map(|(_, reason)| *reason); - for ((label, _), reason) in - recovery_item_labels(&excluded).into_iter().zip(reasons) + for ((label, _), reason) in recovery_item_labels(self.vocabulary, &excluded) + .into_iter() + .zip(reasons) { ui.horizontal(|ui| { ui.add_space(12.0); diff --git a/src/ui/identities/keys/key_info_screen.rs b/src/ui/identities/keys/key_info_screen.rs index f919f0072..9862f3a11 100644 --- a/src/ui/identities/keys/key_info_screen.rs +++ b/src/ui/identities/keys/key_info_screen.rs @@ -13,16 +13,17 @@ use crate::model::secret::Secret; use crate::model::wallet::Wallet; use crate::model::wallet::passphrase::validate_single_key_passphrase; use crate::ui::components::MessageBanner; -use crate::ui::components::component_trait::{Component, ComponentResponse}; +use crate::ui::components::component_trait::Component; use crate::ui::components::info_popup::InfoPopup; use crate::ui::components::left_panel::add_left_panel; -use crate::ui::components::legacy_recovery_section::{LegacyRecoverySection, completion_message}; +use crate::ui::components::legacy_recovery_section::host_offer; use crate::ui::components::password_input::PasswordInput; use crate::ui::components::styled::{ConfirmationDialog, ConfirmationStatus, island_central_panel}; use crate::ui::components::top_panel::add_top_panel; use crate::ui::components::wallet_unlock_popup::{ WalletUnlockPopup, WalletUnlockResult, try_open_wallet_no_password, wallet_needs_unlock, }; +use crate::ui::masternodes::{KeyVocabulary, key_role_label}; use crate::ui::state::legacy_recovery::LegacyRecoveryState; use crate::ui::theme::DashColors; use crate::ui::{MessageType, ScreenLike}; @@ -113,6 +114,11 @@ pub struct KeyInfoScreen { recovery: LegacyRecoveryState, /// A queued restore (the approved items), drained in `ui()`. pending_recovery_restore: Option>, + /// Which of the identity's key stores this key is filed under, when the + /// caller knew. See [`Self::target`] for why that beats deriving it. + target: Option, + /// The screen this key was opened from, for the breadcrumb back to it. + parent: Option<&'static str>, } /// At-rest protection posture of an identity's vault-stored keys. @@ -147,8 +153,6 @@ enum ProtectionStage { } impl ScreenLike for KeyInfoScreen { - fn refresh(&mut self) {} - /// Re-read the record this screen persists, because another writer may have /// changed it while the screen sat in the stack. /// @@ -158,7 +162,14 @@ impl ScreenLike for KeyInfoScreen { /// clone taken when it opened, so a clone that missed a write puts the /// pre-write record back on the next key edit. The masternode detail view /// re-reads on arrival for the same reason. - fn refresh_on_arrival(&mut self) { + /// + /// This is the hook that has to carry it: `AppState` dispatches + /// `refresh_on_arrival` only to root screens, and this screen is always + /// pushed onto the screen stack. What reaches a pushed screen is `refresh` — + /// from `TaskResult::Refresh`, from `AppAction::Refresh`, and from the + /// `PopScreenAndRefresh` that reveals it. `refresh_on_arrival` defaults to + /// delegating here, so both hooks run this. + fn refresh(&mut self) { self.reload_identity(); self.protection_status = None; self.recovery.completed(); @@ -223,34 +234,20 @@ impl ScreenLike for KeyInfoScreen { MessageType::Success, ); } - BackendTaskSuccessResult::LegacyRecoveryCandidates { identity_id, plan } => { - self.recovery.offered(identity_id, plan); - } - BackendTaskSuccessResult::LegacyRecoveryCompleted { - identity_id, - ref applied, - .. - } => { - // A restore this screen never dispatched can land here, since - // results reach whichever screen is visible when they arrive. - // Only this identity's own restore wrote the record the clone - // below is refreshed from, re-sealed the keys the protection - // line reads, or has anything to say to this user. - if self.recovery.completed_for(identity_id) { - // The clone this screen persists on every key edit is now - // stale — writing it back would erase the keys just - // restored. Restored keys also land in the vault, so the - // protection line has to re-read it. + ref result => { + // The offer attributes the result, re-arms itself and reports + // the outcome; what is left is this screen's own. The clone it + // persists on every key edit is now stale — writing it back + // would erase the keys just restored — and restored keys land + // in the vault, so the protection line has to re-read it. + if self + .recovery + .absorb_result(self.app_context.egui_ctx(), result) + { self.reload_identity(); self.protection_status = None; - MessageBanner::set_global( - self.app_context.egui_ctx(), - completion_message(!applied.is_empty()), - MessageType::Success, - ); } } - _ => {} } } @@ -264,30 +261,14 @@ impl ScreenLike for KeyInfoScreen { } } - /// End a recovery operation only when the failure is that operation's. - /// - /// Every failing task routed to the visible screen arrives here, so the - /// identity the error's operation names is what tells this screen's own - /// check or restore from an unrelated failure that merely landed while the - /// screen was open. fn display_backend_task_error(&mut self, context: &BackendTaskContext, _error: &TaskError) { - if let Some(identity_id) = context.legacy_recovery_identity() { - self.recovery.failed_for(identity_id); - } + self.recovery.absorb_error(context); } fn ui(&mut self, ui: &mut egui::Ui) -> AppAction { let ctx = ui.ctx().clone(); let ctx = &ctx; - let mut action = add_top_panel( - ui, - &self.app_context, - vec![ - ("Identities", AppAction::GoToMainScreen), - ("Key Info", AppAction::None), - ], - vec![], - ); + let mut action = add_top_panel(ui, &self.app_context, self.breadcrumb(), vec![]); action |= add_left_panel( ui, @@ -313,13 +294,42 @@ impl ScreenLike for KeyInfoScreen { ui.label(RichText::new(format!("{}", self.key.id())).color(text_primary)); ui.end_row(); - // Purpose + // Purpose, in the same words the keys list and the + // restore offer use for this key — one key cannot be + // called three things across three screens. ui.label(RichText::new("Purpose:").strong().color(text_primary)); - ui.label( - RichText::new(format!("{:?}", self.key.purpose())).color(text_primary), + let (role, role_tip) = key_role_label( + KeyVocabulary::from(self.identity.identity_type), + &self.target(), + &self.key, ); + let purpose_label = ui.label(RichText::new(role).color(text_primary)); + if let Some(tip) = role_tip { + purpose_label.on_hover_text(tip); + } ui.end_row(); + // The raw Platform purpose is Expert diagnostics, so it + // gets a labelled field of its own beside Security Level + // and Type rather than being spliced into the caption + // above, which has to stay one translatable phrase. + if self + .app_context + .user_role() + .at_least(crate::model::user_role::UserRole::Power) + { + ui.label( + RichText::new("Platform purpose:") + .strong() + .color(text_primary), + ); + ui.label( + RichText::new(format!("{:?}", self.key.purpose())) + .color(text_primary), + ); + ui.end_row(); + } + // Security Level ui.label( RichText::new("Security Level:") @@ -750,7 +760,7 @@ impl ScreenLike for KeyInfoScreen { // Vault-backed (InVault) identity-key requests: the raw key is fetched // JIT in the backend and only the public WIF / signature returns. let identity_id = self.identity.identity.id(); - let target: PrivateKeyTarget = self.key.purpose().into(); + let target: PrivateKeyTarget = self.target(); let key_id = self.key.id(); if std::mem::take(&mut self.pending_identity_key_display) { action |= AppAction::BackendTask(BackendTask::WalletTask( @@ -872,9 +882,74 @@ impl KeyInfoScreen { pending_unprotect: None, recovery, pending_recovery_restore: None, + target: None, + parent: None, + } + } + + /// Name the screen this key was opened from, so the breadcrumb can lead back + /// to it. + /// + /// Unset leaves the two-level `Identities > Key Info` trail every other + /// caller has: the screen has nine parents, and naming one of them for all + /// of them would simply mislabel the other eight. + pub fn with_parent(mut self, label: &'static str) -> Self { + self.parent = Some(label); + self + } + + /// The breadcrumb trail, with the parent screen in it when the caller named + /// one. The parent crumb pops back rather than clearing the stack, so the + /// screen underneath is the one the user actually came from. + fn breadcrumb(&self) -> Vec<(&'static str, AppAction)> { + match self.parent { + Some(parent) => vec![ + ("Identities", AppAction::GoToMainScreen), + (parent, AppAction::PopScreenAndRefresh), + ("Key Info", AppAction::None), + ], + None => vec![ + ("Identities", AppAction::GoToMainScreen), + ("Key Info", AppAction::None), + ], } } + /// Record which key store this key is filed under, for a caller that already + /// resolved it from the identity the key was listed from. + /// + /// Prefer this wherever the target is known. Without it the screen falls + /// back to deriving the target from the key's purpose, which cannot + /// distinguish a voting key filed on the main identity from one on a voter + /// identity — see [`Self::target`]. + pub fn with_target(mut self, target: PrivateKeyTarget) -> Self { + self.target = Some(target); + self + } + + /// The key store this key is filed under, for **reads**: what the caller + /// resolved, else derived from the key's purpose. + /// + /// Reads only, and deliberately. Looking somewhere else can only find + /// material that is already there, so it is safe today; *writing* somewhere + /// else would put material where `QualifiedIdentity::sign` never looks. The + /// write and remove paths therefore keep the derived target until every + /// reader is migrated together. + /// + /// The derivation is lossy and cannot be made otherwise: + /// `impl From for PrivateKeyTarget` sends every `Purpose::VOTING` + /// key to the voter identity, but a voting-purpose key filed on the main + /// identity is a supported shape (`masternode_key_presence` reads it as + /// voting readiness on its own). For that key the derivation names a store + /// it was never filed under, so a read misses it and a write or delete + /// lands on a different key that happens to share its id. Only the caller + /// that walked the identity knows which store it came from. + fn target(&self) -> PrivateKeyTarget { + self.target + .clone() + .unwrap_or_else(|| self.key.purpose().into()) + } + /// Re-read this screen's identity from the store, after a backend task /// wrote it. /// @@ -890,10 +965,7 @@ impl KeyInfoScreen { Ok(Some(fresh)) => { self.private_key_data = fresh .private_keys - .get_cloned_private_key_data_and_wallet_info(&( - self.key.purpose().into(), - self.key.id(), - )); + .get_cloned_private_key_data_and_wallet_info(&(self.target(), self.key.id())); self.identity = fresh; } Ok(None) => {} @@ -908,27 +980,21 @@ impl KeyInfoScreen { } } - /// Render the offer to restore this identity's keys from the previous - /// version's saved data, queueing the approved items for dispatch. Renders - /// nothing when detection found nothing, so the section appears only where - /// it has something to say and retires itself once a restore lands. + /// Render the offer, queueing the approved items for dispatch. The rule + /// above it is this screen's own: the offer arrives after the key's details + /// and has to be told apart from them. fn render_recovery_section(&mut self, ui: &mut egui::Ui) { - let restoring = self.recovery.is_restoring(); - let Some(plan) = self.recovery.plan().filter(|plan| !plan.is_empty()) else { + if !self.recovery.has_offer() { return; - }; + } ui.add_space(10.0); ui.separator(); ui.add_space(10.0); - let approved = LegacyRecoverySection::new(plan) - .restoring(restoring) - .show(ui) - .inner - .changed_value() - .clone(); - if approved.is_some() { - self.pending_recovery_restore = approved; - } + self.pending_recovery_restore = host_offer( + &self.recovery, + KeyVocabulary::from(self.identity.identity_type), + ui, + ); } /// Build a key-info screen with the add-protection confirmation already open @@ -999,6 +1065,11 @@ impl KeyInfoScreen { } else if validation_result.expect("invariant: Err handled in the preceding branch") { // If valid, store the private key in the context and reset the input field self.private_key_data = Some((PrivateKeyData::Clear(private_key_bytes), None)); + // Deliberately the purpose-derived target, not `self.target()`: + // `QualifiedIdentity::sign` and `can_sign_with` look the key up that + // way, so material stored anywhere else is material that can never + // sign. Writing the resolved target instead needs every reader + // migrated with it — see the reconciliation TODO. self.identity.private_keys.insert_non_encrypted( (self.key.purpose().into(), self.key.id()), (self.key.clone().into(), private_key_bytes), @@ -1201,6 +1272,12 @@ impl KeyInfoScreen { self.remove_private_key_dialog = None; if result == ConfirmationStatus::Confirmed { self.private_key_data = None; + // The purpose-derived target for the same reason as the + // write above: this has to remove the entry the rest of the + // app would have used. That it can therefore remove a + // different key's material, when a voter identity carries + // the same key id, is the known residual of the split + // conventions — the reconciliation TODO owns it. self.identity .private_keys .private_keys @@ -1711,53 +1788,6 @@ mod tests { .await; } - /// A restore that lands while this screen is off-screen never reaches its - /// `display_task_result` — results go only to the visible screen. Returning - /// to it must re-read the record, or the clone it opened with is written - /// back over the restored keys by the next ordinary key edit, silently and - /// with no error to show for it. - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] - async fn a_restore_that_landed_off_screen_survives_the_next_key_edit() { - let (app_context, _dir) = offline_ctx().await; - - let on_screen_key = public_key(1, Purpose::AUTHENTICATION); - let stored = identity_with(0x4E, &[(on_screen_key.clone(), [0x11; 32])]); - let identity_id = stored.identity.id(); - app_context - .insert_local_qualified_identity(&stored, &None) - .expect("insert the record"); - let mut screen = KeyInfoScreen::new(stored, on_screen_key, None, &app_context); - - // The restore lands while another screen is the visible one, so this - // screen is never told about it. - let restored_key = public_key(2, Purpose::TRANSFER); - write_key_behind_the_screen(&app_context, identity_id, &restored_key, [0x22; 32]); - - screen.refresh_on_arrival(); - - // What every key add and remove on this screen does with its clone. - app_context - .update_local_qualified_identity(&screen.identity) - .expect("the next key edit's write"); - - assert!( - app_context - .get_local_qualified_identity(&identity_id) - .expect("read back") - .expect("still stored") - .private_keys - .private_keys - .contains_key(&(MAIN, restored_key.id())), - "a key edit on this screen must not erase keys restored while it was away", - ); - - app_context - .wallet_backend() - .expect("backend") - .shutdown() - .await; - } - /// A restore dispatched from one identity's Key Info screen can complete /// after the user has opened another's, and results reach whichever screen /// is visible. The stray completion must touch nothing here: not the clone, diff --git a/src/ui/identities/keys/keys_screen.rs b/src/ui/identities/keys/keys_screen.rs index 4c146ff95..e3c3c2104 100644 --- a/src/ui/identities/keys/keys_screen.rs +++ b/src/ui/identities/keys/keys_screen.rs @@ -1,59 +1,281 @@ +//! The identity keys list — every key of one identity, and the way into each. +//! +//! This is the only route into [`KeyInfoScreen`] that does not depend on the +//! identity already holding a usable key. Every other route reaches it from an +//! action (transfer, withdraw, a token operation) gated on holding the key that +//! action needs, which is false for precisely the identities the restore offer +//! below exists to help. Keeping this list ungated is the point of it. + use crate::app::AppAction; +use crate::backend_task::error::TaskError; +use crate::backend_task::{BackendTaskContext, BackendTaskSuccessResult}; use crate::context::AppContext; -use crate::ui::ScreenLike; -use dash_sdk::dpp::identity::Identity; +use crate::model::legacy_recovery::RecoveryItem; +use crate::model::qualified_identity::QualifiedIdentity; +use crate::model::user_role::UserRole; +use crate::ui::components::MessageBanner; +use crate::ui::components::left_panel::add_left_panel; +use crate::ui::components::legacy_recovery_section::host_offer; +use crate::ui::components::styled::island_central_panel; +use crate::ui::components::top_panel::add_top_panel; +use crate::ui::identities::keys::key_info_screen::KeyInfoScreen; +use crate::ui::masternodes::{KeyVocabulary, identity_keys, key_filed_at, manage_keys_labels}; +use crate::ui::state::legacy_recovery::LegacyRecoveryState; +use crate::ui::theme::{ComponentStyles, DashColors, ResponseExt}; +use crate::ui::{MessageType, RootScreenType, Screen, ScreenLike}; use dash_sdk::dpp::identity::accessors::IdentityGettersV0; use dash_sdk::dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; -use eframe::egui::{self}; +use dash_sdk::platform::IdentityPublicKey; +use eframe::egui::{self, RichText, ScrollArea}; use std::sync::Arc; +/// The crumb this screen contributes to a pushed Key Info screen's breadcrumb. +const PARENT_CRUMB: &str = "Keys"; + +/// Shown for a key whose private half this install holds. +const HELD: &str = "This key is saved on this device."; +/// Shown for a key this install has only the public half of. The one fact a +/// user with stranded keys came here to find, so it is stated in words rather +/// than signalled by colour alone. +const NOT_HELD: &str = "This key is not saved on this device."; + pub struct KeysScreen { - pub identity: Identity, + pub identity: QualifiedIdentity, pub app_context: Arc, + /// The offer to restore keys this identity left behind in the previous + /// version's saved data (issue #889). Scoped to the identity, so it belongs + /// on the list rather than inside any one key. + recovery: LegacyRecoveryState, + /// A queued restore (the approved items), drained in `ui()`. + pending_recovery_restore: Option>, } impl ScreenLike for KeysScreen { - fn refresh(&mut self) {} + /// Re-read the record *and* re-arm detection. + /// + /// Both, or the offer goes stale: a restore run from the Key Info screen + /// pushed on top of this one writes the record behind this screen's back, so + /// returning to it with only `reload_identity` shows the restored keys as + /// held while still offering to restore them — and pressing Restore then + /// reports there was nothing left to do. Re-arming is cheap and self-guards + /// on an install with nothing to read. + /// + /// This is the only arrival hook that runs: the framework dispatches + /// `refresh_on_arrival` for root screens, and `refresh` for the screen a + /// `PopScreenAndRefresh` reveals — which is what this screen is. + fn refresh(&mut self) { + self.reload_identity(); + self.recovery.completed(); + } + + /// Re-read the list when this identity's own restore lands. The offer + /// handles the rest — attribution, re-arming, and telling the user. + fn display_task_result(&mut self, backend_task_success_result: BackendTaskSuccessResult) { + if self + .recovery + .absorb_result(self.app_context.egui_ctx(), &backend_task_success_result) + { + self.reload_identity(); + } + } + + fn display_backend_task_error(&mut self, context: &BackendTaskContext, _error: &TaskError) { + self.recovery.absorb_error(context); + } fn ui(&mut self, ui: &mut egui::Ui) -> AppAction { - let mut action = AppAction::None; - egui::CentralPanel::default().show(ui, |ui| { + let mut action = add_top_panel( + ui, + &self.app_context, + vec![ + ("Identities", AppAction::GoToMainScreen), + ("Keys", AppAction::None), + ], + vec![], + ); + + // The hub is where this screen is opened from, and the only nav entry + // that leads back to it — `RootScreenIdentities` is no longer in the + // nav, so selecting it here would highlight nothing. + action |= add_left_panel(ui, &self.app_context, RootScreenType::RootScreenIdentityHub); + + action |= island_central_panel(ui, |ui| { + let mut inner_action = AppAction::None; + let dark_mode = ui.ctx().global_style().visuals.dark_mode; + ui.horizontal(|ui| { - if ui.button("Back").clicked() { - action = AppAction::PopScreen; + if ComponentStyles::add_secondary_button(ui, "Back", dark_mode) + .clickable_tooltip("Return to the identity you came from.") + .clicked() + { + inner_action |= AppAction::PopScreenAndRefresh; } - ui.heading("Identity Keys"); + ui.heading( + RichText::new("Identity Keys").color(DashColors::text_primary(dark_mode)), + ); }); ui.separator(); - egui::ScrollArea::vertical().show(ui, |ui| { - ui.horizontal(|ui| { - ui.label("Key ID"); - ui.label("Purpose"); - ui.label("Security Level"); - ui.label("Type"); - ui.label("Read Only"); - }); - for key in self.identity.public_keys().values() { - ui.horizontal(|ui| { - ui.label(format!("{}", key.id())); - ui.label(format!("{:?}", key.purpose())); - ui.label(format!("{:?}", key.security_level())); - ui.label(format!("{:?}", key.key_type())); - ui.label(format!("{}", key.read_only())); - }); - } + ScrollArea::vertical().show(ui, |ui| { + // Above the list: the offer is about the identity, not about + // any one key, and a user whose keys are missing must not have + // to read past the keys they do have to find the remedy. + self.render_recovery_section(ui); + inner_action |= self.render_key_list(ui, dark_mode); }); + + inner_action }); + + // The passive check goes out once per opened screen, a restore only + // after the user pressed Restore, so the two never contend for + // `action`, which keeps only its most recent value. + if let Some(task) = self.recovery.ensure_checked() { + action |= AppAction::BackendTask(task); + } + if let Some(approved) = self.pending_recovery_restore.take() + && let Some(task) = self.recovery.restore(approved) + { + action |= AppAction::BackendTask(task); + } + action } } impl KeysScreen { - pub fn new(identity: Identity, app_context: &Arc) -> Self { + pub fn new(identity: QualifiedIdentity, app_context: &Arc) -> Self { + let recovery = LegacyRecoveryState::new(app_context, identity.identity.id()); Self { identity, app_context: app_context.clone(), + recovery, + pending_recovery_restore: None, + } + } + + /// One row per key: what it is for, whether this device holds it, and the + /// way into its own page. Ungated — a key the device does not hold is + /// exactly the key a user comes here to do something about. + fn render_key_list(&self, ui: &mut egui::Ui, dark_mode: bool) -> AppAction { + let mut action = AppAction::None; + let keys = identity_keys(&self.identity); + + if keys.is_empty() { + ui.add_space(8.0); + // Not "no keys saved on this device": these rows come from the + // identity's on-chain public keys, and that phrase means held=false + // per-row on this same screen — a different statement entirely. + ui.label( + RichText::new("No keys were found for this identity.") + .color(DashColors::text_primary(dark_mode)), + ); + ui.label( + RichText::new("Refresh this identity to load its keys from Dash Platform.") + .small() + .color(DashColors::text_secondary(dark_mode)), + ); + return action; + } + + let expert = self.app_context.user_role().at_least(UserRole::Power); + let vocabulary = KeyVocabulary::from(self.identity.identity_type); + let labels = manage_keys_labels(vocabulary, &keys); + for ((target, key), (label, tip)) in keys.into_iter().zip(labels) { + let filed_at = key_filed_at(&self.identity, &target, &key); + let held = if filed_at.is_some() { HELD } else { NOT_HELD }; + ui.add_space(4.0); + ui.horizontal(|ui| { + let button = + ComponentStyles::add_secondary_button(ui, format!("{label} ›"), dark_mode); + let button = match tip { + Some(tip) => button.clickable_tooltip(tip), + None => button, + }; + if button.clicked() { + let opened_at = filed_at.clone().unwrap_or_else(|| target.clone()); + let holding = self + .identity + .private_keys + .get_cloned_private_key_data_and_wallet_info(&( + opened_at.clone(), + key.id(), + )); + action |= AppAction::AddScreen(Screen::KeyInfoScreen( + KeyInfoScreen::new( + self.identity.clone(), + key.clone(), + holding, + &self.app_context, + ) + // Where the material actually is, so the screen's own + // re-read finds the same key this row just reported. + .with_target(opened_at) + .with_parent(PARENT_CRUMB), + )); + } + ui.label(RichText::new(held).color(DashColors::text_secondary(dark_mode))); + }); + if expert { + Self::render_expert_detail(ui, &key, dark_mode); + } + } + action + } + + /// The on-chain specifics of one key, for the Expert view. Everyday view + /// gets the role word and held state, which is what it can act on. + fn render_expert_detail(ui: &mut egui::Ui, key: &IdentityPublicKey, dark_mode: bool) { + ui.label( + RichText::new(format!( + "Key {id} · {purpose:?} · {security:?} · {key_type:?}{read_only}", + id = key.id(), + purpose = key.purpose(), + security = key.security_level(), + key_type = key.key_type(), + read_only = if key.read_only() { " · read-only" } else { "" }, + )) + .small() + .monospace() + .color(DashColors::text_secondary(dark_mode)), + ); + } + + /// Render the offer above the key list, queueing what the user approved. + /// The separator is this screen's own: only here does the offer sit above + /// something it must be told apart from. + fn render_recovery_section(&mut self, ui: &mut egui::Ui) { + if !self.recovery.has_offer() { + return; + } + ui.add_space(8.0); + self.pending_recovery_restore = host_offer( + &self.recovery, + KeyVocabulary::from(self.identity.identity_type), + ui, + ); + ui.add_space(8.0); + ui.separator(); + } + + /// Re-read this screen's identity from the store after a backend task wrote + /// it, so the held/not-held column and the rows themselves reflect a + /// restore that has landed. A read failure leaves the clone alone and says + /// so — the change landed, this screen just cannot show it. + fn reload_identity(&mut self) { + let identity_id = self.identity.identity.id(); + match self.app_context.get_local_qualified_identity(&identity_id) { + Ok(Some(fresh)) => self.identity = fresh, + Ok(None) => {} + Err(error) => { + MessageBanner::set_global( + self.app_context.egui_ctx(), + "This identity's keys could not be reloaded. Close this list and open it \ + again to see them.", + MessageType::Error, + ) + .with_details(error); + } } } } diff --git a/src/ui/identities/transfer_screen.rs b/src/ui/identities/transfer_screen.rs index 582c5550c..894432234 100644 --- a/src/ui/identities/transfer_screen.rs +++ b/src/ui/identities/transfer_screen.rs @@ -16,6 +16,7 @@ use crate::ui::components::styled::island_central_panel; use crate::ui::components::top_panel::add_top_panel; use crate::ui::components::{BannerHandle, MessageBanner, OptionBannerExt, ResultBannerExt}; use crate::ui::identities::keys::key_info_screen::KeyInfoScreen; +use crate::ui::identities::keys::keys_screen::KeysScreen; use crate::ui::{MessageType, Screen, ScreenLike}; use dash_sdk::dashcore_rpc::dashcore::Address; use dash_sdk::dashcore_rpc::dashcore::address::NetworkUnchecked; @@ -647,12 +648,28 @@ impl ScreenLike for TransferScreen { ); ui.add_space(10.0); - if ui.button("Add key").clicked() { - inner_action |= AppAction::AddScreen(Screen::AddKeyScreen(AddKeyScreen::new( - self.identity.clone(), - &self.app_context, - ))); - } + ui.horizontal(|ui| { + if ui.button("Add key").clicked() { + inner_action |= AppAction::AddScreen(Screen::AddKeyScreen( + AddKeyScreen::new(self.identity.clone(), &self.app_context), + )); + } + // Entering a key by hand is not the only remedy: a key this + // identity left behind in the previous version's data can be + // brought across from the keys list. The gate above is + // untouched — it still correctly says this identity cannot + // send; the branch just stops being a dead end. + if ui + .button("Manage keys") + .clickable_tooltip("See every key of this identity and what it is for.") + .clicked() + { + inner_action |= AppAction::AddScreen(Screen::KeysScreen(KeysScreen::new( + self.identity.clone(), + &self.app_context, + ))); + } + }); } else { if let Some(key) = key_for_info { if ui.button("Manage Transfer Key").clicked() { diff --git a/src/ui/identity/settings.rs b/src/ui/identity/settings.rs index 320f3c9b5..0497cc6ed 100644 --- a/src/ui/identity/settings.rs +++ b/src/ui/identity/settings.rs @@ -966,7 +966,7 @@ impl SettingsTab { } fn keys_screen_type(identity: &QualifiedIdentity) -> ScreenType { - ScreenType::Keys(identity.identity.clone()) + ScreenType::Keys(identity.clone()) } fn usernames_screen_action() -> AppAction { @@ -1093,14 +1093,17 @@ mod tests { assert!(tab.selected_identity.is_none()); } + /// The keys screen has to receive the qualified record: without the private + /// key storage it cannot say which keys this device holds, and it would open + /// Key Info reporting every key as missing. #[test] - fn advanced_keys_action_opens_the_keys_screen() { + fn advanced_keys_action_opens_the_keys_screen_with_the_qualified_record() { let identity = qualified_identity(); assert!(matches!( keys_screen_type(&identity), ScreenType::Keys(screen_identity) - if screen_identity.id() == identity.identity.id() + if screen_identity.identity.id() == identity.identity.id() )); } diff --git a/src/ui/masternodes/detail_screen.rs b/src/ui/masternodes/detail_screen.rs index 7f97d1948..9672d2ccc 100644 --- a/src/ui/masternodes/detail_screen.rs +++ b/src/ui/masternodes/detail_screen.rs @@ -19,21 +19,21 @@ use std::collections::BTreeMap; use dash_sdk::dpp::voting::vote_choices::resource_vote_choice::ResourceVoteChoice; use crate::app::AppAction; -use crate::backend_task::BackendTask; use crate::backend_task::contested_names::ContestedResourceTask; use crate::backend_task::identity::{IdentityInputToLoad, IdentityLoadMode, IdentityTask}; +use crate::backend_task::{BackendTask, BackendTaskContext, BackendTaskSuccessResult}; use crate::context::AppContext; use crate::model::contested_name::{ContestedName, MasternodeContestSummary}; use crate::model::fee_estimation::format_credits_as_dash; -use crate::model::legacy_recovery::{RecoveryItem, RecoveryPlan}; +use crate::model::legacy_recovery::RecoveryItem; use crate::model::qualified_identity::{ IdentityType, MasternodeKeyPresence, PrivateKeyTarget, QualifiedIdentity, }; use crate::model::secret::Secret; use crate::ui::components::MessageBanner; -use crate::ui::components::component_trait::{Component, ComponentResponse}; +use crate::ui::components::component_trait::Component; use crate::ui::components::confirmation_dialog::{ConfirmationDialog, ConfirmationStatus}; -use crate::ui::components::legacy_recovery_section::LegacyRecoverySection; +use crate::ui::components::legacy_recovery_section::host_offer; use crate::ui::components::password_input::PasswordInput; use crate::ui::identities::keys::key_info_screen::KeyInfoScreen; use crate::ui::identity::identity_picker_card::draw_type_badge; @@ -41,7 +41,9 @@ use crate::ui::identity::identity_pill::shorten_id; use crate::ui::masternodes::card::{ PLATFORM_IDENTITY_STATUS_TOOLTIP, platform_identity_status_label, }; -use crate::ui::masternodes::{disambiguate_role_labels, key_status_tokens, role_label_and_tip}; +use crate::ui::masternodes::{ + KeyVocabulary, identity_keys, key_filed_at, key_status_tokens, manage_keys_labels, +}; use crate::ui::state::legacy_recovery::LegacyRecoveryState; use crate::ui::theme::{ComponentStyles, DashColors, ResponseExt}; use crate::ui::tokens::claim_tokens_screen::ClaimTokensScreen; @@ -123,47 +125,6 @@ fn contest_status_line(candidate_count: usize, end_time: Option /// The fixed top→bottom section order. Actions must precede Keys (TC-FR5-01). pub const SECTION_ORDER: [&str; 5] = ["Header", "Actions", "Keys", "DPNS", "Remove"]; -/// A short role name for a masternode key and its tooltip, from the shared -/// [`role_label_and_tip`] vocabulary. -fn key_role_label( - target: &PrivateKeyTarget, - key: &dash_sdk::platform::IdentityPublicKey, -) -> (String, Option<&'static str>) { - role_label_and_tip( - *target == PrivateKeyTarget::PrivateKeyOnVoterIdentity, - key.purpose(), - ) -} - -/// Button labels (and DIP-3-aligned tooltips) for the "Manage keys" list, one -/// per entry of `keys`, in order. -/// -/// Each label is the key's role word (`Owner`/`Payout address`/`Voting`/…) -/// plus a `(disabled)` marker for keys platform has retired: a node that -/// rotates its payout address keeps the old, disabled Payout key on-chain -/// next to the new active one, so a role word alone is not unique. Keys that -/// would still collide are told apart by their key id. -fn manage_keys_labels( - keys: &[(PrivateKeyTarget, dash_sdk::platform::IdentityPublicKey)], -) -> Vec<(String, Option<&'static str>)> { - let mut labels: Vec<(String, Option<&'static str>)> = keys - .iter() - .map(|(target, key)| { - let (role, tip) = key_role_label(target, key); - let label = if key.is_disabled() { - format!("{role} key (disabled)") - } else { - format!("{role} key") - }; - (label, tip) - }) - .collect(); - - let key_ids: Vec<_> = keys.iter().map(|(_, key)| Some(key.id())).collect(); - disambiguate_role_labels(&mut labels, &key_ids); - labels -} - /// At-rest protection posture of a node's vault keys, reduced to what the detail /// view needs: the tier label and whether an `Add password protection…` action /// applies (only when there are unprotected vault keys to seal). @@ -250,7 +211,17 @@ impl MasternodeDetailView { /// Whether a recovery offer is currently on screen for this node. pub(crate) fn has_recovery_offer_for_test(&self) -> bool { - self.recovery.plan().is_some_and(|plan| !plan.is_empty()) + self.recovery.has_offer() + } + + /// Put a detected plan on offer, as the check's own result does — without + /// the egui context [`Self::absorb_recovery_result`] needs to report one. + pub(crate) fn set_recovery_plan( + &mut self, + identity_id: dash_sdk::platform::Identifier, + plan: crate::model::legacy_recovery::RecoveryPlan, + ) { + self.recovery.offered(identity_id, plan); } /// The key roles this view believes the node holds. @@ -300,13 +271,14 @@ impl MasternodeDetailView { } } - /// Record the plan the on-arrival detection task returned. - pub(crate) fn set_recovery_plan( + /// Route a finished backend task into this node's recovery offer, reporting + /// whether this node's own restore finished. + pub(crate) fn absorb_recovery_result( &mut self, - identity_id: dash_sdk::platform::Identifier, - plan: RecoveryPlan, - ) { - self.recovery.offered(identity_id, plan); + ctx: &egui::Context, + result: &BackendTaskSuccessResult, + ) -> bool { + self.recovery.absorb_result(ctx, result) } /// Re-read this node from the store and re-arm its recovery check. @@ -333,10 +305,9 @@ impl MasternodeDetailView { } /// End this view's recovery operation when the failure that arrived is that - /// operation's own — matched on the identity the failing task names, since - /// every error reaches whichever screen is visible. - pub(crate) fn recovery_failed_for(&mut self, identity_id: dash_sdk::platform::Identifier) { - self.recovery.failed_for(identity_id); + /// operation's own — every error reaches whichever screen is visible. + pub(crate) fn absorb_recovery_error(&mut self, context: &BackendTaskContext) { + self.recovery.absorb_error(context); } /// Load the contests this node can still vote on. Empty when the node has no @@ -680,19 +651,20 @@ impl MasternodeDetailView { let tier = self.protection_tier(); ui.label(RichText::new(tier.label()).color(DashColors::text_secondary(dark_mode))); - // Per-key "Manage keys" list. Each key opens its own `KeyInfoScreen` — - // the real, interactive per-key screen with view/sign/seal actions — - // not the static read-only `KeysScreen` table. This mirrors - // `identities_screen.rs`: one button per key, each pushing - // `Screen::KeyInfoScreen`. + // Per-key "Manage keys" list. Each key opens its own `KeyInfoScreen`, + // the interactive per-key screen with view/sign/seal actions. This + // mirrors `identities_screen.rs` and the identity keys list: one button + // per key, each pushing `Screen::KeyInfoScreen` with the target the row + // found the material at. ui.add_space(4.0); ui.label( RichText::new("Manage keys") .strong() .color(DashColors::text_primary(dark_mode)), ); - let keys = self.identity_keys(); - let labels = manage_keys_labels(&keys); + let keys = identity_keys(&self.identity); + // This page only ever shows masternode and evonode identities. + let labels = manage_keys_labels(KeyVocabulary::from(self.identity.identity_type), &keys); for ((target, key), (label, tip)) in keys.into_iter().zip(labels) { let button = ui.button(format!("{label} ›")); let button = match tip { @@ -723,36 +695,19 @@ impl MasternodeDetailView { action } - /// Render the offer to restore keys stranded in the previous version's - /// saved data, returning the items the user approved this frame. Renders - /// nothing at all when detection found nothing, so the section appears only - /// where it has something to say and vanishes once a restore lands. + /// Render the offer at the foot of the keys section, returning the items the + /// user approved this frame. fn render_recovery_section(&self, ui: &mut Ui) -> Option> { - let restoring = self.recovery.is_restoring(); - let plan = self.recovery.plan().filter(|plan| !plan.is_empty())?; - ui.add_space(8.0); - LegacyRecoverySection::new(plan) - .restoring(restoring) - .show(ui) - .inner - .changed_value() - .clone() - } - - /// Every key of this node, main-identity keys first then voter-identity - /// keys, each paired with the `PrivateKeyTarget` that scopes it. Backs the - /// per-key "Manage keys" list and the Add-protection routing. - fn identity_keys(&self) -> Vec<(PrivateKeyTarget, dash_sdk::platform::IdentityPublicKey)> { - let mut keys = Vec::new(); - for key in self.identity.identity.public_keys().values() { - keys.push((PrivateKeyTarget::PrivateKeyOnMainIdentity, key.clone())); - } - if let Some((voter, _)) = self.identity.associated_voter_identity.as_ref() { - for key in voter.public_keys().values() { - keys.push((PrivateKeyTarget::PrivateKeyOnVoterIdentity, key.clone())); - } + if !self.recovery.has_offer() { + return None; } - keys + ui.add_space(8.0); + // This page only ever shows masternode and evonode identities. + host_offer( + &self.recovery, + KeyVocabulary::from(self.identity.identity_type), + ui, + ) } /// The first key whose private material this node actually holds — the only @@ -761,12 +716,14 @@ impl MasternodeDetailView { fn first_protectable_key( &self, ) -> Option<(PrivateKeyTarget, dash_sdk::platform::IdentityPublicKey)> { - self.identity_keys().into_iter().find(|(target, key)| { - self.identity - .private_keys - .get_cloned_private_key_data_and_wallet_info(&(target.clone(), key.id())) - .is_some() - }) + identity_keys(&self.identity) + .into_iter() + .find(|(target, key)| { + self.identity + .private_keys + .get_cloned_private_key_data_and_wallet_info(&(target.clone(), key.id())) + .is_some() + }) } /// Build the `AddScreen` action that opens `KeyInfoScreen` for one key, @@ -795,10 +752,16 @@ impl MasternodeDetailView { key: &dash_sdk::platform::IdentityPublicKey, mode: KeyInfoOpenMode, ) -> AppAction { + // Where this key's private half actually is, by the one rule every + // "Manage keys" surface uses. The structural target alone would miss + // material filed under the purpose-derived convention — a main-identity + // voting key entered by hand — and report a key as unheld here while the + // identity keys list shows it as saved on this device. + let filed_at = key_filed_at(&self.identity, &target, key).unwrap_or(target); let holding = self .identity .private_keys - .get_cloned_private_key_data_and_wallet_info(&(target, key.id())); + .get_cloned_private_key_data_and_wallet_info(&(filed_at.clone(), key.id())); let identity = self.identity.clone(); let key = key.clone(); let screen = match mode { @@ -809,7 +772,12 @@ impl MasternodeDetailView { KeyInfoScreen::new_with_protection_prompt(identity, key, holding, &self.app_context) } }; - AppAction::AddScreen(Screen::KeyInfoScreen(screen)) + // Hand the resolved location over rather than letting the screen guess. + // Left to itself it re-derives the target from the key's purpose, which + // disagrees for a key filed on the voter identity whose purpose is not + // `VOTING` — it would name the key differently from the row just clicked + // and lose the private half on its own re-read. + AppAction::AddScreen(Screen::KeyInfoScreen(screen.with_target(filed_at))) } /// Render the collapsible DPNS voting section (collapsed by default, @@ -1106,111 +1074,6 @@ mod tests { ); } - /// Build a masternode key with a chosen id / purpose / disabled state. - fn mn_key( - id: dash_sdk::dpp::identity::KeyID, - purpose: dash_sdk::dpp::identity::Purpose, - disabled: bool, - ) -> dash_sdk::platform::IdentityPublicKey { - use dash_sdk::dpp::identity::identity_public_key::v0::IdentityPublicKeyV0; - use dash_sdk::dpp::identity::{KeyType, SecurityLevel}; - use dash_sdk::dpp::platform_value::BinaryData; - IdentityPublicKeyV0 { - id, - key_type: KeyType::ECDSA_HASH160, - purpose, - security_level: SecurityLevel::CRITICAL, - read_only: true, - data: BinaryData::new(vec![id as u8; 20]), - disabled_at: disabled.then_some(1), - contract_bounds: None, - } - .into() - } - - /// An evonode that has rotated its payout address holds two `TRANSFER` - /// (Payout) keys on its main identity — the active new one and the disabled - /// old one — plus the owner key and a voter-identity voting key. Every - /// "Manage keys" button must get a distinct, correct label: the disabled - /// payout key is marked `(disabled)` instead of colliding with the active - /// one under a bare "Payout key". - #[test] - fn manage_keys_labels_disambiguate_rotated_evonode_payout_keys() { - use dash_sdk::dpp::identity::Purpose; - let keys = vec![ - ( - PrivateKeyTarget::PrivateKeyOnMainIdentity, - mn_key(0, Purpose::TRANSFER, false), - ), - ( - PrivateKeyTarget::PrivateKeyOnMainIdentity, - mn_key(1, Purpose::OWNER, false), - ), - ( - PrivateKeyTarget::PrivateKeyOnMainIdentity, - mn_key(2, Purpose::TRANSFER, true), - ), - ( - PrivateKeyTarget::PrivateKeyOnVoterIdentity, - mn_key(0, Purpose::VOTING, false), - ), - ]; - - let labels: Vec = manage_keys_labels(&keys) - .into_iter() - .map(|(label, _tip)| label) - .collect(); - assert_eq!( - labels, - vec![ - "Payout address key".to_string(), - "Owner key".to_string(), - "Payout address key (disabled)".to_string(), - "Voting key".to_string(), - ] - ); - // No two buttons ever share a label. - let unique: std::collections::BTreeSet<_> = labels.iter().collect(); - assert_eq!(unique.len(), labels.len(), "labels must be unique"); - } - - /// When even the role + `(disabled)` marker still collides — a payout - /// address rotated twice leaves two disabled Payout keys — the key id - /// breaks the tie so every button stays unique. - #[test] - fn manage_keys_labels_fall_back_to_key_id_on_residual_collision() { - use dash_sdk::dpp::identity::Purpose; - let keys = vec![ - ( - PrivateKeyTarget::PrivateKeyOnMainIdentity, - mn_key(0, Purpose::TRANSFER, false), - ), - ( - PrivateKeyTarget::PrivateKeyOnMainIdentity, - mn_key(2, Purpose::TRANSFER, true), - ), - ( - PrivateKeyTarget::PrivateKeyOnMainIdentity, - mn_key(3, Purpose::TRANSFER, true), - ), - ]; - - let labels: Vec = manage_keys_labels(&keys) - .into_iter() - .map(|(label, _tip)| label) - .collect(); - assert_eq!( - labels, - vec![ - "Payout address key".to_string(), - "Payout address key (disabled) #2".to_string(), - "Payout address key (disabled) #3".to_string(), - ] - ); - let unique: std::collections::BTreeSet<_> = labels.iter().collect(); - assert_eq!(unique.len(), labels.len(), "labels must be unique"); - } - #[test] fn tc_dpns_02_header_shows_open_contest_count() { assert_eq!(dpns_section_header(3), "DPNS name contests to vote on (3)"); diff --git a/src/ui/masternodes/list_screen.rs b/src/ui/masternodes/list_screen.rs index 97c69bccb..fdf6de331 100644 --- a/src/ui/masternodes/list_screen.rs +++ b/src/ui/masternodes/list_screen.rs @@ -582,9 +582,10 @@ impl ScreenLike for MasternodesScreen { // into the open detail view instead of reloading and re-opening it. // Re-opening would rebuild the view, re-dispatch its check, and // never settle. - BackendTaskSuccessResult::LegacyRecoveryCandidates { identity_id, plan } => { + BackendTaskSuccessResult::LegacyRecoveryCandidates { .. } => { + let ctx = self.app_context.egui_ctx().clone(); if let MasternodesView::Detail(detail) = &mut self.view { - detail.set_recovery_plan(identity_id, plan); + detail.absorb_recovery_result(&ctx, &result); } return; } @@ -650,10 +651,8 @@ impl ScreenLike for MasternodesScreen { context: &crate::backend_task::BackendTaskContext, _error: &crate::backend_task::error::TaskError, ) { - if let Some(identity_id) = context.legacy_recovery_identity() - && let MasternodesView::Detail(detail) = &mut self.view - { - detail.recovery_failed_for(identity_id); + if let MasternodesView::Detail(detail) = &mut self.view { + detail.absorb_recovery_error(context); } } diff --git a/src/ui/masternodes/mod.rs b/src/ui/masternodes/mod.rs index e9f098368..01edf549b 100644 --- a/src/ui/masternodes/mod.rs +++ b/src/ui/masternodes/mod.rs @@ -4,6 +4,13 @@ //! vote on DPNS name contests and manage owner/voting/payout keys. The page is //! a sibling root screen behind the Expert-Mode nav gate (FR-1); its identities //! are page-scoped and never leak into the everyday-user surfaces (FR-6, B1). +//! +//! The gate covers the screens, not this module's shared key helpers +//! ([`role_label_and_tip`], [`manage_keys_labels`], [`identity_keys`], +//! [`key_filed_at`]): those name, enumerate and resolve the keys of any identity +//! and are used from ungated surfaces — the identity keys list and the +//! recovery-offer component — so that one key cannot be called two different +//! things, or reported as saved on one screen and missing on another. pub mod card; pub mod detail_screen; @@ -13,7 +20,11 @@ pub mod testnet_fixture; pub use list_screen::MasternodesScreen; -use crate::model::qualified_identity::MasternodeKeyPresence; +use crate::model::qualified_identity::{ + IdentityType, MasternodeKeyPresence, PrivateKeyTarget, QualifiedIdentity, +}; +use dash_sdk::dpp::identity::accessors::IdentityGettersV0; +use dash_sdk::dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; /// Tooltip copy for the Dash Core DIP-3 ProRegTx key roles, shared by the detail /// view's "Manage keys" list and the load form's key fields so both surfaces use @@ -63,44 +74,328 @@ pub fn key_status_tokens(presence: MasternodeKeyPresence) -> [KeyRoleToken; 3] { } /// Tooltip for an authentication key — Platform-only, so it has no DIP-3 role -/// counterpart. +/// counterpart, and identity-neutral wording already. pub const TIP_AUTH_KEY: &str = "An authentication key signs this identity's actions on Dash Platform."; -/// The role word for a key and its tooltip, aligned with the Dash Core DIP-3 -/// ProRegTx roles. +/// Role tooltips for a plain user identity. The DIP-3 wording above describes +/// masternode registration duties — owning a node, updating its registration, +/// receiving its rewards — none of which a user identity has. Shown there it +/// does not merely read as jargon: it asserts the user owns a masternode. +pub const TIP_USER_TRANSFER_KEY: &str = + "This key lets you send Dash out of this identity, to another identity or to a Dash address."; +pub const TIP_USER_OWNER_KEY: &str = "This key proves you own this identity."; +pub const TIP_USER_VOTING_KEY: &str = "This key signs votes on contested usernames."; + +/// Which vocabulary a key's role is named in — the two differ for the same +/// [`Purpose`](dash_sdk::dpp::identity::Purpose), so it is the identity in front +/// of the user that decides, not the key. +/// +/// A masternode's `TRANSFER` key is its payout address; a user's is how they +/// send funds. Both namings are correct for their own identity, which is why +/// this is a parameter rather than one of them being a bug. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum KeyVocabulary { + /// DIP-3 ProRegTx role words, for masternode and evonode identities. + Masternode, + /// Plain-language role words, for user identities. + User, +} + +impl From for KeyVocabulary { + fn from(identity_type: IdentityType) -> Self { + match identity_type { + IdentityType::User => KeyVocabulary::User, + IdentityType::Masternode | IdentityType::Evonode => KeyVocabulary::Masternode, + } + } +} + +/// Every caption a key's role can carry, as whole translation units. +/// +/// Whole on purpose: a caller handed a bare role word inevitably bolts an +/// English noun onto it (`"{role} key"`), and a state onto that +/// (`"{role} key (disabled)"`), which is a sentence no translator can reorder, +/// re-inflect or re-punctuate. Each caption here is one string a translator +/// owns end to end. Private for the same reason — the only way to a caption is +/// [`role_label_and_tip`]. +const LABEL_VOTING_KEY: &str = "Voting key"; +const LABEL_VOTING_KEY_DISABLED: &str = "Voting key (disabled)"; +const LABEL_OWNER_KEY: &str = "Owner key"; +const LABEL_OWNER_KEY_DISABLED: &str = "Owner key (disabled)"; +const LABEL_PAYOUT_KEY: &str = "Payout address key"; +const LABEL_PAYOUT_KEY_DISABLED: &str = "Payout address key (disabled)"; +const LABEL_TRANSFER_KEY: &str = "Transfer key"; +const LABEL_TRANSFER_KEY_DISABLED: &str = "Transfer key (disabled)"; +const LABEL_AUTH_KEY: &str = "Authentication key"; +const LABEL_AUTH_KEY_DISABLED: &str = "Authentication key (disabled)"; +const LABEL_ENCRYPTION_KEY: &str = "Encryption key"; +const LABEL_ENCRYPTION_KEY_DISABLED: &str = "Encryption key (disabled)"; +const LABEL_DECRYPTION_KEY: &str = "Decryption key"; +const LABEL_DECRYPTION_KEY_DISABLED: &str = "Decryption key (disabled)"; +const LABEL_SYSTEM_KEY: &str = "System key"; +const LABEL_SYSTEM_KEY_DISABLED: &str = "System key (disabled)"; + +/// The complete caption for a key's role, and the tooltip explaining it. /// /// The single source of truth for what a key is called anywhere in this app: -/// the detail view's "Manage keys" buttons and the offer to restore keys from -/// the previous version both label the same key through this function, so the -/// two surfaces cannot name it differently. Voter-identity keys are always the -/// voting key; on the main identity, the Platform Owner and Transfer keys of a -/// masternode identity mirror the ProTx owner key and payout address. Unknown -/// purposes fall back to their name with no tooltip. +/// the detail view's "Manage keys" buttons, the identity keys list, the Key Info +/// page and the offer to restore keys from the previous version all name a key +/// through this function, so no two surfaces can name it differently. +/// +/// Returns a whole caption, never a word to decorate — see the label constants +/// above for why, and note the `&'static str` return makes composing one at a +/// callsite impossible rather than merely discouraged. `disabled` selects the +/// caption for a key Platform has retired: a node that rotates its payout +/// address keeps the old key on chain beside the new one, so a role alone does +/// not name one key. +/// +/// Voter-identity keys are always the voting key; on the main identity, the +/// Platform Owner and Transfer keys of a masternode identity mirror the ProTx +/// owner key and payout address. Every [`Purpose`](dash_sdk::dpp::identity::Purpose) +/// is matched by name and none falls through to a `Debug` rendering, so a raw +/// enum can never reach a user; a variant added upstream breaks this build +/// instead, which is the point. pub fn role_label_and_tip( + vocabulary: KeyVocabulary, is_on_voter_identity: bool, purpose: dash_sdk::dpp::identity::Purpose, -) -> (String, Option<&'static str>) { + disabled: bool, +) -> (&'static str, Option<&'static str>) { use dash_sdk::dpp::identity::Purpose; - if is_on_voter_identity { - return ("Voting".to_string(), Some(TIP_VOTING_KEY)); - } - match purpose { - Purpose::VOTING => ("Voting".to_string(), Some(TIP_VOTING_KEY)), - Purpose::OWNER => ("Owner".to_string(), Some(TIP_OWNER_KEY)), - Purpose::TRANSFER => ("Payout address".to_string(), Some(TIP_PAYOUT_KEY)), - Purpose::AUTHENTICATION => ("Authentication".to_string(), Some(TIP_AUTH_KEY)), - other => (format!("{other:?}"), None), + let user = vocabulary == KeyVocabulary::User; + // (caption, caption once Platform has retired the key, tooltip) + let (active, retired, tip) = if is_on_voter_identity { + // A key on a voter identity is the voting key whatever its purpose says, + // and only a masternode has one. + ( + LABEL_VOTING_KEY, + LABEL_VOTING_KEY_DISABLED, + Some(TIP_VOTING_KEY), + ) + } else { + match purpose { + Purpose::VOTING if user => ( + LABEL_VOTING_KEY, + LABEL_VOTING_KEY_DISABLED, + Some(TIP_USER_VOTING_KEY), + ), + Purpose::OWNER if user => ( + LABEL_OWNER_KEY, + LABEL_OWNER_KEY_DISABLED, + Some(TIP_USER_OWNER_KEY), + ), + Purpose::TRANSFER if user => ( + LABEL_TRANSFER_KEY, + LABEL_TRANSFER_KEY_DISABLED, + Some(TIP_USER_TRANSFER_KEY), + ), + Purpose::VOTING => ( + LABEL_VOTING_KEY, + LABEL_VOTING_KEY_DISABLED, + Some(TIP_VOTING_KEY), + ), + Purpose::OWNER => ( + LABEL_OWNER_KEY, + LABEL_OWNER_KEY_DISABLED, + Some(TIP_OWNER_KEY), + ), + Purpose::TRANSFER => ( + LABEL_PAYOUT_KEY, + LABEL_PAYOUT_KEY_DISABLED, + Some(TIP_PAYOUT_KEY), + ), + Purpose::AUTHENTICATION => { + (LABEL_AUTH_KEY, LABEL_AUTH_KEY_DISABLED, Some(TIP_AUTH_KEY)) + } + // No tooltip: these carry no role a user acts on here, but they + // still get a name of their own rather than a raw enum. + Purpose::ENCRYPTION => (LABEL_ENCRYPTION_KEY, LABEL_ENCRYPTION_KEY_DISABLED, None), + Purpose::DECRYPTION => (LABEL_DECRYPTION_KEY, LABEL_DECRYPTION_KEY_DISABLED, None), + Purpose::SYSTEM => (LABEL_SYSTEM_KEY, LABEL_SYSTEM_KEY_DISABLED, None), + } + }; + (if disabled { retired } else { active }, tip) +} + +/// The complete caption for one key and its tooltip, from the shared +/// [`role_label_and_tip`] vocabulary. Reads the retired state off the key, so +/// no caller has to remember to mark it. +pub(crate) fn key_role_label( + vocabulary: KeyVocabulary, + target: &PrivateKeyTarget, + key: &dash_sdk::platform::IdentityPublicKey, +) -> (&'static str, Option<&'static str>) { + role_label_and_tip( + vocabulary, + *target == PrivateKeyTarget::PrivateKeyOnVoterIdentity, + key.purpose(), + key.is_disabled(), + ) +} + +/// Every key of `identity` a "Manage keys" list shows: main-identity keys +/// first, then voter-identity keys, each paired with the [`PrivateKeyTarget`] +/// that scopes it. +/// +/// Shared so the masternode detail view and the identity keys list enumerate +/// keys identically. The target paired here is the *structural* one — which +/// identity's key map the key came from — which is only half of pairing a public +/// key with the private material the device may hold for it. Resolving that is +/// [`key_filed_at`], shared for the same reason: enumerating alike while +/// resolving differently is how the two surfaces came to disagree about whether +/// one key was saved on this device. +pub fn identity_keys( + identity: &QualifiedIdentity, +) -> Vec<(PrivateKeyTarget, dash_sdk::platform::IdentityPublicKey)> { + let mut keys: Vec<_> = identity + .identity + .public_keys() + .values() + .map(|key| (PrivateKeyTarget::PrivateKeyOnMainIdentity, key.clone())) + .collect(); + if let Some((voter, _)) = identity.associated_voter_identity.as_ref() { + keys.extend( + voter + .public_keys() + .values() + .map(|key| (PrivateKeyTarget::PrivateKeyOnVoterIdentity, key.clone())), + ); } + keys +} + +/// Whether a stored public key and a live one are the same key. +/// +/// Compares every field except `disabled_at`. A Platform identity public key is +/// immutable once added, with that single exception: disabling one rewrites that +/// field. The stored copy is a snapshot taken when the private half was saved, so +/// plain `==` stops matching as soon as a key is disabled or rotated, and a key +/// this device demonstrably holds is reported as missing. +/// +/// Comparing only the id and the key material would fix that and reopen a worse +/// hole in the other direction. Both are shared by construction where it matters: +/// `id` is already the lookup key, and a main identity's voting key and a linked +/// voter identity's key can carry identical `data`, leaving `purpose` as the only +/// thing telling them apart. Conflating those hands over private material the +/// clicked key does not own — so this excludes the one field that legitimately +/// moves, and nothing else. +fn same_key( + stored: &dash_sdk::platform::IdentityPublicKey, + live: &dash_sdk::platform::IdentityPublicKey, +) -> bool { + use dash_sdk::dpp::identity::identity_public_key::v0::IdentityPublicKeyV0; + use dash_sdk::platform::IdentityPublicKey; + + let IdentityPublicKey::V0(stored) = stored; + let IdentityPublicKey::V0(live) = live; + // Destructured exhaustively, and without `..`, on purpose: a field added + // upstream must break this build rather than be silently ignored. A new + // field that distinguishes two keys would otherwise leave this reporting a + // match where there is none — which is how a key's private material ends up + // attributed to a different key. Whoever adds it decides here whether it + // identifies a key or, like `disabled_at`, merely describes its state. + let IdentityPublicKeyV0 { + id, + purpose, + security_level, + contract_bounds, + key_type, + read_only, + data, + // The one field Platform lets move after a key is added: disabling a key + // rewrites it, and the stored snapshot was taken before that happened. + disabled_at: _, + } = stored; + + *id == live.id + && *purpose == live.purpose + && *security_level == live.security_level + && *contract_bounds == live.contract_bounds + && *key_type == live.key_type + && *read_only == live.read_only + && *data == live.data } -/// Make every label in `labelled` unique by appending `#{key id}` to the ones -/// that would otherwise appear more than once. +/// Which store `key`'s private half is actually in, or `None` if this device +/// holds no material for it. +/// +/// `structural` is the target [`identity_keys`] paired the key with — which +/// identity's key map it was found in. Two conventions for that target are in use +/// in this codebase: the structural one, and `impl From for +/// PrivateKeyTarget`, which files by purpose alone. Real installs hold material +/// written under each, so both are tried. /// -/// A role word alone is not unique: an evonode that rotates its payout address -/// holds two Transfer keys. Every list of keys the user is asked to read — and -/// especially one they are asked to approve — needs each row to name exactly -/// one key. Rows with no key id (a role link) are left as they are. +/// Shared by every "Manage keys" surface on purpose. [`identity_keys`] enumerates +/// the same keys for all of them and this resolves held-ness the same way for all +/// of them; splitting either one is how the identity keys list and the masternode +/// detail view came to disagree about whether one key was saved on this device. +/// +/// This is a read. Looking in the second place can only find material that is +/// already there, so it corrects a false "not saved on this device" without +/// moving anything; reconciling the two conventions on the write path is a +/// migration, tracked separately. +/// +/// Each candidate has to hold *this* public key, not merely have its slot filled: +/// a voter identity's own key can share a key id with a main-identity key, so an +/// occupied slot proves nothing about whose material is in it. +/// +/// Reads the stored public half only, never the private one: fetching the entry +/// would clone the raw private key out of the vault unscrubbed, and this runs +/// every frame for every key. +pub fn key_filed_at( + identity: &QualifiedIdentity, + structural: &PrivateKeyTarget, + key: &dash_sdk::platform::IdentityPublicKey, +) -> Option { + let derived: PrivateKeyTarget = key.purpose().into(); + [structural.clone(), derived].into_iter().find(|candidate| { + identity + .private_keys + .public_key_for(&(candidate.clone(), key.id())) + .is_some_and(|stored| same_key(&stored.identity_public_key, key)) + }) +} + +/// Button labels (and DIP-3-aligned tooltips) for a "Manage keys" list, one +/// per entry of `keys`, in order. +/// +/// Each label is the key's complete role caption from [`key_role_label`], which +/// already distinguishes a key Platform has retired. Keys that would still +/// collide are told apart by their key id. +pub fn manage_keys_labels( + vocabulary: KeyVocabulary, + keys: &[(PrivateKeyTarget, dash_sdk::platform::IdentityPublicKey)], +) -> Vec<(String, Option<&'static str>)> { + let mut labels: Vec<(String, Option<&'static str>)> = keys + .iter() + .map(|(target, key)| { + let (label, tip) = key_role_label(vocabulary, target, key); + (label.to_string(), tip) + }) + .collect(); + + let key_ids: Vec<_> = keys.iter().map(|(_, key)| Some(key.id())).collect(); + disambiguate_role_labels(&mut labels, &key_ids); + labels +} + +/// The one form in which a role caption is qualified by the key's on-chain id. +/// +/// A single translation unit with named placeholders, so a translation may +/// reorder or re-punctuate both parts. `label` is always a complete caption +/// from [`role_label_and_tip`] — never a fragment this then completes. +fn label_with_key_id(label: &str, key_id: dash_sdk::dpp::identity::KeyID) -> String { + format!("{label} #{key_id}") +} + +/// Make every label in `labelled` unique by qualifying the ones that would +/// otherwise appear more than once with their key id. +/// +/// A role caption alone is not unique: an evonode that rotates its payout +/// address holds two Transfer keys. Every list of keys the user is asked to +/// read — and especially one they are asked to approve — needs each row to name +/// exactly one key. Rows with no key id (a role link) are left as they are. pub fn disambiguate_role_labels( labelled: &mut [(String, Option<&'static str>)], key_ids: &[Option], @@ -113,7 +408,8 @@ pub fn disambiguate_role_labels( if let Some(key_id) = key_id && counts.get(label.as_str()).copied().unwrap_or(0) > 1 { - *label = format!("{label} #{key_id}"); + let qualified = label_with_key_id(label, *key_id); + *label = qualified; } } } @@ -122,37 +418,267 @@ pub fn disambiguate_role_labels( mod tests { use super::*; + /// Build a masternode key with a chosen id / purpose / disabled state. + fn mn_key( + id: dash_sdk::dpp::identity::KeyID, + purpose: dash_sdk::dpp::identity::Purpose, + disabled: bool, + ) -> dash_sdk::platform::IdentityPublicKey { + use dash_sdk::dpp::identity::identity_public_key::v0::IdentityPublicKeyV0; + use dash_sdk::dpp::identity::{KeyType, SecurityLevel}; + use dash_sdk::dpp::platform_value::BinaryData; + IdentityPublicKeyV0 { + id, + key_type: KeyType::ECDSA_HASH160, + purpose, + security_level: SecurityLevel::CRITICAL, + read_only: true, + data: BinaryData::new(vec![id as u8; 20]), + disabled_at: disabled.then_some(1), + contract_bounds: None, + } + .into() + } + + /// An evonode that has rotated its payout address holds two `TRANSFER` + /// (Payout) keys on its main identity — the active new one and the disabled + /// old one — plus the owner key and a voter-identity voting key. Every + /// "Manage keys" button must get a distinct, correct label: the disabled + /// payout key is marked `(disabled)` instead of colliding with the active + /// one under a bare "Payout key". + #[test] + fn manage_keys_labels_disambiguate_rotated_evonode_payout_keys() { + use dash_sdk::dpp::identity::Purpose; + let keys = vec![ + ( + PrivateKeyTarget::PrivateKeyOnMainIdentity, + mn_key(0, Purpose::TRANSFER, false), + ), + ( + PrivateKeyTarget::PrivateKeyOnMainIdentity, + mn_key(1, Purpose::OWNER, false), + ), + ( + PrivateKeyTarget::PrivateKeyOnMainIdentity, + mn_key(2, Purpose::TRANSFER, true), + ), + ( + PrivateKeyTarget::PrivateKeyOnVoterIdentity, + mn_key(0, Purpose::VOTING, false), + ), + ]; + + let labels: Vec = manage_keys_labels(KeyVocabulary::Masternode, &keys) + .into_iter() + .map(|(label, _tip)| label) + .collect(); + assert_eq!( + labels, + vec![ + "Payout address key".to_string(), + "Owner key".to_string(), + "Payout address key (disabled)".to_string(), + "Voting key".to_string(), + ] + ); + // No two buttons ever share a label. + let unique: std::collections::BTreeSet<_> = labels.iter().collect(); + assert_eq!(unique.len(), labels.len(), "labels must be unique"); + } + + /// When even the role + `(disabled)` marker still collides — a payout + /// address rotated twice leaves two disabled Payout keys — the key id + /// breaks the tie so every button stays unique. + #[test] + fn manage_keys_labels_fall_back_to_key_id_on_residual_collision() { + use dash_sdk::dpp::identity::Purpose; + let keys = vec![ + ( + PrivateKeyTarget::PrivateKeyOnMainIdentity, + mn_key(0, Purpose::TRANSFER, false), + ), + ( + PrivateKeyTarget::PrivateKeyOnMainIdentity, + mn_key(2, Purpose::TRANSFER, true), + ), + ( + PrivateKeyTarget::PrivateKeyOnMainIdentity, + mn_key(3, Purpose::TRANSFER, true), + ), + ]; + + let labels: Vec = manage_keys_labels(KeyVocabulary::Masternode, &keys) + .into_iter() + .map(|(label, _tip)| label) + .collect(); + assert_eq!( + labels, + vec![ + "Payout address key".to_string(), + "Payout address key (disabled) #2".to_string(), + "Payout address key (disabled) #3".to_string(), + ] + ); + let unique: std::collections::BTreeSet<_> = labels.iter().collect(); + assert_eq!(unique.len(), labels.len(), "labels must be unique"); + } + #[test] fn role_labels_follow_dip3_protx_terms() { use dash_sdk::dpp::identity::Purpose; + let node = KeyVocabulary::Masternode; // A voter-identity key is always the voting key, regardless of purpose. assert_eq!( - role_label_and_tip(true, Purpose::AUTHENTICATION), - ("Voting".to_string(), Some(TIP_VOTING_KEY)) + role_label_and_tip(node, true, Purpose::AUTHENTICATION, false), + (LABEL_VOTING_KEY, Some(TIP_VOTING_KEY)) ); // A voting-purpose key on the main identity is the voting key too. assert_eq!( - role_label_and_tip(false, Purpose::VOTING), - ("Voting".to_string(), Some(TIP_VOTING_KEY)) + role_label_and_tip(node, false, Purpose::VOTING, false), + (LABEL_VOTING_KEY, Some(TIP_VOTING_KEY)) ); // Main-identity roles mirror the DIP-3 ProRegTx owner key and payout // address; the Platform Transfer key surfaces as "Payout address". assert_eq!( - role_label_and_tip(false, Purpose::OWNER), - ("Owner".to_string(), Some(TIP_OWNER_KEY)) + role_label_and_tip(node, false, Purpose::OWNER, false), + (LABEL_OWNER_KEY, Some(TIP_OWNER_KEY)) + ); + assert_eq!( + role_label_and_tip(node, false, Purpose::TRANSFER, false), + (LABEL_PAYOUT_KEY, Some(TIP_PAYOUT_KEY)) + ); + assert_eq!( + role_label_and_tip(node, false, Purpose::AUTHENTICATION, false), + (LABEL_AUTH_KEY, Some(TIP_AUTH_KEY)) + ); + // A purpose with no role the user acts on here still gets a name of its + // own — never the raw enum. + assert_eq!( + role_label_and_tip(node, false, Purpose::ENCRYPTION, false), + (LABEL_ENCRYPTION_KEY, None) + ); + } + + /// Every caption the app can show a user is a whole, self-contained phrase: + /// one translation unit ending in the noun it names, never a role word the + /// caller finishes, and never a raw `Purpose` rendered through `Debug`. + /// + /// Exhaustive over the whole vocabulary because that is the only way this + /// holds for the combinations no screen happens to render today — an + /// encryption key on a user identity, a retired system key — which is + /// exactly where a `{:?}` fallback used to hide. + #[test] + fn every_role_caption_is_a_complete_translation_unit() { + use dash_sdk::dpp::identity::Purpose; + + let purposes = [ + Purpose::AUTHENTICATION, + Purpose::ENCRYPTION, + Purpose::DECRYPTION, + Purpose::TRANSFER, + Purpose::SYSTEM, + Purpose::VOTING, + Purpose::OWNER, + ]; + for vocabulary in [KeyVocabulary::Masternode, KeyVocabulary::User] { + for purpose in purposes { + for voter in [false, true] { + for disabled in [false, true] { + let (label, _) = role_label_and_tip(vocabulary, voter, purpose, disabled); + assert!( + !label.contains(&format!("{purpose:?}")), + "a raw Purpose must never reach a caption: {label}" + ); + assert!( + label.starts_with(char::is_uppercase), + "a caption is a phrase of its own, so it opens like one: {label}" + ); + let expected_tail = if disabled { "key (disabled)" } else { "key" }; + assert!( + label.ends_with(expected_tail), + "a caption names the thing it is, complete: {label}" + ); + } + } + } + } + } + + /// A retired key's caption is one unit too, not the active caption with an + /// English state bolted on — the two are separately translatable strings. + #[test] + fn a_retired_key_gets_its_own_whole_caption() { + use dash_sdk::dpp::identity::Purpose; + let node = KeyVocabulary::Masternode; + + assert_eq!( + role_label_and_tip(node, false, Purpose::TRANSFER, true), + (LABEL_PAYOUT_KEY_DISABLED, Some(TIP_PAYOUT_KEY)), + "retiring a key changes its caption, not its role or its tooltip" + ); + // The user vocabulary retires its own wording, not the node's. + assert_eq!( + role_label_and_tip(KeyVocabulary::User, false, Purpose::TRANSFER, true), + (LABEL_TRANSFER_KEY_DISABLED, Some(TIP_USER_TRANSFER_KEY)) + ); + } + + /// A user identity never owns a masternode, so the DIP-3 duties the node + /// wording describes — updating a registration, receiving node rewards — are + /// not merely jargon there: they assert something untrue about the identity + /// in front of the user. Same key, same purpose, different identity, and the + /// words have to follow the identity. + #[test] + fn a_user_identity_gets_plain_language_role_words() { + use dash_sdk::dpp::identity::Purpose; + let user = KeyVocabulary::User; + + assert_eq!( + role_label_and_tip(user, false, Purpose::TRANSFER, false), + (LABEL_TRANSFER_KEY, Some(TIP_USER_TRANSFER_KEY)), + "a user's transfer key is not a masternode payout address" + ); + assert_eq!( + role_label_and_tip(user, false, Purpose::OWNER, false), + (LABEL_OWNER_KEY, Some(TIP_USER_OWNER_KEY)) + ); + assert_eq!( + role_label_and_tip(user, false, Purpose::VOTING, false), + (LABEL_VOTING_KEY, Some(TIP_USER_VOTING_KEY)) ); + // Already identity-neutral, so it is shared verbatim. assert_eq!( - role_label_and_tip(false, Purpose::TRANSFER), - ("Payout address".to_string(), Some(TIP_PAYOUT_KEY)) + role_label_and_tip(user, false, Purpose::AUTHENTICATION, false), + role_label_and_tip( + KeyVocabulary::Masternode, + false, + Purpose::AUTHENTICATION, + false + ) ); + + // None of the node tooltips may reach a user identity. + for purpose in [Purpose::TRANSFER, Purpose::OWNER, Purpose::VOTING] { + let (_, tip) = role_label_and_tip(user, false, purpose, false); + let tip = tip.expect("every mapped role carries a tooltip"); + assert!( + ![TIP_OWNER_KEY, TIP_VOTING_KEY, TIP_PAYOUT_KEY].contains(&tip), + "a user identity must not be told about masternode duties: {tip}" + ); + } + } + + /// The identity type decides the vocabulary, and evonodes read as nodes. + #[test] + fn vocabulary_follows_the_identity_type() { + assert_eq!(KeyVocabulary::from(IdentityType::User), KeyVocabulary::User); assert_eq!( - role_label_and_tip(false, Purpose::AUTHENTICATION), - ("Authentication".to_string(), Some(TIP_AUTH_KEY)) + KeyVocabulary::from(IdentityType::Masternode), + KeyVocabulary::Masternode ); - // An unmapped purpose keeps its name and carries no tooltip. assert_eq!( - role_label_and_tip(false, Purpose::ENCRYPTION), - (format!("{purpose:?}", purpose = Purpose::ENCRYPTION), None,) + KeyVocabulary::from(IdentityType::Evonode), + KeyVocabulary::Masternode ); } @@ -205,4 +731,127 @@ mod tests { ] ); } + + /// Build a `QualifiedIdentity` holding the private half of `stored` filed at + /// `at`, whose on-chain key set is `live`. + fn identity_holding( + at: PrivateKeyTarget, + stored: dash_sdk::platform::IdentityPublicKey, + live: &[dash_sdk::platform::IdentityPublicKey], + ) -> QualifiedIdentity { + use crate::model::qualified_identity::encrypted_key_storage::{KeyStorage, PrivateKeyData}; + use crate::model::qualified_identity::qualified_identity_public_key::QualifiedIdentityPublicKey; + use crate::model::qualified_identity::{IdentityStatus, IdentityType}; + use dash_sdk::dpp::identity::Identity; + use dash_sdk::dpp::version::PlatformVersion; + use dash_sdk::platform::Identifier; + use std::collections::BTreeMap; + + let identity = Identity::new_with_id_and_keys( + Identifier::from([0x60u8; 32]), + live.iter().map(|k| (k.id(), k.clone())).collect(), + PlatformVersion::latest(), + ) + .expect("identity with keys"); + QualifiedIdentity { + identity, + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::Masternode, + alias: Some("filed-at".to_string()), + private_keys: KeyStorage { + private_keys: BTreeMap::from([( + (at, stored.id()), + ( + QualifiedIdentityPublicKey::from(stored), + PrivateKeyData::Clear([0x60; 32]), + ), + )]), + }, + dpns_names: vec![], + associated_wallets: BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::Active, + network: dash_sdk::dpp::dashcore::Network::Testnet, + } + } + + /// The resolution rule every "Manage keys" surface shares, in one place. + /// + /// Two target conventions are in use, so both are tried; a key must be + /// matched by its own public half rather than by an occupied slot; and + /// `disabled_at` must not break the match, because it is the one field + /// Platform lets move after a key is added. + #[test] + fn held_material_is_found_under_either_convention_and_only_for_the_right_key() { + use dash_sdk::dpp::identity::Purpose; + + // Filed structurally, where `identity_keys` says it is. + let key = mn_key(0, Purpose::AUTHENTICATION, false); + let identity = identity_holding( + PrivateKeyTarget::PrivateKeyOnMainIdentity, + key.clone(), + std::slice::from_ref(&key), + ); + assert_eq!( + key_filed_at(&identity, &PrivateKeyTarget::PrivateKeyOnMainIdentity, &key), + Some(PrivateKeyTarget::PrivateKeyOnMainIdentity), + "material filed where the key structurally sits must be found" + ); + + // Filed under the purpose-derived convention instead: a voting key on + // the main identity, entered by hand. Reported as unheld before the + // fallback existed. + let voting = mn_key(0, Purpose::VOTING, false); + let identity = identity_holding( + PrivateKeyTarget::PrivateKeyOnVoterIdentity, + voting.clone(), + std::slice::from_ref(&voting), + ); + assert_eq!( + key_filed_at( + &identity, + &PrivateKeyTarget::PrivateKeyOnMainIdentity, + &voting + ), + Some(PrivateKeyTarget::PrivateKeyOnVoterIdentity), + "material filed by purpose derivation must still be found" + ); + + // A *different* key at the same id must not match. These two share + // `id` and `data`, so purpose is the only thing telling them apart — + // matching here would report one key as held on the strength of + // another's private half. + assert_eq!( + key_filed_at( + &identity, + &PrivateKeyTarget::PrivateKeyOnMainIdentity, + &mn_key(0, Purpose::AUTHENTICATION, false) + ), + None, + "an occupied slot proves nothing about whose material is in it" + ); + + // Disabling a key on chain does not remove its private half from this + // device, so the stored snapshot must still match the live key. + let disabled = mn_key(0, Purpose::AUTHENTICATION, true); + let identity = identity_holding( + PrivateKeyTarget::PrivateKeyOnMainIdentity, + mn_key(0, Purpose::AUTHENTICATION, false), + std::slice::from_ref(&disabled), + ); + assert_eq!( + key_filed_at( + &identity, + &PrivateKeyTarget::PrivateKeyOnMainIdentity, + &disabled + ), + Some(PrivateKeyTarget::PrivateKeyOnMainIdentity), + "`disabled_at` is the one field that legitimately moves, so it must \ + not break the match" + ); + } } diff --git a/src/ui/mod.rs b/src/ui/mod.rs index 82417da35..685464829 100644 --- a/src/ui/mod.rs +++ b/src/ui/mod.rs @@ -47,7 +47,6 @@ use contracts_documents::add_contracts_screen::AddContractsScreen; use contracts_documents::group_actions_screen::GroupActionsScreen; use contracts_documents::register_contract_screen::RegisterDataContractScreen; use contracts_documents::update_contract_screen::UpdateDataContractScreen; -use dash_sdk::dpp::identity::Identity; use dash_sdk::dpp::prelude::IdentityPublicKey; use dash_sdk::platform::Identifier; use dpns::dpns_contested_names_screen::DPNSSubscreen; @@ -154,7 +153,10 @@ pub enum ScreenType { IdentityPublicKey, Option<(PrivateKeyData, Option)>, ), - Keys(Identity), + /// The identity keys list. Carries the qualified record, not the bare + /// `Identity`: the screen has to say which keys this device actually holds + /// and open `KeyInfo` with that key's private material. + Keys(QualifiedIdentity), DocumentQuery, NetworkChooser, RegisterDpnsName(RegisterDpnsNameSource), diff --git a/src/ui/state/legacy_recovery.rs b/src/ui/state/legacy_recovery.rs index c9d0fb573..7cd22a074 100644 --- a/src/ui/state/legacy_recovery.rs +++ b/src/ui/state/legacy_recovery.rs @@ -13,10 +13,13 @@ use dash_sdk::platform::Identifier; -use crate::backend_task::BackendTask; use crate::backend_task::identity::IdentityTask; +use crate::backend_task::{BackendTask, BackendTaskContext, BackendTaskSuccessResult}; use crate::context::AppContext; use crate::model::legacy_recovery::{RecoveryItem, RecoveryPlan}; +use crate::ui::MessageType; +use crate::ui::components::MessageBanner; +use crate::ui::components::legacy_recovery_section::completion_message; /// Where one identity's recovery offer currently stands. enum FetchState { @@ -91,6 +94,13 @@ impl LegacyRecoveryState { matches!(self.state, FetchState::Restoring(_)) } + /// Whether the offer has anything to show this frame — for a host deciding + /// whether to draw the separators and spacing that frame it. The offer + /// itself renders nothing when this is `false`. + pub fn has_offer(&self) -> bool { + self.plan().is_some_and(|plan| !plan.is_empty()) + } + /// The restore task for `approved`, marking it in flight. Returns `None` /// unless there is a plan on offer and no restore already running. pub fn restore(&mut self, approved: Vec) -> Option { @@ -152,7 +162,9 @@ impl LegacyRecoveryState { /// unrelated task's failure must leave this offer alone. Ending a restore /// it never started would re-enable the Restore button while the original /// task still holds the identity, and pressing it again would only report - /// that a load is already in progress. + /// that a load is already in progress. Every screen hosting an offer routes + /// its failures through here, so no two can disagree about whether a restore + /// ended. /// /// A failed restore returns to its offer, so a mistyped identity password /// can be corrected and Restore pressed again. A failed detection is @@ -168,6 +180,65 @@ impl LegacyRecoveryState { }; true } + + /// Route a finished backend task into this offer, reporting whether it was + /// *this* identity's own restore that finished. + /// + /// The whole of what a hosting screen owes a recovery result, so no host has + /// to restate it: adopting a detected plan, attributing a completion through + /// [`Self::completed_for`], and telling the user the outcome in the one + /// wording every host shares. `true` is the host's cue to re-read whatever + /// it derives from the identity record — the clone it persists, a key-role + /// presence line, a cached protection status — since only this identity's + /// own restore wrote that record. + /// + /// A result of any other kind, or a completion for another identity, is + /// ignored and reported as not this offer's: results reach whichever screen + /// is visible when they arrive, and a banner claiming these keys came back + /// when they did not is worse than silence. + pub fn absorb_result( + &mut self, + ctx: &egui::Context, + result: &BackendTaskSuccessResult, + ) -> bool { + match result { + BackendTaskSuccessResult::LegacyRecoveryCandidates { identity_id, plan } => { + self.offered(*identity_id, plan.clone()); + false + } + BackendTaskSuccessResult::LegacyRecoveryCompleted { + identity_id, + applied, + .. + } => { + if !self.completed_for(*identity_id) { + return false; + } + MessageBanner::set_global( + ctx, + completion_message(!applied.is_empty()), + MessageType::Success, + ); + true + } + _ => false, + } + } + + /// Route a failing task's context into this offer, ending a recovery + /// operation only when the failure is that operation's own, and reporting + /// whether it was. + /// + /// The error itself is never claimed — `AppState`'s generic banner is how + /// the user gets to see it. This exists so every host attributes a failure + /// through the single rule in [`Self::failed_for`]: a host that ended a + /// restore on any error that merely landed while it was visible would + /// re-enable Restore while the original task still held the identity. + pub fn absorb_error(&mut self, context: &BackendTaskContext) -> bool { + context + .legacy_recovery_identity() + .is_some_and(|identity_id| self.failed_for(identity_id)) + } } #[cfg(test)] diff --git a/tests/kittest/key_info_screen.rs b/tests/kittest/key_info_screen.rs new file mode 100644 index 000000000..75844dd14 --- /dev/null +++ b/tests/kittest/key_info_screen.rs @@ -0,0 +1,182 @@ +//! Kittest coverage for `KeyInfoScreen`'s arrival refresh. +//! +//! The screen persists the whole identity clone it was opened with on every key +//! add and remove, so a write that lands while it sits on the stack has to be +//! picked up — otherwise the next ordinary key edit puts the pre-write record +//! back, silently and with no error to show for it. +//! +//! That only holds if the re-read hangs off a hook `AppState` actually +//! dispatches to a *pushed* screen. `refresh_on_arrival` is not one: +//! `app.rs` sends it only to root screens, and `KeyInfoScreen` is always pushed +//! onto the screen stack. This drives the dispatch the running app really makes. + +use crate::support::{mount_app, with_isolated_data_dir}; +use dash_evo_tool::app::TaskResult; +use dash_evo_tool::model::qualified_identity::encrypted_key_storage::{KeyStorage, PrivateKeyData}; +use dash_evo_tool::model::qualified_identity::qualified_identity_public_key::QualifiedIdentityPublicKey; +use dash_evo_tool::model::qualified_identity::{ + IdentityStatus, IdentityType, PrivateKeyTarget, QualifiedIdentity, +}; +use dash_evo_tool::ui::{RootScreenType, Screen}; +use dash_sdk::dpp::identity::accessors::IdentityGettersV0; +use dash_sdk::dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; +use dash_sdk::dpp::identity::{Identity, KeyID, Purpose}; +use dash_sdk::dpp::version::PlatformVersion; +use dash_sdk::platform::{Identifier, IdentityPublicKey}; +use egui_kittest::kittest::{NodeT, Queryable}; +use std::collections::BTreeMap; + +const MAIN: PrivateKeyTarget = PrivateKeyTarget::PrivateKeyOnMainIdentity; +/// The row control the keys list renders for an authentication key. +const AUTH_ROW: &str = "Authentication key ›"; + +/// A key with a chosen id and purpose. Deterministic data, because the row +/// label this test navigates by is derived from the purpose. +fn key(id: KeyID, purpose: Purpose) -> IdentityPublicKey { + use dash_sdk::dpp::identity::identity_public_key::v0::IdentityPublicKeyV0; + use dash_sdk::dpp::identity::{KeyType, SecurityLevel}; + use dash_sdk::dpp::platform_value::BinaryData; + IdentityPublicKeyV0 { + id, + key_type: KeyType::ECDSA_HASH160, + purpose, + security_level: SecurityLevel::CRITICAL, + read_only: false, + data: BinaryData::new(vec![id as u8; 20]), + disabled_at: None, + contract_bounds: None, + } + .into() +} + +/// A plain user identity carrying `purposes` as public keys and no private key +/// material — the state that makes every key openable from the keys list. +fn identity_with(id_byte: u8, purposes: &[Purpose], alias: &str) -> QualifiedIdentity { + let mut identity = + Identity::create_basic_identity(Identifier::from([id_byte; 32]), PlatformVersion::latest()) + .expect("basic identity"); + for (index, purpose) in purposes.iter().enumerate() { + identity.add_public_key(key(index as KeyID, *purpose)); + } + QualifiedIdentity { + identity, + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::User, + alias: Some(alias.to_string()), + private_keys: KeyStorage::default(), + dpns_names: vec![], + associated_wallets: BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::Active, + network: dash_sdk::dpp::dashcore::Network::Testnet, + } +} + +/// Walk the route a user takes to one key's own page: Identities → Settings → +/// Advanced → Manage keys → the key. +fn open_key_info(harness: &mut egui_kittest::Harness<'static, dash_evo_tool::app::AppState>) { + // The left nav carries its own "Settings" entry. Both are buttons; only the + // nav one is a toggle, so that is what tells the hub tab apart. + harness + .query_all_by_role_and_label(egui::accesskit::Role::Button, "Settings") + .find(|node| node.accesskit_node().toggled().is_none()) + .expect("the hub must render a Settings tab") + .click(); + harness.run_steps(3); + harness.get_by_label("Advanced").click(); + harness.run_steps(3); + harness.get_by_label("Manage keys").click(); + harness.run_steps(3); + harness.get_by_label(AUTH_ROW).click(); + harness.run_steps(3); +} + +/// A restore that lands while this screen is open never reaches its +/// `display_task_result` — results go only to the screen that is visible, and a +/// restore run from elsewhere is not this screen's. Its clone must still be +/// re-read when the app refreshes it, or the next ordinary key edit writes that +/// clone back over the restored key. +/// +/// Drives the refresh the running app actually performs: `AppState::update` +/// drains `task_result_sender` every frame, and `TaskResult::Refresh` calls +/// `refresh()` on the *visible* screen — the pushed `KeyInfoScreen`. A screen +/// that puts its re-read on `refresh_on_arrival` instead is never asked, since +/// `AppState` sends that hook only to root screens. +#[test] +fn a_write_that_lands_while_key_info_is_open_survives_the_next_key_edit() { + with_isolated_data_dir(|| { + let rt = tokio::runtime::Runtime::new().expect("Failed to create tokio runtime"); + let _guard = rt.enter(); + + let mut harness = mount_app(RootScreenType::RootScreenIdentityHub); + // The Settings tab stacks two columns above the Advanced expander; a + // short window puts the keys controls below the fold, where a simulated + // click lands outside the widget. + harness.set_size(egui::vec2(1280.0, 1800.0)); + let app_context = harness.state().current_app_context().clone(); + let identity = identity_with(0x51, &[Purpose::AUTHENTICATION], "arrival-refresh"); + let identity_id = identity.identity.id(); + app_context + .insert_local_qualified_identity(&identity, &None) + .expect("seed the identity"); + harness.run_steps(5); + + open_key_info(&mut harness); + assert!( + matches!( + harness.state().screen_stack.last(), + Some(Screen::KeyInfoScreen(_)) + ), + "the premise: the key's own page is the visible screen" + ); + + // The restore lands behind this screen's back: the record gains a key + // this screen's clone knows nothing about. + let restored = key(1, Purpose::TRANSFER); + let mut record = app_context + .get_local_qualified_identity(&identity_id) + .expect("read the record") + .expect("record stored"); + record.identity.add_public_key(restored.clone()); + record.private_keys.private_keys.insert( + (MAIN, restored.id()), + ( + QualifiedIdentityPublicKey::from(restored.clone()), + PrivateKeyData::Clear([0x22; 32]), + ), + ); + app_context + .update_local_qualified_identity(&record) + .expect("the other writer's write"); + + harness + .state() + .task_result_sender + .try_send(TaskResult::Refresh) + .expect("queue the refresh the app dispatches"); + harness.run_steps(3); + + // What every key add and remove on this screen does with its clone. + let Some(Screen::KeyInfoScreen(screen)) = harness.state().screen_stack.last() else { + panic!("Key Info must still be the open screen"); + }; + app_context + .update_local_qualified_identity(&screen.identity) + .expect("the next key edit's write"); + + assert!( + app_context + .get_local_qualified_identity(&identity_id) + .expect("read back") + .expect("still stored") + .private_keys + .private_keys + .contains_key(&(MAIN, restored.id())), + "a key edit on this screen must not erase a key written while it was open", + ); + }); +} diff --git a/tests/kittest/keys_screen.rs b/tests/kittest/keys_screen.rs index 3c983762b..5b33668f9 100644 --- a/tests/kittest/keys_screen.rs +++ b/tests/kittest/keys_screen.rs @@ -1,61 +1,1139 @@ -//! Kittest coverage for the Manage Keys detail screen (`KeysScreen`). +//! Kittest coverage for the identity keys list (`KeysScreen`). //! -//! Regression guard for the dead-end lockout: the read-only key list is pushed -//! onto the screen stack, so it must offer a Back control that pops itself off -//! (`AppAction::PopScreen`). Without it the user is trapped on the screen with no -//! way back to the identity view. +//! The screen is the only key-state-independent way into `KeyInfoScreen` for a +//! user identity: every other route is gated on already holding a suitable key, +//! which is false for exactly the identities the restore-stranded-keys offer +//! exists to help. These tests pin that route open, and pin the offer where a +//! user who came looking for missing keys will actually see it. -use crate::support::{fresh_app_context, with_isolated_data_dir}; +use crate::support::{fresh_app_context, mount_app, with_isolated_data_dir}; use dash_evo_tool::app::AppAction; -use dash_evo_tool::ui::ScreenLike; +use dash_evo_tool::backend_task::error::TaskError; +use dash_evo_tool::backend_task::{BackendTaskContext, BackendTaskSuccessResult}; +use dash_evo_tool::model::legacy_recovery::{RecoveryItem, RecoveryItemDescriptor, RecoveryPlan}; +use dash_evo_tool::model::qualified_identity::encrypted_key_storage::{KeyStorage, PrivateKeyData}; +use dash_evo_tool::model::qualified_identity::qualified_identity_public_key::QualifiedIdentityPublicKey; +use dash_evo_tool::model::qualified_identity::{ + IdentityStatus, IdentityType, PrivateKeyTarget, QualifiedIdentity, +}; +use dash_evo_tool::ui::components::MessageBanner; +use dash_evo_tool::ui::components::legacy_recovery_section::recovery_item_labels; use dash_evo_tool::ui::identities::keys::keys_screen::KeysScreen; -use dash_sdk::dpp::identity::Identity; +use dash_evo_tool::ui::masternodes::{KeyVocabulary, manage_keys_labels}; +use dash_evo_tool::ui::{MessageType, Screen, ScreenLike}; +use dash_sdk::dpp::identity::accessors::IdentityGettersV0; +use dash_sdk::dpp::identity::identity_public_key::accessors::v0::IdentityPublicKeyGettersV0; +use dash_sdk::dpp::identity::{Identity, KeyID, Purpose}; use dash_sdk::dpp::version::PlatformVersion; -use dash_sdk::platform::Identifier; +use dash_sdk::platform::{Identifier, IdentityPublicKey}; use egui_kittest::Harness; -use egui_kittest::kittest::Queryable; +use egui_kittest::kittest::{NodeT, Queryable}; use std::cell::RefCell; +use std::collections::BTreeMap; use std::rc::Rc; -/// Clicking Back on the Manage Keys screen pops it off the screen stack, -/// closing the dead-end lockout. +/// The row control for an authentication key, in the shared role vocabulary. +const AUTH_ROW: &str = "Authentication key ›"; +/// The row control for a `TRANSFER`-purpose key on a **user** identity. The +/// same key on a masternode is its payout address; here it is how the user +/// sends funds, and the words follow the identity. +const TRANSFER_ROW: &str = "Transfer key ›"; +/// The crumb `KeyInfoScreen` shows for the keys list that opened it. +const PARENT_CRUMB: &str = "Keys"; +/// The row control for a `VOTING`-purpose key. +const VOTING_ROW: &str = "Voting key ›"; +/// The row control of the recovery offer. +const RESTORE: &str = "Restore keys"; +/// The held/not-held disclosure, mirroring `keys_screen.rs`'s own copy. A +/// divergence here is a test bug, not a screen bug — these are the strings the +/// user reads. +const HELD: &str = "This key is saved on this device."; +const NOT_HELD: &str = "This key is not saved on this device."; +/// The banner text a restore that put keys back reports, from +/// `completion_message(true)`. +const RESTORED: &str = + "Your keys from the previous Dash Evo Tool version have been restored to this identity."; +/// The banner text a restore that put nothing back reports, from +/// `completion_message(false)`. +const NOTHING_RESTORED: &str = "There was nothing left to restore for this identity."; +/// Stands in for the typed error text `AppState` banners when a restore fails. +const RESTORE_FAILED: &str = "Those keys could not be restored. Check the password and try again."; + +/// A key with a chosen id and purpose. Deterministic data, because the row +/// labels under test are derived from purpose and id. +fn key(id: KeyID, purpose: Purpose) -> IdentityPublicKey { + use dash_sdk::dpp::identity::identity_public_key::v0::IdentityPublicKeyV0; + use dash_sdk::dpp::identity::{KeyType, SecurityLevel}; + use dash_sdk::dpp::platform_value::BinaryData; + IdentityPublicKeyV0 { + id, + key_type: KeyType::ECDSA_HASH160, + purpose, + security_level: SecurityLevel::CRITICAL, + read_only: false, + data: BinaryData::new(vec![id as u8; 20]), + disabled_at: None, + contract_bounds: None, + } + .into() +} + +/// A plain user identity carrying `purposes` as public keys and **no private +/// key material at all** — the stranded-key state this whole route exists for. +fn stranded_identity(id_byte: u8, purposes: &[Purpose], alias: &str) -> QualifiedIdentity { + let pv = PlatformVersion::latest(); + let mut identity = Identity::create_basic_identity(Identifier::from([id_byte; 32]), pv) + .expect("basic identity"); + for (index, purpose) in purposes.iter().enumerate() { + identity.add_public_key(key(index as KeyID, *purpose)); + } + QualifiedIdentity { + identity, + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::User, + alias: Some(alias.to_string()), + // The point of the fixture: the device holds nothing. + private_keys: KeyStorage::default(), + dpns_names: vec![], + associated_wallets: BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::Active, + network: dash_sdk::dpp::dashcore::Network::Testnet, + } +} + +/// A user identity holding the private half of one key, filed at `target`. +/// +/// `Purpose::VOTING` filed at `PrivateKeyOnMainIdentity` is the shape where the +/// two target conventions in this codebase disagree: `identity_keys` derives the +/// target from which identity it walked, while `impl From for +/// PrivateKeyTarget` maps every voting key to the voter identity regardless of +/// where it actually sits. A voting-purpose key on the main identity is real — +/// `masternode_key_presence` treats it as voting readiness on its own. +fn identity_holding_key( + id_byte: u8, + purpose: Purpose, + target: PrivateKeyTarget, +) -> QualifiedIdentity { + let public_key = key(0, purpose); + let identity = Identity::new_with_id_and_keys( + Identifier::from([id_byte; 32]), + BTreeMap::from([(public_key.id(), public_key.clone())]), + PlatformVersion::latest(), + ) + .expect("identity with one key"); + QualifiedIdentity { + identity, + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::User, + alias: Some("held-key".to_string()), + private_keys: KeyStorage { + private_keys: BTreeMap::from([( + (target, public_key.id()), + ( + QualifiedIdentityPublicKey::from(public_key), + PrivateKeyData::Clear([id_byte; 32]), + ), + )]), + }, + dpns_names: vec![], + associated_wallets: BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::Active, + network: dash_sdk::dpp::dashcore::Network::Testnet, + } +} + +/// A plan offering one restorable key, so the section has something to say. +fn plan() -> RecoveryPlan { + RecoveryPlan { + items: vec![RecoveryItemDescriptor { + item: RecoveryItem::Key { + target: PrivateKeyTarget::PrivateKeyOnMainIdentity, + key_id: 0, + }, + purpose: Some(Purpose::AUTHENTICATION), + }], + excluded: vec![], + } +} + +/// Drive `screen` in a bare harness, capturing the last non-`None` action it +/// returned. +fn harness_for(mut screen: KeysScreen) -> (Harness<'static>, Rc>) { + let action = Rc::new(RefCell::new(AppAction::None)); + let capture = action.clone(); + let mut harness = Harness::builder() + .with_size(egui::vec2(1100.0, 800.0)) + .build_ui(move |ui| { + let act = screen.ui(ui); + if act != AppAction::None { + *capture.borrow_mut() = act; + } + }); + harness.run_steps(3); + (harness, action) +} + +/// Drive `screen` while keeping a handle on it, so a test can deliver the +/// results and messages `AppState` would between frames. +fn harness_keeping_screen(screen: KeysScreen) -> (Harness<'static>, Rc>) { + let screen = Rc::new(RefCell::new(screen)); + let rendered = screen.clone(); + let mut harness = Harness::builder() + .with_size(egui::vec2(1100.0, 800.0)) + .build_ui(move |ui| { + rendered.borrow_mut().ui(ui); + }); + harness.run_steps(3); + (harness, screen) +} + +/// Drive `screen` with `banner` already set on the harness's own context, the +/// way `AppState` sets one before a screen renders. +fn harness_showing_banner( + mut screen: KeysScreen, + banner: &'static str, + kind: MessageType, +) -> Harness<'static> { + let mut set = false; + let mut harness = Harness::builder() + .with_size(egui::vec2(1100.0, 800.0)) + .build_ui(move |ui| { + if !set { + MessageBanner::set_global(ui.ctx(), banner, kind); + set = true; + } + screen.ui(ui); + }); + harness.run_steps(3); + harness +} + +/// The screen must actually render global banners. It is built on +/// `island_central_panel` for this reason: that is the only banner-rendering +/// path a screen gets from the standard chrome (a screen wanting them without +/// it has to call `MessageBanner::show_global` itself, as +/// `contracts_documents_screen` does). On a bare `CentralPanel` every restore +/// outcome would be invisible and a failed restore would look exactly like a +/// successful one — the offer self-extinguishes on the next check either way. #[test] -fn manage_keys_back_button_pops_the_screen() { +fn the_keys_list_renders_global_banners() { with_isolated_data_dir(|| { let (_rt, app_context) = fresh_app_context(); + let identity = stranded_identity(0x39, &[Purpose::AUTHENTICATION], "banner-channel"); - let identity = Identity::create_basic_identity( - Identifier::from([0x33u8; 32]), - PlatformVersion::latest(), - ) - .expect("basic identity"); + let success = harness_showing_banner( + KeysScreen::new(identity.clone(), &app_context), + RESTORED, + MessageType::Success, + ); + assert!( + success.query_by_label(RESTORED).is_some(), + "a success banner must be visible on the keys list" + ); + + let failure = harness_showing_banner( + KeysScreen::new(identity, &app_context), + RESTORE_FAILED, + MessageType::Error, + ); + assert!( + failure.query_by_label(RESTORE_FAILED).is_some(), + "an error banner must be visible on the keys list too — a restore that \ + failed must not read as one that worked" + ); + }); +} + +/// A finished restore announces itself — and only its own. +/// +/// Results reach whichever screen is visible when they arrive, so a completion +/// for another identity must say nothing here: a banner claiming this +/// identity's keys came back when they did not is worse than silence. The +/// *content* of the banner is pinned by +/// `a_restore_reports_success_and_failure_on_the_keys_list_in_the_running_app`, +/// which runs in the app's own context; `fresh_app_context` drops the harness +/// its context belongs to, so text cannot be read back at this level and +/// `has_global` is all this test can honestly assert. +#[test] +fn a_finished_restore_reports_only_its_own_outcome() { + with_isolated_data_dir(|| { + let (_rt, app_context) = fresh_app_context(); + let identity = stranded_identity(0x3a, &[Purpose::AUTHENTICATION], "restore-outcome"); + let identity_id = identity.identity.id(); let mut screen = KeysScreen::new(identity, &app_context); + let completed = |identity_id| BackendTaskSuccessResult::LegacyRecoveryCompleted { + identity_id, + applied: vec![RecoveryItemDescriptor { + item: RecoveryItem::Key { + target: PrivateKeyTarget::PrivateKeyOnMainIdentity, + key_id: 0, + }, + purpose: Some(Purpose::AUTHENTICATION), + }], + skipped_stale: vec![], + excluded: vec![], + }; - // Capture the action the screen returns on the frame Back is clicked. - let action = Rc::new(RefCell::new(AppAction::None)); - let capture = action.clone(); - let mut harness = Harness::builder() - .with_size(egui::vec2(900.0, 600.0)) - .build_ui(move |ui| { - let act = screen.ui(ui); - if act != AppAction::None { - *capture.borrow_mut() = act; - } - }); + MessageBanner::clear_all_global(app_context.egui_ctx()); + screen.display_task_result(completed(Identifier::from([0xee; 32]))); + assert!( + !MessageBanner::has_global(app_context.egui_ctx()), + "another identity's restore must not report anything on this screen" + ); + + screen.display_task_result(completed(identity_id)); + assert!( + MessageBanner::has_global(app_context.egui_ctx()), + "this identity's own restore must report that it finished" + ); + }); +} + +/// A restore that failed keeps its offer, so a mistyped identity password can +/// be corrected and Restore pressed again rather than the remedy vanishing. +#[test] +fn a_failed_restore_leaves_the_offer_in_place_to_retry() { + with_isolated_data_dir(|| { + let (_rt, app_context) = fresh_app_context(); + let identity = stranded_identity(0x3b, &[Purpose::AUTHENTICATION], "restore-retry"); + let identity_id = identity.identity.id(); + let mut screen = KeysScreen::new(identity, &app_context); + screen.display_task_result(BackendTaskSuccessResult::LegacyRecoveryCandidates { + identity_id, + plan: plan(), + }); + + let (mut harness, screen) = harness_keeping_screen(screen); + harness.get_by_label(RESTORE).click(); + harness.run_steps(2); + assert!( + harness.query_by_label(RESTORE).is_none(), + "the premise: a restore in flight shows progress, not another Restore" + ); + + // An unrelated task failing while the restore runs must not re-arm it. + // Errors are not screen-affine — any task's failure reaches whichever + // screen is visible — and a re-armed Restore can be pressed again, + // dispatching the same restore twice. + for unrelated in [ + BackendTaskContext::Other, + BackendTaskContext::LegacyRecoveryRestore(Identifier::from([0x7c; 32])), + ] { + screen + .borrow_mut() + .display_backend_task_error(&unrelated, &TaskError::LegacyRecoveryIdentityChanged); + harness.run_steps(2); + assert!( + harness.query_by_label(RESTORE).is_none(), + "{unrelated:?} is not this restore, so it must not re-enable Restore mid-restore" + ); + } + + // The restore's own failure does end it, so a mistyped identity password + // can be corrected and Restore pressed again. + screen.borrow_mut().display_backend_task_error( + &BackendTaskContext::LegacyRecoveryRestore(identity_id), + &TaskError::LegacyRecoveryIdentityChanged, + ); + harness.run_steps(2); + assert!( + harness.query_by_label(RESTORE).is_some(), + "a failed restore must leave its offer on screen so it can be retried" + ); + }); +} + +/// AC-6: the offer and the key list must name the same key identically. They +/// are rendered by different modules from different types, so the only thing +/// keeping them in step is that both derive their wording from the shared +/// vocabulary — this pins that, since a private re-implementation on either +/// side would read plausibly and drift silently. +#[test] +fn the_offer_and_the_key_list_name_a_key_identically() { + let voting = key(0, Purpose::VOTING); + let owner = key(1, Purpose::OWNER); + let payout = key(2, Purpose::TRANSFER); + + let node = KeyVocabulary::Masternode; + let list_labels: Vec = manage_keys_labels( + node, + &[ + (PrivateKeyTarget::PrivateKeyOnMainIdentity, voting.clone()), + (PrivateKeyTarget::PrivateKeyOnMainIdentity, owner.clone()), + (PrivateKeyTarget::PrivateKeyOnMainIdentity, payout.clone()), + ], + ) + .into_iter() + .map(|(label, _)| label) + .collect(); + + let descriptors: Vec = [ + (0, Purpose::VOTING), + (1, Purpose::OWNER), + (2, Purpose::TRANSFER), + ] + .into_iter() + .map(|(key_id, purpose)| RecoveryItemDescriptor { + item: RecoveryItem::Key { + target: PrivateKeyTarget::PrivateKeyOnMainIdentity, + key_id, + }, + purpose: Some(purpose), + }) + .collect(); + let offer_labels: Vec = + recovery_item_labels(node, &descriptors.iter().collect::>()) + .into_iter() + .map(|(label, _)| label) + .collect(); + + assert_eq!( + list_labels, offer_labels, + "the keys list and the restore offer must name the same key the same way" + ); + assert_eq!( + list_labels, + vec![ + "Voting key".to_string(), + "Owner key".to_string(), + "Payout address key".to_string() + ], + "and both must use the DIP-3 role words, not raw Platform purposes" + ); +} + +/// Clicking Back on the keys list pops it off the screen stack — and refreshes +/// what it reveals, since anything the user did above it may have changed the +/// record underneath. +#[test] +fn manage_keys_back_button_pops_the_screen() { + with_isolated_data_dir(|| { + let (_rt, app_context) = fresh_app_context(); + let identity = stranded_identity(0x33, &[Purpose::AUTHENTICATION], "back-test"); + let (mut harness, action) = harness_for(KeysScreen::new(identity, &app_context)); - harness.run(); assert!( harness.query_by_label("Back").is_some(), - "the Manage Keys screen must render a Back control" + "the keys list must render a Back control" ); harness.get_by_label("Back").click(); - harness.run(); + harness.run_steps(2); assert_eq!( *action.borrow(), - AppAction::PopScreen, - "clicking Back must pop the Manage Keys screen off the stack" + AppAction::PopScreenAndRefresh, + "clicking Back must pop the keys list and refresh the screen it reveals" + ); + }); +} + +/// AC-2, and the core of the bug: an identity that holds **no** private keys +/// still gets a per-key control, and it opens `KeyInfoScreen`. +/// +/// Every other route into that screen is gated on already holding a suitable +/// key. If this route were gated the same way, the restore offer would be +/// reachable only by identities that do not need it. +#[test] +fn every_key_opens_key_info_even_when_the_device_holds_none() { + with_isolated_data_dir(|| { + let (_rt, app_context) = fresh_app_context(); + let identity = stranded_identity( + 0x34, + &[Purpose::AUTHENTICATION, Purpose::TRANSFER], + "stranded", + ); + let (mut harness, action) = harness_for(KeysScreen::new(identity, &app_context)); + + // Both keys are listed, named in the shared role vocabulary rather + // than raw `Debug` enum output. + assert!( + harness.query_by_label(AUTH_ROW).is_some(), + "an authentication key must render a row control named in role words" + ); + assert!( + harness.query_by_label(TRANSFER_ROW).is_some(), + "a user identity's transfer key must be named in plain language" + ); + assert!( + harness.query_by_label("Payout address key ›").is_none(), + "and must not be given a masternode's payout-address framing" + ); + + harness.get_by_label(AUTH_ROW).click(); + harness.run_steps(2); + + assert!( + matches!( + &*action.borrow(), + AppAction::AddScreen(Screen::KeyInfoScreen(_)) + ), + "activating a key row must open the Key Info screen for that key" + ); + }); +} + +/// AC-2's second half, both directions: the row reports a key this device holds +/// as held, and one it does not as missing. Asserting only the negative half +/// would pass against a screen that says "not saved" about everything. +#[test] +fn a_held_key_is_reported_as_held_and_a_missing_one_is_not() { + with_isolated_data_dir(|| { + let (_rt, app_context) = fresh_app_context(); + + // Held state is stated in words, not conveyed by colour alone + // (WCAG 1.4.1) — it is the single fact a user with stranded keys + // came to this screen to find. + let missing = stranded_identity(0x35, &[Purpose::AUTHENTICATION], "held-state"); + let (missing, _) = harness_for(KeysScreen::new(missing, &app_context)); + assert!( + missing.query_by_label(NOT_HELD).is_some(), + "a key with no private material must say so in text" + ); + assert!( + missing.query_by_label(HELD).is_none(), + "and must not also claim to be held" + ); + + let held = identity_holding_key( + 0x3d, + Purpose::AUTHENTICATION, + PrivateKeyTarget::PrivateKeyOnMainIdentity, + ); + let (held, _) = harness_for(KeysScreen::new(held, &app_context)); + assert!( + held.query_by_label(HELD).is_some(), + "a key this device holds must be reported as held" + ); + assert!( + held.query_by_label(NOT_HELD).is_none(), + "and must not also claim to be missing" + ); + }); +} + +/// SEC-001: the keys list and the Key Info screen it opens must agree about +/// whether a key is held — including for a `Purpose::VOTING` key filed on the +/// main identity, the one shape where this codebase's two target conventions +/// disagree. +/// +/// The list resolves the target from the identity it walked and hands it over. +/// Key Info re-deriving it from the purpose instead lands on the voter identity, +/// finds nothing there, and reports a key the device demonstrably holds as +/// missing — on the screen built to answer that question, about a key whose +/// private half the user can see listed one screen earlier. +#[test] +fn key_info_agrees_with_the_list_about_a_voting_key_held_on_the_main_identity() { + with_isolated_data_dir(|| { + let (_rt, app_context) = fresh_app_context(); + let identity = identity_holding_key( + 0x3e, + Purpose::VOTING, + PrivateKeyTarget::PrivateKeyOnMainIdentity, + ); + app_context + .insert_local_qualified_identity(&identity, &None) + .expect("store the identity so Key Info can re-read it"); + + // What the list says. + let (mut list, action) = harness_for(KeysScreen::new(identity, &app_context)); + assert!( + list.query_by_label(HELD).is_some(), + "the list must report the key it can see the private half of as held" + ); + + list.get_by_label(VOTING_ROW).click(); + list.run_steps(2); + + // What the screen the list opens says, after it re-reads the record — + // which it does on arrival, and after any restore lands. + let opened = std::mem::replace(&mut *action.borrow_mut(), AppAction::None); + let AppAction::AddScreen(Screen::KeyInfoScreen(mut key_info)) = opened else { + panic!("the row must open Key Info"); + }; + key_info.refresh(); + assert!( + key_info.private_key_data.is_some(), + "Key Info must still hold the key the list handed it — re-deriving the \ + target from the purpose loses a voting key filed on the main identity" + ); + }); +} + +/// AC-3: the offer is identity-scoped, so it must be visible without opening +/// any individual key, and it must sit above the key list — a user who arrived +/// because their keys are missing must not have to read past the keys they do +/// have to find the remedy. +#[test] +fn the_restore_offer_renders_above_the_key_list() { + with_isolated_data_dir(|| { + let (_rt, app_context) = fresh_app_context(); + let identity = stranded_identity(0x36, &[Purpose::AUTHENTICATION], "offer-placement"); + let identity_id = identity.identity.id(); + let mut screen = KeysScreen::new(identity, &app_context); + // Answer the detection the screen dispatches on its first frame. + screen.display_task_result(BackendTaskSuccessResult::LegacyRecoveryCandidates { + identity_id, + plan: plan(), + }); + + let (harness, _) = harness_for(screen); + + let restore = harness + .query_by_label(RESTORE) + .expect("the offer must render without opening any key"); + let first_row = harness + .query_by_label(AUTH_ROW) + .expect("the key list must render alongside the offer"); + assert!( + restore.rect().top() < first_row.rect().top(), + "the identity-scoped offer must sit above the per-key list" + ); + }); +} + +/// AC-9: an identity with no keys explains itself instead of rendering an empty +/// table — and does not borrow the per-key "not saved on this device" wording, +/// which says something different (this identity has keys, just not here). +#[test] +fn an_identity_with_no_keys_shows_an_empty_state() { + with_isolated_data_dir(|| { + let (_rt, app_context) = fresh_app_context(); + let identity = stranded_identity(0x37, &[], "no-keys"); + let (harness, _) = harness_for(KeysScreen::new(identity, &app_context)); + + assert!( + harness + .query_by_label("No keys were found for this identity.") + .is_some(), + "an empty key list must say what the user is looking at" + ); + }); +} + +/// AC-1, end to end through the real app: Default view, a user identity whose +/// keys are stranded, no interface-mode change and no send flow — Identities → +/// Settings → Advanced → Manage keys → a key → Key Info. +/// +/// This is the route the bug closed. Component-level tests above prove the +/// screen behaves; this proves a user can actually get to it. +#[test] +fn key_info_is_reachable_from_the_identity_hub_in_default_view() { + use dash_evo_tool::model::user_role::UserRole; + use dash_evo_tool::ui::RootScreenType; + + with_isolated_data_dir(|| { + let rt = tokio::runtime::Runtime::new().expect("Failed to create tokio runtime"); + let _guard = rt.enter(); + + let mut harness = mount_app(RootScreenType::RootScreenIdentityHub); + // The Settings tab stacks two columns above the Advanced expander; a + // short window puts the keys controls below the fold, where a simulated + // click lands outside the widget. + harness.set_size(egui::vec2(1280.0, 1800.0)); + let app_context = harness.state().current_app_context().clone(); + // Everyday view: the persona this feature targets, and the one that + // must not need to raise its own role to reach its own keys. + app_context.set_user_role(UserRole::Everyday); + let identity = stranded_identity(0x38, &[Purpose::AUTHENTICATION], "hub-route"); + app_context + .insert_local_qualified_identity(&identity, &None) + .expect("seed the stranded identity"); + harness.run_steps(5); + + // The left nav carries its own "Settings" entry. Both are buttons; only + // the nav one is a toggle, so that is what tells the hub tab apart. + harness + .query_all_by_role_and_label(egui::accesskit::Role::Button, "Settings") + .find(|node| node.accesskit_node().toggled().is_none()) + .expect("the hub must render a Settings tab") + .click(); + harness.run_steps(3); + harness.get_by_label("Advanced").click(); + harness.run_steps(3); + + harness.get_by_label("Manage keys").click(); + harness.run_steps(3); + assert!( + matches!( + harness.state().screen_stack.last(), + Some(Screen::KeysScreen(_)) + ), + "'Manage keys' must open the identity keys list" + ); + + harness.get_by_label(AUTH_ROW).click(); + harness.run_steps(3); + + assert!( + matches!( + harness.state().screen_stack.last(), + Some(Screen::KeyInfoScreen(_)) + ), + "the keys list must open Key Info — the route this fix restores" + ); + assert!( + harness.query_by_label("Key Information").is_some(), + "the opened Key Info screen must render its heading" + ); + }); +} + +/// Both restore outcomes, reported on the real keys list inside the running +/// app — not a component harness with its own context. +/// +/// The component tests above prove the screen sets a banner and that it renders +/// whatever banner its context carries. Only this one closes the seam between +/// those two facts: that the context the screen writes its outcome into is the +/// context it renders from. It is worth its own test because the keys list only +/// gained `island_central_panel` in this change, and a restore that reported +/// nothing would be indistinguishable from one that worked — the offer retires +/// itself either way. +#[test] +fn a_restore_reports_success_and_failure_on_the_keys_list_in_the_running_app() { + use dash_evo_tool::ui::RootScreenType; + + with_isolated_data_dir(|| { + let rt = tokio::runtime::Runtime::new().expect("Failed to create tokio runtime"); + let _guard = rt.enter(); + + let mut harness = mount_app(RootScreenType::RootScreenIdentityHub); + harness.set_size(egui::vec2(1280.0, 1800.0)); + let app_context = harness.state().current_app_context().clone(); + let identity = stranded_identity(0x3c, &[Purpose::AUTHENTICATION], "restore-report"); + let identity_id = identity.identity.id(); + app_context + .insert_local_qualified_identity(&identity, &None) + .expect("seed the stranded identity"); + harness.run_steps(5); + + harness + .query_all_by_role_and_label(egui::accesskit::Role::Button, "Settings") + .find(|node| node.accesskit_node().toggled().is_none()) + .expect("the hub must render a Settings tab") + .click(); + harness.run_steps(3); + harness.get_by_label("Advanced").click(); + harness.run_steps(3); + harness.get_by_label("Manage keys").click(); + harness.run_steps(3); + + // A restore that put a key back, delivered the way the app delivers one. + match harness.state_mut().visible_screen_mut() { + Screen::KeysScreen(screen) => { + screen.display_task_result(BackendTaskSuccessResult::LegacyRecoveryCompleted { + identity_id, + applied: vec![RecoveryItemDescriptor { + item: RecoveryItem::Key { + target: PrivateKeyTarget::PrivateKeyOnMainIdentity, + key_id: 0, + }, + purpose: Some(Purpose::AUTHENTICATION), + }], + skipped_stale: vec![], + excluded: vec![], + }); + } + _ => panic!("the keys list must be the visible screen"), + } + harness.run_steps(3); + assert!( + harness.query_by_label(RESTORED).is_some(), + "a restore that landed must say so on the keys list itself" + ); + assert!( + harness.query_by_label(NOTHING_RESTORED).is_none(), + "and must not report that there was nothing to restore" + ); + + // The failing case: `AppState` banners a task error centrally, on this + // same context. The keys list has to surface that too, or a restore + // that failed reads exactly like one that worked. + MessageBanner::clear_all_global(app_context.egui_ctx()); + harness.run_steps(2); + MessageBanner::set_global(app_context.egui_ctx(), RESTORE_FAILED, MessageType::Error); + harness.run_steps(3); + assert!( + harness.query_by_label(RESTORE_FAILED).is_some(), + "a restore that failed must say so on the keys list too" + ); + }); +} + +/// QA-007: the Expert-only per-key detail row, both directions. Raw key ids, +/// purposes and security levels are Expert material; the Everyday view gets the +/// role word and the held state, which is what it can act on. +#[test] +fn raw_key_detail_is_expert_only() { + use dash_evo_tool::model::user_role::UserRole; + + with_isolated_data_dir(|| { + let (_rt, app_context) = fresh_app_context(); + let identity = stranded_identity(0x3f, &[Purpose::AUTHENTICATION], "expert-detail"); + + app_context.set_user_role(UserRole::Everyday); + let (everyday, _) = harness_for(KeysScreen::new(identity.clone(), &app_context)); + assert!( + everyday.query_by_label_contains("AUTHENTICATION").is_none(), + "the Everyday view must not show raw key internals" + ); + assert!( + everyday.query_by_label(AUTH_ROW).is_some(), + "but it must still show the key itself, named in role words" + ); + + app_context.set_user_role(UserRole::Power); + let (expert, _) = harness_for(KeysScreen::new(identity, &app_context)); + assert!( + expert.query_by_label_contains("AUTHENTICATION").is_some(), + "the Expert view must show the raw purpose it was promised" + ); + }); +} + +/// The return journey. Every other test in this suite walks *into* Key Info; +/// this one comes back, which is where the stale-offer bug lives. +/// +/// A restore dispatched from the pushed Key Info screen writes the record behind +/// this screen's back. Coming back with only the record re-read shows the keys +/// as held while still offering to restore them — and pressing Restore then +/// reports there was nothing left to do. Both halves have to be re-read on the +/// way back, which is what makes `refresh` the arrival hook that has to do both. +#[test] +fn returning_from_key_info_refreshes_both_the_keys_and_the_offer() { + use dash_evo_tool::ui::RootScreenType; + + with_isolated_data_dir(|| { + let rt = tokio::runtime::Runtime::new().expect("Failed to create tokio runtime"); + let _guard = rt.enter(); + + let mut harness = mount_app(RootScreenType::RootScreenIdentityHub); + harness.set_size(egui::vec2(1280.0, 1800.0)); + let app_context = harness.state().current_app_context().clone(); + let identity = stranded_identity(0x40, &[Purpose::AUTHENTICATION], "return-journey"); + let identity_id = identity.identity.id(); + app_context + .insert_local_qualified_identity(&identity, &None) + .expect("seed the identity"); + harness.run_steps(5); + + harness + .query_all_by_role_and_label(egui::accesskit::Role::Button, "Settings") + .find(|node| node.accesskit_node().toggled().is_none()) + .expect("the hub must render a Settings tab") + .click(); + harness.run_steps(3); + harness.get_by_label("Advanced").click(); + harness.run_steps(3); + harness.get_by_label("Manage keys").click(); + harness.run_steps(3); + + // An offer is on the list, and the key is not held yet. + match harness.state_mut().visible_screen_mut() { + Screen::KeysScreen(screen) => { + screen.display_task_result(BackendTaskSuccessResult::LegacyRecoveryCandidates { + identity_id, + plan: plan(), + }); + } + _ => panic!("the keys list must be the visible screen"), + } + harness.run_steps(3); + assert!( + harness.query_by_label(RESTORE).is_some(), + "the premise: the list is offering to restore this identity's keys" + ); + + // Into Key Info, and restore from there. + harness.get_by_label(AUTH_ROW).click(); + harness.run_steps(3); + assert!( + matches!( + harness.state().screen_stack.last(), + Some(Screen::KeyInfoScreen(_)) + ), + "the row must open Key Info" + ); + // The restore lands: the key is now held, and nothing is left stranded. + let restored = identity_holding_key( + 0x40, + Purpose::AUTHENTICATION, + PrivateKeyTarget::PrivateKeyOnMainIdentity, + ); + app_context + .update_local_qualified_identity(&restored) + .expect("the restore writes the record"); + match harness.state_mut().visible_screen_mut() { + Screen::KeyInfoScreen(screen) => { + screen.display_task_result(BackendTaskSuccessResult::LegacyRecoveryCompleted { + identity_id, + applied: plan().items, + skipped_stale: vec![], + excluded: vec![], + }); + } + _ => panic!("Key Info must be the visible screen"), + } + harness.run_steps(3); + + // Back to the list, via the breadcrumb crumb naming the screen it was + // opened from. Key Info has no Back control of its own. + harness.get_by_label(PARENT_CRUMB).click(); + harness.run_steps(5); + assert!( + matches!( + harness.state().screen_stack.last(), + Some(Screen::KeysScreen(_)) + ), + "Back must return to the keys list" + ); + + assert!( + harness.query_by_label(HELD).is_some(), + "the list must show the restored key as held" + ); + assert!( + harness.query_by_label(RESTORE).is_none(), + "and must not still offer to restore a key it just reported as held" + ); + }); +} + +/// The contested case both reviewers found in the two-candidate lookup: a main +/// identity holding a `Purpose::VOTING` key at id N, and a linked voting +/// identity holding its own, different key at the same id N. +/// +/// An occupied slot proves nothing about whose material is in it. Falling back +/// to the voter slot on key-id alone would report the main identity's key as +/// held — on the strength of a different key's private half — and hand that +/// material to Key Info. The row must be about the key the user clicked. +#[test] +fn a_same_numbered_key_on_the_voter_identity_is_not_mistaken_for_this_one() { + with_isolated_data_dir(|| { + let (_rt, app_context) = fresh_app_context(); + + // Main identity: a voting-purpose key at id 0, private half NOT held. + let main_key = key(0, Purpose::VOTING); + let identity = Identity::new_with_id_and_keys( + Identifier::from([0x41u8; 32]), + BTreeMap::from([(main_key.id(), main_key.clone())]), + PlatformVersion::latest(), + ) + .expect("main identity"); + + // Voter identity: a *different* key, also at id 0, private half held. + let voter_key = key(0, Purpose::AUTHENTICATION); + let voter_identity = Identity::new_with_id_and_keys( + Identifier::from([0x42u8; 32]), + BTreeMap::from([(voter_key.id(), voter_key.clone())]), + PlatformVersion::latest(), + ) + .expect("voter identity"); + + let qualified = QualifiedIdentity { + identity, + associated_voter_identity: Some((voter_identity, voter_key.clone())), + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::Masternode, + alias: Some("id-collision".to_string()), + private_keys: KeyStorage { + private_keys: BTreeMap::from([( + (PrivateKeyTarget::PrivateKeyOnVoterIdentity, voter_key.id()), + ( + QualifiedIdentityPublicKey::from(voter_key), + PrivateKeyData::Clear([0x42; 32]), + ), + )]), + }, + dpns_names: vec![], + associated_wallets: BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::Active, + network: dash_sdk::dpp::dashcore::Network::Testnet, + }; + + let (harness, _) = harness_for(KeysScreen::new(qualified, &app_context)); + + // Two rows: the main identity's voting key (not held) and the voter + // identity's own key (held). Exactly one of each disclosure. + assert_eq!( + harness.query_all_by_label(NOT_HELD).count(), + 1, + "the main identity's voting key holds no private material of its own" + ); + assert_eq!( + harness.query_all_by_label(HELD).count(), + 1, + "only the voter identity's own key is actually held" + ); + }); +} + +/// A key the device holds stays held after it is disabled on chain. +/// +/// The stored copy of a public key is a snapshot taken when its private half was +/// saved. Platform keys are immutable once added with exactly one exception — +/// disabling rewrites `disabled_at` — so full-struct equality between the +/// snapshot and the live key breaks the moment a key is rotated or disabled, and +/// the row reports a key whose private half is demonstrably on this device as +/// missing. Same symptom as SEC-001 from a different trigger. +/// +/// This fails safe, understating possession rather than over-, and the guard +/// against the unsafe direction is +/// `a_same_numbered_key_on_the_voter_identity_is_not_mistaken_for_this_one`: +/// the comparison has to survive `disabled_at` moving while still telling two +/// same-numbered keys apart. +#[test] +fn a_disabled_key_whose_private_half_is_saved_is_still_reported_as_held() { + with_isolated_data_dir(|| { + let (_rt, app_context) = fresh_app_context(); + + // The live key, as Platform reports it after the user disabled it. + let mut live = key(0, Purpose::AUTHENTICATION); + let disabled_at = 1_700_000_000_u64; + match &mut live { + IdentityPublicKey::V0(v0) => v0.disabled_at = Some(disabled_at), + } + let identity = Identity::new_with_id_and_keys( + Identifier::from([0x43u8; 32]), + BTreeMap::from([(live.id(), live.clone())]), + PlatformVersion::latest(), + ) + .expect("identity with one disabled key"); + + // The stored snapshot, written before it was disabled. + let snapshot = key(0, Purpose::AUTHENTICATION); + assert_eq!( + snapshot.disabled_at(), + None, + "the premise: the saved copy predates the key being disabled" + ); + + let qualified = QualifiedIdentity { + identity, + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::User, + alias: Some("disabled-but-held".to_string()), + private_keys: KeyStorage { + private_keys: BTreeMap::from([( + (PrivateKeyTarget::PrivateKeyOnMainIdentity, snapshot.id()), + ( + QualifiedIdentityPublicKey::from(snapshot), + PrivateKeyData::Clear([0x43; 32]), + ), + )]), + }, + dpns_names: vec![], + associated_wallets: BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::Active, + network: dash_sdk::dpp::dashcore::Network::Testnet, + }; + + let (harness, _) = harness_for(KeysScreen::new(qualified, &app_context)); + assert!( + harness.query_by_label(HELD).is_some(), + "disabling a key on chain does not remove its private half from this \ + device, so the row must still report it as saved here" + ); + assert!( + harness.query_by_label(NOT_HELD).is_none(), + "and must not report it as missing" + ); + }); +} + +/// QA-008: every screen hosting the offer must name a key as the identity it is +/// showing would. +/// +/// Three screens render the same offer — the keys list, Key Info, and masternode +/// detail — and each passes its own vocabulary. Scoping two of them left a user +/// identity's Key Info offer calling its transfer key a "Payout address key", +/// asserting the user owns a masternode, and disagreeing with the keys list one +/// screen away. A required constructor argument makes a *missing* vocabulary a +/// compile error, but nothing in the type system catches a host passing the +/// *wrong* one, so the host is where this has to be pinned: this drives Key Info +/// through the route the user takes and reads the words off the rendered offer. +#[test] +fn key_info_names_a_users_transfer_key_as_the_keys_list_does() { + with_isolated_data_dir(|| { + let (_rt, app_context) = fresh_app_context(); + let identity = identity_holding_key( + 0x40, + Purpose::TRANSFER, + PrivateKeyTarget::PrivateKeyOnMainIdentity, + ); + let identity_id = identity.identity.id(); + app_context + .insert_local_qualified_identity(&identity, &None) + .expect("store the identity so Key Info can re-read it"); + + // Arrive at Key Info the way the user does, from the list row. + let (mut list, action) = harness_for(KeysScreen::new(identity, &app_context)); + list.get_by_label(TRANSFER_ROW).click(); + list.run_steps(2); + let opened = std::mem::replace(&mut *action.borrow_mut(), AppAction::None); + let AppAction::AddScreen(Screen::KeyInfoScreen(mut key_info)) = opened else { + panic!("the row must open Key Info"); + }; + + // Offer Key Info a restorable transfer key, as the check would. + key_info.display_task_result(BackendTaskSuccessResult::LegacyRecoveryCandidates { + identity_id, + plan: RecoveryPlan { + items: vec![RecoveryItemDescriptor { + item: RecoveryItem::Key { + target: PrivateKeyTarget::PrivateKeyOnMainIdentity, + key_id: 0, + }, + purpose: Some(Purpose::TRANSFER), + }], + excluded: vec![], + }, + }); + + let mut harness = Harness::builder() + .with_size(egui::vec2(1100.0, 900.0)) + .build_ui(move |ui| { + key_info.ui(ui); + }); + harness.run_steps(3); + + assert!( + harness.query_by_label(RESTORE).is_some(), + "the premise: Key Info is showing the offer" + ); + // Twice over: the page's own Purpose row and the offer below it both + // render the one caption the keys list used, so a count of one would + // mean a surface had gone off and worded it for itself. + assert_eq!( + harness.query_all_by_label("Transfer key").count(), + 2, + "Key Info's offer and its Purpose row must both name the key as the \ + keys list does" + ); + assert_eq!( + harness.query_all_by_label("Payout address key").count(), + 0, + "and neither may tell a plain user their key is a masternode payout \ + address" ); }); } diff --git a/tests/kittest/legacy_recovery_section.rs b/tests/kittest/legacy_recovery_section.rs index 9ed975377..0a1f9c3bd 100644 --- a/tests/kittest/legacy_recovery_section.rs +++ b/tests/kittest/legacy_recovery_section.rs @@ -9,6 +9,7 @@ use dash_evo_tool::model::legacy_recovery::{ use dash_evo_tool::model::qualified_identity::PrivateKeyTarget; use dash_evo_tool::ui::components::legacy_recovery_section::LegacyRecoverySection; use dash_evo_tool::ui::components::{Component, ComponentResponse}; +use dash_evo_tool::ui::masternodes::KeyVocabulary; use dash_sdk::dpp::identity::Purpose; use egui_kittest::Harness; use egui_kittest::kittest::Queryable; @@ -52,7 +53,7 @@ fn render( let mut harness = Harness::builder() .with_size(egui::vec2(600.0, 400.0)) .build_ui(move |ui| { - let response = LegacyRecoverySection::new(&plan) + let response = LegacyRecoverySection::new(&plan, KeyVocabulary::Masternode) .restoring(restoring) .show(ui); if let Some(items) = response.inner.changed_value() { diff --git a/tests/kittest/main.rs b/tests/kittest/main.rs index 571f11d39..c524fe4dc 100644 --- a/tests/kittest/main.rs +++ b/tests/kittest/main.rs @@ -15,6 +15,7 @@ mod identity_hub_switcher; mod identity_selector; mod import_single_key; mod info_popup; +mod key_info_screen; mod keys_screen; mod left_panel; mod legacy_recovery_section; @@ -33,6 +34,7 @@ mod startup; mod support; mod tokens_screen; mod tools_screen; +mod transfer_screen; mod wallets_screen; mod welcome_screen; mod withdraw_screen; diff --git a/tests/kittest/masternode_tab.rs b/tests/kittest/masternode_tab.rs index 393285c74..ee335ad9f 100644 --- a/tests/kittest/masternode_tab.rs +++ b/tests/kittest/masternode_tab.rs @@ -1088,3 +1088,241 @@ fn go_to_main_screen_from_key_info_preserves_masternode_detail() { assert!(harness.query_by_label("Open mn-nav-back-01").is_none()); }); } + +/// Seed a masternode whose voter identity carries a key whose purpose is **not** +/// `VOTING`, with its private half filed structurally on the voter identity. +/// +/// This is the shape where the two target conventions disagree: the structural +/// target is the voter identity, while `impl From for PrivateKeyTarget` +/// sends anything that is not `VOTING` to the main identity. It is the shape +/// `role_label_and_tip`'s voter-identity override exists for — a key on a voter +/// identity is the voting key whatever its purpose field says. +fn seed_node_with_non_voting_purpose_voter_key( + app_context: &Arc, + byte: u8, + alias: &str, +) { + let pv = PlatformVersion::latest(); + // Purpose::AUTHENTICATION (0), not VOTING. + let voter_key = IdentityPublicKey::random_key(1, Some(1), pv); + assert_eq!( + voter_key.purpose(), + dash_sdk::dpp::identity::Purpose::AUTHENTICATION, + "the premise: this key's purpose derives to the MAIN identity, while it \ + actually sits on the voter identity" + ); + + let mut voter_identity = + Identity::create_basic_identity(Identifier::from([byte ^ 0xFF; 32]), pv) + .expect("voter basic identity"); + voter_identity.add_public_key(voter_key.clone()); + + let node_identity = Identity::create_basic_identity(Identifier::from([byte; 32]), pv) + .expect("node basic identity"); + let node_qi = QualifiedIdentity { + identity: node_identity, + associated_voter_identity: Some((voter_identity, voter_key.clone())), + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::Masternode, + alias: Some(alias.to_string()), + private_keys: KeyStorage { + private_keys: BTreeMap::from([( + (PrivateKeyTarget::PrivateKeyOnVoterIdentity, voter_key.id()), + ( + QualifiedIdentityPublicKey::from(voter_key), + PrivateKeyData::Clear([byte; 32]), + ), + )]), + }, + dpns_names: vec![], + associated_wallets: BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::PendingCreation, + network: app_context.network(), + }; + app_context + .insert_local_qualified_identity(&node_qi, &None) + .expect("seed node-with-non-voting-voter-key insert"); +} + +/// AC-6 on the masternode path: the detail view's key list and the Key Info page +/// it opens must call the same key the same thing. +/// +/// The row knows where the material is because it looked there. The page it +/// opens, left to itself, re-derives that location from the key's purpose — which +/// disagrees for a voter-identity key whose purpose is not `VOTING`. The list +/// says "Voting key" (correct: a key on a voter identity is the voting key) and +/// the page said "Authentication key", one click apart, about one key. The same +/// wrong target also drives the page's own re-read, so it loses the private half +/// of a key the device holds. +#[test] +fn key_info_names_a_voter_key_as_the_masternode_key_list_does() { + use dash_evo_tool::ui::Screen; + + with_isolated_data_dir(|| { + let rt = tokio::runtime::Runtime::new().expect("Failed to create tokio runtime"); + let _guard = rt.enter(); + + let mut harness = mount_app(RootScreenType::RootScreenIdentities); + let app_context = harness.state().current_app_context().clone(); + seed_node_with_non_voting_purpose_voter_key(&app_context, 0x9a, "mn-voter-label-01"); + activate_masternodes_tab(&mut harness, &app_context); + harness.get_by_label("Open mn-voter-label-01").click(); + harness.run_steps(3); + + // What the list calls it: the voter-identity override, from the + // structural target. + assert!( + harness.query_by_label("Voting key \u{203a}").is_some(), + "a key on the voter identity is the node's voting key, whatever its \ + purpose field says" + ); + + harness.get_by_label("Voting key \u{203a}").click(); + harness.run_steps(3); + assert!( + matches!( + harness.state().screen_stack.last(), + Some(Screen::KeyInfoScreen(_)) + ), + "the row must open Key Info" + ); + + // What the page one click later calls it — the same whole caption, with + // nothing spliced into it. + assert!( + harness.query_by_label("Voting key").is_some(), + "Key Info must name the key as the list that opened it does" + ); + assert!( + harness.query_by_label("Authentication key").is_none(), + "and must not rename it by re-deriving its location from its purpose" + ); + // The masternode surface is Expert-gated, so the raw purpose shows in a + // labelled field of its own — which is what keeps the disagreement this + // test guards against legible: purpose AUTHENTICATION, role Voting. + assert!( + harness.query_by_label("Platform purpose:").is_some(), + "the Expert view must still show the raw purpose it was promised" + ); + assert!( + harness.query_by_label("AUTHENTICATION").is_some(), + "and that field must carry this key's actual purpose" + ); + + // The same wrong target would also lose the held private half on the + // page's own re-read. + let Some(Screen::KeyInfoScreen(key_info)) = harness.state().screen_stack.last() else { + panic!("Key Info must be the open screen"); + }; + assert!( + key_info.private_key_data.is_some(), + "Key Info must keep the private half the row found for it" + ); + }); +} + +/// Seed a masternode holding a `VOTING`-purpose key on its **main** identity, +/// with the private half filed under the purpose-derived convention +/// (`PrivateKeyOnVoterIdentity`) rather than where the key structurally sits. +/// +/// This is what the paste path produces: `impl From for +/// PrivateKeyTarget` sends every voting key to the voter identity regardless of +/// which identity it actually belongs to. +fn seed_node_with_purpose_filed_voting_key(app_context: &Arc, byte: u8, alias: &str) { + let pv = PlatformVersion::latest(); + let voting_key: IdentityPublicKey = { + use dash_sdk::dpp::identity::identity_public_key::v0::IdentityPublicKeyV0; + use dash_sdk::dpp::identity::{KeyType, Purpose, SecurityLevel}; + use dash_sdk::dpp::platform_value::BinaryData; + IdentityPublicKeyV0 { + id: 0, + key_type: KeyType::ECDSA_HASH160, + purpose: Purpose::VOTING, + security_level: SecurityLevel::HIGH, + read_only: false, + data: BinaryData::new(vec![byte; 20]), + disabled_at: None, + contract_bounds: None, + } + .into() + }; + + let node_identity = Identity::new_with_id_and_keys( + Identifier::from([byte; 32]), + BTreeMap::from([(voting_key.id(), voting_key.clone())]), + pv, + ) + .expect("node identity with a voting key"); + + let node_qi = QualifiedIdentity { + identity: node_identity, + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::Masternode, + alias: Some(alias.to_string()), + private_keys: KeyStorage { + private_keys: BTreeMap::from([( + // Filed by purpose derivation, not structurally. + (PrivateKeyTarget::PrivateKeyOnVoterIdentity, voting_key.id()), + ( + QualifiedIdentityPublicKey::from(voting_key), + PrivateKeyData::Clear([byte; 32]), + ), + )]), + }, + dpns_names: vec![], + associated_wallets: BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::PendingCreation, + network: app_context.network(), + }; + app_context + .insert_local_qualified_identity(&node_qi, &None) + .expect("seed node-with-purpose-filed-voting-key insert"); +} + +/// The masternode detail view and the identity keys list must agree on whether a +/// key is saved on this device. +/// +/// `identity_keys()` is shared between the two surfaces precisely so they cannot +/// disagree about which keys exist and where their material is. Enumeration alone +/// is not enough: resolving *where* the private half sits has to be shared too. +/// The keys list tries both filing conventions; the masternode page used only the +/// structural one, so a voting key held on the main identity but filed by purpose +/// derivation — what entering one by hand produces — read as saved on one screen +/// and missing on the other. +#[test] +fn the_masternode_page_resolves_held_keys_the_way_the_keys_list_does() { + use dash_evo_tool::ui::Screen; + + with_isolated_data_dir(|| { + let rt = tokio::runtime::Runtime::new().expect("Failed to create tokio runtime"); + let _guard = rt.enter(); + + let mut harness = mount_app(RootScreenType::RootScreenIdentities); + let app_context = harness.state().current_app_context().clone(); + seed_node_with_purpose_filed_voting_key(&app_context, 0x9b, "mn-filed-by-purpose-01"); + activate_masternodes_tab(&mut harness, &app_context); + harness.get_by_label("Open mn-filed-by-purpose-01").click(); + harness.run_steps(3); + + harness.get_by_label("Voting key \u{203a}").click(); + harness.run_steps(3); + + let Some(Screen::KeyInfoScreen(key_info)) = harness.state().screen_stack.last() else { + panic!("the key row must open Key Info"); + }; + assert!( + key_info.private_key_data.is_some(), + "the node's page must find the private half wherever it is filed, as \ + the identity keys list does — not only under the structural target" + ); + }); +} diff --git a/tests/kittest/transfer_screen.rs b/tests/kittest/transfer_screen.rs new file mode 100644 index 000000000..998804337 --- /dev/null +++ b/tests/kittest/transfer_screen.rs @@ -0,0 +1,91 @@ +//! Kittest coverage for the transfer screen's no-transfer-key branch. +//! +//! The branch exists to say "you cannot send from this identity". It used to +//! offer only "Add key" — typing a private key by hand — which is the wrong +//! remedy for an identity whose keys are merely stranded in the previous +//! version's saved data. It now also signposts the keys list, where those keys +//! can be brought back. The gate deciding the branch is deliberately untouched: +//! it still correctly reports that the identity cannot send. + +use crate::support::{fresh_app_context, with_isolated_data_dir}; +use dash_evo_tool::app::AppAction; +use dash_evo_tool::model::qualified_identity::encrypted_key_storage::KeyStorage; +use dash_evo_tool::model::qualified_identity::{IdentityStatus, IdentityType, QualifiedIdentity}; +use dash_evo_tool::ui::identities::transfer_screen::TransferScreen; +use dash_evo_tool::ui::{Screen, ScreenLike}; +use dash_sdk::dpp::identity::Identity; +use dash_sdk::dpp::version::PlatformVersion; +use dash_sdk::platform::Identifier; +use egui_kittest::Harness; +use egui_kittest::kittest::Queryable; +use std::cell::RefCell; +use std::collections::BTreeMap; +use std::rc::Rc; + +/// An identity with no transfer key loaded, which is what puts the transfer +/// screen into the branch under test. +fn identity_without_transfer_keys() -> QualifiedIdentity { + let identity = + Identity::create_basic_identity(Identifier::from([0x51u8; 32]), PlatformVersion::latest()) + .expect("basic identity"); + QualifiedIdentity { + identity, + associated_voter_identity: None, + associated_operator_identity: None, + associated_owner_key_id: None, + identity_type: IdentityType::User, + alias: Some("no-transfer-key".to_string()), + private_keys: KeyStorage::default(), + dpns_names: vec![], + associated_wallets: BTreeMap::new(), + secret_access: None, + wallet_index: None, + top_ups: BTreeMap::new(), + status: IdentityStatus::Active, + network: dash_sdk::dpp::dashcore::Network::Testnet, + } +} + +/// The no-key branch signposts the keys list instead of dead-ending on "Add +/// key", and clicking it opens that list. +#[test] +fn the_no_transfer_key_branch_routes_to_the_keys_list() { + with_isolated_data_dir(|| { + let (_rt, app_context) = fresh_app_context(); + let mut screen = TransferScreen::new(identity_without_transfer_keys(), &app_context); + + let action = Rc::new(RefCell::new(AppAction::None)); + let capture = action.clone(); + let mut harness = Harness::builder() + .with_size(egui::vec2(1280.0, 900.0)) + .build_ui(move |ui| { + let act = screen.ui(ui); + if act != AppAction::None { + *capture.borrow_mut() = act; + } + }); + harness.run_steps(3); + + // The premise: this identity cannot send, so the branch is on screen. + assert!( + harness.query_by_label("Add key").is_some(), + "an identity with no transfer key must be in the no-key branch" + ); + + assert!( + harness.query_by_label("Manage keys").is_some(), + "the no-key branch must offer a way to see this identity's keys" + ); + + harness.get_by_label("Manage keys").click(); + harness.run_steps(2); + + assert!( + matches!( + &*action.borrow(), + AppAction::AddScreen(Screen::KeysScreen(_)) + ), + "the signpost must open the keys list, where a stranded key can be restored" + ); + }); +}