From 60f3773a233ff30cef9824c9ee6f211078f9f0fc Mon Sep 17 00:00:00 2001 From: Daniel Lubarov Date: Wed, 9 Jun 2021 16:17:56 -0700 Subject: [PATCH 1/3] Try to simplify open_plonk and fri_combine_initial a bit more (#59) * Try to simplify open_plonk and fri_combine_initial a bit more - Use `alpha.powers()` in `open_plonk` instead of the more "manual" approach - No more "manually" reducing with `alpha_powers`; now using helper methods for that. - Renaming & other small tweaks * Remove type hint * Feedback --- src/fri/verifier.rs | 50 ++++++++++----------- src/plonk_common.rs | 34 +++++++++++---- src/polynomial/commitment.rs | 85 +++++++++++++++--------------------- src/polynomial/polynomial.rs | 7 +++ 4 files changed, 90 insertions(+), 86 deletions(-) diff --git a/src/fri/verifier.rs b/src/fri/verifier.rs index 24e24f64..68f4ee15 100644 --- a/src/fri/verifier.rs +++ b/src/fri/verifier.rs @@ -1,5 +1,4 @@ use anyhow::{ensure, Result}; -use itertools::izip; use crate::field::extension_field::{flatten, Extendable, FieldExtension, OEF}; use crate::field::field::Field; @@ -156,31 +155,29 @@ fn fri_combine_initial, const D: usize>( let mut sum = F::Extension::ZERO; // We will add three terms to `sum`: - // - one for polynomials opened at `x` only - // - one for polynomials opened at `x` and `g x` - // - one for polynomials opened at `x` and its conjugate + // - one for various polynomials which are opened at a single point `x` + // - one for Zs, which are opened at `x` and `g x` + // - one for wire polynomials, which are opened at `x` and its conjugate - let evals = [0, 1, 4] + let single_evals = [0, 1, 4] .iter() .flat_map(|&i| proof.unsalted_evals(i, config)) .map(|&e| F::Extension::from_basefield(e)); - let openings = os + let single_openings = os .constants .iter() .chain(&os.plonk_s_sigmas) .chain(&os.quotient_polys); - let numerator = izip!(evals, openings, &mut alpha_powers) - .map(|(e, &o, a)| a * (e - o)) - .sum::(); - let denominator = subgroup_x - zeta; - sum += numerator / denominator; + let single_diffs = single_evals.zip(single_openings).map(|(e, &o)| e - o); + let single_numerator = reduce_with_iter(single_diffs, &mut alpha_powers); + let single_denominator = subgroup_x - zeta; + sum += single_numerator / single_denominator; - let ev: F::Extension = proof + let zs_evals = proof .unsalted_evals(3, config) .iter() - .zip(alpha_powers.clone()) - .map(|(&e, a)| a * e.into()) - .sum(); + .map(|&e| F::Extension::from_basefield(e)); + let zs_composition_eval = reduce_with_iter(zs_evals, alpha_powers.clone()); let zeta_right = F::Extension::primitive_root_of_unity(degree_log) * zeta; let zs_interpol = interpolant(&[ (zeta, reduce_with_iter(&os.plonk_zs, alpha_powers.clone())), @@ -189,25 +186,24 @@ fn fri_combine_initial, const D: usize>( reduce_with_iter(&os.plonk_zs_right, &mut alpha_powers), ), ]); - let numerator = ev - zs_interpol.eval(subgroup_x); - let denominator = (subgroup_x - zeta) * (subgroup_x - zeta_right); - sum += numerator / denominator; + let zs_numerator = zs_composition_eval - zs_interpol.eval(subgroup_x); + let zs_denominator = (subgroup_x - zeta) * (subgroup_x - zeta_right); + sum += zs_numerator / zs_denominator; - let ev: F::Extension = proof + let wire_evals = proof .unsalted_evals(2, config) .iter() - .zip(alpha_powers.clone()) - .map(|(&e, a)| a * e.into()) - .sum(); + .map(|&e| F::Extension::from_basefield(e)); + let wire_composition_eval = reduce_with_iter(wire_evals, alpha_powers.clone()); let zeta_frob = zeta.frobenius(); - let wire_evals_frob = os.wires.iter().map(|e| e.frobenius()).collect::>(); + let wire_evals_frob = os.wires.iter().map(|e| e.frobenius()); let wires_interpol = interpolant(&[ (zeta, reduce_with_iter(&os.wires, alpha_powers.clone())), - (zeta_frob, reduce_with_iter(&wire_evals_frob, alpha_powers)), + (zeta_frob, reduce_with_iter(wire_evals_frob, alpha_powers)), ]); - let numerator = ev - wires_interpol.eval(subgroup_x); - let denominator = (subgroup_x - zeta) * (subgroup_x - zeta_frob); - sum += numerator / denominator; + let wires_numerator = wire_composition_eval - wires_interpol.eval(subgroup_x); + let wires_denominator = (subgroup_x - zeta) * (subgroup_x - zeta_frob); + sum += wires_numerator / wires_denominator; sum } diff --git a/src/plonk_common.rs b/src/plonk_common.rs index e84cb340..e8c3c995 100644 --- a/src/plonk_common.rs +++ b/src/plonk_common.rs @@ -1,9 +1,12 @@ +use std::borrow::Borrow; + use crate::circuit_builder::CircuitBuilder; use crate::circuit_data::CommonCircuitData; use crate::field::extension_field::target::ExtensionTarget; use crate::field::extension_field::Extendable; use crate::field::field::Field; use crate::gates::gate::GateRef; +use crate::polynomial::polynomial::PolynomialCoeffs; use crate::target::Target; use crate::vars::{EvaluationTargets, EvaluationVars, EvaluationVarsBase}; @@ -209,13 +212,26 @@ pub(crate) fn reduce_with_powers_recursive, const D: usize>( todo!() } -pub(crate) fn reduce_with_iter(terms: &[F], coeffs: I) -> F -where - I: IntoIterator, -{ - let mut sum = F::ZERO; - for (&term, coeff) in terms.iter().zip(coeffs) { - sum += coeff * term; - } - sum +/// Reduce a sequence of field elements by the given coefficients. +pub(crate) fn reduce_with_iter( + terms: impl IntoIterator>, + coeffs: impl IntoIterator>, +) -> F { + terms + .into_iter() + .zip(coeffs) + .map(|(t, c)| *t.borrow() * *c.borrow()) + .sum() +} + +/// Reduce a sequence of polynomials by the given coefficients. +pub(crate) fn reduce_polys_with_iter( + polys: impl IntoIterator>>, + coeffs: impl IntoIterator>, +) -> PolynomialCoeffs { + polys + .into_iter() + .zip(coeffs) + .map(|(p, c)| p.borrow() * *c.borrow()) + .sum() } diff --git a/src/polynomial/commitment.rs b/src/polynomial/commitment.rs index e8cb2542..5169748b 100644 --- a/src/polynomial/commitment.rs +++ b/src/polynomial/commitment.rs @@ -8,7 +8,7 @@ use crate::field::lagrange::interpolant; use crate::fri::{prover::fri_proof, verifier::verify_fri_proof, FriConfig}; use crate::merkle_tree::MerkleTree; use crate::plonk_challenger::Challenger; -use crate::plonk_common::reduce_with_powers; +use crate::plonk_common::{reduce_polys_with_iter, reduce_with_iter}; use crate::polynomial::polynomial::PolynomialCoeffs; use crate::proof::{FriProof, Hash, OpeningSet}; use crate::timed; @@ -90,7 +90,7 @@ impl ListPolynomialCommitment { assert!(D > 1, "Not implemented for D=1."); let degree_log = log2_strict(commitments[0].degree); let g = F::Extension::primitive_root_of_unity(degree_log); - for &p in &[zeta, g * zeta] { + for p in &[zeta, g * zeta] { assert_ne!( p.exp(1 << degree_log as u64), F::Extension::ONE, @@ -110,45 +110,34 @@ impl ListPolynomialCommitment { challenger.observe_opening_set(&os); let alpha = challenger.get_extension_challenge(); - let mut cur_alpha = F::Extension::ONE; + let mut alpha_powers = alpha.powers(); // Final low-degree polynomial that goes into FRI. let mut final_poly = PolynomialCoeffs::empty(); - // Count the total number of polynomials accumulated into `final_poly`. - let mut poly_count = 0; // Polynomials opened at a single point. - let composition_poly = [0, 1, 4] + let single_polys = [0, 1, 4] .iter() .flat_map(|&i| &commitments[i].polynomials) - .rev() - .fold(PolynomialCoeffs::empty(), |acc, p| { - poly_count += 1; - &(&acc * alpha) + &p.to_extension() - }); - let composition_eval = [&os.constants, &os.plonk_s_sigmas, &os.quotient_polys] - .iter() - .flat_map(|v| v.iter()) - .rev() - .fold(F::Extension::ZERO, |acc, &e| acc * alpha + e); + .map(|p| p.to_extension()); + let single_os = [&os.constants, &os.plonk_s_sigmas, &os.quotient_polys]; + let single_evals = single_os.iter().flat_map(|v| v.iter()); + let single_composition_poly = reduce_polys_with_iter(single_polys, alpha_powers.clone()); + let single_composition_eval = reduce_with_iter(single_evals, &mut alpha_powers); - let quotient = Self::compute_quotient(&[zeta], &[composition_eval], &composition_poly); - final_poly = &final_poly + &("ient * cur_alpha); - cur_alpha = alpha.exp(poly_count); + let single_quotient = Self::compute_quotient( + &[zeta], + &[single_composition_eval], + &single_composition_poly, + ); + final_poly = &final_poly + &single_quotient; // Zs polynomials are opened at `zeta` and `g*zeta`. - let zs_composition_poly = - commitments[3] - .polynomials - .iter() - .rev() - .fold(PolynomialCoeffs::empty(), |acc, p| { - poly_count += 1; - &(&acc * alpha) + &p.to_extension() - }); + let zs_polys = commitments[3].polynomials.iter().map(|p| p.to_extension()); + let zs_composition_poly = reduce_polys_with_iter(zs_polys, alpha_powers.clone()); let zs_composition_evals = [ - reduce_with_powers(&os.plonk_zs, alpha), - reduce_with_powers(&os.plonk_zs_right, alpha), + reduce_with_iter(&os.plonk_zs, alpha_powers.clone()), + reduce_with_iter(&os.plonk_zs_right, &mut alpha_powers), ]; let zs_quotient = Self::compute_quotient( @@ -156,33 +145,25 @@ impl ListPolynomialCommitment { &zs_composition_evals, &zs_composition_poly, ); - final_poly = &final_poly + &(&zs_quotient * cur_alpha); - cur_alpha = alpha.exp(poly_count); + final_poly = &final_poly + &zs_quotient; // When working in an extension field, need to check that wires are in the base field. // Check this by opening the wires polynomials at `zeta` and `zeta.frobenius()` and using the fact that // a polynomial `f` is over the base field iff `f(z).frobenius()=f(z.frobenius())` with high probability. - let wires_composition_poly = - commitments[2] - .polynomials - .iter() - .rev() - .fold(PolynomialCoeffs::empty(), |acc, p| { - poly_count += 1; - &(&acc * alpha) + &p.to_extension() - }); + let wire_polys = commitments[2].polynomials.iter().map(|p| p.to_extension()); + let wire_composition_poly = reduce_polys_with_iter(wire_polys, alpha_powers.clone()); let wire_evals_frob = os.wires.iter().map(|e| e.frobenius()).collect::>(); - let wires_composition_evals = [ - reduce_with_powers(&os.wires, alpha), - reduce_with_powers(&wire_evals_frob, alpha), + let wire_composition_evals = [ + reduce_with_iter(&os.wires, alpha_powers.clone()), + reduce_with_iter(&wire_evals_frob, alpha_powers), ]; let wires_quotient = Self::compute_quotient( &[zeta, zeta.frobenius()], - &wires_composition_evals, - &wires_composition_poly, + &wire_composition_evals, + &wire_composition_poly, ); - final_poly = &final_poly + &(&wires_quotient * cur_alpha); + final_poly = &final_poly + &wires_quotient; let lde_final_poly = final_poly.lde(config.rate_bits); let lde_final_values = lde_final_poly @@ -275,9 +256,9 @@ impl, const D: usize> OpeningProof { #[cfg(test)] mod tests { use anyhow::Result; + use rand::Rng; use super::*; - use rand::Rng; fn gen_random_test_case, const D: usize>( k: usize, @@ -359,8 +340,10 @@ mod tests { } mod quadratic { - use super::*; use crate::field::crandall_field::CrandallField; + + use super::*; + #[test] fn test_batch_polynomial_commitment() -> Result<()> { check_batch_polynomial_commitment::() @@ -368,8 +351,10 @@ mod tests { } mod quartic { - use super::*; use crate::field::crandall_field::CrandallField; + + use super::*; + #[test] fn test_batch_polynomial_commitment() -> Result<()> { check_batch_polynomial_commitment::() diff --git a/src/polynomial/polynomial.rs b/src/polynomial/polynomial.rs index 5e3dd9f0..7c30ec02 100644 --- a/src/polynomial/polynomial.rs +++ b/src/polynomial/polynomial.rs @@ -5,6 +5,7 @@ use crate::field::extension_field::Extendable; use crate::field::fft::{fft, ifft}; use crate::field::field::Field; use crate::util::log2_strict; +use std::iter::Sum; /// A polynomial in point-value form. /// @@ -222,6 +223,12 @@ impl Add for &PolynomialCoeffs { } } +impl Sum for PolynomialCoeffs { + fn sum>(iter: I) -> Self { + iter.fold(Self::empty(), |acc, p| &acc + &p) + } +} + impl Sub for &PolynomialCoeffs { type Output = PolynomialCoeffs; From f929f946261d07004ea2d7f1f96bdc62c6f82312 Mon Sep 17 00:00:00 2001 From: Daniel Lubarov Date: Thu, 10 Jun 2021 14:10:35 -0700 Subject: [PATCH 2/3] Have rustfmt group imports (#60) * Have rustfmt group imports See `rustfmt.toml`; the rest is automated changes. * fmt --- rustfmt.toml | 2 ++ src/bin/bench_ldes.rs | 3 +-- src/bin/bench_recursion.rs | 1 - src/field/crandall_field.rs | 4 ++-- src/field/extension_field/algebra.rs | 6 ++++-- src/field/extension_field/quadratic.rs | 10 ++++++---- src/field/field_testing.rs | 3 ++- src/merkle_tree.rs | 3 +-- src/polynomial/commitment.rs | 6 ++---- src/polynomial/polynomial.rs | 5 ++--- src/proof.rs | 3 ++- 11 files changed, 24 insertions(+), 22 deletions(-) create mode 100644 rustfmt.toml diff --git a/rustfmt.toml b/rustfmt.toml new file mode 100644 index 00000000..65106950 --- /dev/null +++ b/rustfmt.toml @@ -0,0 +1,2 @@ +unstable_features = true +group_imports = "StdExternalCrate" diff --git a/src/bin/bench_ldes.rs b/src/bin/bench_ldes.rs index 1fb02d65..e6d7c457 100644 --- a/src/bin/bench_ldes.rs +++ b/src/bin/bench_ldes.rs @@ -1,10 +1,9 @@ use std::time::Instant; -use rayon::prelude::*; - use plonky2::field::crandall_field::CrandallField; use plonky2::field::field::Field; use plonky2::polynomial::polynomial::PolynomialValues; +use rayon::prelude::*; type F = CrandallField; diff --git a/src/bin/bench_recursion.rs b/src/bin/bench_recursion.rs index 35ba280d..19f7716c 100644 --- a/src/bin/bench_recursion.rs +++ b/src/bin/bench_recursion.rs @@ -1,5 +1,4 @@ use env_logger::Env; - use plonky2::circuit_builder::CircuitBuilder; use plonky2::circuit_data::CircuitConfig; use plonky2::field::crandall_field::CrandallField; diff --git a/src/field/crandall_field.rs b/src/field/crandall_field.rs index 46dcb172..0cee8860 100644 --- a/src/field/crandall_field.rs +++ b/src/field/crandall_field.rs @@ -1,5 +1,7 @@ use std::fmt; use std::fmt::{Debug, Display, Formatter}; +use std::hash::{Hash, Hasher}; +use std::iter::{Product, Sum}; use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Neg, Sub, SubAssign}; use num::Integer; @@ -8,8 +10,6 @@ use crate::field::extension_field::quadratic::QuadraticCrandallField; use crate::field::extension_field::quartic::QuarticCrandallField; use crate::field::extension_field::Extendable; use crate::field::field::Field; -use std::hash::{Hash, Hasher}; -use std::iter::{Product, Sum}; /// EPSILON = 9 * 2**28 - 1 const EPSILON: u64 = 2415919103; diff --git a/src/field/extension_field/algebra.rs b/src/field/extension_field/algebra.rs index eac0bbd8..996053bf 100644 --- a/src/field/extension_field/algebra.rs +++ b/src/field/extension_field/algebra.rs @@ -1,8 +1,9 @@ -use crate::field::extension_field::OEF; use std::fmt::{Debug, Display, Formatter}; use std::iter::{Product, Sum}; use std::ops::{Add, AddAssign, Mul, MulAssign, Neg, Sub, SubAssign}; +use crate::field::extension_field::OEF; + /// Let `F_D` be the optimal extension field `F[X]/(X^D-W)`. Then `ExtensionAlgebra` is the quotient `F_D[X]/(X^D-W)`. /// It's a `D`-dimensional algebra over `F_D` useful to lift the multiplication over `F_D` to a multiplication over `(F_D)^D`. #[derive(Copy, Clone)] @@ -154,11 +155,12 @@ impl, const D: usize> PolynomialCoeffsAlgebra { #[cfg(test)] mod tests { + use itertools::Itertools; + use crate::field::crandall_field::CrandallField; use crate::field::extension_field::algebra::ExtensionAlgebra; use crate::field::extension_field::{Extendable, FieldExtension}; use crate::field::field::Field; - use itertools::Itertools; /// Tests that the multiplication on the extension algebra lifts that of the field extension. fn test_extension_algebra, const D: usize>() { diff --git a/src/field/extension_field/quadratic.rs b/src/field/extension_field/quadratic.rs index 27fc33a1..b61ccbaa 100644 --- a/src/field/extension_field/quadratic.rs +++ b/src/field/extension_field/quadratic.rs @@ -1,12 +1,14 @@ -use crate::field::crandall_field::CrandallField; -use crate::field::extension_field::{FieldExtension, OEF}; -use crate::field::field::Field; -use rand::Rng; use std::fmt::{Debug, Display, Formatter}; use std::hash::Hash; use std::iter::{Product, Sum}; use std::ops::{Add, AddAssign, Div, DivAssign, Mul, MulAssign, Neg, Sub, SubAssign}; +use rand::Rng; + +use crate::field::crandall_field::CrandallField; +use crate::field::extension_field::{FieldExtension, OEF}; +use crate::field::field::Field; + #[derive(Copy, Clone, Eq, PartialEq, Hash)] pub struct QuadraticCrandallField([CrandallField; 2]); diff --git a/src/field/field_testing.rs b/src/field/field_testing.rs index 7d2339af..53e9c63c 100644 --- a/src/field/field_testing.rs +++ b/src/field/field_testing.rs @@ -142,9 +142,10 @@ pub fn run_binaryop_test_cases( macro_rules! test_arithmetic { ($field:ty) => { mod arithmetic { - use crate::field::field::Field; use std::ops::{Add, Mul, Neg, Sub}; + use crate::field::field::Field; + // Can be 32 or 64; doesn't have to be computer's actual word // bits. Choosing 32 gives more tests... const WORD_BITS: usize = 32; diff --git a/src/merkle_tree.rs b/src/merkle_tree.rs index db41b8a3..ed092cff 100644 --- a/src/merkle_tree.rs +++ b/src/merkle_tree.rs @@ -84,11 +84,10 @@ impl MerkleTree { mod tests { use anyhow::Result; + use super::*; use crate::field::crandall_field::CrandallField; use crate::merkle_proofs::verify_merkle_proof; - use super::*; - fn random_data(n: usize, k: usize) -> Vec> { (0..n).map(|_| F::rand_vec(k)).collect() } diff --git a/src/polynomial/commitment.rs b/src/polynomial/commitment.rs index 5169748b..355a56e4 100644 --- a/src/polynomial/commitment.rs +++ b/src/polynomial/commitment.rs @@ -340,9 +340,8 @@ mod tests { } mod quadratic { - use crate::field::crandall_field::CrandallField; - use super::*; + use crate::field::crandall_field::CrandallField; #[test] fn test_batch_polynomial_commitment() -> Result<()> { @@ -351,9 +350,8 @@ mod tests { } mod quartic { - use crate::field::crandall_field::CrandallField; - use super::*; + use crate::field::crandall_field::CrandallField; #[test] fn test_batch_polynomial_commitment() -> Result<()> { diff --git a/src/polynomial/polynomial.rs b/src/polynomial/polynomial.rs index 7c30ec02..9f605051 100644 --- a/src/polynomial/polynomial.rs +++ b/src/polynomial/polynomial.rs @@ -1,11 +1,11 @@ use std::cmp::max; +use std::iter::Sum; use std::ops::{Add, Mul, Sub}; use crate::field::extension_field::Extendable; use crate::field::fft::{fft, ifft}; use crate::field::field::Field; use crate::util::log2_strict; -use std::iter::Sum; /// A polynomial in point-value form. /// @@ -279,9 +279,8 @@ mod tests { use rand::{thread_rng, Rng}; - use crate::field::crandall_field::CrandallField; - use super::*; + use crate::field::crandall_field::CrandallField; #[test] fn test_trimmed() { diff --git a/src/proof.rs b/src/proof.rs index 12735b73..b1772c45 100644 --- a/src/proof.rs +++ b/src/proof.rs @@ -1,3 +1,5 @@ +use std::convert::TryInto; + use crate::field::extension_field::Extendable; use crate::field::field::Field; use crate::fri::FriConfig; @@ -5,7 +7,6 @@ use crate::merkle_proofs::{MerkleProof, MerkleProofTarget}; use crate::polynomial::commitment::{ListPolynomialCommitment, OpeningProof}; use crate::polynomial::polynomial::PolynomialCoeffs; use crate::target::Target; -use std::convert::TryInto; /// Represents a ~256 bit hash output. #[derive(Copy, Clone, Debug, Eq, PartialEq)] From 4ed03f4fb1bf547073e7b61801e592a251842551 Mon Sep 17 00:00:00 2001 From: wborgeaud Date: Fri, 11 Jun 2021 19:06:12 +0200 Subject: [PATCH 3/3] Fix `InterpolationGenerator` dependencies. --- src/gates/interpolation.rs | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/gates/interpolation.rs b/src/gates/interpolation.rs index 9bf76c2c..c17be3e0 100644 --- a/src/gates/interpolation.rs +++ b/src/gates/interpolation.rs @@ -209,11 +209,9 @@ impl, const D: usize> SimpleGenerator for InterpolationGener let mut deps = Vec::new(); deps.extend(local_targets(self.gate.wires_evaluation_point())); - deps.extend(local_targets(self.gate.wires_evaluation_value())); for i in 0..self.gate.num_points { deps.push(local_target(self.gate.wire_point(i))); deps.extend(local_targets(self.gate.wires_value(i))); - deps.extend(local_targets(self.gate.wires_coeff(i))); } deps }