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

Implement EIP-2200 #142

Merged
merged 3 commits into from
Nov 12, 2019
Merged

Implement EIP-2200 #142

merged 3 commits into from
Nov 12, 2019

Conversation

gumb0
Copy link
Member

@gumb0 gumb0 commented Aug 29, 2019


// Modified again:
storage.clear();
storage[v1] = {v1, true};
execute(sstore(1, push(2)));
EXPECT_EQ(result.status_code, EVMC_SUCCESS);
EXPECT_EQ(gas_used, rev == EVMC_CONSTANTINOPLE ? 206 : 5006);
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't all these have >= CONSTANTINOPLE?

E.g. (rev >= istanbul) elseif (rev == constantinople) else .. (because it was disabled in petersburg)

Copy link
Member Author

@gumb0 gumb0 Aug 29, 2019

Choose a reason for hiding this comment

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

right, changed now to cover post-Istanbul revisions

... although the loop iterates only till EVMC_ISTANBUL

@codecov-io
Copy link

codecov-io commented Aug 29, 2019

Codecov Report

Merging #142 into master will increase coverage by <.01%.
The diff coverage is 100%.

@@            Coverage Diff             @@
##           master     #142      +/-   ##
==========================================
+ Coverage   99.88%   99.88%   +<.01%     
==========================================
  Files           5        5              
  Lines         845      858      +13     
  Branches      109      113       +4     
==========================================
+ Hits          844      857      +13     
  Misses          1        1

@gumb0 gumb0 changed the title Unit test for EIP-2200 Implement EIP-2200 Aug 29, 2019
@gumb0
Copy link
Member Author

gumb0 commented Aug 29, 2019

"OOG if gas below stipend" is not implemented yet

if (state.rev >= EVMC_ISTANBUL)
{
const auto correction = state.current_block_cost - instr->arg.p.number;
const auto gas_left = static_cast<uint64_t>(state.gas_left + correction);
Copy link
Member Author

Choose a reason for hiding this comment

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

This doesn't seem to work right: for PUSH1 0 PUSH1 0 SSTORE with 2306 starting gas, at the beginning of SSTORE state.gas_left is 2300 and correction is 6

Copy link
Member Author

Choose a reason for hiding this comment

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

got it, arg.p.number should be filled for SSTORE in analysis

@gumb0 gumb0 force-pushed the eip-2200 branch 2 times, most recently from 5f2da5b to 6bead9b Compare September 4, 2019 10:25
@gumb0 gumb0 marked this pull request as ready for review September 4, 2019 10:26
@gumb0 gumb0 force-pushed the eip-2200 branch 2 times, most recently from 0dfe5c9 to 7c65f43 Compare September 17, 2019 17:09
@chfast chfast merged commit a0e6a33 into master Nov 12, 2019
@chfast chfast deleted the eip-2200 branch November 12, 2019 11:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants