-
Notifications
You must be signed in to change notification settings - Fork 252
Export wallets with BIP 389 multipath descriptors for both receive and change branches #2534
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for specter-desktop-docs canceled.
|
…ge branches Co-authored-by: k9ert <[email protected]>
…ptors Co-authored-by: k9ert <[email protected]>
Co-authored-by: k9ert <[email protected]>
Co-authored-by: k9ert <[email protected]>
Co-authored-by: k9ert <[email protected]>
Co-authored-by: k9ert <[email protected]>
al-munazzim
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good fix for including both receive and change branches in exports. A few considerations:
-
BIP 389 compatibility: Verify that target wallets (SeedSigner, Krux, etc.) actually support the
<0;1>/*multipath syntax. Some older firmware versions might not parse it correctly. -
Import regex concern: The conversion
<0;1>/*→0/*in wallet_importer.py:
# What if there are multiple multipath parts in a complex descriptor?
# e.g., multi(2, ../<0;1>/*, .../<0;1>/*)The current regex might not handle all cases. Worth adding a test for multisig descriptors with multipath.
-
Backward compatibility: Good that imports still work - old exports with
/0/*will continue to import fine. -
Checksum: Does changing from
/0/*to/<0;1>/*affect the descriptor checksum? The checksum should be recalculated.
Overall: 👍 Good improvement. Might want to verify hardware wallet compatibility before merging.
Wallet exports (PDF, JSON, QR codes) were only including the receive descriptor
/0/*, omitting change addresses. Hardware wallets importing these backups could not derive change addresses.Changes
Export with combined descriptor (
wallet.py)account_mapproperty to usedescriptor.to_string()which returns BIP 389 multipath syntax<0;1>/*instead of justrecv_descriptor(/0/*)Backward compatible import (
wallet_importer.py)<0;1>/*→0/*during import to satisfy existing validation logicTest coverage (
test_wallet.py)account_mapexports combined descriptor with 2 branchesExample
Before:
{"descriptor": "wpkh([fp/84h/0h/0h]xpub.../0/*)#checksum"}After:
{"descriptor": "wpkh([fp/84h/0h/0h]xpub.../<0;1>/*)#checksum"}The combined descriptor expands to both receive (
/0/*) and change (/1/*) branches when parsed by BIP 389-compliant wallets.Original prompt
💬 We'd love your input! Share your thoughts on Copilot coding agent in our 2 minute survey.