diff --git a/beacon_node/rest_api/src/helpers.rs b/beacon_node/rest_api/src/helpers.rs index 7c675e50f9..1431d9c89b 100644 --- a/beacon_node/rest_api/src/helpers.rs +++ b/beacon_node/rest_api/src/helpers.rs @@ -15,7 +15,8 @@ use std::sync::Arc; use store::{iter::AncestorIter, Store}; use tokio::sync::mpsc; use types::{ - Attestation, BeaconBlock, BeaconState, Epoch, EthSpec, Hash256, RelativeEpoch, Signature, Slot, + Attestation, BeaconBlock, BeaconState, CommitteeIndex, Epoch, EthSpec, Hash256, RelativeEpoch, + Signature, Slot, }; /// Parse a slot. @@ -38,13 +39,13 @@ pub fn parse_epoch(string: &str) -> Result { .map_err(|e| ApiError::BadRequest(format!("Unable to parse epoch: {:?}", e))) } -/// Parse an shard. +/// Parse a CommitteeIndex. /// /// E.g., `"18"` -pub fn parse_shard(string: &str) -> Result { +pub fn parse_committee_index(string: &str) -> Result { string - .parse::() - .map_err(|e| ApiError::BadRequest(format!("Unable to parse shard: {:?}", e))) + .parse::() + .map_err(|e| ApiError::BadRequest(format!("Unable to parse committee index: {:?}", e))) } /// Checks the provided request to ensure that the `content-type` header. diff --git a/beacon_node/rest_api/src/url_query.rs b/beacon_node/rest_api/src/url_query.rs index f0c587a32b..013ddd8e90 100644 --- a/beacon_node/rest_api/src/url_query.rs +++ b/beacon_node/rest_api/src/url_query.rs @@ -1,5 +1,7 @@ +use crate::helpers::{parse_committee_index, parse_epoch, parse_signature, parse_slot}; use crate::ApiError; use hyper::Request; +use types::{CommitteeIndex, Epoch, Signature, Slot}; /// Provides handy functions for parsing the query parameters of a URL. @@ -77,6 +79,30 @@ impl<'a> UrlQuery<'a> { .collect(); Ok(queries) } + + /// Returns the value of the first occurrence of the `epoch` key. + pub fn epoch(self) -> Result { + self.first_of(&["epoch"]) + .and_then(|(_key, value)| parse_epoch(&value)) + } + + /// Returns the value of the first occurrence of the `slot` key. + pub fn slot(self) -> Result { + self.first_of(&["slot"]) + .and_then(|(_key, value)| parse_slot(&value)) + } + + /// Returns the value of the first occurrence of the `committee_index` key. + pub fn committee_index(self) -> Result { + self.first_of(&["committee_index"]) + .and_then(|(_key, value)| parse_committee_index(&value)) + } + + /// Returns the value of the first occurrence of the `randao_reveal` key. + pub fn randao_reveal(self) -> Result { + self.first_of(&["randao_reveal"]) + .and_then(|(_key, value)| parse_signature(&value)) + } } #[cfg(test)] diff --git a/beacon_node/rest_api/src/validator.rs b/beacon_node/rest_api/src/validator.rs index b6d850ca85..0d40c9c06b 100644 --- a/beacon_node/rest_api/src/validator.rs +++ b/beacon_node/rest_api/src/validator.rs @@ -1,13 +1,13 @@ use crate::helpers::{ - check_content_type_for_json, parse_epoch, parse_pubkey, parse_signature, - publish_attestation_to_network, publish_beacon_block_to_network, + check_content_type_for_json, parse_pubkey, publish_attestation_to_network, + publish_beacon_block_to_network, }; use crate::response_builder::ResponseBuilder; use crate::{ApiError, ApiResult, BoxFut, NetworkChannel, UrlQuery}; use beacon_chain::{ AttestationProcessingOutcome, BeaconChain, BeaconChainTypes, BlockProcessingOutcome, }; -use bls::{AggregateSignature, PublicKey}; +use bls::PublicKey; use futures::future::Future; use futures::stream::Stream; use hyper::{Body, Request}; @@ -15,9 +15,9 @@ use serde::{Deserialize, Serialize}; use slog::{info, trace, warn, Logger}; use std::sync::Arc; use types::beacon_state::EthSpec; -use types::{Attestation, BeaconBlock, BitList, CommitteeIndex, RelativeEpoch, Slot}; +use types::{Attestation, BeaconBlock, CommitteeIndex, RelativeEpoch, Slot}; -#[derive(Debug, Serialize, Deserialize)] +#[derive(Debug, Serialize, Deserialize, Clone)] pub struct ValidatorDuty { /// The validator's BLS public key, uniquely identifying them. _48-bytes, hex encoded with 0x prefix, case insensitive._ pub validator_pubkey: PublicKey, @@ -25,6 +25,8 @@ pub struct ValidatorDuty { pub attestation_slot: Option, /// The index of the committee within `slot` of which the validator is a member. pub attestation_committee_index: Option, + /// The position of the validator in the committee. + pub attestation_committee_position: Option, /// The slot in which a validator must propose a block, or `null` if block production is not required. pub block_proposal_slot: Option, } @@ -41,9 +43,7 @@ pub fn get_validator_duties( let query = UrlQuery::from_request(&req)?; - let epoch = query - .first_of(&["epoch"]) - .and_then(|(_key, value)| parse_epoch(&value))?; + let epoch = query.epoch()?; let mut head_state = beacon_chain.head().beacon_state; @@ -108,6 +108,7 @@ pub fn get_validator_duties( validator_pubkey, attestation_slot: duties.map(|d| d.slot), attestation_committee_index: duties.map(|d| d.index), + attestation_committee_position: duties.map(|d| d.committee_position), block_proposal_slot, }) } else { @@ -115,6 +116,7 @@ pub fn get_validator_duties( validator_pubkey, attestation_slot: None, attestation_committee_index: None, + attestation_committee_position: None, block_proposal_slot: None, }) } @@ -131,20 +133,9 @@ pub fn get_new_beacon_block( beacon_chain: Arc>, ) -> ApiResult { let query = UrlQuery::from_request(&req)?; - let slot = query - .first_of(&["slot"]) - .map(|(_key, value)| value)? - .parse::() - .map(Slot::from) - .map_err(|e| { - ApiError::BadRequest(format!("Invalid slot parameter, must be a u64. {:?}", e)) - })?; - let randao_reveal = query - .first_of(&["randao_reveal"]) - .and_then(|(_key, value)| parse_signature(&value)) - .map_err(|e| { - ApiError::BadRequest(format!("Invalid hex string for randao_reveal: {:?}", e)) - })?; + + let slot = query.slot()?; + let randao_reveal = query.randao_reveal()?; let (new_block, _state) = beacon_chain .produce_block(randao_reveal, slot) @@ -212,110 +203,14 @@ pub fn get_new_attestation( req: Request, beacon_chain: Arc>, ) -> ApiResult { - let mut head_state = beacon_chain.head().beacon_state; - let query = UrlQuery::from_request(&req)?; - let val_pk_str = query - .first_of(&["validator_pubkey"]) - .map(|(_key, value)| value)?; - let val_pk = parse_pubkey(val_pk_str.as_str())?; - head_state - .update_pubkey_cache() - .map_err(|e| ApiError::ServerError(format!("Unable to build pubkey cache: {:?}", e)))?; - // Get the validator index from the supplied public key - // If it does not exist in the index, we cannot continue. - 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_else(|| { - ApiError::BadRequest( - "The provided validator public key does not correspond to a validator index." - .into(), - ) - })?; + let slot = query.slot()?; + let index = query.committee_index()?; - // Build cache for the requested epoch - head_state - .build_committee_cache(RelativeEpoch::Current, &beacon_chain.spec) - .map_err(|e| ApiError::ServerError(format!("Unable to build committee cache: {:?}", e)))?; - // 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 = 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_else(|| ApiError::BadRequest("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.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 - .first_of(&["poc_bit"]) - .map(|(_key, value)| value)? - .parse::() - .map_err(|e| { - ApiError::BadRequest(format!("Invalid slot parameter, must be a u64. {:?}", e)) - })?; - - 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_position, poc_bit) - .map_err(|e| { - ApiError::ServerError(format!( - "Unable to set aggregation bits for the attestation: {:?}", - e - )) - })?; - - // 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 requested_slot = query - .first_of(&["slot"]) - .map(|(_key, value)| value)? - .parse::() - .map(Slot::from) - .map_err(|e| { - ApiError::BadRequest(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::BadRequest(format!("Attestation data can only be requested for the current slot ({:?}), not your requested slot ({:?})", current_slot, requested_slot))); - } - - let index = query - .first_of(&["index"]) - .map(|(_key, value)| value)? - .parse::() - .map_err(|e| ApiError::BadRequest(format!("Index is not a valid u64 value: {:?}", e)))?; - - let attestation_data = beacon_chain - .produce_attestation_data(current_slot.into(), index) - .map_err(|e| ApiError::ServerError(format!("Could not produce an attestation: {:?}", e)))?; - - let attestation: Attestation = Attestation { - aggregation_bits, - data: attestation_data, - signature: AggregateSignature::new(), - }; + let attestation = beacon_chain + .produce_attestation(slot, index) + .map_err(|e| ApiError::BadRequest(format!("Unable to produce attestation: {:?}", e)))?; ResponseBuilder::new(&req)?.body(&attestation) } @@ -330,12 +225,8 @@ pub fn publish_attestation( try_future!(check_content_type_for_json(&req)); let response_builder = ResponseBuilder::new(&req); - let body = req.into_body(); - trace!( - log, - "Got the request body, now going to parse it into an attesation." - ); - Box::new(body + Box::new(req + .into_body() .concat2() .map_err(|e| ApiError::ServerError(format!("Unable to get request body: {:?}",e))) .map(|chunk| chunk.iter().cloned().collect::>()) diff --git a/beacon_node/rest_api/tests/test.rs b/beacon_node/rest_api/tests/test.rs index 37d03b4f20..fee986621e 100644 --- a/beacon_node/rest_api/tests/test.rs +++ b/beacon_node/rest_api/tests/test.rs @@ -5,7 +5,7 @@ use node_test_rig::{ environment::{Environment, EnvironmentBuilder}, LocalBeaconNode, }; -use remote_beacon_node::BeaconBlockPublishStatus; +use remote_beacon_node::PublishStatus; use std::sync::Arc; use tree_hash::{SignedRoot, TreeHash}; use types::{ @@ -64,6 +64,8 @@ fn sign_block( fn validator_produce_attestation() { 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"); @@ -73,29 +75,118 @@ fn validator_produce_attestation() { .expect("client should have beacon chain"); let state = beacon_chain.head().beacon_state.clone(); + /* + // Publish a block so that we're not attesting to the genesis block. + { + let slot = Slot::new(1); + let randao_reveal = get_randao_reveal(beacon_chain.clone(), slot, spec); + + let mut block = env + .runtime() + .block_on( + remote_node + .http + .validator() + .produce_block(slot, randao_reveal.clone()), + ) + .expect("should fetch block from http api"); + + sign_block(beacon_chain.clone(), &mut block, spec); + + let publish_status = env + .runtime() + .block_on(remote_node.http.validator().publish_block(block.clone())) + .expect("should publish block"); + assert_eq!( + publish_status, + PublishStatus::Valid, + "should have published block" + ); + } + */ + let validator_index = 0; - let validator_pubkey = state.validators[validator_index].pubkey.clone(); let duties = state .get_attestation_duties(validator_index, RelativeEpoch::Current) .expect("should have attestation duties cache") .expect("should have attestation duties"); - let attestation = env + dbg!(&duties); + + let mut attestation = env .runtime() - .block_on(remote_node.http.validator().produce_attestation( - duties.slot, - duties.shard, - &validator_pubkey, - false, - )) + .block_on( + remote_node + .http + .validator() + .produce_attestation(duties.slot, duties.index), + ) .expect("should fetch attestation from http api"); assert_eq!( - attestation.data.crosslink.shard, duties.shard, - "should have same shard" + attestation.data.index, duties.index, + "should have same index" + ); + assert_eq!(attestation.data.slot, duties.slot, "should have same slot"); + assert_eq!( + attestation.aggregation_bits.num_set_bits(), + 0, + "should have empty aggregation bits" ); - // TODO: try to push the attestation. + let keypair = generate_deterministic_keypair(validator_index); + + // Fetch the duties again, but via HTTP for authenticity. + let duties = env + .runtime() + .block_on(remote_node.http.validator().get_duties( + attestation.data.slot.epoch(E::slots_per_epoch()), + &[keypair.pk.clone()], + )) + .expect("should fetch duties from http api"); + let duties = &duties[0]; + + // Try publishing the attestation without a signature, ensure it is flagged as invalid. + let publish_status = env + .runtime() + .block_on( + remote_node + .http + .validator() + .publish_attestation(attestation.clone()), + ) + .expect("should publish attestation"); + assert!( + !publish_status.is_valid(), + "the unsigned published attestation should not be valid" + ); + + attestation + .sign( + &keypair.sk, + duties + .attestation_committee_position + .expect("should have committee position"), + &state.fork, + spec, + ) + .expect("should sign attestation"); + + // Try publishing the valid attestation. + let publish_status = env + .runtime() + .block_on( + remote_node + .http + .validator() + .publish_attestation(attestation.clone()), + ) + .expect("should publish attestation"); + dbg!(publish_status.clone()); + assert!( + publish_status.is_valid(), + "the signed published attestation should be valid" + ); } #[test] @@ -125,7 +216,7 @@ fn validator_duties() { let duties = env .runtime() .block_on(remote_node.http.validator().get_duties(epoch, &validators)) - .expect("should fetch block from http api"); + .expect("should fetch duties from http api"); assert_eq!( validators.len(), @@ -158,14 +249,14 @@ fn validator_duties() { ); assert_eq!( - Some(attestation_duty.shard), - duty.attestation_shard, - "attestation shard should match" + Some(attestation_duty.index), + duty.attestation_committee_index, + "attestation index should match" ); if let Some(slot) = duty.block_proposal_slot { let expected_proposer = state - .get_beacon_proposer_index(slot, RelativeEpoch::Current, spec) + .get_beacon_proposer_index(slot, spec) .expect("should know proposer"); assert_eq!( expected_proposer, validator_index, @@ -174,7 +265,7 @@ fn validator_duties() { } else { epoch.slot_iter(E::slots_per_epoch()).for_each(|slot| { let slot_proposer = state - .get_beacon_proposer_index(slot, RelativeEpoch::Current, spec) + .get_beacon_proposer_index(slot, spec) .expect("should know proposer"); assert!( slot_proposer != validator_index, @@ -231,7 +322,7 @@ fn validator_block_post() { .expect("should publish block"); assert_eq!( publish_status, - BeaconBlockPublishStatus::Valid, + PublishStatus::Valid, "the signed published block should be valid" ); diff --git a/eth2/types/src/attestation.rs b/eth2/types/src/attestation.rs index 5c08964bc7..e1b2078e93 100644 --- a/eth2/types/src/attestation.rs +++ b/eth2/types/src/attestation.rs @@ -1,4 +1,7 @@ -use super::{AggregateSignature, AttestationData, BitList, EthSpec}; +use super::{ + AggregateSignature, AttestationData, BitList, ChainSpec, Domain, EthSpec, Fork, SecretKey, + Signature, +}; use crate::test_utils::TestRandom; use serde_derive::{Deserialize, Serialize}; @@ -7,6 +10,12 @@ use test_random_derive::TestRandom; use tree_hash::TreeHash; use tree_hash_derive::{SignedRoot, TreeHash}; +#[derive(Debug, PartialEq)] +pub enum Error { + SszTypesError(ssz_types::Error), + AlreadySigned(usize), +} + /// Details an attestation that can be slashable. /// /// Spec v0.9.1 @@ -48,6 +57,37 @@ impl Attestation { self.aggregation_bits = self.aggregation_bits.union(&other.aggregation_bits); self.signature.add_aggregate(&other.signature); } + + /// Signs `self`, setting the `committee_position`'th bit of `aggregation_bits` to `true`. + /// + /// Returns an `AlreadySigned` error if the `committee_position`'th bit is already `true`. + pub fn sign( + &mut self, + secret_key: &SecretKey, + committee_position: usize, + fork: &Fork, + spec: &ChainSpec, + ) -> Result<(), Error> { + if self + .aggregation_bits + .get(committee_position) + .map_err(|e| Error::SszTypesError(e))? + { + Err(Error::AlreadySigned(committee_position)) + } else { + self.aggregation_bits + .set(committee_position, true) + .map_err(|e| Error::SszTypesError(e))?; + + let message = self.data.tree_hash_root(); + let domain = spec.get_domain(self.data.target.epoch, Domain::BeaconAttester, fork); + + self.signature + .add(&Signature::new(&message, domain, secret_key)); + + Ok(()) + } + } } #[cfg(test)] diff --git a/eth2/types/src/lib.rs b/eth2/types/src/lib.rs index 1fe35ccd60..d6d7dae7a8 100644 --- a/eth2/types/src/lib.rs +++ b/eth2/types/src/lib.rs @@ -39,7 +39,7 @@ pub mod validator; use ethereum_types::{H160, H256}; -pub use crate::attestation::Attestation; +pub use crate::attestation::{Attestation, Error as AttestationError}; pub use crate::attestation_data::AttestationData; pub use crate::attestation_duty::AttestationDuty; pub use crate::attester_slashing::AttesterSlashing; diff --git a/eth2/utils/remote_beacon_node/src/lib.rs b/eth2/utils/remote_beacon_node/src/lib.rs index 9fd8ec2294..92904c268f 100644 --- a/eth2/utils/remote_beacon_node/src/lib.rs +++ b/eth2/utils/remote_beacon_node/src/lib.rs @@ -14,7 +14,8 @@ use std::marker::PhantomData; use std::net::SocketAddr; use std::time::Duration; use types::{ - Attestation, BeaconBlock, BeaconState, Epoch, EthSpec, Hash256, PublicKey, Signature, Slot, + Attestation, BeaconBlock, BeaconState, CommitteeIndex, Epoch, EthSpec, Hash256, PublicKey, + Signature, Slot, }; use url::Url; @@ -134,20 +135,20 @@ fn error_for_status( } #[derive(Debug, PartialEq, Clone)] -pub enum BeaconBlockPublishStatus { - /// The block was valid and has been published to the network. +pub enum PublishStatus { + /// The object was valid and has been published to the network. Valid, - /// The block was not valid and may or may not have been published to the network. + /// The object was not valid and may or may not have been published to the network. Invalid(String), - /// The server responsed with an unknown status code. The block may or may not have been + /// The server responsed with an unknown status code. The object may or may not have been /// published to the network. Unknown, } -impl BeaconBlockPublishStatus { - /// Returns `true` if `*self == BeaconBlockPublishStatus::Valid`. +impl PublishStatus { + /// Returns `true` if `*self == PublishStatus::Valid`. pub fn is_valid(&self) -> bool { - *self == BeaconBlockPublishStatus::Valid + *self == PublishStatus::Valid } } @@ -167,18 +168,11 @@ impl Validator { pub fn produce_attestation( &self, slot: Slot, - shard: u64, - validator_pubkey: &PublicKey, - poc_bit: bool, + committee_index: CommitteeIndex, ) -> impl Future, Error = Error> { let query_params = vec![ ("slot".into(), format!("{}", slot)), - ("shard".into(), format!("{}", shard)), - ("poc_bit".into(), format!("{}", poc_bit as u8)), - ( - "validator_pubkey".into(), - pubkey_as_string(validator_pubkey), - ), + ("committee_index".into(), format!("{}", committee_index)), ]; let client = self.0.clone(); @@ -187,6 +181,31 @@ impl Validator { .and_then(move |url| client.json_get(url, query_params)) } + /// Posts an attestation to the beacon node, expecting it to verify it and publish it to the network. + pub fn publish_attestation( + &self, + attestation: Attestation, + ) -> impl Future { + let client = self.0.clone(); + self.url("attestation") + .into_future() + .and_then(move |url| client.json_post::<_>(url, attestation)) + .and_then(|mut response| { + response + .text() + .map(|text| (response, text)) + .map_err(Error::from) + }) + .and_then(|(response, text)| match response.status() { + StatusCode::OK => Ok(PublishStatus::Valid), + StatusCode::ACCEPTED => Ok(PublishStatus::Invalid(text)), + _ => response + .error_for_status() + .map_err(Error::from) + .map(|_| PublishStatus::Unknown), + }) + } + /// Returns the duties required of the given validator pubkeys in the given epoch. pub fn get_duties( &self, @@ -213,7 +232,7 @@ impl Validator { pub fn publish_block( &self, block: BeaconBlock, - ) -> impl Future { + ) -> impl Future { let client = self.0.clone(); self.url("block") .into_future() @@ -225,12 +244,12 @@ impl Validator { .map_err(Error::from) }) .and_then(|(response, text)| match response.status() { - StatusCode::OK => Ok(BeaconBlockPublishStatus::Valid), - StatusCode::ACCEPTED => Ok(BeaconBlockPublishStatus::Invalid(text)), + StatusCode::OK => Ok(PublishStatus::Valid), + StatusCode::ACCEPTED => Ok(PublishStatus::Invalid(text)), _ => response .error_for_status() .map_err(Error::from) - .map(|_| BeaconBlockPublishStatus::Unknown), + .map(|_| PublishStatus::Unknown), }) }