Refactored code logic

cosmos · Oct 22, 2024 · e878390 · e878390
1 parent e40a8c6
commit e878390
Show file tree

Hide file tree

Showing 3 changed files with 30 additions and 156 deletions.
diff --git a/x/ccv/provider/keeper/power_shaping.go b/x/ccv/provider/keeper/power_shaping.go
@@ -88,37 +88,22 @@ func (k Keeper) UpdateMinimumPowerInTopN(ctx sdk.Context, consumerId string, old
 }
 
 // CapValidatorSet caps the provided `validators` if chain with `consumerId` is an Opt In chain with a validator-set cap.
-// It prioritizes validators from the priority list and then adds remaining validators.
 func (k Keeper) CapValidatorSet(
 	ctx sdk.Context,
 	powerShapingParameters types.PowerShapingParameters,
-	priorityValidators []types.ConsensusValidator,
-	nonPriorityValidators []types.ConsensusValidator,
+	validators []types.ConsensusValidator,
 ) []types.ConsensusValidator {
 	if powerShapingParameters.Top_N > 0 {
 		// is a no-op if the chain is a Top N chain
-		return append(priorityValidators, nonPriorityValidators...)
+		return validators
 	}
 
 	validatorSetCap := powerShapingParameters.ValidatorSetCap
-	totalValidators := len(priorityValidators) + len(nonPriorityValidators)
-	if validatorSetCap == 0 || int(validatorSetCap) >= totalValidators {
-		return append(priorityValidators, nonPriorityValidators...)
-	}
-
-	resultValidators := make([]types.ConsensusValidator, 0, validatorSetCap)
-
-	// Add priority validators first
-	for i := 0; i < len(priorityValidators) && len(resultValidators) < int(validatorSetCap); i++ {
-		resultValidators = append(resultValidators, priorityValidators[i])
-	}
-
-	// Add remaining validators up to the cap
-	for i := 0; i < len(nonPriorityValidators) && len(resultValidators) < int(validatorSetCap); i++ {
-		resultValidators = append(resultValidators, nonPriorityValidators[i])
+	if validatorSetCap != 0 && int(validatorSetCap) < len(validators) {
+		return validators[:int(validatorSetCap)]
+	} else {
+		return validators
 	}
-
-	return resultValidators
 }
 
 // CapValidatorsPower caps the power of the validators on chain with `consumerId` and returns an updated slice of validators
@@ -641,34 +626,6 @@ func (k Keeper) UpdatePrioritylist(ctx sdk.Context, consumerId string, priorityl
 	}
 }
 
-// // FilterAndSortPriorityList filters the priority list to include only validators that can validate the chain
-// // by ensuring they are present in the validators list.
-// // It then sorts the filtered list of validators in descending order based on their power.
-// func (k Keeper) FilterAndSortPriorityList(priorityList []string, validators []types.ConsensusValidator) []types.ConsensusValidator {
-// 	validatorMap := make(map[string]types.ConsensusValidator)
-// 	for _, v := range validators {
-// 		validatorMap[string(v.ProviderConsAddr)] = v
-// 	}
-
-// 	filteredPriority := make([]types.ConsensusValidator, 0)
-// 	addedAddresses := make(map[string]bool)
-
-// 	for _, address := range priorityList {
-// 		if validator, exists := validatorMap[address]; exists {
-// 			if !addedAddresses[address] {
-// 				filteredPriority = append(filteredPriority, validator)
-// 				addedAddresses[address] = true
-// 			}
-// 		}
-// 	}
-
-// 	sort.Slice(filteredPriority, func(i, j int) bool {
-// 		return filteredPriority[i].Power > filteredPriority[j].Power
-// 	})
-
-// 	return filteredPriority
-// }
-
 // FilterAndSortPriorityList filters the priority list to include only validators that can validate the chain
 // and splits the validators into priority and non-priority sets.
 func (k Keeper) FilterAndSortPriorityList(ctx sdk.Context, priorityList []string, nextValidators []types.ConsensusValidator) ([]types.ConsensusValidator, []types.ConsensusValidator) {
@@ -683,18 +640,23 @@ func (k Keeper) FilterAndSortPriorityList(ctx sdk.Context, priorityList []string
 
 	// Form priorityValidators
 	for _, address := range priorityList {
-		if validator, exists := validatorMap[address]; exists {
-			if !addedAddresses[address] {
-				priorityValidators = append(priorityValidators, validator)
-				addedAddresses[address] = true
-				delete(validatorMap, address)
+		for _, validator := range nextValidators {
+			if string(validator.ProviderConsAddr) == address {
+				if !addedAddresses[address] {
+					priorityValidators = append(priorityValidators, validator)
+					addedAddresses[address] = true
+				}
+				break
 			}
 		}
 	}
 
 	// Add remaining validators to nonPriorityValidators
-	for _, validator := range validatorMap {
-		nonPriorityValidators = append(nonPriorityValidators, validator)
+	for _, validator := range nextValidators {
+		address := string(validator.ProviderConsAddr)
+		if !addedAddresses[address] {
+			nonPriorityValidators = append(nonPriorityValidators, validator)
+		}
 	}
 
 	sort.Slice(priorityValidators, func(i, j int) bool {

diff --git a/x/ccv/provider/keeper/power_shaping_test.go b/x/ccv/provider/keeper/power_shaping_test.go
@@ -246,7 +246,7 @@ func TestCanValidateChain(t *testing.T) {
 	require.False(t, canValidateChain)
 }
 
-func TestCapValidatorSetAndFilterAndSortPriorityList(t *testing.T) {
+func TestCapValidatorSet(t *testing.T) {
 	providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
 	defer ctrl.Finish()
 
@@ -260,7 +260,7 @@ func TestCapValidatorSetAndFilterAndSortPriorityList(t *testing.T) {
 	powerShapingParameters, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, CONSUMER_ID)
 	require.Error(t, err)
 	priorityValidators, nonPriorityValidators := providerKeeper.FilterAndSortPriorityList(ctx, powerShapingParameters.Prioritylist, validators)
-	consumerValidators := providerKeeper.CapValidatorSet(ctx, powerShapingParameters, priorityValidators, nonPriorityValidators)
+	consumerValidators := providerKeeper.CapValidatorSet(ctx, powerShapingParameters, append(priorityValidators, nonPriorityValidators...))
 	require.Equal(t, []providertypes.ConsensusValidator{validatorD, validatorC, validatorB, validatorA}, consumerValidators)
 
 	testCases := []struct {
@@ -329,96 +329,8 @@ func TestCapValidatorSetAndFilterAndSortPriorityList(t *testing.T) {
 				require.Equal(t, tc.powerShapingParameters, powerShapingParameters)
 			}
 
-			priorityValidators, nonPriorityValidators := providerKeeper.FilterAndSortPriorityList(ctx, powerShapingParameters.Prioritylist, validators)
-			consumerValidators := providerKeeper.CapValidatorSet(ctx, powerShapingParameters, priorityValidators, nonPriorityValidators)
-			require.Equal(t, tc.expectedValidators, consumerValidators)
-		})
-	}
-}
-
-func TestCapValidatorSet(t *testing.T) {
-	providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
-	defer ctrl.Finish()
-
-	validatorA := providertypes.ConsensusValidator{ProviderConsAddr: []byte("providerConsAddrA"), Power: 1, PublicKey: &crypto.PublicKey{}}
-	validatorB := providertypes.ConsensusValidator{ProviderConsAddr: []byte("providerConsAddrB"), Power: 2, PublicKey: &crypto.PublicKey{}}
-	validatorC := providertypes.ConsensusValidator{ProviderConsAddr: []byte("providerConsAddrC"), Power: 3, PublicKey: &crypto.PublicKey{}}
-	validatorD := providertypes.ConsensusValidator{ProviderConsAddr: []byte("providerConsAddrD"), Power: 4, PublicKey: &crypto.PublicKey{}}
-
-	validators := []providertypes.ConsensusValidator{validatorA, validatorB, validatorC, validatorD}
-
-	powerShapingParameters, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, CONSUMER_ID)
-	require.Error(t, err)
-	consumerValidators := providerKeeper.CapValidatorSet(ctx, powerShapingParameters, []providertypes.ConsensusValidator{}, validators)
-	require.Equal(t, validators, consumerValidators)
-
-	testCases := []struct {
-		name                   string
-		powerShapingParameters providertypes.PowerShapingParameters
-		priorityValidators     []providertypes.ConsensusValidator
-		nonPriorityValidators  []providertypes.ConsensusValidator
-		expectedValidators     []providertypes.ConsensusValidator
-	}{
-		{
-			name:                   "ValidatorSetCap = 0 (no capping)",
-			powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 0},
-			priorityValidators:     []providertypes.ConsensusValidator{validatorB, validatorA},
-			nonPriorityValidators:  []providertypes.ConsensusValidator{validatorD, validatorC},
-			expectedValidators:     []providertypes.ConsensusValidator{validatorB, validatorA, validatorD, validatorC},
-		},
-		{
-			name:                   "ValidatorSetCap > total validators (no capping)",
-			powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 100},
-			priorityValidators:     []providertypes.ConsensusValidator{validatorB, validatorA},
-			nonPriorityValidators:  []providertypes.ConsensusValidator{validatorD, validatorC},
-			expectedValidators:     []providertypes.ConsensusValidator{validatorB, validatorA, validatorD, validatorC},
-		},
-		{
-			name:                   "ValidatorSetCap = 1 (capping to highest priority)",
-			powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 1},
-			priorityValidators:     []providertypes.ConsensusValidator{validatorB, validatorA},
-			nonPriorityValidators:  []providertypes.ConsensusValidator{validatorD, validatorC},
-			expectedValidators:     []providertypes.ConsensusValidator{validatorB},
-		},
-		{
-			name:                   "ValidatorSetCap = 2 (capping to two highest priority)",
-			powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 2},
-			priorityValidators:     []providertypes.ConsensusValidator{validatorB, validatorA},
-			nonPriorityValidators:  []providertypes.ConsensusValidator{validatorD, validatorC},
-			expectedValidators:     []providertypes.ConsensusValidator{validatorB, validatorA},
-		},
-		{
-			name:                   "ValidatorSetCap = 3 (capping to all priority and one non-priority)",
-			powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 3},
-			priorityValidators:     []providertypes.ConsensusValidator{validatorB, validatorA},
-			nonPriorityValidators:  []providertypes.ConsensusValidator{validatorD, validatorC},
-			expectedValidators:     []providertypes.ConsensusValidator{validatorB, validatorA, validatorD},
-		},
-		{
-			name:                   "ValidatorSetCap = 2, with only non-priority validators",
-			powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 2},
-			priorityValidators:     []providertypes.ConsensusValidator{},
-			nonPriorityValidators:  []providertypes.ConsensusValidator{validatorD, validatorC, validatorB, validatorA},
-			expectedValidators:     []providertypes.ConsensusValidator{validatorD, validatorC},
-		},
-		{
-			name:                   "ValidatorSetCap = 3, with partial priority list",
-			powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 3},
-			priorityValidators:     []providertypes.ConsensusValidator{validatorA},
-			nonPriorityValidators:  []providertypes.ConsensusValidator{validatorD, validatorC, validatorB},
-			expectedValidators:     []providertypes.ConsensusValidator{validatorA, validatorD, validatorC},
-		},
-	}
-
-	for _, tc := range testCases {
-		t.Run(tc.name, func(t *testing.T) {
-			err := providerKeeper.SetConsumerPowerShapingParameters(ctx, CONSUMER_ID, tc.powerShapingParameters)
-			require.NoError(t, err)
-
-			powerShapingParameters, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, CONSUMER_ID)
-			require.NoError(t, err)
-
-			consumerValidators := providerKeeper.CapValidatorSet(ctx, powerShapingParameters, tc.priorityValidators, tc.nonPriorityValidators)
+			priorityValidators, nonPriorityValidators := providerKeeper.FilterAndSortPriorityList(ctx, tc.powerShapingParameters.Prioritylist, validators)
+			consumerValidators := providerKeeper.CapValidatorSet(ctx, tc.powerShapingParameters, append(priorityValidators, nonPriorityValidators...))
 			require.Equal(t, tc.expectedValidators, consumerValidators)
 		})
 	}
