Use new port router for OnChanUpgradeAck (#7094)

* chore: implement OnChanUpgradeTry

* wip: partial implementation for OnChanUpgradeAck

* chore: updated tests to work with new OnChanUpgradeAck implementation

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

@@ -287,25 +287,7 @@ func (im IBCMiddleware) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, cou
 		return types.ErrControllerSubModuleDisabled
 	}
 
-	if err := im.keeper.OnChanUpgradeAck(ctx, portID, channelID, counterpartyVersion); 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.OnChanUpgradeAck(ctx, portID, channelID, counterpartyVersion)
-	}
-
-	return nil
+	return im.keeper.OnChanUpgradeAck(ctx, portID, channelID, counterpartyVersion)
 }
 
 // OnChanUpgradeOpen implements the IBCModule interface

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

@@ -825,7 +825,6 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeTry() {
 func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeAck() {
 	var (
 		path                *ibctesting.Path
-		isNilApp            bool
 		counterpartyVersion string
 	)
 
@@ -839,9 +838,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeAck() {
 		},
 		{
 			"success: nil underlying app",
-			func() {
-				isNilApp = true
-			},
+			func() {},
 			nil,
 		},
 		{
@@ -861,14 +858,6 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeAck() {
 				}
 			}, 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.OnChanUpgradeAck = func(ctx sdk.Context, portID, channelID string, counterpartyVersion string) error {
-					return ibcmock.MockApplicationCallbackError
-				}
-			}, nil,
-		},
 	}
 
 	for _, ordering := range []channeltypes.Order{channeltypes.UNORDERED, channeltypes.ORDERED} {
@@ -877,7 +866,6 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeAck() {
 
 			suite.Run(tc.name, func() {
 				suite.SetupTest() // reset
-				isNilApp = false
 
 				path = NewICAPath(suite.chainA, suite.chainB, ordering)
 				path.SetupConnections()
@@ -889,18 +877,12 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeAck() {
 
 				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(path.EndpointA.ChannelConfig.PortID)
 				suite.Require().True(ok)
+
 				cbs, ok := app.(porttypes.UpgradableModule)
 				suite.Require().True(ok)
 
-				if isNilApp {
-					cbs = controller.NewIBCMiddleware(suite.chainA.GetSimApp().ICAControllerKeeper)
-				}
-
 				err = cbs.OnChanUpgradeAck(
 					suite.chainA.GetContext(),
 					path.EndpointA.ChannelConfig.PortID,

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

@@ -745,10 +745,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeAck() {
 		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(path.EndpointB.ChannelConfig.PortID)
 		suite.Require().True(ok)
 		cbs, ok := app.(porttypes.UpgradableModule)
 		suite.Require().True(ok)

modules/apps/29-fee/ibc_middleware.go

@@ -312,25 +312,11 @@ func (IBCMiddleware) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string,
 }
 
 // OnChanUpgradeAck implements the IBCModule interface
-func (im IBCMiddleware) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion 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.OnChanUpgradeAck(ctx, portID, channelID, counterpartyVersion)
-	}
-
-	if versionMetadata.FeeVersion != types.Version {
-		return errorsmod.Wrapf(types.ErrInvalidVersion, "expected counterparty fee version: %s, got: %s", types.Version, versionMetadata.FeeVersion)
+func (IBCMiddleware) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion string) error {
+	if counterpartyVersion != types.Version {
+		return errorsmod.Wrapf(types.ErrInvalidVersion, "expected counterparty fee version: %s, got: %s", types.Version, counterpartyVersion)
 	}
-
-	// call underlying app's OnChanUpgradeAck callback with the counterparty app version.
-	return cbs.OnChanUpgradeAck(ctx, portID, channelID, versionMetadata.AppVersion)
+	return nil
 }
 
 // OnChanUpgradeOpen implements the IBCModule interface

modules/apps/29-fee/ibc_middleware_test.go

@@ -1274,10 +1274,7 @@ func (suite *FeeTestSuite) TestOnChanUpgradeAck() {
 
 			counterpartyUpgrade := path.EndpointB.GetChannelUpgrade()
 
-			module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.MockFeePort)
-			suite.Require().NoError(err)
-
-			app, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module)
+			app, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.AppRouter.HandshakeRoute(ibctesting.MockFeePort)
 			suite.Require().True(ok)
 
 			cbs, ok := app.(porttypes.UpgradableModule)

modules/apps/callbacks/ibc_middleware.go

@@ -368,13 +368,8 @@ func (IBCMiddleware) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string,
 }
 
 // OnChanUpgradeAck implements the IBCModule interface
-func (im IBCMiddleware) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion 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.OnChanUpgradeAck(ctx, portID, channelID, counterpartyVersion)
+func (IBCMiddleware) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion string) error {
+	return nil
 }
 
 // OnChanUpgradeOpen implements the IBCModule interface

modules/apps/transfer/ibc_module_test.go

@@ -690,10 +690,7 @@ func (suite *TransferTestSuite) TestOnChanUpgradeAck() {
 
 			tc.malleate()
 
-			module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), types.PortID)
-			suite.Require().NoError(err)
-
-			app, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module)
+			app, ok := suite.chainA.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

