Skip to content

Single attestation "Full" implementation #7444

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

Open
wants to merge 26 commits into
base: unstable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
259 changes: 151 additions & 108 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 @@ -136,7 +136,7 @@ pub fn batch_verify_unaggregated_attestations<'a, T, I>(
) -> Result<Vec<Result<VerifiedUnaggregatedAttestation<'a, T>, Error>>, Error>
where
T: BeaconChainTypes,
I: Iterator<Item = (&'a Attestation<T::EthSpec>, Option<SubnetId>)> + ExactSizeIterator,
I: Iterator<Item = (&'a SingleAttestation, Option<SubnetId>)> + ExactSizeIterator,
{
let mut num_partially_verified = 0;
let mut num_failed = 0;
Expand Down
14 changes: 5 additions & 9 deletions beacon_node/beacon_chain/src/beacon_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2053,7 +2053,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
AttestationError,
>
where
I: Iterator<Item = (&'a Attestation<T::EthSpec>, Option<SubnetId>)> + ExactSizeIterator,
I: Iterator<Item = (&'a SingleAttestation, Option<SubnetId>)> + ExactSizeIterator,
{
batch_verify_unaggregated_attestations(attestations, self)
}
Expand All @@ -2065,7 +2065,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
/// aggregation bit set.
pub fn verify_unaggregated_attestation_for_gossip<'a>(
&self,
unaggregated_attestation: &'a Attestation<T::EthSpec>,
unaggregated_attestation: &'a SingleAttestation,
subnet_id: Option<SubnetId>,
) -> Result<VerifiedUnaggregatedAttestation<'a, T>, AttestationError> {
metrics::inc_counter(&metrics::UNAGGREGATED_ATTESTATION_PROCESSING_REQUESTS);
Expand All @@ -2081,13 +2081,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
.spec
.fork_name_at_slot::<T::EthSpec>(v.attestation().data().slot);
if current_fork.electra_enabled() {
// I don't see a situation where this could return None. The upstream unaggregated attestation checks
// should have already verified that this is an attestation with a single committee bit set.
if let Some(single_attestation) = v.single_attestation() {
event_handler.register(EventKind::SingleAttestation(Box::new(
single_attestation,
)));
}
event_handler.register(EventKind::SingleAttestation(Box::new(
v.single_attestation(),
)));
}
}

Expand Down
52 changes: 33 additions & 19 deletions beacon_node/beacon_chain/src/single_attestation.rs
Original file line number Diff line number Diff line change
@@ -1,9 +1,13 @@
use crate::attestation_verification::Error;
use types::{Attestation, AttestationElectra, BitList, BitVector, EthSpec, SingleAttestation};
use types::{
Attestation, AttestationBase, AttestationElectra, BitList, BitVector, EthSpec, ForkName,
SingleAttestation,
};

pub fn single_attestation_to_attestation<E: EthSpec>(
single_attestation: &SingleAttestation,
committee: &[usize],
fork_name: ForkName,
) -> Result<Attestation<E>, Error> {
let attester_index = single_attestation.attester_index;
let committee_index = single_attestation.committee_index;
Expand All @@ -24,23 +28,33 @@ pub fn single_attestation_to_attestation<E: EthSpec>(
slot,
})?;

let mut committee_bits: BitVector<E::MaxCommitteesPerSlot> = BitVector::default();
committee_bits
.set(committee_index as usize, true)
.map_err(|e| Error::Invalid(e.into()))?;
if fork_name.electra_enabled() {
let mut committee_bits: BitVector<E::MaxCommitteesPerSlot> = BitVector::default();
committee_bits
.set(committee_index as usize, true)
.map_err(|e| Error::Invalid(e.into()))?;

let mut aggregation_bits =
BitList::with_capacity(committee.len()).map_err(|e| Error::Invalid(e.into()))?;
aggregation_bits
.set(aggregation_bit, true)
.map_err(|e| Error::Invalid(e.into()))?;

// TODO(electra): consider eventually allowing conversion to non-Electra attestations as well
// to maintain invertability (`Attestation` -> `SingleAttestation` -> `Attestation`).
Ok(Attestation::Electra(AttestationElectra {
aggregation_bits,
committee_bits,
data: single_attestation.data.clone(),
signature: single_attestation.signature.clone(),
}))
let mut aggregation_bits =
BitList::with_capacity(committee.len()).map_err(|e| Error::Invalid(e.into()))?;
aggregation_bits
.set(aggregation_bit, true)
.map_err(|e| Error::Invalid(e.into()))?;
Ok(Attestation::Electra(AttestationElectra {
aggregation_bits,
committee_bits,
data: single_attestation.data.clone(),
signature: single_attestation.signature.clone(),
}))
} else {
let mut aggregation_bits =
BitList::with_capacity(committee.len()).map_err(|e| Error::Invalid(e.into()))?;
aggregation_bits
.set(aggregation_bit, true)
.map_err(|e| Error::Invalid(e.into()))?;
Ok(Attestation::Base(AttestationBase {
aggregation_bits,
data: single_attestation.data.clone(),
signature: single_attestation.signature.clone(),
}))
}
}
57 changes: 50 additions & 7 deletions beacon_node/beacon_chain/src/test_utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1123,9 +1123,14 @@ where
attn.aggregation_bits
.set(aggregation_bit_index, true)
.unwrap();
attn
Attestation::Electra(attn)
}
Attestation::Base(mut attn) => {
attn.aggregation_bits
.set(aggregation_bit_index, true)
.unwrap();
Attestation::Base(attn)
}
Attestation::Base(_) => panic!("Must be an Electra attestation"),
};

let aggregation_bits = attestation.get_aggregation_bits();
Expand Down Expand Up @@ -1153,8 +1158,10 @@ where
let single_attestation =
attestation.to_single_attestation_with_attester_index(attester_index as u64)?;

let fork_name = self.spec.fork_name_at_slot::<E>(attestation.data().slot);
let attestation: Attestation<E> =
single_attestation_to_attestation(&single_attestation, committee.committee).unwrap();
single_attestation_to_attestation(&single_attestation, committee.committee, fork_name)
.unwrap();

assert_eq!(
single_attestation.committee_index,
Expand Down Expand Up @@ -2424,7 +2431,11 @@ where
})
}

pub fn process_attestations(&self, attestations: HarnessAttestations<E>) {
pub fn process_attestations(
&self,
attestations: HarnessAttestations<E>,
state: &BeaconState<E>,
) {
let num_validators = self.validator_keypairs.len();
let mut unaggregated = Vec::with_capacity(num_validators);
// This is an over-allocation, but it should be fine. It won't be *that* memory hungry and
Expand All @@ -2433,17 +2444,49 @@ where

for (unaggregated_attestations, maybe_signed_aggregate) in attestations.iter() {
for (attn, subnet) in unaggregated_attestations {
unaggregated.push((attn, Some(*subnet)));
let aggregation_bits = attn.get_aggregation_bits();

if aggregation_bits.len() != 1 {
panic!("Must be an unaggregated attestation")
}

let aggregation_bit = *aggregation_bits.first().unwrap();

let committee = state
.get_beacon_committee(attn.data().slot, attn.committee_index().unwrap())
.unwrap();

let attester_index = committee
.committee
.iter()
.enumerate()
.find_map(|(i, &index)| {
if aggregation_bit as usize == i {
return Some(index);
}
None
})
.unwrap();

let single_attestation = attn
.to_single_attestation_with_attester_index(attester_index as u64)
.unwrap();

unaggregated.push((single_attestation, Some(*subnet)));
}

if let Some(a) = maybe_signed_aggregate {
aggregated.push(a)
}
}

//TODO(single-attestation) convert to single attn

for result in self
.chain
.batch_verify_unaggregated_attestations_for_gossip(unaggregated.into_iter())
.batch_verify_unaggregated_attestations_for_gossip(
unaggregated.iter().map(|(attn, subnet)| (attn, *subnet)),
)
.unwrap()
{
let verified = result.unwrap();
Expand Down Expand Up @@ -2510,7 +2553,7 @@ where
) {
let attestations =
self.make_attestations(validators, state, state_root, block_hash, block.slot());
self.process_attestations(attestations);
self.process_attestations(attestations, state);
}

pub fn sync_committee_sign_block(
Expand Down
Loading