From ea5b823332332ed38c459426e6658d00359f5d3e Mon Sep 17 00:00:00 2001 From: Steven Date: Mon, 21 Mar 2022 23:19:23 +0800 Subject: [PATCH] Bus-mapping for opcode `calldatacopy` (#393) * Support generating multiple exec-steps from one geth-step. * Fix build error. * Fix to use bus-mapping to generate bytecode. * Update test cases. * Add basic of calldatacopy bus-mappinng to just support zero call data length. * Update bus-mapping calldatacopy. * Push op of call_data_length and call data offset. * Add `is_root_call` to BytecodeTestConfig. * Replace `OpcodeId` with `ExecState` in bus-mapping ExcStep. * Generate CopyToMemory exection step. * Add TransactionConfig to bus-mapping handle_tx. * Update test code. * 1. Remove TransactionConfig and replace with `call_data`. 2. Remove gas calculation and set in `calldatacopy` bus-mapping. 3. Revert `test_ok_internal` since not want to set internal call. * Update test cases which call `handle_tx` and `new_tx` of circuit input builder. * Update constant max address of state circuit. * Add unit test for calldatacopy bus-mapping. * change api * fix calldatacopy * fix rebase * Set exec_state for BeginTx and EndTx. * Fix to return a new exec step in function `dummy_gen_associated_ops`. * Update bus-mapping calldatacopy unit-test. * Update for fmt and clippy. * Fix doc test. * Update according to code review. * Fix a comment. * Fix to directly use StepAuxiliaryData of bus-mapping in zkevm-circuits. * Revert a comment and a fix. * address comments * fix doc Co-authored-by: Haichen Shen Co-authored-by: Zhang Zhuo --- bus-mapping/src/circuit_input_builder.rs | 206 +++++++++++----- bus-mapping/src/evm/opcodes.rs | 66 +++-- bus-mapping/src/evm/opcodes/calldatacopy.rs | 229 ++++++++++++++++++ bus-mapping/src/evm/opcodes/calldatasize.rs | 24 +- bus-mapping/src/evm/opcodes/caller.rs | 24 +- bus-mapping/src/evm/opcodes/callvalue.rs | 24 +- bus-mapping/src/evm/opcodes/dup.rs | 22 +- bus-mapping/src/evm/opcodes/mload.rs | 26 +- bus-mapping/src/evm/opcodes/mstore.rs | 36 +-- bus-mapping/src/evm/opcodes/number.rs | 3 +- bus-mapping/src/evm/opcodes/selfbalance.rs | 21 +- bus-mapping/src/evm/opcodes/sload.rs | 30 ++- bus-mapping/src/evm/opcodes/stackonlyop.rs | 25 +- bus-mapping/src/evm/opcodes/stop.rs | 10 +- bus-mapping/src/evm/opcodes/swap.rs | 49 ++-- bus-mapping/src/lib.rs | 2 +- mock/src/lib.rs | 20 +- .../src/evm_circuit/execution/bitwise.rs | 5 +- .../src/evm_circuit/execution/calldatacopy.rs | 149 ++---------- .../src/evm_circuit/execution/calldataload.rs | 2 +- .../src/evm_circuit/execution/gas.rs | 2 +- .../src/evm_circuit/execution/memory.rs | 5 +- .../src/evm_circuit/execution/memory_copy.rs | 92 ++++--- .../src/evm_circuit/util/memory_gadget.rs | 14 +- zkevm-circuits/src/evm_circuit/witness.rs | 122 +++++----- zkevm-circuits/src/test_util.rs | 11 +- 26 files changed, 758 insertions(+), 461 deletions(-) create mode 100644 bus-mapping/src/evm/opcodes/calldatacopy.rs diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index 01498c6056..2f57f28c36 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -19,7 +19,7 @@ use crate::rpc::GethClient; use ethers_providers::JsonRpcClient; /// Out of Gas errors by opcode -#[derive(Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub enum OogError { /// Out of Gas for opcodes which have non-zero constant gas cost Constant, @@ -65,7 +65,7 @@ pub enum OogError { } /// EVM Execution Error -#[derive(Debug, PartialEq)] +#[derive(Clone, Debug, PartialEq)] pub enum ExecError { /// Invalid Opcode InvalidOpcode, @@ -101,11 +101,71 @@ pub enum ExecError { MaxCodeSizeExceeded, } +/// Execution state +#[derive(Clone, Debug, Eq, PartialEq)] +pub enum ExecState { + /// EVM Opcode ID + Op(OpcodeId), + /// Virtual step Begin Tx + BeginTx, + /// Virtual step End Tx + EndTx, + /// Virtual step Copy To Memory + CopyToMemory, +} + +impl ExecState { + /// Returns `true` if `ExecState` is an opcode and the opcode is a `PUSHn`. + pub fn is_push(&self) -> bool { + if let ExecState::Op(op) = self { + op.is_push() + } else { + false + } + } + + /// Returns `true` if `ExecState` is an opcode and the opcode is a `DUPn`. + pub fn is_dup(&self) -> bool { + if let ExecState::Op(op) = self { + op.is_dup() + } else { + false + } + } + + /// Returns `true` if `ExecState` is an opcode and the opcode is a `SWAPn`. + pub fn is_swap(&self) -> bool { + if let ExecState::Op(op) = self { + op.is_swap() + } else { + false + } + } +} + +/// Auxiliary data of Execution step +#[derive(Clone, Debug)] +pub enum StepAuxiliaryData { + /// Auxiliary data of Copy To Memory + CopyToMemory { + /// Source start address + src_addr: u64, + /// Destination address + dst_addr: u64, + /// Bytes left + bytes_left: u64, + /// Source end address + src_addr_end: u64, + /// Indicate if copy from transaction call data + from_tx: bool, + }, +} + /// An execution step of the EVM. -#[derive(Debug)] +#[derive(Clone, Debug)] pub struct ExecStep { - /// The opcode ID - pub op: OpcodeId, + /// Execution state + pub exec_state: ExecState, /// Program Counter pub pc: ProgramCounter, /// Stack size @@ -129,10 +189,12 @@ pub struct ExecStep { pub bus_mapping_instance: Vec, /// Error generated by this step pub error: Option, + /// Step auxiliary data + pub aux_data: Option, } impl ExecStep { - /// Create a new Self from a [`GethExecStep`]. + /// Create a new Self from a `GethExecStep`. pub fn new( step: &GethExecStep, call_index: usize, @@ -140,7 +202,7 @@ impl ExecStep { swc: usize, // State Write Counter ) -> Self { ExecStep { - op: step.op, + exec_state: ExecState::Op(step.op), pc: step.pc, stack_size: step.stack.0.len(), memory_size: step.memory.0.len(), @@ -151,6 +213,7 @@ impl ExecStep { swc, bus_mapping_instance: Vec::new(), error: None, + aux_data: None, } } } @@ -158,7 +221,7 @@ impl ExecStep { impl Default for ExecStep { fn default() -> Self { Self { - op: OpcodeId::INVALID(0), + exec_state: ExecState::Op(OpcodeId::INVALID(0)), pc: ProgramCounter(0), stack_size: 0, memory_size: 0, @@ -169,6 +232,7 @@ impl Default for ExecStep { swc: 0, bus_mapping_instance: Vec::new(), error: None, + aux_data: None, } } } @@ -368,12 +432,12 @@ impl Call { /// Context of a [`Call`]. #[derive(Debug, Default)] pub struct CallContext { - // Index of call - index: usize, + /// Index of call + pub index: usize, /// State Write Counter tracks the count of state write operations in the /// call. When a subcall in this call succeeds, the `swc` increases by the /// number of successful state writes in the subcall. - swc: usize, + pub swc: usize, } /// A reversion group is the collection of calls and the operations which are @@ -466,6 +530,11 @@ impl TransactionContext { self.is_last_tx } + /// Return the calls in this transaction. + pub fn calls(&self) -> &[CallContext] { + &self.calls + } + /// Return the index of the current call (the last call in the call stack). fn call_index(&self) -> Result { self.calls @@ -541,8 +610,10 @@ pub struct Transaction { /// Value pub value: Word, /// Input / Call Data - pub input: Vec, // call_data + pub input: Vec, + /// Calls made in the transaction calls: Vec, + /// Execution steps steps: Vec, } @@ -654,21 +725,61 @@ pub struct CircuitInputStateRef<'a> { pub tx: &'a mut Transaction, /// Transaction Context pub tx_ctx: &'a mut TransactionContext, - /// Step - pub step: &'a mut ExecStep, } impl<'a> CircuitInputStateRef<'a> { + /// Create a new step from a `GethExecStep` + pub fn new_step(&self, geth_step: &GethExecStep) -> Result { + let call_ctx = self.tx_ctx.call_ctx()?; + Ok(ExecStep::new( + geth_step, + call_ctx.index, + self.block_ctx.rwc, + call_ctx.swc, + )) + } + + /// Create a new BeginTx step + pub fn new_begin_tx_step(&self) -> ExecStep { + ExecStep { + exec_state: ExecState::BeginTx, + gas_left: Gas(self.tx.gas), + rwc: self.block_ctx.rwc, + ..Default::default() + } + } + + /// Create a new EndTx step + pub fn new_end_tx_step(&self) -> ExecStep { + let prev_step = self + .tx + .steps() + .last() + .expect("steps should have at least one BeginTx step"); + ExecStep { + exec_state: ExecState::EndTx, + gas_left: Gas(prev_step.gas_left.0 - prev_step.gas_cost.0), + rwc: self.block_ctx.rwc, + // For tx without code execution + swc: if let Some(call_ctx) = self.tx_ctx.calls().last() { + call_ctx.swc + } else { + 0 + }, + ..Default::default() + } + } + /// Push an [`Operation`] into the [`OperationContainer`] with the next /// [`RWCounter`] and then adds a reference to the stored operation /// ([`OperationRef`]) inside the bus-mapping instance of the current /// [`ExecStep`]. Then increase the block_ctx [`RWCounter`] by one. - pub fn push_op(&mut self, rw: RW, op: T) { + pub fn push_op(&mut self, step: &mut ExecStep, rw: RW, op: T) { let op_ref = self.block .container .insert(Operation::new(self.block_ctx.rwc.inc_pre(), rw, op)); - self.step.bus_mapping_instance.push(op_ref); + step.bus_mapping_instance.push(op_ref); } /// Push an [`Operation`] with reversible to be true into the @@ -679,13 +790,18 @@ impl<'a> CircuitInputStateRef<'a> { /// This method should be used in `Opcode::gen_associated_ops` instead of /// `push_op` when the operation is `RW::WRITE` and it can be reverted (for /// example, a write `StorageOp`). - pub fn push_op_reversible(&mut self, rw: RW, op: T) -> Result<(), Error> { + pub fn push_op_reversible( + &mut self, + step: &mut ExecStep, + rw: RW, + op: T, + ) -> Result<(), Error> { let op_ref = self.block.container.insert(Operation::new_reversible( self.block_ctx.rwc.inc_pre(), rw, op, )); - self.step.bus_mapping_instance.push(op_ref); + step.bus_mapping_instance.push(op_ref); // Increase state_write_counter self.call_ctx_mut()?.swc += 1; @@ -710,12 +826,13 @@ impl<'a> CircuitInputStateRef<'a> { /// [`RWCounter`] by one. pub fn push_memory_op( &mut self, + step: &mut ExecStep, rw: RW, address: MemoryAddress, value: u8, ) -> Result<(), Error> { let call_id = self.call()?.call_id; - self.push_op(rw, MemoryOp::new(call_id, address, value)); + self.push_op(step, rw, MemoryOp::new(call_id, address, value)); Ok(()) } @@ -726,12 +843,13 @@ impl<'a> CircuitInputStateRef<'a> { /// [`RWCounter`] by one. pub fn push_stack_op( &mut self, + step: &mut ExecStep, rw: RW, address: StackAddress, value: Word, ) -> Result<(), Error> { let call_id = self.call()?.call_id; - self.push_op(rw, StackOp::new(call_id, address, value)); + self.push_op(step, rw, StackOp::new(call_id, address, value)); Ok(()) } @@ -1254,7 +1372,6 @@ impl<'a> CircuitInputBuilder { &'a mut self, tx: &'a mut Transaction, tx_ctx: &'a mut TransactionContext, - step: &'a mut ExecStep, ) -> CircuitInputStateRef { CircuitInputStateRef { sdb: &mut self.sdb, @@ -1263,7 +1380,6 @@ impl<'a> CircuitInputBuilder { block_ctx: &mut self.block_ctx, tx, tx_ctx, - step, } } @@ -1342,50 +1458,25 @@ impl<'a> CircuitInputBuilder { // - execution_state: BeginTx // - op: None // Generate BeginTx step - let mut step = ExecStep { - gas_left: Gas(tx.gas), - rwc: self.block_ctx.rwc, - ..Default::default() - }; - gen_begin_tx_ops(&mut self.state_ref(&mut tx, &mut tx_ctx, &mut step))?; - tx.steps.push(step); + let begin_tx_step = gen_begin_tx_ops(&mut self.state_ref(&mut tx, &mut tx_ctx))?; + tx.steps.push(begin_tx_step); for (index, geth_step) in geth_trace.struct_logs.iter().enumerate() { - let call_ctx = tx_ctx.call_ctx()?; - let mut step = - ExecStep::new(geth_step, call_ctx.index, self.block_ctx.rwc, call_ctx.swc); - let mut state_ref = self.state_ref(&mut tx, &mut tx_ctx, &mut step); - - gen_associated_ops( + let mut state_ref = self.state_ref(&mut tx, &mut tx_ctx); + let exec_steps = gen_associated_ops( &geth_step.op, &mut state_ref, &geth_trace.struct_logs[index..], )?; - - tx.steps.push(step); + tx.steps.extend(exec_steps); } // TODO: Move into gen_associated_steps with // - execution_state: EndTx // - op: None // Generate EndTx step - let step_prev = tx - .steps - .last() - .expect("steps should have at least one BeginTx step"); - let mut step = ExecStep { - gas_left: Gas(step_prev.gas_left.0 - step_prev.gas_cost.0), - rwc: self.block_ctx.rwc, - // For tx without code execution - swc: if let Some(call_ctx) = tx_ctx.calls.last() { - call_ctx.swc - } else { - 0 - }, - ..Default::default() - }; - gen_end_tx_ops(&mut self.state_ref(&mut tx, &mut tx_ctx, &mut step))?; - tx.steps.push(step); + let end_tx_step = gen_end_tx_ops(&mut self.state_ref(&mut tx, &mut tx_ctx))?; + tx.steps.push(end_tx_step); self.block.txs.push(tx); self.sdb.clear_access_list_and_refund(); @@ -1898,8 +1989,7 @@ mod tracer_tests { } fn state_ref(&mut self) -> CircuitInputStateRef { - self.builder - .state_ref(&mut self.tx, &mut self.tx_ctx, &mut self.step) + self.builder.state_ref(&mut self.tx, &mut self.tx_ctx) } } @@ -1970,7 +2060,7 @@ mod tracer_tests { STOP }; let block = - mock::new_single_tx_trace_code_gas(&code, Gas(1_000_000_000_000_000u64)).unwrap(); + mock::new_single_tx_trace_code_gas(&code, Gas(1_000_000_000_000_000u64), None).unwrap(); let struct_logs = &block.geth_traces[0].struct_logs; // get last CALL @@ -2830,7 +2920,7 @@ mod tracer_tests { PUSH1(0x1) PUSH1(0x2) }; - let block = mock::new_single_tx_trace_code_gas(&code, Gas(21004)).unwrap(); + let block = mock::new_single_tx_trace_code_gas(&code, Gas(21004), None).unwrap(); let struct_logs = &block.geth_traces[0].struct_logs; assert_eq!(struct_logs[1].error, Some(GETH_ERR_OUT_OF_GAS.to_string())); diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index e0102367a4..e85d58ac3e 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -1,6 +1,6 @@ //! Definition of each opcode of the EVM. use crate::{ - circuit_input_builder::CircuitInputStateRef, + circuit_input_builder::{CircuitInputStateRef, ExecStep}, evm::OpcodeId, operation::{ AccountField, AccountOp, CallContextField, CallContextOp, TxAccessListAccountOp, @@ -15,6 +15,7 @@ use eth_types::{ }; use log::warn; +mod calldatacopy; mod calldatasize; mod caller; mod callvalue; @@ -27,6 +28,7 @@ mod stackonlyop; mod stop; mod swap; +use calldatacopy::Calldatacopy; use calldatasize::Calldatasize; use caller::Caller; use callvalue::Callvalue; @@ -40,9 +42,9 @@ use stop::Stop; use swap::Swap; /// Generic opcode trait which defines the logic of the -/// [`Operation`](crate::operation::Operation) that should be generated for an -/// [`ExecStep`](crate::circuit_input_builder::ExecStep) depending of the -/// [`OpcodeId`] it contains. +/// [`Operation`](crate::operation::Operation) that should be generated for one +/// or multiple [`ExecStep`](crate::circuit_input_builder::ExecStep) depending +/// of the [`OpcodeId`] it contains. pub trait Opcode: Debug { /// Generate the associated [`MemoryOp`](crate::operation::MemoryOp)s, /// [`StackOp`](crate::operation::StackOp)s, and @@ -50,19 +52,21 @@ pub trait Opcode: Debug { /// is implemented for. fn gen_associated_ops( state: &mut CircuitInputStateRef, - next_steps: &[GethExecStep], - ) -> Result<(), Error>; + geth_steps: &[GethExecStep], + ) -> Result, Error>; } fn dummy_gen_associated_ops( - _state: &mut CircuitInputStateRef, - _next_steps: &[GethExecStep], -) -> Result<(), Error> { - Ok(()) + state: &mut CircuitInputStateRef, + geth_steps: &[GethExecStep], +) -> Result, Error> { + Ok(vec![state.new_step(&geth_steps[0])?]) } -type FnGenAssociatedOps = - fn(state: &mut CircuitInputStateRef, next_steps: &[GethExecStep]) -> Result<(), Error>; +type FnGenAssociatedOps = fn( + state: &mut CircuitInputStateRef, + geth_steps: &[GethExecStep], +) -> Result, Error>; fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { match opcode_id { @@ -100,7 +104,7 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { OpcodeId::CALLVALUE => Callvalue::gen_associated_ops, OpcodeId::CALLDATASIZE => Calldatasize::gen_associated_ops, OpcodeId::CALLDATALOAD => StackOnlyOpcode::<1, 1>::gen_associated_ops, - // OpcodeId::CALLDATACOPY => {}, + OpcodeId::CALLDATACOPY => Calldatacopy::gen_associated_ops, // OpcodeId::CODESIZE => {}, // OpcodeId::CODECOPY => {}, // OpcodeId::GASPRICE => {}, @@ -224,13 +228,14 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { pub fn gen_associated_ops( opcode_id: &OpcodeId, state: &mut CircuitInputStateRef, - next_steps: &[GethExecStep], -) -> Result<(), Error> { + geth_steps: &[GethExecStep], +) -> Result, Error> { let fn_gen_associated_ops = fn_gen_associated_ops(opcode_id); - fn_gen_associated_ops(state, next_steps) + fn_gen_associated_ops(state, geth_steps) } -pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { +pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result { + let mut exec_step = state.new_begin_tx_step(); let call = state.call()?.clone(); for (field, value) in [ @@ -245,6 +250,7 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { ), ] { state.push_op( + &mut exec_step, RW::READ, CallContextOp { call_id: call.call_id, @@ -257,6 +263,7 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { let caller_address = call.caller_address; let nonce_prev = state.sdb.increase_nonce(&caller_address); state.push_op( + &mut exec_step, RW::WRITE, AccountOp { address: caller_address, @@ -269,6 +276,7 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { for address in [call.caller_address, call.address] { state.sdb.add_account_to_access_list(address); state.push_op( + &mut exec_step, RW::WRITE, TxAccessListAccountOp { tx_id: state.tx_ctx.id(), @@ -289,7 +297,7 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { } else { GasCost::TX.as_u64() } + call_data_gas_cost; - state.step.gas_cost = GasCost(intrinsic_gas_cost); + exec_step.gas_cost = GasCost(intrinsic_gas_cost); let (found, caller_account) = state.sdb.get_account_mut(&call.caller_address); if !found { @@ -298,6 +306,7 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { let caller_balance_prev = caller_account.balance; let caller_balance = caller_account.balance - call.value - state.tx.gas_price * state.tx.gas; state.push_op_reversible( + &mut exec_step, RW::WRITE, AccountOp { address: call.caller_address, @@ -315,6 +324,7 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { let callee_balance = callee_account.balance + call.value; let code_hash = callee_account.code_hash; state.push_op_reversible( + &mut exec_step, RW::WRITE, AccountOp { address: call.address, @@ -333,6 +343,7 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { } _ => { state.push_op( + &mut exec_step, RW::READ, AccountOp { address: call.address, @@ -366,6 +377,7 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { (CallContextField::LastCalleeReturnDataLength, 0.into()), ] { state.push_op( + &mut exec_step, RW::READ, CallContextOp { call_id: call.call_id, @@ -375,13 +387,15 @@ pub fn gen_begin_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { ); } - Ok(()) + Ok(exec_step) } -pub fn gen_end_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { +pub fn gen_end_tx_ops(state: &mut CircuitInputStateRef) -> Result { + let mut exec_step = state.new_end_tx_step(); let call = state.tx.calls()[0].clone(); state.push_op( + &mut exec_step, RW::READ, CallContextOp { call_id: call.call_id, @@ -392,6 +406,7 @@ pub fn gen_end_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { let refund = state.sdb.refund(); state.push_op( + &mut exec_step, RW::READ, TxRefundOp { tx_id: state.tx_ctx.id(), @@ -401,15 +416,16 @@ pub fn gen_end_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { ); let effective_refund = - refund.min((state.tx.gas - state.step.gas_left.0) / MAX_REFUND_QUOTIENT_OF_GAS_USED as u64); + refund.min((state.tx.gas - exec_step.gas_left.0) / MAX_REFUND_QUOTIENT_OF_GAS_USED as u64); let (found, caller_account) = state.sdb.get_account_mut(&call.caller_address); if !found { return Err(Error::AccountNotFound(call.caller_address)); } let caller_balance_prev = caller_account.balance; let caller_balance = - caller_account.balance + state.tx.gas_price * (state.step.gas_left.0 + effective_refund); + caller_account.balance + state.tx.gas_price * (exec_step.gas_left.0 + effective_refund); state.push_op( + &mut exec_step, RW::WRITE, AccountOp { address: call.caller_address, @@ -426,8 +442,9 @@ pub fn gen_end_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { } let coinbase_balance_prev = coinbase_account.balance; let coinbase_balance = - coinbase_account.balance + effective_tip * (state.tx.gas - state.step.gas_left.0); + coinbase_account.balance + effective_tip * (state.tx.gas - exec_step.gas_left.0); state.push_op( + &mut exec_step, RW::WRITE, AccountOp { address: state.block.coinbase, @@ -439,6 +456,7 @@ pub fn gen_end_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { if !state.tx_ctx.is_last_tx() { state.push_op( + &mut exec_step, RW::READ, CallContextOp { call_id: state.block_ctx.rwc.0 + 1, @@ -448,5 +466,5 @@ pub fn gen_end_tx_ops(state: &mut CircuitInputStateRef) -> Result<(), Error> { ); } - Ok(()) + Ok(exec_step) } diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs new file mode 100644 index 0000000000..46869e4fde --- /dev/null +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -0,0 +1,229 @@ +use super::Opcode; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecState, ExecStep, StepAuxiliaryData}; +use crate::operation::{CallContextField, CallContextOp, RW}; +use crate::Error; +use eth_types::GethExecStep; + +// The max number of bytes that can be copied in a step limited by the number +// of cells in a step +const MAX_COPY_BYTES: usize = 71; + +#[derive(Clone, Copy, Debug)] +pub(crate) struct Calldatacopy; + +impl Opcode for Calldatacopy { + fn gen_associated_ops( + state: &mut CircuitInputStateRef, + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_steps = vec![gen_calldatacopy_step(state, geth_step)?]; + let memory_copy_steps = gen_memory_copy_steps(state, geth_steps)?; + exec_steps.extend(memory_copy_steps); + Ok(exec_steps) + } +} + +fn gen_calldatacopy_step( + state: &mut CircuitInputStateRef, + geth_step: &GethExecStep, +) -> Result { + let mut exec_step = state.new_step(geth_step)?; + let memory_offset = geth_step.stack.nth_last(0)?; + let data_offset = geth_step.stack.nth_last(1)?; + let length = geth_step.stack.nth_last(2)?; + + state.push_stack_op( + &mut exec_step, + RW::READ, + geth_step.stack.nth_last_filled(0), + memory_offset, + )?; + state.push_stack_op( + &mut exec_step, + RW::READ, + geth_step.stack.nth_last_filled(1), + data_offset, + )?; + state.push_stack_op( + &mut exec_step, + RW::READ, + geth_step.stack.nth_last_filled(2), + length, + )?; + state.push_op( + &mut exec_step, + RW::READ, + CallContextOp { + call_id: state.call()?.call_id, + field: CallContextField::TxId, + value: state.tx_ctx.id().into(), + }, + ); + + if !state.call()?.is_root { + state.push_op( + &mut exec_step, + RW::READ, + CallContextOp { + call_id: state.call()?.call_id, + field: CallContextField::CallDataLength, + value: state.call()?.call_data_length.into(), + }, + ); + state.push_op( + &mut exec_step, + RW::READ, + CallContextOp { + call_id: state.call()?.call_id, + field: CallContextField::CallDataOffset, + value: state.call()?.call_data_offset.into(), + }, + ); + }; + Ok(exec_step) +} + +fn gen_memory_copy_step( + state: &mut CircuitInputStateRef, + exec_step: &mut ExecStep, + src_addr: u64, + dst_addr: u64, + src_addr_end: u64, + bytes_left: usize, + is_root: bool, +) -> Result<(), Error> { + for idx in 0..std::cmp::min(bytes_left, MAX_COPY_BYTES) { + let addr = src_addr + idx as u64; + let byte = if addr < src_addr_end { + if is_root { + state.tx.input[addr as usize] + } else { + // TODO: read caller memory + unimplemented!() + } + } else { + 0 + }; + state.push_memory_op(exec_step, RW::WRITE, (idx + dst_addr as usize).into(), byte)?; + } + + exec_step.aux_data = Some(StepAuxiliaryData::CopyToMemory { + src_addr, + dst_addr, + bytes_left: bytes_left as u64, + src_addr_end, + from_tx: is_root, + }); + + Ok(()) +} + +fn gen_memory_copy_steps( + state: &mut CircuitInputStateRef, + geth_steps: &[GethExecStep], +) -> Result, Error> { + let memory_offset = geth_steps[0].stack.nth_last(0)?.as_u64(); + let data_offset = geth_steps[0].stack.nth_last(1)?.as_u64(); + let length = geth_steps[0].stack.nth_last(2)?.as_usize(); + + let call_data_offset = state.call()?.call_data_offset; + let call_data_length = state.call()?.call_data_length; + let (src_addr, buffer_addr_end) = ( + call_data_offset + data_offset, + call_data_offset + call_data_length, + ); + + let mut copied = 0; + let mut steps = vec![]; + while copied < length { + let mut exec_step = state.new_step(&geth_steps[1])?; + exec_step.exec_state = ExecState::CopyToMemory; + gen_memory_copy_step( + state, + &mut exec_step, + src_addr + copied as u64, + memory_offset + copied as u64, + buffer_addr_end, + length - copied, + state.call()?.is_root, + )?; + steps.push(exec_step); + copied += MAX_COPY_BYTES; + } + + Ok(steps) +} + +#[cfg(test)] +mod calldatacopy_tests { + use super::*; + use crate::circuit_input_builder::ExecState; + use crate::operation::StackOp; + use eth_types::evm_types::{OpcodeId, StackAddress}; + use eth_types::{bytecode, Word}; + use pretty_assertions::assert_eq; + + #[test] + fn calldatacopy_opcode_impl() { + let code = bytecode! { + PUSH32(0) + PUSH32(0) + PUSH32(0x40) + CALLDATACOPY + STOP + }; + + // Get the execution steps from the external tracer + let block = crate::mock::BlockData::new_from_geth_data( + mock::new_single_tx_trace_code(&code).unwrap(), + ); + + let mut builder = block.new_circuit_input_builder(); + builder + .handle_block(&block.eth_block, &block.geth_traces) + .unwrap(); + + let step = builder.block.txs()[0] + .steps() + .iter() + .find(|step| step.exec_state == ExecState::Op(OpcodeId::CALLDATACOPY)) + .unwrap(); + + assert_eq!( + [0, 1, 2] + .map(|idx| &builder.block.container.stack[step.bus_mapping_instance[idx].as_usize()]) + .map(|operation| (operation.rw(), operation.op())), + [ + ( + RW::READ, + &StackOp::new(1, StackAddress::from(1021), Word::from(0x40)) + ), + ( + RW::READ, + &StackOp::new(1, StackAddress::from(1022), Word::from(0)) + ), + ( + RW::READ, + &StackOp::new(1, StackAddress::from(1023), Word::from(0)) + ), + ] + ); + + assert_eq!( + { + let operation = + &builder.block.container.call_context[step.bus_mapping_instance[3].as_usize()]; + (operation.rw(), operation.op()) + }, + ( + RW::READ, + &CallContextOp { + call_id: builder.block.txs()[0].calls()[0].call_id, + field: CallContextField::TxId, + value: Word::from(1), + } + ) + ); + } +} diff --git a/bus-mapping/src/evm/opcodes/calldatasize.rs b/bus-mapping/src/evm/opcodes/calldatasize.rs index 32487d0e12..1ab6ca9902 100644 --- a/bus-mapping/src/evm/opcodes/calldatasize.rs +++ b/bus-mapping/src/evm/opcodes/calldatasize.rs @@ -1,5 +1,5 @@ use crate::{ - circuit_input_builder::CircuitInputStateRef, + circuit_input_builder::{CircuitInputStateRef, ExecStep}, operation::{CallContextField, CallContextOp, RW}, Error, }; @@ -14,11 +14,13 @@ pub(crate) struct Calldatasize; impl Opcode for Calldatasize { fn gen_associated_ops( state: &mut CircuitInputStateRef, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; - let value = steps[1].stack.last()?; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step)?; + let value = geth_steps[1].stack.last()?; state.push_op( + &mut exec_step, RW::READ, CallContextOp { call_id: state.call()?.call_id, @@ -26,13 +28,19 @@ impl Opcode for Calldatasize { value, }, ); - state.push_stack_op(RW::WRITE, step.stack.last_filled().map(|a| a - 1), value)?; - Ok(()) + state.push_stack_op( + &mut exec_step, + RW::WRITE, + geth_step.stack.last_filled().map(|a| a - 1), + value, + )?; + Ok(vec![exec_step]) } } #[cfg(test)] mod calldatasize_tests { + use crate::circuit_input_builder::ExecState; use crate::operation::{CallContextField, CallContextOp, StackOp, RW}; use eth_types::{bytecode, evm_types::OpcodeId, evm_types::StackAddress}; use pretty_assertions::assert_eq; @@ -57,7 +65,7 @@ mod calldatasize_tests { let step = builder.block.txs()[0] .steps() .iter() - .find(|step| step.op == OpcodeId::CALLDATASIZE) + .find(|step| step.exec_state == ExecState::Op(OpcodeId::CALLDATASIZE)) .unwrap(); let call_id = builder.block.txs()[0].calls()[0].call_id; diff --git a/bus-mapping/src/evm/opcodes/caller.rs b/bus-mapping/src/evm/opcodes/caller.rs index a01ea619dc..13922d0ced 100644 --- a/bus-mapping/src/evm/opcodes/caller.rs +++ b/bus-mapping/src/evm/opcodes/caller.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::operation::{CallContextField, CallContextOp, RW}; use crate::Error; use eth_types::GethExecStep; @@ -12,13 +12,15 @@ pub(crate) struct Caller; impl Opcode for Caller { fn gen_associated_ops( state: &mut CircuitInputStateRef, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step)?; // Get caller_address result from next step - let value = steps[1].stack.last()?; + let value = geth_steps[1].stack.last()?; // CallContext read of the caller_address state.push_op( + &mut exec_step, RW::READ, CallContextOp { call_id: state.call()?.call_id, @@ -27,14 +29,20 @@ impl Opcode for Caller { }, ); // Stack write of the caller_address - state.push_stack_op(RW::WRITE, step.stack.last_filled().map(|a| a - 1), value)?; + state.push_stack_op( + &mut exec_step, + RW::WRITE, + geth_step.stack.last_filled().map(|a| a - 1), + value, + )?; - Ok(()) + Ok(vec![exec_step]) } } #[cfg(test)] mod caller_tests { + use crate::circuit_input_builder::ExecState; use crate::operation::{CallContextField, CallContextOp, StackOp, RW}; use eth_types::{bytecode, evm_types::OpcodeId, evm_types::StackAddress, ToWord}; use pretty_assertions::assert_eq; @@ -59,7 +67,7 @@ mod caller_tests { let step = builder.block.txs()[0] .steps() .iter() - .find(|step| step.op == OpcodeId::CALLER) + .find(|step| step.exec_state == ExecState::Op(OpcodeId::CALLER)) .unwrap(); let call_id = builder.block.txs()[0].calls()[0].call_id; diff --git a/bus-mapping/src/evm/opcodes/callvalue.rs b/bus-mapping/src/evm/opcodes/callvalue.rs index c9901c7fe6..a1a43bc2bd 100644 --- a/bus-mapping/src/evm/opcodes/callvalue.rs +++ b/bus-mapping/src/evm/opcodes/callvalue.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::operation::{CallContextField, CallContextOp, RW}; use crate::Error; use eth_types::GethExecStep; @@ -12,13 +12,15 @@ pub(crate) struct Callvalue; impl Opcode for Callvalue { fn gen_associated_ops( state: &mut CircuitInputStateRef, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step)?; // Get call_value result from next step - let value = steps[1].stack.last()?; + let value = geth_steps[1].stack.last()?; // CallContext read of the call_value state.push_op( + &mut exec_step, RW::READ, CallContextOp { call_id: state.call()?.call_id, @@ -27,14 +29,20 @@ impl Opcode for Callvalue { }, ); // Stack write of the call_value - state.push_stack_op(RW::WRITE, step.stack.last_filled().map(|a| a - 1), value)?; + state.push_stack_op( + &mut exec_step, + RW::WRITE, + geth_step.stack.last_filled().map(|a| a - 1), + value, + )?; - Ok(()) + Ok(vec![exec_step]) } } #[cfg(test)] mod callvalue_tests { + use crate::circuit_input_builder::ExecState; use crate::operation::{CallContextField, CallContextOp, StackOp, RW}; use eth_types::{bytecode, evm_types::OpcodeId, evm_types::StackAddress}; use pretty_assertions::assert_eq; @@ -59,7 +67,7 @@ mod callvalue_tests { let step = builder.block.txs()[0] .steps() .iter() - .find(|step| step.op == OpcodeId::CALLVALUE) + .find(|step| step.exec_state == ExecState::Op(OpcodeId::CALLVALUE)) .unwrap(); let call_id = builder.block.txs()[0].calls()[0].call_id; diff --git a/bus-mapping/src/evm/opcodes/dup.rs b/bus-mapping/src/evm/opcodes/dup.rs index d06792369d..3a35fabfd8 100644 --- a/bus-mapping/src/evm/opcodes/dup.rs +++ b/bus-mapping/src/evm/opcodes/dup.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::{operation::RW, Error}; use eth_types::GethExecStep; @@ -11,21 +11,23 @@ pub(crate) struct Dup; impl Opcode for Dup { fn gen_associated_ops( state: &mut CircuitInputStateRef, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step)?; - let stack_value_read = step.stack.nth_last(N - 1)?; - let stack_position = step.stack.nth_last_filled(N - 1); - state.push_stack_op(RW::READ, stack_position, stack_value_read)?; + let stack_value_read = geth_step.stack.nth_last(N - 1)?; + let stack_position = geth_step.stack.nth_last_filled(N - 1); + state.push_stack_op(&mut exec_step, RW::READ, stack_position, stack_value_read)?; state.push_stack_op( + &mut exec_step, RW::WRITE, - step.stack.last_filled().map(|a| a - 1), + geth_step.stack.last_filled().map(|a| a - 1), stack_value_read, )?; - Ok(()) + Ok(vec![exec_step]) } } @@ -69,7 +71,7 @@ mod dup_tests { let step = builder.block.txs()[0] .steps() .iter() - .filter(|step| step.op.is_dup()) + .filter(|step| step.exec_state.is_dup()) .collect_vec()[i]; assert_eq!( diff --git a/bus-mapping/src/evm/opcodes/mload.rs b/bus-mapping/src/evm/opcodes/mload.rs index c4de071d66..5e55aadcda 100644 --- a/bus-mapping/src/evm/opcodes/mload.rs +++ b/bus-mapping/src/evm/opcodes/mload.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::{operation::RW, Error}; use core::convert::TryInto; use eth_types::evm_types::MemoryAddress; @@ -16,23 +16,24 @@ pub(crate) struct Mload; impl Opcode for Mload { fn gen_associated_ops( state: &mut CircuitInputStateRef, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step)?; // // First stack read // - let stack_value_read = step.stack.last()?; - let stack_position = step.stack.last_filled(); + let stack_value_read = geth_step.stack.last()?; + let stack_position = geth_step.stack.last_filled(); // Manage first stack read at latest stack position - state.push_stack_op(RW::READ, stack_position, stack_value_read)?; + state.push_stack_op(&mut exec_step, RW::READ, stack_position, stack_value_read)?; // Read the memory let mut mem_read_addr: MemoryAddress = stack_value_read.try_into()?; // Accesses to memory that hasn't been initialized are valid, and return // 0. - let mem_read_value = steps[1] + let mem_read_value = geth_steps[1] .memory .read_word(mem_read_addr) .unwrap_or_else(|_| Word::zero()); @@ -40,25 +41,26 @@ impl Opcode for Mload { // // First stack write // - state.push_stack_op(RW::WRITE, stack_position, mem_read_value)?; + state.push_stack_op(&mut exec_step, RW::WRITE, stack_position, mem_read_value)?; // // First mem read -> 32 MemoryOp generated. // for byte in mem_read_value.to_be_bytes() { - state.push_memory_op(RW::READ, mem_read_addr, byte)?; + state.push_memory_op(&mut exec_step, RW::READ, mem_read_addr, byte)?; // Update mem_read_addr to next byte's one mem_read_addr += MemoryAddress::from(1); } - Ok(()) + Ok(vec![exec_step]) } } #[cfg(test)] mod mload_tests { use super::*; + use crate::circuit_input_builder::ExecState; use crate::operation::{MemoryOp, StackOp}; use eth_types::bytecode; use eth_types::evm_types::{OpcodeId, StackAddress}; @@ -89,7 +91,7 @@ mod mload_tests { let step = builder.block.txs()[0] .steps() .iter() - .find(|step| step.op == OpcodeId::MLOAD) + .find(|step| step.exec_state == ExecState::Op(OpcodeId::MLOAD)) .unwrap(); assert_eq!( diff --git a/bus-mapping/src/evm/opcodes/mstore.rs b/bus-mapping/src/evm/opcodes/mstore.rs index 7d8d7de05b..494107510e 100644 --- a/bus-mapping/src/evm/opcodes/mstore.rs +++ b/bus-mapping/src/evm/opcodes/mstore.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::{operation::RW, Error}; use core::convert::TryInto; use eth_types::evm_types::MemoryAddress; @@ -14,18 +14,19 @@ pub(crate) struct Mstore; impl Opcode for Mstore { fn gen_associated_ops( state: &mut CircuitInputStateRef, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step)?; // First stack read (offset) - let offset = step.stack.nth_last(0)?; - let offset_pos = step.stack.nth_last_filled(0); - state.push_stack_op(RW::READ, offset_pos, offset)?; + let offset = geth_step.stack.nth_last(0)?; + let offset_pos = geth_step.stack.nth_last_filled(0); + state.push_stack_op(&mut exec_step, RW::READ, offset_pos, offset)?; // Second stack read (value) - let value = step.stack.nth_last(1)?; - let value_pos = step.stack.nth_last_filled(1); - state.push_stack_op(RW::READ, value_pos, value)?; + let value = geth_step.stack.nth_last(1)?; + let value_pos = geth_step.stack.nth_last_filled(1); + state.push_stack_op(&mut exec_step, RW::READ, value_pos, value)?; // First mem write -> 32 MemoryOp generated. let offset_addr: MemoryAddress = offset.try_into()?; @@ -34,6 +35,7 @@ impl Opcode for Mstore { true => { // stack write operation for mstore8 state.push_memory_op( + &mut exec_step, RW::WRITE, offset_addr, *value.to_le_bytes().first().unwrap(), @@ -43,18 +45,24 @@ impl Opcode for Mstore { // stack write each byte for mstore let bytes = value.to_be_bytes(); for (i, byte) in bytes.iter().enumerate() { - state.push_memory_op(RW::WRITE, offset_addr.map(|a| a + i), *byte)?; + state.push_memory_op( + &mut exec_step, + RW::WRITE, + offset_addr.map(|a| a + i), + *byte, + )?; } } } - Ok(()) + Ok(vec![exec_step]) } } #[cfg(test)] mod mstore_tests { use super::*; + use crate::circuit_input_builder::ExecState; use crate::operation::{MemoryOp, StackOp}; use eth_types::bytecode; use eth_types::evm_types::{MemoryAddress, OpcodeId, StackAddress}; @@ -85,7 +93,7 @@ mod mstore_tests { let step = builder.block.txs()[0] .steps() .iter() - .filter(|step| step.op == OpcodeId::MSTORE) + .filter(|step| step.exec_state == ExecState::Op(OpcodeId::MSTORE)) .nth(1) .unwrap(); @@ -146,7 +154,7 @@ mod mstore_tests { let step = builder.block.txs()[0] .steps() .iter() - .find(|step| step.op == OpcodeId::MSTORE8) + .find(|step| step.exec_state == ExecState::Op(OpcodeId::MSTORE8)) .unwrap(); assert_eq!( diff --git a/bus-mapping/src/evm/opcodes/number.rs b/bus-mapping/src/evm/opcodes/number.rs index 8351415d72..99640cd2b0 100644 --- a/bus-mapping/src/evm/opcodes/number.rs +++ b/bus-mapping/src/evm/opcodes/number.rs @@ -38,11 +38,12 @@ mod number_tests { test_builder.block_ctx.rwc, 0, ); - let mut state_ref = test_builder.state_ref(&mut tx, &mut tx_ctx, &mut step); + let mut state_ref = test_builder.state_ref(&mut tx, &mut tx_ctx); // Add the last Stack write let number = block.eth_block.number.unwrap().as_u64(); state_ref.push_stack_op( + &mut step, RW::WRITE, StackAddress::from(1024 - 1), eth_types::U256::from(number), diff --git a/bus-mapping/src/evm/opcodes/selfbalance.rs b/bus-mapping/src/evm/opcodes/selfbalance.rs index f7769a3f4e..a590967bff 100644 --- a/bus-mapping/src/evm/opcodes/selfbalance.rs +++ b/bus-mapping/src/evm/opcodes/selfbalance.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::operation::{AccountField, AccountOp, CallContextField, CallContextOp, RW}; use crate::Error; use eth_types::{GethExecStep, ToWord}; @@ -10,14 +10,16 @@ pub(crate) struct Selfbalance; impl Opcode for Selfbalance { fn gen_associated_ops( state: &mut CircuitInputStateRef, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; - let self_balance = steps[1].stack.last()?; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step)?; + let self_balance = geth_steps[1].stack.last()?; let callee_address = state.call()?.address; // CallContext read of the callee_address state.push_op( + &mut exec_step, RW::READ, CallContextOp { call_id: state.call()?.call_id, @@ -28,6 +30,7 @@ impl Opcode for Selfbalance { // Account read for the balance of the callee_address state.push_op( + &mut exec_step, RW::READ, AccountOp { address: callee_address, @@ -39,18 +42,20 @@ impl Opcode for Selfbalance { // Stack write of self_balance state.push_stack_op( + &mut exec_step, RW::WRITE, - step.stack.last_filled().map(|a| a - 1), + geth_step.stack.last_filled().map(|a| a - 1), self_balance, )?; - Ok(()) + Ok(vec![exec_step]) } } #[cfg(test)] mod selfbalance_tests { use super::*; + use crate::circuit_input_builder::ExecState; use crate::operation::{CallContextField, CallContextOp, StackOp, RW}; use eth_types::{bytecode, evm_types::OpcodeId, evm_types::StackAddress}; use pretty_assertions::assert_eq; @@ -75,7 +80,7 @@ mod selfbalance_tests { let step = builder.block.txs()[0] .steps() .iter() - .find(|step| step.op == OpcodeId::SELFBALANCE) + .find(|step| step.exec_state == ExecState::Op(OpcodeId::SELFBALANCE)) .unwrap(); let call_id = builder.block.txs()[0].calls()[0].call_id; diff --git a/bus-mapping/src/evm/opcodes/sload.rs b/bus-mapping/src/evm/opcodes/sload.rs index 455d199c3a..37ace18d33 100644 --- a/bus-mapping/src/evm/opcodes/sload.rs +++ b/bus-mapping/src/evm/opcodes/sload.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::{ operation::{StorageOp, RW}, Error, @@ -15,20 +15,22 @@ pub(crate) struct Sload; impl Opcode for Sload { fn gen_associated_ops( state: &mut CircuitInputStateRef, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step)?; // First stack read - let stack_value_read = step.stack.last()?; - let stack_position = step.stack.last_filled(); + let stack_value_read = geth_step.stack.last()?; + let stack_position = geth_step.stack.last_filled(); // Manage first stack read at latest stack position - state.push_stack_op(RW::READ, stack_position, stack_value_read)?; + state.push_stack_op(&mut exec_step, RW::READ, stack_position, stack_value_read)?; // Storage read - let storage_value_read = step.storage.get_or_err(&stack_value_read)?; + let storage_value_read = geth_step.storage.get_or_err(&stack_value_read)?; state.push_op( + &mut exec_step, RW::READ, StorageOp::new( state.call()?.address, @@ -41,15 +43,21 @@ impl Opcode for Sload { ); // First stack write - state.push_stack_op(RW::WRITE, stack_position, storage_value_read)?; + state.push_stack_op( + &mut exec_step, + RW::WRITE, + stack_position, + storage_value_read, + )?; - Ok(()) + Ok(vec![exec_step]) } } #[cfg(test)] mod sload_tests { use super::*; + use crate::circuit_input_builder::ExecState; use crate::operation::StackOp; use eth_types::bytecode; use eth_types::evm_types::{OpcodeId, StackAddress}; @@ -83,7 +91,7 @@ mod sload_tests { let step = builder.block.txs()[0] .steps() .iter() - .find(|step| step.op == OpcodeId::SLOAD) + .find(|step| step.exec_state == ExecState::Op(OpcodeId::SLOAD)) .unwrap(); assert_eq!( diff --git a/bus-mapping/src/evm/opcodes/stackonlyop.rs b/bus-mapping/src/evm/opcodes/stackonlyop.rs index f0b3aeeaf8..73603f6ce1 100644 --- a/bus-mapping/src/evm/opcodes/stackonlyop.rs +++ b/bus-mapping/src/evm/opcodes/stackonlyop.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::{operation::RW, Error}; use eth_types::GethExecStep; @@ -15,35 +15,38 @@ pub(crate) struct StackOnlyOpcode; impl Opcode for StackOnlyOpcode { fn gen_associated_ops( state: &mut CircuitInputStateRef, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; - + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step)?; // N_POP stack reads for i in 0..N_POP { state.push_stack_op( + &mut exec_step, RW::READ, - step.stack.nth_last_filled(i), - step.stack.nth_last(i)?, + geth_step.stack.nth_last_filled(i), + geth_step.stack.nth_last(i)?, )?; } // N_PUSH stack writes for i in 0..N_PUSH { state.push_stack_op( + &mut exec_step, RW::WRITE, - steps[1].stack.nth_last_filled(N_PUSH - 1 - i), - steps[1].stack.nth_last(N_PUSH - 1 - i)?, + geth_steps[1].stack.nth_last_filled(N_PUSH - 1 - i), + geth_steps[1].stack.nth_last(N_PUSH - 1 - i)?, )?; } - Ok(()) + Ok(vec![exec_step]) } } #[cfg(test)] mod stackonlyop_tests { use super::*; + use crate::circuit_input_builder::ExecState; use crate::operation::StackOp; use eth_types::bytecode; use eth_types::evm_types::{OpcodeId, StackAddress}; @@ -70,7 +73,7 @@ mod stackonlyop_tests { let step = builder.block.txs()[0] .steps() .iter() - .find(|step| step.op == opcode) + .find(|step| step.exec_state == ExecState::Op(opcode)) .unwrap(); assert_eq!( diff --git a/bus-mapping/src/evm/opcodes/stop.rs b/bus-mapping/src/evm/opcodes/stop.rs index 0f41a40b4c..e3f0015ecc 100644 --- a/bus-mapping/src/evm/opcodes/stop.rs +++ b/bus-mapping/src/evm/opcodes/stop.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::Error; use eth_types::GethExecStep; @@ -15,10 +15,10 @@ pub(crate) struct Stop; impl Opcode for Stop { fn gen_associated_ops( state: &mut CircuitInputStateRef, - _steps: &[GethExecStep], - ) -> Result<(), Error> { + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let exec_step = state.new_step(&geth_steps[0])?; state.handle_return()?; - - Ok(()) + Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/swap.rs b/bus-mapping/src/evm/opcodes/swap.rs index fc288983c9..188fbbb983 100644 --- a/bus-mapping/src/evm/opcodes/swap.rs +++ b/bus-mapping/src/evm/opcodes/swap.rs @@ -1,5 +1,5 @@ use super::Opcode; -use crate::circuit_input_builder::CircuitInputStateRef; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; use crate::{operation::RW, Error}; use eth_types::GethExecStep; @@ -11,23 +11,44 @@ pub(crate) struct Swap; impl Opcode for Swap { fn gen_associated_ops( state: &mut CircuitInputStateRef, - steps: &[GethExecStep], - ) -> Result<(), Error> { - let step = &steps[0]; + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step)?; // Peek b and a - let stack_b_value_read = step.stack.nth_last(N)?; - let stack_b_position = step.stack.nth_last_filled(N); - state.push_stack_op(RW::READ, stack_b_position, stack_b_value_read)?; - let stack_a_value_read = step.stack.last()?; - let stack_a_position = step.stack.last_filled(); - state.push_stack_op(RW::READ, stack_a_position, stack_a_value_read)?; + let stack_b_value_read = geth_step.stack.nth_last(N)?; + let stack_b_position = geth_step.stack.nth_last_filled(N); + state.push_stack_op( + &mut exec_step, + RW::READ, + stack_b_position, + stack_b_value_read, + )?; + let stack_a_value_read = geth_step.stack.last()?; + let stack_a_position = geth_step.stack.last_filled(); + state.push_stack_op( + &mut exec_step, + RW::READ, + stack_a_position, + stack_a_value_read, + )?; // Write a into b_position, write b into a_position - state.push_stack_op(RW::WRITE, stack_b_position, stack_a_value_read)?; - state.push_stack_op(RW::WRITE, stack_a_position, stack_b_value_read)?; + state.push_stack_op( + &mut exec_step, + RW::WRITE, + stack_b_position, + stack_a_value_read, + )?; + state.push_stack_op( + &mut exec_step, + RW::WRITE, + stack_a_position, + stack_b_value_read, + )?; - Ok(()) + Ok(vec![exec_step]) } } @@ -71,7 +92,7 @@ mod swap_tests { let step = builder.block.txs()[0] .steps() .iter() - .filter(|step| step.op.is_swap()) + .filter(|step| step.exec_state.is_swap()) .collect_vec()[i]; let a_pos = StackAddress(1024 - 6); diff --git a/bus-mapping/src/lib.rs b/bus-mapping/src/lib.rs index d071f4575a..965482b714 100644 --- a/bus-mapping/src/lib.rs +++ b/bus-mapping/src/lib.rs @@ -107,7 +107,7 @@ //! //! // We use some mock data as context for the trace //! let mut eth_block = mock::new_block(); -//! let eth_tx = mock::new_tx(ð_block); +//! let eth_tx = mock::new_tx(ð_block, None); //! eth_block.transactions.push(eth_tx.clone()); //! let mut sdb = StateDB::new(); //! sdb.set_account(ð_tx.from, state_db::Account::zero()); diff --git a/mock/src/lib.rs b/mock/src/lib.rs index 701d362cdd..098239f846 100644 --- a/mock/src/lib.rs +++ b/mock/src/lib.rs @@ -64,8 +64,9 @@ pub fn new( pub fn new_single_tx_trace_accounts_gas( accounts: Vec, gas: Gas, + input: Option>, ) -> Result { - let mut eth_tx = new_tx(&new_block()); + let mut eth_tx = new_tx(&new_block(), input); eth_tx.gas = Word::from(gas.0); new(accounts, vec![eth_tx]) } @@ -75,7 +76,7 @@ pub fn new_single_tx_trace_accounts_gas( /// The trace will be generated automatically with the external_tracer /// from the accounts code. pub fn new_single_tx_trace_accounts(accounts: Vec) -> Result { - new_single_tx_trace_accounts_gas(accounts, Gas(1_000_000u64)) + new_single_tx_trace_accounts_gas(accounts, Gas(1_000_000u64), None) } /// Create a new block with a single tx that executes the code passed by @@ -89,9 +90,13 @@ pub fn new_single_tx_trace_code(code: &Bytecode) -> Result { /// Create a new block with a single tx with the given gas limit that /// executes the code passed by argument. The trace will be generated /// automatically with the external_tracer from the code. -pub fn new_single_tx_trace_code_gas(code: &Bytecode, gas: Gas) -> Result { +pub fn new_single_tx_trace_code_gas( + code: &Bytecode, + gas: Gas, + input: Option>, +) -> Result { let tracer_account = new_tracer_account(code); - new_single_tx_trace_accounts_gas(vec![tracer_account], gas) + new_single_tx_trace_accounts_gas(vec![tracer_account], gas, input) } /// Create a new block with a single tx that executes the code_a passed by @@ -133,7 +138,7 @@ pub fn new_block() -> Block { } /// Generate a new mock transaction with preloaded data, useful for tests. -pub fn new_tx(block: &Block) -> eth_types::Transaction { +pub fn new_tx(block: &Block, input: Option>) -> eth_types::Transaction { eth_types::Transaction { hash: Hash::zero(), nonce: Word::zero(), @@ -145,7 +150,10 @@ pub fn new_tx(block: &Block) -> eth_types::Transaction { value: Word::zero(), gas_price: Some(Word::zero()), gas: Word::from(1_000_000u64), - input: Bytes::default(), + input: match input { + Some(data) => Bytes::from(data), + None => Bytes::default(), + }, v: U64::zero(), r: Word::zero(), s: Word::zero(), diff --git a/zkevm-circuits/src/evm_circuit/execution/bitwise.rs b/zkevm-circuits/src/evm_circuit/execution/bitwise.rs index fc62a757f1..0d7b0e4465 100644 --- a/zkevm-circuits/src/evm_circuit/execution/bitwise.rs +++ b/zkevm-circuits/src/evm_circuit/execution/bitwise.rs @@ -128,7 +128,10 @@ mod test { evm_circuit_lookup_tags: get_fixed_table(FixedTableConfig::Complete), ..Default::default() }; - assert_eq!(test_circuits_using_bytecode(bytecode, test_config), Ok(())); + assert_eq!( + test_circuits_using_bytecode(bytecode, test_config, None), + Ok(()) + ); } #[test] diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs index bf5ab2f996..a9d1aaaa88 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs @@ -234,7 +234,9 @@ mod test { test::{calc_memory_copier_gas_cost, rand_bytes, run_test_circuit_incomplete_fixed_table}, witness::{Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, Transaction}, }; + use crate::test_util::{test_circuits_using_bytecode, BytecodeTestConfig}; use eth_types::{ + bytecode, evm_types::{GasCost, OpcodeId}, ToBigEndian, Word, }; @@ -242,140 +244,20 @@ mod test { use pairing::bn256::Fr as Fp; fn test_ok_root(call_data_length: usize, memory_offset: Word, data_offset: Word, length: Word) { - let randomness = Fp::rand(); - let bytecode = Bytecode::new( - [ - vec![OpcodeId::PUSH32.as_u8()], - length.to_be_bytes().to_vec(), - vec![OpcodeId::PUSH32.as_u8()], - data_offset.to_be_bytes().to_vec(), - vec![OpcodeId::PUSH32.as_u8()], - memory_offset.to_be_bytes().to_vec(), - vec![OpcodeId::CALLDATACOPY.as_u8(), OpcodeId::STOP.as_u8()], - ] - .concat(), - ); - let call_id = 1; - let call_data: Vec = rand_bytes(call_data_length); - - let mut rws = RwMap( - [ - ( - RwTableTag::Stack, - vec![ - Rw::Stack { - rw_counter: 1, - is_write: false, - call_id, - stack_pointer: 1021, - value: memory_offset, - }, - Rw::Stack { - rw_counter: 2, - is_write: false, - call_id, - stack_pointer: 1022, - value: data_offset, - }, - Rw::Stack { - rw_counter: 3, - is_write: false, - call_id, - stack_pointer: 1023, - value: length, - }, - ], - ), - ( - RwTableTag::CallContext, - vec![Rw::CallContext { - rw_counter: 4, - is_write: false, - call_id, - field_tag: CallContextFieldTag::TxId, - value: Word::one(), - }], - ), - ] - .into(), - ); - let mut rw_counter = 5; - - let next_memory_word_size = if length.is_zero() { - 0 - } else { - (memory_offset.as_u64() + length.as_u64() + 31) / 32 + let bytecode = bytecode! { + PUSH32(length) + PUSH32(data_offset) + PUSH32(memory_offset) + #[start] + CALLDATACOPY + STOP }; - let gas_cost = GasCost::FASTEST.as_u64() - + calc_memory_copier_gas_cost(0, next_memory_word_size, length.as_u64()); - - let mut steps = vec![ExecStep { - rw_indices: vec![ - (RwTableTag::Stack, 0), - (RwTableTag::Stack, 1), - (RwTableTag::Stack, 2), - (RwTableTag::CallContext, 0), - ], - execution_state: ExecutionState::CALLDATACOPY, - rw_counter: 1, - program_counter: 99, - stack_pointer: 1021, - gas_left: gas_cost, - gas_cost, - memory_size: 0, - opcode: Some(OpcodeId::CALLDATACOPY), - ..Default::default() - }]; - - if !length.is_zero() { - make_memory_copy_steps( - call_id, - &call_data, - 0, - data_offset.as_u64(), - memory_offset.as_u64(), - length.as_usize(), - true, - 100, - 1024, - next_memory_word_size * 32, - &mut rw_counter, - &mut rws, - &mut steps, - ); - } - - steps.push(ExecStep { - execution_state: ExecutionState::STOP, - rw_counter, - program_counter: 100, - stack_pointer: 1024, - opcode: Some(OpcodeId::STOP), - memory_size: next_memory_word_size * 32, - ..Default::default() - }); - - let block = Block { - randomness, - txs: vec![Transaction { - id: 1, - call_data, - call_data_length, - calls: vec![Call { - id: call_id, - is_root: true, - is_create: false, - code_source: CodeSource::Account(bytecode.hash), - ..Default::default() - }], - steps, - ..Default::default() - }], - rws, - bytecodes: vec![bytecode], - ..Default::default() - }; - assert_eq!(run_test_circuit_incomplete_fixed_table(block), Ok(())); + let call_data = rand_bytes(call_data_length); + let test_config = BytecodeTestConfig::default(); + assert_eq!( + test_circuits_using_bytecode(bytecode, test_config, Some(call_data)), + Ok(()) + ); } fn test_ok_internal( @@ -561,6 +443,7 @@ mod test { #[test] fn calldatacopy_gadget_multi_step() { + test_ok_root(128, Word::from(0x40), Word::from(16), Word::from(90)); test_ok_internal( Word::from(0x40), Word::from(128), diff --git a/zkevm-circuits/src/evm_circuit/execution/calldataload.rs b/zkevm-circuits/src/evm_circuit/execution/calldataload.rs index 4e7829d4b5..1d05fbb908 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldataload.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldataload.rs @@ -244,7 +244,7 @@ impl ExecutionGadget for CallDataLoadGadget { src_addr as u64, src_addr_end as u64, &calldata_bytes, - &[1u8; N_BYTES_WORD], + &[true; N_BYTES_WORD], )?; Ok(()) diff --git a/zkevm-circuits/src/evm_circuit/execution/gas.rs b/zkevm-circuits/src/evm_circuit/execution/gas.rs index 40be9b035d..9d4bde8454 100644 --- a/zkevm-circuits/src/evm_circuit/execution/gas.rs +++ b/zkevm-circuits/src/evm_circuit/execution/gas.rs @@ -115,7 +115,7 @@ mod test { }; let config = BytecodeTestConfig::default(); let block_trace = BlockData::new_from_geth_data( - new_single_tx_trace_code_gas(&bytecode, Gas(config.gas_limit)) + new_single_tx_trace_code_gas(&bytecode, Gas(config.gas_limit), None) .expect("could not build block trace"), ); let mut builder = block_trace.new_circuit_input_builder(); diff --git a/zkevm-circuits/src/evm_circuit/execution/memory.rs b/zkevm-circuits/src/evm_circuit/execution/memory.rs index 31c4050830..d0c0e251ff 100644 --- a/zkevm-circuits/src/evm_circuit/execution/memory.rs +++ b/zkevm-circuits/src/evm_circuit/execution/memory.rs @@ -217,7 +217,10 @@ mod test { enable_state_circuit_test: false, ..Default::default() }; - assert_eq!(test_circuits_using_bytecode(bytecode, test_config), Ok(())); + assert_eq!( + test_circuits_using_bytecode(bytecode, test_config, None), + Ok(()) + ); } #[test] diff --git a/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs b/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs index 30362db32f..aba31b6906 100644 --- a/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/memory_copy.rs @@ -10,10 +10,11 @@ use crate::{ memory_gadget::BufferReaderGadget, Cell, }, - witness::{Block, Call, ExecStep, StepAuxiliaryData, Transaction}, + witness::{Block, Call, ExecStep, Transaction}, }, util::Expr, }; +use bus_mapping::circuit_input_builder::StepAuxiliaryData; use eth_types::Field; use halo2_proofs::{circuit::Region, plonk::Error}; @@ -172,7 +173,6 @@ impl ExecutionGadget for CopyToMemoryGadget { bytes_left, src_addr_end, from_tx, - selectors, } = step.aux_data.as_ref().unwrap(); self.src_addr @@ -188,16 +188,16 @@ impl ExecutionGadget for CopyToMemoryGadget { self.tx_id .assign(region, offset, Some(F::from(tx.id as u64)))?; - // Retrieve the bytes - assert_eq!(selectors.len(), MAX_COPY_BYTES); + // Fill in selectors and bytes let mut rw_idx = 0; let mut bytes = vec![0u8; MAX_COPY_BYTES]; - for (idx, selector) in selectors.iter().enumerate() { - let addr = *src_addr as usize + idx; - bytes[idx] = if *selector == 1 && addr < *src_addr_end as usize { + let mut selectors = vec![false; MAX_COPY_BYTES]; + for idx in 0..std::cmp::min(*bytes_left as usize, MAX_COPY_BYTES) { + let src_addr = *src_addr as usize + idx; + selectors[idx] = true; + bytes[idx] = if selectors[idx] && src_addr < *src_addr_end as usize { if *from_tx { - assert!(addr < tx.call_data.len()); - tx.call_data[addr] + tx.call_data[src_addr] } else { rw_idx += 1; block.rws[step.rw_indices[rw_idx]].memory_value() @@ -205,16 +205,14 @@ impl ExecutionGadget for CopyToMemoryGadget { } else { 0 }; - if *selector == 1 { - // increase rw_idx for writing back to memory - rw_idx += 1 - } + // increase rw_idx for writing back to memory + rw_idx += 1 } self.buffer_reader - .assign(region, offset, *src_addr, *src_addr_end, &bytes, selectors)?; + .assign(region, offset, *src_addr, *src_addr_end, &bytes, &selectors)?; - let num_bytes_copied = selectors.iter().fold(0, |acc, s| acc + (*s as u64)); + let num_bytes_copied = std::cmp::min(*bytes_left, MAX_COPY_BYTES as u64); self.finish_gadget.assign( region, offset, @@ -233,11 +231,9 @@ pub mod test { step::ExecutionState, table::RwTableTag, test::{rand_bytes, run_test_circuit_incomplete_fixed_table}, - witness::{ - Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, StepAuxiliaryData, Transaction, - }, + witness::{Block, Bytecode, Call, CodeSource, ExecStep, Rw, RwMap, Transaction}, }; - //use crate::evm_circuit::witness::RwMap; + use bus_mapping::circuit_input_builder::StepAuxiliaryData; use eth_types::evm_types::OpcodeId; use halo2_proofs::arithmetic::BaseExt; use pairing::bn256::Fr as Fp; @@ -258,39 +254,36 @@ pub mod test { rws: &mut RwMap, bytes_map: &HashMap, ) -> (ExecStep, usize) { - let mut selectors = vec![0u8; MAX_COPY_BYTES]; let mut rw_offset: usize = 0; let memory_rws: &mut Vec<_> = rws.0.entry(RwTableTag::Memory).or_insert_with(Vec::new); let rw_idx_start = memory_rws.len(); - for (idx, selector) in selectors.iter_mut().enumerate() { - if idx < bytes_left { - *selector = 1; - let addr = src_addr + idx as u64; - let byte = if addr < src_addr_end { - assert!(bytes_map.contains_key(&addr)); - if !from_tx { - memory_rws.push(Rw::Memory { - rw_counter: rw_counter + rw_offset, - is_write: false, - call_id, - memory_address: src_addr + idx as u64, - byte: bytes_map[&addr], - }); - rw_offset += 1; - } - bytes_map[&addr] - } else { - 0 - }; - memory_rws.push(Rw::Memory { - rw_counter: rw_counter + rw_offset, - is_write: true, - call_id, - memory_address: dst_addr + idx as u64, - byte, - }); - rw_offset += 1; - } + for idx in 0..std::cmp::min(bytes_left, MAX_COPY_BYTES) { + let addr = src_addr + idx as u64; + let byte = if addr < src_addr_end { + assert!(bytes_map.contains_key(&addr)); + if !from_tx { + memory_rws.push(Rw::Memory { + rw_counter: rw_counter + rw_offset, + is_write: false, + call_id, + memory_address: src_addr + idx as u64, + byte: bytes_map[&addr], + }); + rw_offset += 1; + } + bytes_map[&addr] + } else { + 0 + }; + // write to destination memory + memory_rws.push(Rw::Memory { + rw_counter: rw_counter + rw_offset, + is_write: true, + call_id, + memory_address: dst_addr + idx as u64, + byte, + }); + rw_offset += 1; } let rw_idx_end = rws.0[&RwTableTag::Memory].len(); let aux_data = StepAuxiliaryData::CopyToMemory { @@ -299,7 +292,6 @@ pub mod test { bytes_left: bytes_left as u64, src_addr_end, from_tx, - selectors, }; let step = ExecStep { execution_state: ExecutionState::CopyToMemory, diff --git a/zkevm-circuits/src/evm_circuit/util/memory_gadget.rs b/zkevm-circuits/src/evm_circuit/util/memory_gadget.rs index d510ad3e7e..9c1b48ef15 100644 --- a/zkevm-circuits/src/evm_circuit/util/memory_gadget.rs +++ b/zkevm-circuits/src/evm_circuit/util/memory_gadget.rs @@ -396,15 +396,17 @@ impl MemoryCopierGasGadget { #[derive(Clone, Debug)] pub(crate) struct BufferReaderGadget { - // The bytes read from buffer + /// The bytes read from buffer bytes: [Cell; MAX_BYTES], - // The selectors that indicate if bytes contain real data + /// The selectors that indicate if the corrsponding byte contains real data + /// or is padded selectors: [Cell; MAX_BYTES], - // bound_dist[i] = max(addr_end - addr_start - i, 0) + /// `bound_dist` is defined as `max(addr_end - addr_start - i, 0)` for `i` + /// in 0..MAX_BYTES bound_dist: [Cell; MAX_BYTES], - // Check if bound_dist is zero + /// Check if bound_dist is zero bound_dist_is_zero: [IsZeroGadget; MAX_BYTES], - // The min gadget to take the minimum of addr_start and addr_end + /// The min gadget to take the minimum of addr_start and addr_end min_gadget: MinMaxGadget, } @@ -492,7 +494,7 @@ impl addr_start: u64, addr_end: u64, bytes: &[u8], - selectors: &[u8], + selectors: &[bool], ) -> Result<(), Error> { self.min_gadget .assign(region, offset, F::from(addr_start), F::from(addr_end))?; diff --git a/zkevm-circuits/src/evm_circuit/witness.rs b/zkevm-circuits/src/evm_circuit/witness.rs index b5f99c7d6b..de71f1d04a 100644 --- a/zkevm-circuits/src/evm_circuit/witness.rs +++ b/zkevm-circuits/src/evm_circuit/witness.rs @@ -7,7 +7,7 @@ use crate::evm_circuit::{ }, util::RandomLinearCombination, }; -use bus_mapping::circuit_input_builder::{self, ExecError, OogError}; +use bus_mapping::circuit_input_builder::{self, ExecError, OogError, StepAuxiliaryData}; use bus_mapping::operation::{self, AccountField, CallContextField}; use eth_types::evm_types::OpcodeId; use eth_types::{Address, Field, ToLittleEndian, ToScalar, ToWord, Word}; @@ -271,18 +271,6 @@ pub struct Call { pub is_static: bool, } -#[derive(Clone, Debug)] -pub enum StepAuxiliaryData { - CopyToMemory { - src_addr: u64, - dst_addr: u64, - bytes_left: u64, - src_addr_end: u64, - from_tx: bool, - selectors: Vec, - }, -} - #[derive(Clone, Debug, Default)] pub struct ExecStep { /// The index in the Transaction calls @@ -1068,60 +1056,61 @@ impl From<&ExecError> for ExecutionState { } } -impl From<&bus_mapping::circuit_input_builder::ExecStep> for ExecutionState { - fn from(step: &bus_mapping::circuit_input_builder::ExecStep) -> Self { +impl From<&circuit_input_builder::ExecStep> for ExecutionState { + fn from(step: &circuit_input_builder::ExecStep) -> Self { if let Some(error) = step.error.as_ref() { return error.into(); } - if step.op.is_dup() { - return ExecutionState::DUP; - } - if step.op.is_push() { - return ExecutionState::PUSH; - } - if step.op.is_swap() { - return ExecutionState::SWAP; - } - match step.op { - OpcodeId::ADD => ExecutionState::ADD, - OpcodeId::MUL => ExecutionState::MUL, - OpcodeId::SUB => ExecutionState::ADD, - OpcodeId::EQ | OpcodeId::LT | OpcodeId::GT => ExecutionState::CMP, - OpcodeId::SLT | OpcodeId::SGT => ExecutionState::SCMP, - OpcodeId::SIGNEXTEND => ExecutionState::SIGNEXTEND, - // TODO: Convert REVERT and RETURN to their own ExecutionState. - OpcodeId::STOP | OpcodeId::RETURN | OpcodeId::REVERT => ExecutionState::STOP, - OpcodeId::AND => ExecutionState::BITWISE, - OpcodeId::XOR => ExecutionState::BITWISE, - OpcodeId::OR => ExecutionState::BITWISE, - OpcodeId::POP => ExecutionState::POP, - OpcodeId::PUSH32 => ExecutionState::PUSH, - OpcodeId::BYTE => ExecutionState::BYTE, - OpcodeId::MLOAD => ExecutionState::MEMORY, - OpcodeId::MSTORE => ExecutionState::MEMORY, - OpcodeId::MSTORE8 => ExecutionState::MEMORY, - OpcodeId::JUMPDEST => ExecutionState::JUMPDEST, - OpcodeId::JUMP => ExecutionState::JUMP, - OpcodeId::JUMPI => ExecutionState::JUMPI, - OpcodeId::PC => ExecutionState::PC, - OpcodeId::MSIZE => ExecutionState::MSIZE, - OpcodeId::CALLER => ExecutionState::CALLER, - OpcodeId::CALLVALUE => ExecutionState::CALLVALUE, - OpcodeId::COINBASE => ExecutionState::COINBASE, - OpcodeId::TIMESTAMP => ExecutionState::TIMESTAMP, - OpcodeId::NUMBER => ExecutionState::NUMBER, - OpcodeId::GAS => ExecutionState::GAS, - OpcodeId::SELFBALANCE => ExecutionState::SELFBALANCE, - OpcodeId::SLOAD => ExecutionState::SLOAD, - OpcodeId::SSTORE => ExecutionState::SSTORE, - // TODO: Use better way to convert BeginTx and EndTx. - OpcodeId::INVALID(_) if [19, 21].contains(&step.bus_mapping_instance.len()) => { - ExecutionState::BeginTx - } - OpcodeId::INVALID(_) if [4, 5].contains(&step.bus_mapping_instance.len()) => { - ExecutionState::EndTx + match step.exec_state { + circuit_input_builder::ExecState::Op(op) => { + if op.is_dup() { + return ExecutionState::DUP; + } + if op.is_push() { + return ExecutionState::PUSH; + } + if op.is_swap() { + return ExecutionState::SWAP; + } + match op { + OpcodeId::ADD => ExecutionState::ADD, + OpcodeId::MUL => ExecutionState::MUL, + OpcodeId::SUB => ExecutionState::ADD, + OpcodeId::EQ | OpcodeId::LT | OpcodeId::GT => ExecutionState::CMP, + OpcodeId::SLT | OpcodeId::SGT => ExecutionState::SCMP, + OpcodeId::SIGNEXTEND => ExecutionState::SIGNEXTEND, + // TODO: Convert REVERT and RETURN to their own ExecutionState. + OpcodeId::STOP | OpcodeId::RETURN | OpcodeId::REVERT => ExecutionState::STOP, + OpcodeId::AND => ExecutionState::BITWISE, + OpcodeId::XOR => ExecutionState::BITWISE, + OpcodeId::OR => ExecutionState::BITWISE, + OpcodeId::POP => ExecutionState::POP, + OpcodeId::PUSH32 => ExecutionState::PUSH, + OpcodeId::BYTE => ExecutionState::BYTE, + OpcodeId::MLOAD => ExecutionState::MEMORY, + OpcodeId::MSTORE => ExecutionState::MEMORY, + OpcodeId::MSTORE8 => ExecutionState::MEMORY, + OpcodeId::JUMPDEST => ExecutionState::JUMPDEST, + OpcodeId::JUMP => ExecutionState::JUMP, + OpcodeId::JUMPI => ExecutionState::JUMPI, + OpcodeId::PC => ExecutionState::PC, + OpcodeId::MSIZE => ExecutionState::MSIZE, + OpcodeId::CALLER => ExecutionState::CALLER, + OpcodeId::CALLVALUE => ExecutionState::CALLVALUE, + OpcodeId::COINBASE => ExecutionState::COINBASE, + OpcodeId::TIMESTAMP => ExecutionState::TIMESTAMP, + OpcodeId::NUMBER => ExecutionState::NUMBER, + OpcodeId::GAS => ExecutionState::GAS, + OpcodeId::SELFBALANCE => ExecutionState::SELFBALANCE, + OpcodeId::SLOAD => ExecutionState::SLOAD, + OpcodeId::SSTORE => ExecutionState::SSTORE, + OpcodeId::CALLDATACOPY => ExecutionState::CALLDATACOPY, + _ => unimplemented!("unimplemented opcode {:?}", op), + } } - _ => unimplemented!("unimplemented opcode {:?}", step.op), + circuit_input_builder::ExecState::BeginTx => ExecutionState::BeginTx, + circuit_input_builder::ExecState::EndTx => ExecutionState::EndTx, + circuit_input_builder::ExecState::CopyToMemory => ExecutionState::CopyToMemory, } } } @@ -1161,10 +1150,13 @@ fn step_convert(step: &circuit_input_builder::ExecStep) -> ExecStep { stack_pointer: STACK_CAPACITY - step.stack_size, gas_left: step.gas_left.0, gas_cost: step.gas_cost.as_u64(), - opcode: Some(step.op), + opcode: match step.exec_state { + circuit_input_builder::ExecState::Op(op) => Some(op), + _ => None, + }, memory_size: step.memory_size as u64, state_write_counter: step.swc, - aux_data: Default::default(), + aux_data: step.aux_data.clone().map(Into::into), } } diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index ed955b3f03..f435d04770 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -28,6 +28,7 @@ pub fn get_fixed_table(conf: FixedTableConfig) -> Vec { } } +#[derive(Debug, Clone)] pub struct BytecodeTestConfig { pub enable_evm_circuit_test: bool, pub evm_circuit_lookup_tags: Vec, @@ -38,25 +39,26 @@ pub struct BytecodeTestConfig { impl Default for BytecodeTestConfig { fn default() -> Self { Self { - gas_limit: 1_000_000u64, enable_evm_circuit_test: true, enable_state_circuit_test: true, + gas_limit: 1_000_000u64, evm_circuit_lookup_tags: get_fixed_table(FixedTableConfig::Incomplete), } } } pub fn run_test_circuits(bytecode: eth_types::Bytecode) -> Result<(), Vec> { - test_circuits_using_bytecode(bytecode, BytecodeTestConfig::default()) + test_circuits_using_bytecode(bytecode, BytecodeTestConfig::default(), None) } pub fn test_circuits_using_bytecode( bytecode: eth_types::Bytecode, config: BytecodeTestConfig, + call_data: Option>, ) -> Result<(), Vec> { // execute the bytecode and get trace let block_trace = bus_mapping::mock::BlockData::new_from_geth_data( - mock::new_single_tx_trace_code_gas(&bytecode, Gas(config.gas_limit)).unwrap(), + mock::new_single_tx_trace_code_gas(&bytecode, Gas(config.gas_limit), call_data).unwrap(), ); let mut builder = block_trace.new_circuit_input_builder(); builder @@ -65,6 +67,7 @@ pub fn test_circuits_using_bytecode( // build a witness block from trace result let block = crate::evm_circuit::witness::block_convert(&builder.block, &builder.code_db); + // finish required tests according to config using this witness block test_circuits_using_witness_block(block, config) } @@ -86,7 +89,7 @@ pub fn test_circuits_using_witness_block( // circuit must be same if config.enable_state_circuit_test { let state_circuit = - StateCircuit::::new(block.randomness, &block.rws); + StateCircuit::::new(block.randomness, &block.rws); let prover = MockProver::::run(12, &state_circuit, vec![]).unwrap(); prover.verify()?; }