From 617482212f0161342ae33d9930de6cb14e1303a6 Mon Sep 17 00:00:00 2001 From: srdtrk <59252793+srdtrk@users.noreply.github.com> Date: Wed, 17 Jan 2024 21:55:08 +0300 Subject: [PATCH] feat(ica): allow unordered ica channels (#5633) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * Remove order check for ICA host and controller upgrade callbacks (#5561) * imp: remove the channel type = ordered checks from both host and controller (#5578) * rm checks and tests, amend docustring * rm unnecessary test * When a channel reopens the ordering and metadata must not change (#5562) * chore: require active channel be CLOSED before re-opening. (#5601) * docs: Update ICA documentation with support for unordered channels (#5607) * Allow specifying order when registering ICA account (#5608) * proto: Add Order to MsgRegisterInterchainAccount. * chore: apply proto changes to go files. * Add ordering to cli tx for Register. * Add documentation line for tx now accepting ordering. * Address feedback review. Co-authored-by: Carlos Rodriguez * Address Cian's feedback; spacing. --------- Co-authored-by: Carlos Rodriguez * docs: ICA register CLI (#5625) * imp(ica/host): removed previous version validation check (#5613) * imp: removed validation check * test: updated icahost test * docs: added godocs * docs: added godocs * chore(ica/host): require active channel be CLOSED before re-opening (#5630) * chore(ica/host): require active channel be CLOSED before re-opening * Update modules/apps/27-interchain-accounts/host/keeper/handshake_test.go Co-authored-by: DimitrisJim --------- Co-authored-by: DimitrisJim * e2e: ordered ica channel is upgraded to unordered (#5616) * E2E test where unordered channel is used with ICA (#5566) * test: add test to use an unordered ICA channel * chore: add hard coded UNORDERED channel Order * proto: Add Order to MsgRegisterInterchainAccount. * chore: apply proto changes to go files. * chore: apply proto changes to go files. * chore: e2e test passing with hard coded ordered value * Add ordering to cli tx for Register. * Add documentation line for tx now accepting ordering. * Address feedback review. Co-authored-by: Carlos Rodriguez * Address Cian's feedback; spacing. * Update e2e/tests/interchain_accounts/base_test.go Co-authored-by: Charly --------- Co-authored-by: DimitrisJim Co-authored-by: Carlos Rodriguez Co-authored-by: Charly * fix: host chan open try test (#5632) * chore: fix linter and merge main * chore: doc lint issue fix * docs: add extra information about ICA channel reopening (#5631) * docs: add extra information about ICA channel reopening * add link to active channels section * Apply suggestions from code review Co-authored-by: Damian Nolan Co-authored-by: srdtrk <59252793+srdtrk@users.noreply.github.com> * Update 01-overview.md --------- Co-authored-by: Damian Nolan Co-authored-by: srdtrk <59252793+srdtrk@users.noreply.github.com> * chore: rm order checks reintroduced after merge conflict. * e2e: comment out failing e2e * chore: lintertroubles --------- Co-authored-by: Cian Hatton Co-authored-by: Charly Co-authored-by: DimitrisJim Co-authored-by: Carlos Rodriguez Co-authored-by: Damian Nolan Co-authored-by: chatton Co-authored-by: Colin Axnér <25233464+colin-axner@users.noreply.github.com> --- .../02-interchain-accounts/01-overview.md | 4 + .../02-interchain-accounts/05-messages.md | 1 + .../02-interchain-accounts/08-client.md | 19 +++ .../09-active-channels.md | 8 +- e2e/tests/core/04-channel/upgrades_test.go | 1 + e2e/tests/interchain_accounts/base_test.go | 154 +++++++++++++++++- e2e/tests/interchain_accounts/gov_test.go | 3 +- e2e/tests/interchain_accounts/groups_test.go | 3 +- .../interchain_accounts/incentivized_test.go | 5 +- .../interchain_accounts/localhost_test.go | 6 +- e2e/tests/interchain_accounts/params_test.go | 3 +- e2e/tests/upgrades/genesis_test.go | 3 +- e2e/testsuite/testconfig.go | 8 +- .../controller/client/cli/tx.go | 33 +++- .../controller/ibc_middleware_test.go | 42 +++-- .../controller/keeper/account.go | 6 +- .../controller/keeper/handshake.go | 23 +-- .../controller/keeper/handshake_test.go | 86 ++++++---- .../controller/keeper/msg_server.go | 2 +- .../controller/keeper/msg_server_test.go | 2 +- .../controller/types/msgs.go | 4 +- .../controller/types/msgs_test.go | 4 +- .../controller/types/tx.pb.go | 127 +++++++++------ .../host/ibc_module_test.go | 12 +- .../host/keeper/handshake.go | 27 +-- .../host/keeper/handshake_test.go | 79 +++++---- modules/apps/callbacks/callbacks_test.go | 3 +- .../controller/v1/tx.proto | 8 +- 28 files changed, 473 insertions(+), 203 deletions(-) diff --git a/docs/docs/02-apps/02-interchain-accounts/01-overview.md b/docs/docs/02-apps/02-interchain-accounts/01-overview.md index 42e5c6345fd..d09654230b9 100644 --- a/docs/docs/02-apps/02-interchain-accounts/01-overview.md +++ b/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. diff --git a/docs/docs/02-apps/02-interchain-accounts/05-messages.md b/docs/docs/02-apps/02-interchain-accounts/05-messages.md index e94dc7c41fc..1e40a4c3c9c 100644 --- a/docs/docs/02-apps/02-interchain-accounts/05-messages.md +++ b/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 } ``` diff --git a/docs/docs/02-apps/02-interchain-accounts/08-client.md b/docs/docs/02-apps/02-interchain-accounts/08-client.md index 023495a2aec..2fd5f44d513 100644 --- a/docs/docs/02-apps/02-interchain-accounts/08-client.md +++ b/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. diff --git a/docs/docs/02-apps/02-interchain-accounts/09-active-channels.md b/docs/docs/02-apps/02-interchain-accounts/09-active-channels.md index 3cb521b4bf3..09ae7063a68 100644 --- a/docs/docs/02-apps/02-interchain-accounts/09-active-channels.md +++ b/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. diff --git a/e2e/tests/core/04-channel/upgrades_test.go b/e2e/tests/core/04-channel/upgrades_test.go index 2e7a8a8f60c..b3cd671911a 100644 --- a/e2e/tests/core/04-channel/upgrades_test.go +++ b/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" diff --git a/e2e/tests/interchain_accounts/base_test.go b/e2e/tests/interchain_accounts/base_test.go index 1425e512787..1199e0f35b8 100644 --- a/e2e/tests/interchain_accounts/base_test.go +++ b/e2e/tests/interchain_accounts/base_test.go @@ -26,6 +26,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 +48,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 +74,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 +93,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 +172,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 +270,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 +368,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 +423,132 @@ func (s *InterchainAccountsTestSuite) TestMsgSendTx_SuccessfulTransfer_AfterReop s.Require().Equal(expected, balance.Int64()) }) } + +/* +TODO: uncomment when hermes works with upgrades, https://github.com/cosmos/ibc-go/issues/5644 +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) + }) +} +*/ diff --git a/e2e/tests/interchain_accounts/gov_test.go b/e2e/tests/interchain_accounts/gov_test.go index 3505fcd559d..212b5d173f4 100644 --- a/e2e/tests/interchain_accounts/gov_test.go +++ b/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) }) diff --git a/e2e/tests/interchain_accounts/groups_test.go b/e2e/tests/interchain_accounts/groups_test.go index 6fe99fe533b..43faca17a3f 100644 --- a/e2e/tests/interchain_accounts/groups_test.go +++ b/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) diff --git a/e2e/tests/interchain_accounts/incentivized_test.go b/e2e/tests/interchain_accounts/incentivized_test.go index 00f0ae464b0..309a8e92b8f 100644 --- a/e2e/tests/interchain_accounts/incentivized_test.go +++ b/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) diff --git a/e2e/tests/interchain_accounts/localhost_test.go b/e2e/tests/interchain_accounts/localhost_test.go index 99dbc591eb1..d486041de2e 100644 --- a/e2e/tests/interchain_accounts/localhost_test.go +++ b/e2e/tests/interchain_accounts/localhost_test.go @@ -64,7 +64,7 @@ func (s *LocalhostInterchainAccountsTestSuite) TestInterchainAccounts_Localhost( s.Require().NoError(err) t.Run("channel open init localhost - broadcast MsgRegisterInterchainAccount", func(t *testing.T) { - msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(exported.LocalhostConnectionID, userAWallet.FormattedAddress(), version) + msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(exported.LocalhostConnectionID, userAWallet.FormattedAddress(), version, channeltypes.ORDERED) txResp := s.BroadcastMessages(ctx, chainA, userAWallet, msgRegisterAccount) s.AssertTxSuccess(txResp) @@ -219,7 +219,7 @@ func (s *LocalhostInterchainAccountsTestSuite) TestInterchainAccounts_ReopenChan s.Require().NoError(err) t.Run("channel open init localhost - broadcast MsgRegisterInterchainAccount", func(t *testing.T) { - msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(exported.LocalhostConnectionID, userAWallet.FormattedAddress(), version) + msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(exported.LocalhostConnectionID, userAWallet.FormattedAddress(), version, channeltypes.ORDERED) txResp := s.BroadcastMessages(ctx, chainA, userAWallet, msgRegisterAccount) s.AssertTxSuccess(txResp) @@ -347,7 +347,7 @@ func (s *LocalhostInterchainAccountsTestSuite) TestInterchainAccounts_ReopenChan }) t.Run("channel open init localhost: create new channel for existing account", func(t *testing.T) { - msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(exported.LocalhostConnectionID, userAWallet.FormattedAddress(), version) + msgRegisterAccount := controllertypes.NewMsgRegisterInterchainAccount(exported.LocalhostConnectionID, userAWallet.FormattedAddress(), version, channeltypes.ORDERED) txResp := s.BroadcastMessages(ctx, chainA, userAWallet, msgRegisterAccount) s.AssertTxSuccess(txResp) diff --git a/e2e/tests/interchain_accounts/params_test.go b/e2e/tests/interchain_accounts/params_test.go index 5e8e0e10102..4cdc64fa371 100644 --- a/e2e/tests/interchain_accounts/params_test.go +++ b/e2e/tests/interchain_accounts/params_test.go @@ -17,6 +17,7 @@ import ( controllertypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/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" + channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" ibctesting "github.com/cosmos/ibc-go/v8/testing" ) @@ -95,7 +96,7 @@ func (s *InterchainAccountsParamsTestSuite) TestControllerEnabledParam() { t.Run("ensure that broadcasting a MsgRegisterInterchainAccount fails", 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.AssertTxFailure(txResp, controllertypes.ErrControllerSubModuleDisabled) diff --git a/e2e/tests/upgrades/genesis_test.go b/e2e/tests/upgrades/genesis_test.go index 95cba42dcf9..4b387bb128b 100644 --- a/e2e/tests/upgrades/genesis_test.go +++ b/e2e/tests/upgrades/genesis_test.go @@ -24,6 +24,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" ) @@ -89,7 +90,7 @@ func (s *GenesisTestSuite) TestIBCGenesis() { t.Run("ics27: 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) diff --git a/e2e/testsuite/testconfig.go b/e2e/testsuite/testconfig.go index f8cfe1b5b7a..ea759ba1322 100644 --- a/e2e/testsuite/testconfig.go +++ b/e2e/testsuite/testconfig.go @@ -561,7 +561,7 @@ func defaultGovv1ModifyGenesis(version string) func(ibc.ChainConfig, []byte) ([] appState[govtypes.ModuleName] = govGenBz if !testvalues.AllowAllClientsWildcardFeatureReleases.IsSupported(version) { - ibcGenBz, err := modifyClientGenesisAppState(chainConfig, appState[ibcexported.ModuleName]) + ibcGenBz, err := modifyClientGenesisAppState(appState[ibcexported.ModuleName]) if err != nil { return nil, err } @@ -628,7 +628,7 @@ func defaultGovv1Beta1ModifyGenesis(version string) func(ibc.ChainConfig, []byte return nil, fmt.Errorf("failed to extract ibc genesis bytes: %s", err) } - ibcGenesisBytes, err := modifyClientGenesisAppState(chainConfig, ibcModuleBytes) + ibcGenesisBytes, err := modifyClientGenesisAppState(ibcModuleBytes) if err != nil { return nil, err } @@ -703,8 +703,8 @@ func modifyGovv1Beta1AppState(chainConfig ibc.ChainConfig, govAppState []byte) ( return govGenBz, nil } -// modifyClientGenesisAppState takes the existing ibc app state and marshals it to a ibc GenesisState. -func modifyClientGenesisAppState(chainConfig ibc.ChainConfig, ibcAppState []byte) ([]byte, error) { +// modifyClientGenesisAppState takes the existing ibc app state and marshals it to an ibc GenesisState. +func modifyClientGenesisAppState(ibcAppState []byte) ([]byte, error) { cfg := testutil.MakeTestEncodingConfig() cdc := codec.NewProtoCodec(cfg.InterfaceRegistry) diff --git a/modules/apps/27-interchain-accounts/controller/client/cli/tx.go b/modules/apps/27-interchain-accounts/controller/client/cli/tx.go index 39f46d22533..356b7a8fc30 100644 --- a/modules/apps/27-interchain-accounts/controller/client/cli/tx.go +++ b/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.NewMsgRegisterInterchainAccount(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 +} diff --git a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go index 5ec07fb8ea2..1c7925e1842 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/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,6 +992,7 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeOpen() { path *ibctesting.Path isNilApp bool counterpartyVersion string + channelOrder channeltypes.Order ) testCases := []struct { @@ -996,8 +1000,12 @@ func (suite *InterchainAccountsTestSuite) TestOnChanUpgradeOpen() { 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, ) @@ -1215,7 +1225,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.NewMsgRegisterInterchainAccount(path.EndpointA.ConnectionID, suite.chainA.SenderAccount.GetAddress().String(), TestVersion, channeltypes.ORDERED) res, err := msgServer.RegisterInterchainAccount(suite.chainA.GetContext(), msgRegisterInterchainAccount) suite.Require().Error(err) @@ -1228,7 +1238,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.NewMsgRegisterInterchainAccount(path.EndpointA.ConnectionID, suite.chainA.SenderAccount.GetAddress().String(), TestVersion, channeltypes.ORDERED) res, err := msgServer.RegisterInterchainAccount(suite.chainA.GetContext(), msgRegisterInterchainAccount) suite.Require().NotNil(res) @@ -1261,7 +1271,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.NewMsgRegisterInterchainAccount(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) diff --git a/modules/apps/27-interchain-accounts/controller/keeper/account.go b/modules/apps/27-interchain-accounts/controller/keeper/account.go index 42d4c879d8f..cae378f5c91 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/account.go +++ b/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 { diff --git a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go b/modules/apps/27-interchain-accounts/controller/keeper/handshake.go index 8d688efb678..fbbccc7d07f 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake.go +++ b/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 specified 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 { 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 b2fd351cc06..341df9b6295 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/handshake_test.go @@ -25,12 +25,12 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { testCases := []struct { name string malleate func() - expPass bool + expError error }{ { "success", func() {}, - true, + nil, }, { "success: previous active channel closed", @@ -48,14 +48,14 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { path.EndpointA.SetChannel(channel) }, - true, + nil, }, { "success: empty channel version returns default metadata JSON string", func() { channel.Version = "" }, - true, + nil, }, { "success: channel reopening", @@ -72,7 +72,25 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { path.EndpointA.ChannelID = "" path.EndpointB.ChannelID = "" }, - true, + nil, + }, + { + "failure: different ordering from previous channel", + func() { + suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + + counterparty := channeltypes.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) + channel := channeltypes.Channel{ + State: channeltypes.CLOSED, + Ordering: channeltypes.UNORDERED, + Counterparty: counterparty, + ConnectionHops: []string{path.EndpointA.ConnectionID}, + Version: TestVersion, + } + + path.EndpointA.SetChannel(channel) + }, + channeltypes.ErrInvalidChannelOrdering, }, { "invalid metadata - previous metadata is different", @@ -96,21 +114,14 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { } path.EndpointA.SetChannel(closedChannel) }, - false, - }, - { - "invalid order - UNORDERED", - func() { - channel.Ordering = channeltypes.UNORDERED - }, - false, + icatypes.ErrInvalidVersion, }, { "invalid port ID", func() { path.EndpointA.ChannelConfig.PortID = "invalid-port-id" //nolint:goconst }, - false, + icatypes.ErrInvalidControllerPort, }, { "invalid counterparty port ID", @@ -118,7 +129,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { path.EndpointA.SetChannel(*channel) channel.Counterparty.PortId = "invalid-port-id" }, - false, + icatypes.ErrInvalidHostPort, }, { "invalid metadata bytestring", @@ -126,7 +137,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { path.EndpointA.SetChannel(*channel) channel.Version = "invalid-metadata-bytestring" }, - false, + icatypes.ErrUnknownDataType, }, { "unsupported encoding format", @@ -139,7 +150,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { channel.Version = string(versionBytes) path.EndpointA.SetChannel(*channel) }, - false, + icatypes.ErrInvalidCodec, }, { "unsupported transaction type", @@ -152,7 +163,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { channel.Version = string(versionBytes) path.EndpointA.SetChannel(*channel) }, - false, + icatypes.ErrUnknownDataType, }, { "connection not found", @@ -160,7 +171,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { channel.ConnectionHops = []string{"invalid-connnection-id"} path.EndpointA.SetChannel(*channel) }, - false, + connectiontypes.ErrConnectionNotFound, }, { "connection not found with default empty channel version", @@ -168,7 +179,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { channel.ConnectionHops = []string{"connection-10"} channel.Version = "" }, - false, + connectiontypes.ErrConnectionNotFound, }, { "invalid controller connection ID", @@ -181,7 +192,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { channel.Version = string(versionBytes) path.EndpointA.SetChannel(*channel) }, - false, + connectiontypes.ErrInvalidConnection, }, { "invalid host connection ID", @@ -194,7 +205,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { channel.Version = string(versionBytes) path.EndpointA.SetChannel(*channel) }, - false, + connectiontypes.ErrInvalidConnection, }, { "invalid version", @@ -207,10 +218,10 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { channel.Version = string(versionBytes) path.EndpointA.SetChannel(*channel) }, - false, + icatypes.ErrInvalidVersion, }, { - "channel is already active", + "channel is already active (OPEN state)", func() { suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) @@ -224,7 +235,24 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { } suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetChannel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, channel) }, - false, + icatypes.ErrActiveChannelAlreadySet, + }, + { + "channel is already active (FLUSHING state)", + func() { + suite.chainA.GetSimApp().ICAControllerKeeper.SetActiveChannelID(suite.chainA.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + + counterparty := channeltypes.NewCounterparty(path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID) + channel := channeltypes.Channel{ + State: channeltypes.FLUSHING, + Ordering: channeltypes.ORDERED, + Counterparty: counterparty, + ConnectionHops: []string{path.EndpointA.ConnectionID}, + Version: TestVersion, + } + suite.chainA.GetSimApp().IBCKeeper.ChannelKeeper.SetChannel(suite.chainA.GetContext(), path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, channel) + }, + icatypes.ErrActiveChannelAlreadySet, }, } @@ -268,11 +296,13 @@ func (suite *KeeperTestSuite) TestOnChanOpenInit() { path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID, chanCap, channel.Counterparty, channel.Version, ) - if tc.expPass { + expPass := tc.expError == nil + if expPass { suite.Require().NoError(err) suite.Require().Equal(string(versionBytes), version) } else { suite.Require().Error(err) + suite.Require().ErrorIs(err, tc.expError) } }) } @@ -510,11 +540,11 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeInit() { nil, }, { - name: "failure: invalid order", + name: "success: change order", malleate: func() { order = channeltypes.UNORDERED }, - expError: channeltypes.ErrInvalidChannelOrdering, + expError: nil, }, { name: "failure: connectionID not found", diff --git a/modules/apps/27-interchain-accounts/controller/keeper/msg_server.go b/modules/apps/27-interchain-accounts/controller/keeper/msg_server.go index 86355cac4f9..b76d2eb28f3 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/msg_server.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/msg_server.go @@ -39,7 +39,7 @@ 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) + channelID, err := s.registerInterchainAccount(ctx, msg.ConnectionId, portID, msg.Version, msg.Order) if err != nil { s.Logger(ctx).Error("error registering interchain account", "error", err.Error()) return nil, err diff --git a/modules/apps/27-interchain-accounts/controller/keeper/msg_server_test.go b/modules/apps/27-interchain-accounts/controller/keeper/msg_server_test.go index 4518a4c3825..f9040f231a0 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/msg_server_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/msg_server_test.go @@ -75,7 +75,7 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount_MsgServer() { path := NewICAPath(suite.chainA, suite.chainB) suite.coordinator.SetupConnections(path) - msg = types.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, ibctesting.TestAccAddress, "") + msg = types.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, ibctesting.TestAccAddress, "", channeltypes.ORDERED) tc.malleate() diff --git a/modules/apps/27-interchain-accounts/controller/types/msgs.go b/modules/apps/27-interchain-accounts/controller/types/msgs.go index 1afa45929cb..74db1303244 100644 --- a/modules/apps/27-interchain-accounts/controller/types/msgs.go +++ b/modules/apps/27-interchain-accounts/controller/types/msgs.go @@ -8,6 +8,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/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" host "github.com/cosmos/ibc-go/v8/modules/core/24-host" ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" ) @@ -25,11 +26,12 @@ var ( ) // NewMsgRegisterInterchainAccount creates a new instance of MsgRegisterInterchainAccount -func NewMsgRegisterInterchainAccount(connectionID, owner, version string) *MsgRegisterInterchainAccount { +func NewMsgRegisterInterchainAccount(connectionID, owner, version string, order channeltypes.Order) *MsgRegisterInterchainAccount { return &MsgRegisterInterchainAccount{ ConnectionId: connectionID, Owner: owner, Version: version, + Order: order, } } diff --git a/modules/apps/27-interchain-accounts/controller/types/msgs_test.go b/modules/apps/27-interchain-accounts/controller/types/msgs_test.go index 17bf0d98e0c..a282937e8bd 100644 --- a/modules/apps/27-interchain-accounts/controller/types/msgs_test.go +++ b/modules/apps/27-interchain-accounts/controller/types/msgs_test.go @@ -14,6 +14,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" 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" ) @@ -80,6 +81,7 @@ func TestMsgRegisterInterchainAccountValidateBasic(t *testing.T) { ibctesting.FirstConnectionID, ibctesting.TestAccAddress, icatypes.NewDefaultMetadataString(ibctesting.FirstConnectionID, ibctesting.FirstConnectionID), + channeltypes.ORDERED, ) tc.malleate() @@ -97,7 +99,7 @@ func TestMsgRegisterInterchainAccountGetSigners(t *testing.T) { expSigner, err := sdk.AccAddressFromBech32(ibctesting.TestAccAddress) require.NoError(t, err) - msg := types.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, ibctesting.TestAccAddress, "") + msg := types.NewMsgRegisterInterchainAccount(ibctesting.FirstConnectionID, ibctesting.TestAccAddress, "", channeltypes.ORDERED) encodingCfg := moduletestutil.MakeTestEncodingConfig(ica.AppModuleBasic{}) signers, _, err := encodingCfg.Codec.GetMsgV1Signers(msg) require.NoError(t, err) diff --git a/modules/apps/27-interchain-accounts/controller/types/tx.pb.go b/modules/apps/27-interchain-accounts/controller/types/tx.pb.go index 4bb498300a7..f7e724b2e90 100644 --- a/modules/apps/27-interchain-accounts/controller/types/tx.pb.go +++ b/modules/apps/27-interchain-accounts/controller/types/tx.pb.go @@ -10,7 +10,8 @@ import ( _ "github.com/cosmos/gogoproto/gogoproto" grpc1 "github.com/cosmos/gogoproto/grpc" proto "github.com/cosmos/gogoproto/proto" - types "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types" + types1 "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types" + types "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" grpc "google.golang.org/grpc" codes "google.golang.org/grpc/codes" status "google.golang.org/grpc/status" @@ -32,9 +33,10 @@ const _ = proto.GoGoProtoPackageIsVersion3 // please upgrade the proto package // MsgRegisterInterchainAccount defines the payload for Msg/RegisterAccount type MsgRegisterInterchainAccount struct { - Owner string `protobuf:"bytes,1,opt,name=owner,proto3" json:"owner,omitempty"` - ConnectionId string `protobuf:"bytes,2,opt,name=connection_id,json=connectionId,proto3" json:"connection_id,omitempty"` - Version string `protobuf:"bytes,3,opt,name=version,proto3" json:"version,omitempty"` + Owner string `protobuf:"bytes,1,opt,name=owner,proto3" json:"owner,omitempty"` + ConnectionId string `protobuf:"bytes,2,opt,name=connection_id,json=connectionId,proto3" json:"connection_id,omitempty"` + Version string `protobuf:"bytes,3,opt,name=version,proto3" json:"version,omitempty"` + Order types.Order `protobuf:"varint,4,opt,name=order,proto3,enum=ibc.core.channel.v1.Order" json:"order,omitempty"` } func (m *MsgRegisterInterchainAccount) Reset() { *m = MsgRegisterInterchainAccount{} } @@ -111,9 +113,9 @@ var xxx_messageInfo_MsgRegisterInterchainAccountResponse proto.InternalMessageIn // MsgSendTx defines the payload for Msg/SendTx type MsgSendTx struct { - Owner string `protobuf:"bytes,1,opt,name=owner,proto3" json:"owner,omitempty"` - ConnectionId string `protobuf:"bytes,2,opt,name=connection_id,json=connectionId,proto3" json:"connection_id,omitempty"` - PacketData types.InterchainAccountPacketData `protobuf:"bytes,3,opt,name=packet_data,json=packetData,proto3" json:"packet_data"` + Owner string `protobuf:"bytes,1,opt,name=owner,proto3" json:"owner,omitempty"` + ConnectionId string `protobuf:"bytes,2,opt,name=connection_id,json=connectionId,proto3" json:"connection_id,omitempty"` + PacketData types1.InterchainAccountPacketData `protobuf:"bytes,3,opt,name=packet_data,json=packetData,proto3" json:"packet_data"` // Relative timeout timestamp provided will be added to the current block time during transaction execution. // The timeout timestamp must be non-zero. RelativeTimeout uint64 `protobuf:"varint,4,opt,name=relative_timeout,json=relativeTimeout,proto3" json:"relative_timeout,omitempty"` @@ -284,47 +286,49 @@ func init() { } var fileDescriptor_7def041328c84a30 = []byte{ - // 626 bytes of a gzipped FileDescriptorProto - 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xac, 0x54, 0x4f, 0x4f, 0x13, 0x4f, - 0x18, 0xee, 0xfc, 0x58, 0xca, 0x8f, 0x01, 0x45, 0x37, 0x44, 0xca, 0x46, 0x17, 0x52, 0x3d, 0x20, - 0x09, 0x3b, 0x69, 0xd5, 0x68, 0x6a, 0x3c, 0x08, 0x78, 0x68, 0x4c, 0x93, 0x66, 0xc5, 0x84, 0x78, - 0x69, 0xa6, 0xb3, 0x93, 0x61, 0x64, 0x77, 0x66, 0xdd, 0x99, 0xae, 0x78, 0x33, 0x7a, 0x31, 0x1e, - 0x8c, 0x07, 0x3f, 0x00, 0x1f, 0x81, 0x6f, 0x21, 0x47, 0x8e, 0x9e, 0x8c, 0x81, 0x03, 0x37, 0x3f, - 0x83, 0xd9, 0x3f, 0xdd, 0xa2, 0x20, 0xc1, 0xc2, 0xad, 0xef, 0xfb, 0xf6, 0x79, 0xde, 0xe7, 0x79, - 0xe7, 0xdd, 0x17, 0x3e, 0xe4, 0x5d, 0x82, 0x70, 0x18, 0xfa, 0x9c, 0x60, 0xcd, 0xa5, 0x50, 0x88, - 0x0b, 0x4d, 0x23, 0xb2, 0x81, 0xb9, 0xe8, 0x60, 0x42, 0x64, 0x4f, 0x68, 0x85, 0x88, 0x14, 0x3a, - 0x92, 0xbe, 0x4f, 0x23, 0x14, 0xd7, 0x90, 0xde, 0x72, 0xc2, 0x48, 0x6a, 0x69, 0xd6, 0x79, 0x97, - 0x38, 0x47, 0xc1, 0xce, 0x09, 0x60, 0x67, 0x00, 0x76, 0xe2, 0x9a, 0x35, 0xcd, 0x24, 0x93, 0x29, - 0x1c, 0x25, 0xbf, 0x32, 0x26, 0xeb, 0xee, 0x99, 0x64, 0xc4, 0x35, 0x14, 0x62, 0xb2, 0x49, 0x75, - 0x8e, 0x5a, 0x19, 0x42, 0xfc, 0x11, 0x35, 0x19, 0xc9, 0x0c, 0x91, 0x2a, 0x90, 0x0a, 0x05, 0x8a, - 0x25, 0xf5, 0x40, 0xb1, 0xac, 0x50, 0x7d, 0x0f, 0xe0, 0xf5, 0x96, 0x62, 0x2e, 0x65, 0x5c, 0x69, - 0x1a, 0x35, 0x0b, 0xea, 0xc7, 0x19, 0xb3, 0x39, 0x0d, 0x47, 0xe5, 0x6b, 0x41, 0xa3, 0x0a, 0x98, - 0x07, 0x0b, 0xe3, 0x6e, 0x16, 0x98, 0x37, 0xe1, 0x25, 0x22, 0x85, 0xa0, 0x24, 0x51, 0xd4, 0xe1, - 0x5e, 0xe5, 0xbf, 0xb4, 0x3a, 0x39, 0x48, 0x36, 0x3d, 0xb3, 0x02, 0xc7, 0x62, 0x1a, 0x29, 0x2e, - 0x45, 0x65, 0x24, 0x2d, 0xf7, 0xc3, 0xc6, 0xe5, 0x0f, 0xdb, 0x73, 0xa5, 0x77, 0x87, 0x3b, 0x8b, - 0x19, 0x5d, 0xd5, 0x83, 0xb7, 0x4e, 0x13, 0xe1, 0x52, 0x15, 0x4a, 0xa1, 0xa8, 0x79, 0x03, 0x42, - 0xb2, 0x81, 0x85, 0xa0, 0x7e, 0xd2, 0x33, 0x53, 0x34, 0x9e, 0x67, 0x9a, 0x9e, 0x39, 0x03, 0xc7, - 0x42, 0x19, 0xe9, 0x81, 0x9e, 0x72, 0x12, 0x36, 0xbd, 0x86, 0x91, 0xf4, 0xab, 0xfe, 0x04, 0x70, - 0xbc, 0xa5, 0xd8, 0x33, 0x2a, 0xbc, 0xb5, 0xad, 0xf3, 0x18, 0xdb, 0x84, 0x13, 0xd9, 0x13, 0x75, - 0x3c, 0xac, 0x71, 0x6a, 0x6e, 0xa2, 0xbe, 0xea, 0x9c, 0x69, 0x51, 0xe2, 0x9a, 0x73, 0xcc, 0x5f, - 0x3b, 0x25, 0x5b, 0xc5, 0x1a, 0x2f, 0x1b, 0xbb, 0xdf, 0xe7, 0x4a, 0x2e, 0x0c, 0x8b, 0x8c, 0x79, - 0x1b, 0x5e, 0x89, 0xa8, 0x8f, 0x35, 0x8f, 0x69, 0x47, 0xf3, 0x80, 0xca, 0x9e, 0xae, 0x18, 0xf3, - 0x60, 0xc1, 0x70, 0xa7, 0xfa, 0xf9, 0xb5, 0x2c, 0x7d, 0x6c, 0xac, 0xf7, 0xe0, 0xd5, 0xc2, 0x6f, - 0x31, 0x43, 0x0b, 0xfe, 0xaf, 0xe8, 0xab, 0x1e, 0x15, 0x84, 0xa6, 0xd6, 0x0d, 0xb7, 0x88, 0xf3, - 0x39, 0x7d, 0x01, 0x70, 0xaa, 0xa5, 0xd8, 0xf3, 0xd0, 0xc3, 0x9a, 0xb6, 0x71, 0x84, 0x03, 0x65, - 0x5e, 0x83, 0x65, 0xc5, 0xd9, 0x60, 0x5c, 0x79, 0x64, 0xae, 0xc3, 0x72, 0x98, 0xfe, 0x23, 0x1d, - 0xd4, 0x44, 0xbd, 0xe1, 0xfc, 0xfb, 0xe7, 0xe2, 0x64, 0x3d, 0x72, 0xef, 0x39, 0x5f, 0x63, 0xaa, - 0x6f, 0x26, 0x6f, 0x55, 0x9d, 0x85, 0x33, 0x7f, 0xa8, 0xea, 0x7b, 0xaa, 0x7f, 0x34, 0xe0, 0x48, - 0x4b, 0x31, 0xf3, 0x2b, 0x80, 0xb3, 0x7f, 0x5f, 0xe5, 0xf6, 0x30, 0xda, 0x4e, 0xdb, 0x4b, 0x6b, - 0xfd, 0xa2, 0x19, 0x8b, 0x57, 0xfa, 0x04, 0x60, 0x39, 0x5f, 0xd4, 0x47, 0x43, 0x36, 0xc9, 0xe0, - 0xd6, 0x93, 0x73, 0xc1, 0x0b, 0x41, 0xdb, 0x00, 0x4e, 0xfe, 0xb6, 0x11, 0x2b, 0x43, 0xf2, 0x1e, - 0x25, 0xb1, 0x9e, 0x5e, 0x00, 0x49, 0x5f, 0xa2, 0x35, 0xfa, 0xf6, 0x70, 0x67, 0x11, 0x2c, 0xbf, - 0xdc, 0xdd, 0xb7, 0xc1, 0xde, 0xbe, 0x0d, 0x7e, 0xec, 0xdb, 0xe0, 0xf3, 0x81, 0x5d, 0xda, 0x3b, - 0xb0, 0x4b, 0xdf, 0x0e, 0xec, 0xd2, 0x8b, 0x36, 0xe3, 0x7a, 0xa3, 0xd7, 0x75, 0x88, 0x0c, 0x50, - 0x7e, 0x10, 0x79, 0x97, 0x2c, 0x31, 0x89, 0xe2, 0x07, 0x28, 0x90, 0x5e, 0xcf, 0xa7, 0x2a, 0x39, - 0xb5, 0x0a, 0xd5, 0xef, 0x2f, 0x0d, 0x74, 0x2c, 0x9d, 0x74, 0x65, 0xf5, 0x9b, 0x90, 0xaa, 0x6e, - 0x39, 0xbd, 0xa2, 0x77, 0x7e, 0x05, 0x00, 0x00, 0xff, 0xff, 0xa1, 0x90, 0xb0, 0xb9, 0x62, 0x06, - 0x00, 0x00, + // 668 bytes of a gzipped FileDescriptorProto + 0x1f, 0x8b, 0x08, 0x00, 0x00, 0x00, 0x00, 0x00, 0x02, 0xff, 0xac, 0x54, 0x41, 0x4f, 0x13, 0x4f, + 0x14, 0xef, 0xfe, 0x29, 0xe5, 0xcf, 0x80, 0xa0, 0x1b, 0x22, 0x65, 0xa3, 0x05, 0xab, 0x07, 0x24, + 0x61, 0xc6, 0x56, 0x8d, 0xa6, 0xc6, 0x83, 0x80, 0x87, 0xc6, 0x34, 0x36, 0x2b, 0x26, 0xc4, 0x4b, + 0x33, 0x9d, 0x9d, 0x2c, 0x23, 0xdd, 0x99, 0x75, 0x66, 0xba, 0xe2, 0xcd, 0x78, 0x32, 0x1e, 0x8c, + 0x07, 0x3f, 0x00, 0x1f, 0x81, 0x8b, 0x9f, 0x41, 0x8e, 0x1c, 0x3d, 0x19, 0x03, 0x07, 0x6e, 0x7e, + 0x06, 0xb3, 0x3b, 0xdb, 0x2d, 0x0a, 0x12, 0x2c, 0xdc, 0xf6, 0xbd, 0x99, 0xf7, 0x7b, 0xbf, 0xdf, + 0x6f, 0xde, 0x3e, 0xf0, 0x80, 0xb5, 0x09, 0xc2, 0x61, 0xd8, 0x61, 0x04, 0x6b, 0x26, 0xb8, 0x42, + 0x8c, 0x6b, 0x2a, 0xc9, 0x3a, 0x66, 0xbc, 0x85, 0x09, 0x11, 0x5d, 0xae, 0x15, 0x22, 0x82, 0x6b, + 0x29, 0x3a, 0x1d, 0x2a, 0x51, 0x54, 0x41, 0x7a, 0x13, 0x86, 0x52, 0x68, 0x61, 0x57, 0x59, 0x9b, + 0xc0, 0xc3, 0xc5, 0xf0, 0x98, 0x62, 0xd8, 0x2f, 0x86, 0x51, 0xc5, 0x99, 0xf2, 0x85, 0x2f, 0x92, + 0x72, 0x14, 0x7f, 0x19, 0x24, 0xe7, 0xce, 0xa9, 0x68, 0x44, 0x15, 0x14, 0x62, 0xb2, 0x41, 0x75, + 0x5a, 0xb5, 0x3c, 0x00, 0xf9, 0x43, 0x6c, 0x0c, 0xc8, 0x34, 0x11, 0x2a, 0x10, 0x0a, 0x05, 0xca, + 0x8f, 0xcf, 0x03, 0xe5, 0xa7, 0x07, 0xd7, 0x62, 0x74, 0x22, 0x24, 0x45, 0x64, 0x1d, 0x73, 0x4e, + 0x3b, 0x49, 0xb9, 0xf9, 0x34, 0x57, 0xca, 0x5f, 0x2c, 0x70, 0xa5, 0xa1, 0x7c, 0x97, 0xfa, 0x4c, + 0x69, 0x2a, 0xeb, 0x59, 0xf7, 0x47, 0xa6, 0xb9, 0x3d, 0x05, 0x86, 0xc5, 0x6b, 0x4e, 0x65, 0xd1, + 0x9a, 0xb3, 0xe6, 0x47, 0x5d, 0x13, 0xd8, 0xd7, 0xc1, 0x05, 0x22, 0x38, 0xa7, 0x24, 0x26, 0xdd, + 0x62, 0x5e, 0xf1, 0xbf, 0xe4, 0x74, 0xbc, 0x9f, 0xac, 0x7b, 0x76, 0x11, 0x8c, 0x44, 0x54, 0x2a, + 0x26, 0x78, 0x71, 0x28, 0x39, 0xee, 0x85, 0xf6, 0x2d, 0x30, 0x2c, 0xa4, 0x47, 0x65, 0x31, 0x3f, + 0x67, 0xcd, 0x4f, 0x54, 0x1d, 0x18, 0x3f, 0x43, 0x4c, 0x14, 0xf6, 0xd8, 0x45, 0x15, 0xf8, 0x34, + 0xbe, 0xe1, 0x9a, 0x8b, 0xb5, 0x89, 0xf7, 0x5b, 0xb3, 0xb9, 0x77, 0x07, 0xdb, 0x0b, 0x86, 0x40, + 0xd9, 0x03, 0x37, 0x4e, 0xa2, 0xed, 0x52, 0x15, 0x0a, 0xae, 0xa8, 0x7d, 0x15, 0x80, 0x14, 0x32, + 0x66, 0x69, 0x34, 0x8c, 0xa6, 0x99, 0xba, 0x67, 0x4f, 0x83, 0x91, 0x50, 0x48, 0xdd, 0x57, 0x50, + 0x88, 0xc3, 0xba, 0x57, 0xcb, 0xc7, 0xfd, 0xca, 0x3f, 0x2d, 0x30, 0xda, 0x50, 0xfe, 0x33, 0xca, + 0xbd, 0xd5, 0xcd, 0xb3, 0x58, 0xb1, 0x01, 0xc6, 0xcc, 0xbb, 0xb7, 0x3c, 0xac, 0x71, 0x62, 0xc7, + 0x58, 0x75, 0x05, 0x9e, 0x6a, 0xfa, 0xa2, 0x0a, 0x3c, 0xa2, 0xaf, 0x99, 0x80, 0xad, 0x60, 0x8d, + 0x97, 0xf2, 0x3b, 0xdf, 0x67, 0x73, 0x2e, 0x08, 0xb3, 0x8c, 0x7d, 0x13, 0x5c, 0x94, 0xb4, 0x83, + 0x35, 0x8b, 0x68, 0x4b, 0xb3, 0x80, 0x8a, 0xae, 0x4e, 0x8c, 0xce, 0xbb, 0x93, 0xbd, 0xfc, 0xaa, + 0x49, 0x1f, 0xb1, 0xf5, 0x2e, 0xb8, 0x94, 0xe9, 0xcd, 0x3c, 0x74, 0xc0, 0xff, 0x8a, 0xbe, 0xea, + 0x52, 0x4e, 0x68, 0x22, 0x3d, 0xef, 0x66, 0x71, 0xea, 0xd3, 0x67, 0x0b, 0x4c, 0x36, 0x94, 0xff, + 0x3c, 0xf4, 0xb0, 0xa6, 0x4d, 0x2c, 0x71, 0xa0, 0xec, 0xcb, 0xa0, 0xa0, 0x98, 0xdf, 0xb7, 0x2b, + 0x8d, 0xec, 0x35, 0x50, 0x08, 0x93, 0x1b, 0x89, 0x51, 0x63, 0xd5, 0x1a, 0xfc, 0xf7, 0x7f, 0x10, + 0x9a, 0x1e, 0xa9, 0xf6, 0x14, 0xaf, 0x36, 0xd9, 0x13, 0x93, 0xb6, 0x2a, 0xcf, 0x80, 0xe9, 0x3f, + 0x58, 0xf5, 0x34, 0x55, 0x3f, 0xe4, 0xc1, 0x50, 0x43, 0xf9, 0xf6, 0x57, 0x0b, 0xcc, 0xfc, 0x7d, + 0xf8, 0x9b, 0x83, 0x70, 0x3b, 0x69, 0x2e, 0x9d, 0xb5, 0xf3, 0x46, 0xcc, 0x5e, 0xe9, 0xa3, 0x05, + 0x0a, 0xe9, 0xa0, 0x3e, 0x1c, 0xb0, 0x89, 0x29, 0x77, 0x1e, 0x9f, 0xa9, 0x3c, 0x23, 0xb4, 0x65, + 0x81, 0xf1, 0xdf, 0x26, 0x62, 0x79, 0x40, 0xdc, 0xc3, 0x20, 0xce, 0x93, 0x73, 0x00, 0xe9, 0x51, + 0x74, 0x86, 0xdf, 0x1e, 0x6c, 0x2f, 0x58, 0x4b, 0x2f, 0x77, 0xf6, 0x4a, 0xd6, 0xee, 0x5e, 0xc9, + 0xfa, 0xb1, 0x57, 0xb2, 0x3e, 0xed, 0x97, 0x72, 0xbb, 0xfb, 0xa5, 0xdc, 0xb7, 0xfd, 0x52, 0xee, + 0x45, 0xd3, 0x67, 0x7a, 0xbd, 0xdb, 0x86, 0x44, 0x04, 0x28, 0xdd, 0xb2, 0xac, 0x4d, 0x16, 0x7d, + 0x81, 0xa2, 0xfb, 0x28, 0x10, 0x5e, 0xb7, 0x43, 0x55, 0xbc, 0xbf, 0x15, 0xaa, 0xde, 0x5b, 0xec, + 0xf3, 0x58, 0x3c, 0x6e, 0x75, 0xeb, 0x37, 0x21, 0x55, 0xed, 0x42, 0xb2, 0x77, 0x6f, 0xff, 0x0a, + 0x00, 0x00, 0xff, 0xff, 0x5c, 0x9c, 0xca, 0xe1, 0xb7, 0x06, 0x00, 0x00, } // Reference imports to suppress errors if they are not otherwise used. @@ -505,6 +509,11 @@ func (m *MsgRegisterInterchainAccount) MarshalToSizedBuffer(dAtA []byte) (int, e _ = i var l int _ = l + if m.Order != 0 { + i = encodeVarintTx(dAtA, i, uint64(m.Order)) + i-- + dAtA[i] = 0x20 + } if len(m.Version) > 0 { i -= len(m.Version) copy(dAtA[i:], m.Version) @@ -738,6 +747,9 @@ func (m *MsgRegisterInterchainAccount) Size() (n int) { if l > 0 { n += 1 + l + sovTx(uint64(l)) } + if m.Order != 0 { + n += 1 + sovTx(uint64(m.Order)) + } return n } @@ -947,6 +959,25 @@ func (m *MsgRegisterInterchainAccount) Unmarshal(dAtA []byte) error { } m.Version = string(dAtA[iNdEx:postIndex]) iNdEx = postIndex + case 4: + if wireType != 0 { + return fmt.Errorf("proto: wrong wireType = %d for field Order", wireType) + } + m.Order = 0 + for shift := uint(0); ; shift += 7 { + if shift >= 64 { + return ErrIntOverflowTx + } + if iNdEx >= l { + return io.ErrUnexpectedEOF + } + b := dAtA[iNdEx] + iNdEx++ + m.Order |= types.Order(b&0x7F) << shift + if b < 0x80 { + break + } + } default: iNdEx = preIndex skippy, err := skipTx(dAtA[iNdEx:]) diff --git a/modules/apps/27-interchain-accounts/host/ibc_module_test.go b/modules/apps/27-interchain-accounts/host/ibc_module_test.go index 827284ac310..5652805fe16 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -141,7 +141,12 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenTry() { expPass bool }{ { - "success", func() {}, true, + "success w/ ORDERED channel", func() {}, true, + }, + { + "success w/ UNORDERED channel", func() { + channel.Ordering = channeltypes.UNORDERED + }, true, }, { "account address generation is block dependent", func() { @@ -170,11 +175,6 @@ func (suite *InterchainAccountsTestSuite) TestOnChanOpenTry() { } }, true, }, - { - "ICA callback fails - invalid channel order", func() { - channel.Ordering = channeltypes.UNORDERED - }, false, - }, } for _, tc := range testCases { diff --git a/modules/apps/27-interchain-accounts/host/keeper/handshake.go b/modules/apps/27-interchain-accounts/host/keeper/handshake.go index b38f779efbf..46e9e91fa8c 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/handshake.go +++ b/modules/apps/27-interchain-accounts/host/keeper/handshake.go @@ -30,10 +30,6 @@ func (k Keeper) OnChanOpenTry( counterparty channeltypes.Counterparty, counterpartyVersion string, ) (string, error) { - if order != channeltypes.ORDERED { - return "", errorsmod.Wrapf(channeltypes.ErrInvalidChannelOrdering, "expected %s channel, got %s", channeltypes.ORDERED, order) - } - if portID != icatypes.HostPortID { return "", errorsmod.Wrapf(icatypes.ErrInvalidHostPort, "expected %s, got %s", icatypes.HostPortID, portID) } @@ -54,18 +50,13 @@ func (k Keeper) OnChanOpenTry( 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) - } - - appVersion, found := k.GetAppVersion(ctx, portID, activeChannelID) - if !found { - panic(fmt.Errorf("active channel mapping set for %s, but channel does not exist in channel store", activeChannelID)) + 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 !icatypes.IsPreviousMetadataEqual(appVersion, metadata) { - return "", errorsmod.Wrap(icatypes.ErrInvalidVersion, "previous active channel metadata does not match provided version") - } + // if a channel is being reopened, we allow the controller to propose new fields + // which are not exactly the same as the previous. The provided address will + // be overwritten with the correct one before the metadata is returned. } // On the host chain the capability may only be claimed during the OnChanOpenTry @@ -139,19 +130,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) OnChanUpgradeTry(ctx sdk.Context, portID, channelID string, proposedOrder channeltypes.Order, proposedConnectionHops []string, counterpartyVersion 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) - } - if portID != icatypes.HostPortID { return "", errorsmod.Wrapf(porttypes.ErrInvalidPort, "expected %s, got %s", icatypes.HostPortID, portID) } 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 bb69322d40a..60b9e50a38d 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/handshake_test.go @@ -95,6 +95,25 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() { suite.Require().False(found) }, true, }, + { + "success - previous metadata is different", + func() { + // set the active channelID in state + suite.chainB.GetSimApp().ICAHostKeeper.SetActiveChannelID(suite.chainB.GetContext(), path.EndpointB.ConnectionID, path.EndpointA.ChannelConfig.PortID, path.EndpointB.ChannelID) + + // set the previous encoding to be proto3json. + // the new encoding is set to be protobuf in the test below. + metadata.Encoding = icatypes.EncodingProto3JSON + + versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) + suite.Require().NoError(err) + + channel.State = channeltypes.CLOSED + channel.Version = string(versionBytes) + + path.EndpointB.SetChannel(*channel) + }, true, + }, { "reopening account fails - no existing account", func() { @@ -148,35 +167,6 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() { }, false, }, - { - "invalid metadata - previous metadata is different", - func() { - // create a new channel and set it in state - ch := channeltypes.NewChannel(channeltypes.CLOSED, channeltypes.ORDERED, channeltypes.NewCounterparty(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID), []string{path.EndpointA.ConnectionID}, TestVersion) - suite.chainB.GetSimApp().GetIBCKeeper().ChannelKeeper.SetChannel(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, ch) - - // set the active channelID in state - suite.chainB.GetSimApp().ICAHostKeeper.SetActiveChannelID(suite.chainB.GetContext(), path.EndpointB.ConnectionID, path.EndpointA.ChannelConfig.PortID, path.EndpointB.ChannelID) - - // attempt to downgrade version by reinitializing channel with version 1, but setting channel to version 2 - metadata.Version = "ics27-2" - - versionBytes, err := icatypes.ModuleCdc.MarshalJSON(&metadata) - suite.Require().NoError(err) - - channel.Version = string(versionBytes) - - path.EndpointB.SetChannel(*channel) - }, false, - }, - - { - "invalid order - UNORDERED", - func() { - channel.Ordering = channeltypes.UNORDERED - }, - false, - }, { "invalid port ID", func() { @@ -269,7 +259,7 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() { false, }, { - "active channel already set", + "active channel already set (OPEN state)", func() { // create a new channel and set it in state ch := channeltypes.NewChannel(channeltypes.OPEN, channeltypes.ORDERED, channeltypes.NewCounterparty(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID), []string{path.EndpointA.ConnectionID}, ibctesting.DefaultChannelVersion) @@ -279,6 +269,23 @@ func (suite *KeeperTestSuite) TestOnChanOpenTry() { suite.chainB.GetSimApp().ICAHostKeeper.SetActiveChannelID(suite.chainB.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, path.EndpointB.ChannelID) }, false, }, + { + "channel is already active (FLUSHING state)", + func() { + suite.chainB.GetSimApp().ICAHostKeeper.SetActiveChannelID(suite.chainB.GetContext(), ibctesting.FirstConnectionID, path.EndpointA.ChannelConfig.PortID, path.EndpointB.ChannelID) + + counterparty := channeltypes.NewCounterparty(path.EndpointA.ChannelConfig.PortID, path.EndpointA.ChannelID) + channel := channeltypes.Channel{ + State: channeltypes.FLUSHING, + Ordering: channeltypes.ORDERED, + Counterparty: counterparty, + ConnectionHops: []string{path.EndpointB.ConnectionID}, + Version: TestVersion, + } + suite.chainB.GetSimApp().IBCKeeper.ChannelKeeper.SetChannel(suite.chainB.GetContext(), path.EndpointB.ChannelConfig.PortID, path.EndpointB.ChannelID, channel) + }, + false, + }, } for _, tc := range testCases { @@ -458,18 +465,18 @@ func (suite *KeeperTestSuite) TestOnChanUpgradeTry() { nil, }, { - name: "failure: invalid port ID", + name: "success: change order", malleate: func() { - path.EndpointB.ChannelConfig.PortID = "invalid-port-id" + order = channeltypes.UNORDERED }, - expError: porttypes.ErrInvalidPort, + expError: nil, }, { - name: "failure: invalid order", + name: "failure: invalid port ID", malleate: func() { - order = channeltypes.UNORDERED + path.EndpointB.ChannelConfig.PortID = "invalid-port-id" }, - expError: channeltypes.ErrInvalidChannelOrdering, + expError: porttypes.ErrInvalidPort, }, { name: "failure: invalid proposed connectionHops", diff --git a/modules/apps/callbacks/callbacks_test.go b/modules/apps/callbacks/callbacks_test.go index 7f5d5185aa8..c1329417de9 100644 --- a/modules/apps/callbacks/callbacks_test.go +++ b/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.NewMsgRegisterInterchainAccount(s.path.EndpointA.ConnectionID, owner, s.path.EndpointA.ChannelConfig.Version, channeltypes.ORDERED) res, err := s.chainA.SendMsgs(msgRegister) s.Require().NotEmpty(res) diff --git a/proto/ibc/applications/interchain_accounts/controller/v1/tx.proto b/proto/ibc/applications/interchain_accounts/controller/v1/tx.proto index 1287cfa2d37..db0c7b77e39 100644 --- a/proto/ibc/applications/interchain_accounts/controller/v1/tx.proto +++ b/proto/ibc/applications/interchain_accounts/controller/v1/tx.proto @@ -8,6 +8,7 @@ import "gogoproto/gogo.proto"; import "ibc/applications/interchain_accounts/v1/packet.proto"; import "ibc/applications/interchain_accounts/controller/v1/controller.proto"; import "cosmos/msg/v1/msg.proto"; +import "ibc/core/channel/v1/channel.proto"; // Msg defines the 27-interchain-accounts/controller Msg service. service Msg { @@ -27,9 +28,10 @@ message MsgRegisterInterchainAccount { option (gogoproto.goproto_getters) = false; - string owner = 1; - string connection_id = 2; - string version = 3; + string owner = 1; + string connection_id = 2; + string version = 3; + ibc.core.channel.v1.Order order = 4; } // MsgRegisterInterchainAccountResponse defines the response for Msg/RegisterAccount