refactor(Inspector): Add CreateOutcome in create/create_end return (#980

)

* 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: create outcome

* fix: createOutcome properties to pub and removed wrapper functions

* review: fixed some review comments and added some improvement over instruction results

* fix: removed unused is_revert method

* review: adjusted comments, moved create_outcome to its own file

* fix: no std check

---------

Co-authored-by: rakita <dragan0rakita@gmail.com>

crates/interpreter/src/create_outcome.rs

@@ -0,0 +1,68 @@
+use crate::{Gas, InstructionResult, InterpreterResult};
+use revm_primitives::{Address, Bytes};
+
+/// Represents the outcome of a create operation in an interpreter.
+///
+/// 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.
+pub struct CreateOutcome {
+    // The result of the interpreter operation.
+    pub result: InterpreterResult,
+    // An optional address associated with the create operation.
+    pub address: Option<Address>,
+}
+
+impl CreateOutcome {
+    /// Constructs a new `CreateOutcome`.
+    ///
+    /// # Arguments
+    ///
+    /// * `result` - An `InterpreterResult` representing the result of the interpreter operation.
+    /// * `address` - An optional `Address` associated with the create operation.
+    ///
+    /// # Returns
+    ///
+    /// A new `CreateOutcome` instance.
+    pub fn new(result: InterpreterResult, address: Option<Address>) -> Self {
+        Self { result, address }
+    }
+
+    /// Retrieves a reference to the `InstructionResult` from the `InterpreterResult`.
+    ///
+    /// This method provides access to the `InstructionResult` which represents the
+    /// outcome of the instruction execution. It encapsulates the result information
+    /// such as whether the instruction was executed successfully, resulted in a revert,
+    /// or encountered a fatal error.
+    ///
+    /// # Returns
+    ///
+    /// A reference to the `InstructionResult`.
+    pub fn instruction_result(&self) -> &InstructionResult {
+        &self.result.result
+    }
+
+    /// Retrieves a reference to the output bytes from the `InterpreterResult`.
+    ///
+    /// This method returns the output of the interpreted operation. The output is
+    /// typically used when the operation successfully completes and returns data.
+    ///
+    /// # Returns
+    ///
+    /// A reference to the output `Bytes`.
+    pub fn output(&self) -> &Bytes {
+        &self.result.output
+    }
+
+    /// Retrieves a reference to the `Gas` details from the `InterpreterResult`.
+    ///
+    /// This method provides access to the gas details of the operation, which includes
+    /// information about gas used, remaining, and refunded. It is essential for
+    /// understanding the gas consumption of the operation.
+    ///
+    /// # Returns
+    ///
+    /// A reference to the `Gas` details.
+    pub fn gas(&self) -> &Gas {
+        &self.result.gas
+    }
+}

crates/interpreter/src/interpreter.rs

@@ -8,14 +8,15 @@ pub use contract::Contract;
 pub use shared_memory::{next_multiple_of_32, SharedMemory};
 pub use stack::{Stack, STACK_LIMIT};
 
+use crate::alloc::borrow::ToOwned;
 use crate::{
-    primitives::Bytes, push, push_b256, return_ok, return_revert, CallInputs, CreateInputs, Gas,
-    Host, InstructionResult,
+    primitives::Bytes, push, push_b256, return_ok, return_revert, CallInputs, CreateInputs,
+    CreateOutcome, Gas, Host, InstructionResult,
 };
 use alloc::boxed::Box;
 use core::cmp::min;
 use core::ops::Range;
-use revm_primitives::{Address, U256};
+use revm_primitives::U256;
 
 pub use self::shared_memory::EMPTY_SHARED_MEMORY;
 
@@ -93,24 +94,52 @@ impl Interpreter {
         }
     }
 
-    /// When sub create call returns we can insert output of that call into this interpreter.
-    pub fn insert_create_output(&mut self, result: InterpreterResult, address: Option<Address>) {
-        self.return_data_buffer = match result.result {
+    /// Inserts the output of a `create` call into the interpreter.
+    ///
+    /// This function is used after a `create` call has been executed. It processes the outcome
+    /// of that call and updates the state of the interpreter accordingly.
+    ///
+    /// # Arguments
+    ///
+    /// * `create_outcome` - A `CreateOutcome` struct containing the results of the `create` call.
+    ///
+    /// # Behavior
+    ///
+    /// The function updates the `return_data_buffer` with the data from `create_outcome`.
+    /// Depending on the `InstructionResult` indicated by `create_outcome`, it performs one of the following:
+    ///
+    /// - `Ok`: Pushes the address from `create_outcome` to the stack, updates gas costs, and records any gas refunds.
+    /// - `Revert`: Pushes `U256::ZERO` to the stack and updates gas costs.
+    /// - `FatalExternalError`: Sets the `instruction_result` to `InstructionResult::FatalExternalError`.
+    /// - `Default`: Pushes `U256::ZERO` to the stack.
+    ///
+    /// # Side Effects
+    ///
+    /// - Updates `return_data_buffer` with the data from `create_outcome`.
+    /// - Modifies the stack by pushing values depending on the `InstructionResult`.
+    /// - Updates gas costs and records refunds in the interpreter's `gas` field.
+    /// - May alter `instruction_result` in case of external errors.
+    pub fn insert_create_outcome(&mut self, create_outcome: CreateOutcome) {
+        let instruction_result = create_outcome.instruction_result();
+
+        self.return_data_buffer = if instruction_result.is_revert() {
             // Save data to return data buffer if the create reverted
-            return_revert!() => result.output,
+            create_outcome.output().to_owned()
+        } else {
             // Otherwise clear it
-            _ => Bytes::new(),
+            Bytes::new()
         };
 
-        match result.result {
+        match instruction_result {
             return_ok!() => {
+                let address = create_outcome.address;
                 push_b256!(self, address.unwrap_or_default().into_word());
-                self.gas.erase_cost(result.gas.remaining());
-                self.gas.record_refund(result.gas.refunded());
+                self.gas.erase_cost(create_outcome.gas().remaining());
+                self.gas.record_refund(create_outcome.gas().refunded());
             }
             return_revert!() => {
                 push!(self, U256::ZERO);
-                self.gas.erase_cost(result.gas.remaining());
+                self.gas.erase_cost(create_outcome.gas().remaining());
             }
             InstructionResult::FatalExternalError => {
                 self.instruction_result = InstructionResult::FatalExternalError;

crates/interpreter/src/lib.rs

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

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

@@ -1,7 +1,7 @@
 use crate::{
     db::Database,
     interpreter::{
-        return_ok, return_revert, CallInputs, CreateInputs, Gas, InstructionResult,
+        return_ok, return_revert, CallInputs, CreateInputs, CreateOutcome, Gas, InstructionResult,
         InterpreterResult, SharedMemory,
     },
     primitives::{Env, Spec, TransactTo},
@@ -92,9 +92,10 @@ pub fn frame_return<SPEC: Spec, EXT, DB: Database>(
             let Some(parent_stack_frame) = parent_stack_frame else {
                 return Some(result);
             };
+            let create_outcome = CreateOutcome::new(result, Some(created_address));
             parent_stack_frame
                 .interpreter
-                .insert_create_output(result, Some(created_address))
+                .insert_create_outcome(create_outcome)
         }
         FrameData::Call {
             return_memory_range,
@@ -151,10 +152,11 @@ pub fn sub_create<SPEC: Spec, EXT, DB: Database>(
     match context.evm.make_create_frame(SPEC::SPEC_ID, &inputs) {
         FrameOrResult::Frame(new_frame) => Some(new_frame),
         FrameOrResult::Result(result) => {
+            let create_outcome = CreateOutcome::new(result, None);
             // insert result of the failed creation of create CallStackFrame.
             curent_stack_frame
                 .interpreter
-                .insert_create_output(result, None);
+                .insert_create_outcome(create_outcome);
             None
         }
     }

crates/revm/src/inspector.rs

@@ -18,7 +18,7 @@ mod noop;
 // Exports.
 
 pub use handler_register::{inspector_handle_register, inspector_instruction, GetInspector};
-use revm_interpreter::InterpreterResult;
+use revm_interpreter::{CreateOutcome, InterpreterResult};
 /// [Inspector] implementations.
 pub mod inspectors {
     #[cfg(feature = "std")]
@@ -109,7 +109,7 @@ pub trait Inspector<DB: Database> {
         &mut self,
         context: &mut EvmContext<DB>,
         inputs: &mut CreateInputs,
-    ) -> Option<(InterpreterResult, Option<Address>)> {
+    ) -> Option<CreateOutcome> {
         let _ = context;
         let _ = inputs;
         None
@@ -125,9 +125,9 @@ pub trait Inspector<DB: Database> {
         context: &mut EvmContext<DB>,
         result: InterpreterResult,
         address: Option<Address>,
-    ) -> (InterpreterResult, Option<Address>) {
+    ) -> CreateOutcome {
         let _ = context;
-        (result, address)
+        CreateOutcome::new(result, address)
     }
 
     /// Called when a contract has been self-destructed with funds transferred to target.

crates/revm/src/inspector/customprinter.rs

@@ -2,6 +2,7 @@
 //! It is a great tool if some debugging is needed.
 
 use core::ops::Range;
+use revm_interpreter::CreateOutcome;
 
 use crate::{
     inspectors::GasInspector,
@@ -73,7 +74,7 @@ impl<DB: Database> Inspector<DB> for CustomPrintTracer {
         context: &mut EvmContext<DB>,
         result: InterpreterResult,
         address: Option<Address>,
-    ) -> (InterpreterResult, Option<Address>) {
+    ) -> CreateOutcome {
         self.gas_inspector.create_end(context, result, address)
     }
 
@@ -97,7 +98,7 @@ impl<DB: Database> Inspector<DB> for CustomPrintTracer {
         &mut self,
         _context: &mut EvmContext<DB>,
         inputs: &mut CreateInputs,
-    ) -> Option<(InterpreterResult, Option<Address>)> {
+    ) -> Option<CreateOutcome> {
         println!(
             "CREATE CALL: caller:{:?}, scheme:{:?}, value:{:?}, init_code:{:?}, gas:{:?}",
             inputs.caller, inputs.scheme, inputs.value, inputs.init_code, inputs.gas_limit

crates/revm/src/inspector/eip3155.rs

@@ -5,6 +5,7 @@ use crate::{
     EvmContext, GetInspector, Inspector,
 };
 use core::ops::Range;
+use revm_interpreter::CreateOutcome;
 use serde_json::json;
 use std::io::Write;
 
@@ -118,7 +119,7 @@ impl<DB: Database> Inspector<DB> for TracerEip3155 {
         &mut self,
         _context: &mut EvmContext<DB>,
         _inputs: &mut CreateInputs,
-    ) -> Option<(InterpreterResult, Option<Address>)> {
+    ) -> Option<CreateOutcome> {
         None
     }
 
@@ -127,7 +128,7 @@ impl<DB: Database> Inspector<DB> for TracerEip3155 {
         context: &mut EvmContext<DB>,
         result: InterpreterResult,
         address: Option<Address>,
-    ) -> (InterpreterResult, Option<Address>) {
+    ) -> CreateOutcome {
         self.gas_inspector.create_end(context, result, address)
     }
 }

crates/revm/src/inspector/gas.rs

@@ -1,5 +1,7 @@
 //! GasIspector. Helper Inspector to calculate gas for others.
 
+use revm_interpreter::CreateOutcome;
+
 use crate::{
     interpreter::InterpreterResult,
     primitives::{db::Database, Address},
@@ -59,13 +61,15 @@ impl<DB: Database> Inspector<DB> for GasInspector {
         _context: &mut EvmContext<DB>,
         result: InterpreterResult,
         address: Option<Address>,
-    ) -> (InterpreterResult, Option<Address>) {
-        (result, address)
+    ) -> CreateOutcome {
+        CreateOutcome::new(result, address)
     }
 }
 
 #[cfg(test)]
 mod tests {
+    use revm_interpreter::CreateOutcome;
+
     use crate::{
         inspector::GetInspector,
         inspectors::GasInspector,
@@ -128,7 +132,7 @@ mod tests {
             &mut self,
             context: &mut EvmContext<DB>,
             call: &mut CreateInputs,
-        ) -> Option<(InterpreterResult, Option<Address>)> {
+        ) -> Option<CreateOutcome> {
             self.gas_inspector.create(context, call);
             None
         }
@@ -138,7 +142,7 @@ mod tests {
             context: &mut EvmContext<DB>,
             result: InterpreterResult,
             address: Option<Address>,
-        ) -> (InterpreterResult, Option<Address>) {
+        ) -> CreateOutcome {
             self.gas_inspector.create_end(context, result, address)
         }
     }