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

Properly handle timeouts and packet sequence numbers on ORDERED channels #663

Merged
merged 29 commits into from
May 24, 2022

Conversation

jtieri
Copy link
Member

@jtieri jtieri commented Apr 1, 2022

Currently we do not differentiate between ORDERED and UNORDERED channels when handling timeouts in the relayer.

ORDERED channels require a proof of NextSequenceRecv since the invariance on ordered channels says "if packet sequence n times out, then a packet at sequence k > n cannot be received" as per ibc-go docs.

UNORDERED channels require a proof of non-existence via the packet receipt.

This PR in it's final form will address the timeout issue as well as how packet sequences need to be handled on ORDERED channels but will be missing the logic for finishing the channel handshake process which is necessary for full interchain account support. Ideally we will be querying for recent txs and then looking for ChannelOpenInit in the events. If we noticed a ChannelOpenInit event then the relayer will attempt to finish the channel handshake.

Right now we are waiting for two upstream changes which emit the version information in ChannelOpenInit. This is needed to parse the full channel information from the events.

cosmos/ibc-go#1203
cosmos/ibc-go#1204

Closes #321

jtieri and others added 23 commits April 1, 2022 16:38
@jtieri
Copy link
Member Author

jtieri commented May 3, 2022

This PR has been tested using the interchain accounts demo here

To test with this branch checkout commit hash b92f1de and make install the relayer. Then clone down the aforementioned interchain accounts demo and run make init-golang-rly

You should see a interchain account successfully registered, a packet successfully relayed and then a successfully timed out packet.

Not an ideal way to test this but I had started adding these test cases to the IBC-test-framework and then fudged my branch so will have to reimplement this there.

@jtieri jtieri changed the title WIP: properly handle timeouts on ordered channels Properly handle timeouts on ORDERED channels May 3, 2022
@jtieri jtieri changed the title Properly handle timeouts on ORDERED channels Properly handle timeouts and packet sequence numbers on ORDERED channels May 3, 2022
@jtieri jtieri marked this pull request as ready for review May 3, 2022 19:33
@jtieri jtieri marked this pull request as draft May 4, 2022 05:19
Copy link
Member

@mark-rushakoff mark-rushakoff left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I skimmed through the change this morning and everything looks correct AFAICS.

Makefile Outdated Show resolved Hide resolved
relayer/provider/cosmos/provider.go Show resolved Hide resolved
relayer/strategies.go Outdated Show resolved Hide resolved
@jtieri jtieri marked this pull request as ready for review May 24, 2022 16:39
@jtieri
Copy link
Member Author

jtieri commented May 24, 2022

Addressed the few suggestions, this should be gtm.

@boojamya boojamya added this to the v2.0.0 Release ❇️ milestone May 24, 2022
relayer/naive-strategy.go Outdated Show resolved Hide resolved
relayer/provider/cosmos/provider.go Outdated Show resolved Hide resolved
relayer/provider/cosmos/provider.go Outdated Show resolved Hide resolved
relayer/provider/cosmos/provider.go Outdated Show resolved Hide resolved
@jtieri jtieri merged commit e0a2858 into main May 24, 2022
@jtieri jtieri deleted the justin/ordered-timeouts branch May 24, 2022 20:34
@jackzampolin
Copy link
Member

SHIPPING

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

Successfully merging this pull request may close these issues.

Timeouts on ORDERED Channels shouldn't work
4 participants