#37568 [BC-Insight] missing specification logic
Was this helpful?
Was this helpful?
Submitted on Dec 9th 2024 at 10:53:20 UTC by @Pig46940 for
Report ID: #37568
Report Type: Blockchain/DLT
Report severity: Insight
Target: https://github.com/chainsafe/lodestar
Impacts:
(Specifications) A bug in specifications with no direct impact on client implementations
Lacking the logic which the length of KZG commitments is less than or equal to the limitation defined in Consensus Layer
Lacking the logic of the following.
The type of the payload of this topic changes to the (modified) SignedBeaconBlock found in Deneb.
[Modified in Deneb:EIP4844]
New validation:
[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
A bug in specifications with no direct impact on client implementations
https://github.com/ethereum/consensus-specs/blob/dev/specs/deneb/p2p-interface.md#beacon_block
https://github.com/ChainSafe/lodestar/blob/dad9037e7739d5bcbccfe627e715ef40e9ba935b/packages/beacon-node/src/chain/blocks/verifyBlock.ts
https://github.com/ChainSafe/lodestar/blob/dad9037e7739d5bcbccfe627e715ef40e9ba935b/packages/beacon-node/src/chain/blocks/verifyBlocksDataAvailability.ts
To adhere to the specification, you would need an explicit check like:
if (numBlobs > MAX_BLOBS_PER_BLOCK) {
throw new BlockError(block0, {
code: BlockErrorCode.INVALID_BLOB_COUNT,
error: new Error(Block contains ${numBlobs} blobs, exceeding MAX_BLOBS_PER_BLOCK=${MAX_BLOBS_PER_BLOCK}
),
});
}
While the code references and interacts with blobKzgCommitments, it does not implement the specification's requirement to validate the blob count against MAX_BLOBS_PER_BLOCK. Adding the explicit validation step is necessary to ensure compliance.