-
Notifications
You must be signed in to change notification settings - Fork 636
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
Add GRPc Query for Upgrade #3491
Add GRPc Query for Upgrade #3491
Conversation
…e-upgrade-type-and-validate-basic-functions
…e-basic-functions' into cian/issue#3451-add-verifiable-upgrade-type-and-validate-basic-functions
…ype' of https://github.com/cosmos/ibc-go into cian/issue#3447-modify-upgradeinit-to-use-new-upgrade-type
…e instead of just version
…ype' into cian/issue#3481-grpc-query-for-type.upgrade
@@ -11,6 +11,7 @@ import ( | |||
"github.com/cosmos/ibc-go/v7/modules/core/04-channel/types" | |||
"github.com/cosmos/ibc-go/v7/modules/core/exported" | |||
ibctesting "github.com/cosmos/ibc-go/v7/testing" | |||
"github.com/cosmos/ibc-go/v7/testing/mock" | |||
) | |||
|
|||
const doesnotexist = "doesnotexist" |
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.
this is unrelated to this PR i guess, but this const is a bit random here if we are just using string values in all the rest of the tests
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.
diff would be quite large if I changed all of these here
} | ||
}, | ||
false, | ||
}, |
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.
should we add one more test for querying an upgrade on a channel that hasn't initiated an upgrade yet?
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.
this test is actually just directly setting the upgrade in state, the upgrade isn't being set in the ChanUpgradeInit function
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.
actually we can still do this by just deleting the key 👍
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.
one nit but otherwise nice work!
Description
for review after #3456 is mergedcloses: #3481
Commit Message / Changelog Entry
N/A feature branch
Before we can merge this PR, please make sure that all the following items have been
checked off. If any of the checklist items are not applicable, please leave them but
write a little note why.
docs/
) or specification (x/<module>/spec/
).godoc
comments.Files changed
in the Github PR explorer.Codecov Report
in the comment section below once CI passes.