From 6cb1a069a21b8f067337bad71d5d69c074025d4f Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Tue, 28 Feb 2023 15:35:25 +0800 Subject: [PATCH 1/9] circuit for maxcodesize exceed & code store oog --- .../circuit_input_builder/input_state_ref.rs | 9 +- bus-mapping/src/evm/opcodes.rs | 9 +- .../src/evm/opcodes/error_code_store.rs | 69 ++++ zkevm-circuits/src/evm_circuit/execution.rs | 14 +- .../evm_circuit/execution/error_code_store.rs | 301 ++++++++++++++++++ zkevm-circuits/src/evm_circuit/step.rs | 6 +- .../evm_circuit/util/constraint_builder.rs | 4 + zkevm-circuits/src/witness/step.rs | 7 +- 8 files changed, 401 insertions(+), 18 deletions(-) create mode 100644 bus-mapping/src/evm/opcodes/error_code_store.rs create mode 100644 zkevm-circuits/src/evm_circuit/execution/error_code_store.rs diff --git a/bus-mapping/src/circuit_input_builder/input_state_ref.rs b/bus-mapping/src/circuit_input_builder/input_state_ref.rs index 7e7051cd6b..d44379442d 100644 --- a/bus-mapping/src/circuit_input_builder/input_state_ref.rs +++ b/bus-mapping/src/circuit_input_builder/input_state_ref.rs @@ -1003,8 +1003,13 @@ impl<'a> CircuitInputStateRef<'a> { }; let gas_refund = geth_step.gas.0 - memory_expansion_gas_cost - code_deposit_cost; - let caller_gas_left = geth_step_next.gas.0 - gas_refund; - + //let caller_gas_left = geth_step_next.gas.0 - gas_refund; + let caller_gas_left = if !call.is_success && geth_step.op == OpcodeId::RETURN { + geth_step_next.gas.0 + } else { + geth_step_next.gas.0 - gas_refund + }; + for (field, value) in [ (CallContextField::IsRoot, (caller.is_root as u64).into()), ( diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index 9ceb65bdbf..3e99601327 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -63,6 +63,7 @@ mod error_oog_sload_sstore; mod error_return_data_outofbound; mod error_stack_oog_constant; mod error_write_protection; +mod error_code_store; #[cfg(test)] mod memory_expansion_test; @@ -89,6 +90,8 @@ use error_oog_sload_sstore::OOGSloadSstore; use error_return_data_outofbound::ErrorReturnDataOutOfBound; use error_stack_oog_constant::ErrorStackOogConstant; use error_write_protection::ErrorWriteProtection; +use error_code_store::ErrorCodeStore; + use exp::Exponentiation; use extcodecopy::Extcodecopy; use extcodehash::Extcodehash; @@ -255,7 +258,7 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { DummySelfDestruct::gen_associated_ops } OpcodeId::CREATE => { - evm_unimplemented!("Using dummy gen_create_ops for opcode {:?}", opcode_id); + //evm_unimplemented!("Using dummy gen_create_ops for opcode {:?}", opcode_id); DummyCreate::::gen_associated_ops } OpcodeId::CREATE2 => { @@ -284,10 +287,12 @@ fn fn_gen_error_state_associated_ops(error: &ExecError) -> Option Some(CallOpcode::<7>::gen_associated_ops), ExecError::WriteProtection => Some(ErrorWriteProtection::gen_associated_ops), ExecError::ReturnDataOutOfBounds => Some(ErrorReturnDataOutOfBound::gen_associated_ops), + ExecError::OutOfGas(OogError::CodeStore) => Some(ErrorCodeStore::gen_associated_ops), + ExecError::MaxCodeSizeExceeded => Some(ErrorCodeStore::gen_associated_ops), // more future errors place here _ => { - evm_unimplemented!("TODO: error state {:?} not implemented", error); + //evm_unimplemented!("TODO: error state {:?} not implemented", error); None } } diff --git a/bus-mapping/src/evm/opcodes/error_code_store.rs b/bus-mapping/src/evm/opcodes/error_code_store.rs new file mode 100644 index 0000000000..fa543797c1 --- /dev/null +++ b/bus-mapping/src/evm/opcodes/error_code_store.rs @@ -0,0 +1,69 @@ +use crate::{ + circuit_input_builder::{CircuitInputStateRef, ExecStep}, + error::ExecError, + evm::Opcode, + operation::CallContextField, + Error, +}; +use eth_types::GethExecStep; + +#[derive(Debug, Copy, Clone)] +pub struct ErrorCodeStore; + +impl Opcode for ErrorCodeStore { + fn gen_associated_ops( + state: &mut CircuitInputStateRef, + geth_steps: &[GethExecStep], + ) -> Result, Error> { + let geth_step = &geth_steps[0]; + let mut exec_step = state.new_step(geth_step)?; + let next_step = geth_steps.get(1); + + exec_step.error = state.get_step_err(geth_step, next_step)?; + + assert!( + exec_step.error == Some(ExecError::CodeStoreOutOfGas) + || exec_step.error == Some(ExecError::MaxCodeSizeExceeded) + ); + + let offset = geth_step.stack.nth_last(0)?; + let length = geth_step.stack.nth_last(1)?; + state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(0), offset)?; + state.stack_read(&mut exec_step, geth_step.stack.nth_last_filled(1), length)?; + + // in internal call context + let call = state.call()?; + let call_id = call.call_id; + let is_success = call.is_success; + assert!(call.is_create() && !call.is_root); + + // must be in create context + state.call_context_read( + &mut exec_step, + call_id, + CallContextField::IsCreate, + (call.is_create() as u64).into(), + ); + + state.call_context_read( + &mut exec_step, + call_id, + CallContextField::IsSuccess, + (is_success as u64).into(), + ); + + state.call_context_read( + &mut exec_step, + call_id, + CallContextField::RwCounterEndOfReversion, + 0u64.into(), + ); + + // refer to return_revert Case C + state.handle_restore_context(geth_steps, &mut exec_step)?; + + //state.gen_restore_context_ops(&mut exec_step, geth_steps)?; + state.handle_return(geth_step)?; + Ok(vec![exec_step]) + } +} diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index c4ba3743c8..860a7275fe 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -71,6 +71,8 @@ mod error_oog_static_memory; mod error_return_data_oo_bound; mod error_stack; mod error_write_protection; +mod error_code_store; + mod exp; mod extcodecopy; mod extcodehash; @@ -141,6 +143,8 @@ use error_oog_sload_sstore::ErrorOOGSloadSstoreGadget; use error_return_data_oo_bound::ErrorReturnDataOutOfBoundGadget; use error_stack::ErrorStackGadget; use error_write_protection::ErrorWriteProtectionGadget; +use error_code_store::ErrorCodeStoreGadget; + use exp::ExponentiationGadget; use extcodecopy::ExtcodecopyGadget; use extcodehash::ExtcodehashGadget; @@ -296,7 +300,7 @@ pub(crate) struct ExecutionConfig { error_oog_ext_codecopy: DummyGadget, error_oog_create2: DummyGadget, error_oog_self_destruct: DummyGadget, - error_oog_code_store: DummyGadget, + error_code_store: ErrorCodeStoreGadget, error_insufficient_balance: DummyGadget, error_invalid_jump: ErrorInvalidJumpGadget, error_invalid_opcode: ErrorInvalidOpcodeGadget, @@ -550,7 +554,7 @@ impl ExecutionConfig { error_oog_exp: configure_gadget!(), error_oog_create2: configure_gadget!(), error_oog_self_destruct: configure_gadget!(), - error_oog_code_store: configure_gadget!(), + error_code_store: configure_gadget!(), error_insufficient_balance: configure_gadget!(), error_invalid_jump: configure_gadget!(), error_invalid_opcode: configure_gadget!(), @@ -1223,8 +1227,8 @@ impl ExecutionConfig { assign_exec_step!(self.error_oog_self_destruct) } - ExecutionState::ErrorOutOfGasCodeStore => { - assign_exec_step!(self.error_oog_code_store) + ExecutionState::ErrorCodeStore => { + assign_exec_step!(self.error_code_store) } ExecutionState::ErrorStack => { assign_exec_step!(self.error_stack) @@ -1254,8 +1258,6 @@ impl ExecutionConfig { ExecutionState::ErrorReturnDataOutOfBound => { assign_exec_step!(self.error_return_data_out_of_bound) } - - _ => evm_unimplemented!("unimplemented ExecutionState: {:?}", step.execution_state), } // Fill in the witness values for stored expressions diff --git a/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs b/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs new file mode 100644 index 0000000000..215ddfda0c --- /dev/null +++ b/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs @@ -0,0 +1,301 @@ +use crate::{ + evm_circuit::{ + execution::ExecutionGadget, + param::{N_BYTES_GAS, N_BYTES_U64}, + step::ExecutionState, + util::{ + common_gadget::RestoreContextGadget, constraint_builder::ConstraintBuilder, + math_gadget::LtGadget, memory_gadget::MemoryAddressGadget, CachedRegion, Cell, + }, + witness::{Block, Call, ExecStep, Transaction}, + }, + table::CallContextFieldTag, + util::Expr, +}; + +use eth_types::{evm_types::GasCost, Field}; +use halo2_proofs::{circuit::Value, plonk::Error}; + +const MAXCODESIZE: u64 = 0x6000u64; + +/// Gadget for code store oog and max code size exceed +#[derive(Clone, Debug)] +pub(crate) struct ErrorCodeStoreGadget { + opcode: Cell, + is_create: Cell, + memory_address: MemoryAddressGadget, + // check for CodeStoreOutOfGas error + code_store_gas_insufficient: LtGadget, + // check for MaxCodeSizeExceeded error + max_code_size_exceed: LtGadget, + rw_counter_end_of_reversion: Cell, + restore_context: RestoreContextGadget, +} + +impl ExecutionGadget for ErrorCodeStoreGadget { + const NAME: &'static str = "ErrorCodeStore"; + + const EXECUTION_STATE: ExecutionState = ExecutionState::ErrorCodeStore; + + fn configure(cb: &mut ConstraintBuilder) -> Self { + let opcode = cb.query_cell(); + cb.opcode_lookup(opcode.expr(), 1.expr()); + + let offset = cb.query_cell_phase2(); + let length = cb.query_word_rlc(); + let rw_counter_end_of_reversion = cb.query_cell(); + + cb.stack_pop(offset.expr()); + cb.stack_pop(length.expr()); + let memory_address = MemoryAddressGadget::construct(cb, offset, length); + + let is_create = cb.call_context(None, CallContextFieldTag::IsCreate); + cb.require_true("is_create is true", is_create.expr()); + + // constrain code store gas > gas left, that is GasCost::CODE_DEPOSIT_BYTE_COST + // * length > gas left + let code_store_gas_insufficient = LtGadget::construct( + cb, + cb.curr.state.gas_left.expr(), + GasCost::CODE_DEPOSIT_BYTE_COST.expr() * memory_address.length(), + ); + + // constrain code size > MAXCODESIZE + let max_code_size_exceed = + LtGadget::construct(cb, MAXCODESIZE.expr(), memory_address.length()); + + // check must be one of CodeStoreOutOfGas or MaxCodeSizeExceeded + cb.require_in_set( + "CodeStoreOutOfGas or MaxCodeSizeExceeded", + code_store_gas_insufficient.expr() + max_code_size_exceed.expr(), + vec![1.expr(), 2.expr()], + ); + + // restore context as in internal call + cb.require_zero("in internal call", cb.curr.state.is_root.expr()); + + cb.call_context_lookup(false.expr(), None, CallContextFieldTag::IsSuccess, 0.expr()); + cb.call_context_lookup( + false.expr(), + None, + CallContextFieldTag::RwCounterEndOfReversion, + rw_counter_end_of_reversion.expr(), + ); + + // Case C in the return specs. + let restore_context = RestoreContextGadget::construct( + cb, + 0.expr(), + 0.expr(), + memory_address.offset(), + memory_address.length(), + 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, + is_create, + memory_address, + code_store_gas_insufficient, + max_code_size_exceed, + rw_counter_end_of_reversion, + 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(); + self.opcode + .assign(region, offset, Value::known(F::from(opcode.as_u64())))?; + + let [memory_offset, length] = [0, 1].map(|i| block.rws[step.rw_indices[i]].stack_value()); + self.memory_address + .assign(region, offset, memory_offset, length)?; + + self.is_create + .assign(region, offset, Value::known(F::from(call.is_create as u64)))?; + self.code_store_gas_insufficient.assign( + region, + offset, + F::from(step.gas_left), + F::from(GasCost::CODE_DEPOSIT_BYTE_COST.as_u64() * length.as_u64()), + )?; + + self.max_code_size_exceed.assign( + region, + offset, + F::from(MAXCODESIZE), + F::from(length.as_u64()), + )?; + + self.rw_counter_end_of_reversion.assign( + region, + offset, + Value::known(F::from(call.rw_counter_end_of_reversion as u64)), + )?; + self.restore_context + .assign(region, offset, block, call, step, 5)?; + Ok(()) + } +} + +#[cfg(test)] +mod test { + use bus_mapping::circuit_input_builder::CircuitsParams; + use eth_types::{ + address, bytecode, evm_types::OpcodeId, geth_types::Account, Address, Bytecode, Word, + }; + + use lazy_static::lazy_static; + use mock::{eth, TestContext}; + + use crate::test_util::CircuitTestBuilder; + + const CALLEE_ADDRESS: Address = Address::repeat_byte(0xff); + lazy_static! { + static ref CALLER_ADDRESS: Address = address!("0x00bbccddee000000000000000000000000002400"); + } + + const MAXCODESIZE: u64 = 0x6000u64; + + fn run_test_circuits(ctx: TestContext<2, 1>) { + CircuitTestBuilder::new_from_test_ctx(ctx) + .params(CircuitsParams { + max_rws: 4500, + ..Default::default() + }) + .run(); + } + + fn initialization_bytecode(is_oog: bool) -> Bytecode { + let memory_bytes = [0x60; 10]; + let memory_value = Word::from_big_endian(&memory_bytes); + let code_len = if is_oog { 0 } else { MAXCODESIZE + 1 }; + + let mut code = bytecode! { + PUSH10(memory_value) + PUSH32(code_len) + MSTORE + PUSH2(if is_oog { 5 } else { code_len}) // length to copy + PUSH2(32u64 - u64::try_from(memory_bytes.len()).unwrap()) // offset + //PUSH2(0x00) // offset + + }; + code.write_op(OpcodeId::RETURN); + + code + } + + fn creator_bytecode(initialization_bytecode: Bytecode, is_create2: bool) -> Bytecode { + let initialization_bytes = initialization_bytecode.code(); + let mut code = Bytecode::default(); + + //construct maxcodesize + 1 memory bytes + let code_creator: Vec = initialization_bytes + .to_vec() + .iter() + .cloned() + .chain(0u8..((32 - initialization_bytes.len() % 32) as u8)) + .collect(); + for (index, word) in code_creator.chunks(32).enumerate() { + code.push(32, Word::from_big_endian(word)); + code.push(32, Word::from(index * 32)); + code.write_op(OpcodeId::MSTORE); + } + + if is_create2 { + code.append(&bytecode! {PUSH1(45)}); // salt; + } + code.append(&bytecode! { + PUSH32(initialization_bytes.len()) // size + PUSH2(0x00) // offset + PUSH2(23414) // value + }); + code.write_op(if is_create2 { + OpcodeId::CREATE2 + } else { + OpcodeId::CREATE + }); + code.append(&bytecode! { + PUSH1(0) + PUSH1(0) + RETURN + }); + + code + } + + fn test_context(caller: Account, is_oog: bool) -> TestContext<2, 1> { + TestContext::new( + None, + |accs| { + accs[0] + .address(address!("0x000000000000000000000000000000000000cafe")) + .balance(eth(10)); + accs[1].account(&caller); + }, + |mut txs, accs| { + txs[0] + .from(accs[0].address) + .to(accs[1].address) + .gas(if is_oog { + 53800u64.into() + } else { + 103800u64.into() + }); + }, + |block, _| block, + ) + .unwrap() + } + + #[test] + fn test_create_codestore_oog() { + for is_create2 in [false, true] { + let initialization_code = initialization_bytecode(true); + let root_code = creator_bytecode(initialization_code, is_create2); + let caller = Account { + address: *CALLER_ADDRESS, + code: root_code.into(), + nonce: Word::one(), + balance: eth(10), + ..Default::default() + }; + run_test_circuits(test_context(caller, true)); + } + } + + #[test] + fn test_create_max_code_size_exceed() { + for is_create2 in [false, true] { + let initialization_code = initialization_bytecode(false); + let root_code = creator_bytecode(initialization_code, is_create2); + let caller = Account { + address: *CALLER_ADDRESS, + code: root_code.into(), + nonce: Word::one(), + balance: eth(10), + ..Default::default() + }; + run_test_circuits(test_context(caller, false)); + } + } +} diff --git a/zkevm-circuits/src/evm_circuit/step.rs b/zkevm-circuits/src/evm_circuit/step.rs index 6391d38f6a..c2dd8d7f8b 100644 --- a/zkevm-circuits/src/evm_circuit/step.rs +++ b/zkevm-circuits/src/evm_circuit/step.rs @@ -93,7 +93,7 @@ pub enum ExecutionState { ErrorInsufficientBalance, ErrorContractAddressCollision, ErrorInvalidCreationCode, - ErrorMaxCodeSizeExceeded, + ErrorCodeStore, ErrorInvalidJump, ErrorReturnDataOutOfBound, ErrorOutOfGasConstant, @@ -101,7 +101,6 @@ pub enum ExecutionState { ErrorOutOfGasDynamicMemoryExpansion, ErrorOutOfGasMemoryCopy, ErrorOutOfGasAccountAccess, - ErrorOutOfGasCodeStore, ErrorOutOfGasLOG, ErrorOutOfGasEXP, ErrorOutOfGasSHA3, @@ -143,7 +142,7 @@ impl ExecutionState { | Self::ErrorInsufficientBalance | Self::ErrorContractAddressCollision | Self::ErrorInvalidCreationCode - | Self::ErrorMaxCodeSizeExceeded + | Self::ErrorCodeStore | Self::ErrorInvalidJump | Self::ErrorReturnDataOutOfBound | Self::ErrorOutOfGasConstant @@ -151,7 +150,6 @@ impl ExecutionState { | Self::ErrorOutOfGasDynamicMemoryExpansion | Self::ErrorOutOfGasMemoryCopy | Self::ErrorOutOfGasAccountAccess - | Self::ErrorOutOfGasCodeStore | Self::ErrorOutOfGasLOG | Self::ErrorOutOfGasEXP | Self::ErrorOutOfGasSHA3 diff --git a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs index 6af4916f81..73b1480a0d 100644 --- a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs +++ b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs @@ -438,6 +438,10 @@ impl<'a, F: Field> ConstraintBuilder<'a, F> { self.add_constraint(name, constraint); } + pub(crate) fn require_true(&mut self, name: &'static str, constraint: Expression) { + self.require_equal(name, constraint, 1.expr()); + } + pub(crate) fn require_equal( &mut self, name: &'static str, diff --git a/zkevm-circuits/src/witness/step.rs b/zkevm-circuits/src/witness/step.rs index d41ad5c341..ae9b0c2969 100644 --- a/zkevm-circuits/src/witness/step.rs +++ b/zkevm-circuits/src/witness/step.rs @@ -66,8 +66,7 @@ impl From<&ExecError> for ExecutionState { ExecError::InvalidCreationCode => ExecutionState::ErrorInvalidCreationCode, ExecError::InvalidJump => ExecutionState::ErrorInvalidJump, ExecError::ReturnDataOutOfBounds => ExecutionState::ErrorReturnDataOutOfBound, - ExecError::CodeStoreOutOfGas => ExecutionState::ErrorOutOfGasCodeStore, - ExecError::MaxCodeSizeExceeded => ExecutionState::ErrorMaxCodeSizeExceeded, + ExecError::CodeStoreOutOfGas | ExecError::MaxCodeSizeExceeded => ExecutionState::ErrorCodeStore, ExecError::OutOfGas(oog_error) => match oog_error { OogError::Constant => ExecutionState::ErrorOutOfGasConstant, OogError::StaticMemoryExpansion => { @@ -78,7 +77,7 @@ impl From<&ExecError> for ExecutionState { } OogError::MemoryCopy => ExecutionState::ErrorOutOfGasMemoryCopy, OogError::AccountAccess => ExecutionState::ErrorOutOfGasAccountAccess, - OogError::CodeStore => ExecutionState::ErrorOutOfGasCodeStore, + OogError::CodeStore => ExecutionState::ErrorCodeStore, OogError::Log => ExecutionState::ErrorOutOfGasLOG, OogError::Exp => ExecutionState::ErrorOutOfGasEXP, OogError::Sha3 => ExecutionState::ErrorOutOfGasSHA3, @@ -114,7 +113,7 @@ impl From<&circuit_input_builder::ExecStep> for ExecutionState { macro_rules! dummy { ($name:expr) => {{ - evm_unimplemented!("{:?} is implemented with DummyGadget", $name); + //evm_unimplemented!("{:?} is implemented with DummyGadget", $name); $name }}; } From 1b933eb2cf2876ae14f3a8fcd1e30135a48c0cd1 Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Wed, 1 Mar 2023 10:22:55 +0800 Subject: [PATCH 2/9] use step's is_create to avoid state circuit fail --- bus-mapping/src/evm/opcodes.rs | 11 +++++------ bus-mapping/src/evm/opcodes/error_code_store.rs | 10 ++-------- zkevm-circuits/src/evm_circuit/execution.rs | 6 +++--- .../src/evm_circuit/execution/error_code_store.rs | 9 ++------- .../src/evm_circuit/util/constraint_builder.rs | 2 +- zkevm-circuits/src/witness/step.rs | 5 +++-- 6 files changed, 16 insertions(+), 27 deletions(-) diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index 3e99601327..6a1e1eca39 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -54,6 +54,7 @@ mod stackonlyop; mod stop; mod swap; +mod error_code_store; mod error_invalid_jump; mod error_invalid_opcode; mod error_oog_call; @@ -63,7 +64,6 @@ mod error_oog_sload_sstore; mod error_return_data_outofbound; mod error_stack_oog_constant; mod error_write_protection; -mod error_code_store; #[cfg(test)] mod memory_expansion_test; @@ -81,6 +81,7 @@ use codecopy::Codecopy; use codesize::Codesize; use create::DummyCreate; use dup::Dup; +use error_code_store::ErrorCodeStore; use error_invalid_jump::InvalidJump; use error_invalid_opcode::InvalidOpcode; use error_oog_call::OOGCall; @@ -90,7 +91,6 @@ use error_oog_sload_sstore::OOGSloadSstore; use error_return_data_outofbound::ErrorReturnDataOutOfBound; use error_stack_oog_constant::ErrorStackOogConstant; use error_write_protection::ErrorWriteProtection; -use error_code_store::ErrorCodeStore; use exp::Exponentiation; use extcodecopy::Extcodecopy; @@ -287,7 +287,7 @@ fn fn_gen_error_state_associated_ops(error: &ExecError) -> Option Some(CallOpcode::<7>::gen_associated_ops), ExecError::WriteProtection => Some(ErrorWriteProtection::gen_associated_ops), ExecError::ReturnDataOutOfBounds => Some(ErrorReturnDataOutOfBound::gen_associated_ops), - ExecError::OutOfGas(OogError::CodeStore) => Some(ErrorCodeStore::gen_associated_ops), + ExecError::CodeStoreOutOfGas => Some(ErrorCodeStore::gen_associated_ops), ExecError::MaxCodeSizeExceeded => Some(ErrorCodeStore::gen_associated_ops), // more future errors place here @@ -324,10 +324,9 @@ pub fn gen_associated_ops( None }; if let Some(exec_error) = state.get_step_err(geth_step, next_step).unwrap() { - log::warn!( + println!( "geth error {:?} occurred in {:?}", - exec_error, - geth_step.op + exec_error, geth_step.op ); exec_step.error = Some(exec_error.clone()); diff --git a/bus-mapping/src/evm/opcodes/error_code_store.rs b/bus-mapping/src/evm/opcodes/error_code_store.rs index fa543797c1..bf5ac4d517 100644 --- a/bus-mapping/src/evm/opcodes/error_code_store.rs +++ b/bus-mapping/src/evm/opcodes/error_code_store.rs @@ -35,15 +35,9 @@ impl Opcode for ErrorCodeStore { let call = state.call()?; let call_id = call.call_id; let is_success = call.is_success; - assert!(call.is_create() && !call.is_root); - // must be in create context - state.call_context_read( - &mut exec_step, - call_id, - CallContextField::IsCreate, - (call.is_create() as u64).into(), - ); + // create context check + assert!(call.is_create() && !call.is_root); state.call_context_read( &mut exec_step, diff --git a/zkevm-circuits/src/evm_circuit/execution.rs b/zkevm-circuits/src/evm_circuit/execution.rs index 860a7275fe..6160e433ad 100644 --- a/zkevm-circuits/src/evm_circuit/execution.rs +++ b/zkevm-circuits/src/evm_circuit/execution.rs @@ -20,7 +20,7 @@ use crate::{ table::LookupTable, util::{query_expression, Challenges, Expr}, }; -use eth_types::{evm_unimplemented, Field}; +use eth_types::Field; use gadgets::util::not; use halo2_proofs::{ arithmetic::FieldExt, @@ -60,6 +60,7 @@ mod dummy; mod dup; mod end_block; mod end_tx; +mod error_code_store; mod error_invalid_jump; mod error_invalid_opcode; mod error_oog_call; @@ -71,7 +72,6 @@ mod error_oog_static_memory; mod error_return_data_oo_bound; mod error_stack; mod error_write_protection; -mod error_code_store; mod exp; mod extcodecopy; @@ -133,6 +133,7 @@ use dummy::DummyGadget; use dup::DupGadget; use end_block::EndBlockGadget; use end_tx::EndTxGadget; +use error_code_store::ErrorCodeStoreGadget; use error_invalid_jump::ErrorInvalidJumpGadget; use error_invalid_opcode::ErrorInvalidOpcodeGadget; use error_oog_call::ErrorOOGCallGadget; @@ -143,7 +144,6 @@ use error_oog_sload_sstore::ErrorOOGSloadSstoreGadget; use error_return_data_oo_bound::ErrorReturnDataOutOfBoundGadget; use error_stack::ErrorStackGadget; use error_write_protection::ErrorWriteProtectionGadget; -use error_code_store::ErrorCodeStoreGadget; use exp::ExponentiationGadget; use extcodecopy::ExtcodecopyGadget; 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 215ddfda0c..74b3cf3152 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs @@ -22,7 +22,6 @@ const MAXCODESIZE: u64 = 0x6000u64; #[derive(Clone, Debug)] pub(crate) struct ErrorCodeStoreGadget { opcode: Cell, - is_create: Cell, memory_address: MemoryAddressGadget, // check for CodeStoreOutOfGas error code_store_gas_insufficient: LtGadget, @@ -49,8 +48,7 @@ impl ExecutionGadget for ErrorCodeStoreGadget { cb.stack_pop(length.expr()); let memory_address = MemoryAddressGadget::construct(cb, offset, length); - let is_create = cb.call_context(None, CallContextFieldTag::IsCreate); - cb.require_true("is_create is true", is_create.expr()); + cb.require_true("is_create is true", cb.curr.state.is_create.expr()); // constrain code store gas > gas left, that is GasCost::CODE_DEPOSIT_BYTE_COST // * length > gas left @@ -104,7 +102,6 @@ impl ExecutionGadget for ErrorCodeStoreGadget { Self { opcode, - is_create, memory_address, code_store_gas_insufficient, max_code_size_exceed, @@ -130,8 +127,6 @@ impl ExecutionGadget for ErrorCodeStoreGadget { self.memory_address .assign(region, offset, memory_offset, length)?; - self.is_create - .assign(region, offset, Value::known(F::from(call.is_create as u64)))?; self.code_store_gas_insufficient.assign( region, offset, @@ -152,7 +147,7 @@ impl ExecutionGadget for ErrorCodeStoreGadget { Value::known(F::from(call.rw_counter_end_of_reversion as u64)), )?; self.restore_context - .assign(region, offset, block, call, step, 5)?; + .assign(region, offset, block, call, step, 4)?; Ok(()) } } diff --git a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs index 73b1480a0d..f65f2ac492 100644 --- a/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs +++ b/zkevm-circuits/src/evm_circuit/util/constraint_builder.rs @@ -441,7 +441,7 @@ impl<'a, F: Field> ConstraintBuilder<'a, F> { pub(crate) fn require_true(&mut self, name: &'static str, constraint: Expression) { self.require_equal(name, constraint, 1.expr()); } - + pub(crate) fn require_equal( &mut self, name: &'static str, diff --git a/zkevm-circuits/src/witness/step.rs b/zkevm-circuits/src/witness/step.rs index ae9b0c2969..63c9021d66 100644 --- a/zkevm-circuits/src/witness/step.rs +++ b/zkevm-circuits/src/witness/step.rs @@ -4,7 +4,6 @@ use bus_mapping::{ evm::OpcodeId, operation, }; -use eth_types::evm_unimplemented; use crate::{ evm_circuit::{ @@ -66,7 +65,9 @@ impl From<&ExecError> for ExecutionState { ExecError::InvalidCreationCode => ExecutionState::ErrorInvalidCreationCode, ExecError::InvalidJump => ExecutionState::ErrorInvalidJump, ExecError::ReturnDataOutOfBounds => ExecutionState::ErrorReturnDataOutOfBound, - ExecError::CodeStoreOutOfGas | ExecError::MaxCodeSizeExceeded => ExecutionState::ErrorCodeStore, + ExecError::CodeStoreOutOfGas | ExecError::MaxCodeSizeExceeded => { + ExecutionState::ErrorCodeStore + } ExecError::OutOfGas(oog_error) => match oog_error { OogError::Constant => ExecutionState::ErrorOutOfGasConstant, OogError::StaticMemoryExpansion => { From fdca390adda044f5e95472e9cfe3d4d7707056bd Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Wed, 1 Mar 2023 10:36:44 +0800 Subject: [PATCH 3/9] revert some comment code --- bus-mapping/src/circuit_input_builder/input_state_ref.rs | 3 +-- bus-mapping/src/evm/opcodes.rs | 9 +++++---- zkevm-circuits/src/witness/step.rs | 3 ++- 3 files changed, 8 insertions(+), 7 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder/input_state_ref.rs b/bus-mapping/src/circuit_input_builder/input_state_ref.rs index d44379442d..09fcd4a656 100644 --- a/bus-mapping/src/circuit_input_builder/input_state_ref.rs +++ b/bus-mapping/src/circuit_input_builder/input_state_ref.rs @@ -1003,13 +1003,12 @@ impl<'a> CircuitInputStateRef<'a> { }; let gas_refund = geth_step.gas.0 - memory_expansion_gas_cost - code_deposit_cost; - //let caller_gas_left = geth_step_next.gas.0 - gas_refund; let caller_gas_left = if !call.is_success && geth_step.op == OpcodeId::RETURN { geth_step_next.gas.0 } else { geth_step_next.gas.0 - gas_refund }; - + for (field, value) in [ (CallContextField::IsRoot, (caller.is_root as u64).into()), ( diff --git a/bus-mapping/src/evm/opcodes.rs b/bus-mapping/src/evm/opcodes.rs index 6a1e1eca39..135109536f 100644 --- a/bus-mapping/src/evm/opcodes.rs +++ b/bus-mapping/src/evm/opcodes.rs @@ -258,7 +258,7 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps { DummySelfDestruct::gen_associated_ops } OpcodeId::CREATE => { - //evm_unimplemented!("Using dummy gen_create_ops for opcode {:?}", opcode_id); + evm_unimplemented!("Using dummy gen_create_ops for opcode {:?}", opcode_id); DummyCreate::::gen_associated_ops } OpcodeId::CREATE2 => { @@ -292,7 +292,7 @@ fn fn_gen_error_state_associated_ops(error: &ExecError) -> Option { - //evm_unimplemented!("TODO: error state {:?} not implemented", error); + evm_unimplemented!("TODO: error state {:?} not implemented", error); None } } @@ -324,9 +324,10 @@ pub fn gen_associated_ops( None }; if let Some(exec_error) = state.get_step_err(geth_step, next_step).unwrap() { - println!( + log::warn!( "geth error {:?} occurred in {:?}", - exec_error, geth_step.op + exec_error, + geth_step.op ); exec_step.error = Some(exec_error.clone()); diff --git a/zkevm-circuits/src/witness/step.rs b/zkevm-circuits/src/witness/step.rs index 63c9021d66..b8c4b0fc51 100644 --- a/zkevm-circuits/src/witness/step.rs +++ b/zkevm-circuits/src/witness/step.rs @@ -4,6 +4,7 @@ use bus_mapping::{ evm::OpcodeId, operation, }; +use eth_types::evm_unimplemented; use crate::{ evm_circuit::{ @@ -114,7 +115,7 @@ impl From<&circuit_input_builder::ExecStep> for ExecutionState { macro_rules! dummy { ($name:expr) => {{ - //evm_unimplemented!("{:?} is implemented with DummyGadget", $name); + evm_unimplemented!("{:?} is implemented with DummyGadget", $name); $name }}; } From 7b848ac83f04797b4928658304f80ee3ee1dde47 Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Wed, 1 Mar 2023 17:33:52 +0800 Subject: [PATCH 4/9] add test for tx deploy(tx.to = null) --- .../evm_circuit/execution/error_code_store.rs | 31 ++++++++++++++++++- 1 file changed, 30 insertions(+), 1 deletion(-) 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 74b3cf3152..2ea1d32360 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs @@ -157,10 +157,11 @@ mod test { use bus_mapping::circuit_input_builder::CircuitsParams; use eth_types::{ address, bytecode, evm_types::OpcodeId, geth_types::Account, Address, Bytecode, Word, + word, }; use lazy_static::lazy_static; - use mock::{eth, TestContext}; + use mock::{eth, TestContext, gwei, MOCK_ACCOUNTS}; use crate::test_util::CircuitTestBuilder; @@ -293,4 +294,32 @@ mod test { run_test_circuits(test_context(caller, false)); } } + + #[test] + fn tx_deploy_code_store_oog() { + + let code = initialization_bytecode(true); + + let ctx = TestContext::<1, 1>::new( + None, + |accs| { + accs[0] + .address(MOCK_ACCOUNTS[0]) + .balance(eth(20)); + }, + |mut txs, _accs| { + txs[0] + .from(MOCK_ACCOUNTS[0]) + //.gas(53424u64.into()) + .gas(53446u64.into()) + .value(eth(2)) + .input(code.into()); + }, + |block, _tx| block.number(0xcafeu64), + ) + .unwrap(); + + CircuitTestBuilder::new_from_test_ctx(ctx).run(); + } + } From 8e2f9dc6575524e1286bda5092db8164f09a9684 Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Mon, 6 Mar 2023 15:31:10 +0800 Subject: [PATCH 5/9] use common error gadget and fix handle_restore_context in root call --- .../circuit_input_builder/input_state_ref.rs | 4 + .../src/evm/opcodes/error_code_store.rs | 2 +- .../evm_circuit/execution/error_code_store.rs | 91 +++++++++---------- .../src/evm_circuit/util/common_gadget.rs | 20 +++- 4 files changed, 66 insertions(+), 51 deletions(-) diff --git a/bus-mapping/src/circuit_input_builder/input_state_ref.rs b/bus-mapping/src/circuit_input_builder/input_state_ref.rs index 3e391ae42e..4640050640 100644 --- a/bus-mapping/src/circuit_input_builder/input_state_ref.rs +++ b/bus-mapping/src/circuit_input_builder/input_state_ref.rs @@ -956,6 +956,10 @@ impl<'a> CircuitInputStateRef<'a> { exec_step: &mut ExecStep, ) -> Result<(), Error> { let call = self.call()?.clone(); + if call.is_root { + return Ok(()); + } + let caller = self.caller()?.clone(); self.call_context_read( exec_step, diff --git a/bus-mapping/src/evm/opcodes/error_code_store.rs b/bus-mapping/src/evm/opcodes/error_code_store.rs index bf5ac4d517..6e8d77a92d 100644 --- a/bus-mapping/src/evm/opcodes/error_code_store.rs +++ b/bus-mapping/src/evm/opcodes/error_code_store.rs @@ -37,7 +37,7 @@ impl Opcode for ErrorCodeStore { let is_success = call.is_success; // create context check - assert!(call.is_create() && !call.is_root); + assert!(call.is_create()); state.call_context_read( &mut exec_step, 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 2ea1d32360..1ec0eb586f 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs @@ -4,12 +4,11 @@ use crate::{ param::{N_BYTES_GAS, N_BYTES_U64}, step::ExecutionState, util::{ - common_gadget::RestoreContextGadget, constraint_builder::ConstraintBuilder, + common_gadget::CommonErrorGadget, constraint_builder::ConstraintBuilder, math_gadget::LtGadget, memory_gadget::MemoryAddressGadget, CachedRegion, Cell, }, witness::{Block, Call, ExecStep, Transaction}, }, - table::CallContextFieldTag, util::Expr, }; @@ -27,8 +26,7 @@ pub(crate) struct ErrorCodeStoreGadget { code_store_gas_insufficient: LtGadget, // check for MaxCodeSizeExceeded error max_code_size_exceed: LtGadget, - rw_counter_end_of_reversion: Cell, - restore_context: RestoreContextGadget, + common_error_gadget: CommonErrorGadget, } impl ExecutionGadget for ErrorCodeStoreGadget { @@ -38,11 +36,11 @@ impl ExecutionGadget for ErrorCodeStoreGadget { fn configure(cb: &mut ConstraintBuilder) -> Self { let opcode = cb.query_cell(); - cb.opcode_lookup(opcode.expr(), 1.expr()); + //cb.opcode_lookup(opcode.expr(), 1.expr()); let offset = cb.query_cell_phase2(); let length = cb.query_word_rlc(); - let rw_counter_end_of_reversion = cb.query_cell(); + //let rw_counter_end_of_reversion = cb.query_cell(); cb.stack_pop(offset.expr()); cb.stack_pop(length.expr()); @@ -69,35 +67,13 @@ impl ExecutionGadget for ErrorCodeStoreGadget { vec![1.expr(), 2.expr()], ); - // restore context as in internal call - cb.require_zero("in internal call", cb.curr.state.is_root.expr()); - - cb.call_context_lookup(false.expr(), None, CallContextFieldTag::IsSuccess, 0.expr()); - cb.call_context_lookup( - false.expr(), - None, - CallContextFieldTag::RwCounterEndOfReversion, - rw_counter_end_of_reversion.expr(), - ); - // Case C in the return specs. - let restore_context = RestoreContextGadget::construct( + let common_error_gadget = CommonErrorGadget::construct_with_lastcallee_return_data( cb, - 0.expr(), - 0.expr(), + opcode.expr(), + 4.expr(), memory_address.offset(), memory_address.length(), - 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 { @@ -105,8 +81,7 @@ impl ExecutionGadget for ErrorCodeStoreGadget { memory_address, code_store_gas_insufficient, max_code_size_exceed, - rw_counter_end_of_reversion, - restore_context, + common_error_gadget, } } @@ -141,13 +116,8 @@ impl ExecutionGadget for ErrorCodeStoreGadget { F::from(length.as_u64()), )?; - self.rw_counter_end_of_reversion.assign( - region, - offset, - Value::known(F::from(call.rw_counter_end_of_reversion as u64)), - )?; - self.restore_context - .assign(region, offset, block, call, step, 4)?; + self.common_error_gadget + .assign(region, offset, block, call, step, 4 as usize)?; Ok(()) } } @@ -156,12 +126,18 @@ impl ExecutionGadget for ErrorCodeStoreGadget { mod test { use bus_mapping::circuit_input_builder::CircuitsParams; use eth_types::{ - address, bytecode, evm_types::OpcodeId, geth_types::Account, Address, Bytecode, Word, - word, + address, + bytecode, + evm_types::OpcodeId, + geth_types::Account, + Address, + Bytecode, + Word, + //word, }; use lazy_static::lazy_static; - use mock::{eth, TestContext, gwei, MOCK_ACCOUNTS}; + use mock::{eth, TestContext, MOCK_ACCOUNTS}; use crate::test_util::CircuitTestBuilder; @@ -297,15 +273,12 @@ mod test { #[test] fn tx_deploy_code_store_oog() { - let code = initialization_bytecode(true); - + let ctx = TestContext::<1, 1>::new( None, |accs| { - accs[0] - .address(MOCK_ACCOUNTS[0]) - .balance(eth(20)); + accs[0].address(MOCK_ACCOUNTS[0]).balance(eth(20)); }, |mut txs, _accs| { txs[0] @@ -322,4 +295,26 @@ mod test { CircuitTestBuilder::new_from_test_ctx(ctx).run(); } + #[test] + fn tx_deploy_max_code_size_exceed() { + let code = initialization_bytecode(false); + + let ctx = TestContext::<1, 1>::new( + None, + |accs| { + accs[0].address(MOCK_ACCOUNTS[0]).balance(eth(20)); + }, + |mut txs, _accs| { + txs[0] + .from(MOCK_ACCOUNTS[0]) + .gas(58000u64.into()) + .value(eth(2)) + .input(code.into()); + }, + |block, _tx| block.number(0xcafeu64), + ) + .unwrap(); + + CircuitTestBuilder::new_from_test_ctx(ctx).run(); + } } diff --git a/zkevm-circuits/src/evm_circuit/util/common_gadget.rs b/zkevm-circuits/src/evm_circuit/util/common_gadget.rs index 6cb731ec80..a74481874c 100644 --- a/zkevm-circuits/src/evm_circuit/util/common_gadget.rs +++ b/zkevm-circuits/src/evm_circuit/util/common_gadget.rs @@ -843,6 +843,22 @@ impl CommonErrorGadget { cb: &mut ConstraintBuilder, opcode: Expression, rw_counter_delta: Expression, + ) -> Self { + Self::construct_with_lastcallee_return_data( + cb, + opcode, + rw_counter_delta, + 0.expr(), + 0.expr(), + ) + } + + pub(crate) fn construct_with_lastcallee_return_data( + cb: &mut ConstraintBuilder, + opcode: Expression, + rw_counter_delta: Expression, + return_data_offset: Expression, + return_data_length: Expression, ) -> Self { cb.opcode_lookup(opcode.expr(), 1.expr()); @@ -883,8 +899,8 @@ impl CommonErrorGadget { cb, 0.expr(), 0.expr(), - 0.expr(), - 0.expr(), + return_data_offset, + return_data_length, 0.expr(), 0.expr(), ) From 3d50adc9ba8ed75dc158aa48d27405d116c1d4c6 Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Mon, 6 Mar 2023 18:54:55 +0800 Subject: [PATCH 6/9] fix clippy --- .../src/evm_circuit/execution/error_code_store.rs | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) 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 1ec0eb586f..03d4226c6f 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs @@ -13,6 +13,7 @@ use crate::{ }; use eth_types::{evm_types::GasCost, Field}; + use halo2_proofs::{circuit::Value, plonk::Error}; const MAXCODESIZE: u64 = 0x6000u64; @@ -36,11 +37,9 @@ impl ExecutionGadget for ErrorCodeStoreGadget { fn configure(cb: &mut ConstraintBuilder) -> Self { let opcode = cb.query_cell(); - //cb.opcode_lookup(opcode.expr(), 1.expr()); let offset = cb.query_cell_phase2(); let length = cb.query_word_rlc(); - //let rw_counter_end_of_reversion = cb.query_cell(); cb.stack_pop(offset.expr()); cb.stack_pop(length.expr()); @@ -67,7 +66,6 @@ impl ExecutionGadget for ErrorCodeStoreGadget { vec![1.expr(), 2.expr()], ); - // Case C in the return specs. let common_error_gadget = CommonErrorGadget::construct_with_lastcallee_return_data( cb, opcode.expr(), @@ -117,7 +115,7 @@ impl ExecutionGadget for ErrorCodeStoreGadget { )?; self.common_error_gadget - .assign(region, offset, block, call, step, 4 as usize)?; + .assign(region, offset, block, call, step, 4)?; Ok(()) } } From edc24ab251b35c5bc5113555b35e4ce64ab13cf6 Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Tue, 7 Mar 2023 12:16:49 +0800 Subject: [PATCH 7/9] try to resolve ci memory invalid issue --- zkevm-circuits/src/test_util.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index 01dde2cf06..851f20c9df 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -1,7 +1,7 @@ //! Testing utilities use crate::{ - evm_circuit::{cached::EvmCircuitCached, EvmCircuit}, + evm_circuit::EvmCircuit, state_circuit::StateCircuit, util::SubCircuit, witness::{Block, Rw}, @@ -211,7 +211,7 @@ impl CircuitTestBuilder { let (active_gate_rows, active_lookup_rows) = EvmCircuit::::get_active_rows(&block); - let circuit = EvmCircuitCached::get_test_cicuit_from_block(block.clone()); + let circuit = EvmCircuit::get_test_cicuit_from_block(block.clone()); let prover = MockProver::::run(k, &circuit, vec![]).unwrap(); self.evm_checks.as_ref()(prover, &active_gate_rows, &active_lookup_rows) From d89260b257bbe1d68485bee142843189bb935e52 Mon Sep 17 00:00:00 2001 From: Zhang Zhuo Date: Wed, 8 Mar 2023 14:27:45 +0800 Subject: [PATCH 8/9] Update test_util.rs --- zkevm-circuits/src/test_util.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/zkevm-circuits/src/test_util.rs b/zkevm-circuits/src/test_util.rs index 851f20c9df..01dde2cf06 100644 --- a/zkevm-circuits/src/test_util.rs +++ b/zkevm-circuits/src/test_util.rs @@ -1,7 +1,7 @@ //! Testing utilities use crate::{ - evm_circuit::EvmCircuit, + evm_circuit::{cached::EvmCircuitCached, EvmCircuit}, state_circuit::StateCircuit, util::SubCircuit, witness::{Block, Rw}, @@ -211,7 +211,7 @@ impl CircuitTestBuilder { let (active_gate_rows, active_lookup_rows) = EvmCircuit::::get_active_rows(&block); - let circuit = EvmCircuit::get_test_cicuit_from_block(block.clone()); + let circuit = EvmCircuitCached::get_test_cicuit_from_block(block.clone()); let prover = MockProver::::run(k, &circuit, vec![]).unwrap(); self.evm_checks.as_ref()(prover, &active_gate_rows, &active_lookup_rows) From 7fd07aa33f86b3066c4882703dd752a7aedf6ecc Mon Sep 17 00:00:00 2001 From: Dream Wu Date: Tue, 14 Mar 2023 15:02:15 +0800 Subject: [PATCH 9/9] add non static call --- bus-mapping/src/evm/opcodes/error_code_store.rs | 12 ++++++++++-- .../src/evm_circuit/execution/error_code_store.rs | 8 ++++++-- 2 files changed, 16 insertions(+), 4 deletions(-) diff --git a/bus-mapping/src/evm/opcodes/error_code_store.rs b/bus-mapping/src/evm/opcodes/error_code_store.rs index 6db4c6a9e6..33711ca14e 100644 --- a/bus-mapping/src/evm/opcodes/error_code_store.rs +++ b/bus-mapping/src/evm/opcodes/error_code_store.rs @@ -35,9 +35,17 @@ impl Opcode for ErrorCodeStore { let call = state.call()?; let call_id = call.call_id; let is_success = call.is_success; + let is_static = call.is_static; - // create context check - assert!(call.is_create()); + // create context check and non in static call + assert!(call.is_create() && !call.is_static); + + state.call_context_read( + &mut exec_step, + call_id, + CallContextField::IsStatic, + (is_static as u64).into(), + ); state.call_context_read( &mut exec_step, 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 6faaa4b933..209c5d4e72 100644 --- a/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs +++ b/zkevm-circuits/src/evm_circuit/execution/error_code_store.rs @@ -9,6 +9,7 @@ use crate::{ }, witness::{Block, Call, ExecStep, Transaction}, }, + table::CallContextFieldTag, util::Expr, }; @@ -47,6 +48,9 @@ impl ExecutionGadget for ErrorCodeStoreGadget { cb.require_true("is_create is true", cb.curr.state.is_create.expr()); + // constrain in non static call + cb.call_context_lookup(false.expr(), None, CallContextFieldTag::IsStatic, 0.expr()); + // constrain code store gas > gas left, that is GasCost::CODE_DEPOSIT_BYTE_COST // * length > gas left let code_store_gas_insufficient = LtGadget::construct( @@ -69,7 +73,7 @@ impl ExecutionGadget for ErrorCodeStoreGadget { let common_error_gadget = CommonErrorGadget::construct_with_lastcallee_return_data( cb, opcode.expr(), - 4.expr(), + 5.expr(), memory_address.offset(), memory_address.length(), ); @@ -115,7 +119,7 @@ impl ExecutionGadget for ErrorCodeStoreGadget { )?; self.common_error_gadget - .assign(region, offset, block, call, step, 4)?; + .assign(region, offset, block, call, step, 5)?; Ok(()) } }