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

chore: change v2 router mapping to use port id rather than module name #7639

Merged
Show file tree
Hide file tree
Changes from all 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
46 changes: 14 additions & 32 deletions modules/core/api/router.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,13 +3,13 @@ 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 map[string]IBCModule
}

Expand All @@ -20,50 +20,32 @@ func NewRouter() *Router {
}
}

// AddRoute registers a route for a given module name.
func (rtr *Router) AddRoute(module string, cbs IBCModule) *Router {
if !sdk.IsAlphaNumeric(module) {
// 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) {
panic(errors.New("route expressions can only contain alphanumeric characters"))
}

if rtr.HasRoute(module) {
panic(fmt.Errorf("route %s has already been registered", module))
if rtr.HasRoute(portID) {
panic(fmt.Errorf("route %s has already been registered", portID))
}

rtr.routes[module] = cbs
rtr.routes[portID] = cbs

return rtr
}

// Route returns the IBCModule for a given module name.
func (rtr *Router) Route(module string) IBCModule {
route, ok := rtr.routeOrPrefixedRoute(module)
// Route returns the IBCModule for a given portID.
func (rtr *Router) Route(portID string) IBCModule {
route, ok := rtr.routes[portID]
if !ok {
panic(fmt.Sprintf("no route for %s", module))
panic(fmt.Sprintf("no route for %s", portID))
}
return route
}

// HasRoute returns true if the Router has a module registered or false otherwise.
func (rtr *Router) HasRoute(module string) bool {
_, ok := rtr.routeOrPrefixedRoute(module)
// 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
}

// routeOrPrefixedRoute returns the IBCModule for a given module name.
// if an exact match is not found, a route with the provided prefix is searched for instead.
func (rtr *Router) routeOrPrefixedRoute(module string) (IBCModule, bool) {
route, ok := rtr.routes[module]
if ok {
return route, true
}

// it's possible that some routes have been dynamically added e.g. with interchain accounts.
// in this case, we need to check if the module has the specified prefix.
for prefix, ibcModule := range rtr.routes {
if strings.HasPrefix(module, prefix) {
return ibcModule, true
}
}
return nil, false
}
39 changes: 14 additions & 25 deletions modules/core/api/router_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,53 +16,42 @@ func (suite *APITestSuite) TestRouter() {
{
name: "success",
malleate: func() {
router.AddRoute("module01", &mockv2.IBCModule{})
router.AddRoute("port01", &mockv2.IBCModule{})
},
assertionFn: func() {
suite.Require().True(router.HasRoute("module01"))
suite.Require().True(router.HasRoute("port01"))
},
},
{
name: "success: multiple modules",
malleate: func() {
router.AddRoute("module01", &mockv2.IBCModule{})
router.AddRoute("module02", &mockv2.IBCModule{})
router.AddRoute("module03", &mockv2.IBCModule{})
router.AddRoute("port01", &mockv2.IBCModule{})
router.AddRoute("port02", &mockv2.IBCModule{})
router.AddRoute("port03", &mockv2.IBCModule{})
},
assertionFn: func() {
suite.Require().True(router.HasRoute("module01"))
suite.Require().True(router.HasRoute("module02"))
suite.Require().True(router.HasRoute("module03"))
},
},
{
name: "success: find by prefix",
malleate: func() {
router.AddRoute("module01", &mockv2.IBCModule{})
},
assertionFn: func() {
suite.Require().True(router.HasRoute("module01-foo"))
suite.Require().True(router.HasRoute("port01"))
suite.Require().True(router.HasRoute("port02"))
suite.Require().True(router.HasRoute("port03"))
},
},
{
name: "failure: panics on duplicate module",
malleate: func() {
router.AddRoute("module01", &mockv2.IBCModule{})
router.AddRoute("port01", &mockv2.IBCModule{})
},
assertionFn: func() {
suite.Require().PanicsWithError("route module01 has already been registered", func() {
router.AddRoute("module01", &mockv2.IBCModule{})
suite.Require().PanicsWithError("route port01 has already been registered", func() {
router.AddRoute("port01", &mockv2.IBCModule{})
})
},
},
{
name: "failure: panics invalid-name",
malleate: func() {
router.AddRoute("module01", &mockv2.IBCModule{})
},
name: "failure: panics invalid-name",
malleate: func() {},
assertionFn: func() {
suite.Require().PanicsWithError("route expressions can only contain alphanumeric characters", func() {
router.AddRoute("module-02", &mockv2.IBCModule{})
router.AddRoute("port-02", &mockv2.IBCModule{})
})
},
},
Expand Down
2 changes: 1 addition & 1 deletion modules/light-clients/08-wasm/testing/simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -570,7 +570,7 @@ func NewSimApp(

// register the transfer v2 module.
app.TransferKeeperV2 = ibctransferkeeperv2.NewKeeper(app.TransferKeeper, app.IBCKeeper.ChannelKeeperV2)
ibcRouterV2.AddRoute(ibctransfertypes.ModuleName, transferv2.NewIBCModule(app.TransferKeeperV2))
ibcRouterV2.AddRoute(ibctransfertypes.PortID, transferv2.NewIBCModule(app.TransferKeeperV2))

// Seal the IBC Routers.
app.IBCKeeper.SetRouter(ibcRouter)
Expand Down
4 changes: 4 additions & 0 deletions testing/mock/v2/ibc_module.go
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,10 @@ const (
ModuleNameA = ModuleName + "A"
// ModuleNameB is a name that can be used for the second mock application.
ModuleNameB = ModuleName + "B"
// PortIDA is a port ID that can be used for the first mock application.
PortIDA = ModuleNameA
// PortIDB is a port ID that can be used for the second mock application.
PortIDB = ModuleNameB
)

// IBCModule is a mock implementation of the IBCModule interface.
Expand Down
6 changes: 3 additions & 3 deletions testing/simapp/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -480,16 +480,16 @@ func NewSimApp(

// create two separate mock v2 applications so that it is possible to test multi packet data.
mockV2A := mockv2.NewIBCModule()
ibcRouterV2.AddRoute(mockv2.ModuleNameA, mockV2A)
ibcRouterV2.AddRoute(mockv2.PortIDA, mockV2A)
app.MockModuleV2A = mockV2A

mockV2B := mockv2.NewIBCModule()
ibcRouterV2.AddRoute(mockv2.ModuleNameB, mockV2B)
ibcRouterV2.AddRoute(mockv2.PortIDB, mockV2B)
app.MockModuleV2B = mockV2B

// register the transfer v2 module.
app.TransferKeeperV2 = ibctransferkeeperv2.NewKeeper(app.TransferKeeper, app.IBCKeeper.ChannelKeeperV2)
ibcRouterV2.AddRoute(ibctransfertypes.ModuleName, transferv2.NewIBCModule(app.TransferKeeperV2))
ibcRouterV2.AddRoute(ibctransfertypes.PortID, transferv2.NewIBCModule(app.TransferKeeperV2))

// Seal the IBC Router
app.IBCKeeper.SetRouter(ibcRouter)
Expand Down
Loading