Use new port router for onchanupgradetry (#7067)

* chore: add fee implementation for OnChanOpenTry

* chore: fix wiring in app.gos

* chore: updated callbacks tests to handle new OnChanOpenTry

* chore: lint, correct error string

* chore: remove commented code

* chore: addressing PR feedback

modules/apps/27-interchain-accounts/host/ibc_module_test.go

@@ -203,10 +203,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenTry() {
 				// ensure channel on chainB is set in state
 				suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.SetChannel(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, *channel)
 
-				module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID)
-				suite.Require().NoError(err)
-
-				cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(module)
+				cbs, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.AppRouter.HandshakeRoute(icatypes.HostPortID)
 				suite.Require().True(ok)
 
 				version, err := cbs.OnChanOpenTry(suite.chainB.GetContext(), channel.Ordering, channel.ConnectionHops,

modules/apps/29-fee/fee_test.go

@@ -76,23 +76,29 @@ func RemoveFeeMiddleware(chain *ibctesting.TestChain) {
 	chain.GetSimApp().IBCKeeper.PortKeeper.Router = nil
 
 	newRouter := porttypes.NewRouter() // Create a new router
+	newAppRouter := porttypes.NewAppRouter()
+
 	// Remove Fee middleware from transfer module
 	chain.GetSimApp().TransferKeeper.WithICS4Wrapper(channelKeeper)
 	transferStack := transfer.NewIBCModule(chain.GetSimApp().TransferKeeper)
 	newRouter.AddRoute(transfertypes.ModuleName, transferStack)
+	newAppRouter.AddRoute(transfertypes.ModuleName, transferStack)
 
 	// Remove Fee middleware from icahost submodule
 	chain.GetSimApp().ICAHostKeeper.WithICS4Wrapper(channelKeeper)
 	icaHostStack := icahost.NewIBCModule(chain.GetSimApp().ICAHostKeeper)
 	newRouter.AddRoute(icahosttypes.SubModuleName, icaHostStack)
+	newAppRouter.AddRoute(icahosttypes.SubModuleName, icaHostStack)
 
 	// Remove Fee middleware from icacontroller submodule
 	chain.GetSimApp().ICAControllerKeeper.WithICS4Wrapper(channelKeeper)
 	icaControllerStack := icacontroller.NewIBCMiddleware(chain.GetSimApp().ICAControllerKeeper)
 	newRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerStack)
+	newAppRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerStack)
 
 	// Override and seal the router
 	chain.GetSimApp().IBCKeeper.SetRouter(newRouter)
+	chain.GetSimApp().IBCKeeper.SetAppRouter(newAppRouter)
 }
 
 // helper function

modules/apps/29-fee/ibc_middleware.go

@@ -70,33 +70,11 @@ func (im IBCMiddleware) OnChanOpenTry(
 	counterparty channeltypes.Counterparty,
 	counterpartyVersion string,
 ) (string, error) {
-	versionMetadata, err := types.MetadataFromVersion(counterpartyVersion)
-	if err != nil {
-		// Since it is valid for fee version to not be specified, the above middleware version may be for a middleware
-		// lower down in the stack. Thus, if it is not a fee version we pass the entire version string onto the underlying
-		// application.
-		return im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, counterparty, counterpartyVersion)
+	if counterpartyVersion != types.Version {
+		return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, counterpartyVersion)
 	}
-
-	if versionMetadata.FeeVersion != types.Version {
-		return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, versionMetadata.FeeVersion)
-	}
-
 	im.keeper.SetFeeEnabled(ctx, portID, channelID)
-
-	// call underlying app's OnChanOpenTry callback with the app versions
-	appVersion, err := im.app.OnChanOpenTry(ctx, order, connectionHops, portID, channelID, counterparty, versionMetadata.AppVersion)
-	if err != nil {
-		return "", err
-	}
-
-	versionMetadata.AppVersion = appVersion
-	versionBytes, err := types.ModuleCdc.MarshalJSON(&versionMetadata)
-	if err != nil {
-		return "", err
-	}
-
-	return string(versionBytes), nil
+	return counterpartyVersion, nil
 }
 
 // OnChanOpenAck implements the IBCMiddleware interface
@@ -487,7 +465,7 @@ func (IBCMiddleware) WrapVersion(cbVersion, underlyingAppVersion string) string
 	return string(versionBytes)
 }
 
-// UnwrapVersionUnsafe attempts to unmarshal the version string into a ics29 version. An error is returned if unsuccessful. 
+// UnwrapVersionUnsafe attempts to unmarshal the version string into a ics29 version. An error is returned if unsuccessful.
 func (IBCMiddleware) UnwrapVersionUnsafe(version string) (string, string, error) {
 	metadata, err := types.MetadataFromVersion(version)
 	if err != nil {

modules/apps/29-fee/ibc_middleware_test.go

@@ -159,10 +159,7 @@ func (suite *FeeTestSuite) TestOnChanOpenTry() {
 					Version:        tc.cpVersion,
 				}
 
-				module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.MockFeePort)
-				suite.Require().NoError(err)
-
-				cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module)
+				cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.AppRouter.HandshakeRoute(ibctesting.MockFeePort)
 				suite.Require().True(ok)
 
 				_, err = cbs.OnChanOpenTry(suite.chainA.GetContext(), channel.Ordering, channel.ConnectionHops,

modules/apps/callbacks/ibc_middleware.go

@@ -306,7 +306,7 @@ func (IBCMiddleware) processCallback(
 	return err
 }
 
-// OnChanOpenInit defers to the underlying application
+// OnChanOpenInit is a no-op for the callbacks middleware.
 func (IBCMiddleware) OnChanOpenInit(
 	ctx sdk.Context,
 	channelOrdering channeltypes.Order,
@@ -319,16 +319,16 @@ func (IBCMiddleware) OnChanOpenInit(
 	return "", nil
 }
 
-// OnChanOpenTry defers to the underlying application
-func (im IBCMiddleware) OnChanOpenTry(
+// OnChanOpenTry is a no-op for the callbacks middleware.
+func (IBCMiddleware) OnChanOpenTry(
 	ctx sdk.Context,
 	channelOrdering channeltypes.Order,
 	connectionHops []string, portID,
 	channelID string,
 	counterparty channeltypes.Counterparty,
 	counterpartyVersion string,
 ) (string, error) {
-	return im.app.OnChanOpenTry(ctx, channelOrdering, connectionHops, portID, channelID, counterparty, counterpartyVersion)
+	return "", nil
 }
 
 // OnChanOpenAck defers to the underlying application

modules/apps/callbacks/ibc_middleware_test.go

@@ -166,12 +166,10 @@ func (s *CallbacksTestSuite) TestSendPacket() {
 		s.Run(tc.name, func() {
 			s.SetupTransferTest()
 
-			cbs, ok := GetSimApp(s.chainA).IBCKeeper.PortKeeper.AppRoute(transfertypes.ModuleName)
+			cbs, ok := GetSimApp(s.chainA).IBCKeeper.PortKeeper.AppRouter.HandshakeRoute(transfertypes.ModuleName)
 			s.Require().True(ok)
 
-			s.Require().Len(cbs, 1, "expected 1 legacy module")
-
-			legacyModule, ok := cbs[0].(*porttypes.LegacyIBCModule)
+			legacyModule, ok := cbs.(*porttypes.LegacyIBCModule)
 			s.Require().True(ok, "expected there to be a single legacy ibc module")
 
 			legacyModuleCbs := legacyModule.GetCallbacks()

modules/apps/callbacks/testing/simapp/app.go

@@ -527,21 +527,26 @@ func NewSimApp(
 	var icaControllerStack porttypes.ClassicIBCModule
 	icaControllerStack = ibcmock.NewIBCModule(&mockModule, ibcmock.NewIBCApp("", scopedICAMockKeeper))
 	ibcAppRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerStack)
+	ibcAppRouter.AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack)
 	app.ICAAuthModule, ok = icaControllerStack.(ibcmock.IBCModule)
 	if !ok {
 		panic(fmt.Errorf("cannot convert %T to %T", icaControllerStack, app.ICAAuthModule))
 	}
 	icaControllerStack = icacontroller.NewIBCMiddlewareWithAuth(icaControllerStack, app.ICAControllerKeeper)
 	ibcAppRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerStack)
+	ibcAppRouter.AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack)
 	icaControllerStack = ibccallbacks.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper, app.MockContractKeeper, maxCallbackGas)
 	ibcAppRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerStack)
+	ibcAppRouter.AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack)
+
 	var icaICS4Wrapper porttypes.ICS4Wrapper
 	icaICS4Wrapper, ok = icaControllerStack.(porttypes.ICS4Wrapper)
 	if !ok {
 		panic(fmt.Errorf("cannot convert %T to %T", icaControllerStack, icaICS4Wrapper))
 	}
 	icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper)
 	ibcAppRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerStack)
