-
Notifications
You must be signed in to change notification settings - Fork 1.3k
Add cached PTCs to the state #4992
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
Changes from 2 commits
a99e003
215962a
f00224c
141a77a
3622fc2
69c1db1
b60f6f9
d76a278
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -54,6 +54,7 @@ | |
| - [New `compute_balance_weighted_selection`](#new-compute_balance_weighted_selection) | ||
| - [New `compute_balance_weighted_acceptance`](#new-compute_balance_weighted_acceptance) | ||
| - [Modified `compute_proposer_indices`](#modified-compute_proposer_indices) | ||
| - [New `compute_ptc`](#new-compute_ptc) | ||
| - [Beacon state accessors](#beacon-state-accessors) | ||
| - [Modified `get_next_sync_committee_indices`](#modified-get_next_sync_committee_indices) | ||
| - [Modified `get_attestation_participation_flag_indices`](#modified-get_attestation_participation_flag_indices) | ||
|
|
@@ -63,6 +64,7 @@ | |
| - [Beacon state mutators](#beacon-state-mutators) | ||
| - [New `initiate_builder_exit`](#new-initiate_builder_exit) | ||
| - [Beacon chain state transition function](#beacon-chain-state-transition-function) | ||
| - [Modified `process_slots`](#modified-process_slots) | ||
| - [Modified `process_slot`](#modified-process_slot) | ||
| - [Epoch processing](#epoch-processing) | ||
| - [Modified `process_epoch`](#modified-process_epoch) | ||
|
|
@@ -384,6 +386,8 @@ class BeaconState(Container): | |
| latest_block_hash: Hash32 | ||
| # [New in Gloas:EIP7732] | ||
| payload_expected_withdrawals: List[Withdrawal, MAX_WITHDRAWALS_PER_PAYLOAD] | ||
| # [New in Gloas:EIP7732] | ||
| ptc_lookbehind: Vector[Vector[ValidatorIndex, PTC_SIZE], 2] | ||
|
jtraglia marked this conversation as resolved.
Outdated
|
||
| ``` | ||
|
|
||
| ## Dataclasses | ||
|
|
@@ -630,6 +634,26 @@ def compute_proposer_indices( | |
| ] | ||
| ``` | ||
|
|
||
| #### New `compute_ptc` | ||
|
|
||
| ```python | ||
| def compute_ptc(state: BeaconState) -> Vector[ValidatorIndex, PTC_SIZE]: | ||
| """ | ||
| Get the payload timeliness committee for the current slot. | ||
| """ | ||
| epoch = get_current_epoch(state) | ||
| seed = hash(get_seed(state, epoch, DOMAIN_PTC_ATTESTER) + uint_to_bytes(state.slot)) | ||
| indices: List[ValidatorIndex] = [] | ||
| # Concatenate all committees for this slot in order | ||
| committees_per_slot = get_committee_count_per_slot(state, epoch) | ||
| for i in range(committees_per_slot): | ||
| committee = get_beacon_committee(state, state.slot, CommitteeIndex(i)) | ||
| indices.extend(committee) | ||
| return compute_balance_weighted_selection( | ||
| state, indices, seed, size=PTC_SIZE, shuffle_indices=False | ||
| ) | ||
| ``` | ||
|
|
||
| ### Beacon state accessors | ||
|
|
||
| #### Modified `get_next_sync_committee_indices` | ||
|
|
@@ -710,17 +734,8 @@ def get_ptc(state: BeaconState, slot: Slot) -> Vector[ValidatorIndex, PTC_SIZE]: | |
| """ | ||
| Get the payload timeliness committee for the given ``slot``. | ||
| """ | ||
| epoch = compute_epoch_at_slot(slot) | ||
| seed = hash(get_seed(state, epoch, DOMAIN_PTC_ATTESTER) + uint_to_bytes(slot)) | ||
| indices: List[ValidatorIndex] = [] | ||
| # Concatenate all committees for this slot in order | ||
| committees_per_slot = get_committee_count_per_slot(state, epoch) | ||
| for i in range(committees_per_slot): | ||
| committee = get_beacon_committee(state, slot, CommitteeIndex(i)) | ||
| indices.extend(committee) | ||
| return compute_balance_weighted_selection( | ||
| state, indices, seed, size=PTC_SIZE, shuffle_indices=False | ||
| ) | ||
| assert slot == state.slot or slot + 1 == state.slot | ||
|
jtraglia marked this conversation as resolved.
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this change makes I think we should accept slot def get_ptc(state: BeaconState, slot: Slot) -> Vector[ValidatorIndex, PTC_SIZE]:
epoch = get_current_epoch(state)
epoch_start_slot = compute_start_slot_at_epoch(epoch)
epoch_end_slot = epoch_state_slot + SLOTS_PER_EPOCH
assert slot >= state.slot - 1 and slot < epoch_end_slot
if slot == state.slot:
return state.current_ptc
if slot + 1 == state.slot:
return state.previous_ptc
return compute_ptc(state, slot) So
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. clients are free to simply pass the slot to OTOH one think that should be considered IMO is that if clients will anyway cache them, then most likely having the full cache in the spec is more efficient than keeping it in an ad-hoc in-memory cache that needs to be in-sync with the head state. |
||
| return state.ptc_lookbehind[1] if slot == state.slot else state.ptc_lookbehind[0] | ||
| ``` | ||
|
|
||
| #### New `get_indexed_payload_attestation` | ||
|
|
@@ -793,6 +808,21 @@ transitions that trigger an unhandled exception (e.g. a failed `assert` or an | |
| out-of-range list access) are considered invalid. State transitions that cause | ||
| an `uint64` overflow or underflow are also considered invalid. | ||
|
|
||
| ### Modified `process_slots` | ||
|
|
||
| ```python | ||
| def process_slots(state: BeaconState, slot: Slot) -> None: | ||
| assert state.slot < slot | ||
| while state.slot < slot: | ||
| process_slot(state) | ||
| # Process epoch on the start slot of the next epoch | ||
| if (state.slot + 1) % SLOTS_PER_EPOCH == 0: | ||
| process_epoch(state) | ||
| state.slot = Slot(state.slot + 1) | ||
| # [New in Gloas:EIP7732] | ||
| state.ptc_lookbehind = [state.ptc_lookbehind[1], compute_ptc(state)] | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Could be moved to |
||
| ``` | ||
|
|
||
| ### Modified `process_slot` | ||
|
|
||
| ```python | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -159,10 +159,14 @@ def upgrade_to_gloas(pre: fulu.BeaconState) -> BeaconState: | |
| latest_block_hash=pre.latest_execution_payload_header.block_hash, | ||
| # [New in Gloas:EIP7732] | ||
| payload_expected_withdrawals=[], | ||
| # [New in Gloas:EIP7732] | ||
| ptc_lookbehind=[[ValidatorIndex(0)] * PTC_SIZE, [ValidatorIndex(0)] * PTC_SIZE] | ||
|
jtraglia marked this conversation as resolved.
Outdated
|
||
| ) | ||
|
|
||
| # [New in Gloas:EIP7732] | ||
| onboard_builders_from_pending_deposits(post) | ||
| # [New in Gloas:EIP7732] | ||
| ptc_lookbehind[1] = compute_ptc(post) | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Added outside because |
||
|
|
||
| return post | ||
| ``` | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -48,29 +48,9 @@ validator" to implement Gloas. | |
| ### Payload timeliness committee | ||
|
|
||
| A validator may be a member of the new Payload Timeliness Committee (PTC) for a | ||
| given slot. To check for PTC assignments, use | ||
| `get_ptc_assignment(state, epoch, validator_index)` where `epoch <= next_epoch`, | ||
| as PTC committee selection is only stable within the context of the current and | ||
| next epoch. | ||
|
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This sentence was factually wrong which is what started this issue. Removed the helper entirely since there is no need to specify it. |
||
|
|
||
| ```python | ||
| def get_ptc_assignment( | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Leaving a note that |
||
| state: BeaconState, epoch: Epoch, validator_index: ValidatorIndex | ||
| ) -> Optional[Slot]: | ||
| """ | ||
| Returns the slot during the requested epoch in which the validator with | ||
| index ``validator_index`` is a member of the PTC. Returns None if no | ||
| assignment is found. | ||
| """ | ||
| next_epoch = Epoch(get_current_epoch(state) + 1) | ||
| assert epoch <= next_epoch | ||
|
|
||
| start_slot = compute_start_slot_at_epoch(epoch) | ||
| for slot in range(start_slot, start_slot + SLOTS_PER_EPOCH): | ||
| if validator_index in get_ptc(state, Slot(slot)): | ||
| return Slot(slot) | ||
| return None | ||
| ``` | ||
| given slot. Validators can check if their validator index is in the PTC for the current slot | ||
| by checking if their validator index is in `get_ptc(state)`. | ||
| PTC committee selection is only stable within the context of the current epoch. | ||
|
|
||
| ### Lookahead | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This doesn't seem to hold anymore.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. why not?
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right above it says:
which means there is no 1 epoch lookahead.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The committee selection is stable within the context of the current epoch, it doesn't mean that the epoch needs to be cached in the state. It means that any validator that wants to check if it has PTC duties in the current epoch, can do so at the beginning of the epoch. Clients will most likely implement this and cache it no matter which lookahead/lookbehind system we implement in-state.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Lookahead section states that PTC is stable for next epoch.
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. which section? I'm replying only to your comment that is based on the sentence:
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
Lookahead section. I left this comment on Lookahead section.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, yeah what Github shows is confusing.
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. L77-L78:
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. oh yeah that's definitely wrong. |
||
|
|
||
|
|
||
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 be
previous_ptc, current_ptcwhich is ugly as well, but more like previous forks structures.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.
I've just made this change. I prefer this.