Encrypt the wallet file with a password derived from the mnemonic - #16
Encrypt the wallet file with a password derived from the mnemonic#16peachbits wants to merge 4 commits into
Conversation
87b3eb0 to
6e960d4
Compare
e20c16b to
cad72f2
Compare
cad72f2 to
68e15cd
Compare
|
Review pass over this PR. The migration logic is careful and the test suite is genuinely thorough, so most of this is about edge paths rather than the core design. I verified the derivation and the migration matrix by running the suite locally at this head (27 passing, 1.
|
68e15cd to
01e398f
Compare
05ea428 to
0c3390e
Compare
01e398f to
1afe40c
Compare
8130a02 to
56496f5
Compare
1afe40c to
be35dad
Compare
56496f5 to
9e77912
Compare
be35dad to
5982fd2
Compare
9e77912 to
8070c84
Compare
b53644a to
c0a000c
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 c0a000c. Configure here.
c0a000c to
6292d00
Compare
|
As of 1. 2. The SDK-folder tripwire cannot fail the build. Fixed on #15, since that PR is what rewrites the script. 3. A transient verify-reopen discards a migrated file. Fixed. Once the close has reported OK the file really is re-keyed, so only a wrong password on the confirming open means the migration missed. Anything else leaves the file alone. 4. Rebuilding on "no known password" is a silent history wipe. Kept, but narrowed, and the narrowing came out of this. Rebuilding restores from the mnemonic with Detecting a weakly keyed file is unaffected and there is a test pinning it: a wallet that has a passphrase but whose 0.3.0 file was keyed with For wallets with no passphrase it still rebuilds behind a log line. That is your original point, and it is a deliberate call on our side. 5. Em-dash in the changelog. Fixed. 6. 7. Nothing runs the new test suite. Still open. The repo has no |
`startWallet` used its one `seedPassword` argument for two unrelated roles: decrypting the seed phrase, and encrypting the wallet file on disk. The seed passphrase is the empty string for most wallets, so their files -- which hold the seed and spend keys -- were effectively unencrypted, and on iOS they live in the documents directory and reach device backups. The file password is now derived from the mnemonic (domain-separated SHA-512, 32 hex characters -- inside Zano's 40-character password limit, which `wallet2::generate` enforces). Files written by earlier versions are re-keyed in place on their first open: open with the legacy password, `resetWalletPassword`, close to persist, and reopen to verify. A file no known password opens is deleted and rebuilt from the mnemonic, costing one re-scan. The migration is decided entirely by what the file does, so it is idempotent and self-healing. Three native-layer details shape the implementation: - `reset_wallet_password` only assigns the in-memory password; closing the wallet is what re-encrypts the file, so the close result is checked and the migration verified by reopening. - `closeWallet` is used rather than `stopWallet`, whose native path discards the close result and always reports OK. - The catch-all macros report failure as success-shaped payloads (`INTERNAL_ERROR`, `UNINITIALIZED`), which previously resolved into a wallet with an undefined `wallet_id`. `handleRpcResponse` now rejects those, and errors are `ZanoError` instances carrying the parsed code while keeping the historical message shape. `generateSeedPhrase` now deletes the wallet file it writes as a side effect of generating a seed, and `startWallet` gains an optional `log` callback so callers can surface migration events. Everything is covered by a new mocha suite running the bridge against a fake native module that mirrors the reset-then-close-to-persist semantics. Two failures are deliberately not treated as a reason to rebuild. An `ALREADY_EXISTS` from the verifying reopen means another handle took the wallet in the window after our close, and callers recover from that by adopting the open wallet, so it is rethrown rather than answered by deleting an already-migrated file. A close that cannot be confirmed leaves the wallet open on the file, so the file is left alone -- still keyed with the legacy password, and migrated again on the next start. Two failure paths deliberately do not rebuild, because rebuilding costs a full rescan and can loop. The native delete reports OK whether or not it removed anything, so `rebuild` confirms the file is gone before restoring -- restoring onto a survivor answers ALREADY_EXISTS, which callers recover from by adopting an open wallet, and none is open, so every start would rebuild again. And once the close has reported OK the file really is re-keyed, so only a wrong password on the confirming open means the migration missed; anything else leaves the file alone. A file that no known password opens is only rebuilt when the wallet has no seed passphrase. With one set, every password this package writes has already been tried -- the derived one, the passphrase, and the empty string 0.3.0 used -- so the passphrase is the likeliest thing to be wrong, and rebuilding would restore a different wallet from the same mnemonic over a file that was intact. Zano's checksum rejects most wrong passphrases, so that usually meant a deleted file and a failed restore; the rest of the time it meant keys that are not the user's, opening cleanly ever after because the file password comes from the mnemonic alone and cannot tell the two apart. Detecting a weakly-keyed file is unaffected: the passphrase and the empty string are both tried before this point, so a wallet that has a passphrase whose 0.3.0 file was keyed with '' still re-keys in place. The post-delete check asks `isWalletExist` rather than reading `getWalletFiles`, which can answer without an `items` field -- treating that as proof the file was removed would restore onto a survivor, the retry loop the check exists to stop. The same rule governs the re-key failure path. Rebuilding restores from the mnemonic with `seedPassword`, so it only reproduces this wallet when that is the passphrase the file was written with. Opening the file with `seedPassword` proves it; opening it with '' does not, and 0.3.0 wrote '' for wallets it believed had none. When those disagree the file that just opened is the user's wallet and the rebuild would not be, so it is left alone.
The wallet files hold the seed and spend keys. They live in the app's documents directory, so they reach device backups -- and a Finder backup is unencrypted unless the user opts in. Encrypting the file (previous commit) protects it at rest; this keeps it out of the backup entirely. `init` derives three directories from the working directory we pass it -- `wallets`, `logs` and `app_config` -- and creates them on first use. Only we can set the backup flag, and that needs the directory to exist, so create all three up front and flag them there. The SDK is happy to find them already present. Nothing in the app writes to `app_config` today, since the plugin never calls `setAppconfig`, but `init` creates it regardless and the flag costs nothing. Android needs no equivalent: it stores these in private app storage, and the app sets `android:allowBackup="false"`. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
The directories to pre-create and exclude from backups (previous commit) cannot be derived at build time, so the list is maintained by hand in `ios/ZanoModule.mm`. Nothing made that obvious, and the omission that prompted this -- `app_config`, which `init` has always created -- went unnoticed precisely because it was silent. A new SDK directory can only arrive with a `zano_native_lib` pin bump, and `update-sources.ts` is what performs that bump, so it is the one place that always sees the new source. Without a check there, a pin bump reviews as a changed commit hash and nothing else: `tmp/` is gitignored, so the SDK source never appears in a diff. `checkSdkFolders` therefore runs right after the sources are downloaded, reads the folder names the SDK declares, and throws if any is missing from our list. It is a tripwire rather than a proof: it matches `#define`d folder names, which is how the SDK has always declared them, but a path composed inline would slip past. The matching lives in `scripts/utils/sdkFolders.ts` so it can be unit tested. `update-sources.ts` calls `main()` at module scope, so importing it from a test kicks off a native build. Verified against both the previous SDK pin and the current HF6 one: each declares exactly `app_config`, `logs` and `wallets`, and injecting a fourth into the real source makes the check fail.
6292d00 to
e209928
Compare
`update-sources` patches `RIPEMD160.h` to rename a function that collides with Zlib, and that file lives inside the `Zano` submodule. `git submodule update` refuses to move a submodule across a commit that changes a locally modified file, so once the patch is in place the next run aborts with "Your local changes would be overwritten by checkout" and takes the whole script with it. That happens whenever the submodule has to move and `RIPEMD160.h` differs across the move, which is exactly what an SDK pin bump does, and what a run interrupted mid-move leaves behind. `prepack` runs this script, so a build that cannot repeat is a publish that fails on the second attempt. `--force` discards the patch before the checkout, and `downloadSources` re-applies it immediately afterwards, so the build is unchanged and now repeatable. Reproduced both directions against the real submodule: without the flag the update exits 1 on a commit pair where the file differs, with it the update lands on the pinned commit and the working tree is clean.



Fixes the wallet-file half of Zano wallet file passphrase (Kimi K3 F8a, High / conditional Critical), relocated here from EdgeApp/edge-currency-accountbased#1082 so the library that owns the wallet-file API owns its encryption. The accountbased PR shrinks to offline mnemonic utilities.
The problem
startWalletused its oneseedPasswordargument for two unrelated roles: decrypting the seed phrase, and encrypting the wallet file on disk. The seed passphrase is the empty string for every wallet created in-app and every import that does not set one, so those files — which hold the seed and spend keys — were effectively unencrypted. On iOS they live in the documents directory, which reaches iCloud/iTunes backups.The fix
The file password is now derived from the mnemonic: domain-separated SHA-512, first 16 bytes as 32 hex characters. 32 chars is deliberate — Zano caps wallet-file passwords at 40 over a restricted alphabet (
PASSWORD_REGEXP, enforced insidewallet2::generate), so a longer password would passrestore/openand then fail the moment anything calledgenerate. The derivation is pinned by a golden-vector test; changing it orphans every file already encrypted with it.Migration happens inside
startWallet, decided entirely by what the file does (idempotent, self-healing, no persisted state):restorewith the derived file password and the real seed password, now distinct.WRONG_PASSWORD→ try the legacy passwords (seedPassword, then''), and on a hit:resetWalletPassword→closeWallet→ reopen to verify.Signature is backward-compatible (new optional
logcallback only), and thrown errors keep the exact historical<code> <message>shape — accountbased'serror.message.includes('ALREADY_EXISTS')recovery is pinned by a test, trailing space included.Why the sequence is shaped this way
Verified against the pinned SDK (
zano_native_lib239d4a39→ Zanod6be0ecf), each of these silently corrupts wallets if ignored:reset_wallet_passwordonly assigns the in-memory password (wallet2.cpp:3196). The file is re-encrypted when the wallet next stores, which closing does — so the close result is checked and the migration only believed after a verify-reopen.closeWallet, notstopWallet: the async'close'path discardsclose_wallet's return code and always reports OK (plain_wallet_api.cpp:779-785), so it cannot confirm the file was written.store(path, password): it encrypts the keys blob with the argument but the body withm_password(wallet2.cpp:3500vs:3524), producing a file that opens, fails to deserialize its body, wipes history, and silently re-scans. There is a comment against it in the code.Also fixed while here: the catch-all macros report failure as success-shaped payloads (
{result:{return_code:"INTERNAL_ERROR ..."}}, orUNINITIALIZEDbeforeinit), which previously resolved into aWalletDetailswith an undefinedwallet_id. AndgenerateSeedPhraseno longer leaves its side-effect wallet file on disk — the caller only wants the seed, and the firststartWalletrecreates the file properly viarestore.Testing
New mocha suite (
npm test, first test infrastructure in this repo): 21 tests running the realCppBridgeagainst a fake native module at thecallZanostring-protocol level. The fake mirrors the semantics that matter —resetWalletPasswordmutates only in-memory state, and closing is what persists it — so dropping the close-to-persist step fails the suite. Covered: fresh restore with distinct passwords, re-key from''and from a real passphrase (exact call sequence asserted), idempotence, failed reset, failed close, no-known-password rebuild,ALREADY_EXISTSrethrow with message-shape pin, success-shaped error payloads, and the derivation golden vector.Pure TypeScript — no native code changes. The
.cpp/binary layer is untouched;resetWalletPasswordwas already wired natively.Release notes for whoever publishes
This should ship as 0.4.0 (behavior change: files get re-keyed on open). Both consumers currently declare
^0.3.0, which does not admit 0.4.0 — accountbased and the GUI bump their pins at merge time, per the usual stacked-PR flow.prepackruns the full native rebuild; beforenpm publish, verify the packed tarball contains all four Android.sofiles and the iOS xcframework, with sizes comparable to 0.3.0 — two past releases shipped with bad binaries.Not yet done, required before shipping: the on-device QA pass, in particular sending from a migrated wallet, which is the only real proof the re-key preserved the spend key.
Note
High Risk
Changes wallet file encryption, on-disk migration, and backup exclusion for seed-bearing paths; mistakes could corrupt wallets, leak keys via backups, or mis-handle passphrases.
Overview
Wallet files are no longer encrypted with the seed passphrase (often empty).
deriveWalletFilePasswordhashes the normalized mnemonic into a stable 32-character file password;startWalletuses it forrestore/openwhile keeping the seed passphrase separate.startWalletnow migrates existing files: open with the derived password, fall back to legacy passwords, re-key viaresetWalletPassword+closeWalletwith verify-reopen, or delete/rebuild when safe. Wrong seed passphrases and concurrentALREADY_EXISTScases throw instead of overwriting. An optionallogcallback reports recovery and re-key events.ZanoErrorcarries native return codes;handleRpcResponsethrows on error payloads and non-OKreturn_codevalues so callers do not get wallets with undefinedwallet_id.generateSeedPhrasedeletes the side-effect wallet file after closing.On iOS,
ZanoModulepre-createswallets,logs, andapp_configand marks them excluded from backups.update-sourceschecks SDK folder#defines against that list; submodule update uses--forcefor repeatableprepack.Adds
npm test(mocha + fake native module),tweetnaclfor derivation, andtsconfig.eslint.jsonfor lint/verify over tests and scripts.Reviewed by Cursor Bugbot for commit f69632f. Bugbot is set up for automated code reviews on this repo. Configure here.