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

…into electra-devnet-1

.github/workflows/release.yml

@@ -64,7 +64,7 @@ jobs:
             - uses: KyleMayes/install-llvm-action@v1
               if: env.SELF_HOSTED_RUNNERS == 'false' && startsWith(matrix.arch, 'x86_64-windows')
               with:
-                version: "16.0"
+                version: "17.0"
                 directory: ${{ runner.temp }}/llvm
             - name: Set LIBCLANG_PATH
               if: startsWith(matrix.arch, 'x86_64-windows')

.github/workflows/test-suite.yml

@@ -112,11 +112,6 @@ jobs:
     - name: Install make
       if: env.SELF_HOSTED_RUNNERS == 'false'
       run: choco install -y make
-#    - uses: KyleMayes/install-llvm-action@v1
-#         if: env.SELF_HOSTED_RUNNERS == 'false'
-#      with:
-#        version: "16.0"
-#        directory: ${{ runner.temp }}/llvm
     - name: Set LIBCLANG_PATH
       run: echo "LIBCLANG_PATH=$((gcm clang).source -replace "clang.exe")" >> $env:GITHUB_ENV
     - name: Run tests in release

Cargo.toml

@@ -162,7 +162,7 @@ smallvec = "1.11.2"
 snap = "1"
 ssz_types = "0.6"
 strum = { version = "0.24", features = ["derive"] }
-superstruct = { git = "https://github.com/sigp/superstruct", rev = "45eecbfb9708c9fe11dbb6a6a5bd8d618f02269e" }
+superstruct = "0.8"
 syn = "1"
 sysinfo = "0.26"
 tempfile = "3"

FUNDING.json

@@ -0,0 +1,7 @@
+{
+    "drips": {
+        "ethereum": {
+            "ownedBy": "0x25c4a76E7d118705e7Ea2e9b7d8C59930d8aCD3b"
+        }
+    }
+}

Makefile

@@ -229,7 +229,6 @@ lint:
 		-D clippy::manual_let_else \
 		-D warnings \
 		-A clippy::derive_partial_eq_without_eq \
-		-A clippy::from-over-into \
 		-A clippy::upper-case-acronyms \
 		-A clippy::vec-init-then-push \
 		-A clippy::question-mark \

beacon_node/beacon_chain/src/attestation_verification.rs

@@ -35,8 +35,10 @@
 mod batch;
 
 use crate::{
-    beacon_chain::VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT, metrics,
-    observed_aggregates::ObserveOutcome, observed_attesters::Error as ObservedAttestersError,
+    beacon_chain::VALIDATOR_PUBKEY_CACHE_LOCK_TIMEOUT,
+    metrics,
+    observed_aggregates::{ObserveOutcome, ObservedAttestationKey},
+    observed_attesters::Error as ObservedAttestersError,
     BeaconChain, BeaconChainError, BeaconChainTypes,
 };
 use bls::verify_signature_sets;
@@ -58,11 +60,11 @@ use state_processing::{
 use std::borrow::Cow;
 use strum::AsRefStr;
 use tree_hash::TreeHash;
-use tree_hash_derive::TreeHash;
 use types::{
-    Attestation, AttestationData, AttestationRef, BeaconCommittee, BeaconStateError,
-    BeaconStateError::NoCommitteeFound, ChainSpec, CommitteeIndex, Epoch, EthSpec, ForkName,
-    Hash256, IndexedAttestation, SelectionProof, SignedAggregateAndProof, Slot, SubnetId,
+    Attestation, AttestationRef, BeaconCommittee,
+    BeaconStateError::{self, NoCommitteeFound},
+    ChainSpec, CommitteeIndex, Epoch, EthSpec, ForkName, Hash256, IndexedAttestation,
+    SelectionProof, SignedAggregateAndProof, Slot, SubnetId,
 };
 
 pub use batch::{batch_verify_aggregated_attestations, batch_verify_unaggregated_attestations};
@@ -309,12 +311,6 @@ struct IndexedAggregatedAttestation<'a, T: BeaconChainTypes> {
     observed_attestation_key_root: Hash256,
 }
 
-#[derive(TreeHash)]
-pub struct ObservedAttestationKey {
-    pub committee_index: u64,
-    pub attestation_data: AttestationData,
-}
-
 /// Wraps a `Attestation` that has been verified up until the point that an `IndexedAttestation` can
 /// be derived.
 ///
@@ -524,7 +520,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
         .tree_hash_root();
 
         // [New in Electra:EIP7549]
-        verify_committee_index(attestation, &chain.spec)?;
+        verify_committee_index(attestation)?;
 
         if chain
             .observed_attestations
@@ -601,59 +597,62 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
                 ))
             }
         };
+
+        // Committees must be sorted by ascending index order 0..committees_per_slot
         let get_indexed_attestation_with_committee =
             |(committees, _): (Vec<BeaconCommittee>, CommitteesPerSlot)| {
-                match signed_aggregate {
-                    SignedAggregateAndProof::Base(signed_aggregate) => {
-                        let att = &signed_aggregate.message.aggregate;
-                        let aggregator_index = signed_aggregate.message.aggregator_index;
-                        let committee = committees
-                            .iter()
-                            .filter(|&committee| committee.index == att.data.index)
-                            .at_most_one()
-                            .map_err(|_| Error::NoCommitteeForSlotAndIndex {
-                                slot: att.data.slot,
-                                index: att.data.index,
-                            })?;
-
-                        // TODO(electra):
+                let (index, aggregator_index, selection_proof, data) = match signed_aggregate {
+                    SignedAggregateAndProof::Base(signed_aggregate) => (
+                        signed_aggregate.message.aggregate.data.index,
+                        signed_aggregate.message.aggregator_index,
                         // Note: this clones the signature which is known to be a relatively slow operation.
-                        //
                         // Future optimizations should remove this clone.
-                        if let Some(committee) = committee {
-                            let selection_proof = SelectionProof::from(
-                                signed_aggregate.message.selection_proof.clone(),
-                            );
-
-                            if !selection_proof
-                                .is_aggregator(committee.committee.len(), &chain.spec)
-                                .map_err(|e| Error::BeaconChainError(e.into()))?
-                            {
-                                return Err(Error::InvalidSelectionProof { aggregator_index });
-                            }
+                        signed_aggregate.message.selection_proof.clone(),
+                        signed_aggregate.message.aggregate.data.clone(),
+                    ),
+                    SignedAggregateAndProof::Electra(signed_aggregate) => (
+                        signed_aggregate
+                            .message
+                            .aggregate
+                            .committee_index()
+                            .ok_or(Error::NotExactlyOneCommitteeBitSet(0))?,
+                        signed_aggregate.message.aggregator_index,
+                        signed_aggregate.message.selection_proof.clone(),
+                        signed_aggregate.message.aggregate.data.clone(),
+                    ),
+                };
+                let slot = data.slot;
 
-                            // Ensure the aggregator is a member of the committee for which it is aggregating.
-                            if !committee.committee.contains(&(aggregator_index as usize)) {
-                                return Err(Error::AggregatorNotInCommittee { aggregator_index });
-                            }
+                let committee = committees
+                    .get(index as usize)
+                    .ok_or(Error::NoCommitteeForSlotAndIndex { slot, index })?;
 
-                            attesting_indices_base::get_indexed_attestation(
-                                committee.committee,
-                                att,
-                            )
-                            .map_err(|e| BeaconChainError::from(e).into())
-                        } else {
-                            Err(Error::NoCommitteeForSlotAndIndex {
-                                slot: att.data.slot,
-                                index: att.data.index,
-                            })
-                        }
+                if !SelectionProof::from(selection_proof)
+                    .is_aggregator(committee.committee.len(), &chain.spec)
+                    .map_err(|e| Error::BeaconChainError(e.into()))?
+                {
+                    return Err(Error::InvalidSelectionProof { aggregator_index });
+                }
+
+                // Ensure the aggregator is a member of the committee for which it is aggregating.
+                if !committee.committee.contains(&(aggregator_index as usize)) {
+                    return Err(Error::AggregatorNotInCommittee { aggregator_index });
+                }
+
+                // p2p aggregates have a single committee, we can assert that aggregation_bits is always
+                // less then MaxValidatorsPerCommittee
+                match signed_aggregate {
+                    SignedAggregateAndProof::Base(signed_aggregate) => {
+                        attesting_indices_base::get_indexed_attestation(
+                            committee.committee,
+                            &signed_aggregate.message.aggregate,
+                        )
+                        .map_err(|e| BeaconChainError::from(e).into())
                     }
                     SignedAggregateAndProof::Electra(signed_aggregate) => {
-                        attesting_indices_electra::get_indexed_attestation_from_signed_aggregate(
+                        attesting_indices_electra::get_indexed_attestation(
                             &committees,
-                            signed_aggregate,
-                            &chain.spec,
+                            &signed_aggregate.message.aggregate,
                         )
                         .map_err(|e| BeaconChainError::from(e).into())
                     }
@@ -842,7 +841,7 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> {
         }
 
         // [New in Electra:EIP7549]
-        verify_committee_index(attestation, &chain.spec)?;
+        verify_committee_index(attestation)?;
 
         // Attestations must be for a known block. If the block is unknown, we simply drop the
         // attestation and do not delay consideration for later.
@@ -1342,17 +1341,10 @@ pub fn verify_signed_aggregate_signatures<T: BeaconChainTypes>(
 
 /// Verify that the `attestation` committee index is properly set for the attestation's fork.
 /// This function will only apply verification post-Electra.
-pub fn verify_committee_index<E: EthSpec>(
-    attestation: AttestationRef<E>,
-    spec: &ChainSpec,
-) -> Result<(), Error> {
-    if spec.fork_name_at_slot::<E>(attestation.data().slot) >= ForkName::Electra {
+pub fn verify_committee_index<E: EthSpec>(attestation: AttestationRef<E>) -> Result<(), Error> {
+    if let Ok(committee_bits) = attestation.committee_bits() {
         // Check to ensure that the attestation is for a single committee.
-        let num_committee_bits = get_committee_indices::<E>(
-            attestation
-                .committee_bits()
-                .map_err(|e| Error::BeaconChainError(e.into()))?,
-        );
+        let num_committee_bits = get_committee_indices::<E>(committee_bits);
         if num_committee_bits.len() != 1 {
             return Err(Error::NotExactlyOneCommitteeBitSet(
                 num_committee_bits.len(),
@@ -1405,8 +1397,7 @@ pub fn obtain_indexed_attestation_and_committees_per_slot<T: BeaconChainTypes>(
                 attesting_indices_electra::get_indexed_attestation(&committees, att)
                     .map(|attestation| (attestation, committees_per_slot))
                     .map_err(|e| {
-                        let index = att.committee_index().unwrap_or(0);
-                        if e == BlockOperationError::BeaconStateError(NoCommitteeFound(index)) {
+                        if let BlockOperationError::BeaconStateError(NoCommitteeFound(index)) = e {
                             Error::NoCommitteeForSlotAndIndex {
                                 slot: att.data.slot,
                                 index,
@@ -1429,6 +1420,8 @@ pub fn obtain_indexed_attestation_and_committees_per_slot<T: BeaconChainTypes>(
 ///
 /// If the committees for an `attestation`'s slot isn't found in the `shuffling_cache`, we will read a state
 /// from disk and then update the `shuffling_cache`.
+///
+/// Committees are sorted by ascending index order 0..committees_per_slot
 fn map_attestation_committees<T, F, R>(
     chain: &BeaconChain<T>,
     attestation: AttestationRef<T::EthSpec>,

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/beacon_chain.rs

@@ -122,7 +122,6 @@ use store::{
 use task_executor::{ShutdownReason, TaskExecutor};
 use tokio_stream::Stream;
 use tree_hash::TreeHash;
-use types::attestation::AttestationBase;
 use types::blob_sidecar::FixedBlobSidecarList;
 use types::payload::BlockProductionVersion;
 use types::*;
@@ -1994,36 +1993,15 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             };
         drop(cache_timer);
 
-        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(),
-            }))
-        }
+        Ok(Attestation::<T::EthSpec>::empty_for_signing(
+            request_index,
+            committee_len,
+            request_slot,
+            beacon_block_root,
+            justified_checkpoint,
+            target,
+            &self.spec,
+        )?)
     }
 
     /// Performs the same validation as `Self::verify_unaggregated_attestation_for_gossip`, but for