Skip to content

Commit

Permalink
make OptInTopNValidators return an error
Browse files Browse the repository at this point in the history
  • Loading branch information
mpoke committed Sep 12, 2024
1 parent c05141e commit 75a7aef
Show file tree
Hide file tree
Showing 4 changed files with 40 additions and 22 deletions.
8 changes: 7 additions & 1 deletion x/ccv/provider/keeper/consumer_lifecycle.go
Original file line number Diff line number Diff line change
Expand Up @@ -330,8 +330,14 @@ func (k Keeper) MakeConsumerGenesis(
"consumerId", consumerId,
"minPower", minPower,
)
k.OptInTopNValidators(ctx, consumerId, activeValidators, minPower)

// set the minimal power of validators in the top N in the store
k.SetMinimumPowerInTopN(ctx, consumerId, minPower)

err = k.OptInTopNValidators(ctx, consumerId, activeValidators, minPower)
if err != nil {
return gen, nil, fmt.Errorf("unable to opt in topN validators in MakeConsumerGenesis, consumerId(%s): %w", consumerId, err)
}
}

// need to use the bondedValidators, not activeValidators, here since the chain might be opt-in and allow inactive vals
Expand Down
37 changes: 21 additions & 16 deletions x/ccv/provider/keeper/partial_set_security.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,8 @@
package keeper

