-
-
Notifications
You must be signed in to change notification settings - Fork 13
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
Allow fixing invalid bip39 mnemonic #58
Conversation
909c444
to
fb908cf
Compare
fb908cf
to
86b8ab7
Compare
Can we find some short language other than "Press A to fix the checksum" for this PR? The goal here is to allow for seed creation by using dice to words manual techniques that include adding a last word, then based on the randomness of all 12 (or 24 words) such that includes the randomness of the last word, there should be exactly 1 checksum that is the same no matter the implementation. (if we did this with 11 or 23 words, we loose a few bits of randomness, and as there are multiple valid checksums possible making is less deterministic). In particular I'm uncomfortable with the word "fix". There nothing wrong with the last word if you use this technique, you are just choosing to let the app complete the checksum. Maybe "complete"? "mend"? I'm not sure. It is possible that there maybe is no other verb or phrase than "fix" Ideally all the implementations should use the same verb or phrase in all the implementations, i.e. spectre (@stepansnigirev), seedtool (@wolfmcnally), gordian (@Fonta1n3), proofwallet (@hodlwave), and maybe someday even the BIP39 javascript version (@iancoleman) so that someone can confirm no "up my sleeves" tricks with this last word on all the implementations. |
@gorazdko before we add our final github tag for this feature release, we need to add some more documentation, and maybe a PDF worksheet so that someone can use this technique if they choose to. Not required to approve this PR. |
@wolfmcnally Can you install this PR on your LetheKit and confirm that the results are correct with seedtool? We'll also need to add this feature to seedtool. While you are at it, review the install notes. |
BTW, for reference what we need to add to LetheKit docs is some form of docs and worksheet, like what @bjdweck has done.
Though this idea has independently come from a variety of sources, If we use some of his or others work, we need to be sure to offer some attribution. |
@bitcoinheiro, I'd also appreciate your thoughts on this, as you suggested it to Specter in cryptoadvance/specter-diy#79 |
/cc @merland author of https://github.com/merland/seedpicker |
I am concerned that this defeats the purpose of the checksum. |
@gorazdko and I have had some discussion about this, and think that probably this technique should probably not be supported in standard recovery—only as a separate named function (maybe “Show Your Work” or something) with a warning screen “only use this with real dice and a worksheet, don’t try to use it as a brain wallet”. |
ColdCard is now supporting a version of this technique. We really ought to standardize better, come up with a common name for us to use, and share best practices. |
My biased opinions in support of my seedgen method:
|
Thanks for the cc. My two cents: |
Abstract
Allow fixing invalid bip39 mnemonic
Close #55 : matches implementation of cryptoadvance/specter-diy#85
In addition, error messages in the install script are made more noticeable now. This was reported by @wolfmcnally
Status
Ready