refactor(Inspector): add CallOutcome to call/call_end (bluealloy#985)

* rename main Evm structs, introduce wip external type

* tests

* Split evm and external context

* continue previous commit

* wip inspector handle register

* add few more handlers for frame and host

* Add instruction handle

* add instruction handler registration and Inspector wrap

* Rm Spec generic, more handlers, start factory

* move towards the builder, allow EVM modify

* wip on EvmBuilder and modify functionality

* EvmBuilder with stages wip

* Add wip stages for builer

* make handle register simple function, add raw instruction table, split external data from registers

* wip on simple builder functions and handler registry

* Examples and cleanup

* fix lifetime and fmt

* Add more handlers, deduct caller, validate tx agains state

* All handlers counted, started on docs, some cleanup

* renaming and docs

* Support all Inspector functionality with Handler

* Handler restructured. Documentation added

* more docs on registers

* integrate builder, fmt, move optimism l1block

* add utility builder stage functions

* add precompiles, fix bugs with journal spec

* spec to generic, optimism build

* fix optimism test

* fuck macros

* clippy and fmt

* fix trace block example

* ci fixes

* Flatten builder stages to generic and handler stage

* EvmBuilder doc and refactor fn access

* ignore rust code in book

* make revme compile, will refactor this in future

* Rename handles to Pre/Post Execution and ExecutionLoop

* fix optimism clippy

* small rename

* FrameData and docs

* check links mdbook

* comments and cleanup

* comment

* Add initialize interepreter to first frame

* clippy

* clippy2

* feat: add callOutcome and added to inspector.call

* added meaningful rust docs

Author: loocapro

crates/interpreter/src/call_outcome.rs

@@ -0,0 +1,89 @@
+use crate::{Gas, InstructionResult, InterpreterResult};
+use core::ops::Range;
+use revm_primitives::Bytes;
+
+/// Represents the outcome of a call operation in a virtual machine.
+///
+/// This struct encapsulates the result of executing an instruction by an interpreter, including
+/// the result itself, gas usage information, and the memory offset where output data is stored.
+///
+/// # Fields
+///
+/// * `interpreter_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.
+pub struct CallOutcome {
+    pub interpreter_result: InterpreterResult,
+    pub memory_offset: Range<usize>,
+}
+
+impl CallOutcome {
+    /// Constructs a new `CallOutcome`.
+    ///
+    /// Creates an instance of `CallOutcome` with the given interpreter result and memory offset.
+    ///
+    /// # Arguments
+    ///
+    /// * `interpreter_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 {
+        Self {
+            interpreter_result,
+            memory_offset,
+        }
+    }
+
+    /// Returns a reference to the instruction result.
+    ///
+    /// Provides access to the result of the executed instruction.
+    ///
+    /// # Returns
+    ///
+    /// A reference to the `InstructionResult`.
+    pub fn instruction_result(&self) -> &InstructionResult {
+        &self.interpreter_result.result
+    }
+
+    /// Returns the gas usage information.
+    ///
+    /// Provides access to the gas usage details of the executed instruction.
+    ///
+    /// # Returns
+    ///
+    /// An instance of `Gas` representing the gas usage.
+    pub fn gas(&self) -> Gas {
+        self.interpreter_result.gas
+    }
+
+    /// Returns a reference to the output data.
+    ///
+    /// Provides access to the output data generated by the executed instruction.
+    ///
+    /// # Returns
+    ///
+    /// A reference to the output data as `Bytes`.
+    pub fn output(&self) -> &Bytes {
+        &self.interpreter_result.output
+    }
+
+    /// Returns the start position of the memory offset.
+    ///
+    /// Provides the starting index of the memory range where the output data is stored.
+    ///
+    /// # Returns
+    ///
+    /// The starting index of the memory offset as `usize`.
+    pub fn memory_start(&self) -> usize {
+        self.memory_offset.start
+    }
+
+    /// Returns the length of the memory range.
+    ///
+    /// Provides the length of the memory range where the output data is stored.
+    ///
+    /// # Returns
+    ///
+    /// The length of the memory range as `usize`.
+    pub fn memory_length(&self) -> usize {
+        self.memory_offset.len()
+    }
+}

