-
Notifications
You must be signed in to change notification settings - Fork 0
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
update the dns module to the version of cosmos-sdk v0.40.0-rc3 #7
Conversation
2c8e2d2
to
50c4a2f
Compare
c55e129
to
06a66f5
Compare
3d061b2
to
a355bce
Compare
45fcee7
to
c5d4bc9
Compare
c5d4bc9
to
7dceece
Compare
func (p RegisterDomainPacketData) GetTimeoutHeight() uint64 { | ||
return math.MaxUint64 | ||
func (p RegisterDomainPacketData) GetTimeoutHeight() ibcclienttypes.Height { | ||
return ibcclienttypes.NewHeight(0, math.MaxInt64) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If MaxUint64 is specified, Overflow may occur with the following Code
https://github.com/cosmos/cosmos-sdk/blob/master/x/ibc/core/02-client/types/height.go#L61
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've fixed it at this commit.
After a new version that includes this commit is released, we should revert the changes.
356be64
to
441d089
Compare
441d089
to
ca3b663
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@toshihiko-okubo
The implementation looks good!
I have two suggestions:
- Can you move
x/ibc-dns/testing
directory to underx/ibc
? This is because we want to separate ibc testing module from dns package. - And sorry, can you use the SDK version v0.40.3-rc-3?
No description provided.