Clean up and self review

beacon_node/beacon_chain/src/block_production/gloas.rs

@@ -34,6 +34,7 @@ use types::{
     SignedVoluntaryExit, Slot, SyncAggregate, Withdrawal, Withdrawals,
 };
 
+use crate::pending_payload_envelopes::PendingEnvelopeData;
 use crate::{
     BeaconChain, BeaconChainError, BeaconChainTypes, BlockProductionError,
     ProduceBlockVerification, block_production::BlockProductionState,
@@ -609,7 +610,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             let blobs_and_proofs = payload_data.blobs_and_proofs;
             self.pending_payload_envelopes.write().insert(
                 envelope_slot,
-                crate::pending_payload_envelopes::PendingEnvelopeData {
+                PendingEnvelopeData {
                     envelope: signed_envelope.message,
                     blobs_and_proofs: Some(blobs_and_proofs),
                 },

beacon_node/beacon_chain/src/kzg_utils.rs

@@ -296,10 +296,9 @@ pub fn blobs_to_data_column_sidecars<E: EthSpec>(
     }
 }
 
-/// Build Gloas data column sidecars from blobs and cell proofs
+/// Build Gloas data column sidecars from blobs, computing cells and proofs locally.
 pub fn blobs_to_data_column_sidecars_gloas<E: EthSpec>(
     blobs: &[&Blob<E>],
-    cell_proofs: Vec<KzgProof>,
     beacon_block_root: Hash256,
     slot: Slot,
     kzg: &Kzg,
@@ -309,35 +308,16 @@ pub fn blobs_to_data_column_sidecars_gloas<E: EthSpec>(
         return Ok(vec![]);
     }
 
-    if cell_proofs.len() != blobs.len() * E::number_of_columns() {
-        return Err(DataColumnSidecarError::InvalidCellProofLength {
-            expected: blobs.len() * E::number_of_columns(),
-            actual: cell_proofs.len(),
-        });
-    }
-
-    let proof_chunks = cell_proofs
-        .chunks_exact(E::number_of_columns())
-        .collect::<Vec<_>>();
-
-    let zipped: Vec<_> = blobs.iter().zip(proof_chunks).collect();
-    let blob_cells_and_proofs_vec = zipped
+    let blob_cells_and_proofs_vec = blobs
         .into_par_iter()
-        .map(|(blob, proofs)| {
+        .map(|blob| {
             let blob = blob.as_ref().try_into().map_err(|e| {
                 KzgError::InconsistentArrayLength(format!(
                     "blob should have a guaranteed size due to FixedVector: {e:?}"
                 ))
             })?;
 
-            kzg.compute_cells(blob).and_then(|cells| {
-                let proofs = proofs.try_into().map_err(|e| {
-                    KzgError::InconsistentArrayLength(format!(
-                        "proof chunks should have exactly `number_of_columns` proofs: {e:?}"
-                    ))
-                })?;
-                Ok((cells, proofs))
-            })
+            kzg.compute_cells_and_proofs(blob)
         })
         .collect::<Result<Vec<_>, KzgError>>()?;
 
@@ -841,14 +821,13 @@ mod test {
     #[track_caller]
     fn test_build_data_columns_gloas(kzg: &Kzg, spec: &ChainSpec) {
         let num_of_blobs = 2;
-        let (blobs, proofs) = create_test_gloas_blobs::<E>(num_of_blobs);
+        let (blobs, _proofs) = create_test_gloas_blobs::<E>(num_of_blobs);
         let beacon_block_root = Hash256::random();
         let slot = Slot::new(0);
 
         let blob_refs: Vec<_> = blobs.iter().collect();
         let column_sidecars = blobs_to_data_column_sidecars_gloas::<E>(
             &blob_refs,
-            proofs.to_vec(),
             beacon_block_root,
             slot,
             kzg,
@@ -873,7 +852,6 @@ mod test {
         let blob_refs: Vec<&types::Blob<E>> = vec![];
         let column_sidecars = blobs_to_data_column_sidecars_gloas::<E>(
             &blob_refs,
-            vec![],
             Hash256::random(),
             Slot::new(0),
             kzg,

beacon_node/beacon_chain/src/test_utils.rs

@@ -3789,21 +3789,21 @@ pub fn generate_data_column_sidecars_from_block<E: EthSpec>(
     block: &SignedBeaconBlock<E>,
     spec: &ChainSpec,
 ) -> DataColumnSidecarList<E> {
-    let kzg_commitments = block.message().body().blob_kzg_commitments().unwrap();
-    if kzg_commitments.is_empty() {
-        return vec![];
-    }
-
-    let kzg_commitments_inclusion_proof = block
-        .message()
-        .body()
-        .kzg_commitments_merkle_proof()
-        .unwrap();
-    let signed_block_header = block.signed_block_header();
-
     // Load the precomputed column sidecar to avoid computing them for every block in the tests.
     // Then repeat the cells and proofs for every blob
     if block.fork_name_unchecked().gloas_enabled() {
+        let kzg_commitments = &block
+            .message()
+            .body()
+            .signed_execution_payload_bid()
+            .expect("Gloas block should have a payload bid")
+            .message
+            .blob_kzg_commitments;
+        if kzg_commitments.is_empty() {
+            return vec![];
+        }
+        let num_blobs = kzg_commitments.len();
+        let signed_block_header = block.signed_block_header();
         let template_data_columns =
             RuntimeVariableList::<DataColumnSidecarGloas<E>>::from_ssz_bytes(
                 TEST_DATA_COLUMN_SIDECARS_SSZ,
@@ -3826,7 +3826,7 @@ pub fn generate_data_column_sidecars_from_block<E: EthSpec>(
             .collect::<(Vec<_>, Vec<_>)>();
 
         let blob_cells_and_proofs_vec =
-            vec![(cells.try_into().unwrap(), proofs.try_into().unwrap()); kzg_commitments.len()];
+            vec![(cells.try_into().unwrap(), proofs.try_into().unwrap()); num_blobs];
 
         build_data_column_sidecars_gloas(
             signed_block_header.message.tree_hash_root(),
@@ -3836,6 +3836,18 @@ pub fn generate_data_column_sidecars_from_block<E: EthSpec>(
         )
         .unwrap()
     } else {
+        let kzg_commitments = block.message().body().blob_kzg_commitments().unwrap();
+        if kzg_commitments.is_empty() {
+            return vec![];
+        }
+
+        let kzg_commitments_inclusion_proof = block
+            .message()
+            .body()
+            .kzg_commitments_merkle_proof()
+            .unwrap();
+        let signed_block_header = block.signed_block_header();
+
         // load the precomputed column sidecar to avoid computing them for every block in the tests.
         let template_data_columns =
             RuntimeVariableList::<DataColumnSidecarFulu<E>>::from_ssz_bytes(

beacon_node/http_api/src/beacon/execution_payload_envelope.rs

@@ -6,7 +6,7 @@ use crate::version::{
     ResponseIncludesVersion, add_consensus_version_header, add_ssz_content_type_header,
     execution_optimistic_finalized_beacon_response,
 };
-use beacon_chain::data_column_verification::GossipVerifiedDataColumn;
+use beacon_chain::data_column_verification::{GossipDataColumnError, GossipVerifiedDataColumn};
 use beacon_chain::{BeaconChain, BeaconChainTypes};
 use bytes::Bytes;
 use eth2::types as api_types;
@@ -126,11 +126,11 @@ pub async fn publish_execution_payload_envelope<T: BeaconChainTypes>(
     })?;
 
     // Build and publish data column sidecars from the blobs.
-    if let Some((blobs, kzg_proofs)) = blobs_and_proofs
+    if let Some((blobs, _kzg_proofs)) = blobs_and_proofs
         && !blobs.is_empty()
     {
         let gossip_verified_columns =
-            build_gloas_data_columns(&chain, beacon_block_root, slot, &blobs, kzg_proofs)?;
+            build_gloas_data_columns(&chain, beacon_block_root, slot, &blobs)?;
 
         if !gossip_verified_columns.is_empty() {
             publish_column_sidecars(network_tx, &gossip_verified_columns, &chain).map_err(
@@ -169,12 +169,10 @@ fn build_gloas_data_columns<T: BeaconChainTypes>(
     beacon_block_root: types::Hash256,
     slot: types::Slot,
     blobs: &types::BlobsList<T::EthSpec>,
-    kzg_proofs: types::KzgProofs<T::EthSpec>,
 ) -> Result<Vec<GossipVerifiedDataColumn<T>>, Rejection> {
     let blob_refs: Vec<_> = blobs.iter().collect();
     let data_column_sidecars = beacon_chain::kzg_utils::blobs_to_data_column_sidecars_gloas(
         &blob_refs,
-        kzg_proofs.to_vec(),
         beacon_block_root,
         slot,
         &chain.kzg,
@@ -191,7 +189,22 @@ fn build_gloas_data_columns<T: BeaconChainTypes>(
 
     let gossip_verified_columns = data_column_sidecars
         .into_iter()
-        .filter_map(|col| GossipVerifiedDataColumn::new_for_block_publishing(col, chain).ok())
+        .filter_map(|col| {
+            let index = *col.index();
+            match GossipVerifiedDataColumn::new_for_block_publishing(col, chain) {
+                Ok(verified) => Some(verified),
+                Err(GossipDataColumnError::PriorKnownUnpublished) => None,
+                Err(e) => {
+                    warn!(
+                        %slot,
+                        column_index = index,
+                        error = ?e,
+                        "Locally-built data column failed gossip verification"
+                    );
+                    None
+                }
+            }
+        })
         .collect::<Vec<_>>();
 
     debug!(