From 73721253a0f5ba141d5d0d5ff4aded8c98fd9241 Mon Sep 17 00:00:00 2001
From: chatton <github.qpeyb@simplelogin.fr>
Date: Mon, 20 Nov 2023 13:11:21 +0000
Subject: [PATCH 01/30] 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 <github.qpeyb@simplelogin.fr>
Date: Mon, 20 Nov 2023 14:21:24 +0000
Subject: [PATCH 02/30] 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 <github.qpeyb@simplelogin.fr>
Date: Mon, 20 Nov 2023 16:25:32 +0000
Subject: [PATCH 03/30] 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 <github.qpeyb@simplelogin.fr>
Date: Tue, 21 Nov 2023 09:38:57 +0000
Subject: [PATCH 04/30] 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 <github.qpeyb@simplelogin.fr>
Date: Tue, 21 Nov 2023 10:08:03 +0000
Subject: [PATCH 05/30] 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 a49ce39a80c400cf23b1e00ae68c658afeac82f2 Mon Sep 17 00:00:00 2001
From: chatton <github.qpeyb@simplelogin.fr>
Date: Tue, 21 Nov 2023 10:22:55 +0000
Subject: [PATCH 06/30] wip: working on implementation

---
 .../27-interchain-accounts/host/ibc_module.go | 57 +++++++++++++++++++
 1 file changed, 57 insertions(+)

diff --git a/modules/apps/27-interchain-accounts/host/ibc_module.go b/modules/apps/27-interchain-accounts/host/ibc_module.go
index 79315a71bb8..78d0771f8b5 100644
--- a/modules/apps/27-interchain-accounts/host/ibc_module.go
+++ b/modules/apps/27-interchain-accounts/host/ibc_module.go
@@ -2,6 +2,7 @@ package host
 
 import (
 	"fmt"
+	"strings"
 
 	errorsmod "cosmossdk.io/errors"
 
@@ -161,8 +162,64 @@ func (IBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, or
 	return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "channel handshake must be initiated by controller chain")
 }
 
+/*
+// Called on Host Chain by Relayer
+function onChanUpgradeTry(
+  portIdentifier: Identifier,
+  channelIdentifier: Identifier,
+  order: ChannelOrder,
+  connectionHops: [Identifier],
+  upgradeSequence: uint64,
+  counterpartyPortIdentifier: Identifier,
+  counterpartyChannelIdentifier: Identifier,
+  counterpartyVersion: string
+): (version: string, err: Error) {
+  // validate port ID
+  abortTransactionUnless(portIdentifier === "icahost")
+
+  // upgrade version proposed by counterparty
+  abortTransactionUnless(counterpartyVersion !== "")
+
+  // retrieve the existing channel version.
+  // In ibc-go, for example, this is done using the GetAppVersion
+  // function of the ICS4Wrapper interface.
+  // See https://github.com/cosmos/ibc-go/blob/ac6300bd857cd2bd6915ae51e67c92848cbfb086/modules/core/05-port/types/module.go#L128-L132
+  channel = provableStore.get(channelPath(portIdentifier, channelIdentifier))
+  abortTransactionUnless(channel !== null)
+  currentMetadata = UnmarshalJSON(channel.version)
+
+  // validate metadata
+  abortTransactionUnless(metadata.Version === "ics27-1")
+  // all elements in encoding list and tx type list must be supported
+  abortTransactionUnless(IsSupportedEncoding(metadata.Encoding))
+  abortTransactionUnless(IsSupportedTxType(metadata.TxType))
+
+  // the interchain account address on the host chain
+  // must remain the same after the upgrade.
+  abortTransactionUnless(currentMetadata.Address === metadata.Address)
+
+  // 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.
+  abortTransactionUnless(currentMetadata.ControllerConnectionId === metadata.ControllerConnectionId)
+  abortTransactionUnless(currentMetadata.HostConnectionId === metadata.HostConnectionId)
+  // the proposed connection hop must not change
+  abortTransactionUnless(currentMetadata.HostConnectionId === connectionHops[0])
+
+  return counterpartyVersion, nil
+}
+*/
 // OnChanUpgradeTry implements the IBCModule interface
 func (IBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, counterpartyVersion string) (string, error) {
+	if portID != icatypes.HostPortID {
+		return "", errorsmod.Wrapf(porttypes.ErrInvalidPort, "port ID must be %s", icatypes.HostPortID)
+	}
+
+	counterpartyVersion = strings.TrimSpace(counterpartyVersion)
+	if counterpartyVersion == "" {
+		return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "counterparty version cannot be empty")
+	}
+
 	return icatypes.Version, nil
 }
 

From 32dba02e746e70457240192ac76770e9be78a147 Mon Sep 17 00:00:00 2001
From: chatton <github.qpeyb@simplelogin.fr>
Date: Tue, 21 Nov 2023 10:25:38 +0000
Subject: [PATCH 07/30] 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 c38be7c87e01c4bda85fba94b6394dd53489e07c Mon Sep 17 00:00:00 2001
From: chatton <github.qpeyb@simplelogin.fr>
Date: Tue, 21 Nov 2023 10:37:22 +0000
Subject: [PATCH 08/30] chore: adding implementation for OnChanUpgradeTry

---
 .../27-interchain-accounts/host/ibc_module.go | 81 ++++++++-----------
 .../host/keeper/keeper.go                     | 15 ++++
 2 files changed, 47 insertions(+), 49 deletions(-)

diff --git a/modules/apps/27-interchain-accounts/host/ibc_module.go b/modules/apps/27-interchain-accounts/host/ibc_module.go
index 78d0771f8b5..91874c7ec48 100644
--- a/modules/apps/27-interchain-accounts/host/ibc_module.go
+++ b/modules/apps/27-interchain-accounts/host/ibc_module.go
@@ -12,6 +12,7 @@ import (
 	"github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/keeper"
 	"github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/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"
 	porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"
 	ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
@@ -162,55 +163,8 @@ func (IBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, or
 	return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "channel handshake must be initiated by controller chain")
 }
 
-/*
-// Called on Host Chain by Relayer
-function onChanUpgradeTry(
-  portIdentifier: Identifier,
-  channelIdentifier: Identifier,
-  order: ChannelOrder,
-  connectionHops: [Identifier],
-  upgradeSequence: uint64,
-  counterpartyPortIdentifier: Identifier,
-  counterpartyChannelIdentifier: Identifier,
-  counterpartyVersion: string
-): (version: string, err: Error) {
-  // validate port ID
-  abortTransactionUnless(portIdentifier === "icahost")
-
-  // upgrade version proposed by counterparty
-  abortTransactionUnless(counterpartyVersion !== "")
-
-  // retrieve the existing channel version.
-  // In ibc-go, for example, this is done using the GetAppVersion
-  // function of the ICS4Wrapper interface.
-  // See https://github.com/cosmos/ibc-go/blob/ac6300bd857cd2bd6915ae51e67c92848cbfb086/modules/core/05-port/types/module.go#L128-L132
-  channel = provableStore.get(channelPath(portIdentifier, channelIdentifier))
-  abortTransactionUnless(channel !== null)
-  currentMetadata = UnmarshalJSON(channel.version)
-
-  // validate metadata
-  abortTransactionUnless(metadata.Version === "ics27-1")
-  // all elements in encoding list and tx type list must be supported
-  abortTransactionUnless(IsSupportedEncoding(metadata.Encoding))
-  abortTransactionUnless(IsSupportedTxType(metadata.TxType))
-
-  // the interchain account address on the host chain
-  // must remain the same after the upgrade.
-  abortTransactionUnless(currentMetadata.Address === metadata.Address)
-
-  // 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.
-  abortTransactionUnless(currentMetadata.ControllerConnectionId === metadata.ControllerConnectionId)
-  abortTransactionUnless(currentMetadata.HostConnectionId === metadata.HostConnectionId)
-  // the proposed connection hop must not change
-  abortTransactionUnless(currentMetadata.HostConnectionId === connectionHops[0])
-
-  return counterpartyVersion, nil
-}
-*/
 // OnChanUpgradeTry implements the IBCModule interface
-func (IBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, counterpartyVersion string) (string, error) {
+func (im IBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, counterpartyVersion string) (string, error) {
 	if portID != icatypes.HostPortID {
 		return "", errorsmod.Wrapf(porttypes.ErrInvalidPort, "port ID must be %s", icatypes.HostPortID)
 	}
@@ -220,7 +174,36 @@ func (IBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, ord
 		return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "counterparty version cannot be empty")
 	}
 
-	return icatypes.Version, nil
+	metadata, err := icatypes.ParseMedataFromString(counterpartyVersion)
+	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.ValidateHostMetadata(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.HostConnectionId != connectionHops[0] {
+		return "", errorsmod.Wrap(connectiontypes.ErrInvalidConnectionIdentifier, "proposed connection hop must not change")
+	}
+
+	return counterpartyVersion, nil
 }
 
 // OnChanUpgradeAck implements the IBCModule interface
diff --git a/modules/apps/27-interchain-accounts/host/keeper/keeper.go b/modules/apps/27-interchain-accounts/host/keeper/keeper.go
index c426c4cc0cb..c8f6588a2d3 100644
--- a/modules/apps/27-interchain-accounts/host/keeper/keeper.go
+++ b/modules/apps/27-interchain-accounts/host/keeper/keeper.go
@@ -16,6 +16,7 @@ import (
 	genesistypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/genesis/types"
 	"github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types"
 	icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/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"
 	ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
@@ -72,6 +73,20 @@ func NewKeeper(
 	}
 }
 
+// GetChannelKeeper returns the ChannelKeeper
+func (k *Keeper) GetChannelKeeper() icatypes.ChannelKeeper {
+	return k.channelKeeper
+}
+
+// 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
+}
+
 // WithICS4Wrapper sets the ICS4Wrapper. This function may be used after
 // the keepers creation to set the middleware which is above this module
 // in the IBC application stack.

