diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index 8608d8fb8f9..cf9905781f6 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/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 diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go index d36b8a729a8..9b6a9f6f50f 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/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) diff --git a/modules/apps/27-interchain-accounts/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index 17071dabdc6..7be9f6c431b 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/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) diff --git a/modules/apps/29-fee/ibc_middleware.go b/modules/apps/29-fee/ibc_middleware.go index dc07ffc7d6f..060977d1e62 100644 --- a/modules/apps/29-fee/ibc_middleware.go +++ b/modules/apps/29-fee/ibc_middleware.go @@ -294,7 +294,7 @@ func (im IBCMiddleware) OnTimeoutPacket( } // OnChanUpgradeInit implements the IBCModule interface -func (im IBCMiddleware) OnChanUpgradeInit( +func (IBCMiddleware) OnChanUpgradeInit( ctx sdk.Context, portID string, channelID string, @@ -302,34 +302,10 @@ func (im IBCMiddleware) OnChanUpgradeInit( 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 diff --git a/modules/apps/callbacks/ibc_middleware.go b/modules/apps/callbacks/ibc_middleware.go index 024fd1c8d6e..a5935e1b177 100644 --- a/modules/apps/callbacks/ibc_middleware.go +++ b/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 diff --git a/modules/core/05-port/types/ibc_legacy_module.go b/modules/core/05-port/types/ibc_legacy_module.go index 3776d6bc2be..43415290a8e 100644 --- a/modules/core/05-port/types/ibc_legacy_module.go +++ b/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 +} diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index 212e74ed489..6af62cdc465 100644 --- a/modules/core/keeper/msg_server.go +++ b/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) diff --git a/modules/core/keeper/msg_server_test.go b/modules/core/keeper/msg_server_test.go index 74e426e99e3..9824358cef4 100644 --- a/modules/core/keeper/msg_server_test.go +++ b/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() {