feat: Calculate Top N based on active validators only (#2070)

* Add test for inactive vals with top N

* Add test case to predefined tests

* Fix bonded/active validator distinction

* Fix relay test to set max provider consensus vals correctly

* feat!: Add a parameter that determines whether consumer chains allow inactive validators to validate them (#2066)

* Introduce new AllowInactiveValidators param for consumer chains

* Add AllowInactiveValidators param to tests

* Set MaxProviderConsensusValidators in tests

* Add migration to initialize inactive vals

* Add changelog entries for inactive vals param

* Add property-based test for inactive vals

* Add docs for inactive vals param

* Set AllowInactiveVals parameter in e2e test

* test: Add e2e tests for inactive vals (#2064)

* Start adding e2e test for governance

* Debug gov with inactive vals test

* Outline for test scenarios where they are tested

* Add MaxRank steps

* Add e2e tests for min stake and max rank

* Revert formatting change

* Refactor stepsOptIn

* Use adjusted config for e2e tests

* Write for more scenarios where they are tested

* Add test for mint

* Add docstrings for e2e steps

.changelog/unreleased/features/provider/2066-inactive-vals.md

@@ -0,0 +1 @@
+- Add the `allow_inactive_vals` parameter for consumer chains to choose whether inactive validators can validate their chain ([\#2066](https://github.com/cosmos/interchain-security/pull/2066))

.changelog/unreleased/state-breaking/provider/2066-inactive-vals.md

@@ -0,0 +1 @@
+- Add the `allow_inactive_vals` parameter for consumer chains to choose whether inactive validators can validate their chain ([\#2066](https://github.com/cosmos/interchain-security/pull/2066))

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

@@ -62,7 +62,10 @@ The following changes to the state are required:
 * Store the provider consensus validator set in the provider module state under the `LastProviderConsensusValsPrefix` key. This is the last set of validators that the provider sent to the consensus engine. This is needed to compute the ValUpdates to send to the consensus engine (by diffing the current set with this last sent set).
 * Increase the `MaxValidators` parameter of the staking module to the desired size of the potential validator
 set of consumer chains.
-* Introduce two extra per-consumer-chain parameters: `MinStake` and `MaxValidatorRank`. `MinStake` is the minimum amount of stake a validator must have to be considered for validation on the consumer chain. `MaxValidatorRank` is the maximum rank of a validator that can validate on the consumer chain. The provider module will only consider the first `MaxValidatorRank` validators that have at least `MinStake` stake as potential validators for the consumer chain.
+* Introduce extra per-consumer-chain parameters: 
+  * `MinStake`: is the minimum amount of stake a validator must have to be considered for validation on the consumer chain. 
+  * `MaxValidatorRank`: is the maximum rank in the provider validator set that can validate on the consumer chain. For example, setting this to 1 means only the validator with the most stake can validate on the consumer chain.
+  * `AllowInactiveVals`: is a boolean that determines whether validators that are not part of the active set on the provider chain can validate on the consumer chain. If this is set to `true`, validators outside the active set on the provider chain can validate on the consumer chain. If this is set to `true`, validators outside the active set on the provider chain cannot validate on the consumer chain.
 
 ## Risk Mitigations
 
@@ -85,35 +88,84 @@ In the following,
 ### Scenario 1: Inactive validators should not be considered by governance
 
 Inactive validators should not be considered for the purpose of governance.
-In particular, the governance module should not allow inactive validators to vote on proposals,
-and the quorum depends only on the stake bonded by active validators.
+In particular, the quorum should depend only on active validators.
 
-This can be tested by creating a governance proposal, then trying to vote on it with inactive validators.
-The proposal should not pass.
-Afterwards, we create another proposal and vote on it with active validators, too.
-Then, the proposal should pass.
+We test this by:
+* creating a provider chain (either with 3 active validators, or with only 1 active validator), a quorum of 50%, and 3 validators with alice=300, bob=299, charlie=299 stake
+* we create a governance proposal
+* alice votes for the proposal
+* we check that the proposal has the right status:
+  * in the scenario where we have 3 active validators, the proposal *should not* have passed, because alice alone is not enough to fulfill the quorum
+  * in the scenario where we have 1 active validator, the proposal *should* have passed, because alice is the only active validator, and thus fulfills the quorum
+
+Tested by the e2e tests `inactive-provider-validators-governance` (scenario with 1 active val) and `inactive-provider-validators-governance-basecase` (scenario with 3 active vals).
 
 ### Scenario 2: Inactive validators should not get rewards from the provider chain
 
 Inactive validators should not get rewards from the provider chain.
 
 This can be tested by starting a provider chain with inactive validators and checking the rewards of inactive validators.
 
+Checked as part of the e2e test `inactive-provider-validators-on-consumer`.
+
 ### Scenario 3: Inactive validators should get rewards from consumer chains
 
 An inactive validator that is validating on a consumer chain should receive rewards in the consumer chain token.
 
+Checked as part of the e2e test `inactive-provider-validators-on-consumer`.
+
 ### Scenario 4: Inactive validators should not get slashed/jailed for downtime on the provider chain
 
 This can be tested by having an inactive validator go offline on the provider chain for long enough to accrue downtime.
 The validator should be neither slashed nor jailed for downtime.
 
+Checked as part of the e2e test `inactive-provider-validators-on-consumer`.
+
 ### Scenario 5: Inactive validators *should* get jailed for downtime on the provider chain
 
 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.
 
-* **Mint**: 
+Checked as part of the e2e test `inactive-provider-validators-on-consumer`.
+
+### 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
+
+Checked as part of the e2e test `inactive-vals-topN`.
+
+### Scenario 7: Mint does not consider inactive validators
+
+To compute the inflation rate, only the active validators should be considered.
+
+We can check this by querying the inflation rate change over subsequent blocks.
+
+We start a provider chain with these arguments
+* 3 validators with powers alice=290, bob=280, charlie=270
+* either 1 or 3 active validators
+* a bonded goal of 300 tokens (this is given in percent, but we simplify here)
+
+If we have 3 validators active, then the inflation rate should *decrease* between blocks, because the bonded goal is exceeded as all validators are bonded.
+If we have only 1 validator active, then the inflation rate should *increase* between blocks, because the bonded goal is not met.
+
+Checked as part of the e2e tests `inactive-vals-mint` (scenario with 1 active val) and `mint-basecase` (scenario with 3 active vals).
+
+### Scenarios 8: Inactive validators can validate on consumer chains
+
+An inactive validator can opt in and validate on consumer chains (if min stake and max rank allow it)
+
+Checked as part of the e2e test `inactive-provider-validators-on-consumer`.
+
+### Scenario 9: MinStake and MaxRank parameters are respected
+
+Validators that don't meet the criteria for a consumer chain cannot validate on it.
+
+Checked in the e2e tests `min-stake` and `max-rank`.
 
 ## Consequences
 

docs/docs/features/power-shaping.md

@@ -36,9 +36,11 @@ power. For example, consider that the top validator `V` on the provider chain ha
 then if `V` is denylisted, the consumer chain would only be secured by at least 40% of the provider's power.
 :::
 
-1) **Maximum validator rank**: The consumer chain can specify a maximum position in the validator set that a validator can have on the provider chain to be able to validate the consumer chain. This can be used to ensure that only validators with relatively large amounts of stake can validate the consumer chain. For example, setting this to 20 would mean only the 20 validators with the most voting stake on the provider chain can validate the consumer chain.
+4) **Maximum validator rank**: The consumer chain can specify a maximum position in the validator set that a validator can have on the provider chain to be able to validate the consumer chain. This can be used to ensure that only validators with relatively large amounts of stake can validate the consumer chain. For example, setting this to 20 would mean only the 20 validators with the most voting stake on the provider chain can validate the consumer chain.
 
-2) **Minimum validator stake**: The consumer chain can specify a minimum amount of stake that a validator must have on the provider chain to be able to validate the consumer chain. This can be used to ensure that only validators with a certain amount of stake can validate the consumer chain. For example, setting this to 1000 would mean only validators with at least 1000 tokens staked on the provider chain can validate the consumer chain.
+5) **Minimum validator stake**: The consumer chain can specify a minimum amount of stake that a validator must have on the provider chain to be able to validate the consumer chain. This can be used to ensure that only validators with a certain amount of stake can validate the consumer chain. For example, setting this to 1000 would mean only validators with at least 1000 tokens staked on the provider chain can validate the consumer chain.
+
+6) **Allow inactive validators**: The consumer chain can specify whether provider validators that are *not* active in consensus may validate on the consumer chain or not. If this is set to `false`, only active validators on the provider chain can validate the consumer chain. If this is set to `true`, inactive validators can also validate the consumer chain. This can be useful for chains that want to have a larger validator set than the active validators on the provider chain, or for chains that want to have a more decentralized validator set. COnsumer chains that enable this feature should strongly consider setting a maximum validator rank and/or a minimum validator stake to ensure that only validators with some reputation/stake can validate the chain. 
 
 All these mechanisms are set by the consumer chain in the `ConsumerAdditionProposal`. They operate *solely on the provider chain*, meaning the consumer chain simply receives the validator set after these rules have been applied and does not have any knowledge about whether they are applied.
 
@@ -47,6 +49,8 @@ Each of these mechanisms is *set during the consumer addition proposal* (see [On
 The values can be seen by querying the list of consumer chains:
 ```bash
 interchain-security-pd query provider list-consumer-chains
+
+
 ```
 
 ## Guidelines for setting power shaping parameters

proto/interchain_security/ccv/provider/v1/provider.proto

@@ -110,6 +110,8 @@ message ConsumerAdditionProposal {
   uint64 min_stake = 20;
   // Corresponds to the maximal rank in the provider chain validator set that a validator can have to validate on the consumer chain.
   uint32 max_rank = 21;
+  // Corresponds to whether inactive validators are allowed to validate the consumer chain.
+  bool allow_inactive_vals = 22;
 }
 
 // ConsumerRemovalProposal is a governance proposal on the provider chain to
@@ -164,6 +166,8 @@ message ConsumerModificationProposal {
   uint64 min_stake = 9;
   // Corresponds to the maximal rank in the provider chain validator set that a validator can have to validate on the consumer chain.
   uint32 max_rank = 10;
+  // Corresponds to whether inactive validators are allowed to validate the consumer chain.
+  bool allow_inactive_vals = 11;
 }
 
 

proto/interchain_security/ccv/provider/v1/tx.proto

@@ -187,6 +187,8 @@ message MsgConsumerAddition {
   uint64 min_stake = 19;
   // Corresponds to the maximal rank in the provider chain validator set that a validator can have to validate on the consumer chain.
   uint32 max_rank = 20;
+  // Corresponds to whether inactive validators are allowed to validate the consumer chain.
+  bool allow_inactive_vals = 21;
 }
 
 // MsgConsumerAdditionResponse defines response type for MsgConsumerAddition messages
@@ -327,6 +329,8 @@ message MsgConsumerModification {
   uint64 min_stake = 10;
   // Corresponds to the maximal rank in the provider chain validator set that a validator can have to validate on the consumer chain.
   uint32 max_rank = 11;
+  // Corresponds to whether inactive validators are allowed to validate the consumer chain.
+  bool allow_inactive_vals = 12;
 }
 
 message MsgConsumerModificationResponse {}

tests/e2e/actions.go

@@ -263,6 +263,9 @@ type SubmitConsumerAdditionProposalAction struct {
 	ValidatorSetCap     uint32
 	Allowlist           []string
 	Denylist            []string
+	MaxValidatorRank    uint32
+	MinStake            uint64
+	AllowInactiveVals   bool
 }
 
 func (tr Chain) submitConsumerAdditionProposal(
@@ -292,6 +295,9 @@ func (tr Chain) submitConsumerAdditionProposal(
 		ValidatorSetCap:                   action.ValidatorSetCap,
 		Allowlist:                         action.Allowlist,
 		Denylist:                          action.Denylist,
+		MaxValidatorRank:                  action.MaxValidatorRank,
+		MinStake:                          action.MinStake,
+		AllowInactiveVals:                 action.AllowInactiveVals,
 	}
 
 	bz, err := json.Marshal(prop)
@@ -334,7 +340,6 @@ func (tr Chain) submitConsumerAdditionProposal(
 		fmt.Println("submitConsumerAdditionProposal json:", jsonStr)
 	}
 	bz, err = cmd.CombinedOutput()
-
 	if err != nil {
 		log.Fatal(err, "\n", string(bz))
 	}
@@ -1010,7 +1015,6 @@ func (tr Chain) addChainToHermes(
 	action AddChainToRelayerAction,
 	verbose bool,
 ) {
-
 	bz, err := tr.target.ExecCommand("bash", "-c", "hermes", "version").CombinedOutput()
 	if err != nil {
 		log.Fatal(err, "\n error getting hermes version", string(bz))
@@ -1916,7 +1920,7 @@ func (tr Chain) registerRepresentative(
 				panic(fmt.Sprintf("failed writing ccv consumer file : %v", err))
 			}
 			defer file.Close()
-			err = os.WriteFile(file.Name(), []byte(fileContent), 0600)
+			err = os.WriteFile(file.Name(), []byte(fileContent), 0o600)
 			if err != nil {
 				log.Fatalf("Failed writing consumer genesis to file: %v", err)
 			}

tests/e2e/config.go

@@ -94,6 +94,10 @@ const (
 	CompatibilityTestCfg        TestConfigType = "compatibility"
 	SmallMaxValidatorsTestCfg   TestConfigType = "small-max-validators"
 	InactiveProviderValsTestCfg TestConfigType = "inactive-provider-vals"
+	GovTestCfg                  TestConfigType = "gov"
+	InactiveValsGovTestCfg      TestConfigType = "inactive-vals-gov"
+	InactiveValsMintTestCfg     TestConfigType = "inactive-vals-mint"
+	MintTestCfg                 TestConfigType = "mint"
 )
 
 type TestConfig struct {
@@ -184,6 +188,14 @@ func GetTestConfig(cfgType TestConfigType, providerVersion, consumerVersion stri
 		testCfg = SmallMaxValidatorsTestConfig()
 	case InactiveProviderValsTestCfg:
 		testCfg = InactiveProviderValsTestConfig()
+	case GovTestCfg:
+		testCfg = GovTestConfig()
+	case InactiveValsGovTestCfg:
+		testCfg = InactiveValsGovTestConfig()
+	case InactiveValsMintTestCfg:
+		testCfg = InactiveValsMintTestConfig()
+	case MintTestCfg:
+		testCfg = MintTestConfig()
 	default:
 		panic(fmt.Sprintf("Invalid test config: %s", cfgType))
 	}
@@ -581,7 +593,7 @@ func InactiveProviderValsTestConfig() TestConfig {
 	tr.chainConfigs[ChainID("provi")] = proviConfig
 	tr.chainConfigs[ChainID("consu")] = consuConfig
 
-	// make is to that carol does not use a consumer key
+	// make it so that carol does not use a consumer key
 	carolConfig := tr.validatorConfigs[ValidatorID("carol")]
 	carolConfig.UseConsumerKey = false
 	tr.validatorConfigs[ValidatorID("carol")] = carolConfig
@@ -605,6 +617,60 @@ func SmallMaxValidatorsTestConfig() TestConfig {
 	return cfg
 }
 
+func GovTestConfig() TestConfig {
+	cfg := DefaultTestConfig()
+
+	// set the quorum to 50%
+	proviConfig := cfg.chainConfigs[ChainID("provi")]
+	proviConfig.GenesisChanges += "| .app_state.gov.params.quorum = \"0.5\""
+	cfg.chainConfigs[ChainID("provi")] = proviConfig
+
+	carolConfig := cfg.validatorConfigs["carol"]
+	// make carol use her own key
+	carolConfig.UseConsumerKey = false
+	cfg.validatorConfigs["carol"] = carolConfig
+
+	return cfg
+}
+
+func InactiveValsGovTestConfig() TestConfig {
+	cfg := GovTestConfig()
+
+	// set the MaxValidators to 1
+	proviConfig := cfg.chainConfigs[ChainID("provi")]
+	proviConfig.GenesisChanges += "| .app_state.staking.params.max_validators = 1"
+	cfg.chainConfigs[ChainID("provi")] = proviConfig
+
+	return cfg
+}
+
+func MintTestConfig() TestConfig {
+	cfg := GovTestConfig()
+	AdjustMint(cfg)
+
+	return cfg
+}
+
+func InactiveValsMintTestConfig() TestConfig {
+	cfg := InactiveValsGovTestConfig()
+	AdjustMint(cfg)
+
+	return cfg
+}
+
+// AdjustMint adjusts the mint parameters to have a very low goal bonded amount
+// and a high inflation rate change
+func AdjustMint(cfg TestConfig) {
+	proviConfig := cfg.chainConfigs[ChainID("provi")]
+	// total supply is 30000000000stake; we want to set the mint bonded goal to
+	// a small fraction of that
+	proviConfig.GenesisChanges += "| .app_state.mint.params.goal_bonded = \"0.001\"" +
+		"| .app_state.mint.params.inflation_rate_change = \"1\"" +
+		"| .app_state.mint.params.inflation_max = \"0.5\"" +
+		"| .app_state.mint.params.inflation_min = \"0.1\""
+	cfg.chainConfigs[ChainID("provi")] = proviConfig
+}
+
 func MultiConsumerTestConfig() TestConfig {
 	tr := TestConfig{
 		name: string(MulticonsumerTestCfg),

tests/e2e/main.go

@@ -210,6 +210,48 @@ 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,
+	},
+	"inactive-provider-validators-governance": {
+		name:        "inactive-provider-validators-governance",
+		steps:       stepsInactiveProviderValidatorsGovernance(),
+		description: "test governance with inactive validators",
+		testConfig:  InactiveValsGovTestCfg,
+	},
+	"inactive-provider-validators-governance-basecase": {
+		name:        "inactive-provider-validators-governance-basecase",
+		steps:       stepsInactiveProviderValidatorsGovernanceBasecase(),
+		description: "comparison for governance when there are *no* inactive validators, to verify the difference to the governance test *with* inactive validators",
+		testConfig:  GovTestCfg,
+	},
+	"max-rank": {
+		name:        "max-rank",
+		steps:       stepsMaxRank(),
+		description: "checks that the max rank parameter for consumer chains is respected",
+		testConfig:  GovTestCfg, // can reuse the GovTestCfg because all parameters there are ok to use here
+	},
+	"min-stake": {
+		name:        "min-stake",
+		steps:       stepsMinStake(),
+		description: "checks that the min stake parameter for consumer chains is respected",
+		testConfig:  GovTestCfg, // see above: we reuse the GovTestCfg for convenience
+	},
+	"inactive-vals-mint": {
+		name:        "inactive-vals-mint",
+		steps:       stepsInactiveValsMint(),
+		description: "test minting with inactive validators",
+		testConfig:  InactiveValsMintTestCfg,
+	},
+	"mint-basecase": {
+		name:        "mint-basecase",
+		steps:       stepsMintBasecase(),
+		description: "test minting without inactive validators as a sanity check",
+		testConfig:  MintTestCfg,
+	},
 }
 
 func getTestCaseUsageString() string {
@@ -299,7 +341,9 @@ 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",
+			"inactive-provider-validators-on-consumer", "inactive-provider-validators-governance",
+			"max-rank", "min-stake", "inactive-vals-mint",
 		}
 		if includeMultiConsumer != nil && *includeMultiConsumer {
 			selectedPredefinedTests = append(selectedPredefinedTests, "multiconsumer")