-
Notifications
You must be signed in to change notification settings - Fork 744
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
introduce checked_div
for PerThings
and checked_rational_mul_correction
#1936
base: master
Are you sure you want to change the base?
Conversation
The CI pipeline was cancelled due to failure one of the required jobs. |
6e9ad78
to
383633c
Compare
383633c
to
49dd9a7
Compare
fc5e132
to
1bd75e2
Compare
if P::Inner::is_zero(&denom) { | ||
return Err("Division by zero") | ||
} | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In line 529
maybe there is a checked_rem
? Then we dont need this if.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
not sure what you mean by checked_rem
pub const fn multiply_by_rational_with_rounding( | ||
a: u128, | ||
b: u128, | ||
c: u128, | ||
r: Rounding, | ||
) -> Option<u128> { | ||
match checked_multiply_by_rational_with_rounding(a, b, c, r) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
match checked_multiply_by_rational_with_rounding(a, b, c, r) { | |
checked_multiply_by_rational_with_rounding(a, b, c, r).ok() |
impl CheckedDiv for $name { | ||
#[inline] | ||
fn checked_div(&self, rhs: &Self) -> Option<Self> { | ||
if rhs.0.is_zero() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should use from_rational_with_rounding
bot fmt |
@bkchr https://gitlab.parity.io/parity/mirrors/polkadot-sdk/-/jobs/6806184 was started for your command Comment |
We are migrating the command bot to be a GitHub Action |
@Gioyik can you take another look? |
@bkchr Command |
Fixes #199
Follow up from paritytech/substrate#14673
Introduces
checked_div
andchecked_rational_mul_correction