Skip to content

Commit

Permalink
imp: use UNORDERED as default ordering for new ICA channels (#6434)
Browse files Browse the repository at this point in the history
* imp: use UNORDERED as default ordering for new ICA channels

* Update migrations_test.go

* Update 09-active-channels.md

* update docs

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

Co-authored-by: Damian Nolan <damiannolan@gmail.com>

* test both ORDERED and UNORDERED channels in most (probably all) tests involving something with ICA

* remove unnecessary parameter

* gofumpt

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

Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>

---------

Co-authored-by: Damian Nolan <damiannolan@gmail.com>
Co-authored-by: DimitrisJim <d.f.hilliard@gmail.com>
  • Loading branch information
3 people authored Jun 5, 2024
1 parent e2327c8 commit 057466e
Show file tree
Hide file tree
Showing 24 changed files with 2,716 additions and 2,523 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -55,6 +55,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
* (core/04-channel) [\#6023](https://github.com/cosmos/ibc-go/pull/6023) Remove emission of non-hexlified event attributes `packet_data` and `packet_ack`.
* (core) [\#6320](https://github.com/cosmos/ibc-go/pull/6320) Remove unnecessary `Proof` interface from `exported` package.
* (core/05-port) [\#6341](https://github.com/cosmos/ibc-go/pull/6341) Modify `UnmarshalPacketData` interface to take in the context, portID, and channelID. This allows for packet data's to be unmarshaled based on the channel version.
* (apps/27-interchain-accounts) [\#6433](https://github.com/cosmos/ibc-go/pull/6433) Use UNORDERED as the default ordering for new ICA channels.
* (apps/transfer) [\#6440](https://github.com/cosmos/ibc-go/pull/6440) Remove `GetPrefixedDenom`.

### State Machine Breaking
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,14 +19,14 @@ The controller submodule keeper exposes two legacy functions that allow respecti
The authentication module can begin registering interchain accounts by calling `RegisterInterchainAccount`:

```go
if err := keeper.icaControllerKeeper.RegisterInterchainAccount(ctx, connectionID, owner.String(), version); err != nil {
if err := keeper.icaControllerKeeper.RegisterInterchainAccount(ctx, connectionID, owner.String(), version, channeltypes.UNORDERED); err != nil {
return err
}

return nil
```

The `version` argument is used to support ICS-29 fee middleware for relayer incentivization of ICS-27 packets. Consumers of the `RegisterInterchainAccount` are expected to build the appropriate JSON encoded version string themselves and pass it accordingly. If an empty string is passed in the `version` argument, then the version will be initialized to a default value in the `OnChanOpenInit` callback of the controller's handler, so that channel handshake can proceed.
The `version` argument is used to support ICS-29 fee middleware for relayer incentivization of ICS-27 packets. The `ordering` argument allows to specify the ordering of the channel that is created; if `NONE` is passed, then the default ordering will be `UNORDERED`. Consumers of the `RegisterInterchainAccount` are expected to build the appropriate JSON encoded version string themselves and pass it accordingly. If an empty string is passed in the `version` argument, then the version will be initialized to a default value in the `OnChanOpenInit` callback of the controller's handler, so that channel handshake can proceed.

The following code snippet illustrates how to construct an appropriate interchain accounts `Metadata` and encode it as a JSON bytestring:

Expand All @@ -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
}
```
Expand Down Expand Up @@ -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
}
```
Expand Down
11 changes: 11 additions & 0 deletions docs/docs/05-migrations/13-v8-to-v9.md
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,17 @@ func NewKeeper(
) Keeper
```

The legacy function `RegisterInterchainAccount` now takes 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).
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading

0 comments on commit 057466e

Please sign in to comment.