Moved call frame popping to return handling functions

Author: SDartayet

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

@@ -926,35 +926,35 @@ impl<'a> VM<'a> {
         tx_report: &ExecutionReport,
         retdata: RetData,
     ) -> Result<(), VMError> {
+        let previous_call_frame = self.call_frames.pop().ok_or(VMError::Internal(InternalError::CouldNotPopCallframe))?;
         // Return gas left from subcontext
-        let gas_left_from_new_call_frame = self
-            .current_call_frame()?
+        let gas_left_from_new_call_frame = previous_call_frame
             .gas_limit
             .checked_sub(tx_report.gas_used)
             .ok_or(InternalError::GasOverflow)?;
         {
-            let next_call_frame = self.next_call_frame_mut()?;
-            next_call_frame.gas_used = next_call_frame
+            let current_call_frame = self.current_call_frame_mut()?;
+            current_call_frame.gas_used = current_call_frame
                 .gas_used
                 .checked_sub(gas_left_from_new_call_frame)
                 .ok_or(InternalError::GasOverflow)?;
 
-            next_call_frame.logs.extend(tx_report.logs.clone());
+            current_call_frame.logs.extend(tx_report.logs.clone());
             memory::try_store_range(
-                &mut next_call_frame.memory,
+                &mut current_call_frame.memory,
                 retdata.ret_offset,
                 retdata.ret_size,
                 &tx_report.output,
             )?;
-            next_call_frame.sub_return_data = tx_report.output.clone();
+            current_call_frame.sub_return_data = tx_report.output.clone();
         }
 
         // What to do, depending on TxResult
         match tx_report.result {
             TxResult::Success => {
-                self.next_call_frame_mut()?.stack.push(SUCCESS_FOR_CALL)?;
-                for (address, account_opt) in self.current_call_frame()?.cache_backup.clone() {
-                    self.next_call_frame_mut()?
+                self.current_call_frame_mut()?.stack.push(SUCCESS_FOR_CALL)?;
+                for (address, account_opt) in previous_call_frame.cache_backup.clone() {
+                    self.current_call_frame_mut()?
                         .cache_backup
                         .entry(address)
                         .or_insert(account_opt);
@@ -968,11 +968,12 @@ impl<'a> VM<'a> {
                     self.increase_account_balance(retdata.msg_sender, retdata.value)?;
                 }
                 // Push 0 to stack
-                self.next_call_frame_mut()?.stack.push(REVERT_FOR_CALL)?;
+                self.current_call_frame_mut()?.stack.push(REVERT_FOR_CALL)?;
             }
         }
         Ok(())
     }
+
     pub fn handle_return_create(
         &mut self,
         tx_report: &ExecutionReport,
@@ -982,25 +983,25 @@ impl<'a> VM<'a> {
             .max_message_call_gas
             .checked_sub(tx_report.gas_used)
             .ok_or(InternalError::GasOverflow)?;
-
+        let previous_call_frame = self.call_frames.pop().ok_or(VMError::Internal(InternalError::CouldNotPopCallframe))?;
         {
-            let next_call_frame = self.next_call_frame_mut()?;
+            let current_call_frame = self.current_call_frame_mut()?;
             // Return reserved gas
-            next_call_frame.gas_used = next_call_frame
+            current_call_frame.gas_used = current_call_frame
                 .gas_used
                 .checked_sub(unused_gas)
                 .ok_or(InternalError::GasOverflow)?;
 
-            next_call_frame.logs.extend(tx_report.logs.clone());
+            current_call_frame.logs.extend(tx_report.logs.clone());
         }
 
         match tx_report.result.clone() {
             TxResult::Success => {
-                self.next_call_frame_mut()?
+                self.current_call_frame_mut()?
                     .stack
                     .push(address_to_word(retdata.to))?;
-                for (address, account_opt) in self.current_call_frame()?.cache_backup.clone() {
-                    self.next_call_frame_mut()?
+                for (address, account_opt) in previous_call_frame.cache_backup.clone() {
+                    self.current_call_frame_mut()?
                         .cache_backup
                         .entry(address)
                         .or_insert(account_opt);
@@ -1014,12 +1015,12 @@ impl<'a> VM<'a> {
                 cache::remove_account(&mut self.db.cache, &retdata.to);
                 self.accrued_substate.created_accounts.remove(&retdata.to);
 
-                let next_call_frame = self.next_call_frame_mut()?;
+                let current_call_frame = self.current_call_frame_mut()?;
                 // If revert we have to copy the return_data
                 if err == VMError::RevertOpcode {
-                    next_call_frame.sub_return_data = tx_report.output.clone();
+                    current_call_frame.sub_return_data = tx_report.output.clone();
                 }
-                next_call_frame.stack.push(CREATE_DEPLOYMENT_FAIL)?;
+                current_call_frame.stack.push(CREATE_DEPLOYMENT_FAIL)?;
             }
         }
         Ok(())

crates/vm/levm/src/vm.rs

@@ -326,9 +326,6 @@ impl<'a> VM<'a> {
                 .pop()
                 .ok_or(VMError::Internal(InternalError::CouldNotPopCallframe))?;
             let report = self.handle_precompile_result(precompile_result, backup)?;
-            if self.handle_return(&report)? {
-                self.return_to_previous_call_frame()?;
-            }
             self.current_call_frame_mut()?.increment_pc_by(1)?;
             return Ok(report);
         }
@@ -345,7 +342,6 @@ impl<'a> VM<'a> {
                 Ok(OpcodeResult::Halt) => {
                     let report = self.handle_opcode_result()?;
                     if self.handle_return(&report)? {
-                        self.return_to_previous_call_frame()?;
                         self.current_call_frame_mut()?.increment_pc_by(1)?;
                     } else {
                         return Ok(report);
@@ -354,7 +350,6 @@ impl<'a> VM<'a> {
                 Err(error) => {
                     let report = self.handle_opcode_error(error)?;
                     if self.handle_return(&report)? {
-                        self.return_to_previous_call_frame()?;
                         self.current_call_frame_mut()?.increment_pc_by(1)?;
                     } else {
                         return Ok(report);
@@ -450,17 +445,6 @@ impl<'a> VM<'a> {
         ))
     }
 
-    pub fn next_call_frame_mut(&mut self) -> Result<&mut CallFrame, VMError> {
-        let second_to_last_index =
-            self.call_frames
-                .len()
-                .checked_sub(2)
-                .ok_or(VMError::Internal(
-                    InternalError::CouldNotAccessLastCallframe,
-                ))?;
-        Ok(&mut self.call_frames[second_to_last_index])
-    }
-
     fn handle_create_non_empty_account(&mut self) -> Result<ExecutionReport, VMError> {
         let mut report = ExecutionReport {
             result: TxResult::Revert(VMError::AddressAlreadyOccupied),