Skip to content
This repository has been archived by the owner on Jan 6, 2025. It is now read-only.

Update validator shuffling #16

Merged
merged 1 commit into from
Feb 5, 2019

Conversation

drozdziak1
Copy link
Contributor

@drozdziak1 drozdziak1 commented Jan 30, 2019

Update validator shuffling

This PR updates and improves validator shuffling generation.

This most likely needs some polishing. There's a couple things I'm not happy
with yet:

  • The new epoch choice is too big - and therefore surrounded only by
    the active validators who didn't leave the set yet (no "soon-to-leaves" useful
    for delay edge case testing)
  • I did not consider the delays outlined by the spec

Original commit message (may change as I squash new things into it):

Update test vec generation and make it dynamic

This commit updates the validator shuffling generator and tries to make
the test validator sets more life-like.

constants.py:

  • Add FAR_FUTURE_EPOCH

core_helpers.py:

  • Update the helpers needed for validator shuffling

Remove enums.py: status flags are not used since 2018-12-28

tgen_shuffling.py:

  • Use a randomized probabilistic approach for generating configurably
    diverse validator sets; pre-FAR_FUTURE_EPOCH exit epochs truncated for
    readability
  • Change the YAML layout to include validator data needed for
    present-day shuffling

yaml_objects.py:

  • Change ValidatorRecord name to Validator (as in spec); change fields
    for up-to-date shuffling
  • Remove ShardCommittee (not used for shuffling at all as of today)

test_vector_shuffling.yml:

  • Regenerate

@drozdziak1 drozdziak1 force-pushed the 20190130-update branch 2 times, most recently from 51503b8 to 8eb6acd Compare January 31, 2019 13:07
@drozdziak1 drozdziak1 changed the title [WIP] Update validator shuffling Update validator shuffling Jan 31, 2019
eth2_testgen/shuffling/core_helpers.py Outdated Show resolved Hide resolved
eth2_testgen/shuffling/core_helpers.py Show resolved Hide resolved
for idx in range(idx_max):
v = Validator(original_index=idx)
# 4/5 of all validators are active
if random.randint(0, 4):
Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be a bit cleaner to use random.random() < 0.8, because it also allows fractions other than (n - 1) / n

Copy link
Contributor

Choose a reason for hiding this comment

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

Also, I think this would be easier to understand without all the nesting. So I suggest to pick a random number in the beginning, and then do something like this:

if r < 0.2:
    v.activation_epoch = FAR_FUTURE_EPOCH
    v.exit_epoch = FAR_FUTURE_EPOCH
elif r < 0.2 + 0.25:
    # validator will exit in foreseeable future
elif r < 0.45 + 0.25:
    # ...

(even though adding the probabilities feels a bit messy, there's probably a better pattern for this)

Copy link
Contributor Author

@drozdziak1 drozdziak1 Feb 4, 2019

Choose a reason for hiding this comment

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

I really like the floating number approach, included it in my latest push.

I'm not really sold on the elif with probability addition though. I don't think three-deep ifs are as scary.

This commit updates the validator shuffling generator and tries to make
the test validator sets more life-like.

constants.py:
* Add FAR_FUTURE_EPOCH
* Add ENTRY_EXIT_DELAY

core_helpers.py:
* Update the helpers needed for validator shuffling

Remove enums.py: status flags are not used since 2018-12-28

tgen_shuffling.py:
* Use a dynamic approach for generating diverse validator sets;
pre-FAR_FUTURE_EPOCH epochs are kept small for readability
* Change the YAML layout to include validator data needed for
present-day shuffling

yaml_objects.py:
* Change ValidatorRecord name to Validator (as in spec); change fields
for up-to-date shuffling
* Remove ShardCommittee (not used for shuffling at all as of today)

test_vector_shuffling.yml:
* Regenerate
@drozdziak1
Copy link
Contributor Author

FYI this is up-to-date with v0.1 now. Applied most of the advice from @jannikluhn

Copy link
Contributor

@ChihChengLiang ChihChengLiang left a comment

Choose a reason for hiding this comment

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

LGTM

@ChihChengLiang ChihChengLiang merged commit 4698786 into ethereum:master Feb 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants