Skip to content

Optimize GetBlobsV2 Verification #7553

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 4 commits into from
Closed

Optimize GetBlobsV2 Verification #7553

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ac33874
Add a data column verification mode where block proposer + signature …
eserilev Jun 3, 2025
27af137
Rename enum to ConstructedInternally and ignore some more checks
eserilev Jun 5, 2025
74bbfb5
skip observe slashable if constructed internally
eserilev Jun 8, 2025
0e86335
Merge branch 'unstable' into optimize-blob-v2-verification
eserilev Jun 8, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 10 additions & 2 deletions beacon_node/beacon_chain/src/beacon_chain.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,9 @@ use crate::data_availability_checker::{
Availability, AvailabilityCheckError, AvailableBlock, AvailableBlockData,
DataAvailabilityChecker, DataColumnReconstructionResult,
};
use crate::data_column_verification::{GossipDataColumnError, GossipVerifiedDataColumn};
use crate::data_column_verification::{
ConstructedInternally, GossipDataColumnError, GossipVerifiedDataColumn,
};
use crate::early_attester_cache::EarlyAttesterCache;
use crate::errors::{BeaconChainError as Error, BlockProductionError};
use crate::eth1_chain::{Eth1Chain, Eth1ChainBackend};
Expand Down Expand Up @@ -2201,7 +2203,13 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
) -> Result<GossipVerifiedDataColumn<T>, GossipDataColumnError> {
metrics::inc_counter(&metrics::DATA_COLUMN_SIDECAR_PROCESSING_REQUESTS);
let _timer = metrics::start_timer(&metrics::DATA_COLUMN_SIDECAR_GOSSIP_VERIFICATION_TIMES);
GossipVerifiedDataColumn::new(data_column_sidecar, subnet_id, self).inspect(|_| {
GossipVerifiedDataColumn::new(
data_column_sidecar,
subnet_id,
ConstructedInternally::No,
self,
)
.inspect(|_| {
metrics::inc_counter(&metrics::DATA_COLUMN_SIDECAR_PROCESSING_SUCCESSES);
})
}
Expand Down
69 changes: 48 additions & 21 deletions beacon_node/beacon_chain/src/data_column_verification.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,13 @@ use types::{
RuntimeVariableList, SignedBeaconBlockHeader, Slot,
};

/// Used to avoid double-checking internally constructed data column sidecars.
#[derive(Copy, Clone)]
pub enum ConstructedInternally {
Yes,
No,
}

/// An error occurred while validating a gossip data column.
#[derive(Debug)]
pub enum GossipDataColumnError {
Expand Down Expand Up @@ -199,19 +206,25 @@ impl<T: BeaconChainTypes, O: ObservationStrategy> GossipVerifiedDataColumn<T, O>
pub fn new(
column_sidecar: Arc<DataColumnSidecar<T::EthSpec>>,
subnet_id: u64,
constructed_internally: ConstructedInternally,
chain: &BeaconChain<T>,
) -> Result<Self, GossipDataColumnError> {
let header = column_sidecar.signed_block_header.clone();
// We only process slashing info if the gossip verification failed
// since we do not process the data column any further in that case.
validate_data_column_sidecar_for_gossip::<T, O>(column_sidecar, subnet_id, chain).map_err(
|e| {
process_block_slash_info::<_, GossipDataColumnError>(
chain,
BlockSlashInfo::from_early_error_data_column(header, e),
)
},
validate_data_column_sidecar_for_gossip::<T, O>(
column_sidecar,
subnet_id,
constructed_internally,
chain,
)
.map_err(|e| match constructed_internally {
ConstructedInternally::No => process_block_slash_info::<_, GossipDataColumnError>(
chain,
BlockSlashInfo::from_early_error_data_column(header, e),
),
ConstructedInternally::Yes => e,
})
}

/// Create a `GossipVerifiedDataColumn` from `DataColumnSidecar` for testing ONLY.
Expand Down Expand Up @@ -458,6 +471,7 @@ where
pub fn validate_data_column_sidecar_for_gossip<T: BeaconChainTypes, O: ObservationStrategy>(
data_column: Arc<DataColumnSidecar<T::EthSpec>>,
subnet: u64,
constructed_internally: ConstructedInternally,
chain: &BeaconChain<T>,
) -> Result<GossipVerifiedDataColumn<T, O>, GossipDataColumnError> {
let column_slot = data_column.slot();
Expand All @@ -483,23 +497,35 @@ pub fn validate_data_column_sidecar_for_gossip<T: BeaconChainTypes, O: Observati
return Err(GossipDataColumnError::PriorKnownUnpublished);
}

verify_column_inclusion_proof(&data_column)?;
let parent_block = verify_parent_block_and_finalized_descendant(data_column.clone(), chain)?;
verify_slot_higher_than_parent(&parent_block, column_slot)?;
verify_proposer_and_signature(&data_column, &parent_block, chain)?;
match constructed_internally {
ConstructedInternally::No => {
verify_column_inclusion_proof(&data_column)?;
let parent_block =
verify_parent_block_and_finalized_descendant(data_column.clone(), chain)?;
verify_slot_higher_than_parent(&parent_block, column_slot)?;
verify_proposer_and_signature(&data_column, &parent_block, chain)?;
}
ConstructedInternally::Yes => (),
};

let kzg = &chain.kzg;
let kzg_verified_data_column = verify_kzg_for_data_column(data_column.clone(), kzg)
Copy link
Member

@jimmygchen jimmygchen Jun 6, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I might be overthinking, but I'm also tempted to include observe_slashable (without changing the ordering), because it acquires a write lock, and usually data columns arrives in burst (128 simultaneously), and the write lock can only be acquired by one thread at a time, so this write could potentially slow things down even though it's a very fast operation.

    chain
        .observed_slashable
        .write()

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It might be worth writing some bench tests for gossip verification (in a later PR) - given that 1/ gossip verification time is critical to both the network and the performance of the node; 2/ the volume of verification is significantly higher now with data columns.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Created issue for benchmark #7576

.map_err(GossipDataColumnError::InvalidKzgProof)?;

chain
.observed_slashable
.write()
.observe_slashable(
column_slot,
data_column.block_proposer_index(),
data_column.block_root(),
)
.map_err(|e| GossipDataColumnError::BeaconChainError(Box::new(e.into())))?;
match constructed_internally {
ConstructedInternally::No => {
chain
.observed_slashable
.write()
.observe_slashable(
column_slot,
data_column.block_proposer_index(),
data_column.block_root(),
)
.map_err(|e| GossipDataColumnError::BeaconChainError(Box::new(e.into())))?;
}
ConstructedInternally::Yes => (),
}

if O::observe() {
observe_gossip_data_column(&data_column, chain)?;
Expand Down Expand Up @@ -801,7 +827,7 @@ pub fn observe_gossip_data_column<T: BeaconChainTypes>(
#[cfg(test)]
mod test {
use crate::data_column_verification::{
validate_data_column_sidecar_for_gossip, GossipDataColumnError,
validate_data_column_sidecar_for_gossip, ConstructedInternally, GossipDataColumnError,
};
use crate::observed_data_sidecars::Observe;
use crate::test_utils::BeaconChainHarness;
Expand Down Expand Up @@ -845,6 +871,7 @@ mod test {
let result = validate_data_column_sidecar_for_gossip::<_, Observe>(
column_sidecar.into(),
index,
ConstructedInternally::Yes,
&harness.chain,
);
assert!(matches!(
Expand Down
11 changes: 9 additions & 2 deletions beacon_node/beacon_chain/src/fetch_blobs/fetch_blobs_beacon_adapter.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,5 +1,7 @@
use crate::blob_verification::{GossipBlobError, GossipVerifiedBlob};
use crate::data_column_verification::{GossipDataColumnError, GossipVerifiedDataColumn};
use crate::data_column_verification::{
ConstructedInternally, GossipDataColumnError, GossipVerifiedDataColumn,
};
use crate::fetch_blobs::{EngineGetBlobsOutput, FetchEngineBlobError};
use crate::observed_data_sidecars::DoNotObserve;
use crate::{AvailabilityProcessingStatus, BeaconChain, BeaconChainTypes};
Expand Down Expand Up @@ -80,7 +82,12 @@ impl<T: BeaconChainTypes> FetchBlobsBeaconAdapter<T> {
data_column: Arc<DataColumnSidecar<T::EthSpec>>,
) -> Result<GossipVerifiedDataColumn<T, DoNotObserve>, GossipDataColumnError> {
let index = data_column.index;
GossipVerifiedDataColumn::<T, DoNotObserve>::new(data_column, index, &self.chain)
GossipVerifiedDataColumn::<T, DoNotObserve>::new(
data_column,
index,
ConstructedInternally::Yes,
&self.chain,
)
}

pub(crate) async fn process_engine_blobs(
Expand Down
12 changes: 9 additions & 3 deletions beacon_node/http_api/src/publish_blocks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,9 @@ use std::future::Future;

use beacon_chain::blob_verification::{GossipBlobError, GossipVerifiedBlob};
use beacon_chain::block_verification_types::{AsBlock, RpcBlock};
use beacon_chain::data_column_verification::{GossipDataColumnError, GossipVerifiedDataColumn};
use beacon_chain::data_column_verification::{
ConstructedInternally, GossipDataColumnError, GossipVerifiedDataColumn,
};
use beacon_chain::validator_monitor::{get_block_delay_ms, timestamp_now};
use beacon_chain::{
build_blob_data_column_sidecars, AvailabilityProcessingStatus, BeaconChain, BeaconChainError,
Expand Down Expand Up @@ -407,8 +409,12 @@ fn build_gossip_verified_data_columns<T: BeaconChainTypes>(
.map(|data_column_sidecar| {
let column_index = data_column_sidecar.index;
let subnet = DataColumnSubnetId::from_column_index(column_index, &chain.spec);
let gossip_verified_column =
GossipVerifiedDataColumn::new(data_column_sidecar, subnet.into(), chain);
let gossip_verified_column = GossipVerifiedDataColumn::new(
data_column_sidecar,
subnet.into(),
ConstructedInternally::Yes,
chain,
);

match gossip_verified_column {
Ok(blob) => Ok(Some(blob)),
Expand Down
5 changes: 4 additions & 1 deletion beacon_node/network/src/network_beacon_processor/tests.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,9 @@ use crate::{
sync::{manager::BlockProcessType, SyncMessage},
};
use beacon_chain::block_verification_types::RpcBlock;
use beacon_chain::data_column_verification::validate_data_column_sidecar_for_gossip;
use beacon_chain::data_column_verification::{
validate_data_column_sidecar_for_gossip, ConstructedInternally,
};
use beacon_chain::kzg_utils::blobs_to_data_column_sidecars;
use beacon_chain::observed_data_sidecars::DoNotObserve;
use beacon_chain::test_utils::{
Expand Down Expand Up @@ -826,6 +828,7 @@ async fn accept_processed_gossip_data_columns_without_import() {
validate_data_column_sidecar_for_gossip::<_, DoNotObserve>(
data_column,
subnet_id,
ConstructedInternally::No,
&rig.chain,
)
.expect("should be valid data column")
Expand Down