feat: platform-wallet backend rewrite (spec + implementation)#860
feat: platform-wallet backend rewrite (spec + implementation)#860lklimek wants to merge 649 commits into
Conversation
|
Important Review skippedToo many files! This PR contains 542 files, which is 392 over the limit of 150. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (542)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
refactor: complete data.db unwire — shielded + DashPay (stacked on #860)
Doc sync —
|
SPV-start fix (PROJ-001, CRITICAL) + consolidated gap report —
|
DetKv per-object refactor + platform bump to
|
Mock/stub findings + SPV progress + DashPay fund-routing —
|
Platform dep bump →
|
Dead-code cleanup + gap-audit reconciliationPushed 2 commits (
Net: open-gap count holds at 17, but the audit books are accurate and a previously-hidden wiring defect (PROJ-012) is now surfaced as a real finding. 🤖 Co-authored by Claudius the Magnificent AI Agent |
Seedless wallet rehydration (PROJ-010) + platform → PR #3692Pushed 5 commits (
Planning docs (rode along): rehydration design (
🤖 Co-authored by Claudius the Magnificent AI Agent |
Fix: no-password hydrated wallets couldn't sign (Smythe SEC-001/002)
Symptom: after the seedless rehydration, a no-password HD wallet hydrated as Root cause: seeds reach Fix (one chokepoint, every path funnels through it):
The account-xpub fund-routing gate and seedless load path are untouched. 613 lib tests pass, clippy clean. Smythe re-verify in flight.
🤖 Co-authored by Claudius the Magnificent AI Agent |
JIT secret-access refactor — HD seed no longer parked in memory (R3 complete)Pushed 24 commits ( Core (
QA (three independent passes)
Also ( CHANGELOG + user-story WAL-006 updated for the new unlock model.
🤖 Co-authored by Claudius the Magnificent AI Agent |
NEW since last push — Settings Disconnect fix + persister-gap marker + nonce-ownership upstream issue (
|
NEW since last push — re-pin to dashpay/platform#3828: persister-loop fix + Orchard security patch + shielded-API port (
|
NEW since last push — fix PROJ-028/030 nullifier-cursor regression + v0.10-dev feature-parity audit (
|
|
I checked the asset-lock recovery angle from the JS SDK discussion. The identity paths look correct in this PR: But I do not think PR 860 fully handles the same issue for the non-identity asset-lock flows yet:
Those manual paths bypass the upstream So the fresh asset-lock transaction itself is tracked better than JS SDK, but these DET call sites still have a post-asset-lock / pre-final-accounting gap: Platform submit failures or confirmation ambiguity can leave a valid tracked lock in an intermediate/reusable state, and successful submits are not consistently marked Suggested fix: route these non-identity flows through the upstream wallet orchestrators, or mirror the same orchestration locally: resume/upgrade tracked proof, |
|
Verified your analysis against current source — it holds. Here's how PR #860 now addresses it. Shielded asset-lock flow ( Platform-address flows ( Also surfaced by review: the four sibling shielded spend fns ( All tracked in 🤖 Co-authored by Claudius the Magnificent AI Agent |
|
Correction to my earlier reply. I previously said the platform-address consume/retry orchestration was upstream-gated and not possible from DET. That was wrong — it is reachable through the public What landed (pushed):
Commits: Thanks for the catch — your read on the recovery gap was correct, and so was the implication that DET should route through the upstream orchestration. 🤖 Co-authored by Claudius the Magnificent AI Agent |
Two reliability fixes from live testing — crash visibility + DAPI ban storm (
|
Re-pin platform deps to PR dashpay/platform#3828 branch (
|
NEW since last push —
|
Follow-up landed —
|
NEW since last push — shielded backend retired in favour of upstream
|
NEW since last push —
|
Session update — 5 commits pushed (
|
|
Pushed The B-2 barrier is verified as an interim stopgap (closes the common coordinator-thread drain race). The durable fix — a reusable cancel + await-exit shutdown primitive ( 🤖 Co-authored by Claudius the Magnificent AI Agent |
…, and a redundant predicate Four small cleanups; every deletion grep-verified against both the lib and the integration-test crates first. - Ephemeral review-IDs: strip the per-run tags from committed source — the `RUST-002` tag on the app.rs banner-flash TODO (the substance and the #660 issue link stay) and the `CODE-027` prefixes on two settings_db doc comments (replaced with a plain statement of the read-modify-write guarantee). The standards-body `RUSTSEC-2025-0141` reference is deliberately kept. - Tombstones: drop the two deleted-code tombstones in `database/wallet.rs` `get_wallets` (git is the record) and renumber the trailing "step 8" to "step 4" so the step comments read coherently. - Dead code: delete genuinely-unused speculative items and the code they guard — three unconstructed `BackendTaskSuccessResult` variants, the `CoreTask:: GetBestChainLock` variant plus its handler / PartialEq arm / classification test (and the now-orphaned `AppContext::rpc_error_with_url` its handler was the sole caller of), a duplicate `decrypt_private_data`, an unused avatar `to_grayscale`, three unused `KeyStorage` accessors, an unused token-info constructor, a write-only `core_address` field, and a dead visualizer search field. Items that turned out to be live are handled correctly instead of deleted: `AppAction::Refresh`, `IdentityTask::SearchIdentityFromWallet`, `KeyStorage::keys_set`, and `send_screen`'s `selected_wallet_seed_hash` had stale `#[allow(dead_code)]` (the attribute is simply removed); `CORE_APPLICATION` is non-Linux-only, so it becomes `#[cfg(not(target_os = "linux"))]` and is only compiled where used; and `MessageBanner::{has_global, set_auto_dismiss}` plus `build_identity_registration` are exercised by the kittest / backend-e2e integration-test crates (a separate compilation the lib does not see), so they keep `#[allow(dead_code)]` with a reason comment rather than `#[expect]`, which would be unfulfilled under the `--all-targets` gate. - Redundant predicate: inline `is_distinct_change_candidate` (a one-line `candidate != destination` wrapped in a documented fn with two dedicated tests) at its single call site and delete the fn and its tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Present-state architectural rationale about a module boundary — Platform per-address balances and the sync cursor are owned by the upstream coordinator, so DET keeps no at-rest copy and warm-starts from the coordinator's first push. This is an external-constraint explanation, not a git-history tombstone, and was removed in error during the tombstone cleanup. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…y-scoped submodules Pure code-move: converts the 4305-line `src/context/wallet_lifecycle.rs` (the repo's largest file) into a module directory whose `impl AppContext` methods are grouped by responsibility across submodules — the same multi-impl-of-one-struct layout `wallet_backend/` already uses. No logic, signature, or behavior change; the only signature edits are two visibility widenings noted below. New structure (`src/context/wallet_lifecycle/`): - `mod.rs` — module docs, shared imports/constants, the three free helpers (`spv_storage_dir`, `clear_spv_chain_storage`, `cleanup_legacy_shielded_files`), the `AppContext::wallet_arc` lookup, and the submodule + test declarations. - `spv.rs` — backend wiring and chain-storage lifecycle: `clear_spv_data`, `clear_network_database`, `ensure_wallet_backend_and_start_spv`, `mark_spv_error`, `stop_spv`. - `registration.rs` — single-key import and HD-wallet registration/persistence: `import_single_key_wif`, `verify_single_key_passphrase`, `register_wallet`, `register_wallet_upstream`, `write_seed_envelope`, `write_wallet_meta`, `promote_seed_to_session`. - `removal.rs` — `remove_wallet`. - `bootstrap.rs` — address derivation and post-unlock warmup: the `bootstrap_*`/`wallet_needs_bootstrap` methods, managed-identity reconciliation and contact-account/auth-key warming, and the identity-discovery queue. - `unlock.rs` — lock/unlock handling and the open-wallet snapshot: `handle_wallet_unlocked`, `drive_unlock_registration`, `handle_wallet_locked`, `open_wallets`, `unregistered_open_wallet_count`. - `tests.rs` — the `#[cfg(test)]` module, unchanged. Submodules access `AppContext`'s private fields as descendants of `context` (no field-visibility change) and share `mod.rs`'s imports via `use super::*`. The only forced visibility widenings: `open_wallets` and `reconcile_managed_identities` go from private to `pub(super)` because they are now called from a sibling submodule (and, for both, from the test module). Verified move-integrity: the set of `impl AppContext` method names is identical before/after, the test-function count is unchanged (56), clippy `--all-features --all-targets -D warnings` is clean, and the full `--lib` suite passes 1393/1393. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Identity Hub hero card rendered a 14%-opacity blue→purple wash that read as a heavy blue block rather than the subtle diagonal accent the wireframe implied. Per the design rule that wireframes govern placement while colors follow the app theme, drop the gradient entirely: the card is now a flat `DashColors::surface` fill with the existing border and elevation. This supersedes the earlier gradient-containment commit — with no gradient, the reserved-slot machinery and its geometry tests are removed along with the now-unused colour-lerp helper. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…sts; recover remaining identities-screen lock Resolves the QA blockers on the branch. - Merge gate (workspace test) was red: the Wave-6 god-file split moved the wallet_lifecycle test fixtures — which construct legacy-schema `wallet` / `single_key_wallet` rows — from the monolithic file into the split module's `tests.rs`, but `tests/legacy_table_surface.rs`'s allow-list still pointed at the old path, so TC-DEV-001/003 fired on the moved (previously-allowed) fixtures. Repoint the single allow-list entry to `src/context/wallet_lifecycle/tests.rs`. Verified the guard patterns (`FROM wallet`, `INTO single_key_wallet`) appear only in that test file — no production submodule contains legacy-table SQL — so the exemption is unchanged, only its path. - The unwrap sweep missed one `self.identities.lock().unwrap()` in `identities_screen.rs::ui()` (it was split across lines, so the single-line pass skipped it): on a poisoned identities mutex that screen would panic on every repaint instead of self-healing. Convert it to `.lock_recover()` like its ten siblings. A multi-line re-scan of the sweep-touched files found the same line-wrapped gap in four more production wallet/single-key `RwLock` reads (add_existing / add_new identity screens, wallets screen); convert those to `.read_recover()` too. Remaining line-wrapped matches are all test-only code (`#[cfg(test)]` modules, `kv_test_support`) and are left as-is. - Refresh the `poison.rs` module doc to describe its present usage surface — the loaded-wallet map, per-screen view state and settings cache, the Identities / DPNS / token-search screen caches, the coordinator-gate slot, and the SQLite connection mutex — rather than only the two wallet-backend caches it originally named, and note the DashPay address-index mutex's deliberate fail-loud exception. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix: round-2 full-repo architecture audit (21 waves + shielded-screen consolidation)
…et rewrite (#876) * fix(withdraw): pre-select only a locally-signable withdrawal key The Withdraw screen constructor pre-selected a key via the on-chain lookup `identity.get_first_public_key_matching(TRANSFER, ...)`, which is unfiltered by local private-key presence. On loaded masternode/evonode identities where only the Owner key was supplied, this picked a "ghost" Transfer key with no local private material, so the withdrawal failed at signing with a raw, unhelpful protocol error. - model: add `QualifiedIdentity::default_withdrawal_key()` — sourced from `available_withdrawal_keys()` (private-key-backed only), Transfer preferred with Owner fallback, `None` when nothing is signable. - ui: constructor now pre-selects via `default_withdrawal_key()`; the developer-mode on-chain escape hatch is preserved. When no usable key exists the existing empty-state guides the user to add one. - error: add `TaskError::NoWithdrawalSigningKey` (typed `#[source]`, plain-language actionable Display) mapping the SDK `DesiredKeyWithTypePurposeSecurityLevelMissing` protocol error as a defense-in-depth backstop instead of leaking a raw string. - tests: 4 model cases (ghost key rejected, private-backed selected, owner fallback, transfer preferred) + 2 error-mapping/Display cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(claude): correct secret-storage note on identity-key encryption Identity keys (imported/loaded, including masternode voting/owner/payout) are no longer categorically in the deferred keyless tier: they enter unprotected at load time but can be sealed to Tier-2 per-identity via IdentityTask::ProtectIdentityKeys (Key Info screen "Add password protection"). Clarify that the keyless residual is only no-password secrets and keys the user has not opted to protect. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(masternodes): add page-nav model with two-scope selection (A1) Introduce ui/state/global_nav.rs: PageNavSpec (page-aware segment-1 + per-page pill composition) and IdentityPillScope (AppGlobalUser vs PageScopedObject). The PageScopedObject variant carries its own selection and never writes AppContext::selected_identity_id — the structural FR-6 boundary the global switcher (A2) and the Masternodes page (B7) build on. Pure state, renders nothing (module-placement discriminator -> ui/state). Satisfies TC-NAV-13, TC-NAV-14, TC-NAV-15; foundation for TC-NAV-12, TC-FR6-07. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): generalize breadcrumb into page-aware global switcher (A2) Add ui/components/global_nav_switcher.rs: a page-aware switcher driven by PageNavSpec, rendering segment-1 (page label + link) plus composable wallet / identity-or-object pills. GlobalNavEffect generalizes the hub's BreadcrumbEffect and adds SelectPageObject for the page-scoped pill — kept distinct from SelectIdentity so a page-scoped selection never writes the app-global identity (FR-6 boundary at the effect level). Reuses BreadcrumbPill / IdentityPill / BreadcrumbPillMode verbatim (no new pill widget). breadcrumb_switcher.rs becomes a thin hub-facing shim that builds the hub spec, delegates to the generalized render, and maps the effect back (self-nav to the hub root -> OpenPicker), keeping hub behavior unchanged — verified by the existing identity_hub_switcher kittests. Satisfies TC-NAV-01, TC-NAV-02, TC-NAV-03, TC-NAV-13, TC-NAV-16. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): render global switcher on root screens + shared applier (A3) Add the shell glue in top_panel.rs: apply_global_nav_effect (the shared successor to the hub's apply_breadcrumb_effect — silent app-scoped wallet/identity writes, no forced navigation) and add_top_panel_with_global_nav (one-call render-plus-apply), with subdued_everyday_spec / subdued_wallet_only_spec Phase-A rollout helpers. Wire the switcher onto four non-Hub root screens with Subdued (unwired) specs + TODO markers: Identities, DPNS, DashPay (everyday: wallet + identity pills) and Wallets (wallet-only composition, TC-NAV-15). The Hub keeps its existing interactive pills via the breadcrumb shim (regression — full kittest suite green). Document (comment + test, PROJ-010) that set_selected_hd_wallet reconciles the app-global identity as a side effect on non-Hub pages, and that combined with B1's resolution-layer filter it must never reconcile onto an MN/Evonode. Deferred (documented): tokens/tools screens carry in-header sub-navigation that needs the FR-GLOBAL-NAV-5 content-panel back-row migration before their plain breadcrumb can be swapped for the global switcher — a follow-up, not a mechanical swap. Satisfies TC-NAV-06, TC-NAV-15, TC-NAV-16; enables TC-NAV-12/17. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): load-time key encryption plumbing (B0) FR-8: add IdentityInputToLoad.encryption_password: Option<Secret>. When Some, load_identity validates the password up front (fast fail) then, after insert migrates the keyless keys into the vault, seals them Tier-2 through the existing per-identity protect envelope (protect_identity_keys → put_secret_protected via the secret_seam chokepoint) — no new crypto, no second persistence path. When None, the keyless Tier-1 path is unchanged. Relocate validate_protection_password from protect_identity_keys.rs into model/identity_key_protection.rs (PROJ-006, DET validation-placement rule); the seal path and load path both call the model validator. MCP masternode_identity_load passes encryption_password: None (PROJ-007 — GUI-only this iteration, requirements §2.3) with a TODO for headless password parity. Add typed TaskError variants DuplicateProTxHash { identity_id } and MalformedProTxHash { input } for later duplicate/malformed rejection (B1/B4), avoiding string parsing. Tests: model validator (relocated); an offline-wired-AppContext test proving a load-time password seals a masternode's voting (V-target), owner and identity (M-target) keys Tier-2 and round-trips under the password — the exact call load_identity makes (TC-FR8-01/02/10). Load-form UI is B4; end-to-end load routing is covered by the network backend-e2e suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(masternodes): drop ephemeral review ID from A3 reconciliation comment Self-review: replace a transient review-finding ID in the apply_global_nav_effect reconciliation note with the durable FR reference. Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): FR-6 resolution-layer boundary + masternode accessor (B1) FR-6 (R1, release-blocking): keep masternode/evonode identities out of every everyday-user surface by filtering at the resolution layer, not the display call sites. - resolve_selected_identity(): candidate set filtered to IdentityType::User before resolving, so neither keep-if-loaded nor the first-loaded fallback can ever resolve a masternode — even when a masternode is the only/first loaded identity (TC-NAV-12b). - set_selected_hd_wallet(): the wallet-switch identity reconciliation resolves only over the wallet's User identities. - restore_selected_identity_from_kv(): one-time sanitization — a masternode persisted as selected_identity_id in a prior session is cleared on load; a User selection is kept (TC-NAV-12c). In-memory only (non-destructive). - Display sources switched to the established User-only accessor load_local_user_identities(): the global switcher's identity pill + dropdown and the Identity Hub landing/picker now list User identities only, so the wallet-less "no wallet on this device" group can no longer surface an MN/Evonode (TC-NAV-17). Masternodes stay in the legacy Identities table (unfiltered accessor untouched — locked decision #2). New context accessor load_local_masternode_identities() (hydrated MN/Evonode) — the Masternodes-page card list + page-scoped pill source (B3/B7). Tests: offline-wired-AppContext unit tests (accessors, resolution filter incl. lone-masternode fallback, stale-MN sanitization) + a Hub kittest asserting a seeded Masternode+Evonode never appear on the hub while remaining in the masternode accessor. TC-FR6-01…06, TC-NAV-12b/12c, TC-NAV-17. Deferred to consuming tasks (documented): FR-7 refresh (TC-FR7-02/03) composes existing RefreshIdentity + contested-names refresh at the card Refresh button (B3); the per-node open-contest card read accessor lands in B3 where the card consumes it and it is testable against the rendered status line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): register Expert-gated Masternodes root tab (B2) Add RootScreenType::RootScreenMasternodes (stable int 28, round-trip test), ScreenType::Masternodes, Screen::MasternodesScreen, create_screen and all ScreenLike dispatch arms; register the always-present root screen in app.rs (gated at runtime by Expert Mode, not a Cargo feature, so the screen exists to switch into when the gate is on). Nav: a "Masternodes" left-panel entry gated FeatureGate::DeveloperMode, positioned directly below the identity cluster (locked decision #3), independent of the identity-hub feature. Distinct glyph voting.png (TODO: dedicated node/server icon). The existing per-entry gate skip hides the nav item and route when Expert Mode is off. Live de-gating (§10.11): active_root_screen_mut falls the active tab back to Identities (always registered) if Expert Mode flips off while Masternodes is selected, so the gated screen is never shown without its gate. MasternodesScreen is a scaffold (global-nav header + left rail + island placeholder); the empty state + card grid land in B3, the page-scoped masternode pill in B7. Network-switch already calls change_context on main_screens; the sub-screen reset (§10.10) applies once B4/B5 push sub-screens (noted for B8). Tests: RootScreenType round-trip (int 28 stable); kittest — nav absent Expert-off / present Expert-on, and de-gating falls back to Identities. TC-FR1-01…07, TC-EDGE-05/06. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): empty state + card grid + card body (B3) Render the Masternodes root screen content on top of the B2 scaffold: - Empty state (FR-2): canonical §7 copy — heading, body, "Load a masternode" primary CTA, and the ProTxHash reassurance line. - Card grid (FR-3): responsive minmax(260,1fr) grid reusing the identity picker's visual language via a new `MasternodeCard` (monogram + `draw_type_badge`, both now `pub(crate)`). Body adds masternode rows the picker lacks: voter readiness, compact `V O P` key status (glyph, not colour-only — NFR-6), DPNS status line, and the IdentityStatus dot+label. - DPNS status precedence (§10.1): open-contest count first, then a pending scheduled vote, then "No open contests", via a display-layer `AppContext::masternode_contest_summary` read (no new backend concept). - Key presence: `QualifiedIdentity::masternode_key_presence` maps Voting/Owner/Payout to voter-identity / OWNER / TRANSFER keys. - Top-right Refresh toolbar button (FR-7) reloads the cached node list. - Whole card is a single accessible click target (`WidgetInfo::labeled`, NFR-6); selection/load intents are captured for B4/B5a/B7 wiring. Traceability: TC-FR2-01…07, TC-FR3-01…15, TC-FR7-01, TC-NFR6-01/03. Unit tests cover heading/sub-line, DPNS precedence, key tokens, badge, and the 8 V/O/P combinations; kittest covers empty-state copy and the grid. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): dedicated load form + ProTxHash validator (B4) Add the MN/Evonode-only load flow (FR-4), carved out of the generic add-existing-identity path: - `ui/masternodes/load_form.rs`: ProTxHash (required), Masternode/Evonode segmented toggle (default Masternode, no User option), optional alias, V/O/P key inputs (reused hold-to-reveal `PasswordInput`), optional at-load encryption password (drives B0's seal), always-visible Warning-tone key-storage note, and a Load button gated on a non-empty ProTxHash with the §7 disabled tooltip. Switching node type clears all fields (§10.6). No auto-derive affordance — masternode keys are never wallet-derived (US-6 retired, §Locked-#4). - `model/masternode_input.rs`: `is_valid_pro_tx_hash` shape validator (hex or Base58) for inline on-blur validation; the backend load task remains the authoritative existence/duplicate check. - Masternodes screen gains a List/Load view enum; the empty-state CTA and a `+ Load` toolbar button open the form; submit dispatches `IdentityTask::LoadIdentity`; cancel/submit return to the list with a fresh form on reopen. - `add_existing_identity_screen`: remove Masternode/Evonode from the Advanced-Options Identity-Type dropdown (User-only remains) — no competing entry point (§10.2 / TC-FR4-22, FR-6). Traceability: TC-FR4-01…22 (render/logic; live-network accept/duplicate/ error-banner paths land in B8), TC-EDGE-01/02. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): detail view — header, actions, keys, remove (B5a) Add the node detail view (FR-5), reusing existing screens rather than reimplementing: - Fixed section order Header → Actions → Keys → DPNS → Remove (TC-FR5-01, the human-requested Actions-above-Keys correction), pinned by a unit test. - Header: conditional alias, shortened ProTxHash + copy-full-value, type badge (shared `draw_type_badge`), IdentityStatus dot + label. - Actions row (FR-9): Withdraw / Top up / Transfer push the existing WithdrawalScreen / TopUpIdentity / TransferScreen scoped to the node's QualifiedIdentity (both node types). Evonode-only `Claim token rewards ›` cross-link (FR-11), absent for a plain masternode. - Keys section (FR-10): V/O/P presence, copyable voter-identity id, at-rest protection tier (vault-scheme probe), Add-protection offered only Tier-1, `Manage keys ›` into the existing key screen. - DPNS section: collapsible, open-contest count in the header (voting table lands in B5b). - Remove: danger ConfirmationDialog; deletes the node and its voter identity. - `‹ All masternodes` back row + detail Refresh (FR-7/TC-FR7-04); card click opens the detail via a List/Load/Detail view enum. Deviations (documented): the Evonode claim cross-link routes to the Tokens area — precise ClaimTokensScreen token-scoping is deferred to B8 where the evonode reward-token context is resolvable. Add-protection routes into the reused key screen (which hosts the password-entry seal flow) rather than duplicating the form. Traceability: TC-FR5-01…05/07, TC-FR7-04, TC-FR9-01/02, TC-FR11-01/02. Live-network credit/claim routing and TC-FR8-07 land in B8. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): Testnet Fill-Random on the load form (B6) Add the FR-12 dev convenience to the masternode load form: - New `testnet_fixture` module owns the `.testnet_nodes.yml` structs + loader. The loader returns None for BOTH a missing and a malformed file — a malformed file is logged at debug and treated as absent (TC-FR12-04, a deliberate divergence from the legacy screen which banners the parse error). - Fill-Random button + hint render only when Expert Mode is on, the network is Testnet, and the fixture is present — never shown-but-disabled (TC-FR12-01…06). The `dev_mode` gate is a defense-in-depth re-check at the call site (TC-FR12-09 decision: added on future-proofing grounds — a plaintext-key dev tool stays inside the Expert-Mode envelope). - Button label follows the node-type toggle (TC-FR12-01/02). - Autofill pulls from the correct list per type (TC-FR12-07/08): Masternode → `masternodes` (Voting + Owner only; the fixture has no payout key, PROJ-003), Evonode → `hp_masternodes` (all three keys). The node-type toggle still clears autofilled fields (§10.6). - The fixture loads once when the form opens (Testnet only), not per frame. Traceability: TC-FR12-01…09. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): inline DPNS voting + missing-voter prompt (B5b) Populate the detail view's collapsible DPNS section (FR-5): - Collapsed by default; header shows the open-contest count (`DPNS name contests to vote on (N)`, TC-DPNS-01/02). - Voter present + open contests: per-contest Abstain / Lock / Vote-for-candidate choices with the candidate list scoped to that contest's contestants; a `Cast votes` button dispatches the existing `ContestedResourceTask::VoteOnDPNSNames` backend inline (locked decision #1 — not a deep-link). TC-DPNS-03/04/05. - Voter present, zero open contests: exact §7 empty copy (TC-DPNS-08). - Missing voter identity: the actionable §7 message (never the raw NoVotingIdentity error) plus an `Add voting key` action that opens a scoped, in-place voter-key prompt with the node context pre-bound — distinct from FR-4's load form, no ProTxHash re-entry (TC-DPNS-09/10/11, §10.8). Save re-loads this node with just the voting key to update its voter identity. - Detail Refresh now re-reads both the contest summary and the open-contest list. Active/open contests only — scheduled/past history stays on the DPNS Scheduled Votes screen (§10.7). Traceability: TC-DPNS-01…11. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): page-scoped nav pill — FR-6 boundary in code (B7) Wire the Masternodes page into the global-nav switcher with a page-scoped masternode pill whose selection lives on the page and is NEVER written to `AppContext::selected_identity_id` — the structural FR-6 boundary in code, complementing B1's resolution-layer filter. - New `ui/state/masternodes_view.rs` builds the page's `PageNavSpec`: page-aware `Masternodes` segment-1, an interactive wallet pill (funds Top up — FR-9), and a page-scoped identity pill via `IdentityPillScope::PageScopedObject`. Empty → subdued `(no masternode yet)` placeholder; ≥1 node → interactive dropdown with `Choose a masternode` placeholder; the pill reflects the node in detail and resets to the placeholder on `‹ All masternodes` (§10.4). - New `top_panel::add_top_panel_with_global_nav_capturing` surfaces the `SelectPageObject` pick to the caller (applying all other effects as usual) without ever routing it into the app-global identity selection. - The Masternodes screen builds the spec each frame from its node list + current view and opens the picked node's detail — two-way with the card grid. TC-NAV-12 / TC-FR6-07 (release-blocking): a masternode selected on the page never becomes, or resolves as, the app-global identity — verified across Identities and the Identity Hub with no User identity loaded. Traceability: TC-NAV-01/03/04/05/07, TC-NAV-12, TC-FR5-06, TC-FR6-07. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(masternodes): cross-cutting integration coverage (B8) Add the Remove-flow integration kittest (TC-US4-01/02/06/07): the detail danger button opens a confirmation carrying the `Remove masternode` verb, and confirming deletes only the target node — its card disappears while other nodes survive (isolation). Also sets the confirmation's confirm verb to `Remove masternode` (§7 / TC-US4-02), the one small production touch the test surfaced. Deferred to the network/backend-e2e pass (out of kittest reach without live DAPI or heavy vault fixtures, and flagged as such in the plan): TC-FR8-07 (detail reflecting a load-time Tier-2-sealed node — needs the real password-at-load seal path), TC-US4-05 voter-row deletion assertion (needs a seeded voter-identity row), and the live-network credit/vote/claim dispatch paths behind FR-9/FR-11/DPNS Cast-votes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(user-stories): catalog the Masternodes tab, retire the legacy load story Adds MN-001..MN-009 (load, card grid, vote, remove, Hub filter, at-load encryption, credit actions, key management, evonode token-reward cross-link) and UX-003 (global wallet/identity switcher) per the completed Masternodes feature. Flips IDN-003 to superseded — its generic-screen masternode load path was removed when the dedicated tab shipped. * docs(masternodes): commit final design docs (DOC-002) Lands the human-accepted requirements, UX spec, test-case spec, and dev plan under docs/ai-design/, following the existing 2026-04-23-identity-hub-impl numbered-file convention. Resolves the ~100+ FR-/TC-/US-/§-ID references already scattered through the feature's code comments and tests, which pointed at an uncommitted /data/artifacts scratch copy. Internal cross-file references (requirements.md, ux-spec.md, etc.) are updated to the new numbered filenames. * docs(masternodes): trim oversized module docs, catalog global-nav switcher Shortens the four ui/masternodes/*.rs module doc comments to the internal-tier length cap (DOC-003) — they weren't published API, so the 5-10 line public-rustdoc relaxation didn't apply. Adds GlobalNavSwitcher and its top_panel entry point to ui/components/README.md's catalog (DOC-004), so the next screen needing a page-aware switcher finds it instead of reimplementing one. * fix(masternodes): guard identity load against silent overwrite (QA-005/006) Root-cause storage fix. insert_local_qualified_identity is INSERT OR REPLACE, so a load with no guard silently clobbers an already-stored identity and its keys. Thread an IdentityLoadMode through IdentityInputToLoad so each entry point declares intent: - RejectIfExists: the masternode load form rejects a duplicate ProTxHash with TaskError::DuplicateProTxHash before any network fetch (QA-006). - MergeIntoExisting: the scoped Add-voting-key prompt merges the new key into the stored identity, preserving Owner/Payout it did not resupply (QA-005), via merge_existing_keys_into. - Overwrite: legacy User re-load and headless flows unchanged (default). Adds get_local_qualified_identity accessor backing the existence check and merge read. Failing-first TDD: a unit test proving Owner/Payout keys survive a voting-key-only merge, and an offline test proving a duplicate ProTxHash is rejected and the first node is left untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): key routing, network-switch reset, live refresh QA-007: the detail Keys section pushed the static read-only KeysScreen. Render a per-key 'Manage keys' list and route the Add-protection CTA to KeyInfoScreen (interactive view/sign/seal per key), mirroring identities_screen. QA-001: MasternodesScreen had no change_context override, so a network switch left an open load form or cross-network detail view actionable. Add an explicit change_context arm that resets to the List view and reloads from the now-active network. QA-003: both Refresh buttons only re-read the local cache. Wire them to dispatch IdentityTask::RefreshIdentity (per loaded node on the list, the open node on detail) plus a QueryDPNSContests re-query, alongside the optimistic local re-read. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): SHOULD-FIX pass + QA-token cleanup + offline tests - QA-002: remove the legacy Fill-Random HPMN/Masternode bypass from the now User-only add-existing-identity screen (it set identity_type to Evonode/Masternode directly, defeating the User-only restriction). - QA-004: the evonode 'Claim token rewards' CTA pushes a real scoped ClaimTokensScreen when the node holds exactly one token, falling back to My Tokens when the target is ambiguous — no more bare SetMainScreen. - QA-008: refresh the open detail view after its own backend task, not just the card list. - Diziet-F3: render the missing-voter 'Add voting key' CTA above, outside the collapsed DPNS section, so it is visible without expanding. - QA-009: surface a MessageBanner when node removal fails instead of a silent tracing::warn. - SEC-001: log the testnet-fixture parse error by position only, never its Display text (which echoes a private key). - SEC-002: parse fixture key fields as Secret (redacted/zeroized). - Strip stale PROJ-003/PROJ-004 markers and all ephemeral QA-xxx review IDs from source comments (kept only in commit messages). - TODOs for the deferred mixed-protection-tier CTA and the is_valid_pro_tx_hash/decode_identity_id duplication. - Offline tests: TC-FR8-07 (protection_tier reflects a Tier-2 seal) and TC-US4-05 (Remove deletes the associated voter identity). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(dashpay): close FR-6 boundary bypass via DashPay identity selectors (SEC-005) DashPay screens (send_payment, contacts_list, profile_screen, contact_requests, qr_scanner, qr_code_generator, add_contact_screen, profile_search) built their IdentitySelector and constructor seed from the unfiltered load_local_qualified_identities() chained with .syncing_global(...). IdentitySelector::sync_to_global() writes the picked id straight to AppContext::selected_identity_id — a separate path from B1's resolve_selected_identity()/restore filters — so a user could select a masternode/evonode as the app-global operate-as identity from inside DashPay, bypassing the FR-6/R1 boundary B1 established. DashPay operates on User identities only, so every identity list in these screens is sourced from load_local_user_identities() (the same swap B1 made for the global-nav switcher and Identity Hub). This filters the masternode out of both the selector write-path and the constructor seed. Adds a kittest (masternode_never_selectable_in_dashpay_screens) exercising the FR-6 boundary through five DashPay screens — the existing FR-6 kittest only covered Identities/Identity Hub, which is how this slipped through. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(masternodes): Marvin punch-list — in-flight guard + execution tests - QA-012: gate re-submission while a node-load is in flight. Add a load_in_flight flag on MasternodesScreen, set on Submit dispatch and cleared on the task result or a new display_task_error override; the '+ Load' toolbar button and empty-state CTA show a spinner + disabled 'Loading…' while set, so a rapid double-submit of a brand-new ProTxHash cannot race two loads past the pre-fetch existence check. - Extend masternode_never_selectable_in_dashpay_screens to QRScanner, QRCodeGenerator (both seed selected_identity in new()) and assert ProfileSearchScreen's User-filtered data source excludes the masternode — FR-6 coverage now spans all 8 DashPay screens. - Add manage_keys_button_opens_key_info_screen: clicks a per-key 'Voting key ›' button and asserts a KeyInfoScreen is pushed with its 'Key Information' heading (execution-level proof of the QA-007 fix). - Add refresh_from_network unit test: one RefreshIdentity per loaded node plus a trailing QueryDPNSContests, None when empty (QA-003). - Fix two doc-comment lines mangled by the earlier review-ID strip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(changelog): add Masternodes tab and global nav switcher (DOC-005) Covers the user-facing outcomes of the completed Masternodes feature: the new Expert-Mode-gated tab (card list, detail view, load-time key encryption, inline DPNS voting, credit actions, Evonode token-reward claiming) replacing the old generic load path for masternode/evonode identities, the resulting Identity Hub / Identities picker filter, and the wallet/identity switcher now present on every root screen instead of just the Identity Hub. * fix(masternodes): default GUI build broken — masternode_input feature-gated The whole model::masternode_input module was gated behind load form (ui/masternodes/load_form.rs) imports is_valid_pro_tx_hash from it unconditionally. So a plain 'cargo build --bin dash-evo-tool' (default features only — no mcp/cli, the documented quick-start build) failed with E0432 unresolved import. Every gate this feature ran used --all-features, which always pulls mcp+cli and masked it. The module can't be blanket-ungated: its parse/decode helpers return McpToolError (from the feature-gated mcp module). Fix ungates the module and the pure is_valid_pro_tx_hash validator (the only thing the GUI needs), and gates precisely the McpToolError-coupled items — KeyMode, parse_node_type, parse_key_mode, require_at_least_one_signing_key, decode_identity_id, their imports, and their tests — behind mcp/cli. The pure validator's tests move to an always-compiled module so they run in the default build too. Verified: 'cargo build --bin dash-evo-tool' (no flags) compiles; default- feature clippy clean; both default and --all-features test paths pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(masternodes): correct global-nav coverage claim (F-003) CHANGELOG and the components README claimed the global wallet/identity switcher was on "every screen". It ships Phase-A: rendered on Identities, DashPay, DPNS, Wallets, Identity Hub, and Masternodes, interactive only on the Hub and Masternodes (the other four render subdued, read-only pills), and absent from every other root screen (Contracts, Tokens, Tools, Network Chooser, Withdraws, ...). Names the actual screens and notes the rest as a tracked follow-up instead of implying full rollout. * fix(masternodes): Fable final round — Tier-2 merge seal, load gate, ProTxHash error F-001 (MUST-FIX): "Add voting key" on a password-protected (Tier-2) node no longer trips the insert's fail-closed guard. load_identity now verifies the node's object password UP FRONT (before the network fetch, mirroring add_key_to_identity's verify-before-broadcast order) and seals the merged plaintext keys Tier-2 via seal_merged_plaintext_keys just before the at-rest insert. Two regression tests: a scripted-prompt success path proving the new key flips InVault and reads back Protected, and a headless NullSecretPrompt path proving the merge fails closed with SecretPromptUnavailable before fetch. F-002: the list screen's load_in_flight gate is cleared only on the load's own LoadedIdentity result variant (not any routed result), with a refresh_on_arrival backstop so a tab switch mid-load can never strand "+ Load" at "Loading…". F-005: a malformed identity-id input now surfaces MalformedProTxHash for masternode/evonode loads (where the field IS a ProTxHash) and keeps IdentifierParsingError for User loads. Regression test added. F-006: masternodes/evonodes legitimately have no HD wallet, so the "saving identity without wallet" warning is gated to User identities; nodes log at debug instead. F-004: correct the MCP masternode_identity_load comment — Overwrite is a destructive full-replace of stored keys, not a merge/refresh; TODO for a future load-mode param. F-007: strip ephemeral review-ID prefixes (SEC-*, Diziet-*, Smythe) from masternode-scope source comments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): add load-form back link + remove object pill from breadcrumb Live-walkthrough fixes on real testnet data. Fix 1 — load form back link: the load form now renders the same `‹ All masternodes` back link as the detail view (wireframe C shows it on both), at the top of the form, returning to the card list. New kittest `load_form_back_link_returns_to_list` covers it; the existing `load_form_opens_from_cta_and_cancels` gets a taller headless window so the bottom Cancel button stays reachable now that the back row is present. Fix 2 — remove the masternode object/identity pill from the Masternodes breadcrumb. Masternode/evonode identities are never wallet-linked (wallet_info is always None — locked decision #4), so pairing a wallet pill with an object pill implied a wallet↔masternode relationship that does not exist. The breadcrumb now carries only segment-1 + the interactive wallet pill; node selection is driven entirely by card-click → detail and the back link. The Masternodes page switches to add_top_panel_with_global_nav (non-capturing), matching every other non-object page. The masternodes_page_nav_spec builder drops its items/selected params. This does NOT touch the FR-6 boundary, which is enforced structurally at the resolution layer (B1) independent of any pill. The release-blocking FR-6 boundary tests (TC-NAV-12 / TC-FR6-07) remain unchanged and green. The PageScopedObject / SelectPageObject / add_top_panel_with_global_nav_capturing machinery is retained as the documented, tested boundary pattern for future page-scoped-object features (the global_nav_switcher tests still exercise it); only the Masternodes page's use of it is removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): reject load when selected node type mismatches on-chain Loading a masternode/evonode by ProTxHash trusted the load-form Masternode/ Evonode toggle as ground truth with no cross-check. A regular masternode loaded with "Evonode" selected was silently accepted, mis-badged "Evonode", and shown the Evonode-only "Claim token rewards" action. The load task (authoritative layer) now cross-checks the selected type against the node's actual on-chain registration. A masternode's Platform identity id is its ProTxHash, so the node is looked up via Core RPC `protx info`; the `type` field ("Regular"/"Evo") is classified and compared. On a confirmed mismatch the load is rejected with a typed, actionable `TaskError::NodeTypeMismatch` naming both the selected and actual types. When the on-chain type cannot be determined (Core RPC unreachable — e.g. an SPV-only setup with no Core node) the load proceeds unverified, so this adds no regression for those users. Layering per CLAUDE.md: the pure classification (`classify_protx_node_type`) and rejection decision (`node_type_conflict`) live in `model/masternode_input` and are exhaustively unit-tested (the reported Evonode-on-regular case included); the backend task owns the network lookup and enforcement. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): surface a visible warning when node type is unverified Follow-up to the node-type cross-check: when Core RPC is unreachable (the common case for SPV-only users) the node type cannot be verified, and silently proceeding with an unverified badge reproduced the original UX bug downgraded from "wrong" to "unverified". The load task now distinguishes the two success outcomes via a new `BackendTaskSuccessResult::LoadedIdentityTypeUnverified` variant, and the Masternodes screen surfaces a visible warning banner (not just a log line) telling the user the badge reflects their selection and to reload later to confirm. The MCP masternode-load tool reports the same distinction via a new `node_type_verified` output field. Regression tests: the pure reject decision (`node_type_conflict`) and the `NodeTypeMismatch` user message cover the hard-reject path; a kittest drives the unverified-load result into the live screen and asserts the warning banner is surfaced to the UI, not merely logged. Confirmed with team-lead: hard-reject on a confirmed mismatch; a visible (not log-only) warning on the unverified path. The upstream platform-wallet SPV masternode-list passthrough (for verifying node type without Core RPC) is tracked as a separate follow-up against the platform repo. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * revert(masternodes): drop Fix #3 node-type validation entirely Reverts c5167787 and 755eee87. Product decision: trust the user's Masternode/Evonode toggle as-is, with no on-chain node-type verification. Rationale: the only non-type-gated actions (Withdraw/Top up/Transfer) are unaffected by a mis-toggle; the sole type-gated action ("Claim token rewards") degrades to a clean no-op/failed Platform state transition, not a fund-safety issue — so the toggle working as the user set it is correct behavior, not a defect. Dropping verification also removes the dependency on Core RPC (being deleted in the platform-wallet migration) and on fetching the operator identity (extra scope), leaving the load path simpler and migration-proof. Removes: TaskError::NodeTypeMismatch, the onchain_node_type Core RPC cross-check, the classify_protx_node_type/node_type_conflict model helpers, the LoadedIdentityTypeUnverified result variant + UI warning banner, the MCP node_type_verified output field, and all associated tests. Fixes #1 (load-form back link) and #2 (breadcrumb pill removal) in 5db23f72 are untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): adapt retargeted tab to platform-wallet rewrite APIs Rebasing the Masternodes tab onto the platform-wallet backend rewrite (PR #860) surfaced three call sites where the rewrite reshaped an API the masternode-tab code depended on: - `AppContext::identity_kv()` was renamed to `det_kv()`; the load-path existence check (`get_local_qualified_identity`) now calls the new name. - `ContestState::state_is_votable()` was dead-code-removed by the rewrite, but `ContestedName::is_open_for_voter` (Masternodes card DPNS status) relies on it — restored as a live, un-gated method. - The rewrite dropped the `identity-hub` Cargo feature and renders the Identity Hub nav entry unconditionally; the left-panel builder no longer gates that entry behind the removed `#[cfg(feature = "identity-hub")]`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(withdraw): screen-level kittest coverage for default_withdrawal_key fix WithdrawalScreen::new() pre-selecting via default_withdrawal_key() was only unit-tested at the QualifiedIdentity model layer. Add tests/kittest/withdraw_screen.rs to verify the fix at the actual screen layer: ghost-key identities (on-chain-only TRANSFER key) render the no-keys empty state instead of a form, private-key-backed TRANSFER/OWNER keys are pre-selected AND rendered correctly in the key-selection ComboBox (via accesskit value, not label). Also locks in a genuine regression the fix newly exposes: WithdrawalScreen::new() leaks a raw "No key provided when getting selected wallet" error banner for any identity with no locally-signable withdrawal key. * fix(withdraw): skip wallet resolution when no signable key exists WithdrawalScreen::new() called get_selected_wallet with selected_key=None after default_withdrawal_key() correctly began returning None for identities with no locally-signable withdrawal key. With app_context=None that hits the "No key provided" String Err path, which .or_show_error() posted verbatim into a user-facing MessageBanner — violating the plain-language error policy. Guard the call on selected_key being Some, so the raw-string Err branch is structurally unreachable here instead of avoided by luck. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(masternodes): add user story for network-switch reset behavior Live QA of the Masternodes tab confirmed MasternodesScreen::reset_for_network_change() cleanly resets the List view and clears stale banners/form data on a network switch, with no existing story documenting this PR's fix. Add MN-010. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(withdraw): flip banner-leak test to a regression lock (2edbc18) WithdrawalScreen::new() now guards get_selected_wallet on selected_key being Some (2edbc18), so the raw "No key provided..." banner leak for ghost-key-only identities is fixed. Rename ghost_key_construction_leaks_raw_error_banner -> ghost_key_construction_does_not_leak_raw_error_banner and invert the assertion; also verify the no-keys empty state still renders correctly for the same identity, so the fix didn't trade the leak for a broken empty state. * fix(mcp): stop det-cli double-prefixing the HTTP bearer token rmcp 1.7's StreamableHttpClientTransportConfig::auth_header takes the raw token and lets reqwest's bearer_auth prepend "Bearer " itself. The det-cli client passed format!("Bearer {token}"), so the wire header became "Authorization: Bearer Bearer <token>". The server middleware strips one "Bearer " and compares the remaining "Bearer <token>" against the configured key — never equal — so headless HTTP mode returned 401 on every request. The auth path had no end-to-end coverage, which is why it shipped broken. Pass the raw token and add tests/mcp_http_auth.rs pinning the server's wire contract: raw token accepted, a double-"Bearer" prefix rejected, missing credentials rejected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): share Expert Mode flag app-wide; node-specific load error Two live-QA bugs on the Masternodes tab (PR #876). Bug 1 — Expert Mode didn't reveal the Masternodes nav without a restart. `developer_mode` was an independent per-network `AppContext` AtomicBool, kept in sync only by a best-effort loop in the Settings checkbox handler over the contexts that happened to exist at click time. AppState keeps one context per network (only the active one at startup; others created lazily on switch), so the left-nav `FeatureGate::DeveloperMode` gate could read a stale value on a different context than the toggle mutated — the nav entry stayed hidden until a restart re-read the persisted flag into the single fresh context. Promote the flag to one shared `Arc<AtomicBool>`, created once in `AppState::new` and injected into every `AppContext::new` (startup + `SwitchNetwork` via `developer_mode_handle()`), so all per-network contexts observe one flag. Drop the fragile sync loop; request a repaint on toggle since enabling Expert Mode disables animations (which stops continuous repaints). Bug 2 — loading a masternode by a valid-but-unregistered ProTxHash showed the generic "Identity not found — check the ID or name" copy, wrong for a ProTxHash load form. Add a dedicated `TaskError::MasternodeNotFound` variant and return it from the masternode/evonode load path instead of `IdentityNotFound`. Regression tests: `developer_mode_is_shared_across_network_contexts` (RED before the shared-flag fix), `masternode_not_found_message_is_node_specific`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
| ); | ||
| } | ||
| } | ||
| let _resp = resp.info_tooltip(tip); |
There was a problem hiding this comment.
do we need this line when _resp is not used ?
| //! cross into egui. The lone secret type on the wire is `SecretString`, the | ||
| //! `platform_wallet_storage` zeroizing wrapper the project already depends on. | ||
|
|
||
| use async_trait::async_trait; |
There was a problem hiding this comment.
are you sure it belongs to src/ui/components,? Components are designed as items that implement the Component trait. Maybe src/ui/tools?
| @@ -0,0 +1,181 @@ | |||
| //! Shared modal chrome for passphrase entry. | |||
There was a problem hiding this comment.
src/ui/components should implement the Component trait. Implement it if possible.
| @@ -0,0 +1,298 @@ | |||
| //! Per-screen cache of wallets' tracked asset locks. | |||
There was a problem hiding this comment.
are you sure it belongs to src/ui/components? Components are designed as items that implement the Component trait. Maybe src/ui/tools?
| **Persona:** Priya, Jordan | ||
|
|
||
| As a user with an imported private key, I want to send Dash from that single-key wallet so that I can move funds to another address. | ||
|
|
||
| - Send flow works the same as for HD wallets. | ||
| - Temporarily unavailable in this version (Decision #7): single-key send returns a clear, calm "not supported in this version — your data is preserved; use an HD recovery-phrase wallet" message. Single-key wallet data and its UTXOs are retained on disk and load correctly; only the spend action is gated. Re-enabled when single-key moves onto the upstream wallet runtime. |
There was a problem hiding this comment.
We need it implemented.
| @@ -441,6 +488,19 @@ As a user, I want to view all keys associated with my identity so that I can aud | |||
| - Lists all keys with type, purpose, and status. | |||
| - View individual key details. | |||
|
|
|||
| ### IDN-013: Password-protect an identity's signing keys (SEC-001) [Implemented] | |||
There was a problem hiding this comment.
this only applies to non-HD-wallet backed identities. HD-wallet backend identities have per-wallet (seed) password protection capabilities, not identity level.
| @@ -481,6 +541,26 @@ As a user, I want to top up identity credits from a Platform address so that I c | |||
| - Available as funding method in top-up screen. | |||
| - Uses Platform address credits directly. | |||
|
|
|||
| ### IDN-014: Fund identity directly from scanned external payment [Removed — upstream-only funding] | |||
There was a problem hiding this comment.
We need to restore this feature
|
⛔ Blockers found — Sonnet deferred (commit 17f00d6) |
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Source: reviewers: claude-general (opus), codex-general (gpt-5.6-sol, failed rc=1); verifier: opus; specialists: none
Verified both agent findings against the worktree at 4343d03. The PR is a large, well-engineered wallet-backend rewrite; no blocking correctness or security defects were confirmed. Both surviving findings are non-blocking test-coverage observations: a sum-based header-vs-tab reconciliation whose invariant test cannot exercise the immature/locked-funds divergence case (verified at snapshot.rs:506/510-511/1217), and a self-referential wire-layout test that round-trips the current struct rather than pinning a golden pre-change blob (verified at settings.rs:516-546). No prior-finding reconciliation context or CodeRabbit inline findings were supplied, so nothing is carried forward beyond the two current-delta findings.
🟡 1 suggestion(s) | 💬 1 nitpick(s)
2 finding(s) are included in the review details below; GitHub refused the PR diff API because this PR exceeds 300 changed files.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/ui/wallets/wallets_screen/mod.rs`:
- [SUGGESTION] src/ui/wallets/wallets_screen/mod.rs:1225-1231: Header-vs-tab reconciliation is sum-based; untested when balance.total > sum(address_balances)
The PR claims "visible tabs always sum to the header total, in every mode." But the header total is `snapshot.balance.total` = upstream `core_balance.total()` (src/wallet_backend/snapshot.rs:506), which the PR's own doc (snapshot.rs:58-60) states "counts immature coinbase and locked (CoinJoin) funds" that coin selection cannot touch. The per-account tabs derive entirely from `address_balances`, summed only from `state.utxos()` (snapshot.rs:510-511), and the reconciling System/Other tab computes its value by summing categorized system summaries via `hidden_category_balance` (mod.rs:1225-1231) rather than as a residual `total - visible`. So header == Σtabs holds only if `Σ state.utxos() == core_balance.total()`. If upstream `utxos()` ever omits immature coinbase / locked CoinJoin outputs that `total()` still counts, the visible tabs sum to less than the header — reintroducing the divergence class this PR set out to close. The invariant test `header_total_reconciles_with_core_tab_breakdown_through_real_accessors` (snapshot.rs:1198) fixes `total = address_balances.values().sum()` (snapshot.rs:1217), so it structurally cannot catch this. Display-only (no funds at risk) and the immature/locked state is rare on user wallets, hence suggestion severity. A regression test that publishes a snapshot with `balance.total > Σaddress_balances` and asserts the reconciling tab still closes the gap would either confirm the claim or expose the hole.
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Source: reviewers: opus (ok), gpt-5.6-sol (failed: revoked refresh token); verifier: opus; specialists: none
The 4343d03..ee1ce81 delta is a single .gitignore line (ignore local Codex config); no source code changed. Both prior findings reference code that is byte-for-byte identical at this head and remain STILL VALID non-blocking test-coverage observations. No new latest-delta findings. Carried-forward: prior-1 (header-vs-tab reconciliation untested when balance.total > sum(address_balances)) and prior-2 (self-referential reserved-byte wire-layout test).
🟡 1 suggestion(s) | 💬 1 nitpick(s)
2 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/ui/wallets/wallets_screen/mod.rs`:
- [SUGGESTION] src/ui/wallets/wallets_screen/mod.rs:1225-1231: Header-vs-tab reconciliation is sum-based; untested when balance.total > sum(address_balances)
Carried forward from the prior review at 4343d034 (STILL VALID — code unchanged in this delta, which touches only .gitignore). The header total is `snapshot.balance.total` = upstream `core_balance.total()` (src/wallet_backend/snapshot.rs:502-506), which counts immature coinbase and locked (CoinJoin) funds that coin selection cannot touch. The per-account tabs derive entirely from `address_balances`, summed only from `state.utxos()` (snapshot.rs:508-518), and the reconciling System/Other tab computes its value by summing categorized system summaries via `hidden_category_balance` (mod.rs:1225-1231) rather than as a residual `total - visible`. So header == Σtabs holds only if `Σ state.utxos() == core_balance.total()`. If upstream `utxos()` ever omits immature coinbase / locked CoinJoin outputs that `total()` still counts, the visible tabs sum to less than the header, reintroducing the divergence this PR set out to close. The invariant test fixes `total = address_balances.values().sum()`, so it structurally cannot catch this. Display-only (no funds at risk) and the immature/locked state is rare, hence suggestion severity. A regression test that publishes a snapshot with `balance.total > Σaddress_balances` and asserts the reconciling tab still closes the gap would either confirm the claim or expose the hole.
Introduces docs/gui-testing/ as a reusable-across-sessions home for guidance on driving the real compiled app through a real display -- a tier not covered by kittest (no display) or backend-e2e (no UI). Deliberately not date-grouped under docs/ai-design, unlike point-in-time design docs, since this is standing practice agents should discover on every future GUI-verification task, not a historical record. Covers: when to use this tier vs kittest/backend-e2e, safety rules (isolated data dir, never touch a running instance, credential handling, fund-movement caps, preferring consensus-forced destinations), and a scenario file template. CLAUDE.md now points here from both the Testing and Documentation sections so agents find it before running a live GUI test. Co-authored-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Co-authored-by: Claude Opus 4.5 <noreply@anthropic.com>
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Source: orchestrator: openai/gpt-5.6-sol (orchestration-only); reviewers: codex-general=gpt-5.6-sol (ok), sonnet5-general=claude-sonnet-5 (ok); verifier: sonnet5=claude-sonnet-5; specialists: none
Latest delta (ee1ce81..cdd5a70): the new GUI-testing docs have two portability gaps — the reusable scenario hardcodes /data/target/... and DISPLAY=:99, while the guide delegates its launch procedure to an unavailable user-global skill.
Carried-forward prior findings: prior-2 (the self-referential settings wire-layout test) remains STILL VALID as a nitpick. Prior-1 (header/tab reconciliation) is OUTDATED: the pinned upstream key-wallet source derives utxos() and balance().total() from the identical complete funding-account UTXO set, including CoinJoin.
Additional cumulative finding: an SPV initialization failure is swallowed after claiming the one-shot start latch; the resulting Error state exposes only a Disconnect action that the stop guard rejects, so chain sync cannot recover without restarting the process.
🔴 1 blocking | 🟡 2 suggestion(s) | 💬 1 nitpick(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/context/wallet_lifecycle/spv.rs`:
- [BLOCKING] src/context/wallet_lifecycle/spv.rs:141-169: SPV start failure poisons the start latch with no recovery path except a process restart
Verified end-to-end against the current worktree. `WalletBackend::start()` (`src/wallet_backend/mod.rs:1064-1081`) claims the one-shot `start_latch` via `try_begin()` *before* the fallible `spv.start(config).await`, which genuinely can fail (mapped to `TaskError::WalletBackend` at mod.rs:1078-1080) — the doc comment at spv.rs:150-153 claiming 'the current impl is infallible' is itself inaccurate. The latch is reset only inside `stop_in_place` (mod.rs:1250). When `start()` fails, `ensure_wallet_backend_and_start_spv` (spv.rs:141-158) calls `mark_spv_error` (flips the indicator to `SpvStatus::Error`) but then unconditionally returns `Ok(())` — the real error never reaches its own `Result` return value. `ConnectionStatus::refresh_state()` (connection_status.rs:435-436) maps `SpvStatus::Error` to `OverallConnectionState::Error`, which is `!= Disconnected`, so `network_chooser_screen.rs:291` renders the **Disconnect** button (not Connect). Clicking it dispatches `AppAction::StopSpv`, which calls `begin_spv_stop()` (connection_status.rs:235-251) — but that only claims a stop from `Starting`/`Syncing`/`Running`, and silently returns `false` (no-op) when status is `Error`. So `stop_in_place()` — the only code path that resets the latch — never runs. A retry via Connect then hits the still-latched `try_begin() == false` and returns `Ok(())` immediately without actually attempting `spv.start()` again, while the indicator stays stuck on Error. A transient network/storage failure at SPV init time therefore permanently strands chain sync until the app is killed and restarted. This is new lifecycle code introduced by this PR's platform-wallet rewrite (StartLatch, ensure_wallet_backend_and_start_spv), squarely in scope.
In `docs/gui-testing/scenarios/TEMPLATE.md`:
- [SUGGESTION] docs/gui-testing/scenarios/TEMPLATE.md:27: Scenario template hardcodes a non-portable binary path and display
Confirmed by reading the file: the launch line targets `/data/target/debug/dash-evo-tool`, a path no documented build step in this repo produces (the standard `cargo build` output is `${CARGO_TARGET_DIR:-target}/debug/dash-evo-tool` relative to the workspace). It also hardcodes `DISPLAY=:99`, overriding whatever display the runner already set up per the preceding GUI-testing guide. Copying this template as-is on a normal contributor machine fails with 'binary not found' or targets a nonexistent display server.
In `docs/gui-testing/README.md`:
- [SUGGESTION] docs/gui-testing/README.md:28-30: GUI-testing guide's launch procedure depends on a skill that isn't in the repo
Confirmed: `~/.claude/skills/desktop-gui/SKILL.md` does not exist in this environment (`ls ~/.claude/skills/` returns 'No such file or directory'), it is not vendored anywhere in the repository, and it is not among the tool's registered skills. Since `CLAUDE.md` now points contributors at this guide before doing live GUI testing, a fresh contributor, CI runner, or remote agent reaches 'How to run a scenario' and has no way to perform the documented setup — the entire mechanical launch recipe (X display, accessibility tree, logging gotchas) lives only on one developer's home directory.
| pub async fn ensure_wallet_backend_and_start_spv( | ||
| self: &Arc<Self>, | ||
| sender: crate::utils::egui_mpsc::SenderAsync<crate::app::TaskResult>, | ||
| ) -> Result<(), TaskError> { | ||
| if let Err(e) = self.ensure_wallet_backend(sender).await { | ||
| self.mark_spv_error(&e); | ||
| return Err(e); | ||
| } | ||
| let backend = self.wallet_backend()?; | ||
| // Forward-compat: `start()`'s signature is fallible though the current | ||
| // impl is infallible. The reachable start-time failure today is the | ||
| // wiring step above, surfaced via `mark_spv_error`; this branch keeps | ||
| // the start step covered should `start()` begin to fail. | ||
| if let Err(e) = backend.start().await { | ||
| self.mark_spv_error(&e); | ||
| } | ||
| Ok(()) | ||
| } | ||
|
|
||
| /// Flip the SPV connection indicator to [`SpvStatus::Error`] and record the | ||
| /// failure detail. Safe in every context (GUI and headless) — it touches | ||
| /// only `ConnectionStatus` atomics, never an egui context. | ||
| fn mark_spv_error(&self, error: &TaskError) { | ||
| tracing::error!(error = %error, "Failed to start chain sync"); | ||
| self.connection_status | ||
| .set_spv_last_error(Some(format!("{error}"))); | ||
| self.connection_status.set_spv_status(SpvStatus::Error); | ||
| self.connection_status.refresh_state(); | ||
| } |
There was a problem hiding this comment.
🔴 Blocking: SPV start failure poisons the start latch with no recovery path except a process restart
Verified end-to-end against the current worktree. WalletBackend::start() (src/wallet_backend/mod.rs:1064-1081) claims the one-shot start_latch via try_begin() before the fallible spv.start(config).await, which genuinely can fail (mapped to TaskError::WalletBackend at mod.rs:1078-1080) — the doc comment at spv.rs:150-153 claiming 'the current impl is infallible' is itself inaccurate. The latch is reset only inside stop_in_place (mod.rs:1250). When start() fails, ensure_wallet_backend_and_start_spv (spv.rs:141-158) calls mark_spv_error (flips the indicator to SpvStatus::Error) but then unconditionally returns Ok(()) — the real error never reaches its own Result return value. ConnectionStatus::refresh_state() (connection_status.rs:435-436) maps SpvStatus::Error to OverallConnectionState::Error, which is != Disconnected, so network_chooser_screen.rs:291 renders the Disconnect button (not Connect). Clicking it dispatches AppAction::StopSpv, which calls begin_spv_stop() (connection_status.rs:235-251) — but that only claims a stop from Starting/Syncing/Running, and silently returns false (no-op) when status is Error. So stop_in_place() — the only code path that resets the latch — never runs. A retry via Connect then hits the still-latched try_begin() == false and returns Ok(()) immediately without actually attempting spv.start() again, while the indicator stays stuck on Error. A transient network/storage failure at SPV init time therefore permanently strands chain sync until the app is killed and restarted. This is new lifecycle code introduced by this PR's platform-wallet rewrite (StartLatch, ensure_wallet_backend_and_start_spv), squarely in scope.
source: ['codex-general']
There was a problem hiding this comment.
Resolved in this update — SPV start failure poisons the start latch with no recovery path except a process restart no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| # Confirm no conflicting instance is already using this display/data dir | ||
| pgrep -af dash-evo-tool | ||
|
|
||
| DISPLAY=:99 DASH_EVO_DATA_DIR="$DATADIR" nohup /data/target/debug/dash-evo-tool >/tmp/<scenario-slug>.log 2>&1 & |
There was a problem hiding this comment.
🟡 Suggestion: Scenario template hardcodes a non-portable binary path and display
Confirmed by reading the file: the launch line targets /data/target/debug/dash-evo-tool, a path no documented build step in this repo produces (the standard cargo build output is ${CARGO_TARGET_DIR:-target}/debug/dash-evo-tool relative to the workspace). It also hardcodes DISPLAY=:99, overriding whatever display the runner already set up per the preceding GUI-testing guide. Copying this template as-is on a normal contributor machine fails with 'binary not found' or targets a nonexistent display server.
| DISPLAY=:99 DASH_EVO_DATA_DIR="$DATADIR" nohup /data/target/debug/dash-evo-tool >/tmp/<scenario-slug>.log 2>&1 & | |
| BIN="${CARGO_TARGET_DIR:-target}/debug/dash-evo-tool" | |
| DASH_EVO_DATA_DIR="$DATADIR" nohup "$BIN" >"/tmp/<scenario-slug>.log" 2>&1 & |
source: ['codex-general']
There was a problem hiding this comment.
Resolved in this update — Scenario template hardcodes a non-portable binary path and display no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| The mechanical launch recipe (X display setup, accessibility tree, gotchas | ||
| like stderr redirection) lives in the global `desktop-gui` Claude Code skill | ||
| (`~/.claude/skills/desktop-gui/SKILL.md`) — read that first, it's not repeated |
There was a problem hiding this comment.
🟡 Suggestion: GUI-testing guide's launch procedure depends on a skill that isn't in the repo
Confirmed: ~/.claude/skills/desktop-gui/SKILL.md does not exist in this environment (ls ~/.claude/skills/ returns 'No such file or directory'), it is not vendored anywhere in the repository, and it is not among the tool's registered skills. Since CLAUDE.md now points contributors at this guide before doing live GUI testing, a fresh contributor, CI runner, or remote agent reaches 'How to run a scenario' and has no way to perform the documented setup — the entire mechanical launch recipe (X display, accessibility tree, logging gotchas) lives only on one developer's home directory.
source: ['codex-general']
There was a problem hiding this comment.
Resolved in this update — GUI-testing guide's launch procedure depends on a skill that isn't in the repo no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| fn reserved_core_backend_mode_byte_preserves_wire_layout() { | ||
| let wire = AppSettingsWire { | ||
| network: "testnet".to_string(), | ||
| root_screen_type: 3, | ||
| dash_qt_path: Some("/opt/dash-qt".to_string()), | ||
| overwrite_dash_conf: false, | ||
| disable_zmq: true, | ||
| theme_mode: "Dark".to_string(), | ||
| _reserved_core_backend_mode: 0, // legacy "RPC" value from an old blob | ||
| onboarding_completed: true, | ||
| show_evonode_tools: true, | ||
| user_mode: "Beginner".to_string(), | ||
| close_dash_qt_on_exit: false, | ||
| auto_start_spv: true, | ||
| }; | ||
| let encoded = | ||
| bincode::serde::encode_to_vec(&wire, bincode::config::standard()).expect("encode"); | ||
| let (decoded, _): (AppSettings, _) = | ||
| bincode::serde::decode_from_slice(&encoded, bincode::config::standard()) | ||
| .expect("decode"); | ||
| // Fields after the reserved byte must be read from the correct | ||
| // offset — a shifted layout would scramble these. | ||
| assert!(decoded.onboarding_completed); | ||
| assert!(decoded.show_evonode_tools); | ||
| assert_eq!(decoded.user_mode, UserMode::Beginner); | ||
| assert!(!decoded.close_dash_qt_on_exit); | ||
| assert!(decoded.auto_start_spv); | ||
| // Fields before it, for completeness. | ||
| assert_eq!(decoded.network, Network::Testnet); | ||
| assert!(matches!(decoded.theme_mode, ThemeMode::Dark)); | ||
| } |
There was a problem hiding this comment.
💬 Nitpick: Reserved-byte wire-layout test round-trips the current struct, not a real legacy blob
Carried forward from the prior review, code byte-identical at this head. reserved_core_backend_mode_byte_preserves_wire_layout constructs the current AppSettingsWire (with _reserved_core_backend_mode: u8 already in place, settings.rs:516-530) and round-trips it through the current decode path. That only proves the current struct is self-consistent — it doesn't prove a blob written by the pre-change struct (which had a live core_backend_mode field, likely enum-encoded rather than a raw u8) still decodes correctly under the new layout, which is the actual backward-compatibility claim in the doc comment. A fixed golden byte vector captured from the pre-change encoding — or a standalone legacy-shape struct — would be a genuine pin. Low impact: a decode miss on an old blob degrades gracefully to settings defaults, an already-accepted gap.
source: ['sonnet5-general', 'codex-general']
There was a problem hiding this comment.
Resolved in 0696d21 — Reserved-byte wire-layout test round-trips the current struct, not a real legacy blob no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
…lassification, and role-picker UI (#879) * fix(withdraw): pre-select only a locally-signable withdrawal key The Withdraw screen constructor pre-selected a key via the on-chain lookup `identity.get_first_public_key_matching(TRANSFER, ...)`, which is unfiltered by local private-key presence. On loaded masternode/evonode identities where only the Owner key was supplied, this picked a "ghost" Transfer key with no local private material, so the withdrawal failed at signing with a raw, unhelpful protocol error. - model: add `QualifiedIdentity::default_withdrawal_key()` — sourced from `available_withdrawal_keys()` (private-key-backed only), Transfer preferred with Owner fallback, `None` when nothing is signable. - ui: constructor now pre-selects via `default_withdrawal_key()`; the developer-mode on-chain escape hatch is preserved. When no usable key exists the existing empty-state guides the user to add one. - error: add `TaskError::NoWithdrawalSigningKey` (typed `#[source]`, plain-language actionable Display) mapping the SDK `DesiredKeyWithTypePurposeSecurityLevelMissing` protocol error as a defense-in-depth backstop instead of leaking a raw string. - tests: 4 model cases (ghost key rejected, private-backed selected, owner fallback, transfer preferred) + 2 error-mapping/Display cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(claude): correct secret-storage note on identity-key encryption Identity keys (imported/loaded, including masternode voting/owner/payout) are no longer categorically in the deferred keyless tier: they enter unprotected at load time but can be sealed to Tier-2 per-identity via IdentityTask::ProtectIdentityKeys (Key Info screen "Add password protection"). Clarify that the keyless residual is only no-password secrets and keys the user has not opted to protect. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(masternodes): add page-nav model with two-scope selection (A1) Introduce ui/state/global_nav.rs: PageNavSpec (page-aware segment-1 + per-page pill composition) and IdentityPillScope (AppGlobalUser vs PageScopedObject). The PageScopedObject variant carries its own selection and never writes AppContext::selected_identity_id — the structural FR-6 boundary the global switcher (A2) and the Masternodes page (B7) build on. Pure state, renders nothing (module-placement discriminator -> ui/state). Satisfies TC-NAV-13, TC-NAV-14, TC-NAV-15; foundation for TC-NAV-12, TC-FR6-07. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): generalize breadcrumb into page-aware global switcher (A2) Add ui/components/global_nav_switcher.rs: a page-aware switcher driven by PageNavSpec, rendering segment-1 (page label + link) plus composable wallet / identity-or-object pills. GlobalNavEffect generalizes the hub's BreadcrumbEffect and adds SelectPageObject for the page-scoped pill — kept distinct from SelectIdentity so a page-scoped selection never writes the app-global identity (FR-6 boundary at the effect level). Reuses BreadcrumbPill / IdentityPill / BreadcrumbPillMode verbatim (no new pill widget). breadcrumb_switcher.rs becomes a thin hub-facing shim that builds the hub spec, delegates to the generalized render, and maps the effect back (self-nav to the hub root -> OpenPicker), keeping hub behavior unchanged — verified by the existing identity_hub_switcher kittests. Satisfies TC-NAV-01, TC-NAV-02, TC-NAV-03, TC-NAV-13, TC-NAV-16. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): render global switcher on root screens + shared applier (A3) Add the shell glue in top_panel.rs: apply_global_nav_effect (the shared successor to the hub's apply_breadcrumb_effect — silent app-scoped wallet/identity writes, no forced navigation) and add_top_panel_with_global_nav (one-call render-plus-apply), with subdued_everyday_spec / subdued_wallet_only_spec Phase-A rollout helpers. Wire the switcher onto four non-Hub root screens with Subdued (unwired) specs + TODO markers: Identities, DPNS, DashPay (everyday: wallet + identity pills) and Wallets (wallet-only composition, TC-NAV-15). The Hub keeps its existing interactive pills via the breadcrumb shim (regression — full kittest suite green). Document (comment + test, PROJ-010) that set_selected_hd_wallet reconciles the app-global identity as a side effect on non-Hub pages, and that combined with B1's resolution-layer filter it must never reconcile onto an MN/Evonode. Deferred (documented): tokens/tools screens carry in-header sub-navigation that needs the FR-GLOBAL-NAV-5 content-panel back-row migration before their plain breadcrumb can be swapped for the global switcher — a follow-up, not a mechanical swap. Satisfies TC-NAV-06, TC-NAV-15, TC-NAV-16; enables TC-NAV-12/17. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): load-time key encryption plumbing (B0) FR-8: add IdentityInputToLoad.encryption_password: Option<Secret>. When Some, load_identity validates the password up front (fast fail) then, after insert migrates the keyless keys into the vault, seals them Tier-2 through the existing per-identity protect envelope (protect_identity_keys → put_secret_protected via the secret_seam chokepoint) — no new crypto, no second persistence path. When None, the keyless Tier-1 path is unchanged. Relocate validate_protection_password from protect_identity_keys.rs into model/identity_key_protection.rs (PROJ-006, DET validation-placement rule); the seal path and load path both call the model validator. MCP masternode_identity_load passes encryption_password: None (PROJ-007 — GUI-only this iteration, requirements §2.3) with a TODO for headless password parity. Add typed TaskError variants DuplicateProTxHash { identity_id } and MalformedProTxHash { input } for later duplicate/malformed rejection (B1/B4), avoiding string parsing. Tests: model validator (relocated); an offline-wired-AppContext test proving a load-time password seals a masternode's voting (V-target), owner and identity (M-target) keys Tier-2 and round-trips under the password — the exact call load_identity makes (TC-FR8-01/02/10). Load-form UI is B4; end-to-end load routing is covered by the network backend-e2e suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(masternodes): drop ephemeral review ID from A3 reconciliation comment Self-review: replace a transient review-finding ID in the apply_global_nav_effect reconciliation note with the durable FR reference. Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): FR-6 resolution-layer boundary + masternode accessor (B1) FR-6 (R1, release-blocking): keep masternode/evonode identities out of every everyday-user surface by filtering at the resolution layer, not the display call sites. - resolve_selected_identity(): candidate set filtered to IdentityType::User before resolving, so neither keep-if-loaded nor the first-loaded fallback can ever resolve a masternode — even when a masternode is the only/first loaded identity (TC-NAV-12b). - set_selected_hd_wallet(): the wallet-switch identity reconciliation resolves only over the wallet's User identities. - restore_selected_identity_from_kv(): one-time sanitization — a masternode persisted as selected_identity_id in a prior session is cleared on load; a User selection is kept (TC-NAV-12c). In-memory only (non-destructive). - Display sources switched to the established User-only accessor load_local_user_identities(): the global switcher's identity pill + dropdown and the Identity Hub landing/picker now list User identities only, so the wallet-less "no wallet on this device" group can no longer surface an MN/Evonode (TC-NAV-17). Masternodes stay in the legacy Identities table (unfiltered accessor untouched — locked decision #2). New context accessor load_local_masternode_identities() (hydrated MN/Evonode) — the Masternodes-page card list + page-scoped pill source (B3/B7). Tests: offline-wired-AppContext unit tests (accessors, resolution filter incl. lone-masternode fallback, stale-MN sanitization) + a Hub kittest asserting a seeded Masternode+Evonode never appear on the hub while remaining in the masternode accessor. TC-FR6-01…06, TC-NAV-12b/12c, TC-NAV-17. Deferred to consuming tasks (documented): FR-7 refresh (TC-FR7-02/03) composes existing RefreshIdentity + contested-names refresh at the card Refresh button (B3); the per-node open-contest card read accessor lands in B3 where the card consumes it and it is testable against the rendered status line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): register Expert-gated Masternodes root tab (B2) Add RootScreenType::RootScreenMasternodes (stable int 28, round-trip test), ScreenType::Masternodes, Screen::MasternodesScreen, create_screen and all ScreenLike dispatch arms; register the always-present root screen in app.rs (gated at runtime by Expert Mode, not a Cargo feature, so the screen exists to switch into when the gate is on). Nav: a "Masternodes" left-panel entry gated FeatureGate::DeveloperMode, positioned directly below the identity cluster (locked decision #3), independent of the identity-hub feature. Distinct glyph voting.png (TODO: dedicated node/server icon). The existing per-entry gate skip hides the nav item and route when Expert Mode is off. Live de-gating (§10.11): active_root_screen_mut falls the active tab back to Identities (always registered) if Expert Mode flips off while Masternodes is selected, so the gated screen is never shown without its gate. MasternodesScreen is a scaffold (global-nav header + left rail + island placeholder); the empty state + card grid land in B3, the page-scoped masternode pill in B7. Network-switch already calls change_context on main_screens; the sub-screen reset (§10.10) applies once B4/B5 push sub-screens (noted for B8). Tests: RootScreenType round-trip (int 28 stable); kittest — nav absent Expert-off / present Expert-on, and de-gating falls back to Identities. TC-FR1-01…07, TC-EDGE-05/06. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): empty state + card grid + card body (B3) Render the Masternodes root screen content on top of the B2 scaffold: - Empty state (FR-2): canonical §7 copy — heading, body, "Load a masternode" primary CTA, and the ProTxHash reassurance line. - Card grid (FR-3): responsive minmax(260,1fr) grid reusing the identity picker's visual language via a new `MasternodeCard` (monogram + `draw_type_badge`, both now `pub(crate)`). Body adds masternode rows the picker lacks: voter readiness, compact `V O P` key status (glyph, not colour-only — NFR-6), DPNS status line, and the IdentityStatus dot+label. - DPNS status precedence (§10.1): open-contest count first, then a pending scheduled vote, then "No open contests", via a display-layer `AppContext::masternode_contest_summary` read (no new backend concept). - Key presence: `QualifiedIdentity::masternode_key_presence` maps Voting/Owner/Payout to voter-identity / OWNER / TRANSFER keys. - Top-right Refresh toolbar button (FR-7) reloads the cached node list. - Whole card is a single accessible click target (`WidgetInfo::labeled`, NFR-6); selection/load intents are captured for B4/B5a/B7 wiring. Traceability: TC-FR2-01…07, TC-FR3-01…15, TC-FR7-01, TC-NFR6-01/03. Unit tests cover heading/sub-line, DPNS precedence, key tokens, badge, and the 8 V/O/P combinations; kittest covers empty-state copy and the grid. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): dedicated load form + ProTxHash validator (B4) Add the MN/Evonode-only load flow (FR-4), carved out of the generic add-existing-identity path: - `ui/masternodes/load_form.rs`: ProTxHash (required), Masternode/Evonode segmented toggle (default Masternode, no User option), optional alias, V/O/P key inputs (reused hold-to-reveal `PasswordInput`), optional at-load encryption password (drives B0's seal), always-visible Warning-tone key-storage note, and a Load button gated on a non-empty ProTxHash with the §7 disabled tooltip. Switching node type clears all fields (§10.6). No auto-derive affordance — masternode keys are never wallet-derived (US-6 retired, §Locked-#4). - `model/masternode_input.rs`: `is_valid_pro_tx_hash` shape validator (hex or Base58) for inline on-blur validation; the backend load task remains the authoritative existence/duplicate check. - Masternodes screen gains a List/Load view enum; the empty-state CTA and a `+ Load` toolbar button open the form; submit dispatches `IdentityTask::LoadIdentity`; cancel/submit return to the list with a fresh form on reopen. - `add_existing_identity_screen`: remove Masternode/Evonode from the Advanced-Options Identity-Type dropdown (User-only remains) — no competing entry point (§10.2 / TC-FR4-22, FR-6). Traceability: TC-FR4-01…22 (render/logic; live-network accept/duplicate/ error-banner paths land in B8), TC-EDGE-01/02. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): detail view — header, actions, keys, remove (B5a) Add the node detail view (FR-5), reusing existing screens rather than reimplementing: - Fixed section order Header → Actions → Keys → DPNS → Remove (TC-FR5-01, the human-requested Actions-above-Keys correction), pinned by a unit test. - Header: conditional alias, shortened ProTxHash + copy-full-value, type badge (shared `draw_type_badge`), IdentityStatus dot + label. - Actions row (FR-9): Withdraw / Top up / Transfer push the existing WithdrawalScreen / TopUpIdentity / TransferScreen scoped to the node's QualifiedIdentity (both node types). Evonode-only `Claim token rewards ›` cross-link (FR-11), absent for a plain masternode. - Keys section (FR-10): V/O/P presence, copyable voter-identity id, at-rest protection tier (vault-scheme probe), Add-protection offered only Tier-1, `Manage keys ›` into the existing key screen. - DPNS section: collapsible, open-contest count in the header (voting table lands in B5b). - Remove: danger ConfirmationDialog; deletes the node and its voter identity. - `‹ All masternodes` back row + detail Refresh (FR-7/TC-FR7-04); card click opens the detail via a List/Load/Detail view enum. Deviations (documented): the Evonode claim cross-link routes to the Tokens area — precise ClaimTokensScreen token-scoping is deferred to B8 where the evonode reward-token context is resolvable. Add-protection routes into the reused key screen (which hosts the password-entry seal flow) rather than duplicating the form. Traceability: TC-FR5-01…05/07, TC-FR7-04, TC-FR9-01/02, TC-FR11-01/02. Live-network credit/claim routing and TC-FR8-07 land in B8. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): Testnet Fill-Random on the load form (B6) Add the FR-12 dev convenience to the masternode load form: - New `testnet_fixture` module owns the `.testnet_nodes.yml` structs + loader. The loader returns None for BOTH a missing and a malformed file — a malformed file is logged at debug and treated as absent (TC-FR12-04, a deliberate divergence from the legacy screen which banners the parse error). - Fill-Random button + hint render only when Expert Mode is on, the network is Testnet, and the fixture is present — never shown-but-disabled (TC-FR12-01…06). The `dev_mode` gate is a defense-in-depth re-check at the call site (TC-FR12-09 decision: added on future-proofing grounds — a plaintext-key dev tool stays inside the Expert-Mode envelope). - Button label follows the node-type toggle (TC-FR12-01/02). - Autofill pulls from the correct list per type (TC-FR12-07/08): Masternode → `masternodes` (Voting + Owner only; the fixture has no payout key, PROJ-003), Evonode → `hp_masternodes` (all three keys). The node-type toggle still clears autofilled fields (§10.6). - The fixture loads once when the form opens (Testnet only), not per frame. Traceability: TC-FR12-01…09. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): inline DPNS voting + missing-voter prompt (B5b) Populate the detail view's collapsible DPNS section (FR-5): - Collapsed by default; header shows the open-contest count (`DPNS name contests to vote on (N)`, TC-DPNS-01/02). - Voter present + open contests: per-contest Abstain / Lock / Vote-for-candidate choices with the candidate list scoped to that contest's contestants; a `Cast votes` button dispatches the existing `ContestedResourceTask::VoteOnDPNSNames` backend inline (locked decision #1 — not a deep-link). TC-DPNS-03/04/05. - Voter present, zero open contests: exact §7 empty copy (TC-DPNS-08). - Missing voter identity: the actionable §7 message (never the raw NoVotingIdentity error) plus an `Add voting key` action that opens a scoped, in-place voter-key prompt with the node context pre-bound — distinct from FR-4's load form, no ProTxHash re-entry (TC-DPNS-09/10/11, §10.8). Save re-loads this node with just the voting key to update its voter identity. - Detail Refresh now re-reads both the contest summary and the open-contest list. Active/open contests only — scheduled/past history stays on the DPNS Scheduled Votes screen (§10.7). Traceability: TC-DPNS-01…11. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): page-scoped nav pill — FR-6 boundary in code (B7) Wire the Masternodes page into the global-nav switcher with a page-scoped masternode pill whose selection lives on the page and is NEVER written to `AppContext::selected_identity_id` — the structural FR-6 boundary in code, complementing B1's resolution-layer filter. - New `ui/state/masternodes_view.rs` builds the page's `PageNavSpec`: page-aware `Masternodes` segment-1, an interactive wallet pill (funds Top up — FR-9), and a page-scoped identity pill via `IdentityPillScope::PageScopedObject`. Empty → subdued `(no masternode yet)` placeholder; ≥1 node → interactive dropdown with `Choose a masternode` placeholder; the pill reflects the node in detail and resets to the placeholder on `‹ All masternodes` (§10.4). - New `top_panel::add_top_panel_with_global_nav_capturing` surfaces the `SelectPageObject` pick to the caller (applying all other effects as usual) without ever routing it into the app-global identity selection. - The Masternodes screen builds the spec each frame from its node list + current view and opens the picked node's detail — two-way with the card grid. TC-NAV-12 / TC-FR6-07 (release-blocking): a masternode selected on the page never becomes, or resolves as, the app-global identity — verified across Identities and the Identity Hub with no User identity loaded. Traceability: TC-NAV-01/03/04/05/07, TC-NAV-12, TC-FR5-06, TC-FR6-07. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(masternodes): cross-cutting integration coverage (B8) Add the Remove-flow integration kittest (TC-US4-01/02/06/07): the detail danger button opens a confirmation carrying the `Remove masternode` verb, and confirming deletes only the target node — its card disappears while other nodes survive (isolation). Also sets the confirmation's confirm verb to `Remove masternode` (§7 / TC-US4-02), the one small production touch the test surfaced. Deferred to the network/backend-e2e pass (out of kittest reach without live DAPI or heavy vault fixtures, and flagged as such in the plan): TC-FR8-07 (detail reflecting a load-time Tier-2-sealed node — needs the real password-at-load seal path), TC-US4-05 voter-row deletion assertion (needs a seeded voter-identity row), and the live-network credit/vote/claim dispatch paths behind FR-9/FR-11/DPNS Cast-votes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(user-stories): catalog the Masternodes tab, retire the legacy load story Adds MN-001..MN-009 (load, card grid, vote, remove, Hub filter, at-load encryption, credit actions, key management, evonode token-reward cross-link) and UX-003 (global wallet/identity switcher) per the completed Masternodes feature. Flips IDN-003 to superseded — its generic-screen masternode load path was removed when the dedicated tab shipped. * docs(masternodes): commit final design docs (DOC-002) Lands the human-accepted requirements, UX spec, test-case spec, and dev plan under docs/ai-design/, following the existing 2026-04-23-identity-hub-impl numbered-file convention. Resolves the ~100+ FR-/TC-/US-/§-ID references already scattered through the feature's code comments and tests, which pointed at an uncommitted /data/artifacts scratch copy. Internal cross-file references (requirements.md, ux-spec.md, etc.) are updated to the new numbered filenames. * docs(masternodes): trim oversized module docs, catalog global-nav switcher Shortens the four ui/masternodes/*.rs module doc comments to the internal-tier length cap (DOC-003) — they weren't published API, so the 5-10 line public-rustdoc relaxation didn't apply. Adds GlobalNavSwitcher and its top_panel entry point to ui/components/README.md's catalog (DOC-004), so the next screen needing a page-aware switcher finds it instead of reimplementing one. * fix(masternodes): guard identity load against silent overwrite (QA-005/006) Root-cause storage fix. insert_local_qualified_identity is INSERT OR REPLACE, so a load with no guard silently clobbers an already-stored identity and its keys. Thread an IdentityLoadMode through IdentityInputToLoad so each entry point declares intent: - RejectIfExists: the masternode load form rejects a duplicate ProTxHash with TaskError::DuplicateProTxHash before any network fetch (QA-006). - MergeIntoExisting: the scoped Add-voting-key prompt merges the new key into the stored identity, preserving Owner/Payout it did not resupply (QA-005), via merge_existing_keys_into. - Overwrite: legacy User re-load and headless flows unchanged (default). Adds get_local_qualified_identity accessor backing the existence check and merge read. Failing-first TDD: a unit test proving Owner/Payout keys survive a voting-key-only merge, and an offline test proving a duplicate ProTxHash is rejected and the first node is left untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): key routing, network-switch reset, live refresh QA-007: the detail Keys section pushed the static read-only KeysScreen. Render a per-key 'Manage keys' list and route the Add-protection CTA to KeyInfoScreen (interactive view/sign/seal per key), mirroring identities_screen. QA-001: MasternodesScreen had no change_context override, so a network switch left an open load form or cross-network detail view actionable. Add an explicit change_context arm that resets to the List view and reloads from the now-active network. QA-003: both Refresh buttons only re-read the local cache. Wire them to dispatch IdentityTask::RefreshIdentity (per loaded node on the list, the open node on detail) plus a QueryDPNSContests re-query, alongside the optimistic local re-read. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): SHOULD-FIX pass + QA-token cleanup + offline tests - QA-002: remove the legacy Fill-Random HPMN/Masternode bypass from the now User-only add-existing-identity screen (it set identity_type to Evonode/Masternode directly, defeating the User-only restriction). - QA-004: the evonode 'Claim token rewards' CTA pushes a real scoped ClaimTokensScreen when the node holds exactly one token, falling back to My Tokens when the target is ambiguous — no more bare SetMainScreen. - QA-008: refresh the open detail view after its own backend task, not just the card list. - Diziet-F3: render the missing-voter 'Add voting key' CTA above, outside the collapsed DPNS section, so it is visible without expanding. - QA-009: surface a MessageBanner when node removal fails instead of a silent tracing::warn. - SEC-001: log the testnet-fixture parse error by position only, never its Display text (which echoes a private key). - SEC-002: parse fixture key fields as Secret (redacted/zeroized). - Strip stale PROJ-003/PROJ-004 markers and all ephemeral QA-xxx review IDs from source comments (kept only in commit messages). - TODOs for the deferred mixed-protection-tier CTA and the is_valid_pro_tx_hash/decode_identity_id duplication. - Offline tests: TC-FR8-07 (protection_tier reflects a Tier-2 seal) and TC-US4-05 (Remove deletes the associated voter identity). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(dashpay): close FR-6 boundary bypass via DashPay identity selectors (SEC-005) DashPay screens (send_payment, contacts_list, profile_screen, contact_requests, qr_scanner, qr_code_generator, add_contact_screen, profile_search) built their IdentitySelector and constructor seed from the unfiltered load_local_qualified_identities() chained with .syncing_global(...). IdentitySelector::sync_to_global() writes the picked id straight to AppContext::selected_identity_id — a separate path from B1's resolve_selected_identity()/restore filters — so a user could select a masternode/evonode as the app-global operate-as identity from inside DashPay, bypassing the FR-6/R1 boundary B1 established. DashPay operates on User identities only, so every identity list in these screens is sourced from load_local_user_identities() (the same swap B1 made for the global-nav switcher and Identity Hub). This filters the masternode out of both the selector write-path and the constructor seed. Adds a kittest (masternode_never_selectable_in_dashpay_screens) exercising the FR-6 boundary through five DashPay screens — the existing FR-6 kittest only covered Identities/Identity Hub, which is how this slipped through. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(masternodes): Marvin punch-list — in-flight guard + execution tests - QA-012: gate re-submission while a node-load is in flight. Add a load_in_flight flag on MasternodesScreen, set on Submit dispatch and cleared on the task result or a new display_task_error override; the '+ Load' toolbar button and empty-state CTA show a spinner + disabled 'Loading…' while set, so a rapid double-submit of a brand-new ProTxHash cannot race two loads past the pre-fetch existence check. - Extend masternode_never_selectable_in_dashpay_screens to QRScanner, QRCodeGenerator (both seed selected_identity in new()) and assert ProfileSearchScreen's User-filtered data source excludes the masternode — FR-6 coverage now spans all 8 DashPay screens. - Add manage_keys_button_opens_key_info_screen: clicks a per-key 'Voting key ›' button and asserts a KeyInfoScreen is pushed with its 'Key Information' heading (execution-level proof of the QA-007 fix). - Add refresh_from_network unit test: one RefreshIdentity per loaded node plus a trailing QueryDPNSContests, None when empty (QA-003). - Fix two doc-comment lines mangled by the earlier review-ID strip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(changelog): add Masternodes tab and global nav switcher (DOC-005) Covers the user-facing outcomes of the completed Masternodes feature: the new Expert-Mode-gated tab (card list, detail view, load-time key encryption, inline DPNS voting, credit actions, Evonode token-reward claiming) replacing the old generic load path for masternode/evonode identities, the resulting Identity Hub / Identities picker filter, and the wallet/identity switcher now present on every root screen instead of just the Identity Hub. * fix(masternodes): default GUI build broken — masternode_input feature-gated The whole model::masternode_input module was gated behind load form (ui/masternodes/load_form.rs) imports is_valid_pro_tx_hash from it unconditionally. So a plain 'cargo build --bin dash-evo-tool' (default features only — no mcp/cli, the documented quick-start build) failed with E0432 unresolved import. Every gate this feature ran used --all-features, which always pulls mcp+cli and masked it. The module can't be blanket-ungated: its parse/decode helpers return McpToolError (from the feature-gated mcp module). Fix ungates the module and the pure is_valid_pro_tx_hash validator (the only thing the GUI needs), and gates precisely the McpToolError-coupled items — KeyMode, parse_node_type, parse_key_mode, require_at_least_one_signing_key, decode_identity_id, their imports, and their tests — behind mcp/cli. The pure validator's tests move to an always-compiled module so they run in the default build too. Verified: 'cargo build --bin dash-evo-tool' (no flags) compiles; default- feature clippy clean; both default and --all-features test paths pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(masternodes): correct global-nav coverage claim (F-003) CHANGELOG and the components README claimed the global wallet/identity switcher was on "every screen". It ships Phase-A: rendered on Identities, DashPay, DPNS, Wallets, Identity Hub, and Masternodes, interactive only on the Hub and Masternodes (the other four render subdued, read-only pills), and absent from every other root screen (Contracts, Tokens, Tools, Network Chooser, Withdraws, ...). Names the actual screens and notes the rest as a tracked follow-up instead of implying full rollout. * fix(masternodes): Fable final round — Tier-2 merge seal, load gate, ProTxHash error F-001 (MUST-FIX): "Add voting key" on a password-protected (Tier-2) node no longer trips the insert's fail-closed guard. load_identity now verifies the node's object password UP FRONT (before the network fetch, mirroring add_key_to_identity's verify-before-broadcast order) and seals the merged plaintext keys Tier-2 via seal_merged_plaintext_keys just before the at-rest insert. Two regression tests: a scripted-prompt success path proving the new key flips InVault and reads back Protected, and a headless NullSecretPrompt path proving the merge fails closed with SecretPromptUnavailable before fetch. F-002: the list screen's load_in_flight gate is cleared only on the load's own LoadedIdentity result variant (not any routed result), with a refresh_on_arrival backstop so a tab switch mid-load can never strand "+ Load" at "Loading…". F-005: a malformed identity-id input now surfaces MalformedProTxHash for masternode/evonode loads (where the field IS a ProTxHash) and keeps IdentifierParsingError for User loads. Regression test added. F-006: masternodes/evonodes legitimately have no HD wallet, so the "saving identity without wallet" warning is gated to User identities; nodes log at debug instead. F-004: correct the MCP masternode_identity_load comment — Overwrite is a destructive full-replace of stored keys, not a merge/refresh; TODO for a future load-mode param. F-007: strip ephemeral review-ID prefixes (SEC-*, Diziet-*, Smythe) from masternode-scope source comments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): add load-form back link + remove object pill from breadcrumb Live-walkthrough fixes on real testnet data. Fix 1 — load form back link: the load form now renders the same `‹ All masternodes` back link as the detail view (wireframe C shows it on both), at the top of the form, returning to the card list. New kittest `load_form_back_link_returns_to_list` covers it; the existing `load_form_opens_from_cta_and_cancels` gets a taller headless window so the bottom Cancel button stays reachable now that the back row is present. Fix 2 — remove the masternode object/identity pill from the Masternodes breadcrumb. Masternode/evonode identities are never wallet-linked (wallet_info is always None — locked decision #4), so pairing a wallet pill with an object pill implied a wallet↔masternode relationship that does not exist. The breadcrumb now carries only segment-1 + the interactive wallet pill; node selection is driven entirely by card-click → detail and the back link. The Masternodes page switches to add_top_panel_with_global_nav (non-capturing), matching every other non-object page. The masternodes_page_nav_spec builder drops its items/selected params. This does NOT touch the FR-6 boundary, which is enforced structurally at the resolution layer (B1) independent of any pill. The release-blocking FR-6 boundary tests (TC-NAV-12 / TC-FR6-07) remain unchanged and green. The PageScopedObject / SelectPageObject / add_top_panel_with_global_nav_capturing machinery is retained as the documented, tested boundary pattern for future page-scoped-object features (the global_nav_switcher tests still exercise it); only the Masternodes page's use of it is removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): reject load when selected node type mismatches on-chain Loading a masternode/evonode by ProTxHash trusted the load-form Masternode/ Evonode toggle as ground truth with no cross-check. A regular masternode loaded with "Evonode" selected was silently accepted, mis-badged "Evonode", and shown the Evonode-only "Claim token rewards" action. The load task (authoritative layer) now cross-checks the selected type against the node's actual on-chain registration. A masternode's Platform identity id is its ProTxHash, so the node is looked up via Core RPC `protx info`; the `type` field ("Regular"/"Evo") is classified and compared. On a confirmed mismatch the load is rejected with a typed, actionable `TaskError::NodeTypeMismatch` naming both the selected and actual types. When the on-chain type cannot be determined (Core RPC unreachable — e.g. an SPV-only setup with no Core node) the load proceeds unverified, so this adds no regression for those users. Layering per CLAUDE.md: the pure classification (`classify_protx_node_type`) and rejection decision (`node_type_conflict`) live in `model/masternode_input` and are exhaustively unit-tested (the reported Evonode-on-regular case included); the backend task owns the network lookup and enforcement. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): surface a visible warning when node type is unverified Follow-up to the node-type cross-check: when Core RPC is unreachable (the common case for SPV-only users) the node type cannot be verified, and silently proceeding with an unverified badge reproduced the original UX bug downgraded from "wrong" to "unverified". The load task now distinguishes the two success outcomes via a new `BackendTaskSuccessResult::LoadedIdentityTypeUnverified` variant, and the Masternodes screen surfaces a visible warning banner (not just a log line) telling the user the badge reflects their selection and to reload later to confirm. The MCP masternode-load tool reports the same distinction via a new `node_type_verified` output field. Regression tests: the pure reject decision (`node_type_conflict`) and the `NodeTypeMismatch` user message cover the hard-reject path; a kittest drives the unverified-load result into the live screen and asserts the warning banner is surfaced to the UI, not merely logged. Confirmed with team-lead: hard-reject on a confirmed mismatch; a visible (not log-only) warning on the unverified path. The upstream platform-wallet SPV masternode-list passthrough (for verifying node type without Core RPC) is tracked as a separate follow-up against the platform repo. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * revert(masternodes): drop Fix #3 node-type validation entirely Reverts c5167787 and 755eee87. Product decision: trust the user's Masternode/Evonode toggle as-is, with no on-chain node-type verification. Rationale: the only non-type-gated actions (Withdraw/Top up/Transfer) are unaffected by a mis-toggle; the sole type-gated action ("Claim token rewards") degrades to a clean no-op/failed Platform state transition, not a fund-safety issue — so the toggle working as the user set it is correct behavior, not a defect. Dropping verification also removes the dependency on Core RPC (being deleted in the platform-wallet migration) and on fetching the operator identity (extra scope), leaving the load path simpler and migration-proof. Removes: TaskError::NodeTypeMismatch, the onchain_node_type Core RPC cross-check, the classify_protx_node_type/node_type_conflict model helpers, the LoadedIdentityTypeUnverified result variant + UI warning banner, the MCP node_type_verified output field, and all associated tests. Fixes #1 (load-form back link) and #2 (breadcrumb pill removal) in 5db23f72 are untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): adapt retargeted tab to platform-wallet rewrite APIs Rebasing the Masternodes tab onto the platform-wallet backend rewrite (PR #860) surfaced three call sites where the rewrite reshaped an API the masternode-tab code depended on: - `AppContext::identity_kv()` was renamed to `det_kv()`; the load-path existence check (`get_local_qualified_identity`) now calls the new name. - `ContestState::state_is_votable()` was dead-code-removed by the rewrite, but `ContestedName::is_open_for_voter` (Masternodes card DPNS status) relies on it — restored as a live, un-gated method. - The rewrite dropped the `identity-hub` Cargo feature and renders the Identity Hub nav entry unconditionally; the left-panel builder no longer gates that entry behind the removed `#[cfg(feature = "identity-hub")]`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(withdraw): screen-level kittest coverage for default_withdrawal_key fix WithdrawalScreen::new() pre-selecting via default_withdrawal_key() was only unit-tested at the QualifiedIdentity model layer. Add tests/kittest/withdraw_screen.rs to verify the fix at the actual screen layer: ghost-key identities (on-chain-only TRANSFER key) render the no-keys empty state instead of a form, private-key-backed TRANSFER/OWNER keys are pre-selected AND rendered correctly in the key-selection ComboBox (via accesskit value, not label). Also locks in a genuine regression the fix newly exposes: WithdrawalScreen::new() leaks a raw "No key provided when getting selected wallet" error banner for any identity with no locally-signable withdrawal key. * fix(withdraw): skip wallet resolution when no signable key exists WithdrawalScreen::new() called get_selected_wallet with selected_key=None after default_withdrawal_key() correctly began returning None for identities with no locally-signable withdrawal key. With app_context=None that hits the "No key provided" String Err path, which .or_show_error() posted verbatim into a user-facing MessageBanner — violating the plain-language error policy. Guard the call on selected_key being Some, so the raw-string Err branch is structurally unreachable here instead of avoided by luck. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(masternodes): add user story for network-switch reset behavior Live QA of the Masternodes tab confirmed MasternodesScreen::reset_for_network_change() cleanly resets the List view and clears stale banners/form data on a network switch, with no existing story documenting this PR's fix. Add MN-010. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(withdraw): flip banner-leak test to a regression lock (2edbc18e) WithdrawalScreen::new() now guards get_selected_wallet on selected_key being Some (2edbc18e), so the raw "No key provided..." banner leak for ghost-key-only identities is fixed. Rename ghost_key_construction_leaks_raw_error_banner -> ghost_key_construction_does_not_leak_raw_error_banner and invert the assertion; also verify the no-keys empty state still renders correctly for the same identity, so the fix didn't trade the leak for a broken empty state. * fix(mcp): stop det-cli double-prefixing the HTTP bearer token rmcp 1.7's StreamableHttpClientTransportConfig::auth_header takes the raw token and lets reqwest's bearer_auth prepend "Bearer " itself. The det-cli client passed format!("Bearer {token}"), so the wire header became "Authorization: Bearer Bearer <token>". The server middleware strips one "Bearer " and compares the remaining "Bearer <token>" against the configured key — never equal — so headless HTTP mode returned 401 on every request. The auth path had no end-to-end coverage, which is why it shipped broken. Pass the raw token and add tests/mcp_http_auth.rs pinning the server's wire contract: raw token accepted, a double-"Bearer" prefix rejected, missing credentials rejected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): share Expert Mode flag app-wide; node-specific load error Two live-QA bugs on the Masternodes tab (PR #876). Bug 1 — Expert Mode didn't reveal the Masternodes nav without a restart. `developer_mode` was an independent per-network `AppContext` AtomicBool, kept in sync only by a best-effort loop in the Settings checkbox handler over the contexts that happened to exist at click time. AppState keeps one context per network (only the active one at startup; others created lazily on switch), so the left-nav `FeatureGate::DeveloperMode` gate could read a stale value on a different context than the toggle mutated — the nav entry stayed hidden until a restart re-read the persisted flag into the single fresh context. Promote the flag to one shared `Arc<AtomicBool>`, created once in `AppState::new` and injected into every `AppContext::new` (startup + `SwitchNetwork` via `developer_mode_handle()`), so all per-network contexts observe one flag. Drop the fragile sync loop; request a repaint on toggle since enabling Expert Mode disables animations (which stops continuous repaints). Bug 2 — loading a masternode by a valid-but-unregistered ProTxHash showed the generic "Identity not found — check the ID or name" copy, wrong for a ProTxHash load form. Add a dedicated `TaskError::MasternodeNotFound` variant and return it from the masternode/evonode load path instead of `IdentityNotFound`. Regression tests: `developer_mode_is_shared_across_network_contexts` (RED before the shared-flag fix), `masternode_not_found_message_is_node_specific`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(user-role): introduce UserRole + composable FeatureGate checks (Phase 1) Introduce the typed persona axis and generalise the feature gate into a conjunction of heterogeneous checks, with a zero-behaviour-change compat shim over the retired binary Expert Mode flag. Phase 1 only — the ~43 is_developer_mode() callsites and the role-setting UI are untouched. - model/user_role.rs: ordered UserRole { Everyday<Power<Developer }, pinned discriminants, as_str/from_persisted (sentinel-safe)/at_least/from_u8. - context/feature_gate.rs: Capability (ShieldedProtocol predicate moved verbatim, per-network by construction), Check { MinRole, Capability, Experimental }, empty ExperimentalFeature, FeatureGate::checks() table + is_available = checks().all(). DeveloperMode stays mapped to >= Power. - context/mod.rs: re-type the shared app-global atomic Arc<AtomicBool> -> Arc<AtomicU8> (UserRole discriminant); user_role()/set_user_role()/ experimental_enabled()/user_role_handle(); is_developer_mode() and enable_developer_mode() kept as >= Power compat shims; animation gate re-pointed at >= Power. - model/settings.rs: replace UserMode/user_mode with Option<UserRole>/ user_role, reusing the length-prefixed user_mode wire slot (no offset shift). "Advanced"/"Beginner"/empty/unknown decode to None (a sentinel), never a role — mapping the universal legacy default to a role would silently promote every user. - context/settings_db.rs: seed a role-less (None) blob once from .env DEVELOPER_MODE (true -> Power, else Everyday) at get_app_settings, mirroring the impure dash-qt autodetect fallback. - app.rs / backend_task / mcp: seed and share the role atomic from .env. Tests: UserRole ordering/round-trip/sentinel; canonical wire round-trip; "Advanced" -> None; role-less blob seeds Power from .env; explicit role not reseeded. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(ai-design): add persona/capability-gating design doc Commit the design doc referenced by src/model/user_role.rs's doc comment so the pointer resolves once this branch merges — it previously only existed on the separate design/persona-capability-gating branch. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> * feat(user-role): reclassify dev-mode callsites onto role/capability gates (Phase 2/3) (#880) * feat(user-role): reclassify dev-mode callsites onto role/capability gates (Phase 2) Walk every is_developer_mode()/FeatureGate::DeveloperMode callsite and reclassify each per the four-bucket rubric, then wire the Masternodes tab to its own gate. - Add FeatureGate::Masternodes (>= Power); repoint the masternodes nav entry and the app.rs live de-gating guard onto it. - Bucket 1 (disclosure): reclassify to user_role().at_least(Power). - Bucket 2/3 (signing override at state_transition_options + has_keys proceed-without-key bypasses): tighten to at_least(Developer). This is an intentional behavior change from today's single dev flag (== Power). - Bucket 4 (experimental/stability: shielded send + tab, DashPay pay/ subscreens): move to Check::Experimental via new ExperimentalFeature {Shielded, DashPay}; experimental_enabled() stays >= Power for now. - Rename FeatureGate::DeveloperMode -> DeveloperTools (>= Developer) as the forward-looking Developer-tier gate; delete the is_developer_mode() and enable_developer_mode() compat shims (no callers remain). - Remove dead AddressInput::with_developer_mode/set_developer_mode and the never-set developer_mode field. - Update kittest role toggles and the shared-role regression test; flip user story WAL-022 (system accounts) from developer-mode to Power role. PROJ-007 sites (button_text.contains("Test") i18n fragility) left as-is per brief — only their dev-mode gate portion was reclassified. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(user-role): QA follow-ups — IDH-005 wording, dead fallback, override test - docs/user-stories.md IDH-005: retitle to "Bulk identity creation", persona Jordan -> Priya,Jordan, and reword the footer/dropdown criteria to the Power role, matching the Power reclassification of the test-identities footer. - withdraw_screen: tighten the on-chain-only key pre-select fallback from at_least(Power) to at_least(Developer). Only Developer can actually sign with such a key (signing override + the Developer branch of the has_keys gate), so the Power-level pre-select was dead and its comment overclaimed. Comment fixed. - context: add regression test for the state_transition_options signing override — Everyday/Power -> None, Developer -> Some with both allow_signing_with_any_* flags true. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(user-role): role-picker UI on Settings + Welcome, single-source persistence (Phase 3/3) (#881) * feat(user-role): role-picker UI on Settings + Welcome, single-source persistence (Phase 3) Make AppSettings.user_role the single source of truth for the runtime role atomic and add both role-setting UI surfaces. Persistence wiring: - app.rs boot no longer reads .env DEVELOPER_MODE directly; the shared role atomic starts at the default and is seeded from get_app_settings() once the active context exists (the .env parser stays for the v34 migration). - settings_db: get_app_settings now persists the one-time .env seed back to the DB, so the sentinel slot is consumed exactly once and later .env changes no longer move the role. New AppContext::set_and_persist_user_role centralizes "set runtime atomic + write canonical AppSettings string" for both surfaces. UI: - Network Settings: replace the binary Expert-mode checkbox with a three-way UserRole selector — Default view / Detailed view / Developer tools — with a per-mode description. Advanced (RPC/SPV) options stay Power-gated; the Developer-tools sub-panel now keys off the Developer role. - Welcome screen: add an experience-level onboarding row (Everyday/Power/ Developer) writing the same persisted role. Tests: add env-seed-consumed-once regression; existing role/seed tests green. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(user-role): CLI/MCP boot single-source + role-selector UX polish (Phase 3 QA) - mcp/server.rs init_app_context (CLI/MCP standalone boot) seeded the role atomic straight from .env DEVELOPER_MODE, bypassing AppSettings.user_role — a role chosen in the GUI was ignored headless. Now seeds from get_app_settings().user_role, matching the GUI boot path (single source of truth). - UserRole gains label()/description() as the shared selector vocabulary; both the Settings selector and the Welcome onboarding row now use them, so a role picked in one is findable by name in the other. The Everyday description is a complete sentence (i18n rule). - Welcome row now shows the selected mode's description (parity with Settings). - Settings "Interface mode" selector lifted above the force-collapsing Advanced Settings panel so it is always discoverable; its description uses the theme-aware text_secondary(dark_mode) getter. - NetworkChooserScreen::refresh_on_arrival re-syncs selected_role from the app-global role so the radios never show a stale value. - Tests: kittest coverage for both surfaces (set + persist role) and a UserRole label/description test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(user-role): document three-role interface-mode system, close #371 Add docs/user-roles.md covering the Default view/Detailed view/Developer tools model: where to set it (Network Settings "Interface mode" card, Welcome screen onboarding row), reversibility, and the one-time DEVELOPER_MODE .env seed (true -> Detailed view, false/unset -> Default view) that is never re-read once a role is chosen. Link it from README and .env.example. Fix docs/user-stories.md entries (NET-005, NET-006, MN-002, NET-015) still describing the retired Expert Mode toggle/Beginner- Advanced mechanism instead of the shipped role selector. Note: docs/expert-mode.md (added by aebae01b) never merged into v1.0-dev, so this is a net-new doc rather than the planned git-mv rewrite. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(changelog): document Expert-mode replacement with interface levels Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com> --------- Co-authored-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> * fix(user-role): stop settings loss on k/v read failure; gate shielded ops on capability Bot-review fixes on the persona/capability-gating rollout. Settings persistence (data loss): - `load_app_settings_uncached` mapped every k/v read error to `AppSettings::default()`, then — seeing a role-less default — seeded a role and persisted the whole defaults blob. One transient read failure (poisoned lock, SQLite hiccup, schema mismatch) therefore overwrote the user's real settings: network, onboarding, SPV prefs, theme. It now returns `Result`, so an unreadable blob is never mistaken for "nothing stored". `update_app_settings` aborts instead of committing its mutation on top of defaults; `get_app_settings` keeps its in-memory defaults fallback for the frame loop but writes nothing back. - `set_and_persist_user_role` published the role to the runtime atomic before persisting and swallowed a persist failure as a log warning, so the UI accepted a mode that silently reverted on restart. It now persists first, publishes only on success, and returns the error. Both callers surface it: the settings selector reverts its radio group and shows a banner; the onboarding row shows a banner (its role is re-read from the context each frame). Feature gating: - Shielded send sources and shielded destinations called `experimental_enabled` directly, bypassing `Capability::ShieldedProtocol`, so a Power/Developer user was offered shielded options on networks whose protocol version defines no shielded state transitions — while the shielded tab itself was correctly hidden. Adds `FeatureGate::ShieldedOperations` (capability AND experimental — the first multi-check gate) and routes send_screen + shielded_tab through it. - Routes the four raw DashPay `experimental_enabled` callsites through a new `FeatureGate::DashPayOperations`, so every gating decision goes through the single composition point. `FeatureGate::DashPay` (nav entry) is unchanged. Tests: - `FailingKv` (kv_test_support): a store whose reads can be armed to fail, counting puts — proves a failed read writes nothing back and that the stored blob survives. - `context::test_support`: shared `AppContext` fixture, lifted out of settings_db's test module so feature_gate can reuse it. - feature_gate gains its first test module: per-role availability, the empty conjunction, and the AND semantics of the new multi-check gate. No protocol version upstream defines the shielded state transitions today, so the "capability met" half of the AND is not yet reachable; a tripwire test fails loudly when upstream ships them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor(user-role): encapsulate the shared role atomic behind UserRoleCell `AppContext.user_role` was a raw `Arc<AtomicU8>` threaded through the constructor and hand-decoded at every read (`UserRole::from_u8(load(..))`) and write (`store(role as u8, ..)`), with ~20 construction sites spelling out the atomic encoding. Introduce `UserRoleCell` in `model/user_role.rs`, next to the enum it wraps: `get()` / `set()` plus `Clone` as the cheap shared handle that wires sibling per-network contexts to one value. `UserRole::from_u8` drops to private — the encoding is now the cell's business alone. Behaviour-preserving; `user_role_handle()` becomes `user_role_cell()`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: drop the stale DEVELOPER_MODE interface-mode claims `DEVELOPER_MODE` no longer seeds the interface mode at all — the role of an account that never chose one is Power, resolved in memory with no `.env` read. Two living docs still advertised the retired one-time-seed behaviour. CHANGELOG's `[Unreleased]` entry now states what an account without a chosen level actually gets (Detailed view, so nothing the old Expert mode showed is hidden) and that `.env` has no say in it. README's environment-variable table drops the `DEVELOPER_MODE` row outright: the app reads no such variable for configuration, so a row in a table of supported variables is a false claim rather than a stale one. The migration detail it used to carry — that the key survives only as an input to the one-shot v34 SPV database upgrade — already lives in docs/user-roles.md, which the replacement note points at. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
The runtime role cell is seeded once per boot from `get_app_settings()`, which degrades to a defaults blob on ANY read error (poisoned lock, SQLite hiccup). Those defaults resolve to `WHEN_UNSET` — Power — so a single transient failure on that one call handed the whole process the power surface, for a user whose stored role may well be Everyday. Nothing re-seeds the cell from the later reads that succeed. "The user never recorded a role" and "we could not read what the user recorded" are different facts and now resolve in opposite directions: - `UserRole::LEAST_PRIVILEGED` (Everyday) — the value to trust when the stored one cannot be. `WHEN_UNSET` (Power) keeps applying to a *known* absence: fresh installs and pre-role blobs keep the surface they had. - `try_get_app_settings()` reports a failed read instead of erasing it; `get_app_settings()` stays infallible for the frame loop. - `seed_user_role_from_settings()` is the one boot chokepoint for both the GUI and the MCP server, and seeds the least-privileged role on a failed read. Recovery is self-service: the interface-mode selector reads the same cell, so re-picking the mode persists and republishes it. Both callsites' `.unwrap_or(UserRole::WHEN_UNSET)` are gone — a provably unreachable second copy of a default that would have gone stale silently had `with_default_user_role` changed. The invariant they duplicated (a returned `user_role` is always `Some`) is now asserted with an `expect` in one place, and holds on the cached path too: both writers cache the resolved view rather than the raw blob, while the store still keeps a role-less blob role-less until the user picks one. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`UserRoleCell` is shared by every per-network `AppContext`, but `animate` was a per-context mirror of it: `set_user_role` → `enable_animations` only wrote the context the call passed through. A previously-created but idle context (testnet, while mainnet is active) kept animating according to the role the user had already left behind, and only caught up if a later role change happened to land on it. Stop mirroring. `animations_enabled()` derives the answer from the shared role on every read, so staleness is not merely fixed but unrepresentable — no fan-out to sibling contexts, which an `AppContext` has no handle on anyway. The stored flag survives only as what it always really was: an explicit override (`set_animations_disabled`) that automated runs use to keep the frame loop still, independent of whatever role the fixture holds. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…rsion
`Capability::ShieldedProtocol` read `max_version > 0` on each shielded
state transition's `FeatureVersionBounds` as an "unshipped" sentinel. It
is not one: `check_version` is `v >= min && v <= max`, so `{min: 0,
max: 0}` is a legitimately checkable v0 bound — `identity_create_state_
transition`, a live feature, ships exactly that triple. The check only
reads as "not yet defined" by coincidence, because no protocol version
has shipped shielded transitions yet; had they shipped at v0, like every
other v0 DPP feature, the capability would have stayed false forever and
the shielded surface would never have unlocked.
Compare the network's protocol version against a named activation
constant instead. `SHIELDED_ACTIVATION_PROTOCOL_VERSION` is `None` while
the transitions remain unshipped, so behaviour is unchanged today —
unmet on every network — and activating the capability later is a
one-line edit rather than an archaeology exercise.
The tripwire test keeps its purpose, pointed at the constant: it fails
the moment that names a shipped version, forcing whoever activates it to
re-check the shielded gates.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two requirements no longer describe what shipped, and read as unmet rather than superseded. Mark them, and record the decisions: DEVELOPER_MODE was dropped from role resolution entirely rather than migrated, and `WHEN_UNSET` resolves to Power uniformly — fresh installs included — which is a deliberate later product decision, not an oversight. Points at the canonical statements of the behaviour rather than restating them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ail-fast load-skip API (#874) * fix(deps)!: pin platform-wallet/dash-sdk to PR #3968 head, adapt to fail-fast load-skip API Points dash-sdk, rs-sdk-trusted-context-provider, platform-wallet, and platform-wallet-storage at dashpay/platform PR #3968's head commit (0503f80df5529b1d4f91e58c49abe08cde7b283f, feat/platform-wallet-storage-rehydration) instead of the dash-evo-tool integration branch. This rev predates several APIs the dash-evo-tool branch had added, so the consuming code is adapted rather than upstream: - `SkipReason`/`CorruptKind`/`PlatformEventHandler::on_wallet_skipped_on_load` don't exist at this rev — `load_from_persistor()` returns a plain `Result<(), PlatformWalletError>` instead of the tri-state `LoadOutcome`. Removed the DET-side `PersistedLoadSkip` mapping, the "N wallets skipped" banner machinery, and the trait override; any load failure (including a wallet that was already registered) is now treated as fatal, per product decision — DET's existing empty-manager gate should prevent `AlreadyRegistered` from firing on this path; if it fires anyway, that is a DET-side bug and should surface loudly rather than be silently skipped. - `pwm.shutdown()` returns `()` at this rev, not a clean-shutdown report — commented out the report check (user-confirmed) rather than dropping it outright, pending platform restoring the report type. - Six `PlatformWalletError` variants (`RehydrationTopologyUnsupported`, `RehydrationPoolMismatch`, `RehydrationPoolTypeMismatch`, `PersisterLoad`, `AddressNonceMismatch`, `ShieldedShutdownIncomplete`) don't exist at this rev. All six already folded into the same generic wrapper/bucket as every other case at both call sites, so removing their match arms changes no behavior for variants that still exist. - `From<PersistenceError> for PlatformWalletError` doesn't exist at this rev — the two call sites now build `PlatformWalletError::Persistence(String)` directly instead of `.into()`. Full workspace clippy (--all-features --all-targets -- -D warnings) and the full test suite are clean: 1372/1372 lib, 10/10 e2e, 184/184 kittest, 3/3 legacy-table-surface, doctests clean. See the platform TODO (fc45c3da) for the upstream ask to simplify load_from_persistor's error model and restore the removed variants/report type once platform catches this rev up to dash-evo-tool branch. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test: remove orphaned skipped-wallets-banner kittest coverage skipped_wallets_banner_text and the "N wallets skipped" banner it fed were removed in the prior commit as part of adapting to PR #3968's fail-fast load-skip API — this kittest file existed solely to cover that banner and has nothing left to test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(deps): re-pin platform-wallet/dash-sdk to PR #3968 new head, restore typed persistence errors Re-points dash-sdk, rs-sdk-trusted-context-provider, platform-wallet, and platform-wallet-storage from 0503f80d to dashpay/platform PR #3968's new head 93b967f9c7ab0164b47fe825d2bae58b3974625c (branch feat/platform-wallet-storage-rehydration, base v4.1-dev). Cargo.lock moves every platform git package plus the transitive rust-dashcore pin (48a07d3f -> b3fb8244) that platform now requires. Upstream API delta since 0503f80d that touches DET: - `PlatformWalletError::PersisterLoad(#[from] PersistenceError)` restored (upstream 728f480f). The two contact-request / dashpay-payment record sites go back to `e.into()` — a typed `PersisterLoad` wrapping the upstream `PersistenceError` — instead of the manual `Persistence(String)` stringification the 0503f80d pin forced. - `PlatformWalletError::AddressNonceMismatch` restored (upstream aae79a86). Both exhaustive matches (`map_shielded_op_error`, `identity_op_error_kind`) regain their arms; the address-nonce fault buckets as a generic wallet-backend / Other error, unchanged from before. Unchanged at the new head, so the 0503f80d adaptation stands: - `load_from_persistor()` still returns `Result<(), PlatformWalletError>` — no `LoadOutcome`/`SkipReason` tri-state — so the skip-banner machinery stays removed (no per-wallet skip data exists to surface). - `shutdown()` still returns `()` — the clean-shutdown report check stays commented with its TODO(platform-pr3968) marker. - `RehydrationTopologyUnsupported`, `RehydrationPoolMismatch`, `RehydrationPoolTypeMismatch`, and `ShieldedShutdownIncomplete` still absent — the match TODOs are trimmed to just these four. The new upstream classified-SPV-peer API (SpvPeerInfo / connected_peers) is additive and unconsumed by DET; no adaptation needed. Full workspace clippy (--all-features --all-targets -- -D warnings) and the test suite (--all-features --workspace --no-fail-fast) are clean: 1372 lib, 10 e2e, 184 kittest, 3 legacy-table-surface, doctests, backend-e2e gate. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(wallet-backend): correct dropped upstream error-variant TODOs The two error-mapping TODOs claimed RehydrationTopologyUnsupported, RehydrationPoolMismatch, RehydrationPoolTypeMismatch, and ShieldedShutdownIncomplete would be re-added to PlatformWalletError. Per maintainer review the three Rehydration* variants are permanently dropped upstream; only ShieldedShutdownIncomplete may return. Trim the lists to the single variant that may reappear — the exhaustive match already forces a review here when any upstream variant is added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
…op-up (#877) * docs(user-stories): clarify IDN-013 scope is standalone-imported identities only HD-wallet-backed identity keys are derived on demand from the wallet's own seed and already covered by that wallet's password; the Key Protection section is hidden entirely for such identities since there is no separate vault key to protect. Addresses PR #860 review comment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs(test-spec): QR/receive-deposit identity funding test cases (Phase 1b) Test case specification for restoring IDN-014 receive-deposit funding on the Register and Top-Up identity screens. Descriptions/expected-outcomes only, no test code. Corrects the Phase-1a brief: detection is single-address equality (not known_addresses), funding_address is per-screen, deposit transitions are unit-level via display_task_result (not kittest-reachable today). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(qr-funding): Phase 1c development plan for IDN-014 restore Task breakdown for developer-bilby: shared pure helper + FundingMethod variant in funding_common.rs, wiring both identity screens through the existing FundWithWallet path, and the user-stories IDN-014 flip. References TC-QRFUND-01..17. No upstream/backend/TaskError changes in scope. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MXL4vmWNA1AjUeDVu2EqWA * feat(identity): restore QR/receive-deposit funding for register + top-up Restores IDN-014: fund a new identity or a top-up by receiving a Dash deposit to a shown address/QR, routed through the existing FundWithWallet -> AssetLockFunding::FromWalletBalance path. No upstream platform-wallet change, no new BackendTask/WalletTask or TaskError variant. - funding_common: add FundingMethod::ReceiveDeposit with jargon-free labels, plus pure, unit-tested helpers deposit_matches (single-address equality) and deposit_step_after_utxo (waiting-state guard). - add_new_identity_screen + top_up_identity_screen: new "Receive a new deposit" chooser option -> GenerateReceiveAddress -> WaitingOnFunds; revived WaitingOnFunds/FundsReceived flow with QR, address + copy, minimum hint, running-total line, and a never-trap "choose a different funding method" affordance. Confirm reuses the wallet-balance dispatch. - docs/user-stories.md: flip IDN-014 to [Implemented]. Tests: 24 funding_common unit tests (TC-QRFUND-02..08, 16); state-machine detection is verified through the extracted pure helpers since the kittest harness cannot inject task results. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(identity): QA polish for QR/receive-deposit funding Address pre-merge QA findings on the restored IDN-014 funding method: - Smythe LOW-1: on a malformed derived deposit address, surface a MessageBanner error and stop the QR view's infinite re-queue via a new funding_address_request_failed flag; the view offers a "Try again" affordance instead of spinning forever. - Smythe INFO-1: deposit_matches sums attacker-influenced Core output values with saturating_add instead of plain sum (defense in depth). - Adams DOC-2: the minimum-amount hint and the dash: URI now share one round_up_dash_4dp value, so they never disagree or understate the minimum (rounds up to the URI's 4-decimal precision). Unit-tested. - Adams DEDUP-2: reset_to_choose_funding on both screens now sources its (method, step) from default_funding_state(false) instead of hand-writing the tuple. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(identity): fix QR-funding amount pre-fill, per-address total, stale error Three functional bugs a live testnet GUI test found (unit tests + review missed them — the display_task_result-level integration gap we accepted): - Bug 1 (amount not pre-filled): on reaching FundsReceived the amount field stayed empty and the Create/Top-Up button never appeared until the user typed or clicked Max — contradicting the PR body and TC-QRFUND-08. On the deposit-arrival transition, pre-fill the fee-reserve-capped balance so the amount and confirm button populate immediately, still editable. - Bug 2 ("received so far" showed whole-wallet balance): the running total read snapshot spendable, so leftover change made it read "Received X … waiting for Y" while already past the minimum — looked stuck. Now accumulate the amount received at the shown funding_address (via deposit_matches) and display that per-address total. - Bug 3 (stale validation error): the amount input kept rendering during WaitingForAssetLock, and its max recomputed to 0 once spendable was committed to the pending tx, showing a red "exceeds maximum 0" over a succeeding operation. Render the input only in FundsReceived. Extract deposit_event_outcome (step + pre-fill amount) so the pre-fill decision is unit-tested; both screens route through it. Adds three funding_common tests asserting a sufficient deposit advances AND yields a non-zero pre-fill (the bug-1 regression guard), and that sub-minimum / wrong-address deposits neither advance nor pre-fill. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
* docs(user-stories): clarify IDN-013 scope is standalone-imported identities only HD-wallet-backed identity keys are derived on demand from the wallet's own seed and already covered by that wallet's password; the Key Protection section is hidden entirely for such identities since there is no separate vault key to protect. Addresses PR #860 review comment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com> * docs(test-spec): QR/receive-deposit identity funding test cases (Phase 1b) Test case specification for restoring IDN-014 receive-deposit funding on the Register and Top-Up identity screens. Descriptions/expected-outcomes only, no test code. Corrects the Phase-1a brief: detection is single-address equality (not known_addresses), funding_address is per-screen, deposit transitions are unit-level via display_task_result (not kittest-reachable today). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * Revert "docs(test-spec): QR/receive-deposit identity funding test cases (Phase 1b)" This reverts commit 8a150eb. * docs(test-spec): link top-nav wallet selector to Wallets tab test cases (Phase 1b) Test case specification for wiring the top-nav wallet pill (Defect 1: Unwired on Wallets page) and re-syncing WalletsBalancesScreen on arrival (Defect 2). 13 cases (TC-WALLETLINK-01..13), descriptions/expected only, no test code. Flags the dual-hash ambiguity trap: set_selected_hd_wallet preserves the single-key hash while in-tab SK selection clears the HD hash, and construction prefers SK first — so a stale (HD=Some, SK=Some) state resolves to the wrong wallet unless the pill's HD write is made authoritative. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(test-spec): trim wallet-selector-linking spec under word budget Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(wallet-selector): Phase 1c development plan for WAL-028 Task breakdown for developer-bilby: D1 make the Wallets-page top-nav pill interactive (Consumed spec swap, no new plumbing), D2 re-sync the cached selection in refresh_on_arrival and fix the set_selected_hd_wallet / single-key-hash asymmetry so an explicit pill HD pick supersedes a stale single-key selection. References TC-WALLETLINK-01..13. AppContext API shape unchanged; single Bilby pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MXL4vmWNA1AjUeDVu2EqWA * feat(wallets): link top-nav wallet selector to Wallets tab Two defects that left the top-nav wallet pill and the Wallets tab's own picker out of sync (WAL-028): D1 — the Wallets-page pill was inert. Its spec built `PillConsumption::Unwired`, so the already-wired `apply_global_nav_effect` -> `set_selected_hd_wallet` path never fired. Add a `Consumed` `wallet_only_spec` constructor (mirroring the Masternodes page) and swap it in; delete the un-actioned TODO. Pill click now switches the active wallet in place, no navigation. D2 — the cached selection never re-synced on arrival, and a dual-hash asymmetry made a naive re-sync show the wrong wallet: `set_selected_hd_wallet(Some)` preserved a stale single-key hash while resolution prefers single-key-first, so a pill->HD switch re-resolved to the stale single-key wallet. Fix by clearing the single-key hash inside `set_selected_hd_wallet` when an HD wallet is picked (Some) — restoring symmetry with the single-key setter and making (HD, SK) both-set unrepresentable. Clearing the HD wallet (None) still leaves a single-key selection intact. Extract construction's store->wallet resolution into a shared `resolve_selection_from_store` helper; `refresh_on_arrival` now re-reads the store through it and adopts the result (idempotent when the cache agrees), falling back to first-wallet only when the store names nothing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(user-stories): mark WAL-028 implemented, extend WAL-004 Flip WAL-028 (top-nav wallet pill on the Wallets tab) to [Implemented] and add the interactive-pill acceptance line to WAL-004. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(wallets): self-enforce the single-active-wallet invariant across setters Close the residual dual-hash gap QA flagged: the (HD, SK) both-set state was only prevented by convention of the one current caller, not by the setters themselves. - set_selected_single_key_wallet(Some) now clears the HD hash at the context layer (mirror of set_selected_hd_wallet), so a direct single-key set is self-enforcing regardless of caller, not only via the screen path. - set_selected_identity now clears the single-key hash when it sets an HD owner, so selecting a User identity while a single-key wallet is active can no longer leave the pill and the active identity disagreeing. - Remove the now-dead subdued_wallet_only_spec (its only production caller moved to wallet_only_spec) and its contrast test. - Trim three internal comment blocks to the length cap. Tests: add single_key_pick_supersedes_a_prior_hd_selection (the setter mirror of TC-07) and setters_never_leave_both_hashes_set (invariant guard across all three setters). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com>
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
Preliminary Codex-only verification confirms six blocking defects and three lower-severity findings at exact head 17f00d6. The new wallet-selector tests cannot compile, and the wallet-switching, receive-deposit, SPV-retry, and dependency-release requirements remain incomplete. A focused Cargo build was blocked because the sandbox could not create the pinned platform checkout, but the compile mismatch and behavioral findings are established directly from the source.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol/high— general (failed),gpt-5.6-sol— general (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 6 blocking | 🟡 1 suggestion(s) | 💬 1 nitpick(s)
1 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/ui/wallets/wallets_screen/mod.rs`:
- [BLOCKING] src/ui/wallets/wallets_screen/mod.rs:3359: Wallet-selector tests pass the wrong AppContext argument type
`AppContext::new` requires a `UserRoleCell` as its final argument, but this new test helper passes `Arc<AtomicBool>`. These unrelated concrete types have no conversion, so the library test target and `cargo clippy --all-targets` cannot compile.
- [BLOCKING] src/ui/wallets/wallets_screen/mod.rs:2471-2476: The Wallets-page pill leaves the displayed wallet unchanged
`add_top_panel_with_global_nav` applies `SwitchWallet` only to `AppContext` and returns `AppAction::None`. The Wallets screen keeps rendering its existing `selected_wallet`, and the same-page click performs no navigation or `refresh_on_arrival` call to adopt the new store selection. The pill therefore shows the new global wallet while the central panel and in-tab picker continue showing the old wallet. The added tests only exercise store resolution or manually call `refresh_on_arrival`; none covers the actual same-page click lifecycle.
In `src/ui/identities/add_new_identity_screen/by_receive_deposit.rs`:
- [BLOCKING] src/ui/identities/add_new_identity_screen/by_receive_deposit.rs:27-38: Receive-deposit screens repeatedly derive new addresses while awaiting a result
`pending_funding_address_request` is described and checked as an in-flight guard, but `ui()` clears it with `.take()` immediately when dispatching `GenerateReceiveAddress`. On the next repaint, no address, pending value, or failure is present, so another derivation is queued. Late results then overwrite `funding_address`; a deposit sent to an earlier displayed address no longer matches the newest address and cannot advance the flow. `top_up_identity_screen/by_receive_deposit.rs` has the same lifecycle.
In `src/ui/identities/add_new_identity_screen/mod.rs`:
- [BLOCKING] src/ui/identities/add_new_identity_screen/mod.rs:1301-1307: Address-generation errors trap identity creation in a control-less state
An unhandled `GenerateReceiveAddress` error reaches `display_message`, which resets every error or warning to `ReadyToCreate`. The receive-deposit renderer only provides address retry/change-method controls in `WaitingOnFunds` and amount controls in `FundsReceived`; at `ReadyToCreate` it renders the “Deposit received” heading without either set of controls. This pre-existing generic reset is newly exposed by the added receive-deposit task and leaves the user unable to recover in place.
In `src/context/wallet_lifecycle/spv.rs`:
- [BLOCKING] src/context/wallet_lifecycle/spv.rs:154-157: SPV start failure permanently claims the start latch
`WalletBackend::start` claims `start_latch` before awaiting the fallible `spv.start(config)`. This wrapper records a returned error as `SpvStatus::Error` but discards it and returns `Ok(())`. The latch is reset only by `stop_in_place`, while `begin_spv_stop` explicitly refuses `SpvStatus::Error`; subsequent Connect attempts therefore hit the claimed latch and return success without retrying SPV initialization. Recovery requires restarting the process.
In `Cargo.toml`:
- [BLOCKING] Cargo.toml:21-37: Platform wallet dependencies remain pinned to an unreleased PR commit
All four platform dependencies are pinned to `93b967f9c7ab0164b47fe825d2bae58b3974625c`. The introducing commit identifies this as upstream PR #3968's head on `feat/platform-wallet-storage-rehydration`, based on `v4.1-dev`; the local platform object database contains no release tag that includes it. The PR description explicitly lists repinning these dependencies to a released version as a merge-to-ship blocker, so a fixed SHA does not satisfy the stated release boundary.
In `docs/gui-testing/README.md`:
- [SUGGESTION] docs/gui-testing/README.md:28-30: GUI-testing setup depends on a non-repository skill
The required display, accessibility, and launch procedure is delegated to `~/.claude/skills/desktop-gui/SKILL.md`, which is neither repository-owned nor guaranteed to exist for contributors, CI, or review environments. Because `CLAUDE.md` directs reviewers to this guide, a fresh checkout lacks a complete procedure for running the documented scenarios.
| egui::Context::default(), | ||
| app_kv, | ||
| secret_store, | ||
| Arc::new(std::sync::atomic::AtomicBool::new(false)), |
There was a problem hiding this comment.
🔴 Blocking: Wallet-selector tests pass the wrong AppContext argument type
AppContext::new requires a UserRoleCell as its final argument, but this new test helper passes Arc<AtomicBool>. These unrelated concrete types have no conversion, so the library test target and cargo clippy --all-targets cannot compile.
| Arc::new(std::sync::atomic::AtomicBool::new(false)), | |
| crate::model::user_role::UserRoleCell::default(), |
source: ['codex']
| let mut action = add_top_panel_with_global_nav( | ||
| ui, | ||
| &self.app_context, | ||
| vec![("Wallets", AppAction::None)], | ||
| wallet_only_spec("Wallets", RootScreenType::RootScreenWalletsBalances), | ||
| right_buttons, | ||
| ); |
There was a problem hiding this comment.
🔴 Blocking: The Wallets-page pill leaves the displayed wallet unchanged
add_top_panel_with_global_nav applies SwitchWallet only to AppContext and returns AppAction::None. The Wallets screen keeps rendering its existing selected_wallet, and the same-page click performs no navigation or refresh_on_arrival call to adopt the new store selection. The pill therefore shows the new global wallet while the central panel and in-tab picker continue showing the old wallet. The added tests only exercise store resolution or manually call refresh_on_arrival; none covers the actual same-page click lifecycle.
source: ['codex']
| fn queue_funding_address_request(&mut self) { | ||
| if self.funding_address.is_some() | ||
| || self.pending_funding_address_request.is_some() | ||
| || self.funding_address_request_failed | ||
| { | ||
| return; | ||
| } | ||
| if let Some(wallet) = &self.selected_wallet | ||
| && let Ok(seed_hash) = wallet.read().map(|w| w.seed_hash()) | ||
| { | ||
| self.pending_funding_address_request = Some(seed_hash); | ||
| } |
There was a problem hiding this comment.
🔴 Blocking: Receive-deposit screens repeatedly derive new addresses while awaiting a result
pending_funding_address_request is described and checked as an in-flight guard, but ui() clears it with .take() immediately when dispatching GenerateReceiveAddress. On the next repaint, no address, pending value, or failure is present, so another derivation is queued. Late results then overwrite funding_address; a deposit sent to an earlier displayed address no longer matches the newest address and cannot advance the flow. top_up_identity_screen/by_receive_deposit.rs has the same lifecycle.
source: ['codex']
| if matches!(message_type, MessageType::Error | MessageType::Warning) { | ||
| // Reset step so we stop showing "Waiting for Platform acknowledgement". | ||
| // The error itself is displayed by the global MessageBanner. | ||
| let mut step = self.step.write().unwrap(); | ||
| let mut step = self.step.write_recover(); | ||
| *step = WalletFundedScreenStep::ReadyToCreate; | ||
| } |
There was a problem hiding this comment.
🔴 Blocking: Address-generation errors trap identity creation in a control-less state
An unhandled GenerateReceiveAddress error reaches display_message, which resets every error or warning to ReadyToCreate. The receive-deposit renderer only provides address retry/change-method controls in WaitingOnFunds and amount controls in FundsReceived; at ReadyToCreate it renders the “Deposit received” heading without either set of controls. This pre-existing generic reset is newly exposed by the added receive-deposit task and leaves the user unable to recover in place.
source: ['codex']
| if let Err(e) = backend.start().await { | ||
| self.mark_spv_error(&e); | ||
| } | ||
| Ok(()) |
There was a problem hiding this comment.
🔴 Blocking: SPV start failure permanently claims the start latch
WalletBackend::start claims start_latch before awaiting the fallible spv.start(config). This wrapper records a returned error as SpvStatus::Error but discards it and returns Ok(()). The latch is reset only by stop_in_place, while begin_spv_stop explicitly refuses SpvStatus::Error; subsequent Connect attempts therefore hit the claimed latch and return success without retrying SPV initialization. Recovery requires restarting the process.
source: ['codex']
| @@ -28,7 +28,14 @@ dash-sdk = { git = "https://github.com/dashpay/platform", rev = "54048b9352c5c83 | |||
| "core_spv", | |||
| "shielded", | |||
| ] } | |||
| rs-sdk-trusted-context-provider = { git = "https://github.com/dashpay/platform", rev = "54048b9352c5c8361f4cbfaa6a188dd3244e00c4" } | |||
| rs-sdk-trusted-context-provider = { git = "https://github.com/dashpay/platform", rev = "93b967f9c7ab0164b47fe825d2bae58b3974625c" } | |||
| platform-wallet = { git = "https://github.com/dashpay/platform", rev = "93b967f9c7ab0164b47fe825d2bae58b3974625c", features = [ | |||
| "serde", | |||
| "shielded", | |||
| ] } | |||
| platform-wallet-storage = { git = "https://github.com/dashpay/platform", rev = "93b967f9c7ab0164b47fe825d2bae58b3974625c", features = [ | |||
| "shielded", | |||
There was a problem hiding this comment.
🔴 Blocking: Platform wallet dependencies remain pinned to an unreleased PR commit
All four platform dependencies are pinned to 93b967f9c7ab0164b47fe825d2bae58b3974625c. The introducing commit identifies this as upstream PR #3968's head on feat/platform-wallet-storage-rehydration, based on v4.1-dev; the local platform object database contains no release tag that includes it. The PR description explicitly lists repinning these dependencies to a released version as a merge-to-ship blocker, so a fixed SHA does not satisfy the stated release boundary.
source: ['codex']
| The mechanical launch recipe (X display setup, accessibility tree, gotchas | ||
| like stderr redirection) lives in the global `desktop-gui` Claude Code skill | ||
| (`~/.claude/skills/desktop-gui/SKILL.md`) — read that first, it's not repeated |
There was a problem hiding this comment.
🟡 Suggestion: GUI-testing setup depends on a non-repository skill
The required display, accessibility, and launch procedure is delegated to ~/.claude/skills/desktop-gui/SKILL.md, which is neither repository-owned nor guaranteed to exist for contributors, CI, or review environments. Because CLAUDE.md directs reviewers to this guide, a fresh checkout lacks a complete procedure for running the documented scenarios.
source: ['codex']
| fn reserved_core_backend_mode_byte_preserves_wire_layout() { | ||
| let wire = AppSettingsWire { | ||
| network: "testnet".to_string(), | ||
| root_screen_type: 3, | ||
| dash_qt_path: Some("/opt/dash-qt".to_string()), | ||
| overwrite_dash_conf: false, | ||
| disable_zmq: true, | ||
| theme_mode: "Dark".to_string(), | ||
| _reserved_core_backend_mode: 0, // legacy "RPC" value from an old blob | ||
| onboarding_completed: true, | ||
| show_evonode_tools: true, | ||
| user_mode: "Beginner".to_string(), | ||
| close_dash_qt_on_exit: false, | ||
| auto_start_spv: true, | ||
| }; | ||
| let encoded = | ||
| bincode::serde::encode_to_vec(&wire, bincode::config::standard()).expect("encode"); | ||
| let (decoded, _): (AppSettings, _) = | ||
| bincode::serde::decode_from_slice(&encoded, bincode::config::standard()) | ||
| .expect("decode"); |
There was a problem hiding this comment.
💬 Nitpick: Reserved-byte test does not encode an independent legacy layout
The test constructs and serializes the current AppSettingsWire, including the already-reserved field, and decodes it with the current implementation. Both sides therefore share the same field order and types, so the test establishes a current self-round-trip rather than compatibility with bytes emitted when core_backend_mode was a live field. A fixed legacy byte vector or separately defined legacy encoder would pin the claimed compatibility independently.
source: ['codex']
Goal
Dash Evo Tool carried its own large SPV/wallet stack that duplicated logic now maintained upstream. This PR rewrites DET's wallet backend on top of the upstream
platform-walletcrate (dashpay/platform), which owns SPV chain sync, address derivation, identity/asset-lock handling, and the shielded coordinator.DET is now a thin adapter over that crate: its bespoke SPV stack (
src/spv/) and the legacy RPC wallet mode are gone, chain sync and derivation come from upstream, and the shielded subsystem routes through the upstream coordinator. TheBackendTaskaction/channel contract is preserved, so the UI layer is largely unchanged. Existing wallets are migrated onto the new model on first launch (marker-gated, with a*.premigrationbackup). Net effect: far less wallet code to maintain in DET, a single source of truth shared with the rest of the platform, and upstream fixes inherited automatically.Bugs fixed
Funds-safety
watched_addressescould desync fromplatform_address_info). The wallet now reconciles this automatically via seedless reverse-derivation from the account xpub, so a visible balance can always be withdrawn. No funds were ever at risk: the withdrawal failed safely before broadcast.send_paymentnow builds and signs through upstream's nativeTransactionBuilder(upstream removed the previousCoreWallet::send_to_addressesAPI) and broadcasts viabroadcast_transaction_releasing_reservation, so a rejected broadcast can't leak a UTXO reservation. Error messages are now variant-specific — a "too many inputs" failure no longer tells the user to check their balance.Balance display
75.62vs2.16DASH on Platform). Root cause: DET was discarding derivation-index metadata upstream already provides, and the per-category summary only ever looked at DET's own address list, not the backend's authoritative one.platform_balance_duffs), so it can't lag behind. A stopgap runtime health-check banner that used to detect this class of drift after the fact has been removed — it's structurally unreachable now. Default-mode users also get a reconciling tab for any balance in a normally-hidden category (e.g. CoinJoin, or a funded address the managed-wallet pools don't recognize), so visible tabs always sum to the header total, in every mode. The Platform tab is shown immediately on wallet load (matching pre-migration behavior) rather than waiting on a completed sync pass. This fix went through two independent adversarial QA passes; the first caught that the initial version's "cannot diverge" claim was true for Platform but not yet for Core (a deeper root cause — an upstream balance-update path that could silently drop updates under its own lock contention), the second independently re-verified the fix against the actual upstream source and confirmed it closed.Shielded confirmation-safety
*ConfirmationUnknownerrors instead of a false success.Secret hygiene
Data-safety
Lifecycle / sync
Housekeeping
core_backend_modesettings field + DB column,FeatureGate::RpcBackend) was already fully inert — chain sync has been SPV-only since this migration started — but the dead weight itself was left behind pending a dedicated schema migration. It's gone now: settings field removed (with a reserved wire byte to keep the existing bincode-encodeddet:settings:v1blob layout intact — no user data at risk), DB column dropped via a new backward-compatible v38 migration (existence-guarded, idempotent, no drop-and-resync needed), and the always-false feature gate removed. Verified: RPC config fields, chain-lock RPC error plumbing, andsend_screen's Core/Platform source picker are genuinely still live and were left untouched.debug_assert!invariant guards replaced with real runtime checks. Two sites relied ondebug_assert!, which compiles out in release builds — silently disabling the check for every real user. The SPV phase-count drift guard now logs an error instead of vanishing; the tracked-lock funding empty-recipients precondition is now a typedTaskErrorreturned in every build.RUST_MIN_STACKraised to 8 MiB project-wide via.cargo/config.toml, independent of the backend-e2e suite's own dedicated 32 MiB test runtime.e6b508f1→c2135800(108 upstream commits; API drift notes below), thenc2135800→44c20e3to pick up theTransactionBuilderAPI after upstream removedsend_to_addresses(see Bugs fixed → Funds-safety). First bump's adaptations:ManagedIdentityDashPay state sealed behind adashpay()accessor;ContactXpubDatasplit into{ xpub, compact }with the DIP-15account_referencenow HMAC'd over the 69-byte compact xpub (an upstream fix matching the iOS/Android reference clients — DET's own pinned test vector was re-verified against upstream's own known-answer test and re-pinned to the new interop-correct value);IdentityWallet::dashpay_sync()replaced by explicitsync_contact_requests+sync_profilescalls;PlatformWalletErrorgained 5 variants / dropped 3; a handful of signature changes. Full gate re-verified green after each bump.WalletBackendgod-impl split intoshielded.rs/identity_ops.rs/payments.rssibling modules (mirroring the existinghydration.rs/dashpay.rspattern, zero behavior change, funds-signing path moved verbatim); a 14-copy duplicatedInMemoryKvtest fake collapsed into one sharedkv_test_supportmodule; a dead one-implPersistedWalletLoadertrait seam deleted (the G2 mock-boundary swap it existed for already shipped); several near-twinTaskErrorvariants merged; a stringly-typed warning channel in the wallet-refresh banner replaced with a typed error; stale/self-contradicting migration-module comments (some literally asserting things the code no longer does) rewritten to describe present state only; ~150 ephemeral review-round ID references (SEC-NNN/QA-NNN/etc.) stripped from committed comments per the project's own convention against citing IDs that get reassigned on every consolidation run; two unreferenced UI widgets (ContactRow/ActivityRow, ~750 lines) removed; kittest/e2e test-harness boilerplate deduplicated across 19 files. Full details in the audit's own report artifacts (not part of this PR's diff).Breaking changes
platform-walletpin includes a schema change that rewrites migrationV001in place — it drops thecore_derived_addressesandaccount_address_poolstables and hardcodes core UTXOaccount_index = 0. Existingspv//platform-wallet.sqlitedatabases are not forward-compatible and must be deleted and re-synced.Known gaps
platform-wallet/dash-sdkcrates are pinned to an unreleased branch (dash-evo-tool), not a released tag. Must be re-pinned to a released version before merge-to-ship. (Still open — this PR's two in-branch bumps, most recently to44c20e3, pick up upstream fixes within that same unreleased branch; neither resolves the need for an eventual released-tag pin.)432.41vs7.34DASH). Watch-only rehydration rebuilds accounts from xpubs and derives only the default gap window (indices0..=29) without restoring the persisted derivation depth — a regression of bug: Imported wallet shows zero balance — historical UTXOs invisible even after restart #829 Bug 2. The fix belongs upstream inrs-platform-wallet(extend each account'sAddressPoolto the highest restored index on rehydration); tracked. (Distinct from the header-vs-tab single-sourcing fixed above under Bugs fixed → Balance display — that fix closes a DET-side bookkeeping/consistency gap and does not touch rehydration's gap-window derivation, so this item stays open.)SingleKeyWalletsUnsupported, pending upstream watch-only support. Import / data-loss guard / password-restore are done.docs/ai-design/are historical record, not user-facing docs — left as-is, flagged for a possible future trim. Merge-time action: fill the ADR floor SHA placeholder (PROJ-019).Testing
cargo build/cargo build --features headlessclean;cargo clippy --all-features --all-targets -- -D warningsclean;cargo +nightly fmt --all -- --checkclean.network-info,tools,tool-describe,core-wallets-list) passes.docs/ai-design/.cargo test --all-features --workspace— 1327 lib tests passed (0 failed, 1 ignored), 187 kittest passed, 10 e2e passed, 3 legacy-table-surface passed, all doc-tests passed.cargo clippy --all-features --all-targets -- -D warningsclean.Attribution
🤖 Co-authored by Claudius the Magnificent AI Agent