diff --git a/Cargo.lock b/Cargo.lock index 078f699f3c..d42bcd8fc1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3282,9 +3282,9 @@ dependencies = [ [[package]] name = "ethereum_ssz" -version = "0.10.3" +version = "0.10.4" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "368a4a4e4273b0135111fe9464e35465067766a8f664615b5a86338b73864407" +checksum = "e462875ad8693755ea8913d6e905715c76ea4836e2254e18c9cf0f7a8f8c2a13" dependencies = [ "alloy-primitives", "arbitrary", diff --git a/beacon_node/beacon_chain/src/data_column_verification.rs b/beacon_node/beacon_chain/src/data_column_verification.rs index 71562b376b..45cd687b36 100644 --- a/beacon_node/beacon_chain/src/data_column_verification.rs +++ b/beacon_node/beacon_chain/src/data_column_verification.rs @@ -1220,7 +1220,16 @@ pub fn validate_partial_data_column_sidecar_for_gossip( header, }; } - Err(MissingCellsError::UnexpectedError(e)) => todo!("handle unexpected error {:?}", e), + Err(MissingCellsError::UnexpectedError(e)) => { + return PartialColumnVerificationResult::ErrWithValidHeader { + err: GossipDataColumnError::InternalError(format!( + "An unexpected error occurred while validating partial data columns: {:?}", + e + )) + .into(), + header, + }; + } }; // We do not have to check block related data here, as we create the verifiable column from diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index e96c86b17f..ca4ab85524 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -524,11 +524,15 @@ pub(crate) fn publish_column_sidecars( if chain.config.enable_partial_columns && let DataColumnSidecar::Fulu(fulu_data_col) = data_col.as_ref() { - let mut partial = fulu_data_col.to_partial(); - if let Some(header) = partial.sidecar.header.take() { - partial_header = Some(header); + match fulu_data_col.to_partial() { + Ok(mut partial) => { + if let Some(header) = partial.sidecar.header.take() { + partial_header = Some(header); + } + partial_columns.push(Arc::new(partial)); + } + Err(err) => crit!(?err, "Could not convert from full to partial"), } - partial_columns.push(Arc::new(partial)); } let subnet = DataColumnSubnetId::from_column_index(*data_col.index(), &chain.spec); diff --git a/beacon_node/lighthouse_network/src/types/partial.rs b/beacon_node/lighthouse_network/src/types/partial.rs index f25ce9ec36..26705b7106 100644 --- a/beacon_node/lighthouse_network/src/types/partial.rs +++ b/beacon_node/lighthouse_network/src/types/partial.rs @@ -9,7 +9,7 @@ use std::sync::Arc; use tracing::{debug, error}; use types::core::{EthSpec, Hash256}; use types::data::{ - CellBitmap, PartialDataColumn, PartialDataColumnHeader, PartialDataColumnPartsMetadata, + PartialDataColumn, PartialDataColumnHeader, PartialDataColumnPartsMetadata, PartialDataColumnSidecar, PartialDataColumnSidecarRef, }; @@ -32,12 +32,8 @@ impl OutgoingPartialColumn { header_sent_set: HeaderSentSet, ) -> Self { // For now, always request all cells - let mut requests = partial_column.sidecar.cells_present_bitmap.clone(); - for idx in 0..requests.len() { - requests - .set(idx, true) - .expect("Bound asserted via `len` above"); - } + let mut requests = partial_column.sidecar.cells_present_bitmap.clone_zeroed(); + requests.not_inplace(); let metadata = PartialDataColumnPartsMetadata:: { available: partial_column.sidecar.cells_present_bitmap.clone(), requests, @@ -45,10 +41,7 @@ impl OutgoingPartialColumn { .into(); let header_message = PartialDataColumnSidecarRef { - cells_present_bitmap: CellBitmap::::with_capacity( - partial_column.sidecar.cells_present_bitmap.len(), - ) - .expect("Taking length from bitmap with same bound"), + cells_present_bitmap: partial_column.sidecar.cells_present_bitmap.clone_zeroed(), column: vec![], kzg_proofs: vec![], header: Some(header).into(), @@ -210,7 +203,7 @@ impl Partial for OutgoingPartialColumn { let send = self .partial_column .sidecar - .filter(|idx| want.get(idx).expect("Bound checked above")) + .filter(|idx| want.get(idx).unwrap_or(false)) .map_err(|err| { error!(?err, "Unexpected error filtering sidecar"); PartialError::InvalidFormat @@ -262,6 +255,7 @@ mod tests { use fixed_bytes::FixedBytesExtended; use libp2p::identity::Keypair; use ssz_types::FixedVector; + use types::CellBitmap; use types::block::{BeaconBlockHeader, SignedBeaconBlockHeader}; use types::core::{MinimalEthSpec, Slot}; use types::data::PartialDataColumnHeader; diff --git a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs index d34668b138..7a902649cb 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -1381,16 +1381,20 @@ impl NetworkBeaconProcessor { &[&data_column_index.to_string()], ); - let mut column = col.to_partial(); - let header = column.sidecar.header.take(); - if let Some(header) = header { - self.send_network_message(NetworkMessage::PublishPartialColumns { - columns: vec![Arc::new(column)], - header: Arc::new(header), - }); - } else { - crit!("Converting from full to partial yielded headerless partial") - }; + match col.to_partial() { + Ok(mut column) => { + let header = column.sidecar.header.take(); + if let Some(header) = header { + self.send_network_message(NetworkMessage::PublishPartialColumns { + columns: vec![Arc::new(column)], + header: Arc::new(header), + }); + } else { + crit!("Converting from full to partial yielded headerless partial") + }; + } + Err(err) => crit!(?err, "Could not convert from full to partial"), + } } let result = self diff --git a/consensus/types/src/data/data_column_sidecar.rs b/consensus/types/src/data/data_column_sidecar.rs index 170aa99666..d15651730f 100644 --- a/consensus/types/src/data/data_column_sidecar.rs +++ b/consensus/types/src/data/data_column_sidecar.rs @@ -250,19 +250,16 @@ impl DataColumnSidecarFulu { } /// Convert this full data column into a verifiable partial data column. - pub fn to_partial(&self) -> PartialDataColumn { + /// Note: This is not expected to ever fail. + pub fn to_partial(&self) -> Result, PartialDataColumnSidecarError> { let cell_count = self.column.len(); - let mut bitmap = - CellBitmap::::with_capacity(cell_count).expect("our column has the same bound"); - for idx in 0..cell_count { - bitmap - .set(idx, true) - .expect("The correct size is initialized right above"); - } + let mut bitmap = CellBitmap::::with_capacity(cell_count) + .map_err(|_| PartialDataColumnSidecarError::UnexpectedBounds)?; + bitmap.not_inplace(); let block_root = self.block_root(); - PartialDataColumn { + Ok(PartialDataColumn { block_root, index: self.index, sidecar: PartialDataColumnSidecar { @@ -276,7 +273,7 @@ impl DataColumnSidecarFulu { }) .into(), }, - } + }) } }