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

test: fix flaky sim tests #6163

Merged
merged 9 commits into from
Dec 7, 2023
Merged
Show file tree
Hide file tree
Changes from all 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
13 changes: 13 additions & 0 deletions packages/beacon-node/src/sync/options.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
import {SLOTS_PER_EPOCH} from "@lodestar/params";

export type SyncOptions = {
/**
* Allow node to consider itself synced without being connected to a peer.
Expand All @@ -22,11 +24,22 @@ export type SyncOptions = {
backfillBatchSize: number;
/** For testing only, MAX_PENDING_BLOCKS by default */
maxPendingBlocks?: number;

/**
* The number of slots ahead of us that is allowed before starting a RangeSync
* If a peer is within this tolerance (forwards or backwards), it is treated as a fully sync'd peer.
*
* This means that we consider ourselves synced (and hence subscribe to all subnets and block
* gossip if no peers are further than this range ahead of us that we have not already downloaded
* blocks for.
*/
slotImportTolerance?: number;
};

export const defaultSyncOptions: SyncOptions = {
isSingleNode: false,
disableProcessAsChainSegment: false,
/** By default skip the backfill sync */
backfillBatchSize: 0,
slotImportTolerance: SLOTS_PER_EPOCH,
};
13 changes: 2 additions & 11 deletions packages/beacon-node/src/sync/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,6 @@ export class BeaconSync implements IBeaconSync {

/** For metrics only */
private readonly peerSyncType = new Map<string, PeerSyncType>();

/**
* The number of slots ahead of us that is allowed before starting a RangeSync
* If a peer is within this tolerance (forwards or backwards), it is treated as a fully sync'd peer.
*
* This means that we consider ourselves synced (and hence subscribe to all subnets and block
* gossip if no peers are further than this range ahead of us that we have not already downloaded
* blocks for.
*/
private readonly slotImportTolerance: Slot;

constructor(opts: SyncOptions, modules: SyncModules) {
Expand All @@ -48,7 +39,7 @@ export class BeaconSync implements IBeaconSync {
this.logger = logger;
this.rangeSync = new RangeSync(modules, opts);
this.unknownBlockSync = new UnknownBlockSync(config, network, chain, logger, metrics, opts);
this.slotImportTolerance = SLOTS_PER_EPOCH;
this.slotImportTolerance = opts.slotImportTolerance ?? SLOTS_PER_EPOCH;

// Subscribe to RangeSync completing a SyncChain and recompute sync state
if (!opts.disableRangeSync) {
Expand Down Expand Up @@ -241,7 +232,7 @@ export class BeaconSync implements IBeaconSync {
}
}

// If we stopped being synced and falled significantly behind, stop gossip
// If we stopped being synced and fallen significantly behind, stop gossip
else if (state !== SyncState.Synced) {
const syncDiff = this.chain.clock.currentSlot - this.chain.forkChoice.getHead().slot;
if (syncDiff > this.slotImportTolerance * 2) {
Expand Down
10 changes: 10 additions & 0 deletions packages/cli/src/options/beaconNodeOptions/sync.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ export type SyncArgs = {
"sync.disableProcessAsChainSegment"?: boolean;
"sync.disableRangeSync"?: boolean;
"sync.backfillBatchSize"?: number;
"sync.slotImportTolerance"?: number;
};

export function parseArgs(args: SyncArgs): IBeaconNodeOptions["sync"] {
Expand All @@ -14,6 +15,7 @@ export function parseArgs(args: SyncArgs): IBeaconNodeOptions["sync"] {
disableProcessAsChainSegment: args["sync.disableProcessAsChainSegment"],
backfillBatchSize: args["sync.backfillBatchSize"] ?? defaultOptions.sync.backfillBatchSize,
disableRangeSync: args["sync.disableRangeSync"],
slotImportTolerance: args["sync.slotImportTolerance"] ?? defaultOptions.sync.slotImportTolerance,
};
}

Expand All @@ -36,6 +38,14 @@ Use only for local networks with a single node, can be dangerous in regular netw
group: "sync",
},

"sync.slotImportTolerance": {
hidden: true,
type: "number",
description: "Number of slot tolerance to trigger range sync and to measure if node is synced.",
defaultDescription: String(defaultOptions.sync.slotImportTolerance),
group: "sync",
},

"sync.disableProcessAsChainSegment": {
hidden: true,
type: "boolean",
Expand Down
16 changes: 13 additions & 3 deletions packages/cli/test/sim/multi_fork.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -199,19 +199,29 @@ await checkpointSync.execution.job.stop();

// Unknown block sync
// ========================================================
const headForUnknownBlockSync = await env.nodes[0].beacon.api.beacon.getBlockV2("head");
ApiError.assert(headForUnknownBlockSync);
const unknownBlockSync = await env.createNodePair({
id: "unknown-block-sync-node",
beacon: {
type: BeaconClient.Lodestar,
options: {clientOptions: {"network.allowPublishToZeroPeers": true, "sync.disableRangeSync": true}},
options: {
clientOptions: {
"network.allowPublishToZeroPeers": true,
"sync.disableRangeSync": true,
// unknownBlockSync node start when other nodes are multiple epoch ahead and
// unknown block sync can work only if the gap is maximum `slotImportTolerance * 2`
// default value for slotImportTolerance is one epoch, so if gap is more than 2 epoch
// unknown block sync will not work. So why we have to increase it for tests.
"sync.slotImportTolerance": headForUnknownBlockSync.response.data.message.slot / 2 + 2,
},
},
},
execution: ExecutionClient.Geth,
keysCount: 0,
});
await unknownBlockSync.execution.job.start();
await unknownBlockSync.beacon.job.start();
const headForUnknownBlockSync = await env.nodes[0].beacon.api.beacon.getBlockV2("head");
ApiError.assert(headForUnknownBlockSync);
await connectNewNode(unknownBlockSync, env.nodes);

// Wait for EL node to start and sync
Expand Down
1 change: 1 addition & 0 deletions packages/cli/test/unit/options/beaconNodeOptions.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -206,6 +206,7 @@ describe("options / beaconNodeOptions", () => {
},
sync: {
isSingleNode: true,
slotImportTolerance: 32,
disableProcessAsChainSegment: true,
backfillBatchSize: 64,
disableRangeSync: false,
Expand Down
Loading