Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ADR 010: Modular AnteHandler #4942

Merged
merged 16 commits into from
Aug 28, 2019
Merged
Changes from 11 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
173 changes: 173 additions & 0 deletions docs/architecture/adr-009-modular-antehandler.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,173 @@
# ADR 009: Modular AnteHandler
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved

## Changelog

- 2019 Aug 31: Initial draft

## Context

The current AnteHandler design allows users to either use the default AnteHandler provided in `x/auth` or to build their own AnteHandler from scratch. Ideally AnteHandler functionality is split into multiple, modular functions that can be chained together along with custom ante-functions so that users do not have to rewrite common antehandler logic when they want to implement custom behavior.

## Proposals

### Per-Module AnteHandler

One approach is to use the [ModuleManager](https://godoc.org/github.com/cosmos/cosmos-sdk/types/module) and have each module implement its own antehandler if it requires custom antehandler logic. The ModuleManager can then be passed in an AnteHandler order in the same way it has an order for BeginBlockers and EndBlockers. The ModuleManager returns a single AnteHandler function that will take in a tx and run each module's `AnteHandle` in the specified order. The module manager's AnteHandler is set as the baseapp's AnteHandler.

Pros:
1. Simple to implement
2. Utilizes the existing ModuleManager architecture

Cons:
1. Improves granularity but still cannot get more granular than a per-module basis. e.g. If auth's `AnteHandle` function is in charge of validating memo and signatures, users cannot swap the signature-checking functionality while keeping the rest of auth's `AnteHandle` functionality.
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
2. Module AnteHandler are run one after the other. There is no way for one AnteHandler to wrap or "decorate" another.

### Decorator Pattern

The [weave project](https://github.com/iov-one/weave) achieves AnteHandler modularity through the use of a decorator pattern. The interface is designed as follows:

```golang
// Decorator wraps a Handler to provide common functionality
// like authentication, or fee-handling, to many Handlers
type Decorator interface {
Check(ctx Context, store KVStore, tx Tx, next Checker) (*CheckResult, error)
Deliver(ctx Context, store KVStore, tx Tx, next Deliverer) (*DeliverResult, error)
}
```

Each decorator works like a modularized SDK antehandler function, but it can take in a `next` argument that may be another decorator or a Handler (which does not take in a next argument). These decorators can be chained together, one decorator being passed in as the `next` argument of the previous decorator in the chain. The chain ends in a Router which can take a tx and route to the appropriate msg handler.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

After reading this, it's a bit confusing to understand what we're talking about. Message handlers vs chained-antehandler calls?


A key benefit of this approach is that one Decorator can wrap its internal logic around the next Checker/Deliverer. A weave Decorator may do the following:
Copy link
Member Author

@AdityaSripal AdityaSripal Aug 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One example of where the decorator approach would be useful for us is to have a Decorator that wraps all other AnteHandlers with a defer clause that will catch an OutOfGas panic and return the appropriate sdk Result and abort.

In the per-module implementation, it was clear that having the same defer clause in every antehandler made no sense. The best way I found to handle this was to set the ctx.GasMeter in the module manager's antehandler and then place a defer clause there. However, doing this required changing the sdk.Tx interface to include a Gas method.
This can be seen here.

Having a decorator that wraps all subsequent antehandler functions to set the GasMeter and implements the defer clause correctly before calling next may help us avoid changing the sdk.Tx interface. However, any chain that uses something other than auth.StdTx will have to implement their own decorator.


```golang
AdityaSripal marked this conversation as resolved.
Show resolved Hide resolved
// Example Decorator's Deliver function
func (example Decorator) Deliver(ctx Context, store KVStore, tx Tx, next Deliverer) {
// Do some pre-processing logic

res, err := next.Deliver(ctx, store, tx)

// Do some post-processing logic given the result and error
}
```

Pros:
1. Weave Decorators can wrap over the next decorator/handler in the chain. The ability to both pre-process and post-process may be useful in certain settings.
2. Provides a nested modular structure that isn't possible in the solution above, while also allowing for a linear one-after-the-other structure like the solution above.

Cons:
1. It is hard to understand at first glance the state updates that would occur after a Decorator runs given the `ctx`, `store`, and `tx`. A Decorator can have an arbitrary number of nested Decorators being called within its function body, each possibly doing some pre- and post-processing before calling the next decorator on the chain. Thus to understand what a Decorator is doing, one must also understand what every other decorator further along the chain is also doing. This can get quite complicated to understand. A linear, one-after-the-other approach while less powerful, may be much easier to reason about.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is not much harder to reason about.

Decorators are short and should be well-documented.
The only difference with the chained approach is they allow for optional cleanup.
In many cases, they will be do_check(); next(), as in chain. But you can add defer() clauses to handle panics, or various other cleanups - eg. we can wrap the store to record all keys written, and then auto-add tags to the result.

But I guess "hard to reason about" is quite subjective here. I would recommend you refer to existing weave decorators you find confusing. There is only one I can think of, required by complex business logic from iov.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This isn't anything specific to weave. Long chains of middleware/decorator calls do not lend themselves to be easily "readable" -- this is what I think @AdityaSripal meant here and I don't think it's too subjective. We already use this pattern in certain places in the SDK and you have to have some context to understand the execution path.


### Chained Micro-Functions

The benefit of Weave's approach is that the Decorators can be very concise, which when chained together allows for maximum customizability. However, the nested structure can get quite complex and thus hard to reason about.

Our recommended approach is to split the AnteHandler functionality into tightly scoped "micro-functions", while preserving the one-after-the-other ordering that would come from the ModuleManager approach.

We can then have a way to chain these micro-functions so that they run one after the other. Modules may define multiple ante micro-functions and then also provide a default per-module AnteHandler that implements a default, suggested order for these micro-functions.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

May be worth adding another sentence here or depictionary pseudo-code discussing the various forms which the app may take. I'm assuming based on this first description one couldn't mix usage of the "full" ante-handler and the micro-functions... although maybe this would also be great? Something like:

mm.AnteHandlers(
   stakingModule.AnteHandler() // full ante handler
   Combo(bank.SignatureMicroFn, distribution.ValidateMemoMicroFn, ... ) // combination of microfunctions
   mintingModule.AnteHandler()
   ...
)

edit; I see you've gone into more detail, below, pseudo code earlier on would still be nice though

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't it be

Combo(bank.SignatureMicroFn, distribution.ValidateMemoMicroFn, ... ).AnteHandler()

I'm confused to what Combo does/is

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Combo is an invented function which just groups MicroFn's into an AnteHandler


Users can order the AnteHandlers easily by simply using the ModuleManager. The ModuleManager will take in a list of AnteHandlers and return a single AnteHandler that runs each AnteHandler in the order of the list provided. If the user is comfortable with the default ordering of each module, this is as simple as providing a list with each module's antehandler (exactly the same as BeginBlocker and EndBlocker).

If however, users wish to change the order or add, modify, or delete ante micro-functions in anyway; they can always define their own ante micro-functions and add them explicitly to the list that gets passed into module manager.
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved

#### Default Workflow:

##### SDK code:

```golang
func Chainer(order []AnteHandler) AnteHandler {
alexanderbez marked this conversation as resolved.
Show resolved Hide resolved
return func(ctx Context, tx Tx, simulate bool) (newCtx Context, err error) {
for _, ante := range order {
ctx, err := ante(ctx, tx, simulate)
if err != nil {
return ctx, err
}
}
return ctx, err
}
}
```

```golang
func VerifySignatures(ctx Context, tx Tx, simulate bool) (newCtx Context, err error) {
// verify signatures
// Returns InvalidSignature Result and abort=true if sigs invalid
// Return OK result and abort=false if sigs are valid
}

func ValidateMemo(ctx Context, tx Tx, simulate bool) (newCtx Context, err error) {
// validate memo
}

AuthModuleAnteHandler := Chainer([]AnteHandler{VerifySignatures, ValidateMemo})
```

```golang
func DeductFees(ctx Context, tx Tx, simulate bool) (newCtx Context, err error) {
// Deduct fees from tx
// Abort if insufficient funds in account to pay for fees
}

func CheckMempoolFees(ctx Context, tx Tx, simulate bool) (newCtx Context, err error) {
// If CheckTx: Abort if the fees are less than the mempool's minFee parameter
}

DistrModuleAnteHandler := Chainer([]AnteHandler{CheckMempoolFees, DeductFees})
```

```golang
type ModuleManager struct {
// other fields
AnteHandlerOrder []AnteHandler
}

func (mm ModuleManager) GetAnteHandler() AnteHandler {
retun Chainer(mm.AnteHandlerOrder)
}
```

##### User Code:

```golang
moduleManager.SetAnteHandlerOrder([]AnteHandler(AuthModuleAnteHandler, DistrModuleAnteHandler))
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I know you mention weave as hard to reason - but here, the actual stack is split over multiple modules, making it quite hard to see all the pieces and reason about it.

At the least, the entire flow should be visible in one place, to reduce the error of forgetting or duplicating some check (or placing in the wrong order).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah maybe? I like the idea of having all the "micro-functions" names listed in one place as you've suggested - which would make all operations visible simultaneously. I also like the idea of enclosing some more of the redundant microfunctions into larger "ante-handlers" default implementations from a module.

It's a tough call - I think allowing for both is not an unreasonable approach but maybe for Gaia we implement them fully exposed for good practice?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wouldn't mind seeing this piece of code broken out with more items, maybe combining "full" ante-handlers from modules, with combination ante-handlers (as I mentioned in my earlier comment)


app.SetAnteHandler(mm.GetAnteHandler())
```

#### Custom Workflow

##### User Code

```golang
func CustomSigVerify(ctx Context, tx Tx, simulate bool) (newCtx Context, err error) {
// do some custom signature verification logic
}
```

```golang
// Micro-functions allow users to change order of when they get executed, and swap out default ante-functionality with their own custom logic.
// Note that users can still chain the default distribution module handler, and auth micro-function along with their custom ante function
moduleManager.SetAnteHandlerOrder([]AnteHandler(ValidateMemo, CustomSigVerify, DistrModuleAnteHandler))
```

Pros:
1. Allows for ante functionality to be as modular as possible.
2. For users that do not need custom ante-functionality, there is little difference between how antehandlers work and how BeginBlock and EndBlock work in ModuleManager.
3. Still easy to understand

Cons:
1. Cannot wrap antehandlers with decorators like you can with Weave.

## Status

> Proposed

## Consequences

Since pros and cons are written for each approach, it is omitted from this section

## References

- [#4572](https://github.com/cosmos/cosmos-sdk/issues/4572): Modular AnteHandler Issue
- [#4582](https://github.com/cosmos/cosmos-sdk/pull/4583): Initial Implementation of Per-Module AnteHandler Approach
- [Weave Decorator Code](https://github.com/iov-one/weave/blob/master/handler.go#L35)
- [Weave Design Videos](https://vimeo.com/showcase/6189877)