Skip to content

Commit

Permalink
Remove pallet-getter usage from pallet-transaction-payment (#4970)
Browse files Browse the repository at this point in the history
As per #3326, removes usage of the `pallet::getter` macro from the
`transaction-payment` pallet. The syntax `StorageItem::<T, I>::get()`
should be used instead.

Also, adds public functions for compatibility.

NOTE: The `Releases` enum has been made public to transition
`StorageVersion` from `pub(super) type` to `pub type`.

cc @muraca

polkadot address: 5GsLutpKjbzsbTphebs9Uy4YK6gTN47MAaz6njPktidjR5cp

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: Bastian Köcher <git@kchr.de>
Co-authored-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
  • Loading branch information
3 people authored Jul 24, 2024
1 parent 8a96d07 commit 71109c5
Show file tree
Hide file tree
Showing 3 changed files with 37 additions and 19 deletions.
14 changes: 14 additions & 0 deletions prdoc/pr_4970.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json

title: "Remove `pallet::getter` usage from the transaction-payment pallet"

doc:
- audience: Runtime Dev
description: |
This PR removes the `pallet::getter`s from `pallet-transaction-payment`.
The syntax `StorageItem::<T, I>::get()` should be used instead.

crates:
- name: pallet-transaction-payment
bump: minor
20 changes: 12 additions & 8 deletions substrate/frame/transaction-payment/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -207,7 +207,7 @@ where
// the computed ratio is only among the normal class.
let normal_max_weight =
weights.get(DispatchClass::Normal).max_total.unwrap_or(weights.max_block);
let current_block_weight = <frame_system::Pallet<T>>::block_weight();
let current_block_weight = frame_system::Pallet::<T>::block_weight();
let normal_block_weight =
current_block_weight.get(DispatchClass::Normal).min(normal_max_weight);

Expand Down Expand Up @@ -291,7 +291,7 @@ where

/// Storage releases of the pallet.
#[derive(Encode, Decode, Clone, Copy, PartialEq, Eq, RuntimeDebug, TypeInfo, MaxEncodedLen)]
enum Releases {
pub enum Releases {
/// Original version of the pallet.
V1Ancient,
/// One that bumps the usage to FixedU128 from FixedI128.
Expand Down Expand Up @@ -394,12 +394,11 @@ pub mod pallet {
}

#[pallet::storage]
#[pallet::getter(fn next_fee_multiplier)]
pub type NextFeeMultiplier<T: Config> =
StorageValue<_, Multiplier, ValueQuery, NextFeeMultiplierOnEmpty>;

#[pallet::storage]
pub(super) type StorageVersion<T: Config> = StorageValue<_, Releases, ValueQuery>;
pub type StorageVersion<T: Config> = StorageValue<_, Releases, ValueQuery>;

#[pallet::genesis_config]
pub struct GenesisConfig<T: Config> {
Expand Down Expand Up @@ -433,7 +432,7 @@ pub mod pallet {
#[pallet::hooks]
impl<T: Config> Hooks<BlockNumberFor<T>> for Pallet<T> {
fn on_finalize(_: frame_system::pallet_prelude::BlockNumberFor<T>) {
<NextFeeMultiplier<T>>::mutate(|fm| {
NextFeeMultiplier::<T>::mutate(|fm| {
*fm = T::FeeMultiplierUpdate::convert(*fm);
});
}
Expand Down Expand Up @@ -471,7 +470,7 @@ pub mod pallet {
let min_value = T::FeeMultiplierUpdate::min();
let target = target + addition;

<frame_system::Pallet<T>>::set_block_consumed_resources(target, 0);
frame_system::Pallet::<T>::set_block_consumed_resources(target, 0);
let next = T::FeeMultiplierUpdate::convert(min_value);
assert!(
next > min_value,
Expand All @@ -484,6 +483,11 @@ pub mod pallet {
}

impl<T: Config> Pallet<T> {
/// Public function to access the next fee multiplier.
pub fn next_fee_multiplier() -> Multiplier {
NextFeeMultiplier::<T>::get()
}

/// Query the data that we know about the fee of a given `call`.
///
/// This pallet is not and cannot be aware of the internals of a signed extension, for example
Expand Down Expand Up @@ -633,7 +637,7 @@ impl<T: Config> Pallet<T> {
if pays_fee == Pays::Yes {
// the adjustable part of the fee.
let unadjusted_weight_fee = Self::weight_to_fee(weight);
let multiplier = Self::next_fee_multiplier();
let multiplier = NextFeeMultiplier::<T>::get();
// final adjusted weight fee.
let adjusted_weight_fee = multiplier.saturating_mul_int(unadjusted_weight_fee);

Expand Down Expand Up @@ -675,7 +679,7 @@ where
/// share that the weight contributes to the overall fee of a transaction. It is mainly
/// for informational purposes and not used in the actual fee calculation.
fn convert(weight: Weight) -> BalanceOf<T> {
<NextFeeMultiplier<T>>::get().saturating_mul_int(Self::weight_to_fee(weight))
NextFeeMultiplier::<T>::get().saturating_mul_int(Self::weight_to_fee(weight))
}
}

Expand Down
22 changes: 11 additions & 11 deletions substrate/frame/transaction-payment/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -180,7 +180,7 @@ fn signed_extension_transaction_payment_multiplied_refund_works() {
.build()
.execute_with(|| {
let len = 10;
<NextFeeMultiplier<Runtime>>::put(Multiplier::saturating_from_rational(3, 2));
NextFeeMultiplier::<Runtime>::put(Multiplier::saturating_from_rational(3, 2));

let pre = ChargeTransactionPayment::<Runtime>::from(5 /* tipped */)
.pre_dispatch(&2, CALL, &info_from_weight(Weight::from_parts(100, 0)), len)
Expand Down Expand Up @@ -270,7 +270,7 @@ fn signed_ext_length_fee_is_also_updated_per_congestion() {
.build()
.execute_with(|| {
// all fees should be x1.5
<NextFeeMultiplier<Runtime>>::put(Multiplier::saturating_from_rational(3, 2));
NextFeeMultiplier::<Runtime>::put(Multiplier::saturating_from_rational(3, 2));
let len = 10;

assert_ok!(ChargeTransactionPayment::<Runtime>::from(10) // tipped
Expand Down Expand Up @@ -305,7 +305,7 @@ fn query_info_and_fee_details_works() {
.build()
.execute_with(|| {
// all fees should be x1.5
<NextFeeMultiplier<Runtime>>::put(Multiplier::saturating_from_rational(3, 2));
NextFeeMultiplier::<Runtime>::put(Multiplier::saturating_from_rational(3, 2));

assert_eq!(
TransactionPayment::query_info(xt.clone(), len),
Expand Down Expand Up @@ -362,7 +362,7 @@ fn query_call_info_and_fee_details_works() {
.build()
.execute_with(|| {
// all fees should be x1.5
<NextFeeMultiplier<Runtime>>::put(Multiplier::saturating_from_rational(3, 2));
NextFeeMultiplier::<Runtime>::put(Multiplier::saturating_from_rational(3, 2));

assert_eq!(
TransactionPayment::query_call_info(call.clone(), len),
Expand Down Expand Up @@ -401,7 +401,7 @@ fn compute_fee_works_without_multiplier() {
.build()
.execute_with(|| {
// Next fee multiplier is zero
assert_eq!(<NextFeeMultiplier<Runtime>>::get(), Multiplier::one());
assert_eq!(NextFeeMultiplier::<Runtime>::get(), Multiplier::one());

// Tip only, no fees works
let dispatch_info = DispatchInfo {
Expand Down Expand Up @@ -440,7 +440,7 @@ fn compute_fee_works_with_multiplier() {
.build()
.execute_with(|| {
// Add a next fee multiplier. Fees will be x3/2.
<NextFeeMultiplier<Runtime>>::put(Multiplier::saturating_from_rational(3, 2));
NextFeeMultiplier::<Runtime>::put(Multiplier::saturating_from_rational(3, 2));
// Base fee is unaffected by multiplier
let dispatch_info = DispatchInfo {
weight: Weight::from_parts(0, 0),
Expand Down Expand Up @@ -472,7 +472,7 @@ fn compute_fee_works_with_negative_multiplier() {
.build()
.execute_with(|| {
// Add a next fee multiplier. All fees will be x1/2.
<NextFeeMultiplier<Runtime>>::put(Multiplier::saturating_from_rational(1, 2));
NextFeeMultiplier::<Runtime>::put(Multiplier::saturating_from_rational(1, 2));

// Base fee is unaffected by multiplier.
let dispatch_info = DispatchInfo {
Expand Down Expand Up @@ -637,7 +637,7 @@ fn refund_consistent_with_actual_weight() {
let len = 10;
let tip = 5;

<NextFeeMultiplier<Runtime>>::put(Multiplier::saturating_from_rational(5, 4));
NextFeeMultiplier::<Runtime>::put(Multiplier::saturating_from_rational(5, 4));

let pre = ChargeTransactionPayment::<Runtime>::from(tip)
.pre_dispatch(&2, CALL, &info, len)
Expand Down Expand Up @@ -797,7 +797,7 @@ fn post_info_can_change_pays_fee() {
let len = 10;
let tip = 5;

<NextFeeMultiplier<Runtime>>::put(Multiplier::saturating_from_rational(5, 4));
NextFeeMultiplier::<Runtime>::put(Multiplier::saturating_from_rational(5, 4));

let pre = ChargeTransactionPayment::<Runtime>::from(tip)
.pre_dispatch(&2, CALL, &info, len)
Expand Down Expand Up @@ -829,7 +829,7 @@ fn genesis_config_works() {
.build()
.execute_with(|| {
assert_eq!(
<NextFeeMultiplier<Runtime>>::get(),
NextFeeMultiplier::<Runtime>::get(),
Multiplier::saturating_from_integer(100)
);
});
Expand All @@ -838,6 +838,6 @@ fn genesis_config_works() {
#[test]
fn genesis_default_works() {
ExtBuilder::default().build().execute_with(|| {
assert_eq!(<NextFeeMultiplier<Runtime>>::get(), Multiplier::saturating_from_integer(1));
assert_eq!(NextFeeMultiplier::<Runtime>::get(), Multiplier::saturating_from_integer(1));
});
}

0 comments on commit 71109c5

Please sign in to comment.