smol refactor

This commit is contained in:
Eitan Seri-Levi
2026-05-01 10:54:50 +02:00
8 changed files with 73 additions and 56 deletions

View File

@@ -1186,9 +1186,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
indices: &[ColumnIndex],
) -> Result<DataColumnSidecarList<T::EthSpec>, Error> {
let all_cached_columns_opt = self
.pending_payload_cache
.data_availability_checker
.get_data_columns(block_root)
.or_else(|| self.data_availability_checker.get_data_columns(block_root))
.or_else(|| self.pending_payload_cache.get_data_columns(block_root))
.or_else(|| self.early_attester_cache.get_data_columns(block_root));
if let Some(mut all_cached_columns) = all_cached_columns_opt {

View File

@@ -21,7 +21,7 @@ use tracing::{debug, error, instrument};
use types::data::{BlobIdentifier, FixedBlobSidecarList, PartialDataColumn};
use types::{
BlobSidecar, BlobSidecarList, BlockImportSource, ChainSpec, DataColumnSidecar,
DataColumnSidecarList, Epoch, EthSpec, ForkName, Hash256, PartialDataColumnSidecarError,
DataColumnSidecarList, Epoch, EthSpec, Hash256, PartialDataColumnSidecarError,
PartialDataColumnSidecarRef, SignedBeaconBlock, Slot, new_non_zero_usize,
};
@@ -906,7 +906,7 @@ impl<E: EthSpec> AvailableBlock<E> {
match &block_data {
AvailableBlockData::NoData => {
// For Gloas, DA is checked for the PayloadEnvelope, not for the block.
if block.fork_name_unchecked() < ForkName::Gloas {
if !block.fork_name_unchecked().gloas_enabled() {
if columns_required {
return Err(AvailabilityCheckError::MissingCustodyColumns);
} else if blobs_required {

View File

@@ -1096,6 +1096,12 @@ pub fn validate_data_column_sidecar_for_gossip_gloas<
slot: column_slot,
},
)?;
if bid.message.slot != column_slot {
return Err(GossipDataColumnError::BlockSlotMismatch {
block_slot: bid.message.slot,
data_column_slot: column_slot,
});
}
let kzg_commitments = &bid.message.blob_kzg_commitments;
verify_data_column_sidecar_with_commitments_len(
&data_column,

View File

@@ -184,7 +184,6 @@ impl<T: BeaconChainTypes> PendingPayloadCache<T> {
let pending_components =
self.get_pending_components(beacon_block_root, bid, |pending_components| {
pending_components.insert_executed_payload_envelope(executed_envelope);
Ok(())
})?;
let num_expected_columns = self
@@ -422,14 +421,14 @@ impl<T: BeaconChainTypes> PendingPayloadCache<T> {
update_fn: F,
) -> Result<MappedRwLockReadGuard<'_, PendingComponents<T::EthSpec>>, AvailabilityCheckError>
where
F: FnOnce(&mut PendingComponents<T::EthSpec>) -> Result<(), AvailabilityCheckError>,
F: FnOnce(&mut PendingComponents<T::EthSpec>),
{
let mut write_lock = self.availability_cache.write();
{
let pending_components = write_lock
.get_or_insert_mut(block_root, || PendingComponents::new(block_root, bid));
update_fn(pending_components)?
update_fn(pending_components)
}
RwLockReadGuard::try_map(RwLockWriteGuard::downgrade(write_lock), |cache| {
@@ -622,15 +621,7 @@ mod data_availability_checker_tests {
const RNG_SEED: u64 = 0xDEADBEEF;
fn gloas_spec<E: EthSpec>() -> Arc<ChainSpec> {
let mut spec = E::default_spec();
spec.altair_fork_epoch = Some(Epoch::new(0));
spec.bellatrix_fork_epoch = Some(Epoch::new(0));
spec.capella_fork_epoch = Some(Epoch::new(0));
spec.deneb_fork_epoch = Some(Epoch::new(0));
spec.electra_fork_epoch = Some(Epoch::new(0));
spec.fulu_fork_epoch = Some(Epoch::new(0));
spec.gloas_fork_epoch = Some(Epoch::new(0));
Arc::new(spec)
Arc::new(ForkName::Gloas.make_genesis_spec(E::default_spec()))
}
fn get_store_with_spec<E: EthSpec>(

View File

@@ -8,12 +8,6 @@ pub struct PendingColumn<E: EthSpec> {
cells: Vec<Option<(Cell<E>, KzgProof)>>,
}
impl<E: EthSpec> Default for PendingColumn<E> {
fn default() -> Self {
Self { cells: Vec::new() }
}
}
impl<E: EthSpec> PendingColumn<E> {
/// Allocate a `PendingColumn` whose `cells` vec has space for `blob_count` entries, all
/// initialised to `None`. Required so that `insert(idx, ...)` can write into `cells[idx]`.
@@ -42,36 +36,47 @@ impl<E: EthSpec> PendingColumn<E> {
self.cells.len() == blob_count && self.cells.iter().all(|cell| cell.is_some())
}
pub fn try_to_sidecar(
/// Build a `DataColumnSidecar` from the cached cells.
///
/// Caller MUST have checked `is_complete(blob_count)` first; this returns `Err` only on the
/// (currently theoretically impossible) `VariableList` size-bound failures, which we surface
/// as a typed error so the caller can log/metric it instead of silently producing nothing.
pub fn to_sidecar(
&self,
index: ColumnIndex,
slot: Slot,
beacon_block_root: Hash256,
blob_count: usize,
) -> Option<Arc<DataColumnSidecar<E>>> {
if self.cells.len() != blob_count {
return None;
}
let mut column = Vec::with_capacity(blob_count);
) -> Result<Arc<DataColumnSidecar<E>>, PendingColumnError> {
let mut column = Vec::with_capacity(self.cells.len());
let mut kzg_proofs = Vec::with_capacity(self.cells.len());
for cell in self.cells.iter() {
let Some((cell, proof)) = cell else {
return None;
};
let (cell, proof) = cell.as_ref().ok_or(PendingColumnError::IncompleteColumn)?;
// TODO(gloas): we likely want to go and arc all cells
column.push(cell.clone());
kzg_proofs.push(*proof);
}
Some(Arc::new(DataColumnSidecar::Gloas(DataColumnSidecarGloas {
// TODO(gloas): this hard-codes the Gloas sidecar variant. Pass the fork in once
// post-Gloas variants are introduced (or move construction to a fork-aware helper).
Ok(Arc::new(DataColumnSidecar::Gloas(DataColumnSidecarGloas {
index,
// TODO(gloas): this should not error, but we need to catch it
column: VariableList::try_from(column).ok()?,
kzg_proofs: VariableList::try_from(kzg_proofs).ok()?,
column: VariableList::try_from(column)
.map_err(|_| PendingColumnError::ColumnSizeExceedsBound)?,
kzg_proofs: VariableList::try_from(kzg_proofs)
.map_err(|_| PendingColumnError::ProofsSizeExceedsBound)?,
slot,
beacon_block_root,
})))
}
}
/// Errors returned by [`PendingColumn::to_sidecar`]. `IncompleteColumn` should never fire if the
/// caller checks [`PendingColumn::is_complete`] first; the size-bound variants reflect spec-bound
/// invariants and should never fire in practice.
#[derive(Debug, Clone)]
pub enum PendingColumnError {
IncompleteColumn,
ColumnSizeExceedsBound,
ProofsSizeExceedsBound,
}

View File

@@ -7,7 +7,7 @@ use crate::pending_payload_cache::pending_column::PendingColumn;
use std::cmp::Ordering;
use std::collections::HashMap;
use std::sync::Arc;
use tracing::{Span, debug, debug_span};
use tracing::{Span, debug, debug_span, error};
use types::DataColumnSidecar;
use types::{ColumnIndex, EthSpec, Hash256, SignedExecutionPayloadBid};
@@ -30,18 +30,31 @@ impl<E: EthSpec> PendingComponents<E> {
self.bid.message.blob_kzg_commitments.len()
}
/// Returns the completed custody columns
/// Returns the completed custody columns.
///
/// Skips columns that are not yet complete and logs an error if a complete column fails to
/// build (a spec-bound invariant would have to be violated; should never happen in practice).
pub fn get_cached_data_columns(&self) -> Vec<Arc<DataColumnSidecar<E>>> {
let blob_count = self.num_blobs_expected();
let slot = self.bid.message.slot;
let block_root = self.block_root;
self.verified_data_columns
.iter()
.filter_map(|(col_idx, col)| {
col.try_to_sidecar(
*col_idx,
self.bid.message.slot,
self.block_root,
self.num_blobs_expected(),
)
})
.filter(|(_, col)| col.is_complete(blob_count))
.filter_map(
|(col_idx, col)| match col.to_sidecar(*col_idx, slot, block_root) {
Ok(sidecar) => Some(sidecar),
Err(e) => {
error!(
?e,
column_index = %col_idx,
?block_root,
"Failed to build sidecar for complete column"
);
None
}
},
)
.collect()
}
@@ -60,7 +73,7 @@ impl<E: EthSpec> PendingComponents<E> {
pub(crate) fn merge_data_columns(
&mut self,
kzg_verified_data_columns: &[KzgVerifiedCustodyDataColumn<E>],
) -> Result<(), AvailabilityCheckError> {
) {
let num_blobs_expected = self.num_blobs_expected();
for data_column in kzg_verified_data_columns {
let data_column = data_column.as_data_column();
@@ -80,8 +93,6 @@ impl<E: EthSpec> PendingComponents<E> {
col.insert(cell_idx, cell, proof);
}
}
Ok(())
}
// TODO(gloas): merge partial columns
@@ -97,7 +108,7 @@ impl<E: EthSpec> PendingComponents<E> {
pub fn num_completed_columns(&self) -> usize {
self.verified_data_columns
.values()
.filter_map(|col| col.is_complete(self.num_blobs_expected()).then_some(()))
.filter(|col| col.is_complete(self.num_blobs_expected()))
.count()
}

View File

@@ -728,14 +728,14 @@ impl<T: BeaconChainTypes> NetworkBeaconProcessor<T> {
..
} => {
debug!(
action = "requesting block",
action = "ignoring",
%unknown_block_root,
"Unknown block root for column"
);
self.send_sync_message(SyncMessage::UnknownBlockHashFromAttestation(
peer_id,
unknown_block_root,
));
// TODO(gloas): wire this into proper lookup sync. Sending
// `UnknownBlockHashFromAttestation` here is a Fulu-shaped fallback that
// mixes column processing with the attestation lookup path and is not
// the right primitive for Gloas column lookups.
self.propagate_validation_result(
message_id,
peer_id,

View File

@@ -906,6 +906,10 @@ impl<T: BeaconChainTypes> SyncManager<T> {
);
}
DataColumnSidecar::Gloas(_) => {
// TODO(gloas): proper lookup sync for Gloas. Routing into
// `handle_unknown_block_root` here mixes column processing with the
// single-block-lookup path; the Gloas column-arrives-before-block
// case wants its own queue/wakeup.
debug!(%block_root, "Received unknown block data column message");
self.handle_unknown_block_root(peer_id, block_root);
}