feat(ica): allow unordered ica channels

docs/docs/02-apps/02-interchain-accounts/01-overview.md

@@ -42,3 +42,7 @@ The implementation assumes other IBC application modules will not bind to ports
 
 The provided interchain account host and controller implementations do not support `ChanCloseInit`. However, they do support `ChanCloseConfirm`.
 This means that the host and controller modules cannot close channels, but they will confirm channel closures initiated by other implementations of ICS-27.
+
+In the event of a channel closing (due to a packet timeout in an ordered channel, for example), the interchain account associated with that channel can become accessible again if a new channel is created with a (JSON-formatted) version string that encodes the exact same `Metadata` information of the previous channel. The channel can be reopened using either [`MsgRegisterInterchainAccount`](./05-messages.md#msgregisterinterchainaccount) or `MsgChannelOpenInit`. If `MsgRegisterInterchainAccount` is used, then it is possible to leave the `version` field of the message empty, since it will be filled in by the controller submodule. If `MsgChannelOpenInit` is used, then the `version` field must be provided with the correct JSON-encoded `Metadata` string. See section [Understanding Active Channels](./09-active-channels.md#understanding-active-channels) for more information.
+
+When reopening a channel with the default controller submodule, the ordering of the channel cannot be changed. In order to change the ordering of the channel, the channel has to go through a [channel upgrade handshake](../../01-ibc/06-channel-upgrades.md) or reopen the channel with a custom controller implementation.

docs/docs/02-apps/02-interchain-accounts/05-messages.md

@@ -17,6 +17,7 @@ type MsgRegisterInterchainAccount struct {
   Owner        string
   ConnectionID string
   Version      string
+  Order        channeltypes.Order
 }
 ```
 

docs/docs/02-apps/02-interchain-accounts/08-client.md

@@ -37,6 +37,25 @@ The `tx` commands allow users to interact with the controller submodule.
 simd tx interchain-accounts controller --help
 ```
 
+#### `register`
+
+The `register` command allows users to register an interchain account on a host chain on the provided connection.
+
+```shell
+simd tx interchain-accounts controller register [connection-id] [flags]
+```
+
+During registration a new channel is set up between controller and host. There are two flags available that influence the channel that is created:
+
+- `--version` to specify the (JSON-formatted) version string of the channel. For example: `{\"version\":\"ics27-1\",\"encoding\":\"proto3\",\"tx_type\":\"sdk_multi_msg\",\"controller_connection_id\":\"connection-0\",\"host_connection_id\":\"connection-0\"}`. Passing a custom version string is useful if you want to specify, for example, the encoding format of the interchain accounts packet data (either `proto3` or `proto3json`). If not specified the controller submodule will generate a default version string.
+- `--ordering` to specify the ordering of the channel. Available options are `order_ordered` (default if not specified) and `order_unordered`.
+
+Example:
+
+```shell
+simd tx interchain-accounts controller register connection-0 --ordering order_unordered --from cosmos1..
+```
+
 #### `send-tx`
 
 The `send-tx` command allows users to send a transaction on the provided connection to be executed using an interchain account on the host chain.

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

@@ -7,7 +7,13 @@ slug: /apps/interchain-accounts/active-channels
 
 # Understanding Active Channels
 
-The Interchain Accounts module uses [ORDERED channels](https://github.com/cosmos/ibc/tree/master/spec/core/ics-004-channel-and-packet-semantics#ordering) to maintain the order of transactions when sending packets from a controller to a host chain. A limitation when using ORDERED channels is that when a packet times out the channel will be closed.
+The Interchain Accounts module uses either [ORDERED or UNORDERED](https://github.com/cosmos/ibc/tree/master/spec/core/ics-004-channel-and-packet-semantics#ordering) channels. 
+
+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.
+
+> A limitation when using ORDERED channels is that when a packet times out the channel will be closed.
 
 In the case of a channel closing, a controller chain needs to be able to regain access to the interchain account registered on this channel. `Active Channels` enable this functionality.
 

e2e/tests/core/04-channel/upgrades_test.go

@@ -11,6 +11,7 @@ import (
 	testifysuite "github.com/stretchr/testify/suite"
 
 	sdkmath "cosmossdk.io/math"
+
 	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
 
 	"github.com/cosmos/ibc-go/e2e/testsuite"

e2e/tests/interchain_accounts/base_test.go

@@ -17,6 +17,7 @@ import (
 
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	banktypes "github.com/cosmos/cosmos-sdk/x/bank/types"
+	govtypes "github.com/cosmos/cosmos-sdk/x/gov/types"
 
 	"github.com/cosmos/ibc-go/e2e/testsuite"
 	"github.com/cosmos/ibc-go/e2e/testvalues"
@@ -26,6 +27,13 @@ import (
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
 )
 
+// orderMapping is a mapping from channel ordering to the string representation of the ordering.
+// the representation can be different depending on the relayer implementation.
+var orderMapping = map[channeltypes.Order][]string{
+	channeltypes.ORDERED:   {channeltypes.ORDERED.String(), "Ordered"},
+	channeltypes.UNORDERED: {channeltypes.UNORDERED.String(), "Unordered"},
+}
+
 func TestInterchainAccountsTestSuite(t *testing.T) {
 	testifysuite.Run(t, new(InterchainAccountsTestSuite))
 }
@@ -41,6 +49,14 @@ func (s *InterchainAccountsTestSuite) RegisterInterchainAccount(ctx context.Cont
 }
 
 func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer() {
+	s.testMsgSendTxSuccessfulTransfer(channeltypes.ORDERED)
+}
+
+func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_UnorderedChannel() {
+	s.testMsgSendTxSuccessfulTransfer(channeltypes.UNORDERED)
+}
+
+func (s *InterchainAccountsTestSuite) testMsgSendTxSuccessfulTransfer(order channeltypes.Order) {
 	t := s.T()
 	ctx := context.TODO()
 
@@ -59,7 +75,7 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer() {
 	t.Run("broadcast MsgRegisterInterchainAccount", func(t *testing.T) {
 		// explicitly set the version string because we don't want to use incentivized channels.
 		version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
-		msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version)
+		msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, order)
 
 		txResp := s.BroadcastMessages(ctx, chainA, controllerAccount, msgRegisterAccount)
 		s.AssertTxSuccess(txResp)
@@ -78,6 +94,8 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer() {
 		channels, err := relayer.GetChannels(ctx, s.GetRelayerExecReporter(), chainA.Config().ChainID)
 		s.Require().NoError(err)
 		s.Require().Equal(len(channels), 2)
+		icaChannel := channels[0]
+		s.Require().Contains(orderMapping[order], icaChannel.Ordering)
 	})
 
 	t.Run("interchain account executes a bank transfer on behalf of the corresponding owner account", func(t *testing.T) {
@@ -155,7 +173,7 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_FailedTransfer_InsufficientF
 	t.Run("broadcast MsgRegisterInterchainAccount", func(t *testing.T) {
 		// explicitly set the version string because we don't want to use incentivized channels.
 		version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
-		msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version)
+		msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, channeltypes.ORDERED)
 
 		txResp := s.BroadcastMessages(ctx, chainA, controllerAccount, msgRegisterAccount)
 		s.AssertTxSuccess(txResp)
@@ -253,7 +271,7 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterReop
 		var err error
 		// explicitly set the version string because we don't want to use incentivized channels.
 		version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
-		msgRegisterInterchainAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version)
+		msgRegisterInterchainAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, channeltypes.ORDERED)
 		s.RegisterInterchainAccount(ctx, chainA, controllerAccount, msgRegisterInterchainAccount)
 		portID, err = icatypes.NewControllerPortID(controllerAddress)
 		s.Require().NoError(err)
@@ -351,7 +369,7 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterReop
 	t.Run("register interchain account", func(t *testing.T) {
 		// explicitly set the version string because we don't want to use incentivized channels.
 		version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
-		msgRegisterInterchainAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version)
+		msgRegisterInterchainAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, channeltypes.ORDERED)
 		s.RegisterInterchainAccount(ctx, chainA, controllerAccount, msgRegisterInterchainAccount)
 
 		s.Require().NoError(test.WaitForBlocks(ctx, 10, chainA, chainB))
@@ -406,3 +424,129 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterReop
 		s.Require().Equal(expected, balance.Int64())
 	})
 }
+
+func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterUpgradingOrdertoUnordered() {
+	t := s.T()
+	ctx := context.TODO()
+
+	// setup relayers and connection-0 between two chains
+	// channel-0 is a transfer channel but it will not be used in this test case
+	relayer, _ := s.SetupChainsRelayerAndChannel(ctx, nil)
+	chainA, _ := s.GetChains()
+
+	// setup 2 accounts: controller account on chain A, a second chain B account.
+	// host account will be created when the ICA is registered
+	controllerAccount := s.CreateUserOnChainA(ctx, testvalues.StartingTokenAmount)
+	controllerAddress := controllerAccount.FormattedAddress()
+	// chainBAccount := s.CreateUserOnChainB(ctx, testvalues.StartingTokenAmount)
+
+	var (
+		portID      string
+		hostAccount string
+
+		initialChannelID = "channel-1"
+	)
+
+	t.Run("register interchain account", func(t *testing.T) {
+		var err error
+		// explicitly set the version string because we don't want to use incentivized channels.
+		version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
+		msgRegisterInterchainAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAddress, version, channeltypes.ORDERED)
+		s.RegisterInterchainAccount(ctx, chainA, controllerAccount, msgRegisterInterchainAccount)
+		portID, err = icatypes.NewControllerPortID(controllerAddress)
+		s.Require().NoError(err)
+	})
+
+	t.Run("start relayer", func(t *testing.T) {
+		s.StartRelayer(relayer)
+	})
+
+	t.Run("verify interchain account", func(t *testing.T) {
+		var err error
+		hostAccount, err = s.QueryInterchainAccount(ctx, chainA, controllerAddress, ibctesting.FirstConnectionID)
+		s.Require().NoError(err)
+		s.Require().NotZero(len(hostAccount))
+
+		_, err = s.QueryChannel(ctx, chainA, portID, initialChannelID)
+		s.Require().NoError(err)
+	})
+
+	// t.Run("fund interchain account wallet", func(t *testing.T) {
+	// 	// fund the host account account so it has some $$ to send
+	// 	err := chainB.SendFunds(ctx, interchaintest.FaucetAccountKeyName, ibc.WalletAmount{
+	// 		Address: hostAccount,
+	// 		Amount:  sdkmath.NewInt(testvalues.StartingTokenAmount),
+	// 		Denom:   chainB.Config().Denom,
+	// 	})
+	// 	s.Require().NoError(err)
+	// })
+
+	// t.Run("broadcast MsgSendTx", func(t *testing.T) {
+	// 	// assemble bank transfer message from host account to user account on host chain
+	// 	msgSend := &banktypes.MsgSend{
+	// 		FromAddress: hostAccount,
+	// 		ToAddress:   chainBAccount.FormattedAddress(),
+	// 		Amount:      sdk.NewCoins(testvalues.DefaultTransferAmount(chainB.Config().Denom)),
+	// 	}
+
+	// 	cdc := testsuite.Codec()
+
+	// 	bz, err := icatypes.SerializeCosmosTx(cdc, []proto.Message{msgSend}, icatypes.EncodingProtobuf)
+	// 	s.Require().NoError(err)
+
+	// 	packetData := icatypes.InterchainAccountPacketData{
+	// 		Type: icatypes.EXECUTE_TX,
+	// 		Data: bz,
+	// 		Memo: "e2e",
+	// 	}
+
+	// 	msgSendTx := controllertypes.NewMsgSendTx(controllerAddress, ibctesting.FirstConnectionID, uint64(5*time.Minute), packetData)
+
+	// 	resp := s.BroadcastMessages(
+	// 		ctx,
+	// 		chainA,
+	// 		controllerAccount,
+	// 		msgSendTx,
+	// 	)
+
+	// 	s.AssertTxSuccess(resp)
+
+	// 	// time for the packet to be relayed
+	// 	s.Require().NoError(test.WaitForBlocks(ctx, 5, chainA, chainB))
+	// })
+
+	// t.Run("verify tokens transferred", func(t *testing.T) {
+	// 	balance, err := s.QueryBalance(ctx, chainB, chainBAccount.FormattedAddress(), chainB.Config().Denom)
+	// 	s.Require().NoError(err)
+
+	// 	expected := testvalues.IBCTransferAmount + testvalues.StartingTokenAmount
+	// 	s.Require().Equal(expected, balance.Int64())
+	// })
+
+	channel, err := s.QueryChannel(ctx, chainA, portID, initialChannelID)
+	s.Require().NoError(err)
+
+	// upgrade the channel ordering to UNORDERED
+	upgradeFields := channeltypes.NewUpgradeFields(channeltypes.UNORDERED, channel.ConnectionHops, channel.Version)
+
+	t.Run("execute gov proposal to initiate channel upgrade", func(t *testing.T) {
+		govModuleAddress, err := s.QueryModuleAccountAddress(ctx, govtypes.ModuleName, chainA)
+		s.Require().NoError(err)
+		s.Require().NotNil(govModuleAddress)
+
+		msg := channeltypes.NewMsgChannelUpgradeInit(portID, initialChannelID, upgradeFields, govModuleAddress.String())
+		s.ExecuteAndPassGovV1Proposal(ctx, msg, chainA, controllerAccount)
+	})
+
+	t.Run("verify channel A upgraded and is now unordered", func(t *testing.T) {
+		var channel channeltypes.Channel
+		waitErr := test.WaitForCondition(time.Minute*2, time.Second*5, func() (bool, error) {
+			channel, err = s.QueryChannel(ctx, chainA, portID, initialChannelID)
+			if err != nil {
+				return false, err
+			}
+			return channel.Ordering == channeltypes.UNORDERED, nil
+		})
+		s.Require().NoErrorf(waitErr, "channel was not upgraded: expected %s got %s", channeltypes.UNORDERED, channel.Ordering)
+	})
+}

e2e/tests/interchain_accounts/gov_test.go

@@ -23,6 +23,7 @@ import (
 	"github.com/cosmos/ibc-go/e2e/testvalues"
 	controllertypes "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"
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
 )
 
@@ -53,7 +54,7 @@ func (s *InterchainAccountsGovTestSuite) TestInterchainAccountsGovIntegration()
 
 	t.Run("execute proposal for MsgRegisterInterchainAccount", func(t *testing.T) {
 		version := icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID)
-		msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, govModuleAddress.String(), version)
+		msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, govModuleAddress.String(), version, channeltypes.ORDERED)
 		s.ExecuteAndPassGovV1Proposal(ctx, msgRegisterAccount, chainA, controllerAccount)
 	})
 

