diff --git a/binary_port/src/error_code.rs b/binary_port/src/error_code.rs index 4cf649b42a..6e4f1e8b19 100644 --- a/binary_port/src/error_code.rs +++ b/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 for ErrorCode { @@ -430,6 +436,12 @@ impl From 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 = diff --git a/executor/evm/src/error.rs b/executor/evm/src/error.rs index c9d49fe34e..40a12f994f 100644 --- a/executor/evm/src/error.rs +++ b/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), diff --git a/executor/evm/src/executor.rs b/executor/evm/src/executor.rs index 77c30615b5..2af0959dae 100644 --- a/executor/evm/src/executor.rs +++ b/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 = Evm< + CTX, + (), + EthInstructions, + CasperEvmPrecompiles, + EthFrame, +>; 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( &self, data_access_layer: &DataAccessLayer, tracking_copy: &mut TrackingCopy, - block_context: BlockContext, - transaction: &EvmTransaction, + request: ExecuteRequest, ) -> Result<()> where R: StateReader, { - 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, { - 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( + fn prepare_evm<'a, R, S>( &self, - data_access_layer: &DataAccessLayer, - tracking_copy: &mut TrackingCopy, - request: SystemCallRequest, - ) -> Result + data_access_layer: &'a DataAccessLayer, + tracking_copy: &'a mut TrackingCopy, + request: &ExecuteRequest, + ) -> Result>>> where R: StateReader, { @@ -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; diff --git a/executor/evm/src/lib.rs b/executor/evm/src/lib.rs index 43c3dee49b..da5e8e9282 100644 --- a/executor/evm/src/lib.rs +++ b/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; diff --git a/executor/evm/src/request.rs b/executor/evm/src/request.rs index 996fb0e3d5..cd3bb10147 100644 --- a/executor/evm/src/request.rs +++ b/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, -} - /// 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 { diff --git a/executor/evm/src/tx.rs b/executor/evm/src/tx.rs index 973322b100..4e9fc8d42b 100644 --- a/executor/evm/src/tx.rs +++ b/executor/evm/src/tx.rs @@ -22,7 +22,7 @@ pub(crate) fn build_tx_env(config: &EvmConfig, kind: &ExecuteKind) -> Result return Ok(None), Err(EvmExecutorError::Disabled) => CasperEvmTransactionError::Disabled, - Err(EvmExecutorError::MissingChainId) => CasperEvmTransactionError::MissingChainId, - Err(EvmExecutorError::ChainIdMismatch { expected, actual }) => { - CasperEvmTransactionError::ChainIdMismatch { expected, actual } - } Err(EvmExecutorError::InvalidTransaction(error)) => error, Err(EvmExecutorError::Transaction(error)) => CasperEvmTransactionError::Validation(error), Err(error) => { diff --git a/node/src/reactor/main_reactor/tests/fixture.rs b/node/src/reactor/main_reactor/tests/fixture.rs index dbebfc4fbb..0dc4a364ca 100644 --- a/node/src/reactor/main_reactor/tests/fixture.rs +++ b/node/src/reactor/main_reactor/tests/fixture.rs @@ -616,7 +616,7 @@ impl TestFixture { .await } - /// Runs the network until all nodes reach the given completed block height. + /// Runs the network until all nodes reach at least the given completed block height. /// /// Returns an error if the condition isn't met in time. pub(crate) async fn try_run_until_block_height( @@ -627,13 +627,13 @@ impl TestFixture { self.try_run_until( move |nodes: &Nodes| { nodes.values().all(|runner| { + // One node can pass the target before the others reach it. runner .main_reactor() .storage() .get_highest_complete_block() .expect("should not error reading db") - .map(|block| block.height()) - == Some(block_height) + .is_some_and(|block| block.height() >= block_height) }) }, within, @@ -641,7 +641,7 @@ impl TestFixture { .await } - /// Runs the network until all nodes reach the given completed block height. + /// Runs the network until all nodes reach at least the given completed block height. /// /// Panics if the condition isn't met in time. pub(crate) async fn run_until_block_height(&mut self, block_height: u64, within: Duration) { diff --git a/node/src/reactor/main_reactor/tests/network_general.rs b/node/src/reactor/main_reactor/tests/network_general.rs index b4473da8e4..d89ee96c14 100644 --- a/node/src/reactor/main_reactor/tests/network_general.rs +++ b/node/src/reactor/main_reactor/tests/network_general.rs @@ -51,6 +51,21 @@ async fn run_network() { fixture.run_until_consensus_in_era(ERA_TWO, ONE_MIN).await; } +#[tokio::test] +async fn block_height_wait_accepts_already_completed_blocks() { + let initial_stakes = InitialStakes::AllEqual { + count: 2, + stake: 100, + }; + let mut fixture = TestFixture::new(initial_stakes, None).await; + fixture.run_until_block_height(1, ONE_MIN).await; + + fixture + .try_run_until_block_height(0, Duration::from_millis(100)) + .await + .expect("nodes past block 0 have already completed it"); +} + #[tokio::test] async fn historical_sync_with_era_height_1() { let initial_stakes = InitialStakes::Random { count: 5 }; diff --git a/node/src/types/transaction/meta_transaction.rs b/node/src/types/transaction/meta_transaction.rs index 71431ce9cb..a313884b60 100644 --- a/node/src/types/transaction/meta_transaction.rs +++ b/node/src/types/transaction/meta_transaction.rs @@ -655,45 +655,47 @@ mod tests { } #[test] - fn evm_config_compliance_rejects_missing_chain_id() { + fn evm_config_compliance_accepts_unprotected_legacy_transaction() { let chainspec = chainspec(); let meta = evm_meta(&chainspec, legacy_transaction(None, BASE_FEE_WEI, 21_000)); - assert!(matches!( - meta.is_config_compliant(&chainspec, TimeDiff::from_seconds(0), Timestamp::zero()), - Err(InvalidTransaction::Evm(EvmTransactionError::MissingChainId)) - )); + meta.is_config_compliant(&chainspec, TimeDiff::from_seconds(0), Timestamp::zero()) + .expect("Osaka permits unprotected legacy transactions"); } #[test] fn evm_config_compliance_rejects_mismatched_chain_id() { let chainspec = chainspec(); - let meta = evm_meta( - &chainspec, - legacy_transaction(Some(CHAIN_ID + 1), BASE_FEE_WEI, 21_000), - ); - assert!(matches!( - meta.is_config_compliant(&chainspec, TimeDiff::from_seconds(0), Timestamp::zero()), - Err(InvalidTransaction::Evm(EvmTransactionError::ChainIdMismatch { - expected: CHAIN_ID, - actual - })) if actual == CHAIN_ID + 1 - )); + for gas_price in [BASE_FEE_WEI, BASE_FEE_WEI - 1] { + let meta = evm_meta( + &chainspec, + legacy_transaction(Some(CHAIN_ID + 1), gas_price, 21_000), + ); + assert!(matches!( + meta.is_config_compliant(&chainspec, TimeDiff::from_seconds(0), Timestamp::zero()), + Err(InvalidTransaction::Evm(EvmTransactionError::ChainIdMismatch { + expected: CHAIN_ID, + actual + })) if actual == CHAIN_ID + 1 + )); + } } #[test] fn evm_config_compliance_rejects_gas_price_below_base_fee() { let chainspec = chainspec(); - let meta = evm_meta( - &chainspec, - legacy_transaction(Some(CHAIN_ID), BASE_FEE_WEI - 1, 21_000), - ); - assert!(matches!( - meta.is_config_compliant(&chainspec, TimeDiff::from_seconds(0), Timestamp::zero()), - Err(InvalidTransaction::Evm(EvmTransactionError::GasPriceBelowBaseFee { - gas_price, - base_fee - })) if gas_price == BASE_FEE_WEI - 1 && base_fee == BASE_FEE_WEI - )); + for chain_id in [Some(CHAIN_ID), None] { + let meta = evm_meta( + &chainspec, + legacy_transaction(chain_id, BASE_FEE_WEI - 1, 21_000), + ); + assert!(matches!( + meta.is_config_compliant(&chainspec, TimeDiff::from_seconds(0), Timestamp::zero()), + Err(InvalidTransaction::Evm(EvmTransactionError::GasPriceBelowBaseFee { + gas_price, + base_fee + })) if gas_price == BASE_FEE_WEI - 1 && base_fee == BASE_FEE_WEI + )); + } } #[test] diff --git a/node/src/types/transaction/meta_transaction/meta_evm.rs b/node/src/types/transaction/meta_transaction/meta_evm.rs index ae4f672fb2..f239c9a75c 100644 --- a/node/src/types/transaction/meta_transaction/meta_evm.rs +++ b/node/src/types/transaction/meta_transaction/meta_evm.rs @@ -105,11 +105,13 @@ impl MetaEvmTransaction { } let expected = evm_config.chain_id; - let actual = transaction - .chain_id() - .ok_or(EvmTransactionError::MissingChainId)?; - if actual != expected { - return Err(EvmTransactionError::ChainIdMismatch { expected, actual }); + // Follow Osaka's chain-ID semantics: unprotected legacy transactions may omit the ID; + // when present, it must match the network. Typed envelopes require it during verification. + // https://ethereum.github.io/execution-specs/src/ethereum/forks/osaka/fork.py.html#check_transaction + if let Some(actual) = transaction.chain_id() { + if actual != expected { + return Err(EvmTransactionError::ChainIdMismatch { expected, actual }); + } } let gas_limit = transaction.gas_limit(); diff --git a/types/src/evm/transaction.rs b/types/src/evm/transaction.rs index d710f41f58..1fa2bc13b8 100644 --- a/types/src/evm/transaction.rs +++ b/types/src/evm/transaction.rs @@ -628,8 +628,6 @@ pub enum EvmTransactionError { HashMismatch, /// The sender address could not be recovered from the signature. SenderRecovery(String), - /// Reconstructing the signed envelope produced metadata different from this transaction. - InconsistentEnvelope, } impl Display for EvmTransactionError { @@ -780,9 +778,6 @@ impl Display for EvmTransactionError { EvmTransactionError::SenderRecovery(error) => { write!(formatter, "EVM transaction sender recovery error: {error}") } - EvmTransactionError::InconsistentEnvelope => { - formatter.write_str("EVM transaction fields do not match signed envelope") - } } } }