diff --git a/CHANGELOG.md b/CHANGELOG.md index a8651cb65c4a..146e80a51503 100644 --- a/CHANGELOG.md +++ b/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. diff --git a/x/slashing/keeper/keeper_test.go b/x/slashing/keeper/keeper_test.go index ce28a5607b23..cd09211fa8f0 100644 --- a/x/slashing/keeper/keeper_test.go +++ b/x/slashing/keeper/keeper_test.go @@ -4,9 +4,8 @@ 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" @@ -14,6 +13,85 @@ import ( "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 diff --git a/x/slashing/keeper/unjail.go b/x/slashing/keeper/unjail.go index cf57c1e71c4a..a8dab94d25d4 100644 --- a/x/slashing/keeper/unjail.go +++ b/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) diff --git a/x/staking/keeper/delegation.go b/x/staking/keeper/delegation.go index 4a20f74c9026..f70879ca485d 100644 --- a/x/staking/keeper/delegation.go +++ b/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) { diff --git a/x/staking/keeper/delegation_test.go b/x/staking/keeper/delegation_test.go index 8eaee5709bc8..7c64df120371 100644 --- a/x/staking/keeper/delegation_test.go +++ b/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)