Skip to content

Zcash Ironwood migration - #9404

Merged
omurovch merged 17 commits into
masterfrom
zcash-ironwood-hotfix
Jul 27, 2026
Merged

Zcash Ironwood migration#9404
omurovch merged 17 commits into
masterfrom
zcash-ironwood-hotfix

Conversation

@omurovch

@omurovch omurovch commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

#9367

Summary by CodeRabbit

  • New Features
    • Added support for migrating Zcash funds to Ironwood.
    • Introduced a one-time “Migration Required” alert and balance indicator for affected wallets.
    • Added migration UX: required cell → migration bottom sheet → confirmation screen with progress and error handling.
    • Labeled migration transactions in transaction history with the new “Migrate to Ironwood” direction.
  • Improvements
    • Enhanced Zcash balance totals and migration-related calculations.
    • Updated localization and migration UI visuals.
    • Updated app version to 0.49.4.

omurovch added 11 commits July 25, 2026 15:01
Prepare for NU6.3 activation (mainnet 2026-07-28, block 3428143):
depend on our Ironwood-capable SDK fork build via JitPack (branch
ironwood-hotfix) until ECC tags an official release, and include
the new Ironwood pool in the shielded balance aggregation, which
previously summed only Sapling + Orchard and would silently hide
Ironwood funds.
Once the chain tip passes the NU6.3 activation height (3428143
mainnet, 4134000 testnet) and the wallet still holds an Orchard
balance, show a Migration Required row with that amount on the ZEC
token page. Tapping it opens a Migration to Ironwood sheet with a
Migrate button — the entry point for the upcoming Orchard ->
Ironwood migration flow.
Pop the Migration to Ironwood sheet once per session on the balance
tab when a Zcash wallet is enabled and the adapter reports a
migration-required Orchard balance after NU6.3 activation. The
BackupRequiredAlert takes priority: the migration alert defers
until the account has a backup. The sheet composable moves to
modules/balance/ui so the token page and balance tab share it.
Both Migrate buttons open a send-style confirmation page that
proposes an immediate Orchard -> Ironwood migration through
OrchardMigrationSdk, showing the net migrated amount, the implied
fee (Orchard total minus proposed transfers), and a warning that
the migrated amount is publicly visible on-chain. The proposed
schedule is retained on the adapter for the execution step, which
is still stubbed behind the Migrate button.
The Migrate confirmation now signs and broadcasts the retained
immediate-migration schedule (spending key derived per-call from
the seed, as ordinary sends do) and records the broadcast txid in
local storage. Transaction history uses those recorded txids to
label migrations ("Migrate / to Ironwood"); a self-transfer with a
transparent output is an Unshield, and any other self-transfer —
including migrations of a wallet restored from seed, where the
recorded txids are absent — now shows as sent to own address
instead of the misleading Unshield label.
The upstream OrchardMigrationSdk immediate path returns the
staggered ZIP 318 schedule (the documented single-transfer contract
was lost when the SDK was rewired onto the merged
zcash_pool_migration engine), so implement immediate migration
through the ordinary send path instead: post-NU6.3 the Orchard
receiver routes outputs into the Ironwood pool, making a
full-balance transfer to the wallet's own unified address a
single-transaction migration. The proposal converges on the exact
sweep fee (proposeTransfer reports insufficient balance by
throwing, so grow a fee reserve until it succeeds, then tighten);
execution reuses the ordinary send path and records the txid for
history labeling. The SDK-backed schedule proposal stays available
as proposeImmediateMigrationSchedule for the future staggered flow.
A recorded migration txid now classifies the transaction on its
own: once the transaction is mined and re-scanned from the chain,
recipients are no longer reported as the wallet's own account, so
the internal-transaction heuristic stops matching and the migration
degraded to a plain send showing only the fee. Also size the
migration list icon to match the shield icons.
Fork branch ironwood-hotfix2: the ZIP 318 engine squash adapted to
librustzcash main (anchor-checkpoint retention, live-testing scan
fixes, round shuffled denominations), replacing the pre-merge
engine build. Same artifacts as the local
2.6.5-ironwoodlocal-2-SNAPSHOT.
Mid-sync the Orchard balance is provisional — notes spent in blocks
the scan has not reached yet still count as available — so the
Migration Required row and the balance-tab alert appeared during
sync with an inflated amount. Report the migration-required balance
only when the adapter is Synced.
ECC's release delivers Ironwood support and the migration as a
single Synchronizer method: proposeOrchardToIronwoodMigration, an
all-or-nothing sweep of the account's Orchard balance to its own
internal receiver with the fee computed for zero remainder (the
post-NU6.3 Orchard turnstile forbids returning change). This
replaces both the app-side fee-convergence sweep and the retired
OrchardMigrationSdk surface; execution still goes through the
ordinary send path with txid recording. The artifact resolves from
Maven Central; the fork/JitPack pin remains in history as fallback.
@coderabbitai

