diff --git a/CHANGELOG.md b/CHANGELOG.md index c14dcab09a5..f414e1cd8b0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -54,7 +54,8 @@ Ref: https://keepachangelog.com/en/1.0.0/ * (core) [\#6138](https://github.com/cosmos/ibc-go/pull/6138) Remove `Router` reference from IBC core keeper and use instead the router on the existing `PortKeeper` reference. * (core/04-channel) [\#6023](https://github.com/cosmos/ibc-go/pull/6023) Remove emission of non-hexlified event attributes `packet_data` and `packet_ack`. * (core) [\#6320](https://github.com/cosmos/ibc-go/pull/6320) Remove unnecessary `Proof` interface from `exported` package. -* (core/05-port) [\#6341](https://github.com/cosmos/ibc-go/pull/6341) Modify `UnmarshalPacketData` interface to take in the context, portID, and channelID. This allows for packet data's to be unmarshaled based on the channel version. +* (core/05-port) [\#6341](https://github.com/cosmos/ibc-go/pull/6341) Modify `UnmarshalPacketData` interface to take in the context, portID, and channelID. This allows for packet data's to be unmarshaled based on the channel version. +* (apps/27-interchain-accounts) [\#6433](https://github.com/cosmos/ibc-go/pull/6433) Use UNORDERED as the default ordering for new ICA channels. ### State Machine Breaking 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 09ae7063a68..8d1a2321e57 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 @@ -11,7 +11,7 @@ The Interchain Accounts module uses either [ORDERED or UNORDERED](https://github When using `ORDERED` channels, the order of transactions when sending packets from a controller to a host chain is maintained. -When using `UNORDERED` channels, there is no guarantee that the order of transactions when sending packets from the controller to the host chain is maintained. +When using `UNORDERED` channels, there is no guarantee that the order of transactions when sending packets from the controller to the host chain is maintained.If no ordering is specified in `MsgRegisterInterchainAccount`, then the default ordering for new ICA channels is `UNORDERED`. > A limitation when using ORDERED channels is that when a packet times out the channel will be closed. diff --git a/docs/docs/02-apps/02-interchain-accounts/10-legacy/03-keeper-api.md b/docs/docs/02-apps/02-interchain-accounts/10-legacy/03-keeper-api.md index 252c7eaf7ff..127574f7069 100644 --- a/docs/docs/02-apps/02-interchain-accounts/10-legacy/03-keeper-api.md +++ b/docs/docs/02-apps/02-interchain-accounts/10-legacy/03-keeper-api.md @@ -19,7 +19,7 @@ The controller submodule keeper exposes two legacy functions that allow respecti The authentication module can begin registering interchain accounts by calling `RegisterInterchainAccount`: ```go -if err := keeper.icaControllerKeeper.RegisterInterchainAccount(ctx, connectionID, owner.String(), version); err != nil { +if err := keeper.icaControllerKeeper.RegisterInterchainAccount(ctx, connectionID, owner.String(), version, channeltypes.UNORDERED); err != nil { return err } @@ -44,7 +44,7 @@ if err != nil { return err } -if err := keeper.icaControllerKeeper.RegisterInterchainAccount(ctx, controllerConnectionID, owner.String(), string(appVersion)); err != nil { +if err := keeper.icaControllerKeeper.RegisterInterchainAccount(ctx, controllerConnectionID, owner.String(), string(appVersion), channeltypes.UNORDERED); err != nil { return err } ``` @@ -75,7 +75,7 @@ if err != nil { return err } -if err := keeper.icaControllerKeeper.RegisterInterchainAccount(ctx, controllerConnectionID, owner.String(), string(feeEnabledVersion)); err != nil { +if err := keeper.icaControllerKeeper.RegisterInterchainAccount(ctx, controllerConnectionID, owner.String(), string(feeEnabledVersion), channeltypes.UNORDERED); err != nil { return err } ``` diff --git a/docs/docs/05-migrations/13-v8-to-v9.md b/docs/docs/05-migrations/13-v8-to-v9.md index bd67a09515d..1d3d165a133 100644 --- a/docs/docs/05-migrations/13-v8-to-v9.md +++ b/docs/docs/05-migrations/13-v8-to-v9.md @@ -84,6 +84,17 @@ func NewKeeper( ) Keeper ``` +The legacy function `RegisterInterchainAccount` takes now an extra parameter to specify the ordering of new ICA channels: + +```diff +func (k Keeper) RegisterInterchainAccount( + ctx sdk.Context, + connectionID, owner, + version string, ++ ordering channeltypes.Order +) error { +``` + ## Relayers - Renaming of event attribute keys in [#5603](https://github.com/cosmos/ibc-go/pull/5603). 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 128e990d52d..6b938eaf10b 100644 --- a/modules/apps/27-interchain-accounts/controller/client/cli/tx.go +++ b/modules/apps/27-interchain-accounts/controller/client/cli/tx.go @@ -71,7 +71,7 @@ the associated capability.`), } cmd.Flags().String(flagVersion, "", "Controller chain channel version") - cmd.Flags().String(flagOrdering, channeltypes.ORDERED.String(), fmt.Sprintf("Channel ordering, can be one of: %s", strings.Join(connectiontypes.SupportedOrderings, ", "))) + cmd.Flags().String(flagOrdering, channeltypes.UNORDERED.String(), fmt.Sprintf("Channel ordering, can be one of: %s", strings.Join(connectiontypes.SupportedOrderings, ", "))) flags.AddTxFlagsToCmd(cmd) return cmd 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 e9ab95cc1b9..e2af8caf3c8 100644 --- a/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go +++ b/modules/apps/27-interchain-accounts/controller/ibc_middleware_test.go @@ -78,7 +78,7 @@ func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) erro channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext()) - if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, TestVersion); err != nil { + if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, TestVersion, channeltypes.ORDERED); err != nil { return err } diff --git a/modules/apps/27-interchain-accounts/controller/keeper/account.go b/modules/apps/27-interchain-accounts/controller/keeper/account.go index a494bb29a50..cb08bb44e19 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/account.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/account.go @@ -29,7 +29,7 @@ import ( // Prior to v6.x.x of ibc-go, the controller module was only functional as middleware, with authentication performed // by the underlying application. For a full summary of the changes in v6.x.x, please see ADR009. // This API will be removed in later releases. -func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, connectionID, owner, version string) error { +func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, connectionID, owner, version string, ordering channeltypes.Order) error { portID, err := icatypes.NewControllerPortID(owner) if err != nil { return err @@ -41,7 +41,12 @@ func (k Keeper) RegisterInterchainAccount(ctx sdk.Context, connectionID, owner, k.SetMiddlewareEnabled(ctx, portID, connectionID) - _, err = k.registerInterchainAccount(ctx, connectionID, portID, version, channeltypes.ORDERED) + // use ORDER_UNORDERED as default in case ordering is NONE + if ordering == channeltypes.NONE { + ordering = channeltypes.UNORDERED + } + + _, err = k.registerInterchainAccount(ctx, connectionID, portID, version, ordering) if err != nil { return err } diff --git a/modules/apps/27-interchain-accounts/controller/keeper/account_test.go b/modules/apps/27-interchain-accounts/controller/keeper/account_test.go index c462de350f6..cc4a28062f0 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/account_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/account_test.go @@ -72,7 +72,7 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount() { tc.malleate() // malleate mutates test data - err = suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), path.EndpointA.ConnectionID, owner, TestVersion) + err = suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), path.EndpointA.ConnectionID, owner, TestVersion, channeltypes.ORDERED) if tc.expPass { suite.Require().NoError(err) @@ -103,7 +103,7 @@ func (suite *KeeperTestSuite) TestRegisterSameOwnerMultipleConnections() { TxType: icatypes.TxTypeSDKMultiMsg, } - err := suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), pathAToB.EndpointA.ConnectionID, owner, string(icatypes.ModuleCdc.MustMarshalJSON(metadata))) + err := suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), pathAToB.EndpointA.ConnectionID, owner, string(icatypes.ModuleCdc.MustMarshalJSON(metadata)), channeltypes.ORDERED) suite.Require().NoError(err) // build ICS27 metadata with connection identifiers for path A->C @@ -115,6 +115,6 @@ func (suite *KeeperTestSuite) TestRegisterSameOwnerMultipleConnections() { TxType: icatypes.TxTypeSDKMultiMsg, } - err = suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), pathAToC.EndpointA.ConnectionID, owner, string(icatypes.ModuleCdc.MustMarshalJSON(metadata))) + err = suite.chainA.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(suite.chainA.GetContext(), pathAToC.EndpointA.ConnectionID, owner, string(icatypes.ModuleCdc.MustMarshalJSON(metadata)), channeltypes.ORDERED) suite.Require().NoError(err) } diff --git a/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go b/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go index b733adff9ca..00af5616148 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/keeper_test.go @@ -90,7 +90,7 @@ func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) erro channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext()) - if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, TestVersion); err != nil { + if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, TestVersion, channeltypes.ORDERED); err != nil { return err } 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 66ad2afd0a9..27434730132 100644 --- a/modules/apps/27-interchain-accounts/controller/keeper/msg_server.go +++ b/modules/apps/27-interchain-accounts/controller/keeper/msg_server.go @@ -9,6 +9,7 @@ import ( "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/types" icatypes "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/types" + channeltypes "github.com/cosmos/ibc-go/v8/modules/core/04-channel/types" ibcerrors "github.com/cosmos/ibc-go/v8/modules/core/errors" ) @@ -39,7 +40,15 @@ func (s msgServer) RegisterInterchainAccount(goCtx context.Context, msg *types.M s.SetMiddlewareDisabled(ctx, portID, msg.ConnectionId) - channelID, err := s.registerInterchainAccount(ctx, msg.ConnectionId, portID, msg.Version, msg.Ordering) + // use ORDER_UNORDERED as default in case msg's ordering is NONE + var order channeltypes.Order + if msg.Ordering == channeltypes.NONE { + order = channeltypes.UNORDERED + } else { + order = msg.Ordering + } + + channelID, err := s.registerInterchainAccount(ctx, msg.ConnectionId, portID, msg.Version, order) if err != nil { s.Logger(ctx).Error("error registering interchain account", "error", err.Error()) return nil, err 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 625c0441859..71899dacf53 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 @@ -21,6 +21,7 @@ import ( func (suite *KeeperTestSuite) TestRegisterInterchainAccount_MsgServer() { var ( msg *types.MsgRegisterInterchainAccount + expectedOrderding channeltypes.Order expectedChannelID = "channel-0" ) @@ -34,6 +35,14 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount_MsgServer() { true, func() {}, }, + { + "success: ordering falls back to UNORDERED if not specified", + true, + func() { + msg.Ordering = channeltypes.NONE + expectedOrderding = channeltypes.UNORDERED + }, + }, { "invalid connection id", false, @@ -70,6 +79,8 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount_MsgServer() { tc := tc suite.Run(tc.name, func() { + expectedOrderding = channeltypes.ORDERED + suite.SetupTest() path := NewICAPath(suite.chainA, suite.chainB) @@ -92,6 +103,11 @@ func (suite *KeeperTestSuite) TestRegisterInterchainAccount_MsgServer() { suite.Require().Len(events, 2) suite.Require().Equal(events[0].Type, channeltypes.EventTypeChannelOpenInit) suite.Require().Equal(events[1].Type, sdk.EventTypeMessage) + + path.EndpointA.ChannelConfig.PortID = res.PortId + path.EndpointA.ChannelID = res.ChannelId + channel := path.EndpointA.GetChannel() + suite.Require().Equal(expectedOrderding, channel.Ordering) } else { suite.Require().Error(err) suite.Require().Nil(res) diff --git a/modules/apps/27-interchain-accounts/controller/migrations/v6/migrations_test.go b/modules/apps/27-interchain-accounts/controller/migrations/v6/migrations_test.go index 3c88c512d3d..1e9fcde0f81 100644 --- a/modules/apps/27-interchain-accounts/controller/migrations/v6/migrations_test.go +++ b/modules/apps/27-interchain-accounts/controller/migrations/v6/migrations_test.go @@ -8,7 +8,7 @@ import ( sdk "github.com/cosmos/cosmos-sdk/types" capabilitytypes "github.com/cosmos/ibc-go/modules/capability/types" - "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/migrations/v6" + v6 "github.com/cosmos/ibc-go/v8/modules/apps/27-interchain-accounts/controller/migrations/v6" "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" @@ -68,7 +68,7 @@ func (*MigrationsTestSuite) RegisterInterchainAccount(endpoint *ibctesting.Endpo channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext()) - if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version); err != nil { + if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version, channeltypes.ORDERED); err != nil { return err } 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 9ffb67a8f4d..7279f680438 100644 --- a/modules/apps/27-interchain-accounts/host/ibc_module_test.go +++ b/modules/apps/27-interchain-accounts/host/ibc_module_test.go @@ -83,7 +83,7 @@ func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) erro channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext()) - if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version); err != nil { + if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version, channeltypes.ORDERED); err != nil { return err } diff --git a/modules/apps/27-interchain-accounts/host/keeper/keeper_test.go b/modules/apps/27-interchain-accounts/host/keeper/keeper_test.go index acbc274ef2f..cde36a00966 100644 --- a/modules/apps/27-interchain-accounts/host/keeper/keeper_test.go +++ b/modules/apps/27-interchain-accounts/host/keeper/keeper_test.go @@ -112,7 +112,7 @@ func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) erro channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext()) - if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version); err != nil { + if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version, channeltypes.ORDERED); err != nil { return err } diff --git a/modules/apps/29-fee/ica_test.go b/modules/apps/29-fee/ica_test.go index ed121ae6e44..b7c43048593 100644 --- a/modules/apps/29-fee/ica_test.go +++ b/modules/apps/29-fee/ica_test.go @@ -75,7 +75,7 @@ func RegisterInterchainAccount(endpoint *ibctesting.Endpoint, owner string) erro channelSequence := endpoint.Chain.App.GetIBCKeeper().ChannelKeeper.GetNextChannelSequence(endpoint.Chain.GetContext()) - if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version); err != nil { + if err := endpoint.Chain.GetSimApp().ICAControllerKeeper.RegisterInterchainAccount(endpoint.Chain.GetContext(), endpoint.ConnectionID, owner, endpoint.ChannelConfig.Version, channeltypes.ORDERED); err != nil { return err }