+	ibcAppRouter.AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack)
 	// Since the callbacks middleware itself is an ics4wrapper, it needs to be passed to the ica controller keeper
 	app.ICAControllerKeeper.WithICS4Wrapper(icaICS4Wrapper)
 
@@ -563,8 +568,6 @@ func NewSimApp(
 		AddRoute(icahosttypes.SubModuleName, icaHostStack).
 		AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack) // ica with mock auth module stack route to ica (top level of middleware stack)
 
-	ibcAppRouter.AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack)
-
 	// Create Mock IBC Fee module stack for testing
 	// SendPacket, mock module cannot send packets
 

modules/apps/transfer/ibc_module_test.go

@@ -180,10 +180,7 @@ func (suite *TransferTestSuite) TestOnChanOpenTry() {
 			}
 			counterpartyVersion = types.V2
 
-			module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.TransferPort)
-			suite.Require().NoError(err)
-
-			cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module)
+			cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.AppRouter.HandshakeRoute(ibctesting.TransferPort)
 			suite.Require().True(ok)
 
 			tc.malleate() // explicitly change fields in channel and testChannel

modules/core/05-port/types/ibc_legacy_module.go

@@ -74,6 +74,50 @@ func (im *LegacyIBCModule) OnChanOpenInit(
 	return reconstructVersion(im.cbs, negotiatedVersions)
 }
 
