feat(release/v8.3.x): use unordered ordering by default for new ica c…

…hannels (#6251)

* feat(release/v8.3.x): use unordered order for new ICA channels

* add legacy API register interchain account function that accepts ordering

* fix callbacks tests

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

@@ -62,7 +62,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

@@ -82,7 +82,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.RegisterInterchainAccountWithOrdering(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, TestVersion, channeltypes.ORDERED); err != nil {
 		return err
 	}
 
@@ -1164,7 +1164,7 @@ func (suite *InterchainAccountsTestSuite) TestInFlightHandshakeRespectsGoAPICall
 
 	// attempt to start a second handshake via the controller msg server
 	msgServer := controllerkeeper.NewMsgServerImpl(&suite.chainA.GetSimApp().ICAControllerKeeper)
-	msgRegisterInterchainAccount := types.NewMsgRegisterInterchainAccount(path.EndpointA.ConnectionID, suite.chainA.SenderAccount.GetAddress().String(), TestVersion)
+	msgRegisterInterchainAccount := types.NewMsgRegisterInterchainAccountWithOrdering(path.EndpointA.ConnectionID, suite.chainA.SenderAccount.GetAddress().String(), TestVersion, channeltypes.ORDERED)
 
 	res, err := msgServer.RegisterInterchainAccount(suite.chainA.GetContext(), msgRegisterInterchainAccount)
 	suite.Require().Error(err)
@@ -1177,7 +1177,7 @@ func (suite *InterchainAccountsTestSuite) TestInFlightHandshakeRespectsMsgServer
 
 	// initiate a channel handshake such that channel.State == INIT
 	msgServer := controllerkeeper.NewMsgServerImpl(&suite.chainA.GetSimApp().ICAControllerKeeper)
-	msgRegisterInterchainAccount := types.NewMsgRegisterInterchainAccount(path.EndpointA.ConnectionID, suite.chainA.SenderAccount.GetAddress().String(), TestVersion)
+	msgRegisterInterchainAccount := types.NewMsgRegisterInterchainAccountWithOrdering(path.EndpointA.ConnectionID, suite.chainA.SenderAccount.GetAddress().String(), TestVersion, channeltypes.ORDERED)
 
 	res, err := msgServer.RegisterInterchainAccount(suite.chainA.GetContext(), msgRegisterInterchainAccount)
 	suite.Require().NotNil(res)
@@ -1210,7 +1210,7 @@ func (suite *InterchainAccountsTestSuite) TestClosedChannelReopensWithMsgServer(
 
 	// route a new MsgRegisterInterchainAccount in order to reopen the
 	msgServer := controllerkeeper.NewMsgServerImpl(&suite.chainA.GetSimApp().ICAControllerKeeper)
-	msgRegisterInterchainAccount := types.NewMsgRegisterInterchainAccount(path.EndpointA.ConnectionID, suite.chainA.SenderAccount.GetAddress().String(), path.EndpointA.ChannelConfig.Version)
+	msgRegisterInterchainAccount := types.NewMsgRegisterInterchainAccountWithOrdering(path.EndpointA.ConnectionID, suite.chainA.SenderAccount.GetAddress().String(), path.EndpointA.ChannelConfig.Version, channeltypes.ORDERED)
 
 	res, err := msgServer.RegisterInterchainAccount(suite.chainA.GetContext(), msgRegisterInterchainAccount)
 	suite.Require().NoError(err)

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

@@ -41,7 +41,48 @@ func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, connectionID, owner,
 
 	k.SetMiddlewareEnabled(ctx, portID, connectionID)
 
-	_, err = k.registerInterchainAccount(ctx, connectionID, portID, version, channeltypes.ORDERED)
+	_, err = k.registerInterchainAccount(ctx, connectionID, portID, version, channeltypes.UNORDERED)
+	if err != nil {
+		return err
+	}
+
+	return nil
+}
+
+// RegisterInterchainAccountWithOrdering is the entry point to registering an interchain account:
+// - It generates a new port identifier using the provided owner string, binds to the port identifier and claims the associated capability.
+// - Callers are expected to provide the appropriate application version string.
+// - For example, this could be an ICS27 encoded metadata type or an ICS29 encoded metadata type with a nested application version.
+// - A new MsgChannelOpenInit is routed through the MsgServiceRouter, executing the OnOpenChanInit callback stack as configured.
+// - An error is returned if the port identifier is already in use. Gaining access to interchain accounts whose channels
+// have closed cannot be done with this function. A regular MsgChannelOpenInit must be used.
+//
+// Deprecated: this is a legacy API that is only intended to function correctly in workflows where an underlying authentication application has been set.
+// Calling this API will result in all packet callbacks being routed to the underlying application.
+
+// Please use MsgRegisterInterchainAccount for use cases which do not need to route to an underlying application.
+
+// 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) RegisterInterchainAccountWithOrdering(ctx sdk.Context, connectionID, owner, version string, ordering channeltypes.Order) error {
+	portID, err := icatypes.NewControllerPortID(owner)
+	if err != nil {
+		return err
+	}
+
+	if k.IsMiddlewareDisabled(ctx, portID, connectionID) && !k.IsActiveChannelClosed(ctx, connectionID, portID) {
+		return errorsmod.Wrap(icatypes.ErrInvalidChannelFlow, "channel is already active or a handshake is in flight")
+	}
+
+	k.SetMiddlewareEnabled(ctx, portID, connectionID)
+
+	// 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/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.RegisterInterchainAccountWithOrdering(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, TestVersion, channeltypes.ORDERED); err != nil {
 		return err
 	}
 

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

