mirror of
https://github.com/pezkuwichain/pezkuwi-subxt.git
synced 2026-05-01 11:17:56 +00:00
[FRAME] Warn on unchecked weight witness (#1818)
Adds a warning to FRAME pallets when a function argument that starts
with `_` is used in the weight formula.
This is in most cases an error since the weight witness needs to be
checked.
Example:
```rust
#[pallet::call_index(0)]
#[pallet::weight(T::SystemWeightInfo::remark(_remark.len() as u32))]
pub fn remark(_origin: OriginFor<T>, _remark: Vec<u8>) -> DispatchResultWithPostInfo {
Ok(().into())
}
```
Produces this warning:
```pre
warning: use of deprecated constant `pallet::warnings::UncheckedWeightWitness_0::_w`:
It is deprecated to not check weight witness data.
Please instead ensure that all witness data for weight calculation is checked before usage.
For more info see:
<https://github.com/paritytech/polkadot-sdk/pull/1818>
--> substrate/frame/system/src/lib.rs:424:40
|
424 | pub fn remark(_origin: OriginFor<T>, _remark: Vec<u8>) -> DispatchResultWithPostInfo {
| ^^^^^^^
|
= note: `#[warn(deprecated)]` on by default
```
Can be suppressed like this, since in this case it is legit:
```rust
#[pallet::call_index(0)]
#[pallet::weight(T::SystemWeightInfo::remark(remark.len() as u32))]
pub fn remark(_origin: OriginFor<T>, remark: Vec<u8>) -> DispatchResultWithPostInfo {
let _ = remark; // We dont need to check the weight witness.
Ok(().into())
}
```
Changes:
- Add warning on uncheded weight witness
- Respect `subkeys` limit in `System::kill_prefix`
- Fix HRMP pallet and other warnings
- Update`proc_macro_warning` dependency
- Delete random folder `substrate/src/src` 🙈
- Adding Prdoc
---------
Signed-off-by: Oliver Tale-Yazdi <oliver.tale-yazdi@parity.io>
Co-authored-by: joe petrowski <25483142+joepetrowski@users.noreply.github.com>
This commit is contained in:
committed by
GitHub
parent
e3c97e4860
commit
64877492c5
@@ -414,7 +414,7 @@ fn construct_runtime_final_expansion(
|
||||
)
|
||||
.help_links(&["https://github.com/paritytech/substrate/pull/14437"])
|
||||
.span(where_section.span)
|
||||
.build(),
|
||||
.build_or_panic(),
|
||||
)
|
||||
});
|
||||
|
||||
|
||||
@@ -17,12 +17,14 @@
|
||||
|
||||
use crate::{
|
||||
pallet::{
|
||||
expand::warnings::{weight_constant_warning, weight_witness_warning},
|
||||
parse::call::{CallVariantDef, CallWeightDef},
|
||||
Def,
|
||||
},
|
||||
COUNTER,
|
||||
};
|
||||
use proc_macro2::TokenStream as TokenStream2;
|
||||
use proc_macro_warning::Warning;
|
||||
use quote::{quote, ToTokens};
|
||||
use syn::spanned::Spanned;
|
||||
|
||||
@@ -68,7 +70,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
|
||||
continue
|
||||
}
|
||||
|
||||
let warning = proc_macro_warning::Warning::new_deprecated("ImplicitCallIndex")
|
||||
let warning = Warning::new_deprecated("ImplicitCallIndex")
|
||||
.index(call_index_warnings.len())
|
||||
.old("use implicit call indices")
|
||||
.new("ensure that all calls have a `pallet::call_index` attribute or put the pallet into `dev` mode")
|
||||
@@ -77,7 +79,7 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
|
||||
"https://github.com/paritytech/substrate/pull/11381"
|
||||
])
|
||||
.span(method.name.span())
|
||||
.build();
|
||||
.build_or_panic();
|
||||
call_index_warnings.push(warning);
|
||||
}
|
||||
|
||||
@@ -86,18 +88,12 @@ pub fn expand_call(def: &mut Def) -> proc_macro2::TokenStream {
|
||||
for method in &methods {
|
||||
match &method.weight {
|
||||
CallWeightDef::DevModeDefault => fn_weight.push(syn::parse_quote!(0)),
|
||||
CallWeightDef::Immediate(e @ syn::Expr::Lit(lit)) if !def.dev_mode => {
|
||||
let warning = proc_macro_warning::Warning::new_deprecated("ConstantWeight")
|
||||
.index(weight_warnings.len())
|
||||
.old("use hard-coded constant as call weight")
|
||||
.new("benchmark all calls or put the pallet into `dev` mode")
|
||||
.help_link("https://github.com/paritytech/substrate/pull/13798")
|
||||
.span(lit.span())
|
||||
.build();
|
||||
weight_warnings.push(warning);
|
||||
CallWeightDef::Immediate(e) => {
|
||||
weight_constant_warning(e, def.dev_mode, &mut weight_warnings);
|
||||
weight_witness_warning(method, def.dev_mode, &mut weight_warnings);
|
||||
|
||||
fn_weight.push(e.into_token_stream());
|
||||
},
|
||||
CallWeightDef::Immediate(e) => fn_weight.push(e.into_token_stream()),
|
||||
CallWeightDef::Inherited => {
|
||||
let pallet_weight = def
|
||||
.call
|
||||
|
||||
@@ -34,6 +34,7 @@ mod store_trait;
|
||||
mod tt_default_parts;
|
||||
mod type_value;
|
||||
mod validate_unsigned;
|
||||
mod warnings;
|
||||
|
||||
use crate::pallet::Def;
|
||||
use quote::ToTokens;
|
||||
|
||||
@@ -0,0 +1,102 @@
|
||||
// This file is part of Substrate.
|
||||
|
||||
// Copyright (C) Parity Technologies (UK) Ltd.
|
||||
// SPDX-License-Identifier: Apache-2.0
|
||||
|
||||
// Licensed under the Apache License, Version 2.0 (the "License");
|
||||
// you may not use this file except in compliance with the License.
|
||||
// You may obtain a copy of the License at
|
||||
//
|
||||
// http://www.apache.org/licenses/LICENSE-2.0
|
||||
//
|
||||
// Unless required by applicable law or agreed to in writing, software
|
||||
// distributed under the License is distributed on an "AS IS" BASIS,
|
||||
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
|
||||
// See the License for the specific language governing permissions and
|
||||
// limitations under the License.
|
||||
|
||||
//! Generates warnings for undesirable pallet code.
|
||||
|
||||
use crate::pallet::parse::call::{CallVariantDef, CallWeightDef};
|
||||
use proc_macro_warning::Warning;
|
||||
use syn::{
|
||||
spanned::Spanned,
|
||||
visit::{self, Visit},
|
||||
};
|
||||
|
||||
/// Warn if any of the call arguments starts with a underscore and is used in a weight formula.
|
||||
pub(crate) fn weight_witness_warning(
|
||||
method: &CallVariantDef,
|
||||
dev_mode: bool,
|
||||
warnings: &mut Vec<Warning>,
|
||||
) {
|
||||
if dev_mode {
|
||||
return
|
||||
}
|
||||
let CallWeightDef::Immediate(w) = &method.weight else {
|
||||
return;
|
||||
};
|
||||
|
||||
let partial_warning = Warning::new_deprecated("UncheckedWeightWitness")
|
||||
.old("not check weight witness data")
|
||||
.new("ensure that all witness data for weight calculation is checked before usage")
|
||||
.help_link("https://github.com/paritytech/polkadot-sdk/pull/1818");
|
||||
|
||||
for (_, arg_ident, _) in method.args.iter() {
|
||||
if !arg_ident.to_string().starts_with('_') || !contains_ident(w.clone(), &arg_ident) {
|
||||
continue
|
||||
}
|
||||
|
||||
let warning = partial_warning
|
||||
.clone()
|
||||
.index(warnings.len())
|
||||
.span(arg_ident.span())
|
||||
.build_or_panic();
|
||||
|
||||
warnings.push(warning);
|
||||
}
|
||||
}
|
||||
|
||||
/// Warn if the weight is a constant and the pallet not in `dev_mode`.
|
||||
pub(crate) fn weight_constant_warning(
|
||||
weight: &syn::Expr,
|
||||
dev_mode: bool,
|
||||
warnings: &mut Vec<Warning>,
|
||||
) {
|
||||
if dev_mode {
|
||||
return
|
||||
}
|
||||
let syn::Expr::Lit(lit) = weight else {
|
||||
return;
|
||||
};
|
||||
|
||||
let warning = Warning::new_deprecated("ConstantWeight")
|
||||
.index(warnings.len())
|
||||
.old("use hard-coded constant as call weight")
|
||||
.new("benchmark all calls or put the pallet into `dev` mode")
|
||||
.help_link("https://github.com/paritytech/substrate/pull/13798")
|
||||
.span(lit.span())
|
||||
.build_or_panic();
|
||||
|
||||
warnings.push(warning);
|
||||
}
|
||||
|
||||
/// Returns whether `expr` contains `ident`.
|
||||
fn contains_ident(mut expr: syn::Expr, ident: &syn::Ident) -> bool {
|
||||
struct ContainsIdent {
|
||||
ident: syn::Ident,
|
||||
found: bool,
|
||||
}
|
||||
|
||||
impl<'a> Visit<'a> for ContainsIdent {
|
||||
fn visit_ident(&mut self, i: &syn::Ident) {
|
||||
if *i == self.ident {
|
||||
self.found = true;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
let mut visitor = ContainsIdent { ident: ident.clone(), found: false };
|
||||
visit::visit_expr(&mut visitor, &mut expr);
|
||||
visitor.found
|
||||
}
|
||||
Reference in New Issue
Block a user