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: architecture refactor #604

Merged
merged 10 commits into from
Dec 1, 2023
Merged

feat: architecture refactor #604

merged 10 commits into from
Dec 1, 2023

Conversation

enitrat
Copy link
Collaborator

@enitrat enitrat commented Nov 30, 2023

Pull Request type

Please check the type of change your PR introduces:

  • Bugfix
  • Feature
  • Code style update (formatting, renaming)
  • Refactoring (no functional changes, no API changes)
  • Build-related changes
  • Documentation content changes
  • Other (please describe):

What is the current behavior?

Resolves: #598

What is the new behavior?

  • Refactored to the new architecture

Does this introduce a breaking change?

  • Yes
  • No

Copy link
Member

@Eikix Eikix left a 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

crates/evm/src/create_helpers.cairo Outdated Show resolved Hide resolved
/// 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()?;
Copy link
Member

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

Copy link
Collaborator Author

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

@Eikix
Copy link
Member

Eikix commented Dec 1, 2023

scarb fmt check

Copy link
Member

@Eikix Eikix left a comment

Choose a reason for hiding this comment

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

beautiful

@Eikix Eikix merged commit a27836a into main Dec 1, 2023
3 of 4 checks passed
@Eikix Eikix deleted the feat/execution_context_refactor branch December 1, 2023 14:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants