Skip to content

Commit

Permalink
refactor: add clarifying context to error messages
Browse files Browse the repository at this point in the history
  • Loading branch information
k-yang committed Nov 5, 2024
1 parent fa8063c commit 6d0deb8
Show file tree
Hide file tree
Showing 9 changed files with 43 additions and 86 deletions.
6 changes: 3 additions & 3 deletions eth/rpc/backend/call_tx.go
Original file line number Diff line number Diff line change
Expand Up @@ -295,10 +295,10 @@ func (b *Backend) DoCall(
}

if res.Failed() {
if res.VmError != vm.ErrExecutionReverted.Error() {
return nil, status.Error(codes.Internal, res.VmError)
if res.VmError == vm.ErrExecutionReverted.Error() {
return nil, evm.NewRevertError(res.Ret)
}
return nil, evm.NewExecErrorWithReason(res.Ret)
return nil, status.Error(codes.Internal, res.VmError)
}

return res, nil
Expand Down
57 changes: 8 additions & 49 deletions x/evm/errors.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,19 +2,11 @@
package evm

import (
"errors"
"fmt"

errorsmod "cosmossdk.io/errors"
"github.com/ethereum/go-ethereum/common"

"github.com/ethereum/go-ethereum/accounts/abi"
"github.com/ethereum/go-ethereum/common/hexutil"
)

// Errors related to ERC20 calls and FunToken mappings.
const (
ErrOwnable = "Ownable: caller is not the owner"
)

const (
Expand All @@ -26,14 +18,10 @@ const (
codeErrInvalidRefund
codeErrInvalidGasCap
codeErrInvalidBaseFee
codeErrGasOverflow
codeErrInvalidAccount
codeErrInvalidGasLimit
codeErrInactivePrecompile
)

var ErrPostTxProcessing = errors.New("failed to execute post processing")

var (
// ErrInvalidState returns an error resulting from an invalid Storage State.
ErrInvalidState = errorsmod.Register(ModuleName, codeErrInvalidState, "invalid storage state")
Expand All @@ -59,53 +47,24 @@ var (
// ErrInvalidBaseFee returns an error if the base fee cap value is invalid
ErrInvalidBaseFee = errorsmod.Register(ModuleName, codeErrInvalidBaseFee, "invalid base fee")

// ErrGasOverflow returns an error if gas computation overlow/underflow
ErrGasOverflow = errorsmod.Register(ModuleName, codeErrGasOverflow, "gas computation overflow/underflow")

// ErrInvalidAccount returns an error if the account is not an EVM compatible account
ErrInvalidAccount = errorsmod.Register(ModuleName, codeErrInvalidAccount, "account type is not a valid ethereum account")

// ErrInvalidGasLimit returns an error if gas limit value is invalid
ErrInvalidGasLimit = errorsmod.Register(ModuleName, codeErrInvalidGasLimit, "invalid gas limit")
)

// NewExecErrorWithReason unpacks the revert return bytes and returns a wrapped error
// NewRevertError unpacks the revert return bytes and returns a wrapped error
// with the return reason.
func NewExecErrorWithReason(revertReason []byte) *RevertError {
result := common.CopyBytes(revertReason)
reason, errUnpack := abi.UnpackRevert(result)

var err error
errPrefix := "execution reverted"
if errUnpack == nil {
reasonStr := reason
err = fmt.Errorf("%s with reason \"%v\"", errPrefix, reasonStr)
} else if string(result) != "" {
reasonStr := string(result)
err = fmt.Errorf("%s with reason \"%v\"", errPrefix, reasonStr)
} else {
err = errors.New(errPrefix)
}
return &RevertError{
error: err,
reason: hexutil.Encode(result),
func NewRevertError(revertReason []byte) error {
reason, unpackingError := abi.UnpackRevert(revertReason)

if unpackingError != nil {
return fmt.Errorf("execution reverted, but unable to parse reason \"%v\"", string(revertReason))
}

return fmt.Errorf("execution reverted with reason \"%v\"", reason)
}

// 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() any {
return e.reason
}
2 changes: 1 addition & 1 deletion x/evm/keeper/call_contract.go
Original file line number Diff line number Diff line change
Expand Up @@ -121,7 +121,7 @@ func (k Keeper) CallContractWithInput(
return
}
if evmResp.VmError == vm.ErrExecutionReverted.Error() {
err = fmt.Errorf("VMError: %w", evm.NewExecErrorWithReason(evmResp.Ret))
err = fmt.Errorf("VMError: %w", evm.NewRevertError(evmResp.Ret))
return
}
err = fmt.Errorf("VMError: %s", evmResp.VmError)
Expand Down
2 changes: 1 addition & 1 deletion x/evm/keeper/erc20_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ func (s *Suite) TestERC20Calls() {
contract, deps.Sender.EthAddr, evm.EVM_MODULE_ADDRESS,
big.NewInt(69_420), deps.Ctx,
)
s.ErrorContains(err, evm.ErrOwnable)
s.ErrorContains(err, "Ownable: caller is not the owner")
}

s.T().Log("Mint tokens - Success")
Expand Down
11 changes: 4 additions & 7 deletions x/evm/keeper/gas_fees.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,21 +4,18 @@ package keeper
import (
"math/big"

gethcommon "github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core"
gethcore "github.com/ethereum/go-ethereum/core/types"

"cosmossdk.io/errors"
sdkmath "cosmossdk.io/math"
sdk "github.com/cosmos/cosmos-sdk/types"
errortypes "github.com/cosmos/cosmos-sdk/types/errors"
authante "github.com/cosmos/cosmos-sdk/x/auth/ante"

authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
gethcommon "github.com/ethereum/go-ethereum/common"
"github.com/ethereum/go-ethereum/core"
gethcore "github.com/ethereum/go-ethereum/core/types"
"github.com/ethereum/go-ethereum/params"

"github.com/NibiruChain/nibiru/v2/x/evm"

authtypes "github.com/cosmos/cosmos-sdk/x/auth/types"
)

// GetEthIntrinsicGas returns the intrinsic gas cost for the transaction
Expand Down
2 changes: 1 addition & 1 deletion x/evm/keeper/grpc_query.go
Original file line number Diff line number Diff line change
Expand Up @@ -447,7 +447,7 @@ func (k Keeper) EstimateGasForEvmCallType(

if failed && result != nil {
if result.VmError == vm.ErrExecutionReverted.Error() {
return nil, fmt.Errorf("Estimate gas VMError: %w", evm.NewExecErrorWithReason(result.Ret))
return nil, fmt.Errorf("Estimate gas VMError: %w", evm.NewRevertError(result.Ret))
}

if result.VmError == vm.ErrOutOfGas.Error() {
Expand Down
4 changes: 2 additions & 2 deletions x/evm/keeper/keeper.go
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import (
"github.com/ethereum/go-ethereum/core/vm"
gethparams "github.com/ethereum/go-ethereum/params"

sdkerrors "cosmossdk.io/errors"
"cosmossdk.io/errors"
"github.com/cometbft/cometbft/libs/log"

"github.com/cosmos/cosmos-sdk/codec"
Expand Down Expand Up @@ -109,7 +109,7 @@ func (k *Keeper) AddToBlockGasUsed(

blockGasUsed = k.EvmState.BlockGasUsed.GetOr(ctx, 0) + gasUsed
if blockGasUsed < gasUsed {
return 0, sdkerrors.Wrap(evm.ErrGasOverflow, "transient gas used")
return 0, errors.Wrap(core.ErrGasUintOverflow, "transient gas used")
}
k.EvmState.BlockGasUsed.Set(ctx, blockGasUsed)

Expand Down
35 changes: 18 additions & 17 deletions x/evm/keeper/msg_server.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,9 @@ import (
"github.com/ethereum/go-ethereum/crypto"
"github.com/ethereum/go-ethereum/params"

"github.com/NibiruChain/nibiru/v2/x/evm/embeds"

"github.com/NibiruChain/nibiru/v2/eth"
"github.com/NibiruChain/nibiru/v2/x/evm"
"github.com/NibiruChain/nibiru/v2/x/evm/embeds"
"github.com/NibiruChain/nibiru/v2/x/evm/statedb"
)

Expand Down Expand Up @@ -64,12 +63,11 @@ func (k *Keeper) EthereumTx(

// pass true to commit the StateDB
evmResp, _, err = k.ApplyEvmMsg(tmpCtx, evmMsg, nil, true, evmConfig, txConfig, false)

if err != nil {
// when a transaction contains multiple msg, as long as one of the msg fails
// all gas will be deducted. so is not msg.Gas()
k.ResetGasMeterAndConsumeGas(ctx, ctx.GasMeter().Limit())
return nil, errors.Wrap(err, "failed to apply ethereum core message")
return nil, errors.Wrap(err, "EthereumTx: failed to apply ethereum core message")
}

if !evmResp.Failed() {
Expand All @@ -79,7 +77,7 @@ func (k *Keeper) EthereumTx(

blockGasUsed, err := k.AddToBlockGasUsed(ctx, evmResp.GasUsed)
if err != nil {
return nil, errors.Wrap(err, "error adding transient gas used to block")
return nil, errors.Wrap(err, "EthereumTx: error adding transient gas used to block")
}

// refund gas in order to match the Ethereum gas consumption instead of the
Expand All @@ -90,19 +88,19 @@ func (k *Keeper) EthereumTx(
}
weiPerGas := txMsg.EffectiveGasPriceWeiPerGas(evmConfig.BaseFeeWei)
if err = k.RefundGas(ctx, evmMsg.From(), refundGas, weiPerGas); err != nil {
return nil, errors.Wrapf(err, "error refunding leftover gas to sender %s", evmMsg.From())
return nil, errors.Wrapf(err, "EthereumTx: error refunding leftover gas to sender %s", evmMsg.From())
}

// reset the gas meter for current TxMsg (EthereumTx)
k.ResetGasMeterAndConsumeGas(ctx, blockGasUsed)

err = k.EmitEthereumTxEvents(ctx, tx.To(), tx.Type(), evmMsg, evmResp)
if err != nil {
return nil, errors.Wrap(err, "error emitting ethereum tx events")
return nil, errors.Wrap(err, "EthereumTx: error emitting ethereum tx events")
}
err = k.EmitLogEvents(ctx, evmResp)
if err != nil {
return nil, errors.Wrap(err, "error emitting tx logs")
return nil, errors.Wrap(err, "EthereumTx: error emitting tx logs")
}

blockTxIdx := uint64(txConfig.TxIndex) + 1
Expand Down Expand Up @@ -278,10 +276,13 @@ func (k *Keeper) ApplyEvmMsg(ctx sdk.Context,
sender := vm.AccountRef(msg.From())
contractCreation := msg.To() == nil

intrinsicGas, err := k.GetEthIntrinsicGas(ctx, msg, evmConfig.ChainConfig, contractCreation)
intrinsicGas, err := core.IntrinsicGas(
msg.Data(), msg.AccessList(),
contractCreation, true, true,
)
if err != nil {
// should have already been checked on Ante Handler
return nil, evmObj, errors.Wrap(err, "intrinsic gas failed")
return nil, evmObj, errors.Wrap(err, "ApplyEvmMsg: intrinsic gas overflowed")
}

// Check if the provided gas in the message is enough to cover the intrinsic
Expand All @@ -294,7 +295,7 @@ func (k *Keeper) ApplyEvmMsg(ctx sdk.Context,
// eth_estimateGas will check for this exact error
return nil, evmObj, errors.Wrapf(
core.ErrIntrinsicGas,
"apply message msg.Gas = %d, intrinsic gas = %d.",
"ApplyEvmMsg: provided msg.Gas (%d) is less than intrinsic gas cost (%d)",
leftoverGas, intrinsicGas,
)
}
Expand All @@ -312,7 +313,7 @@ func (k *Keeper) ApplyEvmMsg(ctx sdk.Context,

msgWei, err := ParseWeiAsMultipleOfMicronibi(msg.Value())
if err != nil {
return nil, evmObj, err
return nil, evmObj, errors.Wrapf(err, "ApplyEvmMsg: invalid wei amount %s", msg.Value())
}

if contractCreation {
Expand Down Expand Up @@ -346,12 +347,12 @@ func (k *Keeper) ApplyEvmMsg(ctx sdk.Context,
// The dirty states in `StateDB` is either committed or discarded after return
if commit {
if err := stateDB.Commit(); err != nil {
return nil, evmObj, fmt.Errorf("failed to commit stateDB: %w", err)
return nil, evmObj, errors.Wrap(err, "ApplyEvmMsg: failed to commit stateDB")
}
}
// Rare case of uint64 gas overflow
if msg.Gas() < leftoverGas {
return nil, evmObj, errors.Wrap(evm.ErrGasOverflow, "apply message")
return nil, evmObj, errors.Wrapf(core.ErrGasUintOverflow, "ApplyEvmMsg: message gas limit (%d) < leftover gas (%d)", msg.Gas(), leftoverGas)
}

// GAS REFUND
Expand All @@ -373,7 +374,7 @@ func (k *Keeper) ApplyEvmMsg(ctx sdk.Context,
leftoverGas += refund
temporaryGasUsed -= refund
if msg.Gas() < leftoverGas {
return nil, evmObj, errors.Wrapf(evm.ErrGasOverflow, "message gas limit < leftover gas (%d < %d)", msg.Gas(), leftoverGas)
return nil, evmObj, errors.Wrapf(core.ErrGasUintOverflow, "ApplyEvmMsg: message gas limit (%d) < leftover gas (%d)", msg.Gas(), leftoverGas)
}

// Min gas used is a % of gasLimit
Expand Down Expand Up @@ -404,7 +405,7 @@ func ParseWeiAsMultipleOfMicronibi(weiInt *big.Int) (newWeiInt *big.Int, err err
tenPow12 := new(big.Int).Exp(big.NewInt(10), big.NewInt(12), nil)
if weiInt.Cmp(tenPow12) < 0 {
return weiInt, fmt.Errorf(
"wei amount is too small (%s), cannot transfer less than 1 micronibi. 10^18 wei == 1 NIBI == 10^6 micronibi", weiInt)
"wei amount is too small (%s), cannot transfer less than 1 micronibi. 1 NIBI == 10^6 micronibi == 10^18 wei", weiInt)
}

// truncate to highest micronibi amount
Expand Down Expand Up @@ -645,7 +646,7 @@ func (k *Keeper) EmitEthereumTxEvents(
}
err := ctx.EventManager().EmitTypedEvent(eventEthereumTx)
if err != nil {
return errors.Wrap(err, "failed to emit event ethereum tx")
return errors.Wrap(err, "EmitEthereumTxEvents: failed to emit event ethereum tx")
}

// Untyped event: "message", used for tendermint subscription
Expand Down
10 changes: 5 additions & 5 deletions x/evm/msg.go
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ import (
codectypes "github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/crypto/keyring"
sdk "github.com/cosmos/cosmos-sdk/types"
errortypes "github.com/cosmos/cosmos-sdk/types/errors"
sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
"github.com/cosmos/cosmos-sdk/x/auth/ante"
"github.com/cosmos/cosmos-sdk/x/auth/signing"
authtx "github.com/cosmos/cosmos-sdk/x/auth/tx"
Expand Down Expand Up @@ -154,7 +154,7 @@ func (msg MsgEthereumTx) ValidateBasic() error {

// Validate Size_ field, should be kept empty
if msg.Size_ != 0 {
return errorsmod.Wrapf(errortypes.ErrInvalidRequest, "tx size is deprecated")
return errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "tx size is deprecated")
}

txData, err := UnpackTxData(msg.Data)
Expand All @@ -166,12 +166,12 @@ func (msg MsgEthereumTx) ValidateBasic() error {

// prevent txs with 0 gas to fill up the mempool
if gas == 0 {
return errorsmod.Wrap(ErrInvalidGasLimit, "gas limit must not be zero")
return errorsmod.Wrap(sdkerrors.ErrInvalidGasLimit, "gas limit must not be zero")
}

// prevent gas limit from overflow
if g := new(big.Int).SetUint64(gas); !g.IsInt64() {
return errorsmod.Wrap(ErrGasOverflow, "gas limit must be less than math.MaxInt64")
return errorsmod.Wrap(core.ErrGasUintOverflow, "gas limit must be less than math.MaxInt64")
}

if err := txData.Validate(); err != nil {
Expand All @@ -181,7 +181,7 @@ func (msg MsgEthereumTx) ValidateBasic() error {
// Validate EthHash field after validated txData to avoid panic
txHash := msg.AsTransaction().Hash().Hex()
if msg.Hash != txHash {
return errorsmod.Wrapf(errortypes.ErrInvalidRequest, "invalid tx hash %s, expected: %s", msg.Hash, txHash)
return errorsmod.Wrapf(sdkerrors.ErrInvalidRequest, "invalid tx hash %s, expected: %s", msg.Hash, txHash)
}

return nil
Expand Down

0 comments on commit 6d0deb8

Please sign in to comment.