Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Remove pallet-getter usage from pallet-transaction-payment #4970

Merged
merged 6 commits into from
Jul 24, 2024
+37 −19
Merged

Remove pallet-getter usage from pallet-transaction-payment #4970

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
70d2ce1
Remove pallet-getter usage from the transaction-payment pallet
mittal-parth Jul 8, 2024
a44a816
Add PR doc
mittal-parth Jul 8, 2024
79082e9
Merge branch 'master' into remove-getters-transaction-payment
bkchr Jul 20, 2024
d88e288
Merge branch 'master' into remove-getters-transaction-payment
bkchr Jul 23, 2024
ae1dc6a
Move prdoc
ggwpez Jul 23, 2024
465680e
Merge branch 'master' into remove-getters-transaction-payment
bkchr Jul 24, 2024
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
14 changes: 14 additions & 0 deletions prdoc/pr_4970.prdoc
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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));
});
}
Loading