-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Attestation committee refactor #1009
Conversation
* Remove `get_crosslink_committees_at_slot` (that function's ugly man...) * Make the "base" that everything works off instead be `get_crosslink_committee` * Attestations store epoch, start shard and shard, no longer slot (slot can be calculated from the other three) * Retaining start shard in attestations allows `get_attesting_indices` to peek much further back into the past, making it useful for slashings (Phase 1) * Some two-layer-deep nested loops become one-layer-deep loops
I'm working on some edits based on the above feedback |
and now working on some bugs :) |
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.
@JustinDrake if we remove get_state_root
helper, then I'd like to do it in a separate PR with an explanation rather than it just disappearing at some point
I still am not fully comfortable with Here is the usage in phase 1: https://github.com/ethereum/eth2.0-specs/blob/dev/specs/core/1_shard-data-chains.md#get_period_committee One possible compromise I see would be to define a function: def get_indices_of_chunk(list_size: int, chunks: int, chunk: int) -> List[int]:
start = (len(list_size) * chunk) // chunks
end = (len(list_size) * (chunk + 1)) // chunks
return list(range(start, end)) Then the PR would be, in return [
active_indices[get_shuffled_index(i, len(active_indices), seed)]
for i in get_indices_of_chunk(len(active_indices), committee_count, committee_index)
] And the same in |
@vbuterin: I've exposed |
Somewhat cleaner, both conceptually and in terms of presentation. (This is a draft for early feedback—to be fully implemented after #1009 is merged.)
I think the previous function signature of I vote going back to that function signature or something similar |
Agreed. |
|
Mostly cosmetic and enabling some functionality needed in phase 1. A few substantive changes to
Attestation
data structure.get_crosslink_committees_at_slot
(that function's ugly man...)get_crosslink_committee
(simpler and more generic)inclusion_distance
inPendingAttestation
rather thaninclusion_slot
for more direct use in subsequent calculations.get_attesting_indices
to peek much further back into the past, making it useful for slashings (Phase 1)get_winning_crosslink_and_attesting_indices
function signature to mirror that ofget_crosslink_committee
is_double_vote
andis_surround_vote
replaced with singleis_slashable_attestation_data