Skip to content
This repository has been archived by the owner on Apr 4, 2024. It is now read-only.

EVM Error format #350

Merged
merged 7 commits into from
Jul 28, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 7 additions & 10 deletions ethereum/rpc/namespaces/eth/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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"
Expand Down Expand Up @@ -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
Expand Down
4 changes: 2 additions & 2 deletions x/evm/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
)
Expand Down Expand Up @@ -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)
}
Expand Down
35 changes: 30 additions & 5 deletions x/evm/types/errors.go
Original file line number Diff line number Diff line change
@@ -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"
Expand Down Expand Up @@ -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 {
fedekunze marked this conversation as resolved.
Show resolved Hide resolved
var result = common.CopyBytes(revertReason)
reason, errUnpack := abi.UnpackRevert(result)
err := errors.New("execution reverted")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are other types of errors, e.g. out of gas?

Copy link
Contributor Author

@thomas-nguy thomas-nguy Jul 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm I could not find anything special in geth. Is out of gas include in exection reverted?

Copy link
Contributor

@yihuang yihuang Jul 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://github.com/ethereum/go-ethereum/blob/master/core/vm/errors.go#L31
there are several possible cases for the VmError. maybe just change it to errors.New(vmError) here is enough?

Copy link
Contributor Author

@thomas-nguy thomas-nguy Jul 26, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see, you are right, I have missread the if condition. Fixed

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),
}
}

// 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
}

return sdkerrors.Wrapf(ErrExecutionReverted, "%s", hexValue)
// ErrorData returns the hex encoded revert reason.
func (e *RevertError) ErrorData() interface{} {
return e.reason
}
53 changes: 53 additions & 0 deletions x/evm/types/errors_test.go
Original file line number Diff line number Diff line change
@@ -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())
}
}