Skip to content

Rebuild the transaction list during a rescan, and stop mislabelling it expired - #73

Open
peachbits wants to merge 2 commits into
masterfrom
matthew/fix/rescan-expired-mislabel
Open

Rebuild the transaction list during a rescan, and stop mislabelling it expired#73
peachbits wants to merge 2 commits into
masterfrom
matthew/fix/rescan-expired-mislabel

Conversation

@peachbits

@peachbits peachbits commented Aug 6, 2026

Copy link
Copy Markdown
Contributor

CHANGELOG

Does this branch warrant an entry to the CHANGELOG?

  • Yes
  • No

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:

  1. isExpired from the wallet's scan floor, not the network tip (Android) — the reported "Failed" bug.
  2. Report transactions as the rescan finds them, not all at once (both platforms) — a resync now empties the list and rebuilds it one transaction at a time, instead of the whole set reappearing immediately.

Root cause. parseTx reported isExpired from TransactionState.Expired. That state compares an unmined transaction's expiry height against the live network tip:

minedHeight == null && expiryHeight != 0 && expiryHeight <= networkTip  ->  Expired

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 becomes confirmations: 'failed' in edge-currency-accountbased and 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_unmined column (isExpiredUmined) rather than recomputing against the tip. The database's rule is wallet-relative:

mined_height IS NULL AND expiry_height BETWEEN 1 AND blocks_max_height.max_height

Fix. Reach that same verdict from public API. isTxExpired() compares expiry against processor.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's 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 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 minedHeight and its (tip-derived) transactionState while the scan floor crosses its expiry window — so without the extra trigger a genuine expiry would only surface on the next subscribe.

No JS or iOS changes: the event shape (isExpired: boolean) is unchanged and iOS is already correct.

Verification.

  • npm run fix-kotlin (ktlint) clean.
  • Compiles against the pinned SDK: :react-native-zcash:compileDebugKotlin via edge-react-gui's gradle with this file in node_modules — build successful, no warnings from the new code. This also confirms processor.fullyScannedHeight is public at the pinned 2.7.0-rc.4 (it is not lifted onto the Synchronizer interface until a later upstream release).
  • verify-repo.sh passed.

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:

Rewinding to requested height: 3364881 with last local block: 3439915
Rewound to BlockHeight(value=3364881) successfully
→ chainTipHeight = 3,439,915   fullyScannedHeight = 3,364,891

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:

old code (vs tip) DB expired_unmined (what iOS reports) this branch (vs scan floor)
transactions marked expired 6 of 6 2 0

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: fullyScannedHeight trails MAX(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:

14:32:42.601  rescan() called
14:32:43.123  EMITTING 6 tx: mined=3432515, 3432300, 3431970, 3430879 …   ← pre-rewind heights
14:32:44.938  EMITTING 6 tx: mined=null ×6                                 ← rewind landed

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-sending allTransactions in rescan'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:

14:41:35.310  rescan() called
14:41:50.138  fired: total=6 unmined=6 floor=3364891   (no emission)
14:42:00.975  fired: total=6 unmined=6 floor=3364891   (no emission)
…

The app showed Loading Transactions… 35.04076% Complete in place of the list, then transactions reappeared as the scan found them — every emission carrying a real mined height, never null:

14:45:12.885  EMITTING 1 tx: mined=3431970
14:45:15.696  EMITTING 2 tx: mined=3432515, mined=3432300

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 to Loading 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 == nil after 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, isExpired no longer comes from TransactionState.Expired (network tip). A new isTxExpired() compares expiry to processor.fullyScannedHeight, matching iOS / the wallet DB rule so rewound unmined history is not marked expired. Emitted-transaction tracking also records isExpired, re-emits when that verdict flips, suppresses events when a tx loses minedHeight due to rewind, and on rescan only forgets unmined entries (not the full map).

On iOS, rescan no longer sends allTransactions after 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.

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.
@peachbits peachbits changed the title Report isExpired from the wallet's scan floor, not the network tip Rebuild the transaction list during a rescan, and stop mislabelling it expired Aug 7, 2026
@peachbits
peachbits marked this pull request as ready for review August 7, 2026 23:17

@cursor cursor Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Cursor Bugbot has reviewed your changes using high effort and found 1 potential issue.

Fix All in Cursor

❌ 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 }

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

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)
Fix in Cursor Fix in Web

Reviewed by Cursor Bugbot for commit 773f1ce. Configure here.

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.

1 participant