From 6f13727fbef75ff2501eb3248315842eaa83247b Mon Sep 17 00:00:00 2001
From: realbigsean <seananderson33@GMAIL.com>
Date: Tue, 9 Aug 2022 06:05:16 +0000
Subject: [PATCH] Don't use the builder network if the head is optimistic
 (#3412)

## Issue Addressed

Resolves https://github.com/sigp/lighthouse/issues/3394

Adds a check in `is_healthy` about whether the head is optimistic when choosing whether to use the builder network.



Co-authored-by: realbigsean <sean@sigmaprime.io>
---
 beacon_node/beacon_chain/src/beacon_chain.rs | 15 +++++-
 beacon_node/execution_layer/src/lib.rs       |  4 ++
 beacon_node/http_api/tests/tests.rs          | 57 ++++++++++++++++++++
 3 files changed, 74 insertions(+), 2 deletions(-)

diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs
index fec7fe25ff..d6503d687c 100644
--- a/beacon_node/beacon_chain/src/beacon_chain.rs
+++ b/beacon_node/beacon_chain/src/beacon_chain.rs
@@ -3333,7 +3333,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             pubkey,
             slot: state.slot(),
             chain_health: self
-                .is_healthy()
+                .is_healthy(&parent_root)
                 .map_err(BlockProductionError::BeaconChain)?,
         };
 
@@ -4562,7 +4562,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     ///
     /// Since we are likely calling this during the slot we are going to propose in, don't take into
     /// account the current slot when accounting for skips.
-    pub fn is_healthy(&self) -> Result<ChainHealth, Error> {
+    pub fn is_healthy(&self, parent_root: &Hash256) -> Result<ChainHealth, Error> {
         // Check if the merge has been finalized.
         if let Some(finalized_hash) = self
             .canonical_head
@@ -4577,6 +4577,17 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             return Ok(ChainHealth::PreMerge);
         };
 
+        // Check that the parent is NOT optimistic.
+        if let Some(execution_status) = self
+            .canonical_head
+            .fork_choice_read_lock()
+            .get_block_execution_status(parent_root)
+        {
+            if execution_status.is_strictly_optimistic() {
+                return Ok(ChainHealth::Optimistic);
+            }
+        }
+
         if self.config.builder_fallback_disable_checks {
             return Ok(ChainHealth::Healthy);
         }
diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs
index 59c8f009fa..f56ea8f797 100644
--- a/beacon_node/execution_layer/src/lib.rs
+++ b/beacon_node/execution_layer/src/lib.rs
@@ -114,6 +114,7 @@ pub struct BuilderParams {
 pub enum ChainHealth {
     Healthy,
     Unhealthy(FailedCondition),
+    Optimistic,
     PreMerge,
 }
 
@@ -695,6 +696,9 @@ impl<T: EthSpec> ExecutionLayer<T> {
                 }
                 // Intentional no-op, so we never attempt builder API proposals pre-merge.
                 ChainHealth::PreMerge => (),
+                ChainHealth::Optimistic => info!(self.log(), "The local execution engine is syncing \
+                                            so the builder network cannot safely be used. Attempting \
+                                            to build a block with the local execution engine"),
             }
         }
         self.get_full_payload_caching(
diff --git a/beacon_node/http_api/tests/tests.rs b/beacon_node/http_api/tests/tests.rs
index cc0281e454..fa41102292 100644
--- a/beacon_node/http_api/tests/tests.rs
+++ b/beacon_node/http_api/tests/tests.rs
@@ -3044,6 +3044,55 @@ impl ApiTester {
         self
     }
 
+    pub async fn test_builder_chain_health_optimistic_head(self) -> Self {
+        // Make sure the next payload verification will return optimistic before advancing the chain.
+        self.harness.mock_execution_layer.as_ref().map(|el| {
+            el.server.all_payloads_syncing(true);
+            el
+        });
+        self.harness
+            .extend_chain(
+                1,
+                BlockStrategy::OnCanonicalHead,
+                AttestationStrategy::AllValidators,
+            )
+            .await;
+        self.harness.advance_slot();
+
+        let slot = self.chain.slot().unwrap();
+        let epoch = self.chain.epoch().unwrap();
+
+        let (proposer_index, randao_reveal) = self.get_test_randao(slot, epoch).await;
+
+        let payload = self
+            .client
+            .get_validator_blinded_blocks::<E, BlindedPayload<E>>(slot, &randao_reveal, None)
+            .await
+            .unwrap()
+            .data
+            .body()
+            .execution_payload()
+            .unwrap()
+            .clone();
+
+        let expected_fee_recipient = Address::from_low_u64_be(proposer_index as u64);
+        assert_eq!(
+            payload.execution_payload_header.fee_recipient,
+            expected_fee_recipient
+        );
+
+        // If this cache is populated, it indicates fallback to the local EE was correctly used.
+        assert!(self
+            .chain
+            .execution_layer
+            .as_ref()
+            .unwrap()
+            .get_payload_by_root(&payload.tree_hash_root())
+            .is_some());
+
+        self
+    }
+
     #[cfg(target_os = "linux")]
     pub async fn test_get_lighthouse_health(self) -> Self {
         self.client.get_lighthouse_health().await.unwrap();
@@ -4000,6 +4049,14 @@ async fn builder_chain_health_epochs_since_finalization() {
         .await;
 }
 
+#[tokio::test(flavor = "multi_thread", worker_threads = 2)]
+async fn builder_chain_health_optimistic_head() {
+    ApiTester::new_mev_tester()
+        .await
+        .test_builder_chain_health_optimistic_head()
+        .await;
+}
+
 #[tokio::test(flavor = "multi_thread", worker_threads = 2)]
 async fn lighthouse_endpoints() {
     ApiTester::new()