Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat: Optimism execution changes #682

Merged
merged 90 commits into from
Sep 27, 2023
Merged

Conversation

clabby
Copy link
Contributor

@clabby clabby commented Sep 4, 2023

Overview

Implements Optimism's execution changes into revm behind the optimism feature flag. These changes enable native Optimism execution changes in op-reth, foundry, anvil, and other projects building on top of revm.

Goal

With the optimism feature flag enabled, a new configuration option is exposed that allows for enabling Optimism's execution changes at runtime. This is so that tools such as Foundry can swap between regular Ethereum execution and Optimism execution as needed (i.e. for multichain fork testing). revm's regular logic should be unaltered if either the optimism cfg option is false or the optimism feature flag is not enabled.

New Hardforks

  • Bedrock (Ethereum Base: Merge)
  • Regolith (Base: Bedrock)

TODO

  • Tests
    • Unit tests
  • Implement the spec types + extra TxEnv fields within revm-primitives.
  • Ignore transaction field validation and nonce / balance checks on deposit transactions.
  • Alter gas accounting for deposit transactions in both the Bedrock and Regolith hardforks.
  • Mint ETH at the start of the state transition if the transaction is a deposit with a Some mint field.
  • Pay out fees to the fee vaults rather than coinbase in non-deposit transactions when the optimism configuration flag is enabled.
  • Implement the L1 fee calculation by reading the storage of the L1Block contract.

Future

  • Benchmarks
    • How much overhead does the runtime branching on the optimism cfg option add when the optimism feature flag is enabled but the optimism cfg option is not? For example, in order for foundry to swap between optimism and ethereum execution modes, the regular release build will have to enable the optimism feature flag on revm. Does the runtime branching significantly slow down execution for the majority of tests which are not on OP?

crates/revm/src/evm_impl.rs Outdated Show resolved Hide resolved
crates/primitives/src/env.rs Outdated Show resolved Hide resolved
crates/revm/src/evm_impl.rs Outdated Show resolved Hide resolved
crates/revm/src/evm_impl.rs Outdated Show resolved Hide resolved
crates/revm/src/evm_impl.rs Outdated Show resolved Hide resolved
crates/revm/src/optimism.rs Outdated Show resolved Hide resolved
crates/revm/src/optimism.rs Outdated Show resolved Hide resolved
crates/revm/src/evm_impl.rs Outdated Show resolved Hide resolved
crates/revm/Cargo.toml Outdated Show resolved Hide resolved
crates/revm/src/evm_impl.rs Show resolved Hide resolved
crates/primitives/src/env.rs Show resolved Hide resolved
crates/primitives/src/env.rs Outdated Show resolved Hide resolved
@clabby clabby requested a review from rakita September 11, 2023 18:53
@clabby
Copy link
Contributor Author

clabby commented Sep 11, 2023

Think this is ready for a another round of review. What are your normal standards for tests @rakita? @refcell and I looked into integrating with t8n and t9n in order to generate state tests for OP, but that may be a bit further out. Glad to attempt to hook this version into op-reth and test syncing on top of the unit tests we've added!

Feel free to let us know if there is anything we can change as well 😄

@refcell
Copy link
Contributor

refcell commented Sep 18, 2023

Would really really appreciate another round of review here

@clabby clabby marked this pull request as ready for review September 18, 2023 17:43
Copy link
Member

@rakita rakita left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! One nit comment.

bins/revme/src/statetest/runner.rs Outdated Show resolved Hide resolved
@refcell
Copy link
Contributor

refcell commented Sep 27, 2023

Just FYI @rakita neither @clabby nor I have write access so I think it's up to you to hit merge 😄

@rakita rakita merged commit f79d0e1 into bluealloy:main Sep 27, 2023
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants