-
Notifications
You must be signed in to change notification settings - Fork 158
Add sumcheck protocol #973
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
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #973 +/- ##
==========================================
+ Coverage 71.66% 71.91% +0.24%
==========================================
Files 156 159 +3
Lines 34316 34659 +343
==========================================
+ Hits 24593 24924 +331
- Misses 9723 9735 +12 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
poly_evals.push(FieldElement::zero()); | ||
} | ||
|
||
DenseMultilinearPolynomial { | ||
evals: poly_evals.clone(), |
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.
It would be better if we can avoid this clone
let half = 1 << (n - 1); | ||
let new_evals: Vec<FieldElement<F>> = (0..half) | ||
.map(|j| { | ||
let a = self.evals[j].clone(); |
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.
same, would be better to work with references
} | ||
evaluations.push(self.evaluate(point).unwrap()); | ||
} | ||
evaluations |
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.
Given that we are in Lagrange basis, this is just returning the elements contained in the vector defining the polynomial
|
||
pub trait Channel<F: IsField> { | ||
fn append_field_element(&mut self, element: &FieldElement<F>); | ||
fn draw_felt(&mut self) -> FieldElement<F>; |
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.
i would use consistent naming
provers/sumcheck/src/verifier.rs
Outdated
let eval_1 = univar.evaluate(&FieldElement::<F>::one()); | ||
|
||
if self.round == 0 { | ||
println!( |
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 are we keeping the println? The way I see this, the protocol works so long as the polynomial is multilinear
crypto/src/lib.rs
Outdated
@@ -9,3 +9,4 @@ pub mod errors; | |||
pub mod fiat_shamir; | |||
pub mod hash; | |||
pub mod merkle_tree; | |||
//pub mod sumcheck; |
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 do you comment this line? it's better to remove it
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.
Forgot to delete it. Fixed in c8f3822
crypto/src/lib.rs
Outdated
@@ -9,3 +9,4 @@ pub mod errors; | |||
pub mod fiat_shamir; | |||
pub mod hash; | |||
pub mod merkle_tree; | |||
//pub mod sumcheck; |
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 commented?
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.
Forgot to delete it. Fixed in c8f3822
} | ||
|
||
/// Constructs a DenseMultilinearPolynomial from a vector of evaluations and the number of variables. | ||
pub fn from_evaluations_vec(num_vars: usize, evaluations: Vec<FieldElement<F>>) -> Self { |
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 function could be implemented using the From
trait
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.
Implemented in 98320e4
provers/sumcheck/src/verifier.rs
Outdated
}); | ||
} | ||
} else { | ||
let sum = eval_0.clone() + eval_1.clone(); |
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.
is it possible to sum without clonning the elements?
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.
Solved in c8f3822
/// Collapses the last variable by fixing it to 0 and 1, | ||
/// sums the evaluations, and returns a univariate polynomial (as a Polynomial) | ||
/// of the form: sum0 + (sum1 - sum0) * x. | ||
pub fn to_univariate(&self) -> Polynomial<FieldElement<F>> { |
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.
If the polynomial is linear, the prover can save work from the verifier by providing poly(0) and poly(1), and so the verifier just has to check poly(0) + poly(1) = sum. If the polynomial is not linear, this optimization is not as useful
where | ||
<F as IsField>::BaseType: Send + Sync, | ||
{ | ||
pub fn new( |
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.
If the sumcheck is just for multilinear, having two evaluations per polynomial is more than enough, and no additional checks will be needed. However, if the degree is larger, then we must ensure that the maximum degree is bounded by d.
Add sumcheck protocol
Description
This PR introduces a basic naive implementation of the Sumcheck Protocol using Multilinear Polynomials.
Type of change