Disallow attesting to optimistic head (#3140)

## Issue Addressed

NA

## Proposed Changes

Disallow the production of attestations and retrieval of unaggregated attestations when they reference an optimistic head. Add tests to this end.

I also moved `BeaconChain::produce_unaggregated_attestation_for_block` to the `BeaconChainHarness`. It was only being used during tests, so it's nice to stop pretending it's production code. I also needed something that could produce attestations to optimistic blocks in order to simulate scenarios where the justified checkpoint is determined invalid (if no one would attest to an optimistic block, we could never justify it and then flip it to invalid).

## Additional Info

- ~~Blocked on #3126~~

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -75,7 +75,6 @@ use state_processing::{
     state_advance::{complete_state_advance, partial_state_advance},
     BlockSignatureStrategy, SigVerifiedOp, VerifyBlockRoot,
 };
-use std::borrow::Cow;
 use std::cmp::Ordering;
 use std::collections::HashMap;
 use std::collections::HashSet;
@@ -1430,8 +1429,13 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     pub fn get_aggregated_attestation(
         &self,
         data: &AttestationData,
-    ) -> Option<Attestation<T::EthSpec>> {
+    ) -> Result<Option<Attestation<T::EthSpec>>, Error> {
-        self.naive_aggregation_pool.read().get(data)
+        if let Some(attestation) = self.naive_aggregation_pool.read().get(data) {
+            self.filter_optimistic_attestation(attestation)
+                .map(Option::Some)
+        } else {
+            Ok(None)
+        }
     }
 
     /// Returns an aggregated `Attestation`, if any, that has a matching
@@ -1442,10 +1446,43 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         &self,
         slot: Slot,
         attestation_data_root: &Hash256,
-    ) -> Option<Attestation<T::EthSpec>> {
+    ) -> Result<Option<Attestation<T::EthSpec>>, Error> {
-        self.naive_aggregation_pool
+        if let Some(attestation) = self
+            .naive_aggregation_pool
             .read()
             .get_by_slot_and_root(slot, attestation_data_root)
+        {
+            self.filter_optimistic_attestation(attestation)
+                .map(Option::Some)
+        } else {
+            Ok(None)
+        }
+    }
+
+    /// Returns `Ok(attestation)` if the supplied `attestation` references a valid
+    /// `beacon_block_root`.
+    fn filter_optimistic_attestation(
+        &self,
+        attestation: Attestation<T::EthSpec>,
+    ) -> Result<Attestation<T::EthSpec>, Error> {
+        let beacon_block_root = attestation.data.beacon_block_root;
+        match self
+            .fork_choice
+            .read()
+            .get_block_execution_status(&beacon_block_root)
+        {
+            // The attestation references a block that is not in fork choice, it must be
+            // pre-finalization.
+            None => Err(Error::CannotAttestToFinalizedBlock { beacon_block_root }),
+            // The attestation references a fully valid `beacon_block_root`.
+            Some(execution_status) if execution_status.is_valid_or_irrelevant() => Ok(attestation),
+            // The attestation references a block that has not been verified by an EL (i.e. it
+            // is optimistic or invalid). Don't return the block, return an error instead.
+            Some(execution_status) => Err(Error::HeadBlockNotFullyVerified {
+                beacon_block_root,
+                execution_status,
+            }),
+        }
     }
 
     /// Return an aggregated `SyncCommitteeContribution` matching the given `root`.
@@ -1481,6 +1518,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         //
         // In effect, the early attester cache prevents slow database IO from causing missed
         // head/target votes.
+        //
+        // The early attester cache should never contain an optimistically imported block.
         match self
             .early_attester_cache
             .try_attest(request_slot, request_index, &self.spec)
@@ -1597,6 +1636,22 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         }
         drop(head_timer);
 
+        // Only attest to a block if it is fully verified (i.e. not optimistic or invalid).
+        match self
+            .fork_choice
+            .read()
+            .get_block_execution_status(&beacon_block_root)
+        {
+            Some(execution_status) if execution_status.is_valid_or_irrelevant() => (),
+            Some(execution_status) => {
+                return Err(Error::HeadBlockNotFullyVerified {
+                    beacon_block_root,
+                    execution_status,
+                })
+            }
+            None => return Err(Error::HeadMissingFromForkChoice(beacon_block_root)),
+        };
+
         /*
          *  Phase 2/2:
          *
@@ -1657,64 +1712,6 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         })
     }
 
-    /// Produces an "unaggregated" attestation for the given `slot` and `index` that attests to
-    /// `beacon_block_root`. The provided `state` should match the `block.state_root` for the
-    /// `block` identified by `beacon_block_root`.
-    ///
-    /// The attestation doesn't _really_ have anything about it that makes it unaggregated per say,
-    /// however this function is only required in the context of forming an unaggregated
-    /// attestation. It would be an (undetectable) violation of the protocol to create a
-    /// `SignedAggregateAndProof` based upon the output of this function.
-    pub fn produce_unaggregated_attestation_for_block(
-        &self,
-        slot: Slot,
-        index: CommitteeIndex,
-        beacon_block_root: Hash256,
-        mut state: Cow<BeaconState<T::EthSpec>>,
-        state_root: Hash256,
-    ) -> Result<Attestation<T::EthSpec>, Error> {
-        let epoch = slot.epoch(T::EthSpec::slots_per_epoch());
-
-        if state.slot() > slot {
-            return Err(Error::CannotAttestToFutureState);
-        } else if state.current_epoch() < epoch {
-            let mut_state = state.to_mut();
-            // Only perform a "partial" state advance since we do not require the state roots to be
-            // accurate.
-            partial_state_advance(
-                mut_state,
-                Some(state_root),
-                epoch.start_slot(T::EthSpec::slots_per_epoch()),
-                &self.spec,
-            )?;
-            mut_state.build_committee_cache(RelativeEpoch::Current, &self.spec)?;
-        }
-
-        let committee_len = state.get_beacon_committee(slot, index)?.committee.len();
-
-        let target_slot = epoch.start_slot(T::EthSpec::slots_per_epoch());
-        let target_root = if state.slot() <= target_slot {
-            beacon_block_root
-        } else {
-            *state.get_block_root(target_slot)?
-        };
-
-        Ok(Attestation {
-            aggregation_bits: BitList::with_capacity(committee_len)?,
-            data: AttestationData {
-                slot,
-                index,
-                beacon_block_root,
-                source: state.current_justified_checkpoint(),
-                target: Checkpoint {
-                    epoch,
-                    root: target_root,
-                },
-            },
-            signature: AggregateSignature::empty(),
-        })
-    }
-
     /// Performs the same validation as `Self::verify_unaggregated_attestation_for_gossip`, but for
     /// multiple attestations using batch BLS verification. Batch verification can provide
     /// significant CPU-time savings compared to individual verification.
@@ -2678,13 +2675,20 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             }
         }
 
-        // If the block is recent enough, check to see if it becomes the head block. If so, apply it
+        // If the block is recent enough and it was not optimistically imported, check to see if it
-        // to the early attester cache. This will allow attestations to the block without waiting
+        // becomes the head block. If so, apply it to the early attester cache. This will allow
-        // for the block and state to be inserted to the database.
+        // attestations to the block without waiting for the block and state to be inserted to the
+        // database.
         //
         // Only performing this check on recent blocks avoids slowing down sync with lots of calls
         // to fork choice `get_head`.
-        if block.slot() + EARLY_ATTESTER_CACHE_HISTORIC_SLOTS >= current_slot {
+        //
+        // Optimistically imported blocks are not added to the cache since the cache is only useful
+        // for a small window of time and the complexity of keeping track of the optimistic status
+        // is not worth it.
+        if !payload_verification_status.is_optimistic()
+            && block.slot() + EARLY_ATTESTER_CACHE_HISTORIC_SLOTS >= current_slot
+        {
             let new_head_root = fork_choice
                 .get_head(current_slot, &self.spec)
                 .map_err(BeaconChainError::from)?;
@@ -4185,7 +4189,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         let status = match head_block.execution_status {
             ExecutionStatus::Valid(block_hash) => HeadSafetyStatus::Safe(Some(block_hash)),
             ExecutionStatus::Invalid(block_hash) => HeadSafetyStatus::Invalid(block_hash),
-            ExecutionStatus::Unknown(block_hash) => HeadSafetyStatus::Unsafe(block_hash),
+            ExecutionStatus::Optimistic(block_hash) => HeadSafetyStatus::Unsafe(block_hash),
             ExecutionStatus::Irrelevant(_) => HeadSafetyStatus::Safe(None),
         };
 

beacon_node/beacon_chain/src/block_verification.rs

@@ -1165,7 +1165,7 @@ impl<'a, T: BeaconChainTypes> FullyVerifiedBlock<'a, T> {
 
         // If the payload did not validate or invalidate the block, check to see if this block is
         // valid for optimistic import.
-        if payload_verification_status == PayloadVerificationStatus::NotVerified {
+        if payload_verification_status.is_optimistic() {
             let current_slot = chain
                 .slot_clock
                 .now()

beacon_node/beacon_chain/src/errors.rs

@@ -9,6 +9,7 @@ use crate::observed_aggregates::Error as ObservedAttestationsError;
 use crate::observed_attesters::Error as ObservedAttestersError;
 use crate::observed_block_producers::Error as ObservedBlockProducersError;
 use execution_layer::PayloadStatus;
+use fork_choice::ExecutionStatus;
 use futures::channel::mpsc::TrySendError;
 use operation_pool::OpPoolError;
 use safe_arith::ArithError;
@@ -162,6 +163,13 @@ pub enum BeaconChainError {
         fork_choice: Hash256,
     },
     InvalidSlot(Slot),
+    HeadBlockNotFullyVerified {
+        beacon_block_root: Hash256,
+        execution_status: ExecutionStatus,
+    },
+    CannotAttestToFinalizedBlock {
+        beacon_block_root: Hash256,
+    },
 }
 
 easy_from_to!(SlotProcessingError, BeaconChainError);

beacon_node/beacon_chain/src/execution_payload.rs

@@ -61,7 +61,7 @@ pub fn notify_new_payload<T: BeaconChainTypes>(
         Ok(status) => match status {
             PayloadStatus::Valid => Ok(PayloadVerificationStatus::Verified),
             PayloadStatus::Syncing | PayloadStatus::Accepted => {
-                Ok(PayloadVerificationStatus::NotVerified)
+                Ok(PayloadVerificationStatus::Optimistic)
             }
             PayloadStatus::Invalid {
                 latest_valid_hash, ..
@@ -193,7 +193,7 @@ pub fn validate_execution_payload_for_gossip<T: BeaconChainTypes>(
 
         let is_merge_transition_complete = match parent_block.execution_status {
             // Optimistically declare that an "unknown" status block has completed the merge.
-            ExecutionStatus::Valid(_) | ExecutionStatus::Unknown(_) => true,
+            ExecutionStatus::Valid(_) | ExecutionStatus::Optimistic(_) => true,
             // It's impossible for an irrelevant block to have completed the merge. It is pre-merge
             // by definition.
             ExecutionStatus::Irrelevant(_) => false,

beacon_node/beacon_chain/src/fork_revert.rs

@@ -172,7 +172,7 @@ pub fn reset_fork_choice_to_finalization<E: EthSpec, Hot: ItemStore<E>, Cold: It
         // retro-actively determine if they were valid or not.
         //
         // This scenario is so rare that it seems OK to double-verify some blocks.
-        let payload_verification_status = PayloadVerificationStatus::NotVerified;
+        let payload_verification_status = PayloadVerificationStatus::Optimistic;
 
         let (block, _) = block.deconstruct();
         fork_choice

beacon_node/beacon_chain/src/test_utils.rs

@@ -31,7 +31,10 @@ use rayon::prelude::*;
 use sensitive_url::SensitiveUrl;
 use slog::Logger;
 use slot_clock::TestingSlotClock;
-use state_processing::{state_advance::complete_state_advance, StateRootStrategy};
+use state_processing::{
+    state_advance::{complete_state_advance, partial_state_advance},
+    StateRootStrategy,
+};
 use std::borrow::Cow;
 use std::collections::{HashMap, HashSet};
 use std::str::FromStr;
@@ -42,15 +45,7 @@ use task_executor::ShutdownReason;
 use tree_hash::TreeHash;
 use types::sync_selection_proof::SyncSelectionProof;
 pub use types::test_utils::generate_deterministic_keypairs;
-use types::{
+use types::{typenum::U4294967296, *};
-    typenum::U4294967296, Address, AggregateSignature, Attestation, AttestationData,
-    AttesterSlashing, BeaconBlock, BeaconState, BeaconStateHash, ChainSpec, Checkpoint, Deposit,
-    DepositData, Domain, Epoch, EthSpec, ForkName, Graffiti, Hash256, IndexedAttestation, Keypair,
-    ProposerSlashing, PublicKeyBytes, SelectionProof, SignatureBytes, SignedAggregateAndProof,
-    SignedBeaconBlock, SignedBeaconBlockHash, SignedContributionAndProof, SignedRoot,
-    SignedVoluntaryExit, Slot, SubnetId, SyncCommittee, SyncCommitteeContribution,
-    SyncCommitteeMessage, VariableList, VoluntaryExit,
-};
 
 // 4th September 2019
 pub const HARNESS_GENESIS_TIME: u64 = 1_567_552_690;
@@ -685,6 +680,67 @@ where
         (signed_block, pre_state)
     }
 
+    /// Produces an "unaggregated" attestation for the given `slot` and `index` that attests to
+    /// `beacon_block_root`. The provided `state` should match the `block.state_root` for the
+    /// `block` identified by `beacon_block_root`.
+    ///
+    /// The attestation doesn't _really_ have anything about it that makes it unaggregated per say,
+    /// however this function is only required in the context of forming an unaggregated
+    /// attestation. It would be an (undetectable) violation of the protocol to create a
+    /// `SignedAggregateAndProof` based upon the output of this function.
+    ///
+    /// This function will produce attestations to optimistic blocks, which is against the
+    /// specification but useful during testing.
+    pub fn produce_unaggregated_attestation_for_block(
+        &self,
+        slot: Slot,
+        index: CommitteeIndex,
+        beacon_block_root: Hash256,
+        mut state: Cow<BeaconState<E>>,
+        state_root: Hash256,
+    ) -> Result<Attestation<E>, BeaconChainError> {
+        let epoch = slot.epoch(E::slots_per_epoch());
+
+        if state.slot() > slot {
+            return Err(BeaconChainError::CannotAttestToFutureState);
+        } else if state.current_epoch() < epoch {
+            let mut_state = state.to_mut();
+            // Only perform a "partial" state advance since we do not require the state roots to be
+            // accurate.
+            partial_state_advance(
+                mut_state,
+                Some(state_root),
+                epoch.start_slot(E::slots_per_epoch()),
+                &self.spec,
+            )?;
+            mut_state.build_committee_cache(RelativeEpoch::Current, &self.spec)?;
+        }
+
+        let committee_len = state.get_beacon_committee(slot, index)?.committee.len();
+
+        let target_slot = epoch.start_slot(E::slots_per_epoch());
+        let target_root = if state.slot() <= target_slot {
+            beacon_block_root
+        } else {
+            *state.get_block_root(target_slot)?
+        };
+
+        Ok(Attestation {
+            aggregation_bits: BitList::with_capacity(committee_len)?,
+            data: AttestationData {
+                slot,
+                index,
+                beacon_block_root,
+                source: state.current_justified_checkpoint(),
+                target: Checkpoint {
+                    epoch,
+                    root: target_root,
+                },
+            },
+            signature: AggregateSignature::empty(),
+        })
+    }
+
     /// A list of attestations for each committee for the given slot.
     ///
     /// The first layer of the Vec is organised per committee. For example, if the return value is
@@ -716,7 +772,6 @@ where
                             return None;
                         }
                         let mut attestation = self
-                            .chain
                             .produce_unaggregated_attestation_for_block(
                                 attestation_slot,
                                 bc.index,
@@ -899,6 +954,7 @@ where
                         let aggregate = self
                             .chain
                             .get_aggregated_attestation(&attestation.data)
+                            .unwrap()
                             .unwrap_or_else(|| {
                                 committee_attestations.iter().skip(1).fold(
                                     attestation.clone(),

beacon_node/beacon_chain/tests/payload_invalidation.rs

@@ -14,6 +14,7 @@ use proto_array::{Error as ProtoArrayError, ExecutionStatus};
 use slot_clock::SlotClock;
 use std::time::Duration;
 use task_executor::ShutdownReason;
+use tree_hash::TreeHash;
 use types::*;
 
 const VALIDATOR_COUNT: usize = 32;
@@ -221,8 +222,8 @@ impl InvalidPayloadRig {
                 let execution_status = self.execution_status(root.into());
 
                 match is_valid {
-                    Payload::Syncing => assert!(execution_status.is_not_verified()),
+                    Payload::Syncing => assert!(execution_status.is_optimistic()),
-                    Payload::Valid => assert!(execution_status.is_valid()),
+                    Payload::Valid => assert!(execution_status.is_valid_and_post_bellatrix()),
                     Payload::Invalid { .. } => unreachable!(),
                 }
 
@@ -309,7 +310,7 @@ fn invalid_payload_invalidates_parent() {
         latest_valid_hash: Some(latest_valid_hash),
     });
 
-    assert!(rig.execution_status(roots[0]).is_valid());
+    assert!(rig.execution_status(roots[0]).is_valid_and_post_bellatrix());
     assert!(rig.execution_status(roots[1]).is_invalid());
     assert!(rig.execution_status(roots[2]).is_invalid());
 
@@ -397,9 +398,9 @@ fn pre_finalized_latest_valid_hash() {
         let slot = Slot::new(i);
         let root = rig.block_root_at_slot(slot).unwrap();
         if slot == 1 {
-            assert!(rig.execution_status(root).is_valid());
+            assert!(rig.execution_status(root).is_valid_and_post_bellatrix());
         } else {
-            assert!(rig.execution_status(root).is_not_verified());
+            assert!(rig.execution_status(root).is_optimistic());
         }
     }
 }
@@ -446,7 +447,7 @@ fn latest_valid_hash_will_validate() {
         } else if slot == 0 {
             assert!(execution_status.is_irrelevant())
         } else {
-            assert!(execution_status.is_valid())
+            assert!(execution_status.is_valid_and_post_bellatrix())
         }
     }
 }
@@ -484,9 +485,9 @@ fn latest_valid_hash_is_junk() {
         let slot = Slot::new(i);
         let root = rig.block_root_at_slot(slot).unwrap();
         if slot == 1 {
-            assert!(rig.execution_status(root).is_valid());
+            assert!(rig.execution_status(root).is_valid_and_post_bellatrix());
         } else {
-            assert!(rig.execution_status(root).is_not_verified());
+            assert!(rig.execution_status(root).is_optimistic());
         }
     }
 }
@@ -556,7 +557,7 @@ fn invalidates_all_descendants() {
         let execution_status = rig.execution_status(root);
         if slot <= latest_valid_slot {
             // Blocks prior to the latest valid hash are valid.
-            assert!(execution_status.is_valid());
+            assert!(execution_status.is_valid_and_post_bellatrix());
         } else {
             // Blocks after the latest valid hash are invalid.
             assert!(execution_status.is_invalid());
@@ -607,7 +608,7 @@ fn switches_heads() {
     assert_eq!(rig.head_info().block_root, fork_block_root);
 
     // The fork block has not yet been validated.
-    assert!(rig.execution_status(fork_block_root).is_not_verified());
+    assert!(rig.execution_status(fork_block_root).is_optimistic());
 
     for root in blocks {
         let slot = rig.harness.chain.get_block(&root).unwrap().unwrap().slot();
@@ -620,7 +621,7 @@ fn switches_heads() {
         let execution_status = rig.execution_status(root);
         if slot <= latest_valid_slot {
             // Blocks prior to the latest valid hash are valid.
-            assert!(execution_status.is_valid());
+            assert!(execution_status.is_valid_and_post_bellatrix());
         } else {
             // Blocks after the latest valid hash are invalid.
             assert!(execution_status.is_invalid());
@@ -691,13 +692,13 @@ fn manually_validate_child() {
     let parent = rig.import_block(Payload::Syncing);
     let child = rig.import_block(Payload::Syncing);
 
-    assert!(rig.execution_status(parent).is_not_verified());
+    assert!(rig.execution_status(parent).is_optimistic());
-    assert!(rig.execution_status(child).is_not_verified());
+    assert!(rig.execution_status(child).is_optimistic());
 
     rig.validate_manually(child);
 
-    assert!(rig.execution_status(parent).is_valid());
+    assert!(rig.execution_status(parent).is_valid_and_post_bellatrix());
-    assert!(rig.execution_status(child).is_valid());
+    assert!(rig.execution_status(child).is_valid_and_post_bellatrix());
 }
 
 #[test]
@@ -709,13 +710,13 @@ fn manually_validate_parent() {
     let parent = rig.import_block(Payload::Syncing);
     let child = rig.import_block(Payload::Syncing);
 
-    assert!(rig.execution_status(parent).is_not_verified());
+    assert!(rig.execution_status(parent).is_optimistic());
-    assert!(rig.execution_status(child).is_not_verified());
+    assert!(rig.execution_status(child).is_optimistic());
 
     rig.validate_manually(parent);
 
-    assert!(rig.execution_status(parent).is_valid());
+    assert!(rig.execution_status(parent).is_valid_and_post_bellatrix());
-    assert!(rig.execution_status(child).is_not_verified());
+    assert!(rig.execution_status(child).is_optimistic());
 }
 
 #[test]
@@ -819,7 +820,7 @@ fn invalid_parent() {
             block_root,
             Duration::from_secs(0),
             &state,
-            PayloadVerificationStatus::NotVerified,
+            PayloadVerificationStatus::Optimistic,
             &rig.harness.chain.spec
         ),
         Err(ForkChoiceError::ProtoArrayError(message))
@@ -885,3 +886,107 @@ fn payload_preparation_before_transition_block() {
     assert_eq!(payload_attributes.suggested_fee_recipient, fee_recipient);
     assert_eq!(fork_choice_state.head_block_hash, latest_block_hash);
 }
+
+#[test]
+fn attesting_to_optimistic_head() {
+    let mut rig = InvalidPayloadRig::new();
+    rig.move_to_terminal_block();
+    rig.import_block(Payload::Valid); // Import a valid transition block.
+
+    let root = rig.import_block(Payload::Syncing);
+
+    let head = rig.harness.chain.head().unwrap();
+    let slot = head.beacon_block.slot();
+    assert_eq!(
+        head.beacon_block_root, root,
+        "the head should be the latest imported block"
+    );
+    assert!(
+        rig.execution_status(root).is_optimistic(),
+        "the head should be optimistic"
+    );
+
+    /*
+     * Define an attestation for use during testing. It doesn't have a valid signature, but that's
+     * not necessary here.
+     */
+
+    let attestation = {
+        let mut attestation = rig
+            .harness
+            .chain
+            .produce_unaggregated_attestation(Slot::new(0), 0)
+            .unwrap();
+
+        attestation.aggregation_bits.set(0, true).unwrap();
+        attestation.data.slot = slot;
+        attestation.data.beacon_block_root = root;
+
+        rig.harness
+            .chain
+            .naive_aggregation_pool
+            .write()
+            .insert(&attestation)
+            .unwrap();
+
+        attestation
+    };
+
+    /*
+     * Define some closures to produce attestations.
+     */
+
+    let produce_unaggregated = || rig.harness.chain.produce_unaggregated_attestation(slot, 0);
+
+    let get_aggregated = || {
+        rig.harness
+            .chain
+            .get_aggregated_attestation(&attestation.data)
+    };
+
+    let get_aggregated_by_slot_and_root = || {
+        rig.harness
+            .chain
+            .get_aggregated_attestation_by_slot_and_root(
+                attestation.data.slot,
+                &attestation.data.tree_hash_root(),
+            )
+    };
+
+    /*
+     * Ensure attestation production fails with an optimistic head.
+     */
+
+    macro_rules! assert_head_block_not_fully_verified {
+        ($func: expr) => {
+            assert!(matches!(
+                $func,
+                Err(BeaconChainError::HeadBlockNotFullyVerified {
+                    beacon_block_root,
+                    execution_status
+                })
+                if beacon_block_root == root && matches!(execution_status, ExecutionStatus::Optimistic(_))
+            ));
+        }
+    }
+
+    assert_head_block_not_fully_verified!(produce_unaggregated());
+    assert_head_block_not_fully_verified!(get_aggregated());
+    assert_head_block_not_fully_verified!(get_aggregated_by_slot_and_root());
+
+    /*
+     * Ensure attestation production succeeds once the head is verified.
+     *
+     * This is effectively a control for the previous tests.
+     */
+
+    rig.validate_manually(root);
+    assert!(
+        rig.execution_status(root).is_valid_and_post_bellatrix(),
+        "the head should no longer be optimistic"
+    );
+
+    produce_unaggregated().unwrap();
+    get_aggregated().unwrap();
+    get_aggregated_by_slot_and_root().unwrap();
+}