import (
"fmt"

errorsmod "cosmossdk.io/errors"
storetypes "cosmossdk.io/store/types"

Expand Down Expand Up @@ -101,40 +103,43 @@ func (k Keeper) HandleOptOut(ctx sdk.Context, consumerId string, providerAddr ty
}

// OptInTopNValidators opts in to `consumerId` all the `bondedValidators` that have at least `minPowerToOptIn` power
func (k Keeper) OptInTopNValidators(ctx sdk.Context, consumerId string, bondedValidators []stakingtypes.Validator, minPowerToOptIn int64) {
func (k Keeper) OptInTopNValidators(
ctx sdk.Context,
consumerId string,
bondedValidators []stakingtypes.Validator,
minPowerToOptIn int64,
) error {
for _, val := range bondedValidators {
// log the validator
k.Logger(ctx).Debug("Checking whether to opt in validator because of top N", "validator", val.GetOperator())
k.Logger(ctx).Debug("Checking whether to opt in validator because of top N",
"consumerId", consumerId,
"validator", val.GetOperator(),
)

valAddr, err := sdk.ValAddressFromBech32(val.GetOperator())
if err != nil {
k.Logger(ctx).Error("could not retrieve validator address from operator address",
"validator operator address", val.GetOperator(),
"error", err.Error())
continue
return fmt.Errorf("converting operator address to validator address, consumerId(%s), validator(%s): %w",
consumerId, val.GetOperator(), err)
}
power, err := k.stakingKeeper.GetLastValidatorPower(ctx, valAddr)
if err != nil {
k.Logger(ctx).Error("could not retrieve last power of validator",
"validator operator address", val.GetOperator(),
"error", err.Error())
continue
return fmt.Errorf("getting validator power, consumerId(%s), validator(%s): %w",
consumerId, val.GetOperator(), err)
}
if power >= minPowerToOptIn {
consAddr, err := val.GetConsAddr()
if err != nil {
k.Logger(ctx).Error("could not retrieve validator consensus address",
"validator operator address", val.GetOperator(),
"error", err.Error())
continue
return fmt.Errorf("getting validator consensus address, consumerId(%s), validator(%s): %w",
consumerId, val.GetOperator(), err)
}

k.Logger(ctx).Debug("Opting in validator", "validator", val.GetOperator())
k.Logger(ctx).Debug("Opting in validator", "consumerId", consumerId, "validator", val.GetOperator())

// if validator already exists it gets overwritten
// if validator is already opted in, it gets overwritten
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
}
return nil
}

//
Expand Down
12 changes: 8 additions & 4 deletions x/ccv/provider/keeper/partial_set_security_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -187,7 +187,8 @@ func TestOptInTopNValidators(t *testing.T) {
valDConsAddr, _ := valD.GetConsAddr()

// Start Test 1: opt in all validators with power >= 0
providerKeeper.OptInTopNValidators(ctx, CONSUMER_ID, []stakingtypes.Validator{valA, valB, valC, valD}, 0)
err := providerKeeper.OptInTopNValidators(ctx, CONSUMER_ID, []stakingtypes.Validator{valA, valB, valC, valD}, 0)
require.NoError(t, err)
expectedOptedInValidators := []providertypes.ProviderConsAddress{
providertypes.NewProviderConsAddress(valAConsAddr),
providertypes.NewProviderConsAddress(valBConsAddr),
Expand Down Expand Up @@ -216,7 +217,8 @@ func TestOptInTopNValidators(t *testing.T) {
// Start Test 2: opt in all validators with power >= 1
// We expect the same `expectedOptedInValidators` as when we opted in all validators with power >= 0 because the
// validators with the smallest power have power == 1
providerKeeper.OptInTopNValidators(ctx, CONSUMER_ID, []stakingtypes.Validator{valA, valB, valC, valD}, 0)
err = providerKeeper.OptInTopNValidators(ctx, CONSUMER_ID, []stakingtypes.Validator{valA, valB, valC, valD}, 0)
require.NoError(t, err)
actualOptedInValidators = providerKeeper.GetAllOptedIn(ctx, CONSUMER_ID)
sortUpdates(actualOptedInValidators)
require.Equal(t, expectedOptedInValidators, actualOptedInValidators)
Expand All @@ -227,7 +229,8 @@ func TestOptInTopNValidators(t *testing.T) {
providerKeeper.DeleteOptedIn(ctx, CONSUMER_ID, providertypes.NewProviderConsAddress(valDConsAddr))

// Start Test 3: opt in all validators with power >= 2 and hence we do not expect to opt in validator A
providerKeeper.OptInTopNValidators(ctx, CONSUMER_ID, []stakingtypes.Validator{valA, valB, valC, valD}, 2)
err = providerKeeper.OptInTopNValidators(ctx, CONSUMER_ID, []stakingtypes.Validator{valA, valB, valC, valD}, 2)
require.NoError(t, err)
expectedOptedInValidators = []providertypes.ProviderConsAddress{
providertypes.NewProviderConsAddress(valBConsAddr),
providertypes.NewProviderConsAddress(valCConsAddr),
Expand All @@ -246,7 +249,8 @@ func TestOptInTopNValidators(t *testing.T) {
providerKeeper.DeleteOptedIn(ctx, CONSUMER_ID, providertypes.NewProviderConsAddress(valDConsAddr))

// Start Test 4: opt in all validators with power >= 4 and hence we do not expect any opted-in validators
providerKeeper.OptInTopNValidators(ctx, CONSUMER_ID, []stakingtypes.Validator{valA, valB, valC, valD}, 4)
err = providerKeeper.OptInTopNValidators(ctx, CONSUMER_ID, []stakingtypes.Validator{valA, valB, valC, valD}, 4)
require.NoError(t, err)
require.Empty(t, providerKeeper.GetAllOptedIn(ctx, CONSUMER_ID))
}

Expand Down
5 changes: 4 additions & 1 deletion x/ccv/provider/keeper/relay.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,7 +248,10 @@ func (k Keeper) QueueVSCPackets(ctx sdk.Context) error {
// set the minimal power of validators in the top N in the store
k.SetMinimumPowerInTopN(ctx, consumerId, minPower)

k.OptInTopNValidators(ctx, consumerId, activeValidators, minPower)
err := k.OptInTopNValidators(ctx, consumerId, activeValidators, minPower)
if err != nil {
return fmt.Errorf("opting in topN validators, consumerId(%s), minPower(%d): %w", consumerId, minPower, err)
}
}

nextValidators, err := k.ComputeNextValidators(ctx, consumerId, bondedValidators, powerShapingParameters, minPower)
Expand Down

0 comments on commit 75a7aef

Please sign in to comment.