Skip to content

Commit

Permalink
feat: refactor consumer validator set computation (#2175)
Browse files Browse the repository at this point in the history 
* add UT

* nits

* address comments

* Update x/ccv/provider/keeper/partial_set_security.go

Co-authored-by: insumity <[email protected]>

* fix tests

---------

Co-authored-by: insumity <[email protected]>
  • Loading branch information
@sainoe @insumity
sainoe and insumity committed Aug 28, 2024
1 parent 25f6b82 commit 88a6375
Show file tree
Hide file tree
Showing 5 changed files with 177 additions and 40 deletions.
36 changes: 18 additions & 18 deletions x/ccv/provider/keeper/grpc_query.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -437,29 +437,29 @@ func (k Keeper) hasToValidate(
if err != nil {
return false, nil
}

minPowerToOptIn := int64(0)
// If the consumer is TopN compute the minimum power
if topN := k.GetTopN(ctx, consumerId); topN > 0 {
// in a Top-N chain, we automatically opt in all validators that belong to the top N
minPower, found := k.GetMinimumPowerInTopN(ctx, consumerId)
if found {
k.OptInTopNValidators(ctx, consumerId, activeValidators, minPower)
} else {
k.Logger(ctx).Error("did not find min power in top N for chain", "chain", consumerId)
// compute the minimum power to opt-in since the one in the state is stale
// Note that the effective min power will be computed at the end of the epoch
minPowerToOptIn, err = k.ComputeMinPowerInTopN(ctx, activeValidators, topN)
if err != nil {
return false, err
}
}

// if the validator is opted in and belongs to the validators of the next epoch, then if nothing changes
// if the validator belongs to the validators of the next epoch, then if nothing changes
// the validator would have to validate in the next epoch
if k.IsOptedIn(ctx, consumerId, provAddr) {
lastVals, err := k.GetLastBondedValidators(ctx)
if err != nil {
return false, err
}
nextValidators := k.ComputeNextValidators(ctx, consumerId, lastVals)
for _, v := range nextValidators {
consAddr := sdk.ConsAddress(v.ProviderConsAddr)
if provAddr.ToSdkConsAddr().Equals(consAddr) {
return true, nil
}
lastVals, err := k.GetLastBondedValidators(ctx)
if err != nil {
return false, err
}
nextValidators := k.ComputeNextValidators(ctx, consumerId, lastVals, minPowerToOptIn)
for _, v := range nextValidators {
consAddr := sdk.ConsAddress(v.ProviderConsAddr)
if provAddr.ToSdkConsAddr().Equals(consAddr) {
return true, nil
}
}

Expand Down
57 changes: 52 additions & 5 deletions x/ccv/provider/keeper/partial_set_security.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,6 @@ func (k Keeper) OptInTopNValidators(ctx sdk.Context, consumerId string, bondedVa

// if validator already exists it gets overwritten
k.SetOptedIn(ctx, consumerId, types.NewProviderConsAddress(consAddr))
k.SetOptedIn(ctx, consumerId, types.NewProviderConsAddress(consAddr))
} // else validators that do not belong to the top N validators but were opted in, remain opted in
}
}
Expand Down Expand Up @@ -322,9 +321,17 @@ func NoMoreThanPercentOfTheSum(validators []types.ConsensusValidator, percent ui

// CanValidateChain returns true if the validator `providerAddr` is opted-in to chain with `consumerId` and the allowlist
// and denylist do not prevent the validator from validating the chain.
func (k Keeper) CanValidateChain(ctx sdk.Context, consumerId string, providerAddr types.ProviderConsAddress) bool {
func (k Keeper) CanValidateChain(ctx sdk.Context, consumerId string, providerAddr types.ProviderConsAddress, minPowerToOptIn int64) bool {
// check if the validator is already opted-in
optedIn := k.IsOptedIn(ctx, consumerId, providerAddr)

// check if the validator is automatically opted-in for a topN chain
if !optedIn && k.GetTopN(ctx, consumerId) > 0 {
optedIn = k.HasMinPower(ctx, providerAddr, minPowerToOptIn)
}

// only consider opted-in validators
return k.IsOptedIn(ctx, consumerId, providerAddr) &&
return optedIn &&
// if an allowlist is declared, only consider allowlisted validators
(k.IsAllowlistEmpty(ctx, consumerId) ||
k.IsAllowlisted(ctx, consumerId, providerAddr)) &&
Expand Down Expand Up @@ -352,7 +359,7 @@ func (k Keeper) FulfillsMinStake(ctx sdk.Context, consumerId string, providerAdd
}

// ComputeNextValidators computes the validators for the upcoming epoch based on the currently `bondedValidators`.
func (k Keeper) ComputeNextValidators(ctx sdk.Context, consumerId string, bondedValidators []stakingtypes.Validator) []types.ConsensusValidator {
func (k Keeper) ComputeNextValidators(ctx sdk.Context, consumerId string, bondedValidators []stakingtypes.Validator, minPowerToOptIn int64) []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].GetBondedTokens().GT(bondedValidators[j].GetBondedTokens())
Expand All @@ -371,9 +378,49 @@ func (k Keeper) ComputeNextValidators(ctx sdk.Context, consumerId string, bonded

nextValidators := k.FilterValidators(ctx, consumerId, bondedValidators,
func(providerAddr types.ProviderConsAddress) bool {
return k.CanValidateChain(ctx, consumerId, providerAddr) && k.FulfillsMinStake(ctx, consumerId, providerAddr)
return k.CanValidateChain(ctx, consumerId, providerAddr, minPowerToOptIn) && k.FulfillsMinStake(ctx, consumerId, providerAddr)
})

nextValidators = k.CapValidatorSet(ctx, consumerId, nextValidators)
return k.CapValidatorsPower(ctx, consumerId, nextValidators)
}

// HasMinPower returns true if the `providerAddr` voting power is GTE than the given minimum power
func (k Keeper) HasMinPower(ctx sdk.Context, providerAddr types.ProviderConsAddress, minPower int64) bool {
val, err := k.stakingKeeper.GetValidatorByConsAddr(ctx, providerAddr.Address)
if err != nil {
k.Logger(ctx).Error(
"cannot get last validator power",
"provider address",
providerAddr,
"error",
err,
)
return false
}

valAddr, err := sdk.ValAddressFromBech32(val.GetOperator())
if err != nil {
k.Logger(ctx).Error(
"could not retrieve validator address",
"operator address",
val.GetOperator(),
"error",
err,
)
return false
}

power, err := k.stakingKeeper.GetLastValidatorPower(ctx, valAddr)
if err != nil {
k.Logger(ctx).Error("could not retrieve last power of validator address",
"operator address",
val.GetOperator(),
"error",
err,
)
return false
}

return power >= minPower
}
113 changes: 100 additions & 13 deletions x/ccv/provider/keeper/partial_set_security_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package keeper_test

import (
"bytes"
"fmt"
"sort"
"testing"

Expand Down Expand Up @@ -384,29 +385,45 @@ func TestCanValidateChain(t *testing.T) {
providerKeeper, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
defer ctrl.Finish()

validator := createStakingValidator(ctx, mocks, 0, 1, 1)
consumerID := "0"

validator := createStakingValidator(ctx, mocks, 0, 1, 1) // adds GetLastValidatorPower expectation to mocks
consAddr, _ := validator.GetConsAddr()
providerAddr := types.NewProviderConsAddress(consAddr)

// with no allowlist or denylist, the validator has to be opted in, in order to consider it
require.False(t, providerKeeper.CanValidateChain(ctx, "consumerId", providerAddr))
providerKeeper.SetOptedIn(ctx, "consumerId", types.NewProviderConsAddress(consAddr))
require.True(t, providerKeeper.CanValidateChain(ctx, "consumerId", providerAddr))
require.False(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 0))

// with TopN chains, the validator can be considered,
mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(gomock.Any(), providerAddr.Address).Return(validator, nil).Times(2)
providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerID, types.PowerShapingParameters{Top_N: 50})
// validator's power is LT the min power
require.False(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 2))
// validator's power is GTE the min power
require.True(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 1))

// when validator is opted-in it can validate regardless of its min power
providerKeeper.SetOptedIn(ctx, consumerID, types.NewProviderConsAddress(consAddr))
require.True(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 2))

// With OptIn chains, validator can validate only if it has already opted-in
providerKeeper.SetConsumerPowerShapingParameters(ctx, consumerID, types.PowerShapingParameters{Top_N: 0})
require.True(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 2))

// create an allow list but do not add the validator `providerAddr` to it
validatorA := createStakingValidator(ctx, mocks, 1, 1, 2)
consAddrA, _ := validatorA.GetConsAddr()
providerKeeper.SetAllowlist(ctx, "consumerId", types.NewProviderConsAddress(consAddrA))
require.False(t, providerKeeper.CanValidateChain(ctx, "consumerId", providerAddr))
providerKeeper.SetAllowlist(ctx, "consumerId", types.NewProviderConsAddress(consAddr))
require.True(t, providerKeeper.CanValidateChain(ctx, "consumerId", providerAddr))
providerKeeper.SetAllowlist(ctx, consumerID, types.NewProviderConsAddress(consAddrA))
require.False(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 1))
providerKeeper.SetAllowlist(ctx, consumerID, types.NewProviderConsAddress(consAddr))
require.True(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 1))

// create a denylist but do not add validator `providerAddr` to it
providerKeeper.SetDenylist(ctx, "consumerId", types.NewProviderConsAddress(consAddrA))
require.True(t, providerKeeper.CanValidateChain(ctx, "consumerId", providerAddr))
providerKeeper.SetDenylist(ctx, consumerID, types.NewProviderConsAddress(consAddrA))
require.True(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 1))
// add validator `providerAddr` to the denylist
providerKeeper.SetDenylist(ctx, "consumerId", types.NewProviderConsAddress(consAddr))
require.False(t, providerKeeper.CanValidateChain(ctx, "consumerId", providerAddr))
providerKeeper.SetDenylist(ctx, consumerID, types.NewProviderConsAddress(consAddr))
require.False(t, providerKeeper.CanValidateChain(ctx, consumerID, providerAddr, 1))
}

func TestCapValidatorSet(t *testing.T) {
Expand Down Expand Up @@ -820,7 +837,7 @@ func TestIfInactiveValsDisallowedProperty(t *testing.T) {
providerKeeper.SetParams(ctx, params)

// Compute the next validators
nextVals := providerKeeper.ComputeNextValidators(ctx, "consumerId", vals)
nextVals := providerKeeper.ComputeNextValidators(ctx, "consumerId", vals, 0)

// Check that the length of nextVals is at most maxProviderConsensusVals
require.LessOrEqual(r, len(nextVals), int(maxProviderConsensusVals), "The length of nextVals should be at most maxProviderConsensusVals")
Expand All @@ -843,3 +860,73 @@ func TestIfInactiveValsDisallowedProperty(t *testing.T) {
}
})
}

func TestHasMinPower(t *testing.T) {
pk, ctx, ctrl, mocks := testkeeper.GetProviderKeeperAndCtx(t, testkeeper.NewInMemKeeperParams(t))
defer ctrl.Finish()

providerConsPubKey := ed25519.GenPrivKeyFromSecret([]byte{1}).PubKey()
consAddr := sdk.ConsAddress(providerConsPubKey.Address())
providerAddr := types.NewProviderConsAddress(consAddr)

testCases := []struct {
name string
minPower uint64
expectation func(sdk.ConsAddress, testkeeper.MockedKeepers)
hasMinPower bool
}{
{
name: "cannot find validator by cons address",
expectation: func(sdk.ConsAddress, testkeeper.MockedKeepers) {
mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(gomock.Any(), consAddr).
Return(stakingtypes.Validator{}, fmt.Errorf("validator not found")).Times(1)
},
hasMinPower: false,
}, {
name: "cannot convert validator operator address",
expectation: func(consAddr sdk.ConsAddress, mocks testkeeper.MockedKeepers) {
mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(gomock.Any(), consAddr).Return(stakingtypes.Validator{OperatorAddress: "xxxx"}, nil).Times(1)
},
hasMinPower: false,
}, {
name: "cannot find last validator power",
expectation: func(consAddr sdk.ConsAddress, mocks testkeeper.MockedKeepers) {
valAddr := sdk.ValAddress(providerAddr.Address.Bytes())
mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(gomock.Any(), consAddr).
Return(stakingtypes.Validator{OperatorAddress: valAddr.String()}, nil)
mocks.MockStakingKeeper.EXPECT().GetLastValidatorPower(gomock.Any(), valAddr).
Return(int64(0), fmt.Errorf("last power not found")).Times(1)
},
hasMinPower: false,
}, {
name: "validator power is LT min power",
expectation: func(consAddr sdk.ConsAddress, mocks testkeeper.MockedKeepers) {
valAddr := sdk.ValAddress(providerAddr.Address.Bytes())
mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(gomock.Any(), consAddr).
Return(stakingtypes.Validator{OperatorAddress: valAddr.String()}, nil)
mocks.MockStakingKeeper.EXPECT().GetLastValidatorPower(gomock.Any(), valAddr).
Return(int64(5), nil).Times(1)
},
hasMinPower: false,
}, {
name: "validator power is GTE min power",
expectation: func(consAddr sdk.ConsAddress, mocks testkeeper.MockedKeepers) {
valAddr := sdk.ValAddress(providerAddr.Address.Bytes())
mocks.MockStakingKeeper.EXPECT().GetValidatorByConsAddr(gomock.Any(), consAddr).
Return(stakingtypes.Validator{OperatorAddress: valAddr.String()}, nil)
mocks.MockStakingKeeper.EXPECT().GetLastValidatorPower(gomock.Any(), valAddr).
Return(int64(10), nil).Times(1)
},
hasMinPower: true,
},
}

minPower := int64(10)

for _, tc := range testCases {
t.Run(tc.name, func(t *testing.T) {
tc.expectation(consAddr, mocks)
require.Equal(t, tc.hasMinPower, pk.HasMinPower(ctx, providerAddr, minPower))
})
}
}
6 changes: 4 additions & 2 deletions x/ccv/provider/keeper/proposal.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,6 +232,7 @@ func (k Keeper) MakeConsumerGenesis(
return gen, nil, errorsmod.Wrapf(stakingtypes.ErrNoValidatorFound, "error getting last bonded validators: %s", err)
}

minPower := int64(0)
if powerShapingParameters.Top_N > 0 {
// get the consensus active validators
// we do not want to base the power calculation for the top N
Expand All @@ -243,7 +244,7 @@ func (k Keeper) MakeConsumerGenesis(
}

// in a Top-N chain, we automatically opt in all validators that belong to the top N
minPower, err := k.ComputeMinPowerInTopN(ctx, activeValidators, powerShapingParameters.Top_N)
minPower, err = k.ComputeMinPowerInTopN(ctx, activeValidators, powerShapingParameters.Top_N)
if err != nil {
return gen, nil, err
}
Expand All @@ -255,8 +256,9 @@ func (k Keeper) MakeConsumerGenesis(
k.OptInTopNValidators(ctx, consumerId, activeValidators, minPower)
k.SetMinimumPowerInTopN(ctx, consumerId, minPower)
}

// need to use the bondedValidators, not activeValidators, here since the chain might be opt-in and allow inactive vals
nextValidators := k.ComputeNextValidators(ctx, consumerId, bondedValidators)
nextValidators := k.ComputeNextValidators(ctx, consumerId, bondedValidators, minPower)
k.SetConsumerValSet(ctx, consumerId, nextValidators)

// get the initial updates with the latest set consumer public keys
Expand Down
5 changes: 3 additions & 2 deletions x/ccv/provider/keeper/relay.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -199,6 +199,7 @@ func (k Keeper) QueueVSCPackets(ctx sdk.Context) {
}
topN := k.GetTopN(ctx, consumerId)

minPower := int64(0)
if topN > 0 {
// in a Top-N chain, we automatically opt in all validators that belong to the top N
// of the active validators
Expand All @@ -208,7 +209,7 @@ func (k Keeper) QueueVSCPackets(ctx sdk.Context) {
panic(fmt.Errorf("failed to get active validators: %w", err))
}

minPower, err := k.ComputeMinPowerInTopN(ctx, activeValidators, topN)
minPower, err = k.ComputeMinPowerInTopN(ctx, activeValidators, topN)
if err != nil {
// we panic, since the only way to proceed would be to opt in all validators, which is not the intended behavior
panic(fmt.Errorf("failed to compute min power to opt in for chain %v: %w", consumerId, err))
Expand All @@ -220,7 +221,7 @@ func (k Keeper) QueueVSCPackets(ctx sdk.Context) {
k.OptInTopNValidators(ctx, consumerId, activeValidators, minPower)
}

nextValidators := k.ComputeNextValidators(ctx, consumerId, bondedValidators)
nextValidators := k.ComputeNextValidators(ctx, consumerId, bondedValidators, minPower)

valUpdates := DiffValidators(currentValidators, nextValidators)
k.SetConsumerValSet(ctx, consumerId, nextValidators)
Expand Down

0 comments on commit 88a6375

Please sign in to comment.