[examples] Remove prost#709
Conversation
acbf094 to
eee51f3
Compare
eee51f3 to
69cc683
Compare
| }; | ||
| if !C::verify(Some(ACK_NAMESPACE), &payload, public_key, &sig) { | ||
| let payload = payload(round, &sender, &commitment); | ||
| if !C::verify(Some(ACK_NAMESPACE), &payload, public_key, &signature) { |
There was a problem hiding this comment.
I suggest building verify() into the type we are verifying itself. See #735 for examples of how to do this.
There was a problem hiding this comment.
While it might be nice, I think it doesn't make things that much cleaner in this case. Since each of the enum types are quite different, it doesn't make sense to have a verify() function on the top-level enum (because some types like Abort don't need any information to validate), meaning that there would just be optional verify() functions on the individual structs.
Here's what that would look like, which is actually pretty decent, just kind of weird that you have to re-pass-in the round variable even though it is on the Commitment itself
impl<C: Verifier> Commitment<C> {
/// Verifies the acknowledgments against the dealer's public commitment.
pub fn verify(
&self,
round: u64,
dealer: C::PublicKey,
participants: &Vec<C::PublicKey>,
) -> bool {
let payload = payload(round, &dealer, &self.commitment);
for (i, signature) in self.acks.iter() {
let Some(public_key) = participants.get(*i as usize) else {
return false;
};
if !C::verify(Some(ACK_NAMESPACE), &payload, public_key, signature) {
return false;
}
}
true
}
}
In this case, most of the actual validation work for Commitment occurs in cryptography/src/bls12381/dkgs/arbiter.rs, so the validation function isn't actually able to remove that much work unless I start reaching into the cryptography module
Especially since this is an example, I suggest we just leave it for now
There was a problem hiding this comment.
I have cleaned up the section of code that would have used this function, making it less useful
694b5d2 to
bf32c5a
Compare
|
This should now be unblocked! |
907f694 to
9880736
Compare
d53337c to
dbe0134
Compare
| payload.put_u64(round); | ||
| payload.extend_from_slice(dealer); | ||
| payload.extend_from_slice(commitment); | ||
| pub fn payload<P: Array>(round: u64, dealer: &P, commitment: &poly::Public) -> Vec<u8> { |
There was a problem hiding this comment.
I wonder if it is worth just creating an Ack object and implementing Codec over it?
There was a problem hiding this comment.
It mirrors the function here
Let me create an issue to consider modifying both of these functions at once
b513ccc to
9ca32ec
Compare
9ca32ec to
219d74f
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
@@ Coverage Diff @@
## main #709 +/- ##
=======================================
Coverage 89.37% 89.37%
=======================================
Files 165 165
Lines 41293 41305 +12
=======================================
+ Hits 36904 36915 +11
- Misses 4389 4390 +1
... and 2 files with indirect coverage changes Continue to review full report in Codecov by Sentry.
🚀 New features to boost your workflow:
|
Fixes #707