From 1ffee197850993303ce4cbf4e754748b6095a172 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Thu, 26 Dec 2019 19:26:55 -0800 Subject: [PATCH 01/11] add ctx to router.Route() --- baseapp/baseapp.go | 2 +- baseapp/router.go | 2 +- baseapp/router_test.go | 2 +- types/router.go | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index ae1ca19db7f7..21e9c77b6905 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -647,7 +647,7 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (re for i, msg := range msgs { // match message route msgRoute := msg.Route() - handler := app.router.Route(msgRoute) + handler := app.router.Route(ctx, msgRoute) if handler == nil { return sdk.ErrUnknownRequest("unrecognized message type: " + msgRoute).Result() } diff --git a/baseapp/router.go b/baseapp/router.go index 96386e6eddca..382f4f4be95f 100644 --- a/baseapp/router.go +++ b/baseapp/router.go @@ -36,6 +36,6 @@ func (rtr *Router) AddRoute(path string, h sdk.Handler) sdk.Router { // Route returns a handler for a given route path. // // TODO: Handle expressive matches. -func (rtr *Router) Route(path string) sdk.Handler { +func (rtr *Router) Route(ctx sdk.Context, path string) sdk.Handler { return rtr.routes[path] } diff --git a/baseapp/router_test.go b/baseapp/router_test.go index 86e09cf21b79..88489df110e6 100644 --- a/baseapp/router_test.go +++ b/baseapp/router_test.go @@ -21,7 +21,7 @@ func TestRouter(t *testing.T) { }) rtr.AddRoute("testRoute", testHandler) - h := rtr.Route("testRoute") + h := rtr.Route(sdk.Context{}, "testRoute") require.NotNil(t, h) // require panic on duplicate route diff --git a/types/router.go b/types/router.go index c14255d4ec76..f3593f5530ff 100644 --- a/types/router.go +++ b/types/router.go @@ -9,7 +9,7 @@ var IsAlphaNumeric = regexp.MustCompile(`^[a-zA-Z0-9]+$`).MatchString // Router provides handlers for each transaction type. type Router interface { AddRoute(r string, h Handler) Router - Route(path string) Handler + Route(ctx Context, path string) Handler } // QueryRouter provides queryables for each query path. From c696236f68ea3ff89b97f35cb9b931a963ab6a6d Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Thu, 26 Dec 2019 19:34:19 -0800 Subject: [PATCH 02/11] CHANGELOG --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index ef0b35e5206d..7ab673ea1b30 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,6 +47,11 @@ logic has been implemented for v0.38 target version. Applications can migrate vi * (modules) [\#5299](https://github.com/cosmos/cosmos-sdk/pull/5299) Handling of `ABCIEvidenceTypeDuplicateVote` during `BeginBlock` along with the corresponding parameters (`MaxEvidenceAge`) have moved from the `x/slashing` module to the `x/evidence` module. +* (baseapp) [\#5299](https://github.com/cosmos/cosmos-sdk/pull/5299) Handling of `ABCIEvidenceTypeDuplicateVote` +during `BeginBlock` along with the corresponding parameters (`MaxEvidenceAge`) have moved from the +`x/slashing` module to the `x/evidence` module. +* (baseapp) [\#5452](https://github.com/cosmos/cosmos-sdk/pull/5452) An `sdk.Context` is passed into the `router.Route()` +function. ### API Breaking Changes From cc16035a516a436f079a7bea2182a79116a7f5fb Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Thu, 26 Dec 2019 21:05:58 -0800 Subject: [PATCH 03/11] added baseapp option --- baseapp/baseapp_test.go | 3 +++ baseapp/options.go | 8 ++++++++ 2 files changed, 11 insertions(+) diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 52df9bbef0ed..845f706c77df 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -416,6 +416,9 @@ func TestBaseAppOptionSeal(t *testing.T) { require.Panics(t, func() { app.SetFauxMerkleMode() }) + require.Panics(t, func() { + app.SetRouter(NewRouter()) + }) } func TestSetMinGasPrices(t *testing.T) { diff --git a/baseapp/options.go b/baseapp/options.go index 632a97850664..ea3b42ac2451 100644 --- a/baseapp/options.go +++ b/baseapp/options.go @@ -135,3 +135,11 @@ func (app *BaseApp) SetStoreLoader(loader StoreLoader) { } app.storeLoader = loader } + +// SetRouter allows us to customize the router. +func (app *BaseApp) SetRouter(router sdk.Router) { + if app.sealed { + panic("SetRouter() on sealed BaseApp") + } + app.router = router +} From d7bbc88d7b70a274947460cfa536d09b70a8d657 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Sun, 29 Dec 2019 15:40:00 -0800 Subject: [PATCH 04/11] added information to docs --- docs/core/baseapp.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/core/baseapp.md b/docs/core/baseapp.md index 8ef2dbcdfdcc..04dbdfc61b47 100644 --- a/docs/core/baseapp.md +++ b/docs/core/baseapp.md @@ -184,9 +184,9 @@ When messages and queries are received by the application, they must be routed t ### Message Routing -[`Message`s](#../building-modules/messages-and-queries.md#messages) need to be routed after they are extracted from transactions, which are sent from the underlying Tendermint engine via the [`CheckTx`](#checktx) and [`DeliverTx`](#delivertx) ABCI messages. To do so, `baseapp` holds a `router` which maps `paths` (`string`) to the appropriate module [`handler`](../building-modules/handler.md). Usually, the `path` is the name of the module. +[`Message`s](#../building-modules/messages-and-queries.md#messages) need to be routed after they are extracted from transactions, which are sent from the underlying Tendermint engine via the [`CheckTx`](#checktx) and [`DeliverTx`](#delivertx) ABCI messages. To do so, `baseapp` holds a `router` which maps `paths` (`string`) to the appropriate module [`handler`](../building-modules/handler.md) using the `.Route(ctx sdk.Context, path string)` function. Usually, the `path` is the name of the module. -+++ https://github.com/cosmos/cosmos-sdk/blob/master/baseapp/router.go +The [default router included in baseapp](https://github.com/cosmos/cosmos-sdk/blob/master/baseapp/router.go) is stateless. However, some applications may want to make use of more stateful routing mechanisms such as allowing governance to disable certain routes or point them to new modules for upgrade purposes. For this reason, the `sdk.Context` is also passed into the `Route` function of the [Router interface](https://github.com/cosmos/cosmos-sdk/blob/master/types/router.go#L12). For a stateless router that doesn't want to make use of this, can just ignore the ctx. The application's `router` is initilalized with all the routes using the application's [module manager](../building-modules/module-manager.md#manager), which itself is initialized with all the application's modules in the application's [constructor](../basics/app-anatomy.md#app-constructor). From 339f52532fa8bbe625a89663bb99c04201453472 Mon Sep 17 00:00:00 2001 From: PumpkinSeed Date: Wed, 1 Jan 2020 11:50:33 +0100 Subject: [PATCH 05/11] Add new definition for sdk.Router, add WithRouter to the BaseApp --- baseapp/baseapp.go | 7 ++++++- baseapp/router.go | 2 +- baseapp/router_test.go | 2 +- types/router.go | 2 +- 4 files changed, 9 insertions(+), 4 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index 46f3e2649907..bd58fd7ff684 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -221,6 +221,11 @@ func (app *BaseApp) LoadLatestVersion(baseKey *sdk.KVStoreKey) error { return app.initFromMainStore(baseKey) } +// WithRouter adds a new custom Router definition to the BaseApp +func (app *BaseApp) WithRouter(rtr sdk.Router) { + app.router = rtr +} + // DefaultStoreLoader will be used by default and loads the latest version func DefaultStoreLoader(ms sdk.CommitMultiStore) error { return ms.LoadLatestVersion() @@ -654,7 +659,7 @@ func (app *BaseApp) runMsgs(ctx sdk.Context, msgs []sdk.Msg, mode runTxMode) (*s } msgRoute := msg.Route() - handler := app.router.Route(msgRoute) + handler := app.router.Route(ctx, msgRoute) if handler == nil { return nil, sdkerrors.Wrapf(sdkerrors.ErrUnknownRequest, "unrecognized message route: %s; message index: %d", msgRoute, i) } diff --git a/baseapp/router.go b/baseapp/router.go index 96386e6eddca..382f4f4be95f 100644 --- a/baseapp/router.go +++ b/baseapp/router.go @@ -36,6 +36,6 @@ func (rtr *Router) AddRoute(path string, h sdk.Handler) sdk.Router { // Route returns a handler for a given route path. // // TODO: Handle expressive matches. -func (rtr *Router) Route(path string) sdk.Handler { +func (rtr *Router) Route(ctx sdk.Context, path string) sdk.Handler { return rtr.routes[path] } diff --git a/baseapp/router_test.go b/baseapp/router_test.go index 1a6d999bcce6..86b727568d5d 100644 --- a/baseapp/router_test.go +++ b/baseapp/router_test.go @@ -21,7 +21,7 @@ func TestRouter(t *testing.T) { }) rtr.AddRoute("testRoute", testHandler) - h := rtr.Route("testRoute") + h := rtr.Route(sdk.Context{}, "testRoute") require.NotNil(t, h) // require panic on duplicate route diff --git a/types/router.go b/types/router.go index c14255d4ec76..f3593f5530ff 100644 --- a/types/router.go +++ b/types/router.go @@ -9,7 +9,7 @@ var IsAlphaNumeric = regexp.MustCompile(`^[a-zA-Z0-9]+$`).MatchString // Router provides handlers for each transaction type. type Router interface { AddRoute(r string, h Handler) Router - Route(path string) Handler + Route(ctx Context, path string) Handler } // QueryRouter provides queryables for each query path. From 31290a9553c54de9a9be14568e135def449f5a8c Mon Sep 17 00:00:00 2001 From: PumpkinSeed Date: Wed, 1 Jan 2020 12:08:50 +0100 Subject: [PATCH 06/11] Add unit test for WithRouter --- baseapp/baseapp_test.go | 62 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 62 insertions(+) diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 01996e27ef40..1c5c5061896d 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -6,6 +6,7 @@ import ( "fmt" "io/ioutil" "os" + "sync" "testing" "github.com/stretchr/testify/assert" @@ -1443,3 +1444,64 @@ func TestGetMaximumBlockGas(t *testing.T) { app.setConsensusParams(&abci.ConsensusParams{Block: &abci.BlockParams{MaxGas: -5000000}}) require.Panics(t, func() { app.getMaximumBlockGas() }) } + +// NOTE: represents a new custom router for testing purposes of WithRouter() +type testCustomRouter struct { + routes sync.Map +} + +func (rtr *testCustomRouter) AddRoute(path string, h sdk.Handler) sdk.Router { + rtr.routes.Store(path, h) + return rtr +} + +func (rtr *testCustomRouter) Route(ctx sdk.Context, path string) sdk.Handler { + if v, ok := rtr.routes.Load(path); ok { + if h, ok := v.(sdk.Handler); ok { + return h + } + } + return nil +} + +func TestWithRouter(t *testing.T) { + // test increments in the ante + anteKey := []byte("ante-key") + anteOpt := func(bapp *BaseApp) { bapp.SetAnteHandler(anteHandlerTxTest(t, capKey1, anteKey)) } + + // test increments in the handler + deliverKey := []byte("deliver-key") + routerOpt := func(bapp *BaseApp) { + bapp.WithRouter(&testCustomRouter{routes:sync.Map{}}) + bapp.Router().AddRoute(routeMsgCounter, handlerMsgCounter(t, capKey1, deliverKey)) + } + + app := setupBaseApp(t, anteOpt, routerOpt) + app.InitChain(abci.RequestInitChain{}) + + // Create same codec used in txDecoder + codec := codec.New() + registerTestCodec(codec) + + nBlocks := 3 + txPerHeight := 5 + + for blockN := 0; blockN < nBlocks; blockN++ { + header := abci.Header{Height: int64(blockN) + 1} + app.BeginBlock(abci.RequestBeginBlock{Header: header}) + + for i := 0; i < txPerHeight; i++ { + counter := int64(blockN*txPerHeight + i) + tx := newTxCounter(counter, counter) + + txBytes, err := codec.MarshalBinaryLengthPrefixed(tx) + require.NoError(t, err) + + res := app.DeliverTx(abci.RequestDeliverTx{Tx: txBytes}) + require.True(t, res.IsOK(), fmt.Sprintf("%v", res)) + } + + app.EndBlock(abci.RequestEndBlock{}) + app.Commit() + } +} \ No newline at end of file From d295d27e607f162f7c2c9c5f330ba18c3120d323 Mon Sep 17 00:00:00 2001 From: Ferenc Fabian Date: Wed, 1 Jan 2020 13:40:00 +0100 Subject: [PATCH 07/11] Update baseapp/baseapp_test.go Co-Authored-By: Bot from GolangCI <42910462+golangcibot@users.noreply.github.com> --- baseapp/baseapp_test.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 1c5c5061896d..2d586991bf61 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -1472,7 +1472,7 @@ func TestWithRouter(t *testing.T) { // test increments in the handler deliverKey := []byte("deliver-key") routerOpt := func(bapp *BaseApp) { - bapp.WithRouter(&testCustomRouter{routes:sync.Map{}}) + bapp.WithRouter(&testCustomRouter{routes: sync.Map{}}) bapp.Router().AddRoute(routeMsgCounter, handlerMsgCounter(t, capKey1, deliverKey)) } @@ -1504,4 +1504,4 @@ func TestWithRouter(t *testing.T) { app.EndBlock(abci.RequestEndBlock{}) app.Commit() } -} \ No newline at end of file +} From 29111dfe2caa2189c50d5d13faad67e8ba2bd38a Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Wed, 1 Jan 2020 14:30:10 -0800 Subject: [PATCH 08/11] moved to API BREAKING in CHANGELOG --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0bed140c8e9c..2b72eab8df69 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -50,8 +50,6 @@ logic has been implemented for v0.38 target version. Applications can migrate vi * (baseapp) [\#5299](https://github.com/cosmos/cosmos-sdk/pull/5299) Handling of `ABCIEvidenceTypeDuplicateVote` during `BeginBlock` along with the corresponding parameters (`MaxEvidenceAge`) have moved from the `x/slashing` module to the `x/evidence` module. -* (baseapp) [\#5452](https://github.com/cosmos/cosmos-sdk/pull/5452) An `sdk.Context` is passed into the `router.Route()` -function. ### API Breaking Changes @@ -99,6 +97,8 @@ if the provided arguments are invalid. * (keys) [\#4941](https://github.com/cosmos/cosmos-sdk/issues/4941) Initializing a new keybase through `NewKeyringFromHomeFlag`, `NewKeyringFromDir`, `NewKeyBaseFromHomeFlag`, `NewKeyBaseFromDir`, or `NewInMemory` functions now accept optional parameters of type `KeybaseOption`. These optional parameters are also added on the keys subcommands functions, which are now public, and allows these options to be set on the commands or ignored to default to previous behavior. * The option introduced in this PR is `WithKeygenFunc` which allows a custom bytes to key implementation to be defined when keys are created. * (simapp) [\#5419](https://github.com/cosmos/cosmos-sdk/pull/5419) simapp/helpers.GenTx() now accepts a gas argument. +* (baseapp) [\#5455](https://github.com/cosmos/cosmos-sdk/issues/5455) An `sdk.Context` is passed into the `router.Route()` +function. ### Client Breaking Changes From 94616f1b31002f4f56141e0ad87aa500542945a6 Mon Sep 17 00:00:00 2001 From: Alexander Bezobchuk Date: Thu, 2 Jan 2020 08:51:08 -0500 Subject: [PATCH 09/11] Update baseapp/router.go --- baseapp/router.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/baseapp/router.go b/baseapp/router.go index 382f4f4be95f..77d2567c6cb2 100644 --- a/baseapp/router.go +++ b/baseapp/router.go @@ -36,6 +36,6 @@ func (rtr *Router) AddRoute(path string, h sdk.Handler) sdk.Router { // Route returns a handler for a given route path. // // TODO: Handle expressive matches. -func (rtr *Router) Route(ctx sdk.Context, path string) sdk.Handler { +func (rtr *Router) Route(_ sdk.Context, path string) sdk.Handler { return rtr.routes[path] } From f25a7567a34dbd9700b89f013589678e27afa0c5 Mon Sep 17 00:00:00 2001 From: PumpkinSeed Date: Thu, 2 Jan 2020 15:47:04 +0100 Subject: [PATCH 10/11] Remove WithRouter --- baseapp/baseapp.go | 5 ----- baseapp/baseapp_test.go | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/baseapp/baseapp.go b/baseapp/baseapp.go index bd58fd7ff684..3837a34ccc9a 100644 --- a/baseapp/baseapp.go +++ b/baseapp/baseapp.go @@ -221,11 +221,6 @@ func (app *BaseApp) LoadLatestVersion(baseKey *sdk.KVStoreKey) error { return app.initFromMainStore(baseKey) } -// WithRouter adds a new custom Router definition to the BaseApp -func (app *BaseApp) WithRouter(rtr sdk.Router) { - app.router = rtr -} - // DefaultStoreLoader will be used by default and loads the latest version func DefaultStoreLoader(ms sdk.CommitMultiStore) error { return ms.LoadLatestVersion() diff --git a/baseapp/baseapp_test.go b/baseapp/baseapp_test.go index 2c023735f526..179c80463bc4 100644 --- a/baseapp/baseapp_test.go +++ b/baseapp/baseapp_test.go @@ -1475,7 +1475,7 @@ func TestWithRouter(t *testing.T) { // test increments in the handler deliverKey := []byte("deliver-key") routerOpt := func(bapp *BaseApp) { - bapp.WithRouter(&testCustomRouter{routes: sync.Map{}}) + bapp.SetRouter(&testCustomRouter{routes: sync.Map{}}) bapp.Router().AddRoute(routeMsgCounter, handlerMsgCounter(t, capKey1, deliverKey)) } From de274dd6262609dc84d9dbd5a411c1333d401ce4 Mon Sep 17 00:00:00 2001 From: Sunny Aggarwal Date: Thu, 2 Jan 2020 18:24:24 -0500 Subject: [PATCH 11/11] Update CHANGELOG.md --- CHANGELOG.md | 3 --- 1 file changed, 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 2b72eab8df69..60b58a3c2327 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -47,9 +47,6 @@ logic has been implemented for v0.38 target version. Applications can migrate vi * (modules) [\#5299](https://github.com/cosmos/cosmos-sdk/pull/5299) Handling of `ABCIEvidenceTypeDuplicateVote` during `BeginBlock` along with the corresponding parameters (`MaxEvidenceAge`) have moved from the `x/slashing` module to the `x/evidence` module. -* (baseapp) [\#5299](https://github.com/cosmos/cosmos-sdk/pull/5299) Handling of `ABCIEvidenceTypeDuplicateVote` -during `BeginBlock` along with the corresponding parameters (`MaxEvidenceAge`) have moved from the -`x/slashing` module to the `x/evidence` module. ### API Breaking Changes