Review eip-4844 sync code

packages/beacon-node/package.json

@@ -131,6 +131,7 @@
     "@multiformats/multiaddr": "^11.0.0",
     "@types/datastore-level": "^3.0.0",
     "buffer-xor": "^2.0.2",
+    "c-kzg": "^1.0.9",
     "cross-fetch": "^3.1.4",
     "datastore-core": "^8.0.1",
     "datastore-level": "^9.0.1",

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

@@ -7,13 +7,11 @@ import {IChainForkConfig} from "@lodestar/config";
 export enum BlockInputType {
   preDeneb = "preDeneb",
   postDeneb = "postDeneb",
-  postDenebOldBlobs = "postDenebOldBlobs",
 }
 
 export type BlockInput =
   | {type: BlockInputType.preDeneb; block: allForks.SignedBeaconBlock}
-  | {type: BlockInputType.postDeneb; block: allForks.SignedBeaconBlock; blobs: deneb.BlobsSidecar}
-  | {type: BlockInputType.postDenebOldBlobs; block: allForks.SignedBeaconBlock};
+  | {type: BlockInputType.postDeneb; block: allForks.SignedBeaconBlock; blobs: deneb.BlobsSidecar};
 
 export function blockRequiresBlobs(config: IChainForkConfig, blockSlot: Slot, clockSlot: Slot): boolean {
   return (
@@ -44,16 +42,6 @@ export const getBlockInput = {
       blobs,
     };
   },
-
-  postDenebOldBlobs(config: IChainForkConfig, block: allForks.SignedBeaconBlock): BlockInput {
-    if (config.getForkSeq(block.message.slot) < ForkSeq.deneb) {
-      throw Error(`Pre Deneb block slot ${block.message.slot}`);
-    }
-    return {
-      type: BlockInputType.postDenebOldBlobs,
-      block,
-    };
-  },
 };
 
 export enum AttestationImportOpt {

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

@@ -128,9 +128,5 @@ function maybeValidateBlobs(
 
     case BlockInputType.preDeneb:
       return DataAvailableStatus.preDeneb;
-
-    // TODO: Ok to assume old data available?
-    case BlockInputType.postDenebOldBlobs:
-      return DataAvailableStatus.available;
   }
 }

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

@@ -10,11 +10,12 @@ import {deneb, Epoch, phase0, allForks} from "@lodestar/types";
 import {routes} from "@lodestar/api";
 import {IMetrics} from "../metrics/index.js";
 import {ChainEvent, IBeaconChain, IBeaconClock} from "../chain/index.js";
-import {BlockInput, BlockInputType, getBlockInput} from "../chain/blocks/types.js";
+import {BlockInput, BlockInputType} from "../chain/blocks/types.js";
 import {isValidBlsToExecutionChangeForBlockInclusion} from "../chain/opPools/utils.js";
 import {INetworkOptions} from "./options.js";
 import {INetwork, Libp2p} from "./interface.js";
-import {ReqRespBeaconNode, ReqRespHandlers, doBeaconBlocksMaybeBlobsByRange} from "./reqresp/index.js";
+import {ReqRespBeaconNode, ReqRespHandlers, beaconBlocksMaybeBlobsByRange} from "./reqresp/index.js";
+import {beaconBlocksMaybeBlobsByRoot} from "./reqresp/beaconBlocksMaybeBlobsByRoot.js";
 import {
   Eth2Gossipsub,
   getGossipHandlers,
@@ -328,81 +329,25 @@ export class Network implements INetwork {
           beaconBlock: blockInput.block as deneb.SignedBeaconBlock,
           blobsSidecar: blockInput.blobs,
         });
