Skip to content

refactor(send): move route fee estimation out of the SwiftUI view layer - #1061

Open
QuantumExplorer wants to merge 1 commit into
developfrom
claude/modest-shamir-57ae41
Open

refactor(send): move route fee estimation out of the SwiftUI view layer#1061
QuantumExplorer wants to merge 1 commit into
developfrom
claude/modest-shamir-57ae41

Conversation

@QuantumExplorer

Copy link
Copy Markdown
Member

Issue being fixed or feature implemented

CodeRabbit flagged on #1057 that SendConfirmSheet.networkFeeCredits calls PlatformWalletManager.estimateShieldedFee (FFI) and performs per-route fee calculations inside a SwiftUI View — a pattern the repo's SwiftUI-first guardrails ban from View structs. The pattern predates #1057 (that PR only added the .platformToShielded case following the existing convention), so the refactor was declined there as out of scope and split into this PR.

What was done?

  • SendViewModel now owns both confirm-sheet computations:
    • confirmNetworkFeeCredits — the per-route fee switch. The pool-spending routes reuse the existing shieldedFeeKind(for:) mapping instead of duplicated estimateShieldedFee calls, .platformToPlatform reuses the existing platformTransferFeeReserveCredits constant instead of a bare 100_000_000 literal, .platformToCore reads withdrawalPreflight?.estimatedFee directly, and .platformToShielded keeps feat(wallet): pay external shielded addresses from the Platform balance #1057's flat 2-action estimate.
    • confirmTotalDuffs — the Total row's Core → Shielded lock-value derivation (CoreToShieldedAmountPolicy is FFI-backed too, so leaving it would have kept an FFI call in the view).
  • SendConfirmSheet receives the two values as stored let properties and only formats them. SendScreen.swift is now free of PlatformWalletManager / CoreToShieldedAmountPolicy references.
  • Behavior is identical route by route: same estimators, same values, same fallbacks. Update timing is unchanged — the sheet's parameters were already re-evaluated through the observed view model (the same mechanism as the existing withdrawalFeeCredits parameter).

How Has This Been Tested?

Clean dashpay scheme simulator build (-sdk iphonesimulator, arm64) on top of current develop (includes #1057). Pure code motion with no rendering change, verified by diff review of each route's estimator against the previous switch. The unit-test target is broken pre-existing on this branch lineage, so no new tests were added.

Breaking Changes

None.

Checklist:

  • I have performed a self-review of my own code
  • I have commented my code, particularly in hard-to-understand areas
  • I have added or updated relevant unit/integration/functional/e2e tests
  • I have made corresponding changes to the documentation

For repository code-owners and collaborators only

  • I have assigned this pull request to a milestone

🤖 Generated with Claude Code

SendConfirmSheet computed its Network fee row inside the View — calling
PlatformWalletManager.estimateShieldedFee (FFI) per route and deriving
the Core → Shielded lock total with CoreToShieldedAmountPolicy — which
the SwiftUI-first guardrails ban from View structs. Flagged by
CodeRabbit on PR #1057, where it was out of scope (the pattern predates
that PR).

SendViewModel now owns both computations (confirmNetworkFeeCredits,
confirmTotalDuffs), reusing its existing shieldedFeeKind(for:) route
mapping and platformTransferFeeReserveCredits constant instead of the
sheet's duplicated switch and bare 100_000_000 literal. The sheet
receives the two values as stored properties and only formats them.
Estimates and displayed strings are unchanged route by route.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@coderabbitai

coderabbitai Bot commented Aug 25, 2026

Copy link
Copy Markdown

Warning

Review limit reached

Next included review available in 21 minutes.

View limit details

Limit details: You’ve used the included review currently available.

You've used all free OSS reviews for now. Wait for the free limit to reset to keep reviewing this public repository.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: d9156692-8b67-43af-b3df-ab6a1fa81f4d

📥 Commits

Reviewing files that changed from the base of the PR and between 53bb20d and 2722b29.

📒 Files selected for processing (2)
  • DashWallet/Sources/UI/Payments/Pay/SendScreen.swift
  • DashWallet/Sources/UI/Payments/Pay/SendViewModel.swift

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

Comment @coderabbitai help to get the list of available commands.

@thepastaclaw

thepastaclaw commented Aug 31, 2026

Copy link
Copy Markdown

🕓 Ready for review — 8 ahead in queue (commit 2722b29)
Queue position: 9/42 · 2 reviews active
ETA: start ~21:25 UTC · complete ~22:25 UTC (median 1h across 30 recent reviews; 2 slots)
Queued 6h 41m ago · Last checked: 2026-08-31 17:20 UTC

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants