fix(migration/wallet): QA follow-ups for #885 — resurrection, banners, decode limit, wallet naming - #891
Merged
Conversation
… 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>
`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.
…ped 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>
`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>
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>
…entities 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>
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 |
lklimek
marked this pull request as ready for review
July 14, 2026 11:45
lklimek
added a commit
that referenced
this pull request
Jul 14, 2026
* docs(migration): design the v0.9.3 legacy identity import 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> * feat(migration): import legacy v0.9.3 identities and their keys 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> * fix(migration): never let a corrupt vote queue strand identity keys 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> * fix(migration): honor legacy alias fallback and guard identity-import 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> * fix(migration): column-authoritative alias, per-row identity decode, 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> * fix(migration): surface unreadable votes alongside unreadable identities 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> * fix(migration): reconcile legacy keys into partially loaded identities The identity import skipped a legacy row wholesale whenever the id was already in the modern store. But presence is not proof every key survived: before this PR, a masternode could be loaded from only its ProTxHash (voting/owner/payout keys all optional), persisting a partial key map. When such an install upgrades, the legacy blob may still hold owner/voting/payout keys the modern record lacks — and the wholesale skip stranded them. The skipped row was not counted unreadable, so the sentinel landed and those legacy-only keys never reached the vault or got retried: a silent loss of a masternode's control keys, not just a banner glitch. The importer now fetches the existing modern identity and gap-merges the legacy blob into it: the modern record stays authoritative (its keys, alias, protection state, and wallet link always win) and only the keys/associations it lacks are taken from the blob. It re-persists in place via update_local_qualified_identity only when the merge actually recovered something (new `reconciled` counter); an identical record is left untouched, so a retry can never overwrite a user edit with the stale legacy copy. The gap-merge is the same "keep what I have, borrow only what I'm missing" rule load_identity already used for in-place key adds; that private helper is promoted to QualifiedIdentity::merge_gaps_from (model/, single source) and reused by both callers. Regression test `a_present_but_partial_identity_gains_the_legacy_only_keys` stages a partial modern identity plus a legacy blob carrying an extra Owner key and asserts the key is merged in and the record re-persisted once; the existing already-in-store test now proves an identical record is not re-written. Design doc §7 edge-case table updated to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(migration): only reconcile bare identities, never keyed ones The e8b6182 reconcile filled a present identity's gaps from the legacy blob by inferring "missing" from field absence. Two ways that is unsafe for a background migration (both raised in review): 1. Protection downgrade — merging a legacy `Clear` key into an identity whose other keys are password-protected produces a mixed record. On save, `encode_identity_blob_vault_first` refuses it with `IdentityKeyProtectionDowngrade`; the migration then errors before writing its sentinel and fails identically on every launch. 2. Resurrected removals — absence is not proof of a partial load. "Remove private key from DET" deletes a map entry and clearing an alias persists `None`. On a pre-sentinel install (or a retry held open by another unreadable row) the merge would refill those intentional absences from the stale blob, restoring a removed alias or re-adding a deliberately-removed private key. Fix: reconcile only a record that holds NO private keys at all — the one unambiguous "loaded without its keys" signal (the ProTxHash-only masternode load). For a bare record, take the legacy key set and fill the missing masternode role associations; re-persist only when something was recovered. Any record that already holds keys is left untouched: a protected identity always holds keys, so it never reaches the vault-first guard (fixes 1), and a keyed record's absent field is never refilled, so removals are never resurrected (fixes 2). Alias is never merged in migration. A keyed-but-partial or protected identity is recovered instead through the interactive load, which has the identity password. Reverts the shared `QualifiedIdentity::merge_gaps_from` extraction: the gap-merge is a load-path-only tool (safe only with a user present), so it goes back to the private `merge_existing_keys_into` in load_identity. Migration carries its own bare-record reconcile. Tests: `a_present_but_bare_identity_gains_the_legacy_only_keys` (bare record recovers the legacy key + owner association) and `a_present_keyed_identity_is_left_untouched_never_reconciled` (a keyed record is skipped — no downgrade, no resurrected alias/key). Design doc §7 updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(migration): revert legacy-identity reconcile to safe skip-if-present Reconciling legacy-only keys into an already-present identity cannot be done safely without provenance the model does not carry: field absence is indistinguishable from a deliberate user removal ("Remove private key from DET" leaves no tombstone; a cleared alias persists as None), so a blob-first merge would resurrect removed keys or aliases. Merging a plaintext legacy key into a protected identity would additionally trip the vault-first IdentityKeyProtectionDowngrade guard and fail the whole pass. Revert migrate_identities_from_conn to the original skip-if-present body: an identity already in the store is skipped wholesale, never re-persisted. Restore has_local_qualified_identity (presence probe, no decode) as the skip check. Drop the reconciled counter, the get_existing/update closure seams, and the reconcile-specific tests. No data is lost: the legacy data.db is preserved verbatim, so a bare (partially-loaded) identity's stranded keys remain recoverable by a future provenance-aware flow. Document the stranding as a known limitation in the design doc (§7) and track the recovery flow as a follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WVUVpGxaAsciGSiT7t4kuC * fix(migration/wallet): QA follow-ups for #885 — resurrection, banners, 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> * docs(migration): mark the legacy-identity design as shipped and name 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. * build(deps): document the deliberate bincode pin (RUSTSEC-2025-0141) 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. * fix(shielded): let the Verified badge name the balance it vouches for 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. * test(migration): extract the legacy-identity fixture into database::test_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). * docs(legacy-import): state precisely what read_identities logs 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. * fix(migration): name the Identities screen in the unreadable-identity 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. * fix(migration): publish a terminal state for every migration failure `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. --------- Co-authored-by: Lukasz Klimek <lklimek@gmail.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com> Co-authored-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com>
lklimek
added a commit
that referenced
this pull request
Jul 14, 2026
* docs(migration): design the v0.9.3 legacy identity import
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 b14bf32c had already moved and a version-byte
agreement that is not needed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(migration): import legacy v0.9.3 identities and their keys
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>
* fix(migration): never let a corrupt vote queue strand identity keys
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>
* fix(migration): honor legacy alias fallback and guard identity-import 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>
* fix(ui): replace uncovered symbol glyphs with proven-working characters
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>
* fix(masternodes): label rotated evonode payout keys distinctly
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>
* fix(fonts): vendor Noto Sans Symbols2 as an icon-glyph fallback
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>
* fix(masternodes): remove Top up and Transfer from the detail screen
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>
* fix(wallets): list addresses funded past the BIP44 bootstrap window
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>
* fix(wallets): show Shielded tab so tracked shielded funds are viewable
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.
* fix(withdrawal): skip disabled keys when selecting a withdrawal signing 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
* fix(ui): make interface-mode indicator role-aware and consistently named
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
* refactor(wallet): audit legacy address-map retirement; pin resolver invariant
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
* fix(withdrawal): forbid owner-key withdrawals to a non-payout address
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
* fix(migration): column-authoritative alias, per-row identity decode, 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>
* fix(tools): hide the ZK Proofs entry from the Tools menu
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
* fix(nav): hide legacy Identities and Dashpay tabs, rename Identity Hub 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
* fix(masternodes): vertically center hash/copy/badge in detail header row
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
* fix(masternode): align key labels with Dash Core DIP-3 ProTx terms
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
* fix(onboarding): land "Just Explore" on Identities hub, not DashPay profile
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
* fix(masternodes): clarify DPNS contest voting on the node detail screen
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>
* fix(migration): surface unreadable votes alongside unreadable identities
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>
* fix(masternodes): explain each V/O/P role letter on hover
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>
* fix(masternodes): keep load form open on error, lock submit while loading
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>
* test(identity-selector): wire wallet backend synchronously in make_ctx
The identity_selector unit tests built their AppContext via AppState::new(),
which spawns wallet-backend wiring in the background. That async init runs
restore_selected_identity_from_kv(), which — reading the empty k/v of a fresh
temp dir — writes None into the in-memory selected_identity_id mutex. When that
restore landed AFTER a test's set_selected_identity(Some(id)), it clobbered the
selection to None, failing syncing_global_writes_selection_to_app_context at its
precondition (left: None, right: Some(id)). The window is a few instructions
wide, so it surfaced only under CI's oversubscribed scheduling — a false
failure, not a code regression.
Build the context deterministically instead, mirroring context::tests::
offline_ctx: construct AppContext directly and .await ensure_wallet_backend to
completion, so the one-time restore settles before the context is returned and
no background task can race a later set_selected_identity. A block_in_place
drives the async wiring from within the entered multi-thread runtime.
Verified: a temporary sleep-probe reproduced the exact CI signature RED, then
went GREEN under this fix; all four identity_selector tests pass; clippy clean.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(migration): reconcile legacy keys into partially loaded identities
The identity import skipped a legacy row wholesale whenever the id was
already in the modern store. But presence is not proof every key survived:
before this PR, a masternode could be loaded from only its ProTxHash
(voting/owner/payout keys all optional), persisting a partial key map. When
such an install upgrades, the legacy blob may still hold owner/voting/payout
keys the modern record lacks — and the wholesale skip stranded them. The
skipped row was not counted unreadable, so the sentinel landed and those
legacy-only keys never reached the vault or got retried: a silent loss of a
masternode's control keys, not just a banner glitch.
The importer now fetches the existing modern identity and gap-merges the
legacy blob into it: the modern record stays authoritative (its keys, alias,
protection state, and wallet link always win) and only the keys/associations
it lacks are taken from the blob. It re-persists in place via
update_local_qualified_identity only when the merge actually recovered
something (new `reconciled` counter); an identical record is left untouched,
so a retry can never overwrite a user edit with the stale legacy copy.
The gap-merge is the same "keep what I have, borrow only what I'm missing"
rule load_identity already used for in-place key adds; that private helper is
promoted to QualifiedIdentity::merge_gaps_from (model/, single source) and
reused by both callers. Regression test
`a_present_but_partial_identity_gains_the_legacy_only_keys` stages a partial
modern identity plus a legacy blob carrying an extra Owner key and asserts the
key is merged in and the record re-persisted once; the existing
already-in-store test now proves an identical record is not re-written. Design
doc §7 edge-case table updated to match.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(qa-887): close three thepastaclaw blockers on PR #887
Address the three validated blockers from thepastaclaw's review of #887.
1. Wallet address table (address_table.rs): a funded address with no known
derivation path was stored with an empty DerivationPath. Every row still
offered "View Key", and deriving at an empty path returns the BIP-32 master
private key — mislabelled as that address's key. Preserve the missing-path
state explicitly (Option<DerivationPath>, None for unknown) and disable key
export for pathless addresses.
2. Shielded tab (shielded_tab.rs): making the Shielded tab always visible also
exposed its Shield / Send (Private) / Unshield buttons, which open preset
send flows that never evaluate FeatureGate::ShieldedOperations. Gate the
action controls behind ShieldedOperations; balance, address, and note viewing
stay available, with an explanatory notice when operations are unavailable.
3. Identity withdrawal (withdraw_from_identity.rs): callers pass id = None, so
the SDK ran its own TransferPreferred selection, which can sign with a
disabled key or fall back to an OWNER key and bypass the owner-address policy.
An explicit id could also be disabled after the pre-withdrawal refresh. Add
QualifiedIdentity::resolve_withdrawal_signing_key to resolve one active
TRANSFER-or-OWNER key the local signer can use against the refreshed identity,
reject missing/invalid explicit ids, and pass that key to both
resolve_withdrawal_output and the SDK call.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(migration): only reconcile bare identities, never keyed ones
The e8b61821 reconcile filled a present identity's gaps from the legacy
blob by inferring "missing" from field absence. Two ways that is unsafe for
a background migration (both raised in review):
1. Protection downgrade — merging a legacy `Clear` key into an identity whose
other keys are password-protected produces a mixed record. On save,
`encode_identity_blob_vault_first` refuses it with
`IdentityKeyProtectionDowngrade`; the migration then errors before writing
its sentinel and fails identically on every launch.
2. Resurrected removals — absence is not proof of a partial load. "Remove
private key from DET" deletes a map entry and clearing an alias persists
`None`. On a pre-sentinel install (or a retry held open by another
unreadable row) the merge would refill those intentional absences from the
stale blob, restoring a removed alias or re-adding a deliberately-removed
private key.
Fix: reconcile only a record that holds NO private keys at all — the one
unambiguous "loaded without its keys" signal (the ProTxHash-only masternode
load). For a bare record, take the legacy key set and fill the missing
masternode role associations; re-persist only when something was recovered.
Any record that already holds keys is left untouched: a protected identity
always holds keys, so it never reaches the vault-first guard (fixes 1), and a
keyed record's absent field is never refilled, so removals are never
resurrected (fixes 2). Alias is never merged in migration. A keyed-but-partial
or protected identity is recovered instead through the interactive load, which
has the identity password.
Reverts the shared `QualifiedIdentity::merge_gaps_from` extraction: the
gap-merge is a load-path-only tool (safe only with a user present), so it goes
back to the private `merge_existing_keys_into` in load_identity. Migration
carries its own bare-record reconcile.
Tests: `a_present_but_bare_identity_gains_the_legacy_only_keys` (bare record
recovers the legacy key + owner association) and
`a_present_keyed_identity_is_left_untouched_never_reconciled` (a keyed record
is skipped — no downgrade, no resurrected alias/key). Design doc §7 updated.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(masternodes): reconcile load form on arrival instead of blindly unlocking
thepastaclaw flagged a lifecycle regression in the masternode load form
(commit d99b713c): `refresh_on_arrival` unconditionally cleared `load_in_flight`
on every return to the tab. Because task results reach only the visible screen,
a load dispatched then left (tab switched mid-load) never cleared the gate
through `display_task_result`/`display_task_error`; the backstop cleared it
blindly, which:
- re-enabled the still-open form while the load was genuinely pending, letting
it dispatch a second concurrent LoadIdentity that races the non-atomic
RejectIfExists existence check (last write wins, clobbering alias/keys), and
- left an enabled stale form after a load that completed while another screen
was visible, so resubmitting reported a duplicate instead of showing the node.
Reconcile against the local store instead: on arrival, if the node being loaded
is now present the load finished — close the form and clear the gate; if it is
absent the load is still pending (or failed away) so keep the gate locked. The
target id is parsed from the ProTxHash the same way the backend resolves it
(Base58 then hex), so it matches the id the node is stored under. Cancel now
also clears the gate so an abandoned form can never leave `+ Load` disabled.
Adds MasternodeLoadForm::target_identity_id and two regression tests that drive
the navigation route (store reconciliation) the prior direct-callback test did
not cover.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(migration): revert legacy-identity reconcile to safe skip-if-present
Reconciling legacy-only keys into an already-present identity cannot be
done safely without provenance the model does not carry: field absence is
indistinguishable from a deliberate user removal ("Remove private key from
DET" leaves no tombstone; a cleared alias persists as None), so a blob-first
merge would resurrect removed keys or aliases. Merging a plaintext legacy
key into a protected identity would additionally trip the vault-first
IdentityKeyProtectionDowngrade guard and fail the whole pass.
Revert migrate_identities_from_conn to the original skip-if-present body: an
identity already in the store is skipped wholesale, never re-persisted.
Restore has_local_qualified_identity (presence probe, no decode) as the
skip check. Drop the reconciled counter, the get_existing/update closure
seams, and the reconcile-specific tests.
No data is lost: the legacy data.db is preserved verbatim, so a bare
(partially-loaded) identity's stranded keys remain recoverable by a future
provenance-aware flow. Document the stranding as a known limitation in the
design doc (§7) and track the recovery flow as a follow-up.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01WVUVpGxaAsciGSiT7t4kuC
* fix(masternodes): gate the load on the submitted node, not the live form
thepastaclaw flagged two races in the reconcile-on-arrival fix (a77aa6c0).
Both come from the same root cause: nothing anywhere knew whether a dispatched
load was still running. Task results reach only the visible screen, loads have
no cooperative cancellation, and `RejectIfExists` checks the store, then fetches
from the network, then inserts — three steps, not one atomic one.
- The gate keyed off the form's CURRENT ProTxHash. Only the Load button locks
while a load runs, so retargeting the still-open form at an already-loaded node
read as "the submitted load finished": the form closed, the gate cleared, and
the original node could be loaded a second time, concurrently.
- Cancel cleared the gate although it cancels nothing — the load keeps running in
its detached task. Reopening the form and resubmitting the same ProTxHash let
both loads pass the existence check before either inserted.
Make the load task claim its identity for the whole check -> fetch -> insert
span (`AppContext::begin_identity_load`, released by an RAII guard on every
return path). A second load of that identity — from any screen, tool or CLI — is
now rejected up front with `TaskError::IdentityLoadInProgress` instead of racing.
That claim is also the only truthful answer to "is it still running": a *failed*
load leaves no trace in the store, so reconciling against the store alone cannot
tell failure from progress and would strand the gate, disabling `+ Load` for the
rest of the session.
The screen now gates on the identity it actually submitted, parsed with the
model's `decode_identity_id` (the same decode the backend uses — the form's
duplicate copy is gone). Cancel dismisses the form and holds the gate; arrival
releases it once the backend reports the load done, closing the form only if the
node really landed and otherwise leaving every field intact for a resubmit.
Six regression tests drive the routes: submitted-vs-edited target, Cancel with a
live load, failure-while-away, another identity's result, plus the registry's
own exclusion.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(masternodes): have the load report its phase instead of inferring it
thepastaclaw found two more holes in the load gate (07d50eeb), both from the
same shape of mistake: the screen inferred a load's lifecycle from proxies
instead of being told it.
- Registry-claim absence was read as "finished". But a load is outstanding from
the moment it is dispatched, and only claims its identity once the task starts
and validates its input. Arriving inside that window released the gate on a
load that was genuinely still in flight, so its eventual success could no
longer close the form.
- Node-in-the-store was read as "succeeded". But `load_identity` inserts the node
BEFORE sealing its keys, so a failed seal leaves the node persisted by a load
that errored. Arrival then closed the form as a success and discarded the
user's retry state, with the keys unprotected or partly protected.
The lifecycle has more states than either proxy encodes, so make it explicit and
let the task report it: `IdentityLoadPhase` (Submitted → Running → Loaded |
Failed), recorded in the AppContext registry. The screen marks Submitted
synchronously at dispatch — nothing else can, before the task exists — and the
task's guard records the terminal phase on drop: `Failed` on every `?` and on a
panic, `Loaded` only when the task explicitly reports it after its last fallible
step. `Running` for an identity can only be one guard's own claim, so a guard
whose record a newer load superseded never writes over it.
`reconcile_pending_load` now settles purely on that phase, and is the single
place the screen decides a load is over — `display_task_result` and
`display_task_error` delegate to it rather than each re-deriving the answer.
A failed load keeps its form and every field in it for a corrected resubmit,
whether or not it managed to persist the node first.
Five regression tests drive the routes the proxies got wrong: arrival before the
task claims, arrival after a failure that had already persisted the node, plus
the registry's own phase transitions and supersede rule.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(masternodes): stamp every load record with the load it belongs to
thepastaclaw found two more on 0d01a198, both about *which* load a registry
write belongs to.
`mark_identity_load_submitted` overwrote the identity's record unconditionally,
including a `Running` one. Loads are dispatched from several places — the
Masternodes form, Add Existing, the detail screen, MCP tools — so submitting a
node another caller was already loading erased that caller's claim, let a second
task claim the same identity, and the two then raced each other's non-atomic
storage writes. Guards were not correlated to an operation either, so whichever
guard dropped while the record read `Running` published its outcome over the
other load's.
Stamp each record with an `IdentityLoadToken` naming the one load it belongs to,
and check that stamp on every write. A submission for an outstanding identity now
takes no token and disturbs nothing — the dispatch comes back with
`IdentityLoadInProgress` and the banner explains it. A guard reports only onto
its own record, so a superseded load can never publish over the load that
replaced it, and its token stops resolving: its outcome is no longer observable,
which is exactly what the screen needs to know.
The claim also moves ahead of input validation in `load_identity`. A short
password or malformed key returned via `?` before the guard existed, so the load
reported no outcome at all while the screen's `Submitted` mark said otherwise —
form and toolbar stuck on "Loading…" for the rest of the session, Cancel
included. Parse the identity id (which names the load), claim it, then validate:
every fallible step is now inside the guard's span.
The third finding on that commit (a persisted node read as success) was already
closed by 0d01a198 and could not be reproduced: `reconcile_pending_load` reads
only the phase, and `arrival_does_not_read_a_persisted_node_as_a_successful_load`
covers a persisted-then-failed load.
Three regression tests: a submission that must not erase an active claim, a
validation failure that must still report a terminal phase, and a superseded
load whose token stops resolving.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(masternodes): let a load claim only the record it was dispatched under
A load adopted any `Submitted` record it found for its identity, whoever had
opened it. Only the Masternodes form marks its load submitted; Add Existing, the
detail screen and the MCP tool dispatch straight to the task. A load from one of
those three, of the same node, took over the form's record and published ITS
outcome under that token — the form closed reporting success while the keys and
password the user actually submitted were discarded with the load that never ran.
A load now carries the token it was dispatched under (`IdentityInputToLoad::
load_token`) and claims only the record stamped with it. A record opened by
anyone else is outstanding work, not this load's to adopt: the claim is refused
with `IdentityLoadInProgress`, which the banner already explains.
That refusal makes a stranded ticket worse than it was — an outstanding record
now blocks every entry point, not just the form's gate. Two gates in
`run_backend_task` (terminal storage-open, cold-start migration) return before a
wallet-touching task reaches `load_identity`, so the load never claims its
identity and never reports a phase, leaving the record `Submitted` forever. It is
now backstopped for the whole task: a record still `Submitted` under its token
when the task ends is recorded `Failed`, so the user gets a retry instead of a
node stuck on "Loading…" for the session. Being a guard, it also covers a task
dropped or panicking before its claim — not just today's two gates. A load that
did claim reports its own outcome; the backstop leaves that record alone.
Refs: #887
* fix(wallet): refuse root-key derivation at the chokepoint, not at a button
The empty derivation path IS the BIP-32 root, so deriving there hands back
the wallet's master key instead of an address key.
`with_wallet_derived_key` accepted it unvalidated; the only thing standing
between the master key and an export was a disabled UI button on one of the
two callers. `SignMessageWithKey` shares the same seam and carried no gate
at all.
Enforce the invariant where it belongs: the chokepoint now rejects an empty
path with the typed `TaskError::RootKeyDerivationRefused` before the seed is
ever fetched from the vault. It is the sole production route to
`private_key_at_derivation_path_with_seed`, so every present and future
key-bearing wallet task inherits the guard.
The regression test was confirmed RED against the pre-fix code: the empty
path returned `Ok` with the master key derived and handed to the caller. A
positive control pins that real BIP-44 paths still derive.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallets): stop inventing metadata for addresses with no derivation path
`combined_address_paths` goes to the trouble of returning
`Option<DerivationPath>` to say "path unknown" honestly, and the row builder
threw that away, substituting an empty path and tracking the truth in a
parallel `has_known_path: bool`. Only the View Key button read the bool.
Every other cell rendered the placeholder as fact: the Type column matched no
`is_bip44_*` predicate and printed "System", the Index column took `.last()`
on an empty path and printed 0, the Full Path column printed a bare "m". A
funded address of unknown provenance was displayed as a confident
"System, index 0, path m" — every field of which was fabricated.
Make `AddressData.derivation_path` an `Option<DerivationPath>` and delete
`has_known_path`: the `Option` is now the single source of truth, so the
unknown-path state cannot be read past a `match`. Type and Full Path render
"Unknown", Index renders blank, and the row buckets as `Other(Unknown)` —
the same account `collect_account_summaries` already totals it under. The
View Key gate becomes `derivation_path.as_ref()`, type-enforced rather than
boolean-tracked.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(masternodes): bind entered key lifetime to the tab that holds them
The Masternodes tab is a root screen: it lives in AppState.main_screens for
the whole process. Its load form clones rather than drains its secret fields
so a failed load can be corrected in place, and the detail view's "Add voting
key" prompt holds a WIF until submitted. Neither is dropped when the user
navigates away, so plaintext owner/voting/payout keys and the at-load
encryption password stayed resident for the rest of the session, with nothing
to clear them on tab-away, idle or timeout.
Give ScreenLike an on_leave hook — the counterpart of refresh_on_arrival —
and fire it from the one place the selected root screen changes, the forced
de-gate on a role demotion included. The Masternodes screen answers it by
zeroizing every secret the open view holds.
The clear is unconditional, in-flight load or not: the submitted input already
travelled with the task, and a load that fails while the user is elsewhere is
precisely the case that would otherwise strand keys on a screen nobody is
looking at. Keys and password go together — dropping the password alone would
leave the form one click from storing the retained keys unencrypted, whereas
with no keys left a resubmit loads the node read-only, which the form already
supports. What survives is what is tedious to retype and secret to nobody:
ProTxHash, alias, node type. The form says so rather than losing pasted keys
in silence.
Retention while the tab is open is unchanged: a failed load still keeps every
field for a corrected resubmit.
* fix(withdrawal): lock the owner-key destination to the payout address at every role
MN-007 and TC-FR9-06 both promise that a withdrawal signed with a masternode
owner key has its destination fixed to the node's registered Core payout
address, "not user-editable". The address field was editable anyway: the guard
read `!is_owner_key || user_role().at_least(Power)`, and the Masternodes page —
the only route to this screen with an owner key selected — is itself gated at
`MinRole(Power)`. So every user who could reach the Withdraw button satisfied
the `|| Power` escape clause, and the field was ALWAYS a free-text TextEdit,
even for an owner key. The backend's `OwnerKeyWithdrawalNotAllowed` guard only
fired after the user typed an address, confirmed, and submitted.
Drop the role escape: an OWNER purpose on the selected key locks the field at
every role, showing the forced payout destination instead. Also clear any
address typed under a previously selected key, so switching to the owner key in
Advanced Options cannot leave a stale destination the owner key can never pay;
the reconciliation runs before the Withdraw button each frame, so a key switch
settles before a click on it is handled. `resolve_withdrawal_output` stays as
backend defense-in-depth.
Kittests: the destination is locked for an owner key at Everyday, Power AND
Developer (the pre-fix code passed at Everyday and failed at Power — precisely
the reported bug), and the mirror case, a transfer/payout key, keeps its
free-text field (TC-FR9-07) so the lock does not over-reach.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(nav): route both root-screen fallbacks to the Identities hub
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 (965f35d7),
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>
* refactor(wallet): source send-autocomplete from the snapshot, not the 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>
* fix(masternodes): pass None to begin_identity_load in secret-residency test
The registry-token-threading fix (d038d1ac) added a required
`Option<IdentityLoadToken>` parameter to begin_identity_load. The
secret-residency fix (8baa6f41) 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>
* fix(masternodes): claim the outstanding-load test via its own dispatched token
The previous fixup (b3a2fc64) 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>
* fix(identity-hub): stop DashPay profile flicker during sync
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>
* fix(masternodes): sort the node grid by name and surface each node's 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>
* fix(migration/wallet): QA follow-ups for #885 — resurrection, banners, 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 sen…
lklimek
added a commit
that referenced
this pull request
Jul 14, 2026
… migration fix, nav pills, disclosure closure (#882) * docs(user-stories): add missing entries surfaced by v1.0 parity audit Additions only (no edits/removals) covering gaps found while auditing v0.10-dev feature parity against PR #860 (DPNS, network/settings, UX, masternodes, DashPay send/receive, wallet). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * refactor(network-chooser): drop dead dashmate_password_input field The RPC/Dash-Qt backend mode is gone in the SPV-only rewrite, but NetworkChooserScreen still carried a `dashmate_password_input` that was constructed, seeded from disk at startup, and re-seeded via a synchronous `Config::load_from` on every network switch — while never being rendered anywhere. Delete the field and its disk-read plumbing, along with the now purposeless `prev_network` sentinel that existed only to trigger the re-seed. Removes a blocking file read from the network-switch UI path. `NetworkConfig::core_rpc_password` is left intact: it still round-trips through the `.env` serializer in `config.rs` (settings-schema scope). A10 (expert-mode nav refresh) is deferred: PR #879 (UserRole + composable FeatureGate) is still open and reworks this exact mechanism, and #880 stacks on it. Investigation found the nav-refresh bug already fixed on this base by #876 — every AppContext construction path shares one `Arc<AtomicBool>` developer-mode flag, the nav gate re-reads it each frame, and the Masternodes screen is always registered — so the existing comment describes present behavior correctly and needed no edit. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: close v1.0 parity audit disclosure gaps (group 8) Adds the missing paper trail for three undisclosed removals (Masternode List Diff screen gravestone + CHANGELOG + gaps.md row; shielded per-note detail CHANGELOG line; three doc sites amended to disclose the QR-removal notice via CHANGELOG instead of an unshipped in-app notice), strengthens two under-described disclosures (address-table column, Proof Log persistence+viewer loss), and formally signs off ten already-disclosed removals in a new closure record. * fix(tokens): keep dismissed token balances out of refresh watch sets "Stop Tracking Balance" was undone by "Refresh My Tokens": the refresh re-registered the full known-token registry for every local identity, so a dismissed (identity, token) pair was re-watched upstream and its row came back. Upstream owns the watch set in memory only, so the dismissal has to be persisted and re-applied DET-side. Persist dismissed pairs in the per-network k/v store under det:token_untracked:v1 and rebuild each identity's watch set as "local registry minus that identity's dismissals". Re-tracking stays possible through the paths the UI already promises: re-importing a token clears its dismissals for every identity, and explicitly checking one balance clears just that pair. Removing a token from the registry, and the devnet sweep, prune the dismissal list too. Regression test drives the real user action against an offline wired context (stop tracking, then assert the refresh watch set) and was confirmed RED before the fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(shielded): restore shielded receive-address view and copy Users could not view or copy their own shielded receive address, so they could not receive a private transfer at all: the Shielded tab rendered only a placeholder because the live address read is async-only and the egui frame loop is synchronous. Bridge it through the push-snapshot seam this codebase already uses for shielded/platform balances rather than inventing a new one: - AppContext::shielded_addresses — frame-safe snapshot, written on the async backend side by cache_shielded_receive_address() right after ensure_shielded_bound() in bootstrap_wallet_addresses_jit (the seam reached from both cold boot and the unlock gesture), read each frame via the synchronous shielded_receive_address(). - Evicted on wallet removal: a receive address is a payment destination and must not outlive the wallet that owns it. - model::address::encode_shielded_address() — the pure raw->bech32m inverse of parse_shielded_recipient; the MCP tool now shares it. - Shielded tab renders the address with a hint, hover-for-full, and copy on either the address or the Copy button; the truncation is display only and the clipboard always receives the full string. Funds safety: the address comes from the upstream-owned key slot (PlatformWallet::shielded_default_address), i.e. the same OrchardKeySet that bind_shielded registered with the NetworkShieldedCoordinator as the viewing keys it scans with. It is never re-derived DET-side, so a displayed address is always one the wallet can detect notes for. It is Orchard account 0 — the only account DET binds and the only one its spend path (shielded_transfer(.., 0, ..)) can spend from. Diversified-address generation ("+") stays out of scope: upstream exposes no per-index accessor (OrchardKeySet::address_at is reachable only via the crate-private shielded_keys slot). Deriving them DET-side would duplicate Orchard key handling outside the coordinator seam, and mapping "+" onto a new ZIP-32 account would strand funds in an account the single-account spend path cannot spend from. Documented as a TODO and narrowed in WAL-028. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(identity-hub): wire Contacts actions, real contact list, alias, and pay The hub's Contacts tab rendered Accept / Decline / Cancel buttons that did nothing, hardcoded "Active contacts · 0", and offered no way to pay a contact or rename an identity without a detour through the retired legacy screens. - Accept / Decline now dispatch AcceptContactRequest / RejectContactRequest. - Cancel gains a backend task. A DashPay contactRequest document is immutable and undeletable (documentsMutable: false, canBeDeleted: false), so a sent request cannot be withdrawn from Platform. CancelContactRequest therefore re-verifies state, broadcasts a hidden contactInfo document, and records the withdrawal in the DET sidecar — the same shape reject_contact_request uses. The UI copy says so plainly instead of promising a withdrawal the protocol cannot deliver. - load_contact_requests now consults the sidecar, so a declined or cancelled request actually leaves the list instead of reappearing on every reload. - Active contacts render from LoadContacts, with a working search box. - Settings tab gains a local alias ("Name on this device") editor. - Contact rows gain a Pay affordance that opens the existing send-payment screen — no new signing or broadcast logic. Contacts-tab state moves to ui/state/contacts_view.rs per the DET module placement policy (it renders no egui). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(dashpay): clear a stale rejection marker when re-adding a contact Declining a request wrote a permanent local marker, so a request from that person stayed filtered out of the list forever — even after the user deliberately added them again. Sending a contact request now retires the marker, since sending is an explicit re-engagement. Also documents the two new hub stories and the cancel capability in the user-stories catalog. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(migration): import settings, scheduled votes and top-ups from legacy data.db Upgrading from v0.10-dev booted the app with a blank configuration: the network reset to Mainnet (a testnet user relaunched straight into mainnet), theme/onboarding/paths reset, and scheduled DPNS votes were silently dropped — a real vote-window deadline risk for masternode voters. Three imports, all idempotent and sentinel-guarded: - Settings (network, start screen, theme, onboarding, Dash-Qt path, toggles) are imported in `AppState::new_inner` *before* the settings blob is read, because that read is what selects the active network. It runs synchronously there — no AppContext exists yet. The import overwrites an existing blob: until now, an upgrading user's first launch wrote a `default()` blob over their real preferences, and skipping on "a blob exists" would make that reset permanent. The sentinel, not the blob, is the guard. - Scheduled votes and top-up history are imported by `finish_unwire` under their own per-network sentinel, ahead of the wallet-drain gate: an install that already drained its wallets under an earlier build still has these rows in data.db, and a shared sentinel would declare it "done" and strand them. Votes already in the k/v store are left alone so a retry cannot push a stale `executed = 0` over a vote the user has since cast. An undecodable vote row fails the pass (banner + "Retry now") rather than vanishing silently. `scheduled_votes` and `top_up` join the detection gate: a masternode voter who imported identity keys directly has queued votes but no wallet rows at all. The app-data pass probes those tables before reaching for the wallet backend, so an install with nothing to import still completes without it. Readers live in `database/legacy_import.rs` (typed, counters only, no policy); the "what to do on failure" decision stays in `backend_task/migration`. Legacy rows are never deleted. The v0.9.0 ladder fixture now carries a vote, a top-up and settings, asserting they survive the full v5 → current migration. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(wallets): remove dead RPC-mode gate on single-key send, surface limitation in-app Single-key (imported WIF) send and balance/UTXO monitoring remain blocked on upstream platform-wallet. This lands the honest, user-facing half and corrects the record on what upstream actually needs. Feasibility (platform-wallet 44c20e3 / key-wallet 48a07d3): the SPV watch set is the union of every managed account's address-pool addresses, and balances/UTXOs come from the funding accounts, so a single imported P2PKH address WOULD be monitored once it sits in a registered wallet's pool. key-wallet can already build such a pool without derivation (AddressPool::new_without_generation + AddressInfo + KeySource::NoKeySource). What is missing is a way to REGISTER it: PlatformWalletManager::register_wallet is private, the public constructors all require an HD seed, and the inner WalletManager (which does expose a public insert_wallet) is reachable only via PlatformWallet::wallet_manager() — i.e. only when a wallet is already registered, so a single-key-only user has no handle at all. Unblocked by a public seedless register_watch_only_wallet. Changes: - Drop the `is_rpc_mode` gate (hardcoded false; RPC mode no longer exists in this SPV-only build) from the single-key detail view and send screen. - Detail view: Send is explicitly disabled, with the reason and the recovery-phrase workaround in a persistent banner and the button tooltip. - Wallets action bar: selecting a single-key wallet no longer routes into a send screen that could only refuse the payment — it states the limitation. - Send screen: no UI gate; the backend stays the authoritative enforcement layer and refuses with the typed TaskError::SingleKeyWalletsUnsupported. - Correct the stale TODOs in core/mod.rs: the previously-assumed key-wallet single-address pool helper is NOT required; only the upstream registration entry point is. Refresh is not re-enabled as a button — monitoring is meant to be automatic, so that task should be deleted once upstream lands. - Tests: lock the user-facing copy contract (states the limitation, names a self-serve action, no jargon) for both the UI copy and the typed error. - user-stories.md: WAL-030 restated as automatic monitoring (no refresh control) and SND-002 updated; both stay [Gap] with the real blocker named. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(nav): wire the masternode and Wallets global-nav pills Moves two more pills of the FR-GLOBAL-NAV staged rollout from subdued/read-only to fully interactive, and removes a dead click sensor. Masternodes (MN-012, FR-GLOBAL-NAV-3): the page-scoped node pill is now an interactive dropdown of every loaded masternode/evonode, two-way bound with the page — opening a card names that node on the pill, picking a node from the pill opens its detail view. The pill's label follows the node's card heading and its glyph follows the node type (HeroIdentityKind::type_glyph), so the grid and the breadcrumb never name a node differently. The selection stays page-scoped: it maps to SelectPageObject, never SelectIdentity, so a masternode can never become the app-global identity (FR-6). Wallets (FR-GLOBAL-NAV-2 rule 2): the wallet pill is interactive and two-way bound — switching on the pill selects that wallet on the page, and the page's own selection is what the pill reads back. Arrival now adopts a wallet switched from another page's pill, ahead of the first-wallet default that would otherwise silently overrule it. Connection indicator: the click sensor is downgraded to hover-only; the tooltip is its whole interaction. Supporting changes: PageObjectItem carries a type glyph; PageScopedObject carries page-owned tooltip copy, keeping page wording out of the shared component; add_top_panel_with_global_nav_capturing returns the raw GlobalNavEffect so any page can mirror the selection it consumes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(dashpay): narrow the cancel race window and add a way back for hidden contacts Cancel could hide a contact that was established mid-flight: the reciprocal check and the contactInfo broadcast are separate Platform round-trips, and a request arriving in between was never noticed. - Restructure cancellation as `cancel_flow` over a `CancelOps` trait: the reciprocal check is the last read before the write, and a second read right after the broadcast detects a reciprocal request that landed inside the window and undoes the hide, leaving the new contact visible. The trait makes the ordering unit-testable — the race is injected between the two probes. - Add a "Show hidden contacts" section to the Hub Contacts tab with a per-row Unhide (contactInfo broadcast with display_hidden cleared, nickname and note preserved), so a hidden contact is never unreachable from the Hub. - Share one contact-search matcher between the Hub and the legacy DashPay contacts screen, which had drifted onto different field sets. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(user-stories): record the hidden-contact recovery path in DPY-009 Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(ui,tokens): re-disable single-key send, ungate DashPay pay buttons, type the token-dismissal seam Three converged QA findings from the v1.0 parity batch. single-key send screen: the deleted `is_rpc_mode` gate had been the only thing disabling the Send button, so the screen shipped an enabled Send that dispatches a task `CoreTask::SendSingleKeyWalletPayment` always refuses. Its `display_message` only cleared the busy flag on success-shaped text, so the button would also have stuck on "Sending..." forever on that refusal. The screen has no live route today, but the backend handler's TODO names it as the parked send UI to re-point once upstream lands seedless registration, so it is kept and made safe rather than deleted: Send is disabled with the same copy and disabled-hover text as the wallets action bar, every dispatch goes through one choke point that arms the busy flag, and any task result clears it. Regression tests cover the refusal, the arming, and the fee-retry dialog. DashPay: the Identity Hub's "Pay a contact" button was ungated on the premise that no other send flow is dev-gated. It was — the contacts list, contact details and profile viewer all gated the same `DashPaySendPayment` screen behind developer mode, with a stale comment claiming it "requires SPV which is dev mode only" (SPV is the standard backend now). Ungate all three to match, and flag the four entry points for explicit role classification when the UserRole/FeatureGate rework (#879) lands. tokens: the dismissal API took `(identity, token)` at one seam and `(token, identity)` at the next, both bare `Identifier`s — a transposition would have compiled and un-tracked the wrong pair. Thread the existing typed `IdentityTokenIdentifier` through instead. The on-disk payload keeps its `(token_id, identity_id)` layout, now pinned by a test. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs: drop stale Resync references from shielded_tab doc comments Resync/Sync buttons were removed (net-improvement automatic sync); two doc-comments still described the removed action. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix(migration): stop a corrupt vote row from blocking the wallet drain `finish_unwire::run` imported scheduled votes first, unconditionally, and propagated the vote-row failure with `?`. The vote importer is fatal on an unreadable row by design, so a single corrupt legacy `scheduled_votes` row wedged the wallet-seed migration on every launch and every "Retry now" — the row is never deleted and that path has no Skip. A user with funds behind a bad vote row could never reach their wallet again. Decouple the two passes. `run` now holds the app-data result, runs the wallet drain (extracted into `drain_wallets`) regardless, and only judges the app-data outcome once funds are reachable. Undecodable vote rows become a per-row skip-and-count instead of a migration-fatal error: they are surfaced on the new terminal `MigrationState::SucceededWithUnreadableVotes`, which raises a sticky Warning banner naming the recovery action, with no dead-end retry. The app-data sentinel is written once every *importable* row is handled — withholding it would re-run the import each boot and resurrect votes the user has since cast and cleared. Hard app-data failures (unreadable file, k/v write) stay fatal and still leave that sentinel unwritten, but no longer gate the drain. Both invariants hold: the vote sentinel still runs ahead of the wallet-drain gate (identity-only voters keep their import), and no vote is lost in silence — the legacy rows survive in `data.db` and the count reaches the user. Tests: an end-to-end `run()` over a fixture with real wallet rows AND a corrupt vote row proves the wallet lands hydrated + upstream-registered while the bad row is counted (RED before this change: MigrationFailed/ScheduledVotesUnreadable). The tautological TC-MIG-009 sentinel test is replaced by one that calls `run()` twice on the same `AppContext` and pins that the second launch re-fires nothing, including no vote resurrection after the queue is cleared. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * refactor: consolidate six duplicated code paths from the DRY audit Every duplicate below had two or more implementations of one rule, which is how the two identity-ID shorteners silently drifted apart. Identity labels — `display_label` gains the DashPay display-name tier and is now the one resolver for the hub-wide priority rule (nickname -> display name -> DPNS handle -> shortened id). `contact_label` delegates to it and the divergent `abbreviate_id` is gone. User-visible change (intentional): a profile-less contact rendered as `US517G59…` on the Contacts tab and `US517…LFx` in its identity pill — the same identity, two spellings. Both surfaces now use `shorten_id`. Covered by a test that fails against the old code. DashPay — the `toUserId` extraction (5 sites) moves to `model::dashpay::contact_request_recipient`, alongside the existing `model::dpns` document-extraction precedent; the `contactRequest` `DocumentQuery` builder (11 sites) moves to a private `dashpay::contact_request_query`. The hand-rolled `Value::Identifier` pattern-matches are replaced by the same typed accessor the rest of the module already used, so the mutual-contact filter and the resolved-request filter can no longer disagree about what a document's recipient is. Database — `table_exists` / `column_exists` become the single schema probe in `database::mod` (4 duplicate impls, 11 inline `pragma_table_info` queries). The migration modules keep their typed `MigrationError` attribution by mapping the shared probe's error. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(migration): lock the v0.9.3 -> v1.0 upgrade path end to end Every existing test of the upgrade path starts from an already-normalised fixture: the schema ladder from v5 or v27, the settings import from a v0.10-dev `settings` table. Nothing proved the three subsystems compose from real v0.9.3 raw data (schema v11) in the order `AppState` runs them: ladder -> boot settings import -> wallet drain, with the drain's network coming from the imported settings. Adds that test over a byte-faithful v0.9.3 fixture (v11, no `single_key_wallet` table, no `core_wallet_name` column, no `onboarding_completed` column, raw seed with empty salt/nonce, an Argon2 + AES-GCM protected sibling wallet, a masternode identity, a queued DPNS vote and a top-up row). Asserts the seed arrives verbatim in the vault, the protected envelope byte-for-byte, the alias and main flag in the sidecar, the vote and the top-up history in the k/v store, the identity row still linked to its wallet — and, the headline regression, that a testnet user is not relaunched on mainnet. Plus idempotency: a second launch re-fires nothing and deletes no legacy row. Each assertion was verified to bite by mutation (dropping the imported network, the seed drain, the app-data pass and the top-up write each fail exactly the assertion that should catch them). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(tokens): stop concurrent dismissals from clobbering each other Dismissing and re-tracking a token balance each read the entire `det:token_untracked` set, mutated a local copy, and wrote the whole blob back. Both are independent backend tasks, each `tokio::spawn`ed, so two overlapping calls could both read the pre-mutation set and have the later write win — the earlier mutation was silently lost and the dismissed token reappeared on the next refresh. Give each dismissed `(token, identity)` pair its own presence-marker key (`det:token_untracked:v2:<token>:<identity>`). Dismiss is now a single `put`, re-track a single `delete`, and reading the set a prefix scan: no read-modify-write window remains for a concurrent mutation to slip into. Token id leads the key, so dropping every dismissal of one token stays a single prefix scan rather than a full-set rewrite. Covered by two threaded races (concurrent dismissals; a dismissal racing a re-track) that lose an update against the previous scheme, plus a structural test pinning each mutation to one write with no read-back. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(dashpay): scope contact results to their identity, split resolution markers Three defects in the contact-request path, all found by review on #882. Wrong-identity acts (blocking): accept_contact_request and reject_contact_request took the counterparty from the fetched document without ever checking its toUserId was the acting identity — a stale row clicked after an identity switch could sign a real state transition under the wrong key. They now go through sender_of_received_request, the mirror of the check cancel_contact_request already had, and error with ContactRequestNotAddressedToYou instead. The UI layer is fixed too: the DashPayContactRequests and DashPayContactsWithInfo results now carry the identity they were loaded for, and every consumer discards a result whose identity is no longer selected. Silent cancellation failure (blocking): mark_withdrawn dropped both an unavailable wallet backend and a typed storage error, so a cancellation whose marker never landed still reported success while the request came back as pending on the next reload. It now returns Result and cancel_flow propagates it. Undirected rejection marker: cancel and decline shared one marker, checked symmetrically for both directions, so cancelling a request to Bob silently hid the genuine request Bob sent back afterwards, with no recovery path. The marker is now split by direction (declined / withdrawn), each written and read only for its own direction; sending a request retires both. Pre-existing sidecar markers under the old undirected key are inert: a previously resolved request may list as pending once, which the user can resolve again — the safe direction to fail. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(migration): stop the legacy import from losing top-ups, votes and warnings Three defects in the legacy-upgrade path, each able to lose user data or the notice about it. All three are RED-then-GREEN covered. Top-up history was overwritten, and its failures were made permanent. `save_top_ups` replaced the whole stored map instead of merging, so a late migration pass stomped any top-up the user recorded in between. It is now a read-merge-write (incoming wins on a colliding index), matching what the top-up flow already did at the callsite. A top-up read or write failure was also reduced to a `warn!`, so the app-data sentinel was written anyway and the fast path skipped the retry forever — freezing a one-off k/v error into permanent loss. The pass now fails on it (every identity is still attempted first), which withholds the sentinel so the next launch retries. A malformed SQLite column aborted the whole vote import. `read_scheduled_votes` decoded five raw columns with `?` before the row-level skip-and-count logic, so one NULL, type-mismatched or out-of-range value (a negative `time` fails rusqlite's `u64` range check) discarded every valid vote already read and turned a warning into a hard `TaskError`. Column decoding is now per-row, like the domain decoding beside it: log, count `unreadable`, continue. Same treatment for `read_top_ups`. The unreadable-vote warning fired exactly once, ever. The sentinel fast path returned a zero-count outcome on every later launch, so the banner could never be re-published — a user who was away when it appeared never heard about it again, while the vote it names may still have an open deadline. The count now persists in a per-network k/v record, written before the sentinel (a crash in between re-runs the idempotent import rather than losing the warning), and is re-published on every launch until the user acknowledges it via the banner's "Got it" action — a stray dismissal is not an acknowledgement. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(dashpay): stop the Contacts tab losing loads, clicks and accepted accounts Three defects on the Identity Hub's Contacts tab, one test each, all RED before the fix. The tab hydrated with two separate `AppAction::BackendTask`s in one frame. `AppAction`'s `|=` is last-writer-wins, so `LoadContacts` was dropped on the floor and the active-contacts section stayed empty until an unrelated refresh happened to re-fire it. Both loads now travel as one `BackendTasks(Concurrent)` action, and hydration yields to a click in the same frame rather than clobbering it — the load guard is untouched until it actually dispatches, so it simply goes out on the next paint. Accept, Decline, and Cancel each sign and pay for a state transition, and nothing stopped a second click from buying a second one while the first was still in flight. Each request now holds an in-flight guard: its card's buttons are disabled, and the dispatcher refuses a duplicate even if a click gets through. Success releases the guard by request ID; a failure carries no ID, so the hub releases all of them — a row the user can retry beats a row stuck forever. Unhiding a contact rewrote the whole `contactInfo` document with an empty accepted-accounts list, erasing every account the user had accepted. The write path now takes an `AcceptedAccounts` choice: `Replace` for a caller that owns the list, `Preserve` for one that does not, which reads the stored accounts back out of the existing document. Unhide and the contact-details edit form — neither of which has any say over accepted accounts — now preserve them. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test: allow-list v093_upgrade.rs's legacy wallet-table fixture read tc_dev_001_no_live_readers_of_wallet_table failed against the concurrently-merged v093_upgrade.rs (0fcb6e7e): its second-launch assertion reads the legacy `wallet` table row count directly from a scratch fixture database to prove the row survives migration. That's a test-only fixture-verification read, never a cold-boot read, the same exemption already granted to wallet_lifecycle/tests.rs. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test: stabilize suite timing and close kittest wiring race (#884) * test: stabilize suite timing and close kittest wiring race Three test-suite reliability fixes, no product code touched. 1. Ignore 49 wallet_backend secret-storage tests that each pay real Argon2id (64 MiB) cost through platform_wallet_storage's public SecretStore API. There is no downstream fast-KDF hook yet (dashpay/platform#4111 tracks exposing one), so under whole-suite parallelism their peak memory pressure drives the host into swap and inflates every test's wall-clock. The ~50 sibling vault tests in the same modules stay enabled as canary coverage; CI still runs the ignored set via `-- --ignored` (see PR note — the workflow edit is pending, .github is write-protected here). 2. Close the kittest wallet-backend wiring race. AppState::new spawns backend wiring as a background tokio task; a fixed run_steps(N) races it, so seeding via insert_local_qualified_identity intermittently panicked WalletBackendNotYetWired under load. New shared helper support::wait_for_wallet_backend polls the exact precondition (wallet_backend().is_ok()) up to 30s. mount_app / fresh_app_context and every per-file mount helper that seeds now gate on it instead of a fixed step count. 3. No change for the two nextest LEAK flags — reproduced 6x in isolation under low load, always PASS, never LEAK. Both are pure synchronous unit tests; the flag was nextest's wall-clock leak-timeout heuristic false-firing under the same contention finding #1 removes. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test: un-ignore 49 Argon2id tests via argon2 opt-level=3; close wallet-registration race ## argon2 opt-level=3 — un-ignore the 49 secret-storage tests PR #884 marked 49 `wallet_backend::{secret_access,identity_key_store,single_key, det_signer,hydration,secret_seam,wallet_seed_store}` tests `#[ignore]` because their real end-to-end `SecretStore` flows each paid a production-strength 64 MiB Argon2id derivation, running 5-23s under whole-suite contention. The dominant cost was `argon2` compiled at the default dev opt-level=0: each derivation ran for seconds AND held its 64 MiB that whole time, so under parallelism they overlapped into swap pressure. Adding `[profile.dev.package .argon2] opt-level=3` (+ the `test` profile) shrinks each derivation to tens of ms and collapses the memory-hold window. Cargo honors `[profile.*]` only from the workspace root, so platform's own argon2 stanza does not propagate to DET — this must be declared independently. Result (forced-fresh): the 49 now run at min 0.14s / mean 0.59s / max 1.53s, down from 5-23s. All 1772 workspace tests pass. This is a DET-local change with no dependency pin and no cross-revision instability; the upstream fast-KDF mock (dashpay/platform#4111) is not required to hit the target. ## Close the wallet-registration race (CI-only failure on e0c81a9c) `context::wallet_lifecycle::tests::cache_shielded_receive_address_publishes_ bound_account_zero_address` (and its sibling `remove_wallet_evicts_shielded_ receive_address`) wired the backend BEFORE `register_wallet`. With the backend wired, `register_wallet` spawns the fire-and-forget `wallet_upstream_ registration` subtask, which then races the test's explicit `ensure_upstream_registered`: both call `create_wallet_from_seed_bytes`; the loser sees `WalletAlreadyExists` then `get_wallet` returns `None` in the insert gap, exhausting `resolve_registered_wallet`'s retries → `WalletNotFound`. Production never combines both paths per wallet (fresh uses the subtask; cold-boot/loaded uses `ensure_upstream_registered`), so this is a test-only artifact. Fix: register BEFORE wiring the backend (the pattern already documented in the cold-boot test), so the subtask never spawns and `ensure_upstream_registered` is the single upstream writer. Verified 25 iterations (50 test executions) under single-core pinning + 4x background CPU load, all green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com> * test(wallets): pin the pill-click mirroring seam on the Wallets page The Wallets page caches its own wallet handle, but the shared applier the top panel runs on a pill click writes only the app-global selection. Nothing covered the seam between the two, so removing the page's mirroring step would have compiled, passed, and shipped a pill that moves while the page body stays on the previous wallet — a pill click performs no navigation, so the arrival re-sync never fires to cover it. The new test walks the real sequence: run `apply_global_nav_effect`, assert the app-global selection moved AND the page's cache did not, then mirror and assert the page caught up. The middle assertion is the regression this seam exists to prevent. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(migration): import legacy v0.9.3 identities and their keys (#885) * docs(migration): design the v0.9.3 legacy identity import 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 b14bf32c had already moved and a version-byte agreement that is not needed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(migration): import legacy v0.9.3 identities and their keys 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> * fix(migration): never let a corrupt vote queue strand identity keys 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> * fix(migration): honor legacy alias fallback and guard identity-import 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> * fix(migration): column-authoritative alias, per-row identity decode, 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> * fix(migration): surface unreadable votes alongside unreadable identities 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> * fix(migration): reconcile legacy keys into partially loaded identities The identity import skipped a legacy row wholesale whenever the id was already in the modern store. But presence is not proof every key survived: before this PR, a masternode could be loaded from only its ProTxHash (voting/owner/payout keys all optional), persisting a partial key map. When such an install upgrades, the legacy blob may still hold owner/voting/payout keys the modern record lacks — and the wholesale skip stranded them. The skipped row was not counted unreadable, so the sentinel landed and those legacy-only keys never reached the vault or got retried: a silent loss of a masternode's control keys, not just a banner glitch. The importer now fetches the existing modern identity and gap-merges the legacy blob into it: the modern record stays authoritative (its keys, alias, protection state, and wallet link always win) and only the keys/associations it lacks are taken from the blob. It re-persists in place via update_local_qualified_identity only when the merge actually recovered something (new `reconciled` counter); an identical record is left untouched, so a retry can never overwrite a user edit with the stale legacy copy. The gap-merge is the same "keep what I have, borrow only what I'm missing" rule load_identity already used for in-place key adds; that private helper is promoted to QualifiedIdentity::merge_gaps_from (model/, single source) and reused by both callers. Regression test `a_present_but_partial_identity_gains_the_legacy_only_keys` stages a partial modern identity plus a legacy blob carrying an extra Owner key and asserts the key is merged in and the record re-persisted once; the existing already-in-store test now proves an identical record is not re-written. Design doc §7 edge-case table updated to match. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(migration): only reconcile bare identities, never keyed ones The e8b61821 reconcile filled a present identity's gaps from the legacy blob by inferring "missing" from field absence. Two ways that is unsafe for a background migration (both raised in review): 1. Protection downgrade — merging a legacy `Clear` key into an identity whose other keys are password-protected produces a mixed record. On save, `encode_identity_blob_vault_first` refuses it with `IdentityKeyProtectionDowngrade`; the migration then errors before writing its sentinel and fails identically on every launch. 2. Resurrected removals — absence is not proof of a partial load. "Remove private key from DET" deletes a map entry and clearing an alias persists `None`. On a pre-sentinel install (or a retry held open by another unreadable row) the merge would refill those intentional absences from the stale blob, restoring a removed alias or re-adding a deliberately-removed private key. Fix: reconcile only a record that holds NO private keys at all — the one unambiguous "loaded without its keys" signal (the ProTxHash-only masternode load). For a bare record, take the legacy key set and fill the missing masternode role associations; re-persist only when something was recovered. Any record that already holds keys is left untouched: a protected identity always holds keys, so it never reaches the vault-first guard (fixes 1), and a keyed record's absent field is never refilled, so removals are never resurrected (fixes 2). Alias is never merged in migration. A keyed-but-partial or protected identity is recovered instead through the interactive load, which has the identity password. Reverts the shared `QualifiedIdentity::merge_gaps_from` extraction: the gap-merge is a load-path-only tool (safe only with a user present), so it goes back to the private `merge_existing_keys_into` in load_identity. Migration carries its own bare-record reconcile. Tests: `a_present_but_bare_identity_gains_the_legacy_only_keys` (bare record recovers the legacy key + owner association) and `a_present_keyed_identity_is_left_untouched_never_reconciled` (a keyed record is skipped — no downgrade, no resurrected alias/key). Design doc §7 updated. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(migration): revert legacy-identity reconcile to safe skip-if-present Reconciling legacy-only keys into an already-present identity cannot be done safely without provenance the model does not carry: field absence is indistinguishable from a deliberate user removal ("Remove private key from DET" leaves no tombstone; a cleared alias persists as None), so a blob-first merge would resurrect removed keys or aliases. Merging a plaintext legacy key into a protected identity would additionally trip the vault-first IdentityKeyProtectionDowngrade guard and fail the whole pass. Revert migrate_identities_from_conn to the original skip-if-present body: an identity already in the store is skipped wholesale, never re-persisted. Restore has_local_qualified_identity (presence probe, no decode) as the skip check. Drop the reconciled counter, the get_existing/update closure seams, and the reconcile-specific tests. No data is lost: the legacy data.db is preserved verbatim, so a bare (partially-loaded) identity's stranded keys remain recoverable by a future provenance-aware flow. Document the stranding as a known limitation in the design doc (§7) and track the recovery flow as a follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01WVUVpGxaAsciGSiT7t4kuC * fix(migration/wallet): QA follow-ups for #885 — resurrection, banners, 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> * docs(migration): mark the legacy-identity design as shipped and name 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. * build(deps): document the deliberate bincode pin (RUSTSEC-2025-0141) 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. * fix(shielded): let the Verified badge name the balance it vouches for 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. * test(migration): extract the legacy-identity fixture into database::test_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). * docs(legacy-import): state precisely what read_identities logs 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. * fix(migration): name the Identities screen in the unreadable-identity 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. * fix(migration): publish a terminal state for every migration failure `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. --------- Co-authored-by: Luka…
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
1 participant
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
WalletNotLoadedgave no indication of which wallet was still loading.feat/legacy-identity-migration).What was done
Four independently-verified fixes, merged onto one branch:
6af894b5): boundsQualifiedIdentity::from_bytesto 16 MiB so a length-inflated collection prefix errors instead of aborting.e4903e07):TaskError::WalletNotLoadednow carries awallet_label(alias, or a truncated seed-hash hex when unnamed) and names the wallet in the message. Extracted the existingSeedLengthInvalidalias-or-hex rule into a sharedmodel::wallet::meta::wallet_labelhelper (DRY).2a3abe26,770908b8,90214ea5): realigned the legacy-identity design doc with the shipped flow; stopped a readable app-data pass whose later notice-record read failed from being reported as a failed migration; added the identity-banner kittests the rustdoc promised.5953ef7f): the identity import pass now writes its completion sentinel unconditionally (matching its sibling app-data pass), so a corrupt row no longer causes the import to silently re-run — and re-resurrect deleted identities — on every launch. Undecodable rows are now surfaced via a durable, acknowledgeableUnreadableIdentitiesWarningbanner instead.Merging the last two required resolving genuine semantic conflicts (not mechanical ones) in
finish_unwire.rs,migration_status.rs, and the kittest file — both branches touched the same vote-warning-read-failure code path for different reasons; the merge preserves the docs-and-banner fix on top of the resurrection branch's restructuring.Testing
cargo test --lib --all-features: 1686 passed, 0 failedcargo test --test kittest --all-features: 218 passed, 0 failedcargo clippy --all-features --all-targets -- -D warnings: cleancargo +nightly fmt --all -- --check: cleanBreaking changes
None.
TaskError::WalletNotLoadedchanges from a unit variant to{ wallet_label: String }— an internal API change, not user-facing breakage (all match sites in-tree updated).Checklist
Attribution
🤖 Co-authored by Claudius the Magnificent AI Agent