From 7af22d77957214a19481be18011f51c349e585d7 Mon Sep 17 00:00:00 2001 From: tuyennhv Date: Thu, 20 Apr 2023 21:06:41 +0700 Subject: [PATCH] Add TOO_MANY_SKIPPED_SLOTS attestation error (#5390) --- .../src/chain/blocks/importBlock.ts | 12 +++++------ .../src/chain/errors/attestationError.ts | 5 ++++- packages/beacon-node/src/chain/options.ts | 3 +++ .../src/chain/validation/aggregateAndProof.ts | 11 +++++++++- .../src/chain/validation/attestation.ts | 19 ++++++++++++++++-- .../src/metrics/metrics/lodestar.ts | 5 +++++ .../src/network/processor/gossipHandlers.ts | 20 ++++++++++++++----- .../test/utils/validationData/attestation.ts | 2 ++ 8 files changed, 62 insertions(+), 15 deletions(-) diff --git a/packages/beacon-node/src/chain/blocks/importBlock.ts b/packages/beacon-node/src/chain/blocks/importBlock.ts index ec9f0d68c3cd..4657361240e7 100644 --- a/packages/beacon-node/src/chain/blocks/importBlock.ts +++ b/packages/beacon-node/src/chain/blocks/importBlock.ts @@ -89,7 +89,12 @@ export async function importBlock( this.clock.currentSlot, executionStatus ); - this.logger.verbose("Added block to forkchoice", {slot: block.message.slot, root: blockRootHex}); + + // This adds the state necessary to process the next block + // Some block event handlers require state being in state cache so need to do this before emitting EventType.block + this.stateCache.add(postState); + + this.logger.verbose("Added block to forkchoice and state cache", {slot: block.message.slot, root: blockRootHex}); this.emitter.emit(routes.events.EventType.block, { block: toHexString(this.config.getForkTypes(block.message.slot).BeaconBlock.hashTreeRoot(block.message)), slot: block.message.slot, @@ -325,11 +330,6 @@ export async function importBlock( } } - // 7. Add post state to stateCache - // - // This adds the state necessary to process the next block - this.stateCache.add(postState); - if (!isStateValidatorsNodesPopulated(postState)) { this.logger.verbose("After importBlock caching postState without SSZ cache", {slot: postState.slot}); } diff --git a/packages/beacon-node/src/chain/errors/attestationError.ts b/packages/beacon-node/src/chain/errors/attestationError.ts index bb907eda5834..7a98a62109c6 100644 --- a/packages/beacon-node/src/chain/errors/attestationError.ts +++ b/packages/beacon-node/src/chain/errors/attestationError.ts @@ -130,6 +130,8 @@ export enum AttestationErrorCode { * Invalid ssz bytes. */ INVALID_SERIALIZED_BYTES = "ATTESTATION_ERROR_INVALID_SERIALIZED_BYTES", + /** Too many skipped slots. */ + TOO_MANY_SKIPPED_SLOTS = "ATTESTATION_ERROR_TOO_MANY_SKIPPED_SLOTS", } export type AttestationErrorType = @@ -163,7 +165,8 @@ export type AttestationErrorType = | {code: AttestationErrorCode.MISSING_ATTESTATION_HEAD_STATE; error: Error} | {code: AttestationErrorCode.INVALID_AGGREGATOR} | {code: AttestationErrorCode.INVALID_INDEXED_ATTESTATION} - | {code: AttestationErrorCode.INVALID_SERIALIZED_BYTES}; + | {code: AttestationErrorCode.INVALID_SERIALIZED_BYTES} + | {code: AttestationErrorCode.TOO_MANY_SKIPPED_SLOTS; headBlockSlot: Slot; attestationSlot: Slot}; export class AttestationError extends GossipActionError { getMetadata(): Record { diff --git a/packages/beacon-node/src/chain/options.ts b/packages/beacon-node/src/chain/options.ts index a4763a26f36c..37e152f99e07 100644 --- a/packages/beacon-node/src/chain/options.ts +++ b/packages/beacon-node/src/chain/options.ts @@ -74,4 +74,7 @@ export const defaultChainOptions: IChainOptions = { assertCorrectProgressiveBalances: false, archiveStateEpochFrequency: 1024, emitPayloadAttributes: false, + // for gossip block validation, it's unlikely we see a reorg with 32 slots + // for attestation validation, having this value ensures we don't have to regen states most of the time + maxSkipSlots: 32, }; diff --git a/packages/beacon-node/src/chain/validation/aggregateAndProof.ts b/packages/beacon-node/src/chain/validation/aggregateAndProof.ts index 44a2e500e31f..79d8dc878a46 100644 --- a/packages/beacon-node/src/chain/validation/aggregateAndProof.ts +++ b/packages/beacon-node/src/chain/validation/aggregateAndProof.ts @@ -88,7 +88,16 @@ export async function validateGossipAggregateAndProof( // [IGNORE] The block being voted for (attestation.data.beacon_block_root) has been seen (via both gossip // and non-gossip sources) (a client MAY queue attestations for processing once block is retrieved). - const attHeadBlock = verifyHeadBlockAndTargetRoot(chain, attData.beaconBlockRoot, attTarget.root, attEpoch); + // Lighthouse doesn't check maxSkipSlots option here but Lodestar wants to be more strict + // to be more DOS protection + const attHeadBlock = verifyHeadBlockAndTargetRoot( + chain, + attData.beaconBlockRoot, + attTarget.root, + attSlot, + attEpoch, + chain.opts.maxSkipSlots + ); // [IGNORE] The current finalized_checkpoint is an ancestor of the block defined by aggregate.data.beacon_block_root // -- i.e. get_ancestor(store, aggregate.data.beacon_block_root, compute_start_slot_at_epoch(store.finalized_checkpoint.epoch)) == store.finalized_checkpoint.root diff --git a/packages/beacon-node/src/chain/validation/attestation.ts b/packages/beacon-node/src/chain/validation/attestation.ts index fce591d29f8f..ab88616375c8 100644 --- a/packages/beacon-node/src/chain/validation/attestation.ts +++ b/packages/beacon-node/src/chain/validation/attestation.ts @@ -144,7 +144,9 @@ export async function validateGossipAttestation( chain, attestationOrCache.attestation.data.beaconBlockRoot, attestationOrCache.attestation.data.target.root, - attEpoch + attSlot, + attEpoch, + chain.opts.maxSkipSlots ); // [REJECT] The block being voted for (attestation.data.beacon_block_root) passes validation. @@ -334,9 +336,22 @@ export function verifyHeadBlockAndTargetRoot( chain: IBeaconChain, beaconBlockRoot: Root, targetRoot: Root, - attestationEpoch: Epoch + attestationSlot: Slot, + attestationEpoch: Epoch, + maxSkipSlots?: number ): ProtoBlock { const headBlock = verifyHeadBlockIsKnown(chain, beaconBlockRoot); + // Lighthouse rejects the attestation, however Lodestar only ignores considering it's not against the spec + // it's more about a DOS protection to us + // With verifyPropagationSlotRange() and maxSkipSlots = 32, it's unlikely we have to regenerate states in queue + // to validate beacon_attestation and aggregate_and_proof + if (maxSkipSlots !== undefined && attestationSlot - headBlock.slot > maxSkipSlots) { + throw new AttestationError(GossipAction.IGNORE, { + code: AttestationErrorCode.TOO_MANY_SKIPPED_SLOTS, + attestationSlot, + headBlockSlot: headBlock.slot, + }); + } verifyAttestationTargetRoot(headBlock, targetRoot, attestationEpoch); return headBlock; } diff --git a/packages/beacon-node/src/metrics/metrics/lodestar.ts b/packages/beacon-node/src/metrics/metrics/lodestar.ts index 28274aa3b583..48ab83cfdefc 100644 --- a/packages/beacon-node/src/metrics/metrics/lodestar.ts +++ b/packages/beacon-node/src/metrics/metrics/lodestar.ts @@ -276,6 +276,11 @@ export function createLodestarMetrics( help: "Current count of jobs being run on network processor for topic", labelNames: ["topic"], }), + gossipValidationErrorTooManySkippedSlots: register.gauge<"topic">({ + name: "lodestar_gossip_validation_error_too_many_skipped_slots_total", + help: "Count of total gossip validation errors due to too many skipped slots", + labelNames: ["topic"], + }), networkProcessor: { executeWorkCalls: register.gauge({ diff --git a/packages/beacon-node/src/network/processor/gossipHandlers.ts b/packages/beacon-node/src/network/processor/gossipHandlers.ts index 998401b6a106..93f2f8915215 100644 --- a/packages/beacon-node/src/network/processor/gossipHandlers.ts +++ b/packages/beacon-node/src/network/processor/gossipHandlers.ts @@ -203,8 +203,13 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH try { validationResult = await validateGossipAggregateAndProof(chain, signedAggregateAndProof, false, serializedData); } catch (e) { - if (e instanceof AttestationError && e.action === GossipAction.REJECT) { - chain.persistInvalidSszValue(ssz.phase0.SignedAggregateAndProof, signedAggregateAndProof, "gossip_reject"); + if (e instanceof AttestationError) { + if (e.action === GossipAction.REJECT) { + chain.persistInvalidSszValue(ssz.phase0.SignedAggregateAndProof, signedAggregateAndProof, "gossip_reject"); + } + if (e.type.code === AttestationErrorCode.TOO_MANY_SKIPPED_SLOTS) { + metrics?.gossipValidationErrorTooManySkippedSlots.inc({topic: topic.type}); + } } throw e; } @@ -234,7 +239,7 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH } }, - [GossipType.beacon_attestation]: async ({serializedData, msgSlot}, {subnet}, _peer, seenTimestampSec) => { + [GossipType.beacon_attestation]: async ({serializedData, msgSlot}, {type, subnet}, _peer, seenTimestampSec) => { if (msgSlot === undefined) { throw Error("msgSlot is undefined for beacon_attestation topic"); } @@ -247,8 +252,13 @@ export function getGossipHandlers(modules: ValidatorFnsModules, options: GossipH subnet ); } catch (e) { - if (e instanceof AttestationError && e.action === GossipAction.REJECT) { - chain.persistInvalidSszBytes(ssz.phase0.Attestation.typeName, serializedData, "gossip_reject"); + if (e instanceof AttestationError) { + if (e.action === GossipAction.REJECT) { + chain.persistInvalidSszBytes(ssz.phase0.Attestation.typeName, serializedData, "gossip_reject"); + } + if (e.type.code === AttestationErrorCode.TOO_MANY_SKIPPED_SLOTS) { + metrics?.gossipValidationErrorTooManySkippedSlots.inc({topic: type}); + } } throw e; } diff --git a/packages/beacon-node/test/utils/validationData/attestation.ts b/packages/beacon-node/test/utils/validationData/attestation.ts index dc4b0590227c..f27d3c9c020c 100644 --- a/packages/beacon-node/test/utils/validationData/attestation.ts +++ b/packages/beacon-node/test/utils/validationData/attestation.ts @@ -18,6 +18,7 @@ import {signCached} from "../cache.js"; import {ClockStatic} from "../clock.js"; import {SeenAggregatedAttestations} from "../../../src/chain/seenCache/seenAggregateAndProof.js"; import {SeenAttestationDatas} from "../../../src/chain/seenCache/seenAttestationData.js"; +import {defaultChainOptions} from "../../../src/chain/options.js"; export type AttestationValidDataOpts = { currentSlot?: Slot; @@ -125,6 +126,7 @@ export function getAttestationValidData(opts: AttestationValidDataOpts): { bls: new BlsSingleThreadVerifier({metrics: null}), waitForBlock: () => Promise.resolve(false), index2pubkey: state.epochCtx.index2pubkey, + opts: defaultChainOptions, } as Partial as IBeaconChain; return {chain, attestation, subnet, validatorIndex};