Add SubCircuit::unusable_rows and tests for each SubCircuit (scro…

…ll-tech#1282)

### Description

To remove magic numbers for blinding factors, this PR adds an associated
function `unusable_rows` for `SubCircuit` to returns their
`unusable_rows`, which should be `meta.blinding_factors() + 1`.

Tests are also added to make sure the returned values are correct. 

Note that currently `KeccakCircuit` could be configured by environment
variable `KECCAK_ROWS` to decide how many rows a round would take, which
affect the different rotation queried, and it's unfortunately not a easy
way to compute how many different rotations are used, so this PR
hardcodes the unusable rows from `KECCAK_ROWS = 1` to `20` for easy
lookup.

### Issue Link


privacy-scaling-explorations#949

### Type of change

- [ ] Bug fix (non-breaking change which fixes an issue)
- [x] New feature (non-breaking change which adds functionality)
- [ ] Breaking change (fix or feature that would cause existing
functionality to not work as expected)
- [ ] This change requires a documentation update

### Contents

- [_item_]

### Rationale

[_design decisions and extended information_]

### How Has This Been Tested?

[_explanation_]

---------

Co-authored-by: David Nevado <[email protected]>
Co-authored-by: "adria0.eth" <[email protected]>
Co-authored-by: Eduard S <[email protected]>

circuit-benchmarks/src/bytecode_circuit.rs

@@ -23,9 +23,13 @@ mod tests {
     use rand::SeedableRng;
     use rand_xorshift::XorShiftRng;
     use std::env::var;
-    use zkevm_circuits::bytecode_circuit::bytecode_unroller::{unroll, UnrolledBytecode};
-
-    use zkevm_circuits::bytecode_circuit::TestBytecodeCircuit;
+    use zkevm_circuits::{
+        bytecode_circuit::{
+            bytecode_unroller::{unroll, UnrolledBytecode},
+            TestBytecodeCircuit,
+        },
+        util::SubCircuit,
+    };
 
     #[cfg_attr(not(feature = "benches"), ignore)]
     #[test]
@@ -45,8 +49,7 @@ mod tests {
         const MAX_BYTECODE_LEN: usize = 24576;
 
         let num_rows = 1 << degree;
-        const NUM_BLINDING_ROWS: usize = 7 - 1;
-        let max_bytecode_row_num = num_rows - NUM_BLINDING_ROWS;
+        let max_bytecode_row_num = num_rows - TestBytecodeCircuit::<Fr>::unusable_rows();
         let bytecode_len = std::cmp::min(MAX_BYTECODE_LEN, max_bytecode_row_num);
         let bytecodes_num: usize = max_bytecode_row_num / bytecode_len;
 

circuit-benchmarks/src/packed_multi_keccak.rs

@@ -21,7 +21,7 @@ mod tests {
     use rand::SeedableRng;
     use rand_xorshift::XorShiftRng;
     use std::env::var;
-    use zkevm_circuits::keccak_circuit::TestKeccakCircuit;
+    use zkevm_circuits::{keccak_circuit::TestKeccakCircuit, util::SubCircuit};
 
     #[cfg_attr(not(feature = "benches"), ignore)]
     #[test]
@@ -41,7 +41,10 @@ mod tests {
         let inputs = vec![(0u8..135).collect::<Vec<_>>(); 3];
 
         // Create the circuit. Leave last dozens of rows for blinding.
-        let circuit = TestKeccakCircuit::new(2usize.pow(degree) - 64, inputs);
+        let circuit = TestKeccakCircuit::new(
+            2usize.pow(degree) - TestKeccakCircuit::<Fr>::unusable_rows(),
+            inputs,
+        );
 
         // Initialize the polynomial commitment parameters
         let mut rng = XorShiftRng::from_seed([

zkevm-circuits/src/bytecode_circuit/circuit.rs

@@ -879,15 +879,14 @@ impl<F: Field> BytecodeCircuit<F> {
 impl<F: Field> SubCircuit<F> for BytecodeCircuit<F> {
     type Config = BytecodeCircuitConfig<F>;
 
+    fn unusable_rows() -> usize {
+        // No column queried at more than 3 distinct rotations, so returns 6 as
+        // minimum unusable rows.
+        6
+    }
+
     fn new_from_block(block: &witness::Block<F>) -> Self {
-        // TODO: Find a nicer way to add the extra `128`.  Is this to account for
-        // unusable rows? Then it could be calculated like this:
-        // fn unusable_rows<F: Field, C: Circuit<F>>() -> usize {
-        //     let mut cs = ConstraintSystem::default();
-        //     C::configure(&mut cs);
-        //     cs.blinding_factors()
-        // }
-        let bytecode_size = block.circuits_params.max_bytecode + 128;
+        let bytecode_size = block.circuits_params.max_bytecode;
         Self::new_from_block_sized(block, bytecode_size)
     }
 

zkevm-circuits/src/bytecode_circuit/test.rs

@@ -2,13 +2,21 @@
 use crate::{
     bytecode_circuit::{bytecode_unroller::*, circuit::BytecodeCircuit},
     table::BytecodeFieldTag,
-    util::{is_push, keccak, Challenges, SubCircuit},
+    util::{is_push, keccak, unusable_rows, Challenges, SubCircuit},
 };
 use bus_mapping::evm::OpcodeId;
 use eth_types::{Bytecode, Field, Word};
 use halo2_proofs::{dev::MockProver, halo2curves::bn256::Fr};
 use log::error;
 
+#[test]
+fn bytecode_circuit_unusable_rows() {
+    assert_eq!(
+        BytecodeCircuit::<Fr>::unusable_rows(),
+        unusable_rows::<Fr, BytecodeCircuit::<Fr>>(),
+    )
+}
+
 impl<F: Field> BytecodeCircuit<F> {
     /// Verify that the selected bytecode fulfills the circuit
     pub fn verify_raw(k: u32, bytecodes: Vec<Vec<u8>>) {

zkevm-circuits/src/copy_circuit.rs

@@ -731,6 +731,12 @@ impl<F: Field> CopyCircuit<F> {
 impl<F: Field> SubCircuit<F> for CopyCircuit<F> {
     type Config = CopyCircuitConfig<F>;
 
+    fn unusable_rows() -> usize {
+        // No column queried at more than 3 distinct rotations, so returns 6 as
+        // minimum unusable rows.
+        6
+    }
+
     fn new_from_block(block: &witness::Block<F>) -> Self {
         Self::new_with_external_data(
             block.copy_events.clone(),

zkevm-circuits/src/copy_circuit/test.rs

@@ -3,6 +3,7 @@
 use crate::{
     copy_circuit::*,
     evm_circuit::{test::rand_bytes, witness::block_convert},
+    util::unusable_rows,
     witness::Block,
 };
 use bus_mapping::{
@@ -17,6 +18,14 @@ use halo2_proofs::{
 };
 use mock::{test_ctx::helpers::account_0_code_account_1_no_code, TestContext, MOCK_ACCOUNTS};
 
+#[test]
+fn copy_circuit_unusable_rows() {
+    assert_eq!(
+        CopyCircuit::<Fr>::unusable_rows(),
+        unusable_rows::<Fr, CopyCircuit::<Fr>>(),
+    )
+}
+
 /// Test copy circuit from copy events and test data
 pub fn test_copy_circuit<F: Field>(
     k: u32,

zkevm-circuits/src/evm_circuit.rs

@@ -19,6 +19,7 @@ pub use self::EvmCircuit as TestEvmCircuit;
 
 pub use crate::witness;
 use crate::{
+    evm_circuit::param::{MAX_STEP_HEIGHT, STEP_STATE_HEIGHT},
     table::{
         BlockTable, BytecodeTable, CopyTable, ExpTable, KeccakTable, LookupTable, RwTable, TxTable,
     },
@@ -233,6 +234,12 @@ impl<F: Field> EvmCircuit<F> {
 impl<F: Field> SubCircuit<F> for EvmCircuit<F> {
     type Config = EvmCircuitConfig<F>;
 
+    fn unusable_rows() -> usize {
+        // Most columns are queried at MAX_STEP_HEIGHT + STEP_STATE_HEIGHT distinct rotations, so
+        // returns (MAX_STEP_HEIGHT + STEP_STATE_HEIGHT + 3) unusable rows.
+        MAX_STEP_HEIGHT + STEP_STATE_HEIGHT + 3
+    }
+
     fn new_from_block(block: &witness::Block<F>) -> Self {
         Self::new(block.clone())
     }
@@ -439,6 +446,7 @@ mod evm_circuit_stats {
         },
         stats::print_circuit_stats_by_states,
         test_util::CircuitTestBuilder,
+        util::{unusable_rows, SubCircuit},
         witness::block_convert,
     };
     use bus_mapping::{circuit_input_builder::CircuitsParams, mock::BlockData};
@@ -458,6 +466,14 @@ mod evm_circuit_stats {
         MOCK_ACCOUNTS,
     };
 
+    #[test]
+    fn evm_circuit_unusable_rows() {
+        assert_eq!(
+            EvmCircuit::<Fr>::unusable_rows(),
+            unusable_rows::<Fr, EvmCircuit::<Fr>>(),
+        )
+    }
+
     #[test]
     pub fn empty_evm_circuit_no_padding() {
         CircuitTestBuilder::new_from_test_ctx(

zkevm-circuits/src/exp_circuit.rs

@@ -496,6 +496,29 @@ impl<F: Field> ExpCircuit<F> {
 impl<F: Field> SubCircuit<F> for ExpCircuit<F> {
     type Config = ExpCircuitConfig<F>;
 
+    fn unusable_rows() -> usize {
+        // Column base_limb of ExpTable is queried at 8 distinct rotations at
+        // - Rotation(0)
+        // - Rotation(1)
+        // - Rotation(2)
+        // - Rotation(3)
+        // - Rotation(7)
+        // - Rotation(8)
+        // - Rotation(9)
+        // - Rotation(10)
+        // Also column col2 and col3 of are queried at 8 distinct rotations at
+        // - Rotation(0)
+        // - Rotation(1)
+        // - Rotation(2)
+        // - Rotation(3)
+        // - Rotation(4)
+        // - Rotation(5)
+        // - Rotation(6)
+        // - Rotation(9)
+        // so returns 11 unusable rows.
+        11
+    }
+
     fn new_from_block(block: &witness::Block<F>) -> Self {
         // Hardcoded to pass unit tests for now. In the future, insert:
         // "block.circuits_params.max_exp_rows"

zkevm-circuits/src/exp_circuit/test.rs

@@ -2,6 +2,7 @@
 use crate::{
     evm_circuit::witness::{block_convert, Block},
     exp_circuit::ExpCircuit,
+    util::{unusable_rows, SubCircuit},
 };
 use bus_mapping::{
     circuit_input_builder::{CircuitInputBuilder, CircuitsParams},
@@ -14,6 +15,14 @@ use halo2_proofs::{
 };
 use mock::TestContext;
 
+#[test]
+fn exp_circuit_unusable_rows() {
+    assert_eq!(
+        ExpCircuit::<Fr>::unusable_rows(),
+        unusable_rows::<Fr, ExpCircuit::<Fr>>(),
+    )
+}
+
 /// Test exponentiation circuit with the provided block witness
 pub fn test_exp_circuit<F: Field>(k: u32, block: Block<F>) {
     let circuit = ExpCircuit::<F>::new(

zkevm-circuits/src/keccak_circuit.rs

@@ -19,7 +19,7 @@ pub use KeccakCircuitConfig as KeccakConfig;
 
 use self::{
     cell_manager::*,
-    keccak_packed_multi::{multi_keccak, KeccakRow},
+    keccak_packed_multi::{keccak_unusable_rows, multi_keccak, KeccakRow},
     param::*,
     table::*,
     util::*,
@@ -982,6 +982,10 @@ pub struct KeccakCircuit<F: Field> {
 impl<F: Field> SubCircuit<F> for KeccakCircuit<F> {
     type Config = KeccakCircuitConfig<F>;
 
+    fn unusable_rows() -> usize {
+        keccak_unusable_rows()
+    }
+
     /// The `block.circuits_params.keccak_padding` parmeter, when enabled, sets
     /// up the circuit to support a fixed number of permutations/keccak_f's,
     /// independently of the permutations required by `inputs`.

zkevm-circuits/src/keccak_circuit/keccak_packed_multi.rs

@@ -11,11 +11,19 @@ use std::{env::var, vec};
 
 pub(crate) fn get_num_rows_per_round() -> usize {
     var("KECCAK_ROWS")
-        .unwrap_or_else(|_| "12".to_string())
+        .unwrap_or_else(|_| format!("{DEFAULT_KECCAK_ROWS}"))
         .parse()
         .expect("Cannot parse KECCAK_ROWS env var as usize")
 }
 
+pub(crate) fn keccak_unusable_rows() -> usize {
+    const UNUSABLE_ROWS_BY_KECCAK_ROWS: [usize; 24] = [
+        53, 67, 63, 59, 45, 79, 77, 75, 73, 71, 69, 67, 65, 63, 61, 59, 57, 71, 89, 107, 107, 107,
+        107, 107,
+    ];
+    UNUSABLE_ROWS_BY_KECCAK_ROWS[get_num_rows_per_round() - NUM_BYTES_PER_WORD - 1]
+}
+
 pub(crate) fn get_num_bits_per_absorb_lookup() -> usize {
     get_num_bits_per_lookup(ABSORB_LOOKUP_RANGE)
 }

zkevm-circuits/src/keccak_circuit/param.rs

@@ -1,4 +1,5 @@
 pub(crate) const MAX_DEGREE: usize = 9;
+pub(crate) const DEFAULT_KECCAK_ROWS: usize = 12;
 pub(crate) const ABSORB_LOOKUP_RANGE: usize = 3;
 pub(crate) const THETA_C_LOOKUP_RANGE: usize = 6;
 pub(crate) const RHO_PI_LOOKUP_RANGE: usize = 4;

zkevm-circuits/src/keccak_circuit/test.rs

@@ -1,5 +1,6 @@
 #![allow(unused_imports)]
 use super::*;
+use crate::util::unusable_rows;
 use eth_types::Field;
 use halo2_proofs::{
     circuit::{Layouter, SimpleFloorPlanner},
@@ -12,6 +13,21 @@ use std::iter::zip;
 
 use super::util::{target_part_sizes, target_part_sizes_rot, WordParts};
 
+// This needs to be tested independent since it sets the environment variable
+// which might affect other tests.
+#[ignore]
+#[test]
+fn serial_keccak_circuit_unusable_rows() {
+    for keccak_rows in NUM_BYTES_PER_WORD + 1..=32 {
+        std::env::set_var("KECCAK_ROWS", format!("{keccak_rows}"));
+        assert_eq!(
+            KeccakCircuit::<Fr>::unusable_rows(),
+            unusable_rows::<Fr, KeccakCircuit::<Fr>>(),
+        )
+    }
+    std::env::set_var("KECCAK_ROWS", format!("{DEFAULT_KECCAK_ROWS}"));
+}
+
 fn verify<F: Field>(k: u32, inputs: Vec<Vec<u8>>, success: bool) {
     let circuit = KeccakCircuit::new(2usize.pow(k), inputs);
 

zkevm-circuits/src/keccak_circuit/util.rs

@@ -1,6 +1,6 @@
 //! Utility traits, functions used in the crate.
 
-use super::param::*;
+use super::{keccak_packed_multi::keccak_unusable_rows, param::*};
 use eth_types::{Field, ToScalar, Word};
 use halo2_proofs::{circuit::Value, halo2curves::FieldExt};
 use std::env::var;
@@ -224,7 +224,7 @@ pub(crate) fn get_num_bits_per_lookup(range: usize) -> usize {
 
 // Implementation of the above without environment dependency.
 pub(crate) fn get_num_bits_per_lookup_impl(range: usize, log_height: usize) -> usize {
-    let num_unusable_rows = 31;
+    let num_unusable_rows = keccak_unusable_rows();
     let height = 2usize.pow(log_height as u32);
     let mut num_bits = 1;
     while range.pow(num_bits + 1) + num_unusable_rows <= height {

zkevm-circuits/src/pi_circuit.rs

@@ -1141,6 +1141,16 @@ impl<F: Field> PiCircuit<F> {
 impl<F: Field> SubCircuit<F> for PiCircuit<F> {
     type Config = PiCircuitConfig<F>;
 
+    fn unusable_rows() -> usize {
+        // Column raw_public_inputs is queried at 4 distinct rotations at
+        // - Rotation::cur()
+        // - Rotation(BLOCK_LEN + 1 + EXTRA_LEN)
+        // - Rotation(BLOCK_LEN + 1 + EXTRA_LEN + max_txs * TX_LEN + 1)
+        // - Rotation(BLOCK_LEN + 1 + EXTRA_LEN + 2 * (max_txs * TX_LEN + 1))
+        // so returns 7 unusable rows.
+        7
+    }
+
     fn new_from_block(block: &witness::Block<F>) -> Self {
         let public_data = PublicData {
             chain_id: block.context.chain_id,

zkevm-circuits/src/pi_circuit/dev.rs

@@ -17,6 +17,10 @@ impl<F: Field, const MAX_TXS: usize, const MAX_CALLDATA: usize> SubCircuit<F>
 {
     type Config = PiCircuitConfig<F>;
 
+    fn unusable_rows() -> usize {
+        PiCircuit::<F>::unusable_rows()
+    }
+
     fn new_from_block(block: &witness::Block<F>) -> Self {
         assert_eq!(block.circuits_params.max_txs, MAX_TXS);
         assert_eq!(block.circuits_params.max_calldata, MAX_CALLDATA);

zkevm-circuits/src/pi_circuit/test.rs

@@ -1,5 +1,6 @@
 #![allow(unused_imports)]
 use super::{dev::*, *};
+use crate::util::unusable_rows;
 use halo2_proofs::{
     dev::{MockProver, VerifyFailure},
     halo2curves::bn256::Fr,
@@ -8,6 +9,16 @@ use mock::{CORRECT_MOCK_TXS, MOCK_CHAIN_ID};
 use rand::SeedableRng;
 use rand_chacha::ChaCha20Rng;
 
+#[test]
+fn pi_circuit_unusable_rows() {
+    const MAX_TXS: usize = 2;
+    const MAX_CALLDATA: usize = 8;
+    assert_eq!(
+        PiCircuit::<Fr>::unusable_rows(),
+        unusable_rows::<Fr, PiTestCircuit::<Fr, MAX_TXS, MAX_CALLDATA>>(),
+    )
+}
+
 fn run<F: Field, const MAX_TXS: usize, const MAX_CALLDATA: usize>(
     k: u32,
     public_data: PublicData,

zkevm-circuits/src/state_circuit.rs

@@ -455,6 +455,12 @@ impl<F: Field> SubCircuit<F> for StateCircuit<F> {
         Self::new(block.rws.clone(), block.circuits_params.max_rws)
     }
 
+    fn unusable_rows() -> usize {
+        // No column queried at more than 3 distinct rotations, so returns 6 as
+        // minimum unusable rows.
+        6
+    }
+
     /// Return the minimum number of rows required to prove the block
     fn min_num_rows_block(block: &witness::Block<F>) -> (usize, usize) {
         (