Skip to content

Commit

Permalink
refactor!: BaseApp {Check,Deliver}Tx with middleware design (cosmos#9920
Browse files Browse the repository at this point in the history
)

<!--
The default pull request template is for types feat, fix, or refactor.
For other templates, add one of the following parameters to the url:
- template=docs.md
- template=other.md
-->

## Description

ref: cosmos#9585 

This PR if the 1st step (out of 2) in the cosmos#9585 refactor. It transforms baseapp's ABCI {Check,Deliver}Tx into middleware stacks. A middleware is defined by the following interfaces:

```go
// types/tx package

type Handler interface {
	CheckTx(ctx context.Context, tx sdk.Tx, req abci.RequestCheckTx) (abci.ResponseCheckTx, error)
	DeliverTx(ctx context.Context, tx sdk.Tx, req abci.RequestDeliverTx) (abci.ResponseDeliverTx, error)
	SimulateTx(ctx context.Context, tx sdk.Tx, req RequestSimulateTx) (ResponseSimulateTx, error)
}

type Middleware func(Handler) Handler
```

This Pr doesn't migrate antehandlers, but only baseapp's runTx and runMsgs as middlewares. It bundles antehandlers into one single middleware (for now).

More specifically, it introduces the 5 following middlewares:

| Middleware              | Description                                                                                                                                                                                                                                                                                                                                                                                            |
| ----------------------- | ------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------ |
| RunMsgsTxMiddleware     | This middleware replaces the old baseapp's `runMsgs`.                                                                                                                                                                                                                                                                                                                                                  |
| LegacyAnteMiddleware    | This middleware is temporary, it bundles all antehandlers into one middleware. It's only created for the purpose of breaking the refactor into 2 pieces. **It will be removed in Part 2**, and each antehandler will be replaced by its own middleware.                                                                                                                                                                                                  |
| IndexEventsTxMiddleware | This is a simple middleware that chooses which events to index in Tendermint. Replaces `baseapp.indexEvents` (which unfortunately still exists in baseapp too, because it's used to index Begin/EndBlock events)                                                                                                                                                                                        |
| RecoveryTxMiddleware    | This index recovers from panics. It replaces baseapp.runTx's panic recovery.                                                                                                                                                                                                                                                                                                                           |
| GasTxMiddleware         | This replaces the [`Setup`](https://github.com/cosmos/cosmos-sdk/blob/master/x/auth/ante/setup.go) Antehandler. It sets a GasMeter on sdk.Context. Note that before, GasMeter was set on sdk.Context inside the antehandlers, and there was some mess around the fact that antehandlers had their own panic recovery system so that the GasMeter could be read by baseapp's recovery system. Now, this mess is all removed: one middleware sets GasMeter, another one handles recovery. |

---

### Author Checklist

*All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.*

I have...

- [x] included the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [x] added `!` to the type prefix if API or client breaking change
- [x] targeted the correct branch (see [PR Targeting](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#pr-targeting))
- [x] provided a link to the relevant issue or specification
- [ ] followed the guidelines for [building modules](https://github.com/cosmos/cosmos-sdk/blob/master/docs/building-modules)
- [x] included the necessary unit and integration [tests](https://github.com/cosmos/cosmos-sdk/blob/master/CONTRIBUTING.md#testing)
- [x] added a changelog entry to `CHANGELOG.md`
- [x] included comments for [documenting Go code](https://blog.golang.org/godoc)
- [ ] updated the relevant documentation or specification
- [x] reviewed "Files changed" and left comments if necessary
- [ ] confirmed all CI checks have passed

### Reviewers Checklist

*All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.*

I have...

- [ ] confirmed the correct [type prefix](https://github.com/commitizen/conventional-commit-types/blob/v3.0.0/index.json) in the PR title
- [ ] confirmed `!` in the type prefix if API or client breaking change
- [ ] confirmed all author checklist items have been addressed 
- [ ] reviewed state machine logic
- [ ] reviewed API design and naming
- [ ] reviewed documentation is accurate
- [ ] reviewed tests and test coverage
- [ ] manually tested (if applicable)
  • Loading branch information
amaury1093 authored Aug 25, 2021
1 parent d1ac629 commit b858627
Show file tree
Hide file tree
Showing 3 changed files with 40 additions and 15 deletions.
49 changes: 36 additions & 13 deletions app.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@ import (
"github.com/cosmos/cosmos-sdk/client/grpc/tmservice"
"github.com/cosmos/cosmos-sdk/codec"
"github.com/cosmos/cosmos-sdk/codec/types"
"github.com/cosmos/cosmos-sdk/server"
"github.com/cosmos/cosmos-sdk/server/api"
"github.com/cosmos/cosmos-sdk/server/config"
servertypes "github.com/cosmos/cosmos-sdk/server/types"
Expand All @@ -42,6 +43,7 @@ import (
capabilitykeeper "github.com/cosmos/cosmos-sdk/x/capability/keeper"
capabilitytypes "github.com/cosmos/cosmos-sdk/x/capability/types"

authmiddleware "github.com/cosmos/cosmos-sdk/x/auth/middleware"
"github.com/cosmos/cosmos-sdk/x/authz"
authzkeeper "github.com/cosmos/cosmos-sdk/x/authz/keeper"
authzmodule "github.com/cosmos/cosmos-sdk/x/authz/module"
Expand Down Expand Up @@ -140,6 +142,8 @@ type SimApp struct {
legacyAmino *codec.LegacyAmino
appCodec codec.Codec
interfaceRegistry types.InterfaceRegistry
msgSvcRouter *authmiddleware.MsgServiceRouter
legacyRouter sdk.Router

invCheckPeriod uint

Expand Down Expand Up @@ -216,6 +220,8 @@ func NewSimApp(
legacyAmino: legacyAmino,
appCodec: appCodec,
interfaceRegistry: interfaceRegistry,
legacyRouter: authmiddleware.NewLegacyRouter(),
msgSvcRouter: authmiddleware.NewMsgServiceRouter(interfaceRegistry),
invCheckPeriod: invCheckPeriod,
keys: keys,
tkeys: tkeys,
Expand Down Expand Up @@ -266,7 +272,7 @@ func NewSimApp(
stakingtypes.NewMultiStakingHooks(app.DistrKeeper.Hooks(), app.SlashingKeeper.Hooks()),
)

app.AuthzKeeper = authzkeeper.NewKeeper(keys[authzkeeper.StoreKey], appCodec, app.BaseApp.MsgServiceRouter())
app.AuthzKeeper = authzkeeper.NewKeeper(keys[authzkeeper.StoreKey], appCodec, app.msgSvcRouter)

// register the proposal types
govRouter := govtypes.NewRouter()
Expand Down Expand Up @@ -346,8 +352,8 @@ func NewSimApp(
)

app.mm.RegisterInvariants(&app.CrisisKeeper)
app.mm.RegisterRoutes(app.Router(), app.QueryRouter(), encodingConfig.Amino)
app.configurator = module.NewConfigurator(app.appCodec, app.MsgServiceRouter(), app.GRPCQueryRouter())
app.mm.RegisterRoutes(app.legacyRouter, app.QueryRouter(), encodingConfig.Amino)
app.configurator = module.NewConfigurator(app.appCodec, app.msgSvcRouter, app.GRPCQueryRouter())
app.mm.RegisterServices(app.configurator)

// add test gRPC service for testing gRPC queries in isolation
Expand Down Expand Up @@ -382,31 +388,48 @@ func NewSimApp(
// initialize BaseApp
app.SetInitChainer(app.InitChainer)
app.SetBeginBlocker(app.BeginBlocker)
app.SetEndBlocker(app.EndBlocker)
app.setTxHandler(encodingConfig.TxConfig, cast.ToStringSlice(appOpts.Get(server.FlagIndexEvents)))

if loadLatest {
if err := app.LoadLatestVersion(); err != nil {
tmos.Exit(err.Error())
}
}

return app
}

func (app *SimApp) setTxHandler(txConfig client.TxConfig, indexEventsStr []string) {
anteHandler, err := ante.NewAnteHandler(
ante.HandlerOptions{
AccountKeeper: app.AccountKeeper,
BankKeeper: app.BankKeeper,
SignModeHandler: encodingConfig.TxConfig.SignModeHandler(),
SignModeHandler: txConfig.SignModeHandler(),
FeegrantKeeper: app.FeeGrantKeeper,
SigGasConsumer: ante.DefaultSigVerificationGasConsumer,
},
)

if err != nil {
panic(err)
}

app.SetAnteHandler(anteHandler)
app.SetEndBlocker(app.EndBlocker)

if loadLatest {
if err := app.LoadLatestVersion(); err != nil {
tmos.Exit(err.Error())
}
indexEvents := map[string]struct{}{}
for _, e := range indexEventsStr {
indexEvents[e] = struct{}{}
}
txHandler, err := authmiddleware.NewDefaultTxHandler(authmiddleware.TxHandlerOptions{
Debug: app.Trace(),
IndexEvents: indexEvents,
LegacyRouter: app.legacyRouter,
MsgServiceRouter: app.msgSvcRouter,
LegacyAnteHandler: anteHandler,
})
if err != nil {
panic(err)
}

return app
app.SetTxHandler(txHandler)
}

// Name returns the name of the App
Expand Down
4 changes: 3 additions & 1 deletion app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import (
sdk "github.com/cosmos/cosmos-sdk/types"
"github.com/cosmos/cosmos-sdk/types/module"
"github.com/cosmos/cosmos-sdk/x/auth"
authmiddleware "github.com/cosmos/cosmos-sdk/x/auth/middleware"
"github.com/cosmos/cosmos-sdk/x/auth/vesting"
authzmodule "github.com/cosmos/cosmos-sdk/x/authz/module"
"github.com/cosmos/cosmos-sdk/x/bank"
Expand Down Expand Up @@ -82,8 +83,9 @@ func TestRunMigrations(t *testing.T) {
bApp := baseapp.NewBaseApp(appName, logger, db, encCfg.TxConfig.TxDecoder())
bApp.SetCommitMultiStoreTracer(nil)
bApp.SetInterfaceRegistry(encCfg.InterfaceRegistry)
msr := authmiddleware.NewMsgServiceRouter(encCfg.InterfaceRegistry)
app.BaseApp = bApp
app.configurator = module.NewConfigurator(app.appCodec, app.MsgServiceRouter(), app.GRPCQueryRouter())
app.configurator = module.NewConfigurator(app.appCodec, msr, app.GRPCQueryRouter())

// We register all modules on the Configurator, except x/bank. x/bank will
// serve as the test subject on which we run the migration tests.
Expand Down
2 changes: 1 addition & 1 deletion test_helpers.go
Original file line number Diff line number Diff line change
Expand Up @@ -352,7 +352,7 @@ func SignCheckDeliver(

// Simulate a sending a transaction and committing a block
app.BeginBlock(abci.RequestBeginBlock{Header: header})
gInfo, res, err := app.Deliver(txCfg.TxEncoder(), tx)
gInfo, res, err := app.SimDeliver(txCfg.TxEncoder(), tx)

if expPass {
require.NoError(t, err)
Expand Down

0 comments on commit b858627

Please sign in to comment.