fix!: Avoid immediately jailing validators that are no longer opted-out

[mpoke]

Description

Closes: #1517

This PR implements the solution from this comment. Mainly, when a validator can no longer opt out, the StartHeight fields in its SigningInfo struct (in the slashing module) is set to the current height. As a result, the validator cannot be jailed for downtime for at least the sliding window for downtime slashing (SignedBlocksWindow).

---

Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

• Included the correct type prefix in the PR title
• Added ! to the type prefix if the change is state-machine breaking
• Confirmed this PR does not introduce changes requiring state migrations, OR migration code has been added to consumer and/or provider modules
• Targeted the correct branch (see PR Targeting)
• Provided a link to the relevant issue or specification
• Followed the guidelines for building SDK modules
• Included the necessary unit and integration tests
• Added a changelog entry to CHANGELOG.md
• Included comments for documenting Go code
• Updated the relevant documentation or specification
• Reviewed "Files changed" and left comments if necessary
• Confirmed all CI checks have passed
• If this PR is library API breaking, bump the go.mod version string of the repo, and follow through on a new major release

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

• confirmed the correct type prefix in the PR title
• confirmed ! the type prefix if the change is state-machine breaking
• confirmed this PR does not introduce changes requiring state migrations, OR confirmed migration code has been added to consumer and/or provider modules
• confirmed all author checklist items have been addressed
• reviewed state machine logic
• reviewed API design and naming
• reviewed documentation is accurate
• reviewed tests and test coverage

[p-offtermatt]

Curious about this: why reset the start height instead of the missed blocks counter?

afaict, the cleanest solution to me seems to be to, for every opt-out validator, set the missed blocks counter to zero in each block, before it is entered on chain.

This has a few advantages:

• No need for an extra field on validators (though it's still state machine breaking)
• Works out-of-the-box with existing downtime checkers (they will just not see the missed blocks)

I might be missing something though. I think the current solution is also fine, maybe not worth changing this for.

[mpoke]

Curious about this: why reset the start height instead of the missed blocks counter?

@p-offtermatt I didn't want to mess with the downtime logic too much. If you set the missed blocks counter to zero, you risk having the counter negative and potentially never trigger a downtime event. See https://github.com/cosmos/cosmos-sdk/blob/v0.47.7/x/slashing/keeper/infractions.go#L41-L52.

afaict, the cleanest solution to me seems to be to, for every opt-out validator, set the missed blocks counter to zero in each block, before it is entered on chain.

That would be a lot of store writes and I don't see the advantage of doing it like this.

This has a few advantages:

• No need for an extra field on validators (though it's still state machine breaking)

Why is the extra field an issue?

• Works out-of-the-box with existing downtime checkers (they will just not see the missed blocks)

See my comment above. I think reseting the missed blocks counter would mess with the downtime logic.

[p-offtermatt]

Makes sense, thanks. I didn't take a close look at the missed-blocks logic in the slashing module (which is more intricate than I thought), and indeed I think your solution seems good under the constraints.

[bermuell]

pls accept suggested changes on previous comments, thx