Skip to content

Conversation

@gumb0
Copy link
Member

@gumb0 gumb0 commented Aug 29, 2019

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.

5 participants