allow duplicates in indexed payload attestation indices

Author: shane-moore

consensus/state_processing/src/per_block_processing/is_valid_indexed_payload_attestation.rs

@@ -14,31 +14,31 @@ pub fn is_valid_indexed_payload_attestation<E: EthSpec>(
     verify_signatures: VerifySignatures,
     spec: &ChainSpec,
 ) -> Result<(), BlockOperationError<Invalid>> {
-    // Verify indices are sorted and unique
+    // Verify indices are non-empty and sorted (duplicates allowed)
     let indices = &indexed_payload_attestation.attesting_indices;
     verify!(!indices.is_empty(), Invalid::IndicesEmpty);
     let check_sorted = |list: &[u64]| -> Result<(), BlockOperationError<Invalid>> {
         list.iter()
             .tuple_windows()
             .enumerate()
             .try_for_each(|(i, (x, y))| {
-                if x < y {
+                if x <= y {
                     Ok(())
                 } else {
                     Err(error(Invalid::BadValidatorIndicesOrdering(i)))
                 }
             })?;
         Ok(())
     };
-    check_sorted(&indices)?;
+    check_sorted(indices)?;
 
     if verify_signatures.is_true() {
         verify!(
             indexed_payload_attestation_signature_set(
                 state,
                 |i| get_pubkey_from_state(state, i),
                 &indexed_payload_attestation.signature,
-                &indexed_payload_attestation,
+                indexed_payload_attestation,
                 spec
             )?
             .verify(),

consensus/state_processing/src/per_block_processing/process_operations.rs

@@ -796,6 +796,11 @@ pub fn process_consolidation_request<E: EthSpec>(
     Ok(())
 }
 
+// TODO(EIP-7732): Add test cases for `process_payload_attestations` to
+// `consensus/state_processing/src/per_block_processing/tests.rs`.
+// The tests will require being able to build Gloas blocks with PayloadAttestations,
+// which currently fails due to incomplete Gloas block structure as mentioned here
+// https://github.com/sigp/lighthouse/pull/8273
 pub fn process_payload_attestation<E: EthSpec>(
     state: &mut BeaconState<E>,
     payload_attestation: &PayloadAttestation<E>,

consensus/state_processing/src/per_block_processing/verify_payload_attestation.rs

@@ -2,6 +2,7 @@ use super::VerifySignatures;
 use super::errors::{BlockOperationError, PayloadAttestationInvalid as Invalid};
 use crate::ConsensusContext;
 use crate::per_block_processing::is_valid_indexed_payload_attestation;
+use safe_arith::SafeArith;
 use types::*;
 
 pub fn verify_payload_attestation<'ctxt, E: EthSpec>(
@@ -24,7 +25,7 @@ pub fn verify_payload_attestation<'ctxt, E: EthSpec>(
 
     // Check that the attestation is for the previous slot
     verify!(
-        data.slot + 1 == state.slot(),
+        data.slot.safe_add(1)? == state.slot(),
         Invalid::SlotMismatch {
             expected: state.slot().saturating_sub(Slot::new(1)),
             found: data.slot,
@@ -36,7 +37,7 @@ pub fn verify_payload_attestation<'ctxt, E: EthSpec>(
 
     is_valid_indexed_payload_attestation(
         state,
-        &indexed_payload_attestation,
+        indexed_payload_attestation,
         verify_signatures,
         spec,
     )?;

consensus/types/src/beacon_state.rs

@@ -1083,7 +1083,7 @@ impl<E: EthSpec> BeaconState<E> {
 
                 if self.fork_name_unchecked().gloas_enabled() {
                     self.compute_balance_weighted_selection(indices, &seed, 1, true, spec)?
-                        .get(0)
+                        .first()
                         .copied()
                         .ok_or(Error::InsufficientValidators)
                 } else {
@@ -1328,6 +1328,7 @@ impl<E: EthSpec> BeaconState<E> {
     /// Compute the sync committee indices for the next sync committee.
     fn get_next_sync_committee_indices(&self, spec: &ChainSpec) -> Result<Vec<usize>, Error> {
         let epoch = self.current_epoch().safe_add(1)?;
+
         let active_validator_indices = self.get_active_validator_indices(epoch, spec)?;
         let seed = self.get_seed(epoch, Domain::SyncCommittee, spec)?;
 
@@ -2795,8 +2796,6 @@ impl<E: EthSpec> BeaconState<E> {
     ///
     /// If shuffle_indices is True, candidate indices are themselves sampled from indices
     /// by shuffling it, otherwise indices is traversed in order.
-    ///
-    /// Spec: compute_balance_weighted_selection helper
     fn compute_balance_weighted_selection(
         &self,
         indices: &[usize],
@@ -2814,7 +2813,7 @@ impl<E: EthSpec> BeaconState<E> {
         let mut count = 0usize;
 
         while selected.len() < size {
-            let mut next_index = count % total;
+            let mut next_index = count.safe_rem(total)?;
 
             if shuffle_indices {
                 next_index =
@@ -2828,7 +2827,7 @@ impl<E: EthSpec> BeaconState<E> {
                 selected.push(*candidate_index);
             }
 
-            count += 1;
+            count.safe_add_assign(1)?;
         }
 
         Ok(selected)
@@ -2855,7 +2854,8 @@ impl<E: EthSpec> BeaconState<E> {
             MAX_RANDOM_BYTE
         };
 
-        Ok(effective_balance * max_random_value >= max_effective_balance * random_value)
+        Ok(effective_balance.safe_mul(max_random_value)?
+            >= max_effective_balance.safe_mul(random_value)?)
     }
 }
 

consensus/types/src/beacon_state/tests.rs

@@ -53,7 +53,15 @@ async fn build_state<E: EthSpec>(validator_count: usize) -> BeaconState<E> {
         .head_beacon_state_cloned()
 }
 
-/// TODO(EIP7732) Add test for ptc attester index using get_ptc_attester_seed
+/// TODO(EIP-7732): Add tests for PTC (Payload Timeliness Committee) functions:
+/// - get_ptc: Test committee selection, size, balance-weighted selection
+/// - get_ptc_attester_seed: Test seed generation and determinism
+/// - compute_balance_weighted_selection: Test selection algorithm with various balances
+/// - compute_balance_weighted_acceptance: Test acceptance probability
+/// These tests require being able to build Gloas states with initialized committee caches,
+/// which currently fails due to incomplete Gloas block structure as mentioned here:
+/// https://github.com/sigp/lighthouse/pull/8273
+/// Similar to existing committee_consistency_test suite for get_beacon_committee.
 
 async fn test_beacon_proposer_index<E: EthSpec>() {
     let spec = E::default_spec();

consensus/types/src/ptc.rs

@@ -21,108 +21,3 @@ impl<E: EthSpec> IntoIterator for PTC<E> {
         self.0.into_iter()
     }
 }
-
-// TODO(EIP7732) Mark's implementation below was using from_committees to filter out ptc members from attestation committees, but I don't see that in the spec, so perhaps is a deprecated implementation
-// impl<E: EthSpec> PTC<E> {
-//     pub fn from_committees(committees: &[BeaconCommittee]) -> Result<Self, BeaconStateError> {
-//         // this function is only used here and
-//         // I have no idea where else to put it
-//         fn bit_floor(n: u64) -> u64 {
-//             if n == 0 {
-//                 0
-//             } else {
-//                 1 << (n.leading_zeros() as u64 ^ 63)
-//             }
-//         }
-
-//         let committee_count_per_slot = committees.len() as u64;
-//         let committees_per_slot = bit_floor(std::cmp::min(
-//             committee_count_per_slot,
-//             E::PTCSize::to_u64(),
-//         )) as usize;
-//         let members_per_committee = E::PTCSize::to_usize().safe_div(committees_per_slot)?;
-
-//         let mut ptc = Vec::with_capacity(E::PTCSize::to_usize());
-//         for idx in 0..committees_per_slot {
-//             let beacon_committee = committees
-//                 .get(idx as usize)
-//                 .ok_or_else(|| Error::InvalidCommitteeIndex(idx as u64))?;
-//             ptc.extend_from_slice(&beacon_committee.committee[..members_per_committee]);
-//         }
-
-//         Ok(Self(FixedVector::from(ptc)))
-//     }
-// }
-
-// impl<'a, E: EthSpec> IntoIterator for &'a PTC<E> {
-//     type Item = &'a usize;
-//     type IntoIter = std::slice::Iter<'a, usize>;
-
-//     fn into_iter(self) -> Self::IntoIter {
-//         self.0.iter()
-//     }
-// }
-
-// impl<E: EthSpec> IntoIterator for PTC<E> {
-//     type Item = usize;
-//     type IntoIter = std::vec::IntoIter<usize>;
-
-//     fn into_iter(self) -> Self::IntoIter {
-//         self.0.into_iter()
-//     }
-// }
-// use crate::*;
-// use safe_arith::SafeArith;
-
-// /// TODO(EIP-7732): is it easier to return u64 or usize?
-// #[derive(Clone, Debug, PartialEq)]
-// pub struct PTC<E: EthSpec>(FixedVector<usize, E::PTCSize>);
-
-// impl<E: EthSpec> PTC<E> {
-//     pub fn from_committees(committees: &[BeaconCommittee]) -> Result<Self, BeaconStateError> {
-//         // this function is only used here and
-//         // I have no idea where else to put it
-//         fn bit_floor(n: u64) -> u64 {
-//             if n == 0 {
-//                 0
-//             } else {
-//                 1 << (n.leading_zeros() as u64 ^ 63)
-//             }
-//         }
-
-//         let committee_count_per_slot = committees.len() as u64;
-//         let committees_per_slot = bit_floor(std::cmp::min(
-//             committee_count_per_slot,
-//             E::PTCSize::to_u64(),
-//         )) as usize;
-//         let members_per_committee = E::PTCSize::to_usize().safe_div(committees_per_slot)?;
-
-//         let mut ptc = Vec::with_capacity(E::PTCSize::to_usize());
-//         for idx in 0..committees_per_slot {
-//             let beacon_committee = committees
-//                 .get(idx as usize)
-//                 .ok_or_else(|| Error::InvalidCommitteeIndex(idx as u64))?;
-//             ptc.extend_from_slice(&beacon_committee.committee[..members_per_committee]);
-//         }
-
-//         Ok(Self(FixedVector::from(ptc)))
-//     }
-// }
-
-// impl<'a, E: EthSpec> IntoIterator for &'a PTC<E> {
-//     type Item = &'a usize;
-//     type IntoIter = std::slice::Iter<'a, usize>;
-
-//     fn into_iter(self) -> Self::IntoIter {
-//         self.0.iter()
-//     }
-// }
-
-// impl<E: EthSpec> IntoIterator for PTC<E> {
-//     type Item = usize;
-//     type IntoIter = std::vec::IntoIter<usize>;
-
-//     fn into_iter(self) -> Self::IntoIter {
-//         self.0.into_iter()
-//     }
-// }