Add API to Account for mnemonic seed phrases

[fubuloubu]

Fixes: #8 #24
Enables: #25 #37

How was it fixed?

Looked at the entire history of attempts at implementing this feature (#33 #34 etc...). Referencing those attempts, I lifted https://github.com/trezor/python-mnemonic (so as not to add a dependency) and implemented BIP32 (eth_account/hdaccount/deterministic.py) in a more readable fashion so it is specifically helpful to our scenario, and can be reused (currently only support private parent -> private child, but can be adopted to support public parent -> public child for #25 & #37)

Cute Animal Picture

[pipermerriam]

A while back @davesque wrote https://github.com/davesque/py-hdwallet

If you're game I would be interested in removing the HD wallet implementation from this and leveraging the linked implementation (as a package dependency). Thoughts? I like keeping the crypto out of this library when possible so that we can have stable base libraries that implement the crypto which we rarely touch and then just use those public APIs to implement the high level functionality.

[fubuloubu]

I think it makes sense that this should be a different library, to remove critical code from being built in here. I aimed for my hdwallet implementation to be as clear and concise as possible, and have no interdependencies on other parts of eth-account for that reason.

If py-hdwallet will be easier for you to maintain as a separate library and be able to fill the multiple roles required for the class (such as hardware wallets), then perhaps this does make sense, and I would be okay in working with you to integrate py-hdwallet. I do have to say though that I have sank a bit of time into this PR and I very much would like to see it through to production unlike the other attempts at adding this feature because I am needing it for a project of my own.

I know quite a few people who would like to use this, and I think it's time it gets done.

[pipermerriam]

👍 to moving forward with the local implementation and switching over to the py-hdwallet at a later time so as to not hold this up

[fubuloubu]

@pipermerriam as far as testing goes, I have the official BIP32 and BIP39 test vectors of course, and I also lifted MetaMask's and Ganache's test cases from their repos (seed -> address). I've tested it with my own seed phrase and it works well. Was thinking the last steps would be perhaps to generate accounts from a couple popular wallets and put them in there too (tests/core/test_hdaccount.py)

[pipermerriam]

Looking at the size of the diff I think this needs to get divided up a bit into smaller pieces. Can you see about carving this up into some smaller pieces?

[fubuloubu]

@pipermerriam I can split this into 3 such pieces (but no more):

1. BIP32 HDPath support (+217 lines, 1 test module)
2. BIP39 Mnemonic support (+161 lines, 1 test module)
3. API Change to Account (+40 lines, 1 test module)

---

Note that the diff is large because of the 2048 line english word file for BIP39

[pipermerriam]

That would be great. Sent from ProtonMail mobile

…

-------- Original Message --------

On Feb 3, 2020, 10:17 AM, Bryant Eisenbach wrote: ***@***.***(https://github.com/pipermerriam) I can split this into 3 such pieces (but no more): - BIP32 HDPath support (+217 lines, 1 test module) - BIP39 Mnemonic support (+161 lines, 1 test module) - API Change to Account (+40 lines, 1 test module) — You are receiving this because you were mentioned. Reply to this email directly, [view it on GitHub](#87?email_source=notifications&email_token=AAGJHAQ37MAT6KJIVOP5WEDRBBGRHA5CNFSM4KOUF2G2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOEKUU4AY#issuecomment-581520899), or [unsubscribe](https://github.com/notifications/unsubscribe-auth/AAGJHASLIN7XAX2GCVK3WCTRBBGRHANCNFSM4KOUF2GQ).

[fubuloubu]

@pipermerriam This branch is now based off the work in #89 and #88, please feel free to review those first

[fubuloubu]

@kclowes I narrowed it down to 22 commits, which is about as far as I could shrink it

[kclowes]

Thanks @fubuloubu! 🎉 🎉

[lil0uuu]

Thanks for the feature! I see in the docs that create_with_mnemonic() should be used with caution: "This feature is experimental, unaudited, and likely to change soon".

What are the risks in using this function? It's already 2y old - is there any plan to audit/change it soon?

[fubuloubu]

Thanks for the feature! I see in the docs that create_with_mnemonic() should be used with caution: "This feature is experimental, unaudited, and likely to change soon".

What are the risks in using this function? It's already 2y old - is there any plan to audit/change it soon?

The feature is unaudited, but for the most common use case of obtaining private key material from an existing seed phrase it has so far worked pretty flawlessly. Let us know if you ever discover an issue with that use case.

As far as seed phrase generation, there may be risks that the master seed generation is handled inappropriately. There have been some examples where this has been problematic with other similar libraries, but again to the best of my knowledge there is no issue, and let us know if you find any.