Skip to content

EIP-3540: Test EOF initcode cannot create legacy contracts#1112

Closed
hugo-dc wants to merge 1 commit intoethereum:developfrom
ipsilon:eip-3540-update
Closed

EIP-3540: Test EOF initcode cannot create legacy contracts#1112
hugo-dc wants to merge 1 commit intoethereum:developfrom
ipsilon:eip-3540-update

Conversation

@hugo-dc
Copy link
Copy Markdown
Member

@hugo-dc hugo-dc commented Dec 7, 2022

EOF initcode (in tx data, or CREATE/CREATE2 initcode) creating Legacy contracts are now expected to be invalid, previous tests were moved to the corresponding test file testing invalid EOF code/initcode.

Tests were filled using this geth version: https://github.com/ipsilon/go-ethereum/tree/eip-3670

@hugo-dc hugo-dc marked this pull request as ready for review December 7, 2022 04:00
@hugo-dc hugo-dc changed the title Test EOF initcode cannot create legacy contracts EIP-3540: Test EOF initcode cannot create legacy contracts Dec 8, 2022
@gumb0
Copy link
Copy Markdown
Member

gumb0 commented Dec 8, 2022

Need to move this to new folder GeneralStateTests/stEOF/stEIP3540

@winsvega
Copy link
Copy Markdown

GeneralStateTests/EIPTests/stEOF/

@hugo-dc
Copy link
Copy Markdown
Member Author

hugo-dc commented Dec 16, 2022

Rebased develop, tests were moved to the correct directory.

@gumb0
Copy link
Copy Markdown
Member

gumb0 commented Dec 19, 2022

EOF1_Execution and EOF1_Calls filled tests changed, but I don't see a change in fillers, why is that?

@gumb0
Copy link
Copy Markdown
Member

gumb0 commented Dec 19, 2022

EOF1_Execution and EOF1_Calls filled tests changed, but I don't see a change in fillers, why is that?

Maybe it was in those missing fillers, but I think we're missing here to check that inside EOF contract CREATE/CREATE2 with legacy initcode results in a failure.

@gumb0
Copy link
Copy Markdown
Member

gumb0 commented Dec 20, 2022

EOF1_Execution and EOF1_Calls filled tests changed, but I don't see a change in fillers, why is that?

@winsvega this could have been detected by CI (test files updated without update in fillers)

@winsvega
Copy link
Copy Markdown

Its vice versa. We check the filler hash. If filler is changed test must be updated.

We trust that no commit happens like that

@gumb0
Copy link
Copy Markdown
Member

gumb0 commented Dec 20, 2022

You could addionally check that if test was updated, filler hash was changed.

@hugo-dc
Copy link
Copy Markdown
Member Author

hugo-dc commented Dec 22, 2022

EOF1_Execution and EOF1_Calls filled tests changed, but I don't see a change in fillers, why is that?

This is interesting: The filler didn't change at all!

The JSON file is changed because, I'm using a different solc version or a not optimized one.

For example: In my solc version, the Yul code { sstore(1, 1) } gets compiled to PUSH1(1) PUSH1(1) SSTORE, but in your version this would be compiled to: PUSH1(1) DUP1 SSTORE.

@gumb0
Copy link
Copy Markdown
Member

gumb0 commented Dec 22, 2022

I see, I will delete the refilled files without fillers then.

@gumb0
Copy link
Copy Markdown
Member

gumb0 commented Dec 22, 2022

You could addionally check that if test was updated, filler hash was changed.

I guess my suggestion isn't really straightforward - because someone might want to refill the tests without changing the fillers, e.g. with new implementation or new comilers.
(I still can imagine a warning if neither of tooling nor fillers changed, but filled files did)

@gumb0
Copy link
Copy Markdown
Member

gumb0 commented Dec 22, 2022

Maybe it was in those missing fillers, but I think we're missing here to check that inside EOF contract CREATE/CREATE2 with legacy initcode results in a failure.

Actually these cases exist in CREATE_EOF1_FromEOF and CREATE2_EOF1_FromEOF and apparently they succeeded, so implementation was wrong.

We should try next with new geth implementation (and probably move these cases to _Invalid part)

@hugo-dc
Copy link
Copy Markdown
Member Author

hugo-dc commented Jan 2, 2023

Closing in favor of: #1127

@hugo-dc hugo-dc closed this Jan 2, 2023
@axic axic deleted the eip-3540-update branch May 6, 2023 00:23
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.

3 participants