feat: shielded transactions (ZK) integration

[QuantumExplorer]

Summary

• Integrates shielded (ZK/Orchard) transaction support into Dash Evo Tool
• Adds shield, unshield, shielded transfer, shielded withdrawal, and shield-from-asset-lock operations
• Includes shielded note sync, nullifier checking, and commitment tree persistence
• Adds shielded wallet UI tab with address display, balance, and transaction controls
• Migrates platform dependency to 3.1-dev branch with all required API adaptations
• Merges all v1.0-dev improvements (typed TaskError, unified MessageBanner, SPV enhancements, UI fixes)

Test plan

• Verify app launches on mainnet, testnet, and local network configurations
• Test shielded wallet initialization from an existing HD wallet
• Test shield credits from a platform address into the shielded pool
• Test shielded transfer between orchard addresses
• Test unshield credits back to a platform address
• Test shielded withdrawal to a core L1 address
• Test shield-from-asset-lock flow (core DASH → shielded pool)
• Verify shielded note sync and nullifier checking
• Verify SPV sync works with the new dash-spv client API
• Run cargo clippy --all-features --all-targets -- -D warnings
• Run cargo test --all-features --workspace

🤖 Generated with Claude Code

Summary by CodeRabbit

New Features

• Added comprehensive shielded pool support with new wallet operations: shield credits (transparent → private), unshield credits (private → transparent), and private transfers.
• Introduced shielded wallet initialization and synchronization with commitment tree persistence.
• Added new UI screens for managing shielded operations: Shield Credits, Unshield Credits, Shielded Send, and Shield from Asset Lock.
• Extended platform information tools to display current shielded pool state.

Bug Fixes & Improvements

• Enhanced transaction building with improved fee handling.
• Optimized backend task execution for better performance.

[coderabbitai]

Caution

Review failed

Pull request was closed or merged during review

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

Configuration used: Path: .coderabbit.yaml

Review profile: CHILL

Plan: Pro

Run ID: 8afd5904-b79f-4a5f-9df4-fc4d67f3cb5d

📥 Commits

Reviewing files that changed from the base of the PR and between f2e8142 and cfa1dbb.

📒 Files selected for processing (1)

• src/mcp/dispatch.rs

---

📝 Walkthrough

Walkthrough

This PR introduces comprehensive shielded pool support to the Dash wallet application, adding wallet initialization, note synchronization, credit shielding, internal transfers, unshielding, asset-lock funding, and withdrawal operations. It includes Orchard ZIP32 key derivation, commitment tree persistence, SQLite-backed shielded note storage, new backend task types and result variants, four new UI screens, and refactored SPV client management to use RwLock for thread-safe coordination.

Changes

Cohort / File(s)	Summary
Dependency Updates Cargo.toml	Updated dash-sdk to track feat/mempool-support branch with shielded feature enabled; added zip32 = "0.2.0" dependency.
Shielded Backend Core src/backend_task/shielded/mod.rs, src/backend_task/shielded/bundle.rs, src/backend_task/shielded/sync.rs, src/backend_task/shielded/nullifiers.rs	New shielded task module defining ShieldedTask enum and implementing end-to-end operations: building/broadcasting shield transitions, performing shielded transfers, handling asset-lock flows, managing Orchard note selection, Merkle anchor construction, and nullifier synchronization.
Shielded Error Handling src/backend_task/error.rs	Extended TaskError enum with 11 shielded-specific variants covering unspent notes, insufficient balance, Merkle witness, proof building, broadcast failures, invalid addresses, sync failures, and tree updates.
Backend Task Integration src/backend_task/mod.rs	Added BackendTask::ShieldedTask variant and 9 new BackendTaskSuccessResult variants for shielded operations; wired task dispatch to AppContext::run_shielded_task.
Shielded Wallet Model & Crypto src/model/wallet/shielded.rs, src/context/shielded.rs	Introduced OrchardKeySet and ShieldedWalletState types with Orchard ZIP32 derivation, commitment tree management, note persistence helpers, and global ProvingKey caching via OnceLock. Added AppContext::run_shielded_task dispatcher managing initialization, sync, spend operations, and state cleanup.
Shielded Database Layer src/database/shielded.rs, src/database/mod.rs, src/database/initialization.rs	Added shielded-specific tables (shielded_notes, shielded_wallet_meta) with CRUD methods for notes, balance lookups, nullifier sync info, and commitment tree persistence; exposed database connection as public API.
App Initialization & Threading src/app.rs, src/context/mod.rs	Added shielded state cache to AppContext; spawned background proving-key warmup during app startup; refactored backend task handling from tokio::spawn to tokio::task::spawn_blocking with block_on for CPU-intensive work.
SPV & Transaction Building src/spv/manager.rs, src/backend_task/core/mod.rs	Replaced ArcSwapOption with Arc<RwLock<Option<SpvClient>>> for SPV client; refactored fee scaling logic to use immutable UTXO/change snapshots and unbounded iteration with TransactionBuilder instead of bounded retry loop.
Platform Info Extensions src/backend_task/platform_info.rs	Added ShieldedPoolState request type to fetch and format current shielded pool state via SDK query.
Shielded UI Screens src/ui/wallets/shield_credits_screen.rs, src/ui/wallets/shield_from_asset_lock_screen.rs, src/ui/wallets/shielded_send_screen.rs, src/ui/wallets/unshield_credits_screen.rs	Four new screens implementing shielded operations with input validation, progress tracking, batch/parallel execution support (shield credits), and error/success messaging.
Shielded Tab & Integration src/ui/wallets/shielded_tab.rs, src/ui/wallets/wallets_screen/mod.rs	New shielded tab view displaying balance, payment addresses, and sync status; integrated into wallet detail panel with tab selection; added shielded sync progress/note count to status display.
Send Screen Extensions src/ui/wallets/send_screen.rs	Added AddressType::Shielded and SourceSelection::Shielded variants; implemented shielded balance lookup, address parsing, and routing for shielded-to-shielded ("private transfer") and shielded-to-platform ("unshield") transactions.
UI Module Wiring src/ui/mod.rs, src/ui/wallets/mod.rs	Registered four new shielded screen types in routing layer; exported corresponding UI module files.
UI Helpers & Components src/ui/helpers.rs, src/ui/components/password_input.rs, src/ui/network_chooser_screen.rs, src/ui/tools/platform_info_screen.rs	Added platform address validation helper and constants; added non-consuming width setter to PasswordInput; updated network chooser width computation and platform info screen task list.
MCP Dispatch src/mcp/dispatch.rs	Refactored task dispatch to run async backend work inside tokio::task::spawn_blocking with handle.block_on.
Test Utilities src/model/wallet/mod.rs	Updated wallet test helper to require explicit db.store_wallet calls instead of implicit persistence.

Estimated code review effort

🎯 5 (Critical) | ⏱️ ~120 minutes

Possibly related issues

• fix(shielded): fee and transfer calculation correctness issues #795: Directly addresses shielded pool operations including credit shielding, note transfers, and asset-lock flows implemented in the new src/backend_task/shielded/* module and modified src/backend_task/core/mod.rs fee/scale logic.

Poem

🐰 Through commitments deep and notes concealed,
The wallet now shields what should be sealed!
With Orchard keys and trees so keen,
Private transfers—pristine, unseen,
Proving keys and async might,
Cryptographic delight! ✨

🚥 Pre-merge checks | ✅ 3

✅ Passed checks (3 passed)

Check name	Status	Explanation
Description Check	✅ Passed	Check skipped - CodeRabbit’s high-level summary is enabled.
Title check	✅ Passed	The pull request title 'feat: shielded transactions (ZK) integration' accurately summarizes the main change, which is adding comprehensive shielded pool/zero-knowledge transaction support to the Dash Evo Tool.
Docstring Coverage	✅ Passed	Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%.

_{✏️ Tip: You can configure your own custom pre-merge checks in the settings.}

✨ Finishing Touches

📝 Generate docstrings

• Create stacked PR
• Commit on current branch

🧪 Generate unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch zk

---

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.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[coderabbitai]

Actionable comments posted: 20

Caution

Some comments are outside the diff and can’t be posted inline due to platform limitations.

⚠️ Outside diff range comments (2)

src/ui/wallets/send_screen.rs (2)

2310-2319: ⚠️ Potential issue | 🟡 Minor

Don't label shielded outputs as supported in advanced mode yet.

