feat!: Introduce the MaxProviderConsensusValidators param #1992
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| store := ctx.KVStore(k.storeKey) | ||
| bz, err := validator.Marshal() | ||
| if err != nil { | ||
| panic(fmt.Errorf("failed to marshal ConsumerValidator: %w", err)) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
| iterator.Value() | ||
| var validator types.ConsumerValidator | ||
| if err := validator.Unmarshal(iterator.Value()); err != nil { | ||
| panic(fmt.Errorf("failed to unmarshal ConsumerValidator: %w", err)) |
Check warning
Code scanning / CodeQL
Panic in BeginBock or EndBlock consensus methods Warning
github-actions
bot
added
the
C:x/provider
|
Warning Review failedThe pull request is closed. WalkthroughWalkthroughThe changes introduce functions and parameters to manage provider consensus validators within the interchain security module. This includes adding a new Changes
Tip AI model upgrade
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
p-offtermatt
changed the base branch from
feat/inactive-vals-v50
to
ph/refactor-valset-storage
sainoe
reviewed
Jul 2, 2024
| @@ -88,5 +88,7 @@ func GetParamsLegacy(ctx sdk.Context, paramspace ccvtypes.LegacyParamSubspace) t | |||
| getSlashMeterReplenishFraction(ctx, paramspace), | |||
| getConsumerRewardDenomRegistrationFee(ctx, paramspace), | |||
| getBlocksPerEpoch(ctx, paramspace), | |||
| // this parameter is new so it doesn't need to be migrated, just initialized | |||
There was a problem hiding this comment.
It seems to me that MaxProviderConsensusValidators is not part of the legacy params, as it will be introduced either during or after the v6 migration.
There was a problem hiding this comment.
The NewParams function needs to take the MaxProviderConsensus validators as a parameter anyways, so it just is easiest to initialize it here, even though it's not a legacy param.
(That's what the comment is trying to say)
Do you have a suggestion for what to do differently?
insumity
approved these changes
Jul 2, 2024
Base automatically changed from
ph/refactor-valset-storage
to
feat/inactive-vals-v50
July 2, 2024 15:25
…sus validator storage (#1990) * Refactor validator set storage * Add comment for getTotalPower * Add provider consensus validator set storage * Add key to key test * Add unit test for LastTotalProviderConsensusPower * Address comments
p-offtermatt
added a commit
that referenced
this pull request
Jul 22, 2024
* Refactor validator set storage * Add comment for getTotalPower * Add provider consensus validator set storage * Add new MaxProviderConsensusValidators param * Add validation for MaxProviderConsensusValidators * Add function to get MaxProviderConsensusValidators param * refactor!: Refactor the validator set storage and add provider consensus validator storage (#1990) * Refactor validator set storage * Add comment for getTotalPower * Add provider consensus validator set storage * Add key to key test * Add unit test for LastTotalProviderConsensusPower * Address comments
github-merge-queue bot
pushed a commit
that referenced
this pull request
Aug 6, 2024
…er chains (#2079) * refactor!: Refactor the validator set storage and add provider consensus validator storage (#1990) * Refactor validator set storage * Add comment for getTotalPower * Add provider consensus validator set storage * Add key to key test * Add unit test for LastTotalProviderConsensusPower * Address comments * feat!: Introduce the MaxProviderConsensusValidators param (#1992) * Refactor validator set storage * Add comment for getTotalPower * Add provider consensus validator set storage * Add new MaxProviderConsensusValidators param * Add validation for MaxProviderConsensusValidators * Add function to get MaxProviderConsensusValidators param * refactor!: Refactor the validator set storage and add provider consensus validator storage (#1990) * Refactor validator set storage * Add comment for getTotalPower * Add provider consensus validator set storage * Add key to key test * Add unit test for LastTotalProviderConsensusPower * Address comments * feat!: Wire the provider module to return ValidatorUpdates, instead of the staking module (#1993) * Refactor validator set storage * Add comment for getTotalPower * Add provider consensus validator set storage * Add new MaxProviderConsensusValidators param * Add validation for MaxProviderConsensusValidators * Add no_valupdates_staking module * Add function to get MaxProviderConsensusValidators param * Start returning validators in EndBlock * Fix tests * Revert cosmetic change * Revert cosmetic changes * Revert formatting * Add genutil replacer module * Revert formatting * Revert formatting in tests/integration * Revert minor formatting * Fix type * Change wrapped staking to conform to EndBlocker interface * Fix typo * Revert "Fix typo" This reverts commit 62dfd1e. * Add e2e test for inactive vals * Start fixing e2e test * Revert formatting changes * Remove more formatting * Revert extra formatting * Re-wire provider/app.go to use wrapped modules * Remove consumer rewards check * Add inactive provider vals testcase to nightly * Adjust comment * Address comments * Fix nightly test name * feat: Initialize the max validators parameter for existing consumers (#2012) * Add initialization for validator cap * Remove migration test * Fix inconsistent naming * test: enable the simulator for the provider module (#2005) * Refactor validator set storage * Add comment for getTotalPower * Add provider consensus validator set storage * Add new MaxProviderConsensusValidators param * Add validation for MaxProviderConsensusValidators * Add no_valupdates_staking module * Add function to get MaxProviderConsensusValidators param * Start returning validators in EndBlock * Fix tests * Revert cosmetic change * Revert cosmetic changes * Revert formatting * Add genutil replacer module * Revert formatting * Revert formatting in tests/integration * Revert minor formatting * Fix type * Change wrapped staking to conform to EndBlocker interface * Fix typo * Revert "Fix typo" This reverts commit 62dfd1e. * Add e2e test for inactive vals * Start fixing e2e test * Revert formatting changes * Remove more formatting * Revert extra formatting * Re-wire provider/app.go to use wrapped modules * Remove consumer rewards check * Add simulator test * Add randomly generated parameters for provider in sim * Add invariant * Add simulation to Makefile and github workflow * Use simcli instead of just passing true * feat!: Let consumer chains choose a minimum stake and validator rank (#2035) * Add minimum stake key * Add MinValidatorRank prefix * Add keeper and tests for new parameters * Utilize MinStake and MaxRank parameters in computing next validators * Mention MinStake and MaxRank in adr * Add test for FulfillsMinStake * Handle multiple validators with same power * Add min stake and max rank to docs * Add minStake and maxRank to proposals * Check for untyped equality * Handle minStake and maxRank in Msgs * Add integration test for min stake and max rank * Rename test and testfile * Update docs/docs/adrs/adr-017-allowing-inactive-validators.md * Add changelog entries for maxrank and minstake * Address comments * Clarify which feature is disabled by setting maxrank * Test validator powers cap and validator set cap into int param testing function * feat!: Rewire dependencies on the staking module (#2056) * Change wiring for mint and gov to use ProviderKeeper instead of StakingKeeper * Add test for IterateBondedValidatorsByPower * Rewire GovKeeper * Fix docstrings * Test other modified functions * Start writing some testing scenarios * Add TotalBondedTokens to expected staking keeper interface * 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 * Delete hanging changelog entry * Address comments * Address more comments * Add migration for param * Fix allow inactive validators param test * Fix tests * Add LastProviderConsensusValidatorKey to fully defined keys * Fix key for validator set updates * Add info about genesis/endblock ordering * Add unit test for ProviderValidatorUpdates * Add example to proto definition of max_rank * Remove max rank * Remove references to max rank * Start adding an extension to the simulator * Make invariant fail early when param is 0 * Reorder InitGenesis to put Crisis last * Remove canary * Swap equals for not equals * Disable invariant check when max validators != max provider consensus validators * Make the simulator use a random seed * Remove TODO * Remove decoder * Run go mod tidy * Add migration in UPGRADING.md * Fix tests * Put random seed generation into golang code * Rename simulation jobs * Update UPGRADING.md Co-authored-by: Marius Poke <[email protected]> * Update UPGRADING.md Co-authored-by: Marius Poke <[email protected]> * Update x/ccv/provider/keeper/genesis.go Co-authored-by: Marius Poke <[email protected]> * Mention simulation tests in testing.md * Address some comments * Remake protos * Panic when LastActiveBondedValidators fails * Address some comments * Address comments * Reorder tests * Adjust stake_multiplier to stakeMultiplier * Address comments * Add error logging * Fix reference to bank blocked addrs in simulation * Change hasToValidate to only take into account active validators * Update docs/docs/adrs/adr-017-allowing-inactive-validators.md Co-authored-by: insumity <[email protected]> * Clarify: Slash happens on provider Co-authored-by: insumity <[email protected]> --------- Co-authored-by: Marius Poke <[email protected]> Co-authored-by: insumity <[email protected]>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Closes: Part of #1913
Adds a new parameter, which determines how many validators are supplied to the consensus engine
on the provider chain.
The parameter will be used in a followup PR, but is unused as of this PR.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!to the type prefix if the change is state-machine breakingCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
I have...
!the type prefix if the change is state-machine breakingSummary by CodeRabbit
New Features
Refactor
Tests