Dedup attestation constructor logic in attester cache

beacon_node/beacon_chain/src/attester_cache.rs

@@ -15,6 +15,7 @@ use state_processing::state_advance::{partial_state_advance, Error as StateAdvan
 use std::collections::HashMap;
 use std::ops::Range;
 use types::{
+    attestation::Error as AttestationError,
     beacon_state::{
         compute_committee_index_in_epoch, compute_committee_range_in_epoch, epoch_committee_count,
     },
@@ -59,6 +60,7 @@ pub enum Error {
     InverseRange {
         range: Range<usize>,
     },
+    AttestationError(AttestationError),
 }
 
 impl From<BeaconStateError> for Error {

beacon_node/beacon_chain/src/early_attester_cache.rs

@@ -6,7 +6,6 @@ use crate::{
 use parking_lot::RwLock;
 use proto_array::Block as ProtoBlock;
 use std::sync::Arc;
-use types::attestation::AttestationBase;
 use types::*;
 
 pub struct CacheItem<E: EthSpec> {
@@ -123,40 +122,16 @@ impl<E: EthSpec> EarlyAttesterCache<E> {
             item.committee_lengths
                 .get_committee_length::<E>(request_slot, request_index, spec)?;
 
-        let attestation = if spec.fork_name_at_slot::<E>(request_slot).electra_enabled() {
+        let attestation = Attestation::empty_for_signing(
-            let mut committee_bits = BitVector::default();
+            request_index,
-            if committee_len > 0 {
+            committee_len,
-                committee_bits
+            request_slot,
-                    .set(request_index as usize, true)
+            item.beacon_block_root,
-                    .map_err(BeaconStateError::from)?;
+            item.source,
-            }
+            item.target,
-            Attestation::Electra(AttestationElectra {
+            spec,
-                aggregation_bits: BitList::with_capacity(committee_len)
+        )
-                    .map_err(BeaconStateError::from)?,
+        .map_err(Error::AttestationError)?;
-                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);
 

consensus/types/src/attestation.rs

@@ -1,5 +1,6 @@
 use crate::slot_data::SlotData;
 use crate::{test_utils::TestRandom, Hash256, Slot};
+use crate::{Checkpoint, ForkName};
 use derivative::Derivative;
 use safe_arith::ArithError;
 use serde::{Deserialize, Serialize};
@@ -89,20 +90,29 @@ impl<E: EthSpec> Hash for Attestation<E> {
 
 impl<E: EthSpec> Attestation<E> {
     pub fn empty_for_signing(
-        committee_index: usize,
+        committee_index: u64,
         committee_length: usize,
-        data: AttestationData,
+        slot: Slot,
+        beacon_block_root: Hash256,
+        source: Checkpoint,
+        target: Checkpoint,
         spec: &ChainSpec,
     ) -> Result<Self, Error> {
-        if spec.fork_name_at_slot::<E>(data.slot).electra_enabled() {
+        if spec.fork_name_at_slot::<E>(slot) >= ForkName::Electra {
             let mut committee_bits: BitVector<E::MaxCommitteesPerSlot> = BitVector::default();
             committee_bits
-                .set(committee_index, true)
+                .set(committee_index as usize, true)
                 .map_err(|_| Error::InvalidCommitteeIndex)?;
             Ok(Attestation::Electra(AttestationElectra {
                 aggregation_bits: BitList::with_capacity(committee_length)
                     .map_err(|_| Error::InvalidCommitteeLength)?,
-                data,
+                data: AttestationData {
+                    slot,
+                    index: 0u64,
+                    beacon_block_root,
+                    source,
+                    target,
+                },
                 committee_bits,
                 signature: AggregateSignature::infinity(),
             }))
@@ -110,7 +120,13 @@ impl<E: EthSpec> Attestation<E> {
             Ok(Attestation::Base(AttestationBase {
                 aggregation_bits: BitList::with_capacity(committee_length)
                     .map_err(|_| Error::InvalidCommitteeLength)?,
-                data,
+                data: AttestationData {
+                    slot,
+                    index: committee_index,
+                    beacon_block_root,
+                    source,
+                    target,
+                },
                 signature: AggregateSignature::infinity(),
             }))
         }

validator_client/src/attestation_service.rs

@@ -374,9 +374,12 @@ impl<T: SlotClock + 'static, E: EthSpec> AttestationService<T, E> {
             }
 
             let mut attestation = match Attestation::<E>::empty_for_signing(
-                duty.committee_index as usize,
+                duty.committee_index,
                 duty.committee_length as usize,
-                attestation_data.clone(),
+                attestation_data.slot,
+                attestation_data.beacon_block_root,
+                attestation_data.source,
+                attestation_data.target,
                 &self.context.eth2_config.spec,
             ) {
                 Ok(attestation) => attestation,