From f9fb70d2eeebfc27d3a4f8ae3ea93c67d7fb0e6b Mon Sep 17 00:00:00 2001 From: Péter Szilágyi Date: Wed, 16 Aug 2017 17:09:29 +0300 Subject: core/vm: rework reversion to work on a higher level --- core/vm/evm.go | 32 ++++++++++++++++---------------- core/vm/gas_table.go | 4 ++++ core/vm/instructions.go | 19 +++++++++++++------ core/vm/interpreter.go | 14 ++++++-------- core/vm/jump_table.go | 25 ++++++++++--------------- core/vm/memory_table.go | 4 ++++ 6 files changed, 53 insertions(+), 45 deletions(-) (limited to 'core') diff --git a/core/vm/evm.go b/core/vm/evm.go index 448acd469..8d654c666 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 a6346bd80..3b0698ce9 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 b3b7310dd..ece4d2229 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 diff --git a/core/vm/interpreter.go b/core/vm/interpreter.go index 23f930e91..0466bf085 100644 --- a/core/vm/interpreter.go +++ b/core/vm/interpreter.go @@ -209,24 +209,22 @@ func (in *Interpreter) Run(snapshot int, contract *Contract, input []byte) (ret if verifyPool { verifyIntegerPool(in.intPool) } - // checks whether the operation should revert state. - if operation.reverts { - in.evm.StateDB.RevertToSnapshot(snapshot) + // 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 2d7697e9c..f0a922912 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 ( @@ -91,10 +84,12 @@ func NewMetropolisInstructionSet() [256]operation { } instructionSet[REVERT] = operation{ execute: opRevert, - gasCost: constGasFunc(GasFastestStep), + 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 f1b671adc..bec0235bc 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) -- cgit