Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
e2e: schedule IBC software upgrade #4585
e2e: schedule IBC software upgrade #4585
Changes from 16 commits
5550dfa
6d2d16b
fd45b73
1b648a1
e704c0a
5d04194
ab2bd3d
ad00d75
c85f06d
371d0a4
c9872e9
a71a3b5
ce48d0b
a395691
c52b67a
9a1fae9
1642975
ea7183a
ab25260
f8c5503
871de62
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
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 wanted to query if the plan has been stored/upgraded client state was stored, but realised after digging around that the UpgradeKeeper doesn't seem to have a query client. what do you guys think? our
msg_server
tests do perform this check, is that sufficient?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.
You can try to upgrade the client on the counterparty. Hermes might do it automatically. Maybe try starting the relayer and querying the client's status afterward
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.
IIRC correctly hermes previously had an
upgrade client
cmd which actually submitted the proposal on your behalf. See here - and also hereAt a glance I think this is supposed to be doing the upgrade client flow, but I'm unsure how hermes handles this. Does it block and wait(?) until upgrade height, then query the upgradedClient at upgradePath and then upgrade the counterparty.
I think their cmd would need to change for gov v1, maybe it still uses the legacy v1beta1 appraoch
edit: added additional link - looks like they have cmd to send proposal and separate to then carry out the upgrade, my thinking would be that the latter blocks and waits for upgrade height
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.
Might need to check if interchaintest has built in the support for client updates. Hermes can do it based on the links in my first comment. But I'm unsure if rly also has the same support, and interchaintest would need to expose an API for it in their relayer interface.
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.
We added event emission when the consensus state is written allowing the counterparty client to be upgraded, unsure if hermes started listening for that event
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 regress on my suggestion, happy to finish the full e2e of upgrading the client in a followup pr as this isn't essential for v8 testing
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.
after discussion with @colin-axner , will just merge this e2e testing the scope of the
ScheduleIBCSoftwareUpgrade
for now.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.
You're right, @damiannolan: hermes is using gov v1beta proposal. Let's ask them.