evm: update error format (#350)

* return geth error format

* fix format in gasestimate

* deal with other evm errors

* fix import

* fix lint

* add test

Co-authored-by: Federico Kunze Küllmer <31522760+fedekunze@users.noreply.github.com>

ethereum/rpc/namespaces/eth/api.go

@@ -8,6 +8,10 @@ import (
 	"math/big"
 	"strings"
 
+	"github.com/ethereum/go-ethereum/core/vm"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
+
 	"github.com/pkg/errors"
 	"github.com/spf13/viper"
 	"github.com/tendermint/tendermint/libs/log"
@@ -26,7 +30,6 @@ import (
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/common/hexutil"
 	ethtypes "github.com/ethereum/go-ethereum/core/types"
-	"github.com/ethereum/go-ethereum/core/vm"
 	"github.com/ethereum/go-ethereum/crypto"
 
 	"github.com/tharsis/ethermint/crypto/hd"
@@ -559,18 +562,12 @@ func (e *PublicAPI) doCall(
 	if err != nil {
 		return nil, err
 	}
-	if len(res.VmError) > 0 {
-		if res.VmError == vm.ErrExecutionReverted.Error() {
-			return nil, evmtypes.NewExecErrorWithReason(res.Ret)
-		}
-		return nil, errors.New(res.VmError)
-	}
 
 	if res.Failed() {
-		if res.VmError == vm.ErrExecutionReverted.Error() {
-			return nil, evmtypes.NewExecErrorWithReason(res.Ret)
+		if res.VmError != vm.ErrExecutionReverted.Error() {
+			return nil, status.Error(codes.Internal, res.VmError)
 		}
-		return nil, errors.New(res.VmError)
+		return nil, evmtypes.NewExecErrorWithReason(res.Ret)
 	}
 
 	return res, nil

x/evm/keeper/grpc_query.go

@@ -405,7 +405,7 @@ func (k Keeper) EstimateGas(c context.Context, req *types.EthCallRequest) (*type
 
 	// Binary search the gas requirement, as it may be higher than the amount used
 	var (
-		lo  uint64 = ethparams.TxGas - 1
+		lo  = ethparams.TxGas - 1
 		hi  uint64
 		cap uint64
 	)
@@ -478,7 +478,7 @@ func (k Keeper) EstimateGas(c context.Context, req *types.EthCallRequest) (*type
 		if failed {
 			if result != nil && result.VmError != vm.ErrOutOfGas.Error() {
 				if result.VmError == vm.ErrExecutionReverted.Error() {
-					return nil, status.Error(codes.Internal, types.NewExecErrorWithReason(result.Ret).Error())
+					return nil, types.NewExecErrorWithReason(result.Ret)
 				}
 				return nil, status.Error(codes.Internal, result.VmError)
 			}

x/evm/types/errors.go

@@ -1,7 +1,11 @@
 package types
 
 import (
+	"errors"
+	"fmt"
+
 	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	"github.com/ethereum/go-ethereum/common"
 
 	"github.com/ethereum/go-ethereum/accounts/abi"
 	"github.com/ethereum/go-ethereum/common/hexutil"
@@ -75,12 +79,33 @@ var (
 
 // NewExecErrorWithReason unpacks the revert return bytes and returns a wrapped error
 // with the return reason.
-func NewExecErrorWithReason(revertReason []byte) error {
-	hexValue := hexutil.Encode(revertReason)
-	reason, errUnpack := abi.UnpackRevert(revertReason)
+func NewExecErrorWithReason(revertReason []byte) *RevertError {
+	var result = common.CopyBytes(revertReason)
+	reason, errUnpack := abi.UnpackRevert(result)
+	err := errors.New("execution reverted")
 	if errUnpack == nil {
-		return sdkerrors.Wrapf(ErrExecutionReverted, "%s: %s", reason, hexValue)
+		err = fmt.Errorf("execution reverted: %v", reason)
+	}
+	return &RevertError{
+		error:  err,
+		reason: hexutil.Encode(result),
 	}
-
-	return sdkerrors.Wrapf(ErrExecutionReverted, "%s", hexValue)
+}
+
+// RevertError is an API error that encompass an EVM revert 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 revert.
+// 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
 }

x/evm/types/errors_test.go

@@ -0,0 +1,53 @@
+package types
+
+import (
+	"github.com/ethereum/go-ethereum/crypto"
+	"github.com/status-im/keycard-go/hexutils"
+	"github.com/stretchr/testify/require"
+	"testing"
+)
+
+var revertSelector = crypto.Keccak256([]byte("Error(string)"))[:4]
+
+func TestNewExecErrorWithReason(t *testing.T) {
+
+	testCases := []struct {
+		name         string
+		errorMessage string
+		revertReason []byte
+		data         string
+	}{
+		{
+			"Empty reason",
+			"execution reverted",
+			nil,
+			"0x",
+		},
+		{
+			"With unpackable reason",
+			"execution reverted",
+			[]byte("a"),
+			"0x61",
+		},
+		{
+			"With packable reason but empty reason",
+			"execution reverted",
+			revertSelector,
+			"0x08c379a0",
+		},
+		{
+			"With packable reason with reason",
+			"execution reverted: COUNTER_TOO_LOW",
+			hexutils.HexToBytes("08C379A00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000F434F554E5445525F544F4F5F4C4F570000000000000000000000000000000000"),
+			"0x08c379a00000000000000000000000000000000000000000000000000000000000000020000000000000000000000000000000000000000000000000000000000000000f434f554e5445525f544f4f5f4c4f570000000000000000000000000000000000",
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+		errWithReason := NewExecErrorWithReason(tc.revertReason)
+		require.Equal(t, tc.errorMessage, errWithReason.Error())
+		require.Equal(t, tc.data, errWithReason.ErrorData())
+		require.Equal(t, 3, errWithReason.ErrorCode())
+	}
+}