imp: use UNORDERED as default ordering for new ICA channels

CHANGELOG.md

@@ -55,6 +55,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 * (core/04-channel) [\#6023](https://github.com/cosmos/ibc-go/pull/6023) Remove emission of non-hexlified event attributes `packet_data` and `packet_ack`.
 * (core) [\#6320](https://github.com/cosmos/ibc-go/pull/6320) Remove unnecessary `Proof` interface from `exported` package.
 * (core/05-port) [\#6341](https://github.com/cosmos/ibc-go/pull/6341) Modify `UnmarshalPacketData` interface to take in the context, portID, and channelID. This allows for packet data's to be unmarshaled based on the channel version.
+* (apps/27-interchain-accounts) [\#6433](https://github.com/cosmos/ibc-go/pull/6433) Use UNORDERED as the default ordering for new ICA channels.
 * (apps/transfer) [\#6440](https://github.com/cosmos/ibc-go/pull/6440) Remove `GetPrefixedDenom`.
 
 ### State Machine Breaking

docs/docs/02-apps/02-interchain-accounts/09-active-channels.md

@@ -11,7 +11,7 @@ The Interchain Accounts module uses either [ORDERED or UNORDERED](https://github
 
 When using `ORDERED` channels, the order of transactions when sending packets from a controller to a host chain is maintained.
 
-When using `UNORDERED` channels, there is no guarantee that the order of transactions when sending packets from the controller to the host chain is maintained.
+When using `UNORDERED` channels, there is no guarantee that the order of transactions when sending packets from the controller to the host chain is maintained. If no ordering is specified in `MsgRegisterInterchainAccount`, then the default ordering for new ICA channels is `UNORDERED`.
 
 > A limitation when using ORDERED channels is that when a packet times out the channel will be closed.
 

docs/docs/02-apps/02-interchain-accounts/10-legacy/03-keeper-api.md

@@ -19,14 +19,14 @@ The controller submodule keeper exposes two legacy functions that allow respecti
 The authentication module can begin registering interchain accounts by calling `RegisterInterchainAccount`:
 
 ```go
-if err := keeper.icaControllerKeeper.RegisterInterchainAccount(ctx, connectionID, owner.String(), version); err != nil {
+if err := keeper.icaControllerKeeper.RegisterInterchainAccount(ctx, connectionID, owner.String(), version, channeltypes.UNORDERED); err != nil {
   return err
 }
 
 return nil
 ```
 
-The `version` argument is used to support ICS-29 fee middleware for relayer incentivization of ICS-27 packets. Consumers of the `RegisterInterchainAccount` are expected to build the appropriate JSON encoded version string themselves and pass it accordingly. If an empty string is passed in the `version` argument, then the version will be initialized to a default value in the `OnChanOpenInit` callback of the controller's handler, so that channel handshake can proceed.
+The `version` argument is used to support ICS-29 fee middleware for relayer incentivization of ICS-27 packets. The `ordering` argument allows to specify the ordering of the channel that is created; if `NONE` is passed, then the default ordering will be `UNORDERED`. Consumers of the `RegisterInterchainAccount` are expected to build the appropriate JSON encoded version string themselves and pass it accordingly. If an empty string is passed in the `version` argument, then the version will be initialized to a default value in the `OnChanOpenInit` callback of the controller's handler, so that channel handshake can proceed.
 
 The following code snippet illustrates how to construct an appropriate interchain accounts `Metadata` and encode it as a JSON bytestring:
 
@@ -44,7 +44,7 @@ if err != nil {
   return err
 }
 
-if err := keeper.icaControllerKeeper.RegisterInterchainAccount(ctx, controllerConnectionID, owner.String(), string(appVersion)); err != nil {
+if err := keeper.icaControllerKeeper.RegisterInterchainAccount(ctx, controllerConnectionID, owner.String(), string(appVersion), channeltypes.UNORDERED); err != nil {
   return err
 }
 ```
@@ -75,7 +75,7 @@ if err != nil {
   return err
 }
 
-if err := keeper.icaControllerKeeper.RegisterInterchainAccount(ctx, controllerConnectionID, owner.String(), string(feeEnabledVersion)); err != nil {
+if err := keeper.icaControllerKeeper.RegisterInterchainAccount(ctx, controllerConnectionID, owner.String(), string(feeEnabledVersion), channeltypes.UNORDERED); err != nil {
   return err
 }
 ```

docs/docs/05-migrations/13-v8-to-v9.md

@@ -84,6 +84,17 @@ func NewKeeper(
 ) Keeper
 ```
 
+The legacy function `RegisterInterchainAccount` takes now an extra parameter to specify the ordering of new ICA channels:
+
+```diff
+func (k Keeper) RegisterInterchainAccount(
+  ctx sdk.Context,
+  connectionID, owner,
+  version string,
++ ordering channeltypes.Order
+) error {
+```
+
 ## Relayers
 
 - Renaming of event attribute keys in [#5603](https://github.com/cosmos/ibc-go/pull/5603).

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

@@ -71,7 +71,7 @@ the associated capability.`),
 	}
 
 	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, ", ")))
+	cmd.Flags().String(flagOrdering, channeltypes.UNORDERED.String(), fmt.Sprintf("Channel ordering, can be one of: %s", strings.Join(connectiontypes.SupportedOrderings, ", ")))
 	flags.AddTxFlagsToCmd(cmd)
 
 	return cmd

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

@@ -78,7 +78,7 @@ func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) erro
 
 	channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext())
 
-	if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, TestVersion); err != nil {
+	if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, TestVersion, channeltypes.ORDERED); err != nil {
 		return err
 	}
 

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

