feat(sync): implement sync-v2#15325
Conversation
3022d05 to
0b9a490
Compare
…v2/base-functions Co-Authored-By: Claude Opus 4.6 <[email protected]>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #15325 +/- ##
==========================================
- Coverage 69.22% 69.16% -0.06%
==========================================
Files 931 931
Lines 207379 207637 +258
Branches 207379 207637 +258
==========================================
+ Hits 143556 143612 +56
- Misses 57907 58105 +198
- Partials 5916 5920 +4
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
There was a problem hiding this comment.
Pull request overview
Implements an opt-in (currently disabled) “sync-v2” pipeline that separates orchestration from execution by adding side-effect-free run_v2() entrypoints and a new linear sync handler (EpochSync → HeaderSync → StateSync → BlockSync).
Changes:
- Add
run_v2()methods to EpochSync/HeaderSync/BlockSync and refactor shared due/request logic. - Add
handle_sync_needed_v2()to drive a linear sync pipeline behindSYNC_V2_ENABLED. - Gate stale-node reset logic and CLI marker handling on
SYNC_V2_ENABLED, and plumb archival-node awareness into EpochSync.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| neard/src/cli.rs | Switch epoch-sync reset-marker handling gate to near_client::sync::SYNC_V2_ENABLED. |
| chain/client/src/sync/mod.rs | Introduce pub const SYNC_V2_ENABLED: bool = false gating the v2 pipeline. |
| chain/client/src/sync/header.rs | Add run_v2(), extract “request due” logic, and extract peer-banning into a separate method. |
| chain/client/src/sync/handler.rs | Dispatch between v1/v2 handlers; implement linear v2 state machine and initial-phase decision. |
| chain/client/src/sync/epoch.rs | Add is_epoch_sync_needed(), add archival guard via new archive field, add run_v2(), and gate stale reset on SYNC_V2_ENABLED. |
| chain/client/src/sync/block.rs | Add run_v2() and extract block_request_due() from v1’s block_sync_due(). |
| chain/client/src/client.rs | Pass config.archive into EpochSync::new() to support archival guard. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| if !SYNC_V2_ENABLED && tip_height != chain.genesis().height() { | ||
| return Ok(false); | ||
| } | ||
| tracing::debug!(tip_height, highest_height, horizon, "epoch sync needed"); |
There was a problem hiding this comment.
nit: as much as I dislike it, for now we should keep adding the targets
There was a problem hiding this comment.
:(
I too don't like targets but I have the additional argument that none of the tracing statements have targets in this file.
If you'd still like me to add the target, I can add them to all tracing calls
There was a problem hiding this comment.
We can make a separate pr, but let's do it.
run_v2()methods to epoch sync, header sync, and block sync that the V2 sync handler can call without the V1 side effects (status mutation, mode switching, state-sync-needed checks). The V1run()methods are preserved and still used by the V1 handler — no behavioral changes for V1.handle_sync_needed_v2()— the V2 sync handler that implements a single linear pipeline: EpochSync → HeaderSync → StateSync → BlockSync. Gated behindSYNC_V2_ENABLED(currentlyfalse).Why new
run_v2()methods instead of modifying the existing ones?The V1
run()methods mix orchestration with execution: they read and writeSyncStatus, decide whether to trigger other sync phases (e.g. block sync'scheck_state_neededcan trigger state sync), and contain mode-dependent logic (e.g. header sync's Primary/Background/Skip modes based on current status).V2 needs components that just do their work and report back — the handler owns all status transitions. Rather than adding feature flags inside each
run(), we extract the core logic intorun_v2()methods with cleaner signatures:EpochSync::run_v2(&mut EpochSyncStatus)— takes the innerEpochSyncStatusdirectly instead of&mut SyncStatus. The V1run()now delegates torun_v2()after extracting the status.HeaderSync::run_v2(chain, highest_height, peers, ban_stalling_peers)— self-contained header sync tick. Structured aroundsyncing_peerpresence: if a request is in flight, checks batch completion or stalling; if idle, picks a peer and sends a request. Does not touchSyncStatus. Whenban_stalling_peersis true (primary HeaderSync phase), bans stalling peers inline before clearing them — avoiding the peer-identity-loss issue that would occur if the peer were cleared first. V1'srun()andheader_sync_due()are unchanged from master.BlockSync::run_v2(chain, peers)— requests blocks if due. Does not checkstate_needed(V2 never falls back to state sync from block sync). Extractedblock_request_due()fromblock_sync_due()— the latter adds thecheck_state_neededguard used only by V1.handle_sync_needed_v2— the V2 handlerThe V2 handler replaces V1's waterfall (run everything top-to-bottom each tick) with an explicit state machine: a single
matchonSyncStatusthat calls the appropriate component and writes the next status on transition. The handler enters at EpochSync (far horizon) or BlockSync (near horizon) based onis_epoch_sync_needed(), and progresses linearly through the pipeline.Key differences from V1:
decide_initial_phase()runs only when transitioning fromNoSync/AwaitingPeers. V1 re-evaluates every tick.StateSyncDone— transitions directly StateSync → BlockSync.should_state_sync/check_state_needed— state sync only happens on the far-horizon path, never as a fallback from block sync.reset_data_pre_state_sync— stale nodes wipe DB via process restart.Currently gated behind
SYNC_V2_ENABLED = false— will be enabled in a follow-up PR after test validation.Other changes
EpochSync::is_epoch_sync_needed()— extracted the horizon check and stale-node gate fromrun()into a pure query method. Adds an archival node guard (archival nodes must not epoch sync). Changes the stale-node gate fromContinuousEpochSynctoSYNC_V2_ENABLED.archivefield onEpochSync— newarchive: boolfield (passed fromconfig.archiveviaclient.rs) sois_epoch_sync_needed()can reject epoch sync for archival nodes.SYNC_V2_ENABLEDconstant — replacesProtocolFeature::SyncV2. Sync versioning is a node-local concern (which code path to run), not a protocol concern (what messages/blocks are valid). The constant lives insync/mod.rsand is set tofalse.EpochSyncResponseMessagehandler, the gate that triggers data-reset shutdown changed fromProtocolFeature::ContinuousEpochSynctoSYNC_V2_ENABLED.neard/cli.rs—check_epoch_sync_data_reset_markernow checksSYNC_V2_ENABLEDinstead ofProtocolFeature::ContinuousEpochSync.Follow-ups
handle_sync_needed_v1,should_state_sync,check_state_needed,reset_data_pre_state_sync,HeaderSyncActionenum,block_fetch_horizonconfig, and the Primary/Background header sync mode distinction.