Fix runTx Gas Consumption during Tx Aborting (#3244)

PENDING.md

@@ -24,6 +24,8 @@ BREAKING CHANGES
   * [stake] \#2513 Validator power type from Dec -> Int
   * [stake] \#3233 key and value now contain duplicate fields to simplify code
   * [\#3064](https://github.com/cosmos/cosmos-sdk/issues/3064) Sanitize `sdk.Coin` denom. Coins denoms are now case insensitive, i.e. 100fooToken equals to 100FOOTOKEN.
+  * [\#3242](https://github.com/cosmos/cosmos-sdk/issues/3242) Fix infinite gas
+  meter utilization during aborted ante handler executions.
 
 * Tendermint
 

baseapp/baseapp.go

@@ -503,6 +503,7 @@ func (app *BaseApp) BeginBlock(req abci.RequestBeginBlock) (res abci.ResponseBeg
 	} else {
 		gasMeter = sdk.NewInfiniteGasMeter()
 	}
+
 	app.deliverState.ctx = app.deliverState.ctx.WithBlockGasMeter(gasMeter)
 
 	if app.beginBlocker != nil {
@@ -732,7 +733,10 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk
 	defer func() {
 		if mode == runTxModeDeliver {
 			ctx.BlockGasMeter().ConsumeGas(
-				ctx.GasMeter().GasConsumedToLimit(), "block gas meter")
+				ctx.GasMeter().GasConsumedToLimit(),
+				"block gas meter",
+			)
+
 			if ctx.BlockGasMeter().GasConsumed() < startingGas {
 				panic(sdk.ErrorGasOverflow{"tx gas summation"})
 			}
@@ -751,23 +755,29 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk
 
 		// Cache wrap context before anteHandler call in case it aborts.
 		// This is required for both CheckTx and DeliverTx.
-		// https://github.com/cosmos/cosmos-sdk/issues/2772
+		// Ref: https://github.com/cosmos/cosmos-sdk/issues/2772
+		//
 		// NOTE: Alternatively, we could require that anteHandler ensures that
 		// writes do not happen if aborted/failed.  This may have some
 		// performance benefits, but it'll be more difficult to get right.
 		anteCtx, msCache = app.cacheTxContext(ctx, txBytes)
 
 		newCtx, result, abort := app.anteHandler(anteCtx, tx, (mode == runTxModeSimulate))
+		if !newCtx.IsZero() {
+			// At this point, newCtx.MultiStore() is cache-wrapped, or something else
+			// replaced by the ante handler. We want the original multistore, not one
+			// which was cache-wrapped for the ante handler.
+			//
+			// Also, in the case of the tx aborting, we need to track gas consumed via
+			// the instantiated gas meter in the ante handler, so we update the context
+			// prior to returning.
+			ctx = newCtx.WithMultiStore(ms)
+		}
+
 		if abort {
 			return result
 		}
-		if !newCtx.IsZero() {
+
-			// At this point, newCtx.MultiStore() is cache wrapped,
-			// or something else replaced by anteHandler.
-			// We want the original ms, not one which was cache-wrapped
-			// for the ante handler.
-			ctx = newCtx.WithMultiStore(ms)
-		}
 		msCache.Write()
 		gasWanted = result.GasWanted
 	}

x/auth/ante.go

@@ -29,7 +29,10 @@ func NewAnteHandler(ak AccountKeeper, fck FeeCollectionKeeper) sdk.AnteHandler {
 		// all transactions must be of type auth.StdTx
 		stdTx, ok := tx.(StdTx)
 		if !ok {
-			return ctx, sdk.ErrInternal("tx must be StdTx").Result(), true
+			// Set a gas meter with limit 0 as to prevent an infinite gas meter attack
+			// during runTx.
+			newCtx = SetGasMeter(simulate, ctx, 0)
+			return newCtx, sdk.ErrInternal("tx must be StdTx").Result(), true
 		}
 
 		params := ak.GetParams(ctx)
@@ -44,7 +47,7 @@ func NewAnteHandler(ak AccountKeeper, fck FeeCollectionKeeper) sdk.AnteHandler {
 			}
 		}
 
-		newCtx = SetGasMeter(simulate, ctx, stdTx)
+		newCtx = SetGasMeter(simulate, ctx, stdTx.Fee.Gas)
 
 		// AnteHandlers must have their own defer/recover in order for the BaseApp
 		// to know how much gas was used! This is because the GasMeter is created in
@@ -306,14 +309,14 @@ func EnsureSufficientMempoolFees(ctx sdk.Context, stdTx StdTx) sdk.Result {
 }
 
 // SetGasMeter returns a new context with a gas meter set from a given context.
-func SetGasMeter(simulate bool, ctx sdk.Context, stdTx StdTx) sdk.Context {
+func SetGasMeter(simulate bool, ctx sdk.Context, gasLimit uint64) sdk.Context {
 	// In various cases such as simulation and during the genesis block, we do not
 	// meter any gas utilization.
 	if simulate || ctx.BlockHeight() == 0 {
 		return ctx.WithGasMeter(sdk.NewInfiniteGasMeter())
 	}
 
-	return ctx.WithGasMeter(sdk.NewGasMeter(stdTx.Fee.Gas))
+	return ctx.WithGasMeter(sdk.NewGasMeter(gasLimit))
 }
 
 // GetSignBytes returns a slice of bytes to sign over for a given transaction