-
Notifications
You must be signed in to change notification settings - Fork 83
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: architecture refactor #604
Conversation
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.
first review will continue tomorrow
this PR is pixel perfect it's pure music to my ears, juice to my heart
/// Prepare the initialization of a new child or so-called sub-context | ||
/// As part of the CALL family of opcodes. | ||
fn prepare_call(ref self: Machine, call_type: @CallType) -> Result<CallArgs, EVMError> { | ||
fn prepare_call(ref self: VM, call_type: @CallType) -> Result<CallArgs, EVMError> { | ||
let gas = self.stack.pop_u128()?; |
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.
just realizing here that we can't just accept the gas that is popped from the gas, as it can be maxUint256
we have to take a min,
can create a separate issue though: https://github.com/ethereum/execution-specs/blob/3fe6514f2d9d234e760d11af883a47c1263eff51/src/ethereum/shanghai/vm/instructions/system.py#L370C27-L370C27
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.
make it a new issue
scarb fmt check |
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.
beautiful
Pull Request type
Please check the type of change your PR introduces:
What is the current behavior?
Resolves: #598
What is the new behavior?
Does this introduce a breaking change?