chore: genesis keygen doesn't rely on tests

[msgmaxim]

Genesis keygen now works by calling the low level keygen functions to generate key shares instead of running a full ceremony (and thus relying on unit tests).

Notes for the reviewers:

• compute_keygen_result is extracted out of compute_keygen_result_info without changes (it used to construct KeygenResultInfo by setting params and account id mapping, which I don't think belonged in that function);
• generate_key_data should be of the most interest in this PR; it is the new function to be used to generate genesis keys;
• finalize_keygen moved outside of detail namespace mostly unchanged (now it also assembles KeygenResultInfo from parts)
• added genesis_keys_can_sign to verify that keyshares are generated correctly

[kylezs]

Cheers for this, just some unused imports to remove it seems.

[AlastairHolmes]

You call derive_aggregate_pubkey inside this loop. I think it would be nicer to use the already generated one outside of the loop.

[AlastairHolmes]

Will the result of derive_local_pubkeys_for_parties for each loop be the same? Could move that out as well, if so.

[msgmaxim]

compute_keygen_result procudes different keys for different parties (note that we give different shares to each party), so we can't take it outside of the loop.

Will the result of derive_local_pubkeys_for_parties for each loop be the same?

Yes, this particular value will be the same, but I prefer each party calling compute_keygen_result as this more closely resembles what parties do in a proper cermeony (rather than breaking it apart and creating more opportunities to diverge from ceremony code).

[AlastairHolmes]

I'd like to show you what the code could look like if #1671 and the above "move out of loop" where done. As I believe the code will be significantly cleaner, and allow more commonality to be factored out (Thereby actually decreasing the opportunities to diverge).

Thats why I suggested the change, so the checks that pubkey is "good", and be put/factored out into derive_aggregate_pubkey.

[AlastairHolmes]

Should we use generate_key_data in the single_party_keygen?

[AlastairHolmes]

I noticed we derive the pubkey in VerifyCommitmentsBroadcast2, but then we throw it away, and regenerate it at the final stage. Why do we do that? Wouldn't it be nicer to generate it once?

My problem with generating it twice is that we only check it is valid (contract compat etc) the first time, but not when we re-generate the key, which is confusing and dangerous IMO.

[msgmaxim]

Should we use generate_key_data in the single_party_keygen?

Perhaps, but outside of scope. Note that generate_key_data deals with key shares, while signle party keygen does not. It is possible we can make it work with 1 share to get what we want, but I would need to play around with that idea first.

[msgmaxim]

I noticed we derive the pubkey in VerifyCommitmentsBroadcast2, but then we throw it away

I'm assuming you talking about the ceremony code, and not the code contributed here. I don't particularly like having to send it all way through the stages, but you make a valid point, I will open an issue (#1671).

[AlastairHolmes]

Should we use generate_key_data in the single_party_keygen?

Perhaps, but outside of scope. Note that generate_key_data deals with key shares, while signle party keygen does not. It is possible we can make it work with 1 share to get what we want, but I would need to play around with that idea first.

It seems to work for me. I'm not sure I understand your concern.

I do disagree its out of scope though. If you introduce a new method (in this case it is more general than the existing that only allows single partys) of doing something, I believe you should transition all existing code to use that method.