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.

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

Add churn tests for when churn limit scales with v-set size #2586

Merged
merged 4 commits into from
Sep 9, 2021
Merged

Conversation

djrtwo
Copy link
Contributor

@djrtwo djrtwo commented Sep 8, 2021

We were missing tests for the churn limit when it begin to scales at the v-set size -- that is, when len(active_validators) >= CHURN_LIMIT_QUOTIENT * (1 + MIN_PER_VALIDATOR_CHURN_LIMIT). This doesn't happen on mainnet config until 327680 validators but better to get it in now than later!

To achieve this in a "minimal" way for our minimal config it required adjusting CHURN_LIMIT_QUOTIENT down from 65536 to 32. This allows for this scaled case to be triggered at 160 validators instead of ~300k. Although this would break any minimal testnets, as far as I know we do not have any persistent minimal testnets today and thus can safely modify this config for testing purposes.

This PR involves duplicating existing queue churn limit tests so that one test runs for min-churn-limit and one runs for scaled-churn-limit. get_validator_churn_limit is called within process_registry_updates and initiate_validator_exit so we identified tests in test_process_registry_updates and test_voluntary_exit for this scaled-churn-limit extension.

@djrtwo djrtwo requested a review from ralexstokes September 8, 2021 03:00
Copy link
Member

@ralexstokes ralexstokes left a comment

Choose a reason for hiding this comment

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

looks great and adds expanded test coverage!!

are you concerned about testing the scenario where the exit queue spans multiple epochs? the exit tests only consider "spill-over" into 1 epoch at the end of the exit queue -- we can imagine spilling over into several epochs after the end of the exit queue depending on the number of exits

i do see the "multi-epoch" case handled w/ forced ejections due to low balance so that may suffice for now...

@djrtwo djrtwo merged commit b660892 into dev Sep 9, 2021
@djrtwo djrtwo deleted the churn-test branch September 9, 2021 22:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants