diff --git a/packages/beacon-node/src/chain/errors/blockError.ts b/packages/beacon-node/src/chain/errors/blockError.ts index 1bfa3d7b6f9d..e4eb81dbead7 100644 --- a/packages/beacon-node/src/chain/errors/blockError.ts +++ b/packages/beacon-node/src/chain/errors/blockError.ts @@ -61,9 +61,6 @@ export enum BlockErrorCode { TRANSACTIONS_TOO_BIG = "BLOCK_ERROR_TRANSACTIONS_TOO_BIG", /** Execution engine is unavailable, syncing, or api call errored. Peers must not be downscored on this code */ EXECUTION_ENGINE_ERROR = "BLOCK_ERROR_EXECUTION_ERROR", - /** The attestation head block is too far behind the attestation slot, causing many skip slots. - This is deemed a DoS risk */ - TOO_MANY_SKIPPED_SLOTS = "TOO_MANY_SKIPPED_SLOTS", /** The blobs are unavailable */ DATA_UNAVAILABLE = "BLOCK_ERROR_DATA_UNAVAILABLE", /** Block contains too many kzg commitments */ @@ -89,7 +86,6 @@ export type BlockErrorType = | {code: BlockErrorCode.FUTURE_SLOT; blockSlot: Slot; currentSlot: Slot} | {code: BlockErrorCode.STATE_ROOT_MISMATCH} | {code: BlockErrorCode.GENESIS_BLOCK} - | {code: BlockErrorCode.TOO_MANY_SKIPPED_SLOTS; parentSlot: Slot; blockSlot: Slot} | {code: BlockErrorCode.WOULD_REVERT_FINALIZED_SLOT; blockSlot: Slot; finalizedSlot: Slot} | {code: BlockErrorCode.ALREADY_KNOWN; root: RootHex} | {code: BlockErrorCode.REPEAT_PROPOSAL; proposerIndex: ValidatorIndex} diff --git a/packages/beacon-node/src/chain/validation/block.ts b/packages/beacon-node/src/chain/validation/block.ts index 1c79e2717e21..4f4ca45ac5d4 100644 --- a/packages/beacon-node/src/chain/validation/block.ts +++ b/packages/beacon-node/src/chain/validation/block.ts @@ -15,12 +15,17 @@ import {BlockErrorCode, BlockGossipError, GossipAction} from "../errors/index.js import {IBeaconChain} from "../interface.js"; import {RegenCaller} from "../regen/index.js"; +export type GossipBlockValidationResult = { + /** Number of skipped slots between the block and its parent (blockSlot - parentSlot - 1) */ + skippedSlots: number; +}; + export async function validateGossipBlock( config: ChainForkConfig, chain: IBeaconChain, signedBlock: SignedBeaconBlock, fork: ForkName -): Promise { +): Promise { const block = signedBlock.message; const blockSlot = block.slot; const blockEpoch = computeEpochAtSlot(blockSlot); @@ -109,21 +114,6 @@ export async function validateGossipBlock( } } - // [IGNORE] The attestation head block is too far behind the attestation slot, causing many skip slots. - // This is deemed a DoS risk because we need to get the proposerShuffling. To get the shuffling we have - // to do a bunch of epoch transitions, the longer the distance between the parent and block, - // the more we have to do. epochTransitions are expensive ~750ms, so we must limit how many a - // single bad block can trigger - // Note: Ensure this check is done before calling chain.regen.getBlockSlotStat as this is the function that does various epoch transitions. - // Note: This validation check is not part of the spec. - if (chain.opts.maxSkipSlots != null && parentBlock.slot + chain.opts.maxSkipSlots < blockSlot) { - throw new BlockGossipError(GossipAction.IGNORE, { - code: BlockErrorCode.TOO_MANY_SKIPPED_SLOTS, - parentSlot: parentBlock.slot, - blockSlot, - }); - } - // [REJECT] The block is from a higher slot than its parent. if (parentBlock.slot >= blockSlot) { throw new BlockGossipError(GossipAction.REJECT, { @@ -133,6 +123,10 @@ export async function validateGossipBlock( }); } + // Number of skipped slots between block and parent (non-spec). Previously this gated blocks via + // maxSkipSlots; now the caller only observes it so legitimate post-skip blocks are no longer ignored. + const skippedSlots = blockSlot - parentBlock.slot - 1; + // [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 (isForkPostDeneb(fork) && !isForkPostGloas(fork)) { const blobKzgCommitmentsLen = (block as deneb.BeaconBlock).body.blobKzgCommitments.length; @@ -247,4 +241,6 @@ export async function validateGossipBlock( } chain.seenBlockProposers.add(blockSlot, proposerIndex); + + return {skippedSlots}; } diff --git a/packages/beacon-node/src/metrics/metrics/lodestar.ts b/packages/beacon-node/src/metrics/metrics/lodestar.ts index 6b082b7ae09b..ab5e246f1bf5 100644 --- a/packages/beacon-node/src/metrics/metrics/lodestar.ts +++ b/packages/beacon-node/src/metrics/metrics/lodestar.ts @@ -861,6 +861,12 @@ export function createLodestarMetrics( labelNames: ["numBlobs"], }), + skippedSlots: register.histogram({ + name: "lodestar_gossip_block_skipped_slots", + help: "Number of skipped slots between a gossip block and its parent (blockSlot - parentSlot - 1)", + buckets: [0, 1, 2, 4, 8, 16, 32], + }), + processBlockErrors: register.gauge<{error: BlockErrorCode | "NOT_BLOCK_ERROR"}>({ name: "lodestar_gossip_block_process_block_errors", help: "Count of errors, by error type, while processing blocks", diff --git a/packages/beacon-node/src/network/processor/gossipHandlers.ts b/packages/beacon-node/src/network/processor/gossipHandlers.ts index 722d3fd5db6e..040af011d81d 100644 --- a/packages/beacon-node/src/network/processor/gossipHandlers.ts +++ b/packages/beacon-node/src/network/processor/gossipHandlers.ts @@ -185,7 +185,7 @@ function getSequentialHandlers(modules: ValidatorFnsModules, options: GossipHand peerIdStr, }); try { - await validateGossipBlock(config, chain, signedBlock, fork); + const {skippedSlots} = await validateGossipBlock(config, chain, signedBlock, fork); if (isForkPostGloas(fork)) { chain.seenPayloadEnvelopeInputCache.add({ @@ -205,8 +205,15 @@ function getSequentialHandlers(modules: ValidatorFnsModules, options: GossipHand metrics?.gossipBlock.gossipValidation.recvToValidation.observe(recvToValidation); metrics?.gossipBlock.gossipValidation.validationTime.observe(validationTime); + metrics?.gossipBlock.skippedSlots.observe(skippedSlots); - logger.debug("Validated gossip block", {...blockInputMeta, ...logCtx, recvToValidation, validationTime}); + logger.debug("Validated gossip block", { + ...blockInputMeta, + ...logCtx, + recvToValidation, + validationTime, + skippedSlots, + }); chain.emitter.emit(routes.events.EventType.blockGossip, {slot, block: blockRootHex}); diff --git a/packages/beacon-node/test/unit/chain/validation/block.test.ts b/packages/beacon-node/test/unit/chain/validation/block.test.ts index c87012467acf..b42b78f7aaec 100644 --- a/packages/beacon-node/test/unit/chain/validation/block.test.ts +++ b/packages/beacon-node/test/unit/chain/validation/block.test.ts @@ -25,7 +25,6 @@ describe("gossip block validation", () => { const block = ssz.deneb.BeaconBlock.defaultValue(); block.slot = clockSlot; const signature = EMPTY_SIGNATURE; - const maxSkipSlots = 10; const denebConfig = createChainForkConfig({ ...configDef, ALTAIR_FORK_EPOCH: 0, @@ -43,7 +42,7 @@ describe("gossip block validation", () => { chain.forkChoice = forkChoice; regen = chain.regen; - (chain as any).opts = {maxSkipSlots}; + (chain as any).opts = {}; verifySignature = chain.bls.verifySignatureSets; verifySignature.mockResolvedValue(true); @@ -119,18 +118,6 @@ describe("gossip block validation", () => { ); }); - it("TOO_MANY_SKIPPED_SLOTS", async () => { - // Return not known for proposed block - forkChoice.getBlockHexDefaultStatus.mockReturnValueOnce(null); - // Return parent block with 1 slot way back than maxSkipSlots - forkChoice.getBlockHexDefaultStatus.mockReturnValueOnce({slot: block.slot - (maxSkipSlots + 1)} as ProtoBlock); - - await expectRejectedWithLodestarError( - validateGossipBlock(config, chain, job, ForkName.phase0), - BlockErrorCode.TOO_MANY_SKIPPED_SLOTS - ); - }); - it("NOT_LATER_THAN_PARENT", async () => { // Return not known for proposed block forkChoice.getBlockHexDefaultStatus.mockReturnValueOnce(null);