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: [Interchain Security] Add equality to validator.IsMature #12537

Closed
wants to merge 1 commit into from

Conversation

mpoke
Copy link

@mpoke mpoke commented Jul 12, 2022

@mpoke mpoke requested review from jtremback and danwt July 12, 2022 11:07
@mpoke mpoke changed the title Interchain Security: Add equality to validator.IsMature fix: [Interchain Security] Add equality to validator.IsMature Jul 12, 2022
Copy link
Contributor

@alexanderbez alexanderbez left a comment

Choose a reason for hiding this comment

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

I think this is OK and true since it's within the context of the current executing block, but I'm not 100% certain as x/staking has the assumption of H+1 execution due to the current Tendermint model.

I see simulations failing so this makes me slightly even more worried. Regardless, let's add a changelog and then investigate sim failures.

@danwt
Copy link
Contributor

danwt commented Jul 13, 2022

Beware

@mpoke
Copy link
Author

mpoke commented Jul 13, 2022

I think this is OK and true since it's within the context of the current executing block, but I'm not 100% certain as x/staking has the assumption of H+1 execution due to the current Tendermint model.

I see simulations failing so this makes me slightly even more worried. Regardless, let's add a changelog and then investigate sim failures.

@alexanderbez this PR shouldn't break the tests since the change only affects code that is called when Interchain Security (IS) is enabled, and I'm quite sure that there are no checks for that :) The function validator.IsMature() is called only in x/staking/keeper/unbonding.go when an external module (i.e., the CCV module) notifies the staking module that an unbonding operation (undelegation, redelegation, or validator unbonding) can complete from the perspective of IS (i.e., it calls UnbondingCanComplete()). Since the checks are failing also for the branch with all SDK changes needed for IS (https://github.com/cosmos/cosmos-sdk/tree/interchain-security-rebase), I assume that the problem is somewhere there. I've opened an issue on the IS repo, i.e., cosmos/interchain-security#227

@danwt
Copy link
Contributor

danwt commented Jul 14, 2022

I suggest closing this in favour of merging #12578 into #12554 and then merging #12554

@tac0turtle tac0turtle closed this Jul 27, 2022
@mpoke mpoke deleted the marius/ccv-val-ismature branch August 3, 2022 09:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants