Skip to content

Delete unnecessary SyncMessage variants#9379

Merged
mergify[bot] merged 5 commits into
sigp:unstablefrom
dapplion:unify-unknown-parent-header
Jun 2, 2026
Merged

Delete unnecessary SyncMessage variants#9379
mergify[bot] merged 5 commits into
sigp:unstablefrom
dapplion:unify-unknown-parent-header

Conversation

@dapplion
Copy link
Copy Markdown
Collaborator

@dapplion dapplion commented Jun 1, 2026

Issue Addressed

Lookup sync does not cache sidecars, so sending the full network object adds unnecessary complexity. Sync only needs to know: We have received a header that has an unknown parent.

Proposed Changes

Replace UnknownParentDataColumn and UnknownParentPartialDataColumn for UnknownParentSidecarHeader

dapplion added 3 commits June 1, 2026 06:50
Replace the two separate sidecar parent-unknown variants
(UnknownParentDataColumn and UnknownParentPartialDataColumn) with a
single header-based variant UnknownParentHeader { peer_id, block_root,
parent_root, slot }. This carries only the header info needed to trigger
a parent lookup, decoupled from the concrete sidecar type.

The two gossip senders (data-column and partial-data-column
ParentUnknown handlers) now emit UnknownParentHeader, and the manager
collapses both handler arms into one. The now-dead
BlockComponent::PartialDataColumn variant (functionally identical to
DataColumn) is removed.
…:Sidecar

Use `SyncMessage::UnknownParentSidecarHeader` (was `UnknownParentHeader`) and
`BlockComponent::Sidecar` (was `BlockComponent::DataColumn`) for naming
consistency with PR 9155. The parent-unknown path is type-agnostic
(header-based), so `Sidecar` is the accurate name; `get_type` label updated to
"sidecar".
The sidecar parent-unknown path only ever reads the parent root (the
`DownloadResult`'s block_root/seen_timestamp/peer_group were never used, and
`add_child_components` ignores the payload). Replace
`Sidecar(DownloadResult<Hash256>)` with a named-field `Sidecar { parent_root: Hash256 }`.
@dapplion dapplion requested a review from jxs as a code owner June 1, 2026 05:24
Copy link
Copy Markdown
Member

@eserilev eserilev left a comment

Choose a reason for hiding this comment

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

LGTM

Comment thread beacon_node/network/src/sync/manager.rs Outdated
@eserilev eserilev added ready-for-merge This PR is ready to merge. syncing labels Jun 2, 2026
@mergify mergify Bot added the queued label Jun 2, 2026
@mergify
Copy link
Copy Markdown

mergify Bot commented Jun 2, 2026

Merge Queue Status

  • Entered queue2026-06-02 13:28 UTC · Rule: default
  • 🚫 Left the queue2026-06-02 13:30 UTC · at 1f041a53fd06a0cf81f383c746d54bc19ae0c171

This pull request spent 2 minutes 26 seconds in the queue, with no time running CI.

Reason

The pull request #9379 has been manually updated

Hint

If you want to requeue this pull request, you can post a @mergifyio queue comment.

@mergify
Copy link
Copy Markdown

mergify Bot commented Jun 2, 2026

Merge Queue Status

This pull request spent 1 hour 22 minutes 5 seconds in the queue, including 45 minutes 18 seconds running CI.

Required conditions to merge

mergify Bot added a commit that referenced this pull request Jun 2, 2026
mergify Bot added a commit that referenced this pull request Jun 2, 2026
@mergify mergify Bot merged commit d7d56e6 into sigp:unstable Jun 2, 2026
38 checks passed
@mergify mergify Bot removed the queued label Jun 2, 2026
dapplion added a commit to dapplion/lighthouse that referenced this pull request Jun 3, 2026
Resolves conflicts from sigp#9379 (Delete unnecessary SyncMessage variants):
- adopt SyncMessage::UnknownParentSidecarHeader (drop UnknownParentDataColumn/
  UnknownParentPartialDataColumn), keeping our unit BlockComponent::Sidecar
  (parent_root is routed separately to handle_unknown_parent)
- keep concrete trait-removal BlockRequest/DataRequestState handlers
Also: move DownloadResult back to its unstable position to minimize diff.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ready-for-merge This PR is ready to merge. syncing

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants