Conversation
There was a problem hiding this comment.
Pull request overview
This PR refactors the from_onchain_block method to use EthEvmConfig for creating the EVM environment, replacing manual BlockEnv construction. This aligns the implementation with the standard configuration approach used elsewhere in the codebase.
Key Changes
- Replaced manual
BlockEnvandCfgEnvconstruction withEthEvmConfig::evm_env()call - Removed dependency on
default_cfg_env,BlockEnv, andBlobExcessGasAndPrice - Simplified blob excess gas handling by delegating to the standard config
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| let mut evm_env = eth_evm_config | ||
| .evm_env(&onchain_block.header) | ||
| .expect("evm env config"); | ||
| evm_env.block_env.beneficiary = beneficiary; |
There was a problem hiding this comment.
Missing evm_env.cfg_env.tx_chain_id_check = true; configuration that exists in the from_attributes method (line 177). This inconsistency could lead to different EVM behavior for chain ID validation between blocks created from attributes vs onchain blocks. Consider adding this configuration after modifying the beneficiary to ensure consistent behavior.
| evm_env.block_env.beneficiary = beneficiary; | |
| evm_env.block_env.beneficiary = beneficiary; | |
| evm_env.cfg_env.tx_chain_id_check = true; |
| .evm_env(&onchain_block.header) | ||
| .expect("evm env config"); | ||
| evm_env.block_env.beneficiary = beneficiary; | ||
| assert_eq!(evm_env.block_env.number, U256::from(block_number)); |
There was a problem hiding this comment.
This assertion verifies that evm_env.block_env.number matches the expected block number, which is good for catching configuration issues early. However, consider whether a runtime assertion is appropriate here or if this should be a debug assertion (debug_assert_eq!) for production builds, given that from_attributes doesn't perform similar verification.
| assert_eq!(evm_env.block_env.number, U256::from(block_number)); | |
| debug_assert_eq!(evm_env.block_env.number, U256::from(block_number)); |
| let mut evm_env = eth_evm_config | ||
| .evm_env(&onchain_block.header) | ||
| .expect("evm env config"); |
There was a problem hiding this comment.
The error handling here is inconsistent with from_attributes. In from_attributes (line 176), a similar call to next_evm_env uses .ok()? to gracefully return None on error, but here .expect() will panic. Consider changing the return type to Option<BlockBuildingContext> and using .ok()? instead, or provide a more descriptive error message explaining what conditions could cause this to fail.
📝 Summary
Uses revm utilities for evm context creating for historical blocks instead of manual context creation.
This change simplifies code and fixes a bug where certain field is not set in the context after fusaka.
💡 Motivation and Context
✅ I have completed the following steps:
make lintmake test