coderabbitai Bot commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Adds Zcash Ironwood migration support across adapter logic, transaction history, persisted storage, balance alerts, confirmation and execution screens, navigation, resources, SDK configuration, and application release metadata.

Changes

Zcash Ironwood migration

Layer / File(s) Summary
Migration state and adapter operations
gradle/libs.versions.toml, app/src/main/java/io/horizontalsystems/bankwallet/core/...
The Zcash SDK is updated, migration transaction IDs are persisted, Ironwood balances are included, and proposal/execution APIs are added.
Migration transaction classification
app/src/main/java/io/horizontalsystems/bankwallet/core/adapters/zcash/..., app/src/main/java/io/horizontalsystems/bankwallet/entities/..., app/src/main/java/io/horizontalsystems/bankwallet/modules/transactions/...
Transaction outputs and migration predicates are mapped, Ironwood direction classification is added, and migration-specific subtitles are displayed.
Balance migration alerts
app/src/main/java/io/horizontalsystems/bankwallet/modules/balance/...
Balance state exposes migration data, renders migration cells and bottom sheets, and navigates selected wallets to confirmation.
Migration confirmation and execution
app/src/main/java/io/horizontalsystems/bankwallet/modules/zcashmigration/..., app/src/main/res/navigation/main_graph.xml
A migration destination, factory, view model, confirmation screen, proposal loading, execution, and success or error handling are added.
Migration presentation and release metadata
app/src/main/res/values/strings.xml, translation_snapshot.json, app/src/main/res/drawable/ic_migrate_24.xml, app/build.gradle.kts
Migration strings, localization entries, an icon resource, and app version metadata are updated.

Estimated code review effort: 4 (Complex) | ~60 minutes

Sequence Diagram(s)

sequenceDiagram
  participant BalanceViewModel
  participant ZcashAdapter
  participant ZcashMigrationConfirmScreen
  participant ZcashMigrationViewModel
  BalanceViewModel->>ZcashAdapter: read ironwoodMigrationRequiredBalance
  ZcashAdapter-->>BalanceViewModel: return required amount
  ZcashMigrationConfirmScreen->>ZcashMigrationViewModel: start migration
  ZcashMigrationViewModel->>ZcashAdapter: executeIronwoodMigration
  ZcashAdapter-->>ZcashMigrationViewModel: return transaction ID
Loading

Possibly related PRs

