diff --git a/go.mod b/go.mod index af5704ac2b..55467374d0 100644 --- a/go.mod +++ b/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 diff --git a/go.sum b/go.sum index 8ce0bf9202..7511e80720 100644 --- a/go.sum +++ b/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= diff --git a/x/evm/keeper/keeper.go b/x/evm/keeper/keeper.go index 4aaae459cc..586081ba02 100644 --- a/x/evm/keeper/keeper.go +++ b/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)) } // ---------------------------------------------------------------------------- diff --git a/x/evm/keeper/msg_server.go b/x/evm/keeper/msg_server.go index 0af33f67ff..15d2189b97 100644 --- a/x/evm/keeper/msg_server.go +++ b/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() { diff --git a/x/evm/keeper/state_transition.go b/x/evm/keeper/state_transition.go index 88abe21c85..d9fba220f1 100644 --- a/x/evm/keeper/state_transition.go +++ b/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)) diff --git a/x/evm/types/errors.go b/x/evm/types/errors.go index c9f0b2f167..eae5727b8d 100644 --- a/x/evm/types/errors.go +++ b/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