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

improve error message in SendTransaction json-rpc api #786

Merged
merged 2 commits into from
Nov 26, 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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
## Improvements

* (ci) [tharsis#784](https://github.com/tharsis/ethermint/pull/784) Enable automatic backport of PRs.
* (rpc) [tharsis#786](https://github.com/tharsis/ethermint/pull/786) Improve error message of `SendTransaction`/`SendRawTransaction` JSON-RPC APIs.

### Bug Fixes

Expand Down
14 changes: 5 additions & 9 deletions app/ante/ante.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,6 @@ import (
"fmt"
"runtime/debug"

"github.com/palantir/stacktrace"

tmlog "github.com/tendermint/tendermint/libs/log"

sdk "github.com/cosmos/cosmos-sdk/types"
Expand Down Expand Up @@ -69,9 +67,10 @@ func NewAnteHandler(
)

default:
return ctx, stacktrace.Propagate(
sdkerrors.Wrap(sdkerrors.ErrUnknownExtensionOptions, typeURL),
"rejecting tx with unsupported extension option",
return ctx, sdkerrors.Wrapf(
sdkerrors.ErrUnknownExtensionOptions,
"rejecting tx with unsupported extension option: %s",
typeURL,
)
}

Expand Down Expand Up @@ -100,10 +99,7 @@ func NewAnteHandler(
authante.NewIncrementSequenceDecorator(ak), // innermost AnteDecorator
)
default:
return ctx, stacktrace.Propagate(
sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid transaction type: %T", tx),
"transaction is not an SDK tx",
)
return ctx, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "invalid transaction type: %T", tx)
}

return anteHandler(ctx, tx, sim)
Expand Down
175 changes: 55 additions & 120 deletions app/ante/eth.go

Large diffs are not rendered by default.

1 change: 0 additions & 1 deletion go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ require (
github.com/klauspost/compress v1.11.9 // indirect
github.com/miguelmota/go-ethereum-hdwallet v0.1.1
github.com/onsi/ginkgo v1.16.5
github.com/palantir/stacktrace v0.0.0-20161112013806-78658fd2d177
github.com/pkg/errors v0.9.1
github.com/rakyll/statik v0.1.7
github.com/rcrowley/go-metrics v0.0.0-20201227073835-cf1acfcdf475 // indirect
Expand Down
2 changes: 0 additions & 2 deletions go.sum
Original file line number Diff line number Diff line change
Expand Up @@ -799,8 +799,6 @@ github.com/otiai10/curr v1.0.0/go.mod h1:LskTG5wDwr8Rs+nNQ+1LlxRjAtTZZjtJW4rMXl6
github.com/otiai10/mint v1.3.0/go.mod h1:F5AjcsTsWUqX+Na9fpHb52P8pcRX2CI6A3ctIT91xUo=
github.com/otiai10/mint v1.3.2/go.mod h1:/yxELlJQ0ufhjUwhshSj+wFjZ78CnZ48/1wtmBH1OTc=
github.com/pact-foundation/pact-go v1.0.4/go.mod h1:uExwJY4kCzNPcHRj+hCR/HBbOOIwwtUjcrb0b5/5kLM=
github.com/palantir/stacktrace v0.0.0-20161112013806-78658fd2d177 h1:nRlQD0u1871kaznCnn1EvYiMbum36v7hw1DLPEjds4o=
github.com/palantir/stacktrace v0.0.0-20161112013806-78658fd2d177/go.mod h1:ao5zGxj8Z4x60IOVYZUbDSmt3R8Ddo080vEgPosHpak=
github.com/pascaldekloe/goe v0.0.0-20180627143212-57f6aae5913c/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
github.com/pascaldekloe/goe v0.1.0 h1:cBOtyMzM9HTpWjXfbbunk26uA6nG3a8n06Wieeh0MwY=
github.com/pascaldekloe/goe v0.1.0/go.mod h1:lzWF7FIEvWOWxwDKqyGYQf6ZUaNfKdP144TG7ZOy1lc=
Expand Down
9 changes: 5 additions & 4 deletions rpc/ethereum/backend/backend.go
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ import (
"github.com/cosmos/cosmos-sdk/client/flags"
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/server"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
"github.com/ethereum/go-ethereum/accounts/keystore"
"github.com/ethereum/go-ethereum/params"
Expand Down Expand Up @@ -810,10 +811,10 @@ func (e *EVMBackend) SendTransaction(args evmtypes.TransactionArgs) (common.Hash
// NOTE: If error is encountered on the node, the broadcast will not return an error
syncCtx := e.clientCtx.WithBroadcastMode(flags.BroadcastSync)
rsp, err := syncCtx.BroadcastTx(txBytes)
if err != nil || rsp.Code != 0 {
if err == nil {
err = errors.New(rsp.RawLog)
}
if rsp != nil && rsp.Code != 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

&& is correct?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

yes, it try to recover error from rsp if rsp.Code != 0.

err = sdkerrors.ABCIError(rsp.Codespace, rsp.Code, rsp.RawLog)
}
if err != nil {
e.logger.Error("failed to broadcast tx", "error", err.Error())
return txHash, err
}
Expand Down
9 changes: 5 additions & 4 deletions rpc/ethereum/namespaces/eth/api.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"github.com/cosmos/cosmos-sdk/client/flags"
"github.com/cosmos/cosmos-sdk/crypto/keyring"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"

"github.com/ethereum/go-ethereum/accounts/keystore"
Expand Down Expand Up @@ -479,10 +480,10 @@ func (e *PublicAPI) SendRawTransaction(data hexutil.Bytes) (common.Hash, error)

syncCtx := e.clientCtx.WithBroadcastMode(flags.BroadcastSync)
rsp, err := syncCtx.BroadcastTx(txBytes)
if err != nil || rsp.Code != 0 {
if err == nil {
err = errors.New(rsp.RawLog)
}
if rsp != nil && rsp.Code != 0 {
err = sdkerrors.ABCIError(rsp.Codespace, rsp.Code, rsp.RawLog)
}
if err != nil {
e.logger.Error("failed to broadcast tx", "error", err.Error())
return txHash, err
}
Expand Down
10 changes: 5 additions & 5 deletions rpc/ethereum/namespaces/miner/api.go
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package miner

import (
"errors"
"math/big"

"github.com/cosmos/cosmos-sdk/client"
Expand All @@ -11,6 +10,7 @@ import (
"github.com/cosmos/cosmos-sdk/server"
sdkconfig "github.com/cosmos/cosmos-sdk/server/config"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
distributiontypes "github.com/cosmos/cosmos-sdk/x/distribution/types"
Expand Down Expand Up @@ -143,10 +143,10 @@ func (api *API) SetEtherbase(etherbase common.Address) bool {
// NOTE: If error is encountered on the node, the broadcast will not return an error
syncCtx := api.clientCtx.WithBroadcastMode(flags.BroadcastSync)
rsp, err := syncCtx.BroadcastTx(txBytes)
if err != nil || rsp.Code != 0 {
if err == nil {
err = errors.New(rsp.RawLog)
}
if rsp != nil && rsp.Code != 0 {
Copy link
Contributor

Choose a reason for hiding this comment

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

&& is correct ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

err = sdkerrors.ABCIError(rsp.Codespace, rsp.Code, rsp.RawLog)
}
if err != nil {
api.logger.Debug("failed to broadcast tx", "error", err.Error())
return false
}
Expand Down
3 changes: 1 addition & 2 deletions x/evm/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@ import (

"github.com/ethereum/go-ethereum/eth/tracers"

"github.com/palantir/stacktrace"
"google.golang.org/grpc/codes"
"google.golang.org/grpc/status"

Expand Down Expand Up @@ -313,7 +312,7 @@ func (k Keeper) EstimateGas(c context.Context, req *types.EthCallRequest) (*type
rsp, err = k.ApplyMessageWithConfig(msg, nil, false, cfg)

if err != nil {
if errors.Is(stacktrace.RootCause(err), core.ErrIntrinsicGas) {
if errors.Is(err, core.ErrIntrinsicGas) {
return true, nil, nil // Special case, raise gas limit
}
return true, nil, err // Bail out
Expand Down
6 changes: 3 additions & 3 deletions x/evm/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,12 +7,12 @@ import (
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/store/prefix"
sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
paramtypes "github.com/cosmos/cosmos-sdk/x/params/types"
"github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core"
ethtypes "github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/core/vm"
"github.com/palantir/stacktrace"
"github.com/tendermint/tendermint/libs/log"

"github.com/ethereum/go-ethereum/params"
Expand Down Expand Up @@ -341,11 +341,11 @@ func (k Keeper) ClearBalance(addr sdk.AccAddress) (prevBalance sdk.Coin, err err
prevBalance = k.bankKeeper.GetBalance(k.Ctx(), addr, params.EvmDenom)
if prevBalance.IsPositive() {
if err := k.bankKeeper.SendCoinsFromAccountToModule(k.Ctx(), addr, types.ModuleName, sdk.Coins{prevBalance}); err != nil {
return sdk.Coin{}, stacktrace.Propagate(err, "failed to transfer to module account")
return sdk.Coin{}, sdkerrors.Wrap(err, "failed to transfer to module account")
}

if err := k.bankKeeper.BurnCoins(k.Ctx(), types.ModuleName, sdk.Coins{prevBalance}); err != nil {
return sdk.Coin{}, stacktrace.Propagate(err, "failed to burn coins from evm module account")
return sdk.Coin{}, sdkerrors.Wrap(err, "failed to burn coins from evm module account")
}
}

Expand Down
7 changes: 3 additions & 4 deletions x/evm/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,12 +5,11 @@ import (
"encoding/json"
"fmt"

"github.com/palantir/stacktrace"

tmbytes "github.com/tendermint/tendermint/libs/bytes"
tmtypes "github.com/tendermint/tendermint/types"

sdk "github.com/cosmos/cosmos-sdk/types"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"

"github.com/tharsis/ethermint/x/evm/types"
)
Expand All @@ -30,7 +29,7 @@ func (k *Keeper) EthereumTx(goCtx context.Context, msg *types.MsgEthereumTx) (*t

response, err := k.ApplyTransaction(tx)
if err != nil {
return nil, stacktrace.Propagate(err, "failed to apply transaction")
return nil, sdkerrors.Wrap(err, "failed to apply transaction")
}

attrs := []sdk.Attribute{
Expand All @@ -57,7 +56,7 @@ func (k *Keeper) EthereumTx(goCtx context.Context, msg *types.MsgEthereumTx) (*t
for _, log := range response.Logs {
value, err := json.Marshal(log)
if err != nil {
return nil, stacktrace.Propagate(err, "failed to encode log")
return nil, sdkerrors.Wrap(err, "failed to encode log")
}
txLogAttrs = append(txLogAttrs, sdk.NewAttribute(types.AttributeKeyTxLog, string(value)))
}
Expand Down
34 changes: 17 additions & 17 deletions x/evm/keeper/state_transition.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ package keeper
import (
"math/big"

"github.com/palantir/stacktrace"
tmtypes "github.com/tendermint/tendermint/types"

sdk "github.com/cosmos/cosmos-sdk/types"
Expand All @@ -29,7 +28,7 @@ func (k *Keeper) EVMConfig(ctx sdk.Context) (*types.EVMConfig, error) {
// get the coinbase address from the block proposer
coinbase, err := k.GetCoinbaseAddress(ctx)
if err != nil {
return nil, stacktrace.Propagate(err, "failed to obtain coinbase address")
return nil, sdkerrors.Wrap(err, "failed to obtain coinbase address")
}

var baseFee *big.Int
Expand Down Expand Up @@ -176,7 +175,7 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT

cfg, err := k.EVMConfig(ctx)
if err != nil {
return nil, stacktrace.Propagate(err, "failed to load evm config")
return nil, sdkerrors.Wrap(err, "failed to load evm config")
}

// get the latest signer according to the chain rules from the config
Expand All @@ -189,7 +188,7 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT

msg, err := tx.AsMessage(signer, baseFee)
if err != nil {
return nil, stacktrace.Propagate(err, "failed to return ethereum transaction as core message")
return nil, sdkerrors.Wrap(err, "failed to return ethereum transaction as core message")
}

txHash := tx.Hash()
Expand All @@ -215,7 +214,7 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT

res, err := k.ApplyMessageWithConfig(msg, nil, true, cfg)
if err != nil {
return nil, stacktrace.Propagate(err, "failed to apply ethereum core message")
return nil, sdkerrors.Wrap(err, "failed to apply ethereum core message")
}

res.Hash = txHash.Hex()
Expand All @@ -240,7 +239,7 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT

// refund gas according to Ethereum gas accounting rules.
if err := k.RefundGas(msg, msg.Gas()-res.GasUsed, cfg.Params.EvmDenom); err != nil {
return nil, stacktrace.Propagate(err, "failed to refund gas leftover gas to sender %s", msg.From())
return nil, sdkerrors.Wrapf(err, "failed to refund gas leftover gas to sender %s", msg.From())
}

if len(logs) > 0 {
Expand Down Expand Up @@ -314,9 +313,9 @@ func (k *Keeper) ApplyMessageWithConfig(msg core.Message, tracer vm.Tracer, comm

// return error if contract creation or call are disabled through governance
if !cfg.Params.EnableCreate && msg.To() == nil {
return nil, stacktrace.Propagate(types.ErrCreateDisabled, "failed to create new contract")
return nil, sdkerrors.Wrap(types.ErrCreateDisabled, "failed to create new contract")
} else if !cfg.Params.EnableCall && msg.To() != nil {
return nil, stacktrace.Propagate(types.ErrCallDisabled, "failed to call contract")
return nil, sdkerrors.Wrap(types.ErrCallDisabled, "failed to call contract")
}

sender := vm.AccountRef(msg.From())
Expand All @@ -326,12 +325,12 @@ func (k *Keeper) ApplyMessageWithConfig(msg core.Message, tracer vm.Tracer, comm
intrinsicGas, err := k.GetEthIntrinsicGas(msg, cfg.ChainConfig, contractCreation)
if err != nil {
// should have already been checked on Ante Handler
return nil, stacktrace.Propagate(err, "intrinsic gas failed")
return nil, sdkerrors.Wrap(err, "intrinsic gas failed")
}
// Should check again even if it is checked on Ante Handler, because eth_call don't go through Ante Handler.
if msg.Gas() < intrinsicGas {
// eth_estimateGas will check for this exact error
return nil, stacktrace.Propagate(core.ErrIntrinsicGas, "apply message")
return nil, sdkerrors.Wrap(core.ErrIntrinsicGas, "apply message")
}
leftoverGas := msg.Gas() - intrinsicGas

Expand All @@ -356,12 +355,12 @@ func (k *Keeper) ApplyMessageWithConfig(msg core.Message, tracer vm.Tracer, comm

// calculate gas refund
if msg.Gas() < leftoverGas {
return nil, stacktrace.Propagate(types.ErrGasOverflow, "apply message")
return nil, sdkerrors.Wrap(types.ErrGasOverflow, "apply message")
}
gasUsed := msg.Gas() - leftoverGas
refund := k.GasToRefund(gasUsed, refundQuotient)
if refund > gasUsed {
return nil, stacktrace.Propagate(types.ErrGasOverflow, "apply message")
return nil, sdkerrors.Wrap(types.ErrGasOverflow, "apply message")
}
gasUsed -= refund

Expand Down Expand Up @@ -390,7 +389,7 @@ func (k *Keeper) ApplyMessageWithConfig(msg core.Message, tracer vm.Tracer, comm
func (k *Keeper) ApplyMessage(msg core.Message, tracer vm.Tracer, commit bool) (*types.MsgEthereumTxResponse, error) {
cfg, err := k.EVMConfig(k.Ctx())
if err != nil {
return nil, stacktrace.Propagate(err, "failed to load evm config")
return nil, sdkerrors.Wrap(err, "failed to load evm config")
}
return k.ApplyMessageWithConfig(msg, tracer, commit, cfg)
}
Expand Down Expand Up @@ -438,7 +437,7 @@ func (k *Keeper) RefundGas(msg core.Message, leftoverGas uint64, denom string) e
err := k.bankKeeper.SendCoinsFromModuleToAccount(k.Ctx(), authtypes.FeeCollectorName, msg.From().Bytes(), refundedCoins)
if err != nil {
err = sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "fee collector account failed to refund fees: %s", err.Error())
return stacktrace.Propagate(err, "failed to refund %d leftover gas (%s)", leftoverGas, refundedCoins.String())
return sdkerrors.Wrapf(err, "failed to refund %d leftover gas (%s)", leftoverGas, refundedCoins.String())
}
default:
// no refund, consume gas and update the tx gas meter
Expand All @@ -461,9 +460,10 @@ func (k Keeper) GetCoinbaseAddress(ctx sdk.Context) (common.Address, error) {
consAddr := sdk.ConsAddress(ctx.BlockHeader().ProposerAddress)
validator, found := k.stakingKeeper.GetValidatorByConsAddr(ctx, consAddr)
if !found {
return common.Address{}, stacktrace.Propagate(
sdkerrors.Wrap(stakingtypes.ErrNoValidatorFound, consAddr.String()),
"failed to retrieve validator from block proposer address",
return common.Address{}, sdkerrors.Wrapf(
stakingtypes.ErrNoValidatorFound,
"failed to retrieve validator from block proposer address %s",
consAddr.String(),
)
}

Expand Down
Loading