From a0cffc7b93f74d12a10b4726fc3ecd500e1a182e Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 9 Dec 2020 14:19:25 +0000 Subject: [PATCH 1/3] Fix overflow in per_things::from_percent. --- primitives/arithmetic/src/lib.rs | 14 ++++++++++++++ primitives/arithmetic/src/per_things.rs | 5 +++-- 2 files changed, 17 insertions(+), 2 deletions(-) diff --git a/primitives/arithmetic/src/lib.rs b/primitives/arithmetic/src/lib.rs index f6521988c91a5..4bda48d144864 100644 --- a/primitives/arithmetic/src/lib.rs +++ b/primitives/arithmetic/src/lib.rs @@ -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() { + ::from_percent(100); + ::from_percent(100); + ::from_percent(100); + ::from_percent(100); + ::from_percent(100); + } +} diff --git a/primitives/arithmetic/src/per_things.rs b/primitives/arithmetic/src/per_things.rs index 035a704ba3009..2ea8651e12404 100644 --- a/primitives/arithmetic/src/per_things.rs +++ b/primitives/arithmetic/src/per_things.rs @@ -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::(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)) } /// Return the product of multiplication of this value by itself. @@ -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) } } From 549aa6e0156335df1ae9d305b39c1b75a81af409 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Wed, 9 Dec 2020 17:15:31 +0000 Subject: [PATCH 2/3] Fix test --- primitives/arithmetic/src/lib.rs | 10 +++++----- primitives/arithmetic/src/per_things.rs | 2 +- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/primitives/arithmetic/src/lib.rs b/primitives/arithmetic/src/lib.rs index 4bda48d144864..dec7325544d44 100644 --- a/primitives/arithmetic/src/lib.rs +++ b/primitives/arithmetic/src/lib.rs @@ -522,10 +522,10 @@ mod per_thing { #[test] fn per_thing_100_wont_overflow() { - ::from_percent(100); - ::from_percent(100); - ::from_percent(100); - ::from_percent(100); - ::from_percent(100); + assert_eq!(::from_percent(100), Percent::one()); + assert_eq!(::from_percent(100), PerU16::one()); + assert_eq!(::from_percent(100), Permill::one()); + assert_eq!(::from_percent(100), Perbill::one()); + assert_eq!(::from_percent(100), Perquintill::one()); } } diff --git a/primitives/arithmetic/src/per_things.rs b/primitives/arithmetic/src/per_things.rs index 2ea8651e12404..5071440675c3a 100644 --- a/primitives/arithmetic/src/per_things.rs +++ b/primitives/arithmetic/src/per_things.rs @@ -68,7 +68,7 @@ pub trait PerThing: // if Self::ACCURACY % 100 > 0 then we need the correction for accuracy let c = rational_mul_correction::(b, a, 100.into(), Rounding::Nearest); let base: Self::Inner = 100.into(); - Self::from_parts(a / base.saturating_mul(b).saturating_add(c)) + Self::from_parts((a / base).saturating_mul(b).saturating_add(c)) } /// Return the product of multiplication of this value by itself. From d8359e67081e1003b3505fb4352100f173aa6af2 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Thu, 10 Dec 2020 14:35:57 +0000 Subject: [PATCH 3/3] Fix the whole thing.. :| --- primitives/arithmetic/src/lib.rs | 14 -------------- primitives/arithmetic/src/per_things.rs | 21 ++++++++++++++------- 2 files changed, 14 insertions(+), 21 deletions(-) diff --git a/primitives/arithmetic/src/lib.rs b/primitives/arithmetic/src/lib.rs index dec7325544d44..f6521988c91a5 100644 --- a/primitives/arithmetic/src/lib.rs +++ b/primitives/arithmetic/src/lib.rs @@ -515,17 +515,3 @@ 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!(::from_percent(100), Percent::one()); - assert_eq!(::from_percent(100), PerU16::one()); - assert_eq!(::from_percent(100), Permill::one()); - assert_eq!(::from_percent(100), Perbill::one()); - assert_eq!(::from_percent(100), Perquintill::one()); - } -} diff --git a/primitives/arithmetic/src/per_things.rs b/primitives/arithmetic/src/per_things.rs index 5071440675c3a..59d98eea2b780 100644 --- a/primitives/arithmetic/src/per_things.rs +++ b/primitives/arithmetic/src/per_things.rs @@ -61,14 +61,11 @@ pub trait PerThing: fn is_one(&self) -> bool { self.deconstruct() == Self::ACCURACY } /// Build this type from a percent. Equivalent to `Self::from_parts(x * Self::ACCURACY / 100)` - /// but more accurate. + /// but more accurate and can cope with potential type overflows. fn from_percent(x: Self::Inner) -> Self { - let a = x.min(100.into()); - let b = Self::ACCURACY; - // if Self::ACCURACY % 100 > 0 then we need the correction for accuracy - let c = rational_mul_correction::(b, a, 100.into(), Rounding::Nearest); - let base: Self::Inner = 100.into(); - Self::from_parts((a / base).saturating_mul(b).saturating_add(c)) + let a: Self::Inner = x.min(100.into()); + let b: Self::Inner = 100.into(); + Self::from_rational_approximation(a, b) } /// Return the product of multiplication of this value by itself. @@ -708,6 +705,7 @@ macro_rules! implement_per_thing { assert_eq!($name::from_percent(0), $name::from_parts(Zero::zero())); assert_eq!($name::from_percent(10), $name::from_parts($max / 10)); + assert_eq!($name::from_percent(50), $name::from_parts($max / 2)); assert_eq!($name::from_percent(100), $name::from_parts($max)); assert_eq!($name::from_percent(200), $name::from_parts($max)); @@ -718,6 +716,15 @@ macro_rules! implement_per_thing { assert_eq!($name::from_fraction(-1.0), $name::from_parts(Zero::zero())); } + #[test] + fn percent_trait_impl_works() { + assert_eq!(<$name as PerThing>::from_percent(0), $name::from_parts(Zero::zero())); + assert_eq!(<$name as PerThing>::from_percent(10), $name::from_parts($max / 10)); + assert_eq!(<$name as PerThing>::from_percent(50), $name::from_parts($max / 2)); + assert_eq!(<$name as PerThing>::from_percent(100), $name::from_parts($max)); + assert_eq!(<$name as PerThing>::from_percent(200), $name::from_parts($max)); + } + macro_rules! u256ify { ($val:expr) => { Into::::into($val)