Remove mixed type arithmetic Decimal * Uint128

[webmaster128]

There is no natural return type for Decimal * Uint128. It could be Decimal, Uint128 or even Uint512. See https://stackoverflow.com/a/44552464/2013738 for a more elaborate description of the problem.

The choice of Decimal * Uint128 = Uint128 has no reason other than its first use case. It has often confused users who expect Decimal * Uint128 = Decimal.

The better solution for the integer math is something like:

let a: Uint128;
let b: Decimal;

let c = a.multiply_ratio(b.numerator(), b.denominator());

The better solution for the decimal math is something like:

let a: Uint128;
let b: Decimal;

let c = Decimal::try_into(a)? * b;

[ethanfrey]

The better solution for the integer math is something like:
let c = a.multiply_ratio(b.numerator(), b.denomnator());

This is not a "better solution" in my mind. Very cumbersome to use and not adding clarity to devs.

If you want to make let c = a.multiply_ratio(b); that starts getting usable (if verbose).
People should not be forced to deconstruct Decimal to be able to do basic math

[webmaster128]

Yeah, this was not meant as the final API. It should just showcase how Decimal * Uint128 = Uint128 and Decimal * Uint128 = Decimal can be done and the implementation for both operation is there. It can be implemented with something like

fn multiply_by(self, rhs: impl Fraction) -> Uint128

instead to keep the caller code concise.

[0xForerunner]

Along these lines, it would be very useful to have some ceil and floor multiplication for Uints and Decimals. Something like this:

fn mul_ceiled(self, rhs: impl Fraction) -> Self;
fn mul_floored(self, rhs: impl Fraction) -> Self;

This would allow for a lot of flexibility when dealing with rounding errors etc.

[webmaster128]

@uint is 👆 something a macro implementation could help us with? I think we need those 4 functions implemented for Uint64, Uint128 and Uint256:

fn mul_ceiled(self, rhs: impl Fraction) -> Self;
fn mul_floored(self, rhs: impl Fraction) -> Self;
fn checked_mul_ceiled(self, rhs: impl Fraction) -> Result<Self, EnumSinceWeHaveTwoDifferentErrorCases>;
fn checked_mul_floored(self, rhs: impl Fraction) -> Result<Self, EnumSinceWeHaveTwoDifferentErrorCases>;

With those in place, we can confidentally remove the mixed type operator implementation in 2.0.

[uint]

@uint is point_up_2 something a macro implementation could help us with?

Probably! Like an impl_arithmetic!(u64) macro called in every Uint* module?

[webmaster128]

Yeah, or better specifically for the new additions impl_mul_arithmetic!(u64) such that we get them implemented consistently. We discussed deduplicating the Uint* implementations before and I think this is a good opportunity to try this approach. But I'm not that confident writing macros myself due to lack of experience.

Note that Uint512 should not get them since it does not have a full_mul implementation. If we add that we also need Uint1024 and the series would go on indefinitely.

[uint]

Isn't Uint128::checked_multiply_ratio already a floor multiplication? Ah, I see, we want to avoid deconstructing Decimal. Fair.

[uint]

@webmaster128 I took a shot at it yesterday. I think it's not as simple as reusing the logic in checked_multiply_ratio. E.g. if you have

let left: Uint64;
let right: Decimal; // or even Decimal256
let result = left.checked_mul_floored(right).unwrap();

I suspect the multiplication of left * right.numerator() might overflow quite easily, and full_mul doesn't help.

[webmaster128]

Right, this is something I vaguely imagined to become a problem.

So what we do currently support is basically

Uint64 * Fraction<u64>
Uint128 * Fraction<u128>
Uint256 * Fraction<Uint256>

with (more or less) Decimal being a Fraction<u128> and Decimal256 being a Fraction<Uint256>. Also by widening we can easily support

Uint128 * Fraction<u64>
Uint256 * Fraction<u64>
Uint256 * Fraction<u128>

So the number if bits in the integer needs to be >= the number of bits of the decimal. We are lacking an easy way to get

Uint64 * Fraction<u128> (i.e. Uint64 * Decimal)
Uint64 * Fraction<Uint256> (i.e. Uint64 * Decimal256)
Uint128 * Fraction<Uint256> (i.e. Uint128 * Decimal256)

However, if we do not do the multiplication size based on the integer size (current self.full_mul behaviour) but based on the larger of the two, we can get this to work. A simple implementation just takes 256*256=512 bit multiplication to cover all the cases at once.