diff --git a/cmd/ef_tests/blockchain/tests/prague.rs b/cmd/ef_tests/blockchain/tests/prague.rs index 22cbcd48e6..a2b2f93cb4 100644 --- a/cmd/ef_tests/blockchain/tests/prague.rs +++ b/cmd/ef_tests/blockchain/tests/prague.rs @@ -5,36 +5,20 @@ use ethrex_vm::EvmEngine; // TODO: enable these tests once the evm is updated. #[cfg(not(feature = "levm"))] -const SKIPPED_TESTS_REVM: [&str; 13] = [ +const SKIPPED_TESTS_REVM: [&str; 5] = [ "tests/prague/eip7702_set_code_tx/test_set_code_txs.py::test_set_code_to_non_empty_storage[fork_Prague-blockchain_test-zero_nonce]", - "tests/prague/eip7002_el_triggerable_withdrawals/test_contract_deployment.py::test_system_contract_deployment[fork_CancunToPragueAtTime15k-blockchain_test-deploy_after_fork-nonzero_balance]", - "tests/prague/eip7002_el_triggerable_withdrawals/test_contract_deployment.py::test_system_contract_deployment[fork_CancunToPragueAtTime15k-blockchain_test-deploy_after_fork-zero_balance]", - "tests/prague/eip7002_el_triggerable_withdrawals/test_modified_withdrawal_contract.py::test_system_contract_errors[fork_Prague-blockchain_test-system_contract_out_of_gas-system_contract_0x00000961ef480eb55e80d19ad83579a64c007002]", - "tests/prague/eip7002_el_triggerable_withdrawals/test_modified_withdrawal_contract.py::test_system_contract_errors[fork_Prague-blockchain_test-system_contract_reverts-system_contract_0x00000961ef480eb55e80d19ad83579a64c007002]", "tests/prague/eip7002_el_triggerable_withdrawals/test_modified_withdrawal_contract.py::test_system_contract_errors[fork_Prague-blockchain_test-system_contract_reaches_gas_limit-system_contract_0x00000961ef480eb55e80d19ad83579a64c007002]", "tests/prague/eip7002_el_triggerable_withdrawals/test_modified_withdrawal_contract.py::test_system_contract_errors[fork_Prague-blockchain_test-system_contract_throws-system_contract_0x00000961ef480eb55e80d19ad83579a64c007002]", - "tests/prague/eip7251_consolidations/test_contract_deployment.py::test_system_contract_deployment[fork_CancunToPragueAtTime15k-blockchain_test-deploy_after_fork-zero_balance]", - "tests/prague/eip7251_consolidations/test_contract_deployment.py::test_system_contract_deployment[fork_CancunToPragueAtTime15k-blockchain_test-deploy_after_fork-nonzero_balance]", "tests/prague/eip7251_consolidations/test_modified_consolidation_contract.py::test_system_contract_errors[fork_Prague-blockchain_test-system_contract_reaches_gas_limit-system_contract_0x0000bbddc7ce488642fb579f8b00f3a590007251]", - "tests/prague/eip7251_consolidations/test_modified_consolidation_contract.py::test_system_contract_errors[fork_Prague-blockchain_test-system_contract_out_of_gas-system_contract_0x0000bbddc7ce488642fb579f8b00f3a590007251]", "tests/prague/eip7251_consolidations/test_modified_consolidation_contract.py::test_system_contract_errors[fork_Prague-blockchain_test-system_contract_throws-system_contract_0x0000bbddc7ce488642fb579f8b00f3a590007251]", - "tests/prague/eip7251_consolidations/test_modified_consolidation_contract.py::test_system_contract_errors[fork_Prague-blockchain_test-system_contract_reverts-system_contract_0x0000bbddc7ce488642fb579f8b00f3a590007251]", ]; #[cfg(feature = "levm")] -const SKIPPED_TESTS_LEVM: [&str; 46] = [ - "tests/prague/eip7002_el_triggerable_withdrawals/test_contract_deployment.py::test_system_contract_deployment[fork_CancunToPragueAtTime15k-blockchain_test-deploy_after_fork-nonzero_balance]", - "tests/prague/eip7002_el_triggerable_withdrawals/test_contract_deployment.py::test_system_contract_deployment[fork_CancunToPragueAtTime15k-blockchain_test-deploy_after_fork-zero_balance]", - "tests/prague/eip7002_el_triggerable_withdrawals/test_modified_withdrawal_contract.py::test_system_contract_errors[fork_Prague-blockchain_test-system_contract_out_of_gas-system_contract_0x00000961ef480eb55e80d19ad83579a64c007002]", - "tests/prague/eip7002_el_triggerable_withdrawals/test_modified_withdrawal_contract.py::test_system_contract_errors[fork_Prague-blockchain_test-system_contract_reverts-system_contract_0x00000961ef480eb55e80d19ad83579a64c007002]", +const SKIPPED_TESTS_LEVM: [&str; 38] = [ "tests/prague/eip7002_el_triggerable_withdrawals/test_modified_withdrawal_contract.py::test_system_contract_errors[fork_Prague-blockchain_test-system_contract_reaches_gas_limit-system_contract_0x00000961ef480eb55e80d19ad83579a64c007002]", "tests/prague/eip7002_el_triggerable_withdrawals/test_modified_withdrawal_contract.py::test_system_contract_errors[fork_Prague-blockchain_test-system_contract_throws-system_contract_0x00000961ef480eb55e80d19ad83579a64c007002]", - "tests/prague/eip7251_consolidations/test_contract_deployment.py::test_system_contract_deployment[fork_CancunToPragueAtTime15k-blockchain_test-deploy_after_fork-zero_balance]", - "tests/prague/eip7251_consolidations/test_contract_deployment.py::test_system_contract_deployment[fork_CancunToPragueAtTime15k-blockchain_test-deploy_after_fork-nonzero_balance]", "tests/prague/eip7251_consolidations/test_modified_consolidation_contract.py::test_system_contract_errors[fork_Prague-blockchain_test-system_contract_reaches_gas_limit-system_contract_0x0000bbddc7ce488642fb579f8b00f3a590007251]", - "tests/prague/eip7251_consolidations/test_modified_consolidation_contract.py::test_system_contract_errors[fork_Prague-blockchain_test-system_contract_out_of_gas-system_contract_0x0000bbddc7ce488642fb579f8b00f3a590007251]", "tests/prague/eip7251_consolidations/test_modified_consolidation_contract.py::test_system_contract_errors[fork_Prague-blockchain_test-system_contract_throws-system_contract_0x0000bbddc7ce488642fb579f8b00f3a590007251]", - "tests/prague/eip7251_consolidations/test_modified_consolidation_contract.py::test_system_contract_errors[fork_Prague-blockchain_test-system_contract_reverts-system_contract_0x0000bbddc7ce488642fb579f8b00f3a590007251]", "tests/prague/eip7702_set_code_tx/test_set_code_txs.py::test_set_code_to_precompile_not_enough_gas_for_precompile_execution[fork_Prague-precompile_0x0000000000000000000000000000000000000006-blockchain_test_from_state_test]", "tests/prague/eip7702_set_code_tx/test_set_code_txs.py::test_set_code_to_precompile[fork_Prague-precompile_0x0000000000000000000000000000000000000011-call_opcode_CALL-evm_code_type_LEGACY-blockchain_test_from_state_test]", "tests/prague/eip7702_set_code_tx/test_set_code_txs.py::test_set_code_to_precompile_not_enough_gas_for_precompile_execution[fork_Prague-precompile_0x0000000000000000000000000000000000000009-blockchain_test_from_state_test]", @@ -76,6 +60,10 @@ const SKIPPED_TESTS_LEVM: [&str; 46] = [ //"tests/prague/eip7702_set_code_tx/test_set_code_txs.py::test_set_code_max_depth_call_stack[fork_Prague-blockchain_test]", //"tests/prague/eip7702_set_code_tx/test_set_code_txs_2.py::test_pointer_contract_pointer_loop[fork_Prague-blockchain_test]", +// NOTE: The following test fails because of an OutOfGas error. This happens because it tests a system call to a contract that has a +// code with a cost of +29 million gas that when is being sumed to the 21k base intrinsic gas it goes over the 30 million limit. +// "tests/prague/eip7002_el_triggerable_withdrawals/test_modified_withdrawal_contract.py::test_system_contract_errors[fork_Prague-blockchain_test-system_contract_reaches_gas_limit-system_contract_0x00000961ef480eb55e80d19ad83579a64c007002]", + #[cfg(not(feature = "levm"))] fn parse_and_execute_with_revm(path: &Path) -> datatest_stable::Result<()> { parse_and_execute(path, EvmEngine::REVM, Some(&SKIPPED_TESTS_REVM)); diff --git a/crates/vm/backends/levm/mod.rs b/crates/vm/backends/levm/mod.rs index 9d368266f6..8ce65e88a4 100644 --- a/crates/vm/backends/levm/mod.rs +++ b/crates/vm/backends/levm/mod.rs @@ -17,6 +17,7 @@ use ethrex_common::{ }, Address, H256, U256, }; +use ethrex_levm::constants::EMPTY_CODE_HASH; use ethrex_levm::db::gen_db::GeneralizedDatabase; use ethrex_levm::{ errors::{ExecutionReport, TxResult, VMError}, @@ -300,37 +301,53 @@ impl LEVM { pub(crate) fn read_withdrawal_requests( block_header: &BlockHeader, db: &mut GeneralizedDatabase, - ) -> Option { + ) -> Result { let report = generic_system_contract_levm( block_header, Bytes::new(), db, *WITHDRAWAL_REQUEST_PREDEPLOY_ADDRESS, *SYSTEM_ADDRESS, - ) - .ok()?; + )?; + + // According to EIP-7002 we need to check if the WITHDRAWAL_REQUEST_PREDEPLOY_ADDRESS + // has any code after being deployed. If not, the whole block becomes invalid. + // https://github.com/ethereum/EIPs/blob/master/EIPS/eip-7002.md + let account = db.get_account(*WITHDRAWAL_REQUEST_PREDEPLOY_ADDRESS)?; + if account.info.code_hash == EMPTY_CODE_HASH { + return Err(EvmError::Custom("BlockException.SYSTEM_CONTRACT_EMPTY: WITHDRAWAL_REQUEST_PREDEPLOY_ADDRESS has no code after deployment".to_string())); + } match report.result { - TxResult::Success => Some(report), - _ => None, + TxResult::Success => Ok(report), + // EIP-7002 specifies that a failed system call invalidates the entire block. + TxResult::Revert(vm_error) => Err(EvmError::from(vm_error)), } } pub(crate) fn dequeue_consolidation_requests( block_header: &BlockHeader, db: &mut GeneralizedDatabase, - ) -> Option { + ) -> Result { let report = generic_system_contract_levm( block_header, Bytes::new(), db, *CONSOLIDATION_REQUEST_PREDEPLOY_ADDRESS, *SYSTEM_ADDRESS, - ) - .ok()?; + )?; + + // According to EIP-7251 we need to check if the CONSOLIDATION_REQUEST_PREDEPLOY_ADDRESS + // has any code after being deployed. If not, the whole block becomes invalid. + // https://github.com/ethereum/EIPs/blob/master/EIPS/eip-7251.md + let acc = db.get_account(*CONSOLIDATION_REQUEST_PREDEPLOY_ADDRESS)?; + if acc.info.code_hash == EMPTY_CODE_HASH { + return Err(EvmError::Custom("BlockException.SYSTEM_CONTRACT_EMPTY: CONSOLIDATION_REQUEST_PREDEPLOY_ADDRESS has no code after deployment".to_string())); + } match report.result { - TxResult::Success => Some(report), - _ => None, + TxResult::Success => Ok(report), + // EIP-7251 specifies that a failed system call invalidates the entire block. + TxResult::Revert(vm_error) => Err(EvmError::from(vm_error)), } } @@ -628,21 +645,10 @@ pub fn extract_all_requests_levm( } } - let withdrawals_data: Vec = match LEVM::read_withdrawal_requests(header, db) { - Some(report) => { - // the cache is updated inside the generic_system_call - report.output.into() - } - None => Default::default(), - }; - - let consolidation_data: Vec = match LEVM::dequeue_consolidation_requests(header, db) { - Some(report) => { - // the cache is updated inside the generic_system_call - report.output.into() - } - None => Default::default(), - }; + let withdrawals_data: Vec = LEVM::read_withdrawal_requests(header, db)?.output.into(); + let consolidation_data: Vec = LEVM::dequeue_consolidation_requests(header, db)? + .output + .into(); let deposits = Requests::from_deposit_receipts(chain_config.deposit_contract_address, receipts); let withdrawals = Requests::from_withdrawals_data(withdrawals_data); diff --git a/crates/vm/backends/revm/mod.rs b/crates/vm/backends/revm/mod.rs index fed3c20ec8..d5522f356a 100644 --- a/crates/vm/backends/revm/mod.rs +++ b/crates/vm/backends/revm/mod.rs @@ -16,6 +16,7 @@ use ethrex_storage::{error::StoreError, AccountUpdate}; use revm::db::states::bundle_state::BundleRetention; use revm::db::AccountStatus; +use revm::Database; use revm::{ db::AccountState as RevmAccountState, inspectors::TracerEip3155, @@ -185,42 +186,104 @@ impl REVM { )?; Ok(()) } + fn system_contract_account_info( + addr: Address, + state: &mut EvmState, + ) -> Result { + let revm_addr = RevmAddress::from_slice(addr.as_bytes()); + let account_info = match state { + EvmState::Store(db) => { + let mut evm = Evm::builder().with_db(db).build(); + let evm_db = evm.db_mut(); + evm_db.basic(revm_addr)? + } + EvmState::Execution(cache_db) => { + let mut evm = Evm::builder().with_db(cache_db).build(); + let evm_cache_db = evm.db_mut(); + evm_cache_db.basic(revm_addr)? + } + } + .ok_or(EvmError::DB(StoreError::Custom( + "WITHDRAWAL_REQUEST_PREDEPLOY_ADDRESS was not found after deployment".to_string(), + )))?; + Ok(account_info) + } pub(crate) fn read_withdrawal_requests( block_header: &BlockHeader, state: &mut EvmState, - ) -> Option> { + ) -> Result, EvmError> { let tx_result = generic_system_contract_revm( block_header, Bytes::new(), state, *WITHDRAWAL_REQUEST_PREDEPLOY_ADDRESS, *SYSTEM_ADDRESS, - ) - .ok()?; + )?; + + // According to EIP-7002 we need to check if the WITHDRAWAL_REQUEST_PREDEPLOY_ADDRESS + // has any code after being deployed. If not, the whole block becomes invalid. + // https://github.com/ethereum/EIPs/blob/master/EIPS/eip-7002.md + let account_info = + Self::system_contract_account_info(*WITHDRAWAL_REQUEST_PREDEPLOY_ADDRESS, state)?; + if account_info.is_empty_code_hash() { + return Err(EvmError::Custom("BlockException.SYSTEM_CONTRACT_EMPTY: WITHDRAWAL_REQUEST_PREDEPLOY_ADDRESS has no code after deployment".to_string())); + } - if tx_result.is_success() { - Some(tx_result.output().into()) - } else { - None + match tx_result { + ExecutionResult::Success { + gas_used: _, + gas_refunded: _, + logs: _, + output, + } => Ok(output.into()), + // EIP-7002 specifies that a failed system call invalidates the entire block. + ExecutionResult::Halt { reason, gas_used } => { + let err_str = format!("Transaction HALT when calling WITHDRAWAL_REQUEST_PREDEPLOY_ADDRESS with reason: {reason} and with used gas: {gas_used}"); + Err(EvmError::Custom(err_str)) + } + ExecutionResult::Revert { gas_used, output } => { + let err_str = format!("Transaction REVERT when calling WITHDRAWAL_REQUEST_PREDEPLOY_ADDRESS with output: {:?} and with used gas: {gas_used}", output); + Err(EvmError::Custom(err_str)) + } } } pub(crate) fn dequeue_consolidation_requests( block_header: &BlockHeader, state: &mut EvmState, - ) -> Option> { + ) -> Result, EvmError> { let tx_result = generic_system_contract_revm( block_header, Bytes::new(), state, *CONSOLIDATION_REQUEST_PREDEPLOY_ADDRESS, *SYSTEM_ADDRESS, - ) - .ok()?; + )?; - if tx_result.is_success() { - Some(tx_result.output().into()) - } else { - None + // According to EIP-7251 we need to check if the CONSOLIDATION_REQUEST_PREDEPLOY_ADDRESS + // has any code after being deployed. If not, the whole block becomes invalid. + // https://github.com/ethereum/EIPs/blob/master/EIPS/eip-7251.md + let account_info = + Self::system_contract_account_info(*CONSOLIDATION_REQUEST_PREDEPLOY_ADDRESS, state)?; + if account_info.is_empty_code_hash() { + return Err(EvmError::Custom("BlockException.SYSTEM_CONTRACT_EMPTY: CONSOLIDATION_REQUEST_PREDEPLOY_ADDRESS has no code after deployment".to_string())); + } + + match tx_result { + ExecutionResult::Success { + gas_used: _, + gas_refunded: _, + logs: _, + output, + } => Ok(output.into()), + // EIP-7251 specifies that a failed system call invalidates the entire block. + ExecutionResult::Halt { reason, gas_used } => { + let err_str = format!("Transaction HALT when calling CONSOLIDATION_REQUEST_PREDEPLOY_ADDRESS with reason: {reason} and with used gas: {gas_used}"); + Err(EvmError::Custom(err_str)) + } + ExecutionResult::Revert { gas_used, output } => { + let err_str = format!("Transaction REVERT when calling CONSOLIDATION_REQUEST_PREDEPLOY_ADDRESS with output: {:?} and with used gas: {gas_used}", output); + Err(EvmError::Custom(err_str)) + } } } @@ -693,11 +756,11 @@ pub fn extract_all_requests( } let deposits = Requests::from_deposit_receipts(config.deposit_contract_address, receipts); - let withdrawals_data = REVM::read_withdrawal_requests(header, state); - let consolidation_data = REVM::dequeue_consolidation_requests(header, state); + let withdrawals_data = REVM::read_withdrawal_requests(header, state)?; + let consolidation_data = REVM::dequeue_consolidation_requests(header, state)?; - let withdrawals = Requests::from_withdrawals_data(withdrawals_data.unwrap_or_default()); - let consolidation = Requests::from_consolidation_data(consolidation_data.unwrap_or_default()); + let withdrawals = Requests::from_withdrawals_data(withdrawals_data); + let consolidation = Requests::from_consolidation_data(consolidation_data); Ok(vec![deposits, withdrawals, consolidation]) }