Merge remote-tracking branch 'origin/unstable' into faster-block-production

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)?,
         };
 
@@ -4111,21 +4111,32 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                         "Fork choice update invalidated payload";
                         "status" => ?status
                     );
-                    // The execution engine has stated that all blocks between the
-                    // `head_execution_block_hash` and `latest_valid_hash` are invalid.
-                    self.process_invalid_execution_payload(
-                        &InvalidationOperation::InvalidateMany {
-                            head_block_root,
-                            always_invalidate_head: true,
-                            latest_valid_ancestor: latest_valid_hash,
-                        },
-                    )
-                    .await?;
+
+                    // This implies that the terminal block was invalid. We are being explicit in
+                    // invalidating only the head block in this case.
+                    if latest_valid_hash == ExecutionBlockHash::zero() {
+                        self.process_invalid_execution_payload(
+                            &InvalidationOperation::InvalidateOne {
+                                block_root: head_block_root,
+                            },
+                        )
+                        .await?;
+                    } else {
+                        // The execution engine has stated that all blocks between the
+                        // `head_execution_block_hash` and `latest_valid_hash` are invalid.
+                        self.process_invalid_execution_payload(
+                            &InvalidationOperation::InvalidateMany {
+                                head_block_root,
+                                always_invalidate_head: true,
+                                latest_valid_ancestor: latest_valid_hash,
+                            },
+                        )
+                        .await?;
+                    }
 
                     Err(BeaconChainError::ExecutionForkChoiceUpdateInvalid { status })
                 }
-                PayloadStatus::InvalidTerminalBlock { .. }
-                | PayloadStatus::InvalidBlockHash { .. } => {
+                PayloadStatus::InvalidBlockHash { .. } => {
                     warn!(
                         self.log,
                         "Fork choice update invalidated payload";
@@ -4589,7 +4600,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
@@ -4604,6 +4615,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);
         }

beacon_node/beacon_chain/src/canonical_head.rs

@@ -719,6 +719,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         drop(old_cached_head);
 
         // If the finalized checkpoint changed, perform some updates.
+        //
+        // The `after_finalization` function will take a write-lock on `fork_choice`, therefore it
+        // is a dead-lock risk to hold any other lock on fork choice at this point.
         if new_view.finalized_checkpoint != old_view.finalized_checkpoint {
             if let Err(e) =
                 self.after_finalization(&new_cached_head, new_view, finalized_proto_block)
@@ -878,6 +881,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
 
     /// Perform updates to caches and other components after the finalized checkpoint has been
     /// changed.
+    ///
+    /// This function will take a write-lock on `canonical_head.fork_choice`, therefore it would be
+    /// unwise to hold any lock on fork choice while calling this function.
     fn after_finalization(
         self: &Arc<Self>,
         new_cached_head: &CachedHead<T::EthSpec>,
@@ -966,6 +972,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             self.head_tracker.clone(),
         )?;
 
+        // Take a write-lock on the canonical head and signal for it to prune.
+        self.canonical_head.fork_choice_write_lock().prune()?;
+
         Ok(())
     }
 

beacon_node/beacon_chain/src/chain_config.rs

@@ -51,7 +51,7 @@ impl Default for ChainConfig {
             builder_fallback_skips_per_epoch: 8,
             builder_fallback_epochs_since_finalization: 3,
             builder_fallback_disable_checks: false,
-            count_unrealized: false,
+            count_unrealized: true,
         }
     }
 }

beacon_node/beacon_chain/src/execution_payload.rs

