Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
AmitPr opened this issue Jun 26, 2024 · 4 comments
Closed

Comments

@AmitPr
Copy link

AmitPr commented Jun 26, 2024

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:

func NewMsgRegisterInterchainAccount(connectionID, owner, version string) *MsgRegisterInterchainAccount {
return &MsgRegisterInterchainAccount{
ConnectionId: connectionID,
Owner: owner,
Version: version,
Ordering: channeltypes.UNORDERED,
}
}

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
Copy link
Author

AmitPr commented Jun 26, 2024

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

@crodriguezvega
Copy link
Contributor

crodriguezvega commented Jun 27, 2024

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
Copy link
Author

AmitPr commented Jun 27, 2024

@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.

@crodriguezvega
Copy link
Contributor

Hi @AmitPr. Sorry for the late reply. We discussed this internally and we agree that we should probably not have introduced this change in a minor release, but the reason why we did it is because reopening ordered channels that close due to timeouts was one of the biggest complaints about ICA, so we decided to address it as soon as possible. We realise that this change in the default behaviour might be a bit surprising for existing users of ICA, but using unordered channels as the default for new ICA channels provides a better experience. We take for the future the learning point that we shouldn't make behavioural changes in minor releases, but we will not make a new minor release now to change back the ordering, since it would potentially make things even more confusing for our users.

I will close the issue now, but thanks for raising this problem. Feel free still to leave a comment if you have any more thoughts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants