feat: platform-wallet backend rewrite (spec + implementation)#860
feat: platform-wallet backend rewrite (spec + implementation)#860lklimek wants to merge 640 commits into
Conversation
|
Important Review skippedToo many files! This PR contains 526 files, which is 376 over the limit of 150. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (526)
You can disable this status message by setting the Use the checkbox below for a quick retry:
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
lklimek
changed the title
lklimek
changed the title
refactor: complete data.db unwire — shielded + DashPay (stacked on #860)
Doc sync —
|
SPV-start fix (PROJ-001, CRITICAL) + consolidated gap report —
|
DetKv per-object refactor + platform bump to
|
Mock/stub findings + SPV progress + DashPay fund-routing —
|
Platform dep bump →
|
Dead-code cleanup + gap-audit reconciliationPushed 2 commits (
Net: open-gap count holds at 17, but the audit books are accurate and a previously-hidden wiring defect (PROJ-012) is now surfaced as a real finding. 🤖 Co-authored by Claudius the Magnificent AI Agent |
Seedless wallet rehydration (PROJ-010) + platform → PR #3692Pushed 5 commits (
Planning docs (rode along): rehydration design (
🤖 Co-authored by Claudius the Magnificent AI Agent |
Fix: no-password hydrated wallets couldn't sign (Smythe SEC-001/002)
Symptom: after the seedless rehydration, a no-password HD wallet hydrated as Root cause: seeds reach Fix (one chokepoint, every path funnels through it):
The account-xpub fund-routing gate and seedless load path are untouched. 613 lib tests pass, clippy clean. Smythe re-verify in flight.
🤖 Co-authored by Claudius the Magnificent AI Agent |
JIT secret-access refactor — HD seed no longer parked in memory (R3 complete)Pushed 24 commits ( Core (
QA (three independent passes)
Also ( CHANGELOG + user-story WAL-006 updated for the new unlock model.
🤖 Co-authored by Claudius the Magnificent AI Agent |
NEW since last push — Settings Disconnect fix + persister-gap marker + nonce-ownership upstream issue (
|
NEW since last push — re-pin to dashpay/platform#3828: persister-loop fix + Orchard security patch + shielded-API port (
|
NEW since last push — fix PROJ-028/030 nullifier-cursor regression + v0.10-dev feature-parity audit (
|
|
I checked the asset-lock recovery angle from the JS SDK discussion. The identity paths look correct in this PR: But I do not think PR 860 fully handles the same issue for the non-identity asset-lock flows yet:
Those manual paths bypass the upstream So the fresh asset-lock transaction itself is tracked better than JS SDK, but these DET call sites still have a post-asset-lock / pre-final-accounting gap: Platform submit failures or confirmation ambiguity can leave a valid tracked lock in an intermediate/reusable state, and successful submits are not consistently marked Suggested fix: route these non-identity flows through the upstream wallet orchestrators, or mirror the same orchestration locally: resume/upgrade tracked proof, |
|
Verified your analysis against current source — it holds. Here's how PR #860 now addresses it. Shielded asset-lock flow ( Platform-address flows ( Also surfaced by review: the four sibling shielded spend fns ( All tracked in 🤖 Co-authored by Claudius the Magnificent AI Agent |
|
Correction to my earlier reply. I previously said the platform-address consume/retry orchestration was upstream-gated and not possible from DET. That was wrong — it is reachable through the public What landed (pushed):
Commits: Thanks for the catch — your read on the recovery gap was correct, and so was the implication that DET should route through the upstream orchestration. 🤖 Co-authored by Claudius the Magnificent AI Agent |
Two reliability fixes from live testing — crash visibility + DAPI ban storm (
|
Re-pin platform deps to PR dashpay/platform#3828 branch (
|
NEW since last push —
|
Follow-up landed —
|
NEW since last push — shielded backend retired in favour of upstream
|
NEW since last push —
|
Session update — 5 commits pushed (
|
|
Pushed The B-2 barrier is verified as an interim stopgap (closes the common coordinator-thread drain race). The durable fix — a reusable cancel + await-exit shutdown primitive ( 🤖 Co-authored by Claudius the Magnificent AI Agent |
- QA-002: Shield-from-Platform "Max" now reserves the two-action shielded-fee headroom (>50M credits) instead of the plain platform-transfer estimate (~8M). ShieldFromBalance pays the shield fee from the same balance as the amount, so the old reserve under-shot ~6x and a Max attempt was rejected upstream. New model helper `shield_from_balance_fee_headroom` keeps the fee math in model/fee_estimation.rs. Shield-from-Core parity was already correct. - QA-001: restore raw-hex shielded-recipient entry. `AddressKind::detect` and `AddressInput::validate_shielded` now accept the 43-byte (86-hex-char) form via the shared `parse_shielded_recipient`, matching what the old private-send screen advertised. - QA-003: on a network switch, `WalletSendScreen` now drops the wallet seed hash, source, destination, and amount (`reset_for_network_switch`) so a preset flow can no longer resurrect stale cross-network state / balance. Tests: model coverage for the fee headroom (asserts the shielded-fee reserve, not the transfer estimate) and for hex shielded detection at both the model and AddressInput layers. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…unified send
Round-2 audit finding CODE-098 ("unified send screen is the way to
go, make it reusable and reuse"): shield_screen.rs, shielded_send_screen.rs,
and unshield_credits_screen.rs deleted; WalletSendScreen gains a
SendFlow preset enum {General, Shield, ShieldedSend, Unshield} +
with_flow() builder, locking source/destination appropriately per
flow (Shield's destination is dispatch-ignored, satisfied by a
discriminant-only sentinel). shielded_tab.rs routes through
open_send_flow() instead of three standalone screens. Shared shielded-
recipient parsing (bech32m + 43-byte hex) extracted to
model::address::parse_shielded_recipient. Net -178 lines from the fold
itself.
Independent QA pass (funds-adjacent surface, given this touches
shield/unshield/shielded-send dispatch) found and this includes fixes
for:
- Shield-from-Platform's Max button under-reserved the shield fee by
~6x versus the old screen (new shield_from_balance_fee_headroom()
in model/fee_estimation.rs restores the correct >50M-credit reserve).
- Raw-hex shielded-recipient entry was dropped from the UI even though
the model parser still accepted it (AddressInput/AddressKind now
detect and validate the 43-byte hex form too).
- A network switch didn't clear a preset flow's stale wallet/seed-hash
state, risking a stale cross-network balance display (send stayed
blocked, but misleading); WalletSendScreen::reset_for_network_switch()
fixes it.
Sentinel-destination handling, dispatch routing for both Shield
sources, and the unlock gate were independently verified correct with
no changes needed.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Strip development-history narration and ephemeral review IDs from comments; keep present-state invariants only. No executable logic changed. - CODE-021: rewrite archaeological rationale (det_platform_signer, snapshot, wallet_backend/mod, secret_seam, wallet_seed_store, wallet_lifecycle stop_spv); dedup the AlreadyOpen restart-in-place rationale to one copy. - CODE-067: present-state the Phase B/D/E + Post-D4c narration in backend_task/shielded/mod.rs and dashpay.rs; stale "until Phase-E lands" reworded now the push writer exists. - CODE-084: drop RUST-001 / 6a2818cd IDs from fee_estimation.rs and wallet/single_key.rs comments; keep the durable TS-DBG-01 test-spec ID. - CODE-102: delete the shielded_tab tombstone (keep "Fund-moving results only."); present-state the "replaces the dropped/legacy…" docs in wallets_screen, contacts_list, contact_details. - CODE-106: drop INTENTIONAL(CMT-010/RUST-003/CODE-003) prefixes and FIX N markers in theme, message_banner, address_input, add_new_identity_screen, key_info_screen. - PROJ-007: remove the removed-subsystem ZMQ sentence and the "RPC, ZMQ" ConnectionStatus residue from CLAUDE.md; protoc v25.2+ verified against CI. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…(final)
CODE-021/067/084/102/106, PROJ-007 (round-2 audit, run last since it
touches files across every prior wave): comment-only sweep, zero
executable logic changed. Development-history/archaeology narration
("Phase B/D/E", "Post-D4c", "used to read", ephemeral commit-hash/
finding-ID references) rewritten to present-state descriptions or
concrete TODOs across det_platform_signer.rs, snapshot.rs,
wallet_backend/mod.rs, secret_seam.rs, wallet_seed_store.rs,
wallet_lifecycle.rs, backend_task/shielded/mod.rs, dashpay.rs,
model/fee_estimation.rs, model/wallet/single_key.rs, shielded_tab.rs,
wallets_screen/mod.rs, contacts_list.rs, contact_details.rs, theme.rs,
message_banner.rs, address_input.rs, add_new_identity_screen/mod.rs,
key_info_screen.rs; durable test-spec IDs (TS-DBG-01) kept. CLAUDE.md
corrected: removed the stale per-platform ZMQ mention and narrowed the
ConnectionStatus health list to SPV+DAPI (RPC/ZMQ subsystems were
retired in the platform-wallet migration); protoc v25.2+ claim
verified accurate against CI config.
This is the last of 21 planned waves plus the CODE-098 follow-up --
round 2 of the full-repo architecture audit is now fully merged.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Adds Changed/Fixed entries for the genuinely user-observable outcomes of the round-2 architecture audit (21 waves + CODE-098): the shielded screens' fold into the unified Send screen, DashPay's now-optional display name, two identity-funding UX fixes, the My Tokens loading-spinner hang, and a settings-save race. Internal refactors, dedup, and error-typing from the same round are intentionally omitted. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Identity Hub hero card painted its 14%-opacity gradient band using `ui.max_rect()` — the available space, not the card's content bounds — so on the Home tab the band bled downward through every sibling widget below the card (quick actions, onboarding checklist, recent activity). Reserve a shape slot before laying out content, then fill it afterward from `ui.min_rect()`, confining the band to the card's actual bounds. Add unit tests asserting the strips stay within the given rect. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…urces Convert the largest cluster of `detail: String` TaskError variants to typed `#[source]`/`#[from]` variants and delete the dead ones, tightening the Display/Debug separation and reducing `result_large_err` pressure. - platform_info: introduce a `WithdrawalParseError` source enum (platform-value field errors, missing/invalid timestamps, unrecognized status, boxed ProtocolError for the daily limit). `WithdrawalDocumentParsingError` now wraps it via `#[from]`; the two duplicated per-document formatting blocks are extracted into `format_withdrawal_line` / `format_completed_withdrawal_line`. `ShieldedSyncFailed` now carries `Box<SdkError>`. - key input: move `verify_key_input` out of the backend into `model/key_input.rs` as the stateless single source of truth, returning a typed `KeyInputError` (NotHex / BadWif / UnsupportedLength) with complete, i18n-ready sentences. `KeyInputValidationFailed` becomes a transparent `#[from]` wrap, removing the double-naming and fragment concatenation; callers use `?`. Unit-tested. - dashpay: delete the unconstructed `reason: String` variants BroadcastFailed, QueryFailed, PlatformError, RateLimited and their dead retry-classification clone arms; make the sole remaining recoverable variant `NetworkError` fieldless. - delete unconstructed TaskError variants (WalletUtxoReloadFailed, WalletBalanceRecalculationFailed, WalletPaymentFailed, UtxoUpdateFailed, SerializationError, RpcProviderCreationFailed, AssetLockTransactionBuildFailed, ShieldedTreeUpdateFailed, ShieldedNullifierSyncFailed, ShieldedMerkleWitnessUnavailable); make InvalidPrivateKey and NetworkContextCreationFailed fieldless (their strings were hardcoded, not upstream errors). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The 'contract not found when querying from value with contract info' substring match in the three contested-name query retry loops is a deliberate workaround: the condition originates server-side as QuerySyntaxError::DataContractNotFound and reaches the client only as a gRPC Internal status with message text, so no client-side structural SDK variant exists to match on yet. Reference the tracking issue at all three sites. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… the secret seam, and type encryption key hygiene
Three wallet-backend hardening changes.
- Lock-poison policy: add `wallet_backend/poison.rs` with `read_recover` /
`write_recover`, which recover a poisoned `RwLock` guard instead of erroring.
The single-key in-memory index and the secret session cache guard derived,
rebuildable state, so recovery is correct and self-healing. Route the
single-key index (imports, alias, forget, rehydrate, list) and the
`SecretAccess` session cache (lookup, eviction, remember) through it, deleting
the two lying poison mappings — `ImportedKeyNotFound` (told the user to
re-import) and `SecretDecryptFailed` (claimed a decrypt failure).
- Secret seam: route every raw vault access in `single_key.rs` through
`SecretSeam::{put_secret, get_secret, put_secret_protected, delete_secret}`
instead of hand-rolled `SecretStore` calls, so imported keys honor the same
chokepoint and failure variant (`TaskError::SecretSeam`) as their siblings.
The verify-passphrase wrong-password signal is preserved by matching the
seam's typed `SecretSeam` source structurally.
- Encryption hygiene: `derive_password_key` returns `Zeroizing<Vec<u8>>` and
`ClosedKeyItem::decrypt_seed` returns `Zeroizing<[u8; 64]>` (copied straight
into the zeroizing buffer, no bare stack copy), so the derived AES key and the
decrypted seed wipe on drop. Introduce a typed `EncryptionError`
(WrongPassword / Malformed / KeyDerivation / Encryption, Everyday-User
messages) and return it from the encryption primitives and the two
`ClosedSingleKey` crypto methods, replacing their `Result<_, String>`. The
broad `SingleKeyData::open` / `WalletSeed::open` / `SingleKeyWallet::new`
String APIs (pre-existing model/wallet debt that mixes crypto with key-parse
errors) render the typed error through `Display` at the boundary; a full
type-through of those APIs is deferred.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
… the credits-to-address MCP destination Two coupled changes around address↔network validation. - Single source of truth: add `validate_platform_address_for_network` and its Orchard twin `validate_orchard_address_for_network` to `model/address.rs`, returning a typed `AddressNetworkMismatch` (Everyday-User message). They parse the bech32m HRP (`dash1…` mainnet, `tdash1…` testnet, case-insensitive) and reject a cross-network address. `address_input.rs` delegates its hand-rolled `validate_platform` / `validate_shielded` prefix checks to these, keeping the same banner copy; the GUI's format/length/case checks stay put. - Close the MCP network gap (funds-adjacent): `identity_credits_to_address` decoded the destination with `PlatformAddress::from_bech32m_string` and no network check, so a mainnet `dash1…` address could be paid on testnet (or vice-versa). Gate the parsed destination against `ctx.network()` via the new model validator, returning `InvalidParam` on mismatch — mirroring the withdraw tool's Core `require_network` guard. The required `network` param only pins the active network; it never validated the destination, which was the gap. The `resolve::validate_address` first-Base58-char Core heuristic is left as-is: it is a network-agnostic format sanity check with no network parameter, and Core network validation is already performed via `Address::require_network` at each tool. Adding a model Core validator would duplicate that with no clean delegation target, so it is deferred rather than widened here. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…upt address rows; drop per-item logging loops
Three wallet-layer cleanups from PR review.
- Dedup wallet lookup: add `AppContext::wallet_arc(&self, &WalletSeedHash) ->
Result<Arc<RwLock<Wallet>>, TaskError>` as the single source of truth for the
"look up a wallet arc or `WalletNotFound`" pattern, replacing 15 copy-paste
blocks across backend_task/{wallet,identity,shielded} and context. The helper
recovers a poisoned lock via `wallet_backend::poison::read_recover` rather than
erroring: the in-memory wallet map is rebuildable, so recovery matches the
Wave-2 poison discipline for rebuildable state. This is a behavior change on
the rare poison path — the replaced sites previously surfaced `LockPoisoned`
via `.read()?`; they now self-heal, consistent with `mcp::resolve::wallet_arc`
(which now delegates to this helper and re-wraps the id-bearing MCP error).
- Loud failure on corrupt rows: `database/wallet.rs` `get_wallets` skipped
unparseable address rows silently. Silently dropping an address could hide a
missing key and lead to lost funds, so log a `warn!` (row index + error) and
continue, matching the wallet_backend hydration skip-logging discipline.
- Drop per-item logging loops: `fetch_platform_address_balances` logged every
address/balance/nonce and `transfer_platform_credits` logged every input
address/amount at `info!`. Both keep their aggregate summary line; the
per-address financial detail is removed — it violates the no-loop logging rule
and should not sit in plaintext logs at the default level.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…ropagation and graceful UI degradation
Eliminate every bare `.unwrap()` on the non-test production paths (~150 sites)
so a broken invariant degrades instead of aborting the process. Applied by
category:
- Lock poison recovery: extend `wallet_backend/poison.rs` with a `Mutex`
`lock_recover` free fn plus ergonomic `RwLockRecover` (`read_recover` /
`write_recover`) and `MutexRecover` (`lock_recover`) extension traits. Route
the wallet/step/settings `RwLock` and the DPNS/identity/token-search `Mutex`
screen-state locks — all rebuildable in-memory state — through them, so a
thread panicking mid-update never wedges the UI. Unify the coordinator gate's
action mutex on the same recovery policy (its `should_fire` already tolerated
poison via `is_ok_and`, while `arm`/`try_fire`/`reset` panicked). The
`Mutex<Connection>` gains a `Database::locked_conn()` helper that recovers the
guard: a `rusqlite::Connection` carries no invariant a panic can break, so
recovering avoids cascading one unrelated panic into every later DB call.
- Graceful UI fallback: egui frame paths return `AppAction`/`BackendTask` and
cannot `?`. The `Option::unwrap()` selection reads now guard with `let-else` /
`if let` and degrade — an actionable `MessageBanner`, an early return, or a
logged skip — instead of panicking. The document-action builders share a new
`require_selections()` helper that returns `BackendTask::None` with a banner
when a selection is missing.
- Invariant expects: unwraps that are provably infallible after a preceding
guard, on constants, or on a construction invariant are upgraded to
`.expect("invariant: …")` so a future regression fails loudly with context
rather than a bare panic. BIP32 child indices are documented as `< 2^31`
(hardened constants, small coin type, and hash indices masked with
`& 0x7FFFFFFF`).
The `dashpay_increment_send_index` mutex keeps its documented fail-loud
poison contract: a panic mid-increment can leave the address-index counter
inconsistent, and surfacing that is safer than handing out a duplicate index.
Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…, and a redundant predicate Four small cleanups; every deletion grep-verified against both the lib and the integration-test crates first. - Ephemeral review-IDs: strip the per-run tags from committed source — the `RUST-002` tag on the app.rs banner-flash TODO (the substance and the #660 issue link stay) and the `CODE-027` prefixes on two settings_db doc comments (replaced with a plain statement of the read-modify-write guarantee). The standards-body `RUSTSEC-2025-0141` reference is deliberately kept. - Tombstones: drop the two deleted-code tombstones in `database/wallet.rs` `get_wallets` (git is the record) and renumber the trailing "step 8" to "step 4" so the step comments read coherently. - Dead code: delete genuinely-unused speculative items and the code they guard — three unconstructed `BackendTaskSuccessResult` variants, the `CoreTask:: GetBestChainLock` variant plus its handler / PartialEq arm / classification test (and the now-orphaned `AppContext::rpc_error_with_url` its handler was the sole caller of), a duplicate `decrypt_private_data`, an unused avatar `to_grayscale`, three unused `KeyStorage` accessors, an unused token-info constructor, a write-only `core_address` field, and a dead visualizer search field. Items that turned out to be live are handled correctly instead of deleted: `AppAction::Refresh`, `IdentityTask::SearchIdentityFromWallet`, `KeyStorage::keys_set`, and `send_screen`'s `selected_wallet_seed_hash` had stale `#[allow(dead_code)]` (the attribute is simply removed); `CORE_APPLICATION` is non-Linux-only, so it becomes `#[cfg(not(target_os = "linux"))]` and is only compiled where used; and `MessageBanner::{has_global, set_auto_dismiss}` plus `build_identity_registration` are exercised by the kittest / backend-e2e integration-test crates (a separate compilation the lib does not see), so they keep `#[allow(dead_code)]` with a reason comment rather than `#[expect]`, which would be unfulfilled under the `--all-targets` gate. - Redundant predicate: inline `is_distinct_change_candidate` (a one-line `candidate != destination` wrapped in a documented fn with two dedicated tests) at its single call site and delete the fn and its tests. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Present-state architectural rationale about a module boundary — Platform per-address balances and the sync cursor are owned by the upstream coordinator, so DET keeps no at-rest copy and warm-starts from the coordinator's first push. This is an external-constraint explanation, not a git-history tombstone, and was removed in error during the tombstone cleanup. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…y-scoped submodules Pure code-move: converts the 4305-line `src/context/wallet_lifecycle.rs` (the repo's largest file) into a module directory whose `impl AppContext` methods are grouped by responsibility across submodules — the same multi-impl-of-one-struct layout `wallet_backend/` already uses. No logic, signature, or behavior change; the only signature edits are two visibility widenings noted below. New structure (`src/context/wallet_lifecycle/`): - `mod.rs` — module docs, shared imports/constants, the three free helpers (`spv_storage_dir`, `clear_spv_chain_storage`, `cleanup_legacy_shielded_files`), the `AppContext::wallet_arc` lookup, and the submodule + test declarations. - `spv.rs` — backend wiring and chain-storage lifecycle: `clear_spv_data`, `clear_network_database`, `ensure_wallet_backend_and_start_spv`, `mark_spv_error`, `stop_spv`. - `registration.rs` — single-key import and HD-wallet registration/persistence: `import_single_key_wif`, `verify_single_key_passphrase`, `register_wallet`, `register_wallet_upstream`, `write_seed_envelope`, `write_wallet_meta`, `promote_seed_to_session`. - `removal.rs` — `remove_wallet`. - `bootstrap.rs` — address derivation and post-unlock warmup: the `bootstrap_*`/`wallet_needs_bootstrap` methods, managed-identity reconciliation and contact-account/auth-key warming, and the identity-discovery queue. - `unlock.rs` — lock/unlock handling and the open-wallet snapshot: `handle_wallet_unlocked`, `drive_unlock_registration`, `handle_wallet_locked`, `open_wallets`, `unregistered_open_wallet_count`. - `tests.rs` — the `#[cfg(test)]` module, unchanged. Submodules access `AppContext`'s private fields as descendants of `context` (no field-visibility change) and share `mod.rs`'s imports via `use super::*`. The only forced visibility widenings: `open_wallets` and `reconcile_managed_identities` go from private to `pub(super)` because they are now called from a sibling submodule (and, for both, from the test module). Verified move-integrity: the set of `impl AppContext` method names is identical before/after, the test-function count is unchanged (56), clippy `--all-features --all-targets -D warnings` is clean, and the full `--lib` suite passes 1393/1393. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
The Identity Hub hero card rendered a 14%-opacity blue→purple wash that read as a heavy blue block rather than the subtle diagonal accent the wireframe implied. Per the design rule that wireframes govern placement while colors follow the app theme, drop the gradient entirely: the card is now a flat `DashColors::surface` fill with the existing border and elevation. This supersedes the earlier gradient-containment commit — with no gradient, the reserved-slot machinery and its geometry tests are removed along with the now-unused colour-lerp helper. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
…sts; recover remaining identities-screen lock Resolves the QA blockers on the branch. - Merge gate (workspace test) was red: the Wave-6 god-file split moved the wallet_lifecycle test fixtures — which construct legacy-schema `wallet` / `single_key_wallet` rows — from the monolithic file into the split module's `tests.rs`, but `tests/legacy_table_surface.rs`'s allow-list still pointed at the old path, so TC-DEV-001/003 fired on the moved (previously-allowed) fixtures. Repoint the single allow-list entry to `src/context/wallet_lifecycle/tests.rs`. Verified the guard patterns (`FROM wallet`, `INTO single_key_wallet`) appear only in that test file — no production submodule contains legacy-table SQL — so the exemption is unchanged, only its path. - The unwrap sweep missed one `self.identities.lock().unwrap()` in `identities_screen.rs::ui()` (it was split across lines, so the single-line pass skipped it): on a poisoned identities mutex that screen would panic on every repaint instead of self-healing. Convert it to `.lock_recover()` like its ten siblings. A multi-line re-scan of the sweep-touched files found the same line-wrapped gap in four more production wallet/single-key `RwLock` reads (add_existing / add_new identity screens, wallets screen); convert those to `.read_recover()` too. Remaining line-wrapped matches are all test-only code (`#[cfg(test)]` modules, `kv_test_support`) and are left as-is. - Refresh the `poison.rs` module doc to describe its present usage surface — the loaded-wallet map, per-screen view state and settings cache, the Identities / DPNS / token-search screen caches, the coordinator-gate slot, and the SQLite connection mutex — rather than only the two wallet-backend caches it originally named, and note the DashPay address-index mutex's deliberate fail-loud exception. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
fix: round-2 full-repo architecture audit (21 waves + shielded-screen consolidation)
…et rewrite (#876) * fix(withdraw): pre-select only a locally-signable withdrawal key The Withdraw screen constructor pre-selected a key via the on-chain lookup `identity.get_first_public_key_matching(TRANSFER, ...)`, which is unfiltered by local private-key presence. On loaded masternode/evonode identities where only the Owner key was supplied, this picked a "ghost" Transfer key with no local private material, so the withdrawal failed at signing with a raw, unhelpful protocol error. - model: add `QualifiedIdentity::default_withdrawal_key()` — sourced from `available_withdrawal_keys()` (private-key-backed only), Transfer preferred with Owner fallback, `None` when nothing is signable. - ui: constructor now pre-selects via `default_withdrawal_key()`; the developer-mode on-chain escape hatch is preserved. When no usable key exists the existing empty-state guides the user to add one. - error: add `TaskError::NoWithdrawalSigningKey` (typed `#[source]`, plain-language actionable Display) mapping the SDK `DesiredKeyWithTypePurposeSecurityLevelMissing` protocol error as a defense-in-depth backstop instead of leaking a raw string. - tests: 4 model cases (ghost key rejected, private-backed selected, owner fallback, transfer preferred) + 2 error-mapping/Display cases. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(claude): correct secret-storage note on identity-key encryption Identity keys (imported/loaded, including masternode voting/owner/payout) are no longer categorically in the deferred keyless tier: they enter unprotected at load time but can be sealed to Tier-2 per-identity via IdentityTask::ProtectIdentityKeys (Key Info screen "Add password protection"). Clarify that the keyless residual is only no-password secrets and keys the user has not opted to protect. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * feat(masternodes): add page-nav model with two-scope selection (A1) Introduce ui/state/global_nav.rs: PageNavSpec (page-aware segment-1 + per-page pill composition) and IdentityPillScope (AppGlobalUser vs PageScopedObject). The PageScopedObject variant carries its own selection and never writes AppContext::selected_identity_id — the structural FR-6 boundary the global switcher (A2) and the Masternodes page (B7) build on. Pure state, renders nothing (module-placement discriminator -> ui/state). Satisfies TC-NAV-13, TC-NAV-14, TC-NAV-15; foundation for TC-NAV-12, TC-FR6-07. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): generalize breadcrumb into page-aware global switcher (A2) Add ui/components/global_nav_switcher.rs: a page-aware switcher driven by PageNavSpec, rendering segment-1 (page label + link) plus composable wallet / identity-or-object pills. GlobalNavEffect generalizes the hub's BreadcrumbEffect and adds SelectPageObject for the page-scoped pill — kept distinct from SelectIdentity so a page-scoped selection never writes the app-global identity (FR-6 boundary at the effect level). Reuses BreadcrumbPill / IdentityPill / BreadcrumbPillMode verbatim (no new pill widget). breadcrumb_switcher.rs becomes a thin hub-facing shim that builds the hub spec, delegates to the generalized render, and maps the effect back (self-nav to the hub root -> OpenPicker), keeping hub behavior unchanged — verified by the existing identity_hub_switcher kittests. Satisfies TC-NAV-01, TC-NAV-02, TC-NAV-03, TC-NAV-13, TC-NAV-16. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): render global switcher on root screens + shared applier (A3) Add the shell glue in top_panel.rs: apply_global_nav_effect (the shared successor to the hub's apply_breadcrumb_effect — silent app-scoped wallet/identity writes, no forced navigation) and add_top_panel_with_global_nav (one-call render-plus-apply), with subdued_everyday_spec / subdued_wallet_only_spec Phase-A rollout helpers. Wire the switcher onto four non-Hub root screens with Subdued (unwired) specs + TODO markers: Identities, DPNS, DashPay (everyday: wallet + identity pills) and Wallets (wallet-only composition, TC-NAV-15). The Hub keeps its existing interactive pills via the breadcrumb shim (regression — full kittest suite green). Document (comment + test, PROJ-010) that set_selected_hd_wallet reconciles the app-global identity as a side effect on non-Hub pages, and that combined with B1's resolution-layer filter it must never reconcile onto an MN/Evonode. Deferred (documented): tokens/tools screens carry in-header sub-navigation that needs the FR-GLOBAL-NAV-5 content-panel back-row migration before their plain breadcrumb can be swapped for the global switcher — a follow-up, not a mechanical swap. Satisfies TC-NAV-06, TC-NAV-15, TC-NAV-16; enables TC-NAV-12/17. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): load-time key encryption plumbing (B0) FR-8: add IdentityInputToLoad.encryption_password: Option<Secret>. When Some, load_identity validates the password up front (fast fail) then, after insert migrates the keyless keys into the vault, seals them Tier-2 through the existing per-identity protect envelope (protect_identity_keys → put_secret_protected via the secret_seam chokepoint) — no new crypto, no second persistence path. When None, the keyless Tier-1 path is unchanged. Relocate validate_protection_password from protect_identity_keys.rs into model/identity_key_protection.rs (PROJ-006, DET validation-placement rule); the seal path and load path both call the model validator. MCP masternode_identity_load passes encryption_password: None (PROJ-007 — GUI-only this iteration, requirements §2.3) with a TODO for headless password parity. Add typed TaskError variants DuplicateProTxHash { identity_id } and MalformedProTxHash { input } for later duplicate/malformed rejection (B1/B4), avoiding string parsing. Tests: model validator (relocated); an offline-wired-AppContext test proving a load-time password seals a masternode's voting (V-target), owner and identity (M-target) keys Tier-2 and round-trips under the password — the exact call load_identity makes (TC-FR8-01/02/10). Load-form UI is B4; end-to-end load routing is covered by the network backend-e2e suite. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * chore(masternodes): drop ephemeral review ID from A3 reconciliation comment Self-review: replace a transient review-finding ID in the apply_global_nav_effect reconciliation note with the durable FR reference. Comment-only; no behavior change. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): FR-6 resolution-layer boundary + masternode accessor (B1) FR-6 (R1, release-blocking): keep masternode/evonode identities out of every everyday-user surface by filtering at the resolution layer, not the display call sites. - resolve_selected_identity(): candidate set filtered to IdentityType::User before resolving, so neither keep-if-loaded nor the first-loaded fallback can ever resolve a masternode — even when a masternode is the only/first loaded identity (TC-NAV-12b). - set_selected_hd_wallet(): the wallet-switch identity reconciliation resolves only over the wallet's User identities. - restore_selected_identity_from_kv(): one-time sanitization — a masternode persisted as selected_identity_id in a prior session is cleared on load; a User selection is kept (TC-NAV-12c). In-memory only (non-destructive). - Display sources switched to the established User-only accessor load_local_user_identities(): the global switcher's identity pill + dropdown and the Identity Hub landing/picker now list User identities only, so the wallet-less "no wallet on this device" group can no longer surface an MN/Evonode (TC-NAV-17). Masternodes stay in the legacy Identities table (unfiltered accessor untouched — locked decision #2). New context accessor load_local_masternode_identities() (hydrated MN/Evonode) — the Masternodes-page card list + page-scoped pill source (B3/B7). Tests: offline-wired-AppContext unit tests (accessors, resolution filter incl. lone-masternode fallback, stale-MN sanitization) + a Hub kittest asserting a seeded Masternode+Evonode never appear on the hub while remaining in the masternode accessor. TC-FR6-01…06, TC-NAV-12b/12c, TC-NAV-17. Deferred to consuming tasks (documented): FR-7 refresh (TC-FR7-02/03) composes existing RefreshIdentity + contested-names refresh at the card Refresh button (B3); the per-node open-contest card read accessor lands in B3 where the card consumes it and it is testable against the rendered status line. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): register Expert-gated Masternodes root tab (B2) Add RootScreenType::RootScreenMasternodes (stable int 28, round-trip test), ScreenType::Masternodes, Screen::MasternodesScreen, create_screen and all ScreenLike dispatch arms; register the always-present root screen in app.rs (gated at runtime by Expert Mode, not a Cargo feature, so the screen exists to switch into when the gate is on). Nav: a "Masternodes" left-panel entry gated FeatureGate::DeveloperMode, positioned directly below the identity cluster (locked decision #3), independent of the identity-hub feature. Distinct glyph voting.png (TODO: dedicated node/server icon). The existing per-entry gate skip hides the nav item and route when Expert Mode is off. Live de-gating (§10.11): active_root_screen_mut falls the active tab back to Identities (always registered) if Expert Mode flips off while Masternodes is selected, so the gated screen is never shown without its gate. MasternodesScreen is a scaffold (global-nav header + left rail + island placeholder); the empty state + card grid land in B3, the page-scoped masternode pill in B7. Network-switch already calls change_context on main_screens; the sub-screen reset (§10.10) applies once B4/B5 push sub-screens (noted for B8). Tests: RootScreenType round-trip (int 28 stable); kittest — nav absent Expert-off / present Expert-on, and de-gating falls back to Identities. TC-FR1-01…07, TC-EDGE-05/06. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): empty state + card grid + card body (B3) Render the Masternodes root screen content on top of the B2 scaffold: - Empty state (FR-2): canonical §7 copy — heading, body, "Load a masternode" primary CTA, and the ProTxHash reassurance line. - Card grid (FR-3): responsive minmax(260,1fr) grid reusing the identity picker's visual language via a new `MasternodeCard` (monogram + `draw_type_badge`, both now `pub(crate)`). Body adds masternode rows the picker lacks: voter readiness, compact `V O P` key status (glyph, not colour-only — NFR-6), DPNS status line, and the IdentityStatus dot+label. - DPNS status precedence (§10.1): open-contest count first, then a pending scheduled vote, then "No open contests", via a display-layer `AppContext::masternode_contest_summary` read (no new backend concept). - Key presence: `QualifiedIdentity::masternode_key_presence` maps Voting/Owner/Payout to voter-identity / OWNER / TRANSFER keys. - Top-right Refresh toolbar button (FR-7) reloads the cached node list. - Whole card is a single accessible click target (`WidgetInfo::labeled`, NFR-6); selection/load intents are captured for B4/B5a/B7 wiring. Traceability: TC-FR2-01…07, TC-FR3-01…15, TC-FR7-01, TC-NFR6-01/03. Unit tests cover heading/sub-line, DPNS precedence, key tokens, badge, and the 8 V/O/P combinations; kittest covers empty-state copy and the grid. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): dedicated load form + ProTxHash validator (B4) Add the MN/Evonode-only load flow (FR-4), carved out of the generic add-existing-identity path: - `ui/masternodes/load_form.rs`: ProTxHash (required), Masternode/Evonode segmented toggle (default Masternode, no User option), optional alias, V/O/P key inputs (reused hold-to-reveal `PasswordInput`), optional at-load encryption password (drives B0's seal), always-visible Warning-tone key-storage note, and a Load button gated on a non-empty ProTxHash with the §7 disabled tooltip. Switching node type clears all fields (§10.6). No auto-derive affordance — masternode keys are never wallet-derived (US-6 retired, §Locked-#4). - `model/masternode_input.rs`: `is_valid_pro_tx_hash` shape validator (hex or Base58) for inline on-blur validation; the backend load task remains the authoritative existence/duplicate check. - Masternodes screen gains a List/Load view enum; the empty-state CTA and a `+ Load` toolbar button open the form; submit dispatches `IdentityTask::LoadIdentity`; cancel/submit return to the list with a fresh form on reopen. - `add_existing_identity_screen`: remove Masternode/Evonode from the Advanced-Options Identity-Type dropdown (User-only remains) — no competing entry point (§10.2 / TC-FR4-22, FR-6). Traceability: TC-FR4-01…22 (render/logic; live-network accept/duplicate/ error-banner paths land in B8), TC-EDGE-01/02. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): detail view — header, actions, keys, remove (B5a) Add the node detail view (FR-5), reusing existing screens rather than reimplementing: - Fixed section order Header → Actions → Keys → DPNS → Remove (TC-FR5-01, the human-requested Actions-above-Keys correction), pinned by a unit test. - Header: conditional alias, shortened ProTxHash + copy-full-value, type badge (shared `draw_type_badge`), IdentityStatus dot + label. - Actions row (FR-9): Withdraw / Top up / Transfer push the existing WithdrawalScreen / TopUpIdentity / TransferScreen scoped to the node's QualifiedIdentity (both node types). Evonode-only `Claim token rewards ›` cross-link (FR-11), absent for a plain masternode. - Keys section (FR-10): V/O/P presence, copyable voter-identity id, at-rest protection tier (vault-scheme probe), Add-protection offered only Tier-1, `Manage keys ›` into the existing key screen. - DPNS section: collapsible, open-contest count in the header (voting table lands in B5b). - Remove: danger ConfirmationDialog; deletes the node and its voter identity. - `‹ All masternodes` back row + detail Refresh (FR-7/TC-FR7-04); card click opens the detail via a List/Load/Detail view enum. Deviations (documented): the Evonode claim cross-link routes to the Tokens area — precise ClaimTokensScreen token-scoping is deferred to B8 where the evonode reward-token context is resolvable. Add-protection routes into the reused key screen (which hosts the password-entry seal flow) rather than duplicating the form. Traceability: TC-FR5-01…05/07, TC-FR7-04, TC-FR9-01/02, TC-FR11-01/02. Live-network credit/claim routing and TC-FR8-07 land in B8. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): Testnet Fill-Random on the load form (B6) Add the FR-12 dev convenience to the masternode load form: - New `testnet_fixture` module owns the `.testnet_nodes.yml` structs + loader. The loader returns None for BOTH a missing and a malformed file — a malformed file is logged at debug and treated as absent (TC-FR12-04, a deliberate divergence from the legacy screen which banners the parse error). - Fill-Random button + hint render only when Expert Mode is on, the network is Testnet, and the fixture is present — never shown-but-disabled (TC-FR12-01…06). The `dev_mode` gate is a defense-in-depth re-check at the call site (TC-FR12-09 decision: added on future-proofing grounds — a plaintext-key dev tool stays inside the Expert-Mode envelope). - Button label follows the node-type toggle (TC-FR12-01/02). - Autofill pulls from the correct list per type (TC-FR12-07/08): Masternode → `masternodes` (Voting + Owner only; the fixture has no payout key, PROJ-003), Evonode → `hp_masternodes` (all three keys). The node-type toggle still clears autofilled fields (§10.6). - The fixture loads once when the form opens (Testnet only), not per frame. Traceability: TC-FR12-01…09. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): inline DPNS voting + missing-voter prompt (B5b) Populate the detail view's collapsible DPNS section (FR-5): - Collapsed by default; header shows the open-contest count (`DPNS name contests to vote on (N)`, TC-DPNS-01/02). - Voter present + open contests: per-contest Abstain / Lock / Vote-for-candidate choices with the candidate list scoped to that contest's contestants; a `Cast votes` button dispatches the existing `ContestedResourceTask::VoteOnDPNSNames` backend inline (locked decision #1 — not a deep-link). TC-DPNS-03/04/05. - Voter present, zero open contests: exact §7 empty copy (TC-DPNS-08). - Missing voter identity: the actionable §7 message (never the raw NoVotingIdentity error) plus an `Add voting key` action that opens a scoped, in-place voter-key prompt with the node context pre-bound — distinct from FR-4's load form, no ProTxHash re-entry (TC-DPNS-09/10/11, §10.8). Save re-loads this node with just the voting key to update its voter identity. - Detail Refresh now re-reads both the contest summary and the open-contest list. Active/open contests only — scheduled/past history stays on the DPNS Scheduled Votes screen (§10.7). Traceability: TC-DPNS-01…11. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * feat(masternodes): page-scoped nav pill — FR-6 boundary in code (B7) Wire the Masternodes page into the global-nav switcher with a page-scoped masternode pill whose selection lives on the page and is NEVER written to `AppContext::selected_identity_id` — the structural FR-6 boundary in code, complementing B1's resolution-layer filter. - New `ui/state/masternodes_view.rs` builds the page's `PageNavSpec`: page-aware `Masternodes` segment-1, an interactive wallet pill (funds Top up — FR-9), and a page-scoped identity pill via `IdentityPillScope::PageScopedObject`. Empty → subdued `(no masternode yet)` placeholder; ≥1 node → interactive dropdown with `Choose a masternode` placeholder; the pill reflects the node in detail and resets to the placeholder on `‹ All masternodes` (§10.4). - New `top_panel::add_top_panel_with_global_nav_capturing` surfaces the `SelectPageObject` pick to the caller (applying all other effects as usual) without ever routing it into the app-global identity selection. - The Masternodes screen builds the spec each frame from its node list + current view and opens the picked node's detail — two-way with the card grid. TC-NAV-12 / TC-FR6-07 (release-blocking): a masternode selected on the page never becomes, or resolves as, the app-global identity — verified across Identities and the Identity Hub with no User identity loaded. Traceability: TC-NAV-01/03/04/05/07, TC-NAV-12, TC-FR5-06, TC-FR6-07. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(masternodes): cross-cutting integration coverage (B8) Add the Remove-flow integration kittest (TC-US4-01/02/06/07): the detail danger button opens a confirmation carrying the `Remove masternode` verb, and confirming deletes only the target node — its card disappears while other nodes survive (isolation). Also sets the confirmation's confirm verb to `Remove masternode` (§7 / TC-US4-02), the one small production touch the test surfaced. Deferred to the network/backend-e2e pass (out of kittest reach without live DAPI or heavy vault fixtures, and flagged as such in the plan): TC-FR8-07 (detail reflecting a load-time Tier-2-sealed node — needs the real password-at-load seal path), TC-US4-05 voter-row deletion assertion (needs a seeded voter-identity row), and the live-network credit/vote/claim dispatch paths behind FR-9/FR-11/DPNS Cast-votes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(user-stories): catalog the Masternodes tab, retire the legacy load story Adds MN-001..MN-009 (load, card grid, vote, remove, Hub filter, at-load encryption, credit actions, key management, evonode token-reward cross-link) and UX-003 (global wallet/identity switcher) per the completed Masternodes feature. Flips IDN-003 to superseded — its generic-screen masternode load path was removed when the dedicated tab shipped. * docs(masternodes): commit final design docs (DOC-002) Lands the human-accepted requirements, UX spec, test-case spec, and dev plan under docs/ai-design/, following the existing 2026-04-23-identity-hub-impl numbered-file convention. Resolves the ~100+ FR-/TC-/US-/§-ID references already scattered through the feature's code comments and tests, which pointed at an uncommitted /data/artifacts scratch copy. Internal cross-file references (requirements.md, ux-spec.md, etc.) are updated to the new numbered filenames. * docs(masternodes): trim oversized module docs, catalog global-nav switcher Shortens the four ui/masternodes/*.rs module doc comments to the internal-tier length cap (DOC-003) — they weren't published API, so the 5-10 line public-rustdoc relaxation didn't apply. Adds GlobalNavSwitcher and its top_panel entry point to ui/components/README.md's catalog (DOC-004), so the next screen needing a page-aware switcher finds it instead of reimplementing one. * fix(masternodes): guard identity load against silent overwrite (QA-005/006) Root-cause storage fix. insert_local_qualified_identity is INSERT OR REPLACE, so a load with no guard silently clobbers an already-stored identity and its keys. Thread an IdentityLoadMode through IdentityInputToLoad so each entry point declares intent: - RejectIfExists: the masternode load form rejects a duplicate ProTxHash with TaskError::DuplicateProTxHash before any network fetch (QA-006). - MergeIntoExisting: the scoped Add-voting-key prompt merges the new key into the stored identity, preserving Owner/Payout it did not resupply (QA-005), via merge_existing_keys_into. - Overwrite: legacy User re-load and headless flows unchanged (default). Adds get_local_qualified_identity accessor backing the existence check and merge read. Failing-first TDD: a unit test proving Owner/Payout keys survive a voting-key-only merge, and an offline test proving a duplicate ProTxHash is rejected and the first node is left untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): key routing, network-switch reset, live refresh QA-007: the detail Keys section pushed the static read-only KeysScreen. Render a per-key 'Manage keys' list and route the Add-protection CTA to KeyInfoScreen (interactive view/sign/seal per key), mirroring identities_screen. QA-001: MasternodesScreen had no change_context override, so a network switch left an open load form or cross-network detail view actionable. Add an explicit change_context arm that resets to the List view and reloads from the now-active network. QA-003: both Refresh buttons only re-read the local cache. Wire them to dispatch IdentityTask::RefreshIdentity (per loaded node on the list, the open node on detail) plus a QueryDPNSContests re-query, alongside the optimistic local re-read. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): SHOULD-FIX pass + QA-token cleanup + offline tests - QA-002: remove the legacy Fill-Random HPMN/Masternode bypass from the now User-only add-existing-identity screen (it set identity_type to Evonode/Masternode directly, defeating the User-only restriction). - QA-004: the evonode 'Claim token rewards' CTA pushes a real scoped ClaimTokensScreen when the node holds exactly one token, falling back to My Tokens when the target is ambiguous — no more bare SetMainScreen. - QA-008: refresh the open detail view after its own backend task, not just the card list. - Diziet-F3: render the missing-voter 'Add voting key' CTA above, outside the collapsed DPNS section, so it is visible without expanding. - QA-009: surface a MessageBanner when node removal fails instead of a silent tracing::warn. - SEC-001: log the testnet-fixture parse error by position only, never its Display text (which echoes a private key). - SEC-002: parse fixture key fields as Secret (redacted/zeroized). - Strip stale PROJ-003/PROJ-004 markers and all ephemeral QA-xxx review IDs from source comments (kept only in commit messages). - TODOs for the deferred mixed-protection-tier CTA and the is_valid_pro_tx_hash/decode_identity_id duplication. - Offline tests: TC-FR8-07 (protection_tier reflects a Tier-2 seal) and TC-US4-05 (Remove deletes the associated voter identity). Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(dashpay): close FR-6 boundary bypass via DashPay identity selectors (SEC-005) DashPay screens (send_payment, contacts_list, profile_screen, contact_requests, qr_scanner, qr_code_generator, add_contact_screen, profile_search) built their IdentitySelector and constructor seed from the unfiltered load_local_qualified_identities() chained with .syncing_global(...). IdentitySelector::sync_to_global() writes the picked id straight to AppContext::selected_identity_id — a separate path from B1's resolve_selected_identity()/restore filters — so a user could select a masternode/evonode as the app-global operate-as identity from inside DashPay, bypassing the FR-6/R1 boundary B1 established. DashPay operates on User identities only, so every identity list in these screens is sourced from load_local_user_identities() (the same swap B1 made for the global-nav switcher and Identity Hub). This filters the masternode out of both the selector write-path and the constructor seed. Adds a kittest (masternode_never_selectable_in_dashpay_screens) exercising the FR-6 boundary through five DashPay screens — the existing FR-6 kittest only covered Identities/Identity Hub, which is how this slipped through. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(masternodes): Marvin punch-list — in-flight guard + execution tests - QA-012: gate re-submission while a node-load is in flight. Add a load_in_flight flag on MasternodesScreen, set on Submit dispatch and cleared on the task result or a new display_task_error override; the '+ Load' toolbar button and empty-state CTA show a spinner + disabled 'Loading…' while set, so a rapid double-submit of a brand-new ProTxHash cannot race two loads past the pre-fetch existence check. - Extend masternode_never_selectable_in_dashpay_screens to QRScanner, QRCodeGenerator (both seed selected_identity in new()) and assert ProfileSearchScreen's User-filtered data source excludes the masternode — FR-6 coverage now spans all 8 DashPay screens. - Add manage_keys_button_opens_key_info_screen: clicks a per-key 'Voting key ›' button and asserts a KeyInfoScreen is pushed with its 'Key Information' heading (execution-level proof of the QA-007 fix). - Add refresh_from_network unit test: one RefreshIdentity per loaded node plus a trailing QueryDPNSContests, None when empty (QA-003). - Fix two doc-comment lines mangled by the earlier review-ID strip. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(changelog): add Masternodes tab and global nav switcher (DOC-005) Covers the user-facing outcomes of the completed Masternodes feature: the new Expert-Mode-gated tab (card list, detail view, load-time key encryption, inline DPNS voting, credit actions, Evonode token-reward claiming) replacing the old generic load path for masternode/evonode identities, the resulting Identity Hub / Identities picker filter, and the wallet/identity switcher now present on every root screen instead of just the Identity Hub. * fix(masternodes): default GUI build broken — masternode_input feature-gated The whole model::masternode_input module was gated behind load form (ui/masternodes/load_form.rs) imports is_valid_pro_tx_hash from it unconditionally. So a plain 'cargo build --bin dash-evo-tool' (default features only — no mcp/cli, the documented quick-start build) failed with E0432 unresolved import. Every gate this feature ran used --all-features, which always pulls mcp+cli and masked it. The module can't be blanket-ungated: its parse/decode helpers return McpToolError (from the feature-gated mcp module). Fix ungates the module and the pure is_valid_pro_tx_hash validator (the only thing the GUI needs), and gates precisely the McpToolError-coupled items — KeyMode, parse_node_type, parse_key_mode, require_at_least_one_signing_key, decode_identity_id, their imports, and their tests — behind mcp/cli. The pure validator's tests move to an always-compiled module so they run in the default build too. Verified: 'cargo build --bin dash-evo-tool' (no flags) compiles; default- feature clippy clean; both default and --all-features test paths pass. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(masternodes): correct global-nav coverage claim (F-003) CHANGELOG and the components README claimed the global wallet/identity switcher was on "every screen". It ships Phase-A: rendered on Identities, DashPay, DPNS, Wallets, Identity Hub, and Masternodes, interactive only on the Hub and Masternodes (the other four render subdued, read-only pills), and absent from every other root screen (Contracts, Tokens, Tools, Network Chooser, Withdraws, ...). Names the actual screens and notes the rest as a tracked follow-up instead of implying full rollout. * fix(masternodes): Fable final round — Tier-2 merge seal, load gate, ProTxHash error F-001 (MUST-FIX): "Add voting key" on a password-protected (Tier-2) node no longer trips the insert's fail-closed guard. load_identity now verifies the node's object password UP FRONT (before the network fetch, mirroring add_key_to_identity's verify-before-broadcast order) and seals the merged plaintext keys Tier-2 via seal_merged_plaintext_keys just before the at-rest insert. Two regression tests: a scripted-prompt success path proving the new key flips InVault and reads back Protected, and a headless NullSecretPrompt path proving the merge fails closed with SecretPromptUnavailable before fetch. F-002: the list screen's load_in_flight gate is cleared only on the load's own LoadedIdentity result variant (not any routed result), with a refresh_on_arrival backstop so a tab switch mid-load can never strand "+ Load" at "Loading…". F-005: a malformed identity-id input now surfaces MalformedProTxHash for masternode/evonode loads (where the field IS a ProTxHash) and keeps IdentifierParsingError for User loads. Regression test added. F-006: masternodes/evonodes legitimately have no HD wallet, so the "saving identity without wallet" warning is gated to User identities; nodes log at debug instead. F-004: correct the MCP masternode_identity_load comment — Overwrite is a destructive full-replace of stored keys, not a merge/refresh; TODO for a future load-mode param. F-007: strip ephemeral review-ID prefixes (SEC-*, Diziet-*, Smythe) from masternode-scope source comments. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): add load-form back link + remove object pill from breadcrumb Live-walkthrough fixes on real testnet data. Fix 1 — load form back link: the load form now renders the same `‹ All masternodes` back link as the detail view (wireframe C shows it on both), at the top of the form, returning to the card list. New kittest `load_form_back_link_returns_to_list` covers it; the existing `load_form_opens_from_cta_and_cancels` gets a taller headless window so the bottom Cancel button stays reachable now that the back row is present. Fix 2 — remove the masternode object/identity pill from the Masternodes breadcrumb. Masternode/evonode identities are never wallet-linked (wallet_info is always None — locked decision #4), so pairing a wallet pill with an object pill implied a wallet↔masternode relationship that does not exist. The breadcrumb now carries only segment-1 + the interactive wallet pill; node selection is driven entirely by card-click → detail and the back link. The Masternodes page switches to add_top_panel_with_global_nav (non-capturing), matching every other non-object page. The masternodes_page_nav_spec builder drops its items/selected params. This does NOT touch the FR-6 boundary, which is enforced structurally at the resolution layer (B1) independent of any pill. The release-blocking FR-6 boundary tests (TC-NAV-12 / TC-FR6-07) remain unchanged and green. The PageScopedObject / SelectPageObject / add_top_panel_with_global_nav_capturing machinery is retained as the documented, tested boundary pattern for future page-scoped-object features (the global_nav_switcher tests still exercise it); only the Masternodes page's use of it is removed. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): reject load when selected node type mismatches on-chain Loading a masternode/evonode by ProTxHash trusted the load-form Masternode/ Evonode toggle as ground truth with no cross-check. A regular masternode loaded with "Evonode" selected was silently accepted, mis-badged "Evonode", and shown the Evonode-only "Claim token rewards" action. The load task (authoritative layer) now cross-checks the selected type against the node's actual on-chain registration. A masternode's Platform identity id is its ProTxHash, so the node is looked up via Core RPC `protx info`; the `type` field ("Regular"/"Evo") is classified and compared. On a confirmed mismatch the load is rejected with a typed, actionable `TaskError::NodeTypeMismatch` naming both the selected and actual types. When the on-chain type cannot be determined (Core RPC unreachable — e.g. an SPV-only setup with no Core node) the load proceeds unverified, so this adds no regression for those users. Layering per CLAUDE.md: the pure classification (`classify_protx_node_type`) and rejection decision (`node_type_conflict`) live in `model/masternode_input` and are exhaustively unit-tested (the reported Evonode-on-regular case included); the backend task owns the network lookup and enforcement. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): surface a visible warning when node type is unverified Follow-up to the node-type cross-check: when Core RPC is unreachable (the common case for SPV-only users) the node type cannot be verified, and silently proceeding with an unverified badge reproduced the original UX bug downgraded from "wrong" to "unverified". The load task now distinguishes the two success outcomes via a new `BackendTaskSuccessResult::LoadedIdentityTypeUnverified` variant, and the Masternodes screen surfaces a visible warning banner (not just a log line) telling the user the badge reflects their selection and to reload later to confirm. The MCP masternode-load tool reports the same distinction via a new `node_type_verified` output field. Regression tests: the pure reject decision (`node_type_conflict`) and the `NodeTypeMismatch` user message cover the hard-reject path; a kittest drives the unverified-load result into the live screen and asserts the warning banner is surfaced to the UI, not merely logged. Confirmed with team-lead: hard-reject on a confirmed mismatch; a visible (not log-only) warning on the unverified path. The upstream platform-wallet SPV masternode-list passthrough (for verifying node type without Core RPC) is tracked as a separate follow-up against the platform repo. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * revert(masternodes): drop Fix #3 node-type validation entirely Reverts c5167787 and 755eee87. Product decision: trust the user's Masternode/Evonode toggle as-is, with no on-chain node-type verification. Rationale: the only non-type-gated actions (Withdraw/Top up/Transfer) are unaffected by a mis-toggle; the sole type-gated action ("Claim token rewards") degrades to a clean no-op/failed Platform state transition, not a fund-safety issue — so the toggle working as the user set it is correct behavior, not a defect. Dropping verification also removes the dependency on Core RPC (being deleted in the platform-wallet migration) and on fetching the operator identity (extra scope), leaving the load path simpler and migration-proof. Removes: TaskError::NodeTypeMismatch, the onchain_node_type Core RPC cross-check, the classify_protx_node_type/node_type_conflict model helpers, the LoadedIdentityTypeUnverified result variant + UI warning banner, the MCP node_type_verified output field, and all associated tests. Fixes #1 (load-form back link) and #2 (breadcrumb pill removal) in 5db23f72 are untouched. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): adapt retargeted tab to platform-wallet rewrite APIs Rebasing the Masternodes tab onto the platform-wallet backend rewrite (PR #860) surfaced three call sites where the rewrite reshaped an API the masternode-tab code depended on: - `AppContext::identity_kv()` was renamed to `det_kv()`; the load-path existence check (`get_local_qualified_identity`) now calls the new name. - `ContestState::state_is_votable()` was dead-code-removed by the rewrite, but `ContestedName::is_open_for_voter` (Masternodes card DPNS status) relies on it — restored as a live, un-gated method. - The rewrite dropped the `identity-hub` Cargo feature and renders the Identity Hub nav entry unconditionally; the left-panel builder no longer gates that entry behind the removed `#[cfg(feature = "identity-hub")]`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * test(withdraw): screen-level kittest coverage for default_withdrawal_key fix WithdrawalScreen::new() pre-selecting via default_withdrawal_key() was only unit-tested at the QualifiedIdentity model layer. Add tests/kittest/withdraw_screen.rs to verify the fix at the actual screen layer: ghost-key identities (on-chain-only TRANSFER key) render the no-keys empty state instead of a form, private-key-backed TRANSFER/OWNER keys are pre-selected AND rendered correctly in the key-selection ComboBox (via accesskit value, not label). Also locks in a genuine regression the fix newly exposes: WithdrawalScreen::new() leaks a raw "No key provided when getting selected wallet" error banner for any identity with no locally-signable withdrawal key. * fix(withdraw): skip wallet resolution when no signable key exists WithdrawalScreen::new() called get_selected_wallet with selected_key=None after default_withdrawal_key() correctly began returning None for identities with no locally-signable withdrawal key. With app_context=None that hits the "No key provided" String Err path, which .or_show_error() posted verbatim into a user-facing MessageBanner — violating the plain-language error policy. Guard the call on selected_key being Some, so the raw-string Err branch is structurally unreachable here instead of avoided by luck. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * docs(masternodes): add user story for network-switch reset behavior Live QA of the Masternodes tab confirmed MasternodesScreen::reset_for_network_change() cleanly resets the List view and clears stale banners/form data on a network switch, with no existing story documenting this PR's fix. Add MN-010. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * test(withdraw): flip banner-leak test to a regression lock (2edbc18) WithdrawalScreen::new() now guards get_selected_wallet on selected_key being Some (2edbc18), so the raw "No key provided..." banner leak for ghost-key-only identities is fixed. Rename ghost_key_construction_leaks_raw_error_banner -> ghost_key_construction_does_not_leak_raw_error_banner and invert the assertion; also verify the no-keys empty state still renders correctly for the same identity, so the fix didn't trade the leak for a broken empty state. * fix(mcp): stop det-cli double-prefixing the HTTP bearer token rmcp 1.7's StreamableHttpClientTransportConfig::auth_header takes the raw token and lets reqwest's bearer_auth prepend "Bearer " itself. The det-cli client passed format!("Bearer {token}"), so the wire header became "Authorization: Bearer Bearer <token>". The server middleware strips one "Bearer " and compares the remaining "Bearer <token>" against the configured key — never equal — so headless HTTP mode returned 401 on every request. The auth path had no end-to-end coverage, which is why it shipped broken. Pass the raw token and add tests/mcp_http_auth.rs pinning the server's wire contract: raw token accepted, a double-"Bearer" prefix rejected, missing credentials rejected. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> * fix(masternodes): share Expert Mode flag app-wide; node-specific load error Two live-QA bugs on the Masternodes tab (PR #876). Bug 1 — Expert Mode didn't reveal the Masternodes nav without a restart. `developer_mode` was an independent per-network `AppContext` AtomicBool, kept in sync only by a best-effort loop in the Settings checkbox handler over the contexts that happened to exist at click time. AppState keeps one context per network (only the active one at startup; others created lazily on switch), so the left-nav `FeatureGate::DeveloperMode` gate could read a stale value on a different context than the toggle mutated — the nav entry stayed hidden until a restart re-read the persisted flag into the single fresh context. Promote the flag to one shared `Arc<AtomicBool>`, created once in `AppState::new` and injected into every `AppContext::new` (startup + `SwitchNetwork` via `developer_mode_handle()`), so all per-network contexts observe one flag. Drop the fragile sync loop; request a repaint on toggle since enabling Expert Mode disables animations (which stops continuous repaints). Bug 2 — loading a masternode by a valid-but-unregistered ProTxHash showed the generic "Identity not found — check the ID or name" copy, wrong for a ProTxHash load form. Add a dedicated `TaskError::MasternodeNotFound` variant and return it from the masternode/evonode load path instead of `IdentityNotFound`. Regression tests: `developer_mode_is_shared_across_network_contexts` (RED before the shared-flag fix), `masternode_not_found_message_is_node_specific`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> --------- Co-authored-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
| ); | ||
| } | ||
| } | ||
| let _resp = resp.info_tooltip(tip); |
There was a problem hiding this comment.
do we need this line when _resp is not used ?
| //! cross into egui. The lone secret type on the wire is `SecretString`, the | ||
| //! `platform_wallet_storage` zeroizing wrapper the project already depends on. | ||
|
|
||
| use async_trait::async_trait; |
There was a problem hiding this comment.
are you sure it belongs to src/ui/components,? Components are designed as items that implement the Component trait. Maybe src/ui/tools?
| @@ -0,0 +1,181 @@ | |||
| //! Shared modal chrome for passphrase entry. | |||
There was a problem hiding this comment.
src/ui/components should implement the Component trait. Implement it if possible.
| @@ -0,0 +1,298 @@ | |||
| //! Per-screen cache of wallets' tracked asset locks. | |||
There was a problem hiding this comment.
are you sure it belongs to src/ui/components? Components are designed as items that implement the Component trait. Maybe src/ui/tools?
| **Persona:** Priya, Jordan | ||
|
|
||
| As a user with an imported private key, I want to send Dash from that single-key wallet so that I can move funds to another address. | ||
|
|
||
| - Send flow works the same as for HD wallets. | ||
| - Temporarily unavailable in this version (Decision #7): single-key send returns a clear, calm "not supported in this version — your data is preserved; use an HD recovery-phrase wallet" message. Single-key wallet data and its UTXOs are retained on disk and load correctly; only the spend action is gated. Re-enabled when single-key moves onto the upstream wallet runtime. |
There was a problem hiding this comment.
We need it implemented.
| @@ -441,6 +488,19 @@ As a user, I want to view all keys associated with my identity so that I can aud | |||
| - Lists all keys with type, purpose, and status. | |||
| - View individual key details. | |||
|
|
|||
| ### IDN-013: Password-protect an identity's signing keys (SEC-001) [Implemented] | |||
There was a problem hiding this comment.
this only applies to non-HD-wallet backed identities. HD-wallet backend identities have per-wallet (seed) password protection capabilities, not identity level.
| @@ -481,6 +541,26 @@ As a user, I want to top up identity credits from a Platform address so that I c | |||
| - Available as funding method in top-up screen. | |||
| - Uses Platform address credits directly. | |||
|
|
|||
| ### IDN-014: Fund identity directly from scanned external payment [Removed — upstream-only funding] | |||
There was a problem hiding this comment.
We need to restore this feature
|
✅ Review complete (commit ee1ce81) |
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Source: reviewers: claude-general (opus), codex-general (gpt-5.6-sol, failed rc=1); verifier: opus; specialists: none
Verified both agent findings against the worktree at 4343d03. The PR is a large, well-engineered wallet-backend rewrite; no blocking correctness or security defects were confirmed. Both surviving findings are non-blocking test-coverage observations: a sum-based header-vs-tab reconciliation whose invariant test cannot exercise the immature/locked-funds divergence case (verified at snapshot.rs:506/510-511/1217), and a self-referential wire-layout test that round-trips the current struct rather than pinning a golden pre-change blob (verified at settings.rs:516-546). No prior-finding reconciliation context or CodeRabbit inline findings were supplied, so nothing is carried forward beyond the two current-delta findings.
🟡 1 suggestion(s) | 💬 1 nitpick(s)
2 finding(s) are included in the review details below; GitHub refused the PR diff API because this PR exceeds 300 changed files.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/ui/wallets/wallets_screen/mod.rs`:
- [SUGGESTION] src/ui/wallets/wallets_screen/mod.rs:1225-1231: Header-vs-tab reconciliation is sum-based; untested when balance.total > sum(address_balances)
The PR claims "visible tabs always sum to the header total, in every mode." But the header total is `snapshot.balance.total` = upstream `core_balance.total()` (src/wallet_backend/snapshot.rs:506), which the PR's own doc (snapshot.rs:58-60) states "counts immature coinbase and locked (CoinJoin) funds" that coin selection cannot touch. The per-account tabs derive entirely from `address_balances`, summed only from `state.utxos()` (snapshot.rs:510-511), and the reconciling System/Other tab computes its value by summing categorized system summaries via `hidden_category_balance` (mod.rs:1225-1231) rather than as a residual `total - visible`. So header == Σtabs holds only if `Σ state.utxos() == core_balance.total()`. If upstream `utxos()` ever omits immature coinbase / locked CoinJoin outputs that `total()` still counts, the visible tabs sum to less than the header — reintroducing the divergence class this PR set out to close. The invariant test `header_total_reconciles_with_core_tab_breakdown_through_real_accessors` (snapshot.rs:1198) fixes `total = address_balances.values().sum()` (snapshot.rs:1217), so it structurally cannot catch this. Display-only (no funds at risk) and the immature/locked state is rare on user wallets, hence suggestion severity. A regression test that publishes a snapshot with `balance.total > Σaddress_balances` and asserts the reconciling tab still closes the gap would either confirm the claim or expose the hole.
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Code Review
Source: reviewers: opus (ok), gpt-5.6-sol (failed: revoked refresh token); verifier: opus; specialists: none
The 4343d03..ee1ce81 delta is a single .gitignore line (ignore local Codex config); no source code changed. Both prior findings reference code that is byte-for-byte identical at this head and remain STILL VALID non-blocking test-coverage observations. No new latest-delta findings. Carried-forward: prior-1 (header-vs-tab reconciliation untested when balance.total > sum(address_balances)) and prior-2 (self-referential reserved-byte wire-layout test).
🟡 1 suggestion(s) | 💬 1 nitpick(s)
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/ui/wallets/wallets_screen/mod.rs`:
- [SUGGESTION] src/ui/wallets/wallets_screen/mod.rs:1225-1231: Header-vs-tab reconciliation is sum-based; untested when balance.total > sum(address_balances)
Carried forward from the prior review at 4343d034 (STILL VALID — code unchanged in this delta, which touches only .gitignore). The header total is `snapshot.balance.total` = upstream `core_balance.total()` (src/wallet_backend/snapshot.rs:502-506), which counts immature coinbase and locked (CoinJoin) funds that coin selection cannot touch. The per-account tabs derive entirely from `address_balances`, summed only from `state.utxos()` (snapshot.rs:508-518), and the reconciling System/Other tab computes its value by summing categorized system summaries via `hidden_category_balance` (mod.rs:1225-1231) rather than as a residual `total - visible`. So header == Σtabs holds only if `Σ state.utxos() == core_balance.total()`. If upstream `utxos()` ever omits immature coinbase / locked CoinJoin outputs that `total()` still counts, the visible tabs sum to less than the header, reintroducing the divergence this PR set out to close. The invariant test fixes `total = address_balances.values().sum()`, so it structurally cannot catch this. Display-only (no funds at risk) and the immature/locked state is rare, hence suggestion severity. A regression test that publishes a snapshot with `balance.total > Σaddress_balances` and asserts the reconciling tab still closes the gap would either confirm the claim or expose the hole.
3 participants
Goal
Dash Evo Tool carried its own large SPV/wallet stack that duplicated logic now maintained upstream. This PR rewrites DET's wallet backend on top of the upstream
platform-walletcrate (dashpay/platform), which owns SPV chain sync, address derivation, identity/asset-lock handling, and the shielded coordinator.DET is now a thin adapter over that crate: its bespoke SPV stack (
src/spv/) and the legacy RPC wallet mode are gone, chain sync and derivation come from upstream, and the shielded subsystem routes through the upstream coordinator. TheBackendTaskaction/channel contract is preserved, so the UI layer is largely unchanged. Existing wallets are migrated onto the new model on first launch (marker-gated, with a*.premigrationbackup). Net effect: far less wallet code to maintain in DET, a single source of truth shared with the rest of the platform, and upstream fixes inherited automatically.Bugs fixed
Funds-safety
watched_addressescould desync fromplatform_address_info). The wallet now reconciles this automatically via seedless reverse-derivation from the account xpub, so a visible balance can always be withdrawn. No funds were ever at risk: the withdrawal failed safely before broadcast.send_paymentnow builds and signs through upstream's nativeTransactionBuilder(upstream removed the previousCoreWallet::send_to_addressesAPI) and broadcasts viabroadcast_transaction_releasing_reservation, so a rejected broadcast can't leak a UTXO reservation. Error messages are now variant-specific — a "too many inputs" failure no longer tells the user to check their balance.Balance display
75.62vs2.16DASH on Platform). Root cause: DET was discarding derivation-index metadata upstream already provides, and the per-category summary only ever looked at DET's own address list, not the backend's authoritative one.platform_balance_duffs), so it can't lag behind. A stopgap runtime health-check banner that used to detect this class of drift after the fact has been removed — it's structurally unreachable now. Default-mode users also get a reconciling tab for any balance in a normally-hidden category (e.g. CoinJoin, or a funded address the managed-wallet pools don't recognize), so visible tabs always sum to the header total, in every mode. The Platform tab is shown immediately on wallet load (matching pre-migration behavior) rather than waiting on a completed sync pass. This fix went through two independent adversarial QA passes; the first caught that the initial version's "cannot diverge" claim was true for Platform but not yet for Core (a deeper root cause — an upstream balance-update path that could silently drop updates under its own lock contention), the second independently re-verified the fix against the actual upstream source and confirmed it closed.Shielded confirmation-safety
*ConfirmationUnknownerrors instead of a false success.Secret hygiene
Data-safety
Lifecycle / sync
Housekeeping
core_backend_modesettings field + DB column,FeatureGate::RpcBackend) was already fully inert — chain sync has been SPV-only since this migration started — but the dead weight itself was left behind pending a dedicated schema migration. It's gone now: settings field removed (with a reserved wire byte to keep the existing bincode-encodeddet:settings:v1blob layout intact — no user data at risk), DB column dropped via a new backward-compatible v38 migration (existence-guarded, idempotent, no drop-and-resync needed), and the always-false feature gate removed. Verified: RPC config fields, chain-lock RPC error plumbing, andsend_screen's Core/Platform source picker are genuinely still live and were left untouched.debug_assert!invariant guards replaced with real runtime checks. Two sites relied ondebug_assert!, which compiles out in release builds — silently disabling the check for every real user. The SPV phase-count drift guard now logs an error instead of vanishing; the tracked-lock funding empty-recipients precondition is now a typedTaskErrorreturned in every build.RUST_MIN_STACKraised to 8 MiB project-wide via.cargo/config.toml, independent of the backend-e2e suite's own dedicated 32 MiB test runtime.e6b508f1→c2135800(108 upstream commits; API drift notes below), thenc2135800→44c20e3to pick up theTransactionBuilderAPI after upstream removedsend_to_addresses(see Bugs fixed → Funds-safety). First bump's adaptations:ManagedIdentityDashPay state sealed behind adashpay()accessor;ContactXpubDatasplit into{ xpub, compact }with the DIP-15account_referencenow HMAC'd over the 69-byte compact xpub (an upstream fix matching the iOS/Android reference clients — DET's own pinned test vector was re-verified against upstream's own known-answer test and re-pinned to the new interop-correct value);IdentityWallet::dashpay_sync()replaced by explicitsync_contact_requests+sync_profilescalls;PlatformWalletErrorgained 5 variants / dropped 3; a handful of signature changes. Full gate re-verified green after each bump.WalletBackendgod-impl split intoshielded.rs/identity_ops.rs/payments.rssibling modules (mirroring the existinghydration.rs/dashpay.rspattern, zero behavior change, funds-signing path moved verbatim); a 14-copy duplicatedInMemoryKvtest fake collapsed into one sharedkv_test_supportmodule; a dead one-implPersistedWalletLoadertrait seam deleted (the G2 mock-boundary swap it existed for already shipped); several near-twinTaskErrorvariants merged; a stringly-typed warning channel in the wallet-refresh banner replaced with a typed error; stale/self-contradicting migration-module comments (some literally asserting things the code no longer does) rewritten to describe present state only; ~150 ephemeral review-round ID references (SEC-NNN/QA-NNN/etc.) stripped from committed comments per the project's own convention against citing IDs that get reassigned on every consolidation run; two unreferenced UI widgets (ContactRow/ActivityRow, ~750 lines) removed; kittest/e2e test-harness boilerplate deduplicated across 19 files. Full details in the audit's own report artifacts (not part of this PR's diff).Breaking changes
platform-walletpin includes a schema change that rewrites migrationV001in place — it drops thecore_derived_addressesandaccount_address_poolstables and hardcodes core UTXOaccount_index = 0. Existingspv//platform-wallet.sqlitedatabases are not forward-compatible and must be deleted and re-synced.Known gaps
platform-wallet/dash-sdkcrates are pinned to an unreleased branch (dash-evo-tool), not a released tag. Must be re-pinned to a released version before merge-to-ship. (Still open — this PR's two in-branch bumps, most recently to44c20e3, pick up upstream fixes within that same unreleased branch; neither resolves the need for an eventual released-tag pin.)432.41vs7.34DASH). Watch-only rehydration rebuilds accounts from xpubs and derives only the default gap window (indices0..=29) without restoring the persisted derivation depth — a regression of bug: Imported wallet shows zero balance — historical UTXOs invisible even after restart #829 Bug 2. The fix belongs upstream inrs-platform-wallet(extend each account'sAddressPoolto the highest restored index on rehydration); tracked. (Distinct from the header-vs-tab single-sourcing fixed above under Bugs fixed → Balance display — that fix closes a DET-side bookkeeping/consistency gap and does not touch rehydration's gap-window derivation, so this item stays open.)SingleKeyWalletsUnsupported, pending upstream watch-only support. Import / data-loss guard / password-restore are done.docs/ai-design/are historical record, not user-facing docs — left as-is, flagged for a possible future trim. Merge-time action: fill the ADR floor SHA placeholder (PROJ-019).Testing
cargo build/cargo build --features headlessclean;cargo clippy --all-features --all-targets -- -D warningsclean;cargo +nightly fmt --all -- --checkclean.network-info,tools,tool-describe,core-wallets-list) passes.docs/ai-design/.cargo test --all-features --workspace— 1327 lib tests passed (0 failed, 1 ignored), 187 kittest passed, 10 e2e passed, 3 legacy-table-surface passed, all doc-tests passed.cargo clippy --all-features --all-targets -- -D warningsclean.Attribution
🤖 Co-authored by Claudius the Magnificent AI Agent