Skip to content

Commit

Permalink
Merge PR #5006: Modular AnteDecorator
Browse files Browse the repository at this point in the history
  • Loading branch information
@AdityaSripal @alexanderbez
AdityaSripal authored and alexanderbez committed Oct 10, 2019
1 parent 605f9d7 commit c0223a4
Show file tree
Hide file tree
Showing 22 changed files with 1,358 additions and 625 deletions.
36 changes: 36 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand All @@ -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

Expand Down
12 changes: 8 additions & 4 deletions baseapp/baseapp.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -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()
Expand Down
52 changes: 23 additions & 29 deletions baseapp/baseapp_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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 (
Expand Down Expand Up @@ -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
}
}
Expand Down Expand Up @@ -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
})
Expand Down Expand Up @@ -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
})
}
Expand Down Expand Up @@ -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)
}
Expand All @@ -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
})

Expand Down Expand Up @@ -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)
}
Expand All @@ -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
})

Expand Down Expand Up @@ -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)
}
Expand All @@ -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
})
}
Expand Down Expand Up @@ -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
Expand Down
14 changes: 7 additions & 7 deletions docs/architecture/adr-010-modular-antehandler.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:]))
}
}
```
Expand All @@ -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() {
Expand Down Expand Up @@ -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)
Expand All @@ -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
Expand Down
60 changes: 59 additions & 1 deletion types/handler.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
5 changes: 0 additions & 5 deletions x/auth/alias.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit c0223a4

Please sign in to comment.