diff --git a/core/vm/evm.go b/core/vm/evm.go index 448acd469d..8d654c666e 100644 --- a/core/vm/evm.go +++ b/core/vm/evm.go @@ -168,8 +168,10 @@ func (evm *EVM) Call(caller ContractRef, addr common.Address, input []byte, gas // 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 { - contract.UseGas(contract.Gas) evm.StateDB.RevertToSnapshot(snapshot) + if err != errExecutionReverted { + contract.UseGas(contract.Gas) + } } return ret, contract.Gas, err } @@ -207,10 +209,11 @@ func (evm *EVM) CallCode(caller ContractRef, addr common.Address, input []byte, ret, err = run(evm, snapshot, contract, input) if err != nil { - contract.UseGas(contract.Gas) evm.StateDB.RevertToSnapshot(snapshot) + if err != errExecutionReverted { + contract.UseGas(contract.Gas) + } } - return ret, contract.Gas, err } @@ -239,10 +242,11 @@ func (evm *EVM) DelegateCall(caller ContractRef, addr common.Address, input []by ret, err = run(evm, snapshot, contract, input) if err != nil { - contract.UseGas(contract.Gas) evm.StateDB.RevertToSnapshot(snapshot) + if err != errExecutionReverted { + contract.UseGas(contract.Gas) + } } - return ret, contract.Gas, err } @@ -281,8 +285,10 @@ func (evm *EVM) StaticCall(caller ContractRef, addr common.Address, input []byte // when we're in Homestead this also counts for code storage gas errors. ret, err = run(evm, snapshot, contract, input) if err != nil { - contract.UseGas(contract.Gas) evm.StateDB.RevertToSnapshot(snapshot) + if err != errExecutionReverted { + contract.UseGas(contract.Gas) + } } return ret, contract.Gas, err } @@ -339,18 +345,12 @@ func (evm *EVM) Create(caller ContractRef, code []byte, gas uint64, value *big.I // 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 maxCodeSizeExceeded || - (err != nil && (evm.ChainConfig().IsHomestead(evm.BlockNumber) || err != ErrCodeStoreOutOfGas)) { - contract.UseGas(contract.Gas) + if maxCodeSizeExceeded || (err != nil && (evm.ChainConfig().IsHomestead(evm.BlockNumber) || err != ErrCodeStoreOutOfGas)) { evm.StateDB.RevertToSnapshot(snapshot) + if err != errExecutionReverted { + contract.UseGas(contract.Gas) + } } - // If the vm returned with an error the return value should be set to nil. - // This isn't consensus critical but merely to for behaviour reasons such as - // tests, RPC calls, etc. - if err != nil { - ret = nil - } - return ret, contractAddr, contract.Gas, err } diff --git a/core/vm/gas_table.go b/core/vm/gas_table.go index a6346bd806..3b0698ce9e 100644 --- a/core/vm/gas_table.go +++ b/core/vm/gas_table.go @@ -396,6 +396,10 @@ func gasReturn(gt params.GasTable, evm *EVM, contract *Contract, stack *Stack, m return memoryGasCost(mem, memorySize) } +func gasRevert(gt params.GasTable, evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize uint64) (uint64, error) { + return memoryGasCost(mem, memorySize) +} + func gasSuicide(gt params.GasTable, evm *EVM, contract *Contract, stack *Stack, mem *Memory, memorySize uint64) (uint64, error) { var gas uint64 // EIP150 homestead gas reprice fork: diff --git a/core/vm/instructions.go b/core/vm/instructions.go index 4d6197912f..ece4d2229a 100644 --- a/core/vm/instructions.go +++ b/core/vm/instructions.go @@ -32,6 +32,7 @@ var ( bigZero = new(big.Int) errWriteProtection = errors.New("evm: write protection") errReturnDataOutOfBounds = errors.New("evm: return data out of bounds") + errExecutionReverted = errors.New("evm: execution reverted") ) func opAdd(pc *uint64, evm *EVM, contract *Contract, memory *Memory, stack *Stack) ([]byte, error) { @@ -579,7 +580,7 @@ func opCreate(pc *uint64, evm *EVM, contract *Contract, memory *Memory, stack *S } contract.UseGas(gas) - _, addr, returnGas, suberr := evm.Create(contract, input, gas, value) + res, addr, returnGas, suberr := evm.Create(contract, input, gas, value) // Push item on the stack based on the returned error. If the ruleset is // homestead we must check for CodeStoreOutOfGasError (homestead only // rule) and treat as an error, if the ruleset is frontier we must @@ -592,9 +593,11 @@ func opCreate(pc *uint64, evm *EVM, contract *Contract, memory *Memory, stack *S stack.push(addr.Big()) } contract.Gas += returnGas - evm.interpreter.intPool.put(value, offset, size) + if suberr == errExecutionReverted { + return res, nil + } return nil, nil } @@ -622,7 +625,8 @@ func opCall(pc *uint64, evm *EVM, contract *Contract, memory *Memory, stack *Sta stack.push(new(big.Int)) } else { stack.push(big.NewInt(1)) - + } + if err == nil || err == errExecutionReverted { memory.Set(retOffset.Uint64(), retSize.Uint64(), ret) } contract.Gas += returnGas @@ -653,10 +657,10 @@ func opCallCode(pc *uint64, evm *EVM, contract *Contract, memory *Memory, stack ret, returnGas, err := evm.CallCode(contract, address, args, gas, value) if err != nil { stack.push(new(big.Int)) - } else { stack.push(big.NewInt(1)) - + } + if err == nil || err == errExecutionReverted { memory.Set(retOffset.Uint64(), retSize.Uint64(), ret) } contract.Gas += returnGas @@ -676,6 +680,8 @@ func opDelegateCall(pc *uint64, evm *EVM, contract *Contract, memory *Memory, st stack.push(new(big.Int)) } else { stack.push(big.NewInt(1)) + } + if err == nil || err == errExecutionReverted { memory.Set(outOffset.Uint64(), outSize.Uint64(), ret) } contract.Gas += returnGas @@ -704,7 +710,8 @@ func opStaticCall(pc *uint64, evm *EVM, contract *Contract, memory *Memory, stac stack.push(new(big.Int)) } else { stack.push(big.NewInt(1)) - + } + if err == nil || err == errExecutionReverted { memory.Set(retOffset.Uint64(), retSize.Uint64(), ret) } contract.Gas += returnGas @@ -718,7 +725,14 @@ func opReturn(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, nil +} + +func opRevert(pc *uint64, evm *EVM, contract *Contract, memory *Memory, stack *Stack) ([]byte, error) { + offset, size := stack.pop(), stack.pop() + ret := memory.GetPtr(offset.Int64(), size.Int64()) + evm.interpreter.intPool.put(offset, size) return ret, nil } @@ -731,7 +745,6 @@ func opSuicide(pc *uint64, evm *EVM, contract *Contract, memory *Memory, stack * evm.StateDB.AddBalance(common.BigToAddress(stack.pop()), balance) evm.StateDB.Suicide(contract.Address()) - return nil, nil } diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index 954839f2e3..0466bf085c 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -209,20 +209,22 @@ func (in *Interpreter) Run(snapshot int, contract *Contract, input []byte) (ret if verifyPool { verifyIntegerPool(in.intPool) } + // 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 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 - } } return nil, nil } diff --git a/core/vm/jump_table.go b/core/vm/jump_table.go index 2d238f7a16..f0a922912f 100644 --- a/core/vm/jump_table.go +++ b/core/vm/jump_table.go @@ -41,20 +41,13 @@ type operation struct { validateStack stackValidationFunc // memorySize returns the memory size required for the operation memorySize memorySizeFunc - // halts indicates whether the operation shoult halt further execution - // and return - halts bool - // jumps indicates whether operation made a jump. This prevents the program - // counter from further incrementing. - jumps bool - // writes determines whether this a state modifying operation - 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 - // returns determines whether the opertions sets the return data - returns bool + + halts bool // indicates whether the operation shoult halt further execution + jumps bool // indicates whether the program counter should not increment + writes bool // determines whether this a state modifying operation + valid bool // indication whether the retrieved operation is valid and known + reverts bool // determines whether the operation reverts state (implicitly halts) + returns bool // determines whether the opertions sets the return data content } var ( @@ -89,6 +82,15 @@ func NewMetropolisInstructionSet() [256]operation { memorySize: memoryReturnDataCopy, valid: true, } + instructionSet[REVERT] = operation{ + execute: opRevert, + gasCost: gasRevert, + validateStack: makeStackFunc(2, 0), + memorySize: memoryRevert, + valid: true, + reverts: true, + returns: true, + } return instructionSet } diff --git a/core/vm/memory_table.go b/core/vm/memory_table.go index f1b671adcf..bec0235bcc 100644 --- a/core/vm/memory_table.go +++ b/core/vm/memory_table.go @@ -89,6 +89,10 @@ func memoryReturn(stack *Stack) *big.Int { return calcMemSize(stack.Back(0), stack.Back(1)) } +func memoryRevert(stack *Stack) *big.Int { + return calcMemSize(stack.Back(0), stack.Back(1)) +} + func memoryLog(stack *Stack) *big.Int { mSize, mStart := stack.Back(1), stack.Back(0) return calcMemSize(mStart, mSize) diff --git a/core/vm/opcodes.go b/core/vm/opcodes.go index be87cae18b..0c65507355 100644 --- a/core/vm/opcodes.go +++ b/core/vm/opcodes.go @@ -204,6 +204,7 @@ const ( DELEGATECALL STATICCALL = 0xfa + REVERT = 0xfd SELFDESTRUCT = 0xff ) @@ -360,6 +361,7 @@ var opCodeToString = map[OpCode]string{ CALLCODE: "CALLCODE", DELEGATECALL: "DELEGATECALL", STATICCALL: "STATICCALL", + REVERT: "REVERT", SELFDESTRUCT: "SELFDESTRUCT", PUSH: "PUSH", @@ -509,6 +511,7 @@ var stringToOp = map[string]OpCode{ "CALL": CALL, "RETURN": RETURN, "CALLCODE": CALLCODE, + "REVERT": REVERT, "SELFDESTRUCT": SELFDESTRUCT, } diff --git a/tests/state_test.go b/tests/state_test.go index ab6dc423f9..7e5516e7e9 100644 --- a/tests/state_test.go +++ b/tests/state_test.go @@ -40,6 +40,9 @@ func TestState(t *testing.T) { st.fails(`^stRevertTest/RevertDepthCreateAddressCollision\.json/EIP15[08]/[67]`, "bug in test") st.fails(`^stRevertTest/RevertPrecompiledTouch\.json/EIP158`, "bug in test") st.fails(`^stRevertTest/RevertPrefoundEmptyOOG\.json/EIP158`, "bug in test") + st.fails(`^stRevertTest/RevertDepthCreateAddressCollision\.json/Byzantium/[67]`, "bug in test") + st.fails(`^stRevertTest/RevertPrecompiledTouch\.json/Byzantium`, "bug in test") + st.fails(`^stRevertTest/RevertPrefoundEmptyOOG\.json/Byzantium`, "bug in test") st.walk(t, stateTestDir, func(t *testing.T, name string, test *StateTest) { for _, subtest := range test.Subtests() {