From 374fb6a9211e166d5276b4722b56124d07e897fe Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Tomasz=20Drwi=C4=99ga?= <tomusdrw@users.noreply.github.com>
Date: Mon, 4 Oct 2021 16:25:18 +0200
Subject: [PATCH] Rework Transaction Priority calculation (#9834)
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit
* Add transaction validity docs.
* Re-work priority calculation.
* Fix tests.
* Update frame/transaction-payment/src/lib.rs
Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>
* cargo +nightly fmt --all
* Fix an obvious mistake :)
* Re-work again.
* Fix test.
* cargo +nightly fmt --all
* Make VirtualTip dependent on the transaction size.
* cargo +nightly fmt --all
* Update frame/transaction-payment/src/lib.rs
Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>
* Fix compilation.
* Update bin/node/runtime/src/lib.rs
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
Co-authored-by: Alexander Popiak <alexander.popiak@parity.io>
Co-authored-by: Bastian Köcher <bkchr@users.noreply.github.com>
---
substrate/Cargo.lock | 8 +-
.../bin/node-template/runtime/src/lib.rs | 2 +
substrate/bin/node/runtime/src/lib.rs | 2 +
.../frame/balances/src/tests_composite.rs | 2 +
substrate/frame/balances/src/tests_local.rs | 2 +
.../frame/balances/src/tests_reentrancy.rs | 2 +
substrate/frame/executive/src/lib.rs | 6 +-
substrate/frame/support/src/weights.rs | 24 ---
.../system/src/extensions/check_genesis.rs | 5 +
.../system/src/extensions/check_mortality.rs | 4 +
.../system/src/extensions/check_nonce.rs | 7 +-
.../src/extensions/check_spec_version.rs | 5 +
.../system/src/extensions/check_tx_version.rs | 5 +
.../system/src/extensions/check_weight.rs | 63 +-----
.../frame/transaction-payment/src/lib.rs | 193 ++++++++++++++++--
15 files changed, 225 insertions(+), 105 deletions(-)
diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock
index 0e3f3399dbf..5e8932fd565 100644
--- a/substrate/Cargo.lock
+++ b/substrate/Cargo.lock
@@ -2473,9 +2473,9 @@ dependencies = [
[[package]]
name = "git2"
-version = "0.13.21"
+version = "0.13.22"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "659cd14835e75b64d9dba5b660463506763cf0aa6cb640aeeb0e98d841093490"
+checksum = "9c1cbbfc9a1996c6af82c2b4caf828d2c653af4fcdbb0e5674cc966eee5a4197"
dependencies = [
"bitflags",
"libc",
@@ -3294,9 +3294,9 @@ checksum = "789da6d93f1b866ffe175afc5322a4d76c038605a1c3319bb57b06967ca98a36"
[[package]]
name = "libgit2-sys"
-version = "0.12.22+1.1.0"
+version = "0.12.23+1.2.0"
source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "89c53ac117c44f7042ad8d8f5681378dfbc6010e49ec2c0d1f11dfedc7a4a1c3"
+checksum = "29730a445bae719db3107078b46808cc45a5b7a6bae3f31272923af969453356"
dependencies = [
"cc",
"libc",
diff --git a/substrate/bin/node-template/runtime/src/lib.rs b/substrate/bin/node-template/runtime/src/lib.rs
index ca6e6b1822d..5d3ea603265 100644
--- a/substrate/bin/node-template/runtime/src/lib.rs
+++ b/substrate/bin/node-template/runtime/src/lib.rs
@@ -258,11 +258,13 @@ impl pallet_balances::Config for Runtime {
parameter_types! {
pub const TransactionByteFee: Balance = 1;
+ pub OperationalFeeMultiplier: u8 = 5;
}
impl pallet_transaction_payment::Config for Runtime {
type OnChargeTransaction = CurrencyAdapter<Balances, ()>;
type TransactionByteFee = TransactionByteFee;
+ type OperationalFeeMultiplier = OperationalFeeMultiplier;
type WeightToFee = IdentityFee<Balance>;
type FeeMultiplierUpdate = ();
}
diff --git a/substrate/bin/node/runtime/src/lib.rs b/substrate/bin/node/runtime/src/lib.rs
index 881641e1513..f2d3175f094 100644
--- a/substrate/bin/node/runtime/src/lib.rs
+++ b/substrate/bin/node/runtime/src/lib.rs
@@ -413,6 +413,7 @@ impl pallet_balances::Config for Runtime {
parameter_types! {
pub const TransactionByteFee: Balance = 10 * MILLICENTS;
+ pub const OperationalFeeMultiplier: u8 = 5;
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);
@@ -421,6 +422,7 @@ parameter_types! {
impl pallet_transaction_payment::Config for Runtime {
type OnChargeTransaction = CurrencyAdapter<Balances, DealWithFees>;
type TransactionByteFee = TransactionByteFee;
+ type OperationalFeeMultiplier = OperationalFeeMultiplier;
type WeightToFee = IdentityFee<Balance>;
type FeeMultiplierUpdate =
TargetedFeeAdjustment<Self, TargetBlockFullness, AdjustmentVariable, MinimumMultiplier>;
diff --git a/substrate/frame/balances/src/tests_composite.rs b/substrate/frame/balances/src/tests_composite.rs
index f6faebed393..60feedb326d 100644
--- a/substrate/frame/balances/src/tests_composite.rs
+++ b/substrate/frame/balances/src/tests_composite.rs
@@ -76,10 +76,12 @@ impl frame_system::Config for Test {
}
parameter_types! {
pub const TransactionByteFee: u64 = 1;
+ pub const OperationalFeeMultiplier: u8 = 5;
}
impl pallet_transaction_payment::Config for Test {
type OnChargeTransaction = CurrencyAdapter<Pallet<Test>, ()>;
type TransactionByteFee = TransactionByteFee;
+ type OperationalFeeMultiplier = OperationalFeeMultiplier;
type WeightToFee = IdentityFee<u64>;
type FeeMultiplierUpdate = ();
}
diff --git a/substrate/frame/balances/src/tests_local.rs b/substrate/frame/balances/src/tests_local.rs
index d8c07aa9c42..1d758ce4e98 100644
--- a/substrate/frame/balances/src/tests_local.rs
+++ b/substrate/frame/balances/src/tests_local.rs
@@ -78,10 +78,12 @@ impl frame_system::Config for Test {
}
parameter_types! {
pub const TransactionByteFee: u64 = 1;
+ pub const OperationalFeeMultiplier: u8 = 5;
}
impl pallet_transaction_payment::Config for Test {
type OnChargeTransaction = CurrencyAdapter<Pallet<Test>, ()>;
type TransactionByteFee = TransactionByteFee;
+ type OperationalFeeMultiplier = OperationalFeeMultiplier;
type WeightToFee = IdentityFee<u64>;
type FeeMultiplierUpdate = ();
}
diff --git a/substrate/frame/balances/src/tests_reentrancy.rs b/substrate/frame/balances/src/tests_reentrancy.rs
index 9c7ba3e1ec8..25b8fb34f20 100644
--- a/substrate/frame/balances/src/tests_reentrancy.rs
+++ b/substrate/frame/balances/src/tests_reentrancy.rs
@@ -80,10 +80,12 @@ impl frame_system::Config for Test {
}
parameter_types! {
pub const TransactionByteFee: u64 = 1;
+ pub const OperationalFeeMultiplier: u8 = 5;
}
impl pallet_transaction_payment::Config for Test {
type OnChargeTransaction = CurrencyAdapter<Pallet<Test>, ()>;
type TransactionByteFee = TransactionByteFee;
+ type OperationalFeeMultiplier = OperationalFeeMultiplier;
type WeightToFee = IdentityFee<u64>;
type FeeMultiplierUpdate = ();
}
diff --git a/substrate/frame/executive/src/lib.rs b/substrate/frame/executive/src/lib.rs
index 41f679909e6..b1bdf357ec0 100644
--- a/substrate/frame/executive/src/lib.rs
+++ b/substrate/frame/executive/src/lib.rs
@@ -800,10 +800,12 @@ mod tests {
parameter_types! {
pub const TransactionByteFee: Balance = 0;
+ pub const OperationalFeeMultiplier: u8 = 5;
}
impl pallet_transaction_payment::Config for Runtime {
type OnChargeTransaction = CurrencyAdapter<Balances, ()>;
type TransactionByteFee = TransactionByteFee;
+ type OperationalFeeMultiplier = OperationalFeeMultiplier;
type WeightToFee = IdentityFee<Balance>;
type FeeMultiplierUpdate = ();
}
@@ -1110,8 +1112,6 @@ mod tests {
let invalid = TestXt::new(Call::Custom(custom::Call::unallowed_unsigned {}), None);
let mut t = new_test_ext(1);
- let mut default_with_prio_3 = ValidTransaction::default();
- default_with_prio_3.priority = 3;
t.execute_with(|| {
assert_eq!(
Executive::validate_transaction(
@@ -1119,7 +1119,7 @@ mod tests {
valid.clone(),
Default::default(),
),
- Ok(default_with_prio_3),
+ Ok(ValidTransaction::default()),
);
assert_eq!(
Executive::validate_transaction(
diff --git a/substrate/frame/support/src/weights.rs b/substrate/frame/support/src/weights.rs
index 115470a9bf0..ec5f37823ad 100644
--- a/substrate/frame/support/src/weights.rs
+++ b/substrate/frame/support/src/weights.rs
@@ -287,30 +287,6 @@ impl<'a> OneOrMany<DispatchClass> for &'a [DispatchClass] {
}
}
-/// Primitives related to priority management of Frame.
-pub mod priority {
- /// The starting point of all Operational transactions. 3/4 of u64::MAX.
- pub const LIMIT: u64 = 13_835_058_055_282_163_711_u64;
-
- /// Wrapper for priority of different dispatch classes.
- ///
- /// This only makes sure that any value created for the operational dispatch class is
- /// incremented by [`LIMIT`].
- pub enum FrameTransactionPriority {
- Normal(u64),
- Operational(u64),
- }
-
- impl From<FrameTransactionPriority> for u64 {
- fn from(priority: FrameTransactionPriority) -> Self {
- match priority {
- FrameTransactionPriority::Normal(inner) => inner,
- FrameTransactionPriority::Operational(inner) => inner.saturating_add(LIMIT),
- }
- }
- }
-}
-
/// A bundle of static information collected from the `#[weight = $x]` attributes.
#[derive(Clone, Copy, Eq, PartialEq, Default, RuntimeDebug, Encode, Decode, TypeInfo)]
pub struct DispatchInfo {
diff --git a/substrate/frame/system/src/extensions/check_genesis.rs b/substrate/frame/system/src/extensions/check_genesis.rs
index 6f409d5d3d4..9c5c890ee60 100644
--- a/substrate/frame/system/src/extensions/check_genesis.rs
+++ b/substrate/frame/system/src/extensions/check_genesis.rs
@@ -24,6 +24,11 @@ use sp_runtime::{
};
/// Genesis hash check to provide replay protection between different networks.
+///
+/// # Transaction Validity
+///
+/// Note that while a transaction with invalid `genesis_hash` will fail to be decoded,
+/// the extension does not affect any other fields of `TransactionValidity` directly.
#[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)]
#[scale_info(skip_type_params(T))]
pub struct CheckGenesis<T: Config + Send + Sync>(sp_std::marker::PhantomData<T>);
diff --git a/substrate/frame/system/src/extensions/check_mortality.rs b/substrate/frame/system/src/extensions/check_mortality.rs
index 69cca765efe..941f28dc6fc 100644
--- a/substrate/frame/system/src/extensions/check_mortality.rs
+++ b/substrate/frame/system/src/extensions/check_mortality.rs
@@ -27,6 +27,10 @@ use sp_runtime::{
};
/// Check for transaction mortality.
+///
+/// # Transaction Validity
+///
+/// The extension affects `longevity` of the transaction according to the [`Era`] definition.
#[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)]
#[scale_info(skip_type_params(T))]
pub struct CheckMortality<T: Config + Send + Sync>(Era, sp_std::marker::PhantomData<T>);
diff --git a/substrate/frame/system/src/extensions/check_nonce.rs b/substrate/frame/system/src/extensions/check_nonce.rs
index 74be8339842..3c6f9a1b4db 100644
--- a/substrate/frame/system/src/extensions/check_nonce.rs
+++ b/substrate/frame/system/src/extensions/check_nonce.rs
@@ -30,8 +30,11 @@ use sp_std::vec;
/// Nonce check and increment to give replay protection for transactions.
///
-/// Note that this does not set any priority by default. Make sure that AT LEAST one of the signed
-/// extension sets some kind of priority upon validating transactions.
+/// # Transaction Validity
+///
+/// This extension affects `requires` and `provides` tags of validity, but DOES NOT
+/// set the `priority` field. Make sure that AT LEAST one of the signed extension sets
+/// some kind of priority upon validating transactions.
#[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)]
#[scale_info(skip_type_params(T))]
pub struct CheckNonce<T: Config>(#[codec(compact)] pub T::Index);
diff --git a/substrate/frame/system/src/extensions/check_spec_version.rs b/substrate/frame/system/src/extensions/check_spec_version.rs
index 0217aefae6b..688abe99763 100644
--- a/substrate/frame/system/src/extensions/check_spec_version.rs
+++ b/substrate/frame/system/src/extensions/check_spec_version.rs
@@ -21,6 +21,11 @@ use scale_info::TypeInfo;
use sp_runtime::{traits::SignedExtension, transaction_validity::TransactionValidityError};
/// Ensure the runtime version registered in the transaction is the same as at present.
+///
+/// # Transaction Validity
+///
+/// The transaction with incorrect `spec_version` are considered invalid. The validity
+/// is not affected in any other way.
#[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)]
#[scale_info(skip_type_params(T))]
pub struct CheckSpecVersion<T: Config + Send + Sync>(sp_std::marker::PhantomData<T>);
diff --git a/substrate/frame/system/src/extensions/check_tx_version.rs b/substrate/frame/system/src/extensions/check_tx_version.rs
index 9418d3ff5d9..f6bb53e1cba 100644
--- a/substrate/frame/system/src/extensions/check_tx_version.rs
+++ b/substrate/frame/system/src/extensions/check_tx_version.rs
@@ -21,6 +21,11 @@ use scale_info::TypeInfo;
use sp_runtime::{traits::SignedExtension, transaction_validity::TransactionValidityError};
/// Ensure the transaction version registered in the transaction is the same as at present.
+///
+/// # Transaction Validity
+///
+/// The transaction with incorrect `transaction_version` are considered invalid. The validity
+/// is not affected in any other way.
#[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)]
#[scale_info(skip_type_params(T))]
pub struct CheckTxVersion<T: Config + Send + Sync>(sp_std::marker::PhantomData<T>);
diff --git a/substrate/frame/system/src/extensions/check_weight.rs b/substrate/frame/system/src/extensions/check_weight.rs
index 92dc7382fa2..ca885accd66 100644
--- a/substrate/frame/system/src/extensions/check_weight.rs
+++ b/substrate/frame/system/src/extensions/check_weight.rs
@@ -19,19 +19,21 @@ use crate::{limits::BlockWeights, Config, Pallet};
use codec::{Decode, Encode};
use frame_support::{
traits::Get,
- weights::{priority::FrameTransactionPriority, DispatchClass, DispatchInfo, PostDispatchInfo},
+ weights::{DispatchClass, DispatchInfo, PostDispatchInfo},
};
use scale_info::TypeInfo;
use sp_runtime::{
traits::{DispatchInfoOf, Dispatchable, PostDispatchInfoOf, SignedExtension},
- transaction_validity::{
- InvalidTransaction, TransactionPriority, TransactionValidity, TransactionValidityError,
- ValidTransaction,
- },
+ transaction_validity::{InvalidTransaction, TransactionValidity, TransactionValidityError},
DispatchResult,
};
/// Block resource (weight) limit check.
+///
+/// # Transaction Validity
+///
+/// This extension does not influence any fields of `TransactionValidity` in case the
+/// transaction is valid.
#[derive(Encode, Decode, Clone, Eq, PartialEq, Default, TypeInfo)]
#[scale_info(skip_type_params(T))]
pub struct CheckWeight<T: Config + Send + Sync>(sp_std::marker::PhantomData<T>);
@@ -81,23 +83,6 @@ where
}
}
- /// Get the priority of an extrinsic denoted by `info`.
- ///
- /// Operational transaction will be given a fixed initial amount to be fairly distinguished from
- /// the normal ones.
- fn get_priority(info: &DispatchInfoOf<T::Call>) -> TransactionPriority {
- match info.class {
- // Normal transaction.
- DispatchClass::Normal => FrameTransactionPriority::Normal(info.weight.into()).into(),
- // Don't use up the whole priority space, to allow things like `tip` to be taken into
- // account as well.
- DispatchClass::Operational =>
- FrameTransactionPriority::Operational(info.weight.into()).into(),
- // Mandatory extrinsics are only for inherents; never transactions.
- DispatchClass::Mandatory => TransactionPriority::min_value(),
- }
- }
-
/// Creates new `SignedExtension` to check weight of the extrinsic.
pub fn new() -> Self {
Self(Default::default())
@@ -130,7 +115,7 @@ where
// consumption from causing false negatives.
Self::check_extrinsic_weight(info)?;
- Ok(ValidTransaction { priority: Self::get_priority(info), ..Default::default() })
+ Ok(Default::default())
}
}
@@ -368,13 +353,7 @@ mod tests {
};
let len = 0_usize;
- assert_eq!(
- CheckWeight::<Test>::do_validate(&okay, len),
- Ok(ValidTransaction {
- priority: CheckWeight::<Test>::get_priority(&okay),
- ..Default::default()
- })
- );
+ assert_eq!(CheckWeight::<Test>::do_validate(&okay, len), Ok(Default::default()));
assert_err!(
CheckWeight::<Test>::do_validate(&max, len),
InvalidTransaction::ExhaustsResources
@@ -506,30 +485,6 @@ mod tests {
})
}
- #[test]
- fn signed_ext_check_weight_works() {
- new_test_ext().execute_with(|| {
- let normal =
- DispatchInfo { weight: 100, class: DispatchClass::Normal, pays_fee: Pays::Yes };
- let op = DispatchInfo {
- weight: 100,
- class: DispatchClass::Operational,
- pays_fee: Pays::Yes,
- };
- let len = 0_usize;
-
- let priority = CheckWeight::<Test>(PhantomData)
- .validate(&1, CALL, &normal, len)
- .unwrap()
- .priority;
- assert_eq!(priority, 100);
-
- let priority =
- CheckWeight::<Test>(PhantomData).validate(&1, CALL, &op, len).unwrap().priority;
- assert_eq!(priority, frame_support::weights::priority::LIMIT + 100);
- })
- }
-
#[test]
fn signed_ext_check_weight_block_size_works() {
new_test_ext().execute_with(|| {
diff --git a/substrate/frame/transaction-payment/src/lib.rs b/substrate/frame/transaction-payment/src/lib.rs
index 11dbcc010f6..28200bee705 100644
--- a/substrate/frame/transaction-payment/src/lib.rs
+++ b/substrate/frame/transaction-payment/src/lib.rs
@@ -264,6 +264,30 @@ pub mod pallet {
#[pallet::constant]
type TransactionByteFee: Get<BalanceOf<Self>>;
+ /// A fee mulitplier for `Operational` extrinsics to compute "virtual tip" to boost their
+ /// `priority`
+ ///
+ /// This value is multipled by the `final_fee` to obtain a "virtual tip" that is later
+ /// added to a tip component in regular `priority` calculations.
+ /// It means that a `Normal` transaction can front-run a similarly-sized `Operational`
+ /// extrinsic (with no tip), by including a tip value greater than the virtual tip.
+ ///
+ /// ```rust,ignore
+ /// // For `Normal`
+ /// let priority = priority_calc(tip);
+ ///
+ /// // For `Operational`
+ /// let virtual_tip = (inclusion_fee + tip) * OperationalFeeMultiplier;
+ /// let priority = priority_calc(tip + virtual_tip);
+ /// ```
+ ///
+ /// Note that since we use `final_fee` the multiplier applies also to the regular `tip`
+ /// sent with the transaction. So, not only does the transaction get a priority bump based
+ /// on the `inclusion_fee`, but we also amplify the impact of tips applied to `Operational`
+ /// transactions.
+ #[pallet::constant]
+ type OperationalFeeMultiplier: Get<u8>;
+
/// Convert a weight value into a deductible fee based on the currency type.
type WeightToFee: WeightToFeePolynomial<Balance = BalanceOf<Self>>;
@@ -525,6 +549,14 @@ where
/// Require the transactor pay for themselves and maybe include a tip to gain additional priority
/// in the queue.
+///
+/// # Transaction Validity
+///
+/// This extension sets the `priority` field of `TransactionValidity` depending on the amount
+/// of tip being paid per weight unit.
+///
+/// Operational transactions will receive an additional priority bump, so that they are normally
+/// considered before regular transactions.
#[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)]
#[scale_info(skip_type_params(T))]
pub struct ChargeTransactionPayment<T: Config>(#[codec(compact)] BalanceOf<T>);
@@ -566,27 +598,73 @@ where
.map(|i| (fee, i))
}
- /// Get an appropriate priority for a transaction with the given length and info.
+ /// Get an appropriate priority for a transaction with the given `DispatchInfo`, encoded length
+ /// and user-included tip.
///
- /// This will try and optimise the `fee/weight` `fee/length`, whichever is consuming more of the
- /// maximum corresponding limit.
+ /// The priority is based on the amount of `tip` the user is willing to pay per unit of either
+ /// `weight` or `length`, depending which one is more limitting. For `Operational` extrinsics
+ /// we add a "virtual tip" to the calculations.
///
- /// For example, if a transaction consumed 1/4th of the block length and half of the weight, its
- /// final priority is `fee * min(2, 4) = fee * 2`. If it consumed `1/4th` of the block length
- /// and the entire block weight `(1/1)`, its priority is `fee * min(1, 4) = fee * 1`. This means
- /// that the transaction which consumes more resources (either length or weight) with the same
- /// `fee` ends up having lower priority.
- fn get_priority(
- len: usize,
+ /// The formula should simply be `tip / bounded_{weight|length}`, but since we are using
+ /// integer division, we have no guarantees it's going to give results in any reasonable
+ /// range (might simply end up being zero). Hence we use a scaling factor:
+ /// `tip * (max_block_{weight|length} / bounded_{weight|length})`, since given current
+ /// state of-the-art blockchains, number of per-block transactions is expected to be in a
+ /// range reasonable enough to not saturate the `Balance` type while multiplying by the tip.
+ pub fn get_priority(
info: &DispatchInfoOf<T::Call>,
+ len: usize,
+ tip: BalanceOf<T>,
final_fee: BalanceOf<T>,
) -> TransactionPriority {
- let weight_saturation = T::BlockWeights::get().max_block / info.weight.max(1);
- let max_block_length = *T::BlockLength::get().max.get(DispatchClass::Normal);
- let len_saturation = max_block_length as u64 / (len as u64).max(1);
- let coefficient: BalanceOf<T> =
- weight_saturation.min(len_saturation).saturated_into::<BalanceOf<T>>();
- final_fee.saturating_mul(coefficient).saturated_into::<TransactionPriority>()
+ // Calculate how many such extrinsics we could fit into an empty block and take
+ // the limitting factor.
+ let max_block_weight = T::BlockWeights::get().max_block;
+ let max_block_length = *T::BlockLength::get().max.get(info.class) as u64;
+
+ let bounded_weight = info.weight.max(1).min(max_block_weight);
+ let bounded_length = (len as u64).max(1).min(max_block_length);
+
+ let max_tx_per_block_weight = max_block_weight / bounded_weight;
+ let max_tx_per_block_length = max_block_length / bounded_length;
+ // Given our current knowledge this value is going to be in a reasonable range - i.e.
+ // less than 10^9 (2^30), so multiplying by the `tip` value is unlikely to overflow the
+ // balance type. We still use saturating ops obviously, but the point is to end up with some
+ // `priority` distribution instead of having all transactions saturate the priority.
+ let max_tx_per_block = max_tx_per_block_length
+ .min(max_tx_per_block_weight)
+ .saturated_into::<BalanceOf<T>>();
+ let max_reward = |val: BalanceOf<T>| val.saturating_mul(max_tx_per_block);
+
+ // To distribute no-tip transactions a little bit, we set the minimal tip as `1`.
+ // This means that given two transactions without a tip, smaller one will be preferred.
+ let tip = tip.max(1.saturated_into());
+ let scaled_tip = max_reward(tip);
+
+ match info.class {
+ DispatchClass::Normal => {
+ // For normal class we simply take the `tip_per_weight`.
+ scaled_tip
+ },
+ DispatchClass::Mandatory => {
+ // Mandatory extrinsics should be prohibited (e.g. by the [`CheckWeight`]
+ // extensions), but just to be safe let's return the same priority as `Normal` here.
+ scaled_tip
+ },
+ DispatchClass::Operational => {
+ // A "virtual tip" value added to an `Operational` extrinsic.
+ // This value should be kept high enough to allow `Operational` extrinsics
+ // to get in even during congestion period, but at the same time low
+ // enough to prevent a possible spam attack by sending invalid operational
+ // extrinsics which push away regular transactions from the pool.
+ let fee_multiplier = T::OperationalFeeMultiplier::get().saturated_into();
+ let virtual_tip = final_fee.saturating_mul(fee_multiplier);
+ let scaled_virtual_tip = max_reward(virtual_tip);
+
+ scaled_tip.saturating_add(scaled_virtual_tip)
+ },
+ }
+ .saturated_into::<TransactionPriority>()
}
}
@@ -629,8 +707,12 @@ where
info: &DispatchInfoOf<Self::Call>,
len: usize,
) -> TransactionValidity {
- let (fee, _) = self.withdraw_fee(who, call, info, len)?;
- Ok(ValidTransaction { priority: Self::get_priority(len, info, fee), ..Default::default() })
+ let (final_fee, _) = self.withdraw_fee(who, call, info, len)?;
+ let tip = self.0;
+ Ok(ValidTransaction {
+ priority: Self::get_priority(info, len, tip, final_fee),
+ ..Default::default()
+ })
}
fn pre_dispatch(
@@ -743,6 +825,7 @@ mod tests {
pub const BlockHashCount: u64 = 250;
pub static TransactionByteFee: u64 = 1;
pub static WeightToFee: u64 = 1;
+ pub static OperationalFeeMultiplier: u8 = 5;
}
impl frame_system::Config for Runtime {
@@ -822,6 +905,7 @@ mod tests {
impl Config for Runtime {
type OnChargeTransaction = CurrencyAdapter<Balances, DealWithFees>;
type TransactionByteFee = TransactionByteFee;
+ type OperationalFeeMultiplier = OperationalFeeMultiplier;
type WeightToFee = WeightToFee;
type FeeMultiplierUpdate = ();
}
@@ -1335,6 +1419,79 @@ mod tests {
});
}
+ #[test]
+ fn should_alter_operational_priority() {
+ let tip = 5;
+ let len = 10;
+
+ ExtBuilder::default().balance_factor(100).build().execute_with(|| {
+ let normal =
+ DispatchInfo { weight: 100, class: DispatchClass::Normal, pays_fee: Pays::Yes };
+ let priority = ChargeTransactionPayment::<Runtime>(tip)
+ .validate(&2, CALL, &normal, len)
+ .unwrap()
+ .priority;
+
+ assert_eq!(priority, 50);
+
+ let priority = ChargeTransactionPayment::<Runtime>(2 * tip)
+ .validate(&2, CALL, &normal, len)
+ .unwrap()
+ .priority;
+
+ assert_eq!(priority, 100);
+ });
+
+ ExtBuilder::default().balance_factor(100).build().execute_with(|| {
+ let op = DispatchInfo {
+ weight: 100,
+ class: DispatchClass::Operational,
+ pays_fee: Pays::Yes,
+ };
+ let priority = ChargeTransactionPayment::<Runtime>(tip)
+ .validate(&2, CALL, &op, len)
+ .unwrap()
+ .priority;
+ assert_eq!(priority, 5800);
+
+ let priority = ChargeTransactionPayment::<Runtime>(2 * tip)
+ .validate(&2, CALL, &op, len)
+ .unwrap()
+ .priority;
+ assert_eq!(priority, 6100);
+ });
+ }
+
+ #[test]
+ fn no_tip_has_some_priority() {
+ let tip = 0;
+ let len = 10;
+
+ ExtBuilder::default().balance_factor(100).build().execute_with(|| {
+ let normal =
+ DispatchInfo { weight: 100, class: DispatchClass::Normal, pays_fee: Pays::Yes };
+ let priority = ChargeTransactionPayment::<Runtime>(tip)
+ .validate(&2, CALL, &normal, len)
+ .unwrap()
+ .priority;
+
+ assert_eq!(priority, 10);
+ });
+
+ ExtBuilder::default().balance_factor(100).build().execute_with(|| {
+ let op = DispatchInfo {
+ weight: 100,
+ class: DispatchClass::Operational,
+ pays_fee: Pays::Yes,
+ };
+ let priority = ChargeTransactionPayment::<Runtime>(tip)
+ .validate(&2, CALL, &op, len)
+ .unwrap()
+ .priority;
+ assert_eq!(priority, 5510);
+ });
+ }
+
#[test]
fn post_info_can_change_pays_fee() {
ExtBuilder::default()
--
GitLab