Resolve merge conflicts

consensus/fork_choice/src/fork_choice.rs

@@ -1220,7 +1220,6 @@ where
     fn validate_on_payload_attestation(
         &self,
         indexed_payload_attestation: &IndexedPayloadAttestation<E>,
-        is_from_block: AttestationFromBlock,
     ) -> Result<(), InvalidPayloadAttestation> {
         // This check is from `is_valid_indexed_payload_attestation`, but we do it immediately to
         // avoid wasting time on junk attestations.
@@ -1246,25 +1245,6 @@ where
             });
         }
 
-        // PTC votes can only change the vote for their assigned beacon block, return early otherwise
-        if block.slot != indexed_payload_attestation.data.slot {
-            return Ok(());
-        }
-
-        // Gossip payload attestations must be for the current slot.
-        // NOTE: signature is assumed to have been verified by caller.
-        // https://github.com/ethereum/consensus-specs/blob/master/specs/gloas/fork-choice.md
-        if matches!(is_from_block, AttestationFromBlock::False)
-            && indexed_payload_attestation.data.slot != self.fc_store.get_current_slot()
-        {
-            return Err(
-                InvalidPayloadAttestation::PayloadAttestationNotCurrentSlot {
-                    attestation_slot: indexed_payload_attestation.data.slot,
-                    current_slot: self.fc_store.get_current_slot(),
-                },
-            );
-        }
-
         Ok(())
     }
 
@@ -1352,34 +1332,69 @@ where
     pub fn on_payload_attestation(
         &mut self,
         system_time_current_slot: Slot,
-        attestation: &IndexedPayloadAttestation<E>,
+        payload_attestation: &IndexedPayloadAttestation<E>,
         is_from_block: AttestationFromBlock,
         ptc: &[usize],
     ) -> Result<(), Error<T::Error>> {
         self.update_time(system_time_current_slot)?;
 
-        if attestation.data.beacon_block_root.is_zero() {
+        if payload_attestation.data.beacon_block_root.is_zero() {
             return Ok(());
         }
 
         // TODO(gloas): Should ignore wrong-slot payload attestations at the caller, they could
         // have been processed at the correct slot when received on gossip, but then have the
         // wrong-slot by the time they make it to here (TOCTOU).
-        self.validate_on_payload_attestation(attestation, is_from_block)?;
+        // TODO(gloas): Consider inlining validate_on_payload_attestation here to look more like the spec.
+        self.validate_on_payload_attestation(payload_attestation)?;
 
-        // Resolve validator indices to PTC committee positions.
-        let ptc_indices: Vec<usize> = attestation
-            .attesting_indices
-            .iter()
-            .filter_map(|validator_index| ptc.iter().position(|&p| p == *validator_index as usize))
-            .collect();
+        // PTC votes can only change the vote for their assigned beacon block, return early otherwise.
+        let block = self
+            .proto_array
+            .get_block(&payload_attestation.data.beacon_block_root)
+            .ok_or(InvalidPayloadAttestation::UnknownHeadBlock {
+                beacon_block_root: payload_attestation.data.beacon_block_root,
+            })?;
+        if block.slot != payload_attestation.data.slot {
+            return Ok(());
+        }
+
+        // Gossip payload attestations must be for the current slot.
+        if matches!(is_from_block, AttestationFromBlock::False)
+            && payload_attestation.data.slot != self.fc_store.get_current_slot()
+        {
+            return Err(
+                InvalidPayloadAttestation::PayloadAttestationNotCurrentSlot {
+                    attestation_slot: payload_attestation.data.slot,
+                    current_slot: self.fc_store.get_current_slot(),
+                }
+                .into(),
+            );
+        }
+
+        // Resolve validator indices to all PTC committee positions. A validator may
+        // appear multiple times in the PTC committee.
+        let mut ptc_indices = vec![];
+        let mut validators_found = 0;
+        for validator_index in payload_attestation.attesting_indices.iter() {
+            let mut found = false;
+            for (ptc_index, &ptc_validator_index) in ptc.iter().enumerate() {
+                if ptc_validator_index == *validator_index as usize {
+                    ptc_indices.push(ptc_index);
+                    found = true;
+                }
+            }
+            if found {
+                validators_found += 1;
+            }
+        }
 
         // Check that all the attesters are in the PTC
-        if ptc_indices.len() != attestation.attesting_indices.len() {
+        if validators_found != payload_attestation.attesting_indices.len() {
             return Err(
                 InvalidPayloadAttestation::PayloadAttestationAttestersNotInPtc {
-                    attesting_indices_len: attestation.attesting_indices.len(),
-                    attesting_indices_in_ptc: ptc_indices.len(),
+                    attesting_indices_len: payload_attestation.attesting_indices.len(),
+                    attesting_indices_in_ptc: validators_found,
                 }
                 .into(),
             );
@@ -1387,10 +1402,10 @@ where
 
         for &ptc_index in &ptc_indices {
             self.proto_array.process_payload_attestation(
-                attestation.data.beacon_block_root,
+                payload_attestation.data.beacon_block_root,
                 ptc_index,
-                attestation.data.payload_present,
-                attestation.data.blob_data_available,
+                payload_attestation.data.payload_present,
+                payload_attestation.data.blob_data_available,
             )?;
         }
 
@@ -1527,9 +1542,23 @@ where
         }
     }
 
-    /// Returns whether the payload envelope has been received for the given block.
+    /// Returns whether the execution payload for a block has been received.
+    ///
+    /// Returns `false` for unknown blocks and pre-Gloas nodes.
     pub fn is_payload_received(&self, block_root: &Hash256) -> bool {
         self.proto_array.is_payload_received(block_root)
+            && self.is_finalized_checkpoint_or_descendant(*block_root)
+    }
+
+    /// Called by the proposer to decide whether to build on the full or empty parent.
+    pub fn should_build_on_full(
+        &self,
+        block_root: &Hash256,
+        parent_payload_status: PayloadStatus,
+    ) -> Result<bool, Error<T::Error>> {
+        self.proto_array
+            .should_build_on_full::<E>(block_root, parent_payload_status)
+            .map_err(Error::ProtoArrayStringError)
     }
 
     /// Returns whether the proposer should extend the execution payload chain of the given block.

consensus/fork_choice/tests/tests.rs

@@ -79,7 +79,7 @@ impl ForkChoiceTest {
     /// Get a value from the `ForkChoice` instantiation.
     fn get<T, U>(&self, func: T) -> U
     where
-        T: Fn(&BeaconForkChoiceStore<E, MemoryStore<E>, MemoryStore<E>>) -> U,
+        T: Fn(&BeaconForkChoiceStore<E, MemoryStore, MemoryStore>) -> U,
     {
         func(
             self.harness