From be7e326c33a5d219e1c08b4c788405f092ea3a0c Mon Sep 17 00:00:00 2001 From: Paul Hauner Date: Mon, 28 Jan 2019 16:21:33 +1100 Subject: [PATCH] Add FreeAttesation type --- .../src/attestation_aggregator.rs | 28 +++++++------ .../src/attestation_processing.rs | 8 ++-- .../src/validator/beacon_node/attester.rs | 12 ++---- .../src/validator/beacon_node/mod.rs | 11 +++-- .../test_harness/src/validator/mod.rs | 42 +++++++++++++------ .../beacon_chain/test_harness/tests/chain.rs | 14 +++---- eth2/attester/src/lib.rs | 10 ++++- eth2/attester/src/test_utils/beacon_node.rs | 13 ++---- eth2/attester/src/traits.rs | 6 +-- eth2/types/src/free_attestation.rs | 12 ++++++ eth2/types/src/lib.rs | 2 + 11 files changed, 92 insertions(+), 66 deletions(-) create mode 100644 eth2/types/src/free_attestation.rs diff --git a/beacon_node/beacon_chain/src/attestation_aggregator.rs b/beacon_node/beacon_chain/src/attestation_aggregator.rs index 4b83bbebcb..a67923e13f 100644 --- a/beacon_node/beacon_chain/src/attestation_aggregator.rs +++ b/beacon_node/beacon_chain/src/attestation_aggregator.rs @@ -1,6 +1,7 @@ use std::collections::{HashMap, HashSet}; use types::{ - AggregateSignature, Attestation, AttestationData, BeaconState, Bitfield, ChainSpec, Signature, + AggregateSignature, Attestation, AttestationData, BeaconState, Bitfield, ChainSpec, + FreeAttestation, Signature, }; const PHASE_0_CUSTODY_BIT: bool = false; @@ -32,27 +33,30 @@ impl AttestationAggregator { pub fn process_free_attestation( &mut self, state: &BeaconState, - attestation_data: &AttestationData, - signature: &Signature, - validator_index: u64, + free_attestation: &FreeAttestation, ) -> Result { - let validator_index = validator_index as usize; + let validator_index = free_attestation.validator_index as usize; - let signable_message = attestation_data.signable_message(PHASE_0_CUSTODY_BIT); + let signable_message = free_attestation.data.signable_message(PHASE_0_CUSTODY_BIT); let validator_pubkey = &state .validator_registry .get(validator_index) .ok_or_else(|| ProcessError::BadValidatorIndex)? .pubkey; - if !signature.verify(&signable_message, &validator_pubkey) { + if !free_attestation + .signature + .verify(&signable_message, &validator_pubkey) + { return Err(ProcessError::BadSignature); } if let Some(existing_attestation) = self.store.get(&signable_message) { - if let Some(updated_attestation) = - aggregate_attestation(existing_attestation, signature, validator_index) - { + if let Some(updated_attestation) = aggregate_attestation( + existing_attestation, + &free_attestation.signature, + validator_index, + ) { self.store.insert(signable_message, updated_attestation); Ok(ProcessOutcome::Aggregated) } else { @@ -60,11 +64,11 @@ impl AttestationAggregator { } } else { let mut aggregate_signature = AggregateSignature::new(); - aggregate_signature.add(signature); + aggregate_signature.add(&free_attestation.signature); let mut aggregation_bitfield = Bitfield::new(); aggregation_bitfield.set(validator_index, true); let new_attestation = Attestation { - data: attestation_data.clone(), + data: free_attestation.data.clone(), aggregation_bitfield, custody_bitfield: Bitfield::new(), aggregate_signature, diff --git a/beacon_node/beacon_chain/src/attestation_processing.rs b/beacon_node/beacon_chain/src/attestation_processing.rs index 90107dd97b..8704cd4899 100644 --- a/beacon_node/beacon_chain/src/attestation_processing.rs +++ b/beacon_node/beacon_chain/src/attestation_processing.rs @@ -1,7 +1,7 @@ use super::{BeaconChain, ClientDB, SlotClock}; pub use crate::attestation_aggregator::{ProcessError as AggregatorError, ProcessOutcome}; use crate::canonical_head::Error as HeadError; -use types::{AttestationData, Signature}; +use types::FreeAttestation; #[derive(Debug, PartialEq)] pub enum Error { @@ -17,9 +17,7 @@ where { pub fn process_free_attestation( &self, - attestation_data: &AttestationData, - signature: &Signature, - validator_index: u64, + free_attestation: FreeAttestation, ) -> Result { let present_slot = self .present_slot() @@ -29,7 +27,7 @@ where self.attestation_aggregator .write() .expect("Aggregator unlock failed.") - .process_free_attestation(&state, attestation_data, signature, validator_index) + .process_free_attestation(&state, &free_attestation) .map_err(|e| e.into()) } } diff --git a/beacon_node/beacon_chain/test_harness/src/validator/beacon_node/attester.rs b/beacon_node/beacon_chain/test_harness/src/validator/beacon_node/attester.rs index 71c45dd314..4ef0efe361 100644 --- a/beacon_node/beacon_chain/test_harness/src/validator/beacon_node/attester.rs +++ b/beacon_node/beacon_chain/test_harness/src/validator/beacon_node/attester.rs @@ -4,7 +4,7 @@ use beacon_chain::block_processing::Error as ProcessingError; use beacon_chain::block_production::Error as BlockProductionError; use db::ClientDB; use slot_clock::SlotClock; -use types::{AttestationData, Signature}; +use types::{AttestationData, FreeAttestation}; impl AttesterBeaconNode for BenchingBeaconNode where @@ -24,15 +24,9 @@ where fn publish_attestation_data( &self, - attestation_data: AttestationData, - signature: Signature, - validator_index: u64, + free_attestation: FreeAttestation, ) -> Result { - match self.beacon_chain.process_free_attestation( - &attestation_data, - &signature, - validator_index, - ) { + match self.beacon_chain.process_free_attestation(free_attestation) { Ok(_) => Ok(PublishOutcome::ValidAttestation), Err(e) => Err(NodeError::RemoteFailure(format!("{:?}", e))), } diff --git a/beacon_node/beacon_chain/test_harness/src/validator/beacon_node/mod.rs b/beacon_node/beacon_chain/test_harness/src/validator/beacon_node/mod.rs index 60664468df..caed2bc5f9 100644 --- a/beacon_node/beacon_chain/test_harness/src/validator/beacon_node/mod.rs +++ b/beacon_node/beacon_chain/test_harness/src/validator/beacon_node/mod.rs @@ -3,16 +3,11 @@ use db::ClientDB; use parking_lot::RwLock; use slot_clock::SlotClock; use std::sync::Arc; -use types::{AttestationData, BeaconBlock, Signature}; +use types::{BeaconBlock, FreeAttestation}; mod attester; mod producer; -/// An attestation that hasn't been aggregated into an `Attestation`. -/// -/// (attestation_data, signature, validator_index) -pub type FreeAttestation = (AttestationData, Signature, u64); - pub struct BenchingBeaconNode { beacon_chain: Arc>, published_blocks: RwLock>, @@ -31,4 +26,8 @@ impl BenchingBeaconNode { pub fn last_published_block(&self) -> Option { Some(self.published_blocks.read().last()?.clone()) } + + pub fn last_published_free_attestation(&self) -> Option { + Some(self.published_attestations.read().last()?.clone()) + } } diff --git a/beacon_node/beacon_chain/test_harness/src/validator/mod.rs b/beacon_node/beacon_chain/test_harness/src/validator/mod.rs index f2ac960755..3f1b10d5e9 100644 --- a/beacon_node/beacon_chain/test_harness/src/validator/mod.rs +++ b/beacon_node/beacon_chain/test_harness/src/validator/mod.rs @@ -1,24 +1,30 @@ -use attester::Attester; +use attester::{Attester, Error as AttestationPollError}; use beacon_chain::BeaconChain; -use block_producer::{BlockProducer, Error as PollError}; +use block_producer::{BlockProducer, Error as BlockPollError}; use db::MemoryDB; use signer::TestSigner; use slot_clock::TestingSlotClock; use std::sync::Arc; -use types::{BeaconBlock, ChainSpec, Keypair}; - +use types::{BeaconBlock, ChainSpec, FreeAttestation, Keypair}; mod beacon_node; mod direct_duties; mod signer; pub use self::beacon_node::BenchingBeaconNode; pub use self::direct_duties::DirectDuties; -pub use block_producer::PollOutcome; +pub use attester::PollOutcome as AttestationPollOutcome; +pub use block_producer::PollOutcome as BlockPollOutcome; #[derive(Debug, PartialEq)] -pub enum ProduceError { - DidNotProduce(PollOutcome), - PollError(PollError), +pub enum BlockProduceError { + DidNotProduce(BlockPollOutcome), + PollError(BlockPollError), +} + +#[derive(Debug, PartialEq)] +pub enum AttestationProduceError { + DidNotProduce(AttestationPollOutcome), + PollError(AttestationPollError), } pub struct TestValidator { @@ -81,13 +87,13 @@ impl TestValidator { } } - pub fn produce_block(&mut self) -> Result { + pub fn produce_block(&mut self) -> Result { // Using `BenchingBeaconNode`, the validator will always return sucessufully if it tries to // publish a block. match self.block_producer.poll() { - Ok(PollOutcome::BlockProduced(_)) => {} - Ok(outcome) => return Err(ProduceError::DidNotProduce(outcome)), - Err(error) => return Err(ProduceError::PollError(error)), + Ok(BlockPollOutcome::BlockProduced(_)) => {} + Ok(outcome) => return Err(BlockProduceError::DidNotProduce(outcome)), + Err(error) => return Err(BlockProduceError::PollError(error)), }; Ok(self .beacon_node @@ -95,6 +101,18 @@ impl TestValidator { .expect("Unable to obtain produced block.")) } + pub fn produce_free_attestation(&mut self) -> Result { + match self.attester.poll() { + Ok(AttestationPollOutcome::AttestationProduced(_)) => {} + Ok(outcome) => return Err(AttestationProduceError::DidNotProduce(outcome)), + Err(error) => return Err(AttestationProduceError::PollError(error)), + }; + Ok(self + .beacon_node + .last_published_free_attestation() + .expect("Unable to obtain produced attestation.")) + } + pub fn set_slot(&mut self, slot: u64) { self.slot_clock.set_slot(slot) } diff --git a/beacon_node/beacon_chain/test_harness/tests/chain.rs b/beacon_node/beacon_chain/test_harness/tests/chain.rs index 19bdf233f4..bac1e13476 100644 --- a/beacon_node/beacon_chain/test_harness/tests/chain.rs +++ b/beacon_node/beacon_chain/test_harness/tests/chain.rs @@ -4,25 +4,25 @@ use types::ChainSpec; #[test] fn it_can_build_on_genesis_block() { let validator_count = 2; - let mut rig = BeaconChainHarness::new(ChainSpec::foundation(), validator_count); + let mut harness = BeaconChainHarness::new(ChainSpec::foundation(), validator_count); - rig.advance_chain_with_block(); + harness.advance_chain_with_block(); } #[test] #[ignore] fn it_can_produce_past_first_epoch_boundary() { let validator_count = 2; - let mut rig = BeaconChainHarness::new(ChainSpec::foundation(), validator_count); + let mut harness = BeaconChainHarness::new(ChainSpec::foundation(), validator_count); - let blocks = rig.spec.epoch_length + 1; + let blocks = harness.spec.epoch_length + 1; for _ in 0..blocks { - rig.advance_chain_with_block(); + harness.advance_chain_with_block(); } - let dump = rig.chain_dump().expect("Chain dump failed."); + let dump = harness.chain_dump().expect("Chain dump failed."); assert_eq!(dump.len() as u64, blocks + 1); // + 1 for genesis block. - rig.dump_to_file("/tmp/chaindump.json".to_string(), &dump); + harness.dump_to_file("/tmp/chaindump.json".to_string(), &dump); } diff --git a/eth2/attester/src/lib.rs b/eth2/attester/src/lib.rs index 6047626346..447bcb9eaa 100644 --- a/eth2/attester/src/lib.rs +++ b/eth2/attester/src/lib.rs @@ -3,7 +3,7 @@ mod traits; use slot_clock::SlotClock; use std::sync::Arc; -use types::{AttestationData, Signature}; +use types::{AttestationData, FreeAttestation, Signature}; pub use self::traits::{ BeaconNode, BeaconNodeError, DutiesReader, DutiesReaderError, PublishOutcome, Signer, @@ -111,8 +111,14 @@ impl Attester return Ok(PollOutcome::ValidatorIsUnknown(slot)), }; + let free_attestation = FreeAttestation { + data: attestation_data, + signature, + validator_index, + }; + self.beacon_node - .publish_attestation_data(attestation_data, signature, validator_index)?; + .publish_attestation_data(free_attestation)?; Ok(PollOutcome::AttestationProduced(slot)) } diff --git a/eth2/attester/src/test_utils/beacon_node.rs b/eth2/attester/src/test_utils/beacon_node.rs index 4ebcbdf5ab..5b4061c609 100644 --- a/eth2/attester/src/test_utils/beacon_node.rs +++ b/eth2/attester/src/test_utils/beacon_node.rs @@ -1,6 +1,6 @@ use crate::traits::{BeaconNode, BeaconNodeError, PublishOutcome}; use std::sync::RwLock; -use types::{AttestationData, Signature}; +use types::{AttestationData, FreeAttestation}; type ProduceResult = Result, BeaconNodeError>; type PublishResult = Result; @@ -11,7 +11,7 @@ pub struct TestBeaconNode { pub produce_input: RwLock>, pub produce_result: RwLock>, - pub publish_input: RwLock>, + pub publish_input: RwLock>, pub publish_result: RwLock>, } @@ -34,13 +34,8 @@ impl BeaconNode for TestBeaconNode { } } - fn publish_attestation_data( - &self, - attestation_data: AttestationData, - signature: Signature, - validator_index: u64, - ) -> PublishResult { - *self.publish_input.write().unwrap() = Some((attestation_data, signature, validator_index)); + fn publish_attestation_data(&self, free_attestation: FreeAttestation) -> PublishResult { + *self.publish_input.write().unwrap() = Some(free_attestation.clone()); match *self.publish_result.read().unwrap() { Some(ref r) => r.clone(), None => panic!("TestBeaconNode: publish_result == None"), diff --git a/eth2/attester/src/traits.rs b/eth2/attester/src/traits.rs index ce6a635e71..fd07fd1718 100644 --- a/eth2/attester/src/traits.rs +++ b/eth2/attester/src/traits.rs @@ -1,4 +1,4 @@ -use types::{AttestationData, Signature}; +use types::{AttestationData, FreeAttestation, Signature}; #[derive(Debug, PartialEq, Clone)] pub enum BeaconNodeError { @@ -22,9 +22,7 @@ pub trait BeaconNode: Send + Sync { fn publish_attestation_data( &self, - attestation_data: AttestationData, - signature: Signature, - validator_index: u64, + free_attestation: FreeAttestation, ) -> Result; } diff --git a/eth2/types/src/free_attestation.rs b/eth2/types/src/free_attestation.rs new file mode 100644 index 0000000000..16d4f67281 --- /dev/null +++ b/eth2/types/src/free_attestation.rs @@ -0,0 +1,12 @@ +/// Note: this object does not actually exist in the spec. +/// +/// We use it for managing attestations that have not been aggregated. +use super::{AttestationData, Signature}; +use serde_derive::Serialize; + +#[derive(Debug, Clone, PartialEq, Serialize)] +pub struct FreeAttestation { + pub data: AttestationData, + pub signature: Signature, + pub validator_index: u64, +} diff --git a/eth2/types/src/lib.rs b/eth2/types/src/lib.rs index 3bf13e2d4d..79dd182552 100644 --- a/eth2/types/src/lib.rs +++ b/eth2/types/src/lib.rs @@ -15,6 +15,7 @@ pub mod eth1_data; pub mod eth1_data_vote; pub mod exit; pub mod fork; +pub mod free_attestation; pub mod pending_attestation; pub mod proposal_signed_data; pub mod proposer_slashing; @@ -47,6 +48,7 @@ pub use crate::eth1_data::Eth1Data; pub use crate::eth1_data_vote::Eth1DataVote; pub use crate::exit::Exit; pub use crate::fork::Fork; +pub use crate::free_attestation::FreeAttestation; pub use crate::pending_attestation::PendingAttestation; pub use crate::proposal_signed_data::ProposalSignedData; pub use crate::proposer_slashing::ProposerSlashing;