Bump Synedrion to latest master
#946
Conversation
This was removed in: entropyxyz/synedrion#124
This was removed in entropyxyz/synedrion#128.
Introduced in entropyxyz/synedrion#128.
This was introduced in entropyxyz/synedrion#124.
Old one was removed inhttps://github.com/entropyxyz/synedrion/pull/124.
There was a problem hiding this comment.
looks good from my end but should also check with @fjarri before merging
There was a problem hiding this comment.
Looks great. And once this is merged we get some other nice things such as being able to get the session id from a synedrion protocol message.
Normally i would discourage using different versions of synedrion in different crates but since you are planning to fix as soon i am approving.
| @@ -206,24 +209,26 @@ pub async fn execute_dkg( | |||
| tracing::debug!("Executing DKG"); | |||
| let broadcaster = chans.0.clone(); | |||
|
|
|||
| let mut party_ids: Vec<PartyId> = | |||
| let party_ids: BTreeSet<PartyId> = | |||
There was a problem hiding this comment.
+1 for a set rather than a mutable vector
There was a problem hiding this comment.
🙌 thank mr bogdan 🙌
| ) -> Result<(Vec<PartyId>, bool), ProtocolExecutionErr> { | ||
| let mut parties = vec![]; | ||
| ) -> Result<(BTreeSet<PartyId>, bool), ProtocolExecutionErr> { | ||
| let validators = validators.iter().cloned().collect::<Vec<PartyId>>(); |
There was a problem hiding this comment.
i guess for this to work we need this to turn the set into a vector with consistent ordering. Since the tests pass this must be the case.
There was a problem hiding this comment.
I believe this works because BTreeSet requires that any item inserted into the set implements Ord, meaning that the contents of the set will be ordered at any point in time.
The latest
HEADof Synedrion includes child key derivation functionality that I needfor the new registration flow, so I had to update the dependency.
Two things to note:
Implement
serdefor CGGMP21 params synedrion#133 gets merged since that adds someserdesupport that I needTODOs in places where I want to double check that functionalityremained the same, but I'll do that tomorrow. I'd still consider the PR as a whole
ready for review