diff --git a/beacon_node/beacon_chain/Cargo.toml b/beacon_node/beacon_chain/Cargo.toml index abf1bc6472..b6f8d1ab8c 100644 --- a/beacon_node/beacon_chain/Cargo.toml +++ b/beacon_node/beacon_chain/Cargo.toml @@ -26,3 +26,6 @@ state_processing = { path = "../../eth2/state_processing" } tree_hash = { path = "../../eth2/utils/tree_hash" } types = { path = "../../eth2/types" } lmd_ghost = { path = "../../eth2/lmd_ghost" } + +[dev-dependencies] +rand = "0.5.5" diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 0137a0746b..2d82822701 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -6,7 +6,7 @@ use crate::persisted_beacon_chain::{PersistedBeaconChain, BEACON_CHAIN_DB_KEY}; use lmd_ghost::LmdGhost; use log::trace; use operation_pool::DepositInsertStatus; -use operation_pool::OperationPool; +use operation_pool::{OperationPool, PersistedOperationPool}; use parking_lot::{RwLock, RwLockReadGuard}; use slot_clock::SlotClock; use state_processing::per_block_processing::errors::{ @@ -147,11 +147,13 @@ impl BeaconChain { let last_finalized_root = p.canonical_head.beacon_state.finalized_root; let last_finalized_block = &p.canonical_head.beacon_block; + let op_pool = p.op_pool.into_operation_pool(&p.state, &spec); + Ok(Some(BeaconChain { spec, slot_clock, fork_choice: ForkChoice::new(store.clone(), last_finalized_block, last_finalized_root), - op_pool: OperationPool::default(), + op_pool, canonical_head: RwLock::new(p.canonical_head), state: RwLock::new(p.state), genesis_block_root: p.genesis_block_root, @@ -164,6 +166,7 @@ impl BeaconChain { pub fn persist(&self) -> Result<(), Error> { let p: PersistedBeaconChain = PersistedBeaconChain { canonical_head: self.canonical_head.read().clone(), + op_pool: PersistedOperationPool::from_operation_pool(&self.op_pool), genesis_block_root: self.genesis_block_root, state: self.state.read().clone(), }; @@ -506,8 +509,7 @@ impl BeaconChain { &self, deposit: Deposit, ) -> Result { - self.op_pool - .insert_deposit(deposit, &*self.state.read(), &self.spec) + self.op_pool.insert_deposit(deposit) } /// Accept some exit and queue it for inclusion in an appropriate block. diff --git a/beacon_node/beacon_chain/src/persisted_beacon_chain.rs b/beacon_node/beacon_chain/src/persisted_beacon_chain.rs index f5bdfdee15..479e1cd8e9 100644 --- a/beacon_node/beacon_chain/src/persisted_beacon_chain.rs +++ b/beacon_node/beacon_chain/src/persisted_beacon_chain.rs @@ -1,4 +1,5 @@ use crate::{BeaconChainTypes, CheckPoint}; +use operation_pool::PersistedOperationPool; use ssz::{Decode, Encode}; use ssz_derive::{Decode, Encode}; use store::{DBColumn, Error as StoreError, StoreItem}; @@ -10,7 +11,7 @@ pub const BEACON_CHAIN_DB_KEY: &str = "PERSISTEDBEACONCHAINPERSISTEDBEA"; #[derive(Encode, Decode)] pub struct PersistedBeaconChain { pub canonical_head: CheckPoint, - // TODO: operations pool. + pub op_pool: PersistedOperationPool, pub genesis_block_root: Hash256, pub state: BeaconState, } diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 164857e5d8..9b3f7c1cb6 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -14,6 +14,8 @@ use types::{ Hash256, Keypair, RelativeEpoch, SecretKey, Signature, Slot, }; +pub use crate::persisted_beacon_chain::{PersistedBeaconChain, BEACON_CHAIN_DB_KEY}; + /// Indicates how the `BeaconChainHarness` should produce blocks. #[derive(Clone, Copy, Debug)] pub enum BlockStrategy { @@ -68,8 +70,8 @@ where E: EthSpec, { pub chain: BeaconChain>, - keypairs: Vec, - spec: ChainSpec, + pub keypairs: Vec, + pub spec: ChainSpec, } impl BeaconChainHarness diff --git a/beacon_node/beacon_chain/tests/tests.rs b/beacon_node/beacon_chain/tests/tests.rs index 17e373ad6a..882d9f2355 100644 --- a/beacon_node/beacon_chain/tests/tests.rs +++ b/beacon_node/beacon_chain/tests/tests.rs @@ -1,16 +1,21 @@ #![cfg(not(debug_assertions))] -use beacon_chain::test_utils::{AttestationStrategy, BeaconChainHarness, BlockStrategy}; +use beacon_chain::test_utils::{ + AttestationStrategy, BeaconChainHarness, BlockStrategy, CommonTypes, PersistedBeaconChain, + BEACON_CHAIN_DB_KEY, +}; use lmd_ghost::ThreadSafeReducedTree; -use store::MemoryStore; -use types::{EthSpec, MinimalEthSpec, Slot}; +use rand::Rng; +use store::{MemoryStore, Store}; +use types::test_utils::{SeedableRng, TestRandom, XorShiftRng}; +use types::{Deposit, EthSpec, Hash256, MinimalEthSpec, Slot}; // Should ideally be divisible by 3. pub const VALIDATOR_COUNT: usize = 24; -fn get_harness( - validator_count: usize, -) -> BeaconChainHarness, MinimalEthSpec> { +type TestForkChoice = ThreadSafeReducedTree; + +fn get_harness(validator_count: usize) -> BeaconChainHarness { let harness = BeaconChainHarness::new(validator_count); // Move past the zero slot. @@ -225,3 +230,38 @@ fn does_not_finalize_without_attestation() { "no epoch should have been finalized" ); } + +#[test] +fn roundtrip_operation_pool() { + let num_blocks_produced = MinimalEthSpec::slots_per_epoch() * 5; + + let harness = get_harness(VALIDATOR_COUNT); + + // Add some attestations + harness.extend_chain( + num_blocks_produced as usize, + BlockStrategy::OnCanonicalHead, + AttestationStrategy::AllValidators, + ); + assert!(harness.chain.op_pool.num_attestations() > 0); + + // Add some deposits + let rng = &mut XorShiftRng::from_seed([66; 16]); + for _ in 0..rng.gen_range(1, VALIDATOR_COUNT) { + harness + .chain + .process_deposit(Deposit::random_for_test(rng)) + .unwrap(); + } + + // TODO: could add some other operations + harness.chain.persist().unwrap(); + + let key = Hash256::from_slice(&BEACON_CHAIN_DB_KEY.as_bytes()); + let p: PersistedBeaconChain> = + harness.chain.store.get(&key).unwrap().unwrap(); + + let restored_op_pool = p.op_pool.into_operation_pool(&p.state, &harness.spec); + + assert_eq!(harness.chain.op_pool, restored_op_pool); +} diff --git a/eth2/operation_pool/Cargo.toml b/eth2/operation_pool/Cargo.toml index 67d13013ca..770843b754 100644 --- a/eth2/operation_pool/Cargo.toml +++ b/eth2/operation_pool/Cargo.toml @@ -5,9 +5,11 @@ authors = ["Michael Sproul "] edition = "2018" [dependencies] +boolean-bitfield = { path = "../utils/boolean-bitfield" } int_to_bytes = { path = "../utils/int_to_bytes" } itertools = "0.8" parking_lot = "0.7" types = { path = "../types" } state_processing = { path = "../state_processing" } ssz = { path = "../utils/ssz" } +ssz_derive = { path = "../utils/ssz_derive" } diff --git a/eth2/operation_pool/src/attestation.rs b/eth2/operation_pool/src/attestation.rs new file mode 100644 index 0000000000..a2f71c3a48 --- /dev/null +++ b/eth2/operation_pool/src/attestation.rs @@ -0,0 +1,91 @@ +use crate::max_cover::MaxCover; +use boolean_bitfield::BooleanBitfield; +use types::{Attestation, BeaconState, EthSpec}; + +pub struct AttMaxCover<'a> { + /// Underlying attestation. + att: &'a Attestation, + /// Bitfield of validators that are covered by this attestation. + fresh_validators: BooleanBitfield, +} + +impl<'a> AttMaxCover<'a> { + pub fn new(att: &'a Attestation, fresh_validators: BooleanBitfield) -> Self { + Self { + att, + fresh_validators, + } + } +} + +impl<'a> MaxCover for AttMaxCover<'a> { + type Object = Attestation; + type Set = BooleanBitfield; + + fn object(&self) -> Attestation { + self.att.clone() + } + + fn covering_set(&self) -> &BooleanBitfield { + &self.fresh_validators + } + + /// Sneaky: we keep all the attestations together in one bucket, even though + /// their aggregation bitfields refer to different committees. In order to avoid + /// confusing committees when updating covering sets, we update only those attestations + /// whose shard and epoch match the attestation being included in the solution, by the logic + /// that a shard and epoch uniquely identify a committee. + fn update_covering_set( + &mut self, + best_att: &Attestation, + covered_validators: &BooleanBitfield, + ) { + if self.att.data.shard == best_att.data.shard + && self.att.data.target_epoch == best_att.data.target_epoch + { + self.fresh_validators.difference_inplace(covered_validators); + } + } + + fn score(&self) -> usize { + self.fresh_validators.num_set_bits() + } +} + +/// Extract the validators for which `attestation` would be their earliest in the epoch. +/// +/// The reward paid to a proposer for including an attestation is proportional to the number +/// of validators for which the included attestation is their first in the epoch. The attestation +/// is judged against the state's `current_epoch_attestations` or `previous_epoch_attestations` +/// depending on when it was created, and all those validators who have already attested are +/// removed from the `aggregation_bitfield` before returning it. +// TODO: This could be optimised with a map from validator index to whether that validator has +// attested in each of the current and previous epochs. Currently quadratic in number of validators. +pub fn earliest_attestation_validators( + attestation: &Attestation, + state: &BeaconState, +) -> BooleanBitfield { + // Bitfield of validators whose attestations are new/fresh. + let mut new_validators = attestation.aggregation_bitfield.clone(); + + let state_attestations = if attestation.data.target_epoch == state.current_epoch() { + &state.current_epoch_attestations + } else if attestation.data.target_epoch == state.previous_epoch() { + &state.previous_epoch_attestations + } else { + return BooleanBitfield::from_elem(attestation.aggregation_bitfield.len(), false); + }; + + state_attestations + .iter() + // In a single epoch, an attester should only be attesting for one shard. + // TODO: we avoid including slashable attestations in the state here, + // but maybe we should do something else with them (like construct slashings). + .filter(|existing_attestation| existing_attestation.data.shard == attestation.data.shard) + .for_each(|existing_attestation| { + // Remove the validators who have signed the existing attestation (they are not new) + new_validators.difference_inplace(&existing_attestation.aggregation_bitfield); + }); + + new_validators +} diff --git a/eth2/operation_pool/src/attestation_id.rs b/eth2/operation_pool/src/attestation_id.rs new file mode 100644 index 0000000000..a79023a699 --- /dev/null +++ b/eth2/operation_pool/src/attestation_id.rs @@ -0,0 +1,38 @@ +use int_to_bytes::int_to_bytes8; +use ssz::ssz_encode; +use ssz_derive::{Decode, Encode}; +use types::{AttestationData, BeaconState, ChainSpec, Domain, Epoch, EthSpec}; + +/// Serialized `AttestationData` augmented with a domain to encode the fork info. +#[derive(PartialEq, Eq, Clone, Hash, Debug, PartialOrd, Ord, Encode, Decode)] +pub struct AttestationId { + v: Vec, +} + +/// Number of domain bytes that the end of an attestation ID is padded with. +const DOMAIN_BYTES_LEN: usize = 8; + +impl AttestationId { + pub fn from_data( + attestation: &AttestationData, + state: &BeaconState, + spec: &ChainSpec, + ) -> Self { + let mut bytes = ssz_encode(attestation); + let epoch = attestation.target_epoch; + bytes.extend_from_slice(&AttestationId::compute_domain_bytes(epoch, state, spec)); + AttestationId { v: bytes } + } + + pub fn compute_domain_bytes( + epoch: Epoch, + state: &BeaconState, + spec: &ChainSpec, + ) -> Vec { + int_to_bytes8(spec.get_domain(epoch, Domain::Attestation, &state.fork)) + } + + pub fn domain_bytes_match(&self, domain_bytes: &[u8]) -> bool { + &self.v[self.v.len() - DOMAIN_BYTES_LEN..] == domain_bytes + } +} diff --git a/eth2/operation_pool/src/lib.rs b/eth2/operation_pool/src/lib.rs index ec7d5aa905..6c6f1e7524 100644 --- a/eth2/operation_pool/src/lib.rs +++ b/eth2/operation_pool/src/lib.rs @@ -1,13 +1,19 @@ -use int_to_bytes::int_to_bytes8; +mod attestation; +mod attestation_id; +mod max_cover; +mod persistence; + +pub use persistence::PersistedOperationPool; + +use attestation::{earliest_attestation_validators, AttMaxCover}; +use attestation_id::AttestationId; use itertools::Itertools; +use max_cover::maximum_cover; use parking_lot::RwLock; -use ssz::ssz_encode; use state_processing::per_block_processing::errors::{ AttestationValidationError, AttesterSlashingValidationError, DepositValidationError, ExitValidationError, ProposerSlashingValidationError, TransferValidationError, }; -#[cfg(not(test))] -use state_processing::per_block_processing::verify_deposit_merkle_proof; use state_processing::per_block_processing::{ get_slashable_indices_modular, validate_attestation, validate_attestation_time_independent_only, verify_attester_slashing, verify_exit, @@ -16,13 +22,12 @@ use state_processing::per_block_processing::{ }; use std::collections::{btree_map::Entry, hash_map, BTreeMap, HashMap, HashSet}; use std::marker::PhantomData; -use types::chain_spec::Domain; use types::{ - Attestation, AttestationData, AttesterSlashing, BeaconState, ChainSpec, Deposit, Epoch, - EthSpec, ProposerSlashing, Transfer, Validator, VoluntaryExit, + Attestation, AttesterSlashing, BeaconState, ChainSpec, Deposit, EthSpec, ProposerSlashing, + Transfer, Validator, VoluntaryExit, }; -#[derive(Default)] +#[derive(Default, Debug)] pub struct OperationPool { /// Map from attestation ID (see below) to vectors of attestations. attestations: RwLock>>, @@ -43,71 +48,6 @@ pub struct OperationPool { _phantom: PhantomData, } -/// Serialized `AttestationData` augmented with a domain to encode the fork info. -#[derive(PartialEq, Eq, Clone, Hash, Debug)] -struct AttestationId(Vec); - -/// Number of domain bytes that the end of an attestation ID is padded with. -const DOMAIN_BYTES_LEN: usize = 8; - -impl AttestationId { - fn from_data( - attestation: &AttestationData, - state: &BeaconState, - spec: &ChainSpec, - ) -> Self { - let mut bytes = ssz_encode(attestation); - let epoch = attestation.target_epoch; - bytes.extend_from_slice(&AttestationId::compute_domain_bytes(epoch, state, spec)); - AttestationId(bytes) - } - - fn compute_domain_bytes( - epoch: Epoch, - state: &BeaconState, - spec: &ChainSpec, - ) -> Vec { - int_to_bytes8(spec.get_domain(epoch, Domain::Attestation, &state.fork)) - } - - fn domain_bytes_match(&self, domain_bytes: &[u8]) -> bool { - &self.0[self.0.len() - DOMAIN_BYTES_LEN..] == domain_bytes - } -} - -/// Compute a fitness score for an attestation. -/// -/// The score is calculated by determining the number of *new* attestations that -/// the aggregate attestation introduces, and is proportional to the size of the reward we will -/// receive for including it in a block. -// TODO: this could be optimised with a map from validator index to whether that validator has -// attested in each of the current and previous epochs. Currently quadractic in number of validators. -fn attestation_score(attestation: &Attestation, state: &BeaconState) -> usize { - // Bitfield of validators whose attestations are new/fresh. - let mut new_validators = attestation.aggregation_bitfield.clone(); - - let state_attestations = if attestation.data.target_epoch == state.current_epoch() { - &state.current_epoch_attestations - } else if attestation.data.target_epoch == state.previous_epoch() { - &state.previous_epoch_attestations - } else { - return 0; - }; - - state_attestations - .iter() - // In a single epoch, an attester should only be attesting for one shard. - // TODO: we avoid including slashable attestations in the state here, - // but maybe we should do something else with them (like construct slashings). - .filter(|current_attestation| current_attestation.data.shard == attestation.data.shard) - .for_each(|current_attestation| { - // Remove the validators who have signed the existing attestation (they are not new) - new_validators.difference_inplace(¤t_attestation.aggregation_bitfield); - }); - - new_validators.num_set_bits() -} - #[derive(Debug, PartialEq, Clone)] pub enum DepositInsertStatus { /// The deposit was not already in the pool. @@ -176,29 +116,19 @@ impl OperationPool { let current_epoch = state.current_epoch(); let prev_domain_bytes = AttestationId::compute_domain_bytes(prev_epoch, state, spec); let curr_domain_bytes = AttestationId::compute_domain_bytes(current_epoch, state, spec); - self.attestations - .read() + let reader = self.attestations.read(); + let valid_attestations = reader .iter() .filter(|(key, _)| { key.domain_bytes_match(&prev_domain_bytes) || key.domain_bytes_match(&curr_domain_bytes) }) .flat_map(|(_, attestations)| attestations) - // That are not superseded by an attestation included in the state... - .filter(|attestation| !superior_attestation_exists_in_state(state, attestation)) // That are valid... .filter(|attestation| validate_attestation(state, attestation, spec).is_ok()) - // Scored by the number of new attestations they introduce (descending) - // TODO: need to consider attestations introduced in THIS block - .map(|att| (att, attestation_score(att, state))) - // Don't include any useless attestations (score 0) - .filter(|&(_, score)| score != 0) - .sorted_by_key(|&(_, score)| std::cmp::Reverse(score)) - // Limited to the maximum number of attestations per block - .take(spec.max_attestations as usize) - .map(|(att, _)| att) - .cloned() - .collect() + .map(|att| AttMaxCover::new(att, earliest_attestation_validators(att, state))); + + maximum_cover(valid_attestations, spec.max_attestations as usize) } /// Remove attestations which are too old to be included in a block. @@ -219,20 +149,14 @@ impl OperationPool { /// Add a deposit to the pool. /// /// No two distinct deposits should be added with the same index. - #[cfg_attr(test, allow(unused_variables))] pub fn insert_deposit( &self, deposit: Deposit, - state: &BeaconState, - spec: &ChainSpec, ) -> Result { use DepositInsertStatus::*; match self.deposits.write().entry(deposit.index) { Entry::Vacant(entry) => { - // TODO: fix tests to generate valid merkle proofs - #[cfg(not(test))] - verify_deposit_merkle_proof(state, &deposit, spec)?; entry.insert(deposit); Ok(Fresh) } @@ -240,9 +164,6 @@ impl OperationPool { if entry.get() == &deposit { Ok(Duplicate) } else { - // TODO: fix tests to generate valid merkle proofs - #[cfg(not(test))] - verify_deposit_merkle_proof(state, &deposit, spec)?; Ok(Replaced(Box::new(entry.insert(deposit)))) } } @@ -253,7 +174,9 @@ impl OperationPool { /// /// Take at most the maximum number of deposits, beginning from the current deposit index. pub fn get_deposits(&self, state: &BeaconState, spec: &ChainSpec) -> Vec { - // TODO: might want to re-check the Merkle proof to account for Eth1 forking + // TODO: We need to update the Merkle proofs for existing deposits as more deposits + // are added. It probably makes sense to construct the proofs from scratch when forming + // a block, using fresh info from the ETH1 chain for the current deposit root. let start_idx = state.deposit_index; (start_idx..start_idx + spec.max_deposits) .map(|idx| self.deposits.read().get(&idx).cloned()) @@ -484,34 +407,6 @@ impl OperationPool { } } -/// Returns `true` if the state already contains a `PendingAttestation` that is superior to the -/// given `attestation`. -/// -/// A validator has nothing to gain from re-including an attestation and it adds load to the -/// network. -/// -/// An existing `PendingAttestation` is superior to an existing `attestation` if: -/// -/// - Their `AttestationData` is equal. -/// - `attestation` does not contain any signatures that `PendingAttestation` does not have. -fn superior_attestation_exists_in_state( - state: &BeaconState, - attestation: &Attestation, -) -> bool { - state - .current_epoch_attestations - .iter() - .chain(state.previous_epoch_attestations.iter()) - .any(|existing_attestation| { - let bitfield = &attestation.aggregation_bitfield; - let existing_bitfield = &existing_attestation.aggregation_bitfield; - - existing_attestation.data == attestation.data - && bitfield.intersection(existing_bitfield).num_set_bits() - == bitfield.num_set_bits() - }) -} - /// Filter up to a maximum number of operations out of an iterator. fn filter_limit_operations<'a, T: 'a, I, F>(operations: I, filter: F, limit: u64) -> Vec where @@ -547,6 +442,18 @@ fn prune_validator_hash_map( }); } +/// Compare two operation pools. +impl PartialEq for OperationPool { + fn eq(&self, other: &Self) -> bool { + *self.attestations.read() == *other.attestations.read() + && *self.deposits.read() == *other.deposits.read() + && *self.attester_slashings.read() == *other.attester_slashings.read() + && *self.proposer_slashings.read() == *other.proposer_slashings.read() + && *self.voluntary_exits.read() == *other.voluntary_exits.read() + && *self.transfers.read() == *other.transfers.read() + } +} + #[cfg(test)] mod tests { use super::DepositInsertStatus::*; @@ -557,22 +464,15 @@ mod tests { #[test] fn insert_deposit() { let rng = &mut XorShiftRng::from_seed([42; 16]); - let (ref spec, ref state) = test_state(rng); - let op_pool = OperationPool::new(); + let op_pool = OperationPool::::new(); let deposit1 = make_deposit(rng); let mut deposit2 = make_deposit(rng); deposit2.index = deposit1.index; + assert_eq!(op_pool.insert_deposit(deposit1.clone()), Ok(Fresh)); + assert_eq!(op_pool.insert_deposit(deposit1.clone()), Ok(Duplicate)); assert_eq!( - op_pool.insert_deposit(deposit1.clone(), state, spec), - Ok(Fresh) - ); - assert_eq!( - op_pool.insert_deposit(deposit1.clone(), state, spec), - Ok(Duplicate) - ); - assert_eq!( - op_pool.insert_deposit(deposit2, state, spec), + op_pool.insert_deposit(deposit2), Ok(Replaced(Box::new(deposit1))) ); } @@ -591,10 +491,7 @@ mod tests { let deposits = dummy_deposits(rng, start, max_deposits + extra); for deposit in &deposits { - assert_eq!( - op_pool.insert_deposit(deposit.clone(), &state, &spec), - Ok(Fresh) - ); + assert_eq!(op_pool.insert_deposit(deposit.clone()), Ok(Fresh)); } state.deposit_index = start + offset; @@ -610,8 +507,7 @@ mod tests { #[test] fn prune_deposits() { let rng = &mut XorShiftRng::from_seed([42; 16]); - let (spec, state) = test_state(rng); - let op_pool = OperationPool::new(); + let op_pool = OperationPool::::new(); let start1 = 100; // test is super slow in debug mode if this parameter is too high @@ -623,7 +519,7 @@ mod tests { let deposits2 = dummy_deposits(rng, start2, count); for d in deposits1.into_iter().chain(deposits2) { - assert!(op_pool.insert_deposit(d, &state, &spec).is_ok()); + assert!(op_pool.insert_deposit(d).is_ok()); } assert_eq!(op_pool.num_deposits(), 2 * count as usize); @@ -734,15 +630,13 @@ mod tests { state_builder.teleport_to_slot(slot); state_builder.build_caches(&spec).unwrap(); let (state, keypairs) = state_builder.build(); - (state, keypairs, MainnetEthSpec::default_spec()) } #[test] - fn test_attestation_score() { + fn test_earliest_attestation() { let (ref mut state, ref keypairs, ref spec) = attestation_test_state::(1); - let slot = state.slot - 1; let committees = state .get_crosslink_committees_at_slot(slot) @@ -775,9 +669,8 @@ mod tests { assert_eq!( att1.aggregation_bitfield.num_set_bits(), - attestation_score(&att1, state) + earliest_attestation_validators(&att1, state).num_set_bits() ); - state.current_epoch_attestations.push(PendingAttestation { aggregation_bitfield: att1.aggregation_bitfield.clone(), data: att1.data.clone(), @@ -785,7 +678,10 @@ mod tests { proposer_index: 0, }); - assert_eq!(cc.committee.len() - 2, attestation_score(&att2, state)); + assert_eq!( + cc.committee.len() - 2, + earliest_attestation_validators(&att2, state).num_set_bits() + ); } } diff --git a/eth2/operation_pool/src/max_cover.rs b/eth2/operation_pool/src/max_cover.rs new file mode 100644 index 0000000000..75ac140546 --- /dev/null +++ b/eth2/operation_pool/src/max_cover.rs @@ -0,0 +1,189 @@ +/// Trait for types that we can compute a maximum cover for. +/// +/// Terminology: +/// * `item`: something that implements this trait +/// * `element`: something contained in a set, and covered by the covering set of an item +/// * `object`: something extracted from an item in order to comprise a solution +/// See: https://en.wikipedia.org/wiki/Maximum_coverage_problem +pub trait MaxCover { + /// The result type, of which we would eventually like a collection of maximal quality. + type Object; + /// The type used to represent sets. + type Set: Clone; + + /// Extract an object for inclusion in a solution. + fn object(&self) -> Self::Object; + + /// Get the set of elements covered. + fn covering_set(&self) -> &Self::Set; + /// Update the set of items covered, for the inclusion of some object in the solution. + fn update_covering_set(&mut self, max_obj: &Self::Object, max_set: &Self::Set); + /// The quality of this item's covering set, usually its cardinality. + fn score(&self) -> usize; +} + +/// Helper struct to track which items of the input are still available for inclusion. +/// Saves removing elements from the work vector. +struct MaxCoverItem { + item: T, + available: bool, +} + +impl MaxCoverItem { + fn new(item: T) -> Self { + MaxCoverItem { + item, + available: true, + } + } +} + +/// Compute an approximate maximum cover using a greedy algorithm. +/// +/// * Time complexity: `O(limit * items_iter.len())` +/// * Space complexity: `O(item_iter.len())` +pub fn maximum_cover<'a, I, T>(items_iter: I, limit: usize) -> Vec +where + I: IntoIterator, + T: MaxCover, +{ + // Construct an initial vec of all items, marked available. + let mut all_items: Vec<_> = items_iter + .into_iter() + .map(MaxCoverItem::new) + .filter(|x| x.item.score() != 0) + .collect(); + + let mut result = vec![]; + + for _ in 0..limit { + // Select the item with the maximum score. + let (best_item, best_cover) = match all_items + .iter_mut() + .filter(|x| x.available && x.item.score() != 0) + .max_by_key(|x| x.item.score()) + { + Some(x) => { + x.available = false; + (x.item.object(), x.item.covering_set().clone()) + } + None => return result, + }; + + // Update the covering sets of the other items, for the inclusion of the selected item. + // Items covered by the selected item can't be re-covered. + all_items + .iter_mut() + .filter(|x| x.available && x.item.score() != 0) + .for_each(|x| x.item.update_covering_set(&best_item, &best_cover)); + + result.push(best_item); + } + + result +} + +#[cfg(test)] +mod test { + use super::*; + use std::iter::FromIterator; + use std::{collections::HashSet, hash::Hash}; + + impl MaxCover for HashSet + where + T: Clone + Eq + Hash, + { + type Object = Self; + type Set = Self; + + fn object(&self) -> Self { + self.clone() + } + + fn covering_set(&self) -> &Self { + &self + } + + fn update_covering_set(&mut self, _: &Self, other: &Self) { + let mut difference = &*self - other; + std::mem::swap(self, &mut difference); + } + + fn score(&self) -> usize { + self.len() + } + } + + fn example_system() -> Vec> { + vec![ + HashSet::from_iter(vec![3]), + HashSet::from_iter(vec![1, 2, 4, 5]), + HashSet::from_iter(vec![1, 2, 4, 5]), + HashSet::from_iter(vec![1]), + HashSet::from_iter(vec![2, 4, 5]), + ] + } + + #[test] + fn zero_limit() { + let cover = maximum_cover(example_system(), 0); + assert_eq!(cover.len(), 0); + } + + #[test] + fn one_limit() { + let sets = example_system(); + let cover = maximum_cover(sets.clone(), 1); + assert_eq!(cover.len(), 1); + assert_eq!(cover[0], sets[1]); + } + + // Check that even if the limit provides room, we don't include useless items in the soln. + #[test] + fn exclude_zero_score() { + let sets = example_system(); + for k in 2..10 { + let cover = maximum_cover(sets.clone(), k); + assert_eq!(cover.len(), 2); + assert_eq!(cover[0], sets[1]); + assert_eq!(cover[1], sets[0]); + } + } + + fn quality(solution: &[HashSet]) -> usize { + solution.iter().map(HashSet::len).sum() + } + + // Optimal solution is the first three sets (quality 15) but our greedy algorithm + // will select the last three (quality 11). The comment at the end of each line + // shows that set's score at each iteration, with a * indicating that it will be chosen. + #[test] + fn suboptimal() { + let sets = vec![ + HashSet::from_iter(vec![0, 1, 8, 11, 14]), // 5, 3, 2 + HashSet::from_iter(vec![2, 3, 7, 9, 10]), // 5, 3, 2 + HashSet::from_iter(vec![4, 5, 6, 12, 13]), // 5, 4, 2 + HashSet::from_iter(vec![9, 10]), // 4, 4, 2* + HashSet::from_iter(vec![5, 6, 7, 8]), // 4, 4* + HashSet::from_iter(vec![0, 1, 2, 3, 4]), // 5* + ]; + let cover = maximum_cover(sets.clone(), 3); + assert_eq!(quality(&cover), 11); + } + + #[test] + fn intersecting_ok() { + let sets = vec![ + HashSet::from_iter(vec![1, 2, 3, 4, 5, 6, 7, 8]), + HashSet::from_iter(vec![1, 2, 3, 9, 10, 11]), + HashSet::from_iter(vec![4, 5, 6, 12, 13, 14]), + HashSet::from_iter(vec![7, 8, 15, 16, 17, 18]), + HashSet::from_iter(vec![1, 2, 9, 10]), + HashSet::from_iter(vec![1, 5, 6, 8]), + HashSet::from_iter(vec![1, 7, 11, 19]), + ]; + let cover = maximum_cover(sets.clone(), 5); + assert_eq!(quality(&cover), 19); + assert_eq!(cover.len(), 5); + } +} diff --git a/eth2/operation_pool/src/persistence.rs b/eth2/operation_pool/src/persistence.rs new file mode 100644 index 0000000000..aa6df597c8 --- /dev/null +++ b/eth2/operation_pool/src/persistence.rs @@ -0,0 +1,121 @@ +use crate::attestation_id::AttestationId; +use crate::OperationPool; +use parking_lot::RwLock; +use ssz_derive::{Decode, Encode}; +use types::*; + +/// SSZ-serializable version of `OperationPool`. +/// +/// Operations are stored in arbitrary order, so it's not a good idea to compare instances +/// of this type (or its encoded form) for equality. Convert back to an `OperationPool` first. +#[derive(Encode, Decode)] +pub struct PersistedOperationPool { + /// Mapping from attestation ID to attestation mappings. + // We could save space by not storing the attestation ID, but it might + // be difficult to make that roundtrip due to eager aggregation. + attestations: Vec<(AttestationId, Vec)>, + deposits: Vec, + /// Attester slashings. + attester_slashings: Vec, + /// Proposer slashings. + proposer_slashings: Vec, + /// Voluntary exits. + voluntary_exits: Vec, + /// Transfers. + transfers: Vec, +} + +impl PersistedOperationPool { + /// Convert an `OperationPool` into serializable form. + pub fn from_operation_pool(operation_pool: &OperationPool) -> Self { + let attestations = operation_pool + .attestations + .read() + .iter() + .map(|(att_id, att)| (att_id.clone(), att.clone())) + .collect(); + + let deposits = operation_pool + .deposits + .read() + .iter() + .map(|(_, d)| d.clone()) + .collect(); + + let attester_slashings = operation_pool + .attester_slashings + .read() + .iter() + .map(|(_, slashing)| slashing.clone()) + .collect(); + + let proposer_slashings = operation_pool + .proposer_slashings + .read() + .iter() + .map(|(_, slashing)| slashing.clone()) + .collect(); + + let voluntary_exits = operation_pool + .voluntary_exits + .read() + .iter() + .map(|(_, exit)| exit.clone()) + .collect(); + + let transfers = operation_pool.transfers.read().iter().cloned().collect(); + + Self { + attestations, + deposits, + attester_slashings, + proposer_slashings, + voluntary_exits, + transfers, + } + } + + /// Reconstruct an `OperationPool`. + pub fn into_operation_pool( + self, + state: &BeaconState, + spec: &ChainSpec, + ) -> OperationPool { + let attestations = RwLock::new(self.attestations.into_iter().collect()); + let deposits = RwLock::new(self.deposits.into_iter().map(|d| (d.index, d)).collect()); + let attester_slashings = RwLock::new( + self.attester_slashings + .into_iter() + .map(|slashing| { + ( + OperationPool::attester_slashing_id(&slashing, state, spec), + slashing, + ) + }) + .collect(), + ); + let proposer_slashings = RwLock::new( + self.proposer_slashings + .into_iter() + .map(|slashing| (slashing.proposer_index, slashing)) + .collect(), + ); + let voluntary_exits = RwLock::new( + self.voluntary_exits + .into_iter() + .map(|exit| (exit.validator_index, exit)) + .collect(), + ); + let transfers = RwLock::new(self.transfers.into_iter().collect()); + + OperationPool { + attestations, + deposits, + attester_slashings, + proposer_slashings, + voluntary_exits, + transfers, + _phantom: Default::default(), + } + } +} diff --git a/eth2/utils/boolean-bitfield/src/lib.rs b/eth2/utils/boolean-bitfield/src/lib.rs index 08e56e7c3f..ac6ffa89a5 100644 --- a/eth2/utils/boolean-bitfield/src/lib.rs +++ b/eth2/utils/boolean-bitfield/src/lib.rs @@ -13,7 +13,7 @@ use std::default; /// A BooleanBitfield represents a set of booleans compactly stored as a vector of bits. /// The BooleanBitfield is given a fixed size during construction. Reads outside of the current size return an out-of-bounds error. Writes outside of the current size expand the size of the set. -#[derive(Debug, Clone)] +#[derive(Debug, Clone, Hash)] pub struct BooleanBitfield(BitVec); /// Error represents some reason a request against a bitfield was not satisfied @@ -170,6 +170,7 @@ impl cmp::PartialEq for BooleanBitfield { ssz::ssz_encode(self) == ssz::ssz_encode(other) } } +impl Eq for BooleanBitfield {} /// Create a new bitfield that is a union of two other bitfields. /// diff --git a/eth2/utils/ssz/src/decode/impls.rs b/eth2/utils/ssz/src/decode/impls.rs index 75dd5d444c..6f79869459 100644 --- a/eth2/utils/ssz/src/decode/impls.rs +++ b/eth2/utils/ssz/src/decode/impls.rs @@ -41,6 +41,153 @@ impl_decodable_for_uint!(usize, 32); #[cfg(target_pointer_width = "64")] impl_decodable_for_uint!(usize, 64); +macro_rules! impl_decode_for_tuples { + ($( + $Tuple:ident { + $(($idx:tt) -> $T:ident)+ + } + )+) => { + $( + impl<$($T: Decode),+> Decode for ($($T,)+) { + fn is_ssz_fixed_len() -> bool { + $( + <$T as Decode>::is_ssz_fixed_len() && + )* + true + } + + fn ssz_fixed_len() -> usize { + if ::is_ssz_fixed_len() { + $( + <$T as Decode>::ssz_fixed_len() + + )* + 0 + } else { + BYTES_PER_LENGTH_OFFSET + } + } + + fn from_ssz_bytes(bytes: &[u8]) -> Result { + let mut builder = SszDecoderBuilder::new(bytes); + + $( + builder.register_type::<$T>()?; + )* + + let mut decoder = builder.build()?; + + Ok(($( + decoder.decode_next::<$T>()?, + )* + )) + } + } + )+ + } +} + +impl_decode_for_tuples! { + Tuple2 { + (0) -> A + (1) -> B + } + Tuple3 { + (0) -> A + (1) -> B + (2) -> C + } + Tuple4 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + } + Tuple5 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + (4) -> E + } + Tuple6 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + (4) -> E + (5) -> F + } + Tuple7 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + (4) -> E + (5) -> F + (6) -> G + } + Tuple8 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + (4) -> E + (5) -> F + (6) -> G + (7) -> H + } + Tuple9 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + (4) -> E + (5) -> F + (6) -> G + (7) -> H + (8) -> I + } + Tuple10 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + (4) -> E + (5) -> F + (6) -> G + (7) -> H + (8) -> I + (9) -> J + } + Tuple11 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + (4) -> E + (5) -> F + (6) -> G + (7) -> H + (8) -> I + (9) -> J + (10) -> K + } + Tuple12 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + (4) -> E + (5) -> F + (6) -> G + (7) -> H + (8) -> I + (9) -> J + (10) -> K + (11) -> L + } +} + impl Decode for bool { fn is_ssz_fixed_len() -> bool { true @@ -520,4 +667,15 @@ mod tests { }) ); } + + #[test] + fn tuple() { + assert_eq!(<(u16, u16)>::from_ssz_bytes(&[0, 0, 0, 0]), Ok((0, 0))); + assert_eq!(<(u16, u16)>::from_ssz_bytes(&[16, 0, 17, 0]), Ok((16, 17))); + assert_eq!(<(u16, u16)>::from_ssz_bytes(&[0, 1, 2, 0]), Ok((256, 2))); + assert_eq!( + <(u16, u16)>::from_ssz_bytes(&[255, 255, 0, 0]), + Ok((65535, 0)) + ); + } } diff --git a/eth2/utils/ssz/src/encode/impls.rs b/eth2/utils/ssz/src/encode/impls.rs index e0e2d9dbc1..3d68d8911a 100644 --- a/eth2/utils/ssz/src/encode/impls.rs +++ b/eth2/utils/ssz/src/encode/impls.rs @@ -31,6 +31,154 @@ impl_encodable_for_uint!(usize, 32); #[cfg(target_pointer_width = "64")] impl_encodable_for_uint!(usize, 64); +// Based on the `tuple_impls` macro from the standard library. +macro_rules! impl_encode_for_tuples { + ($( + $Tuple:ident { + $(($idx:tt) -> $T:ident)+ + } + )+) => { + $( + impl<$($T: Encode),+> Encode for ($($T,)+) { + fn is_ssz_fixed_len() -> bool { + $( + <$T as Encode>::is_ssz_fixed_len() && + )* + true + } + + fn ssz_fixed_len() -> usize { + if ::is_ssz_fixed_len() { + $( + <$T as Encode>::ssz_fixed_len() + + )* + 0 + } else { + BYTES_PER_LENGTH_OFFSET + } + } + + fn ssz_append(&self, buf: &mut Vec) { + let offset = $( + <$T as Encode>::ssz_fixed_len() + + )* + 0; + + let mut encoder = SszEncoder::container(buf, offset); + + $( + encoder.append(&self.$idx); + )* + + encoder.finalize(); + } + } + )+ + } +} + +impl_encode_for_tuples! { + Tuple2 { + (0) -> A + (1) -> B + } + Tuple3 { + (0) -> A + (1) -> B + (2) -> C + } + Tuple4 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + } + Tuple5 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + (4) -> E + } + Tuple6 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + (4) -> E + (5) -> F + } + Tuple7 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + (4) -> E + (5) -> F + (6) -> G + } + Tuple8 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + (4) -> E + (5) -> F + (6) -> G + (7) -> H + } + Tuple9 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + (4) -> E + (5) -> F + (6) -> G + (7) -> H + (8) -> I + } + Tuple10 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + (4) -> E + (5) -> F + (6) -> G + (7) -> H + (8) -> I + (9) -> J + } + Tuple11 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + (4) -> E + (5) -> F + (6) -> G + (7) -> H + (8) -> I + (9) -> J + (10) -> K + } + Tuple12 { + (0) -> A + (1) -> B + (2) -> C + (3) -> D + (4) -> E + (5) -> F + (6) -> G + (7) -> H + (8) -> I + (9) -> J + (10) -> K + (11) -> L + } +} + /// The SSZ "union" type. impl Encode for Option { fn is_ssz_fixed_len() -> bool { @@ -292,4 +440,11 @@ mod tests { assert_eq!([1, 0, 0, 0].as_ssz_bytes(), vec![1, 0, 0, 0]); assert_eq!([1, 2, 3, 4].as_ssz_bytes(), vec![1, 2, 3, 4]); } + + #[test] + fn tuple() { + assert_eq!((10u8, 11u8).as_ssz_bytes(), vec![10, 11]); + assert_eq!((10u32, 11u8).as_ssz_bytes(), vec![10, 0, 0, 0, 11]); + assert_eq!((10u8, 11u8, 12u8).as_ssz_bytes(), vec![10, 11, 12]); + } } diff --git a/eth2/utils/ssz/tests/tests.rs b/eth2/utils/ssz/tests/tests.rs index 9447cf5372..c19e366622 100644 --- a/eth2/utils/ssz/tests/tests.rs +++ b/eth2/utils/ssz/tests/tests.rs @@ -346,4 +346,34 @@ mod round_trip { round_trip(vec); } + + #[test] + fn tuple_u8_u16() { + let vec: Vec<(u8, u16)> = vec![ + (0, 0), + (0, 1), + (1, 0), + (u8::max_value(), u16::max_value()), + (0, u16::max_value()), + (u8::max_value(), 0), + (42, 12301), + ]; + + round_trip(vec); + } + + #[test] + fn tuple_vec_vec() { + let vec: Vec<(u64, Vec, Vec>)> = vec![ + (0, vec![], vec![vec![]]), + (99, vec![101], vec![vec![], vec![]]), + ( + 42, + vec![12, 13, 14], + vec![vec![99, 98, 97, 96], vec![42, 44, 46, 48, 50]], + ), + ]; + + round_trip(vec); + } }