refactor: move data availability validation out of state transition to allow optimistic sync

packages/beacon-node/src/chain/blocks/index.ts

@@ -57,7 +57,11 @@ export async function processBlocks(
   }
 
   try {
-    const {relevantBlocks, parentSlots, parentBlock} = verifyBlocksSanityChecks(this, blocks, opts);
+    const {relevantBlocks, dataAvailabilityStatuses, parentSlots, parentBlock} = verifyBlocksSanityChecks(
+      this,
+      blocks,
+      opts
+    );
 
     // No relevant blocks, skip verifyBlocksInEpoch()
     if (relevantBlocks.length === 0 || parentBlock === null) {
@@ -71,6 +75,7 @@ export async function processBlocks(
       this,
       parentBlock,
       relevantBlocks,
+      dataAvailabilityStatuses,
       opts
     );
 
@@ -90,6 +95,9 @@ export async function processBlocks(
         postState: postStates[i],
         parentBlockSlot: parentSlots[i],
         executionStatus: executionStatuses[i],
+        // Currently dataAvailableStatus is not used upstream but that can change if we
+        // start supporting optimistic syncing/processing
+        dataAvailableStatus: dataAvailabilityStatuses[i],
         proposerBalanceDelta: proposerBalanceDeltas[i],
         // TODO: Make this param mandatory and capture in gossip
         seenTimestampSec: opts.seenTimestampSec ?? Math.floor(Date.now() / 1000),

packages/beacon-node/src/chain/blocks/types.ts

@@ -1,4 +1,4 @@
-import {CachedBeaconStateAllForks, computeEpochAtSlot} from "@lodestar/state-transition";
+import {CachedBeaconStateAllForks, computeEpochAtSlot, DataAvailableStatus} from "@lodestar/state-transition";
 import {MaybeValidExecutionStatus} from "@lodestar/fork-choice";
 import {allForks, deneb, Slot, WithOptionalBytes} from "@lodestar/types";
 import {ForkSeq, MIN_EPOCHS_FOR_BLOB_SIDECARS_REQUESTS} from "@lodestar/params";
@@ -101,6 +101,7 @@ export type FullyVerifiedBlock = {
    * used in optimistic sync or for merge block
    */
   executionStatus: MaybeValidExecutionStatus;
+  dataAvailableStatus: DataAvailableStatus;
   /** Seen timestamp seconds */
   seenTimestampSec: number;
 };

packages/beacon-node/src/chain/blocks/verifyBlock.ts

@@ -2,6 +2,7 @@ import {
   CachedBeaconStateAllForks,
   computeEpochAtSlot,
   isStateValidatorsNodesPopulated,
+  DataAvailableStatus,
 } from "@lodestar/state-transition";
 import {bellatrix} from "@lodestar/types";
 import {ForkName} from "@lodestar/params";
@@ -35,6 +36,7 @@ export async function verifyBlocksInEpoch(
   this: BeaconChain,
   parentBlock: ProtoBlock,
   blocksInput: BlockInput[],
+  dataAvailabilityStatuses: DataAvailableStatus[],
   opts: BlockProcessOpts & ImportBlockOpts
 ): Promise<{
   postStates: CachedBeaconStateAllForks[];
@@ -87,7 +89,15 @@ export async function verifyBlocksInEpoch(
       verifyBlocksExecutionPayload(this, parentBlock, blocks, preState0, abortController.signal, opts),
       // Run state transition only
       // TODO: Ensure it yields to allow flushing to workers and engine API
-      verifyBlocksStateTransitionOnly(preState0, blocksInput, this.logger, this.metrics, abortController.signal, opts),
+      verifyBlocksStateTransitionOnly(
+        preState0,
+        blocksInput,
+        dataAvailabilityStatuses,
+        this.logger,
+        this.metrics,
+        abortController.signal,
+        opts
+      ),
 
       // All signatures at once
       verifyBlocksSignatures(this.bls, this.logger, this.metrics, preState0, blocks, opts),

packages/beacon-node/src/chain/blocks/verifyBlocksSanityChecks.ts

@@ -1,11 +1,12 @@
-import {computeStartSlotAtEpoch} from "@lodestar/state-transition";
+import {computeStartSlotAtEpoch, DataAvailableStatus} from "@lodestar/state-transition";
 import {ChainForkConfig} from "@lodestar/config";
 import {IForkChoice, ProtoBlock} from "@lodestar/fork-choice";
-import {Slot, WithOptionalBytes} from "@lodestar/types";
+import {Slot, deneb, WithOptionalBytes} from "@lodestar/types";
 import {toHexString} from "@lodestar/utils";
 import {IClock} from "../../util/clock.js";
 import {BlockError, BlockErrorCode} from "../errors/index.js";
-import {BlockInput, ImportBlockOpts} from "./types.js";
+import {validateBlobsSidecar} from "../validation/blobsSidecar.js";
+import {BlockInput, BlockInputType, ImportBlockOpts} from "./types.js";
 
 /**
  * Verifies some early cheap sanity checks on the block before running the full state transition.
@@ -23,12 +24,18 @@ export function verifyBlocksSanityChecks(
   chain: {forkChoice: IForkChoice; clock: IClock; config: ChainForkConfig},
   blocks: WithOptionalBytes<BlockInput>[],
   opts: ImportBlockOpts
-): {relevantBlocks: WithOptionalBytes<BlockInput>[]; parentSlots: Slot[]; parentBlock: ProtoBlock | null} {
+): {
+  relevantBlocks: WithOptionalBytes<BlockInput>[];
+  dataAvailabilityStatuses: DataAvailableStatus[];
+  parentSlots: Slot[];
+  parentBlock: ProtoBlock | null;
+} {
   if (blocks.length === 0) {
     throw Error("Empty partiallyVerifiedBlocks");
   }
 
   const relevantBlocks: WithOptionalBytes<BlockInput>[] = [];
+  const dataAvailabilityStatuses: DataAvailableStatus[] = [];
   const parentSlots: Slot[] = [];
   let parentBlock: ProtoBlock | null = null;
 
@@ -57,6 +64,10 @@ export function verifyBlocksSanityChecks(
       }
     }
 
+    // Validate status of only not yet finalized blocks, we don't need yet to propogate the status
+    // as it is not used upstream anywhere
+    const dataAvailabilityStatus = maybeValidateBlobs(chain.config, blockInput, opts);
+
     let parentBlockSlot: Slot;
 
     if (relevantBlocks.length > 0) {
@@ -94,6 +105,7 @@ export function verifyBlocksSanityChecks(
 
     // Block is relevant
     relevantBlocks.push(blockInput);
+    dataAvailabilityStatuses.push(dataAvailabilityStatus);
     parentSlots.push(parentBlockSlot);
   }
 
@@ -103,5 +115,32 @@ export function verifyBlocksSanityChecks(
     throw Error(`Internal error, parentBlock should not be null for relevantBlocks=${relevantBlocks.length}`);
   }
 
-  return {relevantBlocks, parentSlots, parentBlock};
+  return {relevantBlocks, dataAvailabilityStatuses, parentSlots, parentBlock};
+}
+
+function maybeValidateBlobs(
+  config: ChainForkConfig,
+  blockInput: BlockInput,
+  opts: ImportBlockOpts
+): DataAvailableStatus {
+  // TODO Deneb: Make switch verify it's exhaustive
+  switch (blockInput.type) {
+    case BlockInputType.postDeneb: {
+      if (opts.validBlobsSidecar) {
+        return DataAvailableStatus.available;
+      }
+
+      const {block, blobs} = blockInput;
+      const blockSlot = block.message.slot;
+      const {blobKzgCommitments} = (block as deneb.SignedBeaconBlock).message.body;
+      const beaconBlockRoot = config.getForkTypes(blockSlot).BeaconBlock.hashTreeRoot(block.message);
+      // TODO Deneb: This function throws un-typed errors
+      validateBlobsSidecar(blockSlot, beaconBlockRoot, blobKzgCommitments, blobs);
+
+      return DataAvailableStatus.available;
+    }
+
+    case BlockInputType.preDeneb:
+      return DataAvailableStatus.preDeneb;
+  }
 }

packages/beacon-node/src/chain/blocks/verifyBlocksStateTransitionOnly.ts

@@ -4,15 +4,12 @@ import {
   ExecutionPayloadStatus,
   DataAvailableStatus,
 } from "@lodestar/state-transition";
-import {deneb} from "@lodestar/types";
 import {ErrorAborted, Logger, sleep} from "@lodestar/utils";
-import {ChainForkConfig} from "@lodestar/config";
 import {Metrics} from "../../metrics/index.js";
 import {BlockError, BlockErrorCode} from "../errors/index.js";
 import {BlockProcessOpts} from "../options.js";
 import {byteArrayEquals} from "../../util/bytes.js";
-import {validateBlobsSidecar} from "../validation/blobsSidecar.js";
-import {BlockInput, BlockInputType, ImportBlockOpts} from "./types.js";
+import {BlockInput, ImportBlockOpts} from "./types.js";
 
 /**
  * Verifies 1 or more blocks are fully valid running the full state transition; from a linear sequence of blocks.
@@ -25,24 +22,20 @@ import {BlockInput, BlockInputType, ImportBlockOpts} from "./types.js";
 export async function verifyBlocksStateTransitionOnly(
   preState0: CachedBeaconStateAllForks,
   blocks: BlockInput[],
+  dataAvailabilityStatuses: DataAvailableStatus[],
   logger: Logger,
   metrics: Metrics | null,
   signal: AbortSignal,
   opts: BlockProcessOpts & ImportBlockOpts
 ): Promise<{postStates: CachedBeaconStateAllForks[]; proposerBalanceDeltas: number[]}> {
-  const {config} = preState0;
   const postStates: CachedBeaconStateAllForks[] = [];
   const proposerBalanceDeltas: number[] = [];
 
   for (let i = 0; i < blocks.length; i++) {
     const {validProposerSignature, validSignatures} = opts;
     const {block} = blocks[i];
     const preState = i === 0 ? preState0 : postStates[i - 1];
-
-    // TODO Deneb: Is the best place here to call validateBlobsSidecar()?
-    // TODO Deneb: Gossip may already call validateBlobsSidecar, add some flag to de-dup from here
-    // TODO Deneb: For sync if this function is expensive, consider adding sleep(0) if metrics show it
-    const dataAvailableStatus = maybeValidateBlobs(config, blocks[i], opts);
+    const dataAvailableStatus = dataAvailabilityStatuses[i];
 
     // STFN - per_slot_processing() + per_block_processing()
     // NOTE: `regen.getPreState()` should have dialed forward the state already caching checkpoint states
@@ -54,7 +47,6 @@ export async function verifyBlocksStateTransitionOnly(
         // NOTE: Assume valid for now while sending payload to execution engine in parallel
         // Latter verifyBlocksInEpoch() will make sure that payload is indeed valid
         executionPayloadStatus: ExecutionPayloadStatus.valid,
-        // TODO Deneb: Data is validated above for
         dataAvailableStatus,
         // false because it's verified below with better error typing
         verifyStateRoot: false,
@@ -107,30 +99,3 @@ export async function verifyBlocksStateTransitionOnly(
 
   return {postStates, proposerBalanceDeltas};
 }
-
-function maybeValidateBlobs(
-  config: ChainForkConfig,
-  blockInput: BlockInput,
-  opts: ImportBlockOpts
-): DataAvailableStatus {
-  // TODO Deneb: Make switch verify it's exhaustive
-  switch (blockInput.type) {
-    case BlockInputType.postDeneb: {
-      if (opts.validBlobsSidecar) {
-        return DataAvailableStatus.available;
-      }
-
-      const {block, blobs} = blockInput;
-      const blockSlot = block.message.slot;
-      const {blobKzgCommitments} = (block as deneb.SignedBeaconBlock).message.body;
-      const beaconBlockRoot = config.getForkTypes(blockSlot).BeaconBlock.hashTreeRoot(block.message);
-      // TODO Deneb: This function throws un-typed errors
-      validateBlobsSidecar(blockSlot, beaconBlockRoot, blobKzgCommitments, blobs);
-
-      return DataAvailableStatus.available;
-    }
-
-    case BlockInputType.preDeneb:
-      return DataAvailableStatus.preDeneb;
-  }
-}

packages/state-transition/src/block/index.ts

@@ -63,18 +63,10 @@ export function processBlock(
 
   if (fork >= ForkSeq.deneb) {
     processBlobKzgCommitments(block.body as deneb.BeaconBlockBody);
-
-    // New in Deneb, note: Can sync optimistically without this condition, see note on `is_data_available`
-    // NOTE: Ommitted and should be verified beforehand
-
-    // assert is_data_available(block.slot, hash_tree_root(block), block.body.blob_kzg_commitments)
-    switch (externalData.dataAvailableStatus) {
-      case DataAvailableStatus.preDeneb:
-        throw Error("dataAvailableStatus preDeneb");
-      case DataAvailableStatus.notAvailable:
-        throw Error("dataAvailableStatus notAvailable");
-      case DataAvailableStatus.available:
-        break; // ok
+    // Only throw preDeneb so beacon can also sync/process blocks optimistically
+    // and let forkChoice handle it
+    if (externalData.dataAvailableStatus === DataAvailableStatus.preDeneb) {
+      throw Error("dataAvailableStatus preDeneb");
     }
   }
 }