Zcash: Keep the sync ratio honest across a resync

[peachbits]

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 the wallet reads as fully synced while a resync is still rescanning "behind the scenes".

Root cause. resyncBlockchain deliberately does not stop and restart the synchronizer, so the sync tracker is reset while the synchronizer keeps running and keeps emitting. The rewind then races the event stream: a progress report sampled while the wallet still looked synced (scanProgress === 100) can be delivered after syncTracker.resetSync().

ZcashSyncTracker treats a first-seen 100 as trustworthy — that is the login shortcut for wallets that are already synced — so it latches lastTotalRatio = 1. Its don't-go-backwards ratchet then discards every genuine progress report for the rest of the rescan:

// ZcashSyncTracker.ts
if (!seenFirstUpdate) {
  seenFirstUpdate = true
  if (progressPercent !== 100) return   // a stale 100 gets through, and latches
}
...
if (status.totalRatio <= lastTotalRatio) return   // everything real is now discarded

Fix. Ignore progress reports for the span of the resync — armed before the engine is torn down, cleared in a finally once the rewind has actually happened.

The window is bounded by the method rather than by what the synchronizer reports, because no report can mark that boundary. A stale sub-100 is indistinguishable from early rescan progress, and a wallet that was still syncing when the resync was requested is a common case — being stuck syncing is a reason people resync — so keying off report values lets the ratchet latch a pre-rewind ratio for the whole rescan. A status transition cannot bound it either: rescan sets restart on iOS and updateSyncStatus then swallows the following SYNCED, so a rescan that finishes without ever reporting syncing emits no status event at all.

Clearing in a finally also covers the paths that never reach a rewind — a throw from the teardown calls, or no synchronizer to rescan. Leaving the flag armed there would ignore every report for the rest of the session, a worse failure than the bug being fixed. The inner catch around rescan() is kept separate so setup failures still propagate to the caller rather than being swallowed.

One residual race remains: a report already in flight when the rewind completes. It is bounded by the gap between the rewind finishing and the promise settling, rather than by the whole teardown. Closing it entirely would need to gate on an observed SYNCING transition, which iOS does not reliably deliver.

No changes to ZcashSyncTracker itself, so the ordinary login path — instant 100% for an already-synced wallet — is untouched. Piratechain is not affected: its tracker is block-height based and has no first-update-100 heuristic.

Verification.

• verify-repo.sh passed: eslint on changed files, plus the full test suite.

Device-verified on a Pixel 10 Pro emulator against a real funded wallet, by syncing to 100% and then resyncing from the wallet menu. On confirming the resync the wallet immediately reported Sync in Progress — 0% Complete, and the ratio then climbed monotonically from zero:

0.014 → 0.025 → 0.036 → 0.044 → 0.054 → … → 0.214 → …

Before this change the tracker latched on a stale 100 delivered after resetSync() and its ratchet discarded every one of those readings, leaving the wallet displayed as fully synced for the entire rescan. The wallet returned to synced normally once the rescan finished.

That run predates the rework above, which was prompted by review. The behaviour it demonstrates is unchanged — the ratio resets and climbs — but the boundary that produces it moved, so it is worth re-running before merge. In particular the resync-while-syncing case, which the earlier design got wrong and no test here covered.

The other half of the task (every transaction showing "Failed" during the rescan) is an Android-native bug, fixed in companion PR EdgeApp/react-native-zcash#73. The two are independent and can land in either order; the native fix reaches the app through a later react-native-zcash version bump, not through this PR.

---

Note

Medium Risk
Changes only Zcash resync/sync-progress handling; logic is localized but resync is a sensitive user path and stale vs fresh progress cannot be distinguished without this gate.

Overview
Fixes incorrect “fully synced” UI during a Zcash wallet resync when the synchronizer keeps running and can deliver pre-rewind scanProgress after the sync tracker has been reset.

ZcashEngine now sets rescanSettling for the whole resyncBlockchain flow (engine kill, cache clear, restart, and rescan()), and the synchronizer update handler skips syncTracker.updateProgress while that flag is set. The flag is cleared in a finally block so a failed resync does not leave progress updates suppressed for the rest of the session.

ZcashSyncTracker is unchanged, so the normal login shortcut (first-seen 100% for an already-synced wallet) stays intact.

^{Reviewed by Cursor Bugbot for commit 5a6c44d. Bugbot is set up for automated code reviews on this repo. Configure here.}

---

• To see the specific tasks where the Asana app for GitHub is being used, see below:
  • https://app.asana.com/0/0/1217341748716939

[cursor]

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 dcb0c18. Configure here.}

[j0ntz]

Three notes on the settling window, pointing at one design question: the window's exits infer the rewind boundary from report values and per-platform status semantics, and each inference has a gap. rescan() resolution marks the same boundary directly.

[j0ntz]

The settle window can be closed by a stale report. A resync triggered mid-sync (a wallet stuck syncing is a common reason users resync) produces pre-rewind sub-100 reports that look identical to fresh rescan progress. The first one disarms rescanSettling, and the ratchet then holds the pre-rewind ratio for the whole restarted rescan; if the old sync completes inside the teardown gap, a following stale 100 pins the ratio at 1, which is the bug this PR closes. clearBlockchainCache resets the tracker, so the stale sub-100 is discarded as the untrusted first update and the next stale report is accepted.

