trezor / python-shamir-mnemonic

MIT License
165 stars 59 forks source link

improve API #27

Closed matejcik closed 4 years ago

matejcik commented 4 years ago

This is my second attempt at API rework, this time with a big refactor.

Summary:

Details:

Constants now live in a separate file constants.py.

rs1024-polymod has its own file rs1024.py.

The cipher has its own file cipher.py. encrypt and decrypt are exported into the main shamir_mnemonic module.

Loading wordlist and converting mnemonics to/from indices was moved to wordlist.py.

Converting individual shares to/from mnemonic representation was moved to share.py, and share info is now handled in a Share class, similar to what trezor-core does.

The meat-and-potatoes of the functionality was moved to shamir.py. The main functions combine_mnemonics, generate_mnemonics, split_ems and recover_ems are exported to the top-level module. New functions:

combine_mnemonics was reformulated to use recover_ems internally, and generate_mnemonics was reformulated to use split_ems internally.

generate_mnemonics_random was removed. Its function is rather specific, and in most usecases the caller will need to have the Master Secret (e.g. to save it in storage) as well as the set of shares.

matejcik commented 4 years ago

I fixed some comments, regularized the Share API, and added checks to encrypt/decrypt.

The rest is pending further discussion.

I also regenerated the vectors, because the default iteration exponent was changed to 1 to match trezor-core. Please check if this is what we want and if the vectors are OK. We can also:

andrewkozlik commented 4 years ago

This is so weird, I thought I left a comment here about the test vectors last week, but now I can't find it anywhere. Anyway, in generate_vectors.py use iteration exponent 0 explicitly. There is no reason to change the test vectors if the specification didn't change.

matejcik commented 4 years ago

generate_vectors were modified, so that the original vectors are produced with the new code.

documentation for split_ems and recover_ems was changed, with details of the "deferred" mode to be described in trezor-core

all previous comments should be addressed now