-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
Add BIP85 support #454
Add BIP85 support #454
Conversation
Thanks for this. It's a very clean implementation. This seems useful to include this with the tool. I would like to throw around a few conceptual aspects to make sure I understand the use cases for this feature correctly (I have read bip85 several times but never felt I fully understood the motivation; it seems to me like it allows mnemonics to be derived from other mnemonics, ie bip85 is HD mnemonics which feed into HD keys, does that sound right?). Does the information within the bip85 section only affect bip85 fields? (I guess from looking at the PR that yes, bip85 info is populated from the Mnemonic section but is not used for any other section of the tool). Add explainer text The bip85 section could potentially be confusing when shown, as it may seem that the bip85 child key is being used as the basis for the Derivation Path and Derived Addresses section. The current flow of the tool is top-to-bottom so it could seem like the bip85 section is stepping into the flow of information by being positioned where it is. So maybe a small bit of text to explain the bip85 info being shown is a) not used by the tool for derivation and/or b) must be copied and pasted elsewhere to be used for deriving addresses. A link to bip85 in that explainer text would be great - "For more info see the BIPXX spec" is used throughout the tool. Additionally, it might be clearer for users if the bip85 info shows in a modal so it's clearly 'isolated' from the existing flow (I really dislike modals so am just brainstorming). I thought maybe left indentation might achieve this visual separation, but indenting might not be a clear enough visual language (especially on mobile). I'm still not sure about the UX for the bip85 feature, I'm open to suggestions (or reasons to not worry about it). So far I'm not terribly worried about it but wonder if we can make the use-case for that info clearer somehow? Tests I looked at the bip85 test vectors but couldn't see how to confirm them using this tool. Any tips? Also a test should be added to the Out of curiosity, how does a user specify bip85 usage to another person? For example, I can fully explain how to get private keys and addresses via "BIP39 abandon abandon ability (no passphrase), BIP44 account 0" (anyone should be able to derive 1Di3Vp7tBWtyQaDABLAjfWtF6V7hYKJtug from that) How would bip85 fit into an explained derivation sequence like that? |
Thanks for the detailed feedback. I'll try to address all your points. BIP85 Use Cases
This is one of the use cases, yes. For me personally, this is the most interesting use case, but BIP85 also specifies different "applications" which derive entropy for different purposes. I think this allows you to derive entropy for things like bitcoin-core (which uses the WIF format), or derive entropy for a monero mnemonic. For me, the "real life" use case for BIP85 is the following:
I would advise that people keep a private list of a mapping between index and application where the mnemonic is used, just to make the recovery process easier. So in summary, the big advantage in my opinion is that you can quickly get new, secure mnemonics, without worrying too much about how they are backed up. I personally run into this problem quite a lot. You want to test out a new wallet and need a new mnemonic for that, so I use your bip39 tool to quickly get one. But then I have to back it up, which takes a considerable amount of effort if you want to do it in a secure and redundant way. BIP85 takes that hassle away without compromising security.
Yes. The BIP85 fields only affect other BIP85 fields. The "BIP85 Child Key" is what is calculated out of the original mnemonic and the configuration provided in the BIP85 section. But the "BIP85 Child Key" is not used anywhere, it has to be copied by the user and pasted into the "BIP39 Mnemonic" field at the top in order to see the addresses etc. that the mnemonic generates. Add explainer text
Yes, I was also concerned about that. At first, I thought about putting it into a new tab in the derivation path section, but those are all linked to addresses, so it didn't make sense. If you're ok with an explainer text, then I can do that. It would be easy to do and probably clear up most of the confusion. Maybe we could also consider adding a tabbed section below the "BIP32 Root Key" field, which will contain "BIP85" and the "split mnemonic cards", because they are not directly related to the top-to-bottom flow, but more like additional data that can be generated out of the mnemonic? That table could be hidden by default and only enabled if you tick the "advanced options" box. I think I would prefer that to a modal. TestsI will add some tests.
You can use the mnemonic
I'm not quite sure what you are asking for here, but I'll explain how you get from a "master mnemonic" to one of the "child mnemonics": You take a mnemonic and calculate the BIP32 Root Key:
You then derive a new key from the BIP32 Root Key, using a path according to the BIP85 specs.
You then take the private key of the child node and calculate the hash of it.
That will give you entropy in the length of 64 bytes. It has to be truncated to the length of the new mnemonic:
|
I added one test and an explanation. Is that enough? Also, one of the tests seems to fail for me, even on master. So I didn't touch it:
|
Looks good. I'm happy to merge this. Sorry for the slow response, I've been mulling over the ux for this. One question though, just because I want to make sure I fully understand this before I merge it. bip85 might fit into the workflow logically by adding a radiobutton in the bip85 section for deciding which mnemonic to use as the root key feeding into the derivation section:
So the flow from top to bottom of the tool would be Entropy (optional) -> Bip39 mnemonic + root seed / hdkeys -> Bip85 mnemonic (optional) -> Bip[32|44|49|84|141] derivation path -> Addresses + keys Do the radiobuttons sound like an appropriate addition? |
Hmm, that's a difficult question. In theory I can see how the flow you describe makes sense. But I'm not sure if it wouldn't be clearer to keep it the way it is and have users copy/paste the mnemonic. Because this way, it is clear that it's a new mnemonic, not something that is an "intermediate" step in the flow. If we look at implementations in wallets (there is currently only Coldcard, AirGap is working on it), then the derivation of the child mnemonic is its own flow. So in those wallets, there is a clear separation between the derivation of the child mnemonic, and then deriving the address from that child mnemonic. So after thinking about it for the last 20 minutes, I think I would prefer to keep it this way because it would be more in line with the actual use cases (meaning to separate the derivation of child mnemonic and addresses). What do you think? |
I just noticed this: Should I add a "BIP85" section under "More info"? |
Yes I'm inclined to agree. Let's leave it.
I admit that my first feeling was maybe the bip85 feature should be a different tool altogether and not part of this one, since it's a reasonably different flow to what this tool currently does. But I can see the benefit of it being here, and I feel like we've explored the ux enough that it can be merged satisfactorily. |
Thanks for merging!
I know what you mean. For me personally I think it fits into the general use case of this tool, but if you ever change your mind I'll be happy to create a new tool for BIP85 specifically. |
FYI:
PS: Thanks for your including this in your tool! Excellent addition! |
@8go: AirGap Vault now also supports BIP-85: airgap-it/airgap-vault@d64332f |
Adds support for BIP85 - Deterministic Entropy From BIP32 Keychains.
All applications mentioned in the specs are supported (BIP39, WIF, XPRV and HEX).
This feature was requested here: #424