Skip to content

Commit

Permalink
Ban unsafe arithmetic operations
Browse files Browse the repository at this point in the history
  • Loading branch information
@keithtensor
keithtensor committed May 30, 2024
1 parent 686e6fb commit c5620f9
Show file tree
Hide file tree
Showing 23 changed files with 468 additions and 234 deletions.
3 changes: 3 additions & 0 deletions Cargo.toml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ members = [
resolver = "2"

[workspace.lints.clippy]
indexing-slicing = "deny"
arithmetic-side-effects = "deny"
type_complexity = "allow"
unwrap-used = "deny"

[profile.release]
panic = "unwind"
Expand Down
5 changes: 3 additions & 2 deletions node/src/chain_spec.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ use sp_consensus_grandpa::AuthorityId as GrandpaId;
use sp_core::crypto::Ss58Codec;
use sp_core::{bounded_vec, sr25519, Pair, Public};
use sp_runtime::traits::{IdentifyAccount, Verify};
use sp_runtime::Saturating;
use std::env;

// The URL for the telemetry server.
Expand Down Expand Up @@ -119,7 +120,7 @@ pub fn finney_mainnet_config() -> Result<ChainSpec, String> {
let key_account = sp_runtime::AccountId32::from(key);

processed_balances.push((key_account, *amount));
balances_issuance += *amount;
balances_issuance.saturating_accrue(*amount);
}

// Give front-ends necessary data to present to users
Expand Down Expand Up @@ -298,7 +299,7 @@ pub fn finney_testnet_config() -> Result<ChainSpec, String> {
let key_account = sp_runtime::AccountId32::from(key);

processed_balances.push((key_account, *amount));
balances_issuance += *amount;
balances_issuance.saturating_accrue(*amount);
}

// Give front-ends necessary data to present to users
Expand Down
27 changes: 16 additions & 11 deletions pallets/collective/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@

use scale_info::TypeInfo;
use sp_io::storage;
use sp_runtime::{traits::Hash, RuntimeDebug};
use sp_runtime::{traits::Hash, RuntimeDebug, Saturating};
use sp_std::{marker::PhantomData, prelude::*, result};

use frame_support::{
Expand Down Expand Up @@ -122,7 +122,7 @@ impl DefaultVote for MoreThanMajorityThenPrimeDefaultVote {
_no_votes: MemberCount,
len: MemberCount,
) -> bool {
let more_than_majority = yes_votes * 2 > len;
let more_than_majority = yes_votes.saturating_mul(2) > len;
more_than_majority || prime_vote.unwrap_or(false)
}
}
Expand Down Expand Up @@ -527,7 +527,9 @@ pub mod pallet {
Error::<T, I>::WrongDuration
);

let threshold = (T::GetVotingMembers::get_count() / 2) + 1;
let threshold = T::GetVotingMembers::get_count()
.saturating_div(2)
.saturating_add(1);

let members = Self::members();
let (proposal_len, active_proposals) =
Expand Down Expand Up @@ -724,10 +726,13 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
})?;

let index = Self::proposal_count();
<ProposalCount<T, I>>::mutate(|i| *i += 1);
<ProposalCount<T, I>>::try_mutate(|i| {
*i = i.checked_add(1).ok_or(Error::<T, I>::TooManyProposals)?;
Ok::<(), Error<T, I>>(())
})?;
<ProposalOf<T, I>>::insert(proposal_hash, proposal);
let votes = {
let end = frame_system::Pallet::<T>::block_number() + duration;
let end = frame_system::Pallet::<T>::block_number().saturating_add(duration);
Votes {
index,
threshold,
Expand Down Expand Up @@ -864,10 +869,10 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
// default voting strategy.
let default = T::DefaultVote::default_vote(prime_vote, yes_votes, no_votes, seats);

let abstentions = seats - (yes_votes + no_votes);
let abstentions = seats.saturating_sub(yes_votes.saturating_add(no_votes));
match default {
true => yes_votes += abstentions,
false => no_votes += abstentions,
true => yes_votes = yes_votes.saturating_add(abstentions),
false => no_votes = no_votes.saturating_add(abstentions),
}
let approved = yes_votes >= voting.threshold;

Expand Down Expand Up @@ -983,7 +988,7 @@ impl<T: Config<I>, I: 'static> Pallet<T, I> {
Voting::<T, I>::remove(proposal_hash);
let num_proposals = Proposals::<T, I>::mutate(|proposals| {
proposals.retain(|h| h != &proposal_hash);
proposals.len() + 1 // calculate weight based on original length
proposals.len().saturating_add(1) // calculate weight based on original length
});
num_proposals as u32
}
Expand Down Expand Up @@ -1153,7 +1158,7 @@ impl<
type Success = ();
fn try_origin(o: O) -> Result<Self::Success, O> {
o.into().and_then(|o| match o {
RawOrigin::Members(n, m) if n * D > N * m => Ok(()),
RawOrigin::Members(n, m) if n.saturating_mul(D) > N.saturating_mul(m) => Ok(()),
r => Err(O::from(r)),
})
}
Expand All @@ -1178,7 +1183,7 @@ impl<
type Success = ();
fn try_origin(o: O) -> Result<Self::Success, O> {
o.into().and_then(|o| match o {
RawOrigin::Members(n, m) if n * D >= N * m => Ok(()),
RawOrigin::Members(n, m) if n.saturating_mul(D) >= N.saturating_mul(m) => Ok(()),
r => Err(O::from(r)),
})
}
Expand Down
13 changes: 7 additions & 6 deletions pallets/commitments/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@ pub use types::*;
pub use weights::WeightInfo;

use frame_support::traits::Currency;
use sp_runtime::traits::Zero;
use sp_runtime::{traits::Zero, Saturating};
use sp_std::boxed::Box;

type BalanceOf<T> =
Expand Down Expand Up @@ -129,12 +129,12 @@ pub mod pallet {
let cur_block = <frame_system::Pallet<T>>::block_number();
if let Some(last_commit) = <LastCommitment<T>>::get(netuid, &who) {
ensure!(
cur_block >= last_commit + T::RateLimit::get(),
cur_block >= last_commit.saturating_add(T::RateLimit::get()),
Error::<T>::RateLimitExceeded
);
}

let fd = <BalanceOf<T>>::from(extra_fields) * T::FieldDeposit::get();
let fd = <BalanceOf<T>>::from(extra_fields).saturating_mul(T::FieldDeposit::get());
let mut id = match <CommitmentOf<T>>::get(netuid, &who) {
Some(mut id) => {
id.info = *info;
Expand All @@ -149,12 +149,13 @@ pub mod pallet {
};

let old_deposit = id.deposit;
id.deposit = T::InitialDeposit::get() + fd;
id.deposit = T::InitialDeposit::get().saturating_add(fd);
if id.deposit > old_deposit {
T::Currency::reserve(&who, id.deposit - old_deposit)?;
T::Currency::reserve(&who, id.deposit.saturating_sub(old_deposit))?;
}
if old_deposit > id.deposit {
let err_amount = T::Currency::unreserve(&who, old_deposit - id.deposit);
let err_amount =
T::Currency::unreserve(&who, old_deposit.saturating_sub(id.deposit));
debug_assert!(err_amount.is_zero());
}

Expand Down
6 changes: 3 additions & 3 deletions pallets/commitments/src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ impl Decode for Data {
Ok(match b {
0 => Data::None,
n @ 1..=129 => {
let mut r: BoundedVec<_, _> = vec![0u8; n as usize - 1]
let mut r: BoundedVec<_, _> = vec![0u8; (n as usize).saturating_sub(1)]
.try_into()
.expect("bound checked in match arm condition; qed");
input.read(&mut r[..])?;
Expand All @@ -86,8 +86,8 @@ impl Encode for Data {
match self {
Data::None => vec![0u8; 1],
Data::Raw(ref x) => {
let l = x.len().min(128);
let mut r = vec![l as u8 + 1];
let l = x.len().min(128) as u8;
let mut r = vec![l.saturating_add(1)];
r.extend_from_slice(&x[..]);
r
}
Expand Down
15 changes: 8 additions & 7 deletions pallets/registry/src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ use frame_support::traits::tokens::{
fungible::{self, MutateHold as _},
Precision,
};
use sp_runtime::traits::Zero;
use sp_runtime::{traits::Zero, Saturating};
use sp_std::boxed::Box;

type BalanceOf<T> =
Expand Down Expand Up @@ -122,7 +122,7 @@ pub mod pallet {
Error::<T>::TooManyFields
);

let fd = <BalanceOf<T>>::from(extra_fields) * T::FieldDeposit::get();
let fd = <BalanceOf<T>>::from(extra_fields).saturating_mul(T::FieldDeposit::get());
let mut id = match <IdentityOf<T>>::get(&identified) {
Some(mut id) => {
id.info = *info;
Expand All @@ -135,23 +135,24 @@ pub mod pallet {
};

let old_deposit = id.deposit;
id.deposit = T::InitialDeposit::get() + fd;
id.deposit = T::InitialDeposit::get().saturating_add(fd);
if id.deposit > old_deposit {
T::Currency::hold(
&HoldReason::RegistryIdentity.into(),
&who,
id.deposit - old_deposit,
id.deposit.saturating_sub(old_deposit),
)?;
}
if old_deposit > id.deposit {
let release_res = T::Currency::release(
&HoldReason::RegistryIdentity.into(),
&who,
old_deposit - id.deposit,
old_deposit.saturating_sub(id.deposit),
Precision::BestEffort,
);
debug_assert!(release_res
.is_ok_and(|released_amount| released_amount == (old_deposit - id.deposit)));
debug_assert!(release_res.is_ok_and(
|released_amount| released_amount == old_deposit.saturating_sub(id.deposit)
));
}

<IdentityOf<T>>::insert(&identified, id);
Expand Down
6 changes: 3 additions & 3 deletions pallets/registry/src/types.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,7 @@ impl Decode for Data {
Ok(match b {
0 => Data::None,
n @ 1..=65 => {
let mut r: BoundedVec<_, _> = vec![0u8; n as usize - 1]
let mut r: BoundedVec<_, _> = vec![0u8; (n as usize).saturating_sub(1)]
.try_into()
.expect("bound checked in match arm condition; qed");
input.read(&mut r[..])?;
Expand All @@ -87,8 +87,8 @@ impl Encode for Data {
match self {
Data::None => vec![0u8; 1],
Data::Raw(ref x) => {
let l = x.len().min(64);
let mut r = vec![l as u8 + 1];
let l = x.len().min(64) as u8;
let mut r = vec![l.saturating_add(1)];
r.extend_from_slice(&x[..]);
r
}
Expand Down
Loading

0 comments on commit c5620f9

Please sign in to comment.