These badges mark shielded addresses as recognized, but validate_and_send_advanced() still only routes Core/Platform outputs and falls through to "Invalid output address". Either hide shielded outputs in advanced mode for now or add the missing advanced routing.

Also applies to: 2457-2501

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/wallets/send_screen.rs` around lines 2310 - 2319, The UI currently
labels AddressType::Shielded as recognized in the address-type badge while
validate_and_send_advanced() does not support routing shielded outputs; update
the display logic so shielded addresses are not shown as supported in advanced
mode: in the badge rendering match that uses addr_type (the block producing
type_text/type_color) and the similar match around the other occurrence (lines
~2457-2501), detect when in advanced mode and map AddressType::Shielded to the
Unknown/unsupported case (or hide the badge) so the UI does not indicate
shielded support until validate_and_send_advanced() is implemented; keep the
AddressType::Shielded enum unchanged, only alter the presentation/mapping logic
and ensure both locations use the same behavior.

---

521-541: ⚠️ Potential issue | 🟠 Major

Validate shielded address HRP against current network.

The send_shielded_to_shielded() method parses the destination with OrchardAddress::from_bech32m_string() (line 1282–1283) but discards the decoded HRP via (addr, _). This means a Testnet shielded address (tdash1z...) can be accepted and sent on Mainnet, and vice versa. The is_shielded_address() check (line 531) only validates the prefix.

Compare this to Core addresses in the same file, which call require_network(self.app_context.network) to enforce network matching. Shielded addresses need the same validation: parse the address, extract the HRP from the result, and reject if it does not match self.app_context.network.

Also applies to: 1281–1287

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/ui/wallets/send_screen.rs` around lines 521 - 541, The shielded-address
handling currently only checks string prefixes via is_shielded_address and
discards the HRP when parsing with OrchardAddress::from_bech32m_string, allowing
testnet/mainnet mixes; update both is_shielded_address usage and
send_shielded_to_shielded so that after parsing the address you capture the
returned HRP and call the same network validation used for Core addresses (e.g.,
require_network(self.app_context.network)) to ensure the parsed HRP matches the
current self.app_context.network, and reject/make AddressType::Unknown (or
return an error) when the HRP/network does not match; reference
OrchardAddress::from_bech32m_string, send_shielded_to_shielded,
is_shielded_address, and require_network when making the change.

🧹 Nitpick comments (2)

src/ui/network_chooser_screen.rs (1)

450-453: Avoid fixed button-width reservation in the local password row.

Line 451 hardcodes 200.0 for action buttons. That makes the layout fragile with larger fonts or localized button text. Prefer computing input width from actual remaining width after rendering the buttons.

♻️ Proposed refactor

-                    // Reserve space for "Save" and "Auto Update" buttons + item spacing
-                    let buttons_width = 200.0;
-                    let input_width = (ui.available_width() - buttons_width).max(100.0);
-                    self.dashmate_password_input.set_desired_width(input_width);
-                    self.dashmate_password_input.show(ui);
-
-                    let save_clicked = ui.button("Save").clicked();
+                    let (save_clicked, auto_update_clicked) = ui
+                        .with_layout(egui::Layout::right_to_left(egui::Align::Center), |ui| {
+                            let auto_update_clicked = ui.button("Auto Update").clicked();
+                            let save_clicked = ui.button("Save").clicked();
+                            let input_width = ui.available_width().max(100.0);
+                            self.dashmate_password_input.set_desired_width(input_width);
+                            self.dashmate_password_input.show(ui);
+                            (save_clicked, auto_update_clicked)
+                        })
+                        .inner;
@@
-                    if ui.button("Auto Update").clicked() {
+                    if auto_update_clicked {

src/backend_task/shielded/sync.rs (1)

121-132: Consider logging DB insert failures.

The insert_shielded_note result is discarded with let _ = .... While the note is still added to in-memory state, a DB failure could cause data loss on restart. Consider logging the error at warning level.

🔧 Proposed fix to log DB insert errors

-        let _ = app_context.db.insert_shielded_note(
+        if let Err(e) = app_context.db.insert_shielded_note(
             seed_hash,
             &crate::database::shielded::InsertShieldedNote {
                 note_data: &note_data,
                 position: dn.position,
                 cmx: &dn.cmx,
                 nullifier: &nullifier_bytes,
                 block_height: 0,
                 value,
                 network: &network_str,
             },
-        );
+        ) {
+            tracing::warn!("Failed to persist shielded note at position {}: {}", dn.position, e);
+        }

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/backend_task/shielded/sync.rs` around lines 121 - 132, The DB insert call
currently discards errors with "let _ =
app_context.db.insert_shielded_note(...)" so failures are invisible; update the
code to handle the Result from app_context.db.insert_shielded_note(seed_hash,
&InsertShieldedNote { ... }) and, on Err(e), emit a warning-level log that
includes a clear message and the error details (e.g. "Failed to insert shielded
note into DB" with seed_hash/position/context and e). Use the existing logging
facility available in this module (e.g. app_context.logger or the crate logging
macro used elsewhere) and keep the in-memory state 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
`@docs/ai-design/2026-03-03-fix-nonce-reset-on-refresh/manual-test-scenarios.md`:
- Line 20: Update the heading text "Platform Only refresh preserves nonces" to
use a hyphenated compound modifier: change it to "Platform-Only refresh
preserves nonces" so the title reads "## Test Scenario 2: Platform-Only refresh
preserves nonces"; locate the heading line in the markdown and replace the
unhyphenated phrase with the hyphenated one.

In `@src/app.rs`:
- Around line 771-776: The background warm-up spawn in new_inner() that calls
crate::context::shielded::get_proving_key() should be skipped for testing builds
to avoid long detached builds during tests; guard the spawn with a compile-time
check (e.g. #[cfg(not(feature = "testing"))] or an if !cfg!(feature = "testing")
around the thread::spawn) so the Halo 2 ProvingKey warmup runs in normal builds
but is omitted when the "testing" feature is enabled.

In `@src/backend_task/core/mod.rs`:
- Around line 676-678: The loop that reduces total_amount using scale_factor
currently stops based on a relative scale delta (scale_factor change < 0.0001),
which lets large payments skip necessary additional fixed reductions and cause
subtract_fee_from_amount to return "Insufficient funds"; change the stop
condition to use an absolute duff delta (e.g., check the actual integer
reduction in duffs is < MIN_DUFF_STEP or reaches zero) and/or iterate by
subtracting a fixed duff step (e.g., 100 duffs) from total_amount until
subtract_fee_from_amount succeeds or the remaining amount is below the minimum
allowed, updating scale_factor accordingly; apply the same fix to the duplicate
reduction block that mirrors this logic (the other block around the
subtract_fee_from_amount usage).
- Around line 725-727: The SPV send path always sets FeeRate::normal() when
building the transaction
(TransactionBuilder::new().set_fee_rate(FeeRate::normal()).set_change_address(...)),
so WalletPaymentRequest::override_fee is silently ignored; fix by checking the
incoming WalletPaymentRequest::override_fee in this SPV branch and either apply
it to the builder (convert the override value into a FeeRate and call
set_fee_rate(...) instead of FeeRate::normal()) or explicitly return an error
indicating override_fee is unsupported for SPV sends; update the code around the
TransactionBuilder construction to read the request's override_fee and branch
accordingly.
- Around line 684-716: Add dedicated TaskError variants for the three failure
cases instead of building string details: add e.g.
TaskError::WalletInfoUnavailable, TaskError::MissingBip44Account, and
TaskError::ChangeAddressDerivation { source: Box<dyn std::error::Error + Send +
Sync> } (or similar names matching your enum style) and update the sites that
currently return TaskError::WalletPaymentFailed with string details —
specifically the managed_info lookup, the standard_bip44_accounts lookup using
DEFAULT_BIP44_ACCOUNT_INDEX, and the derive_change_address call — to return the
new typed variants; for derive_change_address wrap/attach the original error
into the new variant via map_err(|e| TaskError::ChangeAddressDerivation {
source: e.into() }) so user-facing Display remains friendly while Debug retains
the underlying error for logs.

In `@src/backend_task/error.rs`:
- Around line 628-669: Replace the untyped "detail: String" fields in the
shielded TaskError variants with typed #[source] fields to preserve error
chains: for ShieldedMerkleWitnessUnavailable, ShieldedTransitionBuildFailed,
ShieldedSyncFailed, ShieldedTreeUpdateFailed, and ShieldedNullifierSyncFailed
change their payloads from detail: String to #[source] source:
Box<dash_sdk::Error> (or the appropriate SDK/error type used elsewhere in this
file), update their constructors/usages to pass the boxed source error instead
of a formatted string, and keep the existing user-facing #[error(...)] messages
intact so the error chain is preserved while surface strings remain for users.

In `@src/backend_task/shielded/bundle.rs`:
- Around line 208-214: The code returns success immediately after
state_transition.broadcast(&sdk, None).await; fix by awaiting the actual
on-chain confirmation and updating the cached platform nonce before returning:
after calling state_transition.broadcast(...) call the same follow-up logic used
by the batch broadcaster (e.g., invoke wait_for_response() or the equivalent
method that waits for confirmation) and then bump the cached platform nonce for
the submitting address prior to returning Ok(()) so subsequent shields cannot
reuse the old nonce; reference state_transition.broadcast, wait_for_response,
and the cached platform nonce update logic in the batch broadcaster to mirror
its behavior.
- Around line 649-683: select_notes_for_amount currently selects notes to cover
only the send amount and must be changed to reserve fee headroom so transitions
won't fail for lack of inputs; update select_notes_for_amount to accept a
fee_headroom: u64 (or compute an estimated_fee inside the function) and use
amount.checked_add(fee_headroom) when checking total_available and when breaking
the greedy selection, returning the accumulated sum; update all callers
(shielded_transfer, unshield_credits, shielded_withdrawal) to pass an estimated
fee_headroom (or call the estimator used elsewhere) and surface
ShieldedInsufficientBalance using the required = amount + fee_headroom so the
error reflects reserved fees.
- Around line 396-401: The code underestimates the duff amount because
platform_fee_credits / CREDITS_PER_DUFF truncates; change the division to a
ceiling integer division before applying the 20% buffer: compute
platform_fee_duffs = ((platform_fee_credits + CREDITS_PER_DUFF - 1) /
CREDITS_PER_DUFF).saturating_mul(120) / 100 (or an equivalent safe ceil integer
division) and then compute asset_lock_duffs =
amount_duffs.saturating_add(platform_fee_duffs); update references to
platform_fee_duffs, platform_fee_credits, CREDITS_PER_DUFF, amount_duffs
accordingly.

In `@src/backend_task/shielded/nullifiers.rs`:
- Around line 57-77: The loop mutates shielded_state and advances checkpoints
even when DB writes fail—change the logic so you first collect matching notes
(using shielded_state.notes and result.found) and perform the database writes
(app_context.db.mark_shielded_note_spent for each nullifier and
app_context.db.set_nullifier_sync_info) and only upon all DB calls succeeding
update shielded_state (set note.is_spent, increment spent_count, and set
last_nullifier_sync_height/last_nullifier_sync_timestamp); if any DB call fails
return Err(TaskError::from(...)) instead of swallowing errors so the operation
is atomic and consistent with the Backend tasks Result<T, TaskError> contract.

In `@src/database/mod.rs`:
- Around line 186-197: In clear_network_data(), the unconditional DELETEs on
commitment_tree_shards, commitment_tree_cap, commitment_tree_checkpoints, and
commitment_tree_checkpoint_marks_removed remove state for all networks and also
suppress real errors via let _ = tx.execute(...); change each tx.execute call to
include a network filter (e.g. "DELETE FROM <table> WHERE network = ?1") and
pass &network_str as the parameter so only the requested network is cleared, and
replace the ignored-result pattern with proper error handling that only ignores
"no such table" (or equivalent missing-table error) while returning/propagating
other SQL errors.

In `@src/database/shielded.rs`:
- Around line 74-94: The mapper closures constructing ShieldedNoteRow currently
call copy_from_slice(&bytes) for the cmx and nullifier fields which will panic
on malformed blob lengths; update both mapping sites (the ShieldedNoteRow
construction blocks that set cmx and nullifier) to validate bytes.len() == 32
before copying and, on mismatch, return a rusqlite::Error instead of panicking
(e.g. return Err(rusqlite::Error::FromSqlConversionFailure(bytes.len(),
rusqlite::types::Type::Blob,
Box::new(std::io::Error::new(std::io::ErrorKind::InvalidData, "invalid blob
length"))))) so malformed rows surface as DB errors rather than causing a panic.
- Around line 171-178: clear_commitment_tree_tables() performs global DELETEs on
the shared commitment_tree_* tables and suppresses all errors; change it to
delete only rows belonging to the current wallet/network (use the relevant
identifying column(s) such as wallet_id or network in WHERE clauses when
executing "DELETE FROM commitment_tree_shards", "commitment_tree_cap",
"commitment_tree_checkpoints", and "commitment_tree_checkpoint_marks_removed")
and handle errors explicitly: execute each DELETE with parameters and
propagate/log any error except the specific "no such table" condition (match
rusqlite::Error and ignore only when the error string/code indicates missing
table), returning Err for other failures instead of discarding them. Ensure you
reference the function clear_commitment_tree_tables and the four
commitment_tree_* table names when making these changes.
- Around line 100-136: The SQL mapping for get_all_shielded_notes is missing the
is_spent column and ShieldedNoteRow lacks a field for it, so callers cannot know
spent state; update the SELECT to include is_spent from shielded_notes, add an
is_spent: bool (or appropriate type) field to the ShieldedNoteRow struct, and
populate that field in the closure (use row.get(...) for is_spent and convert to
bool if needed); repeat the same change for the other function in this file that
maps shielded_notes rows (the similar mapping later in the file) and adjust any
callers to handle the new field.
- Around line 283-292: The current code swallows SQL errors by calling
unwrap_or(0) on the query_row result (query against shielded_notes using
conn.query_row with COALESCE(SUM(...), 0)), which masks real DB/query/migration
failures; change the logic to propagate the query error instead of returning
0—remove unwrap_or(0) and use ? (or map_err) to return the underlying error from
the function, then cast the i64 result to u64 and return Ok(result as u64) so
genuine failures are not mistaken for a zero balance.

In `@src/ui/helpers.rs`:
- Around line 1134-1139: is_platform_address currently does case-sensitive
prefix checks and omits the separator logic used in is_platform_address_string,
causing inconsistent validation; fix by unifying the logic (either delegate to
is_platform_address_string or replicate its behavior): normalize the input by
converting to lowercase, apply the same separator handling/stripping used in
is_platform_address_string, then perform the prefix checks (dash1, tdash1, evo1,
tevo1) so both functions accept uppercase inputs and same formats; update
is_platform_address to call or mirror is_platform_address_string (referencing
the functions is_platform_address and is_platform_address_string).
- Around line 1141-1145: Update the user-facing constants PLATFORM_ADDRESS_HINT
and PLATFORM_ADDRESS_EXAMPLES so they are full, translatable sentences instead
of fragments; replace PLATFORM_ADDRESS_HINT's "dash1... or tdash1..." with a
complete sentence like "Enter a Platform address starting with “dash1” or
“tdash1”.", and change PLATFORM_ADDRESS_EXAMPLES's "dash1.../tdash1..." to a
full sentence such as "Examples of valid prefixes are “dash1” and “tdash1”.",
ensuring the strings remain simple, end with punctuation, and are ready for
translation.

In `@src/ui/mod.rs`:
- Around line 431-438: The ScreenType equality arms currently use wildcards so
variants like ShieldCreditsScreen, ShieldFromAssetLockScreen,
ShieldedSendScreen, and UnshieldCreditsScreen compare equal regardless of which
wallet they belong to; update each matching arm in ScreenType::eq (the pattern
matches for ShieldCreditsScreen(_), ShieldFromAssetLockScreen(_),
ShieldedSendScreen(_), UnshieldCreditsScreen(_)) to destructure the inner data
and compare the contained WalletSeedHash (or equivalent wallet identifier) for
equality instead of using `_`, so only screens tied to the same wallet are
considered equal; keep the variant-to-variant comparisons but replace `_` with
explicit variable names and compare those fields.

In `@src/ui/wallets/shield_credits_screen.rs`:
- Around line 616-631: The balance check in the ShieldCreditsScreen logic
currently compares only amount.saturating_mul(repeat) against
read_address_balance(), ignoring platform/transition fees; update the check in
the method containing this snippet so it queries the fee estimator (the same
estimator used for actual shielding transactions) to compute an estimated
per-transfer fee, multiply that fee by repeat, and subtract it from the
available balance before comparing; if fees make the requested transfers
unaffordable, set error_message as before (using the DASH-formatted values) or
cap the maximum allowed amount accordingly so that amount * repeat +
estimated_fees * repeat <= balance; reference read_address_balance(),
CREDITS_PER_DUFF, and the fee estimator function used elsewhere in the code to
implement this change.

In `@src/ui/wallets/shielded_tab.rs`:
- Around line 62-71: When seed_hash changes in ShieldedTab::update_seed_hash,
also clear all wallet-scoped transient state so the new wallet doesn't inherit
in-progress operations: reset the booleans initializing and syncing to false,
clear pending_task (set to None), dismiss/reset the unlock popup (e.g. set
unlock_popup to None or its default), and reset the address-selection state
(e.g. selected_address / address_selection_state to None/default). Update the
update_seed_hash function to assign these fields alongside is_initialized,
tree_synced, shielded_balance, error_message, and success_message so all
transient state is cleared when seed_hash changes.

---

Outside diff comments:
In `@src/ui/wallets/send_screen.rs`:
- Around line 2310-2319: The UI currently labels AddressType::Shielded as
recognized in the address-type badge while validate_and_send_advanced() does not
support routing shielded outputs; update the display logic so shielded addresses
are not shown as supported in advanced mode: in the badge rendering match that
uses addr_type (the block producing type_text/type_color) and the similar match
around the other occurrence (lines ~2457-2501), detect when in advanced mode and
map AddressType::Shielded to the Unknown/unsupported case (or hide the badge) so
the UI does not indicate shielded support until validate_and_send_advanced() is
implemented; keep the AddressType::Shielded enum unchanged, only alter the
presentation/mapping logic and ensure both locations use the same behavior.
- Around line 521-541: The shielded-address handling currently only checks
string prefixes via is_shielded_address and discards the HRP when parsing with
OrchardAddress::from_bech32m_string, allowing testnet/mainnet mixes; update both
is_shielded_address usage and send_shielded_to_shielded so that after parsing
the address you capture the returned HRP and call the same network validation
used for Core addresses (e.g., require_network(self.app_context.network)) to
ensure the parsed HRP matches the current self.app_context.network, and
reject/make AddressType::Unknown (or return an error) when the HRP/network does
not match; reference OrchardAddress::from_bech32m_string,
send_shielded_to_shielded, is_shielded_address, and require_network when making
the change.

---

Nitpick comments:
In `@src/backend_task/shielded/sync.rs`:
- Around line 121-132: The DB insert call currently discards errors with "let _
= app_context.db.insert_shielded_note(...)" so failures are invisible; update
the code to handle the Result from
app_context.db.insert_shielded_note(seed_hash, &InsertShieldedNote { ... }) and,
on Err(e), emit a warning-level log that includes a clear message and the error
details (e.g. "Failed to insert shielded note into DB" with
seed_hash/position/context and e). Use the existing logging facility available
in this module (e.g. app_context.logger or the crate logging macro used
elsewhere) and keep the in-memory state behavior unchanged.

🪄 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: e48f6fc2-9036-43d5-bc36-b543b77e9c21

📥 Commits

Reviewing files that changed from the base of the PR and between 68f4837 and 875ee14.

⛔ Files ignored due to path filters (1)

• Cargo.lock is excluded by !**/*.lock

📒 Files selected for processing (35)

• Cargo.toml
• docs/ai-design/2025-02-25-spv-peer-rework/manual-tests.md
• docs/ai-design/2026-02-24-spv-sync-error-status/manual-test-scenarios.md
• docs/ai-design/2026-03-03-fix-nonce-reset-on-refresh/manual-test-scenarios.md
• docs/ai-design/2026-03-05-banner-details-overlap/manual-test.md
• src/app.rs
• src/backend_task/core/mod.rs
• src/backend_task/error.rs
• src/backend_task/mod.rs
• src/backend_task/platform_info.rs
• src/backend_task/shielded/bundle.rs
• src/backend_task/shielded/mod.rs
• src/backend_task/shielded/nullifiers.rs
• src/backend_task/shielded/sync.rs
• src/context/mod.rs
• src/context/shielded.rs
• src/database/initialization.rs
• src/database/mod.rs
• src/database/shielded.rs
• src/model/wallet/mod.rs
• src/model/wallet/shielded.rs
• src/spv/manager.rs
• src/ui/components/password_input.rs
• src/ui/helpers.rs
• src/ui/mod.rs
• src/ui/network_chooser_screen.rs
• src/ui/tools/platform_info_screen.rs
• src/ui/wallets/mod.rs
• src/ui/wallets/send_screen.rs
• src/ui/wallets/shield_credits_screen.rs
• src/ui/wallets/shield_from_asset_lock_screen.rs
• src/ui/wallets/shielded_send_screen.rs
• src/ui/wallets/shielded_tab.rs
• src/ui/wallets/unshield_credits_screen.rs
• src/ui/wallets/wallets_screen/mod.rs

[coderabbitai]

Actionable comments posted: 1

🧹 Nitpick comments (1)

src/mcp/dispatch.rs (1)

16-18: The spawn_blocking usage here is theoretically not cancel-safe, but the analysis needs clarification.

You're correct that spawn_blocking tasks cannot be cancelled once started—this is a tokio limitation. However, the comparison to handle_backend_task is misleading: handle_backend_task is a fire-and-forget spawned task (no await), whereas dispatch_task is an async function that is always awaited by its callers (all 4 usages in wallet.rs and platform.rs show .await). They have opposite call semantics.

The actual risk here is narrow: if an MCP client cancels a tool invocation mid-execution (before the dispatch_task future completes), the blocking task continues running in the thread pool. In practice, all current dispatch_task calls are awaited, so this scenario doesn't occur under normal usage.

The spawn_blocking + handle.block_on pattern is required throughout the codebase to work around non-Send SDK types (DataContract, Sdk). Moving to a dedicated current-thread worker would be a significant refactor for an edge case and may not fully resolve the underlying Send constraints. If cancel-safety during tool execution becomes a real concern, that would warrant revisiting the architecture, but the current usage pattern appears safe.

🤖 Prompt for AI Agents

Verify each finding against the current code and only fix it if needed.

In `@src/mcp/dispatch.rs` around lines 16 - 18, Update the comment on
dispatch_task to clarify the cancel-safety nuance: explicitly state that
spawn_blocking tasks cannot be cancelled once started, that dispatch_task
(unlike AppState::handle_backend_task) is always awaited by callers (see usages
in wallet.rs and platform.rs), and therefore the normal call sites avoid
cancellation; but also note the narrow risk that if a caller cancels the
dispatch_task future mid-execution the blocking thread will continue running,
and that the spawn_blocking + handle.block_on pattern is used to work around
non-Send SDK types (DataContract, Sdk) so moving to a current-thread worker
would be a significant refactor and may not fully solve the Send constraints.

🤖 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/mcp/dispatch.rs`:
- Around line 27-29: The receiver binding `_rx` keeps the tokio mpsc receiver
alive and causes unintended backpressure; change the channel creation in
dispatch where you call tokio::sync::mpsc::channel::<TaskResult>(32) so the
receiver is dropped immediately by using a bare `_` instead of `_rx`, ensuring
only `tx` is kept for SenderAsync::new(tx, ...) and then call
app_context.run_backend_task(task, sender).await as before; this prevents
subtasks from blocking when sending to the bounded channel.

---

Nitpick comments:
In `@src/mcp/dispatch.rs`:
- Around line 16-18: Update the comment on dispatch_task to clarify the
cancel-safety nuance: explicitly state that spawn_blocking tasks cannot be
cancelled once started, that dispatch_task (unlike
AppState::handle_backend_task) is always awaited by callers (see usages in
wallet.rs and platform.rs), and therefore the normal call sites avoid
cancellation; but also note the narrow risk that if a caller cancels the
dispatch_task future mid-execution the blocking thread will continue running,
and that the spawn_blocking + handle.block_on pattern is used to work around
non-Send SDK types (DataContract, Sdk) so moving to a current-thread worker
would be a significant refactor and may not fully solve the Send constraints.

🪄 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: ff381d60-eb69-4031-b54e-90d7758cd13f

📥 Commits

Reviewing files that changed from the base of the PR and between e140c60 and f2e8142.

📒 Files selected for processing (1)

• src/mcp/dispatch.rs