-
-      case BlockInputType.postDenebOldBlobs:
-        throw Error(`Attempting to broadcast old BlockInput slot ${blockInput.block.message.slot}`);
     }
   }
 
   async beaconBlocksMaybeBlobsByRange(
     peerId: PeerId,
     request: phase0.BeaconBlocksByRangeRequest
   ): Promise<BlockInput[]> {
-    return doBeaconBlocksMaybeBlobsByRange(this.config, this.reqResp, peerId, request, this.clock.currentEpoch);
+    return beaconBlocksMaybeBlobsByRange(this.config, this.reqResp, peerId, request, this.clock.currentEpoch);
   }
 
   async beaconBlocksMaybeBlobsByRoot(peerId: PeerId, request: phase0.BeaconBlocksByRootRequest): Promise<BlockInput[]> {
-    // Assume all requests are post Deneb
-    if (this.config.getForkSeq(this.chain.forkChoice.getFinalizedBlock().slot) >= ForkSeq.deneb) {
-      const blocksAndBlobs = await this.reqResp.beaconBlockAndBlobsSidecarByRoot(peerId, request);
-      return blocksAndBlobs.map(({beaconBlock, blobsSidecar}) =>
-        getBlockInput.postDeneb(this.config, beaconBlock, blobsSidecar)
-      );
-    }
-
-    // Assume all request are pre Deneb
-    else if (this.config.getForkSeq(this.clock.currentSlot) < ForkSeq.deneb) {
-      const blocks = await this.reqResp.beaconBlocksByRoot(peerId, request);
-      return blocks.map((block) => getBlockInput.preDeneb(this.config, block));
-    }
-
-    // NOTE: Consider blocks may be post or pre Deneb
-    // TODO Deneb: Request either blocks, or blocks+blobs
-    else {
-      const results = await Promise.all(
-        request.map(
-          async (beaconBlockRoot): Promise<BlockInput | null> => {
-            const [resultBlockBlobs, resultBlocks] = await Promise.allSettled([
-              this.reqResp.beaconBlockAndBlobsSidecarByRoot(peerId, [beaconBlockRoot]),
-              this.reqResp.beaconBlocksByRoot(peerId, [beaconBlockRoot]),
-            ]);
-
-            if (resultBlockBlobs.status === "fulfilled" && resultBlockBlobs.value.length === 1) {
-              const {beaconBlock, blobsSidecar} = resultBlockBlobs.value[0];
-              return getBlockInput.postDeneb(this.config, beaconBlock, blobsSidecar);
-            }
-
-            if (resultBlocks.status === "rejected") {
-              return Promise.reject(resultBlocks.reason);
-            }
-
-            // Promise fullfilled + no result = block not found
-            if (resultBlocks.value.length < 1) {
-              return null;
-            }
-
-            const block = resultBlocks.value[0];
-
-            if (this.config.getForkSeq(block.message.slot) >= ForkSeq.deneb) {
-              // beaconBlockAndBlobsSidecarByRoot should have succeeded
-              if (resultBlockBlobs.status === "rejected") {
-                // Recycle existing error for beaconBlockAndBlobsSidecarByRoot if any
-                return Promise.reject(resultBlockBlobs.reason);
-              } else {
-                throw Error(
-                  `Received post Deneb ${beaconBlockRoot} over beaconBlocksByRoot not beaconBlockAndBlobsSidecarByRoot`
-                );
-              }
-            }
-
-            // Block is pre Deneb
-            return getBlockInput.preDeneb(this.config, block);
-          }
-        )
-      );
-
-      return results.filter((blockOrNull): blockOrNull is BlockInput => blockOrNull !== null);
-    }
+    return beaconBlocksMaybeBlobsByRoot(
+      this.config,
+      this.reqResp,
+      peerId,
+      request,
+      this.clock.currentSlot,
+      this.chain.forkChoice.getFinalizedBlock().slot
+    );
   }
 
   /**

packages/beacon-node/src/network/reqresp/doBeaconBlocksMaybeBlobsByRange.ts → packages/beacon-node/src/network/reqresp/beaconBlocksMaybeBlobsByRange.ts

@@ -5,35 +5,49 @@ import {ForkSeq} from "@lodestar/params";
 import {computeEpochAtSlot} from "@lodestar/state-transition";
 
 import {BlockInput, getBlockInput} from "../../chain/blocks/types.js";
-import {ckzg} from "../../util/kzg.js";
+import {getEmptyBlobsSidecar} from "../../util/blobs.js";
 import {IReqRespBeaconNode} from "./interface.js";
 
-export async function doBeaconBlocksMaybeBlobsByRange(
+export async function beaconBlocksMaybeBlobsByRange(
   config: IBeaconConfig,
   reqResp: IReqRespBeaconNode,
   peerId: PeerId,
   request: phase0.BeaconBlocksByRangeRequest,
   currentEpoch: Epoch
 ): Promise<BlockInput[]> {
-  // TODO Deneb: Assumes all blocks in the same epoch
-  // TODO Deneb: Ensure all blocks are in the same epoch
-  if (config.getForkSeq(request.startSlot) < ForkSeq.deneb) {
+  // Code below assumes the request is in the same epoch
+  // Range sync satisfies this condition, but double check here for sanity
+  const {startSlot, count} = request;
+  if (count < 1) {
+    return [];
+  }
+  const endSlot = startSlot + count - 1;
+
+  const startEpoch = computeEpochAtSlot(startSlot);
+  const endEpoch = computeEpochAtSlot(endSlot);
+  if (startEpoch !== endEpoch) {
+    throw Error(
+      `BeaconBlocksByRangeRequest must be in the same epoch startEpoch=${startEpoch} != endEpoch=${endEpoch}`
+    );
+  }
+
+  // Note: Assumes all blocks in the same epoch
+  if (config.getForkSeq(startSlot) < ForkSeq.deneb) {
     const blocks = await reqResp.beaconBlocksByRange(peerId, request);
     return blocks.map((block) => getBlockInput.preDeneb(config, block));
   }
 
   // Only request blobs if they are recent enough
-  else if (computeEpochAtSlot(request.startSlot) >= currentEpoch - config.MIN_EPOCHS_FOR_BLOBS_SIDECARS_REQUESTS) {
-    // TODO Deneb: Do two requests at once for blocks and blobs
-    const blocks = await reqResp.beaconBlocksByRange(peerId, request);
-    const blobsSidecars = await reqResp.blobsSidecarsByRange(peerId, request);
+  else if (computeEpochAtSlot(startSlot) >= currentEpoch - config.MIN_EPOCHS_FOR_BLOBS_SIDECARS_REQUESTS) {
+    const [blocks, blobsSidecars] = await Promise.all([
+      reqResp.beaconBlocksByRange(peerId, request),
+      reqResp.blobsSidecarsByRange(peerId, request),
+    ]);
 
     const blockInputs: BlockInput[] = [];
     let blobSideCarIndex = 0;
     let lastMatchedSlot = -1;
 
-    const emptyKzgAggregatedProof = ckzg.computeAggregateKzgProof([]);
-
     // Match blobSideCar with the block as some blocks would have no blobs and hence
     // would be omitted from the response. If there are any inconsitencies in the
     // response, the validations during import will reject the block and hence this
@@ -56,19 +70,18 @@ export async function doBeaconBlocksMaybeBlobsByRange(
             `Missing blobsSidecar for blockSlot=${block.message.slot} with blobKzgCommitmentsLen=${blobKzgCommitmentsLen}`
           );
         }
-        blobsSidecar = {
-          beaconBlockRoot: config.getForkTypes(block.message.slot).BeaconBlock.hashTreeRoot(block.message),
-          beaconBlockSlot: block.message.slot,
-          blobs: [],
-          kzgAggregatedProof: emptyKzgAggregatedProof,
-        };
+        blobsSidecar = getEmptyBlobsSidecar(config, block as deneb.SignedBeaconBlock);
       }
       blockInputs.push(getBlockInput.postDeneb(config, block, blobsSidecar));
     }
 
     // If there are still unconsumed blobs this means that the response was inconsistent
     // and matching was wrong and hence we should throw error
-    if (blobsSidecars[blobSideCarIndex] !== undefined) {
+    if (
+      blobsSidecars[blobSideCarIndex] !== undefined &&
+      // If there are no blobs, the blobs request can give 1 block outside the requested range
+      blobsSidecars[blobSideCarIndex].beaconBlockSlot <= endSlot
+    ) {
       throw Error(
         `Unmatched blobsSidecars, blocks=${blocks.length}, blobs=${
           blobsSidecars.length
@@ -82,7 +95,6 @@ export async function doBeaconBlocksMaybeBlobsByRange(
 
   // Post Deneb but old blobs
   else {
-    const blocks = await reqResp.beaconBlocksByRange(peerId, request);
-    return blocks.map((block) => getBlockInput.postDenebOldBlobs(config, block));
+    throw Error("Cannot sync blobs outside of blobs prune window");
   }
 }

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

@@ -0,0 +1,83 @@
+import {PeerId} from "@libp2p/interface-peer-id";
+import {IBeaconConfig} from "@lodestar/config";
+import {RequestError, RequestErrorCode} from "@lodestar/reqresp";
+import {Epoch, phase0, Root, Slot} from "@lodestar/types";
+import {toHex} from "@lodestar/utils";
+import {ForkSeq} from "@lodestar/params";
+import {BlockInput, getBlockInput} from "../../chain/blocks/types.js";
+import {wrapError} from "../../util/wrapError.js";
+import {IReqRespBeaconNode} from "./interface.js";
+
+export async function beaconBlocksMaybeBlobsByRoot(
+  config: IBeaconConfig,
+  reqResp: IReqRespBeaconNode,
+  peerId: PeerId,
+  request: phase0.BeaconBlocksByRootRequest,
+  currentSlot: Epoch,
+  finalizedSlot: Slot
+): Promise<BlockInput[]> {
+  // Assume all requests are post Deneb
+  if (config.getForkSeq(finalizedSlot) >= ForkSeq.deneb) {
+    const blocksAndBlobs = await reqResp.beaconBlockAndBlobsSidecarByRoot(peerId, request);
+    return blocksAndBlobs.map(({beaconBlock, blobsSidecar}) =>
+      getBlockInput.postDeneb(config, beaconBlock, blobsSidecar)
+    );
+  }
+
+  // Assume all request are pre EIP-4844
+  else if (config.getForkSeq(currentSlot) < ForkSeq.deneb) {
+    const blocks = await reqResp.beaconBlocksByRoot(peerId, request);
+    return blocks.map((block) => getBlockInput.preDeneb(config, block));
+  }
+
+  // We don't know if a requested root is after the deneb fork or not.
+  // Thus some sort of retry is necessary while deneb is not finalized
+  else {
+    return Promise.all(
+      request.map(async (beaconBlockRoot) =>
+        beaconBlockAndBlobsSidecarByRootFallback(config, reqResp, peerId, beaconBlockRoot)
+      )
+    );
+  }
+}
+
+async function beaconBlockAndBlobsSidecarByRootFallback(
+  config: IBeaconConfig,
+  reqResp: IReqRespBeaconNode,
+  peerId: PeerId,
+  beaconBlockRoot: Root
+): Promise<BlockInput> {
+  const resBlockBlobs = await wrapError(reqResp.beaconBlockAndBlobsSidecarByRoot(peerId, [beaconBlockRoot]));
+
+  if (resBlockBlobs.err) {
+    // From the spec, if the block is from before the fork, errors with 3: ResourceUnavailable
+    // > Clients MUST support requesting blocks and sidecars since minimum_request_epoch, where
+    //   minimum_request_epoch = max(finalized_epoch, current_epoch - MIN_EPOCHS_FOR_BLOBS_SIDECARS_REQUESTS, EIP4844_FORK_EPOCH).
+    //   If any root in the request content references a block earlier than minimum_request_epoch,
+    //   peers SHOULD respond with error code 3: ResourceUnavailable.
+    // Ref: https://github.com/ethereum/consensus-specs/blob/aede132f4999ed54b98d35e27aca9451042a1ee9/specs/eip4844/p2p-interface.md#beaconblockandblobssidecarbyroot-v1
+    if (
+      resBlockBlobs.err instanceof RequestError &&
+      resBlockBlobs.err.type.code === RequestErrorCode.RESOURCE_UNAVAILABLE
+    ) {
+      // retry with blocks
+    } else {
+      // Unexpected error, throw
+      throw resBlockBlobs.err;
+    }
+  } else {
+    if (resBlockBlobs.result.length < 1) {
+      throw Error(`beaconBlockAndBlobsSidecarByRoot return empty for block root ${toHex(beaconBlockRoot)}`);
+    }
+
+    const {beaconBlock, blobsSidecar} = resBlockBlobs.result[0];
+    return getBlockInput.postDeneb(config, beaconBlock, blobsSidecar);
+  }
+
+  const resBlocks = await reqResp.beaconBlocksByRoot(peerId, [beaconBlockRoot]);
+  if (resBlocks.length < 1) {
+    throw Error(`beaconBlocksByRoot return empty for block root ${toHex(beaconBlockRoot)}`);
+  }
+
+  return getBlockInput.preDeneb(config, resBlocks[0]);
+}

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

@@ -1,3 +1,3 @@
 export * from "./ReqRespBeaconNode.js";
 export * from "./interface.js";
-export * from "./doBeaconBlocksMaybeBlobsByRange.js";
+export * from "./beaconBlocksMaybeBlobsByRange.js";

packages/beacon-node/src/util/blobs.ts

@@ -0,0 +1,24 @@
+import {IChainForkConfig} from "@lodestar/config";
+import {deneb} from "@lodestar/types";
+import {ckzg} from "./kzg.js";
+
+// Cache empty KZG proof, compute once lazily if needed
+let emptyKzgAggregatedProof: Uint8Array | null = null;
+function getEmptyKzgAggregatedProof(): Uint8Array {
+  if (!emptyKzgAggregatedProof) {
+    emptyKzgAggregatedProof = ckzg.computeAggregateKzgProof([]);
+  }
+  return emptyKzgAggregatedProof;
+}
+
+/**
+ * Construct a valid BlobsSidecar for a SignedBeaconBlock that references 0 commitments
+ */
+export function getEmptyBlobsSidecar(config: IChainForkConfig, block: deneb.SignedBeaconBlock): deneb.BlobsSidecar {
+  return {
+    beaconBlockRoot: config.getForkTypes(block.message.slot).BeaconBlock.hashTreeRoot(block.message),
+    beaconBlockSlot: block.message.slot,
+    blobs: [],
+    kzgAggregatedProof: getEmptyKzgAggregatedProof(),
+  };
+}