@@ -339,7 +339,39 @@ func (im *LegacyIBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID s
 }
 
 // OnChanUpgradeAck implements the IBCModule interface
-func (LegacyIBCModule) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion string) error {
+func (im *LegacyIBCModule) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion string) error {
+	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
+				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")
+		}
+
+		err := upgradableModule.OnChanUpgradeAck(ctx, portID, channelID, cbVersion)
+		if err != nil {
+			return errorsmod.Wrapf(err, "channel open init callback failed for port ID: %s, channel ID: %s", portID, channelID)
+		}
+	}
 	return nil
 }
 

modules/core/keeper/msg_server.go

@@ -758,7 +758,7 @@ func (k *Keeper) ChannelUpgradeTry(goCtx context.Context, msg *channeltypes.MsgC
 func (k *Keeper) ChannelUpgradeAck(goCtx context.Context, msg *channeltypes.MsgChannelUpgradeAck) (*channeltypes.MsgChannelUpgradeAckResponse, error) {
 	ctx := sdk.UnwrapSDKContext(goCtx)
 
-	app, ok := k.PortKeeper.Route(msg.PortId)
+	app, ok := k.PortKeeper.AppRouter.HandshakeRoute(msg.PortId)
 	if !ok {
 		err := errorsmod.Wrapf(porttypes.ErrInvalidRoute, "route not found to module: %s", msg.PortId)
 		ctx.Logger().Error("channel upgrade ack failed", "port-id", msg.PortId, "error", err)

modules/core/keeper/msg_server_test.go

@@ -1285,70 +1285,6 @@ func (suite *KeeperTestSuite) TestChannelUpgradeAck() {
 				ibctesting.AssertEvents(&suite.Suite, expEvents, events)
 			},
 		},
-		{
-			"application callback returns error and error receipt is written",
-			func() {
-				suite.chainA.GetSimApp().IBCMockModule.IBCApp.OnChanUpgradeAck = func(
-					ctx sdk.Context, portID, channelID, counterpartyVersion string,
-				) error {
-					// set arbitrary value in store to mock application state changes
-					store := ctx.KVStore(suite.chainA.GetSimApp().GetKey(exported.ModuleName))
-					store.Set([]byte("foo"), []byte("bar"))
-					return fmt.Errorf("mock app callback failed")
-				}
-			},
-			func(res *channeltypes.MsgChannelUpgradeAckResponse, events []abci.Event, err error) {
-				suite.Require().NoError(err)
-
-				suite.Require().NotNil(res)
-				suite.Require().Equal(channeltypes.FAILURE, res.Result)
-
-				errorReceipt, found := suite.chainA.GetSimApp().GetIBCKeeper().ChannelKeeper.GetUpgradeErrorReceipt(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID)
-				suite.Require().True(found)
-				suite.Require().Equal(uint64(1), errorReceipt.Sequence)
-
-				// assert application state changes are not committed
-				store := suite.chainA.GetContext().KVStore(suite.chainA.GetSimApp().GetKey(exported.ModuleName))
-				suite.Require().False(store.Has([]byte("foo")))
-
-				channel := path.EndpointB.GetChannel()
-				expEvents := sdk.Events{
-					sdk.NewEvent(
-						channeltypes.EventTypeChannelUpgradeError,
-						sdk.NewAttribute(channeltypes.AttributeKeyPortID, path.EndpointA.ChannelConfig.PortID),
-						sdk.NewAttribute(channeltypes.AttributeKeyChannelID, path.EndpointA.ChannelID),
-						sdk.NewAttribute(channeltypes.AttributeCounterpartyPortID, path.EndpointB.ChannelConfig.PortID),
-						sdk.NewAttribute(channeltypes.AttributeCounterpartyChannelID, path.EndpointB.ChannelID),
-						sdk.NewAttribute(channeltypes.AttributeKeyUpgradeSequence, fmt.Sprintf("%d", channel.UpgradeSequence)),
-						// need to manually insert this because the errorReceipt is a string constant as it is written into state
-						sdk.NewAttribute(channeltypes.AttributeKeyErrorReceipt, "mock app callback failed"),
-					),
-					sdk.NewEvent(
-						sdk.EventTypeMessage,
-						sdk.NewAttribute(sdk.AttributeKeyModule, channeltypes.AttributeValueCategory),
-					),
-				}.ToABCIEvents()
-				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.EndpointA.ChannelConfig.PortID
-				msg.ChannelId = path.EndpointA.ChannelID
-			},
-			func(res *channeltypes.MsgChannelUpgradeAckResponse, events []abci.Event, err error) {
-				suite.Require().ErrorIs(err, porttypes.ErrInvalidRoute)
-				suite.Require().Nil(res)
-				suite.Require().Empty(events)
-			},
-		},
 		{
 			"application callback returns an upgrade error",
 			func() {