Merged Age's changes and ripped out heaps of now obsolete stuff in the validator client.

- Replaced most instances of PublicKey with KeyPair, since they need to be passed into each validator thread now.
 - Pulled out a bunch of FreeAttestations, and replaced with regular Attestations (as per Paul's suggestion)
 - Started generalising pubkeys to 'signers' (though they are still just Keypairs)
 - Added validator_index into a few structs where relevant
 - Removed the SlotClock and DutiesReader from the BlockProducer and Attester services, since this logic is now abstracted to the higher level process.
 - Added a Hash trait to the Keypair (rather than just pubkey) which assumes the Pubkey uniquely defines it.

validator_client/src/duties/epoch_duties.rs

@@ -1,90 +1,125 @@
-use block_proposer::{DutiesReader, DutiesReaderError};
 use std::collections::HashMap;
-use std::sync::RwLock;
-use types::{Epoch, Fork, Slot};
+use std::ops::{Deref, DerefMut};
+use types::{AttestationDuty, Epoch, Keypair, Slot};
+
+/// When work needs to be performed by a validator, this type is given back to the main service
+/// which indicates all the information that required to process the work.
+///
+/// Note: This is calculated per slot, so a validator knows which slot is related to this struct.
+#[derive(Debug, Clone)]
+pub struct WorkInfo {
+    /// Validator needs to produce a block.
+    pub produce_block: bool,
+    /// Validator needs to produce an attestation. This supplies the required attestation data.
+    pub attestation_duty: Option<AttestationDuty>,
+}
 
 /// The information required for a validator to propose and attest during some epoch.
 ///
 /// Generally obtained from a Beacon Node, this information contains the validators canonical index
-/// (thier sequence in the global validator induction process) and the "shuffling" for that index
+/// (their sequence in the global validator induction process) and the "shuffling" for that index
 /// for some epoch.
 #[derive(Debug, PartialEq, Clone, Copy, Default)]
-pub struct EpochDuties {
-    pub validator_index: u64,
+pub struct EpochDuty {
     pub block_production_slot: Option<Slot>,
-    // Future shard info
+    pub attestation_slot: Slot,
+    pub attestation_shard: u64,
+    pub committee_index: u64,
+    pub validator_index: u64,
 }
 
-impl EpochDuties {
-    /// Returns `true` if the supplied `slot` is a slot in which the validator should produce a
-    /// block.
-    pub fn is_block_production_slot(&self, slot: Slot) -> bool {
-        match self.block_production_slot {
+impl EpochDuty {
+    /// Returns `WorkInfo` if work needs to be done in the supplied `slot`
+    pub fn is_work_slot(&self, slot: Slot) -> Option<WorkInfo> {
+        // if validator is required to produce a slot return true
+        let produce_block = match self.block_production_slot {
             Some(s) if s == slot => true,
             _ => false,
+        };
+
+        // if the validator is required to attest to a shard, create the data
+        let mut attestation_duty = None;
+        if self.attestation_slot == slot {
+            attestation_duty = Some(AttestationDuty {
+                slot,
+                shard: self.attestation_shard,
+                committee_index: self.committee_index as usize,
+                validator_index: self.validator_index as usize
+            });
         }
+
+        if produce_block | attestation_duty.is_some() {
+            return Some(WorkInfo {
+                produce_block,
+                attestation_duty,
+            });
+        }
+        None
     }
 }
+/// Maps a list of Keypairs (many validators) to an EpochDuty.
+pub type EpochDuties = HashMap<Keypair, Option<EpochDuty>>;
 
 pub enum EpochDutiesMapError {
     Poisoned,
+    UnknownEpoch,
+    UnknownValidator,
 }
 
 /// Maps an `epoch` to some `EpochDuties` for a single validator.
 pub struct EpochDutiesMap {
     pub slots_per_epoch: u64,
-    pub map: RwLock<HashMap<Epoch, EpochDuties>>,
+    pub map: HashMap<Epoch, EpochDuties>,
 }
 
 impl EpochDutiesMap {
     pub fn new(slots_per_epoch: u64) -> Self {
         Self {
             slots_per_epoch,
-            map: RwLock::new(HashMap::new()),
+            map: HashMap::new(),
         }
     }
-
-    pub fn get(&self, epoch: Epoch) -> Result<Option<EpochDuties>, EpochDutiesMapError> {
-        let map = self.map.read().map_err(|_| EpochDutiesMapError::Poisoned)?;
-        match map.get(&epoch) {
-            Some(duties) => Ok(Some(*duties)),
-            None => Ok(None),
-        }
-    }
-
-    pub fn insert(
-        &self,
-        epoch: Epoch,
-        epoch_duties: EpochDuties,
-    ) -> Result<Option<EpochDuties>, EpochDutiesMapError> {
-        let mut map = self
-            .map
-            .write()
-            .map_err(|_| EpochDutiesMapError::Poisoned)?;
-        Ok(map.insert(epoch, epoch_duties))
-    }
 }
 
-impl DutiesReader for EpochDutiesMap {
-    fn is_block_production_slot(&self, slot: Slot) -> Result<bool, DutiesReaderError> {
+// Expose the hashmap methods
+impl Deref for EpochDutiesMap {
+    type Target = HashMap<Epoch, EpochDuties>;
+
+    fn deref(&self) -> &Self::Target {
+        &self.map
+    }
+}
+impl DerefMut for EpochDutiesMap {
+    fn deref_mut(&mut self) -> &mut HashMap<Epoch, EpochDuties> {
+        &mut self.map
+    }
+}
+
+impl EpochDutiesMap {
+    /// Checks if the validator has work to do.
+    pub fn is_work_slot(
+        &self,
+        slot: Slot,
+        signer: &Keypair,
+    ) -> Result<Option<WorkInfo>, EpochDutiesMapError> {
         let epoch = slot.epoch(self.slots_per_epoch);
 
-        let map = self.map.read().map_err(|_| DutiesReaderError::Poisoned)?;
-        let duties = map
+        let epoch_duties = self
+            .map
             .get(&epoch)
-            .ok_or_else(|| DutiesReaderError::UnknownEpoch)?;
-        Ok(duties.is_block_production_slot(slot))
-    }
-
-    fn fork(&self) -> Result<Fork, DutiesReaderError> {
-        // TODO: this is garbage data.
-        //
-        // It will almost certainly cause signatures to fail verification.
-        Ok(Fork {
-            previous_version: [0; 4],
-            current_version: [0; 4],
-            epoch: Epoch::new(0),
-        })
+            .ok_or_else(|| EpochDutiesMapError::UnknownEpoch)?;
+        if let Some(epoch_duty) = epoch_duties.get(signer) {
+            if let Some(duty) = epoch_duty {
+                // Retrieves the duty for a validator at a given slot
+                return Ok(duty.is_work_slot(slot));
+            } else {
+                // the validator isn't active
+                return Ok(None);
+            }
+        } else {
+            // validator isn't known
+            return Err(EpochDutiesMapError::UnknownValidator);
+        }
     }
 }
 

validator_client/src/duties/grpc.rs

@@ -1,54 +1,56 @@
+use super::epoch_duties::{EpochDuties, EpochDuty};
 use super::traits::{BeaconNode, BeaconNodeError};
-use super::EpochDuties;
-use protos::services::{ProposeBlockSlotRequest, PublicKey as IndexRequest};
+use protos::services::{GetDutiesRequest, Validators};
 use protos::services_grpc::ValidatorServiceClient;
 use ssz::ssz_encode;
-use types::{Epoch, PublicKey, Slot};
+use std::collections::HashMap;
+use types::{Epoch, Keypair, Slot};
 
 impl BeaconNode for ValidatorServiceClient {
-    /// Request the shuffling from the Beacon Node (BN).
-    ///
-    /// As this function takes a `PublicKey`, it will first attempt to resolve the public key into
-    /// a validator index, then call the BN for production/attestation duties.
-    ///
-    /// Note: presently only block production information is returned.
-    fn request_shuffling(
+    /// Requests all duties (block signing and committee attesting) from the Beacon Node (BN).
+    fn request_duties(
         &self,
         epoch: Epoch,
-        public_key: &PublicKey,
-    ) -> Result<Option<EpochDuties>, BeaconNodeError> {
-        // Lookup the validator index for the supplied public key.
-        let validator_index = {
-            let mut req = IndexRequest::new();
-            req.set_public_key(ssz_encode(public_key).to_vec());
-            let resp = self
-                .validator_index(&req)
-                .map_err(|err| BeaconNodeError::RemoteFailure(format!("{:?}", err)))?;
-            resp.get_index()
-        };
-
-        let mut req = ProposeBlockSlotRequest::new();
-        req.set_validator_index(validator_index);
+        signers: &[Keypair],
+    ) -> Result<EpochDuties, BeaconNodeError> {
+        // Get the required duties from all validators
+        // build the request
+        let mut req = GetDutiesRequest::new();
         req.set_epoch(epoch.as_u64());
+        let mut validators = Validators::new();
+        validators.set_public_keys(signers.iter().map(|v| ssz_encode(&v.pk)).collect());
+        req.set_validators(validators);
 
+        // send the request, get the duties reply
         let reply = self
-            .propose_block_slot(&req)
+            .get_validator_duties(&req)
             .map_err(|err| BeaconNodeError::RemoteFailure(format!("{:?}", err)))?;
 
-        let block_production_slot = if reply.has_slot() {
-            Some(reply.get_slot())
-        } else {
-            None
-        };
-
-        let block_production_slot = match block_production_slot {
-            Some(slot) => Some(Slot::new(slot)),
-            None => None,
-        };
-
-        Ok(Some(EpochDuties {
-            validator_index,
-            block_production_slot,
-        }))
+        let mut epoch_duties: HashMap<Keypair, Option<EpochDuty>> = HashMap::new();
+        for (index, validator_duty) in reply.get_active_validators().iter().enumerate() {
+            if !validator_duty.has_duty() {
+                // validator is inactive
+                epoch_duties.insert(signers[index].clone(), None);
+                break;
+            }
+            // active validator
+            let active_duty = validator_duty.get_duty();
+            let block_production_slot = {
+                if active_duty.has_block_production_slot() {
+                    Some(Slot::from(active_duty.get_block_production_slot()))
+                } else {
+                    None
+                }
+            };
+            let epoch_duty = EpochDuty {
+                block_production_slot,
+                attestation_slot: Slot::from(active_duty.get_attestation_slot()),
+                attestation_shard: active_duty.get_attestation_shard(),
+                committee_index: active_duty.get_committee_index(),
+                validator_index: active_duty.get_validator_index(),
+            };
+            epoch_duties.insert(signers[index].clone(), Some(epoch_duty));
+        }
+        Ok(epoch_duties)
     }
 }

validator_client/src/duties/mod.rs

@@ -1,18 +1,20 @@
 mod epoch_duties;
 mod grpc;
-#[cfg(test)]
-mod test_node;
+// TODO: reintroduce tests
+//#[cfg(test)]
+//mod test_node;
 mod traits;
 
-pub use self::epoch_duties::EpochDutiesMap;
 use self::epoch_duties::{EpochDuties, EpochDutiesMapError};
+pub use self::epoch_duties::{EpochDutiesMap, WorkInfo};
 use self::traits::{BeaconNode, BeaconNodeError};
-use bls::PublicKey;
-use slot_clock::SlotClock;
+use futures::Async;
+use slog::{debug, error, info};
 use std::sync::Arc;
-use types::{ChainSpec, Epoch, Slot};
+use std::sync::RwLock;
+use types::{Epoch, Keypair, Slot};
 
-#[derive(Debug, PartialEq, Clone, Copy)]
+#[derive(Debug, PartialEq, Clone)]
 pub enum UpdateOutcome {
     /// The `EpochDuties` were not updated during this poll.
     NoChange(Epoch),
@@ -21,76 +23,116 @@ pub enum UpdateOutcome {
     /// New `EpochDuties` were obtained, different to those which were previously known. This is
     /// likely to be the result of chain re-organisation.
     DutiesChanged(Epoch, EpochDuties),
-    /// The Beacon Node was unable to return the duties as the validator is unknown, or the
-    /// shuffling for the epoch is unknown.
-    UnknownValidatorOrEpoch(Epoch),
 }
 
 #[derive(Debug, PartialEq)]
 pub enum Error {
-    SlotClockError,
-    SlotUnknowable,
+    DutiesMapPoisoned,
     EpochMapPoisoned,
     BeaconNodeError(BeaconNodeError),
+    UnknownEpoch,
+    UnknownValidator,
 }
 
 /// A polling state machine which ensures the latest `EpochDuties` are obtained from the Beacon
 /// Node.
 ///
 /// This keeps track of all validator keys and required voting slots.
-pub struct DutiesManager<T: SlotClock, U: BeaconNode> {
-    pub duties_map: Arc<EpochDutiesMap>,
-    /// A list of all public keys known to the validator service.
-    pub pubkeys: Vec<PublicKey>,
-    pub spec: Arc<ChainSpec>,
-    pub slot_clock: Arc<T>,
+pub struct DutiesManager<U: BeaconNode> {
+    pub duties_map: RwLock<EpochDutiesMap>,
+    /// A list of all signer objects known to the validator service.
+    // TODO: Generalise the signers, so that they're not just keypairs
+    pub signers: Vec<Keypair>,
     pub beacon_node: Arc<U>,
 }
 
-impl<T: SlotClock, U: BeaconNode> DutiesManager<T, U> {
+impl<U: BeaconNode> DutiesManager<U> {
     /// Check the Beacon Node for `EpochDuties`.
     ///
-    /// The present `epoch` will be learned from the supplied `SlotClock`. In production this will
     /// be a wall-clock (e.g., system time, remote server time, etc.).
-    pub fn update(&self, slot: Slot) -> Result<UpdateOutcome, Error> {
-        let epoch = slot.epoch(self.spec.slots_per_epoch);
-
-        if let Some(duties) = self
-            .beacon_node
-            .request_shuffling(epoch, &self.pubkeys[0])?
-        {
-            // If these duties were known, check to see if they're updates or identical.
-            let result = if let Some(known_duties) = self.duties_map.get(epoch)? {
-                if known_duties == duties {
-                    Ok(UpdateOutcome::NoChange(epoch))
-                } else {
-                    Ok(UpdateOutcome::DutiesChanged(epoch, duties))
-                }
+    fn update(&self, epoch: Epoch) -> Result<UpdateOutcome, Error> {
+        let duties = self.beacon_node.request_duties(epoch, &self.signers)?;
+        // If these duties were known, check to see if they're updates or identical.
+        if let Some(known_duties) = self.duties_map.read()?.get(&epoch) {
+            if *known_duties == duties {
+                return Ok(UpdateOutcome::NoChange(epoch));
             } else {
-                Ok(UpdateOutcome::NewDuties(epoch, duties))
-            };
-            self.duties_map.insert(epoch, duties)?;
-            result
+                //TODO: Duties could be large here. Remove from display and avoid the clone.
+                self.duties_map.write()?.insert(epoch, duties.clone());
+                return Ok(UpdateOutcome::DutiesChanged(epoch, duties));
+            }
         } else {
-            Ok(UpdateOutcome::UnknownValidatorOrEpoch(epoch))
+            //TODO: Remove clone by removing duties from outcome
+            self.duties_map.write()?.insert(epoch, duties.clone());
+            return Ok(UpdateOutcome::NewDuties(epoch, duties));
+        };
+    }
+
+    /// A future wrapping around `update()`. This will perform logic based upon the update
+    /// process and complete once the update has completed.
+    pub fn run_update(&self, epoch: Epoch, log: slog::Logger) -> Result<Async<()>, ()> {
+        match self.update(epoch) {
+            Err(error) => error!(log, "Epoch duties poll error"; "error" => format!("{:?}", error)),
+            Ok(UpdateOutcome::NoChange(epoch)) => {
+                debug!(log, "No change in duties"; "epoch" => epoch)
+            }
+            Ok(UpdateOutcome::DutiesChanged(epoch, duties)) => {
+                info!(log, "Duties changed (potential re-org)"; "epoch" => epoch, "duties" => format!("{:?}", duties))
+            }
+            Ok(UpdateOutcome::NewDuties(epoch, duties)) => {
+                info!(log, "New duties obtained"; "epoch" => epoch, "duties" => format!("{:?}", duties))
+            }
+        };
+        Ok(Async::Ready(()))
+    }
+
+    /// Returns a list of (Public, WorkInfo) indicating all the validators that have work to perform
+    /// this slot.
+    pub fn get_current_work(&self, slot: Slot) -> Option<Vec<(Keypair, WorkInfo)>> {
+        let mut current_work: Vec<(Keypair, WorkInfo)> = Vec::new();
+
+        // if the map is poisoned, return None
+        let duties = self.duties_map.read().ok()?;
+
+        for validator_signer in &self.signers {
+            match duties.is_work_slot(slot, &validator_signer) {
+                Ok(Some(work_type)) => current_work.push((validator_signer.clone(), work_type)),
+                Ok(None) => {} // No work for this validator
+                //TODO: This should really log an error, as we shouldn't end up with an err here.
+                Err(_) => {}   // Unknown epoch or validator, no work
+            }
         }
+        if current_work.is_empty() {
+            return None;
+        }
+        Some(current_work)
     }
 }
 
+//TODO: Use error_chain to handle errors
 impl From<BeaconNodeError> for Error {
     fn from(e: BeaconNodeError) -> Error {
         Error::BeaconNodeError(e)
     }
 }
 
+//TODO: Use error_chain to handle errors
+impl<T> From<std::sync::PoisonError<T>> for Error {
+    fn from(_e: std::sync::PoisonError<T>) -> Error {
+        Error::DutiesMapPoisoned
+    }
+}
 impl From<EpochDutiesMapError> for Error {
     fn from(e: EpochDutiesMapError) -> Error {
         match e {
             EpochDutiesMapError::Poisoned => Error::EpochMapPoisoned,
+            EpochDutiesMapError::UnknownEpoch => Error::UnknownEpoch,
+            EpochDutiesMapError::UnknownValidator => Error::UnknownValidator,
         }
     }
 }
 
+/* TODO: Modify tests for new Duties Manager form
 #[cfg(test)]
 mod tests {
     use super::test_node::TestBeaconNode;
@@ -104,6 +146,7 @@ mod tests {
     //
     // These tests should serve as a good example for future tests.
 
+
     #[test]
     pub fn polling() {
         let spec = Arc::new(ChainSpec::foundation());
@@ -154,3 +197,4 @@ mod tests {
         );
     }
 }
+*/

validator_client/src/duties/traits.rs

@@ -1,6 +1,5 @@
 use super::EpochDuties;
-use bls::PublicKey;
-use types::Epoch;
+use types::{Epoch, Keypair};
 
 #[derive(Debug, PartialEq, Clone)]
 pub enum BeaconNodeError {
@@ -9,12 +8,13 @@ pub enum BeaconNodeError {
 
 /// Defines the methods required to obtain a validators shuffling from a Beacon Node.
 pub trait BeaconNode: Send + Sync {
-    /// Get the shuffling for the given epoch and public key.
+    /// Gets the duties for all validators.
     ///
-    /// Returns Ok(None) if the public key is unknown, or the shuffling for that epoch is unknown.
-    fn request_shuffling(
+    /// Returns a vector of EpochDuties for each validator public key. The entry will be None for
+    /// validators that are not activated.
+    fn request_duties(
         &self,
         epoch: Epoch,
-        public_key: &PublicKey,
-    ) -> Result<Option<EpochDuties>, BeaconNodeError>;
+        signers: &[Keypair],
+    ) -> Result<EpochDuties, BeaconNodeError>;
 }