feat: per-wallet Core RPC routing with auto-detection and selection dialog#695
Merged
Conversation
When Dash Core has multiple wallets loaded, wallet-specific RPC calls fail with error -19. This change detects that error, enumerates loaded wallets via `listwallets`, auto-selects when only one is present, and shows a SelectionDialog when multiple wallets exist. The choice is persisted as `core_wallet_name` in the .env config. - Add reusable SelectionDialog component (generalization of ConfirmationDialog) - Add `core_wallet_name` field to NetworkConfig with .env persistence - Add `core_rpc_url()` helper that appends `/wallet/<name>` when configured - Add `set_core_wallet_name()` method on AppContext to persist and reinit - Intercept error -19 in core task RPC calls with auto-select/dialog flow - Render wallet selection dialog as foreground overlay in AppState - Document `core_wallet_name` in .env.example for all networks - Add 11 manual test scenarios Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
📝 WalkthroughWalkthroughAdds multi-wallet Dash Core RPC support: detects RPC error -19 (wallet not specified), attempts auto-detection by address, persists per-wallet Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant UI as Wallets Screen
participant Backend as Backend Task
participant Core as Dash Core RPC
User->>UI: Trigger operation (e.g., Refresh / Send)
UI->>Backend: Execute backend task
Backend->>Core: RPC call (no rpcwallet)
Core-->>Backend: RpcError code -19 (Wallet not specified)
Backend->>Backend: Map to TaskError::CoreWalletNotConfigured
Backend->>Backend: Extract seed_hash + first address
Backend->>Core: listwallets()
Core-->>Backend: [wallet_list]
alt Single wallet or unique owner detected
Backend->>Backend: Persist core_wallet_name
Backend-->>UI: Return MustRetry (retry signal)
UI->>Backend: Retry task with wallet context
Backend->>Core: RPC call with rpcwallet param
Core-->>Backend: Success
Backend-->>UI: Result
else Multiple/ambiguous matches
Backend-->>UI: Propagate CoreWalletNotConfigured
UI->>UI: display_task_error -> try auto-detect
UI->>User: Show SelectionDialog (options)
User->>UI: Select wallet or cancel
UI->>Backend: apply_core_wallet_selection (persist)
Backend-->>UI: Success / Error
UI->>Backend: Retry or abort
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~50 minutes Possibly related issues
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 |
Two bugs in the wallet selection dialog: 1. Buttons and ComboBox were not clickable — only Escape worked. Added a full-screen input-blocking Area at Order::Middle that consumes pointer events before they reach screens underneath. Changed overlay paint order from Background to Middle. 2. After canceling the dialog, the wallet screen spinner stayed forever. Added display_message() call when CoreWalletSelectionNeeded is received, so the screen's progress banner gets dismissed. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The dialog Window was rendering at default order (Middle), same as its dark overlay, causing the entire screen to appear greyed out with no visible dialog. Set Window order to Foreground so it renders above the overlay layer. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Adds multi-wallet Dash Core RPC support by persisting a selected Core wallet name in config and presenting a reusable UI selection dialog when Core returns the “wallet file not specified” (-19) error.
Changes:
- Introduces a reusable
SelectionDialogcomponent for choosing from multiple options. - Adds
core_wallet_nametoNetworkConfigwith.envpersistence and acore_rpc_url()helper that appends/wallet/<name>. - Intercepts Core RPC “wallet not specified” failures to either auto-select the single loaded wallet or request UI-driven wallet selection.
Reviewed changes
Copilot reviewed 11 out of 11 changed files in this pull request and generated 6 comments.
Show a summary per file
| File | Description |
|---|---|
| src/ui/components/styled.rs | Re-exports SelectionDialog types for convenient UI use. |
| src/ui/components/selection_dialog.rs | New modal selection dialog component (ComboBox + confirm/cancel). |
| src/ui/components/mod.rs | Registers the new selection_dialog module. |
| src/spv/tests.rs | Updates test NetworkConfig initializer with core_wallet_name. |
| src/context/mod.rs | Adds core_rpc_url() and set_core_wallet_name(); uses core_rpc_url() for client init/reinit. |
| src/config.rs | Adds core_wallet_name to config and persists it to .env; adds tests for parsing. |
| src/backend_task/mod.rs | Adds CoreWalletSelectionNeeded(Vec<String>) task success result. |
| src/backend_task/core/mod.rs | Detects “wallet not specified” error and calls listwallets to drive selection flow. |
| src/app.rs | Shows selection dialog on CoreWalletSelectionNeeded and persists the chosen wallet name. |
| docs/ai-design/2026-03-04-multi-wallet-rpc/manual-test-scenarios.md | Adds manual test plan for multi-wallet scenarios. |
| .env.example | Documents *_core_wallet_name setting for each network. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Replace global core_wallet_name (.env config) with per-wallet storage in SQLite. Each DET wallet now independently maps to a Core wallet, fixing multi-wallet RPC (-19) errors when different wallets need different Core wallet targets. - Revert global NetworkConfig.core_wallet_name approach - Add DB migration v28: core_wallet_name column on wallet tables - Add core_client_for_wallet() with path traversal validation - Per-wallet error-19 detection, auto-assign (single) or prompt (multiple) - Two-phase wallet creation/import UI with SelectionDialog - App-level runtime recovery handler for CoreWalletSelectionNeeded - Update manual test scenarios for per-wallet behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…n/import The SelectionDialog modal overlay in wallet creation/import screens was not clickable due to rendering conflicts with the screen layout. Replace with an inline ComboBox that appears in the form when multiple Core wallets are detected. The runtime recovery modal in app.rs (for legacy wallets hitting error -19) is unchanged. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
When a DET wallet triggers RPC error -19, try getaddressinfo on each loaded Core wallet before showing the SelectionDialog. If exactly one Core wallet recognizes the address (is_mine or is_watchonly), auto-assign it and persist to SQLite. Falls back to manual selection only when detection is ambiguous. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add MWT-016/017/018 for auto-detection success, ambiguous fallback, and fresh wallet scenarios. Update existing scenarios to reflect the getaddressinfo-based auto-detection step before SelectionDialog fallback. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
display_message() only cleared `refreshing` for Error/Warning types, leaving the spinner running when CoreWalletSelectionNeeded sent Info. Also implement refresh() to call refresh_on_arrival() so post-selection and auto-detection paths properly re-fetch wallet data. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…tion Address review findings from grumpy-review triage: - Add check_wallet_not_specified() recovery to RefreshSingleKeyWalletInfo, SendWalletPayment, SendSingleKeyWalletPayment, and RecoverAssetLocks - Extend apply_core_wallet_selection() and auto-detection to handle single-key wallets (persist via set_single_key_wallet_core_wallet_name) - Change CoreWalletSelectionNeeded.wallet_seed_hash from String to [u8; 32] eliminating hex encode/decode round-trip - Extract core_wallet_first_address() helper to deduplicate 3x pattern - Guard against concurrent dialog overwrites in CoreWalletSelectionNeeded - Switch core_client_for_wallet() from denylist to allowlist validation - Remove redundant outer blocker overlay in SelectionDialog rendering - Fix test scenario button labels (Confirm → Select) and error codes Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
The full-screen blocker Area was at Order::Foreground, competing with the SelectionDialog Window for input. Move it to Order::Middle so it blocks clicks to the background UI while letting the Foreground dialog receive all input. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Add TaskError::CoreWalletNotConfigured variant with actionable message directing users to the Wallets screen. The From<String> impl now parses known error patterns into typed variants automatically, catching error -19 from any backend task path (identity registration, top-up, wallet funding) without modifying individual task files. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…reen Let RPC error -19 flow through the normal TaskError path instead of intercepting it as a fake success result. Only the wallets screen catches CoreWalletNotConfigured to show a SelectionDialog; other screens display a user-friendly error banner. - Simplify check_wallet_not_specified() to return TaskError directly - Remove CoreWalletSelectionNeeded variant from BackendTaskSuccessResult - Add display_task_error() to ScreenLike trait for typed error dispatch - Add SelectionDialog::show_modal() convenience method - Add MessageBanner::set_global_error() helper - Move dialog state and apply_core_wallet_selection() to WalletsBalancesScreen - Update manual test scenarios for new behavior Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
display_task_error() now returns bool — true means the screen handled the error and AppState should skip the default error banner. Fixes the wallets screen showing a "go to Wallets screen" banner instead of the Core wallet selection dialog. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…Configured run_core_task() now returns Result<..., TaskError> instead of Result<..., String>. Previously, CoreWalletNotConfigured was converted to a user-friendly String, which From<String> could not reconstruct back into the typed variant — so display_task_error() on the wallets screen never matched it and the generic error banner appeared instead of the selection dialog. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
UI screens making direct core_client RPC calls (identity QR code,
top-up QR code, create asset lock) bypassed the backend task system
entirely, showing raw "JSON-RPC error: RpcError { code: -19, ... }"
to users.
Add TaskError::user_message() helper that converts raw error strings
to user-friendly text using the same detection as From<String>.
Apply it in all three affected UI screens.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…code matching Replace string-based RPC error detection in UI render functions with type-safe From<dashcore_rpc::Error> conversion. Matches error code -19 structurally via dashcore_rpc::jsonrpc::error::Error::Rpc instead of parsing error message strings. - Add From<dashcore_rpc::Error> impl with code-19 → CoreWalletNotConfigured - Change render_qr_code() and generate_funding_address() to return Result<(), TaskError> instead of Result<(), String> - Remove RpcResultExt trait and TaskError::user_message() (no longer needed) - Replace .map_err(|e| e.to_string())? with bare ? on RPC calls - Add unit tests for From<dashcore_rpc::Error> conversion Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
On multi-wallet Dash Core nodes, wallet-state RPC calls require the wallet name in the URL path. Seven call sites used the base core_client (no wallet path), causing error -19. Add ensure_address_imported() and try_import_address() helpers on AppContext that route through core_client_for_wallet(). Update all call sites to extract core_wallet_name from the Wallet struct and use the correct client. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
- Remove wallet_seed_hash from CoreWalletNotConfigured (PROJ-001) - Propagate TaskError through run_dashpay_task instead of String (RUST-001) - Merge check_wallet_not_specified variants into with_wallet_recovery (RUST-002) - Simplify run_core_task arms via with_wallet_recovery helper (RUST-003) - Propagate dashcore_rpc::Error directly from inner functions (SEC-008) - Reject '..' sequences in Core wallet name validation (SEC-001) - Handle CoreWalletAutoDetected in display_task_result (PROJ-004) - Use defensive .get() for ComboBox index access (PROJ-006) - Log warning on DB write failure instead of discarding (RUST-004) - Parameterize SelectionDialog egui Area ID from title (RUST-006) - Suppress Enter key when ComboBox dropdown is open (RUST-007) - Avoid unnecessary String clone in ComboBox selected_text (RUST-008) - Fix wrong variant name in manual test doc (DOC-001) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
) Add unique id_salt to ScrollArea in render_banner() so each banner's expanded details section gets its own egui ID, preventing visual overlap when multiple banners have "Show details" expanded simultaneously. Closes #681 Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lklimek
commented
Mar 6, 2026
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 31 changed files in this pull request and generated 7 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
![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: 10
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/backend_task/core/refresh_single_key_wallet_info.rs (1)
37-82:⚠️ Potential issue | 🟠 MajorDelete old UTXO rows before inserting the refreshed set.
This rebuilds the current set from
list_unspent, but the persistence path below only inserts current outputs. Spent rows already stored inutxossurvive, andsrc/database/single_key_wallet.rsloads them back into the wallet on restart, so the single-key wallet can come back with stale outputs and balances.reconcile_spv_wallets()insrc/context/wallet_lifecycle.rsalready does the per-address delete first; this Core refresh path needs the same cleanup.Possible fix
let utxo_map = { let utxos = client.list_unspent(Some(0), None, Some(&[&address]), None, None)?; let mut map: HashMap<OutPoint, TxOut> = HashMap::new(); for utxo in utxos { @@ map }; + + self.db + .execute( + "DELETE FROM utxos WHERE address = ? AND network = ?", + rusqlite::params![address.to_string(), self.network.to_string()], + ) + .map_err(|e| e.to_string())?; // Step 4: Calculate balance from UTXOs let total_balance: u64 = utxo_map.values().map(|tx_out| tx_out.value).sum();🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend_task/core/refresh_single_key_wallet_info.rs` around lines 37 - 82, Before inserting refreshed UTXOs, delete existing persisted UTXOs for this single-key wallet so stale spent rows are removed: immediately before the for (outpoint, tx_out) in &utxo_map loop call a DB delete method (e.g. self.db.delete_utxos_for_key_hash(&key_hash) or self.db.delete_utxos_for_single_key(&key_hash)); if such a method doesn’t exist add one (implement deletion by key hash or address in the same table used by insert_utxo), handle and log any error consistently (map_err / tracing::warn or return the error as done elsewhere), and prefer doing the delete and subsequent inserts in the same logical operation/transaction if your DB layer supports it.
♻️ Duplicate comments (1)
src/backend_task/core/mod.rs (1)
179-183:⚠️ Potential issue | 🟡 MinorDon't stringify typed errors in the task entrypoint.
These conversions throw away the original error types and force the new error flow back through string parsing;
ConfigErrorin particular becomesGeneric, and the RPC path skips the dedicatedFrom<dashcore_rpc::Error>impl.♻️ Minimal fix
}) - .map_err(|e| TaskError::from(e.to_string())), + .map_err(TaskError::from), CoreTask::GetBestChainLocks => { // Load configs - let config = Config::load() - .map_err(|e| TaskError::from(format!("Failed to load config: {}", e)))?; + let config = Config::load().map_err(TaskError::from)?;As per coding guidelines, "Backend tasks should return
Result<T, TaskError>fromsrc/backend_task/error.rs. When adding new backend error types, add a#[from]variant toTaskErrorrather than converting toString."🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend_task/core/mod.rs` around lines 179 - 183, The code currently converts typed errors to strings in the CoreTask::GetBestChainLocks branch (around the Config::load call), losing original error types and bypassing TaskError's typed From impls; change the map_err calls to propagate the original error types into TaskError by returning/map_err to TaskError variants using the #[from] conversions instead of e.to_string()—specifically update the Config::load error mapping to map into TaskError (add a #[from] variant in src/backend_task/error.rs for the ConfigError type if missing) and remove stringification so existing From<dashcore_rpc::Error> and other From impls are used end-to-end.
🧹 Nitpick comments (9)
src/ui/identities/add_new_identity_screen/by_wallet_qr_code.rs (2)
151-157: Consider using.with_details()for technical error context.Per coding guidelines, raw backend errors shouldn't be exposed directly to users. While
TaskError::CoreWalletNotConfiguredhas a user-friendly message, other error variants (e.g., RPC failures fromensure_address_imported) may expose technical details.Given that RPC paths are deprecated in the near future (per retrieved learnings), this is low priority but worth noting for consistency with other screens.
♻️ Suggested improvement
if let Err(e) = self.render_qr_code(ui, amount_dash) { - MessageBanner::set_global( + MessageBanner::set_global( ui.ctx(), - e.to_string(), + "Failed to prepare funding address", MessageType::Error, - ); + ) + .with_details(e.to_string()); }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/identities/add_new_identity_screen/by_wallet_qr_code.rs` around lines 151 - 157, The current error path passes raw backend errors into MessageBanner::set_global when render_qr_code fails; instead map error variants from render_qr_code/TaskError (e.g., TaskError::CoreWalletNotConfigured and other cases like RPC failures from ensure_address_imported) to a user-friendly message and attach the technical details via .with_details(e.to_string()) before calling MessageBanner::set_global on ui.ctx(); update the error handling around render_qr_code to pattern-match or use a helper that returns a (friendly_message, details) pair and pass only the friendly_message to the banner while preserving e.to_string() in .with_details().
34-51: Consider consolidating the duplicate import logic.Both branches call
ensure_address_importedwith identical arguments. The logic could be simplified:♻️ Suggested simplification
if let Some(has_address) = self.core_has_funding_address { - if !has_address { - self.app_context.ensure_address_imported( - &receive_address, - core_wallet_name.as_deref(), - Some("Managed by Dash Evo Tool"), - )?; - } - self.funding_address = Some(receive_address); + if !has_address { + self.app_context.ensure_address_imported( + &receive_address, + core_wallet_name.as_deref(), + Some("Managed by Dash Evo Tool"), + )?; + } } else { self.app_context.ensure_address_imported( &receive_address, core_wallet_name.as_deref(), Some("Managed by Dash Evo Tool"), )?; - self.funding_address = Some(receive_address); self.core_has_funding_address = Some(true); } + self.funding_address = Some(receive_address);🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/identities/add_new_identity_screen/by_wallet_qr_code.rs` around lines 34 - 51, The branches around core_has_funding_address duplicate the call to app_context.ensure_address_imported and setting of self.funding_address; refactor by calling self.app_context.ensure_address_imported(&receive_address, core_wallet_name.as_deref(), Some("Managed by Dash Evo Tool")) once unconditionally, then set self.funding_address = Some(receive_address) and ensure self.core_has_funding_address is set to Some(true) if it was None (leave it as-is when Some(true)); update the block that currently references core_has_funding_address, funding_address, core_wallet_name, and app_context.ensure_address_imported to this simplified flow.docs/ai-design/2026-03-05-banner-details-overlap/manual-test.md (1)
9-15: Consider adding more specific trigger instructions.Step 1 instructs testers to "trigger 2+ error banners" but relies on vague examples in parentheses. For better reproducibility, consider adding concrete trigger steps or referencing specific test scenarios that produce the required errors (e.g., "disconnect network in settings, then attempt identity operation").
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@docs/ai-design/2026-03-05-banner-details-overlap/manual-test.md` around lines 9 - 15, Update Step 1 ("Trigger 2+ error banners") to replace vague examples with concrete, reproducible trigger steps: enumerate 2–3 specific scenarios (e.g., toggle network offline in browser devtools and perform the identity operation to produce a network error; attempt a protected API call with an expired token to produce a 401; perform a long-running fetch that will timeout) and instruct testers to perform those exact actions in sequence so that multiple distinct error banners appear; ensure the text in the manual-test.md step explicitly lists the actions, expected error types, and the order to run them.src/ui/components/selection_dialog.rs (3)
254-261: UnnecessaryArcwrapper aroundRichText.
RichTextis typically cheap to clone. Wrapping it inArcadds indirection without benefit here, and the pattern differs from the cancel button handling below (lines 280-287) which uses.into().💡 Suggested fix for consistency
- let confirm_label = if let WidgetText::RichText(rich_text) = - confirm_text - { - rich_text.clone() - } else { - Arc::new(egui::RichText::new(confirm_text.text()).color(text_color)) - }; + let confirm_label = if let WidgetText::RichText(rich_text) = confirm_text { + rich_text.clone() + } else { + egui::RichText::new(confirm_text.text()).color(text_color).into() + };🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/components/selection_dialog.rs` around lines 254 - 261, The confirm button handling currently wraps a constructed egui::RichText in Arc::new, which is unnecessary and inconsistent with the cancel button's use of .into(); update the match for confirm_text in the same block (referencing confirm_label and confirm_text) to return either the cloned RichText or the constructed egui::RichText converted with .into() (or just cloned directly) instead of Arc::new(...), so the branches match the cancel handling and remove the extra Arc indirection.
91-97:current_value()semantics may be confusing.When
is_openis false, this returnsSome(SelectionStatus::Canceled), implying the user canceled. However, a closed dialog might also mean the user confirmed a selection. Thestatusfield exists but isn't used here.Consider returning
self.status.clone()to reflect the actual outcome:💡 Suggested fix
fn current_value(&self) -> Option<Self::DomainType> { - if self.is_open { - None - } else { - Some(SelectionStatus::Canceled) - } + self.status.clone() }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/components/selection_dialog.rs` around lines 91 - 97, The current_value() implementation incorrectly returns Some(SelectionStatus::Canceled) whenever is_open is false, ignoring the actual outcome stored in the status field; update current_value() so it returns None while the dialog is open (self.is_open) and returns self.status.clone() (or an owned clone of the DomainType) when closed, referencing the status field and keeping SelectionStatus as the DomainType to reflect the real result.
38-44: Returning&Nonefromchanged_value()is unusual.The method returns a reference to
&None(a static reference to the constantNone) in the else branch, while returning&self.dialog_responsein the if branch. This works but mixes owned and static references in a subtle way.Consider storing the conditional result in the response struct at construction time, matching how other
ComponentResponseimplementations work.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/components/selection_dialog.rs` around lines 38 - 44, The else branch of changed_value() returns a reference to the constant &None which mixes static and instance references; instead compute and store the conditional result when the response struct is constructed and return a reference to that stored field. Update the response struct (the type that holds dialog_response) to include a field like changed_value: Option<DomainType> (or Option<Self::DomainType> where appropriate), set it during construction based on has_changed()/dialog_response, and change changed_value() to simply return &self.changed_value; keep using existing symbols dialog_response and changed_value() so callers remain unchanged.src/backend_task/dashpay/profile.rs (1)
358-358: Error type conversion maintains backward compatibility.The
send_payment_to_contactnow returnsTaskError, and this conversion toStringkeeps thesend_paymentfunction signature stable.Per coding guidelines, backend tasks should return
Result<T, TaskError>. Consider migrating this function's return type toTaskErrorin a follow-up to align with the codebase direction, which would allow preserving richer error context (includingCoreWalletNotConfiguredif applicable).🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend_task/dashpay/profile.rs` at line 358, The current .map_err(|e| e.to_string()) converts errors to String to keep send_payment's signature stable, but we should migrate to returning Result<_, TaskError> so richer errors (e.g., CoreWalletNotConfigured) are preserved; update send_payment's return type to Result<T, TaskError>, change send_payment_to_contact's error conversion to map_err(TaskError::from) or propagate the TaskError directly (use ? where possible), and adjust callers to handle TaskError instead of String; ensure TaskError implements From for the underlying error types used by send_payment_to_contact.src/context/wallet_lifecycle.rs (1)
186-198: Use different warning text for join errors vs refresh errors.The match now separates
spawn_blockingfailures from inner refresh failures, but both branches still emit the same message. A small branch-specific message or field would make these startup warnings actionable.Also applies to: 211-224
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/context/wallet_lifecycle.rs` around lines 186 - 198, The two match arms that handle tokio::task::spawn_blocking(...).await for ctx.refresh_wallet_info(wallet) should log distinct messages: change the Err(e) arm to indicate a task join/spawn failure (e.g., "Failed to spawn/join auto-refresh task for wallet: {e}") and keep the Ok(Err(e)) arm to indicate the inner refresh failed (e.g., "Failed to auto-refresh wallet UTXOs on startup: {e}"); update both occurrences (the current block around spawn_blocking handling and the similar block at the second occurrence) to use these branch-specific texts and include the error variable in the log for actionable diagnostics while leaving Ok(Ok(_)) unchanged.src/model/wallet/mod.rs (1)
741-749: Consolidate address import into one guarded path.In RPC mode, newly derived receive/change addresses go through
try_import_address()twice: first unconditionally at line 741 with a descriptive label, then again insideregister_address()at line 940–941 whencore_backend_mode() == Rpc. The first call has no backend-mode guard, while the second one does. Sincetry_import_address()ignores errors and has no deduplication logic, the same address is imported twice on the RPC path. In SPV mode, the address is imported once (line 741 only). Prefer passing the optional label intoregister_address()so the import happens from a single guarded location.Also applies to: 940-941
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/model/wallet/mod.rs` around lines 741 - 749, The unguarded call to try_import_address causes duplicate imports in RPC mode; move the formatted label creation (using self.alias and derivation_path) into the register_address call and remove the earlier unconditional try_import_address invocation so that register_address is solely responsible for importing addresses when core_backend_mode() == Rpc; update register_address to accept an Option<&str> label (pass self.core_wallet_name.as_deref() and the formatted label as Some(...)) and ensure calls to register_address propagate the label while leaving SPV behavior unchanged.
🤖 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/backend_task/error.rs`:
- Around line 52-60: The From<String> impl for TaskError currently only matches
the literal "Wallet file not specified" and thus downgrades already-typed
TaskError::CoreWalletNotConfigured that was wrapped with extra context; update
impl From<String> for TaskError to detect the stringified form of the
CoreWalletNotConfigured variant (e.g., check if
s.contains(TaskError::CoreWalletNotConfigured.to_string())) in addition to the
existing literal check, and return TaskError::CoreWalletNotConfigured when
matched so code paths like the post-mining refresh in core::mod (where a
TaskError is re-stringified with context) preserve the original variant instead
of becoming TaskError::Generic.
In `@src/context/mod.rs`:
- Around line 567-599: core_client_for_wallet currently always uses
Auth::UserPass; change it to attempt Auth::CookieFile first and fall back to
Auth::UserPass (the same auth selection logic used in AppContext::new()).
Specifically, when building the Client inside core_client_for_wallet, try
Client::new(&url, Auth::CookieFile(cfg.core_cookie_path.clone())) and if that
returns an Err, attempt Client::new(&url,
Auth::UserPass(cfg.core_rpc_user.clone(), cfg.core_rpc_password.clone())),
returning the first success or a combined/clear error if both fail; keep the
existing wallet name validation in core_client_for_wallet unchanged.
In `@src/database/initialization.rs`:
- Around line 845-855: The v28 migration add_core_wallet_name_column in
add_core_wallet_name_column currently unconditionally ALTERs wallet and
single_key_wallet which can fail if core_wallet_name was already added earlier
(e.g., via initialize_single_key_wallet_table when applying version 18). Make
the migration idempotent by querying PRAGMA table_info('wallet') and PRAGMA
table_info('single_key_wallet') and only run the ALTER TABLE ... ADD COLUMN
core_wallet_name TEXT DEFAULT NULL if that column is not present; locate
add_core_wallet_name_column and modify it to perform the pragma checks (or
alternatively guard in
apply_version_changes(18)/initialize_single_key_wallet_table to avoid re-adding
the column).
In `@src/ui/identities/top_up_identity_screen/by_wallet_qr_code.rs`:
- Around line 115-116: The code currently converts errors from
render_qr_code(...) into plain text via MessageBanner::set_global, which loses
TaskError variants (like TaskError::CoreWalletNotConfigured) needed by the
multi-wallet recovery UI and may expose raw backend messages; change the error
handling to pattern-match the returned Err(e) from self.render_qr_code(ui,
amount_dash): if the error is TaskError::CoreWalletNotConfigured (or other
recovery variants) forward/return that specific TaskError so the recovery UI can
handle it, otherwise call MessageBanner::set_global with a generic user-facing
message and attach the technical details using BannerHandle::with_details(e) (do
not convert e to plain text for the recovery path).
In `@src/ui/mod.rs`:
- Around line 860-866: The WalletsScreen::display_task_error override currently
handles CoreWalletNotConfigured by returning true but does not clear any
existing progress banner, so add a call to self.refresh_banner.take_and_clear()
in the CoreWalletNotConfigured match arm before returning true; locate the
display_task_error implementation (the method overriding
AppState::display_task_error), add the self.refresh_banner.take_and_clear() call
just prior to the return in the CoreWalletNotConfigured branch to ensure cleanup
normally done by display_message() still occurs when you suppress the default
banner.
In `@src/ui/wallets/add_new_wallet_screen.rs`:
- Around line 132-133: The code currently collapses RPC failures and the
single-wallet case into None by using
app_context.list_core_wallets().ok().filter(|w| w.len() > 1) for the
core_wallets field and then writing core_wallet_name = None; instead, preserve
the difference: call list_core_wallets() and treat Err as None but Any Ok(vec)
should be Some(vec) (or at least Some if vec.len() >= 1); only drop to None for
Err, not for vec.len() == 1. Update the initialization of core_wallets (in the
struct fields where core_wallets and selected_core_wallet_index are set) to
accept a Some(vec) when the RPC returned one wallet, and in the later
persistence logic (where core_wallet_name is set around the
create_screen()/lines ~278–281) set core_wallet_name = Some(name) when the
returned list has exactly one wallet instead of unconditionally setting None;
reference list_core_wallets(), the core_wallets field,
selected_core_wallet_index, and core_wallet_name to locate the changes.
In `@src/ui/wallets/create_asset_lock_screen.rs`:
- Around line 570-571: The current error handling after calling
render_qr_code(ui) collapses every TaskError into a banner string via
MessageBanner::set_global(e.to_string()), which prevents
TaskError::CoreWalletNotConfigured from triggering the wallet-selection recovery
flow and also exposes raw internals to users; update the match on the error
returned from render_qr_code (or the enclosing if let Err(e)) to explicitly
detect TaskError::CoreWalletNotConfigured and invoke the wallet-selection
recovery flow used by this screen, and for all other error variants call
MessageBanner::set_global with a generic, user-friendly message while attaching
the technical details via BannerHandle::with_details() (do not call
e.to_string() directly in the banner).
In `@src/ui/wallets/import_mnemonic_screen.rs`:
- Around line 92-93: The state currently stores core_wallets filtered with
.ok().filter(|w| w.len() > 1) which removes the single loaded Core wallet and
breaks the wallet-to-Core binding; change state to keep the full list returned
by app_context.list_core_wallets() (e.g., assign core_wallets =
app_context.list_core_wallets().ok() without filtering) and retain
selected_core_wallet_index logic, then hide the ComboBox UI by gating its render
on core_wallets.as_ref().map_or(false, |v| v.len() > 1) (or equivalent) so the
selector is only shown when more than one core wallet exists; apply the same
change patterns wherever list_core_wallets() was filtered (the blocks around
selected_core_wallet_index and core_wallet_name handling at the other locations
mentioned).
In `@src/ui/wallets/wallets_screen/mod.rs`:
- Around line 239-300: The code currently infers the target wallet from live UI
state which can change while backend tasks are in flight; instead, propagate the
originating wallet identity/type from the backend result/error and use that to
persist selection. Modify the backend task result/error to carry the originating
wallet_hash and a wallet_type flag/enum (e.g., SingleKey vs HD), update
display_task_error (and any task-completion handlers) to pass that identity to
apply_core_wallet_selection, and change apply_core_wallet_selection to accept
the explicit wallet_hash and wallet_type (and then call either
set_wallet_core_wallet_name or set_single_key_wallet_core_wallet_name based on
that flag) rather than reading/inferring from shared maps; update all call sites
accordingly so persistence always targets the wallet that started the operation.
- Around line 2079-2088: The UI thread is calling
self.app_context.list_core_wallets() synchronously; move that RPC off the egui
thread by replacing the blocking call with a non-blocking flow: set up the
SelectionDialog/pending state to a “loading” placeholder (e.g., set
core_wallet_dialog and pending_core_wallet_seed_hash) and spawn a background
task (tokio::spawn, spawn_blocking, or the existing backend task flow) to call
app_context.list_core_wallets(); when the task completes send the wallets back
to the UI (via a channel, a message handler, or a callback) and update
pending_core_wallet_options and core_wallet_dialog with the actual wallets or an
error. Ensure you reference app_context.list_core_wallets(), core_wallet_dialog,
pending_core_wallet_seed_hash, and pending_core_wallet_options when implementing
the async flow so the UI is never blocked by the RPC.
---
Outside diff comments:
In `@src/backend_task/core/refresh_single_key_wallet_info.rs`:
- Around line 37-82: Before inserting refreshed UTXOs, delete existing persisted
UTXOs for this single-key wallet so stale spent rows are removed: immediately
before the for (outpoint, tx_out) in &utxo_map loop call a DB delete method
(e.g. self.db.delete_utxos_for_key_hash(&key_hash) or
self.db.delete_utxos_for_single_key(&key_hash)); if such a method doesn’t exist
add one (implement deletion by key hash or address in the same table used by
insert_utxo), handle and log any error consistently (map_err / tracing::warn or
return the error as done elsewhere), and prefer doing the delete and subsequent
inserts in the same logical operation/transaction if your DB layer supports it.
---
Duplicate comments:
In `@src/backend_task/core/mod.rs`:
- Around line 179-183: The code currently converts typed errors to strings in
the CoreTask::GetBestChainLocks branch (around the Config::load call), losing
original error types and bypassing TaskError's typed From impls; change the
map_err calls to propagate the original error types into TaskError by
returning/map_err to TaskError variants using the #[from] conversions instead of
e.to_string()—specifically update the Config::load error mapping to map into
TaskError (add a #[from] variant in src/backend_task/error.rs for the
ConfigError type if missing) and remove stringification so existing
From<dashcore_rpc::Error> and other From impls are used end-to-end.
---
Nitpick comments:
In `@docs/ai-design/2026-03-05-banner-details-overlap/manual-test.md`:
- Around line 9-15: Update Step 1 ("Trigger 2+ error banners") to replace vague
examples with concrete, reproducible trigger steps: enumerate 2–3 specific
scenarios (e.g., toggle network offline in browser devtools and perform the
identity operation to produce a network error; attempt a protected API call with
an expired token to produce a 401; perform a long-running fetch that will
timeout) and instruct testers to perform those exact actions in sequence so that
multiple distinct error banners appear; ensure the text in the manual-test.md
step explicitly lists the actions, expected error types, and the order to run
them.
In `@src/backend_task/dashpay/profile.rs`:
- Line 358: The current .map_err(|e| e.to_string()) converts errors to String to
keep send_payment's signature stable, but we should migrate to returning
Result<_, TaskError> so richer errors (e.g., CoreWalletNotConfigured) are
preserved; update send_payment's return type to Result<T, TaskError>, change
send_payment_to_contact's error conversion to map_err(TaskError::from) or
propagate the TaskError directly (use ? where possible), and adjust callers to
handle TaskError instead of String; ensure TaskError implements From for the
underlying error types used by send_payment_to_contact.
In `@src/context/wallet_lifecycle.rs`:
- Around line 186-198: The two match arms that handle
tokio::task::spawn_blocking(...).await for ctx.refresh_wallet_info(wallet)
should log distinct messages: change the Err(e) arm to indicate a task
join/spawn failure (e.g., "Failed to spawn/join auto-refresh task for wallet:
{e}") and keep the Ok(Err(e)) arm to indicate the inner refresh failed (e.g.,
"Failed to auto-refresh wallet UTXOs on startup: {e}"); update both occurrences
(the current block around spawn_blocking handling and the similar block at the
second occurrence) to use these branch-specific texts and include the error
variable in the log for actionable diagnostics while leaving Ok(Ok(_))
unchanged.
In `@src/model/wallet/mod.rs`:
- Around line 741-749: The unguarded call to try_import_address causes duplicate
imports in RPC mode; move the formatted label creation (using self.alias and
derivation_path) into the register_address call and remove the earlier
unconditional try_import_address invocation so that register_address is solely
responsible for importing addresses when core_backend_mode() == Rpc; update
register_address to accept an Option<&str> label (pass
self.core_wallet_name.as_deref() and the formatted label as Some(...)) and
ensure calls to register_address propagate the label while leaving SPV behavior
unchanged.
In `@src/ui/components/selection_dialog.rs`:
- Around line 254-261: The confirm button handling currently wraps a constructed
egui::RichText in Arc::new, which is unnecessary and inconsistent with the
cancel button's use of .into(); update the match for confirm_text in the same
block (referencing confirm_label and confirm_text) to return either the cloned
RichText or the constructed egui::RichText converted with .into() (or just
cloned directly) instead of Arc::new(...), so the branches match the cancel
handling and remove the extra Arc indirection.
- Around line 91-97: The current_value() implementation incorrectly returns
Some(SelectionStatus::Canceled) whenever is_open is false, ignoring the actual
outcome stored in the status field; update current_value() so it returns None
while the dialog is open (self.is_open) and returns self.status.clone() (or an
owned clone of the DomainType) when closed, referencing the status field and
keeping SelectionStatus as the DomainType to reflect the real result.
- Around line 38-44: The else branch of changed_value() returns a reference to
the constant &None which mixes static and instance references; instead compute
and store the conditional result when the response struct is constructed and
return a reference to that stored field. Update the response struct (the type
that holds dialog_response) to include a field like changed_value:
Option<DomainType> (or Option<Self::DomainType> where appropriate), set it
during construction based on has_changed()/dialog_response, and change
changed_value() to simply return &self.changed_value; keep using existing
symbols dialog_response and changed_value() so callers remain unchanged.
In `@src/ui/identities/add_new_identity_screen/by_wallet_qr_code.rs`:
- Around line 151-157: The current error path passes raw backend errors into
MessageBanner::set_global when render_qr_code fails; instead map error variants
from render_qr_code/TaskError (e.g., TaskError::CoreWalletNotConfigured and
other cases like RPC failures from ensure_address_imported) to a user-friendly
message and attach the technical details via .with_details(e.to_string()) before
calling MessageBanner::set_global on ui.ctx(); update the error handling around
render_qr_code to pattern-match or use a helper that returns a
(friendly_message, details) pair and pass only the friendly_message to the
banner while preserving e.to_string() in .with_details().
- Around line 34-51: The branches around core_has_funding_address duplicate the
call to app_context.ensure_address_imported and setting of self.funding_address;
refactor by calling self.app_context.ensure_address_imported(&receive_address,
core_wallet_name.as_deref(), Some("Managed by Dash Evo Tool")) once
unconditionally, then set self.funding_address = Some(receive_address) and
ensure self.core_has_funding_address is set to Some(true) if it was None (leave
it as-is when Some(true)); update the block that currently references
core_has_funding_address, funding_address, core_wallet_name, and
app_context.ensure_address_imported to this simplified flow.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: dd26f353-cfd1-4449-8e35-f3da5a2dc459
📒 Files selected for processing (31)
docs/ai-design/2026-03-04-multi-wallet-rpc/manual-test-scenarios.mddocs/ai-design/2026-03-05-banner-details-overlap/manual-test.mdsrc/app.rssrc/backend_task/core/mod.rssrc/backend_task/core/recover_asset_locks.rssrc/backend_task/core/refresh_single_key_wallet_info.rssrc/backend_task/core/refresh_wallet_info.rssrc/backend_task/dashpay.rssrc/backend_task/dashpay/payments.rssrc/backend_task/dashpay/profile.rssrc/backend_task/error.rssrc/backend_task/mod.rssrc/context/mod.rssrc/context/wallet_lifecycle.rssrc/database/initialization.rssrc/database/single_key_wallet.rssrc/database/wallet.rssrc/model/wallet/mod.rssrc/model/wallet/single_key.rssrc/model/wallet/utxos.rssrc/ui/components/message_banner.rssrc/ui/components/mod.rssrc/ui/components/selection_dialog.rssrc/ui/components/styled.rssrc/ui/identities/add_new_identity_screen/by_wallet_qr_code.rssrc/ui/identities/top_up_identity_screen/by_wallet_qr_code.rssrc/ui/mod.rssrc/ui/wallets/add_new_wallet_screen.rssrc/ui/wallets/create_asset_lock_screen.rssrc/ui/wallets/import_mnemonic_screen.rssrc/ui/wallets/wallets_screen/mod.rs
- CMT-001/007: Use TaskError::from directly for dashcore_rpc::Error and JoinError instead of going through e.to_string(). Add From<JoinError> and From<rusqlite::Error> to TaskError. Simplify Config::load() via ?. - CMT-002: set_wallet_core_wallet_name and set_single_key_wallet_core_wallet_name now return rusqlite::Result<bool> (false = 0 rows, true = 1 row, Err on >1 rows). - CMT-003: Rename set_global_error -> set_global_with_error. - CMT-004: apply_core_wallet_selection returns Result<(), String>; error display moved to the caller. - CMT-005: Add rustdoc to display_task_error. - CMT-006: URL-encode wallet name in core_client_for_wallet using urlencoding crate. Add unit test for spaces -> %20. - CMT-008: Propagate DB errors in with_wallet_recovery instead of logging and continuing. Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 31 out of 32 changed files in this pull request and generated 8 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…(CMT-002, CMT-019-022) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ace (CMT-003, CMT-009) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
…ner cleanup (CMT-010-016, CMT-018) Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
lklimek
changed the title
Contributor
There was a problem hiding this comment.
Actionable comments posted: 7
♻️ Duplicate comments (5)
src/backend_task/error.rs (1)
65-72:⚠️ Potential issue | 🟠 MajorPreserve
CoreWalletNotConfiguredwhen it gets stringified.This still only recognizes the raw Dash Core RPC text. Any path that wraps
TaskError::CoreWalletNotConfiguredinto a contextual string and later converts it back throughFrom<String>will downgrade toGeneric, so the wallet-selection recovery flow disappears again.Possible fix
impl From<String> for TaskError { fn from(s: String) -> Self { - if s.contains("Wallet file not specified") { + let core_wallet_msg = TaskError::CoreWalletNotConfigured.to_string(); + if s.contains("Wallet file not specified") || s.contains(&core_wallet_msg) { TaskError::CoreWalletNotConfigured } else { TaskError::Generic(s) } } }🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend_task/error.rs` around lines 65 - 72, The conversion impl From<String> for TaskError currently only looks for the literal RPC text and can’t restore TaskError::CoreWalletNotConfigured if that variant was previously stringified into a contextual message; update the flow so the variant is preserved by (A) adding a unique marker/token when stringifying TaskError::CoreWalletNotConfigured (e.g. include "CORE_WALLET_NOT_CONFIGURED" or the variant name in the stringification site), and (B) changing impl From<String> for TaskError to detect that marker (or an exact match of the variant name) and return TaskError::CoreWalletNotConfigured, otherwise fall back to TaskError::Generic(s); refer to the impl From<String> for TaskError and the TaskError::CoreWalletNotConfigured / TaskError::Generic variants to locate where to adjust stringification and parsing.src/ui/wallets/create_asset_lock_screen.rs (1)
570-576:⚠️ Potential issue | 🟠 MajorStill swallowing the multi-wallet recovery error.
render_qr_code()now returnsTaskError, but Line 570 still reduces every variant to a banner. That preventsTaskError::CoreWalletNotConfiguredfrom reaching the new selection flow, so asset-lock funding can still dead-end on multi-wallet Core nodes.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/wallets/create_asset_lock_screen.rs` around lines 570 - 576, The current error handling around render_qr_code(ui) reduces every TaskError to a generic banner and hides TaskError::CoreWalletNotConfigured; update the match so that when render_qr_code returns Err(TaskError::CoreWalletNotConfigured) you do not convert it to a MessageBanner but instead propagate or call the multi-wallet selection flow (the same path used elsewhere for CoreWalletNotConfigured), and only show MessageBanner::set_global(...) with .with_details(e) for other error variants; locate this in the same function where render_qr_code(ui) is called and branch on TaskError::CoreWalletNotConfigured explicitly.src/ui/identities/top_up_identity_screen/by_wallet_qr_code.rs (1)
115-121:⚠️ Potential issue | 🟠 MajorStill swallowing the multi-wallet recovery error.
Line 115 turns every
render_qr_code()failure into a local banner.ensure_address_imported()can surfaceTaskError::CoreWalletNotConfiguredhere, and once it is consumed the new wallet-selection flow never gets a chance to run, so this screen still dead-ends when Core has multiple wallets loaded.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/identities/top_up_identity_screen/by_wallet_qr_code.rs` around lines 115 - 121, The current code in the top_up_identity_screen handles any error from render_qr_code(...) by converting it to a local MessageBanner, which swallows TaskError::CoreWalletNotConfigured and prevents the multi-wallet selection flow from running; update the error handling around render_qr_code (and any calls to ensure_address_imported) to match on the error: if it is TaskError::CoreWalletNotConfigured (or wraps that variant), propagate/return the error so the caller can trigger the wallet-selection flow instead of consuming it, and only show the MessageBanner for other error variants; reference render_qr_code, ensure_address_imported and TaskError::CoreWalletNotConfigured when making this change.src/ui/wallets/wallets_screen/mod.rs (2)
2102-2104:⚠️ Potential issue | 🟠 MajorMove
list_core_wallets()off the egui thread.This is still a synchronous Core RPC in the UI error path. On a slow or unavailable Core node, opening the recovery flow will stall the wallets screen until
listwalletsreturns.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/wallets/wallets_screen/mod.rs` around lines 2102 - 2104, The UI is calling the synchronous Core RPC via self.app_context.list_core_wallets() on the egui thread (in wallets_screen mod), which can block rendering; move this call to a background task/future and return immediately to the UI: create an async/background worker (or spawn on your existing backend task executor) that invokes app_context.list_core_wallets() off the UI thread, then send the result back to the UI (e.g. via a channel, message, or shared state like a Arc<Mutex>/Atomic/egui::Context.request_repaint) and update the wallets list when the response arrives; ensure the wallets_screen code no longer calls list_core_wallets() directly on the egui thread but reads the cached/placeholder state until the background task populates it.
2085-2100:⚠️ Potential issue | 🟠 MajorDon't infer the target wallet from live screen selection.
display_task_error()still reconstructs the wallet hash/type fromselected_wallet/selected_single_key_walletwhen the async error arrives. If the user switches selection or removes a wallet while the task is in flight, the Core wallet choice can be persisted onto the wrong wallet. Carry the originating wallet hash/type through the task error/result instead of reading mutable UI state here.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/wallets/wallets_screen/mod.rs` around lines 2085 - 2100, display_task_error currently recomputes the target wallet from mutable UI state (selected_wallet/selected_single_key_wallet), which can be wrong if selection changes; instead propagate the originating wallet identity through the async task result/error and use that value in display_task_error. Concretely: modify the task result/error type (or the closure that sends the error) to include (Option<WalletHash>, bool is_single_key) captured at task start (use seed_hash / key_hash to build it), update all places that construct and call display_task_error to pass that originating pair, and remove the selected_wallet / selected_single_key_wallet read logic from display_task_error so it only uses the supplied wallet identity.
🤖 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 1151-1154: The MustRetry branch currently sets a global banner and
immediately calls self.visible_screen_mut().refresh() without dismissing the
in-progress progress banner; update the
TaskResult::Error(TaskError::MustRetry(msg)) handler to clear the screen's
refresh_banner (call the equivalent of self.refresh_banner.take_and_clear() on
the visible screen or its BannerHandle) before calling
MessageBanner::set_global(...) and before self.visible_screen_mut().refresh() so
screens that rely on display_message() to dismiss progress banners do not leave
stale in-progress banners.
In `@src/backend_task/core/mod.rs`:
- Around line 250-260: CoreTask::RefreshSingleKeyWalletInfo and the other
wallet-scoped branches call with_wallet_recovery(...) which persists an
auto-detected wallet and returns TaskError::MustRetry, but these branches
currently propagate that error immediately; change them to mirror
RefreshWalletInfo: after calling with_wallet_recovery(...) detect if it returned
Err(TaskError::MustRetry) and convert that into the same success/retry behavior
used by RefreshWalletInfo (i.e. treat MustRetry as a handled case so the task
gets retried rather than failing). Update the handling in the CoreTask variants
that call with_wallet_recovery (e.g., CoreTask::RefreshSingleKeyWalletInfo and
the other wallet-scoped branches that call refresh_single_key_wallet_info /
recover / asset-lock / payment) to catch TaskError::MustRetry and return the
equivalent success/trigger-for-retry result instead of propagating the error.
In `@src/context/mod.rs`:
- Around line 574-590: core_client_for_wallet() is passing self.devnet_name
(which is initialized to None) into create_core_rpc_client, causing
wallet-scoped clients to use the wrong devnet name; change the call in
core_client_for_wallet to pass the configured devnet name from cfg (e.g.,
cfg.devnet_name or cfg.devnet_name.as_ref()) instead of self.devnet_name so
create_core_rpc_client(&url, self.network, &cfg.devnet_name, &cfg) receives the
actual configured value used at startup.
In `@src/ui/components/selection_dialog.rs`:
- Around line 223-241: The UI currently returns SelectionStatus::Selected(0)
even when self.options is empty because selected_index defaults to 0; update all
places that produce a selection on confirm/Enter (the handlers around where
selected_index, options, combo_popup_id are used — including the blocks at
~223-241, ~249-274, and ~318-322) to validate self.options.is_empty() first and
return a non-selection (e.g., SelectionStatus::None or an explicit error/state)
instead of Selected(self.selected_index); ensure any code paths that reference
self.options[self.selected_index] are guarded by the same check so no
out-of-bounds access can occur.
- Around line 91-97: current_value() currently ignores the stored dialog result
and only uses is_open, causing callers to not see the emitted SelectionStatus;
update the function to return None while is_open is true and return
Some(self.status.clone() or copy) when closed so the recorded status (e.g.,
SelectionStatus::Confirmed or ::Canceled) is observable. Apply the same fix to
the other current_value implementation referenced in the file (the second impl
at the later block) so both use the component's status field instead of
hardcoding Canceled.
In `@src/ui/wallets/add_new_wallet_screen.rs`:
- Around line 132-135: The constructor new() currently calls the blocking Core
method list_core_wallets() which can freeze screen construction; change new()
(or the AddNewWalletScreen init) to set core_wallets to None or an empty list
and selected_core_wallet_index to 0, immediately return Self with a degraded
state, then spawn an asynchronous background task (or schedule via the existing
UI task runner) that calls app_context.list_core_wallets(), updates the screen
state (core_wallets and selected_core_wallet_index) when the result arrives, and
if the call fails push a MessageBanner error into the screen state; keep
create_screen() unchanged and ensure all error handling is internal to the
screen so callers always get a Self quickly.
In `@src/ui/wallets/wallets_screen/mod.rs`:
- Around line 2014-2031: The selection handler currently drops the outstanding
backend task context after assigning a Core wallet (SelectionStatus::Selected
branch using pending_core_wallet_seed_hash, pending_core_wallet_options,
apply_core_wallet_selection and self.refresh), so failed operations (e.g.,
refresh/asset-lock/top-up that errored with -19) never resume; modify this
branch (and the similar branch at the 2105–2115 range) to capture and persist
the original task context before calling apply_core_wallet_selection, and after
a successful apply/core refresh re-dispatch the original backend task (or
enqueue it) so the refresh/asset-lock/top-up operation is retried automatically;
keep references to the task identifier/context in the struct (e.g., a
pending_backend_task field) or re-create the task from existing fields and
ensure re-dispatch happens immediately after self.refresh() succeeds.
---
Duplicate comments:
In `@src/backend_task/error.rs`:
- Around line 65-72: The conversion impl From<String> for TaskError currently
only looks for the literal RPC text and can’t restore
TaskError::CoreWalletNotConfigured if that variant was previously stringified
into a contextual message; update the flow so the variant is preserved by (A)
adding a unique marker/token when stringifying
TaskError::CoreWalletNotConfigured (e.g. include "CORE_WALLET_NOT_CONFIGURED" or
the variant name in the stringification site), and (B) changing impl
From<String> for TaskError to detect that marker (or an exact match of the
variant name) and return TaskError::CoreWalletNotConfigured, otherwise fall back
to TaskError::Generic(s); refer to the impl From<String> for TaskError and the
TaskError::CoreWalletNotConfigured / TaskError::Generic variants to locate where
to adjust stringification and parsing.
In `@src/ui/identities/top_up_identity_screen/by_wallet_qr_code.rs`:
- Around line 115-121: The current code in the top_up_identity_screen handles
any error from render_qr_code(...) by converting it to a local MessageBanner,
which swallows TaskError::CoreWalletNotConfigured and prevents the multi-wallet
selection flow from running; update the error handling around render_qr_code
(and any calls to ensure_address_imported) to match on the error: if it is
TaskError::CoreWalletNotConfigured (or wraps that variant), propagate/return the
error so the caller can trigger the wallet-selection flow instead of consuming
it, and only show the MessageBanner for other error variants; reference
render_qr_code, ensure_address_imported and TaskError::CoreWalletNotConfigured
when making this change.
In `@src/ui/wallets/create_asset_lock_screen.rs`:
- Around line 570-576: The current error handling around render_qr_code(ui)
reduces every TaskError to a generic banner and hides
TaskError::CoreWalletNotConfigured; update the match so that when render_qr_code
returns Err(TaskError::CoreWalletNotConfigured) you do not convert it to a
MessageBanner but instead propagate or call the multi-wallet selection flow (the
same path used elsewhere for CoreWalletNotConfigured), and only show
MessageBanner::set_global(...) with .with_details(e) for other error variants;
locate this in the same function where render_qr_code(ui) is called and branch
on TaskError::CoreWalletNotConfigured explicitly.
In `@src/ui/wallets/wallets_screen/mod.rs`:
- Around line 2102-2104: The UI is calling the synchronous Core RPC via
self.app_context.list_core_wallets() on the egui thread (in wallets_screen mod),
which can block rendering; move this call to a background task/future and return
immediately to the UI: create an async/background worker (or spawn on your
existing backend task executor) that invokes app_context.list_core_wallets() off
the UI thread, then send the result back to the UI (e.g. via a channel, message,
or shared state like a Arc<Mutex>/Atomic/egui::Context.request_repaint) and
update the wallets list when the response arrives; ensure the wallets_screen
code no longer calls list_core_wallets() directly on the egui thread but reads
the cached/placeholder state until the background task populates it.
- Around line 2085-2100: display_task_error currently recomputes the target
wallet from mutable UI state (selected_wallet/selected_single_key_wallet), which
can be wrong if selection changes; instead propagate the originating wallet
identity through the async task result/error and use that value in
display_task_error. Concretely: modify the task result/error type (or the
closure that sends the error) to include (Option<WalletHash>, bool
is_single_key) captured at task start (use seed_hash / key_hash to build it),
update all places that construct and call display_task_error to pass that
originating pair, and remove the selected_wallet / selected_single_key_wallet
read logic from display_task_error so it only uses the supplied wallet identity.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: d51f437b-d6c2-4eb3-9454-7f5304ee4f4a
⛔ Files ignored due to path filters (1)
Cargo.lockis excluded by!**/*.lock
📒 Files selected for processing (19)
Cargo.tomlsrc/app.rssrc/backend_task/core/mod.rssrc/backend_task/core/recover_asset_locks.rssrc/backend_task/core/refresh_single_key_wallet_info.rssrc/backend_task/core/refresh_wallet_info.rssrc/backend_task/error.rssrc/context/mod.rssrc/database/initialization.rssrc/database/single_key_wallet.rssrc/database/wallet.rssrc/model/wallet/utxos.rssrc/ui/components/message_banner.rssrc/ui/components/selection_dialog.rssrc/ui/identities/top_up_identity_screen/by_wallet_qr_code.rssrc/ui/wallets/add_new_wallet_screen.rssrc/ui/wallets/create_asset_lock_screen.rssrc/ui/wallets/import_mnemonic_screen.rssrc/ui/wallets/wallets_screen/mod.rs
✅ Files skipped from review due to trivial changes (1)
- Cargo.toml
🚧 Files skipped from review as they are similar to previous changes (1)
- src/ui/components/message_banner.rs
- CMT-001: Remove unused devnet_name field from AppContext, use cfg.devnet_name - CMT-002: Add clone-and-retry for RefreshSingleKeyWalletInfo on MustRetry - CMT-003: Show actionable warning after Core wallet selection/auto-detection - CMT-004: Call display_message() in MustRetry handler for banner cleanup - CMT-005: Fix SelectionDialog::current_value() to return stored status - CMT-006: Guard confirm/Enter against empty options in SelectionDialog Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
♻️ Duplicate comments (3)
src/backend_task/core/mod.rs (1)
278-285:⚠️ Potential issue | 🟠 MajorRetry these wallet-scoped tasks after
MustRetry, too.Unlike the refresh branches above, these arms return
with_wallet_recovery(...)directly. When auto-detection succeeds,TaskError::MustRetrybubbles tosrc/app.rs, Lines 1151-1156, which only shows a banner and refreshes the screen, so the original asset-lock/payment/recovery action is still dropped on the first attempt.Mirror the
RefreshWalletInfo/RefreshSingleKeyWalletInfopattern here: catchErr(TaskError::MustRetry(_))and rerun the original operation once with the now-persisted Core wallet selection.Also applies to: 286-293, 294-301, 302-311, 313-318
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/backend_task/core/mod.rs` around lines 278 - 285, The CreateRegistrationAssetLock arm (and the other similar arms listed) returns with_wallet_recovery(...) directly so TaskError::MustRetry bubbles out and the original operation is dropped; update each of these CoreTask arms (e.g., CoreTask::CreateRegistrationAssetLock) to follow the RefreshWalletInfo/RefreshSingleKeyWalletInfo pattern: call create_registration_asset_lock(...) and if it returns Err(TaskError::MustRetry(_)) then perform the wallet-recovery/refresh logic and retry the original create_* / pay_* / recover_* operation once using the persisted Core wallet selection (i.e., call with_wallet_recovery to persist selection, then re-invoke the original method like create_registration_asset_lock with the same args), otherwise propagate other errors unchanged.src/ui/wallets/wallets_screen/mod.rs (2)
2088-2103:⚠️ Potential issue | 🟠 MajorDon't recover against the live wallet selection.
This handler derives
wallet_hashandis_single_keyfrom whichever wallet is selected when the error arrives. If the user switches or removes wallets while the backend task is in flight, the Core wallet choice can be persisted onto the wrong wallet—or the recovery UI can become a no-op when the originating wallet is no longer selected.Carry the originating wallet id/type with the error/result instead of reconstructing it from mutable screen state.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/wallets/wallets_screen/mod.rs` around lines 2088 - 2103, The handler currently reconstructs wallet_hash/is_single_key from mutable UI state (using selected_wallet and selected_single_key_wallet) which can be wrong if selection changes; instead modify the async task/result/error type to carry the originating wallet identifier and a flag for single-key (e.g., include seed_hash/key_hash and is_single_key in the result or error), populate those fields at task creation, and then use that carried identifier when handling the completion (replace code that computes wallet_hash/is_single_key from selected_wallet/selected_single_key_wallet with the values from the task result/error); update the places that construct the error/result and the handler that reads wallet_hash/is_single_key so they use the new fields.
2105-2107:⚠️ Potential issue | 🟠 MajorMove
list_core_wallets()off the egui thread.This is still a synchronous RPC round-trip inside
display_task_error(). If Core is slow or unreachable, the UI stalls right before the recovery dialog should appear.Fetch the wallet list via a backend task or spawned worker and update the dialog when the result arrives.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@src/ui/wallets/wallets_screen/mod.rs` around lines 2105 - 2107, display_task_error currently calls self.app_context.list_core_wallets() synchronously on the egui thread causing UI stalls; remove that direct call and instead kick off an asynchronous/background fetch (e.g., spawn_blocking, tokio::spawn or a backend task) to call list_core_wallets(), store the pending/loading state on the WalletsScreen struct (or a new CoreWalletsState field), and update the dialog when the async result arrives (invoke display_task_error to render the recovered data or error once the background task completes). Ensure you reference and update the same state the UI reads so the dialog shows immediately with a loading indicator and is replaced with the wallet list or error when the background task resolves.
🤖 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 1162-1173: Replace the direct use of err.to_string() with a
generic user-facing error message when creating the global banner and when
calling display_message to avoid leaking internals; keep attaching the technical
details only via MessageBanner::set_global(...)/BannerHandle::with_details(&err)
inside the developer-mode branch (current_app_context().is_developer_mode()),
and ensure self.visible_screen_mut().display_message(...) is passed the same
generic MessageType::Error text instead of the raw err string.
---
Duplicate comments:
In `@src/backend_task/core/mod.rs`:
- Around line 278-285: The CreateRegistrationAssetLock arm (and the other
similar arms listed) returns with_wallet_recovery(...) directly so
TaskError::MustRetry bubbles out and the original operation is dropped; update
each of these CoreTask arms (e.g., CoreTask::CreateRegistrationAssetLock) to
follow the RefreshWalletInfo/RefreshSingleKeyWalletInfo pattern: call
create_registration_asset_lock(...) and if it returns
Err(TaskError::MustRetry(_)) then perform the wallet-recovery/refresh logic and
retry the original create_* / pay_* / recover_* operation once using the
persisted Core wallet selection (i.e., call with_wallet_recovery to persist
selection, then re-invoke the original method like
create_registration_asset_lock with the same args), otherwise propagate other
errors unchanged.
In `@src/ui/wallets/wallets_screen/mod.rs`:
- Around line 2088-2103: The handler currently reconstructs
wallet_hash/is_single_key from mutable UI state (using selected_wallet and
selected_single_key_wallet) which can be wrong if selection changes; instead
modify the async task/result/error type to carry the originating wallet
identifier and a flag for single-key (e.g., include seed_hash/key_hash and
is_single_key in the result or error), populate those fields at task creation,
and then use that carried identifier when handling the completion (replace code
that computes wallet_hash/is_single_key from
selected_wallet/selected_single_key_wallet with the values from the task
result/error); update the places that construct the error/result and the handler
that reads wallet_hash/is_single_key so they use the new fields.
- Around line 2105-2107: display_task_error currently calls
self.app_context.list_core_wallets() synchronously on the egui thread causing UI
stalls; remove that direct call and instead kick off an asynchronous/background
fetch (e.g., spawn_blocking, tokio::spawn or a backend task) to call
list_core_wallets(), store the pending/loading state on the WalletsScreen struct
(or a new CoreWalletsState field), and update the dialog when the async result
arrives (invoke display_task_error to render the recovered data or error once
the background task completes). Ensure you reference and update the same state
the UI reads so the dialog shows immediately with a loading indicator and is
replaced with the wallet list or error when the background task resolves.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 595a8580-5f97-4851-91fe-0cd5d7b38288
📒 Files selected for processing (5)
src/app.rssrc/backend_task/core/mod.rssrc/context/mod.rssrc/ui/components/selection_dialog.rssrc/ui/wallets/wallets_screen/mod.rs
Comment on lines
+1162
to
+1173
| if !handled { | ||
| let msg = err.to_string(); | ||
| let handle = MessageBanner::set_global(ctx, &msg, MessageType::Error); | ||
| if self.current_app_context().is_developer_mode() { | ||
| // INTENTIONAL(SEC-003): TaskError Debug output is shown to users | ||
| // in developer mode. This is a local UI app — no third parties | ||
| // see this output. Ensure inner error types don't expose secrets | ||
| // (see #667). | ||
| handle.with_details(&err); | ||
| } | ||
| self.visible_screen_mut() | ||
| .display_message(&msg, MessageType::Error); |
Contributor
There was a problem hiding this comment.
Use a generic fallback message here instead of err.to_string().
Unrecognized TaskError variants now include DB/runtime failures too, so this fallback can expose raw internal details in both the global banner and display_message(). Keep this branch generic and reserve verbatim error text for developer-mode details/logging.
Small fix
- let msg = err.to_string();
- let handle = MessageBanner::set_global(ctx, &msg, MessageType::Error);
+ let msg = "Operation failed. Please try again.";
+ let handle = MessageBanner::set_global(ctx, msg, MessageType::Error);
if self.current_app_context().is_developer_mode() {
// INTENTIONAL(SEC-003): TaskError Debug output is shown to users
// in developer mode. This is a local UI app — no third parties
// see this output. Ensure inner error types don't expose secrets
// (see `#667`).
handle.with_details(&err);
}
self.visible_screen_mut()
- .display_message(&msg, MessageType::Error);
+ .display_message(msg, MessageType::Error);As per coding guidelines, "Never expose raw backend/database errors to users. Use a generic user-friendly message in the banner and attach technical details via BannerHandle::with_details()."
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@src/app.rs` around lines 1162 - 1173, Replace the direct use of
err.to_string() with a generic user-facing error message when creating the
global banner and when calling display_message to avoid leaking internals; keep
attaching the technical details only via
MessageBanner::set_global(...)/BannerHandle::with_details(&err) inside the
developer-mode branch (current_app_context().is_developer_mode()), and ensure
self.visible_screen_mut().display_message(...) is passed the same generic
MessageType::Error text instead of the raw err string.
Member
|
Concept ACK |
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.
Issue being fixed or feature implemented
Closes #98
Closes #693
Follow-up: #700 (async
list_core_wallets())User Story
Imagine you are running Dash Evo Tool and your Dash Core node has multiple wallets loaded (e.g. a personal wallet and a masternode wallet). Previously, every wallet-related operation — refreshing balances, creating asset locks, registering identities — would fail with a cryptic RPC error about "Wallet file not specified". You'd have to close all but one wallet in Core to make DET work.
Now, DET automatically detects which Core wallet owns each DET wallet's addresses. If only one Core wallet is loaded, it's selected transparently. If multiple wallets exist and auto-detection is inconclusive, a clean selection dialog appears asking which Dash Core wallet to use. Your choice is persisted per-wallet in the database, so you never see the dialog again unless your Core setup changes.
What was done?
31 files changed, +2271 / −356 lines across backend, database, context, and UI layers.
Core Architecture Changes
Per-wallet Core RPC routing (
src/context/mod.rs)core_client_for_wallet(name)builds RPC clients targeting/wallet/<name>URLscreate_core_rpc_client()shared helper — cookie-auth-first fallback used by bothAppContext::new()and per-wallet clients (CMT-002)ensure_address_imported(),try_import_address(),list_core_wallets(),try_detect_core_wallet_for_address()..) is rejectedTyped error pipeline (
src/backend_task/error.rs)CoreWalletNotConfiguredvariant — structurally detects RPC error code -19 viaFrom<dashcore_rpc::Error>(no string parsing)MustRetry(String)variant — signals that a prerequisite was auto-fixed and the caller should retry (CMT-003)From<String>also detects "Wallet file not specified" for backwards compatibility#[from]variants:rusqlite::Error,JoinErrorAuto-detection and recovery (
src/backend_task/core/mod.rs)with_wallet_recovery()interceptsCoreWalletNotConfigured, queries all loaded Core wallets, checks address ownership viagetaddressinfo, persists the match, returnsMustRetryRefreshWalletInfocatchesMustRetryand retries immediately (clones wallet Arc before first attempt)tokio::task::block_in_place()to avoid blocking Tokio workers (CMT-009)AppState integration (
src/app.rs)MustRetryerrors show success banner and triggerrefresh()— no error shown to userdisplay_task_error()first — screens can intercept typed errors before the default bannerDatabase & Model
core_wallet_name TEXT DEFAULT NULLcolumn added to bothwalletandsingle_key_wallettablespragma_table_info()idempotency guards — safe on fresh and upgraded databases (CMT-001)set_wallet_core_wallet_name()andset_single_key_wallet_core_wallet_name()settersWalletandSingleKeyWalletmodels carrycore_wallet_name: Option<String>UI Components
SelectionDialog (
src/ui/components/selection_dialog.rs) — 402 linesWalletsBalancesScreen (
src/ui/wallets/wallets_screen/mod.rs)display_task_error()interceptsCoreWalletNotConfigured:SelectionDialogapply_core_wallet_selection()takes explicitis_single_keyflag (CMT-016)AddNewWalletScreen / ImportMnemonicScreen
len() > 1filter) — auto-applied at save time (CMT-012, CMT-013)Error handling improvements
render_qr_code()errors use generic banner +.with_details(e)instead ofe.to_string()(CMT-010, CMT-011)refresh_wallet_info,refresh_single_key_wallet_info,recover_asset_locksuse?directly — no duplicated "Failed to create Core RPC client" prefix (CMT-019–022)Deferred Items
TODO comments added for synchronous
list_core_wallets()RPC on UI thread (CMT-004–008) → tracked in #700.How has this been tested?
cargo test --all-features --workspace— all tests passcargo clippy --all-features --all-targets -- -D warnings— cleancargo +nightly fmt --all— formatteddocs/ai-design/2026-03-04-multi-wallet-rpc/manual-test-scenarios.mdTest plan
See manual-test-scenarios.md — 11 scenarios covering:
Breaking Changes
None. New
core_wallet_namecolumn hasDEFAULT NULL— existing wallets work unchanged. v28 migration is idempotent.Checklist
🤖 Co-authored by Claudius the Magnificent AI Agent
Summary by CodeRabbit
New Features
Improvements
Documentation