Circuit for ErrorInvalidOpcode state

bus-mapping/src/evm/opcodes.rs

@@ -53,6 +53,7 @@ mod stop;
 mod swap;
 
 mod error_invalid_jump;
+mod error_invalid_opcode;
 mod error_oog_call;
 mod error_oog_log;
 mod error_stack_oog_constant;
@@ -73,7 +74,8 @@ use codecopy::Codecopy;
 use codesize::Codesize;
 use create::DummyCreate;
 use dup::Dup;
-use error_invalid_jump::ErrorInvalidJump;
+use error_invalid_jump::InvalidJump;
+use error_invalid_opcode::InvalidOpcode;
 use error_oog_call::OOGCall;
 use error_oog_log::ErrorOOGLog;
 use error_stack_oog_constant::ErrorStackOogConstant;
@@ -259,7 +261,8 @@ fn fn_gen_associated_ops(opcode_id: &OpcodeId) -> FnGenAssociatedOps {
 
 fn fn_gen_error_state_associated_ops(error: &ExecError) -> Option<FnGenAssociatedOps> {
     match error {
-        ExecError::InvalidJump => Some(ErrorInvalidJump::gen_associated_ops),
+        ExecError::InvalidJump => Some(InvalidJump::gen_associated_ops),
+        ExecError::InvalidOpcode => Some(InvalidOpcode::gen_associated_ops),
         ExecError::OutOfGas(OogError::Call) => Some(OOGCall::gen_associated_ops),
         ExecError::OutOfGas(OogError::Constant) => Some(ErrorStackOogConstant::gen_associated_ops),
         ExecError::StackOverflow => Some(ErrorStackOogConstant::gen_associated_ops),
@@ -283,8 +286,6 @@ pub fn gen_associated_ops(
     state: &mut CircuitInputStateRef,
     geth_steps: &[GethExecStep],
 ) -> Result<Vec<ExecStep>, Error> {
-    let fn_gen_associated_ops = fn_gen_associated_ops(opcode_id);
-
     let memory_enabled = !geth_steps.iter().all(|s| s.memory.is_empty());
     if memory_enabled {
         assert_eq!(
@@ -332,6 +333,7 @@ pub fn gen_associated_ops(
         }
     }
     // if no errors, continue as normal
+    let fn_gen_associated_ops = fn_gen_associated_ops(opcode_id);
     fn_gen_associated_ops(state, geth_steps)
 }
 

bus-mapping/src/evm/opcodes/error_invalid_jump.rs

@@ -4,9 +4,9 @@ use crate::Error;
 use eth_types::{GethExecStep, ToAddress, ToWord, Word};
 
 #[derive(Debug, Copy, Clone)]
-pub(crate) struct ErrorInvalidJump;
+pub(crate) struct InvalidJump;
 
-impl Opcode for ErrorInvalidJump {
+impl Opcode for InvalidJump {
     fn gen_associated_ops(
         state: &mut CircuitInputStateRef,
         geth_steps: &[GethExecStep],

bus-mapping/src/evm/opcodes/error_invalid_opcode.rs

@@ -0,0 +1,24 @@
+use crate::circuit_input_builder::{CircuitInputStateRef, ExecStep};
+use crate::error::ExecError;
+use crate::evm::Opcode;
+use crate::Error;
+use eth_types::GethExecStep;
+
+#[derive(Debug, Copy, Clone)]
+pub(crate) struct InvalidOpcode;
+
+impl Opcode for InvalidOpcode {
+    fn gen_associated_ops(
+        state: &mut CircuitInputStateRef,
+        geth_steps: &[GethExecStep],
+    ) -> Result<Vec<ExecStep>, Error> {
+        let geth_step = &geth_steps[0];
+        let mut exec_step = state.new_step(geth_step)?;
+        exec_step.error = Some(ExecError::InvalidOpcode);
+
+        state.gen_restore_context_ops(&mut exec_step, geth_steps)?;
+        state.handle_return(geth_step)?;
+
+        Ok(vec![exec_step])
+    }
+}

eth-types/src/evm_types/opcode_ids.rs

@@ -866,6 +866,16 @@ impl OpcodeId {
             0
         }
     }
+
+    /// Returns the all invalid opcodes.
+    pub fn invalid_opcodes() -> Vec<Self> {
+        (u8::MIN..=u8::MAX).fold(vec![], |mut acc, val| {
+            if let Self::INVALID(val) = val.into() {
+                acc.push(Self::INVALID(val));
+            }
+            acc
+        })
+    }
 }
 
 impl From<u8> for OpcodeId {

zkevm-circuits/src/evm_circuit/execution.rs

@@ -61,6 +61,7 @@ mod dup;
 mod end_block;
 mod end_tx;
 mod error_invalid_jump;
+mod error_invalid_opcode;
 mod error_oog_call;
 mod error_oog_constant;
 mod error_oog_log;
@@ -127,6 +128,7 @@ use dup::DupGadget;
 use end_block::EndBlockGadget;
 use end_tx::EndTxGadget;
 use error_invalid_jump::ErrorInvalidJumpGadget;
+use error_invalid_opcode::ErrorInvalidOpcodeGadget;
 use error_oog_call::ErrorOOGCallGadget;
 use error_oog_constant::ErrorOOGConstantGadget;
 use error_oog_log::ErrorOOGLogGadget;
@@ -288,14 +290,14 @@ pub(crate) struct ExecutionConfig<F> {
     error_oog_code_store: DummyGadget<F, 0, 0, { ExecutionState::ErrorOutOfGasCodeStore }>,
     error_insufficient_balance: DummyGadget<F, 0, 0, { ExecutionState::ErrorInsufficientBalance }>,
     error_invalid_jump: ErrorInvalidJumpGadget<F>,
+    error_invalid_opcode: ErrorInvalidOpcodeGadget<F>,
     error_depth: DummyGadget<F, 0, 0, { ExecutionState::ErrorDepth }>,
     error_write_protection: DummyGadget<F, 0, 0, { ExecutionState::ErrorWriteProtection }>,
     error_contract_address_collision:
         DummyGadget<F, 0, 0, { ExecutionState::ErrorContractAddressCollision }>,
     error_invalid_creation_code: DummyGadget<F, 0, 0, { ExecutionState::ErrorInvalidCreationCode }>,
     error_return_data_out_of_bound:
         DummyGadget<F, 0, 0, { ExecutionState::ErrorReturnDataOutOfBound }>,
-    invalid_opcode_gadget: DummyGadget<F, 0, 0, { ExecutionState::ErrorInvalidOpcode }>,
 }
 
 impl<F: Field> ExecutionConfig<F> {
@@ -537,12 +539,12 @@ impl<F: Field> ExecutionConfig<F> {
             error_oog_code_store: configure_gadget!(),
             error_insufficient_balance: configure_gadget!(),
             error_invalid_jump: configure_gadget!(),
+            error_invalid_opcode: configure_gadget!(),
             error_write_protection: configure_gadget!(),
             error_depth: configure_gadget!(),
             error_contract_address_collision: configure_gadget!(),
             error_invalid_creation_code: configure_gadget!(),
             error_return_data_out_of_bound: configure_gadget!(),
-            invalid_opcode_gadget: configure_gadget!(),
             // step and presets
             step: step_curr,
             height_map,
@@ -1214,6 +1216,9 @@ impl<F: Field> ExecutionConfig<F> {
             ExecutionState::ErrorInvalidJump => {
                 assign_exec_step!(self.error_invalid_jump)
             }
+            ExecutionState::ErrorInvalidOpcode => {
+                assign_exec_step!(self.error_invalid_opcode)
+            }
             ExecutionState::ErrorWriteProtection => {
                 assign_exec_step!(self.error_write_protection)
             }
@@ -1230,10 +1235,6 @@ impl<F: Field> ExecutionConfig<F> {
                 assign_exec_step!(self.error_return_data_out_of_bound)
             }
 
-            ExecutionState::ErrorInvalidOpcode => {
-                assign_exec_step!(self.invalid_opcode_gadget)
-            }
-
             _ => evm_unimplemented!("unimplemented ExecutionState: {:?}", step.execution_state),
         }
 

zkevm-circuits/src/evm_circuit/execution/error_invalid_opcode.rs

@@ -0,0 +1,219 @@
+use crate::evm_circuit::execution::ExecutionGadget;
+use crate::evm_circuit::step::ExecutionState;
+use crate::evm_circuit::table::{FixedTableTag, Lookup};
+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::{not, CachedRegion, Cell};
+use crate::evm_circuit::witness::{Block, Call, ExecStep, Transaction};
+use crate::table::CallContextFieldTag;
+use eth_types::Field;
+use gadgets::util::Expr;
+use halo2_proofs::circuit::Value;
+use halo2_proofs::plonk::Error;
+
+/// Gadget for invalid opcodes. It verifies by a fixed lookup for
+/// ResponsibleOpcode.
+#[derive(Clone, Debug)]
+pub(crate) struct ErrorInvalidOpcodeGadget<F> {
+    opcode: Cell<F>,
+    rw_counter_end_of_reversion: Cell<F>,
+    restore_context: RestoreContextGadget<F>,
+}
+
+impl<F: Field> ExecutionGadget<F> for ErrorInvalidOpcodeGadget<F> {
+    const NAME: &'static str = "ErrorInvalidOpcode";
+
+    const EXECUTION_STATE: ExecutionState = ExecutionState::ErrorInvalidOpcode;
+
+    fn configure(cb: &mut ConstraintBuilder<F>) -> Self {
+        let opcode = cb.query_cell();
+        cb.opcode_lookup(opcode.expr(), 1.expr());
+        cb.add_lookup(
+            "Responsible opcode lookup",
+            Lookup::Fixed {
+                tag: FixedTableTag::ResponsibleOpcode.expr(),
+                values: [
+                    Self::EXECUTION_STATE.as_u64().expr(),
+                    opcode.expr(),
+                    0.expr(),
+                ],
+            },
+        );
+
+        // Current call must be failed.
+        let rw_counter_end_of_reversion = cb.query_cell();
+        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(),
+        );
+
+        // 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(2.expr() + cb.curr.state.reversible_write_counter.expr()),
+                ..StepStateTransition::any()
+            });
+        });
+
+        // When it is an internal call, need to restore caller's state as finishing this
+        // call. Restore caller state to next StepState.
+        let restore_context = cb.condition(not::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,
+            restore_context,
+        }
+    }
+
+    fn assign_exec_step(
+        &self,
+        region: &mut CachedRegion<'_, '_, F>,
+        offset: usize,
+        block: &Block<F>,
+        _: &Transaction,
+        call: &Call,
+        step: &ExecStep,
+    ) -> Result<(), Error> {
+        let opcode = F::from(step.opcode.unwrap().as_u64());
+        self.opcode.assign(region, offset, Value::known(opcode))?;
+
+        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, 2)
+    }
+}
+
+#[cfg(test)]
+mod test {
+    use crate::evm_circuit::test::rand_bytes;
+    use crate::test_util::CircuitTestBuilder;
+    use eth_types::bytecode::Bytecode;
+    use eth_types::{bytecode, ToWord, Word};
+    use lazy_static::lazy_static;
+    use mock::TestContext;
+
+    lazy_static! {
+        static ref TESTING_INVALID_CODES: [Vec<u8>; 6] = [
+            // Single invalid opcode
+            vec![0x0e],
+            vec![0x4f],
+            vec![0xa5],
+            vec![0xf6],
+            vec![0xfe],
+            // Multiple invalid opcodes
+            vec![0x5c, 0x5e, 0x5f],
+        ];
+    }
+
+    #[test]
+    fn invalid_opcode_root() {
+        for invalid_code in TESTING_INVALID_CODES.iter() {
+            test_root_ok(invalid_code);
+        }
+    }
+
+    #[test]
+    fn invalid_opcode_internal() {
+        for invalid_code in TESTING_INVALID_CODES.iter() {
+            test_internal_ok(0x20, 0x00, invalid_code);
+        }
+    }
+
+    fn test_root_ok(invalid_code: &[u8]) {
+        let mut code = Bytecode::default();
+        invalid_code.iter().for_each(|b| {
+            code.write(*b, true);
+        });
+
+        CircuitTestBuilder::new_from_test_ctx(
+            TestContext::<2, 1>::simple_ctx_with_bytecode(code).unwrap(),
+        )
+        .run();
+    }
+
+    fn test_internal_ok(call_data_offset: usize, call_data_length: usize, invalid_code: &[u8]) {
+        let (addr_a, addr_b) = (mock::MOCK_ACCOUNTS[0], mock::MOCK_ACCOUNTS[1]);
+
+        // Code B gets called by code A, so the call is an internal call.
+        let mut code_b = Bytecode::default();
+        invalid_code.iter().for_each(|b| {
+            code_b.write(*b, true);
+        });
+
+        // code A calls code B.
+        let pushdata = rand_bytes(8);
+        let code_a = bytecode! {
+            // populate memory in A's context.
+            PUSH8(Word::from_big_endian(&pushdata))
+            PUSH1(0x00) // offset
+            MSTORE
+            // call ADDR_B.
+            PUSH1(0x00) // retLength
+            PUSH1(0x00) // retOffset
+            PUSH32(call_data_length) // argsLength
+            PUSH32(call_data_offset) // argsOffset
+            PUSH1(0x00) // value
+            PUSH32(addr_b.to_word()) // addr
+            PUSH32(0x1_0000) // 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::MOCK_ACCOUNTS[3])
+                    .balance(Word::from(1_u64 << 20));
+            },
+            |mut txs, accs| {
+                txs[0].to(accs[1].address).from(accs[2].address);
+            },
+            |block, _tx| block,
+        )
+        .unwrap();
+
+        CircuitTestBuilder::new_from_test_ctx(ctx).run();
+    }
+}

zkevm-circuits/src/evm_circuit/step.rs

@@ -303,6 +303,7 @@ impl ExecutionState {
             Self::RETURN_REVERT => vec![OpcodeId::RETURN, OpcodeId::REVERT],
             Self::CREATE2 => vec![OpcodeId::CREATE2],
             Self::SELFDESTRUCT => vec![OpcodeId::SELFDESTRUCT],
+            Self::ErrorInvalidOpcode => OpcodeId::invalid_opcodes(),
             _ => vec![],
         }
     }