fix(x/staking): Refactor GetLastValidators (backport #19226)

CHANGELOG.md

@@ -40,6 +40,7 @@ Ref: https://keepachangelog.com/en/1.0.0/
 
 ### 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.
 * [#19106](https://github.com/cosmos/cosmos-sdk/pull/19106) Allow empty public keys when setting signatures. Public keys aren't needed for every transaction. 
 
 ## [v0.50.3](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.50.3) - 2023-01-15

x/staking/keeper/validator.go

@@ -416,7 +416,6 @@ func (k Keeper) GetLastValidators(ctx context.Context) (validators []types.Valid
 	if err != nil {
 		return nil, err
 	}
-	validators = make([]types.Validator, maxValidators)
 
 	iterator, err := store.Iterator(types.LastValidatorPowerKey, storetypes.PrefixEndBytes(types.LastValidatorPowerKey))
 	if err != nil {
@@ -425,10 +424,16 @@ func (k Keeper) GetLastValidators(ctx context.Context) (validators []types.Valid
 	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")
+			return validators, nil
 		}
 
 		address := types.AddressFromLastValidatorPowerKey(iterator.Key())

x/staking/keeper/validator_test.go

@@ -86,6 +86,50 @@ func (s *KeeperTestSuite) TestValidator() {
 	require.Equal(int64(0), resPower)
 }
 
+func (s *KeeperTestSuite) TestGetLastValidators() {
+	ctx, keeper := s.ctx, s.stakingKeeper
+	require := s.Require()
+
+	params, err := keeper.GetParams(ctx)
+	require.NoError(err)
+
+	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, err := keeper.GetValidator(ctx, sdk.ValAddress(PKs[i].Address().Bytes()))
+		require.NoError(err)
+		require.True(validators[i].MinEqual(&resVal))
+	}
+
+	res, err := keeper.GetLastValidators(ctx)
+	require.NoError(err)
+	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, err = keeper.GetLastValidators(ctx)
+	require.NoError(err)
+	require.Len(res, 30)
+}
+
 // This function tests UpdateValidator, GetValidator, GetLastValidators, RemoveValidator
 func (s *KeeperTestSuite) TestValidatorBasics() {
 	ctx, keeper := s.ctx, s.stakingKeeper