Address some comments

x/ccv/provider/keeper/grpc_query_test.go

@@ -170,7 +170,7 @@ func TestQueryConsumerChainsValidatorHasToValidate(t *testing.T) {
 
 	// set max provider consensus vals to include all validators
 	params := pk.GetParams(ctx)
-	params.MaxProviderConsensusValidators = 180
+	params.MaxProviderConsensusValidators = 3
 	pk.SetParams(ctx, params)
 
 	// `providerAddr` has to validate "chain1" because it is a consumer validator in this chain, as well as "chain3"

x/ccv/provider/keeper/keeper.go

@@ -1201,23 +1201,23 @@ func (k Keeper) DeleteMinStake(
 	store.Delete(types.MinStakeKey(chainID))
 }
 
-// SetAllowInactiveValidators sets whether inactive validators are allowed to validate
+// SetInactiveValidatorsAllowed sets whether inactive validators are allowed to validate
 // a given consumer chain.
-func (k Keeper) SetAllowInactiveValidators(
+func (k Keeper) SetInactiveValidatorsAllowed(
 	ctx sdk.Context,
 	chainID string,
 	allowed bool,
 ) {
 	if allowed {
-		k.AllowInactiveValidators(ctx, chainID)
+		k.EnableInactiveValidators(ctx, chainID)
 	} else {
-		k.DeleteAllowInactiveValidators(ctx, chainID)
+		k.DisableInactiveValidators(ctx, chainID)
 	}
 }
 
-// AllowInactiveValidators sets the flag to signal that inactive validators are allowed to validate
+// EnableInactiveValidators sets the flag to signal that inactive validators are allowed to validate
 // a given consumer chain.
-func (k Keeper) AllowInactiveValidators(
+func (k Keeper) EnableInactiveValidators(
 	ctx sdk.Context,
 	chainID string,
 ) {
@@ -1235,9 +1235,9 @@ func (k Keeper) AllowsInactiveValidators(
 	return store.Has(types.AllowInactiveValidatorsKey(chainID))
 }
 
-// DeleteAllowInactiveValidators removes the flag of whether inactive validators are allowed to validate
+// DisableInactiveValidators removes the flag of whether inactive validators are allowed to validate
 // a given consumer chain.
-func (k Keeper) DeleteAllowInactiveValidators(
+func (k Keeper) DisableInactiveValidators(
 	ctx sdk.Context,
 	chainID string,
 ) {

x/ccv/provider/keeper/keeper_test.go

@@ -555,13 +555,13 @@ func TestAllowInactiveValidators(t *testing.T) {
 	require.False(t, k.AllowsInactiveValidators(ctx, chainID))
 
 	// set the chain to allow inactive validators
-	k.SetAllowInactiveValidators(ctx, chainID, true)
+	k.SetInactiveValidatorsAllowed(ctx, chainID, true)
 
 	// check that AllowsInactiveValidators returns true
 	require.True(t, k.AllowsInactiveValidators(ctx, chainID))
 
 	// set the chain to not allow inactive validators
-	k.SetAllowInactiveValidators(ctx, chainID, false)
+	k.SetInactiveValidatorsAllowed(ctx, chainID, false)
 
 	// check that AllowsInactiveValidators returns false
 	require.False(t, k.AllowsInactiveValidators(ctx, chainID))

x/ccv/provider/keeper/legacy_proposal.go

@@ -93,7 +93,7 @@ func (k Keeper) HandleLegacyConsumerModificationProposal(ctx sdk.Context, p *typ
 	k.SetValidatorsPowerCap(ctx, p.ChainId, p.ValidatorsPowerCap)
 	k.SetValidatorSetCap(ctx, p.ChainId, p.ValidatorSetCap)
 	k.SetMinStake(ctx, p.ChainId, p.MinStake)
-	k.SetAllowInactiveValidators(ctx, p.ChainId, p.AllowInactiveVals)
+	k.SetInactiveValidatorsAllowed(ctx, p.ChainId, p.AllowInactiveVals)
 
 	k.DeleteAllowlist(ctx, p.ChainId)
 	for _, address := range p.Allowlist {

x/ccv/provider/keeper/legacy_proposal_test.go

@@ -287,7 +287,7 @@ func TestHandleConsumerModificationProposal(t *testing.T) {
 	providerKeeper.SetAllowlist(ctx, chainID, providertypes.NewProviderConsAddress([]byte("allowlistedAddr2")))
 	providerKeeper.SetDenylist(ctx, chainID, providertypes.NewProviderConsAddress([]byte("denylistedAddr1")))
 	providerKeeper.SetMinStake(ctx, chainID, 1000)
-	providerKeeper.SetAllowInactiveValidators(ctx, chainID, true)
+	providerKeeper.SetInactiveValidatorsAllowed(ctx, chainID, true)
 
 	expectedTopN := uint32(75)
 	expectedValidatorsPowerCap := uint32(67)

x/ccv/provider/keeper/partial_set_security.go

@@ -329,7 +329,7 @@ func (k Keeper) FulfillsMinStake(ctx sdk.Context, chainID string, providerAddr t
 func (k Keeper) ComputeNextValidators(ctx sdk.Context, chainID string, bondedValidators []stakingtypes.Validator) []types.ConsensusValidator {
 	// sort the bonded validators by number of staked tokens in descending order
 	sort.Slice(bondedValidators, func(i, j int) bool {
-		return bondedValidators[i].GetTokens().GT(bondedValidators[j].GetTokens())
+		return bondedValidators[i].GetBondedTokens().GT(bondedValidators[j].GetTokens())
 	})
 
 	// if inactive validators are not allowed, only consider the first `MaxProviderConsensusValidators` validators

x/ccv/provider/keeper/partial_set_security_test.go

@@ -709,34 +709,34 @@ func createStakingValidatorsAndMocks(ctx sdk.Context, mocks testkeeper.MockedKee
 	return validators, consAddrs
 }
 
-// TestMinStake checks that FulfillsMinStake returns true if the validator has more than the min stake
+// TestFulfillsMinStake checks that FulfillsMinStake returns true if the validator has at least the min stake
 // and false otherwise
-func TestMinStake(t *testing.T) {
+func TestFulfillsMinStake(t *testing.T) {
 	providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
 	defer ctrl.Finish()
 
 	// create two validators with powers 1 and 2
 	_, consAddrs := createStakingValidatorsAndMocks(ctx, mocks, 1, 2)
 
 	testCases := []struct {
-		name           string
-		minStake       uint64
-		expectedFulfil []bool
+		name            string
+		minStake        uint64
+		expectedFulfill []bool
 	}{
 		{
-			name:           "No min stake",
-			minStake:       0,
-			expectedFulfil: []bool{true, true},
+			name:            "No min stake",
+			minStake:        0,
+			expectedFulfill: []bool{true, true},
 		},
 		{
-			name:           "Min stake set to 2",
-			minStake:       2,
-			expectedFulfil: []bool{false, true},
+			name:            "Min stake set to 2",
+			minStake:        2,
+			expectedFulfill: []bool{false, true},
 		},
 		{
-			name:           "Min stake set to 3",
-			minStake:       3,
-			expectedFulfil: []bool{false, false},
+			name:            "Min stake set to 3",
+			minStake:        3,
+			expectedFulfill: []bool{false, false},
 		},
 	}
 
@@ -745,14 +745,14 @@ func TestMinStake(t *testing.T) {
 			providerKeeper.SetMinStake(ctx, "chainID", tc.minStake)
 			for i, valAddr := range consAddrs {
 				result := providerKeeper.FulfillsMinStake(ctx, "chainID", valAddr)
-				require.Equal(t, tc.expectedFulfil[i], result)
+				require.Equal(t, tc.expectedFulfill[i], result)
 			}
 		})
 	}
 }
 
-// TestMaxProviderConsensusValidators checks that the number of validators in the next validator set is at most
-// the maxProviderConsensusValidators parameter if the consumer chain does not allow inactive validators to validate.
+// TestIfInactiveValsDisallowedProperty checks that the number of validators in the next validator set is at most
+// the MaxProviderConsensusValidators parameter if the consumer chain does not allow inactive validators to validate.
 func TestIfInactiveValsDisallowedProperty(t *testing.T) {
 	rapid.Check(t, func(r *rapid.T) {
 		providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
@@ -772,7 +772,9 @@ func TestIfInactiveValsDisallowedProperty(t *testing.T) {
 		maxProviderConsensusVals := rapid.Uint32Range(1, 11).Draw(r, "maxProviderConsensusVals")
 
 		// Set up the parameters in the provider keeper
-		providerKeeper.SetAllowInactiveValidators(ctx, "chainID", false) // do not allow inactive validators
+
+		// do not allow inactive validators
+		providerKeeper.SetInactiveValidatorsAllowed(ctx, "chainID", false)
 		providerKeeper.SetMinStake(ctx, "chainID", minStake)
 		params := providerKeeper.GetParams(ctx)
 		params.MaxProviderConsensusValidators = int64(maxProviderConsensusVals)

x/ccv/provider/keeper/proposal.go

@@ -219,8 +219,8 @@ func (k Keeper) StopConsumerChain(ctx sdk.Context, chainID string, closeChan boo
 	k.DeleteValidatorSetCap(ctx, chainID)
 	k.DeleteAllowlist(ctx, chainID)
 	k.DeleteDenylist(ctx, chainID)
-	k.DeleteAllowInactiveValidators(ctx, chainID)
 	k.DeleteMinStake(ctx, chainID)
+	k.DisableInactiveValidators(ctx, chainID)
 
 	k.DeleteAllOptedIn(ctx, chainID)
 	k.DeleteConsumerValSet(ctx, chainID)
@@ -268,6 +268,9 @@ func (k Keeper) MakeConsumerGenesis(
 
 	if prop.Top_N > 0 {
 		// get the consensus active validators
+		// we do not want to base the power calculation for the top N
+		// on inactive validators, too, since the top N will be a percentage of the active set power
+		// otherwise, it could be that inactive validators are forced to validate
 		activeValidators, err := k.GetLastActiveBondedValidators(ctx)
 		if err != nil {
 			return gen, nil, errorsmod.Wrapf(stakingtypes.ErrNoValidatorFound, "error getting last active bonded validators: %s", err)
@@ -373,7 +376,7 @@ func (k Keeper) BeginBlockInit(ctx sdk.Context) {
 		k.SetValidatorSetCap(cachedCtx, prop.ChainId, prop.ValidatorSetCap)
 		k.SetValidatorsPowerCap(cachedCtx, prop.ChainId, prop.ValidatorsPowerCap)
 		k.SetMinStake(cachedCtx, prop.ChainId, prop.MinStake)
-		k.SetAllowInactiveValidators(cachedCtx, prop.ChainId, prop.AllowInactiveVals)
+		k.SetInactiveValidatorsAllowed(cachedCtx, prop.ChainId, prop.AllowInactiveVals)
 
 		for _, address := range prop.Allowlist {
 			consAddr, err := sdk.ConsAddressFromBech32(address)