feat: rewrite Dash Evo Tool onto the new platform-wallet#860
Conversation
|
Important Review skippedToo many files! This PR contains 527 files, which is 427 over the limit of 100. To get a review, narrow the scope: Upgrade to a paid plan to raise the limit. Usage-priced reviews support at most 300 files. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: ⛔ Files ignored due to path filters (2)
📒 Files selected for processing (579)
You can disable this status message by setting the ✨ 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 |
…lures Fixes thepastaclaw's recurring BLOCKING finding on PR #860: a failed SPV init permanently stranded chain sync with no way to retry or recover via Disconnect. See fix/pr860-spv-start-latch (00201d0). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017x3fA7FZHMKATyRXWD2DEZ
lklimek
changed the title
resp.info_tooltip() isn't #[must_use] and every other call site in the repo calls it bare, no binding — matches lklimek's PR #860 review comment questioning why the unused let _resp exists. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_017x3fA7FZHMKATyRXWD2DEZ
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
The SPV retry defect is fixed: initialization failures reset the latch, propagate to callers, and permit Disconnect recovery; the targeted regression test passes. Two new blocking lifecycle defects are confirmed in token refresh and scheduled-vote sweeping, and the three carried-forward documentation/test findings remain valid.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (failed_pre_evidence),gpt-5.6-sol— general (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 2 blocking
3 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/backend_task/tokens/query_my_token_balances.rs`:
- [BLOCKING] src/backend_task/tokens/query_my_token_balances.rs:197-217: Expired refresh lease permits false-success retries
The maximum-lifetime timer clears only DET's single-flight flag; it does not stop the managed refresh or release upstream's synchronization slot. At the pinned platform revision, `IdentitySyncManager::sync_now()` immediately returns without work while `is_syncing` remains set. After this lease expires, a retry can acquire a new lease, encounter that upstream no-op, republish the unchanged snapshot, and return `FetchedTokenBalances`. In the permanent-hang case the change is intended to recover from, every later refresh can therefore report success while balances remain stale. Couple lease release to completion of the upstream pass or introduce an upstream cancellation/reset mechanism before allowing another successful refresh.
In `src/app.rs`:
- [BLOCKING] src/app.rs:2075-2080: Scheduled-vote guard survives backend task panics
The in-progress network entry is removed only for `ScheduledVoteSweepCompleted` and `ScheduledVoteSweepFailed`. A panic in the spawned backend task is converted by the join watcher into `TaskError::BackendTaskFailed`, while `CastDueScheduledVotes` is classified as `BackendTaskContext::Other`. That result reaches the generic error arm without removing the network from `scheduled_vote_sweeps_in_progress`, so no later sweep is dispatched for that network until restart. Preserve the sweep network in its task context and clear the guard for every terminal result, including join failures.
…llers WalletBackend::start_once() previously let a losing caller in a concurrent-start race fall through to its own independent call into upstream SpvRuntime rather than observing the winner's actual result — so concurrent start() callers could see inconsistent outcomes (a fabricated/generic error on one side, the real error or success on the other). Reworked StartLatch to hold a resettable Arc<StartFlight> backed by a tokio::sync::OnceCell: every concurrent caller now joins one shared-result flight and all observe the same outcome, while a completed flight still short-circuits repeat calls without spawning a second SPV run loop, and a stale flight captured before a reset cannot initialize the replacement generation. TaskError::WalletBackend now carries Arc<PlatformWalletError> (rather than an owned value) so a single flight's error can be cloned out to every waiting caller — TaskError itself isn't Clone. A lifecycle mutex now serializes start() against stop_in_place() so a stop can't race a concurrent start attempt. New test: concurrent_spv_start_failure_is_returned_to_every_caller. context::wallet_lifecycle 62/62 and wallet_backend 321/321 pass; clippy and fmt clean. Grumpy-review PR #860 finding (start-latch loser race), blocking. Co-Authored-By: Codex Sol <noreply@openai.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
… encoding Two grumpy-review findings against PR #860's migration path, plus a guard-bypass gap surfaced during the fix's own re-review: - migrate_identities_from_conn() trusted an index-derived startup snapshot to decide whether a legacy identity row was already migrated. A deletion racing the migration between snapshot and insert could leave a stale legacy row that a later partial-pass retry then resurrected into the modern store as a "deleted" identity reappearing. Added a direct has_current_blob() check against storage before every pending insert, and record_processed() durably advances the processed set after either an insert or a direct-presence discovery so a retry can't repeat the race. - StoredSeedEnvelope's tagged encode path (encode_with_version) built its `[version || bincode(body)]` wire buffer in a plain, non-zeroing Vec<u8> even though the envelope holds a wallet seed — the buffer leaked seed bytes to heap until reallocation. encode() now returns Zeroizing<Vec<u8>>. - Re-review of the deletion-race fix found the shared migration guard was only enforced at the dispatcher, so a direct caller of the public migration entry point could bypass it. Moved lock ownership to that public boundary (run_under_guard) so every entry path is serialized against an in-flight identity deletion. Red-before/green-after confirmed for the guard-bypass gap (public_migration_run_waits_behind_idle_deletion_guard panicked before the fix, passes after). Full migration suite: 92/92 pass; clippy clean. Co-Authored-By: Codex Sol <noreply@openai.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
…nner after DB clear Three grumpy-review findings, all touching the network-database-clear path: - The Network Chooser and the top panel derived a network's display label independently and disagreed after a database clear (e.g. one side showing "Regtest", the other "Local"). Extracted the single source of truth, chooser_network_label(), and routed both call sites through it. - The DPNS contested-names screen rendered its own inline error banner in addition to the global MessageBanner AppState already displays for the same backend-task error, showing the failure twice. Removed the inline render path — MessageBanner is now the sole path for this screen's backend-task errors. - A scheduled-vote-sweep failure raised while no local voting identity exists is expected background noise (there's nothing to sweep for), but it surfaced as a persistent error banner with no way to dismiss it short of navigating away. AppState now recognizes ScheduledVoteSweepFailed wrapping a NoVotingIdentity source via scheduled_vote_sweep_is_quiet() and handles it without a banner. Targeted tests pass; clippy and fmt clean. Co-Authored-By: Codex Sol <noreply@openai.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
…uccess token refreshes Two grumpy-review findings: - Document/claim fetch results were matched back to the screen that requested them by type alone, not by which specific dispatch they answered. A stale error from an earlier, already-superseded fetch could arrive after a newer identical redispatch had already succeeded and suppress or overwrite that success. Introduced a process-monotonic dispatch id (BackendTaskContext::for_dispatch), threaded through AppAction/AppState, so each screen's correlate_fetch_action() / should_suppress_backend_task_error() can tell an outdated response from the one it's actually waiting on. - sync_token_balances_now() reported success whenever it merely *called* the upstream sync, even when upstream's own single-flight guard silently skipped the pass because another refresh was already running (is_syncing() true) — the UI showed balances as fresh when no sync actually happened. Added TokenBalanceSyncOutcome (Performed/AlreadyInFlight) so callers can distinguish a completed refresh from a skipped one, and a new TaskError:: TokenBalanceRefreshSkipped for the skipped case. Targeted suites pass (contracts_documents_screen, view_token_claims_screen, token-balance backend, wallet-outcome unit tests); clippy clean. Co-Authored-By: Codex Sol <noreply@openai.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
…erflow The advanced-send screen coalesced duplicate Platform output/input rows by summing their amounts with plain arithmetic addition — a crafted or pasted set of duplicate rows near the Platform credit range's upper bound panicked the app on overflow (debug builds) or silently wrapped (release builds) instead of being rejected as invalid input. Switched aggregation to checked_add, returning the new typed TaskError::AdvancedPlatformOutputsOverflow / AdvancedPlatformInputsOverflow on overflow. Also added AdvancedPlatformAmountInvalid and AdvancedPlatformOutputAddressInvalid so the surrounding parse failures get dedicated typed variants instead of falling through to a generic error. Red-before/green-after confirmed (old code panicked on the overflow case; new code returns the typed error). ui::wallets::send_screen 14/14 pass; clippy clean. Co-Authored-By: Codex Sol <noreply@openai.com> <sub>🤖 Co-authored by [Claudius the Magnificent](https://github.com/lklimek/claudius) AI Agent</sub>
…g callers (grumpy-review)
… against overflow (grumpy-review)
…success token refreshes (grumpy-review) # Conflicts: # src/wallet_backend/mod.rs
… sticky sweep banner after DB clear (grumpy-review)
… seed encoding (grumpy-review)
Co-Authored-By: Codex GPT-5 <noreply@openai.com>
…s, Settings Closes the UX-003 gap found by the PR892 QA sweep and re-confirmed against current HEAD: the global nav switcher was wired into Wallets, Identity Hub, and Masternodes but not the other four root screens. Verified via Codex Sol (gpt-5.6-sol, high effort): clippy clean, 5 new kittest cases green. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Closes two gaps from the PR892 QA sweep, re-confirmed against current HEAD: SND-005 (no confirmation dialog before broadcasting a send, in either the simple or Advanced Options path) and SND-014 (fee-reserve/balance-too-low hint only rendered inside an unreachable validation branch). Verified via Codex Sol (gpt-5.6-sol, high effort): clippy clean, 80 wallet-screen tests + 9 amount-input tests green, including 3 new regression tests proven red before the fix. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Closes a QA-found gap from the PR860 delta sweep: the Token Creator screen's "Creating token..." banner never dismissed on successful token creation (leaked BannerHandle), same bug class as an earlier-fixed sticky-banner issue but not covered by that fix. Verified via Codex Sol (gpt-5.6-sol, high effort): clippy clean, new regression test proven red before the fix, green after. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Delta re-test against the 2026-07-14/15 175-story sweep: WAL-032 (new story, PASS), 34 baseline-FAIL stories re-verified (large majority now fixed), category-representative PASS spot-checks (DPN/DPY/MN/DEV — DEV-005 upgraded FAIL->PASS, all 8/8 Platform Info tools now working), plus TOK-011 (FIXED) and ALK-002 (still failing, unchanged from baseline). Session was resumed mid-run after an unrelated tooling kill; resumed cleanly from checkpoints. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
thepastaclaw
left a comment
thepastaclaw
left a comment
There was a problem hiding this comment.
Preliminary review — Codex only
Carried-forward prior findings: prior-5, prior-6, and prior-7 remain valid; prior-1 is outdated, while prior-2 through prior-4 are fixed. New findings in the latest delta: the token-refresh outcome inference is racy, and the simple Core-to-Platform confirmation misstates the recipient amount. Additional cumulative current-head inspection: the proposed Core aggregate-overflow blocker was dropped because its UI arithmetic predates the PR base and the claimed reachable failure was not established.
Validated blockers were found in the Codex precheck. Sonnet is deferred until a fresh Codex revalidation clears the blocker gate.
Review provenance
- Codex reviewers:
gpt-5.6-sol— general (failed_pre_evidence),gpt-5.6-sol— general (failed_pre_evidence),gpt-5.6-sol— general (failed_pre_evidence),gpt-5.6-sol— general (failed_pre_evidence),gpt-5.6-sol— general (failed_pre_evidence),gpt-5.6-sol— general (failed_pre_evidence),gpt-5.6-sol— general (failed_pre_evidence),gpt-5.6-sol— general (completed) - Verifier:
gpt-5.6-sol— verifier - Sonnet: not run (deferred by blocker gate)
🔴 2 blocking
3 carried-forward finding(s) already raised on this PR; not re-posting as new inline comments.
🤖 Prompt for all review comments with AI agents
These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.
In `src/wallet_backend/mod.rs`:
- [BLOCKING] src/wallet_backend/mod.rs:124-131: Token refresh ownership cannot be inferred from before/after status samples
Upstream `IdentitySyncManager::sync_now()` atomically claims `is_syncing` and returns no outcome when another pass owns it. The reads before and after that call are not tied to its ownership. If this call completes and the periodic loop claims the flag before the second sample, DET returns `AlreadyInFlight`, skips `refresh_token_balances()`, and surfaces `TokenBalanceRefreshSkipped` even though the requested pass completed. The DET snapshot therefore remains stale. Upstream must return an explicit performed/skipped outcome, or expose an atomic completion generation that DET can use instead of reconstructing ownership from independent samples.
In `src/ui/wallets/send_screen.rs`:
- [BLOCKING] src/ui/wallets/send_screen.rs:2988-2995: Simple confirmation hides that the fee reduces the recipient amount
For a Core-to-Platform send, `current_fee_preview()` returns `FeePreview::deducted_from_amount`, and the prepared backend task sets `fee_deduct_from_output: true`. The final confirmation nevertheless says the entered amount will be sent to the destination and ignores `recipient_receives_credits`. The recipient receives less than the amount the confirmation claims. Include the calculated recipient amount or explicitly state that the fee is deducted from the entered amount, matching the advanced confirmation.
|
Follow-up: the review feedback below that was still unaddressed at merge time has been triaged and fixed in #902 (13 distinct findings — 2 HIGH, 4 MEDIUM, 7 LOW). Each unresolved thread was independently re-verified against current code before being marked; several bot auto-resolutions here were incorrect (commit-diff heuristic, not semantic verification) and have been corrected. Threads now resolved reference this comment for context. 🤖 Co-authored by Claudius the Magnificent AI Agent |
…ad push branch (#898) * ci: run tests and clippy on every non-draft pull request The `pull_request` triggers for both gates were path-filtered, so a PR that touched no Rust source got no test, clippy, or fmt run at all — and reported green anyway. A skipped gate and a passing gate are indistinguishable at the merge button, which makes "CI is green" mean "green, or it never ran". Drop the `paths` filter from both `pull_request` triggers so every non-draft PR runs the full gate. Pushes to master / v*-dev stay path-filtered, where the filter is a pure cost saving and nothing gates on the result. Add `workflow_dispatch` to both. Draft PRs are suppressed by the existing `draft != true` guard and previously had no way to run CI at all; a manual run now covers them (against the branch head, not the PR merge commit). Fix the push trigger's dead `main` branch entry — this repo's release branch is `master`, so pushes to it matched nothing and ran no tests. Document the resulting local-vs-CI split in CLAUDE.md: CI owns the full sweep, so local runs should stay narrowly scoped to the change rather than duplicating what CI is about to do. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq * ci: allow manual runs, cover the toolchain, drop the dead push branch Address review feedback on the previous commit: keep the `paths` filters (documentation changes should not spend CI minutes) and take CLAUDE.md out of this PR entirely — it collides with PR #860, which rewrites the same section of that file, so it ships separately against #860's branch. Three changes remain, all narrow: - Add `workflow_dispatch` to both workflows. Draft PRs are suppressed by the existing `draft != true` guard and previously had no way to run CI at all — the only route to a green check was marking the PR ready for review. A manual run now covers them; it tests the branch head rather than the PR merge commit. - Add `rust-toolchain.toml` to the path filters. The file pins the toolchain for every build, but matched no filter, so a toolchain bump triggered neither tests nor clippy. `**/*.rs` already covers build.rs and all crate sources, so the filters otherwise fire on any Rust code change and on no documentation change, which is the intent. - Drop the push branch list down to `v*-dev`. It previously listed `main`, which does not exist in this repo, and `master` is release-only — neither needs a push-triggered run. The path lists are shared between the `push` and `pull_request` triggers via a YAML anchor so the two cannot drift apart; GitHub Actions has supported anchors since 2025-09-18, and path filters are a documented use case. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq * ci: run the toolchain the project actually pins Review feedback: the previous commit added rust-toolchain.toml to the path filters, but neither job used the file's compiler, so the trigger proved nothing. Both installed a toolchain by name with `override: true`, and a rustup directory override takes precedence over rust-toolchain.toml. Worse than a hollow trigger, the test job named `stable` while the project pins 1.92 — so the suite has never run on the pinned toolchain at all. Clippy named 1.92, matching only because the number was duplicated by hand; the next bump would have silently kept linting the old compiler. Install via actions-rust-lang/setup-rust-toolchain with no toolchain input, so both jobs take the channel and components straight from rust-toolchain.toml. That file becomes the single source of truth, and — since a change to it now triggers both workflows — a toolchain bump is tested and linted by the compiler it introduces. Clippy's components move into the file for the same reason. Two of the action's defaults are pinned rather than inherited: it defaults RUSTFLAGS to `-D warnings`, which would newly fail the test job on any warning (Clippy owns that gate, and its own step already passes the flag), and it defaults to its own cargo cache, which the existing actions/cache step already covers. Also drops actions-rs/toolchain from both jobs; the action is archived, and dtolnay/rust-toolchain would not fix this because it takes its toolchain from its @ref rather than reading rust-toolchain.toml. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_018m6b2TRP1e6S3uQHcaq3Cq --------- Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>
* fix(wallets): disclose deducted fees in confirmations Co-Authored-By: Claude GPT-5 <noreply@anthropic.com> * fix: harden identity deposit onboarding Reconcile deposit state from wallet snapshots, prevent duplicate address requests, and preserve retry controls on derivation failures. Co-Authored-By: Claude GPT-5 Codex <noreply@anthropic.com> * fix: harden app task and network guards Release scheduled-vote sweep guards after panicking task joins and require explicit network confirmation when legacy settings I/O cannot restore the saved network. Co-Authored-By: OpenAI Codex GPT-5 <noreply@openai.com> * docs(gui-testing): make scenario launch portable Co-Authored-By: Codex GPT-5 <noreply@openai.com> * docs(gui-testing): vendor GUI launch procedure Co-Authored-By: Codex GPT-5 <noreply@openai.com> * test(settings): pin legacy wire fixture Co-Authored-By: Codex GPT-5 <noreply@openai.com> * fix(migration): guard partial settings import Co-Authored-By: Codex GPT-5 <noreply@openai.com> * fix: harden wallet history hydration and soften token-sync banner Skip corrupt transaction-history rows instead of aborting wallet registration, and surface a skipped token-balance refresh as an info banner instead of a sticky error (the balance is very likely still fresh from the periodic sync loop that raced it). Co-Authored-By: OpenAI Codex GPT-5 <noreply@openai.com> * docs: log PR #860 follow-up fixes in changelog Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> * fix: address PR 902 review findings Addresses bbca6608394e, e9bdf38afeeb, SEC-001, SEC-002, SEC-004, SEC-005, SEC-007, PROJ-002, PROJ-003, PROJ-004, CODE-002, CODE-003, CODE-004, CODE-005, CODE-007, CODE-008, CODE-009, CODE-010, CODE-012, CODE-013, CODE-014, and CODE-015. Co-Authored-By: Claude GPT-5 <noreply@anthropic.com> * docs: correct changelog and gui-testing docs per PR 902 review (DOC-001..003) - CHANGELOG: add the missing token-balance-refresh-status entry (DOC-002). - docs/gui-testing/README.md + scenarios/TEMPLATE.md: resolve the test binary via `cargo metadata`'s target_directory instead of assuming `${CARGO_TARGET_DIR:-target}`, which silently breaks when the target dir is redirected via .cargo/config.toml rather than the env var (DOC-001). Also stop implying the desktop-gui automation skill is required (DOC-003). Co-Authored-By: Codex Sol <noreply@openai.com> --------- Co-authored-by: Lukasz Klimek <842586+lklimek@users.noreply.github.com> Co-authored-by: Claude GPT-5 <noreply@anthropic.com> Co-authored-by: OpenAI Codex GPT-5 <noreply@openai.com>
) Upstream landed a full rewrite (581 files, new platform-wallet/ wallet_backend crates, egui 0.33->0.35, dash-sdk rev bump, src/spv/ removed) since our original fork point. Rather than merge that through OrchardPay's ~101 mostly-mechanical rebrand-diff files, this re-forks from upstream's current v1.0-dev tip and reapplies: - The dash-evo-tool -> OrchardPay rebrand pass (crate name/path, env vars, app-dir paths, log filenames, Flatpak/macOS bundle IDs, user agent string) across every living file. Historical docs/ai-design/ QA-campaign records are deliberately left untouched — they document what upstream actually observed testing the real "Dash Evo Tool" at the time, and rewriting them would misrepresent history. Real upstream GitHub issue/repo references (e.g. dashpay/dash-evo-tool dashpay#660, dashpay#889) are likewise left pointing at the real upstream, not rewritten into a nonexistent dashpay/orchardpay repo. - The 5 ORCHARDPAY-TODO(dashpay-legacy) markers and LEGACY module doc comments flagging DashPay's contact/messaging code as superseded by OrchardPay's ZK-based model (docs/orchardpay/PROTOCOL_DESIGN.md, docs/ORCHARDPAY_MIGRATION.md), adapted to their new locations (the RootScreenType enum moved into src/model/settings.rs; the standalone Dashpay nav entry is now hidden by upstream itself). - The onboarding guided-setup stepper (src/ui/components/ onboarding_progress.rs, wired into src/app.rs), adapted to eframe 0.35's ui-based App::ui entry point and egui's unified Panel type (TopBottomPanel was replaced by Panel::top). Added a kittest regression test exercising the real stepper end-to-end. - Fixed a rebrand bug the new test caught: src/logging.rs's log rotation used bare "det"/"det-stderr" stems (without a .log suffix) that the original substitution pass missed, causing rotation to look for the wrong filenames. Verified: cargo build/clippy/test all green on the new base (1944 tests, 0 failures). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
3 participants
Goal
This PR started as a wallet-backend swap: move DET's SPV/wallet stack onto the upstream
platform-walletcrate (dashpay/platform), which owns chain sync, address derivation, identity/asset-lock handling, and the shielded coordinator. That rewrite is still the foundation, and the framing below still holds: DET's bespokesrc/spv/stack and the legacy RPC wallet mode are gone, chain sync and derivation come from upstream, and theBackendTaskaction/channel contract is preserved.Since it was opened, ~30 additional PRs (#861–#900, excluding #862) stacked onto this branch and merged. Together they turned this from a backend swap into a full rewrite of Dash Evo Tool against the new backend — the UI layer, the identity/masternode/DPNS/DashPay workflows, the secret-storage model, and the progressive-disclosure interface were all rebuilt, re-verified, and in several cases substantially extended, not just re-plumbed onto the new engine. This PR is now the merge point for that entire rewrite into
v1.0-dev.What shipped beyond the backend swap
Everyday < Power < Developerrole model (UserRole,Capability/FeatureGate::checks()) with a three-way "Interface mode" picker, so future features have a real place to attach beyond "developer or not."v0.10-dev— working Identity Hub Contacts (accept/decline/cancel/search/pay), a restored shielded receive-address view, settings/scheduled-vote/top-up-history migration on upgrade, interactive global nav pills, and a formal disclosure/closure record for the items intentionally not carried forward.wallet_backend/secret_seam.rs) for every wallet secret class, per-secret at-rest encryption (Argon2id + XChaCha20-Poly1305, fresh per-object salt, AAD-bound scope), per-secret independent passwords, and optional per-identity Tier-2 protection for standalone-imported identity keys.-— now derives the fee for outgoing transactions from their known inputs/outputs.docs/gui-testing/): standing rules and a reusable scenario library for driving the compiled app through a real display, used throughout this stack's live-verification passes.docs/user-stories.mdis the fullest account of the resulting feature surface, persona-tagged and marked[Implemented]/[Gap].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.default_withdrawal_key()) could pick a key that exists on the identity but whose private key isn't loaded locally — common on masternode/evonode identities where only the Owner key was supplied — failing signing with an unhelpful technical error. It now sources only from private-key-backed keys (Transfer-preferred, Owner-fallback).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.Transaction history
core_transactionsrows on load. It is now hydrated at wallet load, insert-if-absent by txid, before the first snapshot is published.-: discoveredTransactionRecords were always built withfee: None. Outgoing transaction fees are now derived from known inputs minus outputs, falling back to-(never a wrong number) on incomplete or overflowing data.estimate_platform_feealso now accounts for output count, so the advanced Platform→Platform preview no longer underestimates multi-output transfers.Shielded confirmation-safety
*ConfirmationUnknownerrors instead of a false success.Secret hygiene
Data-safety
Lifecycle / sync
Review-remediation fixes (this session)
A dedicated audit of every stacked sub-PR's review threads (formal comments, review bodies, and bot auto-resolutions — independently re-verified against current code rather than trusted at face value) surfaced 3 HIGH + 20 MEDIUM findings still present in the merged tree. Fixed and re-merged, grouped by theme:
Also: a committed testnet-only mnemonic and WIF were found and redacted from an internal QA doc (
WAL.md) — testnet material, not a live secret, removed per the project's no-committed-secrets policy.Second-pass adversarial review + full QA sweep (this session)
A follow-up multi-agent adversarial review (3 specialist reviewers + an independent Codex reviewer) of this session's own review-remediation diff surfaced 9 additional blocking findings, all fixed and merged:
Arc-backedOnceCell) so every caller sees the same real outcome.Followed by a full desktop-GUI QA sweep — a delta re-test against a prior comprehensive 175-user-story baseline, plus one net-new story (the storage-update/migration safety path, tested live against a real legacy-schema fixture). Confirmed the large majority of previously-broken stories now pass, and found 3 more issues during the walkthrough, all fixed the same way: the wallet/identity switcher was missing from 4 screens (Contracts, Tokens, Tools, Settings); the Send screen never asked for confirmation before broadcasting and its fee-reserve hint was unreachable; and a "Creating token…" progress banner stayed on screen indefinitely after the operation actually finished. Two known issues remain open, unchanged from before this session (an in-progress-deposit list that never populates, and per-key password protection being unreachable from the identity key list) — see
docs/user-stories.mdand the QA report for detail. One upstreamplatform-walletissue was found and triaged rather than blind-fixed in this repo: a background wallet-ID-mismatch error in identity-sync's token-balance-changeset flush, self-resolving, no observed user-visible impact.Full evidence-backed report (plain-language):
docs/ai-design/2026-07-16-pr860-qa-delta/summary-report.md.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 audit-round details live in the audits' 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.DEVELOPER_MODEin.envno longer selects the initial role.Known gaps
Updated from the original draft: the two items below marked (closed) were resolved by PRs folded into this branch since it opened.
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 — the in-branch bumps since the original draft (most recently to pick up thedash-evo-toolPR #3968 head) pick up upstream fixes within that same unreleased branch; none resolve 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. 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 fix under Bugs fixed → Balance display, which closes a DET-side bookkeeping gap and doesn't touch rehydration's gap-window derivation.Non-wallet data not migrated: app settings, top-up history, scheduled DPNS votes reset on upgrade.Closed by feat: v1.0 UI parity batch — Identity Hub contacts, shielded receive, migration fix, nav pills, disclosure closure #882's settings/vote/top-up migration on upgrade, further hardened this session (see Bugs fixed → Review-remediation fixes).Token "stop tracking" undone by a refresh.Closed by feat: v1.0 UI parity batch — Identity Hub contacts, shielded receive, migration fix, nav pills, disclosure closure #882.SingleKeyWalletsUnsupported, pending upstream watch-only registration support (register_walletis private; public constructors require an HD seed). Import / data-loss guard / password-restore are done; confirmed still blocked during the feat: v1.0 UI parity batch — Identity Hub contacts, shielded receive, migration fix, nav pills, disclosure closure #882 parity audit.docs/user-stories.mdUX-004).docs/user-stories.mdIDH-005).identity-hub-activity-feedCargo feature (docs/user-stories.mdIDH-006).docs/user-stories.mdWAL-030).docs/user-stories.mdDPY-010).KeyInfoScreen) has no live UI entry point yet.docs/ai-design/are historical record, not user-facing docs — left as-is, flagged for a possible future trim, and have grown further with this stack's own design docs. Merge-time action: confirm the dependency-pin re-check above before ship.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/.det-cli(identity load + owner-key withdrawal, confirmed viaplatform-withdrawals-get) and the running GUI app (real click-through withdrawal, confirmed via the app's own broadcast-completion log).docs/ai-design/2026-07-16-pr860-qa-delta/summary-report.md.Attribution
🤖 Co-authored by Claudius the Magnificent AI Agent