-
Notifications
You must be signed in to change notification settings - Fork 3.6k
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
Allow Unjail of Non-Bonded Jailed Validator #6061
Conversation
// if the delegation is the operator of the validator and undelegating will decrease the validator's self delegation below their minimum | ||
// trigger a jail validator | ||
// If the delegation is the operator of the validator and undelegating will decrease the validator's | ||
// self-delegation below their minimum, we jail the validator. | ||
if isValidatorOperator && !validator.Jailed && |
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.
Note there is no change here apart from keeping the code DRY and calling k.Jail
instead of manually calling the private internal APIs.
// cannot be unjailed until out of jail | ||
if ctx.BlockHeader().Time.Before(info.JailedUntil) { | ||
return types.ErrValidatorJailed | ||
if found { |
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 core change is here. We now allow a validator to unjail if no ValidatorSigningInfo
is present. This should only ever be the case under the condition of the reported issue. Essentially, we allow a validator to unjail immediately (once they've met their minimum self-delegation).
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.
LGTM, although it not feel like I have enough knowledge of the staking module internals to know whether there could be any unintended consequences.
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.
utACK
I think this is fine, 👀 from @rigelrozanski would be good too.
Codecov Report
@@ Coverage Diff @@
## master #6061 +/- ##
==========================================
+ Coverage 54.35% 54.39% +0.04%
==========================================
Files 430 430
Lines 26103 26106 +3
==========================================
+ Hits 14187 14201 +14
+ Misses 10937 10922 -15
- Partials 979 983 +4 |
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.
ACK - few comments. Good idea
// A validator that is jailed but has no ValidatorSigningInfo object signals | ||
// that the validator was never bonded and must've been jailed due to falling | ||
// below their minimum self-delegation. The validator can unjail at any point | ||
// assuming they've now bonded above their minimum self-delegation. |
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'd prefer to have some form of context by which we can verify the reason for being jailed for situations like this. In the future it may be that there are other reasons that an unbonded validator may be jailed (and other privileges to being a validator "in waiting"). I could see the Validator Jailed bool
field being replaced with a JailedStatus uint8
kind whereby the reason for being jailed could be specified (here, 0
value would probably indicate "not-jailed"). Anyways that's a breaking change, but I think it could be helpful.... probably for another PR or issue!
Description
Allow a validator to immediately unjail due to being jailed for falling below their minimum self-delegation and never having been bonded (i.e. no signing info present), assuming they've now met their minimum self-delegation.
closes: #6004
For contributor use:
docs/
) or specification (x/<module>/spec/
)godoc
comments.Unreleased
section inCHANGELOG.md
Files changed
in the Github PR explorerFor admin use:
WIP
,R4R
,docs
, etc)