diff --git a/CHANGELOG.md b/CHANGELOG.md index be5320f17273..94f33773a4da 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -37,6 +37,10 @@ Ref: https://keepachangelog.com/en/1.0.0/ ## [Unreleased] +### Bug Fixes + +* (x/staking) [#19226](https://github.com/cosmos/cosmos-sdk/pull/19226) Ensure `GetLastValidators` in `x/staking` does not return an error when `MaxValidators` exceeds total number of bonded validators. + ## [v0.47.8](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.47.8) - 2024-01-22 ### Improvements diff --git a/x/staking/keeper/validator.go b/x/staking/keeper/validator.go index d1315ed7793f..0179b3649ef5 100644 --- a/x/staking/keeper/validator.go +++ b/x/staking/keeper/validator.go @@ -314,16 +314,21 @@ func (k Keeper) GetLastValidators(ctx sdk.Context) (validators []types.Validator // add the actual validator power sorted store maxValidators := k.MaxValidators(ctx) - validators = make([]types.Validator, maxValidators) iterator := sdk.KVStorePrefixIterator(store, types.LastValidatorPowerKey) defer iterator.Close() i := 0 + validators = make([]types.Validator, maxValidators) for ; iterator.Valid(); iterator.Next() { - // sanity check + // Note, we do NOT error here as the MaxValidators param may change via on-chain + // governance. In cases where the param is increased, this case should never + // be hit. In cases where the param is decreased, we will simply not return + // the remainder of the validator set, as the ApplyAndReturnValidatorSetUpdates + // call should ensure the validators past the cliff will be moved to the + // unbonding set. if i >= int(maxValidators) { - panic("more validators than maxValidators found") + break } address := types.AddressFromLastValidatorPowerKey(iterator.Key()) diff --git a/x/staking/keeper/validator_test.go b/x/staking/keeper/validator_test.go index d4bc16778010..1c494ec0788f 100644 --- a/x/staking/keeper/validator_test.go +++ b/x/staking/keeper/validator_test.go @@ -81,6 +81,46 @@ func (s *KeeperTestSuite) TestValidator() { require.Equal(int64(0), resPower) } +func (s *KeeperTestSuite) TestGetLastValidators() { + ctx, keeper := s.ctx, s.stakingKeeper + require := s.Require() + + params := keeper.GetParams(ctx) + params.MaxValidators = 50 + require.NoError(keeper.SetParams(ctx, params)) + + // construct 50 validators all with equal power of 100 + var validators [50]stakingtypes.Validator + for i := 0; i < 50; i++ { + validators[i] = testutil.NewValidator(s.T(), sdk.ValAddress(PKs[i].Address().Bytes()), PKs[i]) + validators[i].Status = stakingtypes.Unbonded + validators[i].Tokens = math.ZeroInt() + tokens := keeper.TokensFromConsensusPower(ctx, 100) + + validators[i], _ = validators[i].AddTokensFromDel(tokens) + require.Equal(keeper.TokensFromConsensusPower(ctx, 100), validators[i].Tokens) + + s.bankKeeper.EXPECT().SendCoinsFromModuleToModule(gomock.Any(), stakingtypes.NotBondedPoolName, stakingtypes.BondedPoolName, gomock.Any()) + + validators[i] = stakingkeeper.TestingUpdateValidator(keeper, ctx, validators[i], true) + require.NoError(keeper.SetValidatorByConsAddr(ctx, validators[i])) + + resVal, ok := keeper.GetValidator(ctx, sdk.ValAddress(PKs[i].Address().Bytes())) + require.True(ok) + require.True(validators[i].MinEqual(&resVal)) + } + + res := keeper.GetLastValidators(ctx) + require.Len(res, 50) + + // reduce max validators to 30 and ensure we only get 30 back + params.MaxValidators = 30 + require.NoError(keeper.SetParams(ctx, params)) + + res = keeper.GetLastValidators(ctx) + require.Len(res, 30) +} + // This function tests UpdateValidator, GetValidator, GetLastValidators, RemoveValidator func (s *KeeperTestSuite) TestValidatorBasics() { ctx, keeper := s.ctx, s.stakingKeeper