[feat] testool: report err when ccc overflows (#1135)

* run check * remove print * fix dynamic calldata
scroll-tech · Mar 13, 2024 · 888824a · 888824a
1 parent 6d47ece
commit 888824a
Show file tree

Hide file tree

Showing 3 changed files with 64 additions and 53 deletions.
diff --git a/testool/src/statetest/executor.rs b/testool/src/statetest/executor.rs
@@ -57,6 +57,8 @@ pub enum StateTestError {
     SkipTestBalanceOverflow,
     #[error("Exception(expected:{expected:?}, found:{found:?})")]
     Exception { expected: bool, found: String },
+    #[error("CircuitOverflow(circuit:{circuit:?}, needed:{needed:?})")]
+    CircuitOverflow { circuit: String, needed: usize },
 }
 
 impl StateTestError {
@@ -435,6 +437,7 @@ pub const MAX_RLP_ROWS: usize = 800_000;
 pub const MAX_BYTECODE: usize = 600_000;
 pub const MAX_MPT_ROWS: usize = 1_000_000;
 pub const MAX_KECCAK_ROWS: usize = 1_000_000;
+pub const MAX_SHA256_ROWS: usize = 1_000_000;
 pub const MAX_POSEIDON_ROWS: usize = 1_000_000;
 pub const MAX_VERTICAL_ROWS: usize = 1_000_000;
 pub const MAX_RWS: usize = 1_000_000;
@@ -443,13 +446,15 @@ pub const MAX_PRECOMPILE_EC_MUL: usize = 50;
 pub const MAX_PRECOMPILE_EC_PAIRING: usize = 2;
 
 // TODO: refactor & usage
-fn get_sub_circuit_limit_l2() -> Vec<usize> {
-    vec![
+fn get_sub_circuit_limit() -> Vec<usize> {
+    #[allow(unused_mut)]
+    let mut limit = vec![
         MAX_RWS,           // evm
         MAX_RWS,           // state
         MAX_BYTECODE,      // bytecode
         MAX_RWS,           // copy
         MAX_KECCAK_ROWS,   // keccak
+        MAX_SHA256_ROWS,   // sha256
         MAX_RWS,           // tx
         MAX_RLP_ROWS,      // rlp
         8 * MAX_EXP_STEPS, // exp
@@ -458,8 +463,12 @@ fn get_sub_circuit_limit_l2() -> Vec<usize> {
         MAX_POSEIDON_ROWS, // poseidon
         MAX_VERTICAL_ROWS, // sig
         MAX_VERTICAL_ROWS, // ecc
-        MAX_MPT_ROWS,      // mpt
-    ]
+    ];
+    #[cfg(feature = "scroll")]
+    {
+        limit.push(MAX_MPT_ROWS); // mpt
+    }
+    limit
 }
 
 fn get_params_for_super_circuit_test_l2() -> CircuitsParams {
@@ -606,48 +615,47 @@ pub fn run_test(
     log::debug!("witness_block created");
     //builder.sdb.list_accounts();
 
-    let check_ccc = || {
-        let row_usage = ScrollSuperCircuit::min_num_rows_block_subcircuits(&witness_block);
-        let mut overflow = false;
-        for (num, limit) in row_usage.iter().zip_eq(get_sub_circuit_limit_l2().iter()) {
-            if num.row_num_real > *limit {
-                log::warn!(
-                    "ccc detail: suite.id {}, st.id {}, circuit {}, num {}, limit {}",
-                    suite.id,
-                    st.id,
-                    num.name,
-                    num.row_num_real,
-                    limit
-                );
-                overflow = true;
-            }
-        }
-        let max_row_usage = row_usage.iter().max_by_key(|r| r.row_num_real).unwrap();
-        if overflow {
+    let row_usage = ScrollSuperCircuit::min_num_rows_block_subcircuits(&witness_block);
+    let mut overflow = false;
+    for (num, limit) in row_usage.iter().zip_eq(get_sub_circuit_limit().iter()) {
+        if num.row_num_real > *limit {
             log::warn!(
-                "ccc overflow: st.id {}, detail {} {}",
+                "ccc detail: suite.id {}, st.id {}, circuit {}, num {}, limit {}",
+                suite.id,
                 st.id,
-                max_row_usage.name,
-                max_row_usage.row_num_real
-            );
-            panic!("{} {}", max_row_usage.name, max_row_usage.row_num_real);
-        } else {
-            log::info!(
-                "ccc ok: st.id {}, detail {} {}",
-                st.id,
-                max_row_usage.name,
-                max_row_usage.row_num_real
+                num.name,
+                num.row_num_real,
+                limit
             );
+            overflow = true;
         }
-    };
+    }
+    let max_row_usage = row_usage.iter().max_by_key(|r| r.row_num_real).unwrap();
+    if overflow {
+        log::warn!(
+            "ccc overflow: st.id {}, detail {} {}",
+            st.id,
+            max_row_usage.name,
+            max_row_usage.row_num_real
+        );
+        // panic!("{} {}", max_row_usage.name, max_row_usage.row_num_real);
+        return Err(StateTestError::CircuitOverflow {
+            circuit: max_row_usage.name.to_string(),
+            needed: max_row_usage.row_num_real,
+        });
+    }
+    log::info!(
+        "ccc ok: st.id {}, detail {} {}",
+        st.id,
+        max_row_usage.name,
+        max_row_usage.row_num_real
+    );
 
     if !circuits_config.super_circuit {
         if (*CIRCUIT).is_empty() {
             CircuitTestBuilder::<1, 1>::new_from_block(witness_block)
                 .copy_checks(None)
                 .run();
-        } else if (*CIRCUIT) == "ccc" {
-            check_ccc();
         } else {
             match (*CIRCUIT).as_str() {
                 "modexp" => test_with::<ModExpCircuit<Fr>>(&witness_block),
@@ -669,22 +677,18 @@ pub fn run_test(
         }
     } else {
         log::debug!("test super circuit {}", *CIRCUIT);
-        if (*CIRCUIT) == "ccc" {
-            check_ccc();
-        } else {
-            #[cfg(feature = "inner-prove")]
-            {
-                set_env_coinbase(&st.env.current_coinbase);
-                prover::test::inner_prove(&test_id, &witness_block);
-            }
-            #[cfg(feature = "chunk-prove")]
-            {
-                set_env_coinbase(&st.env.current_coinbase);
-                prover::test::chunk_prove(&test_id, &witness_block);
-            }
-            #[cfg(not(any(feature = "inner-prove", feature = "chunk-prove")))]
-            mock_prove(&test_id, &witness_block);
+        #[cfg(feature = "inner-prove")]
+        {
+            set_env_coinbase(&st.env.current_coinbase);
+            prover::test::inner_prove(&test_id, &witness_block);
+        }
+        #[cfg(feature = "chunk-prove")]
+        {
+            set_env_coinbase(&st.env.current_coinbase);
+            prover::test::chunk_prove(&test_id, &witness_block);
         }
+        #[cfg(not(any(feature = "inner-prove", feature = "chunk-prove")))]
+        mock_prove(&test_id, &witness_block);
     };
     log::debug!("balance_overflow = {balance_overflow}");
     log::debug!(

diff --git a/testool/src/statetest/yaml.rs b/testool/src/statetest/yaml.rs
@@ -796,6 +796,7 @@ arith:
 
         Ok(())
     }
+
     #[test]
     fn bad_balance() -> Result<()> {
         let mut tc = YamlStateTestBuilder::new(&Compiler::default()).load_yaml(

diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs
@@ -3600,8 +3600,14 @@ impl<F: Field> SubCircuit<F> for TxCircuit<F> {
         // Since each call data byte at least takes one row in RLP circuit.
         // For L2 tx, each call data byte takes two row in RLP circuit.
         assert!(block.circuits_params.max_calldata < block.circuits_params.max_rlp_rows);
-        let tx_usage = (block.txs.iter().map(|tx| tx.call_data.len()).sum::<usize>()) as f32
-            / block.circuits_params.max_calldata as f32;
+        let sum_calldata_len = block.txs.iter().map(|tx| tx.call_data.len()).sum::<usize>();
+        let max_calldata = if block.circuits_params.max_calldata == 0 {
+            // dynamic max_calldata
+            sum_calldata_len
+        } else {
+            block.circuits_params.max_calldata
+        };
+        let tx_usage = sum_calldata_len as f32 / max_calldata as f32;
 
         (
             (tx_usage * block.circuits_params.max_vertical_circuit_rows as f32).ceil() as usize,