refactor(levm): moved retdata and state backup to callframe

crates/vm/levm/src/call_frame.rs

@@ -4,6 +4,7 @@ use crate::{
     memory::Memory,
     opcodes::Opcode,
     utils::get_valid_jump_destinations,
+    vm::{RetData, StateBackup},
 };
 use bytes::Bytes;
 use ethrex_common::{
@@ -90,6 +91,10 @@ pub struct CallFrame {
     pub create_op_called: bool,
     /// Everytime we want to write an account during execution of a callframe we store the pre-write state so that we can restore if it reverts
     pub cache_backup: CacheBackup,
+    /// Backup of the state if it needs to be reverted
+    pub state_backup: StateBackup,
+    /// Return data for the context
+    pub retdata: RetData,
 }
 
 pub type CacheBackup = HashMap<Address, Option<Account>>;
@@ -108,6 +113,8 @@ impl CallFrame {
         gas_used: u64,
         depth: usize,
         create_op_called: bool,
+        state_backup: StateBackup,
+        retdata: RetData,
     ) -> Self {
         let valid_jump_destinations = get_valid_jump_destinations(&bytecode).unwrap_or_default();
         Self {
@@ -123,6 +130,8 @@ impl CallFrame {
             gas_used,
             valid_jump_destinations,
             create_op_called,
+            state_backup,
+            retdata,
             ..Default::default()
         }
     }

crates/vm/levm/src/execution_handlers.rs

@@ -1,42 +1,34 @@
 use crate::{
-    call_frame::CallFrame,
     constants::*,
     errors::{ExecutionReport, InternalError, OpcodeResult, OutOfGasError, TxResult, VMError},
     gas_cost::CODE_DEPOSIT_COST,
     opcodes::Opcode,
     utils::*,
-    vm::{StateBackup, VM},
+    vm::VM,
 };
 
-use ethrex_common::types::Fork;
-
 use bytes::Bytes;
 
 impl<'a> VM<'a> {
     pub fn handle_precompile_result(
         &mut self,
         precompile_result: Result<Bytes, VMError>,
-        backup: StateBackup,
-        current_call_frame: &mut CallFrame,
     ) -> Result<ExecutionReport, VMError> {
         match precompile_result {
             Ok(output) => Ok(ExecutionReport {
                 result: TxResult::Success,
-                gas_used: current_call_frame.gas_used,
+                gas_used: self.current_call_frame()?.gas_used,
                 gas_refunded: self.env.refunded_gas,
                 output,
-                logs: std::mem::take(&mut current_call_frame.logs),
+                logs: std::mem::take(&mut self.current_call_frame_mut()?.logs),
             }),
             Err(error) => {
                 if error.is_internal() {
                     return Err(error);
                 }
-
-                self.restore_state(backup, current_call_frame.cache_backup.clone())?;
-
                 Ok(ExecutionReport {
                     result: TxResult::Revert(error),
-                    gas_used: current_call_frame.gas_limit,
+                    gas_used: self.current_call_frame()?.gas_limit,
                     gas_refunded: self.env.refunded_gas,
                     output: Bytes::new(),
                     logs: vec![],
@@ -150,38 +142,35 @@ impl<'a> VM<'a> {
         }
     }
 
-    pub fn handle_opcode_result(
-        &mut self,
-        current_call_frame: &mut CallFrame,
-    ) -> Result<ExecutionReport, VMError> {
-        let backup = self
-            .backups
-            .pop()
-            .ok_or(VMError::Internal(InternalError::CouldNotPopCallframe))?;
-        // On successful create check output validity
-        if (self.is_create() && current_call_frame.depth == 0)
-            || current_call_frame.create_op_called
+    pub fn handle_opcode_result(&mut self) -> Result<ExecutionReport, VMError> {
+        let mut transaction_result = TxResult::Success;
         {
-            let contract_code = std::mem::take(&mut current_call_frame.output);
-            let code_length = contract_code.len();
-
-            let code_length_u64: u64 = code_length
-                .try_into()
-                .map_err(|_| VMError::Internal(InternalError::ConversionError))?;
-
-            let code_deposit_cost: u64 =
-                code_length_u64
-                    .checked_mul(CODE_DEPOSIT_COST)
-                    .ok_or(VMError::Internal(
-                        InternalError::ArithmeticOperationOverflow,
-                    ))?;
-
-            // Revert
-            // If the first byte of code is 0xef
-            // If the code_length > MAX_CODE_SIZE
-            // If current_consumed_gas + code_deposit_cost > gas_limit
-            let validate_create =
-                if code_length > MAX_CODE_SIZE && self.env.config.fork >= Fork::SpuriousDragon {
+            // Check for the case where the transaction is a contract creation one
+            let transaction_is_create = self.is_create();
+            let current_call_frame = self.current_call_frame_mut()?;
+            // On successful create check output validity
+            if (transaction_is_create && current_call_frame.depth == 0)
+                || current_call_frame.create_op_called
+            {
+                let contract_code = std::mem::take(&mut current_call_frame.output);
+                let code_length = contract_code.len();
+
+                let code_length_u64: u64 = code_length
+                    .try_into()
+                    .map_err(|_| VMError::Internal(InternalError::ConversionError))?;
+
+                let code_deposit_cost: u64 =
+                    code_length_u64
+                        .checked_mul(CODE_DEPOSIT_COST)
+                        .ok_or(VMError::Internal(
+                            InternalError::ArithmeticOperationOverflow,
+                        ))?;
+
+                // Revert
+                // If the first byte of code is 0xef
+                // If the code_length > MAX_CODE_SIZE
+                // If current_consumed_gas + code_deposit_cost > gas_limit
+                let validate_create = if code_length > MAX_CODE_SIZE {
                     Err(VMError::ContractOutputTooBig)
                 } else if contract_code.first().unwrap_or(&0) == &INVALID_CONTRACT_PREFIX {
                     Err(VMError::InvalidContractPrefix)
@@ -194,69 +183,69 @@ impl<'a> VM<'a> {
                     Ok(current_call_frame.to)
                 };
 
-            match validate_create {
-                Ok(new_address) => {
-                    // Set bytecode to new account if success
-                    self.update_account_bytecode(new_address, contract_code)?;
+                match validate_create {
+                    Ok(new_address) => {
+                        // Set bytecode to new account if success
+                        self.update_account_bytecode(new_address, contract_code)?;
+                    }
+                    Err(error) => {
+                        // Revert if error
+                        current_call_frame.gas_used = current_call_frame.gas_limit;
+                        transaction_result = TxResult::Revert(error);
+                    }
                 }
-                Err(error) => {
-                    // Revert if error
-                    current_call_frame.gas_used = current_call_frame.gas_limit;
-                    self.restore_state(backup, current_call_frame.cache_backup.clone())?;
+            }
+        }
 
-                    return Ok(ExecutionReport {
-                        result: TxResult::Revert(error),
-                        gas_used: current_call_frame.gas_used,
-                        gas_refunded: self.env.refunded_gas,
-                        output: std::mem::take(&mut current_call_frame.output),
-                        logs: vec![],
-                    });
-                }
+        let logs;
+        match transaction_result {
+            TxResult::Success => {
+                logs = std::mem::take(&mut self.current_call_frame_mut()?.logs);
+            }
+            TxResult::Revert(_) => {
+                logs = vec![];
             }
         }
+        let refunded_gas = self.env.refunded_gas;
+        let current_call_frame = self.current_call_frame_mut()?;
 
         Ok(ExecutionReport {
-            result: TxResult::Success,
+            result: transaction_result,
             gas_used: current_call_frame.gas_used,
-            gas_refunded: self.env.refunded_gas,
+            gas_refunded: refunded_gas,
             output: std::mem::take(&mut current_call_frame.output),
-            logs: std::mem::take(&mut current_call_frame.logs),
+            logs,
         })
     }
 
-    pub fn handle_opcode_error(
-        &mut self,
-        error: VMError,
-        current_call_frame: &mut CallFrame,
-    ) -> Result<ExecutionReport, VMError> {
-        let backup = self
-            .backups
-            .pop()
-            .ok_or(VMError::Internal(InternalError::CouldNotPopCallframe))?;
+    pub fn handle_opcode_error(&mut self, error: VMError) -> Result<ExecutionReport, VMError> {
         if error.is_internal() {
             return Err(error);
         }
 
-        // Unless error is from Revert opcode, all gas is consumed
-        if error != VMError::RevertOpcode {
-            let left_gas = current_call_frame
-                .gas_limit
-                .saturating_sub(current_call_frame.gas_used);
-            current_call_frame.gas_used = current_call_frame.gas_used.saturating_add(left_gas);
-        }
-
-        let refunded = backup.refunded_gas;
-        let output = std::mem::take(&mut current_call_frame.output); // Bytes::new() if error is not RevertOpcode
-        let gas_used = current_call_frame.gas_used;
+        let execution_report;
 
-        self.restore_state(backup, current_call_frame.cache_backup.clone())?;
+        {
+            let current_call_frame = self.current_call_frame_mut()?;
+            // Unless error is from Revert opcode, all gas is consumed
+            if error != VMError::RevertOpcode {
+                let left_gas = current_call_frame
+                    .gas_limit
+                    .saturating_sub(current_call_frame.gas_used);
+                current_call_frame.gas_used = current_call_frame.gas_used.saturating_add(left_gas);
+            }
+            let refunded = current_call_frame.state_backup.refunded_gas;
+            let output = std::mem::take(&mut current_call_frame.output); // Bytes::new() if error is not RevertOpcode
+            let gas_used = current_call_frame.gas_used;
+            execution_report = Ok(ExecutionReport {
+                result: TxResult::Revert(error),
+                gas_used,
+                gas_refunded: refunded,
+                output,
+                logs: vec![],
+            });
+        }
 
-        Ok(ExecutionReport {
-            result: TxResult::Revert(error),
-            gas_used,
-            gas_refunded: refunded,
-            output,
-            logs: vec![],
-        })
+        execution_report
     }
 }