Import a Sphere wallet from a recovery phrase or JSON export - #6
Open
KruGoL wants to merge 10 commits into
Open
Conversation
… reloads on its own branch
…cope boundary with a test
|
Caution The consumer version of Gemini Code Assist on GitHub has been sunset. All code review activity has officially ceased. |
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.
Closes #5.
Lets a wallet created in Sphere be loaded here — by pasting the 12/24-word recovery phrase or by uploading Sphere's JSON export — and land on exactly the address Sphere shows.
The phrase matters more than the JSON: in unicity-sphere/sphere-sdk#696 the user has only the 12 words, because Sphere presents them as the recovery method. Until now this wallet had no BIP39 support at all.
The derivation was already right
This wallet's BIP32 code needed no changes. It reproduces sphere-sdk's own test vectors and a real Sphere export byte for byte:
alpha1addresses atm/44'/0'/0'/0/{i}—DEFAULT_BASE_PATH+/chain/index, bech32 witness v0, hrpalpha. It writes that path into its export asdescriptorPath: "44'/0'/0'", which is this wallet's own field name and format.deriveAddressAtIndex()fed that path produces the same public key Sphere recorded, andcreateBech32()reproduces addresses generated by the SDK'spublicKeyToAddress.The first test in the suite locks that in: it runs the existing derivation against the vectors from
sphere-sdk/tests/integration/wallet-derivation.test.ts(wallets documented there as "exported from webwallet") and fails if it ever drifts.What was missing
HMAC-SHA512("Bitcoin seed"). No new dependencies.CryptoJS.AES.encrypt(value, password)directly, not the PBKDF2 scheme this wallet uses for its own.txtbackups, so it needs its own decrypt path.descriptorPathwas never persisted.generateNewAddress()reads it and falls back tom/84'/1'/0'when it is absent. Every path below that leaked it, which is how one wallet ends up with addresses on two different branches.Bugs found and fixed along the way
.jsonwas routed into the wallet.dat/text parser. The file-inputchangehandler only knew.txtand.dat; anything else fell into the automatic discovery flow, which hides the Load button and feeds the file toextractFromWalletDat()→ "Could not extract master key from file".performWalletScan()droppeddescriptorPathwhen rebuildingextractedWalletData, so the scan walked the84'/1'/0'fallback instead of the wallet's branch. Harmless for.datwallets by coincidence, wrong for anything else. Extracted asmergeScanWalletData()and covered by tests.selectWalletForImport()built the wallet withoutdescriptorPath, losing the branch again after the user picked a scanned address.saveWallet()did not write aDESCRIPTOR PATH:line, and the string appeared nowhere in the file — so even Sphere's ownexportToTxt, which does write it, was imported onto the wrong branch. Import → save.txt→ load that.txtproduced a different address.For backups already saved without the line,
resolveDescriptorPathFromBackup()recovers the branch by deriving each candidate and matching the address the file itself records — an exact check against the file, not a guess. When nothing matches it returns null and the previous default stands.Safety
addresses[0].publicKey; the derived key is compared against it and the import is refused on mismatch, rather than silently showing a different address.44'/0'/0'. A phrase carries no address to check against, so the path is fixed to Sphere's — any other choice would show a different address than Sphere does for the same words.balance > dustThreshold.Compatibility
Old non-BIP32 (WIF/HMAC) backups are unaffected, and this is asserted rather than assumed. The standard-wallet branch of
saveWallet()is untouched;resolveDescriptorPathFromBackup()returns null without a chain code; and a wallet with no chain code stays on HMAC derivation even if a path is forced on it. Pre-existing BIP32 backups still resolve to84'/1'/0'and yield the same address as before.Tests
node test_sphere_import.js— 75 assertions, no framework. The functions under test are extracted fromindex.htmlat runtime, so the tests exercise the shipped file rather than a copy that can drift.One of them guards a trap specific to this file:
index.htmlhas two scopes inside one<script>— functions indented 8 spaces are top level, those indented 12 live inside theDOMContentLoadedcallback. A top-level function calling an inner one throws only when the user clicks, which looks like "the button does nothing" (it happened tosaveWalletduring this work). The test fails on any such call that is not routed throughwindow.<name>, the patternwindow.hexToWIFalready uses.Not in this PR
Found while auditing
index.htmlbefore starting; each deserves its own branch:localStorageand IndexedDB in plaintext (saveWalletData()), and the at-rest encryption function is commented out — despite the README stating that stored data is encrypted.decrypt_*.js/find_jerome_*.jsscripts. Worth checking whether those addresses hold funds.submitDebugReport()POSTs tohttps://unicity-debug-report.dyndns.org:3487with no confirmation step.extractFromWalletDat()logs the master private key to the browser console.