From 5886a48d969207c67d36f5e085148d2d6cccb401 Mon Sep 17 00:00:00 2001 From: Jimmy Chen Date: Wed, 15 Oct 2025 12:52:35 +1100 Subject: [PATCH] Add `max_blobs_per_block` check to data column gossip validation (#8198) Addresses this spec change https://github.com/ethereum/consensus-specs/pull/4650 Add `max_blobs_per_block` to gossip data column check so we reject large columns before processing. (we currently do this check during processing) Co-Authored-By: Jimmy Chen --- .../src/data_column_verification.rs | 115 ++++++++++++++++-- beacon_node/beacon_chain/src/test_utils.rs | 2 +- .../gossip_methods.rs | 1 + 3 files changed, 107 insertions(+), 11 deletions(-) diff --git a/beacon_node/beacon_chain/src/data_column_verification.rs b/beacon_node/beacon_chain/src/data_column_verification.rs index fad7771f01..01e79c49aa 100644 --- a/beacon_node/beacon_chain/src/data_column_verification.rs +++ b/beacon_node/beacon_chain/src/data_column_verification.rs @@ -161,6 +161,15 @@ pub enum GossipDataColumnError { /// /// The column sidecar is invalid and the peer is faulty InconsistentProofsLength { cells_len: usize, proofs_len: usize }, + /// The number of KZG commitments exceeds the maximum number of blobs allowed for the fork. The + /// sidecar is invalid. + /// + /// ## Peer scoring + /// The column sidecar is invalid and the peer is faulty + MaxBlobsPerBlockExceeded { + max_blobs_per_block: usize, + commitments_len: usize, + }, } impl From for GossipDataColumnError { @@ -220,7 +229,7 @@ impl GossipVerifiedDataColumn column_sidecar: Arc>, chain: &BeaconChain, ) -> Result { - verify_data_column_sidecar(&column_sidecar)?; + verify_data_column_sidecar(&column_sidecar, &chain.spec)?; // Check if the data column is already in the DA checker cache. This happens when data columns // are made available through the `engine_getBlobs` method. If it exists in the cache, we know @@ -475,7 +484,7 @@ pub fn validate_data_column_sidecar_for_gossip, ) -> Result, GossipDataColumnError> { let column_slot = data_column.slot(); - verify_data_column_sidecar(&data_column)?; + verify_data_column_sidecar(&data_column, &chain.spec)?; verify_index_matches_subnet(&data_column, subnet, &chain.spec)?; verify_sidecar_not_from_future_slot(chain, column_slot)?; verify_slot_greater_than_latest_finalized_slot(chain, column_slot)?; @@ -529,6 +538,7 @@ pub fn validate_data_column_sidecar_for_gossip( data_column: &DataColumnSidecar, + spec: &ChainSpec, ) -> Result<(), GossipDataColumnError> { if data_column.index >= E::number_of_columns() as u64 { return Err(GossipDataColumnError::InvalidColumnIndex(data_column.index)); @@ -540,6 +550,14 @@ fn verify_data_column_sidecar( 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; + + if commitments_len > max_blobs_per_block { + return Err(GossipDataColumnError::MaxBlobsPerBlockExceeded { + max_blobs_per_block, + commitments_len, + }); + } if cells_len != commitments_len { return Err(GossipDataColumnError::InconsistentCommitmentsLength { @@ -782,16 +800,22 @@ pub fn observe_gossip_data_column( #[cfg(test)] mod test { use crate::data_column_verification::{ - GossipDataColumnError, validate_data_column_sidecar_for_gossip, + GossipDataColumnError, GossipVerifiedDataColumn, validate_data_column_sidecar_for_gossip, }; use crate::observed_data_sidecars::Observe; - use crate::test_utils::BeaconChainHarness; + use crate::test_utils::{ + BeaconChainHarness, EphemeralHarnessType, generate_data_column_sidecars_from_block, + }; + use eth2::types::BlobsBundle; + use execution_layer::test_utils::generate_blobs; + use std::sync::Arc; use types::{DataColumnSidecar, DataColumnSubnetId, EthSpec, ForkName, MainnetEthSpec}; type E = MainnetEthSpec; #[tokio::test] - async fn empty_data_column_sidecars_fails_validation() { + async fn test_validate_data_column_sidecar_for_gossip() { + // Setting up harness is slow, we initialise once and use it for all gossip validation tests. let spec = ForkName::Fulu.make_genesis_spec(E::default_spec()); let harness = BeaconChainHarness::builder(E::default()) .spec(spec.into()) @@ -801,6 +825,44 @@ mod test { .build(); harness.advance_slot(); + let verify_fn = |column_sidecar: DataColumnSidecar| { + let col_index = column_sidecar.index; + validate_data_column_sidecar_for_gossip::<_, Observe>( + column_sidecar.into(), + DataColumnSubnetId::from_column_index(col_index, &harness.spec), + &harness.chain, + ) + }; + empty_data_column_sidecars_fails_validation(&harness, &verify_fn).await; + data_column_sidecar_commitments_exceed_max_blobs_per_block(&harness, &verify_fn).await; + } + + #[tokio::test] + async fn test_new_for_block_publishing() { + // Setting up harness is slow, we initialise once and use it for all gossip validation tests. + let spec = ForkName::Fulu.make_genesis_spec(E::default_spec()); + let harness = BeaconChainHarness::builder(E::default()) + .spec(spec.into()) + .deterministic_keypairs(64) + .fresh_ephemeral_store() + .mock_execution_layer() + .build(); + harness.advance_slot(); + + let verify_fn = |column_sidecar: DataColumnSidecar| { + GossipVerifiedDataColumn::<_>::new_for_block_publishing( + column_sidecar.into(), + &harness.chain, + ) + }; + empty_data_column_sidecars_fails_validation(&harness, &verify_fn).await; + data_column_sidecar_commitments_exceed_max_blobs_per_block(&harness, &verify_fn).await; + } + + async fn empty_data_column_sidecars_fails_validation( + harness: &BeaconChainHarness>, + verify_fn: &impl Fn(DataColumnSidecar) -> Result, + ) { let slot = harness.get_current_slot(); let state = harness.get_current_state(); let ((block, _blobs_opt), _state) = harness @@ -823,14 +885,47 @@ mod test { .unwrap(), }; - let result = validate_data_column_sidecar_for_gossip::<_, Observe>( - column_sidecar.into(), - DataColumnSubnetId::from_column_index(index, &harness.spec), - &harness.chain, - ); + let result = verify_fn(column_sidecar); assert!(matches!( result.err(), Some(GossipDataColumnError::UnexpectedDataColumn) )); } + + async fn data_column_sidecar_commitments_exceed_max_blobs_per_block( + harness: &BeaconChainHarness>, + verify_fn: &impl Fn(DataColumnSidecar) -> Result, + ) { + let slot = harness.get_current_slot(); + let epoch = slot.epoch(E::slots_per_epoch()); + let state = harness.get_current_state(); + let max_blobs_per_block = harness.spec.max_blobs_per_block(epoch) as usize; + let fork = harness.spec.fork_name_at_epoch(epoch); + + // Generate data column sidecar with blob count exceeding max_blobs_per_block. + let blob_count = max_blobs_per_block + 1; + let BlobsBundle:: { + commitments: preloaded_commitments_single, + proofs: _, + blobs: _, + } = generate_blobs(1, fork).unwrap().0; + + let ((block, _blobs_opt), _state) = harness + .make_block_with_modifier(state, slot, |block| { + *block.body_mut().blob_kzg_commitments_mut().unwrap() = + vec![preloaded_commitments_single[0]; blob_count].into(); + }) + .await; + + let column_sidecar = generate_data_column_sidecars_from_block(&block, &harness.spec) + .into_iter() + .next() + .unwrap(); + + let result = verify_fn(Arc::try_unwrap(column_sidecar).unwrap()); + assert!(matches!( + result.err(), + Some(GossipDataColumnError::MaxBlobsPerBlockExceeded { .. }) + )); + } } diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index c2230ba057..1d57550156 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -3380,7 +3380,7 @@ pub fn generate_rand_block_and_data_columns( } /// Generate data column sidecars from pre-computed cells and proofs. -fn generate_data_column_sidecars_from_block( +pub fn generate_data_column_sidecars_from_block( block: &SignedBeaconBlock, spec: &ChainSpec, ) -> DataColumnSidecarList { diff --git a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs index 5fc94c2958..fa6b5fd243 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -708,6 +708,7 @@ impl NetworkBeaconProcessor { | GossipDataColumnError::InvalidKzgProof { .. } | GossipDataColumnError::UnexpectedDataColumn | GossipDataColumnError::InvalidColumnIndex(_) + | GossipDataColumnError::MaxBlobsPerBlockExceeded { .. } | GossipDataColumnError::InconsistentCommitmentsLength { .. } | GossipDataColumnError::InconsistentProofsLength { .. } | GossipDataColumnError::NotFinalizedDescendant { .. } => {