Merge remote-tracking branch 'origin/unstable' into tree-states

consensus/fork_choice/Cargo.toml

@@ -2,7 +2,7 @@
 name = "fork_choice"
 version = "0.1.0"
 authors = ["Paul Hauner <paul@paulhauner.com>"]
-edition = "2018"
+edition = "2021"
 
 # See more keys and their definitions at https://doc.rust-lang.org/cargo/reference/manifest.html
 

consensus/fork_choice/src/fork_choice.rs

@@ -6,8 +6,8 @@ use std::marker::PhantomData;
 use std::time::Duration;
 use types::{
     consts::merge::INTERVALS_PER_SLOT, AttestationShufflingId, BeaconBlock, BeaconState,
-    BeaconStateError, ChainSpec, Checkpoint, Epoch, EthSpec, Hash256, IndexedAttestation,
-    RelativeEpoch, SignedBeaconBlock, Slot,
+    BeaconStateError, ChainSpec, Checkpoint, Epoch, EthSpec, ExecutionBlockHash, Hash256,
+    IndexedAttestation, RelativeEpoch, SignedBeaconBlock, Slot,
 };
 
 #[derive(Debug)]
@@ -17,6 +17,7 @@ pub enum Error<T> {
     ProtoArrayError(String),
     InvalidProtoArrayBytes(String),
     InvalidLegacyProtoArrayBytes(String),
+    FailedToProcessInvalidExecutionPayload(String),
     MissingProtoArrayBlock(Hash256),
     UnknownAncestor {
         ancestor_slot: Slot,
@@ -43,6 +44,12 @@ pub enum Error<T> {
         block_root: Hash256,
         payload_verification_status: PayloadVerificationStatus,
     },
+    MissingJustifiedBlock {
+        justified_checkpoint: Checkpoint,
+    },
+    MissingFinalizedBlock {
+        finalized_checkpoint: Checkpoint,
+    },
 }
 
 impl<T> From<InvalidAttestation> for Error<T> {
@@ -219,7 +226,7 @@ fn dequeue_attestations(
         queued_attestations
             .iter()
             .position(|a| a.slot >= current_slot)
-            .unwrap_or_else(|| queued_attestations.len()),
+            .unwrap_or(queued_attestations.len()),
     );
 
     std::mem::replace(queued_attestations, remaining)
@@ -299,9 +306,15 @@ where
         let execution_status = anchor_block.message_merge().map_or_else(
             |()| ExecutionStatus::irrelevant(),
             |message| {
-                // Assume that this payload is valid, since the anchor should be a trusted block and
-                // state.
-                ExecutionStatus::Valid(message.body.execution_payload.block_hash)
+                let execution_payload = &message.body.execution_payload;
+                if execution_payload == &<_>::default() {
+                    // A default payload does not have execution enabled.
+                    ExecutionStatus::irrelevant()
+                } else {
+                    // Assume that this payload is valid, since the anchor should be a trusted block and
+                    // state.
+                    ExecutionStatus::Valid(message.body.execution_payload.block_hash)
+                }
             },
         );
 
@@ -467,6 +480,17 @@ where
         Ok(true)
     }
 
+    /// See `ProtoArrayForkChoice::process_execution_payload_invalidation` for documentation.
+    pub fn on_invalid_execution_payload(
+        &mut self,
+        head_block_root: Hash256,
+        latest_valid_ancestor_root: Option<ExecutionBlockHash>,
+    ) -> Result<(), Error<T::Error>> {
+        self.proto_array
+            .process_execution_payload_invalidation(head_block_root, latest_valid_ancestor_root)
+            .map_err(Error::FailedToProcessInvalidExecutionPayload)
+    }
+
     /// Add `block` to the fork choice DAG.
     ///
     /// - `block_root` is the root of `block.
@@ -595,7 +619,7 @@ where
         let execution_status = if let Ok(execution_payload) = block.body().execution_payload() {
             let block_hash = execution_payload.block_hash;
 
-            if block_hash == Hash256::zero() {
+            if block_hash == ExecutionBlockHash::zero() {
                 // The block is post-merge-fork, but pre-terminal-PoW block. We don't need to verify
                 // the payload.
                 ExecutionStatus::irrelevant()
@@ -878,6 +902,29 @@ where
         }
     }
 
+    /// Returns the `ProtoBlock` for the justified checkpoint.
+    ///
+    /// ## Notes
+    ///
+    /// This does *not* return the "best justified checkpoint". It returns the justified checkpoint
+    /// that is used for computing balances.
+    pub fn get_justified_block(&self) -> Result<ProtoBlock, Error<T::Error>> {
+        let justified_checkpoint = self.justified_checkpoint();
+        self.get_block(&justified_checkpoint.root)
+            .ok_or(Error::MissingJustifiedBlock {
+                justified_checkpoint,
+            })
+    }
+
+    /// Returns the `ProtoBlock` for the finalized checkpoint.
+    pub fn get_finalized_block(&self) -> Result<ProtoBlock, Error<T::Error>> {
+        let finalized_checkpoint = self.finalized_checkpoint();
+        self.get_block(&finalized_checkpoint.root)
+            .ok_or(Error::MissingFinalizedBlock {
+                finalized_checkpoint,
+            })
+    }
+
     /// Return `true` if `block_root` is equal to the finalized root, or a known descendant of it.
     pub fn is_descendant_of_finalized(&self, block_root: Hash256) -> bool {
         self.proto_array

consensus/fork_choice/tests/tests.rs

@@ -122,18 +122,24 @@ impl ForkChoiceTest {
     }
 
     /// Assert there was a shutdown signal sent by the beacon chain.
-    pub fn assert_shutdown_signal_sent(mut self) -> Self {
-        self.harness.shutdown_receiver.close();
-        let msg = self.harness.shutdown_receiver.try_next().unwrap();
-        assert!(msg.is_some());
+    pub fn shutdown_signal_sent(&self) -> bool {
+        let mutex = self.harness.shutdown_receiver.clone();
+        let mut shutdown_receiver = mutex.lock();
+
+        shutdown_receiver.close();
+        let msg = shutdown_receiver.try_next().unwrap();
+        msg.is_some()
+    }
+
+    /// Assert there was a shutdown signal sent by the beacon chain.
+    pub fn assert_shutdown_signal_sent(self) -> Self {
+        assert!(self.shutdown_signal_sent());
         self
     }
 
     /// Assert no shutdown was signal sent by the beacon chain.
-    pub fn assert_shutdown_signal_not_sent(mut self) -> Self {
-        self.harness.shutdown_receiver.close();
-        let msg = self.harness.shutdown_receiver.try_next().unwrap();
-        assert!(msg.is_none());
+    pub fn assert_shutdown_signal_not_sent(self) -> Self {
+        assert!(!self.shutdown_signal_sent());
         self
     }
 
@@ -479,6 +485,22 @@ fn is_safe_to_update(slot: Slot, spec: &ChainSpec) -> bool {
     slot % E::slots_per_epoch() < spec.safe_slots_to_update_justified
 }
 
+#[test]
+fn justified_and_finalized_blocks() {
+    let tester = ForkChoiceTest::new();
+    let fork_choice = tester.harness.chain.fork_choice.read();
+
+    let justified_checkpoint = fork_choice.justified_checkpoint();
+    assert_eq!(justified_checkpoint.epoch, 0);
+    assert!(justified_checkpoint.root != Hash256::zero());
+    assert!(fork_choice.get_justified_block().is_ok());
+
+    let finalized_checkpoint = fork_choice.finalized_checkpoint();
+    assert_eq!(finalized_checkpoint.epoch, 0);
+    assert!(finalized_checkpoint.root != Hash256::zero());
+    assert!(fork_choice.get_finalized_block().is_ok());
+}
+
 /// - The new justified checkpoint descends from the current.
 /// - Current slot is within `SAFE_SLOTS_TO_UPDATE_JUSTIFIED`
 #[test]
@@ -613,7 +635,7 @@ fn justified_balances() {
 }
 
 macro_rules! assert_invalid_block {
-    ($err: tt, $($error: pat) |+ $( if $guard: expr )?) => {
+    ($err: tt, $($error: pat_param) |+ $( if $guard: expr )?) => {
         assert!(
             matches!(
                 $err,
@@ -719,7 +741,7 @@ fn invalid_block_finalized_descendant() {
 }
 
 macro_rules! assert_invalid_attestation {
-    ($err: tt, $($error: pat) |+ $( if $guard: expr )?) => {
+    ($err: tt, $($error: pat_param) |+ $( if $guard: expr )?) => {
         assert!(
             matches!(
                 $err,