Merge master into version/0.50 - #9494
Merged
Merged
Conversation
Kit c08505d rebuilds the native lib with a 20s/3-attempt daemon RPC timeout (was 4s/1) fixing spurious 'no connection to daemon' send failures, includes available/required amounts in insufficient-funds errors, and throws NodeUnreachableException for node RPC failures. Map that exception to a retry-suggesting message in the send screen.
![translator[bot]](https://github.githubassets.com/images/gravatars/gravatar-user-420.png){kind=avatar}
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.
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().
Contributor
|
Important Review skippedNo new commits to review since the last review. ⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: You can disable this status message by setting the Use the checkbox below for a quick retry:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe change adds Zcash Ironwood migration support across balance detection, navigation, proposal and execution flows, transaction classification, persistence, and confirmation UI. It also updates version metadata and maps Zano node-unreachable errors to localized cautions. ChangesZcash Ironwood migration
Release and send-error updates
Estimated code review effort: 4 (Complex) | ~60 minutes Sequence Diagram(s)sequenceDiagram
participant User
participant BalanceForAccount
participant ZcashMigrationPage
participant ZcashMigrationViewModel
participant ZcashAdapter
User->>BalanceForAccount: select migration action
BalanceForAccount->>ZcashMigrationPage: navigate with wallet
ZcashMigrationPage->>ZcashMigrationViewModel: create migration ViewModel
ZcashMigrationViewModel->>ZcashAdapter: proposeIronwoodMigration()
ZcashAdapter-->>ZcashMigrationViewModel: return amount and fee
User->>ZcashMigrationViewModel: confirm migration
ZcashMigrationViewModel->>ZcashAdapter: executeIronwoodMigration()
ZcashAdapter-->>ZcashMigrationViewModel: return transaction ID
Possibly related PRs
Suggested reviewers: 🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches 💡 1📝 Generate docstrings 💡
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 2
🧹 Nitpick comments (1)
walletkit-chain-zcash/src/main/java/io/horizontalsystems/walletkit/core/adapters/zcash/ZcashAdapter.kt (1)
370-378: 🗄️ Data Integrity & Integration | 🔵 Trivial | ⚡ Quick winPrevent stale migration classification.
If
allTransactionsdelivers the submitted transaction beforeexecuteIronwoodMigration()persists its ID, a duplicate overview with the same hash and mined height is ignored. The cached record then keepsisMigrationTransaction = false. Add an integration test for this ordering and refresh or reclassify cached records after persisting the IDs.🤖 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 `@walletkit-chain-zcash/src/main/java/io/horizontalsystems/walletkit/core/adapters/zcash/ZcashAdapter.kt` around lines 370 - 378, Update executeIronwoodMigration so that after persisting the returned transaction IDs, cached transaction records are refreshed or reclassified using the new migration IDs, ensuring an already-delivered transaction with the same hash and mined height is marked isMigrationTransaction = true. Add an integration test covering allTransactions delivering the transaction before ID persistence and verify the cached overview is corrected.
🤖 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
`@walletkit-chain-zcash/src/main/java/io/horizontalsystems/walletkit/modules/zcashmigration/ZcashMigrationScreen.kt`:
- Around line 24-41: Add explicit proposal-ready state to the Zcash migration
flow and pass it to SendConfirmationScreen so the migration action remains
disabled while proposeIronwoodMigration() is loading or fails. Set the state
only after proposal creation succeeds, and ensure
onClickMigrate/executeIronwoodMigration cannot run before that state is ready.
In
`@walletkit/src/main/java/io/horizontalsystems/walletkit/modules/balance/ui/BalanceForAccount.kt`:
- Around line 208-227: Update both coroutine bodies in the
ZcashMigrationBottomSheet onMigrateClick and onClose handlers to catch failures
before launch terminates, rethrow CancellationException, and handle all other
exceptions using the established error-handling approach.
---
Nitpick comments:
In
`@walletkit-chain-zcash/src/main/java/io/horizontalsystems/walletkit/core/adapters/zcash/ZcashAdapter.kt`:
- Around line 370-378: Update executeIronwoodMigration so that after persisting
the returned transaction IDs, cached transaction records are refreshed or
reclassified using the new migration IDs, ensuring an already-delivered
transaction with the same hash and mined height is marked isMigrationTransaction
= true. Add an integration test covering allTransactions delivering the
transaction before ID persistence and verify the cached overview is corrected.
🪄 Autofix
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: df3a676b-4b28-4ed4-9e4e-b33f56c79070
⛔ Files ignored due to path filters (9)
walletkit/src/main/res/values-de/strings.xmlis excluded by!**/res/values-*/strings.xmlwalletkit/src/main/res/values-es/strings.xmlis excluded by!**/res/values-*/strings.xmlwalletkit/src/main/res/values-fa/strings.xmlis excluded by!**/res/values-*/strings.xmlwalletkit/src/main/res/values-fr/strings.xmlis excluded by!**/res/values-*/strings.xmlwalletkit/src/main/res/values-ko/strings.xmlis excluded by!**/res/values-*/strings.xmlwalletkit/src/main/res/values-pt-rBR/strings.xmlis excluded by!**/res/values-*/strings.xmlwalletkit/src/main/res/values-ru/strings.xmlis excluded by!**/res/values-*/strings.xmlwalletkit/src/main/res/values-tr/strings.xmlis excluded by!**/res/values-*/strings.xmlwalletkit/src/main/res/values-zh/strings.xmlis excluded by!**/res/values-*/strings.xml
📒 Files selected for processing (25)
app/build.gradle.ktsgradle/libs.versions.tomltranslation_snapshot.jsonwalletkit-chain-zano/src/main/java/io/horizontalsystems/walletkit/modules/send/zano/SendZanoViewModel.ktwalletkit-chain-zcash/src/main/java/io/horizontalsystems/walletkit/chain/zcash/ZcashChainPlugin.ktwalletkit-chain-zcash/src/main/java/io/horizontalsystems/walletkit/core/adapters/zcash/ZcashAdapter.ktwalletkit-chain-zcash/src/main/java/io/horizontalsystems/walletkit/core/adapters/zcash/ZcashTransaction.ktwalletkit-chain-zcash/src/main/java/io/horizontalsystems/walletkit/core/adapters/zcash/ZcashTransactionsProvider.ktwalletkit-chain-zcash/src/main/java/io/horizontalsystems/walletkit/modules/zcashmigration/ZcashMigrationModule.ktwalletkit-chain-zcash/src/main/java/io/horizontalsystems/walletkit/modules/zcashmigration/ZcashMigrationPage.ktwalletkit-chain-zcash/src/main/java/io/horizontalsystems/walletkit/modules/zcashmigration/ZcashMigrationScreen.ktwalletkit-chain-zcash/src/main/java/io/horizontalsystems/walletkit/modules/zcashmigration/ZcashMigrationViewModel.ktwalletkit/src/main/java/io/horizontalsystems/walletkit/core/Interfaces.ktwalletkit/src/main/java/io/horizontalsystems/walletkit/core/chain/ChainPlugin.ktwalletkit/src/main/java/io/horizontalsystems/walletkit/core/managers/LocalStorageManager.ktwalletkit/src/main/java/io/horizontalsystems/walletkit/entities/transactionrecords/zcash/ZcashShieldingTransactionRecord.ktwalletkit/src/main/java/io/horizontalsystems/walletkit/modules/balance/BalanceViewModel.ktwalletkit/src/main/java/io/horizontalsystems/walletkit/modules/balance/token/TokenBalanceModule.ktwalletkit/src/main/java/io/horizontalsystems/walletkit/modules/balance/token/TokenBalanceScreen.ktwalletkit/src/main/java/io/horizontalsystems/walletkit/modules/balance/token/TokenBalanceViewModel.ktwalletkit/src/main/java/io/horizontalsystems/walletkit/modules/balance/ui/BalanceForAccount.ktwalletkit/src/main/java/io/horizontalsystems/walletkit/modules/balance/ui/ZcashMigrationBottomSheet.ktwalletkit/src/main/java/io/horizontalsystems/walletkit/modules/transactions/TransactionViewItemFactory.ktwalletkit/src/main/res/drawable/ic_migrate_24.xmlwalletkit/src/main/res/values/strings.xml
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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
`@walletkit-chain-zcash/src/main/java/io/horizontalsystems/walletkit/modules/zcashmigration/ZcashMigrationScreen.kt`:
- Around line 44-70: Move the SendResult.Sent success HUD and SendResult.Failed
ErrorSheet navigation side effects from the top-level when expression into the
existing LaunchedEffect(sendResult) block, keeping the result branching there
and preserving the existing delayed navigation for successful sends. Ensure
composition only renders UI and does not repeat these effects on recomposition.
🪄 Autofix
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: f80bad3b-10cc-4075-833c-4d1acff1d29f
📒 Files selected for processing (1)
walletkit-chain-zcash/src/main/java/io/horizontalsystems/walletkit/modules/zcashmigration/ZcashMigrationScreen.kt
Contributor
Author
|
Re the nitpick about stale migration classification (ZcashAdapter 370-378): acknowledged but leaving as is — it's the 0.49 behavior this merge brings over, the window is the gap between broadcast and persisting the tx ids, and the label self-corrects on the next transactions refresh. Not something the back-merge should redesign. The two Major findings are addressed in ef6d1b0 + 64be39a (Migrate gated on the resolved proposal; success/error side effects moved into LaunchedEffect with the caution string resolved during composition). |
Contributor
Author
|
@coderabbitai review |
Contributor
|
Brings the 0.49.3/0.49.4 hotfix line into 0.50: the Zcash Ironwood migration (SDK 2.7.0-rc.2), the Zano node-unreachable error mapping, and the tron kit update (already contained in 0.50's pin). Beyond textual conflicts, the merge ports master's work onto 0.50's module structure: - The zcashmigration feature moves from app fragments/main_graph to a nav3 HSPage in walletkit-chain-zcash. - Balance screens reach it through new ChainPlugin hooks (migrationRequiredBalance/migrationPage) instead of referencing ZcashAdapter from walletkit, which cannot depend on chain modules. - ShieldDirection gains MigrateToIronwood on the top-level enum rather than master's nested one. - A rename-detection mishap that applied the Zano NodeUnreachable mapping to SendTonViewModel is redirected to SendZanoViewModel. - Version stays 0.50.0 with versionCode 176 (above master's 175); kit pins keep 0.50's hashes, which already contain master's bumps.
The generic SendConfirmationScreen renders additionalFields inside the fee card and offers no button title override, so the ported screen showed the publicly-visible warning nested in the card and a Send button. Rebuild the screen from the shared confirmation sections the way the 0.49 screen did: amount card, fee card, standalone warning below, and a Migrate/Sending/Success button.
The fee only exists once proposeIronwoodMigration() resolves, and ConfirmationBottomSection renders no fee row for a null fee, so the cell popped in late. Render the same fee cell with n/a until the value lands, so only the text changes.
The button was enabled while the proposal was still loading, and tapping it then fails in executeIronwoodMigration with no proposal to send — require the fee (set exactly when the proposal resolves). Also run the success HUD and error sheet from LaunchedEffect instead of composition, so a recomposition can't repeat them.
Caution strings are @composable and cannot be read inside the effect coroutine the previous commit moved the error sheet into.
omurovch
force-pushed
the
merge-master-into-0.50
branch
from
August 12, 2026 11:24
64be39a to
4be8110
Compare
Merged
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Back-merges the 0.49.3/0.49.4 hotfix line (23 commits) into the 0.50 development branch. The headline change is the Zcash Ironwood migration feature built on Zcash SDK 2.7.0-rc.2, plus the Zano node-unreachable error mapping and the tron kit update.
Master's work predates 0.50's module restructure, so beyond the textual conflicts this merge also ports it onto the new architecture:
appfragments +main_graph.xmlto a nav3HSPage(ZcashMigrationPage) inwalletkit-chain-zcash, reusing the genericSendConfirmationScreen(which already covers the success/error flows master's bespoke screen handled).walletkitcannot depend on chain modules, so the balance screens reach the migration through newmigrationRequiredBalance(wallet)/migrationPage(wallet, entryPoint)plugin hooks instead of master's directZcashAdapterreferences, mirroring the existing shield-flow hooks.MigrateToIronwoodadded to 0.50's top-level enum instead of master's nestedZcashTransaction.ShieldDirection.NodeUnreachableExceptionmapping toSendTonViewModel(near-identical clone); redirected toSendZanoViewModel.versionNamestays 0.50.0 withversionCode176 (above master's released 175). Kit pins keep 0.50's hashes: the tron and zano pins were verified to be descendants of master's bumps;zcashSdktakes master's 2.7.0-rc.2.translation_snapshot.json(restructured layout kept, master's new keys folded in).Compiles through
:app:compileBaseDebugKotlin. Worth a device pass on the Zcash migration flow (balance alert → sheet → confirmation page) since it was re-wired onto nav3.Summary by CodeRabbit