Suggested reviewers: abdrasulov, ant013

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.53% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title clearly summarizes the main change: adding Zcash Ironwood migration support.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
✨ 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 zcash-ironwood-hotfix

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.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 10

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@app/src/main/java/io/horizontalsystems/bankwallet/core/adapters/zcash/ZcashAdapter.kt`:
- Line 373: Update the migration transaction ID update in ZcashAdapter to
perform an atomic synchronized read-modify-write on the shared preference Set,
preventing concurrent account migrations from overwriting each other’s IDs.
Preserve the lowercase normalization and existing zcashMigrationTransactionIds
storage behavior.
- Around line 139-151: Align ironwoodMigrationRequiredBalance with
proposeIronwoodMigration by using only the Orchard available balance when
determining the migratable amount. Update the balance calculation and
positive-value check in ironwoodMigrationRequiredBalance, preserving the
existing sync, activation-height, and nullability guards.
- Around line 219-224: Update the migration transaction ID matching callback in
ZcashTransactionsProvider to normalize both txHash.toRawHexString() and
txHash.toReversedHex() to lowercase before comparing with
localStorage.zcashMigrationTransactionIds; apply the same normalization at the
other migration-ID comparison around the referenced secondary location.
- Around line 367-375: Update executeIronwoodMigration() and the send flow it
relies on to retain every successful transaction ID from
TransactionSubmitResult.Success, rather than only firstTxHash. Record all
returned txids in localStorage.zcashMigrationTransactionIds, preserving
lowercase normalization and ensuring each successfully submitted migration
transaction is available for MigrateToIronwood labeling.

In
`@app/src/main/java/io/horizontalsystems/bankwallet/modules/balance/BalanceViewModel.kt`:
- Around line 205-218: Gate migration prompts on synchronized balances: in
BalanceViewModel.checkZcashMigrationRequired, retain the matching Zcash
BalanceItem and return unless its adapter state is AdapterState.Synced before
setting zcashMigrationAlertWallet; in TokenBalanceViewModel, assign
zcashMigrationRequiredAmount only for a synchronized balance item and reset it
to null otherwise.

In
`@app/src/main/java/io/horizontalsystems/bankwallet/modules/balance/token/TokenBalanceScreen.kt`:
- Around line 250-255: Update the MigrationRequiredCell invocation in the
TokenBalanceScreen migration amount block to pass balanceViewItem.balanceHidden,
then update MigrationRequiredCell to render the amount using the app’s standard
masked-value presentation whenever that flag is true while preserving the raw
amount when balances are visible.

In
`@app/src/main/java/io/horizontalsystems/bankwallet/modules/zcashmigration/ZcashMigrationConfirmScreen.kt`:
- Around line 40-66: Move the SendResult.Sent success message and
SendResult.Failed bottom-sheet navigation from the composable body into the
existing LaunchedEffect(sendResult) block in the Zcash migration confirmation
screen. Handle each state once per sendResult change, preserving the 1200ms
delay and back-stack pop for Sent while avoiding repeated snackbar or
bottom-sheet actions during recomposition.

In
`@app/src/main/java/io/horizontalsystems/bankwallet/modules/zcashmigration/ZcashMigrationViewModel.kt`:
- Around line 24-28: Refactor ZcashMigrationViewModel to inherit
ViewModelUiState<ZcashMigrationUiState> instead of ViewModel, and introduce an
exported ZcashMigrationUiState containing coinRate, sendResult, amount, fee, and
error. Implement createState() with the initial values, replace the independent
mutableStateOf fields with state updates through emitState(), and update all
accesses to use the consolidated state.
- Around line 54-63: In ZcashMigrationViewModel, update both the proposal flow
around adapter.proposeIronwoodMigration() and the migration flow at the sibling
site to rethrow CancellationException before handling other Throwable failures.
Keep cancellation from being logged or assigned to error, while preserving the
existing operational-failure logging and error mapping.

In `@app/src/main/res/values/strings.xml`:
- Around line 471-472: Update the Zcash_Migration_PubliclyVisible_Description
string to remove the claim that sender, recipient, and user addresses remain
private; instead distinguish on-chain amount/time visibility from network-layer
privacy and clearly present the potential linkage through lightwallet servers or
network operators before consent.
🪄 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 Plus

Run ID: f840fa03-add5-447f-9f6b-2c703308b6c0

📥 Commits

Reviewing files that changed from the base of the PR and between d886b52 and 590cd07.

⛔ Files ignored due to path filters (9)
  • app/src/main/res/values-de/strings.xml is excluded by !**/res/values-*/strings.xml
  • app/src/main/res/values-es/strings.xml is excluded by !**/res/values-*/strings.xml
  • app/src/main/res/values-fa/strings.xml is excluded by !**/res/values-*/strings.xml
  • app/src/main/res/values-fr/strings.xml is excluded by !**/res/values-*/strings.xml
  • app/src/main/res/values-ko/strings.xml is excluded by !**/res/values-*/strings.xml
  • app/src/main/res/values-pt-rBR/strings.xml is excluded by !**/res/values-*/strings.xml
  • app/src/main/res/values-ru/strings.xml is excluded by !**/res/values-*/strings.xml
  • app/src/main/res/values-tr/strings.xml is excluded by !**/res/values-*/strings.xml
  • app/src/main/res/values-zh/strings.xml is excluded by !**/res/values-*/strings.xml
📒 Files selected for processing (23)
  • app/build.gradle.kts
  • app/src/main/java/io/horizontalsystems/bankwallet/core/Interfaces.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/core/adapters/zcash/ZcashAdapter.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/core/adapters/zcash/ZcashTransaction.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/core/adapters/zcash/ZcashTransactionsProvider.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/core/managers/LocalStorageManager.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/entities/transactionrecords/zcash/ZcashShieldingTransactionRecord.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/modules/balance/BalanceViewModel.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/modules/balance/token/TokenBalanceModule.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/modules/balance/token/TokenBalanceScreen.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/modules/balance/token/TokenBalanceViewModel.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/modules/balance/ui/BalanceForAccount.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/modules/balance/ui/ZcashMigrationBottomSheet.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/modules/transactions/TransactionViewItemFactory.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/modules/zcashmigration/ZcashMigrationConfirmFragment.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/modules/zcashmigration/ZcashMigrationConfirmScreen.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/modules/zcashmigration/ZcashMigrationModule.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/modules/zcashmigration/ZcashMigrationViewModel.kt
  • app/src/main/res/drawable/ic_migrate_24.xml
  • app/src/main/res/navigation/main_graph.xml
  • app/src/main/res/values/strings.xml
  • gradle/libs.versions.toml
  • translation_snapshot.json

Comment on lines +471 to +472
<string name="Zcash_Migration_PubliclyVisible_Title">Publicly Visible Amount</string>
<string name="Zcash_Migration_PubliclyVisible_Description">Moving funds between pools requires the migrated amount to be publicly visible on the blockchain. The sender, recipient and your addresses remain private.</string>

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🔒 Security & Privacy | 🟠 Major | ⚡ Quick win

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "Files matching strings.xml:"
fd -a 'strings\.xml$' . | sed 's#^\./##'

echo
echo "Relevant string:"
grep -n "Zcash_Migration_PubliclyVisible" app/src/main/res/values/strings.xml || true

echo
echo "Related ironwood/zcash migration strings:"
sed -n '440,490p' app/src/main/res/values/strings.xml

echo
echo "Search references to Zcash_Migration_PubliclyVisible:"
rg -n "Zcash_Migration_PubliclyVisible" .

Repository: horizontalsystems/unstoppable-wallet-android

Length of output: 13684


🌐 Web query:

ZIP 318 mandatory user disclosure before pool-crossing transfer timing public network-layer correlation broadcast IP

💡 Result:

ZIP 318, titled "Orchard to Ironwood Migration," specifies best practices for migrating Zcash funds between shielded pools (specifically from Orchard to Ironwood) while mitigating privacy risks [1][2]. The document explicitly addresses the risk of network-layer correlation during pool-crossing transfers [3][1][4]. When funds move between pools, the transaction amount and timing become visible on-chain [3][4]. Without network-level privacy, an observer—such as a light wallet server or a network operator—can correlate the broadcast IP address of the transaction with the on-chain pool-crossing event, which can lead to de-anonymization [3][4]. To mitigate these risks, ZIP 318 mandates specific user disclosures and privacy measures: 1. Mandatory User Disclosure: Wallets are required to provide a disclaimer that explains the IP-correlation risk associated with pool-crossing transfers [3][1]. This ensures that users are informed that even if the transaction amount is obscured through batching or other techniques, the broadcast IP can link the transfer to their wallet if network-level privacy is not utilized [3][4]. 2. Network Privacy Options: Wallets are required to offer users network-privacy options, such as Tor, to decouple the broadcast IP from the user's identity [3][1][5]. The ZIP specifies that wallets must offer Tor and, where available, other privacy networks like Nym [5]. If these options are unavailable, wallets are advised to suggest using a trusted VPN as a fallback [3][5]. 3. Decoupled Synchronization and Broadcast: To further harden privacy, the migration process is designed as a two-phase flow (note-splitting and scheduled broadcast) where synchronization with the light client server is decoupled in time from the actual broadcast of migration transactions [3][6][1]. This prevents simple correlation between the user's application activity (e.g., opening the wallet) and the timing of the pool-crossing transaction [3][6]. These measures aim to maximize anonymity by ensuring that individual migration transactions are difficult to correlate to a specific user, wallet, or behavior [1].

Citations:


🌐 Web query:

Zcash ZIP 318 crosspool migration privacy disclosure amount public time public addresses shielded broadcast IP

💡 Result:

Zcash ZIP 318, titled "Orchard to Ironwood Migration," defines best practices for migrating user funds from the Orchard shielded pool to the Ironwood pool following the NU6.3 network upgrade [1][2]. Because pool-crossing transfers reveal the migrated amount on the blockchain, ZIP 318 addresses the resulting privacy implications [1]. Privacy Disclosures and Risks - Amount Disclosure: The act of moving funds between shielded pools (a "turnstile" migration) inherently reveals the amount being transferred on-chain [1]. ZIP 318 mitigates the risk of linking these amounts to a specific wallet by recommending that wallets decompose balances into canonical denominations (e.g., powers of ten) and split them into multiple smaller transactions [1][2]. - IP Address Disclosure: A significant privacy risk identified is the potential for the server or network operator receiving a transaction broadcast to correlate the user's IP address with the on-chain pool-crossing event [1][3]. - Public Addresses: The migration process itself is designed to move funds between shielded pools; it does not involve the public disclosure of a user's shielded addresses on the blockchain [1]. Mitigation Requirements - Network-Level Privacy: To address IP correlation risks, ZIP 318 requires wallets to offer an opt-in network anonymity layer, such as Tor, and mandates the disclosure of the residual IP-correlation risk to the user [1][4]. Wallets are prohibited from having these network-privacy options pre-selected by default [4][3]. - Disclosure/Disclaimer: Wallets are required to display a disclaimer to the user explaining that pool-crossing transfers are visible on-chain and informing them of the associated IP-correlation risk if network-level privacy (like Tor) is not utilized [1][4]. - User Consent: The migration schedule and the associated risks must be presented to the user, and explicit consent is required before any migration transaction is broadcast [1][4][2]. ZIP 318 emphasizes that these measures are intended to create "cohorts" of users whose transactions share similar characteristics (e.g., timing, canonical amounts), thereby providing "cover" for individual users and making it significantly harder for observers to link specific transfers to a single wallet or individual activity [1][3].

Citations:


Don’t advertise that addresses remain private.

Pool-crossing migration still links the amount and transaction time on-chain, and the broadcast connection/relay may be correlated with that migration by a lightwallet server or network operator. Re-word the privacy notice to separate on-chain shielding from network-layer privacy and present the remaining linkage risk before consent.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@app/src/main/res/values/strings.xml` around lines 471 - 472, Update the
Zcash_Migration_PubliclyVisible_Description string to remove the claim that
sender, recipient, and user addresses remain private; instead distinguish
on-chain amount/time visibility from network-layer privacy and clearly present
the potential linkage through lightwallet servers or network operators before
consent.

