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 6ddc4d0f3b0..c3b7519fbaa 100644 --- a/bus-mapping/src/circuit_input_builder/input_state_ref.rs +++ b/bus-mapping/src/circuit_input_builder/input_state_ref.rs @@ -977,8 +977,17 @@ impl<'a> CircuitInputStateRef<'a> { if !self.call()?.is_root { let (offset, length) = match step.op { OpcodeId::RETURN | OpcodeId::REVERT => { - let offset = step.stack.nth_last(0)?.as_usize(); - let length = step.stack.nth_last(1)?.as_usize(); + let (offset, length) = if step.error.is_some() + || (self.call()?.is_create() && self.call()?.is_success) + { + (0, 0) + } else { + ( + step.stack.nth_last(0)?.as_usize(), + step.stack.nth_last(1)?.as_usize(), + ) + }; + // At the moment it conflicts with `call_ctx` and `caller_ctx`. let callee_memory = self.call_ctx()?.memory.clone(); let caller_ctx = self.caller_ctx_mut()?; @@ -1063,8 +1072,9 @@ impl<'a> CircuitInputStateRef<'a> { let geth_step = steps .get(0) .ok_or(Error::InternalError("invalid index 0"))?; - let is_revert_or_return_call_success = geth_step.op == OpcodeId::REVERT - || geth_step.op == OpcodeId::RETURN && exec_step.error.is_none(); + let is_revert_or_return_call_success = (geth_step.op == OpcodeId::REVERT + || geth_step.op == OpcodeId::RETURN) + && exec_step.error.is_none(); if !is_revert_or_return_call_success && !call.is_success { // add call failure ops for exception cases @@ -1118,25 +1128,29 @@ impl<'a> CircuitInputStateRef<'a> { _ => [Word::zero(), Word::zero()], }; - let curr_memory_word_size = (exec_step.memory_size as u64) / 32; - let next_memory_word_size = if !last_callee_return_data_length.is_zero() { - std::cmp::max( - (last_callee_return_data_offset + last_callee_return_data_length + 31).as_u64() - / 32, - curr_memory_word_size, - ) + let gas_refund = if exec_step.error.is_some() { + 0 } else { - curr_memory_word_size - }; + let curr_memory_word_size = (exec_step.memory_size as u64) / 32; + let next_memory_word_size = if !last_callee_return_data_length.is_zero() { + std::cmp::max( + (last_callee_return_data_offset + last_callee_return_data_length + 31).as_u64() + / 32, + curr_memory_word_size, + ) + } else { + curr_memory_word_size + }; - let memory_expansion_gas_cost = - memory_expansion_gas_cost(curr_memory_word_size, next_memory_word_size); - let code_deposit_cost = if call.is_create() && call.is_success { - GasCost::CODE_DEPOSIT_BYTE_COST * last_callee_return_data_length.as_u64() - } else { - 0 + let memory_expansion_gas_cost = + memory_expansion_gas_cost(curr_memory_word_size, next_memory_word_size); + let code_deposit_cost = if call.is_create() && call.is_success { + GasCost::CODE_DEPOSIT_BYTE_COST * last_callee_return_data_length.as_u64() + } else { + 0 + }; + geth_step.gas - memory_expansion_gas_cost - code_deposit_cost }; - let gas_refund = geth_step.gas - memory_expansion_gas_cost - code_deposit_cost; let caller_gas_left = if is_revert_or_return_call_success || call.is_success { geth_step_next.gas - gas_refund @@ -1170,11 +1184,13 @@ impl<'a> CircuitInputStateRef<'a> { } // EIP-211: CREATE/CREATE2 call successful case should set RETURNDATASIZE = 0 + let discard_return_data = + call.is_create() && geth_step.op == OpcodeId::RETURN || exec_step.error.is_some(); for (field, value) in [ (CallContextField::LastCalleeId, call.call_id.into()), ( CallContextField::LastCalleeReturnDataOffset, - if call.is_create() && call.is_success { + if discard_return_data { U256::zero() } else { last_callee_return_data_offset @@ -1182,7 +1198,7 @@ impl<'a> CircuitInputStateRef<'a> { ), ( CallContextField::LastCalleeReturnDataLength, - if call.is_create() && call.is_success { + if discard_return_data { U256::zero() } else { last_callee_return_data_length diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index 30583d920d5..5a1e477bc5b 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -279,6 +279,9 @@ fn fn_gen_error_state_associated_ops(error: &ExecError) -> Option Some(OOGExp::gen_associated_ops), ExecError::OutOfGas(OogError::Log) => Some(ErrorOOGLog::gen_associated_ops), ExecError::OutOfGas(OogError::MemoryCopy) => Some(OOGMemoryCopy::gen_associated_ops), + ExecError::OutOfGas(OogError::DynamicMemoryExpansion) => { + Some(StackOnlyOpcode::<2, 0, true>::gen_associated_ops) + } ExecError::OutOfGas(OogError::StaticMemoryExpansion) => { Some(StackOnlyOpcode::<1, 0, true>::gen_associated_ops) } diff --git a/bus-mapping/src/evm/opcodes/return_revert.rs b/bus-mapping/src/evm/opcodes/return_revert.rs index 30b96361467..fdd405a96cd 100644 --- a/bus-mapping/src/evm/opcodes/return_revert.rs +++ b/bus-mapping/src/evm/opcodes/return_revert.rs @@ -11,7 +11,6 @@ use eth_types::{evm_types::INVALID_INIT_CODE_FIRST_BYTE, Bytecode, GethExecStep, #[derive(Debug, Copy, Clone)] pub(crate) struct ReturnRevert; -// TODO: rename to indicate this handles REVERT (and maybe STOP)? impl Opcode for ReturnRevert { fn gen_associated_ops( state: &mut CircuitInputStateRef, diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index 7719d52e1d2..89e13690e4d 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -74,6 +74,7 @@ mod error_invalid_opcode; mod error_oog_account_access; mod error_oog_call; mod error_oog_constant; +mod error_oog_dynamic_memory; mod error_oog_exp; mod error_oog_log; mod error_oog_memory_copy; @@ -150,6 +151,7 @@ use error_invalid_opcode::ErrorInvalidOpcodeGadget; use error_oog_account_access::ErrorOOGAccountAccessGadget; use error_oog_call::ErrorOOGCallGadget; use error_oog_constant::ErrorOOGConstantGadget; +use error_oog_dynamic_memory::ErrorOOGDynamicMemoryGadget; use error_oog_exp::ErrorOOGExpGadget; use error_oog_log::ErrorOOGLogGadget; use error_oog_memory_copy::ErrorOOGMemoryCopyGadget; @@ -304,8 +306,7 @@ pub struct ExecutionConfig { error_oog_static_memory_gadget: Box>, error_stack: Box>, error_write_protection: Box>, - error_oog_dynamic_memory_gadget: - Box>, + error_oog_dynamic_memory_gadget: Box>, error_oog_log: Box>, error_oog_sha3: Box>, error_oog_account_access: Box>, diff --git a/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs b/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs index f1bb627084d..3440bd9f44a 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs @@ -82,13 +82,8 @@ impl ExecutionGadget for ErrorCodeStoreGadget { vec![1.expr(), 2.expr()], ); - let common_error_gadget = CommonErrorGadget::construct_with_return_data( - cb, - opcode.expr(), - cb.rw_counter_offset(), - memory_address.offset(), - memory_address.length(), - ); + let common_error_gadget = + CommonErrorGadget::construct(cb, opcode.expr(), cb.rw_counter_offset()); Self { opcode, diff --git a/zkevm-circuits/src/evm_circuit/execution/error_invalid_creation_code.rs b/zkevm-circuits/src/evm_circuit/execution/error_invalid_creation_code.rs index b4052fe5b2d..e076a831bc0 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_invalid_creation_code.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_invalid_creation_code.rs @@ -52,13 +52,8 @@ impl ExecutionGadget for ErrorInvalidCreationCodeGadget { is_first_byte_invalid.expr(), ); - let common_error_gadget = CommonErrorGadget::construct_with_return_data( - cb, - opcode.expr(), - cb.rw_counter_offset(), - memory_address.offset(), - memory_address.length(), - ); + let common_error_gadget = + CommonErrorGadget::construct(cb, opcode.expr(), cb.rw_counter_offset()); Self { opcode, diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_dynamic_memory.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_dynamic_memory.rs new file mode 100644 index 00000000000..a2662ccb782 --- /dev/null +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_dynamic_memory.rs @@ -0,0 +1,221 @@ +use crate::{ + evm_circuit::{ + execution::ExecutionGadget, + param::{N_BYTES_GAS, N_BYTES_MEMORY_ADDRESS}, + step::ExecutionState, + util::{ + common_gadget::CommonErrorGadget, + constraint_builder::{ConstrainBuilderCommon, EVMConstraintBuilder}, + math_gadget::LtGadget, + memory_gadget::{ + CommonMemoryAddressGadget, MemoryExpandedAddressGadget, MemoryExpansionGadget, + }, + CachedRegion, Cell, + }, + }, + witness::{Block, Call, ExecStep, Transaction}, +}; +use eth_types::{evm_types::OpcodeId, Field}; +use gadgets::util::{or, Expr}; +use halo2_proofs::{circuit::Value, plonk::Error}; + +#[derive(Clone, Debug)] +pub(crate) struct ErrorOOGDynamicMemoryGadget { + opcode: Cell, + + memory_expansion: MemoryExpansionGadget, + memory_address: MemoryExpandedAddressGadget, + insufficient_gas: LtGadget, + common_error_gadget: CommonErrorGadget, +} + +impl ExecutionGadget for ErrorOOGDynamicMemoryGadget { + const NAME: &'static str = "ErrorOutOfGasDynamicMemoryExpansion"; + const EXECUTION_STATE: ExecutionState = ExecutionState::ErrorOutOfGasDynamicMemoryExpansion; + + // Support other OOG due to pure memory including RETURN and REVERT + fn configure(cb: &mut EVMConstraintBuilder) -> Self { + let opcode = cb.query_cell(); + + cb.require_in_set( + "ErrorOutOfGasDynamicMemoryExpansion opcode must be RETURN or REVERT", + opcode.expr(), + vec![OpcodeId::RETURN.expr(), OpcodeId::REVERT.expr()], + ); + + let memory_address = MemoryExpandedAddressGadget::construct_self(cb); + cb.stack_pop(memory_address.offset_word()); + cb.stack_pop(memory_address.length_word()); + let memory_expansion = MemoryExpansionGadget::construct(cb, [memory_address.address()]); + + // constant gas of RETURN and REVERT is zero. + let insufficient_gas = LtGadget::construct( + cb, + cb.curr.state.gas_left.expr(), + memory_expansion.gas_cost(), + ); + cb.require_equal( + "Memory address is overflow or gas left is less than required", + or::expr([memory_address.overflow(), insufficient_gas.expr()]), + 1.expr(), + ); + + let common_error_gadget = + CommonErrorGadget::construct(cb, opcode.expr(), cb.rw_counter_offset()); + + Self { + opcode, + memory_address, + memory_expansion, + insufficient_gas, + common_error_gadget, + } + } + + fn assign_exec_step( + &self, + region: &mut CachedRegion<'_, '_, F>, + offset: usize, + block: &Block, + _: &Transaction, + call: &Call, + step: &ExecStep, + ) -> Result<(), Error> { + let opcode = step.opcode().unwrap(); + self.opcode + .assign(region, offset, Value::known(F::from(opcode.as_u64())))?; + + let [memory_offset, length] = [0, 1].map(|idx| block.get_rws(step, idx).stack_value()); + + let expanded_address = self + .memory_address + .assign(region, offset, memory_offset, length)?; + + let (_, memory_expansion_cost) = self.memory_expansion.assign( + region, + offset, + step.memory_word_size(), + [expanded_address], + )?; + + // constant gas of RETURN and REVERT is zero + self.insufficient_gas.assign( + region, + offset, + F::from(step.gas_left), + F::from(memory_expansion_cost), + )?; + + self.common_error_gadget + .assign(region, offset, block, call, step, 4)?; + Ok(()) + } +} + +#[cfg(test)] +mod tests { + use crate::test_util::CircuitTestBuilder; + use eth_types::{bytecode, word, Bytecode, ToWord, U256}; + use mock::{ + eth, test_ctx::helpers::account_0_code_account_1_no_code, TestContext, MOCK_ACCOUNTS, + }; + + #[test] + fn test_oog_dynamic_memory_simple() { + for code in testing_bytecodes(0x40_u64.into(), 0x14.into()).iter() { + test_root(code); + test_internal(code); + } + } + + #[test] + fn test_oog_dynamic_memory_max_expanded_address() { + // 0xffffffff1 + 0xffffffff0 = 0x1fffffffe1 + // > MAX_EXPANDED_MEMORY_ADDRESS (0x1fffffffe0) + for code in testing_bytecodes(0xffffffff1_u64.into(), 0xffffffff0_u64.into()).iter() { + test_root(code); + test_internal(code); + } + } + + #[test] + fn test_oog_dynamic_memory_max_u64_address() { + for code in testing_bytecodes(u64::MAX.into(), u64::MAX.into()).iter() { + test_root(code); + test_internal(code); + } + } + + #[test] + fn test_oog_dynamic_memory_max_word_address() { + for code in testing_bytecodes(U256::MAX, U256::MAX).iter() { + test_root(code); + test_internal(code); + } + } + + fn test_root(code: &Bytecode) { + let ctx = TestContext::<2, 1>::new( + None, + account_0_code_account_1_no_code(code.clone()), + |mut txs, accs| { + txs[0] + .from(accs[1].address) + .to(accs[0].address) + .gas(word!("0x5FFF")); + }, + |block, _tx| block, + ) + .unwrap(); + + CircuitTestBuilder::new_from_test_ctx(ctx).run(); + } + + fn test_internal(code: &Bytecode) { + let code_a = bytecode! { + PUSH1(0x00) // retLength + PUSH1(0x00) // retOffset + PUSH32(0x00) // argsLength + PUSH32(0x00) // argsOffset + PUSH1(0x00) // value + PUSH32(MOCK_ACCOUNTS[1].to_word()) // addr + PUSH32(0xFFFF) // gas + CALL + STOP + }; + + let ctx = TestContext::<3, 1>::new( + None, + |accs| { + accs[0].address(MOCK_ACCOUNTS[0]).code(code_a); + accs[1].address(MOCK_ACCOUNTS[1]).code(code.clone()); + accs[2].address(MOCK_ACCOUNTS[2]).balance(eth(1)); + }, + |mut txs, accs| { + txs[0] + .from(accs[2].address) + .to(accs[0].address) + .gas(word!("0xFFFFF")); + }, + |block, _tx| block, + ) + .unwrap(); + + CircuitTestBuilder::new_from_test_ctx(ctx).run(); + } + + fn testing_bytecodes(offset: U256, size: U256) -> Vec { + vec![ + bytecode! { + PUSH32(size) + PUSH32(offset) + RETURN + }, + bytecode! { + PUSH32(size) + PUSH32(offset) + REVERT + }, + ] + } +}