refactor(ccc): less magic numbers (scroll-tech#1346)

* refactor ccc: less magic numbers * log avg_trie_depth * log avg_trie_depth * more cleanup
hoadainhan66ac · Jun 23, 2024 · e3c2e63 · e3c2e63
1 parent de413e3
commit e3c2e63
Show file tree

Hide file tree

Showing 3 changed files with 26 additions and 22 deletions.
diff --git a/prover/src/zkevm/capacity_checker.rs b/prover/src/zkevm/capacity_checker.rs
@@ -5,11 +5,13 @@ use eth_types::{
     state_db::{CodeDB, StateDB},
     ToWord, H256,
 };
+use halo2_proofs::halo2curves::bn256::Fr;
 use itertools::Itertools;
 use mpt_zktrie::state::ZktrieState;
 use serde_derive::{Deserialize, Serialize};
-use zkevm_circuits::super_circuit::params::{
-    get_sub_circuit_limit_and_confidence, get_super_circuit_params,
+use zkevm_circuits::{
+    poseidon_circuit::{Hashable, HASH_BLOCK_STEP_SIZE},
+    super_circuit::params::{get_sub_circuit_limit_and_confidence, get_super_circuit_params},
 };
 
 pub use super::SubCircuitRowUsage;
@@ -193,7 +195,7 @@ impl CircuitCapacityChecker {
                 assert_eq!(rows[2].name, "bytecode");
                 rows[2].row_number -= bytes_len + 1;
                 assert_eq!(rows[11].name, "poseidon");
-                rows[11].row_number -= bytes_len / (31 * 2) * 9;
+                rows[11].row_number -= bytes_len / HASH_BLOCK_STEP_SIZE * Fr::hash_block_size();
             }
         }
         let tx_row_usage = RowUsage::from_row_usage_details(rows);

diff --git a/prover/src/zkevm/circuit/builder.rs b/prover/src/zkevm/circuit/builder.rs
@@ -25,19 +25,20 @@ pub fn calculate_row_usage_of_witness_block(
     if witness_block.mpt_updates.smt_traces.is_empty() {
         assert_eq!(rows[11].name, "poseidon");
         assert_eq!(rows[14].name, "mpt");
-        // We collected real metrics from Scroll mainnet, and here is the graph
-        // https://ibb.co/gVfvW7h
-        // 6 is already very very conservative. Besides, considering a chunk consists of many txs,
-        // using this number is safe.
-        let poseidon_estimate_ratio = if witness_block.txs.len() > 1 {
-            // follower ccc
-            6
-        } else {
-            // singer ccc or single tx block follower ccc,
-            // even i think 6 is safe, here we still keep the old value
-            12
-        };
-        let mpt_poseidon_rows = rows[14].row_num_real * poseidon_estimate_ratio;
+
+        // TODO: make this a function parameter?
+        let is_follower = witness_block.txs.len() > 1;
+
+        // For a storage access, avg account trie depth + storage trie depth
+        // These 2 numbers are very very conservative now.
+        let avg_trie_depth = if is_follower { 64 } else { 128 };
+
+        // Fr::hash_block_size()
+        let poseidon_round_rows = 9;
+        // 96 is 3 word lookup. See comments of MptCircuit::min_num_rows_block
+        let mpt_updates_num = rows[14].row_num_real / 96;
+        let mpt_poseidon_rows = mpt_updates_num * avg_trie_depth * poseidon_round_rows;
+
         rows[11].row_num_real += mpt_poseidon_rows;
         log::debug!("calculate_row_usage_of_witness_block light mode, adding {mpt_poseidon_rows} poseidon rows");
     } else {

diff --git a/zkevm-circuits/src/poseidon_circuit.rs b/zkevm-circuits/src/poseidon_circuit.rs
@@ -9,7 +9,8 @@ use halo2_proofs::{
     circuit::{Layouter, SimpleFloorPlanner, Value},
     plonk::{Circuit, ConstraintSystem, Error},
 };
-use hash_circuit::hash::{Hashable, PoseidonHashChip, PoseidonHashConfig, PoseidonHashTable};
+pub use hash_circuit::hash::Hashable;
+use hash_circuit::hash::{PoseidonHashChip, PoseidonHashConfig, PoseidonHashTable};
 
 /// re-wrapping for mpt circuit
 #[derive(Default, Clone, Debug)]
@@ -25,7 +26,8 @@ pub struct PoseidonCircuitConfigArgs {
 #[derive(Debug, Clone)]
 pub struct PoseidonCircuitConfig<F: Field>(pub(crate) PoseidonHashConfig<F>);
 
-const HASH_BLOCK_STEP_SIZE: usize = HASHBLOCK_BYTES_IN_FIELD * PoseidonTable::INPUT_WIDTH;
+/// How many bytes a poseidon round can consume.
+pub const HASH_BLOCK_STEP_SIZE: usize = HASHBLOCK_BYTES_IN_FIELD * PoseidonTable::INPUT_WIDTH;
 
 impl<F: Field> SubCircuitConfig<F> for PoseidonCircuitConfig<F> {
     type ConfigArgs = PoseidonCircuitConfigArgs;
@@ -169,11 +171,10 @@ impl<F: Field> SubCircuit<F> for PoseidonCircuit<F> {
             * F::hash_block_size();
         let total_row_num = mpt_row_num + byte_row_num;
         log::debug!("poseidon circuit row num: {mpt_row_num}(mpt) + {byte_row_num}(bytecode) = {total_row_num}");
-        let mpt_circuit_rows = 3 * 32 * block.mpt_updates.len();
-        let poseidon_mpt_ratio = mpt_row_num as f64 / mpt_circuit_rows as f64;
+        let avg_trie_depth = after_dedup_size / block.mpt_updates.len();
         log::debug!(
-            "poseidon_mpt_ratio {poseidon_mpt_ratio:.3}, 12x naive poseidon rows {}",
-            12 * mpt_circuit_rows
+            "avg_trie_depth {avg_trie_depth}, hash num {after_dedup_size}, mpt update num {}",
+            block.mpt_updates.len()
         );
         (
             total_row_num,