From 1dd0f7bcbb9e476028ec9146a149e121ad28ff9e Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 4 Feb 2026 14:37:05 +1100 Subject: [PATCH] Remove `kzg_commitments` from `DataColumnSidecarGloas` (#8739) Co-Authored-By: Eitan Seri- Levi Co-Authored-By: Michael Sproul Co-Authored-By: Jimmy Chen Co-Authored-By: Michael Sproul --- beacon_node/beacon_chain/src/builder.rs | 1 - .../src/data_availability_checker.rs | 2 +- .../src/data_column_verification.rs | 10 ++++- beacon_node/beacon_chain/src/kzg_utils.rs | 45 +++++++++++++++---- .../src/observed_data_sidecars.rs | 1 - beacon_node/beacon_chain/src/test_utils.rs | 1 - .../beacon_chain/tests/block_verification.rs | 2 +- .../lighthouse_network/tests/rpc_tests.rs | 2 - common/eth2/src/types.rs | 6 ++- .../types/src/data/data_column_sidecar.rs | 10 ++--- testing/ef_tests/src/handler.rs | 3 -- 11 files changed, 54 insertions(+), 29 deletions(-) diff --git a/beacon_node/beacon_chain/src/builder.rs b/beacon_node/beacon_chain/src/builder.rs index 5dbe662b9b..9bfb7788d2 100644 --- a/beacon_node/beacon_chain/src/builder.rs +++ b/beacon_node/beacon_chain/src/builder.rs @@ -1217,7 +1217,6 @@ fn build_data_columns_from_blobs( if block.fork_name_unchecked().gloas_enabled() { build_data_column_sidecars_gloas( - kzg_commitments, block.message().tree_hash_root(), block.slot(), blob_cells_and_proofs_vec, diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 7f4848f006..666ba7cc41 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -1120,7 +1120,7 @@ mod test { let invalid_sidecar = DataColumnSidecar::Fulu(DataColumnSidecarFulu { column: DataColumn::::empty(), index: *d.index(), - kzg_commitments: d.kzg_commitments().clone(), + kzg_commitments: d.kzg_commitments().unwrap().clone(), kzg_proofs: d.kzg_proofs().clone(), signed_block_header: d.signed_block_header().unwrap().clone(), kzg_commitments_inclusion_proof: d diff --git a/beacon_node/beacon_chain/src/data_column_verification.rs b/beacon_node/beacon_chain/src/data_column_verification.rs index 03bdde6114..cf3385ec5b 100644 --- a/beacon_node/beacon_chain/src/data_column_verification.rs +++ b/beacon_node/beacon_chain/src/data_column_verification.rs @@ -573,12 +573,18 @@ fn verify_data_column_sidecar( *data_column.index(), )); } - if data_column.kzg_commitments().is_empty() { + + // TODO(gloas): implement Gloas verification that takes kzg_commitments from block as parameter + let commitments_len = match data_column { + DataColumnSidecar::Fulu(dc) => dc.kzg_commitments.len(), + DataColumnSidecar::Gloas(_) => return Err(GossipDataColumnError::InvalidVariant), + }; + + if commitments_len == 0 { return Err(GossipDataColumnError::UnexpectedDataColumn); } let cells_len = data_column.column().len(); - let commitments_len = data_column.kzg_commitments().len(); let proofs_len = data_column.kzg_proofs().len(); let max_blobs_per_block = spec.max_blobs_per_block(data_column.epoch()) as usize; diff --git a/beacon_node/beacon_chain/src/kzg_utils.rs b/beacon_node/beacon_chain/src/kzg_utils.rs index bef96836f1..33b3260361 100644 --- a/beacon_node/beacon_chain/src/kzg_utils.rs +++ b/beacon_node/beacon_chain/src/kzg_utils.rs @@ -75,7 +75,21 @@ where proofs.push(Bytes48::from(proof)); } - for &commitment in data_column.kzg_commitments() { + // In Gloas, commitments come from the block's ExecutionPayloadBid, not the sidecar. + // This function requires Fulu sidecars with embedded commitments. + let kzg_commitments = match data_column.as_ref() { + DataColumnSidecar::Fulu(dc) => &dc.kzg_commitments, + DataColumnSidecar::Gloas(_) => { + return Err(( + Some(col_index), + KzgError::InconsistentArrayLength( + "Gloas data columns require commitments from block".to_string(), + ), + )); + } + }; + + for &commitment in kzg_commitments.iter() { commitments.push(Bytes48::from(commitment)); } @@ -209,7 +223,6 @@ pub fn blobs_to_data_column_sidecars( if block.fork_name_unchecked().gloas_enabled() { build_data_column_sidecars_gloas( - kzg_commitments.clone(), signed_block_header.message.tree_hash_root(), block.slot(), blob_cells_and_proofs_vec, @@ -321,7 +334,6 @@ pub(crate) fn build_data_column_sidecars_fulu( } pub(crate) fn build_data_column_sidecars_gloas( - kzg_commitments: KzgCommitments, beacon_block_root: Hash256, slot: Slot, blob_cells_and_proofs_vec: Vec, @@ -374,7 +386,6 @@ pub(crate) fn build_data_column_sidecars_gloas( index: index as u64, column: DataColumn::::try_from(col) .map_err(|e| format!("MaxBlobCommitmentsPerBlock exceeded: {e:?}"))?, - kzg_commitments: kzg_commitments.clone(), kzg_proofs: VariableList::try_from(proofs) .map_err(|e| format!("MaxBlobCommitmentsPerBlock exceeded: {e:?}"))?, beacon_block_root, @@ -412,7 +423,12 @@ pub fn reconstruct_blobs( let blob_indices: Vec = match blob_indices_opt { Some(indices) => indices.into_iter().map(|i| i as usize).collect(), None => { - let num_of_blobs = first_data_column.kzg_commitments().len(); + // TODO(gloas): support blob reconstruction for Gloas + // https://github.com/sigp/lighthouse/issues/7413 + let num_of_blobs = first_data_column + .kzg_commitments() + .map_err(|_| "Gloas blob reconstruction not yet supported".to_string())? + .len(); (0..num_of_blobs).collect() } }; @@ -497,7 +513,16 @@ pub fn reconstruct_data_columns( "data_columns should have at least one element".to_string(), ))?; - let num_of_blobs = first_data_column.kzg_commitments().len(); + // TODO(gloas): support data column reconstruction for Gloas + // https://github.com/sigp/lighthouse/issues/7413 + let num_of_blobs = first_data_column + .kzg_commitments() + .map_err(|_| { + KzgError::InconsistentArrayLength( + "Gloas data column reconstruction not yet supported".to_string(), + ) + })? + .len(); let blob_cells_and_proofs_vec = (0..num_of_blobs) .into_par_iter() @@ -530,7 +555,6 @@ pub fn reconstruct_data_columns( .map_err(KzgError::ReconstructFailed) } DataColumnSidecar::Gloas(first_column) => build_data_column_sidecars_gloas( - first_column.kzg_commitments.clone(), first_column.beacon_block_root, first_column.slot, blob_cells_and_proofs_vec, @@ -629,11 +653,14 @@ mod test { for (idx, col_sidecar) in column_sidecars.iter().enumerate() { assert_eq!(*col_sidecar.index(), idx as u64); - assert_eq!(col_sidecar.kzg_commitments().len(), num_of_blobs); + assert_eq!(col_sidecar.kzg_commitments().unwrap().len(), num_of_blobs); assert_eq!(col_sidecar.column().len(), num_of_blobs); assert_eq!(col_sidecar.kzg_proofs().len(), num_of_blobs); - assert_eq!(col_sidecar.kzg_commitments().clone(), block_kzg_commitments); + assert_eq!( + col_sidecar.kzg_commitments().unwrap().clone(), + block_kzg_commitments + ); assert_eq!( col_sidecar .kzg_commitments_inclusion_proof() diff --git a/beacon_node/beacon_chain/src/observed_data_sidecars.rs b/beacon_node/beacon_chain/src/observed_data_sidecars.rs index b53eb3955c..894b8d3444 100644 --- a/beacon_node/beacon_chain/src/observed_data_sidecars.rs +++ b/beacon_node/beacon_chain/src/observed_data_sidecars.rs @@ -312,7 +312,6 @@ mod tests { Arc::new(DataColumnSidecar::Gloas(DataColumnSidecarGloas { index, column: vec![].try_into().unwrap(), - kzg_commitments: vec![].try_into().unwrap(), kzg_proofs: vec![].try_into().unwrap(), slot: slot.into(), beacon_block_root, diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index f4e4c37234..dcf8ee4f8e 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -3517,7 +3517,6 @@ pub fn generate_data_column_sidecars_from_block( vec![(cells.try_into().unwrap(), proofs.try_into().unwrap()); kzg_commitments.len()]; build_data_column_sidecars_gloas( - kzg_commitments.clone(), signed_block_header.message.tree_hash_root(), signed_block_header.message.slot, blob_cells_and_proofs_vec, diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 440c0be3e4..417d2811dd 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -285,7 +285,7 @@ fn update_data_column_signed_header( let new_column_sidecar = Arc::new(DataColumnSidecar::Fulu(DataColumnSidecarFulu { index: *old_column_sidecar.index(), column: old_column_sidecar.column().clone(), - kzg_commitments: old_column_sidecar.kzg_commitments().clone(), + kzg_commitments: old_column_sidecar.kzg_commitments().unwrap().clone(), kzg_proofs: old_column_sidecar.kzg_proofs().clone(), signed_block_header: signed_block.signed_block_header(), kzg_commitments_inclusion_proof: signed_block diff --git a/beacon_node/lighthouse_network/tests/rpc_tests.rs b/beacon_node/lighthouse_network/tests/rpc_tests.rs index 553cfa6f0d..53939687d3 100644 --- a/beacon_node/lighthouse_network/tests/rpc_tests.rs +++ b/beacon_node/lighthouse_network/tests/rpc_tests.rs @@ -1017,7 +1017,6 @@ fn test_tcp_columns_by_root_chunked_rpc_for_fork(fork_name: ForkName) { column: vec![vec![0; E::bytes_per_cell()].try_into().unwrap()] .try_into() .unwrap(), - kzg_commitments: vec![KzgCommitment::empty_for_testing()].try_into().unwrap(), kzg_proofs: vec![KzgProof::empty()].try_into().unwrap(), })) } else { @@ -1188,7 +1187,6 @@ fn test_tcp_columns_by_range_chunked_rpc_for_fork(fork_name: ForkName) { column: vec![vec![0; E::bytes_per_cell()].try_into().unwrap()] .try_into() .unwrap(), - kzg_commitments: vec![KzgCommitment::empty_for_testing()].try_into().unwrap(), kzg_proofs: vec![KzgProof::empty()].try_into().unwrap(), })) } else { diff --git a/common/eth2/src/types.rs b/common/eth2/src/types.rs index c1572ca354..4acfe3a640 100644 --- a/common/eth2/src/types.rs +++ b/common/eth2/src/types.rs @@ -1016,7 +1016,11 @@ impl SseDataColumnSidecar { pub fn from_data_column_sidecar( data_column_sidecar: &DataColumnSidecar, ) -> SseDataColumnSidecar { - let kzg_commitments = data_column_sidecar.kzg_commitments().to_vec(); + // TODO(gloas): fetch kzg_commitments from block for Gloas SSE events + let kzg_commitments: Vec = match data_column_sidecar { + DataColumnSidecar::Fulu(dc) => dc.kzg_commitments.to_vec(), + DataColumnSidecar::Gloas(_) => vec![], + }; let versioned_hashes = kzg_commitments .iter() .map(|c| c.calculate_versioned_hash()) diff --git a/consensus/types/src/data/data_column_sidecar.rs b/consensus/types/src/data/data_column_sidecar.rs index d98470297a..c8a49e346a 100644 --- a/consensus/types/src/data/data_column_sidecar.rs +++ b/consensus/types/src/data/data_column_sidecar.rs @@ -81,7 +81,9 @@ pub struct DataColumnSidecar { pub index: ColumnIndex, #[serde(with = "ssz_types::serde_utils::list_of_hex_fixed_vec")] pub column: DataColumn, - /// All the KZG commitments and proofs associated with the block, used for verifying sample cells. + /// All the KZG commitments associated with the block, used for verifying sample cells. + /// In Gloas, commitments come from `block.body.signed_execution_payload_bid.message.blob_kzg_commitments`. + #[superstruct(only(Fulu))] pub kzg_commitments: KzgCommitments, pub kzg_proofs: VariableList, #[superstruct(only(Fulu))] @@ -210,7 +212,6 @@ impl DataColumnSidecarGloas { Self { index: 0, column: VariableList::new(vec![Cell::::default()]).unwrap(), - kzg_commitments: VariableList::new(vec![KzgCommitment::empty_for_testing()]).unwrap(), kzg_proofs: VariableList::new(vec![KzgProof::empty()]).unwrap(), slot: Slot::new(0), beacon_block_root: Hash256::ZERO, @@ -223,11 +224,6 @@ impl DataColumnSidecarGloas { Self { index: 0, column: VariableList::new(vec![Cell::::default(); max_blobs_per_block]).unwrap(), - kzg_commitments: VariableList::new(vec![ - KzgCommitment::empty_for_testing(); - max_blobs_per_block - ]) - .unwrap(), kzg_proofs: VariableList::new(vec![KzgProof::empty(); max_blobs_per_block]).unwrap(), slot: Slot::new(0), beacon_block_root: Hash256::ZERO, diff --git a/testing/ef_tests/src/handler.rs b/testing/ef_tests/src/handler.rs index 117661143b..b0fc90b169 100644 --- a/testing/ef_tests/src/handler.rs +++ b/testing/ef_tests/src/handler.rs @@ -401,10 +401,7 @@ where } fn is_enabled_for_fork(&self, fork_name: ForkName) -> bool { - // TODO(gloas): DataColumnSidecar tests are disabled until we update the DataColumnSidecar - // type. self.supported_forks.contains(&fork_name) - && !(fork_name == ForkName::Gloas && T::name() == "DataColumnSidecar") } }