fix!: make OptInTopNValidators return an error (#2259)

make OptInTopNValidators return an error

x/ccv/provider/keeper/consumer_lifecycle.go

@@ -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

x/ccv/provider/keeper/partial_set_security.go

@@ -1,6 +1,8 @@
 package keeper
 
 import (
+	"fmt"
+
 	errorsmod "cosmossdk.io/errors"
 	storetypes "cosmossdk.io/store/types"
 
@@ -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
 }
 
 //

x/ccv/provider/keeper/partial_set_security_test.go

@@ -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),
@@ -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)
@@ -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),
@@ -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))
 }
 

x/ccv/provider/keeper/relay.go

@@ -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)