fix(wallets): show transaction history that predates the current session - #892
Merged
Conversation
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>
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>
…eanup A10 (expert-mode nav-refresh mismatch) was found already fixed on base (fixed in 4343d03, pre-dates the stale 2026-07-10 audit finding) — no behavior change needed, comment already accurate. B3/net-rpc-password: deleted the dead dashmate_password_input field and its per-network-switch synchronous disk read. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.
CHANGELOG + gap-audit table entries for 3 previously-undisclosed removals, 2 strengthened disclosures, a gravestone comment for the retired Masternode List Diff screen, and a sign-off record for the 10 already-fully-disclosed "not planned" items. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
"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>
Root cause: token_watch_sets re-registered the full token registry per identity on every refresh, resurrecting dismissed rows. Adds DET-side per-identity dismissal persistence in the platform-wallet k/v store (contract_token_db.rs) and excludes dismissed pairs from the refresh watch set. TOK-018 wording corrected to match fixed behavior. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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>
…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>
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>
…ity)
Bridges the async shielded-address read into the frame loop and restores
address display + copy. Address is sourced from the same OrchardKeySet
the shielded coordinator scans with (no re-derivation, no watch-window
hazard), evicted on wallet removal/switch. Diversified-address generation
("+") is deliberately deferred — the pinned platform-wallet rev has no
FVK accessor for it, and both workarounds available today are
funds-unsafe (account 1+ is unspendable via DET's current spend path).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…acy 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>
…imitation 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>
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>
- A1: Accept/Decline wired to existing DashPay backend; Cancel implemented as an honest hide+notify (contactRequest documents are immutable/ undeletable on Platform, so a true withdrawal is impossible) — fixes two related bugs: rejected requests reappearing on reload, and a permanent rejection marker blocking later re-adds. - A2: real active-contacts list rendered from LoadContacts, search wired. - A3: local alias/nickname edit added to Hub Settings tab. - A4: pay-a-contact entry point added, routes to existing send-payment screen; the stale "requires SPV, dev-mode only" gate removed (SPV is now the standard backend) — flagged for a security pass since it's fund-movement gating. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Imports scheduled DPNS votes, network/theme/onboarding settings, and top-up history from the legacy v0.10-dev data.db on first launch after upgrade. Votes use an independent sentinel ahead of the wallet-drain gate (an identity-only voter with no wallet rows would otherwise never trigger migration). Settings import runs synchronously before AppState::new_inner's network read, since a BackendTask would run too late. No silent loss path remains; a failed pass leaves legacy rows intact and surfaces a retry banner instead. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…A7, A8) Investigation confirmed the SPV watch mechanism already supports a bare imported address once it's in a registered wallet's pool — the actual blocker is that PlatformWalletManager::register_wallet (the only path that would register a watch-only, non-HD wallet) is private with no public watch-only entry point. Coin selection for send is blocked transitively on the same gap. A synthetic-HD-wallet workaround was evaluated and rejected on fund-safety grounds (unsignable transactions, inconsistent restart behavior). Ships: dead is_rpc_mode UI gate removed, clear in-app limitation surfaced (banner + tooltip), backend TaskError remains the authoritative refusal. No manual refresh control added, per owner direction for A8. Exact upstream ask documented in the core/mod.rs TODO for whoever files it against platform-wallet. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ll (A5, A11, B3)
Masternode-scoped pill wired two-way with the card grid/detail view;
Wallets pill made fully interactive (chosen over other candidates since
A5 already covers Masternodes' object pill). Fixes a real bug found
while wiring the Wallets pill: WalletsBalancesScreen::refresh_on_arrival
never adopted the app-global wallet selection, so a wallet switched via
the (already-interactive) Masternodes pill was silently overruled by
the first-wallet default. Also corrects a misattributed code comment
("locked decision #4") and downgrades the dead connection-indicator
click sensor to hover-only (B3).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…dden 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>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…s, 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>
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>
…ared search) - QA-001: cancel_contact_request re-checks for a reciprocal request right after the display_hidden broadcast and auto-corrects (un-hides, returns DashPayContactAlreadyEstablished) if one landed inside the window. Residual risk (a reciprocal request arriving after the second read) is documented and bounded by the new unhide path, not eliminated — Platform has no conditional write, so no window width fully closes this. - QA-005: Hub Contacts tab gains a "Show hidden contacts" section with an Unhide action, giving hidden contacts (from the above race, or the pre-existing display_hidden overload) a recovery path without leaving the Hub. - Adams consistency finding: contact search/hidden-filtering unified into one matches_contact_search() helper shared by the Hub and the legacy DashPay screen (field set = their union; legacy behavior unchanged). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
`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>
…onsistency, typed token seam) - QA-102/PROJ-001/SEC-005: single_key_send_screen's Send button re-disabled (matches the sibling screen's pattern) rather than left enabled-but- guaranteed-to-fail; kept the screen (not deleted) since backend_task/ core/mod.rs's TODO documents the parked re-pointing plan for when upstream lands. Also closes a second, previously-unflagged bug: the busy "sending" flag had a double-send window via the fee-retry dialog, fixed by making dispatch the single choke point that arms it. - QA-002/SEC-001: ungated the 3 remaining legacy DashPay Pay/Send entry points (contacts_list.rs, contact_details.rs, contact_profile_viewer.rs) to match the Hub's already-correct behavior; removed the stale "requires SPV, dev mode only" comments; left one TODO on the DashPaySendPayment screen variant itself flagging role classification for the incoming PR #879 UserRole rework. - RUST-001: token-dismissal call seam now threads the existing IdentityTokenIdentifier struct instead of a bare (Identifier, Identifier) tuple, closing the silent-transposition risk; on-disk KV payload order preserved and pinned by a new test. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…t drain QA-101 (HIGH): a single corrupt/unreadable legacy scheduled-vote row previously made finish_unwire::run() propagate a fatal error via ? BEFORE the wallet-drain gate ran, permanently blocking fund-access migration on every launch with no skip path. Fixed by extracting the wallet-drain path into drain_wallets(), running it unconditionally regardless of the app-data (votes/top-ups) migration's outcome. Both original invariants preserved: an identity-only voter with zero wallet rows still triggers vote import (own sentinel, still ahead of the wallet-drain gate), and no vote is silently dropped — an unreadable row now surfaces via a sticky Warning banner (no "Retry now", since a corrupt row won't decode better on a second pass) instead of blocking everything forever. The app-data sentinel is still written once importable rows are handled, deliberately, to avoid re-running the import on every boot and resurrecting an already-cast-and-cleared vote from its stale legacy row. QA-103: replaces the tautological idempotency test (which called read_sentinel directly, never run() twice) with one that actually invokes finish_unwire::run() twice on the same AppContext and is proven non-tautological via mutation (disabling the short-circuit makes it fail). Adds a real end-to-end test proving the wallet drain completes despite a corrupt vote row, confirmed RED before the fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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>
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>
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>
…on 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>
…, decode limit, wallet naming (#891) * fix(identity): bound the bincode decode so a corrupt blob errors, not aborts QualifiedIdentity::from_bytes decoded legacy identity blobs under bincode::config::standard(), which resolves to NoLimit. A length prefix claiming an inflated element count (an ordinary bit-flip or truncation, no attacker required) makes bincode pre-allocate the claimed size before reading anything; when that exceeds available memory the allocator aborts the process (SIGABRT, uncatchable, not a Result::Err). Live-reproduced during PR #885's grumpy-review: a minimal probe encoding a 1 TiB length prefix aborted with exit 134. This defeated the legacy-identity migration's own stated contract ("one bad blob never blocks the identities around it") on exactly the corruption class ordinary disk bit-rot produces, crash-looping the app on every cold start until the user manually repaired data.db. Fix: decode under a bounded Limit (16 MiB, far above any real QualifiedIdentity) via a shared identity_blob_decode_config() function used by both from_bytes and its regression test, so a future edit that weakens the limit is caught rather than silently diverging from what the test actually pins. With a Limit, bincode checks the claimed size against the cap before allocating and returns DecodeError::LimitExceeded -- a normal Err the existing skip-if-present machinery already handles. RED-first: temporarily reverted the config to unbounded and confirmed the new regression test aborts the test process with the exact same "memory allocation of 1099511627776 bytes failed" / SIGABRT signature from the review's live repro, before restoring the fix and confirming green. Full workspace suite (1675 lib tests + kittest/doctests), clippy --all-features --all-targets -D warnings, and cargo +nightly fmt --check all pass clean. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(wallet): name the wallet in the "still loading" error `TaskError::WalletNotLoaded` was a bare unit variant: with several wallets loaded, neither the user nor a developer reading logs could tell which wallet was still loading. It now carries a `wallet_label` — the alias, or a truncated seed-hash hex when the wallet was never named — and the message names it. Both construction sites (`resolve_wallet`, `monitored_receive_addresses`) resolve the label from the wallet-meta sidecar: the wallet is by definition missing from `id_map` there, so there is no live handle to ask. The `id_map` read guard is released before that sidecar read. The alias-or-hex rule was inlined in `wallet_from_envelope` (`SeedLengthInvalid`); it moves to `model::wallet::meta::wallet_label` as the single source of truth for both errors, output unchanged. * docs(migration): realign the legacy-identity design doc with the shipped code The doc was written before a ten-commit iteration and only partly updated afterwards, so three sections described an implementation HEAD never had. - §5: the sketch unwrapped `app_data` before running the identity import, the exact inverse of HEAD. Both DET-owned results are *held* and judged after the drain, because an app-data failure is deterministic: unwrapping it first would skip the identity import on this launch and on every retry, stranding a masternode owner's keys over a corrupt vote queue. Transcribed HEAD's held-then-judged flow, including the per-arm terminal states. - §9 T-ID-01: `LegacyIdentityRow` has no `status` field (the reader folds status and alias straight onto `qi`), and the SQL selects a sixth column, `alias` — the column, not the blob's stale copy, is authoritative. - §10: assertion 9 cited `second_launch_after_a_v093_upgrade_changes_nothing` as proof of skip-if-present, which that test cannot carry — on the clean path the sentinel short-circuits the pass before the check is reached, so it would pass against an importer with no such rule at all. Moved to `a_retry_after_an_unreadable_identity_preserves_user_edits`, where the sentinel is deliberately withheld, and said why. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(migration): stop reporting a readable app-data pass as a failed one `FailedWithUnreadableIdentities` had two producers, and one of them lied. Path 1 — the app-data pass hard-fails alongside undecodable identities — is what the state and its banner describe: "updating the rest of your previous data did not finish… Choose Retry now to finish updating." True, and the retry works, because the app-data sentinel is unwritten. Path 2 — the app-data pass SUCCEEDS and writes its sentinel, and only the follow-up unreadable-vote-warning k/v read fails — published the same state. The user was told a pass that had completed did not finish, and offered a retry that re-runs nothing: on the retry the app-data pass short-circuits on its own sentinel and the same read fails again, so the false error banner returns on every launch. Fall through to the honest `SucceededWithUnreadableIdentities` instead, and log the read failure with its typed error. Nothing is swallowed: the warning record is durable, and this branch re-runs on every launch while the identity sentinel stays unwritten, so the next successful read re-publishes the vote half. The identity signal — the one the user must act on — reaches them either way. Reusing the existing variant over adding a new one keeps the reconciler and the shielded indicator untouched (both already map this state and the old one to the same badge). Regression test `an_unreadable_vote_warning_record_does_not_claim_the_app_data _pass_failed` poisons the warning record with a zero-length bincode body, so the read fails deterministically while the app-data pass runs clean; it asserts the honest state AND that the app-data sentinel is written — the very fact the old banner denied. Confirmed RED before the fix. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(migration): add the identity-banner kittests the rustdoc promised Three banner-copy functions in `app.rs` close their rustdoc with "Exposed for kittest coverage", which is the only thing justifying their `pub` — yet `tests/kittest/` referenced none of them. The promise now holds: - `unreadable_identities_banner_warns_without_a_retry_action` - `unreadable_identities_and_votes_banner_names_both_and_acknowledges` - `failed_with_unreadable_identities_banner_offers_a_working_retry` Each asserts the copy renders verbatim and that the action set matches the outcome: no retry for the two Warning states (the rows are still in the previous version's storage and decode no better on a second pass), a working "Retry now" for the one genuine failure, and the vote acknowledgement on the combined warning so a live deadline cannot be buried by the recurring identity signal. Also adds the missing `MigrationStep::Identities` to `tc_mig_014_running_text_covers_every_step_with_sentence`, which claimed to cover every step while omitting the one this feature added. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(migration): stop the identity import from resurrecting deleted identities The identity pass wrote its completion sentinel only when every legacy row decoded. A genuinely corrupt row never decodes, so the sentinel was never written and the import re-ran on every cold start, forever. Skip-if-present made that harmless for an identity the user had edited, and did nothing for one the user had deleted: the next launch re-imported it, restored the alias the user had cleared, and re-wrote its legacy plaintext keys into the vault — with no banner to explain it and no way to stop it short of editing data.db. Write the sentinel unconditionally, exactly as the sibling app-data pass already does and for the reason it documents. The import becomes a once-only event, so a deletion is durable. The undecodable rows stay in data.db (never deleted) and are carried forward by a durable UnreadableIdentitiesWarning record instead of by an import that retries until it decodes; recovering them after a decoder fix is an explicit user gesture (#889), not an automatic retry that costs a deletion. The durable record is what makes that safe: with the sentinel written the pass short-circuits and reports zero unreadable rows on every later launch, so the banner is now published from storage rather than from pass counters. That record also closes the second hole: the unreadable-identity banner was sticky with no action button, so the user could be told their signing keys had not come across and given no way to say "I understand". It now carries a "Got it" action wired to a new AcknowledgeUnreadableIdentities task, mirroring the vote flow. Acknowledgement deliberately does NOT double as the sentinel-writer — hanging the loop-break on a user gesture would leave the resurrection bug live for anyone who never clicks. The combined banner names both problems, so its single acknowledgement retires both records. Tests (both confirmed RED against the unfixed code): - a_deleted_identity_is_not_resurrected_by_an_unreadable_sibling_row - unreadable_identity_warning_is_republished_until_acknowledged - a_second_launch_after_an_unreadable_identity_preserves_user_edits_and_deletions (rewritten: proves rename AND deletion survive on a real v0.9.3 database) - reconciler + kittest coverage that the banner offers the acknowledgement and routes it to the right task Two existing assertions demanded the withheld sentinel — the defect itself — and were flipped to the corrected contract. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
…te behind Expert mode The status dot/text on masternode cards and the detail screen read as Core masternode network health, but it actually reflects whether the node's Platform identity currently resolves (IdentityStatus). Relabel to "Platform identity: <status>" with a clarifying tooltip, and hide it entirely below Power/Developer role — everyday users no longer see a misleading always-on indicator. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…issue #889 The doc still introduced itself as "design, ready for implementation" against a pre-implementation base commit, and its closing sections read as open questions, long after PR #885 shipped every task in §9 and PR #891 landed the QA follow-ups. - Header states the real status (shipped in #885, follow-ups in #891) and surfaces the known limitation up front. - The §7 known-limitation follow-up now names issue #889 directly instead of pointing at "the GitHub issue referenced from PR #885". - §11 (bincode feasibility spike) is framed as settled, pointing at the golden blob it produced (T-ID-06 / V093_MASTERNODE_BLOB_HEX). - §12 is relabelled as historical design-review findings; the rationale table is kept, but it no longer masquerades as a live defect list.
bincode is flagged unmaintained. The advisory is INFO-level and covers every version, so no bump can clear it: 2.0.1 is the last functional release and 3.0.0 is a tombstone whose lib.rs is a bare `compile_error!`. bincode 1.3.3 also arrives transitively, so dropping the direct dependency would not silence it either. The encoder writes on-disk wallet-secret envelopes and QualifiedIdentity blobs, so swapping it changes the wire format of data users already hold. Record the risk acceptance at the pin so the next reviewer does not re-flag it, and so nobody "fixes" the warning with a bump that would be data-loss-class. No version or lockfile change.
The shielded badge maps every post-drain terminal state — including FailedWithUnreadableIdentities — to Verified, and that mapping is right: those states only fail passes (app data, identity rows) that run after the wallet drain and never touch shielded storage, so the balance is as authoritative as on Success. Downgrading it would lock shielded spends over a corrupt vote row and claim a shielded failure that never happened. What was wrong is the copy. "Verified." took its subject from its position under the balance, so beside the red migration error banner it read as a blanket "all good" — and handed a translator an adjective with no noun to agree with, against the project i18n rule. It now names its subject: "Shielded balance verified." Also covers the three migration states the exhaustiveness test had missed (SucceededWithUnreadableIdentities, SucceededWithUnreadableIdentitiesAndVotes, FailedWithUnreadableIdentities) and records why the green badge under an error banner is deliberate.
…est_helpers The v0.9.3 identity fixture — table DDL, encodable blob, row INSERT — was rebuilt in three modules, so a column added to the legacy shape had to be chased through all of them. It now lives once in database::test_helpers, next to the legacy wallet and scheduled-vote fixtures already shared from there: create_legacy_identity_table, basic_legacy_identity_blob, and a LegacyIdentityFixture builder that states only what a test varies. Deliberately not merged, because they are not the same fixture: - v093_upgrade keeps its verbatim v0.9.3 whole-database DDL (its both-or- neither wallet CHECK is the point of that module) and its keyed blob builder; only its row INSERT now routes through the shared builder. - The shared DDL omits that CHECK on purpose — the import must survive a half-filled wallet link, and no test could stage one if SQLite rejected it. - The minimal (id, network) identity table used by the top-up/vote scoping tests is a different shape and stays where it is. Also folds the thrice-copied corrupt-row insert in finish_unwire's async tests into one local helper, and types the fixture's status as IdentityStatus, which retires v093's raw u8 status arguments (the consts stay as the on-disk assertions they always were, now including Active).
The rustdoc promised that nothing about "the decoded identity" is ever logged because it carries private keys, while the warn branches log the identity's id. The code is right — an identity id is a public, on-chain handle, and it is what lets a user tell which identity did not come across; the blob and its decoded key material are never logged. Only the promise was imprecise, so it now draws that line explicitly instead of over-claiming.
… banners "Load these identities again" named neither a screen nor a control, so an Everyday User who has never opened that flow had no way to act on it — the repo's error-message rules require a concrete, self-serviceable action. All three variants now point at Load Identity on the Identities screen, mirroring how the vote copy already names the Scheduled Votes screen. The kittest asserting every variant names both is the regression net: it fails against the old copy.
`migrate_app_data` propagated the `get_scheduled_votes()` error raw, so a k/v read failure left `run()` returning a `TaskError` that was not `MigrationFailed`. `run_migration_task` published `MigrationState::Failed` only for that one variant, so such an error published nothing and stranded the status on `Running` — where `run_backend_task` rejects every wallet-touching task with `WalletStorageNotReady` and the banner offers no retry. That wedges wallets, identities and sends until the app is restarted. Type the app-data read into `MigrationError::AppDataImport`, and make the publish total: `migration_error_chain` coerces any `TaskError` into the typed `Arc<MigrationError>` chain (a stray error wraps in the new `Unexpected` variant), so no error can skip the terminal state. Also from the same review round: - Drop the `wallet_known` closure seam from `migrate_identities_from_conn`: it could not change behaviour, only gate a `tracing::warn!`. The diagnostic moves to the caller's insert closure, which already holds the backend, so the `WalletBackendUnavailable` gate is unaffected. - Collapse `write_sentinel` into `write_completion_sentinel`, now the sole writer of `MigrationCompletion`, with `network_count` as a parameter. - `run()`'s "No pass gates another" was imprecise: the two DET-owned passes do not gate each other, but the wallet drain is a deliberate prerequisite for the identity import. Say so. - Document why the identity check-and-insert needs no transaction: the migration gate serialises every production identity writer.
…into fix/qa-followups-885
Legacy-data migration completed while password-protected wallets were still locked: `register_migrated_wallets()` excluded locked wallets from the completion check, so `bootstrap_wallet_addresses_jit` never ran for them, their `id_map` entry stayed empty, and every later operation on that wallet failed with `TaskError::WalletNotLoaded`. Migration now blocks on a non-dismissible password prompt for each locked wallet. A new `MigrationState::AwaitingWalletPasswords` carries the pending seed hashes; the frame loop renders the prompt via the existing `WalletUnlockPopup` and drives the entered password through `handle_wallet_unlocked`, which promotes the seed and bootstraps the wallet. A `tokio::sync::Notify` handshake wakes the migration task, which re-checks for locked wallets and only then writes the completion sentinel. Protected seeds are re-encrypted under the current envelope (`SecretScheme::Protected`) rather than left `Absent`. `passphrase_modal` gains a `cancellable` flag so the migration prompt suppresses Cancel, Escape, click-outside and the title-bar close button. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
…migration-password-prompt
The card status text was relabelled to "Platform identity: <status>" so the dot cannot be misread as Core or PoSe health, but the masternode-tab kittest still asserted the bare "Pending Creation" label and went red. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
… migration" This reverts commit c6176f4. Review of the combined PR surfaced four blocking defects in the prompt, so the feature is withdrawn from this PR and reworked in a dedicated one: - The migration awaits a `tokio::sync::Notify` whose only producer is the egui frame loop, but `mcp/resolve.rs` drives the same migration with no frame loop — headless `det-cli`/MCP hangs forever on an install with a password-protected legacy wallet. - A forgotten password wedges the app on every launch, and the only exit the UI permits is `remove_wallet` (called outside the `BackendTask` gate) — deleting the wallet and its seed. - Migration spuriously reports `RegistrationIncomplete` even when the correct password is supplied: unlock spawns a fire-and-forget registration while the migration task inline-awaits its own for the same wallet. - The "non-dismissible" modal does not actually block input; a click reaches widgets behind it. The rest of this PR's QA follow-ups are unaffected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
…tity status `show_platform_identity_status()` gated the status row behind the Power role, but the Masternodes tab is already Power-gated by the nav rail: `app.rs` routes the root screen away below Power, so no card or detail view is reachable at a lower role. Both callsites always received `true`, and the unit test asserting the false branch covered an unreachable path. Remove the gate, its bool parameter on `MasternodeCard::show`, and the dead test. Role gating stays where it belongs — at the nav rail, as the single source of truth. The explicit "Platform identity: <status>" label and its clarifying tooltip are unchanged; they were the point of the change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
Transaction history rendered empty for wallets holding a non-zero balance. The history the UI reads comes from the in-memory snapshot store, which was populated only by live wallet events through the event bridge — nothing ever hydrated it from the persisted `core_transactions` rows. Balances did not share the defect because they are read from persisted UTXO state, so a wallet showed funds with no transactions to account for them. Hydrate the snapshot from the persistence layer at wallet load, before the first snapshot is published. Persisted records seed by txid with insert-if-absent, so a live record already observed is never overwritten by an older persisted one, and a live event arriving after hydration updates the existing row instead of duplicating it. Locked wallets hydrate without secrets; wallets awaiting initial registration hydrate on unlock. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
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 |
Base automatically changed from
feat/v1.0-parity-batch
to
docs/platform-wallet-migration-design
July 14, 2026 14:03
…esign' into fix/tx-history # Conflicts: # src/model/qualified_identity/mod.rs # src/wallet_backend/event_bridge.rs
lklimek
marked this pull request as ready for review
July 15, 2026 12:27
Collaborator
|
⛔ Blockers found — Sonnet deferred (commit 81ef37e) |
13 tasks
* fix(migration): prompt for wallet passwords before completing migration
Restores the migration password prompt reverted from PR #887 so it can be
reworked in isolation. This commit is the original implementation verbatim;
the review findings that caused the revert are fixed in the commits that
follow.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
* fix(migration): free the password prompt from the SPV overlay, add a skip
Two defects found by real-world testing of the migration password prompt.
The SPV progress overlay and the passphrase modal both painted at
`egui::Order::Foreground`. Suppressing only `ProgressOverlay::claim_input`
released the keyboard but left the overlay's pointer sink and dim/card layers
live, so they swallowed clicks aimed at the password field — the prompt was
visible but unusable whenever migration ran alongside an SPV sync (which is
always, at boot). A blocking secret prompt now owns the whole interaction
surface: while one is active the overlay stays logically in its stack but
paints no dimmer, pointer sink, card, or focus trap, and claims no keyboard.
Queued ordinary secret prompts are promoted before the frame's overlay
decision, so their first visible frame is protected too.
The prompt was also inescapable: a user who had forgotten a wallet password
could not proceed, and the only exit the UI permitted was deleting the wallet.
"Skip this wallet" now records the seed hash in a per-run exclusion set, drops
it from the published pending list, and wakes the migration task — so skipping
the last wallet still completes the migration and writes the sentinel. A
skipped wallet stays closed, keeps its legacy protected envelope, and is
registered upstream on a later ordinary unlock via the existing
`handle_wallet_unlocked` -> `bootstrap_wallet_addresses_jit` chokepoint.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
* wip(migration): headless fail-fast, legacy read-only, registration single-flight
INCOMPLETE — DO NOT MERGE. Committed to preserve work across a session
restart; the Codex job producing it was cancelled mid-edit.
State: compiles clean (clippy --all-features --all-targets -D warnings, exit 0),
but the full test gate is RED (exit 101, 1771 passed / 2 failed):
context::wallet_lifecycle::tests::migrated_protected_wallet_blocks_migration_until_password_submission
context::wallet_lifecycle::tests::protected_wallet_registers_upstream_on_unlock_without_restart
Both are tests the legacy-read-only and registration-race changes must rewrite;
the job was cancelled partway through that rewrite. Whoever picks this up must
finish those two and re-run the full gate before trusting any of it.
Intended scope (per review findings + owner directives):
- P1 headless fail-fast: migration must refuse, not block, when a protected
wallet needs a password and no interactive prompt exists (mcp/resolve.rs
drives the same migration with no egui frame loop -> det-cli hung forever).
- P2 legacy DB strictly read-only: never DROP/DELETE/UPDATE the pre-migration
database; write only the new store/vault. Makes a skipped or abandoned
migration cost the user nothing.
- P3 registration race: single-flight per wallet (unlock spawned a
fire-and-forget registration while migration inline-awaited its own for the
same wallet -> spurious RegistrationIncomplete, reproduced as a real failure).
- P4 split MigrationState::is_running(), which silently came to mean
"running OR blocked on a human"; five callers inherited the conflation.
- P5 cross-wallet password bleed (modal state keyed on window title), swallowed
re-encryption failure, unified lock-poisoning policy.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
<sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
* fix(shielded): gate fund-moving shielded tasks at the backend chokepoint
`run_shielded_task` had no capability check, so the five state-changing
shielded operations were reachable from any caller that dispatches a
`ShieldedTask` directly. The MCP shielded tools do exactly that, bypassing
the UI gate at `ui/wallets/shielded_tab.rs`. Shielded operations are not
defined on any current network, so `ShieldFromAssetLock` would create an
asset lock committing real L1 funds and then attempt a state transition no
network can settle — stranding the funds and burning the fee.
Enforce `FeatureGate::ShieldedOperations` as the first statement of
`run_shielded_task`, before any wallet or backend access, mirroring the
`RootKeyDerivationRefused` guard in `backend_task/wallet/mod.rs`. The UI
gate stays as defense in depth.
Scope is exactly the five fund movers: `ShieldedTask` carries only
write variants. Shielded init, sync, balance and address reads reach the
coordinator through their own paths and stay ungated, so shielded funds
remain viewable wherever the wallet runs.
Add `TaskError::ShieldedOperationsUnavailable` and a regression test that
dispatches a write task the way an MCP tool does; it is confirmed failing
without the guard, proving the refusal precedes backend access.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(dashpay): stop erasing accepted accounts, double-submits and silent declines
Three independent defects in the DashPay contact flow.
Contact-info write silently erased the accepted-account allow-list.
`resolve_accepted_accounts` collapsed four distinct states — no document,
missing privateData, decrypt failure, deserialize failure — into an empty
Vec, which `create_or_update_contact_info` then re-encrypted and wrote back
over the live Platform document. Any present-but-unreadable payload (e.g. a
contact whose privateData was written by another DashPay client) lost its
allow-list irreversibly on the next rename or unhide. Only an absent document
now yields an empty list; a present payload that cannot be read aborts the
write with a typed `DashPayContactInfoRead` error. The test that asserted the
data-losing behaviour is inverted, and the missing/undecodable payload states
get their own regressions.
A failed task released every request guard, allowing a paid double-submit.
`display_task_error` cleared all Accept/Decline/Cancel guards on any error, so
an unrelated concurrent failure re-enabled an in-flight Accept and a second
click bought a second state transition. Failures from the three request actions
now carry their request ID in `DashPayContactRequestActionFailed`, so only the
guard named by the error is released. Guards no longer matched by a result
expire on a timeout instead of being cleared wholesale, so a lost result cannot
strand a row forever.
A declined request reappeared after refresh. `reject_contact_request` logged and
swallowed a failed `dashpay_mark_declined` write and still reported success,
even though that local marker is the only thing that retires the row — Platform
keeps the `contactRequest` document forever. The failure now propagates, matching
the sibling `mark_withdrawn` cancel path.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallet): report a corrupted wallet envelope as damage, not a wrong password
A password-protected wallet whose at-rest envelope is corrupted (truncated or
otherwise wrong-length) failed the AES-GCM tag check and surfaced as "The
password is incorrect", trapping the user in a retry loop whose only escape was
deleting the wallet. Structural damage is now classified before the AEAD can
mistake it for a bad password.
- decrypt_message takes the caller's known plaintext length and rejects an
impossible ciphertext/tag or salt length as DecryptError::Malformed.
- WalletSeed::open returns the typed EncryptionError instead of a flattened
String, so callers branch on the variant rather than on message text.
- The unlock popup maps Malformed to the same "saved data looks damaged, re-add
it from your recovery phrase" sentence the unprotected path already shows, and
keeps the password hint on the wrong-password branch only.
Finish the two migration lifecycle tests left red at the previous checkpoint.
Both now install a TestPrompt::never(), which panics if asked and so pins the
contract that migration defers to the UI-owned unlock flow instead of driving a
secret prompt itself:
- the protected wallet waits, is then skipped, and data.db is asserted
byte-unchanged, holding the legacy database strictly read-only;
- the unlock path joins the migration's single registration flight
(registration_attempt_count() == 1).
Verified green on the full workspace suite (2023 passed, 0 failed), the
all-features/all-targets lint gate with warnings denied, and the nightly
formatter check.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(dashpay): preserve saved contact details and keep paid actions guarded
The contactInfo document is written whole, so every writer decides the fate
of the fields it does not edit. Decline, withdraw, unhide and rename each
rebuilt the payload from scratch, erasing the nickname, note and
accepted-account list stored by the user or by another DashPay client.
Replace the implicit `Vec<u32> -> AcceptedAccounts::Replace` coercion, which
made the destructive path the short one, with an explicit `ContactInfoUpdate`
that states field by field what is preserved and what is replaced. Visibility
flips now preserve everything else; only the contact-details form, which owns
the whole form, replaces.
A payload this client cannot read is no longer either silently overwritten or
a permanent dead-end: the write aborts, the user is told, and confirming an
explicit, danger-styled dialog re-runs the write with an overwrite policy, so
a contact with unreadable details can still be unhidden, declined or renamed.
The v0 parser now rejects unknown versions, invalid UTF-8, non-canonical
flags and foreign trailing bytes instead of decoding them as absent details.
Paid request actions (Accept, Decline, Cancel) keep their in-flight guard
across a routine tab switch or refresh, which previously released it and made
the row clickable again while its state transition was still running. An
identity, wallet or network change still clears the guards, since they belong
to the identity being left. Task results reach only the screen that is visible
when they land, so the wall-clock backstop is retained: without it, an action
resolved while the user was on another screen would strand its row with dead
buttons for the rest of the session.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallet): unlock a cold-booted protected wallet with its correct password
A password-protected wallet hydrates from a secret-free model: the
Tier-2/Protected arm of cold-boot reconstruction carries a placeholder
envelope, because the real secret stays in the vault. The unlock popup
verified the password against that placeholder, so after the first
restart the CORRECT password was reported as damaged data and the owner
was permanently locked out of the wallet.
Verify the password only through the secret chokepoint, which reads the
real stored envelope, and flip the in-memory seed open solely after that
succeeds (`mark_open_after_verification`). The popup maps the resulting
typed error to user copy structurally — wrong password vs damaged vault —
instead of pre-checking the model.
Operation-only unlocks now forget the session seed through an RAII guard,
so an early return or panic in the reconciliation subtask can no longer
strand a plaintext seed in the cache. A migration unlock is operation-only
too: that prompt offers no "keep unlocked" choice, so it must not silently
retain the seed for the session.
Regression cover, both entry points against a real cold boot: the context
API and the unlock popup itself. The popup test fails (correct password →
Pending) if the model pre-check is ever reintroduced.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui): block input outside a non-dismissible modal prompt
Removing the progress overlay's pointer sink (so it could not cover a
secret prompt it had triggered) also removed the only barrier in front of
the app: while the storage update paused on the migration password prompt,
clicks still reached the wallet screen behind it.
Give the modal its own barrier instead. A non-dismissible `modal_chrome`
window installs a full-screen pointer sink and registers itself as egui's
modal layer, so every layer beneath it is ignored for interaction while
the window itself — drawn above the sink — stays fully interactive.
Dismissible dialogs keep their existing click-outside behaviour.
The kittest asserts the widget beneath the prompt does NOT register a
click, and that the prompt's own controls remain hittable.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(migration): keep wallet work gated while the storage update awaits a password
`is_running()` had become an alias for `is_executing()`, which reports
`false` while the migration is paused on `AwaitingWalletPasswords`. Every
caller that meant "the storage update has not finished yet" therefore
opened up mid-migration: wallet-touching backend tasks slipped past the
`WalletStorageNotReady` gate and hit a half-migrated vault, the MCP
wait/join logic stopped waiting, and the wallets screen offered
Create/Import CTAs against a wallet list about to be rehydrated.
Replace it with `is_in_progress()` — `Running | AwaitingWalletPasswords` —
and use it at all three sites. `is_executing()` keeps its narrow meaning
for callers that really do mean "a step is running right now".
Covered by a test dispatching an MCP-style wallet task during
`AwaitingWalletPasswords` and asserting `WalletStorageNotReady`.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallet): collect the redundant legacy seed envelope and stop overpromising data removal
Legacy seed-envelope garbage collection, restored for the vault copy only.
Once the current-format secret is durable — the raw seam, a Tier-2 sealed
envelope, or an eager/lazy migration write — the superseded `envelope.v1`
row in the SAME vault is deleted best-effort, so a seed has exactly one
current copy at rest instead of an indefinitely retained duplicate. A
cold-boot scheme probe repeats the sweep after an interrupted run. The
pre-update `data.db` is NOT touched: it stays a read-only recovery
artifact.
Stop promising deletions the app no longer performs. "Remove Wallet" and
"Clear Database" said they erase all local data, while an earlier
version's read-only recovery database — which may still hold wallet
recovery data — stays on disk; the copy now says so. "Clear Platform
Addresses" is disabled rather than pretending to work: its only
implementation wrote to that read-only database.
The two remaining legacy-database writers are signposted as test-only;
neither has a production caller.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui): make every passphrase prompt own the interaction surface
The blocking progress overlay yields to ANY passphrase prompt — the gate is
`has_blocking_secret_prompt()`, true for cancellable and non-dismissible prompts
alike — and paints no dimmer, pointer sink, or focus trap while one is up. But
the replacement barrier was wired to dismissability (`blocks_input: !cancellable`),
so the ordinary just-in-time unlock prompt, which is cancellable, installed no
sink at all: pointer and keyboard fell straight through to the panels the overlay
exists to freeze.
Dismissal and input-blocking are orthogonal. `blocks_input` is now unconditional;
`cancellable` still governs only Cancel / X / Escape / click-outside, which read
raw pointer input and are unaffected by the sink.
The two comments asserting the prompt "supplies its own input barrier" described a
precondition the code did not establish; they now describe what it does.
Covered by a kittest that presses a control behind a cancellable prompt while an
overlay is raised (RED before this change: the control activated), plus one
pinning that the prompt still dismisses from its own Cancel button.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(migration): scope an unlocked seed to the storage update, not to one subtask
A wallet unlocked for the storage update has two consumers of the seed it just
promoted: the unlock gesture's own `wallet_unlock_registration` subtask, and the
update's `bootstrap_loaded_wallets()` pass, which re-enters the seed scope for
the very wallet it prompted for. Their lifetimes overlap in an order nobody
controls, yet the seed's lifetime was owned outright by the subtask's RAII guard.
Whichever finished first evicted the seed from under the other — and a cache miss
on a protected scope prompts, so the update raised a background passphrase prompt
for a wallet the user had just unlocked. If the user ticked "keep unlocked" on
that second prompt, it also silently restored the session-long retention the
migration prompt deliberately withholds.
Retention shorter than the session is now enforced by `SecretLease`, a ref-counted
claim at the secret chokepoint: each consumer holds a clone and the seed is
forgotten when the last one drops. The storage update takes its own lease for the
wallets it prompted for (`WalletUnlockRetention::UntilStorageUpdateComplete`) and
releases it on every exit path, so neither consumer can strand the other, and the
unlock still does not outlive the update.
The regression test drives the losing interleaving explicitly: the unlock subtask
is joined to completion first, then the update's pass must resolve the seed from
the session cache with zero prompts, and the seed must be gone once the run's
lease is released.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(dashpay): release the request guard when a dispatch is refused pre-dispatch
The storage-update gate rejects every wallet-touching task — `DashPayTask`
included — with a bare `WalletStorageNotReady`, before it reaches
`run_dashpay_task`, the only place that wraps a failure into
`DashPayContactRequestActionFailed { request_id, .. }`. That typed variant is
also the only one `release_request_guard_for_error` matched, so a contact
request's Accept / Decline / Cancel clicked during the first launch after an
upgrade claimed a guard nothing would ever release: the row's buttons went dead
for the full five-minute in-flight timeout, long after the update finished.
A pre-dispatch refusal names no request precisely because nothing ran, which is
exactly the condition under which a blanket release is safe. `clear_in_flight` is
restored for that one match arm only — every other failure still keeps its guard,
so an unrelated error cannot re-enable a row whose paid action may still be in
flight.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(backend): pin ShieldedTask inside the wallet-touching migration gate
The shielded family is refused during a storage update only because it is listed
in `is_wallet_touching`; nothing failed if a refactor dropped that membership.
Sibling of `wallet_task_is_rejected_while_migration_awaits_password`, dispatching
a shielded write while migration awaits passwords.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui): state the disabled-tool reason once, as one translation unit
"Clear Platform Addresses" explained its own unavailability twice in the same
row — a tooltip ("...because...") and an italic label ("...while...") — giving a
translator two units for one idea, and drifting on the word that carries the
meaning: the tool is disabled permanently, so "while" is wrong. Keeps the
always-visible label (a tooltip on a disabled control is easy to miss) with the
permanent reading, and drops the tooltip.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs: correct the data-deletion promise and record the migration change set
NET-019 still promised to "permanently delete all local data" and that "the
action cannot be undone", which the shipped Clear Database dialog now
contradicts: it discloses that an earlier version's read-only recovery database
stays on the device and may still contain wallet recovery data. The story now
matches the dialog and its sibling WAL-007 — the population it is written for
(clearing a machine before handing it on) is the one it most misleads.
UX-001 described the progress block yielding its pointer sink to a passphrase
prompt but never said the prompt installs its own in its place, reading as if the
click-through hole were still open. It now states the hand-off as an invariant,
for every prompt, dismissible or not — an unwritten invariant is how that hole
was reopened the first time.
CHANGELOG covered only the DashPay change set. Adds the two user-visible ones it
missed: the per-wallet password prompt on the first launch after an upgrade (with
its safe skip path), the read-only recovery database that "Clear Database" and
"Remove Wallet" no longer erase and the developer tool that is disabled as a
result, and the shielded refusal message.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs(wallet): warn that SecretLease::lease() refcounts per call, not per scope
Independent verification of the SEC-002 fix (integration composition review)
found that lease() mints a fresh Arc on every call — two unrelated consumers
calling lease(scope) directly get two independent refcounts, so the first to
drop can evict the secret while the second is still relying on it. Not
currently reachable (the one call site correctly clones), but the type can't
enforce the invariant, so the next new consumer would reach for the public
lease() API and silently reintroduce the exact race SEC-002 just closed.
Document the footgun at the point of call rather than leave it undiscoverable.
* test(ui): prove the secret prompt's transition-frame click-through
egui resolves each frame's click at begin_pass against the previous frame's
widget geometry and modal layer. On the frame a passphrase prompt first
renders, the control beneath still existed last frame with no sink and no
modal layer above it, so the click completes on it before modal_chrome
installs the sink — mirroring AppState::update, where the visible screen
renders before render_secret_prompt.
- transition_frame_click_leaks_through_a_newly_activated_prompt: RED repro,
parked #[ignore]; un-ignore once the barrier is installed before the
visible screen renders on the activation frame.
- primed_prompt_blocks_the_same_injected_click_sequence: control (green) —
the identical injected click, with the prompt primed one frame earlier, is
absorbed. Isolates the leak to the transition frame, not the test harness.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(backend): refuse unavailable shielded ops early and scope DashPay gate rejections to one request
Two migration-gate refinements in run_backend_task, both closing bot-review
findings on PR #893.
Shielded pre-check: a shielded fund movement now short-circuits with
ShieldedOperationsUnavailable as the very first thing run_backend_task does —
before ensure_wallet_backend materializes seeds, registers upstream, and binds
Orchard for every loaded wallet just to run an op the app refuses. is_available
is a side-effect-free config read, safe before backend init; the in-handler gate
in run_shielded_task stays as belt-and-suspenders. Shielded ops are unavailable
on every network today, so this pre-check also precedes the migration gate: a
shielded write during a storage update now gets the accurate "not available"
message instead of a misleading "wait for the update".
DashPay guard scoping: the migration gate now tags a rejected contact action
(Accept/Reject/Cancel) with DashPayContactRequestActionFailed carrying its
request ID, so the Identity Hub releases only that request's in-flight guard.
The previous stopgap blanket-cleared every guard on a bare WalletStorageNotReady,
which could re-enable a different contact action's row while its paid state
transition was genuinely still in flight. release_request_guard_for_error drops
the blanket-clear arm; the now-unused clear_in_flight is removed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui): drop the transition-frame click when a passphrase prompt activates
egui resolves each frame's click at begin_pass against the previous frame's
widget geometry and modal layer, before update() runs. On the frame a passphrase
prompt first renders, the previous frame had no prompt and no input sink, so a
press-then-release completing now still lands on the control beneath — the modal
installs its sink one frame too late, and reordering the render within the frame
cannot help.
AppState::update now detects the prompt-activation rising edge (covering both the
just-in-time unlock and the migration password prompt, via
has_blocking_secret_prompt) and calls drop_activation_frame_pointer_click, which
clears this frame's pending pointer input before the screen beneath runs. A
widget only reports a click while a Released event is still in input.pointer, so
dropping it strands the leaked click; keyboard input is left intact for the
freshly focused password field, and the sink covers every later frame.
Un-ignores the transition-frame repro (now GREEN) and adds a migration-prompt
sibling; the primed-prompt and yielding-overlay sink tests still pass.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(ui): pin passphrase activation wiring in the real AppState update loop
The two existing transition-frame repro tests mirror
drop_activation_frame_pointer_click directly in a hand-rolled closure — they
never drive AppState::update(), so the production rising-edge call site in
app.rs was untested: deleting it left the suite green. Add
appstate_jit_prompt_activation_drops_transition_frame_click and
appstate_migration_prompt_activation_drops_transition_frame_click, which
mount a real AppState via build_eframe, activate a prompt through the actual
JIT (test_set_secret_prompt_active) and migration (MigrationStatus) paths,
and assert a click completed on the activation frame does not reach the
welcome screen beneath. Independently confirmed both fail when the app.rs
call site is neutralized and pass when restored.
Also corrects a stale doc comment/assertion in hub_screen.rs left over from
3e69b2fd, which removed the blanket WalletStorageNotReady guard-release arm:
the comment still described a "blanket release... scoped to refusals that
prove nothing is running" that no longer exists in the code.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(wallets): keep dialogs open on trigger clicks
Fix SND-003, WAL-005, and WAL-006 by ignoring outside-click dismissal on each dialog's opening frame.
Co-Authored-By: Codex GPT-5 <noreply@openai.com>
* build(deps): bump platform to PR3968 tip (d18020f5), pulls in the AssetLockProof rehydration fix
Updates dash-sdk / rs-sdk-trusted-context-provider / platform-wallet /
platform-wallet-storage git pins from 93b967f9 to d18020f5
(dashpay/platform#3968 tip), which includes the AssetLockEntryWire fix
for the AssetLockProof deserialize_any bug (dashpay/platform#4133) that
was blocking wallet rehydration on every relaunch once any asset lock
existed.
Adapts to unrelated upstream API drift pulled in by the same bump:
DataContractJsonConversionMethodsV0::to_json(&self, platform_version)
was removed as part of dpp's JSON/Value conversion trait unification
(dashpay/platform#3573, already a known pre-existing lint debt in this
branch). The canonical replacement for "give me this contract's
current wire-format JSON" is
DataContractInSerializationFormat::try_from_platform_versioned(...)
+ serde_json::to_value(...) — updated the 3 affected call sites
(contract_chooser_panel.rs x2, update_contract_screen.rs,
token_creator.rs); from_json usage elsewhere is unaffected.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* test(ui): make the opening-click regression test exercise the real guard
opening_click_does_not_immediately_dismiss previously only asserted that
seeding PassphraseModalState with an armed ModalOpeningGuard left the cache
entry readable — a plain data-cache round trip that never called
clicked_outside_window_after_open and could not fail regardless of the
guard's behavior. Rewrite it to simulate an actual outside click via
egui::RawInput and call the real function: the opening click must be
swallowed once, then a later check against the same pending click must
detect it normally. Also drops the internal commit-SHA reference in the
adjacent comment per the "describe present state, not history" convention.
Found by an independent adversarial review of this branch's merge (QA-001,
QA-002).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(contracts): don't panic when contracts can't load on Update Contract screen
UpdateDataContractScreen::new() called app_context.get_contracts().expect(...),
panicking the whole process when the contracts store errors (e.g. an unwired
wallet backend returns Err(WalletBackendNotYetWired)). Degrade gracefully
instead: fall back to an empty contract list and show a calm, actionable
MessageBanner with the underlying error attached via with_details(), matching
the established pattern in document_action_screen.rs and
group_actions_screen.rs.
QA-002. Implemented by Codex Sol, committed by the coordinator (this
sandbox's git metadata for the worktree is read-only, a recurring
environment constraint this session).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(dashpay): accept all integer encodings for contact-request key indices
derive_contact_payment_address() extracted senderKeyIndex/recipientKeyIndex
with a strict match on Value::U32, but network-fetched documents decode
integers as Value::I128, so extraction always failed with "Missing
senderKeyIndex" and DashPay payments could never succeed. Fixed by using the
canonical platform_value helper (to_integer::<u32>()) already used for the
same fields in contact_requests.rs, extracted into a small pure helper
(read_contact_request_key_indices) and unit-tested against I128/U32/I64.
Swept the rest of the DashPay backend for the same strict-match fragility and
converted three more sites the same way: contact_info.rs and contacts.rs
(derivationEncryptionKeyIndex/rootEncryptionKeyIndex) and
auto_accept_handler.rs (accountReference, previously handled with a manual
five-arm match — now the same single helper call).
DPY-006. Implemented by Codex Sol, committed by the coordinator (this
sandbox's git metadata for the worktree is read-only, a recurring
environment constraint this session).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(mcp): hydrate saved wallets before wallet-facing tools read them
ListWalletsTool::invoke (and every other tool that calls resolve::wallet(),
which reads ctx.wallets) ran before any SPV gate wired the wallet backend —
ctx.wallets is only populated inside WalletBackend::new via
AppContext::ensure_wallet_backend, and core_wallets_list deliberately skips
resolve::ensure_spv_synced. A fresh standalone det-cli process therefore
always reported {"wallets":[]} even with wallets already persisted to disk.
Added resolve::ensure_wallets_hydrated(), which wires the backend via
ctx.ensure_wallet_backend() with a throwaway sender — no SPV start, no sync
wait, idempotent on repeat calls — and called it ahead of every resolve::wallet()
call site that wasn't already behind ensure_spv_synced (17 tools across
wallet.rs, identity.rs, and shielded.rs). Updated docs/MCP.md and docs/CLI.md
to describe the new hydrate-on-demand behavior.
Verified with the exact det-cli two-process smoke flow from CLAUDE.md: import
a wallet in one process, list wallets in a fresh process against the same
data dir, confirm it appears with the expected seed hash and alias.
MCP-001. Implemented by Codex Sol, committed by the coordinator (this
sandbox's git metadata for the worktree is read-only, a recurring
environment constraint this session).
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
* fix(ui): stop the opening click from immediately cancelling confirmation dialogs (IDN-006, TOK-005, TOK-011, TOK-018)
Transfer Funds, token creation/registration, token claiming, and stop-tracking
all rendered their confirmation popup in the same egui frame as the button
click that triggered it. clicked_outside_window() read that still-active
click as a dismissal, so the dialog opened and cancelled itself within the
same frame -- visually indistinguishable from the button being a no-op.
Fixes it the same way passphrase_modal.rs's opening-click bug was fixed:
ConfirmationDialog now carries a ModalOpeningGuard, armed on construction and
consulted via clicked_outside_window_after_open() instead of the raw
outside-click check. The data-contract JSON popup gets its own guard for the
same reason.
Also fixes a compounding bug in the token creator: it rebuilt a brand-new
ConfirmationDialog (and therefore a freshly-armed guard) every single frame
via Option::insert(), which meant the dialog could never observe its own
post-opening frame. Switched to get_or_insert_with() so the dialog persists
across frames once created.
TOK-011 and TOK-018 share the same ConfirmationDialog component, so the fix
covers their reported no-op behavior without separate changes.
Implemented by Codex Sol, committed by the coordinator after independent
review of every hunk and a from-scratch fmt/clippy/test verification pass.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* fix(ui): wire missing identity navigation and fix stale post-refresh state (IDN-008, IDN-013a, DPN-008, IDN-009)
Navigation gap (IDN-008, IDN-013a, DPN-008): KeysScreen and the "My
usernames" list existed and worked but had no reachable route from Identity
Settings -> Advanced in the current Identity Hub build. Adds "Manage keys"
and "View all usernames" entries. Also corrects an inverted gate on the
Transfer screen's key-info button -- it only appeared when the identity had
*no* transfer key, backwards from the intended "manage the key you have"
flow. Key Protection (IDN-013a) was already fully implemented; it just
needed the same navigation fix to become reachable.
Refresh staleness (IDN-009): "Refresh identity data" fetched fresh state
from the network and persisted it correctly, but the backend task returned
the stale pre-refresh identity to the UI instead of the newly-fetched one,
and the Settings screen's own selected-identity cache only updated when the
identity's ID changed -- never on same-ID refreshes, which is the only kind
a refresh produces. Combined, a refresh could add a new on-chain key and the
UI would still show the old key count indefinitely. Fixed both: the backend
now returns the updated identity, and Settings reconciles same-ID refreshes
instead of only replacing on an ID change.
Implemented by Codex Sol, committed by the coordinator after independent
review of every hunk and a from-scratch fmt/clippy/test verification pass.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* fix(backend): stop silent hangs and panics in backend tasks (HANG-CLASS, IDN-002, MN-001, DOC-004, TOK-003)
Task-panic watchdog (HANG-CLASS): handle_backend_task/handle_backend_tasks
spawned work via tokio::task::spawn_blocking but dropped the returned
JoinHandle, so a panic inside the spawned closure vanished silently -- the
UI just hung forever with no error. The handle is now kept and awaited by a
managed watcher task; a panic or cancellation surfaces as a new typed
TaskError::BackendTaskFailed with a calm, actionable banner. The raw panic
payload is redacted from diagnostics (BackendTaskJoinError's Debug/Display
only expose task id / cancelled / panicked, never the panic message itself)
to avoid leaking arbitrary panic content into logs.
Network-request timeouts (IDN-002, MN-001, DOC-004, TOK-003): identity
loads (primary, voter, and DPNS-name fetches), document fetches, and token
lookups could all hang indefinitely on a stalled network call with no
feedback. Added a shared await_network_request_with_timeout helper (90s,
NETWORK_REQUEST_TIMEOUT) used at every affected call site, each mapping to
its own typed, actionable TaskError variant.
Token balance refresh needed more care than a plain timeout: the upstream
sync is not safely cancellable -- dropping it mid-flight could leave
is_syncing permanently stuck, trading a hang for silently-disabled sync
forever. Added await_managed_network_request_with_timeout: the request runs
as a detached, task-manager-tracked spawn; only the caller's *wait* on it
times out, so the sync itself always runs to completion even after the UI
gives up on it. A new token_balance_refresh_in_flight flag (RAII guard,
cleared on drop even if the refresh panics) also gives the refresh
single-flight protection so overlapping requests can't race.
Also fixed two more sites with the same forbidden string-match anti-pattern
DOC-004 was originally reported against (matching literal banner text
instead of message type to know when an in-flight fetch failed): the main
Tokens screen's RefreshingStatus and the token-claims screen's FetchStatus
both had the identical fragility and are fixed the same way.
Implemented by Codex Sol, committed by the coordinator after independent
review of every hunk and a from-scratch fmt/clippy/test verification pass.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* docs(user-stories): drop transient review-ID citation from UX-001
SEC-004 is an internal review-finding ID with no meaning outside the
review artifact that produced it — doesn't belong in a durable spec.
Flagged by Claudius-Maginificent's PR894 review.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* fix(app): stop migration frame race and preserve vote eligibility across migration
A single per-frame migration-state snapshot now backs every input-claim
and rendering decision (has_blocking_secret_prompt, claim_overlay_input,
ProgressOverlay::render_global, MigrationReconciler::update_banner)
instead of each call re-reading live state — closing a window where a
mid-frame migration transition could let the underlying screen consume
input for a frame where a blocking prompt was about to appear.
The periodic scheduled-vote sweep now defers while migration is in
progress instead of running unconditionally and silently skipping votes
whose imported identity isn't loaded yet. On migration completion, a
recovery sweep casts any vote whose normal 120s eligibility window
overlapped the deferred period, so a password prompt left open past that
window no longer permanently drops the vote.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* fix(dashpay): make contact-request decline/cancel idempotent
DashPay decline and cancel each broadcast a paid visibility transition
(contactInfo hide) before writing a local retire marker. A crash, retry,
or a second UI surface between the broadcast and the marker could
re-broadcast the hide and re-pay. Guard the flow so the paid hide runs at
most once per request:
- Add a durable per-request recovery journal (ContactRequestActionPhase)
in the DashPay k/v sidecar, scoped to the acting identity, so a retry
resumes at the last committed phase instead of re-broadcasting.
- Add a request-wide async lock plus a process-local in-flight claim so
concurrent declines/cancels serialize on one paid hide.
- Paginate contactInfo lookup and reuse it for a hidden-state probe so a
corrective unhide only fires when the contact is actually hidden.
- Correlate a panicked paid action back to its request id via
DashPayContactRequestActionFailed so the Hub releases only that guard,
and route contact-request results/errors to a hidden Hub screen.
- Retain paid-action guards across view resets; release only on the
correlated terminal result.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallet): fail Clear-Database safely and hydrate legacy wallets for MCP
Two wallet-readiness gaps:
- clear_network_database silently no-op'd its wallet-secret, DashPay
sidecar, and shielded cleanup when the wallet backend was not yet wired,
so "Clear Database" could report success while persisted secrets from an
earlier run survived. Require the wired backend up front and return the
dedicated WalletDataClearUnavailable error, leaving all state intact for
a safe retry.
- Standalone/headless MCP never awaited the cold-start legacy-data
migration, so legacy wallets were invisible to wallet reads. Hydrate and
finish the pending migration in ensure_wallets_hydrated, converting a
terminal MigrationState::Failed back into its typed task error.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(app): correlate task results to their originating operation
A backend-task error was routed purely by message type, so a concurrent
task's failure could trip an unrelated screen's in-flight status, and the
token-balance refresh guard could strand forever on a true hang.
- Introduce BackendTaskContext, attributed to each dispatch, and carry it
on TaskResult::{Success,Error}. Add display_backend_task_result /
display_backend_task_error so screens correlate a result to the exact
operation (document query, token-balance refresh, reward-estimate pair)
instead of matching on message text.
- Document, token, and claims screens now clear their in-flight status
only when the failing/completing task matches the pending one; an
unrelated failure no longer clears a genuine refresh banner.
- Suppress a duplicate token-balance refresh only while the first is
pending, and give the hung-refresh guard honest restart guidance.
Composes with the DashPay request-id correlation: forward_backend_task_join_error
now carries both the optional request id and the task context.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallet): also wipe identity private keys on Clear Database (SEC-001)
Clear Database wiped seeds, single-keys, DashPay overlays, and shielded
state but never removed identity private keys — the identity_key_priv.*
vault entries or the det:identity:* records. Those keys are Tier-1 keyless
(plaintext-recoverable) by default and include masternode voting/owner/
payout keys, so a user who chose to erase all local data still left
fund-control keys recoverable on disk.
The clear-all sweep already fans out over local_identity_ids() to drop
each identity's DashPay overlays; call the existing public helper
delete_local_qualified_identity for each identity in that same loop. It
runs clear_identity_vault_keys (-> IdentityKeyView::delete_all, wiping the
vault key bytes) and purges the identity scope + index (removing the
det:identity:* records), closing both halves of the gap.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallet): report partial failures when Clear Database can't delete every secret (SEC-002)
forget_wallet_local_state and forget_all_wallets_local logged each failed
per-secret delete but returned success unconditionally, so a failed seed,
single-key, or identity-key delete was still reported to the user as a
completed wipe — leaving recoverable secrets on disk behind a false
"cleared" message.
Accumulate delete failures instead of swallowing them:
- forget_wallet_local_state keeps attempting every step (resilient) but
returns the first failure so a partial wipe is never reported as clean.
- forget_all_wallets_local returns a ClearAllOutcome carrying the upstream
ids to remove plus every delete failure.
- clear_network_database collects those failures and the per-identity
wipe failures, still clears the in-memory maps, then returns the new
typed TaskError::WalletDataClearIncomplete { failed, #[source] first_error }
when anything failed. Its Display tells the user to restart and retry.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(dashpay): accept both decrypt-failure variants in unreadable-private-data test
read_contact_info_private_data decrypts contactInfo privateData with
unauthenticated AES-256-CBC + PKCS7 and a random IV. A wrong-key decrypt
usually fails PKCS7 unpadding (DecryptFailed), but roughly 1 in 256 the
random IV produces valid-looking padding and the garbage plaintext then
fails to parse (DeserializeFailed). Both mean the same thing — the stored
payload is present but unreadable, so the write aborts.
Two tests asserted ONLY DecryptFailed, so they flaked ~0.3% of full-suite
runs (confirmed: 4 failures in 1500 isolated runs before, 0 in 2500
after). Widen both assertions to accept DecryptFailed OR DeserializeFailed;
the product code's abort behavior is unchanged.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* test(contracts): isolate update_contract_screen degrade test from shared contract state
constructor_degrades_when_contracts_cannot_be_loaded asserts
known_contracts.is_empty(), which only holds when get_contracts() fails:
on success it always returns the pinned system contracts (dpns,
dashpay, ...) and "dashpay" is not in the constructor's excluded set. The
test built a real AppState, which wires the wallet backend asynchronously
inside the test's Tokio runtime, so whether get_contracts() saw a wired
backend (success -> non-empty) or not (error -> empty) raced the
constructor — the flake (green in the integration gate, red in CI).
Construct the screen from a backend-less test_app_context instead: with no
wallet backend wired, get_contracts() deterministically fails and the
constructor degrades to an empty list, which is exactly the path this test
names. Drops the DASH_EVO_DATA_DIR env-var dance and its module-local lock
entirely (0 failures in 60 isolated runs, was intermittently red).
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallet): report Clear-Database failure when the identity index can't be listed (SEC-VERIFY-001)
If local_identity_ids() fails, clear_network_database skips every
per-identity key wipe — yet it still returned Ok(()), so every identity's
private keys (incl. masternode voting/owner/payout) could survive behind a
false "cleared" message: the exact false-success class SEC-001/SEC-002
close, gated behind a listing error.
Push the listing error into the failures accumulator so it surfaces as
TaskError::WalletDataClearIncomplete instead of a silent success. The
warn log is kept.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui): add Back navigation to the Manage Keys screen (dead-end lockout)
KeysScreen renders a read-only key list pushed onto the screen stack but
returned AppAction::None unconditionally, trapping the user with no way
back to the identity view. Add a Back control in the header row that
returns AppAction::PopScreen, matching the sibling read-only detail
screens (e.g. contact_profile_viewer). A kittest asserts the button
renders and its click pops the screen off the stack.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui): show fee estimate and total before sending Dash (SND-005)
The Send Dash screen dispatched a payment with no fee or total shown, so
the user committed without seeing what would leave their balance. Add a
fee summary rendered directly above the Send button:
- Simple mode: estimated network fee, total deducted, and (when the fee
is taken out of the amount, e.g. Core -> Platform) what the recipient
receives. Covers every cleanly-estimable source/destination pair
(Core->Core/Platform/Shielded, Platform->Platform/Core/Shielded,
Identity->Core/Platform/Identity), reusing the same
model::fee_estimation estimators the amount field's "Max" reserve uses
so the two never disagree. Combinations whose fee depends on inputs the
backend selects at send time (identity top-ups, shielded spends) show a
neutral "calculated when you send" note instead of a wrong number.
- Advanced mode: estimated network fee for the count-driven paths
(Core->Core, Platform->Platform), else the same neutral note.
All fee math stays in model::fee_estimation; FeePreview only arranges
already-estimated numbers for display. Pure unit tests cover the on-top
vs deducted-from-amount total/recipient semantics and saturation.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(dashpay): report the correct cause when Add Contact can't resolve the recipient (NEW-002)
Sending a contact request to a recipient with no DashPay decryption key
raised DashPayError::MissingDecryptionKey, whose message ("Your identity
is missing a decryption key required for contacts") blamed the SENDER —
even though the sender's keys are fine and it is the RECIPIENT
(to_identity) that lacks the key. The Add Contact screen compounded the
error by offering an "Add Decryption Key" button that would add a key to
the sender's own identity, a remedy that cannot fix a recipient-side gap.
Rename the variant to RecipientMissingDecryptionKey and reword it to
correctly attribute the failure to the recipient with an actionable,
jargon-free message. Drop it from requires_user_action() and remove the
misleading self-remedy button — the sender has no key to add; the message
tells them to ask the recipient to finish setting up their profile. Both
error classifiers and their tests are updated to the renamed variant.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* docs: correct SND-005 fee-estimate criterion to match inline pre-send summary
The SND-005 acceptance criterion described the fee estimate as "shown in
confirmation dialog", but the HD-wallet Send Dash screen surfaces it
inline above the Send button (simple and advanced modes) before dispatch.
Reword the criterion to match the implemented behavior.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui): guard reusable modal components against opening-frame dismiss (NEW-003)
InfoPopup and SelectionDialog closed themselves on the same frame they
opened: the click that opened the popup lands outside the not-yet-rendered
window rect, so the unguarded clicked_outside_window() check fired true on
the opening frame and dismissed the popup before it was ever visible
(e.g. the token "More Info" popup never appeared).
Both components are value-constructed every frame from consumer-held state,
so a persistent ModalOpeningGuard field cannot survive across frames. Add
clicked_outside_window_after_open_by_id(), which records the last render
pass in egui temp memory keyed by a stable id and skips the outside-click
check on the opening frame — detected as a gap in rendering, so it re-arms
automatically however the popup was previously dismissed, with no teardown.
Fixing this inside the two components fixes every consumer at once
(InfoPopup: 13 call sites; SelectionDialog: no current consumers, so this
is preventive). Unit tests cover the opening-frame skip and the re-arm.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(ui): guard screen-level popups against opening-frame dismiss (NEW-003)
Six screen-level popups used the unguarded clicked_outside_window(), so the
click that opened them was seen as an outside click on the first render
frame and dismissed them before they appeared. Give each a persistent
ModalOpeningGuard field, arm it where the popup's open state is set, and
switch the check to clicked_outside_window_after_open() — mirroring the
existing wallets_screen rename-dialog and receive-dialog pattern.
Sites fixed:
- contracts_documents_screen: "Select Properties" fields dropdown
- dashpay/profile_screen: avatar-URL popup
- identities_screen: edit-alias modal (both open buttons)
- tokens my_tokens: "More Info" token popup and reward-explanation popup
- wallets add_new_wallet_screen: "Fund Wallet" receive popup
- wallets_screen/dialogs: fund-platform-address and mine-blocks dialogs
(the receive dialog in the same file was already guarded)
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(dashpay): show payment-history amount in DASH, not raw duffs (NEW-004)
The DashPay payment-history row printed the raw duffs value with a "Dash"
label — a 0.001 DASH payment rendered as "-100000 Dash" — because the
amount (duffs, from DashPayPaymentHistory) was formatted with `{} Dash`
and no unit conversion. Format it with `format_duffs_as_dash` so it reads
"-0.001 DASH". Fixed in both the Pay screen history (the live path) and
the contact-details history (currently unpopulated, fixed defensively);
documented that the `Credits`-aliased field actually holds duffs.
Counterparty label (NEW-004 part 2) is left as scoped follow-up: the
payment history resolves names against saved DashPay contacts only, so a
recipient paid by DPNS username (not a mutual contact) shows
"Unknown (<prefix>)". Resolving it needs a DPNS lookup by identity id or
persisting the send-time name — deeper plumbing than this cached-read
path — so it is marked with a TODO(NEW-004) rather than forced.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallets): confirm single-key removal and refresh asset locks after creation
WAL-007: single-key wallets had a separate Remove handler that called
forget() immediately, bypassing the HD wallet's confirmation state
entirely. Both wallet types now route through the same pending-removal
state feeding the existing danger confirmation modal.
ALK-002: Loaded(empty) was a terminal cache state as reported, but the
actual navigation gap was that PopScreenAndRefresh invokes
refresh_on_arrival(), not refresh(), which is where the cache
invalidation previously lived. The selected wallet's asset-lock cache
entry is now invalidated on root-screen arrival so a freshly created
lock is picked up on the next render.
Cherry-picked from 26937906 onto fix/snd-003-receive-inert. Conflict
resolution: the single-key-remove-button block was refactored into
request_selected_wallet_removal() (theirs); the snd003 branch's
customized HD-removal confirmation message (the earlier-version
read-only recovery-database note) was preserved into that method's HD
branch. The branch's NEW-003 rename-dialog ModalOpeningGuard usage in
this file is untouched.
Co-Authored-By: Codex GPT-5 <noreply@openai.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallets): restore password-modal focus without leaking background input
NEW-005 (release-blocking): the wallet-unlock / JIT secret password field
could not receive keyboard focus or typed input. `modal_chrome` registered a
separate full-screen "sink" Area as egui's modal layer; because that sink was
a different layer than the `egui::Window` holding the field, the window
resolved *below* the modal layer, so egui's `Memory::allows_interaction`
silently denied the `TextEdit` focus (and clicks on it).
Register the window's OWN layer as the modal layer instead: comparing a layer
against itself is `Equal`, so the modal's fields always resolve at/above the
modal layer and stay focusable, while every lower layer is blocked. The
full-screen sink is retained — moved to `Order::Middle`, strictly below the
`Order::Foreground` window — because it is load-bearing for background input
blocking: egui's `layer_id_at` only redirects a below-modal click to the modal
layer when some interactable area covers that position, so without full-screen
coverage a click landing outside the centered window would fall through to the
app beneath.
Add a kittest regression, passphrase_modal_password_field_focuses_and_blocks_background,
asserting the field takes focus and receives typed text (surfaced via Submit),
the modal layer is the window's own layer (not the sink), and a widget behind
the modal receives none of it. The pre-existing background-blocking and
dismissal kittests continue to pass, confirming the sink still blocks input.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(migration): gate startup migration on a minimum saved-data version
DET ran its startup data migration without checking the on-disk data
version, so migrating from an unsupported (too-old) version failed in
confusing ways. Read settings.database_version before migrating and gate
at both entry points (legacy-settings import and FinishUnwire) before any
sentinel or state is written; the legacy data.db is opened read-only.
Data versions 11..=40 migrate directly (v0.9.3 = 11 = the supported floor,
already migratable). Older data is rejected with an actionable "install
Dash Evo Tool 0.9.3 first" message; newer data fails closed. Fresh installs
(initialize writes v38 before the gate) are never rejected; a corrupt DB
missing the version row fails closed.
Typed errors SavedDataTooOld/SavedDataTooNew and LegacyDataTooOld/
LegacyDataTooNew carry numeric context and #[source] only (no user strings
in variants). Adds src/model/data_migration.rs (pure version classification).
Implemented by Codex (gpt-5.6-sol, high effort); data-safety reviewed
(gate-before-mutation, exact 11..=40 boundaries, fail-fast-no-write,
fresh-install-safe) — 0 blocking findings.
Co-Authored-By: Codex gpt-5.6-sol <noreply@openai.com>
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* test(migration): cover the too-new fail-fast and upper-accept boundary
Marvin flagged two LOW gaps in the startup DB-version gate. This feature is not
GUI-testable, so tests are its primary safety net.
- Add an async `finish_unwire::run` test (too_new_database_version_is_rejected_
before_migration) mirroring the existing too-old test: a version above the
ceiling (41) is rejected before any pass runs, surfacing the typed
SavedDataTooNew / LegacyDataTooNew chain, and NEITHER the completion sentinel
NOR any migration state is written (state stays Idle).
- Add an upper-accept boundary test (max_supported_database_version_is_accepted_
for_direct_migration): MAX_DIRECT_MIGRATION_VERSION (40) — the top of the
accepted 11..=40 range — is accepted, and the first version above it is
rejected as too new. Previously only 11-accept and 41-too-new were pinned; the
top of the accept range was never asserted accepted.
QA-001: document the deliberate headroom on MAX_DIRECT_MIGRATION_VERSION (40)
above DEFAULT_DB_VERSION (38), so data from a slightly newer build (39, 40) still
migrates rather than failing closed.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
* feat(settings): add developer-only Wipe Platform Data control (NEW-006)
Wire the previously-orphaned SystemTask::WipePlatformData to a developer-only
button on the Identity Hub Settings tab, gated to Devnet (the backend
wipe_devnet handler only clears devnet identities, tokens, and user contracts).
Guarded by a type-"WIPE"-to-confirm destructive dialog via a new
ConfirmationDialog::require_confirmation_text builder. Implemented by Codex Sol.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* feat(contacts): add View Profile action to Identity Hub contacts (NEW-007 / DPY-005)
Each active-contact row now offers a View Profile action that opens the working
ContactProfileViewerScreen for the selected contact, alongside the existing Pay
action. Reuses the same viewer the legacy DashPay paths use; the orphaned
ContactDetailsScreen is left untouched. Implemented by Codex Sol.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* fix(wallet): fail-closed on identity-key wipe and surface Clear-Database failures (SEC-001, SEC-002)
clear_identity_vault_keys now returns Result and propagates instead of
swallowing vault read/decode/delete errors, so identity removal (Clear
Database, identities screen, masternode detail, migration) reports incomplete
rather than clean when private keys — including masternode voting/owner/payout
keys — cannot be deleted. IdentityKeyView::delete_all attempts every key and
returns the first error instead of short-circuiting. DashPay sidecar/overlay
delete failures in clear_network_database are now accumulated into the failures
list (SEC-002) rather than warn-only. Adds a masternode-removal regression test
that injects a vault-key delete failure. Fixes found by security review.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* test: serialize DASH_EVO_DATA_DIR mutation with one shared lock
Replace per-module mutexes guarding DASH_EVO_DATA_DIR with a single crate-wide
lock in a new test_support module. Module-local locks let tests in different
modules race on the process-global env var under parallel execution, causing
intermittent AppState::new failures (e.g. add_token_by_id_screen's
display_task_result test). One shared lock serializes them deterministically.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* fix(ui): give each modal a unique guard id so popups don't dismiss each other (QA-001)
The opening-frame dismiss guard keyed on a single global Id shared by every
InfoPopup and SelectionDialog, so two independent popups on one screen (e.g. the
profile screen's Profile-Guidelines and Avatar-Guidelines info popups) shared
render-history state: closing one via outside-click then opening the other on
the next frame dismissed the second on its own opening frame. Each InfoPopup and
SelectionDialog now takes a caller-provided per-instance Id (mirroring
passphrase_modal), so guards no longer collide. Adds a two-popup regression test.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* fix(ui): move Wipe Platform Data beside its sibling network controls (NEW-006)
The developer-only "Wipe Platform Data" control (story NET-011) wipes data
for the whole devnet, but was rendered on the Identity Hub -> Settings tab,
a per-identity screen. Its siblings are network-scoped and live on the
Network Chooser: "Clear {Network} Database" (NET-019) in the "Database
Maintenance" section and "Clear SPV Data" (NET-020).
Move the control into "Database Maintenance", directly after the Clear
Database button, matching that file's danger-button styling and its
existing selected_role.at_least(UserRole::Developer) gating idiom.
Gating is unchanged and stays deliberately narrow: Developer role AND
Devnet. The devnet condition is load-bearing, not cosmetic, because the
backend wipe_devnet() is devnet-scoped
(delete_all_local_qualified_identities_in_devnet / _tokens_in_devnet /
clear_user_contracts).
Gate is now enforced in three places: the render check, a re-check in
show_wipe_platform_data_confirmation that dismisses the dialog if the gate
stops holding (so a dialog opened on Devnet cannot fire after a network
switch), and a fail-closed wipe_platform_data_action.
The type-WIPE confirmation and all user-facing wording are unchanged. Both
unit tests move to ui::network_chooser_screen::tests and now assert the
negative cases in both directions.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq
* fix(platform): keep cause-less transition results unconfirmed (#897)
A state transition can be accepted for broadcast while the separate result wait fails. Treat SDK broadcast-error envelopes without a structured consensus cause as submitted but unconfirmed, and direct the user to verify completion before retrying.
Co-authored-by: Codex GPT-5 <noreply@openai.com>
* docs(qa): PR892 user-story QA campaign — full retest record (175/175 stories) (#895)
* docs(qa): scaffold PR892 user-story QA campaign checklist
Populated progress.md with all 123 stories from docs/user-stories.md
(112 Implemented to test, 11 Gap pre-marked N/A). Note: source brief
referenced 152 stories incl. UX/IDH/MN categories that don't exist in
the current doc — proceeding with the doc as it actually is.
* docs(qa): confirm PR892 tx-history regression fix; WAL/SND/NET spot checks
Critical result: full quit + cold-boot relaunch on the same data dir now
correctly re-renders transaction history (was the PR892 bug). Verified
with 3 real testnet transactions via the Pasta faucet.
Also: NET-001 (switch networks) PASS, WAL-001/004/010/011/016/023/024
PASS, SND-001 PASS (nav only), SND-003 (Receive button) FAIL — no QR
code or modal appears, reproduced 3x.
* docs(qa): add shared campaign context for delegated per-category agents
* docs(qa): complete remaining WAL user-story QA pass (PR892)
Finishes WAL-002/003/005/006/007/008/012/013/017-020/021/022 a…
lklimek
merged commit Jul 16, 2026
a1f739b
into
docs/platform-wallet-migration-design
4 checks passed
thepastaclaw
suggested changes
Jul 16, 2026
thepastaclaw
left a comment
thepastaclaw
left a comment
Collaborator
There was a problem hiding this comment.
Preliminary review — Codex only
The latest delta introduces a blocking scope mismatch: commit81ef37e0 adds149 files and20,257 lines from unrelated #894 migration, DashPay, token, UI, MCP, and QA work to a PR still scoped to cold-boot transaction-history hydration. Both carried-forward suggestions remain valid at the current head: hydration still bypasses the upstream persistence boundary, and the live-before-hydration ordering remains untested.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (failed),gpt-5.6-sol— rust-quality (failed),gpt-5.6-sol— general (completed),gpt-5.6-sol— rust-quality (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 1 blocking | 🟡 2 suggestion(s)
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `Cargo.toml`:
- [BLOCKING] Cargo.toml:21-36: Remove the unrelated #894 change batch from this PR
The latest commit changes the pinned platform dependencies and adds149 changed files with20,257 insertions and2,385 deletions relative to the previously reviewed head. The batch includes migration, DashPay, token, UI, MCP, shielded, and QA-campaign work unrelated to restoring cold-boot transaction history. Because the PR title and description remain narrowly scoped to transaction-history hydration, this independently scoped batch cannot be reviewed or merged as part of this PR. Remove the #894 batch from the branch, or redefine the PR scope and restart review against the complete change set.
In `src/wallet_backend/mod.rs`:
- [SUGGESTION] src/wallet_backend/mod.rs:999-1043: Expose transaction enumeration through the upstream persister
The new hydration loader opens `platform-wallet.sqlite` directly, queries the private `core_transactions` table, and then performs one `get_core_tx_record` call per transaction. This creates N+1 reads and couples DET to upstream filename and schema details that dependency API checks cannot protect. The persistence trait also guarantees only `txid` and `context`, while `map_transaction_record` consumes the transaction body, amount, fee, and label; the concrete SQLite implementation currently returns those fields, but the API does not promise them. Add an upstream bulk full-record enumeration API so schema ownership and the complete record contract remain inside `platform-wallet-storage`.
In `src/wallet_backend/snapshot.rs`:
- [SUGGESTION] src/wallet_backend/snapshot.rs:443-446: Cover the live-before-hydration ordering
This insert-if-absent behavior protects a newer live record when it arrives before persisted hydration completes, but the only ordering test hydrates first and processes the live update second. Add the opposite-order regression test: accumulate a confirmed live record, hydrate an older unconfirmed record with the same txid, and assert that exactly one confirmed entry remains. Otherwise an unconditional-insert regression could overwrite newer live state while all current tests continued to pass.
Comment on lines
21
to
+36
| @@ -28,12 +28,12 @@ dash-sdk = { git = "https://github.com/dashpay/platform", rev = "93b967f9c7ab016 | |||
| "core_spv", | |||
| "shielded", | |||
| ] } | |||
| rs-sdk-trusted-context-provider = { git = "https://github.com/dashpay/platform", rev = "93b967f9c7ab0164b47fe825d2bae58b3974625c" } | |||
| platform-wallet = { git = "https://github.com/dashpay/platform", rev = "93b967f9c7ab0164b47fe825d2bae58b3974625c", features = [ | |||
| rs-sdk-trusted-context-provider = { git = "https://github.com/dashpay/platform", rev = "d18020f526e2a8eb1d1e868b436a7a9735795abb" } | |||
| platform-wallet = { git = "https://github.com/dashpay/platform", rev = "d18020f526e2a8eb1d1e868b436a7a9735795abb", features = [ | |||
| "serde", | |||
| "shielded", | |||
| ] } | |||
| platform-wallet-storage = { git = "https://github.com/dashpay/platform", rev = "93b967f9c7ab0164b47fe825d2bae58b3974625c", features = [ | |||
| platform-wallet-storage = { git = "https://github.com/dashpay/platform", rev = "d18020f526e2a8eb1d1e868b436a7a9735795abb", features = [ | |||
Collaborator
There was a problem hiding this comment.
🔴 Blocking: Remove the unrelated #894 change batch from this PR
The latest commit changes the pinned platform dependencies and adds149 changed files with20,257 insertions and2,385 deletions relative to the previously reviewed head. The batch includes migration, DashPay, token, UI, MCP, shielded, and QA-campaign work unrelated to restoring cold-boot transaction history. Because the PR title and description remain narrowly scoped to transaction-history hydration, this independently scoped batch cannot be reviewed or merged as part of this PR. Remove the #894 batch from the branch, or redefine the PR scope and restart review against the complete change set.
source: ['codex']
Comment on lines
+999
to
+1043
| let database_path = self.inner.spv_storage_dir.join("platform-wallet.sqlite"); | ||
| let connection = rusqlite::Connection::open_with_flags( | ||
| database_path, | ||
| rusqlite::OpenFlags::SQLITE_OPEN_READ_ONLY, | ||
| ) | ||
| .map_err(&storage_error)?; | ||
| let txid_bytes = { | ||
| // Upstream's public API decodes full records one txid at a time. | ||
| // Enumerate only its keys here, then delegate every record read. | ||
| // Project invalid widths to an empty blob before materialization; | ||
| // `Txid::from_slice` below then returns the typed hash error. | ||
| let mut statement = connection | ||
| .prepare( | ||
| "SELECT CASE WHEN length(txid) = 32 THEN txid ELSE X'' END \ | ||
| FROM core_transactions WHERE wallet_id = ?1 ORDER BY txid", | ||
| ) | ||
| .map_err(&storage_error)?; | ||
| let rows = statement | ||
| .query_map([wallet_id.as_slice()], |row| row.get::<_, Vec<u8>>(0)) | ||
| .map_err(&storage_error)?; | ||
| rows.collect::<Result<Vec<_>, _>>() | ||
| .map_err(&storage_error)? | ||
| }; | ||
| drop(connection); | ||
|
|
||
| let mut records = Vec::with_capacity(txid_bytes.len()); | ||
| for bytes in txid_bytes { | ||
| let txid = dash_sdk::dpp::dashcore::Txid::from_slice(&bytes).map_err(|source| { | ||
| TaskError::WalletTransactionHistoryLoad { | ||
| source: WalletTransactionHistoryError::Persistence { | ||
| source: WalletStorageError::HashDecode { source }.into(), | ||
| }, | ||
| } | ||
| })?; | ||
| let record = self | ||
| .inner | ||
| .persister | ||
| .get_core_tx_record(*wallet_id, &txid) | ||
| .map_err(|source| TaskError::WalletTransactionHistoryLoad { | ||
| source: WalletTransactionHistoryError::Persistence { source }, | ||
| })? | ||
| .ok_or(TaskError::WalletTransactionHistoryLoad { | ||
| source: WalletTransactionHistoryError::RecordMissing { txid }, | ||
| })?; | ||
| records.push(record); |
Collaborator
There was a problem hiding this comment.
🟡 Suggestion: Expose transaction enumeration through the upstream persister
The new hydration loader opens platform-wallet.sqlite directly, queries the private core_transactions table, and then performs one get_core_tx_record call per transaction. This creates N+1 reads and couples DET to upstream filename and schema details that dependency API checks cannot protect. The persistence trait also guarantees only txid and context, while map_transaction_record consumes the transaction body, amount, fee, and label; the concrete SQLite implementation currently returns those fields, but the API does not promise them. Add an upstream bulk full-record enumeration API so schema ownership and the complete record contract remain inside platform-wallet-storage.
source: ['codex']
Comment on lines
+443
to
+446
| for record in records { | ||
| per_wallet | ||
| .entry(record.txid) | ||
| .or_insert_with(|| map_transaction_record(record)); |
Collaborator
There was a problem hiding this comment.
🟡 Suggestion: Cover the live-before-hydration ordering
This insert-if-absent behavior protects a newer live record when it arrives before persisted hydration completes, but the only ordering test hydrates first and processes the live update second. Add the opposite-order regression test: accumulate a confirmed live record, hydrate an older unconfirmed record with the same txid, and assert that exactly one confirmed entry remains. Otherwise an unconditional-insert regression could overwrite newer live state while all current tests continued to pass.
source: ['codex']
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
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this PR exists
Problem: A wallet's transaction history renders empty even though its addresses hold a non-zero balance. The funds are visible; the transactions that produced them are not.
What breaks without it:
Confirmed against a real data directory:
core_transactionsheld 669 persisted rows while the UI showed nothing. The data was never missing — it was never read back.Blocking relationship: stacked atop fix: live-QA follow-up batch (masternodes, wallets, fonts) #887.
Root cause
The transaction list the UI reads (
WalletBackend::transaction_history) is served from the in-memory snapshot store, which was populated only by live wallet events arriving through the event bridge —SnapshotStore::accumulate_transactionshad exactly two non-test callers, both inevent_bridge.rs. Nothing ever hydrated it from the persistedcore_transactionsrows, so the store started empty on every launch and only filled with transactions observed live during that session.Balances did not share the defect because they are read from persisted UTXO state. Hence the asymmetry the user sees: persisted balance, in-memory-only history.
What was done
Deliberately not done: enabling upstream's
keep-finalized-transactionsfeature, or adding a second DET-side transaction cache. The rows are already on disk — the fix is to read them, not to persist them twice.Testing
cold_boot_hydrates_persisted_transaction_history_without_live_events— seeds a real persisted store, loads the wallet cold with no live events replayed, asserts history is returned. Verified RED first: fails on the unfixed code withpersisted history must load at cold boot: left: 0, right: 1(exit 101), passes after (exit 0).cargo +nightly fmt --all— clean.cargo clippy --all-features --all-targets -- -D warnings— clean.cargo test --all-features --workspace— green.Breaking changes
None.
Attribution
🤖 Co-authored by Claudius the Magnificent AI Agent