core/vm: removed err checks from EVM and added snapshot to interpreter

The REVERT op codes reverts the state by returning an error from the
instruction. The instruction was checked during in the EVM. This error
would then determine whether any gas should be consumed or not. If the
error was an errRevert, no gas was further reduced and only state was
reverted.

This solution is error prone as you'll need an err check in multiple
locations and removes the no-err check requirement of the evm. This has
been solved by passing the snapshot to the interpreter and letting the
interpreter do the state reverting if the operation so requires.

core/vm/evm.go

@@ -34,7 +34,7 @@ type (
 )
 
 // run runs the given contract and takes care of running precompiles with a fallback to the byte code interpreter.
-func run(evm *EVM, contract *Contract, input []byte) ([]byte, error) {
+func run(evm *EVM, snapshot int, contract *Contract, input []byte) ([]byte, error) {
 	if contract.CodeAddr != nil {
 		precompiledContracts := PrecompiledContracts
 		if evm.ChainConfig().IsMetropolis(evm.BlockNumber) {
@@ -46,10 +46,11 @@ func run(evm *EVM, contract *Contract, input []byte) ([]byte, error) {
 		}
 	}
 
-	return evm.interpreter.Run(contract, input)
+	return evm.interpreter.Run(snapshot, contract, input)
 }
 
-// Context provides the EVM with auxiliary information. Once provided it shouldn't be modified.
+// Context provides the EVM with auxiliary information. Once provided
+// it shouldn't be modified.
 type Context struct {
 	// CanTransfer returns whether the account contains
 	// sufficient ether to transfer the value
@@ -71,7 +72,13 @@ type Context struct {
 	Difficulty  *big.Int       // Provides information for DIFFICULTY
 }
 
-// EVM provides information about external sources for the EVM
+// EVM is the Ethereum Virtual Machine base object and provides
+// the necessary tools to run a contract on the given state with
+// the provided context. It should be noted that any error
+// generated through any of the calls should be considered a
+// revert-state-and-consume-all-gas operation, no checks on
+// specific errors should ever be performed. The interpreter makes
+// sure that any errors generated are to be considered faulty code.
 //
 // The EVM should never be reused and is not thread safe.
 type EVM struct {
@@ -95,7 +102,8 @@ type EVM struct {
 	abort int32
 }
 
-// NewEVM retutrns a new EVM evmironment.
+// NewEVM retutrns a new EVM evmironment. The returned EVM is not thread safe
+// and should only ever be used *once*.
 func NewEVM(ctx Context, statedb StateDB, chainConfig *params.ChainConfig, vmConfig Config) *EVM {
 	evm := &EVM{
 		Context:     ctx,
@@ -108,8 +116,8 @@ func NewEVM(ctx Context, statedb StateDB, chainConfig *params.ChainConfig, vmCon
 	return evm
 }
 
-// Cancel cancels any running EVM operation. This may be called concurrently and it's safe to be
+// Cancel cancels any running EVM operation. This may be called concurrently and
-// called multiple times.
+// it's safe to be called multiple times.
 func (evm *EVM) Cancel() {
 	atomic.StoreInt32(&evm.abort, 1)
 }
@@ -147,7 +155,7 @@ func (evm *EVM) StaticCall(caller ContractRef, addr common.Address, input []byte
 	contract := NewContract(caller, to, new(big.Int), gas)
 	contract.SetCallCode(&addr, evm.StateDB.GetCodeHash(addr), evm.StateDB.GetCode(addr))
 
-	ret, err = evm.interpreter.Run(contract, input)
+	ret, err = evm.interpreter.Run(snapshot, contract, input)
 	// When an error was returned by the EVM or when setting the creation code
 	// above we revert to the snapshot and consume any gas remaining. Additionally
 	// when we're in homestead this also counts for code storage gas errors.
@@ -195,14 +203,12 @@ func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas
 	contract := NewContract(caller, to, value, gas)
 	contract.SetCallCode(&addr, evm.StateDB.GetCodeHash(addr), evm.StateDB.GetCode(addr))
 
-	ret, err = run(evm, contract, input)
+	ret, err = run(evm, snapshot, contract, input)
 	// When an error was returned by the EVM or when setting the creation code
 	// above we revert to the snapshot and consume any gas remaining. Additionally
 	// when we're in homestead this also counts for code storage gas errors.
 	if err != nil {
-		if err != errRevert {
+		contract.UseGas(contract.Gas)
-			contract.UseGas(contract.Gas)
-		}
 		evm.StateDB.RevertToSnapshot(snapshot)
 	}
 	return ret, contract.Gas, err
@@ -237,12 +243,9 @@ func (evm *EVM) CallCode(caller ContractRef, addr common.Address, input []byte,
 	contract := NewContract(caller, to, value, gas)
 	contract.SetCallCode(&addr, evm.StateDB.GetCodeHash(addr), evm.StateDB.GetCode(addr))
 
-	ret, err = run(evm, contract, input)
+	ret, err = run(evm, snapshot, contract, input)
 	if err != nil {
-		if err != errRevert {
+		contract.UseGas(contract.Gas)
-			contract.UseGas(contract.Gas)
-		}
-
 		evm.StateDB.RevertToSnapshot(snapshot)
 	}
 
@@ -274,12 +277,9 @@ func (evm *EVM) DelegateCall(caller ContractRef, addr common.Address, input []by
 	contract := NewContract(caller, to, nil, gas).AsDelegate()
 	contract.SetCallCode(&addr, evm.StateDB.GetCodeHash(addr), evm.StateDB.GetCode(addr))
 
-	ret, err = run(evm, contract, input)
+	ret, err = run(evm, snapshot, contract, input)
 	if err != nil {
-		if err != errRevert {
+		contract.UseGas(contract.Gas)
-			contract.UseGas(contract.Gas)
-		}
-
 		evm.StateDB.RevertToSnapshot(snapshot)
 	}
 
@@ -319,7 +319,7 @@ func (evm *EVM) Create(caller ContractRef, code []byte, gas uint64, value *big.I
 	contract := NewContract(caller, AccountRef(contractAddr), value, gas)
 	contract.SetCallCode(&contractAddr, crypto.Keccak256Hash(code), code)
 
-	ret, err = run(evm, contract, nil)
+	ret, err = run(evm, snapshot, contract, nil)
 	// check whether the max code size has been exceeded
 	maxCodeSizeExceeded := len(ret) > params.MaxCodeSize
 	// if the contract creation ran successfully and no errors were returned
@@ -340,10 +340,7 @@ func (evm *EVM) Create(caller ContractRef, code []byte, gas uint64, value *big.I
 	// when we're in homestead this also counts for code storage gas errors.
 	if maxCodeSizeExceeded ||
 		(err != nil && (evm.ChainConfig().IsHomestead(evm.BlockNumber) || err != ErrCodeStoreOutOfGas)) {
-		if err != errRevert {
+		contract.UseGas(contract.Gas)
-			contract.UseGas(contract.Gas)
-		}
-
 		evm.StateDB.RevertToSnapshot(snapshot)
 	}
 	// If the vm returned with an error the return value should be set to nil.

core/vm/instructions.go

@@ -31,7 +31,6 @@ import (
 var (
 	bigZero            = new(big.Int)
 	errWriteProtection = errors.New("evm write protection")
-	errRevert          = errors.New("standard throw")
 )
 
 func opAdd(pc *uint64, evm *EVM, contract *Contract, memory *Memory, stack *Stack) ([]byte, error) {
@@ -719,7 +718,7 @@ func opRevert(pc *uint64, evm *EVM, contract *Contract, memory *Memory, stack *S
 	ret := memory.GetPtr(offset.Int64(), size.Int64())
 
 	evm.interpreter.intPool.put(offset, size)
-	return ret, errRevert
+	return ret, nil
 }
 
 func opStop(pc *uint64, evm *EVM, contract *Contract, memory *Memory, stack *Stack) ([]byte, error) {

core/vm/interpreter.go

@@ -18,6 +18,7 @@ package vm
 
 import (
 	"fmt"
+	"math/big"
 	"sync/atomic"
 	"time"
 
@@ -81,8 +82,13 @@ func NewInterpreter(env *EVM, cfg Config) *Interpreter {
 	}
 }
 
-// Run loops and evaluates the contract's code with the given input data
+// Run loops and evaluates the contract's code with the given input data and returns
-func (evm *Interpreter) Run(contract *Contract, input []byte) (ret []byte, err error) {
+// the return byte-slice and an error if one occured.
+//
+// It's important to note that any errors returned by the interpreter should be
+// considered a revert-and-consume-all-gas operation. No error specific checks
+// should be handled to reduce complexity and errors further down the EVM.
+func (evm *Interpreter) Run(snapshot int, contract *Contract, input []byte) (ret []byte, err error) {
 	evm.env.depth++
 	defer func() { evm.env.depth-- }()
 
@@ -101,7 +107,8 @@ func (evm *Interpreter) Run(contract *Contract, input []byte) (ret []byte, err e
 		mem   = NewMemory() // bound memory
 		stack = newstack()  // local stack
 		// For optimisation reason we're using uint64 as the program counter.
-		// It's theoretically possible to go above 2^64. The YP defines the PC to be uint256. Practically much less so feasible.
+		// It's theoretically possible to go above 2^64. The YP defines the PC
+		// to be uint256. Practically much less so feasible.
 		pc   = uint64(0) // program counter
 		cost uint64
 	)
@@ -112,6 +119,8 @@ func (evm *Interpreter) Run(contract *Contract, input []byte) (ret []byte, err e
 		if err != nil && evm.cfg.Debug {
 			// XXX For debugging
 			//fmt.Printf("%04d: %8v    cost = %-8d stack = %-8d ERR = %v\n", pc, op, cost, stack.len(), err)
+			// TODO update the tracer to accept uint64 instead of *big.Int
+			g, c := new(big.Int).SetUint64(contract.Gas), new(big.Int).SetUint64(cost)
 			evm.cfg.Tracer.CaptureState(evm.env, pc, op, contract.Gas, cost, mem, stack, contract, evm.env.depth, err)
 		}
 	}()
@@ -179,7 +188,9 @@ func (evm *Interpreter) Run(contract *Contract, input []byte) (ret []byte, err e
 		}
 
 		if evm.cfg.Debug {
-			evm.cfg.Tracer.CaptureState(evm.env, pc, op, contract.Gas, cost, mem, stack, contract, evm.env.depth, err)
+			// TODO update the tracer to accept uint64 instead of *big.Int
+			g, c := new(big.Int).SetUint64(contract.Gas), new(big.Int).SetUint64(cost)
+			evm.cfg.Tracer.CaptureState(evm.env, pc, op, g, c, mem, stack, contract, evm.env.depth, err)
 		}
 		// XXX For debugging
 		//fmt.Printf("%04d: %8v    cost = %-8d stack = %-8d\n", pc, op, cost, stack.len())
@@ -191,6 +202,12 @@ func (evm *Interpreter) Run(contract *Contract, input []byte) (ret []byte, err e
 		if verifyPool {
 			verifyIntegerPool(evm.intPool)
 		}
+
+		// checks whether the operation should revert state.
+		if operation.reverts {
+			evm.env.StateDB.RevertToSnapshot(snapshot)
+		}
+
 		switch {
 		case err != nil:
 			return nil, err

core/vm/jump_table.go

@@ -51,6 +51,8 @@ type operation struct {
 	writes bool
 	// valid is used to check whether the retrieved operation is valid and known
 	valid bool
+	// reverts determined whether the operation reverts state
+	reverts bool
 }
 
 var defaultJumpTable = NewJumpTable()
@@ -400,6 +402,7 @@ func NewJumpTable() [256]operation {
 			gasCost:       constGasFunc(GasFastestStep),
 			validateStack: makeStackFunc(2, 0),
 			valid:         true,
+			reverts:       true,
 		},
 		JUMPDEST: {
 			execute:       opJumpdest,