core/vm: Simplify error handling in interpreter loop (#6394)

Cherry-pick ethereum/go-ethereum#23952

Co-authored-by: Paweł Bylica <[email protected]>

core/vm/absint_cfg_proof_check.go

@@ -9,8 +9,6 @@ import (
 )
 
 type CfgOpSem struct {
-	reverts  bool
-	halts    bool
 	isPush   bool
 	isDup    bool
 	isSwap   bool
@@ -32,8 +30,6 @@ func NewCfgAbsSem() *CfgAbsSem {
 			continue
 		}
 		opsem := CfgOpSem{}
-		opsem.reverts = op.reverts
-		opsem.halts = op.halts
 		opsem.isPush = op.isPush
 		opsem.isDup = op.isDup
 		opsem.isSwap = op.isSwap
@@ -87,7 +83,7 @@ func resolveCheck(sem *CfgAbsSem, code []byte, st0 *astate, pc0 int) (map[int]bo
 	succs := make(map[int]bool)
 	jumps := make(map[int]bool)
 
-	if opsem == nil || opsem.halts || opsem.reverts {
+	if opsem == nil {
 		return succs, jumps, nil
 	}
 

core/vm/absint_cfg_proof_gen.go

@@ -87,7 +87,7 @@ func toProgram(code []byte) *Program {
 		op := OpCode(code[pc])
 		stmt.opcode = op
 		stmt.operation = jt[op]
-		stmt.ends = stmt.operation == nil || stmt.operation.halts || stmt.operation.reverts
+		stmt.ends = stmt.operation == nil
 		//fmt.Printf("%v %v %v", pc, stmt.opcode, stmt.operation.valid)
 
 		if op.IsPush() {

core/vm/errors.go

@@ -42,6 +42,10 @@ var (
 	ErrReturnStackExceeded      = errors.New("return stack limit reached")
 	ErrInvalidCode              = errors.New("invalid code")
 	ErrNonceUintOverflow        = errors.New("nonce uint64 overflow")
+
+	// errStopToken is an internal token indicating interpreter loop termination,
+	// never returned to outside callers.
+	errStopToken = errors.New("stop token")
 )
 
 // ErrStackUnderflow wraps an evm error when the items on the stack less

core/vm/instructions.go

@@ -582,7 +582,7 @@ func opJump(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byt
 		}
 		return nil, ErrInvalidJump
 	}
-	*pc = pos.Uint64()
+	*pc = pos.Uint64() - 1 // pc will be increased by the interpreter loop
 	return nil, nil
 }
 
@@ -605,9 +605,7 @@ func opJumpi(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]by
 			}
 			return nil, ErrInvalidJump
 		}
-		*pc = pos.Uint64()
-	} else {
-		*pc++
+		*pc = pos.Uint64() - 1 // pc will be increased by the interpreter loop
 	}
 	return nil, nil
 }
@@ -663,8 +661,10 @@ func opCreate(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]b
 	scope.Contract.Gas += returnGas
 
 	if suberr == ErrExecutionReverted {
+		interpreter.returnData = res // set REVERT data to return data buffer
 		return res, nil
 	}
+	interpreter.returnData = nil // clear dirty return data buffer
 	return nil, nil
 }
 
@@ -695,8 +695,10 @@ func opCreate2(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]
 	scope.Contract.Gas += returnGas
 
 	if suberr == ErrExecutionReverted {
+		interpreter.returnData = res // set REVERT data to return data buffer
 		return res, nil
 	}
+	interpreter.returnData = nil // clear dirty return data buffer
 	return nil, nil
 }
 
@@ -731,6 +733,7 @@ func opCall(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byt
 
 	scope.Contract.Gas += returnGas
 
+	interpreter.returnData = ret
 	return ret, nil
 }
 
@@ -764,6 +767,7 @@ func opCallCode(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([
 
 	scope.Contract.Gas += returnGas
 
+	interpreter.returnData = ret
 	return ret, nil
 }
 
@@ -793,6 +797,7 @@ func opDelegateCall(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext
 
 	scope.Contract.Gas += returnGas
 
+	interpreter.returnData = ret
 	return ret, nil
 }
 
@@ -822,27 +827,29 @@ func opStaticCall(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext)
 
 	scope.Contract.Gas += returnGas
 
+	interpreter.returnData = ret
 	return ret, nil
 }
 
 func opReturn(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) {
 	offset, size := scope.Stack.Pop(), scope.Stack.Pop()
 	ret := scope.Memory.GetPtr(offset.Uint64(), size.Uint64())
-	return ret, nil
+	return ret, errStopToken
 }
 
 func opRevert(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) {
 	offset, size := scope.Stack.Pop(), scope.Stack.Pop()
 	ret := scope.Memory.GetPtr(offset.Uint64(), size.Uint64())
-	return ret, nil
+	interpreter.returnData = ret
+	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, nil
+	return nil, errStopToken
 }
 
 func opSuicide(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]byte, error) {
@@ -855,7 +862,7 @@ func opSuicide(pc *uint64, interpreter *EVMInterpreter, scope *ScopeContext) ([]
 	}
 	interpreter.evm.IntraBlockState().AddBalance(beneficiaryAddr, balance)
 	interpreter.evm.IntraBlockState().Suicide(callerAddr)
-	return nil, nil
+	return nil, errStopToken
 }
 
 // following functions are used by the instruction jump  table

