Merge branch 'p2p-electra' of https://github.com/sigp/lighthouse into ef-tests-electra

consensus/fork_choice/tests/tests.rs

@@ -435,7 +435,12 @@ impl ForkChoiceTest {
         let validator_committee_index = 0;
         let validator_index = *head
             .beacon_state
-            .get_beacon_committee(current_slot, attestation.data().index)
+            .get_beacon_committee(
+                current_slot,
+                attestation
+                    .committee_index()
+                    .expect("should get committee index"),
+            )
             .expect("should get committees")
             .committee
             .get(validator_committee_index)

consensus/state_processing/src/common/get_attesting_indices.rs

@@ -13,7 +13,6 @@ pub mod attesting_indices_base {
     ) -> Result<IndexedAttestation<E>, BlockOperationError<Invalid>> {
         let attesting_indices =
             get_attesting_indices::<E>(committee, &attestation.aggregation_bits)?;
-
         Ok(IndexedAttestation::Base(IndexedAttestationBase {
             attesting_indices: VariableList::new(attesting_indices)?,
             data: attestation.data.clone(),
@@ -52,6 +51,100 @@ 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>,

consensus/state_processing/src/per_block_processing/signature_sets.rs

@@ -326,6 +326,7 @@ where
         genesis_validators_root,
     );
 
+    // TODO(electra), signing root isnt unique in the case of electra
     let message = indexed_attestation.data().signing_root(domain);
 
     Ok(SignatureSet::multiple_pubkeys(signature, pubkeys, message))
@@ -436,7 +437,6 @@ where
     let message = slot.signing_root(domain);
     let signature = signed_aggregate_and_proof.message().selection_proof();
     let validator_index = signed_aggregate_and_proof.message().aggregator_index();
-
     Ok(SignatureSet::single_pubkey(
         signature,
         get_pubkey(validator_index as usize).ok_or(Error::ValidatorUnknown(validator_index))?,

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/Cargo.toml

@@ -51,7 +51,6 @@ metastruct = "0.1.0"
 serde_json = { workspace = true }
 smallvec = { workspace = true }
 maplit = { workspace = true }
-strum = { workspace = true }
 milhouse = { workspace = true }
 rpds = { workspace = true }
 

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};
@@ -75,37 +74,17 @@ 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 {
-        let aggregation_bits: BitList<E::MaxValidatorsPerCommittee> = BitList::random_for_test(rng);
-        // let committee_bits: BitList<E::MaxCommitteesPerSlot> = BitList::random_for_test(rng);
+        let aggregation_bits = BitList::random_for_test(rng);
         let data = AttestationData::random_for_test(rng);
         let signature = AggregateSignature::random_for_test(rng);
+        let committee_bits = BitVector::random_for_test(rng);
 
-        Self::Base(AttestationBase {
+        Self::Electra(AttestationElectra {
             aggregation_bits,
-            // committee_bits,
+            committee_bits,
             data,
             signature,
         })
@@ -190,9 +169,9 @@ impl<E: EthSpec> Attestation<E> {
         }
     }
 
-    pub fn committee_index(&self) -> u64 {
+    pub fn committee_index(&self) -> Option<u64> {
         match self {
-            Attestation::Base(att) => att.data.index,
+            Attestation::Base(att) => Some(att.data.index),
             Attestation::Electra(att) => att.committee_index(),
         }
     }
@@ -241,12 +220,31 @@ impl<'a, E: EthSpec> AttestationRef<'a, E> {
         }
     }
 
-    pub fn committee_index(&self) -> u64 {
+    pub fn committee_index(&self) -> Option<u64> {
         match self {
-            AttestationRef::Base(att) => att.data.index,
+            AttestationRef::Base(att) => Some(att.data.index),
             AttestationRef::Electra(att) => att.committee_index(),
         }
     }
+
+    pub fn set_aggregation_bits(&self) -> Vec<usize> {
+        match self {
+            Self::Base(att) => att
+                .aggregation_bits
+                .iter()
+                .enumerate()
+                .filter(|(_i, bit)| *bit)
+                .map(|(i, _bit)| i)
+                .collect::<Vec<_>>(),
+            Self::Electra(att) => att
+                .aggregation_bits
+                .iter()
+                .enumerate()
+                .filter(|(_i, bit)| *bit)
+                .map(|(i, _bit)| i)
+                .collect::<Vec<_>>(),
+        }
+    }
 }
 
 impl<E: EthSpec> AttestationElectra<E> {
@@ -260,8 +258,8 @@ impl<E: EthSpec> AttestationElectra<E> {
             .is_zero()
     }
 
-    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().cloned()
     }
 
     pub fn get_committee_indices(&self) -> Vec<u64> {
@@ -420,6 +418,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::*;
@@ -431,7 +488,7 @@ mod tests {
     // This test will only pass with `blst`, if we run these tests with another
     // BLS library in future we will have to make it generic.
     #[test]
-    fn size_of() {
+    fn size_of_base() {
         use std::mem::size_of;
 
         let aggregation_bits =
@@ -444,16 +501,43 @@ mod tests {
         assert_eq!(signature, 288 + 16);
 
         let attestation_expected = aggregation_bits + attestation_data + signature;
-        // TODO(electra) since we've removed attestation aggregation for electra variant
-        // i've updated the attestation value expected from 488 544
-        // assert_eq!(attestation_expected, 488);
         assert_eq!(attestation_expected, 488);
         assert_eq!(
-            size_of::<Attestation<MainnetEthSpec>>(),
+            size_of::<AttestationBase<MainnetEthSpec>>(),
             attestation_expected
         );
     }
 
-    // TODO(electra): can we do this with both variants or should we?
-    ssz_and_tree_hash_tests!(AttestationBase<MainnetEthSpec>);
+    #[test]
+    fn size_of_electra() {
+        use std::mem::size_of;
+
+        let aggregation_bits =
+            size_of::<BitList<<MainnetEthSpec as EthSpec>::MaxValidatorsPerSlot>>();
+        let attestation_data = size_of::<AttestationData>();
+        let committee_bits =
+            size_of::<BitList<<MainnetEthSpec as EthSpec>::MaxCommitteesPerSlot>>();
+        let signature = size_of::<AggregateSignature>();
+
+        assert_eq!(aggregation_bits, 56);
+        assert_eq!(committee_bits, 56);
+        assert_eq!(attestation_data, 128);
+        assert_eq!(signature, 288 + 16);
+
+        let attestation_expected = aggregation_bits + committee_bits + attestation_data + signature;
+        assert_eq!(attestation_expected, 544);
+        assert_eq!(
+            size_of::<AttestationElectra<MainnetEthSpec>>(),
+            attestation_expected
+        );
+    }
+
+    mod base {
+        use super::*;
+        ssz_and_tree_hash_tests!(AttestationBase<MainnetEthSpec>);
+    }
+    mod electra {
+        use super::*;
+        ssz_and_tree_hash_tests!(AttestationElectra<MainnetEthSpec>);
+    }
 }

consensus/types/src/attester_slashing.rs

@@ -164,7 +164,12 @@ impl<E: EthSpec> AttesterSlashing<E> {
 mod tests {
     use super::*;
     use crate::*;
-
-    // TODO(electra): should this be done for both variants?
-    ssz_and_tree_hash_tests!(AttesterSlashingBase<MainnetEthSpec>);
+    mod base {
+        use super::*;
+        ssz_and_tree_hash_tests!(AttesterSlashingBase<MainnetEthSpec>);
+    }
+    mod electra {
+        use super::*;
+        ssz_and_tree_hash_tests!(AttesterSlashingElectra<MainnetEthSpec>);
+    }
 }

consensus/types/src/beacon_block.rs

@@ -607,14 +607,12 @@ impl<E: EthSpec, Payload: AbstractExecPayload<E>> BeaconBlockElectra<E, Payload>
     /// Return a Electra block where the block has maximum size.
     pub fn full(spec: &ChainSpec) -> Self {
         let base_block: BeaconBlockBase<_, Payload> = BeaconBlockBase::full(spec);
-        // TODO(electra): check this
         let indexed_attestation: IndexedAttestationElectra<E> = IndexedAttestationElectra {
             attesting_indices: VariableList::new(vec![0_u64; E::MaxValidatorsPerSlot::to_usize()])
                 .unwrap(),
             data: AttestationData::default(),
             signature: AggregateSignature::empty(),
         };
-        // TODO(electra): fix this so we calculate this size correctly
         let attester_slashings = vec![
             AttesterSlashingElectra {
                 attestation_1: indexed_attestation.clone(),
@@ -627,7 +625,6 @@ impl<E: EthSpec, Payload: AbstractExecPayload<E>> BeaconBlockElectra<E, Payload>
             aggregation_bits: BitList::with_capacity(E::MaxValidatorsPerSlot::to_usize()).unwrap(),
             data: AttestationData::default(),
             signature: AggregateSignature::empty(),
-            // TODO(electra): does this actually allocate the size correctly?
             committee_bits: BitVector::new(),
         };
         let mut attestations_electra = vec![];

consensus/types/src/beacon_state.rs

@@ -163,6 +163,12 @@ pub enum Error {
     NonExecutionAddresWithdrawalCredential,
     NoCommitteeFound(CommitteeIndex),
     InvalidCommitteeIndex(CommitteeIndex),
+    InvalidSelectionProof {
+        aggregator_index: u64,
+    },
+    AggregatorNotInCommittee {
+        aggregator_index: u64,
+    },
 }
 
 /// Control whether an epoch-indexed field can be indexed at the next epoch or not.

consensus/types/src/indexed_attestation.rs

@@ -206,14 +206,17 @@ impl<E: EthSpec> Decode for IndexedAttestation<E> {
     }
 }
 
+// TODO(electra): think about how to handle fork variants here
 impl<E: EthSpec> TestRandom for IndexedAttestation<E> {
     fn random_for_test(rng: &mut impl RngCore) -> Self {
         let attesting_indices = VariableList::random_for_test(rng);
+        // let committee_bits: BitList<E::MaxCommitteesPerSlot> = BitList::random_for_test(rng);
         let data = AttestationData::random_for_test(rng);
         let signature = AggregateSignature::random_for_test(rng);
 
         Self::Base(IndexedAttestationBase {
             attesting_indices,
+            // committee_bits,
             data,
             signature,
         })

consensus/types/src/signed_aggregate_and_proof.rs

@@ -74,7 +74,6 @@ impl<E: EthSpec> SignedAggregateAndProof<E> {
             genesis_validators_root,
             spec,
         );
-
         let target_epoch = message.aggregate().data().slot.epoch(E::slots_per_epoch());
         let domain = spec.get_domain(
             target_epoch,

consensus/types/src/subnet_id.rs

@@ -40,13 +40,15 @@ impl SubnetId {
     /// Compute the subnet for an attestation where each slot in the
     /// attestation epoch contains `committee_count_per_slot` committees.
     pub fn compute_subnet_for_attestation<E: EthSpec>(
-        attestation: &AttestationRef<E>,
+        attestation: AttestationRef<E>,
         committee_count_per_slot: u64,
         spec: &ChainSpec,
     ) -> Result<SubnetId, ArithError> {
+        let committee_index = attestation.committee_index().ok_or(ArithError::Overflow)?;
+
         Self::compute_subnet::<E>(
             attestation.data().slot,
-            attestation.committee_index(),
+            committee_index,
             committee_count_per_slot,
             spec,
         )

consensus/types/src/test_utils/test_random/bitfield.rs

@@ -26,6 +26,15 @@ impl<N: Unsigned + Clone> TestRandom for BitVector<N> {
     fn random_for_test(rng: &mut impl RngCore) -> Self {
         let mut raw_bytes = smallvec![0; std::cmp::max(1, (N::to_usize() + 7) / 8)];
         rng.fill_bytes(&mut raw_bytes);
+        // If N isn't divisible by 8
+        // zero out bits greater than N
+        if let Some(last_byte) = raw_bytes.last_mut() {
+            let mut mask = 0;
+            for i in 0..N::to_usize() % 8 {
+                mask |= 1 << i;
+            }
+            *last_byte &= mask;
+        }
         Self::from_bytes(raw_bytes).expect("we generate a valid BitVector")
     }
 }