Gloas attestation verification (#8705)

https://github.com/ethereum/consensus-specs/blob/master/specs/gloas/p2p-interface.md#attestation-subnets

Implements attestation verification logic for Gloas and adds a few gloas related tests. Note that a few of these tests rely on gloas test harness block production which hasn't been built out yet. So for now those tests are ignored.


  


Co-Authored-By: Eitan Seri- Levi <eserilev@gmail.com>

Co-Authored-By: Eitan Seri-Levi <eserilev@ucsc.edu>

beacon_node/beacon_chain/src/attestation_verification.rs

@@ -61,8 +61,9 @@ use tracing::{debug, error};
 use tree_hash::TreeHash;
 use types::{
     Attestation, AttestationData, AttestationRef, BeaconCommittee,
-    BeaconStateError::NoCommitteeFound, ChainSpec, CommitteeIndex, Epoch, EthSpec, Hash256,
+    BeaconStateError::NoCommitteeFound, ChainSpec, CommitteeIndex, Epoch, EthSpec, ForkName,
-    IndexedAttestation, SelectionProof, SignedAggregateAndProof, SingleAttestation, Slot, SubnetId,
+    Hash256, IndexedAttestation, SelectionProof, SignedAggregateAndProof, SingleAttestation, Slot,
+    SubnetId,
 };
 
 pub use batch::{batch_verify_aggregated_attestations, batch_verify_unaggregated_attestations};
@@ -160,6 +161,12 @@ pub enum Error {
     ///
     /// The peer has sent an invalid message.
     CommitteeIndexNonZero(usize),
+    /// The validator index is set to an invalid value after Gloas.
+    ///
+    /// ## Peer scoring
+    ///
+    /// The peer has sent an invalid message.
+    CommitteeIndexInvalid,
     /// The `attestation.data.beacon_block_root` block is unknown.
     ///
     /// ## Peer scoring
@@ -550,8 +557,12 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
         }
         .tree_hash_root();
 
+        let fork_name = chain
+            .spec
+            .fork_name_at_slot::<T::EthSpec>(attestation.data().slot);
+
         // [New in Electra:EIP7549]
-        verify_committee_index(attestation)?;
+        verify_committee_index(attestation, fork_name)?;
 
         if chain
             .observed_attestations
@@ -595,6 +606,17 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
         // attestation and do not delay consideration for later.
         let head_block = verify_head_block_is_known(chain, attestation.data(), None)?;
 
+        // [New in Gloas]: If the attested block is from the same slot as the attestation,
+        // index must be 0.
+        if fork_name.gloas_enabled()
+            && head_block.slot == attestation.data().slot
+            && attestation.data().index != 0
+        {
+            return Err(Error::CommitteeIndexNonZero(
+                attestation.data().index as usize,
+            ));
+        }
+
         // Check the attestation target root is consistent with the head root.
         //
         // This check is not in the specification, however we guard against it since it opens us up
@@ -871,7 +893,12 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> {
         let fork_name = chain
             .spec
             .fork_name_at_slot::<T::EthSpec>(attestation.data.slot);
-        if fork_name.electra_enabled() {
+        if fork_name.gloas_enabled() {
+            // [New in Gloas]
+            if attestation.data.index >= 2 {
+                return Err(Error::CommitteeIndexInvalid);
+            }
+        } else if fork_name.electra_enabled() {
             // [New in Electra:EIP7549]
             if attestation.data.index != 0 {
                 return Err(Error::CommitteeIndexNonZero(
@@ -890,6 +917,17 @@ impl<'a, T: BeaconChainTypes> IndexedUnaggregatedAttestation<'a, T> {
             chain.config.import_max_skip_slots,
         )?;
 
+        // [New in Gloas]: If the attested block is from the same slot as the attestation,
+        // index must be 0.
+        if fork_name.gloas_enabled()
+            && head_block.slot == attestation.data.slot
+            && attestation.data.index != 0
+        {
+            return Err(Error::CommitteeIndexNonZero(
+                attestation.data.index as usize,
+            ));
+        }
+
         // Check the attestation target root is consistent with the head root.
         verify_attestation_target_root::<T::EthSpec>(&head_block, &attestation.data)?;
 
@@ -1404,7 +1442,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>) -> Result<(), Error> {
+pub fn verify_committee_index<E: EthSpec>(
+    attestation: AttestationRef<E>,
+    fork_name: ForkName,
+) -> 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>(committee_bits);
@@ -1414,11 +1455,18 @@ pub fn verify_committee_index<E: EthSpec>(attestation: AttestationRef<E>) -> Res
             ));
         }
 
-        // Ensure the attestation index is set to zero post Electra.
+        // Ensure the attestation index is valid for the fork.
-        if attestation.data().index != 0 {
+        let index = attestation.data().index;
-            return Err(Error::CommitteeIndexNonZero(
+        if fork_name.gloas_enabled() {
-                attestation.data().index as usize,
+            // [New in Gloas]: index must be < 2.
-            ));
+            if index >= 2 {
+                return Err(Error::CommitteeIndexInvalid);
+            }
+        } else {
+            // [New in Electra:EIP7549]: index must be 0.
+            if index != 0 {
+                return Err(Error::CommitteeIndexNonZero(index as usize));
+            }
         }
     }
     Ok(())

beacon_node/beacon_chain/tests/attestation_verification.rs

@@ -368,6 +368,13 @@ impl GossipTester {
         self.harness.chain.epoch().unwrap()
     }
 
+    pub fn is_gloas(&self) -> bool {
+        self.harness
+            .spec
+            .fork_name_at_slot::<E>(self.valid_attestation.data.slot)
+            .gloas_enabled()
+    }
+
     pub fn earliest_valid_attestation_slot(&self) -> Slot {
         let offset = if self
             .harness
@@ -522,6 +529,44 @@ impl GossipTester {
 
         self
     }
+
+    /// Like `inspect_aggregate_err`, but only runs the check if gloas is enabled.
+    /// If gloas is not enabled, this is a no-op that returns self.
+    pub fn inspect_aggregate_err_if_gloas<G, I>(
+        self,
+        desc: &str,
+        get_attn: G,
+        inspect_err: I,
+    ) -> Self
+    where
+        G: Fn(&Self, &mut SignedAggregateAndProof<E>),
+        I: Fn(&Self, AttnError),
+    {
+        if self.is_gloas() {
+            self.inspect_aggregate_err(desc, get_attn, inspect_err)
+        } else {
+            self
+        }
+    }
+
+    /// Like `inspect_unaggregate_err`, but only runs the check if gloas is enabled.
+    /// If gloas is not enabled, this is a no-op that returns self.
+    pub fn inspect_unaggregate_err_if_gloas<G, I>(
+        self,
+        desc: &str,
+        get_attn: G,
+        inspect_err: I,
+    ) -> Self
+    where
+        G: Fn(&Self, &mut SingleAttestation, &mut SubnetId, &ChainSpec),
+        I: Fn(&Self, AttnError),
+    {
+        if self.is_gloas() {
+            self.inspect_unaggregate_err(desc, get_attn, inspect_err)
+        } else {
+            self
+        }
+    }
 }
 /// Tests verification of `SignedAggregateAndProof` from the gossip network.
 #[tokio::test]
@@ -854,6 +899,27 @@ async fn aggregated_gossip_verification() {
                 ))
             },
         )
+        /*
+         * [New in Gloas]: attestation.data.index must be < 2
+         */
+        .inspect_aggregate_err_if_gloas(
+            "gloas: aggregate with index >= 2",
+            |_, a| match a.to_mut() {
+                SignedAggregateAndProofRefMut::Base(_) => {
+                    panic!("Expected Electra attestation variant");
+                }
+                SignedAggregateAndProofRefMut::Electra(att) => {
+                    att.message.aggregate.data.index = 2;
+                }
+            },
+            |_, err| {
+                assert!(
+                    matches!(err, AttnError::CommitteeIndexInvalid),
+                    "expected CommitteeIndexInvalid, got {:?}",
+                    err
+                )
+            },
+        )
         // NOTE: from here on, the tests are stateful, and rely on the valid attestation having
         // been seen.
         .import_valid_aggregate()
@@ -1071,6 +1137,22 @@ async fn unaggregated_gossip_verification() {
                 ))
             },
         )
+        /*
+         * [New in Gloas]: attestation.data.index must be < 2
+         */
+        .inspect_unaggregate_err_if_gloas(
+            "gloas: attestation with index >= 2",
+            |_, a, _, _| {
+                a.data.index = 2;
+            },
+            |_, err| {
+                assert!(
+                    matches!(err, AttnError::CommitteeIndexInvalid),
+                    "expected CommitteeIndexInvalid, got {:?}",
+                    err
+                )
+            },
+        )
         // NOTE: from here on, the tests are stateful, and rely on the valid attestation having
         // been seen.
         .import_valid_unaggregate()
@@ -1700,3 +1782,180 @@ async fn aggregated_attestation_verification_use_head_state_fork() {
         );
     }
 }
+
+/// [New in Gloas]: Tests that unaggregated attestations with `data.index == 1` are rejected
+/// when `head_block.slot == attestation.data.slot`.
+///
+/// This test only runs when `FORK_NAME=gloas` is set with `fork_from_env` feature.
+// TODO(EIP-7732): Enable this test once gloas block production works in test harness.
+// `state.latest_execution_payload_header()` not available in Gloas.
+#[ignore]
+#[tokio::test]
+async fn gloas_unaggregated_attestation_same_slot_index_must_be_zero() {
+    let harness = get_harness(VALIDATOR_COUNT);
+
+    // Skip this test if not running with gloas fork
+    if !harness
+        .spec
+        .fork_name_at_epoch(Epoch::new(0))
+        .gloas_enabled()
+    {
+        return;
+    }
+
+    // Extend the chain out a few epochs so we have some chain depth to play with.
+    harness
+        .extend_chain(
+            MainnetEthSpec::slots_per_epoch() as usize * 3 - 1,
+            BlockStrategy::OnCanonicalHead,
+            AttestationStrategy::AllValidators,
+        )
+        .await;
+
+    // Produce a block in the current slot (this creates the same-slot scenario)
+    harness
+        .extend_chain(
+            1,
+            BlockStrategy::OnCanonicalHead,
+            AttestationStrategy::SomeValidators(vec![]),
+        )
+        .await;
+
+    let current_slot = harness.chain.slot().expect("should get slot");
+    let head = harness.chain.head_snapshot();
+
+    // Verify head block is in the current slot
+    assert_eq!(
+        head.beacon_block.slot(),
+        current_slot,
+        "head block should be in current slot for same-slot test"
+    );
+
+    // Produce an attestation for the current slot
+    let (mut attestation, _attester_sk, subnet_id) =
+        get_valid_unaggregated_attestation(&harness.chain);
+
+    // Verify we have a same-slot scenario
+    let attested_block_slot = harness
+        .chain
+        .canonical_head
+        .fork_choice_read_lock()
+        .get_block(&attestation.data.beacon_block_root)
+        .expect("block should exist")
+        .slot;
+    assert_eq!(
+        attested_block_slot, attestation.data.slot,
+        "attested block slot should equal attestation slot for same-slot test"
+    );
+
+    // index == 1 should be rejected when head_block.slot == attestation.data.slot
+    attestation.data.index = 1;
+    let result = harness
+        .chain
+        .verify_unaggregated_attestation_for_gossip(&attestation, Some(subnet_id));
+    assert!(
+        matches!(result, Err(AttnError::CommitteeIndexNonZero(_))),
+        "gloas: attestation with index == 1 when head_block.slot == attestation.data.slot should be rejected, got {:?}",
+        result.err()
+    );
+}
+
+/// [New in Gloas]: Tests that aggregated attestations with `data.index == 1` are rejected
+/// when `head_block.slot == attestation.data.slot`.
+///
+/// This test only runs when `FORK_NAME=gloas` is set with `fork_from_env` feature.
+// TODO(EIP-7732): Enable this test once gloas block production works in test harness.
+// `state.latest_execution_payload_header()` not available in Gloas.
+#[ignore]
+#[tokio::test]
+async fn gloas_aggregated_attestation_same_slot_index_must_be_zero() {
+    let harness = get_harness(VALIDATOR_COUNT);
+
+    // Skip this test if not running with gloas fork
+    if !harness
+        .spec
+        .fork_name_at_epoch(Epoch::new(0))
+        .gloas_enabled()
+    {
+        return;
+    }
+
+    // Extend the chain out a few epochs so we have some chain depth to play with.
+    harness
+        .extend_chain(
+            MainnetEthSpec::slots_per_epoch() as usize * 3 - 1,
+            BlockStrategy::OnCanonicalHead,
+            AttestationStrategy::AllValidators,
+        )
+        .await;
+
+    // Produce a block in the current slot (this creates the same-slot scenario)
+    harness
+        .extend_chain(
+            1,
+            BlockStrategy::OnCanonicalHead,
+            AttestationStrategy::SomeValidators(vec![]),
+        )
+        .await;
+
+    let current_slot = harness.chain.slot().expect("should get slot");
+    let head = harness.chain.head_snapshot();
+
+    // Verify head block is in the current slot
+    assert_eq!(
+        head.beacon_block.slot(),
+        current_slot,
+        "head block should be in current slot for same-slot test"
+    );
+
+    // Produce an attestation for the current slot
+    let (valid_attestation, _attester_sk, _subnet_id) =
+        get_valid_unaggregated_attestation(&harness.chain);
+
+    // Verify we have a same-slot scenario
+    let attested_block_slot = harness
+        .chain
+        .canonical_head
+        .fork_choice_read_lock()
+        .get_block(&valid_attestation.data.beacon_block_root)
+        .expect("block should exist")
+        .slot;
+    assert_eq!(
+        attested_block_slot, valid_attestation.data.slot,
+        "attested block slot should equal attestation slot for same-slot test"
+    );
+
+    // Convert to aggregate
+    let committee = head
+        .beacon_state
+        .get_beacon_committee(current_slot, valid_attestation.committee_index)
+        .expect("should get committee");
+    let fork_name = harness
+        .spec
+        .fork_name_at_slot::<E>(valid_attestation.data.slot);
+    let aggregate_attestation =
+        single_attestation_to_attestation(&valid_attestation, committee.committee, fork_name)
+            .unwrap();
+
+    let (mut valid_aggregate, _, _) =
+        get_valid_aggregated_attestation(&harness.chain, aggregate_attestation);
+
+    // index == 1 should be rejected when head_block.slot == attestation.data.slot
+    match valid_aggregate.to_mut() {
+        SignedAggregateAndProofRefMut::Base(att) => {
+            att.message.aggregate.data.index = 1;
+        }
+        SignedAggregateAndProofRefMut::Electra(att) => {
+            att.message.aggregate.data.index = 1;
+        }
+    }
+
+    let result = harness
+        .chain
+        .verify_aggregated_attestation_for_gossip(&valid_aggregate);
+    assert!(
+        matches!(result, Err(AttnError::CommitteeIndexNonZero(_))),
+        "gloas: aggregate with index == 1 when head_block.slot == attestation.data.slot should be rejected, got {:?}",
+        result.err()
+    );
+}

beacon_node/network/src/network_beacon_processor/gossip_methods.rs

@@ -2415,6 +2415,25 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
                     "attn_comm_index_non_zero",
                 );
             }
+            AttnError::CommitteeIndexInvalid => {
+                /*
+                 * The committee index is invalid after Gloas.
+                 *
+                 * The peer has published an invalid consensus message.
+                 */
+                debug!(
+                    %peer_id,
+                    block = ?beacon_block_root,
+                    ?attestation_type,
+                    "Committee index invalid"
+                );
+                self.propagate_validation_result(message_id, peer_id, MessageAcceptance::Reject);
+                self.gossip_penalize_peer(
+                    peer_id,
+                    PeerAction::LowToleranceError,
+                    "attn_comm_index_invalid",
+                );
+            }
             AttnError::UnknownHeadBlock { beacon_block_root } => {
                 trace!(
                     %peer_id,