From 4c74e08ee17e5271fc1d40033e83c253e067a736 Mon Sep 17 00:00:00 2001
From: chatton <github.qpeyb@simplelogin.fr>
Date: Tue, 21 Nov 2023 10:41:41 +0000
Subject: [PATCH 09/30] test: adding happy path test case

---
 .../controller/keeper/handshake_test.go       | 64 +++++++++++++++++++
 1 file changed, 64 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 827c37ed74e..d99f727b42b 100644
--- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go
+++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go
@@ -575,3 +575,67 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() {
 		})
 	}
 }
+
+func (suite *KeeperTestSuite) TestOnChanUpgradeTry() {
+	var (
+		path     *ibctesting.Path
+		metadata icatypes.Metadata
+	)
+
+	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))
+
+			err = path.EndpointA.ChanUpgradeInit()
+			suite.Require().NoError(err)
+
+			tc.malleate() // malleate mutates test data
+
+			err = path.EndpointB.ChanUpgradeTry()
+
+			expPass := tc.expError == nil
+
+			if expPass {
+				suite.Require().NoError(err)
+			} else {
+				suite.Require().Error(err)
+				suite.Require().Contains(err.Error(), tc.expError.Error())
+			}
+		})
+	}
+}

From e22da0d93b3b600ee3ad334e922719d0386a9b9c Mon Sep 17 00:00:00 2001
From: chatton <github.qpeyb@simplelogin.fr>
Date: Tue, 21 Nov 2023 11:09:16 +0000
Subject: [PATCH 10/30] chore: refactor tests to call keeper fn directly

---
 .../controller/keeper/handshake_test.go       | 31 ++++++++++--
 .../27-interchain-accounts/host/ibc_module.go | 42 +----------------
 .../host/keeper/handshake.go                  | 47 +++++++++++++++++++
 3 files changed, 75 insertions(+), 45 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 d99f727b42b..db3dc6db7d7 100644
--- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go
+++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go
@@ -5,6 +5,7 @@ import (
 	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"
+	porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"
 	host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
 )
@@ -592,6 +593,22 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeTry() {
 			func() {},
 			nil,
 		},
+		{
+			name: "failure: invalid port ID",
+			malleate: func() {
+				path.EndpointB.ChannelConfig.PortID = "invalid-port-id"
+			},
+			expError: porttypes.ErrInvalidPort,
+		},
+		{
+			name: "failure: empty counterparty version",
+			malleate: func() {
+				channel := path.EndpointA.GetChannel()
+				channel.Version = ""
+				path.EndpointA.SetChannel(channel)
+			},
+			expError: channeltypes.ErrInvalidChannelVersion,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -626,15 +643,21 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeTry() {
 
 			tc.malleate() // malleate mutates test data
 
-			err = path.EndpointB.ChanUpgradeTry()
+			version, err := suite.chainB.GetSimApp().ICAHostKeeper.OnChanUpgradeTry(
+				suite.chainB.GetContext(),
+				path.EndpointB.ChannelConfig.PortID,
+				path.EndpointB.ChannelID,
+				channeltypes.ORDERED,
+				[]string{path.EndpointB.ConnectionID},
+				path.EndpointA.GetChannel().Version,
+			)
 
 			expPass := tc.expError == nil
-
 			if expPass {
+				suite.Require().Equal(path.EndpointA.GetChannel().Version, version)
 				suite.Require().NoError(err)
 			} 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/host/ibc_module.go b/modules/apps/27-interchain-accounts/host/ibc_module.go
index 91874c7ec48..76c33b6191b 100644
--- a/modules/apps/27-interchain-accounts/host/ibc_module.go
+++ b/modules/apps/27-interchain-accounts/host/ibc_module.go
@@ -2,7 +2,6 @@ package host
 
 import (
 	"fmt"
-	"strings"
 
 	errorsmod "cosmossdk.io/errors"
 
@@ -12,7 +11,6 @@ import (
 	"github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/keeper"
 	"github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/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"
 	porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"
 	ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
@@ -165,45 +163,7 @@ func (IBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, or
 
 // OnChanUpgradeTry implements the IBCModule interface
 func (im IBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, counterpartyVersion string) (string, error) {
-	if portID != icatypes.HostPortID {
-		return "", errorsmod.Wrapf(porttypes.ErrInvalidPort, "port ID must be %s", icatypes.HostPortID)
-	}
-
-	counterpartyVersion = strings.TrimSpace(counterpartyVersion)
-	if counterpartyVersion == "" {
-		return "", errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "counterparty version cannot be empty")
-	}
-
-	metadata, err := icatypes.ParseMedataFromString(counterpartyVersion)
-	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.ValidateHostMetadata(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.HostConnectionId != connectionHops[0] {
-		return "", errorsmod.Wrap(connectiontypes.ErrInvalidConnectionIdentifier, "proposed connection hop must not change")
-	}
-
-	return counterpartyVersion, nil
+	return im.keeper.OnChanUpgradeTry(ctx, portID, channelID, order, connectionHops, counterpartyVersion)
 }
 
 // OnChanUpgradeAck implements the IBCModule interface
diff --git a/modules/apps/27-interchain-accounts/host/keeper/handshake.go b/modules/apps/27-interchain-accounts/host/keeper/handshake.go
index f3a202a3bae..c5ac779f54f 100644
--- a/modules/apps/27-interchain-accounts/host/keeper/handshake.go
+++ b/modules/apps/27-interchain-accounts/host/keeper/handshake.go
@@ -2,6 +2,7 @@ package keeper
 
 import (
 	"fmt"
+	"strings"
 
 	errorsmod "cosmossdk.io/errors"
 
@@ -9,7 +10,9 @@ 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"
+	porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"
 	host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
 )
 
@@ -130,3 +133,47 @@ func (Keeper) OnChanCloseConfirm(
 ) error {
 	return nil
 }
+
+
+// OnChanUpgradeTry performs the upgrade try step of the channel upgrade handshake.
+func (k Keeper) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, counterpartyVersion string) (string, error) {
+	if portID != icatypes.HostPortID {
+		return "", errorsmod.Wrapf(porttypes.ErrInvalidPort, "port ID must be %s", icatypes.HostPortID)
+	}
+
+	counterpartyVersion = strings.TrimSpace(counterpartyVersion)
+	if counterpartyVersion == "" {
+		return "", errorsmod.Wrap(channeltypes.ErrInvalidChannelVersion, "counterparty version cannot be empty")
+	}
+
+	metadata, err := icatypes.ParseMedataFromString(counterpartyVersion)
+	if err != nil {
+		return "", err
+	}
+
+	channel, err := k.GetChannel(ctx, portID, channelID)
+	if err != nil {
+		return "", err
+	}
+
+	currentMetadata, err := icatypes.ParseMedataFromString(channel.Version)
+	if err != nil {
+		return "", err
+	}
+
+	if err := icatypes.ValidateHostMetadata(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.HostConnectionId != connectionHops[0] {
+		return "", errorsmod.Wrap(connectiontypes.ErrInvalidConnectionIdentifier, "proposed connection hop must not change")
+	}
+
+	return counterpartyVersion, nil
+}

From 292529e9e8441d8139013055c2899bab366b43ad Mon Sep 17 00:00:00 2001
From: chatton <github.qpeyb@simplelogin.fr>
Date: Tue, 21 Nov 2023 11:34:56 +0000
Subject: [PATCH 11/30] 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 <github.qpeyb@simplelogin.fr>
Date: Tue, 21 Nov 2023 11:40:30 +0000
Subject: [PATCH 12/30] 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 15797db542e118c9ed3eb69531a124a6030ee956 Mon Sep 17 00:00:00 2001
From: chatton <github.qpeyb@simplelogin.fr>
Date: Tue, 21 Nov 2023 11:46:45 +0000
Subject: [PATCH 13/30] chore: fix linter

---
 modules/apps/27-interchain-accounts/host/keeper/handshake.go | 1 -
 1 file changed, 1 deletion(-)

diff --git a/modules/apps/27-interchain-accounts/host/keeper/handshake.go b/modules/apps/27-interchain-accounts/host/keeper/handshake.go
index c5ac779f54f..4861d8a6b63 100644
--- a/modules/apps/27-interchain-accounts/host/keeper/handshake.go
+++ b/modules/apps/27-interchain-accounts/host/keeper/handshake.go
@@ -134,7 +134,6 @@ func (Keeper) OnChanCloseConfirm(
 	return nil
 }
 
-
 // OnChanUpgradeTry performs the upgrade try step of the channel upgrade handshake.
 func (k Keeper) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, counterpartyVersion string) (string, error) {
 	if portID != icatypes.HostPortID {

From 0d0e904604f57a4472c0157933bfa70b718d7913 Mon Sep 17 00:00:00 2001
From: chatton <github.qpeyb@simplelogin.fr>
Date: Tue, 21 Nov 2023 11:49:45 +0000
Subject: [PATCH 14/30] chore: adding host enabled check

---
 modules/apps/27-interchain-accounts/host/ibc_module.go | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/modules/apps/27-interchain-accounts/host/ibc_module.go b/modules/apps/27-interchain-accounts/host/ibc_module.go
index 76c33b6191b..7e7a71c2c9f 100644
--- a/modules/apps/27-interchain-accounts/host/ibc_module.go
+++ b/modules/apps/27-interchain-accounts/host/ibc_module.go
@@ -163,6 +163,10 @@ func (IBCModule) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, or
 
 // OnChanUpgradeTry implements the IBCModule interface
 func (im IBCModule) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, counterpartyVersion string) (string, error) {
+	if !im.keeper.GetParams(ctx).HostEnabled {
+		return "", types.ErrHostSubModuleDisabled
+	}
+
 	return im.keeper.OnChanUpgradeTry(ctx, portID, channelID, order, connectionHops, counterpartyVersion)
 }
 

From fce51f54f34df8cf57574778f95ef195a512ab57 Mon Sep 17 00:00:00 2001
From: chatton <github.qpeyb@simplelogin.fr>
Date: Tue, 21 Nov 2023 13:00:34 +0000
Subject: [PATCH 15/30] wip: ack impl

---
 .../controller/ibc_middleware.go              |  8 +++-
 .../controller/keeper/handshake.go            | 43 +++++++++++++++++++
 2 files changed, 49 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 81e132b0c2e..a1b9ef36faa 100644
--- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go
+++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go
@@ -246,8 +246,12 @@ func (IBCMiddleware) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string,
 }
 
 // OnChanUpgradeAck implements the IBCModule interface
-func (IBCMiddleware) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion string) error {
-	return nil
+func (im IBCMiddleware) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion string) error {
+	if !im.keeper.GetParams(ctx).ControllerEnabled {
+		return types.ErrControllerSubModuleDisabled
+	}
+
+	return im.keeper.OnChanUpgradeAck(ctx, portID, channelID, counterpartyVersion)
 }
 
 // OnChanUpgradeOpen 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 a15fb35f4cf..33bca952bdb 100644
--- a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
+++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
@@ -176,3 +176,46 @@ func (k Keeper) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, con
 	metadataBz := icatypes.ModuleCdc.MustMarshalJSON(&metadata)
 	return string(metadataBz), nil
 }
+/*
+// Called on Controller Chain by Relayer
+function onChanUpgradeAck(
+  portIdentifier: Identifier,
+  channelIdentifier: Identifier,
+  counterpartyVersion: string
+): Error {
+  // final upgrade version proposed by counterparty
+  abortTransactionUnless(counterpartyVersion !== "")
+  metadata = UnmarshalJSON(counterpartyVersion)
+
+  // retrieve the existing channel version.
+  // In ibc-go, for example, this is done using the GetAppVersion
+  // function of the ICS4Wrapper interface.
+  // See https://github.com/cosmos/ibc-go/blob/ac6300bd857cd2bd6915ae51e67c92848cbfb086/modules/core/05-port/types/module.go#L128-L132
+  channel = provableStore.get(channelPath(portIdentifier, channelIdentifier))
+  abortTransactionUnless(channel !== null)
+  currentMetadata = UnmarshalJSON(channel.version)
+
+  // validate metadata
+  abortTransactionUnless(metadata.Version === "ics27-1")
+  // all elements in encoding list and tx type list must be supported
+  abortTransactionUnless(IsSupportedEncoding(metadata.Encoding))
+  abortTransactionUnless(IsSupportedTxType(metadata.TxType))
+
+  // the interchain account address on the host chain
+  // must remain the same after the upgrade.
+  abortTransactionUnless(currentMetadata.Address === metadata.Address)
+
+  // 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.
+  abortTransactionUnless(currentMetadata.ControllerConnectionId === metadata.ControllerConnectionId)
+  abortTransactionUnless(currentMetadata.HostConnectionId === metadata.HostConnectionId)
+
+  return nil
+}
+ */
+// OnChanUpgradeAck implements the ack setup of the channel upgrade handshake.
+func (k Keeper) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion string) error {
+	return nil
+}
+

From 236f2d9bf9c6824c821549f3f567b8ed072856a5 Mon Sep 17 00:00:00 2001
From: chatton <github.qpeyb@simplelogin.fr>
Date: Tue, 21 Nov 2023 13:38:11 +0000
Subject: [PATCH 16/30] 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 2656baaddb5aa9bdd54dc8ee5a1a9b1332bb1100 Mon Sep 17 00:00:00 2001
From: chatton <github.qpeyb@simplelogin.fr>
Date: Tue, 21 Nov 2023 14:41:05 +0000
Subject: [PATCH 17/30] chore: adding OnChanUpgradeAck implementation

---
 .../controller/ibc_middleware.go              | 15 +++-
 .../controller/keeper/handshake.go            | 77 +++++++++----------
 .../27-interchain-accounts/types/metadata.go  | 14 ++--
 3 files changed, 60 insertions(+), 46 deletions(-)

diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go
index 4074344c948..0616770343b 100644
--- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go
+++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go
@@ -265,7 +265,20 @@ func (im IBCMiddleware) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, cou
 		return types.ErrControllerSubModuleDisabled
 	}
 
-	return im.keeper.OnChanUpgradeAck(ctx, portID, channelID, counterpartyVersion)
+	if err := im.keeper.OnChanUpgradeAck(ctx, portID, channelID, counterpartyVersion); 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.OnChanUpgradeAck(ctx, portID, channelID, counterpartyVersion)
+	}
+
+	return nil
 }
 
 // OnChanUpgradeOpen 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 33bca952bdb..10222607d4d 100644
--- a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
+++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
@@ -176,46 +176,45 @@ func (k Keeper) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, con
 	metadataBz := icatypes.ModuleCdc.MustMarshalJSON(&metadata)
 	return string(metadataBz), nil
 }
-/*
-// Called on Controller Chain by Relayer
-function onChanUpgradeAck(
-  portIdentifier: Identifier,
-  channelIdentifier: Identifier,
-  counterpartyVersion: string
-): Error {
-  // final upgrade version proposed by counterparty
-  abortTransactionUnless(counterpartyVersion !== "")
-  metadata = UnmarshalJSON(counterpartyVersion)
-
-  // retrieve the existing channel version.
-  // In ibc-go, for example, this is done using the GetAppVersion
-  // function of the ICS4Wrapper interface.
-  // See https://github.com/cosmos/ibc-go/blob/ac6300bd857cd2bd6915ae51e67c92848cbfb086/modules/core/05-port/types/module.go#L128-L132
-  channel = provableStore.get(channelPath(portIdentifier, channelIdentifier))
-  abortTransactionUnless(channel !== null)
-  currentMetadata = UnmarshalJSON(channel.version)
-
-  // validate metadata
-  abortTransactionUnless(metadata.Version === "ics27-1")
-  // all elements in encoding list and tx type list must be supported
-  abortTransactionUnless(IsSupportedEncoding(metadata.Encoding))
-  abortTransactionUnless(IsSupportedTxType(metadata.TxType))
-
-  // the interchain account address on the host chain
-  // must remain the same after the upgrade.
-  abortTransactionUnless(currentMetadata.Address === metadata.Address)
-
-  // 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.
-  abortTransactionUnless(currentMetadata.ControllerConnectionId === metadata.ControllerConnectionId)
-  abortTransactionUnless(currentMetadata.HostConnectionId === metadata.HostConnectionId)
-
-  return nil
-}
- */
+
 // OnChanUpgradeAck implements the ack setup of the channel upgrade handshake.
 func (k Keeper) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion string) error {
+	if strings.TrimSpace(counterpartyVersion) == "" {
+		return errorsmod.Wrap(icatypes.ErrInvalidVersion, "counterparty version cannot be empty")
+	}
+
+	metadata, err := icatypes.ParseMedataFromString(counterpartyVersion)
+	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.ValidateHostMetadata(ctx, k.channelKeeper, nil, metadata); err != nil {
+		return nil
+	}
+
+	// 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 != metadata.ControllerConnectionId {
+		return errorsmod.Wrap(connectiontypes.ErrInvalidConnectionIdentifier, "proposed controller connection ID must not change")
+	}
+
+	if currentMetadata.HostConnectionId != metadata.HostConnectionId {
+		return errorsmod.Wrap(connectiontypes.ErrInvalidConnectionIdentifier, "proposed host connection ID must not change")
+	}
+
 	return nil
 }
-
diff --git a/modules/apps/27-interchain-accounts/types/metadata.go b/modules/apps/27-interchain-accounts/types/metadata.go
index 03e559e8e7d..f02336553de 100644
--- a/modules/apps/27-interchain-accounts/types/metadata.go
+++ b/modules/apps/27-interchain-accounts/types/metadata.go
@@ -120,13 +120,15 @@ func ValidateHostMetadata(ctx sdk.Context, channelKeeper ChannelKeeper, connecti
 		return errorsmod.Wrapf(ErrUnknownDataType, "unsupported transaction type %s", metadata.TxType)
 	}
 
-	connection, err := channelKeeper.GetConnection(ctx, connectionHops[0])
-	if err != nil {
-		return err
-	}
+	if len(connectionHops) > 0 {
+		connection, err := channelKeeper.GetConnection(ctx, connectionHops[0])
+		if err != nil {
+			return err
+		}
 
-	if err := validateConnectionParams(metadata, connection.GetCounterparty().GetConnectionID(), connectionHops[0]); err != nil {
-		return err
+		if err := validateConnectionParams(metadata, connection.GetCounterparty().GetConnectionID(), connectionHops[0]); err != nil {
+			return err
+		}
 	}
 
 	if metadata.Address != "" {

From c0c43d755afcf8ee2d506881650b9731d7a4de0d Mon Sep 17 00:00:00 2001
From: chatton <github.qpeyb@simplelogin.fr>
Date: Wed, 22 Nov 2023 09:43:31 +0000
Subject: [PATCH 18/30] chore: adding more test cases for OnChanUpgradeAck

---
 .../controller/keeper/handshake.go            |   4 +-
 .../controller/keeper/handshake_test.go       | 161 ++++++++++++++++++
 2 files changed, 163 insertions(+), 2 deletions(-)

diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
index 10222607d4d..56f3ef3e044 100644
--- a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
+++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
@@ -180,7 +180,7 @@ func (k Keeper) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, con
 // OnChanUpgradeAck implements the ack setup of the channel upgrade handshake.
 func (k Keeper) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion string) error {
 	if strings.TrimSpace(counterpartyVersion) == "" {
-		return errorsmod.Wrap(icatypes.ErrInvalidVersion, "counterparty version cannot be empty")
+		return errorsmod.Wrap(channeltypes.ErrInvalidChannelVersion, "counterparty version cannot be empty")
 	}
 
 	metadata, err := icatypes.ParseMedataFromString(counterpartyVersion)
@@ -199,7 +199,7 @@ func (k Keeper) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpart
 	}
 
 	if err := icatypes.ValidateHostMetadata(ctx, k.channelKeeper, nil, metadata); err != nil {
-		return nil
+		return err
 	}
 
 	// the interchain account address on the host chain
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 b3218e7744b..88d161586ef 100644
--- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go
+++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go
@@ -666,3 +666,164 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeTry() {
 		})
 	}
 }
+
+func (suite *KeeperTestSuite) TestOnChanUpgradeAck() {
+	const (
+		invalidVersion = "invalid-version"
+	)
+
+	var (
+		path                *ibctesting.Path
+		metadata            icatypes.Metadata
+		counterpartyVersion string
+	)
+
+	// updateMetadata is a helper function which modifies the metadata stored in the channel version
+	// and marshals it into a string to passed to OnChanUpgradeAck as the counterpartyVersion string.
+	updateMetadata := func(modificationFn func(*icatypes.Metadata)) {
+		metadata, err := icatypes.ParseMedataFromString(path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version)
+		suite.Require().NoError(err)
+		modificationFn(&metadata)
+		counterpartyVersion = string(icatypes.ModuleCdc.MustMarshalJSON(&metadata))
+	}
+
+	testCases := []struct {
+		name     string
+		malleate func()
+		expError error
+	}{
+		{
+			"success",
+			func() {},
+			nil,
+		},
+		{
+			name: "failure: invalid port ID",
+			malleate: func() {
+				path.EndpointA.ChannelConfig.PortID = "invalid-port-id"
+			},
+			expError: channeltypes.ErrChannelNotFound,
+		},
+		{
+			name: "failure: empty counterparty version",
+			malleate: func() {
+				counterpartyVersion = ""
+			},
+			expError: channeltypes.ErrInvalidChannelVersion,
+		},
+		{
+			name: "failure: invalid counterparty version",
+			malleate: func() {
+				counterpartyVersion = invalidVersion
+			},
+			expError: icatypes.ErrUnknownDataType,
+		},
+		{
+			name: "failure: invalid encoding",
+			malleate: func() {
+				updateMetadata(func(metadata *icatypes.Metadata) {
+					metadata.Encoding = "invalid-encoding"
+				})
+			},
+			expError: icatypes.ErrInvalidCodec,
+		},
+		{
+			name: "failure: invalid tx type",
+			malleate: func() {
+				updateMetadata(func(metadata *icatypes.Metadata) {
+					metadata.TxType = "invalid-tx-type"
+				})
+			},
+			expError: icatypes.ErrUnknownDataType,
+		},
+		{
+			name: "failure: invalid version",
+			malleate: func() {
+				updateMetadata(func(metadata *icatypes.Metadata) {
+					metadata.Version = invalidVersion
+				})
+			},
+			expError: icatypes.ErrInvalidVersion,
+		},
+		{
+			name: "failure: change address",
+			malleate: func() {
+				updateMetadata(func(metadata *icatypes.Metadata) {
+					metadata.Address = "different-address"
+				})
+			},
+			expError: icatypes.ErrInvalidAccountAddress,
+		},
+		{
+			name: "failure: change controller connection ID",
+			malleate: func() {
+				updateMetadata(func(metadata *icatypes.Metadata) {
+					metadata.ControllerConnectionId = differentConnectionID
+				})
+			},
+			expError: connectiontypes.ErrInvalidConnectionIdentifier,
+		},
+		{
+			name: "failure: change host connection ID",
+			malleate: func() {
+				updateMetadata(func(metadata *icatypes.Metadata) {
+					metadata.HostConnectionId = differentConnectionID
+				})
+			},
+			expError: connectiontypes.ErrInvalidConnectionIdentifier,
+		},
+	}
+
+	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))
+
+			err = path.EndpointA.ChanUpgradeInit()
+			suite.Require().NoError(err)
+
+			err = path.EndpointB.ChanUpgradeTry()
+			suite.Require().NoError(err)
+
+			counterpartyVersion = path.EndpointB.GetChannel().Version
+
+			tc.malleate() // malleate mutates test data
+
+			err = suite.chainA.GetSimApp().ICAControllerKeeper.OnChanUpgradeAck(
+				suite.chainA.GetContext(),
+				path.EndpointA.ChannelConfig.PortID,
+				path.EndpointA.ChannelID,
+				counterpartyVersion,
+			)
+
+			expPass := tc.expError == nil
+			if expPass {
+				suite.Require().NoError(err)
+			} else {
+				suite.Require().ErrorIs(err, tc.expError)
+			}
+		})
+	}
+}

From 50ec448bfefabcbc090574a465da35788f159334 Mon Sep 17 00:00:00 2001
From: Charly <charly@interchain.berlin>
Date: Thu, 7 Dec 2023 17:44:38 +0800
Subject: [PATCH 19/30] fix merge conflicts, update usage of getAppData

---
 .../controller/keeper/handshake_test.go             |  6 ++----
 .../27-interchain-accounts/host/keeper/handshake.go | 13 ++++---------
 .../27-interchain-accounts/host/keeper/keeper.go    |  5 -----
 3 files changed, 6 insertions(+), 18 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 aa60391d46a..348cbad30b1 100644
--- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go
+++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go
@@ -625,9 +625,7 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeTry() {
 			err := SetupICAPath(path, TestOwnerAddress)
 			suite.Require().NoError(err)
 
-			channel := path.EndpointA.GetChannel()
-
-			currentMetadata, err := icatypes.ParseMedataFromString(channel.Version)
+			currentMetadata, err := suite.chainB.GetSimApp().ICAControllerKeeper.GetAppMetadata(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID)
 			suite.Require().NoError(err)
 
 			metadata = icatypes.NewDefaultMetadata(path.EndpointA.ConnectionID, path.EndpointB.ConnectionID)
@@ -656,8 +654,8 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeTry() {
 
 			expPass := tc.expError == nil
 			if expPass {
-				suite.Require().Equal(path.EndpointA.GetChannel().Version, version)
 				suite.Require().NoError(err)
+				suite.Require().Equal(path.EndpointA.GetChannel().Version, version)
 			} else {
 				suite.Require().ErrorIs(err, tc.expError)
 			}
diff --git a/modules/apps/27-interchain-accounts/host/keeper/handshake.go b/modules/apps/27-interchain-accounts/host/keeper/handshake.go
index c66bffdcb67..4e5d4612b08 100644
--- a/modules/apps/27-interchain-accounts/host/keeper/handshake.go
+++ b/modules/apps/27-interchain-accounts/host/keeper/handshake.go
@@ -134,7 +134,7 @@ func (Keeper) OnChanCloseConfirm(
 // OnChanUpgradeTry performs the upgrade try step of the channel upgrade handshake.
 func (k Keeper) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, order channeltypes.Order, connectionHops []string, counterpartyVersion string) (string, error) {
 	if portID != icatypes.HostPortID {
-		return "", errorsmod.Wrapf(porttypes.ErrInvalidPort, "port ID must be %s", icatypes.HostPortID)
+		return "", errorsmod.Wrapf(porttypes.ErrInvalidPort, "expected %s, got %s", icatypes.HostPortID, portID)
 	}
 
 	counterpartyVersion = strings.TrimSpace(counterpartyVersion)
@@ -142,17 +142,12 @@ func (k Keeper) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, orde
 		return "", errorsmod.Wrap(channeltypes.ErrInvalidChannelVersion, "counterparty version cannot be empty")
 	}
 
-	metadata, err := icatypes.ParseMedataFromString(counterpartyVersion)
-	if err != nil {
-		return "", err
-	}
-
-	channel, err := k.GetChannel(ctx, portID, channelID)
+	metadata, err := icatypes.MetadataFromVersion(counterpartyVersion)
 	if err != nil {
 		return "", err
 	}
 
-	currentMetadata, err := icatypes.ParseMedataFromString(channel.Version)
+	currentMetadata, err := k.getAppMetadata(ctx, portID, channelID)
 	if err != nil {
 		return "", err
 	}
@@ -164,7 +159,7 @@ func (k Keeper) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, orde
 	// 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.HostConnectionId != connectionHops[0] {
diff --git a/modules/apps/27-interchain-accounts/host/keeper/keeper.go b/modules/apps/27-interchain-accounts/host/keeper/keeper.go
index fc6054d4efc..8084b474ca7 100644
--- a/modules/apps/27-interchain-accounts/host/keeper/keeper.go
+++ b/modules/apps/27-interchain-accounts/host/keeper/keeper.go
@@ -73,11 +73,6 @@ func NewKeeper(
 	}
 }
 
-// GetChannelKeeper returns the ChannelKeeper
-func (k *Keeper) GetChannelKeeper() icatypes.ChannelKeeper {
-	return k.channelKeeper
-}
-
 // 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)

From e40e93b94bb2ad56058de2d6666511fe49349a98 Mon Sep 17 00:00:00 2001
From: Charly <charly@interchain.berlin>
Date: Thu, 7 Dec 2023 17:46:28 +0800
Subject: [PATCH 20/30] rm unnecessary channel getter

---
 .../apps/27-interchain-accounts/host/keeper/keeper.go  | 10 ----------
 1 file changed, 10 deletions(-)

diff --git a/modules/apps/27-interchain-accounts/host/keeper/keeper.go b/modules/apps/27-interchain-accounts/host/keeper/keeper.go
index 8084b474ca7..be0cfc8a0b5 100644
--- a/modules/apps/27-interchain-accounts/host/keeper/keeper.go
+++ b/modules/apps/27-interchain-accounts/host/keeper/keeper.go
@@ -16,7 +16,6 @@ import (
 	genesistypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/genesis/types"
 	"github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/types"
 	icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/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"
 	ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
@@ -73,15 +72,6 @@ func NewKeeper(
 	}
 }
 
-// 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
-}
-
 // WithICS4Wrapper sets the ICS4Wrapper. This function may be used after
 // the keepers creation to set the middleware which is above this module
 // in the IBC application stack.

From 951f6a2312e569f1226c0ebb5f7e20667b518117 Mon Sep 17 00:00:00 2001
From: Charly <charly@interchain.berlin>
Date: Thu, 7 Dec 2023 21:06:49 +0800
Subject: [PATCH 21/30] update test

---
 .../controller/keeper/handshake_test.go       |  85 -----------
 .../controller/keeper/keeper_test.go          |   1 +
 .../host/keeper/handshake_test.go             | 135 ++++++++++++++++++
 3 files changed, 136 insertions(+), 85 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 348cbad30b1..8dc9211f3da 100644
--- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go
+++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go
@@ -5,7 +5,6 @@ import (
 	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"
-	porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"
 	host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
 )
@@ -578,87 +577,3 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() {
 		})
 	}
 }
-
-func (suite *KeeperTestSuite) TestOnChanUpgradeTry() {
-	var (
-		path     *ibctesting.Path
-		metadata icatypes.Metadata
-	)
-
-	testCases := []struct {
-		name     string
-		malleate func()
-		expError error
-	}{
-		{
-			"success",
-			func() {},
-			nil,
-		},
-		{
-			name: "failure: invalid port ID",
-			malleate: func() {
-				path.EndpointB.ChannelConfig.PortID = "invalid-port-id"
-			},
-			expError: porttypes.ErrInvalidPort,
-		},
-		{
-			name: "failure: empty counterparty version",
-			malleate: func() {
-				channel := path.EndpointA.GetChannel()
-				channel.Version = ""
-				path.EndpointA.SetChannel(channel)
-			},
-			expError: channeltypes.ErrInvalidChannelVersion,
-		},
-	}
-
-	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.chainB.GetSimApp().ICAControllerKeeper.GetAppMetadata(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.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
-
-			path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = string(icatypes.ModuleCdc.MustMarshalJSON(&metadata))
-			path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = string(icatypes.ModuleCdc.MustMarshalJSON(&metadata))
-
-			err = path.EndpointA.ChanUpgradeInit()
-			suite.Require().NoError(err)
-
-			tc.malleate() // malleate mutates test data
-
-			version, err := suite.chainB.GetSimApp().ICAHostKeeper.OnChanUpgradeTry(
-				suite.chainB.GetContext(),
-				path.EndpointB.ChannelConfig.PortID,
-				path.EndpointB.ChannelID,
-				channeltypes.ORDERED,
-				[]string{path.EndpointB.ConnectionID},
-				path.EndpointA.GetChannel().Version,
-			)
-
-			expPass := tc.expError == nil
-			if expPass {
-				suite.Require().NoError(err)
-				suite.Require().Equal(path.EndpointA.GetChannel().Version, version)
-			} else {
-				suite.Require().ErrorIs(err, tc.expError)
-			}
-		})
-	}
-}
diff --git a/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go b/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go
index 2f06fa019f5..3f7118a37f0 100644
--- a/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go
+++ b/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go
@@ -35,6 +35,7 @@ var (
 )
 
 type KeeperTestSuite struct {
+
 	testifysuite.Suite
 
 	coordinator *ibctesting.Coordinator
diff --git a/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go
index 3074faf9b8e..915e5d100c1 100644
--- a/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go
+++ b/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go
@@ -8,11 +8,17 @@ import (
 	capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types"
 	hosttypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/host/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"
+	porttypes "github.com/cosmos/ibc-go/v8/modules/core/05-port/types"
 	host "github.com/cosmos/ibc-go/v8/modules/core/24-host"
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
 )
 
+const (
+	differentConnectionID = "connection-100"
+)
+
 // open and close channel is a helper function for TestOnChanOpenTry for reopening accounts
 func (suite *KeeperTestSuite) openAndCloseChannel(path *ibctesting.Path) {
 	err := path.EndpointB.ChanOpenTry()
@@ -422,3 +428,132 @@ func (suite *KeeperTestSuite) TestOnChanCloseConfirm() {
 		})
 	}
 }
+
+func (suite *KeeperTestSuite) TestOnChanUpgradeTry() {
+	var (
+		path                *ibctesting.Path
+		metadata            icatypes.Metadata
+		counterpartyVersion string
+	)
+
+	testCases := []struct {
+		name     string
+		malleate func()
+		expError error
+	}{
+		{
+			"success",
+			func() {},
+			nil,
+		},
+		{
+			name: "failure: invalid port ID",
+			malleate: func() {
+				path.EndpointB.ChannelConfig.PortID = "invalid-port-id"
+			},
+			expError: porttypes.ErrInvalidPort,
+		},
+		{
+			name: "failure: empty counterparty version",
+			malleate: func() {
+				counterpartyVersion = ""
+			},
+			expError: channeltypes.ErrInvalidChannelVersion,
+		},
+		{
+			name: "failure: cannot parse metadata from counterparty version string",
+			malleate: func() {
+				counterpartyVersion = "invalid-version"
+			},
+			expError: icatypes.ErrUnknownDataType,
+		},
+		{
+			name: "failure: cannot decode version string from channel",
+			malleate: func() {
+				channel := path.EndpointB.GetChannel()
+				channel.Version = "invalid-metadata-string"
+				path.EndpointB.SetChannel(channel)
+			},
+			expError: icatypes.ErrUnknownDataType,
+		},
+		{
+			name: "failure: metadata encoding not supported",
+			malleate: func() {
+				metadata.Encoding = "invalid-encoding-format"
+				counterpartyVersion = string(icatypes.ModuleCdc.MustMarshalJSON(&metadata))
+			},
+			expError: icatypes.ErrInvalidCodec,
+		},
+		{
+			name: "failure: interchain account address has changed",
+			malleate: func() {
+				channel := path.EndpointB.GetChannel()
+				metadata.Address = "invalid address"
+				channel.Version = string(icatypes.ModuleCdc.MustMarshalJSON(&metadata))
+				path.EndpointB.SetChannel(channel)
+			},
+			expError: icatypes.ErrInvalidAccountAddress,
+		},
+		{
+			name: "failure: invalid connection identifier",
+			malleate: func() {
+				channel := path.EndpointB.GetChannel()
+				metadata.HostConnectionId = "invalid-connection-id"
+				channel.Version = string(icatypes.ModuleCdc.MustMarshalJSON(&metadata))
+				path.EndpointB.SetChannel(channel)
+			},
+			expError: connectiontypes.ErrInvalidConnectionIdentifier,
+		},
+	}
+
+	for _, tc := range testCases {
+		tc := tc
+
+		suite.Run(tc.name, func() {
+			suite.SetupTest() // reset
+
+			path = NewICAPath(suite.chainA, suite.chainB, icatypes.EncodingProtobuf)
+			suite.coordinator.SetupConnections(path)
+
+			err := SetupICAPath(path, TestOwnerAddress)
+			suite.Require().NoError(err)
+
+			currentMetadata, err := suite.chainB.GetSimApp().ICAHostKeeper.GetAppMetadata(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.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
+
+			path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version = string(icatypes.ModuleCdc.MustMarshalJSON(&metadata))
+			path.EndpointB.ChannelConfig.ProposedUpgrade.Fields.Version = string(icatypes.ModuleCdc.MustMarshalJSON(&metadata))
+
+			err = path.EndpointA.ChanUpgradeInit()
+			suite.Require().NoError(err)
+
+			counterpartyVersion = path.EndpointA.GetChannel().Version
+
+			tc.malleate() // malleate mutates test data
+
+			version, err := suite.chainB.GetSimApp().ICAHostKeeper.OnChanUpgradeTry(
+				suite.chainB.GetContext(),
+				path.EndpointB.ChannelConfig.PortID,
+				path.EndpointB.ChannelID,
+				channeltypes.ORDERED,
+				[]string{path.EndpointB.ConnectionID},
+				counterpartyVersion,
+			)
+
+			expPass := tc.expError == nil
+			if expPass {
+				suite.Require().NoError(err)
+				suite.Require().Equal(path.EndpointB.GetChannel().Version, version)
+			} else {
+				suite.Require().ErrorIs(err, tc.expError)
+			}
+		})
+	}
+}

From 598650c33d7458fd703fad60272ea16b4b1f2906 Mon Sep 17 00:00:00 2001
From: Charly <charly@interchain.berlin>
Date: Thu, 7 Dec 2023 21:12:31 +0800
Subject: [PATCH 22/30] rm unnnecessary fields

---
 .../27-interchain-accounts/controller/keeper/keeper_test.go   | 1 -
 .../apps/27-interchain-accounts/host/keeper/handshake_test.go | 4 ----
 2 files changed, 5 deletions(-)

diff --git a/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go b/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go
index 3f7118a37f0..2f06fa019f5 100644
--- a/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go
+++ b/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go
@@ -35,7 +35,6 @@ var (
 )
 
 type KeeperTestSuite struct {
-
 	testifysuite.Suite
 
 	coordinator *ibctesting.Coordinator
diff --git a/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go b/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go
index 915e5d100c1..80067200651 100644
--- a/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go
+++ b/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go
@@ -15,10 +15,6 @@ import (
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
 )
 
-const (
-	differentConnectionID = "connection-100"
-)
-
 // open and close channel is a helper function for TestOnChanOpenTry for reopening accounts
 func (suite *KeeperTestSuite) openAndCloseChannel(path *ibctesting.Path) {
 	err := path.EndpointB.ChanOpenTry()

From 93c674e054c80eb155634843e43a1eca24839d84 Mon Sep 17 00:00:00 2001
From: Charly <charly@interchain.berlin>
Date: Thu, 7 Dec 2023 22:16:49 +0800
Subject: [PATCH 23/30] update test

---
 .../controller/keeper/handshake.go            | 22 +++++-----
 .../controller/keeper/handshake_test.go       | 44 ++++++-------------
 2 files changed, 25 insertions(+), 41 deletions(-)

diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
index 3dc022a265f..cb406ac248e 100644
--- a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
+++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
@@ -180,25 +180,16 @@ func (k Keeper) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpart
 		return errorsmod.Wrap(channeltypes.ErrInvalidChannelVersion, "counterparty version cannot be empty")
 	}
 
-	metadata, err := icatypes.ParseMedataFromString(counterpartyVersion)
+	metadata, err := icatypes.MetadataFromVersion(counterpartyVersion)
 	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
 	}
 
-	if err := icatypes.ValidateHostMetadata(ctx, k.channelKeeper, nil, metadata); err != nil {
-		return err
-	}
-
 	// the interchain account address on the host chain
 	// must remain the same after the upgrade.
 	if currentMetadata.Address != metadata.Address {
@@ -213,5 +204,14 @@ func (k Keeper) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpart
 		return errorsmod.Wrap(connectiontypes.ErrInvalidConnectionIdentifier, "proposed host connection ID must not change")
 	}
 
+	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)
+	}
+
+	if err := icatypes.ValidateControllerMetadata(ctx, k.channelKeeper, channel.ConnectionHops, metadata); err != nil {
+		return err
+	}
+
 	return 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 a3d881c4e0b..508b7ec3002 100644
--- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go
+++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go
@@ -577,6 +577,7 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() {
 		})
 	}
 }
+
 func (suite *KeeperTestSuite) TestOnChanUpgradeAck() {
 	const (
 		invalidVersion = "invalid-version"
@@ -591,7 +592,7 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeAck() {
 	// updateMetadata is a helper function which modifies the metadata stored in the channel version
 	// and marshals it into a string to passed to OnChanUpgradeAck as the counterpartyVersion string.
 	updateMetadata := func(modificationFn func(*icatypes.Metadata)) {
-		metadata, err := icatypes.ParseMedataFromString(path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version)
+		metadata, err := icatypes.MetadataFromVersion(path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version)
 		suite.Require().NoError(err)
 		modificationFn(&metadata)
 		counterpartyVersion = string(icatypes.ModuleCdc.MustMarshalJSON(&metadata))
@@ -607,13 +608,6 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeAck() {
 			func() {},
 			nil,
 		},
-		{
-			name: "failure: invalid port ID",
-			malleate: func() {
-				path.EndpointA.ChannelConfig.PortID = "invalid-port-id"
-			},
-			expError: channeltypes.ErrChannelNotFound,
-		},
 		{
 			name: "failure: empty counterparty version",
 			malleate: func() {
@@ -629,13 +623,13 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeAck() {
 			expError: icatypes.ErrUnknownDataType,
 		},
 		{
-			name: "failure: invalid encoding",
+			name: "failure: cannot decode self version string",
 			malleate: func() {
-				updateMetadata(func(metadata *icatypes.Metadata) {
-					metadata.Encoding = "invalid-encoding"
-				})
+				channel := path.EndpointA.GetChannel()
+				channel.Version = invalidVersion
+				path.EndpointA.SetChannel(channel)
 			},
-			expError: icatypes.ErrInvalidCodec,
+			expError: icatypes.ErrUnknownDataType,
 		},
 		{
 			name: "failure: invalid tx type",
@@ -647,16 +641,7 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeAck() {
 			expError: icatypes.ErrUnknownDataType,
 		},
 		{
-			name: "failure: invalid version",
-			malleate: func() {
-				updateMetadata(func(metadata *icatypes.Metadata) {
-					metadata.Version = invalidVersion
-				})
-			},
-			expError: icatypes.ErrInvalidVersion,
-		},
-		{
-			name: "failure: change address",
+			name: "failure: interchain account address has changed",
 			malleate: func() {
 				updateMetadata(func(metadata *icatypes.Metadata) {
 					metadata.Address = "different-address"
@@ -665,19 +650,19 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeAck() {
 			expError: icatypes.ErrInvalidAccountAddress,
 		},
 		{
-			name: "failure: change controller connection ID",
+			name: "failure: controller connection ID has changed",
 			malleate: func() {
 				updateMetadata(func(metadata *icatypes.Metadata) {
-					metadata.ControllerConnectionId = differentConnectionID
+					metadata.ControllerConnectionId = "different-connection-id"
 				})
 			},
 			expError: connectiontypes.ErrInvalidConnectionIdentifier,
 		},
 		{
-			name: "failure: change host connection ID",
+			name: "failure: host connection ID has changed",
 			malleate: func() {
 				updateMetadata(func(metadata *icatypes.Metadata) {
-					metadata.HostConnectionId = differentConnectionID
+					metadata.HostConnectionId = "different-host-id"
 				})
 			},
 			expError: connectiontypes.ErrInvalidConnectionIdentifier,
@@ -696,9 +681,7 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeAck() {
 			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)
@@ -731,6 +714,7 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeAck() {
 			expPass := tc.expError == nil
 			if expPass {
 				suite.Require().NoError(err)
+				suite.Require().Equal(path.EndpointA.GetChannel().Version, counterpartyVersion)
 			} else {
 				suite.Require().ErrorIs(err, tc.expError)
 			}

From 02774a2cf52d407f8d4c41e67bfab1a7165b5722 Mon Sep 17 00:00:00 2001
From: Charly <charly@interchain.berlin>
Date: Mon, 11 Dec 2023 13:24:20 +0100
Subject: [PATCH 24/30] refactor

---
 modules/apps/27-interchain-accounts/host/keeper/handshake.go | 3 +--
 1 file changed, 1 insertion(+), 2 deletions(-)

diff --git a/modules/apps/27-interchain-accounts/host/keeper/handshake.go b/modules/apps/27-interchain-accounts/host/keeper/handshake.go
index 4e5d4612b08..e0fe28381e8 100644
--- a/modules/apps/27-interchain-accounts/host/keeper/handshake.go
+++ b/modules/apps/27-interchain-accounts/host/keeper/handshake.go
@@ -137,8 +137,7 @@ func (k Keeper) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, orde
 		return "", errorsmod.Wrapf(porttypes.ErrInvalidPort, "expected %s, got %s", icatypes.HostPortID, portID)
 	}
 
-	counterpartyVersion = strings.TrimSpace(counterpartyVersion)
-	if counterpartyVersion == "" {
+	if strings.TrimSpace(counterpartyVersion) == "" {
 		return "", errorsmod.Wrap(channeltypes.ErrInvalidChannelVersion, "counterparty version cannot be empty")
 	}
 

From f9889302e515f52c93a1a5a4175aad0f730c92cf Mon Sep 17 00:00:00 2001
From: Charly <charly@interchain.berlin>
Date: Mon, 11 Dec 2023 14:41:54 +0100
Subject: [PATCH 25/30] update validate metadata function

---
 .../controller/keeper/handshake.go            |  8 ++--
 .../host/keeper/handshake.go                  |  4 +-
 .../27-interchain-accounts/types/metadata.go  | 38 +------------------
 .../types/metadata_test.go                    |  4 +-
 4 files changed, 10 insertions(+), 44 deletions(-)

diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
index cb406ac248e..f4262aece88 100644
--- a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
+++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
@@ -61,7 +61,7 @@ func (k Keeper) OnChanOpenInit(
 		}
 	}
 
-	if err := icatypes.ValidateControllerMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
+	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
 		return "", err
 	}
 
@@ -118,7 +118,7 @@ func (k Keeper) OnChanOpenAck(
 		return errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "failed to retrieve channel %s on port %s", channelID, portID)
 	}
 
-	if err := icatypes.ValidateControllerMetadata(ctx, k.channelKeeper, channel.ConnectionHops, metadata); err != nil {
+	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, channel.ConnectionHops, metadata); err != nil {
 		return err
 	}
 
@@ -157,7 +157,7 @@ func (k Keeper) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, con
 		return "", err
 	}
 
-	if err := icatypes.ValidateControllerMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
+	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
 		return "", errorsmod.Wrap(err, "invalid metadata")
 	}
 
@@ -209,7 +209,7 @@ func (k Keeper) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpart
 		return errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "failed to retrieve channel %s on port %s", channelID, portID)
 	}
 
-	if err := icatypes.ValidateControllerMetadata(ctx, k.channelKeeper, channel.ConnectionHops, metadata); err != nil {
+	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, channel.ConnectionHops, metadata); err != nil {
 		return err
 	}
 
diff --git a/modules/apps/27-interchain-accounts/host/keeper/handshake.go b/modules/apps/27-interchain-accounts/host/keeper/handshake.go
index e0fe28381e8..c86499390ea 100644
--- a/modules/apps/27-interchain-accounts/host/keeper/handshake.go
+++ b/modules/apps/27-interchain-accounts/host/keeper/handshake.go
@@ -43,7 +43,7 @@ func (k Keeper) OnChanOpenTry(
 		return "", err
 	}
 
-	if err = icatypes.ValidateHostMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
+	if err = icatypes.ValidateMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
 		return "", err
 	}
 
@@ -151,7 +151,7 @@ func (k Keeper) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, orde
 		return "", err
 	}
 
-	if err := icatypes.ValidateHostMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
+	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
 		return "", errorsmod.Wrap(err, "invalid metadata")
 	}
 
diff --git a/modules/apps/27-interchain-accounts/types/metadata.go b/modules/apps/27-interchain-accounts/types/metadata.go
index b7f2342c65c..84caa737508 100644
--- a/modules/apps/27-interchain-accounts/types/metadata.go
+++ b/modules/apps/27-interchain-accounts/types/metadata.go
@@ -78,8 +78,8 @@ func IsPreviousMetadataEqual(previousVersion string, metadata Metadata) bool {
 		previousMetadata.TxType == metadata.TxType)
 }
 
-// ValidateControllerMetadata performs validation of the provided ICS27 controller metadata parameters
-func ValidateControllerMetadata(ctx sdk.Context, channelKeeper ChannelKeeper, connectionHops []string, metadata Metadata) error {
+// ValidateMetadata performs validation of the provided ICS27 controller/host metadata parameters
+func ValidateMetadata(ctx sdk.Context, channelKeeper ChannelKeeper, connectionHops []string, metadata Metadata) error {
 	if !isSupportedEncoding(metadata.Encoding) {
 		return errorsmod.Wrapf(ErrInvalidCodec, "unsupported encoding format %s", metadata.Encoding)
 	}
@@ -110,40 +110,6 @@ func ValidateControllerMetadata(ctx sdk.Context, channelKeeper ChannelKeeper, co
 	return nil
 }
 
-// ValidateHostMetadata performs validation of the provided ICS27 host metadata parameters
-func ValidateHostMetadata(ctx sdk.Context, channelKeeper ChannelKeeper, connectionHops []string, metadata Metadata) error {
-	if !isSupportedEncoding(metadata.Encoding) {
-		return errorsmod.Wrapf(ErrInvalidCodec, "unsupported encoding format %s", metadata.Encoding)
-	}
-
-	if !isSupportedTxType(metadata.TxType) {
-		return errorsmod.Wrapf(ErrUnknownDataType, "unsupported transaction type %s", metadata.TxType)
-	}
-
-	if len(connectionHops) > 0 {
-		connection, err := channelKeeper.GetConnection(ctx, connectionHops[0])
-		if err != nil {
-			return err
-		}
-
-		if err := validateConnectionParams(metadata, connection.GetCounterparty().GetConnectionID(), connectionHops[0]); err != nil {
-			return err
-		}
-	}
-
-	if metadata.Address != "" {
-		if err := ValidateAccountAddress(metadata.Address); err != nil {
-			return err
-		}
-	}
-
-	if metadata.Version != Version {
-		return errorsmod.Wrapf(ErrInvalidVersion, "expected %s, got %s", Version, metadata.Version)
-	}
-
-	return nil
-}
-
 // isSupportedEncoding returns true if the provided encoding is supported, otherwise false
 func isSupportedEncoding(encoding string) bool {
 	return slices.Contains(getSupportedEncoding(), encoding)
diff --git a/modules/apps/27-interchain-accounts/types/metadata_test.go b/modules/apps/27-interchain-accounts/types/metadata_test.go
index 9dde7d78a3f..f23bf7e29cf 100644
--- a/modules/apps/27-interchain-accounts/types/metadata_test.go
+++ b/modules/apps/27-interchain-accounts/types/metadata_test.go
@@ -275,7 +275,7 @@ func (suite *TypesTestSuite) TestValidateControllerMetadata() {
 
 			tc.malleate() // malleate mutates test data
 
-			err := types.ValidateControllerMetadata(
+			err := types.ValidateMetadata(
 				suite.chainA.GetContext(),
 				suite.chainA.App.GetIBCKeeper().ChannelKeeper,
 				[]string{ibctesting.FirstConnectionID},
@@ -430,7 +430,7 @@ func (suite *TypesTestSuite) TestValidateHostMetadata() {
 
 			tc.malleate() // malleate mutates test data
 
-			err := types.ValidateHostMetadata(
+			err := types.ValidateMetadata(
 				suite.chainA.GetContext(),
 				suite.chainA.App.GetIBCKeeper().ChannelKeeper,
 				[]string{ibctesting.FirstConnectionID},

From a4ae803ef08ad47071f2a3a8cd9fc8547e5ba7d9 Mon Sep 17 00:00:00 2001
From: Charly <charly@interchain.berlin>
Date: Mon, 11 Dec 2023 14:58:37 +0100
Subject: [PATCH 26/30] refactor validate function

---
 .../controller/keeper/handshake.go            |  8 ++--
 .../host/keeper/handshake.go                  |  4 +-
 .../27-interchain-accounts/types/metadata.go  | 43 ++++++++++++++++++-
 .../types/metadata_test.go                    |  2 +
 4 files changed, 49 insertions(+), 8 deletions(-)

diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
index f4262aece88..2c0177c51b3 100644
--- a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
+++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
@@ -61,7 +61,7 @@ func (k Keeper) OnChanOpenInit(
 		}
 	}
 
-	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
+	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, connectionHops, metadata, true); err != nil {
 		return "", err
 	}
 
@@ -118,7 +118,7 @@ func (k Keeper) OnChanOpenAck(
 		return errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "failed to retrieve channel %s on port %s", channelID, portID)
 	}
 
-	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, channel.ConnectionHops, metadata); err != nil {
+	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, channel.ConnectionHops, metadata, true); err != nil {
 		return err
 	}
 
@@ -157,7 +157,7 @@ func (k Keeper) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, con
 		return "", err
 	}
 
-	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
+	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, connectionHops, metadata, true); err != nil {
 		return "", errorsmod.Wrap(err, "invalid metadata")
 	}
 
@@ -209,7 +209,7 @@ func (k Keeper) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpart
 		return errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "failed to retrieve channel %s on port %s", channelID, portID)
 	}
 
-	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, channel.ConnectionHops, metadata); err != nil {
+	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, channel.ConnectionHops, metadata, true); err != nil {
 		return err
 	}
 
diff --git a/modules/apps/27-interchain-accounts/host/keeper/handshake.go b/modules/apps/27-interchain-accounts/host/keeper/handshake.go
index c86499390ea..2021284c4a1 100644
--- a/modules/apps/27-interchain-accounts/host/keeper/handshake.go
+++ b/modules/apps/27-interchain-accounts/host/keeper/handshake.go
@@ -43,7 +43,7 @@ func (k Keeper) OnChanOpenTry(
 		return "", err
 	}
 
-	if err = icatypes.ValidateMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
+	if err = icatypes.ValidateMetadata(ctx, k.channelKeeper, connectionHops, metadata, false); err != nil {
 		return "", err
 	}
 
@@ -151,7 +151,7 @@ func (k Keeper) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, orde
 		return "", err
 	}
 
-	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
+	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, connectionHops, metadata, false); err != nil {
 		return "", errorsmod.Wrap(err, "invalid metadata")
 	}
 
diff --git a/modules/apps/27-interchain-accounts/types/metadata.go b/modules/apps/27-interchain-accounts/types/metadata.go
index 84caa737508..4fc03fc3adf 100644
--- a/modules/apps/27-interchain-accounts/types/metadata.go
+++ b/modules/apps/27-interchain-accounts/types/metadata.go
@@ -79,7 +79,7 @@ func IsPreviousMetadataEqual(previousVersion string, metadata Metadata) bool {
 }
 
 // ValidateMetadata performs validation of the provided ICS27 controller/host metadata parameters
-func ValidateMetadata(ctx sdk.Context, channelKeeper ChannelKeeper, connectionHops []string, metadata Metadata) error {
+func ValidateMetadata(ctx sdk.Context, channelKeeper ChannelKeeper, connectionHops []string, metadata Metadata, isController bool) error {
 	if !isSupportedEncoding(metadata.Encoding) {
 		return errorsmod.Wrapf(ErrInvalidCodec, "unsupported encoding format %s", metadata.Encoding)
 	}
@@ -93,7 +93,46 @@ func ValidateMetadata(ctx sdk.Context, channelKeeper ChannelKeeper, connectionHo
 		return err
 	}
 
-	if err := validateConnectionParams(metadata, connectionHops[0], connection.GetCounterparty().GetConnectionID()); err != nil {
+	// validate connection params expects the controller ID as the first argument, so we need to flip the arguments if
+	// the caller is the host chain
+	if isController {
+	 err = validateConnectionParams(metadata, connectionHops[0], connection.GetCounterparty().GetConnectionID())
+	} else {
+		err =  validateConnectionParams(metadata, connection.GetCounterparty().GetConnectionID(), connectionHops[0])
+	}
+	if err != nil {
+		return err
+	}
+
+	if metadata.Address != "" {
+		if err := ValidateAccountAddress(metadata.Address); err != nil {
+			return err
+		}
+	}
+
+	if metadata.Version != Version {
+		return errorsmod.Wrapf(ErrInvalidVersion, "expected %s, got %s", Version, metadata.Version)
+	}
+
+	return nil
+}
+
+// ValidateHostMetadata performs validation of the provided ICS27 host metadata parameters
+func ValidateHostMetadata(ctx sdk.Context, channelKeeper ChannelKeeper, connectionHops []string, metadata Metadata) error {
+	if !isSupportedEncoding(metadata.Encoding) {
+		return errorsmod.Wrapf(ErrInvalidCodec, "unsupported encoding format %s", metadata.Encoding)
+	}
+
+	if !isSupportedTxType(metadata.TxType) {
+		return errorsmod.Wrapf(ErrUnknownDataType, "unsupported transaction type %s", metadata.TxType)
+	}
+
+	connection, err := channelKeeper.GetConnection(ctx, connectionHops[0])
+	if err != nil {
+		return err
+	}
+
+	if err := validateConnectionParams(metadata, connection.GetCounterparty().GetConnectionID(), connectionHops[0]); err != nil {
 		return err
 	}
 
diff --git a/modules/apps/27-interchain-accounts/types/metadata_test.go b/modules/apps/27-interchain-accounts/types/metadata_test.go
index f23bf7e29cf..cc7d5e7b804 100644
--- a/modules/apps/27-interchain-accounts/types/metadata_test.go
+++ b/modules/apps/27-interchain-accounts/types/metadata_test.go
@@ -280,6 +280,7 @@ func (suite *TypesTestSuite) TestValidateControllerMetadata() {
 				suite.chainA.App.GetIBCKeeper().ChannelKeeper,
 				[]string{ibctesting.FirstConnectionID},
 				metadata,
+				true,
 			)
 
 			if tc.expPass {
@@ -435,6 +436,7 @@ func (suite *TypesTestSuite) TestValidateHostMetadata() {
 				suite.chainA.App.GetIBCKeeper().ChannelKeeper,
 				[]string{ibctesting.FirstConnectionID},
 				metadata,
+				false,
 			)
 
 			if tc.expPass {

From 2116cc6ec1e6521c5f4ecbc1b13e132730aeb86e Mon Sep 17 00:00:00 2001
From: Charly <charly@interchain.berlin>
Date: Mon, 11 Dec 2023 15:25:49 +0100
Subject: [PATCH 27/30] separate validate functions

---
 .../controller/keeper/handshake.go                  |  8 ++++----
 .../27-interchain-accounts/host/keeper/handshake.go |  4 ++--
 .../apps/27-interchain-accounts/types/metadata.go   | 13 +++----------
 .../27-interchain-accounts/types/metadata_test.go   |  6 ++----
 4 files changed, 11 insertions(+), 20 deletions(-)

diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
index 2c0177c51b3..cb406ac248e 100644
--- a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
+++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go
@@ -61,7 +61,7 @@ func (k Keeper) OnChanOpenInit(
 		}
 	}
 
-	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, connectionHops, metadata, true); err != nil {
+	if err := icatypes.ValidateControllerMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
 		return "", err
 	}
 
@@ -118,7 +118,7 @@ func (k Keeper) OnChanOpenAck(
 		return errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "failed to retrieve channel %s on port %s", channelID, portID)
 	}
 
-	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, channel.ConnectionHops, metadata, true); err != nil {
+	if err := icatypes.ValidateControllerMetadata(ctx, k.channelKeeper, channel.ConnectionHops, metadata); err != nil {
 		return err
 	}
 
@@ -157,7 +157,7 @@ func (k Keeper) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, con
 		return "", err
 	}
 
-	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, connectionHops, metadata, true); err != nil {
+	if err := icatypes.ValidateControllerMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
 		return "", errorsmod.Wrap(err, "invalid metadata")
 	}
 
@@ -209,7 +209,7 @@ func (k Keeper) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpart
 		return errorsmod.Wrapf(channeltypes.ErrChannelNotFound, "failed to retrieve channel %s on port %s", channelID, portID)
 	}
 
-	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, channel.ConnectionHops, metadata, true); err != nil {
+	if err := icatypes.ValidateControllerMetadata(ctx, k.channelKeeper, channel.ConnectionHops, metadata); err != nil {
 		return err
 	}
 
diff --git a/modules/apps/27-interchain-accounts/host/keeper/handshake.go b/modules/apps/27-interchain-accounts/host/keeper/handshake.go
index 2021284c4a1..e0fe28381e8 100644
--- a/modules/apps/27-interchain-accounts/host/keeper/handshake.go
+++ b/modules/apps/27-interchain-accounts/host/keeper/handshake.go
@@ -43,7 +43,7 @@ func (k Keeper) OnChanOpenTry(
 		return "", err
 	}
 
-	if err = icatypes.ValidateMetadata(ctx, k.channelKeeper, connectionHops, metadata, false); err != nil {
+	if err = icatypes.ValidateHostMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
 		return "", err
 	}
 
@@ -151,7 +151,7 @@ func (k Keeper) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, orde
 		return "", err
 	}
 
-	if err := icatypes.ValidateMetadata(ctx, k.channelKeeper, connectionHops, metadata, false); err != nil {
+	if err := icatypes.ValidateHostMetadata(ctx, k.channelKeeper, connectionHops, metadata); err != nil {
 		return "", errorsmod.Wrap(err, "invalid metadata")
 	}
 
diff --git a/modules/apps/27-interchain-accounts/types/metadata.go b/modules/apps/27-interchain-accounts/types/metadata.go
index 4fc03fc3adf..61c077a46f0 100644
--- a/modules/apps/27-interchain-accounts/types/metadata.go
+++ b/modules/apps/27-interchain-accounts/types/metadata.go
@@ -78,8 +78,8 @@ func IsPreviousMetadataEqual(previousVersion string, metadata Metadata) bool {
 		previousMetadata.TxType == metadata.TxType)
 }
 
-// ValidateMetadata performs validation of the provided ICS27 controller/host metadata parameters
-func ValidateMetadata(ctx sdk.Context, channelKeeper ChannelKeeper, connectionHops []string, metadata Metadata, isController bool) error {
+// ValidateControllerMetadata performs validation of the provided ICS27 controller metadata parameters
+func ValidateControllerMetadata(ctx sdk.Context, channelKeeper ChannelKeeper, connectionHops []string, metadata Metadata) error {
 	if !isSupportedEncoding(metadata.Encoding) {
 		return errorsmod.Wrapf(ErrInvalidCodec, "unsupported encoding format %s", metadata.Encoding)
 	}
@@ -93,14 +93,7 @@ func ValidateMetadata(ctx sdk.Context, channelKeeper ChannelKeeper, connectionHo
 		return err
 	}
 
-	// validate connection params expects the controller ID as the first argument, so we need to flip the arguments if
-	// the caller is the host chain
-	if isController {
-	 err = validateConnectionParams(metadata, connectionHops[0], connection.GetCounterparty().GetConnectionID())
-	} else {
-		err =  validateConnectionParams(metadata, connection.GetCounterparty().GetConnectionID(), connectionHops[0])
-	}
-	if err != nil {
+	if err := validateConnectionParams(metadata, connectionHops[0], connection.GetCounterparty().GetConnectionID()); err != nil {
 		return err
 	}
 
diff --git a/modules/apps/27-interchain-accounts/types/metadata_test.go b/modules/apps/27-interchain-accounts/types/metadata_test.go
index cc7d5e7b804..913d4244fe1 100644
--- a/modules/apps/27-interchain-accounts/types/metadata_test.go
+++ b/modules/apps/27-interchain-accounts/types/metadata_test.go
@@ -275,12 +275,11 @@ func (suite *TypesTestSuite) TestValidateControllerMetadata() {
 
 			tc.malleate() // malleate mutates test data
 
-			err := types.ValidateMetadata(
+			err := types.ValidateHostMetadata(
 				suite.chainA.GetContext(),
 				suite.chainA.App.GetIBCKeeper().ChannelKeeper,
 				[]string{ibctesting.FirstConnectionID},
 				metadata,
-				true,
 			)
 
 			if tc.expPass {
@@ -431,12 +430,11 @@ func (suite *TypesTestSuite) TestValidateHostMetadata() {
 
 			tc.malleate() // malleate mutates test data
 
-			err := types.ValidateMetadata(
+			err := types.ValidateHostMetadata(
 				suite.chainA.GetContext(),
 				suite.chainA.App.GetIBCKeeper().ChannelKeeper,
 				[]string{ibctesting.FirstConnectionID},
 				metadata,
-				false,
 			)
 
 			if tc.expPass {

From a4ebc97010fdfd5d5d18fa852d570f8cc546e753 Mon Sep 17 00:00:00 2001
From: Charly <charly@interchain.berlin>
Date: Mon, 11 Dec 2023 15:26:38 +0100
Subject: [PATCH 28/30] test

---
 modules/apps/27-interchain-accounts/types/metadata_test.go | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/modules/apps/27-interchain-accounts/types/metadata_test.go b/modules/apps/27-interchain-accounts/types/metadata_test.go
index 913d4244fe1..9dde7d78a3f 100644
--- a/modules/apps/27-interchain-accounts/types/metadata_test.go
+++ b/modules/apps/27-interchain-accounts/types/metadata_test.go
@@ -275,7 +275,7 @@ func (suite *TypesTestSuite) TestValidateControllerMetadata() {
 
 			tc.malleate() // malleate mutates test data
 
-			err := types.ValidateHostMetadata(
+			err := types.ValidateControllerMetadata(
 				suite.chainA.GetContext(),
 				suite.chainA.App.GetIBCKeeper().ChannelKeeper,
 				[]string{ibctesting.FirstConnectionID},

From 82d876a9c357c011761da33ebb51c6a563fbb639 Mon Sep 17 00:00:00 2001
From: Charly <charly@interchain.berlin>
Date: Tue, 12 Dec 2023 10:25:28 +0100
Subject: [PATCH 29/30] typo

---
 .../27-interchain-accounts/controller/keeper/handshake_test.go  | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

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 508b7ec3002..2d5ae4c16b8 100644
--- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go
+++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go
@@ -590,7 +590,7 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeAck() {
 	)
 
 	// updateMetadata is a helper function which modifies the metadata stored in the channel version
-	// and marshals it into a string to passed to OnChanUpgradeAck as the counterpartyVersion string.
+	// and marshals it into a string to pass to OnChanUpgradeAck as the counterpartyVersion string.
 	updateMetadata := func(modificationFn func(*icatypes.Metadata)) {
 		metadata, err := icatypes.MetadataFromVersion(path.EndpointA.ChannelConfig.ProposedUpgrade.Fields.Version)
 		suite.Require().NoError(err)

From 05f768515646abfaa32e2b0cea7bf7aa797a8831 Mon Sep 17 00:00:00 2001
From: Charly <charly@interchain.berlin>
Date: Tue, 12 Dec 2023 11:23:56 +0100
Subject: [PATCH 30/30] update for cbs routing

---
 .../27-interchain-accounts/controller/ibc_middleware.go    | 7 ++++++-
 1 file changed, 6 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 0f6b572349b..bde1c1b6860 100644
--- a/modules/apps/27-interchain-accounts/controller/ibc_middleware.go
+++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware.go
@@ -266,6 +266,11 @@ func (IBCMiddleware) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string,
 
 // OnChanUpgradeAck implements the IBCModule interface
 func (im IBCMiddleware) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, counterpartyVersion string) error {
+	cbs, ok := im.app.(porttypes.UpgradableModule)
+	if !ok {
+		return errorsmod.Wrap(porttypes.ErrInvalidRoute, "upgrade route not found to module in application callstack")
+	}
+
 	if !im.keeper.GetParams(ctx).ControllerEnabled {
 		return types.ErrControllerSubModuleDisabled
 	}
@@ -280,7 +285,7 @@ func (im IBCMiddleware) OnChanUpgradeAck(ctx sdk.Context, portID, channelID, cou
 	}
 
 	if im.app != nil && im.keeper.IsMiddlewareEnabled(ctx, portID, connectionID) {
-		return im.app.OnChanUpgradeAck(ctx, portID, channelID, counterpartyVersion)
+		return cbs.OnChanUpgradeAck(ctx, portID, channelID, counterpartyVersion)
 	}
 
 	return nil