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

multiply_by_rational renamed to multiply_by_rational_with_rounding #11494

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all 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
4 changes: 2 additions & 2 deletions primitives/arithmetic/fuzzer/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,8 @@ name = "per_thing_rational"
path = "src/per_thing_rational.rs"

[[bin]]
name = "multiply_by_rational"
path = "src/multiply_by_rational.rs"
name = "multiply_by_rational_with_rounding"
path = "src/multiply_by_rational_with_rounding.rs"

[[bin]]
name = "fixed_point"
Expand Down
10 changes: 5 additions & 5 deletions primitives/arithmetic/fuzzer/src/multiply_by_rational.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,19 @@
// limitations under the License.

//! # Running
//! Running this fuzzer can be done with `cargo hfuzz run multiply_by_rational`. `honggfuzz` CLI
//! Running this fuzzer can be done with `cargo hfuzz run multiply_by_rational_with_rounding`. `honggfuzz` CLI
//! options can be used by setting `HFUZZ_RUN_ARGS`, such as `-n 4` to use 4 threads.
//!
//! # Debugging a panic
//! Once a panic is found, it can be debugged with
//! `cargo hfuzz run-debug multiply_by_rational hfuzz_workspace/multiply_by_rational/*.fuzz`.
//! `cargo hfuzz run-debug multiply_by_rational_with_rounding hfuzz_workspace/multiply_by_rational_with_rounding/*.fuzz`.
//!
//! # More information
//! More information about `honggfuzz` can be found
//! [here](https://docs.rs/honggfuzz/).

use honggfuzz::fuzz;
use sp_arithmetic::{helpers_128bit::multiply_by_rational, traits::Zero};
use sp_arithmetic::{helpers_128bit::multiply_by_rational_with_rounding, traits::Zero};

fn main() {
loop {
Expand All @@ -42,9 +42,9 @@ fn main() {

println!("++ Equation: {} * {} / {}", a, b, c);

// The point of this fuzzing is to make sure that `multiply_by_rational` is 100%
// The point of this fuzzing is to make sure that `multiply_by_rational_with_rounding` is 100%
// accurate as long as the value fits in a u128.
if let Ok(result) = multiply_by_rational(a, b, c) {
if let Ok(result) = multiply_by_rational_with_rounding(a, b, c) {
let truth = mul_div(a, b, c);

if result != truth && result != truth + 1 {
Expand Down
10 changes: 5 additions & 5 deletions primitives/arithmetic/src/fixed_point.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
//! Decimal Fixed Point implementations for Substrate runtime.

use crate::{
helpers_128bit::multiply_by_rational,
helpers_128bit::multiply_by_rational_with_rounding,
traits::{
Bounded, CheckedAdd, CheckedDiv, CheckedMul, CheckedNeg, CheckedSub, One,
SaturatedConversion, Saturating, UniqueSaturatedInto, Zero,
Expand Down Expand Up @@ -151,7 +151,7 @@ pub trait FixedPointNumber:
let d: I129 = d.into();
let negative = n.negative != d.negative;

multiply_by_rational(n.value, Self::DIV.unique_saturated_into(), d.value)
multiply_by_rational_with_rounding(n.value, Self::DIV.unique_saturated_into(), d.value)
.ok()
.and_then(|value| from_i129(I129 { value, negative }))
.map(Self::from_inner)
Expand All @@ -165,7 +165,7 @@ pub trait FixedPointNumber:
let rhs: I129 = n.into();
let negative = lhs.negative != rhs.negative;

multiply_by_rational(lhs.value, rhs.value, Self::DIV.unique_saturated_into())
multiply_by_rational_with_rounding(lhs.value, rhs.value, Self::DIV.unique_saturated_into())
.ok()
.and_then(|value| from_i129(I129 { value, negative }))
}
Expand Down Expand Up @@ -522,7 +522,7 @@ macro_rules! implement_fixed {
let rhs: I129 = other.0.into();
let negative = lhs.negative != rhs.negative;

multiply_by_rational(lhs.value, Self::DIV as u128, rhs.value)
multiply_by_rational_with_rounding(lhs.value, Self::DIV as u128, rhs.value)
.ok()
.and_then(|value| from_i129(I129 { value, negative }))
.map(Self)
Expand All @@ -535,7 +535,7 @@ macro_rules! implement_fixed {
let rhs: I129 = other.0.into();
let negative = lhs.negative != rhs.negative;

multiply_by_rational(lhs.value, rhs.value, Self::DIV as u128)
multiply_by_rational_with_rounding(lhs.value, rhs.value, Self::DIV as u128)
.ok()
.and_then(|value| from_i129(I129 { value, negative }))
.map(Self)
Expand Down
2 changes: 1 addition & 1 deletion primitives/arithmetic/src/helpers_128bit.rs
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ pub fn to_big_uint(x: u128) -> biguint::BigUint {
/// be safely casted back to u128.
///
/// Invariant: c must be greater than or equal to 1.
pub fn multiply_by_rational(mut a: u128, mut b: u128, mut c: u128) -> Result<u128, &'static str> {
pub fn multiply_by_rational_with_rounding(mut a: u128, mut b: u128, mut c: u128) -> Result<u128, &'static str> {
if a.is_zero() || b.is_zero() {
return Ok(Zero::zero())
}
Expand Down
44 changes: 22 additions & 22 deletions primitives/arithmetic/src/rational.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,7 @@ impl Rational128 {
if den == self.1 {
Ok(self)
} else {
helpers_128bit::multiply_by_rational(self.0, den, self.1).map(|n| Self(n, den))
helpers_128bit::multiply_by_rational_with_rounding(self.0, den, self.1).map(|n| Self(n, den))
}
}

Expand All @@ -163,7 +163,7 @@ impl Rational128 {
return Ok(self.1)
}
let g = helpers_128bit::gcd(self.1, other.1);
helpers_128bit::multiply_by_rational(self.1, other.1, g)
helpers_128bit::multiply_by_rational_with_rounding(self.1, other.1, g)
}

/// A saturating add that assumes `self` and `other` have the same denominator.
Expand Down Expand Up @@ -408,52 +408,52 @@ mod tests {
}

#[test]
fn multiply_by_rational_works() {
assert_eq!(multiply_by_rational(7, 2, 3).unwrap(), 7 * 2 / 3);
assert_eq!(multiply_by_rational(7, 20, 30).unwrap(), 7 * 2 / 3);
assert_eq!(multiply_by_rational(20, 7, 30).unwrap(), 7 * 2 / 3);
fn multiply_by_rational_with_rounding_works() {
assert_eq!(multiply_by_rational_with_rounding(7, 2, 3).unwrap(), 7 * 2 / 3);
assert_eq!(multiply_by_rational_with_rounding(7, 20, 30).unwrap(), 7 * 2 / 3);
assert_eq!(multiply_by_rational_with_rounding(20, 7, 30).unwrap(), 7 * 2 / 3);

assert_eq!(
// MAX128 % 3 == 0
multiply_by_rational(MAX128, 2, 3).unwrap(),
multiply_by_rational_with_rounding(MAX128, 2, 3).unwrap(),
MAX128 / 3 * 2,
);
assert_eq!(
// MAX128 % 7 == 3
multiply_by_rational(MAX128, 5, 7).unwrap(),
multiply_by_rational_with_rounding(MAX128, 5, 7).unwrap(),
(MAX128 / 7 * 5) + (3 * 5 / 7),
);
assert_eq!(
// MAX128 % 7 == 3
multiply_by_rational(MAX128, 11, 13).unwrap(),
multiply_by_rational_with_rounding(MAX128, 11, 13).unwrap(),
(MAX128 / 13 * 11) + (8 * 11 / 13),
);
assert_eq!(
// MAX128 % 1000 == 455
multiply_by_rational(MAX128, 555, 1000).unwrap(),
multiply_by_rational_with_rounding(MAX128, 555, 1000).unwrap(),
(MAX128 / 1000 * 555) + (455 * 555 / 1000),
);

assert_eq!(multiply_by_rational(2 * MAX64 - 1, MAX64, MAX64).unwrap(), 2 * MAX64 - 1);
assert_eq!(multiply_by_rational(2 * MAX64 - 1, MAX64 - 1, MAX64).unwrap(), 2 * MAX64 - 3);
assert_eq!(multiply_by_rational_with_rounding(2 * MAX64 - 1, MAX64, MAX64).unwrap(), 2 * MAX64 - 1);
assert_eq!(multiply_by_rational_with_rounding(2 * MAX64 - 1, MAX64 - 1, MAX64).unwrap(), 2 * MAX64 - 3);

assert_eq!(
multiply_by_rational(MAX64 + 100, MAX64_2, MAX64_2 / 2).unwrap(),
multiply_by_rational_with_rounding(MAX64 + 100, MAX64_2, MAX64_2 / 2).unwrap(),
(MAX64 + 100) * 2,
);
assert_eq!(
multiply_by_rational(MAX64 + 100, MAX64_2 / 100, MAX64_2 / 200).unwrap(),
multiply_by_rational_with_rounding(MAX64 + 100, MAX64_2 / 100, MAX64_2 / 200).unwrap(),
(MAX64 + 100) * 2,
);

assert_eq!(
multiply_by_rational(2u128.pow(66) - 1, 2u128.pow(65) - 1, 2u128.pow(65)).unwrap(),
multiply_by_rational_with_rounding(2u128.pow(66) - 1, 2u128.pow(65) - 1, 2u128.pow(65)).unwrap(),
73786976294838206461,
);
assert_eq!(multiply_by_rational(1_000_000_000, MAX128 / 8, MAX128 / 2).unwrap(), 250000000);
assert_eq!(multiply_by_rational_with_rounding(1_000_000_000, MAX128 / 8, MAX128 / 2).unwrap(), 250000000);

assert_eq!(
multiply_by_rational(
multiply_by_rational_with_rounding(
29459999999999999988000u128,
1000000000000000000u128,
10000000000000000000u128
Expand All @@ -464,16 +464,16 @@ mod tests {
}

#[test]
fn multiply_by_rational_a_b_are_interchangeable() {
assert_eq!(multiply_by_rational(10, MAX128, MAX128 / 2), Ok(20));
assert_eq!(multiply_by_rational(MAX128, 10, MAX128 / 2), Ok(20));
fn multiply_by_rational_with_rounding_a_b_are_interchangeable() {
assert_eq!(multiply_by_rational_with_rounding(10, MAX128, MAX128 / 2), Ok(20));
assert_eq!(multiply_by_rational_with_rounding(MAX128, 10, MAX128 / 2), Ok(20));
}

#[test]
#[ignore]
fn multiply_by_rational_fuzzed_equation() {
fn multiply_by_rational_with_rounding_fuzzed_equation() {
assert_eq!(
multiply_by_rational(154742576605164960401588224, 9223376310179529214, 549756068598),
multiply_by_rational_with_rounding(154742576605164960401588224, 9223376310179529214, 549756068598),
Ok(2596149632101417846585204209223679)
);
}
Expand Down
6 changes: 3 additions & 3 deletions primitives/npos-elections/src/phragmen.rs
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ use crate::{
PerThing128, VoteWeight, Voter,
};
use sp_arithmetic::{
helpers_128bit::multiply_by_rational,
helpers_128bit::multiply_by_rational_with_rounding,
traits::{Bounded, Zero},
Rational128,
};
Expand Down Expand Up @@ -143,7 +143,7 @@ pub fn seq_phragmen_core<AccountId: IdentifierT>(
for edge in &voter.edges {
let mut candidate = edge.candidate.borrow_mut();
if !candidate.elected && !candidate.approval_stake.is_zero() {
let temp_n = multiply_by_rational(
let temp_n = multiply_by_rational_with_rounding(
voter.load.n(),
voter.budget,
candidate.approval_stake,
Expand Down Expand Up @@ -184,7 +184,7 @@ pub fn seq_phragmen_core<AccountId: IdentifierT>(
for edge in &mut voter.edges {
if edge.candidate.borrow().elected {
// update internal state.
edge.weight = multiply_by_rational(voter.budget, edge.load.n(), voter.load.n())
edge.weight = multiply_by_rational_with_rounding(voter.budget, edge.load.n(), voter.load.n())
// If result cannot fit in u128. Not much we can do about it.
.unwrap_or(Bounded::max_value());
} else {
Expand Down
2 changes: 1 addition & 1 deletion primitives/staking/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ pub trait StakingInterface {
///
/// # Note
///
/// This must be strictly greater than the staking systems slash deffer duration.
/// This must be strictly greater than the staking systems slash defer duration.
fn bonding_duration() -> EraIndex;

/// The current era index.
Expand Down