Implement OnChanUpgradeInit on Controller Chain for interchain-accoun…

…ts (#5141)

* chore: adding controller implementation for OnChanUpgradeInit

* chore: happy path test passing

* chore: adding fail case

* chore: adding additional test cases

* chore: fix linting

* chore: improving errors

* chore: refactor to use test keeper function directly

* chore: add check for enabled controller module

* chore: call into middleware if provided

* chore: addressing PR feedback

* revert change in godoc of GetConnectionID

* fix: typo in MetadataFromVersion func

* chore: rm duplicate func

---------

Co-authored-by: Carlos Rodriguez <[email protected]>
Co-authored-by: Damian Nolan <[email protected]>

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

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)
+}

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
+}

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)
+			}
+		})
+	}
+}

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)

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

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
-}