Rust clippy 1.87 lint fixes (#7471)

Fix clippy lints for `rustc` 1.87


  clippy complains about `BeaconChainError` being too large. I went on a bit of a boxing spree because of this. We may instead want to `Box` some of the `BeaconChainError` variants?

Author: eserilev

beacon_node/beacon_chain/src/attestation_verification.rs

@@ -272,12 +272,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(BeaconChainError),
+    BeaconChainError(Box<BeaconChainError>),
 }
 
 impl From<BeaconChainError> for Error {
     fn from(e: BeaconChainError) -> Self {
-        Self::BeaconChainError(e)
+        Self::BeaconChainError(Box::new(e))
     }
 }
 
@@ -525,7 +525,7 @@ impl<'a, T: BeaconChainTypes> IndexedAggregatedAttestation<'a, T> {
             .observed_attestations
             .write()
             .is_known_subset(attestation, observed_attestation_key_root)
-            .map_err(|e| Error::BeaconChainError(e.into()))?
+            .map_err(|e| Error::BeaconChainError(Box::new(e.into())))?
         {
             metrics::inc_counter(&metrics::AGGREGATED_ATTESTATION_SUBSETS);
             return Err(Error::AttestationSupersetKnown(
@@ -628,7 +628,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(e.into()))?
+                    .map_err(|e| Error::BeaconChainError(Box::new(e.into())))?
                 {
                     return Err(Error::InvalidSelectionProof { aggregator_index });
                 }
@@ -698,7 +698,7 @@ impl<'a, T: BeaconChainTypes> VerifiedAggregatedAttestation<'a, T> {
             .observed_attestations
             .write()
             .observe_item(attestation, Some(observed_attestation_key_root))
-            .map_err(|e| Error::BeaconChainError(e.into()))?
+            .map_err(|e| Error::BeaconChainError(Box::new(e.into())))?
         {
             metrics::inc_counter(&metrics::AGGREGATED_ATTESTATION_SUBSETS);
             return Err(Error::AttestationSupersetKnown(

beacon_node/beacon_chain/src/beacon_chain.rs

@@ -2733,7 +2733,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     pub fn filter_chain_segment(
         self: &Arc<Self>,
         chain_segment: Vec<RpcBlock<T::EthSpec>>,
-    ) -> Result<Vec<HashBlockTuple<T::EthSpec>>, ChainSegmentResult> {
+    ) -> Result<Vec<HashBlockTuple<T::EthSpec>>, Box<ChainSegmentResult>> {
         // This function will never import any blocks.
         let imported_blocks = vec![];
         let mut filtered_chain_segment = Vec::with_capacity(chain_segment.len());
@@ -2750,10 +2750,10 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         for (i, block) in chain_segment.into_iter().enumerate() {
             // Ensure the block is the correct structure for the fork at `block.slot()`.
             if let Err(e) = block.as_block().fork_name(&self.spec) {
-                return Err(ChainSegmentResult::Failed {
+                return Err(Box::new(ChainSegmentResult::Failed {
                     imported_blocks,
                     error: BlockError::InconsistentFork(e),
-                });
+                }));
             }
 
             let block_root = block.block_root();
@@ -2765,18 +2765,18 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 // Without this check it would be possible to have a block verified using the
                 // incorrect shuffling. That would be bad, mmkay.
                 if block_root != *child_parent_root {
-                    return Err(ChainSegmentResult::Failed {
+                    return Err(Box::new(ChainSegmentResult::Failed {
                         imported_blocks,
                         error: BlockError::NonLinearParentRoots,
-                    });
+                    }));
                 }
 
                 // Ensure that the slots are strictly increasing throughout the chain segment.
                 if *child_slot <= block.slot() {
-                    return Err(ChainSegmentResult::Failed {
+                    return Err(Box::new(ChainSegmentResult::Failed {
                         imported_blocks,
                         error: BlockError::NonLinearSlots,
-                    });
+                    }));
                 }
             }
 
@@ -2807,18 +2807,18 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 // The block has a known parent that does not descend from the finalized block.
                 // There is no need to process this block or any children.
                 Err(BlockError::NotFinalizedDescendant { block_parent_root }) => {
-                    return Err(ChainSegmentResult::Failed {
+                    return Err(Box::new(ChainSegmentResult::Failed {
                         imported_blocks,
                         error: BlockError::NotFinalizedDescendant { block_parent_root },
-                    });
+                    }));
                 }
                 // If there was an error whilst determining if the block was invalid, return that
                 // error.
                 Err(BlockError::BeaconChainError(e)) => {
-                    return Err(ChainSegmentResult::Failed {
+                    return Err(Box::new(ChainSegmentResult::Failed {
                         imported_blocks,
                         error: BlockError::BeaconChainError(e),
-                    });
+                    }));
                 }
                 // If the block was decided to be irrelevant for any other reason, don't include
                 // this block or any of it's children in the filtered chain segment.
@@ -2863,11 +2863,11 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         );
         let mut filtered_chain_segment = match filtered_chain_segment_future.await {
             Ok(Ok(filtered_segment)) => filtered_segment,
-            Ok(Err(segment_result)) => return segment_result,
+            Ok(Err(segment_result)) => return *segment_result,
             Err(error) => {
                 return ChainSegmentResult::Failed {
                     imported_blocks,
-                    error: BlockError::BeaconChainError(error),
+                    error: BlockError::BeaconChainError(error.into()),
                 }
             }
         };
@@ -2906,7 +2906,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 Err(error) => {
                     return ChainSegmentResult::Failed {
                         imported_blocks,
-                        error: BlockError::BeaconChainError(error),
+                        error: BlockError::BeaconChainError(error.into()),
                     };
                 }
             };
@@ -3444,20 +3444,23 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
 
                 Ok(status)
             }
-            Err(e @ BlockError::BeaconChainError(BeaconChainError::TokioJoin(_))) => {
-                debug!(
-                    error = ?e,
-                    "Beacon block processing cancelled"
-                );
-                Err(e)
-            }
-            // There was an error whilst attempting to verify and import the block. The block might
-            // be partially verified or partially imported.
             Err(BlockError::BeaconChainError(e)) => {
-                crit!(
-                    error = ?e,
-                    "Beacon block processing error"
-                );
+                match e.as_ref() {
+                    BeaconChainError::TokioJoin(e) => {
+                        debug!(
+                            error = ?e,
+                            "Beacon block processing cancelled"
+                        );
+                    }
+                    _ => {
+                        // There was an error whilst attempting to verify and import the block. The block might
+                        // be partially verified or partially imported.
+                        crit!(
+                            error = ?e,
+                            "Beacon block processing error"
+                        );
+                    }
+                };
                 Err(BlockError::BeaconChainError(e))
             }
             // The block failed verification.
@@ -3589,7 +3592,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                         header.message.proposer_index,
                         block_root,
                     )
-                    .map_err(|e| BlockError::BeaconChainError(e.into()))?;
+                    .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))?;
                 if let Some(slasher) = self.slasher.as_ref() {
                     slasher.accept_block_header(header);
                 }
@@ -3674,7 +3677,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                         header.message.proposer_index,
                         block_root,
                     )
-                    .map_err(|e| BlockError::BeaconChainError(e.into()))?;
+                    .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))?;
                 if let Some(slasher) = self.slasher.as_ref() {
                     slasher.accept_block_header(header.clone());
                 }
