-
Notifications
You must be signed in to change notification settings - Fork 1.1k
EIP-7917: Deterministic proposer lookahead #4190
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
Merged
jtraglia
merged 39 commits into
ethereum:dev
from
linoscope:stabalize-next-epoch-lookahead
Jun 3, 2025
Merged
Changes from all commits
Commits
Show all changes
39 commits
Select commit
Hold shift + click to select a range
e73b5d8
EIP-XXXX Stabilize next epoch proposer lookahead
linoscope fb3cd63
EIP-XXXX add simple happy case tests
linoscope 3fbe39c
fixup: Code clean up
linoscope 0a37fe7
fixup: Target correct fork
linoscope a4dd9eb
Merge branch 'dev' into stabalize-next-epoch-lookahead
jtraglia 9b82cb4
Clean up comment
linoscope 7485399
refactor based on review comments
linoscope 6a1d923
fix fork check in genesis helper
linoscope 8fb1a58
improve comments
linoscope 208f368
fix off-by-one error
linoscope af1892b
fix bug in seed calculation
linoscope 53df021
add proper EIP number (EIP7917)
linoscope 94d9dde
fix lint errors
linoscope 83daa1e
add test to check lookahead consistency before/after Fulu fork
linoscope b1b0144
add tests for lookahead changes due to EB changes
linoscope 9610b02
fix test folder
linoscope 898432d
fix import
linoscope 0414e6c
fix imports
linoscope 78b3810
add generator EIP-7917 tests
linoscope 291bed7
Merge branch 'dev' into stabalize-next-epoch-lookahead
jtraglia a54040e
Run make lint
jtraglia cede1ae
better yielding of vectors
linoscope 3c9cf75
move helper and fix type
linoscope ec0243d
Merge branch 'dev' into stabalize-next-epoch-lookahead
jtraglia 835df3d
Run make lint
jtraglia 1860d06
move state accessors outside of function
linoscope faaf52b
use helper in test
linoscope dfce6e4
Move some tests to the `sanity` format to generate vectors for EIP-7917
leolara 016b45d
Merge remote-tracking branch 'origin/dev' into stabalize-next-epoch-l…
linoscope 98adbee
Merge branch 'dev' into stabalize-next-epoch-lookahead
jtraglia a53408e
Run make lint
jtraglia ef950a6
Make test less reliant on specific RANDAO
linoscope 9d5a6be
Break up two long lines
jtraglia c17c9dd
Use double backticks in docstrings
jtraglia ac6b213
fix yield statement in test
linoscope c8811a0
Change proposer_lookahead type from List to Vector
linoscope e9266b2
Compute seed and indicies inside compute_proposer_indicies
linoscope 79bf80d
refactor and introduce get_beacon_proposer_indices
linoscope 5874370
remove trailing whitespaces
linoscope File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
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
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
Empty file.
43 changes: 43 additions & 0 deletions
43
tests/core/pyspec/eth2spec/test/fulu/epoch_processing/test_process_proposer_lookahead.py
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,43 @@ | ||
from eth2spec.test.context import spec_state_test, with_fulu_and_later | ||
from eth2spec.test.helpers.epoch_processing import run_epoch_processing_with | ||
from eth2spec.test.helpers.state import next_epoch | ||
|
||
|
||
@with_fulu_and_later | ||
@spec_state_test | ||
def test_next_epoch_proposer_lookahead_shifted_to_front(spec, state): | ||
"""Test that the next epoch proposer lookahead is shifted to the front at epoch transition.""" | ||
# Transition few epochs to pass the MIN_SEED_LOOKAHEAD | ||
next_epoch(spec, state) | ||
next_epoch(spec, state) | ||
# Get initial lookahead | ||
initial_lookahead = state.proposer_lookahead.copy() | ||
|
||
# Run epoch processing | ||
yield "pre", state | ||
next_epoch(spec, state) | ||
yield "post", state | ||
|
||
# Verify lookahead was shifted correctly | ||
assert ( | ||
state.proposer_lookahead[: spec.SLOTS_PER_EPOCH] | ||
== initial_lookahead[spec.SLOTS_PER_EPOCH :] | ||
) | ||
|
||
|
||
@with_fulu_and_later | ||
@spec_state_test | ||
def test_proposer_lookahead_in_state_matches_computed_lookahead(spec, state): | ||
"""Test that the proposer lookahead in the state matches the lookahead computed on the fly.""" | ||
# Transition few epochs to pass the MIN_SEED_LOOKAHEAD | ||
next_epoch(spec, state) | ||
next_epoch(spec, state) | ||
|
||
# Run epoch processing | ||
yield "pre", state | ||
next_epoch(spec, state) | ||
yield "post", state | ||
|
||
# Verify lookahead in state matches the lookahead computed on the fly | ||
computed_lookahead = spec.initialize_proposer_lookahead(state) | ||
assert state.proposer_lookahead == computed_lookahead |
37 changes: 37 additions & 0 deletions
37
tests/core/pyspec/eth2spec/test/fulu/fork/test_fork_lookahead_consistency.py
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,37 @@ | ||
from eth2spec.test.context import ( | ||
spec_test, | ||
with_phases, | ||
with_state, | ||
) | ||
from eth2spec.test.helpers.constants import ( | ||
ELECTRA, | ||
FULU, | ||
) | ||
from eth2spec.test.helpers.fulu.fork import ( | ||
FULU_FORK_TEST_META_TAGS, | ||
run_fork_test, | ||
) | ||
from eth2spec.test.helpers.state import next_slot | ||
from eth2spec.test.utils import with_meta_tags | ||
from tests.core.pyspec.eth2spec.test.helpers.state import simulate_lookahead | ||
|
||
|
||
@with_phases(phases=[ELECTRA], other_phases=[FULU]) | ||
@spec_test | ||
@with_state | ||
@with_meta_tags(FULU_FORK_TEST_META_TAGS) | ||
def test_lookahead_consistency_at_fork(spec, phases, state): | ||
""" | ||
Test that lookahead is consistent before/after the Fulu fork. | ||
""" | ||
|
||
# Calculate the current and next epoch lookahead by simulating the state progression | ||
# with empty slots and calling `get_beacon_proposer_index` (how it was done pre-Fulu) | ||
pre_fork_proposers = simulate_lookahead(spec, state) | ||
|
||
# Upgrade to Fulu | ||
spec = phases[FULU] | ||
state = yield from run_fork_test(spec, state) | ||
|
||
# Check if the pre-fork simulation matches the post-fork `state.proposer_lookahead` | ||
assert pre_fork_proposers == state.proposer_lookahead |
Empty file.
90 changes: 90 additions & 0 deletions
90
...ore/pyspec/eth2spec/test/fulu/sanity/test_effective_balance_increase_changes_lookahead.py
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
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,90 @@ | ||
from eth2spec.test.context import ( | ||
spec_state_test, | ||
with_phases, | ||
) | ||
from eth2spec.test.helpers.attestations import ( | ||
state_transition_with_full_block, | ||
) | ||
from eth2spec.test.helpers.constants import ELECTRA, FULU | ||
from eth2spec.test.helpers.state import ( | ||
next_epoch, | ||
simulate_lookahead, | ||
) | ||
from eth2spec.test.helpers.withdrawals import ( | ||
set_compounding_withdrawal_credential, | ||
) | ||
|
||
|
||
def run_test_effective_balance_increase_changes_lookahead( | ||
spec, state, randao_setup_epochs, expect_lookahead_changed | ||
): | ||
# Advance few epochs to adjust the RANDAO | ||
for _ in range(randao_setup_epochs): | ||
next_epoch(spec, state) | ||
|
||
# Set all active validators to have balance close to the hysteresis threshold | ||
current_epoch = spec.get_current_epoch(state) | ||
active_validator_indices = spec.get_active_validator_indices(state, current_epoch) | ||
for validator_index in active_validator_indices: | ||
# Set compounding withdrawal credentials for the validator | ||
set_compounding_withdrawal_credential(spec, state, validator_index) | ||
state.validators[validator_index].effective_balance = 32000000000 | ||
# Set balance to close the next hysteresis threshold | ||
state.balances[validator_index] = 33250000000 - 1 | ||
|
||
# Calculate the lookahead of next epoch | ||
next_epoch_lookahead = simulate_lookahead(spec, state)[spec.SLOTS_PER_EPOCH :] | ||
|
||
blocks = [] | ||
yield "pre", state | ||
|
||
# Process 1-epoch worth of blocks with attestations | ||
for _ in range(spec.SLOTS_PER_EPOCH): | ||
block = state_transition_with_full_block( | ||
spec, state, fill_cur_epoch=True, fill_prev_epoch=True | ||
) | ||
blocks.append(block) | ||
|
||
yield "blocks", blocks | ||
yield "post", state | ||
|
||
# Calculate the actual lookahead | ||
actual_lookahead = simulate_lookahead(spec, state)[: spec.SLOTS_PER_EPOCH] | ||
|
||
if expect_lookahead_changed: | ||
assert next_epoch_lookahead != actual_lookahead | ||
else: | ||
assert next_epoch_lookahead == actual_lookahead | ||
|
||
|
||
def run_test_with_randao_setup_epochs(spec, state, randao_setup_epochs): | ||
if spec.fork == ELECTRA: | ||
# Pre-EIP-7917, effective balance changes due to attestation rewards | ||
# changes the next epoch's lookahead | ||
expect_lookahead_changed = True | ||
else: | ||
# Post-EIP-7917, effective balance changes due to attestation rewards | ||
# do not change the next epoch's lookahead | ||
expect_lookahead_changed = False | ||
|
||
yield from run_test_effective_balance_increase_changes_lookahead( | ||
spec, state, randao_setup_epochs, expect_lookahead_changed=expect_lookahead_changed | ||
) | ||
|
||
|
||
@with_phases(phases=[ELECTRA, FULU]) | ||
@spec_state_test | ||
def test_effective_balance_increase_changes_lookahead(spec, state): | ||
# Since this test relies on the RANDAO, we adjust the number of next_epoch transitions | ||
# we do at the setup of the test run until the assertion passes. | ||
# We start with 4 epochs because the test is known to pass with 4 epochs. | ||
for randao_setup_epochs in range(4, 20): | ||
try: | ||
state_copy = state.copy() | ||
yield from run_test_with_randao_setup_epochs(spec, state_copy, randao_setup_epochs) | ||
return | ||
except AssertionError: | ||
# If the randao_setup_epochs is not the right one to make the test pass, | ||
# then try again in the next iteration | ||
pass | ||
assert False, "The test should have succeeded with one of the iterations." |
Oops, something went wrong.
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.
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.
All calls to this function explicitly compute
seed
at the callsite, it is cleaner to have a different signature only taking the state and epoch and computing the seed within this function.Moreover, the same applies to the indices argument, this function should be defined as follows
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.
Current interface was from a suggestion to make it more in line with the interface of
compute_proposer_index
, which takes theseed
andindices
in the argument. Personally don't have strong preference either way, but I do see the benefit of reducing repeated logic in callsites, so right now slightly in favor of reverting back tocompute_proposer_indices(state: BeaconState, epoch: Epoch)
interface as @potuz suggests. What do you think @gfukushima ?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.
In the current spec,
compute_proposer_index()
is meant to be a private/internal/utility function that is only used byget_beacon_proposer_index()
.The usage of
compute_proposer_indices()
is more similar toget_beacon_proposer_index()
thancompute_proposer_index()
. Delegating the seed calculation to the caller is not really necessary imo.If we really want to be consistent with the current spec,
we should have
get_beacon_proposer_indices(state, epoch)
which callscompute_proposer_indices (state, epoch, seed)
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.
Thanks both for the input!
Makes sense.
Made the call to circle back to having
seeds
andindicies
incompute_proposer_indicies
here: e9266b2Uh oh!
There was an error while loading. Please reload this page.
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.
Agree with @ensi321 what I was trying to tell you with my initial comment is that the initial implementation was introducing a circular dependency, having a method in the misc helpers (
compute_proposer_indices
) depending on methods of the beacon state accessor (get_seed
,get_active_validator_indices
) would be unusual (if not a bad practice). It is usually the other way around, beacon state accessor can call misc helpers methods. Having said that I think @ensi321 suggestion is the closest I would agree with but with a slight modification:get_beacon_proposer_indices(state, epoch)
should callcompute_proposer_indices (state, epoch, seed, indices)
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.
Thanks for the explanation, I see your point, makes sense. Will make this change to remove the circular dependency.
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.
Refactor and introduction of
get_beacon_proposer_indices
accessor done here:79bf80d
Thanks everyone for the input!