From 159d576bf5aa59a3ac17fca1577b849f15877561 Mon Sep 17 00:00:00 2001
From: David Nevado <david@davidnevado.xyz>
Date: Wed, 21 Jun 2023 17:41:12 +0200
Subject: [PATCH] Fix `ExpCircuit` parameter bug + Improve tests

- max_rows and max_steps were being confused
- `gen_data` in test now uses dynamic parameters for all tests
  except variadic size test.
---
 zkevm-circuits/src/exp_circuit.rs      | 10 +++++----
 zkevm-circuits/src/exp_circuit/test.rs | 28 +++++++++++++++++---------
 2 files changed, 24 insertions(+), 14 deletions(-)

diff --git a/zkevm-circuits/src/exp_circuit.rs b/zkevm-circuits/src/exp_circuit.rs
index 4da5979a0f..8d108a9557 100644
--- a/zkevm-circuits/src/exp_circuit.rs
+++ b/zkevm-circuits/src/exp_circuit.rs
@@ -291,11 +291,12 @@ impl<F: Field> ExpCircuitConfig<F> {
         &self,
         layouter: &mut impl Layouter<F>,
         exp_events: &[ExpEvent],
-        max_exp_steps: usize,
+        max_exp_rows: usize,
     ) -> Result<(), Error> {
-        let max_exp_rows = max_exp_steps * OFFSET_INCREMENT;
+        let min_n_rows = Self::min_num_rows(exp_events);
+        dbg!(max_exp_rows, min_n_rows);
         debug_assert!(
-            Self::min_num_rows(exp_events) <= max_exp_rows,
+            min_n_rows <= max_exp_rows,
             "insufficient rows to populate the exponentiation trace"
         );
 
@@ -484,7 +485,8 @@ pub struct ExpCircuit<F> {
 
 impl<F: Field> ExpCircuit<F> {
     /// Return a new ExpCircuit
-    pub fn new(exp_events: Vec<ExpEvent>, max_exp_rows: usize) -> Self {
+    pub fn new(exp_events: Vec<ExpEvent>, max_exp_steps: usize) -> Self {
+        let max_exp_rows = max_exp_steps * OFFSET_INCREMENT + UNUSABLE_EXP_ROWS;
         Self {
             exp_events,
             max_exp_rows,
diff --git a/zkevm-circuits/src/exp_circuit/test.rs b/zkevm-circuits/src/exp_circuit/test.rs
index 9ba86f60eb..1c190231e8 100644
--- a/zkevm-circuits/src/exp_circuit/test.rs
+++ b/zkevm-circuits/src/exp_circuit/test.rs
@@ -51,29 +51,37 @@ fn gen_code_multiple(args: Vec<(Word, Word)>) -> Bytecode {
     code
 }
 
-fn gen_data(code: Bytecode) -> CircuitInputBuilder<FixedCParams> {
+fn gen_data(code: Bytecode, default_params: bool) -> CircuitInputBuilder<FixedCParams> {
     let test_ctx = TestContext::<2, 1>::simple_ctx_with_bytecode(code).unwrap();
     let block: GethData = test_ctx.into();
     // Needs default parameters for variadic size test
-    let mut builder =
-        BlockData::new_from_geth_data_with_params(block.clone(), FixedCParams::default())
-            .new_circuit_input_builder();
-    builder
-        .handle_block(&block.eth_block, &block.geth_traces)
-        .unwrap();
+    let builder = if default_params {
+        let mut builder =
+            BlockData::new_from_geth_data_with_params(block.clone(), FixedCParams::default())
+                .new_circuit_input_builder();
+        builder
+            .handle_block(&block.eth_block, &block.geth_traces)
+            .unwrap();
+        builder
+    } else {
+        let builder = BlockData::new_from_geth_data(block.clone()).new_circuit_input_builder();
+        builder
+            .handle_block(&block.eth_block, &block.geth_traces)
+            .unwrap()
+    };
     builder
 }
 
 fn test_ok(base: Word, exponent: Word, k: Option<u32>) {
     let code = gen_code_single(base, exponent);
-    let builder = gen_data(code);
+    let builder = gen_data(code, false);
     let block = block_convert::<Fr>(&builder).unwrap();
     test_exp_circuit(k.unwrap_or(18), block);
 }
 
 fn test_ok_multiple(args: Vec<(Word, Word)>) {
     let code = gen_code_multiple(args);
-    let builder = gen_data(code);
+    let builder = gen_data(code, false);
     let block = block_convert::<Fr>(&builder).unwrap();
     test_exp_circuit(20, block);
 }
@@ -142,7 +150,7 @@ fn variadic_size_check() {
         EXP
         STOP
     };
-    let builder = gen_data(code);
+    let builder = gen_data(code, true);
     let block = block_convert::<Fr>(&builder).unwrap();
     let circuit = ExpCircuit::<Fr>::new(
         block.exp_events.clone(),