From 0cfd5dd72d452a14af598a08669223b408ee9512 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Wed, 8 Feb 2023 22:00:27 +0800 Subject: [PATCH 1/3] Add OOG error state for `EXP`. --- bus-mapping/src/evm/opcodes.rs | 5 +- bus-mapping/src/evm/opcodes/error_oog_exp.rs | 36 +++ eth-types/src/evm_types.rs | 4 + zkevm-circuits/src/evm_circuit/execution.rs | 4 +- .../evm_circuit/execution/error_oog_exp.rs | 216 ++++++++++++++++++ .../src/evm_circuit/execution/exp.rs | 7 +- 6 files changed, 268 insertions(+), 4 deletions(-) create mode 100644 bus-mapping/src/evm/opcodes/error_oog_exp.rs create mode 100644 zkevm-circuits/src/evm_circuit/execution/error_oog_exp.rs diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index aaf813c4ad..4abad24c11 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -55,6 +55,7 @@ mod swap; mod error_invalid_jump; mod error_invalid_opcode; mod error_oog_call; +mod error_oog_exp; mod error_oog_log; mod error_oog_sload_sstore; mod error_return_data_outofbound; @@ -79,6 +80,7 @@ use dup::Dup; use error_invalid_jump::InvalidJump; use error_invalid_opcode::InvalidOpcode; use error_oog_call::OOGCall; +use error_oog_exp::OOGExp; use error_oog_log::ErrorOOGLog; use error_oog_sload_sstore::OOGSloadSstore; use error_return_data_outofbound::ErrorReturnDataOutOfBound; @@ -269,12 +271,13 @@ fn fn_gen_error_state_associated_ops(error: &ExecError) -> Option Some(InvalidOpcode::gen_associated_ops), ExecError::OutOfGas(OogError::Call) => Some(OOGCall::gen_associated_ops), ExecError::OutOfGas(OogError::Constant) => Some(ErrorStackOogConstant::gen_associated_ops), + ExecError::OutOfGas(OogError::Exp) => Some(OOGExp::gen_associated_ops), + ExecError::OutOfGas(OogError::Log) => Some(ErrorOOGLog::gen_associated_ops), ExecError::OutOfGas(OogError::SloadSstore) => Some(OOGSloadSstore::gen_associated_ops), ExecError::StackOverflow => Some(ErrorStackOogConstant::gen_associated_ops), ExecError::StackUnderflow => Some(ErrorStackOogConstant::gen_associated_ops), // call & callcode can encounter InsufficientBalance error, Use pop-7 generic CallOpcode ExecError::InsufficientBalance => Some(CallOpcode::<7>::gen_associated_ops), - ExecError::OutOfGas(OogError::Log) => Some(ErrorOOGLog::gen_associated_ops), ExecError::ReturnDataOutOfBounds => Some(ErrorReturnDataOutOfBound::gen_associated_ops), // more future errors place here diff --git a/bus-mapping/src/evm/opcodes/error_oog_exp.rs b/bus-mapping/src/evm/opcodes/error_oog_exp.rs new file mode 100644 index 0000000000..baee86cfde --- /dev/null +++ b/bus-mapping/src/evm/opcodes/error_oog_exp.rs @@ -0,0 +1,36 @@ +use super::{Opcode, OpcodeId}; +use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep}; +use crate::error::{ExecError, OogError}; +use crate::Error; +use eth_types::GethExecStep; + +/// Placeholder structure used to implement [`Opcode`] trait over it +/// corresponding to the [`OogError::Exp`](crate::error::OogError::Exp). +#[derive(Clone, Copy, Debug)] +pub(crate) struct OOGExp; + +impl Opcode for OOGExp { + fn gen_associated_ops( + state: &mut CircuitInputStateRef, + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + debug_assert_eq!(geth_step.op, OpcodeId::EXP); + + let mut exec_step = state.new_step(geth_step)?; + exec_step.error = Some(ExecError::OutOfGas(OogError::Exp)); + + for i in 0..2 { + state.stack_read( + &mut exec_step, + geth_step.stack.nth_last_filled(i), + geth_step.stack.nth_last(i)?, + )?; + } + + state.gen_restore_context_ops(&mut exec_step, geth_steps)?; + state.handle_return(geth_step)?; + + Ok(vec![exec_step]) + } +} diff --git a/eth-types/src/evm_types.rs b/eth-types/src/evm_types.rs index ddf30ffdf3..0a86aa8d99 100644 --- a/eth-types/src/evm_types.rs +++ b/eth-types/src/evm_types.rs @@ -136,6 +136,10 @@ impl GasCost { pub const MEMORY_EXPANSION_LINEAR_COEFF: Self = Self(3); /// Constant gas for LOG[0-4] op codes pub const LOG: Self = Self(375); + /// Constant gas for LOG[0-4] op codes + /// Times ceil exponent byte size for the EXP instruction, EIP-158 changed + /// it from 10 to 50. + pub const EXP_BYTE_TIMES: Self = Self(50); } impl GasCost { diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index 2b13d54277..8e588cbd25 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -64,6 +64,7 @@ mod error_invalid_jump; mod error_invalid_opcode; mod error_oog_call; mod error_oog_constant; +mod error_oog_exp; mod error_oog_log; mod error_oog_sload_sstore; mod error_oog_static_memory; @@ -133,6 +134,7 @@ use error_invalid_jump::ErrorInvalidJumpGadget; use error_invalid_opcode::ErrorInvalidOpcodeGadget; use error_oog_call::ErrorOOGCallGadget; use error_oog_constant::ErrorOOGConstantGadget; +use error_oog_exp::ErrorOOGExpGadget; use error_oog_log::ErrorOOGLogGadget; use error_oog_sload_sstore::ErrorOOGSloadSstoreGadget; use error_return_data_oo_bound::ErrorReturnDataOutOfBoundGadget; @@ -276,6 +278,7 @@ pub(crate) struct ExecutionConfig { // error gadgets error_oog_call: ErrorOOGCallGadget, error_oog_constant: ErrorOOGConstantGadget, + error_oog_exp: ErrorOOGExpGadget, error_oog_sload_sstore: ErrorOOGSloadSstoreGadget, error_oog_static_memory_gadget: DummyGadget, @@ -287,7 +290,6 @@ pub(crate) struct ExecutionConfig { error_oog_account_access: DummyGadget, error_oog_sha3: DummyGadget, error_oog_ext_codecopy: DummyGadget, - error_oog_exp: DummyGadget, error_oog_create2: DummyGadget, error_oog_self_destruct: DummyGadget, error_oog_code_store: DummyGadget, diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_exp.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_exp.rs new file mode 100644 index 0000000000..ea3dc1c506 --- /dev/null +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_exp.rs @@ -0,0 +1,216 @@ +use crate::evm_circuit::execution::ExecutionGadget; +use crate::evm_circuit::param::N_BYTES_GAS; +use crate::evm_circuit::step::ExecutionState; +use crate::evm_circuit::util::common_gadget::RestoreContextGadget; +use crate::evm_circuit::util::constraint_builder::Transition::{Delta, Same}; +use crate::evm_circuit::util::constraint_builder::{ConstraintBuilder, StepStateTransition}; +use crate::evm_circuit::util::math_gadget::{ByteSizeGadget, LtGadget}; +use crate::evm_circuit::util::{CachedRegion, Cell, Word}; +use crate::evm_circuit::witness::{Block, Call, ExecStep, Transaction}; +use crate::table::CallContextFieldTag; +use crate::util::Expr; +use eth_types::evm_types::{GasCost, OpcodeId}; +use eth_types::{Field, ToLittleEndian}; +use halo2_proofs::circuit::Value; +use halo2_proofs::plonk::Error; + +/// Gadget to implement the corresponding out of gas errors for +/// [`OpcodeId::EXP`]. +#[derive(Clone, Debug)] +pub(crate) struct ErrorOOGExpGadget { + opcode: Cell, + rw_counter_end_of_reversion: Cell, + base: Word, + exponent: Word, + exponent_byte_size: ByteSizeGadget, + insufficient_gas_cost: LtGadget, + restore_context: RestoreContextGadget, +} + +impl ExecutionGadget for ErrorOOGExpGadget { + const NAME: &'static str = "ErrorOutOfGasEXP"; + + const EXECUTION_STATE: ExecutionState = ExecutionState::ErrorOutOfGasEXP; + + fn configure(cb: &mut ConstraintBuilder) -> Self { + let opcode = cb.query_cell(); + cb.opcode_lookup(opcode.expr(), 1.expr()); + + cb.require_equal( + "ErrorOutOfGasEXP opcode must be EXP", + opcode.expr(), + OpcodeId::EXP.expr(), + ); + + let base = cb.query_word_rlc(); + let exponent = cb.query_word_rlc(); + cb.stack_pop(base.expr()); + cb.stack_pop(exponent.expr()); + + let exponent_byte_size = ByteSizeGadget::construct(cb, &exponent); + + let insufficient_gas_cost = LtGadget::construct( + cb, + cb.curr.state.gas_left.expr(), + // static_gas = 10 + // dynamic_gas = exponent_byte_size * 50 + // gas_cost = dynamic_gas + static_gas + exponent_byte_size.byte_size() * GasCost::EXP_BYTE_TIMES.0.expr() + + OpcodeId::EXP.constant_gas_cost().expr(), + ); + + cb.require_equal( + "Gas left is less than gas cost", + insufficient_gas_cost.expr(), + 1.expr(), + ); + + // Current call must fail. + cb.call_context_lookup(false.expr(), None, CallContextFieldTag::IsSuccess, 0.expr()); + + let rw_counter_end_of_reversion = cb.query_cell(); + cb.call_context_lookup( + false.expr(), + None, + CallContextFieldTag::RwCounterEndOfReversion, + rw_counter_end_of_reversion.expr(), + ); + + // Go to EndTx only when is_root. + let is_to_end_tx = cb.next.execution_state_selector([ExecutionState::EndTx]); + cb.require_equal( + "Go to EndTx only when is_root", + cb.curr.state.is_root.expr(), + is_to_end_tx, + ); + + // When it's a root call. + cb.condition(cb.curr.state.is_root.expr(), |cb| { + // Do step state transition. + cb.require_step_state_transition(StepStateTransition { + call_id: Same, + rw_counter: Delta(4.expr() + cb.curr.state.reversible_write_counter.expr()), + ..StepStateTransition::any() + }); + }); + + // When it's an internal call, need to restore caller's state as finishing this + // call. Restore caller state to next StepState. + let restore_context = cb.condition(1.expr() - cb.curr.state.is_root.expr(), |cb| { + RestoreContextGadget::construct( + cb, + 0.expr(), + 0.expr(), + 0.expr(), + 0.expr(), + 0.expr(), + 0.expr(), + ) + }); + + // Constrain RwCounterEndOfReversion. + let rw_counter_end_of_step = + cb.curr.state.rw_counter.expr() + cb.rw_counter_offset() - 1.expr(); + cb.require_equal( + "rw_counter_end_of_reversion = rw_counter_end_of_step + reversible_counter", + rw_counter_end_of_reversion.expr(), + rw_counter_end_of_step + cb.curr.state.reversible_write_counter.expr(), + ); + + Self { + opcode, + rw_counter_end_of_reversion, + base, + exponent, + exponent_byte_size, + insufficient_gas_cost, + restore_context, + } + } + + fn assign_exec_step( + &self, + region: &mut CachedRegion<'_, '_, F>, + offset: usize, + block: &Block, + _tx: &Transaction, + call: &Call, + step: &ExecStep, + ) -> Result<(), Error> { + let opcode = step.opcode.unwrap(); + let [base, exponent] = [0, 1].map(|idx| block.rws[step.rw_indices[idx]].stack_value()); + + log::debug!( + "ErrorOutOfGasEXP: gas_left = {}, gas_cost = {}", + step.gas_left, + step.gas_cost, + ); + + self.opcode + .assign(region, offset, Value::known(F::from(opcode.as_u64())))?; + self.rw_counter_end_of_reversion.assign( + region, + offset, + Value::known(F::from(call.rw_counter_end_of_reversion as u64)), + )?; + self.base.assign(region, offset, Some(base.to_le_bytes()))?; + self.exponent + .assign(region, offset, Some(exponent.to_le_bytes()))?; + self.exponent_byte_size.assign(region, offset, exponent)?; + self.insufficient_gas_cost.assign_value( + region, + offset, + Value::known(F::from(step.gas_left)), + Value::known(F::from(step.gas_cost)), + )?; + self.restore_context + .assign(region, offset, block, call, step, 4) + } +} + +#[cfg(test)] +mod tests { + use crate::evm_circuit::test::rand_word; + use crate::test_util::CircuitTestBuilder; + use eth_types::evm_types::{GasCost, OpcodeId}; + use eth_types::{bytecode, U256}; + use mock::test_ctx::helpers::account_0_code_account_1_no_code; + use mock::TestContext; + + #[test] + fn test_oog_exp() { + test_ok(U256::zero()); + test_ok(U256::one()); + test_ok(1023.into()); + test_ok(U256::MAX); + test_ok(rand_word()); + } + + fn test_ok(exponent: U256) { + let code = bytecode! { + PUSH32(exponent) + PUSH32(rand_word()) + EXP + }; + + let gas_cost = OpcodeId::PUSH32.constant_gas_cost().0 * 2 + + OpcodeId::EXP.constant_gas_cost().0 + + ((exponent.bits() as u64 + 7) / 8) * GasCost::EXP_BYTE_TIMES.0; + + let ctx = TestContext::<2, 1>::new( + None, + account_0_code_account_1_no_code(code), + |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 + gas_cost - 1).into()); + }, + |block, _tx| block.number(0xcafe_u64), + ) + .unwrap(); + + CircuitTestBuilder::new_from_test_ctx(ctx).run(); + } +} diff --git a/zkevm-circuits/src/evm_circuit/execution/exp.rs b/zkevm-circuits/src/evm_circuit/execution/exp.rs index f57dab3d2e..7e0288d88d 100644 --- a/zkevm-circuits/src/evm_circuit/execution/exp.rs +++ b/zkevm-circuits/src/evm_circuit/execution/exp.rs @@ -1,4 +1,5 @@ use bus_mapping::evm::OpcodeId; +use eth_types::evm_types::GasCost; use eth_types::{Field, ToLittleEndian, ToScalar, U256}; use gadgets::util::{and, not, split_u256, sum, Expr}; use halo2_proofs::{circuit::Value, plonk::Error}; @@ -174,13 +175,15 @@ impl ExecutionGadget for ExponentiationGadget { // bytes that can represent the exponent value. let exponent_byte_size = ByteSizeGadget::construct(cb, &exponent_rlc); - // Finally we build an expression for the dynamic gas cost. - let dynamic_gas_cost = 50.expr() * exponent_byte_size.byte_size(); + // Finally we build an expression for the dynamic gas cost as: + // dynamic_gas = 50 * exponent_byte_size + let dynamic_gas_cost = GasCost::EXP_BYTE_TIMES.0.expr() * exponent_byte_size.byte_size(); let step_state_transition = StepStateTransition { rw_counter: Transition::Delta(3.expr()), // 2 stack pops, 1 stack push program_counter: Transition::Delta(1.expr()), stack_pointer: Transition::Delta(1.expr()), gas_left: Transition::Delta( + // gas_cost = static_gas (10) + dynamic_gas -OpcodeId::EXP.constant_gas_cost().expr() - dynamic_gas_cost, ), ..Default::default() From 5ac5c10142043ad4c2207165b94c463f261f831e Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Thu, 23 Feb 2023 10:13:10 +0800 Subject: [PATCH 2/3] Delete mistake comment. --- eth-types/src/evm_types.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/eth-types/src/evm_types.rs b/eth-types/src/evm_types.rs index 0a86aa8d99..90a74ce8ae 100644 --- a/eth-types/src/evm_types.rs +++ b/eth-types/src/evm_types.rs @@ -136,7 +136,6 @@ impl GasCost { pub const MEMORY_EXPANSION_LINEAR_COEFF: Self = Self(3); /// Constant gas for LOG[0-4] op codes pub const LOG: Self = Self(375); - /// Constant gas for LOG[0-4] op codes /// Times ceil exponent byte size for the EXP instruction, EIP-158 changed /// it from 10 to 50. pub const EXP_BYTE_TIMES: Self = Self(50); From 7c7c7dc3ad76562c709916f6d444fe8f2b5a1b87 Mon Sep 17 00:00:00 2001 From: Steven Gu Date: Thu, 23 Feb 2023 10:48:19 +0800 Subject: [PATCH 3/3] Add test cases for internal call. --- .../evm_circuit/execution/error_oog_exp.rs | 101 ++++++++++++++---- 1 file changed, 82 insertions(+), 19 deletions(-) diff --git a/zkevm-circuits/src/evm_circuit/execution/error_oog_exp.rs b/zkevm-circuits/src/evm_circuit/execution/error_oog_exp.rs index ea3dc1c506..40baa8bb76 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_oog_exp.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_oog_exp.rs @@ -170,42 +170,62 @@ impl ExecutionGadget for ErrorOOGExpGadget { #[cfg(test)] mod tests { - use crate::evm_circuit::test::rand_word; + use crate::evm_circuit::test::{rand_bytes, rand_word}; use crate::test_util::CircuitTestBuilder; use eth_types::evm_types::{GasCost, OpcodeId}; - use eth_types::{bytecode, U256}; + use eth_types::{bytecode, Bytecode, ToWord, U256}; use mock::test_ctx::helpers::account_0_code_account_1_no_code; - use mock::TestContext; + use mock::{eth, TestContext, MOCK_ACCOUNTS}; #[test] fn test_oog_exp() { - test_ok(U256::zero()); - test_ok(U256::one()); - test_ok(1023.into()); - test_ok(U256::MAX); - test_ok(rand_word()); + [ + U256::zero(), + U256::one(), + 1023.into(), + U256::MAX, + rand_word(), + ] + .into_iter() + .for_each(|exponent| { + let testing_data = TestingData::new(exponent); + + test_root(&testing_data); + test_internal(&testing_data); + }) } - fn test_ok(exponent: U256) { - let code = bytecode! { - PUSH32(exponent) - PUSH32(rand_word()) - EXP - }; + struct TestingData { + bytecode: Bytecode, + gas_cost: u64, + } - let gas_cost = OpcodeId::PUSH32.constant_gas_cost().0 * 2 - + OpcodeId::EXP.constant_gas_cost().0 - + ((exponent.bits() as u64 + 7) / 8) * GasCost::EXP_BYTE_TIMES.0; + impl TestingData { + pub fn new(exponent: U256) -> Self { + let bytecode = bytecode! { + PUSH32(exponent) + PUSH32(rand_word()) + EXP + }; + let gas_cost = OpcodeId::PUSH32.constant_gas_cost().0 * 2 + + OpcodeId::EXP.constant_gas_cost().0 + + ((exponent.bits() as u64 + 7) / 8) * GasCost::EXP_BYTE_TIMES.0; + + Self { bytecode, gas_cost } + } + } + + fn test_root(testing_data: &TestingData) { let ctx = TestContext::<2, 1>::new( None, - account_0_code_account_1_no_code(code), + 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 + gas_cost - 1).into()); + .gas((GasCost::TX.0 + testing_data.gas_cost - 1).into()); }, |block, _tx| block.number(0xcafe_u64), ) @@ -213,4 +233,47 @@ mod tests { CircuitTestBuilder::new_from_test_ctx(ctx).run(); } + + fn test_internal(testing_data: &TestingData) { + let (addr_a, addr_b) = (MOCK_ACCOUNTS[0], MOCK_ACCOUNTS[1]); + + // code B gets called by code A, so the call is an internal call. + let code_b = testing_data.bytecode.clone(); + let gas_cost_b = testing_data.gas_cost; + + // Code A calls code B. + let code_a = bytecode! { + // populate memory in A's context. + PUSH8(U256::from_big_endian(&rand_bytes(8))) + PUSH1(0x00) // offset + MSTORE + // call ADDR_B. + PUSH1(0x00) // retLength + PUSH1(0x00) // retOffset + PUSH32(0x00) // argsLength + PUSH32(0x20) // argsOffset + PUSH1(0x00) // value + PUSH32(addr_b.to_word()) // addr + // Decrease expected gas cost (by 1) to trigger out of gas error. + PUSH32(gas_cost_b - 1) // gas + CALL + STOP + }; + + let ctx = TestContext::<3, 1>::new( + None, + |accs| { + accs[0].address(addr_b).code(code_b); + accs[1].address(addr_a).code(code_a); + accs[2].address(MOCK_ACCOUNTS[2]).balance(eth(10)); + }, + |mut txs, accs| { + txs[0].from(accs[2].address).to(accs[1].address); + }, + |block, _tx| block, + ) + .unwrap(); + + CircuitTestBuilder::new_from_test_ctx(ctx).run(); + } }