From 1990bf54855c860fff2703a4387ca27b9768b6f1 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Thu, 1 Jun 2023 16:39:42 +0100 Subject: [PATCH 1/7] Capture zk_rows as a variable in the expression framework --- kimchi/src/circuits/expr.rs | 86 +++++++++++++++++++---- kimchi/src/circuits/lookup/constraints.rs | 18 +++-- kimchi/src/linearization.rs | 10 ++- kimchi/src/prover.rs | 1 - kimchi/src/prover_index.rs | 2 +- 5 files changed, 92 insertions(+), 25 deletions(-) diff --git a/kimchi/src/circuits/expr.rs b/kimchi/src/circuits/expr.rs index b62371ebde..92a78ef9a5 100644 --- a/kimchi/src/circuits/expr.rs +++ b/kimchi/src/circuits/expr.rs @@ -459,6 +459,12 @@ impl FeatureFlag { } } +#[derive(Copy, Clone, Debug, PartialEq, Eq, Serialize, Deserialize)] +pub struct RowOffset { + pub zk_rows: bool, + pub offset: i32, +} + /// An multi-variate polynomial over the base ring `C` with /// variables /// @@ -479,7 +485,7 @@ pub enum Expr { VanishesOnZeroKnowledgeAndPreviousRows, /// UnnormalizedLagrangeBasis(i) is /// (x^n - 1) / (x - omega^i) - UnnormalizedLagrangeBasis(i32), + UnnormalizedLagrangeBasis(RowOffset), Pow(Box>, u64), Cache(CacheId, Box>), /// If the feature flag is enabled, return the first expression; otherwise, return the second. @@ -649,7 +655,7 @@ pub enum PolishToken { Mul, Sub, VanishesOnZeroKnowledgeAndPreviousRows, - UnnormalizedLagrangeBasis(i32), + UnnormalizedLagrangeBasis(RowOffset), Store, Load(usize), /// Skip the given number of tokens if the feature is enabled. @@ -725,7 +731,12 @@ impl PolishToken { stack.push(eval_vanishes_on_last_n_rows(d, c.zk_rows + 1, pt)) } UnnormalizedLagrangeBasis(i) => { - stack.push(unnormalized_lagrange_basis(&d, *i, &pt)) + let offset = if i.zk_rows { + -(c.zk_rows as i32) + i.offset + } else { + i.offset + }; + stack.push(unnormalized_lagrange_basis(&d, offset, &pt)) } Literal(x) => stack.push(*x), Dup => stack.push(stack[stack.len() - 1]), @@ -1548,7 +1559,14 @@ impl Expr> { VanishesOnZeroKnowledgeAndPreviousRows => { Ok(eval_vanishes_on_last_n_rows(d, c.zk_rows + 1, pt)) } - UnnormalizedLagrangeBasis(i) => Ok(unnormalized_lagrange_basis(&d, *i, &pt)), + UnnormalizedLagrangeBasis(i) => { + let offset = if i.zk_rows { + -(c.zk_rows as i32) + i.offset + } else { + i.offset + }; + Ok(unnormalized_lagrange_basis(&d, offset, &pt)) + } Cell(v) => v.evaluate(evals), Cache(_, e) => e.evaluate_(d, pt, evals, c), IfFeature(feature, e1, e2) => { @@ -1610,7 +1628,14 @@ impl Expr { VanishesOnZeroKnowledgeAndPreviousRows => { Ok(eval_vanishes_on_last_n_rows(d, zk_rows + 1, pt)) } - UnnormalizedLagrangeBasis(i) => Ok(unnormalized_lagrange_basis(&d, *i, &pt)), + UnnormalizedLagrangeBasis(i) => { + let offset = if i.zk_rows { + -(zk_rows as i32) + i.offset + } else { + i.offset + }; + Ok(unnormalized_lagrange_basis(&d, offset, &pt)) + } Cell(v) => v.evaluate(evals), Cache(_, e) => e.evaluate(d, pt, zk_rows, evals), IfFeature(feature, e1, e2) => { @@ -1748,10 +1773,17 @@ impl Expr { evals: env.vanishes_on_zero_knowledge_and_previous_rows, }, Expr::Constant(x) => EvalResult::Constant(*x), - Expr::UnnormalizedLagrangeBasis(i) => EvalResult::Evals { - domain: d, - evals: unnormalized_lagrange_evals(env.l0_1, *i, d, env), - }, + Expr::UnnormalizedLagrangeBasis(i) => { + let offset = if i.zk_rows { + -(env.constants.zk_rows as i32) + i.offset + } else { + i.offset + }; + EvalResult::Evals { + domain: d, + evals: unnormalized_lagrange_evals(env.l0_1, offset, d, env), + } + } Expr::Cell(Variable { col, row }) => { let evals: &'a Evaluations> = { match env.get_column(col) { @@ -2442,7 +2474,9 @@ where Double(x) => format!("double({})", x.ocaml(cache)), Constant(x) => x.ocaml(), Cell(v) => format!("cell({})", v.ocaml()), - UnnormalizedLagrangeBasis(i) => format!("unnormalized_lagrange_basis({})", *i), + UnnormalizedLagrangeBasis(i) => { + format!("unnormalized_lagrange_basis({}, {})", i.zk_rows, i.offset) + } VanishesOnZeroKnowledgeAndPreviousRows => { "vanishes_on_zero_knowledge_and_previous_rows".to_string() } @@ -2493,7 +2527,18 @@ where Double(x) => format!("2 ({})", x.latex(cache)), Constant(x) => x.latex(), Cell(v) => v.latex(), - UnnormalizedLagrangeBasis(i) => format!("unnormalized\\_lagrange\\_basis({})", *i), + UnnormalizedLagrangeBasis(RowOffset { + zk_rows: true, + offset: i, + }) => { + format!("unnormalized\\_lagrange\\_basis(zk\\_rows + {})", *i) + } + UnnormalizedLagrangeBasis(RowOffset { + zk_rows: false, + offset: i, + }) => { + format!("unnormalized\\_lagrange\\_basis({})", *i) + } VanishesOnZeroKnowledgeAndPreviousRows => { "vanishes\\_on\\_zero\\_knowledge\\_and\\_previous\\_row".to_string() } @@ -2518,7 +2563,24 @@ where Double(x) => format!("double({})", x.text(cache)), Constant(x) => x.text(), Cell(v) => v.text(), - UnnormalizedLagrangeBasis(i) => format!("unnormalized_lagrange_basis({})", *i), + UnnormalizedLagrangeBasis(RowOffset { + zk_rows: true, + offset: i, + }) => { + if *i > 0 { + format!("unnormalized_lagrange_basis(zk_rows + {})", *i) + } else if *i == 0 { + format!("unnormalized_lagrange_basis(zk_rows)") + } else { + format!("unnormalized_lagrange_basis(zk_rows - {})", (-*i)) + } + } + UnnormalizedLagrangeBasis(RowOffset { + zk_rows: false, + offset: i, + }) => { + format!("unnormalized_lagrange_basis({})", *i) + } VanishesOnZeroKnowledgeAndPreviousRows => { "vanishes_on_zero_knowledge_and_previous_rows".to_string() } diff --git a/kimchi/src/circuits/lookup/constraints.rs b/kimchi/src/circuits/lookup/constraints.rs index 220149cd3b..9a5fc37eae 100644 --- a/kimchi/src/circuits/lookup/constraints.rs +++ b/kimchi/src/circuits/lookup/constraints.rs @@ -1,6 +1,6 @@ use crate::{ circuits::{ - expr::{prologue::*, Column, ConstantExpr}, + expr::{prologue::*, Column, ConstantExpr, RowOffset}, gate::{CircuitGate, CurrOrNext}, lookup::lookups::{ JointLookup, JointLookupSpec, JointLookupValue, LocalPosition, LookupInfo, @@ -371,7 +371,6 @@ impl LookupConfiguration { pub fn constraints( configuration: &LookupConfiguration, generate_feature_flags: bool, - zk_rows: usize, ) -> Vec> { // Something important to keep in mind is that the last 2 rows of // all columns will have random values in them to maintain zero-knowledge. @@ -601,14 +600,20 @@ pub fn constraints( let aggreg_equation = E::cell(Column::LookupAggreg, Next) * denominator - E::cell(Column::LookupAggreg, Curr) * numerator; - let final_lookup_row: i32 = -(zk_rows as i32) - 1; + let final_lookup_row = RowOffset { + zk_rows: true, + offset: -1, + }; let mut res = vec![ // the accumulator except for the last zk_rows+1 rows // (contains the zk-rows and the last value of the accumulator) E::VanishesOnZeroKnowledgeAndPreviousRows * aggreg_equation, // the initial value of the accumulator - E::UnnormalizedLagrangeBasis(0) * (E::cell(Column::LookupAggreg, Curr) - E::one()), + E::UnnormalizedLagrangeBasis(RowOffset { + zk_rows: false, + offset: 0, + }) * (E::cell(Column::LookupAggreg, Curr) - E::one()), // Check that the final value of the accumulator is 1 E::UnnormalizedLagrangeBasis(final_lookup_row) * (E::cell(Column::LookupAggreg, Curr) - E::one()), @@ -622,7 +627,10 @@ pub fn constraints( final_lookup_row } else { // Check compatibility of the first elements - 0 + RowOffset { + zk_rows: false, + offset: 0, + } }; let mut expr = E::UnnormalizedLagrangeBasis(first_or_last) * (column(Column::LookupSorted(i)) - column(Column::LookupSorted(i + 1))); diff --git a/kimchi/src/linearization.rs b/kimchi/src/linearization.rs index 3bfcb3081a..b71bfbb64f 100644 --- a/kimchi/src/linearization.rs +++ b/kimchi/src/linearization.rs @@ -38,7 +38,6 @@ use ark_ff::{FftField, PrimeField, SquareRootField, Zero}; pub fn constraints_expr( feature_flags: Option<&FeatureFlags>, generic: bool, - zk_rows: usize, ) -> (Expr>, Alphas) { // register powers of alpha so that we don't reuse them across mutually inclusive constraints let mut powers_of_alpha = Alphas::::default(); @@ -167,7 +166,7 @@ pub fn constraints_expr( let lookup_configuration = LookupConfiguration::new(LookupInfo::create(feature_flags.lookup_features)); let constraints = - lookup::constraints::constraints(&lookup_configuration, false, zk_rows); + lookup::constraints::constraints(&lookup_configuration, false); // note: the number of constraints depends on the lookup configuration, // specifically the presence of runtime tables. @@ -193,7 +192,7 @@ pub fn constraints_expr( joint_lookup_used: true, }; let lookup_configuration = LookupConfiguration::new(LookupInfo::create(all_features)); - let constraints = lookup::constraints::constraints(&lookup_configuration, true, zk_rows); + let constraints = lookup::constraints::constraints(&lookup_configuration, true); // note: the number of constraints depends on the lookup configuration, // specifically the presence of runtime tables. @@ -224,7 +223,7 @@ pub fn constraints_expr( // flags. if cfg!(feature = "check_feature_flags") { if let Some(feature_flags) = feature_flags { - let (feature_flagged_expr, _) = constraints_expr(None, generic, zk_rows); + let (feature_flagged_expr, _) = constraints_expr(None, generic); let feature_flagged_expr = feature_flagged_expr.apply_feature_flags(feature_flags); assert_eq!(expr, feature_flagged_expr); } @@ -321,11 +320,10 @@ pub fn linearization_columns( pub fn expr_linearization( feature_flags: Option<&FeatureFlags>, generic: bool, - zk_rows: usize, ) -> (Linearization>>, Alphas) { let evaluated_cols = linearization_columns::(feature_flags); - let (expr, powers_of_alpha) = constraints_expr(feature_flags, generic, zk_rows); + let (expr, powers_of_alpha) = constraints_expr(feature_flags, generic); let linearization = expr .linearize(evaluated_cols) diff --git a/kimchi/src/prover.rs b/kimchi/src/prover.rs index 42787f9991..b5ec8af5cc 100644 --- a/kimchi/src/prover.rs +++ b/kimchi/src/prover.rs @@ -806,7 +806,6 @@ where let constraints = lookup::constraints::constraints( &lcs.configuration, false, - index.cs.zk_rows as usize, ); let constraints_len = u32::try_from(constraints.len()) .expect("not expecting a large amount of constraints"); diff --git a/kimchi/src/prover_index.rs b/kimchi/src/prover_index.rs index f7e8b5ac6c..7183b1ecc6 100644 --- a/kimchi/src/prover_index.rs +++ b/kimchi/src/prover_index.rs @@ -66,7 +66,7 @@ impl ProverIndex { // pre-compute the linearization let (linearization, powers_of_alpha) = - expr_linearization(Some(&cs.feature_flags), true, cs.zk_rows as usize); + expr_linearization(Some(&cs.feature_flags), true); let evaluated_column_coefficients = cs.evaluated_column_coefficients(); From a85f2cd470ef4655e36acfab2d85ac80088e023b Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Tue, 6 Jun 2023 15:29:47 +0100 Subject: [PATCH 2/7] cargo fmt --- kimchi/src/linearization.rs | 3 +-- kimchi/src/prover.rs | 5 +---- kimchi/src/prover_index.rs | 3 +-- 3 files changed, 3 insertions(+), 8 deletions(-) diff --git a/kimchi/src/linearization.rs b/kimchi/src/linearization.rs index b71bfbb64f..ec42689059 100644 --- a/kimchi/src/linearization.rs +++ b/kimchi/src/linearization.rs @@ -165,8 +165,7 @@ pub fn constraints_expr( if feature_flags.lookup_features.patterns != LookupPatterns::default() { let lookup_configuration = LookupConfiguration::new(LookupInfo::create(feature_flags.lookup_features)); - let constraints = - lookup::constraints::constraints(&lookup_configuration, false); + let constraints = lookup::constraints::constraints(&lookup_configuration, false); // note: the number of constraints depends on the lookup configuration, // specifically the presence of runtime tables. diff --git a/kimchi/src/prover.rs b/kimchi/src/prover.rs index b5ec8af5cc..3978afb376 100644 --- a/kimchi/src/prover.rs +++ b/kimchi/src/prover.rs @@ -803,10 +803,7 @@ where // lookup { if let Some(lcs) = index.cs.lookup_constraint_system.as_ref() { - let constraints = lookup::constraints::constraints( - &lcs.configuration, - false, - ); + let constraints = lookup::constraints::constraints(&lcs.configuration, false); let constraints_len = u32::try_from(constraints.len()) .expect("not expecting a large amount of constraints"); let lookup_alphas = diff --git a/kimchi/src/prover_index.rs b/kimchi/src/prover_index.rs index 7183b1ecc6..faa4c8cc3b 100644 --- a/kimchi/src/prover_index.rs +++ b/kimchi/src/prover_index.rs @@ -65,8 +65,7 @@ impl ProverIndex { cs.endo = endo_q; // pre-compute the linearization - let (linearization, powers_of_alpha) = - expr_linearization(Some(&cs.feature_flags), true); + let (linearization, powers_of_alpha) = expr_linearization(Some(&cs.feature_flags), true); let evaluated_column_coefficients = cs.evaluated_column_coefficients(); From b1bc089560a049a1d447b6678436247c1857de09 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Tue, 10 Oct 2023 19:02:58 +0100 Subject: [PATCH 3/7] Clippy :') --- kimchi/src/circuits/expr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kimchi/src/circuits/expr.rs b/kimchi/src/circuits/expr.rs index 4da1339206..fb0e660695 100644 --- a/kimchi/src/circuits/expr.rs +++ b/kimchi/src/circuits/expr.rs @@ -2609,7 +2609,7 @@ where if *i > 0 { format!("unnormalized_lagrange_basis(zk_rows + {})", *i) } else if *i == 0 { - format!("unnormalized_lagrange_basis(zk_rows)") + "unnormalized_lagrange_basis(zk_rows)".to_string() } else { format!("unnormalized_lagrange_basis(zk_rows - {})", (-*i)) } From 6ec48aa1590bc6d6cfff9b4903cff6076a06387c Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Thu, 12 Oct 2023 18:06:45 +0100 Subject: [PATCH 4/7] More opinionated clippy changes --- kimchi/src/circuits/expr.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/kimchi/src/circuits/expr.rs b/kimchi/src/circuits/expr.rs index fb0e660695..4c742ac5b5 100644 --- a/kimchi/src/circuits/expr.rs +++ b/kimchi/src/circuits/expr.rs @@ -2605,15 +2605,11 @@ where UnnormalizedLagrangeBasis(RowOffset { zk_rows: true, offset: i, - }) => { - if *i > 0 { - format!("unnormalized_lagrange_basis(zk_rows + {})", *i) - } else if *i == 0 { - "unnormalized_lagrange_basis(zk_rows)".to_string() - } else { - format!("unnormalized_lagrange_basis(zk_rows - {})", (-*i)) - } - } + }) => match i.cmp(&0) { + Ordering::Greater => format!("unnormalized_lagrange_basis(zk_rows + {})", *i), + Ordering::Equal => "unnormalized_lagrange_basis(zk_rows)".to_string(), + Ordering::Less => format!("unnormalized_lagrange_basis(zk_rows - {})", (-*i)), + }, UnnormalizedLagrangeBasis(RowOffset { zk_rows: false, offset: i, From a2e577cd98ae4b7b54d0945f46e45a240b510426 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Thu, 12 Oct 2023 18:41:06 +0100 Subject: [PATCH 5/7] Fixup compilation after clippy's 'help' --- kimchi/src/circuits/expr.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/kimchi/src/circuits/expr.rs b/kimchi/src/circuits/expr.rs index 4c742ac5b5..f331d96b1b 100644 --- a/kimchi/src/circuits/expr.rs +++ b/kimchi/src/circuits/expr.rs @@ -21,11 +21,11 @@ use o1_utils::{foreign_field::ForeignFieldHelpers, FieldHelpers}; use rayon::prelude::*; use serde::{Deserialize, Serialize}; use std::ops::{Add, AddAssign, Mul, Neg, Sub}; +use std::{cmp::Ordering, fmt, iter::FromIterator}; use std::{ collections::{HashMap, HashSet}, ops::MulAssign, }; -use std::{fmt, iter::FromIterator}; use thiserror::Error; use CurrOrNext::{Curr, Next}; From fe78e3550ad1b92608b769d3a862008e346bdb92 Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Thu, 12 Oct 2023 20:38:29 +0100 Subject: [PATCH 6/7] Fixup test --- kimchi/src/alphas.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/kimchi/src/alphas.rs b/kimchi/src/alphas.rs index 2dbef2aaf3..27a35f7bd8 100644 --- a/kimchi/src/alphas.rs +++ b/kimchi/src/alphas.rs @@ -326,7 +326,6 @@ mod tests { let (_linearization, powers_of_alpha) = expr_linearization::( Some(&index.cs.feature_flags), true, - index.cs.zk_rows as usize, ); // make sure this is present in the specification let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap(); From 09ba6e26cb614adb437a30213d45f055ffeb58ce Mon Sep 17 00:00:00 2001 From: mrmr1993 Date: Sat, 14 Oct 2023 18:35:50 +0100 Subject: [PATCH 7/7] cargo fmt --- kimchi/src/alphas.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/kimchi/src/alphas.rs b/kimchi/src/alphas.rs index 27a35f7bd8..0eaa047114 100644 --- a/kimchi/src/alphas.rs +++ b/kimchi/src/alphas.rs @@ -323,10 +323,8 @@ mod tests { fn get_alphas_for_spec() { let gates = vec![CircuitGate::::zero(Wire::for_row(0)); 2]; let index = new_index_for_test::(gates, 0); - let (_linearization, powers_of_alpha) = expr_linearization::( - Some(&index.cs.feature_flags), - true, - ); + let (_linearization, powers_of_alpha) = + expr_linearization::(Some(&index.cs.feature_flags), true); // make sure this is present in the specification let manifest_dir = std::env::var("CARGO_MANIFEST_DIR").unwrap(); let spec_path = Path::new(&manifest_dir)