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

Allow Unjail of Non-Bonded Jailed Validator #6061

Merged
merged 11 commits into from
Apr 26, 2020
Prev Previous commit
Next Next commit
Allow unjail without signing info
  • Loading branch information
alexanderbez committed Apr 24, 2020
commit 8061212378192561f4982a83a0e55b792eb25f80
30 changes: 18 additions & 12 deletions x/slashing/keeper/unjail.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,19 +30,25 @@ func (k Keeper) Unjail(ctx sdk.Context, validatorAddr sdk.ValAddress) error {

consAddr := sdk.ConsAddress(validator.GetConsPubKey().Address())

// If the validator has a ValidatorSigningInfo object that signals that the
// validator was bonded and so we must check that the validator is not tombstoned
// and can be unjailed at the current block.
//
// 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 point
// assuming they've now bonded above their minimum self-delegation.
Copy link
Contributor

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!

info, found := k.GetValidatorSigningInfo(ctx, consAddr)
if !found {
return types.ErrNoValidatorForAddress
}

// cannot be unjailed if tombstoned
if info.Tombstoned {
return types.ErrValidatorJailed
}

// cannot be unjailed until out of jail
if ctx.BlockHeader().Time.Before(info.JailedUntil) {
return types.ErrValidatorJailed
if found {
Copy link
Contributor Author

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).

// cannot be unjailed if tombstoned
if info.Tombstoned {
return types.ErrValidatorJailed
}

// cannot be unjailed until out of jail
if ctx.BlockHeader().Time.Before(info.JailedUntil) {
return types.ErrValidatorJailed
}
}

k.sk.Unjail(ctx, consAddr)
Expand Down
1 change: 0 additions & 1 deletion x/staking/keeper/delegation.go
Original file line number Diff line number Diff line change
Expand Up @@ -582,7 +582,6 @@ func (k Keeper) Unbond(
if isValidatorOperator && !validator.Jailed &&
Copy link
Contributor Author

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.

validator.TokensFromShares(delegation.Shares).TruncateInt().LT(validator.MinSelfDelegation) {

// TODO: Create signing info with jailing info set.
k.Jail(ctx, validator.GetConsAddr())
validator = k.mustGetValidator(ctx, validator.OperatorAddress)
}
Expand Down