fix(wallets): link top-nav wallet selector to the Wallets tab by lklimek · Pull Request #878 · dashpay/dash-evo-tool

lklimek · 2026-07-10T14:29:42Z

Why this PR exists

Problem: the top-navigation wallet selector (breadcrumb pill) and the Wallets tab's own wallet picker are two independent selections that don't stay in sync.
What breaks without it: Imagine you are Priya, a power user with several wallets. You switch your active wallet using the top-nav pill while on the Identities page, then click over to the Wallets tab expecting to see that wallet's balances — instead you see whichever wallet the tab happened to remember from before, because the pill is inert on the Wallets page itself (Unwired, an explicit un-actioned TODO in the code) and the tab never re-reads the shared selection when you arrive. Two controls that both claim to answer "which wallet am I looking at" disagree.
Blocking relationship: stacked on top of PR feat: platform-wallet backend rewrite (spec + implementation) #860 (branch docs/platform-wallet-migration-design) — targets that branch, not v1.0-dev.

What was done

Made the top-nav wallet pill interactive (Consumed) on the Wallets tab — previously subdued/inert there, everywhere else it already worked. Reuses the existing effect-dispatch wiring, no new plumbing.
Fixed the Wallets tab's cached wallet handle to re-sync from the shared app-scoped selection every time the tab is arrived at, instead of only at construction/network-switch.
Closed a dual-hash ambiguity invariant discovered during the fix: the app tracks an HD-wallet selection and a single-key-wallet selection as two separate hashes, and none of the three setters that touch them (set_selected_hd_wallet, set_selected_single_key_wallet, set_selected_identity) consistently cleared the other on an explicit pick — meaning a stale selection could silently win over a freshly-chosen one. All three setters now self-enforce "at most one active wallet selection" regardless of which one is called.
Removed a pill-spec constructor that lost its only production caller in this change (dead code cleanup), trimmed a couple of over-length comments.
docs/user-stories.md: added WAL-028, extended WAL-004.

Testing

