Tests for EIP-3541 by gumb0 · Pull Request #835 · ethereum/tests

gumb0 · 2021-04-29T17:30:32Z

Reject new contracts starting with the 0xEF byte https://eips.ethereum.org/EIPS/eip-3541

Imlementation is merged in geth ethereum/go-ethereum#22809

axic · 2021-04-29T20:33:28Z

+       gas: !!int -1
+       value: !!int -1
+      network:
+        - 'Berlin'


Would this work?

Suggested change

- 'Berlin'

- '<=Berlin'

Nonce for created accounts was 0 before state-clearing fork, so we would need either make tests a bit more complicated with another nonce expectation for old forks, or remove nonce expectation, or list only forks starting from SpuriousDragon.

But it looks usual in new tests to check only new behaviour and maximum of one previous fork, not caring about repeating it for old forks with the same rules. @winsvega can correct me if it's not ok approach.

Yes because we have copy of the tests in legacy with old forks expect section. However new tests are targeting only recent forks (as also pointed out by Alexy Akhunov)

Since we already have client consensus with existing tests, no really need to cover previous forks.

You can use a sstore operation and check that it worked (instead of nonce)

Another issue when having tests with 5 or more forks it takes more time to generate and run it. So I plan to archive tests from time to time refreshing the expect section to the most recent fork only.

for full node implementation IMHO it is good to have tests that check that new changes does not break anything in the past. so if you really think we should check it here we can make an exception.

winsvega · 2021-04-29T22:51:59Z

+  transaction:
+    data:
+      #- ':yul { mstore8(0, 0xef) return(0, 1) }'
+      - ':label deploying_0xef :raw 0x60ef60005360016000f3'


Yul didn't work?

It did, I just wanted to show raw code here, maybe include it in EIP text.

gumb0 · 2021-05-04T16:49:35Z

@winsvega On the last ACD call EIP-3541 was "accepted for London unless anyone objects on the next call", so I think it would be ok to start reviewing it.

I will keep PR in draft until EIP is accepted.

axic · 2021-05-04T16:52:13Z

I will keep PR in draft until EIP is accepted.

Just to shrink the size, do you think it makes sense splitting this and merging the non-London specific tests? i.e. would that just mean a test creating a contract with every single starting byte from 0x00 to 0xff?

gumb0 · 2021-05-04T17:26:21Z

I will keep PR in draft until EIP is accepted.

Just to shrink the size, do you think it makes sense splitting this and merging the non-London specific tests? i.e. would that just mean a test creating a contract with every single starting byte from 0x00 to 0xff?

Everything could be merged if we remove London expectations.
But I guess tests that deploy 0xef00...00 contracts would not be very interesting for Berlin.
Tests that deploy all possible starting bytes might be.
And the test undefinedOpcodeFirstByte which checks already deployed contracts behaviour is kind of independent.

I would leave for @winsvega to decide how it would be convenient to split this, if needed.

winsvega · 2021-05-04T17:37:53Z

If there is no London specific logic we can keep the test under any name. Tests fork configs are now translatable with clients configs in retesteth config dir

winsvega · 2021-05-04T20:43:25Z

Is this eip enabled in Aleut?

gumb0 · 2021-05-05T08:16:51Z

Is this eip enabled in Aleut?

No. https://github.com/ethereum/eth1.0-specs/blob/master/network-upgrades/client-integration-testnets/aleut.md

But included in Baikal. https://github.com/ethereum/eth1.0-specs/blob/master/network-upgrades/client-integration-testnets/baikal.md

gumb0 · 2021-05-10T11:45:55Z

Latest geth master supports EIP-3541 as part of London fork, so I replaced Berlin+3541 with London now.
Pulled out undefinedOpcodeFirstByte test into Calling a contract which has an undefined opcode as first byte of code #836
Added (generated) blockchain tests.

gumb0 · 2021-05-19T12:26:46Z

@winsvega I have added currentBaseFee required by the latest change in test format and regenerated tests with the latest geth and retesteth. I think this is ready for review.

The commit Generate London cases for stTransactionTest/Opcodes_TransactionInit is separate only to make simpler the diff for the last commit Init code failing to deploy EF byte and rolling back the side effects.

winsvega

I think this looks really good now.
It is great to have contributions from EIP developers finaly.
Great thanks to Ori and his documentation.

gumb0 force-pushed the eip-3541 branch from 29caeb9 to 0c142f4 Compare April 29, 2021 20:21

axic reviewed Apr 29, 2021

View reviewed changes

winsvega reviewed Apr 29, 2021

View reviewed changes

gumb0 force-pushed the eip-3541 branch from ed80640 to ab3bc30 Compare April 30, 2021 10:43

gumb0 commented May 3, 2021

View reviewed changes

Comment thread src/GeneralStateTestsFiller/stBadOpcode/undefinedOpcodeFirstByteFiller.yml

gumb0 force-pushed the eip-3541 branch 2 times, most recently from 156db28 to 22be707 Compare May 3, 2021 14:41

