diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 5e83fdd81..614cc46d8 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1,4 +1,3 @@ -use crate::attestation_aggregator::{AttestationAggregator, Outcome as AggregationOutcome}; use crate::checkpoint::CheckPoint; use crate::errors::{BeaconChainError as Error, BlockProductionError}; use db::{ @@ -88,7 +87,6 @@ pub struct BeaconChain { pub block_store: Arc>, pub state_store: Arc>, pub slot_clock: U, - pub attestation_aggregator: RwLock, pub op_pool: OperationPool, canonical_head: RwLock, finalized_head: RwLock, @@ -131,7 +129,6 @@ where genesis_state.clone(), state_root, )); - let attestation_aggregator = RwLock::new(AttestationAggregator::new()); genesis_state.build_epoch_cache(RelativeEpoch::Previous, &spec)?; genesis_state.build_epoch_cache(RelativeEpoch::Current, &spec)?; @@ -142,7 +139,6 @@ where block_store, state_store, slot_clock, - attestation_aggregator, op_pool: OperationPool::new(), state: RwLock::new(genesis_state), finalized_head, @@ -477,7 +473,7 @@ where } /// Produce an `AttestationData` that is valid for the present `slot` and given `shard`. - pub fn produce_attestation(&self, shard: u64) -> Result { + pub fn produce_attestation_data(&self, shard: u64) -> Result { trace!("BeaconChain::produce_attestation: shard: {}", shard); let source_epoch = self.state.read().current_justified_epoch; let source_root = *self.state.read().get_block_root( @@ -509,33 +505,6 @@ where }) } - /// Validate a `FreeAttestation` and either: - /// - /// - Create a new `Attestation`. - /// - Aggregate it to an existing `Attestation`. - pub fn process_free_attestation( - &self, - free_attestation: FreeAttestation, - ) -> Result { - let aggregation_outcome = self - .attestation_aggregator - .write() - .process_free_attestation(&self.state.read(), &free_attestation, &self.spec)?; - - // return if the attestation is invalid - if !aggregation_outcome.valid { - return Ok(aggregation_outcome); - } - - // valid attestation, proceed with fork-choice logic - self.fork_choice.write().add_attestation( - free_attestation.validator_index, - &free_attestation.data.beacon_block_root, - &self.spec, - )?; - Ok(aggregation_outcome) - } - /// Accept a new attestation from the network. /// /// If valid, the attestation is added to the `op_pool` and aggregated with another attestation diff --git a/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness.rs b/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness.rs index 7d6a690e0..b7acac9e1 100644 --- a/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness.rs +++ b/beacon_node/beacon_chain/test_harness/src/beacon_chain_harness.rs @@ -10,8 +10,6 @@ use log::debug; use rayon::prelude::*; use slot_clock::TestingSlotClock; use ssz::TreeHash; -use std::collections::HashSet; -use std::iter::FromIterator; use std::sync::Arc; use types::{test_utils::TestingBeaconStateBuilder, *}; @@ -137,51 +135,64 @@ impl BeaconChainHarness { slot } - /// Gather the `FreeAttestation`s from the valiators. - /// - /// Note: validators will only produce attestations _once per slot_. So, if you call this twice - /// you'll only get attestations on the first run. - pub fn gather_free_attesations(&mut self) -> Vec { + pub fn gather_attesations(&mut self) -> Vec { let present_slot = self.beacon_chain.present_slot(); + let state = self.beacon_chain.state.read(); - let attesting_validators = self - .beacon_chain - .state - .read() + let mut attestations = vec![]; + + for committee in state .get_crosslink_committees_at_slot(present_slot, &self.spec) .unwrap() - .iter() - .fold(vec![], |mut acc, c| { - acc.append(&mut c.committee.clone()); - acc - }); - let attesting_validators: HashSet = - HashSet::from_iter(attesting_validators.iter().cloned()); + { + for &validator in &committee.committee { + let duties = state + .get_attestation_duties(validator, &self.spec) + .unwrap() + .expect("Attesting validators by definition have duties"); - let free_attestations: Vec = self - .validators - .par_iter_mut() - .enumerate() - .filter_map(|(i, validator)| { - if attesting_validators.contains(&i) { - // Advance the validator slot. - validator.set_slot(present_slot); + // Obtain `AttestationData` from the beacon chain. + let data = self + .beacon_chain + .produce_attestation_data(duties.shard) + .unwrap(); - // Prompt the validator to produce an attestation (if required). - validator.produce_free_attestation().ok() - } else { - None - } - }) - .collect(); + // Produce an aggregate signature with a single signature. + let aggregate_signature = { + let message = AttestationDataAndCustodyBit { + data: data.clone(), + custody_bit: false, + } + .hash_tree_root(); + let domain = self.spec.get_domain( + state.slot.epoch(self.spec.slots_per_epoch), + Domain::Attestation, + &state.fork, + ); + let sig = + Signature::new(&message, domain, &self.validators[validator].keypair.sk); - debug!( - "Gathered {} FreeAttestations for slot {}.", - free_attestations.len(), - present_slot - ); + let mut agg_sig = AggregateSignature::new(); + agg_sig.add(&sig); - free_attestations + agg_sig + }; + + let mut aggregation_bitfield = Bitfield::with_capacity(committee.committee.len()); + let custody_bitfield = Bitfield::with_capacity(committee.committee.len()); + + aggregation_bitfield.set(duties.committee_index, true); + + attestations.push(Attestation { + aggregation_bitfield, + data, + custody_bitfield, + aggregate_signature, + }) + } + } + + attestations } /// Get the block from the proposer for the slot. @@ -200,7 +211,9 @@ impl BeaconChainHarness { // Ensure the validators slot clock is accurate. self.validators[proposer].set_slot(present_slot); - self.validators[proposer].produce_block().unwrap() + let block = self.validators[proposer].produce_block().unwrap(); + + block } /// Advances the chain with a BeaconBlock and attestations from all validators. @@ -219,20 +232,23 @@ impl BeaconChainHarness { }; debug!("...block processed by BeaconChain."); - debug!("Producing free attestations..."); + debug!("Producing attestations..."); // Produce new attestations. - let free_attestations = self.gather_free_attesations(); + let attestations = self.gather_attesations(); - debug!("Processing free attestations..."); + debug!("Processing {} attestations...", attestations.len()); - free_attestations.par_iter().for_each(|free_attestation| { - self.beacon_chain - .process_free_attestation(free_attestation.clone()) - .unwrap(); - }); + attestations + .par_iter() + .enumerate() + .for_each(|(i, attestation)| { + self.beacon_chain + .process_attestation(attestation.clone()) + .expect(&format!("Attestation {} invalid: {:?}", i, attestation)); + }); - debug!("Free attestations processed."); + debug!("Attestations processed."); block } diff --git a/beacon_node/beacon_chain/test_harness/src/validator_harness/direct_beacon_node.rs b/beacon_node/beacon_chain/test_harness/src/validator_harness/direct_beacon_node.rs index fde8211ab..7853459d7 100644 --- a/beacon_node/beacon_chain/test_harness/src/validator_harness/direct_beacon_node.rs +++ b/beacon_node/beacon_chain/test_harness/src/validator_harness/direct_beacon_node.rs @@ -55,7 +55,7 @@ impl AttesterBeaconNode for DirectBeac _slot: Slot, shard: u64, ) -> Result, NodeError> { - match self.beacon_chain.produce_attestation(shard) { + match self.beacon_chain.produce_attestation_data(shard) { Ok(attestation_data) => Ok(Some(attestation_data)), Err(e) => Err(NodeError::RemoteFailure(format!("{:?}", e))), } diff --git a/eth2/types/src/attestation_duty.rs b/eth2/types/src/attestation_duty.rs index f6e86d263..80d912a83 100644 --- a/eth2/types/src/attestation_duty.rs +++ b/eth2/types/src/attestation_duty.rs @@ -1,7 +1,7 @@ use crate::*; use serde_derive::{Deserialize, Serialize}; -#[derive(Debug, PartialEq, Clone, Default, Serialize, Deserialize)] +#[derive(Debug, PartialEq, Clone, Copy, Default, Serialize, Deserialize)] pub struct AttestationDuty { pub slot: Slot, pub shard: Shard, diff --git a/eth2/types/src/test_utils/generate_deterministic_keypairs.rs b/eth2/types/src/test_utils/generate_deterministic_keypairs.rs index f2ce8709e..37880a988 100644 --- a/eth2/types/src/test_utils/generate_deterministic_keypairs.rs +++ b/eth2/types/src/test_utils/generate_deterministic_keypairs.rs @@ -19,7 +19,7 @@ pub fn generate_deterministic_keypairs(validator_count: usize) -> Vec { .collect::>() .par_iter() .map(|&i| { - let secret = int_to_bytes48(i as u64 + 1); + let secret = int_to_bytes48(i as u64 + 1000); let sk = SecretKey::from_bytes(&secret).unwrap(); let pk = PublicKey::from_secret_key(&sk); Keypair { sk, pk }