all: address comments

accounts/abi/bind/backends/simulated.go

@@ -352,7 +352,7 @@ func (b *SimulatedBackend) CallContract(ctx context.Context, call ethereum.CallM
 	if err != nil {
 		return nil, err
 	}
-	return res.Result, nil
+	return res.Return(), nil
 }
 
 // PendingCallContract executes a contract call on the pending state.
@@ -365,7 +365,7 @@ func (b *SimulatedBackend) PendingCallContract(ctx context.Context, call ethereu
 	if err != nil {
 		return nil, err
 	}
-	return res.Result, nil
+	return res.Return(), nil
 }
 
 // PendingNonceAt implements PendingStateReader.PendingNonceAt, retrieving
@@ -411,7 +411,7 @@ func (b *SimulatedBackend) EstimateGas(ctx context.Context, call ethereum.CallMs
 		b.pendingState.RevertToSnapshot(snapshot)
 
 		if err != nil {
-			if err == core.ErrInsufficientIntrinsicGas {
+			if err == core.ErrIntrinsicGas {
 				return true, nil, nil // Special case, raise gas limit
 			}
 			return true, nil, err // Bail out
@@ -424,8 +424,8 @@ func (b *SimulatedBackend) EstimateGas(ctx context.Context, call ethereum.CallMs
 		failed, _, err := executable(mid)
 
 		// If the error is not nil(consensus error), it means the provided message
-		// call or transaction will never be accpeted no matter how many gas assigened.
-		// Return the error directly, don't struggle any more
+		// call or transaction will never be accepted no matter how much gas it is
+		// assigned. Return the error directly, don't struggle any more
 		if err != nil {
 			return 0, err
 		}
@@ -445,8 +445,8 @@ func (b *SimulatedBackend) EstimateGas(ctx context.Context, call ethereum.CallMs
 			if result != nil {
 				if result.Err != vm.ErrOutOfGas {
 					errMsg := fmt.Sprintf("always failing transaction (%v)", result.Err)
-					if len(result.RevertReason) > 0 {
-						errMsg += fmt.Sprintf(" (0x%x)", result.RevertReason)
+					if len(result.Revert()) > 0 {
+						errMsg += fmt.Sprintf(" (0x%x)", result.Revert())
 					}
 					return 0, errors.New(errMsg)
 				}

core/error.go

@@ -29,8 +29,13 @@ var (
 	ErrNoGenesis = errors.New("genesis not found in chain")
 )
 
-// State transition consensus errors, any of them encountered during
-// the block processing can lead to consensus issue.
+// List of evm-call-message pre-checking errors. All state transtion messages will
+// be pre-checked before execution. If any invalidation detected, the corresponding
+// error should be returned which is defined here.
+//
+// - If the pre-checking happens in the miner, then the transaction won't be packed.
+// - If the pre-checking happens in the block processing procedure, then a "BAD BLOCk"
+// error should be emitted.
 var (
 	// ErrNonceTooLow is returned if the nonce of a transaction is lower than the
 	// one present in the local chain.
@@ -44,18 +49,18 @@ var (
 	// by a transaction is higher than what's left in the block.
 	ErrGasLimitReached = errors.New("gas limit reached")
 
-	// ErrInsufficientBalanceForTransfer is returned if the transaction sender doesn't
-	// have enough balance for transfer(topmost call only).
-	ErrInsufficientBalanceForTransfer = errors.New("insufficient balance for transfer")
+	// ErrInsufficientFundsForTransfer is returned if the transaction sender doesn't
+	// have enough funds for transfer(topmost call only).
+	ErrInsufficientFundsForTransfer = errors.New("insufficient funds for transfer")
 
-	// ErrInsufficientBalanceForFee is returned if transaction sender doesn't have
-	// enough balance to cover transaction fee.
-	ErrInsufficientBalanceForFee = errors.New("insufficient balance to pay fee")
+	// ErrInsufficientFunds is returned if the total cost of executing a transaction
+	// is higher than the balance of the user's account.
+	ErrInsufficientFunds = errors.New("insufficient funds for gas * price + value")
 
-	// ErrGasOverflow is returned when calculating gas usage.
-	ErrGasOverflow = errors.New("gas overflow")
+	// ErrGasUintOverflow is returned when calculating gas usage.
+	ErrGasUintOverflow = errors.New("gas uint64 overflow")
 
-	// ErrInsufficientIntrinsicGas is returned when the gas limit speicified in transaction
-	// is not enought to cover intrinsic gas usage.
-	ErrInsufficientIntrinsicGas = errors.New("insufficient intrinsic gas")
+	// ErrIntrinsicGas is returned if the transaction is specified to use less gas
+	// than required to start the invocation.
+	ErrIntrinsicGas = errors.New("intrinsic gas too low")
 )

core/state_transition.go

@@ -68,13 +68,12 @@ type Message interface {
 	Data() []byte
 }
 
-// ExecutionResult includes all output after executing given evm message
-// no matter the execution itself is successful or not.
+// ExecutionResult includes all output after executing given evm
+// message no matter the execution itself is successful or not.
 type ExecutionResult struct {
-	UsedGas      uint64 // Total used gas but include the refunded gas
-	Err          error  // Any error encountered during the exection(listed in core/vm/errors.go)
-	Result       []byte // Returned value of the calling function
-	RevertReason []byte // Reason to perform revert thrown by solidity code
+	UsedGas    uint64 // Total used gas but include the refunded gas
+	Err        error  // Any error encountered during the exection(listed in core/vm/errors.go)
+	ReturnData []byte // Returned data from evm(function result or data supplied with revert opcode)
 }
 
 // Unwrap returns the internal evm error which allows us for further
@@ -86,6 +85,24 @@ func (result *ExecutionResult) Unwrap() error {
 // Failed returns the indicator whether the execution is successful or not
 func (result *ExecutionResult) Failed() bool { return result.Err != nil }
 
+// Return is a helper function to help caller distinguish between revert reason
+// and function return. Return returns the data after execution if no error occurs.
+func (result *ExecutionResult) Return() []byte {
+	if result.Err != nil {
+		return nil
+	}
+	return common.CopyBytes(result.ReturnData)
+}
+
+// Revert returns the concrete revert reason if the execution is aborted by `REVERT`
+// opcode. Note the reason can be nil if no data supplied with revert opcode.
+func (result *ExecutionResult) Revert() []byte {
+	if result.Err != vm.ErrExecutionReverted {
+		return nil
+	}
+	return common.CopyBytes(result.ReturnData)
+}
+
 // IntrinsicGas computes the 'intrinsic gas' for a message with the given data.
 func IntrinsicGas(data []byte, contractCreation, isHomestead bool, isEIP2028 bool) (uint64, error) {
 	// Set the starting gas for the raw transaction
@@ -110,13 +127,13 @@ func IntrinsicGas(data []byte, contractCreation, isHomestead bool, isEIP2028 boo
 			nonZeroGas = params.TxDataNonZeroGasEIP2028
 		}
 		if (math.MaxUint64-gas)/nonZeroGas < nz {
-			return 0, ErrGasOverflow
+			return 0, ErrGasUintOverflow
 		}
 		gas += nz * nonZeroGas
 
 		z := uint64(len(data)) - nz
 		if (math.MaxUint64-gas)/params.TxDataZeroGas < z {
-			return 0, ErrGasOverflow
+			return 0, ErrGasUintOverflow
 		}
 		gas += z * params.TxDataZeroGas
 	}
@@ -158,7 +175,7 @@ func (st *StateTransition) to() common.Address {
 func (st *StateTransition) buyGas() error {
 	mgval := new(big.Int).Mul(new(big.Int).SetUint64(st.msg.Gas()), st.gasPrice)
 	if st.state.GetBalance(st.msg.From()).Cmp(mgval) < 0 {
-		return ErrInsufficientBalanceForFee
+		return ErrInsufficientFunds
 	}
 	if err := st.gp.SubGas(st.msg.Gas()); err != nil {
 		return err
@@ -188,10 +205,8 @@ func (st *StateTransition) preCheck() error {
 //
 // - used gas:
 //      total gas used (including gas being refunded)
-// - execution result:
-//      the return value of the calling function
-// - revert reason:
-//      reason to perform revert thrown by solidity code
+// - returndata:
+//      the returned data from evm
 // - concrete execution error:
 //      various **EVM** error which aborts the execution,
 //      e.g. ErrOutOfGas, ErrExecutionReverted
@@ -225,13 +240,13 @@ func (st *StateTransition) TransitionDb() (*ExecutionResult, error) {
 		return nil, err
 	}
 	if st.gas < gas {
-		return nil, ErrInsufficientIntrinsicGas
+		return nil, ErrIntrinsicGas
 	}
 	st.gas -= gas
 
 	// Check clause 6
 	if msg.Value().Sign() > 0 && !st.evm.CanTransfer(st.state, msg.From(), msg.Value()) {
-		return nil, ErrInsufficientBalanceForTransfer
+		return nil, ErrInsufficientFundsForTransfer
 	}
 	var (
 		ret   []byte
@@ -247,17 +262,10 @@ func (st *StateTransition) TransitionDb() (*ExecutionResult, error) {
 	st.refundGas()
 	st.state.AddBalance(st.evm.Coinbase, new(big.Int).Mul(new(big.Int).SetUint64(st.gasUsed()), st.gasPrice))
 
-	var revert, result []byte
-	if vmerr == vm.ErrExecutionReverted {
-		revert = ret // Revert reason will be returned in ret iif the vmerr is ErrExecutionReverted
-	} else {
-		result = ret // Otherwise the ret represents the execution result, may nil
-	}
 	return &ExecutionResult{
-		UsedGas:      st.gasUsed(),
-		Err:          vmerr,
-		Result:       result,
-		RevertReason: revert,
+		UsedGas:    st.gasUsed(),
+		Err:        vmerr,
+		ReturnData: ret,
 	}, nil
 }
 

core/tx_pool.go

@@ -67,14 +67,6 @@ var (
 	// with a different one without the required price bump.
 	ErrReplaceUnderpriced = errors.New("replacement transaction underpriced")
 
-	// ErrInsufficientFunds is returned if the total cost of executing a transaction
-	// is higher than the balance of the user's account.
-	ErrInsufficientFunds = errors.New("insufficient funds for gas * price + value")
-
-	// ErrIntrinsicGas is returned if the transaction is specified to use less gas
-	// than required to start the invocation.
-	ErrIntrinsicGas = errors.New("intrinsic gas too low")
-
 	// ErrGasLimit is returned if a transaction's requested gas limit exceeds the
 	// maximum allowance of the current block.
 	ErrGasLimit = errors.New("exceeds block gas limit")

core/vm/errors.go

@@ -50,17 +50,17 @@ func (e *ErrStackUnderflow) Error() string {
 // ErrStackOverflow wraps an evm error when the items on the stack exceeds
 // the maximum allowance.
 type ErrStackOverflow struct {
-	stackLen  int
-	allowance int
+	stackLen int
+	limit    int
 }
 
 func (e *ErrStackOverflow) Error() string {
-	return fmt.Sprintf("stack limit reached %d (%d)", e.stackLen, e.allowance)
+	return fmt.Sprintf("stack limit reached %d (%d)", e.stackLen, e.limit)
 }
 
 // ErrInvalidOpCode wraps an evm error when an invalid opcode is encountered.
 type ErrInvalidOpCode struct {
-	opcode int
+	opcode OpCode
 }
 
-func (e *ErrInvalidOpCode) Error() string { return fmt.Sprintf("invalid opcode 0x%x", e.opcode) }
+func (e *ErrInvalidOpCode) Error() string { return fmt.Sprintf("invalid opcode 0x%x", byte(e.opcode)) }

core/vm/evm.go

@@ -269,8 +269,8 @@ func (evm *EVM) CallCode(caller ContractRef, addr common.Address, input []byte,
 	}
 	// Fail if we're trying to transfer more than the available balance
 	// Note although it's noop to transfer X ether to caller itself. But
-	// if caller doesn't have enough balance, it can lead to an evm error.
-	// So the check here is necessary.
+	// if caller doesn't have enough balance, it would be an error to allow
+	// over-charging itself. So the check here is necessary.
 	if !evm.Context.CanTransfer(evm.StateDB, caller.Address(), value) {
 		return nil, gas, ErrInsufficientBalance
 	}

core/vm/interpreter.go

@@ -222,13 +222,13 @@ func (in *EVMInterpreter) Run(contract *Contract, input []byte, readOnly bool) (
 		op = contract.GetOp(pc)
 		operation := in.cfg.JumpTable[op]
 		if !operation.valid {
-			return nil, &ErrInvalidOpCode{opcode: int(op)}
+			return nil, &ErrInvalidOpCode{opcode: op}
 		}
 		// Validate stack
 		if sLen := stack.len(); sLen < operation.minStack {
 			return nil, &ErrStackUnderflow{stackLen: sLen, required: operation.minStack}
 		} else if sLen > operation.maxStack {
-			return nil, &ErrStackOverflow{stackLen: sLen, allowance: operation.maxStack}
+			return nil, &ErrStackOverflow{stackLen: sLen, limit: operation.maxStack}
 		}
 		// If the operation is valid, enforce and write restrictions
 		if in.readOnly && in.evm.chainRules.IsByzantium {

eth/api_tracer.go

@@ -768,7 +768,7 @@ func (api *PrivateDebugAPI) traceTx(ctx context.Context, message core.Message, v
 		return &ethapi.ExecutionResult{
 			Gas:         result.UsedGas,
 			Failed:      result.Failed(),
-			ReturnValue: fmt.Sprintf("%x", result.Result),
+			ReturnValue: fmt.Sprintf("%x", result.Return()),
 			StructLogs:  ethapi.FormatLogs(tracer.StructLogs()),
 		}, nil
 

graphql/graphql.go

@@ -812,7 +812,7 @@ func (b *Block) Call(ctx context.Context, args struct {
 		status = 0
 	}
 	return &CallResult{
-		data:    result.Result,
+		data:    result.Return(),
 		gasUsed: hexutil.Uint64(result.UsedGas),
 		status:  status,
 	}, nil
@@ -881,7 +881,7 @@ func (p *Pending) Call(ctx context.Context, args struct {
 		status = 0
 	}
 	return &CallResult{
-		data:    result.Result,
+		data:    result.Return(),
 		gasUsed: hexutil.Uint64(result.UsedGas),
 		status:  status,
 	}, nil

internal/ethapi/api.go

@@ -879,7 +879,7 @@ func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNrOr
 	if err != nil {
 		return nil, err
 	}
-	return result.Result, nil
+	return result.Return(), nil
 }
 
 func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash rpc.BlockNumberOrHash, gasCap *big.Int) (hexutil.Uint64, error) {
@@ -915,7 +915,7 @@ func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash
 
 		result, err := DoCall(ctx, b, args, blockNrOrHash, nil, vm.Config{}, 0, gasCap)
 		if err != nil {
-			if err == core.ErrInsufficientIntrinsicGas {
+			if err == core.ErrIntrinsicGas {
 				return true, nil, nil // Special case, raise gas limit
 			}
 			return true, nil, err // Bail out
@@ -928,8 +928,8 @@ func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash
 		failed, _, err := executable(mid)
 
 		// If the error is not nil(consensus error), it means the provided message
-		// call or transaction will never be accpeted no matter how many gas assigened.
-		// Return the error directly, don't struggle any more
+		// call or transaction will never be accepted no matter how much gas it is
+		// assigened. Return the error directly, don't struggle any more.
 		if err != nil {
 			return 0, err
 		}
@@ -949,8 +949,8 @@ func DoEstimateGas(ctx context.Context, b Backend, args CallArgs, blockNrOrHash
 			if result != nil {
 				if result.Err != vm.ErrOutOfGas {
 					errMsg := fmt.Sprintf("always failing transaction (%v)", result.Err)
-					if len(result.RevertReason) > 0 {
-						errMsg += fmt.Sprintf(" (0x%x)", result.RevertReason)
+					if len(result.Revert()) > 0 {
+						errMsg += fmt.Sprintf(" (0x%x)", result.Revert())
 					}
 					return 0, errors.New(errMsg)
 				}

les/odr_test.go

@@ -136,7 +136,7 @@ func odrContractCall(ctx context.Context, db ethdb.Database, config *params.Chai
 				//vmenv := core.NewEnv(statedb, config, bc, msg, header, vm.Config{})
 				gp := new(core.GasPool).AddGas(math.MaxUint64)
 				result, _ := core.ApplyMessage(vmenv, msg, gp)
-				res = append(res, result.Result...)
+				res = append(res, result.Return()...)
 			}
 		} else {
 			header := lc.GetHeaderByHash(bhash)
@@ -148,7 +148,7 @@ func odrContractCall(ctx context.Context, db ethdb.Database, config *params.Chai
 			gp := new(core.GasPool).AddGas(math.MaxUint64)
 			result, _ := core.ApplyMessage(vmenv, msg, gp)
 			if state.Error() == nil {
-				res = append(res, result.Result...)
+				res = append(res, result.Return()...)
 			}
 		}
 	}

light/odr_test.go

@@ -199,7 +199,7 @@ func odrContractCall(ctx context.Context, db ethdb.Database, bc *core.BlockChain
 		vmenv := vm.NewEVM(context, st, config, vm.Config{})
 		gp := new(core.GasPool).AddGas(math.MaxUint64)
 		result, _ := core.ApplyMessage(vmenv, msg, gp)
-		res = append(res, result.Result...)
+		res = append(res, result.Return()...)
 		if st.Error() != nil {
 			return res, st.Error()
 		}