Ignore attestations voting for the wrong finalized checkpoint (#3916)

* Ignore attestations voting for the wrong finalized checkpoint * Fix lint
ChainSafe · Apr 13, 2022 · 053f76a · 053f76a
1 parent 283ba51
commit 053f76a
Show file tree

Hide file tree

Showing 6 changed files with 17 additions and 11 deletions.
diff --git a/packages/lodestar/src/chain/errors/attestationError.ts b/packages/lodestar/src/chain/errors/attestationError.ts
@@ -42,9 +42,9 @@ export enum AttestationErrorCode {
    */
   AGGREGATOR_INDEX_TOO_HIGH = "ATTESTATION_ERROR_AGGREGATOR_INDEX_TOO_HIGH",
   /**
-   * The `attestation.data.beacon_block_root` block is unknown.
+   * The `attestation.data.beacon_block_root` block is unknown or prefinalized.
    */
-  UNKNOWN_BEACON_BLOCK_ROOT = "ATTESTATION_ERROR_UNKNOWN_BEACON_BLOCK_ROOT",
+  UNKNOWN_OR_PREFINALIZED_BEACON_BLOCK_ROOT = "ATTESTATION_ERROR_UNKNOWN_OR_PREFINALIZED_BEACON_BLOCK_ROOT",
   /**
    * The `attestation.data.slot` is not from the same epoch as `data.target.epoch`.
    */
@@ -134,7 +134,7 @@ export type AttestationErrorType =
   | {code: AttestationErrorCode.ATTESTATION_ALREADY_KNOWN; targetEpoch: Epoch; validatorIndex: number}
   | {code: AttestationErrorCode.AGGREGATOR_ALREADY_KNOWN; targetEpoch: Epoch; aggregatorIndex: number}
   | {code: AttestationErrorCode.AGGREGATOR_INDEX_TOO_HIGH; aggregatorIndex: ValidatorIndex}
-  | {code: AttestationErrorCode.UNKNOWN_BEACON_BLOCK_ROOT; root: RootHex}
+  | {code: AttestationErrorCode.UNKNOWN_OR_PREFINALIZED_BEACON_BLOCK_ROOT; root: RootHex}
   | {code: AttestationErrorCode.BAD_TARGET_EPOCH}
   | {code: AttestationErrorCode.HEAD_NOT_TARGET_DESCENDANT}
   | {code: AttestationErrorCode.UNKNOWN_TARGET_ROOT; root: Uint8Array}

diff --git a/packages/lodestar/src/chain/validation/aggregateAndProof.ts b/packages/lodestar/src/chain/validation/aggregateAndProof.ts
@@ -55,7 +55,7 @@ export async function validateGossipAggregateAndProof(
   // and non-gossip sources) (a client MAY queue attestations for processing once block is retrieved).
   const attHeadBlock = verifyHeadBlockAndTargetRoot(chain, attData.beaconBlockRoot, attTarget.root, attEpoch);
 
-  // [REJECT] The current finalized_checkpoint is an ancestor of the block defined by aggregate.data.beacon_block_root
+  // [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
   // > Altready check in `chain.forkChoice.hasBlock(attestation.data.beaconBlockRoot)`
 

diff --git a/packages/lodestar/src/chain/validation/attestation.ts b/packages/lodestar/src/chain/validation/attestation.ts
@@ -71,7 +71,7 @@ export async function validateGossipAttestation(
   // [REJECT] The block being voted for (attestation.data.beacon_block_root) passes validation.
   // > Altready check in `verifyHeadBlockAndTargetRoot()`
 
-  // [REJECT] The current finalized_checkpoint is an ancestor of the block defined by attestation.data.beacon_block_root
+  // [IGNORE] The current finalized_checkpoint is an ancestor of the block defined by attestation.data.beacon_block_root
   // -- i.e. get_ancestor(store, attestation.data.beacon_block_root, compute_start_slot_at_epoch(store.finalized_checkpoint.epoch)) == store.finalized_checkpoint.root
   // > Altready check in `verifyHeadBlockAndTargetRoot()`
 
@@ -217,7 +217,7 @@ export function verifyHeadBlockAndTargetRoot(
  * 2. The block is prior to the latest finalized block.
  *
  * Case (1) is the exact thing we're trying to detect. However case (2) is a little different, but
- * it's still fine to reject here because there's no need for us to handle attestations that are
+ * it's still fine to ignore here because there's no need for us to handle attestations that are
  * already finalized.
  */
 function verifyHeadBlockIsKnown(chain: IBeaconChain, beaconBlockRoot: Root): IProtoBlock {
@@ -226,7 +226,7 @@ function verifyHeadBlockIsKnown(chain: IBeaconChain, beaconBlockRoot: Root): IPr
   const headBlock = chain.forkChoice.getBlock(beaconBlockRoot);
   if (headBlock === null) {
     throw new AttestationError(GossipAction.IGNORE, {
-      code: AttestationErrorCode.UNKNOWN_BEACON_BLOCK_ROOT,
+      code: AttestationErrorCode.UNKNOWN_OR_PREFINALIZED_BEACON_BLOCK_ROOT,
       root: toHexString(beaconBlockRoot as typeof beaconBlockRoot),
     });
   }

diff --git a/packages/lodestar/src/network/gossip/handlers/index.ts b/packages/lodestar/src/network/gossip/handlers/index.ts
@@ -328,7 +328,10 @@ async function validateGossipAggregateAndProofRetryUnknownRoot(
     try {
       return await validateGossipAggregateAndProof(chain, signedAggregateAndProof);
     } catch (e) {
-      if (e instanceof AttestationError && e.type.code === AttestationErrorCode.UNKNOWN_BEACON_BLOCK_ROOT) {
+      if (
+        e instanceof AttestationError &&
+        e.type.code === AttestationErrorCode.UNKNOWN_OR_PREFINALIZED_BEACON_BLOCK_ROOT
+      ) {
         if (unknownBlockRootRetries++ < MAX_UNKNOWN_BLOCK_ROOT_RETRIES) {
           // Trigger unknown block root search here
 
@@ -367,7 +370,10 @@ async function validateGossipAttestationRetryUnknownRoot(
     try {
       return await validateGossipAttestation(chain, attestation, subnet);
     } catch (e) {
-      if (e instanceof AttestationError && e.type.code === AttestationErrorCode.UNKNOWN_BEACON_BLOCK_ROOT) {
+      if (
+        e instanceof AttestationError &&
+        e.type.code === AttestationErrorCode.UNKNOWN_OR_PREFINALIZED_BEACON_BLOCK_ROOT
+      ) {
         if (unknownBlockRootRetries++ < MAX_UNKNOWN_BLOCK_ROOT_RETRIES) {
           // Trigger unknown block root search here
 

diff --git a/packages/lodestar/test/unit/chain/validation/aggregateAndProof.test.ts b/packages/lodestar/test/unit/chain/validation/aggregateAndProof.test.ts
@@ -80,7 +80,7 @@ describe("chain / validation / aggregateAndProof", () => {
     // Set beaconBlockRoot to a root not known by the fork choice
     signedAggregateAndProof.message.aggregate.data.beaconBlockRoot = UNKNOWN_ROOT;
 
-    await expectError(chain, signedAggregateAndProof, AttestationErrorCode.UNKNOWN_BEACON_BLOCK_ROOT);
+    await expectError(chain, signedAggregateAndProof, AttestationErrorCode.UNKNOWN_OR_PREFINALIZED_BEACON_BLOCK_ROOT);
   });
 
   it("INVALID_TARGET_ROOT", async () => {

diff --git a/packages/lodestar/test/unit/chain/validation/attestation.test.ts b/packages/lodestar/test/unit/chain/validation/attestation.test.ts
@@ -85,7 +85,7 @@ describe("chain / validation / attestation", () => {
     // Set beaconBlockRoot to a root not known by the fork choice
     attestation.data.beaconBlockRoot = UNKNOWN_ROOT;
 
-    await expectError(chain, attestation, subnet, AttestationErrorCode.UNKNOWN_BEACON_BLOCK_ROOT);
+    await expectError(chain, attestation, subnet, AttestationErrorCode.UNKNOWN_OR_PREFINALIZED_BEACON_BLOCK_ROOT);
   });
 
   it("INVALID_TARGET_ROOT", async () => {