abdrasulov
abdrasulov previously approved these changes Jul 27, 2026
omurovch added 5 commits July 27, 2026 12:40
The alert balance summed available + pending Orchard funds while
the migration proposal can only sweep spendable notes — and the
all-or-nothing proposal fails outright while any note is pending.
The row could therefore advertise an amount the confirmation screen
cannot migrate, and keep showing a residual right after a
successful migration. Use the available balance for the alerts,
matching the proposal exactly.
A proposal can be executed as multiple transactions, but the send
path kept only the first successful txid, so any later migration
transactions would be labeled as plain sends in history. Return
all successful txids from send() and record each of them for
MigrateToIronwood labeling.
The migration required cell showed the raw amount even with the
balance privacy toggle on. Mask it with the same ***** presentation
the neighboring locked-balance cells use.
The success snackbar and error sheet navigation ran directly in the
composable body, so recomposition while the result stayed Sent or
Failed could duplicate them. Handle both inside the existing
LaunchedEffect(sendResult) so each fires once per state change,
keeping the 1200ms delay before popping on success.
Consolidate the independent mutableStateOf fields into a single
ZcashMigrationUiState per the project's view model convention,
emitting updates through emitState().
@omurovch
omurovch merged commit 8c871e1 into master Jul 27, 2026
2 of 3 checks passed
@omurovch
omurovch deleted the zcash-ironwood-hotfix branch July 27, 2026 06:43

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 1

