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

CHANGELOG.md

@@ -77,6 +77,7 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
 
 ### 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.
 * (baseapp) [#19198](https://github.com/cosmos/cosmos-sdk/pull/19198) Remove usage of pointers in logs in all OE goroutines.
 * (baseapp) [#18727](https://github.com/cosmos/cosmos-sdk/pull/18727) Ensure that `BaseApp.Init` firstly returns any errors from a nil commit multistore instead of panicking on nil dereferencing and before sealing the app.
 * (client) [#18622](https://github.com/cosmos/cosmos-sdk/pull/18622) Fixed a potential under/overflow from `uint64->int64` when computing gas fees as a LegacyDec.
@@ -542,14 +543,14 @@ Every module contains its own CHANGELOG.md. Please refer to the module you are i
 
 ### Bug Fixes
 
-* [#19106](https://github.com/cosmos/cosmos-sdk/pull/19106) Allow empty public keys when setting signatures. Public keys aren't needed for every transaction. 
+* [#19106](https://github.com/cosmos/cosmos-sdk/pull/19106) Allow empty public keys when setting signatures. Public keys aren't needed for every transaction.
 * (server) [#18920](https://github.com/cosmos/cosmos-sdk/pull/18920) Fixes consensus failure while restart node with wrong `chainId` in genesis.
 
 ## [v0.47.7](https://github.com/cosmos/cosmos-sdk/releases/tag/v0.47.7) - 2023-12-20
 
 ### Improvements
 
-* (x/gov) [#18707](https://github.com/cosmos/cosmos-sdk/pull/18707) Improve genesis validation. 
+* (x/gov) [#18707](https://github.com/cosmos/cosmos-sdk/pull/18707) Improve genesis validation.
 * (server) [#18478](https://github.com/cosmos/cosmos-sdk/pull/18478) Add command flag to disable colored logs.
 
 ### Bug Fixes

x/staking/keeper/validator.go

@@ -383,13 +383,19 @@ func (k Keeper) GetLastValidators(ctx context.Context) (validators []types.Valid
 	if err != nil {
 		return nil, err
 	}
-	validators = make([]types.Validator, maxValidators)
 
 	i := 0
+	validators = make([]types.Validator, maxValidators)
+
 	err = k.LastValidatorPower.Walk(ctx, nil, func(key []byte, _ gogotypes.Int64Value) (bool, error) {
-		// 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) {
-			return true, fmt.Errorf("more validators than maxValidators found")
+			return true, nil
 		}
 
 		validator, err := k.GetValidator(ctx, key)
@@ -399,6 +405,7 @@ func (k Keeper) GetLastValidators(ctx context.Context) (validators []types.Valid
 
 		validators[i] = validator
 		i++
+
 		return false, nil
 	})
 	if err != nil {

x/staking/keeper/validator_test.go

@@ -89,6 +89,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.Params.Get(ctx)
+	require.NoError(err)
+
+	params.MaxValidators = 50
+	require.NoError(keeper.Params.Set(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.Params.Set(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