Fix: allow skipping of L1Msg tx part 2 i.e. calculate num_all_txs in tx circuit

circuit-benchmarks/src/tx_circuit.rs

@@ -84,7 +84,7 @@ mod tests {
         let max_txs: usize = 2_usize.pow(degree) / ROWS_PER_TX;
 
         let txs = vec![mock::CORRECT_MOCK_TXS[0].clone().into()];
-        let circuit = TxCircuit::<Fr>::new(max_txs, MAX_CALLDATA, *mock::MOCK_CHAIN_ID, txs);
+        let circuit = TxCircuit::<Fr>::new(max_txs, MAX_CALLDATA, *mock::MOCK_CHAIN_ID, 0, txs);
         (degree as usize, circuit)
     }
 

zkevm-circuits/src/pi_circuit.rs

@@ -28,7 +28,7 @@ use crate::{
     evm_circuit::{util::constraint_builder::BaseConstraintBuilder, EvmCircuitExports},
     pi_circuit::param::{
         BASE_FEE_OFFSET, BLOCK_HEADER_BYTES_NUM, BLOCK_LEN, BLOCK_NUM_OFFSET, BYTE_POW_BASE,
-        CHAIN_ID_OFFSET, GAS_LIMIT_OFFSET, KECCAK_DIGEST_SIZE, NUM_TXS_OFFSET, RPI_CELL_IDX,
+        CHAIN_ID_OFFSET, GAS_LIMIT_OFFSET, KECCAK_DIGEST_SIZE, RPI_CELL_IDX,
         RPI_LENGTH_ACC_CELL_IDX, RPI_RLC_ACC_CELL_IDX, TIMESTAMP_OFFSET,
     },
     state_circuit::StateCircuitExports,
@@ -46,11 +46,12 @@ use once_cell::sync::Lazy;
 
 use crate::{
     evm_circuit::param::{N_BYTES_ACCOUNT_ADDRESS, N_BYTES_U64, N_BYTES_WORD},
-    pi_circuit::param::{COINBASE_OFFSET, DIFFICULTY_OFFSET},
+    pi_circuit::param::{COINBASE_OFFSET, DIFFICULTY_OFFSET, NUM_ALL_TXS_OFFSET},
     table::{
         BlockContextFieldTag,
         BlockContextFieldTag::{
-            BaseFee, ChainId, Coinbase, CumNumTxs, Difficulty, GasLimit, NumTxs, Number, Timestamp,
+            BaseFee, ChainId, Coinbase, CumNumTxs, Difficulty, GasLimit, NumAllTxs, NumTxs, Number,
+            Timestamp,
         },
     },
     util::rlc_be_bytes,
@@ -99,11 +100,12 @@ impl Default for PublicData {
 }
 
 impl PublicData {
-    fn get_num_txs(&self) -> BTreeMap<u64, u64> {
-        let mut num_txs_in_blocks = BTreeMap::new();
+    // Return num of all txs in each block (taking skipped l1 msgs into account)
+    fn get_num_all_txs(&self) -> BTreeMap<u64, u64> {
+        let mut num_all_txs_in_blocks = BTreeMap::new();
         // short for total number of l1 msgs popped before
         let mut total_l1_popped = self.start_l1_queue_index;
-        log::debug!("start_l1_queue_index: {}", total_l1_popped);
+        log::debug!("[public_data] start_l1_queue_index: {}", total_l1_popped);
         for &block_num in self.block_ctxs.ctxs.keys() {
             let num_l2_txs = self
                 .transactions
@@ -118,41 +120,41 @@ impl PublicData {
                 .map(|tx| tx.nonce)
                 .max()
                 .map_or(0, |max_queue_index| max_queue_index - total_l1_popped + 1);
-            total_l1_popped += num_l1_msgs;
 
             let num_txs = num_l2_txs + num_l1_msgs;
-            num_txs_in_blocks.insert(block_num, num_txs);
+            num_all_txs_in_blocks.insert(block_num, num_txs);
 
-            log::trace!(
-                "[block {}] total_l1_popped: {}, num_l1_msgs: {}, num_l2_txs: {}, num_txs: {}",
+            log::debug!(
+                "[public_data][block {}] total_l1_popped_before: {}, num_l1_msgs: {}, num_l2_txs: {}, num_txs: {}",
                 block_num,
                 total_l1_popped,
                 num_l1_msgs,
                 num_l2_txs,
                 num_txs
             );
+            total_l1_popped += num_l1_msgs;
         }
 
-        num_txs_in_blocks
+        num_all_txs_in_blocks
     }
 
     /// Compute the bytes for dataHash from the verifier's perspective.
     fn data_bytes(&self) -> Vec<u8> {
-        let num_txs_in_blocks = self.get_num_txs();
+        let num_all_txs_in_blocks = self.get_num_all_txs();
         let result = iter::empty()
             .chain(self.block_ctxs.ctxs.iter().flat_map(|(block_num, block)| {
-                let num_txs = num_txs_in_blocks
+                let num_all_txs = num_all_txs_in_blocks
                     .get(block_num)
                     .cloned()
-                    .unwrap_or_else(|| panic!("get num_txs in block {block_num}"))
+                    .unwrap_or_else(|| panic!("get num_all_txs in block {block_num}"))
                     as u16;
                 iter::empty()
                     // Block Values
                     .chain(block.number.as_u64().to_be_bytes())
                     .chain(block.timestamp.as_u64().to_be_bytes())
                     .chain(block.base_fee.to_be_bytes())
                     .chain(block.gas_limit.to_be_bytes())
-                    .chain(num_txs.to_be_bytes())
+                    .chain(num_all_txs.to_be_bytes())
             }))
             // Tx Hashes
             .chain(
@@ -196,7 +198,10 @@ impl PublicData {
 
     fn get_pi(&self) -> H256 {
         let data_hash = H256(keccak256(self.data_bytes()));
-        log::debug!("data hash: {}", hex::encode(data_hash.to_fixed_bytes()));
+        log::debug!(
+            "[pi] chunk data hash: {}",
+            hex::encode(data_hash.to_fixed_bytes())
+        );
 
         let pi_bytes = self.pi_bytes(data_hash);
         let pi_hash = keccak256(pi_bytes);
@@ -660,7 +665,7 @@ impl<F: Field> PiCircuitConfig<F> {
             .iter()
             .map(|tx| tx.hash)
             .collect::<Vec<H256>>();
-        let num_txs_in_blocks = public_data.get_num_txs();
+        let num_all_txs_in_blocks = public_data.get_num_all_txs();
 
         let mut offset = 0;
         let mut block_copy_cells = vec![];
@@ -693,8 +698,12 @@ impl<F: Field> PiCircuitConfig<F> {
         {
             let is_rpi_padding = i >= block_values.ctxs.len();
             let block_num = block.number.as_u64();
-            let num_txs = num_txs_in_blocks.get(&block_num).cloned().unwrap_or(0) as u16;
-            log::debug!("num_txs in block {}: {}", block_num, num_txs);
+            let num_all_txs = num_all_txs_in_blocks.get(&block_num).cloned().unwrap_or(0) as u16;
+            log::debug!(
+                "[pi assign] num_all_txs in block {}: {}",
+                block_num,
+                num_all_txs
+            );
 
             // Assign fields in pi columns and connect them to block table
             let fields = vec![
@@ -708,7 +717,7 @@ impl<F: Field> PiCircuitConfig<F> {
                 ), // timestamp
                 (block.base_fee.to_be_bytes().to_vec(), BASE_FEE_OFFSET), // base_fee
                 (block.gas_limit.to_be_bytes().to_vec(), GAS_LIMIT_OFFSET), // gas_limit
-                (num_txs.to_be_bytes().to_vec(), NUM_TXS_OFFSET),         // num_txs
+                (num_all_txs.to_be_bytes().to_vec(), NUM_ALL_TXS_OFFSET), // num_all_txs
             ];
             for (bytes, block_offset) in fields {
                 let cells = self.assign_field_in_pi(
@@ -722,15 +731,10 @@ impl<F: Field> PiCircuitConfig<F> {
                     false,
                     challenges,
                 )?;
-                // do not copy num_txs to block table as the meaning of num_txs
-                // in block table is len(block.txs), and this is different from num_l1_msgs +
-                // num_l2_txs
-                if block_offset != NUM_TXS_OFFSET {
-                    block_copy_cells.push((
-                        cells[RPI_CELL_IDX].clone(),
-                        block_table_offset + block_offset,
-                    ));
-                }
+                block_copy_cells.push((
+                    cells[RPI_CELL_IDX].clone(),
+                    block_table_offset + block_offset,
+                ));
             }
 
             block_table_offset += BLOCK_LEN;
@@ -1355,26 +1359,30 @@ impl<F: Field> PiCircuitConfig<F> {
         let mut cum_num_txs = 0usize;
         let mut block_value_cells = vec![];
         let block_ctxs = &public_data.block_ctxs;
-        // let num_txs_in_blocks = public_data.get_num_txs();
+        let num_all_txs_in_blocks = public_data.get_num_all_txs();
         for block_ctx in block_ctxs.ctxs.values().cloned().chain(
             (block_ctxs.ctxs.len()..max_inner_blocks)
                 .into_iter()
                 .map(|_| BlockContext::padding(public_data.chain_id)),
         ) {
-            // note that
             let num_txs = public_data
                 .transactions
                 .iter()
                 .filter(|tx| tx.block_number == block_ctx.number.as_u64())
                 .count();
+            // unwrap_or(0) for padding block
+            let num_all_txs = num_all_txs_in_blocks
+                .get(&block_ctx.number.as_u64())
+                .cloned()
+                .unwrap_or(0);
             let tag = [
                 Coinbase, Timestamp, Number, Difficulty, GasLimit, BaseFee, ChainId, NumTxs,
-                CumNumTxs,
+                CumNumTxs, NumAllTxs,
             ];
             let mut cum_num_txs_field = F::from(cum_num_txs as u64);
             cum_num_txs += num_txs;
             for (row, tag) in block_ctx
-                .table_assignments(num_txs, cum_num_txs, challenges)
+                .table_assignments(num_txs, cum_num_txs, num_all_txs, challenges)
                 .into_iter()
                 .zip(tag.iter())
             {

zkevm-circuits/src/pi_circuit/param.rs

@@ -1,5 +1,5 @@
 /// Fixed by the spec
-pub(super) const BLOCK_LEN: usize = 9;
+pub(super) const BLOCK_LEN: usize = 10;
 pub(super) const BYTE_POW_BASE: u64 = 256;
 pub(super) const BLOCK_HEADER_BYTES_NUM: usize = 58;
 pub(super) const KECCAK_DIGEST_SIZE: usize = 32;
@@ -19,5 +19,5 @@ pub(super) const DIFFICULTY_OFFSET: usize = 3;
 pub(super) const GAS_LIMIT_OFFSET: usize = 4;
 pub(super) const BASE_FEE_OFFSET: usize = 5;
 pub(super) const CHAIN_ID_OFFSET: usize = 6;
-pub(super) const NUM_TXS_OFFSET: usize = 7;
-pub(super) const CUM_NUM_TXS_OFFSET: usize = 8;
+// pub(super) const CUM_NUM_TXS_OFFSET: usize = 8;
+pub(super) const NUM_ALL_TXS_OFFSET: usize = 9;

zkevm-circuits/src/table.rs

@@ -1189,11 +1189,15 @@ pub enum BlockContextFieldTag {
     /// add it here for convenience.
     ChainId,
     /// In a multi-block setup, this variant represents the total number of txs
-    /// included in this block.
+    /// included (executed) in this block.
     NumTxs,
     /// In a multi-block setup, this variant represents the cumulative number of
     /// txs included up to this block, including the txs in this block.
     CumNumTxs,
+    /// In a multi-block setup, this variant represents the total number of txs
+    /// included in this block which also taking skipped l1 msgs into account.
+    /// This could possibly be larger than NumTxs.
+    NumAllTxs,
 }
 impl_expr!(BlockContextFieldTag);
 
@@ -1254,7 +1258,7 @@ impl BlockTable {
                         .filter(|tx| tx.block_number == block_ctx.number.as_u64())
                         .count();
                     cum_num_txs += num_txs;
-                    for row in block_ctx.table_assignments(num_txs, cum_num_txs, challenges) {
+                    for row in block_ctx.table_assignments(num_txs, cum_num_txs, 0, challenges) {
                         region.assign_fixed(
                             || format!("block table row {offset}"),
                             self.tag,