-
-
Notifications
You must be signed in to change notification settings - Fork 461
feat: implement EIP-8045 exclude slashed validators from proposing #9422
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 all commits
1f072a1
689969e
fafebf5
6251c12
feb4e4e
c1f629b
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 |
|---|---|---|
|
|
@@ -630,9 +630,17 @@ const forkChoiceTest = | |
| name.includes("include_votes_another_empty_chain_with_enough_ffg_votes_current_epoch") || | ||
| name.includes("include_votes_another_empty_chain_with_enough_ffg_votes_previous_epoch") || | ||
| name.includes("include_votes_another_empty_chain_without_enough_ffg_votes_current_epoch"))) || | ||
| // TODO GLOAS: Spec test fixture bug in v1.7.0-alpha.5: wrong_withdrawals envelope SSZ data is | ||
| // byte-for-byte identical to the valid envelope, making it impossible to reject | ||
| name.endsWith("on_execution_payload_envelope__wrong_withdrawals"), | ||
| // TODO: re-enable after "apply proposer boost if dependent roots match" (consensus-specs #5306) | ||
| // is implemented. New behavior in v1.7.0-alpha.9; Lodestar still applies the boost | ||
| // unconditionally. Only the altair vectors exercise the changed condition (other forks pass). | ||
| name.endsWith("altair/fork_choice/on_block/pyspec_tests/justified_update_always_if_better") || | ||
| name.endsWith("altair/fork_choice/on_block/pyspec_tests/justified_update_not_realized_finality") || | ||
| name.endsWith("altair/fork_choice/get_head/pyspec_tests/voting_source_beyond_two_epoch") || | ||
|
Member
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.
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. Confirmed — these 3 altair vectors were added in consensus-specs #5306 (merged 2026-06-01) to switch proposer boost from "same proposer index" to "same dependent root". Spec diff in # Before
if block.proposer_index == get_beacon_proposer_index(head_state):
store.proposer_boost_root = root
# After (#5306)
is_same_dependent_root = get_dependent_root(store, root) == get_dependent_root(store, head)
if is_timely and is_first_block and is_same_dependent_root:
store.proposer_boost_root = rootLodestar's check is at |
||
| // TODO GLOAS: re-enable after the validate_on_attestation payload-status updates | ||
| // (consensus-specs #5275) are implemented. New gloas on_attestation vectors in v1.7.0-alpha.9. | ||
| name.endsWith("validate_on_attestation_beacon_root_payload_check") || | ||
| name.endsWith("validate_on_attestation_payload_invalid_index") || | ||
| name.endsWith("validate_on_attestation_same_slot_full_vote_rejected"), | ||
|
Member
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. a bit confused about these but can be addressed in a follow up
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. Yeah, these 3 are the new gloas
Same |
||
| }, | ||
| }; | ||
| }; | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -72,7 +72,6 @@ export const defaultSkipOpts: SkipOpts = { | |
| /^electra\/light_client\/single_merkle_proof\/BeaconBlockBody.*/, | ||
| /^fulu\/light_client\/single_merkle_proof\/BeaconBlockBody.*/, | ||
| /^.+\/light_client\/data_collection\/.*/, | ||
| /^gloas\/ssz_static\/ForkChoiceNode.*$/, | ||
| // Ignore the partial data column container additions for now. Unskip them when | ||
| // cell level DAS is ready | ||
| /^fulu\/ssz_static\/PartialDataColumn(Header|PartsMetadata|Sidecar)\/.*$/, | ||
|
|
@@ -84,6 +83,11 @@ export const defaultSkipOpts: SkipOpts = { | |
| // New test suite added in v1.7.0-alpha.8 (consensus-specs #5206); gloas PTC fork choice | ||
| // handling is not yet implemented in Lodestar. | ||
| /^gloas\/fork_choice\/on_payload_attestation_message\/.*$/, | ||
| // TODO-FULU: re-enable after the Fulu deposit-mechanism removal (consensus-specs #4704) is | ||
| // implemented. v1.7.0-alpha.9 regenerated these transition vectors so eth1-bridge deposits are | ||
| // no longer processed across the Electra to Fulu boundary; Lodestar still applies the Electra | ||
| // deposit flow, diverging the post-fork state root. | ||
| /^fulu\/transition\/.*/, | ||
|
Member
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. there's more code we might be able to remove due to ethereum/consensus-specs#4704, can be done in separate PR
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. Confirmed. #4704 drops the legacy eth1-bridge deposit path in Fulu+:
Lodestar mirror:
Will unskip |
||
| ], | ||
| skippedTests: [ | ||
| // TODO-GLOAS: re-enable after gloas light client is implemented | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,6 +1,7 @@ | ||
| import {ForkSeq, MIN_SEED_LOOKAHEAD, SLOTS_PER_EPOCH} from "@lodestar/params"; | ||
| import {ssz} from "@lodestar/types"; | ||
| import {CachedBeaconStateFulu, EpochTransitionCache} from "../types.js"; | ||
| import {FLAG_UNSLASHED} from "../util/attesterStatus.js"; | ||
| import {computeEpochShuffling} from "../util/epochShuffling.js"; | ||
| import {computeProposerIndices} from "../util/seed.js"; | ||
|
|
||
|
|
@@ -26,7 +27,13 @@ export function processProposerLookahead( | |
| // Save shuffling to cache so afterProcessEpoch can reuse it instead of recomputing | ||
| cache.nextShuffling = shuffling; | ||
|
|
||
| const lastEpochProposerLookahead = computeProposerIndices(fork, state, shuffling, epoch); | ||
| const activeIndices = | ||
| fork >= ForkSeq.gloas | ||
| ? // Exclude slashed validators from proposing (EIP-8045) | ||
| shuffling.activeIndices.filter((index) => (cache.flags[index] & FLAG_UNSLASHED) !== 0) | ||
| : shuffling.activeIndices; | ||
|
|
||
| const lastEpochProposerLookahead = computeProposerIndices(fork, state, {activeIndices}, epoch); | ||
|
Comment on lines
-29
to
+36
Member
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. for reference, that's the only real change in this PR
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. 👍 Yep, this 7-line filter is the entire EIP-8045 surface — |
||
|
|
||
| state.proposerLookahead = ssz.fulu.ProposerLookahead.toViewDU([ | ||
| ...remainingProposerLookahead, | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,4 +1,4 @@ | ||
| version: v1.7.0-alpha.8 | ||
| version: v1.7.0-alpha.9 | ||
| style: full | ||
|
|
||
| specrefs: | ||
|
|
@@ -25,6 +25,7 @@ exceptions: | |
| - UINT64_MAX_SQRT#phase0 | ||
|
|
||
| # bellatrix | ||
| - EMPTY_BLOCK_HASH#bellatrix | ||
| - PAYLOAD_STATUS_INVALIDATED#bellatrix | ||
| - PAYLOAD_STATUS_NOT_VALIDATED#bellatrix | ||
| - PAYLOAD_STATUS_VALID#bellatrix | ||
|
|
@@ -61,7 +62,6 @@ exceptions: | |
|
|
||
| containers: | ||
| # gloas | ||
| - ForkChoiceNode#gloas | ||
| - LightClientHeader#gloas | ||
| - PartialDataColumnGroupID#gloas | ||
| - PartialDataColumnHeader#gloas | ||
|
|
@@ -81,6 +81,7 @@ exceptions: | |
|
|
||
| dataclasses: | ||
| # phase0 | ||
| - ForkChoiceNode#phase0 | ||
| - LatestMessage#phase0 | ||
| - Seen#phase0 | ||
|
|
||
|
|
@@ -99,6 +100,9 @@ exceptions: | |
| # electra | ||
| - Seen#electra | ||
|
|
||
| # fulu | ||
| - Seen#fulu | ||
|
|
||
| # capella | ||
| - ExpectedWithdrawals#capella | ||
|
|
||
|
|
@@ -107,7 +111,9 @@ exceptions: | |
|
|
||
| # gloas | ||
| - ExpectedWithdrawals#gloas | ||
| - ForkChoiceNode#gloas | ||
| - LatestMessage#gloas | ||
| - Seen#gloas | ||
| - Store#gloas | ||
|
|
||
| # phase0 fast confirmation / not implemented | ||
|
|
@@ -146,10 +152,14 @@ exceptions: | |
| - get_base_reward#phase0 | ||
| - get_checkpoint_block#phase0 | ||
| - get_current_store_epoch#phase0 | ||
| - get_dependent_root#phase0 | ||
|
Member
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. worth to revisit this, we should be able to point this to the proper function in our code
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. Good catch — followed the same pattern as
Diff (apply with diff --git a/packages/state-transition/src/cache/epochTransitionCache.ts b/packages/state-transition/src/cache/epochTransitionCache.ts
--- a/packages/state-transition/src/cache/epochTransitionCache.ts
+++ b/packages/state-transition/src/cache/epochTransitionCache.ts
@@ -296,6 +296,16 @@ export function beforeProcessEpoch(
// Both active validators and slashed-but-not-yet-withdrawn validators are eligible to receive penalties.
// This is done to prevent self-slashing from being a way to escape inactivity leaks.
// TODO: Consider using an array of `eligibleValidatorIndices: number[]`
+ // ```python
+ // def get_eligible_validator_indices(state: BeaconState) -> Sequence[ValidatorIndex]:
+ // previous_epoch = get_previous_epoch(state)
+ // return [
+ // ValidatorIndex(index)
+ // for index, v in enumerate(state.validators)
+ // if is_active_validator(v, previous_epoch)
+ // or (v.slashed and previous_epoch + 1 < v.withdrawable_epoch)
+ // ]
+ // ```
if (isActivePrev || (validator.slashed && prevEpoch + 1 < validator.withdrawableEpoch)) {
flag |= FLAG_ELIGIBLE_ATTESTER;
}
diff --git a/specrefs/.ethspecify.yml b/specrefs/.ethspecify.yml
--- a/specrefs/.ethspecify.yml
+++ b/specrefs/.ethspecify.yml
@@ -153,7 +153,6 @@ exceptions:
- get_checkpoint_block#phase0
- get_current_store_epoch#phase0
- get_dependent_root#phase0
- - get_eligible_validator_indices#phase0
- get_eth1_vote#phase0
- get_filtered_block_tree#phase0
- get_forkchoice_store#phase0
diff --git a/specrefs/functions.yml b/specrefs/functions.yml
--- a/specrefs/functions.yml
+++ b/specrefs/functions.yml
@@ -3630,7 +3630,9 @@
</spec>
- name: get_eligible_validator_indices#phase0
- sources: []
+ sources:
+ - file: packages/state-transition/src/cache/epochTransitionCache.ts
+ search: "// def get_eligible_validator_indices(state: BeaconState) -> Sequence[ValidatorIndex]:"
spec: |
<spec fn="get_eligible_validator_indices" fork="phase0" hash="22a24ce4">
def get_eligible_validator_indices(state: BeaconState) -> Sequence[ValidatorIndex]: |
||
| - get_eligible_validator_indices#phase0 | ||
| - get_eth1_vote#phase0 | ||
| - get_filtered_block_tree#phase0 | ||
| - get_forkchoice_store#phase0 | ||
| - get_node_children#phase0 | ||
| - get_node_for_root#phase0 | ||
| - get_supported_node#phase0 | ||
| - get_head_deltas#phase0 | ||
| - get_inactivity_penalty_deltas#phase0 | ||
| - get_inclusion_delay_deltas#phase0 | ||
|
|
@@ -210,6 +220,7 @@ exceptions: | |
| - compute_fork_version#bellatrix | ||
| - get_execution_payload#bellatrix | ||
| - get_pow_block_at_terminal_total_difficulty#bellatrix | ||
| - get_safe_execution_block_hash#bellatrix | ||
| - get_terminal_pow_block#bellatrix | ||
| - is_execution_block#bellatrix | ||
| - is_merge_transition_block#bellatrix | ||
|
|
@@ -319,6 +330,9 @@ exceptions: | |
| - verify_cell_kzg_proof_batch_impl#fulu | ||
| - verify_partial_data_column_header_inclusion_proof#fulu | ||
| - verify_partial_data_column_sidecar_kzg_proofs#fulu | ||
| - validate_beacon_block_gossip#fulu | ||
| - validate_data_column_sidecar_gossip#fulu | ||
| - validate_partial_data_column_sidecar_gossip#fulu | ||
|
|
||
| # gloas | ||
| - add_builder_to_registry#gloas | ||
|
|
@@ -329,31 +343,37 @@ exceptions: | |
| - get_ancestor#gloas | ||
| - get_attestation_participation_flag_indices#gloas | ||
| - get_attestation_score#gloas | ||
| - get_beacon_proposer_indices#gloas | ||
| - get_builder_withdrawals#gloas | ||
| - get_builders_sweep_withdrawals#gloas | ||
| - get_checkpoint_block#gloas | ||
| - get_dependent_root#gloas | ||
| - get_execution_payload_bid_signature#gloas | ||
| - get_execution_payload_envelope_signature#gloas | ||
| - get_forkchoice_store#gloas | ||
| - get_head#gloas | ||
| - get_index_for_new_builder#gloas | ||
| - get_next_sync_committee_indices#gloas | ||
| - get_node_children#gloas | ||
| - get_node_for_root#gloas | ||
| - get_parent_payload_status#gloas | ||
| - get_payload_attestation_due_ms#gloas | ||
| - get_payload_attestation_message_signature#gloas | ||
| - get_payload_status_tiebreaker#gloas | ||
| - get_pending_balance_to_withdraw#gloas | ||
| - get_proposer_preferences_signature#gloas | ||
| - get_ptc_assignment#gloas | ||
| - get_safe_execution_block_hash#gloas | ||
| - get_supported_node#gloas | ||
| - get_upcoming_proposal_slots#gloas | ||
| - get_weight#gloas | ||
| - has_compounding_withdrawal_credential#gloas | ||
| - is_ancestor#gloas | ||
| - is_head_late#gloas | ||
| - is_head_weak#gloas | ||
| - is_parent_node_full#gloas | ||
| - is_parent_strong#gloas | ||
| - is_supporting_vote#gloas | ||
| - is_previous_slot_payload_decision#gloas | ||
| - is_valid_proposal_slot#gloas | ||
| - notify_ptc_messages#gloas | ||
| - on_block#gloas | ||
|
|
||
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.
another follow up we need to handle
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.
Investigated. Recommendation: keep skipped, tighten the rationale comment so it's no longer a TODO.
What it is:
EMPTY_BLOCK_HASH: Hash32 = Hash32()— a 32-byte zero sentinel introduced in v1.7.0-alpha.9. Only spec use is theif TERMINAL_BLOCK_HASH != EMPTY_BLOCK_HASH:check (validate_merge_block,is_valid_terminal_pow_block,notify_new_payload).Why skip is the right call:
EMPTY_BLOCK_HASHreturns 0 hits on Lighthouse, Prysm, and Teku via GitHub code search. The constant is purely a spec-readability rename of the literal zero hash; nobody imports it.0x0000…0000. Clients comparingchainConfig.TERMINAL_BLOCK_HASHagainst the literal cover the same check.PAYLOAD_STATUS_VALID/INVALIDATED/NOT_VALIDATED(also bellatrix-introduced named sentinels) are skipped on the same grounds.If we want to permanently bless the skip, suggested wording (replacing the current TODO):
Happy to either:
EMPTY_BLOCK_HASHtospecConstantsinpackages/beacon-node/src/api/impl/config/constants.tsastoHexBytes32(EMPTY_BLOCK_HASH)).Your call. Tracking as a 🟢 follow-up in the meantime.