opt: reduce agg circuit phase (#1296)

lispc · noel2004 · roynalnaruto · web-flow · commit cbffb50b0a0b · 2024-05-28T21:44:00.000+08:00
* 1

* better log

* assert cs

* set max phase in aggregator to 1

Signed-off-by: noelwei &lt;fan@scroll.io&gt;

* do not init logger inside single test

Signed-off-by: noelwei &lt;fan@scroll.io&gt;

---------

Signed-off-by: noelwei &lt;fan@scroll.io&gt;
Co-authored-by: noelwei &lt;fan@scroll.io&gt;
Co-authored-by: Rohit Narurkar &lt;rohit.narurkar@proton.me&gt;
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/aggregator/Cargo.toml b/aggregator/Cargo.toml
@@ -11,6 +11,7 @@ gadgets = { path = "../gadgets" }
 zkevm-circuits = { path = "../zkevm-circuits" }
 
 ark-std.workspace = true
+ctor.workspace = true
 env_logger.workspace = true
 ethers-core.workspace = true
 hex.workspace = true
diff --git a/aggregator/src/aggregation/circuit.rs b/aggregator/src/aggregation/circuit.rs
@@ -137,7 +137,7 @@ impl<const N_SNARKS: usize> Circuit<Fr> for AggregationCircuit<N_SNARKS> {
             },
         );
 
-        let challenges = Challenges::construct(meta);
+        let challenges = Challenges::construct_p1(meta);
         let config = AggregationConfig::configure(meta, &params, challenges);
         log::info!(
             "aggregation circuit configured with k = {} and {:?} advice columns",
diff --git a/aggregator/src/aggregation/config.rs b/aggregator/src/aggregation/config.rs
@@ -190,3 +190,31 @@ impl<const N_SNARKS: usize> AggregationConfig<N_SNARKS> {
         EccChip::construct(self.base_field_config.clone())
     }
 }
