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

Uniform pallet warnings #13798

Merged
merged 8 commits into from
Apr 4, 2023
Merged

Uniform pallet warnings #13798

merged 8 commits into from
Apr 4, 2023

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Apr 1, 2023

Cleaning up the code a bit by using proc-macro-warning and emitting a warning when using a hard-coded weight on a non-dev-mode pallet (#13774).
In cases where dev mode is not possible and you explicitly want to ignore this warning; wrap it in curly brackets like {1}.

This enforces consistent formatting and grammar across different warnings for enhanced developer experience.

warning: use of deprecated constant `pallet::warnings::ImplicitCallIndex::_w`: 
                 It is deprecated to use implicit call indices.
                 Please instead ensure that all calls have the `pallet::call_index` attribute or that the `dev-mode` of the pallet is enabled.
         
                 For more info see:
                     <https://github.com/paritytech/substrate/pull/12891>
                     <https://github.com/paritytech/substrate/pull/11381>
    --> frame/nomination-pools/src/lib.rs:2621:10
     |
2621 |         pub fn claim_commission(origin: OriginFor<T>, pool_id: PoolId) -> DispatchResult {
     |                ^^^^^^^^^^^^^^^^
     |

and

warning: use of deprecated constant `pallet::warnings::ConstantWeight::_w`: 
                 It is deprecated to use hard-coded constant as call weight.
                 Please instead benchmark all calls or put the pallet into `dev` mode.
         
                 For more info see:
                     <https://github.com/paritytech/substrate/pull/13798>
    --> frame/nomination-pools/src/lib.rs:2620:20
     |
2620 |         #[pallet::weight(0)]
     |                          

TODO:

  • Update and add UI tests

We could also think about a registry of known warnings / errors and have shortened links to them like parity.link/frame-err-123 to explain how to resolve them.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
@ggwpez ggwpez changed the title Use proc-macro-warning crate Uniform pallet warnings Apr 1, 2023
@ggwpez ggwpez added the A3-in_progress Pull request is in progress. No review needed at this stage. label Apr 1, 2023
@bkchr
Copy link
Member

bkchr commented Apr 1, 2023

Nice!

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Also renamed some of the odd-named ones.

Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
To be fixed in https://github.com/paritytech/substrate/issues/13813

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 3, 2023

bot rebase

@paritytech-processbot
Copy link

Rebased

@ggwpez ggwpez added A0-please_review Pull request needs code review. C1-low PR touches the given topic and has a low impact on builders. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit B1-note_worthy Changes should be noted in the release notes T1-runtime This PR/Issue is related to the topic “runtime”. and removed A3-in_progress Pull request is in progress. No review needed at this stage. labels Apr 3, 2023
@ggwpez ggwpez requested a review from sam0x17 April 3, 2023 19:36
@@ -120,14 +120,14 @@ pub mod pallet {
#[pallet::call]
impl<T: Config> Pallet<T> {
#[pallet::call_index(0)]
#[pallet::weight(0)]
#[pallet::weight({0})]
Copy link
Member Author

@ggwpez ggwpez Apr 3, 2023

Choose a reason for hiding this comment

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

Note: Some test-only pallets cannot be put in dev mode, since they access the storage info.
But we could add custom lint attributes to suppress certain warnings, similar to how clippy works.

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 is great love that you released this feature as a crate as I'd like to use it for some of my things ^_^

@jsidorenko
Copy link
Contributor

Should the places with the #[pallet::weight(0)] inside the docs text also get changed? E.g. here: https://github.com/paritytech/substrate/blob/master/frame/timestamp/src/lib.rs#L83

@ggwpez
Copy link
Member Author

ggwpez commented Apr 4, 2023

Should the places with the #[pallet::weight(0)] inside the docs text also get changed? E.g. here:

I just tried that locally, and those examples actually compile and have no proper WeightInfo.
So having a warning here for anyone that copy&pastes it is good I think.

@ggwpez
Copy link
Member Author

ggwpez commented Apr 4, 2023

bot merge

@paritytech-processbot paritytech-processbot bot merged commit 2961138 into master Apr 4, 2023
@paritytech-processbot paritytech-processbot bot deleted the oty-pallet-warings branch April 4, 2023 11:32
gpestana pushed a commit that referenced this pull request Apr 23, 2023
* Use proc-macro-warning crate

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

* Fixup

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

* Fix pallet_ui tests

Also renamed some of the odd-named ones.

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

* Update dep

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

* Ignore hardcoded weight warning

To be fixed in https://github.com/paritytech/substrate/issues/13813

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

* Fix test pallet

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

* Fix more tests

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 <>
@Polkadot-Forum
Copy link

This pull request has been mentioned on Polkadot Forum. There might be relevant details there:

https://forum.polkadot.network/t/april-updates-for-substrate-and-polkadot-devs/2764/1

OverOrion added a commit to OverOrion/pallets that referenced this pull request May 26, 2023
OverOrion added a commit to OverOrion/pallets that referenced this pull request May 26, 2023
OverOrion added a commit to integritee-network/pallets that referenced this pull request May 31, 2023
* polkadot update to v0.9.42

* remove deprecated trait Store uses

See paritytech/substrate#13535

* remove deprecated Weight::from_{ref_time, proof_size}

See paritytech/substrate#13475

* allowlist one constant weight

See paritytech/substrate#13798

* add new trait associated types

* polkadot update to v0.9.42 (xcm)

* fixup! remove deprecated trait Store uses

* teerex/mock: add missing trait imports

* claims/tests: fix benchmark as described in substrate/12951

See: paritytech/substrate#12951

* claims/tests: fix claiming_while_vested_doesnt_work by giving it the existential deposit (ED)

Co-authored-by: coax1d <andrewburger1991@gmail.com>

---------

Co-authored-by: coax1d <andrewburger1991@gmail.com>
nathanwhit pushed a commit to nathanwhit/substrate that referenced this pull request Jul 19, 2023
* Use proc-macro-warning crate

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

* Fixup

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

* Fix pallet_ui tests

Also renamed some of the odd-named ones.

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

* Update dep

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

* Ignore hardcoded weight warning

To be fixed in https://github.com/paritytech/substrate/issues/13813

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

* Fix test pallet

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

* Fix more tests

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 <>
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. D3-trivial 🧸 PR contains trivial changes in a runtime directory that do not require an audit T1-runtime This PR/Issue is related to the topic “runtime”.
Projects
Status: Done
Development

Successfully merging this pull request may close these issues.

5 participants