Migrate antehandlers to middleware based design

[clevinson]

Currently AnteDecorators only apply to transaction pre-processing. There are many use cases for middleware where the whole transaction is wrapped (see #9569 #2150).

---

cc @aaronc

Proposed Implementation

We propose a 2-part implementation so that PRs are smaller and easier to review:

• [x] Refactor runTx and runMsgs into middlewares #9920
• [ ] Consider merging Response types https://github.com/tendermint/spec/issues/342
• [x] @atheeshp Refactor antehandlers into middlewares

[aaronc]

As a starting point for discussion I propose the following design:

type TxHandler interface {
    CheckTx(ctx Context, tx Tx, req abci.RequestCheckTx) (abci.ResponseCheckTx, error)
    DeliverTx(ctx Context, tx Tx, req abci.RequestDeliverTx) (abci.ResponseDeliverTx, error)
}

type TxMiddleware func(TxHandler) TxHandler

This uses a conventional middleware pattern where middleware is simply a function taking a handler and returning a new handler. next is not routed through as a parameter, rather every middleware handler keeps a reference to the next handler in the chain.

With this design the BaseApp runMsgs processing would be moved into middleware. Actually all of the CheckTx/DeliverTx logic could be handled as a middleware stack.

[aaronc]

@AdityaSripal any thoughts? @ethanfrey ?

[alexanderbez]

Seems reasonable to me. What are the main advantages here? Also, we must still ensure we keep CheckTx as lightweight as possible and also respect re-checktx.

[aaronc]

Seems reasonable to me. What are the main advantages here?

I would say:

• ability to configure what happens after Msg processing. The two use cases I've seen are:
  • refunding gas
  • running hooks/event listeners on events
• fully modular BaseApp

[alexanderbez]

Makes sense to me!

[AdityaSripal]

Looks good to me! I agree with @alexanderbez that we should be careful to design the middleware such that CheckTx is still just doing pre-processing

[amaury1093]

Checking in on this issue: it still has the Needs Architecture Review label, did we finally make a decision around Aaron's design https://github.com/cosmos/cosmos-sdk/issues/9585#issuecomment-868569351?

If it's agreed, we can probably put this as Ready and start working on it.

[aaronc]

I think we can put this into Ready

[tac0turtle]

love this work!! I am super excited for it! Should we get an uber quick ADR just for documentation purposes?

[amaury1093]

Should we get an uber quick ADR just for documentation purposes?

Sure! Also curious to hear what @ryanchristo thinks on ADR vs a docs page for this (i guess target audience for this refactor is app developers who want to create their own middlewares).

[ryanchristo]

I'm not sure if this is an either/or situation. It makes sense to have a docs page showing app developers how to create their own middleware but an ADR for the decision is not unreasonable.

I'm looking at https://docs.cosmos.network/v0.43/core/runtx_middleware.html as an example.

[robert-zaremba]

I think ADR is good to discuss a specification -- when we are still in a design mode. Otherwise the documentation is more important - because this is the place most of the developers will be looking at.

In one call we were talking about the process, and the general flow is:

1. start discussion to share and validate an idea
2. once the idea is validated and we have a draft of a solution -> start ADR to specify the solution
3. implement the solution and update documentation

Sometimes we do all steps at once by having prior discussions and then jumping directly to issue+implementation.

[amaury1093]

Yeah I'll create an ADR in parallel to the implementation started in #9920

[robert-zaremba]

Added "Consider merging Response types https://github.com/tendermint/spec/issues/342" to the checkbox list.

Tendermint extended the ResponseCheckTx by adding two new attributes: sender and priority. https://github.com/tendermint/spec/blob/master/proto/abci/types.proto#L197-L198