Do not block the native-module queue while a Zano wallet catches up by j0ntz · Pull Request #17 · EdgeApp/react-native-zano

j0ntz · 2026-08-18T20:45:50Z

Technical Design Document

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 · 2026-08-18T20:46:06Z

📸 Test evidence

ui during zano scan

after fix relaunch

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

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

paullinator

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”.

cursor · 2026-08-20T18:42:53Z

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 · 2026-08-20T19:40:20Z

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 · 2026-08-20T23:50:16Z

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 · 2026-08-21T19:02:01Z

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 · 2026-08-21T22:09:50Z

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.

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 · 2026-08-25T19:14:59Z

Closing in favour of #18, which supersedes this.

That branch carries the postponed-run scheduling forward, including the generateSeedPhrase half that Paul's review added here, plus the iOS fail-closed directory handling and the rfc4648 password encoding, both byte-identical to this branch. It adds the removal of the transaction-wide payment id, which HF6 makes a hard rejection, and corrects AddressInfo to the shape the native library actually returns.

The design doc that lived on this branch is now #19, updated to describe the shipping implementation rather than this one, so the links to it from the task and the run reports keep resolving once that merges.

Opening a wallet auto-starts its native refresh worker, and the worker holds the per-wallet recursive mutex for the entire first catch-up scan (plain_wallet_api.cpp:508 auto-runs on open; worker_func holds the locked_object proxy across refresh()). The 0.4.0 re-key migration's resetWalletPassword takes that same mutex, so on a wallet weeks behind it blocked for the whole scan while sitting on React Native's shared native-module dispatch queue -- every native call in the app queued behind it, and the app usually died before the migration finished, so the file was never re-keyed and every launch repeated the block. startWallet now configures the native library's postponed_run_wallet mode before its first open, so no probe or migration open starts a worker, and starts the worker explicitly for exactly the wallet it returns. Every throw path leaves either nothing open, or a wallet that reaches its next caller as ALREADY_EXISTS -- and under postponed mode an adopted wallet is not running, so the adopter owns issuing run_wallet for it, which edge-currency-accountbased's adoption path does. The flag is process-wide and sticky; the raw open/restore/generate docs carry the caveat, and any open made after this must be followed by run_wallet once the wallet should sync. generateSeedPhrase had the same shape: its temporary wallet opened with the worker auto-running, so the closeWallet that follows waited on the same lock. It postpones too, and a close that does not report OK now fails the call rather than deleting a file this process still holds open. Both native methods (configure, run_wallet) exist in the shipped 0.4.0 dispatch, so this is JS-only, and runWallet treats the one bare-string native failure answer as a failure report rather than a parse surprise. The fake models the postponed-run contract (runningWallets, an open-wallet table tests can clear to simulate the process dying), and every startWallet success path asserts exactly-the-returned-wallet- running -- verified by mutation: dropping any started() call now fails the suite. Ported from #17. Co-authored-by: Jonathan Tzeng <jnthntzng@gmail.com>

`prepareZanoDirectory` failures were log-and-continue: if the `wallets` directory could not be created or could not be marked excluded from device backups, the module still handed out `documentDirectory` and the SDK went on to write the seed and spend keys into a directory an unencrypted Finder backup would capture -- the exact exposure the 0.4.0 backup exclusion exists to close. A nil documents directory was worse: `[docsDir path]` fed nil into an NSDictionary literal, which throws at module init. The module now withholds `documentDirectory` when `wallets` cannot be prepared, and the `CppBridge` constructor turns that into a hard failure with a clear message, so Zano is disabled for the session rather than silently unprotected. `NativeZanoModule.documentDirectory` becomes optional to match the contract this creates, the bridge keeps a guard-validated copy for its own paths, and a test pins the refusal for both the missing and the empty value. `logs` and `app_config` carry no key material, so their preparation stays best-effort. This trades availability for key protection: a transient exclusion failure now disables Zano until the next launch instead of degrading silently. Deliberate, and worth revisiting only if such failures show up in the field. Ported from #17. Co-authored-by: Jonathan Tzeng <jnthntzng@gmail.com>

Replaces the hand-rolled hex encoder in `deriveWalletFilePassword` with `base16.stringify(...).toLowerCase()` from rfc4648, which is the encoding library the rest of the stack standardizes on. Output is byte-identical for every input -- both produce the lowercase hex of the same 16 digest bytes -- and the unchanged golden-vector test enforces that: any drift here would orphan every wallet file the 0.4.0 migration re-keyed. Strictly cosmetic; no changelog entry. The `utf8Bytes` docstring also grows a paragraph on why the encoder avoids host globals. rfc4648 joins dependencies and the lockfile is regenerated with it, so `npm ci` stays reproducible. Ported from #17. Co-authored-by: Jonathan Tzeng <jnthntzng@gmail.com>

Zano HF6 moved payment ids into the transaction outputs: each integrated destination's embedded id is attached natively, one per output, and the wallet RPC rejects any non-empty request-level `payment_id` outright (WALLET_RPC_ERROR_CODE_WRONG_PAYMENT_ID, "tx-wide payment id you provided is now deprecated"). The shipped 0.4.0 SDK enforces this whether or not the fork has activated. The one real defect was forwarding: a caller-supplied `opts.paymentId` went out on the wire and the node refused the send with the deprecation error. The per-destination validation loop next to it, meanwhile, was inert: it gated on `addressInfo.is_integrated`, a field the native `get_address_info` has never returned -- the real response carries `payment_id` only as a boolean presence flag (plain_wallet_api.cpp:484, "lazy to make struct for it") -- so the loop never resolved an id, never threw its one-id-per-transaction error, and only cost a native round-trip per recipient on every send. There is no longer anything a request-level id can express, so the option is removed rather than kept as decoration: the field is always sent empty, and a caller holding a separate payment id folds it into an integrated destination address before calling -- which is what edge-currency-accountbased now does on the user's behalf, offline via zano-utils-js. The dead loop goes with it, along with the fictional `is_integrated`/string-`payment_id` declarations in `AddressInfo` and the test fake, which now match the real native shape. The fake also models the transfer path (`asyncCall`/`tryPullResult`, `invoke`), which is the gap that let the forwarding go untested: tests pin that nothing non-empty ever reaches the wire, that multi-destination sends pass, and that the send path makes no address lookups. Replaces the `payment_id: paymentId ?? ''` change proposed in #17 -- which was itself a no-op, since the loop feeding `paymentId` never assigned it -- with the removal the HF6 contract actually calls for. References: - Zano HF6 migration guide, "What is intrinsic payment id": https://docs.zano.org/docs/build/exchange-guidelines/HF6-migration-guide/ - wallet_rpc_server.cpp on_transfer (rejection at the pinned SDK)

Opening a wallet auto-starts its native refresh worker, and the worker holds the per-wallet recursive mutex for the entire first catch-up scan (plain_wallet_api.cpp:508 auto-runs on open; worker_func holds the locked_object proxy across refresh()). The 0.4.0 re-key migration's resetWalletPassword takes that same mutex, so on a wallet weeks behind it blocked for the whole scan while sitting on React Native's shared native-module dispatch queue -- every native call in the app queued behind it, and the app usually died before the migration finished, so the file was never re-keyed and every launch repeated the block. startWallet now configures the native library's postponed_run_wallet mode before its first open, so no probe or migration open starts a worker, and starts the worker explicitly for exactly the wallet it returns. Every throw path leaves either nothing open, or a wallet that reaches its next caller as ALREADY_EXISTS -- and under postponed mode an adopted wallet is not running, so the adopter owns issuing run_wallet for it, which edge-currency-accountbased's adoption path does. The flag is process-wide and sticky; the raw open/restore/generate docs carry the caveat, and any open made after this must be followed by run_wallet once the wallet should sync. generateSeedPhrase had the same shape: its temporary wallet opened with the worker auto-running, so the closeWallet that follows waited on the same lock. It postpones too, and a close that does not report OK now fails the call rather than deleting a file this process still holds open. Both native methods (configure, run_wallet) exist in the shipped 0.4.0 dispatch, so this is JS-only, and runWallet treats the one bare-string native failure answer as a failure report rather than a parse surprise. The fake models the postponed-run contract (runningWallets, an open-wallet table tests can clear to simulate the process dying), and every startWallet success path asserts exactly-the-returned-wallet- running -- verified by mutation: dropping any started() call now fails the suite. Ported from #17. Co-authored-by: Jonathan Tzeng <jnthntzng@gmail.com>

`prepareZanoDirectory` failures were log-and-continue: if the `wallets` directory could not be created or could not be marked excluded from device backups, the module still handed out `documentDirectory` and the SDK went on to write the seed and spend keys into a directory an unencrypted Finder backup would capture -- the exact exposure the 0.4.0 backup exclusion exists to close. A nil documents directory was worse: `[docsDir path]` fed nil into an NSDictionary literal, which throws at module init. The module now withholds `documentDirectory` when `wallets` cannot be prepared, and the `CppBridge` constructor turns that into a hard failure with a clear message, so Zano is disabled for the session rather than silently unprotected. `NativeZanoModule.documentDirectory` becomes optional to match the contract this creates, the bridge keeps a guard-validated copy for its own paths, and a test pins the refusal for both the missing and the empty value. `logs` and `app_config` carry no key material, so their preparation stays best-effort. This trades availability for key protection: a transient exclusion failure now disables Zano until the next launch instead of degrading silently. Deliberate, and worth revisiting only if such failures show up in the field. Ported from #17. Co-authored-by: Jonathan Tzeng <jnthntzng@gmail.com>

Replaces the hand-rolled hex encoder in `deriveWalletFilePassword` with `base16.stringify(...).toLowerCase()` from rfc4648, which is the encoding library the rest of the stack standardizes on. Output is byte-identical for every input -- both produce the lowercase hex of the same 16 digest bytes -- and the unchanged golden-vector test enforces that: any drift here would orphan every wallet file the 0.4.0 migration re-keyed. Strictly cosmetic; no changelog entry. The `utf8Bytes` docstring also grows a paragraph on why the encoder avoids host globals. rfc4648 joins dependencies and the lockfile is regenerated with it, so `npm ci` stays reproducible. Ported from #17. Co-authored-by: Jonathan Tzeng <jnthntzng@gmail.com>

Zano HF6 moved payment ids into the transaction outputs: each integrated destination's embedded id is attached natively, one per output, and the wallet RPC rejects any non-empty request-level `payment_id` outright (WALLET_RPC_ERROR_CODE_WRONG_PAYMENT_ID, "tx-wide payment id you provided is now deprecated"). The shipped 0.4.0 SDK enforces this whether or not the fork has activated. The one real defect was forwarding: a caller-supplied `opts.paymentId` went out on the wire and the node refused the send with the deprecation error. The per-destination validation loop next to it, meanwhile, was inert: it gated on `addressInfo.is_integrated`, a field the native `get_address_info` has never returned -- the real response carries `payment_id` only as a boolean presence flag (plain_wallet_api.cpp:484, "lazy to make struct for it") -- so the loop never resolved an id, never threw its one-id-per-transaction error, and only cost a native round-trip per recipient on every send. There is no longer anything a request-level id can express, so the option is removed rather than kept as decoration: the field is always sent empty, and a caller holding a separate payment id folds it into an integrated destination address before calling -- which is what edge-currency-accountbased now does on the user's behalf, offline via zano-utils-js. The dead loop goes with it, along with the fictional `is_integrated`/string-`payment_id` declarations in `AddressInfo` and the test fake, which now match the real native shape. The fake also models the transfer path (`asyncCall`/`tryPullResult`, `invoke`), which is the gap that let the forwarding go untested: tests pin that nothing non-empty ever reaches the wire, that multi-destination sends pass, and that the send path makes no address lookups. Replaces the `payment_id: paymentId ?? ''` change proposed in #17 -- which was itself a no-op, since the loop feeding `paymentId` never assigned it -- with the removal the HF6 contract actually calls for. References: - Zano HF6 migration guide, "What is intrinsic payment id": https://docs.zano.org/docs/build/exchange-guidelines/HF6-migration-guide/ - wallet_rpc_server.cpp on_transfer (rejection at the pinned SDK)

j0ntz mentioned this pull request Aug 18, 2026

Persist the Zano wallet file once synced and run adopted wallets EdgeApp/edge-currency-accountbased#1090

Closed

2 tasks

j0ntz force-pushed the jon/ios-perf-zano-xmr branch from 84e93c8 to 18302e1 Compare August 18, 2026 20:59

j0ntz marked this pull request as ready for review August 18, 2026 20:59

cursor Bot reviewed Aug 18, 2026

View reviewed changes

Comment thread src/CppBridge.ts

This comment was marked as outdated.

Sign in to view

paullinator requested changes Aug 20, 2026

View reviewed changes

Comment thread src/CppBridge.ts

j0ntz force-pushed the jon/ios-perf-zano-xmr branch 2 times, most recently from d5b4755 to b9a68a2 Compare August 20, 2026 18:42

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

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

j0ntz added 2 commits August 21, 2026 15:11

j0ntz force-pushed the jon/ios-perf-zano-xmr branch from c93048f to 3b67325 Compare August 21, 2026 22:11

Document the receive-notification phase

0117343

peachbits mentioned this pull request Aug 24, 2026

Postpone the Zano refresh worker and drop the tx-wide payment id #18

Merged

2 tasks

j0ntz mentioned this pull request Aug 25, 2026

Add the Zano migration design doc #19

Open

2 tasks

j0ntz closed this Aug 25, 2026

Navigation Menu

Uh oh!

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

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

j0ntz commented Aug 18, 2026 •

edited by cursor Bot

Loading

Uh oh!

j0ntz commented Aug 18, 2026

Uh oh!

cursor Bot left a comment

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

paullinator left a comment

Uh oh!

Uh oh!

cursor Bot commented Aug 20, 2026

Uh oh!

j0ntz commented Aug 20, 2026

Uh oh!

j0ntz commented Aug 20, 2026

Uh oh!

cursor Bot commented Aug 21, 2026

Uh oh!

j0ntz commented Aug 21, 2026

Uh oh!

j0ntz commented Aug 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Uh oh!

Conversation

j0ntz commented Aug 18, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Technical Design Document

Description

Testing

Uh oh!

j0ntz commented Aug 18, 2026

📸 Test evidence

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

This comment was marked as outdated.

Uh oh!

This comment was marked as outdated.

Uh oh!

paullinator left a comment

Choose a reason for hiding this comment

Additional Findings

Uh oh!

Uh oh!

cursor Bot commented Aug 20, 2026

Bugbot is paused — on-demand spend limit reached

Uh oh!

j0ntz commented Aug 20, 2026

Uh oh!

j0ntz commented Aug 20, 2026

Uh oh!

cursor Bot commented Aug 21, 2026

Bugbot is paused — on-demand spend limit reached

Uh oh!

j0ntz commented Aug 21, 2026

Uh oh!

j0ntz commented Aug 25, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

j0ntz commented Aug 18, 2026 •

edited by cursor Bot

Loading