-
Notifications
You must be signed in to change notification settings - Fork 20.4k
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
core/vm: implement full EOF suite #26133
Conversation
@hugo-dc is working on EIP-4200 (static relative jumps) implementation: https://github.com/ipsilon/go-ethereum/tree/eip-4200. |
126aad9
to
20270f4
Compare
86bde5a
to
f888ffc
Compare
I wanted to do some cross-client fuzzing, using https://github.com/holiman/txparse/#eof-parser-eofparse as a fuzzing target. However, in order to invoke the parsing from an external package, the external caller needs an instruction set. But none of our instruction sets are public. a) making the instructionsets public, or Locally, I went with option d. diff --git a/core/vm/jump_table.go b/core/vm/jump_table.go
index 120fb56ae2..1aa3fc4703 100644
--- a/core/vm/jump_table.go
+++ b/core/vm/jump_table.go
@@ -63,6 +63,12 @@ var (
shanghaiInstructionSet = newShanghaiInstructionSet()
)
+// NewLatestInstructionSetForTesting returns the 'latest' instruction set, for use in testing.
+// This method will change over time, and cannot be relied upon to stay consistent across releases.
+func NewLatestInstructionSetForTesting() JumpTable {
+ return newShanghaiInstructionSet()
+}
+
// JumpTable contains the EVM opcodes supported at a given fork.
type JumpTable [256]*operation
|
Now that the masterbranch is go1.19 and up, we could also import this test into this PR: https://github.com/holiman/txparse/blob/main/eofparse/eofparse_test.go |
*pc = 0 | ||
*pc -= 1 // hacks xD |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the goal of this hack? Why do you need to set the pc like this?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, so you want it to be 0
after the loop does +1
later on
Co-authored-by: lightclient@protonmail.com <lightclient@protonmail.com> Co-authored-by: Martin Holst Swende <martin@swende.se>
) | ||
|
||
var ( | ||
ErrUndefinedInstruction = errors.New("undefined instrustion") |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
typo: instrustion
var ( | ||
code = scope.Contract.CodeAt(scope.CodeSection) | ||
idx = binary.BigEndian.Uint16(code[*pc+1:]) | ||
typ = scope.Contract.Container.Types[scope.CodeSection] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It is getting the type of the current CodeSection, not the Code section to be called (idx
).
typ = scope.Contract.Container.Types[scope.CodeSection] | |
typ = scope.Contract.Container.Types[idx] |
@@ -262,6 +262,25 @@ func Transition(ctx *cli.Context) error { | |||
return NewError(ErrorConfig, errors.New("EIP-1559 config but missing 'currentBaseFee' in env section")) | |||
} | |||
} | |||
// Sanity check pre-allocated EOF code to not panic in state transition. | |||
if chainConfig.IsShanghai(big.NewInt(int64(prestate.Env.Number))) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think t8n tool should also check that before the EOF fork, code in prestate doesn't start with EF00
if want, have := int(metadata[arg].Input), height; want > have { | ||
return 0, fmt.Errorf("%w: at pos %d", ErrStackUnderflow{stackLen: have, required: want}, pos) | ||
} | ||
if have, limit := int(metadata[arg].Output)+height, int(params.StackLimit); have > limit { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this should check
if have, limit := int(metadata[arg].Output)+height, int(params.StackLimit); have > limit { | |
if have, limit := int(metadata[arg].MaxStackHeight)+height, int(params.StackLimit); have > limit { |
This PR is really stale at this point, I think when we need to implement EOF we can use this for inspiration, but will need to do some significant refactoring. |
(wip)
This PR builds on the Ipsilon team's (@gumb0, @chfast, and @axic) earlier work implementing EIP-3540 and EIP-3670. I'm extending it to support the "full" EOF suite which is: