Retrospective invalidation of exec. payloads for opt. sync (#2837)

## Issue Addressed

NA

## Proposed Changes

Adds the functionality to allow blocks to be validated/invalidated after their import as per the [optimistic sync spec](https://github.com/ethereum/consensus-specs/blob/dev/sync/optimistic.md#how-to-optimistically-import-blocks). This means:

- Updating `ProtoArray` to allow flipping the `execution_status` of ancestors/descendants based on payload validity updates.
- Creating separation between `execution_layer` and the `beacon_chain` by creating a `PayloadStatus` struct.
- Refactoring how the `execution_layer` selects a `PayloadStatus` from the multiple statuses returned from multiple EEs.
- Adding testing framework for optimistic imports.
- Add `ExecutionBlockHash(Hash256)` new-type struct to avoid confusion between *beacon block roots* and *execution payload hashes*.
- Add `merge` to [`FORKS`](c3a793fd73/Makefile (L17)) in the `Makefile` to ensure we test the beacon chain with merge settings.
    - Fix some tests here that were failing due to a missing execution layer.

## TODO

- [ ] Balance tests

Co-authored-by: Mark Mackey <mark@sigmaprime.io>

beacon_node/execution_layer/src/test_utils/execution_block_generator.rs

@@ -1,4 +1,7 @@
 use crate::engine_api::{
+    json_structures::{
+        JsonForkchoiceUpdatedV1Response, JsonPayloadStatusV1, JsonPayloadStatusV1Status,
+    },
     ExecutionBlock, PayloadAttributes, PayloadId, PayloadStatusV1, PayloadStatusV1Status,
 };
 use crate::engines::ForkChoiceState;
@@ -6,7 +9,7 @@ use serde::{Deserialize, Serialize};
 use std::collections::HashMap;
 use tree_hash::TreeHash;
 use tree_hash_derive::TreeHash;
-use types::{EthSpec, ExecutionPayload, Hash256, Uint256};
+use types::{EthSpec, ExecutionBlockHash, ExecutionPayload, Hash256, Uint256};
 
 const GAS_LIMIT: u64 = 16384;
 const GAS_USED: u64 = GAS_LIMIT - 1;
@@ -26,14 +29,14 @@ impl<T: EthSpec> Block<T> {
         }
     }
 
-    pub fn parent_hash(&self) -> Hash256 {
+    pub fn parent_hash(&self) -> ExecutionBlockHash {
         match self {
             Block::PoW(block) => block.parent_hash,
             Block::PoS(payload) => payload.parent_hash,
         }
     }
 
-    pub fn block_hash(&self) -> Hash256 {
+    pub fn block_hash(&self) -> ExecutionBlockHash {
         match self {
             Block::PoW(block) => block.block_hash,
             Block::PoS(payload) => payload.block_hash,
@@ -69,8 +72,8 @@ impl<T: EthSpec> Block<T> {
 #[serde(rename_all = "camelCase")]
 pub struct PoWBlock {
     pub block_number: u64,
-    pub block_hash: Hash256,
-    pub parent_hash: Hash256,
+    pub block_hash: ExecutionBlockHash,
+    pub parent_hash: ExecutionBlockHash,
     pub total_difficulty: Uint256,
 }
 
@@ -78,18 +81,18 @@ pub struct ExecutionBlockGenerator<T: EthSpec> {
     /*
      * Common database
      */
-    blocks: HashMap<Hash256, Block<T>>,
-    block_hashes: HashMap<u64, Hash256>,
+    blocks: HashMap<ExecutionBlockHash, Block<T>>,
+    block_hashes: HashMap<u64, ExecutionBlockHash>,
     /*
      * PoW block parameters
      */
     pub terminal_total_difficulty: Uint256,
     pub terminal_block_number: u64,
-    pub terminal_block_hash: Hash256,
+    pub terminal_block_hash: ExecutionBlockHash,
     /*
      * PoS block parameters
      */
-    pub pending_payloads: HashMap<Hash256, ExecutionPayload<T>>,
+    pub pending_payloads: HashMap<ExecutionBlockHash, ExecutionPayload<T>>,
     pub next_payload_id: u64,
     pub payload_ids: HashMap<PayloadId, ExecutionPayload<T>>,
 }
@@ -98,7 +101,7 @@ impl<T: EthSpec> ExecutionBlockGenerator<T> {
     pub fn new(
         terminal_total_difficulty: Uint256,
         terminal_block_number: u64,
-        terminal_block_hash: Hash256,
+        terminal_block_hash: ExecutionBlockHash,
     ) -> Self {
         let mut gen = Self {
             blocks: <_>::default(),
@@ -141,11 +144,11 @@ impl<T: EthSpec> ExecutionBlockGenerator<T> {
             .map(|block| block.as_execution_block(self.terminal_total_difficulty))
     }
 
-    pub fn block_by_hash(&self, hash: Hash256) -> Option<Block<T>> {
+    pub fn block_by_hash(&self, hash: ExecutionBlockHash) -> Option<Block<T>> {
         self.blocks.get(&hash).cloned()
     }
 
-    pub fn execution_block_by_hash(&self, hash: Hash256) -> Option<ExecutionBlock> {
+    pub fn execution_block_by_hash(&self, hash: ExecutionBlockHash) -> Option<ExecutionBlock> {
         self.block_by_hash(hash)
             .map(|block| block.as_execution_block(self.terminal_total_difficulty))
     }
@@ -187,7 +190,7 @@ impl<T: EthSpec> ExecutionBlockGenerator<T> {
 
     pub fn insert_pow_block(&mut self, block_number: u64) -> Result<(), String> {
         let parent_hash = if block_number == 0 {
-            Hash256::zero()
+            ExecutionBlockHash::zero()
         } else if let Some(hash) = self.block_hashes.get(&(block_number - 1)) {
             *hash
         } else {
@@ -231,7 +234,7 @@ impl<T: EthSpec> ExecutionBlockGenerator<T> {
     }
 
     pub fn get_payload(&mut self, id: &PayloadId) -> Option<ExecutionPayload<T>> {
-        self.payload_ids.remove(id)
+        self.payload_ids.get(id).cloned()
     }
 
     pub fn new_payload(&mut self, payload: ExecutionPayload<T>) -> PayloadStatusV1 {
@@ -267,39 +270,35 @@ impl<T: EthSpec> ExecutionBlockGenerator<T> {
         &mut self,
         forkchoice_state: ForkChoiceState,
         payload_attributes: Option<PayloadAttributes>,
-    ) -> Result<Option<PayloadId>, String> {
+    ) -> Result<JsonForkchoiceUpdatedV1Response, String> {
         if let Some(payload) = self
             .pending_payloads
             .remove(&forkchoice_state.head_block_hash)
         {
             self.insert_block(Block::PoS(payload))?;
         }
-        if !self.blocks.contains_key(&forkchoice_state.head_block_hash) {
-            return Err(format!(
-                "block hash {:?} unknown",
-                forkchoice_state.head_block_hash
-            ));
-        }
-        if !self.blocks.contains_key(&forkchoice_state.safe_block_hash) {
-            return Err(format!(
-                "block hash {:?} unknown",
-                forkchoice_state.head_block_hash
-            ));
-        }
 
-        if forkchoice_state.finalized_block_hash != Hash256::zero()
+        let unknown_head_block_hash = !self.blocks.contains_key(&forkchoice_state.head_block_hash);
+        let unknown_safe_block_hash = !self.blocks.contains_key(&forkchoice_state.safe_block_hash);
+        let unknown_finalized_block_hash = forkchoice_state.finalized_block_hash
+            != ExecutionBlockHash::zero()
             && !self
                 .blocks
-                .contains_key(&forkchoice_state.finalized_block_hash)
-        {
-            return Err(format!(
-                "finalized block hash {:?} is unknown",
-                forkchoice_state.finalized_block_hash
-            ));
+                .contains_key(&forkchoice_state.finalized_block_hash);
+
+        if unknown_head_block_hash || unknown_safe_block_hash || unknown_finalized_block_hash {
+            return Ok(JsonForkchoiceUpdatedV1Response {
+                payload_status: JsonPayloadStatusV1 {
+                    status: JsonPayloadStatusV1Status::Syncing,
+                    latest_valid_hash: None,
+                    validation_error: None,
+                },
+                payload_id: None,
+            });
         }
 
-        match payload_attributes {
-            None => Ok(None),
+        let id = match payload_attributes {
+            None => None,
             Some(attributes) => {
                 if !self.blocks.iter().any(|(_, block)| {
                     block.block_hash() == self.terminal_block_hash
@@ -334,17 +333,27 @@ impl<T: EthSpec> ExecutionBlockGenerator<T> {
                     timestamp: attributes.timestamp,
                     extra_data: "block gen was here".as_bytes().to_vec().into(),
                     base_fee_per_gas: Uint256::one(),
-                    block_hash: Hash256::zero(),
+                    block_hash: ExecutionBlockHash::zero(),
                     transactions: vec![].into(),
                 };
 
-                execution_payload.block_hash = execution_payload.tree_hash_root();
+                execution_payload.block_hash =
+                    ExecutionBlockHash::from_root(execution_payload.tree_hash_root());
 
                 self.payload_ids.insert(id, execution_payload);
 
-                Ok(Some(id))
+                Some(id)
             }
-        }
+        };
+
+        Ok(JsonForkchoiceUpdatedV1Response {
+            payload_status: JsonPayloadStatusV1 {
+                status: JsonPayloadStatusV1Status::Valid,
+                latest_valid_hash: Some(forkchoice_state.head_block_hash),
+                validation_error: None,
+            },
+            payload_id: id.map(Into::into),
+        })
     }
 }
 
@@ -356,7 +365,7 @@ pub fn generate_pow_block(
     terminal_total_difficulty: Uint256,
     terminal_block_number: u64,
     block_number: u64,
-    parent_hash: Hash256,
+    parent_hash: ExecutionBlockHash,
 ) -> Result<PoWBlock, String> {
     if block_number > terminal_block_number {
         return Err(format!(
@@ -378,12 +387,12 @@ pub fn generate_pow_block(
 
     let mut block = PoWBlock {
         block_number,
-        block_hash: Hash256::zero(),
+        block_hash: ExecutionBlockHash::zero(),
         parent_hash,
         total_difficulty,
     };
 
-    block.block_hash = block.tree_hash_root();
+    block.block_hash = ExecutionBlockHash::from_root(block.tree_hash_root());
 
     Ok(block)
 }
@@ -402,7 +411,7 @@ mod test {
         let mut generator: ExecutionBlockGenerator<MainnetEthSpec> = ExecutionBlockGenerator::new(
             TERMINAL_DIFFICULTY.into(),
             TERMINAL_BLOCK,
-            Hash256::zero(),
+            ExecutionBlockHash::zero(),
         );
 
         for i in 0..=TERMINAL_BLOCK {
@@ -420,7 +429,7 @@ mod test {
             let expected_parent = i
                 .checked_sub(1)
                 .map(|i| generator.block_by_number(i).unwrap().block_hash())
-                .unwrap_or_else(Hash256::zero);
+                .unwrap_or_else(ExecutionBlockHash::zero);
             assert_eq!(block.parent_hash(), expected_parent);
 
             assert_eq!(

beacon_node/execution_layer/src/test_utils/handle_rpc.rs

@@ -1,5 +1,5 @@
 use super::Context;
-use crate::engine_api::{http::*, PayloadStatusV1, PayloadStatusV1Status};
+use crate::engine_api::{http::*, *};
 use crate::json_structures::*;
 use serde::de::DeserializeOwned;
 use serde_json::Value as JsonValue;
@@ -57,26 +57,29 @@ pub async fn handle_rpc<T: EthSpec>(
         ENGINE_NEW_PAYLOAD_V1 => {
             let request: JsonExecutionPayloadV1<T> = get_param(params, 0)?;
 
-            let response = if let Some(status) = *ctx.static_new_payload_response.lock() {
-                match status {
-                    PayloadStatusV1Status::Valid => PayloadStatusV1 {
-                        status,
-                        latest_valid_hash: Some(request.block_hash),
-                        validation_error: None,
-                    },
-                    PayloadStatusV1Status::Syncing => PayloadStatusV1 {
-                        status,
-                        latest_valid_hash: None,
-                        validation_error: None,
-                    },
-                    _ => unimplemented!("invalid static newPayloadResponse"),
-                }
+            let (static_response, should_import) =
+                if let Some(mut response) = ctx.static_new_payload_response.lock().clone() {
+                    if response.status.status == PayloadStatusV1Status::Valid {
+                        response.status.latest_valid_hash = Some(request.block_hash)
+                    }
+
+                    (Some(response.status), response.should_import)
+                } else {
+                    (None, true)
+                };
+
+            let dynamic_response = if should_import {
+                Some(
+                    ctx.execution_block_generator
+                        .write()
+                        .new_payload(request.into()),
+                )
             } else {
-                ctx.execution_block_generator
-                    .write()
-                    .new_payload(request.into())
+                None
             };
 
+            let response = static_response.or(dynamic_response).unwrap();
+
             Ok(serde_json::to_value(JsonPayloadStatusV1::from(response)).unwrap())
         }
         ENGINE_GET_PAYLOAD_V1 => {
@@ -95,8 +98,7 @@ pub async fn handle_rpc<T: EthSpec>(
             let forkchoice_state: JsonForkChoiceStateV1 = get_param(params, 0)?;
             let payload_attributes: Option<JsonPayloadAttributesV1> = get_param(params, 1)?;
 
-            let head_block_hash = forkchoice_state.head_block_hash;
-            let id = ctx
+            let response = ctx
                 .execution_block_generator
                 .write()
                 .forkchoice_updated_v1(
@@ -104,15 +106,7 @@ pub async fn handle_rpc<T: EthSpec>(
                     payload_attributes.map(|json| json.into()),
                 )?;
 
-            Ok(serde_json::to_value(JsonForkchoiceUpdatedV1Response {
-                payload_status: JsonPayloadStatusV1 {
-                    status: JsonPayloadStatusV1Status::Valid,
-                    latest_valid_hash: Some(head_block_hash),
-                    validation_error: None,
-                },
-                payload_id: id.map(Into::into),
-            })
-            .unwrap())
+            Ok(serde_json::to_value(response).unwrap())
         }
         other => Err(format!(
             "The method {} does not exist/is not available",

beacon_node/execution_layer/src/test_utils/mock_execution_layer.rs

@@ -58,7 +58,7 @@ impl<T: EthSpec> MockExecutionLayer<T> {
         Self::new(
             DEFAULT_TERMINAL_DIFFICULTY.into(),
             DEFAULT_TERMINAL_BLOCK,
-            Hash256::zero(),
+            ExecutionBlockHash::zero(),
             Epoch::new(0),
         )
     }
@@ -66,7 +66,7 @@ impl<T: EthSpec> MockExecutionLayer<T> {
     pub fn new(
         terminal_total_difficulty: Uint256,
         terminal_block: u64,
-        terminal_block_hash: Hash256,
+        terminal_block_hash: ExecutionBlockHash,
         terminal_block_hash_activation_epoch: Epoch,
     ) -> Self {
         let el_runtime = ExecutionLayerRuntime::default();
@@ -117,7 +117,7 @@ impl<T: EthSpec> MockExecutionLayer<T> {
         self.el
             .notify_forkchoice_updated(
                 parent_hash,
-                Hash256::zero(),
+                ExecutionBlockHash::zero(),
                 Some(PayloadAttributes {
                     timestamp,
                     random,
@@ -145,13 +145,11 @@ impl<T: EthSpec> MockExecutionLayer<T> {
         assert_eq!(payload.timestamp, timestamp);
         assert_eq!(payload.random, random);
 
-        let (payload_response, latest_valid_hash) =
-            self.el.notify_new_payload(&payload).await.unwrap();
-        assert_eq!(payload_response, PayloadStatusV1Status::Valid);
-        assert_eq!(latest_valid_hash, Some(vec![payload.block_hash]));
+        let status = self.el.notify_new_payload(&payload).await.unwrap();
+        assert_eq!(status, PayloadStatus::Valid);
 
         self.el
-            .notify_forkchoice_updated(block_hash, Hash256::zero(), None)
+            .notify_forkchoice_updated(block_hash, ExecutionBlockHash::zero(), None)
             .await
             .unwrap();
 

beacon_node/execution_layer/src/test_utils/mod.rs

@@ -1,7 +1,6 @@
 //! Provides a mock execution engine HTTP JSON-RPC API for use in testing.
 
-use crate::engine_api::http::JSONRPC_VERSION;
-use crate::engine_api::PayloadStatusV1Status;
+use crate::engine_api::{http::JSONRPC_VERSION, PayloadStatusV1, PayloadStatusV1Status};
 use bytes::Bytes;
 use environment::null_logger;
 use execution_block_generator::{Block, PoWBlock};
@@ -15,7 +14,7 @@ use std::marker::PhantomData;
 use std::net::{Ipv4Addr, SocketAddr, SocketAddrV4};
 use std::sync::Arc;
 use tokio::{runtime, sync::oneshot};
-use types::{EthSpec, Hash256, Uint256};
+use types::{EthSpec, ExecutionBlockHash, Uint256};
 use warp::Filter;
 
 pub use execution_block_generator::{generate_pow_block, ExecutionBlockGenerator};
@@ -41,7 +40,7 @@ impl<T: EthSpec> MockServer<T> {
             &runtime::Handle::current(),
             DEFAULT_TERMINAL_DIFFICULTY.into(),
             DEFAULT_TERMINAL_BLOCK,
-            Hash256::zero(),
+            ExecutionBlockHash::zero(),
         )
     }
 
@@ -49,7 +48,7 @@ impl<T: EthSpec> MockServer<T> {
         handle: &runtime::Handle,
         terminal_difficulty: Uint256,
         terminal_block: u64,
-        terminal_block_hash: Hash256,
+        terminal_block_hash: ExecutionBlockHash,
     ) -> Self {
         let last_echo_request = Arc::new(RwLock::new(None));
         let preloaded_responses = Arc::new(Mutex::new(vec![]));
@@ -117,14 +116,54 @@ impl<T: EthSpec> MockServer<T> {
     }
 
     pub fn all_payloads_valid(&self) {
-        *self.ctx.static_new_payload_response.lock() = Some(PayloadStatusV1Status::Valid)
+        let response = StaticNewPayloadResponse {
+            status: PayloadStatusV1 {
+                status: PayloadStatusV1Status::Valid,
+                latest_valid_hash: None,
+                validation_error: None,
+            },
+            should_import: true,
+        };
+        *self.ctx.static_new_payload_response.lock() = Some(response)
+    }
+
+    /// Setting `should_import = true` simulates an EE that initially returns `SYNCING` but obtains
+    /// the block via it's own means (e.g., devp2p).
+    pub fn all_payloads_syncing(&self, should_import: bool) {
+        let response = StaticNewPayloadResponse {
+            status: PayloadStatusV1 {
+                status: PayloadStatusV1Status::Syncing,
+                latest_valid_hash: None,
+                validation_error: None,
+            },
+            should_import,
+        };
+        *self.ctx.static_new_payload_response.lock() = Some(response)
+    }
+
+    pub fn all_payloads_invalid(&self, latest_valid_hash: ExecutionBlockHash) {
+        let response = StaticNewPayloadResponse {
+            status: PayloadStatusV1 {
+                status: PayloadStatusV1Status::Invalid,
+                latest_valid_hash: Some(latest_valid_hash),
+                validation_error: Some("static response".into()),
+            },
+            should_import: true,
+        };
+        *self.ctx.static_new_payload_response.lock() = Some(response)
+    }
+
+    /// Disables any static payload response so the execution block generator will do its own
+    /// verification.
+    pub fn full_payload_verification(&self) {
+        *self.ctx.static_new_payload_response.lock() = None
     }
 
     pub fn insert_pow_block(
         &self,
         block_number: u64,
-        block_hash: Hash256,
-        parent_hash: Hash256,
+        block_hash: ExecutionBlockHash,
+        parent_hash: ExecutionBlockHash,
         total_difficulty: Uint256,
     ) {
         let block = Block::PoW(PoWBlock {
@@ -143,7 +182,7 @@ impl<T: EthSpec> MockServer<T> {
             .unwrap()
     }
 
-    pub fn get_block(&self, block_hash: Hash256) -> Option<Block<T>> {
+    pub fn get_block(&self, block_hash: ExecutionBlockHash) -> Option<Block<T>> {
         self.ctx
             .execution_block_generator
             .read()
@@ -178,6 +217,12 @@ struct MissingIdField;
 
 impl warp::reject::Reject for MissingIdField {}
 
+#[derive(Debug, Clone, PartialEq)]
+pub struct StaticNewPayloadResponse {
+    status: PayloadStatusV1,
+    should_import: bool,
+}
+
 /// A wrapper around all the items required to spawn the HTTP server.
 ///
 /// The server will gracefully handle the case where any fields are `None`.
@@ -187,7 +232,7 @@ pub struct Context<T: EthSpec> {
     pub last_echo_request: Arc<RwLock<Option<Bytes>>>,
     pub execution_block_generator: RwLock<ExecutionBlockGenerator<T>>,
     pub preloaded_responses: Arc<Mutex<Vec<serde_json::Value>>>,
-    pub static_new_payload_response: Arc<Mutex<Option<PayloadStatusV1Status>>>,
+    pub static_new_payload_response: Arc<Mutex<Option<StaticNewPayloadResponse>>>,
     pub _phantom: PhantomData<T>,
 }