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

Validate passphrase when importing account #498

Merged
merged 6 commits into from Jan 8, 2018

Conversation

Projects
None yet
3 participants
@zillionn
Contributor

zillionn commented Jan 3, 2018

People all the time mistyped their passphrase when importing account and then panic when see an empty wallet. This PR should fix this:

  • after clicking IMPORT I'm checking if the passphrase is a valid BIP39 mnemonic, if not, toast an error message: "Not valid 12 words passphrase! Please check all words and spaces."
  • added a switch "custom passphrase", so you still can import with custom passphrase

zillionn added some commits Jan 3, 2018

@j-a-m-l j-a-m-l self-requested a review Jan 5, 2018

@j-a-m-l

Good work, but I'll suggest 1 change:

Instead of "custom passphrase" I'd use something different because users could have problems understanding that is "custom" in this context.

Maybe something like "Permit non-BIP39 passphrases" would be descriptive enough.

What do you think?

@zillionn

This comment has been minimized.

Show comment
Hide comment
@zillionn

zillionn Jan 6, 2018

Contributor

Whatever you guys decide, but imo 99.99% of the people have no idea what BIP39 is :)

Contributor

zillionn commented Jan 6, 2018

Whatever you guys decide, but imo 99.99% of the people have no idea what BIP39 is :)

@j-a-m-l

This comment has been minimized.

Show comment
Hide comment
@j-a-m-l

j-a-m-l Jan 6, 2018

Member

Yes, @zillionn a lot of users don't know what it is, but that text include something that is less ambiguous than "custom". In fact, they would have a way to learn about passphrases, which is something unexpected for a lot of people that are used to choosing their own passwords.

Other idea: "Use a custom (non-BIP39) passphrase", where "BIP39" links to https://github.com/bitcoin/bips/blob/master/bip-0039.mediawiki

Member

j-a-m-l commented Jan 6, 2018

Yes, @zillionn a lot of users don't know what it is, but that text include something that is less ambiguous than "custom". In fact, they would have a way to learn about passphrases, which is something unexpected for a lot of people that are used to choosing their own passwords.

Other idea: "Use a custom (non-BIP39) passphrase", where "BIP39" links to https://github.com/bitcoin/bips/blob/master/bip-0039.mediawiki

@zillionn

This comment has been minimized.

Show comment
Hide comment
@zillionn

zillionn Jan 6, 2018

Contributor

OK change it, cause I have no idea how to do it.

Contributor

zillionn commented Jan 6, 2018

OK change it, cause I have no idea how to do it.

@luciorubeens

This comment has been minimized.

Show comment
Hide comment
@luciorubeens

luciorubeens Jan 7, 2018

Member

True that, the switch is clickable, so we can't click in the 'BIP39'. Maybe add a help icon next to the switch

Member

luciorubeens commented Jan 7, 2018

True that, the switch is clickable, so we can't click in the 'BIP39'. Maybe add a help icon next to the switch

@zillionn

This comment has been minimized.

Show comment
Hide comment
@zillionn

zillionn Jan 8, 2018

Contributor

How about now?

Contributor

zillionn commented Jan 8, 2018

How about now?

@zillionn

This comment has been minimized.

Show comment
Hide comment
@zillionn

zillionn Jan 8, 2018

Contributor

Actually with an icon looks better.

Contributor

zillionn commented Jan 8, 2018

Actually with an icon looks better.

zillionn added some commits Jan 8, 2018

@luciorubeens luciorubeens merged commit f195512 into ArkEcosystem:master Jan 8, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details
@luciorubeens

This comment has been minimized.

Show comment
Hide comment
@luciorubeens

luciorubeens Jan 8, 2018

Member

Good job! 👍

Member

luciorubeens commented Jan 8, 2018

Good job! 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment