feat(ica): allow unordered ica channels (backport #5633)

modules/apps/27-interchain-accounts/controller/client/cli/tx.go

@@ -14,11 +14,15 @@ 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"
+	connectiontypes "github.com/cosmos/ibc-go/v8/modules/core/03-connection/types"
+	channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
 )
 
 const (
 	// The controller chain channel version
-	flagVersion               = "version"
+	flagVersion = "version"
+	// The channel ordering
+	flagOrdering              = "ordering"
 	flagRelativePacketTimeout = "relative-packet-timeout"
 )
 
@@ -29,8 +33,8 @@ func newRegisterInterchainAccountCmd() *cobra.Command {
 		Long: strings.TrimSpace(`Register an account on the counterparty chain via the 
 connection id from the source chain. Connection identifier should be for the source chain 
 and the interchain account will be created on the counterparty chain. Callers are expected to 
-provide the appropriate application version string via {version} flag. Generates a new 
-port identifier using the provided owner string, binds to the port identifier and claims 
+provide the appropriate application version string via {version} flag and the desired ordering
+via the {ordering} flag. Generates a new port identifier using the provided owner string, binds to the port identifier and claims 
 the associated capability.`),
 		Args: cobra.ExactArgs(1),
 		RunE: func(cmd *cobra.Command, args []string) error {
@@ -46,13 +50,19 @@ the associated capability.`),
 				return err
 			}
 
-			msg := types.NewMsgRegisterInterchainAccount(connectionID, owner, version)
+			order, err := parseOrder(cmd)
+			if err != nil {
+				return err
+			}
+
+			msg := types.NewMsgRegisterInterchainAccountWithOrder(connectionID, owner, version, order)
 
 			return tx.GenerateOrBroadcastTxCLI(clientCtx, cmd.Flags(), msg)
 		},
 	}
 
 	cmd.Flags().String(flagVersion, "", "Controller chain channel version")
+	cmd.Flags().String(flagOrdering, channeltypes.ORDERED.String(), fmt.Sprintf("Channel ordering, can be one of: %s", strings.Join(connectiontypes.SupportedOrderings, ", ")))
 	flags.AddTxFlagsToCmd(cmd)
 
 	return cmd
@@ -107,3 +117,18 @@ appropriate relative timeoutTimestamp must be provided with flag {relative-packe
 
 	return cmd
 }
+
+// parseOrder gets the channel ordering from the flags.
+func parseOrder(cmd *cobra.Command) (channeltypes.Order, error) {
+	orderString, err := cmd.Flags().GetString(flagOrdering)
+	if err != nil {
+		return channeltypes.NONE, err
+	}
+
+	order, found := channeltypes.Order_value[strings.ToUpper(orderString)]
+	if !found {
+		return channeltypes.NONE, fmt.Errorf("invalid channel ordering: %s", orderString)
+	}
+
+	return channeltypes.Order(order), nil
+}

modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go

@@ -160,11 +160,6 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenInit() {
 				suite.chainA.GetSimApp().ICAControllerKeeper.SetParams(suite.chainA.GetContext(), types.NewParams(false))
 			}, false,
 		},
-		{
-			"ICA OnChanOpenInit fails - UNORDERED channel", func() {
-				channel.Ordering = channeltypes.UNORDERED
-			}, false,
-		},
 		{
 			"ICA auth module callback fails", func() {
 				suite.chainA.GetSimApp().ICAAuthModule.IBCApp.OnChanOpenInit = func(ctx sdk.Context, order channeltypes.Order, connectionHops []string,
@@ -767,9 +762,10 @@ func (suite *InterchainAccountsTestSuite) TestOnTimeoutPacket() {
 
 func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {
 	var (
-		path     *ibctesting.Path
-		isNilApp bool
-		version  string
+		path         *ibctesting.Path
+		isNilApp     bool
+		version      string
+		channelOrder channeltypes.Order
 	)
 
 	testCases := []struct {
@@ -778,7 +774,12 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {
 		expError error
 	}{
 		{
-			"success", func() {}, nil,
+			"success w/ ORDERED channel", func() {}, nil,
+		},
+		{
+			"success w/ UNORDERED channel", func() {
+				channelOrder = channeltypes.UNORDERED
+			}, nil,
 		},
 		{
 			"success: nil underlying app",
@@ -844,11 +845,13 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeInit() {
 				cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper)
 			}
 
+			channelOrder = channeltypes.ORDERED
+
 			version, err = cbs.OnChanUpgradeInit(
 				suite.chainA.GetContext(),
 				path.EndpointA.ChannelConfig.PortID,
 				path.EndpointA.ChannelID,
-				channeltypes.ORDERED,
+				channelOrder,
 				[]string{path.EndpointA.ConnectionID},
 				version,
 			)
@@ -989,15 +992,20 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeOpen() {
 		path                *ibctesting.Path
 		isNilApp            bool
 		counterpartyVersion string
+		channelOrder        channeltypes.Order
 	)
 
 	testCases := []struct {
 		name     string
 		malleate func()
 	}{
 		{
-			"success",
-			func() {},
+			"success w/ ORDERED channel", func() {},
+		},
+		{
+			"success w/ UNORDERED channel", func() {
+				channelOrder = channeltypes.UNORDERED
+			},
 		},
 		{
 			"success: nil app",
@@ -1044,11 +1052,13 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeOpen() {
 				cbs = controller.NewIBCMiddleware(nil, suite.chainA.GetSimApp().ICAControllerKeeper)
 			}
 
+			channelOrder = channeltypes.ORDERED
+
 			cbs.OnChanUpgradeOpen(
 				suite.chainA.GetContext(),
 				path.EndpointA.ChannelConfig.PortID,
 				path.EndpointA.ChannelID,
-				channeltypes.ORDERED,
+				channelOrder,
 				[]string{path.EndpointA.ConnectionID},
 				counterpartyVersion,
 			)

modules/apps/27-interchain-accounts/controller/keeper/account.go

@@ -41,7 +41,7 @@ func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, connectionID, owner,
 
 	k.SetMiddlewareEnabled(ctx, portID, connectionID)
 
-	_, err = k.registerInterchainAccount(ctx, connectionID, portID, version)
+	_, err = k.registerInterchainAccount(ctx, connectionID, portID, version, channeltypes.ORDERED)
 	if err != nil {
 		return err
 	}
@@ -51,7 +51,7 @@ func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, connectionID, owner,
 
 // registerInterchainAccount registers an interchain account, returning the channel id of the MsgChannelOpenInitResponse
 // and an error if one occurred.
-func (k Keeper) registerInterchainAccount(ctx sdk.Context, connectionID, portID, version string) (string, error) {
+func (k Keeper) registerInterchainAccount(ctx sdk.Context, connectionID, portID, version string, order channeltypes.Order) (string, error) {
 	// if there is an active channel for this portID / connectionID return an error
 	activeChannelID, found := k.GetOpenActiveChannel(ctx, connectionID, portID)
 	if found {
@@ -69,7 +69,7 @@ func (k Keeper) registerInterchainAccount(ctx sdk.Context, connectionID, portID,
 		}
 	}
 
-	msg := channeltypes.NewMsgChannelOpenInit(portID, version, channeltypes.ORDERED, []string{connectionID}, icatypes.HostPortID, authtypes.NewModuleAddress(icatypes.ModuleName).String())
+	msg := channeltypes.NewMsgChannelOpenInit(portID, version, order, []string{connectionID}, icatypes.HostPortID, authtypes.NewModuleAddress(icatypes.ModuleName).String())
 	handler := k.msgRouter.Handler(msg)
 	res, err := handler(ctx, msg)
 	if err != nil {

modules/apps/27-interchain-accounts/controller/keeper/handshake.go

@@ -15,8 +15,7 @@ import (
 )
 
 // OnChanOpenInit performs basic validation of channel initialization.
-// The channel order must be ORDERED, the counterparty port identifier
-// must be the host chain representation as defined in the types package,
+// The counterparty port identifier must be the host chain representation as defined in the types package,
 // the channel version must be equal to the version in the types package,
 // there must not be an active channel for the specfied port identifier,
 // and the interchain accounts module must be able to claim the channel
@@ -31,10 +30,6 @@ func (k Keeper) OnChanOpenInit(
 	counterparty channeltypes.Counterparty,
 	version string,
 ) (string, error) {
-	if order != channeltypes.ORDERED {
-		return "", errorsmod.Wrapf(channeltypes.ErrInvalidChannelOrdering, "expected %s channel, got %s", channeltypes.ORDERED, order)
-	}
-
 	if !strings.HasPrefix(portID, icatypes.ControllerPortPrefix) {
 		return "", errorsmod.Wrapf(icatypes.ErrInvalidControllerPort, "expected %s{owner-account-address}, got %s", icatypes.ControllerPortPrefix, portID)
 	}
@@ -72,8 +67,12 @@ func (k Keeper) OnChanOpenInit(
 			panic(fmt.Errorf("active channel mapping set for %s but channel does not exist in channel store", activeChannelID))
 		}
 
-		if channel.IsOpen() {
-			return "", errorsmod.Wrapf(icatypes.ErrActiveChannelAlreadySet, "existing active channel %s for portID %s is already OPEN", activeChannelID, portID)
+		if channel.State != channeltypes.CLOSED {
+			return "", errorsmod.Wrapf(icatypes.ErrActiveChannelAlreadySet, "existing active channel %s for portID %s must be %s", activeChannelID, portID, channeltypes.CLOSED)
+		}
+
+		if channel.Ordering != order {
+			return "", errorsmod.Wrapf(channeltypes.ErrInvalidChannelOrdering, "order cannot change when reopening a channel expected %s, got %s", channel.Ordering, order)
 		}
 
 		appVersion, found := k.GetAppVersion(ctx, portID, activeChannelID)
@@ -149,19 +148,13 @@ func (Keeper) OnChanCloseConfirm(
 // The following may be changed:
 // - tx type (must be supported)
 // - encoding (must be supported)
+// - order
 //
 // The following may not be changed:
-// - order
 // - connectionHops (and subsequently host/controller connectionIDs)
 // - interchain account address
 // - ICS27 protocol version
 func (k Keeper) OnChanUpgradeInit(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, proposedversion string) (string, error) {
-	// verify order has not changed
-	// support for unordered ICA channels is not implemented yet
-	if proposedOrder != channeltypes.ORDERED {
-		return "", errorsmod.Wrapf(channeltypes.ErrInvalidChannelOrdering, "expected %s channel, got %s", channeltypes.ORDERED, proposedOrder)
-	}
-
 	// verify connection hops has not changed
 	connectionID, err := k.GetConnectionID(ctx, portID, channelID)
 	if err != nil {