feat: default to SPV connection; gate Dash Core RPC behind Expert mode#836
Merged
Conversation
SPV becomes the out-of-the-box connection for every user. Dash Core RPC becomes an opt-in Expert-mode configuration for power users who actually run a local Dash Core node. Behavior changes: - `CoreBackendMode::default() == Spv`; `From<u8>` falls back to Spv (consistent with the DB column default). Runtime default in `ConnectionStatus::new()` flipped to match. - `AppContext::new()` no longer silently force-flips SPV→RPC when developer mode is off. The saved `core_backend_mode` is the single source of truth; developer mode is a visibility toggle only. - `NetworkChooserScreen`: removed the "SPV is experimental" warning, renamed connection options to "Built-in (SPV)" / "Local Dash Core node", updated the Expert-mode tooltip, and removed the legacy force-RPC-when-Expert-off loop. - `try_auto_start_spv` no longer requires developer mode; `spawn_zmq_listener` is now gated through `FeatureGate::RpcBackend` so SPV mode doesn't burn a ZMQ socket retry loop for a local node that isn't there. - New `FeatureGate::RpcBackend` variant centralises the RPC-mode predicate so callers don't reach for raw `core_backend_mode() == Rpc` comparisons. Migration (v33 → v34): - Users with `developer_mode=true` AND at least one network's `core_rpc_password` set keep their existing backend mode. - Everyone else is pinned to SPV. - The migration parses `.env` directly (not via `Config::load_from`) to avoid mutating process environment during DB init — important for test parallelism and for not leaking one-shot migration state into the live process. Tests: - `CoreBackendMode` roundtrip test updated for the new default. - Five new v34 migration unit tests covering all four migration scenarios plus re-run idempotence. - New startup integration test asserting a fresh DB resolves to SPV. Removes three "once SPV is production-ready" TODOs: - `src/ui/network_chooser_screen.rs` connection combobox gate - `src/ui/network_chooser_screen.rs` force-RPC-on-Expert-off loop - `src/app.rs` `try_auto_start_spv` developer-mode guard Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ings Round of QA fix-ups on top of 815eb35 (SPV-default flip). - **ui/network_chooser_screen.rs (Adams PROJ-001, MEDIUM):** the SPV sync progress widget was previously hidden unless `developer_mode` was ON. After the SPV-default flip, the default cohort is exactly the non-Expert users, so hiding their sync feedback is a real regression — they'd see a silent minutes-long sync with no indication. Drop the `developer_mode` gate, replace the raw `core_backend_mode == Spv` check with `FeatureGate::SpvBackend.is_available(...)` to satisfy the Phase 5 DRY rule on feature gating, and kill the stale TODO comment. - **database/initialization.rs (Marvin QA-001, MEDIUM):** the `test_v33_migration_fresh_install` test asserted DB version but not that `core_backend_mode` landed on SPV (1). It does now — this is the user-visible contract of the v34 default-flip and deserves an explicit assertion, not an implicit one tucked into schema checks. - **database/initialization.rs (Marvin QA-002, LOW):** replace the naive `trim_matches('"' | '\'')` .env value strip with a small `parse_env_value` helper that mirrors dotenvy's quote + inline-comment handling: * `KEY=value # comment` → `value` * `KEY="value # with hash"` / `KEY='value # with hash'` → preserved * `KEY=value#nohash` → `value#nohash` (matches dotenvy — `#` only starts a comment when preceded by whitespace outside quotes) Adds three unit tests covering each case. Backslash escapes are intentionally not interpreted — the migration only cares about a bool flag and whether a password is non-empty. - **ui/network_chooser_screen.rs (Adams PROJ-001 / Marvin, LOW):** the SPV Peer Source and SPV Maintenance sections are legitimately Expert-only (advanced configuration + destructive diagnostics). Rewrite the stale `TODO: Remove once SPV is production-ready` comments to explain *why* these are gated rather than implying they should be ungated. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Add NET-015 (Alex persona) covering zero-config SPV connection as the new default, now that SPV is the default connection mechanism and Dash Core RPC is gated behind Expert mode. No CHANGELOG.md exists in this repository; the CHANGELOG entry is noted in the commit message only. See the plan at .claude/plans/clever-booping-lemon.md for the full entry text. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Contributor
|
Warning Rate limit exceeded
Your organization is not enrolled in usage-based pricing. Contact your admin to enable usage-based pricing to continue reviews beyond the rate limit, or try again in 30 minutes and 4 seconds. ⌛ How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. 🚦 How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
📝 WalkthroughWalkthroughThis pull request changes the default backend from RPC to SPV (Simple Payment Verification) across the application. It includes database schema migration logic to preserve existing RPC configurations while defaulting fresh installs to SPV, updates core initialization and context to reflect SPV as the primary backend, adds RPC feature gating, and refactors UI terminology and defaults accordingly. Changes
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Possibly related issues
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
Contributor
There was a problem hiding this comment.
Pull request overview
Shifts Dash Evo Tool’s default connection from Dash Core RPC to the built-in SPV light client, while keeping RPC available behind the Expert mode UI and preserving existing RPC users via a v34 DB migration.
Changes:
- Flip
CoreBackendModedefaults/fallbacks to SPV and update startup/runtime codepaths to treat persistedsettings.core_backend_modeas the source of truth. - Add
FeatureGate::RpcBackendand use feature gates to avoid RPC/ZMQ initialization work when running in SPV mode. - Add a v34 DB migration that pins most users to SPV while preserving RPC for Expert users with a configured RPC password (via
.envparsing).
Reviewed changes
Copilot reviewed 10 out of 10 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ui/network_chooser_screen.rs | Defaults UI state to SPV, gates SPV progress display via FeatureGate, and stops Expert toggle from mutating backend mode. |
| src/spv/tests.rs | Updates roundtrip tests to match SPV default/fallback behavior. |
| src/spv/manager.rs | Changes CoreBackendMode default + From<u8> fallback to SPV and documents the invariants. |
| src/model/feature_gate.rs | Adds RpcBackend gate and centralizes backend predicates. |
| src/mcp/server.rs | Forces SPV backend in headless mode via a volatile override. |
| src/database/initialization.rs | Bumps schema version to v34 and adds one-shot migration driven by direct .env parsing + new tests. |
| src/context/mod.rs | Stops forcing RPC when Expert mode is off; defaults to SPV when settings are absent/unreadable; adds a fresh-DB assertion test. |
| src/context/connection_status.rs | Changes initial backend_mode atomic default to SPV. |
| src/app.rs | Gates ZMQ listener spawn behind FeatureGate::RpcBackend; allows SPV autostart without Expert mode. |
| docs/user-stories.md | Adds NET-015 user story documenting “no local node required” behavior. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Retires the custom line-by-line .env parser and the 3 `parse_env_value` unit tests that tested it. dotenvy now handles all .env edge cases (escapes, variable substitution, multi-line quoted values) correctly and consistently with the main Config::load_from() code path. The non-mutating `from_path_iter` iterator is safe inside the migration transaction and for parallel test runs — unlike Config::load_from() / dotenvy::from_path_override which pollute the process environment. Strictly shorter (-105 lines) and more correct. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
- CMT-1 (docs/user-stories.md): Reword NET-015 acceptance criteria to reflect what this PR actually ships. The default everyday-user UI avoids SPV/RPC/node jargon, while technical terminology may still appear in Expert mode or advanced settings. - CMT-2 (src/spv/manager.rs): Add #[repr(u8)] to CoreBackendMode so its u8 persistence guarantee is explicit, matching the existing SpvStatus enum below it.
Collaborator
Review GateCommit:
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/ui/network_chooser_screen.rs (1)
524-527:⚠️ Potential issue | 🟡 MinorInconsistent fallback default — still
Rpchere while sibling sites were flipped toSpv.Lines 260 and 415 were updated to
.or_insert(CoreBackendMode::Spv)to match the SPV-default flip, but this third occurrence on line 527 in the Connection Status card still insertsRpc. Today this is effectively dead code becauserender_network_tableruns the Connection Settings grid first and always inserts an entry forself.current_network, but it's a footgun: any future refactor that renders the status card independently (or for a different network) would silently fall back to RPC and desync from the actualAppContext::core_backend_mode(). Align it with the other two sites.🔧 Proposed fix
let current_backend_mode = *self .backend_modes .entry(self.current_network) - .or_insert(CoreBackendMode::Rpc); + .or_insert(CoreBackendMode::Spv);Better yet, seed this once from
ctx.core_backend_mode()rather than hard-coding a default in three places.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/network_chooser_screen.rs` around lines 524 - 527, The fallback default for backend_modes when accessing self.backend_modes.entry(self.current_network).or_insert(...) is inconsistent (currently CoreBackendMode::Rpc) and should match the other occurrences; change the insertion to use the global seed instead of hard-coding Rpc — either replace CoreBackendMode::Rpc with CoreBackendMode::Spv or, preferably, call ctx.core_backend_mode() when inserting so backend_modes is seeded from the actual AppContext value; update the entry call in the Connection Status card code (the backend_modes / current_network usage) to use ctx.core_backend_mode() to keep behavior consistent with render_network_table.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@src/app.rs`:
- Around line 725-730: When toggling backend modes the code must tear down or
recreate the ZMQ listeners stored in zmq_listeners so they don't persist across
mode switches; update set_core_backend_mode_inner() to, when
FeatureGate::RpcBackend.is_available(ctx) becomes false, iterate and gracefully
stop/close all entries in zmq_listeners (cancel retry loops, close sockets, join
tasks) and clear the collection, and when RpcBackend becomes available again
ensure any missing listeners are spawned (re-use the startup/network-switch
listener creation logic or call the same spawn function used at boot) so that
switching to SPV drops active listeners and switching back to RPC recreates
them; coordinate this with start_spv()/stop_spv() so SPV status and listener
lifecycle are managed together and avoid double-spawning or leaked sockets.
In `@src/database/initialization.rs`:
- Around line 57-64: The suffix check in the migration loop currently uses
key.ends_with("core_rpc_password") which is case-sensitive while DEVELOPER_MODE
uses eq_ignore_ascii_case; change the suffix check to a case-insensitive form
(e.g., normalize key with to_ascii_lowercase().ends_with("core_rpc_password") or
otherwise compare ignoring ASCII case) in the loop that contains
key.eq_ignore_ascii_case("DEVELOPER_MODE") and has_any_rpc_password handling,
and add a unit test variant (alongside
v34_preserves_mode_when_local_core_configured) named something like
v34_preserves_mode_with_uppercase_password_key that writes an
uppercase/mixed-case key (e.g., MAINNET_CORE_RPC_PASSWORD) into the temp .env,
runs try_perform_migration(33, 34, Some(tmp.path())), and asserts the core
backend mode is preserved.
---
Outside diff comments:
In `@src/ui/network_chooser_screen.rs`:
- Around line 524-527: The fallback default for backend_modes when accessing
self.backend_modes.entry(self.current_network).or_insert(...) is inconsistent
(currently CoreBackendMode::Rpc) and should match the other occurrences; change
the insertion to use the global seed instead of hard-coding Rpc — either replace
CoreBackendMode::Rpc with CoreBackendMode::Spv or, preferably, call
ctx.core_backend_mode() when inserting so backend_modes is seeded from the
actual AppContext value; update the entry call in the Connection Status card
code (the backend_modes / current_network usage) to use ctx.core_backend_mode()
to keep behavior consistent with render_network_table.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 17275b62-0711-4cb8-8f9b-c9f3ab617e50
📒 Files selected for processing (10)
docs/user-stories.mdsrc/app.rssrc/context/connection_status.rssrc/context/mod.rssrc/database/initialization.rssrc/mcp/server.rssrc/model/feature_gate.rssrc/spv/manager.rssrc/spv/tests.rssrc/ui/network_chooser_screen.rs
10 tasks
CR-2 — make core_rpc_password suffix match case-insensitive in the v34 migration, matching DEVELOPER_MODE's treatment. Uppercase keys (MAINNET_CORE_RPC_PASSWORD) would have silently been ignored, flipping the user to SPV despite a configured local Core node. Adds v34_preserves_mode_with_uppercase_password_key test. CR-3 — flip the third or_insert(CoreBackendMode::Rpc) site in network_chooser_screen.rs to Spv, matching the other two already-flipped sites. Dead-code footgun removal. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Contributor
Author
|
On CodeRabbit's outside-diff finding about the third 🤖 Co-authored by Claudius the Magnificent AI Agent |
Add a TODO in set_core_backend_mode_inner flagging that mid-session backend-mode switches do not touch the ZMQ listener lifecycle: an RPC → SPV switch leaves any existing listener running against an unused Core node (pre-existing bug predating the SPV-default flip), and an SPV → RPC switch never spawns a listener, so Expert users toggling to Local Dash Core node lose real-time ZMQ events until restart or network switch (the FeatureGate::RpcBackend gate in spawn_zmq_listener blocks the startup spawn while in SPV mode). Fixing this needs an AppContext → AppState signal to manage the listener map owned by AppState — a cross-module refactor deliberately scoped out of PR #836. Ref: CR-1 on #836. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
lklimek
added a commit
that referenced
this pull request
Apr 23, 2026
* feat(error): add OperationRequiresDashCore task error variant
Introduces a typed TaskError for backend operations that cannot run
when the app is connected via SPV. The variant carries the static
operation name so the user-facing message is specific without any
string concatenation, and the `#[error(...)]` template keeps the
Display/Debug separation consistent with the rest of TaskError.
Used by follow-up commits that gate single-key wallet refresh on
Dash Core availability and will also back future UI "disable in SPV"
affordances.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(shielded): route asset-lock broadcast through backend-mode-aware helper
The `shield_from_asset_lock()` bundle was reaching directly for
`app_context.core_client.read()?.send_raw_transaction(...)`, which
would unconditionally hit Dash Core even when the user had selected
SPV mode. In SPV the `core_client` is still constructed for
legacy/devtool reasons but there is no expectation that it can reach
a live `dashd` — the broadcast failed with `ConnectionRefused`.
Switch to the shared `AppContext::broadcast_raw_transaction()`
dispatcher (used by `create_registration_asset_lock`, `send_wallet_
payment`, `broadcast_and_commit_asset_lock`, etc.). It picks the
right path per `core_backend_mode()` — Core RPC or
`SpvManager::broadcast_transaction` — and keeps the finality tracker
leak-free by cleaning the waiting entry when the broadcast itself
fails.
Addresses the shielded half of the backend-E2E failures flagged for
`tc_014_wallet_platform_lifecycle` and `tc_018_fund_platform_address
_from_asset_lock` (`ConfirmationTimeout` in SPV mode triggered by the
RPC path never actually broadcasting in SPV).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(wallet-spv): broadcast single-key payments via backend-mode helper
`send_single_key_wallet_payment()` built and signed the transaction
locally but then dispatched the broadcast through
`core_client.send_raw_transaction()` without checking
`core_backend_mode()`. In SPV mode that blew up with
`ConnectionRefused` from the unused Core RPC socket instead of going
out on the wire via `SpvManager::broadcast_transaction`.
Replace the direct RPC call with the shared
`AppContext::broadcast_raw_transaction()` dispatcher, which routes
per backend mode. The rest of the function — UTXO selection and
signing from the wallet's in-memory cache — is unchanged and works
in both modes; the caller is now responsible for populating those
UTXOs (see `refresh_single_key_wallet_info`, still Core-RPC only).
Addresses the broadcast half of `core_tasks::tc_009_send_single_key_
wallet_payment`. The UTXO-refresh prerequisite is handled in a
follow-up commit.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(wallet-spv): surface typed error when refreshing single-key wallet in SPV
`refresh_single_key_wallet_info()` relies exclusively on Core RPC
(`import_address`, `list_unspent`) to discover UTXOs for a
standalone WIF/private-key wallet. The SPV subsystem only tracks
addresses derived from HD-wallet account paths registered with its
bloom filter, so there is no SPV-side equivalent for an imported
single-key wallet.
Previously calling the task in SPV mode hit the unused Core RPC
socket and reported `ConnectionRefused` via a raw
`CoreRpcConnectionFailed` banner. Guard the task with an
early-return that produces the new
`TaskError::OperationRequiresDashCore` variant, so the UI shows an
actionable "connect to Dash Core and retry" message instead of a
connection-level error the user cannot map to the actual cause.
Update `core_tasks::tc_003_refresh_single_key_wallet_info` and the
`tc_009` pre-flight refresh to assert the mode-dependent outcome:
typed `OperationRequiresDashCore` in SPV, normal `RefreshedWallet`
in RPC.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(wallet-spv): short-circuit asset lock recovery in SPV mode
`recover_asset_locks()` rescans the Core wallet's transaction and
UTXO indices for asset-lock payloads that weren't tracked locally.
In SPV mode none of the RPC queries it needs (`list_unspent`,
`get_raw_transaction`, `get_raw_transaction_info`) are available,
so the whole pass collapsed into a `ConnectionRefused` failure.
In SPV the reconciliation that this pass performs on RPC is handled
automatically by the InstantLock/ChainLock event stream — whenever
an asset-lock transaction arrives, `received_asset_lock_finality()`
populates `unused_asset_locks` and the DB row directly, so a
standalone "recover" pass has nothing to do. Return an empty
`RecoveredAssetLocks { 0, 0 }` result in SPV mode instead of
failing.
Addresses `core_tasks::tc_006_recover_asset_locks`, which expects
a `RecoveredAssetLocks` response in both backends (0 recoveries is
documented as a valid outcome in the test).
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(ui-spv): disable MasternodeListDiff dev tool when not in RPC mode
The MasternodeListDiff inspector issues ~16 direct Dash Core RPC calls
(see audit section C-1 and `src/backend_task/mnlist.rs:81`). Running it
while the app is on its built-in SPV backend produces a storm of
connection-refused errors, so dim the entry in the Tools side panel and
show a tooltip explaining the constraint instead.
The check is a plain `core_backend_mode() == CoreBackendMode::Rpc`
comparison per the dim-the-menu design chosen for v1.0-dev; the
`FeatureGate::RpcBackend` predicate lands separately with #836.
Complements backend commits 8975d5d / 7e74e69 / 4ee1a5a / b29f416
which surface typed "requires Core" errors for wallet flows.
* fix(ui-spv): grey out single-key wallet actions in SPV mode
Per audit section C-2: single-key wallets are not in the SPV bloom
filter (no `Account` entry to feed addresses from), so `refresh` and
`send` fail even though broadcast itself can now route via
`broadcast_raw_transaction`. Commit 4ee1a5a already surfaces a typed
`OperationRequiresDashCore` error — this commit adds the matching UI
preventive step so users cannot reach that error from the GUI.
Changes:
* In the single-key wallet detail panel, grey out the Send button with
a disabled-hover tooltip and surface an in-panel warning label
explaining the constraint. Receive stays enabled — it only displays
the cached local address.
* On the single-key send screen, prepend an info banner and disable the
Send action with the same tooltip. Guards against users already sitting
on the send screen when they switch the backend mode mid-session.
Raw `core_backend_mode() == CoreBackendMode::Rpc` comparisons per the
v1.0-dev pattern; `FeatureGate::RpcBackend` lands separately with #836.
Complements backend commits 8975d5d / 7e74e69 / 4ee1a5a / b29f416.
* fix(spv): address review comments on single-key wallet SPV gating
- Consolidate duplicate SINGLE_KEY_REQUIRES_CORE copy between single_key_view
and single_key_send_screen.
- Render the SPV-mode warning through MessageBanner instead of inline RichText /
Frame styling (lklimek review feedback).
- Drop the dead CoreBackendMode::Rpc arm in backend-e2e tc_009 — backend-e2e is
SPV-only, so only the typed-error path is exercised.
- Gate explicit button text color on enabled state so egui's disabled visuals
apply correctly (Copilot review).
- Log a warning and recover through the poisoned guard when the
transactions_waiting_for_finality mutex is poisoned, so finality tracking
entries cannot leak silently across retries.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* test(backend-e2e): restore tc_009 send flow as commented-out future work
The removed Rpc-arm assertions were the shape of the full single-key send
test. Keep them commented out next to the SPV typed-error check so the
intent and the assertion shape survive the SPV-only gap — when single-key
wallets gain SPV parity, un-comment the block.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
* fix(ui-spv): store single-key SPV warning banner to stop per-frame log spam
The persistent SPV-mode warning on the single-key wallet detail view and
the single-key send screen was constructed as a fresh `MessageBanner`
every frame. Each fresh `BannerState` starts with `logged: false`, so
`process_banner()` emits a `tracing::warn!` once and then flips the
local flag — but the local is dropped at end of frame and the next frame
repeats the whole dance. Result: a banner warn per repaint (~60/sec when
egui is active) for the entire time the screen is visible on SPV.
Store the banner as a screen-struct field instead, populate it once per
SPV-mode entry (guarded by `has_message()`), clear it on mode change,
and disable auto-dismiss so the state notice persists without needing
the fresh-each-frame hack. Adds a small `MessageBanner::disable_auto_dismiss()`
helper since the component previously only exposed an overriding setter.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(shielded): block on finality tracker cleanup after asset-lock timeout
On the asset-lock timeout path, `try_lock()` could return `WouldBlock`
and silently skip cleanup of `transactions_waiting_for_finality`. That
leaves a stale entry behind for a tx this flow has already abandoned,
so the finality listener keeps servicing it across retries/runs.
Switch to blocking `lock()` with the same poisoned-guard handling the
broadcast-failure branch already uses. The critical section is a single
`BTreeMap::remove`, so holding the std::sync Mutex across the await
boundary is bounded and matches the surrounding pattern.
Addresses copilot review on PR #837.
Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
* fix(spv): address iteration-2 review findings on PR #837
- Narrow single_key_view module scope: re-export SINGLE_KEY_REQUIRES_CORE
via pub(crate) use instead of publicising the whole submodule.
- Replace TaskError::OperationRequiresDashCore Display fragment with a
complete sentence; keep `operation` as a Debug-only field so log
output still identifies the originating callsite.
- Reword the single-key SPV banner to be action-specific
(sending/refresh need Core; Receive still works) per coderabbit +
copilot-pull-request-reviewer feedback.
---------
Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
9 tasks
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
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.
User Story
Imagine you are a new user installing Dash Evo Tool for the first time. You want to check your balance, send a payment, or register a DPNS name — but you don't run a full Dash Core node, don't know what RPC is, and don't want to install
dashd.Today, Dash Evo Tool would silently require you to have a local Dash Core node running and complain when it can't connect. After this PR, it just works: the built-in SPV light client connects you to the Dash network directly. No node, no port forwarding, no RPC credentials. For experienced users who do run a local node, Expert mode still exposes the Dash Core RPC option.
Summary
Flips the default connection axis in Dash Evo Tool:
developer_mode=true+ acore_rpc_passwordin.env). Everyone else is migrated to SPV.Key code changes
CoreBackendMode::default()flipped fromRpctoSpv;From<u8>unknown-byte fallback flipped accordingly.AppContext::new()no longer silently force-flips SPV → RPC when developer mode is off.FeatureGate::RpcBackendvariant so connection-mode predicates go through the central registry (Phase 5 DRY rule).spawn_zmq_listenergated onFeatureGate::RpcBackend— no burning a socket + retry loop for SPV-only users.try_auto_start_spvno longer requires developer mode.initialization.rs. Reads.envviadotenvy::from_path_iter(the non-mutating iterator API) to avoid polluting process environment during DB init — important for test parallelism and one-shot migration hygiene.Config::load_from()/dotenvy::from_path_overridewould have set_var'd every key and raced across parallel migration tests.Follow-up TODOs (filed as deferred work)
ctx.is_developer_mode()call sites across tokens, wallets, identities, etc. toFeatureGate::DeveloperMode. Pure refactor.core_backend_mode() == …comparisons (backend_task/core/,context/connection_status.rs, etc.) toFeatureGate::RpcBackend/SpvBackend. Leave genuine two-branchmatchdispatches alone.developer_modeinto two orthogonal flags: one for the node-operator case, one for signing/security overrides.Test plan
cargo build --all-featurescargo clippy --all-features --all-targets -- -D warningscargo +nightly fmt --all -- --checkcargo test --all-features --workspace— 461 lib + 10 e2e + 72 kittest + 3 doctest; 0 failed, 2 ignored (testnet-bound)developer_mode=false, flip-to-SPV when Expert but no RPC password set, flip-to-SPV when.envmissing, idempotence on re-runcore_backend_mode == Spvpost-initcore_rpc_passwordset): mode preserved after migration; app connects via RPC as beforeRollback
Revert as a single-PR revert. Existing v34 databases do not auto-downgrade — users needing full rollback must restore the backup (timestamped backups are created automatically by the migration framework before each version bump).
Migration
Runs once inside the existing DB migration framework (v33 → v34, inside a transaction, with automatic DB backup). Rules:
No new tracking variables; the DB schema version bump is the one-shot gate.
🤖 Co-authored by Claudius the Magnificent AI Agent
Summary by CodeRabbit
Release Notes
New Features
Documentation
Tests