Tidy, add comments

paulhauner · paulhauner · commit 28b94cb0703d · 2022-03-01T13:24:56.000+11:00
diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs
@@ -1118,6 +1118,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 .beacon_state
                 .proposer_shuffling_decision_root(head.beacon_block_root)?;
 
+            // The `random` value is used whilst producing an `ExecutionPayload` atop the head.
             let current_epoch = head.beacon_state.current_epoch();
             let random = *head.beacon_state.get_randao_mix(current_epoch)?;
 
@@ -3700,12 +3701,12 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                     );
                 }
 
-                // This performing this call immediately after
+                // Performing this call immediately after
                 // `update_execution_engine_forkchoice_blocking` might result in two calls to fork
-                // choice, one *without* payload attributes and then a second *with* payload
-                // attributes.
+                // choice updated, one *without* payload attributes and then a second *with*
+                // payload attributes.
                 //
-                // This seems OK, it's not a significant waste of EL<>CL bandwidth or resources, as
+                // This seems OK. It's not a significant waste of EL<>CL bandwidth or resources, as
                 // far as I know.
                 if let Err(e) = self.prepare_beacon_proposer_blocking() {
                     crit!(
@@ -3751,7 +3752,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
 
         // Nothing to do if there are no proposers registered with the EL, exit early to avoid
         // wasting cycles.
-        if !execution_layer.has_proposers().await {
+        if !execution_layer.has_any_proposer_preparation_data().await {
             return Ok(());
         }
 
diff --git a/beacon_node/beacon_chain/src/proposer_prep_service.rs b/beacon_node/beacon_chain/src/proposer_prep_service.rs
@@ -1,12 +1,20 @@
 use crate::{BeaconChain, BeaconChainTypes};
-use slog::error;
+use slog::{debug, error};
 use slot_clock::SlotClock;
 use std::sync::Arc;
 use task_executor::TaskExecutor;
 use tokio::time::sleep;
 
+/// At 12s slot times, the means that the payload preparation routine will run 4s before the start
+/// of each slot (`12 / 3 = 4`).
 pub const PAYLOAD_PREPARATION_LOOKAHEAD_FACTOR: u32 = 3;
 
+/// Spawns a routine which ensures the EL is provided advance notice of any block producers.
+///
+/// This routine will run once per slot, at `slot_duration / PAYLOAD_PREPARATION_LOOKAHEAD_FACTOR`
+/// before the start of each slot.
+///
+/// The service will not be started if there is no `execution_layer` on the `chain`.
 pub fn start_proposer_prep_service<T: BeaconChainTypes>(
     executor: &TaskExecutor,
     chain: Arc<BeaconChain<T>>,
@@ -20,6 +28,7 @@ pub fn start_proposer_prep_service<T: BeaconChainTypes>(
     }
 }
 
+/// Loop indefinitely, calling `BeaconChain::prepare_beacon_proposer_async` at an interval.
 async fn proposer_prep_service<T: BeaconChainTypes>(chain: Arc<BeaconChain<T>>) {
     let slot_duration = chain.slot_clock.slot_duration();
 
@@ -30,6 +39,11 @@ async fn proposer_prep_service<T: BeaconChainTypes>(chain: Arc<BeaconChain<T>>)
                     - chain.slot_clock.slot_duration() / PAYLOAD_PREPARATION_LOOKAHEAD_FACTOR;
                 sleep(duration + additional_delay).await;
 
+                debug!(
+                    chain.log,
+                    "Proposer prepare routine firing";
+                );
+
                 if let Err(e) = chain.prepare_beacon_proposer_async().await {
                     error!(
                         chain.log,
diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs
@@ -335,15 +335,15 @@ impl ExecutionLayer {
         self.block_on_generic(|_| async move {
             self.update_proposer_preparation(update_epoch, preparation_data)
                 .await
-        })?
+        })
     }
 
     /// Updates the proposer preparation data provided by validators
     async fn update_proposer_preparation(
         &self,
         update_epoch: Epoch,
         preparation_data: &[ProposerPreparationData],
-    ) -> Result<(), Error> {
+    ) {
         let mut proposer_preparation_data = self.proposer_preparation_data().await;
         for preparation_entry in preparation_data {
             proposer_preparation_data.insert(
@@ -354,8 +354,6 @@ impl ExecutionLayer {
                 },
             );
         }
-
-        Ok(())
     }
 
     /// Removes expired entries from cached proposer preparations
@@ -371,10 +369,14 @@ impl ExecutionLayer {
         Ok(())
     }
 
-    pub async fn has_proposers(&self) -> bool {
+    /// Returns `true` if there have been any validators registered via
+    /// `Self::update_proposer_preparation`.
+    pub async fn has_any_proposer_preparation_data(&self) -> bool {
         !self.proposer_preparation_data().await.is_empty()
     }
 
+    /// Returns `true` if the `proposer_index` has registered as a local validator via
+    /// `Self::update_proposer_preparation`.
     pub async fn has_proposer_preparation_data(&self, proposer_index: u64) -> bool {
         self.proposer_preparation_data()
             .await
@@ -523,6 +525,10 @@ impl ExecutionLayer {
         )
     }
 
+    /// Register that the given `validator_index` is going to produce a block at `slot`.
+    ///
+    /// The block will be build atop `head_block_root` and the EL will need to prepare an
+    /// `ExecutionPayload` as defined by the given `payload_attributes`.
     pub async fn insert_proposer(
         &self,
         slot: Slot,
@@ -548,6 +554,9 @@ impl ExecutionLayer {
             .is_some()
     }
 
+    /// If there has been a proposer registered via `Self::insert_proposer` with a matching `slot`
+    /// `head_block_root`, then return the appropriate `PayloadAttributes` for inclusion in
+    /// `forkchoiceUpdated` calls.
     pub async fn payload_attributes(
         &self,
         current_slot: Slot,
