From 4fc0cb121c6f90b12ff10a87d4d091cc11285270 Mon Sep 17 00:00:00 2001
From: Paul Hauner <paul@paulhauner.com>
Date: Wed, 10 Aug 2022 13:06:46 +0000
Subject: [PATCH] Remove some "wontfix" TODOs for the merge (#3449)

## Issue Addressed

NA

## Proposed Changes

Removes three types of TODOs:

1. `execution_layer/src/lib.rs`: It was [determined](https://github.com/ethereum/consensus-specs/issues/2636#issuecomment-988688742) that there is no action required here.
2. `beacon_processor/worker/gossip_methods.rs`: Removed TODOs relating to peer scoring that have already been addressed via `epe.penalize_peer()`.
    - It seems `cargo fmt` wanted to adjust some things here as well :shrug:
3. `proto_array_fork_choice.rs`: it would be nice to remove that useless `bool` for cleanliness, but I don't think it's something we need to do and the TODO just makes things look messier IMO.


## Additional Info

There should be no functional changes to the code in this PR.

There are still some TODOs lingering, those ones require actual changes or more thought.
---
 beacon_node/execution_layer/src/lib.rs           |  8 --------
 .../beacon_processor/worker/gossip_methods.rs    | 16 +++++++++++-----
 .../proto_array/src/proto_array_fork_choice.rs   |  2 +-
 3 files changed, 12 insertions(+), 14 deletions(-)

diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs
index f56ea8f7979..e7bbc6cd5eb 100644
--- a/beacon_node/execution_layer/src/lib.rs
+++ b/beacon_node/execution_layer/src/lib.rs
@@ -1267,14 +1267,6 @@ impl<T: EthSpec> ExecutionLayer<T> {
     }
 
     /// Maps to the `eth_getBlockByHash` JSON-RPC call.
-    ///
-    /// ## TODO(merge)
-    ///
-    /// This will return an execution block regardless of whether or not it was created by a PoW
-    /// miner (pre-merge) or a PoS validator (post-merge). It's not immediately clear if this is
-    /// correct or not, see the discussion here:
-    ///
-    /// https://github.com/ethereum/consensus-specs/issues/2636
     async fn get_pow_block(
         &self,
         engine: &Engine,
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 12172e0e539..e6625e43f8e 100644
--- a/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs
+++ b/beacon_node/network/src/beacon_processor/worker/gossip_methods.rs
@@ -771,12 +771,15 @@ impl<T: BeaconChainTypes> Worker<T> {
                 debug!(self.log, "Could not verify block for gossip, ignoring the block";
                             "error" => %e);
                 // Prevent recurring behaviour by penalizing the peer slightly.
-                self.gossip_penalize_peer(peer_id, PeerAction::HighToleranceError, "gossip_block_high");
+                self.gossip_penalize_peer(
+                    peer_id,
+                    PeerAction::HighToleranceError,
+                    "gossip_block_high",
+                );
                 self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore);
                 return None;
             }
-            // TODO(merge): reconsider peer scoring for this event.
-            Err(ref e @BlockError::ExecutionPayloadError(ref epe)) if !epe.penalize_peer() => {
+            Err(ref e @ BlockError::ExecutionPayloadError(ref epe)) if !epe.penalize_peer() => {
                 debug!(self.log, "Could not verify block for gossip, ignoring the block";
                             "error" => %e);
                 self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Ignore);
@@ -795,7 +798,6 @@ impl<T: BeaconChainTypes> Worker<T> {
             | Err(e @ BlockError::TooManySkippedSlots { .. })
             | Err(e @ BlockError::WeakSubjectivityConflict)
             | Err(e @ BlockError::InconsistentFork(_))
-            // TODO(merge): reconsider peer scoring for this event.
             | Err(e @ BlockError::ExecutionPayloadError(_))
             // TODO(merge): reconsider peer scoring for this event.
             | Err(e @ BlockError::ParentExecutionPayloadInvalid { .. })
@@ -803,7 +805,11 @@ impl<T: BeaconChainTypes> Worker<T> {
                 warn!(self.log, "Could not verify block for gossip, rejecting the block";
                             "error" => %e);
                 self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Reject);
-                self.gossip_penalize_peer(peer_id, PeerAction::LowToleranceError, "gossip_block_low");
+                self.gossip_penalize_peer(
+                    peer_id,
+                    PeerAction::LowToleranceError,
+                    "gossip_block_low",
+                );
                 return None;
             }
         };
diff --git a/consensus/proto_array/src/proto_array_fork_choice.rs b/consensus/proto_array/src/proto_array_fork_choice.rs
index 306c9860185..9902ccb1cc2 100644
--- a/consensus/proto_array/src/proto_array_fork_choice.rs
+++ b/consensus/proto_array/src/proto_array_fork_choice.rs
@@ -38,7 +38,7 @@ pub enum ExecutionStatus {
     ///
     /// This `bool` only exists to satisfy our SSZ implementation which requires all variants
     /// to have a value. It can be set to anything.
-    Irrelevant(bool), // TODO(merge): fix bool.
+    Irrelevant(bool),
 }
 
 impl ExecutionStatus {