From d65625bc926833377ef182a670d7588452c0308e Mon Sep 17 00:00:00 2001 From: kunxian xia Date: Mon, 14 Aug 2023 20:03:10 +0800 Subject: [PATCH 01/10] calculate num_l1_msgs and num_l2_txs in tx circuit --- circuit-benchmarks/src/tx_circuit.rs | 2 +- zkevm-circuits/src/pi_circuit.rs | 3 +- zkevm-circuits/src/tx_circuit.rs | 374 ++++++++++++++++++++++++-- zkevm-circuits/src/tx_circuit/dev.rs | 13 +- zkevm-circuits/src/tx_circuit/test.rs | 32 ++- 5 files changed, 390 insertions(+), 34 deletions(-) diff --git a/circuit-benchmarks/src/tx_circuit.rs b/circuit-benchmarks/src/tx_circuit.rs index 4cf168a665..41fc55d3c6 100644 --- a/circuit-benchmarks/src/tx_circuit.rs +++ b/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::::new(max_txs, MAX_CALLDATA, *mock::MOCK_CHAIN_ID, txs); + let circuit = TxCircuit::::new(max_txs, MAX_CALLDATA, *mock::MOCK_CHAIN_ID, 0, txs); (degree as usize, circuit) } diff --git a/zkevm-circuits/src/pi_circuit.rs b/zkevm-circuits/src/pi_circuit.rs index 84c2b65153..ff5f4b7681 100644 --- a/zkevm-circuits/src/pi_circuit.rs +++ b/zkevm-circuits/src/pi_circuit.rs @@ -1361,7 +1361,8 @@ impl PiCircuitConfig { .into_iter() .map(|_| BlockContext::padding(public_data.chain_id)), ) { - // note that + // Note that the num_txs field in block table is different from num_txs + // of the chunk data hash. They are not necessarily equal. let num_txs = public_data .transactions .iter() diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index fc41d288ca..a08e9e192d 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -97,6 +97,8 @@ enum LookupCondition { pub struct TxCircuitConfig { minimum_rows: usize, + /// Only the 2nd row of tx table is enabled (as the first row is empty). + q_second: Column, tx_table: TxTable, tx_tag_bits: BinaryNumberConfig, @@ -127,6 +129,14 @@ pub struct TxCircuitConfig { is_chain_id: Column, lookup_conditions: HashMap>, + /// Columns for computing num_l1_msgs and num_l2_txs + tx_nonce: Column, + block_num: Column, + block_num_unchanged: IsEqualConfig, + num_l1_msgs: Column, + num_l2_txs: Column, + total_l1_popped_before: Column, + /// Columns for accumulating call_data_length and call_data_gas_cost /// A boolean advice column, which is turned on only for the last byte in /// call data. @@ -194,6 +204,29 @@ impl SubCircuitConfig for TxCircuitConfig { ) -> Self { let q_enable = tx_table.q_enable; + let q_second = meta.fixed_column(); + // Since we allow skipping l1 txs that could cause potential circuit overflow, + // the num_txs (num_l1_msgs + num_l2_txs) in the input to get chunk data hash + // does not necessarily equal to num_txs (self.txs.len()) in block table. + // Therefore we calculated two numbers (num_l1_msgs, num_l2_txs) in tx circuit + // and then asserts that `num_l1_msgs + num_l2_txs = num_txs` in pi circuit. + // + // | is_l1_msg | queue_index | total_l1_popped_before | num_l1_msgs | + // | true | q1 | c | q1-c+1 | + // | false | | q1+1 | q1-c+1 | + // | true | q2 | q1+1 | q2-c+1 | + // | true | q3 | q2+1 | q3-c+1 | + + let tx_nonce = meta.advice_column(); + let block_num = meta.advice_column(); + + let total_l1_popped_before = meta.advice_column(); + // num_l1_msgs takes skipped L1Msg tx into account. + let num_l1_msgs_acc = meta.advice_column(); + let num_l2_txs_acc = meta.advice_column(); + // num_all_txs = num_l1_msgs + num_l2_txs + let num_all_txs_acc = meta.advice_column(); + // tag, rlp_tag, tx_type, is_none let tx_type = meta.advice_column(); let rlp_tag = meta.advice_column(); @@ -299,8 +332,8 @@ impl SubCircuitConfig for TxCircuitConfig { |meta| meta.advice_column_in(SecondPhase), // value is at 2nd phase ); - // tx_id transition - meta.create_gate("tx_id transition", |meta| { + // tx_id transition in the fixed part of tx table + meta.create_gate("tx_id transition in the fixed part of tx table", |meta| { let mut cb = BaseConstraintBuilder::default(); // if tag_next == Nonce, then tx_id' = tx_id + 1 @@ -320,11 +353,30 @@ impl SubCircuitConfig for TxCircuitConfig { meta.query_advice(tx_table.tx_id, Rotation::next()), meta.query_advice(tx_table.tx_id, Rotation::cur()), ); - cb.require_equal( - "tx_type does not change", - meta.query_advice(tx_type, Rotation::next()), - meta.query_advice(tx_type, Rotation::cur()), - ); + // tx meta infos that extracted at some row and need to be copied to all rows of + // same tx + let tx_meta_info_fields = vec![ + ("tx_type", tx_type), // extracted at SigV row + ("is_padding_tx", is_padding_tx), // extracted at CallerAddress row + ("sv_address", sv_address), // extracted at ChainID row + ("block_num", block_num), // extracted at BlockNum row + ("num_l1_msgs", num_l1_msgs_acc), // extracted at BlockNum row + ("total_l1_popped_before", total_l1_popped_before), + ("num_l2_txs_acc", num_l2_txs_acc), + ("num_all_txs_acc", num_all_txs_acc), + // is_l1_msg does not need to spread out as it's extracted from tx_type + + // these do not need to spread out as they are related to tx_table.tag + // (which is fixed col) is_chain_id, + // is_caller_address, is_tag_block_num, is_calldata + ]; + for (col_name, meta_info) in tx_meta_info_fields { + cb.require_equal( + col_name, + meta.query_advice(meta_info, Rotation::next()), + meta.query_advice(meta_info, Rotation::cur()), + ); + } }, ); @@ -675,6 +727,169 @@ impl SubCircuitConfig for TxCircuitConfig { ])) }); + /////////////////////////////////////////////////////////////////////// + /////////////// constraints on num_l1_msgs... /////////////////////// + /////////////////////////////////////////////////////////////////////// + meta.create_gate("tx_nonce", |meta| { + let mut cb = BaseConstraintBuilder::default(); + + cb.condition(is_nonce(meta), |cb| { + cb.require_equal( + "tx_nonce = tx_table.value if tag == Nonce", + meta.query_advice(tx_table.value, Rotation::cur()), + meta.query_advice(tx_nonce, Rotation::cur()), + ); + }); + + cb.gate(meta.query_fixed(q_enable, Rotation::cur())) + }); + + let block_num_unchanged = IsEqualChip::configure( + meta, + |meta| { + and::expr([ + meta.query_fixed(q_enable, Rotation::cur()), + meta.query_advice(is_tag_block_num, Rotation::cur()), + ]) + }, + |meta| meta.query_advice(block_num, Rotation::next()), + |meta| meta.query_advice(block_num, Rotation::cur()), + ); + + meta.create_gate("block_num", |meta| { + let mut cb = BaseConstraintBuilder::default(); + + cb.condition(meta.query_advice(is_tag_block_num, Rotation::cur()), |cb| { + cb.require_equal( + "block_num = tx_table.value if tag == BlockNum", + meta.query_advice(tx_table.value, Rotation::cur()), + meta.query_advice(block_num, Rotation::cur()), + ); + }); + + cb.gate(meta.query_fixed(q_enable, Rotation::cur())) + }); + meta.create_gate("num_l1_msgs...", |meta| { + let mut cb = BaseConstraintBuilder::default(); + // first tx in tx table + cb.condition(meta.query_fixed(q_second, Rotation::cur()), |cb| { + cb.require_equal( + "first block's first tx's num_l1_msgs", + meta.query_advice(num_l1_msgs_acc, Rotation::cur()), + select::expr( + meta.query_advice(is_l1_msg, Rotation::cur()), + // first tx is l1 msg + meta.query_advice(tx_nonce, Rotation::cur()) + - meta.query_advice(total_l1_popped_before, Rotation::cur()) + + 1.expr(), + 0.expr(), + ), + ); + + cb.require_equal( + "first block's first tx's num_l2_txs", + meta.query_advice(num_l2_txs_acc, Rotation::cur()), + select::expr( + meta.query_advice(is_l1_msg, Rotation::cur()), + 0.expr(), + 1.expr(), + ), + ); + }); + + // non-last tx in cur block + cb.condition(block_num_unchanged.expr(), |cb| { + cb.require_equal( + "total_l1_popped' = tx.is_l1_msg ? queue_index + 1 : total_l1_popped", + meta.query_advice(total_l1_popped_before, Rotation::next()), + select::expr( + meta.query_advice(is_l1_msg, Rotation::cur()), + meta.query_advice(tx_nonce, Rotation::cur()) + 1.expr(), + meta.query_advice(total_l1_popped_before, Rotation::cur()), + ), + ); + + cb.require_equal( + "num_l1_msgs' - num_l1_msgs", + meta.query_advice(num_l1_msgs_acc, Rotation::next()) + - meta.query_advice(num_l1_msgs_acc, Rotation::cur()), + select::expr( + meta.query_advice(is_l1_msg, Rotation::next()), + // if next tx is l1_msg, increase by (queue_index' - tlp_before + 1) + meta.query_advice(tx_nonce, Rotation::next()) + - meta.query_advice(total_l1_popped_before, Rotation::cur()) + + 1.expr(), + // if next tx is not l1 msg, num_l1_msgs is not increased + 0.expr(), + ), + ); + + cb.require_equal( + "num_l2_txs' - num_l2_txs", + meta.query_advice(num_l2_txs_acc, Rotation::next()) + - meta.query_advice(num_l2_txs_acc, Rotation::cur()), + select::expr( + meta.query_advice(is_l1_msg, Rotation::next()), + 0.expr(), + 1.expr(), + ), + ); + }); + + // last tx in cur block (next tx is the first tx in next block) + cb.condition( + and::expr([ + not::expr(meta.query_advice(is_calldata, Rotation::next())), + not::expr(block_num_unchanged.expr()), + ]), + |cb| { + // new block's total_l1_popped does not reset. + cb.require_equal( + "total_l1_popped' = tx.is_l1_msg ? queue_index + 1 : total_l1_popped", + meta.query_advice(total_l1_popped_before, Rotation::next()), + select::expr( + meta.query_advice(is_l1_msg, Rotation::cur()), + meta.query_advice(tx_nonce, Rotation::cur()) + 1.expr(), + meta.query_advice(total_l1_popped_before, Rotation::cur()), + ), + ); + + // reset new block's num_l1_msgs + cb.require_equal( + "new block's first tx's num_l1_msgs", + meta.query_advice(num_l1_msgs_acc, Rotation::next()), + select::expr( + meta.query_advice(is_l1_msg, Rotation::next()), + // first tx is l1 msg + meta.query_advice(tx_nonce, Rotation::next()) + - meta.query_advice(total_l1_popped_before, Rotation::next()) + + 1.expr(), + 0.expr(), + ), + ); + + // reset new block's num_l2_txs + cb.require_equal( + "new block's first tx's num_l2_txs", + meta.query_advice(num_l2_txs_acc, Rotation::next()), + select::expr( + meta.query_advice(is_l1_msg, Rotation::next()), + 0.expr(), + 1.expr(), + ), + ); + }, + ); + + cb.gate(and::expr([ + meta.query_fixed(tx_table.q_enable, Rotation::cur()), + not::expr(meta.query_advice(is_padding_tx, Rotation::cur())), + not::expr(meta.query_advice(is_calldata, Rotation::cur())), + // calculate num_l1_msgs at tag = BlockNum row + meta.query_advice(is_tag_block_num, Rotation::cur()), + ])) + }); + /////////////////////////////////////////////////////////////////////// /////////////// constraints on BlockNum ///////////////////////////// /////////////////////////////////////////////////////////////////////// @@ -682,13 +897,11 @@ impl SubCircuitConfig for TxCircuitConfig { let is_tag_caller_addr = is_caller_addr(meta); let mut cb = BaseConstraintBuilder::default(); - // the offset between CallerAddress and BlockNumber - let offset = usize::from(BlockNumber) - usize::from(CallerAddress); // if tag == CallerAddress cb.condition(is_tag_caller_addr.expr(), |cb| { cb.require_equal( "is_padding_tx = true if caller_address = 0", - meta.query_advice(is_padding_tx, Rotation(offset as i32)), + meta.query_advice(is_padding_tx, Rotation::cur()), value_is_zero.expr(Rotation::cur())(meta), ); }); @@ -885,6 +1098,7 @@ impl SubCircuitConfig for TxCircuitConfig { log_deg("tx_circuit", meta); Self { + q_second, minimum_rows: meta.minimum_rows(), tx_tag_bits: tag_bits, tx_type, @@ -902,6 +1116,12 @@ impl SubCircuitConfig for TxCircuitConfig { cum_num_txs, is_padding_tx, lookup_conditions, + tx_nonce, + block_num, + block_num_unchanged, + num_l1_msgs: num_l1_msgs_acc, + num_l2_txs: num_l2_txs_acc, + total_l1_popped_before, is_l1_msg, is_chain_id, is_final, @@ -1208,6 +1428,11 @@ impl TxCircuitConfig { cum_num_txs: Option, is_final: Option, calldata_gas_cost_acc: Option, + cur_block_num: Option, + next_block_number: Option, + num_l1_msgs: Option, + num_l2_txs: Option, + total_l1_popped_before: Option, ) -> Result<(), Error> { // assign to tag, rlp_tag, is_none let tag_chip = BinaryNumberChip::construct(self.tx_tag_bits); @@ -1234,6 +1459,44 @@ impl TxCircuitConfig { || Value::known(F::from(is_none.unwrap_or(false) as u64)), )?; + let block_num_unchanged_chip = IsEqualChip::construct(self.block_num_unchanged.clone()); + block_num_unchanged_chip.assign( + region, + *offset, + Value::known(F::from(next_block_number.unwrap_or(0))), + Value::known(F::from(cur_block_num.unwrap_or(0))), + )?; + region.assign_advice( + || "tx_nonce", + self.tx_nonce, + *offset, + || Value::known(F::from(tx.map_or(0, |tx| tx.nonce))), + )?; + region.assign_advice( + || "block_num", + self.block_num, + *offset, + || Value::known(F::from(cur_block_num.unwrap_or(0))), + )?; + region.assign_advice( + || "num_l1_msgs", + self.num_l1_msgs, + *offset, + || Value::known(F::from(num_l1_msgs.unwrap_or(0))), + )?; + region.assign_advice( + || "num_l2_txs", + self.num_l2_txs, + *offset, + || Value::known(F::from(num_l2_txs.unwrap_or(0))), + )?; + region.assign_advice( + || "total_l1_popped_before", + self.total_l1_popped_before, + *offset, + || Value::known(F::from(total_l1_popped_before.unwrap_or(0))), + )?; + // assign to lookup condition columns let is_l1_msg = tx.map(|tx| tx.tx_type.is_l1_msg()).unwrap_or(false); let mut conditions = HashMap::>::new(); @@ -1505,6 +1768,8 @@ pub struct TxCircuit { pub txs: Vec, /// Chain ID pub chain_id: u64, + /// Start L1 Queue Index + pub start_l1_queue_index: u64, /// Size pub size: usize, _marker: PhantomData, @@ -1512,7 +1777,13 @@ pub struct TxCircuit { impl TxCircuit { /// Return a new TxCircuit - pub fn new(max_txs: usize, max_calldata: usize, chain_id: u64, txs: Vec) -> Self { + pub fn new( + max_txs: usize, + max_calldata: usize, + chain_id: u64, + start_l1_queue_index: u64, + txs: Vec, + ) -> Self { log::info!( "TxCircuit::new(max_txs = {}, max_calldata = {}, chain_id = {})", max_txs, @@ -1527,6 +1798,7 @@ impl TxCircuit { txs, size: Self::min_num_rows(max_txs, max_calldata), chain_id, + start_l1_queue_index, _marker: PhantomData::default(), } } @@ -1635,6 +1907,7 @@ impl TxCircuit { config: &TxCircuitConfig, challenges: &crate::util::Challenges>, layouter: &mut impl Layouter, + start_l1_queue_index: u64, sign_datas: Vec, padding_txs: &[Transaction], ) -> Result<(), Error> { @@ -1649,6 +1922,9 @@ impl TxCircuit { let mut cum_num_txs; let mut is_padding_tx; + let mut num_l2_txs = 0; + let mut num_l1_msgs = 0; + let mut total_l1_popped_before = start_l1_queue_index; // Empty entry config.assign_row( &mut region, @@ -1664,6 +1940,11 @@ impl TxCircuit { None, None, None, + None, + None, + None, + None, + None, )?; // Assign all tx fields except for call data @@ -1682,9 +1963,32 @@ impl TxCircuit { .filter(|tx| tx.block_number <= self.txs[i].block_number) .count(); is_padding_tx = false; + let mut reset_new_block = |tx: &Transaction| { + if tx.tx_type.is_l1_msg() { + num_l2_txs = 0; + num_l1_msgs = tx.nonce - total_l1_popped_before + 1; + total_l1_popped_before = tx.nonce + 1; + } else { + // num_l1_msgs and total_l1_popped_before do not change + num_l2_txs = 1; + } + }; + if i == 0 || tx.block_number != self.txs[i - 1].block_number { + reset_new_block(tx); + } else { + // same block + if tx.tx_type.is_l1_msg() { + num_l1_msgs += tx.nonce - total_l1_popped_before + 1; + total_l1_popped_before = tx.nonce + 1; + } else { + num_l2_txs += 1; + } + } } else { cum_num_txs = 0; is_padding_tx = true; + num_l1_msgs = 0; + num_l2_txs = (i - self.txs.len() + 1) as u64; } let tx_sign_hash = { @@ -1856,20 +2160,30 @@ impl TxCircuit { Value::known(F::from(tx.block_number)), ), ] { - let tx_id_next = match tag { + let (tx_id_next, cur_block_num, next_block_num) = match tag { BlockNumber => { if i == sigs.len() - 1 { - self.txs - .iter() - .enumerate() - .find(|(_i, tx)| !tx.call_data.is_empty()) - .map(|(i, _tx)| i + 1) - .unwrap_or_else(|| 0) + ( + self.txs + .iter() + .enumerate() + .find(|(_i, tx)| !tx.call_data.is_empty()) + .map(|(i, _tx)| i + 1) + .unwrap_or_else(|| 0), + tx.block_number, + 0, + ) } else { - i + 2 + // tx_id in tx table starts with 1 + if i + 1 >= self.txs.len() { + (i + 2, tx.block_number, padding_txs[0].block_number) + } else { + (i + 2, tx.block_number, self.txs[i + 1].block_number) + } } } - _ => i + 1, + _ => (i + 1, tx.block_number, tx.block_number), /* tx_id in tx table + * starts with 1 */ }; config.assign_row( &mut region, @@ -1885,6 +2199,11 @@ impl TxCircuit { Some(cum_num_txs), None, None, + Some(cur_block_num), + Some(next_block_num), + Some(num_l1_msgs), + Some(num_l2_txs), + Some(total_l1_popped_before), )?; let sv_address: F = sign_data.get_addr().to_scalar().unwrap(); region.assign_advice( @@ -1939,6 +2258,11 @@ impl TxCircuit { None, Some(is_final), Some(calldata_gas_cost), + None, + None, + None, + None, + None, )?; } } @@ -1996,6 +2320,7 @@ impl SubCircuit for TxCircuit { block.circuits_params.max_txs, block.circuits_params.max_calldata, block.chain_id, + block.start_l1_queue_index, block.txs.clone(), ) } @@ -2072,7 +2397,14 @@ impl SubCircuit for TxCircuit { } } - self.assign(config, challenges, layouter, sign_datas, &padding_txs)?; + self.assign( + config, + challenges, + layouter, + self.start_l1_queue_index, + sign_datas, + &padding_txs, + )?; Ok(()) } diff --git a/zkevm-circuits/src/tx_circuit/dev.rs b/zkevm-circuits/src/tx_circuit/dev.rs index 7c6fa7057a..75482551b4 100644 --- a/zkevm-circuits/src/tx_circuit/dev.rs +++ b/zkevm-circuits/src/tx_circuit/dev.rs @@ -106,14 +106,20 @@ pub struct TxCircuitTester { impl TxCircuitTester { /// Return a new TxCircuit - pub fn new(max_txs: usize, max_calldata: usize, chain_id: u64, txs: Vec) -> Self { + pub fn new( + max_txs: usize, + max_calldata: usize, + chain_id: u64, + start_l1_queue_index: u64, + txs: Vec, + ) -> Self { TxCircuitTester:: { sig_circuit: SigCircuit { max_verif: max_txs, signatures: get_sign_data(&txs, max_txs, chain_id as usize).unwrap(), _marker: PhantomData, }, - tx_circuit: TxCircuit::new(max_txs, max_calldata, chain_id, txs), + tx_circuit: TxCircuit::new(max_txs, max_calldata, chain_id, start_l1_queue_index, txs), } } } @@ -126,7 +132,8 @@ impl SubCircuit for TxCircuitTester { let max_txs = block.circuits_params.max_txs; let chain_id = block.chain_id; let max_calldata = block.circuits_params.max_calldata; - Self::new(max_txs, max_calldata, chain_id, txs) + let start_l1_queue_index = block.start_l1_queue_index; + Self::new(max_txs, max_calldata, chain_id, start_l1_queue_index, txs) } fn synthesize_sub( diff --git a/zkevm-circuits/src/tx_circuit/test.rs b/zkevm-circuits/src/tx_circuit/test.rs index 165e3bbb90..0bc0ab5f8d 100644 --- a/zkevm-circuits/src/tx_circuit/test.rs +++ b/zkevm-circuits/src/tx_circuit/test.rs @@ -113,6 +113,7 @@ fn run( chain_id: u64, max_txs: usize, max_calldata: usize, + start_l1_queue_index: u64, ) -> Result<(), Vec> { let active_row_num = TxCircuit::::min_num_rows(max_txs, max_calldata); @@ -123,13 +124,14 @@ fn run( signatures: get_sign_data(&txs, max_txs, chain_id as usize).unwrap(), _marker: PhantomData, }, - tx_circuit: TxCircuit::new(max_txs, max_calldata, chain_id, txs), + tx_circuit: TxCircuit::new(max_txs, max_calldata, chain_id, start_l1_queue_index, txs), }; let prover = match MockProver::run(k, &circuit, vec![]) { Ok(prover) => prover, Err(e) => panic!("{e:#?}"), }; + prover.assert_satisfied_par(); prover.verify_at_rows_par(0..active_row_num, 0..active_row_num) } @@ -156,7 +158,8 @@ fn tx_circuit_2tx_2max_tx() { .collect(), *mock::MOCK_CHAIN_ID, MAX_TXS, - MAX_CALLDATA + MAX_CALLDATA, + 0, ), Ok(()) ); @@ -169,7 +172,7 @@ fn tx_circuit_0tx_1max_tx() { const MAX_CALLDATA: usize = 32; assert_eq!( - run::(vec![], *mock::MOCK_CHAIN_ID, MAX_TXS, MAX_CALLDATA), + run::(vec![], *mock::MOCK_CHAIN_ID, MAX_TXS, MAX_CALLDATA, 0), Ok(()) ); } @@ -183,7 +186,7 @@ fn tx_circuit_1tx_1max_tx() { let tx: Transaction = mock::CORRECT_MOCK_TXS[0].clone().into(); assert_eq!( - run::(vec![tx], *mock::MOCK_CHAIN_ID, MAX_TXS, MAX_CALLDATA), + run::(vec![tx], *mock::MOCK_CHAIN_ID, MAX_TXS, MAX_CALLDATA, 0), Ok(()) ); } @@ -197,7 +200,7 @@ fn tx_circuit_1tx_2max_tx() { let tx = build_pre_eip155_tx(); assert_eq!( - run::(vec![tx], *mock::MOCK_CHAIN_ID, MAX_TXS, MAX_CALLDATA), + run::(vec![tx], *mock::MOCK_CHAIN_ID, MAX_TXS, MAX_CALLDATA, 0), Ok(()) ); } @@ -211,7 +214,7 @@ fn tx_circuit_l1_msg_tx() { let tx = build_l1_msg_tx(); assert_eq!( - run::(vec![tx], *mock::MOCK_CHAIN_ID, MAX_TXS, MAX_CALLDATA), + run::(vec![tx], *mock::MOCK_CHAIN_ID, MAX_TXS, MAX_CALLDATA, 0), Ok(()) ); } @@ -226,7 +229,14 @@ fn tx_circuit_bad_address() { // This address doesn't correspond to the account that signed this tx. tx.from = AddrOrWallet::from(address!("0x1230000000000000000000000000000000000456")); - assert!(run::(vec![tx.into()], *mock::MOCK_CHAIN_ID, MAX_TXS, MAX_CALLDATA).is_err(),); + assert!(run::( + vec![tx.into()], + *mock::MOCK_CHAIN_ID, + MAX_TXS, + MAX_CALLDATA, + 0 + ) + .is_err(),); } #[test] @@ -239,7 +249,13 @@ fn tx_circuit_to_is_zero() { tx.transaction_index = U64::from(1); assert_eq!( - run::(vec![tx.into()], *mock::MOCK_CHAIN_ID, MAX_TXS, MAX_CALLDATA), + run::( + vec![tx.into()], + *mock::MOCK_CHAIN_ID, + MAX_TXS, + MAX_CALLDATA, + 0 + ), Ok(()) ); } From b7bcbebf64174cfcfcaf438904f1b5d4913fb528 Mon Sep 17 00:00:00 2001 From: kunxian xia Date: Tue, 15 Aug 2023 00:47:18 +0800 Subject: [PATCH 02/10] fix --- zkevm-circuits/src/tx_circuit.rs | 173 +++++++++++++++---------------- 1 file changed, 81 insertions(+), 92 deletions(-) diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index a08e9e192d..83865fc4b5 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -133,8 +133,7 @@ pub struct TxCircuitConfig { tx_nonce: Column, block_num: Column, block_num_unchanged: IsEqualConfig, - num_l1_msgs: Column, - num_l2_txs: Column, + num_all_txs_acc: Column, total_l1_popped_before: Column, /// Columns for accumulating call_data_length and call_data_gas_cost @@ -206,10 +205,13 @@ impl SubCircuitConfig for TxCircuitConfig { let q_second = meta.fixed_column(); // Since we allow skipping l1 txs that could cause potential circuit overflow, - // the num_txs (num_l1_msgs + num_l2_txs) in the input to get chunk data hash + // the num_all_txs (num_l1_msgs + num_l2_txs) in the input to get chunk data hash // does not necessarily equal to num_txs (self.txs.len()) in block table. // Therefore we calculated two numbers (num_l1_msgs, num_l2_txs) in tx circuit - // and then asserts that `num_l1_msgs + num_l2_txs = num_txs` in pi circuit. + // and then asserts that `num_l1_msgs + num_l2_txs = num_all_txs` in pi circuit. + // + // In more detail, all txs in same block are grouped together and we iterate over + // its txs to get `num_all_txs`. // // | is_l1_msg | queue_index | total_l1_popped_before | num_l1_msgs | // | true | q1 | c | q1-c+1 | @@ -360,9 +362,7 @@ impl SubCircuitConfig for TxCircuitConfig { ("is_padding_tx", is_padding_tx), // extracted at CallerAddress row ("sv_address", sv_address), // extracted at ChainID row ("block_num", block_num), // extracted at BlockNum row - ("num_l1_msgs", num_l1_msgs_acc), // extracted at BlockNum row ("total_l1_popped_before", total_l1_popped_before), - ("num_l2_txs_acc", num_l2_txs_acc), ("num_all_txs_acc", num_all_txs_acc), // is_l1_msg does not need to spread out as it's extracted from tx_type @@ -728,7 +728,7 @@ impl SubCircuitConfig for TxCircuitConfig { }); /////////////////////////////////////////////////////////////////////// - /////////////// constraints on num_l1_msgs... /////////////////////// + /////////////// constraints on num_all_txs // /////////////////////// /////////////////////////////////////////////////////////////////////// meta.create_gate("tx_nonce", |meta| { let mut cb = BaseConstraintBuilder::default(); @@ -744,18 +744,6 @@ impl SubCircuitConfig for TxCircuitConfig { cb.gate(meta.query_fixed(q_enable, Rotation::cur())) }); - let block_num_unchanged = IsEqualChip::configure( - meta, - |meta| { - and::expr([ - meta.query_fixed(q_enable, Rotation::cur()), - meta.query_advice(is_tag_block_num, Rotation::cur()), - ]) - }, - |meta| meta.query_advice(block_num, Rotation::next()), - |meta| meta.query_advice(block_num, Rotation::cur()), - ); - meta.create_gate("block_num", |meta| { let mut cb = BaseConstraintBuilder::default(); @@ -769,29 +757,37 @@ impl SubCircuitConfig for TxCircuitConfig { cb.gate(meta.query_fixed(q_enable, Rotation::cur())) }); - meta.create_gate("num_l1_msgs...", |meta| { + + // block num is the last row of each tx's fixed rows and since block num is + // copied to TX_LEN rows. The row at which tag = BlockNum and tx_id = i, + // its next row has tx_id = i+1. That is, we can use Rotation::next() to get next + // tx's all meta-infos (including block_num, tx_nonce, num_all_txs_acc, ...) + let block_num_unchanged = IsEqualChip::configure( + meta, + |meta| { + and::expr([ + meta.query_fixed(q_enable, Rotation::cur()), + meta.query_advice(is_tag_block_num, Rotation::cur()), + ]) + }, + |meta| meta.query_advice(block_num, Rotation::next()), + |meta| meta.query_advice(block_num, Rotation::cur()), + ); + + meta.create_gate("num_all_txs in a block", |meta| { let mut cb = BaseConstraintBuilder::default(); + let queue_index = tx_nonce; // first tx in tx table cb.condition(meta.query_fixed(q_second, Rotation::cur()), |cb| { cb.require_equal( "first block's first tx's num_l1_msgs", - meta.query_advice(num_l1_msgs_acc, Rotation::cur()), + meta.query_advice(num_all_txs_acc, Rotation::cur()), select::expr( meta.query_advice(is_l1_msg, Rotation::cur()), // first tx is l1 msg - meta.query_advice(tx_nonce, Rotation::cur()) + meta.query_advice(queue_index, Rotation::cur()) - meta.query_advice(total_l1_popped_before, Rotation::cur()) + 1.expr(), - 0.expr(), - ), - ); - - cb.require_equal( - "first block's first tx's num_l2_txs", - meta.query_advice(num_l2_txs_acc, Rotation::cur()), - select::expr( - meta.query_advice(is_l1_msg, Rotation::cur()), - 0.expr(), 1.expr(), ), ); @@ -804,88 +800,85 @@ impl SubCircuitConfig for TxCircuitConfig { meta.query_advice(total_l1_popped_before, Rotation::next()), select::expr( meta.query_advice(is_l1_msg, Rotation::cur()), - meta.query_advice(tx_nonce, Rotation::cur()) + 1.expr(), + meta.query_advice(queue_index, Rotation::cur()) + 1.expr(), meta.query_advice(total_l1_popped_before, Rotation::cur()), ), ); cb.require_equal( - "num_l1_msgs' - num_l1_msgs", - meta.query_advice(num_l1_msgs_acc, Rotation::next()) - - meta.query_advice(num_l1_msgs_acc, Rotation::cur()), + "num_all_txs' - num_all_txs", + meta.query_advice(num_all_txs_acc, Rotation::next()) + - meta.query_advice(num_all_txs_acc, Rotation::cur()), select::expr( meta.query_advice(is_l1_msg, Rotation::next()), - // if next tx is l1_msg, increase by (queue_index' - tlp_before + 1) + // if next tx is l1_msg, + // - num_l1_msgs increase by (queue_index' - tlp_before + 1) + // - num_l2_txs does not increase meta.query_advice(tx_nonce, Rotation::next()) - meta.query_advice(total_l1_popped_before, Rotation::cur()) + 1.expr(), - // if next tx is not l1 msg, num_l1_msgs is not increased - 0.expr(), - ), - ); - - cb.require_equal( - "num_l2_txs' - num_l2_txs", - meta.query_advice(num_l2_txs_acc, Rotation::next()) - - meta.query_advice(num_l2_txs_acc, Rotation::cur()), - select::expr( - meta.query_advice(is_l1_msg, Rotation::next()), - 0.expr(), + // if next tx is l2 tx, + // - num_l1_msgs does not increase, + // - num_l2_txs increase by 1. 1.expr(), ), ); }); // last tx in cur block (next tx is the first tx in next block) + // and cur block is not the last block (s.t. we can init next block's num_all_txs) cb.condition( and::expr([ + // We need this condition because if this is the last tx of fixed part of tx + // table, not(block_num_unchanged.expr()) is very likely to + // be true. Since it does not make sense to assign values + // to `num_all_txs` col in the calldata part of tx table. + // Therefore we can skip assign any values to fixed part related cols + // (e.g. block_num, tx_type, is_padding_tx, ....). The witness assignment of + // calldata part need only make sure that (is_final, + // calldata_gas_cost_acc) are correctly assigned. not::expr(meta.query_advice(is_calldata, Rotation::next())), not::expr(block_num_unchanged.expr()), ]), |cb| { - // new block's total_l1_popped does not reset. cb.require_equal( "total_l1_popped' = tx.is_l1_msg ? queue_index + 1 : total_l1_popped", meta.query_advice(total_l1_popped_before, Rotation::next()), select::expr( meta.query_advice(is_l1_msg, Rotation::cur()), - meta.query_advice(tx_nonce, Rotation::cur()) + 1.expr(), + meta.query_advice(queue_index, Rotation::cur()) + 1.expr(), meta.query_advice(total_l1_popped_before, Rotation::cur()), ), ); - // reset new block's num_l1_msgs + // init new block's num_all_txs cb.require_equal( - "new block's first tx's num_l1_msgs", - meta.query_advice(num_l1_msgs_acc, Rotation::next()), + "init new block's num_all_txs", + meta.query_advice(num_all_txs_acc, Rotation::next()), select::expr( meta.query_advice(is_l1_msg, Rotation::next()), // first tx is l1 msg + // - num_l1_msgs equals to (queue_index' - tlp_before + 1) + // - num_l2_txs is zero meta.query_advice(tx_nonce, Rotation::next()) - meta.query_advice(total_l1_popped_before, Rotation::next()) + 1.expr(), - 0.expr(), - ), - ); - - // reset new block's num_l2_txs - cb.require_equal( - "new block's first tx's num_l2_txs", - meta.query_advice(num_l2_txs_acc, Rotation::next()), - select::expr( - meta.query_advice(is_l1_msg, Rotation::next()), - 0.expr(), + // first tx is l2 tx + // - num_l1_msgs is zero + // - num_l2_txs is one 1.expr(), ), ); }, ); + // no constraints on last tx in the fixed part of tx table + cb.gate(and::expr([ meta.query_fixed(tx_table.q_enable, Rotation::cur()), - not::expr(meta.query_advice(is_padding_tx, Rotation::cur())), + // we are in the fixed part of tx table not::expr(meta.query_advice(is_calldata, Rotation::cur())), - // calculate num_l1_msgs at tag = BlockNum row + // calculate num_all_txs at tag = BlockNum row meta.query_advice(is_tag_block_num, Rotation::cur()), ])) }); @@ -1430,8 +1423,7 @@ impl TxCircuitConfig { calldata_gas_cost_acc: Option, cur_block_num: Option, next_block_number: Option, - num_l1_msgs: Option, - num_l2_txs: Option, + num_all_txs_acc: Option, total_l1_popped_before: Option, ) -> Result<(), Error> { // assign to tag, rlp_tag, is_none @@ -1479,17 +1471,12 @@ impl TxCircuitConfig { || Value::known(F::from(cur_block_num.unwrap_or(0))), )?; region.assign_advice( - || "num_l1_msgs", - self.num_l1_msgs, - *offset, - || Value::known(F::from(num_l1_msgs.unwrap_or(0))), - )?; - region.assign_advice( - || "num_l2_txs", - self.num_l2_txs, + || "num_all_txs_acc", + self.num_all_txs_acc, *offset, - || Value::known(F::from(num_l2_txs.unwrap_or(0))), + || Value::known(F::from(num_all_txs_acc.unwrap_or(0))), )?; + region.assign_advice( || "total_l1_popped_before", self.total_l1_popped_before, @@ -1922,8 +1909,7 @@ impl TxCircuit { let mut cum_num_txs; let mut is_padding_tx; - let mut num_l2_txs = 0; - let mut num_l1_msgs = 0; + let mut num_all_txs_acc = 0; let mut total_l1_popped_before = start_l1_queue_index; // Empty entry config.assign_row( @@ -1963,32 +1949,35 @@ impl TxCircuit { .filter(|tx| tx.block_number <= self.txs[i].block_number) .count(); is_padding_tx = false; - let mut reset_new_block = |tx: &Transaction| { + let mut init_new_block = |tx: &Transaction| { if tx.tx_type.is_l1_msg() { - num_l2_txs = 0; - num_l1_msgs = tx.nonce - total_l1_popped_before + 1; - total_l1_popped_before = tx.nonce + 1; + let queue_index = tx.nonce; + num_all_txs_acc = queue_index - total_l1_popped_before + 1; + total_l1_popped_before = queue_index + 1; } else { - // num_l1_msgs and total_l1_popped_before do not change - num_l2_txs = 1; + // total_l1_popped_before do not change + num_all_txs_acc = 1; } }; + // first tx of all or first tx of next block if i == 0 || tx.block_number != self.txs[i - 1].block_number { - reset_new_block(tx); + init_new_block(tx); } else { // same block if tx.tx_type.is_l1_msg() { - num_l1_msgs += tx.nonce - total_l1_popped_before + 1; - total_l1_popped_before = tx.nonce + 1; + let queue_index = tx.nonce; + num_all_txs_acc += queue_index - total_l1_popped_before + 1; + total_l1_popped_before = queue_index + 1; } else { - num_l2_txs += 1; + // total_l1_popped_before do not change + num_all_txs_acc += 1; } } } else { cum_num_txs = 0; is_padding_tx = true; - num_l1_msgs = 0; - num_l2_txs = (i - self.txs.len() + 1) as u64; + // padding_tx is an l2 tx + num_all_txs_acc = (i - self.txs.len() + 1) as u64; } let tx_sign_hash = { From d2f273e014978d51c656bbaf2afb971681c3132a Mon Sep 17 00:00:00 2001 From: kunxian xia Date: Tue, 15 Aug 2023 00:51:12 +0800 Subject: [PATCH 03/10] fmt and clippy --- zkevm-circuits/src/tx_circuit.rs | 11 ++--------- zkevm-circuits/src/tx_circuit/test.rs | 1 - 2 files changed, 2 insertions(+), 10 deletions(-) diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index 83865fc4b5..26c5bc6b3f 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -223,9 +223,6 @@ impl SubCircuitConfig for TxCircuitConfig { let block_num = meta.advice_column(); let total_l1_popped_before = meta.advice_column(); - // num_l1_msgs takes skipped L1Msg tx into account. - let num_l1_msgs_acc = meta.advice_column(); - let num_l2_txs_acc = meta.advice_column(); // num_all_txs = num_l1_msgs + num_l2_txs let num_all_txs_acc = meta.advice_column(); @@ -1112,8 +1109,7 @@ impl SubCircuitConfig for TxCircuitConfig { tx_nonce, block_num, block_num_unchanged, - num_l1_msgs: num_l1_msgs_acc, - num_l2_txs: num_l2_txs_acc, + num_all_txs_acc, total_l1_popped_before, is_l1_msg, is_chain_id, @@ -1930,7 +1926,6 @@ impl TxCircuit { None, None, None, - None, )?; // Assign all tx fields except for call data @@ -2190,8 +2185,7 @@ impl TxCircuit { None, Some(cur_block_num), Some(next_block_num), - Some(num_l1_msgs), - Some(num_l2_txs), + Some(num_all_txs_acc), Some(total_l1_popped_before), )?; let sv_address: F = sign_data.get_addr().to_scalar().unwrap(); @@ -2251,7 +2245,6 @@ impl TxCircuit { None, None, None, - None, )?; } } diff --git a/zkevm-circuits/src/tx_circuit/test.rs b/zkevm-circuits/src/tx_circuit/test.rs index 0bc0ab5f8d..f1608c708f 100644 --- a/zkevm-circuits/src/tx_circuit/test.rs +++ b/zkevm-circuits/src/tx_circuit/test.rs @@ -131,7 +131,6 @@ fn run( Err(e) => panic!("{e:#?}"), }; - prover.assert_satisfied_par(); prover.verify_at_rows_par(0..active_row_num, 0..active_row_num) } From 9479168f7a3e4d0db69f57a3ae688ba3339d4fa4 Mon Sep 17 00:00:00 2001 From: kunxian xia Date: Tue, 15 Aug 2023 01:18:12 +0800 Subject: [PATCH 04/10] fix: non-last tx requires next is calldata --- zkevm-circuits/src/tx_circuit.rs | 65 ++++++++++++++++++-------------- 1 file changed, 36 insertions(+), 29 deletions(-) diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index 26c5bc6b3f..6eb7519b69 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -791,36 +791,43 @@ impl SubCircuitConfig for TxCircuitConfig { }); // non-last tx in cur block - cb.condition(block_num_unchanged.expr(), |cb| { - cb.require_equal( - "total_l1_popped' = tx.is_l1_msg ? queue_index + 1 : total_l1_popped", - meta.query_advice(total_l1_popped_before, Rotation::next()), - select::expr( - meta.query_advice(is_l1_msg, Rotation::cur()), - meta.query_advice(queue_index, Rotation::cur()) + 1.expr(), - meta.query_advice(total_l1_popped_before, Rotation::cur()), - ), - ); + cb.condition( + and::expr([ + // see the comment below + not::expr(meta.query_advice(is_calldata, Rotation::next())), + block_num_unchanged.expr(), + ]), + |cb| { + cb.require_equal( + "total_l1_popped' = tx.is_l1_msg ? queue_index + 1 : total_l1_popped", + meta.query_advice(total_l1_popped_before, Rotation::next()), + select::expr( + meta.query_advice(is_l1_msg, Rotation::cur()), + meta.query_advice(queue_index, Rotation::cur()) + 1.expr(), + meta.query_advice(total_l1_popped_before, Rotation::cur()), + ), + ); - cb.require_equal( - "num_all_txs' - num_all_txs", - meta.query_advice(num_all_txs_acc, Rotation::next()) - - meta.query_advice(num_all_txs_acc, Rotation::cur()), - select::expr( - meta.query_advice(is_l1_msg, Rotation::next()), - // if next tx is l1_msg, - // - num_l1_msgs increase by (queue_index' - tlp_before + 1) - // - num_l2_txs does not increase - meta.query_advice(tx_nonce, Rotation::next()) - - meta.query_advice(total_l1_popped_before, Rotation::cur()) - + 1.expr(), - // if next tx is l2 tx, - // - num_l1_msgs does not increase, - // - num_l2_txs increase by 1. - 1.expr(), - ), - ); - }); + cb.require_equal( + "num_all_txs' - num_all_txs", + meta.query_advice(num_all_txs_acc, Rotation::next()) + - meta.query_advice(num_all_txs_acc, Rotation::cur()), + select::expr( + meta.query_advice(is_l1_msg, Rotation::next()), + // if next tx is l1_msg, + // - num_l1_msgs increase by (queue_index' - tlp_before + 1) + // - num_l2_txs does not increase + meta.query_advice(tx_nonce, Rotation::next()) + - meta.query_advice(total_l1_popped_before, Rotation::cur()) + + 1.expr(), + // if next tx is l2 tx, + // - num_l1_msgs does not increase, + // - num_l2_txs increase by 1. + 1.expr(), + ), + ); + }, + ); // last tx in cur block (next tx is the first tx in next block) // and cur block is not the last block (s.t. we can init next block's num_all_txs) From 342eefe9cf6ad3b7d5125537e20d66643da2c5a5 Mon Sep 17 00:00:00 2001 From: kunxian xia Date: Tue, 15 Aug 2023 13:38:59 +0800 Subject: [PATCH 05/10] add NumAllTxs in block table and copy it from pi to block table --- zkevm-circuits/src/pi_circuit.rs | 75 ++++++++++++++------------ zkevm-circuits/src/pi_circuit/param.rs | 6 +-- zkevm-circuits/src/table.rs | 8 ++- zkevm-circuits/src/witness/block.rs | 6 +++ 4 files changed, 56 insertions(+), 39 deletions(-) diff --git a/zkevm-circuits/src/pi_circuit.rs b/zkevm-circuits/src/pi_circuit.rs index ff5f4b7681..968e9c3924 100644 --- a/zkevm-circuits/src/pi_circuit.rs +++ b/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 { - 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 { + 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,33 +120,33 @@ 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 { - 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 @@ -152,7 +154,7 @@ impl PublicData { .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 PiCircuitConfig { .iter() .map(|tx| tx.hash) .collect::>(); - 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 PiCircuitConfig { { 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 PiCircuitConfig { ), // 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 PiCircuitConfig { 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,27 +1359,30 @@ impl PiCircuitConfig { 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 the num_txs field in block table is different from num_txs - // of the chunk data hash. They are not necessarily equal. 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()) { diff --git a/zkevm-circuits/src/pi_circuit/param.rs b/zkevm-circuits/src/pi_circuit/param.rs index a9ffc7e93c..0a3c7563f6 100644 --- a/zkevm-circuits/src/pi_circuit/param.rs +++ b/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; diff --git a/zkevm-circuits/src/table.rs b/zkevm-circuits/src/table.rs index 2086483944..c05162aaa9 100644 --- a/zkevm-circuits/src/table.rs +++ b/zkevm-circuits/src/table.rs @@ -1192,11 +1192,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); @@ -1257,7 +1261,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, diff --git a/zkevm-circuits/src/witness/block.rs b/zkevm-circuits/src/witness/block.rs index 3ad812c8e8..2680c113e9 100644 --- a/zkevm-circuits/src/witness/block.rs +++ b/zkevm-circuits/src/witness/block.rs @@ -228,6 +228,7 @@ impl BlockContext { &self, num_txs: usize, cum_num_txs: usize, + num_all_txs: u64, challenges: &Challenges>, ) -> Vec<[Value; 3]> { let current_block_number = self.number.to_scalar().unwrap(); @@ -280,6 +281,11 @@ impl BlockContext { Value::known(current_block_number), Value::known(F::from(cum_num_txs as u64)), ], + [ + Value::known(F::from(BlockContextFieldTag::NumAllTxs as u64)), + Value::known(current_block_number), + Value::known(F::from(num_all_txs as u64)), + ], ], self.block_hash_assignments(randomness), ] From 9452d704caed7ea35d2964753749fcc69a4e03a7 Mon Sep 17 00:00:00 2001 From: kunxian xia Date: Tue, 15 Aug 2023 13:45:52 +0800 Subject: [PATCH 06/10] add lookup for NumAllTxs in tx circuit --- zkevm-circuits/src/tx_circuit.rs | 99 +++++++++++++++++++++++++------- 1 file changed, 78 insertions(+), 21 deletions(-) diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index 6eb7519b69..7c7576014a 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -887,8 +887,28 @@ impl SubCircuitConfig for TxCircuitConfig { ])) }); + meta.lookup_any("num_all_txs in block table", |meta| { + let is_tag_block_num = meta.query_advice(is_tag_block_num, Rotation::cur()); + let block_num = meta.query_advice(tx_table.value, Rotation::cur()); + let num_all_txs_acc = meta.query_advice(num_all_txs_acc, Rotation::cur()); + + let input_expr = vec![NumAllTxs.expr(), block_num, num_all_txs_acc]; + let table_expr = block_table.table_exprs(meta); + let condition = and::expr([ + is_tag_block_num, + not::expr(block_num_unchanged.expr()), // the last tx in each block + not::expr(meta.query_advice(is_padding_tx, Rotation::cur())), + ]); + + input_expr + .into_iter() + .zip(table_expr.into_iter()) + .map(|(input, table)| (input * condition.clone(), table)) + .collect::>() + }); + /////////////////////////////////////////////////////////////////////// - /////////////// constraints on BlockNum ///////////////////////////// + /////// constraints on block_table's num_txs & num_cum_txs ////////// /////////////////////////////////////////////////////////////////////// meta.create_gate("is_padding_tx", |meta| { let is_tag_caller_addr = is_caller_addr(meta); @@ -1839,53 +1859,84 @@ impl TxCircuit { txs_len * TX_LEN + call_data_len } + // assign num_txs, cum_num_txs, num_all_txs only as we only lookup into + // block table for these three fields and this is mainly used for unit-test fn assign_dev_block_table( &self, config: TxCircuitConfig, layouter: &mut impl Layouter, ) -> Result<(), Error> { + let mut total_l1_popped_before = 0; let block_nums = self .txs .iter() .map(|tx| tx.block_number) .collect::>(); let mut num_txs_in_blocks = BTreeMap::new(); + let mut num_all_txs_in_blocks: BTreeMap = BTreeMap::new(); for tx in self.txs.iter() { if let Some(num_txs) = num_txs_in_blocks.get_mut(&tx.block_number) { *num_txs += 1; } else { num_txs_in_blocks.insert(tx.block_number, 1_usize); } + + if let Some(num_all_txs) = num_all_txs_in_blocks.get_mut(&tx.block_number) { + if tx.tx_type.is_l1_msg() { + *num_all_txs += tx.nonce - total_l1_popped_before + 1; + total_l1_popped_before = tx.nonce + 1; + } else { + *num_all_txs += 1; + } + } else { + let num_all_txs = if tx.tx_type.is_l1_msg() { + tx.nonce - total_l1_popped_before + 1 + } else { + 1 + }; + num_all_txs_in_blocks.insert(tx.block_number, num_all_txs); + } } + log::debug!("block_nums: {:?}", block_nums); + log::debug!("num_all_txs: {:?}", num_all_txs_in_blocks); layouter.assign_region( || "dev block table", |mut region| { - for (offset, (block_num, cum_num_txs)) in iter::once((0, 0)) + for (offset, (block_num, cum_num_txs, num_all_txs)) in iter::once((0, 0, 0)) .chain(block_nums.iter().scan(0, |cum_num_txs, block_num| { + let num_all_txs = num_all_txs_in_blocks[block_num]; *cum_num_txs += num_txs_in_blocks[block_num]; - Some((*block_num, *cum_num_txs)) + + Some((*block_num, *cum_num_txs, num_all_txs)) })) .enumerate() { - region.assign_fixed( - || "block_table.tag", - config.block_table.tag, - offset, - || Value::known(F::from(CumNumTxs as u64)), - )?; - region.assign_advice( - || "block_table.index", - config.block_table.index, - offset, - || Value::known(F::from(block_num)), - )?; - region.assign_advice( - || "block_table.value", - config.block_table.value, - offset, - || Value::known(F::from(cum_num_txs as u64)), - )?; + for (j, (tag, value)) in + [(CumNumTxs, cum_num_txs as u64), (NumAllTxs, num_all_txs)] + .into_iter() + .enumerate() + { + let row = offset * 2 + j; + region.assign_fixed( + || "block_table.tag", + config.block_table.tag, + row, + || Value::known(F::from(tag as u64)), + )?; + region.assign_advice( + || "block_table.index", + config.block_table.index, + row, + || Value::known(F::from(block_num)), + )?; + region.assign_advice( + || "block_table.value", + config.block_table.value, + row, + || Value::known(F::from(value)), + )?; + } } Ok(()) }, @@ -2153,6 +2204,12 @@ impl TxCircuit { ] { let (tx_id_next, cur_block_num, next_block_num) = match tag { BlockNumber => { + log::debug!( + "tx_id: {}, block_num: {}, num_all_txs_acc: {}", + i, + tx.block_number, + num_all_txs_acc + ); if i == sigs.len() - 1 { ( self.txs From be33661199abd478c7c9cf018e2425b10876d664 Mon Sep 17 00:00:00 2001 From: kunxian xia Date: Tue, 15 Aug 2023 13:53:41 +0800 Subject: [PATCH 07/10] clippy --- zkevm-circuits/src/witness/block.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zkevm-circuits/src/witness/block.rs b/zkevm-circuits/src/witness/block.rs index 2680c113e9..5c09547358 100644 --- a/zkevm-circuits/src/witness/block.rs +++ b/zkevm-circuits/src/witness/block.rs @@ -284,7 +284,7 @@ impl BlockContext { [ Value::known(F::from(BlockContextFieldTag::NumAllTxs as u64)), Value::known(current_block_number), - Value::known(F::from(num_all_txs as u64)), + Value::known(F::from(num_all_txs)), ], ], self.block_hash_assignments(randomness), From 7a50623602d8e33143f63f492b84acee0a68570a Mon Sep 17 00:00:00 2001 From: kunxian xia Date: Tue, 15 Aug 2023 15:27:04 +0800 Subject: [PATCH 08/10] add block num diff check to avoid two real block have same num --- zkevm-circuits/src/tx_circuit.rs | 23 +++++++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index 7c7576014a..8191b152f2 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -771,6 +771,29 @@ impl SubCircuitConfig for TxCircuitConfig { |meta| meta.query_advice(block_num, Rotation::cur()), ); + meta.lookup( + "block_num' - block_num >= 1 if block_num changed for non-padding tx", + |meta| { + // Block nums like this [1, 3, 5, 4, 0] is rejected by this. But [1, 2, 3, 5, 0] is + // acceptable. + let lookup_condition = and::expr([ + // if next tx is padding, then its block num is 0 (i.e. may have block num + // transition 5 -> 0) + not::expr(meta.query_advice(is_padding_tx, Rotation::next())), + // if tx table is full of real txs, then the last tx's next row is calldata. + // we could have block num transition 5 -> 0. + not::expr(meta.query_advice(is_calldata, Rotation::next())), + not::expr(block_num_unchanged.expr()), + meta.query_advice(is_tag_block_num, Rotation::cur()), + ]); + + let block_num_diff = meta.query_advice(block_num, Rotation::next()) + - meta.query_advice(block_num, Rotation::cur()); + + vec![(lookup_condition * block_num_diff, u16_table.clone().into())] + }, + ); + meta.create_gate("num_all_txs in a block", |meta| { let mut cb = BaseConstraintBuilder::default(); let queue_index = tx_nonce; From 3f860b4a66565315d56f583b1fbe904cfa18195b Mon Sep 17 00:00:00 2001 From: kunxian xia Date: Tue, 15 Aug 2023 15:28:21 +0800 Subject: [PATCH 09/10] clippy --- zkevm-circuits/src/tx_circuit.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index 8191b152f2..2ea8af603a 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -790,7 +790,7 @@ impl SubCircuitConfig for TxCircuitConfig { let block_num_diff = meta.query_advice(block_num, Rotation::next()) - meta.query_advice(block_num, Rotation::cur()); - vec![(lookup_condition * block_num_diff, u16_table.clone().into())] + vec![(lookup_condition * block_num_diff, u16_table.into())] }, ); From 158cb278008ed0ab985bd5e034b24c1802d9d86a Mon Sep 17 00:00:00 2001 From: kunxian xia Date: Tue, 15 Aug 2023 18:10:52 +0800 Subject: [PATCH 10/10] address comments --- zkevm-circuits/src/tx_circuit.rs | 68 +++++++++++++------------------- 1 file changed, 27 insertions(+), 41 deletions(-) diff --git a/zkevm-circuits/src/tx_circuit.rs b/zkevm-circuits/src/tx_circuit.rs index 2ea8af603a..a0622f5986 100644 --- a/zkevm-circuits/src/tx_circuit.rs +++ b/zkevm-circuits/src/tx_circuit.rs @@ -16,7 +16,7 @@ use crate::{ evm_circuit::util::constraint_builder::{BaseConstraintBuilder, ConstrainBuilderCommon}, sig_circuit::SigCircuit, table::{ - BlockContextFieldTag::CumNumTxs, + BlockContextFieldTag::{CumNumTxs, NumAllTxs}, BlockTable, KeccakTable, LookupTable, RlpFsmRlpTable as RlpTable, SigTable, TxFieldTag, TxFieldTag::{ BlockNumber, CallData, CallDataGasCost, CallDataLength, CallDataRLC, CalleeAddress, @@ -213,11 +213,11 @@ impl SubCircuitConfig for TxCircuitConfig { // In more detail, all txs in same block are grouped together and we iterate over // its txs to get `num_all_txs`. // - // | is_l1_msg | queue_index | total_l1_popped_before | num_l1_msgs | + // | is_l1_msg | queue_index | total_l1_popped_before | num_all_txs | // | true | q1 | c | q1-c+1 | - // | false | | q1+1 | q1-c+1 | - // | true | q2 | q1+1 | q2-c+1 | - // | true | q3 | q2+1 | q3-c+1 | + // | false | | q1+1 | q1-c+2 | + // | true | q2 | q1+1 | q2-c+2 | + // | true | q3 | q2+1 | q3-c+2 | let tx_nonce = meta.advice_column(); let block_num = meta.advice_column(); @@ -727,7 +727,7 @@ impl SubCircuitConfig for TxCircuitConfig { /////////////////////////////////////////////////////////////////////// /////////////// constraints on num_all_txs // /////////////////////// /////////////////////////////////////////////////////////////////////// - meta.create_gate("tx_nonce", |meta| { + meta.create_gate("copy tx_nonce", |meta| { let mut cb = BaseConstraintBuilder::default(); cb.condition(is_nonce(meta), |cb| { @@ -741,7 +741,7 @@ impl SubCircuitConfig for TxCircuitConfig { cb.gate(meta.query_fixed(q_enable, Rotation::cur())) }); - meta.create_gate("block_num", |meta| { + meta.create_gate("copy block_num", |meta| { let mut cb = BaseConstraintBuilder::default(); cb.condition(meta.query_advice(is_tag_block_num, Rotation::cur()), |cb| { @@ -771,28 +771,22 @@ impl SubCircuitConfig for TxCircuitConfig { |meta| meta.query_advice(block_num, Rotation::cur()), ); - meta.lookup( - "block_num' - block_num >= 1 if block_num changed for non-padding tx", - |meta| { - // Block nums like this [1, 3, 5, 4, 0] is rejected by this. But [1, 2, 3, 5, 0] is - // acceptable. - let lookup_condition = and::expr([ - // if next tx is padding, then its block num is 0 (i.e. may have block num - // transition 5 -> 0) - not::expr(meta.query_advice(is_padding_tx, Rotation::next())), - // if tx table is full of real txs, then the last tx's next row is calldata. - // we could have block num transition 5 -> 0. - not::expr(meta.query_advice(is_calldata, Rotation::next())), - not::expr(block_num_unchanged.expr()), - meta.query_advice(is_tag_block_num, Rotation::cur()), - ]); + meta.lookup("block_num is non-decreasing till padding txs", |meta| { + // Block nums like this [1, 3, 5, 4, 0] is rejected by this. But [1, 2, 3, 5, 0] is + // acceptable. + let lookup_condition = and::expr([ + // next row should not belong to a padding tx + not::expr(meta.query_advice(is_padding_tx, Rotation::next())), + // next row should not be in the calldata region + not::expr(meta.query_advice(is_calldata, Rotation::next())), + meta.query_advice(is_tag_block_num, Rotation::cur()), + ]); - let block_num_diff = meta.query_advice(block_num, Rotation::next()) - - meta.query_advice(block_num, Rotation::cur()); + let block_num_diff = meta.query_advice(block_num, Rotation::next()) + - meta.query_advice(block_num, Rotation::cur()); - vec![(lookup_condition * block_num_diff, u16_table.into())] - }, - ); + vec![(lookup_condition * block_num_diff, u16_table.into())] + }); meta.create_gate("num_all_txs in a block", |meta| { let mut cb = BaseConstraintBuilder::default(); @@ -800,7 +794,7 @@ impl SubCircuitConfig for TxCircuitConfig { // first tx in tx table cb.condition(meta.query_fixed(q_second, Rotation::cur()), |cb| { cb.require_equal( - "first block's first tx's num_l1_msgs", + "num_all_txs_acc = is_l1_msg ? queue_index - total_l1_popped_before + 1 : 1", meta.query_advice(num_all_txs_acc, Rotation::cur()), select::expr( meta.query_advice(is_l1_msg, Rotation::cur()), @@ -831,21 +825,17 @@ impl SubCircuitConfig for TxCircuitConfig { ), ); + // num_all_txs_acc' - num_all_txs_acc = is_l1_msg' ? queue_index' - + // total_l1_popped + 1 : 1 cb.require_equal( - "num_all_txs' - num_all_txs", + "num_all_txs_acc' - num_all_txs_acc", meta.query_advice(num_all_txs_acc, Rotation::next()) - meta.query_advice(num_all_txs_acc, Rotation::cur()), select::expr( meta.query_advice(is_l1_msg, Rotation::next()), - // if next tx is l1_msg, - // - num_l1_msgs increase by (queue_index' - tlp_before + 1) - // - num_l2_txs does not increase meta.query_advice(tx_nonce, Rotation::next()) - meta.query_advice(total_l1_popped_before, Rotation::cur()) + 1.expr(), - // if next tx is l2 tx, - // - num_l1_msgs does not increase, - // - num_l2_txs increase by 1. 1.expr(), ), ); @@ -879,20 +869,16 @@ impl SubCircuitConfig for TxCircuitConfig { ); // init new block's num_all_txs + // num_all_txs_acc' = is_l1_msg' ? queue_index' - total_l1_popped_before' + 1 : + // 1 cb.require_equal( "init new block's num_all_txs", meta.query_advice(num_all_txs_acc, Rotation::next()), select::expr( meta.query_advice(is_l1_msg, Rotation::next()), - // first tx is l1 msg - // - num_l1_msgs equals to (queue_index' - tlp_before + 1) - // - num_l2_txs is zero meta.query_advice(tx_nonce, Rotation::next()) - meta.query_advice(total_l1_popped_before, Rotation::next()) + 1.expr(), - // first tx is l2 tx - // - num_l1_msgs is zero - // - num_l2_txs is one 1.expr(), ), );