AnteDecorator

CHANGELOG.md

@@ -55,10 +55,23 @@ have this method perform a no-op.
   * Update gov keys to use big endian encoding instead of little endian
 * (modules) [\#5017](https://github.com/cosmos/cosmos-sdk/pull/5017) The `x/genaccounts` module has been
 deprecated and all components removed except the `legacy/` package.
+* (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) Modular `AnteHandler` via composable decorators:
+  * The `AnteHandler` interface now returns `(newCtx Context, err error)` instead of `(newCtx Context, result sdk.Result, abort bool)`
+  * The `NewAnteHandler` function returns an `AnteHandler` function that returns the new `AnteHandler`
+  interface and has been moved into the `auth/ante` directory.
+  * `ValidateSigCount`, `ValidateMemo`, `ProcessPubKey`, `EnsureSufficientMempoolFee`, and `GetSignBytes`
+  have all been removed as public functions.
+  * Invalid Signatures may return `InvalidPubKey` instead of `Unauthorized` error, since the transaction
+  will first hit `SetPubKeyDecorator` before the `SigVerificationDecorator` runs.
+  * `StdTx#GetSignatures` will return an array of just signature byte slices `[][]byte` instead of
+  returning an array of `StdSignature` structs. To replicate the old behavior, use the public field
+  `StdTx.Signatures` to get back the array of StdSignatures `[]StdSignature`.
 
 ### Client Breaking Changes
 
 * (rest) [\#4783](https://github.com/cosmos/cosmos-sdk/issues/4783) The balance field in the DelegationResponse type is now sdk.Coin instead of sdk.Int
+* (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) The gas required to pass the `AnteHandler` has
+increased significantly due to modular `AnteHandler` support. Increase GasLimit accordingly.
 
 ### Features
 
@@ -79,6 +92,29 @@ upgrade via: `sudo rm -rf /Library/Developer/CommandLineTools; xcode-select --in
 correct version via: `pkgutil --pkg-info=com.apple.pkg.CLTools_Executables`.
 * (keys) [\#5097](https://github.com/cosmos/cosmos-sdk/pull/5097) New `keys migrate` command to assist users migrate their keys
 to the new keyring.
+* (x/auth) [\#5006](https://github.com/cosmos/cosmos-sdk/pull/5006) Modular `AnteHandler` via composable decorators:
+  * The `AnteDecorator` interface has been introduced to allow users to implement modular `AnteHandler`
+  functionality that can be composed together to create a single `AnteHandler` rather than implementing
+  a custom `AnteHandler` completely from scratch, where each `AnteDecorator` allows for custom behavior in
+  tightly defined and logically isolated manner. These custom `AnteDecorator` can then be chained together
+  with default `AnteDecorator` or third-party `AnteDecorator` to create a modularized `AnteHandler`
+  which will run each `AnteDecorator` in the order specified in `ChainAnteDecorators`. For details
+  on the new architecture, refer to the [ADR](docs/architecture/adr-010-modular-antehandler.md).
+  * `ChainAnteDecorators` function has been introduced to take in a list of `AnteDecorators` and chain
+  them in sequence and return a single `AnteHandler`:
+    * `SetUpContextDecorator`: Sets `GasMeter` in context and creates defer clause to recover from any
+    `OutOfGas` panics in future AnteDecorators and return `OutOfGas` error to `BaseApp`. It MUST be the
+    first `AnteDecorator` in the chain for any application that uses gas (or another one that sets the gas meter).
+    * `ValidateBasicDecorator`: Calls tx.ValidateBasic and returns any non-nil error.
+    * `ValidateMemoDecorator`: Validates tx memo with application parameters and returns any non-nil error.
+    * `ConsumeGasTxSizeDecorator`: Consumes gas proportional to the tx size based on application parameters.
+    * `MempoolFeeDecorator`: Checks if fee is above local mempool `minFee` parameter during `CheckTx`.
+    * `DeductFeeDecorator`: Deducts the `FeeAmount` from first signer of the transaction.
+    * `SetPubKeyDecorator`: Sets pubkey of account in any account that does not already have pubkey saved in state machine.
+    * `SigGasConsumeDecorator`: Consume parameter-defined amount of gas for each signature.
+    * `SigVerificationDecorator`: Verify each signature is valid, return if there is an error.
+    * `ValidateSigCountDecorator`: Validate the number of signatures in tx based on app-parameters.
+    * `IncrementSequenceDecorator`: Increments the account sequence for each signer to prevent replay attacks.
 
 ### Improvements
 

baseapp/baseapp.go

@@ -585,7 +585,7 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk
 		// 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)
+		newCtx, err := 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
@@ -597,10 +597,14 @@ func (app *BaseApp) runTx(mode runTxMode, txBytes []byte, tx sdk.Tx) (result sdk
 			ctx = newCtx.WithMultiStore(ms)
 		}
 
-		gasWanted = result.GasWanted
+		// GasMeter expected to be set in AnteHandler
+		gasWanted = ctx.GasMeter().Limit()
 
-		if abort {
-			return result
+		if err != nil {
+			res := sdk.ResultFromError(err)
+			res.GasWanted = gasWanted
+			res.GasUsed = ctx.GasMeter().GasConsumed()
+			return res
 		}
 
 		msCache.Write()

baseapp/baseapp_test.go

@@ -19,6 +19,7 @@ import (
 	"github.com/cosmos/cosmos-sdk/store/rootmulti"
 	store "github.com/cosmos/cosmos-sdk/store/types"
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 )
 
 var (
@@ -594,15 +595,18 @@ func testTxDecoder(cdc *codec.Codec) sdk.TxDecoder {
 }
 
 func anteHandlerTxTest(t *testing.T, capKey *sdk.KVStoreKey, storeKey []byte) sdk.AnteHandler {
-	return func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) {
+	return func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) {
 		store := ctx.KVStore(capKey)
 		txTest := tx.(txTest)
 
 		if txTest.FailOnAnte {
-			return newCtx, sdk.ErrInternal("ante handler failure").Result(), true
+			return newCtx, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "ante handler failure")
 		}
 
-		res = incrementingCounter(t, store, storeKey, txTest.Counter)
+		res := incrementingCounter(t, store, storeKey, txTest.Counter)
+		if !res.IsOK() {
+			err = sdkerrors.ABCIError(string(res.Codespace), uint32(res.Code), res.Log)
+		}
 		return
 	}
 }
@@ -835,7 +839,7 @@ func TestSimulateTx(t *testing.T) {
 	gasConsumed := uint64(5)
 
 	anteOpt := func(bapp *BaseApp) {
-		bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) {
+		bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) {
 			newCtx = ctx.WithGasMeter(sdk.NewGasMeter(gasConsumed))
 			return
 		})
@@ -896,7 +900,7 @@ func TestSimulateTx(t *testing.T) {
 
 func TestRunInvalidTransaction(t *testing.T) {
 	anteOpt := func(bapp *BaseApp) {
-		bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) {
+		bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) {
 			return
 		})
 	}
@@ -980,17 +984,19 @@ func TestRunInvalidTransaction(t *testing.T) {
 func TestTxGasLimits(t *testing.T) {
 	gasGranted := uint64(10)
 	anteOpt := func(bapp *BaseApp) {
-		bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) {
+		bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) {
 			newCtx = ctx.WithGasMeter(sdk.NewGasMeter(gasGranted))
 
+			// 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
+			// the AnteHandler, but if it panics the context won't be set properly in
+			// runTx's recover call.
 			defer func() {
 				if r := recover(); r != nil {
 					switch rType := r.(type) {
 					case sdk.ErrorOutOfGas:
 						log := fmt.Sprintf("out of gas in location: %v", rType.Descriptor)
-						res = sdk.ErrOutOfGas(log).Result()
-						res.GasWanted = gasGranted
-						res.GasUsed = newCtx.GasMeter().GasConsumed()
+						err = sdkerrors.Wrap(sdkerrors.ErrOutOfGas, log)
 					default:
 						panic(r)
 					}
@@ -999,9 +1005,7 @@ func TestTxGasLimits(t *testing.T) {
 
 			count := tx.(*txTest).Counter
 			newCtx.GasMeter().ConsumeGas(uint64(count), "counter-ante")
-			res = sdk.Result{
-				GasWanted: gasGranted,
-			}
+
 			return
 		})
 
@@ -1065,17 +1069,14 @@ func TestTxGasLimits(t *testing.T) {
 func TestMaxBlockGasLimits(t *testing.T) {
 	gasGranted := uint64(10)
 	anteOpt := func(bapp *BaseApp) {
-		bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) {
+		bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) {
 			newCtx = ctx.WithGasMeter(sdk.NewGasMeter(gasGranted))
 
 			defer func() {
 				if r := recover(); r != nil {
 					switch rType := r.(type) {
 					case sdk.ErrorOutOfGas:
-						log := fmt.Sprintf("out of gas in location: %v", rType.Descriptor)
-						res = sdk.ErrOutOfGas(log).Result()
-						res.GasWanted = gasGranted
-						res.GasUsed = newCtx.GasMeter().GasConsumed()
+						err = sdkerrors.Wrapf(sdkerrors.ErrOutOfGas, "out of gas in location: %v", rType.Descriptor)
 					default:
 						panic(r)
 					}
@@ -1084,9 +1085,7 @@ func TestMaxBlockGasLimits(t *testing.T) {
 
 			count := tx.(*txTest).Counter
 			newCtx.GasMeter().ConsumeGas(uint64(count), "counter-ante")
-			res = sdk.Result{
-				GasWanted: gasGranted,
-			}
+
 			return
 		})
 
@@ -1234,17 +1233,15 @@ func TestBaseAppAnteHandler(t *testing.T) {
 func TestGasConsumptionBadTx(t *testing.T) {
 	gasWanted := uint64(5)
 	anteOpt := func(bapp *BaseApp) {
-		bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) {
+		bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) {
 			newCtx = ctx.WithGasMeter(sdk.NewGasMeter(gasWanted))
 
 			defer func() {
 				if r := recover(); r != nil {
 					switch rType := r.(type) {
 					case sdk.ErrorOutOfGas:
 						log := fmt.Sprintf("out of gas in location: %v", rType.Descriptor)
-						res = sdk.ErrOutOfGas(log).Result()
-						res.GasWanted = gasWanted
-						res.GasUsed = newCtx.GasMeter().GasConsumed()
+						err = sdkerrors.Wrap(sdkerrors.ErrOutOfGas, log)
 					default:
 						panic(r)
 					}
@@ -1254,12 +1251,9 @@ func TestGasConsumptionBadTx(t *testing.T) {
 			txTest := tx.(txTest)
 			newCtx.GasMeter().ConsumeGas(uint64(txTest.Counter), "counter-ante")
 			if txTest.FailOnAnte {
-				return newCtx, sdk.ErrInternal("ante handler failure").Result(), true
+				return newCtx, sdkerrors.Wrap(sdkerrors.ErrUnauthorized, "ante handler failure")
 			}
 
-			res = sdk.Result{
-				GasWanted: gasWanted,
-			}
 			return
 		})
 	}
@@ -1310,7 +1304,7 @@ func TestGasConsumptionBadTx(t *testing.T) {
 func TestQuery(t *testing.T) {
 	key, value := []byte("hello"), []byte("goodbye")
 	anteOpt := func(bapp *BaseApp) {
-		bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, res sdk.Result, abort bool) {
+		bapp.SetAnteHandler(func(ctx sdk.Context, tx sdk.Tx, simulate bool) (newCtx sdk.Context, err error) {
 			store := ctx.KVStore(capKey1)
 			store.Set(key, value)
 			return

docs/architecture/adr-010-modular-antehandler.md

@@ -191,16 +191,16 @@ type AnteDecorator interface {
 ```
 
 ```go
-// ChainDecorators will recursively link all of the AnteDecorators in the chain and return a final AnteHandler function
+// ChainAnteDecorators will recursively link all of the AnteDecorators in the chain and return a final AnteHandler function
 // This is done to preserve the ability to set a single AnteHandler function in the baseapp.
-func ChainDecorators(chain ...AnteDecorator) AnteHandler {
+func ChainAnteDecorators(chain ...AnteDecorator) AnteHandler {
     if len(chain) == 1 {
         return func(ctx Context, tx Tx, simulate bool) {
             chain[0].AnteHandle(ctx, tx, simulate, nil)
         }
     }
     return func(ctx Context, tx Tx, simulate bool) {
-        chain[0].AnteHandle(ctx, tx, simulate, ChainDecorators(chain[1:]))
+        chain[0].AnteHandle(ctx, tx, simulate, ChainAnteDecorators(chain[1:]))
     }
 }
 ```
@@ -211,9 +211,9 @@ Define AnteDecorator functions
 
 ```go
 // Setup GasMeter, catch OutOfGasPanic and handle appropriately
-type SetupDecorator struct{}
+type SetUpContextDecorator struct{}
 
-func (sud SetupDecorator) AnteHandle(ctx Context, tx Tx, simulate bool, next AnteHandler) (newCtx Context, err error) {
+func (sud SetUpContextDecorator) AnteHandle(ctx Context, tx Tx, simulate bool, next AnteHandler) (newCtx Context, err error) {
     ctx.GasMeter = NewGasMeter(tx.Gas)
 
     defer func() {
@@ -251,7 +251,7 @@ Link AnteDecorators to create a final AnteHandler. Set this AnteHandler in basea
 
 ```go
 // Create final antehandler by chaining the decorators together
-antehandler := ChainDecorators(NewSetupDecorator(), NewSigVerifyDecorator(), NewUserDefinedDecorator())
+antehandler := ChainAnteDecorators(NewSetUpContextDecorator(), NewSigVerifyDecorator(), NewUserDefinedDecorator())
 
 // Set chained Antehandler in the baseapp
 bapp.SetAnteHandler(antehandler)
@@ -264,7 +264,7 @@ Pros:
 
 Cons:
 
-1. Decorator pattern may have a deeply nested structure that is hard to understand, this is mitigated by having the decorator order explicitly listed in the `ChainDecorators` function.
+1. Decorator pattern may have a deeply nested structure that is hard to understand, this is mitigated by having the decorator order explicitly listed in the `ChainAnteDecorators` function.
 2. Does not make use of the ModuleManager design. Since this is already being used for BeginBlocker/EndBlocker, this proposal seems unaligned with that design pattern.
 
 ## Status

types/handler.go

@@ -5,4 +5,62 @@ type Handler func(ctx Context, msg Msg) Result
 
 // AnteHandler authenticates transactions, before their internal messages are handled.
 // If newCtx.IsZero(), ctx is used instead.
-type AnteHandler func(ctx Context, tx Tx, simulate bool) (newCtx Context, result Result, abort bool)
+type AnteHandler func(ctx Context, tx Tx, simulate bool) (newCtx Context, err error)
+
+// AnteDecorator wraps the next AnteHandler to perform custom pre- and post-processing.
+type AnteDecorator interface {
+	AnteHandle(ctx Context, tx Tx, simulate bool, next AnteHandler) (newCtx Context, err error)
+}
+
+// ChainDecorator chains AnteDecorators together with each AnteDecorator
+// wrapping over the decorators further along chain and returns a single AnteHandler.
+//
+// NOTE: The first element is outermost decorator, while the last element is innermost
+// decorator. Decorator ordering is critical since some decorators will expect
+// certain checks and updates to be performed (e.g. the Context) before the decorator
+// is run. These expectations should be documented clearly in a CONTRACT docline
+// in the decorator's godoc.
+//
+// NOTE: Any application that uses GasMeter to limit transaction processing cost
+// MUST set GasMeter with the FIRST AnteDecorator. Failing to do so will cause
+// transactions to be processed with an infinite gasmeter and open a DOS attack vector.
+// Use `ante.SetUpContextDecorator` or a custom Decorator with similar functionality.
+func ChainAnteDecorators(chain ...AnteDecorator) AnteHandler {
+	if (chain[len(chain)-1] != Terminator{}) {
+		chain = append(chain, Terminator{})
+	}
+
+	if len(chain) == 1 {
+		return func(ctx Context, tx Tx, simulate bool) (Context, error) {
+			return chain[0].AnteHandle(ctx, tx, simulate, nil)
+		}
+	}
+
+	return func(ctx Context, tx Tx, simulate bool) (Context, error) {
+		return chain[0].AnteHandle(ctx, tx, simulate, ChainAnteDecorators(chain[1:]...))
+	}
+}
+
+// Terminator AnteDecorator will get added to the chain to simplify decorator code
+// Don't need to check if next == nil further up the chain
+//                        ______
+//                     <((((((\\\
+//                     /      . }\
+//                     ;--..--._|}
+//  (\                 '--/\--'  )
+//   \\                | '-'  :'|
+//    \\               . -==- .-|
+//     \\               \.__.'   \--._
+//     [\\          __.--|       //  _/'--.
+//     \ \\       .'-._ ('-----'/ __/      \
+//      \ \\     /   __>|      | '--.       |
+//       \ \\   |   \   |     /    /       /
+//        \ '\ /     \  |     |  _/       /
+//         \  \       \ |     | /        /
+//   snd    \  \      \        /
+type Terminator struct{}
+
+// Simply return provided Context and nil error
+func (t Terminator) AnteHandle(ctx Context, _ Tx, _ bool, _ AnteHandler) (Context, error) {
+	return ctx, nil
+}

x/auth/alias.go

@@ -31,14 +31,9 @@ var (
 	// functions aliases
 	NewAnteHandler                    = ante.NewAnteHandler
 	GetSignerAcc                      = ante.GetSignerAcc
-	ValidateSigCount                  = ante.ValidateSigCount
-	ValidateMemo                      = ante.ValidateMemo
-	ProcessPubKey                     = ante.ProcessPubKey
 	DefaultSigVerificationGasConsumer = ante.DefaultSigVerificationGasConsumer
 	DeductFees                        = ante.DeductFees
-	EnsureSufficientMempoolFees       = ante.EnsureSufficientMempoolFees
 	SetGasMeter                       = ante.SetGasMeter
-	GetSignBytes                      = ante.GetSignBytes
 	NewAccountKeeper                  = keeper.NewAccountKeeper
 	NewQuerier                        = keeper.NewQuerier
 	NewBaseAccount                    = types.NewBaseAccount