Allow Unjail of Non-Bonded Jailed Validator (#6061)

* Use correct API and add TODO

* Allow unjail without signing info

* Add unit test

* Update godoc

* Add changelog entries

* Fix TestUndelegateSelfDelegationBelowMinSelfDelegation

* Fix delegation tests

* Revert API call

Author: alexanderbez

CHANGELOG.md

@@ -113,6 +113,8 @@ information on how to implement the new `Keyring` interface.
 
 ### Bug Fixes
 
+* (x/staking) [\#6061](https://github.com/cosmos/cosmos-sdk/pull/6061) Allow a validator to immediately unjail when no signing info is present due to
+falling below their minimum self-delegation and never having been bonded. The validator may immediately unjail once they've met their minimum self-delegation.
 * (types) [\#5741](https://github.com/cosmos/cosmos-sdk/issues/5741) Prevent ChainAnteDecorators() from panicking when empty AnteDecorator slice is supplied.
 * (modules) [\#5569](https://github.com/cosmos/cosmos-sdk/issues/5569) `InitGenesis`, for the relevant modules, now ensures module accounts exist.
 * (crypto/keyring) [\#5844](https://github.com/cosmos/cosmos-sdk/pull/5844) `Keyring.Sign()` methods no longer decode amino signatures when method receivers
@@ -128,6 +130,8 @@ invalid or incomplete requests.
 
 ### State Machine Breaking
 
+* (x/staking) [\#6061](https://github.com/cosmos/cosmos-sdk/pull/6061) Allow a validator to immediately unjail when no signing info is present due to
+falling below their minimum self-delegation and never having been bonded. The validator may immediately unjail once they've met their minimum self-delegation.
 * (x/supply) [\#6010](https://github.com/cosmos/cosmos-sdk/pull/6010) Removed the `x/supply` module by merging the existing types and APIs into the `x/bank` module.
 * (modules) [\#5572](https://github.com/cosmos/cosmos-sdk/pull/5572) Separate balance from accounts per ADR 004.
   * Account balances are now persisted and retrieved via the `x/bank` module.

x/slashing/keeper/keeper_test.go

@@ -4,16 +4,94 @@ import (
 	"testing"
 	"time"
 
-	abci "github.com/tendermint/tendermint/abci/types"
-
 	"github.com/stretchr/testify/require"
+	abci "github.com/tendermint/tendermint/abci/types"
 
 	"github.com/cosmos/cosmos-sdk/simapp"
 	sdk "github.com/cosmos/cosmos-sdk/types"
 	"github.com/cosmos/cosmos-sdk/x/slashing/keeper"
 	"github.com/cosmos/cosmos-sdk/x/staking"
 )
 
+func TestUnJailNotBonded(t *testing.T) {
+	app := simapp.Setup(false)
+	ctx := app.BaseApp.NewContext(false, abci.Header{})
+
+	p := app.StakingKeeper.GetParams(ctx)
+	p.MaxValidators = 5
+	app.StakingKeeper.SetParams(ctx, p)
+
+	amt := sdk.TokensFromConsensusPower(100)
+	sh := staking.NewHandler(app.StakingKeeper)
+
+	addrDels := simapp.AddTestAddrsIncremental(app, ctx, 6, sdk.TokensFromConsensusPower(200))
+	valAddrs := simapp.ConvertAddrsToValAddrs(addrDels)
+	pks := simapp.CreateTestPubKeys(6)
+
+	// create max (5) validators all with the same power
+	for i := uint32(0); i < p.MaxValidators; i++ {
+		addr, val := valAddrs[i], pks[i]
+		res, err := sh(ctx, keeper.NewTestMsgCreateValidator(addr, val, amt))
+		require.NoError(t, err)
+		require.NotNil(t, res)
+	}
+
+	staking.EndBlocker(ctx, app.StakingKeeper)
+	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1)
+
+	// create a 6th validator with less power than the cliff validator (won't be bonded)
+	addr, val := valAddrs[5], pks[5]
+	createValMsg := keeper.NewTestMsgCreateValidator(addr, val, sdk.TokensFromConsensusPower(50))
+	createValMsg.MinSelfDelegation = sdk.TokensFromConsensusPower(50)
+	res, err := sh(ctx, createValMsg)
+	require.NoError(t, err)
+	require.NotNil(t, res)
+
+	staking.EndBlocker(ctx, app.StakingKeeper)
+	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1)
+
+	validator, ok := app.StakingKeeper.GetValidator(ctx, addr)
+	require.True(t, ok)
+	require.False(t, validator.Jailed)
+	require.Equal(t, sdk.BondStatusUnbonded, validator.GetStatus().String())
+
+	// unbond below minimum self-delegation
+	msgUnbond := staking.NewMsgUndelegate(sdk.AccAddress(addr), addr, sdk.NewCoin(p.BondDenom, sdk.TokensFromConsensusPower(1)))
+	res, err = sh(ctx, msgUnbond)
+	require.NoError(t, err)
+	require.NotNil(t, res)
+
+	staking.EndBlocker(ctx, app.StakingKeeper)
+	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1)
+
+	// verify that validator is jailed
+	validator, ok = app.StakingKeeper.GetValidator(ctx, addr)
+	require.True(t, ok)
+	require.True(t, validator.Jailed)
+
+	// verify we cannot unjail (yet)
+	require.Error(t, app.SlashingKeeper.Unjail(ctx, addr))
+
+	staking.EndBlocker(ctx, app.StakingKeeper)
+	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1)
+
+	// bond to meet minimum self-delegation
+	msgBond := staking.NewMsgDelegate(sdk.AccAddress(addr), addr, sdk.NewCoin(p.BondDenom, sdk.TokensFromConsensusPower(1)))
+	res, err = sh(ctx, msgBond)
+	require.NoError(t, err)
+	require.NotNil(t, res)
+
+	staking.EndBlocker(ctx, app.StakingKeeper)
+	ctx = ctx.WithBlockHeight(ctx.BlockHeight() + 1)
+
+	// verify we can immediately unjail
+	require.NoError(t, app.SlashingKeeper.Unjail(ctx, addr))
+
+	validator, ok = app.StakingKeeper.GetValidator(ctx, addr)
+	require.True(t, ok)
+	require.False(t, validator.Jailed)
+}
+
 // Test a new validator entering the validator set
 // Ensure that SigningInfo.StartHeight is set correctly
 // and that they are not immediately jailed

x/slashing/keeper/unjail.go

@@ -2,6 +2,7 @@ package keeper
 
 import (
 	sdk "github.com/cosmos/cosmos-sdk/types"
+	sdkerrors "github.com/cosmos/cosmos-sdk/types/errors"
 	"github.com/cosmos/cosmos-sdk/x/slashing/types"
 )
 
@@ -19,8 +20,12 @@ func (k Keeper) Unjail(ctx sdk.Context, validatorAddr sdk.ValAddress) error {
 		return types.ErrMissingSelfDelegation
 	}
 
-	if validator.TokensFromShares(selfDel.GetShares()).TruncateInt().LT(validator.GetMinSelfDelegation()) {
-		return types.ErrSelfDelegationTooLowToUnjail
+	tokens := validator.TokensFromShares(selfDel.GetShares()).TruncateInt()
+	minSelfBond := validator.GetMinSelfDelegation()
+	if tokens.LT(minSelfBond) {
+		return sdkerrors.Wrapf(
+			types.ErrSelfDelegationTooLowToUnjail, "%s less than %s", tokens, minSelfBond,
+		)
 	}
 
 	// cannot be unjailed if not jailed
@@ -30,19 +35,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 any point
+	// assuming they've now bonded above their minimum self-delegation.
 	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 {
+		// 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)

x/staking/keeper/delegation.go

@@ -577,8 +577,8 @@ func (k Keeper) Unbond(
 
 	isValidatorOperator := delegation.DelegatorAddress.Equals(validator.OperatorAddress)
 
-	// 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 &&
 		validator.TokensFromShares(delegation.Shares).TruncateInt().LT(validator.MinSelfDelegation) {
 

x/staking/keeper/delegation_test.go

@@ -327,6 +327,7 @@ func TestUndelegateSelfDelegationBelowMinSelfDelegation(t *testing.T) {
 	app.AccountKeeper.SetModuleAccount(ctx, notBondedPool)
 
 	validator = keeper.TestingUpdateValidator(app.StakingKeeper, ctx, validator, true)
+	app.StakingKeeper.SetValidatorByConsAddr(ctx, validator)
 	require.True(t, validator.IsBonded())
 
 	selfDelegation := types.NewDelegation(sdk.AccAddress(addrVals[0].Bytes()), addrVals[0], issuedShares)
@@ -380,6 +381,7 @@ func TestUndelegateFromUnbondingValidator(t *testing.T) {
 
 	//create a validator with a self-delegation
 	validator := types.NewValidator(addrVals[0], PKs[0], types.Description{})
+	app.StakingKeeper.SetValidatorByConsAddr(ctx, validator)
 
 	validator, issuedShares := validator.AddTokensFromDel(delTokens)
 	require.Equal(t, delTokens, issuedShares.RoundInt())
@@ -480,6 +482,7 @@ func TestUndelegateFromUnbondedValidator(t *testing.T) {
 
 	// create a validator with a self-delegation
 	validator := types.NewValidator(addrVals[0], PKs[0], types.Description{})
+	app.StakingKeeper.SetValidatorByConsAddr(ctx, validator)
 
 	valTokens := sdk.TokensFromConsensusPower(10)
 	validator, issuedShares := validator.AddTokensFromDel(valTokens)
@@ -564,6 +567,7 @@ func TestUnbondingAllDelegationFromValidator(t *testing.T) {
 
 	//create a validator with a self-delegation
 	validator := types.NewValidator(addrVals[0], PKs[0], types.Description{})
+	app.StakingKeeper.SetValidatorByConsAddr(ctx, validator)
 
 	valTokens := sdk.TokensFromConsensusPower(10)
 	validator, issuedShares := validator.AddTokensFromDel(valTokens)
@@ -820,6 +824,8 @@ func TestRedelegateSelfDelegation(t *testing.T) {
 
 	//create a validator with a self-delegation
 	validator := types.NewValidator(addrVals[0], PKs[0], types.Description{})
+	app.StakingKeeper.SetValidatorByConsAddr(ctx, validator)
+
 	valTokens := sdk.TokensFromConsensusPower(10)
 	validator, issuedShares := validator.AddTokensFromDel(valTokens)
 	require.Equal(t, valTokens, issuedShares.RoundInt())
@@ -877,6 +883,7 @@ func TestRedelegateFromUnbondingValidator(t *testing.T) {
 
 	//create a validator with a self-delegation
 	validator := types.NewValidator(addrVals[0], PKs[0], types.Description{})
+	app.StakingKeeper.SetValidatorByConsAddr(ctx, validator)
 
 	valTokens := sdk.TokensFromConsensusPower(10)
 	validator, issuedShares := validator.AddTokensFromDel(valTokens)
@@ -961,6 +968,7 @@ func TestRedelegateFromUnbondedValidator(t *testing.T) {
 
 	//create a validator with a self-delegation
 	validator := types.NewValidator(addrVals[0], PKs[0], types.Description{})
+	app.StakingKeeper.SetValidatorByConsAddr(ctx, validator)
 
 	valTokens := sdk.TokensFromConsensusPower(10)
 	validator, issuedShares := validator.AddTokensFromDel(valTokens)