address comment on fixed cells

Author: zhenfeizhang

aggregator/src/aggregation/circuit.rs

@@ -154,7 +154,7 @@ impl Circuit<Fr> for AggregationCircuit {
         config: Self::Config,
         mut layouter: impl Layouter<Fr>,
     ) -> Result<(), Error> {
-        let (config, challenge) = config;
+        let (mut config, challenge) = config;
 
         let witness_time = start_timer!(|| "synthesize | Aggregation Circuit");
 
@@ -266,7 +266,7 @@ impl Circuit<Fr> for AggregationCircuit {
 
             let timer = start_timer!(|| ("assign hash cells").to_string());
             let (hash_digest_cells, num_valid_snarks) = assign_batch_hashes(
-                &config,
+                &mut config,
                 &mut layouter,
                 challenges,
                 &preimages,

aggregator/src/aggregation/rlc/config.rs

@@ -1,4 +1,5 @@
 use halo2_proofs::{
+    circuit::AssignedCell,
     halo2curves::bn256::Fr,
     plonk::{Advice, Column, ConstraintSystem, Fixed, SecondPhase, Selector},
     poly::Rotation,
@@ -10,7 +11,7 @@ use zkevm_circuits::util::Challenges;
 
 /// This config is used to compute RLCs for bytes.
 /// It requires a phase 2 column
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone)]
 pub struct RlcConfig {
     #[cfg(test)]
     // Test requires a phase 1 column before proceed to phase 2.
@@ -19,6 +20,8 @@ pub struct RlcConfig {
     pub(crate) selector: Selector,
     pub(crate) fixed: Column<Fixed>,
     pub(crate) enable_challenge: Selector,
+    // fixed cells storing 0, 1, 2, 4, 8, 32
+    pub(crate) fixed_cells: Vec<AssignedCell<Fr, Fr>>,
 }
 
 impl RlcConfig {
@@ -73,6 +76,7 @@ impl RlcConfig {
             selector,
             fixed,
             enable_challenge,
+            fixed_cells: vec![],
         }
     }
 }

aggregator/src/aggregation/rlc/gates.rs

@@ -1,5 +1,5 @@
 use halo2_proofs::{
-    circuit::{AssignedCell, Cell, Region, RegionIndex, Value},
+    circuit::{AssignedCell, Region, Value},
     halo2curves::bn256::Fr,
     plonk::Error,
 };
@@ -10,81 +10,47 @@ use crate::{constants::LOG_DEGREE, util::assert_equal};
 use super::RlcConfig;
 
 impl RlcConfig {
-    /// initialize the chip with fixed 0 and 1 cells
-    pub(crate) fn init(&self, region: &mut Region<Fr>) -> Result<(), Error> {
-        region.assign_fixed(|| "const zero", self.fixed, 0, || Value::known(Fr::zero()))?;
-        region.assign_fixed(|| "const one", self.fixed, 1, || Value::known(Fr::one()))?;
-        region.assign_fixed(|| "const two", self.fixed, 2, || Value::known(Fr::from(2)))?;
-        region.assign_fixed(|| "const four", self.fixed, 3, || Value::known(Fr::from(4)))?;
-        region.assign_fixed(
+    /// initialize the chip with fixed values storing 0, 1, 2, 4, 8, 32
+    pub(crate) fn init(&mut self, region: &mut Region<Fr>) -> Result<(), Error> {
+        self.fixed_cells.push(region.assign_fixed(
+            || "const zero",
+            self.fixed,
+            0,
+            || Value::known(Fr::zero()),
+        )?);
+        self.fixed_cells.push(region.assign_fixed(
+            || "const one",
+            self.fixed,
+            1,
+            || Value::known(Fr::one()),
+        )?);
+        self.fixed_cells.push(region.assign_fixed(
+            || "const two",
+            self.fixed,
+            2,
+            || Value::known(Fr::from(2)),
+        )?);
+        self.fixed_cells.push(region.assign_fixed(
+            || "const four",
+            self.fixed,
+            3,
+            || Value::known(Fr::from(4)),
+        )?);
+        self.fixed_cells.push(region.assign_fixed(
             || "const eight",
             self.fixed,
             4,
             || Value::known(Fr::from(8)),
-        )?;
-        region.assign_fixed(
+        )?);
+        self.fixed_cells.push(region.assign_fixed(
             || "const thirty two",
             self.fixed,
             5,
             || Value::known(Fr::from(32)),
-        )?;
+        )?);
         Ok(())
     }
 
-    #[inline]
-    pub(crate) fn zero_cell(&self, region_index: RegionIndex) -> Cell {
-        Cell {
-            region_index,
-            row_offset: 0,
-            column: self.fixed.into(),
-        }
-    }
-
-    #[inline]
-    pub(crate) fn one_cell(&self, region_index: RegionIndex) -> Cell {
-        Cell {
-            region_index,
-            row_offset: 1,
-            column: self.fixed.into(),
-        }
-    }
-
-    #[inline]
-    pub(crate) fn two_cell(&self, region_index: RegionIndex) -> Cell {
-        Cell {
-            region_index,
-            row_offset: 2,
-            column: self.fixed.into(),
-        }
-    }
-
-    #[inline]
-    pub(crate) fn four_cell(&self, region_index: RegionIndex) -> Cell {
-        Cell {
-            region_index,
-            row_offset: 3,
-            column: self.fixed.into(),
-        }
-    }
-
-    #[inline]
-    pub(crate) fn eight_cell(&self, region_index: RegionIndex) -> Cell {
-        Cell {
-            region_index,
-            row_offset: 4,
-            column: self.fixed.into(),
-        }
-    }
-
-    #[inline]
-    pub(crate) fn thirty_two_cell(&self, region_index: RegionIndex) -> Cell {
-        Cell {
-            region_index,
-            row_offset: 5,
-            column: self.fixed.into(),
-        }
-    }
-
     pub(crate) fn load_private(
         &self,
         region: &mut Region<Fr>,
@@ -125,8 +91,8 @@ impl RlcConfig {
         region: &mut Region<Fr>,
         f: &AssignedCell<Fr, Fr>,
     ) -> Result<(), Error> {
-        let zero_cell = self.zero_cell(f.cell().region_index);
-        region.constrain_equal(f.cell(), zero_cell)
+        let zero_cell = &self.fixed_cells[0];
+        region.constrain_equal(f.cell(), zero_cell.cell())
     }
 
     /// Enforce the element in f is a binary element.
@@ -150,16 +116,10 @@ impl RlcConfig {
         offset: &mut usize,
     ) -> Result<AssignedCell<Fr, Fr>, Error> {
         self.selector.enable(region, *offset)?;
-        let one_cell = self.one_cell(a.cell().region_index);
+        let one_cell = &self.fixed_cells[1];
 
         a.copy_advice(|| "a", region, self.phase_2_column, *offset)?;
-        let one = region.assign_advice(
-            || "c",
-            self.phase_2_column,
-            *offset + 1,
-            || Value::known(Fr::one()),
-        )?;
-        region.constrain_equal(one.cell(), one_cell)?;
+        one_cell.copy_advice(|| "b", region, self.phase_2_column, *offset + 1)?;
         b.copy_advice(|| "c", region, self.phase_2_column, *offset + 2)?;
         let d = region.assign_advice(
             || "d",
@@ -181,21 +141,15 @@ impl RlcConfig {
         offset: &mut usize,
     ) -> Result<AssignedCell<Fr, Fr>, Error> {
         self.selector.enable(region, *offset)?;
-        let one_cell = self.one_cell(a.cell().region_index);
+        let one_cell = &self.fixed_cells[1];
 
         let res = region.assign_advice(
             || "a",
             self.phase_2_column,
             *offset,
             || a.value() - b.value(),
         )?;
-        let one = region.assign_advice(
-            || "b",
-            self.phase_2_column,
-            *offset + 1,
-            || Value::known(Fr::one()),
-        )?;
-        region.constrain_equal(one.cell(), one_cell)?;
+        one_cell.copy_advice(|| "b", region, self.phase_2_column, *offset + 1)?;
         b.copy_advice(|| "c", region, self.phase_2_column, *offset + 2)?;
         a.copy_advice(|| "d", region, self.phase_2_column, *offset + 3)?;
         *offset += 4;
@@ -212,17 +166,11 @@ impl RlcConfig {
         offset: &mut usize,
     ) -> Result<AssignedCell<Fr, Fr>, Error> {
         self.selector.enable(region, *offset)?;
-        let zero_cell = self.zero_cell(a.cell().region_index);
+        let zero_cell = &self.fixed_cells[0];
 
         a.copy_advice(|| "a", region, self.phase_2_column, *offset)?;
         b.copy_advice(|| "b", region, self.phase_2_column, *offset + 1)?;
-        let zero = region.assign_advice(
-            || "b",
-            self.phase_2_column,
-            *offset + 2,
-            || Value::known(Fr::zero()),
-        )?;
-        region.constrain_equal(zero.cell(), zero_cell)?;
+        zero_cell.copy_advice(|| "c", region, self.phase_2_column, *offset + 2)?;
         let d = region.assign_advice(
             || "d",
             self.phase_2_column,
@@ -267,10 +215,8 @@ impl RlcConfig {
         a: &AssignedCell<Fr, Fr>,
         offset: &mut usize,
     ) -> Result<AssignedCell<Fr, Fr>, Error> {
-        let one_cell = self.one_cell(a.cell().region_index);
-        let one = self.load_private(region, &Fr::one(), offset)?;
-        region.constrain_equal(one_cell, one.cell())?;
-        self.sub(region, &one, a, offset)
+        let one_cell = &self.fixed_cells[1];
+        self.sub(region, one_cell, a, offset)
     }
 
     // if cond = 1 return a, else b
@@ -372,22 +318,10 @@ impl RlcConfig {
             })
             .collect::<Result<Vec<_>, Error>>()?;
 
-        let mut acc = {
-            let zero = self.load_private(region, &Fr::from(0), offset)?;
-            let zero_cell = self.zero_cell(zero.cell().region_index);
-            region.constrain_equal(zero_cell, zero.cell())?;
-            zero
-        };
-
-        let two = {
-            let two = self.load_private(region, &Fr::from(2), offset)?;
-            let two_cell = self.two_cell(two.cell().region_index);
-            region.constrain_equal(two_cell, two.cell())?;
-            two
-        };
-
+        let mut acc = self.fixed_cells[0].clone();
+        let two = &self.fixed_cells[2];
         for bit in bit_cells.iter().rev() {
-            acc = self.mul_add(region, &acc, &two, bit, offset)?;
+            acc = self.mul_add(region, &acc, two, bit, offset)?;
         }
 
         // sanity check

aggregator/src/core.rs

@@ -103,7 +103,7 @@ pub(crate) fn extract_accumulators_and_proof(
 // 7. chunk[i]'s data_hash == "" when chunk[i] is padded
 #[allow(clippy::type_complexity)]
 pub(crate) fn assign_batch_hashes(
-    config: &AggregationConfig,
+    config: &mut AggregationConfig,
     layouter: &mut impl Layouter<Fr>,
     challenges: Challenges<Value<Fr>>,
     preimages: &[Vec<u8>],
@@ -129,7 +129,7 @@ pub(crate) fn assign_batch_hashes(
     // 6. chunk[i]'s prev_state_root == post_state_root when chunk[i] is padded
     // 7. chunk[i]'s data_hash == "" when chunk[i] is padded
     let num_valid_snarks = conditional_constraints(
-        &config.rlc_config,
+        &mut config.rlc_config,
         // config.flex_gate(),
         layouter,
         challenges,
@@ -408,7 +408,7 @@ fn copy_constraints(
 // 7. chunk[i]'s data_hash == "" when chunk[i] is padded
 #[allow(clippy::too_many_arguments)]
 pub(crate) fn conditional_constraints(
-    rlc_config: &RlcConfig,
+    rlc_config: &mut RlcConfig,
     layouter: &mut impl Layouter<Fr>,
     challenges: Challenges<Value<Fr>>,
     hash_input_cells: &[AssignedCell<Fr, Fr>],
@@ -456,29 +456,20 @@ pub(crate) fn conditional_constraints(
                 // 5,6,7,8       | 32                  | 0, 1, 0
                 // 9,10          | 64                  | 0, 0, 1
 
-                let four = {
-                    let four = rlc_config.load_private(&mut region, &Fr::from(4), &mut offset)?;
-                    let four_cell = rlc_config.four_cell(four.cell().region_index);
-                    region.constrain_equal(four_cell, four.cell())?;
-                    four
-                };
-                let eight = {
-                    let eight = rlc_config.load_private(&mut region, &Fr::from(8), &mut offset)?;
-                    let eight_cell = rlc_config.eight_cell(eight.cell().region_index);
-                    region.constrain_equal(eight_cell, eight.cell())?;
-                    eight
-                };
+                let four = &rlc_config.fixed_cells[3];
+                let eight = &rlc_config.fixed_cells[4];
+
                 let flag1 = rlc_config.is_smaller_than(
                     &mut region,
                     &num_of_valid_snarks_cell[0],
-                    &four,
+                    four,
                     &mut offset,
                 )?;
                 let not_flag1 = rlc_config.not(&mut region, &flag1, &mut offset)?;
                 let not_flag3 = rlc_config.is_smaller_than(
                     &mut region,
                     &num_of_valid_snarks_cell[0],
-                    &eight,
+                    eight,
                     &mut offset,
                 )?;
                 let flag3 = rlc_config.not(&mut region, &not_flag3, &mut offset)?;
@@ -660,13 +651,11 @@ pub(crate) fn conditional_constraints(
 
                 // 7. chunk[i]'s data_hash == "" when chunk[i] is padded
                 // that means the data_hash length is 32 * number_of_valid_snarks
-                let const32 = rlc_config.load_private(&mut region, &Fr::from(32), &mut offset)?;
-                let const32_cell = rlc_config.thirty_two_cell(const32.cell().region_index);
-                region.constrain_equal(const32.cell(), const32_cell)?;
+                let const32 = &rlc_config.fixed_cells[5];
                 let data_hash_inputs = rlc_config.mul(
                     &mut region,
                     &num_of_valid_snarks_cell[0],
-                    &const32,
+                    const32,
                     &mut offset,
                 )?;
 

aggregator/src/tests/mock_chunk.rs

@@ -18,7 +18,7 @@ use crate::{
 
 /// This config is used to compute RLCs for bytes.
 /// It requires a phase 2 column
-#[derive(Debug, Clone, Copy)]
+#[derive(Debug, Clone)]
 pub struct MockConfig {
     pub(crate) rlc_config: RlcConfig,
     /// Instance for public input; stores

aggregator/src/tests/rlc/dynamic_hashes.rs

@@ -77,7 +77,7 @@ impl Circuit<Fr> for DynamicHashCircuit {
         config: Self::Config,
         mut layouter: impl Layouter<Fr>,
     ) -> Result<(), Error> {
-        let (config, challenges) = config;
+        let (mut config, challenges) = config;
 
         config
             .keccak_circuit_config

aggregator/src/tests/rlc/gates.rs

@@ -38,7 +38,7 @@ impl Circuit<Fr> for ArithTestCircuit {
 
     fn synthesize(
         &self,
-        config: Self::Config,
+        mut config: Self::Config,
         mut layouter: impl Layouter<Fr>,
     ) -> Result<(), Error> {
         let mut rng = test_rng();