From 73721253a0f5ba141d5d0d5ff4aded8c98fd9241 Mon Sep 17 00:00:00 2001 From: chatton Date: Mon, 20 Nov 2023 13:11:21 +0000 Subject: [PATCH 01/13] chore: adding controller implementation for OnChanUpgradeInit --- .../controller/ibc_middleware.go | 49 ++++++++++++++++++- .../controller/keeper/keeper.go | 14 ++++++ .../27-interchain-accounts/types/metadata.go | 9 ++++ 3 files changed, 70 insertions(+), 2 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index 8c004012694..c64bf998246 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -232,8 +232,53 @@ func (im IBCMiddleware) OnTimeoutPacket( } // OnChanUpgradeInit implements the IBCModule interface -func (IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) { - return icatypes.Version, nil +func (im IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) { + if version == "" { + return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "version cannot be empty") + } + + metadata, err := icatypes.ParseMedataFromString(version) + if err != nil { + return "", err + } + + channel, err := im.keeper.GetChannel(ctx, portID, channelID) + if err != nil { + return "", err + } + + currentMetadata, err := icatypes.ParseMedataFromString(channel.Version) + if err != nil { + return "", err + } + + if err := icatypes.ValidateControllerMetadata(ctx, im.keeper.GetChannelKeeper(), connectionHops, metadata); err != nil { + return "", errorsmod.Wrap(err, "invalid metadata") + } + + // the interchain account address on the host chain + // must remain the same after the upgrade. + if currentMetadata.Address != metadata.Address { + return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "address cannot be changed") + } + + // at the moment it is not supported to perform upgrades that + // change the connection ID of the controller or host chains. + // therefore these connection IDs much remain the same as before. + if currentMetadata.ControllerConnectionId != metadata.ControllerConnectionId { + return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "controller connection ID cannot be changed") + } + + if currentMetadata.HostConnectionId != metadata.HostConnectionId { + return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "host connection ID cannot be changed") + } + + if currentMetadata.ControllerConnectionId != connectionHops[0] { + return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "proposed connection hop must not change") + } + + metadataBz := icatypes.ModuleCdc.MustMarshalJSON(&metadata) + return string(metadataBz), nil } // OnChanUpgradeTry implements the IBCModule interface diff --git a/modules/apps/27-interchain-accounts/controller/keeper/keeper.go b/modules/apps/27-interchain-accounts/controller/keeper/keeper.go index 63e606c395e..6a55887db78 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/keeper.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/keeper.go @@ -85,6 +85,11 @@ func (k Keeper) GetConnectionID(ctx sdk.Context, portID, channelID string) (stri return channel.ConnectionHops[0], nil } +// GetChannelKeeper returns the ChannelKeeper +func (k Keeper) GetChannelKeeper() icatypes.ChannelKeeper { + return k.channelKeeper +} + // GetAllPorts returns all ports to which the interchain accounts controller module is bound. Used in ExportGenesis func (k Keeper) GetAllPorts(ctx sdk.Context) []string { store := ctx.KVStore(k.storeKey) @@ -156,6 +161,15 @@ func (k Keeper) GetOpenActiveChannel(ctx sdk.Context, connectionID, portID strin return "", false } +// GetChannel returns the channel associated with the provided portID and channelID. +func (k Keeper) GetChannel(ctx sdk.Context, portID, channelID string) (channeltypes.Channel, error) { + channel, found := k.channelKeeper.GetChannel(ctx, portID, channelID) + if !found { + return channeltypes.Channel{}, errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "port ID (%s) channel ID (%s)", portID, channelID) + } + return channel, nil +} + // IsActiveChannelClosed retrieves the active channel from the store and returns true if the channel state is CLOSED, otherwise false func (k Keeper) IsActiveChannelClosed(ctx sdk.Context, connectionID, portID string) bool { channelID, found := k.GetActiveChannelID(ctx, connectionID, portID) diff --git a/modules/apps/27-interchain-accounts/types/metadata.go b/modules/apps/27-interchain-accounts/types/metadata.go index 29d1da45d42..03e559e8e7d 100644 --- a/modules/apps/27-interchain-accounts/types/metadata.go +++ b/modules/apps/27-interchain-accounts/types/metadata.go @@ -54,6 +54,15 @@ func NewDefaultMetadataString(controllerConnectionID, hostConnectionID string) s return string(ModuleCdc.MustMarshalJSON(&metadata)) } +// ParseMedataFromString parses Metadata from a json encoded version string. +func ParseMedataFromString(versionString string) (Metadata, error) { + var metadata Metadata + if err := ModuleCdc.UnmarshalJSON([]byte(versionString), &metadata); err != nil { + return Metadata{}, errorsmod.Wrapf(ErrUnknownDataType, "cannot unmarshal ICS-27 interchain accounts metadata") + } + return metadata, nil +} + // IsPreviousMetadataEqual compares a metadata to a previous version string set in a channel struct. // It ensures all fields are equal except the Address string func IsPreviousMetadataEqual(previousVersion string, metadata Metadata) bool { From 4427f1df245bda8e2de48080c437aa9ccb3f967d Mon Sep 17 00:00:00 2001 From: chatton Date: Mon, 20 Nov 2023 14:21:24 +0000 Subject: [PATCH 02/13] chore: happy path test passing --- .../controller/keeper/handshake_test.go | 55 +++++++++++++++++++ 1 file changed, 55 insertions(+) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go index 29ebf75dec1..e22143df4fc 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go @@ -471,3 +471,58 @@ func (suite *KeeperTestSuite) TestOnChanCloseConfirm() { }) } } + +func (suite *KeeperTestSuite) TestOnChanUpgradeInit() { + var path *ibctesting.Path + + testCases := []struct { + name string + malleate func() + expError error + }{ + { + "success", func() {}, nil, + }, + } + + for _, tc := range testCases { + tc := tc + + suite.Run(tc.name, func() { + suite.SetupTest() // reset + + path = NewICAPath(suite.chainA, suite.chainB) + suite.coordinator.SetupConnections(path) + + err := SetupICAPath(path, TestOwnerAddress) + suite.Require().NoError(err) + + channel := path.EndpointA.GetChannel() + + currentMetadata, err := icatypes.ParseMedataFromString(channel.Version) + suite.Require().NoError(err) + + metadata := icatypes.NewDefaultMetadata(path.EndpointA.ConnectionID, path.EndpointB.ConnectionID) + // use the same address as the previous metadata. + metadata.Address = currentMetadata.Address + + // this is the actual change to the version. + metadata.Encoding = icatypes.EncodingProto3JSON + + path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = string(icatypes.ModuleCdc.MustMarshalJSON(&metadata)) + path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = string(icatypes.ModuleCdc.MustMarshalJSON(&metadata)) + + tc.malleate() // malleate mutates test data + + err = path.EndpointA.ChanUpgradeInit() + + expPass := tc.expError == nil + + if expPass { + suite.Require().NoError(err) + } else { + suite.Require().Equal(tc.expError, err) + } + }) + } +} From 978a7a2ec1654e6d32a893ff81d7e5f87d5e7d88 Mon Sep 17 00:00:00 2001 From: chatton Date: Mon, 20 Nov 2023 16:25:32 +0000 Subject: [PATCH 03/13] chore: adding fail case --- .../controller/keeper/handshake_test.go | 32 +++++++++++++++---- 1 file changed, 26 insertions(+), 6 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go index e22143df4fc..6a3560ab1b5 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go @@ -473,7 +473,10 @@ func (suite *KeeperTestSuite) TestOnChanCloseConfirm() { } func (suite *KeeperTestSuite) TestOnChanUpgradeInit() { - var path *ibctesting.Path + var ( + path *ibctesting.Path + metadata icatypes.Metadata + ) testCases := []struct { name string @@ -481,7 +484,24 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() { expError error }{ { - "success", func() {}, nil, + "success", + func() {}, + nil, + }, + { + "failure: no changes made in upgrade", + func() { + // revert the change so that proposed metadata is the same as current. + metadata.Encoding = icatypes.EncodingProtobuf + }, + channeltypes.ErrChannelExists, + }, + { + name: "failure: invalid metadata", + malleate: func() { + + }, + expError: nil, }, } @@ -502,18 +522,18 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() { currentMetadata, err := icatypes.ParseMedataFromString(channel.Version) suite.Require().NoError(err) - metadata := icatypes.NewDefaultMetadata(path.EndpointA.ConnectionID, path.EndpointB.ConnectionID) + metadata = icatypes.NewDefaultMetadata(path.EndpointA.ConnectionID, path.EndpointB.ConnectionID) // use the same address as the previous metadata. metadata.Address = currentMetadata.Address // this is the actual change to the version. metadata.Encoding = icatypes.EncodingProto3JSON + tc.malleate() // malleate mutates test data + path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = string(icatypes.ModuleCdc.MustMarshalJSON(&metadata)) path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = string(icatypes.ModuleCdc.MustMarshalJSON(&metadata)) - tc.malleate() // malleate mutates test data - err = path.EndpointA.ChanUpgradeInit() expPass := tc.expError == nil @@ -521,7 +541,7 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() { if expPass { suite.Require().NoError(err) } else { - suite.Require().Equal(tc.expError, err) + suite.Require().Contains(err.Error(), tc.expError.Error()) } }) } From 925392f1036856ab3ca5a284ad94e8c819b2c331 Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 21 Nov 2023 09:38:57 +0000 Subject: [PATCH 04/13] chore: adding additional test cases --- .../controller/ibc_middleware.go | 14 ++------- .../controller/keeper/handshake_test.go | 29 +++++++++++++++++-- 2 files changed, 29 insertions(+), 14 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index c64bf998246..ff0f7985202 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -2,6 +2,7 @@ package controller import ( "errors" + "strings" errorsmod "cosmossdk.io/errors" @@ -233,7 +234,7 @@ func (im IBCMiddleware) OnTimeoutPacket( // OnChanUpgradeInit implements the IBCModule interface func (im IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) { - if version == "" { + if strings.TrimSpace(version) == "" { return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "version cannot be empty") } @@ -262,17 +263,6 @@ func (im IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID str return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "address cannot be changed") } - // at the moment it is not supported to perform upgrades that - // change the connection ID of the controller or host chains. - // therefore these connection IDs much remain the same as before. - if currentMetadata.ControllerConnectionId != metadata.ControllerConnectionId { - return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "controller connection ID cannot be changed") - } - - if currentMetadata.HostConnectionId != metadata.HostConnectionId { - return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "host connection ID cannot be changed") - } - if currentMetadata.ControllerConnectionId != connectionHops[0] { return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "proposed connection hop must not change") } diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go index 6a3560ab1b5..42d8e5cac01 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go @@ -3,6 +3,7 @@ package keeper_test import ( capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types" icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types" + connectiontypes "github.com/cosmos/ibc-go/v8/modules/core/03-connection/types" channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" host "github.com/cosmos/ibc-go/v8/modules/core/24-host" ibctesting "github.com/cosmos/ibc-go/v8/testing" @@ -499,9 +500,32 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() { { name: "failure: invalid metadata", malleate: func() { - + metadata.Address = TestOwnerAddress + }, + expError: icatypes.ErrInvalidChannelFlow, + }, + { + name: "failure: change connection id", + malleate: func() { + metadata.ControllerConnectionId = "connection-100" }, - expError: nil, + expError: connectiontypes.ErrInvalidConnection, + }, + { + name: "failure: change host connection id", + malleate: func() { + metadata.HostConnectionId = "connection-100" + }, + expError: connectiontypes.ErrInvalidConnection, + }, + { + name: "failure: invalid metadata string", + malleate: func() { + channel := path.EndpointA.GetChannel() + channel.Version = "invalid-metadata-string" + path.EndpointA.SetChannel(channel) + }, + expError: icatypes.ErrUnknownDataType, }, } @@ -541,6 +565,7 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() { if expPass { suite.Require().NoError(err) } else { + suite.Require().Error(err) suite.Require().Contains(err.Error(), tc.expError.Error()) } }) From fa5e34989b4a9e13c4f3b7769b159566981eb513 Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 21 Nov 2023 10:08:03 +0000 Subject: [PATCH 05/13] chore: fix linting --- .../controller/keeper/handshake_test.go | 8 ++++++-- modules/capability/module.go | 2 +- scripts/go-lint-all.sh | 4 ++-- 3 files changed, 9 insertions(+), 5 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go index 42d8e5cac01..5a6b2e2ef60 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go @@ -9,6 +9,10 @@ import ( ibctesting "github.com/cosmos/ibc-go/v8/testing" ) +const ( + differentConnectionID = "connection-100" +) + func (suite *KeeperTestSuite) TestOnChanOpenInit() { var ( channel *channeltypes.Channel @@ -507,14 +511,14 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() { { name: "failure: change connection id", malleate: func() { - metadata.ControllerConnectionId = "connection-100" + metadata.ControllerConnectionId = differentConnectionID }, expError: connectiontypes.ErrInvalidConnection, }, { name: "failure: change host connection id", malleate: func() { - metadata.HostConnectionId = "connection-100" + metadata.HostConnectionId = differentConnectionID }, expError: connectiontypes.ErrInvalidConnection, }, diff --git a/modules/capability/module.go b/modules/capability/module.go index 786e41cf6e8..5f048a3b838 100644 --- a/modules/capability/module.go +++ b/modules/capability/module.go @@ -47,7 +47,7 @@ func NewAppModuleBasic(cdc codec.Codec) AppModuleBasic { } // Name returns the capability module's name. -func (am AppModuleBasic) Name() string { +func (AppModuleBasic) Name() string { return types.ModuleName } diff --git a/scripts/go-lint-all.sh b/scripts/go-lint-all.sh index 7a7883b36aa..a6811de23b7 100755 --- a/scripts/go-lint-all.sh +++ b/scripts/go-lint-all.sh @@ -9,10 +9,10 @@ lint_module() { local root="$1" shift cd "$(dirname "$root")" && - echo "linting $(grep "^module" go.mod) [$(date -Iseconds -u)]" && + echo "linting $(grep "^module" go.mod) [$(date -u +"%Y-%m-%dT%H:%M:%S")]" && golangci-lint run ./... -c "${REPO_ROOT}/.golangci.yml" "$@" } export -f lint_module find "${REPO_ROOT}" -type f -name go.mod -print0 | - xargs -0 -I{} bash -c 'lint_module "$@"' _ {} "$@" \ No newline at end of file + xargs -0 -I{} bash -c 'lint_module "$@"' _ {} "$@" From 32dba02e746e70457240192ac76770e9be78a147 Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 21 Nov 2023 10:25:38 +0000 Subject: [PATCH 06/13] chore: improving errors --- .../27-interchain-accounts/controller/ibc_middleware.go | 7 ++++--- .../controller/keeper/handshake_test.go | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index ff0f7985202..0efbf0e909f 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -13,6 +13,7 @@ import ( "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/types" icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" + connectiontypes "github.com/cosmos/ibc-go/v8/modules/core/03-connection/types" channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types" host "github.com/cosmos/ibc-go/v8/modules/core/24-host" @@ -235,7 +236,7 @@ func (im IBCMiddleware) OnTimeoutPacket( // OnChanUpgradeInit implements the IBCModule interface func (im IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) { if strings.TrimSpace(version) == "" { - return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "version cannot be empty") + return "", errorsmod.Wrap(icatypes.ErrInvalidVersion, "version cannot be empty") } metadata, err := icatypes.ParseMedataFromString(version) @@ -260,11 +261,11 @@ func (im IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID str // the interchain account address on the host chain // must remain the same after the upgrade. if currentMetadata.Address != metadata.Address { - return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "address cannot be changed") + return "", errorsmod.Wrap(icatypes.ErrInvalidAccountAddress, "address cannot be changed") } if currentMetadata.ControllerConnectionId != connectionHops[0] { - return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "proposed connection hop must not change") + return "", errorsmod.Wrap(connectiontypes.ErrInvalidConnectionIdentifier, "proposed connection hop must not change") } metadataBz := icatypes.ModuleCdc.MustMarshalJSON(&metadata) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go index 5a6b2e2ef60..827c37ed74e 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go @@ -506,7 +506,7 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() { malleate: func() { metadata.Address = TestOwnerAddress }, - expError: icatypes.ErrInvalidChannelFlow, + expError: icatypes.ErrInvalidAccountAddress, }, { name: "failure: change connection id", From 292529e9e8441d8139013055c2899bab366b43ad Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 21 Nov 2023 11:34:56 +0000 Subject: [PATCH 07/13] chore: refactor to use test keeper function directly --- .../controller/ibc_middleware.go | 38 +----------------- .../controller/keeper/handshake.go | 40 +++++++++++++++++++ .../controller/keeper/handshake_test.go | 30 ++++++++------ .../controller/keeper/keeper.go | 14 ------- 4 files changed, 58 insertions(+), 64 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index 0efbf0e909f..6fb9a7a357d 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -2,7 +2,6 @@ package controller import ( "errors" - "strings" errorsmod "cosmossdk.io/errors" @@ -13,7 +12,6 @@ import ( "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/types" icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types" clienttypes "github.com/cosmos/ibc-go/v8/modules/core/02-client/types" - connectiontypes "github.com/cosmos/ibc-go/v8/modules/core/03-connection/types" channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types" host "github.com/cosmos/ibc-go/v8/modules/core/24-host" @@ -235,41 +233,7 @@ func (im IBCMiddleware) OnTimeoutPacket( // OnChanUpgradeInit implements the IBCModule interface func (im IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) { - if strings.TrimSpace(version) == "" { - return "", errorsmod.Wrap(icatypes.ErrInvalidVersion, "version cannot be empty") - } - - metadata, err := icatypes.ParseMedataFromString(version) - if err != nil { - return "", err - } - - channel, err := im.keeper.GetChannel(ctx, portID, channelID) - if err != nil { - return "", err - } - - currentMetadata, err := icatypes.ParseMedataFromString(channel.Version) - if err != nil { - return "", err - } - - if err := icatypes.ValidateControllerMetadata(ctx, im.keeper.GetChannelKeeper(), connectionHops, metadata); err != nil { - return "", errorsmod.Wrap(err, "invalid metadata") - } - - // the interchain account address on the host chain - // must remain the same after the upgrade. - if currentMetadata.Address != metadata.Address { - return "", errorsmod.Wrap(icatypes.ErrInvalidAccountAddress, "address cannot be changed") - } - - if currentMetadata.ControllerConnectionId != connectionHops[0] { - return "", errorsmod.Wrap(connectiontypes.ErrInvalidConnectionIdentifier, "proposed connection hop must not change") - } - - metadataBz := icatypes.ModuleCdc.MustMarshalJSON(&metadata) - return string(metadataBz), nil + return im.keeper.OnChanUpgradeInit(ctx, portID, channelID, connectionHops, version) } // OnChanUpgradeTry implements the IBCModule interface diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go index 0ce3802b48e..a15fb35f4cf 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go @@ -10,6 +10,7 @@ import ( capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types" icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types" + connectiontypes "github.com/cosmos/ibc-go/v8/modules/core/03-connection/types" channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" ) @@ -136,3 +137,42 @@ func (Keeper) OnChanCloseConfirm( ) error { return nil } + +// OnChanUpgradeInit performs the upgrade init step of the channel upgrade handshake. +func (k Keeper) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, connectionHops []string, version string) (string, error) { + if strings.TrimSpace(version) == "" { + return "", errorsmod.Wrap(icatypes.ErrInvalidVersion, "version cannot be empty") + } + + metadata, err := icatypes.ParseMedataFromString(version) + if err != nil { + return "", err + } + + channel, found := k.channelKeeper.GetChannel(ctx, portID, channelID) + if !found { + return "", errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "failed to retrieve channel %s on port %s", channelID, portID) + } + + currentMetadata, err := icatypes.ParseMedataFromString(channel.Version) + if err != nil { + return "", err + } + + if err := icatypes.ValidateControllerMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil { + return "", errorsmod.Wrap(err, "invalid metadata") + } + + // the interchain account address on the host chain + // must remain the same after the upgrade. + if currentMetadata.Address != metadata.Address { + return "", errorsmod.Wrap(icatypes.ErrInvalidAccountAddress, "address cannot be changed") + } + + if currentMetadata.ControllerConnectionId != connectionHops[0] { + return "", errorsmod.Wrap(connectiontypes.ErrInvalidConnectionIdentifier, "proposed connection hop must not change") + } + + metadataBz := icatypes.ModuleCdc.MustMarshalJSON(&metadata) + return string(metadataBz), nil +} diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go index 827c37ed74e..ef556f5d9ff 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go @@ -493,14 +493,6 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() { func() {}, nil, }, - { - "failure: no changes made in upgrade", - func() { - // revert the change so that proposed metadata is the same as current. - metadata.Encoding = icatypes.EncodingProtobuf - }, - channeltypes.ErrChannelExists, - }, { name: "failure: invalid metadata", malleate: func() { @@ -531,6 +523,13 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() { }, expError: icatypes.ErrUnknownDataType, }, + { + name: "failure: invalid connection hops", + malleate: func() { + path.EndpointA.ConnectionID = differentConnectionID + }, + expError: connectiontypes.ErrConnectionNotFound, + }, } for _, tc := range testCases { @@ -559,18 +558,23 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() { tc.malleate() // malleate mutates test data - path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = string(icatypes.ModuleCdc.MustMarshalJSON(&metadata)) - path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = string(icatypes.ModuleCdc.MustMarshalJSON(&metadata)) + version := string(icatypes.ModuleCdc.MustMarshalJSON(&metadata)) - err = path.EndpointA.ChanUpgradeInit() + upgradeVersion, err := path.EndpointA.Chain.GetSimApp().ICAControllerKeeper.OnChanUpgradeInit( + path.EndpointA.Chain.GetContext(), + path.EndpointA.ChannelConfig.PortID, + path.EndpointA.ChannelID, + []string{path.EndpointA.ConnectionID}, + version, + ) expPass := tc.expError == nil if expPass { suite.Require().NoError(err) + suite.Require().Equal(upgradeVersion, version) } else { - suite.Require().Error(err) - suite.Require().Contains(err.Error(), tc.expError.Error()) + suite.Require().ErrorIs(err, tc.expError) } }) } diff --git a/modules/apps/27-interchain-accounts/controller/keeper/keeper.go b/modules/apps/27-interchain-accounts/controller/keeper/keeper.go index 6a55887db78..63e606c395e 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/keeper.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/keeper.go @@ -85,11 +85,6 @@ func (k Keeper) GetConnectionID(ctx sdk.Context, portID, channelID string) (stri return channel.ConnectionHops[0], nil } -// GetChannelKeeper returns the ChannelKeeper -func (k Keeper) GetChannelKeeper() icatypes.ChannelKeeper { - return k.channelKeeper -} - // GetAllPorts returns all ports to which the interchain accounts controller module is bound. Used in ExportGenesis func (k Keeper) GetAllPorts(ctx sdk.Context) []string { store := ctx.KVStore(k.storeKey) @@ -161,15 +156,6 @@ func (k Keeper) GetOpenActiveChannel(ctx sdk.Context, connectionID, portID strin return "", false } -// GetChannel returns the channel associated with the provided portID and channelID. -func (k Keeper) GetChannel(ctx sdk.Context, portID, channelID string) (channeltypes.Channel, error) { - channel, found := k.channelKeeper.GetChannel(ctx, portID, channelID) - if !found { - return channeltypes.Channel{}, errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "port ID (%s) channel ID (%s)", portID, channelID) - } - return channel, nil -} - // IsActiveChannelClosed retrieves the active channel from the store and returns true if the channel state is CLOSED, otherwise false func (k Keeper) IsActiveChannelClosed(ctx sdk.Context, connectionID, portID string) bool { channelID, found := k.GetActiveChannelID(ctx, connectionID, portID) From eb7631407ca6e0c3d66d77098e6b3e7e3342107a Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 21 Nov 2023 11:40:30 +0000 Subject: [PATCH 08/13] chore: add check for enabled controller module --- .../apps/27-interchain-accounts/controller/ibc_middleware.go | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index 6fb9a7a357d..81e132b0c2e 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -233,6 +233,10 @@ func (im IBCMiddleware) OnTimeoutPacket( // OnChanUpgradeInit implements the IBCModule interface func (im IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, version string) (string, error) { + if !im.keeper.GetParams(ctx).ControllerEnabled { + return "", types.ErrControllerSubModuleDisabled + } + return im.keeper.OnChanUpgradeInit(ctx, portID, channelID, connectionHops, version) } From 236f2d9bf9c6824c821549f3f567b8ed072856a5 Mon Sep 17 00:00:00 2001 From: chatton Date: Tue, 21 Nov 2023 13:38:11 +0000 Subject: [PATCH 09/13] chore: call into middleware if provided --- .../controller/ibc_middleware.go | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index 81e132b0c2e..043afff714d 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -237,7 +237,21 @@ func (im IBCMiddleware) OnChanUpgradeInit(ctx sdk.Context, portID, channelID str return "", types.ErrControllerSubModuleDisabled } - return im.keeper.OnChanUpgradeInit(ctx, portID, channelID, connectionHops, version) + version, err := im.keeper.OnChanUpgradeInit(ctx, portID, channelID, connectionHops, version) + 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) { + return im.app.OnChanUpgradeInit(ctx, portID, channelID, order, connectionHops, version) + } + + return version, nil } // OnChanUpgradeTry implements the IBCModule interface From bc477b9682417f28495bac74f2e9e81146de9dbf Mon Sep 17 00:00:00 2001 From: chatton Date: Wed, 29 Nov 2023 11:20:52 +0000 Subject: [PATCH 10/13] chore: addressing PR feedback --- .../controller/keeper/export_test.go | 12 +++++++++++- .../controller/keeper/handshake.go | 14 ++++---------- .../controller/keeper/handshake_test.go | 10 ++++------ .../controller/keeper/keeper.go | 13 ++++++++++++- .../27-interchain-accounts/host/keeper/keeper.go | 10 ++-------- .../apps/27-interchain-accounts/types/metadata.go | 4 ++-- 6 files changed, 35 insertions(+), 28 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/export_test.go b/modules/apps/27-interchain-accounts/controller/keeper/export_test.go index 382dde7a805..2cd4338dde2 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/export_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/export_test.go @@ -4,9 +4,19 @@ package keeper This file is to allow for unexported functions and fields to be accessible to the testing package. */ -import porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types" +import ( + sdk "github.com/cosmos/cosmos-sdk/types" + + icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types" + porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types" +) // GetICS4Wrapper is a getter for the keeper's ICS4Wrapper. func (k *Keeper) GetICS4Wrapper() porttypes.ICS4Wrapper { return k.ics4Wrapper } + +// GetAppMetadata is a wrapper around getAppMetadata to allow the function to be directly called in tests. +func (k Keeper) GetAppMetadata(ctx sdk.Context, portID, channelID string) (icatypes.Metadata, error) { + return k.getAppMetadata(ctx, portID, channelID) +} diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go index a15fb35f4cf..1bb2ae663d7 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go @@ -144,17 +144,12 @@ func (k Keeper) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, con return "", errorsmod.Wrap(icatypes.ErrInvalidVersion, "version cannot be empty") } - metadata, err := icatypes.ParseMedataFromString(version) + metadata, err := icatypes.MedataFromVersion(version) if err != nil { return "", err } - channel, found := k.channelKeeper.GetChannel(ctx, portID, channelID) - if !found { - return "", errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "failed to retrieve channel %s on port %s", channelID, portID) - } - - currentMetadata, err := icatypes.ParseMedataFromString(channel.Version) + currentMetadata, err := k.getAppMetadata(ctx, portID, channelID) if err != nil { return "", err } @@ -166,13 +161,12 @@ func (k Keeper) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, con // the interchain account address on the host chain // must remain the same after the upgrade. if currentMetadata.Address != metadata.Address { - return "", errorsmod.Wrap(icatypes.ErrInvalidAccountAddress, "address cannot be changed") + return "", errorsmod.Wrap(icatypes.ErrInvalidAccountAddress, "interchain account address cannot be changed") } if currentMetadata.ControllerConnectionId != connectionHops[0] { return "", errorsmod.Wrap(connectiontypes.ErrInvalidConnectionIdentifier, "proposed connection hop must not change") } - metadataBz := icatypes.ModuleCdc.MustMarshalJSON(&metadata) - return string(metadataBz), nil + return version, nil } diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go index ef556f5d9ff..8dc9211f3da 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go @@ -494,14 +494,14 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() { nil, }, { - name: "failure: invalid metadata", + name: "failure: change ICA address", malleate: func() { metadata.Address = TestOwnerAddress }, expError: icatypes.ErrInvalidAccountAddress, }, { - name: "failure: change connection id", + name: "failure: change controller connection id", malleate: func() { metadata.ControllerConnectionId = differentConnectionID }, @@ -515,7 +515,7 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() { expError: connectiontypes.ErrInvalidConnection, }, { - name: "failure: invalid metadata string", + name: "failure: cannot decode version string", malleate: func() { channel := path.EndpointA.GetChannel() channel.Version = "invalid-metadata-string" @@ -544,9 +544,7 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() { err := SetupICAPath(path, TestOwnerAddress) suite.Require().NoError(err) - channel := path.EndpointA.GetChannel() - - currentMetadata, err := icatypes.ParseMedataFromString(channel.Version) + currentMetadata, err := suite.chainA.GetSimApp().ICAControllerKeeper.GetAppMetadata(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) suite.Require().NoError(err) metadata = icatypes.NewDefaultMetadata(path.EndpointA.ConnectionID, path.EndpointB.ConnectionID) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/keeper.go b/modules/apps/27-interchain-accounts/controller/keeper/keeper.go index 63e606c395e..b642a50acee 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/keeper.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/keeper.go @@ -20,6 +20,7 @@ import ( channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types" host "github.com/cosmos/ibc-go/v8/modules/core/24-host" + ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" "github.com/cosmos/ibc-go/v8/modules/core/exported" ) @@ -76,7 +77,7 @@ func (Keeper) Logger(ctx sdk.Context) log.Logger { return ctx.Logger().With("module", fmt.Sprintf("x/%s-%s", exported.ModuleName, icatypes.ModuleName)) } -// GetConnectionID returns the connection id for the given port and channelIDs. +// ConnectionID returns the connection id for the given port and channelIDs. func (k Keeper) GetConnectionID(ctx sdk.Context, portID, channelID string) (string, error) { channel, found := k.channelKeeper.GetChannel(ctx, portID, channelID) if !found { @@ -280,6 +281,16 @@ func (k Keeper) GetAuthority() string { return k.authority } +// getAppMetadata retrieves the interchain accounts channel metadata from the store associated with the provided portID and channelID +func (k Keeper) getAppMetadata(ctx sdk.Context, portID, channelID string) (icatypes.Metadata, error) { + appVersion, found := k.GetAppVersion(ctx, portID, channelID) + if !found { + return icatypes.Metadata{}, errorsmod.Wrapf(ibcerrors.ErrNotFound, "app version not found for port %s and channel %s", portID, channelID) + } + + return icatypes.MedataFromVersion(appVersion) +} + // GetParams returns the current ica/controller submodule parameters. func (k Keeper) GetParams(ctx sdk.Context) types.Params { store := ctx.KVStore(k.storeKey) diff --git a/modules/apps/27-interchain-accounts/host/keeper/keeper.go b/modules/apps/27-interchain-accounts/host/keeper/keeper.go index c426c4cc0cb..429551f4772 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/keeper.go +++ b/modules/apps/27-interchain-accounts/host/keeper/keeper.go @@ -111,20 +111,14 @@ func (k Keeper) GetAppVersion(ctx sdk.Context, portID, channelID string) (string return k.ics4Wrapper.GetAppVersion(ctx, portID, channelID) } -// GetAppMetadata retrieves the interchain accounts channel metadata from the store associated with the provided portID and channelID +// getAppMetadata retrieves the interchain accounts channel metadata from the store associated with the provided portID and channelID func (k Keeper) getAppMetadata(ctx sdk.Context, portID, channelID string) (icatypes.Metadata, error) { appVersion, found := k.GetAppVersion(ctx, portID, channelID) if !found { return icatypes.Metadata{}, errorsmod.Wrapf(ibcerrors.ErrNotFound, "app version not found for port %s and channel %s", portID, channelID) } - var metadata icatypes.Metadata - if err := icatypes.ModuleCdc.UnmarshalJSON([]byte(appVersion), &metadata); err != nil { - // UnmarshalJSON errors are indeterminate and therefore are not wrapped and included in failed acks - return icatypes.Metadata{}, errorsmod.Wrapf(icatypes.ErrUnknownDataType, "cannot unmarshal ICS-27 interchain accounts metadata") - } - - return metadata, nil + return icatypes.MedataFromVersion(appVersion) } // GetActiveChannelID retrieves the active channelID from the store keyed by the provided connectionID and portID diff --git a/modules/apps/27-interchain-accounts/types/metadata.go b/modules/apps/27-interchain-accounts/types/metadata.go index 03e559e8e7d..e1fd2a5ecfc 100644 --- a/modules/apps/27-interchain-accounts/types/metadata.go +++ b/modules/apps/27-interchain-accounts/types/metadata.go @@ -54,8 +54,8 @@ func NewDefaultMetadataString(controllerConnectionID, hostConnectionID string) s return string(ModuleCdc.MustMarshalJSON(&metadata)) } -// ParseMedataFromString parses Metadata from a json encoded version string. -func ParseMedataFromString(versionString string) (Metadata, error) { +// MedataFromVersion parses Metadata from a json encoded version string. +func MedataFromVersion(versionString string) (Metadata, error) { var metadata Metadata if err := ModuleCdc.UnmarshalJSON([]byte(versionString), &metadata); err != nil { return Metadata{}, errorsmod.Wrapf(ErrUnknownDataType, "cannot unmarshal ICS-27 interchain accounts metadata") From c8c6c6ba99b821c11e6c40562e5ebe055a499057 Mon Sep 17 00:00:00 2001 From: Carlos Rodriguez Date: Sat, 2 Dec 2023 23:02:50 +0100 Subject: [PATCH 11/13] revert change in godoc of GetConnectionID --- modules/apps/27-interchain-accounts/controller/keeper/keeper.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/keeper.go b/modules/apps/27-interchain-accounts/controller/keeper/keeper.go index b642a50acee..e5e5c18d5d4 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/keeper.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/keeper.go @@ -77,7 +77,7 @@ func (Keeper) Logger(ctx sdk.Context) log.Logger { return ctx.Logger().With("module", fmt.Sprintf("x/%s-%s", exported.ModuleName, icatypes.ModuleName)) } -// ConnectionID returns the connection id for the given port and channelIDs. +// GetConnectionID returns the connection id for the given port and channelIDs. func (k Keeper) GetConnectionID(ctx sdk.Context, portID, channelID string) (string, error) { channel, found := k.channelKeeper.GetChannel(ctx, portID, channelID) if !found { From 21773dbb38f342000b306d6e6c3252b3c744af95 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 5 Dec 2023 12:09:24 +0100 Subject: [PATCH 12/13] fix: typo in MetadataFromVersion func --- .../27-interchain-accounts/controller/keeper/handshake.go | 2 +- .../apps/27-interchain-accounts/controller/keeper/keeper.go | 2 +- modules/apps/27-interchain-accounts/host/keeper/keeper.go | 2 +- modules/apps/27-interchain-accounts/types/metadata.go | 4 ++-- 4 files changed, 5 insertions(+), 5 deletions(-) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go index 1bb2ae663d7..b055dff20f8 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go @@ -144,7 +144,7 @@ func (k Keeper) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, con return "", errorsmod.Wrap(icatypes.ErrInvalidVersion, "version cannot be empty") } - metadata, err := icatypes.MedataFromVersion(version) + metadata, err := icatypes.MetadataFromVersion(version) if err != nil { return "", err } diff --git a/modules/apps/27-interchain-accounts/controller/keeper/keeper.go b/modules/apps/27-interchain-accounts/controller/keeper/keeper.go index e5e5c18d5d4..4e11ea993e7 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/keeper.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/keeper.go @@ -288,7 +288,7 @@ func (k Keeper) getAppMetadata(ctx sdk.Context, portID, channelID string) (icaty return icatypes.Metadata{}, errorsmod.Wrapf(ibcerrors.ErrNotFound, "app version not found for port %s and channel %s", portID, channelID) } - return icatypes.MedataFromVersion(appVersion) + return icatypes.MetadataFromVersion(appVersion) } // GetParams returns the current ica/controller submodule parameters. diff --git a/modules/apps/27-interchain-accounts/host/keeper/keeper.go b/modules/apps/27-interchain-accounts/host/keeper/keeper.go index 429551f4772..be0cfc8a0b5 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/keeper.go +++ b/modules/apps/27-interchain-accounts/host/keeper/keeper.go @@ -118,7 +118,7 @@ func (k Keeper) getAppMetadata(ctx sdk.Context, portID, channelID string) (icaty return icatypes.Metadata{}, errorsmod.Wrapf(ibcerrors.ErrNotFound, "app version not found for port %s and channel %s", portID, channelID) } - return icatypes.MedataFromVersion(appVersion) + return icatypes.MetadataFromVersion(appVersion) } // GetActiveChannelID retrieves the active channelID from the store keyed by the provided connectionID and portID diff --git a/modules/apps/27-interchain-accounts/types/metadata.go b/modules/apps/27-interchain-accounts/types/metadata.go index e1fd2a5ecfc..d3c0f32a5f5 100644 --- a/modules/apps/27-interchain-accounts/types/metadata.go +++ b/modules/apps/27-interchain-accounts/types/metadata.go @@ -54,8 +54,8 @@ func NewDefaultMetadataString(controllerConnectionID, hostConnectionID string) s return string(ModuleCdc.MustMarshalJSON(&metadata)) } -// MedataFromVersion parses Metadata from a json encoded version string. -func MedataFromVersion(versionString string) (Metadata, error) { +// MetadataFromVersion parses Metadata from a json encoded version string. +func MetadataFromVersion(versionString string) (Metadata, error) { var metadata Metadata if err := ModuleCdc.UnmarshalJSON([]byte(versionString), &metadata); err != nil { return Metadata{}, errorsmod.Wrapf(ErrUnknownDataType, "cannot unmarshal ICS-27 interchain accounts metadata") From f1c10fdf105016bee7cab6c88ba9db51f2202498 Mon Sep 17 00:00:00 2001 From: Damian Nolan Date: Tue, 5 Dec 2023 12:58:10 +0100 Subject: [PATCH 13/13] chore: rm duplicate func --- modules/apps/27-interchain-accounts/types/metadata.go | 9 --------- 1 file changed, 9 deletions(-) diff --git a/modules/apps/27-interchain-accounts/types/metadata.go b/modules/apps/27-interchain-accounts/types/metadata.go index 99b39a83c10..61c077a46f0 100644 --- a/modules/apps/27-interchain-accounts/types/metadata.go +++ b/modules/apps/27-interchain-accounts/types/metadata.go @@ -174,12 +174,3 @@ func validateConnectionParams(metadata Metadata, controllerConnectionID, hostCon return nil } - -// MetadataFromVersion parses Metadata from a json encoded version string. -func MetadataFromVersion(versionString string) (Metadata, error) { - var metadata Metadata - if err := ModuleCdc.UnmarshalJSON([]byte(versionString), &metadata); err != nil { - return Metadata{}, errorsmod.Wrapf(ErrUnknownDataType, "cannot unmarshal ICS-27 interchain accounts metadata") - } - return metadata, nil -}