Skip to content

Fix for #7305: Verify blobs and data columns during backfill #7353

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.

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

Open
wants to merge 5 commits into
base: unstable
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 1 addition & 1 deletion beacon_node/beacon_chain/src/block_verification.rs
Original file line number Diff line number Diff line change
Expand Up @@ -636,7 +636,7 @@ pub fn signature_verify_chain_segment<T: BeaconChainTypes>(
let (roots, blocks): (Vec<_>, Vec<_>) = chain_segment.into_iter().unzip();
let maybe_available_blocks = chain
.data_availability_checker
.verify_kzg_for_rpc_blocks(blocks)?;
.verify_kzg_for_rpc_blocks(&blocks)?;
// zip it back up
let mut signature_verified_blocks = roots
.into_iter()
Expand Down
90 changes: 88 additions & 2 deletions beacon_node/beacon_chain/src/block_verification_types.rs
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,8 @@ use std::fmt::{Debug, Formatter};
use std::sync::Arc;
use types::blob_sidecar::BlobIdentifier;
use types::{
BeaconBlockRef, BeaconState, BlindedPayload, BlobSidecarList, ChainSpec, Epoch, EthSpec,
Hash256, RuntimeVariableList, SignedBeaconBlock, SignedBeaconBlockHeader, Slot,
BeaconBlockRef, BeaconState, BlindedPayload, BlobSidecarList, ChainSpec, ColumnIndex, Epoch,
EthSpec, Hash256, RuntimeVariableList, SignedBeaconBlock, SignedBeaconBlockHeader, Slot,
};

/// A block that has been received over RPC. It has 2 internal variants:
Expand Down Expand Up @@ -80,6 +80,70 @@ impl<E: EthSpec> RpcBlock<E> {
RpcBlockInner::BlockAndCustodyColumns(_, data_columns) => Some(data_columns),
}
}

/// Returns indices of blobs or data columns that have conflicting signature with block's
/// signature.
pub fn validate_non_matching_signed_headers(&self) -> Result<(), AvailabilityCheckError> {
match &self.block {
RpcBlockInner::Block(_) => Ok(()),
RpcBlockInner::BlockAndBlobs(block, blobs) => {
let indices: Vec<_> = blobs
.iter()
.filter(|blob| &blob.signed_block_header.signature != block.signature())
.map(|blob| blob.index)
.collect();
if !indices.is_empty() {
return Err(AvailabilityCheckError::InvalidBlobsSignature(indices));
}
Ok(())
}
RpcBlockInner::BlockAndCustodyColumns(block, data_columns) => {
let indices: Vec<_> = data_columns
.iter()
.filter(|column| {
&column.as_data_column().signed_block_header.signature != block.signature()
})
.map(|column| column.index())
.collect();
if !indices.is_empty() {
return Err(AvailabilityCheckError::InvalidDataColumnsSignature(indices));
}
Ok(())
}
}
}

/// Returns indices of blobs that have conflicting signature with block's signature.
pub fn non_matching_blobs_signed_headers(&self) -> Option<Vec<ColumnIndex>> {
match &self.block {
RpcBlockInner::Block(_) => None,
RpcBlockInner::BlockAndBlobs(block, blobs) => Some(
blobs
.iter()
.filter(|blob| &blob.signed_block_header.signature != block.signature())
.map(|blob| blob.index)
.collect(),
),
RpcBlockInner::BlockAndCustodyColumns(..) => None,
}
}

/// Returns indices of custody columns that have conflicting signature with block's signature.
pub fn non_matching_custody_columns_signed_headers(&self) -> Option<Vec<ColumnIndex>> {
match &self.block {
RpcBlockInner::Block(_) => None,
RpcBlockInner::BlockAndBlobs(..) => None,
RpcBlockInner::BlockAndCustodyColumns(block, data_columns) => Some(
data_columns
.iter()
.filter(|column| {
&column.as_data_column().signed_block_header.signature != block.signature()
})
.map(|column| column.index())
.collect(),
),
}
}
}

/// Note: This variant is intentionally private because we want to safely construct the
Expand Down Expand Up @@ -186,6 +250,28 @@ impl<E: EthSpec> RpcBlock<E> {
})
}

/// Only used for testing
pub fn __new_for_testing(
block_root: Hash256,
block: Arc<SignedBeaconBlock<E>>,
blobs: Option<BlobSidecarList<E>>,
custody_columns: Option<CustodyDataColumnList<E>>,
custody_columns_count: usize,
) -> Self {
let inner = if let Some(blobs) = blobs {
RpcBlockInner::BlockAndBlobs(block, blobs)
} else if let Some(columns) = custody_columns {
RpcBlockInner::BlockAndCustodyColumns(block, columns)
} else {
RpcBlockInner::Block(block)
};
Self {
block_root,
block: inner,
custody_columns_count,
}
}

#[allow(clippy::type_complexity)]
pub fn deconstruct(
self,
Expand Down
30 changes: 11 additions & 19 deletions beacon_node/beacon_chain/src/data_availability_checker.rs
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,9 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
&self,
block: RpcBlock<T::EthSpec>,
) -> Result<MaybeAvailableBlock<T::EthSpec>, AvailabilityCheckError> {
// Verify that blobs or data columns signatures match
block.validate_non_matching_signed_headers()?;

let custody_columns_count = block.custody_columns_count();
let (block_root, block, blobs, data_columns) = block.deconstruct();
if self.blobs_required_for_block(&block) {
Expand Down Expand Up @@ -391,8 +394,14 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {
/// check if there are any missing blobs.
pub fn verify_kzg_for_rpc_blocks(
&self,
blocks: Vec<RpcBlock<T::EthSpec>>,
blocks: &[RpcBlock<T::EthSpec>],
) -> Result<Vec<MaybeAvailableBlock<T::EthSpec>>, AvailabilityCheckError> {
// Verify that blobs or data columns signatures match
blocks
.iter()
.map(|block| block.validate_non_matching_signed_headers())
.collect::<Result<Vec<_>, AvailabilityCheckError>>()?;

let mut results = Vec::with_capacity(blocks.len());
let all_blobs = blocks
.iter()
Expand Down Expand Up @@ -428,7 +437,7 @@ impl<T: BeaconChainTypes> DataAvailabilityChecker<T> {

for block in blocks {
let custody_columns_count = block.custody_columns_count();
let (block_root, block, blobs, data_columns) = block.deconstruct();
let (block_root, block, blobs, data_columns) = block.clone().deconstruct();

let maybe_available_block = if self.blobs_required_for_block(&block) {
if let Some(blobs) = blobs {
Expand Down Expand Up @@ -783,23 +792,6 @@ impl<E: EthSpec> AvailableBlock<E> {
} = self;
(block_root, block, blob_data)
}

/// Only used for testing
Copy link
Author

Choose a reason for hiding this comment

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

Removed as it is no longer called in testing (moved to RpcBlock)

pub fn __clone_without_recv(&self) -> Result<Self, String> {
Ok(Self {
block_root: self.block_root,
block: self.block.clone(),
blob_data: match &self.blob_data {
AvailableBlockData::NoData => AvailableBlockData::NoData,
AvailableBlockData::Blobs(blobs) => AvailableBlockData::Blobs(blobs.clone()),
AvailableBlockData::DataColumns(data_columns) => {
AvailableBlockData::DataColumns(data_columns.clone())
}
},
blobs_available_timestamp: self.blobs_available_timestamp,
spec: self.spec.clone(),
})
}
}

#[derive(Debug)]
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,10 @@ pub enum Error {
BlockReplayError(state_processing::BlockReplayError),
RebuildingStateCaches(BeaconStateError),
SlotClockError,
/// One or more signatures in a BlobSidecar of an RpcBlock are invalid
InvalidBlobsSignature(Vec<u64>),
/// One or more signatures in a DataColumnSidecar of an RpcBlock are invalid
InvalidDataColumnsSignature(Vec<ColumnIndex>),
}

#[derive(PartialEq, Eq)]
Expand All @@ -44,7 +48,9 @@ impl Error {
| Error::ParentStateMissing(_)
| Error::BlockReplayError(_)
| Error::RebuildingStateCaches(_)
| Error::SlotClockError => ErrorCategory::Internal,
| Error::SlotClockError
| Error::InvalidBlobsSignature(_)
| Error::InvalidDataColumnsSignature(_) => ErrorCategory::Internal,
Error::InvalidBlobs { .. }
| Error::InvalidColumn { .. }
| Error::ReconstructColumnsError { .. }
Expand Down
Loading