-
Notifications
You must be signed in to change notification settings - Fork 370
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
Berlin support #40
Berlin support #40
Conversation
This reverts commit d7ea767.
Can you merge master and do rustfmt again? Thank you! |
src/executor/stack/mod.rs
Outdated
|
||
/// Stack-based executor. | ||
pub struct StackExecutor<'config, S> { | ||
pub struct StackExecutor<'config, S: StackState<'config>, P: Precompiles<S>> { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What's the rationale for this change? One of the drawback of this is that it'll be potentially difficult for users to get a single type for StackExecutor
suitable for different configurations of precompiles. Same reason why Config
is not a generic parameter.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
That might've been an overzealous mistake there. I'll double check.
2b60e2d
to
7da1cbd
Compare
Co-authored-by: Wei Tang <accounts@that.world>
7da1cbd
to
d16bdec
Compare
051ec20
to
a252024
Compare
Updated our nightly and we are getting benefits of reduced compile size and speed. So, going to keep it in. |
impl<'config, S: StackState<'config>> StackExecutor<'config, S> { | ||
/// Create a new stack-based executor. | ||
pub fn new(state: S, config: &'config Config) -> Self { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can we add this function back?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM mostly, but it would be great to add StackExecutor::new
back.
This PR adds both EIP-2929 and EIP-2930 into the EVM.
This is a key PR as it brings the necessary changes to get Berlin hard fork working with the EVM. Since this is a critical update, a discussion and review will be very welcomed. Do note that we will also be updating evm-tests as well to ensure full compatibility with Ethereum VM.
Others related to Berlin, EIP-2565 and EIP-2718, should be done on libraries implementing this change.