-
Notifications
You must be signed in to change notification settings - Fork 0
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
[EVM-Equivalence-YUL] Fix overflow checks #29
Conversation
evmGasLeft := chargeGas(evmGasLeft,expandMemory(add(offset,size))) | ||
|
||
checkOverflow(offset,MEM_OFFSET_INNER(), evmGasLeft) |
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.
It is already covered above, isn't it?
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.
I think it's enough to add a comment about it instead of 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.
Right, I already removed them and added a comment instead. I also made the same changes in the return opcode since the checks were identical.
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.
Left one comment, also please run prettier etc.
Otherwise, 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.
Also it’s better to update hashes
What ❔
This PR removes the // todo invalid comments that needed fixes
In revert opcode:
Changed the wrong checks, mimicking the ones in return opcode.
In calldatacopy:
Removed the overflow checks, since it would never fall into the second one, which is the correct one.
Why ❔
Checklist