Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: the unknown block sync timeout #6031

Merged
merged 5 commits into from
Oct 13, 2023
Merged
Changes from 4 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
22 changes: 19 additions & 3 deletions packages/beacon-node/src/sync/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,14 +57,30 @@ export class BeaconSync implements IBeaconSync {
this.rangeSync.on(RangeSyncEvent.completedChain, this.updateSyncState);
this.network.events.on(NetworkEvent.peerConnected, this.addPeer);
this.network.events.on(NetworkEvent.peerDisconnected, this.removePeer);
this.chain.clock.on(ClockEvent.epoch, this.onClockEpoch);
} else {
// test code, this is needed for Unknown block sync sim test
this.unknownBlockSync.subscribeToNetwork();
this.logger.debug("RangeSync disabled.");
}

// TODO: It's okay to start this on initial sync?
this.chain.clock.on(ClockEvent.epoch, this.onClockEpoch);
// In case node is started with `rangeSync` disabled and `unknownBlockSync` is enabled.
// If the epoch boundary happens right away the `onClockEpoch` will check for the `syncDiff` and if
// it's more than 2 epoch will disable the disabling the `unknownBlockSync` as well.
// This will result into node hanging on the head slot and not syncing any blocks.
// This was the scenario in the test case `Unknown block sync` in `packages/cli/test/sim/multi_fork.test.ts`
// So we are adding a particular delay to ensure that the `unknownBlockSync` is enabled.
const syncStartSlot = this.chain.clock.currentSlot;
// Having one epoch time for the node to connect to peers and start a syncing process
const epochCheckFotSyncSlot = syncStartSlot + SLOTS_PER_EPOCH;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
const epochCheckFotSyncSlot = syncStartSlot + SLOTS_PER_EPOCH;
const epochCheckForSyncSlot = syncStartSlot + SLOTS_PER_EPOCH;

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Will fix this typo in upcoming PR.

Copy link
Member

Choose a reason for hiding this comment

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

Fixed by @tuyennhv already, see 1f36e8d

const initiateEpochCheckForSync = (): void => {
if (this.chain.clock.currentSlot > epochCheckFotSyncSlot) {
this.logger.info("Initiating epoch check for sync progress");
this.chain.clock.off(ClockEvent.slot, initiateEpochCheckForSync);
this.chain.clock.on(ClockEvent.epoch, this.onClockEpoch);
}
};
this.chain.clock.on(ClockEvent.slot, initiateEpochCheckForSync);
}

if (metrics) {
metrics.syncStatus.addCollect(() => this.scrapeMetrics(metrics));
Expand Down