diff --git a/x/ccv/provider/keeper/power_shaping.go b/x/ccv/provider/keeper/power_shaping.go index 7bf4674b18..d09fd6f426 100644 --- 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 index 0fb6a03092..a9b95ec113 100644 --- 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 index 5f43101de1..eb46332749 100644 --- 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)