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.

Sign up for GitHub

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

Jump to bottom

Electra attestations #5712 review #5940

Merged
merged 30 commits into from
Jun 19, 2024
Merged

Electra attestations #5712 review #5940

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
d87541c
De-dup attestation constructor logic
dapplion Jun 17, 2024
dd0d5e2
Remove unwraps in Attestation construction
dapplion Jun 17, 2024
3ec21a2
Dedup match_attestation_data
dapplion Jun 17, 2024
795eff9
Remove outdated TODO
dapplion Jun 17, 2024
960f8c5
Use ForkName Ord in fork-choice tests
dapplion Jun 17, 2024
1d0e3f4
Use ForkName Ord in BeaconBlockBody
dapplion Jun 17, 2024
5acc052
Make to_electra not fallible
dapplion Jun 17, 2024
4f08f6e
Remove TestRandom impl for IndexedAttestation
dapplion Jun 17, 2024
f049285
Remove IndexedAttestation faulty Decode impl
dapplion Jun 17, 2024
5070ab2
Drop TestRandom impl
dapplion Jun 17, 2024
45d007a
Add PendingAttestationInElectra
dapplion Jun 17, 2024
9e84779
Indexed att on disk (#35)
realbigsean Jun 18, 2024
7af3f2e
add electra fork enabled fn to ForkName impl (#36)
eserilev Jun 18, 2024
2634a1f
Update common/eth2/src/types.rs
dapplion Jun 18, 2024
dec7cff
Dedup attestation constructor logic in attester cache
dapplion Jun 17, 2024
6a4d842
Use if let Ok for committee_bits
dapplion Jun 18, 2024
6f0b784
Dedup Attestation constructor code
dapplion Jun 18, 2024
444cd62
Diff reduction in tests
dapplion Jun 18, 2024
d264736
Fix beacon_chain tests
dapplion Jun 18, 2024
7521f97
Diff reduction
dapplion Jun 18, 2024
4d3edfe
Use Ord for ForkName in pubsub
dapplion Jun 18, 2024
7fce143
Resolve into_attestation_and_indices todo
dapplion Jun 18, 2024
4d4c268
Remove stale TODO
dapplion Jun 18, 2024
370d511
Fix beacon_chain tests
dapplion Jun 18, 2024
cbb7c5d
Test spec invariant
dapplion Jun 19, 2024
70a2d4d
Use electra_enabled in pubsub
dapplion Jun 19, 2024
9e6e76f
Remove get_indexed_attestation_from_signed_aggregate
dapplion Jun 19, 2024
a8d8989
Use ok_or instead of if let else
dapplion Jun 19, 2024
d67270f
committees are sorted
dapplion Jun 19, 2024
3977b92
remove dup method `get_indexed_attestation_from_committees`
realbigsean Jun 19, 2024
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
99 changes: 52 additions & 47 deletions beacon_node/beacon_chain/src/attestation_verification.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,10 @@ use std::borrow::Cow;
use strum::AsRefStr;
use tree_hash::TreeHash;
use types::{
Attestation, AttestationRef, BeaconCommittee, BeaconStateError,
BeaconStateError::NoCommitteeFound, ChainSpec, CommitteeIndex, Epoch, EthSpec, ForkName,
Hash256, IndexedAttestation, SelectionProof, SignedAggregateAndProof, Slot, SubnetId,
Attestation, AttestationRef, BeaconCommittee,
BeaconStateError::{self, 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 @@ -598,57 +599,61 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
};
let get_indexed_attestation_with_committee =
|(committees, _): (Vec<BeaconCommittee>, CommitteesPerSlot)| {
match signed_aggregate {
SignedAggregateAndProof::Base(signed_aggregate) => {
let att = &signed_aggregate.message.aggregate;
let aggregator_index = signed_aggregate.message.aggregator_index;
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,
})?;

// TODO(electra):
let (index, aggregator_index, selection_proof, data) = match signed_aggregate {
SignedAggregateAndProof::Base(signed_aggregate) => (
signed_aggregate.message.aggregate.data.index,
signed_aggregate.message.aggregator_index,
// Note: this clones the signature which is known to be a relatively slow operation.
//
// Future optimizations should remove this clone.
if let Some(committee) = committee {
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 });
}
signed_aggregate.message.selection_proof.clone(),
signed_aggregate.message.aggregate.data.clone(),
),
SignedAggregateAndProof::Electra(signed_aggregate) => (
signed_aggregate
.message
.aggregate
.committee_index()
.ok_or(Error::NotExactlyOneCommitteeBitSet(0))?,
signed_aggregate.message.aggregator_index,
signed_aggregate.message.selection_proof.clone(),
signed_aggregate.message.aggregate.data.clone(),
),
};
let slot = data.slot;

// 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 });
}
let committee = committees
.iter()
.filter(|&committee| committee.index == index)
.at_most_one()
.map_err(|_| Error::NoCommitteeForSlotAndIndex { slot, index })?
.ok_or(Error::NoCommitteeForSlotAndIndex { slot, 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,
})
}
if !SelectionProof::from(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 });
}

// p2p aggregates have a single committee, we can assert that aggregation_bits is always
// less then MaxValidatorsPerCommittee
match signed_aggregate {
SignedAggregateAndProof::Base(signed_aggregate) => {
attesting_indices_base::get_indexed_attestation(
committee.committee,
&signed_aggregate.message.aggregate,
)
.map_err(|e| BeaconChainError::from(e).into())
}
SignedAggregateAndProof::Electra(signed_aggregate) => {
attesting_indices_electra::get_indexed_attestation_from_signed_aggregate(
attesting_indices_electra::get_indexed_attestation_from_committees(
&committees,
signed_aggregate,
&chain.spec,
&signed_aggregate.message.aggregate,
)
.map_err(|e| BeaconChainError::from(e).into())
}
Expand Down
103 changes: 8 additions & 95 deletions consensus/state_processing/src/common/get_attesting_indices.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -51,100 +51,6 @@ pub mod attesting_indices_electra {
use safe_arith::SafeArith;
use types::*;

// TODO(electra) remove duplicate code
// get_indexed_attestation is almost an exact duplicate
// the only differences are the invalid selection proof
// and aggregator not in committee checks
pub fn get_indexed_attestation_from_signed_aggregate<E: EthSpec>(
committees: &[BeaconCommittee],
signed_aggregate: &SignedAggregateAndProofElectra<E>,
spec: &ChainSpec,
) -> Result<IndexedAttestation<E>, BeaconStateError> {
let mut output: HashSet<u64> = HashSet::new();

let committee_bits = &signed_aggregate.message.aggregate.committee_bits;
let aggregation_bits = &signed_aggregate.message.aggregate.aggregation_bits;
let aggregator_index = signed_aggregate.message.aggregator_index;
let attestation = &signed_aggregate.message.aggregate;

let committee_indices = get_committee_indices::<E>(committee_bits);

let mut committee_offset = 0;

let committees_map: HashMap<u64, &BeaconCommittee> = committees
.iter()
.map(|committee| (committee.index, committee))
.collect();

let committee_count_per_slot = committees.len() as u64;
let mut participant_count = 0;

// TODO(electra):
// 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());

for index in committee_indices {
if let Some(&beacon_committee) = committees_map.get(&index) {
if !selection_proof
.is_aggregator(beacon_committee.committee.len(), spec)
.map_err(BeaconStateError::ArithError)?
{
return Err(BeaconStateError::InvalidSelectionProof { aggregator_index });
}

if !beacon_committee
.committee
.contains(&(aggregator_index as usize))
{
return Err(BeaconStateError::AggregatorNotInCommittee { aggregator_index });
}

// This check is new to the spec's `process_attestation` in Electra.
if index >= committee_count_per_slot {
return Err(BeaconStateError::InvalidCommitteeIndex(index));
}

participant_count.safe_add_assign(beacon_committee.committee.len() as u64)?;
let committee_attesters = beacon_committee
.committee
.iter()
.enumerate()
.filter_map(|(i, &index)| {
if let Ok(aggregation_bit_index) = committee_offset.safe_add(i) {
if aggregation_bits.get(aggregation_bit_index).unwrap_or(false) {
return Some(index as u64);
}
}
None
})
.collect::<HashSet<u64>>();

output.extend(committee_attesters);

committee_offset.safe_add_assign(beacon_committee.committee.len())?;
} else {
return Err(Error::NoCommitteeFound(index));
}
}

// This check is new to the spec's `process_attestation` in Electra.
if participant_count as usize != aggregation_bits.len() {
return Err(Error::InvalidBitfield);
}

let mut indices = output.into_iter().collect_vec();
indices.sort_unstable();

Ok(IndexedAttestation::Electra(IndexedAttestationElectra {
attesting_indices: VariableList::new(indices)?,
data: attestation.data.clone(),
signature: attestation.signature.clone(),
}))
}

pub fn get_indexed_attestation<E: EthSpec>(
committees: &[BeaconCommittee],
attestation: &AttestationElectra<E>,
Expand All @@ -167,8 +73,15 @@ pub mod attesting_indices_electra {
attestation: &AttestationElectra<E>,
) -> Result<IndexedAttestation<E>, BlockOperationError<Invalid>> {
let committees = beacon_state.get_beacon_committees_at_slot(attestation.data.slot)?;
get_indexed_attestation_from_committees(&committees, attestation)
}

pub fn get_indexed_attestation_from_committees<E: EthSpec>(
realbigsean marked this conversation as resolved.
Show resolved Hide resolved
committees: &[BeaconCommittee],
attestation: &AttestationElectra<E>,
) -> Result<IndexedAttestation<E>, BlockOperationError<Invalid>> {
let attesting_indices = get_attesting_indices::<E>(
&committees,
committees,
&attestation.aggregation_bits,
&attestation.committee_bits,
)?;
Expand Down