-
Notifications
You must be signed in to change notification settings - Fork 3
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
Dedupe (Working) #37
base: main
Are you sure you want to change the base?
Dedupe (Working) #37
Conversation
|
||
let claim_inner_final_expected = (evals[0] + r * evals[1] + r * r * evals[2]) * eval_Z; | ||
if claim_inner_final != claim_inner_final_expected { | ||
// DEDUPE(arasuarun): add |
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.
Let's add a more specific error here and remove the comment
}; | ||
|
||
|
||
let evals = multi_evaluate_uniform(&[&vk.S_single.A, &vk.S_single.B, &vk.S_single.C], &r_x, &r_y, vk.num_steps); |
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 lambda does not get reused, let's remove the wrapper and add a comment instead.
src/spartan/upsnark.rs
Outdated
|
||
// verify claim_inner_final | ||
// this should be log (num segments) | ||
let N_PREFIX = (vk.num_vars_total.trailing_zeros() as usize - vk.num_steps.trailing_zeros() as usize) + 1; |
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.
nit: better comment
// constant term | ||
let mut poly_X = vec![(0, 1.into())]; | ||
//remaining inputs | ||
poly_X.extend( |
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 will allocate twice
let claim_outer_final_expected = | ||
taus_bound_rx * (claim_Az * claim_Bz - claim_Cz); | ||
if claim_outer_final != claim_outer_final_expected { | ||
return Err(SpartanError::InvalidSumcheckProof); |
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.
Use more specific error here
src/spartan/upsnark.rs
Outdated
let poly_ABC = { | ||
let NUM_STEPS_BITS = pk.num_steps.trailing_zeros(); | ||
let (rx_con, rx_ts) = r_x.split_at(r_x.len() - NUM_STEPS_BITS as usize); | ||
let eq_rx_con = EqPolynomial::new(rx_con.to_vec()).evals(); |
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.
Parallelize these eq.evals()
calls
src/spartan/upsnark.rs
Outdated
let eq_rx_con = EqPolynomial::new(rx_con.to_vec()).evals(); | ||
let eq_rx_ts = EqPolynomial::new(rx_ts.to_vec()).evals(); | ||
|
||
let N_STEPS = pk.num_steps; |
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.
nit: rm caps from non-constant
src/spartan/upsnark.rs
Outdated
|
||
// this is the polynomial extended from the vector r_A * A(r_x, y) + r_B * B(r_x, y) + r_C * C(r_x, y) for all y | ||
let poly_ABC = { | ||
let NUM_STEPS_BITS = pk.num_steps.trailing_zeros(); |
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.
nit: rm caps from non-constant
src/spartan/upsnark.rs
Outdated
// The number of prefix bits needed to identify a segment within the witness vector | ||
// assuming that num_vars_total is a power of 2 and each segment has length num_steps, which is also a power of 2. | ||
// The +1 is the first element used to separate the inputs and the witness. | ||
let N_PREFIX = (pk.num_vars_total.trailing_zeros() as usize - pk.num_steps.trailing_zeros() as usize) + 1; |
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.
nit: rm caps from non-constant
I don't think there will be too many changes for deduping on the Spartan side (mostly just on Jolt).