From e40a8c63de01152007c5d5deb4b24800c65b9b52 Mon Sep 17 00:00:00 2001 From: kirdatatjana Date: Mon, 21 Oct 2024 12:52:29 +0200 Subject: [PATCH] Refactored code logic --- x/ccv/provider/keeper/power_shaping.go | 91 +++++---- x/ccv/provider/keeper/power_shaping_test.go | 180 ++++++++++++++---- x/ccv/provider/keeper/validator_set_update.go | 4 +- 3 files changed, 206 insertions(+), 69 deletions(-) diff --git a/x/ccv/provider/keeper/power_shaping.go b/x/ccv/provider/keeper/power_shaping.go index 36ccb884b2..7bf4674b18 100644 --- a/x/ccv/provider/keeper/power_shaping.go +++ b/x/ccv/provider/keeper/power_shaping.go @@ -88,48 +88,34 @@ 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 up to the cap. +// It prioritizes validators from the priority list and then adds remaining validators. func (k Keeper) CapValidatorSet( ctx sdk.Context, powerShapingParameters types.PowerShapingParameters, - validators []types.ConsensusValidator, + priorityValidators []types.ConsensusValidator, + nonPriorityValidators []types.ConsensusValidator, ) []types.ConsensusValidator { if powerShapingParameters.Top_N > 0 { // is a no-op if the chain is a Top N chain - return validators + return append(priorityValidators, nonPriorityValidators...) } validatorSetCap := powerShapingParameters.ValidatorSetCap - if validatorSetCap == 0 || int(validatorSetCap) >= len(validators) { - return validators + totalValidators := len(priorityValidators) + len(nonPriorityValidators) + if validatorSetCap == 0 || int(validatorSetCap) >= totalValidators { + return append(priorityValidators, nonPriorityValidators...) } - // Filter and sort the priority list by power - priorityValidators := k.FilterAndSortPriorityList(powerShapingParameters.Prioritylist, validators) - - // Create a map to keep track of added validators - addedValidators := make(map[string]bool) - resultValidators := make([]types.ConsensusValidator, 0, validatorSetCap) - // Add priority validators first. - // For each added validator, it marks it as added in the addedValidators map to avoid duplicates later. + // Add priority validators first for i := 0; i < len(priorityValidators) && len(resultValidators) < int(validatorSetCap); i++ { resultValidators = append(resultValidators, priorityValidators[i]) - addedValidators[string(priorityValidators[i].ProviderConsAddr)] = true } - // Sort validators by power - sort.Slice(validators, func(i, j int) bool { - return validators[i].Power > validators[j].Power - }) - // Add remaining validators up to the cap - for i := 0; i < len(validators) && len(resultValidators) < int(validatorSetCap); i++ { - if !addedValidators[string(validators[i].ProviderConsAddr)] { - resultValidators = append(resultValidators, validators[i]) - addedValidators[string(validators[i].ProviderConsAddr)] = true - } + for i := 0; i < len(nonPriorityValidators) && len(resultValidators) < int(validatorSetCap); i++ { + resultValidators = append(resultValidators, nonPriorityValidators[i]) } return resultValidators @@ -655,30 +641,69 @@ 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 -// 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 { +// 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) { validatorMap := make(map[string]types.ConsensusValidator) - for _, v := range validators { + for _, v := range nextValidators { validatorMap[string(v.ProviderConsAddr)] = v } - filteredPriority := make([]types.ConsensusValidator, 0) + priorityValidators := make([]types.ConsensusValidator, 0) + nonPriorityValidators := make([]types.ConsensusValidator, 0) addedAddresses := make(map[string]bool) + // Form priorityValidators for _, address := range priorityList { if validator, exists := validatorMap[address]; exists { if !addedAddresses[address] { - filteredPriority = append(filteredPriority, validator) + priorityValidators = append(priorityValidators, validator) addedAddresses[address] = true + delete(validatorMap, address) } } } - sort.Slice(filteredPriority, func(i, j int) bool { - return filteredPriority[i].Power > filteredPriority[j].Power + // Add remaining validators to nonPriorityValidators + for _, validator := range validatorMap { + nonPriorityValidators = append(nonPriorityValidators, validator) + } + + sort.Slice(priorityValidators, func(i, j int) bool { + return priorityValidators[i].Power > priorityValidators[j].Power + }) + + sort.Slice(nonPriorityValidators, func(i, j int) bool { + return nonPriorityValidators[i].Power > nonPriorityValidators[j].Power }) - return filteredPriority + return priorityValidators, nonPriorityValidators } diff --git a/x/ccv/provider/keeper/power_shaping_test.go b/x/ccv/provider/keeper/power_shaping_test.go index 727d285a31..0fb6a03092 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 TestCapValidatorSet(t *testing.T) { +func TestCapValidatorSetAndFilterAndSortPriorityList(t *testing.T) { providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() @@ -256,10 +256,12 @@ func TestCapValidatorSet(t *testing.T) { validatorD := providertypes.ConsensusValidator{ProviderConsAddr: []byte("providerConsAddrD"), Power: 4, PublicKey: &crypto.PublicKey{}} validators := []providertypes.ConsensusValidator{validatorA, validatorB, validatorC, validatorD} + // Initial error check powerShapingParameters, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, CONSUMER_ID) require.Error(t, err) - consumerValidators := providerKeeper.CapValidatorSet(ctx, powerShapingParameters, validators) - require.Equal(t, validators, consumerValidators) + priorityValidators, nonPriorityValidators := providerKeeper.FilterAndSortPriorityList(ctx, powerShapingParameters.Prioritylist, validators) + consumerValidators := providerKeeper.CapValidatorSet(ctx, powerShapingParameters, priorityValidators, nonPriorityValidators) + require.Equal(t, []providertypes.ConsensusValidator{validatorD, validatorC, validatorB, validatorA}, consumerValidators) testCases := []struct { name string @@ -270,12 +272,12 @@ func TestCapValidatorSet(t *testing.T) { { name: "ValidatorSetCap = 0 (no capping)", powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 0}, - expectedValidators: []providertypes.ConsensusValidator{validatorA, validatorB, validatorC, validatorD}, + expectedValidators: []providertypes.ConsensusValidator{validatorD, validatorC, validatorB, validatorA}, }, { name: "ValidatorSetCap > len(validators) (no capping)", powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 100}, - expectedValidators: []providertypes.ConsensusValidator{validatorA, validatorB, validatorC, validatorD}, + expectedValidators: []providertypes.ConsensusValidator{validatorD, validatorC, validatorB, validatorA}, }, { name: "ValidatorSetCap = 1 (capping to highest power, no priority list)", @@ -288,7 +290,7 @@ func TestCapValidatorSet(t *testing.T) { expectedValidators: []providertypes.ConsensusValidator{validatorD, validatorC}, }, { - name: "ValidatorSetCap = 3 (capping to two highest power, no priority list)", + name: "ValidatorSetCap = 3 (capping to three highest power, no priority list)", powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 3}, expectedValidators: []providertypes.ConsensusValidator{validatorD, validatorC, validatorB}, }, @@ -327,7 +329,96 @@ func TestCapValidatorSet(t *testing.T) { require.Equal(t, tc.powerShapingParameters, powerShapingParameters) } - consumerValidators := providerKeeper.CapValidatorSet(ctx, powerShapingParameters, validators) + 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) require.Equal(t, tc.expectedValidators, consumerValidators) }) } @@ -1070,7 +1161,7 @@ func TestUpdatePrioritylist(t *testing.T) { } func TestFilterAndSortPriorityList(t *testing.T) { - providerKeeper, _, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) + providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t)) defer ctrl.Finish() // Create test validators @@ -1098,52 +1189,71 @@ func TestFilterAndSortPriorityList(t *testing.T) { validators := []providertypes.ConsensusValidator{validator1, validator2, validator3, validator4} testCases := []struct { - name string - priorityList []string - expected []providertypes.ConsensusValidator + name string + priorityList []string + expectedPriority []providertypes.ConsensusValidator + expectedNonPriority []providertypes.ConsensusValidator }{ { - name: "Empty priority list", - priorityList: []string{}, - expected: []providertypes.ConsensusValidator{}, + name: "Empty priority list", + priorityList: []string{}, + expectedPriority: []providertypes.ConsensusValidator{}, + expectedNonPriority: []providertypes.ConsensusValidator{validator2, validator3, validator1, validator4}, }, { - name: "Priority list with non-existent addresses", - priorityList: []string{"providerConsAddr5", "providerConsAddr6"}, - expected: []providertypes.ConsensusValidator{}, + name: "Priority list with non-existent addresses", + priorityList: []string{"providerConsAddr5", "providerConsAddr6"}, + expectedPriority: []providertypes.ConsensusValidator{}, + expectedNonPriority: []providertypes.ConsensusValidator{validator2, validator3, validator1, validator4}, }, { - name: "Priority list with some existing addresses", - priorityList: []string{"providerConsAddr2", "providerConsAddr5", "providerConsAddr4"}, - expected: []providertypes.ConsensusValidator{validator2, validator4}, + name: "Priority list with some existing addresses", + priorityList: []string{"providerConsAddr2", "providerConsAddr5", "providerConsAddr4"}, + expectedPriority: []providertypes.ConsensusValidator{validator2, validator4}, + expectedNonPriority: []providertypes.ConsensusValidator{validator3, validator1}, }, { - name: "Priority list with all existing addresses in different order", - priorityList: []string{"providerConsAddr4", "providerConsAddr1", "providerConsAddr3", "providerConsAddr2"}, - expected: []providertypes.ConsensusValidator{validator2, validator3, validator1, validator4}, + name: "Priority list with all existing addresses in different order", + priorityList: []string{"providerConsAddr4", "providerConsAddr1", "providerConsAddr3", "providerConsAddr2"}, + expectedPriority: []providertypes.ConsensusValidator{validator2, validator3, validator1, validator4}, + expectedNonPriority: []providertypes.ConsensusValidator{}, }, { - name: "Priority list with duplicate addresses", - priorityList: []string{"providerConsAddr1", "providerConsAddr2", "providerConsAddr1", "providerConsAddr3"}, - expected: []providertypes.ConsensusValidator{validator2, validator3, validator1}, + name: "Priority list with duplicate addresses", + priorityList: []string{"providerConsAddr1", "providerConsAddr2", "providerConsAddr1", "providerConsAddr3"}, + expectedPriority: []providertypes.ConsensusValidator{validator2, validator3, validator1}, + expectedNonPriority: []providertypes.ConsensusValidator{validator4}, }, } for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { - result := providerKeeper.FilterAndSortPriorityList(tc.priorityList, validators) - - require.Equal(t, len(tc.expected), len(result), "Length of result doesn't match expected") + priorityResult, nonPriorityResult := providerKeeper.FilterAndSortPriorityList(ctx, tc.priorityList, validators) - for i, v := range result { - require.Equal(t, tc.expected[i].ProviderConsAddr, v.ProviderConsAddr, "Validator address doesn't match expected at index %d", i) - require.Equal(t, tc.expected[i].Power, v.Power, "Validator power doesn't match expected at index %d", i) + // Check priority validators + require.Equal(t, len(tc.expectedPriority), len(priorityResult), "Length of priority result doesn't match expected") + for i, v := range priorityResult { + require.Equal(t, tc.expectedPriority[i].ProviderConsAddr, v.ProviderConsAddr, "Priority validator address doesn't match expected at index %d", i) + require.Equal(t, tc.expectedPriority[i].Power, v.Power, "Priority validator power doesn't match expected at index %d", i) } - // Check if the result is sorted by power in descending order - for i := 1; i < len(result); i++ { - require.GreaterOrEqual(t, result[i-1].Power, result[i].Power, "Result is not sorted by power in descending order") + // Check non-priority validators + require.Equal(t, len(tc.expectedNonPriority), len(nonPriorityResult), "Length of non-priority result doesn't match expected") + for i, v := range nonPriorityResult { + require.Equal(t, tc.expectedNonPriority[i].ProviderConsAddr, v.ProviderConsAddr, "Non-priority validator address doesn't match expected at index %d", i) + require.Equal(t, tc.expectedNonPriority[i].Power, v.Power, "Non-priority validator power doesn't match expected at index %d", i) } + + // Check if both results are sorted by power in descending order + checkSortedByPower(t, priorityResult) + checkSortedByPower(t, nonPriorityResult) }) } } + +// Helper function to check if validators are sorted by power in descending order +func checkSortedByPower(t *testing.T, validators []providertypes.ConsensusValidator) { + for i := 1; i < len(validators); i++ { + require.GreaterOrEqual(t, validators[i-1].Power, validators[i].Power, "Validators are not sorted by power in descending order") + } +} diff --git a/x/ccv/provider/keeper/validator_set_update.go b/x/ccv/provider/keeper/validator_set_update.go index d375d42d8f..5f43101de1 100644 --- a/x/ccv/provider/keeper/validator_set_update.go +++ b/x/ccv/provider/keeper/validator_set_update.go @@ -233,7 +233,9 @@ func (k Keeper) ComputeNextValidators( return []types.ConsensusValidator{}, err } - nextValidators = k.CapValidatorSet(ctx, powerShapingParameters, nextValidators) + priorityValidators, nonPriorityValidators := k.FilterAndSortPriorityList(ctx, powerShapingParameters.Prioritylist, nextValidators) + + nextValidators = k.CapValidatorSet(ctx, powerShapingParameters, priorityValidators, nonPriorityValidators) nextValidators = k.CapValidatorsPower(ctx, powerShapingParameters.ValidatorsPowerCap, nextValidators)