Skip to content

Commit

Permalink
Fixed bug for priority list
Browse files Browse the repository at this point in the history
  • Loading branch information
@kirdatatjana
kirdatatjana committed Oct 23, 2024
1 parent 04e9c57 commit f866876
Show file tree
Hide file tree
Showing 3 changed files with 31 additions and 120 deletions.
17 changes: 8 additions & 9 deletions x/ccv/provider/keeper/power_shaping.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
stakingtypes "github.com/cosmos/cosmos-sdk/x/staking/types"

"github.com/cosmos/interchain-security/v6/x/ccv/provider/types"
providertypes "github.com/cosmos/interchain-security/v6/x/ccv/provider/types"
ccvtypes "github.com/cosmos/interchain-security/v6/x/ccv/types"
)

Expand Down Expand Up @@ -628,7 +629,7 @@ func (k Keeper) UpdatePrioritylist(ctx sdk.Context, consumerId string, priorityl

// 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) {
func (k Keeper) FilterAndSortPriorityList(ctx sdk.Context, consumerId string, nextValidators []types.ConsensusValidator) ([]types.ConsensusValidator, []types.ConsensusValidator) {
validatorMap := make(map[string]types.ConsensusValidator)
for _, v := range nextValidators {
validatorMap[string(v.ProviderConsAddr)] = v
Expand All @@ -639,14 +640,12 @@ func (k Keeper) FilterAndSortPriorityList(ctx sdk.Context, priorityList []string
addedAddresses := make(map[string]bool)

// Form priorityValidators
for _, address := range priorityList {
for _, validator := range nextValidators {
if string(validator.ProviderConsAddr) == address {
if !addedAddresses[address] {
priorityValidators = append(priorityValidators, validator)
addedAddresses[address] = true
}
break
for _, validator := range nextValidators {
addr := providertypes.NewProviderConsAddress(validator.ProviderConsAddr)
if k.IsPrioritylisted(ctx, consumerId, addr) {
if !addedAddresses[string(validator.ProviderConsAddr)] {
priorityValidators = append(priorityValidators, validator)
addedAddresses[string(validator.ProviderConsAddr)] = true
}
}
}
Expand Down
132 changes: 22 additions & 110 deletions x/ccv/provider/keeper/power_shaping_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -250,16 +250,21 @@ 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{}}
consAddrA, _ := sdk.ConsAddressFromBech32("cosmosvalcons1ezyrq65s3gshhx5585w6mpusq3xsj3ayzf4uv6")
consAddrB, _ := sdk.ConsAddressFromBech32("cosmosvalcons1nx7n5uh0ztxsynn4sje6eyq2ud6rc6klc96w39")
consAddrC, _ := sdk.ConsAddressFromBech32("cosmosvalcons1qmq08eruchr5sf5s3rwz7djpr5a25f7xw4mceq")
consAddrD, _ := sdk.ConsAddressFromBech32("cosmosvalcons1kswr5sq599365kcjmhgufevfps9njf43e4lwdk")

validatorA := providertypes.ConsensusValidator{ProviderConsAddr: consAddrA, Power: 1, PublicKey: &crypto.PublicKey{}}
validatorB := providertypes.ConsensusValidator{ProviderConsAddr: consAddrB, Power: 2, PublicKey: &crypto.PublicKey{}}
validatorC := providertypes.ConsensusValidator{ProviderConsAddr: consAddrC, Power: 3, PublicKey: &crypto.PublicKey{}}
validatorD := providertypes.ConsensusValidator{ProviderConsAddr: consAddrD, 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)
priorityValidators, nonPriorityValidators := providerKeeper.FilterAndSortPriorityList(ctx, powerShapingParameters.Prioritylist, validators)
priorityValidators, nonPriorityValidators := providerKeeper.FilterAndSortPriorityList(ctx, CONSUMER_ID, validators)
consumerValidators := providerKeeper.CapValidatorSet(ctx, powerShapingParameters, append(priorityValidators, nonPriorityValidators...))
require.Equal(t, []providertypes.ConsensusValidator{validatorD, validatorC, validatorB, validatorA}, consumerValidators)

Expand Down Expand Up @@ -296,22 +301,22 @@ func TestCapValidatorSet(t *testing.T) {
},
{
name: "ValidatorSetCap = 2, with priority list",
powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 2, Prioritylist: []string{string(validatorA.ProviderConsAddr), string(validatorB.ProviderConsAddr)}},
powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 2, Prioritylist: []string{"cosmosvalcons1ezyrq65s3gshhx5585w6mpusq3xsj3ayzf4uv6", "cosmosvalcons1nx7n5uh0ztxsynn4sje6eyq2ud6rc6klc96w39"}},
expectedValidators: []providertypes.ConsensusValidator{validatorB, validatorA},
},
{
name: "ValidatorSetCap = 3, with partial priority list",
powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 3, Prioritylist: []string{string(validatorA.ProviderConsAddr)}},
powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 3, Prioritylist: []string{"cosmosvalcons1ezyrq65s3gshhx5585w6mpusq3xsj3ayzf4uv6"}},
expectedValidators: []providertypes.ConsensusValidator{validatorA, validatorD, validatorC},
},
{
name: "All validators in priority list",
powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 4, Prioritylist: []string{string(validatorC.ProviderConsAddr), string(validatorA.ProviderConsAddr), string(validatorD.ProviderConsAddr), string(validatorB.ProviderConsAddr)}},
powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 4, Prioritylist: []string{"cosmosvalcons1qmq08eruchr5sf5s3rwz7djpr5a25f7xw4mceq", "cosmosvalcons1ezyrq65s3gshhx5585w6mpusq3xsj3ayzf4uv6", "cosmosvalcons1kswr5sq599365kcjmhgufevfps9njf43e4lwdk", "cosmosvalcons1nx7n5uh0ztxsynn4sje6eyq2ud6rc6klc96w39"}},
expectedValidators: []providertypes.ConsensusValidator{validatorD, validatorC, validatorB, validatorA},
},
{
name: "ValidatorSetCap = 1 (capping to highest power, with priority list)",
powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 1, Prioritylist: []string{string(validatorA.ProviderConsAddr)}},
powerShapingParameters: providertypes.PowerShapingParameters{ValidatorSetCap: 1, Prioritylist: []string{"cosmosvalcons1ezyrq65s3gshhx5585w6mpusq3xsj3ayzf4uv6"}},
expectedValidators: []providertypes.ConsensusValidator{validatorA},
},
}
Expand All @@ -328,9 +333,8 @@ func TestCapValidatorSet(t *testing.T) {
require.NoError(t, err)
require.Equal(t, tc.powerShapingParameters, powerShapingParameters)
}

priorityValidators, nonPriorityValidators := providerKeeper.FilterAndSortPriorityList(ctx, tc.powerShapingParameters.Prioritylist, validators)
consumerValidators := providerKeeper.CapValidatorSet(ctx, tc.powerShapingParameters, append(priorityValidators, nonPriorityValidators...))
priorityValidators, nonPriorityValidators := providerKeeper.FilterAndSortPriorityList(ctx, CONSUMER_ID, validators)
consumerValidators := providerKeeper.CapValidatorSet(ctx, powerShapingParameters, append(priorityValidators, nonPriorityValidators...))
require.Equal(t, tc.expectedValidators, consumerValidators)
})
}
Expand Down Expand Up @@ -767,13 +771,16 @@ func TestConsumerPowerShapingParameters(t *testing.T) {
sortProviderConsAddr(expectedAllowlist)
expectedDenylist := []providertypes.ProviderConsAddress{providerConsAddr[2], providerConsAddr[3]}
sortProviderConsAddr(expectedDenylist)
expectedPrioritylist := []providertypes.ProviderConsAddress{providerConsAddr[1]}
sortProviderConsAddr(expectedPrioritylist)
err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, expectedPowerShapingParameters)
require.NoError(t, err)
actualPowerShapingParameters, err := providerKeeper.GetConsumerPowerShapingParameters(ctx, consumerId)
require.NoError(t, err)
require.Equal(t, expectedPowerShapingParameters, actualPowerShapingParameters)
require.Equal(t, expectedAllowlist, providerKeeper.GetAllowList(ctx, consumerId))
require.Equal(t, expectedDenylist, providerKeeper.GetDenyList(ctx, consumerId))
require.Equal(t, expectedPrioritylist, providerKeeper.GetPriorityList(ctx, consumerId))

// assert that overwriting the current initialization record works
expectedPowerShapingParameters = providertypes.PowerShapingParameters{
Expand All @@ -790,13 +797,16 @@ func TestConsumerPowerShapingParameters(t *testing.T) {
sortProviderConsAddr(expectedAllowlist)
expectedDenylist = []providertypes.ProviderConsAddress{providerConsAddr[2], providerConsAddr[3]}
sortProviderConsAddr(expectedDenylist)
expectedPrioritylist = []providertypes.ProviderConsAddress{providerConsAddr[4], providerConsAddr[5]}
sortProviderConsAddr(expectedAllowlist)
err = providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerId, expectedPowerShapingParameters)
require.NoError(t, err)
actualPowerShapingParameters, err = providerKeeper.GetConsumerPowerShapingParameters(ctx, consumerId)
require.NoError(t, err)
require.Equal(t, expectedPowerShapingParameters, actualPowerShapingParameters)
require.Equal(t, expectedAllowlist, providerKeeper.GetAllowList(ctx, consumerId))
require.Equal(t, expectedDenylist, providerKeeper.GetDenyList(ctx, consumerId))
require.Equal(t, expectedPrioritylist, providerKeeper.GetPriorityList(ctx, consumerId))
}

// TestAllowlist tests the `SetAllowlist`, `IsAllowlisted`, `DeleteAllowlist`, and `IsAllowlistEmpty` methods
Expand Down Expand Up @@ -1071,101 +1081,3 @@ func TestUpdatePrioritylist(t *testing.T) {
}
require.Equal(t, expectedPrioritylist, providerKeeper.GetPriorityList(ctx, consumerId))
}

func TestFilterAndSortPriorityList(t *testing.T) {
providerKeeper, ctx, ctrl, _ := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
defer ctrl.Finish()

// Create test validators
validator1 := providertypes.ConsensusValidator{
ProviderConsAddr: []byte("providerConsAddr1"),
Power: 300,
PublicKey: &crypto.PublicKey{},
}
validator2 := providertypes.ConsensusValidator{
ProviderConsAddr: []byte("providerConsAddr2"),
Power: 200,
PublicKey: &crypto.PublicKey{},
}
validator3 := providertypes.ConsensusValidator{
ProviderConsAddr: []byte("providerConsAddr3"),
Power: 150,
PublicKey: &crypto.PublicKey{},
}
validator4 := providertypes.ConsensusValidator{
ProviderConsAddr: []byte("providerConsAddr4"),
Power: 50,
PublicKey: &crypto.PublicKey{},
}

validators := []providertypes.ConsensusValidator{validator1, validator2, validator3, validator4}

testCases := []struct {
name string
priorityList []string
expectedPriority []providertypes.ConsensusValidator
expectedNonPriority []providertypes.ConsensusValidator
}{
{
name: "Empty priority list",
priorityList: []string{},
expectedPriority: []providertypes.ConsensusValidator{},
expectedNonPriority: []providertypes.ConsensusValidator{validator1, validator2, validator3, validator4},
},
{
name: "Priority list with non-existent addresses",
priorityList: []string{"providerConsAddr5", "providerConsAddr6"},
expectedPriority: []providertypes.ConsensusValidator{},
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{validator1, validator3},
},
{
name: "Priority list with all existing addresses",
priorityList: []string{"providerConsAddr4", "providerConsAddr1", "providerConsAddr3", "providerConsAddr2"},
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{validator1, validator2, validator3},
expectedNonPriority: []providertypes.ConsensusValidator{validator4},
},
}

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
priorityResult, nonPriorityResult := providerKeeper.FilterAndSortPriorityList(ctx, tc.priorityList, validators)

// 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 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)
})
}
}

// Checks 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")
}
}
2 changes: 1 addition & 1 deletion x/ccv/provider/keeper/validator_set_update.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -233,7 +233,7 @@ func (k Keeper) ComputeNextValidators(
return []types.ConsensusValidator{}, err
}

priorityValidators, nonPriorityValidators := k.FilterAndSortPriorityList(ctx, powerShapingParameters.Prioritylist, nextValidators)
priorityValidators, nonPriorityValidators := k.FilterAndSortPriorityList(ctx, consumerId, nextValidators)

nextValidators = k.CapValidatorSet(ctx, powerShapingParameters, append(priorityValidators, nonPriorityValidators...))

Expand Down

0 comments on commit f866876

Please sign in to comment.