feat(ui): unified shield screen with address selection

[lklimek]

Summary

• Merge ShieldCreditsScreen and ShieldFromAssetLockScreen into a single ShieldScreen that uses the AddressInput component for source address selection
• Screen adapts its behavior based on the selected address type: platform addresses dispatch ShieldCredits, core addresses dispatch ShieldFromAssetLock
• Replace two separate "Shield" and "Shield from Core" buttons in the shielded tab with one unified "Shield" button

Additional improvements

• Iterative fee estimation: Replace hardcoded SHIELDED_FEE_HEADROOM (0.1 DASH) with iterative note selection that computes exact fees from compute_minimum_shielded_fee based on actual note count — converges in 2-3 iterations
• UTXO address filtering: Thread source_address: Option<&Address> through the wallet UTXO selection chain (select_unspent_utxos_for → select_utxos_with_fee_retry → generic_asset_lock_transaction), eliminating the restrict_utxos hack that temporarily mutated wallet state via std::mem::replace
• Shield screen hardening: Replace .lock().unwrap() panics with graceful fallbacks, use DashColors theme constants instead of hardcoded Color32::from_rgb(...), deduct estimated fees from max-amount display, and migrate success_message/error_message fields to MessageBanner
• Code quality: Replace inverted if-let guard with idiomatic !matches!(), consolidate duplicate doc comments, address review findings (self-send prevention, per-address balance display, cache invalidation)

Test plan

• Open the shielded tab and verify only one "Shield" button is shown
• Click "Shield", select a platform address from the dropdown, enter an amount, and confirm the shield operation completes successfully
• Click "Shield", select a core address from the dropdown, enter an amount, and confirm the asset lock + shield operation completes
• Verify the "Max" button on Core source deducts estimated fees (not raw balance)
• Verify balance display updates correctly for both platform (credits) and core (DASH) address types
• In developer mode, verify batch controls (repeat/parallel) appear only when a platform address is selected
• Verify success/error messages appear as banners (not inline colored labels)
• Verify the "Cancel" and "Done" buttons work as expected in both flows

_{🤖 Co-authored by Claudius the Magnificent AI Agent}

Summary by CodeRabbit

Release Notes

• New Features

  • Unified shield screen supporting both platform and core wallet shielding with optional source address selection

• Improvements

  • Enhanced fee estimation for shielded operations
  • Improved transaction display performance through optimized caching

[coderabbitai]

Note

Reviews paused

It looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the reviews.auto_review.auto_pause_after_reviewed_commits setting.

Use the following commands to manage reviews:

• @coderabbitai resume to resume automatic reviews.
• @coderabbitai review to trigger a single review.

Use the checkboxes below for quick actions:

• ▶️ Resume reviews
• 🔍 Trigger review

📝 Walkthrough

Walkthrough

Consolidates two shield screens into a single ShieldScreen with source selection (Core or Platform); threads fee-aware note selection and exact fee through shielded flows; adds optional source_address propagation to asset-lock/UTXO selection; triggers a PlatformInfo fetch on first sync; improves transaction cache invalidation.

Changes

Cohort / File(s)	Summary
UI: Shield screen consolidation src/ui/mod.rs, src/ui/wallets/mod.rs, src/ui/wallets/shield_screen.rs, src/ui/wallets/shield_credits_screen.rs, src/ui/wallets/shield_from_asset_lock_screen.rs, src/ui/wallets/shielded_tab.rs	Removed separate ShieldCreditsScreen and ShieldFromAssetLockScreen; added unified ShieldScreen and updated Screen/ScreenType variants, dispatch, routing, and shielded-tab navigation.
Shielded backend: fee-aware note selection src/backend_task/shielded/bundle.rs, src/backend_task/shielded/mod.rs, src/context/shielded.rs	Introduced select_notes_with_fee(...) returning exact_fee; updated shielded operations to use exact_fee and pass source_address for asset-lock shielding; added source_address to task variant.
Model: UTXO/source-address support & fee helpers src/model/wallet/utxos.rs, src/model/wallet/asset_lock_transaction.rs, src/model/wallet/mod.rs, src/model/fee_estimation.rs	Added source_address: Option<&Address> through UTXO selection and asset-lock builders; added shield fee estimation helpers and tests.
UI: Shield screen behaviour & send changes src/ui/wallets/shield_screen.rs, src/ui/wallets/send_screen.rs	New unified ShieldScreen implements address/amount input, batch/sequential modes for platform shielding, core shielding flow, and updated send-screen to include source_address: None for Core→Shielded.
Removed legacy shield screens src/ui/wallets/shield_credits_screen.rs, src/ui/wallets/shield_from_asset_lock_screen.rs	Deleted old implementations and their ScreenLike handlers.
Asset-lock builders callsite updates src/backend_task/core/create_asset_lock.rs, src/backend_task/identity/register_identity.rs, src/backend_task/identity/top_up_identity.rs, src/backend_task/wallet/fund_platform_address_from_wallet_utxos.rs, src/mcp/tools/shielded.rs	Updated calls to asset-lock transaction builders to pass an added None argument (new optional source_address parameter).
App & UI maintenance src/app.rs, src/ui/wallets/wallets_screen/mod.rs	On first transition to Synced, enqueue BackendTask::PlatformInfo(...CurrentEpochInfo...); improved transaction cache invalidation by tracking source length and rebuilding filtered indices when stale.
Docs CLAUDE.md	Documented centralization rule for fee estimation into model/fee_estimation.rs.

Sequence Diagram(s)

sequenceDiagram
    actor User
    participant ShieldScreen
    participant Model/Wallet
    participant Backend

    User->>ShieldScreen: Open screen / select source (Core|Platform)
    ShieldScreen->>Model/Wallet: Query balance / nonce for selected source
    Model/Wallet-->>ShieldScreen: Return balance/nonce
    User->>ShieldScreen: Enter amount / confirm
    ShieldScreen->>ShieldScreen: select_notes_with_fee(amount) -> exact_fee
    ShieldScreen->>Backend: Dispatch ShieldedTask (ShieldCredits or ShieldFromAssetLock {..., source_address?})
    Backend->>Model/Wallet: (if core) build asset-lock tx using source_address constraint
    Backend-->>ShieldScreen: Task result (success|error)
    ShieldScreen->>Backend: Enqueue SyncNotes on success
    User->>ShieldScreen: Done -> Pop screen

Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~45 minutes

Possibly related issues

• fix(shielded): fee and transfer calculation correctness issues #795: The change introduces fee-inclusive note selection and fee estimation which aligns with the issue's objectives about fee reservation during shielded note selection.

Possibly related PRs

• fix(ui): wallet screen identity, state management, and input validation (#797) #800: Modifies the same UI ScreenType/Screen variants and behavior around shielded screens; likely directly related to these refactors.

Poem

🐰 A rabbit hops where shields unite,

Two paths now one beneath moonlight.
Fees counted fair, addresses picked,
Notes gathered wise, and tasks ticked.
Hooray—one screen, the code takes flight!

🚥 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 PR title accurately describes the main change: consolidating two shield screen variants into a single unified screen with address selection capability.
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 unit tests (beta)

• Create PR with unit tests
• Commit unit tests in branch feat/unified-shield-screen

---

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: 1

🧹 Nitpick comments (1)

src/ui/wallets/shield_screen.rs (1)

586-588: Consider using saturating multiplication for safety.

While unlikely to overflow in practice (would require ~18.4 million billion DASH), the multiplication read_core_balance_duffs() * CREDITS_PER_DUFF could theoretically overflow. Using saturating_mul would be more defensive.

♻️ Proposed fix

 Some(AddressKind::Core) => {
-    Some(self.read_core_balance_duffs() * CREDITS_PER_DUFF)
+    Some(self.read_core_balance_duffs().saturating_mul(CREDITS_PER_DUFF))
 }

🤖 Prompt for AI Agents

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

In `@src/ui/wallets/shield_screen.rs` around lines 586 - 588, Replace the direct
multiplication in the AddressKind::Core branch with a saturating multiplication
to prevent potential overflow: call
read_core_balance_duffs().saturating_mul(CREDITS_PER_DUFF) (or cast to an
integer type that provides saturating_mul if needed) instead of using the *
operator with CREDITS_PER_DUFF, ensuring the result type remains the same as the
surrounding code.

🤖 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/ui/mod.rs`:
- Line 872: The ShieldScreen match arm in change_context
(Screen::ShieldScreen(screen) => screen.app_context = app_context.clone()) needs
to also call a new method invalidate_address_input on the ShieldScreen to clear
address/amount state when the network context changes; add a public method named
invalidate_address_input to the ShieldScreen implementation that clears
address_input, validated_source, amount_input, and amount, then update the
Screen::ShieldScreen branch in change_context to call
screen.invalidate_address_input() after assigning app_context.

---

Nitpick comments:
In `@src/ui/wallets/shield_screen.rs`:
- Around line 586-588: Replace the direct multiplication in the
AddressKind::Core branch with a saturating multiplication to prevent potential
overflow: call read_core_balance_duffs().saturating_mul(CREDITS_PER_DUFF) (or
cast to an integer type that provides saturating_mul if needed) instead of using
the * operator with CREDITS_PER_DUFF, ensuring the result type remains the same
as the surrounding code.

🪄 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: 2ff9396c-2c19-4654-a15a-20178ed92d09

📥 Commits

Reviewing files that changed from the base of the PR and between 97230f2 and 7b757ce.

📒 Files selected for processing (5)

• src/ui/mod.rs
• src/ui/wallets/mod.rs
• src/ui/wallets/shield_from_asset_lock_screen.rs
• src/ui/wallets/shield_screen.rs
• src/ui/wallets/shielded_tab.rs

💤 Files with no reviewable changes (1)

• src/ui/wallets/shield_from_asset_lock_screen.rs

[Copilot]

Pull request overview

This PR unifies the shield UX by replacing separate “shield credits” and “shield from core” screens with a single ShieldScreen that uses the AddressInput component to select a source address type and dispatch the appropriate backend task.

Changes:

• Replace the two shield buttons in the shielded tab with a single “Shield” button that opens the unified screen.
• Introduce ShieldScreen that supports both Platform (ShieldCredits) and Core (ShieldFromAssetLock) flows via AddressInput-based source selection.
• Remove ShieldFromAssetLockScreen and update screen wiring/enums to reference ShieldScreen.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.

Show a summary per file

File	Description
src/ui/wallets/shielded_tab.rs	Replaces two shield actions/buttons with a single entry point to the unified shield screen.
src/ui/wallets/shield_screen.rs	Implements the unified shielding UI/logic, including platform batch mode and core asset-lock flow.
src/ui/wallets/shield_from_asset_lock_screen.rs	Deletes the now-redundant dedicated core shielding screen.
src/ui/wallets/mod.rs	Updates wallets module exports to use shield_screen instead of the removed screens.
src/ui/mod.rs	Rewires screen types/constructors/enums to use ShieldScreen and drops old screen variants.

Comments suppressed due to low confidence (3)

src/ui/wallets/shield_screen.rs:724

• Core flow converts the entered amount from credits to duffs via integer division (amount / CREDITS_PER_DUFF). If the user enters less than 1 duff (i.e., < CREDITS_PER_DUFF credits), this becomes 0 duffs and can trigger an asset-lock flow that still spends fees but shields 0 value. Add validation to reject amounts < 1 duff (and ideally enforce duff granularity for core shielding), showing a user-facing error instead of dispatching the task.
  src/ui/wallets/shield_screen.rs:521
• The screen forces selecting a specific Core address via AddressInput (selection_only), but the Core shielding path doesn’t use the selected address at all (it only uses seed_hash and the wallet’s total balance). This makes the selection misleading and can block the Core flow if no eligible Core address appears in autocomplete. Consider using an explicit source-type toggle (Core wallet vs Platform address) or a non-address "Core wallet" option so the UI matches what the backend task actually does.
  src/ui/wallets/shield_screen.rs:588
• For the Core flow, the max amount is set from the full wallet balance (total_balance_duffs * CREDITS_PER_DUFF). The backend asset-lock flow adds platform fees (and Core tx fees), so using Max is likely to fail due to insufficient funds. Consider reducing the max by an estimated fee reserve and/or showing a max-exceeded hint explaining the reserved amount.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[coderabbitai]

Actionable comments posted: 1

🤖 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 1065-1069: The CurrentEpochInfo cache refresh should not flow
through the normal TaskResult/display pipeline; change the code path where
BackendTask::PlatformInfo(CurrentEpochInfo) is created and sent by
handle_backend_task() so it uses a silent cache-refresh path or emits a
dedicated result variant that AppState consumes without calling
display_task_result(). Specifically, avoid sending this via
handle_backend_task()/TaskResult that can produce PlatformInfo::TextResult for
screens; instead implement a private/silent handler called by the update path
that updates AppContext.fee_multiplier_permille and
AppContext.platform_protocol_version directly (or emit a BackendTask result
variant like PlatformInfo::SilentUpdate that AppState treats as internal), and
ensure AppState's receiver routing ignores/suppresses display_task_result() for
that variant so no banners or unsolicited TextResult are shown.

🪄 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: 73de2b59-39ab-4300-9e5a-955b9c012908

📥 Commits

Reviewing files that changed from the base of the PR and between 7b757ce and 40e2614.

📒 Files selected for processing (1)

• src/app.rs

[thepastaclaw]

Code Review

Verified 1 convergent finding against the current source. The core-source shield flow ignores the selected address; the empty-selection finding is not supported by the current ShieldScreen and AddressInput logic.

Reviewed commit: c4d3a56

🔴 1 blocking

🤖 Prompt for all review comments with AI agents

These findings are from an automated code review. Verify each finding against the current code and only fix it if needed.

In `src/ui/wallets/shield_screen.rs`:
- [BLOCKING] lines 717-724: Selected core address is ignored when dispatching shield-from-core
  Verified. In the Core branch, the UI dispatches `ShieldedTask::ShieldFromAssetLock` with only `seed_hash` and `amount_duffs`, even though the screen requires the user to choose a specific source address earlier in the flow. The task enum itself has no field for a core source address (`src/backend_task/shielded/mod.rs:44-48`), and the backend implementation builds the asset lock by calling `wallet.generic_asset_lock_transaction(...)` without any selected-address input (`src/backend_task/shielded/bundle.rs:457-489`). So the chosen Core address cannot influence which funds are used, making the selection ineffective. Confidence is boosted because both agents reported the same regression and the code path confirms it end-to-end.

[coderabbitai]

Actionable comments posted: 5

🤖 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/shielded/bundle.rs`:
- Line 652: The line calling select_notes_for_amount exceeds the line-length
limit; reformat it by breaking the argument list across multiple lines or by
extracting SHIELDED_FEE_HEADROOM into a temporary variable so the let binding
uses shorter lines. Specifically update the let binding that assigns
(spendable_notes, total_input_value) from
select_notes_for_amount(shielded_state, amount, SHIELDED_FEE_HEADROOM) in
bundle.rs: either place each argument on its own indented line or introduce a
local variable (e.g., let fee_headroom = SHIELDED_FEE_HEADROOM;) and call
select_notes_for_amount(shielded_state, amount, fee_headroom) to keep the line
length within limits.
- Line 259: The call to select_notes_for_amount is too long for rustfmt; break
its arguments onto multiple lines so cargo fmt passes: split the call at the
opening parenthesis and place each argument (shielded_state, amount,
SHIELDED_FEE_HEADROOM) on its own indented line, then close the call and assign
to (spendable_notes, total_input_value); apply the same multiline formatting for
the other occurrences of select_notes_for_amount (the calls around the current
lines 352 and 652) to satisfy line-length rules.
- Line 352: The line calling select_notes_for_amount is too long; break it into
multiple shorter lines to satisfy the formatter by moving the assignment or the
tuple destructuring to a new line. For example, split the let binding across
lines (reference: select_notes_for_amount, spendable_notes, total_input_value,
shielded_state, amount, SHIELDED_FEE_HEADROOM) so the call and/or variable names
are wrapped and the line length is under the limit.

In `@src/context/wallet_lifecycle.rs`:
- Around line 144-146: The predicate assigning has_platform_addresses is
misformatted; update the expression using
guard.watched_addresses.values().any(...) so it meets rustfmt rules (e.g., place
the closure on one line or wrap args to match rustfmt), ensuring the comparison
uses crate::model::wallet::DerivationPathReference::PlatformPayment exactly as
before; then run cargo +nightly fmt (or rustfmt) and recommit so cargo fmt
--check passes.

In `@src/ui/wallets/wallets_screen/mod.rs`:
- Around line 1548-1553: The cache invalidation currently only clears
cached_tx_indices when any cached index is out-of-bounds against
wallet_guard.transactions.len(), which misses the case where the transaction
list has grown (stale but still in-bounds indices); add a new field
cached_tx_source_len: Option<usize> to the WalletsBalancesScreen struct, set it
to Some(wallet_guard.transactions.len()) whenever cached_tx_indices is
populated, and update the invalidation logic to also compare
cached_tx_source_len to wallet_guard.transactions.len() and clear both
cached_tx_indices and cached_tx_source_len when the lengths differ (also ensure
cached_tx_source_len is cleared anywhere cached_tx_indices is reset).

🪄 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: 15d9041a-fea9-488f-9f3c-d64457cd59f2

📥 Commits

Reviewing files that changed from the base of the PR and between 40e2614 and f4d280e.

📒 Files selected for processing (3)

• src/backend_task/shielded/bundle.rs
• src/context/wallet_lifecycle.rs
• src/ui/wallets/wallets_screen/mod.rs

[coderabbitai]

Actionable comments posted: 4

🤖 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/ui/wallets/send_screen.rs`:
- Around line 2062-2082: Preloading identities and shielded info currently runs
every repaint before address_input.get_or_insert_with(...), causing repeated
filtering and locking; wrap that preload behind a lazy-check so it only runs
when the AddressInput is absent: move the block that computes
source_identity_id, loaded_identities, and shielded_info into an if
self.address_input.is_none() { ... } guard (or into the AddressInput
initialization path in show()), referencing selected_source /
SourceSelection::Identity, get_loaded_identities(), and
selected_wallet_seed_hash / app_context.shielded_states so the heavy work only
happens on first creation.
- Around line 1891-1897: The preview card is still using identities.first()
while selection logic chooses self.selected_identity.clone().or_else(||
identities.iter().max_by_key(|qi| qi.identity.balance()).cloned()), causing
mismatched balances; compute the default identity once (e.g., let
default_identity = self.selected_identity.clone().or_else(||
identities.iter().max_by_key(|qi| qi.identity.balance()).cloned())) and use that
same default_identity for both the selection logic and the identity card preview
rendering so the displayed summary always matches the selected identity.
- Around line 2062-2074: The autocomplete filter only removes the source from
suggestions but doesn't stop manual entry; add an explicit guard inside
send_identity_to_identity() to detect when the destination identity id equals
the currently selected source identity id (use the same identity.id() comparison
used when computing source_identity_id) and return/emit an error early instead
of dispatching the send; also keep the existing get_loaded_identities() filter
as UX-only and reference selected_source/SourceSelection::Identity to obtain the
current source id for the equality check.
- Around line 2078-2082: The tuple construction block using OrchardAddress
(lines referencing state.keys.default_address.to_raw_address_bytes(),
OrchardAddress::from_raw_bytes, addr.to_bech32m_string, and
state.shielded_balance) is misformatted and failing CI; run rustfmt by executing
`cargo +nightly fmt` (or format just the file) to reflow and fix the formatting
in src/ui/wallets/send_screen.rs around the OrchardAddress tuple expression so
the tuple construction conforms to project formatting rules.

🪄 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: 58ead717-42c3-42c2-b746-3273bdd527dd

📥 Commits

Reviewing files that changed from the base of the PR and between 2507e9e and 89d335e.

📒 Files selected for processing (2)

• src/ui/components/address_input.rs
• src/ui/wallets/send_screen.rs

✅ Files skipped from review due to trivial changes (1)

• src/ui/components/address_input.rs

[lklimek]

@coderabbitai review all

[coderabbitai]

@lklimek Sure! I'll kick off a full review of all the changes in this PR right away.

✅ Actions performed

Full review triggered.

[coderabbitai]

Actionable comments posted: 2

Caution

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

⚠️ Outside diff range comments (1)

src/ui/wallets/shield_screen.rs (1)

42-54: 🛠️ Refactor suggestion | 🟠 Major

Use the shared MessageBanner flow for shield feedback.

This screen keeps error_message/success_message locally and display_message() writes its own UI text, so task feedback bypasses the centralized banner path that AppState already manages. That can also double-render the same result. Please switch this screen to MessageBanner/BannerHandle state instead of inline labels.

As per coding guidelines, "Use MessageBanner for all user-facing messages (errors, warnings, success, infos)." and "Screens that run backend tasks should store a refresh_banner: Option<BannerHandle> field."

Also applies to: 499-510, 811-829

🤖 Prompt for AI Agents

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

In `@src/ui/wallets/shield_screen.rs` around lines 42 - 54, The screen currently
stores error_message and success_message and renders them in display_message(),
bypassing the centralized banner; replace those fields with a single
refresh_banner: Option<BannerHandle> and remove inline label rendering in
display_message(), then switch all places that set error_message/success_message
(and any direct calls that render inline messages) to create/update the
MessageBanner via BannerHandle (use AppState's MessageBanner flow) so task
callbacks use refresh_banner.unwrap_or_create(...) and call
show_error/show_success/show_info as appropriate; ensure pending_refresh_task
still triggers the banner update path and remove duplicate rendering to avoid
double messages.

♻️ Duplicate comments (4)

src/ui/wallets/wallets_screen/mod.rs (1)

1548-1553: ⚠️ Potential issue | 🟠 Major

Cache invalidation still misses transaction-list growth.

At Line 1548, invalidation only checks out-of-bounds indices. If the transaction list is replaced with a longer list, cached indices remain in-bounds but stale, so new transactions can be omitted from the table.

Suggested fix

diff --git a/src/ui/wallets/wallets_screen/mod.rs b/src/ui/wallets/wallets_screen/mod.rs
@@
     /// Cached filtered transaction indices for the currently selected wallet.
     /// Invalidated (set to None) on wallet switch or transaction updates.
     cached_tx_indices: Option<Vec<usize>>,
+    /// Source transaction length for `cached_tx_indices`.
+    cached_tx_source_len: Option<usize>,
@@
             pending_list_wallet_hash: None,
             pending_list_is_single_key: false,
             cached_tx_indices: None,
+            cached_tx_source_len: None,
         }
@@
         self.selected_account = None;
         self.selected_account_tab = AccountTab::default();
         self.cached_tx_indices = None;
+        self.cached_tx_source_len = None;
@@
         self.mine_dialog.address_input = None;
         self.mine_dialog.validated_address = None;
         self.cached_tx_indices = None;
+        self.cached_tx_source_len = None;
@@
-        // Invalidate cache if transaction count changed (wallet refreshed).
-        if let Some(ref cached) = self.cached_tx_indices {
-            if cached.iter().any(|&i| i >= wallet_guard.transactions.len()) {
-                self.cached_tx_indices = None;
-            }
-        }
+        // Invalidate cache when transaction length changes or indices go stale.
+        let tx_len = wallet_guard.transactions.len();
+        if self.cached_tx_source_len != Some(tx_len)
+            || self
+                .cached_tx_indices
+                .as_ref()
+                .is_some_and(|cached| cached.iter().any(|&i| i >= tx_len))
+        {
+            self.cached_tx_indices = None;
+            self.cached_tx_source_len = Some(tx_len);
+        }
         let relevant_indices = self.cached_tx_indices.get_or_insert_with(|| {
-            (0..wallet_guard.transactions.len())
+            (0..tx_len)
                 .filter(|&i| wallet_guard.transactions[i].is_ours)
                 .collect()
         });
@@
                 self.refreshing = false;
                 self.cached_tx_indices = None;
+                self.cached_tx_source_len = None;

🤖 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 1548 - 1553, The cache
invalidation only checks for out-of-bounds indices and misses when the
transaction list grows; update the logic in the block that references
self.cached_tx_indices and wallet_guard.transactions.len() to also detect list
growth by computing the maximum cached index (e.g., cached.iter().max()) and
invalidating if that max is less than wallet_guard.transactions.len() - 1
(meaning new transactions exist), while keeping the existing out-of-bounds check
(any(|&i| i >= wallet_guard.transactions.len())).

src/ui/mod.rs (1)

872-872: ⚠️ Potential issue | 🟡 Minor

Clear ShieldScreen input state when the context changes.

ShieldScreen now caches AddressInput, validated_source, and AmountInput, but this arm only swaps app_context. If the network or backing wallet set changes while the screen is open, the selector can keep validating against stale state. Please mirror the ShieldedSendScreen/UnshieldCreditsScreen pattern and invalidate the cached inputs here.

🔧 Suggested fix

-            Screen::ShieldScreen(screen) => screen.app_context = app_context.clone(),
+            Screen::ShieldScreen(screen) => {
+                screen.app_context = app_context.clone();
+                screen.invalidate_address_input();
+            }

🤖 Prompt for AI Agents

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

In `@src/ui/mod.rs` at line 872, When handling Screen::ShieldScreen(screen) and
swapping screen.app_context = app_context.clone(), also invalidate the cached
input state (clear/reset screen.AddressInput, screen.validated_source, and
screen.AmountInput) so selectors revalidate against the new context; mirror the
reset pattern used in ShieldedSendScreen and UnshieldCreditsScreen to ensure
address/amount/validated_source are set to their empty/default/None equivalents
whenever app_context changes.

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

259-259: ⚠️ Potential issue | 🟡 Minor

Run cargo fmt on these updated note-selection bindings.

CI is still reporting rustfmt diffs on these call sites, so this file is not formatter-clean yet.

🔧 Suggested fix

-    let (spendable_notes, total_input_value) = select_notes_for_amount(shielded_state, amount, SHIELDED_FEE_HEADROOM)?;
+    let (spendable_notes, total_input_value) =
+        select_notes_for_amount(shielded_state, amount, SHIELDED_FEE_HEADROOM)?;

As per coding guidelines, "**/*.rs: Run cargo clippy and cargo +nightly fmt when finalizing work."

Also applies to: 352-352, 652-652

🤖 Prompt for AI Agents

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

In `@src/backend_task/shielded/bundle.rs` at line 259, The tuple binding for the
note-selection call is not formatted to project rustfmt rules; run the formatter
and adjust the binding sites so they match rustfmt output. Specifically,
reformat the call to select_notes_for_amount and its destructured bindings
(e.g., spendable_notes, total_input_value) and the other similar tuple-binding
call sites (the other select_notes_for_amount usages noted) by running cargo
+nightly fmt (or rustfmt) so the whitespace/line-breaks match CI expectations.
Ensure the function name select_notes_for_amount and the tuple destructuring are
left intact, only fixing formatting.

src/ui/wallets/shield_screen.rs (1)

717-724: ⚠️ Potential issue | 🔴 Critical

The selected Core address is still ignored.

This branch only dispatches { seed_hash, amount_duffs }. ShieldedTask::ShieldFromAssetLock and the backend asset-lock path do not accept a source address, so the picker is cosmetic right now. Either thread the selected Core address through the task/backend or remove Core address selection until it can actually influence funding.

🤖 Prompt for AI Agents

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

In `@src/ui/wallets/shield_screen.rs` around lines 717 - 724, The Core address
selected in the AddressKind::Core branch is never passed to the backend (only
seed_hash and amount_duffs are sent), so either thread the chosen address
through the task or remove the selection UI; to fix, add a source_address (or
similar) field to the ShieldedTask::ShieldFromAssetLock variant and its backend
handler, then when constructing the task in the AddressKind::Core branch include
the screen's selected Core address alongside seed_hash and amount_duffs (update
types/serialization and the backend asset-lock path to consume source_address),
or if you prefer the quick alternative, remove the Core selection code path so
the picker cannot be chosen until funding support exists.

🤖 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/shielded/bundle.rs`:
- Around line 738-756: In select_notes_for_amount, stop passing the
fee-inclusive `required` into the user-facing ShieldedInsufficientBalance
variant; instead pass the original `amount` (so the banner shows the
user-requested send amount) and surface the fee/headroom separately via a new
error variant or via BannerHandle::with_details() so the banner can state the
fee reserve reason clearly; update the error construction around
ShieldedInsufficientBalance (or add ShieldedInsufficientBalanceWithFee) and
ensure references to `unspent`, `required`, and `total_available` remain correct
while keeping technical fee numbers out of the primary user message.

In `@src/ui/wallets/shield_screen.rs`:
- Around line 582-604: The Core max amount currently uses
read_core_balance_duffs() * CREDITS_PER_DUFF without reserving the
address_funding_asset_lock_cost, so Max/near-max selections can later fail;
update the Core branch in the max_credits calculation (where source_kind ==
Some(AddressKind::Core), read_core_balance_duffs, CREDITS_PER_DUFF, and
amount_input.set_max_amount are used) to subtract an estimated asset_lock fee
(use the same estimate logic or the address_funding_asset_lock_cost
constant/calculation from src/backend_task/shielded/bundle.rs) before returning
max_credits, and add a validation step when the user submits or updates the
amount to ensure entered amount <= (core_balance_in_credits -
estimated_asset_lock_fee) so UI validation prevents amounts that would be
insufficient after the backend adds the asset lock fee; apply the same change to
the other referenced block (lines ~717-724) that sets max/validations.

---

Outside diff comments:
In `@src/ui/wallets/shield_screen.rs`:
- Around line 42-54: The screen currently stores error_message and
success_message and renders them in display_message(), bypassing the centralized
banner; replace those fields with a single refresh_banner: Option<BannerHandle>
and remove inline label rendering in display_message(), then switch all places
that set error_message/success_message (and any direct calls that render inline
messages) to create/update the MessageBanner via BannerHandle (use AppState's
MessageBanner flow) so task callbacks use refresh_banner.unwrap_or_create(...)
and call show_error/show_success/show_info as appropriate; ensure
pending_refresh_task still triggers the banner update path and remove duplicate
rendering to avoid double messages.

---

Duplicate comments:
In `@src/backend_task/shielded/bundle.rs`:
- Line 259: The tuple binding for the note-selection call is not formatted to
project rustfmt rules; run the formatter and adjust the binding sites so they
match rustfmt output. Specifically, reformat the call to select_notes_for_amount
and its destructured bindings (e.g., spendable_notes, total_input_value) and the
other similar tuple-binding call sites (the other select_notes_for_amount usages
noted) by running cargo +nightly fmt (or rustfmt) so the whitespace/line-breaks
match CI expectations. Ensure the function name select_notes_for_amount and the
tuple destructuring are left intact, only fixing formatting.

In `@src/ui/mod.rs`:
- Line 872: When handling Screen::ShieldScreen(screen) and swapping
screen.app_context = app_context.clone(), also invalidate the cached input state
(clear/reset screen.AddressInput, screen.validated_source, and
screen.AmountInput) so selectors revalidate against the new context; mirror the
reset pattern used in ShieldedSendScreen and UnshieldCreditsScreen to ensure
address/amount/validated_source are set to their empty/default/None equivalents
whenever app_context changes.

In `@src/ui/wallets/shield_screen.rs`:
- Around line 717-724: The Core address selected in the AddressKind::Core branch
is never passed to the backend (only seed_hash and amount_duffs are sent), so
either thread the chosen address through the task or remove the selection UI; to
fix, add a source_address (or similar) field to the
ShieldedTask::ShieldFromAssetLock variant and its backend handler, then when
constructing the task in the AddressKind::Core branch include the screen's
selected Core address alongside seed_hash and amount_duffs (update
types/serialization and the backend asset-lock path to consume source_address),
or if you prefer the quick alternative, remove the Core selection code path so
the picker cannot be chosen until funding support exists.

In `@src/ui/wallets/wallets_screen/mod.rs`:
- Around line 1548-1553: The cache invalidation only checks for out-of-bounds
indices and misses when the transaction list grows; update the logic in the
block that references self.cached_tx_indices and wallet_guard.transactions.len()
to also detect list growth by computing the maximum cached index (e.g.,
cached.iter().max()) and invalidating if that max is less than
wallet_guard.transactions.len() - 1 (meaning new transactions exist), while
keeping the existing out-of-bounds check (any(|&i| i >=
wallet_guard.transactions.len())).

🪄 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: 9bde4906-aa1b-478d-9e0d-8a2343724ce5

📥 Commits

Reviewing files that changed from the base of the PR and between 97230f2 and c4d3a56.

📒 Files selected for processing (8)

• src/app.rs
• src/backend_task/shielded/bundle.rs
• src/ui/mod.rs
• src/ui/wallets/mod.rs
• src/ui/wallets/shield_from_asset_lock_screen.rs
• src/ui/wallets/shield_screen.rs
• src/ui/wallets/shielded_tab.rs
• src/ui/wallets/wallets_screen/mod.rs

💤 Files with no reviewable changes (1)

• src/ui/wallets/shield_from_asset_lock_screen.rs

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 1 comment.

Comments suppressed due to low confidence (3)

src/ui/wallets/shield_screen.rs:521

• The AddressInput here applies .with_balance_range(1..) and .with_exclude_change(true) to Core addresses as well. For the Core->shielded (asset lock) flow, the selected Core address isn’t actually used as a funding source (the task spends wallet UTXOs), so filtering out change addresses / zero-balance addresses can prevent users from proceeding even though the wallet has funds. Consider relaxing these filters for Core selections (or making them conditional per kind) so the Core flow remains usable in wallets whose funds currently reside on change addresses.
  src/ui/wallets/shield_screen.rs:724
• Core amount conversion uses amount / CREDITS_PER_DUFF, which silently truncates if the credit amount isn’t an exact duff multiple and bypasses existing validation helpers. Since self.amount is an Amount in DASH, prefer amount_obj.dash_to_duffs() (and surface an error if it fails) to avoid unit/rounding mistakes if decimal places or units change.
  src/ui/wallets/shield_screen.rs:588
• self.read_core_balance_duffs() * CREDITS_PER_DUFF can overflow u64 in theory. It’s probably safe for normal DASH balances, but using saturating_mul (or checked_mul with a user-facing error) would make this more robust and consistent with other overflow-safe arithmetic in the codebase.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 8 out of 8 changed files in this pull request and generated 4 comments.

Comments suppressed due to low confidence (1)

src/ui/wallets/shield_screen.rs:529

• AddressInput is configured with selection_only, balance_range(1..), and exclude_change(true) while allowing Core addresses. This can make the Core shielding flow impossible for wallets whose spendable funds currently reside only on change addresses (a common case after sending), even though ShieldFromAssetLock uses wallet UTXOs and does not actually use the selected Core address. Consider either (a) not excluding change addresses when Core is an enabled kind, (b) not requiring a specific Core address selection for the Core flow, or (c) wiring the selected Core address into the shielding-from-core task so the selection is meaningful.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 8 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[lklimek]

@coderabbitai review all

[coderabbitai]

@lklimek Sure! I'll kick off a full review of all the changes in this PR right away.

✅ Actions performed

Full review triggered.

[coderabbitai]

Actionable comments posted: 4

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 (1)

1723-1759: ⚠️ Potential issue | 🟠 Major

Reserve fee headroom before dispatching identity transfers.

This method still accepts amount_credits == identity_balance, but the backend charges a transfer fee on top. The screen can approve an amount that only fails after IdentityTask::Transfer is dispatched. Reuse the same estimate_credit_transfer() reserve you already surface in the amount widget.

Suggested guard

         let identity_balance = qualified_identity.identity.balance();
-        if amount_credits > identity_balance {
+        let estimated_fee = self.app_context.fee_estimator().estimate_credit_transfer();
+        let max_sendable = identity_balance.saturating_sub(estimated_fee);
+        if amount_credits > max_sendable {
             return Err(format!(
-                "Insufficient identity balance. Need {} but have {}",
-                format_credits_as_dash(amount_credits),
-                format_credits_as_dash(identity_balance)
+                "Insufficient identity balance. You can send up to {} after the estimated fee of {}.",
+                format_credits_as_dash(max_sendable),
+                format_credits_as_dash(estimated_fee)
             ));
         }

Based on learnings: "When modeling transfers in Dash Platform (identity-to-identity), ensure the transfer amount considers estimated transaction fees; set a practical minimum based on fee requirements so that a transfer doesn't fail due to insufficient credits. Validate in UI and backend calculations where transfers are initiated."

🤖 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 1723 - 1759, The
send_identity_to_identity function currently allows amount_credits equal to
identity_balance which fails when backend adds transfer fees; before accepting
the amount, call the same estimate_credit_transfer(...) used by the amount
widget to get the required fee/reserve and enforce that amount_credits +
estimated_fee <= qualified_identity.identity.balance() (or equivalently require
amount_credits <= identity_balance - estimated_fee), returning a clear Err if
not; keep the existing IdentityTask::Transfer dispatch but only after the
reserve check passes.

src/model/wallet/asset_lock_transaction.rs (1)

116-156: ⚠️ Potential issue | 🟠 Major

Let the fee/UTXO retry converge for address-scoped selections.

source_address makes single-address UTXO sets a real path here, and those can be much more fragmented than the whole wallet. This helper still retries only once after the first fee bump, so the second pass can pull in extra inputs, increase the real fee again, and return a false insufficient-funds error even though the selected address still has more spendable UTXOs. Please loop until the input count and fee stabilize, or selection truly fails.

🤖 Prompt for AI Agents

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

In `@src/model/wallet/asset_lock_transaction.rs` around lines 116 - 156,
select_utxos_with_fee_retry currently only retries once, which can still fail
for address-scoped UTXO sets; change the fixed for 0..2 retry to a loop that
continues until convergence: call select_unspent_utxos_for and then
calculate_asset_lock_fee, and if calculate_asset_lock_fee returns Ok return it;
on Err recompute fee_estimate using estimate_tx_size (as you already do), but
continue looping and only bail out with the "Not enough spendable funds..."
error when either select_unspent_utxos_for truly returns None or the computed
fee_estimate and resulting selected UTXO set stabilize (no change in
fee_estimate or utxos between iterations) indicating further retries won't help;
keep using MIN_ASSET_LOCK_FEE, and reference select_utxos_with_fee_retry,
select_unspent_utxos_for, calculate_asset_lock_fee, fee_estimate,
MIN_ASSET_LOCK_FEE, and estimate_tx_size to locate and update the logic.

♻️ Duplicate comments (2)

src/app.rs (1)

1065-1069: ⚠️ Potential issue | 🟠 Major

Keep epoch-info warm-up off the normal screen task-result path.

On Line 1066 this internal refresh is dispatched via handle_backend_task, so its success/error can be routed to the currently visible screen and surface unrelated UI messages. Please move this to a silent internal refresh path (or a dedicated result variant consumed by AppState without display_task_result() routing).

As per coding guidelines, "Results return via MPSC channel as TaskResult. Main update() loop polls receiver each frame and routes results to visible screen's display_task_result()."

🤖 Prompt for AI Agents

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

In `@src/app.rs` around lines 1065 - 1069, The internal epoch-info refresh is
being dispatched via handle_backend_task using
BackendTask::PlatformInfo(PlatformInfoTaskRequestType::CurrentEpochInfo), which
causes its TaskResult to be routed to the visible screen via
display_task_result; instead, dispatch this refresh through the silent/internal
path: either send it through the existing MPSC channel as a distinct,
non-displayed TaskResult variant that AppState consumes directly (so update()
polls the receiver but AppState handles that variant without calling
display_task_result), or add a dedicated internal method on AppState (e.g.,
refresh_epoch_info_silently) that performs the work or submits an internal-only
backend request; ensure handle_backend_task is not used for this call and that
the TaskResult variant for epoch warm-up is consumed by AppState without
invoking display_task_result.

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

1751-1759: ⚠️ Potential issue | 🟠 Major

Do not leave self-transfer prevention in the UI layer only.

This blocks the wallet screen, but the backend transfer handler and the MCP identity-transfer tool can still submit IdentityTask::Transfer without the same equality check. Please enforce the invariant in the backend task path too and keep this guard as the UX layer.

🤖 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 1751 - 1759, The UI currently
prevents self-transfers but the invariant must also be enforced in the backend:
add the same equality check where IdentityTask::Transfer is handled (the backend
task handler that matches IdentityTask::Transfer and the MCP identity-transfer
command path that constructs/executes transfers), return an error if
qualified_identity.identity.id() == to_identity_id, and ensure the handler does
not proceed to debit/credit or schedule the transfer; keep the UI check as-is
but make the backend (the function/method that processes IdentityTask::Transfer
and the MCP transfer command) authoritative by returning a clear error result
when the IDs are equal.

🧹 Nitpick comments (1)

src/model/wallet/mod.rs (1)

2957-3099: Add one regression test for Some(&Address) UTXO filtering.

All of the updated select_unspent_utxos_for(...) tests still pass None, so the new source-address filtering path this PR depends on is not pinned here yet. A two-address case that passes Some(&address) would make sure UTXOs from other addresses are excluded.

🤖 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 2957 - 3099, Add a regression test that
exercises the source-address filtering branch of select_unspent_utxos_for by
constructing a wallet with UTXOs on two different addresses (use test_wallet or
test_wallet_with_utxo plus add_utxo and test_address), then call
wallet.select_unspent_utxos_for(amount, fee, <allow flag>, Some(&addr)) with one
of the addresses and assert the returned utxos only include entries from that
address (and exclude the other), plus verify change/total behavior as in
existing tests; place it alongside the other select_unspent_utxos_for tests.

🤖 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/shielded/bundle.rs`:
- Around line 766-782: The final fallback returns (notes, total, exact_fee)
without verifying funds; change the logic in the retry loop around
select_notes_for_amount and shielded_fee_for_actions (use variables
fee_estimate, notes, total, exact_fee, min_actions, shielded_state, amount,
platform_version) so you keep iterating until the computed exact_fee and the
selected notes/input count stabilizes (i.e., fee_estimate == exact_fee and total
>= amount + exact_fee) or the retry cap is hit; if after the cap the selection
is still underfunded (total < amount + exact_fee) return
TaskError::ShieldedInsufficientBalance instead of returning an underfunded
(notes, total, exact_fee). Ensure the final return only occurs when total >=
amount + exact_fee and otherwise return the ShieldedInsufficientBalance error.

In `@src/model/fee_estimation.rs`:
- Around line 271-281: The platform fee calculation in
estimate_shield_from_core_fees_duffs uses
min_fees.address_funding_asset_lock_cost directly and converts to duffs before
applying the 20% buffer, but it also omits applying fee_multiplier_permille;
update the calculation to first apply self.fee_multiplier_permille to
self.min_fees.address_funding_asset_lock_cost (scaling by permille), then
convert the result to duffs using CREDITS_PER_DUFF, and finally apply the 20%
buffer so that the platform_fee_duffs reflects the network fee multiplier and
buffer (referencing estimate_shield_from_core_fees_duffs,
self.fee_multiplier_permille, self.min_fees.address_funding_asset_lock_cost, and
CREDITS_PER_DUFF).

In `@src/ui/wallets/shield_screen.rs`:
- Around line 179-188: When a sequential batch starts we must snapshot/freeze
the source and amount so later iterations don't read live widget state; update
the batch start path to copy self.amount.value() and
self.selected_platform_address() into dedicated persisted fields (e.g.
batch_amount and batch_platform_addr) and have queue_next_sequential(),
make_shield_credits_task and any other batch-queuing code read those frozen
fields instead of self.amount or selected_platform_address(); ensure
batch_remaining and pending_next_task still decrement/queue as before and that
these frozen fields are set when entering Status::BatchInProgress and cleared
when the batch finishes or is cancelled.
- Around line 533-551: The Complete branch currently overwrites action (losing
any queued self.pending_refresh_task) and display_message() can reset status to
NotStarted after a refresh failure, so change the "Done" handling to preserve
pending tasks and avoid reverting a completed shield: in the
island_central_panel closure where self.status == Status::Complete, replace
action = AppAction::PopScreen with action |= AppAction::PopScreen so any
previously queued AppAction::BackendTask (from self.pending_refresh_task) is
kept; also adjust display_message() (or its caller path where it sets status =
Status::NotStarted) to only reset status when the current status is not
Status::Complete (or only clear status after explicitly cancelling/completing),
ensuring Status::Complete remains visible if a background SyncNotes task fails.

---

Outside diff comments:
In `@src/model/wallet/asset_lock_transaction.rs`:
- Around line 116-156: select_utxos_with_fee_retry currently only retries once,
which can still fail for address-scoped UTXO sets; change the fixed for 0..2
retry to a loop that continues until convergence: call select_unspent_utxos_for
and then calculate_asset_lock_fee, and if calculate_asset_lock_fee returns Ok
return it; on Err recompute fee_estimate using estimate_tx_size (as you already
do), but continue looping and only bail out with the "Not enough spendable
funds..." error when either select_unspent_utxos_for truly returns None or the
computed fee_estimate and resulting selected UTXO set stabilize (no change in
fee_estimate or utxos between iterations) indicating further retries won't help;
keep using MIN_ASSET_LOCK_FEE, and reference select_utxos_with_fee_retry,
select_unspent_utxos_for, calculate_asset_lock_fee, fee_estimate,
MIN_ASSET_LOCK_FEE, and estimate_tx_size to locate and update the logic.

In `@src/ui/wallets/send_screen.rs`:
- Around line 1723-1759: The send_identity_to_identity function currently allows
amount_credits equal to identity_balance which fails when backend adds transfer
fees; before accepting the amount, call the same estimate_credit_transfer(...)
used by the amount widget to get the required fee/reserve and enforce that
amount_credits + estimated_fee <= qualified_identity.identity.balance() (or
equivalently require amount_credits <= identity_balance - estimated_fee),
returning a clear Err if not; keep the existing IdentityTask::Transfer dispatch
but only after the reserve check passes.

---

Duplicate comments:
In `@src/app.rs`:
- Around line 1065-1069: The internal epoch-info refresh is being dispatched via
handle_backend_task using
BackendTask::PlatformInfo(PlatformInfoTaskRequestType::CurrentEpochInfo), which
causes its TaskResult to be routed to the visible screen via
display_task_result; instead, dispatch this refresh through the silent/internal
path: either send it through the existing MPSC channel as a distinct,
non-displayed TaskResult variant that AppState consumes directly (so update()
polls the receiver but AppState handles that variant without calling
display_task_result), or add a dedicated internal method on AppState (e.g.,
refresh_epoch_info_silently) that performs the work or submits an internal-only
backend request; ensure handle_backend_task is not used for this call and that
the TaskResult variant for epoch warm-up is consumed by AppState without
invoking display_task_result.

In `@src/ui/wallets/send_screen.rs`:
- Around line 1751-1759: The UI currently prevents self-transfers but the
invariant must also be enforced in the backend: add the same equality check
where IdentityTask::Transfer is handled (the backend task handler that matches
IdentityTask::Transfer and the MCP identity-transfer command path that
constructs/executes transfers), return an error if
qualified_identity.identity.id() == to_identity_id, and ensure the handler does
not proceed to debit/credit or schedule the transfer; keep the UI check as-is
but make the backend (the function/method that processes IdentityTask::Transfer
and the MCP transfer command) authoritative by returning a clear error result
when the IDs are equal.

---

Nitpick comments:
In `@src/model/wallet/mod.rs`:
- Around line 2957-3099: Add a regression test that exercises the source-address
filtering branch of select_unspent_utxos_for by constructing a wallet with UTXOs
on two different addresses (use test_wallet or test_wallet_with_utxo plus
add_utxo and test_address), then call wallet.select_unspent_utxos_for(amount,
fee, <allow flag>, Some(&addr)) with one of the addresses and assert the
returned utxos only include entries from that address (and exclude the other),
plus verify change/total behavior as in existing tests; place it alongside the
other select_unspent_utxos_for tests.

🪄 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: 61d53386-c478-4d7b-afeb-16307d92ac8f

📥 Commits

Reviewing files that changed from the base of the PR and between 97230f2 and b22f370.

📒 Files selected for processing (22)

• CLAUDE.md
• src/app.rs
• src/backend_task/core/create_asset_lock.rs
• src/backend_task/identity/register_identity.rs
• src/backend_task/identity/top_up_identity.rs
• src/backend_task/shielded/bundle.rs
• src/backend_task/shielded/mod.rs
• src/backend_task/wallet/fund_platform_address_from_wallet_utxos.rs
• src/context/shielded.rs
• src/mcp/tools/shielded.rs
• src/model/fee_estimation.rs
• src/model/wallet/asset_lock_transaction.rs
• src/model/wallet/mod.rs
• src/model/wallet/utxos.rs
• src/ui/mod.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/shield_screen.rs
• src/ui/wallets/shielded_tab.rs
• src/ui/wallets/wallets_screen/mod.rs

💤 Files with no reviewable changes (2)

• src/ui/wallets/shield_credits_screen.rs
• src/ui/wallets/shield_from_asset_lock_screen.rs

[Copilot]

Pull request overview

Copilot reviewed 22 out of 22 changed files in this pull request and generated 3 comments.

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

Fee headroom is computed using PlatformVersion::latest(), which can diverge from the network-specific version the app is configured to use (AppContext::platform_version()). This can cause incorrect max-amount calculations (and potentially user-visible failures when using Max) on networks where the latest SDK version doesn’t match the active platform version. Use self.app_context.platform_version() here (and consider keeping all fee estimation decisions inside model/fee_estimation.rs per CLAUDE.md).

Suggested change

	let fee_headroom =
	shielded_fee_for_actions(2, PlatformVersion::latest())
	.saturating_mul(2);
	let fee_headroom = shielded_fee_for_actions(
	2,
	self.app_context.platform_version(),
	)
	.saturating_mul(2);

[Copilot]

The new test hard-codes absolute fee bounds and assumes roughly linear per-action pricing. If the protocol fee schedule changes in a future platform version, this test may start failing for reasons unrelated to this crate’s logic. Consider limiting assertions to invariants that should remain true across versions (e.g., monotonic increase and non-zero), or pinning to a specific platform version/fee schedule in the test setup.

Suggested change

	// Fees should be positive and increase with action count
	assert!(fee_2 > 0, "fee for 2 actions should be positive");
	assert!(fee_3 > fee_2, "fee for 3 actions should exceed fee for 2");
	assert!(fee_5 > fee_3, "fee for 5 actions should exceed fee for 3");
	assert!(fee_10 > fee_5, "fee for 10 actions should exceed fee for 5");
	// Sanity bounds: fee for 2 actions should be in a reasonable range
	assert!(
	fee_2 > 50_000_000,
	"fee for 2 actions should be at least 50M credits"
	);
	assert!(
	fee_2 < 1_000_000_000,
	"fee for 2 actions should be under 1B credits"
	);
	// Fee growth should be roughly linear (per-action cost is constant)
	let per_action_cost_low = (fee_5 - fee_2) / 3;
	let per_action_cost_high = (fee_10 - fee_5) / 5;
	let ratio = per_action_cost_low as f64 / per_action_cost_high as f64;
	assert!(
	(0.8..=1.2).contains(&ratio),
	"per-action cost should be roughly constant, got ratio {ratio}"
	);
	// Invariants that should hold across platform versions:
	// - Fees are positive.
	// - Fees increase with the number of actions.
	assert!(fee_2 > 0, "fee for 2 actions should be positive");
	assert!(fee_3 > fee_2, "fee for 3 actions should exceed fee for 2");
	assert!(fee_5 > fee_3, "fee for 5 actions should exceed fee for 3");
	assert!(fee_10 > fee_5, "fee for 10 actions should exceed fee for 5");

[Copilot]

The new source_address parameter changes UTXO selection behavior, but the test suite here only exercises the None (no restriction) path. Add a focused test that passes Some(&addr) and verifies selection is restricted to that address (and returns None when the address-specific balance is insufficient). This will guard the new shielding-from-core address filtering behavior against regressions.