feat: tiered attestation scoring#791
Open
MegaRedHand wants to merge 18 commits into
Open
Conversation
Adds three integration tests that build real block chains and verify the core behaviors of _select_attestations: cascading projected justification across rounds, accepting a gap-closing attestation with an older-but-valid source, and enforcing the MAX_ATTESTATIONS_DATA cap.
An older-source candidate (source.slot < projected_finalized_slot), reachable after a finalize-tier pick advances the finalized boundary or on an already-finalized head, made the finalize predicate call is_justifiable_after with a slot behind the finalized boundary, which asserts. Clamp the scan to start above the finalized slot: such slots are finalized, not pending, so they never block finalization. Add regression tests for the finalize tier and the older-source path.
- Require source.slot >= projected_finalized_slot before classifying an entry as FINALIZE tier. Finalization is monotonic, so an entry whose source is behind the projected boundary cannot advance it. The previous scan_start clamp avoided the is_justifiable_after assertion but still allowed FINALIZE to fire with a vacuously-empty range, which then dropped the projected finalized_slot below its current value and corrupted subsequent is_slot_justified lookups (silent wrong answer or IndexError). - Add precondition assert on slot > parent_slot so a same-slot misuse fails fast instead of underflowing Uint64 into a 2^64-element list allocation. - Extract _is_genesis_self_vote helper to dedupe the predicate across the scorer and the filter. - Add a focused regression test for the boundary-regression scenario.
Collapse the three genesis-self-vote exemption guards in the entry filter into a single branch, hash each candidate's data root once up front instead of re-hashing every round, and accumulate running votes only for BUILD-tier targets since justify and finalize discard them immediately.
The per-round re-hashing it avoided is not worth the extra parameter and map allocation; restore the inline data-root hash at the tiebreak site.
…on-scoring # Conflicts: # src/lean_spec/spec/forks/lstar/spec.py
The tiered greedy scorer changes which attestations a block carries relative to the old fixed-point builder, so two fork-choice fixtures needed updating. Divergence self-healing: the produced block carries only the divergence-closing vote. The other pool entry's voters are already recorded on-chain for that target, so it adds no new voters and the scorer omits it rather than re-stating a vote the post-state holds. Attestation-data cap: gossip one more than the limit using the first justifiable target slots, one voter each so no entry justifies. The cap is then exercised by entry count alone, with the justifiability filter and justification dynamics held out of the candidate set.
…on-scoring # Conflicts: # src/lean_spec/spec/forks/lstar/spec.py # tests/consensus/lstar/fc/test_block_production.py
The state transition only advances finalization when the attestation source lies strictly past the finalized boundary (PR leanEthereum#802). A source at the boundary is already final: it may justify a newer target but must not re-finalize. The tiered block-building scorer projected finalization independently and used a non-strict source check, so an entry whose source sat exactly on the finalized boundary was classed FINALIZE and over-ranked ahead of genuine justify entries, reordering selection near the data-entry cap. Mirror the strict source guard in the scorer and add a regression test covering a source at the finalized boundary.
…on-scoring Main split the monolithic lstar spec into per-concern mixins and flattened the test tree, so the tiered attestation scorer was re-applied onto the new layout rather than textually merged: - Port the tiered scorer (selection tiers, entry scoring, projected justification) from the old spec.py into block_production.py, using the renamed coverage selector and module-level chain-match helper. - Restore the justified-bitfield update and window-shift helpers that main's dead-code pass removed; only the scorer uses them. - Re-apply tiered-scorer docs to validator_duties.py. - Re-apply test expectation changes onto the relocated fork-choice tests (fc/ -> fork_choice/, Given/When/Then docstrings). - Drop the scorer's pytest unit tests: forks are now tested exclusively through consensus vectors, and the fork pytest tree no longer exists.
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
1 participant
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.
🗒️ Description
Previously, we relaxed the block building algorithm to also take into account attestations with an older source. This fixed a devnet breaking bug but also made block building much less efficient, with useless attestations now being packed in blocks.
This PR changes the block building algorithm to order attestations by usefulness, with attestations that finalize a slot being included before any that only justify a slot, and those being preferred before attestations that only add weight. This is implemented by grouping attestations by tiers (Finalize, Justify, Build), and scoring them by coverage.
✅ Checklist
just check