From 439303458df319d0ffbae6c47595f63afd31c486 Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Mon, 12 Jul 2021 11:56:53 -0700 Subject: [PATCH 1/5] fix --- src/gadgets/insert.rs | 15 ++++++++++----- 1 file changed, 10 insertions(+), 5 deletions(-) diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs index 1f69cb24..f618eb2b 100644 --- a/src/gadgets/insert.rs +++ b/src/gadgets/insert.rs @@ -37,8 +37,7 @@ impl, const D: usize> CircuitBuilder { self.sub(one, not_equal) } - /// Inserts a `Target` in a vector at a non-deterministic index. This is done by rotating to the - /// left, inserting at 0 and then rotating to the right. + /// Inserts a `Target` in a vector at a non-deterministic index. /// Note: `index` is not range-checked. pub fn insert( &mut self, @@ -49,7 +48,11 @@ impl, const D: usize> CircuitBuilder { let mut already_inserted = self.zero(); let mut new_list = Vec::new(); - for i in 0..v.len() { + // Add a dummy value at the end (which will never be used). + let mut new_v = v.clone(); + new_v.push(ExtensionTarget([Target::PublicInput{index: 0}; D])); + + for i in 0..new_v.len() { let one = self.one(); let cur_index = self.constant(F::from_canonical_usize(i)); @@ -58,12 +61,14 @@ impl, const D: usize> CircuitBuilder { let mut new_item = self.zero_extension(); new_item = self.scalar_mul_add_extension(insert_here, element, new_item); if i > 0 { - new_item = self.scalar_mul_add_extension(already_inserted, v[i - 1], new_item); + new_item = self.scalar_mul_add_extension(already_inserted, new_v[i - 1], new_item); } already_inserted = self.add(already_inserted, insert_here); let not_already_inserted = self.sub(one, already_inserted); - new_item = self.scalar_mul_add_extension(not_already_inserted, v[i], new_item); + // On the last round, v[i] is the dummy value, but by that round not_already_inserted + // will always be 0. + new_item = self.scalar_mul_add_extension(not_already_inserted, new_v[i], new_item); new_list.push(new_item); } From 2df81e15c81caa30ab31c317d8b2eea0ebc3ef4f Mon Sep 17 00:00:00 2001 From: Nicholas Ward Date: Mon, 12 Jul 2021 12:16:13 -0700 Subject: [PATCH 2/5] switched to William's way, and len assert in test --- src/gadgets/insert.rs | 20 +++++++++----------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/src/gadgets/insert.rs b/src/gadgets/insert.rs index f618eb2b..6632eae7 100644 --- a/src/gadgets/insert.rs +++ b/src/gadgets/insert.rs @@ -48,31 +48,27 @@ impl, const D: usize> CircuitBuilder { let mut already_inserted = self.zero(); let mut new_list = Vec::new(); - // Add a dummy value at the end (which will never be used). - let mut new_v = v.clone(); - new_v.push(ExtensionTarget([Target::PublicInput{index: 0}; D])); - - for i in 0..new_v.len() { - let one = self.one(); - + let one = self.one(); + for i in 0..=v.len() { let cur_index = self.constant(F::from_canonical_usize(i)); let insert_here = self.is_equal(cur_index, index); let mut new_item = self.zero_extension(); new_item = self.scalar_mul_add_extension(insert_here, element, new_item); if i > 0 { - new_item = self.scalar_mul_add_extension(already_inserted, new_v[i - 1], new_item); + new_item = self.scalar_mul_add_extension(already_inserted, v[i - 1], new_item); } already_inserted = self.add(already_inserted, insert_here); let not_already_inserted = self.sub(one, already_inserted); - // On the last round, v[i] is the dummy value, but by that round not_already_inserted - // will always be 0. - new_item = self.scalar_mul_add_extension(not_already_inserted, new_v[i], new_item); + if i < v.len() { + new_item = self.scalar_mul_add_extension(not_already_inserted, v[i], new_item); + } new_list.push(new_item); } + new_list } } @@ -111,6 +107,8 @@ mod tests { let inserted = real_insert(i, elem, &v); let purported_inserted = builder.insert(it, elem, v.clone()); + assert_eq!(inserted.len(), purported_inserted.len()); + for (x, y) in inserted.into_iter().zip(purported_inserted) { builder.route_extension(x, y); } From 02f0715040fb99e8b0650cad58a172e8e1c906af Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Wed, 14 Jul 2021 08:39:09 +0200 Subject: [PATCH 3/5] PR feedback --- src/plonk_common.rs | 24 +++++++++++++++--------- src/verifier.rs | 5 +++++ 2 files changed, 20 insertions(+), 9 deletions(-) diff --git a/src/plonk_common.rs b/src/plonk_common.rs index 5179b474..b1f29803 100644 --- a/src/plonk_common.rs +++ b/src/plonk_common.rs @@ -73,7 +73,7 @@ pub(crate) fn eval_vanishing_poly, const D: usize>( gammas: &[F], alphas: &[F], ) -> Vec { - let max_degree = common_data.quotient_degree_factor; + let partial_products_degree = common_data.quotient_degree_factor; let (num_prods, final_num_prod) = common_data.num_partial_products; let constraint_terms = @@ -113,12 +113,15 @@ pub(crate) fn eval_vanishing_poly, const D: usize>( // The partial products considered for this iteration of `i`. let current_partial_products = &partial_products[i * num_prods..(i + 1) * num_prods]; // Check the quotient partial products. - let mut partial_product_check = - check_partial_products("ient_values, current_partial_products, max_degree); + let mut partial_product_check = check_partial_products( + "ient_values, + current_partial_products, + partial_products_degree, + ); // The first checks are of the form `q - n/d` which is a rational function not a polynomial. // We multiply them by `d` to get checks of the form `q*d - n` which low-degree polynomials. denominator_values - .chunks(max_degree) + .chunks(partial_products_degree) .zip(partial_product_check.iter_mut()) .for_each(|(d, q)| { *q *= d.iter().copied().product(); @@ -160,7 +163,7 @@ pub(crate) fn eval_vanishing_poly_base, const D: usize>( alphas: &[F], z_h_on_coset: &ZeroPolyOnCoset, ) -> Vec { - let max_degree = common_data.quotient_degree_factor; + let partial_products_degree = common_data.quotient_degree_factor; let (num_prods, final_num_prod) = common_data.num_partial_products; let constraint_terms = @@ -199,13 +202,16 @@ pub(crate) fn eval_vanishing_poly_base, const D: usize>( // The partial products considered for this iteration of `i`. let current_partial_products = &partial_products[i * num_prods..(i + 1) * num_prods]; - // Check the numerator partial products. - let mut partial_product_check = - check_partial_products("ient_values, current_partial_products, max_degree); + // Check the quotient partial products. + let mut partial_product_check = check_partial_products( + "ient_values, + current_partial_products, + partial_products_degree, + ); // The first checks are of the form `q - n/d` which is a rational function not a polynomial. // We multiply them by `d` to get checks of the form `q*d - n` which low-degree polynomials. denominator_values - .chunks(max_degree) + .chunks(partial_products_degree) .zip(partial_product_check.iter_mut()) .for_each(|(d, q)| { *q *= d.iter().copied().product(); diff --git a/src/verifier.rs b/src/verifier.rs index 64d4b371..d7c96365 100644 --- a/src/verifier.rs +++ b/src/verifier.rs @@ -60,6 +60,11 @@ pub(crate) fn verify, const D: usize>( let quotient_polys_zeta = &proof.openings.quotient_polys; let zeta_pow_deg = zeta.exp_power_of_2(common_data.degree_bits); let z_h_zeta = zeta_pow_deg - F::Extension::ONE; + // `quotient_polys_zeta` holds `num_challenges * quotient_degree_factor` evaluations. + // Each chunk of `quotient_degree_factor` holds the evaluations of `t_0(zeta),...,t_{quotient_degree_factor-1}(zeta)` + // where the "real" quotient polynomial is `t(X) = t_0(X) + t_1(X)*X^n + t_2(X)*X^{2n} + ...`. + // So to reconstruct `t(zeta)` we can compute `reduce_with_powers(chunk, zeta^n)` for each + // `quotient_degree_factor`-sized chunk of the original evaluations. for (i, chunk) in quotient_polys_zeta .chunks(common_data.quotient_degree_factor) .enumerate() From e68be5108542076a629563d20cd527fac5f322cf Mon Sep 17 00:00:00 2001 From: Daniel Lubarov Date: Wed, 14 Jul 2021 21:42:14 -0700 Subject: [PATCH 4/5] Imports --- src/bin/bench_recursion.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/src/bin/bench_recursion.rs b/src/bin/bench_recursion.rs index b55ee841..b75d4bdc 100644 --- a/src/bin/bench_recursion.rs +++ b/src/bin/bench_recursion.rs @@ -5,9 +5,6 @@ use plonky2::field::crandall_field::CrandallField; use plonky2::field::extension_field::Extendable; use plonky2::field::field::Field; use plonky2::fri::FriConfig; -use plonky2::gates::constant::ConstantGate; -use plonky2::gates::gmimc::GMiMCGate; -use plonky2::hash::GMIMC_ROUNDS; use plonky2::witness::PartialWitness; fn main() { From c678c554520c49390794ad1c62cf5685cda2792d Mon Sep 17 00:00:00 2001 From: Daniel Lubarov Date: Wed, 14 Jul 2021 21:43:55 -0700 Subject: [PATCH 5/5] Imports --- src/polynomial/commitment.rs | 2 +- src/polynomial/polynomial.rs | 1 - src/verifier.rs | 2 +- 3 files changed, 2 insertions(+), 3 deletions(-) diff --git a/src/polynomial/commitment.rs b/src/polynomial/commitment.rs index 7298331c..7698c39f 100644 --- a/src/polynomial/commitment.rs +++ b/src/polynomial/commitment.rs @@ -5,7 +5,7 @@ use crate::circuit_data::CommonCircuitData; use crate::field::extension_field::Extendable; use crate::field::extension_field::{FieldExtension, Frobenius}; use crate::field::field::Field; -use crate::fri::{prover::fri_proof, verifier::verify_fri_proof, FriConfig}; +use crate::fri::{prover::fri_proof, verifier::verify_fri_proof}; use crate::merkle_tree::MerkleTree; use crate::plonk_challenger::Challenger; use crate::plonk_common::PlonkPolynomials; diff --git a/src/polynomial/polynomial.rs b/src/polynomial/polynomial.rs index aa06d641..81d07b8f 100644 --- a/src/polynomial/polynomial.rs +++ b/src/polynomial/polynomial.rs @@ -1,7 +1,6 @@ use std::cmp::max; use std::iter::Sum; use std::ops::{Add, AddAssign, Mul, MulAssign, Sub, SubAssign}; -use std::time::Instant; use anyhow::{ensure, Result}; diff --git a/src/verifier.rs b/src/verifier.rs index d7c96365..d97d60e5 100644 --- a/src/verifier.rs +++ b/src/verifier.rs @@ -4,7 +4,7 @@ use crate::circuit_data::{CommonCircuitData, VerifierOnlyCircuitData}; use crate::field::extension_field::Extendable; use crate::field::field::Field; use crate::plonk_challenger::Challenger; -use crate::plonk_common::{eval_vanishing_poly, eval_zero_poly, reduce_with_powers}; +use crate::plonk_common::{eval_vanishing_poly, reduce_with_powers}; use crate::proof::Proof; use crate::vars::EvaluationVars;