From a4fa635d7e9c7a62890cc4dd4ce47fd13d7ef247 Mon Sep 17 00:00:00 2001 From: rakita Date: Thu, 28 Nov 2024 14:02:27 +0100 Subject: [PATCH] cleanup --- Cargo.lock | 1 - crates/context/Cargo.toml | 1 - crates/handler/Cargo.toml | 1 - crates/handler/interface/Cargo.toml | 30 ++----------------- crates/handler/interface/src/execution.rs | 3 +- crates/handler/interface/src/util.rs | 4 +++ crates/handler/src/execution.rs | 7 ++--- crates/handler/src/frame.rs | 8 ++--- crates/handler/src/pre_execution.rs | 2 +- crates/handler/src/validation.rs | 17 ++++------- crates/inspector/src/eip3155.rs | 2 +- crates/inspector/src/gas.rs | 6 ++++ crates/inspector/src/inspector.rs | 22 +++++++++----- .../interpreter/src/instructions/contract.rs | 4 +-- .../interpreter/src/instructions/control.rs | 2 +- crates/interpreter/src/interpreter.rs | 2 +- .../src/interpreter/ext_bytecode.rs | 9 ++---- crates/interpreter/src/interpreter/stack.rs | 16 ++++++++-- crates/interpreter/src/interpreter_wiring.rs | 9 ++++++ crates/interpreter/src/table.rs | 6 ++-- crates/revm/Cargo.toml | 3 +- crates/revm/src/evm.rs | 11 +------ crates/revm/src/exec.rs | 10 +++++++ crates/revm/src/lib.rs | 2 ++ crates/specification/src/hardfork.rs | 6 ++-- 25 files changed, 92 insertions(+), 92 deletions(-) create mode 100644 crates/revm/src/exec.rs diff --git a/Cargo.lock b/Cargo.lock index d1a0a6653d..187d4f71b5 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2952,7 +2952,6 @@ dependencies = [ "revm-database", "revm-interpreter", "revm-primitives", - "serde", ] [[package]] diff --git a/crates/context/Cargo.toml b/crates/context/Cargo.toml index fcd3ecdf8f..1fc05fe7cb 100644 --- a/crates/context/Cargo.toml +++ b/crates/context/Cargo.toml @@ -30,7 +30,6 @@ database-interface.workspace = true state.workspace = true specification.workspace = true bytecode.workspace = true -database = { workspace = true, optional = true } # misc derive-where.workspace = true diff --git a/crates/handler/Cargo.toml b/crates/handler/Cargo.toml index 53cf7d605b..3037d07d0c 100644 --- a/crates/handler/Cargo.toml +++ b/crates/handler/Cargo.toml @@ -30,7 +30,6 @@ primitives.workspace = true state.workspace = true specification.workspace = true bytecode.workspace = true -database = { workspace = true, optional = true } handler-interface.workspace = true # Optional diff --git a/crates/handler/interface/Cargo.toml b/crates/handler/interface/Cargo.toml index 6b0e05fafc..402098f165 100644 --- a/crates/handler/interface/Cargo.toml +++ b/crates/handler/interface/Cargo.toml @@ -26,36 +26,10 @@ all = "warn" primitives.workspace = true interpreter.workspace = true - -# Optional -serde = { version = "1.0", default-features = false, features = [ - "derive", - "rc", -], optional = true } - [dev-dependencies] database.workspace = true [features] default = ["std"] -std = ["serde?/std"] -serde = ["dep:serde", "primitives/serde"] -serde-json = ["serde"] - -# Enable additional features for development -# They add functions to Cfg trait. -dev = [ - "memory_limit", - "optional_balance_check", - "optional_block_gas_limit", - "optional_eip3607", - "optional_gas_refund", - "optional_no_base_fee", - -] -memory_limit = [] -optional_balance_check = [] -optional_block_gas_limit = [] -optional_eip3607 = [] -optional_gas_refund = [] -optional_no_base_fee = [] +std = [] +serde = ["std", "primitives/serde", "interpreter/serde"] diff --git a/crates/handler/interface/src/execution.rs b/crates/handler/interface/src/execution.rs index de3690d97b..437f12d408 100644 --- a/crates/handler/interface/src/execution.rs +++ b/crates/handler/interface/src/execution.rs @@ -1,3 +1,4 @@ +use crate::util::FrameOrFrameResult; pub use crate::{Frame, FrameOrResultGen}; pub use std::{vec, vec::Vec}; @@ -12,7 +13,7 @@ pub trait ExecutionHandler { &mut self, context: &mut Self::Context, gas_limit: u64, - ) -> Result::FrameResult>, Self::Error>; + ) -> Result, Self::Error>; /// Execute create. fn last_frame_result( diff --git a/crates/handler/interface/src/util.rs b/crates/handler/interface/src/util.rs index dda445b36b..560d8341bc 100644 --- a/crates/handler/interface/src/util.rs +++ b/crates/handler/interface/src/util.rs @@ -1,3 +1,5 @@ +use crate::Frame; + pub enum FrameOrResultGen { Frame(Frame), Result(Result), @@ -18,3 +20,5 @@ impl FrameOrResultGen { } } } + +pub type FrameOrFrameResult = FrameOrResultGen::FrameResult>; diff --git a/crates/handler/src/execution.rs b/crates/handler/src/execution.rs index 60d733606e..7132a9b1a2 100644 --- a/crates/handler/src/execution.rs +++ b/crates/handler/src/execution.rs @@ -4,7 +4,7 @@ use context_interface::{ result::InvalidTransaction, BlockGetter, Cfg, CfgGetter, ErrorGetter, JournalStateGetter, JournalStateGetterDBError, Transaction, TransactionGetter, }; -use handler_interface::{ExecutionHandler, Frame as FrameTrait, FrameOrResultGen}; +use handler_interface::{util::FrameOrFrameResult, ExecutionHandler, Frame as FrameTrait}; use interpreter::{ interpreter::{EthInstructionProvider, EthInterpreter}, CallInputs, CallScheme, CallValue, CreateInputs, CreateScheme, EOFCreateInputs, EOFCreateKind, @@ -49,8 +49,7 @@ where &mut self, context: &mut Self::Context, gas_limit: u64, - ) -> Result::FrameResult>, Self::Error> - { + ) -> Result, Self::Error> { // Make new frame action. let spec = context.cfg().spec().into(); let tx = context.tx(); @@ -113,7 +112,7 @@ where gas.record_refund(refunded); } - Ok(frame_result.into()) + Ok(frame_result) } } diff --git a/crates/handler/src/frame.rs b/crates/handler/src/frame.rs index 468c8038a1..4a4de401c6 100644 --- a/crates/handler/src/frame.rs +++ b/crates/handler/src/frame.rs @@ -651,12 +651,10 @@ where return_revert!() => U256::from(1), _ => U256::from(2), } + } else if ins_result.is_ok() { + U256::from(1) } else { - if ins_result.is_ok() { - U256::from(1) - } else { - U256::ZERO - } + U256::ZERO } }; // Safe to push without stack limit check diff --git a/crates/handler/src/pre_execution.rs b/crates/handler/src/pre_execution.rs index 2a5eaaa4d6..0fef8169b0 100644 --- a/crates/handler/src/pre_execution.rs +++ b/crates/handler/src/pre_execution.rs @@ -166,7 +166,7 @@ pub fn apply_eip7702_auth_list< }; // 2. Verify the chain id is either 0 or the chain's current ID. - if !(authorization.chain_id == 0) && authorization.chain_id != chain_id { + if authorization.chain_id != 0 && authorization.chain_id != chain_id { continue; } diff --git a/crates/handler/src/validation.rs b/crates/handler/src/validation.rs index 7cc193e5f3..3e5af7a089 100644 --- a/crates/handler/src/validation.rs +++ b/crates/handler/src/validation.rs @@ -66,8 +66,7 @@ where fn validate_initial_tx_gas(&self, ctx: &Self::Context) -> Result { let spec = ctx.cfg().spec().into(); - validate_initial_tx_gas::<&Self::Context, InvalidTransaction>(&ctx, spec) - .map_err(Into::into) + validate_initial_tx_gas::<&Self::Context, InvalidTransaction>(ctx, spec).map_err(Into::into) } } @@ -327,16 +326,12 @@ where // Check if account has enough balance for `gas_limit * max_fee`` and value transfer. // Transfer will be done inside `*_inner` functions. - if balance_check > account.info.balance { - if !ctx.cfg().is_balance_check_disabled() { - return Err(InvalidTransaction::LackOfFundForMaxFee { - fee: Box::new(balance_check), - balance: Box::new(account.info.balance), - } - .into()); + if balance_check > account.info.balance && !ctx.cfg().is_balance_check_disabled() { + return Err(InvalidTransaction::LackOfFundForMaxFee { + fee: Box::new(balance_check), + balance: Box::new(account.info.balance), } - // TODO wiring Add transaction cost to balance to ensure execution doesn't fail. - // else { account.info.balance = account.info.balance.saturating_add(balance_check)}; + .into()); } Ok(()) diff --git a/crates/inspector/src/eip3155.rs b/crates/inspector/src/eip3155.rs index a38a781aba..c1ea9b9bc0 100644 --- a/crates/inspector/src/eip3155.rs +++ b/crates/inspector/src/eip3155.rs @@ -197,7 +197,7 @@ pub trait CloneStack { impl CloneStack for Stack { fn clone_from(&self) -> Vec { - self.data().iter().map(|b| b.clone()).collect() + self.data().to_vec() } } diff --git a/crates/inspector/src/gas.rs b/crates/inspector/src/gas.rs index 637f6119c0..706d07914b 100644 --- a/crates/inspector/src/gas.rs +++ b/crates/inspector/src/gas.rs @@ -15,6 +15,12 @@ pub struct GasInspector { _phantom: core::marker::PhantomData<(CTX, INTR)>, } +impl Default for GasInspector { + fn default() -> Self { + Self::new() + } +} + impl GasInspector { pub fn gas_remaining(&self) -> u64 { self.gas_remaining diff --git a/crates/inspector/src/inspector.rs b/crates/inspector/src/inspector.rs index f03bb609f4..6f6caf2026 100644 --- a/crates/inspector/src/inspector.rs +++ b/crates/inspector/src/inspector.rs @@ -199,6 +199,12 @@ pub struct StepPrintInspector { _phantom: core::marker::PhantomData, } +impl Default for StepPrintInspector { + fn default() -> Self { + Self::new() + } +} + impl StepPrintInspector { pub fn new() -> Self { Self { @@ -557,9 +563,7 @@ where } fn from_base(instruction: Instruction) -> Self { - Self { - instruction: instruction, - } + Self { instruction } } } @@ -615,14 +619,18 @@ where unsafe { MaybeUninit::uninit().assume_init() }; for (i, element) in table.iter_mut().enumerate() { - let foo = InspectorInstruction { + let function = InspectorInstruction { instruction: main_table[i], }; - *element = MaybeUninit::new(foo); + *element = MaybeUninit::new(function); } - let mut table = - unsafe { core::mem::transmute::<_, [InspectorInstruction; 256]>(table) }; + let mut table = unsafe { + core::mem::transmute::< + [MaybeUninit>; 256], + [InspectorInstruction; 256], + >(table) + }; // inspector log wrapper diff --git a/crates/interpreter/src/instructions/contract.rs b/crates/interpreter/src/instructions/contract.rs index b0c8a7bccc..8d4a87017e 100644 --- a/crates/interpreter/src/instructions/contract.rs +++ b/crates/interpreter/src/instructions/contract.rs @@ -145,7 +145,7 @@ pub fn return_contract( let output: Bytes = output.into(); let result = InstructionResult::ReturnContract; - let gas = interpreter.control.gas().clone(); + let gas = *interpreter.control.gas(); interpreter.control.set_next_action( crate::InterpreterAction::Return { result: InterpreterResult { @@ -398,7 +398,7 @@ pub fn create( let scheme = if IS_CREATE2 { popn!([salt], interpreter); // SAFETY: len is reasonable in size as gas for it is already deducted. - gas_or_fail!(interpreter, gas::create2_cost(len.try_into().unwrap())); + gas_or_fail!(interpreter, gas::create2_cost(len)); CreateScheme::Create2 { salt } } else { gas!(interpreter, gas::CREATE); diff --git a/crates/interpreter/src/instructions/control.rs b/crates/interpreter/src/instructions/control.rs index a901dbf185..4cbb331908 100644 --- a/crates/interpreter/src/instructions/control.rs +++ b/crates/interpreter/src/instructions/control.rs @@ -211,7 +211,7 @@ fn return_inner( output = interpreter.memory.slice_len(offset, len).to_vec().into() } - let gas = interpreter.control.gas().clone(); + let gas = *interpreter.control.gas(); interpreter.control.set_next_action( InterpreterAction::Return { result: InterpreterResult { diff --git a/crates/interpreter/src/interpreter.rs b/crates/interpreter/src/interpreter.rs index b7112f4123..14f25d9842 100644 --- a/crates/interpreter/src/interpreter.rs +++ b/crates/interpreter/src/interpreter.rs @@ -197,7 +197,7 @@ impl Interpreter { result: self.control.instruction_result(), // return empty bytecode output: Bytes::new(), - gas: self.control.gas().clone(), + gas: *self.control.gas(), }, } } diff --git a/crates/interpreter/src/interpreter/ext_bytecode.rs b/crates/interpreter/src/interpreter/ext_bytecode.rs index b465500389..fc3ef0b9d7 100644 --- a/crates/interpreter/src/interpreter/ext_bytecode.rs +++ b/crates/interpreter/src/interpreter/ext_bytecode.rs @@ -101,15 +101,13 @@ impl EofCodeInfo for ExtBytecode { fn code_section_info(&self, idx: usize) -> Option<&TypesSection> { self.base .eof() - .map(|eof| eof.body.types_section.get(idx)) - .flatten() + .and_then(|eof| eof.body.types_section.get(idx)) } fn code_section_pc(&self, idx: usize) -> Option { self.base .eof() - .map(|eof| eof.body.eof_code_section_start(idx)) - .flatten() + .and_then(|eof| eof.body.eof_code_section_start(idx)) } } @@ -131,8 +129,7 @@ impl EofContainer for ExtBytecode { fn eof_container(&self, index: usize) -> Option<&Bytes> { self.base .eof() - .map(|eof| eof.body.container_section.get(index)) - .flatten() + .and_then(|eof| eof.body.container_section.get(index)) } } diff --git a/crates/interpreter/src/interpreter/stack.rs b/crates/interpreter/src/interpreter/stack.rs index 4958796bcc..9ae250aeaa 100644 --- a/crates/interpreter/src/interpreter/stack.rs +++ b/crates/interpreter/src/interpreter/stack.rs @@ -154,6 +154,11 @@ impl Stack { self.data.get_unchecked_mut(len - 1) } + /// Pops `N` values from the stack. + /// + /// # Safety + /// + /// The caller is responsible for checking the length of the stack. #[inline] #[cfg_attr(debug_assertions, track_caller)] pub unsafe fn popn(&mut self) -> [U256; N] { @@ -167,6 +172,11 @@ impl Stack { result } + /// Pops `N` values from the stack and returns the top of the stack. + /// + /// # Safety + /// + /// The caller is responsible for checking the length of the stack. #[inline] #[cfg_attr(debug_assertions, track_caller)] pub unsafe fn popn_top(&mut self) -> ([U256; POPN], &mut U256) { @@ -439,7 +449,7 @@ mod tests { // Test cloning a partially filled stack let mut partial_stack = Stack::new(); for i in 0..10 { - assert_eq!(partial_stack.push(U256::from(i)), true); + assert!(partial_stack.push(U256::from(i))); } let mut cloned_partial = partial_stack.clone(); assert_eq!(partial_stack, cloned_partial); @@ -447,7 +457,7 @@ mod tests { assert_eq!(cloned_partial.data().capacity(), STACK_LIMIT); // Test that modifying the clone doesn't affect the original - assert_eq!(cloned_partial.push(U256::from(100)), true); + assert!(cloned_partial.push(U256::from(100))); assert_ne!(partial_stack, cloned_partial); assert_eq!(partial_stack.len(), 10); assert_eq!(cloned_partial.len(), 11); @@ -455,7 +465,7 @@ mod tests { // Test cloning a full stack let mut full_stack = Stack::new(); for i in 0..STACK_LIMIT { - assert_eq!(full_stack.push(U256::from(i)), true); + assert!(full_stack.push(U256::from(i))); } let mut cloned_full = full_stack.clone(); assert_eq!(full_stack, cloned_full); diff --git a/crates/interpreter/src/interpreter_wiring.rs b/crates/interpreter/src/interpreter_wiring.rs index b0958cceec..b327914176 100644 --- a/crates/interpreter/src/interpreter_wiring.rs +++ b/crates/interpreter/src/interpreter_wiring.rs @@ -83,6 +83,10 @@ pub trait EofContainer { pub trait SubRoutineStack { fn len(&self) -> usize; + fn is_empty(&self) -> bool { + self.len() == 0 + } + fn routine_idx(&self) -> usize; /// Sets new code section without touching subroutine stack. @@ -102,6 +106,11 @@ pub trait StackTrait { /// Returns stack length. fn len(&self) -> usize; + /// Returns `true` if stack is empty. + fn is_empty(&self) -> bool { + self.len() == 0 + } + /// Pushes values to the stack /// Return `true` if push was successful, `false` if stack overflow. /// diff --git a/crates/interpreter/src/table.rs b/crates/interpreter/src/table.rs index 418d57abf2..43a5d5fbc8 100644 --- a/crates/interpreter/src/table.rs +++ b/crates/interpreter/src/table.rs @@ -36,8 +36,8 @@ pub enum InstructionTables< H: ?Sized, CI: CustomInstruction, > { - Plain(InstructionTable), - Custom(CustomInstructionTable), + Plain(Box>), + Custom(Box>), } impl InstructionTables @@ -83,7 +83,7 @@ where let Self::Plain(table) = self else { unreachable!() }; - *self = Self::Custom(make_custom_instruction_table(table, f)); + *self = Self::Custom(Box::new(make_custom_instruction_table(table, f))); let Self::Custom(boxed) = self else { unreachable!() }; diff --git a/crates/revm/Cargo.toml b/crates/revm/Cargo.toml index cb24abba02..9098136484 100644 --- a/crates/revm/Cargo.toml +++ b/crates/revm/Cargo.toml @@ -30,7 +30,6 @@ database-interface.workspace = true state.workspace = true specification.workspace = true bytecode.workspace = true -database = { workspace = true, optional = true } context.workspace = true context-interface.workspace = true handler.workspace = true @@ -73,7 +72,7 @@ arbitrary = ["primitives/arbitrary"] asm-keccak = ["primitives/asm-keccak"] portable = ["precompile/portable"] -test-utils = ["database"] +test-utils = [] dev = [ "memory_limit", diff --git a/crates/revm/src/evm.rs b/crates/revm/src/evm.rs index 8177525921..823d97dac2 100644 --- a/crates/revm/src/evm.rs +++ b/crates/revm/src/evm.rs @@ -2,7 +2,7 @@ use context::{block::BlockEnv, tx::TxEnv, CfgEnv, Context}; use context_interface::{ journaled_state::JournaledState, result::{EVMError, HaltReason, InvalidHeader, InvalidTransaction, ResultAndState}, - Block, BlockGetter, CfgGetter, DatabaseGetter, ErrorGetter, JournalStateGetter, + BlockGetter, CfgGetter, DatabaseGetter, ErrorGetter, JournalStateGetter, JournalStateGetterDBError, Transaction, TransactionGetter, }; use database_interface::Database; @@ -33,15 +33,6 @@ pub type EthContext = Context; /// Mainnet EVM type. pub type MainEvm = Evm, EthContext>; -pub trait EvmRunner { - type TX: Transaction; - type BLOCK: Block; - - fn set_block(&mut self, block: Self::BLOCK); - - fn transact(&mut self, tx: &Self::TX); -} - impl Evm> where diff --git a/crates/revm/src/exec.rs b/crates/revm/src/exec.rs new file mode 100644 index 0000000000..535a90f974 --- /dev/null +++ b/crates/revm/src/exec.rs @@ -0,0 +1,10 @@ +use context_interface::{Block, Transaction}; + +pub trait EvmExec { + type TX: Transaction; + type BLOCK: Block; + + fn set_block(&mut self, block: Self::BLOCK); + + fn transact(&mut self, tx: Self::TX); +} diff --git a/crates/revm/src/lib.rs b/crates/revm/src/lib.rs index 91ac08a8ab..cf9be904e4 100644 --- a/crates/revm/src/lib.rs +++ b/crates/revm/src/lib.rs @@ -21,6 +21,7 @@ pub use state; // Modules. mod evm; +pub mod exec; // Export items. @@ -28,3 +29,4 @@ pub use context::journaled_state::{JournalEntry, JournaledState}; pub use context::Context; pub use database_interface::{Database, DatabaseCommit, DatabaseRef}; pub use evm::{Error, EthContext, Evm, MainEvm}; +pub use exec::EvmExec; diff --git a/crates/specification/src/hardfork.rs b/crates/specification/src/hardfork.rs index 4d15cd585e..a8c27bbf1f 100644 --- a/crates/specification/src/hardfork.rs +++ b/crates/specification/src/hardfork.rs @@ -136,9 +136,9 @@ impl From for &'static str { } } -impl ToString for SpecId { - fn to_string(&self) -> String { - <&'static str>::from(*self).into() +impl core::fmt::Display for SpecId { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + write!(f, "{}", <&'static str>::from(*self)) } }