From 09c15f03fccdaff8e1fd80519a5a07e035ccd0d7 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 17 Nov 2025 16:20:21 -0300 Subject: [PATCH 01/24] Make RPCBlock a variant with an available and maybeavailable variants --- .../src/block_verification_types.rs | 225 +++++++++++++----- .../src/data_availability_checker.rs | 2 +- .../src/data_availability_checker/error.rs | 4 +- beacon_node/beacon_chain/src/test_utils.rs | 14 +- beacon_node/http_api/src/publish_blocks.rs | 9 +- .../src/sync/block_sidecar_coupling.rs | 7 +- .../network/src/sync/network_context.rs | 3 +- beacon_node/network/src/sync/tests/lookups.rs | 3 +- beacon_node/network/src/sync/tests/range.rs | 6 +- testing/ef_tests/src/cases/fork_choice.rs | 34 +-- 10 files changed, 215 insertions(+), 92 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_verification_types.rs b/beacon_node/beacon_chain/src/block_verification_types.rs index 5978e97c4d9..0fa026051c5 100644 --- a/beacon_node/beacon_chain/src/block_verification_types.rs +++ b/beacon_node/beacon_chain/src/block_verification_types.rs @@ -9,8 +9,8 @@ use std::fmt::{Debug, Formatter}; use std::sync::Arc; use types::blob_sidecar::BlobIdentifier; use types::{ - BeaconBlockRef, BeaconState, BlindedPayload, BlobSidecarList, Epoch, EthSpec, Hash256, - SignedBeaconBlock, SignedBeaconBlockHeader, Slot, + BeaconBlockRef, BeaconState, BlindedPayload, BlobSidecarList, Epoch, EthSpec, ForkName, + Hash256, RuntimeVariableList, SignedBeaconBlock, SignedBeaconBlockHeader, Slot, }; /// A block that has been received over RPC. It has 2 internal variants: @@ -28,40 +28,68 @@ use types::{ /// over rpc do not contain the proposer signature for dos resistance. #[derive(Clone, Educe)] #[educe(Hash(bound(E: EthSpec)))] -pub struct RpcBlock { +pub enum RpcBlock { + Available(AvailableRpcBlock), + MaybeAvailable(MaybeAvailableRpcBlock), +} + +#[derive(Clone, Educe)] +#[educe(Hash(bound(E: EthSpec)))] +pub struct MaybeAvailableRpcBlock { + block_root: Hash256, + block: RpcBlockInner, +} + +#[derive(Clone, Educe)] +#[educe(Hash(bound(E: EthSpec)))] +pub struct AvailableRpcBlock { block_root: Hash256, block: RpcBlockInner, } impl Debug for RpcBlock { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { - write!(f, "RpcBlock({:?})", self.block_root) + write!(f, "RpcBlock({:?})", self.block_root()) } } impl RpcBlock { pub fn block_root(&self) -> Hash256 { - self.block_root + match self { + RpcBlock::Available(available_rpc_block) => available_rpc_block.block_root, + RpcBlock::MaybeAvailable(maybe_available_rpc_block) => { + maybe_available_rpc_block.block_root + } + } + } + + pub fn rpc_block_inner(&self) -> &RpcBlockInner { + match self { + RpcBlock::Available(available_rpc_block) => &available_rpc_block.block, + RpcBlock::MaybeAvailable(maybe_available_rpc_block) => &maybe_available_rpc_block.block, + } } pub fn as_block(&self) -> &SignedBeaconBlock { - match &self.block { - RpcBlockInner::Block(block) => block, - RpcBlockInner::BlockAndBlobs(block, _) => block, - RpcBlockInner::BlockAndCustodyColumns(block, _) => block, + match self.rpc_block_inner() { + RpcBlockInner::Block(signed_beacon_block) => signed_beacon_block, + RpcBlockInner::BlockAndBlobs(signed_beacon_block, _) => signed_beacon_block, + RpcBlockInner::BlockAndCustodyColumns(signed_beacon_block, _) => signed_beacon_block, } } pub fn block_cloned(&self) -> Arc> { - match &self.block { - RpcBlockInner::Block(block) => block.clone(), - RpcBlockInner::BlockAndBlobs(block, _) => block.clone(), - RpcBlockInner::BlockAndCustodyColumns(block, _) => block.clone(), + match self.rpc_block_inner() { + RpcBlockInner::Block(signed_beacon_block) => signed_beacon_block.clone(), + RpcBlockInner::BlockAndBlobs(signed_beacon_block, _) => signed_beacon_block.clone(), + RpcBlockInner::BlockAndCustodyColumns(signed_beacon_block, _) => { + signed_beacon_block.clone() + } } } pub fn blobs(&self) -> Option<&BlobSidecarList> { - match &self.block { + match self.rpc_block_inner() { RpcBlockInner::Block(_) => None, RpcBlockInner::BlockAndBlobs(_, blobs) => Some(blobs), RpcBlockInner::BlockAndCustodyColumns(_, _) => None, @@ -69,7 +97,7 @@ impl RpcBlock { } pub fn custody_columns(&self) -> Option<&CustodyDataColumnList> { - match &self.block { + match self.rpc_block_inner() { RpcBlockInner::Block(_) => None, RpcBlockInner::BlockAndBlobs(_, _) => None, RpcBlockInner::BlockAndCustodyColumns(_, data_columns) => Some(data_columns), @@ -93,34 +121,27 @@ enum RpcBlockInner { BlockAndCustodyColumns(Arc>, CustodyDataColumnList), } -impl RpcBlock { - /// Constructs a `Block` variant. - pub fn new_without_blobs( - block_root: Option, - block: Arc>, - ) -> Self { - let block_root = block_root.unwrap_or_else(|| get_block_root(&block)); - - Self { - block_root, - block: RpcBlockInner::Block(block), - } - } - +impl RpcBlockInner { /// Constructs a new `BlockAndBlobs` variant after making consistency - /// checks between the provided blocks and blobs. This struct makes no + /// checks between the provided block and blobs. This struct makes no /// guarantees about whether blobs should be present, only that they are /// consistent with the block. An empty list passed in for `blobs` is /// viewed the same as `None` passed in. - pub fn new( - block_root: Option, + fn new_block_and_blobs( block: Arc>, blobs: Option>, ) -> Result { - let block_root = block_root.unwrap_or_else(|| get_block_root(&block)); + match block.fork_name_unchecked() { + ForkName::Base + | ForkName::Altair + | ForkName::Bellatrix + | ForkName::Capella + | ForkName::Fulu + | ForkName::Gloas => return Err(AvailabilityCheckError::InvalidFork), + ForkName::Deneb | ForkName::Electra => (), + } // Treat empty blob lists as if they are missing. let blobs = blobs.filter(|b| !b.is_empty()); - if let (Some(blobs), Ok(block_commitments)) = ( blobs.as_ref(), block.message().body().blob_kzg_commitments(), @@ -138,37 +159,117 @@ impl RpcBlock { } } } - let inner = match blobs { - Some(blobs) => RpcBlockInner::BlockAndBlobs(block, blobs), - None => RpcBlockInner::Block(block), + + let blobs = if let Some(blobs) = blobs { + blobs + } else { + RuntimeVariableList::new(vec![], E::max_blob_commitments_per_block())? + }; + + Ok(Self::BlockAndBlobs(block, blobs)) + } + + /// Constructs a new `BlockAndCustodyColumns` variant after making consistency + /// checks between the provided block and columns. This struct makes no + /// guarantees about whether columns should be present, only that they are + /// consistent with the block. An empty list passed in for `custody_columns` is + /// viewed the same as `None` passed in. + pub fn new_block_and_columns( + block: Arc>, + custody_columns: Option>>, + ) -> Result { + match block.fork_name_unchecked() { + ForkName::Base + | ForkName::Altair + | ForkName::Bellatrix + | ForkName::Capella + | ForkName::Deneb + | ForkName::Electra + | ForkName::Gloas => return Err(AvailabilityCheckError::InvalidFork), + ForkName::Fulu => (), + } + + let columns = if let Some(custody_columns) = custody_columns { + if block.num_expected_blobs() > 0 && custody_columns.is_empty() { + // The number of required custody columns is out of scope here. + return Err(AvailabilityCheckError::MissingCustodyColumns); + } + // Treat empty data column lists as if they are missing. + if !custody_columns.is_empty() { + VariableList::new(custody_columns)? + } else { + VariableList::new(vec![])? + } + } else { + VariableList::new(vec![])? + }; + + Ok(RpcBlockInner::BlockAndCustodyColumns(block, columns)) + } + + /// Constructs a new `Block` variant. + pub fn new(block: Arc>) -> Result { + match block.fork_name_unchecked() { + ForkName::Deneb | ForkName::Electra | ForkName::Fulu | ForkName::Gloas => { + return Err(AvailabilityCheckError::InvalidFork); + } + ForkName::Base | ForkName::Altair | ForkName::Bellatrix | ForkName::Capella => (), + } + Ok(Self::Block(block)) + } +} + +impl RpcBlock { + /// Constructs an `RpcBlock::MaybeAvailable`. Consistency checks for this variant must be made + /// before indicating that the block is fully available. An empty list or `None` passed in for + /// `blobs` or `columns` could mean that node may have not received blobs for this column yet. + pub fn new_maybe_available( + block_root: Option, + block: Arc>, + blobs: Option>, + columns: Option>>, + ) -> Result { + let block_root = block_root.unwrap_or_else(|| get_block_root(&block)); + + let rpc_block_inner = if blobs.is_some() { + RpcBlockInner::new_block_and_blobs(block, blobs)? + } else if columns.is_some() { + RpcBlockInner::new_block_and_columns(block, columns)? + } else { + RpcBlockInner::new(block)? }; - Ok(Self { + + Ok(RpcBlock::MaybeAvailable(MaybeAvailableRpcBlock { block_root, - block: inner, - }) + block: rpc_block_inner, + })) } - pub fn new_with_custody_columns( + /// Constructs an `RpcBlock::Available` variant. All consistency checks + /// for the `RpcBlock` should be made before calling this function. Successfully constructing + /// an `Available` variant indicates that the block is fully available from the nodes perspective. + /// An empty list or `None` passed in for `blobs` or `columns` means that there + /// are no blobs for the given block. + pub fn new_available( block_root: Option, block: Arc>, - custody_columns: Vec>, + blobs: Option>, + columns: Option>>, ) -> Result { let block_root = block_root.unwrap_or_else(|| get_block_root(&block)); - if block.num_expected_blobs() > 0 && custody_columns.is_empty() { - // The number of required custody columns is out of scope here. - return Err(AvailabilityCheckError::MissingCustodyColumns); - } - // Treat empty data column lists as if they are missing. - let inner = if !custody_columns.is_empty() { - RpcBlockInner::BlockAndCustodyColumns(block, VariableList::new(custody_columns)?) + let rpc_block_inner = if blobs.is_some() { + RpcBlockInner::new_block_and_blobs(block, blobs)? + } else if columns.is_some() { + RpcBlockInner::new_block_and_columns(block, columns)? } else { - RpcBlockInner::Block(block) + RpcBlockInner::new(block)? }; - Ok(Self { + + Ok(RpcBlock::Available(AvailableRpcBlock { block_root, - block: inner, - }) + block: rpc_block_inner, + })) } #[allow(clippy::type_complexity)] @@ -181,22 +282,24 @@ impl RpcBlock { Option>, ) { let block_root = self.block_root(); - match self.block { - RpcBlockInner::Block(block) => (block_root, block, None, None), - RpcBlockInner::BlockAndBlobs(block, blobs) => (block_root, block, Some(blobs), None), + match self.rpc_block_inner() { + RpcBlockInner::Block(block) => (block_root, block.clone(), None, None), + RpcBlockInner::BlockAndBlobs(block, blobs) => { + (block_root, block.clone(), Some(blobs.clone()), None) + } RpcBlockInner::BlockAndCustodyColumns(block, data_columns) => { - (block_root, block, None, Some(data_columns)) + (block_root, block.clone(), None, Some(data_columns.clone())) } } } pub fn n_blobs(&self) -> usize { - match &self.block { + match self.rpc_block_inner() { RpcBlockInner::Block(_) | RpcBlockInner::BlockAndCustodyColumns(_, _) => 0, RpcBlockInner::BlockAndBlobs(_, blobs) => blobs.len(), } } pub fn n_data_columns(&self) -> usize { - match &self.block { + match self.rpc_block_inner() { RpcBlockInner::Block(_) | RpcBlockInner::BlockAndBlobs(_, _) => 0, RpcBlockInner::BlockAndCustodyColumns(_, data_columns) => data_columns.len(), } @@ -500,14 +603,14 @@ impl AsBlock for RpcBlock { self.as_block().message() } fn as_block(&self) -> &SignedBeaconBlock { - match &self.block { + match &self.rpc_block_inner() { RpcBlockInner::Block(block) => block, RpcBlockInner::BlockAndBlobs(block, _) => block, RpcBlockInner::BlockAndCustodyColumns(block, _) => block, } } fn block_cloned(&self) -> Arc> { - match &self.block { + match &self.rpc_block_inner() { RpcBlockInner::Block(block) => block.clone(), RpcBlockInner::BlockAndBlobs(block, _) => block.clone(), RpcBlockInner::BlockAndCustodyColumns(block, _) => block.clone(), diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 644c4716985..442e4ff6c9f 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -1076,7 +1076,7 @@ mod test { .collect::>() }; - RpcBlock::new_with_custody_columns(None, Arc::new(block), custody_columns) + RpcBlock::new_available(None, Arc::new(block), None, Some(custody_columns)) .expect("should create RPC block with custody columns") }) .collect::>(); diff --git a/beacon_node/beacon_chain/src/data_availability_checker/error.rs b/beacon_node/beacon_chain/src/data_availability_checker/error.rs index c9efb7a4149..1d9b8820d36 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker/error.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker/error.rs @@ -22,6 +22,7 @@ pub enum Error { BlockReplayError(state_processing::BlockReplayError), RebuildingStateCaches(BeaconStateError), SlotClockError, + InvalidFork, } #[derive(PartialEq, Eq)] @@ -44,7 +45,8 @@ impl Error { | Error::ParentStateMissing(_) | Error::BlockReplayError(_) | Error::RebuildingStateCaches(_) - | Error::SlotClockError => ErrorCategory::Internal, + | Error::SlotClockError + | Error::InvalidFork => ErrorCategory::Internal, Error::InvalidBlobs { .. } | Error::InvalidColumn { .. } | Error::ReconstructColumnsError { .. } diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 9601618e927..587718d0a6d 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -2429,7 +2429,8 @@ where .blob_kzg_commitments() .is_ok_and(|c| !c.is_empty()); if !has_blobs { - return RpcBlock::new_without_blobs(Some(block_root), block); + // TODO(can refactor so we dont call new_avail a bunch) + return RpcBlock::new_available(Some(block_root), block, None, None).unwrap(); } // Blobs are stored as data columns from Fulu (PeerDAS) @@ -2439,10 +2440,10 @@ where .into_iter() .map(CustodyDataColumn::from_asserted_custody) .collect::>(); - RpcBlock::new_with_custody_columns(Some(block_root), block, custody_columns).unwrap() + RpcBlock::new_available(Some(block_root), block, None, Some(custody_columns)).unwrap() } else { let blobs = self.chain.get_blobs(&block_root).unwrap().blobs(); - RpcBlock::new(Some(block_root), block, blobs).unwrap() + RpcBlock::new_available(Some(block_root), block, blobs, None).unwrap() } } @@ -2466,9 +2467,10 @@ where .filter(|d| sampling_columns.contains(&d.index)) .map(CustodyDataColumn::from_asserted_custody) .collect::>(); - RpcBlock::new_with_custody_columns(Some(block_root), block, columns)? + // TODO(can combine these and clean it up) + RpcBlock::new_available(Some(block_root), block, None, Some(columns))? } else { - RpcBlock::new_without_blobs(Some(block_root), block) + RpcBlock::new_available(Some(block_root), block, None, None)? } } else { let blobs = blob_items @@ -2477,7 +2479,7 @@ where }) .transpose() .unwrap(); - RpcBlock::new(Some(block_root), block, blobs)? + RpcBlock::new_available(Some(block_root), block, blobs, None)? }) } diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index bfe41c8706c..fab86d70793 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -312,9 +312,16 @@ pub async fn publish_block>( slot = %block.slot(), "Block previously seen" ); + let Ok(rpc_block) = + RpcBlock::new_maybe_available(Some(block_root), block.clone(), None, None) + else { + return Err(warp_utils::reject::custom_bad_request( + "Unable to construct rpc block".to_string(), + )); + }; let import_result = Box::pin(chain.process_block( block_root, - RpcBlock::new_without_blobs(Some(block_root), block.clone()), + rpc_block, NotifyExecutionLayer::Yes, BlockImportSource::HttpApi, publish_fn, diff --git a/beacon_node/network/src/sync/block_sidecar_coupling.rs b/beacon_node/network/src/sync/block_sidecar_coupling.rs index 01929cbf906..df7d5bbdca4 100644 --- a/beacon_node/network/src/sync/block_sidecar_coupling.rs +++ b/beacon_node/network/src/sync/block_sidecar_coupling.rs @@ -315,7 +315,7 @@ impl RangeBlockComponentsRequest { CouplingError::BlobPeerFailure("Blobs returned exceeds max length".to_string()) })?; responses.push( - RpcBlock::new(None, block, Some(blobs)) + RpcBlock::new_available(None, block, Some(blobs), None) .map_err(|e| CouplingError::BlobPeerFailure(format!("{e:?}")))?, ) } @@ -414,11 +414,12 @@ impl RangeBlockComponentsRequest { ); } - RpcBlock::new_with_custody_columns(Some(block_root), block, custody_columns) + RpcBlock::new_available(Some(block_root), block, None, Some(custody_columns)) .map_err(|e| CouplingError::InternalError(format!("{:?}", e)))? } else { // Block has no data, expects zero columns - RpcBlock::new_without_blobs(Some(block_root), block) + RpcBlock::new_available(Some(block_root), block, None, None) + .map_err(|e| CouplingError::InternalError(format!("{:?}", e)))? }); } diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 2e0c56db23f..1e932588dfe 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -1606,7 +1606,8 @@ impl SyncNetworkContext { .beacon_processor_if_enabled() .ok_or(SendErrorProcessor::ProcessorNotAvailable)?; - let block = RpcBlock::new_without_blobs(Some(block_root), block); + let block = RpcBlock::new_maybe_available(Some(block_root), block, None, None) + .map_err(|_| SendErrorProcessor::SendError)?; debug!(block = ?block_root, id, "Sending block for processing"); // Lookup sync event safety: If `beacon_processor.send_rpc_beacon_block` returns Ok() sync diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index 63bcd176f52..9a10aadcc22 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -2286,12 +2286,13 @@ mod deneb_only { let max_len = self.rig.spec.max_blobs_per_block(block.epoch()) as usize; // Now this block is the one we expect requests from self.block = block.clone(); - let block = RpcBlock::new( + let block = RpcBlock::new_maybe_available( Some(block.canonical_root()), block, self.unknown_parent_blobs .take() .map(|vec| RuntimeVariableList::new(vec, max_len).unwrap()), + None, ) .unwrap(); self.rig.parent_block_processed( diff --git a/beacon_node/network/src/sync/tests/range.rs b/beacon_node/network/src/sync/tests/range.rs index cb728a90c1b..7a30c25f5d1 100644 --- a/beacon_node/network/src/sync/tests/range.rs +++ b/beacon_node/network/src/sync/tests/range.rs @@ -446,13 +446,13 @@ fn build_rpc_block( ) -> RpcBlock { match data_sidecars { Some(DataSidecars::Blobs(blobs)) => { - RpcBlock::new(None, block, Some(blobs.clone())).unwrap() + RpcBlock::new_available(None, block, Some(blobs.clone()), None).unwrap() } Some(DataSidecars::DataColumns(columns)) => { - RpcBlock::new_with_custody_columns(None, block, columns.clone()).unwrap() + RpcBlock::new_available(None, block, None, Some(columns.clone())).unwrap() } // Block has no data, expects zero columns - None => RpcBlock::new_without_blobs(None, block), + None => RpcBlock::new_available(None, block, None, None).unwrap(), } } diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 8e9d438a243..0e4f5e941e2 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -542,13 +542,16 @@ impl Tester { let block = Arc::new(block); let result: Result, _> = self - .block_on_dangerous(self.harness.chain.process_block( - block_root, - RpcBlock::new_without_blobs(Some(block_root), block.clone()), - NotifyExecutionLayer::Yes, - BlockImportSource::Lookup, - || Ok(()), - ))? + .block_on_dangerous( + self.harness.chain.process_block( + block_root, + RpcBlock::new_maybe_available(Some(block_root), block.clone(), None, None) + .map_err(|e| Error::InternalError(format!("{:?}", e)))?, + NotifyExecutionLayer::Yes, + BlockImportSource::Lookup, + || Ok(()), + ), + )? .map(|avail: AvailabilityProcessingStatus| avail.try_into()); let success = data_column_success && result.as_ref().is_ok_and(|inner| inner.is_ok()); if success != valid { @@ -632,13 +635,16 @@ impl Tester { let block = Arc::new(block); let result: Result, _> = self - .block_on_dangerous(self.harness.chain.process_block( - block_root, - RpcBlock::new_without_blobs(Some(block_root), block.clone()), - NotifyExecutionLayer::Yes, - BlockImportSource::Lookup, - || Ok(()), - ))? + .block_on_dangerous( + self.harness.chain.process_block( + block_root, + RpcBlock::new_maybe_available(Some(block_root), block.clone(), None, None) + .map_err(|e| Error::InternalError(format!("{:?}", e)))?, + NotifyExecutionLayer::Yes, + BlockImportSource::Lookup, + || Ok(()), + ), + )? .map(|avail: AvailabilityProcessingStatus| avail.try_into()); let success = blob_success && result.as_ref().is_ok_and(|inner| inner.is_ok()); if success != valid { From d97a9ffffc053fa0b45456f16fd3680df4d591bf Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 18 Nov 2025 19:15:35 -0300 Subject: [PATCH 02/24] Fix tests --- .../src/block_verification_types.rs | 2 +- .../beacon_chain/tests/block_verification.rs | 48 +++++++++++++------ .../tests/payload_invalidation.rs | 11 +++-- beacon_node/beacon_chain/tests/store_tests.rs | 6 ++- .../src/network_beacon_processor/tests.rs | 16 ++++++- 5 files changed, 59 insertions(+), 24 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_verification_types.rs b/beacon_node/beacon_chain/src/block_verification_types.rs index 0fa026051c5..a8bbc343b3e 100644 --- a/beacon_node/beacon_chain/src/block_verification_types.rs +++ b/beacon_node/beacon_chain/src/block_verification_types.rs @@ -63,7 +63,7 @@ impl RpcBlock { } } - pub fn rpc_block_inner(&self) -> &RpcBlockInner { + fn rpc_block_inner(&self) -> &RpcBlockInner { match self { RpcBlock::Available(available_rpc_block) => &available_rpc_block.block, RpcBlock::MaybeAvailable(maybe_available_rpc_block) => &maybe_available_rpc_block.block, diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 881885cef23..5076639d843 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -146,12 +146,12 @@ fn build_rpc_block( ) -> RpcBlock { match data_sidecars { Some(DataSidecars::Blobs(blobs)) => { - RpcBlock::new(None, block, Some(blobs.clone())).unwrap() + RpcBlock::new_available(None, block, Some(blobs.clone()), None).unwrap() } Some(DataSidecars::DataColumns(columns)) => { - RpcBlock::new_with_custody_columns(None, block, columns.clone()).unwrap() + RpcBlock::new_available(None, block, None, Some(columns.clone())).unwrap() } - None => RpcBlock::new_without_blobs(None, block), + None => RpcBlock::new_available(None, block, None, None).unwrap(), } } @@ -368,10 +368,13 @@ async fn chain_segment_non_linear_parent_roots() { let (mut block, signature) = blocks[3].as_block().clone().deconstruct(); *block.parent_root_mut() = Hash256::zero(); - blocks[3] = RpcBlock::new_without_blobs( + blocks[3] = RpcBlock::new_available( None, Arc::new(SignedBeaconBlock::from_block(block, signature)), - ); + None, + None, + ) + .unwrap(); assert!( matches!( @@ -404,10 +407,13 @@ async fn chain_segment_non_linear_slots() { .collect(); let (mut block, signature) = blocks[3].as_block().clone().deconstruct(); *block.slot_mut() = Slot::new(0); - blocks[3] = RpcBlock::new_without_blobs( + blocks[3] = RpcBlock::new_available( None, Arc::new(SignedBeaconBlock::from_block(block, signature)), - ); + None, + None, + ) + .unwrap(); assert!( matches!( @@ -430,10 +436,13 @@ async fn chain_segment_non_linear_slots() { .collect(); let (mut block, signature) = blocks[3].as_block().clone().deconstruct(); *block.slot_mut() = blocks[2].slot(); - blocks[3] = RpcBlock::new_without_blobs( + blocks[3] = RpcBlock::new_available( None, Arc::new(SignedBeaconBlock::from_block(block, signature)), - ); + None, + None, + ) + .unwrap(); assert!( matches!( @@ -567,7 +576,8 @@ async fn invalid_signature_gossip_block() { .into_block_error() .expect("should import all blocks prior to the one being tested"); let signed_block = SignedBeaconBlock::from_block(block, junk_signature()); - let rpc_block = RpcBlock::new_without_blobs(None, Arc::new(signed_block)); + let rpc_block = + RpcBlock::new_maybe_available(None, Arc::new(signed_block), None, None).unwrap(); let process_res = harness .chain .process_block( @@ -1568,7 +1578,8 @@ async fn add_base_block_to_altair_chain() { )); // Ensure that it would be impossible to import via `BeaconChain::process_block`. - let base_rpc_block = RpcBlock::new_without_blobs(None, Arc::new(base_block.clone())); + let base_rpc_block = + RpcBlock::new_maybe_available(None, Arc::new(base_block.clone()), None, None).unwrap(); assert!(matches!( harness .chain @@ -1592,7 +1603,9 @@ async fn add_base_block_to_altair_chain() { harness .chain .process_chain_segment( - vec![RpcBlock::new_without_blobs(None, Arc::new(base_block))], + vec![ + RpcBlock::new_maybe_available(None, Arc::new(base_block), None, None).unwrap() + ], NotifyExecutionLayer::Yes, ) .await, @@ -1705,7 +1718,8 @@ async fn add_altair_block_to_base_chain() { )); // Ensure that it would be impossible to import via `BeaconChain::process_block`. - let altair_rpc_block = RpcBlock::new_without_blobs(None, Arc::new(altair_block.clone())); + let altair_rpc_block = + RpcBlock::new_maybe_available(None, Arc::new(altair_block.clone()), None, None).unwrap(); assert!(matches!( harness .chain @@ -1729,7 +1743,10 @@ async fn add_altair_block_to_base_chain() { harness .chain .process_chain_segment( - vec![RpcBlock::new_without_blobs(None, Arc::new(altair_block))], + vec![ + RpcBlock::new_maybe_available(None, Arc::new(altair_block), None, None) + .unwrap() + ], NotifyExecutionLayer::Yes ) .await, @@ -1792,7 +1809,8 @@ async fn import_duplicate_block_unrealized_justification() { // Create two verified variants of the block, representing the same block being processed in // parallel. let notify_execution_layer = NotifyExecutionLayer::Yes; - let rpc_block = RpcBlock::new_without_blobs(Some(block_root), block.clone()); + let rpc_block = + RpcBlock::new_maybe_available(Some(block_root), block.clone(), None, None).unwrap(); let verified_block1 = rpc_block .clone() .into_execution_pending_block(block_root, chain, notify_execution_layer) diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index 5bd43835e33..b47c5abc164 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -685,7 +685,8 @@ async fn invalidates_all_descendants() { assert_eq!(fork_parent_state.slot(), fork_parent_slot); let ((fork_block, _), _fork_post_state) = rig.harness.make_block(fork_parent_state, fork_slot).await; - let fork_rpc_block = RpcBlock::new_without_blobs(None, fork_block.clone()); + let fork_rpc_block = + RpcBlock::new_maybe_available(None, fork_block.clone(), None, None).unwrap(); let fork_block_root = rig .harness .chain @@ -787,7 +788,8 @@ async fn switches_heads() { let ((fork_block, _), _fork_post_state) = rig.harness.make_block(fork_parent_state, fork_slot).await; let fork_parent_root = fork_block.parent_root(); - let fork_rpc_block = RpcBlock::new_without_blobs(None, fork_block.clone()); + let fork_rpc_block = + RpcBlock::new_maybe_available(None, fork_block.clone(), None, None).unwrap(); let fork_block_root = rig .harness .chain @@ -1059,7 +1061,7 @@ async fn invalid_parent() { )); // Ensure the block built atop an invalid payload is invalid for import. - let rpc_block = RpcBlock::new_without_blobs(None, block.clone()); + let rpc_block = RpcBlock::new_maybe_available(None, block.clone(), None, None).unwrap(); assert!(matches!( rig.harness.chain.process_block(rpc_block.block_root(), rpc_block, NotifyExecutionLayer::Yes, BlockImportSource::Lookup, || Ok(()), @@ -1384,7 +1386,8 @@ async fn recover_from_invalid_head_by_importing_blocks() { } = InvalidHeadSetup::new().await; // Import the fork block, it should become the head. - let fork_rpc_block = RpcBlock::new_without_blobs(None, fork_block.clone()); + let fork_rpc_block = + RpcBlock::new_maybe_available(None, fork_block.clone(), None, None).unwrap(); rig.harness .chain .process_block( diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 638c221a7fa..01c7100dfe3 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -3533,7 +3533,8 @@ async fn process_blocks_and_attestations_for_unaligned_checkpoint() { assert_eq!(split.block_root, valid_fork_block.parent_root()); assert_ne!(split.state_root, unadvanced_split_state_root); - let invalid_fork_rpc_block = RpcBlock::new_without_blobs(None, invalid_fork_block.clone()); + let invalid_fork_rpc_block = + RpcBlock::new_maybe_available(None, invalid_fork_block.clone(), None, None).unwrap(); // Applying the invalid block should fail. let err = harness .chain @@ -3549,7 +3550,8 @@ async fn process_blocks_and_attestations_for_unaligned_checkpoint() { assert!(matches!(err, BlockError::WouldRevertFinalizedSlot { .. })); // Applying the valid block should succeed, but it should not become head. - let valid_fork_rpc_block = RpcBlock::new_without_blobs(None, valid_fork_block.clone()); + let valid_fork_rpc_block = + RpcBlock::new_maybe_available(None, valid_fork_block.clone(), None, None).unwrap(); harness .chain .process_block( diff --git a/beacon_node/network/src/network_beacon_processor/tests.rs b/beacon_node/network/src/network_beacon_processor/tests.rs index d83059ad278..7892f3d9edf 100644 --- a/beacon_node/network/src/network_beacon_processor/tests.rs +++ b/beacon_node/network/src/network_beacon_processor/tests.rs @@ -398,7 +398,13 @@ impl TestRig { self.network_beacon_processor .send_rpc_beacon_block( block_root, - RpcBlock::new_without_blobs(Some(block_root), self.next_block.clone()), + RpcBlock::new_maybe_available( + Some(block_root), + self.next_block.clone(), + None, + None, + ) + .unwrap(), std::time::Duration::default(), BlockProcessType::SingleBlock { id: 0 }, ) @@ -410,7 +416,13 @@ impl TestRig { self.network_beacon_processor .send_rpc_beacon_block( block_root, - RpcBlock::new_without_blobs(Some(block_root), self.next_block.clone()), + RpcBlock::new_maybe_available( + Some(block_root), + self.next_block.clone(), + None, + None, + ) + .unwrap(), std::time::Duration::default(), BlockProcessType::SingleBlock { id: 1 }, ) From f92e1df7a5127a7621d5b4fb1ac4e49d70f66b08 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 25 Nov 2025 13:16:51 -0300 Subject: [PATCH 03/24] Fix tests --- .../src/block_verification_types.rs | 54 +++++-------------- 1 file changed, 12 insertions(+), 42 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_verification_types.rs b/beacon_node/beacon_chain/src/block_verification_types.rs index a8bbc343b3e..397dd311b98 100644 --- a/beacon_node/beacon_chain/src/block_verification_types.rs +++ b/beacon_node/beacon_chain/src/block_verification_types.rs @@ -131,15 +131,6 @@ impl RpcBlockInner { block: Arc>, blobs: Option>, ) -> Result { - match block.fork_name_unchecked() { - ForkName::Base - | ForkName::Altair - | ForkName::Bellatrix - | ForkName::Capella - | ForkName::Fulu - | ForkName::Gloas => return Err(AvailabilityCheckError::InvalidFork), - ForkName::Deneb | ForkName::Electra => (), - } // Treat empty blob lists as if they are missing. let blobs = blobs.filter(|b| !b.is_empty()); if let (Some(blobs), Ok(block_commitments)) = ( @@ -174,21 +165,10 @@ impl RpcBlockInner { /// guarantees about whether columns should be present, only that they are /// consistent with the block. An empty list passed in for `custody_columns` is /// viewed the same as `None` passed in. - pub fn new_block_and_columns( + fn new_block_and_columns( block: Arc>, custody_columns: Option>>, ) -> Result { - match block.fork_name_unchecked() { - ForkName::Base - | ForkName::Altair - | ForkName::Bellatrix - | ForkName::Capella - | ForkName::Deneb - | ForkName::Electra - | ForkName::Gloas => return Err(AvailabilityCheckError::InvalidFork), - ForkName::Fulu => (), - } - let columns = if let Some(custody_columns) = custody_columns { if block.num_expected_blobs() > 0 && custody_columns.is_empty() { // The number of required custody columns is out of scope here. @@ -208,14 +188,18 @@ impl RpcBlockInner { } /// Constructs a new `Block` variant. - pub fn new(block: Arc>) -> Result { + pub fn new( + block: Arc>, + blobs: Option>, + columns: Option>>, + ) -> Result { match block.fork_name_unchecked() { - ForkName::Deneb | ForkName::Electra | ForkName::Fulu | ForkName::Gloas => { - return Err(AvailabilityCheckError::InvalidFork); + ForkName::Base | ForkName::Altair | ForkName::Bellatrix | ForkName::Capella => { + Ok(RpcBlockInner::Block(block)) } - ForkName::Base | ForkName::Altair | ForkName::Bellatrix | ForkName::Capella => (), + ForkName::Deneb | ForkName::Electra => Self::new_block_and_blobs(block, blobs), + ForkName::Fulu | ForkName::Gloas => Self::new_block_and_columns(block, columns), } - Ok(Self::Block(block)) } } @@ -230,14 +214,7 @@ impl RpcBlock { columns: Option>>, ) -> Result { let block_root = block_root.unwrap_or_else(|| get_block_root(&block)); - - let rpc_block_inner = if blobs.is_some() { - RpcBlockInner::new_block_and_blobs(block, blobs)? - } else if columns.is_some() { - RpcBlockInner::new_block_and_columns(block, columns)? - } else { - RpcBlockInner::new(block)? - }; + let rpc_block_inner = RpcBlockInner::new(block, blobs, columns)?; Ok(RpcBlock::MaybeAvailable(MaybeAvailableRpcBlock { block_root, @@ -257,14 +234,7 @@ impl RpcBlock { columns: Option>>, ) -> Result { let block_root = block_root.unwrap_or_else(|| get_block_root(&block)); - - let rpc_block_inner = if blobs.is_some() { - RpcBlockInner::new_block_and_blobs(block, blobs)? - } else if columns.is_some() { - RpcBlockInner::new_block_and_columns(block, columns)? - } else { - RpcBlockInner::new(block)? - }; + let rpc_block_inner = RpcBlockInner::new(block, blobs, columns)?; Ok(RpcBlock::Available(AvailableRpcBlock { block_root, From ba5a58ee12e812c23896afce3a122f6c64b91e66 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 25 Nov 2025 13:29:08 -0300 Subject: [PATCH 04/24] Fix tests --- beacon_node/beacon_chain/src/test_utils.rs | 23 +++++++++++++++---- .../beacon_chain/tests/blob_verification.rs | 2 +- .../beacon_chain/tests/column_verification.rs | 2 +- 3 files changed, 20 insertions(+), 7 deletions(-) diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 53eba4cb180..0886e6fec1a 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -2368,7 +2368,7 @@ where self.set_current_slot(slot); let (block, blob_items) = block_contents; - let rpc_block = self.build_rpc_block_from_blobs(block_root, block, blob_items)?; + let rpc_block = self.build_rpc_block_from_blobs(block_root, block, blob_items, true)?; let block_hash: SignedBeaconBlockHash = self .chain .process_block( @@ -2392,7 +2392,7 @@ where let (block, blob_items) = block_contents; let block_root = block.canonical_root(); - let rpc_block = self.build_rpc_block_from_blobs(block_root, block, blob_items)?; + let rpc_block = self.build_rpc_block_from_blobs(block_root, block, blob_items, true)?; let block_hash: SignedBeaconBlockHash = self .chain .process_block( @@ -2447,6 +2447,7 @@ where block_root: Hash256, block: Arc>>, blob_items: Option<(KzgProofs, BlobsList)>, + is_available: bool, ) -> Result, BlockError> { Ok(if self.spec.is_peer_das_enabled_for_epoch(block.epoch()) { let epoch = block.slot().epoch(E::slots_per_epoch()); @@ -2462,9 +2463,17 @@ where .map(CustodyDataColumn::from_asserted_custody) .collect::>(); // TODO(can combine these and clean it up) - RpcBlock::new_available(Some(block_root), block, None, Some(columns))? + if is_available { + RpcBlock::new_available(Some(block_root), block, None, Some(columns))? + } else { + RpcBlock::new_maybe_available(Some(block_root), block, None, Some(columns))? + } } else { - RpcBlock::new_available(Some(block_root), block, None, None)? + if is_available { + RpcBlock::new_available(Some(block_root), block, None, None)? + } else { + RpcBlock::new_maybe_available(Some(block_root), block, None, None)? + } } } else { let blobs = blob_items @@ -2473,7 +2482,11 @@ where }) .transpose() .unwrap(); - RpcBlock::new_available(Some(block_root), block, blobs, None)? + if is_available { + RpcBlock::new_available(Some(block_root), block, blobs, None)? + } else { + RpcBlock::new_maybe_available(Some(block_root), block, blobs, None)? + } }) } diff --git a/beacon_node/beacon_chain/tests/blob_verification.rs b/beacon_node/beacon_chain/tests/blob_verification.rs index c42a2828c01..8dde52fae9c 100644 --- a/beacon_node/beacon_chain/tests/blob_verification.rs +++ b/beacon_node/beacon_chain/tests/blob_verification.rs @@ -76,7 +76,7 @@ async fn rpc_blobs_with_invalid_header_signature() { // Process the block without blobs so that it doesn't become available. harness.advance_slot(); let rpc_block = harness - .build_rpc_block_from_blobs(block_root, signed_block.clone(), None) + .build_rpc_block_from_blobs(block_root, signed_block.clone(), None, false) .unwrap(); let availability = harness .chain diff --git a/beacon_node/beacon_chain/tests/column_verification.rs b/beacon_node/beacon_chain/tests/column_verification.rs index 229ae1e1998..0dbe939839f 100644 --- a/beacon_node/beacon_chain/tests/column_verification.rs +++ b/beacon_node/beacon_chain/tests/column_verification.rs @@ -80,7 +80,7 @@ async fn rpc_columns_with_invalid_header_signature() { // Process the block without blobs so that it doesn't become available. harness.advance_slot(); let rpc_block = harness - .build_rpc_block_from_blobs(block_root, signed_block.clone(), None) + .build_rpc_block_from_blobs(block_root, signed_block.clone(), None, false) .unwrap(); let availability = harness .chain From 545bf412923101de7253824b1d16a4d679db3b60 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 25 Nov 2025 21:33:42 -0300 Subject: [PATCH 05/24] Temp --- beacon_node/beacon_chain/src/beacon_chain.rs | 1 + beacon_node/beacon_chain/src/block_verification.rs | 1 + beacon_node/beacon_chain/src/data_availability_checker.rs | 3 +++ beacon_node/beacon_chain/tests/blob_verification.rs | 5 ++++- crypto/kzg/src/lib.rs | 3 +++ 5 files changed, 12 insertions(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 494346e7ff2..b5e898dfe5c 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -3345,6 +3345,7 @@ impl BeaconChain { publish_fn: impl FnOnce() -> Result<(), BlockError>, ) -> Result { let block_slot = unverified_block.block().slot(); + println!("HI I AM HERE?"); // Set observed time if not already set. Usually this should be set by gossip or RPC, // but just in case we set it again here (useful for tests). diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 374f1e2b360..5f1e5d8096c 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -1294,6 +1294,7 @@ impl IntoExecutionPendingBlock for RpcBlock chain: &Arc>, notify_execution_layer: NotifyExecutionLayer, ) -> Result, BlockSlashInfo> { + println!("YOOOOO"); // Perform an early check to prevent wasting time on irrelevant blocks. let block_root = check_block_relevancy(self.as_block(), block_root, chain) .map_err(|e| BlockSlashInfo::SignatureNotChecked(self.signed_block_header(), e))?; diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index b91a5a0ea30..9c6c7b4d73e 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -377,7 +377,10 @@ impl DataAvailabilityChecker { ) -> Result, AvailabilityCheckError> { let (block_root, block, blobs, data_columns) = block.deconstruct(); if self.blobs_required_for_block(&block) { + println!("BLOBS REQUURED"); + return if let Some(blob_list) = blobs { + println!("{:?}", blob_list.len()); verify_kzg_for_blob_list(blob_list.iter(), &self.kzg) .map_err(AvailabilityCheckError::InvalidBlobs)?; Ok(MaybeAvailableBlock::Available(AvailableBlock { diff --git a/beacon_node/beacon_chain/tests/blob_verification.rs b/beacon_node/beacon_chain/tests/blob_verification.rs index 8dde52fae9c..04370793151 100644 --- a/beacon_node/beacon_chain/tests/blob_verification.rs +++ b/beacon_node/beacon_chain/tests/blob_verification.rs @@ -84,11 +84,12 @@ async fn rpc_blobs_with_invalid_header_signature() { block_root, rpc_block, NotifyExecutionLayer::Yes, - BlockImportSource::RangeSync, + BlockImportSource::Lookup, || Ok(()), ) .await .unwrap(); + assert_eq!( availability, AvailabilityProcessingStatus::MissingComponents(slot, block_root) @@ -113,6 +114,8 @@ async fn rpc_blobs_with_invalid_header_signature() { .process_rpc_blobs(slot, block_root, blob_sidecars) .await .unwrap_err(); + + println!("{:?}", err); assert!(matches!( err, BlockError::InvalidSignature(InvalidSignature::ProposerSignature) diff --git a/crypto/kzg/src/lib.rs b/crypto/kzg/src/lib.rs index 0fe95b77230..c7875d737c1 100644 --- a/crypto/kzg/src/lib.rs +++ b/crypto/kzg/src/lib.rs @@ -155,6 +155,7 @@ impl Kzg { kzg_commitments: &[KzgCommitment], kzg_proofs: &[KzgProof], ) -> Result<(), Error> { + println!("yyy {:?}", blobs.len()); let commitments_bytes = kzg_commitments .iter() .map(|comm| Bytes48::from(*comm)) @@ -170,8 +171,10 @@ impl Kzg { &commitments_bytes, &proofs_bytes, )? { + println!("FAILED"); Err(Error::KzgVerificationFailed) } else { + println!("SUCCESS"); Ok(()) } } From 44e56725b1f9984999654f95b542c56f6fa4fbd4 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Sun, 30 Nov 2025 16:34:07 -0300 Subject: [PATCH 06/24] Fix --- beacon_node/beacon_chain/src/beacon_chain.rs | 1 - beacon_node/beacon_chain/src/block_verification.rs | 1 - beacon_node/beacon_chain/src/data_availability_checker.rs | 7 +++---- beacon_node/beacon_chain/src/test_utils.rs | 8 +++----- beacon_node/beacon_chain/tests/blob_verification.rs | 2 +- crypto/kzg/src/lib.rs | 3 --- 6 files changed, 7 insertions(+), 15 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index b5e898dfe5c..494346e7ff2 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -3345,7 +3345,6 @@ impl BeaconChain { publish_fn: impl FnOnce() -> Result<(), BlockError>, ) -> Result { let block_slot = unverified_block.block().slot(); - println!("HI I AM HERE?"); // Set observed time if not already set. Usually this should be set by gossip or RPC, // but just in case we set it again here (useful for tests). diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 5f1e5d8096c..374f1e2b360 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -1294,7 +1294,6 @@ impl IntoExecutionPendingBlock for RpcBlock chain: &Arc>, notify_execution_layer: NotifyExecutionLayer, ) -> Result, BlockSlashInfo> { - println!("YOOOOO"); // Perform an early check to prevent wasting time on irrelevant blocks. let block_root = check_block_relevancy(self.as_block(), block_root, chain) .map_err(|e| BlockSlashInfo::SignatureNotChecked(self.signed_block_header(), e))?; diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 9c6c7b4d73e..07dc759c23d 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -377,10 +377,9 @@ impl DataAvailabilityChecker { ) -> Result, AvailabilityCheckError> { let (block_root, block, blobs, data_columns) = block.deconstruct(); if self.blobs_required_for_block(&block) { - println!("BLOBS REQUURED"); - - return if let Some(blob_list) = blobs { - println!("{:?}", blob_list.len()); + return if let Some(blob_list) = blobs + && blob_list.len() == block.num_expected_blobs() + { verify_kzg_for_blob_list(blob_list.iter(), &self.kzg) .map_err(AvailabilityCheckError::InvalidBlobs)?; Ok(MaybeAvailableBlock::Available(AvailableBlock { diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index 0886e6fec1a..f915fb6027f 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -2468,12 +2468,10 @@ where } else { RpcBlock::new_maybe_available(Some(block_root), block, None, Some(columns))? } + } else if is_available { + RpcBlock::new_available(Some(block_root), block, None, None)? } else { - if is_available { - RpcBlock::new_available(Some(block_root), block, None, None)? - } else { - RpcBlock::new_maybe_available(Some(block_root), block, None, None)? - } + RpcBlock::new_maybe_available(Some(block_root), block, None, None)? } } else { let blobs = blob_items diff --git a/beacon_node/beacon_chain/tests/blob_verification.rs b/beacon_node/beacon_chain/tests/blob_verification.rs index 04370793151..dfb03b3fd2f 100644 --- a/beacon_node/beacon_chain/tests/blob_verification.rs +++ b/beacon_node/beacon_chain/tests/blob_verification.rs @@ -89,7 +89,7 @@ async fn rpc_blobs_with_invalid_header_signature() { ) .await .unwrap(); - + assert_eq!( availability, AvailabilityProcessingStatus::MissingComponents(slot, block_root) diff --git a/crypto/kzg/src/lib.rs b/crypto/kzg/src/lib.rs index c7875d737c1..0fe95b77230 100644 --- a/crypto/kzg/src/lib.rs +++ b/crypto/kzg/src/lib.rs @@ -155,7 +155,6 @@ impl Kzg { kzg_commitments: &[KzgCommitment], kzg_proofs: &[KzgProof], ) -> Result<(), Error> { - println!("yyy {:?}", blobs.len()); let commitments_bytes = kzg_commitments .iter() .map(|comm| Bytes48::from(*comm)) @@ -171,10 +170,8 @@ impl Kzg { &commitments_bytes, &proofs_bytes, )? { - println!("FAILED"); Err(Error::KzgVerificationFailed) } else { - println!("SUCCESS"); Ok(()) } } From aad7828f0c6184085d08a0223846ebbdfeb3c4d6 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Sun, 30 Nov 2025 17:18:49 -0300 Subject: [PATCH 07/24] Fix verify_kzg --- .../src/block_verification_types.rs | 46 ++++++++++ .../src/data_availability_checker.rs | 87 +++++++++++++++++-- 2 files changed, 125 insertions(+), 8 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_verification_types.rs b/beacon_node/beacon_chain/src/block_verification_types.rs index 397dd311b98..b20820e0f52 100644 --- a/beacon_node/beacon_chain/src/block_verification_types.rs +++ b/beacon_node/beacon_chain/src/block_verification_types.rs @@ -40,6 +40,29 @@ pub struct MaybeAvailableRpcBlock { block: RpcBlockInner, } +impl MaybeAvailableRpcBlock { + #[allow(clippy::type_complexity)] + pub fn deconstruct( + self, + ) -> ( + Hash256, + Arc>, + Option>, + Option>, + ) { + let block_root = self.block_root; + match self.block { + RpcBlockInner::Block(block) => (block_root, block.clone(), None, None), + RpcBlockInner::BlockAndBlobs(block, blobs) => { + (block_root, block.clone(), Some(blobs.clone()), None) + } + RpcBlockInner::BlockAndCustodyColumns(block, data_columns) => { + (block_root, block.clone(), None, Some(data_columns.clone())) + } + } + } +} + #[derive(Clone, Educe)] #[educe(Hash(bound(E: EthSpec)))] pub struct AvailableRpcBlock { @@ -47,6 +70,29 @@ pub struct AvailableRpcBlock { block: RpcBlockInner, } +impl AvailableRpcBlock { + #[allow(clippy::type_complexity)] + pub fn deconstruct( + self, + ) -> ( + Hash256, + Arc>, + Option>, + Option>, + ) { + let block_root = self.block_root; + match self.block { + RpcBlockInner::Block(block) => (block_root, block.clone(), None, None), + RpcBlockInner::BlockAndBlobs(block, blobs) => { + (block_root, block.clone(), Some(blobs.clone()), None) + } + RpcBlockInner::BlockAndCustodyColumns(block, data_columns) => { + (block_root, block.clone(), None, Some(data_columns.clone())) + } + } + } +} + impl Debug for RpcBlock { fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { write!(f, "RpcBlock({:?})", self.block_root()) diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 07dc759c23d..6d0fe73f020 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -2,7 +2,8 @@ use crate::blob_verification::{ GossipVerifiedBlob, KzgVerifiedBlob, KzgVerifiedBlobList, verify_kzg_for_blob_list, }; use crate::block_verification_types::{ - AvailabilityPendingExecutedBlock, AvailableExecutedBlock, RpcBlock, + AvailabilityPendingExecutedBlock, AvailableExecutedBlock, AvailableRpcBlock, + MaybeAvailableRpcBlock, RpcBlock, }; use crate::data_availability_checker::overflow_lru_cache::{ DataAvailabilityCheckerInner, ReconstructColumnsDecision, @@ -366,14 +367,9 @@ impl DataAvailabilityChecker { .remove_pre_execution_block(block_root); } - /// Verifies kzg commitments for an RpcBlock, returns a `MaybeAvailableBlock` that may - /// include the fully available block. - /// - /// WARNING: This function assumes all required blobs are already present, it does NOT - /// check if there are any missing blobs. - pub fn verify_kzg_for_rpc_block( + pub fn verify_kzg_for_maybe_available_rpc_block( &self, - block: RpcBlock, + block: MaybeAvailableRpcBlock, ) -> Result, AvailabilityCheckError> { let (block_root, block, blobs, data_columns) = block.deconstruct(); if self.blobs_required_for_block(&block) { @@ -428,6 +424,81 @@ impl DataAvailabilityChecker { })) } + pub fn verify_kzg_for_available_rpc_block( + &self, + block: AvailableRpcBlock, + ) -> Result, AvailabilityCheckError> { + let (block_root, block, blobs, data_columns) = block.deconstruct(); + + if self.blobs_required_for_block(&block) { + return if let Some(blob_list) = blobs { + verify_kzg_for_blob_list(blob_list.iter(), &self.kzg) + .map_err(AvailabilityCheckError::InvalidBlobs)?; + Ok(MaybeAvailableBlock::Available(AvailableBlock { + block_root, + block, + blob_data: AvailableBlockData::Blobs(blob_list), + blobs_available_timestamp: None, + spec: self.spec.clone(), + })) + } else { + return Err(AvailabilityCheckError::MissingBlobs); + }; + } + + if self.data_columns_required_for_block(&block) { + return if let Some(data_column_list) = data_columns { + verify_kzg_for_data_column_list( + data_column_list + .iter() + .map(|custody_column| custody_column.as_data_column()), + &self.kzg, + ) + .map_err(AvailabilityCheckError::InvalidColumn)?; + Ok(MaybeAvailableBlock::Available(AvailableBlock { + block_root, + block, + blob_data: AvailableBlockData::DataColumns( + data_column_list + .into_iter() + .map(|d| d.clone_arc()) + .collect(), + ), + blobs_available_timestamp: None, + spec: self.spec.clone(), + })) + } else { + return Err(AvailabilityCheckError::MissingCustodyColumns); + }; + } + Ok(MaybeAvailableBlock::Available(AvailableBlock { + block_root, + block, + blob_data: AvailableBlockData::NoData, + blobs_available_timestamp: None, + spec: self.spec.clone(), + })) + } + + /// Verifies kzg commitments for an RpcBlock, returns a `MaybeAvailableBlock` that may + /// include the fully available block. + /// + /// WARNING: This function assumes all required blobs are already present, it does NOT + /// check if there are any missing blobs. + pub fn verify_kzg_for_rpc_block( + &self, + block: RpcBlock, + ) -> Result, AvailabilityCheckError> { + match block { + RpcBlock::Available(available_rpc_block) => { + self.verify_kzg_for_available_rpc_block(available_rpc_block) + } + RpcBlock::MaybeAvailable(maybe_available_rpc_block) => { + self.verify_kzg_for_maybe_available_rpc_block(maybe_available_rpc_block) + } + } + } + /// Checks if a vector of blocks are available. Returns a vector of `MaybeAvailableBlock` /// This is more efficient than calling `verify_kzg_for_rpc_block` in a loop as it does /// all kzg verification at once From 6f07e447136e52629723ae8df0887025008fae59 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 1 Dec 2025 11:40:00 -0300 Subject: [PATCH 08/24] Fix test --- beacon_node/beacon_chain/src/data_availability_checker.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 6d0fe73f020..33c8aa7dffb 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -390,7 +390,9 @@ impl DataAvailabilityChecker { }; } if self.data_columns_required_for_block(&block) { - return if let Some(data_column_list) = data_columns.as_ref() { + return if let Some(data_column_list) = data_columns.as_ref() + && data_column_list.len() == T::EthSpec::number_of_columns() + { verify_kzg_for_data_column_list( data_column_list .iter() From 2b3b92390389737e55a193c5685dc0412ff0c2c1 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 1 Dec 2025 12:20:24 -0300 Subject: [PATCH 09/24] Update kzg block verif for list --- .../src/data_availability_checker.rs | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 33c8aa7dffb..b19353890ea 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -544,10 +544,17 @@ impl DataAvailabilityChecker { } for block in blocks { + let is_available = match block { + RpcBlock::Available(_) => true, + RpcBlock::MaybeAvailable(_) => false, + }; + let (block_root, block, blobs, data_columns) = block.deconstruct(); let maybe_available_block = if self.blobs_required_for_block(&block) { - if let Some(blobs) = blobs { + if let Some(blobs) = blobs + && blobs.len() == block.num_expected_blobs() + { MaybeAvailableBlock::Available(AvailableBlock { block_root, block, @@ -556,10 +563,15 @@ impl DataAvailabilityChecker { spec: self.spec.clone(), }) } else { + if is_available { + return Err(AvailabilityCheckError::MissingBlobs); + } MaybeAvailableBlock::AvailabilityPending { block_root, block } } } else if self.data_columns_required_for_block(&block) { - if let Some(data_columns) = data_columns { + if let Some(data_columns) = data_columns + && data_columns.len() == T::EthSpec::number_of_columns() + { MaybeAvailableBlock::Available(AvailableBlock { block_root, block, @@ -570,6 +582,9 @@ impl DataAvailabilityChecker { spec: self.spec.clone(), }) } else { + if is_available { + return Err(AvailabilityCheckError::MissingCustodyColumns); + } MaybeAvailableBlock::AvailabilityPending { block_root, block } } } else { From be9d7c9fa85e56e49261283f95272d9c0010f72e Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 1 Dec 2025 13:13:28 -0300 Subject: [PATCH 10/24] Fix --- beacon_node/beacon_chain/src/data_availability_checker.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index b19353890ea..45e826a4356 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -570,7 +570,6 @@ impl DataAvailabilityChecker { } } else if self.data_columns_required_for_block(&block) { if let Some(data_columns) = data_columns - && data_columns.len() == T::EthSpec::number_of_columns() { MaybeAvailableBlock::Available(AvailableBlock { block_root, From 1cb189bc19486423739f0cbf6fc4d6683450d289 Mon Sep 17 00:00:00 2001 From: Mark Mackey Date: Thu, 4 Dec 2025 11:03:49 -0600 Subject: [PATCH 11/24] beginning changes --- .../src/block_verification_types.rs | 266 ++---------------- .../src/data_availability_checker.rs | 14 + 2 files changed, 43 insertions(+), 237 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_verification_types.rs b/beacon_node/beacon_chain/src/block_verification_types.rs index b20820e0f52..3fc27eb41bd 100644 --- a/beacon_node/beacon_chain/src/block_verification_types.rs +++ b/beacon_node/beacon_chain/src/block_verification_types.rs @@ -1,5 +1,5 @@ use crate::data_availability_checker::AvailabilityCheckError; -pub use crate::data_availability_checker::{AvailableBlock, MaybeAvailableBlock}; +pub use crate::data_availability_checker::{AvailableBlock, AvailableBlockData, MaybeAvailableBlock}; use crate::data_column_verification::{CustodyDataColumn, CustodyDataColumnList}; use crate::{PayloadVerificationOutcome, get_block_root}; use educe::Educe; @@ -29,68 +29,11 @@ use types::{ #[derive(Clone, Educe)] #[educe(Hash(bound(E: EthSpec)))] pub enum RpcBlock { - Available(AvailableRpcBlock), - MaybeAvailable(MaybeAvailableRpcBlock), -} - -#[derive(Clone, Educe)] -#[educe(Hash(bound(E: EthSpec)))] -pub struct MaybeAvailableRpcBlock { - block_root: Hash256, - block: RpcBlockInner, -} - -impl MaybeAvailableRpcBlock { - #[allow(clippy::type_complexity)] - pub fn deconstruct( - self, - ) -> ( - Hash256, - Arc>, - Option>, - Option>, - ) { - let block_root = self.block_root; - match self.block { - RpcBlockInner::Block(block) => (block_root, block.clone(), None, None), - RpcBlockInner::BlockAndBlobs(block, blobs) => { - (block_root, block.clone(), Some(blobs.clone()), None) - } - RpcBlockInner::BlockAndCustodyColumns(block, data_columns) => { - (block_root, block.clone(), None, Some(data_columns.clone())) - } - } - } -} - -#[derive(Clone, Educe)] -#[educe(Hash(bound(E: EthSpec)))] -pub struct AvailableRpcBlock { - block_root: Hash256, - block: RpcBlockInner, -} - -impl AvailableRpcBlock { - #[allow(clippy::type_complexity)] - pub fn deconstruct( - self, - ) -> ( - Hash256, - Arc>, - Option>, - Option>, - ) { - let block_root = self.block_root; - match self.block { - RpcBlockInner::Block(block) => (block_root, block.clone(), None, None), - RpcBlockInner::BlockAndBlobs(block, blobs) => { - (block_root, block.clone(), Some(blobs.clone()), None) - } - RpcBlockInner::BlockAndCustodyColumns(block, data_columns) => { - (block_root, block.clone(), None, Some(data_columns.clone())) - } - } - } + FullyAvailable(AvailableBlock), + BlockOnly { + block: Arc>, + block_root: Hash256, + }, } impl Debug for RpcBlock { @@ -102,191 +45,45 @@ impl Debug for RpcBlock { impl RpcBlock { pub fn block_root(&self) -> Hash256 { match self { - RpcBlock::Available(available_rpc_block) => available_rpc_block.block_root, - RpcBlock::MaybeAvailable(maybe_available_rpc_block) => { - maybe_available_rpc_block.block_root - } - } - } - - fn rpc_block_inner(&self) -> &RpcBlockInner { - match self { - RpcBlock::Available(available_rpc_block) => &available_rpc_block.block, - RpcBlock::MaybeAvailable(maybe_available_rpc_block) => &maybe_available_rpc_block.block, + RpcBlock::Available(available_block) => available_block.block_root, + RpcBlock::BlockOnly { block_root, .. } => block_root, } } pub fn as_block(&self) -> &SignedBeaconBlock { - match self.rpc_block_inner() { - RpcBlockInner::Block(signed_beacon_block) => signed_beacon_block, - RpcBlockInner::BlockAndBlobs(signed_beacon_block, _) => signed_beacon_block, - RpcBlockInner::BlockAndCustodyColumns(signed_beacon_block, _) => signed_beacon_block, + match self { + RpcBlock::FullyAvailable(available_block) => available_block.block(), + RpcBlock::BlockOnly { block, .. } => block, } } pub fn block_cloned(&self) -> Arc> { - match self.rpc_block_inner() { - RpcBlockInner::Block(signed_beacon_block) => signed_beacon_block.clone(), - RpcBlockInner::BlockAndBlobs(signed_beacon_block, _) => signed_beacon_block.clone(), - RpcBlockInner::BlockAndCustodyColumns(signed_beacon_block, _) => { - signed_beacon_block.clone() - } - } - } - - pub fn blobs(&self) -> Option<&BlobSidecarList> { - match self.rpc_block_inner() { - RpcBlockInner::Block(_) => None, - RpcBlockInner::BlockAndBlobs(_, blobs) => Some(blobs), - RpcBlockInner::BlockAndCustodyColumns(_, _) => None, + match self { + RpcBlock::FullyAvailable(available_block) => available_block.block_cloned(), + RpcBlock::BlockOnly { block, .. } => block.clone(), } } - pub fn custody_columns(&self) -> Option<&CustodyDataColumnList> { - match self.rpc_block_inner() { - RpcBlockInner::Block(_) => None, - RpcBlockInner::BlockAndBlobs(_, _) => None, - RpcBlockInner::BlockAndCustodyColumns(_, data_columns) => Some(data_columns), + pub fn block_data(&self) -> Option<&AvailableBlockData> { + match self { + RpcBlock::FullyAvailable(available_block) => Some(available_block.data()), + RpcBlock::BlockOnly { .. } => None, } } } -/// Note: This variant is intentionally private because we want to safely construct the -/// internal variants after applying consistency checks to ensure that the block and blobs -/// are consistent with respect to each other. -#[derive(Debug, Clone, Educe)] -#[educe(Hash(bound(E: EthSpec)))] -enum RpcBlockInner { - /// Single block lookup response. This should potentially hit the data availability cache. - Block(Arc>), - /// This variant is used with parent lookups and by-range responses. It should have all blobs - /// ordered, all block roots matching, and the correct number of blobs for this block. - BlockAndBlobs(Arc>, BlobSidecarList), - /// This variant is used with parent lookups and by-range responses. It should have all - /// requested data columns, all block roots matching for this block. - BlockAndCustodyColumns(Arc>, CustodyDataColumnList), -} -impl RpcBlockInner { - /// Constructs a new `BlockAndBlobs` variant after making consistency - /// checks between the provided block and blobs. This struct makes no - /// guarantees about whether blobs should be present, only that they are - /// consistent with the block. An empty list passed in for `blobs` is - /// viewed the same as `None` passed in. - fn new_block_and_blobs( - block: Arc>, - blobs: Option>, - ) -> Result { - // Treat empty blob lists as if they are missing. - let blobs = blobs.filter(|b| !b.is_empty()); - if let (Some(blobs), Ok(block_commitments)) = ( - blobs.as_ref(), - block.message().body().blob_kzg_commitments(), - ) { - if blobs.len() != block_commitments.len() { - return Err(AvailabilityCheckError::MissingBlobs); - } - for (blob, &block_commitment) in blobs.iter().zip(block_commitments.iter()) { - let blob_commitment = blob.kzg_commitment; - if blob_commitment != block_commitment { - return Err(AvailabilityCheckError::KzgCommitmentMismatch { - block_commitment, - blob_commitment, - }); - } - } - } - - let blobs = if let Some(blobs) = blobs { - blobs - } else { - RuntimeVariableList::new(vec![], E::max_blob_commitments_per_block())? - }; - - Ok(Self::BlockAndBlobs(block, blobs)) - } - - /// Constructs a new `BlockAndCustodyColumns` variant after making consistency - /// checks between the provided block and columns. This struct makes no - /// guarantees about whether columns should be present, only that they are - /// consistent with the block. An empty list passed in for `custody_columns` is - /// viewed the same as `None` passed in. - fn new_block_and_columns( - block: Arc>, - custody_columns: Option>>, - ) -> Result { - let columns = if let Some(custody_columns) = custody_columns { - if block.num_expected_blobs() > 0 && custody_columns.is_empty() { - // The number of required custody columns is out of scope here. - return Err(AvailabilityCheckError::MissingCustodyColumns); - } - // Treat empty data column lists as if they are missing. - if !custody_columns.is_empty() { - VariableList::new(custody_columns)? - } else { - VariableList::new(vec![])? - } - } else { - VariableList::new(vec![])? - }; - - Ok(RpcBlockInner::BlockAndCustodyColumns(block, columns)) - } +impl RpcBlock { - /// Constructs a new `Block` variant. pub fn new( block: Arc>, - blobs: Option>, - columns: Option>>, + block_data: Option>, ) -> Result { - match block.fork_name_unchecked() { - ForkName::Base | ForkName::Altair | ForkName::Bellatrix | ForkName::Capella => { - Ok(RpcBlockInner::Block(block)) - } - ForkName::Deneb | ForkName::Electra => Self::new_block_and_blobs(block, blobs), - ForkName::Fulu | ForkName::Gloas => Self::new_block_and_columns(block, columns), + match block_data { + Some(block_data) => Ok(RpcBlock::FullyAvailable(AvailableBlock::new(block, block_data)?)), + None => Ok(RpcBlock::BlockOnly { block, block_root: block.canonical_root() }), } } -} - -impl RpcBlock { - /// Constructs an `RpcBlock::MaybeAvailable`. Consistency checks for this variant must be made - /// before indicating that the block is fully available. An empty list or `None` passed in for - /// `blobs` or `columns` could mean that node may have not received blobs for this column yet. - pub fn new_maybe_available( - block_root: Option, - block: Arc>, - blobs: Option>, - columns: Option>>, - ) -> Result { - let block_root = block_root.unwrap_or_else(|| get_block_root(&block)); - let rpc_block_inner = RpcBlockInner::new(block, blobs, columns)?; - - Ok(RpcBlock::MaybeAvailable(MaybeAvailableRpcBlock { - block_root, - block: rpc_block_inner, - })) - } - - /// Constructs an `RpcBlock::Available` variant. All consistency checks - /// for the `RpcBlock` should be made before calling this function. Successfully constructing - /// an `Available` variant indicates that the block is fully available from the nodes perspective. - /// An empty list or `None` passed in for `blobs` or `columns` means that there - /// are no blobs for the given block. - pub fn new_available( - block_root: Option, - block: Arc>, - blobs: Option>, - columns: Option>>, - ) -> Result { - let block_root = block_root.unwrap_or_else(|| get_block_root(&block)); - let rpc_block_inner = RpcBlockInner::new(block, blobs, columns)?; - - Ok(RpcBlock::Available(AvailableRpcBlock { - block_root, - block: rpc_block_inner, - })) - } #[allow(clippy::type_complexity)] pub fn deconstruct( @@ -294,26 +91,21 @@ impl RpcBlock { ) -> ( Hash256, Arc>, - Option>, - Option>, + Option>, ) { - let block_root = self.block_root(); - match self.rpc_block_inner() { - RpcBlockInner::Block(block) => (block_root, block.clone(), None, None), - RpcBlockInner::BlockAndBlobs(block, blobs) => { - (block_root, block.clone(), Some(blobs.clone()), None) - } - RpcBlockInner::BlockAndCustodyColumns(block, data_columns) => { - (block_root, block.clone(), None, Some(data_columns.clone())) - } + match self { + RpcBlock::FullyAvailable(available_block) => available_block.deconstruct(), + RpcBlock::BlockOnly { block, block_root } => (block_root, block, None), } } + pub fn n_blobs(&self) -> usize { match self.rpc_block_inner() { RpcBlockInner::Block(_) | RpcBlockInner::BlockAndCustodyColumns(_, _) => 0, RpcBlockInner::BlockAndBlobs(_, blobs) => blobs.len(), } } + pub fn n_data_columns(&self) -> usize { match self.rpc_block_inner() { RpcBlockInner::Block(_) | RpcBlockInner::BlockAndBlobs(_, _) => 0, diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 45e826a4356..a391340e32d 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -875,6 +875,20 @@ impl AvailableBlock { } } + pub fn new( + block: Arc>, + block_data: AvailableBlockData, + ) -> Result { + // check blob lengths match + // POSSIBLY - verify kzg - but probably not + // but this variant *should* ensure the data IS available + Ok(Self { + block_root: block.canonical_root(), + block, + blob_data: block_data, + }) + } + pub fn block(&self) -> &SignedBeaconBlock { &self.block } From 95f6971d86fba3687481450099aa321c34a9c50d Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Thu, 4 Dec 2025 15:48:16 -0300 Subject: [PATCH 12/24] Updates --- beacon_node/beacon_chain/src/beacon_chain.rs | 34 +- .../beacon_chain/src/block_verification.rs | 9 +- .../src/block_verification_types.rs | 68 ++-- .../src/data_availability_checker.rs | 309 +++++++----------- .../beacon_chain/src/historical_blocks.rs | 36 +- beacon_node/beacon_chain/src/test_utils.rs | 34 +- .../network_beacon_processor/sync_methods.rs | 11 +- .../src/sync/block_sidecar_coupling.rs | 63 ++-- .../network/src/sync/network_context.rs | 4 +- beacon_node/network/src/sync/tests/lookups.rs | 11 +- beacon_node/network/src/sync/tests/range.rs | 20 +- 11 files changed, 294 insertions(+), 305 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index adc400b1c17..8a1fa3d1606 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -3976,22 +3976,24 @@ impl BeaconChain { // See https://github.com/sigp/lighthouse/issues/2028 let (_, signed_block, block_data) = signed_block.deconstruct(); - match self.get_blobs_or_columns_store_op(block_root, signed_block.slot(), block_data) { - Ok(Some(blobs_or_columns_store_op)) => { - ops.push(blobs_or_columns_store_op); - } - Ok(None) => {} - Err(e) => { - error!( - msg = "Restoring fork choice from disk", - error = &e, - ?block_root, - "Failed to store data columns into the database" - ); - return Err(self - .handle_import_block_db_write_error(fork_choice) - .err() - .unwrap_or(BlockError::InternalError(e))); + if let Some(block_data) = block_data { + match self.get_blobs_or_columns_store_op(block_root, signed_block.slot(), block_data) { + Ok(Some(blobs_or_columns_store_op)) => { + ops.push(blobs_or_columns_store_op); + } + Ok(None) => {} + Err(e) => { + error!( + msg = "Restoring fork choice from disk", + error = &e, + ?block_root, + "Failed to store data columns into the database" + ); + return Err(self + .handle_import_block_db_write_error(fork_choice) + .err() + .unwrap_or(BlockError::InternalError(e))); + } } } diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 374f1e2b360..574f0b53396 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -646,7 +646,14 @@ pub fn signature_verify_chain_segment( )?; // unzip chain segment and verify kzg in bulk - let (roots, blocks): (Vec<_>, Vec<_>) = chain_segment.into_iter().unzip(); + let (roots, blocks): (Vec<_>, Vec<_>) = chain_segment + .into_iter() + .filter_map(|(block_root, block)| match block { + RpcBlock::FullyAvailable(available_block) => Some((block_root, available_block)), + RpcBlock::BlockOnly { .. } => None, + }) + .unzip(); + let maybe_available_blocks = chain .data_availability_checker .verify_kzg_for_rpc_blocks(blocks)?; diff --git a/beacon_node/beacon_chain/src/block_verification_types.rs b/beacon_node/beacon_chain/src/block_verification_types.rs index 3fc27eb41bd..621a24cdce4 100644 --- a/beacon_node/beacon_chain/src/block_verification_types.rs +++ b/beacon_node/beacon_chain/src/block_verification_types.rs @@ -1,16 +1,16 @@ +use crate::PayloadVerificationOutcome; use crate::data_availability_checker::AvailabilityCheckError; -pub use crate::data_availability_checker::{AvailableBlock, AvailableBlockData, MaybeAvailableBlock}; -use crate::data_column_verification::{CustodyDataColumn, CustodyDataColumnList}; -use crate::{PayloadVerificationOutcome, get_block_root}; +pub use crate::data_availability_checker::{ + AvailableBlock, AvailableBlockData, MaybeAvailableBlock, +}; use educe::Educe; -use ssz_types::VariableList; use state_processing::ConsensusContext; use std::fmt::{Debug, Formatter}; use std::sync::Arc; use types::blob_sidecar::BlobIdentifier; use types::{ - BeaconBlockRef, BeaconState, BlindedPayload, BlobSidecarList, Epoch, EthSpec, ForkName, - Hash256, RuntimeVariableList, SignedBeaconBlock, SignedBeaconBlockHeader, Slot, + BeaconBlockRef, BeaconState, BlindedPayload, ChainSpec, Epoch, EthSpec, Hash256, + SignedBeaconBlock, SignedBeaconBlockHeader, Slot, }; /// A block that has been received over RPC. It has 2 internal variants: @@ -45,8 +45,8 @@ impl Debug for RpcBlock { impl RpcBlock { pub fn block_root(&self) -> Hash256 { match self { - RpcBlock::Available(available_block) => available_block.block_root, - RpcBlock::BlockOnly { block_root, .. } => block_root, + RpcBlock::FullyAvailable(available_block) => available_block.block_root(), + RpcBlock::BlockOnly { block_root, .. } => *block_root, } } @@ -72,16 +72,20 @@ impl RpcBlock { } } - impl RpcBlock { - pub fn new( block: Arc>, block_data: Option>, + spec: Arc, ) -> Result { match block_data { - Some(block_data) => Ok(RpcBlock::FullyAvailable(AvailableBlock::new(block, block_data)?)), - None => Ok(RpcBlock::BlockOnly { block, block_root: block.canonical_root() }), + Some(block_data) => Ok(RpcBlock::FullyAvailable(AvailableBlock::new( + block, block_data, spec, + )?)), + None => Ok(RpcBlock::BlockOnly { + block_root: block.canonical_root(), + block, + }), } } @@ -100,16 +104,24 @@ impl RpcBlock { } pub fn n_blobs(&self) -> usize { - match self.rpc_block_inner() { - RpcBlockInner::Block(_) | RpcBlockInner::BlockAndCustodyColumns(_, _) => 0, - RpcBlockInner::BlockAndBlobs(_, blobs) => blobs.len(), + if let Some(block_data) = self.block_data() { + match block_data { + AvailableBlockData::NoData | AvailableBlockData::DataColumns(_) => 0, + AvailableBlockData::Blobs(blobs) => blobs.len(), + } + } else { + 0 } } pub fn n_data_columns(&self) -> usize { - match self.rpc_block_inner() { - RpcBlockInner::Block(_) | RpcBlockInner::BlockAndBlobs(_, _) => 0, - RpcBlockInner::BlockAndCustodyColumns(_, data_columns) => data_columns.len(), + if let Some(block_data) = self.block_data() { + match block_data { + AvailableBlockData::NoData | AvailableBlockData::Blobs(_) => 0, + AvailableBlockData::DataColumns(columns) => columns.len(), + } + } else { + 0 } } } @@ -411,17 +423,21 @@ impl AsBlock for RpcBlock { self.as_block().message() } fn as_block(&self) -> &SignedBeaconBlock { - match &self.rpc_block_inner() { - RpcBlockInner::Block(block) => block, - RpcBlockInner::BlockAndBlobs(block, _) => block, - RpcBlockInner::BlockAndCustodyColumns(block, _) => block, + match self { + Self::BlockOnly { + block, + block_root: _, + } => &block, + Self::FullyAvailable(available_block) => available_block.block(), } } fn block_cloned(&self) -> Arc> { - match &self.rpc_block_inner() { - RpcBlockInner::Block(block) => block.clone(), - RpcBlockInner::BlockAndBlobs(block, _) => block.clone(), - RpcBlockInner::BlockAndCustodyColumns(block, _) => block.clone(), + match self { + RpcBlock::FullyAvailable(available_block) => available_block.block_cloned(), + RpcBlock::BlockOnly { + block, + block_root: _, + } => block.clone(), } } fn canonical_root(&self) -> Hash256 { diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index a391340e32d..ae139fc461c 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -2,8 +2,7 @@ use crate::blob_verification::{ GossipVerifiedBlob, KzgVerifiedBlob, KzgVerifiedBlobList, verify_kzg_for_blob_list, }; use crate::block_verification_types::{ - AvailabilityPendingExecutedBlock, AvailableExecutedBlock, AvailableRpcBlock, - MaybeAvailableRpcBlock, RpcBlock, + AvailabilityPendingExecutedBlock, AvailableExecutedBlock, RpcBlock, }; use crate::data_availability_checker::overflow_lru_cache::{ DataAvailabilityCheckerInner, ReconstructColumnsDecision, @@ -11,6 +10,7 @@ use crate::data_availability_checker::overflow_lru_cache::{ use crate::{ BeaconChain, BeaconChainTypes, BeaconStore, BlockProcessStatus, CustodyContext, metrics, }; +use educe::Educe; use kzg::Kzg; use slot_clock::SlotClock; use std::fmt; @@ -32,8 +32,8 @@ mod state_lru_cache; use crate::data_availability_checker::error::Error; use crate::data_column_verification::{ - CustodyDataColumn, GossipVerifiedDataColumn, KzgVerifiedCustodyDataColumn, - KzgVerifiedDataColumn, verify_kzg_for_data_column_list, + GossipVerifiedDataColumn, KzgVerifiedCustodyDataColumn, KzgVerifiedDataColumn, + verify_kzg_for_data_column_list, }; use crate::metrics::{ KZG_DATA_COLUMN_RECONSTRUCTION_ATTEMPTS, KZG_DATA_COLUMN_RECONSTRUCTION_FAILURES, @@ -367,119 +367,41 @@ impl DataAvailabilityChecker { .remove_pre_execution_block(block_root); } - pub fn verify_kzg_for_maybe_available_rpc_block( + pub fn verify_kzg_for_fully_available_rpc_block( &self, - block: MaybeAvailableRpcBlock, + available_block: AvailableBlock, ) -> Result, AvailabilityCheckError> { - let (block_root, block, blobs, data_columns) = block.deconstruct(); - if self.blobs_required_for_block(&block) { - return if let Some(blob_list) = blobs - && blob_list.len() == block.num_expected_blobs() - { - verify_kzg_for_blob_list(blob_list.iter(), &self.kzg) - .map_err(AvailabilityCheckError::InvalidBlobs)?; - Ok(MaybeAvailableBlock::Available(AvailableBlock { - block_root, - block, - blob_data: AvailableBlockData::Blobs(blob_list), - blobs_available_timestamp: None, - spec: self.spec.clone(), - })) - } else { - Ok(MaybeAvailableBlock::AvailabilityPending { block_root, block }) - }; - } - if self.data_columns_required_for_block(&block) { - return if let Some(data_column_list) = data_columns.as_ref() - && data_column_list.len() == T::EthSpec::number_of_columns() - { - verify_kzg_for_data_column_list( - data_column_list - .iter() - .map(|custody_column| custody_column.as_data_column()), - &self.kzg, - ) - .map_err(AvailabilityCheckError::InvalidColumn)?; - Ok(MaybeAvailableBlock::Available(AvailableBlock { - block_root, - block, - blob_data: AvailableBlockData::DataColumns( - data_column_list - .into_iter() - .map(|d| d.clone_arc()) - .collect(), - ), - blobs_available_timestamp: None, - spec: self.spec.clone(), - })) - } else { - Ok(MaybeAvailableBlock::AvailabilityPending { block_root, block }) - }; - } - - Ok(MaybeAvailableBlock::Available(AvailableBlock { - block_root, - block, - blob_data: AvailableBlockData::NoData, - blobs_available_timestamp: None, - spec: self.spec.clone(), - })) - } - - pub fn verify_kzg_for_available_rpc_block( - &self, - block: AvailableRpcBlock, - ) -> Result, AvailabilityCheckError> { - let (block_root, block, blobs, data_columns) = block.deconstruct(); - - if self.blobs_required_for_block(&block) { - return if let Some(blob_list) = blobs { - verify_kzg_for_blob_list(blob_list.iter(), &self.kzg) - .map_err(AvailabilityCheckError::InvalidBlobs)?; - Ok(MaybeAvailableBlock::Available(AvailableBlock { - block_root, - block, - blob_data: AvailableBlockData::Blobs(blob_list), - blobs_available_timestamp: None, - spec: self.spec.clone(), - })) - } else { - return Err(AvailabilityCheckError::MissingBlobs); - }; + match &available_block.blob_data { + AvailableBlockData::NoData => { + if self.blobs_required_for_block(&available_block.block) + || self.data_columns_required_for_block(&available_block.block) + { + if available_block.block.fork_name_unchecked().fulu_enabled() { + return Err(AvailabilityCheckError::MissingCustodyColumns); + } else { + return Err(AvailabilityCheckError::MissingBlobs); + } + } + } + AvailableBlockData::Blobs(blobs) => { + // TODO(rpc-block) should we return an error if blobs not required? + // the blobs variant should only exist for a block that requires blobs + if self.blobs_required_for_block(&available_block.block) { + verify_kzg_for_blob_list(blobs.iter(), &self.kzg) + .map_err(AvailabilityCheckError::InvalidBlobs)?; + } + } + AvailableBlockData::DataColumns(data_columns) => { + // TODO(rpc-block) should we return an error if columns not required? + // the columns variant should only exist for a block that requires columns + if self.data_columns_required_for_block(&available_block.block) { + verify_kzg_for_data_column_list(data_columns.iter(), &self.kzg) + .map_err(AvailabilityCheckError::InvalidColumn)?; + } + } } - if self.data_columns_required_for_block(&block) { - return if let Some(data_column_list) = data_columns { - verify_kzg_for_data_column_list( - data_column_list - .iter() - .map(|custody_column| custody_column.as_data_column()), - &self.kzg, - ) - .map_err(AvailabilityCheckError::InvalidColumn)?; - Ok(MaybeAvailableBlock::Available(AvailableBlock { - block_root, - block, - blob_data: AvailableBlockData::DataColumns( - data_column_list - .into_iter() - .map(|d| d.clone_arc()) - .collect(), - ), - blobs_available_timestamp: None, - spec: self.spec.clone(), - })) - } else { - return Err(AvailabilityCheckError::MissingCustodyColumns); - }; - } - Ok(MaybeAvailableBlock::Available(AvailableBlock { - block_root, - block, - blob_data: AvailableBlockData::NoData, - blobs_available_timestamp: None, - spec: self.spec.clone(), - })) + return Ok(MaybeAvailableBlock::Available(available_block)); } /// Verifies kzg commitments for an RpcBlock, returns a `MaybeAvailableBlock` that may @@ -492,11 +414,11 @@ impl DataAvailabilityChecker { block: RpcBlock, ) -> Result, AvailabilityCheckError> { match block { - RpcBlock::Available(available_rpc_block) => { - self.verify_kzg_for_available_rpc_block(available_rpc_block) + RpcBlock::FullyAvailable(available_rpc_block) => { + self.verify_kzg_for_fully_available_rpc_block(available_rpc_block) } - RpcBlock::MaybeAvailable(maybe_available_rpc_block) => { - self.verify_kzg_for_maybe_available_rpc_block(maybe_available_rpc_block) + RpcBlock::BlockOnly { block_root, block } => { + Ok(MaybeAvailableBlock::AvailabilityPending { block_root, block }) } } } @@ -510,14 +432,14 @@ impl DataAvailabilityChecker { #[instrument(skip_all)] pub fn verify_kzg_for_rpc_blocks( &self, - blocks: Vec>, + available_blocks: Vec>, ) -> Result>, AvailabilityCheckError> { - let mut results = Vec::with_capacity(blocks.len()); - let all_blobs = blocks + let mut results = Vec::with_capacity(available_blocks.len()); + let all_blobs = available_blocks .iter() - .filter(|block| self.blobs_required_for_block(block.as_block())) + .filter(|available_block| self.blobs_required_for_block(&available_block.block)) // this clone is cheap as it's cloning an Arc - .filter_map(|block| block.blobs().cloned()) + .filter_map(|available_block| available_block.blob_data.blobs()) .flatten() .collect::>(); @@ -527,13 +449,12 @@ impl DataAvailabilityChecker { .map_err(AvailabilityCheckError::InvalidBlobs)?; } - let all_data_columns = blocks + let all_data_columns = available_blocks .iter() - .filter(|block| self.data_columns_required_for_block(block.as_block())) + .filter(|available_block| self.data_columns_required_for_block(&available_block.block)) // this clone is cheap as it's cloning an Arc - .filter_map(|block| block.custody_columns().cloned()) + .filter_map(|available_block| available_block.blob_data.data_columns()) .flatten() - .map(CustodyDataColumn::into_inner) .collect::>(); // verify kzg for all data columns at once @@ -543,60 +464,14 @@ impl DataAvailabilityChecker { .map_err(AvailabilityCheckError::InvalidColumn)?; } - for block in blocks { - let is_available = match block { - RpcBlock::Available(_) => true, - RpcBlock::MaybeAvailable(_) => false, - }; - - let (block_root, block, blobs, data_columns) = block.deconstruct(); - - let maybe_available_block = if self.blobs_required_for_block(&block) { - if let Some(blobs) = blobs - && blobs.len() == block.num_expected_blobs() - { - MaybeAvailableBlock::Available(AvailableBlock { - block_root, - block, - blob_data: AvailableBlockData::Blobs(blobs), - blobs_available_timestamp: None, - spec: self.spec.clone(), - }) - } else { - if is_available { - return Err(AvailabilityCheckError::MissingBlobs); - } - MaybeAvailableBlock::AvailabilityPending { block_root, block } - } - } else if self.data_columns_required_for_block(&block) { - if let Some(data_columns) = data_columns - { - MaybeAvailableBlock::Available(AvailableBlock { - block_root, - block, - blob_data: AvailableBlockData::DataColumns( - data_columns.into_iter().map(|d| d.into_inner()).collect(), - ), - blobs_available_timestamp: None, - spec: self.spec.clone(), - }) - } else { - if is_available { - return Err(AvailabilityCheckError::MissingCustodyColumns); - } - MaybeAvailableBlock::AvailabilityPending { block_root, block } - } - } else { - MaybeAvailableBlock::Available(AvailableBlock { - block_root, - block, - blob_data: AvailableBlockData::NoData, - blobs_available_timestamp: None, - spec: self.spec.clone(), - }) - }; + for available_block in available_blocks { + if self.blobs_required_for_block(&available_block.block) { + return Err(AvailabilityCheckError::MissingBlobs); + } else if self.data_columns_required_for_block(&available_block.block) { + return Err(AvailabilityCheckError::MissingCustodyColumns); + } - results.push(maybe_available_block); + results.push(MaybeAvailableBlock::Available(available_block.clone())); } Ok(results) @@ -838,7 +713,7 @@ async fn availability_cache_maintenance_service( } } -#[derive(Debug)] +#[derive(Debug, Clone)] pub enum AvailableBlockData { /// Block is pre-Deneb or has zero blobs NoData, @@ -848,14 +723,49 @@ pub enum AvailableBlockData { DataColumns(DataColumnSidecarList), } +impl AvailableBlockData { + pub fn new( + blobs: Option>, + columns: Option>, + ) -> Self { + if let Some(blobs) = blobs { + Self::Blobs(blobs) + } else if let Some(columns) = columns { + Self::DataColumns(columns) + } else { + Self::NoData + } + } + + pub fn blobs(&self) -> Option> { + match self { + AvailableBlockData::NoData => None, + AvailableBlockData::Blobs(blobs) => Some(blobs.clone()), + AvailableBlockData::DataColumns(_) => None, + } + } + + pub fn data_columns(&self) -> Option> { + match self { + AvailableBlockData::NoData => None, + AvailableBlockData::Blobs(_) => None, + AvailableBlockData::DataColumns(data_columns) => Some(data_columns.clone()), + } + } +} + /// A fully available block that is ready to be imported into fork choice. -#[derive(Debug)] +#[derive(Debug, Clone, Educe)] +#[educe(Hash(bound(E: EthSpec)))] pub struct AvailableBlock { block_root: Hash256, block: Arc>, + #[educe(Hash(ignore))] blob_data: AvailableBlockData, + #[educe(Hash(ignore))] /// Timestamp at which this block first became available (UNIX timestamp, time since 1970). blobs_available_timestamp: Option, + #[educe(Hash(ignore))] pub spec: Arc, } @@ -878,6 +788,7 @@ impl AvailableBlock { pub fn new( block: Arc>, block_data: AvailableBlockData, + spec: Arc, ) -> Result { // check blob lengths match // POSSIBLY - verify kzg - but probably not @@ -886,6 +797,8 @@ impl AvailableBlock { block_root: block.canonical_root(), block, blob_data: block_data, + blobs_available_timestamp: None, + spec, }) } @@ -904,6 +817,10 @@ impl AvailableBlock { &self.blob_data } + pub fn block_root(&self) -> Hash256 { + self.block_root + } + pub fn has_blobs(&self) -> bool { match self.blob_data { AvailableBlockData::NoData => false, @@ -913,14 +830,20 @@ impl AvailableBlock { } #[allow(clippy::type_complexity)] - pub fn deconstruct(self) -> (Hash256, Arc>, AvailableBlockData) { + pub fn deconstruct( + self, + ) -> ( + Hash256, + Arc>, + Option>, + ) { let AvailableBlock { block_root, block, blob_data, .. } = self; - (block_root, block, blob_data) + (block_root, block, Some(blob_data)) } /// Only used for testing @@ -968,6 +891,7 @@ mod test { use super::*; use crate::CustodyContext; use crate::custody_context::NodeCustodyType; + use crate::data_column_verification::CustodyDataColumn; use crate::test_utils::{ EphemeralHarnessType, NumBlobs, generate_data_column_indices_rand_order, generate_rand_block_and_data_columns, get_kzg, @@ -1160,9 +1084,6 @@ mod test { let custody_columns = if index == 0 { // 128 valid data columns in the first block data_columns - .into_iter() - .map(CustodyDataColumn::from_asserted_custody) - .collect::>() } else { // invalid data columns in the second block data_columns @@ -1173,17 +1094,29 @@ mod test { ..d.as_ref().clone() }; CustodyDataColumn::from_asserted_custody(Arc::new(invalid_sidecar)) + .as_data_column() + .clone() }) .collect::>() }; - RpcBlock::new_available(None, Arc::new(block), None, Some(custody_columns)) + let block_data = AvailableBlockData::new(None, Some(custody_columns)); + + RpcBlock::new(Arc::new(block), Some(block_data), spec) .expect("should create RPC block with custody columns") }) .collect::>(); + let available_blocks = blocks_with_columns + .iter() + .filter_map(|block| match block { + RpcBlock::FullyAvailable(available_block) => Some(available_block.clone()), + RpcBlock::BlockOnly { block, block_root } => None, + }) + .collect::>(); + // WHEN verifying all blocks together (totalling 256 data columns) - let verification_result = da_checker.verify_kzg_for_rpc_blocks(blocks_with_columns); + let verification_result = da_checker.verify_kzg_for_rpc_blocks(available_blocks); // THEN batch block verification should fail due to 128 invalid columns in the second block verification_result.expect_err("should have failed to verify blocks"); diff --git a/beacon_node/beacon_chain/src/historical_blocks.rs b/beacon_node/beacon_chain/src/historical_blocks.rs index e4040eea6b0..d183b924a58 100644 --- a/beacon_node/beacon_chain/src/historical_blocks.rs +++ b/beacon_node/beacon_chain/src/historical_blocks.rs @@ -155,26 +155,26 @@ impl BeaconChain { ); } - match &block_data { - AvailableBlockData::NoData => {} - AvailableBlockData::Blobs(..) => { - new_oldest_blob_slot = Some(block.slot()); + if let Some(block_data) = block_data { + match &block_data { + AvailableBlockData::NoData => todo!(), + AvailableBlockData::Blobs(_) => new_oldest_blob_slot = Some(block.slot()), + AvailableBlockData::DataColumns(_) => { + new_oldest_data_column_slot = Some(block.slot()) + } } - AvailableBlockData::DataColumns(_) => { - new_oldest_data_column_slot = Some(block.slot()); - } - } - // Store the blobs or data columns too - if let Some(op) = self - .get_blobs_or_columns_store_op(block_root, block.slot(), block_data) - .map_err(|e| { - HistoricalBlockError::StoreError(StoreError::DBError { - message: format!("get_blobs_or_columns_store_op error {e:?}"), - }) - })? - { - blob_batch.extend(self.store.convert_to_kv_batch(vec![op])?); + // Store the blobs or data columns too + if let Some(op) = self + .get_blobs_or_columns_store_op(block_root, block.slot(), block_data) + .map_err(|e| { + HistoricalBlockError::StoreError(StoreError::DBError { + message: format!("get_blobs_or_columns_store_op error {e:?}"), + }) + })? + { + blob_batch.extend(self.store.convert_to_kv_batch(vec![op])?); + } } // Store block roots, including at all skip slots in the freezer DB. diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index b71acbc2524..a91207c1b2f 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -1,5 +1,5 @@ use crate::blob_verification::GossipVerifiedBlob; -use crate::block_verification_types::{AsBlock, RpcBlock}; +use crate::block_verification_types::{AsBlock, AvailableBlockData, RpcBlock}; use crate::custody_context::NodeCustodyType; use crate::data_column_verification::CustodyDataColumn; use crate::kzg_utils::build_data_column_sidecars; @@ -2424,8 +2424,8 @@ where .blob_kzg_commitments() .is_ok_and(|c| !c.is_empty()); if !has_blobs { - // TODO(can refactor so we dont call new_avail a bunch) - return RpcBlock::new_available(Some(block_root), block, None, None).unwrap(); + return RpcBlock::new(block, Some(AvailableBlockData::NoData), self.spec.clone()) + .unwrap(); } // Blobs are stored as data columns from Fulu (PeerDAS) @@ -2433,12 +2433,15 @@ where let columns = self.chain.get_data_columns(&block_root).unwrap().unwrap(); let custody_columns = columns .into_iter() - .map(CustodyDataColumn::from_asserted_custody) + // TODO(investigate the custody data column conversion) + // .map(CustodyDataColumn::from_asserted_custody) .collect::>(); - RpcBlock::new_available(Some(block_root), block, None, Some(custody_columns)).unwrap() + let block_data = AvailableBlockData::new(None, Some(custody_columns)); + RpcBlock::new(block, Some(block_data), self.spec.clone()).unwrap() } else { let blobs = self.chain.get_blobs(&block_root).unwrap().blobs(); - RpcBlock::new_available(Some(block_root), block, blobs, None).unwrap() + let block_data = AvailableBlockData::new(blobs, None); + RpcBlock::new(block, Some(block_data), self.spec.clone()).unwrap() } } @@ -2461,18 +2464,20 @@ where let columns = generate_data_column_sidecars_from_block(&block, &self.spec) .into_iter() .filter(|d| sampling_columns.contains(&d.index)) - .map(CustodyDataColumn::from_asserted_custody) + // TODO(investigate the custody data column conversion) + // .map(CustodyDataColumn::from_asserted_custody) .collect::>(); - // TODO(can combine these and clean it up) if is_available { - RpcBlock::new_available(Some(block_root), block, None, Some(columns))? + let block_data: AvailableBlockData = + AvailableBlockData::new(None, Some(columns)); + RpcBlock::new(block, Some(block_data), self.spec.clone())? } else { - RpcBlock::new_maybe_available(Some(block_root), block, None, Some(columns))? + RpcBlock::new(block, None, self.spec.clone())? } } else if is_available { - RpcBlock::new_available(Some(block_root), block, None, None)? + RpcBlock::new(block, Some(AvailableBlockData::NoData), self.spec.clone())? } else { - RpcBlock::new_maybe_available(Some(block_root), block, None, None)? + RpcBlock::new(block, None, self.spec.clone())? } } else { let blobs = blob_items @@ -2482,9 +2487,10 @@ where .transpose() .unwrap(); if is_available { - RpcBlock::new_available(Some(block_root), block, blobs, None)? + let block_data: AvailableBlockData = AvailableBlockData::new(blobs, None); + RpcBlock::new(block, Some(block_data), self.spec.clone())? } else { - RpcBlock::new_maybe_available(Some(block_root), block, blobs, None)? + RpcBlock::new(block, None, self.spec.clone())? } }) } diff --git a/beacon_node/network/src/network_beacon_processor/sync_methods.rs b/beacon_node/network/src/network_beacon_processor/sync_methods.rs index e49ae134fe4..ede1fce5f42 100644 --- a/beacon_node/network/src/network_beacon_processor/sync_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/sync_methods.rs @@ -722,10 +722,19 @@ impl NetworkBeaconProcessor { downloaded_blocks: Vec>, ) -> (usize, Result<(), ChainSegmentFailed>) { let total_blocks = downloaded_blocks.len(); + let available_blocks = downloaded_blocks + .iter() + .filter_map(|rpc_block| match rpc_block { + RpcBlock::FullyAvailable(available_block) => Some(available_block.clone()), + // TODO this shouldn't be possible here. the rpc block must be available. maybe log an error message just in case + RpcBlock::BlockOnly { .. } => None, + }) + .collect::>(); + let available_blocks = match self .chain .data_availability_checker - .verify_kzg_for_rpc_blocks(downloaded_blocks) + .verify_kzg_for_rpc_blocks(available_blocks) { Ok(blocks) => blocks .into_iter() diff --git a/beacon_node/network/src/sync/block_sidecar_coupling.rs b/beacon_node/network/src/sync/block_sidecar_coupling.rs index 9319eabf6c3..7cbe2e97465 100644 --- a/beacon_node/network/src/sync/block_sidecar_coupling.rs +++ b/beacon_node/network/src/sync/block_sidecar_coupling.rs @@ -1,5 +1,7 @@ use beacon_chain::{ - block_verification_types::RpcBlock, data_column_verification::CustodyDataColumn, get_block_root, + block_verification_types::{AvailableBlockData, RpcBlock}, + data_column_verification::CustodyDataColumn, + get_block_root, }; use lighthouse_network::{ PeerId, @@ -194,7 +196,7 @@ impl RangeBlockComponentsRequest { /// Returns `Some(Err(_))` if there are issues coupling blocks with their data. pub fn responses( &mut self, - spec: &ChainSpec, + spec: Arc, ) -> Option>, CouplingError>> { let Some(blocks) = self.blocks_request.to_finished() else { return None; @@ -202,9 +204,11 @@ impl RangeBlockComponentsRequest { // Increment the attempt once this function returns the response or errors match &mut self.block_data_request { - RangeBlockDataRequest::NoData => { - Some(Self::responses_with_blobs(blocks.to_vec(), vec![], spec)) - } + RangeBlockDataRequest::NoData => Some(Self::responses_with_blobs( + blocks.to_vec(), + vec![], + spec.clone(), + )), RangeBlockDataRequest::Blobs(request) => { let Some(blobs) = request.to_finished() else { return None; @@ -248,6 +252,7 @@ impl RangeBlockComponentsRequest { column_to_peer_id, expected_custody_columns, *attempt, + spec, ); if let Err(CouplingError::DataColumnPeerFailure { @@ -272,7 +277,7 @@ impl RangeBlockComponentsRequest { fn responses_with_blobs( blocks: Vec>>, blobs: Vec>>, - spec: &ChainSpec, + spec: Arc, ) -> Result>, CouplingError> { // There can't be more more blobs than blocks. i.e. sending any blob (empty // included) for a skipped slot is not permitted. @@ -315,8 +320,9 @@ impl RangeBlockComponentsRequest { .map_err(|_| { CouplingError::BlobPeerFailure("Blobs returned exceeds max length".to_string()) })?; + let block_data = AvailableBlockData::new(Some(blobs), None); responses.push( - RpcBlock::new_available(None, block, Some(blobs), None) + RpcBlock::new(block, Some(block_data), spec.clone()) .map_err(|e| CouplingError::BlobPeerFailure(format!("{e:?}")))?, ) } @@ -339,6 +345,7 @@ impl RangeBlockComponentsRequest { column_to_peer: HashMap, expects_custody_columns: &[ColumnIndex], attempt: usize, + spec: Arc, ) -> Result>, CouplingError> { // Group data columns by block_root and index let mut data_columns_by_block = @@ -415,11 +422,13 @@ impl RangeBlockComponentsRequest { ); } - RpcBlock::new_available(Some(block_root), block, None, Some(custody_columns)) + let block_data = AvailableBlockData::new(None, Some(custody_columns.iter().map(|c| c.as_data_column().clone()).collect::>())); + + RpcBlock::new(block, Some(block_data), spec.clone()) .map_err(|e| CouplingError::InternalError(format!("{:?}", e)))? } else { // Block has no data, expects zero columns - RpcBlock::new_available(Some(block_root), block, None, None) + RpcBlock::new(block, Some(AvailableBlockData::NoData), spec.clone()) .map_err(|e| CouplingError::InternalError(format!("{:?}", e)))? }); } @@ -513,8 +522,8 @@ mod tests { } fn is_finished(info: &mut RangeBlockComponentsRequest) -> bool { - let spec = test_spec::(); - info.responses(&spec).is_some() + let spec = Arc::new(test_spec::()); + info.responses(spec).is_some() } #[test] @@ -535,8 +544,10 @@ mod tests { // Send blocks and complete terminate response info.add_blocks(blocks_req_id, blocks).unwrap(); + let spec = Arc::new(test_spec::()); + // Assert response is finished and RpcBlocks can be constructed - info.responses(&test_spec::()).unwrap().unwrap(); + info.responses(spec).unwrap().unwrap(); } #[test] @@ -566,15 +577,17 @@ mod tests { // Expect no blobs returned info.add_blobs(blobs_req_id, vec![]).unwrap(); + let spec = Arc::new(test_spec::()); + // Assert response is finished and RpcBlocks can be constructed, even if blobs weren't returned. // This makes sure we don't expect blobs here when they have expired. Checking this logic should // be hendled elsewhere. - info.responses(&test_spec::()).unwrap().unwrap(); + info.responses(spec).unwrap().unwrap(); } #[test] fn rpc_block_with_custody_columns() { - let spec = test_spec::(); + let spec = Arc::new(test_spec::()); let expects_custody_columns = vec![1, 2, 3, 4]; let mut rng = XorShiftRng::from_seed([42; 16]); let blocks = (0..4) @@ -639,12 +652,12 @@ mod tests { } // All completed construct response - info.responses(&spec).unwrap().unwrap(); + info.responses(spec).unwrap().unwrap(); } #[test] fn rpc_block_with_custody_columns_batched() { - let spec = test_spec::(); + let spec = Arc::new(test_spec::()); let batched_column_requests = [vec![1_u64, 2], vec![3, 4]]; let expects_custody_columns = batched_column_requests .iter() @@ -724,13 +737,13 @@ mod tests { } // All completed construct response - info.responses(&spec).unwrap().unwrap(); + info.responses(spec).unwrap().unwrap(); } #[test] fn missing_custody_columns_from_faulty_peers() { // GIVEN: A request expecting custody columns from multiple peers - let spec = test_spec::(); + let spec = Arc::new(test_spec::()); let expected_custody_columns = vec![1, 2, 3, 4]; let mut rng = XorShiftRng::from_seed([42; 16]); let blocks = (0..2) @@ -793,7 +806,7 @@ mod tests { } // WHEN: Attempting to construct RPC blocks - let result = info.responses(&spec).unwrap(); + let result = info.responses(spec).unwrap(); // THEN: Should fail with PeerFailure identifying the faulty peers assert!(result.is_err()); @@ -816,7 +829,7 @@ mod tests { #[test] fn retry_logic_after_peer_failures() { // GIVEN: A request expecting custody columns where some peers initially fail - let spec = test_spec::(); + let spec = Arc::new(test_spec::()); let expected_custody_columns = vec![1, 2]; let mut rng = XorShiftRng::from_seed([42; 16]); let blocks = (0..2) @@ -875,7 +888,7 @@ mod tests { info.add_custody_columns(*req2, vec![]).unwrap(); // WHEN: First attempt to get responses fails - let result = info.responses(&spec).unwrap(); + let result = info.responses(spec.clone()).unwrap(); assert!(result.is_err()); // AND: We retry with a new peer for the failed column @@ -898,7 +911,7 @@ mod tests { .unwrap(); // WHEN: Attempting to get responses again - let result = info.responses(&spec).unwrap(); + let result = info.responses(spec).unwrap(); // THEN: Should succeed with complete RPC blocks assert!(result.is_ok()); @@ -909,7 +922,7 @@ mod tests { #[test] fn max_retries_exceeded_behavior() { // GIVEN: A request where peers consistently fail to provide required columns - let spec = test_spec::(); + let spec = Arc::new(test_spec::()); let expected_custody_columns = vec![1, 2]; let mut rng = XorShiftRng::from_seed([42; 16]); let blocks = (0..1) @@ -969,7 +982,7 @@ mod tests { // WHEN: Multiple retry attempts are made (up to max retries) for _ in 0..MAX_COLUMN_RETRIES { - let result = info.responses(&spec).unwrap(); + let result = info.responses(spec.clone()).unwrap(); assert!(result.is_err()); if let Err(super::CouplingError::DataColumnPeerFailure { @@ -982,7 +995,7 @@ mod tests { } // AND: One final attempt after exceeding max retries - let result = info.responses(&spec).unwrap(); + let result = info.responses(spec).unwrap(); // THEN: Should fail with exceeded_retries = true assert!(result.is_err()); diff --git a/beacon_node/network/src/sync/network_context.rs b/beacon_node/network/src/sync/network_context.rs index 1e932588dfe..ff7b476a96f 100644 --- a/beacon_node/network/src/sync/network_context.rs +++ b/beacon_node/network/src/sync/network_context.rs @@ -777,7 +777,7 @@ impl SyncNetworkContext { } let range_req = entry.get_mut(); - if let Some(blocks_result) = range_req.responses(&self.chain.spec) { + if let Some(blocks_result) = range_req.responses(self.chain.spec.clone()) { if let Err(CouplingError::DataColumnPeerFailure { error, faulty_peers: _, @@ -1606,7 +1606,7 @@ impl SyncNetworkContext { .beacon_processor_if_enabled() .ok_or(SendErrorProcessor::ProcessorNotAvailable)?; - let block = RpcBlock::new_maybe_available(Some(block_root), block, None, None) + let block = RpcBlock::new(block, None, self.chain.spec.clone()) .map_err(|_| SendErrorProcessor::SendError)?; debug!(block = ?block_root, id, "Sending block for processing"); diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index 2b51eac6a1a..3d3a9f71c0c 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -2283,18 +2283,9 @@ mod deneb_only { fn parent_block_unknown_parent(mut self) -> Self { self.rig.log("parent_block_unknown_parent"); let block = self.unknown_parent_block.take().unwrap(); - let max_len = self.rig.spec.max_blobs_per_block(block.epoch()) as usize; // Now this block is the one we expect requests from self.block = block.clone(); - let block = RpcBlock::new_maybe_available( - Some(block.canonical_root()), - block, - self.unknown_parent_blobs - .take() - .map(|vec| RuntimeVariableList::new(vec, max_len).unwrap()), - None, - ) - .unwrap(); + let block = RpcBlock::new(block, None, self.rig.spec.clone()).unwrap(); self.rig.parent_block_processed( self.block_root, BlockProcessingResult::Err(BlockError::ParentUnknown { diff --git a/beacon_node/network/src/sync/tests/range.rs b/beacon_node/network/src/sync/tests/range.rs index 7a30c25f5d1..7098374a028 100644 --- a/beacon_node/network/src/sync/tests/range.rs +++ b/beacon_node/network/src/sync/tests/range.rs @@ -5,6 +5,7 @@ use crate::sync::SyncMessage; use crate::sync::manager::SLOT_IMPORT_TOLERANCE; use crate::sync::network_context::RangeRequestId; use crate::sync::range_sync::RangeSyncType; +use beacon_chain::block_verification_types::AvailableBlockData; use beacon_chain::data_column_verification::CustodyDataColumn; use beacon_chain::test_utils::{AttestationStrategy, BlockStrategy}; use beacon_chain::{EngineState, NotifyExecutionLayer, block_verification_types::RpcBlock}; @@ -427,7 +428,7 @@ impl TestRig { .chain .process_block( block_root, - build_rpc_block(block.into(), &data_sidecars), + build_rpc_block(block.into(), &data_sidecars, self.spec.clone()), NotifyExecutionLayer::Yes, BlockImportSource::RangeSync, || Ok(()), @@ -443,16 +444,27 @@ impl TestRig { fn build_rpc_block( block: Arc>, data_sidecars: &Option>, + spec: Arc, ) -> RpcBlock { match data_sidecars { Some(DataSidecars::Blobs(blobs)) => { - RpcBlock::new_available(None, block, Some(blobs.clone()), None).unwrap() + let block_data = AvailableBlockData::new(Some(blobs.clone()), None); + RpcBlock::new(block, Some(block_data), spec).unwrap() } Some(DataSidecars::DataColumns(columns)) => { - RpcBlock::new_available(None, block, None, Some(columns.clone())).unwrap() + let block_data = AvailableBlockData::new( + None, + Some( + columns + .iter() + .map(|c| c.as_data_column().clone()) + .collect::>(), + ), + ); + RpcBlock::new(block, Some(block_data), spec).unwrap() } // Block has no data, expects zero columns - None => RpcBlock::new_available(None, block, None, None).unwrap(), + None => RpcBlock::new(block, Some(AvailableBlockData::NoData), spec).unwrap(), } } From 00e6419148cbd2ae77e3c1a84805bba6ef375aaa Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Thu, 4 Dec 2025 16:08:22 -0300 Subject: [PATCH 13/24] Fix --- .../src/data_availability_checker.rs | 2 +- .../beacon_chain/tests/block_verification.rs | 19 ++++++------------- .../tests/payload_invalidation.rs | 8 ++++---- beacon_node/beacon_chain/tests/store_tests.rs | 4 ++-- beacon_node/http_api/src/publish_blocks.rs | 4 +--- .../network_beacon_processor/sync_methods.rs | 2 +- .../src/network_beacon_processor/tests.rs | 16 ++-------------- testing/ef_tests/src/cases/fork_choice.rs | 4 ++-- 8 files changed, 19 insertions(+), 40 deletions(-) diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index ae139fc461c..3b9b283a531 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -1102,7 +1102,7 @@ mod test { let block_data = AvailableBlockData::new(None, Some(custody_columns)); - RpcBlock::new(Arc::new(block), Some(block_data), spec) + RpcBlock::new(Arc::new(block), Some(block_data), spec.clone()) .expect("should create RPC block with custody columns") }) .collect::>(); diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 5076639d843..9c155db7dcb 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -576,8 +576,7 @@ async fn invalid_signature_gossip_block() { .into_block_error() .expect("should import all blocks prior to the one being tested"); let signed_block = SignedBeaconBlock::from_block(block, junk_signature()); - let rpc_block = - RpcBlock::new_maybe_available(None, Arc::new(signed_block), None, None).unwrap(); + let rpc_block = RpcBlock::new(Arc::new(signed_block), None, harness.spec.clone()).unwrap(); let process_res = harness .chain .process_block( @@ -1579,7 +1578,7 @@ async fn add_base_block_to_altair_chain() { // Ensure that it would be impossible to import via `BeaconChain::process_block`. let base_rpc_block = - RpcBlock::new_maybe_available(None, Arc::new(base_block.clone()), None, None).unwrap(); + RpcBlock::new(Arc::new(base_block.clone()), None, harness.spec.clone()).unwrap(); assert!(matches!( harness .chain @@ -1603,9 +1602,7 @@ async fn add_base_block_to_altair_chain() { harness .chain .process_chain_segment( - vec![ - RpcBlock::new_maybe_available(None, Arc::new(base_block), None, None).unwrap() - ], + vec![RpcBlock::new(Arc::new(base_block), None, harness.spec.clone()).unwrap()], NotifyExecutionLayer::Yes, ) .await, @@ -1719,7 +1716,7 @@ async fn add_altair_block_to_base_chain() { // Ensure that it would be impossible to import via `BeaconChain::process_block`. let altair_rpc_block = - RpcBlock::new_maybe_available(None, Arc::new(altair_block.clone()), None, None).unwrap(); + RpcBlock::new(Arc::new(altair_block.clone()), None, harness.spec.clone()).unwrap(); assert!(matches!( harness .chain @@ -1743,10 +1740,7 @@ async fn add_altair_block_to_base_chain() { harness .chain .process_chain_segment( - vec![ - RpcBlock::new_maybe_available(None, Arc::new(altair_block), None, None) - .unwrap() - ], + vec![RpcBlock::new(Arc::new(altair_block), None, harness.spec.clone()).unwrap()], NotifyExecutionLayer::Yes ) .await, @@ -1809,8 +1803,7 @@ async fn import_duplicate_block_unrealized_justification() { // Create two verified variants of the block, representing the same block being processed in // parallel. let notify_execution_layer = NotifyExecutionLayer::Yes; - let rpc_block = - RpcBlock::new_maybe_available(Some(block_root), block.clone(), None, None).unwrap(); + let rpc_block = RpcBlock::new(block.clone(), None, harness.spec.clone()).unwrap(); let verified_block1 = rpc_block .clone() .into_execution_pending_block(block_root, chain, notify_execution_layer) diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index b47c5abc164..9d2b152a9d8 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -686,7 +686,7 @@ async fn invalidates_all_descendants() { let ((fork_block, _), _fork_post_state) = rig.harness.make_block(fork_parent_state, fork_slot).await; let fork_rpc_block = - RpcBlock::new_maybe_available(None, fork_block.clone(), None, None).unwrap(); + RpcBlock::new(fork_block.clone(), None, rig.harness.chain.spec.clone()).unwrap(); let fork_block_root = rig .harness .chain @@ -789,7 +789,7 @@ async fn switches_heads() { rig.harness.make_block(fork_parent_state, fork_slot).await; let fork_parent_root = fork_block.parent_root(); let fork_rpc_block = - RpcBlock::new_maybe_available(None, fork_block.clone(), None, None).unwrap(); + RpcBlock::new(fork_block.clone(), None, rig.harness.chain.spec.clone()).unwrap(); let fork_block_root = rig .harness .chain @@ -1061,7 +1061,7 @@ async fn invalid_parent() { )); // Ensure the block built atop an invalid payload is invalid for import. - let rpc_block = RpcBlock::new_maybe_available(None, block.clone(), None, None).unwrap(); + let rpc_block = RpcBlock::new(block.clone(), None, rig.harness.chain.spec.clone()).unwrap(); assert!(matches!( rig.harness.chain.process_block(rpc_block.block_root(), rpc_block, NotifyExecutionLayer::Yes, BlockImportSource::Lookup, || Ok(()), @@ -1387,7 +1387,7 @@ async fn recover_from_invalid_head_by_importing_blocks() { // Import the fork block, it should become the head. let fork_rpc_block = - RpcBlock::new_maybe_available(None, fork_block.clone(), None, None).unwrap(); + RpcBlock::new(fork_block.clone(), None, rig.harness.chain.spec.clone()).unwrap(); rig.harness .chain .process_block( diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 1f80b597a52..d6951d952f6 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -3544,7 +3544,7 @@ async fn process_blocks_and_attestations_for_unaligned_checkpoint() { assert_ne!(split.state_root, unadvanced_split_state_root); let invalid_fork_rpc_block = - RpcBlock::new_maybe_available(None, invalid_fork_block.clone(), None, None).unwrap(); + RpcBlock::new(invalid_fork_block.clone(), None, harness.spec.clone()).unwrap(); // Applying the invalid block should fail. let err = harness .chain @@ -3561,7 +3561,7 @@ async fn process_blocks_and_attestations_for_unaligned_checkpoint() { // Applying the valid block should succeed, but it should not become head. let valid_fork_rpc_block = - RpcBlock::new_maybe_available(None, valid_fork_block.clone(), None, None).unwrap(); + RpcBlock::new(valid_fork_block.clone(), None, harness.spec.clone()).unwrap(); harness .chain .process_block( diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index 2dea79ff376..7e90d73ccf0 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -314,9 +314,7 @@ pub async fn publish_block>( slot = %block.slot(), "Block previously seen" ); - let Ok(rpc_block) = - RpcBlock::new_maybe_available(Some(block_root), block.clone(), None, None) - else { + let Ok(rpc_block) = RpcBlock::new(block.clone(), None, chain.spec.clone()) else { return Err(warp_utils::reject::custom_bad_request( "Unable to construct rpc block".to_string(), )); diff --git a/beacon_node/network/src/network_beacon_processor/sync_methods.rs b/beacon_node/network/src/network_beacon_processor/sync_methods.rs index ede1fce5f42..de82e86f56f 100644 --- a/beacon_node/network/src/network_beacon_processor/sync_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/sync_methods.rs @@ -726,7 +726,7 @@ impl NetworkBeaconProcessor { .iter() .filter_map(|rpc_block| match rpc_block { RpcBlock::FullyAvailable(available_block) => Some(available_block.clone()), - // TODO this shouldn't be possible here. the rpc block must be available. maybe log an error message just in case + // TODO filter and return error if this variant is found RpcBlock::BlockOnly { .. } => None, }) .collect::>(); diff --git a/beacon_node/network/src/network_beacon_processor/tests.rs b/beacon_node/network/src/network_beacon_processor/tests.rs index a6f685a8e75..a0201eb4a7d 100644 --- a/beacon_node/network/src/network_beacon_processor/tests.rs +++ b/beacon_node/network/src/network_beacon_processor/tests.rs @@ -399,13 +399,7 @@ impl TestRig { self.network_beacon_processor .send_rpc_beacon_block( block_root, - RpcBlock::new_maybe_available( - Some(block_root), - self.next_block.clone(), - None, - None, - ) - .unwrap(), + RpcBlock::new(self.next_block.clone(), None, self._harness.spec.clone()).unwrap(), std::time::Duration::default(), BlockProcessType::SingleBlock { id: 0 }, ) @@ -417,13 +411,7 @@ impl TestRig { self.network_beacon_processor .send_rpc_beacon_block( block_root, - RpcBlock::new_maybe_available( - Some(block_root), - self.next_block.clone(), - None, - None, - ) - .unwrap(), + RpcBlock::new(self.next_block.clone(), None, self._harness.spec.clone()).unwrap(), std::time::Duration::default(), BlockProcessType::SingleBlock { id: 1 }, ) diff --git a/testing/ef_tests/src/cases/fork_choice.rs b/testing/ef_tests/src/cases/fork_choice.rs index 0e4f5e941e2..49c0b17ddaf 100644 --- a/testing/ef_tests/src/cases/fork_choice.rs +++ b/testing/ef_tests/src/cases/fork_choice.rs @@ -545,7 +545,7 @@ impl Tester { .block_on_dangerous( self.harness.chain.process_block( block_root, - RpcBlock::new_maybe_available(Some(block_root), block.clone(), None, None) + RpcBlock::new(block.clone(), None, self.harness.chain.spec.clone()) .map_err(|e| Error::InternalError(format!("{:?}", e)))?, NotifyExecutionLayer::Yes, BlockImportSource::Lookup, @@ -638,7 +638,7 @@ impl Tester { .block_on_dangerous( self.harness.chain.process_block( block_root, - RpcBlock::new_maybe_available(Some(block_root), block.clone(), None, None) + RpcBlock::new(block.clone(), None, self.harness.chain.spec.clone()) .map_err(|e| Error::InternalError(format!("{:?}", e)))?, NotifyExecutionLayer::Yes, BlockImportSource::Lookup, From 59b45db437afdc389f8ea7c7e12b2bc025dbf2aa Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Thu, 4 Dec 2025 16:21:50 -0300 Subject: [PATCH 14/24] Update --- .../src/block_verification_types.rs | 2 +- .../src/data_availability_checker.rs | 4 +- beacon_node/beacon_chain/src/test_utils.rs | 6 +- .../beacon_chain/tests/blob_verification.rs | 2 +- .../beacon_chain/tests/block_verification.rs | 106 +++++++++++------- .../beacon_chain/tests/column_verification.rs | 2 +- beacon_node/beacon_chain/tests/store_tests.rs | 2 +- beacon_node/network/src/sync/tests/lookups.rs | 1 - 8 files changed, 75 insertions(+), 50 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_verification_types.rs b/beacon_node/beacon_chain/src/block_verification_types.rs index 621a24cdce4..6108d43f1f7 100644 --- a/beacon_node/beacon_chain/src/block_verification_types.rs +++ b/beacon_node/beacon_chain/src/block_verification_types.rs @@ -427,7 +427,7 @@ impl AsBlock for RpcBlock { Self::BlockOnly { block, block_root: _, - } => &block, + } => block, Self::FullyAvailable(available_block) => available_block.block(), } } diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 3b9b283a531..1b402cae4f4 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -401,7 +401,7 @@ impl DataAvailabilityChecker { } } - return Ok(MaybeAvailableBlock::Available(available_block)); + Ok(MaybeAvailableBlock::Available(available_block)) } /// Verifies kzg commitments for an RpcBlock, returns a `MaybeAvailableBlock` that may @@ -1111,7 +1111,7 @@ mod test { .iter() .filter_map(|block| match block { RpcBlock::FullyAvailable(available_block) => Some(available_block.clone()), - RpcBlock::BlockOnly { block, block_root } => None, + RpcBlock::BlockOnly { .. } => None, }) .collect::>(); diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index a91207c1b2f..bd891d7fb79 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -1,7 +1,6 @@ use crate::blob_verification::GossipVerifiedBlob; use crate::block_verification_types::{AsBlock, AvailableBlockData, RpcBlock}; use crate::custody_context::NodeCustodyType; -use crate::data_column_verification::CustodyDataColumn; use crate::kzg_utils::build_data_column_sidecars; use crate::observed_operations::ObservationOutcome; pub use crate::persisted_beacon_chain::PersistedBeaconChain; @@ -2369,7 +2368,7 @@ where self.set_current_slot(slot); let (block, blob_items) = block_contents; - let rpc_block = self.build_rpc_block_from_blobs(block_root, block, blob_items, true)?; + let rpc_block = self.build_rpc_block_from_blobs(block, blob_items, true)?; let block_hash: SignedBeaconBlockHash = self .chain .process_block( @@ -2393,7 +2392,7 @@ where let (block, blob_items) = block_contents; let block_root = block.canonical_root(); - let rpc_block = self.build_rpc_block_from_blobs(block_root, block, blob_items, true)?; + let rpc_block = self.build_rpc_block_from_blobs(block, blob_items, true)?; let block_hash: SignedBeaconBlockHash = self .chain .process_block( @@ -2448,7 +2447,6 @@ where /// Builds an `RpcBlock` from a `SignedBeaconBlock` and `BlobsList`. pub fn build_rpc_block_from_blobs( &self, - block_root: Hash256, block: Arc>>, blob_items: Option<(KzgProofs, BlobsList)>, is_available: bool, diff --git a/beacon_node/beacon_chain/tests/blob_verification.rs b/beacon_node/beacon_chain/tests/blob_verification.rs index dfb03b3fd2f..7ad60572c60 100644 --- a/beacon_node/beacon_chain/tests/blob_verification.rs +++ b/beacon_node/beacon_chain/tests/blob_verification.rs @@ -76,7 +76,7 @@ async fn rpc_blobs_with_invalid_header_signature() { // Process the block without blobs so that it doesn't become available. harness.advance_slot(); let rpc_block = harness - .build_rpc_block_from_blobs(block_root, signed_block.clone(), None, false) + .build_rpc_block_from_blobs(signed_block.clone(), None, false) .unwrap(); let availability = harness .chain diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 9c155db7dcb..231a154cb09 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -1,6 +1,7 @@ #![cfg(not(debug_assertions))] use beacon_chain::block_verification_types::{AsBlock, ExecutedBlock, RpcBlock}; +use beacon_chain::data_availability_checker::AvailableBlockData; use beacon_chain::data_column_verification::CustodyDataColumn; use beacon_chain::{ AvailabilityProcessingStatus, BeaconChain, BeaconChainTypes, ExecutionPendingBlock, @@ -129,13 +130,14 @@ fn get_harness( fn chain_segment_blocks( chain_segment: &[BeaconSnapshot], chain_segment_sidecars: &[Option>], + spec: Arc, ) -> Vec> { chain_segment .iter() .zip(chain_segment_sidecars.iter()) .map(|(snapshot, data_sidecars)| { let block = snapshot.beacon_block.clone(); - build_rpc_block(block, data_sidecars) + build_rpc_block(block, data_sidecars, spec.clone()) }) .collect() } @@ -143,15 +145,26 @@ fn chain_segment_blocks( fn build_rpc_block( block: Arc>, data_sidecars: &Option>, + spec: Arc, ) -> RpcBlock { match data_sidecars { Some(DataSidecars::Blobs(blobs)) => { - RpcBlock::new_available(None, block, Some(blobs.clone()), None).unwrap() + let block_data = AvailableBlockData::new(Some(blobs.clone()), None); + RpcBlock::new(block, Some(block_data), spec).unwrap() } Some(DataSidecars::DataColumns(columns)) => { - RpcBlock::new_available(None, block, None, Some(columns.clone())).unwrap() + let block_data = AvailableBlockData::new( + None, + Some( + columns + .iter() + .map(|c| c.as_data_column().clone()) + .collect::>(), + ), + ); + RpcBlock::new(block, Some(block_data), spec).unwrap() } - None => RpcBlock::new_available(None, block, None, None).unwrap(), + None => RpcBlock::new(block, Some(AvailableBlockData::NoData), spec).unwrap(), } } @@ -262,9 +275,10 @@ fn update_data_column_signed_header( async fn chain_segment_full_segment() { let harness = get_harness(VALIDATOR_COUNT, NodeCustodyType::Fullnode); let (chain_segment, chain_segment_blobs) = get_chain_segment().await; - let blocks: Vec> = chain_segment_blocks(&chain_segment, &chain_segment_blobs) - .into_iter() - .collect(); + let blocks: Vec> = + chain_segment_blocks(&chain_segment, &chain_segment_blobs, harness.spec.clone()) + .into_iter() + .collect(); harness .chain @@ -298,9 +312,11 @@ async fn chain_segment_full_segment() { #[tokio::test] async fn chain_segment_varying_chunk_size() { let (chain_segment, chain_segment_blobs) = get_chain_segment().await; - let blocks: Vec> = chain_segment_blocks(&chain_segment, &chain_segment_blobs) - .into_iter() - .collect(); + let harness = get_harness(VALIDATOR_COUNT, NodeCustodyType::Fullnode); + let blocks: Vec> = + chain_segment_blocks(&chain_segment, &chain_segment_blobs, harness.spec.clone()) + .into_iter() + .collect(); for chunk_size in &[1, 2, 31, 32, 33] { let harness = get_harness(VALIDATOR_COUNT, NodeCustodyType::Fullnode); @@ -342,9 +358,10 @@ async fn chain_segment_non_linear_parent_roots() { /* * Test with a block removed. */ - let mut blocks: Vec> = chain_segment_blocks(&chain_segment, &chain_segment_blobs) - .into_iter() - .collect(); + let mut blocks: Vec> = + chain_segment_blocks(&chain_segment, &chain_segment_blobs, harness.spec.clone()) + .into_iter() + .collect(); blocks.remove(2); assert!( @@ -362,17 +379,17 @@ async fn chain_segment_non_linear_parent_roots() { /* * Test with a modified parent root. */ - let mut blocks: Vec> = chain_segment_blocks(&chain_segment, &chain_segment_blobs) - .into_iter() - .collect(); + let mut blocks: Vec> = + chain_segment_blocks(&chain_segment, &chain_segment_blobs, harness.spec.clone()) + .into_iter() + .collect(); let (mut block, signature) = blocks[3].as_block().clone().deconstruct(); *block.parent_root_mut() = Hash256::zero(); - blocks[3] = RpcBlock::new_available( - None, + blocks[3] = RpcBlock::new( Arc::new(SignedBeaconBlock::from_block(block, signature)), - None, - None, + Some(AvailableBlockData::NoData), + harness.spec.clone(), ) .unwrap(); @@ -402,16 +419,16 @@ async fn chain_segment_non_linear_slots() { * Test where a child is lower than the parent. */ - let mut blocks: Vec> = chain_segment_blocks(&chain_segment, &chain_segment_blobs) - .into_iter() - .collect(); + let mut blocks: Vec> = + chain_segment_blocks(&chain_segment, &chain_segment_blobs, harness.spec.clone()) + .into_iter() + .collect(); let (mut block, signature) = blocks[3].as_block().clone().deconstruct(); *block.slot_mut() = Slot::new(0); - blocks[3] = RpcBlock::new_available( - None, + blocks[3] = RpcBlock::new( Arc::new(SignedBeaconBlock::from_block(block, signature)), - None, - None, + Some(AvailableBlockData::NoData), + harness.spec.clone(), ) .unwrap(); @@ -431,16 +448,16 @@ async fn chain_segment_non_linear_slots() { * Test where a child is equal to the parent. */ - let mut blocks: Vec> = chain_segment_blocks(&chain_segment, &chain_segment_blobs) - .into_iter() - .collect(); + let mut blocks: Vec> = + chain_segment_blocks(&chain_segment, &chain_segment_blobs, harness.spec.clone()) + .into_iter() + .collect(); let (mut block, signature) = blocks[3].as_block().clone().deconstruct(); *block.slot_mut() = blocks[2].slot(); - blocks[3] = RpcBlock::new_available( - None, + blocks[3] = RpcBlock::new( Arc::new(SignedBeaconBlock::from_block(block, signature)), - None, - None, + Some(AvailableBlockData::NoData), + harness.spec.clone(), ) .unwrap(); @@ -468,7 +485,9 @@ async fn assert_invalid_signature( let blocks: Vec> = snapshots .iter() .zip(chain_segment_blobs.iter()) - .map(|(snapshot, blobs)| build_rpc_block(snapshot.beacon_block.clone(), blobs)) + .map(|(snapshot, blobs)| { + build_rpc_block(snapshot.beacon_block.clone(), blobs, harness.spec.clone()) + }) .collect(); // Ensure the block will be rejected if imported in a chain segment. @@ -493,7 +512,9 @@ async fn assert_invalid_signature( .iter() .take(block_index) .zip(chain_segment_blobs.iter()) - .map(|(snapshot, blobs)| build_rpc_block(snapshot.beacon_block.clone(), blobs)) + .map(|(snapshot, blobs)| { + build_rpc_block(snapshot.beacon_block.clone(), blobs, harness.spec.clone()) + }) .collect(); // We don't care if this fails, we just call this to ensure that all prior blocks have been // imported prior to this test. @@ -510,6 +531,7 @@ async fn assert_invalid_signature( build_rpc_block( snapshots[block_index].beacon_block.clone(), &chain_segment_blobs[block_index], + harness.spec.clone(), ), NotifyExecutionLayer::Yes, BlockImportSource::Lookup, @@ -567,7 +589,9 @@ async fn invalid_signature_gossip_block() { .iter() .take(block_index) .zip(chain_segment_blobs.iter()) - .map(|(snapshot, blobs)| build_rpc_block(snapshot.beacon_block.clone(), blobs)) + .map(|(snapshot, blobs)| { + build_rpc_block(snapshot.beacon_block.clone(), blobs, harness.spec.clone()) + }) .collect(); harness .chain @@ -618,7 +642,9 @@ async fn invalid_signature_block_proposal() { let blocks: Vec> = snapshots .iter() .zip(chain_segment_blobs.iter()) - .map(|(snapshot, blobs)| build_rpc_block(snapshot.beacon_block.clone(), blobs)) + .map(|(snapshot, blobs)| { + build_rpc_block(snapshot.beacon_block.clone(), blobs, harness.spec.clone()) + }) .collect::>(); // Ensure the block will be rejected if imported in a chain segment. let process_res = harness @@ -935,7 +961,9 @@ async fn invalid_signature_deposit() { let blocks: Vec> = snapshots .iter() .zip(chain_segment_blobs.iter()) - .map(|(snapshot, blobs)| build_rpc_block(snapshot.beacon_block.clone(), blobs)) + .map(|(snapshot, blobs)| { + build_rpc_block(snapshot.beacon_block.clone(), blobs, harness.spec.clone()) + }) .collect(); assert!( !matches!( diff --git a/beacon_node/beacon_chain/tests/column_verification.rs b/beacon_node/beacon_chain/tests/column_verification.rs index 0dbe939839f..4f3e928170d 100644 --- a/beacon_node/beacon_chain/tests/column_verification.rs +++ b/beacon_node/beacon_chain/tests/column_verification.rs @@ -80,7 +80,7 @@ async fn rpc_columns_with_invalid_header_signature() { // Process the block without blobs so that it doesn't become available. harness.advance_slot(); let rpc_block = harness - .build_rpc_block_from_blobs(block_root, signed_block.clone(), None, false) + .build_rpc_block_from_blobs(signed_block.clone(), None, false) .unwrap(); let availability = harness .chain diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index d6951d952f6..79de89abb6e 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -3040,7 +3040,7 @@ async fn weak_subjectivity_sync_test( AvailableBlock::__new_for_testing( block_root, Arc::new(corrupt_block), - data, + data.expect("Expect block data"), Arc::new(spec), ) }; diff --git a/beacon_node/network/src/sync/tests/lookups.rs b/beacon_node/network/src/sync/tests/lookups.rs index 3d3a9f71c0c..34a55f46f4f 100644 --- a/beacon_node/network/src/sync/tests/lookups.rs +++ b/beacon_node/network/src/sync/tests/lookups.rs @@ -1929,7 +1929,6 @@ mod deneb_only { block_verification_types::{AsBlock, RpcBlock}, data_availability_checker::AvailabilityCheckError, }; - use ssz_types::RuntimeVariableList; use std::collections::VecDeque; struct DenebTester { From e8549335f096e24ec3eda97e4395df49b852fb16 Mon Sep 17 00:00:00 2001 From: Mark Mackey Date: Thu, 4 Dec 2025 13:38:56 -0600 Subject: [PATCH 15/24] verify_kzg_for_blocks refactor --- beacon_node/beacon_chain/src/block_verification.rs | 12 ++++++------ .../beacon_chain/src/data_availability_checker.rs | 4 ++-- .../src/network_beacon_processor/sync_methods.rs | 9 +-------- 3 files changed, 9 insertions(+), 16 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 574f0b53396..816ab862cbe 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -654,18 +654,18 @@ pub fn signature_verify_chain_segment( }) .unzip(); - let maybe_available_blocks = chain + let available_blocks = chain .data_availability_checker .verify_kzg_for_rpc_blocks(blocks)?; // zip it back up let mut signature_verified_blocks = roots .into_iter() - .zip(maybe_available_blocks) - .map(|(block_root, maybe_available_block)| { - let consensus_context = ConsensusContext::new(maybe_available_block.slot()) - .set_current_block_root(block_root); + .zip(available_blocks) + .map(|(block_root, available_block)| { + let consensus_context = + ConsensusContext::new(available_block.slot()).set_current_block_root(block_root); SignatureVerifiedBlock { - block: maybe_available_block, + block: MaybeAvailableBlock::Available(available_block), block_root, parent: None, consensus_context, diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 1b402cae4f4..7c6d7ecdba9 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -433,7 +433,7 @@ impl DataAvailabilityChecker { pub fn verify_kzg_for_rpc_blocks( &self, available_blocks: Vec>, - ) -> Result>, AvailabilityCheckError> { + ) -> Result>, AvailabilityCheckError> { let mut results = Vec::with_capacity(available_blocks.len()); let all_blobs = available_blocks .iter() @@ -471,7 +471,7 @@ impl DataAvailabilityChecker { return Err(AvailabilityCheckError::MissingCustodyColumns); } - results.push(MaybeAvailableBlock::Available(available_block.clone())); + results.push(available_block); } Ok(results) diff --git a/beacon_node/network/src/network_beacon_processor/sync_methods.rs b/beacon_node/network/src/network_beacon_processor/sync_methods.rs index de82e86f56f..6cb0acf35a4 100644 --- a/beacon_node/network/src/network_beacon_processor/sync_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/sync_methods.rs @@ -8,7 +8,6 @@ use crate::sync::{ }; use beacon_chain::block_verification_types::{AsBlock, RpcBlock}; use beacon_chain::data_availability_checker::AvailabilityCheckError; -use beacon_chain::data_availability_checker::MaybeAvailableBlock; use beacon_chain::historical_data_columns::HistoricalDataColumnError; use beacon_chain::{ AvailabilityProcessingStatus, BeaconChainTypes, BlockError, ChainSegmentResult, @@ -736,13 +735,7 @@ impl NetworkBeaconProcessor { .data_availability_checker .verify_kzg_for_rpc_blocks(available_blocks) { - Ok(blocks) => blocks - .into_iter() - .filter_map(|maybe_available| match maybe_available { - MaybeAvailableBlock::Available(block) => Some(block), - MaybeAvailableBlock::AvailabilityPending { .. } => None, - }) - .collect::>(), + Ok(blocks) => blocks, Err(e) => match e { AvailabilityCheckError::StoreError(_) => { return ( From 622c199a322f176805a5316bd388395cf6434f61 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 8 Dec 2025 13:07:27 -0300 Subject: [PATCH 16/24] Rename kzg verification for blocks and improve comments --- .../beacon_chain/src/block_verification.rs | 7 ++++--- .../src/data_availability_checker.rs | 18 +++++++----------- .../network_beacon_processor/sync_methods.rs | 4 ++-- 3 files changed, 13 insertions(+), 16 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 816ab862cbe..efd784114b2 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -646,7 +646,7 @@ pub fn signature_verify_chain_segment( )?; // unzip chain segment and verify kzg in bulk - let (roots, blocks): (Vec<_>, Vec<_>) = chain_segment + let (roots, available_blocks): (Vec<_>, Vec<_>) = chain_segment .into_iter() .filter_map(|(block_root, block)| match block { RpcBlock::FullyAvailable(available_block) => Some((block_root, available_block)), @@ -654,9 +654,10 @@ pub fn signature_verify_chain_segment( }) .unzip(); - let available_blocks = chain + chain .data_availability_checker - .verify_kzg_for_rpc_blocks(blocks)?; + .batch_verify_kzg_for_available_blocks(&available_blocks)?; + // zip it back up let mut signature_verified_blocks = roots .into_iter() diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 7c6d7ecdba9..5c7a6a8440c 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -423,18 +423,16 @@ impl DataAvailabilityChecker { } } - /// Checks if a vector of blocks are available. Returns a vector of `MaybeAvailableBlock` - /// This is more efficient than calling `verify_kzg_for_rpc_block` in a loop as it does - /// all kzg verification at once + /// Performs batch kzg verification for a vector of `AvailableBlocks`. This is more efficient than + /// calling `verify_kzg_for_available_block` in a loop. /// /// WARNING: This function assumes all required blobs are already present, it does NOT /// check if there are any missing blobs. #[instrument(skip_all)] - pub fn verify_kzg_for_rpc_blocks( + pub fn batch_verify_kzg_for_available_blocks( &self, - available_blocks: Vec>, - ) -> Result>, AvailabilityCheckError> { - let mut results = Vec::with_capacity(available_blocks.len()); + available_blocks: &Vec>, + ) -> Result<(), AvailabilityCheckError> { let all_blobs = available_blocks .iter() .filter(|available_block| self.blobs_required_for_block(&available_block.block)) @@ -470,11 +468,9 @@ impl DataAvailabilityChecker { } else if self.data_columns_required_for_block(&available_block.block) { return Err(AvailabilityCheckError::MissingCustodyColumns); } - - results.push(available_block); } - Ok(results) + Ok(()) } /// Determines the blob requirements for a block. If the block is pre-deneb, no blobs are required. @@ -1116,7 +1112,7 @@ mod test { .collect::>(); // WHEN verifying all blocks together (totalling 256 data columns) - let verification_result = da_checker.verify_kzg_for_rpc_blocks(available_blocks); + let verification_result = da_checker.batch_verify_kzg_for_available_blocks(&available_blocks); // THEN batch block verification should fail due to 128 invalid columns in the second block verification_result.expect_err("should have failed to verify blocks"); diff --git a/beacon_node/network/src/network_beacon_processor/sync_methods.rs b/beacon_node/network/src/network_beacon_processor/sync_methods.rs index 6cb0acf35a4..094e29a0710 100644 --- a/beacon_node/network/src/network_beacon_processor/sync_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/sync_methods.rs @@ -730,10 +730,10 @@ impl NetworkBeaconProcessor { }) .collect::>(); - let available_blocks = match self + match self .chain .data_availability_checker - .verify_kzg_for_rpc_blocks(available_blocks) + .batch_verify_kzg_for_available_blocks(&available_blocks) { Ok(blocks) => blocks, Err(e) => match e { From d669bec8a5124a8e4bed9f2a5ae2c48c20772c65 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 8 Dec 2025 13:07:38 -0300 Subject: [PATCH 17/24] Fmt --- beacon_node/beacon_chain/src/data_availability_checker.rs | 3 ++- .../network/src/network_beacon_processor/sync_methods.rs | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 5c7a6a8440c..9d5094622b9 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -1112,7 +1112,8 @@ mod test { .collect::>(); // WHEN verifying all blocks together (totalling 256 data columns) - let verification_result = da_checker.batch_verify_kzg_for_available_blocks(&available_blocks); + let verification_result = + da_checker.batch_verify_kzg_for_available_blocks(&available_blocks); // THEN batch block verification should fail due to 128 invalid columns in the second block verification_result.expect_err("should have failed to verify blocks"); diff --git a/beacon_node/network/src/network_beacon_processor/sync_methods.rs b/beacon_node/network/src/network_beacon_processor/sync_methods.rs index 094e29a0710..d6d8842ce64 100644 --- a/beacon_node/network/src/network_beacon_processor/sync_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/sync_methods.rs @@ -730,7 +730,7 @@ impl NetworkBeaconProcessor { }) .collect::>(); - match self + match self .chain .data_availability_checker .batch_verify_kzg_for_available_blocks(&available_blocks) From f082560561522862236f2957bad88a219f8c0266 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 8 Dec 2025 13:30:17 -0300 Subject: [PATCH 18/24] Refactor kzg verifcation for rpc block --- .../beacon_chain/src/block_verification.rs | 32 +++++++++++------ .../src/data_availability_checker.rs | 36 ++++++------------- .../tests/attestation_production.rs | 34 +++++++++--------- beacon_node/beacon_chain/tests/store_tests.rs | 21 ++++++----- 4 files changed, 62 insertions(+), 61 deletions(-) diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index efd784114b2..1b52e8c76ff 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -1305,16 +1305,28 @@ impl IntoExecutionPendingBlock for RpcBlock // Perform an early check to prevent wasting time on irrelevant blocks. let block_root = check_block_relevancy(self.as_block(), block_root, chain) .map_err(|e| BlockSlashInfo::SignatureNotChecked(self.signed_block_header(), e))?; - let maybe_available = chain - .data_availability_checker - .verify_kzg_for_rpc_block(self.clone()) - .map_err(|e| { - BlockSlashInfo::SignatureNotChecked( - self.signed_block_header(), - BlockError::AvailabilityCheck(e), - ) - })?; - SignatureVerifiedBlock::check_slashable(maybe_available, block_root, chain)? + + let maybe_available_block = match &self { + RpcBlock::FullyAvailable(available_block) => { + chain + .data_availability_checker + .verify_kzg_for_available_block(available_block) + .map_err(|e| { + BlockSlashInfo::SignatureNotChecked( + self.signed_block_header(), + BlockError::AvailabilityCheck(e), + ) + })?; + MaybeAvailableBlock::Available(available_block.clone()) + } + // No need to perform KZG verification unless we have a fully available block + RpcBlock::BlockOnly { block, block_root } => MaybeAvailableBlock::AvailabilityPending { + block_root: *block_root, + block: block.clone(), + }, + }; + + SignatureVerifiedBlock::check_slashable(maybe_available_block, block_root, chain)? .into_execution_pending_block_slashable(block_root, chain, notify_execution_layer) } diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 9d5094622b9..83d37fdf493 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -1,9 +1,7 @@ use crate::blob_verification::{ GossipVerifiedBlob, KzgVerifiedBlob, KzgVerifiedBlobList, verify_kzg_for_blob_list, }; -use crate::block_verification_types::{ - AvailabilityPendingExecutedBlock, AvailableExecutedBlock, RpcBlock, -}; +use crate::block_verification_types::{AvailabilityPendingExecutedBlock, AvailableExecutedBlock}; use crate::data_availability_checker::overflow_lru_cache::{ DataAvailabilityCheckerInner, ReconstructColumnsDecision, }; @@ -367,10 +365,14 @@ impl DataAvailabilityChecker { .remove_pre_execution_block(block_root); } - pub fn verify_kzg_for_fully_available_rpc_block( + /// Verifies kzg commitments for an `AvailableBlock`.` + /// + /// WARNING: This function assumes all required blobs are already present, it does NOT + /// check if there are any missing blobs. + pub fn verify_kzg_for_available_block( &self, - available_block: AvailableBlock, - ) -> Result, AvailabilityCheckError> { + available_block: &AvailableBlock, + ) -> Result<(), AvailabilityCheckError> { match &available_block.blob_data { AvailableBlockData::NoData => { if self.blobs_required_for_block(&available_block.block) @@ -401,26 +403,7 @@ impl DataAvailabilityChecker { } } - Ok(MaybeAvailableBlock::Available(available_block)) - } - - /// Verifies kzg commitments for an RpcBlock, returns a `MaybeAvailableBlock` that may - /// include the fully available block. - /// - /// WARNING: This function assumes all required blobs are already present, it does NOT - /// check if there are any missing blobs. - pub fn verify_kzg_for_rpc_block( - &self, - block: RpcBlock, - ) -> Result, AvailabilityCheckError> { - match block { - RpcBlock::FullyAvailable(available_rpc_block) => { - self.verify_kzg_for_fully_available_rpc_block(available_rpc_block) - } - RpcBlock::BlockOnly { block_root, block } => { - Ok(MaybeAvailableBlock::AvailabilityPending { block_root, block }) - } - } + Ok(()) } /// Performs batch kzg verification for a vector of `AvailableBlocks`. This is more efficient than @@ -886,6 +869,7 @@ impl MaybeAvailableBlock { mod test { use super::*; use crate::CustodyContext; + use crate::block_verification_types::RpcBlock; use crate::custody_context::NodeCustodyType; use crate::data_column_verification::CustodyDataColumn; use crate::test_utils::{ diff --git a/beacon_node/beacon_chain/tests/attestation_production.rs b/beacon_node/beacon_chain/tests/attestation_production.rs index 0acb23d5126..9f969c11ed8 100644 --- a/beacon_node/beacon_chain/tests/attestation_production.rs +++ b/beacon_node/beacon_chain/tests/attestation_production.rs @@ -1,6 +1,7 @@ #![cfg(not(debug_assertions))] use beacon_chain::attestation_simulator::produce_unaggregated_attestation; +use beacon_chain::block_verification_types::RpcBlock; use beacon_chain::test_utils::{AttestationStrategy, BeaconChainHarness, BlockStrategy}; use beacon_chain::validator_monitor::UNAGGREGATED_ATTESTATION_LAG_SLOTS; use beacon_chain::{StateSkipConfig, WhenSlotSkipped, metrics}; @@ -222,15 +223,16 @@ async fn produces_attestations() { let rpc_block = harness.build_rpc_block_from_store_blobs(Some(block_root), Arc::new(block.clone())); - let beacon_chain::data_availability_checker::MaybeAvailableBlock::Available( - available_block, - ) = chain - .data_availability_checker - .verify_kzg_for_rpc_block(rpc_block) - .unwrap() - else { - panic!("block should be available") - }; + + match rpc_block { + RpcBlock::FullyAvailable(available_block) => { + chain + .data_availability_checker + .verify_kzg_for_available_block(&available_block) + .unwrap(); + } + RpcBlock::BlockOnly { .. } => panic!("block should be available"), + } let early_attestation = { let proto_block = chain @@ -295,15 +297,15 @@ async fn early_attester_cache_old_request() { let rpc_block = harness .build_rpc_block_from_store_blobs(Some(head.beacon_block_root), head.beacon_block.clone()); - let beacon_chain::data_availability_checker::MaybeAvailableBlock::Available(available_block) = - harness + + match rpc_block { + RpcBlock::FullyAvailable(available_block) => harness .chain .data_availability_checker - .verify_kzg_for_rpc_block(rpc_block) - .unwrap() - else { - panic!("block should be available") - }; + .verify_kzg_for_available_block(&available_block) + .unwrap(), + RpcBlock::BlockOnly { block, block_root } => panic!("block should be available"), + } harness .chain diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 79de89abb6e..26f865bd474 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -3016,16 +3016,19 @@ async fn weak_subjectivity_sync_test( .expect("should get block") .expect("should get block"); - if let MaybeAvailableBlock::Available(block) = harness - .chain - .data_availability_checker - .verify_kzg_for_rpc_block( + let rpc_block = + harness.build_rpc_block_from_store_blobs(Some(block_root), Arc::new(full_block)); + + match rpc_block { + RpcBlock::FullyAvailable(available_block) => { harness - .build_rpc_block_from_store_blobs(Some(block_root), Arc::new(full_block)), - ) - .expect("should verify kzg") - { - available_blocks.push(block); + .chain + .data_availability_checker + .verify_kzg_for_available_block(&available_block) + .expect("should verify kzg"); + available_blocks.push(available_block); + } + RpcBlock::BlockOnly { .. } => panic!("Should be an available block"), } } From ba3abcd1de599ce0a0e8f6a2b9035fb4a92db5ee Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 8 Dec 2025 14:37:33 -0300 Subject: [PATCH 19/24] Fix --- .../tests/attestation_production.rs | 24 +++++++++++-------- beacon_node/beacon_chain/tests/store_tests.rs | 1 - 2 files changed, 14 insertions(+), 11 deletions(-) diff --git a/beacon_node/beacon_chain/tests/attestation_production.rs b/beacon_node/beacon_chain/tests/attestation_production.rs index 9f969c11ed8..2ec6f24521b 100644 --- a/beacon_node/beacon_chain/tests/attestation_production.rs +++ b/beacon_node/beacon_chain/tests/attestation_production.rs @@ -224,15 +224,16 @@ async fn produces_attestations() { let rpc_block = harness.build_rpc_block_from_store_blobs(Some(block_root), Arc::new(block.clone())); - match rpc_block { + let available_block = match rpc_block { RpcBlock::FullyAvailable(available_block) => { chain .data_availability_checker .verify_kzg_for_available_block(&available_block) .unwrap(); + available_block } RpcBlock::BlockOnly { .. } => panic!("block should be available"), - } + }; let early_attestation = { let proto_block = chain @@ -298,14 +299,17 @@ async fn early_attester_cache_old_request() { let rpc_block = harness .build_rpc_block_from_store_blobs(Some(head.beacon_block_root), head.beacon_block.clone()); - match rpc_block { - RpcBlock::FullyAvailable(available_block) => harness - .chain - .data_availability_checker - .verify_kzg_for_available_block(&available_block) - .unwrap(), - RpcBlock::BlockOnly { block, block_root } => panic!("block should be available"), - } + let available_block = match rpc_block { + RpcBlock::FullyAvailable(available_block) => { + harness + .chain + .data_availability_checker + .verify_kzg_for_available_block(&available_block) + .unwrap(); + available_block + } + RpcBlock::BlockOnly { .. } => panic!("block should be available"), + }; harness .chain diff --git a/beacon_node/beacon_chain/tests/store_tests.rs b/beacon_node/beacon_chain/tests/store_tests.rs index 26f865bd474..76c0f2903e3 100644 --- a/beacon_node/beacon_chain/tests/store_tests.rs +++ b/beacon_node/beacon_chain/tests/store_tests.rs @@ -21,7 +21,6 @@ use beacon_chain::{ compute_proposer_duties_from_head, ensure_state_can_determine_proposers_for_epoch, }, custody_context::NodeCustodyType, - data_availability_checker::MaybeAvailableBlock, historical_blocks::HistoricalBlockError, migrate::MigratorConfig, }; From 0e997cb803d8741f52b179f842d18ca3ec2b2e25 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 8 Dec 2025 15:20:24 -0300 Subject: [PATCH 20/24] Fix tests --- .../src/data_availability_checker.rs | 24 +++++++++++++++++-- 1 file changed, 22 insertions(+), 2 deletions(-) diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 83d37fdf493..546a7810f85 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -446,9 +446,13 @@ impl DataAvailabilityChecker { } for available_block in available_blocks { - if self.blobs_required_for_block(&available_block.block) { + if self.blobs_required_for_block(&available_block.block) + && available_block.data().blobs_len() == 0 + { return Err(AvailabilityCheckError::MissingBlobs); - } else if self.data_columns_required_for_block(&available_block.block) { + } else if self.data_columns_required_for_block(&available_block.block) + && available_block.data().data_columns_len() == 0 + { return Err(AvailabilityCheckError::MissingCustodyColumns); } } @@ -724,6 +728,14 @@ impl AvailableBlockData { } } + pub fn blobs_len(&self) -> usize { + if let Some(blobs) = self.blobs() { + blobs.len() + } else { + 0 + } + } + pub fn data_columns(&self) -> Option> { match self { AvailableBlockData::NoData => None, @@ -731,6 +743,14 @@ impl AvailableBlockData { AvailableBlockData::DataColumns(data_columns) => Some(data_columns.clone()), } } + + pub fn data_columns_len(&self) -> usize { + if let Some(data_columns) = self.data_columns() { + data_columns.len() + } else { + 0 + } + } } /// A fully available block that is ready to be imported into fork choice. From c2a180a83f50914878126e3f2f885db99b3b7e91 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 8 Dec 2025 15:33:16 -0300 Subject: [PATCH 21/24] Cleanup --- .../src/data_availability_checker.rs | 40 ++++++++----------- 1 file changed, 17 insertions(+), 23 deletions(-) diff --git a/beacon_node/beacon_chain/src/data_availability_checker.rs b/beacon_node/beacon_chain/src/data_availability_checker.rs index 546a7810f85..1b0d8e45ce5 100644 --- a/beacon_node/beacon_chain/src/data_availability_checker.rs +++ b/beacon_node/beacon_chain/src/data_availability_checker.rs @@ -373,11 +373,11 @@ impl DataAvailabilityChecker { &self, available_block: &AvailableBlock, ) -> Result<(), AvailabilityCheckError> { - match &available_block.blob_data { + let block_data_required = self.blobs_required_for_block(&available_block.block) + || self.data_columns_required_for_block(&available_block.block); + match available_block.data() { AvailableBlockData::NoData => { - if self.blobs_required_for_block(&available_block.block) - || self.data_columns_required_for_block(&available_block.block) - { + if block_data_required { if available_block.block.fork_name_unchecked().fulu_enabled() { return Err(AvailabilityCheckError::MissingCustodyColumns); } else { @@ -386,20 +386,12 @@ impl DataAvailabilityChecker { } } AvailableBlockData::Blobs(blobs) => { - // TODO(rpc-block) should we return an error if blobs not required? - // the blobs variant should only exist for a block that requires blobs - if self.blobs_required_for_block(&available_block.block) { - verify_kzg_for_blob_list(blobs.iter(), &self.kzg) - .map_err(AvailabilityCheckError::InvalidBlobs)?; - } + verify_kzg_for_blob_list(blobs.iter(), &self.kzg) + .map_err(AvailabilityCheckError::InvalidBlobs)?; } AvailableBlockData::DataColumns(data_columns) => { - // TODO(rpc-block) should we return an error if columns not required? - // the columns variant should only exist for a block that requires columns - if self.data_columns_required_for_block(&available_block.block) { - verify_kzg_for_data_column_list(data_columns.iter(), &self.kzg) - .map_err(AvailabilityCheckError::InvalidColumn)?; - } + verify_kzg_for_data_column_list(data_columns.iter(), &self.kzg) + .map_err(AvailabilityCheckError::InvalidColumn)?; } } @@ -446,14 +438,16 @@ impl DataAvailabilityChecker { } for available_block in available_blocks { - if self.blobs_required_for_block(&available_block.block) - && available_block.data().blobs_len() == 0 - { - return Err(AvailabilityCheckError::MissingBlobs); - } else if self.data_columns_required_for_block(&available_block.block) - && available_block.data().data_columns_len() == 0 + let block_data_required = self.blobs_required_for_block(&available_block.block) + || self.data_columns_required_for_block(&available_block.block); + if let AvailableBlockData::NoData = available_block.data() + && block_data_required { - return Err(AvailabilityCheckError::MissingCustodyColumns); + if available_block.block.fork_name_unchecked().fulu_enabled() { + return Err(AvailabilityCheckError::MissingCustodyColumns); + } else { + return Err(AvailabilityCheckError::MissingBlobs); + } } } From 7e1bada267229a0dc3bd5a8551acc59dd3799f48 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Mon, 8 Dec 2025 15:50:56 -0300 Subject: [PATCH 22/24] Fix test --- beacon_node/beacon_chain/tests/block_verification.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/tests/block_verification.rs b/beacon_node/beacon_chain/tests/block_verification.rs index 231a154cb09..6c531e900fc 100644 --- a/beacon_node/beacon_chain/tests/block_verification.rs +++ b/beacon_node/beacon_chain/tests/block_verification.rs @@ -1831,7 +1831,12 @@ async fn import_duplicate_block_unrealized_justification() { // Create two verified variants of the block, representing the same block being processed in // parallel. let notify_execution_layer = NotifyExecutionLayer::Yes; - let rpc_block = RpcBlock::new(block.clone(), None, harness.spec.clone()).unwrap(); + let rpc_block = RpcBlock::new( + block.clone(), + Some(AvailableBlockData::NoData), + harness.spec.clone(), + ) + .unwrap(); let verified_block1 = rpc_block .clone() .into_execution_pending_block(block_root, chain, notify_execution_layer) From ce377c704c39f01496f344e7d1977704f1f60fc9 Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 9 Dec 2025 12:05:38 -0300 Subject: [PATCH 23/24] Fix --- beacon_node/beacon_chain/src/test_utils.rs | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/test_utils.rs b/beacon_node/beacon_chain/src/test_utils.rs index bd891d7fb79..2ccde4b4eb9 100644 --- a/beacon_node/beacon_chain/src/test_utils.rs +++ b/beacon_node/beacon_chain/src/test_utils.rs @@ -2392,7 +2392,15 @@ where let (block, blob_items) = block_contents; let block_root = block.canonical_root(); - let rpc_block = self.build_rpc_block_from_blobs(block, blob_items, true)?; + // Determine if block is available: it's available if it doesn't require blobs, + // or if it requires blobs and we have them + let has_blob_commitments = block + .message() + .body() + .blob_kzg_commitments() + .is_ok_and(|c| !c.is_empty()); + let is_available = !has_blob_commitments || blob_items.is_some(); + let rpc_block = self.build_rpc_block_from_blobs(block, blob_items, is_available)?; let block_hash: SignedBeaconBlockHash = self .chain .process_block( From 6e7f5b75e0e820f696b236002a31f87e0faab41e Mon Sep 17 00:00:00 2001 From: Eitan Seri-Levi Date: Tue, 9 Dec 2025 14:44:24 -0300 Subject: [PATCH 24/24] Fix test --- beacon_node/beacon_chain/src/historical_blocks.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/beacon_node/beacon_chain/src/historical_blocks.rs b/beacon_node/beacon_chain/src/historical_blocks.rs index d183b924a58..c5b6e6c37a7 100644 --- a/beacon_node/beacon_chain/src/historical_blocks.rs +++ b/beacon_node/beacon_chain/src/historical_blocks.rs @@ -157,7 +157,7 @@ impl BeaconChain { if let Some(block_data) = block_data { match &block_data { - AvailableBlockData::NoData => todo!(), + AvailableBlockData::NoData => (), AvailableBlockData::Blobs(_) => new_oldest_blob_slot = Some(block.slot()), AvailableBlockData::DataColumns(_) => { new_oldest_data_column_slot = Some(block.slot())