Skip to content
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.

Sign up for GitHub

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

Jump to bottom

core/vm: fill gaps in jump table with opUndefined #24031

Merged
merged 1 commit into from Dec 3, 2021
Merged

core/vm: fill gaps in jump table with opUndefined #24031

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
4 changes: 4 additions & 0 deletions core/vm/instructions.go
View file
Open in desktop
Expand Up @@ -801,6 +801,10 @@ func opRevert(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]b
return ret, ErrExecutionReverted
}

func opUndefined(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) {
return nil, &ErrInvalidOpCode{opcode: OpCode(scope.Contract.Code[*pc])}
}

func opStop(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) {
return nil, errStopToken
}
Expand Down
3 changes: 0 additions & 3 deletions core/vm/interpreter.go
View file
Open in desktop
Expand Up @@ -193,9 +193,6 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) (
// enough stack items available to perform the operation.
op = contract.GetOp(pc)
operation := in.cfg.JumpTable[op]
if operation == nil {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry to be constantly nagging about tracing haha but this is gonna do a lot of CaptureState(opUndefined...)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opUndefined still aborts with ErrInvalidOpCode so there shouldn't really be a change there?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How this is a problem?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@s1na Well, the fix is it's basically the same as for the previous one: whenever there are two outputs for the same step of execution (same pc twice in a row), then ignore the first and only care about the second one.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

opUndefined still aborts with ErrInvalidOpCode so there shouldn't really be a change there?

@axic the change is that with this PR, it will do a capturestate prior to execute, which it didn't do previously -- erroring before it reached execute.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gonna do a lot of... was an exaggeration, since there aren't many txes which actually execute undefined opcodes, I think? but yeah not saying this is a show-stopper or anything. Mainly wanted to document the change.

return nil, &ErrInvalidOpCode{opcode: op}
}
// Validate stack
if sLen := stack.len(); sLen < operation.minStack {
return nil, &ErrStackUnderflow{stackLen: sLen, required: operation.minStack}
Expand Down
11 changes: 10 additions & 1 deletion core/vm/jump_table.go
View file
Open in desktop
Expand Up @@ -200,7 +200,7 @@ func newHomesteadInstructionSet() JumpTable {
// newFrontierInstructionSet returns the frontier instructions
// that can be executed during the frontier phase.
func newFrontierInstructionSet() JumpTable {
return JumpTable{
tbl := JumpTable{
STOP: {
execute: opStop,
constantGas: 0,
Expand Down Expand Up @@ -1002,4 +1002,13 @@ func newFrontierInstructionSet() JumpTable {
maxStack: maxStack(1, 0),
},
}

// Fill all unassigned slots with opUndefined.
for i, entry := range tbl {
if entry == nil {
tbl[i] = &operation{execute: opUndefined, maxStack: maxStack(0, 0)}
}
}

return tbl
}