From 6b3b6ccf5172dbbb43019789b840ea953ab25b22 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 27 Apr 2026 14:52:52 +0200 Subject: [PATCH] Clean up and self review --- .../src/block_production/gloas.rs | 3 +- beacon_node/beacon_chain/src/kzg_utils.rs | 32 +++------------- beacon_node/beacon_chain/src/test_utils.rs | 38 ++++++++++++------- .../src/beacon/execution_payload_envelope.rs | 25 +++++++++--- 4 files changed, 51 insertions(+), 47 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_production/gloas.rs b/beacon_node/beacon_chain/src/block_production/gloas.rs index c087ff029a..25d2c7ef86 100644 --- a/beacon_node/beacon_chain/src/block_production/gloas.rs +++ b/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 BeaconChain { 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), }, diff --git a/beacon_node/beacon_chain/src/kzg_utils.rs b/beacon_node/beacon_chain/src/kzg_utils.rs index ee8b0437bb..b05a896777 100644 --- a/beacon_node/beacon_chain/src/kzg_utils.rs +++ b/beacon_node/beacon_chain/src/kzg_utils.rs @@ -296,10 +296,9 @@ pub fn blobs_to_data_column_sidecars( } } -/// 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( blobs: &[&Blob], - cell_proofs: Vec, beacon_block_root: Hash256, slot: Slot, kzg: &Kzg, @@ -309,35 +308,16 @@ pub fn blobs_to_data_column_sidecars_gloas( 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::>(); - - 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::, 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::(num_of_blobs); + let (blobs, _proofs) = create_test_gloas_blobs::(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::( &blob_refs, - proofs.to_vec(), beacon_block_root, slot, kzg, @@ -873,7 +852,6 @@ mod test { let blob_refs: Vec<&types::Blob> = vec![]; let column_sidecars = blobs_to_data_column_sidecars_gloas::( &blob_refs, - vec![], Hash256::random(), Slot::new(0), kzg, diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 274f41d1cb..610897e8d9 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -3789,21 +3789,21 @@ pub fn generate_data_column_sidecars_from_block( block: &SignedBeaconBlock, spec: &ChainSpec, ) -> DataColumnSidecarList { - 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::>::from_ssz_bytes( TEST_DATA_COLUMN_SIDECARS_SSZ, @@ -3826,7 +3826,7 @@ pub fn generate_data_column_sidecars_from_block( .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( ) .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::>::from_ssz_bytes( diff --git a/beacon_node/http_api/src/beacon/execution_payload_envelope.rs b/beacon_node/http_api/src/beacon/execution_payload_envelope.rs index f33562698b..e06a7e9201 100644 --- a/beacon_node/http_api/src/beacon/execution_payload_envelope.rs +++ b/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( })?; // 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( beacon_block_root: types::Hash256, slot: types::Slot, blobs: &types::BlobsList, - kzg_proofs: types::KzgProofs, ) -> Result>, 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( 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::>(); debug!(