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

mnemonicToSeed should throw on non-string input #77

Open
3 tasks
dcousens opened this issue Apr 27, 2018 · 4 comments
Open
3 tasks

mnemonicToSeed should throw on non-string input #77

dcousens opened this issue Apr 27, 2018 · 4 comments
Labels
Milestone

Comments

@dcousens
Copy link
Contributor

dcousens commented Apr 27, 2018

It should probably additionally throw on empty string? But maybe someone has a use case there...

Additionally:

  • enforce no duplicate whitespace
  • enforce non-zero length
  • enforce only printable characters
@junderw
Copy link
Member

junderw commented Apr 27, 2018

IMO NACK... this is not something the library should be throwing on.

The application should decide whether to throw or not on zero length / multiple whitespace etc.

@dcousens
Copy link
Contributor Author

dcousens commented Apr 27, 2018

@junderw then let us provide an "unsafe" option.

mnemonicToSeedUnsafe

The advanced user needs protection too.

@dcousens dcousens added feature and removed bug labels Apr 27, 2018
@dcousens dcousens added this to the 3.0.0 milestone Apr 27, 2018
@afk11
Copy link

afk11 commented Apr 27, 2018

concept ACK - it's so easy for whitespace to be introduced, and then the mnemonic input is directly hashed.. It can be pretty easy to miss the typo 'abandon abandon abandon' and 'abandon abandon abandon' when it's a large block.

The whitespace isn't covered by the checksum, and if we allow users to make this mistake, we can expect people to panic when the recover from a seed that was transcribed to paper..

Empty string is forbidden by BIP39 - the 'recommended sizes' are the only acceptable ones it turns out, so a zero length mnemonic is invalid :)

@dcousens
Copy link
Contributor Author

the 'recommended sizes' are the only acceptable ones it turns out, so a zero length mnemonic is invalid :)

AFAIK, validateMnemonic should make that distinction, not this function. This function should support non-strict BIP39 mnemonics, but I think the above error cases are common enough to hurt both types of user.

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

No branches or pull requests

4 participants
@dcousens @afk11 @junderw and others