diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index 1277eceb93b..864e060f062 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -71,16 +71,6 @@ func (im IBCMiddleware) OnChanOpenInit( if err != nil { return "", err } - - // call underlying app's OnChanOpenInit callback with the passed in version - // the version returned is discarded as the ica-auth module does not have permission to edit the version string. - // ics27 will always return the version string containing the Metadata struct which is created during the `RegisterInterchainAccount` call. - if im.app != nil && im.keeper.IsMiddlewareEnabled(ctx, portID, connectionHops[0]) { - if _, err := im.app.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, counterparty, version); err != nil { - return "", err - } - } - return version, nil } @@ -395,3 +385,17 @@ func (im IBCMiddleware) UnmarshalPacketData(ctx sdk.Context, portID string, chan return data, version, nil } + +// WrapVersion returns the wrapped version based on the provided version string and underlying application version. +// TODO: decide how we want to handle the underlying app. For now I made it backwards compatible. +// https://github.com/cosmos/ibc-go/issues/7063 +func (IBCMiddleware) WrapVersion(cbVersion, underlyingAppVersion string) string { + // ignore underlying app version + return cbVersion +} + +// UnwrapVersionUnsafe returns the version. Interchain accounts does not wrap versions. +func (IBCMiddleware) UnwrapVersionUnsafe(version string) (string, string, error) { + // ignore underlying app version + return version, "", nil +} 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 620e3792a4d..f621e84ca62 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -113,72 +113,28 @@ func SetupICAPath(path *ibctesting.Path, owner string) error { func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() { var ( - channel *channeltypes.Channel - isNilApp bool - path *ibctesting.Path + channel *channeltypes.Channel + counterparty channeltypes.Counterparty + path *ibctesting.Path ) testCases := []struct { name string malleate func() - expPass bool + expError error }{ { - "success", func() {}, true, - }, - { - "ICA auth module does not claim channel capability", func() { - suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnChanOpenInit = func(ctx sdk.Context, order channeltypes.Order, connectionHops []string, - portID, channelID string, - counterparty channeltypes.Counterparty, version string, - ) (string, error) { - return version, nil - } - }, true, - }, - { - "ICA auth module modification of channel version is ignored", func() { - // NOTE: explicitly modify the channel version via the auth module callback, - // ensuring the expected JSON encoded metadata is not modified upon return - suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnChanOpenInit = func(ctx sdk.Context, order channeltypes.Order, connectionHops []string, - portID, channelID string, - counterparty channeltypes.Counterparty, version string, - ) (string, error) { - return "invalid-version", nil - } - }, true, + "success", func() {}, nil, }, { "controller submodule disabled", func() { suite.chainA.GetSimApp().ICAControllerKeeper.SetParams(suite.chainA.GetContext(), types.NewParams(false)) - }, false, - }, - { - "ICA auth module callback fails", func() { - suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnChanOpenInit = func(ctx sdk.Context, order channeltypes.Order, connectionHops []string, - portID, channelID string, - counterparty channeltypes.Counterparty, version string, - ) (string, error) { - return "", fmt.Errorf("mock ica auth fails") - } - }, false, - }, - { - "nil underlying app", func() { - isNilApp = true - }, true, + }, types.ErrControllerSubModuleDisabled, }, { - "middleware disabled", func() { - suite.chainA.GetSimApp().ICAControllerKeeper.DeleteMiddlewareEnabled(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ConnectionID) - - suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnChanOpenInit = func(ctx sdk.Context, order channeltypes.Order, connectionHops []string, - portID, channelID string, - counterparty channeltypes.Counterparty, version string, - ) (string, error) { - return "", fmt.Errorf("error should be unreachable") - } - }, true, + "keeper call fails - counterparty portID incorrect", func() { + counterparty.PortId = "invalid-portID" + }, icatypes.ErrInvalidHostPort, }, } @@ -188,7 +144,6 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() { suite.Run(tc.name, func() { suite.SetupTest() // reset - isNilApp = false path = NewICAPath(suite.chainA, suite.chainB, ordering) path.SetupConnections() @@ -206,7 +161,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() { suite.chainA.GetSimApp().ICAControllerKeeper.SetMiddlewareEnabled(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ConnectionID) // default values - counterparty := channeltypes.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) + counterparty = channeltypes.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) channel = &channeltypes.Channel{ State: channeltypes.INIT, Ordering: ordering, @@ -220,21 +175,21 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() { // ensure channel on chainA is set in state suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetChannel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, *channel) - module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID) - suite.Require().NoError(err) - - cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module) + cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.AppRouter.HandshakeRoute(path.EndpointA.ChannelConfig.PortID) suite.Require().True(ok) - if isNilApp { - cbs = controller.NewIBCMiddleware(suite.chainA.GetSimApp().ICAControllerKeeper) - } + legacyModule, ok := cbs.(*porttypes.LegacyIBCModule) + suite.Require().True(ok, "expected there to be a single legacy ibc module") + + legacyModuleCbs := legacyModule.GetCallbacks() + controllerModule, ok := legacyModuleCbs[1].(controller.IBCMiddleware) // fee module is routed second + suite.Require().True(ok) - version, err := cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.ConnectionHops, - path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, channel.Counterparty, channel.Version, + version, err := controllerModule.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.ConnectionHops, + path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, counterparty, channel.Version, ) - if tc.expPass { + if tc.expError == nil { suite.Require().Equal(TestVersion, version) suite.Require().NoError(err) } else { diff --git a/modules/apps/29-fee/ibc_middleware.go b/modules/apps/29-fee/ibc_middleware.go index 36699316901..50cc1257bb9 100644 --- a/modules/apps/29-fee/ibc_middleware.go +++ b/modules/apps/29-fee/ibc_middleware.go @@ -2,6 +2,7 @@ package fee import ( "encoding/json" + "fmt" "strings" errorsmod "cosmossdk.io/errors" @@ -21,6 +22,7 @@ var ( _ porttypes.Middleware = (*IBCMiddleware)(nil) _ porttypes.PacketDataUnmarshaler = (*IBCMiddleware)(nil) _ porttypes.UpgradableModule = (*IBCMiddleware)(nil) + _ porttypes.VersionWrapper = (*IBCMiddleware)(nil) ) // IBCMiddleware implements the ICS26 callbacks for the fee middleware given the @@ -48,45 +50,12 @@ func (im IBCMiddleware) OnChanOpenInit( counterparty channeltypes.Counterparty, version string, ) (string, error) { - var versionMetadata types.Metadata - - if strings.TrimSpace(version) == "" { - // default version - versionMetadata = types.Metadata{ - FeeVersion: types.Version, - AppVersion: "", - } - } else { - metadata, err := types.MetadataFromVersion(version) - 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.OnChanOpenInit(ctx, order, connectionHops, portID, channelID, - counterparty, version) - } - versionMetadata = metadata - } - - if versionMetadata.FeeVersion != types.Version { - return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, versionMetadata.FeeVersion) - } - - appVersion, err := im.app.OnChanOpenInit(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 + if strings.TrimSpace(version) != "" && version != types.Version { + return "", errorsmod.Wrapf(types.ErrInvalidVersion, "expected %s, got %s", types.Version, version) } im.keeper.SetFeeEnabled(ctx, portID, channelID) - - // call underlying app's OnChanOpenInit callback with the appVersion - return string(versionBytes), nil + return types.Version, nil } // OnChanOpenTry implements the IBCMiddleware interface @@ -501,3 +470,30 @@ func unwrapAppVersion(channelVersion string) string { return metadata.AppVersion } + +// WrapVersion returns the wrapped ics29 version based on the provided ics29 version and the underlying application version. +func (IBCMiddleware) WrapVersion(cbVersion, underlyingAppVersion string) string { + if cbVersion != types.Version { + panic(fmt.Errorf("invalid ics29 version provided. expected: %s, got: %s", types.Version, cbVersion)) + } + + metadata := types.Metadata{ + FeeVersion: cbVersion, + AppVersion: underlyingAppVersion, + } + + versionBytes := types.ModuleCdc.MustMarshalJSON(&metadata) + + return string(versionBytes) +} + +// 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 { + // not an ics29 version + return "", version, err + } + + return metadata.FeeVersion, metadata.AppVersion, nil +} diff --git a/modules/apps/29-fee/ibc_middleware_test.go b/modules/apps/29-fee/ibc_middleware_test.go index 9ea47a7f204..25b882bd1d0 100644 --- a/modules/apps/29-fee/ibc_middleware_test.go +++ b/modules/apps/29-fee/ibc_middleware_test.go @@ -29,46 +29,24 @@ var ( // Tests OnChanOpenInit on ChainA func (suite *FeeTestSuite) TestOnChanOpenInit() { testCases := []struct { - name string - version string - expPass bool - isFeeEnabled bool + name string + version string + expError error }{ { - "success - valid fee middleware and mock version", - string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: ibcmock.Version})), - true, - true, + "success - valid fee version", + types.Version, + nil, }, { - "success - fee version not included, only perform mock logic", - ibcmock.Version, - true, - false, + "success - empty version", + "", + nil, }, { "invalid fee middleware version", - string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: "invalid-ics29-1", AppVersion: ibcmock.Version})), - false, - false, - }, - { - "invalid mock version", - string(types.ModuleCdc.MustMarshalJSON(&types.Metadata{FeeVersion: types.Version, AppVersion: "invalid-mock-version"})), - false, - false, - }, - { - "mock version not wrapped", - types.Version, - false, - false, - }, - { - "passing an empty string returns default version", - "", - true, - true, + "invalid-ics29-1", + types.ErrInvalidVersion, }, } @@ -81,17 +59,6 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() { suite.SetupTest() suite.path.SetupConnections() - // setup mock callback - suite.chainA.GetSimApp().FeeMockModule.IBCApp.OnChanOpenInit = func(ctx sdk.Context, order channeltypes.Order, connectionHops []string, - portID, channelID string, - counterparty channeltypes.Counterparty, version string, - ) (string, error) { - if version != ibcmock.Version { - return "", fmt.Errorf("incorrect mock version") - } - return ibcmock.Version, nil - } - suite.path.EndpointA.ChannelID = ibctesting.FirstChannelID counterparty := channeltypes.NewCounterparty(suite.path.EndpointB.ChannelConfig.PortID, suite.path.EndpointB.ChannelID) @@ -103,34 +70,24 @@ func (suite *FeeTestSuite) TestOnChanOpenInit() { Version: tc.version, } - module, _, err := suite.chainA.App.GetIBCKeeper().PortKeeper.LookupModuleByPort(suite.chainA.GetContext(), ibctesting.MockFeePort) - suite.Require().NoError(err) + cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.AppRouter.HandshakeRoute(ibctesting.MockFeePort) + suite.Require().True(ok) - cbs, ok := suite.chainA.App.GetIBCKeeper().PortKeeper.Route(module) + legacyModule, ok := cbs.(*porttypes.LegacyIBCModule) + suite.Require().True(ok, "expected there to be a single legacy ibc module") + + legacyModuleCbs := legacyModule.GetCallbacks() + feeModule, ok := legacyModuleCbs[1].(ibcfee.IBCMiddleware) // fee module is routed second suite.Require().True(ok) - version, err := cbs.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.ConnectionHops, + version, err := feeModule.OnChanOpenInit(suite.chainA.GetContext(), channel.Ordering, channel.ConnectionHops, suite.path.EndpointA.ChannelConfig.PortID, suite.path.EndpointA.ChannelID, counterparty, channel.Version) - if tc.expPass { - // check if the channel is fee enabled. If so version string should include metaData - if tc.isFeeEnabled { - versionMetadata := types.Metadata{ - FeeVersion: types.Version, - AppVersion: ibcmock.Version, - } - - versionBytes, err := types.ModuleCdc.MarshalJSON(&versionMetadata) - suite.Require().NoError(err) - - suite.Require().Equal(version, string(versionBytes)) - } else { - suite.Require().Equal(ibcmock.Version, version) - } - + if tc.expError == nil { + suite.Require().Equal(types.Version, version) suite.Require().NoError(err, "unexpected error from version: %s", tc.version) } else { - suite.Require().Error(err, "error not returned for version: %s", tc.version) + suite.Require().ErrorIs(err, tc.expError, "error not returned for version: %s", tc.version) suite.Require().Equal("", version) } }) diff --git a/modules/apps/callbacks/ibc_middleware.go b/modules/apps/callbacks/ibc_middleware.go index 99b63fb5501..01e40351bca 100644 --- a/modules/apps/callbacks/ibc_middleware.go +++ b/modules/apps/callbacks/ibc_middleware.go @@ -307,7 +307,7 @@ func (IBCMiddleware) processCallback( } // OnChanOpenInit defers to the underlying application -func (im IBCMiddleware) OnChanOpenInit( +func (IBCMiddleware) OnChanOpenInit( ctx sdk.Context, channelOrdering channeltypes.Order, connectionHops []string, @@ -316,7 +316,7 @@ func (im IBCMiddleware) OnChanOpenInit( counterparty channeltypes.Counterparty, version string, ) (string, error) { - return im.app.OnChanOpenInit(ctx, channelOrdering, connectionHops, portID, channelID, counterparty, version) + return "", nil } // OnChanOpenTry defers to the underlying application diff --git a/modules/apps/callbacks/ibc_middleware_test.go b/modules/apps/callbacks/ibc_middleware_test.go index 302f6c02868..d79aaddc8e3 100644 --- a/modules/apps/callbacks/ibc_middleware_test.go +++ b/modules/apps/callbacks/ibc_middleware_test.go @@ -171,7 +171,7 @@ func (s *CallbacksTestSuite) TestSendPacket() { s.Require().Len(cbs, 1, "expected 1 legacy module") - legacyModule, ok := cbs[0].(porttypes.LegacyIBCModule) + legacyModule, ok := cbs[0].(*porttypes.LegacyIBCModule) s.Require().True(ok, "expected there to be a single legacy ibc module") legacyModuleCbs := legacyModule.GetCallbacks() diff --git a/modules/apps/callbacks/testing/simapp/app.go b/modules/apps/callbacks/testing/simapp/app.go index 36e14716323..43a9b74eb3a 100644 --- a/modules/apps/callbacks/testing/simapp/app.go +++ b/modules/apps/callbacks/testing/simapp/app.go @@ -526,6 +526,7 @@ func NewSimApp( // initialize ICA module with mock module as the authentication module on the controller side var icaControllerStack porttypes.ClassicIBCModule icaControllerStack = ibcmock.NewIBCModule(&mockModule, ibcmock.NewIBCApp("", scopedICAMockKeeper)) + ibcAppRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerStack) app.ICAAuthModule, ok = icaControllerStack.(ibcmock.IBCModule) if !ok { panic(fmt.Errorf("cannot convert %T to %T", icaControllerStack, app.ICAAuthModule)) @@ -549,7 +550,9 @@ func NewSimApp( 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. @@ -560,9 +563,7 @@ 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) + ibcAppRouter.AddRoute(ibcmock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack) // Create Mock IBC Fee module stack for testing // SendPacket, mock module cannot send packets @@ -576,10 +577,13 @@ func NewSimApp( // create fee wrapped mock module feeMockModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewIBCApp(MockFeePort, scopedFeeMockKeeper)) app.FeeMockModule = feeMockModule + ibcAppRouter.AddRoute(MockFeePort, feeMockModule) + var feeWithMockModule porttypes.Middleware = ibcfee.NewIBCMiddleware(feeMockModule, app.IBCFeeKeeper) + ibcAppRouter.AddRoute(MockFeePort, feeWithMockModule) + feeWithMockModule = ibccallbacks.NewIBCMiddleware(feeWithMockModule, app.IBCFeeKeeper, app.MockContractKeeper, maxCallbackGas) ibcRouter.AddRoute(MockFeePort, feeWithMockModule) - ibcAppRouter.AddRoute(MockFeePort, feeWithMockModule) // Seal the IBC Router diff --git a/modules/core/05-port/types/ibc_legacy_module.go b/modules/core/05-port/types/ibc_legacy_module.go index 6879a3a1683..858c5d49b6e 100644 --- a/modules/core/05-port/types/ibc_legacy_module.go +++ b/modules/core/05-port/types/ibc_legacy_module.go @@ -1,12 +1,15 @@ package types import ( + "strings" + errorsmod "cosmossdk.io/errors" sdk "github.com/cosmos/cosmos-sdk/types" clienttypes "github.com/cosmos/ibc-go/v9/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v9/modules/core/04-channel/types" + ibcerrors "github.com/cosmos/ibc-go/v9/modules/core/errors" ibcexported "github.com/cosmos/ibc-go/v9/modules/core/exported" ) @@ -22,13 +25,13 @@ func (im *LegacyIBCModule) GetCallbacks() []ClassicIBCModule { // NewLegacyIBCModule creates a new IBCModule given the keeper func NewLegacyIBCModule(cbs ...ClassicIBCModule) ClassicIBCModule { - return LegacyIBCModule{ + return &LegacyIBCModule{ cbs: cbs, } } -// OnChanOpenInit implements the IBCModule interface -func (LegacyIBCModule) OnChanOpenInit( +// OnChanOpenInit implements the IBCModule interface. +func (im *LegacyIBCModule) OnChanOpenInit( ctx sdk.Context, order channeltypes.Order, connectionHops []string, @@ -37,7 +40,54 @@ func (LegacyIBCModule) OnChanOpenInit( counterparty channeltypes.Counterparty, version string, ) (string, error) { - return "", nil + negotiatedVersions := make([]string, len(im.cbs)) + + for i := len(im.cbs) - 1; i >= 0; i-- { + cbVersion := version + + // 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(version) != "" { + appVersion, underlyingAppVersion, err := wrapper.UnwrapVersionUnsafe(version) + if err != nil { + // middleware disabled + negotiatedVersions[i] = "" + continue + } + cbVersion, version = appVersion, underlyingAppVersion + } + + negotiatedVersion, err := im.cbs[i].OnChanOpenInit(ctx, order, connectionHops, portID, channelID, counterparty, 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) +} + +// 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 } // OnChanOpenTry implements the IBCModule interface. diff --git a/modules/core/05-port/types/module.go b/modules/core/05-port/types/module.go index bfcdd6bee1d..f43a6efc504 100644 --- a/modules/core/05-port/types/module.go +++ b/modules/core/05-port/types/module.go @@ -121,6 +121,24 @@ type IBCModule interface { ) error } +// VersionWrapper is an optional interface which should be implemented by middleware which wrap the channel version +// to ensure backwards compatibility. +type VersionWrapper interface { + // WrapVersion is required in order to remove middleware wiring and the ICS4Wrapper + // while maintaining backwards compatibility. It will be removed in the future. + // Applications should wrap the provided version with their application version. + // If they do not need to wrap, they may simply return the version provided. + WrapVersion(cbVersion, underlyingAppVersion string) string + // UnwrapVersionUnsafe is required in order to remove middleware wiring and the ICS4Wrapper + // while maintaining backwards compatibility. It will be removed in the future. + // Applications should unwrap the provided version with into their application version. + // and the underlying application version. If they are unsuccessful they should return an error. + // UnwrapVersionUnsafe will be used during opening handshakes and channel upgrades when the version + // is still being negotiated. + UnwrapVersionUnsafe(string) (cbVersion, underlyingAppVersion string, err error) + // UnwrapVersionSafe(ctx sdk.Context, portID, channelID, version string) (appVersion, version string) +} + // UpgradableModule defines the callbacks required to perform a channel upgrade. // Note: applications must ensure that state related to packet processing remains unmodified until the OnChanUpgradeOpen callback is executed. // This guarantees that in-flight packets are correctly flushed using the existing channel parameters. diff --git a/modules/core/05-port/types/router_v2.go b/modules/core/05-port/types/router_v2.go index 1c52e4a06ba..83c40e2ecd0 100644 --- a/modules/core/05-port/types/router_v2.go +++ b/modules/core/05-port/types/router_v2.go @@ -66,7 +66,7 @@ func (rtr *AppRouter) AddRoute(module string, cbs IBCModule) *AppRouter { // in order to facilitate having a single LegacyIBCModule, but also allowing for // consecutive calls to AddRoute to support existing functionality, we can re-create // the legacy module with the routes as they get added. - if classicRoutes, ok := rtr.classicRoutes[module]; ok && len(classicRoutes) > 1 { + if classicRoutes, ok := rtr.classicRoutes[module]; ok { rtr.legacyRoutes[module] = NewLegacyIBCModule(classicRoutes...) } } else { diff --git a/modules/core/keeper/msg_server.go b/modules/core/keeper/msg_server.go index eb534a12958..08c8c66cf00 100644 --- a/modules/core/keeper/msg_server.go +++ b/modules/core/keeper/msg_server.go @@ -194,7 +194,7 @@ func (k *Keeper) ChannelOpenInit(goCtx context.Context, msg *channeltypes.MsgCha 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 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/simapp/app.go b/simapp/app.go index 9fbd17c744f..cf9a8cc0a7a 100644 --- a/simapp/app.go +++ b/simapp/app.go @@ -514,6 +514,8 @@ func NewSimApp( // initialize ICA module with mock module as the authentication module on the controller side var icaControllerStack porttypes.ClassicIBCModule icaControllerStack = mock.NewIBCModule(&mockModule, mock.NewIBCApp("", scopedICAMockKeeper)) + ibcAppRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerStack) + ibcAppRouter.AddRoute(mock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack) var ok bool app.ICAAuthModule, ok = icaControllerStack.(mock.IBCModule) if !ok { @@ -521,15 +523,20 @@ func NewSimApp( } icaControllerStack = icacontroller.NewIBCMiddleware(app.ICAControllerKeeper) ibcAppRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerStack) + ibcAppRouter.AddRoute(mock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack) + icaControllerStack = ibcfee.NewIBCMiddleware(icaControllerStack, app.IBCFeeKeeper) ibcAppRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerStack) + ibcAppRouter.AddRoute(mock.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. @@ -540,9 +547,6 @@ func NewSimApp( AddRoute(icahosttypes.SubModuleName, icaHostStack). AddRoute(mock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack) // ica with mock auth module stack route to ica (top level of middleware stack) - ibcAppRouter. - AddRoute(icahosttypes.SubModuleName, icaHostStack). - AddRoute(mock.ModuleName+icacontrollertypes.SubModuleName, icaControllerStack) // Create Mock IBC Fee module stack for testing // SendPacket, mock module cannot send packets @@ -555,6 +559,8 @@ func NewSimApp( // create fee wrapped mock module feeMockModule := mock.NewIBCModule(&mockModule, mock.NewIBCApp(MockFeePort, scopedFeeMockKeeper)) app.FeeMockModule = feeMockModule + ibcAppRouter.AddRoute(MockFeePort, feeMockModule) + feeWithMockModule := ibcfee.NewIBCMiddleware(feeMockModule, app.IBCFeeKeeper) ibcRouter.AddRoute(MockFeePort, feeWithMockModule) ibcAppRouter.AddRoute(MockFeePort, feeWithMockModule) diff --git a/testing/simapp/app.go b/testing/simapp/app.go index 4c5d688eb7f..b1449658164 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -479,6 +479,7 @@ func NewSimApp( // initialize ICA module with mock module as the authentication module on the controller side var icaControllerStack porttypes.ClassicIBCModule icaControllerStack = ibcmock.NewIBCModule(&mockModule, ibcmock.NewIBCApp("", scopedICAMockKeeper)) + ibcAppRouter.AddRoute(icacontrollertypes.SubModuleName, icaControllerStack) var ok bool app.ICAAuthModule, ok = icaControllerStack.(ibcmock.IBCModule) if !ok { @@ -521,9 +522,10 @@ func NewSimApp( // create fee wrapped mock module feeMockModule := ibcmock.NewIBCModule(&mockModule, ibcmock.NewIBCApp(MockFeePort, scopedFeeMockKeeper)) app.FeeMockModule = feeMockModule + ibcAppRouter.AddRoute(MockFeePort, feeMockModule) + feeWithMockModule := ibcfee.NewIBCMiddleware(feeMockModule, app.IBCFeeKeeper) ibcRouter.AddRoute(MockFeePort, feeWithMockModule) - ibcAppRouter.AddRoute(MockFeePort, feeWithMockModule) // Seal the IBC Router