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

Introduce type MnemonicSize with accompanying tests. #498

Merged
merged 1 commit into from Jul 3, 2019

Conversation

jonathanknowles
Copy link
Member

@jonathanknowles jonathanknowles commented Jul 3, 2019

Issue Number

#357

Overview

This PR:

  • adds a MnemonicSize type, which represents the set of valid mnemonic sentence lengths.
  • adds ToText and FromText instances, with accompanying roundtrip tests.
  • uses this type for the mnemonic generate --size option, instead of String.

This:

  • restores the clean separation between parsing and execution.
  • allows unit testing of the parsing code.
  • produces a more standard error when the parsing fails:
$ cardano-wallet mnemonic generate --size 22
option --size: Invalid mnemonic size. Expected one of: 9, 12, 15, 18, 21, 24.

Usage: cardano-wallet mnemonic generate [--size INT]
  Generate English BIP-0039 compatible mnemonic words.

Comments

@jonathanknowles jonathanknowles requested a review from rvl July 3, 2019 04:23
@jonathanknowles jonathanknowles self-assigned this Jul 3, 2019
Copy link
Contributor

@rvl rvl left a comment

Choose a reason for hiding this comment

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

I think the way it was before is fine. But this is also fine. MnemonicSize looks a bit like a type level list of Nats ... not sure if we want to go there ...

@jonathanknowles jonathanknowles force-pushed the jonathanknowles/mnemonic-size-type branch from 98f2e76 to 1d5cfa4 Compare July 3, 2019 06:11
@jonathanknowles jonathanknowles merged commit d86a0f1 into master Jul 3, 2019
@KtorZ KtorZ deleted the jonathanknowles/mnemonic-size-type branch July 4, 2019 16:15
@KtorZ KtorZ added this to the Bugs & Debts - Sprint 27-28 milestone Jul 24, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants