fix: use full gas on overflow (#10897)

## Description

Investigating missing gas consumption

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [ ] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] added `!` to the type prefix if API or client breaking change
- [ ] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [ ] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [ ] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [ ] added a changelog entry to `CHANGELOG.md`
- [ ] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [ ] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)

(cherry picked from commit b0d3ef9)

# Conflicts:
#	x/auth/middleware/fee.go
#	x/auth/middleware/middleware.go

CHANGELOG.md

@@ -104,6 +104,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (bank) [\#10394](https://github.com/cosmos/cosmos-sdk/pull/10394) Fix: query account balance by ibc denom.
 * [\10608](https://github.com/cosmos/cosmos-sdk/pull/10608) Change the order of module migration by pushing x/auth to the end. Auth module depends on other modules and should be run last. We have updated the documentation to provide more details how to change module migration order. This is technically a breaking change, but only impacts updates between the upgrades with version change, hence migrating from the previous patch release doesn't cause new migration and doesn't break the state.
 * [\#10674](https://github.com/cosmos/cosmos-sdk/pull/10674) Fix issue with `Error.Wrap` and `Error.Wrapf` usage with `errors.Is`.
+* [\#10897](https://github.com/cosmos/cosmos-sdk/pull/10897) Fix: set a non-zero value on gas overflow.
 
 ## [v0.44.3](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.44.3) - 2021-10-21
 

store/types/gas.go

@@ -89,9 +89,9 @@ func addUint64Overflow(a, b uint64) (uint64, bool) {
 
 func (g *basicGasMeter) ConsumeGas(amount Gas, descriptor string) {
 	var overflow bool
-	// TODO: Should we set the consumed field after overflow checking?
 	g.consumed, overflow = addUint64Overflow(g.consumed, amount)
 	if overflow {
+		g.consumed = math.MaxUint64
 		panic(ErrorGasOverflow{descriptor})
 	}
 

x/auth/middleware/fee.go

@@ -0,0 +1,197 @@
+package middleware
+
+import (
+	"context"
+	"fmt"
+
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	"github.com/cosmos/cosmos-sdk/types/tx"
+	"github.com/cosmos/cosmos-sdk/x/auth/types"
+)
+
+var _ tx.Handler = mempoolFeeTxHandler{}
+
+type mempoolFeeTxHandler struct {
+	next tx.Handler
+}
+
+// MempoolFeeMiddleware will check if the transaction's fee is at least as large
+// as the local validator's minimum gasFee (defined in validator config).
+// If fee is too low, middleware returns error and tx is rejected from mempool.
+// Note this only applies when ctx.CheckTx = true
+// If fee is high enough or not CheckTx, then call next middleware
+// CONTRACT: Tx must implement FeeTx to use MempoolFeeMiddleware
+func MempoolFeeMiddleware(txh tx.Handler) tx.Handler {
+	return mempoolFeeTxHandler{
+		next: txh,
+	}
+}
+
+// CheckTx implements tx.Handler.CheckTx. It is responsible for determining if a
+// transaction's fees meet the required minimum of the processing node. Note, a
+// node can have zero fees set as the minimum. If non-zero minimum fees are set
+// and the transaction does not meet the minimum, the transaction is rejected.
+//
+// Recall, a transaction's fee is determined by ceil(minGasPrice * gasLimit).
+func (txh mempoolFeeTxHandler) CheckTx(ctx context.Context, req tx.Request, checkReq tx.RequestCheckTx) (tx.Response, tx.ResponseCheckTx, error) {
+	sdkCtx := sdk.UnwrapSDKContext(ctx)
+
+	feeTx, ok := req.Tx.(sdk.FeeTx)
+	if !ok {
+		return tx.Response{}, tx.ResponseCheckTx{}, sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx")
+	}
+
+	feeCoins := feeTx.GetFee()
+	gas := feeTx.GetGas()
+
+	// Ensure that the provided fees meet a minimum threshold for the validator,
+	// if this is a CheckTx. This is only for local mempool purposes, and thus
+	// is only ran on check tx.
+	minGasPrices := sdkCtx.MinGasPrices()
+	if !minGasPrices.IsZero() {
+		requiredFees := make(sdk.Coins, len(minGasPrices))
+
+		// Determine the required fees by multiplying each required minimum gas
+		// price by the gas limit, where fee = ceil(minGasPrice * gasLimit).
+		glDec := sdk.NewDec(int64(gas))
+		for i, gp := range minGasPrices {
+			fee := gp.Amount.Mul(glDec)
+			requiredFees[i] = sdk.NewCoin(gp.Denom, fee.Ceil().RoundInt())
+		}
+
+		if !feeCoins.IsAnyGTE(requiredFees) {
+			return tx.Response{}, tx.ResponseCheckTx{}, sdkerrors.Wrapf(sdkerrors.ErrInsufficientFee, "insufficient fees; got: %s required: %s", feeCoins, requiredFees)
+		}
+	}
+
+	return txh.next.CheckTx(ctx, req, checkReq)
+}
+
+// DeliverTx implements tx.Handler.DeliverTx.
+func (txh mempoolFeeTxHandler) DeliverTx(ctx context.Context, req tx.Request) (tx.Response, error) {
+	return txh.next.DeliverTx(ctx, req)
+}
+
+// SimulateTx implements tx.Handler.SimulateTx.
+func (txh mempoolFeeTxHandler) SimulateTx(ctx context.Context, req tx.Request) (tx.Response, error) {
+	return txh.next.SimulateTx(ctx, req)
+}
+
+var _ tx.Handler = deductFeeTxHandler{}
+
+type deductFeeTxHandler struct {
+	accountKeeper  AccountKeeper
+	bankKeeper     types.BankKeeper
+	feegrantKeeper FeegrantKeeper
+	next           tx.Handler
+}
+
+// DeductFeeMiddleware deducts fees from the first signer of the tx
+// If the first signer does not have the funds to pay for the fees, return with InsufficientFunds error
+// Call next middleware if fees successfully deducted
+// CONTRACT: Tx must implement FeeTx interface to use deductFeeTxHandler
+func DeductFeeMiddleware(ak AccountKeeper, bk types.BankKeeper, fk FeegrantKeeper) tx.Middleware {
+	return func(txh tx.Handler) tx.Handler {
+		return deductFeeTxHandler{
+			accountKeeper:  ak,
+			bankKeeper:     bk,
+			feegrantKeeper: fk,
+			next:           txh,
+		}
+	}
+}
+
+func (dfd deductFeeTxHandler) checkDeductFee(ctx context.Context, sdkTx sdk.Tx) error {
+	sdkCtx := sdk.UnwrapSDKContext(ctx)
+	feeTx, ok := sdkTx.(sdk.FeeTx)
+	if !ok {
+		return sdkerrors.Wrap(sdkerrors.ErrTxDecode, "Tx must be a FeeTx")
+	}
+
+	if addr := dfd.accountKeeper.GetModuleAddress(types.FeeCollectorName); addr == nil {
+		return fmt.Errorf("Fee collector module account (%s) has not been set", types.FeeCollectorName)
+	}
+
+	fee := feeTx.GetFee()
+	feePayer := feeTx.FeePayer()
+	feeGranter := feeTx.FeeGranter()
+
+	deductFeesFrom := feePayer
+
+	// if feegranter set deduct fee from feegranter account.
+	// this works with only when feegrant enabled.
+	if feeGranter != nil {
+		if dfd.feegrantKeeper == nil {
+			return sdkerrors.ErrInvalidRequest.Wrap("fee grants are not enabled")
+		} else if !feeGranter.Equals(feePayer) {
+			err := dfd.feegrantKeeper.UseGrantedFees(sdkCtx, feeGranter, feePayer, fee, sdkTx.GetMsgs())
+			if err != nil {
+				return sdkerrors.Wrapf(err, "%s does not not allow to pay fees for %s", feeGranter, feePayer)
+			}
+		}
+
+		deductFeesFrom = feeGranter
+	}
+
+	deductFeesFromAcc := dfd.accountKeeper.GetAccount(sdkCtx, deductFeesFrom)
+	if deductFeesFromAcc == nil {
+		return sdkerrors.ErrUnknownAddress.Wrapf("fee payer address: %s does not exist", deductFeesFrom)
+	}
+
+	// deduct the fees
+	if !feeTx.GetFee().IsZero() {
+		err := DeductFees(dfd.bankKeeper, sdkCtx, deductFeesFromAcc, feeTx.GetFee())
+		if err != nil {
+			return err
+		}
+	}
+
+	events := sdk.Events{sdk.NewEvent(sdk.EventTypeTx,
+		sdk.NewAttribute(sdk.AttributeKeyFee, feeTx.GetFee().String()),
+	)}
+	sdkCtx.EventManager().EmitEvents(events)
+
+	return nil
+}
+
+// CheckTx implements tx.Handler.CheckTx.
+func (dfd deductFeeTxHandler) CheckTx(ctx context.Context, req tx.Request, checkReq tx.RequestCheckTx) (tx.Response, tx.ResponseCheckTx, error) {
+	if err := dfd.checkDeductFee(ctx, req.Tx); err != nil {
+		return tx.Response{}, tx.ResponseCheckTx{}, err
+	}
+
+	return dfd.next.CheckTx(ctx, req, checkReq)
+}
+
+// DeliverTx implements tx.Handler.DeliverTx.
+func (dfd deductFeeTxHandler) DeliverTx(ctx context.Context, req tx.Request) (tx.Response, error) {
+	if err := dfd.checkDeductFee(ctx, req.Tx); err != nil {
+		return tx.Response{}, err
+	}
+
+	return dfd.next.DeliverTx(ctx, req)
+}
+
+func (dfd deductFeeTxHandler) SimulateTx(ctx context.Context, req tx.Request) (tx.Response, error) {
+	if err := dfd.checkDeductFee(ctx, req.Tx); err != nil {
+		return tx.Response{}, err
+	}
+
+	return dfd.next.SimulateTx(ctx, req)
+}
+
+// Deprecated: DeductFees deducts fees from the given account.
+// This function will be private in the next release.
+func DeductFees(bankKeeper types.BankKeeper, ctx sdk.Context, acc types.AccountI, fees sdk.Coins) error {
+	if !fees.IsValid() {
+		return sdkerrors.ErrInsufficientFee.Wrapf("invalid fee amount: %s", fees)
+	}
+
+	err := bankKeeper.SendCoinsFromAccountToModule(ctx, acc.GetAddress(), types.FeeCollectorName, fees)
+	if err != nil {
+		return sdkerrors.ErrInsufficientFunds.Wrap(err.Error())
+	}
+
+	return nil
+}

x/auth/middleware/middleware.go

@@ -0,0 +1,111 @@
+package middleware
+
+import (
+	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
+	"github.com/cosmos/cosmos-sdk/types/tx"
+	"github.com/cosmos/cosmos-sdk/types/tx/signing"
+	authsigning "github.com/cosmos/cosmos-sdk/x/auth/signing"
+	"github.com/cosmos/cosmos-sdk/x/auth/types"
+)
+
+// ComposeMiddlewares compose multiple middlewares on top of a tx.Handler. The
+// middleware order in the variadic arguments is from outer to inner.
+//
+// Example: Given a base tx.Handler H, and two middlewares A and B, the
+// middleware stack:
+// ```
+// A.pre
+//   B.pre
+//     H
+//   B.post
+// A.post
+// ```
+// is created by calling `ComposeMiddlewares(H, A, B)`.
+func ComposeMiddlewares(txHandler tx.Handler, middlewares ...tx.Middleware) tx.Handler {
+	for i := len(middlewares) - 1; i >= 0; i-- {
+		txHandler = middlewares[i](txHandler)
+	}
+
+	return txHandler
+}
+
+type TxHandlerOptions struct {
+	Debug bool
+
+	// TxDecoder is used to decode the raw tx bytes into a sdk.Tx.
+	TxDecoder sdk.TxDecoder
+
+	// IndexEvents defines the set of events in the form {eventType}.{attributeKey},
+	// which informs Tendermint what to index. If empty, all events will be indexed.
+	IndexEvents map[string]struct{}
+
+	LegacyRouter     sdk.Router
+	MsgServiceRouter *MsgServiceRouter
+
+	AccountKeeper   AccountKeeper
+	BankKeeper      types.BankKeeper
+	FeegrantKeeper  FeegrantKeeper
+	SignModeHandler authsigning.SignModeHandler
+	SigGasConsumer  func(meter sdk.GasMeter, sig signing.SignatureV2, params types.Params) error
+}
+
+// NewDefaultTxHandler defines a TxHandler middleware stacks that should work
+// for most applications.
+func NewDefaultTxHandler(options TxHandlerOptions) (tx.Handler, error) {
+	if options.TxDecoder == nil {
+		return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "txDecoder is required for middlewares")
+	}
+
+	if options.AccountKeeper == nil {
+		return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "account keeper is required for middlewares")
+	}
+
+	if options.BankKeeper == nil {
+		return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "bank keeper is required for middlewares")
+	}
+
+	if options.SignModeHandler == nil {
+		return nil, sdkerrors.Wrap(sdkerrors.ErrLogic, "sign mode handler is required for middlewares")
+	}
+
+	var sigGasConsumer = options.SigGasConsumer
+	if sigGasConsumer == nil {
+		sigGasConsumer = DefaultSigVerificationGasConsumer
+	}
+
+	return ComposeMiddlewares(
+		NewRunMsgsTxHandler(options.MsgServiceRouter, options.LegacyRouter),
+		NewTxDecoderMiddleware(options.TxDecoder),
+		// Set a new GasMeter on sdk.Context.
+		//
+		// Make sure the Gas middleware is outside of all other middlewares
+		// that reads the GasMeter. In our case, the Recovery middleware reads
+		// the GasMeter to populate GasInfo.
+		GasTxMiddleware,
+		// Recover from panics. Panics outside of this middleware won't be
+		// caught, be careful!
+		RecoveryTxMiddleware,
+		// Choose which events to index in Tendermint. Make sure no events are
+		// emitted outside of this middleware.
+		NewIndexEventsTxMiddleware(options.IndexEvents),
+		// Reject all extension options which can optionally be included in the
+		// tx.
+		RejectExtensionOptionsMiddleware,
+		MempoolFeeMiddleware,
+		ValidateBasicMiddleware,
+		TxTimeoutHeightMiddleware,
+		ValidateMemoMiddleware(options.AccountKeeper),
+		ConsumeTxSizeGasMiddleware(options.AccountKeeper),
+		// No gas should be consumed in any middleware above in a "post" handler part. See
+		// ComposeMiddlewares godoc for details.
+		DeductFeeMiddleware(options.AccountKeeper, options.BankKeeper, options.FeegrantKeeper),
+		TxPriorityMiddleware,
+		SetPubKeyMiddleware(options.AccountKeeper),
+		ValidateSigCountMiddleware(options.AccountKeeper),
+		SigGasConsumeMiddleware(options.AccountKeeper, sigGasConsumer),
+		SigVerificationMiddleware(options.AccountKeeper, options.SignModeHandler),
+		NewTipMiddleware(options.BankKeeper),
+		IncrementSequenceMiddleware(options.AccountKeeper),
+	), nil
+}