From e1318497c27110fc94c5f2357a1a8bf96755c79b Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 15 Sep 2022 17:56:51 +0200 Subject: [PATCH 01/13] Maximum value for MultiplierUpdate --- bin/node/runtime/src/impls.rs | 5 +++-- bin/node/runtime/src/lib.rs | 12 +++++++++--- frame/transaction-payment/src/lib.rs | 25 +++++++++++++++++++++---- 3 files changed, 33 insertions(+), 9 deletions(-) diff --git a/bin/node/runtime/src/impls.rs b/bin/node/runtime/src/impls.rs index fb2f3cec65290..f7cb5fe65b2e1 100644 --- a/bin/node/runtime/src/impls.rs +++ b/bin/node/runtime/src/impls.rs @@ -126,8 +126,8 @@ mod multiplier_tests { use crate::{ constants::{currency::*, time::*}, - AdjustmentVariable, MinimumMultiplier, Runtime, RuntimeBlockWeights as BlockWeights, - System, TargetBlockFullness, TransactionPayment, + AdjustmentVariable, MaximumMultiplier, MinimumMultiplier, Runtime, + RuntimeBlockWeights as BlockWeights, System, TargetBlockFullness, TransactionPayment, }; use frame_support::{ dispatch::DispatchClass, @@ -156,6 +156,7 @@ mod multiplier_tests { TargetBlockFullness, AdjustmentVariable, MinimumMultiplier, + MaximumMultiplier, >::convert(fm) } diff --git a/bin/node/runtime/src/lib.rs b/bin/node/runtime/src/lib.rs index 2f0efcaa56740..ff98d4a5f4c4e 100644 --- a/bin/node/runtime/src/lib.rs +++ b/bin/node/runtime/src/lib.rs @@ -65,7 +65,7 @@ use sp_runtime::{ curve::PiecewiseLinear, generic, impl_opaque_keys, traits::{ - self, BlakeTwo256, Block as BlockT, ConvertInto, NumberFor, OpaqueKeys, + self, BlakeTwo256, Block as BlockT, Bounded, ConvertInto, NumberFor, OpaqueKeys, SaturatedConversion, StaticLookup, }, transaction_validity::{TransactionPriority, TransactionSource, TransactionValidity}, @@ -447,6 +447,7 @@ parameter_types! { pub const TargetBlockFullness: Perquintill = Perquintill::from_percent(25); pub AdjustmentVariable: Multiplier = Multiplier::saturating_from_rational(1, 100_000); pub MinimumMultiplier: Multiplier = Multiplier::saturating_from_rational(1, 1_000_000_000u128); + pub MaximumMultiplier: Multiplier = Bounded::max_value(); } impl pallet_transaction_payment::Config for Runtime { @@ -455,8 +456,13 @@ impl pallet_transaction_payment::Config for Runtime { type OperationalFeeMultiplier = OperationalFeeMultiplier; type WeightToFee = IdentityFee; type LengthToFee = ConstantMultiplier; - type FeeMultiplierUpdate = - TargetedFeeAdjustment; + type FeeMultiplierUpdate = TargetedFeeAdjustment< + Self, + TargetBlockFullness, + AdjustmentVariable, + MinimumMultiplier, + MaximumMultiplier, + >; } impl pallet_asset_tx_payment::Config for Runtime { diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 0447ea6bfd99d..e29030e53c8fc 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -127,12 +127,14 @@ type BalanceOf = <::OnChargeTransaction as OnChargeTransaction -pub struct TargetedFeeAdjustment(sp_std::marker::PhantomData<(T, S, V, M)>); +pub struct TargetedFeeAdjustment(sp_std::marker::PhantomData<(T, S, V, M, X)>); /// Something that can convert the current multiplier to the next one. pub trait MultiplierUpdate: Convert { /// Minimum multiplier fn min() -> Multiplier; + /// Maximum multiplier. Any outcome of the `convert` function should be less than this. + fn max() -> Multiplier; /// Target block saturation level fn target() -> Perquintill; /// Variability factor @@ -143,6 +145,9 @@ impl MultiplierUpdate for () { fn min() -> Multiplier { Default::default() } + fn max() -> Multiplier { + ::max_value() + } fn target() -> Perquintill { Default::default() } @@ -151,16 +156,20 @@ impl MultiplierUpdate for () { } } -impl MultiplierUpdate for TargetedFeeAdjustment +impl MultiplierUpdate for TargetedFeeAdjustment where T: frame_system::Config, S: Get, V: Get, M: Get, + X: Get, { fn min() -> Multiplier { M::get() } + fn max() -> Multiplier { + X::get() + } fn target() -> Perquintill { S::get() } @@ -169,12 +178,13 @@ where } } -impl Convert for TargetedFeeAdjustment +impl Convert for TargetedFeeAdjustment where T: frame_system::Config, S: Get, V: Get, M: Get, + X: Get, { fn convert(previous: Multiplier) -> Multiplier { // Defensive only. The multiplier in storage should always be at most positive. Nonetheless @@ -217,7 +227,11 @@ where if positive { let excess = first_term.saturating_add(second_term).saturating_mul(previous); - previous.saturating_add(excess).max(min_multiplier) + let next_multiplier = previous.saturating_add(excess).max(min_multiplier); + if next_multiplier > X::get() { + return X::get() + } + next_multiplier } else { // Defensive-only: first_term > second_term. Safe subtraction. let negative = first_term.saturating_sub(second_term).saturating_mul(previous); @@ -233,6 +247,9 @@ impl> MultiplierUpdate for ConstFeeMultiplier { fn min() -> Multiplier { M::get() } + fn max() -> Multiplier { + ::max_value() + } fn target() -> Perquintill { Default::default() } From 16d81fa1401256578a0b3095640cf191e4c021d9 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Thu, 15 Sep 2022 21:09:47 +0200 Subject: [PATCH 02/13] Update frame/transaction-payment/src/lib.rs Co-authored-by: Stephen Shelton --- frame/transaction-payment/src/lib.rs | 9 ++++----- 1 file changed, 4 insertions(+), 5 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index e29030e53c8fc..afe5dbed91e77 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -227,11 +227,10 @@ where if positive { let excess = first_term.saturating_add(second_term).saturating_mul(previous); - let next_multiplier = previous.saturating_add(excess).max(min_multiplier); - if next_multiplier > X::get() { - return X::get() - } - next_multiplier + previous + .saturating_add(excess) + .max(min_multiplier) + .min(max_multiplier) } else { // Defensive-only: first_term > second_term. Safe subtraction. let negative = first_term.saturating_sub(second_term).saturating_mul(previous); From 833ff88df68fc959f43b6a8b7edbed37c69f1f7b Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Thu, 15 Sep 2022 21:34:23 +0200 Subject: [PATCH 03/13] Update lib.rs --- frame/transaction-payment/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index afe5dbed91e77..c9f8e4a8275f5 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -229,8 +229,8 @@ where let excess = first_term.saturating_add(second_term).saturating_mul(previous); previous .saturating_add(excess) - .max(min_multiplier) - .min(max_multiplier) + .max(M::get()) + .min(X::get()) } else { // Defensive-only: first_term > second_term. Safe subtraction. let negative = first_term.saturating_sub(second_term).saturating_mul(previous); From 308a78685d5d53df075be2f763ebc323f9c607a1 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Fri, 16 Sep 2022 07:02:04 +0200 Subject: [PATCH 04/13] return constant --- frame/transaction-payment/src/lib.rs | 12 ++++++++---- 1 file changed, 8 insertions(+), 4 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index c9f8e4a8275f5..1a49f232d0714 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -240,14 +240,17 @@ where } /// A struct to make the fee multiplier a constant -pub struct ConstFeeMultiplier>(sp_std::marker::PhantomData); +pub struct ConstFeeMultiplier, X: Get>( + sp_std::marker::PhantomData, + sp_std::marker::PhantomData, +); -impl> MultiplierUpdate for ConstFeeMultiplier { +impl, X: Get> MultiplierUpdate for ConstFeeMultiplier { fn min() -> Multiplier { M::get() } fn max() -> Multiplier { - ::max_value() + X::get() } fn target() -> Perquintill { Default::default() @@ -257,9 +260,10 @@ impl> MultiplierUpdate for ConstFeeMultiplier { } } -impl Convert for ConstFeeMultiplier +impl Convert for ConstFeeMultiplier where M: Get, + X: Get, { fn convert(_previous: Multiplier) -> Multiplier { Self::min() From d1eee7f4c7b4a99dc7c65518aa4a20746097637e Mon Sep 17 00:00:00 2001 From: Szegoo Date: Fri, 16 Sep 2022 07:11:02 +0200 Subject: [PATCH 05/13] fix in runtime --- bin/node-template/runtime/src/lib.rs | 8 +++++--- frame/transaction-payment/src/lib.rs | 5 +---- 2 files changed, 6 insertions(+), 7 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 099654a12a420..c7532b245a920 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -15,7 +15,8 @@ use sp_core::{crypto::KeyTypeId, OpaqueMetadata}; use sp_runtime::{ create_runtime_str, generic, impl_opaque_keys, traits::{ - AccountIdLookup, BlakeTwo256, Block as BlockT, IdentifyAccount, NumberFor, One, Verify, + AccountIdLookup, BlakeTwo256, Block as BlockT, Bounded, IdentifyAccount, NumberFor, One, + Verify, }, transaction_validity::{TransactionSource, TransactionValidity}, ApplyExtrinsicResult, MultiSignature, @@ -253,7 +254,8 @@ impl pallet_balances::Config for Runtime { } parameter_types! { - pub FeeMultiplier: Multiplier = Multiplier::one(); + pub FeeMultiplierMin: Multiplier = Multiplier::one(); + pub FeeMultiplierMax: Multiplier = Bounded::max_value(); } impl pallet_transaction_payment::Config for Runtime { @@ -262,7 +264,7 @@ impl pallet_transaction_payment::Config for Runtime { type OperationalFeeMultiplier = ConstU8<5>; type WeightToFee = IdentityFee; type LengthToFee = IdentityFee; - type FeeMultiplierUpdate = ConstFeeMultiplier; + type FeeMultiplierUpdate = ConstFeeMultiplier; } impl pallet_sudo::Config for Runtime { diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 1a49f232d0714..c8ef046deaeea 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -227,10 +227,7 @@ where if positive { let excess = first_term.saturating_add(second_term).saturating_mul(previous); - previous - .saturating_add(excess) - .max(M::get()) - .min(X::get()) + previous.saturating_add(excess).max(M::get()).min(X::get()) } else { // Defensive-only: first_term > second_term. Safe subtraction. let negative = first_term.saturating_sub(second_term).saturating_mul(previous); From c58bd09fc2a039cc5290ea4b3c5cd093571c9d19 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Wed, 21 Sep 2022 19:40:14 +0200 Subject: [PATCH 06/13] Update frame/transaction-payment/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- frame/transaction-payment/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index c8ef046deaeea..83ac946b867ec 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -131,7 +131,7 @@ pub struct TargetedFeeAdjustment(sp_std::marker::PhantomData<(T, /// Something that can convert the current multiplier to the next one. pub trait MultiplierUpdate: Convert { - /// Minimum multiplier + /// Minimum multiplier. Any outcome of the `convert` function should be less than this. fn min() -> Multiplier; /// Maximum multiplier. Any outcome of the `convert` function should be less than this. fn max() -> Multiplier; From 717cb7ea7b00e54027140fa96e1728285f532a50 Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Wed, 21 Sep 2022 19:40:43 +0200 Subject: [PATCH 07/13] Update frame/transaction-payment/src/lib.rs Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- frame/transaction-payment/src/lib.rs | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 83ac946b867ec..3452e34a303e4 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -237,10 +237,7 @@ where } /// A struct to make the fee multiplier a constant -pub struct ConstFeeMultiplier, X: Get>( - sp_std::marker::PhantomData, - sp_std::marker::PhantomData, -); +pub struct ConstFeeMultiplier, X: Get>(sp_std::marker::PhantomData<(M, X)>); impl, X: Get> MultiplierUpdate for ConstFeeMultiplier { fn min() -> Multiplier { From b6b6e4a997c4006a28b4249a63be871877b9486d Mon Sep 17 00:00:00 2001 From: Szegoo Date: Wed, 21 Sep 2022 19:49:23 +0200 Subject: [PATCH 08/13] fixes --- bin/node-template/runtime/src/lib.rs | 3 +-- frame/transaction-payment/src/lib.rs | 9 ++++----- 2 files changed, 5 insertions(+), 7 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 2e63f2320c673..9eecf9046dd7b 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -255,7 +255,6 @@ impl pallet_balances::Config for Runtime { parameter_types! { pub FeeMultiplierMin: Multiplier = Multiplier::one(); - pub FeeMultiplierMax: Multiplier = Bounded::max_value(); } impl pallet_transaction_payment::Config for Runtime { @@ -264,7 +263,7 @@ impl pallet_transaction_payment::Config for Runtime { type OperationalFeeMultiplier = ConstU8<5>; type WeightToFee = IdentityFee; type LengthToFee = IdentityFee; - type FeeMultiplierUpdate = ConstFeeMultiplier; + type FeeMultiplierUpdate = ConstFeeMultiplier; } impl pallet_sudo::Config for Runtime { diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 3452e34a303e4..3dbe0162b4bf4 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -237,14 +237,14 @@ where } /// A struct to make the fee multiplier a constant -pub struct ConstFeeMultiplier, X: Get>(sp_std::marker::PhantomData<(M, X)>); +pub struct ConstFeeMultiplier>(sp_std::marker::PhantomData); -impl, X: Get> MultiplierUpdate for ConstFeeMultiplier { +impl> MultiplierUpdate for ConstFeeMultiplier { fn min() -> Multiplier { M::get() } fn max() -> Multiplier { - X::get() + M::get() } fn target() -> Perquintill { Default::default() @@ -254,10 +254,9 @@ impl, X: Get> MultiplierUpdate for ConstFeeMultip } } -impl Convert for ConstFeeMultiplier +impl Convert for ConstFeeMultiplier where M: Get, - X: Get, { fn convert(_previous: Multiplier) -> Multiplier { Self::min() From db5cdeff075b0a2f1fd933d29e35847fed26db8a Mon Sep 17 00:00:00 2001 From: Szegoo Date: Wed, 21 Sep 2022 19:55:54 +0200 Subject: [PATCH 09/13] remove unused import --- bin/node-template/runtime/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index b8b6e20fd4ecb..7d98325ec2c0d 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -15,8 +15,7 @@ use sp_core::{crypto::KeyTypeId, OpaqueMetadata}; use sp_runtime::{ create_runtime_str, generic, impl_opaque_keys, traits::{ - AccountIdLookup, BlakeTwo256, Block as BlockT, Bounded, IdentifyAccount, NumberFor, One, - Verify, + AccountIdLookup, BlakeTwo256, Block as BlockT, IdentifyAccount, NumberFor, One, Verify, }, transaction_validity::{TransactionSource, TransactionValidity}, ApplyExtrinsicResult, MultiSignature, From b630fb18f410caae3b60bfa252c9dd38b258473c Mon Sep 17 00:00:00 2001 From: Sergej Sakac <73715684+Szegoo@users.noreply.github.com> Date: Thu, 6 Oct 2022 09:55:37 +0200 Subject: [PATCH 10/13] Update lib.rs --- frame/transaction-payment/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 941d09835e12c..599715d933786 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -131,7 +131,7 @@ pub struct TargetedFeeAdjustment(sp_std::marker::PhantomData<(T, /// Something that can convert the current multiplier to the next one. pub trait MultiplierUpdate: Convert { - /// Minimum multiplier. Any outcome of the `convert` function should be less than this. + /// Minimum multiplier. Any outcome of the `convert` function should be more than this. fn min() -> Multiplier; /// Maximum multiplier. Any outcome of the `convert` function should be less than this. fn max() -> Multiplier; From e651e5486f395ee16d90874217d96dcc9eeca0fc Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 6 Oct 2022 10:50:40 +0200 Subject: [PATCH 11/13] more readable --- frame/transaction-payment/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 599715d933786..3440acc246479 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -227,7 +227,9 @@ where if positive { let excess = first_term.saturating_add(second_term).saturating_mul(previous); - previous.saturating_add(excess).max(M::get()).min(X::get()) + let min_multiplier = M::get(); + let max_multiplier = X::get(); + previous.saturating_add(excess).max(min_multiplier).min(max_multiplier) } else { // Defensive-only: first_term > second_term. Safe subtraction. let negative = first_term.saturating_sub(second_term).saturating_mul(previous); From 39ba563090c9cf7f427d809e888e7b439adb3a21 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 6 Oct 2022 10:54:33 +0200 Subject: [PATCH 12/13] fix --- frame/transaction-payment/src/lib.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 3440acc246479..408afa5834282 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -191,6 +191,7 @@ where // we recover here in case of errors, because any value below this would be stale and can // never change. let min_multiplier = M::get(); + let max_multiplier = X::get(); let previous = previous.max(min_multiplier); let weights = T::BlockWeights::get(); @@ -227,8 +228,6 @@ where if positive { let excess = first_term.saturating_add(second_term).saturating_mul(previous); - let min_multiplier = M::get(); - let max_multiplier = X::get(); previous.saturating_add(excess).max(min_multiplier).min(max_multiplier) } else { // Defensive-only: first_term > second_term. Safe subtraction. From 3517f139cca2cf2c85a6ecab0d9d3550cdd002a5 Mon Sep 17 00:00:00 2001 From: Szegoo Date: Thu, 6 Oct 2022 11:37:06 +0200 Subject: [PATCH 13/13] fix nits --- bin/node-template/runtime/src/lib.rs | 4 ++-- frame/transaction-payment/src/lib.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bin/node-template/runtime/src/lib.rs b/bin/node-template/runtime/src/lib.rs index 7d98325ec2c0d..f801068b10fda 100644 --- a/bin/node-template/runtime/src/lib.rs +++ b/bin/node-template/runtime/src/lib.rs @@ -253,7 +253,7 @@ impl pallet_balances::Config for Runtime { } parameter_types! { - pub FeeMultiplierMin: Multiplier = Multiplier::one(); + pub FeeMultiplier: Multiplier = Multiplier::one(); } impl pallet_transaction_payment::Config for Runtime { @@ -262,7 +262,7 @@ impl pallet_transaction_payment::Config for Runtime { type OperationalFeeMultiplier = ConstU8<5>; type WeightToFee = IdentityFee; type LengthToFee = IdentityFee; - type FeeMultiplierUpdate = ConstFeeMultiplier; + type FeeMultiplierUpdate = ConstFeeMultiplier; } impl pallet_sudo::Config for Runtime { diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 408afa5834282..b71f2bead1544 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -131,9 +131,9 @@ pub struct TargetedFeeAdjustment(sp_std::marker::PhantomData<(T, /// Something that can convert the current multiplier to the next one. pub trait MultiplierUpdate: Convert { - /// Minimum multiplier. Any outcome of the `convert` function should be more than this. + /// Minimum multiplier. Any outcome of the `convert` function should be at least this. fn min() -> Multiplier; - /// Maximum multiplier. Any outcome of the `convert` function should be less than this. + /// Maximum multiplier. Any outcome of the `convert` function should be less or equal this. fn max() -> Multiplier; /// Target block saturation level fn target() -> Perquintill; @@ -232,7 +232,7 @@ where } else { // Defensive-only: first_term > second_term. Safe subtraction. let negative = first_term.saturating_sub(second_term).saturating_mul(previous); - previous.saturating_sub(negative).max(min_multiplier) + previous.saturating_sub(negative).max(min_multiplier).min(max_multiplier) } } }