-
Notifications
You must be signed in to change notification settings - Fork 83
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.
What are the limitations of this approach?
It's a bit more expensive as we need to track each stack/memory's context-related segment length in a dict instead of as a simple variable. |
Let's discuss it in our weekly |
|
@Eikix If you're okay with it I propose to merge the current design proposal, and to apply any modifications from SW's feedback in a new PR |
@Eikix Reworked this:
|
@Eikix Changes applied |
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 🔥
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.
mb just noticed this PR does not have mentions of "dirtyStorage"?
is dirtyStorage the equivalent of local_changes
?
should we modify the other pr #360
dirtyStorage is a name that is unique to Geth. For us, it's the combination of |
Pull Request type
In this design document, we dive into a refactor of the Kakarot's EVM-Machine implementation, compatible with Cairo's limitations raised in starkware-libs/cairo#4032.
The accepted proposed design will need to:
I believe that the proposed one performs well in all the points mentioned above. Please discuss it extensively!
Please check the type of change your PR introduces:
What is the current behavior?
Resolves: #
What is the new behavior?
Does this introduce a breaking change?