scan multiple ZIP 32 account indices during recovery#59
Open
evanbranigan-dotcom wants to merge 4 commits into
Open
scan multiple ZIP 32 account indices during recovery#59evanbranigan-dotcom wants to merge 4 commits into
evanbranigan-dotcom wants to merge 4 commits into
Conversation
The ZWL parser unconditionally read Orchard keys from all wallet.dat files, but the original ZecWallet Lite format (Keys version <= 21) never included Orchard keys. Pre-Orchard wallets panicked with "Unable to deserialize a valid Orchard FullViewingKey from bytes" because the parser tried to interpret Sapling key bytes as Orchard data. Skip Orchard key deserialization for Keys version <= 21 and read fields in the original order (zkeys, tkeys — no okeys). Also replace unwrap() in WalletParserFactory::read() with proper error propagation so parse failures surface as error messages instead of panics. Closes zingolabs#36 AI Disclosure: Claude (Anthropic) assisted with codebase analysis and patch authoring. All code was reviewed and understood by the committer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
ZecWallet Lite scattered funds across multiple ZIP 32 account indices (m/32'/133'/0', m/32'/133'/1', etc.) instead of using diversified addresses from a single account. Recovery must scan all indices. - Wallet file path: detect account count from parsed wallet, add 5 gap-limit buffer, pass to LightWallet::new() - Mnemonic recovery: default to 5 accounts - Aggregate balances across all synced accounts in both sync log and export view - Log per-account balances when non-zero for visibility Contributes to zingolabs#2 AI Disclosure: Claude (Anthropic) assisted with codebase analysis and patch authoring. All code was reviewed and understood by the committer. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Verify that the parser detects multiple accounts from ZecWallet Lite's scattered-index wallets (w2: 8 accounts) and correctly handles pre-Orchard wallets (w3: version 24, 11 sapling keys, 0 orchard keys). Test vectors from zwl-lib/test_vectors/. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Background
ZecWallet Lite incorrectly generated new Sapling addresses by incrementing the ZIP 32 account index (
m/32'/133'/N') instead of using diversified addresses from a single key. Funds are scattered across multiple account indices.Zingolib's
UnifiedSpendingKey::from_seed()derives Sapling keys at the same ZIP 32 paths — so bumpingno_of_accountsinLightWallet::new()finds the scattered funds without custom derivation logic.Test plan
test_multi_address_wallet_account_count— w2 wallet parses with 8 accounts detectedtest_pre_orchard_wallet_parses— w3 pre-Orchard wallet (v24) parses without crash, 0 okeysNote: Includes the parser fix from #58 (pre-Orchard wallet support). If #58 hasn't merged yet, this PR depends on it.
Contributes to #2
AI Disclosure
Claude (Anthropic) assisted with codebase analysis, derivation path verification, and patch authoring. All code was reviewed and understood by the committer.