Skip to content

Commit

Permalink
fix: skip only proofs validation on individually validated gossip blo…
Browse files Browse the repository at this point in the history
…bs (#6066)

fix: skip only proofs validation on gossiped blobs
  • Loading branch information
g11tech committed Oct 25, 2023
1 parent 6985cd0 commit 629a84d
Show file tree
Hide file tree
Showing 5 changed files with 41 additions and 10 deletions.
13 changes: 12 additions & 1 deletion packages/beacon-node/src/chain/blocks/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,17 @@ export enum AttestationImportOpt {
Force,
}

export enum BlobSidecarValidation {
/** When recieved in gossip the blobs are individually verified before import */
Individual,
/**
* Blobs when recieved in req/resp can be fully verified before import
* but currently used in spec tests where blobs come without proofs and assumed
* to be valid
*/
Full,
}

export type ImportBlockOpts = {
/**
* TEMP: Review if this is safe, Lighthouse always imports attestations even in finalized sync.
Expand Down Expand Up @@ -229,7 +240,7 @@ export type ImportBlockOpts = {
*/
validSignatures?: boolean;
/** Set to true if already run `validateBlobSidecars()` sucessfully on the blobs */
validBlobSidecars?: boolean;
validBlobSidecars?: BlobSidecarValidation;
/** Seen timestamp seconds */
seenTimestampSec?: number;
/** Set to true if persist block right at verification time */
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import {toHexString} from "@lodestar/utils";
import {IClock} from "../../util/clock.js";
import {BlockError, BlockErrorCode} from "../errors/index.js";
import {validateBlobSidecars} from "../validation/blobSidecar.js";
import {BlockInput, BlockInputType, ImportBlockOpts} from "./types.js";
import {BlockInput, BlockInputType, ImportBlockOpts, BlobSidecarValidation} from "./types.js";

/**
* Verifies some early cheap sanity checks on the block before running the full state transition.
Expand Down Expand Up @@ -125,15 +125,20 @@ function maybeValidateBlobs(
): DataAvailableStatus {
switch (blockInput.type) {
case BlockInputType.postDeneb: {
if (opts.validBlobSidecars) {
if (opts.validBlobSidecars === BlobSidecarValidation.Full) {
return DataAvailableStatus.available;
}

// run full validation
const {block, blobs} = blockInput;
const blockSlot = block.message.slot;
const {blobKzgCommitments} = (block as deneb.SignedBeaconBlock).message.body;
const beaconBlockRoot = config.getForkTypes(blockSlot).BeaconBlock.hashTreeRoot(block.message);
validateBlobSidecars(blockSlot, beaconBlockRoot, blobKzgCommitments, blobs);

// if the blob siddecars have been individually verified then we can skip kzg proof check
// but other checks to match blobs with block data still need to be performed
const skipProofsCheck = opts.validBlobSidecars === BlobSidecarValidation.Individual;
validateBlobSidecars(blockSlot, beaconBlockRoot, blobKzgCommitments, blobs, {skipProofsCheck});

return DataAvailableStatus.available;
}
Expand Down
8 changes: 6 additions & 2 deletions packages/beacon-node/src/chain/validation/blobSidecar.ts
Original file line number Diff line number Diff line change
Expand Up @@ -151,7 +151,8 @@ export function validateBlobSidecars(
blockSlot: Slot,
blockRoot: Root,
expectedKzgCommitments: deneb.BlobKzgCommitments,
blobSidecars: deneb.BlobSidecars
blobSidecars: deneb.BlobSidecars,
opts: {skipProofsCheck: boolean} = {skipProofsCheck: false}
): void {
// assert len(expected_kzg_commitments) == len(blobs)
if (expectedKzgCommitments.length !== blobSidecars.length) {
Expand Down Expand Up @@ -182,7 +183,10 @@ export function validateBlobSidecars(
blobs.push(blobSidecar.blob);
proofs.push(blobSidecar.kzgProof);
}
validateBlobsAndProofs(expectedKzgCommitments, blobs, proofs);

if (!opts.skipProofsCheck) {
validateBlobsAndProofs(expectedKzgCommitments, blobs, proofs);
}
}
}

Expand Down
10 changes: 8 additions & 2 deletions packages/beacon-node/src/network/processor/gossipHandlers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,13 @@ import {PeerAction} from "../peers/index.js";
import {validateLightClientFinalityUpdate} from "../../chain/validation/lightClientFinalityUpdate.js";
import {validateLightClientOptimisticUpdate} from "../../chain/validation/lightClientOptimisticUpdate.js";
import {validateGossipBlobSidecar} from "../../chain/validation/blobSidecar.js";
import {BlockInput, BlockSource, getBlockInput, GossipedInputType} from "../../chain/blocks/types.js";
import {
BlockInput,
BlockSource,
getBlockInput,
GossipedInputType,
BlobSidecarValidation,
} from "../../chain/blocks/types.js";
import {sszDeserialize} from "../gossip/topic.js";
import {INetworkCore} from "../core/index.js";
import {INetwork} from "../interface.js";
Expand Down Expand Up @@ -227,7 +233,7 @@ function getDefaultHandlers(modules: ValidatorFnsModules, options: GossipHandler
// proposer signature already checked in validateBeaconBlock()
validProposerSignature: true,
// blobSidecars already checked in validateGossipBlobSidecars()
validBlobSidecars: true,
validBlobSidecars: BlobSidecarValidation.Individual,
// It's critical to keep a good number of mesh peers.
// To do that, the Gossip Job Wait Time should be consistently <3s to avoid the behavior penalties in gossip
// Gossip Job Wait Time depends on the BLS Job Wait Time
Expand Down
9 changes: 7 additions & 2 deletions packages/beacon-node/test/spec/presets/fork_choice.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,12 @@ import {ExecutionEngineMockBackend} from "../../../src/execution/engine/mock.js"
import {defaultChainOptions} from "../../../src/chain/options.js";
import {getStubbedBeaconDb} from "../../utils/mocks/db.js";
import {ClockStopped} from "../../utils/mocks/clock.js";
import {getBlockInput, AttestationImportOpt, BlockSource} from "../../../src/chain/blocks/types.js";
import {
getBlockInput,
AttestationImportOpt,
BlockSource,
BlobSidecarValidation,
} from "../../../src/chain/blocks/types.js";
import {ZERO_HASH_HEX} from "../../../src/constants/constants.js";
import {PowMergeBlock} from "../../../src/eth1/interface.js";
import {assertCorrectProgressiveBalances} from "../config.js";
Expand Down Expand Up @@ -216,7 +221,7 @@ const forkChoiceTest =

await chain.processBlock(blockImport, {
seenTimestampSec: tickTime,
validBlobSidecars: true,
validBlobSidecars: BlobSidecarValidation.Full,
importAttestations: AttestationImportOpt.Force,
});
if (!isValid) throw Error("Expect error since this is a negative test");
Expand Down

0 comments on commit 629a84d

Please sign in to comment.