cargo clippy -p dash-evo-tool --all-features --all-targets -- -D warnings — clean.
cargo test/nextest -p dash-evo-tool --all-features (wallets/top_panel/hub_screen/global_nav/selected scope) — 46 passed, 0 failed, including a RED-then-GREEN test proving the dual-hash fix is actually necessary (it fails without the fix) and an invariant guard across all three setters (setters_never_leave_both_hashes_set).
cargo +nightly fmt --check — clean.
Independent QA pass: adversarial test execution against the committed tree (not the implementer's report), a security/correctness audit of the shared-setter change (no CRITICAL/HIGH/MEDIUM findings; confirmed no secret material touches this path, no persistence/atomicity issue, no cross-network leakage), and a project-consistency/dedup/docs review (no blockers). Two independent reviewers converged on the same residual asymmetry (the mirror direction of the fix), which was folded in before merge rather than deferred.
Known, accepted residual: no kittest test drives a literal pill-click on the Wallets tab specifically (the underlying effect-dispatch logic is unit-tested, and the pill's render/click path is covered generically by an existing global_nav_switcher kittest) — a manual GUI spot-check covers cross-network visual behavior, locked-wallet UX, and confirms no Wallets-tab dialog needs a "peek at a different wallet" capability this change would break.

Breaking changes

None. Internal state-sync fix; no public API, schema, or backend-task changes.

Checklist

cargo build / cargo clippy --all-features --all-targets -- -D warnings clean
cargo +nightly fmt --all -- --check clean
Unit tests added and green, including a test that fails without the fix
docs/user-stories.md updated
Manual GUI spot-check (cross-network, locked-wallet UX) — planned as a follow-up alongside the sibling QR-funding PR's GUI pass

Attribution

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

…tities only HD-wallet-backed identity keys are derived on demand from the wallet's own seed and already covered by that wallet's password; the Key Protection section is hidden entirely for such identities since there is no separate vault key to protect. Addresses PR #860 review comment. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…e 1b) Test case specification for restoring IDN-014 receive-deposit funding on the Register and Top-Up identity screens. Descriptions/expected-outcomes only, no test code. Corrects the Phase-1a brief: detection is single-address equality (not known_addresses), funding_address is per-screen, deposit transitions are unit-level via display_task_result (not kittest-reachable today). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…es (Phase 1b)" This reverts commit 8a150eb.

…es (Phase 1b) Test case specification for wiring the top-nav wallet pill (Defect 1: Unwired on Wallets page) and re-syncing WalletsBalancesScreen on arrival (Defect 2). 13 cases (TC-WALLETLINK-01..13), descriptions/expected only, no test code. Flags the dual-hash ambiguity trap: set_selected_hd_wallet preserves the single-key hash while in-tab SK selection clears the HD hash, and construction prefers SK first — so a stale (HD=Some, SK=Some) state resolves to the wrong wallet unless the pill's HD write is made authoritative. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Task breakdown for developer-bilby: D1 make the Wallets-page top-nav pill interactive (Consumed spec swap, no new plumbing), D2 re-sync the cached selection in refresh_on_arrival and fix the set_selected_hd_wallet / single-key-hash asymmetry so an explicit pill HD pick supersedes a stale single-key selection. References TC-WALLETLINK-01..13. AppContext API shape unchanged; single Bilby pass. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MXL4vmWNA1AjUeDVu2EqWA

Two defects that left the top-nav wallet pill and the Wallets tab's own picker out of sync (WAL-028): D1 — the Wallets-page pill was inert. Its spec built `PillConsumption::Unwired`, so the already-wired `apply_global_nav_effect` -> `set_selected_hd_wallet` path never fired. Add a `Consumed` `wallet_only_spec` constructor (mirroring the Masternodes page) and swap it in; delete the un-actioned TODO. Pill click now switches the active wallet in place, no navigation. D2 — the cached selection never re-synced on arrival, and a dual-hash asymmetry made a naive re-sync show the wrong wallet: `set_selected_hd_wallet(Some)` preserved a stale single-key hash while resolution prefers single-key-first, so a pill->HD switch re-resolved to the stale single-key wallet. Fix by clearing the single-key hash inside `set_selected_hd_wallet` when an HD wallet is picked (Some) — restoring symmetry with the single-key setter and making (HD, SK) both-set unrepresentable. Clearing the HD wallet (None) still leaves a single-key selection intact. Extract construction's store->wallet resolution into a shared `resolve_selection_from_store` helper; `refresh_on_arrival` now re-reads the store through it and adopts the result (idempotent when the cache agrees), falling back to first-wallet only when the store names nothing. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Flip WAL-028 (top-nav wallet pill on the Wallets tab) to [Implemented] and add the interactive-pill acceptance line to WAL-004. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

…setters Close the residual dual-hash gap QA flagged: the (HD, SK) both-set state was only prevented by convention of the one current caller, not by the setters themselves. - set_selected_single_key_wallet(Some) now clears the HD hash at the context layer (mirror of set_selected_hd_wallet), so a direct single-key set is self-enforcing regardless of caller, not only via the screen path. - set_selected_identity now clears the single-key hash when it sets an HD owner, so selecting a User identity while a single-key wallet is active can no longer leave the pill and the active identity disagreeing. - Remove the now-dead subdued_wallet_only_spec (its only production caller moved to wallet_only_spec) and its contrast test. - Trim three internal comment blocks to the length cap. Tests: add single_key_pick_supersedes_a_prior_hd_selection (the setter mirror of TC-07) and setters_never_leave_both_hashes_set (invariant guard across all three setters). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

coderabbitai · 2026-07-10T14:29:49Z

Important

Review skipped

Draft detected.

Please check the settings in the CodeRabbit UI or the .coderabbit.yaml file in this repository. To trigger a single review, invoke the @coderabbitai review command.

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 6a063b38-5d6a-4800-b376-74b174bab1fb

You can disable this status message by setting the reviews.review_status to false in the CodeRabbit configuration file.

Use the checkbox below for a quick retry:

🔍 Trigger review

✨ Finishing Touches

🧪 Generate unit tests (beta)

Create PR with unit tests
Commit unit tests in branch feat/link-topnav-wallet-selector

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.

❤️ Share

_{Comment @coderabbitai help to get the list of available commands.}

lklimek · 2026-07-10T15:59:58Z

First live GUI pass — wallet-selector linking

Tested on Testnet with 3 wallets loaded (qrtest [funded, has identities], wallet2, wallet3-locked [password-protected]), built from an isolated genuine recompile of HEAD c9cd89b3. Screenshots (LAN): http://agentic/dash-evo-tool/2026-07-10/partB-wallet-selector/

#	Test	Result	Evidence
1	Pill interactive on Wallets tab	✅ PASS	Clicking the top-nav pill on the Wallets tab opens the wallet switcher and switching updates the tab's own display (in-content picker, header, balance, address list) in place, no navigation. (`B01`→`B02`)
2	Re-sync on arrival	✅ PASS	Switched wallet via the pill on Identity Hub (a non-Wallets Consumed-pill page) → navigated to Wallets tab → it shows the just-picked wallet (qrtest), not the stale prior one. (`B04`→`B05`)
3	Two-way regression	✅ PASS	Switched via the Wallets tab's own in-content `ComboBox` → the top-nav pill reflects it on another page. (`B03`)
4	Single-wallet (Alex) regression	✅ PASS	Pill is count-gated (`global_nav_switcher.rs:65`: 0→placeholder, 1→subdued, ≥2→interactive). With one wallet the pill is subdued/info-only everywhere — unchanged by this PR (which only swaps the Wallets spec to `Consumed`). Observed: single-wallet pill opened no switcher.
5	Cross-network	✅ PASS	Switched to Mainnet → Wallets tab shows "No wallets yet" and the pill shows the "(no wallet yet)" placeholder — the 3 testnet wallets do not leak. Selection is per-network `AppContext`-scoped. (`B07`)
6	Locked-wallet UX	✅ PASS (core assertion)	Locked wallet3-locked, switched away, then switched to it via the pill — no unexpected password dialog appeared; the switch is a pure selection op. (`B06`) Caveat: the password was session-cached from creation, so a fully-cold locked-state test would need an app restart; the switching path touches no secrets so it cannot itself prompt.
7	UX consistency	✅ PASS	The now-interactive Wallets pill opens the identical wallet-switcher dropdown (wallet list + "Set up another wallet") as the Identity Hub / Masternodes Consumed pills, with uniform count-based behavior. The everyday pages (e.g. Identities) intentionally keep an `Unwired` subdued pill — a deliberate progressive-disclosure choice, not an inconsistency introduced here.

Verdict: ready to un-draft. The core linking (items 1–3) works correctly — the two selectors are now linked and sync both ways, immediately, across pages. Single-wallet, cross-network, and locked-wallet paths all behave correctly. Two minor follow-up-if-desired notes (not blockers): (a) item 6's fully-cold locked-state and (b) any additional edge of item 5 could be re-confirmed after an app restart, but neither is expected to change given the code paths.

Env note: the shared /data/target binary was clobbered by a concurrent worktree build during Part A of this pass; I built Part B into an isolated target with a forced sccache-off recompile and verified the feature is present before testing.

🤖 Generated with Claude Code

lklimek and others added 9 commits July 10, 2026 12:43

Revert "docs(test-spec): QR/receive-deposit identity funding test cas…

42e0a45

…es (Phase 1b)" This reverts commit 8a150eb.

docs(test-spec): trim wallet-selector-linking spec under word budget

ea3a661

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

docs(user-stories): mark WAL-028 implemented, extend WAL-004

4053869

Flip WAL-028 (top-nav wallet pill on the Wallets tab) to [Implemented] and add the interactive-pill acceptance line to WAL-004. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

fix(wallets): link top-nav wallet selector to the Wallets tab#878

fix(wallets): link top-nav wallet selector to the Wallets tab#878
lklimek wants to merge 9 commits into
docs/platform-wallet-migration-designfrom
feat/link-topnav-wallet-selector

lklimek commented Jul 10, 2026

Uh oh!

coderabbitai Bot commented Jul 10, 2026

Review skipped

Uh oh!

lklimek commented Jul 10, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Uh oh!

Conversation

lklimek commented Jul 10, 2026

Why this PR exists

What was done

Testing

Breaking changes

Checklist

Attribution

Uh oh!

coderabbitai Bot commented Jul 10, 2026

Review skipped

Uh oh!

lklimek commented Jul 10, 2026

First live GUI pass — wallet-selector linking

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant