fix(levm): next opcode handling (#1368)

Fixes:
- If there's no opcode present at the given PC (PC > bytecode length),
then we stop the execution (as [it is done in the execution
specs](https://github.com/ethereum/execution-specs/blob/master/src/ethereum/cancun/vm/interpreter.py#L293-L303))
- If at the address given by the JUMP instruction there's a valid opcode
present then it's a valid jump otherwise it isn't.

---------

Co-authored-by: Jeremías Salomón <[email protected]>

crates/vm/levm/src/call_frame.rs

@@ -119,10 +119,11 @@ impl CallFrame {
         }
     }
 
-    pub fn next_opcode(&mut self) -> Result<Option<Opcode>, VMError> {
-        let opcode = self.opcode_at(self.pc);
-        self.increment_pc()?;
-        Ok(opcode)
+    pub fn next_opcode(&mut self) -> Opcode {
+        match self.bytecode.get(self.pc).copied().map(Opcode::from) {
+            Some(opcode) => opcode,
+            None => Opcode::STOP,
+        }
     }
 
     pub fn increment_pc_by(&mut self, count: usize) -> Result<(), VMError> {
@@ -142,29 +143,27 @@ impl CallFrame {
     }
 
     /// Jump to the given address, returns false if the jump position wasn't a JUMPDEST
-    pub fn jump(&mut self, jump_address: U256) -> Result<bool, VMError> {
+    pub fn jump(&mut self, jump_address: U256) -> Result<(), VMError> {
         let jump_address_usize = jump_address
             .try_into()
             .map_err(|_err| VMError::VeryLargeNumber)?;
 
-        if !self.valid_jump(jump_address_usize) {
-            return Ok(false);
-        }
+        self.validate_jump(jump_address_usize)?;
         self.pc = jump_address_usize;
-        Ok(true)
-    }
-
-    fn valid_jump(&self, jump_address: usize) -> bool {
-        self.opcode_at(jump_address)
-            .map(|opcode| opcode.eq(&Opcode::JUMPDEST))
-            .is_some_and(|is_jumpdest| is_jumpdest)
+        Ok(())
     }
 
-    fn opcode_at(&self, offset: usize) -> Option<Opcode> {
-        self.bytecode
-            .get(offset)
-            .copied()
-            .map(Opcode::try_from)?
-            .ok()
+    fn validate_jump(&self, to_jump_address: usize) -> Result<(), VMError> {
+        if matches!(
+            self.bytecode
+                .get(to_jump_address)
+                .copied()
+                .map(Opcode::try_from),
+            Some(Ok(Opcode::JUMPDEST))
+        ) {
+            Ok(())
+        } else {
+            Err(VMError::InvalidJump)
+        }
     }
 }

crates/vm/levm/src/errors.rs

@@ -174,6 +174,8 @@ pub enum InternalError {
     ExcessBlobGasShouldNotBeNone,
     #[error("Error in utils file")]
     UtilsError,
+    #[error("PC out of bounds")]
+    PCOutOfBounds,
 }
 
 #[derive(Debug, Clone)]

crates/vm/levm/src/opcode_handlers/stack_memory_storage_flow.rs

@@ -289,9 +289,7 @@ impl VM {
         self.increase_consumed_gas(current_call_frame, gas_cost::JUMP)?;
 
         let jump_address = current_call_frame.stack.pop()?;
-        if !current_call_frame.jump(jump_address)? {
-            return Err(VMError::InvalidJump);
-        }
+        current_call_frame.jump(jump_address)?;
 
         Ok(OpcodeSuccess::Continue)
     }
@@ -304,11 +302,11 @@ impl VM {
         let jump_address = current_call_frame.stack.pop()?;
         let condition = current_call_frame.stack.pop()?;
 
-        if condition != U256::zero() && !current_call_frame.jump(jump_address)? {
-            return Err(VMError::InvalidJump);
-        }
-
         self.increase_consumed_gas(current_call_frame, gas_cost::JUMPI)?;
+
+        if !condition.is_zero() {
+            current_call_frame.jump(jump_address)?;
+        }
         Ok(OpcodeSuccess::Continue)
     }
 

crates/vm/levm/src/opcodes.rs

@@ -1,5 +1,3 @@
-use crate::errors::VMError;
-
 #[derive(Debug, PartialEq, Eq, Clone, PartialOrd)]
 pub enum Opcode {
     // Stop and Arithmetic Operations
@@ -174,11 +172,9 @@ pub enum Opcode {
 
 impl Copy for Opcode {}
 
-impl TryFrom<u8> for Opcode {
-    type Error = VMError;
-
-    fn try_from(byte: u8) -> Result<Self, Self::Error> {
-        let op = match byte {
+impl From<u8> for Opcode {
+    fn from(byte: u8) -> Self {
+        match byte {
             0x00 => Opcode::STOP,
             0x01 => Opcode::ADD,
             0x16 => Opcode::AND,
@@ -326,11 +322,9 @@ impl TryFrom<u8> for Opcode {
             0xF4 => Opcode::DELEGATECALL,
             0xFA => Opcode::STATICCALL,
             0xFD => Opcode::REVERT,
-            0xFE => Opcode::INVALID,
             0xFF => Opcode::SELFDESTRUCT,
-            _ => return Err(VMError::InvalidOpcode),
-        };
-        Ok(op)
+            _ => Opcode::INVALID,
+        }
     }
 }
 

crates/vm/levm/src/operations.rs

@@ -188,9 +188,9 @@ impl Operation {
                     )
                     .ok_or(VMError::Internal(InternalError::SlicingError))?;
                 assert_eq!(value_to_push.len(), n_usize);
-                let opcode = Opcode::try_from((u8::from(Opcode::PUSH0)).checked_add(*n).ok_or(
+                let opcode = Opcode::from((u8::from(Opcode::PUSH0)).checked_add(*n).ok_or(
                     VMError::Internal(InternalError::ArithmeticOperationOverflow),
-                )?)?;
+                )?);
                 let mut bytes = vec![u8::from(opcode)];
                 bytes.extend_from_slice(value_to_push);
 

crates/vm/levm/src/vm.rs

@@ -180,7 +180,8 @@ impl VM {
         );
 
         loop {
-            let opcode = current_call_frame.next_opcode()?.unwrap_or(Opcode::STOP); // This will execute opcode stop if there are no more opcodes, there are other ways of solving this but this is the simplest and doesn't change VM behavior.
+            let opcode = current_call_frame.next_opcode();
+            current_call_frame.increment_pc()?;
 
             let op_result: Result<OpcodeSuccess, VMError> = match opcode {
                 Opcode::STOP => Ok(OpcodeSuccess::Result(ResultReason::Stop)),