Skip to content

Commit

Permalink
get tests to pass
Browse files Browse the repository at this point in the history
  • Loading branch information
eserilev committed May 7, 2024
1 parent ed1cfcf commit d455c1d
Show file tree
Hide file tree
Showing 14 changed files with 199 additions and 86 deletions.
42 changes: 30 additions & 12 deletions beacon_node/beacon_chain/src/beacon_chain.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1917,18 +1917,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
46 changes: 34 additions & 12 deletions beacon_node/beacon_chain/src/early_attester_cache.rs
Original file line number Diff line number Diff line change
Expand Up @@ -124,18 +124,40 @@ impl<E: EthSpec> EarlyAttesterCache<E> {
.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
32 changes: 21 additions & 11 deletions beacon_node/beacon_chain/src/observed_aggregates.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub type ObservedSyncContributions<E> = ObservedAggregates<
pub type ObservedAggregateAttestations<E> = ObservedAggregates<
Attestation<E>,
E,
BitList<<E as types::EthSpec>::MaxValidatorsPerCommittee>,
BitList<<E as types::EthSpec>::MaxValidatorsPerSlot>,
>;

/// A trait use to associate capacity constants with the type being stored in `ObservedAggregates`.
Expand Down Expand Up @@ -103,29 +103,39 @@ pub trait SubsetItem {
}

impl<'a, E: EthSpec> SubsetItem for AttestationRef<'a, E> {
type Item = BitList<E::MaxValidatorsPerCommittee>;
type Item = BitList<E::MaxValidatorsPerSlot>;
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(),
}
}

Expand Down
66 changes: 47 additions & 19 deletions beacon_node/beacon_chain/src/test_utils.rs
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,20 @@ 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
59 changes: 45 additions & 14 deletions beacon_node/operation_pool/src/attestation_storage.rs
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ pub struct CompactIndexedAttestation<E: EthSpec> {
#[superstruct(only(Base), partial_getter(rename = "aggregation_bits_base"))]
pub aggregation_bits: BitList<E::MaxValidatorsPerCommittee>,
#[superstruct(only(Electra), partial_getter(rename = "aggregation_bits_electra"))]
pub aggregation_bits: BitList<E::MaxValidatorsPerCommitteePerSlot>,
pub aggregation_bits: BitList<E::MaxValidatorsPerSlot>,
pub signature: AggregateSignature,
pub index: u64,
#[superstruct(only(Electra))]
Expand Down Expand Up @@ -164,8 +164,8 @@ impl<E: EthSpec> CompactIndexedAttestation<E> {
(CompactIndexedAttestation::Base(this), CompactIndexedAttestation::Base(other)) => {
this.signers_disjoint_from(other)
}
(CompactIndexedAttestation::Electra(_), CompactIndexedAttestation::Electra(_)) => {
todo!()
(CompactIndexedAttestation::Electra(this), CompactIndexedAttestation::Electra(other)) => {
this.signers_disjoint_from(other)
}
// TODO(electra) is a mix of electra and base compact indexed attestations an edge case we need to deal with?
_ => false,
Expand All @@ -177,8 +177,8 @@ impl<E: EthSpec> CompactIndexedAttestation<E> {
(CompactIndexedAttestation::Base(this), CompactIndexedAttestation::Base(other)) => {
this.aggregate(other)
}
(CompactIndexedAttestation::Electra(_), CompactIndexedAttestation::Electra(_)) => {
todo!()
(CompactIndexedAttestation::Electra(this), CompactIndexedAttestation::Electra(other)) => {
this.aggregate(other)
}
// TODO(electra) is a mix of electra and base compact indexed attestations an edge case we need to deal with?
_ => (),
Expand All @@ -205,13 +205,34 @@ impl<E: EthSpec> CompactIndexedAttestationBase<E> {
}
}

impl<E: EthSpec> CompactIndexedAttestationElectra<E> {
// TODO(electra) update to match spec requirements
pub fn signers_disjoint_from(&self, other: &Self) -> bool {
self.aggregation_bits
.intersection(&other.aggregation_bits)
.is_zero()
}

// TODO(electra) update to match spec requirements
pub fn aggregate(&mut self, other: &Self) {
self.attesting_indices = self
.attesting_indices
.drain(..)
.merge(other.attesting_indices.iter().copied())
.dedup()
.collect();
self.aggregation_bits = self.aggregation_bits.union(&other.aggregation_bits);
self.signature.add_assign_aggregate(&other.signature);
}
}

impl<E: EthSpec> AttestationMap<E> {
pub fn insert(&mut self, attestation: Attestation<E>, attesting_indices: Vec<u64>) {
let SplitAttestation {
checkpoint,
data,
indexed,
} = SplitAttestation::new(attestation, attesting_indices);
} = SplitAttestation::new(attestation.clone(), attesting_indices);

let attestation_map = self.checkpoint_map.entry(checkpoint).or_default();
let attestations = attestation_map.attestations.entry(data).or_default();
Expand All @@ -220,14 +241,24 @@ impl<E: EthSpec> AttestationMap<E> {
// NOTE: this is sub-optimal and in future we will remove this in favour of max-clique
// aggregation.
let mut aggregated = false;
for existing_attestation in attestations.iter_mut() {
if existing_attestation.signers_disjoint_from(&indexed) {
existing_attestation.aggregate(&indexed);
aggregated = true;
} else if *existing_attestation == indexed {
aggregated = true;
}
}

match attestation {
Attestation::Base(_) => {
for existing_attestation in attestations.iter_mut() {
if existing_attestation.signers_disjoint_from(&indexed) {
existing_attestation.aggregate(&indexed);
aggregated = true;
} else if *existing_attestation == indexed {
aggregated = true;
}
}
},
// TODO(electra) in order to be devnet ready, we can skip
// aggregating here for now. this will result in "poorly"
// constructed blocks, but that should be fine for devnet
Attestation::Electra(_) => (),
};


if !aggregated {
attestations.push(indexed);
Expand Down
2 changes: 1 addition & 1 deletion beacon_node/store/src/consensus_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ pub struct OnDiskConsensusContext<E: EthSpec> {
indexed_attestations: HashMap<
(
AttestationData,
BitList<E::MaxValidatorsPerCommitteePerSlot>,
BitList<E::MaxValidatorsPerSlot>,
),
IndexedAttestation<E>,
>,
Expand Down
9 changes: 7 additions & 2 deletions consensus/fork_choice/tests/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -830,8 +830,13 @@ async fn invalid_attestation_empty_bitfield() {
.await
.apply_attestation_to_chain(
MutationDelay::NoDelay,
|attestation, _| {
*attestation.attesting_indices_base_mut().unwrap() = vec![].into();
|attestation, _| match attestation {
IndexedAttestation::Base(ref mut att) => {
att.attesting_indices = vec![].into();
}
IndexedAttestation::Electra(ref mut att) => {
att.attesting_indices = vec![].into();
}
},
|result| {
assert_invalid_attestation!(result, InvalidAttestation::EmptyAggregationBitfield)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -99,7 +99,7 @@ pub mod attesting_indices_electra {
/// Returns validator indices which participated in the attestation, sorted by increasing index.
pub fn get_attesting_indices<E: EthSpec>(
committees: &[BeaconCommittee],
aggregation_bits: &BitList<E::MaxValidatorsPerCommitteePerSlot>,
aggregation_bits: &BitList<E::MaxValidatorsPerSlot>,
committee_bits: &BitVector<E::MaxCommitteesPerSlot>,
) -> Result<Vec<u64>, BeaconStateError> {
let mut output: HashSet<u64> = HashSet::new();
Expand Down
4 changes: 2 additions & 2 deletions consensus/state_processing/src/consensus_context.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ pub struct ConsensusContext<E: EthSpec> {
pub indexed_attestations: HashMap<
(
AttestationData,
BitList<E::MaxValidatorsPerCommitteePerSlot>,
BitList<E::MaxValidatorsPerSlot>,
),
IndexedAttestation<E>,
>,
Expand Down Expand Up @@ -208,7 +208,7 @@ impl<E: EthSpec> ConsensusContext<E> {
attestations: HashMap<
(
AttestationData,
BitList<E::MaxValidatorsPerCommitteePerSlot>,
BitList<E::MaxValidatorsPerSlot>,
),
IndexedAttestation<E>,
>,
Expand Down
Loading

0 comments on commit d455c1d

Please sign in to comment.