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 3 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
4 changes: 2 additions & 2 deletions bin/node-template/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -258,13 +258,13 @@ impl pallet_balances::Config for Runtime {

parameter_types! {
pub const TransactionByteFee: Balance = 1;
pub OperationalVirtualTip: Balance = ExistentialDeposit::get();
pub OperationalFeeMultiplier: u8 = 5;
}

impl pallet_transaction_payment::Config for Runtime {
type OnChargeTransaction = CurrencyAdapter<Balances, ()>;
type TransactionByteFee = TransactionByteFee;
type OperationalVirtualTip = OperationalVirtualTip;
type OperationalFeeMultiplier = OperationalFeeMultiplierBalance;
type WeightToFee = IdentityFee<Balance>;
type FeeMultiplierUpdate = ();
}
Expand Down
5 changes: 3 additions & 2 deletions bin/node/runtime/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -413,7 +413,7 @@ impl pallet_balances::Config for Runtime {

parameter_types! {
pub const TransactionByteFee: Balance = 10 * MILLICENTS;
pub const OperationalVirtualTip: Balance = 1 * DOLLARS;
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 @@ -422,7 +422,8 @@ parameter_types! {
impl pallet_transaction_payment::Config for Runtime {
type OnChargeTransaction = CurrencyAdapter<Balances, DealWithFees>;
type TransactionByteFee = TransactionByteFee;
type OperationalVirtualTip = OperationalVirtualTip;
type OperationalFeeMultiplier = OperationalFeeMultiplier;

tomusdrw marked this conversation as resolved.
Show resolved Hide resolved
type WeightToFee = IdentityFee<Balance>;
type FeeMultiplierUpdate =
TargetedFeeAdjustment<Self, TargetBlockFullness, AdjustmentVariable, MinimumMultiplier>;
Expand Down
4 changes: 2 additions & 2 deletions frame/balances/src/tests_composite.rs
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,12 @@ impl frame_system::Config for Test {
}
parameter_types! {
pub const TransactionByteFee: u64 = 1;
pub const OperationalVirtualTip: u64 = 1;
pub const OperationalFeeMultiplier: u8 = 5;
}
impl pallet_transaction_payment::Config for Test {
type OnChargeTransaction = CurrencyAdapter<Pallet<Test>, ()>;
type TransactionByteFee = TransactionByteFee;
type OperationalVirtualTip = OperationalVirtualTip;
type OperationalFeeMultiplier = OperationalFeeMultiplier;
type WeightToFee = IdentityFee<u64>;
type FeeMultiplierUpdate = ();
}
Expand Down
4 changes: 2 additions & 2 deletions frame/balances/src/tests_local.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,12 +78,12 @@ impl frame_system::Config for Test {
}
parameter_types! {
pub const TransactionByteFee: u64 = 1;
pub const OperationalVirtualTip: u64 = 1;
pub const OperationalFeeMultiplier: u8 = 5;
}
impl pallet_transaction_payment::Config for Test {
type OnChargeTransaction = CurrencyAdapter<Pallet<Test>, ()>;
type TransactionByteFee = TransactionByteFee;
type OperationalVirtualTip = OperationalVirtualTip;
type OperationalFeeMultiplier = OperationalFeeMultiplier;
type WeightToFee = IdentityFee<u64>;
type FeeMultiplierUpdate = ();
}
Expand Down
4 changes: 2 additions & 2 deletions frame/balances/src/tests_reentrancy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,12 +80,12 @@ impl frame_system::Config for Test {
}
parameter_types! {
pub const TransactionByteFee: u64 = 1;
pub const OperationalVirtualTip: u64 = 1;
pub const OperationalFeeMultiplier: u8 = 5;
}
impl pallet_transaction_payment::Config for Test {
type OnChargeTransaction = CurrencyAdapter<Pallet<Test>, ()>;
type TransactionByteFee = TransactionByteFee;
type OperationalVirtualTip = OperationalVirtualTip;
type OperationalFeeMultiplier = OperationalFeeMultiplier;
type WeightToFee = IdentityFee<u64>;
type FeeMultiplierUpdate = ();
}
Expand Down
4 changes: 2 additions & 2 deletions frame/executive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -800,12 +800,12 @@ mod tests {

parameter_types! {
pub const TransactionByteFee: Balance = 0;
pub const OperationalVirtualTip: Balance = 1;
pub const OperationalFeeMultiplier: u8 = 5;
}
impl pallet_transaction_payment::Config for Runtime {
type OnChargeTransaction = CurrencyAdapter<Balances, ()>;
type TransactionByteFee = TransactionByteFee;
type OperationalVirtualTip = OperationalVirtualTip;
type OperationalFeeMultiplier = OperationalFeeMultiplier;
type WeightToFee = IdentityFee<Balance>;
type FeeMultiplierUpdate = ();
}
Expand Down
47 changes: 34 additions & 13 deletions frame/transaction-payment/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,13 +264,29 @@ pub mod pallet {
#[pallet::constant]
type TransactionByteFee: Get<BalanceOf<Self>>;

/// A virtual (not payed) tip added to `Operational` extrinsics to boost their `priority`.
/// A fee mulitplier for `Operational` extrinsics to compute "virtual tip" to boost their
/// `priority`
///
/// This value is simply included to a tip component in regular `priority` calculations.
/// 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 this.
/// 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 the transaction get's a priority bump based
/// on the `inclusion_fee` but also we amplify the impact of tips applied to `Operational`
/// transactions.
tomusdrw marked this conversation as resolved.
Show resolved Hide resolved
#[pallet::constant]
type OperationalVirtualTip: Get<BalanceOf<Self>>;
type OperationalFeeMultiplier: Get<u8>;

/// Convert a weight value into a deductible fee based on the currency type.
type WeightToFee: WeightToFeePolynomial<Balance = BalanceOf<Self>>;
Expand Down Expand Up @@ -595,10 +611,11 @@ where
/// `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.
Copy link

@gww-parity gww-parity Sep 29, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What about adding some assert in a code to ensure assumptions? (i.e. that values are in expected ranges, preventing saturation)

fn get_priority(
pub fn get_priority(
info: &DispatchInfoOf<T::Call>,
len: usize,
tip: BalanceOf<T>,
final_fee: BalanceOf<T>,
) -> TransactionPriority {
// Calculate how many such extrinsics we could fit into an empty block and take
// the limitting factor.
Expand Down Expand Up @@ -640,7 +657,8 @@ where
// 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 virtual_tip = T::OperationalVirtualTip::get();
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)
Expand Down Expand Up @@ -689,9 +707,12 @@ where
info: &DispatchInfoOf<Self::Call>,
len: usize,
) -> TransactionValidity {
let (_fee, _) = self.withdraw_fee(who, call, info, len)?;
let (final_fee, _) = self.withdraw_fee(who, call, info, len)?;
let tip = self.0;
Ok(ValidTransaction { priority: Self::get_priority(info, len, tip), ..Default::default() })
Ok(ValidTransaction {
priority: Self::get_priority(info, len, tip, final_fee),
..Default::default()
})
}

fn pre_dispatch(
Expand Down Expand Up @@ -804,7 +825,7 @@ mod tests {
pub const BlockHashCount: u64 = 250;
pub static TransactionByteFee: u64 = 1;
pub static WeightToFee: u64 = 1;
pub static OperationalVirtualTip: u64 = 5 * 5;
pub static OperationalFeeMultiplier: u8 = 5;
}

impl frame_system::Config for Runtime {
Expand Down Expand Up @@ -884,7 +905,7 @@ mod tests {
impl Config for Runtime {
type OnChargeTransaction = CurrencyAdapter<Balances, DealWithFees>;
type TransactionByteFee = TransactionByteFee;
type OperationalVirtualTip = OperationalVirtualTip;
type OperationalFeeMultiplier = OperationalFeeMultiplier;
type WeightToFee = WeightToFee;
type FeeMultiplierUpdate = ();
}
Expand Down Expand Up @@ -1431,13 +1452,13 @@ mod tests {
.validate(&2, CALL, &op, len)
.unwrap()
.priority;
assert_eq!(priority, 300);
assert_eq!(priority, 5800);

let priority = ChargeTransactionPayment::<Runtime>(2 * tip)
.validate(&2, CALL, &op, len)
.unwrap()
.priority;
assert_eq!(priority, 350);
assert_eq!(priority, 6100);
});
}

Expand Down Expand Up @@ -1467,7 +1488,7 @@ mod tests {
.validate(&2, CALL, &op, len)
.unwrap()
.priority;
assert_eq!(priority, 260);
assert_eq!(priority, 5510);
});
}

Expand Down