Introduce compute_shuffling_lookahead_start_slot helper - #5602
Merged
Conversation
compute_shuffling_decision_slot helpercompute_shuffling_decision_slot helper
jtraglia
reviewed
Sep 4, 2026
Comment on lines
+723
to
+727
| def compute_shuffling_decision_slot(epoch: Epoch) -> Slot: | ||
| if epoch <= MIN_SEED_LOOKAHEAD: | ||
| return GENESIS_SLOT | ||
| return compute_start_slot_at_epoch(epoch - MIN_SEED_LOOKAHEAD) | ||
| ``` |
Member
There was a problem hiding this comment.
Naming things is tough. I'm thinking we should do:
compute_lookahead_start_slot(preferred)compute_shuffling_lookahead_start_slot
I've looked through the usages below & I believe this to be fine. I do not believe it is necessary to mention "shuffling" here.
Member
Author
There was a problem hiding this comment.
I'd go with compute_shuffling_lookahead_start_slot. I believe it should mention shuffling; otherwise, it's ambiguous which lookahead is being referred to. For instance, the lookahead for proposer has changed post-Fulu. See #5374 for more context.
jihoonsong
force-pushed
the
shuffling-decision-slot
branch
from
September 4, 2026 16:19
e9dbb6b to
6d8cd21
Compare
compute_shuffling_decision_slot helpercompute_shuffling_lookahead_start_slot helper
jtraglia
approved these changes
Sep 4, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
shuffling_decision_slotis the next slot todependent_slot, except for the first two epochs. For GENESIS_EPOCH and GENESIS_EPOCH + MIN_SEED_LOOKAHEAD, it's equal todependent_slot, which is GENESIS_SLOT. I've found this useful for advancing dependent state just as necessary as to advance proposer lookahead.As for the concern of advancing state, my interpretation of this chunk of code is to fetch and use cached proposer lookahead. That said, I'm open to a better expression than what we have.
I stole the name from
is_previous_slot_payload_decision. Other names that I considered are:IMO, known and available are ambiguous because it can be any slot in current and past epochs. It should mean the first slot of the previous epoch.
Another way is introducing
compute_shuffling_lookahead_epochand doingcompute_start_slot_at_epoch(compute_shuffling_lookahead_epoch(epoch)). But I think adding a helper returning the start slot of lookahead epoch brings us slightly more value.