From b60836be7c0eabb2c0f86946d011e666084d3684 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Wed, 20 Nov 2019 18:33:21 +1100 Subject: [PATCH] More testing for api --- beacon_node/rest_api/src/helpers.rs | 14 ++- beacon_node/rest_api/src/validator.rs | 159 ++++++++++++-------------- 2 files changed, 84 insertions(+), 89 deletions(-) diff --git a/beacon_node/rest_api/src/helpers.rs b/beacon_node/rest_api/src/helpers.rs index 8061ee267e..d6b50b5ecb 100644 --- a/beacon_node/rest_api/src/helpers.rs +++ b/beacon_node/rest_api/src/helpers.rs @@ -15,10 +15,10 @@ use std::sync::Arc; use store::{iter::AncestorIter, Store}; use tokio::sync::mpsc; use types::{ - Attestation, BeaconBlock, BeaconState, EthSpec, Hash256, RelativeEpoch, Signature, Slot, + Attestation, BeaconBlock, BeaconState, Epoch, EthSpec, Hash256, RelativeEpoch, Signature, Slot, }; -/// Parse a slot from a `0x` preixed string. +/// Parse a slot. /// /// E.g., `"1234"` pub fn parse_slot(string: &str) -> Result { @@ -28,6 +28,16 @@ pub fn parse_slot(string: &str) -> Result { .map_err(|e| ApiError::BadRequest(format!("Unable to parse slot: {:?}", e))) } +/// Parse an epoch. +/// +/// E.g., `"13"` +pub fn parse_epoch(string: &str) -> Result { + string + .parse::() + .map(Epoch::from) + .map_err(|e| ApiError::BadRequest(format!("Unable to parse epoch: {:?}", e))) +} + /// Checks the provided request to ensure that the `content-type` header. /// /// The content-type header should either be omitted, in which case JSON is assumed, or it should diff --git a/beacon_node/rest_api/src/validator.rs b/beacon_node/rest_api/src/validator.rs index 26959707e6..0cba9ee30a 100644 --- a/beacon_node/rest_api/src/validator.rs +++ b/beacon_node/rest_api/src/validator.rs @@ -1,6 +1,6 @@ use crate::helpers::{ - check_content_type_for_json, parse_pubkey, parse_signature, publish_attestation_to_network, - publish_beacon_block_to_network, + check_content_type_for_json, parse_epoch, parse_pubkey, parse_signature, + publish_attestation_to_network, publish_beacon_block_to_network, }; use crate::response_builder::ResponseBuilder; use crate::{ApiError, ApiResult, BoxFut, NetworkChannel, UrlQuery}; @@ -15,7 +15,7 @@ use serde::{Deserialize, Serialize}; use slog::{info, trace, warn, Logger}; use std::sync::Arc; use types::beacon_state::EthSpec; -use types::{Attestation, BeaconBlock, BitList, Epoch, RelativeEpoch, Shard, Slot}; +use types::{Attestation, BeaconBlock, BitList, RelativeEpoch, Shard, Slot}; #[derive(Debug, Serialize, Deserialize)] pub struct ValidatorDuty { @@ -30,56 +30,46 @@ pub struct ValidatorDuty { } /// HTTP Handler to retrieve a the duties for a set of validators during a particular epoch +/// +/// The given `epoch` must be within one epoch of the current epoch. pub fn get_validator_duties( req: Request, beacon_chain: Arc>, log: Logger, ) -> ApiResult { slog::trace!(log, "Validator duties requested of API: {:?}", &req); + + let query = UrlQuery::from_request(&req)?; + + let epoch = query + .first_of(&["epoch"]) + .and_then(|(_key, value)| parse_epoch(&value))?; + let mut head_state = beacon_chain.head().beacon_state; - slog::trace!(log, "Got head state from request."); - // Parse and check query parameters - let query = UrlQuery::from_request(&req)?; let current_epoch = head_state.current_epoch(); - let epoch = match query.first_of(&["epoch"]) { - Ok((_, v)) => { - slog::trace!(log, "Requested epoch {:?}", v); - Epoch::new(v.parse::().map_err(|e| { - slog::info!(log, "Invalid epoch {:?}", e); - ApiError::BadRequest(format!("Invalid epoch parameter, must be a u64. {:?}", e)) - })?) - } - Err(_) => { - // epoch not supplied, use the current epoch - slog::info!(log, "Using default epoch {:?}", current_epoch); - current_epoch - } - }; - let relative_epoch = RelativeEpoch::from_epoch(current_epoch, epoch).map_err(|e| { - slog::info!(log, "Requested epoch out of range."); + let relative_epoch = RelativeEpoch::from_epoch(current_epoch, epoch).map_err(|_| { ApiError::BadRequest(format!( - "Cannot get RelativeEpoch, epoch out of range: {:?}", - e + "Epoch must be within one epoch of the current epoch", )) })?; - let validators: Vec = query - .all_of("validator_pubkeys")? - .iter() - .map(|pk| parse_pubkey(pk)) - .collect::, _>>()?; - let mut duties: Vec = Vec::new(); - // Build cache for the requested epoch head_state .build_committee_cache(relative_epoch, &beacon_chain.spec) .map_err(|e| ApiError::ServerError(format!("Unable to build committee cache: {:?}", e)))?; - // Get a list of all validators for this epoch - let validator_proposers: Vec = epoch + head_state + .update_pubkey_cache() + .map_err(|e| ApiError::ServerError(format!("Unable to build pubkey cache: {:?}", e)))?; + + // Get a list of all validators for this epoch. + // + // Used for quickly determining the slot for a proposer. + let validator_proposers: Vec<(usize, Slot)> = epoch .slot_iter(T::EthSpec::slots_per_epoch()) .map(|slot| { head_state .get_beacon_proposer_index(slot, relative_epoch, &beacon_chain.spec) + .map(|i| (i, slot)) .map_err(|e| { ApiError::ServerError(format!( "Unable to get proposer index for validator: {:?}", @@ -87,61 +77,51 @@ pub fn get_validator_duties( )) }) }) - .collect::, _>>()?; + .collect::, _>>()?; - // Look up duties for each validator - for val_pk in validators { - let mut duty = ValidatorDuty { - validator_pubkey: val_pk.clone(), - attestation_slot: None, - attestation_shard: None, - block_proposal_slot: None, - }; + let duties = query + .all_of("validator_pubkeys")? + .iter() + .map(|string| parse_pubkey(string)) + .collect::, _>>()? + .into_iter() + .map(|validator_pubkey| { + if let Some(validator_index) = head_state + .get_validator_index(&validator_pubkey) + .map_err(|e| { + ApiError::ServerError(format!("Unable to read pubkey cache: {:?}", e)) + })? + { + let duties = head_state + .get_attestation_duties(validator_index, relative_epoch) + .map_err(|e| { + ApiError::ServerError(format!( + "Unable to obtain attestation duties: {:?}", + e + )) + })?; - // Get the validator index - // If it does not exist in the index, just add a null duty and move on. - let val_index: usize = match head_state.get_validator_index(&val_pk) { - Ok(Some(i)) => i, - Ok(None) => { - duties.append(&mut vec![duty]); - continue; + let block_proposal_slot = validator_proposers + .iter() + .find(|(i, _slot)| validator_index == *i) + .map(|(_i, slot)| *slot); + + Ok(ValidatorDuty { + validator_pubkey, + attestation_slot: duties.map(|d| d.slot), + attestation_shard: duties.map(|d| d.shard), + block_proposal_slot, + }) + } else { + Ok(ValidatorDuty { + validator_pubkey, + attestation_slot: None, + attestation_shard: None, + block_proposal_slot: None, + }) } - Err(e) => { - return Err(ApiError::ServerError(format!( - "Unable to read validator index cache. {:?}", - e - ))); - } - }; - - // Set attestation duties - match head_state.get_attestation_duties(val_index, relative_epoch) { - Ok(Some(d)) => { - duty.attestation_slot = Some(d.slot); - duty.attestation_shard = Some(d.shard); - } - Ok(None) => {} - Err(e) => { - return Err(ApiError::ServerError(format!( - "Unable to read cache for attestation duties: {:?}", - e - ))) - } - }; - - // If the validator is to propose a block, identify the slot - if let Some(slot) = validator_proposers.iter().position(|&v| val_index == v) { - duty.block_proposal_slot = Some(Slot::new( - relative_epoch - .into_epoch(current_epoch) - .start_slot(T::EthSpec::slots_per_epoch()) - .as_u64() - + slot as u64, - )); - } - - duties.append(&mut vec![duty]); - } + }) + .collect::, ApiError>>()?; ResponseBuilder::new(&req)?.body_no_ssz(&duties) } @@ -278,9 +258,12 @@ pub fn get_new_attestation( let present_slot = beacon_chain.slot().map_err(|e| ApiError::ServerError( format!("Beacon node is unable to determine present slot, either the state isn't generated or the chain hasn't begun. {:?}", e) ))?; + + /* if val_duty.slot != present_slot { return Err(ApiError::BadRequest(format!("Validator is only able to request an attestation during the slot they are allocated. Current slot: {:?}, allocated slot: {:?}", head_state.slot, val_duty.slot))); } + */ // Parse the POC bit and insert it into the aggregation bits let poc_bit = query @@ -291,8 +274,10 @@ pub fn get_new_attestation( ApiError::BadRequest(format!("Invalid slot parameter, must be a u64. {:?}", e)) })?; - let mut aggregation_bits = BitList::with_capacity(val_duty.committee_len) - .expect("An empty BitList should always be created, or we have bigger problems."); + let mut aggregation_bits = BitList::with_capacity(val_duty.committee_len).map_err(|e| { + ApiError::ServerError(format!("Unable to create aggregation bitlist: {:?}", e)) + })?; + aggregation_bits .set(val_duty.committee_index, poc_bit) .map_err(|e| {