Electra attestation changes rm decode impl (#5856)

* Remove Crappy Decode impl for Attestation

* Remove Inefficient Attestation Decode impl

* Implement Schema Upgrade / Downgrade

* Update beacon_node/beacon_chain/src/schema_change/migration_schema_v20.rs

Co-authored-by: Michael Sproul <micsproul@gmail.com>

---------

Co-authored-by: Michael Sproul <micsproul@gmail.com>

beacon_node/beacon_chain/src/schema_change.rs

@@ -2,6 +2,7 @@
 mod migration_schema_v17;
 mod migration_schema_v18;
 mod migration_schema_v19;
+mod migration_schema_v20;
 
 use crate::beacon_chain::BeaconChainTypes;
 use crate::types::ChainSpec;
@@ -78,6 +79,14 @@ pub fn migrate_schema<T: BeaconChainTypes>(
             let ops = migration_schema_v19::downgrade_from_v19::<T>(db.clone(), log)?;
             db.store_schema_version_atomically(to, ops)
         }
+        (SchemaVersion(19), SchemaVersion(20)) => {
+            let ops = migration_schema_v20::upgrade_to_v20::<T>(db.clone(), log)?;
+            db.store_schema_version_atomically(to, ops)
+        }
+        (SchemaVersion(20), SchemaVersion(19)) => {
+            let ops = migration_schema_v20::downgrade_from_v20::<T>(db.clone(), log)?;
+            db.store_schema_version_atomically(to, ops)
+        }
         // Anything else is an error.
         (_, _) => Err(HotColdDBError::UnsupportedSchemaVersion {
             target_version: to,

beacon_node/beacon_chain/src/schema_change/migration_schema_v20.rs

@@ -0,0 +1,103 @@
+use crate::beacon_chain::{BeaconChainTypes, OP_POOL_DB_KEY};
+use operation_pool::{
+    PersistedOperationPool, PersistedOperationPoolV15, PersistedOperationPoolV20,
+};
+use slog::{debug, info, Logger};
+use std::sync::Arc;
+use store::{Error, HotColdDB, KeyValueStoreOp, StoreItem};
+use types::Attestation;
+
+pub fn upgrade_to_v20<T: BeaconChainTypes>(
+    db: Arc<HotColdDB<T::EthSpec, T::HotStore, T::ColdStore>>,
+    log: Logger,
+) -> Result<Vec<KeyValueStoreOp>, Error> {
+    // Load a V15 op pool and transform it to V20.
+    let Some(PersistedOperationPoolV15::<T::EthSpec> {
+        attestations_v15,
+        sync_contributions,
+        attester_slashings_v15,
+        proposer_slashings,
+        voluntary_exits,
+        bls_to_execution_changes,
+        capella_bls_change_broadcast_indices,
+    }) = db.get_item(&OP_POOL_DB_KEY)?
+    else {
+        debug!(log, "Nothing to do, no operation pool stored");
+        return Ok(vec![]);
+    };
+
+    let attestations = attestations_v15
+        .into_iter()
+        .map(|(attestation, indices)| (Attestation::Base(attestation).into(), indices))
+        .collect();
+
+    let attester_slashings = attester_slashings_v15
+        .into_iter()
+        .map(|slashing| slashing.into())
+        .collect();
+
+    let v20 = PersistedOperationPool::V20(PersistedOperationPoolV20 {
+        attestations,
+        sync_contributions,
+        attester_slashings,
+        proposer_slashings,
+        voluntary_exits,
+        bls_to_execution_changes,
+        capella_bls_change_broadcast_indices,
+    });
+    Ok(vec![v20.as_kv_store_op(OP_POOL_DB_KEY)])
+}
+
+pub fn downgrade_from_v20<T: BeaconChainTypes>(
+    db: Arc<HotColdDB<T::EthSpec, T::HotStore, T::ColdStore>>,
+    log: Logger,
+) -> Result<Vec<KeyValueStoreOp>, Error> {
+    // Load a V20 op pool and transform it to V15.
+    let Some(PersistedOperationPoolV20::<T::EthSpec> {
+        attestations,
+        sync_contributions,
+        attester_slashings,
+        proposer_slashings,
+        voluntary_exits,
+        bls_to_execution_changes,
+        capella_bls_change_broadcast_indices,
+    }) = db.get_item(&OP_POOL_DB_KEY)?
+    else {
+        debug!(log, "Nothing to do, no operation pool stored");
+        return Ok(vec![]);
+    };
+
+    let attestations_v15 = attestations
+        .into_iter()
+        .filter_map(|(attestation, indices)| {
+            if let Attestation::Base(attestation) = attestation.into() {
+                Some((attestation, indices))
+            } else {
+                info!(log, "Dropping attestation during downgrade"; "reason" => "not a base attestation");
+                None
+            }
+        })
+        .collect();
+
+    let attester_slashings_v15 = attester_slashings
+        .into_iter()
+        .filter_map(|slashing| match slashing.try_into() {
+            Ok(slashing) => Some(slashing),
+            Err(_) => {
+                info!(log, "Dropping attester slashing during downgrade"; "reason" => "not a base attester slashing");
+                None
+            }
+        })
+        .collect();
+
+    let v15 = PersistedOperationPool::V15(PersistedOperationPoolV15 {
+        attestations_v15,
+        sync_contributions,
+        attester_slashings_v15,
+        proposer_slashings,
+        voluntary_exits,
+        bls_to_execution_changes,
+        capella_bls_change_broadcast_indices,
+    });
+    Ok(vec![v15.as_kv_store_op(OP_POOL_DB_KEY)])
+}

beacon_node/lighthouse_network/src/types/pubsub.rs

@@ -7,13 +7,14 @@ use ssz::{Decode, Encode};
 use std::io::{Error, ErrorKind};
 use std::sync::Arc;
 use types::{
-    Attestation, AttesterSlashing, AttesterSlashingBase, AttesterSlashingElectra, BlobSidecar,
-    EthSpec, ForkContext, ForkName, LightClientFinalityUpdate, LightClientOptimisticUpdate,
-    ProposerSlashing, SignedAggregateAndProof, SignedAggregateAndProofBase,
-    SignedAggregateAndProofElectra, SignedBeaconBlock, SignedBeaconBlockAltair,
-    SignedBeaconBlockBase, SignedBeaconBlockBellatrix, SignedBeaconBlockCapella,
-    SignedBeaconBlockDeneb, SignedBeaconBlockElectra, SignedBlsToExecutionChange,
-    SignedContributionAndProof, SignedVoluntaryExit, SubnetId, SyncCommitteeMessage, SyncSubnetId,
+    Attestation, AttestationBase, AttestationElectra, AttesterSlashing, AttesterSlashingBase,
+    AttesterSlashingElectra, BlobSidecar, EthSpec, ForkContext, ForkName,
+    LightClientFinalityUpdate, LightClientOptimisticUpdate, ProposerSlashing,
+    SignedAggregateAndProof, SignedAggregateAndProofBase, SignedAggregateAndProofElectra,
+    SignedBeaconBlock, SignedBeaconBlockAltair, SignedBeaconBlockBase, SignedBeaconBlockBellatrix,
+    SignedBeaconBlockCapella, SignedBeaconBlockDeneb, SignedBeaconBlockElectra,
+    SignedBlsToExecutionChange, SignedContributionAndProof, SignedVoluntaryExit, SubnetId,
+    SyncCommitteeMessage, SyncSubnetId,
 };
 
 #[derive(Debug, Clone, PartialEq)]
@@ -182,7 +183,26 @@ impl<E: EthSpec> PubsubMessage<E> {
                     }
                     GossipKind::Attestation(subnet_id) => {
                         let attestation =
-                            Attestation::from_ssz_bytes(data).map_err(|e| format!("{:?}", e))?;
+                            match fork_context.from_context_bytes(gossip_topic.fork_digest) {
+                                Some(ForkName::Base)
+                                | Some(ForkName::Altair)
+                                | Some(ForkName::Bellatrix)
+                                | Some(ForkName::Capella)
+                                | Some(ForkName::Deneb) => Attestation::Base(
+                                    AttestationBase::from_ssz_bytes(data)
+                                        .map_err(|e| format!("{:?}", e))?,
+                                ),
+                                Some(ForkName::Electra) => Attestation::Electra(
+                                    AttestationElectra::from_ssz_bytes(data)
+                                        .map_err(|e| format!("{:?}", e))?,
+                                ),
+                                None => {
+                                    return Err(format!(
+                                        "Unknown gossipsub fork digest: {:?}",
+                                        gossip_topic.fork_digest
+                                    ))
+                                }
+                            };
                         Ok(PubsubMessage::Attestation(Box::new((
                             *subnet_id,
                             attestation,

beacon_node/operation_pool/src/lib.rs

@@ -14,8 +14,7 @@ pub use attestation::{earliest_attestation_validators, AttMaxCover};
 pub use attestation_storage::{AttestationRef, SplitAttestation};
 pub use max_cover::MaxCover;
 pub use persistence::{
-    PersistedOperationPool, PersistedOperationPoolV12, PersistedOperationPoolV14,
-    PersistedOperationPoolV15, PersistedOperationPoolV5,
+    PersistedOperationPool, PersistedOperationPoolV15, PersistedOperationPoolV20,
 };
 pub use reward_cache::RewardCache;
 use state_processing::epoch_cache::is_epoch_cache_initialized;

beacon_node/operation_pool/src/persistence.rs

@@ -1,4 +1,3 @@
-use crate::attestation_id::AttestationId;
 use crate::attestation_storage::AttestationMap;
 use crate::bls_to_execution_changes::{BlsToExecutionChanges, ReceivedPreCapella};
 use crate::sync_aggregate_id::SyncAggregateId;
@@ -12,6 +11,7 @@ use state_processing::SigVerifiedOp;
 use std::collections::HashSet;
 use std::mem;
 use store::{DBColumn, Error as StoreError, StoreItem};
+use types::attestation::AttestationOnDisk;
 use types::*;
 
 type PersistedSyncContributions<E> = Vec<(SyncAggregateId, Vec<SyncCommitteeContribution<E>>)>;
@@ -21,7 +21,7 @@ type PersistedSyncContributions<E> = Vec<(SyncAggregateId, Vec<SyncCommitteeCont
 /// Operations are stored in arbitrary order, so it's not a good idea to compare instances
 /// of this type (or its encoded form) for equality. Convert back to an `OperationPool` first.
 #[superstruct(
-    variants(V5, V12, V14, V15),
+    variants(V15, V20),
     variant_attributes(
         derive(Derivative, PartialEq, Debug, Encode, Decode),
         derivative(Clone),
@@ -31,36 +31,26 @@ type PersistedSyncContributions<E> = Vec<(SyncAggregateId, Vec<SyncCommitteeCont
 #[derive(PartialEq, Debug, Encode)]
 #[ssz(enum_behaviour = "transparent")]
 pub struct PersistedOperationPool<E: EthSpec> {
-    /// [DEPRECATED] Mapping from attestation ID to attestation mappings.
-    #[superstruct(only(V5))]
-    pub attestations_v5: Vec<(AttestationId, Vec<Attestation<E>>)>,
+    #[superstruct(only(V15))]
+    pub attestations_v15: Vec<(AttestationBase<E>, Vec<u64>)>,
     /// Attestations and their attesting indices.
-    #[superstruct(only(V12, V14, V15))]
-    pub attestations: Vec<(Attestation<E>, Vec<u64>)>,
+    #[superstruct(only(V20))]
+    pub attestations: Vec<(AttestationOnDisk<E>, Vec<u64>)>,
     /// Mapping from sync contribution ID to sync contributions and aggregate.
     pub sync_contributions: PersistedSyncContributions<E>,
-    /// TODO(electra): we've made a DB change here!!!
+    #[superstruct(only(V15))]
+    pub attester_slashings_v15: Vec<SigVerifiedOp<AttesterSlashingBase<E>, E>>,
     /// Attester slashings.
-    #[superstruct(only(V12, V14, V15))]
+    #[superstruct(only(V20))]
     pub attester_slashings: Vec<SigVerifiedOp<AttesterSlashing<E>, E>>,
-    /// [DEPRECATED] Proposer slashings.
-    #[superstruct(only(V5))]
-    pub proposer_slashings_v5: Vec<ProposerSlashing>,
     /// Proposer slashings with fork information.
-    #[superstruct(only(V12, V14, V15))]
     pub proposer_slashings: Vec<SigVerifiedOp<ProposerSlashing, E>>,
-    /// [DEPRECATED] Voluntary exits.
-    #[superstruct(only(V5))]
-    pub voluntary_exits_v5: Vec<SignedVoluntaryExit>,
     /// Voluntary exits with fork information.
-    #[superstruct(only(V12, V14, V15))]
     pub voluntary_exits: Vec<SigVerifiedOp<SignedVoluntaryExit, E>>,
     /// BLS to Execution Changes
-    #[superstruct(only(V14, V15))]
     pub bls_to_execution_changes: Vec<SigVerifiedOp<SignedBlsToExecutionChange, E>>,
     /// Validator indices with BLS to Execution Changes to be broadcast at the
     /// Capella fork.
-    #[superstruct(only(V15))]
     pub capella_bls_change_broadcast_indices: Vec<u64>,
 }
 
@@ -73,7 +63,7 @@ impl<E: EthSpec> PersistedOperationPool<E> {
             .iter()
             .map(|att| {
                 (
-                    att.clone_as_attestation(),
+                    AttestationOnDisk::from(att.clone_as_attestation()),
                     att.indexed.attesting_indices().clone(),
                 )
             })
@@ -121,7 +111,7 @@ impl<E: EthSpec> PersistedOperationPool<E> {
             .copied()
             .collect();
 
-        PersistedOperationPool::V15(PersistedOperationPoolV15 {
+        PersistedOperationPool::V20(PersistedOperationPoolV20 {
             attestations,
             sync_contributions,
             attester_slashings,
@@ -134,56 +124,86 @@ impl<E: EthSpec> PersistedOperationPool<E> {
 
     /// Reconstruct an `OperationPool`.
     pub fn into_operation_pool(mut self) -> Result<OperationPool<E>, OpPoolError> {
-        let attester_slashings = RwLock::new(self.attester_slashings()?.iter().cloned().collect());
+        let attester_slashings = match &self {
+            PersistedOperationPool::V15(pool_v15) => RwLock::new(
+                pool_v15
+                    .attester_slashings_v15
+                    .iter()
+                    .map(|slashing| slashing.clone().into())
+                    .collect(),
+            ),
+            PersistedOperationPool::V20(pool_v20) => {
+                RwLock::new(pool_v20.attester_slashings.iter().cloned().collect())
+            }
+        };
+
         let proposer_slashings = RwLock::new(
-            self.proposer_slashings()?
+            self.proposer_slashings()
                 .iter()
                 .cloned()
                 .map(|slashing| (slashing.as_inner().proposer_index(), slashing))
                 .collect(),
         );
         let voluntary_exits = RwLock::new(
-            self.voluntary_exits()?
+            self.voluntary_exits()
                 .iter()
                 .cloned()
                 .map(|exit| (exit.as_inner().message.validator_index, exit))
                 .collect(),
         );
         let sync_contributions = RwLock::new(self.sync_contributions().iter().cloned().collect());
-        let attestations = match self {
-            PersistedOperationPool::V5(_) | PersistedOperationPool::V12(_) => {
-                return Err(OpPoolError::IncorrectOpPoolVariant)
-            }
-            PersistedOperationPool::V14(_) | PersistedOperationPool::V15(_) => {
+        let attestations = match &self {
+            PersistedOperationPool::V15(pool_v15) => {
                 let mut map = AttestationMap::default();
-                for (att, attesting_indices) in self.attestations()?.clone() {
+                for (att, attesting_indices) in
+                    pool_v15
+                        .attestations_v15
+                        .iter()
+                        .map(|(att, attesting_indices)| {
+                            (Attestation::Base(att.clone()), attesting_indices.clone())
+                        })
+                {
+                    map.insert(att, attesting_indices);
+                }
+                RwLock::new(map)
+            }
+            PersistedOperationPool::V20(pool_v20) => {
+                let mut map = AttestationMap::default();
+                for (att, attesting_indices) in
+                    pool_v20
+                        .attestations
+                        .iter()
+                        .map(|(att, attesting_indices)| {
+                            (
+                                AttestationRef::from(att.to_ref()).clone_as_attestation(),
+                                attesting_indices.clone(),
+                            )
+                        })
+                {
                     map.insert(att, attesting_indices);
                 }
                 RwLock::new(map)
             }
         };
+
         let mut bls_to_execution_changes = BlsToExecutionChanges::default();
-        if let Ok(persisted_changes) = self.bls_to_execution_changes_mut() {
-            let persisted_changes = mem::take(persisted_changes);
+        let persisted_changes = mem::take(self.bls_to_execution_changes_mut());
+        let broadcast_indices: HashSet<_> =
+            mem::take(self.capella_bls_change_broadcast_indices_mut())
+                .into_iter()
+                .collect();
 
-            let broadcast_indices =
-                if let Ok(indices) = self.capella_bls_change_broadcast_indices_mut() {
-                    mem::take(indices).into_iter().collect()
-                } else {
-                    HashSet::new()
-                };
-
-            for bls_to_execution_change in persisted_changes {
-                let received_pre_capella = if broadcast_indices
-                    .contains(&bls_to_execution_change.as_inner().message.validator_index)
-                {
-                    ReceivedPreCapella::Yes
-                } else {
-                    ReceivedPreCapella::No
-                };
-                bls_to_execution_changes.insert(bls_to_execution_change, received_pre_capella);
-            }
+        for bls_to_execution_change in persisted_changes {
+            let received_pre_capella = if broadcast_indices
+                .contains(&bls_to_execution_change.as_inner().message.validator_index)
+            {
+                ReceivedPreCapella::Yes
+            } else {
+                ReceivedPreCapella::No
+            };
+            bls_to_execution_changes.insert(bls_to_execution_change, received_pre_capella);
         }
+
         let op_pool = OperationPool {
             attestations,
             sync_contributions,
@@ -198,48 +218,6 @@ impl<E: EthSpec> PersistedOperationPool<E> {
     }
 }
 
-impl<E: EthSpec> StoreItem for PersistedOperationPoolV5<E> {
-    fn db_column() -> DBColumn {
-        DBColumn::OpPool
-    }
-
-    fn as_store_bytes(&self) -> Vec<u8> {
-        self.as_ssz_bytes()
-    }
-
-    fn from_store_bytes(bytes: &[u8]) -> Result<Self, StoreError> {
-        PersistedOperationPoolV5::from_ssz_bytes(bytes).map_err(Into::into)
-    }
-}
-
-impl<E: EthSpec> StoreItem for PersistedOperationPoolV12<E> {
-    fn db_column() -> DBColumn {
-        DBColumn::OpPool
-    }
-
-    fn as_store_bytes(&self) -> Vec<u8> {
-        self.as_ssz_bytes()
-    }
-
-    fn from_store_bytes(bytes: &[u8]) -> Result<Self, StoreError> {
-        PersistedOperationPoolV12::from_ssz_bytes(bytes).map_err(Into::into)
-    }
-}
-
-impl<E: EthSpec> StoreItem for PersistedOperationPoolV14<E> {
-    fn db_column() -> DBColumn {
-        DBColumn::OpPool
-    }
-
-    fn as_store_bytes(&self) -> Vec<u8> {
-        self.as_ssz_bytes()
-    }
-
-    fn from_store_bytes(bytes: &[u8]) -> Result<Self, StoreError> {
-        PersistedOperationPoolV14::from_ssz_bytes(bytes).map_err(Into::into)
-    }
-}
-
 impl<E: EthSpec> StoreItem for PersistedOperationPoolV15<E> {
     fn db_column() -> DBColumn {
         DBColumn::OpPool
@@ -254,6 +232,20 @@ impl<E: EthSpec> StoreItem for PersistedOperationPoolV15<E> {
     }
 }
 
+impl<E: EthSpec> StoreItem for PersistedOperationPoolV20<E> {
+    fn db_column() -> DBColumn {
+        DBColumn::OpPool
+    }
+
+    fn as_store_bytes(&self) -> Vec<u8> {
+        self.as_ssz_bytes()
+    }
+
+    fn from_store_bytes(bytes: &[u8]) -> Result<Self, StoreError> {
+        PersistedOperationPoolV20::from_ssz_bytes(bytes).map_err(Into::into)
+    }
+}
+
 /// Deserialization for `PersistedOperationPool` defaults to `PersistedOperationPool::V12`.
 impl<E: EthSpec> StoreItem for PersistedOperationPool<E> {
     fn db_column() -> DBColumn {

beacon_node/store/src/metadata.rs

@@ -4,7 +4,7 @@ use ssz::{Decode, Encode};
 use ssz_derive::{Decode, Encode};
 use types::{Checkpoint, Hash256, Slot};
 
-pub const CURRENT_SCHEMA_VERSION: SchemaVersion = SchemaVersion(19);
+pub const CURRENT_SCHEMA_VERSION: SchemaVersion = SchemaVersion(20);
 
 // All the keys that get stored under the `BeaconMeta` column.
 //

consensus/state_processing/src/verify_operation.rs

@@ -12,7 +12,9 @@ use smallvec::{smallvec, SmallVec};
 use ssz::{Decode, Encode};
 use ssz_derive::{Decode, Encode};
 use std::marker::PhantomData;
-use types::{AttesterSlashing, AttesterSlashingOnDisk, AttesterSlashingRefOnDisk};
+use types::{
+    AttesterSlashing, AttesterSlashingBase, AttesterSlashingOnDisk, AttesterSlashingRefOnDisk,
+};
 use types::{
     BeaconState, ChainSpec, Epoch, EthSpec, Fork, ForkVersion, ProposerSlashing,
     SignedBlsToExecutionChange, SignedVoluntaryExit,
@@ -366,6 +368,49 @@ impl<E: EthSpec> TransformPersist for AttesterSlashing<E> {
     }
 }
 
+// TODO: Remove this once we no longer support DB schema version 17
+impl<E: EthSpec> TransformPersist for types::AttesterSlashingBase<E> {
+    type Persistable = Self;
+    type PersistableRef<'a> = &'a Self;
+
+    fn as_persistable_ref(&self) -> Self::PersistableRef<'_> {
+        self
+    }
+
+    fn from_persistable(persistable: Self::Persistable) -> Self {
+        persistable
+    }
+}
+// TODO: Remove this once we no longer support DB schema version 17
+impl<E: EthSpec> From<SigVerifiedOp<AttesterSlashingBase<E>, E>>
+    for SigVerifiedOp<AttesterSlashing<E>, E>
+{
+    fn from(base: SigVerifiedOp<AttesterSlashingBase<E>, E>) -> Self {
+        SigVerifiedOp {
+            op: AttesterSlashing::Base(base.op),
+            verified_against: base.verified_against,
+            _phantom: PhantomData,
+        }
+    }
+}
+// TODO: Remove this once we no longer support DB schema version 17
+impl<E: EthSpec> TryFrom<SigVerifiedOp<AttesterSlashing<E>, E>>
+    for SigVerifiedOp<AttesterSlashingBase<E>, E>
+{
+    type Error = String;
+
+    fn try_from(slashing: SigVerifiedOp<AttesterSlashing<E>, E>) -> Result<Self, Self::Error> {
+        match slashing.op {
+            AttesterSlashing::Base(base) => Ok(SigVerifiedOp {
+                op: base,
+                verified_against: slashing.verified_against,
+                _phantom: PhantomData,
+            }),
+            AttesterSlashing::Electra(_) => Err("non-base attester slashing".to_string()),
+        }
+    }
+}
+
 impl TransformPersist for ProposerSlashing {
     type Persistable = Self;
     type PersistableRef<'a> = &'a Self;

consensus/types/src/attestation.rs

@@ -4,7 +4,6 @@ use derivative::Derivative;
 use rand::RngCore;
 use safe_arith::ArithError;
 use serde::{Deserialize, Serialize};
-use ssz::Decode;
 use ssz_derive::{Decode, Encode};
 use ssz_types::BitVector;
 use std::hash::{Hash, Hasher};
@@ -72,26 +71,6 @@ pub struct Attestation<E: EthSpec> {
     pub signature: AggregateSignature,
 }
 
-impl<E: EthSpec> Decode for Attestation<E> {
-    fn is_ssz_fixed_len() -> bool {
-        false
-    }
-
-    fn from_ssz_bytes(bytes: &[u8]) -> Result<Self, ssz::DecodeError> {
-        if let Ok(result) = AttestationBase::from_ssz_bytes(bytes) {
-            return Ok(Attestation::Base(result));
-        }
-
-        if let Ok(result) = AttestationElectra::from_ssz_bytes(bytes) {
-            return Ok(Attestation::Electra(result));
-        }
-
-        Err(ssz::DecodeError::BytesInvalid(String::from(
-            "bytes not valid for any fork variant",
-        )))
-    }
-}
-
 // TODO(electra): think about how to handle fork variants here
 impl<E: EthSpec> TestRandom for Attestation<E> {
     fn random_for_test(rng: &mut impl RngCore) -> Self {
@@ -417,6 +396,65 @@ impl<'a, E: EthSpec> SlotData for AttestationRef<'a, E> {
     }
 }
 
+#[derive(Debug, Clone, Encode, Decode, PartialEq)]
+#[ssz(enum_behaviour = "union")]
+pub enum AttestationOnDisk<E: EthSpec> {
+    Base(AttestationBase<E>),
+    Electra(AttestationElectra<E>),
+}
+
+impl<E: EthSpec> AttestationOnDisk<E> {
+    pub fn to_ref(&self) -> AttestationRefOnDisk<E> {
+        match self {
+            AttestationOnDisk::Base(att) => AttestationRefOnDisk::Base(att),
+            AttestationOnDisk::Electra(att) => AttestationRefOnDisk::Electra(att),
+        }
+    }
+}
+
+#[derive(Debug, Clone, Encode)]
+#[ssz(enum_behaviour = "union")]
+pub enum AttestationRefOnDisk<'a, E: EthSpec> {
+    Base(&'a AttestationBase<E>),
+    Electra(&'a AttestationElectra<E>),
+}
+
+impl<E: EthSpec> From<Attestation<E>> for AttestationOnDisk<E> {
+    fn from(attestation: Attestation<E>) -> Self {
+        match attestation {
+            Attestation::Base(attestation) => Self::Base(attestation),
+            Attestation::Electra(attestation) => Self::Electra(attestation),
+        }
+    }
+}
+
+impl<E: EthSpec> From<AttestationOnDisk<E>> for Attestation<E> {
+    fn from(attestation: AttestationOnDisk<E>) -> Self {
+        match attestation {
+            AttestationOnDisk::Base(attestation) => Self::Base(attestation),
+            AttestationOnDisk::Electra(attestation) => Self::Electra(attestation),
+        }
+    }
+}
+
+impl<'a, E: EthSpec> From<AttestationRef<'a, E>> for AttestationRefOnDisk<'a, E> {
+    fn from(attestation: AttestationRef<'a, E>) -> Self {
+        match attestation {
+            AttestationRef::Base(attestation) => Self::Base(attestation),
+            AttestationRef::Electra(attestation) => Self::Electra(attestation),
+        }
+    }
+}
+
+impl<'a, E: EthSpec> From<AttestationRefOnDisk<'a, E>> for AttestationRef<'a, E> {
+    fn from(attestation: AttestationRefOnDisk<'a, E>) -> Self {
+        match attestation {
+            AttestationRefOnDisk::Base(attestation) => Self::Base(attestation),
+            AttestationRefOnDisk::Electra(attestation) => Self::Electra(attestation),
+        }
+    }
+}
+
 #[cfg(test)]
 mod tests {
     use super::*;

testing/ef_tests/src/cases/fork_choice.rs

@@ -180,10 +180,24 @@ impl<E: EthSpec> LoadCase for ForkChoiceTest<E> {
                         valid,
                     })
                 }
-                Step::Attestation { attestation } => {
-                    ssz_decode_file(&path.join(format!("{}.ssz_snappy", attestation)))
-                        .map(|attestation| Step::Attestation { attestation })
-                }
+                Step::Attestation { attestation } => match fork_name {
+                    ForkName::Base
+                    | ForkName::Altair
+                    | ForkName::Bellatrix
+                    | ForkName::Capella
+                    | ForkName::Deneb => ssz_decode_file(
+                        &path.join(format!("{}.ssz_snappy", attestation)),
+                    )
+                    .map(|attestation| Step::Attestation {
+                        attestation: Attestation::Base(attestation),
+                    }),
+                    ForkName::Electra => ssz_decode_file(
+                        &path.join(format!("{}.ssz_snappy", attestation)),
+                    )
+                    .map(|attestation| Step::Attestation {
+                        attestation: Attestation::Electra(attestation),
+                    }),
+                },
                 Step::AttesterSlashing { attester_slashing } => match fork_name {
                     ForkName::Base
                     | ForkName::Altair

testing/ef_tests/src/cases/operations.rs

@@ -78,8 +78,12 @@ impl<E: EthSpec> Operation<E> for Attestation<E> {
         "attestation".into()
     }
 
-    fn decode(path: &Path, _fork_name: ForkName, _spec: &ChainSpec) -> Result<Self, Error> {
-        ssz_decode_file(path)
+    fn decode(path: &Path, fork_name: ForkName, _spec: &ChainSpec) -> Result<Self, Error> {
+        if fork_name < ForkName::Electra {
+            Ok(Self::Base(ssz_decode_file(path)?))
+        } else {
+            Ok(Self::Electra(ssz_decode_file(path)?))
+        }
     }
 
     fn apply_to(