Use new port router for OnChanUpgradeInit (#7082)

* 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: implementing OnChanUpgradeInit

* chore: linter

* chore: removed redundant test

* chore: addressing PR feedback

modules/apps/27-interchain-accounts/controller/ibc_middleware.go

@@ -273,26 +273,7 @@ func (im IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID str
 		return "", types.ErrControllerSubModuleDisabled
 	}
 
-	proposedVersion, err := im.keeper.OnChanUpgradeInit(ctx, portID, channelID, proposedOrder, proposedConnectionHops, proposedVersion)
-	if err != nil {
-		return "", err
-	}
-
-	connectionID, err := im.keeper.GetConnectionID(ctx, portID, channelID)
-	if err != nil {
-		return "", err
-	}
-
-	if im.app != nil && im.keeper.IsMiddlewareEnabled(ctx, portID, connectionID) {
-		// Only cast to UpgradableModule if the application is set.
-		cbs, ok := im.app.(porttypes.UpgradableModule)
-		if !ok {
-			return "", errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack")
-		}
-		return cbs.OnChanUpgradeInit(ctx, portID, channelID, proposedOrder, proposedConnectionHops, proposedVersion)
-	}
-
-	return proposedVersion, nil
+	return im.keeper.OnChanUpgradeInit(ctx, portID, channelID, proposedOrder, proposedConnectionHops, proposedVersion)
 }
 
 // OnChanUpgradeTry implements the IBCModule interface

modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go

@@ -747,14 +747,6 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {
 				}
 			}, ibcmock.MockApplicationCallbackError,
 		},
-		{
-			"middleware disabled", func() {
-				suite.chainA.GetSimApp().ICAControllerKeeper.DeleteMiddlewareEnabled(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ConnectionID)
-				suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnChanUpgradeInit = func(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) {
-					return "", ibcmock.MockApplicationCallbackError
-				}
-			}, nil,
-		},
 	}
 
 	for _, ordering := range []channeltypes.Order{channeltypes.UNORDERED, channeltypes.ORDERED} {
@@ -775,10 +767,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {
 
 				tc.malleate() // malleate mutates test data
 
-				module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID)
-				suite.Require().NoError(err)
-
-				app, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module)
+				app, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.AppRouter.HandshakeRoute(types.SubModuleName)
 				suite.Require().True(ok)
 				cbs, ok := app.(porttypes.UpgradableModule)
 				suite.Require().True(ok)

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

@@ -658,11 +658,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {
 		err := SetupICAPath(path, TestOwnerAddress)
 		suite.Require().NoError(err)
 
-		// call application callback directly
-		module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID)
-		suite.Require().NoError(err)
-
-		app, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(module)
+		app, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.AppRouter.HandshakeRoute(types.SubModuleName)
 		suite.Require().True(ok)
 		cbs, ok := app.(porttypes.UpgradableModule)
 		suite.Require().True(ok)

modules/apps/29-fee/ibc_middleware.go

@@ -294,42 +294,18 @@ func (im IBCMiddleware) OnTimeoutPacket(
 }
 
 // OnChanUpgradeInit implements the IBCModule interface
-func (im IBCMiddleware) OnChanUpgradeInit(
+func (IBCMiddleware) OnChanUpgradeInit(
 	ctx sdk.Context,
 	portID string,
 	channelID string,
 	proposedOrder channeltypes.Order,
 	proposedConnectionHops []string,
 	proposedVersion string,
 ) (string, error) {
-	cbs, ok := im.app.(porttypes.UpgradableModule)
-	if !ok {
-		return "", errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack")
+	if proposedVersion != types.Version {
+		return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, proposedVersion)
 	}
-
-	versionMetadata, err := types.MetadataFromVersion(proposedVersion)
-	if err != nil {
-		// since it is valid for fee version to not be specified, the upgrade version may be for a middleware
-		// or application further down in the stack. Thus, pass through to next middleware or application in callstack.
-		return cbs.OnChanUpgradeInit(ctx, portID, channelID, proposedOrder, proposedConnectionHops, proposedVersion)
-	}
-
-	if versionMetadata.FeeVersion != types.Version {
-		return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, versionMetadata.FeeVersion)
-	}
-
-	appVersion, err := cbs.OnChanUpgradeInit(ctx, portID, channelID, proposedOrder, proposedConnectionHops, versionMetadata.AppVersion)
-	if err != nil {
-		return "", err
-	}
-
-	versionMetadata.AppVersion = appVersion
-	versionBz, err := types.ModuleCdc.MarshalJSON(&versionMetadata)
-	if err != nil {
-		return "", err
-	}
-
-	return string(versionBz), nil
+	return types.Version, nil
 }
 
 // OnChanUpgradeTry implements the IBCModule interface

modules/apps/callbacks/ibc_middleware.go

@@ -358,13 +358,8 @@ func (im IBCMiddleware) OnChanCloseConfirm(ctx sdk.Context, portID, channelID st
 }
 
 // OnChanUpgradeInit implements the IBCModule interface
