Zano: offline seed phrase generation and validation - #1082
Conversation
493692c to
51f1111
Compare
51f1111 to
174f047
Compare
6d4b60a to
1334847
Compare
The previous build emitted a 25-word seed phrase with a trailing space instead of 26 words whenever the checksum landed on CHECKSUM_MAX, about one seed in 814. That path is already reachable from `parseUri`, which decodes scanned base58 raw seeds through `seedToMnemonic`. The new build also exports `verifySeedPhrase` and `isSeedPhrasePasswordProtected`, so seed phrases can be validated without calling into the native Zano library. Add a test that guards the pin, since downgrading it would silently hand users malformed backup phrases again.
1a81ab9 to
8871cf1
Compare
|
Review pass over the move to offline seed generation and validation. 1. Dropping the checksum result is a regression, and the comment justifying it does not hold
The first half is right. The second half is not, for 26-word phrases. Traced through the SDK:
So a 26-word phrase with a wrong checksum word used to be rejected at import and now imports silently, deriving an address for the wrong account. That is the realistic transcription error: one word mistyped into another word that also happens to be in the dictionary. Honoring the return value cannot break 25-word imports, because Suggest 2. The new address guard leaks a native handle and disables itself (
|
8871cf1 to
7d945e5
Compare
cf708f0 to
8ec7f6b
Compare
There was a problem hiding this comment.
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 8ec7f6b. Configure here.
`deriveAddressFromMnemonic`, `validateMnemonic`, `normalizeMnemonic`, `verifyMnemonicChecksum` and `mnemonicMatchesKeysSeed` derive, validate and self-check Zano seed phrases in pure JS, with no native library involved. Nothing calls them yet. Only phrases without a seed passphrase can be handled this way: the JS library cannot decrypt a protected seed, and Zano core asserts that the phrase's password flag matches the password supplied to it, so a protected phrase decoded without its passphrase yields a wrong address rather than an error. The helpers detect that case and throw. `validateMnemonic` enforces the checksum word on 26-word phrases, matching the native path it replaces: `account_base::restore_from_seed_phrase` returns false on a mismatch and `get_seed_phrase_info` then reports an empty address, which callers threw on. Without it, one word mistyped onto another dictionary word imports silently as a different account. 25-word phrases carry no checksum word and are unaffected, which is what Zano core does too. Every helper here refuses a passphrase-protected phrase, and the module says so. `verifySeedPhrase` throws on one by itself, but `mnemonicToSeed` does not -- it answers with a seed derived without decrypting, so a protected phrase produces a wrong key rather than an error. Guarding `mnemonicMatchesKeysSeed` rather than trusting its callers keeps that from reading as a plain mismatch. Protected phrases go to the native library through `ZanoTools`, which is the only implementation that supports them.
… library
`createPrivateKey` obtained a seed phrase by calling
`generateSeedPhrase`, which writes a wallet file to disk as a side
effect and can only encrypt it with the seed passphrase -- the empty
string, for every wallet created in the app. Generate the phrase from
the plugin's own entropy instead, so no wallet file exists until the
engine creates one, and creating a wallet starts nothing native at all.
A generated phrase is the user's only backup, so it is self-checked
twice before being returned, both times offline: the words must decode
back to the entropy they were built from, and the checksum word must
match them. The round-trip covers the encoder, which is where the
25-word phrases came from.
There is deliberately no native cross-check. `ZanoEngine` compares the
native wallet's address against this phrase's derived one every time the
wallet starts, so a JS/native disagreement still stops the wallet before
it can sync, and it does so against the real wallet rather than
instance 0. Starting the SDK -- creating its working directories and log
file -- is not something wallet creation should be doing to re-derive an
address we already have.
`importPrivateKey` and `derivePublicKey` now validate and derive offline
when no passphrase is involved, which is the common case and the one
`parseUri` hits for every scanned payload. Phrases protected by a seed
passphrase still go through the native library, which is the only
implementation that supports them: `@zano-project/zano-utils-js` has no
decryption path at all, and `mnemonicToSeed` takes no password. Those
two branches keep their own `init` calls, which cannot be left to
`ZanoEngine` -- both run before an engine for the wallet exists.
Imported phrases are stored normalized, since react-native-zano derives
the wallet-file password from the stored phrase.
`derivePublicKey`'s passphrase branch checks the native response the same
way `importPrivateKey` does on the identical call. Without it a wrong
passphrase returned `{ publicKey: '' }` and the wallet was created with an
empty address instead of failing, while the offline branch alongside it
threw.
…blic key The public key is now derived from the seed phrase in pure JS for wallets without a passphrase, so check it against the address the native wallet actually reports when it opens. A mismatch fails engine startup loudly instead of syncing a wallet whose address is not the one shown to the user. Two things keep the check from being trivially bypassed. It closes the wallet before throwing, since `startWallet` has already opened it and the lifecycle manager does not run `onStop` for an `onStart` that threw -- left open, the handle leaks and the next start finds it through ALREADY_EXISTS. And the ALREADY_EXISTS recovery path applies the same comparison to the wallet it adopts, so retrying cannot walk around the check.
The dev and peer ranges were `^0.2.7` while edge-react-gui installs `^0.3.0`. Caret ranges below 1.0 do not widen past the minor version, so those two do not overlap and the peer dependency was unsatisfiable -- visible only because both repos set `legacy-peer-deps`.
`react-native-zano`'s `startWallet` takes an optional `log` callback and uses it to report what happened to the wallet file on disk: re-keyed with the derived password, rebuilt because no known password opened it, or recovered by the native library. We passed no callback, so all of that was silent -- a wallet that lost its history to a rebuild looked identical to one that migrated cleanly. Route it to `this.log.warn` so the events land in the wallet log alongside the rest of the engine's output. Requires the unreleased `startWallet` overload from react-native-zano; the dependency pin follows when that publishes. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
`getDisplayPublicSeed` fetched the private view key through
`getWalletInfo`, which calls `wallets_manager::get_wallet_info` -- an
unbounded `lock()` on the per-wallet mutex, with no timeout. Its
neighbour `get_recent_transfers` uses `try_lock` and returns CORE_BUSY
instead, so the blocking behaviour here looks unintentional.
That matters because the GUI asks for this key for every wallet a few
seconds after login, as part of its compromised-key sweep. The native
module runs its calls on one serial queue, so a single wallet that is
mid-refresh holds the lock and every other Zano call queues behind it --
taking the app with it. Sampling a frozen app showed 100% of samples
parked in `plain_wallet::get_wallet_info` under `__psynch_mutexwait`.
`getOpenedWallets` returns the same secret without the lock: it takes
only the shared wallet-map lock and fills each entry with
`get_wallet_info_unlocked`. `wi.view_sec_key` and
`wi_extended.view_private_key` are both `pod_to_hex(view_secret_key)`.
Both calls read the same unfiltered `m_wallets` map, so a wallet we hold
an id for is always listed -- there is no case where the old call could
answer and this one could not. Rather than keep it as a fallback that
would silently reintroduce the lock, throw: if that ever fires, the
assumption above is wrong and we want to see it.
Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The catch reports `String(error)` rather than `JSON.stringify(error)`,
which yields `{}` for an Error and made every failure here read the same.
The list-and-find is now one `listOpenedWallets` helper, shared with the
ALREADY_EXISTS recovery path in `onStart` that was already doing the same
shape check. The failure names the wallet id, since reaching it usually
means the wallet closed or was still opening between the id being handed
out and this snapshot -- a race rather than the bug the old wording
suggested.
8ec7f6b to
ec25287
Compare
|
As of 1. Dropping the checksum result is a regression. Confirmed against the SDK and fixed. Two existing tests asserted the old behavior and are inverted. One justified itself by claiming its phrase was the fixture used by 2. The address guard leaks a handle and disables itself. Confirmed and fixed. 3. 4. 5. The view-key change, with one caveat. Both parts done. The failure names the wallet id and says what it means, that the wallet closed or was still opening between the id being handed out and the snapshot. The list-and-find is now one 6. The tests cannot catch a JS versus native divergence. Still true as written. The fake answers 7. The PR cannot compile against the dependency it pins. Expected. The lockfile cannot point at #16 until it publishes, so this clears at merge time in the order you set out. |

CHANGELOG
Does this branch warrant an entry to the CHANGELOG?
Dependencies
v0.0.4-edge.2, already pinned here.^0.4.0) lands here at merge time, per the usual stacked-PR flow, along with thesecurity:changelog entry crediting it.Description
Part of Zano wallet file passphrase (Kimi K3 F8a). Scope note: this PR was originally the whole fix, including the engine-side wallet-file re-key migration. That migration now lives inside
react-native-zano'sstartWallet(EdgeApp/react-native-zano#16), where the library that owns the wallet-file API owns its encryption — so this PR shrank to the accountbased-side work: generating and validating seed phrases offline.What changed
createPrivateKeyobtained a seed phrase by callinggenerateSeedPhrase, which writes a wallet file to disk as a side effect and can only encrypt it with the seed passphrase — the empty string for every wallet created in the app. It now generates fromio.random(32)through@zano-project/zano-utils-js, so no wallet file exists until the engine creates one — by which point react-native-zano encrypts it with a derived password.A generated phrase is the user's only backup, so it is self-checked twice before being returned: against the checksum rules, and against the native library's own address derivation (
getSeedPhraseInforuns on instance 0 — no wallet opened, no file written).importPrivateKeyandderivePublicKeyvalidate and derive offline when no passphrase is involved — the common case, and the oneparseUrihits for every scanned payload. Passphrase-protected phrases still use the native library, the only implementation that supports them: the pure-JS library has nocrypt_with_pass, and Zano core asserts the phrase's password flag matches the supplied password, so decoding a protected phrase without it yields a wrong address rather than an error. The helpers detect that flag and throw instead.Imported phrases are stored normalized (trimmed, single-spaced), since react-native-zano derives the wallet-file password from the stored phrase.
The engine gains one line of defense: after
startWallet, the native wallet's address must match the storedpublicKey, failing startup loudly rather than syncing a wallet whose address is not the one shown to the user. No-op for existing wallets (their keys were derived natively); a hard guard for everything created or swept after this change.Deliberate choices
get_seed_phrase_infopath never enforced it, and scanned v2 full seeds embed whatever checksum word their payload carries — rejecting would break imports that work today. Pinned by a test whose fixture is exactly such a phrase.v0.0.4-edge.2) fixes a real bug already reachable in production: ~1 in 814 generated phrases came out as 25 words plus a trailing space, because the JS library was missing Zano core's checksum wrap-around clamp. A guard test keeps the pin from regressing.Testing
520 unit tests pass. New coverage: the offline helpers (golden-vector address derivation, passphrase-flag detection, checksum boundary), and
ZanoToolskey operations against a recording fake bridge — including the security regression test thatcreatePrivateKeyperforms no file-writing native calls, and that no-passphrase import/derive make zero native calls.Device QA happens on the combined build with react-native-zano#16; the checklist lives on that PR.
Note
High Risk
Changes wallet creation, seed backup, address derivation, and startup guards—any bug could produce unrecoverable or mismatched wallets; mitigated by self-checks, engine address verification, and broad tests.
Overview
Zano wallet creation and key handling no longer depend on the native SDK for the common (no seed passphrase) path. New pure-JS helpers in
zanoMnemonic.tsnormalize phrases, derive addresses, validate checksums, and self-check generated entropy.createPrivateKeybuilds 26-word phrases fromio.randomvia@zano-project/zano-utils-jswith round-trip and checksum checks—nogenerateSeedPhraseand no wallet file on disk at create time.importPrivateKey/derivePublicKeyvalidate and derive offline without a passphrase; passphrase-protected seeds still go throughgetSeedPhraseInfo. Imported mnemonics are stored normalized.ZanoEnginefails startup if the native wallet address does not match storedpublicKey(including when adopting anALREADY_EXISTSwallet), closes mismatched handles to avoid leaks, forwards wallet-file migration logs fromstartWallet, and reads the private view key viagetOpenedWalletsinstead of blockinggetWalletInfo.Pins
@zano-project/zano-utils-jsv0.0.4-edge.2 (checksum wrap-around fix) and bumpsreact-native-zanoto ^0.3.0 for peer alignment. New unit tests cover mnemonic helpers, key flows, and the checksum regression guard.Reviewed by Cursor Bugbot for commit ec25287. Bugbot is set up for automated code reviews on this repo. Configure here.