From 76d43aac090d65a46ce11c11c5ad775b0554bf3d Mon Sep 17 00:00:00 2001 From: PoulavBhowmick03 Date: Sat, 21 Jun 2025 11:16:41 +0530 Subject: [PATCH 1/6] Added Box to some fields in BeaconChainError --- .../beacon_chain/src/beacon_block_streamer.rs | 13 ++++--- beacon_node/beacon_chain/src/beacon_chain.rs | 36 ++++++++++++------- .../beacon_chain/src/beacon_proposer_cache.rs | 4 ++- .../beacon_chain/src/block_verification.rs | 4 +-- .../beacon_chain/src/canonical_head.rs | 16 ++++----- beacon_node/beacon_chain/src/errors.rs | 36 +++++++++---------- 6 files changed, 62 insertions(+), 47 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_block_streamer.rs b/beacon_node/beacon_chain/src/beacon_block_streamer.rs index e37a69040db..ace507d65dd 100644 --- a/beacon_node/beacon_chain/src/beacon_block_streamer.rs +++ b/beacon_node/beacon_chain/src/beacon_block_streamer.rs @@ -121,8 +121,8 @@ fn reconstruct_default_header_block( Err(BeaconChainError::InconsistentPayloadReconstructed { slot: blinded_block.slot(), exec_block_hash: header_from_block.block_hash(), - canonical_transactions_root: header_from_block.transactions_root(), - reconstructed_transactions_root: header_from_payload.transactions_root(), + canonical_transactions_root: Box::new(header_from_block.transactions_root()), + reconstructed_transactions_root: Box::new(header_from_payload.transactions_root()), }) } } @@ -152,9 +152,12 @@ fn reconstruct_blocks( let error = BeaconChainError::InconsistentPayloadReconstructed { slot: block_parts.blinded_block.slot(), exec_block_hash: block_parts.header.block_hash(), - canonical_transactions_root: block_parts.header.transactions_root(), - reconstructed_transactions_root: header_from_payload - .transactions_root(), + canonical_transactions_root: Box::new( + block_parts.header.transactions_root(), + ), + reconstructed_transactions_root: Box::new( + header_from_payload.transactions_root(), + ), }; debug!(?root, ?error, "Failed to reconstruct block"); block_map.insert(root, Arc::new(Err(error))); diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index e6cdd84b405..d30094f3475 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1179,8 +1179,8 @@ impl BeaconChain { return Err(Error::InconsistentPayloadReconstructed { slot: blinded_block.slot(), exec_block_hash, - canonical_transactions_root: execution_payload_header.transactions_root(), - reconstructed_transactions_root: header_from_payload.transactions_root(), + canonical_transactions_root: Box::new(execution_payload_header.transactions_root()), + reconstructed_transactions_root: Box::new(header_from_payload.transactions_root()), }); } @@ -1629,7 +1629,9 @@ impl BeaconChain { .canonical_head .fork_choice_read_lock() .get_block_execution_status(&head_block_root) - .ok_or(Error::AttestationHeadNotInForkChoice(head_block_root))?; + .ok_or(Error::AttestationHeadNotInForkChoice(Box::new( + head_block_root, + )))?; let (duties, dependent_root) = self.with_committee_cache( head_block_root, @@ -1687,8 +1689,8 @@ impl BeaconChain { || latest_block_root != *checkpoint.root { return Err(BeaconChainError::InvalidCheckpoint { - state_root, - checkpoint, + state_root: Box::new(state_root), + checkpoint: Box::new(checkpoint), }); } @@ -1776,13 +1778,15 @@ impl BeaconChain { { // The attestation references a block that is not in fork choice, it must be // pre-finalization. - None => Err(Error::CannotAttestToFinalizedBlock { beacon_block_root }), + None => Err(Error::CannotAttestToFinalizedBlock { + beacon_block_root: Box::new(beacon_block_root), + }), // The attestation references a fully valid `beacon_block_root`. Some(execution_status) if execution_status.is_valid_or_irrelevant() => Ok(attestation), // The attestation references a block that has not been verified by an EL (i.e. it // is optimistic or invalid). Don't return the block, return an error instead. Some(execution_status) => Err(Error::HeadBlockNotFullyVerified { - beacon_block_root, + beacon_block_root: Box::new(beacon_block_root), execution_status, }), } @@ -1817,13 +1821,15 @@ impl BeaconChain { { // The contribution references a block that is not in fork choice, it must be // pre-finalization. - None => Err(Error::SyncContributionDataReferencesFinalizedBlock { beacon_block_root }), + None => Err(Error::SyncContributionDataReferencesFinalizedBlock { + beacon_block_root: Box::new(beacon_block_root), + }), // The contribution references a fully valid `beacon_block_root`. Some(execution_status) if execution_status.is_valid_or_irrelevant() => Ok(contribution), // The contribution references a block that has not been verified by an EL (i.e. it // is optimistic or invalid). Don't return the block, return an error instead. Some(execution_status) => Err(Error::HeadBlockNotFullyVerified { - beacon_block_root, + beacon_block_root: Box::new(beacon_block_root), execution_status, }), } @@ -1979,11 +1985,15 @@ impl BeaconChain { Some(execution_status) if execution_status.is_valid_or_irrelevant() => (), Some(execution_status) => { return Err(Error::HeadBlockNotFullyVerified { - beacon_block_root, + beacon_block_root: Box::new(beacon_block_root), execution_status, }) } - None => return Err(Error::HeadMissingFromForkChoice(beacon_block_root)), + None => { + return Err(Error::HeadMissingFromForkChoice(Box::new( + beacon_block_root, + ))) + } }; /* @@ -2467,7 +2477,7 @@ impl BeaconChain { let fork_choice_lock = self.canonical_head.fork_choice_read_lock(); let block = fork_choice_lock .get_block(block_root) - .ok_or(Error::AttestationHeadNotInForkChoice(*block_root))?; + .ok_or(Error::AttestationHeadNotInForkChoice(Box::new(*block_root)))?; drop(fork_choice_lock); let block_shuffling_id = if target_epoch == block.current_epoch_shuffling_id.shuffling_epoch @@ -5887,7 +5897,7 @@ impl BeaconChain { // Return an error here to try and prevent progression by upstream functions. return Err(Error::JustifiedPayloadInvalid { - justified_root: justified_block.root, + justified_root: Box::new(justified_block.root), execution_block_hash: justified_block.execution_status.block_hash(), }); } diff --git a/beacon_node/beacon_chain/src/beacon_proposer_cache.rs b/beacon_node/beacon_chain/src/beacon_proposer_cache.rs index 12970214c6a..3c770291d59 100644 --- a/beacon_node/beacon_chain/src/beacon_proposer_cache.rs +++ b/beacon_node/beacon_chain/src/beacon_proposer_cache.rs @@ -175,7 +175,9 @@ pub fn compute_proposer_duties_from_head( .canonical_head .fork_choice_read_lock() .get_block_execution_status(&head_block_root) - .ok_or(BeaconChainError::HeadMissingFromForkChoice(head_block_root))?; + .ok_or(BeaconChainError::HeadMissingFromForkChoice(Box::new( + head_block_root, + )))?; // Advance the state into the requested epoch. ensure_state_is_in_epoch(&mut state, head_state_root, request_epoch, &chain.spec)?; diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 317ec02cc1e..335990c06d7 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -1432,9 +1432,9 @@ impl ExecutionPendingBlock { let parent_slot = parent.beacon_block.slot(); if state.slot() < parent_slot || state.slot() > block.slot() { return Err(BeaconChainError::BadPreState { - parent_root: parent.beacon_block_root, + parent_root: Box::new(parent.beacon_block_root), parent_slot, - block_root, + block_root: Box::new(block_root), block_slot: block.slot(), state_slot: state.slot(), } diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index a6f5179fdc6..3b61a190381 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -334,7 +334,7 @@ impl CanonicalHead { let head_block_root = self.cached_head().head_block_root(); self.fork_choice_read_lock() .get_block_execution_status(&head_block_root) - .ok_or(Error::HeadMissingFromForkChoice(head_block_root)) + .ok_or(Error::HeadMissingFromForkChoice(Box::new(head_block_root))) } /// Returns a clone of the `CachedHead` and the execution status of the contained head block. @@ -350,7 +350,7 @@ impl CanonicalHead { let execution_status = self .fork_choice_read_lock() .get_block_execution_status(&head_block_root) - .ok_or(Error::HeadMissingFromForkChoice(head_block_root))?; + .ok_or(Error::HeadMissingFromForkChoice(Box::new(head_block_root)))?; Ok((head, execution_status)) } @@ -592,9 +592,9 @@ impl BeaconChain { let new_head_proto_block = fork_choice_read_lock .get_block(&new_view.head_block_root) - .ok_or(Error::HeadBlockMissingFromForkChoice( + .ok_or(Error::HeadBlockMissingFromForkChoice(Box::new( new_view.head_block_root, - ))?; + )))?; // Do not allow an invalid block to become the head. // @@ -610,7 +610,7 @@ impl BeaconChain { // However, this check is cheap. if new_head_proto_block.execution_status.is_invalid() { return Err(Error::HeadHasInvalidPayload { - block_root: new_head_proto_block.root, + block_root: Box::new(new_head_proto_block.root), execution_status: new_head_proto_block.execution_status, }); } @@ -1049,7 +1049,7 @@ fn check_finalized_payload_validity( // Exit now, the node is in an invalid state. return Err(Error::InvalidFinalizedPayload { - finalized_root: finalized_proto_block.root, + finalized_root: Box::new(finalized_proto_block.root), execution_block_hash: block_hash, }); } @@ -1070,8 +1070,8 @@ fn check_against_finality_reversion( Ok(()) } else { Err(Error::RevertedFinalizedEpoch { - old: old_view.finalized_checkpoint, - new: new_view.finalized_checkpoint, + old: Box::new(old_view.finalized_checkpoint), + new: Box::new(new_view.finalized_checkpoint), }) } } diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index b6db3fa84f2..aaca9e1c58e 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -47,8 +47,8 @@ pub enum BeaconChainError { UnableToReadSlot, UnableToComputeTimeAtSlot, RevertedFinalizedEpoch { - old: Checkpoint, - new: Checkpoint, + old: Box, + new: Box, }, SlotClockDidNotStart, NoStateForSlot(Slot), @@ -116,9 +116,9 @@ pub enum BeaconChainError { request_slot: Slot, }, BadPreState { - parent_root: Hash256, + parent_root: Box, parent_slot: Slot, - block_root: Hash256, + block_root: Box, block_slot: Slot, state_slot: Slot, }, @@ -152,8 +152,8 @@ pub enum BeaconChainError { InconsistentPayloadReconstructed { slot: Slot, exec_block_hash: ExecutionBlockHash, - canonical_transactions_root: Hash256, - reconstructed_transactions_root: Hash256, + canonical_transactions_root: Box, + reconstructed_transactions_root: Box, }, BlockStreamerError(BlockStreamerError), AddPayloadLogicError, @@ -168,33 +168,33 @@ pub enum BeaconChainError { BlockRewardSyncError, SyncCommitteeRewardsSyncError, AttestationRewardsError, - HeadMissingFromForkChoice(Hash256), - FinalizedBlockMissingFromForkChoice(Hash256), - HeadBlockMissingFromForkChoice(Hash256), + HeadMissingFromForkChoice(Box), + FinalizedBlockMissingFromForkChoice(Box), + HeadBlockMissingFromForkChoice(Box), InvalidFinalizedPayload { - finalized_root: Hash256, + finalized_root: Box, execution_block_hash: ExecutionBlockHash, }, InvalidFinalizedPayloadShutdownError(TrySendError), JustifiedPayloadInvalid { - justified_root: Hash256, + justified_root: Box, execution_block_hash: Option, }, ForkchoiceUpdate(execution_layer::Error), InvalidCheckpoint { - state_root: Hash256, - checkpoint: Checkpoint, + state_root: Box, + checkpoint: Box, }, InvalidSlot(Slot), HeadBlockNotFullyVerified { - beacon_block_root: Hash256, + beacon_block_root: Box, execution_status: ExecutionStatus, }, CannotAttestToFinalizedBlock { - beacon_block_root: Hash256, + beacon_block_root: Box, }, SyncContributionDataReferencesFinalizedBlock { - beacon_block_root: Hash256, + beacon_block_root: Box, }, RuntimeShutdown, TokioJoin(tokio::task::JoinError), @@ -205,10 +205,10 @@ pub enum BeaconChainError { }, ForkchoiceUpdateParamsMissing, HeadHasInvalidPayload { - block_root: Hash256, + block_root: Box, execution_status: ExecutionStatus, }, - AttestationHeadNotInForkChoice(Hash256), + AttestationHeadNotInForkChoice(Box), MissingPersistedForkChoice, CommitteePromiseFailed(oneshot_broadcast::Error), MaxCommitteePromises(usize), From 10b07a568d14e503370ff1261129f5dea1d91995 Mon Sep 17 00:00:00 2001 From: PoulavBhowmick03 Date: Sat, 21 Jun 2025 13:40:59 +0530 Subject: [PATCH 2/6] Reverted adding Box to BeaconChainError calls throughout beacon_node --- .../src/attestation_verification.rs | 10 ++++----- beacon_node/beacon_chain/src/beacon_chain.rs | 22 +++++++++---------- .../beacon_chain/src/blob_verification.rs | 10 ++++----- .../beacon_chain/src/block_verification.rs | 16 +++++++------- .../src/data_column_verification.rs | 10 ++++----- beacon_node/beacon_chain/src/errors.rs | 2 +- .../beacon_chain/src/execution_payload.rs | 6 ++--- .../beacon_chain/src/fetch_blobs/mod.rs | 2 +- .../beacon_chain/src/state_advance_timer.rs | 2 +- .../src/sync_committee_verification.rs | 8 +++---- .../http_api/src/publish_attestations.rs | 4 ++-- beacon_node/http_api/src/publish_blocks.rs | 15 +++++-------- beacon_node/http_api/src/sync_committees.rs | 16 ++++++-------- beacon_node/http_api/src/validator.rs | 5 ++--- .../gossip_methods.rs | 2 +- .../network_beacon_processor/rpc_methods.rs | 5 ++--- 16 files changed, 64 insertions(+), 71 deletions(-) diff --git a/beacon_node/beacon_chain/src/attestation_verification.rs b/beacon_node/beacon_chain/src/attestation_verification.rs index 2f7c0be229f..b1e8a7d0d38 100644 --- a/beacon_node/beacon_chain/src/attestation_verification.rs +++ b/beacon_node/beacon_chain/src/attestation_verification.rs @@ -267,12 +267,12 @@ pub enum Error { /// /// We were unable to process this attestation due to an internal error. It's unclear if the /// attestation is valid. - BeaconChainError(Box), + BeaconChainError(BeaconChainError), } impl From for Error { fn from(e: BeaconChainError) -> Self { - Self::BeaconChainError(Box::new(e)) + Self::BeaconChainError(e) } } @@ -533,7 +533,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> { .observed_attestations .write() .is_known_subset(attestation, observed_attestation_key_root) - .map_err(|e| Error::BeaconChainError(Box::new(e.into())))? + .map_err(|e| Error::BeaconChainError(e.into()))? { metrics::inc_counter(&metrics::AGGREGATED_ATTESTATION_SUBSETS); return Err(Error::AttestationSupersetKnown( @@ -636,7 +636,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> { if !SelectionProof::from(selection_proof) .is_aggregator(committee.committee.len(), &chain.spec) - .map_err(|e| Error::BeaconChainError(Box::new(e.into())))? + .map_err(|e| Error::BeaconChainError(e.into()))? { return Err(Error::InvalidSelectionProof { aggregator_index }); } @@ -706,7 +706,7 @@ impl<'a, T: BeaconChainTypes> VerifiedAggregatedAttestation<'a, T> { .observed_attestations .write() .observe_item(attestation, Some(observed_attestation_key_root)) - .map_err(|e| Error::BeaconChainError(Box::new(e.into())))? + .map_err(|e| Error::BeaconChainError(e.into()))? { metrics::inc_counter(&metrics::AGGREGATED_ATTESTATION_SUBSETS); return Err(Error::AttestationSupersetKnown( diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index d30094f3475..dd23536240a 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -3477,8 +3477,8 @@ impl BeaconChain { Ok(status) } Err(BlockError::BeaconChainError(e)) => { - match e.as_ref() { - BeaconChainError::TokioJoin(e) => { + match e { + BeaconChainError::TokioJoin(ref e) => { debug!( error = ?e, "Beacon block processing cancelled" @@ -3626,7 +3626,7 @@ impl BeaconChain { header.message.proposer_index, block_root, ) - .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))?; + .map_err(|e| BlockError::BeaconChainError(e.into()))?; if let Some(slasher) = self.slasher.as_ref() { slasher.accept_block_header(header); } @@ -3722,7 +3722,7 @@ impl BeaconChain { header.message.proposer_index, block_root, ) - .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))?; + .map_err(|e| BlockError::BeaconChainError(e.into()))?; if let Some(slasher) = self.slasher.as_ref() { slasher.accept_block_header(header); } @@ -3902,7 +3902,7 @@ impl BeaconChain { payload_verification_status, &self.spec, ) - .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))?; + .map_err(|e| BlockError::BeaconChainError(e.into()))?; } // If the block is recent enough and it was not optimistically imported, check to see if it @@ -4095,7 +4095,7 @@ impl BeaconChain { warning = "The database is likely corrupt now, consider --purge-db", "No stored fork choice found to restore from" ); - Err(BlockError::BeaconChainError(Box::new(e))) + Err(BlockError::BeaconChainError(e)) } else { Ok(()) } @@ -4150,9 +4150,9 @@ impl BeaconChain { Provided block root is not a checkpoint.", )) .map_err(|err| { - BlockError::BeaconChainError(Box::new( + BlockError::BeaconChainError( BeaconChainError::WeakSubjectivtyShutdownError(err), - )) + ) })?; return Err(BlockError::WeakSubjectivityConflict); } @@ -5218,16 +5218,16 @@ impl BeaconChain { .validators() .get(proposer_index as usize) .map(|v| v.pubkey) - .ok_or(BlockProductionError::BeaconChain(Box::new( + .ok_or(BlockProductionError::BeaconChain( BeaconChainError::ValidatorIndexUnknown(proposer_index as usize), - )))?; + ))?; let builder_params = BuilderParams { pubkey, slot: state.slot(), chain_health: self .is_healthy(&parent_root) - .map_err(|e| BlockProductionError::BeaconChain(Box::new(e)))?, + .map_err(|e| BlockProductionError::BeaconChain(e))?, }; // If required, start the process of loading an execution payload from the EL early. This diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index a78224fb708..33a91a6342c 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -42,7 +42,7 @@ pub enum GossipBlobError { /// /// We were unable to process this blob due to an internal error. It's /// unclear if the blob is valid. - BeaconChainError(Box), + BeaconChainError(BeaconChainError), /// The `BlobSidecar` was gossiped over an incorrect subnet. /// @@ -444,7 +444,7 @@ pub fn validate_blob_sidecar_for_gossip( .observed_blob_sidecars .write() .observe_sidecar(blob_sidecar) - .map_err(|e| GossipBlobError::BeaconChainError(Box::new(e.into())))? + .map_err(|e| GossipBlobError::BeaconChainError(e.into()))? { return Err(GossipBlobError::RepeatBlob { proposer: blob_sidecar.block_proposer_index(), diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 335990c06d7..bc2832a340b 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -251,7 +251,7 @@ pub enum BlockError { /// /// We were unable to process this block due to an internal error. It's unclear if the block is /// valid. - BeaconChainError(Box), + BeaconChainError(BeaconChainError), /// There was an error whilst verifying weak subjectivity. This block conflicts with the /// configured weak subjectivity checkpoint and was not imported. /// @@ -994,7 +994,7 @@ impl GossipVerifiedBlock { .observed_slashable .write() .observe_slashable(block.slot(), block.message().proposer_index(), block_root) - .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))?; + .map_err(|e| BlockError::BeaconChainError(e.into()))?; // Now the signature is valid, store the proposal so we don't accept another from this // validator and slot. // @@ -1004,7 +1004,7 @@ impl GossipVerifiedBlock { .observed_block_producers .write() .observe_proposal(block_root, block.message()) - .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))? + .map_err(|e| BlockError::BeaconChainError(e.into()))? { SeenBlock::Slashable => { return Err(BlockError::Slashable); @@ -1313,13 +1313,13 @@ impl ExecutionPendingBlock { .observed_slashable .write() .observe_slashable(block.slot(), block.message().proposer_index(), block_root) - .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))?; + .map_err(|e| BlockError::BeaconChainError(e.into()))?; chain .observed_block_producers .write() .observe_proposal(block_root, block.message()) - .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))?; + .map_err(|e| BlockError::BeaconChainError(e.into()))?; if let Some(parent) = chain .canonical_head @@ -1629,7 +1629,7 @@ impl ExecutionPendingBlock { // Ignore invalid attestations whilst importing attestations from a block. The // block might be very old and therefore the attestations useless to fork choice. Err(ForkChoiceError::InvalidAttestation(_)) => Ok(()), - Err(e) => Err(BlockError::BeaconChainError(Box::new(e.into()))), + Err(e) => Err(BlockError::BeaconChainError(e.into())), }?; } drop(fork_choice); @@ -1720,7 +1720,7 @@ pub fn check_block_is_finalized_checkpoint_or_descendant< if chain .store .block_exists(&block.parent_root()) - .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))? + .map_err(|e| BlockError::BeaconChainError(e.into()))? { Err(BlockError::NotFinalizedDescendant { block_parent_root: block.parent_root(), @@ -1865,7 +1865,7 @@ fn load_parent>( let root = block.parent_root(); let parent_block = chain .get_blinded_block(&block.parent_root()) - .map_err(|e| BlockError::BeaconChainError(Box::new(e)))? + .map_err(|e| BlockError::BeaconChainError(e))? .ok_or_else(|| { // Return a `MissingBeaconBlock` error instead of a `ParentUnknown` error since // we've already checked fork choice for this block. diff --git a/beacon_node/beacon_chain/src/data_column_verification.rs b/beacon_node/beacon_chain/src/data_column_verification.rs index 3009522bf60..1ae5756c882 100644 --- a/beacon_node/beacon_chain/src/data_column_verification.rs +++ b/beacon_node/beacon_chain/src/data_column_verification.rs @@ -32,7 +32,7 @@ pub enum GossipDataColumnError { /// /// We were unable to process this data column due to an internal error. It's /// unclear if the data column is valid. - BeaconChainError(Box), + BeaconChainError(BeaconChainError), /// The proposal signature in invalid. /// /// ## Peer scoring @@ -508,7 +508,7 @@ pub fn validate_data_column_sidecar_for_gossip( .observed_column_sidecars .read() .proposer_is_known(data_column) - .map_err(|e| GossipDataColumnError::BeaconChainError(Box::new(e.into())))? + .map_err(|e| GossipDataColumnError::BeaconChainError(e.into()))? { return Err(GossipDataColumnError::PriorKnown { proposer: data_column.block_proposer_index(), @@ -664,7 +664,7 @@ fn verify_proposer_and_signature( let (parent_state_root, mut parent_state) = chain .store .get_advanced_hot_state(block_parent_root, column_slot, parent_block.state_root) - .map_err(|e| GossipDataColumnError::BeaconChainError(Box::new(e.into())))? + .map_err(|e| GossipDataColumnError::BeaconChainError(e.into()))? .ok_or_else(|| { BeaconChainError::DBInconsistent(format!( "Missing state for parent block {block_parent_root:?}", @@ -797,7 +797,7 @@ pub fn observe_gossip_data_column( .observed_column_sidecars .write() .observe_sidecar(data_column_sidecar) - .map_err(|e| GossipDataColumnError::BeaconChainError(Box::new(e.into())))? + .map_err(|e| GossipDataColumnError::BeaconChainError(e.into()))? { return Err(GossipDataColumnError::PriorKnown { proposer: data_column_sidecar.block_proposer_index(), diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index aaca9e1c58e..c79940568aa 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -294,7 +294,7 @@ pub enum BlockProductionError { MissingExecutionPayload, MissingKzgCommitment(String), TokioJoin(JoinError), - BeaconChain(Box), + BeaconChain(BeaconChainError), InvalidPayloadFork, InvalidBlockVariant(String), KzgError(kzg::Error), diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index aa98310c121..783add42105 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -319,9 +319,9 @@ pub fn validate_execution_payload_for_gossip( .slot_clock .start_of(block.slot()) .map(|d| d.as_secs()) - .ok_or(BlockError::BeaconChainError(Box::new( + .ok_or(BlockError::BeaconChainError( BeaconChainError::UnableToComputeTimeAtSlot, - )))?; + ))?; // The block's execution payload timestamp is correct with respect to the slot if execution_payload.timestamp() != expected_timestamp { @@ -504,7 +504,7 @@ where "prepare_execution_payload_forkchoice_update_params", ) .await - .map_err(|e| BlockProductionError::BeaconChain(Box::new(e)))?; + .map_err(|e| BlockProductionError::BeaconChain(e))?; let suggested_fee_recipient = execution_layer .get_suggested_fee_recipient(proposer_index) diff --git a/beacon_node/beacon_chain/src/fetch_blobs/mod.rs b/beacon_node/beacon_chain/src/fetch_blobs/mod.rs index e02405ddbad..cffbfbfe4f2 100644 --- a/beacon_node/beacon_chain/src/fetch_blobs/mod.rs +++ b/beacon_node/beacon_chain/src/fetch_blobs/mod.rs @@ -54,7 +54,7 @@ pub enum EngineGetBlobsOutput { #[derive(Debug)] pub enum FetchEngineBlobError { BeaconStateError(BeaconStateError), - BeaconChainError(Box), + BeaconChainError(BeaconChainError), BlobProcessingError(BlockError), BlobSidecarError(BlobSidecarError), DataColumnSidecarError(DataColumnSidecarError), diff --git a/beacon_node/beacon_chain/src/state_advance_timer.rs b/beacon_node/beacon_chain/src/state_advance_timer.rs index ad7e31a8f00..bf324fc715f 100644 --- a/beacon_node/beacon_chain/src/state_advance_timer.rs +++ b/beacon_node/beacon_chain/src/state_advance_timer.rs @@ -44,7 +44,7 @@ const MAX_FORK_CHOICE_DISTANCE: u64 = 256; #[derive(Debug)] enum Error { - BeaconChain(Box), + BeaconChain(BeaconChainError), // We don't use the inner value directly, but it's used in the Debug impl. HeadMissingFromSnapshotCache(#[allow(dead_code)] Hash256), BeaconState(#[allow(dead_code)] BeaconStateError), diff --git a/beacon_node/beacon_chain/src/sync_committee_verification.rs b/beacon_node/beacon_chain/src/sync_committee_verification.rs index 768c971f94d..cee2e0f24b1 100644 --- a/beacon_node/beacon_chain/src/sync_committee_verification.rs +++ b/beacon_node/beacon_chain/src/sync_committee_verification.rs @@ -189,7 +189,7 @@ pub enum Error { /// /// We were unable to process this sync committee message due to an internal error. It's unclear if the /// sync committee message is valid. - BeaconChainError(Box), + BeaconChainError(BeaconChainError), /// There was an error whilst processing the sync contribution. It is not known if it is valid or invalid. /// /// ## Peer scoring @@ -334,7 +334,7 @@ impl VerifiedSyncContribution { .observed_sync_contributions .write() .is_known_subset(contribution, contribution_data_root) - .map_err(|e| Error::BeaconChainError(Box::new(e.into())))? + .map_err(|e| Error::BeaconChainError(e.into()))? { metrics::inc_counter(&metrics::SYNC_CONTRIBUTION_SUBSETS); return Err(Error::SyncContributionSupersetKnown(contribution_data_root)); @@ -363,7 +363,7 @@ impl VerifiedSyncContribution { if !selection_proof .is_aggregator::() - .map_err(|e| Error::BeaconChainError(Box::new(e.into())))? + .map_err(|e| Error::BeaconChainError(e.into()))? { return Err(Error::InvalidSelectionProof { aggregator_index }); } @@ -395,7 +395,7 @@ impl VerifiedSyncContribution { .observed_sync_contributions .write() .observe_item(contribution, Some(contribution_data_root)) - .map_err(|e| Error::BeaconChainError(Box::new(e.into())))? + .map_err(|e| Error::BeaconChainError(e.into()))? { metrics::inc_counter(&metrics::SYNC_CONTRIBUTION_SUBSETS); return Err(Error::SyncContributionSupersetKnown(contribution_data_root)); diff --git a/beacon_node/http_api/src/publish_attestations.rs b/beacon_node/http_api/src/publish_attestations.rs index a4fcb27b1db..8746c0ef289 100644 --- a/beacon_node/http_api/src/publish_attestations.rs +++ b/beacon_node/http_api/src/publish_attestations.rs @@ -55,7 +55,7 @@ use types::SingleAttestation; pub enum Error { Validation(AttestationError), Publication, - ForkChoice(#[allow(dead_code)] Box), + ForkChoice(#[allow(dead_code)] BeaconChainError), AggregationPool(#[allow(dead_code)] AttestationError), ReprocessDisabled, ReprocessFull, @@ -115,7 +115,7 @@ fn verify_and_publish_attestation( } if let Err(e) = fc_result { - Err(Error::ForkChoice(Box::new(e))) + Err(Error::ForkChoice(e)) } else if let Err(e) = naive_aggregation_result { Err(Error::AggregationPool(e)) } else { diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index 75979bbb1d7..ef548a305fe 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -123,9 +123,8 @@ pub async fn publish_block>( "Signed block published to network via HTTP API" ); - crate::publish_pubsub_message(&sender, PubsubMessage::BeaconBlock(block.clone())).map_err( - |_| BlockError::BeaconChainError(Box::new(BeaconChainError::UnableToPublish)), - )?; + crate::publish_pubsub_message(&sender, PubsubMessage::BeaconBlock(block.clone())) + .map_err(|_| BlockError::BeaconChainError(BeaconChainError::UnableToPublish))?; Ok(()) }; @@ -510,7 +509,7 @@ fn publish_blob_sidecars( ) -> Result<(), BlockError> { let pubsub_message = PubsubMessage::BlobSidecar(Box::new((blob.index(), blob.clone_blob()))); crate::publish_pubsub_message(sender_clone, pubsub_message) - .map_err(|_| BlockError::BeaconChainError(Box::new(BeaconChainError::UnableToPublish))) + .map_err(|_| BlockError::BeaconChainError(BeaconChainError::UnableToPublish)) } fn publish_column_sidecars( @@ -540,7 +539,7 @@ fn publish_column_sidecars( }) .collect::>(); crate::publish_pubsub_messages(sender_clone, pubsub_messages) - .map_err(|_| BlockError::BeaconChainError(Box::new(BeaconChainError::UnableToPublish))) + .map_err(|_| BlockError::BeaconChainError(BeaconChainError::UnableToPublish)) } async fn post_block_import_logging_and_response( @@ -597,9 +596,7 @@ async fn post_block_import_logging_and_response( Err(warp_utils::reject::custom_bad_request(msg)) } } - Err(BlockError::BeaconChainError(e)) - if matches!(e.as_ref(), BeaconChainError::UnableToPublish) => - { + Err(BlockError::BeaconChainError(e)) if matches!(e, BeaconChainError::UnableToPublish) => { Err(warp_utils::reject::custom_server_error( "unable to publish to network channel".to_string(), )) @@ -795,7 +792,7 @@ fn check_slashable( block_clone.message().proposer_index(), block_root, ) - .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))? + .map_err(|e| BlockError::BeaconChainError(e.into()))? { warn!( slot = %block_clone.slot(), diff --git a/beacon_node/http_api/src/sync_committees.rs b/beacon_node/http_api/src/sync_committees.rs index aa126bbc822..61cb4e96ff6 100644 --- a/beacon_node/http_api/src/sync_committees.rs +++ b/beacon_node/http_api/src/sync_committees.rs @@ -59,7 +59,7 @@ pub fn sync_committee_duties( } let duties = duties_from_state_load(request_epoch, request_indices, altair_fork_epoch, chain) - .map_err(|e| match *e { + .map_err(|e| match e { BeaconChainError::SyncDutiesError(BeaconStateError::SyncCommitteeNotKnown { current_epoch, .. @@ -81,7 +81,7 @@ fn duties_from_state_load( request_indices: &[u64], altair_fork_epoch: Epoch, chain: &BeaconChain, -) -> Result, BeaconStateError>>, Box> { +) -> Result, BeaconStateError>>, BeaconChainError> { // Determine what the current epoch would be if we fast-forward our system clock by // `MAXIMUM_GOSSIP_CLOCK_DISPARITY`. // @@ -92,17 +92,16 @@ fn duties_from_state_load( let tolerant_current_epoch = chain .slot_clock .now_with_future_tolerance(chain.spec.maximum_gossip_clock_disparity()) - .ok_or(BeaconChainError::UnableToReadSlot) - .map_err(Box::new)? + .ok_or(BeaconChainError::UnableToReadSlot)? .epoch(T::EthSpec::slots_per_epoch()); let max_sync_committee_period = tolerant_current_epoch .sync_committee_period(&chain.spec) - .map_err(|e| Box::new(e.into()))? + .map_err(|e| e)? + 1; let sync_committee_period = request_epoch .sync_committee_period(&chain.spec) - .map_err(|e| Box::new(e.into()))?; + .map_err(|e| e)?; if tolerant_current_epoch < altair_fork_epoch { // Empty response if the epoch is pre-Altair. @@ -125,14 +124,13 @@ fn duties_from_state_load( state .get_sync_committee_duties(request_epoch, request_indices, &chain.spec) .map_err(BeaconChainError::SyncDutiesError) - .map_err(Box::new) } else { - Err(Box::new(BeaconChainError::SyncDutiesError( + Err(BeaconChainError::SyncDutiesError( BeaconStateError::SyncCommitteeNotKnown { current_epoch, epoch: request_epoch, }, - ))) + )) } } diff --git a/beacon_node/http_api/src/validator.rs b/beacon_node/http_api/src/validator.rs index 25b0feb99e8..baa41e33ed5 100644 --- a/beacon_node/http_api/src/validator.rs +++ b/beacon_node/http_api/src/validator.rs @@ -7,10 +7,9 @@ pub fn pubkey_to_validator_index( chain: &BeaconChain, state: &BeaconState, pubkey: &PublicKeyBytes, -) -> Result, Box> { +) -> Result, BeaconChainError> { chain - .validator_index(pubkey) - .map_err(Box::new)? + .validator_index(pubkey)? .filter(|&index| { state .validators() 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 fcab34c93e6..95b9669328e 100644 --- a/beacon_node/network/src/network_beacon_processor/gossip_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/gossip_methods.rs @@ -2628,7 +2628,7 @@ impl NetworkBeaconProcessor { ); } AttnError::BeaconChainError(e) => { - match e.as_ref() { + match e { BeaconChainError::DBError(Error::HotColdDBError( HotColdDBError::FinalizedStateNotInHotDatabase { .. }, )) => { diff --git a/beacon_node/network/src/network_beacon_processor/rpc_methods.rs b/beacon_node/network/src/network_beacon_processor/rpc_methods.rs index 4004305f83c..0dd01788b7b 100644 --- a/beacon_node/network/src/network_beacon_processor/rpc_methods.rs +++ b/beacon_node/network/src/network_beacon_processor/rpc_methods.rs @@ -66,7 +66,7 @@ impl NetworkBeaconProcessor { fn check_peer_relevance( &self, remote: &StatusMessage, - ) -> Result, Box> { + ) -> Result, BeaconChainError> { let local = self.chain.status_message(); let start_slot = |epoch: Epoch| epoch.start_slot(T::EthSpec::slots_per_epoch()); @@ -112,8 +112,7 @@ impl NetworkBeaconProcessor { if self .chain .block_root_at_slot(remote_finalized_slot, WhenSlotSkipped::Prev) - .map(|root_opt| root_opt != Some(*remote.finalized_root())) - .map_err(Box::new)? + .map(|root_opt| root_opt != Some(*remote.finalized_root()))? { Some("Different finalized chain".to_string()) } else { From 0af22230139be90e704d9df297d8ee0733b8b12b Mon Sep 17 00:00:00 2001 From: PoulavBhowmick03 Date: Sun, 22 Jun 2025 12:39:53 +0530 Subject: [PATCH 3/6] Boxed more variants in BeaconChainError and fixed errors --- .../beacon_chain/src/attestation_rewards.rs | 2 +- beacon_node/beacon_chain/src/beacon_chain.rs | 39 ++++++++++--------- .../beacon_chain/src/blob_verification.rs | 4 +- .../beacon_chain/src/block_verification.rs | 20 +++++----- .../beacon_chain/src/canonical_head.rs | 14 +++++-- .../src/data_column_verification.rs | 4 +- beacon_node/beacon_chain/src/errors.rs | 22 +++++------ .../beacon_chain/src/execution_payload.rs | 2 +- .../fetch_blobs/fetch_blobs_beacon_adapter.rs | 2 +- .../beacon_chain/src/fetch_blobs/mod.rs | 2 +- .../src/light_client_server_cache.rs | 18 +++++---- .../beacon_chain/src/state_advance_timer.rs | 2 +- .../src/sync_committee_verification.rs | 2 +- .../tests/payload_invalidation.rs | 4 +- .../http_api/src/attestation_performance.rs | 15 ++++--- .../http_api/src/block_packing_efficiency.rs | 10 +++-- beacon_node/http_api/src/block_rewards.rs | 4 +- beacon_node/http_api/src/light_client.rs | 4 +- beacon_node/http_api/src/publish_blocks.rs | 28 +++++++------ .../http_api/src/sync_committee_rewards.rs | 8 ++-- beacon_node/http_api/src/sync_committees.rs | 9 +---- .../src/network_beacon_processor/mod.rs | 6 +-- 22 files changed, 123 insertions(+), 98 deletions(-) diff --git a/beacon_node/beacon_chain/src/attestation_rewards.rs b/beacon_node/beacon_chain/src/attestation_rewards.rs index 7d88268cf9f..3e7c5102d33 100644 --- a/beacon_node/beacon_chain/src/attestation_rewards.rs +++ b/beacon_node/beacon_chain/src/attestation_rewards.rs @@ -55,7 +55,7 @@ impl BeaconChain { // to cache states so that future calls are faster. let state = self .get_state(&state_root, Some(state_slot), true)? - .ok_or(BeaconChainError::MissingBeaconState(state_root))?; + .ok_or(BeaconChainError::MissingBeaconState(Box::new(state_root)))?; if state.fork_name_unchecked().altair_enabled() { self.compute_attestation_rewards_altair(state, validators) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index dd23536240a..d202842df6f 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -768,12 +768,12 @@ impl BeaconChain { ) -> Result> + '_, Error> { let block = self .get_blinded_block(&block_root)? - .ok_or(Error::MissingBeaconBlock(block_root))?; + .ok_or(Error::MissingBeaconBlock(Box::new(block_root)))?; // This method is only used in tests, so we may as well cache states to make CI go brr. // TODO(release-v7) move this method out of beacon chain and into `store_tests`` or something equivalent. let state = self .get_state(&block.state_root(), Some(block.slot()), true)? - .ok_or_else(|| Error::MissingBeaconState(block.state_root()))?; + .ok_or_else(|| Error::MissingBeaconState(Box::new(block.state_root())))?; let iter = BlockRootsIterator::owned(&self.store, state); Ok(std::iter::once(Ok((block_root, block.slot()))) .chain(iter) @@ -1237,7 +1237,7 @@ impl BeaconChain { if reconstruction_possible { reconstruct_blobs(&self.kzg, &columns, None, &block, &self.spec) .map(Some) - .map_err(Error::FailedToReconstructBlobs) + .map_err(|e| Error::FailedToReconstructBlobs(Box::new(e))) } else { Err(Error::InsufficientColumnsToReconstructBlobs { columns_found: columns.len(), @@ -1683,7 +1683,9 @@ impl BeaconChain { .store .load_hot_state_summary(&state_root) .map_err(BeaconChainError::DBError)? - .ok_or(BeaconChainError::MissingHotStateSummary(state_root))?; + .ok_or(BeaconChainError::MissingHotStateSummary(Box::new( + state_root, + )))?; if slot != checkpoint.epoch.start_slot(T::EthSpec::slots_per_epoch()) || latest_block_root != *checkpoint.root @@ -2859,7 +2861,7 @@ impl BeaconChain { Err(error) => { return ChainSegmentResult::Failed { imported_blocks, - error: BlockError::BeaconChainError(error.into()), + error: BlockError::BeaconChainError(error), } } }; @@ -2898,7 +2900,7 @@ impl BeaconChain { Err(error) => { return ChainSegmentResult::Failed { imported_blocks, - error: BlockError::BeaconChainError(error.into()), + error: BlockError::BeaconChainError(error), }; } }; @@ -4818,11 +4820,11 @@ impl BeaconChain { } else { let block = self .get_blinded_block(&parent_block_root)? - .ok_or(Error::MissingBeaconBlock(parent_block_root))?; + .ok_or(Error::MissingBeaconBlock(Box::new(parent_block_root)))?; let (state_root, state) = self .store .get_advanced_hot_state(parent_block_root, proposal_slot, block.state_root())? - .ok_or(Error::MissingBeaconState(block.state_root()))?; + .ok_or(Error::MissingBeaconState(Box::new(block.state_root())))?; (Cow::Owned(state), state_root) }; @@ -4833,7 +4835,7 @@ impl BeaconChain { if head_state.current_epoch() == proposal_epoch { return get_expected_withdrawals(&unadvanced_state, &self.spec) .map(|(withdrawals, _)| withdrawals) - .map_err(Error::PrepareProposerFailed); + .map_err(|e| Error::PrepareProposerFailed(Box::new(e))); } // Advance the state using the partial method. @@ -4851,7 +4853,7 @@ impl BeaconChain { )?; get_expected_withdrawals(&advanced_state, &self.spec) .map(|(withdrawals, _)| withdrawals) - .map_err(Error::PrepareProposerFailed) + .map_err(|e| Error::PrepareProposerFailed(Box::new(e))) } /// Determine whether a fork choice update to the execution layer should be overridden. @@ -4909,7 +4911,7 @@ impl BeaconChain { &self.config.re_org_disallowed_offsets, self.config.re_org_max_epochs_since_finalization, ) - .map_err(|e| e.map_inner_error(Error::ProposerHeadForkChoiceError))?; + .map_err(|e| e.map_inner_error(|e| Error::ProposerHeadForkChoiceError(Box::new(e))))?; // The slot of our potential re-org block is always 1 greater than the head block because we // only attempt single-slot re-orgs. @@ -5227,7 +5229,7 @@ impl BeaconChain { slot: state.slot(), chain_health: self .is_healthy(&parent_root) - .map_err(|e| BlockProductionError::BeaconChain(e))?, + .map_err(BlockProductionError::BeaconChain)?, }; // If required, start the process of loading an execution payload from the EL early. This @@ -6196,11 +6198,12 @@ impl BeaconChain { .await { // We are a proposer, check for terminal_pow_block_hash - if let Some(terminal_pow_block_hash) = execution_layer + let terminal_pow_block_hash_opt = execution_layer .get_terminal_pow_block_hash(&self.spec, payload_attributes.timestamp()) .await - .map_err(Error::ForkchoiceUpdate)? - { + .map_err(|e| Error::ForkchoiceUpdate(Box::new(e)))?; + + if let Some(terminal_pow_block_hash) = terminal_pow_block_hash_opt { info!( slot = %next_slot, "Prepared POS transition block proposer" @@ -6237,7 +6240,7 @@ impl BeaconChain { head_block_root, ) .await - .map_err(Error::ExecutionForkChoiceUpdateFailed); + .map_err(|e| Error::ExecutionForkChoiceUpdateFailed(Box::new(e))); // The head has been read and the execution layer has been updated. It is now valid to send // another fork choice update. @@ -6591,7 +6594,7 @@ impl BeaconChain { .canonical_head .fork_choice_read_lock() .get_block(&head_block_root) - .ok_or(Error::MissingBeaconBlock(head_block_root))?; + .ok_or(Error::MissingBeaconBlock(Box::new(head_block_root)))?; let shuffling_id = BlockShufflingIds { current: head_block.current_epoch_shuffling_id.clone(), @@ -6688,7 +6691,7 @@ impl BeaconChain { let (state_root, state) = self .store .get_advanced_hot_state(head_block_root, target_slot, head_block.state_root)? - .ok_or(Error::MissingBeaconState(head_block.state_root))?; + .ok_or(Error::MissingBeaconState(Box::new(head_block.state_root)))?; (state, state_root) }; diff --git a/beacon_node/beacon_chain/src/blob_verification.rs b/beacon_node/beacon_chain/src/blob_verification.rs index 33a91a6342c..617b9ae1e2a 100644 --- a/beacon_node/beacon_chain/src/blob_verification.rs +++ b/beacon_node/beacon_chain/src/blob_verification.rs @@ -147,13 +147,13 @@ impl std::fmt::Display for GossipBlobError { impl From for GossipBlobError { fn from(e: BeaconChainError) -> Self { - GossipBlobError::BeaconChainError(e.into()) + GossipBlobError::BeaconChainError(e) } } impl From for GossipBlobError { fn from(e: BeaconStateError) -> Self { - GossipBlobError::BeaconChainError(BeaconChainError::BeaconStateError(e).into()) + GossipBlobError::BeaconChainError(BeaconChainError::BeaconStateError(e)) } } diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index bc2832a340b..7c5ed5e071e 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -474,40 +474,38 @@ impl From for BlockError { block, local_shuffling, }, - e => BlockError::BeaconChainError( - BeaconChainError::BlockSignatureVerifierError(e).into(), - ), + e => BlockError::BeaconChainError(BeaconChainError::BlockSignatureVerifierError(e)), } } } impl From for BlockError { fn from(e: BeaconChainError) -> Self { - BlockError::BeaconChainError(e.into()) + BlockError::BeaconChainError(e) } } impl From for BlockError { fn from(e: BeaconStateError) -> Self { - BlockError::BeaconChainError(BeaconChainError::BeaconStateError(e).into()) + BlockError::BeaconChainError(BeaconChainError::BeaconStateError(e)) } } impl From for BlockError { fn from(e: SlotProcessingError) -> Self { - BlockError::BeaconChainError(BeaconChainError::SlotProcessingError(e).into()) + BlockError::BeaconChainError(BeaconChainError::SlotProcessingError(e)) } } impl From for BlockError { fn from(e: DBError) -> Self { - BlockError::BeaconChainError(BeaconChainError::DBError(e).into()) + BlockError::BeaconChainError(BeaconChainError::DBError(e)) } } impl From for BlockError { fn from(e: ArithError) -> Self { - BlockError::BeaconChainError(BeaconChainError::ArithError(e).into()) + BlockError::BeaconChainError(BeaconChainError::ArithError(e)) } } @@ -1865,14 +1863,16 @@ fn load_parent>( let root = block.parent_root(); let parent_block = chain .get_blinded_block(&block.parent_root()) - .map_err(|e| BlockError::BeaconChainError(e))? + .map_err(BlockError::BeaconChainError)? .ok_or_else(|| { // Return a `MissingBeaconBlock` error instead of a `ParentUnknown` error since // we've already checked fork choice for this block. // // It's an internal error if the block exists in fork choice but not in the // database. - BlockError::from(BeaconChainError::MissingBeaconBlock(block.parent_root())) + BlockError::from(BeaconChainError::MissingBeaconBlock(Box::new( + block.parent_root(), + ))) })?; // Load the parent block's state from the database, returning an error if it is not found. diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index 3b61a190381..17607922300 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -295,11 +295,13 @@ impl CanonicalHead { let beacon_block_root = fork_choice_view.head_block_root; let beacon_block = store .get_full_block(&beacon_block_root)? - .ok_or(Error::MissingBeaconBlock(beacon_block_root))?; + .ok_or(Error::MissingBeaconBlock(Box::new(beacon_block_root)))?; let current_slot = fork_choice.fc_store().get_current_slot(); let (_, beacon_state) = store .get_advanced_hot_state(beacon_block_root, current_slot, beacon_block.state_root())? - .ok_or(Error::MissingBeaconState(beacon_block.state_root()))?; + .ok_or(Error::MissingBeaconState(Box::new( + beacon_block.state_root(), + )))?; let snapshot = BeaconSnapshot { beacon_block_root, @@ -644,7 +646,9 @@ impl BeaconChain { let beacon_block = self .store .get_full_block(&new_view.head_block_root)? - .ok_or(Error::MissingBeaconBlock(new_view.head_block_root))?; + .ok_or(Error::MissingBeaconBlock(Box::new( + new_view.head_block_root, + )))?; let (_, beacon_state) = self .store @@ -653,7 +657,9 @@ impl BeaconChain { current_slot, beacon_block.state_root(), )? - .ok_or(Error::MissingBeaconState(beacon_block.state_root()))?; + .ok_or(Error::MissingBeaconState(Box::new( + beacon_block.state_root(), + )))?; BeaconSnapshot { beacon_block: Arc::new(beacon_block), diff --git a/beacon_node/beacon_chain/src/data_column_verification.rs b/beacon_node/beacon_chain/src/data_column_verification.rs index 1ae5756c882..4697a4ee6bf 100644 --- a/beacon_node/beacon_chain/src/data_column_verification.rs +++ b/beacon_node/beacon_chain/src/data_column_verification.rs @@ -166,13 +166,13 @@ pub enum GossipDataColumnError { impl From for GossipDataColumnError { fn from(e: BeaconChainError) -> Self { - GossipDataColumnError::BeaconChainError(e.into()) + GossipDataColumnError::BeaconChainError(e) } } impl From for GossipDataColumnError { fn from(e: BeaconStateError) -> Self { - GossipDataColumnError::BeaconChainError(BeaconChainError::BeaconStateError(e).into()) + GossipDataColumnError::BeaconChainError(BeaconChainError::BeaconStateError(e)) } } diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index c79940568aa..d5fd22a9c20 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -58,15 +58,15 @@ pub enum BeaconChainError { DBError(store::Error), ForkChoiceError(ForkChoiceError), ForkChoiceStoreError(ForkChoiceStoreError), - MissingBeaconBlock(Hash256), - MissingBeaconState(Hash256), - MissingHotStateSummary(Hash256), + MissingBeaconBlock(Box), + MissingBeaconState(Box), + MissingHotStateSummary(Box), SlotProcessingError(SlotProcessingError), EpochProcessingError(EpochProcessingError), StateAdvanceError(StateAdvanceError), UnableToAdvanceState(String), NoStateForAttestation { - beacon_block_root: Hash256, + beacon_block_root: Box, }, CannotAttestToFutureState, AttestationValidationError(AttestationValidationError), @@ -157,8 +157,8 @@ pub enum BeaconChainError { }, BlockStreamerError(BlockStreamerError), AddPayloadLogicError, - ExecutionForkChoiceUpdateFailed(execution_layer::Error), - PrepareProposerFailed(BlockProcessingError), + ExecutionForkChoiceUpdateFailed(Box), + PrepareProposerFailed(Box), ExecutionForkChoiceUpdateInvalid { status: PayloadStatus, }, @@ -180,7 +180,7 @@ pub enum BeaconChainError { justified_root: Box, execution_block_hash: Option, }, - ForkchoiceUpdate(execution_layer::Error), + ForkchoiceUpdate(Box), InvalidCheckpoint { state_root: Box, checkpoint: Box, @@ -215,12 +215,12 @@ pub enum BeaconChainError { BlsToExecutionPriorToCapella, BlsToExecutionConflictsWithPool, InconsistentFork(InconsistentFork), - ProposerHeadForkChoiceError(fork_choice::Error), + ProposerHeadForkChoiceError(Box>), UnableToPublish, - UnableToBuildColumnSidecar(String), + UnableToBuildColumnSidecar(Box), AvailabilityCheckError(AvailabilityCheckError), LightClientUpdateError(LightClientUpdateError), - LightClientBootstrapError(String), + LightClientBootstrapError(Box), UnsupportedFork, MilhouseError(MilhouseError), EmptyRpcCustodyColumns, @@ -229,7 +229,7 @@ pub enum BeaconChainError { InsufficientColumnsToReconstructBlobs { columns_found: usize, }, - FailedToReconstructBlobs(String), + FailedToReconstructBlobs(Box), } easy_from_to!(SlotProcessingError, BeaconChainError); diff --git a/beacon_node/beacon_chain/src/execution_payload.rs b/beacon_node/beacon_chain/src/execution_payload.rs index 783add42105..1da8cb413b5 100644 --- a/beacon_node/beacon_chain/src/execution_payload.rs +++ b/beacon_node/beacon_chain/src/execution_payload.rs @@ -504,7 +504,7 @@ where "prepare_execution_payload_forkchoice_update_params", ) .await - .map_err(|e| BlockProductionError::BeaconChain(e))?; + .map_err(BlockProductionError::BeaconChain)?; let suggested_fee_recipient = execution_layer .get_suggested_fee_recipient(proposer_index) diff --git a/beacon_node/beacon_chain/src/fetch_blobs/fetch_blobs_beacon_adapter.rs b/beacon_node/beacon_chain/src/fetch_blobs/fetch_blobs_beacon_adapter.rs index 4a7a5aeea21..77ec3f81310 100644 --- a/beacon_node/beacon_chain/src/fetch_blobs/fetch_blobs_beacon_adapter.rs +++ b/beacon_node/beacon_chain/src/fetch_blobs/fetch_blobs_beacon_adapter.rs @@ -110,7 +110,7 @@ impl FetchBlobsBeaconAdapter { self.chain .process_engine_blobs(slot, block_root, blobs) .await - .map_err(FetchEngineBlobError::BlobProcessingError) + .map_err(|e| FetchEngineBlobError::BlobProcessingError(Box::new(e))) } pub(crate) fn fork_choice_contains_block(&self, block_root: &Hash256) -> bool { diff --git a/beacon_node/beacon_chain/src/fetch_blobs/mod.rs b/beacon_node/beacon_chain/src/fetch_blobs/mod.rs index cffbfbfe4f2..e4b1f4504cd 100644 --- a/beacon_node/beacon_chain/src/fetch_blobs/mod.rs +++ b/beacon_node/beacon_chain/src/fetch_blobs/mod.rs @@ -55,7 +55,7 @@ pub enum EngineGetBlobsOutput { pub enum FetchEngineBlobError { BeaconStateError(BeaconStateError), BeaconChainError(BeaconChainError), - BlobProcessingError(BlockError), + BlobProcessingError(Box), BlobSidecarError(BlobSidecarError), DataColumnSidecarError(DataColumnSidecarError), ExecutionLayerMissing, diff --git a/beacon_node/beacon_chain/src/light_client_server_cache.rs b/beacon_node/beacon_chain/src/light_client_server_cache.rs index b7b6d1df18a..eb34a8c5922 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -357,8 +357,8 @@ impl LightClientServerCache { chain_spec: &ChainSpec, ) -> Result, ForkName)>, BeaconChainError> { let Some(block) = store.get_blinded_block(block_root)? else { - return Err(BeaconChainError::LightClientBootstrapError(format!( - "Block root {block_root} not found" + return Err(BeaconChainError::LightClientBootstrapError(Box::new( + format!("Block root {block_root} not found"), ))); }; @@ -373,21 +373,23 @@ impl LightClientServerCache { let Some(current_sync_committee_branch) = store.get_sync_committee_branch(block_root)? else { - return Err(BeaconChainError::LightClientBootstrapError(format!( - "Sync committee branch for block root {:?} not found", - block_root + return Err(BeaconChainError::LightClientBootstrapError(Box::new( + format!( + "Sync committee branch for block root {:?} not found", + block_root + ), ))); }; if sync_committee_period > finalized_period { return Err(BeaconChainError::LightClientBootstrapError( - format!("The blocks sync committee period {sync_committee_period} is greater than the current finalized period {finalized_period}"), + Box::new(format!("The blocks sync committee period {sync_committee_period} is greater than the current finalized period {finalized_period}")), )); } let Some(current_sync_committee) = store.get_sync_committee(sync_committee_period)? else { - return Err(BeaconChainError::LightClientBootstrapError(format!( - "Sync committee for period {sync_committee_period} not found" + return Err(BeaconChainError::LightClientBootstrapError(Box::new( + format!("Sync committee for period {sync_committee_period} not found"), ))); }; diff --git a/beacon_node/beacon_chain/src/state_advance_timer.rs b/beacon_node/beacon_chain/src/state_advance_timer.rs index bf324fc715f..82df2a7c0bd 100644 --- a/beacon_node/beacon_chain/src/state_advance_timer.rs +++ b/beacon_node/beacon_chain/src/state_advance_timer.rs @@ -64,7 +64,7 @@ enum Error { impl From for Error { fn from(e: BeaconChainError) -> Self { - Self::BeaconChain(e.into()) + Self::BeaconChain(e) } } diff --git a/beacon_node/beacon_chain/src/sync_committee_verification.rs b/beacon_node/beacon_chain/src/sync_committee_verification.rs index cee2e0f24b1..e1a5de56d1c 100644 --- a/beacon_node/beacon_chain/src/sync_committee_verification.rs +++ b/beacon_node/beacon_chain/src/sync_committee_verification.rs @@ -232,7 +232,7 @@ pub enum Error { impl From for Error { fn from(e: BeaconChainError) -> Self { - Error::BeaconChainError(e.into()) + Error::BeaconChainError(e) } } diff --git a/beacon_node/beacon_chain/tests/payload_invalidation.rs b/beacon_node/beacon_chain/tests/payload_invalidation.rs index 05fae7aa70f..2f7ae5c1b60 100644 --- a/beacon_node/beacon_chain/tests/payload_invalidation.rs +++ b/beacon_node/beacon_chain/tests/payload_invalidation.rs @@ -509,7 +509,7 @@ async fn justified_checkpoint_becomes_invalid() { }; rig.import_block_parametric(is_valid, is_valid, None, |error| match error { BlockError::BeaconChainError(e) => { - matches!(e.as_ref(), BeaconChainError::JustifiedPayloadInvalid { .. }) + matches!(e, BeaconChainError::JustifiedPayloadInvalid { .. }) } _ => false, }) @@ -1244,7 +1244,7 @@ async fn attesting_to_optimistic_head() { beacon_block_root, execution_status }) - if beacon_block_root == root && matches!(execution_status, ExecutionStatus::Optimistic(_)) + if *beacon_block_root == root && matches!(execution_status, ExecutionStatus::Optimistic(_)) )); } } diff --git a/beacon_node/http_api/src/attestation_performance.rs b/beacon_node/http_api/src/attestation_performance.rs index 23ab5e3752b..06c3f04eb8d 100644 --- a/beacon_node/http_api/src/attestation_performance.rs +++ b/beacon_node/http_api/src/attestation_performance.rs @@ -111,7 +111,9 @@ pub fn get_attestation_performance( let first_block = chain .get_blinded_block(first_block_root) .and_then(|maybe_block| { - maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(*first_block_root)) + maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(Box::new( + *first_block_root, + ))) }) .map_err(unhandled_error)?; @@ -119,8 +121,9 @@ pub fn get_attestation_performance( let prior_block = chain .get_blinded_block(&first_block.parent_root()) .and_then(|maybe_block| { - maybe_block - .ok_or_else(|| BeaconChainError::MissingBeaconBlock(first_block.parent_root())) + maybe_block.ok_or_else(|| { + BeaconChainError::MissingBeaconBlock(Box::new(first_block.parent_root())) + }) }) .map_err(unhandled_error)?; @@ -131,7 +134,9 @@ pub fn get_attestation_performance( // to cache states so that future calls are faster. let state = chain .get_state(&state_root, Some(prior_slot), true) - .and_then(|maybe_state| maybe_state.ok_or(BeaconChainError::MissingBeaconState(state_root))) + .and_then(|maybe_state| { + maybe_state.ok_or(BeaconChainError::MissingBeaconState(Box::new(state_root))) + }) .map_err(unhandled_error)?; // Allocate an AttestationPerformance vector for each validator in the range. @@ -199,7 +204,7 @@ pub fn get_attestation_performance( chain .get_blinded_block(root) .and_then(|maybe_block| { - maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(*root)) + maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(Box::new(*root))) }) .map_err(unhandled_error) }) diff --git a/beacon_node/http_api/src/block_packing_efficiency.rs b/beacon_node/http_api/src/block_packing_efficiency.rs index 249a6732dcb..f645ff27d8d 100644 --- a/beacon_node/http_api/src/block_packing_efficiency.rs +++ b/beacon_node/http_api/src/block_packing_efficiency.rs @@ -278,7 +278,9 @@ pub fn get_block_packing_efficiency( let first_block = chain .get_blinded_block(first_block_root) .and_then(|maybe_block| { - maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(*first_block_root)) + maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(Box::new( + *first_block_root, + ))) }) .map_err(unhandled_error)?; @@ -290,7 +292,9 @@ pub fn get_block_packing_efficiency( let starting_state = chain .get_state(&starting_state_root, Some(prior_slot), true) .and_then(|maybe_state| { - maybe_state.ok_or(BeaconChainError::MissingBeaconState(starting_state_root)) + maybe_state.ok_or(BeaconChainError::MissingBeaconState(Box::new( + starting_state_root, + ))) }) .map_err(unhandled_error)?; @@ -392,7 +396,7 @@ pub fn get_block_packing_efficiency( chain .get_blinded_block(root) .and_then(|maybe_block| { - maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(*root)) + maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(Box::new(*root))) }) .map_err(unhandled_error) }) diff --git a/beacon_node/http_api/src/block_rewards.rs b/beacon_node/http_api/src/block_rewards.rs index 29b23e89a79..e4c85609571 100644 --- a/beacon_node/http_api/src/block_rewards.rs +++ b/beacon_node/http_api/src/block_rewards.rs @@ -46,7 +46,9 @@ pub fn get_block_rewards( // to cache states so that future calls are faster. let mut state = chain .get_state(&state_root, Some(prior_slot), true) - .and_then(|maybe_state| maybe_state.ok_or(BeaconChainError::MissingBeaconState(state_root))) + .and_then(|maybe_state| { + maybe_state.ok_or(BeaconChainError::MissingBeaconState(Box::new(state_root))) + }) .map_err(unhandled_error)?; state diff --git a/beacon_node/http_api/src/light_client.rs b/beacon_node/http_api/src/light_client.rs index 24b1338a724..d0aa0dc3afa 100644 --- a/beacon_node/http_api/src/light_client.rs +++ b/beacon_node/http_api/src/light_client.rs @@ -71,9 +71,9 @@ pub fn get_light_client_bootstrap( println!("{:?}", err); err } else { - "No LightClientBootstrap found".to_string() + Box::new("No LightClientBootstrap found".to_string()) }; - warp_utils::reject::custom_not_found(error_message) + warp_utils::reject::custom_not_found(*error_message) })? .ok_or(warp_utils::reject::custom_not_found( "No LightClientBootstrap found".to_string(), diff --git a/beacon_node/http_api/src/publish_blocks.rs b/beacon_node/http_api/src/publish_blocks.rs index ef548a305fe..158720557c6 100644 --- a/beacon_node/http_api/src/publish_blocks.rs +++ b/beacon_node/http_api/src/publish_blocks.rs @@ -170,7 +170,7 @@ pub async fn publish_block>( seen_timestamp, )?, BroadcastValidation::ConsensusAndEquivocation => { - check_slashable(&chain, block_root, &block_to_publish)?; + check_slashable(&chain, block_root, &block_to_publish).map_err(|e| *e)?; publish_block_p2p( block_to_publish.clone(), sender_clone.clone(), @@ -506,17 +506,20 @@ fn build_gossip_verified_blobs( fn publish_blob_sidecars( sender_clone: &UnboundedSender>, blob: &GossipVerifiedBlob, -) -> Result<(), BlockError> { +) -> Result<(), Box> { let pubsub_message = PubsubMessage::BlobSidecar(Box::new((blob.index(), blob.clone_blob()))); - crate::publish_pubsub_message(sender_clone, pubsub_message) - .map_err(|_| BlockError::BeaconChainError(BeaconChainError::UnableToPublish)) + crate::publish_pubsub_message(sender_clone, pubsub_message).map_err(|_| { + Box::new(BlockError::BeaconChainError( + BeaconChainError::UnableToPublish, + )) + }) } fn publish_column_sidecars( sender_clone: &UnboundedSender>, data_column_sidecars: &[Option>], chain: &BeaconChain, -) -> Result<(), BlockError> { +) -> Result<(), Box> { let malicious_withhold_count = chain.config.malicious_withhold_count; let mut data_column_sidecars = data_column_sidecars .iter() @@ -538,8 +541,11 @@ fn publish_column_sidecars( PubsubMessage::DataColumnSidecar(Box::new((subnet, data_col))) }) .collect::>(); - crate::publish_pubsub_messages(sender_clone, pubsub_messages) - .map_err(|_| BlockError::BeaconChainError(BeaconChainError::UnableToPublish)) + crate::publish_pubsub_messages(sender_clone, pubsub_messages).map_err(|_| { + Box::new(BlockError::BeaconChainError( + BeaconChainError::UnableToPublish, + )) + }) } async fn post_block_import_logging_and_response( @@ -596,7 +602,7 @@ async fn post_block_import_logging_and_response( Err(warp_utils::reject::custom_bad_request(msg)) } } - Err(BlockError::BeaconChainError(e)) if matches!(e, BeaconChainError::UnableToPublish) => { + Err(BlockError::BeaconChainError(BeaconChainError::UnableToPublish)) => { Err(warp_utils::reject::custom_server_error( "unable to publish to network channel".to_string(), )) @@ -784,7 +790,7 @@ fn check_slashable( chain_clone: &BeaconChain, block_root: Hash256, block_clone: &SignedBeaconBlock>, -) -> Result<(), BlockError> { +) -> Result<(), Box> { let slashable_cache = chain_clone.observed_slashable.read(); if slashable_cache .is_slashable( @@ -792,13 +798,13 @@ fn check_slashable( block_clone.message().proposer_index(), block_root, ) - .map_err(|e| BlockError::BeaconChainError(e.into()))? + .map_err(|e| Box::new(BlockError::BeaconChainError(e.into())))? { warn!( slot = %block_clone.slot(), "Not publishing equivocating block" ); - return Err(BlockError::Slashable); + return Err(Box::new(BlockError::Slashable)); } Ok(()) } diff --git a/beacon_node/http_api/src/sync_committee_rewards.rs b/beacon_node/http_api/src/sync_committee_rewards.rs index 9bc1f6ead4d..513c8322bf6 100644 --- a/beacon_node/http_api/src/sync_committee_rewards.rs +++ b/beacon_node/http_api/src/sync_committee_rewards.rs @@ -52,7 +52,8 @@ pub fn get_state_before_applying_block( let parent_block: SignedBlindedBeaconBlock = chain .get_blinded_block(&block.parent_root()) .and_then(|maybe_block| { - maybe_block.ok_or_else(|| BeaconChainError::MissingBeaconBlock(block.parent_root())) + maybe_block + .ok_or_else(|| BeaconChainError::MissingBeaconBlock(Box::new(block.parent_root()))) }) .map_err(|e| custom_not_found(format!("Parent block is not available! {:?}", e)))?; @@ -61,8 +62,9 @@ pub fn get_state_before_applying_block( let parent_state = chain .get_state(&parent_block.state_root(), Some(parent_block.slot()), true) .and_then(|maybe_state| { - maybe_state - .ok_or_else(|| BeaconChainError::MissingBeaconState(parent_block.state_root())) + maybe_state.ok_or_else(|| { + BeaconChainError::MissingBeaconState(Box::new(parent_block.state_root())) + }) }) .map_err(|e| custom_not_found(format!("Parent state is not available! {:?}", e)))?; diff --git a/beacon_node/http_api/src/sync_committees.rs b/beacon_node/http_api/src/sync_committees.rs index 61cb4e96ff6..9ca1a2401a0 100644 --- a/beacon_node/http_api/src/sync_committees.rs +++ b/beacon_node/http_api/src/sync_committees.rs @@ -95,13 +95,8 @@ fn duties_from_state_load( .ok_or(BeaconChainError::UnableToReadSlot)? .epoch(T::EthSpec::slots_per_epoch()); - let max_sync_committee_period = tolerant_current_epoch - .sync_committee_period(&chain.spec) - .map_err(|e| e)? - + 1; - let sync_committee_period = request_epoch - .sync_committee_period(&chain.spec) - .map_err(|e| e)?; + let max_sync_committee_period = tolerant_current_epoch.sync_committee_period(&chain.spec)? + 1; + let sync_committee_period = request_epoch.sync_committee_period(&chain.spec)?; if tolerant_current_epoch < altair_fork_epoch { // Empty response if the epoch is pre-Altair. diff --git a/beacon_node/network/src/network_beacon_processor/mod.rs b/beacon_node/network/src/network_beacon_processor/mod.rs index f7c3a1bf8db..28ed15e5200 100644 --- a/beacon_node/network/src/network_beacon_processor/mod.rs +++ b/beacon_node/network/src/network_beacon_processor/mod.rs @@ -846,9 +846,9 @@ impl NetworkBeaconProcessor { "Fetch blobs completed without import" ); } - Err(FetchEngineBlobError::BlobProcessingError(BlockError::DuplicateFullyImported( - .., - ))) => { + Err(FetchEngineBlobError::BlobProcessingError(block_error)) + if matches!(*block_error, BlockError::DuplicateFullyImported { .. }) => + { debug!( %block_root, "Fetch blobs duplicate import" From 5cb1966de9cd923515179265d4522d9e1e7691cf Mon Sep 17 00:00:00 2001 From: PoulavBhowmick03 Date: Sun, 22 Jun 2025 16:50:48 +0530 Subject: [PATCH 4/6] Removed Box from Hash256 in BeaconChainError --- .../beacon_chain/src/attestation_rewards.rs | 2 +- .../beacon_chain/src/beacon_block_streamer.rs | 13 ++---- beacon_node/beacon_chain/src/beacon_chain.rs | 46 ++++++++----------- .../beacon_chain/src/beacon_proposer_cache.rs | 4 +- .../beacon_chain/src/block_verification.rs | 8 ++-- .../beacon_chain/src/canonical_head.rs | 26 ++++------- beacon_node/beacon_chain/src/errors.rs | 38 +++++++-------- .../http_api/src/attestation_performance.rs | 15 ++---- .../http_api/src/block_packing_efficiency.rs | 10 ++-- beacon_node/http_api/src/block_rewards.rs | 4 +- .../http_api/src/sync_committee_rewards.rs | 8 ++-- 11 files changed, 70 insertions(+), 104 deletions(-) diff --git a/beacon_node/beacon_chain/src/attestation_rewards.rs b/beacon_node/beacon_chain/src/attestation_rewards.rs index 3e7c5102d33..7d88268cf9f 100644 --- a/beacon_node/beacon_chain/src/attestation_rewards.rs +++ b/beacon_node/beacon_chain/src/attestation_rewards.rs @@ -55,7 +55,7 @@ impl BeaconChain { // to cache states so that future calls are faster. let state = self .get_state(&state_root, Some(state_slot), true)? - .ok_or(BeaconChainError::MissingBeaconState(Box::new(state_root)))?; + .ok_or(BeaconChainError::MissingBeaconState(state_root))?; if state.fork_name_unchecked().altair_enabled() { self.compute_attestation_rewards_altair(state, validators) diff --git a/beacon_node/beacon_chain/src/beacon_block_streamer.rs b/beacon_node/beacon_chain/src/beacon_block_streamer.rs index ace507d65dd..e37a69040db 100644 --- a/beacon_node/beacon_chain/src/beacon_block_streamer.rs +++ b/beacon_node/beacon_chain/src/beacon_block_streamer.rs @@ -121,8 +121,8 @@ fn reconstruct_default_header_block( Err(BeaconChainError::InconsistentPayloadReconstructed { slot: blinded_block.slot(), exec_block_hash: header_from_block.block_hash(), - canonical_transactions_root: Box::new(header_from_block.transactions_root()), - reconstructed_transactions_root: Box::new(header_from_payload.transactions_root()), + canonical_transactions_root: header_from_block.transactions_root(), + reconstructed_transactions_root: header_from_payload.transactions_root(), }) } } @@ -152,12 +152,9 @@ fn reconstruct_blocks( let error = BeaconChainError::InconsistentPayloadReconstructed { slot: block_parts.blinded_block.slot(), exec_block_hash: block_parts.header.block_hash(), - canonical_transactions_root: Box::new( - block_parts.header.transactions_root(), - ), - reconstructed_transactions_root: Box::new( - header_from_payload.transactions_root(), - ), + canonical_transactions_root: block_parts.header.transactions_root(), + reconstructed_transactions_root: header_from_payload + .transactions_root(), }; debug!(?root, ?error, "Failed to reconstruct block"); block_map.insert(root, Arc::new(Err(error))); diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index d202842df6f..51e85528bbb 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -768,12 +768,12 @@ impl BeaconChain { ) -> Result> + '_, Error> { let block = self .get_blinded_block(&block_root)? - .ok_or(Error::MissingBeaconBlock(Box::new(block_root)))?; + .ok_or(Error::MissingBeaconBlock(block_root))?; // This method is only used in tests, so we may as well cache states to make CI go brr. // TODO(release-v7) move this method out of beacon chain and into `store_tests`` or something equivalent. let state = self .get_state(&block.state_root(), Some(block.slot()), true)? - .ok_or_else(|| Error::MissingBeaconState(Box::new(block.state_root())))?; + .ok_or_else(|| Error::MissingBeaconState(block.state_root()))?; let iter = BlockRootsIterator::owned(&self.store, state); Ok(std::iter::once(Ok((block_root, block.slot()))) .chain(iter) @@ -1179,8 +1179,8 @@ impl BeaconChain { return Err(Error::InconsistentPayloadReconstructed { slot: blinded_block.slot(), exec_block_hash, - canonical_transactions_root: Box::new(execution_payload_header.transactions_root()), - reconstructed_transactions_root: Box::new(header_from_payload.transactions_root()), + canonical_transactions_root: execution_payload_header.transactions_root(), + reconstructed_transactions_root: header_from_payload.transactions_root(), }); } @@ -1629,9 +1629,7 @@ impl BeaconChain { .canonical_head .fork_choice_read_lock() .get_block_execution_status(&head_block_root) - .ok_or(Error::AttestationHeadNotInForkChoice(Box::new( - head_block_root, - )))?; + .ok_or(Error::AttestationHeadNotInForkChoice(head_block_root))?; let (duties, dependent_root) = self.with_committee_cache( head_block_root, @@ -1683,15 +1681,13 @@ impl BeaconChain { .store .load_hot_state_summary(&state_root) .map_err(BeaconChainError::DBError)? - .ok_or(BeaconChainError::MissingHotStateSummary(Box::new( - state_root, - )))?; + .ok_or(BeaconChainError::MissingHotStateSummary(state_root))?; if slot != checkpoint.epoch.start_slot(T::EthSpec::slots_per_epoch()) || latest_block_root != *checkpoint.root { return Err(BeaconChainError::InvalidCheckpoint { - state_root: Box::new(state_root), + state_root, checkpoint: Box::new(checkpoint), }); } @@ -1781,14 +1777,14 @@ impl BeaconChain { // The attestation references a block that is not in fork choice, it must be // pre-finalization. None => Err(Error::CannotAttestToFinalizedBlock { - beacon_block_root: Box::new(beacon_block_root), + beacon_block_root, }), // The attestation references a fully valid `beacon_block_root`. Some(execution_status) if execution_status.is_valid_or_irrelevant() => Ok(attestation), // The attestation references a block that has not been verified by an EL (i.e. it // is optimistic or invalid). Don't return the block, return an error instead. Some(execution_status) => Err(Error::HeadBlockNotFullyVerified { - beacon_block_root: Box::new(beacon_block_root), + beacon_block_root, execution_status, }), } @@ -1824,14 +1820,14 @@ impl BeaconChain { // The contribution references a block that is not in fork choice, it must be // pre-finalization. None => Err(Error::SyncContributionDataReferencesFinalizedBlock { - beacon_block_root: Box::new(beacon_block_root), + beacon_block_root, }), // The contribution references a fully valid `beacon_block_root`. Some(execution_status) if execution_status.is_valid_or_irrelevant() => Ok(contribution), // The contribution references a block that has not been verified by an EL (i.e. it // is optimistic or invalid). Don't return the block, return an error instead. Some(execution_status) => Err(Error::HeadBlockNotFullyVerified { - beacon_block_root: Box::new(beacon_block_root), + beacon_block_root, execution_status, }), } @@ -1987,15 +1983,11 @@ impl BeaconChain { Some(execution_status) if execution_status.is_valid_or_irrelevant() => (), Some(execution_status) => { return Err(Error::HeadBlockNotFullyVerified { - beacon_block_root: Box::new(beacon_block_root), + beacon_block_root, execution_status, }) } - None => { - return Err(Error::HeadMissingFromForkChoice(Box::new( - beacon_block_root, - ))) - } + None => return Err(Error::HeadMissingFromForkChoice(beacon_block_root)), }; /* @@ -2479,7 +2471,7 @@ impl BeaconChain { let fork_choice_lock = self.canonical_head.fork_choice_read_lock(); let block = fork_choice_lock .get_block(block_root) - .ok_or(Error::AttestationHeadNotInForkChoice(Box::new(*block_root)))?; + .ok_or(Error::AttestationHeadNotInForkChoice(*block_root))?; drop(fork_choice_lock); let block_shuffling_id = if target_epoch == block.current_epoch_shuffling_id.shuffling_epoch @@ -4820,11 +4812,11 @@ impl BeaconChain { } else { let block = self .get_blinded_block(&parent_block_root)? - .ok_or(Error::MissingBeaconBlock(Box::new(parent_block_root)))?; + .ok_or(Error::MissingBeaconBlock(parent_block_root))?; let (state_root, state) = self .store .get_advanced_hot_state(parent_block_root, proposal_slot, block.state_root())? - .ok_or(Error::MissingBeaconState(Box::new(block.state_root())))?; + .ok_or(Error::MissingBeaconState(block.state_root()))?; (Cow::Owned(state), state_root) }; @@ -5899,7 +5891,7 @@ impl BeaconChain { // Return an error here to try and prevent progression by upstream functions. return Err(Error::JustifiedPayloadInvalid { - justified_root: Box::new(justified_block.root), + justified_root: justified_block.root, execution_block_hash: justified_block.execution_status.block_hash(), }); } @@ -6594,7 +6586,7 @@ impl BeaconChain { .canonical_head .fork_choice_read_lock() .get_block(&head_block_root) - .ok_or(Error::MissingBeaconBlock(Box::new(head_block_root)))?; + .ok_or(Error::MissingBeaconBlock(head_block_root))?; let shuffling_id = BlockShufflingIds { current: head_block.current_epoch_shuffling_id.clone(), @@ -6691,7 +6683,7 @@ impl BeaconChain { let (state_root, state) = self .store .get_advanced_hot_state(head_block_root, target_slot, head_block.state_root)? - .ok_or(Error::MissingBeaconState(Box::new(head_block.state_root)))?; + .ok_or(Error::MissingBeaconState(head_block.state_root))?; (state, state_root) }; diff --git a/beacon_node/beacon_chain/src/beacon_proposer_cache.rs b/beacon_node/beacon_chain/src/beacon_proposer_cache.rs index 3c770291d59..12970214c6a 100644 --- a/beacon_node/beacon_chain/src/beacon_proposer_cache.rs +++ b/beacon_node/beacon_chain/src/beacon_proposer_cache.rs @@ -175,9 +175,7 @@ pub fn compute_proposer_duties_from_head( .canonical_head .fork_choice_read_lock() .get_block_execution_status(&head_block_root) - .ok_or(BeaconChainError::HeadMissingFromForkChoice(Box::new( - head_block_root, - )))?; + .ok_or(BeaconChainError::HeadMissingFromForkChoice(head_block_root))?; // Advance the state into the requested epoch. ensure_state_is_in_epoch(&mut state, head_state_root, request_epoch, &chain.spec)?; diff --git a/beacon_node/beacon_chain/src/block_verification.rs b/beacon_node/beacon_chain/src/block_verification.rs index 7c5ed5e071e..00c9625140f 100644 --- a/beacon_node/beacon_chain/src/block_verification.rs +++ b/beacon_node/beacon_chain/src/block_verification.rs @@ -1430,9 +1430,9 @@ impl ExecutionPendingBlock { let parent_slot = parent.beacon_block.slot(); if state.slot() < parent_slot || state.slot() > block.slot() { return Err(BeaconChainError::BadPreState { - parent_root: Box::new(parent.beacon_block_root), + parent_root: parent.beacon_block_root, parent_slot, - block_root: Box::new(block_root), + block_root, block_slot: block.slot(), state_slot: state.slot(), } @@ -1870,9 +1870,7 @@ fn load_parent>( // // It's an internal error if the block exists in fork choice but not in the // database. - BlockError::from(BeaconChainError::MissingBeaconBlock(Box::new( - block.parent_root(), - ))) + BlockError::from(BeaconChainError::MissingBeaconBlock(block.parent_root())) })?; // Load the parent block's state from the database, returning an error if it is not found. diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index 17607922300..3a090de48e0 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -295,13 +295,11 @@ impl CanonicalHead { let beacon_block_root = fork_choice_view.head_block_root; let beacon_block = store .get_full_block(&beacon_block_root)? - .ok_or(Error::MissingBeaconBlock(Box::new(beacon_block_root)))?; + .ok_or(Error::MissingBeaconBlock(beacon_block_root))?; let current_slot = fork_choice.fc_store().get_current_slot(); let (_, beacon_state) = store .get_advanced_hot_state(beacon_block_root, current_slot, beacon_block.state_root())? - .ok_or(Error::MissingBeaconState(Box::new( - beacon_block.state_root(), - )))?; + .ok_or(Error::MissingBeaconState(beacon_block.state_root()))?; let snapshot = BeaconSnapshot { beacon_block_root, @@ -336,7 +334,7 @@ impl CanonicalHead { let head_block_root = self.cached_head().head_block_root(); self.fork_choice_read_lock() .get_block_execution_status(&head_block_root) - .ok_or(Error::HeadMissingFromForkChoice(Box::new(head_block_root))) + .ok_or(Error::HeadMissingFromForkChoice(head_block_root)) } /// Returns a clone of the `CachedHead` and the execution status of the contained head block. @@ -352,7 +350,7 @@ impl CanonicalHead { let execution_status = self .fork_choice_read_lock() .get_block_execution_status(&head_block_root) - .ok_or(Error::HeadMissingFromForkChoice(Box::new(head_block_root)))?; + .ok_or(Error::HeadMissingFromForkChoice(head_block_root))?; Ok((head, execution_status)) } @@ -594,9 +592,9 @@ impl BeaconChain { let new_head_proto_block = fork_choice_read_lock .get_block(&new_view.head_block_root) - .ok_or(Error::HeadBlockMissingFromForkChoice(Box::new( + .ok_or(Error::HeadBlockMissingFromForkChoice( new_view.head_block_root, - )))?; + ))?; // Do not allow an invalid block to become the head. // @@ -612,7 +610,7 @@ impl BeaconChain { // However, this check is cheap. if new_head_proto_block.execution_status.is_invalid() { return Err(Error::HeadHasInvalidPayload { - block_root: Box::new(new_head_proto_block.root), + block_root: new_head_proto_block.root, execution_status: new_head_proto_block.execution_status, }); } @@ -646,9 +644,7 @@ impl BeaconChain { let beacon_block = self .store .get_full_block(&new_view.head_block_root)? - .ok_or(Error::MissingBeaconBlock(Box::new( - new_view.head_block_root, - )))?; + .ok_or(Error::MissingBeaconBlock(new_view.head_block_root))?; let (_, beacon_state) = self .store @@ -657,9 +653,7 @@ impl BeaconChain { current_slot, beacon_block.state_root(), )? - .ok_or(Error::MissingBeaconState(Box::new( - beacon_block.state_root(), - )))?; + .ok_or(Error::MissingBeaconState(beacon_block.state_root()))?; BeaconSnapshot { beacon_block: Arc::new(beacon_block), @@ -1055,7 +1049,7 @@ fn check_finalized_payload_validity( // Exit now, the node is in an invalid state. return Err(Error::InvalidFinalizedPayload { - finalized_root: Box::new(finalized_proto_block.root), + finalized_root: finalized_proto_block.root, execution_block_hash: block_hash, }); } diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index d5fd22a9c20..ec22854ce60 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -58,15 +58,15 @@ pub enum BeaconChainError { DBError(store::Error), ForkChoiceError(ForkChoiceError), ForkChoiceStoreError(ForkChoiceStoreError), - MissingBeaconBlock(Box), - MissingBeaconState(Box), - MissingHotStateSummary(Box), + MissingBeaconBlock(Hash256), + MissingBeaconState(Hash256), + MissingHotStateSummary(Hash256), SlotProcessingError(SlotProcessingError), EpochProcessingError(EpochProcessingError), StateAdvanceError(StateAdvanceError), UnableToAdvanceState(String), NoStateForAttestation { - beacon_block_root: Box, + beacon_block_root: Hash256, }, CannotAttestToFutureState, AttestationValidationError(AttestationValidationError), @@ -116,9 +116,9 @@ pub enum BeaconChainError { request_slot: Slot, }, BadPreState { - parent_root: Box, + parent_root: Hash256, parent_slot: Slot, - block_root: Box, + block_root: Hash256, block_slot: Slot, state_slot: Slot, }, @@ -152,8 +152,8 @@ pub enum BeaconChainError { InconsistentPayloadReconstructed { slot: Slot, exec_block_hash: ExecutionBlockHash, - canonical_transactions_root: Box, - reconstructed_transactions_root: Box, + canonical_transactions_root: Hash256, + reconstructed_transactions_root: Hash256, }, BlockStreamerError(BlockStreamerError), AddPayloadLogicError, @@ -168,33 +168,33 @@ pub enum BeaconChainError { BlockRewardSyncError, SyncCommitteeRewardsSyncError, AttestationRewardsError, - HeadMissingFromForkChoice(Box), - FinalizedBlockMissingFromForkChoice(Box), - HeadBlockMissingFromForkChoice(Box), + HeadMissingFromForkChoice(Hash256), + FinalizedBlockMissingFromForkChoice(Hash256), + HeadBlockMissingFromForkChoice(Hash256), InvalidFinalizedPayload { - finalized_root: Box, + finalized_root: Hash256, execution_block_hash: ExecutionBlockHash, }, InvalidFinalizedPayloadShutdownError(TrySendError), JustifiedPayloadInvalid { - justified_root: Box, + justified_root: Hash256, execution_block_hash: Option, }, ForkchoiceUpdate(Box), InvalidCheckpoint { - state_root: Box, + state_root: Hash256, checkpoint: Box, }, InvalidSlot(Slot), HeadBlockNotFullyVerified { - beacon_block_root: Box, + beacon_block_root: Hash256, execution_status: ExecutionStatus, }, CannotAttestToFinalizedBlock { - beacon_block_root: Box, + beacon_block_root: Hash256, }, SyncContributionDataReferencesFinalizedBlock { - beacon_block_root: Box, + beacon_block_root: Hash256, }, RuntimeShutdown, TokioJoin(tokio::task::JoinError), @@ -205,10 +205,10 @@ pub enum BeaconChainError { }, ForkchoiceUpdateParamsMissing, HeadHasInvalidPayload { - block_root: Box, + block_root: Hash256, execution_status: ExecutionStatus, }, - AttestationHeadNotInForkChoice(Box), + AttestationHeadNotInForkChoice(Hash256), MissingPersistedForkChoice, CommitteePromiseFailed(oneshot_broadcast::Error), MaxCommitteePromises(usize), diff --git a/beacon_node/http_api/src/attestation_performance.rs b/beacon_node/http_api/src/attestation_performance.rs index 06c3f04eb8d..23ab5e3752b 100644 --- a/beacon_node/http_api/src/attestation_performance.rs +++ b/beacon_node/http_api/src/attestation_performance.rs @@ -111,9 +111,7 @@ pub fn get_attestation_performance( let first_block = chain .get_blinded_block(first_block_root) .and_then(|maybe_block| { - maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(Box::new( - *first_block_root, - ))) + maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(*first_block_root)) }) .map_err(unhandled_error)?; @@ -121,9 +119,8 @@ pub fn get_attestation_performance( let prior_block = chain .get_blinded_block(&first_block.parent_root()) .and_then(|maybe_block| { - maybe_block.ok_or_else(|| { - BeaconChainError::MissingBeaconBlock(Box::new(first_block.parent_root())) - }) + maybe_block + .ok_or_else(|| BeaconChainError::MissingBeaconBlock(first_block.parent_root())) }) .map_err(unhandled_error)?; @@ -134,9 +131,7 @@ pub fn get_attestation_performance( // to cache states so that future calls are faster. let state = chain .get_state(&state_root, Some(prior_slot), true) - .and_then(|maybe_state| { - maybe_state.ok_or(BeaconChainError::MissingBeaconState(Box::new(state_root))) - }) + .and_then(|maybe_state| maybe_state.ok_or(BeaconChainError::MissingBeaconState(state_root))) .map_err(unhandled_error)?; // Allocate an AttestationPerformance vector for each validator in the range. @@ -204,7 +199,7 @@ pub fn get_attestation_performance( chain .get_blinded_block(root) .and_then(|maybe_block| { - maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(Box::new(*root))) + maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(*root)) }) .map_err(unhandled_error) }) diff --git a/beacon_node/http_api/src/block_packing_efficiency.rs b/beacon_node/http_api/src/block_packing_efficiency.rs index f645ff27d8d..249a6732dcb 100644 --- a/beacon_node/http_api/src/block_packing_efficiency.rs +++ b/beacon_node/http_api/src/block_packing_efficiency.rs @@ -278,9 +278,7 @@ pub fn get_block_packing_efficiency( let first_block = chain .get_blinded_block(first_block_root) .and_then(|maybe_block| { - maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(Box::new( - *first_block_root, - ))) + maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(*first_block_root)) }) .map_err(unhandled_error)?; @@ -292,9 +290,7 @@ pub fn get_block_packing_efficiency( let starting_state = chain .get_state(&starting_state_root, Some(prior_slot), true) .and_then(|maybe_state| { - maybe_state.ok_or(BeaconChainError::MissingBeaconState(Box::new( - starting_state_root, - ))) + maybe_state.ok_or(BeaconChainError::MissingBeaconState(starting_state_root)) }) .map_err(unhandled_error)?; @@ -396,7 +392,7 @@ pub fn get_block_packing_efficiency( chain .get_blinded_block(root) .and_then(|maybe_block| { - maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(Box::new(*root))) + maybe_block.ok_or(BeaconChainError::MissingBeaconBlock(*root)) }) .map_err(unhandled_error) }) diff --git a/beacon_node/http_api/src/block_rewards.rs b/beacon_node/http_api/src/block_rewards.rs index e4c85609571..29b23e89a79 100644 --- a/beacon_node/http_api/src/block_rewards.rs +++ b/beacon_node/http_api/src/block_rewards.rs @@ -46,9 +46,7 @@ pub fn get_block_rewards( // to cache states so that future calls are faster. let mut state = chain .get_state(&state_root, Some(prior_slot), true) - .and_then(|maybe_state| { - maybe_state.ok_or(BeaconChainError::MissingBeaconState(Box::new(state_root))) - }) + .and_then(|maybe_state| maybe_state.ok_or(BeaconChainError::MissingBeaconState(state_root))) .map_err(unhandled_error)?; state diff --git a/beacon_node/http_api/src/sync_committee_rewards.rs b/beacon_node/http_api/src/sync_committee_rewards.rs index 513c8322bf6..9bc1f6ead4d 100644 --- a/beacon_node/http_api/src/sync_committee_rewards.rs +++ b/beacon_node/http_api/src/sync_committee_rewards.rs @@ -52,8 +52,7 @@ pub fn get_state_before_applying_block( let parent_block: SignedBlindedBeaconBlock = chain .get_blinded_block(&block.parent_root()) .and_then(|maybe_block| { - maybe_block - .ok_or_else(|| BeaconChainError::MissingBeaconBlock(Box::new(block.parent_root()))) + maybe_block.ok_or_else(|| BeaconChainError::MissingBeaconBlock(block.parent_root())) }) .map_err(|e| custom_not_found(format!("Parent block is not available! {:?}", e)))?; @@ -62,9 +61,8 @@ pub fn get_state_before_applying_block( let parent_state = chain .get_state(&parent_block.state_root(), Some(parent_block.slot()), true) .and_then(|maybe_state| { - maybe_state.ok_or_else(|| { - BeaconChainError::MissingBeaconState(Box::new(parent_block.state_root())) - }) + maybe_state + .ok_or_else(|| BeaconChainError::MissingBeaconState(parent_block.state_root())) }) .map_err(|e| custom_not_found(format!("Parent state is not available! {:?}", e)))?; From 42c7e5cb76d73e5d15108a2f011ba7d79d888aac Mon Sep 17 00:00:00 2001 From: PoulavBhowmick03 Date: Sun, 22 Jun 2025 17:57:25 +0530 Subject: [PATCH 5/6] Removed Box from String in BeaconChainError --- beacon_node/beacon_chain/src/beacon_chain.rs | 10 +++------- beacon_node/beacon_chain/src/errors.rs | 6 +++--- .../src/light_client_server_cache.rs | 18 ++++++++---------- beacon_node/http_api/src/light_client.rs | 4 ++-- 4 files changed, 16 insertions(+), 22 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index 51e85528bbb..e9d410d1bc0 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1237,7 +1237,7 @@ impl BeaconChain { if reconstruction_possible { reconstruct_blobs(&self.kzg, &columns, None, &block, &self.spec) .map(Some) - .map_err(|e| Error::FailedToReconstructBlobs(Box::new(e))) + .map_err(Error::FailedToReconstructBlobs) } else { Err(Error::InsufficientColumnsToReconstructBlobs { columns_found: columns.len(), @@ -1776,9 +1776,7 @@ impl BeaconChain { { // The attestation references a block that is not in fork choice, it must be // pre-finalization. - None => Err(Error::CannotAttestToFinalizedBlock { - beacon_block_root, - }), + None => Err(Error::CannotAttestToFinalizedBlock { beacon_block_root }), // The attestation references a fully valid `beacon_block_root`. Some(execution_status) if execution_status.is_valid_or_irrelevant() => Ok(attestation), // The attestation references a block that has not been verified by an EL (i.e. it @@ -1819,9 +1817,7 @@ impl BeaconChain { { // The contribution references a block that is not in fork choice, it must be // pre-finalization. - None => Err(Error::SyncContributionDataReferencesFinalizedBlock { - beacon_block_root, - }), + None => Err(Error::SyncContributionDataReferencesFinalizedBlock { beacon_block_root }), // The contribution references a fully valid `beacon_block_root`. Some(execution_status) if execution_status.is_valid_or_irrelevant() => Ok(contribution), // The contribution references a block that has not been verified by an EL (i.e. it diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index ec22854ce60..a22485dc7b9 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -217,10 +217,10 @@ pub enum BeaconChainError { InconsistentFork(InconsistentFork), ProposerHeadForkChoiceError(Box>), UnableToPublish, - UnableToBuildColumnSidecar(Box), + UnableToBuildColumnSidecar(String), AvailabilityCheckError(AvailabilityCheckError), LightClientUpdateError(LightClientUpdateError), - LightClientBootstrapError(Box), + LightClientBootstrapError(String), UnsupportedFork, MilhouseError(MilhouseError), EmptyRpcCustodyColumns, @@ -229,7 +229,7 @@ pub enum BeaconChainError { InsufficientColumnsToReconstructBlobs { columns_found: usize, }, - FailedToReconstructBlobs(Box), + FailedToReconstructBlobs(String), } easy_from_to!(SlotProcessingError, BeaconChainError); diff --git a/beacon_node/beacon_chain/src/light_client_server_cache.rs b/beacon_node/beacon_chain/src/light_client_server_cache.rs index eb34a8c5922..66344ea1f79 100644 --- a/beacon_node/beacon_chain/src/light_client_server_cache.rs +++ b/beacon_node/beacon_chain/src/light_client_server_cache.rs @@ -357,8 +357,8 @@ impl LightClientServerCache { chain_spec: &ChainSpec, ) -> Result, ForkName)>, BeaconChainError> { let Some(block) = store.get_blinded_block(block_root)? else { - return Err(BeaconChainError::LightClientBootstrapError(Box::new( - format!("Block root {block_root} not found"), + return Err(BeaconChainError::LightClientBootstrapError(format!( + "Block root {block_root} not found" ))); }; @@ -373,23 +373,21 @@ impl LightClientServerCache { let Some(current_sync_committee_branch) = store.get_sync_committee_branch(block_root)? else { - return Err(BeaconChainError::LightClientBootstrapError(Box::new( - format!( - "Sync committee branch for block root {:?} not found", - block_root - ), + return Err(BeaconChainError::LightClientBootstrapError(format!( + "Sync committee branch for block root {:?} not found", + block_root ))); }; if sync_committee_period > finalized_period { return Err(BeaconChainError::LightClientBootstrapError( - Box::new(format!("The blocks sync committee period {sync_committee_period} is greater than the current finalized period {finalized_period}")), + format!("The blocks sync committee period {sync_committee_period} is greater than the current finalized period {finalized_period}"), )); } let Some(current_sync_committee) = store.get_sync_committee(sync_committee_period)? else { - return Err(BeaconChainError::LightClientBootstrapError(Box::new( - format!("Sync committee for period {sync_committee_period} not found"), + return Err(BeaconChainError::LightClientBootstrapError(format!( + "Sync committee for period {sync_committee_period} not found", ))); }; diff --git a/beacon_node/http_api/src/light_client.rs b/beacon_node/http_api/src/light_client.rs index d0aa0dc3afa..24b1338a724 100644 --- a/beacon_node/http_api/src/light_client.rs +++ b/beacon_node/http_api/src/light_client.rs @@ -71,9 +71,9 @@ pub fn get_light_client_bootstrap( println!("{:?}", err); err } else { - Box::new("No LightClientBootstrap found".to_string()) + "No LightClientBootstrap found".to_string() }; - warp_utils::reject::custom_not_found(*error_message) + warp_utils::reject::custom_not_found(error_message) })? .ok_or(warp_utils::reject::custom_not_found( "No LightClientBootstrap found".to_string(), From 16a348b56d94918cb2875fed97759d65c91c25c2 Mon Sep 17 00:00:00 2001 From: PoulavBhowmick03 Date: Sun, 22 Jun 2025 19:12:59 +0530 Subject: [PATCH 6/6] removed other reduntant Box and checked clippy pass --- beacon_node/beacon_chain/src/beacon_chain.rs | 8 ++++---- beacon_node/beacon_chain/src/canonical_head.rs | 4 ++-- beacon_node/beacon_chain/src/errors.rs | 10 +++++----- 3 files changed, 11 insertions(+), 11 deletions(-) diff --git a/beacon_node/beacon_chain/src/beacon_chain.rs b/beacon_node/beacon_chain/src/beacon_chain.rs index e9d410d1bc0..bc992520f85 100644 --- a/beacon_node/beacon_chain/src/beacon_chain.rs +++ b/beacon_node/beacon_chain/src/beacon_chain.rs @@ -1688,7 +1688,7 @@ impl BeaconChain { { return Err(BeaconChainError::InvalidCheckpoint { state_root, - checkpoint: Box::new(checkpoint), + checkpoint, }); } @@ -4823,7 +4823,7 @@ impl BeaconChain { if head_state.current_epoch() == proposal_epoch { return get_expected_withdrawals(&unadvanced_state, &self.spec) .map(|(withdrawals, _)| withdrawals) - .map_err(|e| Error::PrepareProposerFailed(Box::new(e))); + .map_err(Error::PrepareProposerFailed); } // Advance the state using the partial method. @@ -4841,7 +4841,7 @@ impl BeaconChain { )?; get_expected_withdrawals(&advanced_state, &self.spec) .map(|(withdrawals, _)| withdrawals) - .map_err(|e| Error::PrepareProposerFailed(Box::new(e))) + .map_err(Error::PrepareProposerFailed) } /// Determine whether a fork choice update to the execution layer should be overridden. @@ -4899,7 +4899,7 @@ impl BeaconChain { &self.config.re_org_disallowed_offsets, self.config.re_org_max_epochs_since_finalization, ) - .map_err(|e| e.map_inner_error(|e| Error::ProposerHeadForkChoiceError(Box::new(e))))?; + .map_err(|e| e.map_inner_error(Error::ProposerHeadForkChoiceError))?; // The slot of our potential re-org block is always 1 greater than the head block because we // only attempt single-slot re-orgs. diff --git a/beacon_node/beacon_chain/src/canonical_head.rs b/beacon_node/beacon_chain/src/canonical_head.rs index 3a090de48e0..a6f5179fdc6 100644 --- a/beacon_node/beacon_chain/src/canonical_head.rs +++ b/beacon_node/beacon_chain/src/canonical_head.rs @@ -1070,8 +1070,8 @@ fn check_against_finality_reversion( Ok(()) } else { Err(Error::RevertedFinalizedEpoch { - old: Box::new(old_view.finalized_checkpoint), - new: Box::new(new_view.finalized_checkpoint), + old: old_view.finalized_checkpoint, + new: new_view.finalized_checkpoint, }) } } diff --git a/beacon_node/beacon_chain/src/errors.rs b/beacon_node/beacon_chain/src/errors.rs index a22485dc7b9..222f7a349c0 100644 --- a/beacon_node/beacon_chain/src/errors.rs +++ b/beacon_node/beacon_chain/src/errors.rs @@ -47,8 +47,8 @@ pub enum BeaconChainError { UnableToReadSlot, UnableToComputeTimeAtSlot, RevertedFinalizedEpoch { - old: Box, - new: Box, + old: Checkpoint, + new: Checkpoint, }, SlotClockDidNotStart, NoStateForSlot(Slot), @@ -158,7 +158,7 @@ pub enum BeaconChainError { BlockStreamerError(BlockStreamerError), AddPayloadLogicError, ExecutionForkChoiceUpdateFailed(Box), - PrepareProposerFailed(Box), + PrepareProposerFailed(BlockProcessingError), ExecutionForkChoiceUpdateInvalid { status: PayloadStatus, }, @@ -183,7 +183,7 @@ pub enum BeaconChainError { ForkchoiceUpdate(Box), InvalidCheckpoint { state_root: Hash256, - checkpoint: Box, + checkpoint: Checkpoint, }, InvalidSlot(Slot), HeadBlockNotFullyVerified { @@ -215,7 +215,7 @@ pub enum BeaconChainError { BlsToExecutionPriorToCapella, BlsToExecutionConflictsWithPool, InconsistentFork(InconsistentFork), - ProposerHeadForkChoiceError(Box>), + ProposerHeadForkChoiceError(fork_choice::Error), UnableToPublish, UnableToBuildColumnSidecar(String), AvailabilityCheckError(AvailabilityCheckError),