Method for bip39 mnemonic validation

[flexsurfer]

Problem

status-im/status-mobile#9050

We want to recover only valid mnemonics (words from the dictionary and checksum is valid)

Implementation

validateMnemonic(mnemonicString)

words from the dictionary and checksum are valid

[3esmit]

I see that we need to open another issue to request the addition of method of bip39 mnemonic word suggestion (as nano ledger does). I will track progress on this one and open it if does not get solved. Just mentioning because this is a planned feature.

[rachelhamlin]

Does anyone own this issue @flexsurfer?

[flexsurfer]

@rachelhamlin don't think so :(

[rachelhamlin]

Okay - thanks :) I added to core project board.

FYI @cammellos @pombeirp @adambabik this is a new status-go v1 requirement.

[acolytec3]

Is this something I could work on? Seems like this would just adding in some new validations for RecoverAccount so maybe a "good first issue"?

[rachelhamlin]

Keen to learn go @acolytec3? Let's try it - I've invited you to a bounty, and I think @adambabik can offer advice if you need help getting started on this one.

[flexsurfer]

@gravityblast could assist as well, so @acolytec3 what we need here is a separate method we can call from status-react, which will validate mnemonic, checks if all words from our dictionary and checksum is valid

[karansinghgit]

@flexsurfer Hey, are there any similar beginner level issues I could help out with?

[flexsurfer]

@karansinghgit hey, go or clojure?

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

Work has been started.

These users each claimed they can complete the work by 6 days, 9 hours from now.
Please review their action plans below:

1) acolytec3 has been approved to start work.

I would like to work on this bounty per our discussion on the issue on Github.

Learn more on the Gitcoin Issue Details page.

[acolytec3]

@rachelhamlin Yes! I've tinkered in Go before though nothing too serious and would love to get my feet wet in the backend of the project.

[rachelhamlin]

Awesome - have at it, you're approved @acolytec3!

[guylouis]

cc @gravityblast so that he does not do the work too :)

[gravityblast]

a good starting point would be taking a look at the MnemonicPhrase method of the Mnemonic struct of the extkeys package.
That method generates the mnemonic phrase from the entropy and its checksum, so basically what we need is the validation doing the same but from phrase to entropy+checksum.

[acolytec3]

Thanks! I'll try and dig into it over the weekend.

…

On Fri, Nov 22, 2019 at 10:18 AM Andrea Franz ***@***.***> wrote: a good starting point would be taking a look at the MnemonicPhrase method of the Mnemonic struct of the extkeys package. That method generates the mnemonic phrase from the entropy and its checksum, so basically what we need is the validation doing the same but from phrase to entropy+checksum. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1663?email_source=notifications&email_token=AEENFXCMGYV2H7VSJ3U66JDQU7Z4FA5CNFSM4JJYZNRKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEE56CPY#issuecomment-557572415>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AEENFXGXFLYJGNWD43IT4M3QU7Z4FANCNFSM4JJYZNRA> .

[karansinghgit]

@flexsurfer Go :)

[gitcoinbot]

@acolytec3 Hello from Gitcoin Core - are you still working on this issue? Please submit a WIP PR or comment back within the next 3 days or you will be removed from this ticket and it will be returned to an ‘Open’ status. Please let us know if you have questions!

• reminder (3 days)
• escalation to mods (6 days)

Funders only: Snooze warnings for 1 day | 3 days | 5 days | 10 days | 100 days

[acolytec3]

@gitcoinbot Yes, thanks for bugging me.

[acolytec3]

@gravityblast To make sure I'm following, validate that the seed phrase matches the status dictionary (like validateMnemonic and then back out the checksum from the and make sure it matches the first 12 bits of the SHA256 hash of the entropy, right? Does it make sense to just add that checksum validation to the validateMnemonic function in mnemonic.go and expose that to status-react?

[gravityblast]

sorry @acolytec3 , yeah it makes sense!

[andremedeiros]

@acolytec3 I think the path forward is:

• Validation needs to check checksum and fail if it's not valid

You'll have to augment ValidMnemonic to include that check. There is a Golang bip39 implementation that has a checksum check you can borrow code / ideas from.

• Validation needs to return specific errors (instead of the presence of one)

To that end, I think we need to introduce a ValidateMnemonic counterpart to the above function that returns a specific error when it encounters it.

Finally,

• To preserve API compatibility, we need to keep ValidMnemonic

After implementing the above, it should look like:

func (m *Mnemonic) ValidMnemonic(mnemonic string, language Language) bool {
    return m.ValidateMnemonic(mnemonic, language) != nil
}

func (m *Mnemonic) ValidateMnemonic(mnemonic string, language Language) error {
    // Validations go here
}

Does this make sense?

[acolytec3]

@andremedeiros I think so. I've started looking at the Go bip39 implementation you referenced so will try and get a PR started in the next few days.

[guylouis]

Hi, any news on that ?

[acolytec3]

@guylouis I believe my PR is ready for review but need input on whether to leave the current additional test in place. My instinct says remove the validation step I added to the part of the test that looks at the precomputed seed vectors since it looks like at least the Japanese ones use words not in the Japanese word dictionary.

I believe the code that validates the checksum is sound and ready for merging on its own.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

Work for 1.267 ETH (187.33 USD @ $147.85/ETH) has been submitted by:

1. @acolytec3

@StatusSceptre please take a look at the submitted work:

• PR by @acolytec3

---

• Learn more on the Gitcoin Issue Details page
• Want to chip in? Add your own contribution here.
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $100,544.23 more funded OSS Work available on the Gitcoin Issue Explorer

[guylouis]

@flexsurfer @gravityblast
need your insight on who should review this, thanks !

[andremedeiros]

@guylouis we're actively reviewing and it's just taking a little bit of polish to get merged.

[gitcoinbot]

Issue Status: 1. Open 2. Started 3. Submitted 4. Done

---

The funding of 1.267 ETH (179.15 USD @ $141.39/ETH) attached to this issue has been approved & issued to @acolytec3.

• Learn more on the Gitcoin Issue Details page
• Questions? Checkout Gitcoin Help or the Gitcoin Slack
• $75,438.15 more funded OSS Work available on the Gitcoin Issue Explorer