feat: sync past finalized to head - #19
Conversation
There was a problem hiding this comment.
Pull request overview
This PR updates the networking syncer’s range-sync target from “finalized” to “head” so that forward sync can progress beyond finalized toward the chain head, and adjusts data-availability handling/tests to avoid crashing when legacy blob sidecars are unavailable.
Changes:
- Added
PeerManager::head_slot()and updated range sync to targethead_slot().or(finalized_slot()). - Renamed the sync completeness check to
is_synced_to_head_slotand updated the network manager to use it. - Adjusted blob-sidecar/data-availability behavior and updated tests (including a new mismatched-map test).
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| crates/networking/syncer/src/block_range/peer_manager.rs | Adds head_slot() to derive a head-based sync target from peer status. |
| crates/networking/syncer/src/block_range/mod.rs | Uses head-based target slot in both the sync loop and “is synced” check; adjusts blob-sidecar/DA behavior and tests. |
| crates/networking/syncer/src/block_range/block_cache.rs | Renames “finalized” terminology to “target” and updates tests around post-Fulu blob behavior. |
| crates/networking/manager/src/service.rs | Switches sync-start gating to is_synced_to_head_slot. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
4149b80 to
f671023
Compare
Both were rejected with a 404, so `/eth/v1/beacon/headers/head` — the ordinary way to ask a node where it is — returned "This ID type is currently not supported" and any consumer polling it concluded the node was offline. Head resolves through fork choice, which reads entirely out of the database, and genesis through the slot index.
…d post-Fulu DA gaps
5e0815d to
cbd3037
Compare
tosynthegeek
left a comment
tosynthegeek
left a comment
There was a problem hiding this comment.
the metrics that we discussed are still missing
| }) => { | ||
| for slot in start_slot..start_slot + count { | ||
| let Ok(Some(block_root)) = ream_db.slot_index_provider().get(slot) else { | ||
| trace!("No block root found for slot {slot}"); | ||
| p2p_sender.send_error_response( | ||
| peer_id, | ||
| connection_id, | ||
| stream_id, | ||
| &format!("No block root found for slot {slot}"), | ||
| ); | ||
| return; | ||
| if count > MAX_REQUEST_BLOCKS_DENEB { |
There was a problem hiding this comment.
lets avoid using hardcoded const when we can read from the beacon state: beacon_network_spec().max_request_blocks_deneb
There was a problem hiding this comment.
The description says sync target selection now votes on (slot, root) together so a peer on a different fork can't be counted toward quorum, but this file doesn't do that. Status already carries finalized_root and head_root, yet TargetSelection::Ready only stores target_slot and eligible_peers, best_finalized() votes purely on finalized_epoch, best_non_finalized() votes purely on the epoch derived from head_slot, and peers_satisfying() qualifies candidates on finalized_epoch/head_slot thresholds alone.
Also, fetch_idle_peer_from/fetch_idle_peer_from_excluding also uses peer order with no root comparison. So two peers on different forks reporting same slot still counts as agreeing votes and are equally eligible for selection
| ); | ||
| return; | ||
| if count > MAX_REQUEST_BLOCKS { | ||
| p2p_sender.send_invalid_request( |
There was a problem hiding this comment.
Same here, let's read from the beacon state: beacon_network_spec().max_request_blocks
What was wrong?
Fixes ReamLabs#1550.
Added syncing to head path to address the first issue, then try to implement a simple version or base lane to fix all problems need to address in range sync flow.
How was it fixed?
is_synced_to_head_slotnow also checks the local wall-clock slot, not peer reports alone. This is what actually stops the syncer from reporting "synced" with 0 blocks downloaded, which was the main bug in the issue.Peer classification
(slot, root)together, not slot alone. A peer on a different fork no longer gets counted as agreeing with peers on another fork.Peer scoring
EmptyResponse,ProtocolError,InvalidProof) instead of a free-text string, so severity can be read programmatically later.Data availability fetching
DataColumnSidecarsByRangeandDataColumnSidecarsByRootrequests. A range request is sent for each downloaded block range that needs columns; a root request is sent as a fallback for blocks that arrived through the parent-lookup path instead of a range.Other fixes found while testing
Tested end to end against live Sepolia: checkpoint sync, block range fetch, data column fetch, and block import all confirmed working.
To-Do