Skip to content

Count PTC votes from duplicated validators - #5222

Merged
jtraglia merged 7 commits into
ethereum:masterfrom
nflaig:ptc-duplicate-votes
May 8, 2026
Merged

Count PTC votes from duplicated validators#5222
jtraglia merged 7 commits into
ethereum:masterfrom
nflaig:ptc-duplicate-votes

Conversation

@nflaig

@nflaig nflaig commented May 8, 2026

Copy link
Copy Markdown
Member

A validator can be part of the PTC multiple times in a slot because compute_ptc samples by effective balance.

Right now, on_payload_attestation_message only records the vote at ptc.index(validator_index), so the validator's other entries stay None. With enough duplicates this makes the fixed PAYLOAD_TIMELY_THRESHOLD = PTC_SIZE // 2 unreachable. While this is very unlikely to have any meaningful impact on mainnet, it's notable during testing, especially if we have many 0x02 validators in the network.

In principle, it also seems more correct to count these votes, since the PTC is not one validator, one vote, it's balance weighted. We also already do the same for sync committee where process_sync_aggregate counts duplicate votes. I don't see a reason why PTC should work differently.

Related discussion on discord

@github-actions github-actions Bot added testing CI, actions, tests, testing infra gloas labels May 8, 2026

@mkalinin mkalinin left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

@jtraglia jtraglia left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🥇 will let @brech1 handle merging this.

@jtraglia
jtraglia requested a review from brech1 May 8, 2026 12:49
Comment thread specs/gloas/beacon-chain.md

@brech1 brech1 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, I can add tests for this update on #5206

@github-actions github-actions Bot removed the testing CI, actions, tests, testing infra label May 8, 2026
@brech1
brech1 requested review from jtraglia and mkalinin May 8, 2026 15:48
@brech1

brech1 commented May 8, 2026

Copy link
Copy Markdown
Contributor

Comment thread specs/gloas/fork-choice.md Outdated

@nflaig nflaig left a comment

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@brech1 your changes look good to me, as discussed, tests will be added in #5206

Comment thread specs/gloas/fork-choice.md Outdated
Comment thread specs/gloas/fork-choice.md Outdated
Comment thread specs/gloas/fork-choice.md Outdated
Co-authored-by: Justin Traglia <95511699+jtraglia@users.noreply.github.com>
@jtraglia
jtraglia merged commit 96838e5 into ethereum:master May 8, 2026
15 checks passed
brech1 added a commit to brech1/consensus-specs that referenced this pull request May 11, 2026
brech1 added a commit to brech1/consensus-specs that referenced this pull request May 12, 2026
twoeths added a commit to ChainSafe/lodestar that referenced this pull request May 18, 2026
**Motivation**

Follow consensus-specs PR
ethereum/consensus-specs#5222.

`compute_ptc` samples PTC seats by effective balance and may place the
same validator at multiple positions in a slot's PTC.
The previous `on_payload_attestation_message` recorded the vote only at
`ptc.index(validator_index)` (first occurrence), leaving the other
duplicate seats as `None`.
With enough duplicates this can make `PAYLOAD_TIMELY_THRESHOLD =
PTC_SIZE // 2` unreachable, particularly in testnets with many `0x02`
validators.
Sync committee already counts duplicate votes; PTC should match.

**Description**

- replace `getIndexInPayloadTimelinessCommittee` with
`getIndicesInPayloadTimelinessCommittee` returning every PTC position a
validator occupies
- handle `PayloadAttestationPool.add()` to support multiple validator
committee indices

**AI Assistance Disclosure**

Used Claude Code.

Co-authored-by: Tuyen Nguyen <twoeths@users.noreply.github.com>
pull Bot pushed a commit to Hawthorne001/prysm that referenced this pull request Jun 11, 2026
…inLabs#16934)

The gloas `on_payload_attestation_message` fork choice spec tests were
silently passing because the runner's `Step`/`Check` types had no fields
for `payload_attestation_message` steps or the `payload_timeliness_vote`
/ `payload_data_availability_vote` checks, so YAML unmarshalling
silently dropped both.

- Add `payload_attestation_message` step support: unmarshals the message
and feeds it through `ReceivePayloadAttestationMessage`, asserting
acceptance/rejection per the step's`valid` field
- Add `payload_timeliness_vote` / `payload_data_availability_vote` check
support, comparing the per-seat PTC vote bitvectors in forkchoice
(`null` = no vote, `true`/`false` = recorded value)
- Add a `PTCVotes` getter on the doubly-linked-tree forkchoice to expose
the vote bit vectors
- Convert the skip list to `map[string]string` so each skip logs its
rationale

With the steps actually wired up, 2 of 7 test cases pass (`from_block`,
`not_ptc_member`). The remaining 5 are skipped with rationales:

  | Test | Reason |
  |---|---|
| `valid`, `multiple_ptc_members_vote_independently` | PTC votes are
only recorded at the validator's first committee seat
[consensus-specs#5222](ethereum/consensus-specs#5222)
requires recording at every duplicate seat (balance-weighted committee)
|
| `current_slot_and_signature` | Signature and current-slot checks live
in gossip validation (`validate_payload_attestation.go`), not the fork
choice handler |
| `unknown_block_root` | Unknown block root check lives in gossip
validation (`VerifyBlockRootSeen`) |
| `slot_mismatch` | Block slot match check lives in gossip validation
(`VerifyBlockSlotMatches`) |
pull Bot pushed a commit to Hawthorne001/prysm that referenced this pull request Jul 7, 2026
see ethereum/consensus-specs#5222

---------

Co-authored-by: Potuz <potuz@prysmaticlabs.com>
Co-authored-by: terence <terence@prysmaticlabs.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants