diff --git a/CHANGELOG.md b/CHANGELOG.md index 5451c22d9f3..ed3cc7a78df 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -45,6 +45,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ ### State Machine Breaking ### Improvements +* [\#8303](https://github.com/cosmos/ibc-go/pull/8303) Prefix-based routing in IBCv2 Router ### Bug Fixes diff --git a/docs/docs/01-ibc/02-integration.md b/docs/docs/01-ibc/02-integration.md index dcf2e52efad..104909aad60 100644 --- a/docs/docs/01-ibc/02-integration.md +++ b/docs/docs/01-ibc/02-integration.md @@ -184,7 +184,7 @@ func NewApp(...args) *App { #### IBC v2 Router -With IBC v2, there is a new [router](https://github.com/cosmos/ibc-go/blob/main/modules/core/api/router.go) that needs to register the routes for a portID to a given IBCModule. +With IBC v2, there is a new [router](https://github.com/cosmos/ibc-go/blob/main/modules/core/api/router.go) that needs to register the routes for a portID to a given IBCModule. It routes IBCv2 messages based on the prefixes of port IDs. For example, if a route named `someModule` exists, messages addressed to port IDs like `someModuleRandomPort1`, `someModuleRandomPort2`, etc., will be passed to the corresponding module. ```go // IBC v2 router creation diff --git a/modules/core/api/router.go b/modules/core/api/router.go index 753bfc7f9c0..43b434d8cc5 100644 --- a/modules/core/api/router.go +++ b/modules/core/api/router.go @@ -3,13 +3,14 @@ package api import ( "errors" "fmt" + "strings" sdk "github.com/cosmos/cosmos-sdk/types" ) // Router contains all the module-defined callbacks required by IBC Protocol V2. type Router struct { - // routes is a map from portID to IBCModule + // routes is a map associating port prefixes to the IBCModules implementations. routes map[string]IBCModule } @@ -20,32 +21,57 @@ func NewRouter() *Router { } } -// AddRoute registers a route for a given portID to a given IBCModule. -func (rtr *Router) AddRoute(portID string, cbs IBCModule) *Router { - if !sdk.IsAlphaNumeric(portID) { +// AddRoute registers a route for a given port ID prefix to a given IBCModule. +// There can be up to one prefix registered for a given port ID in the router. +// +// Panics: +// - if a prefix of `portIDprefix` is already a registered route. +// - if `portIDprefix` is a prefix of already registered route. +func (rtr *Router) AddRoute(portIDprefix string, cbs IBCModule) *Router { + if !sdk.IsAlphaNumeric(portIDprefix) { panic(errors.New("route expressions can only contain alphanumeric characters")) } - if rtr.HasRoute(portID) { - panic(fmt.Errorf("route %s has already been registered", portID)) + for prefix := range rtr.routes { + // Prevent two scenarios: + // * Adding a string that prefix is already registered e.g. + // add prefix "portPrefix" and try to add "portPrefixSomeSuffix". + // * Adding a string that is a prefix of already registered route e.g. + // add prefix "portPrefix" and try to add "port". + if strings.HasPrefix(portIDprefix, prefix) { + panic(fmt.Errorf("route %s has already been covered by registered prefix: %s", portIDprefix, prefix)) + } + if strings.HasPrefix(prefix, portIDprefix) { + panic(fmt.Errorf("route %s is a prefix for already registered route: %s", portIDprefix, prefix)) + } } - rtr.routes[portID] = cbs + rtr.routes[portIDprefix] = cbs return rtr } // Route returns the IBCModule for a given portID. func (rtr *Router) Route(portID string) IBCModule { - route, ok := rtr.routes[portID] + _, route, ok := rtr.getRoute(portID) if !ok { panic(fmt.Sprintf("no route for %s", portID)) } return route } -// HasRoute returns true if the Router has a module registered for the portID or false otherwise. -func (rtr *Router) HasRoute(portID string) bool { - _, ok := rtr.routes[portID] - return ok +// HasRoute returns true along with a prefix if the router has a module +// registered for the given portID or its prefix. Returns false otherwise. +func (rtr *Router) HasRoute(portID string) (bool, string) { + prefix, _, ok := rtr.getRoute(portID) + return ok, prefix +} + +func (rtr *Router) getRoute(portID string) (string, IBCModule, bool) { + for prefix, module := range rtr.routes { + if strings.HasPrefix(portID, prefix) { + return prefix, module, true + } + } + return "", nil, false } diff --git a/modules/core/api/router_test.go b/modules/core/api/router_test.go index 9e0e8be5921..5d11a8d78be 100644 --- a/modules/core/api/router_test.go +++ b/modules/core/api/router_test.go @@ -35,13 +35,29 @@ func (suite *APITestSuite) TestRouter() { suite.Require().True(router.HasRoute("port03")) }, }, + { + name: "success: prefix based routing works", + malleate: func() { + router.AddRoute("somemodule", &mockv2.IBCModule{}) + router.AddRoute("port01", &mockv2.IBCModule{}) + }, + assertionFn: func() { + suite.Require().True(router.HasRoute("somemodule")) + suite.Require().True(router.HasRoute("somemoduleport01")) + ok, prefix := router.HasRoute("somemoduleport01") + suite.Require().Equal(true, ok) + suite.Require().Equal("somemodule", prefix) + suite.Require().NotNil(router.Route("somemoduleport01")) + suite.Require().True(router.HasRoute("port01")) + }, + }, { name: "failure: panics on duplicate module", malleate: func() { router.AddRoute("port01", &mockv2.IBCModule{}) }, assertionFn: func() { - suite.Require().PanicsWithError("route port01 has already been registered", func() { + suite.Require().PanicsWithError("route port01 has already been covered by registered prefix: port01", func() { router.AddRoute("port01", &mockv2.IBCModule{}) }) }, @@ -55,6 +71,26 @@ func (suite *APITestSuite) TestRouter() { }) }, }, + { + name: "failure: panics conflicting routes registered", + malleate: func() {}, + assertionFn: func() { + suite.Require().PanicsWithError("route someModuleWithSpecificPath has already been covered by registered prefix: someModule", func() { + router.AddRoute("someModule", &mockv2.IBCModule{}) + router.AddRoute("someModuleWithSpecificPath", &mockv2.IBCModule{}) + }) + }, + }, + { + name: "failure: panics conflicting routes registered, when shorter prefix is added", + malleate: func() {}, + assertionFn: func() { + suite.Require().PanicsWithError("route someLonger is a prefix for already registered route: someLongerPrefixModule", func() { + router.AddRoute("someLongerPrefixModule", &mockv2.IBCModule{}) + router.AddRoute("someLonger", &mockv2.IBCModule{}) + }) + }, + }, } for _, tc := range testCases { suite.Run(tc.name, func() {