-
Notifications
You must be signed in to change notification settings - Fork 920
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
Persistent committee size per slot reduced to max 128 #734
Conversation
Cuts down the cost of verifying a shard chain and aggregating signatures for a shard chain, and also makes the shard chain signatures more usable by light clients for verification as they would only need to keep track of a max 256-sized committee.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Only two questions here. 🙂
@@ -147,15 +168,24 @@ def get_shuffled_committee(state: BeaconState, | |||
```python | |||
def get_persistent_committee(state: BeaconState, | |||
shard: Shard, | |||
epoch: Epoch) -> List[ValidatorIndex]: | |||
slot: Slot) -> List[ValidatorIndex]: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we add an assertion for the valid domain range of the givenslot
?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Technically we don't need to because generate_seed
and specifically get_randao_mix
and get_active_index_root
inside there are doing the check. I suppose we could add a comment saying how far back it can check.
(SHARD_COUNT * TARGET_COMMITTEE_SIZE), | ||
len(get_active_validator_indices(state.validator_registry, later_start_epoch)) // | ||
(SHARD_COUNT * TARGET_COMMITTEE_SIZE), | ||
) + 1 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you explain why "committee_count adds 1 instead of maxing 1" commit? Or what you wanted was a ceil
function here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal is to have a committee size which is always not greater than TARGET_COMMITTEE_SIZE
; max(1, ...)
does not do that, ,1 + ...
does. So yeah, ceil would work too, but it doesn't make a big difference either way.
* Persistent committee size per slot target 128 max 256 Cuts down the cost of verifying a shard chain and aggregating signatures for a shard chain, and also makes the shard chain signatures more usable by light clients for verification as they would only need to keep track of a max 256-sized committee.
Cuts down the cost of verifying a shard chain and aggregating signatures for a shard chain, and also makes the shard chain signatures more usable by light clients for verification as they would only need to keep track of a max 256-sized committee.