feat: use impl instead of dyn in GetInspector (bluealloy#1157)

* feat: use `impl` instead of `dyn` in `GetInspector`

* chore: clippy

bins/revme/src/cmd/statetest/runner.rs

@@ -115,7 +115,7 @@ fn check_evm_execution<EXT>(
     evm: &Evm<'_, EXT, &mut State<EmptyDB>>,
     print_json_outcome: bool,
 ) -> Result<(), TestError> {
-    let logs_root = log_rlp_hash(&exec_result.as_ref().map(|r| r.logs()).unwrap_or_default());
+    let logs_root = log_rlp_hash(exec_result.as_ref().map(|r| r.logs()).unwrap_or_default());
     let state_root = state_merkle_trie_root(evm.context.evm.db.cache.trie_account());
 
     let print_json_output = |error: Option<String>| {

crates/interpreter/src/instructions/i256.rs

@@ -136,10 +136,8 @@ mod tests {
         let one = U256::from(1);
         let one_hundred = U256::from(100);
         let fifty = U256::from(50);
-        let _fifty_sign = Sign::Plus;
         let two = U256::from(2);
         let neg_one_hundred = U256::from(100);
-        let _neg_one_hundred_sign = Sign::Minus;
         let minus_one = U256::from(1);
         let max_value = U256::from(2).pow(U256::from(255)) - U256::from(1);
         let neg_max_value = U256::from(2).pow(U256::from(255)) - U256::from(1);

crates/interpreter/src/interpreter.rs

@@ -5,7 +5,7 @@ mod stack;
 
 pub use analysis::BytecodeLocked;
 pub use contract::Contract;
-pub use shared_memory::{next_multiple_of_32, SharedMemory};
+pub use shared_memory::{next_multiple_of_32, SharedMemory, EMPTY_SHARED_MEMORY};
 pub use stack::{Stack, STACK_LIMIT};
 
 use crate::{
@@ -17,8 +17,6 @@ use revm_primitives::U256;
 use std::borrow::ToOwned;
 use std::boxed::Box;
 
-pub use self::shared_memory::EMPTY_SHARED_MEMORY;
-
 #[derive(Debug)]
 pub struct Interpreter {
     /// Contract information and invoking data

crates/primitives/src/result.rs

@@ -52,14 +52,6 @@ impl ExecutionResult {
         matches!(self, Self::Halt { .. })
     }
 
-    /// Return logs, if execution is not successful, function will return empty vec.
-    pub fn logs(&self) -> Vec<Log> {
-        match self {
-            Self::Success { logs, .. } => logs.clone(),
-            _ => Vec::new(),
-        }
-    }
-
     /// Returns the output data of the execution.
     ///
     /// Returns `None` if the execution was halted.
@@ -82,20 +74,29 @@ impl ExecutionResult {
         }
     }
 
-    /// Consumes the type and returns logs, if execution is not successful, function will return empty vec.
+    /// Returns the logs if execution is successful, or an empty list otherwise.
+    pub fn logs(&self) -> &[Log] {
+        match self {
+            Self::Success { logs, .. } => logs,
+            _ => &[],
+        }
+    }
+
+    /// Consumes `self` and returns the logs if execution is successful, or an empty list otherwise.
     pub fn into_logs(self) -> Vec<Log> {
         match self {
             Self::Success { logs, .. } => logs,
             _ => Vec::new(),
         }
     }
 
+    /// Returns the gas used.
     pub fn gas_used(&self) -> u64 {
-        let (Self::Success { gas_used, .. }
-        | Self::Revert { gas_used, .. }
-        | Self::Halt { gas_used, .. }) = self;
-
-        *gas_used
+        match *self {
+            Self::Success { gas_used, .. }
+            | Self::Revert { gas_used, .. }
+            | Self::Halt { gas_used, .. } => gas_used,
+        }
     }
 }
 
@@ -123,6 +124,14 @@ impl Output {
             Output::Create(data, _) => data,
         }
     }
+
+    /// Returns the created address, if any.
+    pub fn address(&self) -> Option<&Address> {
+        match self {
+            Output::Call(_) => None,
+            Output::Create(_, address) => address.as_ref(),
+        }
+    }
 }
 
 /// Main EVM error.

crates/revm/src/builder.rs

@@ -497,10 +497,9 @@ mod test {
             .build();
 
         let Context {
-            external,
-            evm: EvmContext { db, .. },
+            external: _,
+            evm: EvmContext { db: _, .. },
         } = evm.into_context();
-        let _ = (external, db);
     }
 
     #[test]

crates/revm/src/db/in_memory_db.rs

@@ -428,7 +428,9 @@ mod tests {
 
         let (key, value) = (U256::from(123), U256::from(456));
         let mut new_state = CacheDB::new(init_state);
-        let _ = new_state.insert_account_storage(account, key, value);
+        new_state
+            .insert_account_storage(account, key, value)
+            .unwrap();
 
         assert_eq!(new_state.basic(account).unwrap().unwrap().nonce, nonce);
         assert_eq!(new_state.storage(account, key), Ok(value));
@@ -449,10 +451,14 @@ mod tests {
 
         let (key0, value0) = (U256::from(123), U256::from(456));
         let (key1, value1) = (U256::from(789), U256::from(999));
-        let _ = init_state.insert_account_storage(account, key0, value0);
+        init_state
+            .insert_account_storage(account, key0, value0)
+            .unwrap();
 
         let mut new_state = CacheDB::new(init_state);
-        let _ = new_state.replace_account_storage(account, [(key1, value1)].into());
+        new_state
+            .replace_account_storage(account, [(key1, value1)].into())
+            .unwrap();
 
         assert_eq!(new_state.basic(account).unwrap().unwrap().nonce, nonce);
         assert_eq!(new_state.storage(account, key0), Ok(U256::ZERO));

crates/revm/src/frame.rs

@@ -110,10 +110,9 @@ impl FrameResult {
 
 /// Contains either a frame or a result.
 pub enum FrameOrResult {
-    /// Boxed call frame,
+    /// Boxed call or create frame.
     Frame(Frame),
-    /// Boxed create frame
-    /// Interpreter result
+    /// Call or create result.
     Result(FrameResult),
 }
 
@@ -187,6 +186,16 @@ impl Frame {
             Self::Create(create_frame) => &mut create_frame.frame_data,
         }
     }
+
+    /// Returns a reference to the interpreter.
+    pub fn interpreter(&self) -> &Interpreter {
+        &self.frame_data().interpreter
+    }
+
+    /// Returns a mutable reference to the interpreter.
+    pub fn interpreter_mut(&mut self) -> &mut Interpreter {
+        &mut self.frame_data_mut().interpreter
+    }
 }
 
 impl FrameOrResult {

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

@@ -37,14 +37,15 @@ pub fn frame_return_with_refund_flag<SPEC: Spec>(
         }
         _ => {}
     }
+
     // Calculate gas refund for transaction.
     // If config is set to disable gas refund, it will return 0.
     // If spec is set to london, it will decrease the maximum refund amount to 5th part of
     // gas spend. (Before london it was 2th part of gas spend)
     if refund_enabled {
         // EIP-3529: Reduction in refunds
-        gas.set_final_refund::<SPEC>()
-    };
+        gas.set_final_refund::<SPEC>();
+    }
 }
 
 /// Handle output of the transaction

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

@@ -1,7 +1,7 @@
 use crate::{
     interpreter::{Gas, SuccessOrHalt},
     primitives::{
-        db::Database, EVMError, ExecutionResult, Output, ResultAndState, Spec, SpecId::LONDON, U256,
+        db::Database, EVMError, ExecutionResult, ResultAndState, Spec, SpecId::LONDON, U256,
     },
     Context, FrameResult,
 };
@@ -94,10 +94,7 @@ pub fn output<EXT, DB: Database>(
         },
         SuccessOrHalt::Revert => ExecutionResult::Revert {
             gas_used: final_gas_used,
-            output: match output {
-                Output::Call(return_value) => return_value,
-                Output::Create(return_value, _) => return_value,
-            },
+            output: output.into_data(),
         },
         SuccessOrHalt::Halt(reason) => ExecutionResult::Halt {
             reason,

crates/revm/src/inspector/handler_register.rs

@@ -11,11 +11,13 @@ use std::{boxed::Box, rc::Rc, sync::Arc, vec::Vec};
 
 /// Provides access to an `Inspector` instance.
 pub trait GetInspector<DB: Database> {
-    fn get_inspector(&mut self) -> &mut dyn Inspector<DB>;
+    /// Returns the associated `Inspector`.
+    fn get_inspector(&mut self) -> &mut impl Inspector<DB>;
 }
 
 impl<DB: Database, INSP: Inspector<DB>> GetInspector<DB> for INSP {
-    fn get_inspector(&mut self) -> &mut dyn Inspector<DB> {
+    #[inline(always)]
+    fn get_inspector(&mut self) -> &mut impl Inspector<DB> {
         self
     }
 }
@@ -37,8 +39,7 @@ pub fn inspector_handle_register<'a, DB: Database, EXT: GetInspector<DB>>(
 ) {
     // Every instruction inside flat table that is going to be wrapped by inspector calls.
     let table = handler
-        .instruction_table
-        .take()
+        .take_instruction_table()
         .expect("Handler must have instruction table");
     let mut table = match table {
         InstructionTables::Plain(table) => table
@@ -123,7 +124,7 @@ pub fn inspector_handle_register<'a, DB: Database, EXT: GetInspector<DB>>(
     }
 
     // cast vector to array.
-    handler.instruction_table = Some(InstructionTables::Boxed(
+    handler.set_instruction_table(InstructionTables::Boxed(
         table.try_into().unwrap_or_else(|_| unreachable!()),
     ));
 
@@ -146,10 +147,10 @@ pub fn inspector_handle_register<'a, DB: Database, EXT: GetInspector<DB>>(
             create_input_stack_inner.borrow_mut().push(inputs.clone());
 
             let mut frame_or_result = old_handle(ctx, inputs);
-
-            let inspector = ctx.external.get_inspector();
             if let Ok(FrameOrResult::Frame(frame)) = &mut frame_or_result {
-                inspector.initialize_interp(&mut frame.frame_data_mut().interpreter, &mut ctx.evm)
+                ctx.external
+                    .get_inspector()
+                    .initialize_interp(frame.interpreter_mut(), &mut ctx.evm)
             }
             frame_or_result
         },
@@ -160,20 +161,18 @@ pub fn inspector_handle_register<'a, DB: Database, EXT: GetInspector<DB>>(
     let old_handle = handler.execution.call.clone();
     handler.execution.call = Arc::new(
         move |ctx, mut inputs| -> Result<FrameOrResult, EVMError<DB::Error>> {
-            let inspector = ctx.external.get_inspector();
-            let _mems = inputs.return_memory_offset.clone();
-            // call inspector callto change input or return outcome.
-            if let Some(outcome) = inspector.call(&mut ctx.evm, &mut inputs) {
-                call_input_stack_inner.borrow_mut().push(inputs.clone());
+            // Call inspector to change input or return outcome.
+            let outcome = ctx.external.get_inspector().call(&mut ctx.evm, &mut inputs);
+            call_input_stack_inner.borrow_mut().push(inputs.clone());
+            if let Some(outcome) = outcome {
                 return Ok(FrameOrResult::Result(FrameResult::Call(outcome)));
             }
-            call_input_stack_inner.borrow_mut().push(inputs.clone());
 
             let mut frame_or_result = old_handle(ctx, inputs);
-
-            let inspector = ctx.external.get_inspector();
             if let Ok(FrameOrResult::Frame(frame)) = &mut frame_or_result {
-                inspector.initialize_interp(&mut frame.frame_data_mut().interpreter, &mut ctx.evm)
+                ctx.external
+                    .get_inspector()
+                    .initialize_interp(frame.interpreter_mut(), &mut ctx.evm)
             }
             frame_or_result
         },
@@ -184,19 +183,23 @@ pub fn inspector_handle_register<'a, DB: Database, EXT: GetInspector<DB>>(
     let old_handle = handler.execution.insert_call_outcome.clone();
     handler.execution.insert_call_outcome =
         Arc::new(move |ctx, frame, shared_memory, mut outcome| {
-            let inspector = ctx.external.get_inspector();
             let call_inputs = call_input_stack_inner.borrow_mut().pop().unwrap();
-            outcome = inspector.call_end(&mut ctx.evm, &call_inputs, outcome);
+            outcome = ctx
+                .external
+                .get_inspector()
+                .call_end(&mut ctx.evm, &call_inputs, outcome);
             old_handle(ctx, frame, shared_memory, outcome)
         });
 
     // create outcome
     let create_input_stack_inner = create_input_stack.clone();
     let old_handle = handler.execution.insert_create_outcome.clone();
     handler.execution.insert_create_outcome = Arc::new(move |ctx, frame, mut outcome| {
-        let inspector = ctx.external.get_inspector();
         let create_inputs = create_input_stack_inner.borrow_mut().pop().unwrap();
-        outcome = inspector.create_end(&mut ctx.evm, &create_inputs, outcome);
+        outcome = ctx
+            .external
+            .get_inspector()
+            .create_end(&mut ctx.evm, &create_inputs, outcome);
         old_handle(ctx, frame, outcome)
     });
 
@@ -214,7 +217,6 @@ pub fn inspector_handle_register<'a, DB: Database, EXT: GetInspector<DB>>(
                 *outcome = inspector.create_end(&mut ctx.evm, &create_inputs, outcome.clone());
             }
         }
-        //inspector.last_frame_return(ctx, frame_result);
         old_handle(ctx, frame_result)
     });
 }