Skip to content
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

Invalid BIP39 seed phrases allowed #6860

Open
jlopp opened this issue Dec 17, 2020 · 16 comments
Open

Invalid BIP39 seed phrases allowed #6860

jlopp opened this issue Dec 17, 2020 · 16 comments
Labels
mnemonic/seed 🌼 topic-wizard 🧙‍♂️ related to wallet creation/restore wizard

Comments

@jlopp
Copy link

jlopp commented Dec 17, 2020

I was helping a user recover funds recently and had them import their seed phrase into Electrum on an airgapped computer, but it wasn't generating the addresses we expected.

It took us quite a while to figure out that one of the seed words was wrong, which led me to wonder how the user ended up successfully creating a wallet with a clearly invalid checksum.

So I went through the wallet creation flow and lo and behold, it looks like Electrum will accept anything for BIP39 seed import.

Long story short, in addition to displaying a very easy to miss "checksum failed" message I suggest greying out the "Next" button like you do when an invalid electrum seed phrase is attempted to be imported.

electrum

@SomberNight
Copy link
Member

SomberNight commented Dec 17, 2020

Long story short, in addition to displaying a very easy to miss "checksum failed" message I suggest greying out the "Next" button like you do when an invalid electrum seed phrase is attempted to be imported.

We cannot do that because then people who were scammed into creating non-English wordlist BIP39 seeds could not restore.
The checksum being based on the wordlist and there being an unlimited number of wordlists is a serious inherent issue of BIP39.

@SomberNight SomberNight added mnemonic/seed 🌼 topic-wizard 🧙‍♂️ related to wallet creation/restore wizard labels Dec 17, 2020
@jlopp
Copy link
Author

jlopp commented Dec 17, 2020

Oops. Gotcha. IDK if it would be possible to make the checksum error more prominent or something. The person I was helping completely missed it.

@SomberNight
Copy link
Member

Right, it sounds reasonable to make the warning more prominent. We could maybe show a popup warning to click through or similar.

@ecdsa
Copy link
Member

ecdsa commented Dec 18, 2020

We could also make the checksum active by default (as in, disable next button), and add a "disable bip39 checksum" option.
I hope that non-English bip39 seeds are rare and discouraged.

@ecdsa
Copy link
Member

ecdsa commented Dec 31, 2020

We cannot do that because then people who were scammed into creating non-English wordlist BIP39 seeds could not restore.

On a second thought, I would not mind doing that. The people who were "scammed" into creating a non-English bip39 seed are not our users anyway, and we have been displaying a clear warning that bip39 seeds might not be supported in the future. Thus, we have no commitment to keep supporting non-English bip39 seeds. That would remove the need for an extra option.

@SomberNight
Copy link
Member

SomberNight commented Dec 31, 2020 via email

@ecdsa
Copy link
Member

ecdsa commented Dec 31, 2020

IDK if it would be possible to make the checksum error more prominent or something. The person I was helping completely missed it.

Could it be because the completions popup grabbed your attention? Everytime a new feature is added to a dialog, it competes with the existing widgets.

@Zhell1
Copy link

Zhell1 commented Dec 31, 2020

When you hear devs saying that a UX improvement for non english speakers is a "scam", you know the project is doomed. Shouldn't software development be about improving the user experience by fixing problems, instead of avoiding them and calling attempts a scam ? well, do whatever you want it's your project

@SomberNight
Copy link
Member

SomberNight commented Dec 31, 2020 via email

@Zhell1
Copy link

Zhell1 commented Dec 31, 2020

I understand your concern and that's the problem all "standards" face I guess

I also appreciate your efforts to keep supporting those "unconventional" seeds in Electrum, despite your concerns

As someone living in a non-english country, I can tell you that all the people who don't speak english are not comfortable with writing a seed in a language they do not understand. Writing a seed is new enough already for them, and having to convince them of how important that is. Imagine how you would feel if you had to write a seed in chinese, that's their experience.

So all the software devs I know are working on supporting seeds in the language of the country they are developing for, as far as I know, and this is the reason why these unconventional seeds are so used, and I have used them myself, both as a user and as a developer building projects for non-english users.

I guess there are no easy solution to this problem because it's between UX, safety, and avoiding complex code. But I would imagine that it would be possible - but probably a lot of work just for this - to use the first few words to find the language by parsing all languages lists, and then applying the correct checksum.

From my experience, getting Bitcoin adoption in non-english countries is particularly hard because of problems like this that have remained unsolved for years.

Once again let me say that I really appreciate your efforts to supporting these seeds, and understand that you want safety for users and also have limited ressources to spend and probably more important issues to solve.

@ecdsa
Copy link
Member

ecdsa commented Jan 2, 2021

@jlopp I added that check on kivy/android, because the virtual keyboard imposes English anyway. 07b0873

@SomberNight
Copy link
Member

Note that BIP39 itself specifies that "invalid checksum" seeds should not be outright rejected.

https://github.com/bitcoin/bips/blob/cf0b529e78860fa2d4fe77944091aa98c5e04624/bip-0039.mediawiki#From_mnemonic_to_seed

Although using a mnemonic not generated by the algorithm described in "Generating the mnemonic" section is possible, this is not advised and software must compute a checksum for the mnemonic sentence using a wordlist and issue a warning if it is invalid.

@ecdsa
Copy link
Member

ecdsa commented Jan 21, 2021

Note that BIP39 itself specifies that "invalid checksum" seeds should not be outright rejected.

oh well... it is impossible to please everyone I guess :-)

@Acyber2020
Copy link

I had a copay in bip39. I saved the seeds, but I cannot recover my btc. I am searching everywhere. Cannot find the solution to this. I formated the phone with the copay app, and now there is no way to recover my btc

@doge2021

This comment was marked as off-topic.

@doge2021

This comment was marked as off-topic.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
mnemonic/seed 🌼 topic-wizard 🧙‍♂️ related to wallet creation/restore wizard
Projects
None yet
Development

No branches or pull requests

6 participants