🧹 Nitpick comments (1)
app/src/main/java/io/horizontalsystems/bankwallet/modules/zcashmigration/ZcashMigrationViewModel.kt (1)

47-49: 🩺 Stability & Availability | 🔵 Trivial | ⚡ Quick win

Handle exchange-rate flow failures explicitly.

Verify that collectWith catches upstream exceptions; otherwise add a catch handler so a failed rate stream does not silently stop updating the UI. Preserve cancellation by rethrowing CancellationException.

As per coding guidelines, uncaught coroutine exceptions must be handled to prevent silent termination.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@app/src/main/java/io/horizontalsystems/bankwallet/modules/zcashmigration/ZcashMigrationViewModel.kt`
around lines 47 - 49, Update the exchange-rate collection around
xRateService.getRateFlow(wallet.coin.uid).collectWith in the ViewModel to handle
upstream failures explicitly, adding a catch path if collectWith does not
already do so. Handle/report non-cancellation failures without silently stopping
UI updates, while rethrowing CancellationException to preserve coroutine
cancellation.

Source: Coding guidelines

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In
`@app/src/main/java/io/horizontalsystems/bankwallet/modules/zcashmigration/ZcashMigrationViewModel.kt`:
- Around line 31-49: Serialize all mutable UI-state updates in
ZcashMigrationViewModel so createState() cannot observe partially updated fields
or emit stale snapshots. Use one dispatcher or a single immutable state holder
consistently across the rate collector and IO coroutine paths, including fee,
sendResult, amount, and error updates, while preserving the existing state
values and transitions.

---

Nitpick comments:
In
`@app/src/main/java/io/horizontalsystems/bankwallet/modules/zcashmigration/ZcashMigrationViewModel.kt`:
- Around line 47-49: Update the exchange-rate collection around
xRateService.getRateFlow(wallet.coin.uid).collectWith in the ViewModel to handle
upstream failures explicitly, adding a catch path if collectWith does not
already do so. Handle/report non-cancellation failures without silently stopping
UI updates, while rethrowing CancellationException to preserve coroutine
cancellation.
🪄 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 Plus

Run ID: edc615da-ecfb-4958-b222-c96bbea85d50

📥 Commits

Reviewing files that changed from the base of the PR and between 590cd07 and 8c871e1.

📒 Files selected for processing (4)
  • app/src/main/java/io/horizontalsystems/bankwallet/core/adapters/zcash/ZcashAdapter.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/modules/balance/token/TokenBalanceScreen.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/modules/zcashmigration/ZcashMigrationConfirmScreen.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/modules/zcashmigration/ZcashMigrationViewModel.kt
🚧 Files skipped from review as they are similar to previous changes (3)
  • app/src/main/java/io/horizontalsystems/bankwallet/modules/zcashmigration/ZcashMigrationConfirmScreen.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/modules/balance/token/TokenBalanceScreen.kt
  • app/src/main/java/io/horizontalsystems/bankwallet/core/adapters/zcash/ZcashAdapter.kt

Comment on lines +31 to +49
private var coinRate = xRateService.getRate(wallet.coin.uid)
private var sendResult: SendResult? = null
// Migration-required Orchard balance shown while the exact proposal is loading
private var amount = adapter.ironwoodMigrationRequiredBalance ?: BigDecimal.ZERO
private var fee: BigDecimal? = null
private var error: Throwable? = null

override fun createState() = ZcashMigrationUiState(
coinRate = coinRate,
sendResult = sendResult,
amount = amount,
fee = fee,
error = error,
)

init {
xRateService.getRateFlow(wallet.coin.uid).collectWith(viewModelScope) {
coinRate = it
emitState()

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟠 Major | 🏗️ Heavy lift

Serialize the concurrent UI-state updates.

createState() snapshots independent mutable fields while the rate collector and IO coroutines update those fields and call emitState(). A stale snapshot can overwrite newer data—for example, revert a loaded fee or change Sent back to Sending. Marshal all state updates onto one dispatcher or update one immutable state atomically.

Also applies to: 52-61, 71-88

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In
`@app/src/main/java/io/horizontalsystems/bankwallet/modules/zcashmigration/ZcashMigrationViewModel.kt`
around lines 31 - 49, Serialize all mutable UI-state updates in
ZcashMigrationViewModel so createState() cannot observe partially updated fields
or emit stale snapshots. Use one dispatcher or a single immutable state holder
consistently across the rate collector and IO coroutine paths, including fee,
sendResult, amount, and error updates, while preserving the existing state
values and transitions.

This was referenced Aug 12, 2026
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.

3 participants