feat: Calculate Top N based on active validators only

docs/docs/adrs/adr-017-allowing-inactive-validators.md

@@ -113,6 +113,15 @@ The validator should be neither slashed nor jailed for downtime.
 This can be tested by having an inactive validator go offline on a consumer chain for long enough to accrue downtime.
 The consumer chain should send a SlashPacket to the provider chain, which should jail the validator.
 
+### Scenario 6: Inactive validators should not be counted when computing the minimum power in the top N
+
+This can be tested like this:
+* Start a provider chain with validator powers alice=300, bob=200, charlie=100 and 2 max provider consensus validators
+  * So alice and bob will validate on the provider
+* Start a consumer chain with top N = 51%. 
+  * Without inactive validators, this means both alice and bob have to validate. But since charlie is inactive, this means bob is *not* in the top N
+* Verify that alice is in the top N, but bob is not
+
 * **Mint**: 
 
 ## Consequences

tests/e2e/main.go

@@ -210,6 +210,12 @@ var stepChoices = map[string]StepChoice{
 		description: "test inactive validators on consumer",
 		testConfig:  InactiveProviderValsTestCfg,
 	},
+	"inactive-vals-topN": {
+		name:        "inactive-vals-topN",
+		steps:       stepsInactiveValsWithTopN(),
+		description: "test inactive validators on topN chain",
+		testConfig:  InactiveProviderValsTestCfg,
+	},
 }
 
 func getTestCaseUsageString() string {
@@ -299,7 +305,7 @@ func getTestCases(selectedPredefinedTests, selectedTestFiles TestSet, providerVe
 			"partial-set-security-validator-set-cap", "partial-set-security-validators-power-cap",
 			"partial-set-security-validators-allowlisted", "partial-set-security-validators-denylisted",
 			"partial-set-security-modification-proposal",
-			"active-set-changes",
+			"active-set-changes", "inactive-vals-topN",
 		}
 		if includeMultiConsumer != nil && *includeMultiConsumer {
 			selectedPredefinedTests = append(selectedPredefinedTests, "multiconsumer")

tests/e2e/steps_inactive_vals.go

@@ -499,3 +499,119 @@ func setupOptInChain() []Step {
 		},
 	}
 }
+
+func stepsInactiveValsWithTopN() []Step {
+	return []Step{
+		{
+			Action: StartChainAction{
+				Chain: ChainID("provi"),
+				Validators: []StartChainValidator{
+					{Id: ValidatorID("alice"), Stake: 100000000, Allocation: 10000000000},
+					{Id: ValidatorID("bob"), Stake: 200000000, Allocation: 10000000000},
+					{Id: ValidatorID("carol"), Stake: 300000000, Allocation: 10000000000},
+				},
+			},
+			State: State{
+				ChainID("provi"): ChainState{
+					ValPowers: &map[ValidatorID]uint{
+						ValidatorID("alice"): 0, // max consensus validators is 2, so alice should not be in power
+						ValidatorID("bob"):   200,
+						ValidatorID("carol"): 300,
+					},
+					StakedTokens: &map[ValidatorID]uint{
+						ValidatorID("alice"): 100000000,
+						ValidatorID("bob"):   200000000,
+						ValidatorID("carol"): 300000000,
+					},
+					Rewards: &Rewards{
+						IsNativeDenom:       true, // check for rewards in the provider denom
+						IsIncrementalReward: true, // we need to get incremental rewards
+						// if we would look at total rewards, alice would trivially also get rewards,
+						// because she gets rewards in the first block due to being in the genesis
+						IsRewarded: map[ValidatorID]bool{
+							ValidatorID("alice"): false,
+							ValidatorID("bob"):   true,
+							ValidatorID("carol"): true,
+						},
+					},
+				},
+			},
+		},
+		{
+			Action: SubmitConsumerAdditionProposalAction{
+				Chain:         ChainID("provi"),
+				From:          ValidatorID("alice"),
+				Deposit:       10000001,
+				ConsumerChain: ChainID("consu"),
+				SpawnTime:     0,
+				InitialHeight: clienttypes.Height{RevisionNumber: 0, RevisionHeight: 1},
+				TopN:          51,
+			},
+			State: State{
+				ChainID("provi"): ChainState{
+					Proposals: &map[uint]Proposal{
+						1: ConsumerAdditionProposal{
+							Deposit:       10000001,
+							Chain:         ChainID("consu"),
+							SpawnTime:     0,
+							InitialHeight: clienttypes.Height{RevisionNumber: 0, RevisionHeight: 1},
+							Status:        strconv.Itoa(int(gov.ProposalStatus_PROPOSAL_STATUS_VOTING_PERIOD)),
+						},
+					},
+				},
+			},
+		},
+		{
+			Action: VoteGovProposalAction{
+				Chain:      ChainID("provi"),
+				From:       []ValidatorID{ValidatorID("alice"), ValidatorID("bob")},
+				Vote:       []string{"yes", "yes"},
+				PropNumber: 1,
+			},
+			State: State{
+				ChainID("provi"): ChainState{
+					Proposals: &map[uint]Proposal{
+						1: ConsumerAdditionProposal{
+							Deposit:       10000001,
+							Chain:         ChainID("consu"),
+							SpawnTime:     0,
+							InitialHeight: clienttypes.Height{RevisionNumber: 0, RevisionHeight: 1},
+							Status:        strconv.Itoa(int(gov.ProposalStatus_PROPOSAL_STATUS_PASSED)),
+						},
+					},
+					HasToValidate: &map[ValidatorID][]ChainID{
+						ValidatorID("alice"): {},
+						ValidatorID("bob"):   {}, // bob doesn't have to validate because he is not in the top N
+						ValidatorID("carol"): {"consu"},
+					},
+				},
+			},
+		},
+		{
+			Action: StartConsumerChainAction{
+				ConsumerChain: ChainID("consu"),
+				ProviderChain: ChainID("provi"),
+				Validators: []StartChainValidator{
+					{Id: ValidatorID("alice"), Stake: 100000000, Allocation: 10000000000},
+					{Id: ValidatorID("bob"), Stake: 200000000, Allocation: 10000000000},
+					{Id: ValidatorID("carol"), Stake: 300000000, Allocation: 10000000000},
+				},
+				// For consumers that're launching with the provider being on an earlier version
+				// of ICS before the soft opt-out threshold was introduced, we need to set the
+				// soft opt-out threshold to 0.05 in the consumer genesis to ensure that the
+				// consumer binary doesn't panic. Sdk requires that all params are set to valid
+				// values from the genesis file.
+				GenesisChanges: ".app_state.ccvconsumer.params.soft_opt_out_threshold = \"0.05\"",
+			},
+			State: State{
+				ChainID("consu"): ChainState{
+					ValPowers: &map[ValidatorID]uint{
+						ValidatorID("alice"): 0, // alice and bob are not in the top N, so aren't in the validator set
+						ValidatorID("bob"):   0,
+						ValidatorID("carol"): 300,
+					},
+				},
+			},
+		},
+	}
+}

