Address review comments

- data_availability_checker.rs: use !gloas_enabled() instead of < ForkName::Gloas
  (jimmygchen, dapplion).
- beacon_chain.rs: get_data_columns checks data_availability_checker first,
  then pending_payload_cache (dapplion).
- pending_components.rs: merge_data_columns drops the unused Result return
  (jimmygchen). num_completed_columns uses filter() instead of filter_map (jimmygchen).
- pending_column.rs: TODO marker on the hard-coded Gloas variant in
  try_to_sidecar (jimmygchen).
- pending_payload_cache/mod.rs: gloas_spec test helper collapsed to
  ForkName::Gloas.make_genesis_spec(E::default_spec()) (jimmygchen).
- gossip_methods.rs / sync/manager.rs: replace UnknownBlockHashFromAttestation
  fallback with TODO(gloas) for proper Gloas lookup sync (dapplion).

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -1187,9 +1187,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         indices: &[ColumnIndex],
     ) -> Result<DataColumnSidecarList<T::EthSpec>, Error> {
         let all_cached_columns_opt = self
-            .pending_payload_cache
+            .data_availability_checker
             .get_data_columns(block_root)
-            .or_else(|| self.data_availability_checker.get_data_columns(block_root))
+            .or_else(|| self.pending_payload_cache.get_data_columns(block_root))
             .or_else(|| self.early_attester_cache.get_data_columns(block_root));
 
         if let Some(mut all_cached_columns) = all_cached_columns_opt {

beacon_node/beacon_chain/src/data_availability_checker.rs

@@ -21,7 +21,7 @@ use tracing::{debug, error, instrument};
 use types::data::{BlobIdentifier, FixedBlobSidecarList, PartialDataColumn};
 use types::{
     BlobSidecar, BlobSidecarList, BlockImportSource, ChainSpec, DataColumnSidecar,
-    DataColumnSidecarList, Epoch, EthSpec, ForkName, Hash256, PartialDataColumnSidecarError,
+    DataColumnSidecarList, Epoch, EthSpec, Hash256, PartialDataColumnSidecarError,
     PartialDataColumnSidecarRef, SignedBeaconBlock, Slot, new_non_zero_usize,
 };
 
@@ -906,7 +906,7 @@ impl<E: EthSpec> AvailableBlock<E> {
         match &block_data {
             AvailableBlockData::NoData => {
                 // For Gloas, DA is checked for the PayloadEnvelope, not for the block.
-                if block.fork_name_unchecked() < ForkName::Gloas {
+                if !block.fork_name_unchecked().gloas_enabled() {
                     if columns_required {
                         return Err(AvailabilityCheckError::MissingCustodyColumns);
                     } else if blobs_required {

beacon_node/beacon_chain/src/pending_payload_cache/mod.rs

@@ -185,7 +185,6 @@ impl<T: BeaconChainTypes> PendingPayloadCache<T> {
         let pending_components =
             self.get_pending_components(beacon_block_root, bid, |pending_components| {
                 pending_components.insert_executed_payload_envelope(executed_envelope);
-                Ok(())
             })?;
 
         let num_expected_columns = self
@@ -423,14 +422,14 @@ impl<T: BeaconChainTypes> PendingPayloadCache<T> {
         update_fn: F,
     ) -> Result<MappedRwLockReadGuard<'_, PendingComponents<T::EthSpec>>, AvailabilityCheckError>
     where
-        F: FnOnce(&mut PendingComponents<T::EthSpec>) -> Result<(), AvailabilityCheckError>,
+        F: FnOnce(&mut PendingComponents<T::EthSpec>),
     {
         let mut write_lock = self.availability_cache.write();
 
         {
             let pending_components = write_lock
                 .get_or_insert_mut(block_root, || PendingComponents::new(block_root, bid));
-            update_fn(pending_components)?
+            update_fn(pending_components)
         }
 
         RwLockReadGuard::try_map(RwLockWriteGuard::downgrade(write_lock), |cache| {
@@ -623,15 +622,7 @@ mod data_availability_checker_tests {
     const RNG_SEED: u64 = 0xDEADBEEF;
 
     fn gloas_spec<E: EthSpec>() -> Arc<ChainSpec> {
-        let mut spec = E::default_spec();
+        Arc::new(ForkName::Gloas.make_genesis_spec(E::default_spec()))
-        spec.altair_fork_epoch = Some(Epoch::new(0));
-        spec.bellatrix_fork_epoch = Some(Epoch::new(0));
-        spec.capella_fork_epoch = Some(Epoch::new(0));
-        spec.deneb_fork_epoch = Some(Epoch::new(0));
-        spec.electra_fork_epoch = Some(Epoch::new(0));
-        spec.fulu_fork_epoch = Some(Epoch::new(0));
-        spec.gloas_fork_epoch = Some(Epoch::new(0));
-        Arc::new(spec)
     }
 
     fn get_store_with_spec<E: EthSpec>(

beacon_node/beacon_chain/src/pending_payload_cache/pending_column.rs

@@ -59,6 +59,8 @@ impl<E: EthSpec> PendingColumn<E> {
             kzg_proofs.push(*proof);
         }
 
+        // TODO(gloas): this hard-codes the Gloas sidecar variant. Pass the fork in once
+        // post-Gloas variants are introduced (or move construction to a fork-aware helper).
         Some(Arc::new(DataColumnSidecar::Gloas(DataColumnSidecarGloas {
             index,
             // TODO(gloas): this should not error, but we need to catch it

beacon_node/beacon_chain/src/pending_payload_cache/pending_components.rs

@@ -78,7 +78,7 @@ impl<E: EthSpec> PendingComponents<E> {
     pub(crate) fn merge_data_columns(
         &mut self,
         kzg_verified_data_columns: &[KzgVerifiedCustodyDataColumn<E>],
-    ) -> Result<(), AvailabilityCheckError> {
+    ) {
         let num_blobs_expected = self.num_blobs_expected();
         for data_column in kzg_verified_data_columns {
             let data_column = data_column.as_data_column();
@@ -98,8 +98,6 @@ impl<E: EthSpec> PendingComponents<E> {
                 col.insert(cell_idx, cell, proof);
             }
         }
-
-        Ok(())
     }
 
     // TODO(gloas): merge partial columns
@@ -115,7 +113,7 @@ impl<E: EthSpec> PendingComponents<E> {
     pub fn num_completed_columns(&self) -> usize {
         self.verified_data_columns
             .values()
-            .filter_map(|col| col.is_complete(self.num_blobs_expected()).then_some(()))
+            .filter(|col| col.is_complete(self.num_blobs_expected()))
             .count()
     }
 

beacon_node/network/src/network_beacon_processor/gossip_methods.rs

@@ -728,14 +728,14 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
                         ..
                     } => {
                         debug!(
-                            action = "requesting block",
+                            action = "ignoring",
                             %unknown_block_root,
                             "Unknown block root for column"
                         );
-                        self.send_sync_message(SyncMessage::UnknownBlockHashFromAttestation(
+                        // TODO(gloas): wire this into proper lookup sync. Sending
-                            peer_id,
+                        // `UnknownBlockHashFromAttestation` here is a Fulu-shaped fallback that
-                            unknown_block_root,
+                        // mixes column processing with the attestation lookup path and is not
-                        ));
+                        // the right primitive for Gloas column lookups.
                         self.propagate_validation_result(
                             message_id,
                             peer_id,

beacon_node/network/src/sync/manager.rs

@@ -906,6 +906,10 @@ impl<T: BeaconChainTypes> SyncManager<T> {
                         );
                     }
                     DataColumnSidecar::Gloas(_) => {
+                        // TODO(gloas): proper lookup sync for Gloas. Routing into
+                        // `handle_unknown_block_root` here mixes column processing with the
+                        // single-block-lookup path; the Gloas column-arrives-before-block
+                        // case wants its own queue/wakeup.
                         debug!(%block_root, "Received unknown block data column message");
                         self.handle_unknown_block_root(peer_id, block_root);
                     }