If a validator was jailed because it's self delegation less than minimum due to the undelegate operation, and this validator has never been bonded, then it will never be unjailed. More details below:
unjail transactionsigningInfo to check if it could be unjailed. If this 'signingInfo' can not be found, then we can't unjail itsigningInfo can only be created when a validator is bonded.so, when a validator unbound its self delegation to cause its self delegation less than minimum , it will be jailed, and if it has never been bonded, then its signingInfo will never be created. If it delegates again to let its self delegation more than minimum, then it is reasonable that we allow it to unjail itself by submitting 'unjail' transaction. however, it is not allowed according to our current logic, and it will never be unjailed.
master
You may still bond to that validator correct? Regardless, it seems this is a flaw.
The hooks make sense in the way they're called. Namely, you should only have signing info once you are bonded. However, the Unjail handler requires signing info to be present as to check if the validator is tombstoned and when its safe to unjail.
So the behavior here seems undefined. You must further bond to that validator.
I am just wondering why can only bonded validator be unjailed? In current stake module, it is possible that one unboned validator can be jailed, right? And it can not be boned until it is unjailed. In addition, do we have to check the signing info's existence when trying to unjail a validator? If its signing info is missing, it means the validator has never been bonded which also means it must not be tombstoned. So perhaps we may give the opportunity to unjail a validator without signing info, right?
I am just wondering why can only bonded validator be unjailed?
Because validators that were never bonded don't have a corresponding SigningInfo.
In current stake module, it is possible that one unboned validator can be jailed, right?
Yes, as you've laid out, if you dip below the minimum self-bond.
And it can not be boned until it is unjailed.
You cannot be in the validator set, correct.
So perhaps we may give the opportunity to unjail a validator without signing info, right?
Yes, but that's not trivial. To unjail, you need SigningInfo and you need that SigningInfo to have the correct JailedUntil attribute.
An approach to fix this is the following:
Here we jail the validator if they dip below the min-self-bond:
// 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 isValidatorOperator && !validator.Jailed &&
validator.TokensFromShares(delegation.Shares).TruncateInt().LT(validator.MinSelfDelegation) {
k.jailValidator(ctx, validator)
validator = k.mustGetValidator(ctx, validator.OperatorAddress)
}
But this does nothing WRT to SigningInfo. We instead need to call out to x/slashing somehow. This call needs to:
SigningInfo for the validator (if it does not exist)JailedUntil to the correct value. // 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 isValidatorOperator && !validator.Jailed &&
validator.TokensFromShares(delegation.Shares).TruncateInt().LT(validator.MinSelfDelegation) {
k.slashingKeeper.Jail(ctx, validator) // this is what we need to call somehow
validator = k.mustGetValidator(ctx, validator.OperatorAddress)
}
Really appreciate your patient reply. That is good advice. Btw, does this SDK plan to add this feature? I mean, calling out to x/slashing somehow to create corresponding signing info and set JailedUntil
Yes, this seems like a bug to me that we need to fix. It'll be helpful to get an ACK from @cwgoes or @rigelrozanski
@alexanderbez Yes, I agree, and I think your proposed fix should be fine.
It sounds like this may be worth a backport to 0.38 and 0.37? @cwgoes @alexanderbez wdyt?
It sounds like this may be worth a backport to 0.38 and 0.37? @cwgoes @alexanderbez wdyt?
Unfortunately it cannot be back-ported. The solution is state-machine breaking. A tx that was once deemed unsuccessful, will now be successful.