x/ccv/consumer/keeper/keeper.go

@@ -716,5 +716,9 @@ func (k Keeper) IsPrevStandaloneChain(ctx sdk.Context) bool {
 // GetLastBondedValidators iterates the last validator powers in the staking module
 // and returns the first MaxValidators many validators with the largest powers.
 func (k Keeper) GetLastBondedValidators(ctx sdk.Context) ([]stakingtypes.Validator, error) {
-	return ccv.GetLastBondedValidatorsUtil(ctx, k.standaloneStakingKeeper, k.Logger(ctx))
+	maxVals, err := k.standaloneStakingKeeper.MaxValidators(ctx)
+	if err != nil {
+		return nil, err
+	}
+	return ccv.GetLastBondedValidatorsUtil(ctx, k.standaloneStakingKeeper, k.Logger(ctx), maxVals)
 }

x/ccv/provider/keeper/legacy_proposal.go

@@ -125,11 +125,11 @@ func (k Keeper) HandleLegacyConsumerModificationProposal(ctx sdk.Context, p *typ
 	if p.Top_N != oldTopN {
 		if p.Top_N > 0 {
 			// if the chain receives a non-zero top N value, store the minimum power in the top N
-			bondedValidators, err := k.GetLastBondedValidators(ctx)
+			activeValidators, err := k.GetLastActiveBondedValidators(ctx)
 			if err != nil {
 				return err
 			}
-			minPower, err := k.ComputeMinPowerInTopN(ctx, bondedValidators, p.Top_N)
+			minPower, err := k.ComputeMinPowerInTopN(ctx, activeValidators, p.Top_N)
 			if err != nil {
 				return err
 			}

x/ccv/provider/keeper/proposal.go

@@ -286,14 +286,20 @@ func (k Keeper) MakeConsumerGenesis(
 	}
 
 	if prop.Top_N > 0 {
+		// get the consensus active validators
+		activeValidators, err := k.GetLastActiveBondedValidators(ctx)
+		if err != nil {
+			return gen, nil, errorsmod.Wrapf(stakingtypes.ErrNoValidatorFound, "error getting last active bonded validators: %s", err)
+		}
 		// in a Top-N chain, we automatically opt in all validators that belong to the top N
-		minPower, err := k.ComputeMinPowerInTopN(ctx, bondedValidators, prop.Top_N)
+		minPower, err := k.ComputeMinPowerInTopN(ctx, activeValidators, prop.Top_N)
 		if err != nil {
 			return gen, nil, err
 		}
-		k.OptInTopNValidators(ctx, chainID, bondedValidators, minPower)
+		k.OptInTopNValidators(ctx, chainID, activeValidators, minPower)
 		k.SetMinimumPowerInTopN(ctx, chainID, minPower)
 	}
+	// need to use the bondedValidators, not activeValidators, here since the chain might be opt-in and allow inactive vals
 	nextValidators := k.ComputeNextValidators(ctx, chainID, bondedValidators)
 
 	k.SetConsumerValSet(ctx, chainID, nextValidators)

x/ccv/provider/keeper/relay.go

@@ -282,12 +282,18 @@ func (k Keeper) QueueVSCPackets(ctx sdk.Context) {
 
 		if topN > 0 {
 			// in a Top-N chain, we automatically opt in all validators that belong to the top N
-			minPower, err := k.ComputeMinPowerInTopN(ctx, bondedValidators, topN)
+			// of the active validators
+			activeValidators, err := k.GetLastActiveBondedValidators(ctx)
+			if err != nil {
+				// we just log here and do not panic because panic-ing would halt the provider chain
+				k.Logger(ctx).Error("failed to get active validators", "error", err)
+			}
+			minPower, err := k.ComputeMinPowerInTopN(ctx, activeValidators, topN)
 			if err == nil {
 				// set the minimal power of validators in the top N in the store
 				k.SetMinimumPowerInTopN(ctx, chainID, minPower)
 
-				k.OptInTopNValidators(ctx, chainID, bondedValidators, minPower)
+				k.OptInTopNValidators(ctx, chainID, activeValidators, minPower)
 			} else {
 				// we just log here and do not panic because panic-ing would halt the provider chain
 				k.Logger(ctx).Error("failed to compute min power to opt in for chain", "chain", chainID, "error", err)

x/ccv/provider/keeper/relay_test.go

@@ -852,6 +852,11 @@ func TestQueueVSCPacketsWithPowerCapping(t *testing.T) {
 	// set a power-capping of 40%
 	providerKeeper.SetValidatorsPowerCap(ctx, "chainID", 40)
 
+	// set max provider consensus validators to a value that is larger than the number of validators
+	params := providerKeeper.GetParams(ctx)
+	params.MaxProviderConsensusValidators = 180
+	providerKeeper.SetParams(ctx, params)
+
 	providerKeeper.QueueVSCPackets(ctx)
 
 	actualQueuedVSCPackets := providerKeeper.GetPendingVSCPackets(ctx, "chainID")

x/ccv/provider/keeper/validator_set_update.go

@@ -167,5 +167,14 @@ func (k Keeper) FilterValidators(
 // GetLastBondedValidators iterates the last validator powers in the staking module
 // and returns the first MaxValidators many validators with the largest powers.
 func (k Keeper) GetLastBondedValidators(ctx sdk.Context) ([]stakingtypes.Validator, error) {
-	return ccv.GetLastBondedValidatorsUtil(ctx, k.stakingKeeper, k.Logger(ctx))
+	maxVals, err := k.stakingKeeper.MaxValidators(ctx)
+	if err != nil {
+		return nil, err
+	}
+	return ccv.GetLastBondedValidatorsUtil(ctx, k.stakingKeeper, k.Logger(ctx), maxVals)
+}
+
+func (k Keeper) GetLastActiveBondedValidators(ctx sdk.Context) ([]stakingtypes.Validator, error) {
+	maxVals := k.GetMaxProviderConsensusValidators(ctx)
+	return ccv.GetLastBondedValidatorsUtil(ctx, k.stakingKeeper, k.Logger(ctx), uint32(maxVals))
 }

x/ccv/types/utils.go

@@ -125,16 +125,13 @@ func GetConsAddrFromBech32(bech32str string) (sdk.ConsAddress, error) {
 	return sdk.ConsAddress(addr), nil
 }
 
-func GetLastBondedValidatorsUtil(ctx sdk.Context, stakingKeeper StakingKeeper, logger log.Logger) ([]stakingtypes.Validator, error) {
-	maxVals, err := stakingKeeper.MaxValidators(ctx)
-	if err != nil {
-		return nil, err
-	}
-
+// GetLastBondedValidatorsUtil iterates the last validator powers in the staking module
+// and returns the first MaxValidators many validators with the largest powers.
+func GetLastBondedValidatorsUtil(ctx sdk.Context, stakingKeeper StakingKeeper, logger log.Logger, maxVals uint32) ([]stakingtypes.Validator, error) {
 	lastPowers := make([]stakingtypes.LastValidatorPower, maxVals)
 
 	i := 0
-	err = stakingKeeper.IterateLastValidatorPowers(ctx, func(addr sdk.ValAddress, power int64) (stop bool) {
+	err := stakingKeeper.IterateLastValidatorPowers(ctx, func(addr sdk.ValAddress, power int64) (stop bool) {
 		lastPowers[i] = stakingtypes.LastValidatorPower{Address: addr.String(), Power: power}
 		i++
 		return i >= int(maxVals) // stop iteration if true