Skip to content

Fix dropping from SYNCING to IN_SYNC while far behind the head - #11219

Open
zilm13 wants to merge 5 commits into
Consensys-Incorporated:masterfrom
zilm13:fix-syncing
Open

Fix dropping from SYNCING to IN_SYNC while far behind the head#11219
zilm13 wants to merge 5 commits into
Consensys-Incorporated:masterfrom
zilm13:fix-syncing

Conversation

@zilm13

@zilm13 zilm13 commented Sep 2, 2026

Copy link
Copy Markdown
Contributor

PR Description

Issue: how we drop to IN_SYNC while far behind

SyncStateTracker.updateCurrentState() chose IN_SYNC purely from !syncActive — it never
checked how far behind the head actually was. Forward sync deactivates whenever it has no target
chain to work on, which includes "all peers dropped" and "stalled, waiting to retry". Both OOM
logs show exactly that: sync deactivates with the head still 850–1100 slots back, and the node
logs Syncing completed and reports IN_SYNC.

What it leads to

SyncState.isInSync()ForkChoiceNotifier.inSync = true
ProposersDataManager.calculatePayloadBuildingAttributes passes its !inSync guard and asks for
the state at currentSlot + 1 on top of a head ~1000 slots older.
StateAtSlotTask.regenerateFromState replays that gap: ~27–34 epoch transitions, each
materialising ~1M Validator SSZ views plus ValidatorStatus objects.

It also runs many times over:

  • fired every slot and on every head change;
  • each new head root is a fresh cache key;
  • streamIntermediateSteps() only looks back 640 slots, so a larger gap defeats rebasing and
    deduplication entirely;
  • CachingTaskQueue allows availableProcessors of them concurrently.

9 GB heap exhausted → OutOfMemoryError.

The fix

Judge behind-ness against the head our peers report, not the wall-clock slot.
SyncStateTracker implements SlotEventsChannel, re-evaluates each slot, and holds SYNCING
while our head is more than slotsPerEpoch * MAX_SEED_LOOKAHEAD (128 slots) behind the
peer-reported head from Eth2Peer.getStatus().getHeadSlot().

peers reference head behind?
0, startupTargetPeerCount == 0 none no — standalone node, keep proposing
0, startupTargetPeerCount > 0 none yes — can't trust our own head
1–2 highest peer head distance > 128
3+ 2nd-highest peer head distance > 128

Requiring two peers to agree once 3+ are connected means one peer overstating its head can't
wedge us. Sync retry is untouched — a restart just moves the node from held-SYNCING to
active-SYNCING.

Using peer heads rather than the current slot is the load-bearing detail: the obvious version
(head vs current slot) deadlocks on a chain with >128 consecutive empty slots, because the held
SYNCING makes ValidatorApiHandler.isSyncActive() reject every duty, so nobody proposes and the
head never advances. Peer heads are equally old during such an outage, so the comparison stays
correct.

Fixed Issue(s)

Documentation

  • I thought about documentation and added the doc-change-required label to this PR if updates are required.

Changelog

  • I thought about adding a changelog entry, and added one if I deemed necessary.

@zilm13
zilm13 marked this pull request as ready for review September 2, 2026 18:58
Comment on lines +47 to +48
* Peer count from which we stop trusting a single peer's claimed head and require a second peer
* to corroborate it.

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.

so, really what we're saying is we need the 'MIN_PEERS_FOR_AGREEMENT' to agree, where as the comment suggests 2...

Comment on lines +120 to +122
this.maxSlotsBehindHead =
UInt64.valueOf(
(long) genesisSpec.getSlotsPerEpoch() * genesisSpec.getConfig().getMaxSeedLookahead());

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.

given we have RecentChainData anyway, maybe just refactor RecentChainData to allow us to query the value - in this way we're not duplicating this computation if we're wanting them to be the same

its also specVersion dependent so potentially we should be just calling RecentChainData for the right value rather than storing


private void updateCurrentState() {
final SyncState previousState = currentState;
boolean heldBehindHead = false;

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.

what do we mean 'heldBehindHead'? that is a confusing term to me

@zilm13

zilm13 commented Sep 3, 2026

Copy link
Copy Markdown
Contributor Author

@rolfyone addressed feedback, simplified the things

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.

2 participants