internal/ethapi: EstimateGas and Call handle revert error(#173) (#200)

hot fix for EstimateGas and Call handle revert error XinFinOrg/XDPoSChain#173

accounts/abi/abi.go

@@ -19,8 +19,11 @@ package abi
 import (
 	"bytes"
 	"encoding/json"
+	"errors"
 	"fmt"
 	"io"
+
+	"github.com/XinFinOrg/XDC-Subnet/crypto"
 )
 
 // The ABI holds information about a contract's context and available
@@ -144,3 +147,26 @@ func (abi *ABI) MethodById(sigdata []byte) (*Method, error) {
 	}
 	return nil, fmt.Errorf("no method with id: %#x", sigdata[:4])
 }
+
+
+// revertSelector is a special function selector for revert reason unpacking.
+var revertSelector = crypto.Keccak256([]byte("Error(string)"))[:4]
+
+// UnpackRevert resolves the abi-encoded revert reason. According to the solidity
+// spec https://solidity.readthedocs.io/en/latest/control-structures.html#revert,
+// the provided revert reason is abi-encoded as if it were a call to a function
+// `Error(string)`. So it's a special tool for it.
+func UnpackRevert(data []byte) (string, error) {
+	if len(data) < 4 {
+		return "", errors.New("invalid data for unpacking")
+	}
+	if !bytes.Equal(data[:4], revertSelector) {
+		return "", errors.New("invalid data for unpacking")
+	}
+	typ, _ := NewType("string")
+	unpacked, err := (Arguments{{Type: typ}}).Unpack2(data[4:])
+	if err != nil {
+		return "", err
+	}
+	return unpacked[0].(string), nil
+}

accounts/abi/argument.go

@@ -18,6 +18,7 @@ package abi
 
 import (
 	"encoding/json"
+	"errors"
 	"fmt"
 	"reflect"
 	"strings"
@@ -100,6 +101,17 @@ func (arguments Arguments) Unpack(v interface{}, data []byte) error {
 	return arguments.unpackAtomic(v, marshalledValues)
 }
 
+// Unpack2 performs the operation hexdata -> Go format.
+func (arguments Arguments) Unpack2(data []byte) ([]interface{}, error) {
+	if len(data) == 0 {
+		if len(arguments.NonIndexed()) != 0 {
+			return nil, errors.New("abi: attempting to unmarshall an empty string while arguments are expected")
+		}
+		return make([]interface{}, 0), nil
+	}
+	return arguments.UnpackValues(data)
+}
+
 func (arguments Arguments) unpackTuple(v interface{}, marshalledValues []interface{}) error {
 
 	var (

accounts/abi/bind/backends/simulated.go

@@ -363,7 +363,7 @@ func (b *SimulatedBackend) EstimateGas(ctx context.Context, call XDPoSChain.Call
 
 // callContract implements common code between normal and pending contract calls.
 // state is modified during execution, make sure to copy it if necessary.
-func (b *SimulatedBackend) callContract(ctx context.Context, call XDPoSChain.CallMsg, block *types.Block, statedb *state.StateDB) ([]byte, uint64, bool, error) {
+func (b *SimulatedBackend) callContract(ctx context.Context, call XDPoSChain.CallMsg, block *types.Block, statedb *state.StateDB) (ret []byte, usedGas uint64, failed bool, err error) {
 	// Ensure message is initialized properly.
 	if call.GasPrice == nil {
 		call.GasPrice = big.NewInt(1)
@@ -391,7 +391,8 @@ func (b *SimulatedBackend) callContract(ctx context.Context, call XDPoSChain.Cal
 	vmenv := vm.NewEVM(evmContext, statedb, nil, b.config, vm.Config{})
 	gaspool := new(core.GasPool).AddGas(math.MaxUint64)
 	owner := common.Address{}
-	return core.NewStateTransition(vmenv, msg, gaspool).TransitionDb(owner)
+	ret, usedGas, failed, err, _ = core.NewStateTransition(vmenv, msg, gaspool).TransitionDb(owner)
+	return
 }
 
 // SendTransaction updates the pending block to include the given transaction.

core/state_processor.go

@@ -409,7 +409,7 @@ func ApplyTransaction(config *params.ChainConfig, tokensFee map[common.Address]*
 	// End Bypass blacklist address
 
 	// Apply the transaction to the current state (included in the env)
-	_, gas, failed, err := ApplyMessage(vmenv, msg, gp, coinbaseOwner)
+	_, gas, failed, err, _ := ApplyMessage(vmenv, msg, gp, coinbaseOwner)
 
 	if err != nil {
 		return nil, 0, err, false

core/state_transition.go

@@ -131,7 +131,7 @@ func NewStateTransition(evm *vm.EVM, msg Message, gp *GasPool) *StateTransition
 // the gas used (which includes gas refunds) and an error if it failed. An error always
 // indicates a core error meaning that the message would always fail for that particular
 // state and would never be accepted within a block.
-func ApplyMessage(evm *vm.EVM, msg Message, gp *GasPool, owner common.Address) ([]byte, uint64, bool, error) {
+func ApplyMessage(evm *vm.EVM, msg Message, gp *GasPool, owner common.Address) ([]byte, uint64, bool, error, error) {
 	return NewStateTransition(evm, msg, gp).TransitionDb(owner)
 }
 
@@ -217,7 +217,7 @@ func (st *StateTransition) preCheck() error {
 // TransitionDb will transition the state by applying the current message and
 // returning the result including the the used gas. It returns an error if it
 // failed. An error indicates a consensus issue.
-func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedGas uint64, failed bool, err error) {
+func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedGas uint64, failed bool, err error, vmErr error) {
 	if err = st.preCheck(); err != nil {
 		return
 	}
@@ -230,10 +230,10 @@ func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedG
 	// Pay intrinsic gas
 	gas, err := IntrinsicGas(st.data, contractCreation, homestead)
 	if err != nil {
-		return nil, 0, false, err
+		return nil, 0, false, err, nil
 	}
 	if err = st.useGas(gas); err != nil {
-		return nil, 0, false, err
+		return nil, 0, false, err, nil
 	}
 
 	var (
@@ -263,7 +263,7 @@ func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedG
 		// sufficient balance to make the transfer happen. The first
 		// balance transfer may never fail.
 		if vmerr == vm.ErrInsufficientBalance {
-			return nil, 0, false, vmerr
+			return nil, 0, false, vmerr, nil
 		}
 	}
 	st.refundGas()
@@ -276,7 +276,7 @@ func (st *StateTransition) TransitionDb(owner common.Address) (ret []byte, usedG
 		st.state.AddBalance(st.evm.Coinbase, new(big.Int).Mul(new(big.Int).SetUint64(st.gasUsed()), st.gasPrice))
 	}
 
-	return ret, st.gasUsed(), vmerr != nil, err
+	return ret, st.gasUsed(), vmerr != nil, err, vmerr
 }
 
 func (st *StateTransition) refundGas() {

core/token_validator.go

@@ -112,7 +112,7 @@ func CallContractWithState(call ethereum.CallMsg, chain consensus.ChainContext,
 	vmenv := vm.NewEVM(evmContext, statedb, nil, chain.Config(), vm.Config{})
 	gaspool := new(GasPool).AddGas(1000000)
 	owner := common.Address{}
-	rval, _, _, err := NewStateTransition(vmenv, msg, gaspool).TransitionDb(owner)
+	rval, _, _, err, _ := NewStateTransition(vmenv, msg, gaspool).TransitionDb(owner)
 	if err != nil {
 		return nil, err
 	}

eth/api_tracer.go

@@ -480,7 +480,7 @@ func (api *PrivateDebugAPI) traceBlock(ctx context.Context, block *types.Block,
 
 		vmenv := vm.NewEVM(vmctx, statedb, XDCxState, api.config, vm.Config{})
 		owner := common.Address{}
-		if _, _, _, err := core.ApplyMessage(vmenv, msg, new(core.GasPool).AddGas(msg.Gas()), owner); err != nil {
+		if _, _, _, err, _ := core.ApplyMessage(vmenv, msg, new(core.GasPool).AddGas(msg.Gas()), owner); err != nil {
 			failed = err
 			break
 		}
@@ -636,7 +636,7 @@ func (api *PrivateDebugAPI) traceTx(ctx context.Context, message core.Message, v
 	vmenv := vm.NewEVM(vmctx, statedb, nil, api.config, vm.Config{Debug: true, Tracer: tracer})
 
 	owner := common.Address{}
-	ret, gas, failed, err := core.ApplyMessage(vmenv, message, new(core.GasPool).AddGas(message.Gas()), owner)
+	ret, gas, failed, err, _ := core.ApplyMessage(vmenv, message, new(core.GasPool).AddGas(message.Gas()), owner)
 	if err != nil {
 		return nil, fmt.Errorf("tracing failed: %v", err)
 	}

eth/tracers/tracers_test.go

@@ -183,7 +183,7 @@ func TestPrestateTracerCreate2(t *testing.T) {
 		t.Fatalf("failed to prepare transaction for tracing: %v", err)
 	}
 	st := core.NewStateTransition(evm, msg, new(core.GasPool).AddGas(tx.Gas()))
-	if _, _, _, err = st.TransitionDb(common.Address{}); err != nil {
+	if _, _, _, err, _ = st.TransitionDb(common.Address{}); err != nil {
 		t.Fatalf("failed to execute transaction: %v", err)
 	}
 	// Retrieve the trace result and compare against the etalon
@@ -258,7 +258,7 @@ func TestCallTracer(t *testing.T) {
 				t.Fatalf("failed to prepare transaction for tracing: %v", err)
 			}
 			st := core.NewStateTransition(evm, msg, new(core.GasPool).AddGas(tx.Gas()))
-			if _, _, _, err = st.TransitionDb(common.Address{}); err != nil {
+			if _, _, _, err, _ = st.TransitionDb(common.Address{}); err != nil {
 				t.Fatalf("failed to execute transaction: %v", err)
 			}
 			// Retrieve the trace result and compare against the etalon

internal/ethapi/api.go

@@ -32,6 +32,7 @@ import (
 
 	"github.com/XinFinOrg/XDC-Subnet/accounts"
 	"github.com/XinFinOrg/XDC-Subnet/accounts/abi/bind"
+	"github.com/XinFinOrg/XDC-Subnet/accounts/abi"
 	"github.com/XinFinOrg/XDC-Subnet/accounts/keystore"
 	"github.com/XinFinOrg/XDC-Subnet/common"
 	"github.com/XinFinOrg/XDC-Subnet/common/hexutil"
@@ -1045,12 +1046,12 @@ type CallArgs struct {
 	Data     hexutil.Bytes   `json:"data"`
 }
 
-func (s *PublicBlockChainAPI) doCall(ctx context.Context, args CallArgs, blockNr rpc.BlockNumber, vmCfg vm.Config, timeout time.Duration) ([]byte, uint64, bool, error) {
+func (s *PublicBlockChainAPI) doCall(ctx context.Context, args CallArgs, blockNr rpc.BlockNumber, vmCfg vm.Config, timeout time.Duration) ([]byte, uint64, bool, error, error) {
 	defer func(start time.Time) { log.Debug("Executing EVM call finished", "runtime", time.Since(start)) }(time.Now())
 
 	statedb, header, err := s.b.StateAndHeaderByNumber(ctx, blockNr)
 	if statedb == nil || err != nil {
-		return nil, 0, false, err
+		return nil, 0, false, err, nil
 	}
 	// Set sender address or use a default if none specified
 	addr := args.From
@@ -1088,20 +1089,20 @@ func (s *PublicBlockChainAPI) doCall(ctx context.Context, args CallArgs, blockNr
 
 	block, err := s.b.BlockByNumber(ctx, blockNr)
 	if err != nil {
-		return nil, 0, false, err
+		return nil, 0, false, err, nil
 	}
 	author, err := s.b.GetEngine().Author(block.Header())
 	if err != nil {
-		return nil, 0, false, err
+		return nil, 0, false, err, nil
 	}
 	XDCxState, err := s.b.XDCxService().GetTradingState(block, author)
 	if err != nil {
-		return nil, 0, false, err
+		return nil, 0, false, err, nil
 	}
 	// Get a new instance of the EVM.
 	evm, vmError, err := s.b.GetEVM(ctx, msg, statedb, XDCxState, header, vmCfg)
 	if err != nil {
-		return nil, 0, false, err
+		return nil, 0, false, err, nil
 	}
 	// Wait for the context to be done and cancel the evm. Even if the
 	// EVM has finished, cancelling may be done (repeatedly)
@@ -1114,18 +1115,64 @@ func (s *PublicBlockChainAPI) doCall(ctx context.Context, args CallArgs, blockNr
 	// and apply the message.
 	gp := new(core.GasPool).AddGas(math.MaxUint64)
 	owner := common.Address{}
-	res, gas, failed, err := core.ApplyMessage(evm, msg, gp, owner)
+	res, gas, failed, err, vmErr := core.ApplyMessage(evm, msg, gp, owner)
 	if err := vmError(); err != nil {
-		return nil, 0, false, err
+		return nil, 0, false, err, nil
 	}
-	return res, gas, failed, err
+
+	// If the timer caused an abort, return an appropriate error message
+	if evm.Cancelled() {
+		return nil, 0, false, fmt.Errorf("execution aborted (timeout = %v)", timeout), nil
+	}
+	if err != nil {
+		return res, 0, false, fmt.Errorf("err: %w (supplied gas %d)", err, msg.Gas()), nil
+	}
+	return res, gas, failed, err, vmErr
+}
+
+func newRevertError(res []byte) *revertError {
+	reason, errUnpack := abi.UnpackRevert(res)
+	err := errors.New("execution reverted")
+	if errUnpack == nil {
+		err = fmt.Errorf("execution reverted: %v", reason)
+	}
+	return &revertError{
+		error:  err,
+		reason: hexutil.Encode(res),
+	}
+}
+
+// revertError is an API error that encompasses an EVM revertal with JSON error
+// code and a binary data blob.
+type revertError struct {
+	error
+	reason string // revert reason hex encoded
+}
+
+// ErrorCode returns the JSON error code for a revertal.
+// See: https://github.com/ethereum/wiki/wiki/JSON-RPC-Error-Codes-Improvement-Proposal
+func (e *revertError) ErrorCode() int {
+	return 3
+}
+
+// ErrorData returns the hex encoded revert reason.
+func (e *revertError) ErrorData() interface{} {
+	return e.reason
 }
 
 // Call executes the given transaction on the state for the given block number.
 // It doesn't make and changes in the state/blockchain and is useful to execute and retrieve values.
 func (s *PublicBlockChainAPI) Call(ctx context.Context, args CallArgs, blockNr rpc.BlockNumber) (hexutil.Bytes, error) {
-	result, _, _, err := s.doCall(ctx, args, blockNr, vm.Config{}, 5*time.Second)
-	return (hexutil.Bytes)(result), err
+	result, _, failed, err, vmErr := s.doCall(ctx, args, blockNr, vm.Config{}, 5*time.Second)
+	if err != nil {
+		return nil, err
+	}
+	// If the result contains a revert reason, try to unpack and return it.
+	if failed && len(result) > 0 {
+		return nil, newRevertError(result)
+	}
+
+	return (hexutil.Bytes)(result), vmErr
 }
 
 // EstimateGas returns an estimate of the amount of gas needed to execute the
@@ -1150,29 +1197,58 @@ func (s *PublicBlockChainAPI) EstimateGas(ctx context.Context, args CallArgs) (h
 	cap = hi
 
 	// Create a helper to check if a gas allowance results in an executable transaction
-	executable := func(gas uint64) bool {
+	executable := func(gas uint64) (bool, []byte, error, error) {
 		args.Gas = hexutil.Uint64(gas)
 
-		_, _, failed, err := s.doCall(ctx, args, rpc.LatestBlockNumber, vm.Config{}, 0)
-		if err != nil || failed {
-			log.Warn("[EstimateGas] api", "err", err)
-			return false
+		res, _, failed, err, vmErr := s.doCall(ctx, args, rpc.LatestBlockNumber, vm.Config{}, 0)
+		if err != nil {
+			if errors.Is(err, core.ErrIntrinsicGas) {
+				return false, nil, nil, nil // Special case, raise gas limit
+			}
+			return false, nil, err, nil // Bail out
 		}
-		return true
+		if failed {
+			return false, res, nil, vmErr
+		}
+
+		return true, nil, nil, nil
 	}
 	// Execute the binary search and hone in on an executable gas limit
 	for lo+1 < hi {
 		mid := (hi + lo) / 2
-		if !executable(mid) {
+		ok, _, err, _ := executable(mid)
+
+		// If the error is not nil(consensus error), it means the provided message
+		// 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
+		}
+
+		if !ok {
 			lo = mid
 		} else {
 			hi = mid
 		}
 	}
+
 	// Reject the transaction as invalid if it still fails at the highest allowance
 	if hi == cap {
-		if !executable(hi) {
-			return 0, fmt.Errorf("gas required exceeds allowance or always failing transaction")
+		ok, res, err, vmErr := executable(hi)
+		if err != nil {
+			return 0, err
+		}
+
+		if !ok {
+			if vmErr != vm.ErrOutOfGas {
+				if len(res) > 0 {
+					return 0, newRevertError(res)
+				}
+				return 0, vmErr
+			}
+
+			// Otherwise, the specified gas cap is too low
+			return 0, fmt.Errorf("gas required exceeds allowance (%d)", cap)						
 		}
 	}
 	return hexutil.Uint64(hi), nil

les/odr_test.go

@@ -141,7 +141,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)
 				owner := common.Address{}
-				ret, _, _, _ := core.ApplyMessage(vmenv, msg, gp, owner)
+				ret, _, _, _, _ := core.ApplyMessage(vmenv, msg, gp, owner)
 				res = append(res, ret...)
 			}
 		} else {
@@ -158,7 +158,7 @@ func odrContractCall(ctx context.Context, db ethdb.Database, config *params.Chai
 			vmenv := vm.NewEVM(context, statedb, nil, config, vm.Config{})
 			gp := new(core.GasPool).AddGas(math.MaxUint64)
 			owner := common.Address{}
-			ret, _, _, _ := core.ApplyMessage(vmenv, msg, gp, owner)
+			ret, _, _, _, _ := core.ApplyMessage(vmenv, msg, gp, owner)
 			if statedb.Error() == nil {
 				res = append(res, ret...)
 			}

light/odr_test.go

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