diff --git a/bus-mapping/Cargo.toml b/bus-mapping/Cargo.toml index 96d833f9af..7fc367a2bd 100644 --- a/bus-mapping/Cargo.toml +++ b/bus-mapping/Cargo.toml @@ -40,7 +40,7 @@ rand.workspace = true rayon.workspace = true [features] -default = ["test", "enable-stack", "enable-storage"] +default = ["test", "enable-storage"] test = ["mock", "rand"] scroll = ["eth-types/scroll", "mock?/scroll"] # Enable shanghai feature of mock only if mock is enabled (by test). diff --git a/bus-mapping/src/circuit_input_builder.rs b/bus-mapping/src/circuit_input_builder.rs index d3e5ab1f8f..8fa700498c 100644 --- a/bus-mapping/src/circuit_input_builder.rs +++ b/bus-mapping/src/circuit_input_builder.rs @@ -27,10 +27,10 @@ pub use call::{Call, CallContext, CallKind}; use core::fmt::Debug; use eth_types::{ self, - evm_types::{GasCost, OpcodeId}, + evm_types::GasCost, geth_types, sign_types::{pk_bytes_le, pk_bytes_swap_endianness, SignData}, - Address, GethExecStep, GethExecTrace, ToBigEndian, ToWord, Word, H256, + Address, GethExecTrace, ToBigEndian, ToWord, Word, H256, }; use ethers_providers::JsonRpcClient; pub use execution::{ @@ -55,6 +55,9 @@ pub use transaction::{ Transaction, TransactionContext, TxL1Fee, TX_L1_COMMIT_EXTRA_COST, TX_L1_FEE_PRECISION, }; +#[cfg(feature = "enable-stack")] +use eth_types::evm_types::OpcodeId; + /// Setup parameters for ECC-related precompile calls. #[derive(Debug, Clone, Copy)] pub struct PrecompileEcParams { @@ -616,53 +619,58 @@ impl<'a> CircuitInputBuilder { state_ref.call().map(|c| c.call_id).unwrap_or(0), state_ref.call_ctx()?.memory.len(), geth_step.refund.0, - if geth_step.op.is_push_with_data() { - format!("{:?}", geth_trace.struct_logs.get(index + 1).map(|step| step.stack.last())) - } else if geth_step.op.is_call_without_value() { - format!( - "{:?} {:40x} {:?} {:?} {:?} {:?}", - geth_step.stack.last(), - geth_step.stack.nth_last(1).unwrap_or_default(), - geth_step.stack.nth_last(2), - geth_step.stack.nth_last(3), - geth_step.stack.nth_last(4), - geth_step.stack.nth_last(5) - ) - } else if geth_step.op.is_call_with_value() { - format!( - "{:?} {:40x} {:?} {:?} {:?} {:?} {:?}", - geth_step.stack.last(), - geth_step.stack.nth_last(1).unwrap_or_default(), - geth_step.stack.nth_last(2), - geth_step.stack.nth_last(3), - geth_step.stack.nth_last(4), - geth_step.stack.nth_last(5), - geth_step.stack.nth_last(6), - ) - } else if geth_step.op.is_create() { - format!( - "value {:?} offset {:?} size {:?} {}", - geth_step.stack.last(), - geth_step.stack.nth_last(1), - geth_step.stack.nth_last(2), - if geth_step.op == OpcodeId::CREATE2 { - format!("salt {:?}", geth_step.stack.nth_last(3)) - } else { - "".to_string() - } - ) - } else if matches!(geth_step.op, OpcodeId::SSTORE) { - format!( - "{:?} {:?} {:?}", - state_ref.call().map(|c| c.address), - geth_step.stack.last(), - geth_step.stack.nth_last(1), - ) - } else { - let stack_input_num = 1024 - geth_step.op.valid_stack_ptr_range().1 as usize; - (0..stack_input_num).map(|i| - format!("{:?}", geth_step.stack.nth_last(i)) - ).collect_vec().join(" ") + { + #[cfg(feature = "enable-stack")] + if geth_step.op.is_push_with_data() { + format!("{:?}", geth_trace.struct_logs.get(index + 1).map(|step| step.stack.last())) + } else if geth_step.op.is_call_without_value() { + format!( + "{:?} {:40x} {:?} {:?} {:?} {:?}", + geth_step.stack.last(), + geth_step.stack.nth_last(1).unwrap_or_default(), + geth_step.stack.nth_last(2), + geth_step.stack.nth_last(3), + geth_step.stack.nth_last(4), + geth_step.stack.nth_last(5) + ) + } else if geth_step.op.is_call_with_value() { + format!( + "{:?} {:40x} {:?} {:?} {:?} {:?} {:?}", + geth_step.stack.last(), + geth_step.stack.nth_last(1).unwrap_or_default(), + geth_step.stack.nth_last(2), + geth_step.stack.nth_last(3), + geth_step.stack.nth_last(4), + geth_step.stack.nth_last(5), + geth_step.stack.nth_last(6), + ) + } else if geth_step.op.is_create() { + format!( + "value {:?} offset {:?} size {:?} {}", + geth_step.stack.last(), + geth_step.stack.nth_last(1), + geth_step.stack.nth_last(2), + if geth_step.op == OpcodeId::CREATE2 { + format!("salt {:?}", geth_step.stack.nth_last(3)) + } else { + "".to_string() + } + ) + } else if matches!(geth_step.op, OpcodeId::SSTORE) { + format!( + "{:?} {:?} {:?}", + state_ref.call().map(|c| c.address), + geth_step.stack.last(), + geth_step.stack.nth_last(1), + ) + } else { + let stack_input_num = 1024 - geth_step.op.valid_stack_ptr_range().1 as usize; + (0..stack_input_num).map(|i| + format!("{:?}", geth_step.stack.nth_last(i)) + ).collect_vec().join(" ") + } + #[cfg(not(feature = "enable-stack"))] + "N/A".to_string() } ); debug_assert_eq!( @@ -910,9 +918,9 @@ pub fn keccak_inputs_tx_circuit(txs: &[geth_types::Transaction]) -> Result Result, Error> { - let offset = step.stack.nth_last(1)?.low_u64() as usize; - let length = step.stack.nth_last(2)?.as_usize(); +pub fn get_create_init_code(call_ctx: &CallContext) -> Result, Error> { + let offset = call_ctx.stack.nth_last(1)?.low_u64() as usize; + let length = call_ctx.stack.nth_last(2)?.as_usize(); let mem_len = call_ctx.memory.0.len(); let mut result = vec![0u8; length]; @@ -925,9 +933,12 @@ pub fn get_create_init_code(call_ctx: &CallContext, step: &GethExecStep) -> Resu } /// Retrieve the memory offset and length of call. -pub fn get_call_memory_offset_length(step: &GethExecStep, nth: usize) -> Result<(u64, u64), Error> { - let offset = step.stack.nth_last(nth)?; - let length = step.stack.nth_last(nth + 1)?; +pub fn get_call_memory_offset_length( + call_ctx: &CallContext, + nth: usize, +) -> Result<(u64, u64), Error> { + let offset = call_ctx.stack.nth_last(nth)?; + let length = call_ctx.stack.nth_last(nth + 1)?; if length.is_zero() { Ok((0, 0)) } else { diff --git a/bus-mapping/src/circuit_input_builder/call.rs b/bus-mapping/src/circuit_input_builder/call.rs index 4195fb163a..ee9ffe54cc 100644 --- a/bus-mapping/src/circuit_input_builder/call.rs +++ b/bus-mapping/src/circuit_input_builder/call.rs @@ -1,7 +1,7 @@ use super::CodeSource; use crate::{exec_trace::OperationRef, Error}; use eth_types::{ - evm_types::{Memory, OpcodeId}, + evm_types::{Memory, OpcodeId, Stack}, Address, Hash, Word, }; @@ -130,7 +130,7 @@ impl Call { } /// Context of a [`Call`]. -#[derive(Debug, Default)] +#[derive(Debug, Clone, Default)] pub struct CallContext { /// Index of call pub index: usize, @@ -143,6 +143,8 @@ pub struct CallContext { pub call_data: Vec, /// memory context of current call pub memory: Memory, + /// stack context of current call + pub stack: Stack, /// return data buffer pub return_data: Vec, } diff --git a/bus-mapping/src/circuit_input_builder/execution.rs b/bus-mapping/src/circuit_input_builder/execution.rs index f3ae4300f2..00c1dbe146 100644 --- a/bus-mapping/src/circuit_input_builder/execution.rs +++ b/bus-mapping/src/circuit_input_builder/execution.rs @@ -81,7 +81,7 @@ impl ExecStep { ExecStep { exec_state: ExecState::Op(step.op), pc: step.pc, - stack_size: step.stack.0.len(), + stack_size: call_ctx.stack.0.len(), memory_size: call_ctx.memory.len(), gas_left: step.gas, gas_cost: step.gas_cost, diff --git a/bus-mapping/src/circuit_input_builder/input_state_ref.rs b/bus-mapping/src/circuit_input_builder/input_state_ref.rs index 49a35db807..bbed79b5b9 100644 --- a/bus-mapping/src/circuit_input_builder/input_state_ref.rs +++ b/bus-mapping/src/circuit_input_builder/input_state_ref.rs @@ -375,12 +375,23 @@ impl<'a> CircuitInputStateRef<'a> { Ok(()) } + /// same as stack_write, but update the call ctx's stack + pub fn stack_push(&mut self, step: &mut ExecStep, value: Word) -> Result<(), Error> { + let call_id = self.call()?.call_id; + let stack = &mut self.call_ctx_mut()?.stack; + stack.push(value)?; + let address = stack.nth_last_filled(0); + self.push_op(step, RW::WRITE, StackOp::new(call_id, address, value))?; + Ok(()) + } + /// Push a read type [`StackOp`] into the /// [`OperationContainer`](crate::operation::OperationContainer) with the /// next [`RWCounter`](crate::operation::RWCounter) and `call_id`, 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`](crate::operation::RWCounter) by one. + /// TODO: change signature and removes the `value` parameter pub fn stack_read( &mut self, step: &mut ExecStep, @@ -392,6 +403,23 @@ impl<'a> CircuitInputStateRef<'a> { Ok(()) } + /// same as stack_read, but update the call ctx's stack + pub fn stack_pop(&mut self, step: &mut ExecStep) -> Result { + let call_id = self.call()?.call_id; + let stack = &mut self.call_ctx_mut()?.stack; + let address = stack.nth_last_filled(0); + let value = stack.pop()?; + self.push_op(step, RW::READ, StackOp::new(call_id, address, value))?; + Ok(value) + } + + /// pop n values from stack variant + pub fn stack_pops(&mut self, step: &mut ExecStep, n: usize) -> Result, Error> { + (0..n) + .map(|_| self.stack_pop(step)) + .collect::, Error>>() + } + /// First check the validity and consistency of the rw operation against the /// account in the StateDB, then if the rw operation is a write, apply /// it to the corresponding account in the StateDB. @@ -954,10 +982,12 @@ impl<'a> CircuitInputStateRef<'a> { /// Return the contract address of a CREATE2 step. This is calculated /// deterministically from the arguments in the stack. - pub(crate) fn create2_address(&self, step: &GethExecStep) -> Result { - let salt = step.stack.nth_last(3)?; + pub(crate) fn create2_address(&self, _step: &GethExecStep) -> Result { let call_ctx = self.call_ctx()?; - let init_code = get_create_init_code(call_ctx, step)?.to_vec(); + #[cfg(feature = "enable-stack")] + assert_eq!(call_ctx.stack, _step.stack); + let salt = call_ctx.stack.nth_last(3)?; + let init_code = get_create_init_code(call_ctx)?.to_vec(); let address = get_create2_address(self.call()?.address, salt.to_be_bytes(), init_code); log::trace!( "create2_address {:?}, from {:?}, salt {:?}", @@ -1013,40 +1043,37 @@ impl<'a> CircuitInputStateRef<'a> { pub fn parse_call_partial(&mut self, step: &GethExecStep) -> Result { let kind = CallKind::try_from(step.op)?; let caller = self.call()?; - let caller_ctx = self.call_ctx()?; + let caller_ctx = self.call_ctx()?.clone(); + let stack = &caller_ctx.stack; + #[cfg(feature = "enable-stack")] + assert_eq!(stack, &step.stack); let (caller_address, address, value) = match kind { CallKind::Call => ( caller.address, - step.stack.nth_last(1)?.to_address(), - step.stack.nth_last(2)?, + stack.nth_last(1)?.to_address(), + stack.nth_last(2)?, ), - CallKind::CallCode => (caller.address, caller.address, step.stack.nth_last(2)?), + CallKind::CallCode => (caller.address, caller.address, stack.nth_last(2)?), CallKind::DelegateCall => (caller.caller_address, caller.address, caller.value), CallKind::StaticCall => ( caller.address, - step.stack.nth_last(1)?.to_address(), + stack.nth_last(1)?.to_address(), Word::zero(), ), - CallKind::Create => (caller.address, self.create_address()?, step.stack.last()?), - CallKind::Create2 => ( - caller.address, - self.create2_address(step)?, - step.stack.last()?, - ), + CallKind::Create => (caller.address, self.create_address()?, stack.last()?), + CallKind::Create2 => (caller.address, self.create2_address(step)?, stack.last()?), }; let (code_source, code_hash) = match kind { CallKind::Create | CallKind::Create2 => { - let init_code = get_create_init_code(caller_ctx, step)?.to_vec(); + let init_code = get_create_init_code(&caller_ctx)?.to_vec(); let code_hash = self.code_db.insert(init_code); (CodeSource::Memory, code_hash) } _ => { let code_address = match kind { - CallKind::CallCode | CallKind::DelegateCall => { - step.stack.nth_last(1)?.to_address() - } + CallKind::CallCode | CallKind::DelegateCall => stack.nth_last(1)?.to_address(), _ => address, }; if is_precompiled(&code_address) { @@ -1065,13 +1092,13 @@ impl<'a> CircuitInputStateRef<'a> { let (call_data_offset, call_data_length, return_data_offset, return_data_length) = match kind { CallKind::Call | CallKind::CallCode => { - let call_data = get_call_memory_offset_length(step, 3)?; - let return_data = get_call_memory_offset_length(step, 5)?; + let call_data = get_call_memory_offset_length(&caller_ctx, 3)?; + let return_data = get_call_memory_offset_length(&caller_ctx, 5)?; (call_data.0, call_data.1, return_data.0, return_data.1) } CallKind::DelegateCall | CallKind::StaticCall => { - let call_data = get_call_memory_offset_length(step, 2)?; - let return_data = get_call_memory_offset_length(step, 4)?; + let call_data = get_call_memory_offset_length(&caller_ctx, 2)?; + let return_data = get_call_memory_offset_length(&caller_ctx, 4)?; (call_data.0, call_data.1, return_data.0, return_data.1) } CallKind::Create | CallKind::Create2 => (0, 0, 0, 0), @@ -1102,11 +1129,18 @@ impl<'a> CircuitInputStateRef<'a> { last_callee_return_data_length: 0, last_callee_memory: Memory::default(), }; - Ok(call) } - /// Parse [`Call`] from a *CALL*/CREATE* step + /// Get the next call's success status. + pub fn next_call_is_success(&self) -> Option { + self.tx_ctx + .call_is_success + .get(self.tx.calls().len() - self.tx_ctx.call_is_success_offset) + .copied() + } + + /// Parse [`Call`] from a *CALL*/CREATE* step. pub fn parse_call(&mut self, step: &GethExecStep) -> Result { let is_success = *self .tx_ctx @@ -1245,6 +1279,7 @@ impl<'a> CircuitInputStateRef<'a> { /// `caller_ctx.return_data` should be updated **before** this method (except error cases). pub fn handle_return( &mut self, + (offset, length): (Option, Option), current_exec_steps: &mut [&mut ExecStep], geth_steps: &[GethExecStep], need_restore: bool, @@ -1276,6 +1311,7 @@ impl<'a> CircuitInputStateRef<'a> { ); } self.handle_restore_context( + (offset, length), current_exec_steps[current_exec_steps.len() - 1], geth_steps, )?; @@ -1288,8 +1324,13 @@ impl<'a> CircuitInputStateRef<'a> { // Store deployed code if it's a successful create if call_success_create { - let offset = step.stack.last()?; - let length = step.stack.nth_last(1)?; + let offset = offset.expect("offset not set"); + let length = length.expect("length not set"); + #[cfg(feature = "enable-stack")] + { + assert_eq!(offset, step.stack.nth_last(0)?); + assert_eq!(length, step.stack.nth_last(1)?); + } let code = callee_memory.read_chunk(MemoryRange::new_with_length( offset.low_u64(), length.low_u64(), @@ -1328,7 +1369,10 @@ impl<'a> CircuitInputStateRef<'a> { && step.error.is_none() && !call_success_create { - step.stack.last()?.low_u64() + let offset = offset.expect("offset not set"); + #[cfg(feature = "enable-stack")] + assert_eq!(offset, step.stack.nth_last(0)?); + offset.low_u64() } else { // common err, call empty, call precompile 0 @@ -1350,6 +1394,7 @@ impl<'a> CircuitInputStateRef<'a> { // be non 0 while the call_ctx.return should be empty for this case. EIP-211: CREATE/CREATE2 // call successful case should set RETURNDATASIZE = 0 fn get_return_data_offset_and_len( + (offset, length): (Option, Option), exec_step: &ExecStep, geth_step: &GethExecStep, caller_ctx: &CallContext, @@ -1374,8 +1419,13 @@ impl<'a> CircuitInputStateRef<'a> { [Word::zero(), return_data_length] } OpcodeId::REVERT | OpcodeId::RETURN => { - let offset = geth_step.stack.last()?; - let length = geth_step.stack.nth_last(1)?; + let offset = offset.expect("offset not set"); + let length = length.expect("length not set"); + #[cfg(feature = "enable-stack")] + { + assert_eq!(offset, geth_step.stack.nth_last(0)?); + assert_eq!(length, geth_step.stack.nth_last(1)?); + } // This is the convention we are using for memory addresses so that there is no // memory expansion cost when the length is 0. // https://github.com/privacy-scaling-explorations/zkevm-circuits/pull/279/files#r787806678 @@ -1397,6 +1447,7 @@ impl<'a> CircuitInputStateRef<'a> { /// Bus mapping for the RestoreContextGadget as used in RETURN. pub fn handle_restore_context( &mut self, + (offset, length): (Option, Option), exec_step: &mut ExecStep, steps: &[GethExecStep], ) -> Result<(), Error> { @@ -1443,6 +1494,8 @@ impl<'a> CircuitInputStateRef<'a> { let geth_step_next = steps .get(1) .ok_or(Error::InternalError("invalid index 1"))?; + #[cfg(feature = "enable-stack")] + assert_eq!(self.caller_ctx()?.stack, geth_step_next.stack); self.call_context_read( exec_step, call.call_id, @@ -1451,7 +1504,12 @@ impl<'a> CircuitInputStateRef<'a> { )?; let (last_callee_return_data_offset, last_callee_return_data_length) = - Self::get_return_data_offset_and_len(exec_step, geth_step, self.caller_ctx()?)?; + Self::get_return_data_offset_and_len( + (offset, length), + exec_step, + geth_step, + self.caller_ctx()?, + )?; let gas_refund = if is_err { 0 @@ -1499,7 +1557,7 @@ impl<'a> CircuitInputStateRef<'a> { (CallContextField::ProgramCounter, geth_step_next.pc.0.into()), ( CallContextField::StackPointer, - geth_step_next.stack.stack_pointer().0.into(), + self.caller_ctx()?.stack.stack_pointer().0.into(), ), (CallContextField::GasLeft, caller_gas_left.into()), ( @@ -1571,6 +1629,7 @@ impl<'a> CircuitInputStateRef<'a> { } let call = self.call()?; + trace!("get_step_err: step:\n\tstep:{step:?}\n\tnext_step:{next_step:?}\n\tcall:{call:?}"); if next_step.is_none() { // enumerating call scope successful cases @@ -1594,20 +1653,23 @@ impl<'a> CircuitInputStateRef<'a> { } let next_depth = next_step.map(|s| s.depth).unwrap_or(0); - let next_result = next_step - .map(|s| s.stack.last().unwrap_or_else(|_| Word::zero())) - .unwrap_or_else(Word::zero); + // let next_result = next_step + // .map(|s| s.stack.last().unwrap_or_else(|_| Word::zero())) + // .unwrap_or_else(Word::zero); + let next_success = self.next_call_is_success().unwrap_or(true); let call_ctx = self.call_ctx()?; + #[cfg(feature = "enable-stack")] + assert_eq!(call_ctx.stack, step.stack); // get value first if call/create let value = match step.op { - OpcodeId::CALL | OpcodeId::CALLCODE => step.stack.nth_last(2)?, - OpcodeId::CREATE | OpcodeId::CREATE2 => step.stack.last()?, + OpcodeId::CALL | OpcodeId::CALLCODE => call_ctx.stack.nth_last(2)?, + OpcodeId::CREATE | OpcodeId::CREATE2 => call_ctx.stack.last()?, _ => Word::zero(), }; // Return from a call with a failure - if step.depth == next_depth + 1 && next_result.is_zero() { + if step.depth == next_depth + 1 && !call.is_success { if !matches!(step.op, OpcodeId::RETURN) { // Without calling RETURN return Ok(match step.op { @@ -1642,8 +1704,8 @@ impl<'a> CircuitInputStateRef<'a> { } else { // Return from a {CREATE, CREATE2} with a failure, via RETURN if call.is_create() { - let offset = step.stack.last()?; - let length = step.stack.nth_last(1)?; + let offset = call_ctx.stack.last()?; + let length = call_ctx.stack.nth_last(1)?; if length > Word::from(MAX_CODE_SIZE) { return Ok(Some(ExecError::MaxCodeSizeExceeded)); } else if length > Word::zero() @@ -1674,7 +1736,7 @@ impl<'a> CircuitInputStateRef<'a> { // Return from a call without calling RETURN or STOP and having success // is unexpected. if step.depth == next_depth + 1 - && next_result != Word::zero() + && call.is_success && !matches!( step.op, OpcodeId::RETURN | OpcodeId::STOP | OpcodeId::SELFDESTRUCT @@ -1697,7 +1759,7 @@ impl<'a> CircuitInputStateRef<'a> { | OpcodeId::STATICCALL | OpcodeId::CREATE | OpcodeId::CREATE2 - ) && next_result.is_zero() + ) && !next_success && next_pc != 0 { if step.depth == 1025 { @@ -1769,7 +1831,7 @@ impl<'a> CircuitInputStateRef<'a> { step.op, OpcodeId::CALL | OpcodeId::CALLCODE | OpcodeId::DELEGATECALL | OpcodeId::STATICCALL ) { - let code_address = step.stack.nth_last(1)?.to_address(); + let code_address = call_ctx.stack.nth_last(1)?.to_address(); // NOTE: we do not know the amount of gas that precompile got here // because the callGasTemp might probably be smaller than the gas // on top of the stack (step.stack.last()) diff --git a/bus-mapping/src/circuit_input_builder/l2.rs b/bus-mapping/src/circuit_input_builder/l2.rs index b480637022..1deedae846 100644 --- a/bus-mapping/src/circuit_input_builder/l2.rs +++ b/bus-mapping/src/circuit_input_builder/l2.rs @@ -2,13 +2,14 @@ pub use super::block::{Block, BlockContext}; use crate::{ circuit_input_builder::{self, BlockHead, CircuitInputBuilder, CircuitsParams}, error::Error, + precompile::is_precompiled, state_db::{self, CodeDB, StateDB}, }; use eth_types::{ self, evm_types::OpcodeId, l2_types::{BlockTrace, EthBlock, ExecStep, StorageTrace}, - Address, ToAddress, ToWord, Word, H256, + Address, ToWord, Word, H256, }; use ethers_core::types::Bytes; use mpt_zktrie::state::{AccountData, ZktrieState}; @@ -51,32 +52,34 @@ fn trace_code( code_hash: Option, code: Bytes, step: &ExecStep, + addr: Option
, sdb: &StateDB, - stack_pos: usize, ) { // first, try to read from sdb - let stack = match step.stack.as_ref() { - Some(stack) => stack, - None => { - log::error!("stack underflow, step {step:?}"); - return; - } - }; - if stack_pos >= stack.len() { - log::error!("stack underflow, step {step:?}"); - return; - } - let addr = stack[stack.len() - stack_pos - 1].to_address(); //stack N-stack_pos - + // let stack = match step.stack.as_ref() { + // Some(stack) => stack, + // None => { + // log::error!("stack underflow, step {step:?}"); + // return; + // } + // }; + // if stack_pos >= stack.len() { + // log::error!("stack underflow, step {step:?}"); + // return; + // } + // let addr = stack[stack.len() - stack_pos - 1].to_address(); //stack N-stack_pos + // let code_hash = code_hash.or_else(|| { - let (_existed, acc_data) = sdb.get_account(&addr); - if acc_data.code_hash != CodeDB::empty_code_hash() && !code.is_empty() { - // they must be same - Some(acc_data.code_hash) - } else { - // let us re-calculate it - None - } + addr.and_then(|addr| { + let (_existed, acc_data) = sdb.get_account(&addr); + if acc_data.code_hash != CodeDB::empty_code_hash() && !code.is_empty() { + // they must be same + Some(acc_data.code_hash) + } else { + // let us re-calculate it + None + } + }) }); let code_hash = match code_hash { Some(code_hash) => { @@ -141,13 +144,57 @@ fn update_codedb(cdb: &mut CodeDB, sdb: &StateDB, block: &BlockTrace) -> Result< } } - for step in execution_result.exec_steps.iter().rev() { + // filter all precompile calls, empty calls and create + let mut call_trace = execution_result + .call_trace + .flatten_trace(&execution_result.prestate) + .into_iter() + .inspect(|c| println!("{c:?}")) + .filter(|call| { + let is_call_to_precompile = call.to.as_ref().map(is_precompiled).unwrap_or(false); + let is_call_to_empty = call.gas_used.is_zero() + && !call.call_type.is_create() + && call.is_callee_code_empty; + !(is_call_to_precompile || is_call_to_empty || call.call_type.is_create()) + }) + .collect::>(); + log::trace!("call_trace: {call_trace:?}"); + + for (idx, step) in execution_result.exec_steps.iter().enumerate().rev() { + if step.op.is_create() { + continue; + } + let call = if step.op.is_call_or_create() { + // filter call to empty/precompile/!precheck_ok + if let Some(next_step) = execution_result.exec_steps.get(idx + 1) { + // the call doesn't have inner steps, it could be: + // - a call to a precompiled contract + // - a call to an empty account + // - a call that !is_precheck_ok + if next_step.depth != step.depth + 1 { + log::trace!("skip call step due to no inner step, curr: {step:?}, next: {next_step:?}"); + continue; + } + } else { + // this is the final step, no inner steps + log::trace!("skip call step due this is the final step: {step:?}"); + continue; + } + let call = call_trace.pop(); + log::trace!("call_trace pop: {call:?}, current step: {step:?}"); + call + } else { + None + }; + if let Some(data) = &step.extra_data { match step.op { OpcodeId::CALL | OpcodeId::CALLCODE | OpcodeId::DELEGATECALL | OpcodeId::STATICCALL => { + let call = call.unwrap(); + assert_eq!(call.call_type, step.op, "{call:?}"); let code_idx = if block.transactions[er_idx].to.is_none() { 0 } else { @@ -164,18 +211,20 @@ fn update_codedb(cdb: &mut CodeDB, sdb: &StateDB, block: &BlockTrace) -> Result< OpcodeId::STATICCALL => data.get_code_hash_at(0), _ => None, }; + let addr = call.to.unwrap(); trace_code( cdb, code_hash, callee_code.unwrap_or_default(), step, + Some(addr), sdb, - 1, ); } OpcodeId::CREATE | OpcodeId::CREATE2 => { // notice we do not need to insert code for CREATE, // bustmapping do this job + unreachable!() } OpcodeId::EXTCODESIZE | OpcodeId::EXTCODECOPY => { let code = data.get_code_at(0); @@ -183,7 +232,7 @@ fn update_codedb(cdb: &mut CodeDB, sdb: &StateDB, block: &BlockTrace) -> Result< log::warn!("unable to fetch code from step. {step:?}"); continue; } - trace_code(cdb, None, code.unwrap(), step, sdb, 0); + trace_code(cdb, None, code.unwrap(), step, None, sdb); } _ => {} diff --git a/bus-mapping/src/circuit_input_builder/tracer_tests.rs b/bus-mapping/src/circuit_input_builder/tracer_tests.rs index d4ef0463ea..d79f7c59c2 100644 --- a/bus-mapping/src/circuit_input_builder/tracer_tests.rs +++ b/bus-mapping/src/circuit_input_builder/tracer_tests.rs @@ -10,7 +10,7 @@ use eth_types::{ address, bytecode, evm_types::{stack::Stack, Gas, Memory, OpcodeId}, geth_types::GethData, - word, Bytecode, GethExecError, Hash, ToAddress, ToWord, Word, + word, Bytecode, GethExecError, GethExecStep, Hash, ToAddress, ToWord, Word, }; use mock::test_ctx::{helpers::*, LoggerConfig, TestContext}; use pretty_assertions::assert_eq; diff --git a/bus-mapping/src/circuit_input_builder/transaction.rs b/bus-mapping/src/circuit_input_builder/transaction.rs index 7e4484e88a..724482892a 100644 --- a/bus-mapping/src/circuit_input_builder/transaction.rs +++ b/bus-mapping/src/circuit_input_builder/transaction.rs @@ -7,7 +7,7 @@ use crate::{ Error, }; use eth_types::{ - evm_types::{gas_utils::tx_data_gas_cost, Memory, OpcodeId}, + evm_types::{gas_utils::tx_data_gas_cost, OpcodeId}, geth_types, geth_types::{get_rlp_unsigned, TxType}, AccessList, Address, GethExecTrace, Signature, Word, H256, @@ -156,8 +156,7 @@ impl TransactionContext { index: call_idx, reversible_write_counter: 0, call_data, - memory: Memory::default(), - return_data: vec![], + ..Default::default() }); } diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index dfd9a1ff19..de3dd4d432 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -12,7 +12,7 @@ use crate::{ use core::fmt::Debug; use eth_types::{evm_unimplemented, GethExecStep, ToAddress, ToWord, Word}; -#[cfg(feature = "enable-memory")] +#[cfg(any(feature = "enable-memory", feature = "enable-stack"))] use crate::util::GETH_TRACE_CHECK_LEVEL; #[cfg(any(feature = "test", test))] @@ -426,6 +426,39 @@ pub fn gen_associated_ops( } } } + #[cfg(feature = "enable-stack")] + if GETH_TRACE_CHECK_LEVEL.should_check() { + if state.call_ctx()?.stack != geth_steps[0].stack { + log::error!( + "wrong stack before {:?}. len in state {}, len in step {}", + opcode_id, + &state.call_ctx()?.stack.len(), + &geth_steps[0].stack.len(), + ); + log::error!("state stack {:?}", &state.call_ctx()?.stack); + log::error!("step stack {:?}", &geth_steps[0].stack); + + for i in 0..std::cmp::min(state.call_ctx()?.stack.0.len(), geth_steps[0].stack.0.len()) + { + let state_stack = state.call_ctx()?.stack.0[i]; + let step_stack = geth_steps[0].stack.0[i]; + if state_stack != step_stack { + log::error!( + "diff at {}: state {:?} != step {:?}", + i, + state_stack, + step_stack + ); + } + } + if GETH_TRACE_CHECK_LEVEL.should_panic() { + panic!("stack wrong"); + } + state.call_ctx_mut()?.stack = geth_steps[0].stack.clone(); + } else { + log::debug!("stack sanity check passed"); + } + } // check if have error let geth_step = &geth_steps[0]; @@ -469,7 +502,12 @@ pub fn gen_associated_ops( need_restore = false; } - state.handle_return(&mut [&mut exec_step], geth_steps, need_restore)?; + state.handle_return( + (None, None), + &mut [&mut exec_step], + geth_steps, + need_restore, + )?; return Ok(vec![exec_step]); } } @@ -516,7 +554,9 @@ fn dummy_gen_selfdestruct_ops( let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; let sender = state.call()?.address; - let receiver = geth_step.stack.last()?.to_address(); + let receiver = state.call_ctx_mut()?.stack.pop()?.to_address(); + #[cfg(feature = "enable-stack")] + assert_eq!(receiver, geth_step.stack.last()?.to_address()); let is_warm = state.sdb.check_account_in_access_list(&receiver); state.push_op_reversible( @@ -594,6 +634,11 @@ fn dummy_gen_selfdestruct_ops( if let Ok(caller) = state.caller_ctx_mut() { caller.return_data.clear(); } - state.handle_return(&mut [&mut exec_step], geth_steps, !state.call()?.is_root)?; + state.handle_return( + (None, None), + &mut [&mut exec_step], + geth_steps, + !state.call()?.is_root, + )?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/address.rs b/bus-mapping/src/evm/opcodes/address.rs index 7a682b7c01..5993635272 100644 --- a/bus-mapping/src/evm/opcodes/address.rs +++ b/bus-mapping/src/evm/opcodes/address.rs @@ -4,7 +4,7 @@ use crate::{ operation::CallContextField, Error, }; -use eth_types::GethExecStep; +use eth_types::{GethExecStep, ToWord}; #[derive(Clone, Copy, Debug)] pub(crate) struct Address; @@ -17,8 +17,9 @@ impl Opcode for Address { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; - // Get address result from next step. - let address = geth_steps[1].stack.last()?; + let address = state.call()?.address.to_word(); + #[cfg(feature = "enable-stack")] + assert_eq!(address, geth_steps[1].stack.last()?); // Read the callee address in call context. state.call_context_read( @@ -29,11 +30,7 @@ impl Opcode for Address { )?; // Write the address to stack. - state.stack_write( - &mut exec_step, - geth_step.stack.last_filled().map(|a| a - 1), - address, - )?; + state.stack_push(&mut exec_step, address)?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/arithmetic.rs b/bus-mapping/src/evm/opcodes/arithmetic.rs index 2ffd3cb1e3..a059983627 100644 --- a/bus-mapping/src/evm/opcodes/arithmetic.rs +++ b/bus-mapping/src/evm/opcodes/arithmetic.rs @@ -4,6 +4,7 @@ use crate::{ Error, }; use eth_types::{evm_types::OpcodeId, GethExecStep, ToBigEndian, ToLittleEndian, Word, U256, U512}; +#[cfg(feature = "enable-stack")] use itertools::Itertools; use std::{ cmp::Ordering, @@ -335,23 +336,19 @@ where let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; - let stack_inputs: [Word; N_POPS] = (0..N_POPS) - .map(|i| geth_step.stack.nth_last(i)) - .collect::, _>>()? + let stack_inputs: [Word; N_POPS] = state + .stack_pops(&mut exec_step, N_POPS)? .try_into() .unwrap(); - for (i, value) in stack_inputs.iter().enumerate() { - state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(i), *value)?; + #[cfg(feature = "enable-stack")] + for (i, input) in stack_inputs.iter().enumerate() { + assert_eq!(*input, geth_step.stack.nth_last(i)?); } - let output = Self::handle(stack_inputs); + state.stack_push(&mut exec_step, output)?; - state.stack_write( - &mut exec_step, - geth_steps[1].stack.nth_last_filled(0), - output, - )?; + #[cfg(feature = "enable-stack")] assert_eq!( output, geth_steps[1].stack.nth_last(0)?, diff --git a/bus-mapping/src/evm/opcodes/balance.rs b/bus-mapping/src/evm/opcodes/balance.rs index 6c334a09b1..1854024238 100644 --- a/bus-mapping/src/evm/opcodes/balance.rs +++ b/bus-mapping/src/evm/opcodes/balance.rs @@ -18,9 +18,10 @@ impl Opcode for Balance { let mut exec_step = state.new_step(geth_step)?; // Read account address from stack. - let address_word = geth_step.stack.last()?; + let address_word = state.stack_pop(&mut exec_step)?; let address = address_word.to_address(); - state.stack_read(&mut exec_step, geth_step.stack.last_filled(), address_word)?; + #[cfg(feature = "enable-stack")] + assert_eq!(address_word, geth_step.stack.last()?); // Read transaction ID, rw_counter_end_of_reversion, and is_persistent // from call context. @@ -64,7 +65,6 @@ impl Opcode for Balance { } else { H256::zero() }; - debug_assert_eq!(balance, geth_steps[1].stack.last()?); state.account_read( &mut exec_step, address, @@ -76,11 +76,9 @@ impl Opcode for Balance { } // Write the BALANCE result to stack. - state.stack_write( - &mut exec_step, - geth_steps[1].stack.last_filled(), - geth_steps[1].stack.last()?, - )?; + #[cfg(feature = "enable-stack")] + assert_eq!(geth_steps[1].stack.last()?, balance); + state.stack_push(&mut exec_step, balance)?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/blockhash.rs b/bus-mapping/src/evm/opcodes/blockhash.rs index ba414338e8..8fa284e9c6 100644 --- a/bus-mapping/src/evm/opcodes/blockhash.rs +++ b/bus-mapping/src/evm/opcodes/blockhash.rs @@ -20,27 +20,29 @@ impl Opcode for Blockhash { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; - let block_number = geth_step.stack.last()?; - state.stack_read(&mut exec_step, geth_step.stack.last_filled(), block_number)?; - - let block_hash = geth_steps[1].stack.last()?; - state.stack_write( - &mut exec_step, - geth_steps[1].stack.last_filled(), - block_hash, - )?; + let block_number = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + assert_eq!(block_number, geth_step.stack.last()?); let current_block_number = state.tx.block_num; - if is_valid_block_number(block_number, current_block_number.into()) { - let (sha3_input, _sha3_output) = - calculate_block_hash(state.block.chain_id, block_number); - state.block.sha3_inputs.push(sha3_input); - - #[cfg(feature = "scroll")] - assert_eq!(block_hash, _sha3_output); + let block_hash = if is_valid_block_number(block_number, current_block_number.into()) { + if cfg!(feature = "scroll") { + let (sha3_input, sha3_output) = + calculate_block_hash(state.block.chain_id, block_number); + state.block.sha3_inputs.push(sha3_input); + sha3_output + } else { + let block_head = state.block.headers.get(¤t_block_number).unwrap(); + let offset = (current_block_number - block_number.as_u64()) as usize; + let total_history_hashes = block_head.history_hashes.len(); + block_head.history_hashes[total_history_hashes - offset] + } } else { - assert_eq!(block_hash, 0.into()); - } + 0.into() + }; + #[cfg(feature = "enable-stack")] + assert_eq!(block_hash, geth_steps[1].stack.last()?); + state.stack_push(&mut exec_step, block_hash)?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/calldatacopy.rs b/bus-mapping/src/evm/opcodes/calldatacopy.rs index 917158c58e..6d93debe1d 100644 --- a/bus-mapping/src/evm/opcodes/calldatacopy.rs +++ b/bus-mapping/src/evm/opcodes/calldatacopy.rs @@ -6,7 +6,7 @@ use crate::{ operation::CallContextField, Error, }; -use eth_types::GethExecStep; +use eth_types::{GethExecStep, Word}; #[derive(Clone, Copy, Debug)] pub(crate) struct Calldatacopy; @@ -17,78 +17,66 @@ impl Opcode for Calldatacopy { geth_steps: &[GethExecStep], ) -> Result, Error> { let geth_step = &geth_steps[0]; - let mut exec_steps = vec![gen_calldatacopy_step(state, geth_step)?]; - let copy_event = gen_copy_event(state, geth_step, &mut exec_steps[0])?; - state.push_copy(&mut exec_steps[0], copy_event); - 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.last()?; - let data_offset = geth_step.stack.nth_last(1)?; - let length = geth_step.stack.nth_last(2)?; - - state.stack_read(&mut exec_step, geth_step.stack.last_filled(), memory_offset)?; - state.stack_read( - &mut exec_step, - geth_step.stack.nth_last_filled(1), - data_offset, - )?; - state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(2), length)?; - if state.call()?.is_root { - state.call_context_read( - &mut exec_step, - state.call()?.call_id, - CallContextField::TxId, - state.tx_ctx.id().into(), - )?; - state.call_context_read( - &mut exec_step, - state.call()?.call_id, - CallContextField::CallDataLength, - state.call()?.call_data_length.into(), - )?; - } else { - state.call_context_read( - &mut exec_step, - state.call()?.call_id, - CallContextField::CallerId, - state.call()?.caller_id.into(), - )?; - state.call_context_read( - &mut exec_step, - state.call()?.call_id, - CallContextField::CallDataLength, - state.call()?.call_data_length.into(), - )?; - state.call_context_read( - &mut exec_step, - state.call()?.call_id, - CallContextField::CallDataOffset, - state.call()?.call_data_offset.into(), - )?; - }; + let mut exec_step = state.new_step(geth_step)?; + let memory_offset = state.stack_pop(&mut exec_step)?; + let data_offset = state.stack_pop(&mut exec_step)?; + let length = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + { + assert_eq!(memory_offset, geth_step.stack.nth_last(0)?); + assert_eq!(data_offset, geth_step.stack.nth_last(1)?); + assert_eq!(length, geth_step.stack.nth_last(2)?); + } + if state.call()?.is_root { + state.call_context_read( + &mut exec_step, + state.call()?.call_id, + CallContextField::TxId, + state.tx_ctx.id().into(), + )?; + state.call_context_read( + &mut exec_step, + state.call()?.call_id, + CallContextField::CallDataLength, + state.call()?.call_data_length.into(), + )?; + } else { + state.call_context_read( + &mut exec_step, + state.call()?.call_id, + CallContextField::CallerId, + state.call()?.caller_id.into(), + )?; + state.call_context_read( + &mut exec_step, + state.call()?.call_id, + CallContextField::CallDataLength, + state.call()?.call_data_length.into(), + )?; + state.call_context_read( + &mut exec_step, + state.call()?.call_id, + CallContextField::CallDataOffset, + state.call()?.call_data_offset.into(), + )?; + }; - Ok(exec_step) + let copy_event = gen_copy_event(state, memory_offset, data_offset, length, &mut exec_step)?; + state.push_copy(&mut exec_step, copy_event); + Ok(vec![exec_step]) + } } fn gen_copy_event( state: &mut CircuitInputStateRef, - geth_step: &GethExecStep, + memory_offset: Word, + data_offset: Word, + length: Word, exec_step: &mut ExecStep, ) -> Result { let rw_counter_start = state.block_ctx.rwc; - let memory_offset = geth_step.stack.last()?; - let data_offset = geth_step.stack.nth_last(1)?; - let length = geth_step.stack.nth_last(2)?; - let (memory_offset, length) = (memory_offset.low_u64(), length.as_u64()); let call_data_offset = state.call()?.call_data_offset; diff --git a/bus-mapping/src/evm/opcodes/calldataload.rs b/bus-mapping/src/evm/opcodes/calldataload.rs index 75b5cf81ba..c349e918fa 100644 --- a/bus-mapping/src/evm/opcodes/calldataload.rs +++ b/bus-mapping/src/evm/opcodes/calldataload.rs @@ -20,8 +20,9 @@ impl Opcode for Calldataload { // fetch the top of the stack, i.e. offset in calldata to start reading 32-bytes // from. - let offset = geth_step.stack.last()?; - state.stack_read(&mut exec_step, geth_step.stack.last_filled(), offset)?; + let offset = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + assert_eq!(offset, geth_step.stack.last()?); // Check if offset is Uint64 overflow. let calldata_word = if let Ok(offset) = u64::try_from(offset) { @@ -96,7 +97,7 @@ impl Opcode for Calldataload { U256::zero() }; - state.stack_write(&mut exec_step, geth_step.stack.last_filled(), calldata_word)?; + state.stack_push(&mut exec_step, calldata_word)?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/calldatasize.rs b/bus-mapping/src/evm/opcodes/calldatasize.rs index 736049951d..5292a63cef 100644 --- a/bus-mapping/src/evm/opcodes/calldatasize.rs +++ b/bus-mapping/src/evm/opcodes/calldatasize.rs @@ -4,7 +4,7 @@ use crate::{ Error, }; -use eth_types::GethExecStep; +use eth_types::{GethExecStep, Word}; use super::Opcode; @@ -18,20 +18,16 @@ impl Opcode for Calldatasize { ) -> Result, Error> { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; - let value = geth_steps[1].stack.last()?; + let call_data_length = Word::from(state.call()?.call_data_length); state.call_context_read( &mut exec_step, state.call()?.call_id, CallContextField::CallDataLength, - value, + call_data_length, )?; - - state.stack_write( - &mut exec_step, - geth_step.stack.last_filled().map(|a| a - 1), - value, - )?; - + #[cfg(feature = "enable-stack")] + assert_eq!(call_data_length, geth_steps[1].stack.last()?); + state.stack_push(&mut exec_step, call_data_length)?; Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/caller.rs b/bus-mapping/src/evm/opcodes/caller.rs index a0e6c44849..7b72cd9863 100644 --- a/bus-mapping/src/evm/opcodes/caller.rs +++ b/bus-mapping/src/evm/opcodes/caller.rs @@ -4,7 +4,7 @@ use crate::{ operation::CallContextField, Error, }; -use eth_types::GethExecStep; +use eth_types::{GethExecStep, ToWord}; /// Placeholder structure used to implement [`Opcode`] trait over it /// corresponding to the [`OpcodeId::CALLER`](crate::evm::OpcodeId::CALLER) `OpcodeId`. @@ -19,21 +19,19 @@ impl Opcode for Caller { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; // Get caller_address result from next step - let value = geth_steps[1].stack.last()?; + let caller_address = state.call()?.caller_address.to_word(); // CallContext read of the caller_address state.call_context_read( &mut exec_step, state.call()?.call_id, CallContextField::CallerAddress, - value, + caller_address, )?; // Stack write of the caller_address - state.stack_write( - &mut exec_step, - geth_step.stack.last_filled().map(|a| a - 1), - value, - )?; + #[cfg(feature = "enable-stack")] + assert_eq!(caller_address, geth_steps[1].stack.last()?); + state.stack_push(&mut exec_step, caller_address)?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/callop.rs b/bus-mapping/src/evm/opcodes/callop.rs index 3a7c830a0d..98ed62ad69 100644 --- a/bus-mapping/src/evm/opcodes/callop.rs +++ b/bus-mapping/src/evm/opcodes/callop.rs @@ -38,10 +38,14 @@ impl Opcode for CallOpcode { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; - let args_offset = geth_step.stack.nth_last(N_ARGS - 4)?.low_u64() as usize; - let args_length = geth_step.stack.nth_last(N_ARGS - 3)?.as_usize(); - let ret_offset = geth_step.stack.nth_last(N_ARGS - 2)?.low_u64() as usize; - let ret_length = geth_step.stack.nth_last(N_ARGS - 1)?.as_usize(); + let [args_offset, args_length, ret_offset, ret_length] = { + let stack = &state.call_ctx()?.stack; + let args_offset = stack.nth_last(N_ARGS - 4)?.low_u64() as usize; + let args_length = stack.nth_last(N_ARGS - 3)?.as_usize(); + let ret_offset = stack.nth_last(N_ARGS - 2)?.low_u64() as usize; + let ret_length = stack.nth_last(N_ARGS - 1)?.as_usize(); + [args_offset, args_length, ret_offset, ret_length] + }; // we need to keep the memory until parse_call complete state.call_expand_memory(args_offset, args_length, ret_offset, ret_length)?; @@ -61,7 +65,7 @@ impl Opcode for CallOpcode { debug_assert!(found); let caller_balance = sender_account.balance; let call_value = match callee_kind { - CallKind::Call | CallKind::CallCode => geth_step.stack.nth_last(2)?, + CallKind::Call | CallKind::CallCode => state.call_ctx()?.stack.nth_last(2)?, CallKind::DelegateCall => caller_call.value, CallKind::StaticCall => Word::zero(), CallKind::Create | CallKind::Create2 => { @@ -125,19 +129,12 @@ impl Opcode for CallOpcode { state.call_context_read(&mut exec_step, caller_call.call_id, field, value)?; } - for i in 0..N_ARGS { - state.stack_read( - &mut exec_step, - geth_step.stack.nth_last_filled(i), - geth_step.stack.nth_last(i)?, - )?; + let stack_inputs: Vec = state.stack_pops(&mut exec_step, N_ARGS)?; + #[cfg(feature = "enable-stack")] + for (i, input) in stack_inputs.iter().enumerate() { + assert_eq!(*input, geth_step.stack.nth_last(i)?); } - - state.stack_write( - &mut exec_step, - geth_step.stack.nth_last_filled(N_ARGS - 1), - (callee_call.is_success as u64).into(), - )?; + state.stack_push(&mut exec_step, (callee_call.is_success as u64).into())?; let callee_code_hash = callee_call.code_hash; let callee_acc = state.sdb.get_account(&callee_address).1; @@ -181,6 +178,9 @@ impl Opcode for CallOpcode { state.call_context_write(&mut exec_step, callee_call.call_id, field, value)?; } + let is_precheck_ok = + geth_step.depth < 1025 && (!is_call_or_callcode || caller_balance >= callee_call.value); + // read balance of caller to compare to value for insufficient_balance checking // in circuit, also use for callcode successful case check balance is // indeed larger than transfer value. for call opcode, it does in @@ -239,7 +239,7 @@ impl Opcode for CallOpcode { } else { 0 } + memory_expansion_gas_cost; - let gas_specified = geth_step.stack.last()?; + let gas_specified = stack_inputs[0]; debug_assert!( geth_step.gas.0 >= gas_cost, "gas {:?} gas_cost {:?} memory_expansion_gas_cost {:?}", @@ -264,13 +264,13 @@ impl Opcode for CallOpcode { // panic with full info let info1 = format!("callee_gas_left {callee_gas_left} gas_specified {gas_specified} gas_cost {gas_cost} is_warm {is_warm} has_value {has_value} current_memory_word_size {curr_memory_word_size} next_memory_word_size {next_memory_word_size}, memory_expansion_gas_cost {memory_expansion_gas_cost}"); let info2 = format!("args gas:{:?} addr:{:?} value:{:?} cd_pos:{:?} cd_len:{:?} rd_pos:{:?} rd_len:{:?}", - geth_step.stack.last(), - geth_step.stack.nth_last(1), - geth_step.stack.nth_last(2), - geth_step.stack.nth_last(3), - geth_step.stack.nth_last(4), - geth_step.stack.nth_last(5), - geth_step.stack.nth_last(6) + stack_inputs[0], + stack_inputs[1], + stack_inputs[2], + stack_inputs[3], + stack_inputs[4], + stack_inputs[5], + stack_inputs[6] ); let full_ctx = format!( "step0 {:?} step1 {:?} call {:?}, {} {}", @@ -346,6 +346,11 @@ impl Opcode for CallOpcode { state.call_context_write(&mut exec_step, callee_call.call_id, field, value)?; } + #[cfg(feature = "enable-stack")] + assert_eq!( + state.caller_ctx()?.stack.stack_pointer().0, + geth_step.stack.stack_pointer().0 + N_ARGS - 1 + ); // return while restoring some of caller's context. for (field, value) in [ ( @@ -354,7 +359,7 @@ impl Opcode for CallOpcode { ), ( CallContextField::StackPointer, - (geth_step.stack.stack_pointer().0 + N_ARGS - 1).into(), + state.caller_ctx()?.stack.stack_pointer().0.into(), ), ( CallContextField::GasLeft, @@ -500,7 +505,12 @@ impl Opcode for CallOpcode { oog_step.gas_cost = GasCost(precompile_call_gas_cost); // Make the Precompile execution step to handle return logic and restore to // caller context (similar as STOP and RETURN). - state.handle_return(&mut [&mut exec_step, &mut oog_step], geth_steps, true)?; + state.handle_return( + (None, None), + &mut [&mut exec_step, &mut oog_step], + geth_steps, + true, + )?; Ok(vec![exec_step, oog_step]) } else { @@ -520,6 +530,7 @@ impl Opcode for CallOpcode { // Make the Precompile execution step to handle return logic and restore to // caller context (similar as STOP and RETURN). state.handle_return( + (Some(ret_offset.into()), Some(ret_length.into())), &mut [&mut exec_step, &mut precompile_step], geth_steps, true, @@ -545,12 +556,17 @@ impl Opcode for CallOpcode { state.call_context_write(&mut exec_step, caller_call.call_id, field, value)?; } state.caller_ctx_mut()?.return_data.clear(); - state.handle_return(&mut [&mut exec_step], geth_steps, false)?; + state.handle_return((None, None), &mut [&mut exec_step], geth_steps, false)?; Ok(vec![exec_step]) } // 3. Call to account with non-empty code. (false, _, false) => { + #[cfg(feature = "enable-stack")] + assert_eq!( + state.caller_ctx()?.stack.stack_pointer().0, + geth_step.stack.stack_pointer().0 + N_ARGS - 1 + ); for (field, value) in [ ( CallContextField::ProgramCounter, @@ -558,7 +574,7 @@ impl Opcode for CallOpcode { ), ( CallContextField::StackPointer, - (geth_step.stack.stack_pointer().0 + N_ARGS - 1).into(), + state.caller_ctx()?.stack.stack_pointer().0.into(), ), ( CallContextField::GasLeft, @@ -640,7 +656,7 @@ impl Opcode for CallOpcode { state.call_context_write(&mut exec_step, caller_call.call_id, field, value)?; } state.caller_ctx_mut()?.return_data.clear(); - state.handle_return(&mut [&mut exec_step], geth_steps, false)?; + state.handle_return((None, None), &mut [&mut exec_step], geth_steps, false)?; Ok(vec![exec_step]) } // } @@ -649,6 +665,7 @@ impl Opcode for CallOpcode { #[cfg(any(test, feature = "test"))] pub mod tests { + use eth_types::{evm_types::OpcodeId, Bytecode, Word}; /// Precompile call args @@ -1040,18 +1057,21 @@ pub mod tests { .handle_block(&block.eth_block, &block.geth_traces) .unwrap(); - let step = block.geth_traces[0] - .struct_logs - .last() - .expect("at least one step"); - log::debug!("{:?}", step.stack); - for (offset, (_, stack_value)) in test_call.stack_value.iter().enumerate() { - assert_eq!( - *stack_value, - step.stack.nth_last(offset).expect("stack value not found"), - "stack output mismatch {}", - test_call.name - ); + #[cfg(feature = "enable-stack")] + { + let step = block.geth_traces[0] + .struct_logs + .last() + .expect("at least one step"); + log::debug!("{:?}", step.stack); + for (offset, (_, stack_value)) in test_call.stack_value.iter().enumerate() { + assert_eq!( + *stack_value, + step.stack.nth_last(offset).expect("stack value not found"), + "stack output mismatch {}", + test_call.name + ); + } } } } diff --git a/bus-mapping/src/evm/opcodes/callvalue.rs b/bus-mapping/src/evm/opcodes/callvalue.rs index 1f66d520f5..df37f4183b 100644 --- a/bus-mapping/src/evm/opcodes/callvalue.rs +++ b/bus-mapping/src/evm/opcodes/callvalue.rs @@ -19,7 +19,7 @@ impl Opcode for Callvalue { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; // Get call_value result from next step - let value = geth_steps[1].stack.last()?; + let value = state.call()?.value; // CallContext read of the call_value state.call_context_read( &mut exec_step, @@ -29,11 +29,9 @@ impl Opcode for Callvalue { )?; // Stack write of the call_value - state.stack_write( - &mut exec_step, - geth_step.stack.last_filled().map(|a| a - 1), - value, - )?; + #[cfg(feature = "enable-stack")] + assert_eq!(value, geth_steps[1].stack.last()?); + state.stack_push(&mut exec_step, value)?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/codecopy.rs b/bus-mapping/src/evm/opcodes/codecopy.rs index ac03acf886..1c81416bc9 100644 --- a/bus-mapping/src/evm/opcodes/codecopy.rs +++ b/bus-mapping/src/evm/opcodes/codecopy.rs @@ -4,7 +4,7 @@ use crate::{ }, Error, }; -use eth_types::{Bytecode, GethExecStep}; +use eth_types::{Bytecode, GethExecStep, Word}; use super::Opcode; @@ -17,53 +17,46 @@ impl Opcode for Codecopy { geth_steps: &[GethExecStep], ) -> Result, Error> { let geth_step = &geth_steps[0]; - let mut exec_steps = vec![gen_codecopy_step(state, geth_step)?]; + let mut exec_step = state.new_step(geth_step)?; - let copy_event = gen_copy_event(state, geth_step, &mut exec_steps[0])?; - state.push_copy(&mut exec_steps[0], copy_event); - Ok(exec_steps) - } -} + let dest_offset = state.stack_pop(&mut exec_step)?; + let code_offset = state.stack_pop(&mut exec_step)?; + let length = state.stack_pop(&mut exec_step)?; -fn gen_codecopy_step( - state: &mut CircuitInputStateRef, - geth_step: &GethExecStep, -) -> Result { - let mut exec_step = state.new_step(geth_step)?; - - let dest_offset = geth_step.stack.last()?; - let code_offset = geth_step.stack.nth_last(1)?; - let length = geth_step.stack.nth_last(2)?; - - // stack reads - state.stack_read(&mut exec_step, geth_step.stack.last_filled(), dest_offset)?; - state.stack_read( - &mut exec_step, - geth_step.stack.nth_last_filled(1), - code_offset, - )?; - state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(2), length)?; - - Ok(exec_step) + #[cfg(feature = "enable-stack")] + { + assert_eq!(dest_offset, geth_step.stack.nth_last(0)?); + assert_eq!(code_offset, geth_step.stack.nth_last(1)?); + assert_eq!(length, geth_step.stack.nth_last(2)?); + } + + let copy_event = gen_copy_event( + state, + dest_offset, + code_offset, + length.as_u64(), + &mut exec_step, + )?; + state.push_copy(&mut exec_step, copy_event); + Ok(vec![exec_step]) + } } fn gen_copy_event( state: &mut CircuitInputStateRef, - geth_step: &GethExecStep, + dest_offset: Word, + code_offset: Word, + length: u64, exec_step: &mut ExecStep, ) -> Result { let rw_counter_start = state.block_ctx.rwc; - let dst_offset = geth_step.stack.last()?; - let code_offset = geth_step.stack.nth_last(1)?; - let length = geth_step.stack.nth_last(2)?.as_u64(); - let code_hash = state.call()?.code_hash; let bytecode: Bytecode = state.code(code_hash)?.into(); let code_size = bytecode.code.len() as u64; // Get low Uint64 of offset. - let dst_addr = dst_offset.low_u64(); + let dst_addr = dest_offset.low_u64(); let src_addr_end = code_size; // Reset offset to Uint64 maximum value if overflow, and set source start to the diff --git a/bus-mapping/src/evm/opcodes/codesize.rs b/bus-mapping/src/evm/opcodes/codesize.rs index d1163b3237..eaf8a9be36 100644 --- a/bus-mapping/src/evm/opcodes/codesize.rs +++ b/bus-mapping/src/evm/opcodes/codesize.rs @@ -22,13 +22,9 @@ impl Opcode for Codesize { let code = state.code(code_hash)?; let codesize = code.len(); - debug_assert_eq!(codesize, geth_steps[1].stack.last()?.as_usize()); - - state.stack_write( - &mut exec_step, - geth_step.stack.last_filled().map(|a| a - 1), - codesize.into(), - )?; + #[cfg(feature = "enable-stack")] + assert_eq!(codesize, geth_steps[1].stack.last()?.as_usize()); + state.stack_push(&mut exec_step, codesize.into())?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/create.rs b/bus-mapping/src/evm/opcodes/create.rs index e8f1b56bd1..c0a12dd453 100644 --- a/bus-mapping/src/evm/opcodes/create.rs +++ b/bus-mapping/src/evm/opcodes/create.rs @@ -36,7 +36,7 @@ impl Opcode for Create { }; let callee_account = &state.sdb.get_account(&address).1.clone(); let callee_exists = !callee_account.is_empty(); - let callee_value = geth_step.stack.last()?; + let callee_value = state.call_ctx()?.stack.last()?; if !callee_exists && callee_value.is_zero() { state.sdb.get_account_mut(&address).1.storage.clear(); } @@ -70,7 +70,6 @@ impl Opcode for Create { tx_id.to_word(), )?; - let depth = caller.depth; state.call_context_read( &mut exec_step, caller.call_id, @@ -83,8 +82,12 @@ impl Opcode for Create { // stack operation // Get low Uint64 of offset to generate copy steps. Since offset could // be Uint64 overflow if length is zero. - let offset = geth_step.stack.nth_last(1)?.low_u64() as usize; - let length = geth_step.stack.nth_last(2)?.as_usize(); + let [offset, length] = { + let stack = &state.call_ctx()?.stack; + let offset = stack.nth_last(1)?.low_u64() as usize; + let length = stack.nth_last(2)?.as_usize(); + [offset, length] + }; if length != 0 { state @@ -102,23 +105,14 @@ impl Opcode for Create { )?; let n_pop = if IS_CREATE2 { 4 } else { 3 }; - for i in 0..n_pop { - state.stack_read( - &mut exec_step, - geth_step.stack.nth_last_filled(i), - geth_step.stack.nth_last(i)?, - )?; + let stack_inputs = state.stack_pops(&mut exec_step, n_pop)?; + #[cfg(feature = "enable-stack")] + for (i, value) in stack_inputs.iter().enumerate() { + assert_eq!(*value, geth_step.stack.nth_last(i)?); } - let address = if IS_CREATE2 { - state.create2_address(&geth_steps[0])? - } else { - state.create_address()? - }; - - state.stack_write( + state.stack_push( &mut exec_step, - geth_step.stack.nth_last_filled(n_pop - 1), if callee.is_success { address.to_word() } else { @@ -219,7 +213,7 @@ impl Opcode for Create { ), ( CallContextField::StackPointer, - geth_step.stack.nth_last_filled(n_pop - 1).0.into(), + state.call_ctx()?.stack.stack_pointer().0.into(), ), (CallContextField::GasLeft, caller_gas_left.into()), (CallContextField::MemorySize, next_memory_word_size.into()), @@ -244,7 +238,7 @@ impl Opcode for Create { if is_precheck_ok { // handle keccak_table_lookup let keccak_input = if IS_CREATE2 { - let salt = geth_step.stack.nth_last(3)?; + let salt = stack_inputs[3]; assert_eq!( address, get_create2_address( @@ -331,7 +325,7 @@ impl Opcode for Create { ] { state.call_context_write(&mut exec_step, caller.call_id, field, value)?; } - state.handle_return(&mut [&mut exec_step], geth_steps, false)?; + state.handle_return((None, None), &mut [&mut exec_step], geth_steps, false)?; } } // failed case: is_precheck_ok is false or is_address_collision is true @@ -343,7 +337,7 @@ impl Opcode for Create { ] { state.call_context_write(&mut exec_step, caller.call_id, field, value)?; } - state.handle_return(&mut [&mut exec_step], geth_steps, false)?; + state.handle_return((None, None), &mut [&mut exec_step], geth_steps, false)?; } Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/dup.rs b/bus-mapping/src/evm/opcodes/dup.rs index 87f4f84d58..f6cea90bc1 100644 --- a/bus-mapping/src/evm/opcodes/dup.rs +++ b/bus-mapping/src/evm/opcodes/dup.rs @@ -18,14 +18,15 @@ impl Opcode for Dup { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; - let stack_value_read = geth_step.stack.nth_last(N - 1)?; - let stack_position = geth_step.stack.nth_last_filled(N - 1); + let stack_value_read = state.call_ctx()?.stack.nth_last(N - 1)?; + let stack_position = state.call_ctx()?.stack.nth_last_filled(N - 1); + #[cfg(feature = "enable-stack")] + { + assert_eq!(stack_value_read, geth_step.stack.nth_last(N - 1)?); + assert_eq!(stack_position, geth_step.stack.nth_last_filled(N - 1)); + } state.stack_read(&mut exec_step, stack_position, stack_value_read)?; - state.stack_write( - &mut exec_step, - geth_step.stack.last_filled().map(|a| a - 1), - stack_value_read, - )?; + state.stack_push(&mut exec_step, stack_value_read)?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/environment.rs b/bus-mapping/src/evm/opcodes/environment.rs index 4980ac14f8..05c739b8ce 100644 --- a/bus-mapping/src/evm/opcodes/environment.rs +++ b/bus-mapping/src/evm/opcodes/environment.rs @@ -67,8 +67,9 @@ where let block_head = state.block.headers.get(&state.tx.block_num).unwrap(); let output = Self::handle(block_head); - state.stack_write(&mut exec_step, geth_steps[1].stack.last_filled(), output)?; + #[cfg(feature = "enable-stack")] assert_eq!(output, geth_steps[1].stack.last()?); + state.stack_push(&mut exec_step, output)?; Ok(vec![exec_step]) } @@ -85,8 +86,9 @@ impl Opcode for Pc { let mut exec_step = state.new_step(&geth_steps[0])?; let output: U256 = geth_steps[0].pc.0.into(); - state.stack_write(&mut exec_step, geth_steps[1].stack.last_filled(), output)?; + #[cfg(feature = "enable-stack")] assert_eq!(output, geth_steps[1].stack.last()?); + state.stack_push(&mut exec_step, output)?; Ok(vec![exec_step]) } @@ -103,8 +105,9 @@ impl Opcode for Msize { let mut exec_step = state.new_step(&geth_steps[0])?; let output: U256 = state.call_ctx()?.memory.len().into(); - state.stack_write(&mut exec_step, geth_steps[1].stack.last_filled(), output)?; + #[cfg(feature = "enable-stack")] assert_eq!(output, geth_steps[1].stack.last()?); + state.stack_push(&mut exec_step, output)?; Ok(vec![exec_step]) } @@ -121,8 +124,9 @@ impl Opcode for Gas { let mut exec_step = state.new_step(&geth_steps[0])?; let output: U256 = geth_steps[1].gas.0.into(); - state.stack_write(&mut exec_step, geth_steps[1].stack.last_filled(), output)?; + #[cfg(feature = "enable-stack")] assert_eq!(output, geth_steps[1].stack.last()?); + state.stack_push(&mut exec_step, output)?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/error_codestore.rs b/bus-mapping/src/evm/opcodes/error_codestore.rs index 55e9267637..9cca5f2c63 100644 --- a/bus-mapping/src/evm/opcodes/error_codestore.rs +++ b/bus-mapping/src/evm/opcodes/error_codestore.rs @@ -25,10 +25,13 @@ impl Opcode for ErrorCodeStore { || exec_step.error == Some(ExecError::MaxCodeSizeExceeded) ); - let offset = geth_step.stack.last()?; - let length = geth_step.stack.nth_last(1)?; - state.stack_read(&mut exec_step, geth_step.stack.last_filled(), offset)?; - state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(1), length)?; + let offset = state.stack_pop(&mut exec_step)?; + let length = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + { + assert_eq!(offset, geth_step.stack.nth_last(0)?); + assert_eq!(length, geth_step.stack.nth_last(1)?); + } // in internal call context let call = state.call()?; @@ -36,7 +39,12 @@ impl Opcode for ErrorCodeStore { // create context check assert!(call.is_create()); - state.handle_return(&mut [&mut exec_step], geth_steps, true)?; + state.handle_return( + (Some(offset), Some(length)), + &mut [&mut exec_step], + geth_steps, + true, + )?; Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/error_contract_address_collision.rs b/bus-mapping/src/evm/opcodes/error_contract_address_collision.rs index 11446d8ae4..d4e46ba095 100644 --- a/bus-mapping/src/evm/opcodes/error_contract_address_collision.rs +++ b/bus-mapping/src/evm/opcodes/error_contract_address_collision.rs @@ -18,8 +18,12 @@ impl Opcode for ContractAddressCollision { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; - let offset = geth_step.stack.nth_last(1)?.as_usize(); - let length = geth_step.stack.nth_last(2)?.as_usize(); + let [offset, length] = { + let stack = &state.call_ctx()?.stack; + let offset = stack.nth_last(1)?.as_usize(); + let length = stack.nth_last(2)?.as_usize(); + [offset, length] + }; if length != 0 { state @@ -29,12 +33,10 @@ impl Opcode for ContractAddressCollision { } let n_pop = if IS_CREATE2 { 4 } else { 3 }; - for i in 0..n_pop { - state.stack_read( - &mut exec_step, - geth_step.stack.nth_last_filled(i), - geth_step.stack.nth_last(i)?, - )?; + let _stack_inputs = state.stack_pops(&mut exec_step, n_pop)?; + #[cfg(feature = "enable-stack")] + for (i, value) in _stack_inputs.iter().enumerate() { + assert_eq!(*value, geth_step.stack.nth_last(i)?); } let _address = if IS_CREATE2 { @@ -44,11 +46,7 @@ impl Opcode for ContractAddressCollision { }; // TODO: assert address is collision - state.stack_write( - &mut exec_step, - geth_step.stack.nth_last_filled(n_pop - 1), - Word::zero(), - )?; + state.stack_push(&mut exec_step, Word::zero())?; let caller = state.call()?.clone(); diff --git a/bus-mapping/src/evm/opcodes/error_invalid_creation_code.rs b/bus-mapping/src/evm/opcodes/error_invalid_creation_code.rs index 467df5307a..00905ab32a 100644 --- a/bus-mapping/src/evm/opcodes/error_invalid_creation_code.rs +++ b/bus-mapping/src/evm/opcodes/error_invalid_creation_code.rs @@ -19,10 +19,13 @@ impl Opcode for ErrorCreationCode { exec_step.error = Some(ExecError::InvalidCreationCode); - let offset = geth_step.stack.last()?; - let length = geth_step.stack.nth_last(1)?; - state.stack_read(&mut exec_step, geth_step.stack.last_filled(), offset)?; - state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(1), length)?; + let offset = state.stack_pop(&mut exec_step)?; + let length = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + { + assert_eq!(offset, geth_step.stack.nth_last(0)?); + assert_eq!(length, geth_step.stack.nth_last(1)?); + } // in create context let call = state.call()?; @@ -36,15 +39,19 @@ impl Opcode for ErrorCreationCode { let byte = state.call_ctx()?.memory.0[offset.as_usize()]; assert!(byte == 0xef); - let offset = offset.as_u64(); - let shift = offset % 32; - let slot = offset - shift; + let shift = offset.as_u64() % 32; + let slot = offset.as_u64() - shift; //state.memory_read(&mut exec_step, offset.try_into()?, byte)?; state.memory_read_word(&mut exec_step, slot.into())?; // refer to return_revert Case C - state.handle_return(&mut [&mut exec_step], geth_steps, true)?; + state.handle_return( + (Some(offset), Some(length)), + &mut [&mut exec_step], + geth_steps, + true, + )?; Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/error_invalid_jump.rs b/bus-mapping/src/evm/opcodes/error_invalid_jump.rs index 87abde9497..dfdea91f15 100644 --- a/bus-mapping/src/evm/opcodes/error_invalid_jump.rs +++ b/bus-mapping/src/evm/opcodes/error_invalid_jump.rs @@ -3,7 +3,7 @@ use crate::{ evm::{Opcode, OpcodeId}, Error, }; -use eth_types::GethExecStep; +use eth_types::{GethExecStep, Word}; #[derive(Debug, Copy, Clone)] pub(crate) struct InvalidJump; @@ -23,21 +23,23 @@ impl Opcode for InvalidJump { exec_step.error = state.get_step_err(geth_step, next_step).unwrap(); // assert op code can only be JUMP or JUMPI assert!(geth_step.op == OpcodeId::JUMP || geth_step.op == OpcodeId::JUMPI); - state.stack_read( - &mut exec_step, - geth_step.stack.last_filled(), - geth_step.stack.last()?, - )?; - if geth_step.op == OpcodeId::JUMPI { - state.stack_read( - &mut exec_step, - geth_step.stack.nth_last_filled(1), - geth_step.stack.nth_last(1)?, - )?; + let _counter = state.stack_pop(&mut exec_step)?; + let is_jumpi = geth_step.op == OpcodeId::JUMPI; + let _condition: Word = if is_jumpi { + state.stack_pop(&mut exec_step)? + } else { + Word::zero() + }; + #[cfg(feature = "enable-stack")] + { + assert_eq!(_counter, geth_step.stack.last()?); + if is_jumpi { + assert_eq!(_condition, geth_step.stack.nth_last(1)?); + } } // `IsSuccess` call context operation is added in handle_return - state.handle_return(&mut [&mut exec_step], geth_steps, true)?; + state.handle_return((None, None), &mut [&mut exec_step], geth_steps, true)?; Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/error_oog_account_access.rs b/bus-mapping/src/evm/opcodes/error_oog_account_access.rs index 799dd998df..9348157101 100644 --- a/bus-mapping/src/evm/opcodes/error_oog_account_access.rs +++ b/bus-mapping/src/evm/opcodes/error_oog_account_access.rs @@ -27,9 +27,10 @@ impl Opcode for ErrorOOGAccountAccess { ] .contains(&geth_step.op)); // Read account address from stack. - let address_word = geth_step.stack.last()?; + let address_word = state.stack_pop(&mut exec_step)?; let address = address_word.to_address(); - state.stack_read(&mut exec_step, geth_step.stack.last_filled(), address_word)?; + #[cfg(feature = "enable-stack")] + assert_eq!(address_word, geth_step.stack.last()?); // Read transaction ID from call context. state.call_context_read( @@ -54,7 +55,7 @@ impl Opcode for ErrorOOGAccountAccess { )?; // common error handling - state.handle_return(&mut [&mut exec_step], geth_steps, true)?; + state.handle_return((None, None), &mut [&mut exec_step], geth_steps, true)?; Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/error_oog_call.rs b/bus-mapping/src/evm/opcodes/error_oog_call.rs index 8210dd6329..62561c76d5 100644 --- a/bus-mapping/src/evm/opcodes/error_oog_call.rs +++ b/bus-mapping/src/evm/opcodes/error_oog_call.rs @@ -34,7 +34,6 @@ impl Opcode for OOGCall { exec_step.error = state.get_step_err(geth_step, next_step).unwrap(); let tx_id = state.tx_ctx.id(); - let call_address = geth_step.stack.nth_last(1)?.to_address(); let current_call = state.call()?.clone(); for (field, value) in [ @@ -47,20 +46,16 @@ impl Opcode for OOGCall { state.call_context_read(&mut exec_step, current_call.call_id, field, value)?; } - for i in 0..stack_input_num { - state.stack_read( - &mut exec_step, - geth_step.stack.nth_last_filled(i), - geth_step.stack.nth_last(i)?, - )?; + let stack_inputs = state.stack_pops(&mut exec_step, stack_input_num)?; + let call_address = stack_inputs[1].to_address(); + #[cfg(feature = "enable-stack")] + { + for (i, v) in stack_inputs.iter().enumerate() { + assert_eq!(*v, geth_step.stack.nth_last(i)?); + } } - - state.stack_write( - &mut exec_step, - geth_step.stack.nth_last_filled(stack_input_num - 1), - // Must fail. - (0_u64).into(), - )?; + // Must fail. + state.stack_push(&mut exec_step, (0_u64).into())?; let (_, callee_account) = state.sdb.get_account(&call_address); let callee_exists = !callee_account.is_empty(); @@ -90,7 +85,7 @@ impl Opcode for OOGCall { }, )?; - state.handle_return(&mut [&mut exec_step], geth_steps, true)?; + state.handle_return((None, None), &mut [&mut exec_step], geth_steps, true)?; Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/error_oog_log.rs b/bus-mapping/src/evm/opcodes/error_oog_log.rs index 3c5c1fb57f..c609e7bb19 100644 --- a/bus-mapping/src/evm/opcodes/error_oog_log.rs +++ b/bus-mapping/src/evm/opcodes/error_oog_log.rs @@ -31,11 +31,14 @@ impl Opcode for ErrorOOGLog { OpcodeId::LOG4 ] .contains(&geth_step.op)); - let mstart = geth_step.stack.last()?; - let msize = geth_step.stack.nth_last(1)?; + let _mstart = state.stack_pop(&mut exec_step)?; + let _msize = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + { + assert_eq!(_mstart, geth_step.stack.nth_last(0)?); + assert_eq!(_msize, geth_step.stack.nth_last(1)?); + } - state.stack_read(&mut exec_step, geth_step.stack.last_filled(), mstart)?; - state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(1), msize)?; // read static call property state.call_context_read( &mut exec_step, @@ -44,7 +47,7 @@ impl Opcode for ErrorOOGLog { Word::from(state.call()?.is_static as u8), )?; - state.handle_return(&mut [&mut exec_step], geth_steps, true)?; + state.handle_return((None, None), &mut [&mut exec_step], geth_steps, true)?; Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/error_oog_memory_copy.rs b/bus-mapping/src/evm/opcodes/error_oog_memory_copy.rs index 50327115ba..f6fc2a5acb 100644 --- a/bus-mapping/src/evm/opcodes/error_oog_memory_copy.rs +++ b/bus-mapping/src/evm/opcodes/error_oog_memory_copy.rs @@ -42,7 +42,7 @@ impl Opcode for OOGMemoryCopy { state.tx_ctx.id().into(), )?; - let external_address = geth_step.stack.last()?.to_address(); + let external_address = state.call_ctx()?.stack.last()?.to_address(); let is_warm = state.sdb.check_account_in_access_list(&external_address); state.push_op( &mut exec_step, @@ -59,15 +59,13 @@ impl Opcode for OOGMemoryCopy { // Each of CALLDATACOPY, CODECOPY and RETURNDATACOPY has 3 stack read values. // But EXTCODECOPY has 4. It has an extra stack pop for external address. let stack_read_num = if is_extcodecopy { 4 } else { 3 }; - for i in 0..stack_read_num { - state.stack_read( - &mut exec_step, - geth_step.stack.nth_last_filled(i), - geth_step.stack.nth_last(i)?, - )?; + let _stack_inputs = state.stack_pops(&mut exec_step, stack_read_num)?; + #[cfg(feature = "enable-stack")] + for (i, v) in _stack_inputs.iter().enumerate() { + assert_eq!(*v, geth_step.stack.nth_last(i)?); } - state.handle_return(&mut [&mut exec_step], geth_steps, true)?; + state.handle_return((None, None), &mut [&mut exec_step], geth_steps, true)?; Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/error_oog_sload_sstore.rs b/bus-mapping/src/evm/opcodes/error_oog_sload_sstore.rs index 285ba59607..1b994434f0 100644 --- a/bus-mapping/src/evm/opcodes/error_oog_sload_sstore.rs +++ b/bus-mapping/src/evm/opcodes/error_oog_sload_sstore.rs @@ -49,8 +49,9 @@ impl Opcode for OOGSloadSstore { callee_address.to_word(), )?; - let key = geth_step.stack.last()?; - state.stack_read(&mut exec_step, geth_step.stack.last_filled(), key)?; + let key = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + assert_eq!(key, geth_step.stack.last()?); let is_warm = state .sdb @@ -69,8 +70,9 @@ impl Opcode for OOGSloadSstore { // Special operations are only used for SSTORE. if geth_step.op == OpcodeId::SSTORE { - let value = geth_step.stack.nth_last(1)?; - state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(1), value)?; + let _value = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + assert_eq!(_value, geth_step.stack.nth_last(1)?); let (_, value_prev) = state.sdb.get_storage(&callee_address, &key); let (_, original_value) = state.sdb.get_committed_storage(&callee_address, &key); @@ -89,7 +91,7 @@ impl Opcode for OOGSloadSstore { )?; } - state.handle_return(&mut [&mut exec_step], geth_steps, true)?; + state.handle_return((None, None), &mut [&mut exec_step], geth_steps, true)?; Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/error_precompile_failed.rs b/bus-mapping/src/evm/opcodes/error_precompile_failed.rs index 16c02dfa0b..29281ba3fb 100644 --- a/bus-mapping/src/evm/opcodes/error_precompile_failed.rs +++ b/bus-mapping/src/evm/opcodes/error_precompile_failed.rs @@ -25,10 +25,14 @@ impl Opcode for PrecompileFailed { let mut exec_step = state.new_step(geth_step)?; exec_step.error = Some(ExecError::PrecompileFailed); - let args_offset = geth_step.stack.nth_last(stack_input_num - 4)?.as_usize(); - let args_length = geth_step.stack.nth_last(stack_input_num - 3)?.as_usize(); - let ret_offset = geth_step.stack.nth_last(stack_input_num - 2)?.as_usize(); - let ret_length = geth_step.stack.nth_last(stack_input_num - 1)?.as_usize(); + let [args_offset, args_length, ret_offset, ret_length] = { + let stack = &state.call_ctx()?.stack; + let args_offset = stack.nth_last(stack_input_num - 4)?.low_u64() as usize; + let args_length = stack.nth_last(stack_input_num - 3)?.as_usize(); + let ret_offset = stack.nth_last(stack_input_num - 2)?.low_u64() as usize; + let ret_length = stack.nth_last(stack_input_num - 1)?.as_usize(); + [args_offset, args_length, ret_offset, ret_length] + }; // we need to keep the memory until parse_call complete state.call_expand_memory(args_offset, args_length, ret_offset, ret_length)?; @@ -36,22 +40,16 @@ impl Opcode for PrecompileFailed { let call = state.parse_call(geth_step)?; state.push_call(call.clone()); state.caller_ctx_mut()?.return_data.clear(); - state.handle_return(&mut [&mut exec_step], geth_steps, false)?; + state.handle_return((None, None), &mut [&mut exec_step], geth_steps, false)?; - for i in 0..stack_input_num { - state.stack_read( - &mut exec_step, - geth_step.stack.nth_last_filled(i), - geth_step.stack.nth_last(i)?, - )?; + let _stack_inputs = state.stack_pops(&mut exec_step, stack_input_num)?; + #[cfg(feature = "enable-stack")] + for (i, v) in _stack_inputs.into_iter().enumerate() { + assert_eq!(v, geth_step.stack.nth_last(i)?); } - state.stack_write( - &mut exec_step, - geth_step.stack.nth_last_filled(stack_input_num - 1), - // Must fail. - (0_u64).into(), - )?; + // Must fail. + state.stack_push(&mut exec_step, (0_u64).into())?; for (field, value) in [ (CallContextField::LastCalleeId, call.call_id.into()), diff --git a/bus-mapping/src/evm/opcodes/error_return_data_outofbound.rs b/bus-mapping/src/evm/opcodes/error_return_data_outofbound.rs index 121dbabd58..d0b17866d8 100644 --- a/bus-mapping/src/evm/opcodes/error_return_data_outofbound.rs +++ b/bus-mapping/src/evm/opcodes/error_return_data_outofbound.rs @@ -25,17 +25,15 @@ impl Opcode for ErrorReturnDataOutOfBound { Some(ExecError::ReturnDataOutOfBounds) ); - let memory_offset = geth_step.stack.last()?; - let data_offset = geth_step.stack.nth_last(1)?; - let length = geth_step.stack.nth_last(2)?; - - state.stack_read(&mut exec_step, geth_step.stack.last_filled(), memory_offset)?; - state.stack_read( - &mut exec_step, - geth_step.stack.nth_last_filled(1), - data_offset, - )?; - state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(2), length)?; + let _memory_offset = state.stack_pop(&mut exec_step)?; + let data_offset = state.stack_pop(&mut exec_step)?; + let length = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + { + assert_eq!(_memory_offset, geth_step.stack.nth_last(0)?); + assert_eq!(data_offset, geth_step.stack.nth_last(1)?); + assert_eq!(length, geth_step.stack.nth_last(2)?); + } let call_id = state.call()?.call_id; let call_ctx = state.call_ctx()?; @@ -65,7 +63,7 @@ impl Opcode for ErrorReturnDataOutOfBound { )?; // `IsSuccess` call context operation is added in handle_return - state.handle_return(&mut [&mut exec_step], geth_steps, true)?; + state.handle_return((None, None), &mut [&mut exec_step], geth_steps, true)?; Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/error_write_protection.rs b/bus-mapping/src/evm/opcodes/error_write_protection.rs index 2c9bd0b87e..963de9e080 100644 --- a/bus-mapping/src/evm/opcodes/error_write_protection.rs +++ b/bus-mapping/src/evm/opcodes/error_write_protection.rs @@ -46,12 +46,10 @@ impl Opcode for ErrorWriteProtection { if geth_step.op == OpcodeId::CALL { // get only the frist three stack elements since the third one is the value we // want to check. - for i in 0..3 { - state.stack_read( - &mut exec_step, - geth_step.stack.nth_last_filled(i), - geth_step.stack.nth_last(i)?, - )?; + for _i in 0..3 { + let _v = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + assert_eq!(_v, geth_step.stack.nth_last(_i)?); } } @@ -63,7 +61,7 @@ impl Opcode for ErrorWriteProtection { )?; // `IsSuccess` call context operation is added in handle_return - state.handle_return(&mut [&mut exec_step], geth_steps, true)?; + state.handle_return((None, None), &mut [&mut exec_step], geth_steps, true)?; Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/exp.rs b/bus-mapping/src/evm/opcodes/exp.rs index e8c00842e4..820537aa3d 100644 --- a/bus-mapping/src/evm/opcodes/exp.rs +++ b/bus-mapping/src/evm/opcodes/exp.rs @@ -41,17 +41,16 @@ impl Opcode for Exponentiation { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; - let base = geth_step.stack.last()?; - state.stack_read(&mut exec_step, geth_step.stack.last_filled(), base)?; - let exponent = geth_step.stack.nth_last(1)?; - state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(1), exponent)?; + let base = state.stack_pop(&mut exec_step)?; + let exponent = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + { + assert_eq!(base, geth_step.stack.nth_last(0)?); + assert_eq!(exponent, geth_step.stack.nth_last(1)?); + } let (exponentiation, _) = base.overflowing_pow(exponent); - state.stack_write( - &mut exec_step, - geth_steps[1].stack.last_filled(), - exponentiation, - )?; + state.stack_push(&mut exec_step, exponentiation)?; let mut steps = Vec::new(); let exponentiation_calc = exp_by_squaring(base, exponent, &mut steps); diff --git a/bus-mapping/src/evm/opcodes/extcodecopy.rs b/bus-mapping/src/evm/opcodes/extcodecopy.rs index f6444ee5b4..c6fa9a12c1 100644 --- a/bus-mapping/src/evm/opcodes/extcodecopy.rs +++ b/bus-mapping/src/evm/opcodes/extcodecopy.rs @@ -6,7 +6,7 @@ use crate::{ operation::{AccountField, CallContextField, TxAccessListAccountOp}, Error, }; -use eth_types::{Bytecode, GethExecStep, ToAddress, ToWord, H256, U256}; +use eth_types::{Address, Bytecode, GethExecStep, ToAddress, ToWord, Word, H256}; #[derive(Clone, Copy, Debug)] pub(crate) struct Extcodecopy; @@ -17,93 +17,85 @@ impl Opcode for Extcodecopy { geth_steps: &[GethExecStep], ) -> Result, Error> { let geth_step = &geth_steps[0]; - let mut exec_steps = vec![gen_extcodecopy_step(state, geth_step)?]; - - let copy_event = gen_copy_event(state, geth_step, &mut exec_steps[0])?; - state.push_copy(&mut exec_steps[0], copy_event); - Ok(exec_steps) - } -} + let mut exec_step = state.new_step(geth_step)?; + + let external_address_word = state.stack_pop(&mut exec_step)?; + let external_address = external_address_word.to_address(); + let dest_offset = state.stack_pop(&mut exec_step)?; + let offset = state.stack_pop(&mut exec_step)?; + let length = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + { + assert_eq!(external_address_word, geth_step.stack.nth_last(0)?); + assert_eq!(dest_offset, geth_step.stack.nth_last(1)?); + assert_eq!(offset, geth_step.stack.nth_last(2)?); + assert_eq!(length, geth_step.stack.nth_last(3)?); + } -fn gen_extcodecopy_step( - state: &mut CircuitInputStateRef, - geth_step: &GethExecStep, -) -> Result { - let mut exec_step = state.new_step(geth_step)?; - - let external_address_word = geth_step.stack.last()?; - let external_address = external_address_word.to_address(); - let dest_offset = geth_step.stack.nth_last(1)?; - let offset = geth_step.stack.nth_last(2)?; - let length = geth_step.stack.nth_last(3)?; - - // stack reads - state.stack_read( - &mut exec_step, - geth_step.stack.last_filled(), - external_address_word, - )?; - state.stack_read( - &mut exec_step, - geth_step.stack.nth_last_filled(1), - dest_offset, - )?; - state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(2), offset)?; - state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(3), length)?; - - for (field, value) in [ - (CallContextField::TxId, U256::from(state.tx_ctx.id())), - ( - CallContextField::RwCounterEndOfReversion, - U256::from(state.call()?.rw_counter_end_of_reversion as u64), - ), - ( - CallContextField::IsPersistent, - U256::from(state.call()?.is_persistent as u64), - ), - ] { - state.call_context_read(&mut exec_step, state.call()?.call_id, field, value)?; - } + for (field, value) in [ + (CallContextField::TxId, Word::from(state.tx_ctx.id())), + ( + CallContextField::RwCounterEndOfReversion, + Word::from(state.call()?.rw_counter_end_of_reversion as u64), + ), + ( + CallContextField::IsPersistent, + Word::from(state.call()?.is_persistent as u64), + ), + ] { + state.call_context_read(&mut exec_step, state.call()?.call_id, field, value)?; + } - let is_warm = state.sdb.check_account_in_access_list(&external_address); - state.push_op_reversible( - &mut exec_step, - TxAccessListAccountOp { - tx_id: state.tx_ctx.id(), - address: external_address, - is_warm: true, - is_warm_prev: is_warm, - }, - )?; + let is_warm = state.sdb.check_account_in_access_list(&external_address); + state.push_op_reversible( + &mut exec_step, + TxAccessListAccountOp { + tx_id: state.tx_ctx.id(), + address: external_address, + is_warm: true, + is_warm_prev: is_warm, + }, + )?; - let account = state.sdb.get_account(&external_address).1; - let exists = !account.is_empty(); - let code_hash = if exists { - account.code_hash - } else { - H256::zero() - }; + let account = state.sdb.get_account(&external_address).1; + let exists = !account.is_empty(); + let code_hash = if exists { + account.code_hash + } else { + H256::zero() + }; - state.account_read( - &mut exec_step, - external_address, - AccountField::CodeHash, - code_hash.to_word(), - )?; - Ok(exec_step) + state.account_read( + &mut exec_step, + external_address, + AccountField::CodeHash, + code_hash.to_word(), + )?; + + let copy_event = gen_copy_event( + state, + external_address, + dest_offset, + offset, + length, + &mut exec_step, + )?; + state.push_copy(&mut exec_step, copy_event); + Ok(vec![exec_step]) + } } fn gen_copy_event( state: &mut CircuitInputStateRef, - geth_step: &GethExecStep, + external_address: Address, + dest_offset: Word, + offset: Word, + length: Word, exec_step: &mut ExecStep, ) -> Result { let rw_counter_start = state.block_ctx.rwc; - let external_address = geth_step.stack.last()?.to_address(); - let dst_offset = geth_step.stack.nth_last(1)?; - let code_offset = geth_step.stack.nth_last(2)?; - let length = geth_step.stack.nth_last(3)?.as_u64(); + let length = length.as_u64(); let account = state.sdb.get_account(&external_address).1; let exists = !account.is_empty(); @@ -121,14 +113,12 @@ fn gen_copy_event( let code_size = bytecode.code.len() as u64; // Get low Uint64 of offset. - let dst_addr = dst_offset.low_u64(); + let dst_addr = dest_offset.low_u64(); let src_addr_end = code_size; // Reset offset to Uint64 maximum value if overflow, and set source start to the // minimum value of offset and code size. - let src_addr = u64::try_from(code_offset) - .unwrap_or(u64::MAX) - .min(src_addr_end); + let src_addr = u64::try_from(offset).unwrap_or(u64::MAX).min(src_addr_end); let (copy_steps, prev_bytes) = state.gen_copy_steps_for_bytecode(exec_step, &bytecode, src_addr, dst_addr, length)?; diff --git a/bus-mapping/src/evm/opcodes/extcodehash.rs b/bus-mapping/src/evm/opcodes/extcodehash.rs index 91898b9d15..d9ebeb2df0 100644 --- a/bus-mapping/src/evm/opcodes/extcodehash.rs +++ b/bus-mapping/src/evm/opcodes/extcodehash.rs @@ -17,12 +17,12 @@ impl Opcode for Extcodehash { ) -> Result, Error> { let step = &steps[0]; let mut exec_step = state.new_step(step)?; - let stack_address = step.stack.last_filled(); // Pop external address off stack - let external_address_word = step.stack.last()?; + let external_address_word = state.stack_pop(&mut exec_step)?; let external_address = external_address_word.to_address(); - state.stack_read(&mut exec_step, stack_address, external_address_word)?; + #[cfg(feature = "enable-stack")] + assert_eq!(external_address_word, step.stack.last()?); // Read transaction id, rw_counter_end_of_reversion, and is_persistent from call // context @@ -76,9 +76,10 @@ impl Opcode for Extcodehash { }, code_hash.to_word(), )?; - debug_assert_eq!(steps[1].stack.last()?, code_hash.to_word()); + #[cfg(feature = "enable-stack")] + assert_eq!(steps[1].stack.last()?, code_hash.to_word()); // Stack write of the result of EXTCODEHASH. - state.stack_write(&mut exec_step, stack_address, steps[1].stack.last()?)?; + state.stack_push(&mut exec_step, code_hash.to_word())?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/extcodesize.rs b/bus-mapping/src/evm/opcodes/extcodesize.rs index e991b5f3f9..db04a50c08 100644 --- a/bus-mapping/src/evm/opcodes/extcodesize.rs +++ b/bus-mapping/src/evm/opcodes/extcodesize.rs @@ -18,9 +18,10 @@ impl Opcode for Extcodesize { let mut exec_step = state.new_step(geth_step)?; // Read account address from stack. - let address_word = geth_step.stack.last()?; + let address_word = state.stack_pop(&mut exec_step)?; let address = address_word.to_address(); - state.stack_read(&mut exec_step, geth_step.stack.last_filled(), address_word)?; + #[cfg(feature = "enable-stack")] + assert_eq!(address_word, geth_step.stack.last()?); // Read transaction ID, rw_counter_end_of_reversion, and is_persistent from call // context. @@ -77,8 +78,9 @@ impl Opcode for Extcodesize { } // Write the EXTCODESIZE result to stack. - debug_assert_eq!(code_size, geth_steps[1].stack.last()?); - state.stack_write(&mut exec_step, geth_steps[1].stack.last_filled(), code_size)?; + #[cfg(feature = "enable-stack")] + assert_eq!(code_size, geth_steps[1].stack.last()?); + state.stack_push(&mut exec_step, code_size)?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/gasprice.rs b/bus-mapping/src/evm/opcodes/gasprice.rs index 0abeb1c446..4340c3ba05 100644 --- a/bus-mapping/src/evm/opcodes/gasprice.rs +++ b/bus-mapping/src/evm/opcodes/gasprice.rs @@ -19,7 +19,7 @@ impl Opcode for GasPrice { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; // Get gasprice result from next step - let value = geth_steps[1].stack.last()?; + let gasprice = state.tx.gas_price; let tx_id = state.tx_ctx.id(); // CallContext read of the TxId @@ -31,11 +31,9 @@ impl Opcode for GasPrice { )?; // Stack write of the gasprice value - state.stack_write( - &mut exec_step, - geth_step.stack.last_filled().map(|a| a - 1), - value, - )?; + #[cfg(feature = "enable-stack")] + assert_eq!(gasprice, geth_steps[1].stack.last()?); + state.stack_push(&mut exec_step, gasprice)?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/logs.rs b/bus-mapping/src/evm/opcodes/logs.rs index a7b074fc42..d187e6ded3 100644 --- a/bus-mapping/src/evm/opcodes/logs.rs +++ b/bus-mapping/src/evm/opcodes/logs.rs @@ -17,93 +17,42 @@ impl Opcode for Log { geth_steps: &[GethExecStep], ) -> Result, Error> { let geth_step = &geth_steps[0]; - let mut exec_step = gen_log_step(state, geth_step)?; - if state.call()?.is_persistent { - let copy_event = gen_copy_event(state, geth_step, &mut exec_step)?; - state.push_copy(&mut exec_step, copy_event); - state.tx_ctx.log_id += 1; - } - - // reconstruction - let offset = geth_step.stack.last()?; - let length = geth_step.stack.nth_last(1)?.as_u64(); - if length != 0 { - // Offset should be within range of Uint64 if length is non-zero. - let memory_length = offset - .as_u64() - .checked_add(length) - .and_then(|val| usize::try_from(val).ok()) - .unwrap(); + let mut exec_step = state.new_step(geth_step)?; - state.call_ctx_mut()?.memory.extend_at_least(memory_length); + let mstart = state.stack_pop(&mut exec_step)?; + let msize = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + { + assert_eq!(mstart, geth_step.stack.nth_last(0)?); + assert_eq!(msize, geth_step.stack.nth_last(1)?); } - Ok(vec![exec_step]) - } -} + let call_id = state.call()?.call_id; -fn gen_log_step( - state: &mut CircuitInputStateRef, - geth_step: &GethExecStep, -) -> Result { - let mut exec_step = state.new_step(geth_step)?; - - let mstart = geth_step.stack.last()?; - let msize = geth_step.stack.nth_last(1)?; - - let call_id = state.call()?.call_id; - state.stack_read(&mut exec_step, geth_step.stack.last_filled(), mstart)?; - state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(1), msize)?; - - state.call_context_read( - &mut exec_step, - call_id, - CallContextField::TxId, - state.tx_ctx.id().into(), - )?; - state.call_context_read( - &mut exec_step, - call_id, - CallContextField::IsStatic, - Word::from(state.call()?.is_static as u8), - )?; - state.call_context_read( - &mut exec_step, - call_id, - CallContextField::CalleeAddress, - state.call()?.address.to_word(), - )?; - state.call_context_read( - &mut exec_step, - call_id, - CallContextField::IsPersistent, - Word::from(state.call()?.is_persistent as u8), - )?; - - if state.call()?.is_persistent { - state.tx_log_write( + state.call_context_read( &mut exec_step, - state.tx_ctx.id(), - state.tx_ctx.log_id + 1, - TxLogField::Address, - 0, + call_id, + CallContextField::TxId, + state.tx_ctx.id().into(), + )?; + state.call_context_read( + &mut exec_step, + call_id, + CallContextField::IsStatic, + Word::from(state.call()?.is_static as u8), + )?; + state.call_context_read( + &mut exec_step, + call_id, + CallContextField::CalleeAddress, state.call()?.address.to_word(), )?; - } - - // generates topic operation dynamically - let topic_count = match exec_step.exec_state { - ExecState::Op(op_id) => op_id.postfix().expect("opcode with postfix") as usize, - _ => panic!("currently only handle successful log state"), - }; - - for i in 0..topic_count { - let topic = geth_step.stack.nth_last(2 + i)?; - state.stack_read( + state.call_context_read( &mut exec_step, - geth_step.stack.nth_last_filled(2 + i), - topic, + call_id, + CallContextField::IsPersistent, + Word::from(state.call()?.is_persistent as u8), )?; if state.call()?.is_persistent { @@ -111,31 +60,71 @@ fn gen_log_step( &mut exec_step, state.tx_ctx.id(), state.tx_ctx.log_id + 1, - TxLogField::Topic, - i, - topic, + TxLogField::Address, + 0, + state.call()?.address.to_word(), )?; } - } - Ok(exec_step) + // generates topic operation dynamically + let topic_count = match exec_step.exec_state { + ExecState::Op(op_id) => op_id.postfix().expect("opcode with postfix") as usize, + _ => panic!("currently only handle successful log state"), + }; + + for i in 0..topic_count { + let topic = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + assert_eq!(topic, geth_step.stack.nth_last(2 + i)?); + + if state.call()?.is_persistent { + state.tx_log_write( + &mut exec_step, + state.tx_ctx.id(), + state.tx_ctx.log_id + 1, + TxLogField::Topic, + i, + topic, + )?; + } + } + + if state.call()?.is_persistent { + // Get low Uint64 for memory start as below reference. Memory size must be + // within range of Uint64, otherwise returns ErrGasUintOverflow. + // https://github.com/ethereum/go-ethereum/blob/b80f05bde2c4e93ae64bb3813b6d67266b5fc0e6/core/vm/instructions.go#L850 + let copy_event = + gen_copy_event(state, mstart.low_u64(), msize.low_u64(), &mut exec_step)?; + state.push_copy(&mut exec_step, copy_event); + state.tx_ctx.log_id += 1; + } + + // reconstruction + if msize.as_u64() != 0 { + // Offset should be within range of Uint64 if length is non-zero. + let memory_length = mstart + .as_u64() + .checked_add(msize.as_u64()) + .and_then(|val| usize::try_from(val).ok()) + .unwrap(); + + state.call_ctx_mut()?.memory.extend_at_least(memory_length); + } + + Ok(vec![exec_step]) + } } fn gen_copy_event( state: &mut CircuitInputStateRef, - geth_step: &GethExecStep, + memory_start: u64, + msize: u64, exec_step: &mut ExecStep, ) -> Result { let rw_counter_start = state.block_ctx.rwc; assert!(state.call()?.is_persistent, "Error: Call is not persistent"); - // Get low Uint64 for memory start as below reference. Memory size must be - // within range of Uint64, otherwise returns ErrGasUintOverflow. - // https://github.com/ethereum/go-ethereum/blob/b80f05bde2c4e93ae64bb3813b6d67266b5fc0e6/core/vm/instructions.go#L850 - let memory_start = geth_step.stack.last()?.low_u64(); - let msize = geth_step.stack.nth_last(1)?.as_u64(); - let (src_addr, src_addr_end) = (memory_start, memory_start.checked_add(msize).unwrap()); let (read_steps, write_steps) = state.gen_copy_steps_for_log(exec_step, src_addr, msize)?; diff --git a/bus-mapping/src/evm/opcodes/mload.rs b/bus-mapping/src/evm/opcodes/mload.rs index c1cfdd258d..e71d277990 100644 --- a/bus-mapping/src/evm/opcodes/mload.rs +++ b/bus-mapping/src/evm/opcodes/mload.rs @@ -20,23 +20,24 @@ impl Opcode for Mload { ) -> Result, Error> { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; - // First stack read - // - 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.stack_read(&mut exec_step, stack_position, stack_value_read)?; + let stack_value_read = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + assert_eq!(stack_value_read, geth_step.stack.last()?); // Read the memory value from the next step of the trace. - let mem_read_value = geth_steps[1].stack.last()?; + let mem_read_value = state + .call_ctx()? + .memory + .read_word(stack_value_read.as_usize().into()); let offset = stack_value_read.as_u64(); let shift = offset % 32; let slot = offset - shift; // First stack write - state.stack_write(&mut exec_step, stack_position, mem_read_value)?; + #[cfg(feature = "enable-stack")] + assert_eq!(mem_read_value, geth_steps[1].stack.last()?); + state.stack_push(&mut exec_step, mem_read_value)?; state.memory_read_word(&mut exec_step, slot.into())?; state.memory_read_word(&mut exec_step, (slot + 32).into())?; diff --git a/bus-mapping/src/evm/opcodes/mstore.rs b/bus-mapping/src/evm/opcodes/mstore.rs index 0752ac45f6..7735f87e73 100644 --- a/bus-mapping/src/evm/opcodes/mstore.rs +++ b/bus-mapping/src/evm/opcodes/mstore.rs @@ -18,15 +18,13 @@ impl Opcode for Mstore { ) -> Result, Error> { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; - // First stack read (offset) - let offset = geth_step.stack.last()?; - let offset_pos = geth_step.stack.last_filled(); - state.stack_read(&mut exec_step, offset_pos, offset)?; - - // Second stack read (value) - let value = geth_step.stack.nth_last(1)?; - let value_pos = geth_step.stack.nth_last_filled(1); - state.stack_read(&mut exec_step, value_pos, value)?; + let offset = state.stack_pop(&mut exec_step)?; + let value = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + { + assert_eq!(offset, geth_step.stack.nth_last(0)?); + assert_eq!(value, geth_step.stack.nth_last(1)?); + } let offset_u64 = offset.as_u64() as usize; let shift = offset_u64 % 32; diff --git a/bus-mapping/src/evm/opcodes/origin.rs b/bus-mapping/src/evm/opcodes/origin.rs index 901fc594f1..7549d91847 100644 --- a/bus-mapping/src/evm/opcodes/origin.rs +++ b/bus-mapping/src/evm/opcodes/origin.rs @@ -4,7 +4,7 @@ use crate::{ operation::CallContextField, Error, }; -use eth_types::GethExecStep; +use eth_types::{GethExecStep, ToWord}; #[derive(Debug, Copy, Clone)] pub(crate) struct Origin; @@ -16,8 +16,8 @@ impl Opcode for Origin { ) -> Result, Error> { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; - // Get origin result from next step - let value = geth_steps[1].stack.last()?; + // TODO: Get origin result + let value = state.tx.from.to_word(); let tx_id = state.tx_ctx.id(); // CallContext read of the TxId @@ -29,11 +29,9 @@ impl Opcode for Origin { )?; // Stack write of the origin address value - state.stack_write( - &mut exec_step, - geth_step.stack.last_filled().map(|a| a - 1), - value, - )?; + #[cfg(feature = "enable-stack")] + assert_eq!(value, geth_steps[1].stack.last()?); + state.stack_push(&mut exec_step, value)?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/push0.rs b/bus-mapping/src/evm/opcodes/push0.rs index eeffd7ec4f..86da88675e 100644 --- a/bus-mapping/src/evm/opcodes/push0.rs +++ b/bus-mapping/src/evm/opcodes/push0.rs @@ -16,11 +16,7 @@ impl Opcode for Push0 { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; - state.stack_write( - &mut exec_step, - geth_steps[1].stack.last_filled(), - U256::zero(), - )?; + state.stack_push(&mut exec_step, U256::zero())?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/pushn.rs b/bus-mapping/src/evm/opcodes/pushn.rs index c76476bf4a..fecdbb5ee5 100644 --- a/bus-mapping/src/evm/opcodes/pushn.rs +++ b/bus-mapping/src/evm/opcodes/pushn.rs @@ -3,7 +3,7 @@ use crate::{ circuit_input_builder::{CircuitInputStateRef, ExecStep}, Error, }; -use eth_types::GethExecStep; +use eth_types::{GethExecStep, Word}; #[derive(Clone, Copy, Debug)] pub(crate) struct PushN; @@ -16,27 +16,34 @@ impl Opcode for PushN { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; - let max_len = { - let code_hash = state.call()?.code_hash; - let code_size = state.code(code_hash)?.len(); - let pc = geth_step.pc.0; - code_size - (pc + 1) - }; - + let code_hash = state.call()?.code_hash; + let code = state.code(code_hash)?; + let code_size = code.len(); + let pc = geth_step.pc.0; let data_len = geth_step.op.data_len(); - - let real_value = geth_steps[1].stack.last()?; - - let value = if data_len <= max_len { - real_value + let data_start = pc + 1; + let max_len = code_size - data_start; + let mut value_bytes = [0u8; 32]; + if data_len <= max_len { + value_bytes[32 - data_len..].copy_from_slice(&code[data_start..data_start + data_len]); } else { - // If the bytecode is truncated, the bytecode circuit interprets only the actual data - // without zero-padding. - let missing_bits = (data_len - max_len) * 8; - real_value >> missing_bits + value_bytes[32 - data_len..32 - data_len + max_len] + .copy_from_slice(&code[data_start..]); }; - - state.stack_write(&mut exec_step, geth_steps[1].stack.last_filled(), value)?; + let real_value = Word::from_big_endian(&value_bytes); + #[cfg(feature = "enable-stack")] + assert_eq!(real_value, geth_steps[1].stack.last()?); + let missing_bits = data_len.saturating_sub(max_len) * 8; + + state.call_ctx_mut()?.stack.push(real_value)?; + + // If the bytecode is truncated, the bytecode circuit interprets only the actual data + // without zero-padding. + state.stack_write( + &mut exec_step, + state.call_ctx()?.stack.last_filled(), + real_value >> missing_bits, + )?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/return_revert.rs b/bus-mapping/src/evm/opcodes/return_revert.rs index 2702d1d1c4..f055a2e083 100644 --- a/bus-mapping/src/evm/opcodes/return_revert.rs +++ b/bus-mapping/src/evm/opcodes/return_revert.rs @@ -27,10 +27,13 @@ impl Opcode for ReturnRevert { let step = &steps[0]; let mut exec_step = state.new_step(step)?; - let offset = step.stack.last()?; - let length = step.stack.nth_last(1)?; - state.stack_read(&mut exec_step, step.stack.last_filled(), offset)?; - state.stack_read(&mut exec_step, step.stack.nth_last_filled(1), length)?; + let offset = state.stack_pop(&mut exec_step)?; + let length = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + { + assert_eq!(offset, step.stack.nth_last(0)?); + assert_eq!(length, step.stack.nth_last(1)?); + } if !length.is_zero() { state @@ -149,7 +152,11 @@ impl Opcode for ReturnRevert { // Case C in the specs. if !call.is_root { - state.handle_restore_context(&mut exec_step, steps)?; + state.handle_restore_context( + (Some(offset.into()), Some(length.into())), + &mut exec_step, + steps, + )?; } // Case D in the specs. @@ -203,7 +210,12 @@ impl Opcode for ReturnRevert { } } - state.handle_return(&mut [&mut exec_step], steps, false)?; + state.handle_return( + (Some(offset.into()), Some(length.into())), + &mut [&mut exec_step], + steps, + false, + )?; Ok(vec![exec_step]) } } diff --git a/bus-mapping/src/evm/opcodes/returndatacopy.rs b/bus-mapping/src/evm/opcodes/returndatacopy.rs index 7f12f67458..8243af1c09 100644 --- a/bus-mapping/src/evm/opcodes/returndatacopy.rs +++ b/bus-mapping/src/evm/opcodes/returndatacopy.rs @@ -6,7 +6,7 @@ use crate::{ operation::CallContextField, Error, }; -use eth_types::GethExecStep; +use eth_types::{GethExecStep, Word}; #[derive(Clone, Copy, Debug)] pub(crate) struct Returndatacopy; @@ -17,78 +17,70 @@ impl Opcode for Returndatacopy { geth_steps: &[GethExecStep], ) -> Result, Error> { let geth_step = &geth_steps[0]; - let mut exec_steps = vec![gen_returndatacopy_step(state, geth_step)?]; - - let copy_event = gen_copy_event(state, geth_step, &mut exec_steps[0])?; - state.push_copy(&mut exec_steps[0], copy_event); - Ok(exec_steps) - } -} - -fn gen_returndatacopy_step( - state: &mut CircuitInputStateRef, - geth_step: &GethExecStep, -) -> Result { - let mut exec_step = state.new_step(geth_step)?; - let memory_offset = geth_step.stack.last()?; - let data_offset = geth_step.stack.nth_last(1)?; - let length = geth_step.stack.nth_last(2)?; - - state.stack_read(&mut exec_step, geth_step.stack.last_filled(), memory_offset)?; - state.stack_read( - &mut exec_step, - geth_step.stack.nth_last_filled(1), - data_offset, - )?; - state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(2), length)?; - - let call_id = state.call()?.call_id; - let call_ctx = state.call_ctx()?; - let return_data_len = call_ctx.return_data.len(); - let last_callee_id = state.call()?.last_callee_id; - let last_callee_return_data_offset = state.call()?.last_callee_return_data_offset; - let last_callee_return_data_length = state.call()?.last_callee_return_data_length; - - assert_eq!( - last_callee_return_data_length as usize, return_data_len, - "callee return data size should be correct" - ); - - // read last callee info - for (field, value) in [ - (CallContextField::LastCalleeId, last_callee_id.into()), - ( - CallContextField::LastCalleeReturnDataOffset, - if return_data_len == 0 { - 0.into() - } else { - last_callee_return_data_offset.into() - }, - ), - ( - CallContextField::LastCalleeReturnDataLength, - return_data_len.into(), - ), - ] { - state.call_context_read(&mut exec_step, call_id, field, value)?; + let mut exec_step = state.new_step(geth_step)?; + let memory_offset = state.stack_pop(&mut exec_step)?; + let data_offset = state.stack_pop(&mut exec_step)?; + let length = state.stack_pop(&mut exec_step)?; + + #[cfg(feature = "enable-stack")] + { + assert_eq!(memory_offset, geth_step.stack.nth_last(0)?); + assert_eq!(data_offset, geth_step.stack.nth_last(1)?); + assert_eq!(length, geth_step.stack.nth_last(2)?); + } + + let call_id = state.call()?.call_id; + let call_ctx = state.call_ctx()?; + let return_data_len = call_ctx.return_data.len(); + let last_callee_id = state.call()?.last_callee_id; + let last_callee_return_data_offset = state.call()?.last_callee_return_data_offset; + let last_callee_return_data_length = state.call()?.last_callee_return_data_length; + + assert_eq!( + last_callee_return_data_length as usize, return_data_len, + "callee return data size should be correct" + ); + + // read last callee info + for (field, value) in [ + (CallContextField::LastCalleeId, last_callee_id.into()), + ( + CallContextField::LastCalleeReturnDataOffset, + if return_data_len == 0 { + 0.into() + } else { + last_callee_return_data_offset.into() + }, + ), + ( + CallContextField::LastCalleeReturnDataLength, + return_data_len.into(), + ), + ] { + state.call_context_read(&mut exec_step, call_id, field, value)?; + } + + let copy_event = gen_copy_event(state, memory_offset, data_offset, length, &mut exec_step)?; + state.push_copy(&mut exec_step, copy_event); + Ok(vec![exec_step]) } - Ok(exec_step) } fn gen_copy_event( state: &mut CircuitInputStateRef, - geth_step: &GethExecStep, + memory_offset: Word, + data_offset: Word, + length: Word, exec_step: &mut ExecStep, ) -> Result { let rw_counter_start = state.block_ctx.rwc; // Get low Uint64 of offset. - let dst_addr = geth_step.stack.last()?; - let data_offset = geth_step.stack.nth_last(1)?; - let length = geth_step.stack.nth_last(2)?; - - let (dst_addr, data_offset, length) = - (dst_addr.low_u64(), data_offset.as_u64(), length.as_u64()); + let (dst_addr, data_offset, length) = ( + memory_offset.low_u64(), + data_offset.as_u64(), + length.as_u64(), + ); let last_callee_return_data_offset = state.call()?.last_callee_return_data_offset; let last_callee_return_data_length = state.call()?.last_callee_return_data_length; diff --git a/bus-mapping/src/evm/opcodes/returndatasize.rs b/bus-mapping/src/evm/opcodes/returndatasize.rs index 0bb5d67174..b9dfceee44 100644 --- a/bus-mapping/src/evm/opcodes/returndatasize.rs +++ b/bus-mapping/src/evm/opcodes/returndatasize.rs @@ -18,33 +18,29 @@ impl Opcode for Returndatasize { ) -> Result, Error> { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; - let value = geth_steps[1].stack.last()?; + let size = state.call_ctx()?.return_data.len(); state.call_context_read( &mut exec_step, state.call()?.call_id, CallContextField::LastCalleeReturnDataLength, - value, + size.into(), )?; // TODO: fix error in deposit_ether.json... - let real_return_data_len = value.as_usize(); - let call_ctx = state.call_ctx()?; - let local_return_data_len = call_ctx.return_data.len(); - if real_return_data_len != local_return_data_len { - log::error!( - "return_data.len() != RETURNDATASIZE value, {} != {}, step: {:?}", - local_return_data_len, - real_return_data_len, - geth_step - ); - debug_assert_eq!(real_return_data_len, local_return_data_len); + #[cfg(feature = "enable-stack")] + { + let real_size = geth_steps[1].stack.last()?.as_usize(); + if real_size != size { + log::error!( + "return_data.len() != RETURNDATASIZE value, {} != {}, step: {:?}", + size, + real_size, + geth_step + ); + assert_eq!(real_size, size); + } } - - state.stack_write( - &mut exec_step, - geth_step.stack.last_filled().map(|a| a - 1), - value, - )?; + state.stack_push(&mut exec_step, size.into())?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/selfbalance.rs b/bus-mapping/src/evm/opcodes/selfbalance.rs index 119d47499a..59051167c2 100644 --- a/bus-mapping/src/evm/opcodes/selfbalance.rs +++ b/bus-mapping/src/evm/opcodes/selfbalance.rs @@ -16,8 +16,10 @@ impl Opcode for Selfbalance { ) -> 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; + let self_balance = state.sdb.get_balance(&callee_address); + #[cfg(feature = "enable-stack")] + assert_eq!(self_balance, geth_steps[1].stack.last()?); // CallContext read of the callee_address state.call_context_read( @@ -36,11 +38,7 @@ impl Opcode for Selfbalance { )?; // Stack write of self_balance - state.stack_write( - &mut exec_step, - geth_step.stack.last_filled().map(|a| a - 1), - self_balance, - )?; + state.stack_push(&mut exec_step, self_balance)?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/evm/opcodes/sha3.rs b/bus-mapping/src/evm/opcodes/sha3.rs index 297a711fbc..4db828acdb 100644 --- a/bus-mapping/src/evm/opcodes/sha3.rs +++ b/bus-mapping/src/evm/opcodes/sha3.rs @@ -24,15 +24,13 @@ impl Opcode for Sha3 { let geth_step = &geth_steps[0]; let mut exec_step = state.new_step(geth_step)?; - let expected_sha3 = geth_steps[1].stack.last()?; - - // byte offset in the memory. - let offset = geth_step.stack.last()?; - state.stack_read(&mut exec_step, geth_step.stack.last_filled(), offset)?; - - // byte size to read in the memory. - let size = geth_step.stack.nth_last(1)?; - state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(1), size)?; + let offset = state.stack_pop(&mut exec_step)?; + let size = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + { + assert_eq!(offset, geth_step.stack.nth_last(0)?); + assert_eq!(size, geth_step.stack.nth_last(1)?); + } if size.gt(&U256::zero()) { state @@ -51,12 +49,10 @@ impl Opcode for Sha3 { // keccak-256 hash of the given data in memory. let sha3 = keccak256(&sha3_input); - debug_assert_eq!(Word::from_big_endian(&sha3), expected_sha3); - state.stack_write( - &mut exec_step, - geth_steps[1].stack.last_filled(), - sha3.into(), - )?; + let _output = Word::from_big_endian(&sha3); + #[cfg(feature = "enable-stack")] + assert_eq!(_output, geth_steps[1].stack.last()?); + state.stack_push(&mut exec_step, sha3.into())?; // Memory read operations let rw_counter_start = state.block_ctx.rwc; diff --git a/bus-mapping/src/evm/opcodes/sload.rs b/bus-mapping/src/evm/opcodes/sload.rs index 18d049b9b5..b8f39a4feb 100644 --- a/bus-mapping/src/evm/opcodes/sload.rs +++ b/bus-mapping/src/evm/opcodes/sload.rs @@ -52,20 +52,21 @@ impl Opcode for Sload { )?; // First stack read - let key = geth_step.stack.last()?; - let stack_position = geth_step.stack.last_filled(); - - // Manage first stack read at latest stack position - state.stack_read(&mut exec_step, stack_position, key)?; + let key = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + assert_eq!(key, geth_step.stack.last()?); // Storage read let value_from_statedb = *state.sdb.get_storage(&contract_addr, &key).1; { let value_from_step = geth_step.storage.get_or_err(&key)?; - let value_from_stack = geth_steps[1].stack.last().unwrap(); - if !(value_from_step == value_from_statedb && value_from_step == value_from_stack) { - panic!("inconsistent sload: step proof {value_from_step:?}, local statedb {value_from_statedb:?}, result {value_from_stack:?} in contract {contract_addr:?}, key {key:?}", ); - } + + // 1. value_from_step == value_from_statedb + assert_eq!(value_from_step, value_from_statedb, "inconsistent sload: step proof {value_from_step:?}, local statedb {value_from_statedb:?} in contract {contract_addr:?}, key {key:?}",); + + // 2. value_from_step == value_from_stack + #[cfg(feature = "enable-stack")] + assert_eq!(value_from_step, geth_steps[1].stack.last()?, "inconsistent sload: step proof {value_from_step:?}, result {:?} in contract {contract_addr:?}, key {key:?}", geth_steps[1].stack.last()?); } let value = value_from_statedb; @@ -89,7 +90,7 @@ impl Opcode for Sload { )?; // First stack write - state.stack_write(&mut exec_step, stack_position, value)?; + state.stack_push(&mut exec_step, value)?; state.push_op( &mut exec_step, RW::READ, diff --git a/bus-mapping/src/evm/opcodes/sstore.rs b/bus-mapping/src/evm/opcodes/sstore.rs index 85a7f6f334..6213919b8a 100644 --- a/bus-mapping/src/evm/opcodes/sstore.rs +++ b/bus-mapping/src/evm/opcodes/sstore.rs @@ -58,13 +58,13 @@ impl Opcode for Sstore { state.call()?.address.to_word(), )?; - let key = geth_step.stack.last()?; - let key_stack_position = geth_step.stack.last_filled(); - let value = geth_step.stack.nth_last(1)?; - let value_stack_position = geth_step.stack.nth_last_filled(1); - - state.stack_read(&mut exec_step, key_stack_position, key)?; - state.stack_read(&mut exec_step, value_stack_position, value)?; + let key = state.stack_pop(&mut exec_step)?; + let value = state.stack_pop(&mut exec_step)?; + #[cfg(feature = "enable-stack")] + { + assert_eq!(key, geth_step.stack.nth_last(0)?); + assert_eq!(value, geth_step.stack.nth_last(1)?); + } let is_warm = state .sdb diff --git a/bus-mapping/src/evm/opcodes/stackonlyop.rs b/bus-mapping/src/evm/opcodes/stackonlyop.rs index 5cadcda351..ca398a3190 100644 --- a/bus-mapping/src/evm/opcodes/stackonlyop.rs +++ b/bus-mapping/src/evm/opcodes/stackonlyop.rs @@ -22,19 +22,17 @@ impl Opcode for StackPopOnlyOpcode Opcode for Swap { let mut exec_step = state.new_step(geth_step)?; // Peek b and a - let stack_b_value_read = geth_step.stack.nth_last(N)?; - let stack_b_position = geth_step.stack.nth_last_filled(N); + let stack_b_value_read = state.call_ctx()?.stack.nth_last(N)?; + let stack_b_position = state.call_ctx()?.stack.nth_last_filled(N); state.stack_read(&mut exec_step, 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(); + let stack_a_value_read = state.call_ctx()?.stack.last()?; + let stack_a_position = state.call_ctx()?.stack.last_filled(); state.stack_read(&mut exec_step, stack_a_position, stack_a_value_read)?; + #[cfg(feature = "enable-stack")] + { + assert_eq!(stack_b_value_read, geth_step.stack.nth_last(N)?); + assert_eq!(stack_b_position, geth_step.stack.nth_last_filled(N)); + + assert_eq!(stack_a_value_read, geth_step.stack.last()?); + assert_eq!(stack_a_position, geth_step.stack.last_filled()); + } + // Write a into b_position, write b into a_position state.stack_write(&mut exec_step, stack_b_position, stack_a_value_read)?; state.stack_write(&mut exec_step, stack_a_position, stack_b_value_read)?; + state.call_ctx_mut()?.stack.swap(N)?; Ok(vec![exec_step]) } diff --git a/bus-mapping/src/mock.rs b/bus-mapping/src/mock.rs index fb5147f262..79f60b77e1 100644 --- a/bus-mapping/src/mock.rs +++ b/bus-mapping/src/mock.rs @@ -55,7 +55,7 @@ impl BlockData { let mut sdb = StateDB::new(); let mut code_db = CodeDB::new(); - let access_set: AccessSet = AccessSet::from_geth_data(&geth_data); + let access_set = AccessSet::from_geth_data(&geth_data); // Initialize all accesses accounts to zero for addr in access_set.state.keys() { sdb.set_account(addr, state_db::Account::zero()); diff --git a/eth-types/src/evm_types/stack.rs b/eth-types/src/evm_types/stack.rs index 1f6839aee9..2b7d77df49 100644 --- a/eth-types/src/evm_types/stack.rs +++ b/eth-types/src/evm_types/stack.rs @@ -110,6 +110,16 @@ impl Stack { Stack(words) } + /// Returns the length of the stack. + pub fn len(&self) -> usize { + self.0.len() + } + + /// Returns if the stack is empty. + pub fn is_empty(&self) -> bool { + self.0.is_empty() + } + /// Returns the first available/free `StackAddress`. pub fn stack_pointer(&self) -> StackAddress { // Stack has 1024 slots. @@ -142,6 +152,30 @@ impl Stack { .cloned() .ok_or(Error::InvalidStackPointer) } + + /// pops the last [`Word`] allocated in the `Stack`. + pub fn pop(&mut self) -> Result { + self.0.pop().ok_or(Error::InvalidStackPointer) + } + + /// pushes a [`Word`] allocated in the `Stack`. + pub fn push(&mut self, word: Word) -> Result<(), Error> { + if self.0.len() >= 1024 { + return Err(Error::InvalidStackPointer); + } + self.0.push(word); + Ok(()) + } + + /// swap last and n_last [`Word`] allocated in the `Stack`. + pub fn swap(&mut self, n_last: usize) -> Result<(), Error> { + if self.0.len() < (n_last + 1) { + return Err(Error::InvalidStackPointer); + } + let last = self.0.len() - 1; + self.0.swap(last, last - n_last); + Ok(()) + } } #[cfg(test)] diff --git a/eth-types/src/geth_types.rs b/eth-types/src/geth_types.rs index e82becfe8e..d49db9178c 100644 --- a/eth-types/src/geth_types.rs +++ b/eth-types/src/geth_types.rs @@ -1,5 +1,7 @@ //! Types needed for generating Ethereum traces +#[cfg(feature = "scroll")] +use crate::l2_types::BlockTrace; use crate::{ sign_types::{biguint_to_32bytes_le, ct_option_ok_or, recover_pk2, SignData, SECP256K1_Q}, AccessList, Address, Block, Bytes, Error, GethExecTrace, Hash, ToBigEndian, ToLittleEndian, @@ -396,6 +398,9 @@ pub struct GethData { pub geth_traces: Vec, /// Accounts pub accounts: Vec, + /// block trace + #[cfg(feature = "scroll")] + pub block_trace: BlockTrace, } /* impl GethData { diff --git a/eth-types/src/lib.rs b/eth-types/src/lib.rs index e2dd90b439..3a27ee2ed8 100644 --- a/eth-types/src/lib.rs +++ b/eth-types/src/lib.rs @@ -28,12 +28,9 @@ pub mod geth_types; pub mod l2_types; pub mod sign_types; +use crate::evm_types::{Gas, GasCost, OpcodeId, ProgramCounter}; pub use bytecode::Bytecode; pub use error::Error; -use halo2_base::utils::ScalarField; -use halo2_proofs::halo2curves::{bn256::Fr, group::ff::PrimeField}; - -use crate::evm_types::{Gas, GasCost, OpcodeId, ProgramCounter}; use ethers_core::types; pub use ethers_core::{ abi::ethereum_types::{BigEndianHash, U512}, @@ -45,9 +42,11 @@ pub use ethers_core::{ Address, Block, Bytes, Signature, H160, H256, H64, U256, U64, }, }; +use halo2_base::utils::ScalarField; +use halo2_proofs::halo2curves::{bn256::Fr, group::ff::PrimeField}; use serde::{de, Deserialize, Deserializer, Serialize}; use std::{ - collections::HashMap, + collections::{HashMap, HashSet}, fmt, fmt::{Display, Formatter}, str::FromStr, @@ -680,6 +679,7 @@ pub struct ResultGethExecTrace { /// the memory size before the expansion, so that it corresponds to the memory /// before the step is executed. #[derive(Deserialize, Serialize, Clone, Debug, Eq, PartialEq)] +#[serde(deny_unknown_fields)] pub struct GethExecTrace { /// L1 fee #[serde(default)] @@ -749,18 +749,37 @@ pub struct GethCallTrace { #[serde(default)] calls: Vec, error: Option, - // from: Address, + from: Address, // gas: U256, - // #[serde(rename = "gasUsed")] - // gas_used: U256, + #[serde(rename = "gasUsed")] + gas_used: U256, // input: Bytes, - // output: Bytes, - // to: Option
, + output: Option, + to: Option
, #[serde(rename = "type")] call_type: String, // value: U256, } +/// Flattened Call Trace +#[derive(Clone, Debug, Eq, PartialEq)] +pub struct FlatGethCallTrace { + // error: Option, + /// from + pub from: Address, + // gas: U256, + /// gas used + pub gas_used: U256, + // input: Bytes, + /// is callee code empty + pub is_callee_code_empty: bool, + /// to + pub to: Option
, + /// call type + pub call_type: OpcodeId, + // value: U256, +} + impl GethCallTrace { /// generate the call_is_success vec pub fn gen_call_is_success(&self, mut call_is_success: Vec) -> Vec { @@ -770,6 +789,58 @@ impl GethCallTrace { } call_is_success } + + /// flatten the call trace as it is. + pub fn flatten_trace( + &self, + prestate: &HashMap, + ) -> Vec { + let mut trace = vec![]; + // store the code that created in this tx, which is not include in prestate + let mut created = HashSet::new(); + self.flatten_trace_inner(prestate, &mut trace, &mut created); + trace + } + + fn flatten_trace_inner( + &self, + prestate: &HashMap, + trace: &mut Vec, + created: &mut HashSet
, + ) { + let call_type = OpcodeId::from_str(&self.call_type).unwrap(); + let is_callee_code_empty = self + .to + .as_ref() + .map(|addr| { + !created.contains(addr) + && prestate + .get(addr) + .unwrap() + .code + .as_ref() + .unwrap() + .is_empty() + }) + .unwrap_or(false); + + trace.push(FlatGethCallTrace { + from: self.from, + to: self.to, + is_callee_code_empty, + gas_used: self.gas_used, + call_type, + }); + + for call in &self.calls { + call.flatten_trace_inner(prestate, trace, created); + } + + let has_output = self.output.as_ref().map(|x| !x.is_empty()).unwrap_or(false); + if call_type.is_create() && has_output { + created.insert(self.to.unwrap()); + } + } } #[macro_export] @@ -808,10 +879,12 @@ macro_rules! word_map { #[cfg(test)] mod tests { use super::*; - use crate::evm_types::{opcode_ids::OpcodeId, stack::Stack}; + use crate::evm_types::opcode_ids::OpcodeId; #[cfg(feature = "enable-memory")] use crate::evm_types::memory::Memory; + #[cfg(feature = "enable-stack")] + use crate::evm_types::stack::Stack; #[test] fn test_to_u16_array() { @@ -890,7 +963,11 @@ mod tests { "callTrace": { "calls": [], "error": null, - "type": "CALL" + "from": "0x000000000000000000000000000000000cafe001", + "to": null, + "gasUsed": "0x0", + "type": "CALL", + "output": "0x00" } } "#; @@ -978,7 +1055,11 @@ mod tests { call_trace: GethCallTrace { calls: Vec::new(), error: None, + from: address!("0x000000000000000000000000000000000cafe001"), + to: None, + gas_used: U256::zero(), call_type: "CALL".to_string(), + output: Some(Bytes::from([0x00])) } } ); diff --git a/external-tracer/src/lib.rs b/external-tracer/src/lib.rs index 0ab92d5ff4..c3f0ba09e7 100644 --- a/external-tracer/src/lib.rs +++ b/external-tracer/src/lib.rs @@ -65,6 +65,13 @@ impl LoggerConfig { ..Self::default() } } + + pub fn enable_stack() -> Self { + Self { + disable_stack: false, + ..Self::default() + } + } } /// Configuration structure for `params.ChainConfig` diff --git a/mock/src/test_ctx.rs b/mock/src/test_ctx.rs index 33c603f38d..af5d7a8c6f 100644 --- a/mock/src/test_ctx.rs +++ b/mock/src/test_ctx.rs @@ -94,7 +94,6 @@ pub struct TestContext { pub eth_block: eth_types::Block, /// Execution Trace from geth pub geth_traces: Vec, - #[cfg(feature = "scroll")] block_trace: BlockTrace, } @@ -107,6 +106,8 @@ impl From> for GethD eth_block: ctx.eth_block, geth_traces: ctx.geth_traces.to_vec(), accounts: ctx.accounts.into(), + #[cfg(feature = "scroll")] + block_trace: ctx.block_trace, } } } diff --git a/testool/Cargo.toml b/testool/Cargo.toml index f879c12d34..49d13d0f2c 100644 --- a/testool/Cargo.toml +++ b/testool/Cargo.toml @@ -40,7 +40,7 @@ urlencoding = "2.1.2" ctor.workspace = true [features] -default = ["ignore-test-docker", "skip-self-destruct", "shanghai", "enable-stack", "enable-storage"] +default = ["ignore-test-docker", "skip-self-destruct", "shanghai", "enable-storage"] onephase = ["zkevm-circuits/onephase"] ignore-test-docker = [] skip-self-destruct = [] diff --git a/testool/src/statetest/executor.rs b/testool/src/statetest/executor.rs index 16efcf1fee..f2bc8e1c43 100644 --- a/testool/src/statetest/executor.rs +++ b/testool/src/statetest/executor.rs @@ -189,7 +189,8 @@ fn into_traceconfig(st: StateTest) -> (String, TraceConfig, StateTestResult) { logger_config: LoggerConfig { enable_memory: cfg!(feature = "enable-memory") && bus_mapping::util::GETH_TRACE_CHECK_LEVEL.should_check(), - disable_stack: !cfg!(feature = "enable-stack"), + disable_stack: !cfg!(feature = "enable-stack") + && bus_mapping::util::GETH_TRACE_CHECK_LEVEL.should_check(), disable_storage: !cfg!(feature = "enable-storage"), ..Default::default() }, @@ -548,7 +549,7 @@ pub fn run_test( log::info!("{test_id}: run-test BEGIN - {circuits_config:?}"); // get the geth traces - #[allow(unused_mut)] + #[cfg_attr(not(feature = "scroll"), allow(unused_mut))] let (_, mut trace_config, post) = into_traceconfig(st.clone()); let balance_overflow = trace_config diff --git a/zkevm-circuits/Cargo.toml b/zkevm-circuits/Cargo.toml index 3ce800c05a..5821068662 100644 --- a/zkevm-circuits/Cargo.toml +++ b/zkevm-circuits/Cargo.toml @@ -55,7 +55,7 @@ cli-table = "0.4" paste = "1.0" [features] -default = ["test", "test-circuits", "shanghai", "debug-annotations", "parallel_syn", "enable-stack", "enable-storage"] +default = ["test", "test-circuits", "shanghai", "debug-annotations", "parallel_syn", "enable-storage"] test = ["ethers-signers", "mock", "bus-mapping/test"] scroll = ["bus-mapping/scroll", "eth-types/scroll", "mock?/scroll", "zktrie", "poseidon-codehash"] @@ -70,7 +70,6 @@ poseidon-codehash = [] parallel_syn = ["hash-circuit/parallel_syn", "halo2_proofs/parallel_syn"] debug-annotations = [] - enable-stack = ["bus-mapping/enable-stack"] enable-memory = ["bus-mapping/enable-memory"] enable-storage = ["bus-mapping/enable-storage"] diff --git a/zkevm-circuits/src/evm_circuit/execution/addmod.rs b/zkevm-circuits/src/evm_circuit/execution/addmod.rs index 9d695ca064..d5c9c55e39 100644 --- a/zkevm-circuits/src/evm_circuit/execution/addmod.rs +++ b/zkevm-circuits/src/evm_circuit/execution/addmod.rs @@ -226,10 +226,13 @@ impl ExecutionGadget for AddModGadget { #[cfg(test)] mod test { use crate::test_util::CircuitTestBuilder; - use eth_types::{bytecode, evm_types::Stack, Word}; + use eth_types::{bytecode, Word}; use mock::TestContext; - fn test(a: Word, b: Word, n: Word, r: Option, ok: bool) { + #[cfg(feature = "enable-stack")] + use eth_types::evm_types::Stack; + + fn test(a: Word, b: Word, n: Word, _r: Option, ok: bool) { let bytecode = bytecode! { PUSH32(n) PUSH32(b) @@ -238,9 +241,12 @@ mod test { STOP }; + #[allow(unused_mut)] let mut ctx = TestContext::<2, 1>::simple_ctx_with_bytecode(bytecode).unwrap(); - if let Some(r) = r { - let last = ctx + #[cfg(feature = "enable-stack")] + if let Some(r) = _r { + #[allow(unused_mut)] + let mut last = ctx .geth_traces .first_mut() .unwrap() @@ -295,18 +301,21 @@ mod test { #[test] fn addmod_bad_r_on_nonzero_n() { test_ok_u32(7, 18, 10, Some(5)); + // skip due to stack reconstruct assert equal // test_ko_u32(7, 18, 10, Some(6)) } #[test] fn addmod_bad_r_on_zero_n() { test_ok_u32(2, 3, 0, Some(0)); + // skip due to stack reconstruct assert equal // test_ko_u32(2, 3, 0, Some(1)) } #[test] fn addmod_bad_r_bigger_n() { test_ok_u32(2, 3, 4, Some(1)); + // skip due to stack reconstruct assert equal // test_ko_u32(2, 3, 4, Some(5)) } } diff --git a/zkevm-circuits/src/evm_circuit/execution/callop.rs b/zkevm-circuits/src/evm_circuit/execution/callop.rs index 2a081a82d3..6a201f440b 100644 --- a/zkevm-circuits/src/evm_circuit/execution/callop.rs +++ b/zkevm-circuits/src/evm_circuit/execution/callop.rs @@ -491,11 +491,10 @@ impl ExecutionGadget for CallOpGadget { is_create: To(false.expr()), code_hash: To(cb.empty_code_hash_rlc()), program_counter: Delta(1.expr()), - stack_pointer: Delta(stack_pointer_delta.expr()), gas_left: To(callee_gas_left.expr()), memory_word_size: To(precompile_output_rws.expr()), reversible_write_counter: To(callee_reversible_rwc_delta.expr()), - ..StepStateTransition::default() + ..StepStateTransition::new_context() }); let precompile_gadget = PrecompileGadget::construct( @@ -1721,17 +1720,20 @@ mod test_precompiles { ) .unwrap(); - let step = ctx.geth_traces[0] - .struct_logs - .last() - .expect("at least one step"); - log::debug!("{:?}", step.stack); - for (offset, (_, stack_value)) in arg.stack_value.iter().enumerate() { - assert_eq!( - *stack_value, - step.stack.nth_last(offset).expect("stack value not found"), - "stack output mismatch" - ); + #[cfg(feature = "enable-stack")] + { + let step = ctx.geth_traces[0] + .struct_logs + .last() + .expect("at least one step"); + log::debug!("{:?}", step.stack); + for (offset, (_, stack_value)) in arg.stack_value.iter().enumerate() { + assert_eq!( + *stack_value, + step.stack.nth_last(offset).expect("stack value not found"), + "stack output mismatch" + ); + } } log::debug!( diff --git a/zkevm-circuits/src/evm_circuit/execution/mulmod.rs b/zkevm-circuits/src/evm_circuit/execution/mulmod.rs index 3908422de6..f1347b73c9 100644 --- a/zkevm-circuits/src/evm_circuit/execution/mulmod.rs +++ b/zkevm-circuits/src/evm_circuit/execution/mulmod.rs @@ -167,10 +167,13 @@ impl ExecutionGadget for MulModGadget { #[cfg(test)] mod test { use crate::test_util::CircuitTestBuilder; - use eth_types::{bytecode, evm_types::Stack, Word, U256}; + use eth_types::{bytecode, Word, U256}; use mock::TestContext; - fn test(a: Word, b: Word, n: Word, r: Option, ok: bool) { + #[cfg(feature = "enable-stack")] + use eth_types::evm_types::Stack; + + fn test(a: Word, b: Word, n: Word, _r: Option, ok: bool) { let bytecode = bytecode! { PUSH32(n) PUSH32(b) @@ -179,9 +182,12 @@ mod test { STOP }; + #[allow(unused_mut)] let mut ctx = TestContext::<2, 1>::simple_ctx_with_bytecode(bytecode).unwrap(); - if let Some(r) = r { - let last = ctx + #[cfg(feature = "enable-stack")] + if let Some(r) = _r { + #[allow(unused_mut)] + let mut last = ctx .geth_traces .first_mut() .unwrap() @@ -257,19 +263,23 @@ mod test { #[test] fn mulmod_bad_r_on_nonzero_n() { test_ok_u32(7, 18, 10, Some(6)); + // skip due to stack reconstruct assert equal // test_ko_u32(7, 18, 10, Some(7)); + // skip due to stack reconstruct assert equal // test_ko_u32(7, 18, 10, Some(5)); } #[test] fn mulmod_bad_r_on_zero_n() { test_ok_u32(2, 3, 0, Some(0)); + // skip due to stack reconstruct assert equal // test_ko_u32(2, 3, 0, Some(1)); } #[test] fn mulmod_bad_r_bigger_n() { test_ok_u32(2, 3, 5, Some(1)); + // skip due to stack reconstruct assert equal // test_ko_u32(2, 3, 5, Some(5)); } } diff --git a/zkevm-circuits/src/evm_circuit/execution/return_revert.rs b/zkevm-circuits/src/evm_circuit/execution/return_revert.rs index 6a086ce435..4c241e74f2 100644 --- a/zkevm-circuits/src/evm_circuit/execution/return_revert.rs +++ b/zkevm-circuits/src/evm_circuit/execution/return_revert.rs @@ -503,14 +503,15 @@ impl ExecutionGadget for ReturnRevertGadget { mod test { use crate::test_util::CircuitTestBuilder; use eth_types::{ - address, bytecode, - evm_types::OpcodeId, - geth_types::{Account, GethData}, - Address, Bytecode, ToWord, Word, U256, + address, bytecode, evm_types::OpcodeId, geth_types::Account, Address, Bytecode, ToWord, + Word, }; use itertools::Itertools; use mock::{eth, TestContext, MOCK_ACCOUNTS}; + #[cfg(feature = "enable-stack")] + use eth_types::{geth_types::GethData, U256}; + const CALLEE_ADDRESS: Address = Address::repeat_byte(0xff); const CALLER_ADDRESS: Address = Address::repeat_byte(0x34); @@ -933,10 +934,12 @@ mod test { RETURNDATACOPY }); - let block: GethData = TestContext::<2, 1>::simple_ctx_with_bytecode(bytecode.clone()) - .unwrap() - .into(); + #[cfg(feature = "enable-stack")] if is_return { + let block: GethData = + TestContext::<2, 1>::simple_ctx_with_bytecode(bytecode.clone()) + .unwrap() + .into(); // collect return opcode, retrieve next step, assure both contract create // successfully let created_contract_addr = block.geth_traces[0]