@@ -114,6 +114,11 @@ async fn notify_new_payload<'a, T: BeaconChainTypes>(
             PayloadStatus::Invalid {
                 latest_valid_hash, ..
             } => {
+                // latest_valid_hash == 0 implies that this was the terminal block
+                // Hence, we don't need to run `BeaconChain::process_invalid_execution_payload`.
+                if latest_valid_hash == ExecutionBlockHash::zero() {
+                    return Err(ExecutionPayloadError::RejectedByExecutionEngine { status }.into());
+                }
                 // This block has not yet been applied to fork choice, so the latest block that was
                 // imported to fork choice was the parent.
                 let latest_root = block.parent_root();
@@ -127,7 +132,7 @@ async fn notify_new_payload<'a, T: BeaconChainTypes>(
 
                 Err(ExecutionPayloadError::RejectedByExecutionEngine { status }.into())
             }
-            PayloadStatus::InvalidTerminalBlock { .. } | PayloadStatus::InvalidBlockHash { .. } => {
+            PayloadStatus::InvalidBlockHash { .. } => {
                 // Returning an error here should be sufficient to invalidate the block. We have no
                 // information to indicate its parent is invalid, so no need to run
                 // `BeaconChain::process_invalid_execution_payload`.

beacon_node/beacon_chain/src/lib.rs

@@ -41,7 +41,7 @@ pub mod sync_committee_verification;
 pub mod test_utils;
 mod timeout_rw_lock;
 pub mod validator_monitor;
-mod validator_pubkey_cache;
+pub mod validator_pubkey_cache;
 
 pub use self::beacon_chain::{
     AttestationProcessingOutcome, BeaconChain, BeaconChainTypes, BeaconStore, ChainSegmentResult,

beacon_node/beacon_chain/src/test_utils.rs

@@ -132,8 +132,7 @@ pub fn test_spec<E: EthSpec>() -> ChainSpec {
                 FORK_NAME_ENV_VAR, e
             )
         });
-        let fork = ForkName::from_str(fork_name.as_str())
-            .unwrap_or_else(|()| panic!("unknown FORK_NAME: {}", fork_name));
+        let fork = ForkName::from_str(fork_name.as_str()).unwrap();
         fork.make_genesis_spec(E::default_spec())
     } else {
         E::default_spec()
@@ -157,6 +156,7 @@ pub struct Builder<T: BeaconChainTypes> {
     execution_layer: Option<ExecutionLayer<T::EthSpec>>,
     mock_execution_layer: Option<MockExecutionLayer<T::EthSpec>>,
     mock_builder: Option<TestingBuilder<T::EthSpec>>,
+    testing_slot_clock: Option<TestingSlotClock>,
     runtime: TestRuntime,
     log: Logger,
 }
@@ -289,6 +289,7 @@ where
             execution_layer: None,
             mock_execution_layer: None,
             mock_builder: None,
+            testing_slot_clock: None,
             runtime,
             log,
         }
@@ -435,6 +436,11 @@ where
         self
     }
 
+    pub fn testing_slot_clock(mut self, slot_clock: TestingSlotClock) -> Self {
+        self.testing_slot_clock = Some(slot_clock);
+        self
+    }
+
     pub fn build(self) -> BeaconChainHarness<BaseHarnessType<E, Hot, Cold>> {
         let (shutdown_tx, shutdown_receiver) = futures::channel::mpsc::channel(1);
 
@@ -475,7 +481,9 @@ where
         };
 
         // Initialize the slot clock only if it hasn't already been initialized.
-        builder = if builder.get_slot_clock().is_none() {
+        builder = if let Some(testing_slot_clock) = self.testing_slot_clock {
+            builder.slot_clock(testing_slot_clock)
+        } else if builder.get_slot_clock().is_none() {
             builder
                 .testing_slot_clock(Duration::from_secs(seconds_per_slot))
                 .expect("should configure testing slot clock")

beacon_node/beacon_chain/src/validator_pubkey_cache.rs

@@ -156,6 +156,11 @@ impl<T: BeaconChainTypes> ValidatorPubkeyCache<T> {
     pub fn len(&self) -> usize {
         self.indices.len()
     }
+
+    /// Returns `true` if there are no validators in the cache.
+    pub fn is_empty(&self) -> bool {
+        self.indices.is_empty()
+    }
 }
 
 /// Wrapper for a public key stored in the database.