-
Notifications
You must be signed in to change notification settings - Fork 690
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
chore: update go.mod to prevent CI build breakages (add ICS v4.1.0-lsm-rc2) #3039
Conversation
@@ -26,6 +27,11 @@ func CreateUpgradeHandler( | |||
// Enable ICA controller | |||
keepers.ICAControllerKeeper.SetParams(ctx, icacontrollertypes.DefaultParams()) | |||
|
|||
// Set default blocks per epoch | |||
providerParams := keepers.ProviderKeeper.GetParams(ctx) |
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 will panic since BlocksPerEpoch
key doesn't exist in the paramstore at this point.
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.
The upgrade code is in cosmos/interchain-security#1757
It will should be available in the next release candidate of [email protected]
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.
Changes are available here:
providerParams := keepers.ProviderKeeper.GetParams(ctx) | ||
providerParams.BlocksPerEpoch = providertypes.DefaultBlocksPerEpoch | ||
keepers.ProviderKeeper.SetParams(ctx, providerParams) | ||
|
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 can do this instead:
subspace := keepers.GetSubspace(providertypes.ModuleName)
subspace.Set(ctx, providertypes.KeyBlocksPerEpoch, int64(providertypes.DefaultBlocksPerEpoch))
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.
The upgrade code is in cosmos/interchain-security#1757
It will should be available in the next release candidate of [email protected]
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.
Okay, that makes sense to do the upgrade in ICS. So why is the BlocksPerEpoch
param set to default here (again)?
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.
Making it explicit. Otherwise you have to reference ICS docs - reviewing and tracking changelogs becomes a pain.
We did a similar thing for ICA controller.
Please wait with the merge. ICS v4.1.0-rc2 needs to be linked in go.mod. EDIT: |
This PR fixes a CI issue where the build breaks during liveness and upgrade tests.
Part of the fix is updating ICS version to v4.1.0-rc1.