fix: round-2 full-repo architecture audit (21 waves + shielded-screen consolidation)#873
Merged
lklimek merged 67 commits intoJul 9, 2026
Conversation
show_wallet_balance() on both the Create-Identity and Top-Up "use wallet balance" screens read DetWalletBalance.total, while the insufficient-funds banner, gate, and picker on the same screens all read .spendable(). With immature or CoinJoin-locked funds present, the user saw a positive "Wallet Balance" directly contradicted by a "not enough Dash" banner one line below. Switch both to .spendable() so all four surfaces agree. Raised on PR #869 review (thepastaclaw); verified still live post-merge. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Full-repo audit CODE-005 (untriaged) found that handle_wallet_unlocked's None-passphrase early return skips drive_unlock_registration, so a non-remember unlock never re-registers the wallet with the upstream SPV backend until next launch. User decision 2026-07-08: defer rather than fix now, to keep the adjacent CODE-024 cleanup (Wave 11) unconstrained. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Wave 9 of the wallet-backend architecture audit — caches & nits. - Avatar cache eviction now orders entries by a sibling `det:avatar_ts:` timestamp index (small i64 reads) instead of deserializing every cached image's full bytes on each put. put/invalidate/clear/evict keep the index in lock-step with the byte entries. - Trim test-only / over-public wallet-backend surface: gate `wallet_count` and `AuthPubkeyCacheView::delete` behind `#[cfg(test)]`; make the `AVATAR_TTL_MS`, `MAX_AVATAR_ENTRIES`, and `DASHPAY_REQUEST_EXPIRY_DAYS` constants private; delete the unused `shielded_activity`/`shielded_notes` stubs; correct the stale `ensure_wallets_registered` migration-engine doc. - Name the Devnet/Regtest SPV P2P ports and collapse `spv_primary_peer_socket` into one match with an early `_ => None`. - Unify the two SPV error-snippet truncations under one `SPV_ERROR_SNIPPET_MAX` constant and drop the shadowing `use` lines in `on_platform_address_sync_completed`. - Drop the dead `TokenBalanceSnapshot` re-export. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CODE-013/017/047/049 (round-2 audit): avatar-cache eviction stops deserializing every image's bytes (sibling timestamp index), test-only accessors gated, dead constants privatized, shielded stubs removed, devnet/regtest port match collapsed, dead TokenBalanceSnapshot re-export dropped. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…e helpers
Wave 6 of the architecture audit — context/identity_db.rs plus the shared
kv-access surface it touches.
- CODE-001: replace the `format!("{:?}", identity_type)` Debug-repr
discriminator (four writers) and the hardcoded string filters with a
stable `IdentityType::as_tag()` / `from_tag()` mapping. Writer and filter
now share one source of truth, immune to a variant rename. Adds a
round-trip test.
- CODE-007: extract `hydrate_stored_identity()`, shared by the bulk-load
and single-get paths so both reconstruct an identity identically.
- CODE-014: introduce per-domain `err`-style free fns (`identity_err`,
`scheduled_vote_err`, `top_up_err`, `contract_err`, `token_err`,
`contest_err`) replacing the verbose `.map_err(|source| …)` closures and
the fully-qualified error paths; collapse the three `*_kv()` accessors
into one `AppContext::det_kv()`.
- CODE-035: add `scheduled_vote_keys()` and `remove_vote_voter_from_index()`
helpers; the three hand-rolled list/delete/prune loops shrink to a few
lines.
- CODE-041: delete the dead `load_local_qualified_identities_in_wallets`
and its speculative `#[allow(dead_code)]`.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CODE-001/007/014/035/041 (round-2 audit): stable IdentityType tag replaces Debug-repr discriminator; shared hydrate_stored_identity() for bulk/single load; AppContext::det_kv() collapses three per-domain kv accessors, per-domain error-mapper free fns replace closures; scheduled-vote index helpers extracted; dead load_local_qualified_identities_in_wallets removed. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Wave 20 dead-UI-surface cleanup:
- Delete the unused `left_wallet_panel.rs` (no opener anywhere) and its
module declaration.
- Consolidate the three copied `Assets`/`load_icon`/`load_svg_icon`
RustEmbed icon loaders into one shared `components/icons.rs`; drop the
dead copy in `top_panel.rs` and the duplicate in `left_panel.rs`.
- Remove dead styled widgets `GlassCard`, `HeroSection`, `AnimatedIcon`,
`AnimatedGradientCard` and `styled_text_edit_multiline`, plus their
catalog rows in `components/README.md`.
- Collapse `StyledButton` to its single reachable configuration (drop the
never-constructed `ButtonVariant`/`ButtonSize` enums and arms); remove
the commented-out `StyledCard` builders and dead `title` field/branch;
remove the inert `GradientButton::glow` field/builder and its no-op
call site in `left_panel.rs`.
- Drop the six blanket `#[allow(dead_code)]` blocks in `theme.rs`. This is
a library crate, so `dead_code` never fires on `pub` items; clippy
`--all-features --all-targets -D warnings` is clean without them, so no
per-item `#[expect(dead_code)]` is warranted (it would be unfulfilled).
- Delete five zero-caller `helpers.rs` items (`BUTTON_ADJUSTMENT_PADDING_TOP`,
`render_key_selector`, `is_platform_address`, `PLATFORM_ADDRESS_HINT`,
`PLATFORM_ADDRESS_EXAMPLES`); the `is_platform_address_string` re-export
stays (four live callers).
- Delete `OptionBannerExt::replace` (unused alias + its essay); drop the
duplicate inherent `AmountInputResponse::{is_valid,has_changed}` (the
`ComponentResponse` impl provides them); `add_connection_indicator`
returns `()`.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CODE-119/120/118/114/111/121/122 (round-2 audit): delete left_wallet_panel.rs and dead styled-component widgets (GlassCard, HeroSection, AnimatedIcon, AnimatedGradientCard, styled_text_edit_multiline) plus their README catalog rows; dedup icon loaders into components/icons.rs; drop unreachable ButtonVariant/ButtonSize and inert GradientButton::glow; remove blanket dead_code allows (verified clean without them, this is a lib crate so #[expect(dead_code)] doesn't fire on pub items); delete zero-caller helpers.rs items and OptionBannerExt::replace. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…Wave 2)
Wave 2 of the architecture audit — wallet-backend funds paths and error
integrity. Behavior-preserving throughout; funds/secret-adjacent code kept
conservative.
- CODE-004: extract `WalletBackend::seed_wallet` for the repeated
`from_seed_bytes` construction; replace fabricated
`PlatformWalletError::{WalletCreation,AssetLockTransaction,InvalidIdentityData}`
string wraps with typed `SeedWalletBuildFailed` /
`IdentityFundingAccountProvisionFailed` variants carrying the real
`#[source]` (`key_wallet::Error`).
- CODE-011: replace `unreachable!("handled by pre-flight")` in
`map_shielded_op_error` with a defensive `WalletBackend` fallthrough so an
ambiguous shielded-spend result can never panic a funds path.
- CODE-012: rewrite the DashPay adapter module doc in present tense and fix
the inverted sidecar-scope comment (2 Global / 4 Identity families).
- CODE-015: delete dead `flush_persister` (+ orphaned
`WalletPersistenceFlushFailed`) and `broadcast_transaction`; keep the
documented single-key signing chokepoint `sign_single_key` (intentional,
test-exercised infra awaiting single-key send) but fix its
source-discarding `map_err` via a typed
`SingleKeySignFailed { #[source] DetSignerError }`.
- CODE-025: parse identity funding intent once into a local `Funding` enum —
the repeated `AccountType` matches, `unreachable!` arms and now-impossible
`UnsupportedIdentityFundingAccount` disappear; fix the semantically-wrong
`WalletRegistrationXpubMismatch` reuse in `upstream_identity_from_seed`
(-> `WalletStateInconsistent`).
- CODE-029: collapse `record_sent`/`record_incoming_contact_request` into one
`record_contact_request(..., direction)` over a 2-variant enum; the public
names stay thin wrappers.
- CODE-034: extract a `with_managed` closure helper for the 4x DashPay view
find-wallet -> kv -> state -> managed_identity preamble.
- CODE-046: collapse the byte-identical `contact_sidecar_key` into
`sidecar_key`.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… integrity CODE-004/011/012/015/025/029/034/046 (round-2 audit): seed_wallet helper replaces 4x repeated from_seed_bytes construction; fabricated error string-wraps replaced with typed #[source]-carrying variants; unreachable! shielded-op fallthrough replaced with a safe error path; dashpay.rs doc corrected to present tense and right Global/Identity scope count; funding-account parsing collapsed into one Funding enum; record_contact_request unifies sent/incoming; with_managed closure dedups 4 DashPay view preambles; sidecar_key dedup. Dead flush_persister/broadcast_transaction deleted (verified zero callers); sign_single_key kept per its documented not-yet-ungated-send-flow intent, with a typed SingleKeySignFailed replacing the source-discard. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
`SingleKeyView::sign_with`, `raw_key_bytes`, and the `sign_message_with_raw_key` free fn were production-visible but reachable only from unit tests — production JIT single-key signing goes through `SecretAccess` + `DetSigner`, which signs inline. Move all three behind `#[cfg(test)]` (next to the tests module) and gate their now-test-only `Message`/`Signature` imports. Delete the dead `has_passphrase` method (zero callers) and its stale doc. Audit R2 CODE-010. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…& key mapping
Wave 1 of the R2 architecture audit — secret access & signing surface dedup.
No behavior change: error mappings and length validation are preserved exactly.
- CODE-003: extract one `decrypt_message` + two-variant `DecryptError` in
model/wallet/encryption.rs; route all three AES-GCM legacy readers
(decrypt_hd_seed, SingleKeyEntry::decrypt, ClosedKeyItem::decrypt_seed)
through it. Those readers are retained deliberately — they decode secrets
already written to users' disks pending the lazy Tier-2 re-wrap, so this is
the safe local dedup, NOT the upstream per-secret migration. See
docs/ai-design/2026-07-08-secret-decrypt-dedup. Wave-16 CODE-087 still applies
unchanged (encryption.rs is kept).
- CODE-031: make `identity_key_from_bytes` and `identity_flavored` pub(crate);
route IdentityKeyView::get/get_protected through the shared length check
(standardizing get's mapping onto IdentityKeyMalformed); drop the hand-rolled
SecretSeam->IdentityKeyVault map in seal_new_identity_key_with_password.
- CODE-038: extract `versioned_bincode::{encode_tagged, decode_tagged_or}`;
wire wallet_seed_store and SingleKeyEntry encode/decode through it, per-format
fallback staying a closure.
- CODE-039: maybe_remember takes the plaintext by value and moves it into the
cache box (copied once, not twice); the duplicated per-variant boxing match
is gone.
- CODE-050: merge the identical WalletPromptMeta/IdentityPromptMeta into one
PromptMeta.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Consolidate the machinery behind the network-scoped KV sidecars and offline caches so five near-identical patterns live in one place. - Generic `SidecarView<V>` (new `sidecar.rs`): the wallet-meta, identity-meta, and auth-pubkey-cache views become thin typed wrappers parameterised by infix, value type, scope, and error mapper. Domains needing a legacy-format fallback override `SidecarValue::read` (WalletMeta's dual-format migration + one-shot re-store). - One `network_prefix` in `kv.rs`, imported at 7 sites (SPV storage paths, migration sentinels, the four sidecar keys). The MCP `network_display_name` keeps its distinct Regtest->"local" mapping. - `kv_get_logged` / `kv_get_or_default` in `kv.rs` replace seven read-then-default/None call sites (selected wallet/identity, DashPay timestamps, avatar + contact-profile caches, auth-pubkey cache via the generic view). - One `map_kv_storage_error` funnel behind all five sidecar error mappers; adds `TaskError::ContactProfileCacheStorage` so contact-profile failures stop misreporting as `AvatarCacheStorage`. - One `bip44_account0_xpub` helper replaces four copies of the fund-routing gate's BIP44 account-0 predicate (2 production + 2 test). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Mechanical typed-error and placement pass across the DashPay backend
subdomain, plus supporting cleanups.
- CODE-068 + CODE-051: move the pure DIP-14/DIP-15 derivation files to
`model/dashpay_derivation/{dip14,hd}.rs` and introduce a typed
`DerivationError` (`#[from]` bip32/secp256k1), replacing ~stringly
`Result<_, String>` derivation signatures. Wire it into `DashPayError`.
- CODE-051: replace the four `DashPayError::Internal { message }` boundary
wraps with typed `TaskError` at their source functions
(`payments::load_payment_history`, `register_dashpay_addresses_for_identity`,
`generate_auto_accept_proof`); add a typed `KeyVerificationError` for the
three identity voting/owner/payout key checks; carry the per-vote
`DPNSVoteResults` error as `Arc<TaskError>` instead of a pre-formatted
String.
- CODE-053: carry DashPay payment amounts as `u64` duffs end-to-end
(task field, backend fn, `DashPayPaymentSent` success variant); the UI
resolves duffs at its edge — no f64 crosses the backend boundary.
- CODE-055: drop the redundant `encryption_tests.rs` (its three checks are
already covered by superior `#[cfg(test)]` tests in `encryption.rs`);
delete the module decl and the dead task builders.
- CODE-059: migrate the two UI callers to `Display`; delete `user_message()`.
- CODE-060: delete `ckd_pub_256`, the legacy `send_payment_to_contact`
wrapper (rename `_impl`), the dead `profile::send_payment` /
`identity_to_child_number`, `From<String>`, the `ToDashPayError` trait,
the `DashPayResult` alias, the unused error-factory helpers, and the
now-unconstructed `Internal` + dead taxonomy variants, with their
speculative allows.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…urface Wave 4 architecture-audit cleanup. Consolidates duplicated fee, amount, and DPNS logic onto single sources of truth and removes a dead send dialog. - CODE-090: replace the 9-arg estimate_contract_create_detailed with a ContractComponents struct; delete the argument-ignoring from_platform_version (no callers); /1000 -> /CREDITS_PER_DUFF; derive CREDITS_PER_DASH from DASH_DECIMAL_PLACES. - CODE-095: move estimate_platform_fee, both transition-based estimators, AddressAllocationResult and allocate_platform_addresses[_with_fee] out of send_screen.rs into model/fee_estimation.rs; hoist ESTIMATED_BYTES_PER_INPUT to one module constant. Add unit tests for the convergence, shortfall and fee-payer/destination paths. - CODE-093: consolidate all credits/duffs->DASH display formatting on the model formatters (add format_duffs_as_dash); delete the private format_dash/ format_credits copies, the local CREDITS_PER_DUFF re-declaration and the hand-rolled float conversions; one trimmed-Amount precision policy. - CODE-077: Amount::partial_cmp returns None unless is_same_token, keeping ordering consistent with equality. - CODE-092: strip_dash_suffix branches on has_dash_suffix and slices once; validate_dpns_input returns a fieldless NonDashDomainError. - CODE-103: move account_summary into ui/state (non-widget view state). - CODE-099: delete the unreachable SendDialogState, render_send_dialog, prepare_send_action, the field/init/render call and orphaned imports. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…dead editor Wave 17 — DashPay UI cleanup (three findings). PROJ-003 + CODE-097 — single avatar pipeline. Add a rendering `Avatar` component (`ui/components/avatar.rs`) backed by an `AvatarCache` fetch state (`ui/state/avatar_cache.rs`) that dispatches through the App Task System (`DashPayTask::FetchAvatar` → `BackendTaskSuccessResult::DashPayAvatar`). Disk-cache consult/populate now happens once, in the backend task (`avatar_processing::fetch_avatar_cached`). The three screens (contact_profile_viewer, contacts_list, profile_screen) delegate to the component instead of each running a raw `tokio::spawn` with its own texture map and decode/stash copies. No orphaned frame-loop spawns remain. CODE-094 — one profile-field validator. Add `validate_profile_fields` plus `MAX_*_CHARS` constants and `ProfileFieldError` to `model/dashpay.rs` (char-count, matching the protocol). The backend size check and both editors (profile_screen, identity settings) delegate to it, collapsing four divergent copies — including the avatar-URL cap, unified on the DIP-0015 value of 2048. Empty display name is decided legal (matches the backend and DIP-0015). CODE-100 — delete the dead `contact_info_editor.rs` (no opener anywhere) and its `ui/mod.rs` wiring. Extract the shared nickname/note/hidden save into `persist_contact_private_info`, used by the three live inline editors (contact_profile_viewer, contact_details, contacts_list). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CODE-002/006/008/022/026 (round-2 audit): generic SidecarView<V> replaces four near-identical sidecar wrappers (wallet_meta, identity_meta, auth_pubkey_cache); one network_prefix helper replaces 8 duplicated inline copies; map_kv_storage_error funnels 5 error mappers, fixing contact_profile_cache misreporting as AvatarCacheStorage; kv_get_logged/kv_get_or_default collapse 7 KV read call sites; bip44_account0_xpub dedups all 4 BIP44 xpub-finder copies (prod + test). Resolved a real conflict in upstream_identity_from_seed: this wave's base predates Wave 2's seed_wallet() helper and WalletStateInconsistent fix (CODE-004/CODE-025), so it independently reconstructed the wallet and kept the old WalletRegistrationXpubMismatch variant. Combined both: seed_wallet() for construction, bip44_account0_xpub() for the account lookup, WalletStateInconsistent for the absence case. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…t params Structural fee honesty and token-op deduplication across the token backend: - FeeResult.actual_fee is now Option<u64> with a FeeResult::estimated_only constructor; the 22 sites that faked an actual fee by passing the estimate twice now report an estimate only, so the UI/MCP no longer claim a settled fee the platform never returned. - Extract AppContext::execute_token_op: a single executor owning the SDK-call error mapping, post-broadcast side effects, and the fee tail. All 11 token state-transition ops shrink to their builder setup plus a delegation. - Introduce TokenContractParams, replacing the 26-field RegisterTokenContract variant and the 26-argument build_data_contract_v1_with_one_token with one grouped struct; delete the always-NotTradeable marketplace_trade_mode field and its dead selector (its match arms were identical). - run_token_task now matches the owned task, moving fields into handlers instead of cloning, aligning with document.rs/dashpay.rs. Consumers (send screen, MCP identity/masternode outputs, e2e assertions) updated for the Option-typed actual fee. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Remove fragile string parsing and duplication from the contract/document/ contested-names recovery paths: - Delete extract_contract_id_from_error (parsed the contract id out of an error message string). register/update now use data_contract.id(), already in scope before broadcast, and share one AppContext::recover_contract_after_ proof_error helper built on log_drive_proof_error with named refetch delays. - Add a shared log_contested_proof_error for the GroveDB proof-failure shape the three contested-resource queries surface, collapsing three identical logging blocks; run_contested_resource_task now matches the owned task. - Extract DocumentTask::fetch_document_for_mutation (fetch-by-id + bump revision) used by transfer/purchase/set-price, and convert the six multi-field DocumentTask tuple variants to named-field struct variants for readable construction at the UI call sites. - Document/contract fee tails switch to FeeResult::estimated_only. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CODE-010/031/038/039/050 (round-2 audit): test-only single-key signing helpers scoped to #[cfg(test)]; identity-key length check + corrupt-key error mapping unified (SecretDecryptFailed -> IdentityKeyMalformed, more accurate for a corrupt identity key); versioned_bincode helper dedups wallet-seed/single-key-entry framing; maybe_remember takes plaintext by value, eliminating a double-copy; WalletPromptMeta and IdentityPromptMeta merged into one PromptMeta. CODE-003: adopted route (a), a shared decrypt_message/DecryptError next to encrypt_message, after investigation showed the three "duplicated" decrypt sites are the legacy-format reader needed for secrets not yet re-wrapped to Tier-2 encryption (which already exists and already lazy-migrates on unlock) -- deleting them would lock out any user who hasn't unlocked since Tier-2 shipped. This supersedes my earlier direction to attempt the full migration; see docs/ai-design/2026-07-08-secret-decrypt-dedup/README.md for the route-(b) migration playbook if it's ever scheduled. Wave-16 CODE-087 still applies unchanged. Independently security-reviewed (Smythe, opus) before merge: all 6 changes verdict SAFE, error classification preserved exactly at all three CODE-003 call sites, zero production callers of the now-test-only signing helpers confirmed by tracing the JIT path, no production match on the changed SecretDecryptFailed variant, zeroization holds on every exit path. One LOW/non-blocking finding: the resolve loop now unconditionally copies plaintext before checking the remember policy (zeroized immediately, no leak, just an avoidable transient copy when policy is None) -- left as a minor follow-up, not merge-blocking. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Wave 15 of the architecture audit — strip upward edges out of `model/`, type the MCP tool errors, and consolidate the tool helpers. - PROJ-005: move `RootScreenType`/`ThemeMode` into `model::settings` (re-exported from `ui`); invert the `model → backend_task` error edges in `passphrase.rs` and `wallet/mod.rs` via model-local error enums wired into `TaskError` with manual `From` impls. `qualified_identity` keeps `TaskError` because the `SecretAccess::with_secret` chokepoint contract requires it (documented). - CODE-070: relocate `FeatureGate` + predicate to `context/` (dead `FeatureGateUiExt` deleted); move token `From` conversions next to their UI types; `IdentityStatus → Color32` mapping to `ui::theme`; model-local `MasternodeInputError` converted to `McpToolError` at the tool boundary. - CODE-072: move GroveSTARK proof generation/verification into `backend_task::grovestark` (data types + serialization stay in `model`), drop 59 info-level hex dumps, type `GroveSTARKError` with `#[source]` fields. - CODE-073: `qualified_identity_public_key` extracts `find_wallet_path` and warn-skips malformed network-supplied key data instead of panicking. - CODE-080/081/082/083: delete dead items (gate test-only ones with `#[cfg(test)]`), collapse the auth-key accessors behind `authentication_keys_matching`, drop dead `RequestType` u8 conversions and rename the module to `request_type`, delegate `borrow_decode` to `decode`. - CODE-079: `tool_ctx()` returns `McpToolError` natively — kills the lossy `McpError → String` round-trip at all 26 tool invocations. - CODE-085: fold the blank-`network` check into `resolve::require_network`; merge `validate_amount`/`validate_credits` into `validate_positive_amount`. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CODE-054/052/057/056/058/063/064 (round-2 audit): AppContext::execute_token_op consolidates 11 token state-transition ops (builder-setup + delegation only, per-op files shrink accordingly); FeeResult.actual_fee becomes Option<u64> with estimated_only() so estimate-vs-actual is structurally honest (22 sites converted, 6 genuine-actual sites keep new()); extract_contract_id_from_error string-parsing deleted in favor of data_contract.id(); recover_contract_after_proof_error / log_contested_proof_error share contract/contested-names proof-error recovery; fetch_document_for_mutation extracted, DocumentTask tuple variants become named-field structs; TokenContractParams struct replaces a 26-field/26-arg construction, dead marketplace_trade_mode selector deleted; both token/contested-resource dispatchers match by value instead of cloning. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
… identity-hub flag Wave 12 of the full-repo architecture audit — app.rs boot path, feature flags, and the context provider. PROJ-001: collapse the four duplicated eager wallet-backend init shapes (boot, network switch, post-onboarding auto-start, manual Connect) into one `AppState::spawn_backend_init` parameterized by a `BackendInitReason`. Extract the per-frame reconcilers accreted on `AppState` into small `update()`-style structs under `src/app/reconcilers.rs` — `AccessibilityActivator`, `SpvBlockReconciler`, `ConnectionBanner`, and `MigrationReconciler` (the latter two return a `BackendTask` for `AppState` to dispatch, keeping the channel chokepoint in one place). Reduces `AppState` from ~13 flat reconciler fields to four cohesive members; no behavior change. PROJ-004: move scheduled-vote casting off the UI thread into a new `ContestedResourceTask::CastDueScheduledVotes` backend task. The 60s tick only dispatches; the DB query, identity load, and casting run in the task; the DPNS Scheduled Votes screen learns progress via `display_task_result` (`ScheduledVotesInProgress` then per-vote `CastScheduledVote`). Removes the two frame-aborting `return`s and names the 2-minute lateness window (`SCHEDULED_VOTE_MAX_LATENESS_MS`). PROJ-006: delete the `identity-hub` and `identity-hub-activity-feed` Cargo features and every `cfg` fallback branch — the hub is now always built and registered; the Activity tab renders its gated "coming soon" message unconditionally. PROJ-008: fold `context_provider_spv.rs` into `context_provider.rs`. Drop the unused `db` field and `_db` params, make `SpvProvider::new` infallible, and replace the `Mutex` with an `Arc<ArcSwapOption>` so the manual `Clone` impl and lock-poison plumbing disappear. Keeps the `SYSTEM_CONTRACT_COUNT` compile check. PROJ-009: give dir/env/logger boot setup a single owner, `boot::prepare_environment()`, called from both `main` and `AppState::boot_inputs`; the logger's `Once` guard keeps the second call a no-op. Preserves the `testing`-cfg `boot_inputs` split. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…rcing CODE-095/093/090/077/092/103/099 (round-2 audit): fee estimation helpers moved into model/fee_estimation.rs with one ESTIMATED_BYTES_PER_INPUT constant and new convergence/shortfall/ fee-payer tests; all credits/duffs-to-DASH display formatting consolidated on the model formatters; ContractComponents struct replaces a 9-arg fee estimator; Amount::partial_cmp returns None across different tokens; strip_dash_suffix branches once; dead SendDialogState/render_send_dialog/prepare_send_action deleted; account_summary.rs moved to ui/state/. CODE-098 (folding the three shielded screens into the unified send screen) deliberately deferred to its own follow-up wave. Resolved a conflict in send_screen.rs: Wave 3 (merged first) added format_fee_info for the new Option<u64> actual_fee; Wave 4 deleted the now-dead format_dash/format_credits (consolidated onto model formatters, zero remaining callers confirmed) from the same region. Kept format_fee_info, removed the dead pair. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…erivation move
CODE-051/068/053/055/059/060 (round-2 audit): dip14_derivation.rs and
hd_derivation.rs moved to model/dashpay_derivation/ with a new typed
DerivationError wired into DashPayError; four stringly Internal{message}
boundary wraps replaced with typed TaskError returns; new
KeyVerificationError for the three identity voting/owner/payout key
checks; DPNSVoteResults per-vote error becomes Arc<TaskError>; payment
amounts carried as u64/duffs end-to-end instead of f64; dead
encryption_tests.rs deleted (already covered by better in-file
#[cfg(test)] tests); two UI error callers migrated off user_message()
to Display; 12 dead/never-constructed error items removed.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
PROJ-003 + CODE-097 (one task), CODE-094, CODE-100 (round-2 audit): single avatar pipeline -- rendering Avatar component backed by a pure-state AvatarCache dispatching fetch through a new DashPayTask::FetchAvatar backend task; validate_profile_fields + char-limit constants added to model/dashpay.rs; avatar-URL cap unified on 2048; dead contact_info_editor.rs deleted with its ui/mod.rs wiring. Behavior change worth flagging for the changelog: empty display name is now legal (matches the backend/DIP-0015), so profile_screen no longer blocks Save on a blank display name. Resolved two conflicts: ui/state/mod.rs needed both this wave's avatar_cache module and Wave 4's account_summary module declarations (purely additive). backend_task/mod.rs's DashPayPaymentSent variant -- this wave's base predated Wave 13's CODE-053 fix (amount f64 -> u64 duffs); kept Wave 13's u64 signature and added this wave's new DashPayAvatar variant alongside it. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
PROJ-005/CODE-070/072/073/080/081/082/083/079/085 (round-2 audit): RootScreenType/ThemeMode moved into model/settings; model-local PassphraseError/PaymentValidationError/WalletCreationError wired into TaskError via manual From impls; FeatureGate + predicate moved to context/, dead FeatureGateUiExt deleted; GroveSTARK proving moved to backend_task/, 59 info-level hex dumps removed; find_wallet_path extracted, three network-supplied-key .expect() panics converted to warn-skip; dead items deleted; authentication_keys_matching(predicate) replaces two near-duplicate methods; proof_log_item renamed to request_type; MCP tool_ctx() returns McpToolError natively at all 26 invoke sites; blank-network/amount validation consolidated. Resolved 3 merge conflicts (all Wave-3/Wave-4 base-drift, mechanical): register_contract.rs and update_data_contract.rs each had a stale request_type::RequestType import left over from before Wave 3's CODE-056 refactor removed the code that used it -- dropped as dead. wallets_screen/mod.rs needed the NEW context::feature_gate::FeatureGate path (post-move) plus Wave 4's format_duffs_as_dash, dropped an unused model::amount::Amount import. Also fixed one non-conflicting stale import this rename missed: backend_task/contested_names/mod.rs's log_contested_proof_error (added by Wave 3, same rename collision). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ename Missed in the Wave 15 merge commit -- contested_names/mod.rs's log_contested_proof_error (added by Wave 3) imported RequestType from the old model::proof_log_item path, which Wave 15 renamed to model::request_type. Same collision class as the two request_type import conflicts resolved in that merge, just not flagged by git since the lines didn't overlap. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…t_provider PROJ-001/004/006/008/009 (round-2 audit): four duplicated eager wallet-backend init shapes collapsed into one AppState::spawn_backend_init + BackendInitReason; accreted per-frame reconcilers extracted into app/reconcilers.rs; scheduled-vote casting moved into a ContestedResourceTask backend task; identity-hub / identity-hub-activity-feed features deleted entirely; context_provider_spv.rs folded into context_provider.rs, db field/params dropped, SpvProvider::new infallible, Arc<ArcSwapOption> replaces Mutex; single boot::prepare_environment() owns dir/env/logger setup. Resolved one conflict in context/mod.rs: needed both Wave 15's context::feature_gate::FeatureGate import (post feature_gate move) and this wave's context_provider::SpvProvider import (post context_provider_spv.rs fold). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…, param & fn moves Wave 7 of the round-2 architecture audit on context/contract_token_db.rs: - CODE-018: propagate TokenConfiguration encode failures through a new typed TaskError::TokenConfigSerialization variant instead of silently no-oping; drop the pre-C7 tombstone comment. - CODE-019: extract load_sorted_tokens(); both token listers map its output, killing the warn-vs-silent drift on unparseable token keys. - CODE-030: drop three dead parameters — get_contracts pagination (limit/offset, all callers passed None,None), ConnectionStatus::tooltip_text app_context, and request_to_det_contact's unused ContactRequest. - CODE-033: move remove_wallet to context/wallet_lifecycle.rs, next to register_wallet and its existing tests. - CODE-044: iterate the system contracts for the five get_contracts insertion blocks; fold the five system-contract load blocks in context/mod.rs into one local closure. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…ontext nits CODE-016/023/027/040/042/045 (round-2 audit): write-only Core-RPC health plumbing deleted (rpc_online/rpc_last_error + dead ChainLocks handling, zero readers); settings updaters consolidated into a closure-based update_app_settings() that holds the cached_settings write guard across the full read-modify-write, fixing the lost-write race under concurrent updates (two new tests: sequential RMW + a 6-thread concurrent-distinct -field-flip proving no lost writes); seed_platform_address_entries() extracted, both platform-address seeding callers now thin adapters over it; contest-duration magic numbers named with spec refs; four small dedups (default_platform_version arms, spv_connected inlined, scheduled-vote fns take borrowed slices, identity dead map_kv_error wrapper removed). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CODE-107: Extract one generic add_subscreen_chooser_panel + nav_button (subscreen_chooser_panel.rs); the four dashpay/dpns/tokens/tools panels now build item lists and delegate. Move ToolsSubscreen next to the tools screens (ui/tools/mod.rs), matching its siblings. CODE-109: Extract modal_chrome (backdrop + centered bordered Window) from passphrase_modal; rebase passphrase_modal and the confirmation/selection/ info dialogs onto it. Dedupe the NOTHING sentinel into modal_chrome. Drop ConfirmationDialog's write-only `status` field (current_value never read it). CODE-110: Rewrite ScreenType::eq as explicit payload arms + discriminant equality (no `_ => false`). Collapse the seven ScreenLike delegation methods onto one exhaustive delegate_to_screen! macro — a missing variant is now a compile error. (Macro, not a dyn accessor, to preserve inherent-method resolution for screens with an inherent refresh.) CODE-113: Extract key_eligibility, render_key_combo, render_no_eligible_key_group and render_info_section in ui/helpers.rs; the two key choosers and two success screens delegate. Unify the divergent no-eligible-key wording. CODE-115: Wire the contract-chooser right-click via Response::context_menu on the header row (Copy Hex / Copy JSON), removing the unreachable manual Window and its three dead state fields; derive Default; flatten the font-size ladder. CODE-117: Extract stage_progress + window_fraction in network_chooser_screen; the five progress calculators shrink to thin wrappers. Also folds in a stray cargo-fmt import-order fix in backend_task/contested_names/mod.rs (Wave 15 rename residue). Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
Model-layer cleanups across the wallet crate. All error paths in model/wallet now carry typed WalletError up to the TaskError boundary; the platform-payment registration, coin-type mapping, and AES-GCM envelope shapes are single-sourced. - CODE-071: model/wallet/mod.rs derivation/registration functions return Result<_, WalletError> instead of stringly errors (~17 KeyDerivation erasures + 5 bare erasures gone). New typed WalletError variants: PublicKeyParse, AccountDerivationPath, PlatformAddressConversion, AddressNetworkMismatch. TaskError::WalletAddressProviderSetupFailed now holds a typed #[source] instead of a String. database/utxo.rs get_utxos_by_address returns rusqlite::Result. - CODE-076: delete Wallet::coin_type + four inline network matches; call the canonical coin_type_for_network everywhere. - CODE-078: extract Wallet::register_platform_payment_entry; the five duplicated known/watched insert blocks collapse to one call. The vestigial post-T-W-01 `register` param on generate_platform_receive_address_with_seed is dropped. - CODE-087: replace the (Vec<u8>,Vec<u8>,Vec<u8>) crypto triple (and its type_complexity allows) with a named EncryptedEnvelope struct. - CODE-088: one summary log per platform-address sync; the per-key dumps in WalletAddressProvider::on_address_found and QualifiedIdentity::sign move behind their failure paths, off the hot loop. - CODE-089: delete the unused DASH_SECRET_MESSAGE constant. - CODE-091: cross-link + status-mark the three single-key modules (LIVE imported-key sidecar vs the LEGACY Decision-#7 runtime/DB pair). cargo clippy --all-features --all-targets -D warnings clean; full lib suite (1364 tests) green. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CODE-107/109/110/113/115/117 (round-2 audit): generic add_subscreen_chooser_panel + nav_button replace 4 near-identical dashpay/dpns/tokens/tools panels (~500 dup lines gone); modal_chrome.rs extracted from passphrase_modal, rebasing confirmation/selection/info dialogs onto it; ScreenType::eq rewritten as explicit payload arms + discriminant catch-all, seven ScreenLike delegation methods collapsed onto one delegate_to_screen! macro (preserves inherent-method resolution, verified behavior-identical against every non-default override); helpers.rs key-eligibility rendering deduped across 3 screens; contract-chooser context menu made actually reachable via right-click (was a dead fixed-pos Window nothing ever opened); five progress-stage calculators reduced to thin wrappers over two shared functions. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CODE-071/076/078/087/088/089/091 (round-2 audit): derivation/ registration fns return typed Result<_, WalletError> instead of stringifying errors (new PublicKeyParse/AccountDerivationPath/ PlatformAddressConversion/AddressNetworkMismatch variants; TaskError::WalletAddressProviderSetupFailed's detail field typed); Wallet::coin_type + 4 inline match-network copies collapsed onto coin_type_for_network; register_platform_payment_entry extracted, collapsing 5 duplicated known/watched insert blocks; the (Vec<u8>,Vec<u8>,Vec<u8>) triple replaced by a named EncryptedEnvelope struct (CODE-003 resolved as route (a), local dedup, so encryption.rs stayed and this applies as planned); per-key debug dumps moved behind their failure paths, one summary log per platform-address sync; dead DASH_SECRET_MESSAGE deleted; the three single-key modules cross-linked and status-marked (LIVE sidecar vs LEGACY runtime/DB pair) rather than renamed, for zero-risk traceability. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…E-098)
The three standalone shielded screens (shield / shielded-send / unshield)
duplicated flow logic the unified WalletSendScreen already owns end to end
(dispatch + result handling for every shield/unshield/private-send path).
Consolidate them into routes on the one canonical send screen.
- Add `SendFlow` preset {General, Shield, ShieldedSend, Unshield}. A preset
locks the source — and, for Shield, auto-targets the wallet's own pool with
a sentinel shielded destination (the shield dispatch ignores the destination
address) — so the screen shows only the controls that flow needs while
reusing the unified validation, fee/amount limits, and dispatch.
- Extract shared shielded-recipient parsing into
`model::address::parse_shielded_recipient` (Bech32m or 43-byte hex) so the
send dispatch and any validation path cannot diverge.
- Route the Shielded tab's Shield / Send (Private) / Unshield buttons to open
the unified send screen pre-configured for the flow.
- `ScreenType::WalletSendScreen` now carries the `SendFlow`; remove the three
`ScreenType`/`Screen` shielded variants and delete the standalone screen
files.
Behavioral parity preserved for all five shielded dispatches. One delta:
raw-hex shielded recipient entry is dropped from the UI (AddressInput accepts
canonical Bech32m only); the parser still accepts hex at dispatch.
Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
…renderers Collapse the seven near-identical single-shot token action screens (pause, resume, freeze, unfreeze, destroy-frozen-funds, burn, mint) onto one parameterised scaffold, and merge the two duplicated control-rule renderers. - Add `TokenActionScreen<A: TokenAction>` (ui/tokens/token_action_screen.rs): the shared shell — authorization resolution, wallet unlock, advanced key chooser, per-action form slot, public note, fee estimate, confirmation dialog, success screen and status. Each action supplies only its diffs (labels, rules accessor, form, task builder, success variant) via the `TokenAction` trait; the seven screens become thin `type` aliases + a small action struct. Net ~3,900 lines removed. - Fold the token authorization check into the shared scaffold's one call site via the previously zero-caller `check_token_authorization`; also route `update_token_config`'s inline resolver through it. - Merge `render_mint_control_change_rules_ui` into `render_control_change_rules_ui` via an optional `MintRecipientSection`, deleting the ~180-line duplicate; update all call sites. - `change_context` now rebinds token-screen context through `.common` via a `set_app_context` setter and a `common_set` macro arm. Deferred (TODO markers in-file): set_token_price migration onto the scaffold (large pricing form; verb-based auth message template does not fit "set price" grammatically) and the TokensScreen god-struct per-subscreen split. cargo fmt + clippy (all-features, all-targets) clean; lib unit tests and the kittest suite pass. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
CODE-104/105/108 (round-2 audit): new parameterized TokenActionScreen<A: TokenAction> is the shared shell (auth resolution, wallet unlock, advanced key chooser, form slot, public note, fee estimate, confirmation dialog, success screen, status/refresh/ display_task_result) for 7 of the 8 token action screens (pause/ resume/freeze/unfreeze/destroy_frozen_funds/burn/mint), each now a type alias + small Action struct; the scaffold's constructor is the single call site of the previously zero-caller check_token_authorization, subsuming 7 inline copies; render_mint_control_change_rules_ui merged into render_control_change_rules_ui via an optional bundle, deleting a ~180-line duplicate across 12 call sites. Net ~3900 lines removed. set_token_price (8th screen) and the TokensScreen god-struct split are deliberately left as TODOs -- the former needs a UX decision on an ungrammatical verb template, the latter is a separate large refactor that would rewire thousands of field references; both flagged for their own future dedicated work rather than riding along in this wave. Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
- 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>
Contributor
|
Caution Review failedAn error occurred during the review process. Please try again later. ✨ 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 |
Open
4 tasks
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>
lklimek
merged commit 61df418
into
docs/platform-wallet-migration-design
6 checks passed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why this PR exists
CLAUDE.md's DET Module Placement Policy).String-typed errors that bypass the type system, dead code that misleads future readers, and a few latent bugs that only surface under specific conditions (see "What was done" below for the concrete ones this PR actually fixes).docs/platform-wallet-migration-design(PR feat: platform-wallet backend rewrite (spec + implementation) #860's head branch). It is meant to merge into that branch, not directly intov1.0-dev.What was done
131 findings from a full-repo architecture audit were triaged; ~120 were fixed across 21 independently implemented and merged waves plus one funds-adjacent follow-up (the shielded-screen consolidation), each built and verified in its own isolated git worktree before integration into this branch. The remainder were deferred with self-contained
TODOcomments or accepted as documented, low-risk exceptions.The two highest-risk waves — the secret/signing-surface changes (Wave 1) and the shielded-screen consolidation (CODE-098) — each went through an independent QA and/or security review pass in addition to the standard build/clippy/test gate, given their proximity to secret and fund handling.
This work is overwhelmingly internal and has no user-visible effect: type-safety improvements (stringified errors replaced with typed
TaskError/WalletErrorvariants), deduplication of near-identical UI screens and backend helpers, dead-code removal, and module reorganization to match the project's documented layering rules.A small number of findings did surface genuine, user-observable fixes and one deliberate UI consolidation:
All of the above are also reflected in
CHANGELOG.md. Full per-finding detail lives in the 22 merge commit messages on this branch (Merge branch 'fix/r2-w1'…'fix/r2-w21','fix/r2-code098'). The design rationale behind Wave 1's secret-decrypt dedup is in-repo atdocs/ai-design/2026-07-08-secret-decrypt-dedup/.Testing
cargo +nightly fmt --all -- --check— cleancargo clippy --all-features --all-targets -- -D warnings— cleancargo test --all-features --workspace— all green:#[ignore]d in this environment)ui::components::identity_selector::tests::with_app_default_inert_when_global_id_not_in_candidate_listfails intermittently under parallel--libexecution (shared global secret-store lock) but passes clean single-threaded and on repeated full-suite runs — not a regression introduced by this round.Breaking changes
None for end users. Internal API surface changed extensively (typed errors replacing stringified ones, several modules moved/renamed per the placement policy) — anything depending on this crate's internals rather than its UI would need to update call sites, but there are no external consumers of this crate.
Checklist
CHANGELOG.mdupdated with user-facing changesAttribution
🤖 Co-authored by Claudius the Magnificent AI Agent
Follow-up remediation — independent full-repo review (round 3)
After the waves above, an independent full-codebase review (single reviewer, structural/consistency/quality focus) surfaced 18 additional findings. All 18 were triaged fix and remediated across 6 sequential waves plus an adversarial QA pass, each committed on this branch:
detail: String/reason: Stringcluster (21platform_infosites + others) to#[source]-carrying variants; relocated key-format validation tomodel/key_input.rswith a typed error; deleted deadDashPayErrorvariants; DPNScontract not foundsubstring-match TODOs now reference tracked issue Replace 'contract not found' error-string matching in contested-name queries with a structural SDK variant #875 (the SDK exposes no structural variant client-side).wallet_backend/poison.rstraits +Database::locked_conn()(fail-loud preserved for the fund-safety send-index); routedsingle_key.rsraw vault I/O through theSecretSeamdoorway; zeroized the encryption module's derived keys and typed its errors.model/address.rsand closed a funds-adjacent gap: theidentity_credits_to_addressMCP tool now rejects a destination whose network HRP mismatches the active network (previously unchecked).AppContext::wallet_arc()replaces a 15× copy-paste; corrupt address rows now log instead of silently dropping; eliminated all 263 panickingunwrap()/expect()in production paths (error-propagation where fallible, gracefulMessageBannerdegradation in egui frame code).dead_code, and a redundant predicate; split the 4305-linecontext/wallet_lifecycle.rsgod-file into responsibility-scoped submodules (verified a pure zero-behavior code-move).surfacecard (the earlier gradient read as a misplaced blue block; wireframes govern placement, the app theme governs colors).Verification: an adversarial QA pass independently confirmed the god-file split is a byte-for-byte pure move and the funds gate is sound, and caught two blockers (a relocated-test allow-list path + a missed repaint-loop lock) which were fixed. Final gate on the merged tree:
clippy --all-features --all-targets -D warningsclean,cargo test --all-features --workspace1596 passed / 0 failed.Deferred follow-ups (tracked, out of scope here): typing the dashpay ECDH
TaskError::EncryptionErrorcascade; the fullSingleKeyWallet/WalletSeedString-API type-through; stretch god-file splits (send_screen,tokens_screen, the 2859-LOCtests.rs); makingresolve::validate_addressnetwork-aware for Core addresses.