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

Add with_weight extrinsic #12848

Merged
merged 2 commits into from
Dec 5, 2022
Merged

Add with_weight extrinsic #12848

merged 2 commits into from
Dec 5, 2022

Conversation

shawntabrizi
Copy link
Member

This adds a new utility function with_weight that allows Root origin to override the weight of any function call.

@shawntabrizi shawntabrizi added B7-runtimenoteworthy C1-low PR touches the given topic and has a low impact on builders. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited labels Dec 5, 2022
@github-actions github-actions bot added the A0-please_review Pull request needs code review. label Dec 5, 2022
) -> DispatchResult {
ensure_root(origin)?;
let res = call.dispatch_bypass_filter(frame_system::RawOrigin::Root.into());
res.map(|_| ()).map_err(|e| e.error)
Copy link
Member Author

@shawntabrizi shawntabrizi Dec 5, 2022

Choose a reason for hiding this comment

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

Other functions similar to this emit an event with the result, swallow the result, and just return Ok(()). I think now that we have storage layer by default, we can simply return the result, which I have opted for here.

@MOZGIII
Copy link
Contributor

MOZGIII commented Feb 6, 2023

Why add this if pallet_sudo with sudo_unchecked_weight exists?

@bkchr
Copy link
Member

bkchr commented Feb 6, 2023

Why add this if pallet_sudo with sudo_unchecked_weight exists?

Because you don't have the sudo pallet on production networks after the initial phase.

@MOZGIII
Copy link
Contributor

MOZGIII commented Feb 6, 2023

Should the sudo_unchecked_weight be removed from pallet_sudo then?
Also, the difference between them is that sudo has Pays::No. Was it lost here, or was it intentionally omitted?

@bkchr
Copy link
Member

bkchr commented Feb 6, 2023

Should the sudo_unchecked_weight be removed from pallet_sudo then?

I can imagine runtimes where you only have sudo, but not pallet-utility. So, I would keep it.

Also, the difference between them is that sudo has Pays::No. Was it lost here, or was it intentionally omitted?

This call can never be called directly. It requires that you are root, which requires that you are either going through the sudo pallet or it is executed by the scheduler etc. None of these charge fees, so setting Pays::No would not change anything.

@MOZGIII
Copy link
Contributor

MOZGIII commented Feb 6, 2023

I see. So, the sudo_unchecked_weight at pallet_sudo can also effectively omit the Pays::No, since it must be called from the sudo account? Or is it now the same? It seems like the sudo_unchecked_weight, unlike this one, can be just called signed with the sudo key, and then it won't qualify as "run through the sudo pallet" as you've put it, which means the Pays::No is in fact required there. Correct?

@bkchr
Copy link
Member

bkchr commented Feb 6, 2023

I see. So, the sudo_unchecked_weight at pallet_sudo can also effectively omit the Pays::No, since it must be called from the sudo account?

Yes it can only be called by the sudo account, but in this case by the "real account". It uses ensure_signed to ensure that this is called by a normal account (which should be the sudo account) and then dispatches the call as Root. So, we should not remove the Pays::No as the caller will first need to pay for the call and then gets the fees refunded.

ltfschoen pushed a commit to ltfschoen/substrate that referenced this pull request Feb 22, 2023
* add with weight extrinsic

* improve test
ark0f pushed a commit to gear-tech/substrate that referenced this pull request Feb 27, 2023
* add with weight extrinsic

* improve test
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. C1-low PR touches the given topic and has a low impact on builders. D9-needsaudit 👮 PR contains changes to fund-managing logic that should be properly reviewed and externally audited
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants