From ca1dc61927a6cb0dfa66495ed5c1207801480134 Mon Sep 17 00:00:00 2001 From: peachbits Date: Wed, 26 Aug 2026 17:05:05 -0700 Subject: [PATCH 1/3] fixed: Keep Zano calls off the RN bridge thread On the legacy Android architecture every @ReactMethod runs on the single shared mqt_native_modules thread, which also executes UIManager's view commands, so a Zano call that blocked in C++ froze every view update in the app: taps dead, screens frozen, native scrolling alive. A JDWP dump of the frozen app shows callZano parked on that thread inside callZanoJNI. callZano now hops to a dedicated single-thread executor and settles the promise from there. The executor is static: the Zano SDK is one instance per process while React module instances are not, so a JS reload would otherwise run the old instance's still-draining call concurrently with the new instance's calls. A single process-wide thread preserves the strict global call ordering across reloads too, and is named "zano" so it identifies itself in future dumps. The catch widens to Throwable: off the bridge thread an escaping Error would hit the default uncaught handler and kill the process with the promise unsettled, where RN's dispatcher previously caught it. Argument extraction from the ReadableArray stays on the caller thread. --- CHANGELOG.md | 2 ++ .../java/app/edge/rnzano/RnZanoModule.java | 32 ++++++++++++++++--- 2 files changed, 29 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index a836886..c50f311 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,8 @@ ## Unreleased +- fixed: Android runs native Zano calls on a dedicated thread instead of React Native's shared native-modules thread. On the legacy architecture that shared thread also executes UIManager's view commands, so a Zano call that blocked in C++ froze every view update in the app -- taps dead, screens frozen -- while native scrolling kept working. A blocked call now stalls only Zano. + ## 0.5.0 (2026-08-25) - added: `runWallet`, which starts the refresh worker for an open wallet. `startWallet` rethrows `ALREADY_EXISTS` for its caller to adopt the already-open wallet, and an adopted wallet does not sync until it is run, so adopting callers need this without reimplementing the raw `run_wallet` response contract. diff --git a/android/src/main/java/app/edge/rnzano/RnZanoModule.java b/android/src/main/java/app/edge/rnzano/RnZanoModule.java index 0e7d347..6560d7f 100644 --- a/android/src/main/java/app/edge/rnzano/RnZanoModule.java +++ b/android/src/main/java/app/edge/rnzano/RnZanoModule.java @@ -7,8 +7,25 @@ import com.facebook.react.bridge.ReactMethod; import java.util.HashMap; import java.util.Map; +import java.util.concurrent.ExecutorService; +import java.util.concurrent.Executors; public class RnZanoModule extends ReactContextBaseJavaModule { + // Zano calls run on their own thread, never on the caller's. On the + // legacy architecture every @ReactMethod runs on the shared + // mqt_native_modules thread, which also executes UIManager's view + // commands -- so a Zano call that blocks in C++ (a close waiting out a + // scan chunk, or the SDK's close-during-scan lock inversion) froze every + // view update in the app. Static, because the Zano SDK is one instance + // per process while React module instances are not: a JS reload builds a + // new module while a call from the old one may still be draining, and + // per-instance threads would let both drive the SDK at once. One + // process-wide thread keeps the strict global call ordering, across + // reloads too. Named, because this freeze was diagnosed from thread + // dumps, and "pool-1-thread-1" will not identify itself in the next one. + private static final ExecutorService executor = + Executors.newSingleThreadExecutor(r -> new Thread(r, "zano")); + private native String callZanoJNI(String method, String[] arguments); private native String[] getMethodNames(); @@ -42,10 +59,15 @@ public void callZano(String method, ReadableArray arguments, Promise promise) { strings[i] = arguments.getString(i); } - try { - promise.resolve(callZanoJNI(method, strings)); - } catch (Exception e) { - promise.reject("ZanoError", e); - } + executor.execute(() -> { + try { + promise.resolve(callZanoJNI(method, strings)); + } catch (Throwable e) { + // Throwable, not Exception: off the bridge thread an escaping Error + // reaches the default uncaught handler and kills the process with + // the promise unsettled; RN's own dispatcher used to catch it. + promise.reject("ZanoError", e); + } + }); } } From ea9638d6081218711db2986c8b87a65056e1d52a Mon Sep 17 00:00:00 2001 From: peachbits Date: Thu, 27 Aug 2026 11:41:29 -0700 Subject: [PATCH 2/3] fixed: Patch the Zano close-during-scan deadlock The SDK's close_wallet holds m_wallets_lock exclusively across two waits on the wallet being closed: the store() call, which needs the per-wallet lock the refresh worker holds for the whole of a scan chunk, and the map erase, whose destructor joins the worker thread. The refresh path re-enters the manager through wallet callbacks that take m_wallets_lock shared, so a close issued while the wallet is catching up wedges the worker, the close, and then every Zano call in the process, permanently. The app's periodic mid-sync checkpoint saves trip this within minutes of importing a wallet on Android. update-sources now rewrites close_wallet in the downloaded sources to detach the map node under the lock and do the store and the join with no manager lock held. Everything that can throw stays inside the try, so failures keep reporting as return codes rather than escaping into the JNI catch-all, which callers do not expect; the node is declared outside it so the worker join in its destructor runs at function exit rather than during unwinding. The log-prefix write gains a bounds check: the manager lock no longer serializes it against reset()'s vector clear, so the blind vector[wallet_id] store could go out of bounds. The transform requires the found function to match the pinned original exactly, modulo trailing whitespace and line endings, so a pin bump that changes close_wallet in any way fails the build for a human to re-evaluate the patch instead of silently keeping or dropping it. Only Android builds from these sources; iOS links the prebuilt xcframework and keeps the original blocking semantics. --- CHANGELOG.md | 1 + scripts/update-sources.ts | 12 ++ scripts/utils/closeWalletPatch.ts | 225 ++++++++++++++++++++++++++++++ test/closeWalletPatch.test.ts | 182 ++++++++++++++++++++++++ 4 files changed, 420 insertions(+) create mode 100644 scripts/utils/closeWalletPatch.ts create mode 100644 test/closeWalletPatch.test.ts diff --git a/CHANGELOG.md b/CHANGELOG.md index c50f311..f375378 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -3,6 +3,7 @@ ## Unreleased - fixed: Android runs native Zano calls on a dedicated thread instead of React Native's shared native-modules thread. On the legacy architecture that shared thread also executes UIManager's view commands, so a Zano call that blocked in C++ froze every view update in the app -- taps dead, screens frozen -- while native scrolling kept working. A blocked call now stalls only Zano. +- fixed: Closing a wallet while it is catching up on blocks no longer deadlocks the native wallet manager permanently on Android. The SDK's `close_wallet` held the wallet-manager lock while waiting out the wallet's refresh worker, which needed that same lock to reach its stop flags, so the close, the worker, and every Zano call after them hung forever; the app's periodic mid-sync saves tripped this within minutes of importing a wallet. The Android build now rewrites `close_wallet` to detach the wallet from the manager before waiting on it. iOS links the prebuilt Zano framework and runs the same cycle without wedging, so it is unchanged. ## 0.5.0 (2026-08-25) diff --git a/scripts/update-sources.ts b/scripts/update-sources.ts index 91c6f88..a62729a 100644 --- a/scripts/update-sources.ts +++ b/scripts/update-sources.ts @@ -30,6 +30,7 @@ import { cpus } from 'os' import { join } from 'path' import { getNdkPath } from './utils/android-tools' +import { patchCloseWallet } from './utils/closeWalletPatch' import { captureExec, fileExists, @@ -81,6 +82,17 @@ async function downloadSources(): Promise { const mdPath = join(tmpPath, 'zano_native_lib/Zano/src/crypto/RIPEMD160.h') const mdText = await readFile(mdPath, 'utf8') await writeFile(mdPath, '#define compress md_compress\n' + mdText) + + // Rework close_wallet so it does not hold the wallet-manager lock while + // it waits on the wallet, which permanently deadlocks the manager when a + // wallet is closed mid-scan (see closeWalletPatch.ts). Only the Android + // libraries build from these sources; iOS links the prebuilt framework: + const wmPath = join( + tmpPath, + 'zano_native_lib/Zano/src/wallet/wallets_manager.cpp' + ) + const wmText = await readFile(wmPath, 'utf8') + await writeFile(wmPath, patchCloseWallet(wmText)) } /** diff --git a/scripts/utils/closeWalletPatch.ts b/scripts/utils/closeWalletPatch.ts new file mode 100644 index 0000000..5b31cc7 --- /dev/null +++ b/scripts/utils/closeWalletPatch.ts @@ -0,0 +1,225 @@ +const patchMarker = 'Edge patch: close-during-refresh deadlock' + +const hint = + 'The SDK sources changed at this pin. If upstream has fixed the ' + + 'close-during-refresh lock inversion, delete this patch; otherwise port ' + + 'it to the new body.' + +/** + * `close_wallet` as pinned at zano_native_lib 91085c0, modulo invisible + * trailing whitespace. The transform refuses to run unless the function it + * found matches this byte-for-byte after trailing whitespace is stripped, + * so ANY upstream drift -- not just drift through the lines the deadlock + * analysis rests on -- fails the build for a human to re-evaluate. + */ +const original = `std::string wallets_manager::close_wallet(size_t wallet_id) +{ + EXCLUSIVE_CRITICAL_REGION_LOCAL(m_wallets_lock); + + auto it = m_wallets.find(wallet_id); + if (it == m_wallets.end()) + return API_RETURN_CODE_WALLET_WRONG_ID; + + + try + { + it->second.major_stop = true; + it->second.stop_for_refresh = true; + it->second.w.unlocked_get()->stop(); + + it->second.w->get()->store(); + m_wallets.erase(it); + { + CRITICAL_REGION_LOCAL(m_wallet_log_prefixes_lock); + m_wallet_log_prefixes[wallet_id] = std::string("[") + epee::string_tools::num_to_string_fast(wallet_id) + ":CLOSED] "; + } + } + + catch (const std::exception& e) + { + return std::string(API_RETURN_CODE_FAIL) + ":" + e.what(); + } + catch (...) + { + return API_RETURN_CODE_INTERNAL_ERROR; + } + //m_pview->hide_wallet(); + return API_RETURN_CODE_OK; +}` + +/** + * The replacement `close_wallet`. The pinned original holds + * `m_wallets_lock` exclusively across two waits on the wallet being + * closed: the `store()` call, which needs the per-wallet lock the refresh + * worker holds for the whole of a scan chunk, and the map erase, whose + * destructor joins the worker thread. The refresh path re-enters the + * manager through wallet callbacks (`on_transfer2`, `on_transfer_canceled`) + * that take `m_wallets_lock` shared, and every other API call takes it + * shared up front -- so a close issued while the wallet is catching up + * wedges the worker, the close, and then every Zano call in the process, + * permanently. Thread dumps of the frozen app show the close parked inside + * the native call, and no checkpoint ever lands again. + * + * The rewrite detaches the map node while holding the lock, then does the + * slow parts -- the store and the implicit thread join when the node dies + * -- with no manager lock held at all. Node extraction keeps the element + * at its address, so the worker thread's references stay valid; wallet ids + * are never reused, so late callbacks cannot hit a recycled id. The + * log-prefix write gains a bounds check, because the manager lock no + * longer serializes it against `reset()`'s vector clear. + * + * Deliberate semantics changes, all confined to the window after the node + * leaves the map: + * + * - "Absent from `m_wallets`" no longer implies "closed and stored". + * During the store/join window the wallet is invisible to + * `open_wallet`'s ALREADY_EXISTS check and to status calls while its + * `wallet2` still writes the file. Safe for this bridge, which strictly + * sequences close-before-reopen per wallet on one executor thread; a + * caller without that discipline could double-open the file. Say so in + * any upstream submission. + * - The refresh worker's own callbacks miss where they used to hit. Both + * `on_transfer2` and `on_transfer_canceled` look the wallet up by id + * under a shared `m_wallets_lock` while the store runs, and the entry is + * no longer there. Both lookups are checked -- `on_transfer2` through + * `GET_WALLET_OPTIONS_BY_ID_VOID_RET`, which returns on a miss, and + * `on_transfer_canceled` with an explicit `end()` test that logs and + * returns -- so the cost is a dropped view notification for a wallet + * that is closing anyway, not an unchecked dereference. Every + * `m_wallets.find` in the file is checked this way; the three bare + * `m_wallets[...]` sites all insert a freshly counted id in the open, + * restore and generate paths, and ids are never reused. + * - When `store()` throws, the original left the entry in the map as a + * zombie (its stop flags already set, so it could never sync again); + * the rewrite reports the same error but the wallet is gone. A + * `closeWallet` retry therefore reports WALLET_WRONG_ID instead of + * retrying the store, so `CppBridge`'s re-key migration takes its + * "leave the file alone" branch with the wallet already released; the + * next launch retries the migration. + * + * iOS links the prebuilt `libzano-plain-wallet` framework and keeps the + * original blocking semantics throughout. + */ +const replacement = `std::string wallets_manager::close_wallet(size_t wallet_id) +{ + // ${patchMarker}. + // + // The original held m_wallets_lock exclusively across store() and across + // the map erase, whose destructor joins the refresh worker. Both wait on + // a wallet that may be mid-refresh, and the refresh path re-enters the + // manager through wallet callbacks that take m_wallets_lock shared -- so + // a close issued during a long refresh held the very lock the worker + // needed to reach its stop flags, and neither side could ever proceed. + // Detach the map node under the lock instead, then store and join with + // no manager lock held. The node is declared outside the try so its + // destructor -- which joins the worker thread the stop flags told to + // finish -- runs at function exit rather than during unwinding, while + // everything that can throw stays inside the try, so failures keep + // reporting as return codes exactly as they did before this patch. + decltype(m_wallets)::node_type wallet_node; + try + { + { + EXCLUSIVE_CRITICAL_REGION_LOCAL(m_wallets_lock); + + auto it = m_wallets.find(wallet_id); + if (it == m_wallets.end()) + return API_RETURN_CODE_WALLET_WRONG_ID; + + it->second.major_stop = true; + it->second.stop_for_refresh = true; + it->second.w.unlocked_get()->stop(); + wallet_node = m_wallets.extract(it); + } + + wallet_node.mapped().w->get()->store(); + { + CRITICAL_REGION_LOCAL(m_wallet_log_prefixes_lock); + // The manager lock no longer serializes this write against reset()'s + // clear of the vector, so respect its current size: + if (wallet_id < m_wallet_log_prefixes.size()) + m_wallet_log_prefixes[wallet_id] = std::string("[") + epee::string_tools::num_to_string_fast(wallet_id) + ":CLOSED] "; + } + } + catch (const std::exception& e) + { + return std::string(API_RETURN_CODE_FAIL) + ":" + e.what(); + } + catch (...) + { + return API_RETURN_CODE_INTERNAL_ERROR; + } + //m_pview->hide_wallet(); + return API_RETURN_CODE_OK; +}` + +/** Strips trailing whitespace per line, the one formatting freedom the + * comparison allows -- the pinned file carries an invisible trailing + * space that transcriptions of it should not have to reproduce. `\r` is + * in the class so a CRLF checkout reports the drift it has, rather than + * one carriage return per line. */ +function normalize(code: string): string { + return code + .split('\n') + .map(line => line.replace(/[ \t\r]+$/, '')) + .join('\n') +} + +/** + * Rewrites the SDK's `wallets_manager::close_wallet` so it does not hold + * the wallet-manager lock while waiting on the wallet being closed. See + * the comment on `replacement` above for the deadlock this removes and + * the semantics it deliberately changes. + * + * Only the Android libraries pick this up: iOS links the prebuilt + * `libzano-plain-wallet` xcframework rather than building these sources, + * and iOS runs the same close-during-catch-up cycle without wedging. + * + * The function is located by its unique signature, delimited by brace + * counting, and then required to match the pinned original exactly + * (modulo trailing whitespace), so a pin bump that changes `close_wallet` + * in any way fails the build here instead of silently keeping (or + * dropping) a stale patch. The brace counter would be fooled by a brace + * inside a string literal, but the full-body comparison catches that case + * too: a mis-delimited body cannot match the original. + * + * @param text - The contents of the SDK's `wallets_manager.cpp`. + * @returns The patched contents. Already-patched input comes back + * unchanged, so the caller does not need to track whether it ran. + */ +export function patchCloseWallet(text: string): string { + if (text.includes(patchMarker)) return text + + const anchor = 'std::string wallets_manager::close_wallet(size_t wallet_id)' + const start = text.indexOf(anchor) + if (start < 0 || text.includes(anchor, start + 1)) { + throw new Error( + `Cannot find a unique wallets_manager::close_wallet to patch. ${hint}` + ) + } + + // Take the whole function by brace balance: + let depth = 0 + let end = -1 + for (let i = text.indexOf('{', start); i >= 0 && i < text.length; ++i) { + if (text[i] === '{') ++depth + if (text[i] === '}' && --depth === 0) { + end = i + 1 + break + } + } + if (end < 0) { + throw new Error( + `Cannot delimit the body of wallets_manager::close_wallet. ${hint}` + ) + } + + if (normalize(text.slice(start, end)) !== normalize(original)) { + throw new Error( + 'wallets_manager::close_wallet does not match the pinned original ' + + `this patch was written against. ${hint}` + ) + } + + return text.slice(0, start) + replacement + text.slice(end) +} diff --git a/test/closeWalletPatch.test.ts b/test/closeWalletPatch.test.ts new file mode 100644 index 0000000..f477210 --- /dev/null +++ b/test/closeWalletPatch.test.ts @@ -0,0 +1,182 @@ +import { strict as assert } from 'assert' + +import { patchCloseWallet } from '../scripts/utils/closeWalletPatch' + +// The function as it appears in the SDK's `wallets_manager.cpp` at +// zano_native_lib 91085c0, modulo invisible trailing whitespace, the one +// difference the full-body comparison forgives. This is an independent +// transcription: the suite passing proves it agrees with the copy the +// transform itself pins. +const REAL_CLOSE_WALLET = `std::string wallets_manager::close_wallet(size_t wallet_id) +{ + EXCLUSIVE_CRITICAL_REGION_LOCAL(m_wallets_lock); + + auto it = m_wallets.find(wallet_id); + if (it == m_wallets.end()) + return API_RETURN_CODE_WALLET_WRONG_ID; + + + try + { + it->second.major_stop = true; + it->second.stop_for_refresh = true; + it->second.w.unlocked_get()->stop(); + + it->second.w->get()->store(); + m_wallets.erase(it); + { + CRITICAL_REGION_LOCAL(m_wallet_log_prefixes_lock); + m_wallet_log_prefixes[wallet_id] = std::string("[") + epee::string_tools::num_to_string_fast(wallet_id) + ":CLOSED] "; + } + } + + catch (const std::exception& e) + { + return std::string(API_RETURN_CODE_FAIL) + ":" + e.what(); + } + catch (...) + { + return API_RETURN_CODE_INTERNAL_ERROR; + } + //m_pview->hide_wallet(); + return API_RETURN_CODE_OK; +}` + +const before = 'std::string wallets_manager::open_wallet() { return ""; }\n\n' +const after = '\n\nvoid wallets_manager::init_wallet_entry() { }\n' +const REAL_FILE = before + REAL_CLOSE_WALLET + after + +describe('patchCloseWallet', () => { + it('rewrites the pinned close_wallet', () => { + const patched = patchCloseWallet(REAL_FILE) + + // The deadlock shape is gone: nothing slow runs under the lock... + assert.ok(!patched.includes('m_wallets.erase(it)')) + assert.ok(patched.includes('wallet_node = m_wallets.extract(it)')) + + // ...and the store happens against the detached node: + assert.ok(patched.includes('wallet_node.mapped().w->get()->store()')) + }) + + it('keeps the rest of the file untouched', () => { + const patched = patchCloseWallet(REAL_FILE) + assert.ok(patched.startsWith(before)) + assert.ok(patched.endsWith(after)) + }) + + it('keeps the behavior around the deadlock fix verbatim', () => { + const patched = patchCloseWallet(REAL_FILE) + + // Stop flags, the store, the log-prefix update, and the return codes + // all survive; only the locking around them changes: + for (const kept of [ + 'major_stop = true;', + 'stop_for_refresh = true;', + 'w.unlocked_get()->stop();', + 'EXCLUSIVE_CRITICAL_REGION_LOCAL(m_wallets_lock);', + 'CRITICAL_REGION_LOCAL(m_wallet_log_prefixes_lock);', + ':CLOSED] ', + 'API_RETURN_CODE_WALLET_WRONG_ID', + 'API_RETURN_CODE_FAIL', + 'API_RETURN_CODE_INTERNAL_ERROR', + 'API_RETURN_CODE_OK' + ]) { + assert.ok(patched.includes(kept), `patched close_wallet lost ${kept}`) + } + }) + + it('bounds-checks the log-prefix write it moves off the manager lock', () => { + // The original wrote m_wallet_log_prefixes[wallet_id] while holding + // m_wallets_lock exclusively, which serialized it against reset()'s + // vector clear. The rewrite's write is outside that lock, so it must + // respect the vector's current size: + const patched = patchCloseWallet(REAL_FILE) + assert.ok(patched.includes('if (wallet_id < m_wallet_log_prefixes.size())')) + }) + + it('keeps every throwing step inside the try', () => { + // The original reported failures as return codes, and callers branch + // on those. Anything that escapes close_wallet instead surfaces as a + // thrown Java exception, which reaches JS as a rejected promise rather + // than a resolved `{ response: 'FAIL:...' }`: + const patched = patchCloseWallet(REAL_FILE) + const body = patched.slice( + patched.indexOf('close_wallet(size_t wallet_id)') + ) + // The statement, not the word where the comment above explains it: + const tryAt = body.indexOf('\n try\n {') + assert.ok(tryAt > 0) + for (const throwing of [ + 'EXCLUSIVE_CRITICAL_REGION_LOCAL(m_wallets_lock);', + 'w.unlocked_get()->stop();', + 'm_wallets.extract(it)', + 'store();' + ]) { + assert.ok( + body.indexOf(throwing) > tryAt, + `${throwing} runs outside the try` + ) + } + // ...but the node itself is declared outside it, so the worker join in + // its destructor runs at function exit rather than during unwinding: + assert.ok(body.indexOf('node_type wallet_node;') < tryAt) + }) + + it('is idempotent', () => { + const patched = patchCloseWallet(REAL_FILE) + assert.equal(patchCloseWallet(patched), patched) + }) + + it('accepts a CRLF checkout of the pinned source', () => { + // A CRLF working tree must not read as upstream drift: + const crlf = REAL_FILE.replace(/\n/g, '\r\n') + assert.ok(patchCloseWallet(crlf).includes('m_wallets.extract(it)')) + }) + + it('tolerates trailing whitespace differences from the pin', () => { + // The pinned file carries an invisible trailing space; the comparison + // must not depend on reproducing it: + const spaced = REAL_FILE.replace( + 'EXCLUSIVE_CRITICAL_REGION_LOCAL(m_wallets_lock);\n', + 'EXCLUSIVE_CRITICAL_REGION_LOCAL(m_wallets_lock); \n' + ) + assert.ok(patchCloseWallet(spaced).includes('m_wallets.extract(it)')) + }) + + it('throws when close_wallet is missing', () => { + // A pin bump that renames or removes the function must fail the build + // rather than shipping Android libraries with the deadlock back: + assert.throws( + () => patchCloseWallet(before + after), + /Cannot find a unique/ + ) + }) + + it('throws when close_wallet appears twice', () => { + assert.throws( + () => patchCloseWallet(REAL_FILE + '\n' + REAL_CLOSE_WALLET), + /Cannot find a unique/ + ) + }) + + it('throws when the body no longer matches the pin', () => { + // An upstream rewrite -- including the fix we hope they ship -- must + // force a human to re-evaluate this patch rather than silently + // stacking on top of it: + const changed = REAL_FILE.replace( + 'm_wallets.erase(it);', + 'erase_wallet(it);' + ) + assert.throws(() => patchCloseWallet(changed), /pinned original/) + }) + + it('throws on any drift, not just drift through load-bearing lines', () => { + // A new statement that leaves the analyzed lines intact still changes + // what the function does, and must not be silently discarded: + const drifted = REAL_FILE.replace( + 'm_wallets.erase(it);', + 'm_wallets.erase(it);\n ++m_close_count;' + ) + assert.throws(() => patchCloseWallet(drifted), /pinned original/) + }) +}) From 67a8fce1238d51c434f6194142b90ef1552428cc Mon Sep 17 00:00:00 2001 From: peachbits Date: Thu, 27 Aug 2026 13:14:22 -0700 Subject: [PATCH 3/3] fixed: Rebuild unreadable Zano wallet files A crash during a wallet file's very first write leaves a file the SDK cannot parse -- typically zero bytes -- and opening it fails with INVALID_FILE before any password is consulted. startWallet's recovery ladder is keyed on WRONG_PASSWORD, so the error propagated as-is and the engine retried the same doomed open every second, forever: a full native init and load per attempt, per wallet, with no path back to health. Observed live after an emulator was hard-killed mid-restore: two of four wallets left zero-byte files and spun in the retry loop while the other two synced. Route INVALID_FILE to the same policy as a file no known password opens, skipping the pointless password ladder: with no seed passphrase set, delete the file and rebuild it from the mnemonic, which recreates the identical wallet at the cost of a re-scan; with a passphrase set, refuse with a clear error, since an unreadable file cannot corroborate the passphrase and a wrong one would rebuild a different wallet. --- CHANGELOG.md | 1 + src/CppBridge.ts | 37 ++++++++++++++++++++++++++++++++----- test/startWallet.test.ts | 36 ++++++++++++++++++++++++++++++++++-- 3 files changed, 67 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index f375378..8615cb6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -4,6 +4,7 @@ - fixed: Android runs native Zano calls on a dedicated thread instead of React Native's shared native-modules thread. On the legacy architecture that shared thread also executes UIManager's view commands, so a Zano call that blocked in C++ froze every view update in the app -- taps dead, screens frozen -- while native scrolling kept working. A blocked call now stalls only Zano. - fixed: Closing a wallet while it is catching up on blocks no longer deadlocks the native wallet manager permanently on Android. The SDK's `close_wallet` held the wallet-manager lock while waiting out the wallet's refresh worker, which needed that same lock to reach its stop flags, so the close, the worker, and every Zano call after them hung forever; the app's periodic mid-sync saves tripped this within minutes of importing a wallet. The Android build now rewrites `close_wallet` to detach the wallet from the manager before waiting on it. iOS links the prebuilt Zano framework and runs the same cycle without wedging, so it is unchanged. +- fixed: A Zano wallet file the SDK cannot read -- typically zero bytes, left by a crash during the file's very first write -- no longer traps the wallet in a permanent open-retry loop. Such a file fails with `INVALID_FILE` before any password is consulted, which bypassed the password-recovery ladder entirely, so the engine retried the same doomed open every second forever. The file is now deleted and rebuilt from the mnemonic, costing one re-scan, under the same policy as a file no password opens: a wallet with a seed passphrase still refuses, since an unreadable file cannot corroborate the passphrase and a wrong one would rebuild a different wallet. ## 0.5.0 (2026-08-25) diff --git a/src/CppBridge.ts b/src/CppBridge.ts index b5a5bd1..bc04422 100644 --- a/src/CppBridge.ts +++ b/src/CppBridge.ts @@ -51,6 +51,10 @@ function isWrongPassword(error: unknown): boolean { return error instanceof ZanoError && error.code === 'WRONG_PASSWORD' } +function isInvalidFile(error: unknown): boolean { + return error instanceof ZanoError && error.code === 'INVALID_FILE' +} + function isAlreadyExists(error: unknown): boolean { return error instanceof ZanoError && error.code === 'ALREADY_EXISTS' } @@ -438,11 +442,13 @@ export class CppBridge { * for both roles, so files written by them were keyed with the seed * passphrase -- the empty string for most wallets. A file still encrypted * that way is re-keyed in place the first time it opens. A file that no - * known password opens is deleted and rebuilt from the mnemonic, costing - * one re-scan -- but only for a wallet with no seed passphrase. With one - * set, the passphrase is far and away the likeliest thing to be wrong, and - * rebuilding would restore a different wallet over a file that was intact, - * so that case throws instead. + * known password opens, or that the SDK cannot parse a wallet out of at + * all -- the leavings of a crash during the file's first write -- is + * deleted and rebuilt from the mnemonic, costing one re-scan -- but only + * for a wallet with no seed passphrase. With one set, the passphrase is + * far and away the likeliest thing to be wrong, and rebuilding would + * restore a different wallet over a file that was intact, so that case + * throws instead. * * The migration is decided entirely by what the file does, so it is * idempotent and self-healing: an interrupted re-key leaves the file on @@ -522,6 +528,27 @@ export class CppBridge { try { return await started(await openWith(filePassword)) } catch (error: unknown) { + // A file the SDK cannot parse a wallet header out of -- zero bytes + // after a crash during its first write, or other corruption -- fails + // with INVALID_FILE before any password is consulted, so the password + // ladder below has nothing to probe. Without this branch the error + // propagated as-is and the engine retried the same doomed open + // forever. Route it to the same policy as a file no password opens: + // without a passphrase, the rebuild recreates the identical wallet at + // the cost of a re-scan; with one, an unreadable file cannot + // corroborate the passphrase, and a wrong one would rebuild a + // different wallet, so refuse. + if (isInvalidFile(error)) { + if (seedPassword !== '') { + throw new Error( + 'The Zano wallet file is unreadable, and cannot be rebuilt ' + + 'because a seed passphrase is set' + ) + } + log('Zano wallet file is unreadable, rebuilding it') + return await started(await rebuild()) + } + // Anything other than a bad password -- including ALREADY_EXISTS, // which callers recover from by adopting the open wallet -- is not // ours to handle. diff --git a/test/startWallet.test.ts b/test/startWallet.test.ts index 56aaf3f..c938cab 100644 --- a/test/startWallet.test.ts +++ b/test/startWallet.test.ts @@ -416,7 +416,11 @@ describe('startWallet', () => { assert.ok(!state.calls.join('\n').includes('deleteWallet')) }) - it('does not try legacy passwords after a non-password failure', async () => { + it('rebuilds an unreadable file for a wallet with no passphrase', async () => { + // A crash during the file's first write leaves a file the SDK cannot + // parse -- INVALID_FILE before any password is consulted. The engine + // used to retry the same doomed open forever; the rebuild recreates the + // identical wallet from the mnemonic at the cost of a re-scan. const state = makeState({ filePassword: DERIVED, mnemonic: MNEMONIC }) state.openResult = JSON.stringify({ id: 0, @@ -425,7 +429,35 @@ describe('startWallet', () => { }) const bridge = makeBridge(state) - await assert.rejects(bridge.startWallet(MNEMONIC, '', STORAGE_PATH)) + const wallet = await bridge.startWallet(MNEMONIC, '', STORAGE_PATH) + + assert.ok(state.calls.join('\n').includes('deleteWallet')) + assert.equal(state.files.get(STORAGE_PATH)?.filePassword, DERIVED) + assert.deepEqual([...(state.runningWallets ?? [])], [wallet.wallet_id]) + // The password ladder has nothing to probe on a file that fails before + // password checking, so the failing open must be the only one: + assert.equal(state.calls.filter(call => call.startsWith('open(')).length, 1) + }) + + it('refuses to rebuild an unreadable file for a passphrase wallet', async () => { + // An unreadable file cannot corroborate the passphrase, and rebuilding + // with a wrong one would restore a different wallet, so this stays the + // same refusal the no-password-opens path applies: + const state = makeState({ filePassword: 'hunter2', mnemonic: MNEMONIC }) + state.openResult = JSON.stringify({ + id: 0, + jsonrpc: '2.0', + error: { code: 'INVALID_FILE', message: '' } + }) + const bridge = makeBridge(state) + + await assert.rejects( + bridge.startWallet(MNEMONIC, 'hunter2', STORAGE_PATH), + /unreadable/ + ) + + assert.ok(!state.calls.join('\n').includes('deleteWallet')) + assert.equal(state.files.get(STORAGE_PATH)?.filePassword, 'hunter2') assert.equal(state.calls.filter(call => call.startsWith('open(')).length, 1) })