Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Fix PerThing::from_percent. #7701

Merged
merged 4 commits into from
Dec 11, 2020
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 14 additions & 0 deletions primitives/arithmetic/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -515,3 +515,17 @@ mod threshold_compare_tests {
assert_eq!(Saturating::saturating_pow(i32::max_value(), 2), i32::max_value());
}
}

#[cfg(test)]
mod per_thing {
use super::*;

#[test]
fn per_thing_100_wont_overflow() {
assert_eq!(<Percent as PerThing>::from_percent(100), Percent::one());
kianenigma marked this conversation as resolved.
Show resolved Hide resolved
assert_eq!(<PerU16 as PerThing>::from_percent(100), PerU16::one());
assert_eq!(<Permill as PerThing>::from_percent(100), Permill::one());
assert_eq!(<Perbill as PerThing>::from_percent(100), Perbill::one());
assert_eq!(<Perquintill as PerThing>::from_percent(100), Perquintill::one());
}
}
5 changes: 3 additions & 2 deletions primitives/arithmetic/src/per_things.rs
Original file line number Diff line number Diff line change
Expand Up @@ -67,7 +67,8 @@ pub trait PerThing:
let b = Self::ACCURACY;
// if Self::ACCURACY % 100 > 0 then we need the correction for accuracy
let c = rational_mul_correction::<Self::Inner, Self>(b, a, 100.into(), Rounding::Nearest);
Self::from_parts(a / 100.into() * b + c)
let base: Self::Inner = 100.into();
Self::from_parts((a / base).saturating_mul(b).saturating_add(c))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fuck, I'm too dumb xD How does this work?

When a is 50u32. And I do a / base, I will get 0?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yes you are right, this function doesn't work, and should have tests generated by macro

Copy link
Contributor Author

@kianenigma kianenigma Dec 10, 2020

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wasn't sure how this function works either tbh :D seem to have been broken

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Addressed, I just used the well tested from_rational.

}

/// Return the product of multiplication of this value by itself.
Expand Down Expand Up @@ -334,7 +335,7 @@ macro_rules! implement_per_thing {
&self.0
}
fn decode_from(x: Self::As) -> Self {
// Saturates if `x` is more than `$max` internally.
// Saturates if `x` is more than `$max` internally.
Self::from_parts(x)
}
}
Expand Down