refactor: cleanup some of the deneb todos (#6047)

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

@@ -47,7 +47,6 @@ type BlockInputCacheType = {
 };
 
 const MAX_GOSSIPINPUT_CACHE = 5;
-// TODO deneb: export from types package
 // ssz.deneb.BlobSidecars.elementType.fixedSize;
 const BLOBSIDECAR_FIXED_SIZE = 131256;
 

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

@@ -123,7 +123,6 @@ function maybeValidateBlobs(
   blockInput: BlockInput,
   opts: ImportBlockOpts
 ): DataAvailableStatus {
-  // TODO Deneb: Make switch verify it's exhaustive
   switch (blockInput.type) {
     case BlockInputType.postDeneb: {
       if (opts.validBlobSidecars) {
@@ -134,7 +133,6 @@ function maybeValidateBlobs(
       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
       validateBlobSidecars(blockSlot, beaconBlockRoot, blobKzgCommitments, blobs);
 
       return DataAvailableStatus.available;

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

@@ -130,7 +130,6 @@ export class BeaconChain implements IBeaconChain {
 
   readonly beaconProposerCache: BeaconProposerCache;
   readonly checkpointBalancesCache: CheckpointBalancesCache;
-  // TODO DENEB: Prune data structure every time period, for both old entries
   /** Map keyed by executionPayload.blockHash of the block for those blobs */
   readonly producedBlobSidecarsCache = new Map<BlockHash, {blobSidecars: deneb.BlobSidecars; slot: Slot}>();
   readonly producedBlindedBlobSidecarsCache = new Map<

packages/beacon-node/src/chain/errors/blobSidecarError.ts

@@ -1,18 +1,27 @@
-import {Slot} from "@lodestar/types";
+import {Slot, RootHex, ValidatorIndex} from "@lodestar/types";
 import {GossipActionError} from "./gossipValidation.js";
 
 export enum BlobSidecarErrorCode {
-  INVALID_INDEX = "BLOBS_SIDECAR_ERROR_INVALID_INDEX",
+  INVALID_INDEX = "BLOB_SIDECAR_ERROR_INVALID_INDEX",
   /** !bls.KeyValidate(block.body.blob_kzg_commitments[i]) */
-  INVALID_KZG = "BLOBS_SIDECAR_ERROR_INVALID_KZG",
+  INVALID_KZG = "BLOB_SIDECAR_ERROR_INVALID_KZG",
   /** !verify_kzg_commitments_against_transactions(block.body.execution_payload.transactions, block.body.blob_kzg_commitments) */
-  INVALID_KZG_TXS = "BLOBS_SIDECAR_ERROR_INVALID_KZG_TXS",
+  INVALID_KZG_TXS = "BLOB_SIDECAR_ERROR_INVALID_KZG_TXS",
   /** sidecar.beacon_block_slot != block.slot */
-  INCORRECT_SLOT = "BLOBS_SIDECAR_ERROR_INCORRECT_SLOT",
+  INCORRECT_SLOT = "BLOB_SIDECAR_ERROR_INCORRECT_SLOT",
   /** BLSFieldElement in valid range (x < BLS_MODULUS) */
-  INVALID_BLOB = "BLOBS_SIDECAR_ERROR_INVALID_BLOB",
+  INVALID_BLOB = "BLOB_SIDECAR_ERROR_INVALID_BLOB",
   /** !bls.KeyValidate(blobs_sidecar.kzg_aggregated_proof) */
   INVALID_KZG_PROOF = "BLOBS_SIDECAR_ERROR_INVALID_KZG_PROOF",
+
+  // following errors are adapted from the block errors
+  FUTURE_SLOT = "BLOB_SIDECAR_ERROR_FUTURE_SLOT",
+  WOULD_REVERT_FINALIZED_SLOT = "BLOB_SIDECAR_ERROR_WOULD_REVERT_FINALIZED_SLOT",
+  ALREADY_KNOWN = "BLOB_SIDECAR_ERROR_ALREADY_KNOWN",
+  PARENT_UNKNOWN = "BLOB_SIDECAR_ERROR_PARENT_UNKNOWN",
+  NOT_LATER_THAN_PARENT = "BLOB_SIDECAR_ERROR_NOT_LATER_THAN_PARENT",
+  PROPOSAL_SIGNATURE_INVALID = "BLOB_SIDECAR_ERROR_PROPOSAL_SIGNATURE_INVALID",
+  INCORRECT_PROPOSER = "BLOB_SIDECAR_ERROR_INCORRECT_PROPOSER",
 }
 
 export type BlobSidecarErrorType =
@@ -21,6 +30,13 @@ export type BlobSidecarErrorType =
   | {code: BlobSidecarErrorCode.INVALID_KZG_TXS}
   | {code: BlobSidecarErrorCode.INCORRECT_SLOT; blockSlot: Slot; blobSlot: Slot; blobIdx: number}
   | {code: BlobSidecarErrorCode.INVALID_BLOB; blobIdx: number}
-  | {code: BlobSidecarErrorCode.INVALID_KZG_PROOF; blobIdx: number};
+  | {code: BlobSidecarErrorCode.INVALID_KZG_PROOF; blobIdx: number}
+  | {code: BlobSidecarErrorCode.FUTURE_SLOT; blockSlot: Slot; currentSlot: Slot}
+  | {code: BlobSidecarErrorCode.WOULD_REVERT_FINALIZED_SLOT; blockSlot: Slot; finalizedSlot: Slot}
+  | {code: BlobSidecarErrorCode.ALREADY_KNOWN; root: RootHex}
+  | {code: BlobSidecarErrorCode.PARENT_UNKNOWN; parentRoot: RootHex}
+  | {code: BlobSidecarErrorCode.NOT_LATER_THAN_PARENT; parentSlot: Slot; slot: Slot}
+  | {code: BlobSidecarErrorCode.PROPOSAL_SIGNATURE_INVALID}
+  | {code: BlobSidecarErrorCode.INCORRECT_PROPOSER; proposerIndex: ValidatorIndex};
 
-export class BlobSidecarError extends GossipActionError<BlobSidecarErrorType> {}
+export class BlobSidecarGossipError extends GossipActionError<BlobSidecarErrorType> {}

packages/beacon-node/src/chain/validation/blobSidecar.ts

@@ -3,16 +3,13 @@ import {deneb, Root, Slot} from "@lodestar/types";
 import {toHex} from "@lodestar/utils";
 import {getBlobProposerSignatureSet, computeStartSlotAtEpoch} from "@lodestar/state-transition";
 
-import {BlobSidecarError, BlobSidecarErrorCode} from "../errors/blobSidecarError.js";
+import {BlobSidecarGossipError, BlobSidecarErrorCode} from "../errors/blobSidecarError.js";
 import {GossipAction} from "../errors/gossipValidation.js";
 import {ckzg} from "../../util/kzg.js";
 import {byteArrayEquals} from "../../util/bytes.js";
 import {IBeaconChain} from "../interface.js";
 import {RegenCaller} from "../regen/index.js";
 
-// TODO: freetheblobs define blobs own gossip error
-import {BlockGossipError, BlockErrorCode} from "../errors/index.js";
-
 export async function validateGossipBlobSidecar(
   config: ChainForkConfig,
   chain: IBeaconChain,
@@ -24,7 +21,7 @@ export async function validateGossipBlobSidecar(
 
   // [REJECT] The sidecar is for the correct topic -- i.e. sidecar.index matches the topic {index}.
   if (blobSidecar.index !== gossipIndex) {
-    throw new BlobSidecarError(GossipAction.REJECT, {
+    throw new BlobSidecarGossipError(GossipAction.REJECT, {
       code: BlobSidecarErrorCode.INVALID_INDEX,
       blobIdx: blobSidecar.index,
       gossipIndex,
@@ -36,8 +33,8 @@ export async function validateGossipBlobSidecar(
   // the appropriate slot).
   const currentSlotWithGossipDisparity = chain.clock.currentSlotWithGossipDisparity;
   if (currentSlotWithGossipDisparity < blobSlot) {
-    throw new BlockGossipError(GossipAction.IGNORE, {
-      code: BlockErrorCode.FUTURE_SLOT,
+    throw new BlobSidecarGossipError(GossipAction.IGNORE, {
+      code: BlobSidecarErrorCode.FUTURE_SLOT,
       currentSlot: currentSlotWithGossipDisparity,
       blockSlot: blobSlot,
     });
@@ -48,8 +45,8 @@ export async function validateGossipBlobSidecar(
   const finalizedCheckpoint = chain.forkChoice.getFinalizedCheckpoint();
   const finalizedSlot = computeStartSlotAtEpoch(finalizedCheckpoint.epoch);
   if (blobSlot <= finalizedSlot) {
-    throw new BlockGossipError(GossipAction.IGNORE, {
-      code: BlockErrorCode.WOULD_REVERT_FINALIZED_SLOT,
+    throw new BlobSidecarGossipError(GossipAction.IGNORE, {
+      code: BlobSidecarErrorCode.WOULD_REVERT_FINALIZED_SLOT,
       blockSlot: blobSlot,
       finalizedSlot,
     });
@@ -63,7 +60,7 @@ export async function validateGossipBlobSidecar(
   // already know this block.
   const blockRoot = toHex(blobSidecar.blockRoot);
   if (chain.forkChoice.getBlockHex(blockRoot) !== null) {
-    throw new BlockGossipError(GossipAction.IGNORE, {code: BlockErrorCode.ALREADY_KNOWN, root: blockRoot});
+    throw new BlobSidecarGossipError(GossipAction.IGNORE, {code: BlobSidecarErrorCode.ALREADY_KNOWN, root: blockRoot});
   }
 
   // TODO: freetheblobs - check for badblock
@@ -85,13 +82,13 @@ export async function validateGossipBlobSidecar(
     //    descend from the finalized root.
     // (Non-Lighthouse): Since we prune all blocks non-descendant from finalized checking the `db.block` database won't be useful to guard
     // against known bad fork blocks, so we throw PARENT_UNKNOWN for cases (1) and (2)
-    throw new BlockGossipError(GossipAction.IGNORE, {code: BlockErrorCode.PARENT_UNKNOWN, parentRoot});
+    throw new BlobSidecarGossipError(GossipAction.IGNORE, {code: BlobSidecarErrorCode.PARENT_UNKNOWN, parentRoot});
   }
 
   // [REJECT] The blob is from a higher slot than its parent.
   if (parentBlock.slot >= blobSlot) {
-    throw new BlockGossipError(GossipAction.IGNORE, {
-      code: BlockErrorCode.NOT_LATER_THAN_PARENT,
+    throw new BlobSidecarGossipError(GossipAction.IGNORE, {
+      code: BlobSidecarErrorCode.NOT_LATER_THAN_PARENT,
       parentSlot: parentBlock.slot,
       slot: blobSlot,
     });
@@ -106,16 +103,16 @@ export async function validateGossipBlobSidecar(
   const blockState = await chain.regen
     .getBlockSlotState(parentRoot, blobSlot, {dontTransferCache: true}, RegenCaller.validateGossipBlob)
     .catch(() => {
-      throw new BlockGossipError(GossipAction.IGNORE, {code: BlockErrorCode.PARENT_UNKNOWN, parentRoot});
+      throw new BlobSidecarGossipError(GossipAction.IGNORE, {code: BlobSidecarErrorCode.PARENT_UNKNOWN, parentRoot});
     });
 
   // _[REJECT]_ The proposer signature, `signed_blob_sidecar.signature`, is valid with respect to the
   // `sidecar.proposer_index` pubkey.
   const signatureSet = getBlobProposerSignatureSet(blockState, signedBlob);
   // Don't batch so verification is not delayed
   if (!(await chain.bls.verifySignatureSets([signatureSet], {verifyOnMainThread: true}))) {
-    throw new BlockGossipError(GossipAction.REJECT, {
-      code: BlockErrorCode.PROPOSAL_SIGNATURE_INVALID,
+    throw new BlobSidecarGossipError(GossipAction.REJECT, {
+      code: BlobSidecarErrorCode.PROPOSAL_SIGNATURE_INVALID,
     });
   }
 
@@ -132,11 +129,21 @@ export async function validateGossipBlobSidecar(
   // a case _do not_ `REJECT`, instead `IGNORE` this message.
   const proposerIndex = blobSidecar.proposerIndex;
   if (blockState.epochCtx.getBeaconProposer(blobSlot) !== proposerIndex) {
-    throw new BlockGossipError(GossipAction.REJECT, {code: BlockErrorCode.INCORRECT_PROPOSER, proposerIndex});
+    throw new BlobSidecarGossipError(GossipAction.REJECT, {
+      code: BlobSidecarErrorCode.INCORRECT_PROPOSER,
+      proposerIndex,
+    });
   }
 
   // blob, proof and commitment as a valid BLS G1 point gets verified in batch validation
-  validateBlobsAndProofs([blobSidecar.kzgCommitment], [blobSidecar.blob], [blobSidecar.kzgProof]);
+  try {
+    validateBlobsAndProofs([blobSidecar.kzgCommitment], [blobSidecar.blob], [blobSidecar.kzgProof]);
+  } catch (_e) {
+    throw new BlobSidecarGossipError(GossipAction.REJECT, {
+      code: BlobSidecarErrorCode.INVALID_KZG_PROOF,
+      blobIdx: blobSidecar.index,
+    });
+  }
 }
 
 // https://github.com/ethereum/consensus-specs/blob/dev/specs/eip4844/beacon-chain.md#validate_blobs_sidecar

packages/beacon-node/src/db/repositories/blobSidecars.ts

@@ -16,7 +16,6 @@ export const blobSidecarsWrapperSsz = new ContainerType(
 export type BlobSidecarsWrapper = ValueOf<typeof blobSidecarsWrapperSsz>;
 
 export const BLOB_SIDECARS_IN_WRAPPER_INDEX = 44;
-// TODO deneb: export from types package
 // ssz.deneb.BlobSidecars.elementType.fixedSize;
 export const BLOBSIDECAR_FIXED_SIZE = 131256;
 

packages/beacon-node/src/execution/engine/mock.ts

@@ -136,7 +136,7 @@ export class ExecutionEngineMockBackend implements JsonRpcBackend {
    */
   private notifyNewPayload(
     executionPayloadRpc: EngineApiRpcParamTypes["engine_newPayloadV1"][0],
-    // TODO deneb: add versionedHashes validation
+    // add versionedHashes validation later if required
     _versionedHashes?: EngineApiRpcParamTypes["engine_newPayloadV3"][1]
   ): EngineApiRpcReturnTypes["engine_newPayloadV1"] {
     const blockHash = executionPayloadRpc.blockHash;

packages/beacon-node/src/metrics/metrics/lodestar.ts

@@ -650,6 +650,13 @@ export function createLodestarMetrics(
         labelNames: ["error"],
       }),
     },
+    gossipBlob: {
+      receivedToGossipValidate: register.histogram({
+        name: "lodestar_gossip_blob_received_to_gossip_validate",
+        help: "Time elapsed between blob received and blob validated",
+        buckets: [0.05, 0.1, 0.2, 0.5, 1, 1.5, 2, 4],
+      }),
+    },
     importBlock: {
       persistBlockNoSerializedDataCount: register.gauge({
         name: "lodestar_import_block_persist_block_no_serialized_data_count",

packages/beacon-node/src/network/processor/gossipHandlers.ts

@@ -12,6 +12,8 @@ import {
   BlockError,
   BlockErrorCode,
   BlockGossipError,
+  BlobSidecarErrorCode,
+  BlobSidecarGossipError,
   GossipAction,
   GossipActionError,
   SyncCommitteeError,
@@ -143,19 +145,18 @@ function getDefaultHandlers(modules: ValidatorFnsModules, options: GossipHandler
 
     try {
       await validateGossipBlock(config, chain, signedBlock, fork);
-      // TODO: freetheblobs add some serialized data
       return blockInput;
     } catch (e) {
       if (e instanceof BlockGossipError) {
         // Don't trigger this yet if full block and blobs haven't arrived yet
-        if (e instanceof BlockGossipError && e.type.code === BlockErrorCode.PARENT_UNKNOWN && blockInput !== null) {
+        if (e.type.code === BlockErrorCode.PARENT_UNKNOWN && blockInput !== null) {
           logger.debug("Gossip block has error", {slot, root: blockHex, code: e.type.code});
           events.emit(NetworkEvent.unknownBlockParent, {blockInput, peer: peerIdStr});
         }
-      }
 
-      if (e instanceof BlockGossipError && e.action === GossipAction.REJECT) {
-        chain.persistInvalidSszValue(forkTypes.SignedBeaconBlock, signedBlock, `gossip_reject_slot_${slot}`);
+        if (e.action === GossipAction.REJECT) {
+          chain.persistInvalidSszValue(forkTypes.SignedBeaconBlock, signedBlock, `gossip_reject_slot_${slot}`);
+        }
       }
 
       throw e;
@@ -180,8 +181,7 @@ function getDefaultHandlers(modules: ValidatorFnsModules, options: GossipHandler
       blobBytes,
     });
 
-    // TODO: freetheblobs
-    // metrics?.gossipBlock.receivedToGossipValidate.observe(recvToVal);
+    metrics?.gossipBlob.receivedToGossipValidate.observe(recvToVal);
     logger.verbose("Received gossip blob", {
       slot: slot,
       root: blockHex,
@@ -197,16 +197,16 @@ function getDefaultHandlers(modules: ValidatorFnsModules, options: GossipHandler
       await validateGossipBlobSidecar(config, chain, signedBlob, gossipIndex);
       return blockInput;
     } catch (e) {
-      if (e instanceof BlockGossipError) {
+      if (e instanceof BlobSidecarGossipError) {
         // Don't trigger this yet if full block and blobs haven't arrived yet
-        if (e instanceof BlockGossipError && e.type.code === BlockErrorCode.PARENT_UNKNOWN && blockInput !== null) {
+        if (e.type.code === BlobSidecarErrorCode.PARENT_UNKNOWN && blockInput !== null) {
           logger.debug("Gossip blob has error", {slot, root: blockHex, code: e.type.code});
           events.emit(NetworkEvent.unknownBlockParent, {blockInput, peer: peerIdStr});
         }
-      }
 
-      if (e instanceof BlockGossipError && e.action === GossipAction.REJECT) {
-        chain.persistInvalidSszValue(ssz.deneb.SignedBlobSidecar, signedBlob, `gossip_reject_slot_${slot}`);
+        if (e.action === GossipAction.REJECT) {
+          chain.persistInvalidSszValue(ssz.deneb.SignedBlobSidecar, signedBlob, `gossip_reject_slot_${slot}`);
+        }
       }
 
       throw e;

packages/beacon-node/src/network/reqresp/beaconBlocksMaybeBlobsByRoot.ts

@@ -1,5 +1,6 @@
 import {ChainForkConfig} from "@lodestar/config";
-import {Epoch, phase0, deneb, Slot} from "@lodestar/types";
+import {phase0, deneb} from "@lodestar/types";
+import {ForkSeq} from "@lodestar/params";
 import {BlockInput, BlockSource} from "../../chain/blocks/types.js";
 import {PeerIdStr} from "../../util/peerId.js";
 import {INetwork} from "../interface.js";
@@ -9,19 +10,21 @@ export async function beaconBlocksMaybeBlobsByRoot(
   config: ChainForkConfig,
   network: INetwork,
   peerId: PeerIdStr,
-  request: phase0.BeaconBlocksByRootRequest,
-  // TODO DENEB: Some validations can be done to see if this is deneb block, ignoring below two for now
-  _currentSlot: Epoch,
-  _finalizedSlot: Slot
+  request: phase0.BeaconBlocksByRootRequest
 ): Promise<BlockInput[]> {
   const allBlocks = await network.sendBeaconBlocksByRoot(peerId, request);
   const blobIdentifiers: deneb.BlobIdentifier[] = [];
 
   for (const block of allBlocks) {
-    const blockRoot = config.getForkTypes(block.data.message.slot).BeaconBlock.hashTreeRoot(block.data.message);
-    const blobKzgCommitmentsLen = (block.data.message.body as deneb.BeaconBlockBody).blobKzgCommitments?.length ?? 0;
-    for (let index = 0; index < blobKzgCommitmentsLen; index++) {
-      blobIdentifiers.push({blockRoot, index});
+    const slot = block.data.message.slot;
+    const blockRoot = config.getForkTypes(slot).BeaconBlock.hashTreeRoot(block.data.message);
+    const fork = config.getForkName(slot);
+
+    if (ForkSeq[fork] >= ForkSeq.deneb) {
+      const blobKzgCommitmentsLen = (block.data.message.body as deneb.BeaconBlockBody).blobKzgCommitments.length;
+      for (let index = 0; index < blobKzgCommitmentsLen; index++) {
+        blobIdentifiers.push({blockRoot, index});
+      }
     }
   }