gumb0 commented May 3, 2021

View reviewed changes

Comment thread src/GeneralStateTestsFiller/stTransactionTest/Opcodes_TransactionInitFiller.json Outdated

gumb0 requested a review from winsvega May 4, 2021 16:49

gumb0 mentioned this pull request May 6, 2021

Calling a contract which has an undefined opcode as first byte of code #836

Merged

winsvega added this to the London milestone May 7, 2021

axic mentioned this pull request May 9, 2021

Implement EIP3541 ethereumjs/ethereumjs-monorepo#1240

Merged

This was referenced May 9, 2021

EIP-3541 ethereum/tests Run ethereumjs/ethereumjs-monorepo#1246

Closed

EIP-3541 ethereum/tests Run ethereumjs/ethereumjs-monorepo#1247

Closed

gumb0 force-pushed the eip-3541 branch from ee644d1 to e28e58d Compare May 10, 2021 11:42

gumb0 force-pushed the eip-3541 branch from e28e58d to f8e9525 Compare May 10, 2021 13:20

gumb0 mentioned this pull request May 10, 2021

Generate London cases for stTransactionTest/Opcodes_TransactionInit #839

Closed

gumb0 force-pushed the eip-3541 branch from f8e9525 to 39bf2a1 Compare May 10, 2021 13:34

axic mentioned this pull request May 10, 2021

Proposal to include EIP-3541 in London ethereum/pm#307

Closed

gumb0 force-pushed the eip-3541 branch from 39bf2a1 to d506919 Compare May 10, 2021 18:27

This was referenced May 11, 2021

Init code with invalid first byte #842

Merged

CREATE/CREATE2 a contract with all possible first bytes #843

Merged

gumb0 force-pushed the eip-3541 branch from d506919 to a8b927e Compare May 14, 2021 14:59

gumb0 force-pushed the eip-3541 branch from a8b927e to 0ff4dd9 Compare May 14, 2021 15:12

gumb0 mentioned this pull request May 14, 2021

Check for correct rollback of init code with side effects #844

Merged

gumb0 force-pushed the eip-3541 branch from 0ff4dd9 to 7f1dc16 Compare May 17, 2021 17:17

gumb0 mentioned this pull request May 18, 2021

Tests for EIP-3540: EVM Object Format (EOF) v1 #847

Merged

gumb0 added 3 commits May 19, 2021 12:46

EIP-3541 tests for creation transaction

8fe408c

EIP-3541 tests for CREATE opcode

15af41f

EIP-3541 tests for CREATE2 opcode

2936b3b

gumb0 force-pushed the eip-3541 branch 2 times, most recently from 0a94b64 to 6fdc1e5 Compare May 19, 2021 12:20

gumb0 added 2 commits May 19, 2021 14:24

Update CREATE_FirstByte_loop test for London

41cb04e

Update CREATE2_FirstByte_loop test for London

c26a09d

gumb0 force-pushed the eip-3541 branch from 6fdc1e5 to 9cc202b Compare May 19, 2021 12:24

gumb0 marked this pull request as ready for review May 19, 2021 12:27

gumb0 commented May 19, 2021

View reviewed changes

Comment thread src/GeneralStateTestsFiller/stTransactionTest/Opcodes_TransactionInitFiller.json Outdated

gumb0 added 2 commits May 19, 2021 14:33

Generate London cases for stTransactionTest/Opcodes_TransactionInit

0eecf31

Init code failing to deploy EF byte and rolling back the side effects.

fe77780

gumb0 force-pushed the eip-3541 branch from 9cc202b to fe77780 Compare May 19, 2021 12:35

winsvega approved these changes May 19, 2021

View reviewed changes

winsvega added the ready-to-merge PR is ready to merge and not WIP label May 19, 2021

winsvega requested a review from axic May 19, 2021 21:28

winsvega merged commit b0ef527 into ethereum:develop May 22, 2021

Conversation

gumb0 commented Apr 29, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

axic Apr 29, 2021

Choose a reason for hiding this comment

Uh oh!

gumb0 Apr 30, 2021

Choose a reason for hiding this comment

Uh oh!

winsvega Apr 30, 2021

Choose a reason for hiding this comment

Uh oh!

winsvega May 19, 2021

Choose a reason for hiding this comment

Uh oh!

winsvega Apr 29, 2021

Choose a reason for hiding this comment

Uh oh!

gumb0 Apr 30, 2021

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

gumb0 commented May 4, 2021

Uh oh!

axic commented May 4, 2021

Uh oh!

gumb0 commented May 4, 2021

Uh oh!

winsvega commented May 4, 2021

Uh oh!

winsvega commented May 4, 2021

Uh oh!

gumb0 commented May 5, 2021

Uh oh!

gumb0 commented May 10, 2021

Uh oh!

gumb0 commented May 19, 2021

Uh oh!

Uh oh!

winsvega left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

gumb0 commented Apr 29, 2021 •

edited

Loading