Added more test coverage, simplified Attestation conversion, and other minor refactors

beacon_node/beacon_chain/src/attestation_verification.rs

@@ -325,14 +325,16 @@ impl<T: BeaconChainTypes> VerifiedUnaggregatedAttestation<'_, T> {
         self.indexed_attestation
     }
 
-    pub fn single_attestation(&self) -> SingleAttestation {
-        // TODO(single-attestation) unwrap
-        SingleAttestation {
-            committee_index: self.attestation.committee_index().unwrap_or(0) as usize,
+    pub fn single_attestation(&self) -> Option<SingleAttestation> {
+        let Some(committee_index) = self.attestation.committee_index() else {
+            return None;
+        };
+        Some(SingleAttestation {
+            committee_index: committee_index as usize,
             attester_index: self.validator_index,
             data: self.attestation.data().clone(),
             signature: self.attestation.signature().clone(),
-        }
+        })
     }
 }
 

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -2038,9 +2038,13 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                             .spec
                             .fork_name_at_slot::<T::EthSpec>(v.attestation().data().slot);
                         if current_fork.electra_enabled() {
-                            event_handler.register(EventKind::SingleAttestation(Box::new(
-                                v.single_attestation(),
-                            )));
+                            // I don't see a situation where this could return None. The upstream unaggregated attestation checks
+                            // should have already verified that this is an attestation with a single committee bit set.
+                            if let Some(single_attestation) = v.single_attestation() {
+                                event_handler.register(EventKind::SingleAttestation(Box::new(
+                                    single_attestation,
+                                )));
+                            }
                         } else {
                             event_handler.register(EventKind::Attestation(Box::new(
                                 v.attestation().clone_as_attestation(),

beacon_node/beacon_chain/src/test_utils.rs

@@ -668,8 +668,10 @@ pub type CommitteeAttestations<E> = Vec<(Attestation<E>, SubnetId)>;
 pub type HarnessAttestations<E> =
     Vec<(CommitteeAttestations<E>, Option<SignedAggregateAndProof<E>>)>;
 
-pub type HarnessSingleAttestations<E> =
-    Vec<(CommitteeSingleAttestations, Option<SignedAggregateAndProof<E>>)>;
+pub type HarnessSingleAttestations<E> = Vec<(
+    CommitteeSingleAttestations,
+    Option<SignedAggregateAndProof<E>>,
+)>;
 
 pub type HarnessSyncContributions<E> = Vec<(
     Vec<(SyncCommitteeMessage, usize)>,
@@ -1028,6 +1030,7 @@ where
         )
     }
 
+    #[allow(clippy::too_many_arguments)]
     pub fn produce_single_attestation_for_block(
         &self,
         slot: Slot,
@@ -1038,7 +1041,6 @@ where
         aggregation_bit_index: usize,
         validator_index: usize,
     ) -> Result<SingleAttestation, BeaconChainError> {
-
         let epoch = slot.epoch(E::slots_per_epoch());
 
         if state.slot() > slot {
@@ -1063,7 +1065,7 @@ where
             *state.get_block_root(target_slot)?
         };
 
-        let mut attestation: Attestation<E> = Attestation::empty_for_signing(
+        let attestation: Attestation<E> = Attestation::empty_for_signing(
             index,
             committee_len,
             slot,
@@ -1078,29 +1080,29 @@ where
 
         let attestation = match attestation {
             Attestation::Electra(mut attn) => {
-                attn.aggregation_bits.set(aggregation_bit_index, true).unwrap();
+                attn.aggregation_bits
+                    .set(aggregation_bit_index, true)
+                    .unwrap();
                 attn
-            },
+            }
             Attestation::Base(_) => panic!("MUST BE AN ELECTRA ATTESTATION"),
         };
 
-        let committee = state.get_beacon_committee(slot, index)?;
+        let committee = state.get_beacon_committee(slot, index).unwrap();
 
-        // let committees = state.get_beacon_committees_at_epoch(RelativeEpoch::Current)?;
+        let single_attestation = attestation.to_single_attestation(Some(committee.clone()))?;
 
-        let single_attestation = attestation.to_single_attestation(
-            &vec![committee.clone()]
-        )?;
+        let attestation: Attestation<E> = single_attestation.to_attestation(Some(committee))?;
 
-        let attestation: Attestation<E> = single_attestation.to_attestation(&vec![committee])?;
-
-        assert_eq!(single_attestation.committee_index, attestation.committee_index().unwrap() as usize);
+        assert_eq!(
+            single_attestation.committee_index,
+            attestation.committee_index().unwrap() as usize
+        );
         assert_eq!(single_attestation.attester_index, validator_index);
         // assert_eq!(single_attestation.attester_index, attestation.attester_index());
         Ok(single_attestation)
     }
 
-
     /// Produces an "unaggregated" attestation for the given `slot` and `index` that attests to
     /// `beacon_block_root`. The provided `state` should match the `block.state_root` for the
     /// `block` identified by `beacon_block_root`.
@@ -1158,7 +1160,7 @@ where
         )?)
     }
 
-     /// A list of attestations for each committee for the given slot.
+    /// A list of attestations for each committee for the given slot.
     ///
     /// The first layer of the Vec is organised per committee. For example, if the return value is
     /// called `all_attestations`, then all attestations in `all_attestations[0]` will be for
@@ -1255,7 +1257,7 @@ where
                                 Cow::Borrowed(state),
                                 state_root,
                                 i,
-                                *validator_index
+                                *validator_index,
                             )
                             .unwrap();
 
@@ -1485,7 +1487,7 @@ where
         )
     }
 
-     /// A list of attestations for each committee for the given slot.
+    /// A list of attestations for each committee for the given slot.
     ///
     /// The first layer of the Vec is organised per committee. For example, if the return value is
     /// called `all_attestations`, then all attestations in `all_attestations[0]` will be for