core/vm/interpreter.go

@@ -295,24 +295,18 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) (
 		}
 		// execute the operation
 		res, err = operation.execute(pc, in, callContext)
-		// if the operation clears the return data (e.g. it has returning data)
-		// set the last return to the result of the operation.
-		if operation.returns {
-			in.returnData = res
-		}
 
-		switch {
-		case err != nil:
-			return nil, err
-		case operation.reverts:
-			return res, ErrExecutionReverted
-		case operation.halts:
-			return res, nil
-		case !operation.jumps:
-			_pc++
+		if err != nil {
+			break
 		}
+		_pc++
 	}
-	return nil, nil
+
+	if err == errStopToken {
+		err = nil // clear stop token error
+	}
+
+	return res, err
 }
 
 func (vm *VM) setReadonly(outerReadonly bool) func() {

core/vm/jump_table.go

@@ -50,11 +50,7 @@ type operation struct {
 	// memorySize returns the memory size required for the operation
 	memorySize memorySizeFunc
 
-	halts   bool // indicates whether the operation should halt further execution
-	jumps   bool // indicates whether the program counter should not increment
-	writes  bool // determines whether this a state modifying operation
-	reverts bool // determines whether the operation reverts state (implicitly halts)
-	returns bool // determines whether the operations sets the return data content
+	writes bool // determines whether this a state modifying operation
 }
 
 var (
@@ -188,7 +184,6 @@ func newConstantinopleInstructionSet() JumpTable {
 		numPush:     1,
 		memorySize:  memoryCreate2,
 		writes:      true,
-		returns:     true,
 	}
 	validate(&instructionSet)
 	return instructionSet
@@ -207,7 +202,6 @@ func newByzantiumInstructionSet() JumpTable {
 		numPop:      6,
 		numPush:     1,
 		memorySize:  memoryStaticCall,
-		returns:     true,
 	}
 	instructionSet[RETURNDATASIZE] = &operation{
 		execute:     opReturnDataSize,
@@ -235,8 +229,6 @@ func newByzantiumInstructionSet() JumpTable {
 		numPop:     2,
 		numPush:    0,
 		memorySize: memoryRevert,
-		reverts:    true,
-		returns:    true,
 	}
 	validate(&instructionSet)
 	return instructionSet
@@ -278,7 +270,6 @@ func newHomesteadInstructionSet() JumpTable {
 		numPop:      6,
 		numPush:     1,
 		memorySize:  memoryDelegateCall,
-		returns:     true,
 	}
 	validate(&instructionSet)
 	return instructionSet
@@ -295,7 +286,6 @@ func newFrontierInstructionSet() JumpTable {
 			maxStack:    maxStack(0, 0),
 			numPop:      0,
 			numPush:     0,
-			halts:       true,
 		},
 		ADD: {
 			execute:     opAdd,
@@ -704,7 +694,6 @@ func newFrontierInstructionSet() JumpTable {
 			maxStack:    maxStack(1, 0),
 			numPop:      1,
 			numPush:     0,
-			jumps:       true,
 		},
 		JUMPI: {
 			execute:     opJumpi,
@@ -713,7 +702,6 @@ func newFrontierInstructionSet() JumpTable {
 			maxStack:    maxStack(2, 0),
 			numPop:      2,
 			numPush:     0,
-			jumps:       true,
 		},
 		PC: {
 			execute:     opPc,
@@ -1447,7 +1435,6 @@ func newFrontierInstructionSet() JumpTable {
 			numPush:     1,
 			memorySize:  memoryCreate,
 			writes:      true,
-			returns:     true,
 		},
 		CALL: {
 			execute:     opCall,
@@ -1458,7 +1445,6 @@ func newFrontierInstructionSet() JumpTable {
 			numPop:      7,
 			numPush:     1,
 			memorySize:  memoryCall,
-			returns:     true,
 		},
 		CALLCODE: {
 			execute:     opCallCode,
@@ -1469,7 +1455,6 @@ func newFrontierInstructionSet() JumpTable {
 			numPop:      7,
 			numPush:     1,
 			memorySize:  memoryCall,
-			returns:     true,
 		},
 		RETURN: {
 			execute:    opReturn,
@@ -1479,7 +1464,6 @@ func newFrontierInstructionSet() JumpTable {
 			numPop:     2,
 			numPush:    0,
 			memorySize: memoryReturn,
-			halts:      true,
 		},
 		SELFDESTRUCT: {
 			execute:    opSuicide,
@@ -1488,7 +1472,6 @@ func newFrontierInstructionSet() JumpTable {
 			maxStack:   maxStack(1, 0),
 			numPop:     1,
 			numPush:    0,
-			halts:      true,
 			writes:     true,
 		},
 	}