Skip to content
Merged
Changes from 1 commit
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
33 changes: 9 additions & 24 deletions packages/beacon-node/test/e2e/sync/unknownBlockSync.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -158,15 +158,6 @@ describe("sync / unknown block sync thru gloas", () => {
100000,
({block}) => block === headRootHex
);
const maybeWaitForPayloadImported =
event === ChainEvent.unknownEnvelopeBlockRoot || event === ChainEvent.incompletePayloadEnvelope
? Promise.resolve()
: waitForEvent<routes.events.EventData[routes.events.EventType.executionPayload]>(
bn2.chain.emitter,
routes.events.EventType.executionPayload,
100000,
({blockRoot}) => blockRoot === headRootHex
);

const connected = Promise.all([onPeerConnect(bn2.network), onPeerConnect(bn.network)]);
await connect(bn2.network, bn.network);
Expand Down Expand Up @@ -235,22 +226,19 @@ describe("sync / unknown block sync thru gloas", () => {
throw Error("Unknown event type");
}

// Wait for the block root to be processed in node B. Payload-aware entrypoints should also import
// the separated payload envelope for the same root.
// Wait for the block root to be processed in node B. The unknown-block-sync flow imports
// parent payloads as needed to satisfy sanity checks but does not fetch the head block's
// own payload envelope (which would normally arrive via gossip on a live network).
await waitForSynced;

switch (event) {
case ChainEvent.incompletePayloadEnvelope: {
// After it syncs, send an incomplete payload envelope
// and assert the payload gets imported
const payloadInput = bn2.chain.seenPayloadEnvelopeInputCache.add({
blockRootHex: headRootHex,
forkName: bn2.config.getForkName(headSlot),
block: head,
sampledColumns: bn2.chain.custodyConfig.sampledColumns,
custodyColumns: bn2.chain.custodyConfig.custodyColumns,
timeCreatedSec: Math.floor(Date.now() / 1000),
});
// After it syncs, retrieve the PayloadEnvelopeInput created during block import
// and emit incompletePayloadEnvelope to exercise the sync handler.
const payloadInput = bn2.chain.seenPayloadEnvelopeInputCache.get(headRootHex);
if (!payloadInput) {
throw Error(`Expected PayloadEnvelopeInput for ${headRootHex} after block sync`);
}
Comment on lines +271 to +274

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.

medium

The incompletePayloadEnvelope test case triggers a payload sync by emitting an event but does not wait for the sync to complete. Since the sync happens in the background, the test might finish and close the node (via afterEach) before the payload is actually imported. This could lead to flakiness or a false positive where the sync logic is triggered but not actually completed. Consider waiting for the routes.events.EventType.executionPayload event after emitting the trigger to ensure the sync is fully exercised.

bn2.chain.emitter.emit(ChainEvent.incompletePayloadEnvelope, {
payloadInput,
peer: sourcePeerId,
Expand All @@ -261,9 +249,6 @@ describe("sync / unknown block sync thru gloas", () => {
default:
break;
}

// only await payload import for events that imply importing it
await maybeWaitForPayloadImported;

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

hmm might be that the test is just not waiting now? tests are passing but mayeb we a not asserting stuff anymore

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Case Drives head-payload import? Currently asserted?
blockUnknownParent no — only parent payloads head event only — correct
unknownBlockRoot no — only parent payloads head event only — correct
incompleteBlockInput no — only parent payloads head event only — correct
unknownEnvelopeBlockRoot yes head event only — gap
envelopeUnknownBlock (new) yes head + payload — fixed
incompletePayloadEnvelope yes head + payload — fixed

we are only missing unknownEnvelopeBlockRoot, gonna add that

});
}
});
Loading