fix: live-QA follow-up batch (masternodes, wallets, fonts) - #887
Merged
Conversation
The schema ladder preserves the legacy `identity` table, but no production code path imports it into the modern `StoredQualifiedIdentity` k/v store, so an upgrading v0.9.3 user silently loses every identity and all of its key material. Specify the import: what moves, where the step plugs in, its idempotency strategy, the byte contract it must produce, and the test that locks it. Also correct the 2026-05-28 migration notes, whose `identity` entry named a destination that commit b14bf32 had already moved and a version-byte agreement that is not needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A v0.9.3 install that upgrades to v1.0 kept its `identity` rows in `data.db` but nothing ever read them, so the user booted into an empty Identities screen — and a masternode owner silently lost the owner and voting keys they had loaded, since v0.9.3 stores them inside the identity blob and nowhere else. Add a third migration pass, under its own per-network sentinel (`det:migration:identities:<net>:v1`), running after the wallet drain so the backend is wired, the vault is reachable and `ctx.wallets` is hydrated for wallet-derived keys to attach to. Reusing the drain's sentinel would have skipped the import for exactly the installs that already drained under a build without it. Key material is never handled here: each decoded identity goes straight to `AppContext::insert_local_qualified_identity`, which routes keys through the secret seam and leaves only `InVault` placeholders on disk. No new secret-handling path is introduced. Details: - `legacy_import::read_identities` filters `is_local = 1 AND data IS NOT NULL` (v0.9.3's observed-identity cache is not user data) and restores `status` from its column — the bincode blob does not carry it, so every identity would otherwise read back as `Unknown`. - Skip-if-present before insert: the writer is INSERT-OR-REPLACE, so a retry after a withheld sentinel would otherwise overwrite a user's post-import edit with the stale legacy blob. - A link to an absent or still-locked wallet is preserved, never nulled: it is what re-attaches the identity when that wallet is unlocked. - An undecodable blob is counted and reported, never fatal: it withholds the sentinel (an unreadable blob may be a decoder defect a later build fixes) but does not block the identities that do decode. - `identity` joins `LEGACY_TABLES`, so an identity-only install (a masternode voter with no HD wallet) now trips legacy detection. Tests pin the v0.9.3 -> v1.0 contract end to end, including a golden blob produced by the real v0.9.3 binary (bincode 2.0.0-rc.3) asserted to decode on this tree (2.0.1) — the one cross-version claim that could not be settled by reading struct definitions. The no-plaintext-on-disk assertion reads the stored bytes before any load path runs, because the eager load-path repair would otherwise mask an importer that wrote plaintext. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
QA-001 (medium): `run()` unwrapped the app-data result with `?` before the identity import, so a hard failure in the vote/top-up pass — one malformed `det:scheduled_vote_voters:v1` blob is enough — returned early and the identity import never ran. That failure is deterministic and the app-data sentinel is never written on it, so the pass failed identically on every subsequent launch: a masternode owner's owner and voting keys would never reach the vault, on any launch, because of a broken vote queue they cannot see or repair. Run the identity import before either DET-owned result is judged, and fold both outcomes at the terminal-state step. Neither pass gates the other; a hard failure in either still surfaces to the user's retry banner, with the identity failure taking precedence when both fail — keys outrank votes. QA-002 (low): `read_identities` read `status` and `wallet_index` through a narrow `row.get::<u8>` / `row.get::<u32>`, so an out-of-range value raised `IntegralValueOutOfRange` through `?` and took the entire identity read down with it — keys included. Every other row-level corruption in that loop (bad id length, bad seed hash, half-filled wallet link, undecodable blob) is counted as `unreadable` and skipped. The legacy schema puts no `CHECK` on either column, so an out-of-range value is storable; widen the read and apply the same row-level policy. Both fixes carry a regression test confirmed RED against the unfixed code: the vote-index one imports 0 identities under the old ordering. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Merges origin/feat/v1.0-parity-batch (1d61b22 — now carrying the PR #860 platform-wallet rewrite) into feat/legacy-identity-migration, the "Update branch" step for PR #885. Three conflicts, all additive — each side appended a different item at the same location, so every one resolves as a union with nothing dropped: - app/reconcilers.rs: #885 imports migration_unreadable_identities_text, #882 imports MIGRATION_VOTES_ACK_ACTION_ID. - migration/finish_unwire.rs: #885 adds the IdentityImportFailed error variant, #882 adds TopUpHistoryWrite and VoteWarningRecord. All three kept. - database/legacy_import.rs: #885 adds read_identities, #882 adds the decode_scheduled_vote_columns / decode_top_up_columns row helpers. The migration orchestrator `run()` auto-merged, and its two orderings compose: the app-data pass (votes, top-ups) and the identity pass are each held rather than propagated, so neither DET-owned pass can gate the other or the wallet drain; identities outrank votes when both are damaged; and #882's durable vote-warning record is read back from storage, so a warning suppressed by an identity banner on one launch is re-raised on the next rather than lost. `a_corrupt_vote_index_never_strands_the_identity_keys` covers exactly that interaction and passes. Tests: build --workspace --all-features clean; clippy --all-features --all-targets -D warnings clean; full suite 1903 passed, 0 failed. Both sides' migration coverage intact — #885's six identity-import cases and #882's vote/top-up/warning cases all run. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… edge cases Address three review findings on the legacy v0.9.3 identity import path: - read_identities now selects the alias column and applies it as a fallback only when the decoded blob's own alias is None, matching the design doc's documented column-is-fallback contract. - read_identities rejects rows whose blob-embedded identity id disagrees with the row's id column, closing a gap where the skip-if-present precheck (keyed on the row id) could diverge from the actual vault write (keyed on the blob's id) and silently overwrite an unrelated identity. - finish_unwire::run now checks identities.unreadable before unwrapping the app_data result, so a deterministic app-data failure (e.g. a corrupt vote-index blob) can no longer mask the identity-unreadable banner that tells a masternode owner to reload their identity. Adds regression tests for the alias fallback and id-mismatch cases, both confirmed red against the prior code before the fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The breadcrumb pill's interactive-mode chevron (U+25BE ▾) and the masternode detail screen's icon-only Copy buttons (U+29C9 ⧉) render as missing-glyph boxes: no font this app bundles covers either codepoint (verified via direct cmap inspection of Ubuntu-Light, NotoEmoji-Regular, emoji-icon-font, and the project's NotoSans-Light). Replace both with characters already proven to render in the exact same UI: the breadcrumb separator glyph "›" (U+203A, already used elsewhere in the same nav bar) for the chevron, and the plain text "Copy" (the convention every other Copy button in the app already uses) for the icon-only buttons. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
update_owner_withdrawal_address disables the old TRANSFER key and appends a new one rather than replacing it in place, so a masternode whose payout address was rotated has two TRANSFER keys on its owner identity. key_role_label() mapped both to "Payout key" purely by purpose, producing two identical buttons in Manage keys. Add manage_keys_labels(), which appends "(disabled)" for retired keys and falls back to the key id on a residual collision (e.g. two disabled payout keys after a double rotation), guaranteeing distinct labels. Unrotated evonodes are unaffected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Several bare Unicode symbol-block glyphs used as UI icons (e.g. ▾ U+25BE, ▸ U+25B8, ✓ U+2713) are covered by none of the app's bundled fonts (Ubuntu-Light, NotoEmoji-Regular, emoji-icon-font, the Noto Sans regional set), rendering as missing-glyph boxes. Vendor Noto Sans Symbols2 (OFL-1.1, same license family as the other bundled Noto fonts) and add it to both the Proportional and Monospace fallback chains, after NotoEmoji-Regular so real emoji still resolve there first. Verified via a headless render probe using the app's actual font-loading path: the previously-tofu glyphs now render correctly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Per QA request: the masternode detail screen's Actions row keeps only Withdraw and Claim token rewards. Top Up and Transfer remain available for User identities on the Identities pages; only this screen's buttons and their navigation wiring are removed, not the shared TopUpIdentityScreen/TransferScreen. Updates the kittest assertion that previously checked for these buttons, and narrows MN-007's scope to Withdraw only. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Contributor
|
Important Review skippedAuto reviews are disabled on base/target branches other than the default branch. 🗂️ Base branches to auto review (2)
Please check the settings in the CodeRabbit UI or the ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ 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 |
The Core tab's address list enumerated wallet.known_addresses, the legacy in-memory Wallet model's frozen bootstrap window (32 external + 16 change = 48 addresses, derived once at load and never grown). The header total and tab label instead read the live WalletBackend snapshot via collect_account_summaries, which already reconciles funds on addresses derived past that window. Funds beyond BIP44 index 32 therefore counted toward the total but never appeared in the list — exactly the reported case (2 visible + 46 hidden = 48, the bootstrap window size). Add combined_address_paths(): unions known_addresses with the snapshot's address_paths (past-window funded addresses, with their real BIP44 path) plus any stray funded address outside both. Strictly additive, so existing rows and the Platform tab are unaffected. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Shielded account tab was gated by FeatureGate::Shielded, which resolved
to Capability::ShieldedProtocol -> SHIELDED_ACTIVATION_PROTOCOL_VERSION. That
constant is None ("shielded state transitions not shipped anywhere"), so the
capability was unmet on every network and the tab was hidden everywhere.
But DET configures the shielded coordinator unconditionally in
WalletBackend::new and binds Orchard keys on every cold boot / unlock, so
shielded balances are tracked and shown in the wallet balance breakdown with
no tab to view or receive them. The gate conflated two distinct things:
viewing shielded funds (a client-side scan, always possible) versus creating
shielded state transitions (needs the network capability).
Decouple the tab from the capability: FeatureGate::Shielded is now always
available (the shielded pool structurally exists on every network DET
connects to). FeatureGate::ShieldedOperations keeps the ShieldedProtocol
capability gate and its activation tripwire, so shielded send/receive
operations stay correctly gated.
set_platform_protocol_version's retroactive init path fired only on a
false->true flip of FeatureGate::Shielded, which can no longer happen; drop
it and the now-unused init_missing_shielded_wallets. Shielded binding is
already covered by the cold-boot and unlock bootstrap paths.
…ng key
Rotating a masternode payout address disables the original Purpose::TRANSFER
key (id 0) and appends a new active TRANSFER key at a higher id, so a rotated
owner identity holds two TRANSFER keys. `available_withdrawal_keys` collected
both and `default_withdrawal_key` pre-selected the first found — the disabled
id-0 key — so the withdrawal state transition was signed with a key Platform
rejects, failing with PublicKeyIsDisabledError { public_key_id: 0 } and
blocking real user funds.
Filter disabled keys out of `available_withdrawal_keys` (the single source
feeding pre-selection, the MCP masternode resolve path, and the screen gate)
and out of the manual key-chooser combo, so no signing flow can offer a key
Platform will not accept.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WVUVpGxaAsciGSiT7t4kuC
The bottom-left nav indicator rendered a fixed "🔧 Expert" for every role at or above Power, so switching between the two raised modes (or starting at the Power default) produced no visible change — the reported "switching does nothing". The role plumbing itself was correct (the shared UserRoleCell propagates live); the indicator's display simply collapsed three roles into a binary shown/hidden with one hardcoded label. The Settings/onboarding selectors and the indicator also disagreed on names: selectors read "Detailed view" / "Developer tools" while the indicator read "Expert". Fix: - Adopt one three-tier vocabulary everywhere: Default view / Expert view / Developer view (UserRole::label). Wire strings (as_str) are untouched. - Add UserRole::indicator_label — None / "Expert" / "Dev" — so the nav indicator is hidden at Default and distinct per raised role. - Drive the left-panel indicator (text + tooltip) from the live role. - Update docs/user-roles.md to the new names. Tests: unit coverage for indicator_label distinctness; a kittest asserting the indicator tracks the role and separates Expert from Dev; fixed the settings selector kittest for the renamed radio. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WVUVpGxaAsciGSiT7t4kuC
…nvariant
The `known_addresses`/`watched_addresses` retirement TODO named six readers
that must move to the display snapshot's `address_paths` before the maps and
their bootstrap can be deleted. Audited each against what the snapshot actually
carries (upstream `all_accounts()` pools + a raw address→path map, no
`path_reference`).
Finding: retirement is BLOCKED, not deferrable-yet. The snapshot structurally
cannot carry two address classes these maps hold:
- identity *authentication* keys (DIP-13/15) — upstream has no account type
for them, so they never appear in `all_accounts()`;
- DIP-17 platform-payment addresses — tracked in `platform_payment_accounts`
but omitted from `all_accounts()`.
It also drops the `path_reference`/`DerivationPathType` metadata.
Per-reader outcome (only account-summary was already snapshot-sourced):
- identity-key resolver (`qualified_identity_public_key`) resolves a User
identity's ECDSA auth keys, which live only in `known_addresses`; migrating
it to `address_paths` would leave the key unlinked and unsignable — left on
the legacy map;
- `system_tab_sections` needs per-category counts keyed by `path_reference`
and counts identity-auth addresses — the snapshot has neither;
- send-autocomplete lists DIP-17 platform addresses absent from the snapshot;
- the `wallet_lifecycle` gate is the writer-gate for the maps themselves.
No reader is safely migratable, so the maps stay (no half-delete). Rewrote the
TODO to document the concrete blockers and what upstream must expose first.
Added the resolver's first regression coverage, pinning that an identity
authentication key registered in `known_addresses` links to the owning wallet's
seed hash and exact path (and that absent/non-address keys stay unlinked). These
go RED the moment anyone points the resolver at the auth-key-blind snapshot — a
guard rail around a signing-critical path.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WVUVpGxaAsciGSiT7t4kuC
Follow-on to the disabled-key fix. When a rotated masternode owner identity has no enabled TRANSFER key loaded, `default_withdrawal_key` correctly falls back to the OWNER key (the only signable withdrawal key). A Power user could then type an explicit destination address, and DET passed it through as the withdrawal output script — but Platform's `validate_signature_purpose_matches_requirements` rejects any output script when signing with an OWNER key (WithdrawalOutputScriptNotAllowedWhenSigning WithOwnerKeyError), routing owner-key withdrawals to the registered payout address instead. The withdrawal was rejected at broadcast, blocking funds. Add `QualifiedIdentity::resolve_withdrawal_output`, a pure resolver that omits the output script for an owner-key withdrawal to the registered payout address (so Platform pays the payout address) and rejects an owner-key withdrawal to any other address with a typed `OwnerKeyWithdrawalNotAllowed` error rather than silently redirecting funds. Wire it into `withdraw_from_identity` so the guard is enforced authoritatively for every caller. Non-OWNER signing keys pass the requested address through unchanged. Note: key selection already prefers an enabled TRANSFER key over OWNER (the disabled-key fix made `active_transfer_preferred_over_lower_id_owner` pass), so the root cause was the owner+output-script combination, not key precedence. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WVUVpGxaAsciGSiT7t4kuC
…combined failure surfacing Address round-2 review on PR #885 (three blocking findings): - Alias precedence (finding 1): the v0.9.3 SQL `alias` column is authoritative. `set_identity_alias` wrote ONLY the column, and every identity loader decoded the blob then unconditionally overwrote `alias` with the column value, so a rename or removal left the blob stale and the column always won at load. The import now assigns the column unconditionally — including a NULL column clearing a stale blob alias — instead of a blob-first fallback that would resurrect a renamed-away alias. Verified against the `v0.9.3` tag; the design doc claim was backwards and is corrected. - Per-row identity column decode (finding 3): a wrong SQLite storage class on any of id/data/status/wallet/wallet_index/alias raised `InvalidColumnType` through `?`, discarding every identity already accumulated in the batch. Decoding through `decode_identity_columns` (mirroring `decode_scheduled_vote_columns`) counts-and-skips the bad row, matching the function's row-isolation policy. - Combined failure surfacing (finding 2): when unreadable identities and a hard app-data failure coincided on one launch, the run published only `SucceededWithUnreadableIdentities` and returned Ok, swallowing the app-data failure with no retry banner — every launch. Added `MigrationState::FailedWithUnreadableIdentities { count, error }`, a retryable error banner naming both problems, so neither masks the other. Funds stay safe (the drain still runs) and neither DET-owned sentinel is written, so both retry next launch. Regression tests added for all three, including a RED-verified malformed-type test proving the batch survives and an end-to-end both-failures test proving both signals surface. <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
Drop the GroveSTARK ("ZK Proofs") row from the Tools chooser panel's
visible list so it no longer appears in the menu. The screen, its
RootScreenToolsGroveSTARKScreen route, backend task, and MCP tools are
left fully intact — the tab is hidden, not removed, and stays reachable
through other entry points.
Extracts the visible nav list into `visible_tools_nav_items()` and adds
unit tests asserting ZK Proofs is absent while the other tools remain.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WVUVpGxaAsciGSiT7t4kuC
…b to Identities Live-QA request: the left nav should surface a single "Identities" entry (the former Identity Hub), with the old standalone Identities and Dashpay entries gone. GUI-only. The underlying screens, RootScreenType variants, backend tasks, and MCP tools are untouched and stay reachable through other paths (deep links, MCP tools, direct construction) — only nav visibility and the hub's label change. - Replace the runtime-assembled nav button list with a static `nav_button_specs()` that omits `RootScreenIdentities` and `RootScreenDashPayProfile` and labels `RootScreenIdentityHub` "Identities". - Add a unit test asserting both legacy screen types are absent from the nav and exactly one entry (the hub) is labeled "Identities". The Identity Hub screen already renders "Identities" in its breadcrumb and onboarding header, so no in-screen title change was needed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WVUVpGxaAsciGSiT7t4kuC
The masternode detail header rendered the truncated ProTxHash (and the voter-identity line) with a full-size ui.button copy affordance. The app's button padding (16x8) makes that button ~31px tall — far taller than the ~15px monospace hash and the ~19px type badge. In egui's left_to_right (Align::Center) row, the leading label is laid out against the initial row height and is not re-centered when the tall button grows the row, so the hash floated ~6.5px above the button and Evonode badge. Switch both copy affordances to ui.small_button (text-height), matching the inline-copy convention already used in wallets_screen. The hash, copy button, and badge now share one vertical center; residual badge offset is sub-pixel. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WVUVpGxaAsciGSiT7t4kuC
The masternode detail view labeled the Platform Transfer key "Payout key", which doesn't match the Dash Core ProRegTx role a node operator recognizes. For a masternode's Platform identity, the Owner/Voting/Transfer keys are the same key material as the DIP-3 owner key, voting key, and payout address — so the labels now follow the spec: - Detail view "Manage keys": "Payout key" → "Payout address key"; each key button and the V/O/P roles indicator gain a tooltip explaining what the key authorizes. - Load form: the three key fields (already spec-named) gain matching tooltips. - Tooltip copy is shared between both surfaces via masternodes::mod constants so the wording stays single-sourced. The operator BLS key, Platform node key, and collateral are held by the node operator (not the Platform identity DET manages) and remain intentionally absent — no new UI added for them. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WVUVpGxaAsciGSiT7t4kuC
…rofile The "Just Explore" onboarding path defaulted its landing screen to RootScreenDashPayProfile. With the standalone DashPay nav tab now hidden, that screen is orphaned — a user who explores and then navigates away has no nav entry to return to it. Land on RootScreenIdentityHub, the single user-facing "Identities" nav entry, instead. Adds a kittest that boots onboarding, clicks "Just Explore", and asserts the app dismisses the welcome screen and lands on the Identities hub. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WVUVpGxaAsciGSiT7t4kuC
lklimek
marked this pull request as ready for review
July 13, 2026 19:16
Collaborator
|
⛔ Blockers found — Sonnet deferred (commit e4d29b4) |
The node detail view's DPNS voting section showed a bare contest label (`det`) with no framing, and a `Cast votes` button that is disabled until a choice is picked but offered no hint — so a masternode owner clicked an inert-looking button and nothing happened. - Frame the section: an intro line explaining a name is being contested, the full `.dash` domain per contest, a status line with contestant count and voting deadline, and each candidate's running tally. - Nudge under any contest with no pick, so the disabled state is explained. - Add enabled/disabled hover tooltips to `Cast votes` telling the user what unlocks it. Pure display helpers (`contest_display_name`, `candidate_choice_label`, `contest_status_line`) are unit-tested. No backend change — the dispatch was already correct; the button was simply disabled with no affordance. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
An unreadable legacy identity permanently hid an unreadable legacy vote. The identity import withholds its sentinel while any row fails to decode, so `identities.unreadable > 0` recurs on every launch — and that branch returned early, ahead of the durable `read_vote_warning` re-publish. The app-data pass, meanwhile, short-circuits on its own sentinel from the second launch on and honestly reports zero unreadable votes, so taking the count from its counters could not have rescued the vote half either. Net effect: a user with one corrupt identity row and one corrupt vote row was never told about the vote — on any launch — and could miss a live deadline. The identity branch now reads the durable vote warning from storage and publishes both counts on one terminal state, `SucceededWithUnreadableIdentitiesAndVotes`, rendered as a single sticky Warning banner naming both remedies. Acknowledging retires only the vote half; the identity half keeps arriving until a build with a fixed decoder imports the rows. A k/v read that itself fails is surfaced as the retryable combined failure rather than dropping either signal. Adds IDN-016 (identities and keys preserved across an app upgrade), the user story CLAUDE.md requires for this PR's user-facing migration behavior. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The `Roles:` row on the node detail screen showed bare `V O P` letters with
a single legend tooltip on the `Roles:` label. A user hovering the letter
they actually care about got nothing, and the legend only expanded the
letters — it never said what the keys do.
Each letter now carries the DIP-3 ProTx role wording already used by the
"Manage keys" buttons directly below it and by the load form
(TIP_VOTING_KEY / TIP_OWNER_KEY / TIP_PAYOUT_KEY), so hovering `V` explains
the voting key. Absent roles render as `·` and keep their tooltip, so the
user can see what a missing key would have done.
To keep the letters and their meanings from drifting apart, `key_status_tokens`
moves from `card.rs` to the `masternodes` module and now returns a
`KeyRoleToken { letter, tooltip, present }` — one source of truth for the list
card and the detail screen. The card intentionally does not attach per-letter
tooltips: the whole card is a click target, and a Help cursor inside it would
fight its PointingHand affordance.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…ding The "Add masternode" load form closed itself the instant Load was clicked (render_load_view switched to the List view before the async load resolved), so a validation/network error dropped the user back to an empty form and forced them to retype every field. build_input also drained the key secrets via take_secret, so the values were gone even if the form had stayed open. Now: - Submit keeps the Load view open and sets load_in_flight; the submit button locks with a spinner while the load runs (no double-submit). - On success, display_task_result closes the form and returns to the list where the new node's card appears. - On error, display_task_error clears the in-flight gate so submit re-enables with every field intact — the user fixes one field and resubmits, no full re-entry. - build_input clones the secret fields instead of draining them, so the form retains all values for an in-place retry; the form's copies zeroize on drop when it closes on success. Errors already surface as typed TaskError variants via the global banner; no string parsing involved. Tests: load_form field-preservation unit test; list_screen lifecycle test asserting error keeps the form open + re-enables submit and success closes it. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Hiding the standalone Identities and Dashpay nav entries left `RootScreenIdentities` with no nav button pointing at it. The "Just Explore" onboarding landing was repointed at the hub for exactly that reason (965f35d), but two other fallbacks still dropped the user on the de-navigated screen: - live de-gating, when the role drops below Power while the Masternodes tab is active (`active_root_screen_mut`); - an unregistered persisted root screen at startup (`AppState::new`). Both stranded the user on a screen with no nav entry highlighted and no way back — the same dead end the onboarding fix closed. Name the target once, `FALLBACK_ROOT_SCREEN`, and point both sites at it, so a future fallback cannot silently pick a different, orphaned screen. A unit test locks the invariant that actually matters — the fallback screen must have a nav entry, and an ungated one, or the fallback is itself filtered out of the rail at the very role that triggered it. The de-gating kittest now asserts the hub. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
… legacy maps
The prior audit called DIP-17 platform-payment addresses a hard blocker: it
checked upstream's `all_accounts()`, found no platform-payment pool, and
concluded the snapshot structurally could not carry them. That checked one
accessor, not the crate.
`ManagedAccountCollection` keys platform-payment accounts separately (they hold
credits, not Core UTXOs, so they are not a `ManagedAccountRef`) and exposes them
through a *different* public accessor — `all_platform_accounts()`
(key-wallet `managed_account_collection.rs:1001`), backed by the `pub`
`platform_payment_accounts` field. Their `AddressPool` carries the same
`AddressInfo { address, path }` entries every other pool does, and
`WalletAccountCreationOptions::Default` — the option DET registers every wallet
with — creates account (0,0) unconditionally (`wallet/helper.rs:139`). The data
was reachable all along.
So:
- `address_paths_from_info` now walks `all_platform_accounts()` alongside
`all_accounts()`, and the snapshot carries DIP-17 paths;
- the send-autocomplete sources BOTH its Core and Platform entries from the
snapshot's `address_paths` and no longer reads `known_addresses` /
`watched_addresses` at all.
This is a funds-safety improvement, not just a cleanup. DET's own bootstrap
derives addresses independently of upstream; anything it derived past the gap
limit (or rehydrated stale) could be offered as a send/receive target that SPV
never watches. The snapshot is upstream's actual generated set, so the
autocomplete can now only ever offer an address the wallet really owns.
Retirement of the maps themselves stays blocked, on ONE gap rather than three:
identity *authentication* keys (DIP-13/15). key-wallet can derive that path
(`DerivationPath::identity_authentication_path`) but has no `AccountType`, no
`ManagedAccountType`, and no pool that tracks the resulting addresses, so they
cannot reach any snapshot. The identity-key resolver, `system_tab_sections`, and
the address-table union all still depend on the maps for exactly that class.
The third alleged blocker — `path_reference` / `DerivationPathType` metadata —
is not one: `categorize_account_path` already treats path shape as authoritative
over the stored reference, so DET recomputes it.
Tests: the platform-pool path assertion is confirmed RED against the
`all_accounts()`-only source and green after; the autocomplete gains a
funds-safety test pinning that a fully-populated legacy map yields no entries
when the snapshot is empty. The old canary asserting platform addresses never
reach the generated-path set is replaced — its tripwire fired by design.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…y test The registry-token-threading fix (d038d1a) added a required `Option<IdentityLoadToken>` parameter to begin_identity_load. The secret-residency fix (8baa6f4) added this test on an independent branch before that signature landed, so cherry-picking both together left a stale 1-arg call. None matches the documented semantics for a load claimed without a prior submission (see begin_identity_load's own doc comment) — this test claims the load directly, not through the submit-then-dispatch path. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…hed token The previous fixup (b3a2fc6) passed None to begin_identity_load, but the test submits through apply_load_outcome first, which mints a real token via mark_identity_load_submitted and stores it in screen.pending_load. Claiming with None instead of that token made begin_identity_load see an existing Submitted record whose token doesn't match, so it fell through to the IdentityLoadInProgress arm and the test failed for real (not the compile error the first fixup addressed). Use the existing claim_dispatched_load test helper, which reads screen.pending_load and claims under its actual token — this is the same pattern the neighboring test on the same file already uses for a dispatched load. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
EventBridge::nudge_refresh() sent TaskResult::Refresh on every SPV sync-progress tick (many times a second while syncing), which routed into IdentityHubScreen::refresh() and unconditionally wiped the async DashPay profile cache, contacts state, and search buffers. Because the profile-load round trip can't keep up with the tick rate, the Home tab's display name flipped between loaded and reset dozens of times a second. The frame-loop nudge was already redundant with the repaint: SenderAsync / SenderSync request a repaint on every send regardless of payload. Route ambient sync ticks through a new TaskResult::Repaint (a no-op result — the repaint already happened) instead of Refresh, so they no longer clear per-screen caches. Explicit low-frequency Refresh producers (vote cast, DPNS re-query, token balance refresh) are unchanged. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…balance Two live-QA UX gaps: the Masternodes card grid had no defined order (whatever the store returned), and a node's Platform balance was only visible on the Withdraw Funds screen — users had no way to check a masternode's balance without leaving the tab. - MasternodesScreen::reload() now sorts nodes case-insensitively by the same display name used as the card heading (alias, else shortened ProTxHash), reusing card::card_heading so the grid and the page-nav node pill share one ordering. - MasternodeCard gains an optional with_balance_credits() builder; balance renders in bold monospace under the heading, formatted with the existing format_credits_as_dash (same formatting as Withdraw Funds, no new format invented). - The node detail header gains a "Balance: <amount>" row next to the ProTxHash/type badge, so the balance is visible immediately on open. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…, decode limit, wallet naming (#891) * fix(identity): bound the bincode decode so a corrupt blob errors, not aborts QualifiedIdentity::from_bytes decoded legacy identity blobs under bincode::config::standard(), which resolves to NoLimit. A length prefix claiming an inflated element count (an ordinary bit-flip or truncation, no attacker required) makes bincode pre-allocate the claimed size before reading anything; when that exceeds available memory the allocator aborts the process (SIGABRT, uncatchable, not a Result::Err). Live-reproduced during PR #885's grumpy-review: a minimal probe encoding a 1 TiB length prefix aborted with exit 134. This defeated the legacy-identity migration's own stated contract ("one bad blob never blocks the identities around it") on exactly the corruption class ordinary disk bit-rot produces, crash-looping the app on every cold start until the user manually repaired data.db. Fix: decode under a bounded Limit (16 MiB, far above any real QualifiedIdentity) via a shared identity_blob_decode_config() function used by both from_bytes and its regression test, so a future edit that weakens the limit is caught rather than silently diverging from what the test actually pins. With a Limit, bincode checks the claimed size against the cap before allocating and returns DecodeError::LimitExceeded -- a normal Err the existing skip-if-present machinery already handles. RED-first: temporarily reverted the config to unbounded and confirmed the new regression test aborts the test process with the exact same "memory allocation of 1099511627776 bytes failed" / SIGABRT signature from the review's live repro, before restoring the fix and confirming green. Full workspace suite (1675 lib tests + kittest/doctests), clippy --all-features --all-targets -D warnings, and cargo +nightly fmt --check all pass clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(wallet): name the wallet in the "still loading" error `TaskError::WalletNotLoaded` was a bare unit variant: with several wallets loaded, neither the user nor a developer reading logs could tell which wallet was still loading. It now carries a `wallet_label` — the alias, or a truncated seed-hash hex when the wallet was never named — and the message names it. Both construction sites (`resolve_wallet`, `monitored_receive_addresses`) resolve the label from the wallet-meta sidecar: the wallet is by definition missing from `id_map` there, so there is no live handle to ask. The `id_map` read guard is released before that sidecar read. The alias-or-hex rule was inlined in `wallet_from_envelope` (`SeedLengthInvalid`); it moves to `model::wallet::meta::wallet_label` as the single source of truth for both errors, output unchanged. * docs(migration): realign the legacy-identity design doc with the shipped code The doc was written before a ten-commit iteration and only partly updated afterwards, so three sections described an implementation HEAD never had. - §5: the sketch unwrapped `app_data` before running the identity import, the exact inverse of HEAD. Both DET-owned results are *held* and judged after the drain, because an app-data failure is deterministic: unwrapping it first would skip the identity import on this launch and on every retry, stranding a masternode owner's keys over a corrupt vote queue. Transcribed HEAD's held-then-judged flow, including the per-arm terminal states. - §9 T-ID-01: `LegacyIdentityRow` has no `status` field (the reader folds status and alias straight onto `qi`), and the SQL selects a sixth column, `alias` — the column, not the blob's stale copy, is authoritative. - §10: assertion 9 cited `second_launch_after_a_v093_upgrade_changes_nothing` as proof of skip-if-present, which that test cannot carry — on the clean path the sentinel short-circuits the pass before the check is reached, so it would pass against an importer with no such rule at all. Moved to `a_retry_after_an_unreadable_identity_preserves_user_edits`, where the sentinel is deliberately withheld, and said why. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(migration): stop reporting a readable app-data pass as a failed one `FailedWithUnreadableIdentities` had two producers, and one of them lied. Path 1 — the app-data pass hard-fails alongside undecodable identities — is what the state and its banner describe: "updating the rest of your previous data did not finish… Choose Retry now to finish updating." True, and the retry works, because the app-data sentinel is unwritten. Path 2 — the app-data pass SUCCEEDS and writes its sentinel, and only the follow-up unreadable-vote-warning k/v read fails — published the same state. The user was told a pass that had completed did not finish, and offered a retry that re-runs nothing: on the retry the app-data pass short-circuits on its own sentinel and the same read fails again, so the false error banner returns on every launch. Fall through to the honest `SucceededWithUnreadableIdentities` instead, and log the read failure with its typed error. Nothing is swallowed: the warning record is durable, and this branch re-runs on every launch while the identity sentinel stays unwritten, so the next successful read re-publishes the vote half. The identity signal — the one the user must act on — reaches them either way. Reusing the existing variant over adding a new one keeps the reconciler and the shielded indicator untouched (both already map this state and the old one to the same badge). Regression test `an_unreadable_vote_warning_record_does_not_claim_the_app_data _pass_failed` poisons the warning record with a zero-length bincode body, so the read fails deterministically while the app-data pass runs clean; it asserts the honest state AND that the app-data sentinel is written — the very fact the old banner denied. Confirmed RED before the fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(migration): add the identity-banner kittests the rustdoc promised Three banner-copy functions in `app.rs` close their rustdoc with "Exposed for kittest coverage", which is the only thing justifying their `pub` — yet `tests/kittest/` referenced none of them. The promise now holds: - `unreadable_identities_banner_warns_without_a_retry_action` - `unreadable_identities_and_votes_banner_names_both_and_acknowledges` - `failed_with_unreadable_identities_banner_offers_a_working_retry` Each asserts the copy renders verbatim and that the action set matches the outcome: no retry for the two Warning states (the rows are still in the previous version's storage and decode no better on a second pass), a working "Retry now" for the one genuine failure, and the vote acknowledgement on the combined warning so a live deadline cannot be buried by the recurring identity signal. Also adds the missing `MigrationStep::Identities` to `tc_mig_014_running_text_covers_every_step_with_sentence`, which claimed to cover every step while omitting the one this feature added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(migration): stop the identity import from resurrecting deleted identities The identity pass wrote its completion sentinel only when every legacy row decoded. A genuinely corrupt row never decodes, so the sentinel was never written and the import re-ran on every cold start, forever. Skip-if-present made that harmless for an identity the user had edited, and did nothing for one the user had deleted: the next launch re-imported it, restored the alias the user had cleared, and re-wrote its legacy plaintext keys into the vault — with no banner to explain it and no way to stop it short of editing data.db. Write the sentinel unconditionally, exactly as the sibling app-data pass already does and for the reason it documents. The import becomes a once-only event, so a deletion is durable. The undecodable rows stay in data.db (never deleted) and are carried forward by a durable UnreadableIdentitiesWarning record instead of by an import that retries until it decodes; recovering them after a decoder fix is an explicit user gesture (#889), not an automatic retry that costs a deletion. The durable record is what makes that safe: with the sentinel written the pass short-circuits and reports zero unreadable rows on every later launch, so the banner is now published from storage rather than from pass counters. That record also closes the second hole: the unreadable-identity banner was sticky with no action button, so the user could be told their signing keys had not come across and given no way to say "I understand". It now carries a "Got it" action wired to a new AcknowledgeUnreadableIdentities task, mirroring the vote flow. Acknowledgement deliberately does NOT double as the sentinel-writer — hanging the loop-break on a user gesture would leave the resurrection bug live for anyone who never clicks. The combined banner names both problems, so its single acknowledgement retires both records. Tests (both confirmed RED against the unfixed code): - a_deleted_identity_is_not_resurrected_by_an_unreadable_sibling_row - unreadable_identity_warning_is_republished_until_acknowledged - a_second_launch_after_an_unreadable_identity_preserves_user_edits_and_deletions (rewritten: proves rename AND deletion survive on a real v0.9.3 database) - reconciler + kittest coverage that the banner offers the acknowledgement and routes it to the right task Two existing assertions demanded the withheld sentinel — the defect itself — and were flipped to the corrected contract. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…te behind Expert mode The status dot/text on masternode cards and the detail screen read as Core masternode network health, but it actually reflects whether the node's Platform identity currently resolves (IdentityStatus). Relabel to "Platform identity: <status>" with a clarifying tooltip, and hide it entirely below Power/Developer role — everyday users no longer see a misleading always-on indicator. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…issue #889 The doc still introduced itself as "design, ready for implementation" against a pre-implementation base commit, and its closing sections read as open questions, long after PR #885 shipped every task in §9 and PR #891 landed the QA follow-ups. - Header states the real status (shipped in #885, follow-ups in #891) and surfaces the known limitation up front. - The §7 known-limitation follow-up now names issue #889 directly instead of pointing at "the GitHub issue referenced from PR #885". - §11 (bincode feasibility spike) is framed as settled, pointing at the golden blob it produced (T-ID-06 / V093_MASTERNODE_BLOB_HEX). - §12 is relabelled as historical design-review findings; the rationale table is kept, but it no longer masquerades as a live defect list.
bincode is flagged unmaintained. The advisory is INFO-level and covers every version, so no bump can clear it: 2.0.1 is the last functional release and 3.0.0 is a tombstone whose lib.rs is a bare `compile_error!`. bincode 1.3.3 also arrives transitively, so dropping the direct dependency would not silence it either. The encoder writes on-disk wallet-secret envelopes and QualifiedIdentity blobs, so swapping it changes the wire format of data users already hold. Record the risk acceptance at the pin so the next reviewer does not re-flag it, and so nobody "fixes" the warning with a bump that would be data-loss-class. No version or lockfile change.
The shielded badge maps every post-drain terminal state — including FailedWithUnreadableIdentities — to Verified, and that mapping is right: those states only fail passes (app data, identity rows) that run after the wallet drain and never touch shielded storage, so the balance is as authoritative as on Success. Downgrading it would lock shielded spends over a corrupt vote row and claim a shielded failure that never happened. What was wrong is the copy. "Verified." took its subject from its position under the balance, so beside the red migration error banner it read as a blanket "all good" — and handed a translator an adjective with no noun to agree with, against the project i18n rule. It now names its subject: "Shielded balance verified." Also covers the three migration states the exhaustiveness test had missed (SucceededWithUnreadableIdentities, SucceededWithUnreadableIdentitiesAndVotes, FailedWithUnreadableIdentities) and records why the green badge under an error banner is deliberate.
…est_helpers The v0.9.3 identity fixture — table DDL, encodable blob, row INSERT — was rebuilt in three modules, so a column added to the legacy shape had to be chased through all of them. It now lives once in database::test_helpers, next to the legacy wallet and scheduled-vote fixtures already shared from there: create_legacy_identity_table, basic_legacy_identity_blob, and a LegacyIdentityFixture builder that states only what a test varies. Deliberately not merged, because they are not the same fixture: - v093_upgrade keeps its verbatim v0.9.3 whole-database DDL (its both-or- neither wallet CHECK is the point of that module) and its keyed blob builder; only its row INSERT now routes through the shared builder. - The shared DDL omits that CHECK on purpose — the import must survive a half-filled wallet link, and no test could stage one if SQLite rejected it. - The minimal (id, network) identity table used by the top-up/vote scoping tests is a different shape and stays where it is. Also folds the thrice-copied corrupt-row insert in finish_unwire's async tests into one local helper, and types the fixture's status as IdentityStatus, which retires v093's raw u8 status arguments (the consts stay as the on-disk assertions they always were, now including Active).
The rustdoc promised that nothing about "the decoded identity" is ever logged because it carries private keys, while the warn branches log the identity's id. The code is right — an identity id is a public, on-chain handle, and it is what lets a user tell which identity did not come across; the blob and its decoded key material are never logged. Only the promise was imprecise, so it now draws that line explicitly instead of over-claiming.
… banners "Load these identities again" named neither a screen nor a control, so an Everyday User who has never opened that flow had no way to act on it — the repo's error-message rules require a concrete, self-serviceable action. All three variants now point at Load Identity on the Identities screen, mirroring how the vote copy already names the Scheduled Votes screen. The kittest asserting every variant names both is the regression net: it fails against the old copy.
`migrate_app_data` propagated the `get_scheduled_votes()` error raw, so a k/v read failure left `run()` returning a `TaskError` that was not `MigrationFailed`. `run_migration_task` published `MigrationState::Failed` only for that one variant, so such an error published nothing and stranded the status on `Running` — where `run_backend_task` rejects every wallet-touching task with `WalletStorageNotReady` and the banner offers no retry. That wedges wallets, identities and sends until the app is restarted. Type the app-data read into `MigrationError::AppDataImport`, and make the publish total: `migration_error_chain` coerces any `TaskError` into the typed `Arc<MigrationError>` chain (a stray error wraps in the new `Unexpected` variant), so no error can skip the terminal state. Also from the same review round: - Drop the `wallet_known` closure seam from `migrate_identities_from_conn`: it could not change behaviour, only gate a `tracing::warn!`. The diagnostic moves to the caller's insert closure, which already holds the backend, so the `WalletBackendUnavailable` gate is unaffected. - Collapse `write_sentinel` into `write_completion_sentinel`, now the sole writer of `MigrationCompletion`, with `network_count` as a parameter. - `run()`'s "No pass gates another" was imprecise: the two DET-owned passes do not gate each other, but the wallet drain is a deliberate prerequisite for the identity import. Say so. - Document why the identity check-and-insert needs no transaction: the migration gate serialises every production identity writer.
…into fix/qa-followups-885
Legacy-data migration completed while password-protected wallets were still locked: `register_migrated_wallets()` excluded locked wallets from the completion check, so `bootstrap_wallet_addresses_jit` never ran for them, their `id_map` entry stayed empty, and every later operation on that wallet failed with `TaskError::WalletNotLoaded`. Migration now blocks on a non-dismissible password prompt for each locked wallet. A new `MigrationState::AwaitingWalletPasswords` carries the pending seed hashes; the frame loop renders the prompt via the existing `WalletUnlockPopup` and drives the entered password through `handle_wallet_unlocked`, which promotes the seed and bootstraps the wallet. A `tokio::sync::Notify` handshake wakes the migration task, which re-checks for locked wallets and only then writes the completion sentinel. Protected seeds are re-encrypted under the current envelope (`SecretScheme::Protected`) rather than left `Absent`. `passphrase_modal` gains a `cancellable` flag so the migration prompt suppresses Cancel, Escape, click-outside and the title-bar close button. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
…migration-password-prompt
The card status text was relabelled to "Platform identity: <status>" so the dot cannot be misread as Core or PoSe health, but the masternode-tab kittest still asserted the bare "Pending Creation" label and went red. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
… migration" This reverts commit c6176f4. Review of the combined PR surfaced four blocking defects in the prompt, so the feature is withdrawn from this PR and reworked in a dedicated one: - The migration awaits a `tokio::sync::Notify` whose only producer is the egui frame loop, but `mcp/resolve.rs` drives the same migration with no frame loop — headless `det-cli`/MCP hangs forever on an install with a password-protected legacy wallet. - A forgotten password wedges the app on every launch, and the only exit the UI permits is `remove_wallet` (called outside the `BackendTask` gate) — deleting the wallet and its seed. - Migration spuriously reports `RegistrationIncomplete` even when the correct password is supplied: unlock spawns a fire-and-forget registration while the migration task inline-awaits its own for the same wallet. - The "non-dismissible" modal does not actually block input; a click reaches widgets behind it. The rest of this PR's QA follow-ups are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
…tity status `show_platform_identity_status()` gated the status row behind the Power role, but the Masternodes tab is already Power-gated by the nav rail: `app.rs` routes the root screen away below Power, so no card or detail view is reachable at a lower role. Both callsites always received `true`, and the unit test asserting the false branch covered an unreachable path. Remove the gate, its bool parameter on `MasternodeCard::show`, and the dead test. Role gating stays where it belongs — at the nav rail, as the single source of truth. The explicit "Platform identity: <status>" label and its clarifying tooltip are unchanged; they were the point of the change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
thepastaclaw
suggested changes
Jul 14, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Preliminary review — Codex only
HEAD matches e4d29b4. Three in-scope blocking defects remain: two carried forward from the prior review, plus a latest-delta recovery message that directs unreadable masternode and evonode identities to a User-only loader. The address-table caching proposal is dropped as an unmeasured optimization rather than a demonstrated regression; verification was source-based under read-only permissions.
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— general (completed),gpt-5.6-sol— rust-quality (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 3 blocking
3 additional finding(s) omitted (not in diff).
🤖 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/send_screen.rs`:
- [BLOCKING] src/ui/wallets/send_screen.rs:2168-2199: Refresh autocomplete when wallet snapshots change
AddressInput now obtains every Core and Platform suggestion from snapshot_address_paths(), but build_address_input() clones those paths only when the lazily initialized widget is created. SnapshotStore::register_wallet() does not publish an initial snapshot; the first paths arrive when a later wallet event calls recompute(). If the send form renders before that event, it captures an empty map. TaskResult::Repaint only redraws the existing widget, WalletSendScreen::refresh() is empty, and the screen never calls AddressInput::set_wallets(), so the published addresses remain unavailable until an unrelated source change invalidates the widget or the screen is reopened. Refresh the initialized component when the snapshot generation changes, or invalidate it when wallet events publish new paths.
In `src/backend_task/migration/finish_unwire.rs`:
- [BLOCKING] src/backend_task/migration/finish_unwire.rs:870-871: Clear an obsolete identity warning after a successful retry
migrate_identities() writes the durable unreadable-identity warning before its completion sentinel. If the process terminates between those writes, the import runs again on the next launch. A retry that can now decode every row—for example, after upgrading to a corrected decoder—passes zero here, but the previous nonzero warning remains in storage before the sentinel is written. run() then republishes that obsolete warning indefinitely, claiming identities and signing keys were not carried across even though the retry recovered them. A zero-count retry must remove any warning left by an interrupted pass.
In `src/app.rs`:
- [BLOCKING] src/app.rs:195-196: Direct unreadable masternodes to their actual loader
The migration warning aggregates every unreadable legacy identity row, including Masternode and Evonode records, but it now directs all affected users to Load Identity on the Identities screen. That flow hard-sets IdentityType::User and its advanced selector offers only User identities; masternodes and evonodes must be restored through + Load on the Masternodes screen. The combined and failed-warning variants repeat the same incorrect direction at lines 211 and 226-227. Update all three messages to explain both recovery routes because the stored warning count does not retain the affected identity types.
Base automatically changed from
feat/legacy-identity-migration
to
feat/v1.0-parity-batch
July 14, 2026 14:01
lklimek
added a commit
that referenced
this pull request
Jul 16, 2026
* fix(migration): prompt for wallet passwords before completing migration
Restores the migration password prompt reverted from PR #887 so it can be
reworked in isolation. This commit is the original implementation verbatim;
the review findings that caused the revert are fixed in the commits that
follow.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
* fix(migration): free the password prompt from the SPV overlay, add a skip
Two defects found by real-world testing of the migration password prompt.
The SPV progress overlay and the passphrase modal both painted at
`egui::Order::Foreground`. Suppressing only `ProgressOverlay::claim_input`
released the keyboard but left the overlay's pointer sink and dim/card layers
live, so they swallowed clicks aimed at the password field — the prompt was
visible but unusable whenever migration ran alongside an SPV sync (which is
always, at boot). A blocking secret prompt now owns the whole interaction
surface: while one is active the overlay stays logically in its stack but
paints no dimmer, pointer sink, card, or focus trap, and claims no keyboard.
Queued ordinary secret prompts are promoted before the frame's overlay
decision, so their first visible frame is protected too.
The prompt was also inescapable: a user who had forgotten a wallet password
could not proceed, and the only exit the UI permitted was deleting the wallet.
"Skip this wallet" now records the seed hash in a per-run exclusion set, drops
it from the published pending list, and wakes the migration task — so skipping
the last wallet still completes the migration and writes the sentinel. A
skipped wallet stays closed, keeps its legacy protected envelope, and is
registered upstream on a later ordinary unlock via the existing
`handle_wallet_unlocked` -> `bootstrap_wallet_addresses_jit` chokepoint.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
* wip(migration): headless fail-fast, legacy read-only, registration single-flight
INCOMPLETE — DO NOT MERGE. Committed to preserve work across a session
restart; the Codex job producing it was cancelled mid-edit.
State: compiles clean (clippy --all-features --all-targets -D warnings, exit 0),
but the full test gate is RED (exit 101, 1771 passed / 2 failed):
context::wallet_lifecycle::tests::migrated_protected_wallet_blocks_migration_until_password_submission
context::wallet_lifecycle::tests::protected_wallet_registers_upstream_on_unlock_without_restart
Both are tests the legacy-read-only and registration-race changes must rewrite;
the job was cancelled partway through that rewrite. Whoever picks this up must
finish those two and re-run the full gate before trusting any of it.
Intended scope (per review findings + owner directives):
- P1 headless fail-fast: migration must refuse, not block, when a protected
wallet needs a password and no interactive prompt exists (mcp/resolve.rs
drives the same migration with no egui frame loop -> det-cli hung forever).
- P2 legacy DB strictly read-only: never DROP/DELETE/UPDATE the pre-migration
database; write only the new store/vault. Makes a skipped or abandoned
migration cost the user nothing.
- P3 registration race: single-flight per wallet (unlock spawned a
fire-and-forget registration while migration inline-awaited its own for the
same wallet -> spurious RegistrationIncomplete, reproduced as a real failure).
- P4 split MigrationState::is_running(), which silently came to mean
"running OR blocked on a human"; five callers inherited the conflation.
- P5 cross-wallet password bleed (modal state keyed on window title), swallowed
re-encryption failure, unified lock-poisoning policy.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
* fix(shielded): gate fund-moving shielded tasks at the backend chokepoint
`run_shielded_task` had no capability check, so the five state-changing
shielded operations were reachable from any caller that dispatches a
`ShieldedTask` directly. The MCP shielded tools do exactly that, bypassing
the UI gate at `ui/wallets/shielded_tab.rs`. Shielded operations are not
defined on any current network, so `ShieldFromAssetLock` would create an
asset lock committing real L1 funds and then attempt a state transition no
network can settle — stranding the funds and burning the fee.
Enforce `FeatureGate::ShieldedOperations` as the first statement of
`run_shielded_task`, before any wallet or backend access, mirroring the
`RootKeyDerivationRefused` guard in `backend_task/wallet/mod.rs`. The UI
gate stays as defense in depth.
Scope is exactly the five fund movers: `ShieldedTask` carries only
write variants. Shielded init, sync, balance and address reads reach the
coordinator through their own paths and stay ungated, so shielded funds
remain viewable wherever the wallet runs.
Add `TaskError::ShieldedOperationsUnavailable` and a regression test that
dispatches a write task the way an MCP tool does; it is confirmed failing
without the guard, proving the refusal precedes backend access.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(dashpay): stop erasing accepted accounts, double-submits and silent declines
Three independent defects in the DashPay contact flow.
Contact-info write silently erased the accepted-account allow-list.
`resolve_accepted_accounts` collapsed four distinct states — no document,
missing privateData, decrypt failure, deserialize failure — into an empty
Vec, which `create_or_update_contact_info` then re-encrypted and wrote back
over the live Platform document. Any present-but-unreadable payload (e.g. a
contact whose privateData was written by another DashPay client) lost its
allow-list irreversibly on the next rename or unhide. Only an absent document
now yields an empty list; a present payload that cannot be read aborts the
write with a typed `DashPayContactInfoRead` error. The test that asserted the
data-losing behaviour is inverted, and the missing/undecodable payload states
get their own regressions.
A failed task released every request guard, allowing a paid double-submit.
`display_task_error` cleared all Accept/Decline/Cancel guards on any error, so
an unrelated concurrent failure re-enabled an in-flight Accept and a second
click bought a second state transition. Failures from the three request actions
now carry their request ID in `DashPayContactRequestActionFailed`, so only the
guard named by the error is released. Guards no longer matched by a result
expire on a timeout instead of being cleared wholesale, so a lost result cannot
strand a row forever.
A declined request reappeared after refresh. `reject_contact_request` logged and
swallowed a failed `dashpay_mark_declined` write and still reported success,
even though that local marker is the only thing that retires the row — Platform
keeps the `contactRequest` document forever. The failure now propagates, matching
the sibling `mark_withdrawn` cancel path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallet): report a corrupted wallet envelope as damage, not a wrong password
A password-protected wallet whose at-rest envelope is corrupted (truncated or
otherwise wrong-length) failed the AES-GCM tag check and surfaced as "The
password is incorrect", trapping the user in a retry loop whose only escape was
deleting the wallet. Structural damage is now classified before the AEAD can
mistake it for a bad password.
- decrypt_message takes the caller's known plaintext length and rejects an
impossible ciphertext/tag or salt length as DecryptError::Malformed.
- WalletSeed::open returns the typed EncryptionError instead of a flattened
String, so callers branch on the variant rather than on message text.
- The unlock popup maps Malformed to the same "saved data looks damaged, re-add
it from your recovery phrase" sentence the unprotected path already shows, and
keeps the password hint on the wrong-password branch only.
Finish the two migration lifecycle tests left red at the previous checkpoint.
Both now install a TestPrompt::never(), which panics if asked and so pins the
contract that migration defers to the UI-owned unlock flow instead of driving a
secret prompt itself:
- the protected wallet waits, is then skipped, and data.db is asserted
byte-unchanged, holding the legacy database strictly read-only;
- the unlock path joins the migration's single registration flight
(registration_attempt_count() == 1).
Verified green on the full workspace suite (2023 passed, 0 failed), the
all-features/all-targets lint gate with warnings denied, and the nightly
formatter check.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(dashpay): preserve saved contact details and keep paid actions guarded
The contactInfo document is written whole, so every writer decides the fate
of the fields it does not edit. Decline, withdraw, unhide and rename each
rebuilt the payload from scratch, erasing the nickname, note and
accepted-account list stored by the user or by another DashPay client.
Replace the implicit `Vec<u32> -> AcceptedAccounts::Replace` coercion, which
made the destructive path the short one, with an explicit `ContactInfoUpdate`
that states field by field what is preserved and what is replaced. Visibility
flips now preserve everything else; only the contact-details form, which owns
the whole form, replaces.
A payload this client cannot read is no longer either silently overwritten or
a permanent dead-end: the write aborts, the user is told, and confirming an
explicit, danger-styled dialog re-runs the write with an overwrite policy, so
a contact with unreadable details can still be unhidden, declined or renamed.
The v0 parser now rejects unknown versions, invalid UTF-8, non-canonical
flags and foreign trailing bytes instead of decoding them as absent details.
Paid request actions (Accept, Decline, Cancel) keep their in-flight guard
across a routine tab switch or refresh, which previously released it and made
the row clickable again while its state transition was still running. An
identity, wallet or network change still clears the guards, since they belong
to the identity being left. Task results reach only the screen that is visible
when they land, so the wall-clock backstop is retained: without it, an action
resolved while the user was on another screen would strand its row with dead
buttons for the rest of the session.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallet): unlock a cold-booted protected wallet with its correct password
A password-protected wallet hydrates from a secret-free model: the
Tier-2/Protected arm of cold-boot reconstruction carries a placeholder
envelope, because the real secret stays in the vault. The unlock popup
verified the password against that placeholder, so after the first
restart the CORRECT password was reported as damaged data and the owner
was permanently locked out of the wallet.
Verify the password only through the secret chokepoint, which reads the
real stored envelope, and flip the in-memory seed open solely after that
succeeds (`mark_open_after_verification`). The popup maps the resulting
typed error to user copy structurally — wrong password vs damaged vault —
instead of pre-checking the model.
Operation-only unlocks now forget the session seed through an RAII guard,
so an early return or panic in the reconciliation subtask can no longer
strand a plaintext seed in the cache. A migration unlock is operation-only
too: that prompt offers no "keep unlocked" choice, so it must not silently
retain the seed for the session.
Regression cover, both entry points against a real cold boot: the context
API and the unlock popup itself. The popup test fails (correct password →
Pending) if the model pre-check is ever reintroduced.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui): block input outside a non-dismissible modal prompt
Removing the progress overlay's pointer sink (so it could not cover a
secret prompt it had triggered) also removed the only barrier in front of
the app: while the storage update paused on the migration password prompt,
clicks still reached the wallet screen behind it.
Give the modal its own barrier instead. A non-dismissible `modal_chrome`
window installs a full-screen pointer sink and registers itself as egui's
modal layer, so every layer beneath it is ignored for interaction while
the window itself — drawn above the sink — stays fully interactive.
Dismissible dialogs keep their existing click-outside behaviour.
The kittest asserts the widget beneath the prompt does NOT register a
click, and that the prompt's own controls remain hittable.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(migration): keep wallet work gated while the storage update awaits a password
`is_running()` had become an alias for `is_executing()`, which reports
`false` while the migration is paused on `AwaitingWalletPasswords`. Every
caller that meant "the storage update has not finished yet" therefore
opened up mid-migration: wallet-touching backend tasks slipped past the
`WalletStorageNotReady` gate and hit a half-migrated vault, the MCP
wait/join logic stopped waiting, and the wallets screen offered
Create/Import CTAs against a wallet list about to be rehydrated.
Replace it with `is_in_progress()` — `Running | AwaitingWalletPasswords` —
and use it at all three sites. `is_executing()` keeps its narrow meaning
for callers that really do mean "a step is running right now".
Covered by a test dispatching an MCP-style wallet task during
`AwaitingWalletPasswords` and asserting `WalletStorageNotReady`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallet): collect the redundant legacy seed envelope and stop overpromising data removal
Legacy seed-envelope garbage collection, restored for the vault copy only.
Once the current-format secret is durable — the raw seam, a Tier-2 sealed
envelope, or an eager/lazy migration write — the superseded `envelope.v1`
row in the SAME vault is deleted best-effort, so a seed has exactly one
current copy at rest instead of an indefinitely retained duplicate. A
cold-boot scheme probe repeats the sweep after an interrupted run. The
pre-update `data.db` is NOT touched: it stays a read-only recovery
artifact.
Stop promising deletions the app no longer performs. "Remove Wallet" and
"Clear Database" said they erase all local data, while an earlier
version's read-only recovery database — which may still hold wallet
recovery data — stays on disk; the copy now says so. "Clear Platform
Addresses" is disabled rather than pretending to work: its only
implementation wrote to that read-only database.
The two remaining legacy-database writers are signposted as test-only;
neither has a production caller.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui): make every passphrase prompt own the interaction surface
The blocking progress overlay yields to ANY passphrase prompt — the gate is
`has_blocking_secret_prompt()`, true for cancellable and non-dismissible prompts
alike — and paints no dimmer, pointer sink, or focus trap while one is up. But
the replacement barrier was wired to dismissability (`blocks_input: !cancellable`),
so the ordinary just-in-time unlock prompt, which is cancellable, installed no
sink at all: pointer and keyboard fell straight through to the panels the overlay
exists to freeze.
Dismissal and input-blocking are orthogonal. `blocks_input` is now unconditional;
`cancellable` still governs only Cancel / X / Escape / click-outside, which read
raw pointer input and are unaffected by the sink.
The two comments asserting the prompt "supplies its own input barrier" described a
precondition the code did not establish; they now describe what it does.
Covered by a kittest that presses a control behind a cancellable prompt while an
overlay is raised (RED before this change: the control activated), plus one
pinning that the prompt still dismisses from its own Cancel button.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(migration): scope an unlocked seed to the storage update, not to one subtask
A wallet unlocked for the storage update has two consumers of the seed it just
promoted: the unlock gesture's own `wallet_unlock_registration` subtask, and the
update's `bootstrap_loaded_wallets()` pass, which re-enters the seed scope for
the very wallet it prompted for. Their lifetimes overlap in an order nobody
controls, yet the seed's lifetime was owned outright by the subtask's RAII guard.
Whichever finished first evicted the seed from under the other — and a cache miss
on a protected scope prompts, so the update raised a background passphrase prompt
for a wallet the user had just unlocked. If the user ticked "keep unlocked" on
that second prompt, it also silently restored the session-long retention the
migration prompt deliberately withholds.
Retention shorter than the session is now enforced by `SecretLease`, a ref-counted
claim at the secret chokepoint: each consumer holds a clone and the seed is
forgotten when the last one drops. The storage update takes its own lease for the
wallets it prompted for (`WalletUnlockRetention::UntilStorageUpdateComplete`) and
releases it on every exit path, so neither consumer can strand the other, and the
unlock still does not outlive the update.
The regression test drives the losing interleaving explicitly: the unlock subtask
is joined to completion first, then the update's pass must resolve the seed from
the session cache with zero prompts, and the seed must be gone once the run's
lease is released.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(dashpay): release the request guard when a dispatch is refused pre-dispatch
The storage-update gate rejects every wallet-touching task — `DashPayTask`
included — with a bare `WalletStorageNotReady`, before it reaches
`run_dashpay_task`, the only place that wraps a failure into
`DashPayContactRequestActionFailed { request_id, .. }`. That typed variant is
also the only one `release_request_guard_for_error` matched, so a contact
request's Accept / Decline / Cancel clicked during the first launch after an
upgrade claimed a guard nothing would ever release: the row's buttons went dead
for the full five-minute in-flight timeout, long after the update finished.
A pre-dispatch refusal names no request precisely because nothing ran, which is
exactly the condition under which a blanket release is safe. `clear_in_flight` is
restored for that one match arm only — every other failure still keeps its guard,
so an unrelated error cannot re-enable a row whose paid action may still be in
flight.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(backend): pin ShieldedTask inside the wallet-touching migration gate
The shielded family is refused during a storage update only because it is listed
in `is_wallet_touching`; nothing failed if a refactor dropped that membership.
Sibling of `wallet_task_is_rejected_while_migration_awaits_password`, dispatching
a shielded write while migration awaits passwords.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui): state the disabled-tool reason once, as one translation unit
"Clear Platform Addresses" explained its own unavailability twice in the same
row — a tooltip ("...because...") and an italic label ("...while...") — giving a
translator two units for one idea, and drifting on the word that carries the
meaning: the tool is disabled permanently, so "while" is wrong. Keeps the
always-visible label (a tooltip on a disabled control is easy to miss) with the
permanent reading, and drops the tooltip.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs: correct the data-deletion promise and record the migration change set
NET-019 still promised to "permanently delete all local data" and that "the
action cannot be undone", which the shipped Clear Database dialog now
contradicts: it discloses that an earlier version's read-only recovery database
stays on the device and may still contain wallet recovery data. The story now
matches the dialog and its sibling WAL-007 — the population it is written for
(clearing a machine before handing it on) is the one it most misleads.
UX-001 described the progress block yielding its pointer sink to a passphrase
prompt but never said the prompt installs its own in its place, reading as if the
click-through hole were still open. It now states the hand-off as an invariant,
for every prompt, dismissible or not — an unwritten invariant is how that hole
was reopened the first time.
CHANGELOG covered only the DashPay change set. Adds the two user-visible ones it
missed: the per-wallet password prompt on the first launch after an upgrade (with
its safe skip path), the read-only recovery database that "Clear Database" and
"Remove Wallet" no longer erase and the developer tool that is disabled as a
result, and the shielded refusal message.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(wallet): warn that SecretLease::lease() refcounts per call, not per scope
Independent verification of the SEC-002 fix (integration composition review)
found that lease() mints a fresh Arc on every call — two unrelated consumers
calling lease(scope) directly get two independent refcounts, so the first to
drop can evict the secret while the second is still relying on it. Not
currently reachable (the one call site correctly clones), but the type can't
enforce the invariant, so the next new consumer would reach for the public
lease() API and silently reintroduce the exact race SEC-002 just closed.
Document the footgun at the point of call rather than leave it undiscoverable.
* test(ui): prove the secret prompt's transition-frame click-through
egui resolves each frame's click at begin_pass against the previous frame's
widget geometry and modal layer. On the frame a passphrase prompt first
renders, the control beneath still existed last frame with no sink and no
modal layer above it, so the click completes on it before modal_chrome
installs the sink — mirroring AppState::update, where the visible screen
renders before render_secret_prompt.
- transition_frame_click_leaks_through_a_newly_activated_prompt: RED repro,
parked #[ignore]; un-ignore once the barrier is installed before the
visible screen renders on the activation frame.
- primed_prompt_blocks_the_same_injected_click_sequence: control (green) —
the identical injected click, with the prompt primed one frame earlier, is
absorbed. Isolates the leak to the transition frame, not the test harness.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(backend): refuse unavailable shielded ops early and scope DashPay gate rejections to one request
Two migration-gate refinements in run_backend_task, both closing bot-review
findings on PR #893.
Shielded pre-check: a shielded fund movement now short-circuits with
ShieldedOperationsUnavailable as the very first thing run_backend_task does —
before ensure_wallet_backend materializes seeds, registers upstream, and binds
Orchard for every loaded wallet just to run an op the app refuses. is_available
is a side-effect-free config read, safe before backend init; the in-handler gate
in run_shielded_task stays as belt-and-suspenders. Shielded ops are unavailable
on every network today, so this pre-check also precedes the migration gate: a
shielded write during a storage update now gets the accurate "not available"
message instead of a misleading "wait for the update".
DashPay guard scoping: the migration gate now tags a rejected contact action
(Accept/Reject/Cancel) with DashPayContactRequestActionFailed carrying its
request ID, so the Identity Hub releases only that request's in-flight guard.
The previous stopgap blanket-cleared every guard on a bare WalletStorageNotReady,
which could re-enable a different contact action's row while its paid state
transition was genuinely still in flight. release_request_guard_for_error drops
the blanket-clear arm; the now-unused clear_in_flight is removed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui): drop the transition-frame click when a passphrase prompt activates
egui resolves each frame's click at begin_pass against the previous frame's
widget geometry and modal layer, before update() runs. On the frame a passphrase
prompt first renders, the previous frame had no prompt and no input sink, so a
press-then-release completing now still lands on the control beneath — the modal
installs its sink one frame too late, and reordering the render within the frame
cannot help.
AppState::update now detects the prompt-activation rising edge (covering both the
just-in-time unlock and the migration password prompt, via
has_blocking_secret_prompt) and calls drop_activation_frame_pointer_click, which
clears this frame's pending pointer input before the screen beneath runs. A
widget only reports a click while a Released event is still in input.pointer, so
dropping it strands the leaked click; keyboard input is left intact for the
freshly focused password field, and the sink covers every later frame.
Un-ignores the transition-frame repro (now GREEN) and adds a migration-prompt
sibling; the primed-prompt and yielding-overlay sink tests still pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(ui): pin passphrase activation wiring in the real AppState update loop
The two existing transition-frame repro tests mirror
drop_activation_frame_pointer_click directly in a hand-rolled closure — they
never drive AppState::update(), so the production rising-edge call site in
app.rs was untested: deleting it left the suite green. Add
appstate_jit_prompt_activation_drops_transition_frame_click and
appstate_migration_prompt_activation_drops_transition_frame_click, which
mount a real AppState via build_eframe, activate a prompt through the actual
JIT (test_set_secret_prompt_active) and migration (MigrationStatus) paths,
and assert a click completed on the activation frame does not reach the
welcome screen beneath. Independently confirmed both fail when the app.rs
call site is neutralized and pass when restored.
Also corrects a stale doc comment/assertion in hub_screen.rs left over from
3e69b2fd, which removed the blanket WalletStorageNotReady guard-release arm:
the comment still described a "blanket release... scoped to refusals that
prove nothing is running" that no longer exists in the code.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(wallets): keep dialogs open on trigger clicks
Fix SND-003, WAL-005, and WAL-006 by ignoring outside-click dismissal on each dialog's opening frame.
Co-Authored-By: Codex GPT-5 <noreply@openai.com>
* build(deps): bump platform to PR3968 tip (d18020f5), pulls in the AssetLockProof rehydration fix
Updates dash-sdk / rs-sdk-trusted-context-provider / platform-wallet /
platform-wallet-storage git pins from 93b967f9 to d18020f5
(dashpay/platform#3968 tip), which includes the AssetLockEntryWire fix
for the AssetLockProof deserialize_any bug (dashpay/platform#4133) that
was blocking wallet rehydration on every relaunch once any asset lock
existed.
Adapts to unrelated upstream API drift pulled in by the same bump:
DataContractJsonConversionMethodsV0::to_json(&self, platform_version)
was removed as part of dpp's JSON/Value conversion trait unification
(dashpay/platform#3573, already a known pre-existing lint debt in this
branch). The canonical replacement for "give me this contract's
current wire-format JSON" is
DataContractInSerializationFormat::try_from_platform_versioned(...)
+ serde_json::to_value(...) — updated the 3 affected call sites
(contract_chooser_panel.rs x2, update_contract_screen.rs,
token_creator.rs); from_json usage elsewhere is unaffected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* test(ui): make the opening-click regression test exercise the real guard
opening_click_does_not_immediately_dismiss previously only asserted that
seeding PassphraseModalState with an armed ModalOpeningGuard left the cache
entry readable — a plain data-cache round trip that never called
clicked_outside_window_after_open and could not fail regardless of the
guard's behavior. Rewrite it to simulate an actual outside click via
egui::RawInput and call the real function: the opening click must be
swallowed once, then a later check against the same pending click must
detect it normally. Also drops the internal commit-SHA reference in the
adjacent comment per the "describe present state, not history" convention.
Found by an independent adversarial review of this branch's merge (QA-001,
QA-002).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(contracts): don't panic when contracts can't load on Update Contract screen
UpdateDataContractScreen::new() called app_context.get_contracts().expect(...),
panicking the whole process when the contracts store errors (e.g. an unwired
wallet backend returns Err(WalletBackendNotYetWired)). Degrade gracefully
instead: fall back to an empty contract list and show a calm, actionable
MessageBanner with the underlying error attached via with_details(), matching
the established pattern in document_action_screen.rs and
group_actions_screen.rs.
QA-002. Implemented by Codex Sol, committed by the coordinator (this
sandbox's git metadata for the worktree is read-only, a recurring
environment constraint this session).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(dashpay): accept all integer encodings for contact-request key indices
derive_contact_payment_address() extracted senderKeyIndex/recipientKeyIndex
with a strict match on Value::U32, but network-fetched documents decode
integers as Value::I128, so extraction always failed with "Missing
senderKeyIndex" and DashPay payments could never succeed. Fixed by using the
canonical platform_value helper (to_integer::<u32>()) already used for the
same fields in contact_requests.rs, extracted into a small pure helper
(read_contact_request_key_indices) and unit-tested against I128/U32/I64.
Swept the rest of the DashPay backend for the same strict-match fragility and
converted three more sites the same way: contact_info.rs and contacts.rs
(derivationEncryptionKeyIndex/rootEncryptionKeyIndex) and
auto_accept_handler.rs (accountReference, previously handled with a manual
five-arm match — now the same single helper call).
DPY-006. Implemented by Codex Sol, committed by the coordinator (this
sandbox's git metadata for the worktree is read-only, a recurring
environment constraint this session).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(mcp): hydrate saved wallets before wallet-facing tools read them
ListWalletsTool::invoke (and every other tool that calls resolve::wallet(),
which reads ctx.wallets) ran before any SPV gate wired the wallet backend —
ctx.wallets is only populated inside WalletBackend::new via
AppContext::ensure_wallet_backend, and core_wallets_list deliberately skips
resolve::ensure_spv_synced. A fresh standalone det-cli process therefore
always reported {"wallets":[]} even with wallets already persisted to disk.
Added resolve::ensure_wallets_hydrated(), which wires the backend via
ctx.ensure_wallet_backend() with a throwaway sender — no SPV start, no sync
wait, idempotent on repeat calls — and called it ahead of every resolve::wallet()
call site that wasn't already behind ensure_spv_synced (17 tools across
wallet.rs, identity.rs, and shielded.rs). Updated docs/MCP.md and docs/CLI.md
to describe the new hydrate-on-demand behavior.
Verified with the exact det-cli two-process smoke flow from CLAUDE.md: import
a wallet in one process, list wallets in a fresh process against the same
data dir, confirm it appears with the expected seed hash and alias.
MCP-001. Implemented by Codex Sol, committed by the coordinator (this
sandbox's git metadata for the worktree is read-only, a recurring
environment constraint this session).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(ui): stop the opening click from immediately cancelling confirmation dialogs (IDN-006, TOK-005, TOK-011, TOK-018)
Transfer Funds, token creation/registration, token claiming, and stop-tracking
all rendered their confirmation popup in the same egui frame as the button
click that triggered it. clicked_outside_window() read that still-active
click as a dismissal, so the dialog opened and cancelled itself within the
same frame -- visually indistinguishable from the button being a no-op.
Fixes it the same way passphrase_modal.rs's opening-click bug was fixed:
ConfirmationDialog now carries a ModalOpeningGuard, armed on construction and
consulted via clicked_outside_window_after_open() instead of the raw
outside-click check. The data-contract JSON popup gets its own guard for the
same reason.
Also fixes a compounding bug in the token creator: it rebuilt a brand-new
ConfirmationDialog (and therefore a freshly-armed guard) every single frame
via Option::insert(), which meant the dialog could never observe its own
post-opening frame. Switched to get_or_insert_with() so the dialog persists
across frames once created.
TOK-011 and TOK-018 share the same ConfirmationDialog component, so the fix
covers their reported no-op behavior without separate changes.
Implemented by Codex Sol, committed by the coordinator after independent
review of every hunk and a from-scratch fmt/clippy/test verification pass.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* fix(ui): wire missing identity navigation and fix stale post-refresh state (IDN-008, IDN-013a, DPN-008, IDN-009)
Navigation gap (IDN-008, IDN-013a, DPN-008): KeysScreen and the "My
usernames" list existed and worked but had no reachable route from Identity
Settings -> Advanced in the current Identity Hub build. Adds "Manage keys"
and "View all usernames" entries. Also corrects an inverted gate on the
Transfer screen's key-info button -- it only appeared when the identity had
*no* transfer key, backwards from the intended "manage the key you have"
flow. Key Protection (IDN-013a) was already fully implemented; it just
needed the same navigation fix to become reachable.
Refresh staleness (IDN-009): "Refresh identity data" fetched fresh state
from the network and persisted it correctly, but the backend task returned
the stale pre-refresh identity to the UI instead of the newly-fetched one,
and the Settings screen's own selected-identity cache only updated when the
identity's ID changed -- never on same-ID refreshes, which is the only kind
a refresh produces. Combined, a refresh could add a new on-chain key and the
UI would still show the old key count indefinitely. Fixed both: the backend
now returns the updated identity, and Settings reconciles same-ID refreshes
instead of only replacing on an ID change.
Implemented by Codex Sol, committed by the coordinator after independent
review of every hunk and a from-scratch fmt/clippy/test verification pass.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* fix(backend): stop silent hangs and panics in backend tasks (HANG-CLASS, IDN-002, MN-001, DOC-004, TOK-003)
Task-panic watchdog (HANG-CLASS): handle_backend_task/handle_backend_tasks
spawned work via tokio::task::spawn_blocking but dropped the returned
JoinHandle, so a panic inside the spawned closure vanished silently -- the
UI just hung forever with no error. The handle is now kept and awaited by a
managed watcher task; a panic or cancellation surfaces as a new typed
TaskError::BackendTaskFailed with a calm, actionable banner. The raw panic
payload is redacted from diagnostics (BackendTaskJoinError's Debug/Display
only expose task id / cancelled / panicked, never the panic message itself)
to avoid leaking arbitrary panic content into logs.
Network-request timeouts (IDN-002, MN-001, DOC-004, TOK-003): identity
loads (primary, voter, and DPNS-name fetches), document fetches, and token
lookups could all hang indefinitely on a stalled network call with no
feedback. Added a shared await_network_request_with_timeout helper (90s,
NETWORK_REQUEST_TIMEOUT) used at every affected call site, each mapping to
its own typed, actionable TaskError variant.
Token balance refresh needed more care than a plain timeout: the upstream
sync is not safely cancellable -- dropping it mid-flight could leave
is_syncing permanently stuck, trading a hang for silently-disabled sync
forever. Added await_managed_network_request_with_timeout: the request runs
as a detached, task-manager-tracked spawn; only the caller's *wait* on it
times out, so the sync itself always runs to completion even after the UI
gives up on it. A new token_balance_refresh_in_flight flag (RAII guard,
cleared on drop even if the refresh panics) also gives the refresh
single-flight protection so overlapping requests can't race.
Also fixed two more sites with the same forbidden string-match anti-pattern
DOC-004 was originally reported against (matching literal banner text
instead of message type to know when an in-flight fetch failed): the main
Tokens screen's RefreshingStatus and the token-claims screen's FetchStatus
both had the identical fragility and are fixed the same way.
Implemented by Codex Sol, committed by the coordinator after independent
review of every hunk and a from-scratch fmt/clippy/test verification pass.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* docs(user-stories): drop transient review-ID citation from UX-001
SEC-004 is an internal review-finding ID with no meaning outside the
review artifact that produced it — doesn't belong in a durable spec.
Flagged by Claudius-Maginificent's PR894 review.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* fix(app): stop migration frame race and preserve vote eligibility across migration
A single per-frame migration-state snapshot now backs every input-claim
and rendering decision (has_blocking_secret_prompt, claim_overlay_input,
ProgressOverlay::render_global, MigrationReconciler::update_banner)
instead of each call re-reading live state — closing a window where a
mid-frame migration transition could let the underlying screen consume
input for a frame where a blocking prompt was about to appear.
The periodic scheduled-vote sweep now defers while migration is in
progress instead of running unconditionally and silently skipping votes
whose imported identity isn't loaded yet. On migration completion, a
recovery sweep casts any vote whose normal 120s eligibility window
overlapped the deferred period, so a password prompt left open past that
window no longer permanently drops the vote.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* fix(dashpay): make contact-request decline/cancel idempotent
DashPay decline and cancel each broadcast a paid visibility transition
(contactInfo hide) before writing a local retire marker. A crash, retry,
or a second UI surface between the broadcast and the marker could
re-broadcast the hide and re-pay. Guard the flow so the paid hide runs at
most once per request:
- Add a durable per-request recovery journal (ContactRequestActionPhase)
in the DashPay k/v sidecar, scoped to the acting identity, so a retry
resumes at the last committed phase instead of re-broadcasting.
- Add a request-wide async lock plus a process-local in-flight claim so
concurrent declines/cancels serialize on one paid hide.
- Paginate contactInfo lookup and reuse it for a hidden-state probe so a
corrective unhide only fires when the contact is actually hidden.
- Correlate a panicked paid action back to its request id via
DashPayContactRequestActionFailed so the Hub releases only that guard,
and route contact-request results/errors to a hidden Hub screen.
- Retain paid-action guards across view resets; release only on the
correlated terminal result.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallet): fail Clear-Database safely and hydrate legacy wallets for MCP
Two wallet-readiness gaps:
- clear_network_database silently no-op'd its wallet-secret, DashPay
sidecar, and shielded cleanup when the wallet backend was not yet wired,
so "Clear Database" could report success while persisted secrets from an
earlier run survived. Require the wired backend up front and return the
dedicated WalletDataClearUnavailable error, leaving all state intact for
a safe retry.
- Standalone/headless MCP never awaited the cold-start legacy-data
migration, so legacy wallets were invisible to wallet reads. Hydrate and
finish the pending migration in ensure_wallets_hydrated, converting a
terminal MigrationState::Failed back into its typed task error.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(app): correlate task results to their originating operation
A backend-task error was routed purely by message type, so a concurrent
task's failure could trip an unrelated screen's in-flight status, and the
token-balance refresh guard could strand forever on a true hang.
- Introduce BackendTaskContext, attributed to each dispatch, and carry it
on TaskResult::{Success,Error}. Add display_backend_task_result /
display_backend_task_error so screens correlate a result to the exact
operation (document query, token-balance refresh, reward-estimate pair)
instead of matching on message text.
- Document, token, and claims screens now clear their in-flight status
only when the failing/completing task matches the pending one; an
unrelated failure no longer clears a genuine refresh banner.
- Suppress a duplicate token-balance refresh only while the first is
pending, and give the hung-refresh guard honest restart guidance.
Composes with the DashPay request-id correlation: forward_backend_task_join_error
now carries both the optional request id and the task context.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallet): also wipe identity private keys on Clear Database (SEC-001)
Clear Database wiped seeds, single-keys, DashPay overlays, and shielded
state but never removed identity private keys — the identity_key_priv.*
vault entries or the det:identity:* records. Those keys are Tier-1 keyless
(plaintext-recoverable) by default and include masternode voting/owner/
payout keys, so a user who chose to erase all local data still left
fund-control keys recoverable on disk.
The clear-all sweep already fans out over local_identity_ids() to drop
each identity's DashPay overlays; call the existing public helper
delete_local_qualified_identity for each identity in that same loop. It
runs clear_identity_vault_keys (-> IdentityKeyView::delete_all, wiping the
vault key bytes) and purges the identity scope + index (removing the
det:identity:* records), closing both halves of the gap.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallet): report partial failures when Clear Database can't delete every secret (SEC-002)
forget_wallet_local_state and forget_all_wallets_local logged each failed
per-secret delete but returned success unconditionally, so a failed seed,
single-key, or identity-key delete was still reported to the user as a
completed wipe — leaving recoverable secrets on disk behind a false
"cleared" message.
Accumulate delete failures instead of swallowing them:
- forget_wallet_local_state keeps attempting every step (resilient) but
returns the first failure so a partial wipe is never reported as clean.
- forget_all_wallets_local returns a ClearAllOutcome carrying the upstream
ids to remove plus every delete failure.
- clear_network_database collects those failures and the per-identity
wipe failures, still clears the in-memory maps, then returns the new
typed TaskError::WalletDataClearIncomplete { failed, #[source] first_error }
when anything failed. Its Display tells the user to restart and retry.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(dashpay): accept both decrypt-failure variants in unreadable-private-data test
read_contact_info_private_data decrypts contactInfo privateData with
unauthenticated AES-256-CBC + PKCS7 and a random IV. A wrong-key decrypt
usually fails PKCS7 unpadding (DecryptFailed), but roughly 1 in 256 the
random IV produces valid-looking padding and the garbage plaintext then
fails to parse (DeserializeFailed). Both mean the same thing — the stored
payload is present but unreadable, so the write aborts.
Two tests asserted ONLY DecryptFailed, so they flaked ~0.3% of full-suite
runs (confirmed: 4 failures in 1500 isolated runs before, 0 in 2500
after). Widen both assertions to accept DecryptFailed OR DeserializeFailed;
the product code's abort behavior is unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(contracts): isolate update_contract_screen degrade test from shared contract state
constructor_degrades_when_contracts_cannot_be_loaded asserts
known_contracts.is_empty(), which only holds when get_contracts() fails:
on success it always returns the pinned system contracts (dpns,
dashpay, ...) and "dashpay" is not in the constructor's excluded set. The
test built a real AppState, which wires the wallet backend asynchronously
inside the test's Tokio runtime, so whether get_contracts() saw a wired
backend (success -> non-empty) or not (error -> empty) raced the
constructor — the flake (green in the integration gate, red in CI).
Construct the screen from a backend-less test_app_context instead: with no
wallet backend wired, get_contracts() deterministically fails and the
constructor degrades to an empty list, which is exactly the path this test
names. Drops the DASH_EVO_DATA_DIR env-var dance and its module-local lock
entirely (0 failures in 60 isolated runs, was intermittently red).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallet): report Clear-Database failure when the identity index can't be listed (SEC-VERIFY-001)
If local_identity_ids() fails, clear_network_database skips every
per-identity key wipe — yet it still returned Ok(()), so every identity's
private keys (incl. masternode voting/owner/payout) could survive behind a
false "cleared" message: the exact false-success class SEC-001/SEC-002
close, gated behind a listing error.
Push the listing error into the failures accumulator so it surfaces as
TaskError::WalletDataClearIncomplete instead of a silent success. The
warn log is kept.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui): add Back navigation to the Manage Keys screen (dead-end lockout)
KeysScreen renders a read-only key list pushed onto the screen stack but
returned AppAction::None unconditionally, trapping the user with no way
back to the identity view. Add a Back control in the header row that
returns AppAction::PopScreen, matching the sibling read-only detail
screens (e.g. contact_profile_viewer). A kittest asserts the button
renders and its click pops the screen off the stack.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui): show fee estimate and total before sending Dash (SND-005)
The Send Dash screen dispatched a payment with no fee or total shown, so
the user committed without seeing what would leave their balance. Add a
fee summary rendered directly above the Send button:
- Simple mode: estimated network fee, total deducted, and (when the fee
is taken out of the amount, e.g. Core -> Platform) what the recipient
receives. Covers every cleanly-estimable source/destination pair
(Core->Core/Platform/Shielded, Platform->Platform/Core/Shielded,
Identity->Core/Platform/Identity), reusing the same
model::fee_estimation estimators the amount field's "Max" reserve uses
so the two never disagree. Combinations whose fee depends on inputs the
backend selects at send time (identity top-ups, shielded spends) show a
neutral "calculated when you send" note instead of a wrong number.
- Advanced mode: estimated network fee for the count-driven paths
(Core->Core, Platform->Platform), else the same neutral note.
All fee math stays in model::fee_estimation; FeePreview only arranges
already-estimated numbers for display. Pure unit tests cover the on-top
vs deducted-from-amount total/recipient semantics and saturation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(dashpay): report the correct cause when Add Contact can't resolve the recipient (NEW-002)
Sending a contact request to a recipient with no DashPay decryption key
raised DashPayError::MissingDecryptionKey, whose message ("Your identity
is missing a decryption key required for contacts") blamed the SENDER —
even though the sender's keys are fine and it is the RECIPIENT
(to_identity) that lacks the key. The Add Contact screen compounded the
error by offering an "Add Decryption Key" button that would add a key to
the sender's own identity, a remedy that cannot fix a recipient-side gap.
Rename the variant to RecipientMissingDecryptionKey and reword it to
correctly attribute the failure to the recipient with an actionable,
jargon-free message. Drop it from requires_user_action() and remove the
misleading self-remedy button — the sender has no key to add; the message
tells them to ask the recipient to finish setting up their profile. Both
error classifiers and their tests are updated to the renamed variant.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs: correct SND-005 fee-estimate criterion to match inline pre-send summary
The SND-005 acceptance criterion described the fee estimate as "shown in
confirmation dialog", but the HD-wallet Send Dash screen surfaces it
inline above the Send button (simple and advanced modes) before dispatch.
Reword the criterion to match the implemented behavior.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui): guard reusable modal components against opening-frame dismiss (NEW-003)
InfoPopup and SelectionDialog closed themselves on the same frame they
opened: the click that opened the popup lands outside the not-yet-rendered
window rect, so the unguarded clicked_outside_window() check fired true on
the opening frame and dismissed the popup before it was ever visible
(e.g. the token "More Info" popup never appeared).
Both components are value-constructed every frame from consumer-held state,
so a persistent ModalOpeningGuard field cannot survive across frames. Add
clicked_outside_window_after_open_by_id(), which records the last render
pass in egui temp memory keyed by a stable id and skips the outside-click
check on the opening frame — detected as a gap in rendering, so it re-arms
automatically however the popup was previously dismissed, with no teardown.
Fixing this inside the two components fixes every consumer at once
(InfoPopup: 13 call sites; SelectionDialog: no current consumers, so this
is preventive). Unit tests cover the opening-frame skip and the re-arm.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui): guard screen-level popups against opening-frame dismiss (NEW-003)
Six screen-level popups used the unguarded clicked_outside_window(), so the
click that opened them was seen as an outside click on the first render
frame and dismissed them before they appeared. Give each a persistent
ModalOpeningGuard field, arm it where the popup's open state is set, and
switch the check to clicked_outside_window_after_open() — mirroring the
existing wallets_screen rename-dialog and receive-dialog pattern.
Sites fixed:
- contracts_documents_screen: "Select Properties" fields dropdown
- dashpay/profile_screen: avatar-URL popup
- identities_screen: edit-alias modal (both open buttons)
- tokens my_tokens: "More Info" token popup and reward-explanation popup
- wallets add_new_wallet_screen: "Fund Wallet" receive popup
- wallets_screen/dialogs: fund-platform-address and mine-blocks dialogs
(the receive dialog in the same file was already guarded)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(dashpay): show payment-history amount in DASH, not raw duffs (NEW-004)
The DashPay payment-history row printed the raw duffs value with a "Dash"
label — a 0.001 DASH payment rendered as "-100000 Dash" — because the
amount (duffs, from DashPayPaymentHistory) was formatted with `{} Dash`
and no unit conversion. Format it with `format_duffs_as_dash` so it reads
"-0.001 DASH". Fixed in both the Pay screen history (the live path) and
the contact-details history (currently unpopulated, fixed defensively);
documented that the `Credits`-aliased field actually holds duffs.
Counterparty label (NEW-004 part 2) is left as scoped follow-up: the
payment history resolves names against saved DashPay contacts only, so a
recipient paid by DPNS username (not a mutual contact) shows
"Unknown (<prefix>)". Resolving it needs a DPNS lookup by identity id or
persisting the send-time name — deeper plumbing than this cached-read
path — so it is marked with a TODO(NEW-004) rather than forced.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallets): confirm single-key removal and refresh asset locks after creation
WAL-007: single-key wallets had a separate Remove handler that called
forget() immediately, bypassing the HD wallet's confirmation state
entirely. Both wallet types now route through the same pending-removal
state feeding the existing danger confirmation modal.
ALK-002: Loaded(empty) was a terminal cache state as reported, but the
actual navigation gap was that PopScreenAndRefresh invokes
refresh_on_arrival(), not refresh(), which is where the cache
invalidation previously lived. The selected wallet's asset-lock cache
entry is now invalidated on root-screen arrival so a freshly created
lock is picked up on the next render.
Cherry-picked from 26937906 onto fix/snd-003-receive-inert. Conflict
resolution: the single-key-remove-button block was refactored into
request_selected_wallet_removal() (theirs); the snd003 branch's
customized HD-removal confirmation message (the earlier-version
read-only recovery-database note) was preserved into that method's HD
branch. The branch's NEW-003 rename-dialog ModalOpeningGuard usage in
this file is untouched.
Co-Authored-By: Codex GPT-5 <noreply@openai.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallets): restore password-modal focus without leaking background input
NEW-005 (release-blocking): the wallet-unlock / JIT secret password field
could not receive keyboard focus or typed input. `modal_chrome` registered a
separate full-screen "sink" Area as egui's modal layer; because that sink was
a different layer than the `egui::Window` holding the field, the window
resolved *below* the modal layer, so egui's `Memory::allows_interaction`
silently denied the `TextEdit` focus (and clicks on it).
Register the window's OWN layer as the modal layer instead: comparing a layer
against itself is `Equal`, so the modal's fields always resolve at/above the
modal layer and stay focusable, while every lower layer is blocked. The
full-screen sink is retained — moved to `Order::Middle`, strictly below the
`Order::Foreground` window — because it is load-bearing for background input
blocking: egui's `layer_id_at` only redirects a below-modal click to the modal
layer when some interactable area covers that position, so without full-screen
coverage a click landing outside the centered window would fall through to the
app beneath.
Add a kittest regression, passphrase_modal_password_field_focuses_and_blocks_background,
asserting the field takes focus and receives typed text (surfaced via Submit),
the modal layer is the window's own layer (not the sink), and a widget behind
the modal receives none of it. The pre-existing background-blocking and
dismissal kittests continue to pass, confirming the sink still blocks input.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(migration): gate startup migration on a minimum saved-data version
DET ran its startup data migration without checking the on-disk data
version, so migrating from an unsupported (too-old) version failed in
confusing ways. Read settings.database_version before migrating and gate
at both entry points (legacy-settings import and FinishUnwire) before any
sentinel or state is written; the legacy data.db is opened read-only.
Data versions 11..=40 migrate directly (v0.9.3 = 11 = the supported floor,
already migratable). Older data is rejected with an actionable "install
Dash Evo Tool 0.9.3 first" message; newer data fails closed. Fresh installs
(initialize writes v38 before the gate) are never rejected; a corrupt DB
missing the version row fails closed.
Typed errors SavedDataTooOld/SavedDataTooNew and LegacyDataTooOld/
LegacyDataTooNew carry numeric context and #[source] only (no user strings
in variants). Adds src/model/data_migration.rs (pure version classification).
Implemented by Codex (gpt-5.6-sol, high effort); data-safety reviewed
(gate-before-mutation, exact 11..=40 boundaries, fail-fast-no-write,
fresh-install-safe) — 0 blocking findings.
Co-Authored-By: Codex gpt-5.6-sol <noreply@openai.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* test(migration): cover the too-new fail-fast and upper-accept boundary
Marvin flagged two LOW gaps in the startup DB-version gate. This feature is not
GUI-testable, so tests are its primary safety net.
- Add an async `finish_unwire::run` test (too_new_database_version_is_rejected_
before_migration) mirroring the existing too-old test: a version above the
ceiling (41) is rejected before any pass runs, surfacing the typed
SavedDataTooNew / LegacyDataTooNew chain, and NEITHER the completion sentinel
NOR any migration state is written (state stays Idle).
- Add an upper-accept boundary test (max_supported_database_version_is_accepted_
for_direct_migration): MAX_DIRECT_MIGRATION_VERSION (40) — the top of the
accepted 11..=40 range — is accepted, and the first version above it is
rejected as too new. Previously only 11-accept and 41-too-new were pinned; the
top of the accept range was never asserted accepted.
QA-001: document the deliberate headroom on MAX_DIRECT_MIGRATION_VERSION (40)
above DEFAULT_DB_VERSION (38), so data from a slightly newer build (39, 40) still
migrates rather than failing closed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(settings): add developer-only Wipe Platform Data control (NEW-006)
Wire the previously-orphaned SystemTask::WipePlatformData to a developer-only
button on the Identity Hub Settings tab, gated to Devnet (the backend
wipe_devnet handler only clears devnet identities, tokens, and user contracts).
Guarded by a type-"WIPE"-to-confirm destructive dialog via a new
ConfirmationDialog::require_confirmation_text builder. Implemented by Codex Sol.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* feat(contacts): add View Profile action to Identity Hub contacts (NEW-007 / DPY-005)
Each active-contact row now offers a View Profile action that opens the working
ContactProfileViewerScreen for the selected contact, alongside the existing Pay
action. Reuses the same viewer the legacy DashPay paths use; the orphaned
ContactDetailsScreen is left untouched. Implemented by Codex Sol.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* fix(wallet): fail-closed on identity-key wipe and surface Clear-Database failures (SEC-001, SEC-002)
clear_identity_vault_keys now returns Result and propagates instead of
swallowing vault read/decode/delete errors, so identity removal (Clear
Database, identities screen, masternode detail, migration) reports incomplete
rather than clean when private keys — including masternode voting/owner/payout
keys — cannot be deleted. IdentityKeyView::delete_all attempts every key and
returns the first error instead of short-circuiting. DashPay sidecar/overlay
delete failures in clear_network_database are now accumulated into the failures
list (SEC-002) rather than warn-only. Adds a masternode-removal regression test
that injects a vault-key delete failure. Fixes found by security review.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* test: serialize DASH_EVO_DATA_DIR mutation with one shared lock
Replace per-module mutexes guarding DASH_EVO_DATA_DIR with a single crate-wide
lock in a new test_support module. Module-local locks let tests in different
modules race on the process-global env var under parallel execution, causing
intermittent AppState::new failures (e.g. add_token_by_id_screen's
display_task_result test). One shared lock serializes them deterministically.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* fix(ui): give each modal a unique guard id so popups don't dismiss each other (QA-001)
The opening-frame dismiss guard keyed on a single global Id shared by every
InfoPopup and SelectionDialog, so two independent popups on one screen (e.g. the
profile screen's Profile-Guidelines and Avatar-Guidelines info popups) shared
render-history state: closing one via outside-click then opening the other on
the next frame dismissed the second on its own opening frame. Each InfoPopup and
SelectionDialog now takes a caller-provided per-instance Id (mirroring
passphrase_modal), so guards no longer collide. Adds a two-popup regression test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* fix(ui): move Wipe Platform Data beside its sibling network controls (NEW-006)
The developer-only "Wipe Platform Data" control (story NET-011) wipes data
for the whole devnet, but was rendered on the Identity Hub -> Settings tab,
a per-identity screen. Its siblings are network-scoped and live on the
Network Chooser: "Clear {Network} Database" (NET-019) in the "Database
Maintenance" section and "Clear SPV Data" (NET-020).
Move the control into "Database Maintenance", directly after the Clear
Database button, matching that file's danger-button styling and its
existing selected_role.at_least(UserRole::Developer) gating idiom.
Gating is unchanged and stays deliberately narrow: Developer role AND
Devnet. The devnet condition is load-bearing, not cosmetic, because the
backend wipe_devnet() is devnet-scoped
(delete_all_local_qualified_identities_in_devnet / _tokens_in_devnet /
clear_user_contracts).
Gate is now enforced in three places: the render check, a re-check in
show_wipe_platform_data_confirmation that dismisses the dialog if the gate
stops holding (so a dialog opened on Devnet cannot fire after a network
switch), and a fail-closed wipe_platform_data_action.
The type-WIPE confirmation and all user-facing wording are unchanged. Both
unit tests move to ui::network_chooser_screen::tests and now assert the
negative cases in both directions.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* fix(platform): keep cause-less transition results unconfirmed (#897)
A state transition can be accepted for broadcast while the separate result wait fails. Treat SDK broadcast-error envelopes without a structured consensus cause as submitted but unconfirmed, and direct the user to verify completion before retrying.
Co-authored-by: Codex GPT-5 <noreply@openai.com>
* docs(qa): PR892 user-story QA campaign — full retest record (175/175 stories) (#895)
* docs(qa): scaffold PR892 user-story QA campaign checklist
Populated progress.md with all 123 stories from docs/user-stories.md
(112 Implemented to test, 11 Gap pre-marked N/A). Note: source brief
referenced 152 stories incl. UX/IDH/MN categories that don't exist in
the current doc — proceeding with the doc as it actually is.
* docs(qa): confirm PR892 tx-history regression fix; WAL/SND/NET spot checks
Critical result: full quit + cold-boot relaunch on the same data dir now
correctly re-renders transaction history (was the PR892 bug). Verified
with 3 real testnet transactions via the Pasta faucet.
Also: NET-001 (switch networks) PASS, WAL-001/004/010/011/016/023/024
PASS, SND-001 PASS (nav only), SND-003 (Receive button) FAIL — no QR
code or modal appears, reproduced 3x.
* docs(qa): add shared campaign context for delegated per-category agents
* docs(qa): complete remaining WAL user-story QA pass (PR892)
Finishes WAL-002/003/005/006/007/008/012/013/017-020/021/022 a…
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this PR exists
feat/legacy-identity-migration), whose head is merged in.What was done
fix(masternodes): relabel the status dot/text as "Platform identity: <status>" with a clarifying tooltip ("…does not show Core network or PoSe status").fix(masternodes): sort the node grid by display name.Withdrawn from this PR
The migration wallet-password prompt was implemented here and has been reverted (
91dc077e). A review of the combined diff surfaced four blocking defects, and given the blast radius it is being reworked in a dedicated PR stacked on this one rather than fixed in place:tokio::sync::Notifywhose only producer is the egui frame loop, whilemcp/resolve.rsdrives the same migration with no frame loop — headlessdet-cli/MCP would hang forever on an install with a password-protected legacy wallet.remove_wallet— deleting the wallet and its seed.RegistrationIncompleteeven when the correct password was supplied (double-registration race; reproduced as a real test failure).The rest of this PR is unaffected by the revert.
Testing
cargo +nightly fmt --all— clean.cargo clippy --all-features --all-targets -- -D warnings— clean.cargo test --all-features --workspace— green on the reverted tree.Breaking changes
None.
Checklist
Attribution
🤖 Co-authored by Claudius the Magnificent AI Agent