Skip to content

Commit

Permalink
fix error message in SendTransaction json-rpc api
Browse files Browse the repository at this point in the history
Closes: evmos#785

Solution:
- Remove `stacktrace.Propagate`s, and recover error message in jsonrpc server

changelog

fix error messages
  • Loading branch information
yihuang committed Nov 26, 2021
1 parent 2d8be4e commit 77af78b
Show file tree
Hide file tree
Showing 14 changed files with 112 additions and 190 deletions.
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 {
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 {
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, sdkerror.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

0 comments on commit 77af78b

Please sign in to comment.