Skip to content

Commit

Permalink
Ban unsafe arithmetic operations
Browse files Browse the repository at this point in the history
  • Loading branch information
keithtensor committed Jun 18, 2024
1 parent b4ca34b commit 847da3d
Show file tree
Hide file tree
Showing 44 changed files with 398 additions and 228 deletions.
3 changes: 3 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,10 @@ members = [
resolver = "2"

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

[workspace.dependencies]
cargo-husky = { version = "1", default-features = false }
Expand Down
6 changes: 4 additions & 2 deletions node/src/chain_spec/finney.rs
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ use super::*;

pub fn finney_mainnet_config() -> Result<ChainSpec, String> {
let path: PathBuf = std::path::PathBuf::from("./snapshot.json");
let wasm_binary = WASM_BINARY.ok_or_else(|| "Development wasm not available".to_string())?;
let wasm_binary = WASM_BINARY.ok_or("Development wasm not available".to_string())?;

// We mmap the file into memory first, as this is *a lot* faster than using
// `serde_json::from_reader`. See https://github.com/serde-rs/json/issues/160
Expand Down Expand Up @@ -53,7 +53,9 @@ 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 = balances_issuance
.checked_add(*amount)
.ok_or("Balances issuance overflowed".to_string())?;
}

// Give front-ends necessary data to present to users
Expand Down
6 changes: 4 additions & 2 deletions node/src/chain_spec/testnet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ pub fn finney_testnet_config() -> Result<ChainSpec, String> {
};

let old_state: ColdkeyHotkeys =
json::from_slice(&bytes).map_err(|e| format!("Error parsing genesis file: {}", e))?;
json::from_slice(&bytes).map_err(|e| format!("Error parsing genesis file: {e}"))?;

let mut processed_stakes: Vec<(
sp_runtime::AccountId32,
Expand Down Expand Up @@ -53,7 +53,9 @@ 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 = balances_issuance
.checked_add(*amount)
.ok_or("Balances issuance overflowed".to_string())?;
}

// Give front-ends necessary data to present to users
Expand Down
1 change: 1 addition & 0 deletions pallets/admin-utils/src/benchmarking.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Benchmarking setup
#![cfg(feature = "runtime-benchmarks")]
#![allow(clippy::arithmetic_side_effects)]
use super::*;

#[allow(unused)]
Expand Down
2 changes: 2 additions & 0 deletions pallets/admin-utils/tests/mock.rs
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
#![allow(clippy::arithmetic_side_effects, clippy::unwrap_used)]

use frame_support::{
assert_ok, derive_impl, parameter_types,
traits::{Everything, Hooks},
Expand Down
3 changes: 2 additions & 1 deletion pallets/collective/src/benchmarking.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@
// limitations under the License.

//! Staking pallet benchmarking.
#![allow(clippy::arithmetic_side_effects, clippy::indexing_slicing)]

use super::*;
use crate::Pallet as Collective;
Expand Down Expand Up @@ -70,7 +71,7 @@ benchmarks_instance_pallet! {
// Proposals should be different so that different proposal hashes are generated
let proposal: T::Proposal = SystemCall::<T>::remark { remark: id_to_remark_data(i, length) }.into();
Collective::<T, I>::propose(
SystemOrigin::Signed(old_members.last().unwrap().clone()).into(),
SystemOrigin::Signed(old_members.last().expect("m is greater than 0; old_members must have at least 1 element; qed").clone()).into(),
Box::new(proposal.clone()),
MAX_BYTES,
TryInto::<BlockNumberFor<T>>::try_into(3u64).ok().expect("convert u64 to block number.")
Expand Down
29 changes: 18 additions & 11 deletions pallets/collective/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,7 +54,7 @@ use frame_support::{
use scale_info::TypeInfo;
use sp_io::storage;
use sp_runtime::traits::Dispatchable;
use sp_runtime::{traits::Hash, RuntimeDebug};
use sp_runtime::{traits::Hash, RuntimeDebug, Saturating};
use sp_std::{marker::PhantomData, prelude::*, result};

#[cfg(test)]
Expand Down Expand Up @@ -119,7 +119,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 @@ -545,7 +545,9 @@ pub mod pallet {
Error::<T, I>::DurationLowerThanConfiguredMotionDuration
);

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 @@ -716,10 +718,15 @@ 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>::TooManyActiveProposals)?;
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 @@ -862,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 @@ -981,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 @@ -1154,7 +1161,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 @@ -1179,7 +1186,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
2 changes: 1 addition & 1 deletion pallets/collective/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
// See the License for the specific language governing permissions and
// limitations under the License.

#![allow(non_camel_case_types)]
#![allow(non_camel_case_types, clippy::indexing_slicing, clippy::unwrap_used)]

use super::{Event as CollectiveEvent, *};
use crate as pallet_collective;
Expand Down
7 changes: 6 additions & 1 deletion pallets/commitments/src/benchmarking.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Benchmarking setup
#![cfg(feature = "runtime-benchmarks")]
#![allow(clippy::arithmetic_side_effects)]
use super::*;

#[allow(unused)]
Expand All @@ -17,7 +18,11 @@ fn assert_last_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {
// This creates an `IdentityInfo` object with `num_fields` extra fields.
// All data is pre-populated with some arbitrary bytes.
fn create_identity_info<T: Config>(_num_fields: u32) -> CommitmentInfo<T::MaxFields> {
let _data = Data::Raw(vec![0; 32].try_into().unwrap());
let _data = Data::Raw(
vec![0; 32]
.try_into()
.expect("vec length is less than 64; qed"),
);

CommitmentInfo {
fields: Default::default(),
Expand Down
13 changes: 7 additions & 6 deletions pallets/commitments/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,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 @@ -136,12 +136,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>::CommitmentSetRateLimitExceeded
);
}

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 @@ -156,12 +156,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
7 changes: 4 additions & 3 deletions pallets/commitments/src/types.rs
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 Expand Up @@ -344,6 +344,7 @@ impl<
}

#[cfg(test)]
#[allow(clippy::indexing_slicing, clippy::unwrap_used)]
mod tests {
use super::*;

Expand Down
7 changes: 6 additions & 1 deletion pallets/registry/src/benchmarking.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
//! Benchmarking setup
#![cfg(feature = "runtime-benchmarks")]
#![allow(clippy::arithmetic_side_effects, clippy::unwrap_used)]
use super::*;

#[allow(unused)]
Expand All @@ -19,7 +20,11 @@ fn assert_last_event<T: Config>(generic_event: <T as Config>::RuntimeEvent) {
// This creates an `IdentityInfo` object with `num_fields` extra fields.
// All data is pre-populated with some arbitrary bytes.
fn create_identity_info<T: Config>(_num_fields: u32) -> IdentityInfo<T::MaxAdditionalFields> {
let data = Data::Raw(vec![0; 32].try_into().unwrap());
let data = Data::Raw(
vec![0; 32]
.try_into()
.expect("size does not exceed 64; qed"),
);

IdentityInfo {
additional: Default::default(),
Expand Down
15 changes: 8 additions & 7 deletions pallets/registry/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,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 @@ -132,7 +132,7 @@ pub mod pallet {
Error::<T>::TooManyFieldsInIdentityInfo
);

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 @@ -145,23 +145,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
7 changes: 4 additions & 3 deletions pallets/registry/src/types.rs
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 Expand Up @@ -403,6 +403,7 @@ impl<
}

#[cfg(test)]
#[allow(clippy::indexing_slicing, clippy::unwrap_used)]
mod tests {
use super::*;

Expand Down
2 changes: 1 addition & 1 deletion pallets/subtensor/src/benchmarks.rs
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
//! Subtensor pallet benchmarking.

#![allow(clippy::arithmetic_side_effects, clippy::unwrap_used)]
#![cfg(feature = "runtime-benchmarks")]

use crate::Pallet as Subtensor;
Expand Down
Loading

0 comments on commit 847da3d

Please sign in to comment.