FIX: ImportWallet and ImportSpeed derive the same seed at different paths - #151
Merged
theanmolsharma merged 2 commits intoSep 2, 2026
Conversation
There was a problem hiding this comment.
Pull request overview
This PR unifies how HDSilentPaymentsWallet instances are constructed from a mnemonic across the two import screens to ensure both screens derive the same wallet (same derivation path and addresses), and aligns ImportSpeed’s invalid-mnemonic error with the existing localized string used elsewhere.
Changes:
- Added
HDSilentPaymentsWallet.fromMnemonic(mnemonic)factory and updated both import screens to use it (removingImportWallet’s prior hardcodedm/84'/0'/0'override so both rely on the BIP86 defaultm/86'/0'/0'). - Localized
ImportSpeed’s invalid mnemonic error usingloc.wallet_birth.error_invalid_mnemonic. - Added unit tests covering the new factory behavior and ensuring it matches manual setup for the same mnemonic.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 1 comment.
| File | Description |
|---|---|
class/wallets/hd-bip352-wallet.ts |
Adds fromMnemonic() factory to centralize mnemonic-based wallet construction without overriding derivation path. |
screen/wallets/ImportWallet.tsx |
Switches import flow to use fromMnemonic() (removing the previous hardcoded derivation path). |
screen/wallets/ImportSpeed.tsx |
Switches import flow to use fromMnemonic() and replaces hardcoded English error with loc. |
tests/unit/bip352.test.ts |
Adds tests verifying fromMnemonic() uses the BIP86 default path and matches manual setup outputs. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
theanmolsharma
requested changes
Sep 1, 2026
…aths ImportWallet.tsx forced m/84'/0'/0' after setSecret; ImportSpeed.tsx never overrode the path, so it fell through to HDTaprootWallet's m/86'/0'/0' default. Same seed, imported via the two screens, watched two different sets of addresses. Adds HDSilentPaymentsWallet.fromMnemonic(mnemonic), a shared factory both screens now call, deliberately not overriding the derivation path so both land on the class default. Also fixes ImportSpeed.tsx's hardcoded English error string to use loc.
- hd-bip352-wallet.ts: add static readonly derivationPath, override setDerivationPath to throw (SP wallets never move off BIP-86), and let fromMnemonic take an optional passphrase - ImportSpeed.tsx: invalid-mnemonic path now uses presentAlert + return (matching ImportWallet.tsx) instead of throw/catch with an untitled bare-message alert; passphrase passed into the factory instead of a separate setPassphrase call - ImportWallet.tsx: drop redundant .trim() (setSecret already trims) - bip352.test.ts: split the derivation-path invariant into its own describe block (was previously exercising the constructor default under a misleading 'fromMnemonic' describe), replace the tautological factory-vs-manual-setup comparison with a golden-vector test cross-checked against hd-taproot-wallet.test.ts's independent fixture - importWallet.test.tsx: fix a pre-existing assertion still expecting the old m/84' path this PR removes Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01S9qoCZXTiuCXN2GXZANv7C
chaitika
force-pushed
the
fix/import-derivation-path-mismatch
branch
from
September 2, 2026 08:28
8ce71d6 to
29a11d5
Compare
theanmolsharma
approved these changes
Sep 2, 2026
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
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.
Problem
ImportWallet.tsxandImportSpeed.tsxboth build anHDSilentPaymentsWalletfrom a mnemonic (setSecret+ validate), but onlyImportWallet.tsxcalledsetDerivationPath("m/84'/0'/0'")afterward.ImportSpeed.tsxnever did, so it fell through toHDTaprootWallet's class default (m/86'/0'/0'). Same seed, imported through the two screens, watched two different sets of addresses.ImportSpeed.tsxalso threw a hardcoded English error string instead of usingloc.Fix
HDSilentPaymentsWallet.fromMnemonic(mnemonic), a shared factory both screens now call. It doesn't override the derivation path — both land on the class defaultm/86'/0'/0', one source of truth instead of two that can drift.ImportSpeed.tsx's error now usesloc.wallet_birth.error_invalid_mnemonic(same keyImportWallet.tsxalready uses for this case).Left the pre-existing dead validation condition in
ImportWallet.tsx(!wallet.validateMnemonic() && !wallet.getSecret()) untouched — that's #145's fix, out of scope here.Testing
New
fromMnemonicblock intests/unit/bip352.test.ts:m/86'/0'/0'.hd-taproot-wallet.test.ts's independentHDTaprootWalletfixture for the same mnemonic — exact match.ImportWallet.tsxandImportSpeed.tsxnow produce byte-identical wallets for the same seed.npm run lint && npm run tslint && npm run unitpass.