@@ -1167,7 +1079,7 @@ func TestFilterAndSortPriorityList(t *testing.T) {
 	// Create test validators
 	validator1 := providertypes.ConsensusValidator{
 		ProviderConsAddr: []byte("providerConsAddr1"),
-		Power:            100,
+		Power:            300,
 		PublicKey:        &crypto.PublicKey{},
 	}
 	validator2 := providertypes.ConsensusValidator{
@@ -1198,30 +1110,30 @@ func TestFilterAndSortPriorityList(t *testing.T) {
 			name:                "Empty priority list",
 			priorityList:        []string{},
 			expectedPriority:    []providertypes.ConsensusValidator{},
-			expectedNonPriority: []providertypes.ConsensusValidator{validator2, validator3, validator1, validator4},
+			expectedNonPriority: []providertypes.ConsensusValidator{validator1, validator2, validator3, validator4},
 		},
 		{
 			name:                "Priority list with non-existent addresses",
 			priorityList:        []string{"providerConsAddr5", "providerConsAddr6"},
 			expectedPriority:    []providertypes.ConsensusValidator{},
-			expectedNonPriority: []providertypes.ConsensusValidator{validator2, validator3, validator1, validator4},
+			expectedNonPriority: []providertypes.ConsensusValidator{validator1, validator2, validator3, validator4},
 		},
 		{
 			name:                "Priority list with some existing addresses",
 			priorityList:        []string{"providerConsAddr2", "providerConsAddr5", "providerConsAddr4"},
 			expectedPriority:    []providertypes.ConsensusValidator{validator2, validator4},
-			expectedNonPriority: []providertypes.ConsensusValidator{validator3, validator1},
+			expectedNonPriority: []providertypes.ConsensusValidator{validator1, validator3},
 		},
 		{
-			name:                "Priority list with all existing addresses in different order",
+			name:                "Priority list with all existing addresses",
 			priorityList:        []string{"providerConsAddr4", "providerConsAddr1", "providerConsAddr3", "providerConsAddr2"},
-			expectedPriority:    []providertypes.ConsensusValidator{validator2, validator3, validator1, validator4},
+			expectedPriority:    []providertypes.ConsensusValidator{validator1, validator2, validator3, validator4},
 			expectedNonPriority: []providertypes.ConsensusValidator{},
 		},
 		{
 			name:                "Priority list with duplicate addresses",
 			priorityList:        []string{"providerConsAddr1", "providerConsAddr2", "providerConsAddr1", "providerConsAddr3"},
-			expectedPriority:    []providertypes.ConsensusValidator{validator2, validator3, validator1},
+			expectedPriority:    []providertypes.ConsensusValidator{validator1, validator2, validator3},
 			expectedNonPriority: []providertypes.ConsensusValidator{validator4},
 		},
 	}

diff --git a/x/ccv/provider/keeper/validator_set_update.go b/x/ccv/provider/keeper/validator_set_update.go
@@ -235,7 +235,7 @@ func (k Keeper) ComputeNextValidators(
 
 	priorityValidators, nonPriorityValidators := k.FilterAndSortPriorityList(ctx, powerShapingParameters.Prioritylist, nextValidators)
 
-	nextValidators = k.CapValidatorSet(ctx, powerShapingParameters, priorityValidators, nonPriorityValidators)
+	nextValidators = k.CapValidatorSet(ctx, powerShapingParameters, append(priorityValidators, nonPriorityValidators...))
 
 	nextValidators = k.CapValidatorsPower(ctx, powerShapingParameters.ValidatorsPowerCap, nextValidators)