-func (im IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) (string, error) {
-	cbs, ok := im.app.(porttypes.UpgradableModule)
-	if !ok {
-		return "", errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack")
-	}
-
-	return cbs.OnChanUpgradeInit(ctx, portID, channelID, proposedOrder, proposedConnectionHops, proposedVersion)
+func (IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) (string, error) {
+	return "", nil
 }
 
 // OnChanUpgradeTry implements the IBCModule interface

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

@@ -126,22 +126,6 @@ func (im *LegacyIBCModule) OnChanOpenTry(
 	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) {
-	version := negotiatedVersions[0] // base version
-	for i := 1; i < len(cbs); i++ {  // iterate over the remaining callbacks
-		if strings.TrimSpace(negotiatedVersions[i]) != "" {
-			wrapper, ok := cbs[i].(VersionWrapper)
-			if !ok {
-				return "", ibcerrors.ErrInvalidVersion
-			}
-			version = wrapper.WrapVersion(negotiatedVersions[i], version)
-		}
-	}
-	return version, nil
-}
-
 // OnChanOpenAck implements the IBCModule interface.
 // NOTE: The callback will occur for all applications in the callback list.
 // If the application is provided an empty string for the counterparty version,
@@ -260,8 +244,45 @@ func (LegacyIBCModule) OnTimeoutPacket(
 }
 
 // OnChanUpgradeInit implements the IBCModule interface
-func (LegacyIBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) (string, error) {
-	return "", nil
+func (im *LegacyIBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedVersion string) (string, error) {
+	negotiatedVersions := make([]string, len(im.cbs))
+
+	for i := len(im.cbs) - 1; i >= 0; i-- {
+		cbVersion := proposedVersion
+
+		// 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(proposedVersion) != "" {
+			appVersion, underlyingAppVersion, err := wrapper.UnwrapVersionUnsafe(proposedVersion)
+			if err != nil {
+				// middleware disabled
+				negotiatedVersions[i] = ""
+				continue
+			}
+			cbVersion, proposedVersion = appVersion, underlyingAppVersion
+		}
+
+		// in order to maintain backwards compatibility, every callback in the stack must implement the UpgradableModule interface.
+		upgradableModule, ok := im.cbs[i].(UpgradableModule)
+		if !ok {
+			return "", errorsmod.Wrap(ErrInvalidRoute, "upgrade route not found to module in application callstack")
+		}
+
+		negotiatedVersion, err := upgradableModule.OnChanUpgradeInit(ctx, portID, channelID, proposedOrder, proposedConnectionHops, cbVersion)
+		if err != nil {
+			return "", errorsmod.Wrapf(err, "channel open init callback failed for port ID: %s, channel ID: %s", portID, channelID)
+		}
+		negotiatedVersions[i] = negotiatedVersion
+	}
+
+	return reconstructVersion(im.cbs, negotiatedVersions)
 }
 
 // OnChanUpgradeTry implements the IBCModule interface
@@ -284,3 +305,19 @@ func (LegacyIBCModule) OnChanUpgradeOpen(ctx sdk.Context, portID, channelID stri
 func (LegacyIBCModule) UnmarshalPacketData(ctx sdk.Context, portID, channelID string, bz []byte) (interface{}, error) {
 	return nil, nil
 }
+
+// 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) {
+	version := negotiatedVersions[0] // base version
+	for i := 1; i < len(cbs); i++ {  // iterate over the remaining callbacks
+		if strings.TrimSpace(negotiatedVersions[i]) != "" {
+			wrapper, ok := cbs[i].(VersionWrapper)
+			if !ok {
+				return "", ibcerrors.ErrInvalidVersion
+			}
+			version = wrapper.WrapVersion(negotiatedVersions[i], version)
+		}
+	}
+	return version, nil
+}

modules/core/keeper/msg_server.go

@@ -661,7 +661,7 @@ func (k *Keeper) ChannelUpgradeInit(goCtx context.Context, msg *channeltypes.Msg
 		return nil, errorsmod.Wrapf(ibcerrors.ErrUnauthorized, "expected %s, got %s", k.GetAuthority(), msg.Signer)
 	}
 
-	app, ok := k.PortKeeper.Route(msg.PortId)
+	app, ok := k.PortKeeper.AppRouter.HandshakeRoute(msg.PortId)
 	if !ok {
 		ctx.Logger().Error("channel upgrade init 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)

modules/core/keeper/msg_server_test.go

@@ -974,29 +974,6 @@ func (suite *KeeperTestSuite) TestChannelUpgradeInit() {
 				suite.Require().Empty(events)
 			},
 		},
-		{
-			"ibc application does not implement the UpgradeableModule interface",
-			func() {
-				path = ibctesting.NewPath(suite.chainA, suite.chainB)
-				path.EndpointA.ChannelConfig.PortID = ibcmock.MockBlockUpgrade
-				path.EndpointB.ChannelConfig.PortID = ibcmock.MockBlockUpgrade
-
-				path.Setup()
-
-				msg = channeltypes.NewMsgChannelUpgradeInit(
-					path.EndpointA.ChannelConfig.PortID,
-					path.EndpointA.ChannelID,
-					path.EndpointA.GetProposedUpgrade().Fields,
-					path.EndpointA.Chain.GetSimApp().IBCKeeper.GetAuthority(),
-				)
-			},
-			func(res *channeltypes.MsgChannelUpgradeInitResponse, events []abci.Event, err error) {
-				suite.Require().ErrorIs(err, porttypes.ErrInvalidRoute)
-				suite.Require().Nil(res)
-
-				suite.Require().Empty(events)
-			},
-		},
 		{
 			"ibc application does not commit state changes in callback",
 			func() {