diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 798036f3d8..e8a917cc01 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -371,20 +371,27 @@ impl CircuitInputBuilder { self.set_end_chunk(max_rws); return; } - - // set end block - let mut end_block_not_last = self.block.block_steps.end_block_not_last.clone(); - let mut end_block_last = self.block.block_steps.end_block_last.clone(); - end_block_not_last.rwc = self.block_ctx.rwc; - end_block_last.rwc = self.block_ctx.rwc; - end_block_not_last.rwc_inner_chunk = self - .chunk_ctx - .as_ref() - .map_or_else(RWCounter::new, |chunk_ctx| chunk_ctx.rwc); - end_block_last.rwc_inner_chunk = self + // set padding + let last_step = self + .block + .txs() + .last() + .map(|tx| tx.last_step()) + .unwrap_or_else(|| &self.block.block_steps.padding); + let mut padding = last_step.clone(); // padding step need to keep transition context + padding.bus_mapping_instance = vec![]; // there is no rw in padding step + padding.exec_state = ExecState::Padding; + padding.rwc = self.block_ctx.rwc; + padding.rwc_inner_chunk = self .chunk_ctx .as_ref() .map_or_else(RWCounter::new, |chunk_ctx| chunk_ctx.rwc); + // TODO automatially assign all field + + // set end block + let mut end_block = padding.clone(); + end_block.exec_state = ExecState::EndBlock; + let is_first_chunk = self .chunk_ctx .as_ref() @@ -395,7 +402,7 @@ impl CircuitInputBuilder { if let Some(call_id) = state.block.txs.last().map(|tx| tx.calls[0].call_id) { state.call_context_read( - &mut end_block_last, + &mut end_block, call_id, CallContextField::TxId, Word::from(state.block.txs.len() as u64), @@ -421,7 +428,7 @@ impl CircuitInputBuilder { if is_first_chunk { push_op( &mut state.block.container, - &mut end_block_last, + &mut end_block, RWCounter(1), RWCounter(1), RW::READ, @@ -433,7 +440,7 @@ impl CircuitInputBuilder { let (padding_start, padding_end) = (total_rws + 1, max_rws - 1); push_op( &mut state.block.container, - &mut end_block_last, + &mut end_block, RWCounter(padding_start), RWCounter(padding_start), RW::READ, @@ -442,7 +449,7 @@ impl CircuitInputBuilder { if padding_end != padding_start { push_op( &mut state.block.container, - &mut end_block_last, + &mut end_block, RWCounter(padding_end), RWCounter(padding_end), RW::READ, @@ -451,8 +458,8 @@ impl CircuitInputBuilder { } } - self.block.block_steps.end_block_not_last = end_block_not_last; - self.block.block_steps.end_block_last = end_block_last; + self.block.block_steps.padding = padding; + self.block.block_steps.end_block = end_block; // set final rwc value to chunkctx if let Some(chunk_ctx) = self.chunk_ctx.as_mut() { diff --git a/bus-mapping/src/circuit_input_builder/block.rs b/bus-mapping/src/circuit_input_builder/block.rs index 4921814b62..a94e543e79 100644 --- a/bus-mapping/src/circuit_input_builder/block.rs +++ b/bus-mapping/src/circuit_input_builder/block.rs @@ -45,11 +45,11 @@ impl BlockContext { /// Block-wise execution steps that don't belong to any Transaction. #[derive(Debug)] pub struct BlockSteps { - /// EndBlock step that is repeated after the last transaction and before + /// Padding step that is repeated after the last transaction and before /// reaching the last EVM row. - pub end_block_not_last: ExecStep, - /// Last EndBlock step that appears in the last EVM row. - pub end_block_last: ExecStep, + pub padding: ExecStep, + /// EndBlock step that appears in the last chunk last EVM row. + pub end_block: ExecStep, /// TODO Define and move chunk related step to Chunk struct /// Begin op of a chunk pub begin_chunk: ExecStep, @@ -136,11 +136,11 @@ impl Block { exec_state: ExecState::BeginChunk, ..ExecStep::default() }, - end_block_not_last: ExecStep { - exec_state: ExecState::EndBlock, + padding: ExecStep { + exec_state: ExecState::Padding, ..ExecStep::default() }, - end_block_last: ExecStep { + end_block: ExecStep { exec_state: ExecState::EndBlock, ..ExecStep::default() }, diff --git a/bus-mapping/src/circuit_input_builder/execution.rs b/bus-mapping/src/circuit_input_builder/execution.rs index 3dbf636e86..2a6e00d4c7 100644 --- a/bus-mapping/src/circuit_input_builder/execution.rs +++ b/bus-mapping/src/circuit_input_builder/execution.rs @@ -134,6 +134,8 @@ pub enum ExecState { BeginTx, /// Virtual step End Tx EndTx, + /// Virtual step Padding + Padding, /// Virtual step End Block EndBlock, /// Virtual step End Chunk diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index 52b69eb678..908d5f01db 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -105,6 +105,7 @@ mod mulmod; #[path = "execution/not.rs"] mod opcode_not; mod origin; +mod padding; mod pc; mod pop; mod push; @@ -185,6 +186,7 @@ use mul_div_mod::MulDivModGadget; use mulmod::MulModGadget; use opcode_not::NotGadget; use origin::OriginGadget; +use padding::PaddingGadget; use pc::PcGadget; use pop::PopGadget; use push::PushGadget; @@ -246,6 +248,7 @@ pub struct ExecutionConfig { // internal state gadgets begin_tx_gadget: Box>, end_block_gadget: Box>, + padding_gadget: Box>, end_tx_gadget: Box>, begin_chunk_gadget: Box>, end_chunk_gadget: Box>, @@ -386,7 +389,7 @@ impl ExecutionConfig { let (execute_state_first_step_whitelist, execute_state_last_step_whitelist) = ( HashSet::from([ ExecutionState::BeginTx, - ExecutionState::EndBlock, + ExecutionState::Padding, ExecutionState::BeginChunk, ]), HashSet::from([ExecutionState::EndBlock, ExecutionState::EndChunk]), @@ -553,6 +556,7 @@ impl ExecutionConfig { // internal states begin_tx_gadget: configure_gadget!(), end_block_gadget: configure_gadget!(), + padding_gadget: configure_gadget!(), end_tx_gadget: configure_gadget!(), begin_chunk_gadget: configure_gadget!(), end_chunk_gadget: configure_gadget!(), @@ -876,11 +880,12 @@ impl ExecutionConfig { .chain( IntoIterator::into_iter([ ( - "EndTx can only transit to BeginTx or EndBlock or EndChunk", + "EndTx can only transit to BeginTx or Padding or EndBlock or EndChunk", ExecutionState::EndTx, vec![ ExecutionState::BeginTx, ExecutionState::EndBlock, + ExecutionState::Padding, ExecutionState::EndChunk, ], ), @@ -889,6 +894,15 @@ impl ExecutionConfig { ExecutionState::EndChunk, vec![ExecutionState::EndChunk], ), + ( + "Padding can only transit to Padding or EndBlock or EndChunk", + ExecutionState::Padding, + vec![ + ExecutionState::Padding, + ExecutionState::EndBlock, + ExecutionState::EndChunk, + ], + ), ( "EndBlock can only transit to EndBlock", ExecutionState::EndBlock, @@ -914,9 +928,13 @@ impl ExecutionConfig { .collect(), ), ( - "Only EndTx or EndBlock can transit to EndBlock", + "Only EndTx or EndBlock or Padding can transit to EndBlock", ExecutionState::EndBlock, - vec![ExecutionState::EndTx, ExecutionState::EndBlock], + vec![ + ExecutionState::EndTx, + ExecutionState::EndBlock, + ExecutionState::Padding, + ], ), ( "Only BeginChunk can transit to BeginChunk", @@ -1063,8 +1081,8 @@ impl ExecutionConfig { let is_last_chunk = block.chunk_context.chunk_index == block.chunk_context.total_chunks - 1; - let end_block_not_last = &block.end_block_not_last; - let end_block_last = &block.end_block_last; + let padding = &block.padding; + let end_block = &block.end_block; let begin_chunk = &block.begin_chunk; let end_chunk = &block.end_chunk; // Collect all steps @@ -1077,7 +1095,7 @@ impl ExecutionConfig { .map(move |step| (tx, &tx.calls()[step.call_index], step)) })) // add last dummy step just to satisfy below logic, which will not be assigned and count as real step - .chain(std::iter::once((&dummy_tx, &last_call, end_block_not_last))) + .chain(std::iter::once((&dummy_tx, &last_call, padding))) .peekable(); let evm_rows = block.circuits_params.max_evm_rows; @@ -1112,7 +1130,7 @@ impl ExecutionConfig { offset += height; } - // part2: assign non-last EndBlock steps when padding needed + // part2: assign Padding steps when padding needed if !no_padding { if offset >= evm_rows { log::error!( @@ -1122,11 +1140,11 @@ impl ExecutionConfig { ); return Err(Error::Synthesis); } - let height = ExecutionState::EndBlock.get_step_height(); + let height = ExecutionState::Padding.get_step_height(); debug_assert_eq!(height, 1); let last_row = evm_rows - 1; log::trace!( - "assign non-last EndBlock in range [{},{})", + "assign Padding in range [{},{})", offset, last_row ); @@ -1137,7 +1155,7 @@ impl ExecutionConfig { block, &dummy_tx, &last_call, - end_block_not_last, + padding, height, challenges, assign_pass, @@ -1160,7 +1178,7 @@ impl ExecutionConfig { block, &dummy_tx, &last_call, - end_block_last, + end_block, height, None, challenges, @@ -1268,7 +1286,7 @@ impl ExecutionConfig { } assert_eq!(height, 1); assert!(step.rw_indices_len() == 0); - assert!(matches!(step.execution_state(), ExecutionState::EndBlock)); + assert!(matches!(step.execution_state(), ExecutionState::Padding)); // Disable access to next step deliberately for "repeatable" step let region = &mut CachedRegion::<'_, '_, F>::new( @@ -1312,7 +1330,7 @@ impl ExecutionConfig { challenges: &Challenges>, assign_pass: usize, ) -> Result<(), Error> { - if !matches!(step.execution_state(), ExecutionState::EndBlock) { + if !matches!(step.execution_state(), ExecutionState::Padding) { log::trace!( "assign_exec_step offset: {} state {:?} step: {:?} call: {:?}", offset, @@ -1390,6 +1408,7 @@ impl ExecutionConfig { // internal states ExecutionState::BeginTx => assign_exec_step!(self.begin_tx_gadget), ExecutionState::EndTx => assign_exec_step!(self.end_tx_gadget), + ExecutionState::Padding => assign_exec_step!(self.padding_gadget), ExecutionState::EndBlock => assign_exec_step!(self.end_block_gadget), ExecutionState::BeginChunk => assign_exec_step!(self.begin_chunk_gadget), ExecutionState::EndChunk => assign_exec_step!(self.end_chunk_gadget), @@ -1536,8 +1555,7 @@ impl ExecutionConfig { // enable with `RUST_LOG=debug` if log::log_enabled!(log::Level::Debug) { - let is_padding_step = matches!(step.execution_state(), ExecutionState::EndBlock) - && step.rw_indices_len() == 0; + let is_padding_step = matches!(step.execution_state(), ExecutionState::Padding); if !is_padding_step { // expensive function call Self::check_rw_lookup( diff --git a/zkevm-circuits/src/evm_circuit/execution/end_block.rs b/zkevm-circuits/src/evm_circuit/execution/end_block.rs index 0802c83936..3499560ac9 100644 --- a/zkevm-circuits/src/evm_circuit/execution/end_block.rs +++ b/zkevm-circuits/src/evm_circuit/execution/end_block.rs @@ -176,17 +176,9 @@ mod test { .run(); } - // Test where the EVM circuit contains an exact number of rows corresponding to - // the trace steps + 1 EndBlock + // Test steps + 1 EndBlock without padding #[test] - fn end_block_exact() { + fn end_block_no_padding() { test_circuit(0); } - - // Test where the EVM circuit has a fixed size and contains several padding - // EndBlocks at the end after the trace steps - #[test] - fn end_block_padding() { - test_circuit(50); - } } diff --git a/zkevm-circuits/src/evm_circuit/execution/end_tx.rs b/zkevm-circuits/src/evm_circuit/execution/end_tx.rs index c438a8665a..19c7f44630 100644 --- a/zkevm-circuits/src/evm_circuit/execution/end_tx.rs +++ b/zkevm-circuits/src/evm_circuit/execution/end_tx.rs @@ -180,7 +180,8 @@ impl ExecutionGadget for EndTxGadget { ); cb.condition( - cb.next.execution_state_selector([ExecutionState::EndBlock]), + cb.next + .execution_state_selector([ExecutionState::EndBlock, ExecutionState::Padding]), |cb| { cb.require_step_state_transition(StepStateTransition { rw_counter: Delta(9.expr() - is_first_tx.expr() + coinbase_reward.rw_delta()), diff --git a/zkevm-circuits/src/evm_circuit/execution/padding.rs b/zkevm-circuits/src/evm_circuit/execution/padding.rs new file mode 100644 index 0000000000..4d70d885b9 --- /dev/null +++ b/zkevm-circuits/src/evm_circuit/execution/padding.rs @@ -0,0 +1,78 @@ +use std::marker::PhantomData; + +use crate::evm_circuit::{ + execution::ExecutionGadget, + step::ExecutionState, + util::{ + constraint_builder::{EVMConstraintBuilder, StepStateTransition}, + CachedRegion, + }, + witness::{Block, Call, ExecStep, Transaction}, +}; +use eth_types::Field; +use halo2_proofs::plonk::Error; + +#[derive(Clone, Debug)] +pub(crate) struct PaddingGadget { + _phantom: PhantomData, +} + +impl ExecutionGadget for PaddingGadget { + const NAME: &'static str = "Padding"; + + const EXECUTION_STATE: ExecutionState = ExecutionState::Padding; + + fn configure(cb: &mut EVMConstraintBuilder) -> Self { + cb.require_step_state_transition(StepStateTransition { + ..StepStateTransition::same() + }); + + Self { + _phantom: PhantomData, + } + } + + fn assign_exec_step( + &self, + _region: &mut CachedRegion<'_, '_, F>, + _offset: usize, + _block: &Block, + _: &Transaction, + _: &Call, + _step: &ExecStep, + ) -> Result<(), Error> { + Ok(()) + } +} + +#[cfg(test)] +mod test { + use crate::test_util::CircuitTestBuilder; + + use eth_types::bytecode; + + use mock::TestContext; + + fn test_circuit(evm_circuit_pad_to: usize) { + let bytecode = bytecode! { + PUSH1(0) + STOP + }; + + let ctx = TestContext::<2, 1>::simple_ctx_with_bytecode(bytecode).unwrap(); + + // finish required tests using this witness block + CircuitTestBuilder::<2, 1>::new_from_test_ctx(ctx) + .block_modifier(Box::new(move |block| { + block.circuits_params.max_evm_rows = evm_circuit_pad_to + })) + .run(); + } + + // Test where the EVM circuit has a fixed size and contains several Padding + // at the end after the trace steps + #[test] + fn padding() { + test_circuit(50); + } +} diff --git a/zkevm-circuits/src/evm_circuit/param.rs b/zkevm-circuits/src/evm_circuit/param.rs index 0ebe0a9a53..88779a9e75 100644 --- a/zkevm-circuits/src/evm_circuit/param.rs +++ b/zkevm-circuits/src/evm_circuit/param.rs @@ -8,7 +8,7 @@ use halo2_proofs::{ use std::collections::HashMap; // Step dimension -pub(crate) const STEP_WIDTH: usize = 131; +pub(crate) const STEP_WIDTH: usize = 132; /// Step height pub const MAX_STEP_HEIGHT: usize = 19; /// The height of the state of a step, used by gates that connect two diff --git a/zkevm-circuits/src/evm_circuit/step.rs b/zkevm-circuits/src/evm_circuit/step.rs index e423f853b6..a0b26e7f52 100644 --- a/zkevm-circuits/src/evm_circuit/step.rs +++ b/zkevm-circuits/src/evm_circuit/step.rs @@ -42,6 +42,7 @@ pub enum ExecutionState { BeginTx, EndTx, EndBlock, + Padding, BeginChunk, EndChunk, // Opcode successful cases @@ -302,6 +303,7 @@ impl From<&ExecStep> for ExecutionState { } ExecState::BeginTx => ExecutionState::BeginTx, ExecState::EndTx => ExecutionState::EndTx, + ExecState::Padding => ExecutionState::Padding, ExecState::EndBlock => ExecutionState::EndBlock, ExecState::BeginChunk => ExecutionState::BeginChunk, ExecState::EndChunk => ExecutionState::EndChunk, diff --git a/zkevm-circuits/src/witness/block.rs b/zkevm-circuits/src/witness/block.rs index e09e644959..8fc1521c0c 100644 --- a/zkevm-circuits/src/witness/block.rs +++ b/zkevm-circuits/src/witness/block.rs @@ -27,11 +27,11 @@ pub struct Block { pub randomness: F, /// Transactions in the block pub txs: Vec, - /// EndBlock step that is repeated after the last transaction and before + /// Padding step that is repeated after the last transaction and before /// reaching the last EVM row. - pub end_block_not_last: ExecStep, - /// Last EndBlock step that appears in the last EVM row. - pub end_block_last: ExecStep, + pub padding: ExecStep, + /// EndBlock step that appears in the last chunk last EVM row. + pub end_block: ExecStep, /// BeginChunk step to propagate State pub begin_chunk: ExecStep, /// EndChunk step that appears in the last EVM row for all the chunks other than the last. @@ -282,8 +282,8 @@ pub fn block_convert( // TODO get permutation fingerprint & challenges permu_alpha: F::from(103), permu_gamma: F::from(101), - end_block_not_last: block.block_steps.end_block_not_last.clone(), - end_block_last: block.block_steps.end_block_last.clone(), + padding: block.block_steps.padding.clone(), + end_block: block.block_steps.end_block.clone(), // TODO refactor chunk related field to chunk structure begin_chunk: block.block_steps.begin_chunk.clone(), end_chunk: block.block_steps.end_chunk.clone(),