@@ -40,10 +40,10 @@ func (s msgServer) RegisterInterchainAccount(goCtx context.Context, msg *types.M
 
 	s.SetMiddlewareDisabled(ctx, portID, msg.ConnectionId)
 
-	// use ORDER_ORDERED as default in case msg's ordering is NONE
+	// use ORDER_UNORDERED as default in case msg's ordering is NONE
 	var order channeltypes.Order
 	if msg.Ordering == channeltypes.NONE {
-		order = channeltypes.ORDERED
+		order = channeltypes.UNORDERED
 	} else {
 		order = msg.Ordering
 	}

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"
 	)
 
@@ -35,10 +36,11 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount_MsgServer() {
 			func() {},
 		},
 		{
-			"success: ordering falls back to ORDERED if not specified",
+			"success: ordering falls back to UNORDERED if not specified",
 			true,
 			func() {
 				msg.Ordering = channeltypes.NONE
+				expectedOrderding = channeltypes.UNORDERED
 			},
 		},
 		{
@@ -77,12 +79,14 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount_MsgServer() {
 		tc := tc
 
 		suite.Run(tc.name, func() {
+			expectedOrderding = channeltypes.ORDERED
+
 			suite.SetupTest()
 
 			path := NewICAPath(suite.chainA, suite.chainB)
 			suite.coordinator.SetupConnections(path)
 
-			msg = types.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, ibctesting.TestAccAddress, "")
+			msg = types.NewMsgRegisterInterchainAccountWithOrdering(ibctesting.FirstConnectionID, ibctesting.TestAccAddress, "", channeltypes.ORDERED)
 
 			tc.malleate()
 
@@ -103,7 +107,7 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount_MsgServer() {
 				path.EndpointA.ChannelConfig.PortID = res.PortId
 				path.EndpointA.ChannelID = res.ChannelId
 				channel := path.EndpointA.GetChannel()
-				suite.Require().Equal(channeltypes.ORDERED, channel.Ordering)
+				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

@@ -66,7 +66,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.RegisterInterchainAccountWithOrdering(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version, channeltypes.ORDERED); err != nil {
 		return err
 	}
 

modules/apps/27-interchain-accounts/controller/types/msgs.go

@@ -36,14 +36,14 @@ func NewMsgRegisterInterchainAccountWithOrdering(connectionID, owner, version st
 }
 
 // NewMsgRegisterInterchainAccount creates a new instance of MsgRegisterInterchainAccount.
-// It uses channeltypes.ORDERED as the default ordering. Breakage in v9.0.0 will allow the ordering to be provided
-// directly. Use NewMsgRegisterInterchainAccountWithOrder to provide the ordering in previous versions.
+// It uses channeltypes.UNORDERED as the default ordering. Breakage in v9.0.0 will allow the ordering to be provided
+// directly. Use NewMsgRegisterInterchainAccountWithOrdering to provide the ordering in previous versions.
 func NewMsgRegisterInterchainAccount(connectionID, owner, version string) *MsgRegisterInterchainAccount {
 	return &MsgRegisterInterchainAccount{
 		ConnectionId: connectionID,
 		Owner:        owner,
 		Version:      version,
-		Ordering:     channeltypes.ORDERED,
+		Ordering:     channeltypes.UNORDERED,
 	}
 }
 

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.RegisterInterchainAccountWithOrdering(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.RegisterInterchainAccountWithOrdering(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.RegisterInterchainAccountWithOrdering(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version, channeltypes.ORDERED); err != nil {
 		return err
 	}
 

modules/apps/callbacks/callbacks_test.go

@@ -24,6 +24,7 @@ import (
 	icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types"
 	feetypes "github.com/cosmos/ibc-go/v8/modules/apps/29-fee/types"
 	transfertypes "github.com/cosmos/ibc-go/v8/modules/apps/transfer/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"
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
 	ibcmock "github.com/cosmos/ibc-go/v8/testing/mock"
@@ -153,7 +154,7 @@ func (s *CallbacksTestSuite) SetupICATest() string {
 // RegisterInterchainAccount submits a MsgRegisterInterchainAccount and updates the controller endpoint with the
 // channel created.
 func (s *CallbacksTestSuite) RegisterInterchainAccount(owner string) {
-	msgRegister := icacontrollertypes.NewMsgRegisterInterchainAccount(s.path.EndpointA.ConnectionID, owner, s.path.EndpointA.ChannelConfig.Version)
+	msgRegister := icacontrollertypes.NewMsgRegisterInterchainAccountWithOrdering(s.path.EndpointA.ConnectionID, owner, s.path.EndpointA.ChannelConfig.Version, channeltypes.ORDERED)
 
 	res, err := s.chainA.SendMsgs(msgRegister)
 	s.Require().NotEmpty(res)