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

Long Passphrase bug fix - Closes #986 #988

Merged
merged 2 commits into from
Nov 20, 2017

Conversation

joerodrig
Copy link
Contributor

@joerodrig joerodrig commented Nov 17, 2017

What was the problem?

Entering a 13-word passphrase would throw an error in the console, preventing further checks.

How did I fix it?

I set up a try/catch statement where the error was being thrown. If an error is caught, the isValidPassphrase method returns false. This assumes that if an error is thrown from the mnemonic library, the passphrase is invalid.

The error is thrown from a 3rd party lib, so there isn't much that can be done to resolve the issue there.

QUESTION: on line 92, there's a check in place to ensure that the passphrase is greater than or equal to 12 words, based off of spaces. If that check was to verify whether the passphrase contains EXACTLY 12 spaces, this issue could also be resolved. Is there a case where the passphrase can be valid with more than 12 spaces? I was unable to find anything to confirm that.

How to test it?

  • Open nano
  • Enter 12-word passphrase, e.g.awkward service glimpse punch genre calm grow life bullet boil match like
  • type one more valid word, e.g. life
  • You should see the error "Passphrase is not valid"

Review checklist

@joerodrig joerodrig changed the title Passphrase bugfix #986 Long Passphrase Bugfix Nov 17, 2017
@reyraa reyraa changed the title #986 Long Passphrase Bugfix Long Passphrase bug fix - Closes #986 Nov 20, 2017
@slaweet slaweet self-assigned this Nov 20, 2017
@slaweet slaweet self-requested a review November 20, 2017 09:05
Copy link
Contributor

@slaweet slaweet left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good job @joerodrig

@slaweet slaweet merged commit acc7e1e into LiskArchive:development Nov 20, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants