From 871a799ab117143d9d0d19a3edaca192e33ec51d Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Mon, 19 Sep 2022 12:10:36 +0200 Subject: [PATCH 1/4] adding unwrapping of channel version to ica controller handshake reopening flow --- .../27-interchain-accounts/controller/keeper/handshake.go | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go index 842857dc844..7847c72a458 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go @@ -70,7 +70,12 @@ func (k Keeper) OnChanOpenInit( return "", sdkerrors.Wrapf(icatypes.ErrActiveChannelAlreadySet, "existing active channel %s for portID %s is already OPEN", activeChannelID, portID) } - if !icatypes.IsPreviousMetadataEqual(channel.Version, metadata) { + appVersion, found := k.GetAppVersion(ctx, portID, activeChannelID) + if !found { + panic(fmt.Sprintf("active channel mapping set for %s, but channel does not exist in channel store", activeChannelID)) + } + + if !icatypes.IsPreviousMetadataEqual(appVersion, metadata) { return "", sdkerrors.Wrap(icatypes.ErrInvalidVersion, "previous active channel metadata does not match provided version") } } From 5e9be5bfc820a6687dd828f8f9747f40e5cf73d2 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Mon, 19 Sep 2022 12:17:03 +0200 Subject: [PATCH 2/4] adding changelog --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5c555e6e586..b5c60caa9ea 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -109,6 +109,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (light-clients/solomachine) [#1839](https://github.com/cosmos/ibc-go/issues/1839) Fixed usage of the new diversifier in validation of changing diversifiers for the solo machine. The current diversifier must sign over the new diversifier. * (light-clients/07-tendermint) [\#1674](https://github.com/cosmos/ibc-go/pull/1674) Submitted ClientState is zeroed out before checking the proof in order to prevent the proposal from containing information governance is not actually voting on. * (modules/core/02-client)[\#1676](https://github.com/cosmos/ibc-go/pull/1676) ClientState must be zeroed out for `UpgradeProposals` to pass validation. This prevents a proposal containing information governance is not actually voting on. +* (apps/27-interchain-accounts)[\#2302](https://github.com/cosmos/ibc-go/pull/2302) Use correct application version in favour of `channel.Version` in interchain accounts channel reopening handshake flow. ## [v4.0.0](https://github.com/cosmos/ibc-go/releases/tag/v4.0.0) - 2022-08-12 From 6d6bdaf8ba182109dd85a2dcd5f7e31f2333fb00 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Mon, 19 Sep 2022 20:45:16 +0200 Subject: [PATCH 3/4] adding ics4Wrapper to ics27 host submodule, handling unwrapping channel version in OnChanOpenTry handshake cb --- .../host/ibc_module_test.go | 15 ++++++++++++++- .../host/keeper/handshake.go | 7 ++++++- .../27-interchain-accounts/host/keeper/keeper.go | 9 ++++++++- testing/simapp/app.go | 1 + 4 files changed, 29 insertions(+), 3 deletions(-) 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 0dd7c71f8ed..64dd9a8e6e2 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -14,6 +14,7 @@ import ( "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/host/types" icatypes "github.com/cosmos/ibc-go/v5/modules/apps/27-interchain-accounts/types" + feetypes "github.com/cosmos/ibc-go/v5/modules/apps/29-fee/types" clienttypes "github.com/cosmos/ibc-go/v5/modules/core/02-client/types" channeltypes "github.com/cosmos/ibc-go/v5/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/v5/modules/core/24-host" @@ -84,7 +85,7 @@ func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) erro channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext()) - if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, TestVersion); err != nil { + if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version); err != nil { return err } @@ -611,6 +612,18 @@ func (suite *InterchainAccountsTestSuite) fundICAWallet(ctx sdk.Context, portID func (suite *InterchainAccountsTestSuite) TestControlAccountAfterChannelClose() { // create channel + init interchain account on a particular port path := NewICAPath(suite.chainA, suite.chainB) + + // use a fee enabled version to cover unwrapping channel version code paths + feeMetadata := feetypes.Metadata{ + FeeVersion: feetypes.Version, + AppVersion: TestVersion, + } + + feeICAVersion := string(feetypes.ModuleCdc.MustMarshalJSON(&feeMetadata)) + + path.EndpointA.ChannelConfig.Version = feeICAVersion + path.EndpointB.ChannelConfig.Version = feeICAVersion + suite.coordinator.SetupConnections(path) err := SetupICAPath(path, TestOwnerAddress) suite.Require().NoError(err) diff --git a/modules/apps/27-interchain-accounts/host/keeper/handshake.go b/modules/apps/27-interchain-accounts/host/keeper/handshake.go index c8a6914debe..07e09c2b587 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/handshake.go +++ b/modules/apps/27-interchain-accounts/host/keeper/handshake.go @@ -59,7 +59,12 @@ func (k Keeper) OnChanOpenTry( return "", sdkerrors.Wrapf(icatypes.ErrActiveChannelAlreadySet, "existing active channel %s for portID %s is already OPEN", activeChannelID, portID) } - if !icatypes.IsPreviousMetadataEqual(channel.Version, metadata) { + appVersion, found := k.GetAppVersion(ctx, portID, activeChannelID) + if !found { + panic(fmt.Sprintf("active channel mapping set for %s, but channel does not exist in channel store", activeChannelID)) + } + + if !icatypes.IsPreviousMetadataEqual(appVersion, metadata) { return "", sdkerrors.Wrap(icatypes.ErrInvalidVersion, "previous active channel metadata does not match provided version") } } diff --git a/modules/apps/27-interchain-accounts/host/keeper/keeper.go b/modules/apps/27-interchain-accounts/host/keeper/keeper.go index 693df6fcd67..93912498170 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/keeper.go +++ b/modules/apps/27-interchain-accounts/host/keeper/keeper.go @@ -24,6 +24,7 @@ type Keeper struct { cdc codec.BinaryCodec paramSpace paramtypes.Subspace + ics4Wrapper icatypes.ICS4Wrapper channelKeeper icatypes.ChannelKeeper portKeeper icatypes.PortKeeper accountKeeper icatypes.AccountKeeper @@ -36,7 +37,7 @@ type Keeper struct { // NewKeeper creates a new interchain accounts host Keeper instance func NewKeeper( cdc codec.BinaryCodec, key storetypes.StoreKey, paramSpace paramtypes.Subspace, - channelKeeper icatypes.ChannelKeeper, portKeeper icatypes.PortKeeper, + ics4Wrapper icatypes.ICS4Wrapper, channelKeeper icatypes.ChannelKeeper, portKeeper icatypes.PortKeeper, accountKeeper icatypes.AccountKeeper, scopedKeeper icatypes.ScopedKeeper, msgRouter icatypes.MessageRouter, ) Keeper { // ensure ibc interchain accounts module account is set @@ -53,6 +54,7 @@ func NewKeeper( storeKey: key, cdc: cdc, paramSpace: paramSpace, + ics4Wrapper: ics4Wrapper, channelKeeper: channelKeeper, portKeeper: portKeeper, accountKeeper: accountKeeper, @@ -90,6 +92,11 @@ func (k Keeper) ClaimCapability(ctx sdk.Context, cap *capabilitytypes.Capability return k.scopedKeeper.ClaimCapability(ctx, cap, name) } +// GetAppVersion calls the ICS4Wrapper GetAppVersion function. +func (k Keeper) GetAppVersion(ctx sdk.Context, portID, channelID string) (string, bool) { + return k.ics4Wrapper.GetAppVersion(ctx, portID, channelID) +} + // GetActiveChannelID retrieves the active channelID from the store keyed by the provided connectionID and portID func (k Keeper) GetActiveChannelID(ctx sdk.Context, connectionID, portID string) (string, bool) { store := ctx.KVStore(k.storeKey) diff --git a/testing/simapp/app.go b/testing/simapp/app.go index bbc1faa6440..b53249a05cb 100644 --- a/testing/simapp/app.go +++ b/testing/simapp/app.go @@ -406,6 +406,7 @@ func NewSimApp( // ICA Host keeper app.ICAHostKeeper = icahostkeeper.NewKeeper( appCodec, keys[icahosttypes.StoreKey], app.GetSubspace(icahosttypes.SubModuleName), + app.IBCFeeKeeper, // use ics29 fee as ics4Wrapper in middleware stack app.IBCKeeper.ChannelKeeper, &app.IBCKeeper.PortKeeper, app.AccountKeeper, scopedICAHostKeeper, app.MsgServiceRouter(), ) From c18da2c147a72a4b49f635a1136f5c817620a8e5 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Mon, 19 Sep 2022 20:52:26 +0200 Subject: [PATCH 4/4] updating changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b5c60caa9ea..acea73a528d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -67,6 +67,7 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (transfer) [\#2034](https://github.com/cosmos/ibc-go/pull/2034) Transfer Keeper now expects a keeper which fulfills the expected `ScopedKeeper` interface for the capability keeper. * (05-port) [\#2025](https://github.com/cosmos/ibc-go/pull/2025) Port Keeper now expects a keeper which fulfills the expected `ScopedKeeper` interface for the capability keeper. * (04-channel) [\#2024](https://github.com/cosmos/ibc-go/pull/2024) Channel Keeper now expects a keeper which fulfills the expected `ScopedKeeper` interface for the capability keeper. +* (apps/27-interchain-accounts)[\#2302](https://github.com/cosmos/ibc-go/pull/2302) Handle unwrapping of channel version in interchain accounts channel reopening handshake flow. The `host` submodule `Keeper` now requires an `ICS4Wrapper` similarly to the `controller` submodule. ### State Machine Breaking @@ -109,7 +110,6 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (light-clients/solomachine) [#1839](https://github.com/cosmos/ibc-go/issues/1839) Fixed usage of the new diversifier in validation of changing diversifiers for the solo machine. The current diversifier must sign over the new diversifier. * (light-clients/07-tendermint) [\#1674](https://github.com/cosmos/ibc-go/pull/1674) Submitted ClientState is zeroed out before checking the proof in order to prevent the proposal from containing information governance is not actually voting on. * (modules/core/02-client)[\#1676](https://github.com/cosmos/ibc-go/pull/1676) ClientState must be zeroed out for `UpgradeProposals` to pass validation. This prevents a proposal containing information governance is not actually voting on. -* (apps/27-interchain-accounts)[\#2302](https://github.com/cosmos/ibc-go/pull/2302) Use correct application version in favour of `channel.Version` in interchain accounts channel reopening handshake flow. ## [v4.0.0](https://github.com/cosmos/ibc-go/releases/tag/v4.0.0) - 2022-08-12