Follow Osaka validation rules and simplify the EVM executor

binary_port/src/error_code.rs

@@ -397,6 +397,12 @@ pub enum ErrorCode {
     /// EVM transaction nonce is further ahead of the account nonce than the chainspec allows.
     #[error("the EVM transaction nonce is too far ahead of the account nonce")]
     InvalidTransactionEvmNonceTooFarAhead = 126,
+    /// EVM transaction gas price is below the configured base fee.
+    #[error("the EVM transaction gas price is below the base fee")]
+    InvalidTransactionEvmGasPriceBelowBaseFee = 127,
+    /// EVM transaction does not include a chain ID.
+    #[error("the EVM transaction is missing a chain ID")]
+    InvalidTransactionEvmMissingChainId = 128,
 }
 
 impl TryFrom<u16> for ErrorCode {
@@ -430,6 +436,12 @@ impl From<InvalidTransaction> for ErrorCode {
             InvalidTransaction::Evm(EvmTransactionError::NonceTooFarAhead { .. }) => {
                 ErrorCode::InvalidTransactionEvmNonceTooFarAhead
             }
+            InvalidTransaction::Evm(EvmTransactionError::GasPriceBelowBaseFee { .. }) => {
+                ErrorCode::InvalidTransactionEvmGasPriceBelowBaseFee
+            }
+            InvalidTransaction::Evm(EvmTransactionError::MissingChainId) => {
+                ErrorCode::InvalidTransactionEvmMissingChainId
+            }
             InvalidTransaction::Evm(EvmTransactionError::PositiveEffectivePriorityFeePerGas {
                 ..
             }) => ErrorCode::InvalidTransactionEvmPositiveEffectivePriorityFeePerGas,
@@ -637,6 +649,34 @@ mod tests {
     };
     use strum::IntoEnumIterator;
 
+    #[test]
+    fn evm_missing_chain_id_has_specific_error_code() {
+        assert_eq!(
+            ErrorCode::from(InvalidTransaction::Evm(EvmTransactionError::MissingChainId)),
+            ErrorCode::InvalidTransactionEvmMissingChainId
+        );
+        assert_eq!(
+            ErrorCode::try_from(128u16).unwrap(),
+            ErrorCode::InvalidTransactionEvmMissingChainId
+        );
+    }
+
+    #[test]
+    fn evm_gas_price_below_base_fee_has_specific_error_code() {
+        let error = InvalidTransaction::Evm(EvmTransactionError::GasPriceBelowBaseFee {
+            gas_price: 100,
+            base_fee: 5_000,
+        });
+        assert_eq!(
+            ErrorCode::from(error),
+            ErrorCode::InvalidTransactionEvmGasPriceBelowBaseFee
+        );
+        assert_eq!(
+            ErrorCode::try_from(127u16).unwrap(),
+            ErrorCode::InvalidTransactionEvmGasPriceBelowBaseFee
+        );
+    }
+
     #[test]
     fn osaka_transaction_cap_error_code_is_appended() {
         let error =

executor/evm/src/error.rs

@@ -17,17 +17,6 @@ pub enum Error {
     /// EVM wei-to-mote conversion ratio is invalid.
     #[error("EVM wei_per_mote must be greater than zero")]
     InvalidWeiPerMote,
-    /// Signed EVM transaction does not include an EIP-155 replay-protection chain id.
-    #[error("EVM transaction is missing replay-protection chain id")]
-    MissingChainId,
-    /// Transaction chain id does not match the executor configuration.
-    #[error("EVM transaction chain id {actual} does not match configured chain id {expected}")]
-    ChainIdMismatch {
-        /// Chain id configured in the chainspec.
-        expected: u64,
-        /// Chain id recovered from the signed transaction.
-        actual: u64,
-    },
     /// Failed to read from the Casper tracking copy as a revm database.
     #[error(transparent)]
     Database(#[from] DbError),

executor/evm/src/executor.rs

@@ -7,18 +7,19 @@ use casper_storage::{
     global_state::{error::Error as GlobalStateError, state::StateReader},
     TrackingCopy,
 };
-use casper_types::{EvmConfig, EvmSpec, EvmTransaction, EvmTransactionError, Key, StoredValue};
+use casper_types::{EvmConfig, EvmSpec, EvmTransactionError, Key, StoredValue};
 use revm::{
-    context::CfgEnv,
+    context::{CfgEnv, Evm},
     context_interface::result::{EVMError, InvalidTransaction as RevmInvalidTransaction},
-    handler::{Handler, MainnetHandler},
-    primitives::{hardfork::SpecId, Bytes},
-    Context, ExecuteEvm, MainBuilder, MainContext, SystemCallEvm,
+    handler::{instructions::EthInstructions, EthFrame, Handler, MainnetContext, MainnetHandler},
+    interpreter::interpreter::EthInterpreter,
+    primitives::hardfork::SpecId,
+    Context, ExecuteEvm, MainBuilder, MainContext,
 };
 
 use crate::{
-    db::CasperDb, precompiles::CasperEvmPrecompiles, state, tx, BlockContext, DbError, Error,
-    ExecuteKind, ExecuteRequest, ExecutionOutcome, Result, SystemCallRequest,
+    db::CasperDb, precompiles::CasperEvmPrecompiles, state, tx, CallValidation, DbError, Error,
+    ExecuteKind, ExecuteRequest, ExecutionOutcome, Result,
 };
 
 /// Executes EVM transactions and calls against a Casper tracking copy.
@@ -31,12 +32,13 @@ pub struct EvmExecutor {
     config: EvmConfig,
 }
 
-#[derive(Clone, Copy, Debug, Eq, PartialEq)]
-enum EvmExecutionMode {
-    Checked,
-    UncheckedSimulation,
-    SystemCall,
-}
+type CasperEvm<CTX> = Evm<
+    CTX,
+    (),
+    EthInstructions<EthInterpreter, CTX>,
+    CasperEvmPrecompiles,
+    EthFrame<EthInterpreter>,
+>;
 
 impl EvmExecutor {
     /// Creates a new executor from chainspec EVM configuration.
@@ -49,47 +51,17 @@ impl EvmExecutor {
         &self.config
     }
 
-    /// Validates an EVM transaction without executing it or applying state changes.
+    /// Validates an EVM transaction or call without executing it or applying state changes.
     pub fn validate_transaction<R, S>(
         &self,
         data_access_layer: &DataAccessLayer<S>,
         tracking_copy: &mut TrackingCopy<R>,
-        block_context: BlockContext,
-        transaction: &EvmTransaction,
+        request: ExecuteRequest,
     ) -> Result<()>
     where
         R: StateReader<Key, StoredValue, Error = GlobalStateError>,
     {
-        if !self.config.enabled {
-            return Err(Error::Disabled);
-        }
-        if self.config.wei_per_mote == 0 {
-            return Err(Error::InvalidWeiPerMote);
-        }
-
-        let Some(actual) = transaction.chain_id() else {
-            return Err(Error::MissingChainId);
-        };
-        if actual != self.config.chain_id {
-            return Err(Error::ChainIdMismatch {
-                expected: self.config.chain_id,
-                actual,
-            });
-        }
-
-        let kind = ExecuteKind::Transaction(Box::new(transaction.clone()));
-        let tx_env = tx::build_tx_env(&self.config, &kind)?;
-        let block = block_context.to_revm_block(&self.config)?;
-        let db = CasperDb::new(data_access_layer, tracking_copy, self.config.wei_per_mote);
-        let mut evm = Context::mainnet()
-            .with_db(db)
-            .with_block(block)
-            .with_tx(tx_env)
-            .modify_cfg_chained(|cfg| {
-                configure_evm_cfg(cfg, &self.config, EvmExecutionMode::Checked);
-            })
-            .build_mainnet()
-            .with_precompiles(CasperEvmPrecompiles::new(spec_id(self.config.spec)));
+        let mut evm = self.prepare_evm(data_access_layer, tracking_copy, &request)?;
 
         // revm's validate() includes the caller state phase. Running that phase
         // again would bump a call transaction's nonce twice in this journal.
@@ -120,47 +92,9 @@ impl EvmExecutor {
     where
         R: StateReader<Key, StoredValue, Error = GlobalStateError>,
     {
-        if !self.config.enabled {
-            return Err(Error::Disabled);
-        }
-        if self.config.wei_per_mote == 0 {
-            return Err(Error::InvalidWeiPerMote);
-        }
-
-        if let ExecuteKind::Transaction(transaction) = &request.kind {
-            let Some(actual) = transaction.chain_id() else {
-                return Err(Error::MissingChainId);
-            };
-            if actual != self.config.chain_id {
-                return Err(Error::ChainIdMismatch {
-                    expected: self.config.chain_id,
-                    actual,
-                });
-            }
-        }
-
-        let tx_env = tx::build_tx_env(&self.config, &request.kind)?;
-        let block = request.block.to_revm_block(&self.config)?;
-        let execution_mode = match &request.kind {
-            ExecuteKind::Transaction(_) => EvmExecutionMode::Checked,
-            ExecuteKind::Call(call) if call.validation.is_unchecked_simulation() => {
-                EvmExecutionMode::UncheckedSimulation
-            }
-            ExecuteKind::Call(_) => EvmExecutionMode::Checked,
-        };
-
         let result_and_state = {
-            let db = CasperDb::new(data_access_layer, tracking_copy, self.config.wei_per_mote);
-            let mut evm = Context::mainnet()
-                .with_db(db)
-                .with_block(block)
-                .modify_cfg_chained(|cfg| {
-                    configure_evm_cfg(cfg, &self.config, execution_mode);
-                })
-                .build_mainnet()
-                .with_precompiles(CasperEvmPrecompiles::new(spec_id(self.config.spec)));
-
-            evm.transact(tx_env).map_err(map_revm_error)?
+            let mut evm = self.prepare_evm(data_access_layer, tracking_copy, &request)?;
+            evm.replay().map_err(map_revm_error)?
         };
 
         let balance_losses = state::apply(
@@ -175,13 +109,12 @@ impl EvmExecutor {
         ))
     }
 
-    /// Executes a system call against the supplied tracking copy.
-    pub fn execute_system_call<R, S>(
+    fn prepare_evm<'a, R, S>(
         &self,
-        data_access_layer: &DataAccessLayer<S>,
-        tracking_copy: &mut TrackingCopy<R>,
-        request: SystemCallRequest,
-    ) -> Result<ExecutionOutcome>
+        data_access_layer: &'a DataAccessLayer<S>,
+        tracking_copy: &'a mut TrackingCopy<R>,
+        request: &ExecuteRequest,
+    ) -> Result<CasperEvm<MainnetContext<CasperDb<'a, R, S>>>>
     where
         R: StateReader<Key, StoredValue, Error = GlobalStateError>,
     {
@@ -192,51 +125,35 @@ impl EvmExecutor {
             return Err(Error::InvalidWeiPerMote);
         }
 
+        let tx_env = tx::build_tx_env(&self.config, &request.kind)?;
         let block = request.block.to_revm_block(&self.config)?;
-        let result_and_state = {
-            let db = CasperDb::new(data_access_layer, tracking_copy, self.config.wei_per_mote);
-            let mut evm = Context::mainnet()
-                .with_db(db)
-                .with_block(block)
-                .modify_cfg_chained(|cfg| {
-                    configure_evm_cfg(cfg, &self.config, EvmExecutionMode::SystemCall);
-                })
-                .build_mainnet()
-                .with_precompiles(CasperEvmPrecompiles::new(spec_id(self.config.spec)));
-
-            evm.system_call(
-                tx::to_revm_address(request.target),
-                Bytes::from(request.input),
-            )
-            .map_err(map_revm_error)?
+        let validation = match &request.kind {
+            ExecuteKind::Transaction(_) => CallValidation::Checked,
+            ExecuteKind::Call(call) => call.validation,
         };
-
-        let balance_losses = state::apply(
-            tracking_copy,
-            result_and_state.state,
-            self.config.wei_per_mote,
-        )?;
-        Ok(ExecutionOutcome::from_revm_result(
-            &result_and_state.result,
-            balance_losses,
-            self.config.wei_per_mote,
-        ))
+        let db = CasperDb::new(data_access_layer, tracking_copy, self.config.wei_per_mote);
+        Ok(Context::mainnet()
+            .with_db(db)
+            .with_block(block)
+            .with_tx(tx_env)
+            .modify_cfg_chained(|cfg| {
+                configure_evm_cfg(cfg, &self.config, validation);
+            })
+            .build_mainnet()
+            .with_precompiles(CasperEvmPrecompiles::new(spec_id(self.config.spec))))
     }
 }
 
-fn configure_evm_cfg(cfg: &mut CfgEnv, config: &EvmConfig, execution_mode: EvmExecutionMode) {
-    let skip_validation = matches!(
-        execution_mode,
-        EvmExecutionMode::UncheckedSimulation | EvmExecutionMode::SystemCall
-    );
+fn configure_evm_cfg(cfg: &mut CfgEnv, config: &EvmConfig, validation: CallValidation) {
+    let skip_validation = validation.is_unchecked_simulation();
 
     cfg.spec = spec_id(config.spec);
     cfg.chain_id = config.chain_id;
-    cfg.tx_chain_id_check = matches!(execution_mode, EvmExecutionMode::Checked);
+    cfg.tx_chain_id_check = !skip_validation;
     // Read-only simulations may consume the block budget, following eth_call.
     // All checked execution retains revm's Osaka transaction cap.
     cfg.tx_gas_limit_cap = skip_validation.then_some(u64::MAX);
-    cfg.disable_block_gas_limit = matches!(execution_mode, EvmExecutionMode::SystemCall);
+    cfg.disable_block_gas_limit = false;
     cfg.disable_base_fee = skip_validation;
     cfg.disable_balance_check = skip_validation;
     cfg.disable_nonce_check = skip_validation;

executor/evm/src/lib.rs

@@ -16,9 +16,7 @@ mod tx;
 pub use error::{DbError, Error, Result};
 pub use executor::EvmExecutor;
 pub use outcome::{ExecutionOutcome, ExecutionStatus};
-pub use request::{
-    BlockContext, CallRequest, CallValidation, ExecuteKind, ExecuteRequest, SystemCallRequest,
-};
+pub use request::{BlockContext, CallRequest, CallValidation, ExecuteKind, ExecuteRequest};
 
 use casper_types::evm;
 

executor/evm/src/request.rs

@@ -4,7 +4,7 @@ use casper_types::{evm, EvmConfig, EvmTransaction, U256};
 
 use crate::{tx, Error};
 
-/// Request passed to [`crate::EvmExecutor::execute`].
+/// Request passed to [`crate::EvmExecutor::validate_transaction`] or [`crate::EvmExecutor::execute`].
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct ExecuteRequest {
     /// Block context available to EVM opcodes and validation.
@@ -13,17 +13,6 @@ pub struct ExecuteRequest {
     pub kind: ExecuteKind,
 }
 
-/// Request passed to [`crate::EvmExecutor::execute_system_call`].
-#[derive(Clone, Debug, Eq, PartialEq)]
-pub struct SystemCallRequest {
-    /// Block context available to EVM opcodes.
-    pub block: BlockContext,
-    /// System contract address.
-    pub target: evm::Address,
-    /// System call input bytes.
-    pub input: Vec<u8>,
-}
-
 /// EVM work item to execute.
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub enum ExecuteKind {
@@ -35,8 +24,8 @@ pub enum ExecuteKind {
 
 /// Unsigned EVM call request.
 ///
-/// Calls are useful for views, simulations, tests, and controlled system
-/// execution. They still write effects into the supplied tracking copy, so
+/// Calls are useful for views, simulations, and tests.
+/// They still write effects into the supplied tracking copy, so
 /// callers should pass a fork when they want to discard the result.
 #[derive(Clone, Debug, Eq, PartialEq)]
 pub struct CallRequest {

executor/evm/src/tx.rs

@@ -22,7 +22,7 @@ pub(crate) fn build_tx_env(config: &EvmConfig, kind: &ExecuteKind) -> Result<TxE
                 .value(to_revm_u256(transaction.value()))
                 .data(Bytes::from(transaction.input().to_vec()))
                 .nonce(transaction.nonce())
-                .chain_id(transaction.chain_id().or(Some(config.chain_id)))
+                .chain_id(transaction.chain_id())
                 .access_list(to_revm_access_list(transaction.access_list()));
 
             builder = match transaction.kind() {