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

…7302)
ChainSafe · wemeetagain · Dec 20, 2024 · Oct 30, 2024 · Nov 1, 2024 · Nov 1, 2024
commit 7cd296b0466f75136cbf544aaec5ce000c89a8ed
@@ -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> {}
 

@@ -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.

@@ -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);
   });
 });