Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Commit

Permalink
FRAME: inherited call weight syntax (#13932)
Browse files Browse the repository at this point in the history
* First approach on pallet::call_weight

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Use attr on pallet::call instead

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Ui tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Rename to weight(prefix = ...))

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Simplify to #[pallet::call(weight(T))]

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Add stray token error

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Cleanup

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Migrate remaining pallets

Using script from https://github.com/ggwpez/substrate-scripts/blob/e1b5ea5b5b4018867f3e869fce6f448b4ba9d71f/frame-code-migration/src/call_weight.rs

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Try to add some docs

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Revert "Migrate remaining pallets"

Lets do this as a follow-up, I dont want to bloat this small MR.

This reverts commit 331d4b4.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Renames

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Review fixes

Co-authored-by: Sam Johnson <sam@durosoft.com>

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Test weights

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update UI tests

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update frame/support/procedural/src/pallet/parse/mod.rs

Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>

* Remove old code

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

* Update docs

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>

---------

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: parity-processbot <>
Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>
  • Loading branch information
ggwpez and muharem committed Apr 27, 2023
1 parent 5d1b74c commit f2b6680
Show file tree
Hide file tree
Showing 30 changed files with 698 additions and 92 deletions.
12 changes: 1 addition & 11 deletions frame/alliance/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -497,7 +497,7 @@ pub mod pallet {
pub type UnscrupulousWebsites<T: Config<I>, I: 'static = ()> =
StorageValue<_, BoundedVec<UrlOf<T, I>, T::MaxUnscrupulousItems>, ValueQuery>;

#[pallet::call]
#[pallet::call(weight(<T as Config<I>>::WeightInfo))]
impl<T: Config<I>, I: 'static> Pallet<T, I> {
/// Add a new proposal to be voted on.
///
Expand Down Expand Up @@ -649,7 +649,6 @@ pub mod pallet {

/// Set a new IPFS CID to the alliance rule.
#[pallet::call_index(5)]
#[pallet::weight(T::WeightInfo::set_rule())]
pub fn set_rule(origin: OriginFor<T>, rule: Cid) -> DispatchResult {
T::AdminOrigin::ensure_origin(origin)?;

Expand All @@ -661,7 +660,6 @@ pub mod pallet {

/// Make an announcement of a new IPFS CID about alliance issues.
#[pallet::call_index(6)]
#[pallet::weight(T::WeightInfo::announce())]
pub fn announce(origin: OriginFor<T>, announcement: Cid) -> DispatchResult {
T::AnnouncementOrigin::ensure_origin(origin)?;

Expand All @@ -677,7 +675,6 @@ pub mod pallet {

/// Remove an announcement.
#[pallet::call_index(7)]
#[pallet::weight(T::WeightInfo::remove_announcement())]
pub fn remove_announcement(origin: OriginFor<T>, announcement: Cid) -> DispatchResult {
T::AnnouncementOrigin::ensure_origin(origin)?;

Expand All @@ -695,7 +692,6 @@ pub mod pallet {

/// Submit oneself for candidacy. A fixed deposit is reserved.
#[pallet::call_index(8)]
#[pallet::weight(T::WeightInfo::join_alliance())]
pub fn join_alliance(origin: OriginFor<T>) -> DispatchResult {
let who = ensure_signed(origin)?;

Expand Down Expand Up @@ -732,7 +728,6 @@ pub mod pallet {
/// A Fellow can nominate someone to join the alliance as an Ally. There is no deposit
/// required from the nominator or nominee.
#[pallet::call_index(9)]
#[pallet::weight(T::WeightInfo::nominate_ally())]
pub fn nominate_ally(origin: OriginFor<T>, who: AccountIdLookupOf<T>) -> DispatchResult {
let nominator = ensure_signed(origin)?;
ensure!(Self::has_voting_rights(&nominator), Error::<T, I>::NoVotingRights);
Expand All @@ -757,7 +752,6 @@ pub mod pallet {

/// Elevate an Ally to Fellow.
#[pallet::call_index(10)]
#[pallet::weight(T::WeightInfo::elevate_ally())]
pub fn elevate_ally(origin: OriginFor<T>, ally: AccountIdLookupOf<T>) -> DispatchResult {
T::MembershipManager::ensure_origin(origin)?;
let ally = T::Lookup::lookup(ally)?;
Expand All @@ -774,7 +768,6 @@ pub mod pallet {
/// As a member, give a retirement notice and start a retirement period required to pass in
/// order to retire.
#[pallet::call_index(11)]
#[pallet::weight(T::WeightInfo::give_retirement_notice())]
pub fn give_retirement_notice(origin: OriginFor<T>) -> DispatchResult {
let who = ensure_signed(origin)?;
let role = Self::member_role_of(&who).ok_or(Error::<T, I>::NotMember)?;
Expand All @@ -797,7 +790,6 @@ pub mod pallet {
/// This can only be done once you have called `give_retirement_notice` and the
/// `RetirementPeriod` has passed.
#[pallet::call_index(12)]
#[pallet::weight(T::WeightInfo::retire())]
pub fn retire(origin: OriginFor<T>) -> DispatchResult {
let who = ensure_signed(origin)?;
let retirement_period_end = RetiringMembers::<T, I>::get(&who)
Expand All @@ -820,7 +812,6 @@ pub mod pallet {

/// Kick a member from the Alliance and slash its deposit.
#[pallet::call_index(13)]
#[pallet::weight(T::WeightInfo::kick_member())]
pub fn kick_member(origin: OriginFor<T>, who: AccountIdLookupOf<T>) -> DispatchResult {
T::MembershipManager::ensure_origin(origin)?;
let member = T::Lookup::lookup(who)?;
Expand Down Expand Up @@ -922,7 +913,6 @@ pub mod pallet {
/// who do not want to leave the Alliance but do not have the capacity to participate
/// operationally for some time.
#[pallet::call_index(17)]
#[pallet::weight(T::WeightInfo::abdicate_fellow_status())]
pub fn abdicate_fellow_status(origin: OriginFor<T>) -> DispatchResult {
let who = ensure_signed(origin)?;
let role = Self::member_role_of(&who).ok_or(Error::<T, I>::NotMember)?;
Expand Down
9 changes: 9 additions & 0 deletions frame/alliance/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -629,3 +629,12 @@ fn remove_unscrupulous_items_works() {
assert_eq!(Alliance::unscrupulous_accounts(), Vec::<u64>::new());
});
}

#[test]
fn weights_sane() {
let info = crate::Call::<Test>::join_alliance {}.get_dispatch_info();
assert_eq!(<() as crate::WeightInfo>::join_alliance(), info.weight);

let info = crate::Call::<Test>::nominate_ally { who: 10 }.get_dispatch_info();
assert_eq!(<() as crate::WeightInfo>::nominate_ally(), info.weight);
}
25 changes: 1 addition & 24 deletions frame/assets/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -568,7 +568,7 @@ pub mod pallet {
CallbackFailed,
}

#[pallet::call]
#[pallet::call(weight(<T as Config<I>>::WeightInfo))]
impl<T: Config<I>, I: 'static> Pallet<T, I> {
/// Issue a new class of fungible assets from a public origin.
///
Expand All @@ -590,7 +590,6 @@ pub mod pallet {
///
/// Weight: `O(1)`
#[pallet::call_index(0)]
#[pallet::weight(T::WeightInfo::create())]
pub fn create(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand Down Expand Up @@ -654,7 +653,6 @@ pub mod pallet {
///
/// Weight: `O(1)`
#[pallet::call_index(1)]
#[pallet::weight(T::WeightInfo::force_create())]
pub fn force_create(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand All @@ -680,7 +678,6 @@ pub mod pallet {
///
/// The asset class must be frozen before calling `start_destroy`.
#[pallet::call_index(2)]
#[pallet::weight(T::WeightInfo::start_destroy())]
pub fn start_destroy(origin: OriginFor<T>, id: T::AssetIdParameter) -> DispatchResult {
let maybe_check_owner = match T::ForceOrigin::try_origin(origin) {
Ok(_) => None,
Expand Down Expand Up @@ -749,7 +746,6 @@ pub mod pallet {
///
/// Each successful call emits the `Event::Destroyed` event.
#[pallet::call_index(5)]
#[pallet::weight(T::WeightInfo::finish_destroy())]
pub fn finish_destroy(origin: OriginFor<T>, id: T::AssetIdParameter) -> DispatchResult {
let _ = ensure_signed(origin)?;
let id: T::AssetId = id.into();
Expand All @@ -769,7 +765,6 @@ pub mod pallet {
/// Weight: `O(1)`
/// Modes: Pre-existing balance of `beneficiary`; Account pre-existence of `beneficiary`.
#[pallet::call_index(6)]
#[pallet::weight(T::WeightInfo::mint())]
pub fn mint(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand Down Expand Up @@ -799,7 +794,6 @@ pub mod pallet {
/// Weight: `O(1)`
/// Modes: Post-existence of `who`; Pre & post Zombie-status of `who`.
#[pallet::call_index(7)]
#[pallet::weight(T::WeightInfo::burn())]
pub fn burn(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand Down Expand Up @@ -834,7 +828,6 @@ pub mod pallet {
/// Modes: Pre-existence of `target`; Post-existence of sender; Account pre-existence of
/// `target`.
#[pallet::call_index(8)]
#[pallet::weight(T::WeightInfo::transfer())]
pub fn transfer(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand Down Expand Up @@ -868,7 +861,6 @@ pub mod pallet {
/// Modes: Pre-existence of `target`; Post-existence of sender; Account pre-existence of
/// `target`.
#[pallet::call_index(9)]
#[pallet::weight(T::WeightInfo::transfer_keep_alive())]
pub fn transfer_keep_alive(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand Down Expand Up @@ -903,7 +895,6 @@ pub mod pallet {
/// Modes: Pre-existence of `dest`; Post-existence of `source`; Account pre-existence of
/// `dest`.
#[pallet::call_index(10)]
#[pallet::weight(T::WeightInfo::force_transfer())]
pub fn force_transfer(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand Down Expand Up @@ -931,7 +922,6 @@ pub mod pallet {
///
/// Weight: `O(1)`
#[pallet::call_index(11)]
#[pallet::weight(T::WeightInfo::freeze())]
pub fn freeze(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand Down Expand Up @@ -968,7 +958,6 @@ pub mod pallet {
///
/// Weight: `O(1)`
#[pallet::call_index(12)]
#[pallet::weight(T::WeightInfo::thaw())]
pub fn thaw(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand Down Expand Up @@ -1004,7 +993,6 @@ pub mod pallet {
///
/// Weight: `O(1)`
#[pallet::call_index(13)]
#[pallet::weight(T::WeightInfo::freeze_asset())]
pub fn freeze_asset(origin: OriginFor<T>, id: T::AssetIdParameter) -> DispatchResult {
let origin = ensure_signed(origin)?;
let id: T::AssetId = id.into();
Expand All @@ -1031,7 +1019,6 @@ pub mod pallet {
///
/// Weight: `O(1)`
#[pallet::call_index(14)]
#[pallet::weight(T::WeightInfo::thaw_asset())]
pub fn thaw_asset(origin: OriginFor<T>, id: T::AssetIdParameter) -> DispatchResult {
let origin = ensure_signed(origin)?;
let id: T::AssetId = id.into();
Expand Down Expand Up @@ -1059,7 +1046,6 @@ pub mod pallet {
///
/// Weight: `O(1)`
#[pallet::call_index(15)]
#[pallet::weight(T::WeightInfo::transfer_ownership())]
pub fn transfer_ownership(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand Down Expand Up @@ -1103,7 +1089,6 @@ pub mod pallet {
///
/// Weight: `O(1)`
#[pallet::call_index(16)]
#[pallet::weight(T::WeightInfo::set_team())]
pub fn set_team(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand Down Expand Up @@ -1173,7 +1158,6 @@ pub mod pallet {
///
/// Weight: `O(1)`
#[pallet::call_index(18)]
#[pallet::weight(T::WeightInfo::clear_metadata())]
pub fn clear_metadata(origin: OriginFor<T>, id: T::AssetIdParameter) -> DispatchResult {
let origin = ensure_signed(origin)?;
let id: T::AssetId = id.into();
Expand Down Expand Up @@ -1257,7 +1241,6 @@ pub mod pallet {
///
/// Weight: `O(1)`
#[pallet::call_index(20)]
#[pallet::weight(T::WeightInfo::force_clear_metadata())]
pub fn force_clear_metadata(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand Down Expand Up @@ -1297,7 +1280,6 @@ pub mod pallet {
///
/// Weight: `O(1)`
#[pallet::call_index(21)]
#[pallet::weight(T::WeightInfo::force_asset_status())]
pub fn force_asset_status(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand Down Expand Up @@ -1354,7 +1336,6 @@ pub mod pallet {
///
/// Weight: `O(1)`
#[pallet::call_index(22)]
#[pallet::weight(T::WeightInfo::approve_transfer())]
pub fn approve_transfer(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand All @@ -1381,7 +1362,6 @@ pub mod pallet {
///
/// Weight: `O(1)`
#[pallet::call_index(23)]
#[pallet::weight(T::WeightInfo::cancel_approval())]
pub fn cancel_approval(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand Down Expand Up @@ -1418,7 +1398,6 @@ pub mod pallet {
///
/// Weight: `O(1)`
#[pallet::call_index(24)]
#[pallet::weight(T::WeightInfo::force_cancel_approval())]
pub fn force_cancel_approval(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand Down Expand Up @@ -1468,7 +1447,6 @@ pub mod pallet {
///
/// Weight: `O(1)`
#[pallet::call_index(25)]
#[pallet::weight(T::WeightInfo::transfer_approved())]
pub fn transfer_approved(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand Down Expand Up @@ -1531,7 +1509,6 @@ pub mod pallet {
///
/// Emits `AssetMinBalanceChanged` event when successful.
#[pallet::call_index(28)]
#[pallet::weight(T::WeightInfo::set_min_balance())]
pub fn set_min_balance(
origin: OriginFor<T>,
id: T::AssetIdParameter,
Expand Down
10 changes: 10 additions & 0 deletions frame/assets/src/tests.rs
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ use super::*;
use crate::{mock::*, Error};
use frame_support::{
assert_noop, assert_ok,
dispatch::GetDispatchInfo,
traits::{fungibles::InspectEnumerable, tokens::Preservation::Protect, Currency},
};
use pallet_balances::Error as BalancesError;
Expand Down Expand Up @@ -1355,3 +1356,12 @@ fn multiple_transfer_alls_work_ok() {
assert_eq!(Balances::free_balance(&1337), 100);
});
}

#[test]
fn weights_sane() {
let info = crate::Call::<Test>::create { id: 10, admin: 4, min_balance: 3 }.get_dispatch_info();
assert_eq!(<() as crate::WeightInfo>::create(), info.weight);

let info = crate::Call::<Test>::finish_destroy { id: 10 }.get_dispatch_info();
assert_eq!(<() as crate::WeightInfo>::finish_destroy(), info.weight);
}
7 changes: 1 addition & 6 deletions frame/balances/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -526,7 +526,7 @@ pub mod pallet {
}
}

#[pallet::call]
#[pallet::call(weight(<T as Config<I>>::WeightInfo))]
impl<T: Config<I>, I: 'static> Pallet<T, I> {
/// Transfer some liquid free balance to another account.
///
Expand All @@ -536,7 +536,6 @@ pub mod pallet {
///
/// The dispatch origin for this call must be `Signed` by the transactor.
#[pallet::call_index(0)]
#[pallet::weight(T::WeightInfo::transfer_allow_death())]
pub fn transfer_allow_death(
origin: OriginFor<T>,
dest: AccountIdLookupOf<T>,
Expand Down Expand Up @@ -598,7 +597,6 @@ pub mod pallet {
/// Exactly as `transfer_allow_death`, except the origin must be root and the source account
/// may be specified.
#[pallet::call_index(2)]
#[pallet::weight(T::WeightInfo::force_transfer())]
pub fn force_transfer(
origin: OriginFor<T>,
source: AccountIdLookupOf<T>,
Expand All @@ -619,7 +617,6 @@ pub mod pallet {
///
/// [`transfer_allow_death`]: struct.Pallet.html#method.transfer
#[pallet::call_index(3)]
#[pallet::weight(T::WeightInfo::transfer_keep_alive())]
pub fn transfer_keep_alive(
origin: OriginFor<T>,
dest: AccountIdLookupOf<T>,
Expand Down Expand Up @@ -647,7 +644,6 @@ pub mod pallet {
/// transfer everything except at least the existential deposit, which will guarantee to
/// keep the sender account alive (true).
#[pallet::call_index(4)]
#[pallet::weight(T::WeightInfo::transfer_all())]
pub fn transfer_all(
origin: OriginFor<T>,
dest: AccountIdLookupOf<T>,
Expand All @@ -674,7 +670,6 @@ pub mod pallet {
///
/// Can only be called by ROOT.
#[pallet::call_index(5)]
#[pallet::weight(T::WeightInfo::force_unreserve())]
pub fn force_unreserve(
origin: OriginFor<T>,
who: AccountIdLookupOf<T>,
Expand Down
11 changes: 10 additions & 1 deletion frame/balances/src/tests/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ use crate::{self as pallet_balances, AccountData, Config, CreditOf, Error, Palle
use codec::{Decode, Encode, MaxEncodedLen};
use frame_support::{
assert_err, assert_noop, assert_ok, assert_storage_noop,
dispatch::DispatchInfo,
dispatch::{DispatchInfo, GetDispatchInfo},
parameter_types,
traits::{
tokens::fungible, ConstU32, ConstU64, ConstU8, Imbalance as ImbalanceT, OnUnbalanced,
Expand Down Expand Up @@ -294,3 +294,12 @@ pub fn events() -> Vec<RuntimeEvent> {
pub fn info_from_weight(w: Weight) -> DispatchInfo {
DispatchInfo { weight: w, ..Default::default() }
}

#[test]
fn weights_sane() {
let info = crate::Call::<Test>::transfer_allow_death { dest: 10, value: 4 }.get_dispatch_info();
assert_eq!(<() as crate::WeightInfo>::transfer_allow_death(), info.weight);

let info = crate::Call::<Test>::force_unreserve { who: 10, amount: 4 }.get_dispatch_info();
assert_eq!(<() as crate::WeightInfo>::force_unreserve(), info.weight);
}
Loading

0 comments on commit f2b6680

Please sign in to comment.