-
Notifications
You must be signed in to change notification settings - Fork 10
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
Validity checks #38
Validity checks #38
Conversation
e5fe1ad
to
dab1a40
Compare
0d5d274
to
6d51e71
Compare
3a6e32c
to
991685e
Compare
Benchmark for a8b951fClick to view benchmark
|
991685e
to
da92818
Compare
Benchmark for 190d8bfClick to view benchmark
|
# Conflicts: # .github/workflows/workspace.yml # ferveo/benches/benchmarks/pvdkg.rs # ferveo/src/dkg/pv.rs # ferveo/src/vss/pvss.rs # tpke/src/ciphertext.rs # tpke/src/combine.rs # tpke/src/context.rs # tpke/src/decryption.rs # tpke/src/key_share.rs # tpke/src/lib.rs
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 great, only suggestion is to benchmark verify_optimistic
and verify_full
@theref Added missing benchmarks, please see the dropdown in the OP. |
ca83a9e
to
dd9e458
Compare
ferveo/src/vss/pvss.rs
Outdated
// Y = \sum_i y_i \alpha^i | ||
// A = \sum_i a_i \alpha^i |
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.
// Y = \sum_i y_i \alpha^i | |
// A = \sum_i a_i \alpha^i |
I think these lines are outdated, there's no alpha_i anymore
ferveo/src/vss/pvss.rs
Outdated
@@ -335,6 +338,28 @@ mod test_pvss { | |||
assert!(!pvss.verify_optimistic()); | |||
} | |||
|
|||
/// Check that if PVSS shares are tempered with, the full verification fails |
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.
/// Check that if PVSS shares are tempered with, the full verification fails | |
/// Check that if PVSS shares are tampered with, the full verification fails |
// Optimistic verification should not catch this issue | ||
assert!(bad_pvss.verify_optimistic()); | ||
// Full verification should catch this issue | ||
assert!(!bad_pvss.verify_full(&dkg)); |
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.
Nice test
tpke/src/ciphertext.rs
Outdated
pub fn check_ciphertext_validity<E: PairingEngine>( | ||
c: &Ciphertext<E>, | ||
aad: &[u8], | ||
) -> bool { | ||
) -> Result<()> { | ||
let g_inv = E::G1Prepared::from(-E::G1Affine::prime_subgroup_generator()); |
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.
Can we take this from SetupParams
?
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, we just need to pass g_inv
through a lot of places. See my latest commit for details.
e5fe1ad
verify_full
Benchmarks
Ciphertext validity
PVSS Validity (optimistic vs full verification)