@@ -29,7 +29,7 @@ import (
 // Prior to v6.x.x of ibc-go, the controller module was only functional as middleware, with authentication performed
 // by the underlying application. For a full summary of the changes in v6.x.x, please see ADR009.
 // This API will be removed in later releases.
-func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, connectionID, owner, version string) error {
+func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, connectionID, owner, version string, ordering channeltypes.Order) error {
 	portID, err := icatypes.NewControllerPortID(owner)
 	if err != nil {
 		return err
@@ -41,7 +41,12 @@ func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, connectionID, owner,
 
 	k.SetMiddlewareEnabled(ctx, portID, connectionID)
 
-	_, err = k.registerInterchainAccount(ctx, connectionID, portID, version, channeltypes.ORDERED)
+	// use ORDER_UNORDERED as default in case ordering is NONE
+	if ordering == channeltypes.NONE {
+		ordering = channeltypes.UNORDERED
+	}
+
+	_, err = k.registerInterchainAccount(ctx, connectionID, portID, version, ordering)
 	if err != nil {
 		return err
 	}

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

@@ -72,7 +72,7 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount() {
 
 			tc.malleate() // malleate mutates test data
 
-			err = suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), path.EndpointA.ConnectionID, owner, TestVersion)
+			err = suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), path.EndpointA.ConnectionID, owner, TestVersion, channeltypes.ORDERED)
 
 			if tc.expPass {
 				suite.Require().NoError(err)
@@ -103,7 +103,7 @@ func (suite *KeeperTestSuite) TestRegisterSameOwnerMultipleConnections() {
 		TxType:                 icatypes.TxTypeSDKMultiMsg,
 	}
 
-	err := suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), pathAToB.EndpointA.ConnectionID, owner, string(icatypes.ModuleCdc.MustMarshalJSON(metadata)))
+	err := suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), pathAToB.EndpointA.ConnectionID, owner, string(icatypes.ModuleCdc.MustMarshalJSON(metadata)), channeltypes.ORDERED)
 	suite.Require().NoError(err)
 
 	// build ICS27 metadata with connection identifiers for path A->C
@@ -115,6 +115,6 @@ func (suite *KeeperTestSuite) TestRegisterSameOwnerMultipleConnections() {
 		TxType:                 icatypes.TxTypeSDKMultiMsg,
 	}
 
-	err = suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), pathAToC.EndpointA.ConnectionID, owner, string(icatypes.ModuleCdc.MustMarshalJSON(metadata)))
+	err = suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), pathAToC.EndpointA.ConnectionID, owner, string(icatypes.ModuleCdc.MustMarshalJSON(metadata)), channeltypes.ORDERED)
 	suite.Require().NoError(err)
 }

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

@@ -90,7 +90,7 @@ func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) erro
 
 	channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext())
 
-	if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, TestVersion); err != nil {
+	if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, TestVersion, channeltypes.ORDERED); err != nil {
 		return err
 	}
 

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

@@ -9,6 +9,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"
+	channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
 	ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors"
 )
 
@@ -39,7 +40,15 @@ func (s msgServer) RegisterInterchainAccount(goCtx context.Context, msg *types.M
 
 	s.SetMiddlewareDisabled(ctx, portID, msg.ConnectionId)
 
-	channelID, err := s.registerInterchainAccount(ctx, msg.ConnectionId, portID, msg.Version, msg.Ordering)
+	// use ORDER_UNORDERED as default in case msg's ordering is NONE
+	var order channeltypes.Order
+	if msg.Ordering == channeltypes.NONE {
+		order = channeltypes.UNORDERED
+	} else {
+		order = msg.Ordering
+	}
+
+	channelID, err := s.registerInterchainAccount(ctx, msg.ConnectionId, portID, msg.Version, order)
 	if err != nil {
 		s.Logger(ctx).Error("error registering interchain account", "error", err.Error())
 		return nil, err

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

@@ -21,6 +21,7 @@ import (
 func (suite *KeeperTestSuite) TestRegisterInterchainAccount_MsgServer() {
 	var (
 		msg               *types.MsgRegisterInterchainAccount
+		expectedOrderding channeltypes.Order
 		expectedChannelID = "channel-0"
 	)
 
@@ -34,6 +35,14 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount_MsgServer() {
 			true,
 			func() {},
 		},
+		{
+			"success: ordering falls back to UNORDERED if not specified",
+			true,
+			func() {
+				msg.Ordering = channeltypes.NONE
+				expectedOrderding = channeltypes.UNORDERED
+			},
+		},
 		{
 			"invalid connection id",
 			false,
@@ -70,6 +79,8 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount_MsgServer() {
 		tc := tc
 
 		suite.Run(tc.name, func() {
+			expectedOrderding = channeltypes.ORDERED
+
 			suite.SetupTest()
 
 			path := NewICAPath(suite.chainA, suite.chainB)
@@ -92,6 +103,11 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount_MsgServer() {
 				suite.Require().Len(events, 2)
 				suite.Require().Equal(events[0].Type, channeltypes.EventTypeChannelOpenInit)
 				suite.Require().Equal(events[1].Type, sdk.EventTypeMessage)
+
+				path.EndpointA.ChannelConfig.PortID = res.PortId
+				path.EndpointA.ChannelID = res.ChannelId
+				channel := path.EndpointA.GetChannel()
+				suite.Require().Equal(expectedOrderding, channel.Ordering)
 			} else {
 				suite.Require().Error(err)
 				suite.Require().Nil(res)

modules/apps/27-interchain-accounts/controller/migrations/v6/migrations_test.go

@@ -68,7 +68,7 @@ func (*MigrationsTestSuite) RegisterInterchainAccount(endpoint *ibctesting.Endpo
 
 	channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext())
 
-	if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version); err != nil {
+	if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version, channeltypes.ORDERED); err != nil {
 		return err
 	}
 

modules/apps/27-interchain-accounts/host/ibc_module_test.go

@@ -83,7 +83,7 @@ func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) erro
 
 	channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext())
 
-	if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version); err != nil {
+	if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version, channeltypes.ORDERED); err != nil {
 		return err
 	}
 

modules/apps/27-interchain-accounts/host/keeper/keeper_test.go

@@ -112,7 +112,7 @@ func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) erro
 
 	channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext())
 
-	if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version); err != nil {
+	if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version, channeltypes.ORDERED); err != nil {
 		return err
 	}
 

modules/apps/29-fee/ica_test.go

@@ -75,7 +75,7 @@ func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) erro
 
 	channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext())
 
-	if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version); err != nil {
+	if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version, channeltypes.ORDERED); err != nil {
 		return err
 	}