search

cosmos / ibc-go

Inter-Blockchain Communication Protocol (IBC) implementation in Golang.
https://ibc.cosmos.network/
MIT License
524 stars 565 forks source link

ibc-go v.7.5.0 (and v7.6.0) breaks ICS27 backwards compatibility by default #6714

Open AmitPr opened 6 days ago

AmitPr commented 6 days ago

Summary of Bug

7.5.0 introduces channel ordering options in the interchain accounts controller module. Older versions of ibc-go will fail to create the ICA channel if the ordering is not ORDER_ORDERED. However, the default ordering in NewMsgRegisterInterchainAccount is ORDER_UNORDERED, meaning that existing code that has been upgraded to ibc-go >=7.5.0 will no longer be compatible with older chains.

Relevant code: https://github.com/cosmos/ibc-go/blob/b605529d6ccd1170e3d4bdec9fb9b79fd22a2141/modules/apps/27-interchain-accounts/controller/types/msgs.go#L34-L41

Expected Behaviour

Channels are ORDER_ORDERED by default, to preserve compatibility with older ibc-go versions

Version

Observed on ibc-go 7.6.0 (Kujira)

Steps to Reproduce

N/A

AmitPr commented 6 days ago

Note: I haven't taken a look at ibc-go v8 and beyond, as we are not yet using it on Kujira.

crodriguezvega commented 6 days ago

Hi @AmitPr. This change was documented in the release notes of v7.5.0 and in the documentation (here and here). You can keep the previous behaviour by setting the ordering to UNORDERED in the MsgRegisterInterchainAccount, or if you are using the legacy API, you can also use the RegisterInterchainAccountWithOrdering function to explicitly set the ordering to UNORDERED. Does this help?

AmitPr commented 6 days ago

@crodriguezvega the issue is that the default method for creating MsgRegisterInterchainAccount creates an UNORDERED channel, which is not backwards compatible, since previous versions of ibc-go are only compatible with ORDERED.

A module we have creates ICAs, and this default behavior silently broke the functionality and backwards compatibility of this module because of the change in default behavior. I will also note that this breakage was implemented in a minor semver upgrade.