From 0c1ceab5276d97b18a21163fcb6199d2c283b593 Mon Sep 17 00:00:00 2001 From: Luke Anderson Date: Wed, 4 Sep 2019 13:43:45 +1000 Subject: [PATCH] Addressed Paul's suggestions. - Updated some comments. - Replaced match statements with map functions. --- beacon_node/rest_api/src/metrics.rs | 2 +- beacon_node/rest_api/src/network.rs | 14 +-- beacon_node/rest_api/src/validator.rs | 162 +++++++++++--------------- 3 files changed, 74 insertions(+), 104 deletions(-) diff --git a/beacon_node/rest_api/src/metrics.rs b/beacon_node/rest_api/src/metrics.rs index 9d2ecc343..2239249b6 100644 --- a/beacon_node/rest_api/src/metrics.rs +++ b/beacon_node/rest_api/src/metrics.rs @@ -67,7 +67,7 @@ pub fn get_prometheus(req: Request) -> ApiR String::from_utf8(buffer) .map(|string| { let mut response = success_response(Body::from(string)); - // Need to change the header to text/plain for prometheius + // Need to change the header to text/plain for prometheus response.headers_mut().insert( "content-type", HeaderValue::from_static("text/plain; charset=utf-8"), diff --git a/beacon_node/rest_api/src/network.rs b/beacon_node/rest_api/src/network.rs index dffa949c9..4f1f53bb9 100644 --- a/beacon_node/rest_api/src/network.rs +++ b/beacon_node/rest_api/src/network.rs @@ -4,7 +4,7 @@ use eth2_libp2p::{Enr, Multiaddr, PeerId}; use hyper::{Body, Request}; use std::sync::Arc; -/// HTTP handle to return the list of libp2p multiaddr the client is listening on. +/// HTTP handler to return the list of libp2p multiaddr the client is listening on. /// /// Returns a list of `Multiaddr`, serialized according to their `serde` impl. pub fn get_listen_addresses(req: Request) -> ApiResult { @@ -21,9 +21,9 @@ pub fn get_listen_addresses(req: Request) -> ApiResul ))) } -/// HTTP handle to return network port the client is listening on. +/// HTTP handler to return the network port the client is listening on. /// -/// Returns a list of `Multiaddr`, serialized according to their `serde` impl. +/// Returns the TCP port number in its plain form (which is also valid JSON serialization) pub fn get_listen_port(req: Request) -> ApiResult { let network = req .extensions() @@ -36,7 +36,7 @@ pub fn get_listen_port(req: Request) -> ApiResult { ))) } -/// HTTP handle to return the Discv5 ENR from the client's libp2p service. +/// HTTP handler to return the Discv5 ENR from the client's libp2p service. /// /// ENR is encoded as base64 string. pub fn get_enr(req: Request) -> ApiResult { @@ -53,7 +53,7 @@ pub fn get_enr(req: Request) -> ApiResult { ))) } -/// HTTP handle to return the `PeerId` from the client's libp2p service. +/// HTTP handler to return the `PeerId` from the client's libp2p service. /// /// PeerId is encoded as base58 string. pub fn get_peer_id(req: Request) -> ApiResult { @@ -70,7 +70,7 @@ pub fn get_peer_id(req: Request) -> ApiResult { ))) } -/// HTTP handle to return the number of peers connected in the client's libp2p service. +/// HTTP handler to return the number of peers connected in the client's libp2p service. pub fn get_peer_count(req: Request) -> ApiResult { let network = req .extensions() @@ -85,7 +85,7 @@ pub fn get_peer_count(req: Request) -> ApiResult { ))) } -/// HTTP handle to return the list of peers connected to the client's libp2p service. +/// HTTP handler to return the list of peers connected to the client's libp2p service. /// /// Peers are presented as a list of `PeerId::to_string()`. pub fn get_peer_list(req: Request) -> ApiResult { diff --git a/beacon_node/rest_api/src/validator.rs b/beacon_node/rest_api/src/validator.rs index 84ea485b5..229d84674 100644 --- a/beacon_node/rest_api/src/validator.rs +++ b/beacon_node/rest_api/src/validator.rs @@ -136,32 +136,24 @@ pub fn get_new_beacon_block(req: Request) - let beacon_chain = get_beacon_chain_from_request::(&req)?; let query = UrlQuery::from_request(&req)?; - let slot = match query.first_of(&["slot"]) { - Ok((_, v)) => Slot::new(v.parse::().map_err(|e| { - ApiError::InvalidQueryParams(format!("Invalid slot parameter, must be a u64. {:?}", e)) - })?), - Err(e) => { - return Err(e); - } - }; - let randao_reveal = match query.first_of(&["randao_reveal"]) { - Ok((_, v)) => Signature::from_bytes( - hex::decode(&v) - .map_err(|e| { - ApiError::InvalidQueryParams(format!( - "Invalid hex string for randao_reveal: {:?}", - e - )) - })? - .as_slice(), - ) + let slot = query + .first_of(&["slot"]) + .map(|(_key, value)| value)? + .parse::() + .map(Slot::from) .map_err(|e| { - ApiError::InvalidQueryParams(format!("randao_reveal is not a valid signature: {:?}", e)) - })?, - Err(e) => { - return Err(e); - } - }; + ApiError::InvalidQueryParams(format!("Invalid slot parameter, must be a u64. {:?}", e)) + })?; + let randao_bytes = query + .first_of(&["randao_reveal"]) + .map(|(_key, value)| value) + .map(hex::decode)? + .map_err(|e| { + ApiError::InvalidQueryParams(format!("Invalid hex string for randao_reveal: {:?}", e)) + })?; + let randao_reveal = Signature::from_bytes(randao_bytes.as_slice()).map_err(|e| { + ApiError::InvalidQueryParams(format!("randao_reveal is not a valid signature: {:?}", e)) + })?; let (new_block, _state) = beacon_chain .produce_block(randao_reveal, slot) @@ -185,43 +177,33 @@ pub fn get_new_attestation(req: Request) -> let head_state = &beacon_chain.head().beacon_state; let query = UrlQuery::from_request(&req)?; - let val_pk: PublicKey = match query.first_of(&["validator_pubkey"]) { - Ok((_, v)) => parse_pubkey(v.as_str())?, - Err(e) => { - return Err(e); - } - }; + let val_pk_str = query + .first_of(&["validator_pubkey"]) + .map(|(_key, value)| value)?; + let val_pk = parse_pubkey(val_pk_str.as_str())?; + // Get the validator index from the supplied public key // If it does not exist in the index, we cannot continue. - let val_index: usize = match head_state.get_validator_index(&val_pk) { - Ok(Some(i)) => i, - Ok(None) => { - return Err(ApiError::InvalidQueryParams( - "The provided validator public key does not correspond to a validator index." - .into(), - )); - } - Err(e) => { - return Err(ApiError::ServerError(format!( - "Unable to read validator index cache. {:?}", - e - ))); - } - }; + let val_index = head_state + .get_validator_index(&val_pk) + .map_err(|e| { + ApiError::ServerError(format!("Unable to read validator index cache. {:?}", e)) + })? + .ok_or(ApiError::InvalidQueryParams( + "The provided validator public key does not correspond to a validator index.".into(), + ))?; + // Get the duties of the validator, to make sure they match up. // If they don't have duties this epoch, then return an error - let val_duty = match head_state.get_attestation_duties(val_index, RelativeEpoch::Current) { - Ok(Some(d)) => d, - Ok(None) => { - return Err(ApiError::InvalidQueryParams("No validator duties could be found for the requested validator. Cannot provide valid attestation.".into())); - } - Err(e) => { - return Err(ApiError::ServerError(format!( + let val_duty = head_state + .get_attestation_duties(val_index, RelativeEpoch::Current) + .map_err(|e| { + ApiError::ServerError(format!( "unable to read cache for attestation duties: {:?}", e - ))) - } - }; + )) + })? + .ok_or(ApiError::InvalidQueryParams("No validator duties could be found for the requested validator. Cannot provide valid attestation.".into()))?; // Check that we are requesting an attestation during the slot where it is relevant. let present_slot = beacon_chain.read_slot_clock().ok_or(ApiError::ServerError( @@ -232,14 +214,14 @@ pub fn get_new_attestation(req: Request) -> } // Parse the POC bit and insert it into the aggregation bits - let poc_bit: bool = match query.first_of(&["poc_bit"]) { - Ok((_, v)) => v.parse::().map_err(|e| { - ApiError::InvalidQueryParams(format!("poc_bit is not a valid boolean value: {:?}", e)) - })?, - Err(e) => { - return Err(e); - } - }; + let poc_bit = query + .first_of(&["poc_bit"]) + .map(|(_key, value)| value)? + .parse::() + .map_err(|e| { + ApiError::InvalidQueryParams(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."); aggregation_bits @@ -251,41 +233,29 @@ pub fn get_new_attestation(req: Request) -> )) })?; - // Allow a provided slot parameter to check against the expected slot as a sanity check. + // Allow a provided slot parameter to check against the expected slot as a sanity check only. // Presently, we don't support attestations at future or past slots. - let _slot = match query.first_of(&["slot"]) { - Ok((_, v)) => { - let requested_slot = v.parse::().map_err(|e| { - ApiError::InvalidQueryParams(format!( - "Invalid slot parameter, must be a u64. {:?}", - e - )) - })?; - let current_slot = beacon_chain.head().beacon_state.slot.as_u64(); - if requested_slot != current_slot { - return Err(ApiError::InvalidQueryParams(format!("Attestation data can only be requested for the current slot ({:?}), not your requested slot ({:?})", current_slot, requested_slot))); - } - Slot::new(requested_slot) - } - Err(ApiError::InvalidQueryParams(_)) => { - // Just fill _slot with a dummy value for now, making the slot parameter optional - // We'll get the real slot from the ValidatorDuty - Slot::new(0) - } - Err(e) => { - return Err(e); - } - }; + let requested_slot = query + .first_of(&["slot"]) + .map(|(_key, value)| value)? + .parse::() + .map(Slot::from) + .map_err(|e| { + ApiError::InvalidQueryParams(format!("Invalid slot parameter, must be a u64. {:?}", e)) + })?; + let current_slot = beacon_chain.head().beacon_state.slot.as_u64(); + if requested_slot != current_slot { + return Err(ApiError::InvalidQueryParams(format!("Attestation data can only be requested for the current slot ({:?}), not your requested slot ({:?})", current_slot, requested_slot))); + } - let shard: Shard = match query.first_of(&["shard"]) { - Ok((_, v)) => v.parse::().map_err(|e| { + let shard = query + .first_of(&["shard"]) + .map(|(_key, value)| value)? + .parse::() + .map_err(|e| { ApiError::InvalidQueryParams(format!("Shard is not a valid u64 value: {:?}", e)) - })?, - Err(e) => { - // This is a mandatory parameter, return the error - return Err(e); - } - }; + })?; + let attestation_data = beacon_chain .produce_attestation_data(shard) .map_err(|e| ApiError::ServerError(format!("Could not produce an attestation: {:?}", e)))?;