feat: add kzg commitment length check when validating gossip blocks (#7302)

Author: ensi321

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

@@ -64,6 +64,8 @@ export enum BlockErrorCode {
   TOO_MANY_SKIPPED_SLOTS = "TOO_MANY_SKIPPED_SLOTS",
   /** The blobs are unavailable */
   DATA_UNAVAILABLE = "BLOCK_ERROR_DATA_UNAVAILABLE",
+  /** Block contains too many kzg commitments */
+  TOO_MANY_KZG_COMMITMENTS = "BLOCK_ERROR_TOO_MANY_KZG_COMMITMENTS",
 }
 
 type ExecutionErrorStatus = Exclude<
@@ -105,7 +107,8 @@ export type BlockErrorType =
   | {code: BlockErrorCode.SAME_PARENT_HASH; blockHash: RootHex}
   | {code: BlockErrorCode.TRANSACTIONS_TOO_BIG; size: number; max: number}
   | {code: BlockErrorCode.EXECUTION_ENGINE_ERROR; execStatus: ExecutionErrorStatus; errorMessage: string}
-  | {code: BlockErrorCode.DATA_UNAVAILABLE};
+  | {code: BlockErrorCode.DATA_UNAVAILABLE}
+  | {code: BlockErrorCode.TOO_MANY_KZG_COMMITMENTS; blobKzgCommitmentsLen: number; commitmentLimit: number};
 
 export class BlockGossipError extends GossipActionError<BlockErrorType> {}
 

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

@@ -1,5 +1,5 @@
 import {ChainForkConfig} from "@lodestar/config";
-import {ForkName} from "@lodestar/params";
+import {ForkName, isForkBlobs} from "@lodestar/params";
 import {
   computeStartSlotAtEpoch,
   computeTimeAtSlot,
@@ -8,7 +8,7 @@ import {
   isExecutionEnabled,
   isExecutionStateType,
 } from "@lodestar/state-transition";
-import {SignedBeaconBlock} from "@lodestar/types";
+import {SignedBeaconBlock, deneb} from "@lodestar/types";
 import {sleep, toRootHex} from "@lodestar/utils";
 import {MAXIMUM_GOSSIP_CLOCK_DISPARITY} from "../../constants/index.js";
 import {BlockErrorCode, BlockGossipError, GossipAction} from "../errors/index.js";
@@ -110,6 +110,18 @@ export async function validateGossipBlock(
     });
   }
 
+  // [REJECT] The length of KZG commitments is less than or equal to the limitation defined in Consensus Layer -- i.e. validate that len(body.signed_beacon_block.message.blob_kzg_commitments) <= MAX_BLOBS_PER_BLOCK
+  if (isForkBlobs(fork)) {
+    const blobKzgCommitmentsLen = (block as deneb.BeaconBlock).body.blobKzgCommitments.length;
+    if (blobKzgCommitmentsLen > chain.config.MAX_BLOBS_PER_BLOCK) {
+      throw new BlockGossipError(GossipAction.REJECT, {
+        code: BlockErrorCode.TOO_MANY_KZG_COMMITMENTS,
+        blobKzgCommitmentsLen,
+        commitmentLimit: chain.config.MAX_BLOBS_PER_BLOCK,
+      });
+    }
+  }
+
   // use getPreState to reload state if needed. It also checks for whether the current finalized checkpoint is an ancestor of the block.
   // As a result, we throw an IGNORE (whereas the spec says we should REJECT for this scenario).
   // this is something we should change this in the future to make the code airtight to the spec.

packages/beacon-node/test/unit/chain/validation/block.test.ts

@@ -1,6 +1,6 @@
 import {config} from "@lodestar/config/default";
 import {ProtoBlock} from "@lodestar/fork-choice";
-import {ForkName} from "@lodestar/params";
+import {ForkBlobs, ForkName} from "@lodestar/params";
 import {SignedBeaconBlock, ssz} from "@lodestar/types";
 import {Mock, Mocked, beforeEach, describe, it, vi} from "vitest";
 import {BlockErrorCode} from "../../../../src/chain/errors/index.js";
@@ -20,12 +20,15 @@ describe("gossip block validation", () => {
   let job: SignedBeaconBlock;
   const proposerIndex = 0;
   const clockSlot = 32;
-  const block = ssz.phase0.BeaconBlock.defaultValue();
+  const block = ssz.deneb.BeaconBlock.defaultValue();
   block.slot = clockSlot;
   const signature = EMPTY_SIGNATURE;
   const maxSkipSlots = 10;
 
   beforeEach(() => {
+    // Fill up with kzg commitments
+    block.body.blobKzgCommitments = Array.from({length: config.MAX_BLOBS_PER_BLOCK}, () => new Uint8Array([0]));
+
     chain = getMockedBeaconChain();
     vi.spyOn(chain.clock, "currentSlotWithGossipDisparity", "get").mockReturnValue(clockSlot);
     forkChoice = chain.forkChoice;
@@ -184,9 +187,47 @@ describe("gossip block validation", () => {
     regen.getPreState.mockResolvedValue(state);
     // BLS signature verifier returns valid
     verifySignature.mockResolvedValue(true);
-    // Force proposer shuffling cache to return wrong value
+    // Force proposer shuffling cache to return correct value
     vi.spyOn(state.epochCtx, "getBeaconProposer").mockReturnValue(proposerIndex);
 
     await validateGossipBlock(config, chain, job, ForkName.phase0);
   });
+
+  it("deneb - TOO_MANY_KZG_COMMITMENTS", async () => {
+    // Return not known for proposed block
+    forkChoice.getBlockHex.mockReturnValueOnce(null);
+    // Returned parent block is latter than proposed block
+    forkChoice.getBlockHex.mockReturnValueOnce({slot: clockSlot - 1} as ProtoBlock);
+    // Regen returns some state
+    const state = generateCachedState();
+    regen.getPreState.mockResolvedValue(state);
+    // BLS signature verifier returns valid
+    verifySignature.mockResolvedValue(true);
+    // Force proposer shuffling cache to return correct value
+    vi.spyOn(state.epochCtx, "getBeaconProposer").mockReturnValue(proposerIndex + 1);
+    // Add one extra kzg commitment in the block so it goes over the limit
+    (job as SignedBeaconBlock<ForkBlobs>).message.body.blobKzgCommitments.push(new Uint8Array([0]));
+
+    await expectRejectedWithLodestarError(
+      validateGossipBlock(config, chain, job, ForkName.deneb),
+      BlockErrorCode.TOO_MANY_KZG_COMMITMENTS
+    );
+  });
+
+  it("deneb - valid", async () => {
+    // Return not known for proposed block
+    forkChoice.getBlockHex.mockReturnValueOnce(null);
+    // Returned parent block is latter than proposed block
+    forkChoice.getBlockHex.mockReturnValueOnce({slot: clockSlot - 1} as ProtoBlock);
+    // Regen returns some state
+    const state = generateCachedState();
+    regen.getPreState.mockResolvedValue(state);
+    // BLS signature verifier returns valid
+    verifySignature.mockResolvedValue(true);
+    // Force proposer shuffling cache to return correct value
+    vi.spyOn(state.epochCtx, "getBeaconProposer").mockReturnValue(proposerIndex);
+    // Keep number of kzg commitments as is so it stays within the limit
+
+    await validateGossipBlock(config, chain, job, ForkName.deneb);
+  });
 });