beacon_node/http_api/src/lib.rs

@@ -2138,6 +2138,12 @@ pub fn serve<T: BeaconChainTypes>(
                             query.slot,
                             &query.attestation_data_root,
                         )
+                        .map_err(|e| {
+                            warp_utils::reject::custom_bad_request(format!(
+                                "unable to fetch aggregate: {:?}",
+                                e
+                            ))
+                        })?
                         .map(api_types::GenericResponse::from)
                         .ok_or_else(|| {
                             warp_utils::reject::custom_not_found(

consensus/fork_choice/src/fork_choice.rs

@@ -122,11 +122,22 @@ pub enum PayloadVerificationStatus {
     /// An EL has declared the execution payload to be valid.
     Verified,
     /// An EL has not yet made a determination about the execution payload.
-    NotVerified,
+    Optimistic,
     /// The block is either pre-merge-fork, or prior to the terminal PoW block.
     Irrelevant,
 }
 
+impl PayloadVerificationStatus {
+    /// Returns `true` if the payload was optimistically imported.
+    pub fn is_optimistic(&self) -> bool {
+        match self {
+            PayloadVerificationStatus::Verified => false,
+            PayloadVerificationStatus::Optimistic => true,
+            PayloadVerificationStatus::Irrelevant => false,
+        }
+    }
+}
+
 /// Calculate how far `slot` lies from the start of its epoch.
 ///
 /// ## Specification
@@ -668,7 +679,9 @@ where
             } else {
                 match payload_verification_status {
                     PayloadVerificationStatus::Verified => ExecutionStatus::Valid(block_hash),
-                    PayloadVerificationStatus::NotVerified => ExecutionStatus::Unknown(block_hash),
+                    PayloadVerificationStatus::Optimistic => {
+                        ExecutionStatus::Optimistic(block_hash)
+                    }
                     // It would be a logic error to declare a block irrelevant if it has an
                     // execution payload with a non-zero block hash.
                     PayloadVerificationStatus::Irrelevant => {
@@ -944,6 +957,15 @@ where
         }
     }
 
+    /// Returns an `ExecutionStatus` if the block is known **and** a descendant of the finalized root.
+    pub fn get_block_execution_status(&self, block_root: &Hash256) -> Option<ExecutionStatus> {
+        if self.is_descendant_of_finalized(*block_root) {
+            self.proto_array.get_block_execution_status(block_root)
+        } else {
+            None
+        }
+    }
+
     /// Returns the `ProtoBlock` for the justified checkpoint.
     ///
     /// ## Notes

consensus/fork_choice/src/lib.rs

@@ -6,4 +6,4 @@ pub use crate::fork_choice::{
     PayloadVerificationStatus, PersistedForkChoice, QueuedAttestation,
 };
 pub use fork_choice_store::ForkChoiceStore;
-pub use proto_array::{Block as ProtoBlock, InvalidationOperation};
+pub use proto_array::{Block as ProtoBlock, ExecutionStatus, InvalidationOperation};

consensus/proto_array/src/fork_choice_test_definition.rs

@@ -85,7 +85,7 @@ impl ForkChoiceTestDefinition {
             self.finalized_checkpoint,
             junk_shuffling_id.clone(),
             junk_shuffling_id,
-            ExecutionStatus::Unknown(ExecutionBlockHash::zero()),
+            ExecutionStatus::Optimistic(ExecutionBlockHash::zero()),
         )
         .expect("should create fork choice struct");
 
@@ -189,9 +189,9 @@ impl ForkChoiceTestDefinition {
                         justified_checkpoint,
                         finalized_checkpoint,
                         // All blocks are imported optimistically.
-                        execution_status: ExecutionStatus::Unknown(ExecutionBlockHash::from_root(
+                        execution_status: ExecutionStatus::Optimistic(
-                            root,
+                            ExecutionBlockHash::from_root(root),
-                        )),
+                        ),
                     };
                     fork_choice.process_block(block).unwrap_or_else(|e| {
                         panic!(

consensus/proto_array/src/proto_array.rs

@@ -387,7 +387,7 @@ impl ProtoArray {
                 ExecutionStatus::Irrelevant(_) => return Ok(()),
                 // The block has an unknown status, set it to valid since any ancestor of a valid
                 // payload can be considered valid.
-                ExecutionStatus::Unknown(payload_block_hash) => {
+                ExecutionStatus::Optimistic(payload_block_hash) => {
                     node.execution_status = ExecutionStatus::Valid(payload_block_hash);
                     if let Some(parent_index) = node.parent {
                         parent_index
@@ -458,7 +458,7 @@ impl ProtoArray {
             match node.execution_status {
                 ExecutionStatus::Valid(hash)
                 | ExecutionStatus::Invalid(hash)
-                | ExecutionStatus::Unknown(hash) => {
+                | ExecutionStatus::Optimistic(hash) => {
                     // If we're no longer processing the `head_block_root` and the last valid
                     // ancestor is unknown, exit this loop and proceed to invalidate and
                     // descendants of `head_block_root`/`latest_valid_ancestor_root`.
@@ -516,7 +516,7 @@ impl ProtoArray {
                             payload_block_hash: *hash,
                         })
                     }
-                    ExecutionStatus::Unknown(hash) => {
+                    ExecutionStatus::Optimistic(hash) => {
                         invalidated_indices.insert(index);
                         node.execution_status = ExecutionStatus::Invalid(*hash);
 
@@ -580,7 +580,7 @@ impl ProtoArray {
                                 payload_block_hash: *hash,
                             })
                         }
-                        ExecutionStatus::Unknown(hash) | ExecutionStatus::Invalid(hash) => {
+                        ExecutionStatus::Optimistic(hash) | ExecutionStatus::Invalid(hash) => {
                             node.execution_status = ExecutionStatus::Invalid(*hash)
                         }
                         ExecutionStatus::Irrelevant(_) => {

consensus/proto_array/src/proto_array_fork_choice.rs

@@ -1,5 +1,5 @@
 use crate::error::Error;
-use crate::proto_array::{InvalidationOperation, Iter, ProposerBoost, ProtoArray};
+use crate::proto_array::{InvalidationOperation, Iter, ProposerBoost, ProtoArray, ProtoNode};
 use crate::ssz_container::SszContainer;
 use serde_derive::{Deserialize, Serialize};
 use ssz::{Decode, Encode};
@@ -28,7 +28,7 @@ pub enum ExecutionStatus {
     /// An EL has determined that the payload is invalid.
     Invalid(ExecutionBlockHash),
     /// An EL has not yet verified the execution payload.
-    Unknown(ExecutionBlockHash),
+    Optimistic(ExecutionBlockHash),
     /// The block is either prior to the merge fork, or after the merge fork but before the terminal
     /// PoW block has been found.
     ///
@@ -52,30 +52,48 @@ impl ExecutionStatus {
         match self {
             ExecutionStatus::Valid(hash)
             | ExecutionStatus::Invalid(hash)
-            | ExecutionStatus::Unknown(hash) => Some(*hash),
+            | ExecutionStatus::Optimistic(hash) => Some(*hash),
             ExecutionStatus::Irrelevant(_) => None,
         }
     }
 
     /// Returns `true` if the block:
     ///
-    /// - Has execution enabled
+    /// - Has a valid payload, OR
+    /// - Does not have execution enabled.
+    ///
+    /// Whenever this function returns `true`, the block is *fully valid*.
+    pub fn is_valid_or_irrelevant(&self) -> bool {
+        matches!(
+            self,
+            ExecutionStatus::Valid(_) | ExecutionStatus::Irrelevant(_)
+        )
+    }
+
+    /// Returns `true` if the block:
+    ///
+    /// - Has execution enabled, AND
     /// - Has a valid payload
-    pub fn is_valid(&self) -> bool {
+    ///
+    /// This function will return `false` for any block from a slot prior to the Bellatrix fork.
+    /// This means that some blocks that are perfectly valid will still receive a `false` response.
+    /// See `Self::is_valid_or_irrelevant` for a function that will always return `true` given any
+    /// perfectly valid block.
+    pub fn is_valid_and_post_bellatrix(&self) -> bool {
         matches!(self, ExecutionStatus::Valid(_))
     }
 
     /// Returns `true` if the block:
     ///
-    /// - Has execution enabled
+    /// - Has execution enabled, AND
     /// - Has a payload that has not yet been verified by an EL.
-    pub fn is_not_verified(&self) -> bool {
+    pub fn is_optimistic(&self) -> bool {
-        matches!(self, ExecutionStatus::Unknown(_))
+        matches!(self, ExecutionStatus::Optimistic(_))
     }
 
     /// Returns `true` if the block:
     ///
-    /// - Has execution enabled
+    /// - Has execution enabled, AND
     /// - Has an invalid payload.
     pub fn is_invalid(&self) -> bool {
         matches!(self, ExecutionStatus::Invalid(_))
@@ -294,9 +312,13 @@ impl ProtoArrayForkChoice {
         self.proto_array.indices.contains_key(block_root)
     }
 
-    pub fn get_block(&self, block_root: &Hash256) -> Option<Block> {
+    fn get_proto_node(&self, block_root: &Hash256) -> Option<&ProtoNode> {
         let block_index = self.proto_array.indices.get(block_root)?;
-        let block = self.proto_array.nodes.get(*block_index)?;
+        self.proto_array.nodes.get(*block_index)
+    }
+
+    pub fn get_block(&self, block_root: &Hash256) -> Option<Block> {
+        let block = self.get_proto_node(block_root)?;
         let parent_root = block
             .parent
             .and_then(|i| self.proto_array.nodes.get(i))
@@ -325,6 +347,12 @@ impl ProtoArrayForkChoice {
         }
     }
 
+    /// Returns the `block.execution_status` field, if the block is present.
+    pub fn get_block_execution_status(&self, block_root: &Hash256) -> Option<ExecutionStatus> {
+        let block = self.get_proto_node(block_root)?;
+        Some(block.execution_status)
+    }
+
     /// Returns the weight of a given block.
     pub fn get_weight(&self, block_root: &Hash256) -> Option<u64> {
         let block_index = self.proto_array.indices.get(block_root)?;