Count PTC votes from duplicated validators + apply attestations from blocks - #10732
Conversation
tbenr
left a comment
tbenr
left a comment
There was a problem hiding this comment.
LGTM. just a consideration
| // Note: not using thenRun here because we want to ensure each step is on the event thread | ||
| transaction.commit().join(); | ||
| blockImportPerformance.ifPresent(BlockImportPerformance::transactionCommitted); | ||
| applyPayloadAttestationsFromBlock(block); |
There was a problem hiding this comment.
i'd move this closer to the block in which we call apply*FromBlock
the reason why it was not implemented in the first place is because it is not a big deal:
1- PTC votes influence only the FULL\EMPTY selection (no "long term" weight applied to FC)
2- in terms of PTC packing and block inclusion during block production, the PTC are included only if they are from the previous slot. So the attestationPool complication is not needed.
so, missing those messages is not really a big deal.
Due to that, even if not super-aligned with the spec, we could move this inside
if (spec.computeEpochAtSlot(block.getSlot())
.isGreaterThanOrEqualTo(currentEpoch.minusMinZero(1))) {
....
}
together with the others, so while syncing far from head we won't do useless work (if spec will change around it we may need to reevaluate) and it will be nicely together with the others.
There was a problem hiding this comment.
Good clarification! I've updated the code
3 participants
PR Description
ethereum/consensus-specs#5222) -
It also includes applying attestations from block which we missed
Fixed Issue(s)
Documentation
doc-change-requiredlabel to this PR if updates are required.Changelog
Note
High Risk
Changes fork-choice vote accounting and block-import processing for Gloas payload attestations; mistakes could alter head selection or payload-timeliness decisions in consensus-critical logic.
Overview
Fixes Gloas payload-attestation vote accounting by tracking votes per PTC position (not per validator index), so duplicated validators in the committee contribute correctly to
is_payload_timely/is_payload_data_availablethresholds.Introduces
ValidatablePayloadAttestationMessageto precompute/cache PTC positions during gossip validation and before updating fork choice, adds a validated-subscriber path in the payload attestation pool, and updatesForkChoice/ForkChoiceStrategy/PtcVoteTrackerAPIs accordingly.Applies payload attestations included in imported blocks by expanding aggregated attestations into per-position messages and recording their votes during
onBlockprocessing; reference tests and unit tests are updated to cover duplicate-PTC and block-sourced attestations.Reviewed by Cursor Bugbot for commit 0748348. Bugbot is set up for automated code reviews on this repo. Configure here.