gnosis/lighthouse
1
0
Fork 0
mirror of https://github.com/sigp/lighthouse.git synced 2026-03-11 18:04:18 +00:00
Code Issues Packages Projects Releases Wiki Activity

Fix failing attestation tests and misc electra attestation cleanup (#5810)

Browse Source 
* - get attestation related beacon chain tests to pass
- observed attestations are now keyed off of data + committee index
- rename op pool attestationref to compactattestationref
- remove unwraps in agg pool and use options instead
- cherry pick some changes from ef-tests-electra

* cargo fmt

* fix failing test

* Revert dockerfile changes

* make committee_index return option

* function args shouldnt be a ref to attestation ref

* fmt

* fix dup imports

---------

Co-authored-by: realbigsean <seananderson33@GMAIL.com>
This commit is contained in:
Eitan Seri-Levi
2024-05-30 17:51:34 +02:00
committed by GitHub
parent 75432e1135
commit e340998241
28 changed files with 765 additions and 292 deletions
Download Patch File Download Diff File Expand all files Collapse all files

20
beacon_node/operation_pool/src/attestation.rs
View File

@@ -1,4 +1,4 @@
use crate::attestation_storage::{AttestationRef, CompactIndexedAttestation};
use crate::attestation_storage::{CompactAttestationRef, CompactIndexedAttestation};
use crate::max_cover::MaxCover;
use crate::reward_cache::RewardCache;
use state_processing::common::{
@@ -14,14 +14,14 @@ use types::{
#[derive(Debug, Clone)]
pub struct AttMaxCover<'a, E: EthSpec> {
/// Underlying attestation.
pub att: AttestationRef<'a, E>,
pub att: CompactAttestationRef<'a, E>,
/// Mapping of validator indices and their rewards.
pub fresh_validators_rewards: HashMap<u64, u64>,
}
impl<'a, E: EthSpec> AttMaxCover<'a, E> {
pub fn new(
att: AttestationRef<'a, E>,
att: CompactAttestationRef<'a, E>,
state: &BeaconState<E>,
reward_cache: &'a RewardCache,
total_active_balance: u64,
@@ -36,7 +36,7 @@ impl<'a, E: EthSpec> AttMaxCover<'a, E> {
/// Initialise an attestation cover object for base/phase0 hard fork.
pub fn new_for_base(
att: AttestationRef<'a, E>,
att: CompactAttestationRef<'a, E>,
state: &BeaconState<E>,
base_state: &BeaconStateBase<E>,
total_active_balance: u64,
@@ -69,7 +69,7 @@ impl<'a, E: EthSpec> AttMaxCover<'a, E> {
/// Initialise an attestation cover object for Altair or later.
pub fn new_for_altair_deneb(
att: AttestationRef<'a, E>,
att: CompactAttestationRef<'a, E>,
state: &BeaconState<E>,
reward_cache: &'a RewardCache,
spec: &ChainSpec,
@@ -119,14 +119,14 @@ impl<'a, E: EthSpec> AttMaxCover<'a, E> {
impl<'a, E: EthSpec> MaxCover for AttMaxCover<'a, E> {
type Object = Attestation<E>;
type Intermediate = AttestationRef<'a, E>;
type Intermediate = CompactAttestationRef<'a, E>;
type Set = HashMap<u64, u64>;
fn intermediate(&self) -> &AttestationRef<'a, E> {
fn intermediate(&self) -> &CompactAttestationRef<'a, E> {
&self.att
}
fn convert_to_object(att_ref: &AttestationRef<'a, E>) -> Attestation<E> {
fn convert_to_object(att_ref: &CompactAttestationRef<'a, E>) -> Attestation<E> {
att_ref.clone_as_attestation()
}
@@ -153,7 +153,7 @@ impl<'a, E: EthSpec> MaxCover for AttMaxCover<'a, E> {
/// executing the `retain` when the `committee_bits` of the two attestations intersect.
fn update_covering_set(
&mut self,
best_att: &AttestationRef<'a, E>,
best_att: &CompactAttestationRef<'a, E>,
covered_validators: &HashMap<u64, u64>,
) {
if self.att.data.slot == best_att.data.slot && self.att.data.index == best_att.data.index {
@@ -177,7 +177,7 @@ impl<'a, E: EthSpec> MaxCover for AttMaxCover<'a, E> {
///
/// This isn't optimal, but with the Altair fork this code is obsolete and not worth upgrading.
pub fn earliest_attestation_validators<E: EthSpec>(
attestation: &AttestationRef<E>,
attestation: &CompactAttestationRef<E>,
state: &BeaconState<E>,
base_state: &BeaconStateBase<E>,
) -> BitList<E::MaxValidatorsPerCommittee> {

118
beacon_node/operation_pool/src/attestation_storage.rs
View File

@@ -41,9 +41,8 @@ pub struct SplitAttestation<E: EthSpec> {
pub indexed: CompactIndexedAttestation<E>,
}
// TODO(electra): rename this type
#[derive(Debug, Clone)]
pub struct AttestationRef<'a, E: EthSpec> {
pub struct CompactAttestationRef<'a, E: EthSpec> {
pub checkpoint: &'a CheckpointKey,
pub data: &'a CompactAttestationData,
pub indexed: &'a CompactIndexedAttestation<E>,
@@ -97,8 +96,8 @@ impl<E: EthSpec> SplitAttestation<E> {
}
}
pub fn as_ref(&self) -> AttestationRef<E> {
AttestationRef {
pub fn as_ref(&self) -> CompactAttestationRef<E> {
CompactAttestationRef {
checkpoint: &self.checkpoint,
data: &self.data,
indexed: &self.indexed,
@@ -106,7 +105,7 @@ impl<E: EthSpec> SplitAttestation<E> {
}
}
impl<'a, E: EthSpec> AttestationRef<'a, E> {
impl<'a, E: EthSpec> CompactAttestationRef<'a, E> {
pub fn attestation_data(&self) -> AttestationData {
AttestationData {
slot: self.data.slot,
@@ -171,7 +170,7 @@ impl<E: EthSpec> CompactIndexedAttestation<E> {
}
}
pub fn aggregate(&mut self, other: &Self) {
pub fn aggregate(&mut self, other: &Self) -> Option<()> {
match (self, other) {
(CompactIndexedAttestation::Base(this), CompactIndexedAttestation::Base(other)) => {
this.aggregate(other)
@@ -181,7 +180,7 @@ impl<E: EthSpec> CompactIndexedAttestation<E> {
CompactIndexedAttestation::Electra(other),
) => this.aggregate_same_committee(other),
// TODO(electra) is a mix of electra and base compact indexed attestations an edge case we need to deal with?
_ => (),
_ => None,
}
}
}
@@ -193,7 +192,7 @@ impl<E: EthSpec> CompactIndexedAttestationBase<E> {
.is_zero()
}
pub fn aggregate(&mut self, other: &Self) {
pub fn aggregate(&mut self, other: &Self) -> Option<()> {
self.attesting_indices = self
.attesting_indices
.drain(..)
@@ -202,6 +201,8 @@ impl<E: EthSpec> CompactIndexedAttestationBase<E> {
.collect();
self.aggregation_bits = self.aggregation_bits.union(&other.aggregation_bits);
self.signature.add_assign_aggregate(&other.signature);
Some(())
}
}
@@ -215,9 +216,11 @@ impl<E: EthSpec> CompactIndexedAttestationElectra<E> {
.is_zero()
}
pub fn aggregate_same_committee(&mut self, other: &Self) {
pub fn aggregate_same_committee(&mut self, other: &Self) -> Option<()> {
// TODO(electra): remove assert in favour of Result
assert_eq!(self.committee_bits, other.committee_bits);
if self.committee_bits != other.committee_bits {
return None;
}
self.aggregation_bits = self.aggregation_bits.union(&other.aggregation_bits);
self.attesting_indices = self
.attesting_indices
@@ -226,34 +229,48 @@ impl<E: EthSpec> CompactIndexedAttestationElectra<E> {
.dedup()
.collect();
self.signature.add_assign_aggregate(&other.signature);
Some(())
}
pub fn aggregate_with_disjoint_committees(&mut self, other: &Self) {
// TODO(electra): remove asserts or use Result
assert!(self
pub fn aggregate_with_disjoint_committees(&mut self, other: &Self) -> Option<()> {
if !self
.committee_bits
.intersection(&other.committee_bits)
.is_zero(),);
.is_zero()
{
return None;
}
// The attestation being aggregated in must only have 1 committee bit set.
assert_eq!(other.committee_bits.num_set_bits(), 1);
if other.committee_bits.num_set_bits() != 1 {
return None;
}
// Check we are aggregating in increasing committee index order (so we can append
// aggregation bits).
assert!(self.committee_bits.highest_set_bit() < other.committee_bits.highest_set_bit());
if self.committee_bits.highest_set_bit() >= other.committee_bits.highest_set_bit() {
return None;
}
self.committee_bits = self.committee_bits.union(&other.committee_bits);
self.aggregation_bits =
bitlist_extend(&self.aggregation_bits, &other.aggregation_bits).unwrap();
self.attesting_indices = self
.attesting_indices
.drain(..)
.merge(other.attesting_indices.iter().copied())
.dedup()
.collect();
self.signature.add_assign_aggregate(&other.signature);
if let Some(agg_bits) = bitlist_extend(&self.aggregation_bits, &other.aggregation_bits) {
self.aggregation_bits = agg_bits;
self.attesting_indices = self
.attesting_indices
.drain(..)
.merge(other.attesting_indices.iter().copied())
.dedup()
.collect();
self.signature.add_assign_aggregate(&other.signature);
return Some(());
}
None
}
pub fn committee_index(&self) -> u64 {
*self.get_committee_indices().first().unwrap_or(&0u64)
pub fn committee_index(&self) -> Option<u64> {
self.get_committee_indices().first().copied()
}
pub fn get_committee_indices(&self) -> Vec<u64> {
@@ -350,27 +367,28 @@ impl<E: EthSpec> AttestationMap<E> {
continue;
}
};
let committee_index = electra_attestation.committee_index();
if let Some(existing_attestation) =
best_attestations_by_committee.get_mut(&committee_index)
{
// Search for the best (most aggregation bits) attestation for this committee
// index.
if electra_attestation.aggregation_bits.num_set_bits()
> existing_attestation.aggregation_bits.num_set_bits()
if let Some(committee_index) = electra_attestation.committee_index() {
if let Some(existing_attestation) =
best_attestations_by_committee.get_mut(&committee_index)
{
// New attestation is better than the previously known one for this
// committee. Replace it.
std::mem::swap(existing_attestation, &mut electra_attestation);
// Search for the best (most aggregation bits) attestation for this committee
// index.
if electra_attestation.aggregation_bits.num_set_bits()
> existing_attestation.aggregation_bits.num_set_bits()
{
// New attestation is better than the previously known one for this
// committee. Replace it.
std::mem::swap(existing_attestation, &mut electra_attestation);
}
// Put the inferior attestation into the list of aggregated attestations
// without performing any cross-committee aggregation.
aggregated_attestations
.push(CompactIndexedAttestation::Electra(electra_attestation));
} else {
// First attestation seen for this committee. Place it in the map
// provisionally.
best_attestations_by_committee.insert(committee_index, electra_attestation);
}
// Put the inferior attestation into the list of aggregated attestations
// without performing any cross-committee aggregation.
aggregated_attestations
.push(CompactIndexedAttestation::Electra(electra_attestation));
} else {
// First attestation seen for this committee. Place it in the map
// provisionally.
best_attestations_by_committee.insert(committee_index, electra_attestation);
}
}
@@ -399,7 +417,7 @@ impl<E: EthSpec> AttestationMap<E> {
pub fn get_attestations<'a>(
&'a self,
checkpoint_key: &'a CheckpointKey,
) -> impl Iterator<Item = AttestationRef<'a, E>> + 'a {
) -> impl Iterator<Item = CompactAttestationRef<'a, E>> + 'a {
self.checkpoint_map
.get(checkpoint_key)
.into_iter()
@@ -407,7 +425,7 @@ impl<E: EthSpec> AttestationMap<E> {
}
/// Iterate all attestations in the map.
pub fn iter(&self) -> impl Iterator<Item = AttestationRef<E>> {
pub fn iter(&self) -> impl Iterator<Item = CompactAttestationRef<E>> {
self.checkpoint_map
.iter()
.flat_map(|(checkpoint_key, attestation_map)| attestation_map.iter(checkpoint_key))
@@ -438,9 +456,9 @@ impl<E: EthSpec> AttestationDataMap<E> {
pub fn iter<'a>(
&'a self,
checkpoint_key: &'a CheckpointKey,
) -> impl Iterator<Item = AttestationRef<'a, E>> + 'a {
) -> impl Iterator<Item = CompactAttestationRef<'a, E>> + 'a {
self.attestations.iter().flat_map(|(data, vec_indexed)| {
vec_indexed.iter().map(|indexed| AttestationRef {
vec_indexed.iter().map(|indexed| CompactAttestationRef {
checkpoint: checkpoint_key,
data,
indexed,

12
beacon_node/operation_pool/src/lib.rs
View File

@@ -11,7 +11,7 @@ mod sync_aggregate_id;
pub use crate::bls_to_execution_changes::ReceivedPreCapella;
pub use attestation::{earliest_attestation_validators, AttMaxCover};
pub use attestation_storage::{AttestationRef, SplitAttestation};
pub use attestation_storage::{CompactAttestationRef, SplitAttestation};
pub use max_cover::MaxCover;
pub use persistence::{
PersistedOperationPool, PersistedOperationPoolV15, PersistedOperationPoolV20,
@@ -227,7 +227,7 @@ impl<E: EthSpec> OperationPool<E> {
state: &'a BeaconState<E>,
reward_cache: &'a RewardCache,
total_active_balance: u64,
validity_filter: impl FnMut(&AttestationRef<'a, E>) -> bool + Send,
validity_filter: impl FnMut(&CompactAttestationRef<'a, E>) -> bool + Send,
spec: &'a ChainSpec,
) -> impl Iterator<Item = AttMaxCover<'a, E>> + Send {
all_attestations
@@ -251,8 +251,8 @@ impl<E: EthSpec> OperationPool<E> {
pub fn get_attestations(
&self,
state: &BeaconState<E>,
prev_epoch_validity_filter: impl for<'a> FnMut(&AttestationRef<'a, E>) -> bool + Send,
curr_epoch_validity_filter: impl for<'a> FnMut(&AttestationRef<'a, E>) -> bool + Send,
prev_epoch_validity_filter: impl for<'a> FnMut(&CompactAttestationRef<'a, E>) -> bool + Send,
curr_epoch_validity_filter: impl for<'a> FnMut(&CompactAttestationRef<'a, E>) -> bool + Send,
spec: &ChainSpec,
) -> Result<Vec<Attestation<E>>, OpPoolError> {
let fork_name = state.fork_name_unchecked();
@@ -1281,9 +1281,7 @@ mod release_tests {
for att in &best_attestations {
match fork_name {
ForkName::Electra => {
// TODO(electra) some attestations only have 2 or 3 agg bits set
// others have 5
assert!(att.num_set_aggregation_bits() >= 2);
assert!(att.num_set_aggregation_bits() >= small_step_size);
}
_ => {
assert!(att.num_set_aggregation_bits() >= big_step_size);