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

fix: updating timeout to use clienttypes.Height instead of uint64 #483

Merged
merged 4 commits into from
Mar 30, 2021

Conversation

seantking
Copy link
Contributor

@seantking seantking commented Mar 29, 2021

This PR updates the building and unpacking of ibc packets to use the same clienttypes.Height type rather than referencing only the RevisionHeight which is a uint64.

Presently, for IBC application developers it is not possible to specify that timeout height should not be used as we always add version when rebuilding the packet before relaying ->

clienttypes.NewHeight(version, packet.timeout),

This PR allows for an IBC application to pass clienttypes.ZeroHeight as timeoutHeight when building an IBC packet. The relayer will then ignore timeoutHeight as intended.

@colin-axner short term solution for #481 ?

Copy link
Contributor

@colin-axner colin-axner left a comment

Choose a reason for hiding this comment

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

ACK - nice fix!!

I'm going to commit my suggestions so hopefully they don't break compilation or the linter

relayer/ibc-client.go Outdated Show resolved Hide resolved
relayer/naive-strategy.go Outdated Show resolved Hide resolved
@colin-axner
Copy link
Contributor

this closes #481 . It is the solution I was hoping for

@colin-axner colin-axner merged commit 13245f1 into cosmos:master Mar 30, 2021
@colin-axner colin-axner added the backport Backport to the latest long-living release branch label Mar 30, 2021
colin-axner added a commit that referenced this pull request Apr 14, 2021
* fix: updating timeout to use clienttypes.Height instead of uint64

* Update relayer/ibc-client.go

* Update relayer/naive-strategy.go

* Update relayer/naive-strategy.go

Co-authored-by: colin axnér <25233464+colin-axner@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport Backport to the latest long-living release branch
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants