Add TOO_MANY_SKIPPED_SLOTS attestation error (#5390)

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

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<AttestationErrorType> {
   getMetadata(): Record<string, string | number | null> {

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,
 };

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

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

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({

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

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<IBeaconChain> as IBeaconChain;
 
   return {chain, attestation, subnet, validatorIndex};