From 4c60857f9b648acd6112bb223e9fd4ad8c223862 Mon Sep 17 00:00:00 2001 From: rakita Date: Tue, 9 Apr 2024 02:18:14 +0200 Subject: [PATCH] chore: move some files. cleanup comments --- bins/revm-test/Cargo.toml | 2 +- bins/revme/src/cmd/statetest/runner.rs | 4 +- crates/interpreter/src/function_stack.rs | 3 + crates/interpreter/src/gas/calc.rs | 14 +++- crates/interpreter/src/inner_models.rs | 52 -------------- .../interpreter/src/instructions/contract.rs | 8 ++- .../interpreter/src/interpreter/analysis.rs | 19 ++---- crates/interpreter/src/interpreter/stack.rs | 2 +- crates/interpreter/src/interpreter_action.rs | 67 +++++++++++++++++++ .../{ => interpreter_action}/call_inputs.rs | 0 .../{ => interpreter_action}/call_outcome.rs | 0 .../{ => interpreter_action}/create_inputs.rs | 0 .../create_outcome.rs | 0 .../eof_create_inputs.rs | 0 .../eof_create_outcome.rs | 7 +- crates/interpreter/src/lib.rs | 20 ++---- crates/primitives/src/bytecode.rs | 2 +- crates/primitives/src/env.rs | 38 +++++++---- crates/revm/src/handler/mainnet/validation.rs | 3 +- 19 files changed, 135 insertions(+), 106 deletions(-) delete mode 100644 crates/interpreter/src/inner_models.rs create mode 100644 crates/interpreter/src/interpreter_action.rs rename crates/interpreter/src/{ => interpreter_action}/call_inputs.rs (100%) rename crates/interpreter/src/{ => interpreter_action}/call_outcome.rs (100%) rename crates/interpreter/src/{ => interpreter_action}/create_inputs.rs (100%) rename crates/interpreter/src/{ => interpreter_action}/create_outcome.rs (100%) rename crates/interpreter/src/{ => interpreter_action}/eof_create_inputs.rs (100%) rename crates/interpreter/src/{ => interpreter_action}/eof_create_outcome.rs (90%) diff --git a/bins/revm-test/Cargo.toml b/bins/revm-test/Cargo.toml index 7605826a1d..f26c4e7882 100644 --- a/bins/revm-test/Cargo.toml +++ b/bins/revm-test/Cargo.toml @@ -7,7 +7,7 @@ edition = "2021" [dependencies] bytes = "1.6" hex = "0.4" -revm = { path = "../../crates/revm", version = "8.0.0",default-features=false} +revm = { path = "../../crates/revm", version = "8.0.0", default-features=false } microbench = "0.5" alloy-sol-macro = "0.7.0" alloy-sol-types = "0.7.0" diff --git a/bins/revme/src/cmd/statetest/runner.rs b/bins/revme/src/cmd/statetest/runner.rs index 1bfc2c175b..f2a58b6115 100644 --- a/bins/revme/src/cmd/statetest/runner.rs +++ b/bins/revme/src/cmd/statetest/runner.rs @@ -67,9 +67,7 @@ pub fn find_all_json_tests(path: &Path) -> Vec { fn skip_test(path: &Path) -> bool { let path_str = path.to_str().expect("Path is not valid UTF-8"); let name = path.file_name().unwrap().to_str().unwrap(); - if path_str.contains("stRandom") { - return true; - } + matches!( name, // funky test with `bigint 0x00` value in json :) not possible to happen on mainnet and require diff --git a/crates/interpreter/src/function_stack.rs b/crates/interpreter/src/function_stack.rs index cf2690c913..ef786cd662 100644 --- a/crates/interpreter/src/function_stack.rs +++ b/crates/interpreter/src/function_stack.rs @@ -10,11 +10,13 @@ pub struct FunctionReturnFrame { } impl FunctionReturnFrame { + /// Return new function frame. pub fn new(idx: usize, pc: usize) -> Self { Self { idx, pc } } } +/// Function Stack #[derive(Debug, Default)] pub struct FunctionStack { pub return_stack: Vec, @@ -22,6 +24,7 @@ pub struct FunctionStack { } impl FunctionStack { + /// Returns new function stack. pub fn new() -> Self { Self { return_stack: Vec::new(), diff --git a/crates/interpreter/src/gas/calc.rs b/crates/interpreter/src/gas/calc.rs index 214f794b00..18d0fd70bf 100644 --- a/crates/interpreter/src/gas/calc.rs +++ b/crates/interpreter/src/gas/calc.rs @@ -1,3 +1,5 @@ +use revm_primitives::Bytes; + use super::constants::*; use crate::{ primitives::{ @@ -356,10 +358,18 @@ pub fn validate_initial_tx_gas( input: &[u8], is_create: bool, access_list: &[(Address, Vec)], + initcodes: &[Bytes], ) -> u64 { let mut initial_gas = 0; - let zero_data_len = input.iter().filter(|v| **v == 0).count() as u64; - let non_zero_data_len = input.len() as u64 - zero_data_len; + let mut zero_data_len = input.iter().filter(|v| **v == 0).count() as u64; + let mut non_zero_data_len = input.len() as u64 - zero_data_len; + + // Enabling of initcode is checked in `validate_env` handler. + for initcode in initcodes { + let zeros = initcode.iter().filter(|v| **v == 0).count() as u64; + zero_data_len += zeros; + non_zero_data_len += initcode.len() as u64 - zeros; + } // initdate stipend initial_gas += zero_data_len * TRANSACTION_ZERO_DATA; diff --git a/crates/interpreter/src/inner_models.rs b/crates/interpreter/src/inner_models.rs deleted file mode 100644 index 7bec10611b..0000000000 --- a/crates/interpreter/src/inner_models.rs +++ /dev/null @@ -1,52 +0,0 @@ -pub use crate::primitives::CreateScheme; -use crate::primitives::{Address, Bytes, TransactTo, TxEnv, U256}; -use core::ops::Range; -use std::boxed::Box; - -/// Inputs for a create call. -#[derive(Clone, Debug, PartialEq, Eq, Hash)] -#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] -pub struct CreateInputs { - /// Caller address of the EVM. - pub caller: Address, - /// The create scheme. - pub scheme: CreateScheme, - /// The value to transfer. - pub value: U256, - /// The init code of the contract. - pub init_code: Bytes, - /// The gas limit of the call. - pub gas_limit: u64, -} - -impl CreateInputs { - /// Creates new create inputs. - pub fn new(tx_env: &TxEnv, gas_limit: u64) -> Option { - let TransactTo::Create(scheme) = tx_env.transact_to else { - return None; - }; - - Some(CreateInputs { - caller: tx_env.caller, - scheme, - value: tx_env.value, - init_code: tx_env.data.clone(), - gas_limit, - }) - } - - /// Returns boxed create inputs. - pub fn new_boxed(tx_env: &TxEnv, gas_limit: u64) -> Option> { - 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 { - CreateScheme::Create => self.caller.create(nonce), - CreateScheme::Create2 { salt } => self - .caller - .create2_from_code(salt.to_be_bytes(), &self.init_code), - } - } -} diff --git a/crates/interpreter/src/instructions/contract.rs b/crates/interpreter/src/instructions/contract.rs index b390416086..b684f7b34a 100644 --- a/crates/interpreter/src/instructions/contract.rs +++ b/crates/interpreter/src/instructions/contract.rs @@ -110,7 +110,13 @@ pub fn txcreate(interpreter: &mut Interpreter, host: &mut H) { }; // fetch initcode, if not found push ZERO. - let Some(initcode) = host.env().tx.eof_initcodes.get(&tx_initcode_hash).cloned() else { + let Some(initcode) = host + .env() + .tx + .eof_initcodes_hashed + .get(&tx_initcode_hash) + .cloned() + else { push!(interpreter, U256::ZERO); return; }; diff --git a/crates/interpreter/src/interpreter/analysis.rs b/crates/interpreter/src/interpreter/analysis.rs index 176db35496..bc2d96d2f5 100644 --- a/crates/interpreter/src/interpreter/analysis.rs +++ b/crates/interpreter/src/interpreter/analysis.rs @@ -73,13 +73,8 @@ pub fn validate_raw_eof(bytecode: Bytes) -> Result { } /// Validate Eof structures. -/// -/// do perf test on: -/// max eof containers -/// max depth of containers. -/// bytecode iteration. pub fn validate_eof(eof: &Eof) -> Result<(), EofError> { - // clone is cheat as it is Bytes and a header. + // clone is cheap as it is Bytes and a header. let mut queue = vec![eof.clone()]; while let Some(eof) = queue.pop() { @@ -316,8 +311,6 @@ pub fn validate_eof_code( let this_instruction = &mut jumps[i]; - //println!("next b: {next_biggest}, next s: {next_smallest} terminal: {is_after_termination} opcode: {}",opcode.name); - // update biggest/smallest values for next instruction only if it is not after termination. if !is_after_termination { this_instruction.smallest = core::cmp::min(this_instruction.smallest, next_smallest); @@ -497,10 +490,9 @@ pub fn validate_eof_code( return Err(EofValidationError::StackUnderflow); } - //println!("stack_io_diff: {}", stack_io_diff); next_smallest = this_instruction.smallest + stack_io_diff; next_biggest = this_instruction.biggest + stack_io_diff; - //println!("absolute_jumpdest: {absolute_jumpdest:?}"); + // check if jumpdest are correct and mark forward jumps. for absolute_jump in absolute_jumpdest { if absolute_jump < 0 { @@ -524,7 +516,6 @@ pub fn validate_eof_code( target_jump.is_jumpdest = true; if absolute_jump <= i { - //println!("JUMP: {i} -> {target_jump:?}"); // backward jumps should have same smallest and biggest stack items. if target_jump.biggest != next_biggest { // wrong jumpdest. @@ -572,7 +563,7 @@ mod test { #[test] fn test1() { - //result:Result { result: false, exception: Some("EOF_ConflictingStackHeight") } + // result:Result { result: false, exception: Some("EOF_ConflictingStackHeight") } let err = validate_raw_eof(hex!("ef0001010004020001000704000000008000016000e200fffc00").into()); assert!(err.is_err(), "{err:#?}"); @@ -580,7 +571,7 @@ mod test { #[test] fn test2() { - //result:Result { result: false, exception: Some("EOF_InvalidNumberOfOutputs") } + // result:Result { result: false, exception: Some("EOF_InvalidNumberOfOutputs") } let err = validate_raw_eof(hex!("ef000101000c02000300040004000204000000008000020002000100010001e30001005fe500025fe4").into()); assert!(err.is_ok(), "{err:#?}"); @@ -588,7 +579,7 @@ mod test { #[test] fn test3() { - //result:Result { result: false, exception: Some("EOF_InvalidNumberOfOutputs") } + // result:Result { result: false, exception: Some("EOF_InvalidNumberOfOutputs") } let err = validate_raw_eof(hex!("ef000101000c02000300040008000304000000008000020002000503010003e30001005f5f5f5f5fe500025050e4").into()); assert_eq!( diff --git a/crates/interpreter/src/interpreter/stack.rs b/crates/interpreter/src/interpreter/stack.rs index ec0ca19d8d..44dbb48a64 100644 --- a/crates/interpreter/src/interpreter/stack.rs +++ b/crates/interpreter/src/interpreter/stack.rs @@ -171,7 +171,7 @@ impl Stack { (pop1, pop2, pop3, pop4) } - /// Pops 4 values from the stack. + /// Pops 5 values from the stack. /// /// # Safety /// diff --git a/crates/interpreter/src/interpreter_action.rs b/crates/interpreter/src/interpreter_action.rs new file mode 100644 index 0000000000..a6a3aa216b --- /dev/null +++ b/crates/interpreter/src/interpreter_action.rs @@ -0,0 +1,67 @@ +mod call_inputs; +mod call_outcome; +mod create_inputs; +mod create_outcome; +mod eof_create_inputs; +mod eof_create_outcome; + +pub use call_inputs::{CallInputs, CallScheme, TransferValue}; +pub use call_outcome::CallOutcome; +pub use create_inputs::{CreateInputs, CreateScheme}; +pub use create_outcome::CreateOutcome; +pub use eof_create_inputs::EOFCreateInput; +pub use eof_create_outcome::EOFCreateOutcome; + +use crate::InterpreterResult; +use std::boxed::Box; + +#[derive(Clone, Debug, Default, PartialEq, Eq)] +pub enum InterpreterAction { + /// CALL, CALLCODE, DELEGATECALL, STATICCALL + /// or EOF EXT instuction called. + Call { inputs: Box }, + /// CREATE or CREATE2 instruction called. + Create { inputs: Box }, + /// EOF CREATE instruction called. + EOFCreate { inputs: Box }, + /// 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 { + match self { + InterpreterAction::Return { result } => Some(result), + _ => None, + } + } +} diff --git a/crates/interpreter/src/call_inputs.rs b/crates/interpreter/src/interpreter_action/call_inputs.rs similarity index 100% rename from crates/interpreter/src/call_inputs.rs rename to crates/interpreter/src/interpreter_action/call_inputs.rs diff --git a/crates/interpreter/src/call_outcome.rs b/crates/interpreter/src/interpreter_action/call_outcome.rs similarity index 100% rename from crates/interpreter/src/call_outcome.rs rename to crates/interpreter/src/interpreter_action/call_outcome.rs diff --git a/crates/interpreter/src/create_inputs.rs b/crates/interpreter/src/interpreter_action/create_inputs.rs similarity index 100% rename from crates/interpreter/src/create_inputs.rs rename to crates/interpreter/src/interpreter_action/create_inputs.rs diff --git a/crates/interpreter/src/create_outcome.rs b/crates/interpreter/src/interpreter_action/create_outcome.rs similarity index 100% rename from crates/interpreter/src/create_outcome.rs rename to crates/interpreter/src/interpreter_action/create_outcome.rs diff --git a/crates/interpreter/src/eof_create_inputs.rs b/crates/interpreter/src/interpreter_action/eof_create_inputs.rs similarity index 100% rename from crates/interpreter/src/eof_create_inputs.rs rename to crates/interpreter/src/interpreter_action/eof_create_inputs.rs diff --git a/crates/interpreter/src/eof_create_outcome.rs b/crates/interpreter/src/interpreter_action/eof_create_outcome.rs similarity index 90% rename from crates/interpreter/src/eof_create_outcome.rs rename to crates/interpreter/src/interpreter_action/eof_create_outcome.rs index 754a29e913..031e11803f 100644 --- a/crates/interpreter/src/eof_create_outcome.rs +++ b/crates/interpreter/src/interpreter_action/eof_create_outcome.rs @@ -18,16 +18,17 @@ pub struct EOFCreateOutcome { } impl EOFCreateOutcome { - /// Constructs a new `CreateOutcome`. + /// Constructs a new [`EOFCreateOutcome`]. /// /// # Arguments /// /// * `result` - An `InterpreterResult` representing the result of the interpreter operation. /// * `address` - An optional `Address` associated with the create operation. + /// * `return_memory_range` - The memory range that Revert bytes are going to be written. /// /// # Returns /// - /// A new `CreateOutcome` instance. + /// A new [`EOFCreateOutcome`] instance. pub fn new( result: InterpreterResult, address: Address, @@ -40,7 +41,7 @@ impl EOFCreateOutcome { } } - /// Retrieves a reference to the `InstructionResult` from the `InterpreterResult`. + /// 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 diff --git a/crates/interpreter/src/lib.rs b/crates/interpreter/src/lib.rs index e0dbb6f6e3..bf2a49f314 100644 --- a/crates/interpreter/src/lib.rs +++ b/crates/interpreter/src/lib.rs @@ -19,37 +19,29 @@ use serde_json as _; #[cfg(test)] use walkdir as _; -mod call_inputs; -mod call_outcome; -mod create_inputs; -mod create_outcome; -mod eof_create_inputs; -mod eof_create_outcome; mod function_stack; pub mod gas; mod host; mod instruction_result; pub mod instructions; pub mod interpreter; +pub mod interpreter_action; pub mod opcode; // Reexport primary types. -pub use call_inputs::{CallInputs, CallScheme, TransferValue}; -pub use call_outcome::CallOutcome; -pub use create_inputs::{CreateInputs, CreateScheme}; -pub use create_outcome::CreateOutcome; -pub use eof_create_inputs::EOFCreateInput; -pub use eof_create_outcome::EOFCreateOutcome; pub use function_stack::{FunctionReturnFrame, FunctionStack}; pub use gas::Gas; pub use host::{DummyHost, Host, LoadAccountResult, SStoreResult, SelfDestructResult}; pub use instruction_result::*; -pub use opcode::{Instruction, OpCode, OPCODE_INFO_JUMPTABLE}; - pub use interpreter::{ analysis, next_multiple_of_32, Contract, Interpreter, InterpreterAction, InterpreterResult, SharedMemory, Stack, EMPTY_SHARED_MEMORY, STACK_LIMIT, }; +pub use interpreter_action::{ + CallInputs, CallOutcome, CallScheme, CreateInputs, CreateOutcome, CreateScheme, EOFCreateInput, + EOFCreateOutcome, TransferValue, +}; +pub use opcode::{Instruction, OpCode, OPCODE_INFO_JUMPTABLE}; pub use primitives::{MAX_CODE_SIZE, MAX_INITCODE_SIZE}; #[doc(hidden)] diff --git a/crates/primitives/src/bytecode.rs b/crates/primitives/src/bytecode.rs index 64c9f986bf..538a57af44 100644 --- a/crates/primitives/src/bytecode.rs +++ b/crates/primitives/src/bytecode.rs @@ -74,7 +74,7 @@ impl Bytecode { /// /// # Safety /// - /// Bytecode need to end with STOP (0x00) opcode as checked bytecode assumes + /// Bytecode needs to end with STOP (0x00) opcode as checked bytecode assumes /// that it is safe to iterate over bytecode without checking lengths. pub unsafe fn new_analyzed( bytecode: Bytes, diff --git a/crates/primitives/src/env.rs b/crates/primitives/src/env.rs index f8ea6826e5..9a4470a912 100644 --- a/crates/primitives/src/env.rs +++ b/crates/primitives/src/env.rs @@ -199,22 +199,25 @@ impl Env { if matches!(self.tx.transact_to, TransactTo::Call(_)) { return Err(InvalidTransaction::EofCrateShouldHaveToAddress); } + } else { + // If initcode is set check its bounds. + if self.tx.eof_initcodes.len() > 256 { + return Err(InvalidTransaction::EofInitcodesNumberLimit); + } + if self + .tx + .eof_initcodes_hashed + .iter() + .any(|(_, i)| i.len() >= MAX_INITCODE_SIZE) + { + return Err(InvalidTransaction::EofInitcodesSizeLimit); + } } } else { + // Initcode set when not supported. if !self.tx.eof_initcodes.is_empty() { return Err(InvalidTransaction::EofInitcodesNotSupported); } - if self.tx.eof_initcodes.len() > 256 { - return Err(InvalidTransaction::EofInitcodesNumberLimit); - } - if self - .tx - .eof_initcodes - .iter() - .any(|(_, i)| i.len() >= MAX_INITCODE_SIZE) - { - return Err(InvalidTransaction::EofInitcodesSizeLimit); - } } Ok(()) @@ -577,10 +580,18 @@ pub struct TxEnv { /// Incorporated as part of the Prague upgrade via [EOF] /// /// [EOF]: https://eips.ethereum.org/EIPS/eip-4844 - pub eof_initcodes: HashMap, + pub eof_initcodes: Vec, + + /// Internal Temporary field that stores the hashes of the EOF initcodes. + /// + /// Those are always cleared after the transaction is executed. + /// And calculated/overwritten every time transaction starts. + /// They are calculated from the [`Self::eof_initcodes`] field. + pub eof_initcodes_hashed: HashMap, #[cfg_attr(feature = "serde", serde(flatten))] #[cfg(feature = "optimism")] + /// Optimism fields. pub optimism: OptimismFields, } @@ -622,7 +633,8 @@ impl Default for TxEnv { access_list: Vec::new(), blob_hashes: Vec::new(), max_fee_per_blob_gas: None, - eof_initcodes: HashMap::new(), + eof_initcodes: Vec::new(), + eof_initcodes_hashed: HashMap::new(), #[cfg(feature = "optimism")] optimism: OptimismFields::default(), } diff --git a/crates/revm/src/handler/mainnet/validation.rs b/crates/revm/src/handler/mainnet/validation.rs index 176e0e8282..22dfb5f4bc 100644 --- a/crates/revm/src/handler/mainnet/validation.rs +++ b/crates/revm/src/handler/mainnet/validation.rs @@ -42,9 +42,10 @@ pub fn validate_initial_tx_gas( let input = &env.tx.data; let is_create = env.tx.transact_to.is_create(); let access_list = &env.tx.access_list; + let initcodes = &env.tx.eof_initcodes; let initial_gas_spend = - gas::validate_initial_tx_gas(SPEC::SPEC_ID, input, is_create, access_list); + gas::validate_initial_tx_gas(SPEC::SPEC_ID, input, is_create, access_list, initcodes); // Additional check to see if limit is big enough to cover initial gas. if initial_gas_spend > env.tx.gas_limit {