From 0a5e4db0699d027de15c2e03980caa44e6f60a7a Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 15 Oct 2019 16:21:31 +0200 Subject: [PATCH 1/7] Better fee parameters --- node/runtime/src/constants.rs | 16 ------- node/runtime/src/impls.rs | 79 +++++++++++++---------------------- node/runtime/src/lib.rs | 10 +++-- srml/system/src/lib.rs | 29 +++++++------ 4 files changed, 53 insertions(+), 81 deletions(-) diff --git a/node/runtime/src/constants.rs b/node/runtime/src/constants.rs index 79d3dbd8eb2bc..fba4c7ac79e56 100644 --- a/node/runtime/src/constants.rs +++ b/node/runtime/src/constants.rs @@ -66,19 +66,3 @@ pub mod time { pub const HOURS: BlockNumber = MINUTES * 60; pub const DAYS: BlockNumber = HOURS * 24; } - -// CRITICAL NOTE: The system module maintains two constants: a _maximum_ block weight and a -// _ratio_ of it yielding the portion which is accessible to normal transactions (reserving the rest -// for operational ones). `TARGET_BLOCK_FULLNESS` is entirely independent and the system module is -// not aware of if, nor should it care about it. This constant simply denotes on which ratio of the -// _maximum_ block weight we tweak the fees. It does NOT care about the type of the dispatch. -// -// For the system to be configured in a sane way, `TARGET_BLOCK_FULLNESS` should always be less than -// the ratio that `system` module uses to find normal transaction quota. -/// Fee-related. -pub mod fee { - pub use sr_primitives::Perbill; - - /// The block saturation level. Fees will be updates based on this value. - pub const TARGET_BLOCK_FULLNESS: Perbill = Perbill::from_percent(25); -} diff --git a/node/runtime/src/impls.rs b/node/runtime/src/impls.rs index 2e1fcc8826e03..6e1407f608433 100644 --- a/node/runtime/src/impls.rs +++ b/node/runtime/src/impls.rs @@ -21,8 +21,10 @@ use sr_primitives::weights::{Weight, WeightMultiplier}; use sr_primitives::traits::{Convert, Saturating}; use sr_primitives::Fixed64; use support::traits::{OnUnbalanced, Currency}; -use crate::{Balances, Authorship, MaximumBlockWeight, NegativeImbalance}; -use crate::constants::fee::TARGET_BLOCK_FULLNESS; +use crate::{ + Balances, Authorship, MaximumBlockWeight, NegativeImbalance, WeightToFee, + TargetedFeeAdjustment +}; pub struct Author; impl OnUnbalanced for Author { @@ -47,48 +49,26 @@ impl Convert for CurrencyToVoteHandler { fn convert(x: u128) -> Balance { x * Self::factor() } } -/// Handles converting a weight scalar to a fee value, based on the scale and granularity of the -/// node's balance type. -/// -/// This should typically create a mapping between the following ranges: -/// - [0, system::MaximumBlockWeight] -/// - [Balance::min, Balance::max] -/// -/// Yet, it can be used for any other sort of change to weight-fee. Some examples being: -/// - Setting it to `0` will essentially disable the weight fee. -/// - Setting it to `1` will cause the literal `#[weight = x]` values to be charged. -/// -/// By default, substrate node will have a weight range of [0, 1_000_000_000]. -pub struct WeightToFee; +/// Simply multiply by a coefficient denoted by the `Get` implementation. impl Convert for WeightToFee { fn convert(x: Weight) -> Balance { - // substrate-node a weight of 10_000 (smallest non-zero weight) to be mapped to 10^7 units of - // fees, hence: - Balance::from(x).saturating_mul(1_000) + Balance::from(x).saturating_mul(Self::get()) } } -/// A struct that updates the weight multiplier based on the saturation level of the previous block. -/// This should typically be called once per-block. +/// Update the given multiplier based on the following formula /// -/// This assumes that weight is a numeric value in the u32 range. -/// -/// Given `TARGET_BLOCK_FULLNESS = 1/2`, a block saturation greater than 1/2 will cause the system -/// fees to slightly grow and the opposite for block saturations less than 1/2. -/// -/// Formula: /// diff = (target_weight - current_block_weight) /// v = 0.00004 /// next_weight = weight * (1 + (v . diff) + (v . diff)^2 / 2) /// +/// Where `target_weight` must be implemented as the `Get` trait. /// https://research.web3.foundation/en/latest/polkadot/Token%20Economics/#relay-chain-transaction-fees -pub struct WeightMultiplierUpdateHandler; - -impl Convert<(Weight, WeightMultiplier), WeightMultiplier> for WeightMultiplierUpdateHandler { +impl Convert<(Weight, WeightMultiplier), WeightMultiplier> for TargetedFeeAdjustment { fn convert(previous_state: (Weight, WeightMultiplier)) -> WeightMultiplier { let (block_weight, multiplier) = previous_state; let max_weight = MaximumBlockWeight::get(); - let target_weight = (TARGET_BLOCK_FULLNESS * max_weight) as u128; + let target_weight = (Self::get() * max_weight) as u128; let block_weight = block_weight as u128; // determines if the first_term is positive @@ -100,8 +80,8 @@ impl Convert<(Weight, WeightMultiplier), WeightMultiplier> for WeightMultiplierU // 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. + // 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 first_term = v.saturating_mul(diff); @@ -174,7 +154,7 @@ mod tests { let mut wm = WeightMultiplier::default(); let mut iterations: u64 = 0; loop { - let next = WeightMultiplierUpdateHandler::convert((block_weight, wm)); + let next = TargetedFeeAdjustment::convert((block_weight, wm)); wm = next; if wm == WeightMultiplier::from_rational(-1, 1) { break; } iterations += 1; @@ -192,13 +172,14 @@ mod tests { let mut wm = WeightMultiplier::default(); let mut iterations: u64 = 0; loop { - let next = WeightMultiplierUpdateHandler::convert((block_weight, wm)); + let next = TargetedFeeAdjustment::convert((block_weight, wm)); if wm == next { break; } wm = next; iterations += 1; let fee = ::WeightToFee::convert(wm.apply_to(tx_weight)); println!( - "iteration {}, new wm = {:?}. Fee at this point is: {} millicents, {} cents, {} dollars", + "iteration {}, new wm = {:?}. Fee at this point is: {} millicents, {} cents\ + , {} dollars", iterations, wm, fee / MILLICENTS, @@ -212,22 +193,22 @@ mod tests { fn stateless_weight_mul() { // Light block. Fee is reduced a little. assert_eq!( - WeightMultiplierUpdateHandler::convert((target() / 4, WeightMultiplier::default())), + TargetedFeeAdjustment::convert((target() / 4, WeightMultiplier::default())), wm(-7500) ); // a bit more. Fee is decreased less, meaning that the fee increases as the block grows. assert_eq!( - WeightMultiplierUpdateHandler::convert((target() / 2, WeightMultiplier::default())), + TargetedFeeAdjustment::convert((target() / 2, WeightMultiplier::default())), wm(-5000) ); // ideal. Original fee. No changes. assert_eq!( - WeightMultiplierUpdateHandler::convert((target(), WeightMultiplier::default())), + TargetedFeeAdjustment::convert((target(), WeightMultiplier::default())), wm(0) ); // // More than ideal. Fee is increased. assert_eq!( - WeightMultiplierUpdateHandler::convert(((target() * 2), WeightMultiplier::default())), + TargetedFeeAdjustment::convert(((target() * 2), WeightMultiplier::default())), wm(10000) ); } @@ -235,20 +216,20 @@ mod tests { #[test] fn stateful_weight_mul_grow_to_infinity() { assert_eq!( - WeightMultiplierUpdateHandler::convert((target() * 2, WeightMultiplier::default())), + TargetedFeeAdjustment::convert((target() * 2, WeightMultiplier::default())), wm(10000) ); assert_eq!( - WeightMultiplierUpdateHandler::convert((target() * 2, wm(10000))), + TargetedFeeAdjustment::convert((target() * 2, wm(10000))), wm(20000) ); assert_eq!( - WeightMultiplierUpdateHandler::convert((target() * 2, wm(20000))), + TargetedFeeAdjustment::convert((target() * 2, wm(20000))), wm(30000) ); // ... assert_eq!( - WeightMultiplierUpdateHandler::convert((target() * 2, wm(1_000_000_000))), + TargetedFeeAdjustment::convert((target() * 2, wm(1_000_000_000))), wm(1_000_000_000 + 10000) ); } @@ -256,20 +237,20 @@ mod tests { #[test] fn stateful_weight_mil_collapse_to_minus_one() { assert_eq!( - WeightMultiplierUpdateHandler::convert((0, WeightMultiplier::default())), + TargetedFeeAdjustment::convert((0, WeightMultiplier::default())), wm(-10000) ); assert_eq!( - WeightMultiplierUpdateHandler::convert((0, wm(-10000))), + TargetedFeeAdjustment::convert((0, wm(-10000))), wm(-20000) ); assert_eq!( - WeightMultiplierUpdateHandler::convert((0, wm(-20000))), + TargetedFeeAdjustment::convert((0, wm(-20000))), wm(-30000) ); // ... assert_eq!( - WeightMultiplierUpdateHandler::convert((0, wm(1_000_000_000 * -1))), + TargetedFeeAdjustment::convert((0, wm(1_000_000_000 * -1))), wm(-1_000_000_000) ); } @@ -283,7 +264,7 @@ mod tests { vec![0, 1, 10, 1000, kb, 10 * kb, 100 * kb, mb, 10 * mb, Weight::max_value() / 2, Weight::max_value()] .into_iter() .for_each(|i| { - WeightMultiplierUpdateHandler::convert((i, WeightMultiplier::default())); + TargetedFeeAdjustment::convert((i, WeightMultiplier::default())); }); // Some values that are all above the target and will cause an increase. @@ -291,7 +272,7 @@ mod tests { vec![t + 100, t * 2, t * 4] .into_iter() .for_each(|i| { - let fm = WeightMultiplierUpdateHandler::convert(( + let fm = TargetedFeeAdjustment::convert(( i, max_fm )); diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 70b1b8f16a7cb..8530f9e31b17e 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -65,7 +65,7 @@ pub use staking::StakerStatus; /// Implementations of some helper traits passed into runtime modules as associated types. pub mod impls; -use impls::{CurrencyToVoteHandler, WeightMultiplierUpdateHandler, Author, WeightToFee}; +use impls::{CurrencyToVoteHandler, Author}; /// Constant values used within the runtime. pub mod constants; @@ -110,9 +110,11 @@ pub type DealWithFees = SplitTwoWays< parameter_types! { pub const BlockHashCount: BlockNumber = 250; pub const MaximumBlockWeight: Weight = 1_000_000_000; - pub const AvailableBlockRatio: Perbill = Perbill::from_percent(75); pub const MaximumBlockLength: u32 = 5 * 1024 * 1024; pub const Version: RuntimeVersion = VERSION; + pub const AvailableBlockRatio: Perbill = Perbill::from_percent(75); + // for a sane configuration, this should always be less than `AvailableBlockRatio`. + pub const TargetedFeeAdjustment: Perbill = Perbill::from_percent(25); } impl system::Trait for Runtime { @@ -125,12 +127,12 @@ impl system::Trait for Runtime { type AccountId = AccountId; type Lookup = Indices; type Header = generic::Header; - type WeightMultiplierUpdate = WeightMultiplierUpdateHandler; type Event = Event; type BlockHashCount = BlockHashCount; type MaximumBlockWeight = MaximumBlockWeight; type MaximumBlockLength = MaximumBlockLength; type AvailableBlockRatio = AvailableBlockRatio; + type WeightMultiplierUpdate = TargetedFeeAdjustment; type Version = Version; } @@ -163,6 +165,8 @@ parameter_types! { pub const CreationFee: Balance = 1 * CENTS; pub const TransactionBaseFee: Balance = 1 * CENTS; pub const TransactionByteFee: Balance = 10 * MILLICENTS; + // setting this to zero will disable the weight fee. + pub const WeightToFee: Balance = 1_000; } impl balances::Trait for Runtime { diff --git a/srml/system/src/lib.rs b/srml/system/src/lib.rs index 71c99af3b3212..0b608456b4465 100644 --- a/srml/system/src/lib.rs +++ b/srml/system/src/lib.rs @@ -155,25 +155,27 @@ pub fn extrinsics_data_root(xts: Vec>) -> H::Output { pub trait Trait: 'static + Eq + Clone { /// The aggregated `Origin` type used by dispatchable calls. - type Origin: Into, Self::Origin>> + From>; + type Origin: + Into, Self::Origin>> + From>; /// The aggregated `Call` type. type Call; - /// Account index (aka nonce) type. This stores the number of previous transactions associated with a sender - /// account. + /// Account index (aka nonce) type. This stores the number of previous transactions associated + /// with a sender account. type Index: - Parameter + Member + MaybeSerializeDebugButNotDeserialize + Default + MaybeDisplay + SimpleArithmetic + Copy; + Parameter + Member + MaybeSerializeDebugButNotDeserialize + Default + MaybeDisplay + + SimpleArithmetic + Copy; /// The block number type used by the runtime. type BlockNumber: - Parameter + Member + MaybeSerializeDebug + MaybeDisplay + SimpleArithmetic + Default + Bounded + Copy - + rstd::hash::Hash; + Parameter + Member + MaybeSerializeDebug + MaybeDisplay + SimpleArithmetic + Default + + Bounded + Copy + rstd::hash::Hash; /// The output of the `Hashing` function. type Hash: - Parameter + Member + MaybeSerializeDebug + MaybeDisplay + SimpleBitOps + Default + Copy + CheckEqual - + rstd::hash::Hash + AsRef<[u8]> + AsMut<[u8]>; + Parameter + Member + MaybeSerializeDebug + MaybeDisplay + SimpleBitOps + Default + Copy + + CheckEqual + rstd::hash::Hash + AsRef<[u8]> + AsMut<[u8]>; /// The hashing system (algorithm) being used in the runtime (e.g. Blake2). type Hashing: Hash; @@ -183,15 +185,16 @@ pub trait Trait: 'static + Eq + Clone { /// Converting trait to take a source type and convert to `AccountId`. /// - /// Used to define the type and conversion mechanism for referencing accounts in transactions. It's perfectly - /// reasonable for this to be an identity conversion (with the source type being `AccountId`), but other modules - /// (e.g. Indices module) may provide more functional/efficient alternatives. + /// Used to define the type and conversion mechanism for referencing accounts in transactions. + /// It's perfectly reasonable for this to be an identity conversion (with the source type being + /// `AccountId`), but other modules (e.g. Indices module) may provide more functional/efficient + /// alternatives. type Lookup: StaticLookup; /// Handler for updating the weight multiplier at the end of each block. /// - /// It receives the current block's weight as input and returns the next weight multiplier for next - /// block. + /// It receives the current block's weight as input and returns the next weight multiplier for + /// next block. /// /// Note that passing `()` will keep the value constant. type WeightMultiplierUpdate: Convert<(Weight, WeightMultiplier), WeightMultiplier>; From fd1867a8a5ffb1f42f9898bd97e53ae438feda93 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 15 Oct 2019 17:02:00 +0200 Subject: [PATCH 2/7] Fix build --- node/executor/src/lib.rs | 4 ++-- node/runtime/src/impls.rs | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index 2c2ad479f5c19..dee9fb19c41a1 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -55,8 +55,8 @@ mod tests { use system::{EventRecord, Phase}; use node_runtime::{ Header, Block, UncheckedExtrinsic, CheckedExtrinsic, Call, Runtime, Balances, BuildStorage, - System, Event, TransferFee, TransactionBaseFee, TransactionByteFee, - constants::currency::*, impls::WeightToFee, + System, Event, TransferFee, TransactionBaseFee, TransactionByteFee, WeightToFee, + constants::currency::*, }; use node_primitives::{Balance, Hash, BlockNumber}; use node_testing::keyring::*; diff --git a/node/runtime/src/impls.rs b/node/runtime/src/impls.rs index 6e1407f608433..581333af1bd1e 100644 --- a/node/runtime/src/impls.rs +++ b/node/runtime/src/impls.rs @@ -121,7 +121,7 @@ mod tests { } fn target() -> Weight { - TARGET_BLOCK_FULLNESS * max() + ::WeightMultiplierUpdate::get() * max() } // poc reference implementation. From 734bfd8454e59ab8e302d4939a4e0197fd7f850c Mon Sep 17 00:00:00 2001 From: kianenigma Date: Fri, 18 Oct 2019 11:36:55 +0200 Subject: [PATCH 3/7] Better runtime tests --- core/sr-primitives/src/sr_arithmetic.rs | 6 ++++ node/executor/src/lib.rs | 2 -- node/runtime/src/impls.rs | 45 +++++++++++++------------ 3 files changed, 30 insertions(+), 23 deletions(-) diff --git a/core/sr-primitives/src/sr_arithmetic.rs b/core/sr-primitives/src/sr_arithmetic.rs index 5f5b5dc1e523b..c11845dfe58ee 100644 --- a/core/sr-primitives/src/sr_arithmetic.rs +++ b/core/sr-primitives/src/sr_arithmetic.rs @@ -557,6 +557,12 @@ impl Fixed64 { Self(int.saturating_mul(DIV)) } + /// Return the inner value. Only for testing. + #[cfg(any(feature = "std", test))] + pub fn into_inner(self) -> i64 { + self.0 + } + /// Return the accuracy of the type. Given that this function returns the value `X`, it means /// that an instance composed of `X` parts (`Fixed64::from_parts(X)`) is equal to `1`. pub fn accuracy() -> i64 { diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index d6665dc9ec836..f5cb7c8c64bb6 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -480,8 +480,6 @@ mod tests { ).0.unwrap(); t.execute_with(|| { - // NOTE: fees differ slightly in tests that execute more than one block due to the - // weight update. Hence, using `assert_eq_error_rate`. assert_eq!( Balances::total_balance(&alice()), alice_last_known_balance - 10 * DOLLARS - transfer_fee(&xt(), fm), diff --git a/node/runtime/src/impls.rs b/node/runtime/src/impls.rs index 6e1b1ca1f83d6..ae53c59a057a5 100644 --- a/node/runtime/src/impls.rs +++ b/node/runtime/src/impls.rs @@ -23,7 +23,7 @@ use sr_primitives::{Fixed64, Perbill}; use support::traits::{OnUnbalanced, Currency, Get}; use crate::{ Balances, System, Authorship, MaximumBlockWeight, NegativeImbalance, WeightToFee, - TargetedFeeAdjustment, TransactionPayment, + TargetedFeeAdjustment, }; pub struct Author; @@ -58,7 +58,7 @@ impl Convert for WeightToFee { /// Update the given multiplier based on the following formula /// -/// diff = (target_weight - current_block_weight) +/// diff = (target_weight - previous_block_weight) /// v = 0.00004 /// next_weight = weight * (1 + (v . diff) + (v . diff)^2 / 2) /// @@ -112,8 +112,9 @@ impl Convert for TargetedFeeAdjustment { mod tests { use super::*; use sr_primitives::weights::Weight; + use sr_primitives::assert_eq_error_rate; use crate::{MaximumBlockWeight, AvailableBlockRatio, Runtime}; - use crate::constants::currency::*; + use crate::{constants::currency::*, TransactionPayment}; fn max() -> Weight { MaximumBlockWeight::get() @@ -135,8 +136,7 @@ mod tests { // Current saturation in terms of weight let s = block_weight; - let fm = (v * (s/m - ss/m)) + (v.powi(2) * (s/m - ss/m).powi(2)) / 2.0; - dbg!(fm * 1_000_000_000_f32); + 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); previous.saturating_add(addition_fm) } @@ -158,8 +158,7 @@ mod tests { fn fee_multiplier_update_poc_works() { let fm = Fixed64::from_rational(0, 1); let test_set = vec![ - // TODO: this has a rounding error and fails. - // (0, fm.clone()), + (0, fm.clone()), (100, fm.clone()), (target(), fm.clone()), (max() / 2, fm.clone()), @@ -167,12 +166,10 @@ mod tests { ]; test_set.into_iter().for_each(|(w, fm)| { run_with_system_weight(w, || { - assert_eq!( - fee_multiplier_update(w, fm), - TargetedFeeAdjustment::convert(fm), - "failed for weight {} and prev fm {:?}", - w, - fm, + assert_eq_error_rate!( + fee_multiplier_update(w, fm).into_inner(), + TargetedFeeAdjustment::convert(fm).into_inner(), + 5, ); }) }) @@ -181,7 +178,7 @@ mod tests { #[test] fn empty_chain_simulation() { // just a few txs per_block. - let block_weight = 1000; + let block_weight = 0; run_with_system_weight(block_weight, || { let mut fm = Fixed64::default(); let mut iterations: u64 = 0; @@ -192,6 +189,13 @@ mod tests { iterations += 1; } println!("iteration {}, new fm = {:?}. Weight fee is now zero", iterations, fm); + assert!(iterations > 50_000, "This assertion is just a warning; Don't panic. \ + Current substrate/polkadot node are configured with a _slow adjusting fee_ \ + mechanism. Hence, it is really unlikely that fees collapse to zero even on an empty chain \ + in less than at least of couple of thousands of empty blocks. But this simulation + indicates that fees collapsed to zero after {} almost-empty blocks. Check it", + iterations, + ); }) } @@ -201,12 +205,10 @@ mod tests { // `cargo test congested_chain_simulation -- --nocapture` to get some insight. // almost full. The entire quota of normal transactions is taken. - let block_weight = AvailableBlockRatio::get() * max(); + let block_weight = AvailableBlockRatio::get() * max() - 100; - // default minimum substrate weight - let tx_weight = 10_000u32; - let tx_per_block = 100; - let block_weight = tx_weight * tx_per_block; + // Default substrate minimum. + let tx_weight = 10_000; run_with_system_weight(block_weight, || { // initial value configured on module @@ -216,7 +218,8 @@ mod tests { let mut iterations: u64 = 0; loop { let next = TargetedFeeAdjustment::convert(fm); - if fm == next { break; } + // if no change, panic. This should never happen in this case. + if fm == next { panic!("The fee should ever increase"); } fm = next; iterations += 1; let fee = ::WeightToFee::convert(tx_weight); @@ -337,7 +340,7 @@ mod tests { run_with_system_weight(i, || { let next = TargetedFeeAdjustment::convert(Fixed64::default()); let truth = fee_multiplier_update(i, Fixed64::default()); - assert_eq!(next, truth); + assert_eq_error_rate!(truth.into_inner(), next.into_inner(), 5); }); }); From 9fef4e56cf9cef80a3ff469040ec81012de65b77 Mon Sep 17 00:00:00 2001 From: Joshy Orndorff Date: Tue, 22 Oct 2019 06:05:10 -0400 Subject: [PATCH 4/7] Price to Weight ratio as type parameter (#3856) * Price to Weight ration as type parameter * Kian feedback * Some renames. * Fix executor tests * Getting Closer. * Phantom Data * Actually fix executor tests. --- node/executor/src/lib.rs | 5 +++-- node/runtime/src/impls.rs | 18 +++++++++++++----- node/runtime/src/lib.rs | 6 +++--- 3 files changed, 19 insertions(+), 10 deletions(-) diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index f5cb7c8c64bb6..cb8627f65beab 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -56,8 +56,9 @@ mod tests { use node_runtime::{ Header, Block, UncheckedExtrinsic, CheckedExtrinsic, Call, Runtime, Balances, BuildStorage, System, TransactionPayment, Event, TransferFee, TransactionBaseFee, TransactionByteFee, - WeightToFee, constants::currency::*, + WeightFeeCoefficient, constants::currency::*, }; + use node_runtime::impls::LinearWeightToFee; use node_primitives::{Balance, Hash, BlockNumber}; use node_testing::keyring::*; use wabt; @@ -1046,7 +1047,7 @@ mod tests { balance_alice -= length_fee; let weight = default_transfer_call().get_dispatch_info().weight; - let weight_fee = WeightToFee::convert(weight); + let weight_fee = LinearWeightToFee::::convert(weight); // we know that weight to fee multiplier is effect-less in block 1. assert_eq!(weight_fee as Balance, MILLICENTS); diff --git a/node/runtime/src/impls.rs b/node/runtime/src/impls.rs index ae53c59a057a5..26a90fe6097f8 100644 --- a/node/runtime/src/impls.rs +++ b/node/runtime/src/impls.rs @@ -22,7 +22,7 @@ use sr_primitives::traits::{Convert, Saturating}; use sr_primitives::{Fixed64, Perbill}; use support::traits::{OnUnbalanced, Currency, Get}; use crate::{ - Balances, System, Authorship, MaximumBlockWeight, NegativeImbalance, WeightToFee, + Balances, System, Authorship, MaximumBlockWeight, NegativeImbalance, TargetedFeeAdjustment, }; @@ -49,10 +49,18 @@ impl Convert for CurrencyToVoteHandler { fn convert(x: u128) -> Balance { x * Self::factor() } } -/// Simply multiply by a coefficient denoted by the `Get` implementation. -impl Convert for WeightToFee { - fn convert(x: Weight) -> Balance { - Balance::from(x).saturating_mul(Self::get()) +/// Convert from weight to balance via a simple coefficient multiplication +/// The associated type C encapsulates a constant in units of balance per weight +pub struct LinearWeightToFee(rstd::marker::PhantomData); + +impl Convert for LinearWeightToFee + where C: Get { + + fn convert(w: Weight) -> Balance { + // substrate-node a weight of 10_000 (smallest non-zero weight) to be mapped to 10^7 units of + // fees, hence: + let coefficient = C::get(); + Balance::from(w).saturating_mul(coefficient) } } diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 944ef5127d40b..25e3ad1dff588 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -65,7 +65,7 @@ pub use staking::StakerStatus; /// Implementations of some helper traits passed into runtime modules as associated types. pub mod impls; -use impls::{CurrencyToVoteHandler, Author}; +use impls::{CurrencyToVoteHandler, Author, LinearWeightToFee}; /// Constant values used within the runtime. pub mod constants; @@ -178,7 +178,7 @@ parameter_types! { pub const TransactionBaseFee: Balance = 1 * CENTS; pub const TransactionByteFee: Balance = 10 * MILLICENTS; // setting this to zero will disable the weight fee. - pub const WeightToFee: Balance = 1_000; + pub const WeightFeeCoefficient: Balance = 1_000; // for a sane configuration, this should always be less than `AvailableBlockRatio`. pub const TargetedFeeAdjustment: Perbill = Perbill::from_percent(25); } @@ -188,7 +188,7 @@ impl transaction_payment::Trait for Runtime { type OnTransactionPayment = DealWithFees; type TransactionBaseFee = TransactionBaseFee; type TransactionByteFee = TransactionByteFee; - type WeightToFee = WeightToFee; + type WeightToFee = LinearWeightToFee; type FeeMultiplierUpdate = TargetedFeeAdjustment; } From d1d57d10db5290b296859e6aa744588ad37532d5 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 22 Oct 2019 14:34:53 +0200 Subject: [PATCH 5/7] Fix tests. --- node/executor/src/lib.rs | 4 +-- node/runtime/src/impls.rs | 62 +++++++++++++++++++-------------------- node/runtime/src/lib.rs | 6 ++-- 3 files changed, 34 insertions(+), 38 deletions(-) diff --git a/node/executor/src/lib.rs b/node/executor/src/lib.rs index cb8627f65beab..827aab1c09f1b 100644 --- a/node/executor/src/lib.rs +++ b/node/executor/src/lib.rs @@ -37,9 +37,7 @@ mod tests { use super::Executor; use {balances, contracts, indices, system, timestamp}; use codec::{Encode, Decode, Joiner}; - use runtime_support::{ - Hashable, StorageValue, StorageMap, traits::Currency, - }; + use runtime_support::{Hashable, StorageValue, StorageMap, traits::Currency}; use state_machine::TestExternalities as CoreTestExternalities; use primitives::{ Blake2Hasher, NeverNativeValue, NativeOrEncoded, map, diff --git a/node/runtime/src/impls.rs b/node/runtime/src/impls.rs index 26a90fe6097f8..2e9bd38c8f822 100644 --- a/node/runtime/src/impls.rs +++ b/node/runtime/src/impls.rs @@ -21,10 +21,7 @@ use sr_primitives::weights::Weight; use sr_primitives::traits::{Convert, Saturating}; use sr_primitives::{Fixed64, Perbill}; use support::traits::{OnUnbalanced, Currency, Get}; -use crate::{ - Balances, System, Authorship, MaximumBlockWeight, NegativeImbalance, - TargetedFeeAdjustment, -}; +use crate::{Balances, System, Authorship, MaximumBlockWeight, NegativeImbalance}; pub struct Author; impl OnUnbalanced for Author { @@ -53,9 +50,7 @@ impl Convert for CurrencyToVoteHandler { /// The associated type C encapsulates a constant in units of balance per weight pub struct LinearWeightToFee(rstd::marker::PhantomData); -impl Convert for LinearWeightToFee - where C: Get { - +impl> Convert for LinearWeightToFee { fn convert(w: Weight) -> Balance { // substrate-node a weight of 10_000 (smallest non-zero weight) to be mapped to 10^7 units of // fees, hence: @@ -70,13 +65,15 @@ impl Convert for LinearWeightToFee /// v = 0.00004 /// next_weight = weight * (1 + (v . diff) + (v . diff)^2 / 2) /// -/// Where `target_weight` must be implemented as the `Get` trait. +/// Where `target_weight` must be given as the `Get` implementation of the `T` generic type. /// https://research.web3.foundation/en/latest/polkadot/Token%20Economics/#relay-chain-transaction-fees -impl Convert for TargetedFeeAdjustment { +pub struct TargetedFeeAdjustment(rstd::marker::PhantomData); + +impl> Convert for TargetedFeeAdjustment { fn convert(multiplier: Fixed64) -> Fixed64 { let block_weight = System::all_extrinsics_weight(); let max_weight = MaximumBlockWeight::get(); - let target_weight = (>::get() * max_weight) as u128; + let target_weight = (T::get() * max_weight) as u128; let block_weight = block_weight as u128; // determines if the first_term is positive @@ -122,14 +119,14 @@ mod tests { use sr_primitives::weights::Weight; use sr_primitives::assert_eq_error_rate; use crate::{MaximumBlockWeight, AvailableBlockRatio, Runtime}; - use crate::{constants::currency::*, TransactionPayment}; + use crate::{constants::currency::*, TransactionPayment, TargetBlockFullness}; fn max() -> Weight { MaximumBlockWeight::get() } fn target() -> Weight { - ::FeeMultiplierUpdate::get() * max() + TargetBlockFullness::get() * max() } // poc reference implementation. @@ -176,7 +173,7 @@ mod tests { run_with_system_weight(w, || { assert_eq_error_rate!( fee_multiplier_update(w, fm).into_inner(), - TargetedFeeAdjustment::convert(fm).into_inner(), + TargetedFeeAdjustment::::convert(fm).into_inner(), 5, ); }) @@ -191,7 +188,7 @@ mod tests { let mut fm = Fixed64::default(); let mut iterations: u64 = 0; loop { - let next = TargetedFeeAdjustment::convert(fm); + let next = TargetedFeeAdjustment::::convert(fm); fm = next; if fm == Fixed64::from_rational(-1, 1) { break; } iterations += 1; @@ -199,9 +196,10 @@ mod tests { println!("iteration {}, new fm = {:?}. Weight fee is now zero", iterations, fm); assert!(iterations > 50_000, "This assertion is just a warning; Don't panic. \ Current substrate/polkadot node are configured with a _slow adjusting fee_ \ - mechanism. Hence, it is really unlikely that fees collapse to zero even on an empty chain \ - in less than at least of couple of thousands of empty blocks. But this simulation - indicates that fees collapsed to zero after {} almost-empty blocks. Check it", + mechanism. Hence, it is really unlikely that fees collapse to zero even on an \ + empty chain in less than at least of couple of thousands of empty blocks. But this \ + simulation indicates that fees collapsed to zero after {} almost-empty blocks. \ + Check it", iterations, ); }) @@ -225,7 +223,7 @@ mod tests { let mut iterations: u64 = 0; loop { - let next = TargetedFeeAdjustment::convert(fm); + let next = TargetedFeeAdjustment::::convert(fm); // if no change, panic. This should never happen in this case. if fm == next { panic!("The fee should ever increase"); } fm = next; @@ -251,14 +249,14 @@ mod tests { run_with_system_weight(target() / 4, || { // Light block. Fee is reduced a little. assert_eq!( - TargetedFeeAdjustment::convert(Fixed64::default()), + TargetedFeeAdjustment::::convert(Fixed64::default()), feemul(-7500), ); }); run_with_system_weight(target() / 2, || { // a bit more. Fee is decreased less, meaning that the fee increases as the block grows. assert_eq!( - TargetedFeeAdjustment::convert(Fixed64::default()), + TargetedFeeAdjustment::::convert(Fixed64::default()), feemul(-5000), ); @@ -266,14 +264,14 @@ mod tests { run_with_system_weight(target(), || { // ideal. Original fee. No changes. assert_eq!( - TargetedFeeAdjustment::convert(Fixed64::default()), + TargetedFeeAdjustment::::convert(Fixed64::default()), feemul(0), ); }); run_with_system_weight(target() * 2, || { // // More than ideal. Fee is increased. assert_eq!( - TargetedFeeAdjustment::convert(Fixed64::default()), + TargetedFeeAdjustment::::convert(Fixed64::default()), feemul(10000), ); }); @@ -283,20 +281,20 @@ mod tests { fn stateful_weight_mul_grow_to_infinity() { run_with_system_weight(target() * 2, || { assert_eq!( - TargetedFeeAdjustment::convert(Fixed64::default()), + TargetedFeeAdjustment::::convert(Fixed64::default()), feemul(10000) ); assert_eq!( - TargetedFeeAdjustment::convert(feemul(10000)), + TargetedFeeAdjustment::::convert(feemul(10000)), feemul(20000) ); assert_eq!( - TargetedFeeAdjustment::convert(feemul(20000)), + TargetedFeeAdjustment::::convert(feemul(20000)), feemul(30000) ); // ... assert_eq!( - TargetedFeeAdjustment::convert(feemul(1_000_000_000)), + TargetedFeeAdjustment::::convert(feemul(1_000_000_000)), feemul(1_000_000_000 + 10000) ); }); @@ -306,20 +304,20 @@ mod tests { fn stateful_weight_mil_collapse_to_minus_one() { run_with_system_weight(0, || { assert_eq!( - TargetedFeeAdjustment::convert(Fixed64::default()), + TargetedFeeAdjustment::::convert(Fixed64::default()), feemul(-10000) ); assert_eq!( - TargetedFeeAdjustment::convert(feemul(-10000)), + TargetedFeeAdjustment::::convert(feemul(-10000)), feemul(-20000) ); assert_eq!( - TargetedFeeAdjustment::convert(feemul(-20000)), + TargetedFeeAdjustment::::convert(feemul(-20000)), feemul(-30000) ); // ... assert_eq!( - TargetedFeeAdjustment::convert(feemul(1_000_000_000 * -1)), + TargetedFeeAdjustment::::convert(feemul(1_000_000_000 * -1)), feemul(-1_000_000_000) ); }) @@ -346,7 +344,7 @@ mod tests { Weight::max_value() ].into_iter().for_each(|i| { run_with_system_weight(i, || { - let next = TargetedFeeAdjustment::convert(Fixed64::default()); + let next = TargetedFeeAdjustment::::convert(Fixed64::default()); let truth = fee_multiplier_update(i, Fixed64::default()); assert_eq_error_rate!(truth.into_inner(), next.into_inner(), 5); }); @@ -358,7 +356,7 @@ mod tests { .into_iter() .for_each(|i| { run_with_system_weight(i, || { - let fm = TargetedFeeAdjustment::convert(max_fm); + let fm = TargetedFeeAdjustment::::convert(max_fm); // won't grow. The convert saturates everything. assert_eq!(fm, max_fm); }) diff --git a/node/runtime/src/lib.rs b/node/runtime/src/lib.rs index 25e3ad1dff588..477f69986b29f 100644 --- a/node/runtime/src/lib.rs +++ b/node/runtime/src/lib.rs @@ -65,7 +65,7 @@ pub use staking::StakerStatus; /// Implementations of some helper traits passed into runtime modules as associated types. pub mod impls; -use impls::{CurrencyToVoteHandler, Author, LinearWeightToFee}; +use impls::{CurrencyToVoteHandler, Author, LinearWeightToFee, TargetedFeeAdjustment}; /// Constant values used within the runtime. pub mod constants; @@ -180,7 +180,7 @@ parameter_types! { // setting this to zero will disable the weight fee. pub const WeightFeeCoefficient: Balance = 1_000; // for a sane configuration, this should always be less than `AvailableBlockRatio`. - pub const TargetedFeeAdjustment: Perbill = Perbill::from_percent(25); + pub const TargetBlockFullness: Perbill = Perbill::from_percent(25); } impl transaction_payment::Trait for Runtime { @@ -189,7 +189,7 @@ impl transaction_payment::Trait for Runtime { type TransactionBaseFee = TransactionBaseFee; type TransactionByteFee = TransactionByteFee; type WeightToFee = LinearWeightToFee; - type FeeMultiplierUpdate = TargetedFeeAdjustment; + type FeeMultiplierUpdate = TargetedFeeAdjustment; } parameter_types! { From 1709e84f0b068fbc79793a34e11a4342c5f248b2 Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 22 Oct 2019 14:37:59 +0200 Subject: [PATCH 6/7] Remove todo --- core/sr-arithmetic/src/biguint.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/core/sr-arithmetic/src/biguint.rs b/core/sr-arithmetic/src/biguint.rs index b7d93c2f0d3ca..c0836e09b301f 100644 --- a/core/sr-arithmetic/src/biguint.rs +++ b/core/sr-arithmetic/src/biguint.rs @@ -21,7 +21,7 @@ use rstd::{cmp::Ordering, ops, prelude::*, cell::RefCell, convert::TryFrom}; // A sensible value for this would be half of the dword size of the host machine. Since the // runtime is compiled to 32bit webassembly, using 32 and 64 for single and double respectively -// should yield the most performance. TODO #3745 we could benchmark this and verify. +// should yield the most performance. /// Representation of a single limb. pub type Single = u32; /// Representation of two limbs. From 3448f85f1a9c81e8106c37be0ebfb4e5053f56aa Mon Sep 17 00:00:00 2001 From: kianenigma Date: Tue, 22 Oct 2019 20:56:36 +0200 Subject: [PATCH 7/7] Fix build --- core/sr-arithmetic/src/fixed64.rs | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/core/sr-arithmetic/src/fixed64.rs b/core/sr-arithmetic/src/fixed64.rs index 3bac75898efbb..bd58e36940ae6 100644 --- a/core/sr-arithmetic/src/fixed64.rs +++ b/core/sr-arithmetic/src/fixed64.rs @@ -48,6 +48,12 @@ impl Fixed64 { DIV } + /// Consume self and return the inner value. + /// + /// This should only be used for testing. + #[cfg(any(feature = "std", test))] + pub fn into_inner(self) -> i64 { self.0 } + /// Raw constructor. Equal to `parts / 1_000_000_000`. pub fn from_parts(parts: i64) -> Self { Self(parts)