From 1e5725f03b366cf094f18c68994bc1d7397a17dd Mon Sep 17 00:00:00 2001 From: Jeffrey Wilcke Date: Thu, 9 Mar 2017 09:10:54 +0100 Subject: [PATCH] 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 | 51 +++++++++++++++++++---------------------- core/vm/instructions.go | 3 +-- core/vm/interpreter.go | 25 ++++++++++++++++---- core/vm/jump_table.go | 3 +++ 4 files changed, 49 insertions(+), 33 deletions(-) diff --git a/core/vm/evm.go b/core/vm/evm.go index 9264d88659..09197e14fe 100644 --- a/core/vm/evm.go +++ b/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 -// called multiple times. +// Cancel cancels any running EVM operation. This may be called concurrently and +// 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. diff --git a/core/vm/instructions.go b/core/vm/instructions.go index 2e1959e8c0..181a04d124 100644 --- a/core/vm/instructions.go +++ b/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) { diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index b38d42ed3f..cf96200305 100644 --- a/core/vm/interpreter.go +++ b/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 -func (evm *Interpreter) Run(contract *Contract, input []byte) (ret []byte, err error) { +// Run loops and evaluates the contract's code with the given input data and returns +// 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 diff --git a/core/vm/jump_table.go b/core/vm/jump_table.go index 3e5097857d..2ffd300bd1 100644 --- a/core/vm/jump_table.go +++ b/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,