Skip to content
This repository has been archived by the owner on Nov 21, 2019. It is now read-only.

Mnemonic-based wallet generation is strange in dist #566

Closed
leafcutterant opened this issue Jun 10, 2017 · 5 comments
Closed

Mnemonic-based wallet generation is strange in dist #566

leafcutterant opened this issue Jun 10, 2017 · 5 comments

Comments

@leafcutterant
Copy link

In the dist release, when trying to add a wallet based on the mnemonic (ran in Firefox), the textbox becomes green and a button appears only when 3- or 6-word phrases are entered which have BIP39 words.

Why is this? Could you change it to accepting any kind of mnemonics?

@leafcutterant leafcutterant changed the title Menmonic-based wallet generation is strange in dist Mnemonic-based wallet generation is strange in dist Jun 10, 2017
@tayvano
Copy link
Contributor

tayvano commented Jun 11, 2017

@kvhnuke Can you comment on why this is this way and what we should keep in mind while building the module for v4 so that this isn't an issue, but also doesn't create further issues?

@mikeyb
Copy link

mikeyb commented Jun 11, 2017

@leafcutterant what generated the mnemonic you are attempting to use and why doesn't it use bip39? Wondering what is doing this and their reason for not using bip39 words?

I don't see issues with using non bip39 words per se, but you will run into portability issues with wallets adhering strictly to bip39 (which seems like a reasonable proposal to stick to)

@leafcutterant
Copy link
Author

@mikeyb what generated a mnemonic is the user's own business and it should remain that way, for reasons of privacy, respect and security. It can be diceware (e.g. for reasons of true physical randomness); a non-BIP39-compliant software; the old version of a BIP39-compliant software from times before BIP39 was even created; someone's own phrases; a seed in a language which is not yet supported by BIP39; etc.

My main point is that not everyone in the industry uses BIP39 and not everyone by far is a fan of the BIP39 specification. For one example, see Electrum's case: http://docs.electrum.org/en/latest/seedphrase.html#motivation
For an example when BIP39's implementation caused loss of money, see this: iancoleman/bip39#58

I'm fine with encouraging standards, but not with excluding users who don't conform to a certain standard. Portability means exactly the opposite of enforcing BIP39—it means that those who work with non-BIP39 mnemonics can also easily create a wallet in MyEtherWallet and they won't have to generate, back up and optionally memorize a new mnemonic, thus making their funds harder to recover (thus easier to lose) due to fragmentation.

@mikeyb
Copy link

mikeyb commented Jun 12, 2017

@mikeyb what generated a mnemonic is the user's own business and it should remain that way, for reasons of privacy, respect and security. It can be diceware (e.g. for reasons of true physical randomness); a non-BIP39-compliant software; the old version of a BIP39-compliant software from times before BIP39 was even created; someone's own phrases; a seed in a language which is not yet supported by BIP39; etc.

I only asked so I could go look at specific cases and understand their reasoning, assuming you had one.

My main point is that not everyone in the industry uses BIP39 and not everyone by far is a fan of the BIP39 specification. For one example, see Electrum's case: http://docs.electrum.org/en/latest/seedphrase.html#motivation
For an example when BIP39's implementation caused loss of money, see this: iancoleman/bip39#58

I'm fine with encouraging standards, but not with excluding users who don't conform to a certain standard. Portability means exactly the opposite of enforcing BIP39—it means that those who work with non-BIP39 mnemonics can also easily create a wallet in MyEtherWallet and they won't have to generate, back up and optionally memorize a new mnemonic, thus making their funds harder to recover (thus easier to lose) due to fragmentation.

Fair enough. I am mostly trying to understand why or why not I would implement bip39 specifically in a wallet. I have plans to build and open source my own wallet management tools specific to my needs and trying to come up with some best practices.

Thanks for the info. I will look into the electrum seeding mechanism.

@tayvano
Copy link
Contributor

tayvano commented Jun 18, 2017

@leafcutterant

We have so many new users that frankly we need to increase the validation on all these fields, not decrease it. While I appreciate that you are an advanced user and MEW used be more targeted towards these users, people are jumping in headfirst and losing ETH left and right.

I would recommend that you fork MEW yourself and run it locally with whatever validation, or lackthereof, that you want. I believe if you simply remove this line, you should be able to enter whatever your heart desires:

https://github.com/kvhnuke/etherwallet/blob/fe60485cda0b8c0554f7cee0d0ab1d0e969b583e/app/scripts/directives/walletDecryptDrtv.html#L98

Whenever we make updates, you should be able to pull them without issues (unless we happen to change that one line) ensuring you are still working on the latest version.

I understand that this probably isn't the answer you are looking for, but the struggle between serving advanced users who understand how this works / have custom-crafted security / keys and new users forces us to make decisions like this.

Also it is worth noting that the bug you pointed to did not cause lost funds. In order to access your funds sent to an address generated there, you would need to simply use a site also using that library. One benefit of BIP39 is that the issues lie with the derivation of address sets and the original seed, not between the actual PK and addresses. This means that a "wrong" address isn't so much wrong as it is different than other sites. It still has a valid private / public key set that can be used to send funds.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants