Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Rework Transaction Priority calculation #9834

Merged
19 commits merged into from
Oct 4, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 2 additions & 0 deletions bin/node-template/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ();
}
Expand Down
2 changes: 2 additions & 0 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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>;
Expand Down
2 changes: 2 additions & 0 deletions frame/balances/src/tests_composite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ();
}
Expand Down
2 changes: 2 additions & 0 deletions frame/balances/src/tests_local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ();
}
Expand Down
2 changes: 2 additions & 0 deletions frame/balances/src/tests_reentrancy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ();
}
Expand Down
6 changes: 3 additions & 3 deletions frame/executive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 = ();
}
Expand Down Expand Up @@ -1110,16 +1112,14 @@ 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(
TransactionSource::InBlock,
valid.clone(),
Default::default(),
),
Ok(default_with_prio_3),
Ok(ValidTransaction::default()),
);
assert_eq!(
Executive::validate_transaction(
Expand Down
24 changes: 0 additions & 24 deletions frame/support/src/weights.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
5 changes: 5 additions & 0 deletions frame/system/src/extensions/check_genesis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>);
Expand Down
4 changes: 4 additions & 0 deletions frame/system/src/extensions/check_mortality.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>);
Expand Down
7 changes: 5 additions & 2 deletions frame/system/src/extensions/check_nonce.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
5 changes: 5 additions & 0 deletions frame/system/src/extensions/check_spec_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>);
Expand Down
5 changes: 5 additions & 0 deletions frame/system/src/extensions/check_tx_version.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>);
Expand Down
63 changes: 9 additions & 54 deletions frame/system/src/extensions/check_weight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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>);
Expand Down Expand Up @@ -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())
Expand Down Expand Up @@ -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())
}
}

Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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(|| {
Expand Down
Loading