merge queue: checking #9675 on unstable (73e4dc1) - #10143
Closed
mergify[bot] wants to merge 24 commits into
Closed
mergify[bot] wants to merge 24 commits into
mergify[bot] wants to merge 24 commits into
Conversation
Publish sync committee messages as soon as a non-optimistic head event for the current slot arrives, instead of always sleeping to the due point, mirroring the attestation service change from #7892. Contributions remain delayed to their point in the slot. - Convert the head event channel to tokio broadcast so multiple services can subscribe via BeaconNodeFallback::subscribe_to_head_events. - Share the head-event-vs-deadline select (including the degrade to timer-only on channel close) between the attestation and sync committee services via beacon_head_monitor::head_event_or_deadline. This also fixes the attestation service busy-looping if the head event channel ever closed. - If a head event arrives before sync duties are computed, wait until the sync message deadline and check once more, reusing the event root. - Make sync message timing fork-aware via ChainSpec::get_sync_message_due_at_slot (SYNC_MESSAGE_DUE_BPS_GLOAS). - Extend MockBeaconNode with sync duty, head root, sync message pool and subscription mocks, and add sync committee service tests on the shared ValidatorClientHarness. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSiv1h1jtjqp8JE2AXbntQ
Three fixes from adversarial review of the eager sync message loop: - Derive next slot and its start offset from a single clock read (next_slot_with_duration), so a slot boundary passing between reads cannot anchor the timer to one slot's start with another slot's due point (previously ~1s early for one slot at the Gloas transition). - Pass the triggering slot into spawn_contribution_tasks instead of re-reading the clock, keeping the signed slot, dedupe marking, and duty lookup consistent with a single slot value. - Discard the head event root when the missing-duties retry sleeps to the deadline, falling back to a fresh non-optimistic head lookup, as the attestation service's deadline retry does. The event root may be stale after sleeping most of a slot. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSiv1h1jtjqp8JE2AXbntQ
Take the slot for sync message signing from the trigger itself: the validated event slot for the head event arm, or the armed next slot for the timer arm. Previously the loop re-read the clock after triggering, so a slot boundary (or backwards clock movement, which SlotClock permits) between the event's validation and the re-read could sign slot N+1 with slot N's root and, worse, mark N+1 as handled so the real N+1 head event was suppressed by the dedupe check. With the trigger-derived slot there are no post-trigger clock reads: a late event signs for its own slot (or is skipped by the expired-slot guard), and the next slot's messages are never suppressed. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSiv1h1jtjqp8JE2AXbntQ
Move sync_message_due_bps_gloas next to sync_message_due_bps in the gnosis constructor, which places both pairs of due keys adjacent to their base values. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSiv1h1jtjqp8JE2AXbntQ
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSiv1h1jtjqp8JE2AXbntQ
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSiv1h1jtjqp8JE2AXbntQ
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01MSiv1h1jtjqp8JE2AXbntQ
…-committee-messages
Merges cleanly. The attestation loop now pairs upstream's fork-aware `attestation_deadline()` with this branch's shared `head_event_or_deadline()` helper; the sync side keeps `get_sync_message_due_at_slot()`. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_019xd8HZnvqcMotvkuEQGh8W
…-committee-messages # Conflicts: # consensus/types/src/core/chain_spec.rs # validator_client/validator_services/src/sync_committee_service.rs
Replaces next_slot_with_duration with the sync_message_deadline helper that landed on unstable, restoring its unit test and the defensive pre-genesis fallback, and mirroring attestation_deadline in the attestation service. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_012YyYbMaMeNuxVq3Bv4a4FA
Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
Keep unstable's `new_with_spec_and_config` harness constructor and drop the branch's duplicate `new_with_spec`; sync committee tests now call it. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Selection proofs can be stored after a head event triggers the slot, for example with `--distributed`, where the proof for a slot is only computed once that slot has started. Reading the aggregators at trigger time then skipped the contribution. Read them at the contribution deadline instead. Add tests for the aggregate path, a head event arriving after the timer handled the slot, and `head_event_or_deadline`. Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>
|
|
1 similar comment
|
|
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.
🎉 This pull request has been checked successfully and will be merged soon. 🎉
#9675 is queued for merge on branch unstable (73e4dc1).
This pull request has been created by Mergify to check the mergeability of #9675.
You don't need to do anything. Mergify will close this pull request automatically when it is complete.
Required conditions of queue rule
defaultfor merge:check-success=local-testnet-successcheck-success=test-suite-successgithub-review-approved[🛡 GitHub branch protection]Required conditions to stay in the queue:
#approved-reviews-by >= 1check-success=license/clacheck-success=target-branch-checkgithub-review-approved[🛡 GitHub branch protection]label!=do-not-merge