Fix bugs in integration test (#1160)

* split blob data region into two separate regions

* fix

* load tables for BlobDataConfig

* fix wrong rlc in blob data rows

* fix challenge z lookup

* keccak lookup for metadata row and minor fixes (#1162)

* fmt

* fix challenge z

* rlc keccak vs evm (#1163)

* put barycentric eval and snark aggregation in same region

* fix copy constraints (limb construction and endianness) (#1164)

* fix copy constraints (limb construction and endianness)

* fix typo

---------

Co-authored-by: Mason Liang <mason@scroll.io>

* cleanup

---------

Co-authored-by: Rohit Narurkar <rohit.narurkar@proton.me>
Co-authored-by: Mason Liang <mason@scroll.io>

Author: 3 people

aggregator/src/aggregation/blob_data.rs

@@ -9,6 +9,8 @@ use halo2_proofs::{
 };
 use itertools::Itertools;
 use rand::Rng;
+#[cfg(not(feature = "disable_proof_aggregation"))]
+use std::rc::Rc;
 use std::{env, fs::File};
 
 #[cfg(not(feature = "disable_proof_aggregation"))]
@@ -156,24 +158,59 @@ impl Circuit<Fr> for AggregationCircuit {
 
         let timer = start_timer!(|| "aggregation");
 
+        // load lookup table in range config
+        config
+            .range()
+            .load_lookup_table(&mut layouter)
+            .expect("load range lookup table");
         // ==============================================
         // Step 1: snark aggregation circuit
         // ==============================================
-        #[cfg(not(feature = "disable_proof_aggregation"))]
-        let (accumulator_instances, snark_inputs) = {
-            config
-                .range()
-                .load_lookup_table(&mut layouter)
-                .expect("load range lookup table");
+        #[cfg(feature = "disable_proof_aggregation")]
+        let barycentric = {
+            let mut first_pass = halo2_base::SKIP_FIRST_PASS;
+            layouter.assign_region(
+                || "barycentric evaluation",
+                |region| {
+                    if first_pass {
+                        first_pass = false;
+                        return Ok(BarycentricEvaluationCells::default());
+                    }
+
+                    let mut ctx = Context::new(
+                        region,
+                        ContextParams {
+                            max_rows: config.flex_gate().max_rows,
+                            num_context_ids: 1,
+                            fixed_columns: config.flex_gate().constants.clone(),
+                        },
+                    );
+
+                    let barycentric = config.barycentric.assign2(
+                        &mut ctx,
+                        self.batch_hash.blob.coefficients,
+                        self.batch_hash.blob.challenge_digest,
+                        self.batch_hash.blob.evaluation,
+                    );
+
+                    config.barycentric.scalar.range.finalize(&mut ctx);
+                    ctx.print_stats(&["barycentric evaluation"]);
+
+                    Ok(barycentric)
+                },
+            )?
+        };
 
+        #[cfg(not(feature = "disable_proof_aggregation"))]
+        let (accumulator_instances, snark_inputs, barycentric) = {
             let mut first_pass = halo2_base::SKIP_FIRST_PASS;
 
-            let (accumulator_instances, snark_inputs) = layouter.assign_region(
+            let (accumulator_instances, snark_inputs, barycentric) = layouter.assign_region(
                 || "aggregation",
-                |region| -> Result<(Vec<AssignedValue<Fr>>, Vec<AssignedValue<Fr>>), Error> {
+                |region| {
                     if first_pass {
                         first_pass = false;
-                        return Ok((vec![], vec![]));
+                        return Ok((vec![], vec![], BarycentricEvaluationCells::default()));
                     }
 
                     // stores accumulators for all snarks, including the padded ones
@@ -222,16 +259,26 @@ impl Circuit<Fr> for AggregationCircuit {
                             .flat_map(|instance_column| instance_column.iter().skip(ACC_LEN)),
                     );
 
-                    config.range().finalize(&mut loader.ctx_mut());
+                    loader.ctx_mut().print_stats(&["snark aggregation"]);
+
+                    let mut ctx = Rc::into_inner(loader).unwrap().into_ctx();
+                    let barycentric = config.barycentric.assign2(
+                        &mut ctx,
+                        self.batch_hash.blob.coefficients,
+                        self.batch_hash.blob.challenge_digest,
+                        self.batch_hash.blob.evaluation,
+                    );
 
-                    loader.ctx_mut().print_stats(&["Range"]);
+                    ctx.print_stats(&["barycentric"]);
 
-                    Ok((accumulator_instances, snark_inputs))
+                    config.range().finalize(&mut ctx);
+
+                    Ok((accumulator_instances, snark_inputs, barycentric))
                 },
             )?;
 
             assert_eq!(snark_inputs.len(), MAX_AGG_SNARKS * DIGEST_LEN);
-            (accumulator_instances, snark_inputs)
+            (accumulator_instances, snark_inputs, barycentric)
         };
         end_timer!(timer);
         // ==============================================
@@ -242,7 +289,7 @@ impl Circuit<Fr> for AggregationCircuit {
 
         let timer = start_timer!(|| "load aux table");
 
-        let (hash_digest_cells, expected_blob_cells, rlc_config_offset) = {
+        let (hash_digest_cells, expected_blob_cells) = {
             config
                 .keccak_circuit_config
                 .load_aux_tables(&mut layouter)?;
@@ -281,11 +328,7 @@ impl Circuit<Fr> for AggregationCircuit {
             .map_err(|_e| Error::ConstraintSystemFailure)?;
 
             end_timer!(timer);
-            (
-                assigned_batch_hash.hash_output,
-                assigned_batch_hash.blob,
-                assigned_batch_hash.rlc_config_offset,
-            )
+            (assigned_batch_hash.hash_output, assigned_batch_hash.blob)
         };
         // digests
         let (batch_pi_hash_digest, chunk_pi_hash_digests, _potential_batch_data_hash_digest) =
@@ -382,40 +425,13 @@ impl Circuit<Fr> for AggregationCircuit {
 
         // blob data config
         {
-            let mut is_first_pass = true;
-            let be = layouter.assign_region(
-                || "blob data and barycentric evaluation",
-                |region| {
-                    if is_first_pass {
-                        is_first_pass = false;
-                        return Ok(BarycentricEvaluationCells::default());
-                    }
-
-                    let mut ctx = Context::new(
-                        region,
-                        ContextParams {
-                            max_rows: config.flex_gate().max_rows,
-                            num_context_ids: 1,
-                            fixed_columns: config.flex_gate().constants.clone(),
-                        },
-                    );
-
-                    Ok(config.barycentric.assign2(
-                        &mut ctx,
-                        self.batch_hash.blob.coefficients,
-                        self.batch_hash.blob.challenge_digest,
-                        self.batch_hash.blob.evaluation,
-                    ))
-                },
-            )?;
-            let barycentric_assignments = &be.barycentric_assignments;
-            let challenge_le = &be.z_le;
-            let evaluation_le = &be.y_le;
+            let barycentric_assignments = &barycentric.barycentric_assignments;
+            let challenge_le = &barycentric.z_le;
+            let evaluation_le = &barycentric.y_le;
 
             let blob_data_exports = config.blob_data_config.assign(
                 &mut layouter,
                 challenges,
-                rlc_config_offset,
                 &config.rlc_config,
                 &self.batch_hash,
                 barycentric_assignments,
@@ -481,14 +497,15 @@ impl CircuitExt<Fr> for AggregationCircuit {
 
     fn selectors(config: &Self::Config) -> Vec<Selector> {
         // - advice columns from flex gate
-        // - selector from RLC gate
+        // - selectors from RLC gate
         config.0.flex_gate().basic_gates[0]
             .iter()
             .map(|gate| gate.q_enable)
             .chain(
                 [
                     config.0.rlc_config.selector,
-                    config.0.rlc_config.enable_challenge,
+                    config.0.rlc_config.enable_challenge1,
+                    config.0.rlc_config.enable_challenge2,
                 ]
                 .iter()
                 .cloned(),

aggregator/src/aggregation/circuit.rs

@@ -6,7 +6,7 @@ use halo2_proofs::{
 
 #[cfg(test)]
 use halo2_proofs::plonk::FirstPhase;
-use zkevm_circuits::util::Challenges;
+use zkevm_circuits::util::{Challenges, Expr};
 
 /// This config is used to compute RLCs for bytes.
 /// It requires a phase 2 column
@@ -18,13 +18,15 @@ pub struct RlcConfig {
     pub(crate) phase_2_column: Column<Advice>,
     pub(crate) selector: Selector,
     pub(crate) fixed: Column<Fixed>,
-    pub(crate) enable_challenge: Selector,
+    pub(crate) enable_challenge1: Selector,
+    pub(crate) enable_challenge2: Selector,
 }
 
 impl RlcConfig {
     pub(crate) fn configure(meta: &mut ConstraintSystem<Fr>, challenge: Challenges) -> Self {
         let selector = meta.complex_selector();
-        let enable_challenge = meta.complex_selector();
+        let enable_challenge1 = meta.complex_selector();
+        let enable_challenge2 = meta.complex_selector();
         let challenge_expr = challenge.exprs(meta);
 
         #[cfg(test)]
@@ -61,18 +63,22 @@ impl RlcConfig {
             // constraint: q2*(a-challenge) = 0
             // FIXME later: Pretty wasteful to have a dedicated custom gate and selector column just
             // to extract the keccak challenge cell...
-            let q2 = meta.query_selector(enable_challenge);
-            let cs2 = q2 * (a - challenge_expr.keccak_input());
+            let q2 = meta.query_selector(enable_challenge1);
+            let cs2 = q2 * (a.expr() - challenge_expr.keccak_input());
 
-            vec![cs1, cs2]
+            let q3 = meta.query_selector(enable_challenge2);
+            let cs3 = q3 * (a - challenge_expr.evm_word());
+
+            vec![cs1, cs2, cs3]
         });
         Self {
             #[cfg(test)]
             _phase_1_column,
             phase_2_column,
             selector,
             fixed,
-            enable_challenge,
+            enable_challenge1,
+            enable_challenge2,
         }
     }
 }

aggregator/src/aggregation/rlc/config.rs

@@ -197,20 +197,38 @@ impl RlcConfig {
         res
     }
 
-    pub(crate) fn read_challenge(
+    pub(crate) fn read_challenge1(
         &self,
         region: &mut Region<Fr>,
         challenge_value: Challenges<Value<Fr>>,
         offset: &mut usize,
     ) -> Result<AssignedCell<Fr, Fr>, Error> {
         let challenge_value = challenge_value.keccak_input();
         let challenge_cell = region.assign_advice(
-            || "assign challenge",
+            || "assign challenge1",
             self.phase_2_column,
             *offset,
             || challenge_value,
         )?;
-        self.enable_challenge.enable(region, *offset)?;
+        self.enable_challenge1.enable(region, *offset)?;
+        *offset += 1;
+        Ok(challenge_cell)
+    }
+
+    pub(crate) fn read_challenge2(
+        &self,
+        region: &mut Region<Fr>,
+        challenge_value: Challenges<Value<Fr>>,
+        offset: &mut usize,
+    ) -> Result<AssignedCell<Fr, Fr>, Error> {
+        let challenge_value = challenge_value.evm_word();
+        let challenge_cell = region.assign_advice(
+            || "assign challenge2",
+            self.phase_2_column,
+            *offset,
+            || challenge_value,
+        )?;
+        self.enable_challenge2.enable(region, *offset)?;
         *offset += 1;
         Ok(challenge_cell)
     }
@@ -250,7 +268,7 @@ impl RlcConfig {
 
         a.copy_advice(|| "a", region, self.phase_2_column, *offset)?;
         let one = region.assign_advice(
-            || "c",
+            || "b",
             self.phase_2_column,
             *offset + 1,
             || Value::known(Fr::one()),