sequenceDiagram
  participant N as Native synchronizer
  participant E as ZcashEngine
  participant T as syncTracker
  E->>N: rescan requested, rescanSettling = true
  N-->>E: update 87 (sampled pre-rewind)
  E->>E: sub-100, so rescanSettling = false
  E->>T: updateProgress(87), dropped as untrusted first update
  N-->>E: update 90 (sampled pre-rewind)
  E->>T: updateProgress(90), ratchet holds 0.9 while the rescan restarts from the birthday

Loading

Consider disarming on rescan() resolution instead of on the first sub-100: the promise settles after the rewind on both platforms, so the boundary stops depending on report values (a report already in flight at resolution is the residual case to check).

[peachbits]

Confirmed, and this is the one that showed the approach was wrong rather than incomplete. Fixed in 5a6c44d.

Your trace is exactly right: a wallet still syncing when the resync is requested emits sub-100 progress that is indistinguishable from early rescan progress, so the first one disarms the window and the ratchet then holds a pre-rewind ratio for the whole restarted rescan. My guard only ever considered a stale 100 dangerous; the stale sub-100 case is both more likely and harder to see, since the ratio looks plausible rather than obviously stuck.

Taking your suggestion, with one change. The window is now bounded by resyncBlockchain itself - armed before the teardown, cleared in a finally - rather than by rescan() resolution alone, so the same boundary also covers the paths that never reach a rewind (your third comment). While armed, every report is ignored rather than only 100s, since after the reset there is nothing a pre-rewind report can correctly say.

The residual you flagged - a report already in flight at resolution - is still there, now narrowed from the whole teardown to the gap between the rewind completing and the promise settling. Closing it fully would need an observed SYNCING transition to gate on, which is precisely what your second comment shows is unavailable on iOS, so I have taken the narrow race over a platform-dependent boundary.

[j0ntz]

This fallback does not fire on iOS in the case it was written for. In react-native-zcash, rescan sets restart = true (ios/RNZcash.swift:451), and updateSyncStatus returns early on .synced while restart is set (line 751); restart clears only when a .syncing state arrives (line 749). A rescan that finishes without ever reporting syncing emits no status event at all, which is exactly the trivial rescan this branch targets: no sub-100 update, no SYNCED, so rescanSettling stays armed, every 100 is swallowed, and the wallet reads unsynced until the next login. Android forwards the status StateFlow verbatim, so the transition does arrive there.

The reverse direction also holds: a SYNCED emitted just before the resync but delivered after the flag is armed satisfies this check, closes the window, and pushes 100 into a freshly reset tracker; that is the same delivery race the update handler guards against.

[peachbits]

Confirmed in the source, and the fallback is deleted in 5a6c44d rather than repaired.

rescan sets restart = true (ios/RNZcash.swift:451), and updateSyncStatus returns early on .synced while restart is set, clearing it only on .syncing. So the trivial rescan this branch existed for - finishes without ever reporting syncing - emits no status event at all on iOS, which is the one case where the branch had to fire. Armed flag, every 100 swallowed, unsynced until the next login. Android forwards the status StateFlow verbatim, which is why testing there did not surface it.

Both directions of your point applied: the branch could fail to fire when needed, and a pre-resync SYNCED delivered after arming would have fired it when it should not, pushing 100 into a freshly reset tracker.

It is gone entirely rather than fixed, because it only existed to close a window that the old design could not close on its own. Now that resyncBlockchain closes its own window in a finally, the trivial rescan needs no special handling: the flag clears when the rewind is done, the wallet reports 100, and the tracker takes it as a first-update-that-is-100. statusChanged is back to only recording status.

[j0ntz]

rescanSettling can stay armed with no rewind behind it, and armed forever means every 100 is swallowed and the wallet never reads synced again this session:

• A throw in super.killEngine(), clearBlockchainCache(), or startEngine() propagates out with the flag set; this catch never runs.
• this.synchronizer?.rescan() resolves to undefined without throwing when the synchronizer is not set (the critical-error handler's killEngine() clears it mid-await, and it is created lazily on the first syncNetwork tick), so the no-rewind reset is skipped.

Moving the three setup awaits inside the try and making the null-synchronizer case reset the flag explicitly covers both.

[peachbits]

Both paths confirmed, fixed in 5a6c44d.

The catch only wrapped rescan(), so a throw from killEngine, clearBlockchainCache or startEngine propagated with the flag set, and this.synchronizer?.rescan() resolving to undefined skipped the reset without ever throwing. Either way the flag stayed armed with no rewind behind it, and armed forever means every report is ignored for the rest of the session - the wallet simply never reads synced again. Worse failure than the bug the flag was added for.

The three setup awaits are now inside the try, and the reset moved to a finally so it runs on every path including the two above. The inner catch around rescan() is kept separate so setup failures still propagate to the caller as they do today, rather than being silently swallowed by a catch wide enough to cover everything.