feat: refactor sig_circuit and ecc_circuit row capacity estimation (#669)

* feat: add trace-level log on halo2-lib ctx pos for ecc

* add constant row accounting for ec ops

* Add blinding factors to CircuitStats

* feat: increase sig circuit degree to 20

* impl: correct sig circuit min_num_rows_block fn

* remove unnecessary import

* correct documentation

* restore super circuit tests

* impl unusable_row fn for ecc/sig to conform to blinding factor

* preserve naming

* add minimum row to sig circuit as a fractional repr for total capacity

* correct min_row_num accounting for ecc/sig

* remove unnecessary import

* try super circuit integration tests

* restore ignored tests

* resolve max_tx_count for sig circuit

* conform to rustfmt

* fmt for clippy

* correct tx circuit target degree

* exclude tx circuit tests from default

---------

Co-authored-by: Rohit Narurkar <[email protected]>
Co-authored-by: Zhang Zhuo <[email protected]>

Author: 3 people

zkevm-circuits/src/ecc_circuit.rs

@@ -32,6 +32,8 @@ use itertools::Itertools;
 use log::error;
 
 use crate::{
+    evm_circuit::EvmCircuit,
+    keccak_circuit::KeccakCircuit,
     table::{EccTable, LookupTable},
     util::{Challenges, SubCircuit, SubCircuitConfig},
     witness::Block,
@@ -47,6 +49,12 @@ use util::{
 
 use self::util::LOG_TOTAL_NUM_ROWS;
 
+macro_rules! log_context_cursor {
+    ($ctx: ident) => {{
+        log::trace!("Ctx cell pos: {:?}", $ctx.advice_alloc);
+    }};
+}
+
 /// Arguments accepted to configure the EccCircuitConfig.
 #[derive(Clone, Debug)]
 pub struct EccCircuitConfigArgs<F: Field> {
@@ -405,10 +413,16 @@ impl<F: Field, const XI_0: i64> EccCircuit<F, XI_0> {
         powers_of_rand: &[QuantumCell<F>],
         op: &EcAddOp,
     ) -> EcAddAssigned<F> {
+        log::trace!("[ECC] ==> EcAdd Assignmnet START:");
+        log_context_cursor!(ctx);
+
         let point_p = self.assign_g1(ctx, ecc_chip, op.p, powers_of_rand);
         let point_q = self.assign_g1(ctx, ecc_chip, op.q, powers_of_rand);
         let point_r = self.assign_g1(ctx, ecc_chip, op.r, powers_of_rand);
 
+        log::trace!("[ECC] EcAdd Inputs Assigned:");
+        log_context_cursor!(ctx);
+
         // We follow the approach mentioned below to handle many edge cases for the points P, Q and
         // R so that we can maintain the same fixed and permutation columns and reduce the overall
         // validation process from the EVM Circuit.
@@ -472,6 +486,8 @@ impl<F: Field, const XI_0: i64> EccCircuit<F, XI_0> {
 
         ecc_chip.assert_equal(ctx, &rand_point, &sum3);
 
+        log::trace!("[ECC] EcAdd Assignmnet END:");
+        log_context_cursor!(ctx);
         EcAddAssigned {
             point_p,
             point_q,
@@ -490,9 +506,16 @@ impl<F: Field, const XI_0: i64> EccCircuit<F, XI_0> {
         powers_of_rand: &[QuantumCell<F>],
         op: &EcMulOp,
     ) -> EcMulAssigned<F> {
+        log::trace!("[ECC] ==> EcMul Assignmnet START:");
+        log_context_cursor!(ctx);
+
         let point_p = self.assign_g1(ctx, ecc_chip, op.p, powers_of_rand);
         let scalar_s = self.assign_fr(ctx, fr_chip, op.s);
         let point_r = self.assign_g1(ctx, ecc_chip, op.r, powers_of_rand);
+
+        log::trace!("[ECC] EcMul Inputs Assigned:");
+        log_context_cursor!(ctx);
+
         let point_r_got = ecc_chip.scalar_mult(
             ctx,
             &point_p.decomposed.ec_point,
@@ -501,6 +524,10 @@ impl<F: Field, const XI_0: i64> EccCircuit<F, XI_0> {
             4, // TODO: window bits?
         );
         ecc_chip.assert_equal(ctx, &point_r.decomposed.ec_point, &point_r_got);
+
+        log::trace!("[ECC] EcMul Assignmnet END:");
+        log_context_cursor!(ctx);
+
         EcMulAssigned {
             point_p,
             scalar_s,
@@ -519,6 +546,9 @@ impl<F: Field, const XI_0: i64> EccCircuit<F, XI_0> {
         powers_of_rand: &[QuantumCell<F>],
         op: &EcPairingOp,
     ) -> EcPairingAssigned<F> {
+        log::trace!("[ECC] ==> EcPairing Assignment START:");
+        log_context_cursor!(ctx);
+
         let g1s = op
             .pairs
             .iter()
@@ -544,6 +574,10 @@ impl<F: Field, const XI_0: i64> EccCircuit<F, XI_0> {
                 }
             })
             .collect_vec();
+
+        log::trace!("[ECC] EcPairing g1s Assigned:");
+        log_context_cursor!(ctx);
+
         let g2s = op
             .pairs
             .iter()
@@ -583,6 +617,9 @@ impl<F: Field, const XI_0: i64> EccCircuit<F, XI_0> {
             })
             .collect_vec();
 
+        log::trace!("[ECC] EcPairing g2s Assigned:");
+        log_context_cursor!(ctx);
+
         // RLC over the entire input bytes.
         let input_cells = g1s
             .iter()
@@ -606,6 +643,9 @@ impl<F: Field, const XI_0: i64> EccCircuit<F, XI_0> {
             powers_of_rand.iter().cloned(),
         );
 
+        log::trace!("[ECC] EcPairing Inputs RLC Assigned:");
+        log_context_cursor!(ctx);
+
         let pairs = g1s
             .iter()
             .zip(g2s.iter())
@@ -630,6 +670,9 @@ impl<F: Field, const XI_0: i64> EccCircuit<F, XI_0> {
             )),
         );
 
+        log::trace!("[ECC] EcPairingAssignment END:");
+        log_context_cursor!(ctx);
+
         EcPairingAssigned {
             g1s,
             g2s,
@@ -732,6 +775,19 @@ impl<F: Field, const XI_0: i64> SubCircuit<F> for EccCircuit<F, XI_0> {
         }
     }
 
+    /// Returns number of unusable rows of the SubCircuit, which should be
+    /// `meta.blinding_factors() + 1`.
+    fn unusable_rows() -> usize {
+        [
+            KeccakCircuit::<F>::unusable_rows(),
+            EvmCircuit::<F>::unusable_rows(),
+            // may include additional subcircuits here
+        ]
+        .into_iter()
+        .max()
+        .unwrap()
+    }
+
     fn synthesize_sub(
         &self,
         config: &Self::Config,
@@ -743,7 +799,33 @@ impl<F: Field, const XI_0: i64> SubCircuit<F> for EccCircuit<F, XI_0> {
         Ok(())
     }
 
-    fn min_num_rows_block(_block: &Block<F>) -> (usize, usize) {
-        unimplemented!()
+    fn min_num_rows_block(block: &Block<F>) -> (usize, usize) {
+        // EccCircuit can't determine usable rows independently.
+        // Instead, the blinding area is determined by other advise columns with most counts of
+        // rotation queries. This value is typically determined by either the Keccak or EVM
+        // circuit.
+
+        let max_blinding_factor = Self::unusable_rows() - 1;
+
+        // same formula as halo2-lib's FlexGate
+        let row_num = (1 << LOG_TOTAL_NUM_ROWS) - (max_blinding_factor + 3);
+
+        let ec_adds = block.get_ec_add_ops().len();
+        let ec_muls = block.get_ec_mul_ops().len();
+        let ec_pairings = block.get_ec_pairing_ops().len();
+
+        // Instead of showing actual minimum row usage,
+        // halo2-lib based circuits use min_row_num to represent a percentage of total-used capacity
+        // This functionality allows l2geth to decide if additional ops can be added.
+        let min_row_num = [
+            (row_num / block.circuits_params.max_ec_ops.ec_add) * ec_adds,
+            (row_num / block.circuits_params.max_ec_ops.ec_mul) * ec_muls,
+            (row_num / block.circuits_params.max_ec_ops.ec_pairing) * ec_pairings,
+        ]
+        .into_iter()
+        .max()
+        .unwrap();
+
+        (min_row_num, row_num)
     }
 }

zkevm-circuits/src/ecc_circuit/util.rs

@@ -2,8 +2,16 @@ use eth_types::Field;
 use halo2_base::{AssignedValue, QuantumCell};
 use halo2_ecc::{bigint::CRTInteger, ecc::EcPoint, fields::FieldExtPoint};
 
+// Total number of rows allowable for ECC circuit
 pub const LOG_TOTAL_NUM_ROWS: u32 = 20;
 
+// Cell usage accounting for EcAdd, EcMul and EcPairing
+// Roud up to nearest 100
+pub(super) const EC_ADD_CELLS: usize = 6_900; // actual: 6_851
+pub(super) const EC_MUL_CELLS: usize = 405_500; // actual: 405_476
+pub(super) const EC_PAIRING_CELLS: usize = 6_627_500; // actual: 6_627_442
+pub(super) const COLUMN_NUM_LIMIT: usize = 150; // Max number of columns allowed
+
 pub struct G1Decomposed<F: Field> {
     pub ec_point: EcPoint<F, CRTInteger<F>>,
     pub x_cells: Vec<QuantumCell<F>>,

zkevm-circuits/src/sig_circuit.rs

@@ -15,7 +15,11 @@ mod dev;
 mod test;
 
 use crate::{
-    evm_circuit::util::{not, rlc},
+    evm_circuit::{
+        util::{not, rlc},
+        EvmCircuit,
+    },
+    keccak_circuit::KeccakCircuit,
     sig_circuit::ecdsa::ecdsa_verify_no_pubkey_check,
     table::{KeccakTable, SigTable},
     util::{Challenges, Expr, SubCircuit, SubCircuitConfig},
@@ -221,6 +225,19 @@ impl<F: Field> SubCircuit<F> for SigCircuit<F> {
         }
     }
 
+    /// Returns number of unusable rows of the SubCircuit, which should be
+    /// `meta.blinding_factors() + 1`.
+    fn unusable_rows() -> usize {
+        [
+            KeccakCircuit::<F>::unusable_rows(),
+            EvmCircuit::<F>::unusable_rows(),
+            // may include additional subcircuits here
+        ]
+        .into_iter()
+        .max()
+        .unwrap()
+    }
+
     fn synthesize_sub(
         &self,
         config: &Self::Config,
@@ -235,8 +252,17 @@ impl<F: Field> SubCircuit<F> for SigCircuit<F> {
     // Since sig circuit / halo2-lib use veticle cell assignment,
     // so the returned pair is consisted of same values
     fn min_num_rows_block(block: &crate::witness::Block<F>) -> (usize, usize) {
-        let row_num = Self::min_num_rows(block.circuits_params.max_txs);
-        (row_num, row_num)
+        let row_num = Self::min_num_rows();
+
+        let tx_count = block.txs.len();
+        let max_tx_count = block.circuits_params.max_txs;
+
+        // Instead of showing actual minimum row usage,
+        // halo2-lib based circuits use min_row_num to represent a percentage of total-used capacity
+        // This functionality allows l2geth to decide if additional ops can be added.
+        let min_row_num = (row_num / max_tx_count) * tx_count;
+
+        (min_row_num, row_num)
     }
 }
 
@@ -252,32 +278,20 @@ impl<F: Field> SigCircuit<F> {
 
     /// Return the minimum number of rows required to prove an input of a
     /// particular size.
-    /// TODO: minus 256?
-    pub fn min_num_rows(num_verif: usize) -> usize {
+    pub fn min_num_rows() -> usize {
+        // SigCircuit can't determine usable rows independently.
+        // Instead, the blinding area is determined by other advise columns with most counts of
+        // rotation queries. This value is typically determined by either the Keccak or EVM
+        // circuit.
+
         // the cells are allocated vertically, i.e., given a TOTAL_NUM_ROWS * NUM_ADVICE
         // matrix, the allocator will try to use all the cells in the first column, then
         // the second column, etc.
-        let row_num = 1 << LOG_TOTAL_NUM_ROWS;
-        let col_num = calc_required_advices(num_verif);
-        if num_verif * CELLS_PER_SIG > col_num * row_num {
-            log::error!(
-                "ecdsa chip not enough rows. rows: {}, advice {}, num of sigs {}, cells per sig {}",
-                row_num,
-                col_num,
-                num_verif,
-                CELLS_PER_SIG
-            )
-        } else {
-            log::debug!(
-                "ecdsa chip: rows: {}, advice {}, num of sigs {}, cells per sig {}",
-                row_num,
-                col_num,
-                num_verif,
-                CELLS_PER_SIG
-            )
-        }
 
-        row_num
+        let max_blinding_factor = Self::unusable_rows() - 1;
+
+        // same formula as halo2-lib's FlexGate
+        (1 << LOG_TOTAL_NUM_ROWS) - (max_blinding_factor + 3)
     }
 }
 

zkevm-circuits/src/sig_circuit/utils.rs

@@ -17,7 +17,7 @@ pub(super) const MAX_NUM_SIG: usize = 32;
 // We set CELLS_PER_SIG = 535000 to allows for a few buffer
 pub(super) const CELLS_PER_SIG: usize = 535000;
 // Total number of rows allocated for ecdsa chip
-pub(super) const LOG_TOTAL_NUM_ROWS: usize = 19;
+pub(super) const LOG_TOTAL_NUM_ROWS: usize = 20;
 // Max number of columns allowed
 pub(super) const COLUMN_NUM_LIMIT: usize = 150;