-
Notifications
You must be signed in to change notification settings - Fork 285
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
Implement EIP-663: Unlimited SWAP and DUP instructions #529
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #529 +/- ##
==========================================
+ Coverage 98.03% 98.08% +0.05%
==========================================
Files 59 59
Lines 5687 5837 +150
==========================================
+ Hits 5575 5725 +150
Misses 112 112
Flags with carried forward coverage won't be shown. Click here to find out more.
|
a8861e5
to
996d37b
Compare
132aed6
to
c2c0237
Compare
lib/evmone/instructions.hpp
Outdated
return nullptr; | ||
} | ||
|
||
stack.push(stack[n]); |
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 this misses the < 1024
check. push
does not seem enforce the 1024 limit.
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.
The stack overflow check is done automatically by the interpreter. This information is in the instruction traits.
test/unittests/evm_test.cpp
Outdated
EXPECT_STATUS(EVMC_SUCCESS); | ||
EXPECT_OUTPUT_INT(256); | ||
|
||
execute(full_stack_code + OP_DUPN + "fe" + OP_DUPN + "ff"); |
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.
would it overflow with POP as in checks above?
execute(full_stack_code + OP_DUPN + "fe" + OP_DUPN + "ff"); | |
execute(full_stack_code + OP_POP + OP_DUPN + "fe" + OP_DUPN + "ff"); |
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.
Ah I see OP_POP was needed because of ret_top()
in the end
https://eips.ethereum.org/EIPS/eip-663