Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

feat: Initialize the max validators parameter for existing consumers #2012

Merged
merged 3 commits into from
Jul 15, 2024
Merged

feat: Initialize the max validators parameter for existing consumers #2012

merged 3 commits into from
Jul 15, 2024

Conversation

p-offtermatt
Copy link
Contributor

Description

Closes: Part of #1913

Out of an abundance of caution, I believe we should initialize
the validator set size cap to a value for all existing chains.
Otherwise, with the inactive validators feature, they might get an excessive amount of validators opting in.

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

  • Included the correct type prefix in the PR title
  • Added ! to the type prefix if the change is state-machine breaking
  • Confirmed this PR does not introduce changes requiring state migrations, OR migration code has been added to consumer and/or provider modules
  • Targeted the correct branch (see PR Targeting)
  • Provided a link to the relevant issue or specification
  • Followed the guidelines for building SDK modules
  • Included the necessary unit and integration tests
  • Added a changelog entry to CHANGELOG.md
  • Included comments for documenting Go code
  • Updated the relevant documentation or specification
  • Reviewed "Files changed" and left comments if necessary
  • Confirmed all CI checks have passed
  • If this PR is library API breaking, bump the go.mod version string of the repo, and follow through on a new major release

Reviewers 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...

  • confirmed the correct type prefix in the PR title
  • confirmed ! the type prefix if the change is state-machine breaking
  • confirmed this PR does not introduce changes requiring state migrations, OR confirmed migration code has been added to consumer and/or provider modules
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage

@p-offtermatt p-offtermatt requested a review from a team as a code owner July 4, 2024 07:02
@github-actions github-actions bot added the C:x/provider Assigned automatically by the PR labeler label Jul 4, 2024
insumity
insumity approved these changes Jul 4, 2024
View reviewed changes
Copy link
Contributor

@insumity insumity left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be good to inform upcoming consumer chains about this as well. E.g., are evmos and elys going to have a validator set cap? I believe elys does, but not sure about evmos.

@p-offtermatt
Copy link
Contributor Author

Good point, we should make sure that they are aware.

@p-offtermatt p-offtermatt merged commit 60e9e7f into ph/inactive-vals-endblocker Jul 15, 2024
8 of 12 checks passed
@p-offtermatt p-offtermatt deleted the ph/max-consumer-validators-migration branch July 15, 2024 11:30
p-offtermatt added a commit that referenced this pull request Jul 15, 2024
@p-offtermatt
feat!: Wire the provider module to return ValidatorUpdates, instead o…
757680b 
…f 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
p-offtermatt added a commit that referenced this pull request Jul 22, 2024
@p-offtermatt
feat!: Wire the provider module to return ValidatorUpdates, instead o…
e71129e 
…f 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
github-merge-queue bot pushed a commit that referenced this pull request Aug 6, 2024
@p-offtermatt @mpoke @insumity
feat!: Allow inactive provider chain validators to validate on consum…
0e080c3 
…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
Reviewers

@MSalopek MSalopek MSalopek approved these changes

@insumity insumity insumity approved these changes

Assignees
No one assigned
Labels
C:x/provider Assigned automatically by the PR labeler
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@p-offtermatt @insumity @MSalopek