Implement FixedPoint trait. (#5877)

* Implement Fixed trait.

* Fix tests

* Fix tests

* Fix tests 2

* Address review comment regarding from_i129.

* Remove precision by using log10() as suggested in review.

* Add small comments.

* Use checked versions + panic for ops::*.

* Remove repeated test.

* Uncomment test.

* Remove casts.

* Add more comments.

* Add tests.

* Panic on saturating_div_int

* More tests.

* More docs.

* Saturating renames.

* Fix to_bound doc.

* Move some impl to trait.

* Add range

* Add macro pre.

* More round() tests.

* Delete confusion.

* More impl to trait

* Add doc for fixedpoint op.

* Remove trailing spaces.

* Suggested docs changes.

* More tests and comments for roundings.

* Some quickcheck tests.

* Add missing panic, more test/comments.

* Nits.

* Rename.

* Remove primitives-types import.

* Apply review suggestions

* Fix long lines and add some fuzz.

* fix long line

* Update fuzzer

* Bump impl

* fix warnings

Co-authored-by: Gavin Wood <gavin@parity.io>
Co-authored-by: Shawn Tabrizi <shawntabrizi@gmail.com>
Commit:
c5280a6 [c5280a6]
Parents:
d047cb3
Author:
Marcio Diaz <marcio.diaz@gmail.com>
Date:
22 May 2020 at 5:32:44 AM NZST
Committer:
AlexSedNZ <alex.sedighi@centrality.ai>
Commit Date:
29 September 2020 at 2:12:51 PM NZDT
Labels:
origin/fix/141-weight-multiplier
GPG Details:
[GOOD] Alex Sedighi

bin/node/executor/tests/basic.rs

@@ -24,7 +24,7 @@ use sp_core::{
 	NeverNativeValue, map, traits::Externalities, storage::{well_known_keys, Storage},
 };
 use sp_runtime::{
-	ApplyExtrinsicResult, Fixed64,
+	ApplyExtrinsicResult, Fixed128, FixedPointNumber,
 	traits::{Hash as HashT, Convert, BlakeTwo256},
 	transaction_validity::InvalidTransaction,
 };
@@ -59,9 +59,9 @@ fn transfer_fee<E: Encode>(extrinsic: &E, fee_multiplier: Fixed64) -> Balance {
 	let weight = default_transfer_call().get_dispatch_info().weight;
 	let weight_fee = <Runtime as pallet_transaction_payment::Trait>
 		::WeightToFee::convert(weight);
-
+	
 	let base_fee = TransactionBaseFee::get();
-	base_fee + fee_multiplier.saturated_multiply_accumulate(length_fee + weight_fee)
+	base_fee + fee_multiplier.saturating_mul_acc_int(length_fee + weight_fee)
 }
 
 fn xt() -> UncheckedExtrinsic {

bin/node/executor/tests/fees.rs

@@ -21,7 +21,7 @@ use frame_support::{
 	weights::GetDispatchInfo,
 };
 use sp_core::{NeverNativeValue, map, storage::Storage};
-use sp_runtime::{Fixed64, Perbill, traits::{Convert, BlakeTwo256}};
+use sp_runtime::{FixedPointNumber, Fixed128, Perbill, traits::{Convert, BlakeTwo256}};
 use node_runtime::{
 	CheckedExtrinsic, Call, Runtime, Balances, TransactionPayment, TransactionBaseFee,
 	TransactionByteFee, WeightFeeCoefficient,
@@ -39,7 +39,7 @@ fn fee_multiplier_increases_and_decreases_on_big_weight() {
 	let mut t = new_test_ext(COMPACT_CODE, false);
 
 	// initial fee multiplier must be zero
-	let mut prev_multiplier = Fixed64::from_parts(0);
+	let mut prev_multiplier = Fixed128::from_inner(0);
 
 	t.execute_with(|| {
 		assert_eq!(TransactionPayment::next_fee_multiplier(), prev_multiplier);

bin/node/runtime/src/impls.rs

@@ -18,7 +18,7 @@
 
 use node_primitives::Balance;
 use sp_runtime::traits::{Convert, Saturating};
-use sp_runtime::{Fixed64, Perbill};
+use sp_runtime::{FixedPointNumber, Fixed128, Perbill};
 use frame_support::{traits::{OnUnbalanced, Currency, Get}, weights::Weight};
 use crate::{Balances, System, Authorship, MaximumBlockWeight, NegativeImbalance};
 
@@ -68,8 +68,8 @@ impl<C: Get<Balance>> Convert<Weight, Balance> for LinearWeightToFee<C> {
 /// https://research.web3.foundation/en/latest/polkadot/Token%20Economics/#relay-chain-transaction-fees
 pub struct TargetedFeeAdjustment<T>(sp_std::marker::PhantomData<T>);
 
-impl<T: Get<Perbill>> Convert<Fixed64, Fixed64> for TargetedFeeAdjustment<T> {
-	fn convert(multiplier: Fixed64) -> Fixed64 {
+impl<T: Get<Perbill>> Convert<Fixed128, Fixed128> for TargetedFeeAdjustment<T> {
+	fn convert(multiplier: Fixed128) -> Fixed128 {
 		let block_weight = System::all_extrinsics_weight();
 		let max_weight = MaximumBlockWeight::get();
 		let target_weight = (T::get() * max_weight) as u128;
@@ -78,15 +78,14 @@ impl<T: Get<Perbill>> Convert<Fixed64, Fixed64> for TargetedFeeAdjustment<T> {
 		// determines if the first_term is positive
 		let positive = block_weight >= target_weight;
 		let diff_abs = block_weight.max(target_weight) - block_weight.min(target_weight);
-		// diff is within u32, safe.
-		let diff = Fixed64::from_rational(diff_abs as i64, max_weight as u64);
+		// safe, diff_abs cannot exceed u64.
+		let diff = Fixed128::saturating_from_rational(diff_abs, max_weight.max(1));
 		let diff_squared = diff.saturating_mul(diff);
 
 		// 0.00004 = 4/100_000 = 40_000/10^9
-		let v = Fixed64::from_rational(4, 100_000);
-		// 0.00004^2 = 16/10^10 ~= 2/10^9. Taking the future /2 into account, then it is just 1
-		// parts from a billionth.
-		let v_squared_2 = Fixed64::from_rational(1, 1_000_000_000);
+		let v = Fixed128::saturating_from_rational(4, 100_000);
+		// 0.00004^2 = 16/10^10 Taking the future /2 into account... 8/10^10
+		let v_squared_2 = Fixed128::saturating_from_rational(8, 10_000_000_000u64);
 
 		let first_term = v.saturating_mul(diff);
 		// It is very unlikely that this will exist (in our poor perbill estimate) but we are giving
@@ -107,7 +106,7 @@ impl<T: Get<Perbill>> Convert<Fixed64, Fixed64> for TargetedFeeAdjustment<T> {
 				// multiplier. While at -1, it means that the network is so un-congested that all
 				// transactions have no weight fee. We stop here and only increase if the network
 				// became more busy.
-				.max(Fixed64::from_rational(-1, 1))
+				.max(Fixed128::saturating_from_integer(-1))
 		}
 	}
 }
@@ -118,7 +117,7 @@ mod tests {
 	use sp_runtime::assert_eq_error_rate;
 	use crate::{MaximumBlockWeight, AvailableBlockRatio, Runtime};
 	use crate::{constants::currency::*, TransactionPayment, TargetBlockFullness};
-	use frame_support::weights::Weight;
+	use frame_support::weights::{Weight, WeightToFeePolynomial};
 
 	fn max() -> Weight {
 		MaximumBlockWeight::get()
@@ -141,7 +140,7 @@ mod tests {
 		let s = block_weight;
 
 		let fm = v * (s/m - ss/m) + v.powi(2) * (s/m - ss/m).powi(2) / 2.0;
-		let addition_fm = Fixed64::from_parts((fm * 1_000_000_000_f32).round() as i64);
+		let addition_fm = Fixed128::from_inner((fm * Fixed128::accuracy() as f64).round() as i128);
 		previous.saturating_add(addition_fm)
 	}
 
@@ -160,7 +159,7 @@ mod tests {
 
 	#[test]
 	fn fee_multiplier_update_poc_works() {
-		let fm = Fixed64::from_rational(0, 1);
+		let fm = Fixed128::saturating_from_rational(0, 1);
 		let test_set = vec![
 			(0, fm.clone()),
 			(100, fm.clone()),
@@ -171,9 +170,10 @@ mod tests {
 		test_set.into_iter().for_each(|(w, fm)| {
 			run_with_system_weight(w, || {
 				assert_eq_error_rate!(
-					fee_multiplier_update(w, fm).into_inner(),
-					TargetedFeeAdjustment::<TargetBlockFullness>::convert(fm).into_inner(),
-					5,
+					fee_multiplier_update(w, fm),
+					TargetedFeeAdjustment::<TargetBlockFullness>::convert(fm),
+					// Error is only 1 in 10^18
+					Fixed128::from_inner(1),
 				);
 			})
 		})
@@ -189,7 +189,7 @@ mod tests {
 			loop {
 				let next = TargetedFeeAdjustment::<TargetBlockFullness>::convert(fm);
 				fm = next;
-				if fm == Fixed64::from_rational(-1, 1) { break; }
+				if fm == Fixed128::saturating_from_integer(-1) { break; }
 				iterations += 1;
 			}
 			println!("iteration {}, new fm = {:?}. Weight fee is now zero", iterations, fm);
@@ -228,7 +228,7 @@ mod tests {
 				fm = next;
 				iterations += 1;
 				let fee = <Runtime as pallet_transaction_payment::Trait>::WeightToFee::convert(tx_weight);
-				let adjusted_fee = fm.saturated_multiply_accumulate(fee);
+				let adjusted_fee = fm.saturating_mul_acc_int(fee);
 				println!(
 					"iteration {}, new fm = {:?}. Fee at this point is: {} units / {} millicents, \
 					{} cents, {} dollars",
@@ -316,8 +316,8 @@ mod tests {
 			);
 			// ...
 			assert_eq!(
-				TargetedFeeAdjustment::<TargetBlockFullness>::convert(feemul(1_000_000_000 * -1)),
-				feemul(-1_000_000_000)
+				TargetedFeeAdjustment::<TargetBlockFullness>::convert(Fixed128::saturating_from_integer(-1)),
+				Fixed128::saturating_from_integer(-1)
 			);
 		})
 	}
@@ -326,7 +326,7 @@ mod tests {
 	fn weight_to_fee_should_not_overflow_on_large_weights() {
 		let kb = 1024 as Weight;
 		let mb = kb * kb;
-		let max_fm = Fixed64::from_natural(i64::max_value());
+		let max_fm = Fixed128::saturating_from_integer(i128::max_value());
 
 		// check that for all values it can compute, correctly.
 		vec![
@@ -343,9 +343,9 @@ mod tests {
 			Weight::max_value(),
 		].into_iter().for_each(|i| {
 			run_with_system_weight(i, || {
-				let next = TargetedFeeAdjustment::<TargetBlockFullness>::convert(Fixed64::default());
-				let truth = fee_multiplier_update(i, Fixed64::default());
-				assert_eq_error_rate!(truth.into_inner(), next.into_inner(), 5);
+				let next = TargetedFeeAdjustment::<TargetBlockFullness>::convert(Fixed128::default());
+				let truth = fee_multiplier_update(i, Fixed128::default());
+				assert_eq_error_rate!(truth, next, Fixed128::from_inner(50_000_000));
 			});
 		});
 

bin/node/runtime/src/lib.rs

@@ -89,7 +89,7 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	// implementation changes and behavior does not, then leave spec_version as
 	// is and increment impl_version.
 	spec_version: 241,
-	impl_version: 0,
+	impl_version: 1,
 	apis: RUNTIME_API_VERSIONS,
 };
 

frame/balances/src/tests.rs

@@ -21,7 +21,7 @@ macro_rules! decl_tests {
 	($test:ty, $ext_builder:ty, $existential_deposit:expr) => {
 
 		use crate::*;
-		use sp_runtime::{Fixed64, traits::{SignedExtension, BadOrigin}};
+		use sp_runtime::{FixedPointNumber, Fixed128, traits::{SignedExtension, BadOrigin}};
 		use frame_support::{
 			assert_noop, assert_ok, assert_err,
 			traits::{LockableCurrency, LockIdentifier, WithdrawReason, WithdrawReasons,
@@ -127,7 +127,7 @@ macro_rules! decl_tests {
 				.monied(true)
 				.build()
 				.execute_with(|| {
-					pallet_transaction_payment::NextFeeMultiplier::put(Fixed64::from_natural(1));
+					pallet_transaction_payment::NextFeeMultiplier::put(Fixed128::saturating_from_integer(1));
 					Balances::set_lock(ID_1, &1, 10, WithdrawReason::Reserve.into());
 					assert_noop!(
 						<Balances as Currency<_>>::transfer(&1, &2, 1, AllowDeath),

frame/transaction-payment/src/lib.rs

@@ -39,7 +39,7 @@ use frame_support::{
 	weights::{Weight, DispatchInfo, GetDispatchInfo},
 };
 use sp_runtime::{
-	Fixed64,
+	Fixed128, FixedPointNumber,
 	transaction_validity::{
 		TransactionPriority, ValidTransaction, InvalidTransaction, TransactionValidityError,
 		TransactionValidity,
@@ -48,7 +48,7 @@ use sp_runtime::{
 };
 use pallet_transaction_payment_rpc_runtime_api::RuntimeDispatchInfo;
 
-type Multiplier = Fixed64;
+type Multiplier = Fixed128;
 type BalanceOf<T> =
 	<<T as Trait>::Currency as Currency<<T as frame_system::Trait>::AccountId>>::Balance;
 type NegativeImbalanceOf<T> =
@@ -78,7 +78,7 @@ pub trait Trait: frame_system::Trait {
 
 decl_storage! {
 	trait Store for Module<T: Trait> as TransactionPayment {
-		pub NextFeeMultiplier get(fn next_fee_multiplier): Multiplier = Multiplier::from_parts(0);
+		pub NextFeeMultiplier get(fn next_fee_multiplier): Multiplier = Multiplier::from_inner(0);
 	}
 }
 
@@ -178,11 +178,10 @@ impl<T: Trait + Send + Sync> ChargeTransactionPayment<T> {
 			// the adjustable part of the fee
 			let adjustable_fee = len_fee.saturating_add(weight_fee);
 			let targeted_fee_adjustment = NextFeeMultiplier::get();
-			// adjusted_fee = adjustable_fee + (adjustable_fee * targeted_fee_adjustment)
-			let adjusted_fee = targeted_fee_adjustment.saturated_multiply_accumulate(adjustable_fee);
+			let adjusted_fee = targeted_fee_adjustment.saturating_mul_acc_int(adjustable_fee.saturated_into());
 
 			let base_fee = T::TransactionBaseFee::get();
-			base_fee.saturating_add(adjusted_fee).saturating_add(tip)
+			base_fee.saturating_add(adjusted_fee.saturated_into()).saturating_add(tip)
 		} else {
 			tip
 		}
@@ -525,7 +524,7 @@ mod tests {
 			.execute_with(||
 		{
 			// all fees should be x1.5
-			NextFeeMultiplier::put(Fixed64::from_rational(1, 2));
+			NextFeeMultiplier::put(Fixed128::saturating_from_rational(1, 2));
 			let len = 10;
 
 			assert!(
@@ -553,7 +552,7 @@ mod tests {
 			.execute_with(||
 		{
 			// all fees should be x1.5
-			NextFeeMultiplier::put(Fixed64::from_rational(1, 2));
+			NextFeeMultiplier::put(Fixed128::saturating_from_rational(1, 2));
 
 			assert_eq!(
 				TransactionPayment::query_info(xt, len),
@@ -582,7 +581,7 @@ mod tests {
 			.execute_with(||
 		{
 			// Next fee multiplier is zero
-			assert_eq!(NextFeeMultiplier::get(), Fixed64::from_natural(0));
+			assert_eq!(NextFeeMultiplier::get(), Fixed128::saturating_from_integer(0));
 
 			// Tip only, no fees works
 			let dispatch_info = DispatchInfo {
@@ -622,7 +621,7 @@ mod tests {
 			.execute_with(||
 		{
 			// Add a next fee multiplier
-			NextFeeMultiplier::put(Fixed64::from_rational(1, 2)); // = 1/2 = .5
+			NextFeeMultiplier::put(Fixed128::saturating_from_rational(1, 2)); // = 1/2 = .5
 			// Base fee is unaffected by multiplier
 			let dispatch_info = DispatchInfo {
 				weight: 0,