trezor / python-shamir-mnemonic

MIT License
165 stars 59 forks source link

Support extendable backup flag #46

Closed andrewkozlik closed 1 month ago

andrewkozlik commented 1 month ago

Related to spec update https://github.com/satoshilabs/slips/pull/1724.

Note that we need to get this merged and released in order for the firmware device tests to pass on the corresponding firmware branch https://github.com/trezor/trezor-firmware/compare/andrewkozlik/slip39.

andrewkozlik commented 1 month ago

https://github.com/trezor/python-shamir-mnemonic/pull/46/commits/a1cb726876dfafdedf8e27d3460dbf57632e7c91 Updated a comment.

The customization string shamir_extendable is not used in the PBKDF2 salt. We wanted to completely decouple the decryption method from the sharing method and the share representation, i.e. not use the customization string from the checksums in decryption, and I believe avoiding the mention of shamir in the PBKDF2 salt is right, because it allows us to cleanly update backups even to different secret sharing schemes in the future if we want.

andrewkozlik commented 1 month ago

https://github.com/trezor/python-shamir-mnemonic/commit/706934d84a7eccff7854133c2fb681ef90658a9e breaks generate_vectors.py, line 66:

mnemonic = wordlist.mnemonic_from_indices(
    indices + rs1024.create_checksum(indices, False)
)
andrewkozlik commented 1 month ago

Fixed in https://github.com/trezor/python-shamir-mnemonic/pull/46/commits/38964c02f95320dd19a7a18d724ccd9253f0ec5b. LGTM now. @matejcik please recheck and then we can squash and merge.

matejcik commented 1 month ago

LGTM, go ahead