From 1829250ee4df6c4e69687ccdcc69ab79d968e986 Mon Sep 17 00:00:00 2001
From: Michael Sproul <michael@sigmaprime.io>
Date: Fri, 4 Mar 2022 00:41:22 +0000
Subject: [PATCH] Ignore attestations to finalized blocks (don't reject)
 (#3052)

## Issue Addressed

Addresses spec changes from v1.1.0:

- https://github.com/ethereum/consensus-specs/pull/2830
- https://github.com/ethereum/consensus-specs/pull/2846

## Proposed Changes

* Downgrade the REJECT for `HeadBlockFinalized` to an IGNORE. This applies to both unaggregated and aggregated attestations.

## Additional Info

I thought about also changing the penalty for `UnknownTargetRoot` but I don't think it's reachable in practice.
---
 .../beacon_chain/src/attestation_verification.rs       |  1 -
 .../src/beacon_processor/worker/gossip_methods.rs      | 10 ++++------
 2 files changed, 4 insertions(+), 7 deletions(-)

diff --git a/beacon_node/beacon_chain/src/attestation_verification.rs b/beacon_node/beacon_chain/src/attestation_verification.rs
index 059b3c4d2..406c0049a 100644
--- a/beacon_node/beacon_chain/src/attestation_verification.rs
+++ b/beacon_node/beacon_chain/src/attestation_verification.rs
@@ -961,7 +961,6 @@ impl<'a, T: BeaconChainTypes> VerifiedUnaggregatedAttestation<'a, T> {
 }
 
 /// Returns `Ok(())` if the `attestation.data.beacon_block_root` is known to this chain.
-/// You can use this `shuffling_id` to read from the shuffling cache.
 ///
 /// The block root may not be known for two reasons:
 ///
diff --git a/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs b/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs
index bb85d063e..b367f7f6d 100644
--- a/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs
+++ b/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs
@@ -1557,7 +1557,7 @@ impl<T: BeaconChainTypes> Worker<T> {
                 /*
                  * The block indicated by the target root is not known to us.
                  *
-                 * We should always get `AttnError::UnknwonHeadBlock` before we get this
+                 * We should always get `AttnError::UnknownHeadBlock` before we get this
                  * error, so this means we can get this error if:
                  *
                  * 1. The target root does not represent a valid block.
@@ -1566,7 +1566,7 @@ impl<T: BeaconChainTypes> Worker<T> {
                  * For (2), we should only be processing attestations when we should have
                  * all the available information. Note: if we do a weak-subjectivity sync
                  * it's possible that this situation could occur, but I think it's
-                 * unlikely. For now, we will declare this to be an invalid message>
+                 * unlikely. For now, we will declare this to be an invalid message.
                  *
                  * The peer has published an invalid consensus message.
                  */
@@ -1713,14 +1713,12 @@ impl<T: BeaconChainTypes> Worker<T> {
             AttnError::HeadBlockFinalized { beacon_block_root } => {
                 debug!(
                     self.log,
-                    "Rejected attestation to finalized block";
+                    "Ignored attestation to finalized block";
                     "block_root" => ?beacon_block_root,
                     "attestation_slot" => failed_att.attestation().data.slot,
                 );
 
-                // We have to reject the message as it isn't a descendant of the finalized
-                // checkpoint.
-                self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Reject);
+                self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore);
 
                 // The peer that sent us this could be a lagger, or a spammer, or this failure could
                 // be due to us processing attestations extremely slowly. Don't be too harsh.