Skip to content

Commit ed7cd3b

Browse files
dappliondapplion
authored
Drop block data from BlockError and BlobError (#5735)
* Drop block data from BlockError and BlobError * Debug release tests * Fix release tests * Revert unnecessary changes to lighthouse_metrics
1 parent a685dde commit ed7cd3b

File tree

15 files changed

+208
-288
lines changed

15 files changed

+208
-288
lines changed

‎beacon_node/beacon_chain/src/beacon_chain.rs‎

Lines changed: 32 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ impl TryInto<Hash256> for AvailabilityProcessingStatus {
206206
}
207207

208208
/// The result of a chain segment processing.
209-
pub enum ChainSegmentResult<E: EthSpec> {
209+
pub enum ChainSegmentResult {
210210
/// Processing this chain segment finished successfully.
211211
Successful {
212212
imported_blocks: Vec<(Hash256, Slot)>,
@@ -215,7 +215,7 @@ pub enum ChainSegmentResult<E: EthSpec> {
215215
/// have been imported.
216216
Failed {
217217
imported_blocks: Vec<(Hash256, Slot)>,
218-
error: BlockError<E>,
218+
error: BlockError,
219219
},
220220
}
221221

@@ -2159,7 +2159,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
21592159
self: &Arc<Self>,
21602160
blob_sidecar: Arc<BlobSidecar<T::EthSpec>>,
21612161
subnet_id: u64,
2162-
) -> Result<GossipVerifiedBlob<T>, GossipBlobError<T::EthSpec>> {
2162+
) -> Result<GossipVerifiedBlob<T>, GossipBlobError> {
21632163
metrics::inc_counter(&metrics::BLOBS_SIDECAR_PROCESSING_REQUESTS);
21642164
let _timer = metrics::start_timer(&metrics::BLOBS_SIDECAR_GOSSIP_VERIFICATION_TIMES);
21652165
GossipVerifiedBlob::new(blob_sidecar, subnet_id, self).map(|v| {
@@ -2698,7 +2698,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
26982698
pub fn filter_chain_segment(
26992699
self: &Arc<Self>,
27002700
chain_segment: Vec<RpcBlock<T::EthSpec>>,
2701-
) -> Result<Vec<HashBlockTuple<T::EthSpec>>, ChainSegmentResult<T::EthSpec>> {
2701+
) -> Result<Vec<HashBlockTuple<T::EthSpec>>, ChainSegmentResult> {
27022702
// This function will never import any blocks.
27032703
let imported_blocks = vec![];
27042704
let mut filtered_chain_segment = Vec::with_capacity(chain_segment.len());
@@ -2805,7 +2805,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
28052805
self: &Arc<Self>,
28062806
chain_segment: Vec<RpcBlock<T::EthSpec>>,
28072807
notify_execution_layer: NotifyExecutionLayer,
2808-
) -> ChainSegmentResult<T::EthSpec> {
2808+
) -> ChainSegmentResult {
28092809
let mut imported_blocks = vec![];
28102810

28112811
// Filter uninteresting blocks from the chain segment in a blocking task.
@@ -2938,7 +2938,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
29382938
pub async fn verify_block_for_gossip(
29392939
self: &Arc<Self>,
29402940
block: Arc<SignedBeaconBlock<T::EthSpec>>,
2941-
) -> Result<GossipVerifiedBlock<T>, BlockError<T::EthSpec>> {
2941+
) -> Result<GossipVerifiedBlock<T>, BlockError> {
29422942
let chain = self.clone();
29432943
self.task_executor
29442944
.clone()
@@ -2986,7 +2986,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
29862986
pub async fn process_gossip_blob(
29872987
self: &Arc<Self>,
29882988
blob: GossipVerifiedBlob<T>,
2989-
) -> Result<AvailabilityProcessingStatus, BlockError<T::EthSpec>> {
2989+
) -> Result<AvailabilityProcessingStatus, BlockError> {
29902990
let block_root = blob.block_root();
29912991

29922992
// If this block has already been imported to forkchoice it must have been available, so
@@ -3026,7 +3026,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
30263026
AvailabilityProcessingStatus,
30273027
DataColumnsToPublish<T::EthSpec>,
30283028
),
3029-
BlockError<T::EthSpec>,
3029+
BlockError,
30303030
> {
30313031
let Ok((slot, block_root)) = data_columns
30323032
.iter()
@@ -3062,7 +3062,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
30623062
slot: Slot,
30633063
block_root: Hash256,
30643064
blobs: FixedBlobSidecarList<T::EthSpec>,
3065-
) -> Result<AvailabilityProcessingStatus, BlockError<T::EthSpec>> {
3065+
) -> Result<AvailabilityProcessingStatus, BlockError> {
30663066
// If this block has already been imported to forkchoice it must have been available, so
30673067
// we don't need to process its blobs again.
30683068
if self
@@ -3099,7 +3099,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
30993099
AvailabilityProcessingStatus,
31003100
DataColumnsToPublish<T::EthSpec>,
31013101
),
3102-
BlockError<T::EthSpec>,
3102+
BlockError,
31033103
> {
31043104
let Ok((slot, block_root)) = custody_columns
31053105
.iter()
@@ -3133,8 +3133,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
31333133
fn remove_notified(
31343134
&self,
31353135
block_root: &Hash256,
3136-
r: Result<AvailabilityProcessingStatus, BlockError<T::EthSpec>>,
3137-
) -> Result<AvailabilityProcessingStatus, BlockError<T::EthSpec>> {
3136+
r: Result<AvailabilityProcessingStatus, BlockError>,
3137+
) -> Result<AvailabilityProcessingStatus, BlockError> {
31383138
let has_missing_components =
31393139
matches!(r, Ok(AvailabilityProcessingStatus::MissingComponents(_, _)));
31403140
if !has_missing_components {
@@ -3148,8 +3148,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
31483148
fn remove_notified_custody_columns<P>(
31493149
&self,
31503150
block_root: &Hash256,
3151-
r: Result<(AvailabilityProcessingStatus, P), BlockError<T::EthSpec>>,
3152-
) -> Result<(AvailabilityProcessingStatus, P), BlockError<T::EthSpec>> {
3151+
r: Result<(AvailabilityProcessingStatus, P), BlockError>,
3152+
) -> Result<(AvailabilityProcessingStatus, P), BlockError> {
31533153
let has_missing_components = matches!(
31543154
r,
31553155
Ok((AvailabilityProcessingStatus::MissingComponents(_, _), _))
@@ -3168,7 +3168,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
31683168
unverified_block: B,
31693169
block_source: BlockImportSource,
31703170
notify_execution_layer: NotifyExecutionLayer,
3171-
) -> Result<AvailabilityProcessingStatus, BlockError<T::EthSpec>> {
3171+
) -> Result<AvailabilityProcessingStatus, BlockError> {
31723172
self.reqresp_pre_import_cache
31733173
.write()
31743174
.insert(block_root, unverified_block.block_cloned());
@@ -3204,8 +3204,8 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
32043204
unverified_block: B,
32053205
notify_execution_layer: NotifyExecutionLayer,
32063206
block_source: BlockImportSource,
3207-
publish_fn: impl FnOnce() -> Result<(), BlockError<T::EthSpec>> + Send + 'static,
3208-
) -> Result<AvailabilityProcessingStatus, BlockError<T::EthSpec>> {
3207+
publish_fn: impl FnOnce() -> Result<(), BlockError> + Send + 'static,
3208+
) -> Result<AvailabilityProcessingStatus, BlockError> {
32093209
// Start the Prometheus timer.
32103210
let _full_timer = metrics::start_timer(&metrics::BLOCK_PROCESSING_TIMES);
32113211

@@ -3326,7 +3326,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
33263326
pub async fn into_executed_block(
33273327
self: Arc<Self>,
33283328
execution_pending_block: ExecutionPendingBlock<T>,
3329-
) -> Result<ExecutedBlock<T::EthSpec>, BlockError<T::EthSpec>> {
3329+
) -> Result<ExecutedBlock<T::EthSpec>, BlockError> {
33303330
let ExecutionPendingBlock {
33313331
block,
33323332
import_data,
@@ -3381,7 +3381,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
33813381
async fn check_block_availability_and_import(
33823382
self: &Arc<Self>,
33833383
block: AvailabilityPendingExecutedBlock<T::EthSpec>,
3384-
) -> Result<AvailabilityProcessingStatus, BlockError<T::EthSpec>> {
3384+
) -> Result<AvailabilityProcessingStatus, BlockError> {
33853385
let slot = block.block.slot();
33863386
let availability = self
33873387
.data_availability_checker
@@ -3394,7 +3394,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
33943394
async fn check_gossip_blob_availability_and_import(
33953395
self: &Arc<Self>,
33963396
blob: GossipVerifiedBlob<T>,
3397-
) -> Result<AvailabilityProcessingStatus, BlockError<T::EthSpec>> {
3397+
) -> Result<AvailabilityProcessingStatus, BlockError> {
33983398
let slot = blob.slot();
33993399
if let Some(slasher) = self.slasher.as_ref() {
34003400
slasher.accept_block_header(blob.signed_block_header());
@@ -3416,7 +3416,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
34163416
AvailabilityProcessingStatus,
34173417
DataColumnsToPublish<T::EthSpec>,
34183418
),
3419-
BlockError<T::EthSpec>,
3419+
BlockError,
34203420
> {
34213421
if let Some(slasher) = self.slasher.as_ref() {
34223422
for data_colum in &data_columns {
@@ -3440,7 +3440,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
34403440
slot: Slot,
34413441
block_root: Hash256,
34423442
blobs: FixedBlobSidecarList<T::EthSpec>,
3443-
) -> Result<AvailabilityProcessingStatus, BlockError<T::EthSpec>> {
3443+
) -> Result<AvailabilityProcessingStatus, BlockError> {
34443444
// Need to scope this to ensure the lock is dropped before calling `process_availability`
34453445
// Even an explicit drop is not enough to convince the borrow checker.
34463446
{
@@ -3450,7 +3450,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
34503450
.filter_map(|b| b.as_ref().map(|b| b.signed_block_header.clone()))
34513451
.unique()
34523452
{
3453-
if verify_header_signature::<T, BlockError<T::EthSpec>>(self, &header).is_ok() {
3453+
if verify_header_signature::<T, BlockError>(self, &header).is_ok() {
34543454
slashable_cache
34553455
.observe_slashable(
34563456
header.message.slot,
@@ -3484,7 +3484,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
34843484
AvailabilityProcessingStatus,
34853485
DataColumnsToPublish<T::EthSpec>,
34863486
),
3487-
BlockError<T::EthSpec>,
3487+
BlockError,
34883488
> {
34893489
// Need to scope this to ensure the lock is dropped before calling `process_availability`
34903490
// Even an explicit drop is not enough to convince the borrow checker.
@@ -3493,7 +3493,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
34933493
// Assumes all items in custody_columns are for the same block_root
34943494
if let Some(column) = custody_columns.first() {
34953495
let header = &column.signed_block_header;
3496-
if verify_header_signature::<T, BlockError<T::EthSpec>>(self, header).is_ok() {
3496+
if verify_header_signature::<T, BlockError>(self, header).is_ok() {
34973497
slashable_cache
34983498
.observe_slashable(
34993499
header.message.slot,
@@ -3530,7 +3530,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
35303530
self: &Arc<Self>,
35313531
slot: Slot,
35323532
availability: Availability<T::EthSpec>,
3533-
) -> Result<AvailabilityProcessingStatus, BlockError<T::EthSpec>> {
3533+
) -> Result<AvailabilityProcessingStatus, BlockError> {
35343534
match availability {
35353535
Availability::Available(block) => {
35363536
// Block is fully available, import into fork choice
@@ -3545,7 +3545,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
35453545
pub async fn import_available_block(
35463546
self: &Arc<Self>,
35473547
block: Box<AvailableExecutedBlock<T::EthSpec>>,
3548-
) -> Result<AvailabilityProcessingStatus, BlockError<T::EthSpec>> {
3548+
) -> Result<AvailabilityProcessingStatus, BlockError> {
35493549
let AvailableExecutedBlock {
35503550
block,
35513551
import_data,
@@ -3624,7 +3624,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
36243624
parent_block: SignedBlindedBeaconBlock<T::EthSpec>,
36253625
parent_eth1_finalization_data: Eth1FinalizationData,
36263626
mut consensus_context: ConsensusContext<T::EthSpec>,
3627-
) -> Result<Hash256, BlockError<T::EthSpec>> {
3627+
) -> Result<Hash256, BlockError> {
36283628
// ----------------------------- BLOCK NOT YET ATTESTABLE ----------------------------------
36293629
// Everything in this initial section is on the hot path between processing the block and
36303630
// being able to attest to it. DO NOT add any extra processing in this initial section
@@ -3934,7 +3934,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
39343934
block: BeaconBlockRef<T::EthSpec>,
39353935
block_root: Hash256,
39363936
state: &BeaconState<T::EthSpec>,
3937-
) -> Result<(), BlockError<T::EthSpec>> {
3937+
) -> Result<(), BlockError> {
39383938
// Only perform the weak subjectivity check if it was configured.
39393939
let Some(wss_checkpoint) = self.config.weak_subjectivity_checkpoint else {
39403940
return Ok(());
@@ -4269,7 +4269,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
42694269
&self,
42704270
block_root: Hash256,
42714271
state: &mut BeaconState<T::EthSpec>,
4272-
) -> Result<(), BlockError<T::EthSpec>> {
4272+
) -> Result<(), BlockError> {
42734273
for relative_epoch in [RelativeEpoch::Current, RelativeEpoch::Next] {
42744274
let shuffling_id = AttestationShufflingId::new(block_root, state, relative_epoch)?;
42754275

@@ -7042,8 +7042,8 @@ impl From<BeaconStateError> for Error {
70427042
}
70437043
}
70447044

7045-
impl<E: EthSpec> ChainSegmentResult<E> {
7046-
pub fn into_block_error(self) -> Result<(), BlockError<E>> {
7045+
impl ChainSegmentResult {
7046+
pub fn into_block_error(self) -> Result<(), BlockError> {
70477047
match self {
70487048
ChainSegmentResult::Failed { error, .. } => Err(error),
70497049
ChainSegmentResult::Successful { .. } => Ok(()),

‎beacon_node/beacon_chain/src/blob_verification.rs‎

Lines changed: 13 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ use types::{
2222

2323
/// An error occurred while validating a gossip blob.
2424
#[derive(Debug)]
25-
pub enum GossipBlobError<E: EthSpec> {
25+
pub enum GossipBlobError {
2626
/// The blob sidecar is from a slot that is later than the current slot (with respect to the
2727
/// gossip clock disparity).
2828
///
@@ -95,7 +95,7 @@ pub enum GossipBlobError<E: EthSpec> {
9595
/// ## Peer scoring
9696
///
9797
/// We cannot process the blob without validating its parent, the peer isn't necessarily faulty.
98-
BlobParentUnknown(Arc<BlobSidecar<E>>),
98+
BlobParentUnknown { parent_root: Hash256 },
9999

100100
/// Invalid kzg commitment inclusion proof
101101
/// ## Peer scoring
@@ -145,28 +145,19 @@ pub enum GossipBlobError<E: EthSpec> {
145145
NotFinalizedDescendant { block_parent_root: Hash256 },
146146
}
147147

148-
impl<E: EthSpec> std::fmt::Display for GossipBlobError<E> {
148+
impl std::fmt::Display for GossipBlobError {
149149
fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
150-
match self {
151-
GossipBlobError::BlobParentUnknown(blob_sidecar) => {
152-
write!(
153-
f,
154-
"BlobParentUnknown(parent_root:{})",
155-
blob_sidecar.block_parent_root()
156-
)
157-
}
158-
other => write!(f, "{:?}", other),
159-
}
150+
write!(f, "{:?}", self)
160151
}
161152
}
162153

163-
impl<E: EthSpec> From<BeaconChainError> for GossipBlobError<E> {
154+
impl From<BeaconChainError> for GossipBlobError {
164155
fn from(e: BeaconChainError) -> Self {
165156
GossipBlobError::BeaconChainError(e)
166157
}
167158
}
168159

169-
impl<E: EthSpec> From<BeaconStateError> for GossipBlobError<E> {
160+
impl From<BeaconStateError> for GossipBlobError {
170161
fn from(e: BeaconStateError) -> Self {
171162
GossipBlobError::BeaconChainError(BeaconChainError::BeaconStateError(e))
172163
}
@@ -190,12 +181,12 @@ impl<T: BeaconChainTypes> GossipVerifiedBlob<T> {
190181
blob: Arc<BlobSidecar<T::EthSpec>>,
191182
subnet_id: u64,
192183
chain: &BeaconChain<T>,
193-
) -> Result<Self, GossipBlobError<T::EthSpec>> {
184+
) -> Result<Self, GossipBlobError> {
194185
let header = blob.signed_block_header.clone();
195186
// We only process slashing info if the gossip verification failed
196187
// since we do not process the blob any further in that case.
197188
validate_blob_sidecar_for_gossip(blob, subnet_id, chain).map_err(|e| {
198-
process_block_slash_info::<_, GossipBlobError<T::EthSpec>>(
189+
process_block_slash_info::<_, GossipBlobError>(
199190
chain,
200191
BlockSlashInfo::from_early_error_blob(header, e),
201192
)
@@ -384,7 +375,7 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
384375
blob_sidecar: Arc<BlobSidecar<T::EthSpec>>,
385376
subnet: u64,
386377
chain: &BeaconChain<T>,
387-
) -> Result<GossipVerifiedBlob<T>, GossipBlobError<T::EthSpec>> {
378+
) -> Result<GossipVerifiedBlob<T>, GossipBlobError> {
388379
let blob_slot = blob_sidecar.slot();
389380
let blob_index = blob_sidecar.index;
390381
let block_parent_root = blob_sidecar.block_parent_root();
@@ -466,7 +457,9 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
466457
// We have already verified that the blob is past finalization, so we can
467458
// just check fork choice for the block's parent.
468459
let Some(parent_block) = fork_choice.get_block(&block_parent_root) else {
469-
return Err(GossipBlobError::BlobParentUnknown(blob_sidecar));
460+
return Err(GossipBlobError::BlobParentUnknown {
461+
parent_root: block_parent_root,
462+
});
470463
};
471464

472465
// Do not process a blob that does not descend from the finalized root.
@@ -516,7 +509,7 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes>(
516509
))
517510
})?;
518511

519-
let state = cheap_state_advance_to_obtain_committees::<_, GossipBlobError<T::EthSpec>>(
512+
let state = cheap_state_advance_to_obtain_committees::<_, GossipBlobError>(
520513
&mut parent_state,
521514
Some(parent_state_root),
522515
blob_slot,

0 commit comments

Comments
 (0)