evm: stacktrace errors for keeper (#105)

* evm: stacktrace errors for keeper

* fix godoc

go.mod

@@ -30,6 +30,7 @@ require (
 	github.com/kr/pretty v0.2.0 // indirect
 	github.com/mattn/go-colorable v0.1.8 // indirect
 	github.com/miguelmota/go-ethereum-hdwallet v0.0.0-20200123000308-a60dcd172b4c
+	github.com/palantir/stacktrace v0.0.0-20161112013806-78658fd2d177 // indirect
 	github.com/pkg/errors v0.9.1
 	github.com/prometheus/tsdb v0.10.0 // indirect
 	github.com/rakyll/statik v0.1.7

go.sum

@@ -652,6 +652,8 @@ github.com/otiai10/mint v1.3.0/go.mod h1:F5AjcsTsWUqX+Na9fpHb52P8pcRX2CI6A3ctIT9
 github.com/otiai10/mint v1.3.2 h1:VYWnrP5fXmz1MXvjuUvcBrXSjGE6xjON+axB/UrpO3E=
 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=

x/evm/keeper/keeper.go

@@ -162,10 +162,10 @@ func (k Keeper) IncreaseTxIndexTransient() {
 	store.Set(types.KeyPrefixTransientBloom, sdk.Uint64ToBigEndian(txIndex+1))
 }
 
-// ResetRefundTransient resets the refund gas value.
+// ResetRefundTransient resets the available refund amount to 0
 func (k Keeper) ResetRefundTransient(ctx sdk.Context) {
 	store := ctx.TransientStore(k.transientKey)
-	store.Delete(types.KeyPrefixTransientRefund)
+	store.Set(types.KeyPrefixTransientRefund, sdk.Uint64ToBigEndian(0))
 }
 
 // ----------------------------------------------------------------------------

x/evm/keeper/msg_server.go

@@ -5,6 +5,7 @@ import (
 	"time"
 
 	"github.com/armon/go-metrics"
+	"github.com/palantir/stacktrace"
 
 	tmbytes "github.com/tendermint/tendermint/libs/bytes"
 	tmtypes "github.com/tendermint/tendermint/types"
@@ -40,7 +41,7 @@ func (k *Keeper) EthereumTx(goCtx context.Context, msg *types.MsgEthereumTx) (*t
 
 	response, err := k.ApplyTransaction(tx)
 	if err != nil {
-		return nil, err
+		return nil, stacktrace.Propagate(err, "failed to apply transaction")
 	}
 
 	defer func() {

x/evm/keeper/state_transition.go

@@ -2,11 +2,11 @@ package keeper
 
 import (
 	"errors"
-	"fmt"
 	"math/big"
 	"os"
 	"time"
 
+	"github.com/palantir/stacktrace"
 	tmtypes "github.com/tendermint/tendermint/types"
 
 	"github.com/cosmos/cosmos-sdk/telemetry"
@@ -117,13 +117,15 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT
 
 	cfg, found := k.GetChainConfig(infCtx)
 	if !found {
-		return nil, types.ErrChainConfigNotFound
+		return nil, stacktrace.Propagate(types.ErrChainConfigNotFound, "configuration not found")
 	}
 	ethCfg := cfg.EthereumConfig(k.eip155ChainID)
 
-	msg, err := tx.AsMessage(ethtypes.MakeSigner(ethCfg, big.NewInt(k.ctx.BlockHeight())))
+	signer := ethtypes.MakeSigner(ethCfg, big.NewInt(k.ctx.BlockHeight()))
+
+	msg, err := tx.AsMessage(signer)
 	if err != nil {
-		return nil, err
+		return nil, stacktrace.Propagate(err, "failed to return ethereum transaction as core message")
 	}
 
 	evm := k.NewEVM(msg, ethCfg)
@@ -134,7 +136,7 @@ func (k *Keeper) ApplyTransaction(tx *ethtypes.Transaction) (*types.MsgEthereumT
 	// create an ethereum StateTransition instance and run TransitionDb
 	res, err := k.ApplyMessage(evm, msg, ethCfg)
 	if err != nil {
-		return nil, err
+		return nil, stacktrace.Propagate(err, "failed to apply ethereum core message")
 	}
 
 	txHash := tx.Hash()
@@ -184,7 +186,7 @@ func (k *Keeper) ApplyMessage(evm *vm.EVM, msg core.Message, cfg *params.ChainCo
 	// ensure gas is consistent during CheckTx
 	if k.ctx.IsCheckTx() {
 		if err := k.CheckGasConsumption(msg, cfg, gasConsumed, contractCreation); err != nil {
-			return nil, err
+			return nil, stacktrace.Propagate(err, "gas consumption check failed during CheckTx")
 		}
 	}
 
@@ -196,17 +198,17 @@ func (k *Keeper) ApplyMessage(evm *vm.EVM, msg core.Message, cfg *params.ChainCo
 
 	// refund gas prior to handling the vm error in order to set the updated gas meter
 	if err := k.RefundGas(msg, leftoverGas); err != nil {
-		return nil, err
+		return nil, stacktrace.Propagate(err, "failed to refund gas leftover gas to sender %s", msg.From())
 	}
 
 	if vmErr != nil {
 		if errors.Is(vmErr, vm.ErrExecutionReverted) {
 			// unpack the return data bytes from the err if the execution has been reverted on the VM
-			return nil, types.NewExecErrorWithReson(ret)
+			return nil, stacktrace.Propagate(types.NewExecErrorWithReson(ret), "transaction reverted")
 		}
 
 		// wrap the VM error
-		return nil, sdkerrors.Wrap(types.ErrVMExecution, vmErr.Error())
+		return nil, stacktrace.Propagate(sdkerrors.Wrap(types.ErrVMExecution, vmErr.Error()), "vm execution failed")
 	}
 
 	return &types.MsgEthereumTxResponse{
@@ -224,11 +226,11 @@ func (k *Keeper) CheckGasConsumption(msg core.Message, cfg *params.ChainConfig,
 	intrinsicGas, err := core.IntrinsicGas(msg.Data(), msg.AccessList(), isContractCreation, homestead, istanbul)
 	if err != nil {
 		// should have already been checked on Ante Handler
-		return err
+		return stacktrace.Propagate(err, "intrinsic gas failed")
 	}
 
 	if intrinsicGas != gasConsumed {
-		return fmt.Errorf("inconsistent gas. Expected gas consumption to be %d (intrinsic gas only), got %d", intrinsicGas, gasConsumed)
+		return sdkerrors.Wrapf(types.ErrInconsistentGas, "expected gas consumption to be %d (intrinsic gas only), got %d", intrinsicGas, gasConsumed)
 	}
 
 	return nil
@@ -239,15 +241,31 @@ func (k *Keeper) CheckGasConsumption(msg core.Message, cfg *params.ChainConfig,
 // returned by the EVM execution, thus ignoring the previous intrinsic gas inconsumed during in the
 // AnteHandler.
 func (k *Keeper) RefundGas(msg core.Message, leftoverGas uint64) error {
+	if leftoverGas > msg.Gas() {
+		return stacktrace.Propagate(
+			sdkerrors.Wrapf(types.ErrInconsistentGas, "leftover gas cannot be greater than gas limit (%d > %d)", leftoverGas, msg.Gas()),
+			"failed to update gas consumed after refund of leftover gas",
+		)
+	}
+
 	gasConsumed := msg.Gas() - leftoverGas
 
 	// Apply refund counter, capped to half of the used gas.
 	refund := gasConsumed / 2
-	if refund > k.GetRefund() {
-		refund = k.GetRefund()
+	availableRefund := k.GetRefund()
+	if refund > availableRefund {
+		refund = availableRefund
 	}
 
 	leftoverGas += refund
+
+	if leftoverGas > msg.Gas() {
+		return stacktrace.Propagate(
+			sdkerrors.Wrapf(types.ErrInconsistentGas, "leftover gas cannot be greater than gas limit (%d > %d)", leftoverGas, msg.Gas()),
+			"failed to update gas consumed after refund of %d gas", refund,
+		)
+	}
+
 	gasConsumed = msg.Gas() - leftoverGas
 
 	// Return EVM tokens for remaining gas, exchanged at the original rate.
@@ -259,15 +277,18 @@ func (k *Keeper) RefundGas(msg core.Message, leftoverGas uint64) error {
 	switch remaining.Sign() {
 	case -1:
 		// negative refund errors
-		return fmt.Errorf("refunded amount value cannot be negative %d", remaining.Int64())
+		return sdkerrors.Wrapf(types.ErrInvalidRefund, "refunded amount value cannot be negative %d", remaining.Int64())
 	case 1:
 		// positive amount refund
 		params := k.GetParams(infCtx)
 		refundedCoins := sdk.Coins{sdk.NewCoin(params.EvmDenom, sdk.NewIntFromBigInt(remaining))}
 
 		// refund to sender from the fee collector module account, which is the escrow account in charge of collecting tx fees
-		if err := k.bankKeeper.SendCoinsFromModuleToAccount(infCtx, authtypes.FeeCollectorName, msg.From().Bytes(), refundedCoins); err != nil {
-			return sdkerrors.Wrapf(sdkerrors.ErrInsufficientFunds, "fee collector account failed to refund fees: %s", err.Error())
+
+		err := k.bankKeeper.SendCoinsFromModuleToAccount(infCtx, 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())
 		}
 	default:
 		// no refund, consume gas and update the tx gas meter
@@ -277,6 +298,7 @@ func (k *Keeper) RefundGas(msg core.Message, leftoverGas uint64) error {
 	// original gas limit defined in the msg and will consume the gas now that the amount has been
 	// refunded
 	gasMeter := sdk.NewGasMeter(msg.Gas())
+	// NOTE: gas consumed will always be less than the limit
 	gasMeter.ConsumeGas(gasConsumed, "update gas consumption after refund")
 	k.WithContext(k.ctx.WithGasMeter(gasMeter))
 

x/evm/types/errors.go

@@ -22,6 +22,8 @@ const (
 	codeErrInvalidAmount
 	codeErrInvalidGasPrice
 	codeErrVMExecution
+	codeErrInvalidRefund
+	codeErrInconsistentGas
 )
 
 var (
@@ -63,6 +65,12 @@ var (
 
 	// ErrVMExecution returns an error resulting from an error in EVM execution.
 	ErrVMExecution = sdkerrors.Register(ModuleName, codeErrVMExecution, "evm transaction execution failed")
+
+	// ErrInvalidRefund returns an error if a the gas refund value is invalid.
+	ErrInvalidRefund = sdkerrors.Register(ModuleName, codeErrInvalidRefund, "invalid gas refund amount")
+
+	// ErrInconsistentGas returns an error if a the gas differs from the expected one.
+	ErrInconsistentGas = sdkerrors.Register(ModuleName, codeErrInconsistentGas, "inconsistent gas")
 )
 
 // NewExecErrorWithReson unpacks the revert return bytes and returns a wrapped error