e2e/tests/interchain_accounts/groups_test.go

@@ -23,6 +23,7 @@ import (
 	"github.com/cosmos/ibc-go/e2e/testvalues"
 	controllertypes "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"
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
 )
 
@@ -114,7 +115,7 @@ func (s *InterchainAccountsGroupsTestSuite) TestInterchainAccountsGroupsIntegrat
 
 	t.Run("submit proposal for MsgRegisterInterchainAccount", func(t *testing.T) {
 		groupPolicyAddr = s.QueryGroupPolicyAddress(ctx, chainA)
-		msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, groupPolicyAddr, icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID))
+		msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, groupPolicyAddr, icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID), channeltypes.ORDERED)
 
 		msgSubmitProposal, err := grouptypes.NewMsgSubmitProposal(groupPolicyAddr, []string{chainAAddress}, []sdk.Msg{msgRegisterAccount}, DefaultMetadata, grouptypes.Exec_EXEC_UNSPECIFIED, "e2e groups proposal: for MsgRegisterInterchainAccount", "e2e groups proposal: for MsgRegisterInterchainAccount")
 		s.Require().NoError(err)

e2e/tests/interchain_accounts/incentivized_test.go

@@ -23,6 +23,7 @@ import (
 	controllertypes "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"
 	feetypes "github.com/cosmos/ibc-go/v8/modules/apps/29-fee/types"
+	channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types"
 	ibctesting "github.com/cosmos/ibc-go/v8/testing"
 )
 
@@ -71,7 +72,7 @@ func (s *IncentivizedInterchainAccountsTestSuite) TestMsgSendTx_SuccessfulBankSe
 
 	t.Run("broadcast MsgRegisterInterchainAccount", func(t *testing.T) {
 		version := "" // allow version to be specified by the controller chain since both chains should support incentivized channels
-		msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAccount.FormattedAddress(), version)
+		msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAccount.FormattedAddress(), version, channeltypes.ORDERED)
 
 		txResp := s.BroadcastMessages(ctx, chainA, controllerAccount, msgRegisterAccount)
 		s.AssertTxSuccess(txResp)
@@ -249,7 +250,7 @@ func (s *IncentivizedInterchainAccountsTestSuite) TestMsgSendTx_FailedBankSend_I
 
 	t.Run("broadcast MsgRegisterInterchainAccount", func(t *testing.T) {
 		version := "" // allow version to be specified by the controller chain since both chains should support incentivized channels
-		msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAccount.FormattedAddress(), version)
+		msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, controllerAccount.FormattedAddress(), version, channeltypes.ORDERED)
 
 		txResp := s.BroadcastMessages(ctx, chainA, controllerAccount, msgRegisterAccount)
 		s.AssertTxSuccess(txResp)