diff --git a/frame/support/src/weights.rs b/frame/support/src/weights.rs index db1e25ca7ab2e..1d19eeef70d79 100644 --- a/frame/support/src/weights.rs +++ b/frame/support/src/weights.rs @@ -242,6 +242,30 @@ impl Default for DispatchClass { } } +/// Primitives related to priority management of Frame. +pub mod priority { + /// The starting point of all Operational transactions. 3/4 of u64::max_value(). + 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 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)] pub struct DispatchInfo { diff --git a/frame/system/src/extensions/check_nonce.rs b/frame/system/src/extensions/check_nonce.rs index 1af3a1210aaf4..e7316457aaffc 100644 --- a/frame/system/src/extensions/check_nonce.rs +++ b/frame/system/src/extensions/check_nonce.rs @@ -25,12 +25,15 @@ use sp_runtime::{ traits::{SignedExtension, DispatchInfoOf, Dispatchable, One}, transaction_validity::{ ValidTransaction, TransactionValidityError, InvalidTransaction, TransactionValidity, - TransactionLongevity, TransactionPriority, + TransactionLongevity, }, }; 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. #[derive(Encode, Decode, Clone, Eq, PartialEq)] pub struct CheckNonce(#[codec(compact)] T::Index); @@ -90,7 +93,7 @@ impl SignedExtension for CheckNonce where &self, who: &Self::AccountId, _call: &Self::Call, - info: &DispatchInfoOf, + _info: &DispatchInfoOf, _len: usize, ) -> TransactionValidity { // check index @@ -107,7 +110,7 @@ impl SignedExtension for CheckNonce where }; Ok(ValidTransaction { - priority: info.weight as TransactionPriority, + priority: 0, requires, provides, longevity: TransactionLongevity::max_value(), diff --git a/frame/system/src/extensions/check_weight.rs b/frame/system/src/extensions/check_weight.rs index 1395aa87efbcf..092ac59da97c8 100644 --- a/frame/system/src/extensions/check_weight.rs +++ b/frame/system/src/extensions/check_weight.rs @@ -27,7 +27,7 @@ use sp_runtime::{ }; use frame_support::{ traits::{Get}, - weights::{PostDispatchInfo, DispatchInfo, DispatchClass}, + weights::{PostDispatchInfo, DispatchInfo, DispatchClass, priority::FrameTransactionPriority}, StorageValue, }; @@ -157,12 +157,18 @@ impl CheckWeight 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) -> TransactionPriority { match info.class { - DispatchClass::Normal => info.weight.into(), - // Don't use up the whole priority space, to allow things like `tip` - // to be taken into account as well. - DispatchClass::Operational => TransactionPriority::max_value() / 2, + // 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(), } @@ -496,7 +502,7 @@ mod tests { } #[test] - fn signed_ext() { + 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 }; @@ -512,7 +518,7 @@ mod tests { .validate(&1, CALL, &op, len) .unwrap() .priority; - assert_eq!(priority, u64::max_value() / 2); + assert_eq!(priority, frame_support::weights::priority::LIMIT + 100); }) } diff --git a/frame/transaction-payment/src/lib.rs b/frame/transaction-payment/src/lib.rs index 244b4280ade08..4e4bc5311dacd 100644 --- a/frame/transaction-payment/src/lib.rs +++ b/frame/transaction-payment/src/lib.rs @@ -467,6 +467,23 @@ impl ChargeTransactionPayment where Err(_) => Err(InvalidTransaction::Payment.into()), } } + + /// Get an appropriate priority for a transaction with the given length and info. + /// + /// This will try and optimise the `fee/weight` `fee/length`, whichever is consuming more of the + /// maximum corresponding limit. + /// + /// 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, info: &DispatchInfoOf, final_fee: BalanceOf) -> TransactionPriority { + let weight_saturation = T::MaximumBlockWeight::get() / info.weight.max(1); + let len_saturation = T::MaximumBlockLength::get() as u64 / (len as u64).max(1); + let coefficient: BalanceOf = weight_saturation.min(len_saturation).saturated_into::>(); + final_fee.saturating_mul(coefficient).saturated_into::() + } } impl sp_std::fmt::Debug for ChargeTransactionPayment { @@ -499,12 +516,10 @@ impl SignedExtension for ChargeTransactionPayment whe len: usize, ) -> TransactionValidity { let (fee, _) = self.withdraw_fee(who, info, len)?; - - let mut r = ValidTransaction::default(); - // NOTE: we probably want to maximize the _fee (of any type) per weight unit_ here, which - // will be a bit more than setting the priority to tip. For now, this is enough. - r.priority = fee.saturated_into::(); - Ok(r) + Ok(ValidTransaction { + priority: Self::get_priority(len, info, fee), + ..Default::default() + }) } fn pre_dispatch(