core/vm: implement eof opcodes #30511
Conversation
MariusVanDerWijden
requested review from
fjl,
holiman,
karalabe,
gballet,
rjl493456442,
zsfelfoldi,
s1na and
lightclient
as code owners
|
Merged eof-validation, rebase time :) |
MariusVanDerWijden
force-pushed
the
eof-opcodes
branch
from
f6dd3c8
to
73a3a54
Compare
|
Rebased, looks like I broke some verkle stuff, will fix later |
core/state_transition.go
Outdated
| codeHash := st.state.GetCodeHash(msg.From) | ||
| if codeHash != (common.Hash{}) && codeHash != types.EmptyCodeHash { | ||
| code := st.state.GetCode(msg.From) | ||
| if 0 < len(code) && !bytes.HasPrefix(code, []byte{0xef, 0x01, 0x00}) { |
There was a problem hiding this comment.
What is this? Previously, we entered here if codehash told is it was "non-empty-code".
Now, we enter here if length of code is >0, ok, but why check against ef, 01, 00?
And that's not even the eof-prefix+version, which is ef, 00,01.
Also please don't do if 0 < foo, do if foo > 0. We're not testing the 0, we're testing the foo.
There was a problem hiding this comment.
I have no idea where that code came from, I am very certain that I didn't write it^^
There was a problem hiding this comment.
I think its fine to just revert back to the codelength check
core/state_transition.go
Outdated
| @@ -439,15 +441,25 @@ func (st *StateTransition) TransitionDb() (*ExecutionResult, error) { | |||
| // - reset transient storage(eip 1153) | |||
| st.state.Prepare(rules, msg.From, st.evm.Context.Coinbase, msg.To, vm.ActivePrecompiles(rules), msg.AccessList) | |||
|
|
|||
| if !contractCreation { | |||
There was a problem hiding this comment.
Why did you move this here?
There was a problem hiding this comment.
I think this is an artifact from the whole special casing when the eof initcode is invalid. I will revert it
MariusVanDerWijden
force-pushed
the
eof-opcodes
branch
from
b864b15
to
e027042
Compare
based on #30418
rework of #29518
I could not test this properly yet, as the validity of the blockchain tests depend on the prague spec. In order to test this correctly, we need to upstream 6110 and 7002
Unfortunately I couldn't preserve the authorship, I think by using the coauthored tag, people will receive their due credit once this is merged.
Co-authored-by: lightclient [email protected]
Co-authored-by: shemnon [email protected]
Co-authored-by: holiman [email protected]