From b0b2010700f40bdc0772432f45e1a3f9cb1ff757 Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Sun, 17 Nov 2019 19:48:50 +1100 Subject: [PATCH] Add validator duties tests --- beacon_node/rest_api/src/helpers.rs | 6 +- beacon_node/rest_api/src/validator.rs | 25 +++---- beacon_node/rest_api/tests/test.rs | 91 +++++++++++++++++++++++- eth2/utils/remote_beacon_node/Cargo.toml | 1 + eth2/utils/remote_beacon_node/src/lib.rs | 66 +++++++++++++++-- 5 files changed, 163 insertions(+), 26 deletions(-) diff --git a/beacon_node/rest_api/src/helpers.rs b/beacon_node/rest_api/src/helpers.rs index 2a8e7db428..8061ee267e 100644 --- a/beacon_node/rest_api/src/helpers.rs +++ b/beacon_node/rest_api/src/helpers.rs @@ -55,7 +55,7 @@ pub fn parse_signature(string: &str) -> Result { .map_err(|e| ApiError::BadRequest(format!("Unable to parse signature bytes: {:?}", e))) } else { Err(ApiError::BadRequest( - "Signature must have a '0x' prefix".to_string(), + "Signature must have a 0x prefix".to_string(), )) } } @@ -73,7 +73,7 @@ pub fn parse_root(string: &str) -> Result { .map_err(|e| ApiError::BadRequest(format!("Unable to parse root: {:?}", e))) } else { Err(ApiError::BadRequest( - "Root must have a '0x' prefix".to_string(), + "Root must have a 0x prefix".to_string(), )) } } @@ -90,7 +90,7 @@ pub fn parse_pubkey(string: &str) -> Result { Ok(pubkey) } else { Err(ApiError::BadRequest( - "Public key must have a '0x' prefix".to_string(), + "Public key must have a 0x prefix".to_string(), )) } } diff --git a/beacon_node/rest_api/src/validator.rs b/beacon_node/rest_api/src/validator.rs index 392338831a..26959707e6 100644 --- a/beacon_node/rest_api/src/validator.rs +++ b/beacon_node/rest_api/src/validator.rs @@ -7,7 +7,7 @@ use crate::{ApiError, ApiResult, BoxFut, NetworkChannel, UrlQuery}; use beacon_chain::{ AttestationProcessingOutcome, BeaconChain, BeaconChainTypes, BlockProcessingOutcome, }; -use bls::{AggregateSignature, PublicKey, BLS_PUBLIC_KEY_BYTE_SIZE}; +use bls::{AggregateSignature, PublicKey}; use futures::future::Future; use futures::stream::Stream; use hyper::{Body, Request}; @@ -29,18 +29,6 @@ pub struct ValidatorDuty { pub block_proposal_slot: Option, } -impl ValidatorDuty { - pub fn new() -> ValidatorDuty { - ValidatorDuty { - validator_pubkey: PublicKey::from_bytes(vec![0; BLS_PUBLIC_KEY_BYTE_SIZE].as_slice()) - .expect("Should always be able to create a 'zero' BLS public key."), - attestation_slot: None, - attestation_shard: None, - block_proposal_slot: None, - } - } -} - /// HTTP Handler to retrieve a the duties for a set of validators during a particular epoch pub fn get_validator_duties( req: Request, @@ -103,8 +91,12 @@ pub fn get_validator_duties( // Look up duties for each validator for val_pk in validators { - let mut duty = ValidatorDuty::new(); - duty.validator_pubkey = val_pk.clone(); + let mut duty = ValidatorDuty { + validator_pubkey: val_pk.clone(), + attestation_slot: None, + attestation_shard: None, + block_proposal_slot: None, + }; // Get the validator index // If it does not exist in the index, just add a null duty and move on. @@ -131,7 +123,7 @@ pub fn get_validator_duties( Ok(None) => {} Err(e) => { return Err(ApiError::ServerError(format!( - "unable to read cache for attestation duties: {:?}", + "Unable to read cache for attestation duties: {:?}", e ))) } @@ -150,6 +142,7 @@ pub fn get_validator_duties( duties.append(&mut vec![duty]); } + ResponseBuilder::new(&req)?.body_no_ssz(&duties) } diff --git a/beacon_node/rest_api/tests/test.rs b/beacon_node/rest_api/tests/test.rs index a0b96c2e38..345cb8974e 100644 --- a/beacon_node/rest_api/tests/test.rs +++ b/beacon_node/rest_api/tests/test.rs @@ -9,8 +9,8 @@ use remote_beacon_node::BeaconBlockPublishStatus; use std::sync::Arc; use tree_hash::{SignedRoot, TreeHash}; use types::{ - test_utils::generate_deterministic_keypair, BeaconBlock, ChainSpec, Domain, EthSpec, - MinimalEthSpec, Signature, Slot, + test_utils::generate_deterministic_keypair, BeaconBlock, ChainSpec, Domain, Epoch, EthSpec, + MinimalEthSpec, RelativeEpoch, Signature, Slot, }; type E = MinimalEthSpec; @@ -60,6 +60,93 @@ fn sign_block( block.signature = Signature::new(&message, domain, &keypair.sk); } +#[test] +fn validator_duties() { + let mut env = build_env(); + + let spec = &E::default_spec(); + + let node = LocalBeaconNode::production(env.core_context()); + let remote_node = node.remote_node().expect("should produce remote node"); + + let beacon_chain = node + .client + .beacon_chain() + .expect("client should have beacon chain"); + + let epoch = Epoch::new(0); + + let validators = beacon_chain + .head() + .beacon_state + .validators + .iter() + .map(|v| v.pubkey.clone()) + .collect::>(); + + let duties = env + .runtime() + .block_on(remote_node.http.validator().get_duties(epoch, &validators)) + .expect("should fetch block from http api"); + + assert_eq!( + validators.len(), + duties.len(), + "there should be a duty for each validator" + ); + + let state = beacon_chain.head().beacon_state.clone(); + + validators + .iter() + .zip(duties.iter()) + .for_each(|(validator, duty)| { + assert_eq!(*validator, duty.validator_pubkey, "pubkey should match"); + + let validator_index = state + .get_validator_index(validator) + .expect("should have pubkey cache") + .expect("pubkey should exist"); + + let attestation_duty = state + .get_attestation_duties(validator_index, RelativeEpoch::Current) + .expect("should have attestation duties cache") + .expect("should have attestation duties"); + + assert_eq!( + Some(attestation_duty.slot), + duty.attestation_slot, + "attestation slot should match" + ); + + assert_eq!( + Some(attestation_duty.shard), + duty.attestation_shard, + "attestation shard should match" + ); + + if let Some(slot) = duty.block_proposal_slot { + let expected_proposer = state + .get_beacon_proposer_index(slot, RelativeEpoch::Current, spec) + .expect("should know proposer"); + assert_eq!( + expected_proposer, validator_index, + "should get correct proposal slot" + ); + } else { + epoch.slot_iter(E::slots_per_epoch()).for_each(|slot| { + let slot_proposer = state + .get_beacon_proposer_index(slot, RelativeEpoch::Current, spec) + .expect("should know proposer"); + assert!( + slot_proposer != validator_index, + "validator should not have proposal slot in this epoch" + ) + }) + } + }); +} + #[test] fn validator_block_post() { let mut env = build_env(); diff --git a/eth2/utils/remote_beacon_node/Cargo.toml b/eth2/utils/remote_beacon_node/Cargo.toml index 677646b4d4..9cc0649ee5 100644 --- a/eth2/utils/remote_beacon_node/Cargo.toml +++ b/eth2/utils/remote_beacon_node/Cargo.toml @@ -15,3 +15,4 @@ types = { path = "../../../eth2/types" } rest_api = { path = "../../../beacon_node/rest_api" } hex = "0.3" eth2_ssz = { path = "../../../eth2/utils/ssz" } +serde_json = "^1.0" diff --git a/eth2/utils/remote_beacon_node/src/lib.rs b/eth2/utils/remote_beacon_node/src/lib.rs index 6559f30f49..538218ccc8 100644 --- a/eth2/utils/remote_beacon_node/src/lib.rs +++ b/eth2/utils/remote_beacon_node/src/lib.rs @@ -3,7 +3,7 @@ //! //! Presently, this is only used for testing but it _could_ become a user-facing library. -use futures::{Future, IntoFuture}; +use futures::{future, Future, IntoFuture}; use reqwest::{ r#async::{Client, ClientBuilder, Response}, StatusCode, @@ -13,11 +13,10 @@ use ssz::Encode; use std::marker::PhantomData; use std::net::SocketAddr; use std::time::Duration; -use types::{BeaconBlock, BeaconState, EthSpec, Signature}; -use types::{Hash256, Slot}; +use types::{BeaconBlock, BeaconState, Epoch, EthSpec, Hash256, PublicKey, Signature, Slot}; use url::Url; -pub use rest_api::HeadResponse; +pub use rest_api::{HeadResponse, ValidatorDuty}; pub const REQUEST_TIMEOUT_SECONDS: u64 = 5; @@ -37,8 +36,14 @@ impl RemoteBeaconNode { #[derive(Debug)] pub enum Error { + /// Unable to parse a URL. Check the server URL. UrlParseError(url::ParseError), + /// The `reqwest` library returned an error. ReqwestError(reqwest::Error), + /// There was an error when encoding/decoding an object using serde. + SerdeJsonError(serde_json::Error), + /// The server responded to the request, however it did not return a 200-type success code. + DidNotSucceed { status: StatusCode, body: String }, } #[derive(Clone)] @@ -102,11 +107,30 @@ impl HttpClient { .get(&url.to_string()) .send() .map_err(Error::from) - .and_then(|response| response.error_for_status().map_err(Error::from)) + .and_then(|response| error_for_status(response).map_err(Error::from)) .and_then(|mut success| success.json::().map_err(Error::from)) } } +/// Returns an `Error` (with a description) if the `response` was not a 200-type success response. +/// +/// Distinct from `Response::error_for_status` because it includes the body of the response as +/// text. This ensures the error message from the server is not discarded. +fn error_for_status( + mut response: Response, +) -> Box + Send> { + let status = response.status(); + + if status.is_success() { + Box::new(future::ok(response)) + } else { + Box::new(response.text().then(move |text_result| match text_result { + Err(e) => Err(Error::ReqwestError(e)), + Ok(body) => Err(Error::DidNotSucceed { status, body }), + })) + } +} + #[derive(Debug, PartialEq, Clone)] pub enum BeaconBlockPublishStatus { /// The block was valid and has been published to the network. @@ -137,6 +161,28 @@ impl Validator { .map_err(Into::into) } + /// Returns the duties required of the given validator pubkeys in the given epoch. + pub fn get_duties( + &self, + epoch: Epoch, + validator_pubkeys: &[PublicKey], + ) -> impl Future, Error = Error> { + let validator_pubkeys: Vec = + validator_pubkeys.iter().map(pubkey_as_string).collect(); + + let client = self.0.clone(); + self.url("duties").into_future().and_then(move |url| { + let mut query_params = validator_pubkeys + .into_iter() + .map(|pubkey| ("validator_pubkeys".to_string(), pubkey)) + .collect::>(); + + query_params.push(("epoch".into(), format!("{}", epoch.as_u64()))); + + client.json_get::<_>(url, query_params) + }) + } + /// Posts a block to the beacon node, expecting it to verify it and publish it to the network. pub fn publish_block( &self, @@ -285,6 +331,10 @@ fn signature_as_string(signature: &Signature) -> String { format!("0x{}", hex::encode(signature.as_ssz_bytes())) } +fn pubkey_as_string(pubkey: &PublicKey) -> String { + format!("0x{}", hex::encode(pubkey.as_ssz_bytes())) +} + impl From for Error { fn from(e: reqwest::Error) -> Error { Error::ReqwestError(e) @@ -296,3 +346,9 @@ impl From for Error { Error::UrlParseError(e) } } + +impl From for Error { + fn from(e: serde_json::Error) -> Error { + Error::SerdeJsonError(e) + } +}