Skip to content

Conversation

ralexstokes
Copy link
Member

Summary

  • Changes attestation timing specification from "should" to "MUST"
  • Requires validators to attest immediately upon receiving a valid block
  • The 4-second timeout only applies when no valid block is received

Rationale

Currently, the specification allows implementations to wait until the 4-second mark even if they've already received a valid block. This change ensures more consistent behavior across implementations and helps proposers receive attestations earlier when blocks are produced on time.

Note on sync committee timing

The sync committee message timing in the Altair specification (specs/altair/validator.md) follows a similar pattern but was not updated in this PR. It may benefit from the same change to ensure consistent immediate response upon block reception.

Test plan

  • Verify existing tests pass
  • Consider if additional tests are needed for the stricter timing requirement

🤖 Generated with Claude Code

Change attestation timing from "should" to "MUST" to require validators to attest immediately upon receiving a valid block, with the timeout only as a fallback when no block is received.

This ensures more consistent behavior across implementations and helps proposers receive attestations earlier when blocks are produced on time.

🤖 Generated with [Claude Code](https://claude.ai/code)

Co-Authored-By: Claude <[email protected]>
@arnetheduck
Copy link
Contributor

follows a similar pattern but was not updated in this PR.

I would recommend we use the wording from https://github.com/ethereum/consensus-specs/pull/3433/files#diff-ec5d1e18eeb8287e86befaebb407862e0841fad9955e1fb692aa909d518d478dR209

In particular, it addresses the problem @potuz / Prysm has raised in the past, that in order for this "must" to work, the receiving end also must have a mechanism to handle out-of-order messages - attestations typically travel faster than blocks and without clients handling said messages, we would see greater loss enabling this.

I'd also recommend updating sync committee messages at the same time to avoid having two sets of rules for what fundamentally should be the same kind of message.

@nflaig
Copy link
Member

nflaig commented Jul 23, 2025

In particular, it addresses the problem @potuz / Prysm has raised in the past, that in order for this "must" to work, the receiving end also must have a mechanism to handle out-of-order messages - attestations typically travel faster than blocks and without clients handling said messages, we would see greater loss enabling this.

I would assume in practice most clients already do this since Lodestar publishes attestations asap already since years and attestation performance is not affected by this.

@arnetheduck
Copy link
Contributor

I would assume in practice most clients already do this since Lodestar publishes attestations asap already since years and attestation performance is not affected by this.

From a spec perspective, it's valuable to point out in case of future doubt, given it has caused trouble in the past.

@ralexstokes
Copy link
Member Author

didn't realize this was effectively in #3433 already

i'll leave this open in the event we want to merge these changes in separately, but we may also just want to merge #3433 in lieu of this PR

@potuz
Copy link
Contributor

potuz commented Jul 24, 2025

I think the plan was to merge 3433 ASAP, it was agreed already on the merge and only timing is the issue. If any slot change is chosen for Glamsterdam, we will need 3433 anyway kinda right now. In particular 7732 is blocked from being rebased on top of Fulu because it depends on 3433. Replace 3433 by 3510 :)

@arnetheduck
Copy link
Contributor

#3510 I think lacks the timing requirement - I'm happy to pull out the timing requirement text in a separate PR if #3510 is going, then we can merge both without doing the actual rebalancing that #3433 also includes.

@jtraglia jtraglia added the phase0 label Aug 5, 2025
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