fix: ensure block in forkchoice before validate by_root payload (#9479)

Author: nflaig

packages/beacon-node/src/sync/unknownBlock.ts

@@ -850,25 +850,28 @@ export class BlockInputSync {
     }
 
     const payloadInput = this.chain.seenPayloadEnvelopeInputCache.get(rootHex);
-    if (!payloadInput) {
-      if (!this.chain.forkChoice.hasBlockHex(rootHex)) {
-        // Column commitments live on the block body, so an envelope-only entry has to pull the block first.
-        if (!this.pendingBlocks.has(rootHex)) {
-          this.addByRootHex(rootHex);
-        }
+    if (!this.chain.forkChoice.hasBlockHex(rootHex)) {
+      // Block not in fork choice yet. payloadInput may be seeded from the block body during download, so a
+      // non-null payloadInput does not imply the block is imported; defer regardless and pull the block first.
+      // onBlockImported re-triggers the search to resume this envelope.
+      if (!this.pendingBlocks.has(rootHex)) {
+        this.addByRootHex(rootHex);
+      }
 
-        const pendingBlock = this.pendingBlocks.get(rootHex);
-        if (pendingBlock && this.network.getConnectedPeers().length > 0) {
-          await this.downloadBlock(pendingBlock);
-        }
-      } else {
-        this.logger.debug("Missing PayloadEnvelopeInput for known block while reconciling payload envelope", {
-          root: rootHex,
-        });
+      const pendingBlock = this.pendingBlocks.get(rootHex);
+      if (pendingBlock && this.network.getConnectedPeers().length > 0) {
+        await this.downloadBlock(pendingBlock);
       }
       return;
     }
 
+    if (!payloadInput) {
+      this.logger.debug("Missing PayloadEnvelopeInput for known block while reconciling payload envelope", {
+        root: rootHex,
+      });
+      return;
+    }
+
     if (!payloadInput.hasPayloadEnvelope()) {
       const validationResult = await wrapError(
         validateGossipExecutionPayloadEnvelope(this.chain, pendingPayload.envelope)
@@ -1073,11 +1076,11 @@ export class BlockInputSync {
         }
 
         payloadInput ??= this.chain.seenPayloadEnvelopeInputCache.get(rootHex);
-        if (!payloadInput) {
-          if (this.chain.forkChoice.hasBlockHex(rootHex)) {
-            throw new Error(`Missing PayloadEnvelopeInput for known block ${rootHex}`);
-          }
-          // Keep the validated envelope around, but wait for the block body before turning it into a full payload input.
+        if (!this.chain.forkChoice.hasBlockHex(rootHex)) {
+          // Block not in fork choice yet. Validating now would throw BLOCK_ROOT_UNKNOWN, so keep the downloaded
+          // envelope and wait for the block body; reconcilePayloadEnvelope validates once the block lands.
+          // payloadInput may be seeded from the block body during download, so a non-null payloadInput does not
+          // imply the block is imported.
           return {
             status: PendingPayloadInputStatus.waitingForBlock,
             envelope,
@@ -1086,6 +1089,11 @@ export class BlockInputSync {
           };
         }
 
+        if (!payloadInput) {
+          // Block is in fork choice but no PayloadEnvelopeInput exists, should have been created during block import.
+          throw new Error(`Missing PayloadEnvelopeInput for known block ${rootHex}`);
+        }
+
         if (!payloadInput.hasPayloadEnvelope()) {
           await validateGossipExecutionPayloadEnvelope(this.chain, envelope);
         }

packages/beacon-node/test/unit/sync/unknownBlock.test.ts

@@ -722,8 +722,8 @@ describe("UnknownBlockSync", () => {
 
     beforeEach(() => {
       vi.useFakeTimers({shouldAdvanceTime: true});
-      vi.mocked(validateGossipExecutionPayloadEnvelope).mockClear();
-      vi.mocked(validateGloasBlockDataColumnSidecars).mockClear();
+      vi.mocked(validateGossipExecutionPayloadEnvelope).mockReset().mockResolvedValue(undefined);
+      vi.mocked(validateGloasBlockDataColumnSidecars).mockReset().mockResolvedValue(undefined);
     });
 
     it("fetches and processes unknown envelope by root when payload input exists", async () => {
@@ -938,6 +938,107 @@ describe("UnknownBlockSync", () => {
       expect(processExecutionPayload).toHaveBeenCalledWith(payloadInput);
     });
 
+    it("defers envelope validation until the block is in fork choice when payload input is seeded from the block body", async () => {
+      const peer = await getRandPeerIdStr();
+      const {block, blockRoot, blockRootHex, payloadInput, envelope} = buildPayloadFixture({
+        blobCount: 0,
+        sampledColumns: [],
+        slot: 1,
+      });
+      const parentRootHex = toRootHex(block.message.parentRoot);
+
+      // payloadInput is seeded from the block body during download, so the cache returns it before the block
+      // is imported into fork choice. Block becomes known only once processBlock imports it.
+      let blockImported = false;
+      const knownRoots = new Set([parentRootHex]);
+
+      const sendExecutionPayloadEnvelopesByRoot = vi.fn().mockResolvedValue([envelope]);
+      const sendBeaconBlocksByRoot = vi.fn().mockResolvedValue([block]);
+      const processExecutionPayload = vi.fn().mockResolvedValue(undefined);
+
+      let emitter!: ChainEventEmitter;
+      const processBlock = vi.fn().mockImplementation(async () => {
+        blockImported = true;
+        knownRoots.add(blockRootHex);
+        emitter.emit(routes.events.EventType.block, {slot: 1, block: blockRootHex, executionOptimistic: false});
+      });
+
+      // Reproduce BLOCK_ROOT_UNKNOWN: validation rejects while the block is absent from fork choice. The fix must
+      // not call it until the block is imported.
+      vi.mocked(validateGossipExecutionPayloadEnvelope).mockImplementation(async () => {
+        if (!blockImported) {
+          throw new Error("EXECUTION_PAYLOAD_ENVELOPE_ERROR_BLOCK_ROOT_UNKNOWN");
+        }
+      });
+
+      ({emitter} = setupPayloadSyncTest({
+        chainOverrides: {
+          processBlock,
+          processExecutionPayload,
+          seenPayloadEnvelopeInputCache: {
+            add: vi.fn(),
+            get: vi.fn().mockImplementation((root: string) => (root === blockRootHex ? payloadInput : undefined)),
+            prune: vi.fn(),
+          } as unknown as IBeaconChain["seenPayloadEnvelopeInputCache"],
+          seenBlockInputCache: {
+            getByBlock: ({
+              block,
+              blockRootHex,
+              seenTimestampSec,
+              source,
+            }: {
+              block: gloas.SignedBeaconBlock;
+              blockRootHex: string;
+              seenTimestampSec: number;
+              source: BlockInputSource;
+            }) =>
+              createGloasBlockInput({
+                block,
+                blockRootHex,
+                seenTimestampSec,
+                source,
+              }),
+            prune: vi.fn(),
+          } as unknown as SeenBlockInput,
+          forkChoice: {
+            hasPayloadHexUnsafe: vi.fn().mockReturnValue(false),
+            hasBlockHex: vi.fn().mockImplementation((root: string) => knownRoots.has(root)),
+            getBlockHexAndBlockHash: vi
+              .fn()
+              .mockImplementation((root: string, hash: string) =>
+                root === parentRootHex &&
+                hash === toRootHex(block.message.body.signedExecutionPayloadBid.message.parentBlockHash)
+                  ? ({slot: 0} as ProtoBlock)
+                  : null
+              ),
+            getFinalizedBlock: vi.fn().mockReturnValue({slot: 0} as ProtoBlock),
+          } as unknown as IForkChoice,
+        },
+        networkOverrides: {
+          sendExecutionPayloadEnvelopesByRoot,
+          sendBeaconBlocksByRoot,
+        },
+        peers: [{peerId: peer}],
+      }));
+
+      emitter.emit(ChainEvent.unknownEnvelopeBlockRoot, {
+        rootHex: blockRootHex,
+        peer,
+        source: BlockInputSource.gossip,
+      });
+
+      await sleep(80);
+
+      // Envelope downloaded, block pulled because validation was deferred, then envelope validated and processed
+      // only after the block landed in fork choice.
+      expect(sendExecutionPayloadEnvelopesByRoot).toHaveBeenCalledWith(peer, [blockRoot]);
+      expect(sendBeaconBlocksByRoot).toHaveBeenCalledWith(peer, [blockRoot]);
+      expect(processBlock).toHaveBeenCalledTimes(1);
+      expect(validateGossipExecutionPayloadEnvelope).toHaveBeenCalledOnce();
+      expect(processExecutionPayload).toHaveBeenCalledTimes(1);
+      expect(processExecutionPayload).toHaveBeenCalledWith(payloadInput);
+    });
+
     it("downloads the block and retries payload import when EL reports block not in fork choice", async () => {
       const peer = await getRandPeerIdStr();
       const {block, blockRoot, blockRootHex, payloadInput, envelope} = buildPayloadFixture({