Skip to content

Commit

Permalink
pallet-staking: Converts all math operations to safe (paritytech#2435)
Browse files Browse the repository at this point in the history
This PR converts unsafe math operations to safe in the staking pallet.
 
Closes paritytech#2431

---------

Co-authored-by: Ankan <[email protected]>
  • Loading branch information
gpestana and Ank4n authored Nov 24, 2023
1 parent e80253d commit 40984e3
Show file tree
Hide file tree
Showing 4 changed files with 33 additions and 23 deletions.
11 changes: 6 additions & 5 deletions substrate/frame/staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -530,12 +530,12 @@ impl<T: Config> StakingLedger<T> {
let mut unlocking_balance = BalanceOf::<T>::zero();

while let Some(last) = self.unlocking.last_mut() {
if unlocking_balance + last.value <= value {
if unlocking_balance.defensive_saturating_add(last.value) <= value {
unlocking_balance += last.value;
self.active += last.value;
self.unlocking.pop();
} else {
let diff = value - unlocking_balance;
let diff = value.defensive_saturating_sub(unlocking_balance);

unlocking_balance += diff;
self.active += diff;
Expand Down Expand Up @@ -589,7 +589,7 @@ impl<T: Config> StakingLedger<T> {

// for a `slash_era = x`, any chunk that is scheduled to be unlocked at era `x + 28`
// (assuming 28 is the bonding duration) onwards should be slashed.
let slashable_chunks_start = slash_era + T::BondingDuration::get();
let slashable_chunks_start = slash_era.saturating_add(T::BondingDuration::get());

// `Some(ratio)` if this is proportional, with `ratio`, `None` otherwise. In both cases, we
// slash first the active chunk, and then `slash_chunks_priority`.
Expand Down Expand Up @@ -1185,8 +1185,9 @@ impl<T: Config> EraInfo<T> {

let nominator_count = exposure.others.len();
// expected page count is the number of nominators divided by the page size, rounded up.
let expected_page_count =
nominator_count.defensive_saturating_add(page_size as usize - 1) / page_size as usize;
let expected_page_count = nominator_count
.defensive_saturating_add((page_size as usize).defensive_saturating_sub(1))
.saturating_div(page_size as usize);

let (exposure_metadata, exposure_pages) = exposure.into_pages(page_size);
defensive_assert!(exposure_pages.len() == expected_page_count, "unexpected page count");
Expand Down
14 changes: 8 additions & 6 deletions substrate/frame/staking/src/pallet/impls.rs
Original file line number Diff line number Diff line change
Expand Up @@ -27,8 +27,8 @@ use frame_support::{
dispatch::WithPostDispatchInfo,
pallet_prelude::*,
traits::{
Currency, Defensive, EstimateNextNewSession, Get, Imbalance, Len, OnUnbalanced, TryCollect,
UnixTime,
Currency, Defensive, DefensiveSaturating, EstimateNextNewSession, Get, Imbalance, Len,
OnUnbalanced, TryCollect, UnixTime,
},
weights::Weight,
};
Expand Down Expand Up @@ -148,7 +148,7 @@ impl<T: Config> Pallet<T> {
// `consolidate_unlocked` strictly subtracts balance.
if new_total < old_total {
// Already checked that this won't overflow by entry condition.
let value = old_total - new_total;
let value = old_total.defensive_saturating_sub(new_total);
Self::deposit_event(Event::<T>::Withdrawn { stash, amount: value });
}

Expand Down Expand Up @@ -262,7 +262,8 @@ impl<T: Config> Pallet<T> {
// total commission validator takes across all nominator pages
let validator_total_commission_payout = validator_commission * validator_total_payout;

let validator_leftover_payout = validator_total_payout - validator_total_commission_payout;
let validator_leftover_payout =
validator_total_payout.defensive_saturating_sub(validator_total_commission_payout);
// Now let's calculate how this is split to the validator.
let validator_exposure_part = Perbill::from_rational(exposure.own(), exposure.total());
let validator_staking_payout = validator_exposure_part * validator_leftover_payout;
Expand Down Expand Up @@ -475,7 +476,7 @@ impl<T: Config> Pallet<T> {
bonded.push((active_era, start_session));

if active_era > bonding_duration {
let first_kept = active_era - bonding_duration;
let first_kept = active_era.defensive_saturating_sub(bonding_duration);

// Prune out everything that's from before the first-kept index.
let n_to_prune =
Expand All @@ -501,7 +502,8 @@ impl<T: Config> Pallet<T> {
if let Some(active_era_start) = active_era.start {
let now_as_millis_u64 = T::UnixTime::now().as_millis().saturated_into::<u64>();

let era_duration = (now_as_millis_u64 - active_era_start).saturated_into::<u64>();
let era_duration = (now_as_millis_u64.defensive_saturating_sub(active_era_start))
.saturated_into::<u64>();
let staked = Self::eras_total_stake(&active_era.index);
let issuance = T::Currency::total_issuance();
let (validator_payout, remainder) =
Expand Down
4 changes: 3 additions & 1 deletion substrate/frame/staking/src/pallet/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1068,7 +1068,9 @@ pub mod pallet {
ensure!(ledger.active >= min_active_bond, Error::<T>::InsufficientBond);

// Note: in case there is no current era it is fine to bond one era more.
let era = Self::current_era().unwrap_or(0) + T::BondingDuration::get();
let era = Self::current_era()
.unwrap_or(0)
.defensive_saturating_add(T::BondingDuration::get());
if let Some(chunk) = ledger.unlocking.last_mut().filter(|chunk| chunk.era == era) {
// To keep the chunk count down, we only keep one chunk per era. Since
// `unlocking` is a FiFo queue, if a chunk exists for `era` we know that it will
Expand Down
27 changes: 16 additions & 11 deletions substrate/frame/staking/src/slashing.rs
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ use crate::{
use codec::{Decode, Encode, MaxEncodedLen};
use frame_support::{
ensure,
traits::{Currency, Defensive, Get, Imbalance, OnUnbalanced},
traits::{Currency, Defensive, DefensiveSaturating, Get, Imbalance, OnUnbalanced},
};
use scale_info::TypeInfo;
use sp_runtime::{
Expand Down Expand Up @@ -85,7 +85,7 @@ pub(crate) struct SlashingSpan {

impl SlashingSpan {
fn contains_era(&self, era: EraIndex) -> bool {
self.start <= era && self.length.map_or(true, |l| self.start + l > era)
self.start <= era && self.length.map_or(true, |l| self.start.saturating_add(l) > era)
}
}

Expand Down Expand Up @@ -123,15 +123,15 @@ impl SlashingSpans {
// returns `true` if a new span was started, `false` otherwise. `false` indicates
// that internal state is unchanged.
pub(crate) fn end_span(&mut self, now: EraIndex) -> bool {
let next_start = now + 1;
let next_start = now.defensive_saturating_add(1);
if next_start <= self.last_start {
return false
}

let last_length = next_start - self.last_start;
let last_length = next_start.defensive_saturating_sub(self.last_start);
self.prior.insert(0, last_length);
self.last_start = next_start;
self.span_index += 1;
self.span_index.defensive_saturating_accrue(1);
true
}

Expand All @@ -141,9 +141,9 @@ impl SlashingSpans {
let mut index = self.span_index;
let last = SlashingSpan { index, start: last_start, length: None };
let prior = self.prior.iter().cloned().map(move |length| {
let start = last_start - length;
let start = last_start.defensive_saturating_sub(length);
last_start = start;
index -= 1;
index.defensive_saturating_reduce(1);

SlashingSpan { index, start, length: Some(length) }
});
Expand All @@ -164,13 +164,18 @@ impl SlashingSpans {
let old_idx = self
.iter()
.skip(1) // skip ongoing span.
.position(|span| span.length.map_or(false, |len| span.start + len <= window_start));
.position(|span| {
span.length
.map_or(false, |len| span.start.defensive_saturating_add(len) <= window_start)
});

let earliest_span_index = self.span_index - self.prior.len() as SpanIndex;
let earliest_span_index =
self.span_index.defensive_saturating_sub(self.prior.len() as SpanIndex);
let pruned = match old_idx {
Some(o) => {
self.prior.truncate(o);
let new_earliest = self.span_index - self.prior.len() as SpanIndex;
let new_earliest =
self.span_index.defensive_saturating_sub(self.prior.len() as SpanIndex);
Some((earliest_span_index, new_earliest))
},
None => None,
Expand Down Expand Up @@ -500,7 +505,7 @@ impl<'a, T: 'a + Config> InspectingSpans<'a, T> {

let reward = if span_record.slashed < slash {
// new maximum span slash. apply the difference.
let difference = slash - span_record.slashed;
let difference = slash.defensive_saturating_sub(span_record.slashed);
span_record.slashed = slash;

// compute reward.
Expand Down

0 comments on commit 40984e3

Please sign in to comment.