Skip to content
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

Fix multiplication in Uint64::multiply_ratio and Uint128::multiply_ratio #920

Closed
webmaster128 opened this issue May 25, 2021 · 0 comments · Fixed by #938
Closed

Fix multiplication in Uint64::multiply_ratio and Uint128::multiply_ratio #920

webmaster128 opened this issue May 25, 2021 · 0 comments · Fixed by #938
Milestone

Comments

@webmaster128
Copy link
Member

Right now we use an implementation like this:

        // TODO: minimize rounding that takes place (using gcd algorithm)
        let val = self.u64() * numerator / denominator;
        Uint64::from(val)

which is executed as (self.u64() * numerator) / denominator. This means that as soon as the multiplication overflows, the execution panics. This is an undesired behaviour because all of those:

base.multiply_ratio(1u64, 1u64);
base.multiply_ratio(3u64, 3u64);
base.multiply_ratio(u64::MAX, u64::MAX)

should return the same value.

In order to make this happen, we need to support a full multiplication that cannot overflow. For the Uint64 case this means multiplying two u64s into a u128 and for the Uint128 case this means multiplying two u128s into a 256 bit integer. The former can be done by casting to u128. The later is implemented e.g. in paritytech/parity-common#546. Maybe we can implement it manually though.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant