From 328eb6bfeadb4c3e3b210901ab897f12a8ba4e1e Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 9 Apr 2021 11:05:48 +0200 Subject: [PATCH 1/2] clean arithmetic and unify names with the new api --- Cargo.lock | 1 + bin/node/runtime/src/impls.rs | 2 +- frame/contracts/proc-macro/src/lib.rs | 6 ++-- primitives/arithmetic/Cargo.toml | 1 + primitives/arithmetic/src/biguint.rs | 45 +++++++++++------------- primitives/arithmetic/src/fixed_point.rs | 2 +- 6 files changed, 27 insertions(+), 30 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 9e6e30cf2d00f..bfc66760757b9 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8570,6 +8570,7 @@ dependencies = [ "serde_json", "sp-debug-derive", "sp-std", + "static_assertions", ] [[package]] diff --git a/bin/node/runtime/src/impls.rs b/bin/node/runtime/src/impls.rs index 416266119cb09..ba8929b959209 100644 --- a/bin/node/runtime/src/impls.rs +++ b/bin/node/runtime/src/impls.rs @@ -74,7 +74,7 @@ mod multiplier_tests { let m = max_normal() as f64; // block weight always truncated to max weight let block_weight = (block_weight as f64).min(m); - let v: f64 = AdjustmentVariable::get().to_fraction(); + let v: f64 = AdjustmentVariable::get().to_float(); // Ideal saturation in terms of weight let ss = target() as f64; diff --git a/frame/contracts/proc-macro/src/lib.rs b/frame/contracts/proc-macro/src/lib.rs index 6fc2fbe82e037..3b8b1ea5e6636 100644 --- a/frame/contracts/proc-macro/src/lib.rs +++ b/frame/contracts/proc-macro/src/lib.rs @@ -117,17 +117,17 @@ fn format_weight(field: &Ident) -> TokenStream { &if self.#field > 1_000_000_000 { format!( "{:.1?} ms", - Fixed::saturating_from_rational(self.#field, 1_000_000_000).to_fraction() + Fixed::saturating_from_rational(self.#field, 1_000_000_000).to_float() ) } else if self.#field > 1_000_000 { format!( "{:.1?} µs", - Fixed::saturating_from_rational(self.#field, 1_000_000).to_fraction() + Fixed::saturating_from_rational(self.#field, 1_000_000).to_float() ) } else if self.#field > 1_000 { format!( "{:.1?} ns", - Fixed::saturating_from_rational(self.#field, 1_000).to_fraction() + Fixed::saturating_from_rational(self.#field, 1_000).to_float() ) } else { format!("{} ps", self.#field) diff --git a/primitives/arithmetic/Cargo.toml b/primitives/arithmetic/Cargo.toml index 76751cdee81bd..3c3b5a35c164a 100644 --- a/primitives/arithmetic/Cargo.toml +++ b/primitives/arithmetic/Cargo.toml @@ -17,6 +17,7 @@ targets = ["x86_64-unknown-linux-gnu"] [dependencies] codec = { package = "parity-scale-codec", version = "2.0.0", default-features = false, features = ["derive"] } integer-sqrt = "0.1.2" +static_assertions = "1.1.0" num-traits = { version = "0.2.8", default-features = false } sp-std = { version = "3.0.0", default-features = false, path = "../std" } serde = { version = "1.0.101", optional = true, features = ["derive"] } diff --git a/primitives/arithmetic/src/biguint.rs b/primitives/arithmetic/src/biguint.rs index 9813277506c40..e42f1998e5410 100644 --- a/primitives/arithmetic/src/biguint.rs +++ b/primitives/arithmetic/src/biguint.rs @@ -33,6 +33,10 @@ const SHIFT: usize = 32; /// short form of _Base_. Analogous to the value 10 in base-10 decimal numbers. const B: Double = Single::max_value() as Double + 1; +static_assertions::const_assert!( + sp_std::mem::size_of::() - sp_std::mem::size_of::() == SHIFT / 8 +); + /// Splits a [`Double`] limb number into a tuple of two [`Single`] limb numbers. pub fn split(a: Double) -> (Single, Single) { let al = a as Single; @@ -187,6 +191,7 @@ impl BigUint { let u = Double::from(self.checked_get(j).unwrap_or(0)); let v = Double::from(other.checked_get(j).unwrap_or(0)); let s = u + v + k; + // proof: any number % B will fit into `Single`. w.set(j, (s % B) as Single); k = s / B; } @@ -209,28 +214,23 @@ impl BigUint { let s = { let u = Double::from(self.checked_get(j).unwrap_or(0)); let v = Double::from(other.checked_get(j).unwrap_or(0)); - let mut needs_borrow = false; - let mut t = 0; + let mut t: Double = 0; - if let Some(v1) = u.checked_sub(v) { - if let Some(v2) = v1.checked_sub(k) { - t = v2; - k = 0; - } else { - needs_borrow = true; - } + if let Some(v2) = u.checked_sub(v).and_then(|v1| v1.checked_sub(k)) { + // no borrow is needed. u - v - k can be computed as-is + t = v2; + k = 0; } else { - needs_borrow = true; - } - if needs_borrow { + // borrow is needed. Add a `B` to u, before subtracting. + // PROOF: addition: `u + B < 2*B`, thus can fit in double. + // PROOF: subtraction: if `u - v - k < 0`, then `u + B - v - k < B`. + // NOTE: the order of operations is critical to ensure underflow won't happen. t = u + B - v - k; k = 1; } + t }; - // PROOF: t either comes from `v2`, or from `u + B - v - k`. The former is - // trivial. The latter will not overflow this branch will only happen if the sum of - // `u - v - k` part has been negative, hence `u + B - v - k < B`. w.set(j, s as Single); } @@ -264,10 +264,9 @@ impl BigUint { let mut k = 0; for i in 0..m { // PROOF: (B−1) × (B−1) + (B−1) + (B−1) = B^2 −1 < B^2. addition is safe. - let t = - mul_single(self.get(j), other.get(i)) - + Double::from(w.get(i + j)) - + Double::from(k); + let t = mul_single(self.get(j), other.get(i)) + + Double::from(w.get(i + j)) + + Double::from(k); w.set(i + j, (t % B) as Single); // PROOF: (B^2 - 1) / B < B. conversion is safe. k = (t / B) as Single; @@ -580,12 +579,6 @@ pub mod tests { BigUint { digits: vec![1; n] } } - #[test] - fn shift_check() { - let shift = sp_std::mem::size_of::() - sp_std::mem::size_of::(); - assert_eq!(shift * 8, SHIFT); - } - #[test] fn split_works() { let a = SHIFT / 2; @@ -732,12 +725,14 @@ pub mod tests { let c = BigUint { digits: vec![1, 1, 2] }; let d = BigUint { digits: vec![0, 2] }; let e = BigUint { digits: vec![0, 1, 1, 2] }; + let f = BigUint { digits: vec![7, 8] }; assert!(a.clone().div(&b, true).is_none()); assert!(c.clone().div(&a, true).is_none()); assert!(c.clone().div(&d, true).is_none()); assert!(e.clone().div(&a, true).is_none()); + assert!(f.clone().div(&b, true).is_none()); assert!(c.clone().div(&b, true).is_some()); } diff --git a/primitives/arithmetic/src/fixed_point.rs b/primitives/arithmetic/src/fixed_point.rs index b837c360c7c54..3dd8b9a1f7ad0 100644 --- a/primitives/arithmetic/src/fixed_point.rs +++ b/primitives/arithmetic/src/fixed_point.rs @@ -381,7 +381,7 @@ macro_rules! implement_fixed { } #[cfg(any(feature = "std", test))] - pub fn to_fraction(self) -> f64 { + pub fn to_float(self) -> f64 { self.0 as f64 / ::DIV as f64 } } From 027f907ce75e2596de6ab23c6e1c68d6471d6cca Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 9 Apr 2021 12:56:10 +0200 Subject: [PATCH 2/2] Fix warn --- primitives/arithmetic/src/biguint.rs | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/primitives/arithmetic/src/biguint.rs b/primitives/arithmetic/src/biguint.rs index e42f1998e5410..906c4d0cfd316 100644 --- a/primitives/arithmetic/src/biguint.rs +++ b/primitives/arithmetic/src/biguint.rs @@ -214,22 +214,23 @@ impl BigUint { let s = { let u = Double::from(self.checked_get(j).unwrap_or(0)); let v = Double::from(other.checked_get(j).unwrap_or(0)); - let mut t: Double = 0; if let Some(v2) = u.checked_sub(v).and_then(|v1| v1.checked_sub(k)) { // no borrow is needed. u - v - k can be computed as-is - t = v2; + let t = v2; k = 0; + + t } else { // borrow is needed. Add a `B` to u, before subtracting. // PROOF: addition: `u + B < 2*B`, thus can fit in double. // PROOF: subtraction: if `u - v - k < 0`, then `u + B - v - k < B`. // NOTE: the order of operations is critical to ensure underflow won't happen. - t = u + B - v - k; + let t = u + B - v - k; k = 1; - } - t + t + } }; w.set(j, s as Single); }