From 4d062d77f92e5ec67a4082f0dc7ce5930ea0c7e4 Mon Sep 17 00:00:00 2001
From: Paul Hauner <paul@paulhauner.com>
Date: Fri, 1 Feb 2019 16:55:37 +1100
Subject: [PATCH] Move `CheckPoint` into file, tidy canonical_head

---
 .../beacon_chain/src/canonical_head.rs        |  7 ++++
 beacon_node/beacon_chain/src/checkpoint.rs    | 41 +++++++++++++++++++
 beacon_node/beacon_chain/src/lib.rs           | 40 ++----------------
 beacon_node/beacon_chain/src/state.rs         |  4 ++
 4 files changed, 55 insertions(+), 37 deletions(-)
 create mode 100644 beacon_node/beacon_chain/src/checkpoint.rs

diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs
index 35262961e..32f2dc000 100644
--- a/beacon_node/beacon_chain/src/canonical_head.rs
+++ b/beacon_node/beacon_chain/src/canonical_head.rs
@@ -12,6 +12,7 @@ where
     T: ClientDB,
     U: SlotClock,
 {
+    /// Update the canonical head to some new values.
     pub fn update_canonical_head(
         &self,
         new_beacon_block: BeaconBlock,
@@ -28,6 +29,12 @@ where
         );
     }
 
+    /// Returns a read-lock guarded `CheckPoint` struct for reading the head (as chosen by the
+    /// fork-choice rule).
+    ///
+    /// It is important to note that the `beacon_state` returned may not match the present slot. It
+    /// is the state as it was when the head block was recieved, which could be some slots prior to
+    /// now.
     pub fn head(&self) -> RwLockReadGuard<CheckPoint> {
         self.canonical_head.read()
     }
diff --git a/beacon_node/beacon_chain/src/checkpoint.rs b/beacon_node/beacon_chain/src/checkpoint.rs
new file mode 100644
index 000000000..b2e0f9baa
--- /dev/null
+++ b/beacon_node/beacon_chain/src/checkpoint.rs
@@ -0,0 +1,41 @@
+use types::{BeaconBlock, BeaconState, Hash256};
+
+/// Represents some block and it's associated state. Generally, this will be used for tracking the
+/// head, justified head and finalized head.
+pub struct CheckPoint {
+    pub beacon_block: BeaconBlock,
+    pub beacon_block_root: Hash256,
+    pub beacon_state: BeaconState,
+    pub beacon_state_root: Hash256,
+}
+
+impl CheckPoint {
+    /// Create a new checkpoint.
+    pub fn new(
+        beacon_block: BeaconBlock,
+        beacon_block_root: Hash256,
+        beacon_state: BeaconState,
+        beacon_state_root: Hash256,
+    ) -> Self {
+        Self {
+            beacon_block,
+            beacon_block_root,
+            beacon_state,
+            beacon_state_root,
+        }
+    }
+
+    /// Update all fields of the checkpoint.
+    pub fn update(
+        &mut self,
+        beacon_block: BeaconBlock,
+        beacon_block_root: Hash256,
+        beacon_state: BeaconState,
+        beacon_state_root: Hash256,
+    ) {
+        self.beacon_block = beacon_block;
+        self.beacon_block_root = beacon_block_root;
+        self.beacon_state = beacon_state;
+        self.beacon_state_root = beacon_state_root;
+    }
+}
diff --git a/beacon_node/beacon_chain/src/lib.rs b/beacon_node/beacon_chain/src/lib.rs
index 314f18dc9..ddbb2732c 100644
--- a/beacon_node/beacon_chain/src/lib.rs
+++ b/beacon_node/beacon_chain/src/lib.rs
@@ -6,6 +6,7 @@ mod block_graph;
 pub mod block_processing;
 pub mod block_production;
 mod canonical_head;
+mod checkpoint;
 pub mod dump;
 mod finalized_head;
 mod info;
@@ -14,6 +15,7 @@ mod state;
 
 use self::attestation_targets::AttestationTargets;
 use self::block_graph::BlockGraph;
+use self::checkpoint::CheckPoint;
 use attestation_aggregator::AttestationAggregator;
 use db::{
     stores::{BeaconBlockStore, BeaconStateStore},
@@ -24,7 +26,7 @@ use parking_lot::RwLock;
 use slot_clock::SlotClock;
 use ssz::ssz_encode;
 use std::sync::Arc;
-use types::{BeaconBlock, BeaconState, ChainSpec, Hash256};
+use types::{BeaconState, ChainSpec, Hash256};
 
 pub use self::block_processing::Outcome as BlockProcessingOutcome;
 
@@ -35,42 +37,6 @@ pub enum BeaconChainError {
     DBError(String),
 }
 
-pub struct CheckPoint {
-    pub beacon_block: BeaconBlock,
-    pub beacon_block_root: Hash256,
-    pub beacon_state: BeaconState,
-    pub beacon_state_root: Hash256,
-}
-
-impl CheckPoint {
-    pub fn new(
-        beacon_block: BeaconBlock,
-        beacon_block_root: Hash256,
-        beacon_state: BeaconState,
-        beacon_state_root: Hash256,
-    ) -> Self {
-        Self {
-            beacon_block,
-            beacon_block_root,
-            beacon_state,
-            beacon_state_root,
-        }
-    }
-
-    pub fn update(
-        &mut self,
-        beacon_block: BeaconBlock,
-        beacon_block_root: Hash256,
-        beacon_state: BeaconState,
-        beacon_state_root: Hash256,
-    ) {
-        self.beacon_block = beacon_block;
-        self.beacon_block_root = beacon_block_root;
-        self.beacon_state = beacon_state;
-        self.beacon_state_root = beacon_state_root;
-    }
-}
-
 pub struct BeaconChain<T: ClientDB + Sized, U: SlotClock> {
     pub block_store: Arc<BeaconBlockStore<T>>,
     pub state_store: Arc<BeaconStateStore<T>>,
diff --git a/beacon_node/beacon_chain/src/state.rs b/beacon_node/beacon_chain/src/state.rs
index 33e50ce35..db1119c8a 100644
--- a/beacon_node/beacon_chain/src/state.rs
+++ b/beacon_node/beacon_chain/src/state.rs
@@ -12,6 +12,10 @@ where
     ///
     /// The `previous_block_root` will be set to the root of the current head block (as determined
     /// by the fork-choice rule).
+    ///
+    /// It is important to note that this is _not_ the state corresponding to the canonical head
+    /// block, instead it is that state which may or may not have had additional per slot/epoch
+    /// processing applied to it.
     pub fn advance_state(&self, slot: u64) -> Result<(), SlotProcessingError> {
         let state_slot = self.state.read().slot;
         let head_block_root = self.head().beacon_block_root;