Rust 1.71 lints (#4503)

## Issue Addressed N/A ## Proposed Changes Add lints for rust 1.71 [3789134](3789134) is probably the one that needs most attention as it changes beacon state code. I changed the `is_in_inactivity_leak ` function to return a `ArithError` as not all consumers of that function work well with a `BeaconState::Error`.
sigp · Jul 13, 2023 · 33b9e28 · 33b9e28
1 parent ed3228b
commit 33b9e28
Show file tree

Hide file tree

Showing 20 changed files with 56 additions and 50 deletions.
diff --git a/beacon_node/beacon_chain/src/attestation_rewards.rs b/beacon_node/beacon_chain/src/attestation_rewards.rs
@@ -86,7 +86,7 @@ impl<T: BeaconChainTypes> BeaconChain<T> {
                 let ideal_reward = reward_numerator
                     .safe_div(active_increments)?
                     .safe_div(WEIGHT_DENOMINATOR)?;
-                if !state.is_in_inactivity_leak(previous_epoch, spec) {
+                if !state.is_in_inactivity_leak(previous_epoch, spec)? {
                     ideal_rewards_hashmap
                         .insert((flag_index, effective_balance), (ideal_reward, penalty));
                 } else {

diff --git a/beacon_node/execution_layer/src/lib.rs b/beacon_node/execution_layer/src/lib.rs
@@ -163,7 +163,7 @@ impl<T: EthSpec, Payload: AbstractExecPayload<T>> BlockProposalContents<T, Paylo
                 BlockProposalContents::Payload {
                     payload: Payload::default_at_fork(fork_name)?,
                     block_value: Uint256::zero(),
-                    _phantom: PhantomData::default(),
+                    _phantom: PhantomData,
                 }
             }
         })
@@ -858,7 +858,7 @@ impl<T: EthSpec> ExecutionLayer<T> {
                                     BlockProposalContents::Payload {
                                         payload: relay.data.message.header,
                                         block_value: relay.data.message.value,
-                                        _phantom: PhantomData::default(),
+                                        _phantom: PhantomData,
                                     },
                                 )),
                                 Err(reason) if !reason.payload_invalid() => {
@@ -913,7 +913,7 @@ impl<T: EthSpec> ExecutionLayer<T> {
                                     BlockProposalContents::Payload {
                                         payload: relay.data.message.header,
                                         block_value: relay.data.message.value,
-                                        _phantom: PhantomData::default(),
+                                        _phantom: PhantomData,
                                     },
                                 )),
                                 // If the payload is valid then use it. The local EE failed
@@ -922,7 +922,7 @@ impl<T: EthSpec> ExecutionLayer<T> {
                                     BlockProposalContents::Payload {
                                         payload: relay.data.message.header,
                                         block_value: relay.data.message.value,
-                                        _phantom: PhantomData::default(),
+                                        _phantom: PhantomData,
                                     },
                                 )),
                                 Err(reason) => {
@@ -1129,7 +1129,7 @@ impl<T: EthSpec> ExecutionLayer<T> {
                 Ok(BlockProposalContents::Payload {
                     payload: execution_payload.into(),
                     block_value,
-                    _phantom: PhantomData::default(),
+                    _phantom: PhantomData,
                 })
             })
             .await
@@ -2018,6 +2018,22 @@ async fn timed_future<F: Future<Output = T>, T>(metric: &str, future: F) -> (T,
     (result, duration)
 }
 
+fn noop<T: EthSpec>(
+    _: &ExecutionLayer<T>,
+    _: ExecutionPayloadRef<T>,
+) -> Option<ExecutionPayload<T>> {
+    None
+}
+
+#[cfg(test)]
+/// Returns the duration since the unix epoch.
+fn timestamp_now() -> u64 {
+    SystemTime::now()
+        .duration_since(UNIX_EPOCH)
+        .unwrap_or_else(|_| Duration::from_secs(0))
+        .as_secs()
+}
+
 #[cfg(test)]
 mod test {
     use super::*;
@@ -2164,19 +2180,3 @@ mod test {
             .await;
     }
 }
