diff --git a/beacon_node/rest_api/src/router.rs b/beacon_node/rest_api/src/router.rs index db488ef0f..783fd3eff 100644 --- a/beacon_node/rest_api/src/router.rs +++ b/beacon_node/rest_api/src/router.rs @@ -122,12 +122,7 @@ pub fn route( into_boxfut(response) } (&Method::POST, "/validator/subscribe") => { - validator::post_validator_subscriptions::( - req, - beacon_chain, - network_channel, - log, - ) + validator::post_validator_subscriptions::(req, network_channel) } (&Method::GET, "/validator/duties/all") => { into_boxfut(validator::get_all_validator_duties::(req, beacon_chain)) diff --git a/beacon_node/rest_api/src/validator.rs b/beacon_node/rest_api/src/validator.rs index d8308f944..af04bbd96 100644 --- a/beacon_node/rest_api/src/validator.rs +++ b/beacon_node/rest_api/src/validator.rs @@ -59,9 +59,7 @@ pub fn post_validator_duties( /// organise peer discovery and topic subscription for known validators. pub fn post_validator_subscriptions( req: Request, - beacon_chain: Arc>, mut network_chan: NetworkChannel, - log: Logger, ) -> BoxFut { try_future!(check_content_type_for_json(&req)); let response_builder = ResponseBuilder::new(&req); @@ -79,41 +77,6 @@ pub fn post_validator_subscriptions( }) }) .and_then(move |subscriptions: Vec| { - let fork = beacon_chain - .wall_clock_state() - .map(|state| state.fork.clone()) - .map_err(|e| { - error!(log, "Unable to get current beacon state"); - ApiError::ServerError(format!("Error getting current beacon state {:?}", e)) - })?; - - // verify the signatures in parallel - subscriptions.par_iter().try_for_each(|subscription| { - if let Some(pubkey) = - &beacon_chain.validator_pubkey(subscription.validator_index as usize)? - { - if subscription.verify( - pubkey, - &beacon_chain.spec, - &fork, - T::EthSpec::slots_per_epoch(), - ) { - Ok(()) - } else { - error!(log, "HTTP RPC sent invalid signatures"); - Err(ApiError::ProcessingError(format!( - "Could not verify signatures" - ))) - } - } else { - error!(log, "HTTP RPC sent unknown validator"); - Err(ApiError::ProcessingError(format!( - "Could not verify signatures" - ))) - } - })?; - - // subscriptions are verified, send them to the network thread network_chan .try_send(NetworkMessage::Subscribe { subscriptions }) .map_err(|e| { diff --git a/eth2/types/src/aggregate_and_proof.rs b/eth2/types/src/aggregate_and_proof.rs index e67bc5ffc..a71aa17ed 100644 --- a/eth2/types/src/aggregate_and_proof.rs +++ b/eth2/types/src/aggregate_and_proof.rs @@ -1,5 +1,6 @@ use super::{ - Attestation, ChainSpec, Domain, EthSpec, Fork, PublicKey, SecretKey, Signature, SignedRoot, + Attestation, ChainSpec, Domain, EthSpec, Fork, PublicKey, SecretKey, SelectionProof, Signature, + SignedRoot, }; use crate::test_utils::TestRandom; use serde_derive::{Deserialize, Serialize}; @@ -32,20 +33,13 @@ impl AggregateAndProof { fork: &Fork, spec: &ChainSpec, ) -> Self { - let slot = aggregate.data.slot; - - let domain = spec.get_domain( - slot.epoch(T::slots_per_epoch()), - Domain::SelectionProof, - fork, - ); - - let message = slot.signing_root(domain); + let selection_proof = + SelectionProof::new::(aggregate.data.slot, secret_key, fork, spec).into(); Self { aggregator_index, aggregate, - selection_proof: Signature::new(message.as_bytes(), secret_key), + selection_proof, } } diff --git a/eth2/types/src/lib.rs b/eth2/types/src/lib.rs index 7f185fdda..e8339eb45 100644 --- a/eth2/types/src/lib.rs +++ b/eth2/types/src/lib.rs @@ -32,6 +32,7 @@ pub mod indexed_attestation; pub mod pending_attestation; pub mod proposer_slashing; pub mod relative_epoch; +pub mod selection_proof; pub mod signed_aggregate_and_proof; pub mod signed_beacon_block; pub mod signed_beacon_block_header; @@ -72,6 +73,7 @@ pub use crate::indexed_attestation::IndexedAttestation; pub use crate::pending_attestation::PendingAttestation; pub use crate::proposer_slashing::ProposerSlashing; pub use crate::relative_epoch::{Error as RelativeEpochError, RelativeEpoch}; +pub use crate::selection_proof::SelectionProof; pub use crate::signed_aggregate_and_proof::SignedAggregateAndProof; pub use crate::signed_beacon_block::SignedBeaconBlock; pub use crate::signed_beacon_block_header::SignedBeaconBlockHeader; diff --git a/eth2/types/src/selection_proof.rs b/eth2/types/src/selection_proof.rs new file mode 100644 index 000000000..5f9daefab --- /dev/null +++ b/eth2/types/src/selection_proof.rs @@ -0,0 +1,42 @@ +use crate::{ChainSpec, Domain, EthSpec, Fork, SecretKey, Signature, SignedRoot, Slot}; +use std::convert::TryInto; +use tree_hash::TreeHash; + +#[derive(PartialEq, Debug, Clone)] +pub struct SelectionProof(Signature); + +impl SelectionProof { + pub fn new( + slot: Slot, + secret_key: &SecretKey, + fork: &Fork, + spec: &ChainSpec, + ) -> Self { + let domain = spec.get_domain( + slot.epoch(T::slots_per_epoch()), + Domain::SelectionProof, + fork, + ); + let message = slot.signing_root(domain); + + Self(Signature::new(message.as_bytes(), secret_key)) + } + + pub fn is_aggregator(&self, modulo: u64) -> bool { + let signature_hash = self.0.tree_hash_root(); + let signature_hash_int = u64::from_le_bytes( + signature_hash[0..8] + .as_ref() + .try_into() + .expect("first 8 bytes of signature should always convert to fixed array"), + ); + + signature_hash_int % modulo == 0 + } +} + +impl Into for SelectionProof { + fn into(self) -> Signature { + self.0 + } +} diff --git a/eth2/utils/rest_types/src/validator.rs b/eth2/utils/rest_types/src/validator.rs index 0666ffd87..a3bd34e7e 100644 --- a/eth2/utils/rest_types/src/validator.rs +++ b/eth2/utils/rest_types/src/validator.rs @@ -3,7 +3,7 @@ use eth2_hashing::hash; use serde::{Deserialize, Serialize}; use ssz_derive::{Decode, Encode}; use std::convert::TryInto; -use types::{ChainSpec, CommitteeIndex, Domain, Epoch, Fork, SignedRoot, Slot}; +use types::{CommitteeIndex, Epoch, Slot}; /// A Validator duty with the validator public key represented a `PublicKeyBytes`. pub type ValidatorDutyBytes = ValidatorDutyBase; @@ -64,45 +64,9 @@ pub struct ValidatorSubscription { /// The index of the committee within `slot` of which the validator is a member. Used by the /// beacon node to quickly evaluate the associated `SubnetId`. pub attestation_committee_index: CommitteeIndex, - /// The slot the validator is signing. + /// The slot in which to subscribe. pub slot: Slot, - /// The signature of the slot by the validator. - pub slot_signature: Signature, -} - -impl ValidatorSubscription { - pub fn new( - validator_index: u64, - attestation_committee_index: CommitteeIndex, - slot: Slot, - slot_signature: Signature, - ) -> Self { - ValidatorSubscription { - validator_index, - attestation_committee_index, - slot, - slot_signature, - } - } - - /// Verifies the subscription signature. - pub fn verify( - &self, - pubkey: &PublicKey, - spec: &ChainSpec, - fork: &Fork, - slots_per_epoch: u64, - ) -> bool { - let domain = spec.get_domain( - self.slot.epoch(slots_per_epoch), - Domain::SelectionProof, - &fork, - ); - let message = self.slot.signing_root(domain); - if self.slot_signature.verify(message.as_bytes(), pubkey) { - true - } else { - false - } - } + /// If true, the validator is an aggregator and the beacon node should aggregate attestations + /// for this slot. + pub is_aggregator: bool, } diff --git a/validator_client/src/attestation_service.rs b/validator_client/src/attestation_service.rs index c41fa6e1e..c8220775d 100644 --- a/validator_client/src/attestation_service.rs +++ b/validator_client/src/attestation_service.rs @@ -256,48 +256,34 @@ impl AttestationService { /// slot allowing the beacon node to connect to the required subnet and determine /// if attestations need to be aggregated. fn send_subscriptions(&self, duties: Vec) -> impl Future { - let mut validator_subscriptions = Vec::new(); - let mut successful_duties = Vec::new(); - let service_1 = self.clone(); - let duties_no = duties.len(); + let num_duties = duties.len(); let log_1 = self.context.log.clone(); let log_2 = self.context.log.clone(); - // builds a list of subscriptions - for duty in duties { - if let Some((slot, attestation_committee_index, _, validator_index)) = - duty.attestation_duties() - { - if let Some(slot_signature) = self + let (validator_subscriptions, successful_duties): (Vec<_>, Vec<_>) = duties + .into_iter() + .filter_map(|duty| { + let (slot, attestation_committee_index, _, validator_index) = + duty.attestation_duties()?; + let selection_proof = self .validator_store - .sign_slot(duty.validator_pubkey(), slot) - { - let is_aggregator_proof = if duty.is_aggregator() { - Some(slot_signature.clone()) - } else { - None - }; + .produce_selection_proof(duty.validator_pubkey(), slot)?; + let modulo = duty.duty.aggregator_modulo?; - let subscription = ValidatorSubscription::new( - validator_index, - attestation_committee_index, - slot, - slot_signature, - ); - validator_subscriptions.push(subscription); + let subscription = ValidatorSubscription { + validator_index, + attestation_committee_index, + slot, + is_aggregator: selection_proof.is_aggregator(modulo), + }; - // add successful duties to the list, along with whether they are aggregation - // duties or not - successful_duties.push((duty, is_aggregator_proof)); - } - } else { - crit!(log_2, "Validator duty doesn't have required fields"); - } - } + Some((subscription, (duty, selection_proof))) + }) + .unzip(); - let failed_duties = duties_no - successful_duties.len(); + let num_failed_duties = num_duties - successful_duties.len(); self.beacon_node .http @@ -308,8 +294,8 @@ impl AttestationService { PublishStatus::Valid => info!( log_1, "Successfully subscribed validators"; - "validators" => duties_no, - "failed_validators" => failed_duties, + "validators" => num_duties, + "failed_validators" => num_failed_duties, ), PublishStatus::Invalid(msg) => crit!( log_1, @@ -321,10 +307,10 @@ impl AttestationService { } }) .and_then(move |_| { - for (duty, is_aggregator_proof) in successful_duties { + for (duty, selection_proof) in successful_duties { service_1 .duties_service - .subscribe_duty(&duty.duty, is_aggregator_proof); + .subscribe_duty(&duty.duty, selection_proof); } Ok(()) }) diff --git a/validator_client/src/duties_service.rs b/validator_client/src/duties_service.rs index 7dbbdebd5..3a9f73790 100644 --- a/validator_client/src/duties_service.rs +++ b/validator_client/src/duties_service.rs @@ -1,5 +1,4 @@ use crate::validator_store::ValidatorStore; -use bls::Signature; use environment::RuntimeContext; use exit_future::Signal; use futures::{future, Future, IntoFuture, Stream}; @@ -14,7 +13,7 @@ use std::ops::Deref; use std::sync::Arc; use std::time::{Duration, Instant}; use tokio::timer::Interval; -use types::{ChainSpec, CommitteeIndex, Epoch, EthSpec, PublicKey, Slot}; +use types::{ChainSpec, CommitteeIndex, Epoch, EthSpec, PublicKey, SelectionProof, Slot}; /// 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(100); @@ -28,11 +27,9 @@ type BaseHashMap = HashMap>; pub enum DutyState { /// This duty has not been subscribed to the beacon node. NotSubscribed, - /// The duty has been subscribed to the beacon node. - Subscribed, /// The duty has been subscribed and the validator is an aggregator for this duty. The /// selection proof is provided to construct the `AggregateAndProof` struct. - SubscribedAggregator(Signature), + SubscribedAggregator(SelectionProof), } #[derive(Debug, Clone)] @@ -49,13 +46,12 @@ impl DutyAndState { pub fn is_aggregator(&self) -> bool { match self.state { DutyState::NotSubscribed => false, - DutyState::Subscribed => false, DutyState::SubscribedAggregator(_) => true, } } /// Returns the selection proof if the duty is an aggregation duty. - pub fn selection_proof(&self) -> Option { + pub fn selection_proof(&self) -> Option { match &self.state { DutyState::SubscribedAggregator(proof) => Some(proof.clone()), _ => None, @@ -66,7 +62,6 @@ impl DutyAndState { pub fn is_subscribed(&self) -> bool { match self.state { DutyState::NotSubscribed => false, - DutyState::Subscribed => true, DutyState::SubscribedAggregator(_) => true, } } @@ -425,14 +420,14 @@ impl DutiesService { /// Marks the duty as being subscribed to the beacon node. /// /// If the duty is to be marked as an aggregator duty, a selection proof is also provided. - pub fn subscribe_duty(&self, duty: &ValidatorDuty, aggregator_proof: Option) { - let state = match aggregator_proof { - Some(proof) => DutyState::SubscribedAggregator(proof), - None => DutyState::Subscribed, - }; + pub fn subscribe_duty(&self, duty: &ValidatorDuty, proof: SelectionProof) { if let Some(slot) = duty.attestation_slot { - self.store - .set_duty_state(&duty.validator_pubkey, slot, state, E::slots_per_epoch()) + self.store.set_duty_state( + &duty.validator_pubkey, + slot, + DutyState::SubscribedAggregator(proof), + E::slots_per_epoch(), + ) } } diff --git a/validator_client/src/validator_store.rs b/validator_client/src/validator_store.rs index d41208ff1..cbf239869 100644 --- a/validator_client/src/validator_store.rs +++ b/validator_client/src/validator_store.rs @@ -12,8 +12,8 @@ use std::path::PathBuf; use std::sync::Arc; use tempdir::TempDir; use types::{ - Attestation, BeaconBlock, ChainSpec, Domain, Epoch, EthSpec, Fork, PublicKey, Signature, - SignedAggregateAndProof, SignedBeaconBlock, SignedRoot, Slot, + Attestation, BeaconBlock, ChainSpec, Domain, Epoch, EthSpec, Fork, PublicKey, SelectionProof, + Signature, SignedAggregateAndProof, SignedBeaconBlock, SignedRoot, Slot, }; #[derive(Clone)] @@ -221,22 +221,21 @@ impl ValidatorStore { )) } - /// Signs a slot for a given validator. - /// - /// This is used to subscribe a validator to a beacon node and is used to determine if the - /// validator is to aggregate attestations for this slot. - pub fn sign_slot(&self, validator_pubkey: &PublicKey, slot: Slot) -> Option { + /// Produces a `SelectionProof` for the `slot`, signed by with corresponding secret key to + /// `validator_pubkey`. + pub fn produce_selection_proof( + &self, + validator_pubkey: &PublicKey, + slot: Slot, + ) -> Option { let validators = self.validators.read(); let voting_keypair = validators.get(validator_pubkey)?.voting_keypair.as_ref()?; - let domain = self.spec.get_domain( - slot.epoch(E::slots_per_epoch()), - Domain::SelectionProof, + Some(SelectionProof::new::( + slot, + &voting_keypair.sk, &self.fork()?, - ); - - let message = slot.signing_root(domain); - - Some(Signature::new(message.as_bytes(), &voting_keypair.sk)) + &self.spec, + )) } }