-
Notifications
You must be signed in to change notification settings - Fork 8
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Add socket comms for Participant demo #131
Conversation
Remove group signature verification from participant demo
…to socket_comms_participant_92
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good, thanks! I have some questions/suggestions but they are optional. We can merge this and improve it later
let _ = handler | ||
.network() | ||
.listen(Transport::FramedTcp, addr) | ||
.unwrap(); | ||
.map_err(|e| println!("{}", e)); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why was this needed? This prints the error but ignores it, continuing to run
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oops! I was debugging. I can remove.
@@ -16,7 +16,7 @@ pub struct CommitmentsConfig { | |||
pub signer_commitments: BTreeMap<Identifier, SigningCommitments>, | |||
} | |||
|
|||
pub fn step_2( | |||
pub async fn step_2( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why is this async now? Doesn't seem to need to
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it doesn't need to be. I can't remember why I put that there, but I meant to remove it.
pub fn generate_nonces_and_commitments( | ||
key_package: &KeyPackage, | ||
rng: &mut ThreadRng, | ||
) -> (SigningNonces, SigningCommitments) { | ||
let (nonces, commitments) = frost::round1::commit(key_package.signing_share(), rng); | ||
|
||
// TODO: Store nonces | ||
|
||
(nonces, commitments) | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This doesn't seem to be needed
identifier: Identifier, | ||
#[cfg(feature = "redpallas")] randomizer: frost::round2::Randomizer, | ||
) -> Result<SigningPackage, Box<dyn Error>>; | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The randomizer should be an output here, not an input, and should be send by the Coordinator (I also got it wrong and forgot to send the randomizer in the Coordinator). We need to figure out the best way to do this, but I think we could go with the "ugly" solution of returning a (SigningPackage, Randomizer) tuple if redpallas
is enabled. But let's do this in a separate PR and make it sure it works with Ed25519 first.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sounds good
This closes #92
The coordinator server has already been set up and this creates the option of using the participant demos as a client
The group signature verification has been removed from the participant demo as it's not needed