From 494d7ad62b894ccd8344c74a95071e185518f874 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Sun, 5 May 2024 14:19:34 +0200 Subject: [PATCH] compute on chain aggregate, rename attestationref, and clean up some todos --- beacon_node/beacon_chain/src/beacon_chain.rs | 10 ++-- .../beacon_chain/src/observed_aggregates.rs | 33 +++++++---- beacon_node/operation_pool/src/attestation.rs | 20 +++---- .../operation_pool/src/attestation_storage.rs | 59 +++++++++++++++---- beacon_node/operation_pool/src/lib.rs | 8 +-- consensus/types/src/attestation.rs | 39 +++++++++++- 6 files changed, 129 insertions(+), 40 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index a99b5db6051..8bab50bde1d 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -85,7 +85,9 @@ use futures::channel::mpsc::Sender; use itertools::process_results; use itertools::Itertools; use kzg::Kzg; -use operation_pool::{AttestationRef, OperationPool, PersistedOperationPool, ReceivedPreCapella}; +use operation_pool::{ + CompactAttestationRef, OperationPool, PersistedOperationPool, ReceivedPreCapella, +}; use parking_lot::{Mutex, RwLock}; use proto_array::{DoNotReOrg, ProposerHeadError}; use safe_arith::SafeArith; @@ -2284,7 +2286,7 @@ impl BeaconChain { pub fn filter_op_pool_attestation( &self, filter_cache: &mut HashMap<(Hash256, Epoch), bool>, - att: &AttestationRef, + att: &CompactAttestationRef, state: &BeaconState, ) -> bool { *filter_cache @@ -4895,11 +4897,11 @@ impl BeaconChain { initialize_epoch_cache(&mut state, &self.spec)?; let mut prev_filter_cache = HashMap::new(); - let prev_attestation_filter = |att: &AttestationRef| { + let prev_attestation_filter = |att: &CompactAttestationRef| { self.filter_op_pool_attestation(&mut prev_filter_cache, att, &state) }; let mut curr_filter_cache = HashMap::new(); - let curr_attestation_filter = |att: &AttestationRef| { + let curr_attestation_filter = |att: &CompactAttestationRef| { self.filter_op_pool_attestation(&mut curr_filter_cache, att, &state) }; diff --git a/beacon_node/beacon_chain/src/observed_aggregates.rs b/beacon_node/beacon_chain/src/observed_aggregates.rs index 857b0edb348..78d13b38e4b 100644 --- a/beacon_node/beacon_chain/src/observed_aggregates.rs +++ b/beacon_node/beacon_chain/src/observed_aggregates.rs @@ -20,7 +20,7 @@ pub type ObservedSyncContributions = ObservedAggregates< pub type ObservedAggregateAttestations = ObservedAggregates< Attestation, E, - BitList<::MaxValidatorsPerCommittee>, + BitList<::MaxValidatorsPerCommitteePerSlot>, >; /// A trait use to associate capacity constants with the type being stored in `ObservedAggregates`. @@ -103,29 +103,40 @@ pub trait SubsetItem { } impl<'a, E: EthSpec> SubsetItem for AttestationRef<'a, E> { - type Item = BitList; + type Item = BitList; fn is_subset(&self, other: &Self::Item) -> bool { match self { - Self::Base(att) => att.aggregation_bits.is_subset(other), - // TODO(electra) implement electra variant - Self::Electra(_) => todo!(), + Self::Base(att) => { + if let Ok(extended_aggregation_bits) = att.extend_aggregation_bits() { + return extended_aggregation_bits.is_subset(other) + } + false + }, + Self::Electra(att) => att.aggregation_bits.is_subset(other), } } fn is_superset(&self, other: &Self::Item) -> bool { match self { - Self::Base(att) => other.is_subset(&att.aggregation_bits), - // TODO(electra) implement electra variant - Self::Electra(_) => todo!(), + + Self::Base(att) => { + if let Ok(extended_aggregation_bits) = att.extend_aggregation_bits() { + return other.is_subset(&extended_aggregation_bits) + } + false + }, + Self::Electra(att) => other.is_subset(&att.aggregation_bits), } } /// Returns the sync contribution aggregation bits. fn get_item(&self) -> Self::Item { match self { - Self::Base(att) => att.aggregation_bits.clone(), - // TODO(electra) implement electra variant - Self::Electra(_) => todo!(), + Self::Base(att) => { + // TODO(electra) fix unwrap + att.extend_aggregation_bits().unwrap() + }, + Self::Electra(att) => att.aggregation_bits.clone(), } } diff --git a/beacon_node/operation_pool/src/attestation.rs b/beacon_node/operation_pool/src/attestation.rs index 60074cde740..c01d6b6b926 100644 --- a/beacon_node/operation_pool/src/attestation.rs +++ b/beacon_node/operation_pool/src/attestation.rs @@ -1,4 +1,4 @@ -use crate::attestation_storage::{AttestationRef, CompactIndexedAttestation}; +use crate::attestation_storage::{CompactAttestationRef, CompactIndexedAttestation}; use crate::max_cover::MaxCover; use crate::reward_cache::RewardCache; use state_processing::common::{ @@ -14,14 +14,14 @@ use types::{ #[derive(Debug, Clone)] pub struct AttMaxCover<'a, E: EthSpec> { /// Underlying attestation. - pub att: AttestationRef<'a, E>, + pub att: CompactAttestationRef<'a, E>, /// Mapping of validator indices and their rewards. pub fresh_validators_rewards: HashMap, } impl<'a, E: EthSpec> AttMaxCover<'a, E> { pub fn new( - att: AttestationRef<'a, E>, + att: CompactAttestationRef<'a, E>, state: &BeaconState, reward_cache: &'a RewardCache, total_active_balance: u64, @@ -36,7 +36,7 @@ impl<'a, E: EthSpec> AttMaxCover<'a, E> { /// Initialise an attestation cover object for base/phase0 hard fork. pub fn new_for_base( - att: AttestationRef<'a, E>, + att: CompactAttestationRef<'a, E>, state: &BeaconState, base_state: &BeaconStateBase, total_active_balance: u64, @@ -69,7 +69,7 @@ impl<'a, E: EthSpec> AttMaxCover<'a, E> { /// Initialise an attestation cover object for Altair or later. pub fn new_for_altair_deneb( - att: AttestationRef<'a, E>, + att: CompactAttestationRef<'a, E>, state: &BeaconState, reward_cache: &'a RewardCache, spec: &ChainSpec, @@ -119,14 +119,14 @@ impl<'a, E: EthSpec> AttMaxCover<'a, E> { impl<'a, E: EthSpec> MaxCover for AttMaxCover<'a, E> { type Object = Attestation; - type Intermediate = AttestationRef<'a, E>; + type Intermediate = CompactAttestationRef<'a, E>; type Set = HashMap; - fn intermediate(&self) -> &AttestationRef<'a, E> { + fn intermediate(&self) -> &CompactAttestationRef<'a, E> { &self.att } - fn convert_to_object(att_ref: &AttestationRef<'a, E>) -> Attestation { + fn convert_to_object(att_ref: &CompactAttestationRef<'a, E>) -> Attestation { att_ref.clone_as_attestation() } @@ -146,7 +146,7 @@ impl<'a, E: EthSpec> MaxCover for AttMaxCover<'a, E> { /// of slashable voting, which is rare. fn update_covering_set( &mut self, - best_att: &AttestationRef<'a, E>, + best_att: &CompactAttestationRef<'a, E>, covered_validators: &HashMap, ) { if self.att.data.slot == best_att.data.slot && self.att.data.index == best_att.data.index { @@ -170,7 +170,7 @@ impl<'a, E: EthSpec> MaxCover for AttMaxCover<'a, E> { /// /// This isn't optimal, but with the Altair fork this code is obsolete and not worth upgrading. pub fn earliest_attestation_validators( - attestation: &AttestationRef, + attestation: &CompactAttestationRef, state: &BeaconState, base_state: &BeaconStateBase, ) -> BitList { diff --git a/beacon_node/operation_pool/src/attestation_storage.rs b/beacon_node/operation_pool/src/attestation_storage.rs index 927e8c9b12c..f82aa27c919 100644 --- a/beacon_node/operation_pool/src/attestation_storage.rs +++ b/beacon_node/operation_pool/src/attestation_storage.rs @@ -43,7 +43,7 @@ pub struct SplitAttestation { } #[derive(Debug, Clone)] -pub struct AttestationRef<'a, E: EthSpec> { +pub struct CompactAttestationRef<'a, E: EthSpec> { pub checkpoint: &'a CheckpointKey, pub data: &'a CompactAttestationData, pub indexed: &'a CompactIndexedAttestation, @@ -81,8 +81,15 @@ impl SplitAttestation { index: data.index, }) } - // TODO(electra) implement electra variant - Attestation::Electra(_) => todo!(), + Attestation::Electra(attn) => { + CompactIndexedAttestation::Electra(CompactIndexedAttestationElectra { + attesting_indices, + aggregation_bits: attn.aggregation_bits, + signature: attestation.signature().clone(), + index: data.index, + committee_bits: attn.committee_bits, + }) + }, }; Self { @@ -92,8 +99,8 @@ impl SplitAttestation { } } - pub fn as_ref(&self) -> AttestationRef { - AttestationRef { + pub fn as_ref(&self) -> CompactAttestationRef { + CompactAttestationRef { checkpoint: &self.checkpoint, data: &self.data, indexed: &self.indexed, @@ -101,7 +108,7 @@ impl SplitAttestation { } } -impl<'a, E: EthSpec> AttestationRef<'a, E> { +impl<'a, E: EthSpec> CompactAttestationRef<'a, E> { pub fn attestation_data(&self) -> AttestationData { AttestationData { slot: self.data.slot, @@ -198,6 +205,38 @@ impl CompactIndexedAttestationBase { } } + +impl CompactIndexedAttestationElectra { + pub fn signers_disjoint_from(&self, other: &Self) -> bool { + if self + .committee_bits + .intersection(&other.committee_bits) + .is_zero() + { + if self + .aggregation_bits + .intersection(&other.aggregation_bits) + .is_zero() + { + return true; + } else { + for (index, committee_bit) in self.committee_bits.iter().enumerate() { + if committee_bit { + if let Ok(aggregation_bit) = self.aggregation_bits.get(index) { + if let Ok(other_aggregation_bit) = other.aggregation_bits.get(index) { + if aggregation_bit == other_aggregation_bit { + return false; + } + } + } + } + } + } + } + true + } +} + impl AttestationMap { pub fn insert(&mut self, attestation: Attestation, attesting_indices: Vec) { let SplitAttestation { @@ -231,7 +270,7 @@ impl AttestationMap { pub fn get_attestations<'a>( &'a self, checkpoint_key: &'a CheckpointKey, - ) -> impl Iterator> + 'a { + ) -> impl Iterator> + 'a { self.checkpoint_map .get(checkpoint_key) .into_iter() @@ -239,7 +278,7 @@ impl AttestationMap { } /// Iterate all attestations in the map. - pub fn iter(&self) -> impl Iterator> { + pub fn iter(&self) -> impl Iterator> { self.checkpoint_map .iter() .flat_map(|(checkpoint_key, attestation_map)| attestation_map.iter(checkpoint_key)) @@ -270,9 +309,9 @@ impl AttestationDataMap { pub fn iter<'a>( &'a self, checkpoint_key: &'a CheckpointKey, - ) -> impl Iterator> + 'a { + ) -> impl Iterator> + 'a { self.attestations.iter().flat_map(|(data, vec_indexed)| { - vec_indexed.iter().map(|indexed| AttestationRef { + vec_indexed.iter().map(|indexed| CompactAttestationRef { checkpoint: checkpoint_key, data, indexed, diff --git a/beacon_node/operation_pool/src/lib.rs b/beacon_node/operation_pool/src/lib.rs index f700eecf6bd..0a026b78292 100644 --- a/beacon_node/operation_pool/src/lib.rs +++ b/beacon_node/operation_pool/src/lib.rs @@ -11,7 +11,7 @@ mod sync_aggregate_id; pub use crate::bls_to_execution_changes::ReceivedPreCapella; pub use attestation::{earliest_attestation_validators, AttMaxCover}; -pub use attestation_storage::{AttestationRef, SplitAttestation}; +pub use attestation_storage::{CompactAttestationRef, SplitAttestation}; pub use max_cover::MaxCover; pub use persistence::{ PersistedOperationPool, PersistedOperationPoolV12, PersistedOperationPoolV14, @@ -228,7 +228,7 @@ impl OperationPool { state: &'a BeaconState, reward_cache: &'a RewardCache, total_active_balance: u64, - validity_filter: impl FnMut(&AttestationRef<'a, E>) -> bool + Send, + validity_filter: impl FnMut(&CompactAttestationRef<'a, E>) -> bool + Send, spec: &'a ChainSpec, ) -> impl Iterator> + Send { all_attestations @@ -252,8 +252,8 @@ impl OperationPool { pub fn get_attestations( &self, state: &BeaconState, - prev_epoch_validity_filter: impl for<'a> FnMut(&AttestationRef<'a, E>) -> bool + Send, - curr_epoch_validity_filter: impl for<'a> FnMut(&AttestationRef<'a, E>) -> bool + Send, + prev_epoch_validity_filter: impl for<'a> FnMut(&CompactAttestationRef<'a, E>) -> bool + Send, + curr_epoch_validity_filter: impl for<'a> FnMut(&CompactAttestationRef<'a, E>) -> bool + Send, spec: &ChainSpec, ) -> Result>, OpPoolError> { if !matches!(state, BeaconState::Base(_)) { diff --git a/consensus/types/src/attestation.rs b/consensus/types/src/attestation.rs index 0d3f29ae289..4e6d16429cc 100644 --- a/consensus/types/src/attestation.rs +++ b/consensus/types/src/attestation.rs @@ -224,9 +224,32 @@ impl<'a, E: EthSpec> AttestationRef<'a, E> { impl AttestationElectra { /// Are the aggregation bitfields of these attestations disjoint? pub fn signers_disjoint_from(&self, other: &Self) -> bool { - self.aggregation_bits + if self + .committee_bits + .intersection(&other.committee_bits) + .is_zero() + { + if self + .aggregation_bits .intersection(&other.aggregation_bits) .is_zero() + { + return true; + } else { + for (index, committee_bit) in self.committee_bits.iter().enumerate() { + if committee_bit { + if let Ok(aggregation_bit) = self.aggregation_bits.get(index) { + if let Ok(other_aggregation_bit) = other.aggregation_bits.get(index) { + if aggregation_bit == other_aggregation_bit { + return false; + } + } + } + } + } + } + } + true } pub fn committee_index(&self) -> u64 { @@ -247,7 +270,9 @@ impl AttestationElectra { pub fn aggregate(&mut self, other: &Self) { debug_assert_eq!(self.data, other.data); debug_assert!(self.signers_disjoint_from(other)); + // TODO(electra) add debug asset to check that committee bits are disjoint self.aggregation_bits = self.aggregation_bits.union(&other.aggregation_bits); + self.committee_bits = self.committee_bits.union(&other.committee_bits); self.signature.add_assign_aggregate(&other.signature); } @@ -364,6 +389,18 @@ impl AttestationBase { Ok(()) } } + + + pub fn extend_aggregation_bits(&self) -> Result, ssz_types::Error>{ + let mut extended_aggregation_bits: BitList = + BitList::with_capacity(self.aggregation_bits.len())?; + + for (i, bit) in self.aggregation_bits.iter().enumerate() { + extended_aggregation_bits + .set(i, bit)?; + } + Ok(extended_aggregation_bits) + } } impl SlotData for Attestation {