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 5 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 @@ -257,11 +257,13 @@ impl pallet_balances::Config for Runtime {

parameter_types! {
pub const TransactionByteFee: Balance = 1;
pub WeightTipRatio: Balance = BlockWeights::get().max_block as Balance / ExistentialDeposit::get();
}

impl pallet_transaction_payment::Config for Runtime {
type OnChargeTransaction = CurrencyAdapter<Balances, ()>;
type TransactionByteFee = TransactionByteFee;
type WeightTipRatio = WeightTipRatio;
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 @@ -412,6 +412,7 @@ impl pallet_balances::Config for Runtime {

parameter_types! {
pub const TransactionByteFee: Balance = 10 * MILLICENTS;
pub const WeightTipRatio: Balance = MAXIMUM_BLOCK_WEIGHT as Balance / 1 * DOLLARS;
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 @@ -420,6 +421,7 @@ parameter_types! {
impl pallet_transaction_payment::Config for Runtime {
type OnChargeTransaction = CurrencyAdapter<Balances, DealWithFees>;
type TransactionByteFee = TransactionByteFee;
type WeightTipRatio = WeightTipRatio;
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 WeightTipRatio: u64 = 1;
}
impl pallet_transaction_payment::Config for Test {
type OnChargeTransaction = CurrencyAdapter<Pallet<Test>, ()>;
type TransactionByteFee = TransactionByteFee;
type WeightTipRatio = WeightTipRatio;
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 WeightTipRatio: u64 = 1;
}
impl pallet_transaction_payment::Config for Test {
type OnChargeTransaction = CurrencyAdapter<Pallet<Test>, ()>;
type TransactionByteFee = TransactionByteFee;
type WeightTipRatio = WeightTipRatio;
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 WeightTipRatio: u64 = 1;
}
impl pallet_transaction_payment::Config for Test {
type OnChargeTransaction = CurrencyAdapter<Pallet<Test>, ()>;
type TransactionByteFee = TransactionByteFee;
type WeightTipRatio = WeightTipRatio;
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 @@ -775,10 +775,12 @@ mod tests {

parameter_types! {
pub const TransactionByteFee: Balance = 0;
pub const WeightTipRatio: Balance = 1;
}
impl pallet_transaction_payment::Config for Runtime {
type OnChargeTransaction = CurrencyAdapter<Balances, ()>;
type TransactionByteFee = TransactionByteFee;
type WeightTipRatio = WeightTipRatio;
type WeightToFee = IdentityFee<Balance>;
type FeeMultiplierUpdate = ();
}
Expand Down Expand Up @@ -1085,16 +1087,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
58 changes: 9 additions & 49 deletions frame/system/src/extensions/check_weight.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,19 +19,23 @@ 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,
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 +85,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 +117,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 @@ -370,10 +357,7 @@ mod tests {

assert_eq!(
CheckWeight::<Test>::do_validate(&okay, len),
Ok(ValidTransaction {
priority: CheckWeight::<Test>::get_priority(&okay),
..Default::default()
})
Ok(Default::default())
);
assert_err!(
CheckWeight::<Test>::do_validate(&max, len),
Expand Down Expand Up @@ -506,30 +490,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