Attempt to remove the where bounds in arithmetic.

[kianenigma]

Needed for #7929

So far, these where clauses have been haunting me everywhere up to the runtime. The reason for their existence is the use of <P: PerThing>, which means that we don't initially know what is the type P::Inner and if it meets all the requirements of other functions (like from_rational_approximation).

So far, I have let this be, we need generic P in some places.

During #7929, I was quite annoyed by this as the where clause literally propagates to almost every single file and function of staking, if you are to make OffchainAccuracy generic.

Note that we already tolerate and accept this in #7909 as well, but there it is cleaner because everything is wrapper in impl Pallet.

So I tried to just remove this where clause from the source, which is this PR. In essence, the only main assumption added is that type Inner is always Into<u128>. This can easily hold as long as we keep our per-things within the standard primitive types (note: unsigned types are already not allowed, so you have u8 -> u128 left). But this would disallow using per-things with higher accuracy levels such as U256 etc.

I had to shuffle a bit of From<U> for T into Into<T> for U, which to this moment still confuses me, but seemingly it was necessary since rustc does not assume it automatically. At first, I really expected just adding type Inner: Into<u128> but it is not.

[kianenigma]

TLDR ^^^^^^^^^^^^^^^^^^^^^^

[gui1117]

I had to shuffle a bit of From for T into Into for U, which to this moment still confuses me, but seemingly it was necessary since rustc does not assume it automatically.

I think this is because: although From<T> for U implies Into<U> for T the opposite is not true. Into doesn't implies From

[gui1117]

TLDR: we could change from_rational_approximation to bound N: TryFrom<Self::Inner> instead of N: From<Self::Inner>, but it would make code even more tricky for probably no benefit. So I'm fine with the change you proposed.

All this is because from_rational_approximation requires N to be From<Self::Inner>
I required it because we usually used the function with N being more than Inner arithmetic.
But now that PerThing::Inner is generic in trait PerThing we need to add these where_clause everywhere even when N is u128.

But actually if N arithmetic fit into Self::Inner (i.e. N is u16 and Self::Inner is u32) then implementing from_rational_approximation is doable:

diff --git a/primitives/arithmetic/src/per_things.rs b/primitives/arithmetic/src/per_things.rs
index c6a31a0ff..9304ff59f 100644
--- a/primitives/arithmetic/src/per_things.rs
+++ b/primitives/arithmetic/src/per_things.rs
@@ -362,7 +362,7 @@ macro_rules! implement_per_thing {
                        }
 
                        fn from_rational_approximation<N>(p: N, q: N) -> Self
-                       where N: Clone + Ord + From<Self::Inner> + TryInto<Self::Inner> + TryInto<Self::Upper>
+                       where N: Clone + Ord + TryFrom<Self::Inner> + TryInto<Self::Inner> + TryInto<Self::Upper>
                                + ops::Div<N, Output=N> + ops::Rem<N, Output=N> + ops::Add<N, Output=N> + Unsigned
                        {
                                let div_ceil = |x: N, f: N| -> N {
@@ -379,7 +379,23 @@ macro_rules! implement_per_thing {
                                // p should not be bigger than q.
                                let p: N = p.min(q.clone());
 
-                               let factor: N = div_ceil(q.clone(), $max.into()).max((1 as Self::Inner).into());
+                               let acc: N = if let Ok(acc) = $max.try_into() {
+                                       acc
+                               } else {
+                                       // $max doesn't fit into `N` arithmetic,
+                                       // thus $max is strictly more than `N::max_value()`,
+                                       // thus `N::max_value()` is strictly less than $max,
+                                       // but p and q are less than `N::max_value()`,
+                                       // thus p and q are less than $max,
+                                       // thus p * $max fits into `Upper` (Upper must be able to compute `$max^2`)
+                                       // and q fits into `Inner`
+                                       let part =
+                                               Self::Inner::try_from(p).expect("..") as $upper_type
+                                               * $max as $upper_type
+                                               / q as $upper_type;
+                                       return $name(part as Self::Inner)
+                               };
+                               let factor: N = div_ceil(q.clone(), acc).max((1 as Self::Inner).into());
 
                                // q cannot overflow: (q / (q/$max)) < $max. p < q hence p also cannot overflow.
                                let q_reduce: $type = (q.clone() / factor.clone())

EDIT: thinking of it again it might be wrong. Because then it allows ppl to use stuff like NonZeroU128 for N maybe, and this is not good.

[shawntabrizi]

this formatting cant be right

[kianenigma]

bot merge

[ghost]

Waiting for commit status.

[ghost]

Head SHA changed; merge aborted.

[kianenigma]

bot merge

[ghost]

Waiting for commit status.