Skip to content

Do not block the native-module queue while a Zano wallet catches up - #17

Open
j0ntz wants to merge 3 commits into
mainfrom
jon/ios-perf-zano-xmr
Open

Do not block the native-module queue while a Zano wallet catches up#17
j0ntz wants to merge 3 commits into
mainfrom
jon/ios-perf-zano-xmr

Conversation

@j0ntz

@j0ntz j0ntz commented Aug 18, 2026

Copy link
Copy Markdown

Technical Design Document

zano-migration-off-module-queue.md

Description

QA reported severe app-wide iOS slowdowns after edge-react-gui upgraded to react-native-zano 0.4.0 (task: https://app.asana.com/0/1215088146871429/1217559756673909).

Root cause, captured live with a thread sample on the sim: startWallet's wallet-file re-key migration opens the wallet with the legacy password, and that open auto-starts the native refresh worker, which takes the per-wallet recursive mutex for the entire first catch-up scan. The migration's next step, resetWalletPassword, blocks on that mutex until the scan finishes (minutes for a wallet weeks behind, hours for a fresh rescan) while sitting on React Native's shared native-module dispatch queue. Every native-module call in the app queues behind it, which presents as a severe app-wide performance drop. The app usually dies before the migration completes, so the file is never re-keyed and every launch repeats the same blocked migration. 0.3.0 had no re-key step, which is why the regression tracks the 0.4.0 upgrade.

The fix is scheduling only, and JS-only: configure the native library's postponed_run_wallet mode before the migration's opens, so no probe or migration open starts a refresh worker, and start the worker explicitly (run_wallet, idempotent) for the one wallet startWallet returns. Both methods already exist in the shipped native dispatch. The migration's correctness rules (legacy password order, rebuild guards, passphrase handling) are untouched.

Verified on the iOS sim against an account with three Zano wallets that were weeks behind:

  • Before: resetWalletPassword observed blocked on the wallet mutex for 15+ minutes on com.meta.react.turbomodulemanager.queue; a probe polling getOpenedWallets starved the whole time; wallet files never re-keyed across many launches.
  • After: the migration completed and all three files were re-keyed seconds after engine start, the probe polled uninterrupted through the whole catch-up scan, and a relaunch opened all three wallets first-try with the derived password.

Testing

  • npm test (34 passing, including new coverage: the migration call sequence now postpones the worker and runs only the returned wallet; every terminal path leaves exactly the returned wallet running)
  • npx tsc --noEmit, verify-repo.sh clean
  • On-sim end-to-end drive of the migration + relaunch, described above

Note

Medium Risk
Touches wallet open/migration scheduling and process-wide postponed-run mode; behavior is well-tested but incorrect run_wallet timing could leave wallets not syncing until callers adopt/run them.

Overview
Fixes app-wide iOS slowdown after 0.4.0 by changing when the native refresh worker starts, not migration logic.

startWallet now calls native configure with postponed_run_wallet: true before any open/restore, so probe and re-key opens do not auto-start a catch-up scan that holds the per-wallet lock. New helpers configurePostponedRun and runWallet wrap those sync calls; every successful return path runs started(), which calls run_wallet only for the wallet being returned.

Migration rules (legacy passwords, rebuild, passphrase handling) are unchanged. Tests and the fake module track runningWallets to assert configure first, run_wallet last, and that only the returned wallet syncs. CHANGELOG and a design doc document the TurboModule queue / mutex interaction.

Reviewed by Cursor Bugbot for commit 7e73277. Bugbot is set up for automated code reviews on this repo. Configure here.

@j0ntz

j0ntz commented Aug 18, 2026

Copy link
Copy Markdown
Author

📸 Test evidence

ui during zano scan

ui during zano scan

after fix relaunch

after fix relaunch

Captured by the agent's in-app test run (build-and-test).

@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 18302e1. Configure here.

Comment thread src/CppBridge.ts
paullinator

This comment was marked as outdated.

paullinator

This comment was marked as outdated.

@paullinator paullinator left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Moving the generateSeedPhrase / postponed-run warning onto configurePostponedRun in startWallet (GitHub will not attach a comment to generateSeedPhrase itself because that method is unchanged in this diff).

Additional Findings

  • warning git:1: All four PR commits exceed the 50-character subject limit (worst is 61: “Open wallets with the refresh worker postponed in startWallet”). Subjects are otherwise imperative and capitalized.
    • Rewrite subjects to ≤50 characters, e.g. “Postpone refresh worker in startWallet”, “Add migration scheduling design doc”.

Comment thread src/CppBridge.ts
@j0ntz
j0ntz force-pushed the jon/ios-perf-zano-xmr branch 2 times, most recently from d5b4755 to b9a68a2 Compare August 20, 2026 18:42
@cursor

cursor Bot commented Aug 20, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@j0ntz
j0ntz force-pushed the jon/ios-perf-zano-xmr branch from b9a68a2 to f3d11d0 Compare August 20, 2026 19:39
@j0ntz

j0ntz commented Aug 20, 2026

Copy link
Copy Markdown
Author

Commit subjects are now within 50 characters, and the branch is the two commits you asked for:

  • Postpone refresh worker in startWallet (38)
  • Add migration scheduling design doc (35), squashing the four design-doc commits

Also folded in from the earlier review, since they were still live in the code:

  • CppBridge.ts transfer sent opts.paymentId ?? '' after the loop had resolved a payment id from an integrated address, so an integrated-address transfer with no explicit id broadcast an empty one. It now sends the resolved value.
  • generateSeedPhrase configures postponed-run before generate, and a non-OK closeWallet throws instead of falling through to deleteWallet on a handle this process still holds.
  • package.json is 0.4.1 and the changelog section is retitled, so the branch no longer collides with the published 0.4.0 tarball.
  • Tests: the already-derived open now asserts configure ran and that the returned wallet is the only running one; the fake returns ALREADY_EXISTS when a path is already open; the idempotent-launches case clears the open table to model a new process; and a new case covers a second start while the first is still open.

Not done here, with reasons: the ZanoModule.mm backup-exclusion hardening changes native failure behavior on a path that needs device testing, so it wants its own change rather than riding a JS scheduling fix; walletFilePassword's hand-rolled hex is unchanged because the derivation is golden-vector tested and I did not want to touch key derivation in this PR; and the branch name cannot change without recreating the PR.

@j0ntz
j0ntz force-pushed the jon/ios-perf-zano-xmr branch from f3d11d0 to 8a13dae Compare August 20, 2026 23:50
@j0ntz

j0ntz commented Aug 20, 2026

Copy link
Copy Markdown
Author

Correcting my previous reply: two of the three things I deferred there did not deserve deferring, and one misread what you asked. All three are handled now.

  • ZanoModule.mm fails closed. prepareZanoDirectory returns a BOOL, URLForDirectory: takes an NSError ** and bails on a nil URL, and a failure to create or exclude wallets withholds documentDirectory from the exported constants. CppBridge's constructor throws when that value is missing, so the failure stops there instead of concatenating undefined into a storage path. logs and app_config still only warn, since neither holds key material. Compile-checked with clang -fsyntax-only against the RN headers, and I confirmed the check fails on a planted typo so it is a real check rather than a no-op. It is not run through a device build.
  • walletFilePassword uses base16 from rfc4648 (now a declared dependency) instead of the hand-rolled table. My earlier reasoning was backwards: the golden vector is what makes this safe to change, not a reason to avoid it, and it still passes, so the derived password is byte-identical. utf8Bytes stays, and its comment now says why: this module also runs inside the accb plugin WebView, and TextEncoder is not guaranteed across every engine and polyfill combination there, while the derivation has to produce identical bytes everywhere forever.
  • On branch naming, you asked for <firstname>/<camelCaseName> on future branches and I answered a question you had not asked about renaming this one. Noted for the next branch.

@cursor

cursor Bot commented Aug 21, 2026

Copy link
Copy Markdown

Bugbot is paused — on-demand spend limit reached

Bugbot uses usage-based billing for this team and has hit its on-demand spend limit.

A team admin can raise the spend limit in the Cursor dashboard, or wait for the next billing cycle to continue.

@j0ntz

j0ntz commented Aug 21, 2026

Copy link
Copy Markdown
Author

Additional in-app evidence from today's run (iOS sim, edge-funds, two Zano wallets), which also shows the user-visible shape of the stall:

Before (published react-native-zano 0.4.0 installed): the Send scene could not even accept a recipient address. ZanoTools.isValidAddress calls getAddressInfo, that call sits on the same serial callZano dispatch, and it never resolved: the address tile spun indefinitely across three app launches and 15+ minutes, with plain_wallet.log stopping at init. Nothing in the send flow was reachable while a Zano wallet was catching up.

After (this branch linked, plus edge-currency-accountbased#1090): the same drive resolved the address immediately, the wallet finished its catch-up scan (blocks 3,528,465 to 3,826,151), and a 0.063 ZANO send between the two wallets completed to the success scene.

So the queue starvation this PR fixes was not only a slowdown, it made address entry and sending unusable while any Zano wallet was behind.

j0ntz added 2 commits August 21, 2026 15:11
The 0.4.0 re-key migration ran resetWalletPassword against a wallet whose
refresh worker had already taken the per-wallet lock for its entire first
catch-up scan, so the call blocked for the whole scan (minutes to hours)
while sitting on React Native's shared native-module dispatch queue. On
iOS every native-module call in the app queued behind it, which presented
as a severe app-wide performance drop whenever a Zano wallet was behind
on blocks.

Configure the native library's postponed_run_wallet mode before the
migration's opens, so no probe or migration open starts a refresh, and
start the worker explicitly for the one wallet startWallet returns. Both
methods already exist in the shipped native dispatch, so this is a
JS-only change.

Creating a wallet took the same shape: generateSeedPhrase opened its
temporary wallet without postponing the worker, so the closeWallet that
follows waited on the per-wallet lock. Configure postponed-run there
too, and treat a non-OK close as a failure rather than deleting a file
this process still holds open.

Also fixes transfer sending an empty payment_id for an integrated
address when the caller passed none, and bumps the package to 0.4.1.
Covers the investigation evidence, the postponed-run design across this
repo and edge-currency-accountbased, and the post-implementation
retrospective.

Covers the phase history through the Android validation-build fix, the
QA verification, and the create-wallet instance of the same stall.
@j0ntz
j0ntz force-pushed the jon/ios-perf-zano-xmr branch from c93048f to 3b67325 Compare August 21, 2026 22:11
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.

2 participants