+// OnChanOpenTry implements the IBCModule interface.
+func (im *LegacyIBCModule) OnChanOpenTry(
+	ctx sdk.Context,
+	order channeltypes.Order,
+	connectionHops []string,
+	portID,
+	channelID string,
+	counterparty channeltypes.Counterparty,
+	counterpartyVersion string,
+) (string, error) {
+	negotiatedVersions := make([]string, len(im.cbs))
+
+	for i := len(im.cbs) - 1; i >= 0; i-- {
+		cbVersion := counterpartyVersion
+
+		// To maintain backwards compatibility, we must handle two cases:
+		// - relayer provides empty version (use default versions)
+		// - relayer provides version which chooses to not enable a middleware
+		//
+		// If an application is a VersionWrapper which means it modifies the version string
+		// and the version string is non-empty (don't use default), then the application must
+		// attempt to unmarshal the version using the UnwrapVersionUnsafe interface function.
+		// If it is unsuccessful, no callback will occur to this application as the version
+		// indicates it should be disabled.
+		if wrapper, ok := im.cbs[i].(VersionWrapper); ok && strings.TrimSpace(counterpartyVersion) != "" {
+			appVersion, underlyingAppVersion, err := wrapper.UnwrapVersionUnsafe(counterpartyVersion)
+			if err != nil {
+				// middleware disabled
+				negotiatedVersions[i] = ""
+				continue
+			}
+			cbVersion, counterpartyVersion = appVersion, underlyingAppVersion
+		}
+
+		negotiatedVersion, err := im.cbs[i].OnChanOpenTry(ctx, order, connectionHops, portID, channelID, counterparty, cbVersion)
+		if err != nil {
+			return "", errorsmod.Wrapf(err, "channel open try callback failed for port ID: %s, channel ID: %s", portID, channelID)
+		}
+		negotiatedVersions[i] = negotiatedVersion
+	}
+
+	return reconstructVersion(im.cbs, negotiatedVersions)
+}
+
 // reconstructVersion will generate the channel version by applying any version wrapping as necessary.
 // Version wrapping will only occur if the negotiated version is non=empty and the application is a VersionWrapper.
 func reconstructVersion(cbs []ClassicIBCModule, negotiatedVersions []string) (string, error) {
@@ -90,19 +134,6 @@ func reconstructVersion(cbs []ClassicIBCModule, negotiatedVersions []string) (st
 	return version, nil
 }
 
-// OnChanOpenTry implements the IBCModule interface.
-func (LegacyIBCModule) OnChanOpenTry(
-	ctx sdk.Context,
-	order channeltypes.Order,
-	connectionHops []string,
-	portID,
-	channelID string,
-	counterparty channeltypes.Counterparty,
-	counterpartyVersion string,
-) (string, error) {
-	return "", nil
-}
-
 // OnChanOpenAck implements the IBCModule interface
 func (LegacyIBCModule) OnChanOpenAck(
 	ctx sdk.Context,

modules/core/keeper/msg_server.go

@@ -235,7 +235,7 @@ func (k *Keeper) ChannelOpenTry(goCtx context.Context, msg *channeltypes.MsgChan
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
 	// Retrieve application callbacks from router
-	cbs, ok := k.PortKeeper.Route(msg.PortId)
+	cbs, ok := k.PortKeeper.AppRouter.HandshakeRoute(msg.PortId)
 	if !ok {
 		ctx.Logger().Error("channel open try failed", "port-id", msg.PortId, "error", errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", msg.PortId))
 		return nil, errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", msg.PortId)

testing/simapp/app.go

@@ -480,22 +480,27 @@ func NewSimApp(
 	var icaControllerStack porttypes.ClassicIBCModule
 	icaControllerStack = ibcmock.NewIBCModule(&mockModule, ibcmock.NewIBCApp("", scopedICAMockKeeper))
 	ibcAppRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerStack)
+	ibcAppRouter.AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack)
 	var ok bool
 	app.ICAAuthModule, ok = icaControllerStack.(ibcmock.IBCModule)
 	if !ok {
 		panic(fmt.Errorf("cannot convert %T into %T", icaControllerStack, app.ICAAuthModule))
 	}
 	icaControllerStack = icacontroller.NewIBCMiddlewareWithAuth(icaControllerStack, app.ICAControllerKeeper)
 	ibcAppRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerStack)
+	ibcAppRouter.AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack)
 	icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper)
 	ibcAppRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerStack)
+	ibcAppRouter.AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack)
 
 	// RecvPacket, message that originates from core IBC and goes down to app, the flow is:
 	// channel.RecvPacket -> fee.OnRecvPacket -> icaHost.OnRecvPacket
 
 	var icaHostStack porttypes.ClassicIBCModule
 	icaHostStack = icahost.NewIBCModule(app.ICAHostKeeper)
+	ibcAppRouter.AddRoute(icahosttypes.SubModuleName, icaHostStack)
 	icaHostStack = ibcfee.NewIBCMiddleware(icaHostStack, app.IBCFeeKeeper)
+	ibcAppRouter.AddRoute(icahosttypes.SubModuleName, icaHostStack)
 
 	// Add host, controller & ica auth modules to IBC router
 	ibcRouter.
@@ -506,10 +511,6 @@ func NewSimApp(
 		AddRoute(icahosttypes.SubModuleName, icaHostStack).
 		AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack) // ica with mock auth module stack route to ica (top level of middleware stack)
 
-	ibcAppRouter.
-		AddRoute(icahosttypes.SubModuleName, icaHostStack).
-		AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack)
-
 	// Create Mock IBC Fee module stack for testing
 	// SendPacket, mock module cannot send packets