Process recent attestations in head sync (#3805)

packages/lodestar/src/chain/blocks/importBlock.ts

@@ -33,6 +33,11 @@ import {PendingEvents} from "./utils/pendingEvents";
 import {FullyVerifiedBlock} from "./types";
 // import {ForkChoiceError, ForkChoiceErrorCode} from "@chainsafe/lodestar-fork-choice/lib/forkChoice/errors";
 
+/**
+ * Fork-choice allows to import attestations from current (0) or past (1) epoch.
+ */
+const FORK_CHOICE_ATT_EPOCH_LIMIT = 1;
+
 export type ImportBlockModules = {
   db: IBeaconDb;
   eth1: IEth1ForBlockProduction;
@@ -122,12 +127,17 @@ export async function importBlock(chain: ImportBlockModules, fullyVerifiedBlock:
   // - Register state and block to the validator monitor
   // TODO
 
+  const currentEpoch = computeEpochAtSlot(chain.forkChoice.getTime());
+  const blockEpoch = computeEpochAtSlot(block.message.slot);
+
   // - For each attestation
   //   - Get indexed attestation
   //   - Register attestation with fork-choice
   //   - Register attestation with validator monitor (only after sync)
-  // Only process attestations in response to an non-prefinalized block
-  if (!skipImportingAttestations) {
+  // Only process attestations of blocks with relevant attestations for the fork-choice:
+  // If current epoch is N, and block is epoch X, block may include attestations for epoch X or X - 1.
+  // The latest block that is useful is at epoch N - 1 which may include attestations for epoch N - 1 or N - 2.
+  if (!skipImportingAttestations && blockEpoch >= currentEpoch - FORK_CHOICE_ATT_EPOCH_LIMIT) {
     const attestations = Array.from(readonlyValues(block.message.body.attestations));
     const rootCache = new altair.RootCache(postState);
     const parentSlot = chain.forkChoice.getBlock(block.message.parentRoot)?.slot;
@@ -136,10 +146,18 @@ export async function importBlock(chain: ImportBlockModules, fullyVerifiedBlock:
     for (const attestation of attestations) {
       try {
         const indexedAttestation = postState.epochCtx.getIndexedAttestation(attestation);
-        chain.forkChoice.onAttestation(indexedAttestation);
+        const targetEpoch = attestation.data.target.epoch;
+
+        // Duplicated logic from fork-choice onAttestation validation logic.
+        // Attestations outside of this range will be dropped as Errors, so no need to import
+        if (targetEpoch <= currentEpoch && targetEpoch >= currentEpoch - FORK_CHOICE_ATT_EPOCH_LIMIT) {
+          chain.forkChoice.onAttestation(indexedAttestation);
+        }
+
         if (parentSlot !== undefined) {
           chain.metrics?.registerAttestationInBlock(indexedAttestation, parentSlot, rootCache);
         }
+
         pendingEvents.push(ChainEvent.attestation, attestation);
       } catch (e) {
         // a block has a lot of attestations and it may has same error, we don't want to log all of them

packages/lodestar/src/sync/range/chain.ts

@@ -35,7 +35,7 @@ export type SyncChainFns = {
    * Must return if ALL blocks are processed successfully
    * If SOME blocks are processed must throw BlockProcessorError()
    */
-  processChainSegment: (blocks: allForks.SignedBeaconBlock[]) => Promise<void>;
+  processChainSegment: (blocks: allForks.SignedBeaconBlock[], syncType: RangeSyncType) => Promise<void>;
   /** Must download blocks, and validate their range */
   downloadBeaconBlocksByRange: (
     peer: PeerId,
@@ -417,7 +417,7 @@ export class SyncChain {
     const blocks = batch.startProcessing();
 
     // wrapError ensures to never call both batch success() and batch error()
-    const res = await wrapError(this.processChainSegment(blocks));
+    const res = await wrapError(this.processChainSegment(blocks, this.syncType));
 
     if (!res.err) {
       batch.processingSuccess();

packages/lodestar/src/sync/range/range.ts

@@ -188,11 +188,13 @@ export class RangeSync extends (EventEmitter as {new (): RangeSyncEmitter}) {
   }
 
   /** Convenience method for `SyncChain` */
-  private processChainSegment: SyncChainFns["processChainSegment"] = async (blocks) => {
+  private processChainSegment: SyncChainFns["processChainSegment"] = async (blocks, syncType) => {
     // Not trusted, verify signatures
     const flags: PartiallyVerifiedBlockFlags = {
+      // Only skip importing attestations for finalized sync. For head sync attestation are valuable.
+      // Importing attestations also triggers a head update, see https://github.com/ChainSafe/lodestar/issues/3804
       // TODO: Review if this is okay, can we prevent some attacks by importing attestations?
-      skipImportingAttestations: true,
+      skipImportingAttestations: syncType === RangeSyncType.Finalized,
       // Ignores ALREADY_KNOWN or GENESIS_BLOCK errors, and continues with the next block in chain segment
       ignoreIfKnown: true,
       // Ignore WOULD_REVERT_FINALIZED_SLOT error, continue with the next block in chain segment