Skip to content

Commit

Permalink
fix hanging tests
Browse files Browse the repository at this point in the history
  • Loading branch information
@eserilev
eserilev committed May 6, 2024
1 parent cfefe14 commit 5a647d0
Show file tree
Hide file tree
Showing 17 changed files with 450 additions and 171 deletions.
129 changes: 96 additions & 33 deletions beacon_node/beacon_chain/src/attestation_verification.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -40,12 +40,13 @@ use crate::{
BeaconChain, BeaconChainError, BeaconChainTypes,
};
use bls::verify_signature_sets;
use itertools::Itertools;
use proto_array::Block as ProtoBlock;
use slog::debug;
use slot_clock::SlotClock;
use state_processing::{
common::get_indexed_attestation,
per_block_processing::errors::AttestationValidationError,
common::{attesting_indices_base, attesting_indices_electra},
per_block_processing::errors::{AttestationValidationError, BlockOperationError},
signature_sets::{
indexed_attestation_signature_set_from_pubkeys,
signed_aggregate_selection_proof_signature_set, signed_aggregate_signature_set,
Expand All @@ -55,8 +56,9 @@ use std::borrow::Cow;
use strum::AsRefStr;
use tree_hash::TreeHash;
use types::{
Attestation, BeaconCommittee, ChainSpec, CommitteeIndex, Epoch, EthSpec, ForkName, Hash256,
IndexedAttestation, SelectionProof, SignedAggregateAndProof, Slot, SubnetId,
Attestation, BeaconCommittee, BeaconStateError::NoCommitteeFound, ChainSpec, CommitteeIndex,
Epoch, EthSpec, ForkName, Hash256, IndexedAttestation, SelectionProof, SignedAggregateAndProof,
Slot, SubnetId,
};

pub use batch::{batch_verify_aggregated_attestations, batch_verify_unaggregated_attestations};
Expand Down Expand Up @@ -540,27 +542,54 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
};

let get_indexed_attestation_with_committee =
|(committee, _): (BeaconCommittee, CommitteesPerSlot)| {
// Note: this clones the signature which is known to be a relatively slow operation.
//
// Future optimizations should remove this clone.
let selection_proof =
SelectionProof::from(signed_aggregate.message.selection_proof.clone());

if !selection_proof
.is_aggregator(committee.committee.len(), &chain.spec)
.map_err(|e| Error::BeaconChainError(e.into()))?
{
return Err(Error::InvalidSelectionProof { aggregator_index });
}

// Ensure the aggregator is a member of the committee for which it is aggregating.
if !committee.committee.contains(&(aggregator_index as usize)) {
return Err(Error::AggregatorNotInCommittee { aggregator_index });
|(committees, _): (Vec<BeaconCommittee>, CommitteesPerSlot)| {
match attestation {
Attestation::Base(att) => {
let committee = committees
.iter()
.filter(|&committee| committee.index == att.data.index)
.at_most_one()
.map_err(|_| Error::NoCommitteeForSlotAndIndex {
slot: att.data.slot,
index: att.data.index,
})?;

if let Some(committee) = committee {
// Note: this clones the signature which is known to be a relatively slow operation.
//
// Future optimizations should remove this clone.
let selection_proof = SelectionProof::from(
signed_aggregate.message.selection_proof.clone(),
);

if !selection_proof
.is_aggregator(committee.committee.len(), &chain.spec)
.map_err(|e| Error::BeaconChainError(e.into()))?
{
return Err(Error::InvalidSelectionProof { aggregator_index });
}

// Ensure the aggregator is a member of the committee for which it is aggregating.
if !committee.committee.contains(&(aggregator_index as usize)) {
return Err(Error::AggregatorNotInCommittee { aggregator_index });
}
attesting_indices_base::get_indexed_attestation(
committee.committee,
att,
)
.map_err(|e| BeaconChainError::from(e).into())
} else {
Err(Error::NoCommitteeForSlotAndIndex {
slot: att.data.slot,
index: att.data.index,
})
}
}
Attestation::Electra(att) => {
attesting_indices_electra::get_indexed_attestation(&committees, att)
.map_err(|e| BeaconChainError::from(e).into())
}
}

get_indexed_attestation(committee.committee, attestation.to_ref())
.map_err(|e| BeaconChainError::from(e).into())
};

let indexed_attestation = match map_attestation_committee(
Expand Down Expand Up @@ -1242,10 +1271,44 @@ pub fn obtain_indexed_attestation_and_committees_per_slot<T: BeaconChainTypes>(
chain: &BeaconChain<T>,
attestation: &Attestation<T::EthSpec>,
) -> Result<(IndexedAttestation<T::EthSpec>, CommitteesPerSlot), Error> {
map_attestation_committee(chain, attestation, |(committee, committees_per_slot)| {
get_indexed_attestation(committee.committee, attestation.to_ref())
.map(|attestation| (attestation, committees_per_slot))
.map_err(Error::Invalid)
map_attestation_committee(chain, attestation, |(committees, committees_per_slot)| {
match attestation {
Attestation::Base(att) => {
let committee = committees
.iter()
.filter(|&committee| committee.index == att.data.index)
.at_most_one()
.map_err(|_| Error::NoCommitteeForSlotAndIndex {
slot: att.data.slot,
index: att.data.index,
})?;

if let Some(committee) = committee {
attesting_indices_base::get_indexed_attestation(committee.committee, att)
.map(|attestation| (attestation, committees_per_slot))
.map_err(Error::Invalid)
} else {
Err(Error::NoCommitteeForSlotAndIndex {
slot: att.data.slot,
index: att.data.index,
})
}
}
Attestation::Electra(att) => {
attesting_indices_electra::get_indexed_attestation(&committees, att)
.map(|attestation| (attestation, committees_per_slot))
.map_err(|e| {
if e == BlockOperationError::BeaconStateError(NoCommitteeFound) {
Error::NoCommitteeForSlotAndIndex {
slot: att.data.slot,
index: att.committee_index(),
}
} else {
Error::Invalid(e)
}
})
}
}
})
}

Expand All @@ -1265,7 +1328,7 @@ fn map_attestation_committee<T, F, R>(
) -> Result<R, Error>
where
T: BeaconChainTypes,
F: Fn((BeaconCommittee, CommitteesPerSlot)) -> Result<R, Error>,
F: Fn((Vec<BeaconCommittee>, CommitteesPerSlot)) -> Result<R, Error>,
{
let attestation_epoch = attestation.data().slot.epoch(T::EthSpec::slots_per_epoch());
let target = &attestation.data().target;
Expand All @@ -1291,12 +1354,12 @@ where
let committees_per_slot = committee_cache.committees_per_slot();

Ok(committee_cache
.get_beacon_committee(attestation.data().slot, attestation.data().index)
.map(|committee| map_fn((committee, committees_per_slot)))
.unwrap_or_else(|| {
.get_beacon_committees_at_slot(attestation.data().slot)
.map(|committees| map_fn((committees, committees_per_slot)))
.unwrap_or_else(|_| {
Err(Error::NoCommitteeForSlotAndIndex {
slot: attestation.data().slot,
index: attestation.data().index,
index: attestation.committee_index(),
})
}))
})
Expand Down
42 changes: 30 additions & 12 deletions beacon_node/beacon_chain/src/beacon_chain.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1919,18 +1919,36 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
};
drop(cache_timer);

// TODO(electra) implement electra variant
Ok(Attestation::Base(AttestationBase {
aggregation_bits: BitList::with_capacity(committee_len)?,
data: AttestationData {
slot: request_slot,
index: request_index,
beacon_block_root,
source: justified_checkpoint,
target,
},
signature: AggregateSignature::empty(),
}))
if self.spec.fork_name_at_slot::<T::EthSpec>(request_slot) >= ForkName::Electra {
let mut committee_bits = BitVector::default();
if committee_len > 0 {
committee_bits.set(request_index as usize, true)?;
}
Ok(Attestation::Electra(AttestationElectra {
aggregation_bits: BitList::with_capacity(committee_len)?,
data: AttestationData {
slot: request_slot,
index: 0u64,
beacon_block_root,
source: justified_checkpoint,
target,
},
committee_bits,
signature: AggregateSignature::empty(),
}))
} else {
Ok(Attestation::Base(AttestationBase {
aggregation_bits: BitList::with_capacity(committee_len)?,
data: AttestationData {
slot: request_slot,
index: request_index,
beacon_block_root,
source: justified_checkpoint,
target,
},
signature: AggregateSignature::empty(),
}))
}
}

/// Performs the same validation as `Self::verify_unaggregated_attestation_for_gossip`, but for
Expand Down
47 changes: 34 additions & 13 deletions beacon_node/beacon_chain/src/early_attester_cache.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -123,19 +123,40 @@ impl<E: EthSpec> EarlyAttesterCache<E> {
item.committee_lengths
.get_committee_length::<E>(request_slot, request_index, spec)?;

// TODO(electra) make fork-agnostic
let attestation = Attestation::Base(AttestationBase {
aggregation_bits: BitList::with_capacity(committee_len)
.map_err(BeaconStateError::from)?,
data: AttestationData {
slot: request_slot,
index: request_index,
beacon_block_root: item.beacon_block_root,
source: item.source,
target: item.target,
},
signature: AggregateSignature::empty(),
});
let attestation = if spec.fork_name_at_slot::<E>(request_slot) >= ForkName::Electra {
let mut committee_bits = BitVector::default();
if committee_len > 0 {
committee_bits
.set(request_index as usize, true)
.map_err(BeaconStateError::from)?;
}
Attestation::Electra(AttestationElectra {
aggregation_bits: BitList::with_capacity(committee_len)
.map_err(BeaconStateError::from)?,
committee_bits,
data: AttestationData {
slot: request_slot,
index: 0u64,
beacon_block_root: item.beacon_block_root,
source: item.source,
target: item.target,
},
signature: AggregateSignature::empty(),
})
} else {
Attestation::Base(AttestationBase {
aggregation_bits: BitList::with_capacity(committee_len)
.map_err(BeaconStateError::from)?,
data: AttestationData {
slot: request_slot,
index: request_index,
beacon_block_root: item.beacon_block_root,
source: item.source,
target: item.target,
},
signature: AggregateSignature::empty(),
})
};

metrics::inc_counter(&metrics::BEACON_EARLY_ATTESTER_CACHE_HITS);

Expand Down
60 changes: 41 additions & 19 deletions beacon_node/beacon_chain/src/test_utils.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1032,21 +1032,40 @@ where
*state.get_block_root(target_slot)?
};

// TODO(electra) make test fork-agnostic
Ok(Attestation::Base(AttestationBase {
aggregation_bits: BitList::with_capacity(committee_len)?,
data: AttestationData {
slot,
index,
beacon_block_root,
source: state.current_justified_checkpoint(),
target: Checkpoint {
epoch,
root: target_root,
if self.spec.fork_name_at_slot::<E>(slot) >= ForkName::Electra {
let mut committee_bits = BitVector::default();
committee_bits.set(index as usize, true)?;
Ok(Attestation::Electra(AttestationElectra {
aggregation_bits: BitList::with_capacity(committee_len)?,
committee_bits,
data: AttestationData {
slot,
index: 0u64,
beacon_block_root,
source: state.current_justified_checkpoint(),
target: Checkpoint {
epoch,
root: target_root,
},
},
},
signature: AggregateSignature::empty(),
}))
signature: AggregateSignature::empty(),
}))
} else {
Ok(Attestation::Base(AttestationBase {
aggregation_bits: BitList::with_capacity(committee_len)?,
data: AttestationData {
slot,
index,
beacon_block_root,
source: state.current_justified_checkpoint(),
target: Checkpoint {
epoch,
root: target_root,
},
},
signature: AggregateSignature::empty(),
}))
}
}

/// A list of attestations for each committee for the given slot.
Expand Down Expand Up @@ -1121,11 +1140,14 @@ where
)
.unwrap();

attestation
.aggregation_bits_base_mut()
.unwrap()
.set(i, true)
.unwrap();
match attestation {
Attestation::Base(ref mut att) => {
att.aggregation_bits.set(i, true).unwrap();
}
Attestation::Electra(ref mut att) => {
att.aggregation_bits.set(i, true).unwrap();
}
}

*attestation.signature_mut() = {
let domain = self.spec.get_domain(
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/network/src/sync/block_lookups/parent_chain.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,7 @@ pub(crate) fn compute_parent_chains(nodes: &[Node]) -> Vec<NodeChain> {
// Iterate blocks with no children
for tip in nodes {
let mut block_root = tip.block_root;
if parent_to_child.get(&block_root).is_none() {
if !parent_to_child.contains_key(&block_root) {
let mut chain = vec![];

// Resolve chain of blocks
Expand Down
8 changes: 6 additions & 2 deletions beacon_node/operation_pool/src/attestation.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,7 @@ use crate::attestation_storage::{CompactAttestationRef, CompactIndexedAttestatio
use crate::max_cover::MaxCover;
use crate::reward_cache::RewardCache;
use state_processing::common::{
base, get_attestation_participation_flag_indices, get_attesting_indices,
attesting_indices_base, base, get_attestation_participation_flag_indices,
};
use std::collections::HashMap;
use types::{
Expand Down Expand Up @@ -46,7 +46,11 @@ impl<'a, E: EthSpec> AttMaxCover<'a, E> {
let committee = state
.get_beacon_committee(att.data.slot, att.data.index)
.ok()?;
let indices = get_attesting_indices::<E>(committee.committee, &fresh_validators).ok()?;
let indices = attesting_indices_base::get_attesting_indices::<E>(
committee.committee,
&fresh_validators,
)
.ok()?;
let sqrt_total_active_balance = base::SqrtTotalActiveBalance::new(total_active_balance);
let fresh_validators_rewards: HashMap<u64, u64> = indices
.iter()
Expand Down
Loading

0 comments on commit 5a647d0

Please sign in to comment.