core/vm: rework reversion to work on a higher level

6 files changed, 53 insertions, 45 deletions

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)