Rebuild the transaction list during a rescan, and stop mislabelling it expired - #73
Rebuild the transaction list during a rescan, and stop mislabelling it expired#73peachbits wants to merge 2 commits into
Conversation
A resync rewinds the wallet to its birthday, which un-mines every stored transaction until the scan re-reaches its block. TransactionState.Expired compares an unmined transaction's expiry height against the live network tip, so during that window the entire history counts as expired and the app flashes every transaction as failed - Android only, since iOS reports the database's expired_unmined column instead of recomputing against the tip. Reach the database's own verdict from public API: unmined, expiry enabled, and the fully-scanned floor past the expiry window. The floor trails MAX(blocks.height) while ranges scan out of order, so this is equal-or- more conservative than the DB flag and converges with it (and with iOS) once the wallet is synced. The emitted-transaction tracking also records the computed verdict, since it can flip without any tracked SDK field changing: a transaction stuck unmined keeps its minedHeight and (tip-expired) transactionState while the scan floor crosses its expiry window. Without the extra trigger, a genuine expiry would only surface on the next subscribe.
The app empties its transaction list for a resync and rebuilds it from what we report, but both platforms sent the whole set straight back, so the list refilled before the rescan had scanned anything. Android did it twice. rescan() cleared the emitted-transaction tracking, so the next collector pass saw every transaction as new and reported the lot at their pre-rewind heights - 19 seconds before the rewind had even landed, describing nothing that had changed. The rewind then unmined every row, which read as a change and sent the same set again, this time as unmined, which is what left settled history looking pending. iOS did it once, explicitly: rescan re-sent allTransactions as soon as the rewind finished. Keep the tracking across a rescan instead of clearing it, and treat a transaction losing its mined height as the rewind undoing our own scan rather than news about the transaction. Tracking still follows it to the unmined state, so re-mining reads as a change and reports normally, which is how the list rebuilds one transaction at a time. Unmined transactions are the exception on both platforms. Scanning only discovers transactions in mined blocks, so nothing would bring back a send still waiting to be mined; those are re-reported so they survive the resync. iOS has to collect them before the rewind, since afterwards every transaction looks unmined.
There was a problem hiding this comment.
Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.
❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.
Reviewed by Cursor Bugbot for commit 773f1ce. Configure here.
| // | ||
| // Clearing the whole map instead would re-emit every transaction at | ||
| // its pre-rewind height, refilling the list the app had just emptied. | ||
| emittedTransactions[alias]?.values?.removeAll { it.minedHeight == null } |
There was a problem hiding this comment.
Rescan mishandles already-unmined history
Medium Severity
Both platforms treat “currently unmined” as “pending send to keep,” but after a rewind every settled transaction looks unmined. A second rescan (or a rescan while a prior rewind’s unmined state is still in tracking / the DB) therefore clears Android’s whole emit map or fills iOS priorUnminedIds with the full history, and the emptied UI is refilled with pending rows again.
Additional Locations (2)
Reviewed by Cursor Bugbot for commit 773f1ce. Configure here.


CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
none
Description
Fixes half of ZEC - resync shows failed transactions and incorrect sync status — the half where a resync flashes every transaction in the wallet as Failed until the rescan finishes — and then makes a resync actually rebuild the transaction list, on both platforms.
Two commits:
isExpiredfrom the wallet's scan floor, not the network tip (Android) — the reported "Failed" bug.Root cause.
parseTxreportedisExpiredfromTransactionState.Expired. That state compares an unmined transaction's expiry height against the live network tip:A resync calls
rewindToNearestHeight(birthday), which un-mines every stored transaction until the scan re-reaches its block. During that window the whole history is unmined with expiry heights far below the tip, so every transaction is reported expired. Downstream that becomesconfirmations: 'failed'inedge-currency-accountbasedand a red "Failed" row in the GUI, which clears itself as the rescan re-mines each transaction — matching QA's note that it resolves once the wallet finishes syncing.iOS never had this: it reports the wallet database's
expired_unminedcolumn (isExpiredUmined) rather than recomputing against the tip. The database's rule is wallet-relative:Fix. Reach that same verdict from public API.
isTxExpired()compares expiry againstprocessor.fullyScannedHeight— the wallet's own contiguous scan floor — so a transaction counts as expired only once this wallet has scanned past its expiry window without finding it mined. The floor trails the database'sMAX(blocks.height)while ranges scan out of order, so this is equal-or-more conservative than the DB flag and converges with it (and with iOS) once synced.The emitted-transaction tracking now also records the computed verdict. It can flip without any tracked SDK field changing — a transaction stuck unmined keeps its
minedHeightand its (tip-derived)transactionStatewhile the scan floor crosses its expiry window — so without the extra trigger a genuine expiry would only surface on the nextsubscribe.No JS or iOS changes: the event shape (
isExpired: boolean) is unchanged and iOS is already correct.Verification.
npm run fix-kotlin(ktlint) clean.:react-native-zcash:compileDebugKotlinvia edge-react-gui's gradle with this file innode_modules— build successful, no warnings from the new code. This also confirmsprocessor.fullyScannedHeightis public at the pinned2.7.0-rc.4(it is not lifted onto theSynchronizerinterface until a later upstream release).verify-repo.shpassed.Device-verified on a Pixel 10 Pro emulator against a real funded wallet (6 transactions), by syncing to 100% and then resyncing. The rewind reported:
All six transactions have expiry heights between 3,365,360 and 3,432,555 — every one of them inside that 75,024-block gap, i.e. above the scan floor but below the network tip. That is exactly the range where the two implementations disagree, so the bug condition was fully exercised rather than merely absent:
expired_unmined(what iOS reports)During the rescan every row — sent and received alike — rendered "Pending" rather than "Failed", and each returned to confirmed as the scan re-reached its block.
The DB flag reading 2 while this branch reports 0 is the conservatism described above, not a discrepancy:
fullyScannedHeighttrailsMAX(blocks.height)while ranges scan out of order. Both transactions had been mined before the rewind and were re-mined by the end of the rescan, so not calling them expired was the correct answer.Second commit: rebuilding the list during a rescan
The app empties its own transaction list for a resync and rebuilds it from what we report. Both platforms sent the whole set straight back, so the list refilled before the rescan had scanned anything.
Instrumenting the Android module showed it happening twice:
The first is pure noise — it fired before the rewind landed, describing nothing that had changed, purely because
rescan()cleared the emitted-transaction tracking and every row then looked new. The second is what left settled history reading as pending. iOS did the equivalent once, explicitly re-sendingallTransactionsinrescan's completion handler.Now: keep the tracking across a rescan, and treat a transaction losing its mined height as the rewind undoing our own scan rather than news about the transaction. Tracking still follows it to the unmined state, so re-mining reads as a change and reports normally — which is how the list rebuilds one transaction at a time.
Unmined transactions are the exception on both platforms, since scanning only discovers transactions in mined blocks and nothing would bring back a send still waiting to be mined. iOS has to collect those before the rewind, because afterwards every transaction looks unmined.
Verified on device (same emulator and wallet). After the resync the module goes quiet — the collector keeps firing with all six transactions unmined and reports nothing:
The app showed
Loading Transactions… 35.04076% Completein place of the list, then transactions reappeared as the scan found them — every emission carrying a real mined height, nevernull:They came back confirmed, with no "Pending" or "Failed" state in between.
iOS is device-verified too, on the iPhone 17 simulator against the same wallet (same alias, same six transactions, same heights). Synced to 100%, then resynced from the wallet menu: the sync banner reset to
0% Complete, the transaction list emptied toLoading Transactions… 89.29972% Complete, and the transactions then reappeared confirmed as the scan re-found them — no "Pending" or "Failed" in between. Identical to Android.Capturing the unmined set before the rewind is what makes that work. Filtering on
minedHeight == nilafter the rewind would match every transaction, since the rewind unmines the whole history, and the full set would be re-sent exactly as before.The other half of the task (sync status reading 100% during the rescan) is a separate bug in
edge-currency-accountbased, fixed in companion PR EdgeApp/edge-currency-accountbased#1083; the two are independent and can land in either order.Note
Medium Risk
Touches native transaction emission and expiry semantics during resync—user-visible wallet history—but scope is limited to bridge logic, aligns Android with existing iOS behavior, and was device-verified without API changes.
Overview
Fixes resync UX on Android and iOS: settled history no longer shows as Failed during a rewind, and the transaction list is rebuilt as the scan re-finds each tx instead of refilling immediately.
On Android,
isExpiredno longer comes fromTransactionState.Expired(network tip). A newisTxExpired()compares expiry toprocessor.fullyScannedHeight, matching iOS / the wallet DB rule so rewound unmined history is not marked expired. Emitted-transaction tracking also recordsisExpired, re-emits when that verdict flips, suppresses events when a tx losesminedHeightdue to rewind, and onrescanonly forgets unmined entries (not the full map).On iOS,
rescanno longer sendsallTransactionsafter rewind; it captures unmined tx ids before rewind and re-emits only those still-pending sends, since scanning cannot rediscover them.CHANGELOG documents both behaviors. No JS event shape changes.
Reviewed by Cursor Bugbot for commit 773f1ce. Bugbot is set up for automated code reviews on this repo. Configure here.