-
-fn noop<T: EthSpec>(
-    _: &ExecutionLayer<T>,
-    _: ExecutionPayloadRef<T>,
-) -> Option<ExecutionPayload<T>> {
-    None
-}
-
-#[cfg(test)]
-/// Returns the duration since the unix epoch.
-fn timestamp_now() -> u64 {
-    SystemTime::now()
-        .duration_since(UNIX_EPOCH)
-        .unwrap_or_else(|_| Duration::from_secs(0))
-        .as_secs()
-}
diff --git a/beacon_node/http_api/src/block_packing_efficiency.rs b/beacon_node/http_api/src/block_packing_efficiency.rs
@@ -75,7 +75,7 @@ impl<T: EthSpec> PackingEfficiencyHandler<T> {
             available_attestations: HashSet::new(),
             included_attestations: HashMap::new(),
             committee_store: CommitteeStore::new(),
-            _phantom: PhantomData::default(),
+            _phantom: PhantomData,
         };
 
         handler.compute_epoch(start_epoch, &starting_state, spec)?;

diff --git a/consensus/state_processing/src/lib.rs b/consensus/state_processing/src/lib.rs
@@ -2,7 +2,7 @@
 #![cfg_attr(
     not(test),
     deny(
-        clippy::integer_arithmetic,
+        clippy::arithmetic_side_effects,
         clippy::disallowed_methods,
         clippy::indexing_slicing,
         clippy::unwrap_used,

diff --git a/consensus/state_processing/src/per_block_processing/block_signature_verifier.rs b/consensus/state_processing/src/per_block_processing/block_signature_verifier.rs
@@ -1,4 +1,4 @@
-#![allow(clippy::integer_arithmetic)]
+#![allow(clippy::arithmetic_side_effects)]
 
 use super::signature_sets::{Error as SignatureSetError, *};
 use crate::per_block_processing::errors::{AttestationInvalid, BlockOperationError};

diff --git a/consensus/state_processing/src/per_block_processing/tests.rs b/consensus/state_processing/src/per_block_processing/tests.rs
@@ -960,7 +960,7 @@ async fn fork_spanning_exit() {
     spec.bellatrix_fork_epoch = Some(Epoch::new(4));
     spec.shard_committee_period = 0;
 
-    let harness = BeaconChainHarness::builder(MainnetEthSpec::default())
+    let harness = BeaconChainHarness::builder(MainnetEthSpec)
         .spec(spec.clone())
         .deterministic_keypairs(VALIDATOR_COUNT)
         .mock_execution_layer()

diff --git a/consensus/state_processing/src/per_epoch_processing/altair/inactivity_updates.rs b/consensus/state_processing/src/per_epoch_processing/altair/inactivity_updates.rs
@@ -34,7 +34,7 @@ pub fn process_inactivity_updates<T: EthSpec>(
                 .safe_add_assign(spec.inactivity_score_bias)?;
         }
         // Decrease the score of all validators for forgiveness when not during a leak
-        if !state.is_in_inactivity_leak(previous_epoch, spec) {
+        if !state.is_in_inactivity_leak(previous_epoch, spec)? {
             let inactivity_score = state.get_inactivity_score_mut(index)?;
             inactivity_score
                 .safe_sub_assign(min(spec.inactivity_score_recovery_rate, *inactivity_score))?;

diff --git a/consensus/state_processing/src/per_epoch_processing/altair/rewards_and_penalties.rs b/consensus/state_processing/src/per_epoch_processing/altair/rewards_and_penalties.rs
@@ -77,7 +77,7 @@ pub fn get_flag_index_deltas<T: EthSpec>(
         let mut delta = Delta::default();
 
         if unslashed_participating_indices.contains(index)? {
-            if !state.is_in_inactivity_leak(previous_epoch, spec) {
+            if !state.is_in_inactivity_leak(previous_epoch, spec)? {
                 let reward_numerator = base_reward
                     .safe_mul(weight)?
                     .safe_mul(unslashed_participating_increments)?;

diff --git a/consensus/state_processing/src/verify_operation.rs b/consensus/state_processing/src/verify_operation.rs
@@ -138,7 +138,7 @@ impl<E: EthSpec> VerifyOperation<E> for SignedVoluntaryExit {
         Ok(SigVerifiedOp::new(self, state))
     }
 
-    #[allow(clippy::integer_arithmetic)]
+    #[allow(clippy::arithmetic_side_effects)]
     fn verification_epochs(&self) -> SmallVec<[Epoch; MAX_FORKS_VERIFIED_AGAINST]> {
         smallvec![self.message.epoch]
     }
@@ -156,7 +156,7 @@ impl<E: EthSpec> VerifyOperation<E> for AttesterSlashing<E> {
         Ok(SigVerifiedOp::new(self, state))
     }
 
-    #[allow(clippy::integer_arithmetic)]
+    #[allow(clippy::arithmetic_side_effects)]
     fn verification_epochs(&self) -> SmallVec<[Epoch; MAX_FORKS_VERIFIED_AGAINST]> {
         smallvec![
             self.attestation_1.data.target.epoch,
@@ -177,7 +177,7 @@ impl<E: EthSpec> VerifyOperation<E> for ProposerSlashing {
         Ok(SigVerifiedOp::new(self, state))
     }
 
-    #[allow(clippy::integer_arithmetic)]
+    #[allow(clippy::arithmetic_side_effects)]
     fn verification_epochs(&self) -> SmallVec<[Epoch; MAX_FORKS_VERIFIED_AGAINST]> {
         // Only need a single epoch because the slots of the two headers must be equal.
         smallvec![self
@@ -200,7 +200,7 @@ impl<E: EthSpec> VerifyOperation<E> for SignedBlsToExecutionChange {
         Ok(SigVerifiedOp::new(self, state))
     }
 
-    #[allow(clippy::integer_arithmetic)]
+    #[allow(clippy::arithmetic_side_effects)]
     fn verification_epochs(&self) -> SmallVec<[Epoch; MAX_FORKS_VERIFIED_AGAINST]> {
         smallvec![]
     }

diff --git a/consensus/types/src/beacon_state.rs b/consensus/types/src/beacon_state.rs
@@ -442,7 +442,7 @@ impl<T: EthSpec> BeaconState<T> {
     }
 
     /// Specialised deserialisation method that uses the `ChainSpec` as context.
-    #[allow(clippy::integer_arithmetic)]
+    #[allow(clippy::arithmetic_side_effects)]
     pub fn from_ssz_bytes(bytes: &[u8], spec: &ChainSpec) -> Result<Self, ssz::DecodeError> {
         // Slot is after genesis_time (u64) and genesis_validators_root (Hash256).
         let slot_start = <u64 as Decode>::ssz_fixed_len() + <Hash256 as Decode>::ssz_fixed_len();
@@ -1734,16 +1734,22 @@ impl<T: EthSpec> BeaconState<T> {
         previous_epoch: Epoch,
         val_index: usize,
     ) -> Result<bool, Error> {
-        self.get_validator(val_index).map(|val| {
-            val.is_active_at(previous_epoch)
-                || (val.slashed && previous_epoch + Epoch::new(1) < val.withdrawable_epoch)
-        })
+        let val = self.get_validator(val_index)?;
+        Ok(val.is_active_at(previous_epoch)
+            || (val.slashed && previous_epoch.safe_add(Epoch::new(1))? < val.withdrawable_epoch))
     }
 
     /// Passing `previous_epoch` to this function rather than computing it internally provides
     /// a tangible speed improvement in state processing.
-    pub fn is_in_inactivity_leak(&self, previous_epoch: Epoch, spec: &ChainSpec) -> bool {
-        (previous_epoch - self.finalized_checkpoint().epoch) > spec.min_epochs_to_inactivity_penalty
+    pub fn is_in_inactivity_leak(
+        &self,
+        previous_epoch: Epoch,
+        spec: &ChainSpec,
+    ) -> Result<bool, safe_arith::ArithError> {
+        Ok(
+            (previous_epoch.safe_sub(self.finalized_checkpoint().epoch)?)
+                > spec.min_epochs_to_inactivity_penalty,
+        )
     }
 
     /// Get the `SyncCommittee` associated with the next slot. Useful because sync committees

diff --git a/consensus/types/src/beacon_state/committee_cache.rs b/consensus/types/src/beacon_state/committee_cache.rs
@@ -1,4 +1,4 @@
-#![allow(clippy::integer_arithmetic)]
+#![allow(clippy::arithmetic_side_effects)]
 
 use super::BeaconState;
 use crate::*;

diff --git a/consensus/types/src/beacon_state/tree_hash_cache.rs b/consensus/types/src/beacon_state/tree_hash_cache.rs
@@ -1,4 +1,4 @@
-#![allow(clippy::integer_arithmetic)]
+#![allow(clippy::arithmetic_side_effects)]
 #![allow(clippy::disallowed_methods)]
 #![allow(clippy::indexing_slicing)]
 

diff --git a/consensus/types/src/chain_spec.rs b/consensus/types/src/chain_spec.rs
@@ -451,7 +451,7 @@ impl ChainSpec {
         Hash256::from(domain)
     }
 
-    #[allow(clippy::integer_arithmetic)]
+    #[allow(clippy::arithmetic_side_effects)]
     pub const fn attestation_subnet_prefix_bits(&self) -> u32 {
         let attestation_subnet_count_bits = self.attestation_subnet_count.ilog2();
         self.attestation_subnet_extra_bits as u32 + attestation_subnet_count_bits

diff --git a/consensus/types/src/execution_payload.rs b/consensus/types/src/execution_payload.rs
@@ -106,7 +106,7 @@ impl<T: EthSpec> ExecutionPayload<T> {
         }
     }
 
-    #[allow(clippy::integer_arithmetic)]
+    #[allow(clippy::arithmetic_side_effects)]
     /// Returns the maximum size of an execution payload.
     pub fn max_execution_payload_merge_size() -> usize {
         // Fixed part
@@ -117,7 +117,7 @@ impl<T: EthSpec> ExecutionPayload<T> {
             + (T::max_transactions_per_payload() * (ssz::BYTES_PER_LENGTH_OFFSET + T::max_bytes_per_transaction()))
     }
 
-    #[allow(clippy::integer_arithmetic)]
+    #[allow(clippy::arithmetic_side_effects)]
     /// Returns the maximum size of an execution payload.
     pub fn max_execution_payload_capella_size() -> usize {
         // Fixed part

diff --git a/consensus/types/src/lib.rs b/consensus/types/src/lib.rs
@@ -3,7 +3,7 @@
 #![cfg_attr(
     not(test),
     deny(
-        clippy::integer_arithmetic,
+        clippy::arithmetic_side_effects,
         clippy::disallowed_methods,
         clippy::indexing_slicing
     )

diff --git a/consensus/types/src/participation_list.rs b/consensus/types/src/participation_list.rs
@@ -1,4 +1,4 @@
-#![allow(clippy::integer_arithmetic)]
+#![allow(clippy::arithmetic_side_effects)]
 
 use crate::{Hash256, ParticipationFlags, Unsigned, VariableList};
 use cached_tree_hash::{int_log, CacheArena, CachedTreeHash, Error, TreeHashCache};

diff --git a/consensus/types/src/subnet_id.rs b/consensus/types/src/subnet_id.rs
@@ -72,7 +72,7 @@ impl SubnetId {
             .into())
     }
 
-    #[allow(clippy::integer_arithmetic)]
+    #[allow(clippy::arithmetic_side_effects)]
     /// Computes the set of subnets the node should be subscribed to during the current epoch,
     /// along with the first epoch in which these subscriptions are no longer valid.
     pub fn compute_subnets_for_epoch<T: EthSpec>(

diff --git a/consensus/types/src/test_utils/mod.rs b/consensus/types/src/test_utils/mod.rs
@@ -1,4 +1,4 @@
-#![allow(clippy::integer_arithmetic)]
+#![allow(clippy::arithmetic_side_effects)]
 
 use std::fmt::Debug;
 

diff --git a/consensus/types/src/test_utils/test_random.rs b/consensus/types/src/test_utils/test_random.rs
@@ -28,7 +28,7 @@ pub trait TestRandom {
 
 impl<T> TestRandom for PhantomData<T> {
     fn random_for_test(_rng: &mut impl RngCore) -> Self {
-        PhantomData::default()
+        PhantomData
     }
 }
 

diff --git a/slasher/tests/backend.rs b/slasher/tests/backend.rs
@@ -1,4 +1,4 @@
-#![cfg(all(feature = "lmdb"))]
+#![cfg(feature = "lmdb")]
 
 use slasher::{config::MDBX_DATA_FILENAME, Config, DatabaseBackend, DatabaseBackendOverride};
 use std::fs::File;