Add comments, more validity checks

validator_client/src/block_service.rs

@@ -82,6 +82,7 @@ impl<T: SlotClock + 'static, E: EthSpec> BlockServiceBuilder<T, E> {
     }
 }
 
+/// Helper to minimise `Arc` usage.
 pub struct Inner<T, E: EthSpec> {
     duties_service: DutiesService<T, E>,
     validator_store: ValidatorStore<T, E>,
@@ -90,6 +91,7 @@ pub struct Inner<T, E: EthSpec> {
     context: RuntimeContext<E>,
 }
 
+/// Attempts to produce attestations for any block producer(s) at the start of the epoch.
 pub struct BlockService<T, E: EthSpec> {
     inner: Arc<Inner<T, E>>,
 }
@@ -111,6 +113,7 @@ impl<T, E: EthSpec> Deref for BlockService<T, E> {
 }
 
 impl<T: SlotClock + 'static, E: EthSpec> BlockService<T, E> {
+    /// Starts the service that periodically attempts to produce blocks.
     pub fn start_update_service(&self, spec: &ChainSpec) -> Result<Signal, String> {
         let log = self.context.log.clone();
 
@@ -153,6 +156,7 @@ impl<T: SlotClock + 'static, E: EthSpec> BlockService<T, E> {
         Ok(exit_signal)
     }
 
+    /// Attempt to produce a block for any block producers in the `ValidatorStore`.
     fn do_update(self) -> impl Future<Item = (), Error = ()> {
         let service = self.clone();
         let log = self.context.log.clone();

validator_client/src/duties_service.rs

@@ -22,9 +22,15 @@ const PRUNE_DEPTH: u64 = 4;
 type BaseHashMap = HashMap<PublicKey, HashMap<Epoch, ValidatorDuty>>;
 
 enum InsertOutcome {
+    /// The duties were previously unknown and have been stored.
     New,
+    /// The duties were identical to some already in the store.
     Identical,
+    /// There were duties for this validator and epoch in the store that were different to the ones
+    /// provided. The existing duties were replaced.
     Replaced,
+    /// The given duties were invalid.
+    Invalid,
 }
 
 #[derive(Default)]
@@ -74,9 +80,13 @@ impl DutiesStore {
             .collect()
     }
 
-    fn insert(&self, epoch: Epoch, duties: ValidatorDuty) -> InsertOutcome {
+    fn insert(&self, epoch: Epoch, duties: ValidatorDuty, slots_per_epoch: u64) -> InsertOutcome {
         let mut store = self.store.write();
 
+        if !duties_match_epoch(&duties, epoch, slots_per_epoch) {
+            return InsertOutcome::Invalid;
+        }
+
         if store.contains_key(&duties.validator_pubkey) {
             let validator_map = store.get_mut(&duties.validator_pubkey).expect(
                 "Store is exclusively locked and this path is guarded to ensure the key exists.",
@@ -340,15 +350,26 @@ impl<T: SlotClock + 'static, E: EthSpec> DutiesService<T, E> {
                 let mut new = 0;
                 let mut identical = 0;
                 let mut replaced = 0;
+                let mut invalid = 0;
 
                 all_duties.into_iter().for_each(|duties| {
-                    match service_2.store.insert(epoch, duties) {
+                    match service_2.store.insert(epoch, duties, E::slots_per_epoch()) {
                         InsertOutcome::New => new += 1,
                         InsertOutcome::Identical => identical += 1,
                         InsertOutcome::Replaced => replaced += 1,
+                        InsertOutcome::Invalid => invalid += 1,
                     };
                 });
 
+                if invalid > 0 {
+                    error!(
+                        service_2.context.log,
+                        "Received invalid duties from beacon node";
+                        "bad_duty_count" => invalid,
+                        "info" => "Duties are from wrong epoch."
+                    )
+                }
+
                 trace!(
                     service_2.context.log,
                     "Performed duties update";
@@ -368,3 +389,20 @@ impl<T: SlotClock + 'static, E: EthSpec> DutiesService<T, E> {
             })
     }
 }
+
+/// Returns `true` if the slots in the `duties` are from the given `epoch`
+fn duties_match_epoch(duties: &ValidatorDuty, epoch: Epoch, slots_per_epoch: u64) -> bool {
+    if let Some(attestation_slot) = duties.attestation_slot {
+        if attestation_slot.epoch(slots_per_epoch) != epoch {
+            return false;
+        }
+    }
+
+    if let Some(block_proposal_slot) = duties.block_proposal_slot {
+        if block_proposal_slot.epoch(slots_per_epoch) != epoch {
+            return false;
+        }
+    }
+
+    true
+}

validator_client/src/fork_service.rs

@@ -14,6 +14,7 @@ use types::{ChainSpec, EthSpec, Fork};
 /// Delay this period of time after the slot starts. This allows the node to process the new slot.
 const TIME_DELAY_FROM_SLOT: Duration = Duration::from_millis(80);
 
+/// Builds a `ForkService`.
 pub struct ForkServiceBuilder<T, E: EthSpec> {
     fork: Option<Fork>,
     slot_clock: Option<T>,
@@ -64,6 +65,7 @@ impl<T: SlotClock + 'static, E: EthSpec> ForkServiceBuilder<T, E> {
     }
 }
 
+/// Helper to minimise `Arc` usage.
 pub struct Inner<T, E: EthSpec> {
     fork: RwLock<Option<Fork>>,
     beacon_node: RemoteBeaconNode<E>,
@@ -71,6 +73,7 @@ pub struct Inner<T, E: EthSpec> {
     slot_clock: T,
 }
 
+/// Attempts to download the `Fork` struct from the beacon node at the start of each epoch.
 pub struct ForkService<T, E: EthSpec> {
     inner: Arc<Inner<T, E>>,
 }
@@ -92,10 +95,12 @@ impl<T, E: EthSpec> Deref for ForkService<T, E> {
 }
 
 impl<T: SlotClock + 'static, E: EthSpec> ForkService<T, E> {
+    /// Returns the last fork downloaded from the beacon node, if any.
     pub fn fork(&self) -> Option<Fork> {
         self.fork.read().clone()
     }
 
+    /// Starts the service that periodically polls for the `Fork`.
     pub fn start_update_service(&self, spec: &ChainSpec) -> Result<Signal, String> {
         let log = self.context.log.clone();
 
@@ -141,6 +146,7 @@ impl<T: SlotClock + 'static, E: EthSpec> ForkService<T, E> {
         Ok(exit_signal)
     }
 
+    /// Attempts to download the `Fork` from the server.
     fn do_update(&self) -> impl Future<Item = (), Error = ()> {
         let service_1 = self.clone();
         let log_1 = service_1.context.log.clone();