Address clippy arith lints (#1038)

beacon_node/rest_api/src/helpers.rs

@@ -9,7 +9,7 @@ use network::NetworkMessage;
 use ssz::Decode;
 use store::{iter::AncestorIter, Store};
 use types::{
-    Attestation, BeaconState, CommitteeIndex, Epoch, EthSpec, Hash256, RelativeEpoch,
+    Attestation, BeaconState, ChainSpec, CommitteeIndex, Epoch, EthSpec, Hash256, RelativeEpoch,
     SignedAggregateAndProof, SignedBeaconBlock, Slot,
 };
 
@@ -251,15 +251,22 @@ pub fn publish_beacon_block_to_network<T: BeaconChainTypes + 'static>(
 pub fn publish_raw_attestations_to_network<T: BeaconChainTypes + 'static>(
     mut chan: NetworkChannel<T::EthSpec>,
     attestations: Vec<Attestation<T::EthSpec>>,
+    spec: &ChainSpec,
 ) -> Result<(), ApiError> {
     let messages = attestations
         .into_iter()
         .map(|attestation| {
             // create the gossip message to send to the network
-            let subnet_id = attestation.subnet_id();
+            let subnet_id = attestation
-            PubsubMessage::Attestation(Box::new((subnet_id, attestation)))
+                .subnet_id(spec)
+                .map_err(|e| ApiError::ServerError(format!("Unable to get subnet id: {:?}", e)))?;
+
+            Ok(PubsubMessage::Attestation(Box::new((
+                subnet_id,
+                attestation,
+            ))))
         })
-        .collect::<Vec<_>>();
+        .collect::<Result<Vec<_>, ApiError>>()?;
 
     // Publish the attestations to the p2p network via gossipsub.
     if let Err(e) = chan.try_send(NetworkMessage::Publish { messages }) {

beacon_node/rest_api/src/validator.rs

@@ -507,10 +507,11 @@ pub fn publish_attestations<T: BeaconChainTypes>(
                     }
                 }
             })?;
-            Ok(attestations)
+
+            Ok((attestations, beacon_chain))
             })
-            .and_then(|attestations| {
+            .and_then(|(attestations, beacon_chain)| {
-                   publish_raw_attestations_to_network::<T>(network_chan, attestations)
+                   publish_raw_attestations_to_network::<T>(network_chan, attestations, &beacon_chain.spec)
             })
             .and_then(|_| response_builder?.body_no_ssz(&())),
     )

eth2/types/src/attestation.rs

@@ -3,6 +3,7 @@ use super::{
     Signature, SignedRoot, SubnetId,
 };
 use crate::{test_utils::TestRandom, Hash256};
+use safe_arith::{ArithError, SafeArith};
 
 use serde_derive::{Deserialize, Serialize};
 use ssz_derive::{Decode, Encode};
@@ -13,6 +14,7 @@ use tree_hash_derive::TreeHash;
 pub enum Error {
     SszTypesError(ssz_types::Error),
     AlreadySigned(usize),
+    SubnetCountIsZero(ArithError),
 }
 
 /// Details an attestation that can be slashable.
@@ -86,8 +88,12 @@ impl<T: EthSpec> Attestation<T> {
     ///
     /// Note, this will return the subnet id for an aggregated attestation. This is done
     /// to avoid checking aggregate bits every time we wish to get an id.
-    pub fn subnet_id(&self) -> SubnetId {
+    pub fn subnet_id(&self, spec: &ChainSpec) -> Result<SubnetId, Error> {
-        SubnetId::new(self.data.index % T::default_spec().attestation_subnet_count)
+        self.data
+            .index
+            .safe_rem(spec.attestation_subnet_count)
+            .map(SubnetId::new)
+            .map_err(Error::SubnetCountIsZero)
     }
 }
 

eth2/types/src/beacon_state.rs

@@ -485,7 +485,7 @@ impl<T: EthSpec> BeaconState<T> {
         let committee = self.get_beacon_committee(slot, index)?;
         let modulo = std::cmp::max(
             1,
-            committee.committee.len() as u64 / spec.target_aggregators_per_committee,
+            (committee.committee.len() as u64).safe_div(spec.target_aggregators_per_committee)?,
         );
         let signature_hash = hash(&slot_signature.as_bytes());
         let signature_hash_int = u64::from_le_bytes(
@@ -493,7 +493,8 @@ impl<T: EthSpec> BeaconState<T> {
                 .try_into()
                 .expect("first 8 bytes of signature should always convert to fixed array"),
         );
-        Ok(signature_hash_int % modulo == 0)
+
+        Ok(signature_hash_int.safe_rem(modulo)? == 0)
     }
 
     /// Returns the beacon proposer index for the `slot` in the given `relative_epoch`.

eth2/types/src/selection_proof.rs

@@ -1,4 +1,5 @@
 use crate::{ChainSpec, Domain, EthSpec, Fork, Hash256, SecretKey, Signature, SignedRoot, Slot};
+use safe_arith::{ArithError, SafeArith};
 use std::convert::TryInto;
 use tree_hash::TreeHash;
 
@@ -24,7 +25,7 @@ impl SelectionProof {
         Self(Signature::new(message.as_bytes(), secret_key))
     }
 
-    pub fn is_aggregator(&self, modulo: u64) -> bool {
+    pub fn is_aggregator(&self, modulo: u64) -> Result<bool, ArithError> {
         let signature_hash = self.0.tree_hash_root();
         let signature_hash_int = u64::from_le_bytes(
             signature_hash[0..8]
@@ -33,7 +34,7 @@ impl SelectionProof {
                 .expect("first 8 bytes of signature should always convert to fixed array"),
         );
 
-        signature_hash_int % modulo == 0
+        signature_hash_int.safe_rem(modulo).map(|rem| rem == 0)
     }
 }
 

validator_client/src/attestation_service.rs

@@ -271,12 +271,14 @@ impl<T: SlotClock + 'static, E: EthSpec> AttestationService<T, E> {
                     .validator_store
                     .produce_selection_proof(duty.validator_pubkey(), slot)?;
                 let modulo = duty.duty.aggregator_modulo?;
-
                 let subscription = ValidatorSubscription {
                     validator_index,
                     attestation_committee_index,
                     slot,
-                    is_aggregator: selection_proof.is_aggregator(modulo),
+                    is_aggregator: selection_proof
+                        .is_aggregator(modulo)
+                        .map_err(|e| crit!(log_1, "Unable to determine aggregator: {:?}", e))
+                        .ok()?,
                 };
 
                 Some((subscription, (duty, selection_proof)))