From f9361ece943a9a53a90bd435402c72de2b0c2a83 Mon Sep 17 00:00:00 2001 From: Steven Date: Mon, 10 Apr 2023 16:57:47 +0800 Subject: [PATCH] Implement circuit for `ErrorGasUintOverflow` state. (#442) --- .../src/circuit_input_builder/tracer_tests.rs | 4 +- bus-mapping/src/error.rs | 7 +- bus-mapping/src/evm/opcodes.rs | 1 - mock/src/lib.rs | 3 + zkevm-circuits/src/evm_circuit/execution.rs | 5 - .../src/evm_circuit/execution/calldatacopy.rs | 5 +- .../src/evm_circuit/execution/callop.rs | 18 +- .../src/evm_circuit/execution/codecopy.rs | 5 +- .../src/evm_circuit/execution/create.rs | 4 +- .../evm_circuit/execution/error_code_store.rs | 7 +- .../execution/error_invalid_creation_code.rs | 7 +- .../evm_circuit/execution/error_oog_call.rs | 73 +++- .../execution/error_oog_create2.rs | 196 ++++++----- .../execution/error_oog_dynamic_memory.rs | 198 +++++------ .../evm_circuit/execution/error_oog_log.rs | 219 ++++++------ .../execution/error_oog_memory_copy.rs | 139 ++++++-- .../evm_circuit/execution/error_oog_sha3.rs | 32 +- .../execution/error_oog_static_memory.rs | 172 +++++----- .../execution/error_precompile_failed.rs | 6 +- .../src/evm_circuit/execution/extcodecopy.rs | 5 +- .../src/evm_circuit/execution/logs.rs | 4 +- .../evm_circuit/execution/return_revert.rs | 4 +- .../evm_circuit/execution/returndatacopy.rs | 5 +- .../src/evm_circuit/execution/sha3.rs | 5 +- zkevm-circuits/src/evm_circuit/step.rs | 2 - .../src/evm_circuit/util/common_gadget.rs | 29 +- .../src/evm_circuit/util/memory_gadget.rs | 311 +++++++++++------- zkevm-circuits/src/witness/step.rs | 1 - 28 files changed, 814 insertions(+), 653 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder/tracer_tests.rs b/bus-mapping/src/circuit_input_builder/tracer_tests.rs index 6c831426f8..74e96159be 100644 --- a/bus-mapping/src/circuit_input_builder/tracer_tests.rs +++ b/bus-mapping/src/circuit_input_builder/tracer_tests.rs @@ -1,7 +1,7 @@ use super::*; use crate::{ circuit_input_builder::access::gen_state_access_trace, - error::{ExecError, InsufficientBalanceError}, + error::{ExecError, InsufficientBalanceError, OogError}, geth_errors::{ GETH_ERR_GAS_UINT_OVERFLOW, GETH_ERR_OUT_OF_GAS, GETH_ERR_STACK_OVERFLOW, GETH_ERR_STACK_UNDERFLOW, @@ -1411,7 +1411,7 @@ fn tracer_err_gas_uint_overflow() { let mut builder = CircuitInputBuilderTx::new(&block, step); assert_eq!( builder.state_ref().get_step_err(step, next_step).unwrap(), - Some(ExecError::GasUintOverflow) + Some(ExecError::OutOfGas(OogError::StaticMemoryExpansion)) ); } diff --git a/bus-mapping/src/error.rs b/bus-mapping/src/error.rs index ede1cc69cd..9f65371f9f 100644 --- a/bus-mapping/src/error.rs +++ b/bus-mapping/src/error.rs @@ -143,17 +143,12 @@ pub enum ExecError { /// For CALL, CALLCODE, DELEGATECALL, STATICCALL PrecompileFailed, /// .. - GasUintOverflow, - /// .. NonceUintOverflow, } // TODO: Move to impl block. pub(crate) fn get_step_reported_error(op: &OpcodeId, error: &str) -> ExecError { - if error == GETH_ERR_GAS_UINT_OVERFLOW { - return ExecError::GasUintOverflow; - } - if error == GETH_ERR_OUT_OF_GAS { + if [GETH_ERR_OUT_OF_GAS, GETH_ERR_GAS_UINT_OVERFLOW].contains(&error) { // NOTE: We report a GasUintOverflow error as an OutOfGas error let oog_err = match op { OpcodeId::MLOAD | OpcodeId::MSTORE | OpcodeId::MSTORE8 => { diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index bd4e7edba7..204dd811f3 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -337,7 +337,6 @@ fn fn_gen_error_state_associated_ops( OpcodeId::CREATE2 => Some(StackOnlyOpcode::<4, 1>::gen_associated_ops), _ => unreachable!(), }, - ExecError::GasUintOverflow => Some(StackOnlyOpcode::<0, 0, true>::gen_associated_ops), ExecError::InvalidCreationCode => Some(ErrorCreationCode::gen_associated_ops), // more future errors place here _ => { diff --git a/mock/src/lib.rs b/mock/src/lib.rs index 3a3c176e8c..1b94ee75fb 100644 --- a/mock/src/lib.rs +++ b/mock/src/lib.rs @@ -15,6 +15,9 @@ pub(crate) use block::MockBlock; pub use test_ctx::TestContext; pub use transaction::{AddrOrWallet, MockTransaction, CORRECT_MOCK_TXS}; +/// Mock block gas limit +pub const MOCK_BLOCK_GAS_LIMIT: u64 = 10_000_000_000_000_000; + lazy_static! { /// Mock 1 ETH pub static ref MOCK_1_ETH: Word = eth(1); diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index 0840b0aa8f..c2526ed2be 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -340,7 +340,6 @@ pub(crate) struct ExecutionConfig { error_invalid_creation_code: Box>, error_precompile_failed: Box>, error_return_data_out_of_bound: Box>, - error_gas_uint_overflow: Box>, } impl ExecutionConfig { @@ -603,7 +602,6 @@ impl ExecutionConfig { error_invalid_creation_code: configure_gadget!(), error_return_data_out_of_bound: configure_gadget!(), error_precompile_failed: configure_gadget!(), - error_gas_uint_overflow: configure_gadget!(), // step and presets step: step_curr, height_map, @@ -1448,9 +1446,6 @@ impl ExecutionConfig { ExecutionState::ErrorPrecompileFailed => { assign_exec_step!(self.error_precompile_failed) } - ExecutionState::ErrorGasUintOverflow => { - assign_exec_step!(self.error_gas_uint_overflow) - } } // Fill in the witness values for stored expressions diff --git a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs index 703f689e5b..42ed45716e 100644 --- a/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/calldatacopy.rs @@ -9,7 +9,10 @@ use crate::{ ConstraintBuilder, StepStateTransition, Transition::{Delta, To}, }, - memory_gadget::{MemoryAddressGadget, MemoryCopierGasGadget, MemoryExpansionGadget}, + memory_gadget::{ + CommonMemoryAddressGadget, MemoryAddressGadget, MemoryCopierGasGadget, + MemoryExpansionGadget, + }, not, select, CachedRegion, Cell, }, witness::{Block, Call, ExecStep, Transaction}, diff --git a/zkevm-circuits/src/evm_circuit/execution/callop.rs b/zkevm-circuits/src/evm_circuit/execution/callop.rs index b52c3f5036..93f4c40e73 100644 --- a/zkevm-circuits/src/evm_circuit/execution/callop.rs +++ b/zkevm-circuits/src/evm_circuit/execution/callop.rs @@ -13,6 +13,7 @@ use crate::{ math_gadget::{ ConstantDivisionGadget, IsZeroGadget, LtGadget, LtWordGadget, MinMaxGadget, }, + memory_gadget::{CommonMemoryAddressGadget, MemoryAddressGadget}, not, or, select, CachedRegion, Cell, Word, }, witness::{Block, Call, ExecStep, Transaction}, @@ -43,7 +44,7 @@ pub(crate) struct CallOpGadget { current_caller_address: Cell, is_static: Cell, depth: Cell, - call: CommonCallGadget, + call: CommonCallGadget, true>, current_value: Word, is_warm: Cell, is_warm_prev: Cell, @@ -99,13 +100,14 @@ impl ExecutionGadget for CallOpGadget { ) }); - let call_gadget = CommonCallGadget::construct( - cb, - is_call.expr(), - is_callcode.expr(), - is_delegatecall.expr(), - is_staticcall.expr(), - ); + let call_gadget: CommonCallGadget, true> = + CommonCallGadget::construct( + cb, + is_call.expr(), + is_callcode.expr(), + is_delegatecall.expr(), + is_staticcall.expr(), + ); cb.condition(not::expr(is_call.expr() + is_callcode.expr()), |cb| { cb.require_zero( "for non call/call code, value is zero", diff --git a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs index bddc32056f..8a888d93f7 100644 --- a/zkevm-circuits/src/evm_circuit/execution/codecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/codecopy.rs @@ -9,7 +9,10 @@ use crate::{ util::{ common_gadget::{SameContextGadget, WordByteCapGadget}, constraint_builder::{ConstraintBuilder, StepStateTransition, Transition}, - memory_gadget::{MemoryAddressGadget, MemoryCopierGasGadget, MemoryExpansionGadget}, + memory_gadget::{ + CommonMemoryAddressGadget, MemoryAddressGadget, MemoryCopierGasGadget, + MemoryExpansionGadget, + }, not, select, CachedRegion, Cell, }, witness::{Block, Call, ExecStep, Transaction}, diff --git a/zkevm-circuits/src/evm_circuit/execution/create.rs b/zkevm-circuits/src/evm_circuit/execution/create.rs index 32a383e256..566f1ec3ed 100644 --- a/zkevm-circuits/src/evm_circuit/execution/create.rs +++ b/zkevm-circuits/src/evm_circuit/execution/create.rs @@ -15,7 +15,9 @@ use crate::{ math_gadget::{ ConstantDivisionGadget, ContractCreateGadget, IsZeroGadget, LtWordGadget, }, - memory_gadget::{MemoryAddressGadget, MemoryExpansionGadget}, + memory_gadget::{ + CommonMemoryAddressGadget, MemoryAddressGadget, MemoryExpansionGadget, + }, not, select, CachedRegion, Cell, Word, }, witness::{Block, Call, ExecStep, Transaction}, 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 412e37ab90..ae1a128ec0 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs @@ -4,8 +4,11 @@ use crate::{ param::{N_BYTES_GAS, N_BYTES_U64}, step::ExecutionState, util::{ - common_gadget::CommonErrorGadget, constraint_builder::ConstraintBuilder, - math_gadget::LtGadget, memory_gadget::MemoryAddressGadget, CachedRegion, Cell, + common_gadget::CommonErrorGadget, + constraint_builder::ConstraintBuilder, + math_gadget::LtGadget, + memory_gadget::{CommonMemoryAddressGadget, MemoryAddressGadget}, + CachedRegion, Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, 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 282c46852e..d824f2682b 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 @@ -3,8 +3,11 @@ use crate::{ execution::ExecutionGadget, step::ExecutionState, util::{ - common_gadget::CommonErrorGadget, constraint_builder::ConstraintBuilder, - math_gadget::IsEqualGadget, memory_gadget::MemoryAddressGadget, CachedRegion, Cell, + common_gadget::CommonErrorGadget, + constraint_builder::ConstraintBuilder, + math_gadget::IsEqualGadget, + memory_gadget::{CommonMemoryAddressGadget, MemoryAddressGadget}, + CachedRegion, Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs index 8aaaf7cb80..7e1cdbc934 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_call.rs @@ -7,7 +7,8 @@ use crate::{ common_gadget::{CommonCallGadget, CommonErrorGadget}, constraint_builder::ConstraintBuilder, math_gadget::{IsZeroGadget, LtGadget}, - CachedRegion, Cell, + memory_gadget::MemoryExpandedAddressGadget, + or, CachedRegion, Cell, }, }, table::CallContextFieldTag, @@ -30,8 +31,8 @@ pub(crate) struct ErrorOOGCallGadget { is_staticcall: IsZeroGadget, tx_id: Cell, is_static: Cell, - call: CommonCallGadget, is_warm: Cell, + call: CommonCallGadget, false>, insufficient_gas: LtGadget, common_error_gadget: CommonErrorGadget, } @@ -54,13 +55,14 @@ impl ExecutionGadget for ErrorOOGCallGadget { let tx_id = cb.call_context(None, CallContextFieldTag::TxId); let is_static = cb.call_context(None, CallContextFieldTag::IsStatic); - let call_gadget = CommonCallGadget::construct( - cb, - is_call.expr(), - is_callcode.expr(), - is_delegatecall.expr(), - is_staticcall.expr(), - ); + let call_gadget: CommonCallGadget, false> = + CommonCallGadget::construct( + cb, + is_call.expr(), + is_callcode.expr(), + is_delegatecall.expr(), + is_staticcall.expr(), + ); // Add callee to access list let is_warm = cb.query_bool(); @@ -82,9 +84,14 @@ impl ExecutionGadget for ErrorOOGCallGadget { // Check if the amount of gas available is less than the amount of gas required let insufficient_gas = LtGadget::construct(cb, cb.curr.state.gas_left.expr(), gas_cost); + cb.require_equal( - "gas left is less than gas required", - insufficient_gas.expr(), + "Either Memory address is overflow or gas left is less than cost", + or::expr([ + call_gadget.cd_address.overflow(), + call_gadget.rd_address.overflow(), + insufficient_gas.expr(), + ]), 1.expr(), ); @@ -104,8 +111,8 @@ impl ExecutionGadget for ErrorOOGCallGadget { is_staticcall, tx_id, is_static, - call: call_gadget, is_warm, + call: call_gadget, insufficient_gas, common_error_gadget, } @@ -401,4 +408,46 @@ mod test { }); test_oog(&caller(OpcodeId::CALL, stack), &callee, true); } + + #[test] + fn test_oog_call_max_expanded_address() { + // 0xffffffff1 + 0xffffffff0 = 0x1fffffffe1 + // > MAX_EXPANDED_MEMORY_ADDRESS (0x1fffffffe0) + let stack = Stack { + gas: Word::MAX, + cd_offset: 0xffffffff1, + cd_length: 0xffffffff0, + rd_offset: 0xffffffff1, + rd_length: 0xffffffff0, + ..Default::default() + }; + let callee = callee(bytecode! { + PUSH32(Word::from(0)) + PUSH32(Word::from(0)) + STOP + }); + for opcode in TEST_CALL_OPCODES { + test_oog(&caller(*opcode, stack), &callee, true); + } + } + + #[test] + fn test_oog_call_max_u64_address() { + let stack = Stack { + gas: Word::MAX, + cd_offset: u64::MAX, + cd_length: u64::MAX, + rd_offset: u64::MAX, + rd_length: u64::MAX, + ..Default::default() + }; + let callee = callee(bytecode! { + PUSH32(Word::from(0)) + PUSH32(Word::from(0)) + STOP + }); + for opcode in TEST_CALL_OPCODES { + test_oog(&caller(*opcode, stack), &callee, true); + } + } } diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_create2.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_create2.rs index eaa6bb298c..b8c7d4e99d 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_create2.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_create2.rs @@ -1,17 +1,17 @@ use crate::{ evm_circuit::{ execution::ExecutionGadget, - param::{N_BYTES_GAS, N_BYTES_MEMORY_ADDRESS}, + param::{N_BYTES_GAS, N_BYTES_MEMORY_WORD_SIZE}, step::ExecutionState, util::{ - and, common_gadget::CommonErrorGadget, constraint_builder::ConstraintBuilder, - math_gadget::{IsZeroGadget, LtGadget}, + math_gadget::LtGadget, memory_gadget::{ - address_high, address_low, MemoryExpansionGadget, MemoryWordSizeGadget, + CommonMemoryAddressGadget, MemoryExpandedAddressGadget, MemoryExpansionGadget, + MemoryWordSizeGadget, }, - CachedRegion, Cell, Word, + or, CachedRegion, Cell, Word, }, }, witness::{Block, Call, ExecStep, Transaction}, @@ -24,18 +24,11 @@ use halo2_proofs::{circuit::Value, plonk::Error}; pub(crate) struct ErrorOOGCreate2Gadget { opcode: Cell, value: Word, - address: Word, - size: Word, salt: Word, - address_in_range_high: IsZeroGadget, - size_in_range_high: IsZeroGadget, - expanded_address_in_range: LtGadget, minimum_word_size: MemoryWordSizeGadget, - memory_expansion: MemoryExpansionGadget, + memory_address: MemoryExpandedAddressGadget, + memory_expansion: MemoryExpansionGadget, insufficient_gas: LtGadget, - // gupeng - // rw_counter_end_of_reversion: Cell, - // restore_context: RestoreContextGadget, common_error_gadget: CommonErrorGadget, } @@ -54,41 +47,18 @@ impl ExecutionGadget for ErrorOOGCreate2Gadget { ); let value = cb.query_word_rlc(); - cb.stack_pop(value.expr()); - let address = cb.query_word_rlc(); - cb.stack_pop(address.expr()); - let size = cb.query_word_rlc(); - cb.stack_pop(size.expr()); let salt = cb.query_word_rlc(); - cb.stack_pop(salt.expr()); - let address_high = address_high::expr(&address); - let address_in_range_high = IsZeroGadget::construct(cb, address_high); - let size_high = address_high::expr(&size); - let size_in_range_high = IsZeroGadget::construct(cb, size_high); - let address_low = address_low::expr(&address); - let size_low = address_low::expr(&size); - let expanded_address = address_low.expr() + size_low.expr(); - let expanded_address_in_range = LtGadget::construct( - cb, - expanded_address.expr(), - (1u64 << (N_BYTES_MEMORY_ADDRESS * 8)).expr(), - ); + let memory_address = MemoryExpandedAddressGadget::construct_self(cb); - cb.require_equal( - "address and size must less than 5 bytes", - and::expr([ - address_in_range_high.expr(), - size_in_range_high.expr(), - expanded_address_in_range.expr(), - ]), - true.expr(), - ); + cb.stack_pop(value.expr()); + cb.stack_pop(memory_address.offset_rlc()); + cb.stack_pop(memory_address.length_rlc()); + cb.stack_pop(salt.expr()); - let minimum_word_size = MemoryWordSizeGadget::construct(cb, size_low.expr()); + let minimum_word_size = MemoryWordSizeGadget::construct(cb, memory_address.length()); - let memory_expansion = - MemoryExpansionGadget::construct(cb, [address_low::expr(&address) + size_low.expr()]); + let memory_expansion = MemoryExpansionGadget::construct(cb, [memory_address.address()]); let insufficient_gas = LtGadget::construct( cb, @@ -99,9 +69,9 @@ impl ExecutionGadget for ErrorOOGCreate2Gadget { ); cb.require_equal( - "gas_left must less than gas_cost", - insufficient_gas.expr(), - true.expr(), + "Memory address is overflow or gas left is less than cost", + or::expr([memory_address.overflow(), insufficient_gas.expr()]), + 1.expr(), ); let common_error_gadget = CommonErrorGadget::construct(cb, opcode.expr(), 6.expr()); @@ -109,13 +79,9 @@ impl ExecutionGadget for ErrorOOGCreate2Gadget { Self { opcode, value, - address, - size, salt, - address_in_range_high, - size_in_range_high, - expanded_address_in_range, minimum_word_size, + memory_address, memory_expansion, insufficient_gas, common_error_gadget, @@ -142,11 +108,12 @@ impl ExecutionGadget for ErrorOOGCreate2Gadget { Some(block.rws[step.rw_indices[0]].stack_value().to_le_bytes()), )?; - let address = block.rws[step.rw_indices[1]].stack_value(); - let size = block.rws[step.rw_indices[2]].stack_value(); - self.address - .assign(region, offset, Some(address.to_le_bytes()))?; - self.size.assign(region, offset, Some(size.to_le_bytes()))?; + let memory_offset = block.rws[step.rw_indices[1]].stack_value(); + let memory_length = block.rws[step.rw_indices[2]].stack_value(); + + let memory_address = + self.memory_address + .assign(region, offset, memory_offset, memory_length)?; self.salt.assign( region, @@ -154,45 +121,14 @@ impl ExecutionGadget for ErrorOOGCreate2Gadget { Some(block.rws[step.rw_indices[3]].stack_value().to_le_bytes()), )?; - let address_high = address_high::value::(address.to_le_bytes()); - assert_eq!( - address_high, - F::zero(), - "address overflow {} bytes", - N_BYTES_MEMORY_ADDRESS - ); - self.address_in_range_high - .assign(region, offset, address_high)?; - let size_high = address_high::value::(size.to_le_bytes()); - assert_eq!( - size_high, - F::zero(), - "size overflow {} bytes", - N_BYTES_MEMORY_ADDRESS - ); - self.size_in_range_high.assign(region, offset, size_high)?; - - let address_value = address_low::value(address.to_le_bytes()); - let size_value = address_low::value(size.to_le_bytes()); - let expanded_address = address_value - .checked_add(size_value) - .expect("address overflow u64"); - assert!( - expanded_address < (1u64 << (N_BYTES_MEMORY_ADDRESS * 8)), - "expanded address overflow {} bytes", - N_BYTES_MEMORY_ADDRESS - ); - self.expanded_address_in_range.assign( + let minimum_word_size = self.minimum_word_size.assign( region, offset, - F::from(expanded_address), - F::from(1u64 << (N_BYTES_MEMORY_ADDRESS * 8)), + MemoryExpandedAddressGadget::::length_value(memory_offset, memory_length), )?; - - let minimum_word_size = self.minimum_word_size.assign(region, offset, size_value)?; let memory_expansion_gas = self .memory_expansion - .assign(region, offset, step.memory_word_size(), [expanded_address])? + .assign(region, offset, step.memory_word_size(), [memory_address])? .1; let constant_gas_cost = opcode.constant_gas_cost().0; @@ -214,11 +150,11 @@ impl ExecutionGadget for ErrorOOGCreate2Gadget { #[cfg(test)] mod tests { use crate::test_util::CircuitTestBuilder; - use eth_types::{bytecode, word, Bytecode, ToWord, Word}; + use eth_types::{bytecode, word, Bytecode, ToWord, Word, U256}; use mock::{ eth, test_ctx::{helpers::account_0_code_account_1_no_code, LoggerConfig}, - TestContext, MOCK_ACCOUNTS, + TestContext, MOCK_ACCOUNTS, MOCK_BLOCK_GAS_LIMIT, }; struct TestCase { @@ -227,13 +163,12 @@ mod tests { } #[test] - fn test_oog_create2() { + fn test_oog_create2_simple() { let cases = [ - // memory expansion TestCase { bytecode: bytecode! { - PUSH8(0x0) - PUSH8(0xFF) + PUSH8(0x0) // salt + PUSH8(0xFF) // size PUSH32(word!("0xffffffff")) // offset PUSH8(0x0) // value CREATE2 @@ -241,13 +176,12 @@ mod tests { }, gas: word!("0xFFFF"), }, - // simple TestCase { bytecode: bytecode! { - PUSH1(2) - PUSH1(4) - PUSH1(0x0) - PUSH1(0x0) + PUSH1(2) // salt + PUSH1(4) // size + PUSH1(0x0) // offset + PUSH1(0x0) // value CREATE2 }, gas: word!("0x7D0F"), @@ -260,6 +194,62 @@ mod tests { } } + #[test] + fn test_oog_create2_max_expanded_address() { + // 0xffffffff1 + 0xffffffff0 = 0x1fffffffe1 + // > MAX_EXPANDED_MEMORY_ADDRESS (0x1fffffffe0) + let case = TestCase { + bytecode: bytecode! { + PUSH8(0) // salt + PUSH32(0xffffffff0_u64) // size + PUSH32(0xffffffff1_u64) // offset + PUSH8(0) // value + CREATE2 + STOP + }, + gas: MOCK_BLOCK_GAS_LIMIT.into(), + }; + + test_root(&case); + test_internal(&case); + } + + #[test] + fn test_oog_create2_max_u64_address() { + let case = TestCase { + bytecode: bytecode! { + PUSH8(0) // salt + PUSH32(u64::MAX) // size + PUSH32(u64::MAX) // offset + PUSH8(0) // value + CREATE2 + STOP + }, + gas: MOCK_BLOCK_GAS_LIMIT.into(), + }; + + test_root(&case); + test_internal(&case); + } + + #[test] + fn test_oog_create2_max_word_address() { + let case = TestCase { + bytecode: bytecode! { + PUSH8(0) // salt + PUSH32(U256::MAX) // size + PUSH32(U256::MAX) // offset + PUSH8(0) // value + CREATE2 + STOP + }, + gas: MOCK_BLOCK_GAS_LIMIT.into(), + }; + + test_root(&case); + test_internal(&case); + } + fn test_root(case: &TestCase) { let ctx = TestContext::<2, 1>::new_with_logger_config( None, @@ -278,7 +268,7 @@ mod tests { ) .unwrap(); - println!("{:?}", ctx.geth_traces[0]); + log::debug!("{:?}", ctx.geth_traces[0]); CircuitTestBuilder::new_from_test_ctx(ctx).run(); } 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 index 48f02c1e44..585e9b31bd 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_dynamic_memory.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_dynamic_memory.rs @@ -1,7 +1,7 @@ use crate::{ evm_circuit::{ execution::ExecutionGadget, - param::{N_BYTES_GAS, N_BYTES_MEMORY_ADDRESS}, + param::{N_BYTES_GAS, N_BYTES_MEMORY_WORD_SIZE}, step::ExecutionState, util::{ common_gadget::RestoreContextGadget, @@ -9,16 +9,18 @@ use crate::{ ConstraintBuilder, StepStateTransition, Transition::{Delta, Same}, }, - math_gadget::{IsEqualGadget, IsZeroGadget, LtGadget}, - memory_gadget::{address_high, address_low, MemoryExpansionGadget}, - CachedRegion, Cell, Word, + math_gadget::{IsEqualGadget, LtGadget}, + memory_gadget::{ + CommonMemoryAddressGadget, MemoryExpandedAddressGadget, MemoryExpansionGadget, + }, + or, CachedRegion, Cell, Word, }, }, table::CallContextFieldTag, witness::{Block, Call, ExecStep, Transaction}, }; use eth_types::{evm_types::OpcodeId, Field, ToLittleEndian}; -use gadgets::util::{and, not, select, Expr}; +use gadgets::util::{not, select, Expr}; use halo2_proofs::{circuit::Value, plonk::Error}; #[derive(Clone, Debug)] @@ -27,12 +29,8 @@ pub(crate) struct ErrorOOGDynamicMemoryGadget { is_create: IsEqualGadget, is_return: IsEqualGadget, value: Word, - address: Word, - size: Word, - address_in_range_high: IsZeroGadget, - size_in_range_high: IsZeroGadget, - expanded_address_in_range: LtGadget, - memory_expansion: MemoryExpansionGadget, + memory_address: MemoryExpandedAddressGadget, + memory_expansion: MemoryExpansionGadget, insufficient_gas: LtGadget, rw_counter_end_of_reversion: Cell, restore_context: RestoreContextGadget, @@ -64,42 +62,17 @@ impl ExecutionGadget for ErrorOOGDynamicMemoryGadget { ), ); + let memory_address = MemoryExpandedAddressGadget::construct_self(cb); + let value = cb.query_word_rlc(); cb.condition(is_create.expr(), |cb| { cb.stack_pop(value.expr()); }); - let address = cb.query_word_rlc(); - cb.stack_pop(address.expr()); - let size = cb.query_word_rlc(); - cb.stack_pop(size.expr()); - - let address_high = address_high::expr(&address); - let address_in_range_high = IsZeroGadget::construct(cb, address_high); - let size_high = address_high::expr(&size); - let size_in_range_high = IsZeroGadget::construct(cb, size_high); - - let address_low = address_low::expr(&address); - let size_low = address_low::expr(&size); - let expanded_address = address_low.expr() + size_low.expr(); - let expanded_address_in_range = LtGadget::construct( - cb, - expanded_address.expr(), - (1u64 << (N_BYTES_MEMORY_ADDRESS * 8)).expr(), - ); - - cb.require_equal( - "address and size must less than 5 bytes", - and::expr([ - address_in_range_high.expr(), - size_in_range_high.expr(), - expanded_address_in_range.expr(), - ]), - true.expr(), - ); + cb.stack_pop(memory_address.offset_rlc()); + cb.stack_pop(memory_address.length_rlc()); - let memory_expansion = - MemoryExpansionGadget::construct(cb, [address_low::expr(&address) + size_low.expr()]); + let memory_expansion = MemoryExpansionGadget::construct(cb, [memory_address.address()]); let insufficient_gas = LtGadget::construct( cb, @@ -116,9 +89,9 @@ impl ExecutionGadget for ErrorOOGDynamicMemoryGadget { ); cb.require_equal( - "gas_left must less than gas_cost", - insufficient_gas.expr(), - true.expr(), + "Memory address is overflow or gas left is less than cost", + or::expr([memory_address.overflow(), insufficient_gas.expr()]), + 1.expr(), ); // Current call must fail. @@ -158,11 +131,7 @@ impl ExecutionGadget for ErrorOOGDynamicMemoryGadget { is_create, is_return, value, - address, - size, - address_in_range_high, - size_in_range_high, - expanded_address_in_range, + memory_address, memory_expansion, insufficient_gas, rw_counter_end_of_reversion, @@ -203,50 +172,15 @@ impl ExecutionGadget for ErrorOOGDynamicMemoryGadget { .assign(region, offset, Some(value.to_le_bytes()))?; rw_offset += 1; } - let address = block.rws[step.rw_indices[rw_offset]].stack_value(); - let size = block.rws[step.rw_indices[rw_offset + 1]].stack_value(); - self.address - .assign(region, offset, Some(address.to_le_bytes()))?; - self.size.assign(region, offset, Some(size.to_le_bytes()))?; - - let address_high = address_high::value::(address.to_le_bytes()); - assert_eq!( - address_high, - F::zero(), - "address overflow {} bytes", - N_BYTES_MEMORY_ADDRESS - ); - self.address_in_range_high - .assign(region, offset, address_high)?; - let size_high = address_high::value::(size.to_le_bytes()); - assert_eq!( - size_high, - F::zero(), - "size overflow {} bytes", - N_BYTES_MEMORY_ADDRESS - ); - self.size_in_range_high.assign(region, offset, size_high)?; - - let address_value = address_low::value(address.to_le_bytes()); - let size_value = address_low::value(size.to_le_bytes()); - let expanded_address = address_value - .checked_add(size_value) - .expect("address overflow u64"); - assert!( - expanded_address < (1u64 << (N_BYTES_MEMORY_ADDRESS * 8)), - "expanded address overflow {} bytes", - N_BYTES_MEMORY_ADDRESS - ); - self.expanded_address_in_range.assign( - region, - offset, - F::from(expanded_address), - F::from(1u64 << (N_BYTES_MEMORY_ADDRESS * 8)), - )?; + let memory_offset = block.rws[step.rw_indices[rw_offset]].stack_value(); + let memory_length = block.rws[step.rw_indices[rw_offset + 1]].stack_value(); + let memory_address = + self.memory_address + .assign(region, offset, memory_offset, memory_length)?; let memory_expansion_gas = self .memory_expansion - .assign(region, offset, step.memory_word_size(), [expanded_address])? + .assign(region, offset, step.memory_word_size(), [memory_address])? .1; let constant_gas_cost = opcode.constant_gas_cost().0; @@ -278,43 +212,49 @@ impl ExecutionGadget for ErrorOOGDynamicMemoryGadget { #[cfg(test)] mod tests { use crate::test_util::CircuitTestBuilder; - use eth_types::{bytecode, word, Bytecode, ToWord}; + 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() { - let codes = [ - bytecode! { - PUSH8(0xFF) - PUSH32(word!("0xffffffff")) // offset - RETURN - }, - bytecode! { - PUSH8(0xFF) - PUSH32(word!("0xffffffff")) // offset - REVERT - }, - bytecode! { - PUSH8(0xFF) - PUSH32(word!("0xffffffff")) // offset - PUSH8(0x0) // value - CREATE - STOP - }, - ]; + fn test_oog_dynamic_memory_simple() { + for code in testing_bytecodes(0xffffffff_u64.into(), 0xff.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); + } + } - for code in codes.iter() { - test_root(code.clone()); - test_internal(code.clone()); + #[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) { + fn test_root(code: &Bytecode) { let ctx = TestContext::<2, 1>::new( None, - account_0_code_account_1_no_code(code), + account_0_code_account_1_no_code(code.clone()), |mut txs, accs| { txs[0] .from(accs[1].address) @@ -328,7 +268,7 @@ mod tests { CircuitTestBuilder::new_from_test_ctx(ctx).run(); } - fn test_internal(code: Bytecode) { + fn test_internal(code: &Bytecode) { let code_a = bytecode! { PUSH1(0x00) // retLength PUSH1(0x00) // retOffset @@ -345,7 +285,7 @@ mod tests { None, |accs| { accs[0].address(MOCK_ACCOUNTS[0]).code(code_a); - accs[1].address(MOCK_ACCOUNTS[1]).code(code); + accs[1].address(MOCK_ACCOUNTS[1]).code(code.clone()); accs[2].address(MOCK_ACCOUNTS[2]).balance(eth(1)); }, |mut txs, accs| { @@ -360,4 +300,26 @@ mod tests { 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 + }, + bytecode! { + PUSH32(size) + PUSH32(offset) + PUSH8(0) // value + CREATE + STOP + }, + ] + } } diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_log.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_log.rs index 41b3695c9e..8679f6865f 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_log.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_log.rs @@ -7,8 +7,10 @@ use crate::{ common_gadget::CommonErrorGadget, constraint_builder::ConstraintBuilder, math_gadget::LtGadget, - memory_gadget::{MemoryAddressGadget, MemoryExpansionGadget}, - CachedRegion, Cell, + memory_gadget::{ + CommonMemoryAddressGadget, MemoryExpandedAddressGadget, MemoryExpansionGadget, + }, + or, CachedRegion, Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -25,7 +27,7 @@ use halo2_proofs::{circuit::Value, plonk::Error}; pub(crate) struct ErrorOOGLogGadget { opcode: Cell, // memory address - memory_address: MemoryAddressGadget, + memory_address: MemoryExpandedAddressGadget, is_static_call: Cell, is_opcode_logn: LtGadget, // constrain gas left is less than gas cost @@ -41,12 +43,12 @@ impl ExecutionGadget for ErrorOOGLogGadget { fn configure(cb: &mut ConstraintBuilder) -> Self { let opcode = cb.query_cell(); - let mstart = cb.query_cell_phase2(); - let msize = cb.query_word_rlc(); + + let memory_address = MemoryExpandedAddressGadget::construct_self(cb); // Pop mstart_address, msize from stack - cb.stack_pop(mstart.expr()); - cb.stack_pop(msize.expr()); + cb.stack_pop(memory_address.offset_rlc()); + cb.stack_pop(memory_address.length_rlc()); // constrain not in static call let is_static_call = cb.call_context(None, CallContextFieldTag::IsStatic); @@ -60,9 +62,6 @@ impl ExecutionGadget for ErrorOOGLogGadget { 1.expr(), ); - // check memory - let memory_address = MemoryAddressGadget::construct(cb, mstart, msize); - // Calculate the next memory size and the gas cost for this memory // access let memory_expansion = MemoryExpansionGadget::construct(cb, [memory_address.address()]); @@ -75,9 +74,10 @@ impl ExecutionGadget for ErrorOOGLogGadget { // Check if the amount of gas available is less than the amount of gas // required let insufficient_gas = LtGadget::construct(cb, cb.curr.state.gas_left.expr(), gas_cost); + cb.require_equal( - "gas left is less than gas required ", - insufficient_gas.expr(), + "Memory address is overflow or gas left is less than cost", + or::expr([memory_address.overflow(), insufficient_gas.expr()]), 1.expr(), ); @@ -130,7 +130,7 @@ impl ExecutionGadget for ErrorOOGLogGadget { let gas_cost = GasCost::LOG.as_u64() + GasCost::LOG.as_u64() * topic_count - + 8 * msize.low_u64() + + 8 * MemoryExpandedAddressGadget::::length_value(memory_start, msize) + memory_expansion_cost; // Gas insufficient check self.insufficient_gas @@ -144,33 +144,69 @@ impl ExecutionGadget for ErrorOOGLogGadget { #[cfg(test)] mod test { use crate::test_util::CircuitTestBuilder; - use bus_mapping::evm::OpcodeId; use eth_types::{ - self, address, bytecode, bytecode::Bytecode, evm_types::GasCost, geth_types::Account, - Address, ToWord, Word, + address, bytecode, bytecode::Bytecode, evm_types::GasCost, geth_types::Account, Address, + ToWord, Transaction, Word, U256, }; - use mock::{ eth, gwei, test_ctx::helpers::account_0_code_account_1_no_code, TestContext, MOCK_ACCOUNTS, }; - fn gas(call_data: &[u8]) -> Word { - Word::from( - GasCost::TX.as_u64() - + 2 * OpcodeId::PUSH32.constant_gas_cost().as_u64() - + call_data - .iter() - .map(|&x| if x == 0 { 4 } else { 16 }) - .sum::(), - ) + #[test] + fn test_oog_log_root_simple() { + test_root(100.into(), 0.into()); } - fn test_oog_log(tx: eth_types::Transaction) { + #[test] + fn test_oog_log_internal_simple() { + let bytecode = bytecode! { + PUSH32(Word::from(0)) + PUSH32(Word::from(10)) + PUSH32(Word::from(224)) + PUSH32(Word::from(1025)) + PUSH32(Word::from(5089)) + LOG2 + STOP + }; + let callee = callee(bytecode); + test_internal(caller(), callee); + } + + #[test] + fn test_oog_log_max_expanded_address() { + // 0xffffffff1 + 0xffffffff0 = 0x1fffffffe1 + // > MAX_EXPANDED_MEMORY_ADDRESS (0x1fffffffe0) + test_root(0xffffffff1_u64.into(), 0xffffffff0_u64.into()); + } + + #[test] + fn test_oog_log_max_u64_address() { + test_root(u64::MAX.into(), u64::MAX.into()); + } + + #[test] + fn test_oog_log_max_word_address() { + test_root(U256::MAX, U256::MAX); + } + + #[derive(Clone, Copy, Debug, Default)] + struct Stack { + gas: u64, + value: Word, + cd_offset: u64, + cd_length: u64, + rd_offset: u64, + rd_length: u64, + } + + fn test_root(offset: U256, size: U256) { + let tx = mock_tx(eth(1), gwei(2), vec![]); + let code = bytecode! { PUSH1(0) - PUSH1(0) - PUSH1(100) + PUSH32(size) + PUSH32(offset) LOG0 }; @@ -184,7 +220,7 @@ mod test { .from(tx.from) .gas_price(tx.gas_price.unwrap()) .gas(tx.gas + 5) - .input(tx.input) + .input(tx.input.clone()) .value(tx.value); }, |block, _tx| block.number(0xcafeu64), @@ -194,34 +230,35 @@ mod test { CircuitTestBuilder::new_from_test_ctx(ctx).run(); } - fn mock_tx(value: Word, gas_price: Word, calldata: Vec) -> eth_types::Transaction { - let from = MOCK_ACCOUNTS[1]; - let to = MOCK_ACCOUNTS[0]; - eth_types::Transaction { - from, - to: Some(to), - value, - gas: gas(&calldata), - gas_price: Some(gas_price), - input: calldata.into(), - ..Default::default() - } - } - - #[test] - // test oog log in root call - fn test_oog_log_root() { - test_oog_log(mock_tx(eth(1), gwei(2), vec![])); - } + fn test_internal(caller: Account, callee: Account) { + let ctx = TestContext::<3, 1>::new( + None, + |accs| { + accs[0] + .address(address!("0x000000000000000000000000000000000000cafe")) + .balance(Word::from(10u64.pow(19))); + accs[1] + .address(caller.address) + .code(caller.code) + .nonce(caller.nonce) + .balance(caller.balance); + accs[2] + .address(callee.address) + .code(callee.code) + .nonce(callee.nonce) + .balance(callee.balance); + }, + |mut txs, accs| { + txs[0] + .from(accs[0].address) + .to(accs[1].address) + .gas(24000.into()); + }, + |block, _tx| block.number(0xcafeu64), + ) + .unwrap(); - #[derive(Clone, Copy, Debug, Default)] - struct Stack { - gas: u64, - value: Word, - cd_offset: u64, - cd_length: u64, - rd_offset: u64, - rd_length: u64, + CircuitTestBuilder::new_from_test_ctx(ctx).run(); } fn caller() -> Account { @@ -263,37 +300,6 @@ mod test { } } - fn oog_log_internal_call(caller: Account, callee: Account) { - let ctx = TestContext::<3, 1>::new( - None, - |accs| { - accs[0] - .address(address!("0x000000000000000000000000000000000000cafe")) - .balance(Word::from(10u64.pow(19))); - accs[1] - .address(caller.address) - .code(caller.code) - .nonce(caller.nonce) - .balance(caller.balance); - accs[2] - .address(callee.address) - .code(callee.code) - .nonce(callee.nonce) - .balance(callee.balance); - }, - |mut txs, accs| { - txs[0] - .from(accs[0].address) - .to(accs[1].address) - .gas(24000.into()); - }, - |block, _tx| block.number(0xcafeu64), - ) - .unwrap(); - - CircuitTestBuilder::new_from_test_ctx(ctx).run(); - } - fn callee(code: Bytecode) -> Account { let code = code.to_vec(); let is_empty = code.is_empty(); @@ -306,19 +312,28 @@ mod test { } } - #[test] - // test oog log in internal call - fn test_oog_log_internal() { - let bytecode = bytecode! { - PUSH32(Word::from(0)) - PUSH32(Word::from(10)) - PUSH32(Word::from(224)) - PUSH32(Word::from(1025)) - PUSH32(Word::from(5089)) - LOG2 - STOP - }; - let callee = callee(bytecode); - oog_log_internal_call(caller(), callee); + fn gas(call_data: &[u8]) -> Word { + Word::from( + GasCost::TX.as_u64() + + 2 * OpcodeId::PUSH32.constant_gas_cost().as_u64() + + call_data + .iter() + .map(|&x| if x == 0 { 4 } else { 16 }) + .sum::(), + ) + } + + fn mock_tx(value: Word, gas_price: Word, calldata: Vec) -> Transaction { + let from = MOCK_ACCOUNTS[1]; + let to = MOCK_ACCOUNTS[0]; + Transaction { + from, + to: Some(to), + value, + gas: gas(&calldata), + gas_price: Some(gas_price), + input: calldata.into(), + ..Default::default() + } } } diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs index 4313f1df6f..ea75656615 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_memory_copy.rs @@ -8,8 +8,11 @@ use crate::{ constraint_builder::ConstraintBuilder, from_bytes, math_gadget::{IsZeroGadget, LtGadget}, - memory_gadget::{MemoryAddressGadget, MemoryCopierGasGadget, MemoryExpansionGadget}, - select, CachedRegion, Cell, Word, + memory_gadget::{ + CommonMemoryAddressGadget, MemoryCopierGasGadget, MemoryExpandedAddressGadget, + MemoryExpansionGadget, + }, + or, select, CachedRegion, Cell, Word, }, witness::{Block, Call, ExecStep, Transaction}, }, @@ -36,7 +39,7 @@ pub(crate) struct ErrorOOGMemoryCopyGadget { /// Source offset src_offset: Word, /// Destination offset and size to copy - dst_memory_addr: MemoryAddressGadget, + dst_memory_addr: MemoryExpandedAddressGadget, memory_expansion: MemoryExpansionGadget, memory_copier_gas: MemoryCopierGasGadget, insufficient_gas: LtGadget, @@ -62,9 +65,7 @@ impl ExecutionGadget for ErrorOOGMemoryCopyGadget { ], ); - let dst_offset = cb.query_cell_phase2(); let src_offset = cb.query_word_rlc(); - let copy_size = cb.query_word_rlc(); let external_address = cb.query_word_rlc(); let is_warm = cb.query_bool(); let tx_id = cb.query_cell(); @@ -86,11 +87,12 @@ impl ExecutionGadget for ErrorOOGMemoryCopyGadget { cb.stack_pop(external_address.expr()); }); - cb.stack_pop(dst_offset.expr()); + let dst_memory_addr = MemoryExpandedAddressGadget::construct_self(cb); + + cb.stack_pop(dst_memory_addr.offset_rlc()); cb.stack_pop(src_offset.expr()); - cb.stack_pop(copy_size.expr()); + cb.stack_pop(dst_memory_addr.length_rlc()); - let dst_memory_addr = MemoryAddressGadget::construct(cb, dst_offset, copy_size); let memory_expansion = MemoryExpansionGadget::construct(cb, [dst_memory_addr.address()]); let memory_copier_gas = MemoryCopierGasGadget::construct( cb, @@ -118,8 +120,8 @@ impl ExecutionGadget for ErrorOOGMemoryCopyGadget { ); cb.require_equal( - "Gas left is less than gas cost", - insufficient_gas.expr(), + "Memory address is overflow or gas left is less than cost", + or::expr([dst_memory_addr.overflow(), insufficient_gas.expr()]), 1.expr(), ); @@ -197,7 +199,7 @@ impl ExecutionGadget for ErrorOOGMemoryCopyGadget { let memory_copier_gas = self.memory_copier_gas.assign( region, offset, - copy_size.as_u64(), + MemoryExpandedAddressGadget::::length_value(dst_offset, copy_size), memory_expansion_cost, )?; let constant_gas_cost = if is_extcodecopy { @@ -248,6 +250,7 @@ mod tests { use itertools::Itertools; use mock::{ eth, test_ctx::helpers::account_0_code_account_1_no_code, TestContext, MOCK_ACCOUNTS, + MOCK_BLOCK_GAS_LIMIT, }; const TESTING_COMMON_OPCODES: &[OpcodeId] = &[ @@ -265,7 +268,8 @@ mod tests { .iter() .cartesian_product(TESTING_DST_OFFSET_COPY_SIZE_PAIRS.iter()) { - let testing_data = TestingData::new_for_common_opcode(*opcode, *dst_offset, *copy_size); + let testing_data = + TestingData::new_for_common_opcode(*opcode, *dst_offset, *copy_size, None); test_root(&testing_data); test_internal(&testing_data); @@ -278,20 +282,38 @@ mod tests { .iter() .cartesian_product(TESTING_DST_OFFSET_COPY_SIZE_PAIRS.iter()) { - let testing_data = TestingData::new_for_extcodecopy(*is_warm, *dst_offset, *copy_size); + let testing_data = + TestingData::new_for_extcodecopy(*is_warm, *dst_offset, *copy_size, None); test_root(&testing_data); test_internal(&testing_data); } } + #[test] + fn test_oog_memory_copy_max_expanded_address() { + // 0xffffffff1 + 0xffffffff0 = 0x1fffffffe1 + // > MAX_EXPANDED_MEMORY_ADDRESS (0x1fffffffe0) + test_for_edge_memory_size(0xffffffff1, 0xffffffff0); + } + + #[test] + fn test_oog_memory_copy_max_u64_address() { + test_for_edge_memory_size(u64::MAX, u64::MAX); + } + struct TestingData { bytecode: Bytecode, gas_cost: u64, } impl TestingData { - pub fn new_for_common_opcode(opcode: OpcodeId, dst_offset: u64, copy_size: u64) -> Self { + pub fn new_for_common_opcode( + opcode: OpcodeId, + dst_offset: u64, + copy_size: u64, + gas_cost: Option, + ) -> Self { let bytecode = bytecode! { PUSH32(copy_size) PUSH32(rand_word()) @@ -299,16 +321,23 @@ mod tests { .write_op(opcode) }; - let memory_word_size = (dst_offset + copy_size + 31) / 32; + let gas_cost = gas_cost.unwrap_or_else(|| { + let memory_word_size = (dst_offset + copy_size + 31) / 32; - let gas_cost = OpcodeId::PUSH32.constant_gas_cost().0 * 3 - + opcode.constant_gas_cost().0 - + memory_copier_gas_cost(0, memory_word_size, copy_size, GasCost::COPY.as_u64()); + OpcodeId::PUSH32.constant_gas_cost().0 * 3 + + opcode.constant_gas_cost().0 + + memory_copier_gas_cost(0, memory_word_size, copy_size, GasCost::COPY.as_u64()) + }); Self { bytecode, gas_cost } } - pub fn new_for_extcodecopy(is_warm: bool, dst_offset: u64, copy_size: u64) -> Self { + pub fn new_for_extcodecopy( + is_warm: bool, + dst_offset: u64, + copy_size: u64, + gas_cost: Option, + ) -> Self { let external_address = MOCK_ACCOUNTS[4]; let mut bytecode = bytecode! { @@ -319,12 +348,6 @@ mod tests { EXTCODECOPY }; - let memory_word_size = (dst_offset + copy_size + 31) / 32; - - let mut gas_cost = OpcodeId::PUSH32.constant_gas_cost().0 * 4 - + GasCost::COLD_ACCOUNT_ACCESS.0 - + memory_copier_gas_cost(0, memory_word_size, copy_size, GasCost::COPY.as_u64()); - if is_warm { bytecode.append(&bytecode! { PUSH32(copy_size) @@ -333,31 +356,59 @@ mod tests { PUSH32(external_address.to_word()) EXTCODECOPY }); + } + + let gas_cost = gas_cost.unwrap_or_else(|| { + let memory_word_size = (dst_offset + copy_size + 31) / 32; - gas_cost += OpcodeId::PUSH32.constant_gas_cost().0 * 4 - + GasCost::WARM_ACCESS.0 + let gas_cost = OpcodeId::PUSH32.constant_gas_cost().0 * 4 + + GasCost::COLD_ACCOUNT_ACCESS.0 + memory_copier_gas_cost( - memory_word_size, + 0, memory_word_size, copy_size, GasCost::COPY.as_u64(), ); - } + + if is_warm { + gas_cost + + OpcodeId::PUSH32.constant_gas_cost().0 * 4 + + GasCost::WARM_ACCESS.0 + + memory_copier_gas_cost( + memory_word_size, + memory_word_size, + copy_size, + GasCost::COPY.as_u64(), + ) + } else { + gas_cost + } + }); Self { bytecode, gas_cost } } } fn test_root(testing_data: &TestingData) { + let gas_cost = GasCost::TX + .0 + // Decrease expected gas cost (by 1) to trigger out of gas error. + .checked_add(testing_data.gas_cost - 1) + .unwrap_or(MOCK_BLOCK_GAS_LIMIT); + let gas_cost = if gas_cost > MOCK_BLOCK_GAS_LIMIT { + MOCK_BLOCK_GAS_LIMIT + } else { + gas_cost + }; + let ctx = TestContext::<2, 1>::new( None, account_0_code_account_1_no_code(testing_data.bytecode.clone()), |mut txs, accs| { - // Decrease expected gas cost (by 1) to trigger out of gas error. txs[0] .from(accs[1].address) .to(accs[0].address) - .gas((GasCost::TX.0 + testing_data.gas_cost - 1).into()); + .gas(gas_cost.into()); }, |block, _tx| block.number(0xcafe_u64), ) @@ -408,4 +459,30 @@ mod tests { CircuitTestBuilder::new_from_test_ctx(ctx).run(); } + + fn test_for_edge_memory_size(dst_offset: u64, copy_size: u64) { + TESTING_COMMON_OPCODES.iter().for_each(|opcode| { + let testing_data = TestingData::new_for_common_opcode( + *opcode, + dst_offset, + copy_size, + Some(MOCK_BLOCK_GAS_LIMIT), + ); + + test_root(&testing_data); + test_internal(&testing_data); + }); + + [false, true].into_iter().for_each(|is_warm| { + let testing_data = TestingData::new_for_extcodecopy( + is_warm, + dst_offset, + copy_size, + Some(MOCK_BLOCK_GAS_LIMIT), + ); + + test_root(&testing_data); + test_internal(&testing_data); + }); + } } diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_sha3.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_sha3.rs index 7340914e77..02761117bd 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_sha3.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_sha3.rs @@ -8,7 +8,8 @@ use crate::{ constraint_builder::ConstraintBuilder, math_gadget::LtGadget, memory_gadget::{ - MemoryCopierGasGadget, MemoryExpandedAddressGadget, MemoryExpansionGadget, + CommonMemoryAddressGadget, MemoryCopierGasGadget, MemoryExpandedAddressGadget, + MemoryExpansionGadget, }, or, CachedRegion, Cell, }, @@ -47,7 +48,7 @@ impl ExecutionGadget for ErrorOOGSha3Gadget { OpcodeId::SHA3.expr(), ); - let memory_address = MemoryExpandedAddressGadget::construct(cb); + let memory_address = MemoryExpandedAddressGadget::construct_self(cb); cb.stack_pop(memory_address.offset_rlc()); cb.stack_pop(memory_address.length_rlc()); @@ -65,8 +66,8 @@ impl ExecutionGadget for ErrorOOGSha3Gadget { ); cb.require_equal( - "Offset plus length is greater than maximum expanded address or gas left is less than cost", - or::expr([memory_address.address_overflow(), insufficient_gas.expr()]), + "Memory address is overflow or gas left is less than cost", + or::expr([memory_address.overflow(), insufficient_gas.expr()]), 1.expr(), ); @@ -115,7 +116,7 @@ impl ExecutionGadget for ErrorOOGSha3Gadget { let memory_copier_gas = self.memory_copier_gas.assign( region, offset, - memory_length.low_u64(), + MemoryExpandedAddressGadget::::length_value(memory_offset, memory_length), memory_expansion_cost, )?; self.insufficient_gas.assign_value( @@ -143,10 +144,9 @@ mod tests { }; use mock::{ eth, test_ctx::helpers::account_0_code_account_1_no_code, TestContext, MOCK_ACCOUNTS, + MOCK_BLOCK_GAS_LIMIT, }; - const BLOCK_GAS_LIMIT: u64 = 10_000_000_000_000_000; - #[test] fn test_oog_sha3_less_than_constant_gas() { let testing_data = TestingData::new(0x20, 0, OpcodeId::SHA3.constant_gas_cost().0); @@ -171,7 +171,7 @@ mod tests { fn test_oog_sha3_max_expanded_address() { // 0xffffffff1 + 0xffffffff0 = 0x1fffffffe1 // > MAX_EXPANDED_MEMORY_ADDRESS (0x1fffffffe0) - let testing_data = TestingData::new(0xffffffff1, 0xffffffff0, BLOCK_GAS_LIMIT); + let testing_data = TestingData::new(0xffffffff1, 0xffffffff0, MOCK_BLOCK_GAS_LIMIT); test_root(&testing_data); test_internal(&testing_data); @@ -179,9 +179,7 @@ mod tests { #[test] fn test_oog_sha3_max_u64_address() { - // If `offset + length > MAX_U64 - 31`, return ErrGasUintOverflow. - // https://github.com/ethereum/go-ethereum/blob/e6b6a8b738069ad0579f6798ee59fde93ed13b43/core/vm/common.go#L68 - let testing_data = TestingData::new(u64::MAX - 100 - 31, 100, BLOCK_GAS_LIMIT); + let testing_data = TestingData::new(u64::MAX, u64::MAX, MOCK_BLOCK_GAS_LIMIT); test_root(&testing_data); test_internal(&testing_data); @@ -202,9 +200,9 @@ mod tests { let gas_cost = gas_cost .checked_add(OpcodeId::PUSH32.constant_gas_cost().0 * 2) - .unwrap_or(BLOCK_GAS_LIMIT); - let gas_cost = if gas_cost > BLOCK_GAS_LIMIT { - BLOCK_GAS_LIMIT + .unwrap_or(MOCK_BLOCK_GAS_LIMIT); + let gas_cost = if gas_cost > MOCK_BLOCK_GAS_LIMIT { + MOCK_BLOCK_GAS_LIMIT } else { gas_cost }; @@ -229,9 +227,9 @@ mod tests { .0 // Decrease expected gas cost (by 1) to trigger out of gas error. .checked_add(testing_data.gas_cost - 1) - .unwrap_or(BLOCK_GAS_LIMIT); - let gas_cost = if gas_cost > BLOCK_GAS_LIMIT { - BLOCK_GAS_LIMIT + .unwrap_or(MOCK_BLOCK_GAS_LIMIT); + let gas_cost = if gas_cost > MOCK_BLOCK_GAS_LIMIT { + MOCK_BLOCK_GAS_LIMIT } else { gas_cost }; diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_static_memory.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_static_memory.rs index 0b15f4aee3..2baac651e8 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_static_memory.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_static_memory.rs @@ -1,43 +1,33 @@ use crate::{ evm_circuit::{ execution::ExecutionGadget, - param::{N_BYTES_GAS, N_BYTES_MEMORY_ADDRESS, N_BYTES_MEMORY_WORD_SIZE}, + param::{N_BYTES_GAS, N_BYTES_MEMORY_WORD_SIZE}, step::ExecutionState, util::{ - and, common_gadget::RestoreContextGadget, constraint_builder::{ ConstraintBuilder, StepStateTransition, Transition::{Delta, Same}, }, - math_gadget::{IsEqualGadget, IsZeroGadget, LtGadget}, - memory_gadget::{address_high, address_low, MemoryExpansionGadget}, - not, select, CachedRegion, Cell, Word, + math_gadget::{IsEqualGadget, LtGadget}, + memory_gadget::{ + CommonMemoryAddressGadget, MemoryExpandedAddressGadget, MemoryExpansionGadget, + }, + not, or, select, CachedRegion, Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, table::CallContextFieldTag, util::Expr, }; -use eth_types::{evm_types::OpcodeId, Field, ToLittleEndian}; +use eth_types::{evm_types::OpcodeId, Field}; use halo2_proofs::{circuit::Value, plonk::Error}; #[derive(Clone, Debug)] pub(crate) struct ErrorOOGStaticMemoryGadget { opcode: Cell, - address: Word, - // Allow memory size to expand to 5 bytes, because memory address could be - // at most 2^40 - 1, after constant division by 32, the memory word size - // then could be at most 2^35 - 1. - // So generic N_BYTES_MEMORY_WORD_SIZE for MemoryExpansionGadget needs to - // be larger by 1 than normal usage (to be 5), to be able to contain - // number up to 2^35 - 1. - address_in_range_high: IsZeroGadget, - expanded_address_in_range: LtGadget, - memory_expansion: MemoryExpansionGadget, - // Even memory size at most could be 2^35 - 1, the qudratic part of memory - // expansion gas cost could be at most 2^61 - 2^27, due to the constant - // division by 512, which still fits in 8 bytes. + memory_address: MemoryExpandedAddressGadget, + memory_expansion: MemoryExpansionGadget, insufficient_gas: LtGadget, is_mload: IsEqualGadget, is_mstore8: IsEqualGadget, @@ -55,9 +45,8 @@ impl ExecutionGadget for ErrorOOGStaticMemoryGadget { let opcode = cb.query_cell(); cb.opcode_lookup(opcode.expr(), 1.expr()); - // Query address by a full word - let address = cb.query_word_rlc(); - cb.stack_pop(address.expr()); + let memory_address = MemoryExpandedAddressGadget::construct_self(cb); + cb.stack_pop(memory_address.offset_rlc()); // Check if this is an MSTORE8 let is_mload = IsEqualGadget::construct(cb, opcode.expr(), OpcodeId::MLOAD.expr()); @@ -77,31 +66,14 @@ impl ExecutionGadget for ErrorOOGStaticMemoryGadget { ), ); - // Check if the memory address is too large - let address_low = address_low::expr(&address); - let address_high = address_high::expr(&address); - let size = select::expr(is_mstore8.expr(), 1.expr(), 32.expr()); - let expanded_address = address_low.expr() + size.expr(); - - // sanity check - let address_in_range_high = IsZeroGadget::construct(cb, address_high); - let expanded_address_in_range = LtGadget::construct( - cb, - expanded_address.expr(), - (1u64 << (N_BYTES_MEMORY_ADDRESS * 8)).expr(), - ); - cb.require_equal( - "address must less than 5 bytes", - and::expr([ - address_in_range_high.expr(), - expanded_address_in_range.expr(), - ]), - true.expr(), + "Memory length must be 32 for MLOAD and MSTORE, and 1 for MSTORE8", + memory_address.length_rlc(), + select::expr(is_mstore8.expr(), 1.expr(), 32.expr()), ); // Get the next memory size and the gas cost for this memory access - let memory_expansion = MemoryExpansionGadget::construct(cb, [expanded_address]); + let memory_expansion = MemoryExpansionGadget::construct(cb, [memory_address.address()]); // Check if the amount of gas available is less than the amount of gas // required @@ -112,9 +84,9 @@ impl ExecutionGadget for ErrorOOGStaticMemoryGadget { ); cb.require_equal( - "gas_left must less than gas_cost", - insufficient_gas.expr(), - true.expr(), + "Memory address is overflow or gas left is less than cost", + or::expr([memory_address.overflow(), insufficient_gas.expr()]), + 1.expr(), ); // Current call must fail. @@ -150,9 +122,7 @@ impl ExecutionGadget for ErrorOOGStaticMemoryGadget { Self { opcode, - address, - address_in_range_high, - expanded_address_in_range, + memory_address, memory_expansion, insufficient_gas, is_mload, @@ -174,9 +144,16 @@ impl ExecutionGadget for ErrorOOGStaticMemoryGadget { let opcode = step.opcode.unwrap(); // Inputs/Outputs - let address = block.rws[step.rw_indices[0]].stack_value(); - self.address - .assign(region, offset, Some(address.to_le_bytes()))?; + let memory_offset = block.rws[step.rw_indices[0]].stack_value(); + + // Memory lengths are set to default values for MLOAD, MSTORE and + // MSTORE8 in go-ethereum. + // + let memory_length = match opcode { + OpcodeId::MLOAD | OpcodeId::MSTORE => 32, + OpcodeId::MSTORE8 => 1, + _ => unreachable!(), + }; self.opcode .assign(region, offset, Value::known(F::from(opcode.as_u64())))?; @@ -193,32 +170,13 @@ impl ExecutionGadget for ErrorOOGStaticMemoryGadget { F::from(OpcodeId::MSTORE8.as_u64()), )?; - // Address in range check - let address_high = address_high::value::(address.to_le_bytes()); - self.address_in_range_high - .assign(region, offset, address_high)?; - - let address_value = address_low::value(address.to_le_bytes()); - let size = if opcode == OpcodeId::MSTORE8 { 1 } else { 32 }; - - let expanded_address = address_value - .checked_add(size) - .expect("address overflow u64"); - self.expanded_address_in_range.assign( - region, - offset, - F::from(expanded_address), - F::from(1u64 << (N_BYTES_MEMORY_ADDRESS * 8)), - )?; - - // TODO: sanity check, remove this after fixing #347 missing ErrGasUintOverflow - if address_high != F::zero() || expanded_address > (1u64 << (N_BYTES_MEMORY_ADDRESS * 8)) { - panic!("address overflow {} bytes", N_BYTES_MEMORY_ADDRESS); - } + let memory_address = + self.memory_address + .assign(region, offset, memory_offset, memory_length.into())?; let memory_expansion_cost = self .memory_expansion - .assign(region, offset, step.memory_word_size(), [expanded_address])? + .assign(region, offset, step.memory_word_size(), [memory_address])? .1; // Gas insufficient check @@ -245,43 +203,69 @@ impl ExecutionGadget for ErrorOOGStaticMemoryGadget { #[cfg(test)] mod tests { use crate::test_util::CircuitTestBuilder; - use eth_types::{bytecode, word, Bytecode, ToWord}; + 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() { - let codes = [ + fn test_oog_static_memory_simple() { + for code in testing_bytecodes(0xffffffff_u64.into()).iter() { + test_root(code); + test_internal(code); + } + } + + #[test] + fn test_oog_static_memory_max_expanded_address() { + // > MAX_EXPANDED_MEMORY_ADDRESS (0x1fffffffe0) + for code in testing_bytecodes(0x1fffffffe1_u64.into()).iter() { + test_root(code); + test_internal(code); + } + } + + #[test] + fn test_oog_static_memory_max_u64_address() { + for code in testing_bytecodes(u64::MAX.into()).iter() { + test_root(code); + test_internal(code); + } + } + + #[test] + fn test_oog_static_memory_max_word_address() { + for code in testing_bytecodes(U256::MAX).iter() { + test_root(code); + test_internal(code); + } + } + + fn testing_bytecodes(offset: U256) -> Vec { + vec![ bytecode! { - PUSH8(0xFF) - PUSH32(word!("0xffffffff")) // offset + PUSH8(0xff) + PUSH32(offset) MSTORE STOP }, bytecode! { - PUSH8(0xFF) - PUSH32(word!("0xffffffff")) // offset + PUSH8(0xff) + PUSH32(offset) MSTORE8 STOP }, bytecode! { - PUSH32(word!("0xffffffff")) // offset + PUSH32(offset) MLOAD STOP }, - ]; - - for code in codes.iter() { - test_root(code.clone()); - test_internal(code.clone()); - } + ] } - - fn test_root(code: Bytecode) { + fn test_root(code: &Bytecode) { let ctx = TestContext::<2, 1>::new( None, - account_0_code_account_1_no_code(code), + account_0_code_account_1_no_code(code.clone()), |mut txs, accs| { txs[0] .from(accs[1].address) @@ -295,7 +279,7 @@ mod tests { CircuitTestBuilder::new_from_test_ctx(ctx).run(); } - fn test_internal(code: Bytecode) { + fn test_internal(code: &Bytecode) { let code_a = bytecode! { PUSH1(0x00) // retLength PUSH1(0x00) // retOffset @@ -313,7 +297,7 @@ mod tests { None, |accs| { accs[0].address(MOCK_ACCOUNTS[0]).code(code_a); - accs[1].address(MOCK_ACCOUNTS[1]).code(code); + accs[1].address(MOCK_ACCOUNTS[1]).code(code.clone()); accs[2].address(MOCK_ACCOUNTS[2]).balance(eth(1)); }, |mut txs, accs| { diff --git a/zkevm-circuits/src/evm_circuit/execution/error_precompile_failed.rs b/zkevm-circuits/src/evm_circuit/execution/error_precompile_failed.rs index ec63e43aa0..9d075b3809 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_precompile_failed.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_precompile_failed.rs @@ -3,8 +3,10 @@ use crate::{ execution::ExecutionGadget, step::ExecutionState, util::{ - constraint_builder::ConstraintBuilder, math_gadget::IsZeroGadget, - memory_gadget::MemoryAddressGadget, CachedRegion, Cell, Word, + constraint_builder::ConstraintBuilder, + math_gadget::IsZeroGadget, + memory_gadget::{CommonMemoryAddressGadget, MemoryAddressGadget}, + CachedRegion, Cell, Word, }, }, util::Expr, diff --git a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs index dc5e7e8c99..4fc6633912 100644 --- a/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/extcodecopy.rs @@ -8,7 +8,10 @@ use crate::{ ConstraintBuilder, ReversionInfo, StepStateTransition, Transition, }, from_bytes, - memory_gadget::{MemoryAddressGadget, MemoryCopierGasGadget, MemoryExpansionGadget}, + memory_gadget::{ + CommonMemoryAddressGadget, MemoryAddressGadget, MemoryCopierGasGadget, + MemoryExpansionGadget, + }, not, select, CachedRegion, Cell, Word, }, witness::{Block, Call, ExecStep, Transaction}, diff --git a/zkevm-circuits/src/evm_circuit/execution/logs.rs b/zkevm-circuits/src/evm_circuit/execution/logs.rs index 0c17bd7c93..2e64aac56d 100644 --- a/zkevm-circuits/src/evm_circuit/execution/logs.rs +++ b/zkevm-circuits/src/evm_circuit/execution/logs.rs @@ -9,7 +9,9 @@ use crate::{ ConstraintBuilder, StepStateTransition, Transition::{Delta, To}, }, - memory_gadget::{MemoryAddressGadget, MemoryExpansionGadget}, + memory_gadget::{ + CommonMemoryAddressGadget, MemoryAddressGadget, MemoryExpansionGadget, + }, not, sum, CachedRegion, Cell, }, witness::{Block, Call, ExecStep, Transaction}, diff --git a/zkevm-circuits/src/evm_circuit/execution/return_revert.rs b/zkevm-circuits/src/evm_circuit/execution/return_revert.rs index 66f88a05f1..328330f5f4 100644 --- a/zkevm-circuits/src/evm_circuit/execution/return_revert.rs +++ b/zkevm-circuits/src/evm_circuit/execution/return_revert.rs @@ -10,7 +10,9 @@ use crate::{ Transition::{Delta, To}, }, math_gadget::{IsZeroGadget, MinMaxGadget}, - memory_gadget::{MemoryAddressGadget, MemoryExpansionGadget}, + memory_gadget::{ + CommonMemoryAddressGadget, MemoryAddressGadget, MemoryExpansionGadget, + }, not, CachedRegion, Cell, }, witness::{Block, Call, ExecStep, Transaction}, diff --git a/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs b/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs index b09b11eabd..2f8e515d2b 100644 --- a/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs +++ b/zkevm-circuits/src/evm_circuit/execution/returndatacopy.rs @@ -11,7 +11,10 @@ use crate::{ }, from_bytes, math_gadget::RangeCheckGadget, - memory_gadget::{MemoryAddressGadget, MemoryCopierGasGadget, MemoryExpansionGadget}, + memory_gadget::{ + CommonMemoryAddressGadget, MemoryAddressGadget, MemoryCopierGasGadget, + MemoryExpansionGadget, + }, CachedRegion, Cell, MemoryAddress, }, witness::{Block, Call, ExecStep, Transaction}, diff --git a/zkevm-circuits/src/evm_circuit/execution/sha3.rs b/zkevm-circuits/src/evm_circuit/execution/sha3.rs index db74186fb5..37b675b0b4 100644 --- a/zkevm-circuits/src/evm_circuit/execution/sha3.rs +++ b/zkevm-circuits/src/evm_circuit/execution/sha3.rs @@ -9,7 +9,10 @@ use crate::evm_circuit::{ util::{ common_gadget::SameContextGadget, constraint_builder::{ConstraintBuilder, StepStateTransition, Transition}, - memory_gadget::{MemoryAddressGadget, MemoryCopierGasGadget, MemoryExpansionGadget}, + memory_gadget::{ + CommonMemoryAddressGadget, MemoryAddressGadget, MemoryCopierGasGadget, + MemoryExpansionGadget, + }, rlc, CachedRegion, Cell, Word, }, witness::{Block, Call, ExecStep, Transaction}, diff --git a/zkevm-circuits/src/evm_circuit/step.rs b/zkevm-circuits/src/evm_circuit/step.rs index 9ac681344c..a250d364a6 100644 --- a/zkevm-circuits/src/evm_circuit/step.rs +++ b/zkevm-circuits/src/evm_circuit/step.rs @@ -110,7 +110,6 @@ pub enum ExecutionState { ErrorOutOfGasSloadSstore, ErrorOutOfGasCREATE2, ErrorOutOfGasSELFDESTRUCT, - ErrorGasUintOverflow, } impl Default for ExecutionState { @@ -145,7 +144,6 @@ impl ExecutionState { | Self::ErrorInvalidCreationCode | Self::ErrorInvalidJump | Self::ErrorReturnDataOutOfBound - | Self::ErrorGasUintOverflow | Self::ErrorOutOfGasConstant | Self::ErrorOutOfGasStaticMemoryExpansion | Self::ErrorOutOfGasDynamicMemoryExpansion diff --git a/zkevm-circuits/src/evm_circuit/util/common_gadget.rs b/zkevm-circuits/src/evm_circuit/util/common_gadget.rs index fa16f0382f..5b289536ab 100644 --- a/zkevm-circuits/src/evm_circuit/util/common_gadget.rs +++ b/zkevm-circuits/src/evm_circuit/util/common_gadget.rs @@ -1,7 +1,7 @@ use super::{ from_bytes, math_gadget::{IsEqualGadget, IsZeroGadget, LtGadget}, - memory_gadget::{MemoryAddressGadget, MemoryExpansionGadget}, + memory_gadget::{CommonMemoryAddressGadget, MemoryExpansionGadget}, CachedRegion, }; use crate::{ @@ -612,15 +612,15 @@ impl TransferGadget { } #[derive(Clone, Debug)] -pub(crate) struct CommonCallGadget { +pub(crate) struct CommonCallGadget { pub is_success: Cell, pub gas: Word, pub gas_is_u64: IsZeroGadget, pub callee_address: Word, pub value: Word, - pub cd_address: MemoryAddressGadget, - pub rd_address: MemoryAddressGadget, + pub cd_address: MemAddrGadget, + pub rd_address: MemAddrGadget, pub memory_expansion: MemoryExpansionGadget, value_is_zero: IsZeroGadget, @@ -631,7 +631,9 @@ pub(crate) struct CommonCallGadget { pub callee_not_exists: IsZeroGadget, } -impl CommonCallGadget { +impl, const IS_SUCCESS_CALL: bool> + CommonCallGadget +{ pub(crate) fn construct( cb: &mut ConstraintBuilder, is_call: Expression, @@ -649,12 +651,11 @@ impl CommonCallGadget let gas_word = cb.query_word_rlc(); let callee_address_word = cb.query_word_rlc(); let value = cb.query_word_rlc(); - let cd_offset = cb.query_cell_phase2(); - let cd_length = cb.query_word_rlc(); - let rd_offset = cb.query_cell_phase2(); - let rd_length = cb.query_word_rlc(); let is_success = cb.query_bool(); + let cd_address = MemAddrGadget::construct_self(cb); + let rd_address = MemAddrGadget::construct_self(cb); + // Lookup values from stack // `callee_address` is poped from stack and used to check if it exists in // access list and get code hash. @@ -668,10 +669,10 @@ impl CommonCallGadget // `CALL` and `CALLCODE` opcodes have an additional stack pop `value`. cb.condition(is_call + is_callcode, |cb| cb.stack_pop(value.expr())); - cb.stack_pop(cd_offset.expr()); - cb.stack_pop(cd_length.expr()); - cb.stack_pop(rd_offset.expr()); - cb.stack_pop(rd_length.expr()); + cb.stack_pop(cd_address.offset_rlc()); + cb.stack_pop(cd_address.length_rlc()); + cb.stack_pop(rd_address.offset_rlc()); + cb.stack_pop(rd_address.length_rlc()); cb.stack_push(if IS_SUCCESS_CALL { is_success.expr() } else { @@ -680,8 +681,6 @@ impl CommonCallGadget // Recomposition of random linear combination to integer let gas_is_u64 = IsZeroGadget::construct(cb, sum::expr(&gas_word.cells[N_BYTES_GAS..])); - let cd_address = MemoryAddressGadget::construct(cb, cd_offset, cd_length); - let rd_address = MemoryAddressGadget::construct(cb, rd_offset, rd_length); let memory_expansion = MemoryExpansionGadget::construct(cb, [cd_address.address(), rd_address.address()]); diff --git a/zkevm-circuits/src/evm_circuit/util/memory_gadget.rs b/zkevm-circuits/src/evm_circuit/util/memory_gadget.rs index db924adbfc..5e5048c08e 100644 --- a/zkevm-circuits/src/evm_circuit/util/memory_gadget.rs +++ b/zkevm-circuits/src/evm_circuit/util/memory_gadget.rs @@ -3,13 +3,14 @@ use crate::{ evm_circuit::{ param::{N_BYTES_GAS, N_BYTES_MEMORY_ADDRESS, N_BYTES_MEMORY_WORD_SIZE, N_BYTES_U64}, util::{ - common_gadget::WordByteRangeGadget, + and, constraint_builder::ConstraintBuilder, from_bytes, math_gadget::{ - ConstantDivisionGadget, IsZeroGadget, LtGadget, MinMaxGadget, RangeCheckGadget, + AddWordsGadget, ConstantDivisionGadget, IsZeroGadget, LtGadget, MinMaxGadget, + RangeCheckGadget, }, - not, select, sum, Cell, CellType, MemoryAddress, + not, or, select, sum, Cell, CellType, MemoryAddress, }, }, util::Expr, @@ -20,6 +21,7 @@ use eth_types::{ Field, ToLittleEndian, U256, }; use halo2_proofs::{ + arithmetic::FieldExt, circuit::Value, plonk::{Error, Expression}, }; @@ -63,6 +65,32 @@ pub(crate) mod address_high { } } +/// Memory address trait to adapt for right and Uint overflow cases. +pub(crate) trait CommonMemoryAddressGadget { + fn construct_self(cb: &mut ConstraintBuilder) -> Self; + + /// Return the memory address (offset + length). + fn assign( + &self, + region: &mut CachedRegion<'_, '_, F>, + offset: usize, + memory_offset: U256, + memory_length: U256, + ) -> Result; + + /// Return original word of memory offset. + fn offset_rlc(&self) -> Expression; + + /// Return original word of memory length. + fn length_rlc(&self) -> Expression; + + /// Return valid memory length of Uint64. + fn length(&self) -> Expression; + + /// Return valid memory offset plus length. + fn address(&self) -> Expression; +} + /// Convert the dynamic memory offset and length from random linear combination /// to integer. It handles the "no expansion" feature by setting the /// `memory_offset_bytes` to zero when `memory_length` is zero. In this case, @@ -75,43 +103,14 @@ pub(crate) struct MemoryAddressGadget { memory_length_is_zero: IsZeroGadget, } -impl MemoryAddressGadget { - pub(crate) fn construct( - cb: &mut ConstraintBuilder, - memory_offset: Cell, - memory_length: MemoryAddress, - ) -> Self { - debug_assert_eq!( - CellType::StoragePhase2, - cb.curr.cell_manager.columns()[memory_offset.cell_column_index].cell_type - ); - let memory_length_is_zero = IsZeroGadget::construct(cb, sum::expr(&memory_length.cells)); - let memory_offset_bytes = cb.query_word_rlc(); - - let has_length = 1.expr() - memory_length_is_zero.expr(); - cb.condition(has_length, |cb| { - cb.require_equal( - "Offset decomposition into 5 bytes", - memory_offset_bytes.expr(), - memory_offset.expr(), - ); - }); - - Self { - memory_offset, - memory_offset_bytes, - memory_length, - memory_length_is_zero, - } - } - - pub(crate) fn construct_2(cb: &mut ConstraintBuilder) -> Self { +impl CommonMemoryAddressGadget for MemoryAddressGadget { + fn construct_self(cb: &mut ConstraintBuilder) -> Self { let offset = cb.query_cell_phase2(); let length = cb.query_word_rlc(); Self::construct(cb, offset, length) } - pub(crate) fn assign( + fn assign( &self, region: &mut CachedRegion<'_, '_, F>, offset: usize, @@ -155,146 +154,214 @@ impl MemoryAddressGadget { }) } - pub(crate) fn has_length(&self) -> Expression { - 1.expr() - self.memory_length_is_zero.expr() - } - - pub(crate) fn offset(&self) -> Expression { - self.has_length() * from_bytes::expr(&self.memory_offset_bytes.cells) - } - - pub(crate) fn offset_rlc(&self) -> Expression { + fn offset_rlc(&self) -> Expression { self.memory_offset.expr() } - pub(crate) fn length_rlc(&self) -> Expression { + fn length_rlc(&self) -> Expression { self.memory_length.expr() } - pub(crate) fn length(&self) -> Expression { + fn length(&self) -> Expression { from_bytes::expr(&self.memory_length.cells) } - pub(crate) fn address(&self) -> Expression { + fn address(&self) -> Expression { self.offset() + self.length() } } -/// Convert the dynamic memory offset and length from random linear combination to Uint64. It is -/// mainly used for memory expanded address in error execution states except Uint64 overflow. It -/// also checks with MAX_EXPANDED_MEMORY_ADDRESS for out of gas, and the returned expanded address -/// should be within the range of N_BYTES_MEMORY_WORD_SIZE which is constrained in other memory -/// gadgets (as MemoryWordSizeGadget). +impl MemoryAddressGadget { + pub(crate) fn construct( + cb: &mut ConstraintBuilder, + memory_offset: Cell, + memory_length: MemoryAddress, + ) -> Self { + debug_assert_eq!( + CellType::StoragePhase2, + cb.curr.cell_manager.columns()[memory_offset.cell_column_index].cell_type + ); + let memory_length_is_zero = IsZeroGadget::construct(cb, sum::expr(&memory_length.cells)); + let memory_offset_bytes = cb.query_word_rlc(); + + let has_length = 1.expr() - memory_length_is_zero.expr(); + cb.condition(has_length, |cb| { + cb.require_equal( + "Offset decomposition into 5 bytes", + memory_offset_bytes.expr(), + memory_offset.expr(), + ); + }); + + Self { + memory_offset, + memory_offset_bytes, + memory_length, + memory_length_is_zero, + } + } + + pub(crate) fn has_length(&self) -> Expression { + 1.expr() - self.memory_length_is_zero.expr() + } + + pub(crate) fn offset(&self) -> Expression { + self.has_length() * from_bytes::expr(&self.memory_offset_bytes.cells) + } +} + +/// Check if memory offset plus length is within range or Uint overflow. +/// The sum of memory offset and length should also be less than or equal to +/// `0x1FFFFFFFE0` (which is less than `u64::MAX - 31`). +/// Reference go-ethereum code as: +/// . [calcMemSize64WithUint](https://github.com/ethereum/go-ethereum/blob/db18293c32f6dc5d6886e5e68ab8bfd12e33cad6/core/vm/common.go#L37) +/// . [memoryGasCost](https://github.com/ethereum/go-ethereum/blob/db18293c32f6dc5d6886e5e68ab8bfd12e33cad6/core/vm/gas_table.go#L38) +/// . [toWordSize](https://github.com/ethereum/go-ethereum/blob/db18293c32f6dc5d6886e5e68ab8bfd12e33cad6/core/vm/common.go#L67) #[derive(Clone, Debug)] pub(crate) struct MemoryExpandedAddressGadget { - offset: WordByteRangeGadget, - length: WordByteRangeGadget, length_is_zero: IsZeroGadget, - // Offset plus length may be overflow for Uint64. - address_within_range: LtGadget, + offset_length_sum: AddWordsGadget, + sum_lt_cap: LtGadget, + sum_within_u64: IsZeroGadget, } -impl MemoryExpandedAddressGadget { - pub(crate) fn construct(cb: &mut ConstraintBuilder) -> Self { - let offset = WordByteRangeGadget::construct(cb); - let length = WordByteRangeGadget::construct(cb); +impl CommonMemoryAddressGadget for MemoryExpandedAddressGadget { + fn construct_self(cb: &mut ConstraintBuilder) -> Self { + let offset = cb.query_word_rlc(); + let length = cb.query_word_rlc(); + let sum = cb.query_word_rlc(); - cb.require_zero("Memory length cannot be Uint64 overflow", length.overflow()); - let length_is_zero = IsZeroGadget::construct(cb, length.valid_value()); + let sum_lt_cap = LtGadget::construct( + cb, + from_bytes::expr(&sum.cells[..N_BYTES_U64]), + (MAX_EXPANDED_MEMORY_ADDRESS + 1).expr(), + ); - cb.condition(not::expr(length_is_zero.expr()), |cb| { - cb.require_zero( - "Memory offset cannot be Uint64 overflow if length is non-zero", - offset.overflow(), - ); - }); + let sum_overflow_hi = sum::expr(&sum.cells[N_BYTES_U64..]); + let sum_within_u64 = IsZeroGadget::construct(cb, sum_overflow_hi); - let address = select::expr( - length_is_zero.expr(), - 0.expr(), - offset.valid_value() + length.valid_value(), - ); - let address_within_range = - LtGadget::construct(cb, address, (MAX_EXPANDED_MEMORY_ADDRESS + 1).expr()); + let length_is_zero = IsZeroGadget::construct(cb, sum::expr(&length.cells)); + let offset_length_sum = AddWordsGadget::construct(cb, [offset, length], sum); Self { - offset, - length, length_is_zero, - address_within_range, + offset_length_sum, + sum_lt_cap, + sum_within_u64, } } - pub(crate) fn assign( + fn assign( &self, region: &mut CachedRegion<'_, '_, F>, offset: usize, memory_offset: U256, memory_length: U256, ) -> Result { - self.offset.assign(region, offset, memory_offset)?; - self.length.assign(region, offset, memory_length)?; - - let memory_offset = memory_offset.low_u64(); - let memory_length = memory_length.low_u64(); + let length_bytes = memory_length + .to_le_bytes() + .iter() + .fold(0, |acc, val| acc + u64::from(*val)); self.length_is_zero - .assign(region, offset, memory_length.into())?; + .assign(region, offset, F::from(length_bytes))?; - // Address should be within 9 (N_BYTES_U64 + 1) bytes. - let address = if memory_length == 0 { - 0 - } else { - u128::from(memory_offset) + u128::from(memory_length) - }; - let address_cap = u128::from(MAX_EXPANDED_MEMORY_ADDRESS + 1); - self.address_within_range.assign( + let (sum, sum_word_overflow) = memory_offset.overflowing_add(memory_length); + self.offset_length_sum + .assign(region, offset, [memory_offset, memory_length], sum)?; + + self.sum_lt_cap.assign( region, offset, - F::from_u128(address), - F::from_u128(address_cap), + F::from(sum.low_u64()), + F::from(MAX_EXPANDED_MEMORY_ADDRESS + 1), )?; - // Return MAX_EXPANDED_MEMORY_ADDRESS if address overflow. - Ok(if address < address_cap { - address.try_into().unwrap() + let sum_overflow_hi_bytes = sum.to_le_bytes()[N_BYTES_U64..] + .iter() + .fold(0, |acc, val| acc + u64::from(*val)); + self.sum_within_u64 + .assign(region, offset, F::from(sum_overflow_hi_bytes))?; + + let address = if length_bytes == 0 + || sum_overflow_hi_bytes != 0 + || sum_word_overflow + || sum.low_u64() > MAX_EXPANDED_MEMORY_ADDRESS + { + 0 } else { - MAX_EXPANDED_MEMORY_ADDRESS - }) - } + sum.low_u64() + }; - /// Return word of memory offset. - pub(crate) fn offset_rlc(&self) -> Expression { - self.offset.original_word() + Ok(address) } - /// Return word of memory length. - pub(crate) fn length_rlc(&self) -> Expression { - self.length.original_word() + fn offset_rlc(&self) -> Expression { + let addends = self.offset_length_sum.addends(); + addends[0].expr() } - /// Return Uint64 of memory length. - pub(crate) fn length(&self) -> Expression { - self.length.valid_value() + fn length_rlc(&self) -> Expression { + let addends = self.offset_length_sum.addends(); + addends[1].expr() } - /// Return expanded address if within range, otherwise return MAX_EXPANDED_MEMORY_ADDRESS. - pub(crate) fn address(&self) -> Expression { - let address = select::expr( - self.length_is_zero.expr(), + fn length(&self) -> Expression { + let addends = self.offset_length_sum.addends(); + select::expr( + self.within_range(), + from_bytes::expr(&addends[1].cells[..N_BYTES_U64]), 0.expr(), - self.offset.valid_value() + self.length.valid_value(), - ); + ) + } + /// Return expanded address if within range, otherwise return 0. + fn address(&self) -> Expression { select::expr( - self.address_within_range.expr(), - address, - MAX_EXPANDED_MEMORY_ADDRESS.expr(), + self.length_is_zero.expr(), + 0.expr(), + select::expr( + self.within_range(), + from_bytes::expr(&self.offset_length_sum.sum().cells[..N_BYTES_U64]), + 0.expr(), + ), ) } +} + +impl MemoryExpandedAddressGadget { + /// Return the valid length value corresponding to function `length` + /// (which returns an Expression). + pub(crate) fn length_value(memory_offset: U256, memory_length: U256) -> u64 { + if memory_length.is_zero() { + return 0; + } + + memory_offset + .checked_add(memory_length) + .map_or(0, |address| { + if address > MAX_EXPANDED_MEMORY_ADDRESS.into() { + 0 + } else { + memory_length.as_u64() + } + }) + } + + /// Check if overflow. + pub(crate) fn overflow(&self) -> Expression { + not::expr(self.within_range()) + } - /// Check if expanded address is greater than MAX_EXPANDED_MEMORY_ADDRESS. - pub(crate) fn address_overflow(&self) -> Expression { - not::expr(self.address_within_range.expr()) + /// Check if within range. + pub(crate) fn within_range(&self) -> Expression { + or::expr([ + self.length_is_zero.expr(), + and::expr([ + self.sum_lt_cap.expr(), + self.sum_within_u64.expr(), + not::expr(self.offset_length_sum.carry().as_ref().unwrap()), + ]), + ]) } } diff --git a/zkevm-circuits/src/witness/step.rs b/zkevm-circuits/src/witness/step.rs index 07f606111e..2385e5b391 100644 --- a/zkevm-circuits/src/witness/step.rs +++ b/zkevm-circuits/src/witness/step.rs @@ -80,7 +80,6 @@ impl From<&ExecError> for ExecutionState { ExecutionState::ErrorCodeStore } ExecError::PrecompileFailed => ExecutionState::ErrorPrecompileFailed, - ExecError::GasUintOverflow => ExecutionState::ErrorGasUintOverflow, ExecError::OutOfGas(oog_error) => match oog_error { OogError::Constant => ExecutionState::ErrorOutOfGasConstant, OogError::StaticMemoryExpansion => {