fix: apply gossip clock disparity to proposer_preferences slot check - #9633
Conversation
… check The `[IGNORE] preferences.proposal_slot has not already passed` gossip validation compared `proposal_slot` against the exact `current_slot`, so a preference whose `proposal_slot` is within `MAXIMUM_GOSSIP_CLOCK_DISPARITY` of the next-slot boundary was accepted as `valid` instead of `ignore`. The boundary is sub-slot, so the integer `current_slot` comparison cannot express it. Use `slotWithFutureTolerance(MAXIMUM_GOSSIP_CLOCK_DISPARITY / 1000)` as the `current_slot` reference, matching the existing convention in `verifyPropagationSlotRange` (attestation.ts). `MAXIMUM_GOSSIP_CLOCK_DISPARITY` is a global gossip allowance that applies even where the spec states the check as a plain `proposal_slot > current_slot` (as previously confirmed for the analogous execution_payload_bid slot check, ChainSafe#9627). Verified against the Gloas networking gossip reference tests (consensus-specs PR ChainSafe#5294): fixes `gossip_proposer_preferences__ignore_slot_outside_disparity` and keeps `valid_slot_at_disparity_edge` / `valid_at_lookahead_upper_edge` passing (13/13 minimal + 13/13 mainnet). 🤖 Generated with AI assistance
There was a problem hiding this comment.
Code Review
This pull request updates the proposer preferences validation to account for the maximum gossip clock disparity when checking if a proposal slot has passed. The reviewer suggested using the existing currentSlotWithGossipDisparity property on the clock to simplify the implementation and make it more idiomatic.
Important
The consumer version of Gemini Code Assist on GitHub is being sunset. Starting June 18, 2026, new organization installations will be blocked, and all code review activity will officially cease on July 17, 2026.
For more details on the timeline and next steps, please review the Help Documentation.
| const currentSlot = chain.clock.currentSlot; | ||
| // [IGNORE] `preferences.proposal_slot` has not already passed, i.e. `proposal_slot > current_slot`, | ||
| // allowing for `MAXIMUM_GOSSIP_CLOCK_DISPARITY`. | ||
| const currentSlot = chain.clock.slotWithFutureTolerance(chain.config.MAXIMUM_GOSSIP_CLOCK_DISPARITY / 1000); |
There was a problem hiding this comment.
Instead of manually calculating the tolerance and calling slotWithFutureTolerance, we can use the existing currentSlotWithGossipDisparity property on the clock. This property is specifically designed to handle the MAXIMUM_GOSSIP_CLOCK_DISPARITY check at slot boundaries, making the code cleaner and more idiomatic.
| const currentSlot = chain.clock.slotWithFutureTolerance(chain.config.MAXIMUM_GOSSIP_CLOCK_DISPARITY / 1000); | |
| const currentSlot = chain.clock.currentSlotWithGossipDisparity; |
There was a problem hiding this comment.
Good call — applied in e75f482a60. currentSlotWithGossipDisparity is behaviour-equivalent here (both resolve to currentSlot, or currentSlot + 1 when within MAXIMUM_GOSSIP_CLOCK_DISPARITY of the next slot start) and drops the manual ms→s conversion. Biome clean.
…-prefs slot check Address Gemini review: replace `slotWithFutureTolerance(MAXIMUM_GOSSIP_CLOCK_DISPARITY / 1000)` with the dedicated `currentSlotWithGossipDisparity` getter for the proposal-slot-passed IGNORE check. Behaviour-equivalent — both return `currentSlot`, or `currentSlot + 1` when within `MAXIMUM_GOSSIP_CLOCK_DISPARITY` of the next slot start — and it drops the manual ms→s conversion. 🤖 Generated with AI assistance Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
|
🎉 This PR is included in v1.45.0 🎉 |
Motivation
The Gloas
proposer_preferencesgossip validation[IGNORE] preferences.proposal_slot has not already passedcomparesproposal_slotagainst the exactcurrent_slot. A preference whoseproposal_slotfalls withinMAXIMUM_GOSSIP_CLOCK_DISPARITYof the next-slot boundary is accepted asvalidwhen it should beignored. The boundary is sub-slot (a 1 ms shift flips the outcome), so the integercurrent_slotcomparison cannot express it.This is the same class of gap fixed for the
execution_payload_bidslot check in #9627.Description
Use
slotWithFutureTolerance(MAXIMUM_GOSSIP_CLOCK_DISPARITY / 1000)as thecurrent_slotreference, matching the existing convention inverifyPropagationSlotRange(attestation.ts).MAXIMUM_GOSSIP_CLOCK_DISPARITYis a global gossip allowance that applies even where the spec states the check as a plainpreferences.proposal_slot > current_slot(as confirmed for the analogous bid check).Verification
Ran the Gloas networking gossip reference tests from consensus-specs PR #5294:
gossip_proposer_preferences__ignore_slot_outside_disparity→ nowignore(wasvalid)gossip_proposer_preferences__valid_slot_at_disparity_edge→ stillvalidgossip_proposer_preferences__valid_at_lookahead_upper_edge→ stillvalidFull
gossip_proposer_preferences: 13/13 minimal + 13/13 mainnet, no regressions in the rest of the Gloas networking suite.🤖 Generated with AI assistance