refactor(Execution): Granular handles create/call,call_return,insert_…

…call_outcome (#1024)

* fix(State): Preserve original values on delete revert

* clear storage only if original is None

* refactor(ExecutionHandle): Introduce more granular handles create/call,call_return,insert_call_outcome

* cleanup, rm comments, let Inspector use CallOutput

* update doc

* add alloc box

crates/interpreter/src/call_outcome.rs

@@ -9,10 +9,11 @@ use revm_primitives::Bytes;
 ///
 /// # Fields
 ///
-/// * `interpreter_result` - The result of the interpreter's execution, including output data and gas usage.
+/// * `result` - The result of the interpreter's execution, including output data and gas usage.
 /// * `memory_offset` - The range in memory where the output data is located.
+#[derive(Clone, Debug)]
 pub struct CallOutcome {
-    pub interpreter_result: InterpreterResult,
+    pub result: InterpreterResult,
     pub memory_offset: Range<usize>,
 }
 
@@ -23,11 +24,11 @@ impl CallOutcome {
     ///
     /// # Arguments
     ///
-    /// * `interpreter_result` - The result of the interpreter's execution.
+    /// * `result` - The result of the interpreter's execution.
     /// * `memory_offset` - The range in memory indicating where the output data is stored.
-    pub fn new(interpreter_result: InterpreterResult, memory_offset: Range<usize>) -> Self {
+    pub fn new(result: InterpreterResult, memory_offset: Range<usize>) -> Self {
         Self {
-            interpreter_result,
+            result,
             memory_offset,
         }
     }
@@ -40,7 +41,7 @@ impl CallOutcome {
     ///
     /// A reference to the `InstructionResult`.
     pub fn instruction_result(&self) -> &InstructionResult {
-        &self.interpreter_result.result
+        &self.result.result
     }
 
     /// Returns the gas usage information.
@@ -51,7 +52,7 @@ impl CallOutcome {
     ///
     /// An instance of `Gas` representing the gas usage.
     pub fn gas(&self) -> Gas {
-        self.interpreter_result.gas
+        self.result.gas
     }
 
     /// Returns a reference to the output data.
@@ -62,7 +63,7 @@ impl CallOutcome {
     ///
     /// A reference to the output data as `Bytes`.
     pub fn output(&self) -> &Bytes {
-        &self.interpreter_result.output
+        &self.result.output
     }
 
     /// Returns the start position of the memory offset.

crates/interpreter/src/create_outcome.rs

@@ -5,6 +5,7 @@ use revm_primitives::{Address, Bytes};
 ///
 /// This struct holds the result of the operation along with an optional address.
 /// It provides methods to determine the next action based on the result of the operation.
+#[derive(Debug, Clone)]
 pub struct CreateOutcome {
     // The result of the interpreter operation.
     pub result: InterpreterResult,

crates/interpreter/src/inner_models.rs

@@ -1,7 +1,7 @@
-use revm_primitives::{TransactTo, TxEnv};
+use crate::primitives::{Address, Bytes, TransactTo, TxEnv, U256};
+use alloc::boxed::Box;
 
 pub use crate::primitives::CreateScheme;
-use crate::primitives::{Address, Bytes, U256};
 
 /// Inputs for a call.
 #[derive(Clone, Debug, PartialEq, Eq, Hash)]
@@ -38,6 +38,7 @@ pub struct CreateInputs {
 }
 
 impl CallInputs {
+    /// Creates new call inputs.
     pub fn new(tx_env: &TxEnv, gas_limit: u64) -> Option<Self> {
         let TransactTo::Call(address) = tx_env.transact_to else {
             return None;
@@ -62,9 +63,15 @@ impl CallInputs {
             is_static: false,
         })
     }
+
+    /// Returns boxed call inputs.
+    pub fn new_boxed(tx_env: &TxEnv, gas_limit: u64) -> Option<Box<Self>> {
+        Self::new(tx_env, gas_limit).map(Box::new)
+    }
 }
 
 impl CreateInputs {
+    /// Creates new create inputs.
     pub fn new(tx_env: &TxEnv, gas_limit: u64) -> Option<Self> {
         let TransactTo::Create(scheme) = tx_env.transact_to else {
             return None;
@@ -78,6 +85,12 @@ impl CreateInputs {
             gas_limit,
         })
     }
+
+    /// Returns boxed create inputs.
+    pub fn new_boxed(tx_env: &TxEnv, gas_limit: u64) -> Option<Box<Self>> {
+        Self::new(tx_env, gas_limit).map(Box::new)
+    }
+
     /// Returns the address that this create call will create.
     pub fn created_address(&self, nonce: u64) -> Address {
         match self.scheme {

crates/interpreter/src/instruction_result.rs

@@ -166,7 +166,7 @@ pub enum SuccessOrHalt {
     FatalExternalError,
     /// Internal instruction that signals Interpreter should continue running.
     InternalContinue,
-    /// Internal instruction that signals subcall.
+    /// Internal instruction that signals call or create.
     InternalCallOrCreate,
 }
 

crates/interpreter/src/instructions/control.rs

@@ -61,13 +61,13 @@ fn return_inner(interpreter: &mut Interpreter, instruction_result: InstructionRe
         output = interpreter.shared_memory.slice(offset, len).to_vec().into()
     }
     interpreter.instruction_result = instruction_result;
-    interpreter.next_action = Some(crate::InterpreterAction::Return {
+    interpreter.next_action = crate::InterpreterAction::Return {
         result: InterpreterResult {
             output,
             gas: interpreter.gas,
             result: instruction_result,
         },
-    });
+    };
 }
 
 pub fn ret<H: Host>(interpreter: &mut Interpreter, _host: &mut H) {

crates/interpreter/src/instructions/host.rs

@@ -298,15 +298,15 @@ pub fn create<const IS_CREATE2: bool, H: Host, SPEC: Spec>(
     gas!(interpreter, gas_limit);
 
     // Call host to interact with target contract
-    interpreter.next_action = Some(InterpreterAction::Create {
+    interpreter.next_action = InterpreterAction::Create {
         inputs: Box::new(CreateInputs {
             caller: interpreter.contract.address,
             scheme,
             value,
             init_code: code,
             gas_limit,
         }),
-    });
+    };
     interpreter.instruction_result = InstructionResult::CallOrCreate;
 }
 
@@ -346,7 +346,7 @@ pub fn call<H: Host, SPEC: Spec>(interpreter: &mut Interpreter, host: &mut H) {
     }
 
     // Call host to interact with target contract
-    interpreter.next_action = Some(InterpreterAction::SubCall {
+    interpreter.next_action = InterpreterAction::Call {
         inputs: Box::new(CallInputs {
             contract: to,
             transfer: Transfer {
@@ -366,7 +366,7 @@ pub fn call<H: Host, SPEC: Spec>(interpreter: &mut Interpreter, host: &mut H) {
             is_static: interpreter.is_static,
         }),
         return_memory_offset,
-    });
+    };
     interpreter.instruction_result = InstructionResult::CallOrCreate;
 }
 
@@ -401,7 +401,7 @@ pub fn call_code<H: Host, SPEC: Spec>(interpreter: &mut Interpreter, host: &mut
     }
 
     // Call host to interact with target contract
-    interpreter.next_action = Some(InterpreterAction::SubCall {
+    interpreter.next_action = InterpreterAction::Call {
         inputs: Box::new(CallInputs {
             contract: to,
             transfer: Transfer {
@@ -421,7 +421,7 @@ pub fn call_code<H: Host, SPEC: Spec>(interpreter: &mut Interpreter, host: &mut
             is_static: interpreter.is_static,
         }),
         return_memory_offset,
-    });
+    };
     interpreter.instruction_result = InstructionResult::CallOrCreate;
 }
 
@@ -445,7 +445,7 @@ pub fn delegate_call<H: Host, SPEC: Spec>(interpreter: &mut Interpreter, host: &
     gas!(interpreter, gas_limit);
 
     // Call host to interact with target contract
-    interpreter.next_action = Some(InterpreterAction::SubCall {
+    interpreter.next_action = InterpreterAction::Call {
         inputs: Box::new(CallInputs {
             contract: to,
             // This is dummy send for StaticCall and DelegateCall,
@@ -467,7 +467,7 @@ pub fn delegate_call<H: Host, SPEC: Spec>(interpreter: &mut Interpreter, host: &
             is_static: interpreter.is_static,
         }),
         return_memory_offset,
-    });
+    };
     interpreter.instruction_result = InstructionResult::CallOrCreate;
 }
 
@@ -491,7 +491,7 @@ pub fn static_call<H: Host, SPEC: Spec>(interpreter: &mut Interpreter, host: &mu
     gas!(interpreter, gas_limit);
 
     // Call host to interact with target contract
-    interpreter.next_action = Some(InterpreterAction::SubCall {
+    interpreter.next_action = InterpreterAction::Call {
         inputs: Box::new(CallInputs {
             contract: to,
             // This is dummy send for StaticCall and DelegateCall,
@@ -513,6 +513,6 @@ pub fn static_call<H: Host, SPEC: Spec>(interpreter: &mut Interpreter, host: &mu
             is_static: true,
         }),
         return_memory_offset,
-    });
+    };
     interpreter.instruction_result = InstructionResult::CallOrCreate;
 }

crates/interpreter/src/interpreter.rs

@@ -50,7 +50,7 @@ pub struct Interpreter {
     ///
     /// Set inside CALL or CREATE instructions and RETURN or REVERT instructions. Additionally those instructions will set
     /// InstructionResult to CallOrCreate/Return/Revert so we know the reason.
-    pub next_action: Option<InterpreterAction>,
+    pub next_action: InterpreterAction,
 }
 
 /// The result of an interpreter operation.
@@ -64,22 +64,59 @@ pub struct InterpreterResult {
     pub gas: Gas,
 }
 
-#[derive(Debug, Clone)]
+#[derive(Debug, Default, Clone)]
 pub enum InterpreterAction {
-    SubCall {
+    /// CALL, CALLCODE, DELEGATECALL or STATICCALL instruction called.
+    Call {
         /// Call inputs
         inputs: Box<CallInputs>,
         /// The offset into `self.memory` of the return data.
         ///
         /// This value must be ignored if `self.return_len` is 0.
         return_memory_offset: Range<usize>,
     },
-    Create {
-        inputs: Box<CreateInputs>,
-    },
-    Return {
-        result: InterpreterResult,
-    },
+    /// CREATE or CREATE2 instruction called.
+    Create { inputs: Box<CreateInputs> },
+    /// Interpreter finished execution.
+    Return { result: InterpreterResult },
+    /// No action
+    #[default]
+    None,
+}
+
+impl InterpreterAction {
+    /// Returns true if action is call.
+    pub fn is_call(&self) -> bool {
+        matches!(self, InterpreterAction::Call { .. })
+    }
+
+    /// Returns true if action is create.
+    pub fn is_create(&self) -> bool {
+        matches!(self, InterpreterAction::Create { .. })
+    }
+
+    /// Returns true if action is return.
+    pub fn is_return(&self) -> bool {
+        matches!(self, InterpreterAction::Return { .. })
+    }
+
+    /// Returns true if action is none.
+    pub fn is_none(&self) -> bool {
+        matches!(self, InterpreterAction::None)
+    }
+
+    /// Returns true if action is some.
+    pub fn is_some(&self) -> bool {
+        !self.is_none()
+    }
+
+    /// Returns result if action is return.
+    pub fn into_result_return(self) -> Option<InterpreterResult> {
+        match self {
+            InterpreterAction::Return { result } => Some(result),
+            _ => None,
+        }
+    }
 }
 
 impl Interpreter {
@@ -94,7 +131,7 @@ impl Interpreter {
             return_data_buffer: Bytes::new(),
             shared_memory: EMPTY_SHARED_MEMORY,
             stack: Stack::new(),
-            next_action: None,
+            next_action: InterpreterAction::None,
         }
     }
 
@@ -281,7 +318,7 @@ impl Interpreter {
     where
         FN: Fn(&mut Interpreter, &mut H),
     {
-        self.next_action = None;
+        self.next_action = InterpreterAction::None;
         self.instruction_result = InstructionResult::Continue;
         self.shared_memory = shared_memory;
         // main loop
@@ -290,10 +327,10 @@ impl Interpreter {
         }
 
         // Return next action if it is some.
-        if let Some(action) = self.next_action.take() {
-            return action;
+        if self.next_action.is_some() {
+            return core::mem::take(&mut self.next_action);
         }
-        // If not, return action without output.
+        // If not, return action without output as it is a halt.
         InterpreterAction::Return {
             result: InterpreterResult {
                 result: self.instruction_result,