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

FRAME: inherited call weight syntax #13932

Merged
merged 19 commits into from
Apr 27, 2023
Merged

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Apr 17, 2023

There was a request to create a less repetitive syntax in the case that calls use the WeightInfo trait to calculate their weight.

This MR adds an attribute to the #[pallet::call] macro in the form of #[pallet::call(weight($type))].
$type will then be prepended to the call name to generate a #[pallet::weight] attribute for each call that does not explicitly annotate one. Alternatively a = token can be used like #[pallet::call(weight = …)].
It is not restricted to use <T as Config>::WeightInfo and could use other types as well (although not advised).

The pallets Alliance, Assets, Balances and example-basic are updated to demonstrate the effect.

Old Syntax

With the pallet::weight annotation above each call.

#[pallet::call]
impl<T: Config> Pallet<T> {

	#[pallet::weight(T::WeightInfo::create())]
	pub fn create(
		...

New Syntax

Intended usage together with a WeightInfo trait. A Config<I> can be used for instanced pallets.

#[pallet::call(weight(<T as Config>::WeightInfo))]
impl<T: Config> Pallet<T> {

	pub fn create(
		...

It is possible to override the inherited value per call:

#[pallet::call(weight = <T as Config>::WeightInfo)]
impl<T: Config> Pallet<T> {

	#[pallet::weight(T::WeightInfo::not_create())]
	pub fn create(
		...

... or using something else entirely (for whatever reason):

struct MyWeight;
impl MyWeight {
	fn create() -> Weight {
		Weight::zero()
	}
}

#[pallet::call(weight(crate::MyWeight))]
impl<T: Config> Pallet<T> {

	pub fn create(
		...

Dev Mode

The inherited weight has precedence over the default weight (=zero).

Follow-ups

TODOs

  • Check UI errors
  • Tests to assert that the resulting weight value is correct.
  • Update / Add docs

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez added A3-in_progress Pull request is in progress. No review needed at this stage. C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. B1-note_worthy Changes should be noted in the release notes T1-runtime This PR/Issue is related to the topic “runtime”. labels Apr 17, 2023
@ggwpez ggwpez self-assigned this Apr 17, 2023
@ggwpez ggwpez added A0-please_review Pull request needs code review. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Apr 17, 2023
@KiChjang
Copy link
Contributor

I prefer making it simple, so something like #[pallet::call(weight($type))] or #[pallet::call(weight = $type)] is more reasonable to me. Putting a prefix there makes people believe that you can put more attributes other than prefix there, which isn't true (yet).

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
Copy link
Member Author

ggwpez commented Apr 17, 2023

I prefer making it simple, so something like #[pallet::call(weight($type))] or #[pallet::call(weight = $type)] is more reasonable to me. Putting a prefix there makes people believe that you can put more attributes other than prefix there, which isn't true (yet).

Yep updated it to use weight($type) and weight = $type.
It stays extensible since we can still use keywords like weight(const 123), if we ever want to.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez requested a review from a team April 18, 2023 16:16
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>
@ggwpez ggwpez requested review from sam0x17 and removed request for a team, andresilva and athei April 18, 2023 16:18
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Copy link
Contributor

@sam0x17 sam0x17 left a comment

Choose a reason for hiding this comment

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

This looks good to me, just a few nits / comments

frame/support/procedural/src/pallet/expand/call.rs Outdated Show resolved Hide resolved
frame/support/procedural/src/pallet/parse/call.rs Outdated Show resolved Hide resolved
frame/support/procedural/src/pallet/parse/mod.rs Outdated Show resolved Hide resolved
Co-authored-by: Sam Johnson <sam@durosoft.com>

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
Copy link
Member Author

ggwpez commented Apr 25, 2023

bot rebase

@paritytech-processbot
Copy link

Rebased

frame/support/procedural/src/pallet/parse/mod.rs Outdated Show resolved Hide resolved
frame/support/procedural/src/pallet/parse/mod.rs Outdated Show resolved Hide resolved
frame/support/procedural/src/pallet/parse/pallet_struct.rs Outdated Show resolved Hide resolved
ggwpez and others added 4 commits April 26, 2023 17:50
Co-authored-by: Muharem Ismailov <ismailov.m.h@gmail.com>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez
Copy link
Member Author

ggwpez commented Apr 27, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit f2b6680 into master Apr 27, 2023
@paritytech-processbot paritytech-processbot bot deleted the oty-pallet-call-weight branch April 27, 2023 14:08
gpestana pushed a commit that referenced this pull request May 4, 2023
* 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>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
* 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>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
A0-please_review Pull request needs code review. B1-note_worthy Changes should be noted in the release notes C1-low PR touches the given topic and has a low impact on builders. D5-nicetohaveaudit ⚠️ PR contains trivial changes to logic that should be properly reviewed. T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

4 participants