Match updated wallet spec

jtobin commented 5 years ago

This is a bigger PR than I'd normally want to make, but there were a lot of changes to make, and I want to get them all in before this week is out. It thus wound up being easier to just go into bull-in-china-shop mode than be particularly surgical here. So, this covers a bit of ground. From the CHANGELOG:

Fix all seeds at 256 bits.
Always derive BIP32 nodes according to the path m/44'/60'/0'/0/0.
Remove the optional 'password' salt.
Render and handle (non-network) seeds as BIP39 mnemonics.
Displays corresponding Ethereum addresses in derived BIP32 nodes.
Replaces XOR-based 2-3 sharding with concat-based 2-3 sharding. Shards are also now reported in the 'shards' property of the final wallet.
Add an 'ownership' seed, plus slight changes to other seed type names.
Miscellaneous housekeeping and simplification throughout.

I believe this resolves #32, #27, #26, #25, and #22, and takes us to the updated wallet spec and also addresses various concerns from the auditors' reports.

The housekeeping/simplification axed a few hundred lines of code in src/index.js, and the test suite slimmed down as a result. I also made some improvements there -- added some property tests, improved whole-wallet unit tests, etc.

For reviewing: I would ignore the diff and just read/skim the new src/index.js from top to bottom, perhaps with reference to the updated spec (which it maps to very clearly). It's not very long, and should be pretty clear throughout.

g-a-v-i-n commented 5 years ago

There are issues with reserved word usage here – public and private, I will push fixes.

g-a-v-i-n commented 5 years ago

100% test coverage!

g-a-v-i-n commented 5 years ago

This is actually beautiful work

Fang- commented 5 years ago

(I'm not joking about the semicolons btw, I would like to see those back in. Of course, I am a mere mortal, making demands in front of a god...)

jtobin commented 5 years ago

@Fang- Will make all changes you requested. Except, of course, for the semicolons. 😁

(If I were to adopt a particular style guide it would probably be this one, just because it appears to be.. uh, standard (boy they really nailed it with the name choice), and which just happens to require no semicolons.)

Fang- commented 5 years ago

Except, of course, for the semicolons. 😁

(This was before your time I think, but fyi: ) https://github.com/urbit/keygen-js/pull/2#issuecomment-416728380

jtobin commented 5 years ago

Thanks for the reviews gents!

flowerornament commented 5 years ago

I put in an extra review for good measure – beautiful work

urbit / urbit-key-generation

Match updated wallet spec #34