crates/interpreter/src/interpreter.rs

@@ -10,8 +10,8 @@ pub use stack::{Stack, STACK_LIMIT};
 
 use crate::alloc::borrow::ToOwned;
 use crate::{
-    primitives::Bytes, push, push_b256, return_ok, return_revert, CallInputs, CreateInputs,
-    CreateOutcome, Gas, Host, InstructionResult,
+    primitives::Bytes, push, push_b256, return_ok, return_revert, CallInputs, CallOutcome,
+    CreateInputs, CreateOutcome, Gas, Host, InstructionResult,
 };
 use alloc::boxed::Box;
 use core::cmp::min;
@@ -150,32 +150,51 @@ impl Interpreter {
         }
     }
 
-    /// When sub call returns we can insert output of that call into this interpreter.
+    /// Inserts the outcome of a call into the virtual machine's state.
     ///
-    /// Note that shared memory is required as a input field.
-    /// As SharedMemory inside Interpreter is taken and replaced with empty (not valid) memory.
-    pub fn insert_call_output(
+    /// This function takes the result of a call, represented by `CallOutcome`,
+    /// and updates the virtual machine's state accordingly. It involves updating
+    /// the return data buffer, handling gas accounting, and setting the memory
+    /// in shared storage based on the outcome of the call.
+    ///
+    /// # Arguments
+    ///
+    /// * `shared_memory` - A mutable reference to the shared memory used by the virtual machine.
+    /// * `call_outcome` - The outcome of the call to be processed, containing details such as
+    ///   instruction result, gas information, and output data.
+    ///
+    /// # Behavior
+    ///
+    /// The function first copies the output data from the call outcome to the virtual machine's
+    /// return data buffer. It then checks the instruction result from the call outcome:
+    ///
+    /// - `return_ok!()`: Processes successful execution, refunds gas, and updates shared memory.
+    /// - `return_revert!()`: Handles a revert by only updating the gas usage and shared memory.
+    /// - `InstructionResult::FatalExternalError`: Sets the instruction result to a fatal external error.
+    /// - Any other result: No specific action is taken.
+    pub fn insert_call_outcome(
         &mut self,
         shared_memory: &mut SharedMemory,
-        result: InterpreterResult,
-        memory_return_offset: Range<usize>,
+        call_outcome: CallOutcome,
     ) {
-        let out_offset = memory_return_offset.start;
-        let out_len = memory_return_offset.len();
+        let out_offset = call_outcome.memory_start();
+        let out_len = call_outcome.memory_length();
 
-        self.return_data_buffer = result.output;
+        self.return_data_buffer = call_outcome.output().to_owned();
         let target_len = min(out_len, self.return_data_buffer.len());
 
-        match result.result {
+        match call_outcome.instruction_result() {
             return_ok!() => {
                 // return unspend gas.
-                self.gas.erase_cost(result.gas.remaining());
-                self.gas.record_refund(result.gas.refunded());
+                let remaining = call_outcome.gas().remaining();
+                let refunded = call_outcome.gas().refunded();
+                self.gas.erase_cost(remaining);
+                self.gas.record_refund(refunded);
                 shared_memory.set(out_offset, &self.return_data_buffer[..target_len]);
                 push!(self, U256::from(1));
             }
             return_revert!() => {
-                self.gas.erase_cost(result.gas.remaining());
+                self.gas.erase_cost(call_outcome.gas().remaining());
                 shared_memory.set(out_offset, &self.return_data_buffer[..target_len]);
                 push!(self, U256::ZERO);
             }

crates/interpreter/src/lib.rs

@@ -12,6 +12,7 @@ extern crate alloc;
 #[macro_use]
 mod macros;
 
+mod call_outcome;
 mod create_outcome;
 pub mod gas;
 mod host;
@@ -21,6 +22,7 @@ pub mod instructions;
 mod interpreter;
 
 // Reexport primary types.
+pub use call_outcome::CallOutcome;
 pub use create_outcome::CreateOutcome;
 pub use gas::Gas;
 pub use host::{DummyHost, Host};

crates/revm/src/handler/mainnet/execution_loop.rs

@@ -9,6 +9,7 @@ use crate::{
 };
 use alloc::boxed::Box;
 use core::ops::Range;
+use revm_interpreter::CallOutcome;
 
 /// Creates first frame.
 #[inline]
@@ -106,12 +107,11 @@ pub fn frame_return<SPEC: Spec, EXT, DB: Database>(
             let Some(parent_stack_frame) = parent_stack_frame else {
                 return Some(result);
             };
+            let call_outcome = CallOutcome::new(result, return_memory_range);
 
-            parent_stack_frame.interpreter.insert_call_output(
-                shared_memory,
-                result,
-                return_memory_range,
-            )
+            parent_stack_frame
+                .interpreter
+                .insert_call_outcome(shared_memory, call_outcome)
         }
     }
     None
@@ -132,11 +132,10 @@ pub fn sub_call<SPEC: Spec, EXT, DB: Database>(
     {
         FrameOrResult::Frame(new_frame) => Some(new_frame),
         FrameOrResult::Result(result) => {
-            curent_stack_frame.interpreter.insert_call_output(
-                shared_memory,
-                result,
-                return_memory_offset,
-            );
+            let call_outcome = CallOutcome::new(result, return_memory_offset);
+            curent_stack_frame
+                .interpreter
+                .insert_call_outcome(shared_memory, call_outcome);
             None
         }
     }

crates/revm/src/inspector.rs

@@ -1,5 +1,3 @@
-use core::ops::Range;
-
 use crate::{
     interpreter::{CallInputs, CreateInputs, Interpreter},
     primitives::{db::Database, Address, Log, U256},
@@ -18,7 +16,8 @@ mod noop;
 // Exports.
 
 pub use handler_register::{inspector_handle_register, inspector_instruction, GetInspector};
-use revm_interpreter::{CreateOutcome, InterpreterResult};
+use revm_interpreter::{CallOutcome, CreateOutcome, InterpreterResult};
+
 /// [Inspector] implementations.
 pub mod inspectors {
     #[cfg(feature = "std")]
@@ -81,7 +80,7 @@ pub trait Inspector<DB: Database> {
         &mut self,
         context: &mut EvmContext<DB>,
         inputs: &mut CallInputs,
-    ) -> Option<(InterpreterResult, Range<usize>)> {
+    ) -> Option<CallOutcome> {
         let _ = context;
         let _ = inputs;
         None

crates/revm/src/inspector/customprinter.rs

@@ -1,7 +1,7 @@
 //! Custom print inspector, it has step level information of execution.
 //! It is a great tool if some debugging is needed.
 
-use core::ops::Range;
+use revm_interpreter::CallOutcome;
 use revm_interpreter::CreateOutcome;
 
 use crate::{
@@ -82,7 +82,7 @@ impl<DB: Database> Inspector<DB> for CustomPrintTracer {
         &mut self,
         _context: &mut EvmContext<DB>,
         inputs: &mut CallInputs,
-    ) -> Option<(InterpreterResult, Range<usize>)> {
+    ) -> Option<CallOutcome> {
         println!(
             "SM CALL:   {:?}, context:{:?}, is_static:{:?}, transfer:{:?}, input_size:{:?}",
             inputs.contract,

crates/revm/src/inspector/eip3155.rs

@@ -4,7 +4,8 @@ use crate::{
     primitives::{db::Database, hex, Address, U256},
     EvmContext, GetInspector, Inspector,
 };
-use core::ops::Range;
+
+use revm_interpreter::CallOutcome;
 use revm_interpreter::CreateOutcome;
 use serde_json::json;
 use std::io::Write;
@@ -90,7 +91,7 @@ impl<DB: Database> Inspector<DB> for TracerEip3155 {
         &mut self,
         _context: &mut EvmContext<DB>,
         _inputs: &mut CallInputs,
-    ) -> Option<(InterpreterResult, Range<usize>)> {
+    ) -> Option<CallOutcome> {
         None
     }
 

crates/revm/src/inspector/gas.rs

@@ -68,6 +68,7 @@ impl<DB: Database> Inspector<DB> for GasInspector {
 
 #[cfg(test)]
 mod tests {
+    use revm_interpreter::CallOutcome;
     use revm_interpreter::CreateOutcome;
 
     use crate::{
@@ -77,7 +78,6 @@ mod tests {
         primitives::{Address, Log},
         Database, EvmContext, Inspector,
     };
-    use core::ops::Range;
 
     #[derive(Default, Debug)]
     struct StackInspector {
@@ -116,7 +116,7 @@ mod tests {
             &mut self,
             context: &mut EvmContext<DB>,
             call: &mut CallInputs,
-        ) -> Option<(InterpreterResult, Range<usize>)> {
+        ) -> Option<CallOutcome> {
             self.gas_inspector.call(context, call)
         }
 

crates/revm/src/inspector/handler_register.rs

@@ -9,7 +9,7 @@ use crate::{
     CallStackFrame, Evm, FrameData, FrameOrResult, Inspector, JournalEntry,
 };
 use alloc::{boxed::Box, sync::Arc, vec::Vec};
-use revm_interpreter::CreateInputs;
+use revm_interpreter::{CallOutcome, CreateInputs};
 
 pub trait GetInspector<'a, DB: Database> {
     fn get_inspector(&mut self) -> &mut dyn Inspector<DB>;
@@ -106,7 +106,7 @@ pub fn inspector_handle_register<'a, DB: Database, EXT: GetInspector<'a, DB>>(
                         .get_inspector()
                         .call(&mut context.evm, &mut call_inputs)
                     {
-                        return FrameOrResult::Result(output.0);
+                        return FrameOrResult::Result(output.interpreter_result);
                     }
                     // first call frame does not have return range.
                     context.evm.make_call_frame(&call_inputs, 0..0)
@@ -203,8 +203,8 @@ pub fn inspector_handle_register<'a, DB: Database, EXT: GetInspector<'a, DB>>(
         move |context, mut inputs, frame, memory, return_memory_offset| -> Option<Box<_>> {
             // inspector handle
             let inspector = context.external.get_inspector();
-            if let Some((result, range)) = inspector.call(&mut context.evm, &mut inputs) {
-                frame.interpreter.insert_call_output(memory, result, range);
+            if let Some(call_outcome) = inspector.call(&mut context.evm, &mut inputs) {
+                frame.interpreter.insert_call_outcome(memory, call_outcome);
                 return None;
             }
             match context
@@ -218,9 +218,8 @@ pub fn inspector_handle_register<'a, DB: Database, EXT: GetInspector<'a, DB>>(
                 FrameOrResult::Result(result) => {
                     // inspector handle
                     let result = inspector.call_end(&mut context.evm, result);
-                    frame
-                        .interpreter
-                        .insert_call_output(memory, result, return_memory_offset);
+                    let call_outcome = CallOutcome::new(result, return_memory_offset);
+                    frame.interpreter.insert_call_outcome(memory, call_outcome);
                     None
                 }
             }
@@ -299,7 +298,7 @@ mod tests {
         primitives::{Address, BerlinSpec},
         Database, Evm, EvmContext, Inspector,
     };
-    use core::ops::Range;
+
     use revm_interpreter::CreateOutcome;
 
     #[test]
@@ -349,7 +348,7 @@ mod tests {
             &mut self,
             _context: &mut EvmContext<DB>,
             _call: &mut CallInputs,
-        ) -> Option<(InterpreterResult, Range<usize>)> {
+        ) -> Option<CallOutcome> {
             if self.call {
                 unreachable!("call should not be called twice")
             }