From ed2f0b797c9ae7ac1229268b8d8067a8e4848f95 Mon Sep 17 00:00:00 2001
From: Justin <drakefjustin@gmail.com>
Date: Mon, 18 May 2020 03:59:03 +0100
Subject: [PATCH] Cleanup `ExitCache` (#1091)

* Cleanup `ExitCache`

Minor suggested cleanups after familiarising myself with the `ExitCache`.

* Remove "validators exiting/exited at a given epoch" comment in favour of the notion of exit epoch (less wishy-washy).
* Remove "or zero if not known" comment. The number of validators with that exit epoch is known, even in the case where it's zero.
* Rename `epoch` to `exit_epoch` for consistency and clarity.
* Rename `exits_per_epoch` to `exit_epoch_counts` for precision and clarity.
* Remove seemingly unnecessary complexity with `force_build`.
* Consider renaming `ExitCache` to `ExitEpochCache` for clarity.

* Update exit_cache.rs
---
 eth2/types/src/beacon_state/exit_cache.rs | 36 ++++++++---------------
 1 file changed, 13 insertions(+), 23 deletions(-)

diff --git a/eth2/types/src/beacon_state/exit_cache.rs b/eth2/types/src/beacon_state/exit_cache.rs
index 0fc2e05754..ed8cc6541d 100644
--- a/eth2/types/src/beacon_state/exit_cache.rs
+++ b/eth2/types/src/beacon_state/exit_cache.rs
@@ -3,41 +3,33 @@ use safe_arith::SafeArith;
 use serde_derive::{Deserialize, Serialize};
 use std::collections::HashMap;
 
-/// Map from exit epoch to the number of validators known to be exiting/exited at that epoch.
+/// Map from exit epoch to the number of validators with that exit epoch.
 #[derive(Debug, Default, Clone, PartialEq, Serialize, Deserialize)]
 pub struct ExitCache {
     initialized: bool,
-    exits_per_epoch: HashMap<Epoch, u64>,
+    exit_epoch_counts: HashMap<Epoch, u64>,
 }
 
 impl ExitCache {
-    /// Ensure the cache is built, and do nothing if it's already initialized.
+    /// Build the cache if not initialized.
     pub fn build(
         &mut self,
         validators: &[Validator],
         spec: &ChainSpec,
     ) -> Result<(), BeaconStateError> {
         if self.initialized {
-            Ok(())
-        } else {
-            self.force_build(validators, spec)
+            return Ok(())
         }
-    }
 
-    /// Add all validators with a non-trivial exit epoch to the cache.
-    pub fn force_build(
-        &mut self,
-        validators: &[Validator],
-        spec: &ChainSpec,
-    ) -> Result<(), BeaconStateError> {
         self.initialized = true;
+        // Add all validators with a non-default exit epoch to the cache.
         validators
             .iter()
             .filter(|validator| validator.exit_epoch != spec.far_future_epoch)
             .try_for_each(|validator| self.record_validator_exit(validator.exit_epoch))
     }
 
-    /// Check that the cache is initialized and return an error if it isn't.
+    /// Check that the cache is initialized and return an error if it is not.
     pub fn check_initialized(&self) -> Result<(), BeaconStateError> {
         if self.initialized {
             Ok(())
@@ -46,28 +38,26 @@ impl ExitCache {
         }
     }
 
-    /// Record the exit of a single validator in the cache.
-    ///
-    /// Must only be called once per exiting validator.
+    /// Record the exit epoch of a validator. Must be called only once per exiting validator.
     pub fn record_validator_exit(&mut self, exit_epoch: Epoch) -> Result<(), BeaconStateError> {
         self.check_initialized()?;
-        self.exits_per_epoch
+        self.exit_epoch_counts
             .entry(exit_epoch)
             .or_insert(0)
             .increment()?;
         Ok(())
     }
 
-    /// Get the greatest epoch for which validator exits are known.
+    /// Get the largest exit epoch with a non-zero exit epoch count.
     pub fn max_epoch(&self) -> Result<Option<Epoch>, BeaconStateError> {
         self.check_initialized()?;
-        Ok(self.exits_per_epoch.keys().max().cloned())
+        Ok(self.exit_epoch_counts.keys().max().cloned())
     }
 
-    /// Get the number of validators exiting/exited at a given epoch, or zero if not known.
-    pub fn get_churn_at(&self, epoch: Epoch) -> Result<u64, BeaconStateError> {
+    /// Get number of validators with the given exit epoch. (Return 0 for the default exit epoch.)
+    pub fn get_churn_at(&self, exit_epoch: Epoch) -> Result<u64, BeaconStateError> {
         self.check_initialized()?;
-        Ok(self.exits_per_epoch.get(&epoch).cloned().unwrap_or(0))
+        Ok(self.exit_epoch_counts.get(&exit_epoch).cloned().unwrap_or(0))
     }
 }