chore: implement OnChanUpgradeTry (#7092)

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

@@ -809,12 +809,9 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeTry() {
 		err := SetupICAPath(path, TestOwnerAddress)
 		suite.Require().NoError(err)
 
-		// call application callback directly
-		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(path.EndpointA.ChannelConfig.PortID)
 		suite.Require().True(ok)
+
 		cbs, ok := app.(porttypes.UpgradableModule)
 		suite.Require().True(ok)
 

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

@@ -710,11 +710,9 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeTry() {
 
 				tc.malleate() // malleate mutates test data
 
-				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(path.EndpointB.ChannelConfig.PortID)
 				suite.Require().True(ok)
+
 				cbs, ok := app.(porttypes.UpgradableModule)
 				suite.Require().True(ok)
 

modules/apps/29-fee/ibc_middleware.go

@@ -308,35 +308,11 @@ func (IBCMiddleware) OnChanUpgradeInit(
 }
 
 // OnChanUpgradeTry implements the IBCModule interface
-func (im IBCMiddleware) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, counterpartyVersion 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")
-	}
-
-	versionMetadata, err := types.MetadataFromVersion(counterpartyVersion)
-	if err != nil {
-		// since it is valid for fee version to not be specified, the counterparty 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.OnChanUpgradeTry(ctx, portID, channelID, proposedOrder, proposedConnectionHops, counterpartyVersion)
-	}
-
-	if versionMetadata.FeeVersion != types.Version {
-		return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, versionMetadata.FeeVersion)
-	}
-
-	appVersion, err := cbs.OnChanUpgradeTry(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
+func (IBCMiddleware) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, counterpartyVersion string) (string, error) {
+	if counterpartyVersion != types.Version {
+		return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, counterpartyVersion)
 	}
-
-	return string(versionBz), nil
+	return counterpartyVersion, nil
 }
 
 // OnChanUpgradeAck implements the IBCModule interface

modules/apps/callbacks/ibc_middleware.go

@@ -363,13 +363,8 @@ func (IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string
 }
 
 // OnChanUpgradeTry implements the IBCModule interface
-func (im IBCMiddleware) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, counterpartyVersion 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.OnChanUpgradeTry(ctx, portID, channelID, proposedOrder, proposedConnectionHops, counterpartyVersion)
+func (IBCMiddleware) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, counterpartyVersion string) (string, error) {
+	return "", nil
 }
 
 // OnChanUpgradeAck implements the IBCModule interface

modules/apps/transfer/ibc_module_test.go

@@ -621,10 +621,7 @@ func (suite *TransferTestSuite) TestOnChanUpgradeTry() {
 
 			tc.malleate()
 
-			module, _, err := suite.chainB.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainB.GetContext(), types.PortID)
-			suite.Require().NoError(err)
-
-			app, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.Route(module)
+			app, ok := suite.chainB.App.GetIBCKeeper().PortKeeper.AppRouter.HandshakeRoute(types.PortID)
 			suite.Require().True(ok)
 
 			cbs, ok := app.(porttypes.UpgradableModule)

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

@@ -292,8 +292,45 @@ func (im *LegacyIBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID
 }
 
 // OnChanUpgradeTry implements the IBCModule interface
-func (LegacyIBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, counterpartyVersion string) (string, error) {
-	return "", nil
+func (im *LegacyIBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, 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
+		}
+
+		// 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.OnChanUpgradeTry(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)
 }
 
 // OnChanUpgradeAck implements the IBCModule interface

modules/core/keeper/msg_server.go

@@ -703,7 +703,7 @@ func (k *Keeper) ChannelUpgradeInit(goCtx context.Context, msg *channeltypes.Msg
 func (k *Keeper) ChannelUpgradeTry(goCtx context.Context, msg *channeltypes.MsgChannelUpgradeTry) (*channeltypes.MsgChannelUpgradeTryResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
-	app, ok := k.PortKeeper.Route(msg.PortId)
+	app, ok := k.PortKeeper.AppRouter.HandshakeRoute(msg.PortId)
 	if !ok {
 		ctx.Logger().Error("channel upgrade 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)

modules/core/keeper/msg_server_test.go

@@ -1109,53 +1109,6 @@ func (suite *KeeperTestSuite) TestChannelUpgradeTry() {
 				ibctesting.AssertEvents(&suite.Suite, expEvents, 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.PortId = path.EndpointB.ChannelConfig.PortID
-				msg.ChannelId = path.EndpointB.ChannelID
-			},
-			func(res *channeltypes.MsgChannelUpgradeTryResponse, 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() {
-				suite.chainA.GetSimApp().IBCMockModule.IBCApp.OnChanUpgradeTry = func(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, counterpartyVersion string) (string, error) {
-					storeKey := suite.chainA.GetSimApp().GetKey(exported.ModuleName)
-					store := ctx.KVStore(storeKey)
-					store.Set(ibcmock.TestKey, ibcmock.TestValue)
-
-					ctx.EventManager().EmitEvent(sdk.NewEvent(ibcmock.MockEventType))
-					return ibcmock.UpgradeVersion, nil
-				}
-			},
-			func(res *channeltypes.MsgChannelUpgradeTryResponse, events []abci.Event, err error) {
-				suite.Require().NoError(err)
-				suite.Require().NotNil(res)
-				suite.Require().Equal(uint64(1), res.UpgradeSequence)
-
-				storeKey := suite.chainA.GetSimApp().GetKey(exported.ModuleName)
-				store := suite.chainA.GetContext().KVStore(storeKey)
-				suite.Require().Nil(store.Get(ibcmock.TestKey))
-
-				for _, event := range events {
-					if event.GetType() == ibcmock.MockEventType {
-						suite.Fail("expected application callback events to be discarded")
-					}
-				}
-			},
-		},
 	}
 
 	for _, tc := range cases {