@@ -3857,7 +3860,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                     payload_verification_status,
                     &self.spec,
                 )
-                .map_err(|e| BlockError::BeaconChainError(e.into()))?;
+                .map_err(|e| BlockError::BeaconChainError(Box::new(e.into())))?;
         }
 
         // If the block is recent enough and it was not optimistically imported, check to see if it
@@ -4070,7 +4073,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 warning = "The database is likely corrupt now, consider --purge-db",
                 "No stored fork choice found to restore from"
             );
-            Err(BlockError::BeaconChainError(e))
+            Err(BlockError::BeaconChainError(Box::new(e)))
         } else {
             Ok(())
         }
@@ -4125,9 +4128,9 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                              Provided block root is not a checkpoint.",
                     ))
                     .map_err(|err| {
-                        BlockError::BeaconChainError(
+                        BlockError::BeaconChainError(Box::new(
                             BeaconChainError::WeakSubjectivtyShutdownError(err),
-                        )
+                        ))
                     })?;
                 return Err(BlockError::WeakSubjectivityConflict);
             }
@@ -4901,7 +4904,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         canonical_forkchoice_params: ForkchoiceUpdateParameters,
     ) -> Result<ForkchoiceUpdateParameters, Error> {
         self.overridden_forkchoice_update_params_or_failure_reason(&canonical_forkchoice_params)
-            .or_else(|e| match e {
+            .or_else(|e| match *e {
                 ProposerHeadError::DoNotReOrg(reason) => {
                     trace!(
                         %reason,
@@ -4916,19 +4919,19 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
     pub fn overridden_forkchoice_update_params_or_failure_reason(
         &self,
         canonical_forkchoice_params: &ForkchoiceUpdateParameters,
-    ) -> Result<ForkchoiceUpdateParameters, ProposerHeadError<Error>> {
+    ) -> Result<ForkchoiceUpdateParameters, Box<ProposerHeadError<Error>>> {
         let _timer = metrics::start_timer(&metrics::FORK_CHOICE_OVERRIDE_FCU_TIMES);
 
         // Never override if proposer re-orgs are disabled.
         let re_org_head_threshold = self
             .config
             .re_org_head_threshold
-            .ok_or(DoNotReOrg::ReOrgsDisabled)?;
+            .ok_or(Box::new(DoNotReOrg::ReOrgsDisabled.into()))?;
 
         let re_org_parent_threshold = self
             .config
             .re_org_parent_threshold
-            .ok_or(DoNotReOrg::ReOrgsDisabled)?;
+            .ok_or(Box::new(DoNotReOrg::ReOrgsDisabled.into()))?;
 
         let head_block_root = canonical_forkchoice_params.head_root;
 
@@ -4969,7 +4972,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             false
         };
         if !current_slot_ok {
-            return Err(DoNotReOrg::HeadDistance.into());
+            return Err(Box::new(DoNotReOrg::HeadDistance.into()));
         }
 
         // Only attempt a re-org if we have a proposer registered for the re-org slot.
@@ -4992,7 +4995,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                         decision_root = ?shuffling_decision_root,
                         "Fork choice override proposer shuffling miss"
                     );
-                    DoNotReOrg::NotProposing
+                    Box::new(DoNotReOrg::NotProposing.into())
                 })?
                 .index as u64;
 
@@ -5002,7 +5005,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 .has_proposer_preparation_data_blocking(proposer_index)
         };
         if !proposing_at_re_org_slot {
-            return Err(DoNotReOrg::NotProposing.into());
+            return Err(Box::new(DoNotReOrg::NotProposing.into()));
         }
 
         // If the current slot is already equal to the proposal slot (or we are in the tail end of
@@ -5017,18 +5020,22 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             (true, true)
         };
         if !head_weak {
-            return Err(DoNotReOrg::HeadNotWeak {
-                head_weight: info.head_node.weight,
-                re_org_head_weight_threshold: info.re_org_head_weight_threshold,
-            }
-            .into());
+            return Err(Box::new(
+                DoNotReOrg::HeadNotWeak {
+                    head_weight: info.head_node.weight,
+                    re_org_head_weight_threshold: info.re_org_head_weight_threshold,
+                }
+                .into(),
+            ));
         }
         if !parent_strong {
-            return Err(DoNotReOrg::ParentNotStrong {
-                parent_weight: info.parent_node.weight,
-                re_org_parent_weight_threshold: info.re_org_parent_weight_threshold,
-            }
-            .into());
+            return Err(Box::new(
+                DoNotReOrg::ParentNotStrong {
+                    parent_weight: info.parent_node.weight,
+                    re_org_parent_weight_threshold: info.re_org_parent_weight_threshold,
+                }
+                .into(),
+            ));
         }
 
         // Check that the head block arrived late and is vulnerable to a re-org. This check is only
@@ -5039,7 +5046,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
         let head_block_late =
             self.block_observed_after_attestation_deadline(head_block_root, head_slot);
         if !head_block_late {
-            return Err(DoNotReOrg::HeadNotLate.into());
+            return Err(Box::new(DoNotReOrg::HeadNotLate.into()));
         }
 
         let parent_head_hash = info.parent_node.execution_status.block_hash();
@@ -5253,16 +5260,16 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
             .validators()
             .get(proposer_index as usize)
             .map(|v| v.pubkey)
-            .ok_or(BlockProductionError::BeaconChain(
+            .ok_or(BlockProductionError::BeaconChain(Box::new(
                 BeaconChainError::ValidatorIndexUnknown(proposer_index as usize),
-            ))?;
+            )))?;
 
         let builder_params = BuilderParams {
             pubkey,
             slot: state.slot(),
             chain_health: self
                 .is_healthy(&parent_root)
-                .map_err(BlockProductionError::BeaconChain)?,
+                .map_err(|e| BlockProductionError::BeaconChain(Box::new(e)))?,
         };
 
         // If required, start the process of loading an execution payload from the EL early. This

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(BeaconChainError),
+    BeaconChainError(Box<BeaconChainError>),
 
     /// The `BlobSidecar` was gossiped over an incorrect subnet.
     ///
@@ -147,13 +147,13 @@ impl std::fmt::Display for GossipBlobError {
 
 impl From<BeaconChainError> for GossipBlobError {
     fn from(e: BeaconChainError) -> Self {
-        GossipBlobError::BeaconChainError(e)
+        GossipBlobError::BeaconChainError(e.into())
     }
 }
 
 impl From<BeaconStateError> for GossipBlobError {
     fn from(e: BeaconStateError) -> Self {
-        GossipBlobError::BeaconChainError(BeaconChainError::BeaconStateError(e))
+        GossipBlobError::BeaconChainError(BeaconChainError::BeaconStateError(e).into())
     }
 }
 
@@ -446,7 +446,7 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes, O: ObservationStrat
         .observed_blob_sidecars
         .read()
         .proposer_is_known(&blob_sidecar)
-        .map_err(|e| GossipBlobError::BeaconChainError(e.into()))?
+        .map_err(|e| GossipBlobError::BeaconChainError(Box::new(e.into())))?
     {
         return Err(GossipBlobError::RepeatBlob {
             proposer: blob_proposer_index,
@@ -511,7 +511,7 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes, O: ObservationStrat
         let (parent_state_root, mut parent_state) = chain
             .store
             .get_advanced_hot_state(block_parent_root, blob_slot, parent_block.state_root)
-            .map_err(|e| GossipBlobError::BeaconChainError(e.into()))?
+            .map_err(|e| GossipBlobError::BeaconChainError(Box::new(e.into())))?
             .ok_or_else(|| {
                 BeaconChainError::DBInconsistent(format!(
                     "Missing state for parent block {block_parent_root:?}",
@@ -582,7 +582,7 @@ pub fn validate_blob_sidecar_for_gossip<T: BeaconChainTypes, O: ObservationStrat
             blob_sidecar.block_proposer_index(),
             block_root,
         )
-        .map_err(|e| GossipBlobError::BeaconChainError(e.into()))?;
+        .map_err(|e| GossipBlobError::BeaconChainError(Box::new(e.into())))?;
 
     if O::observe() {
         observe_gossip_blob(&kzg_verified_blob.blob, chain)?;
@@ -628,7 +628,7 @@ fn observe_gossip_blob<T: BeaconChainTypes>(
         .observed_blob_sidecars
         .write()
         .observe_sidecar(blob_sidecar)
-        .map_err(|e| GossipBlobError::BeaconChainError(e.into()))?
+        .map_err(|e| GossipBlobError::BeaconChainError(Box::new(e.into())))?
     {
         return Err(GossipBlobError::RepeatBlob {
             proposer: blob_sidecar.block_proposer_index(),