Skip to content

feat(sync): implement sync-v2#15325

Merged
shreyan-gupta merged 15 commits into
masterfrom
shreyan-gupta/sync-v2/base-functions
Mar 11, 2026
Merged

feat(sync): implement sync-v2#15325
shreyan-gupta merged 15 commits into
masterfrom
shreyan-gupta/sync-v2/base-functions

Conversation

@shreyan-gupta

@shreyan-gupta shreyan-gupta commented Mar 5, 2026

Copy link
Copy Markdown
Contributor
  • Adds 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 V1 run() methods are preserved and still used by the V1 handler — no behavioral changes for V1.
  • Adds handle_sync_needed_v2() — the V2 sync handler that implements a single linear pipeline: EpochSync → HeaderSync → StateSync → BlockSync. Gated behind SYNC_V2_ENABLED (currently false).

Why new run_v2() methods instead of modifying the existing ones?

The V1 run() methods mix orchestration with execution: they read and write SyncStatus, decide whether to trigger other sync phases (e.g. block sync's check_state_needed can 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 into run_v2() methods with cleaner signatures:

  • EpochSync::run_v2(&mut EpochSyncStatus) — takes the inner EpochSyncStatus directly instead of &mut SyncStatus. The V1 run() now delegates to run_v2() after extracting the status.
  • HeaderSync::run_v2(chain, highest_height, peers, ban_stalling_peers) — self-contained header sync tick. Structured around syncing_peer presence: if a request is in flight, checks batch completion or stalling; if idle, picks a peer and sends a request. Does not touch SyncStatus. When ban_stalling_peers is 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's run() and header_sync_due() are unchanged from master.
  • BlockSync::run_v2(chain, peers) — requests blocks if due. Does not check state_needed (V2 never falls back to state sync from block sync). Extracted block_request_due() from block_sync_due() — the latter adds the check_state_needed guard used only by V1.

handle_sync_needed_v2 — the V2 handler

The V2 handler replaces V1's waterfall (run everything top-to-bottom each tick) with an explicit state machine: a single match on SyncStatus that calls the appropriate component and writes the next status on transition. The handler enters at EpochSync (far horizon) or BlockSync (near horizon) based on is_epoch_sync_needed(), and progresses linearly through the pipeline.

Key differences from V1:

  • Entry decision made oncedecide_initial_phase() runs only when transitioning from NoSync/AwaitingPeers. V1 re-evaluates every tick.
  • No StateSyncDone — transitions directly StateSync → BlockSync.
  • No should_state_sync / check_state_needed — state sync only happens on the far-horizon path, never as a fallback from block sync.
  • No reset_data_pre_state_sync — stale nodes wipe DB via process restart.
  • Header sync only where needed — called during HeaderSync and BlockSync phases, not every tick with mode switching.

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 from run() into a pure query method. Adds an archival node guard (archival nodes must not epoch sync). Changes the stale-node gate from ContinuousEpochSync to SYNC_V2_ENABLED.
  • archive field on EpochSync — new archive: bool field (passed from config.archive via client.rs) so is_epoch_sync_needed() can reject epoch sync for archival nodes.
  • SYNC_V2_ENABLED constant — replaces ProtocolFeature::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 in sync/mod.rs and is set to false.
  • Stale-node shutdown gate — in the EpochSyncResponseMessage handler, the gate that triggers data-reset shutdown changed from ProtocolFeature::ContinuousEpochSync to SYNC_V2_ENABLED.
  • neard/cli.rscheck_epoch_sync_data_reset_marker now checks SYNC_V2_ENABLED instead of ProtocolFeature::ContinuousEpochSync.

Follow-ups

  • Test-loop tests — dedicated V2 sync tests (far-horizon full pipeline, near-horizon block sync, stale node shutdown, archival skip, etc.) will be added in a follow-up PR once the handler is validated.
  • V1 cleanup — once V2 is proven stable, ~600 lines of V1-only code can be removed: handle_sync_needed_v1, should_state_sync, check_state_needed, reset_data_pre_state_sync, HeaderSyncAction enum, block_fetch_horizon config, and the Primary/Background header sync mode distinction.

@shreyan-gupta shreyan-gupta force-pushed the shreyan-gupta/sync-v2/base-functions branch from 3022d05 to 0b9a490 Compare March 6, 2026 22:18
@shreyan-gupta shreyan-gupta changed the base branch from master to shreyan-gupta/sync/consolidate-state-sync March 6, 2026 22:18
Base automatically changed from shreyan-gupta/sync/consolidate-state-sync to master March 7, 2026 02:51
@shreyan-gupta shreyan-gupta changed the title draft(sync): SyncV2 initial helper functions feat(sync): implement sync-v2 Mar 10, 2026
@codecov

codecov Bot commented Mar 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 21.89781% with 214 lines in your changes missing coverage. Please review.
✅ Project coverage is 69.16%. Comparing base (7ee16cb) to head (0d28c23).
⚠️ Report is 3 commits behind head on master.

Files with missing lines Patch % Lines
chain/client/src/sync/handler.rs 14.87% 103 Missing ⚠️
chain/client/src/sync/header.rs 0.00% 91 Missing ⚠️
chain/client/src/sync/block.rs 20.00% 12 Missing ⚠️
chain/client/src/sync/epoch.rs 84.44% 2 Missing and 5 partials ⚠️
neard/src/cli.rs 0.00% 1 Missing ⚠️
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     
Flag Coverage Δ
pytests-nightly 1.27% <0.00%> (-0.01%) ⬇️
unittests 68.51% <21.89%> (-0.06%) ⬇️
unittests-nightly 68.70% <21.89%> (-0.07%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@shreyan-gupta shreyan-gupta marked this pull request as ready for review March 10, 2026 18:38
@shreyan-gupta shreyan-gupta requested a review from a team as a code owner March 10, 2026 18:38

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

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 behind SYNC_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.

Comment thread chain/client/src/sync/header.rs
Comment thread chain/client/src/sync/handler.rs
if !SYNC_V2_ENABLED && tip_height != chain.genesis().height() {
return Ok(false);
}
tracing::debug!(tip_height, highest_height, horizon, "epoch sync needed");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

nit: as much as I dislike it, for now we should keep adding the targets

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

:(
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

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

We can make a separate pr, but let's do it.

Comment thread chain/client/src/sync/mod.rs
Comment thread chain/client/src/sync/handler.rs Outdated
Comment thread chain/client/src/sync/handler.rs Outdated
Comment thread chain/client/src/sync/handler.rs Outdated
Comment thread chain/client/src/sync/header.rs Outdated
Comment thread chain/client/src/sync/header.rs
Comment thread chain/client/src/sync/header.rs
Comment thread chain/client/src/sync/handler.rs
@shreyan-gupta shreyan-gupta added this pull request to the merge queue Mar 11, 2026
Merged via the queue into master with commit 72408b7 Mar 11, 2026
31 of 33 checks passed
@shreyan-gupta shreyan-gupta deleted the shreyan-gupta/sync-v2/base-functions branch March 11, 2026 17:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants