From a31c31c968fc5b0b3aa4e7db6e9f7b3a4c22f361 Mon Sep 17 00:00:00 2001 From: clabby Date: Tue, 5 Sep 2023 11:57:57 -0400 Subject: [PATCH] review: Address first few comments --- Cargo.lock | 1 - crates/primitives/src/env.rs | 13 +++++++++++++ crates/revm/Cargo.toml | 2 -- crates/revm/src/evm_impl.rs | 27 ++------------------------- crates/revm/src/optimism.rs | 27 ++++++++++++--------------- 5 files changed, 27 insertions(+), 43 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 1bdcb2f8a4..ae802d0394 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2215,7 +2215,6 @@ dependencies = [ "futures", "hex", "hex-literal", - "once_cell", "rayon", "revm-interpreter", "revm-precompile", diff --git a/crates/primitives/src/env.rs b/crates/primitives/src/env.rs index 37f12fbab4..e22e15549d 100644 --- a/crates/primitives/src/env.rs +++ b/crates/primitives/src/env.rs @@ -301,6 +301,19 @@ impl Env { /// Return initial spend gas (Gas needed to execute transaction). #[inline] pub fn validate_tx(&self) -> Result<(), InvalidTransaction> { + #[cfg(feature = "optimism")] + if self.cfg.optimism { + // Do not allow for a system transaction to be processed if Regolith is enabled. + if self.tx.is_system_transaction.unwrap_or(false) && SPEC::enabled(SpecId::REGOLITH) { + return Err(InvalidTransaction::DepositSystemTxPostRegolith); + } + + // Do not perform any extra validation for deposit transactions, they are pre-verified on L1. + if self.tx.source_hash.is_some() { + return Ok(()); + } + } + let gas_limit = self.tx.gas_limit; let effective_gas_price = self.effective_gas_price(); let is_create = self.tx.transact_to.is_create(); diff --git a/crates/revm/Cargo.toml b/crates/revm/Cargo.toml index b54598dd60..deb223ae3e 100644 --- a/crates/revm/Cargo.toml +++ b/crates/revm/Cargo.toml @@ -15,8 +15,6 @@ revm-interpreter = { path = "../interpreter", version = "1.1.2", default-feature #misc auto_impl = { version = "1.1", default-features = false } -once_cell = "1.18.0" -bytes = "1.4.0" # Optional serde = { version = "1.0", features = ["derive", "rc"], optional = true } diff --git a/crates/revm/src/evm_impl.rs b/crates/revm/src/evm_impl.rs index 45c1b05556..fba4a15b24 100644 --- a/crates/revm/src/evm_impl.rs +++ b/crates/revm/src/evm_impl.rs @@ -99,30 +99,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> Transact fn preverify_transaction(&mut self) -> Result<(), EVMError> { let env = self.env(); - #[cfg(feature = "optimism")] - let is_deposit = env.cfg.optimism && env.tx.source_hash.is_some(); - env.validate_block_env::()?; - - // If the transaction is a deposit transaction on Optimism, there are no fee fields to check, - // no nonce to check, and no need to check if EOA (L1 already verified it for us) - // Gas is free, but no refunds! - #[cfg(feature = "optimism")] - if env.cfg.optimism { - if !is_deposit { - env.validate_tx::()?; - } - - // Do not allow for a system transaction to be processed if Regolith is enabled. - if is_deposit - && env.tx.is_system_transaction.unwrap_or(false) - && GSPEC::enabled(SpecId::REGOLITH) - { - return Err(InvalidTransaction::DepositSystemTxPostRegolith.into()); - } - } - - #[cfg(not(feature = "optimism"))] env.validate_tx::()?; let tx_caller = env.tx.caller; @@ -474,7 +451,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, let Ok((l1_fee_vault_account, _)) = self .data .journaled_state - .load_account(*optimism::L1_FEE_RECIPIENT, self.data.db) + .load_account(optimism::L1_FEE_RECIPIENT.into(), self.data.db) else { panic!("[OPTIMISM] Failed to load L1 Fee Vault account"); }; @@ -486,7 +463,7 @@ impl<'a, GSPEC: Spec, DB: Database, const INSPECT: bool> EVMImpl<'a, GSPEC, DB, let Ok((base_fee_vault_account, _)) = self .data .journaled_state - .load_account(*optimism::BASE_FEE_RECIPIENT, self.data.db) + .load_account(optimism::BASE_FEE_RECIPIENT.into(), self.data.db) else { panic!("[OPTIMISM] Failed to load Base Fee Vault account"); }; diff --git a/crates/revm/src/optimism.rs b/crates/revm/src/optimism.rs index d60da6553d..df3348a432 100644 --- a/crates/revm/src/optimism.rs +++ b/crates/revm/src/optimism.rs @@ -1,28 +1,25 @@ //! Optimism-specific constants, types, and helpers. -use bytes::Bytes; use core::ops::Mul; -use once_cell::sync::Lazy; -use revm_interpreter::primitives::{db::Database, hex_literal::hex, Address, Spec, SpecId, U256}; +use revm_interpreter::primitives::{ + db::Database, hex_literal::hex, Bytes, Spec, SpecId, B160, U256, +}; const ZERO_BYTE_COST: u64 = 4; const NON_ZERO_BYTE_COST: u64 = 16; -static L1_BASE_FEE_SLOT: Lazy = Lazy::new(|| U256::from(1)); -static L1_OVERHEAD_SLOT: Lazy = Lazy::new(|| U256::from(5)); -static L1_SCALAR_SLOT: Lazy = Lazy::new(|| U256::from(6)); +const L1_BASE_FEE_SLOT: U256 = U256::from_limbs([1u64, 0, 0, 0]); +const L1_OVERHEAD_SLOT: U256 = U256::from_limbs([5u64, 0, 0, 0]); +const L1_SCALAR_SLOT: U256 = U256::from_limbs([6u64, 0, 0, 0]); /// The address of L1 fee recipient. -pub static L1_FEE_RECIPIENT: Lazy
= - Lazy::new(|| Address::from_slice(&hex!("420000000000000000000000000000000000001A"))); +pub const L1_FEE_RECIPIENT: B160 = B160(hex!("420000000000000000000000000000000000001A")); /// The address of the base fee recipient. -pub static BASE_FEE_RECIPIENT: Lazy
= - Lazy::new(|| Address::from_slice(&hex!("4200000000000000000000000000000000000019"))); +pub const BASE_FEE_RECIPIENT: B160 = B160(hex!("4200000000000000000000000000000000000019")); /// The address of the L1Block contract. -pub static L1_BLOCK_CONTRACT: Lazy
= - Lazy::new(|| Address::from_slice(&hex!("4200000000000000000000000000000000000015"))); +pub const L1_BLOCK_CONTRACT: B160 = B160(hex!("4200000000000000000000000000000000000015")); /// L1 block info /// @@ -48,9 +45,9 @@ pub struct L1BlockInfo { impl L1BlockInfo { /// Fetches the L1 block info from the `L1Block` contract in the database. pub fn try_fetch(db: &mut DB) -> Result { - let l1_base_fee = db.storage(*L1_BLOCK_CONTRACT, *L1_BASE_FEE_SLOT)?; - let l1_fee_overhead = db.storage(*L1_BLOCK_CONTRACT, *L1_OVERHEAD_SLOT)?; - let l1_fee_scalar = db.storage(*L1_BLOCK_CONTRACT, *L1_SCALAR_SLOT)?; + let l1_base_fee = db.storage(L1_BLOCK_CONTRACT.into(), L1_BASE_FEE_SLOT)?; + let l1_fee_overhead = db.storage(L1_BLOCK_CONTRACT.into(), L1_OVERHEAD_SLOT)?; + let l1_fee_scalar = db.storage(L1_BLOCK_CONTRACT.into(), L1_SCALAR_SLOT)?; Ok(L1BlockInfo { l1_base_fee,