search

trezor / python-shamir-mnemonic

MIT License
165 stars 59 forks source link

Remove option to generate 1ofX for X > 1 #9

Closed slush0 closed 5 years ago

slush0 commented 5 years ago

Actually there's no reason to use 1ofX for groups with X > 1 and it is rather confusing that shares are almost the same, except indexes. I would rather to communicate by the script that if user want 1ofX for X>1 it is rather an user mistake or misunderstanding of things.

I was myself surprised at first moment and expected that's a bug of the library, when I was testing the tool. Thus this is UX improvement for idiots like me.

matejcik commented 5 years ago

i'm thinking, disallow this in the spec and raise an exception if a parameter like this is passed to the library?

slush0 commented 5 years ago

Actually it does not need to be prohibited by the spec (but it won't hurt also). This is more like UX stuff in the specific implementation. But I'm open to both ways.

slush0 commented 5 years ago

Okay, @prusnak is inclining to disabling 1ofX for X>1 in specs makes sense. So let's disallow it in spec...

prusnak commented 5 years ago

However, I think that 1-of-X GROUPS make sense. Let's disallow only 1-of-X shares (of a certain group).

matejcik commented 5 years ago

this is now fixed in code. reopening to track a possible change in spec

andrewkozlik commented 5 years ago

I added a note in the spec to discourage creating multiple member shares with threshold 1 and explained the rationale: https://github.com/satoshilabs/slips/commit/4b4d554318c219f518f63635249b83ea124224b1