+
+#[test]
+fn aggregation_circuit_degree() {
+    use halo2_ecc::fields::fp::FpStrategy;
+    let mut cs = ConstraintSystem::<Fr>::default();
+    let param = ConfigParams {
+        strategy: FpStrategy::Simple,
+        degree: 20,
+        num_advice: vec![59],
+        num_lookup_advice: vec![7],
+        num_fixed: 2,
+        lookup_bits: 18,
+        limb_bits: 88,
+        num_limbs: 3,
+    };
+    let challenges = Challenges::construct_p1(&mut cs);
+    AggregationConfig::<{ crate::constants::MAX_AGG_SNARKS }>::configure(
+        &mut cs, &param, challenges,
+    );
+    cs = cs.chunk_lookups();
+    let stats = zkevm_circuits::util::circuit_stats(&cs);
+    let degree = cs.degree();
+    let phases = cs.max_phase();
+    assert!(degree <= 9);
+    assert!(phases <= 1);
+    log::info!("stats {stats:#?}");
+    log::info!("agg circuit degree: {}", degree);
+}
diff --git a/aggregator/src/aggregation/decoder.rs b/aggregator/src/aggregation/decoder.rs
@@ -5461,7 +5461,7 @@ mod tests {
         }
 
         fn configure(meta: &mut ConstraintSystem<Fr>) -> Self::Config {
-            let challenges = Challenges::construct(meta);
+            let challenges = Challenges::construct_p1(meta);
             let challenges_expr = challenges.exprs(meta);
 
             let pow_rand_table = PowOfRandTable::construct(meta, &challenges_expr);
diff --git a/aggregator/src/aggregation/decoder/seq_exec.rs b/aggregator/src/aggregation/decoder/seq_exec.rs
@@ -1154,7 +1154,7 @@ mod tests {
 
             let inst_tbl = SeqInstTable::configure(meta);
 
-            let chng_mock = MockChallenges::construct(meta);
+            let chng_mock = MockChallenges::construct_p1(meta);
             let chng = chng_mock.exprs(meta);
 
             let config = SeqExecConfig::configure(
diff --git a/aggregator/src/tests/aggregation.rs b/aggregator/src/tests/aggregation.rs
@@ -13,8 +13,6 @@ use crate::{
 
 #[test]
 fn test_max_agg_snarks_aggregation_circuit() {
-    env_logger::init();
-
     let k = 20;
 
     // This set up requires one round of keccak for chunk's data hash
@@ -47,7 +45,7 @@ fn test_14_snark_aggregation_circuit() {
 #[ignore = "it takes too much time"]
 #[test]
 fn test_aggregation_circuit_all_possible_num_snarks() {
-    env_logger::init();
+    //env_logger::init();
 
     let k = 20;
 
@@ -66,7 +64,7 @@ fn test_aggregation_circuit_all_possible_num_snarks() {
 #[ignore = "it takes too much time"]
 #[test]
 fn test_aggregation_circuit_full() {
-    env_logger::init();
+    //env_logger::init();
     let process_id = process::id();
     let k = 25;
 
diff --git a/aggregator/src/tests/blob.rs b/aggregator/src/tests/blob.rs
@@ -58,7 +58,7 @@ impl Circuit<Fr> for BlobCircuit {
     fn configure(meta: &mut ConstraintSystem<Fr>) -> Self::Config {
         let u8_table = U8Table::construct(meta);
         let range_table = RangeTable::construct(meta);
-        let challenges = Challenges::construct(meta);
+        let challenges = Challenges::construct_p1(meta);
         let keccak_table = KeccakTable::construct(meta);
 
         let rlc = RlcConfig::configure(meta, &keccak_table, challenges);
diff --git a/aggregator/src/tests/compression.rs b/aggregator/src/tests/compression.rs
@@ -23,7 +23,7 @@ use crate::{
 #[ignore = "it takes too much time"]
 #[test]
 fn test_mock_compression() {
-    env_logger::init();
+    // env_logger::init();
 
     if std::path::Path::new("data").is_dir() {
         println!("data folder already exists\n");
diff --git a/aggregator/src/tests/mock_chunk.rs b/aggregator/src/tests/mock_chunk.rs
@@ -77,7 +77,7 @@ impl Circuit<Fr> for MockChunkCircuit {
     fn configure(meta: &mut ConstraintSystem<Fr>) -> Self::Config {
         meta.set_minimum_degree(4);
 
-        let challenges = Challenges::construct(meta);
+        let challenges = Challenges::construct_p1(meta);
         let keccak_table = KeccakTable::construct(meta);
         let rlc_config = RlcConfig::configure(meta, &keccak_table, challenges);
         let instance = meta.instance_column();
diff --git a/aggregator/src/tests/rlc/dynamic_hashes.rs b/aggregator/src/tests/rlc/dynamic_hashes.rs
@@ -40,7 +40,7 @@ impl Circuit<Fr> for DynamicHashCircuit {
     }
 
     fn configure(meta: &mut ConstraintSystem<Fr>) -> Self::Config {
-        let challenges = Challenges::construct(meta);
+        let challenges = Challenges::construct_p1(meta);
 
         // hash config
         // hash configuration for aggregation circuit
diff --git a/aggregator/src/tests/rlc/gates.rs b/aggregator/src/tests/rlc/gates.rs
@@ -30,7 +30,7 @@ impl Circuit<Fr> for ArithTestCircuit {
     }
 
     fn configure(meta: &mut ConstraintSystem<Fr>) -> Self::Config {
-        let challenges = Challenges::construct(meta);
+        let challenges = Challenges::construct_p1(meta);
         let keccak_table = KeccakTable::construct(meta);
         RlcConfig::configure(meta, &keccak_table, challenges)
     }
diff --git a/aggregator/src/util.rs b/aggregator/src/util.rs
@@ -1,6 +1,13 @@
 use eth_types::Field;
 use halo2_proofs::{circuit::AssignedCell, halo2curves::bn256::Fr, plonk::Error};
 
+#[cfg(test)]
+#[ctor::ctor]
+fn init_env_logger() {
+    // Enable RUST_LOG during tests
+    env_logger::Builder::from_env(env_logger::Env::default().default_filter_or("error")).init();
+}
+
 #[inline]
 // assert two cells have same value
 // (NOT constraining equality in circuit)
diff --git a/zkevm-circuits/src/util.rs b/zkevm-circuits/src/util.rs
@@ -48,50 +48,74 @@ pub(crate) fn rlc_be_bytes<F: Field>(bytes: &[u8], rand: Value<F>) -> Value<F> {
     })
 }
 
-/// All challenges used in `SuperCircuit`.
+/// Wrap multiple challenges:
+/// `construct`: the default consturct route to provide all challenges used in `SuperCircuit`.
+/// `construct_p1`: construct challenge up to second phase
 #[derive(Default, Clone, Copy, Debug)]
 pub struct Challenges<T = Challenge> {
     evm_word: T,
     keccak_input: T,
-    lookup_input: T,
+    lookup_input: Option<T>,
 }
 
 /// ..
 #[derive(Default, Clone, Copy, Debug)]
 pub struct MockChallenges {
     evm_word: u64,
     keccak_input: u64,
-    lookup_input: u64,
+    lookup_input: Option<u64>,
 }
 
 impl MockChallenges {
     /// ..
-    pub fn construct<F: Field>(_meta: &mut ConstraintSystem<F>) -> Self {
+    pub fn construct<F: Field>(meta: &mut ConstraintSystem<F>) -> Self {
+        Self {
+            lookup_input: Some(0x100),
+            ..Self::construct_p1(meta)
+        }
+    }
+    /// ..
+    pub fn construct_p1<F: Field>(_meta: &mut ConstraintSystem<F>) -> Self {
         Self {
             evm_word: 0x100,
             keccak_input: 0x101,
-            lookup_input: 0x100,
+            lookup_input: None,
         }
     }
     /// ..
     pub fn exprs<F: Field>(&self, _meta: &mut ConstraintSystem<F>) -> Challenges<Expression<F>> {
         Challenges {
             evm_word: Expression::Constant(F::from(self.evm_word)),
             keccak_input: Expression::Constant(F::from(self.keccak_input)),
-            lookup_input: Expression::Constant(F::from(self.lookup_input)),
+            lookup_input: self.lookup_input.map(|c| Expression::Constant(F::from(c))),
         }
     }
     /// ..
     pub fn values<F: Field>(&self, _layouter: &impl Layouter<F>) -> Challenges<Value<F>> {
         Challenges {
             evm_word: Value::known(F::from(self.evm_word)),
             keccak_input: Value::known(F::from(self.keccak_input)),
-            lookup_input: Value::known(F::from(self.lookup_input)),
+            lookup_input: self.lookup_input.map(|c| Value::known(F::from(c))),
         }
     }
 }
 
 impl Challenges {
+    /// Construct `Challenges` by allocating challenges only to secondary phases.
+    pub fn construct_p1<F: Field>(meta: &mut ConstraintSystem<F>) -> Self {
+        #[cfg(any(not(feature = "onephase"), feature = "test", test))]
+        let _dummy_cols = [
+            meta.advice_column(),
+            meta.advice_column_in(halo2_proofs::plonk::SecondPhase),
+        ];
+
+        Self {
+            evm_word: meta.challenge_usable_after(FirstPhase),
+            keccak_input: meta.challenge_usable_after(FirstPhase),
+            lookup_input: None,
+        }
+    }
+
     /// Construct `Challenges` by allocating challenges in specific phases.
     pub fn construct<F: Field>(meta: &mut ConstraintSystem<F>) -> Self {
         #[cfg(any(not(feature = "onephase"), feature = "test", test))]
@@ -104,16 +128,18 @@ impl Challenges {
         Self {
             evm_word: meta.challenge_usable_after(FirstPhase),
             keccak_input: meta.challenge_usable_after(FirstPhase),
-            lookup_input: meta.challenge_usable_after(SecondPhase),
+            lookup_input: Some(meta.challenge_usable_after(SecondPhase)),
         }
     }
 
     /// Returns `Expression` of challenges from `ConstraintSystem`.
     pub fn exprs<F: Field>(&self, meta: &mut ConstraintSystem<F>) -> Challenges<Expression<F>> {
-        let [evm_word, keccak_input, lookup_input] = query_expression(meta, |meta| {
-            [self.evm_word, self.keccak_input, self.lookup_input]
-                .map(|challenge| meta.query_challenge(challenge))
+        let [evm_word, keccak_input] = query_expression(meta, |meta| {
+            [self.evm_word, self.keccak_input].map(|challenge| meta.query_challenge(challenge))
         });
+        let lookup_input = self
+            .lookup_input
+            .map(|c| query_expression(meta, |meta| meta.query_challenge(c)));
         Challenges {
             evm_word,
             keccak_input,
@@ -126,7 +152,7 @@ impl Challenges {
         Challenges {
             evm_word: layouter.get_challenge(self.evm_word),
             keccak_input: layouter.get_challenge(self.keccak_input),
-            lookup_input: layouter.get_challenge(self.lookup_input),
+            lookup_input: self.lookup_input.map(|c| layouter.get_challenge(c)),
         }
     }
 }
@@ -144,20 +170,29 @@ impl<T: Clone> Challenges<T> {
 
     /// Returns challenge of `lookup_input`.
     pub fn lookup_input(&self) -> T {
-        self.lookup_input.clone()
+        self.lookup_input
+            .as_ref()
+            .expect("created for supercircuit")
+            .clone()
     }
 
     /// Returns the challenges indexed by the challenge index
     pub fn indexed(&self) -> [&T; 3] {
-        [&self.evm_word, &self.keccak_input, &self.lookup_input]
+        [
+            &self.evm_word,
+            &self.keccak_input,
+            self.lookup_input
+                .as_ref()
+                .expect("created for supercircuit"),
+        ]
     }
 
     /// ..
     pub fn mock(evm_word: T, keccak_input: T, lookup_input: T) -> Self {
         Self {
             evm_word,
             keccak_input,
-            lookup_input,
+            lookup_input: Some(lookup_input),
         }
     }
 }
@@ -186,7 +221,12 @@ impl<F: Field> Challenges<Expression<F>> {
 
     /// Returns powers of randomness for lookups
     pub fn lookup_input_powers_of_randomness<const S: usize>(&self) -> [Expression<F>; S] {
-        Self::powers_of(self.lookup_input.clone())
+        Self::powers_of(
+            self.lookup_input
+                .as_ref()
+                .expect("created for supercircuit")
+                .clone(),
+        )
     }
 }
 
@@ -272,8 +312,9 @@ pub(crate) fn get_push_size(byte: u8) -> u64 {
     }
 }
 
+/// Basic stats of circuit config
 #[derive(Debug)]
-pub(crate) struct CircuitStats {
+pub struct CircuitStats {
     num_constraints: usize,
     num_fixed_columns: usize,
     num_lookups: usize,
@@ -282,6 +323,7 @@ pub(crate) struct CircuitStats {
     num_selectors: usize,
     num_simple_selectors: usize,
     num_permutation_columns: usize,
+    num_vk_commitment: usize,
     degree: usize,
     blinding_factors: usize,
     num_challenges: usize,
@@ -292,7 +334,8 @@ pub(crate) struct CircuitStats {
     num_verification_ecmul: usize,
 }
 
-pub(crate) fn circuit_stats<F: Field>(meta: &ConstraintSystem<F>) -> CircuitStats {
+/// Basic stats of circuit config
+pub fn circuit_stats<F: Field>(meta: &ConstraintSystem<F>) -> CircuitStats {
     let rotations = meta
         .advice_queries
         .iter()
@@ -311,6 +354,9 @@ pub(crate) fn circuit_stats<F: Field>(meta: &ConstraintSystem<F>) -> CircuitStat
         num_selectors: meta.num_selectors,
         num_simple_selectors: meta.num_simple_selectors,
         num_permutation_columns: meta.permutation.columns.len(),
+        num_vk_commitment: meta.num_fixed_columns
+            + meta.num_selectors
+            + meta.permutation.columns.len(),
         degree: meta.degree(),
         blinding_factors: meta.blinding_factors(),
         num_challenges: meta.num_challenges(),

Original file line number	Diff line number	Diff line change
`@@ -5461,7 +5461,7 @@ mod tests {`
`5461`	`5461`	`}`
`5462`	`5462`
`5463`	`5463`	`fn configure(meta: &mut ConstraintSystem<Fr>) -> Self::Config {`
`5464`		`- let challenges = Challenges::construct(meta);`
	`5464`	`+ let challenges = Challenges::construct_p1(meta);`
`5465`	`5465`	`let challenges_expr = challenges.exprs(meta);`
`5466`	`5466`
`5467`	`5467`	`let pow_rand_table = PowOfRandTable::construct(meta, &challenges_expr);`