diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go index 8c004012694..043afff714d 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go @@ -232,8 +232,26 @@ 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 !im.keeper.GetParams(ctx).ControllerEnabled { + return "", types.ErrControllerSubModuleDisabled + } + + 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 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 eabb5612d2d..8146b83cebc 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" ) @@ -139,3 +140,36 @@ 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.MetadataFromVersion(version) + if err != nil { + return "", err + } + + currentMetadata, err := k.getAppMetadata(ctx, portID, channelID) + 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, "interchain account address cannot be changed") + } + + if currentMetadata.ControllerConnectionId != connectionHops[0] { + return "", errorsmod.Wrap(connectiontypes.ErrInvalidConnectionIdentifier, "proposed connection hop must not change") + } + + 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 29ebf75dec1..8dc9211f3da 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go @@ -3,11 +3,16 @@ 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" ) +const ( + differentConnectionID = "connection-100" +) + func (suite *KeeperTestSuite) TestOnChanOpenInit() { var ( channel *channeltypes.Channel @@ -471,3 +476,104 @@ func (suite *KeeperTestSuite) TestOnChanCloseConfirm() { }) } } + +func (suite *KeeperTestSuite) TestOnChanUpgradeInit() { + var ( + path *ibctesting.Path + metadata icatypes.Metadata + ) + + testCases := []struct { + name string + malleate func() + expError error + }{ + { + "success", + func() {}, + nil, + }, + { + name: "failure: change ICA address", + malleate: func() { + metadata.Address = TestOwnerAddress + }, + expError: icatypes.ErrInvalidAccountAddress, + }, + { + name: "failure: change controller connection id", + malleate: func() { + metadata.ControllerConnectionId = differentConnectionID + }, + expError: connectiontypes.ErrInvalidConnection, + }, + { + name: "failure: change host connection id", + malleate: func() { + metadata.HostConnectionId = differentConnectionID + }, + expError: connectiontypes.ErrInvalidConnection, + }, + { + name: "failure: cannot decode version string", + malleate: func() { + channel := path.EndpointA.GetChannel() + channel.Version = "invalid-metadata-string" + path.EndpointA.SetChannel(channel) + }, + expError: icatypes.ErrUnknownDataType, + }, + { + name: "failure: invalid connection hops", + malleate: func() { + path.EndpointA.ConnectionID = differentConnectionID + }, + expError: connectiontypes.ErrConnectionNotFound, + }, + } + + 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) + + 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) + // 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 + + version := string(icatypes.ModuleCdc.MustMarshalJSON(&metadata)) + + 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().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 63e606c395e..4e11ea993e7 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" ) @@ -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.MetadataFromVersion(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 4a86251c323..be0cfc8a0b5 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) } - metadata, err := icatypes.MetadataFromVersion(appVersion) - if err != nil { - // UnmarshalJSON errors are indeterminate and therefore are not wrapped and included in failed acks - return icatypes.Metadata{}, err - } - - return metadata, nil + 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 4f96f437ff2..61c077a46f0 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)) } +// 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 +} + // 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 { @@ -165,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 -}