fixed a bug when generating shares with group_threshold=1

[howech]

The _split_secret function only returns a single share when you pass in a group_threshold of 1. This change makes spreads that single share around to all of the groups in the case that the group_threshold is 1, but there is more than one group.

[matejcik]

ISTM the fix should be in the _split_secret function? at the threshold == 1 check, you should return [(i, shared_secret) for i in range(share_count)]?

also the test case is a copy-paste of the other group share test. that's not great, it is unclear what it is that you're testing.

instead you should make a test case like this:

@pytest.mark.parametrize("threshold", (1, 2, 5))
def test_all_groups_exist(threshold):
    mnemonics = shamir.generate_mnenonics(threshold, [(2, 5)] * 5, MS)
    assert len(mnemonics) == 5
    # plus assert that 'threshold' groups combined is enough or something

[andrewkozlik]

Good work finding this bug @howech! I think that the bug is actually in the specification https://github.com/satoshilabs/slips/blob/master/slip-0039.md#splitsecrett-n-s, where step 2 states that If T is 1, then let y1 = S and return. It should say If T is 1, then let yi = S for all i, 1 ≤ i ≤ N, and return. So I am in favor of making the fix inside _split_share like @matejcik proposed. In fact, that is exactly the code that we had there before the code review, which is paradoxically when this bug got introduced.

[andrewkozlik]

I made the correction to the SLIP-0039 specification as described above.

[andrewkozlik]

I think the test_group_sharing_threshold_1() that @howech added was OK, it just needed some cleanup. I added it back in with some minor changes and merged to master: https://github.com/trezor/python-shamir-mnemonic/commit/b836a948369866803061d45dd8b0fe392b666c0a Thanks!