Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix failing attestation tests and misc electra attestation cleanup #5810

Merged
Merged
187 changes: 138 additions & 49 deletions beacon_node/beacon_chain/src/attestation_verification.rs

Large diffs are not rendered by default.

Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ where
.ok_or(BeaconChainError::ValidatorPubkeyCacheLockTimeout)?;

let mut signature_sets = Vec::with_capacity(num_indexed * 3);

// Iterate, flattening to get only the `Ok` values.
for indexed in indexing_results.iter().flatten() {
let signed_aggregate = &indexed.signed_aggregate;
Expand Down
28 changes: 22 additions & 6 deletions beacon_node/beacon_chain/src/beacon_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -1609,6 +1611,20 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
Ok((duties, dependent_root, execution_status))
}

pub fn get_aggregated_attestation(
&self,
attestation: &AttestationRef<T::EthSpec>,
) -> Result<Option<Attestation<T::EthSpec>>, Error> {
match attestation {
AttestationRef::Base(att) => self.get_aggregated_attestation_base(&att.data),
AttestationRef::Electra(att) => self.get_aggregated_attestation_electra(
att.data.slot,
&att.data.tree_hash_root(),
att.committee_index().ok_or(Error::AttestationCommitteeIndexNotSet)?,
),
}
}

/// Returns an aggregated `Attestation`, if any, that has a matching `attestation.data`.
///
/// The attestation will be obtained from `self.naive_aggregation_pool`.
Expand Down Expand Up @@ -2185,7 +2201,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
self.log,
"Stored unaggregated attestation";
"outcome" => ?outcome,
"index" => attestation.data().index,
"index" => attestation.committee_index(),
"slot" => attestation.data().slot.as_u64(),
),
Err(NaiveAggregationError::SlotTooLow {
Expand All @@ -2204,7 +2220,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
self.log,
"Failed to store unaggregated attestation";
"error" => ?e,
"index" => attestation.data().index,
"index" => attestation.committee_index(),
"slot" => attestation.data().slot.as_u64(),
);
return Err(Error::from(e).into());
Expand Down Expand Up @@ -2329,7 +2345,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
pub fn filter_op_pool_attestation(
&self,
filter_cache: &mut HashMap<(Hash256, Epoch), bool>,
att: &AttestationRef<T::EthSpec>,
att: &CompactAttestationRef<T::EthSpec>,
state: &BeaconState<T::EthSpec>,
) -> bool {
*filter_cache
Expand Down Expand Up @@ -4940,11 +4956,11 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
initialize_epoch_cache(&mut state, &self.spec)?;

let mut prev_filter_cache = HashMap::new();
let prev_attestation_filter = |att: &AttestationRef<T::EthSpec>| {
let prev_attestation_filter = |att: &CompactAttestationRef<T::EthSpec>| {
self.filter_op_pool_attestation(&mut prev_filter_cache, att, &state)
};
let mut curr_filter_cache = HashMap::new();
let curr_attestation_filter = |att: &AttestationRef<T::EthSpec>| {
let curr_attestation_filter = |att: &CompactAttestationRef<T::EthSpec>| {
self.filter_op_pool_attestation(&mut curr_filter_cache, att, &state)
};

Expand Down
1 change: 0 additions & 1 deletion beacon_node/beacon_chain/src/early_attester_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,6 @@ impl<E: EthSpec> EarlyAttesterCache<E> {
item.committee_lengths
.get_committee_length::<E>(request_slot, request_index, spec)?;

// TODO(electra) make fork-agnostic
let attestation = if spec.fork_name_at_slot::<E>(request_slot) >= ForkName::Electra {
let mut committee_bits = BitVector::default();
if committee_len > 0 {
Expand Down
3 changes: 3 additions & 0 deletions beacon_node/beacon_chain/src/errors.rs
Original file line number Diff line number Diff line change
Expand Up @@ -226,6 +226,8 @@ pub enum BeaconChainError {
LightClientError(LightClientError),
UnsupportedFork,
MilhouseError(MilhouseError),
AttestationError(AttestationError),
AttestationCommitteeIndexNotSet,
}

easy_from_to!(SlotProcessingError, BeaconChainError);
Expand Down Expand Up @@ -256,6 +258,7 @@ easy_from_to!(AvailabilityCheckError, BeaconChainError);
easy_from_to!(EpochCacheError, BeaconChainError);
easy_from_to!(LightClientError, BeaconChainError);
easy_from_to!(MilhouseError, BeaconChainError);
easy_from_to!(AttestationError, BeaconChainError);

#[derive(Debug)]
pub enum BlockProductionError {
Expand Down
6 changes: 3 additions & 3 deletions beacon_node/beacon_chain/src/naive_aggregation_pool.rs
Original file line number Diff line number Diff line change
Expand Up @@ -261,9 +261,9 @@ impl<E: EthSpec> AggregateMap for AggregatedAttestationMap<E> {
};

let attestation_key = AttestationKey::from_attestation_ref(a)?;
let attestation_data_root = attestation_key.tree_hash_root();
let attestation_key_root = attestation_key.tree_hash_root();

if let Some(existing_attestation) = self.map.get_mut(&attestation_data_root) {
if let Some(existing_attestation) = self.map.get_mut(&attestation_key_root) {
if existing_attestation
.get_aggregation_bit(aggregation_bit)
.map_err(|_| Error::InconsistentBitfieldLengths)?
Expand All @@ -285,7 +285,7 @@ impl<E: EthSpec> AggregateMap for AggregatedAttestationMap<E> {
}

self.map
.insert(attestation_data_root, a.clone_as_attestation());
.insert(attestation_key_root, a.clone_as_attestation());
Ok(InsertOutcome::NewItemInserted {
committee_index: aggregation_bit,
})
Expand Down
Loading
Loading