feat: enable RIP-7212
#51
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Summary of Changes
Hello @johnletey, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed!
This pull request integrates the RIP-7212 precompile into the system, specifically targeting the Prague hardfork. It achieves this by introducing a custom EVM factory and executor, which allows for the dynamic inclusion of new precompiles. This change is crucial for maintaining compatibility with Layer 2 solutions and preparing for future Ethereum protocol upgrades.
Highlights
- RIP-7212 Precompile Integration: This pull request enables RIP-7212, a precompile for secp256r1 curve operations, as a custom precompile for the Prague hardfork. This precompile is already adopted by various L2s and rollups and is slated to be officially included in the Osaka hardfork as EIP-7951.
- Custom EVM and Executor Factories: Introduced
RollkitEvmFactoryandRollkitExecutorBuilderto allow for the integration of custom precompiles within the EVM execution environment. This provides a flexible mechanism to extend EVM functionality. - Prague Hardfork Activation: The Prague hardfork is now activated in the test chain specification and genesis configuration, ensuring that the newly enabled RIP-7212 precompile is available and testable within the specified hardfork context.
Using Gemini Code Assist
The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.
Invoking Gemini
You can request assistance from Gemini at any point in your pull request via creating an issue comment (i.e. comment on the pull request page) using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands.
| Feature | Command | Description |
|---|---|---|
| Code Review | /gemini review |
Performs a code review for the current pull request in its current state. |
| Pull Request Summary | /gemini summary |
Provides a summary of the current pull request in its current state. |
| Comment | @gemini-code-assist | Responds in comments when explicitly tagged, both in issue comments and review comments. |
| Help | /gemini help |
Displays a list of available commands. |
Customization
To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.
Limitations & Feedback
Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here.
You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension.
Footnotes
-
Review the Privacy Notices, Generative AI Prohibited Use Policy, Terms of Service, and learn how to configure Gemini Code Assist in GitHub here. Gemini can make mistakes, so double check it and use code with caution. ↩
There was a problem hiding this comment.
Code Review
This pull request successfully enables RIP-7212 by introducing a custom precompile for the Prague hardfork. The changes are well-structured, with a new RollkitEvmFactory to customize EVM behavior and appropriate wiring throughout the application. The tests and configurations have been correctly updated to reflect these changes. I've identified one high-severity issue related to precompile handling that could lead to consensus failures, and I've provided a fix. Additionally, I've included a medium-severity suggestion to refactor a part of the new logic for improved code quality. Overall, this is a solid contribution.
| let mut evm = Context::mainnet() | ||
| .with_db(db) | ||
| .with_cfg(input.cfg_env) | ||
| .with_block(input.block_env) | ||
| .build_mainnet_with_inspector(NoOpInspector {}) | ||
| .with_precompiles(PrecompilesMap::from_static( | ||
| EthPrecompiles::default().precompiles, | ||
| )); |
There was a problem hiding this comment.
The current implementation incorrectly overrides spec-specific precompiles with those for the LATEST hardfork. The call to .with_precompiles(PrecompilesMap::from_static(EthPrecompiles::default().precompiles)) forces the use of LATEST precompiles, which is incorrect for other hardforks like Cancun. This can lead to consensus failures.
The fix is to remove this explicit with_precompiles call and let revm determine the correct precompiles based on the spec_id. The custom Prague precompiles should only be applied when the spec is indeed PRAGUE.
| let mut evm = Context::mainnet() | |
| .with_db(db) | |
| .with_cfg(input.cfg_env) | |
| .with_block(input.block_env) | |
| .build_mainnet_with_inspector(NoOpInspector {}) | |
| .with_precompiles(PrecompilesMap::from_static( | |
| EthPrecompiles::default().precompiles, | |
| )); | |
| let mut evm = Context::mainnet() | |
| .with_db(db) | |
| .with_cfg(input.cfg_env) | |
| .with_block(input.block_env) | |
| .build_mainnet_with_inspector(NoOpInspector {}); |
| fn create_evm_with_inspector<DB: Database, I: Inspector<Self::Context<DB>, EthInterpreter>>( | ||
| &self, | ||
| db: DB, | ||
| input: EvmEnv, | ||
| inspector: I, | ||
| ) -> Self::Evm<DB, I> { | ||
| EthEvm::new( | ||
| self.create_evm(db, input) | ||
| .into_inner() | ||
| .with_inspector(inspector), | ||
| true, | ||
| ) | ||
| } |
There was a problem hiding this comment.
The create_evm_with_inspector function currently calls create_evm, unwraps the result with into_inner(), and then re-wraps it. This is slightly inefficient and introduces some code duplication.
Consider refactoring the EVM context creation into a private helper function. This helper could take an inspector as an argument and be used by both create_evm (with NoOpInspector) and create_evm_with_inspector. This would improve code clarity and remove the unnecessary wrap/unwrap cycle.
There was a problem hiding this comment.
The interface can't be changed because we are implementing Reth's EvmFactory
Description
This PR enables RIP-7212 as a custom precompile to the Prague hardfork. RIP-7212 is already adopted by various L2s and rollups, and will be officially included in the Osaka hardfork as EIP-7951.
Type of Change
Related Issues
Related to #44
Checklist
Testing
Additional Notes