From e5694501bf45805756952c05c765cdf667969488 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Wed, 10 Nov 2021 21:51:16 -0800 Subject: [PATCH 01/75] Allow pallet errors to contain at most one field --- .../support/procedural/src/pallet/parse/error.rs | 16 +++++++++++----- ...o_fieldless.rs => error_more_than_1_field.rs} | 1 + .../pallet_ui/error_more_than_1_field.stderr | 5 +++++ .../tests/pallet_ui/error_no_fieldless.stderr | 5 ----- 4 files changed, 17 insertions(+), 10 deletions(-) rename frame/support/test/tests/pallet_ui/{error_no_fieldless.rs => error_more_than_1_field.rs} (96%) create mode 100644 frame/support/test/tests/pallet_ui/error_more_than_1_field.stderr delete mode 100644 frame/support/test/tests/pallet_ui/error_no_fieldless.stderr diff --git a/frame/support/procedural/src/pallet/parse/error.rs b/frame/support/procedural/src/pallet/parse/error.rs index 9c9a95105c53c..439ed9a211905 100644 --- a/frame/support/procedural/src/pallet/parse/error.rs +++ b/frame/support/procedural/src/pallet/parse/error.rs @@ -18,7 +18,7 @@ use super::helper; use frame_support_procedural_tools::get_doc_literals; use quote::ToTokens; -use syn::spanned::Spanned; +use syn::{Fields, spanned::Spanned}; /// List of additional token to be used for parsing. mod keyword { @@ -70,12 +70,18 @@ impl ErrorDef { .variants .iter() .map(|variant| { - if !matches!(variant.fields, syn::Fields::Unit) { - let msg = "Invalid pallet::error, unexpected fields, must be `Unit`"; - return Err(syn::Error::new(variant.fields.span(), msg)) + match &variant.fields { + Fields::Unit => {}, + Fields::Named(f) if f.named.len() < 2 => {}, + Fields::Unnamed(u) if u.unnamed.len() < 2 => {}, + _ => { + let msg = "Invalid pallet::error, unexpected fields, must be `Unit` or \ + contain only 1 field"; + return Err(syn::Error::new(variant.fields.span(), msg)) + } } if variant.discriminant.is_some() { - let msg = "Invalid pallet::error, unexpected discriminant, discriminant \ + let msg = "Invalid pallet::error, unexpected discriminant, discriminants \ are not supported"; let span = variant.discriminant.as_ref().unwrap().0.span(); return Err(syn::Error::new(span, msg)) diff --git a/frame/support/test/tests/pallet_ui/error_no_fieldless.rs b/frame/support/test/tests/pallet_ui/error_more_than_1_field.rs similarity index 96% rename from frame/support/test/tests/pallet_ui/error_no_fieldless.rs rename to frame/support/test/tests/pallet_ui/error_more_than_1_field.rs index c9d444d6f90dd..e972d9c205af3 100644 --- a/frame/support/test/tests/pallet_ui/error_no_fieldless.rs +++ b/frame/support/test/tests/pallet_ui/error_more_than_1_field.rs @@ -17,6 +17,7 @@ mod pallet { #[pallet::error] pub enum Error { + Tuple(u8, u8), U8(u8), } } diff --git a/frame/support/test/tests/pallet_ui/error_more_than_1_field.stderr b/frame/support/test/tests/pallet_ui/error_more_than_1_field.stderr new file mode 100644 index 0000000000000..f764db17a29b0 --- /dev/null +++ b/frame/support/test/tests/pallet_ui/error_more_than_1_field.stderr @@ -0,0 +1,5 @@ +error: Invalid pallet::error, unexpected fields, must be `Unit` or contain only 1 field + --> $DIR/error_more_than_1_field.rs:20:8 + | +20 | Tuple(u8, u8), + | ^^^^^^^^ diff --git a/frame/support/test/tests/pallet_ui/error_no_fieldless.stderr b/frame/support/test/tests/pallet_ui/error_no_fieldless.stderr deleted file mode 100644 index 1d69fbeff9aac..0000000000000 --- a/frame/support/test/tests/pallet_ui/error_no_fieldless.stderr +++ /dev/null @@ -1,5 +0,0 @@ -error: Invalid pallet::error, unexpected fields, must be `Unit` - --> $DIR/error_no_fieldless.rs:20:5 - | -20 | U8(u8), - | ^^^^ From c4f7b5163714328afee772e7fc15b9083bc3fd15 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 11 Nov 2021 13:44:57 -0800 Subject: [PATCH 02/75] Update docs on pallet::error --- frame/support/src/lib.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index d81300a404c4f..4cc905338fccd 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1613,11 +1613,15 @@ pub mod pallet_prelude { /// #[pallet::error] /// pub enum Error { /// /// $some_optional_doc -/// $SomeFieldLessVariant, +/// $SomeFieldLessOr1FieldVariant, /// ... /// } /// ``` -/// I.e. a regular rust enum named `Error`, with generic `T` and fieldless variants. +/// I.e. a regular rust enum named `Error`, with generic `T` and fieldless or single-field +/// variants. +/// Any field in the enum variants must implement `scale_info::TypeInfo` in order to be properly +/// used in the metadata, and its encoded size should be as small as possible, preferably 1 byte +/// in size. /// The generic `T` mustn't bound anything and where clause is not allowed. But bounds and /// where clause shouldn't be needed for any usecase. /// From 788ea42880388395724c9bac4d19d1dc026e85d8 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 11 Nov 2021 14:03:04 -0800 Subject: [PATCH 03/75] Reword documentation --- frame/support/src/lib.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 4cc905338fccd..fd01637acbc1f 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1613,7 +1613,9 @@ pub mod pallet_prelude { /// #[pallet::error] /// pub enum Error { /// /// $some_optional_doc -/// $SomeFieldLessOr1FieldVariant, +/// $SomeFieldLessVariant, +/// /// $some_more_optional_doc +/// $SomeSingleFieldVariant(FieldType), /// ... /// } /// ``` From 0b70512c6f5b796af06026a61624b3d16a4d0d68 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 11 Nov 2021 14:40:16 -0800 Subject: [PATCH 04/75] cargo fmt --- frame/support/procedural/src/pallet/parse/error.rs | 4 ++-- frame/support/src/lib.rs | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/frame/support/procedural/src/pallet/parse/error.rs b/frame/support/procedural/src/pallet/parse/error.rs index 439ed9a211905..6b74a14fc45f3 100644 --- a/frame/support/procedural/src/pallet/parse/error.rs +++ b/frame/support/procedural/src/pallet/parse/error.rs @@ -18,7 +18,7 @@ use super::helper; use frame_support_procedural_tools::get_doc_literals; use quote::ToTokens; -use syn::{Fields, spanned::Spanned}; +use syn::{spanned::Spanned, Fields}; /// List of additional token to be used for parsing. mod keyword { @@ -78,7 +78,7 @@ impl ErrorDef { let msg = "Invalid pallet::error, unexpected fields, must be `Unit` or \ contain only 1 field"; return Err(syn::Error::new(variant.fields.span(), msg)) - } + }, } if variant.discriminant.is_some() { let msg = "Invalid pallet::error, unexpected discriminant, discriminants \ diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index fd01637acbc1f..9358e21222a3c 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1621,9 +1621,9 @@ pub mod pallet_prelude { /// ``` /// I.e. a regular rust enum named `Error`, with generic `T` and fieldless or single-field /// variants. -/// Any field in the enum variants must implement `scale_info::TypeInfo` in order to be properly -/// used in the metadata, and its encoded size should be as small as possible, preferably 1 byte -/// in size. +/// Any field in the enum variants must implement `scale_info::TypeInfo` in order to be +/// properly used in the metadata, and its encoded size should be as small as possible, +/// preferably 1 byte in size. /// The generic `T` mustn't bound anything and where clause is not allowed. But bounds and /// where clause shouldn't be needed for any usecase. /// From d3e5c7cf5655c10f77471fe0529718b4377d5553 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 18 Nov 2021 01:24:07 -0800 Subject: [PATCH 05/75] Introduce CompactPalletError trait and require #[pallet::error] fields to implement them --- .../procedural/src/pallet/expand/error.rs | 39 ++++++++++++++--- .../procedural/src/pallet/parse/error.rs | 18 ++++---- .../src/pallet/parse/pallet_struct.rs | 4 +- frame/support/src/traits.rs | 3 ++ frame/support/src/traits/error.rs | 43 +++++++++++++++++++ .../test/tests/pallet_ui/error_not_compact.rs | 19 ++++++++ .../tests/pallet_ui/error_not_compact.stderr | 12 ++++++ 7 files changed, 123 insertions(+), 15 deletions(-) create mode 100644 frame/support/src/traits/error.rs create mode 100644 frame/support/test/tests/pallet_ui/error_not_compact.rs create mode 100644 frame/support/test/tests/pallet_ui/error_not_compact.stderr diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index c6925db07a26f..d8cd5f366c243 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -39,13 +39,21 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { ) ); - let as_u8_matches = error.variants.iter().enumerate().map( - |(i, (variant, _))| quote::quote_spanned!(error.attr_span => Self::#variant => #i as u8,), - ); + let as_u8_matches = error.variants.iter().enumerate().map(|(i, (variant, field_ty, _))| { + if field_ty.is_some() { + quote::quote_spanned!(error.attr_span => Self::#variant(..) => #i as u8,) + } else { + quote::quote_spanned!(error.attr_span => Self::#variant => #i as u8,) + } + }); - let as_str_matches = error.variants.iter().map(|(variant, _)| { + let as_str_matches = error.variants.iter().map(|(variant, field_ty, _)| { let variant_str = format!("{}", variant); - quote::quote_spanned!(error.attr_span => Self::#variant => #variant_str,) + if field_ty.is_some() { + quote::quote_spanned!(error.attr_span => Self::#variant(..) => #variant_str,) + } else { + quote::quote_spanned!(error.attr_span => Self::#variant => #variant_str,) + } }); let error_item = { @@ -75,6 +83,20 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { )); } + let field_tys = error + .variants + .iter() + .filter_map(|(_, field_ty, _)| field_ty.as_ref()) + .collect::>(); + + let compactness_check = if field_tys.is_empty() { + quote::quote!(true) + } else { + quote::quote! { + #( <#field_tys as #frame_support::traits::CompactPalletError>::check_compactness() )&&* + } + }; + quote::quote_spanned!(error.attr_span => impl<#type_impl_gen> #frame_support::sp_std::fmt::Debug for #error_ident<#type_use_gen> #config_where_clause @@ -128,5 +150,12 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { } } } + + impl<#type_impl_gen> #frame_support::traits::CompactPalletError + for #error_ident<#type_use_gen> + #config_where_clause + { + fn check_compactness() -> bool { #compactness_check } + } ) } diff --git a/frame/support/procedural/src/pallet/parse/error.rs b/frame/support/procedural/src/pallet/parse/error.rs index 6b74a14fc45f3..14783c9001ae3 100644 --- a/frame/support/procedural/src/pallet/parse/error.rs +++ b/frame/support/procedural/src/pallet/parse/error.rs @@ -30,8 +30,8 @@ mod keyword { pub struct ErrorDef { /// The index of error item in pallet module. pub index: usize, - /// Variants ident and doc literals (ordered as declaration order) - pub variants: Vec<(syn::Ident, Vec)>, + /// Variants ident, optional field and doc literals (ordered as declaration order) + pub variants: Vec<(syn::Ident, Option, Vec)>, /// A set of usage of instance, must be check for consistency with trait. pub instances: Vec, /// The keyword error used (contains span). @@ -70,16 +70,18 @@ impl ErrorDef { .variants .iter() .map(|variant| { - match &variant.fields { - Fields::Unit => {}, - Fields::Named(f) if f.named.len() < 2 => {}, - Fields::Unnamed(u) if u.unnamed.len() < 2 => {}, + let field_ty = match &variant.fields { + Fields::Unit => None, + Fields::Named(f) if f.named.len() == 1 => + Some(f.named.first().unwrap().ty.clone()), + Fields::Unnamed(u) if u.unnamed.len() == 1 => + Some(u.unnamed.first().unwrap().ty.clone()), _ => { let msg = "Invalid pallet::error, unexpected fields, must be `Unit` or \ contain only 1 field"; return Err(syn::Error::new(variant.fields.span(), msg)) }, - } + }; if variant.discriminant.is_some() { let msg = "Invalid pallet::error, unexpected discriminant, discriminants \ are not supported"; @@ -87,7 +89,7 @@ impl ErrorDef { return Err(syn::Error::new(span, msg)) } - Ok((variant.ident.clone(), get_doc_literals(&variant.attrs))) + Ok((variant.ident.clone(), field_ty, get_doc_literals(&variant.attrs))) }) .collect::>()?; diff --git a/frame/support/procedural/src/pallet/parse/pallet_struct.rs b/frame/support/procedural/src/pallet/parse/pallet_struct.rs index 278f46e13818e..c528faf669ee3 100644 --- a/frame/support/procedural/src/pallet/parse/pallet_struct.rs +++ b/frame/support/procedural/src/pallet/parse/pallet_struct.rs @@ -130,12 +130,12 @@ impl PalletStructDef { if generate_storage_info.is_none() => { generate_storage_info = Some(span); - } + }, PalletStructAttr::StorageVersion { storage_version, .. } if storage_version_found.is_none() => { storage_version_found = Some(storage_version); - } + }, attr => { let msg = "Unexpected duplicated attribute"; return Err(syn::Error::new(attr.span(), msg)) diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index bb990e25646db..1d70f9e1ec9cf 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -45,6 +45,9 @@ pub use validation::{ ValidatorSetWithIdentification, VerifySeal, }; +mod error; +pub use error::CompactPalletError; + mod filter; pub use filter::{ClearFilterGuard, FilterStack, FilterStackGuard, InstanceFilter, IntegrityTest}; diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs new file mode 100644 index 0000000000000..6541d58ceab37 --- /dev/null +++ b/frame/support/src/traits/error.rs @@ -0,0 +1,43 @@ +// This file is part of Substrate. + +// Copyright (C) 2021 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. + +//! Traits for describing and constraining pallet error types. + +use scale_info::TypeInfo; + +/// Trait denoting that the implementing type has the most compact encoded size that is fit to be +/// included as a field in a variant of the `#[pallet::error]` enum type. +pub trait CompactPalletError: TypeInfo { + /// Function that checks whether implementing types are either 1 bytes in size, or that its + /// nested types are 1 bytes in size, i.e. whether they are as memory efficient as possible. + /// + /// It is up to the implementing type to prove that it is maximally compact, thus this + /// function defaults to false. + fn check_compactness() -> bool { false } +} + +macro_rules! impl_for_types { + ($($typ:ty),+) => { + $( + impl CompactPalletError for $typ { + fn check_compactness() -> bool { true } + } + )+ + }; +} + +impl_for_types!(u8, i8, bool, char, Option); diff --git a/frame/support/test/tests/pallet_ui/error_not_compact.rs b/frame/support/test/tests/pallet_ui/error_not_compact.rs new file mode 100644 index 0000000000000..d85e6c32ebb8a --- /dev/null +++ b/frame/support/test/tests/pallet_ui/error_not_compact.rs @@ -0,0 +1,19 @@ +#[frame_support::pallet] +mod pallet { + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::pallet] + pub struct Pallet(core::marker::PhantomData); + + #[pallet::error] + pub enum Error { + CustomError(crate::MyError), + } +} + +#[derive(scale_info::TypeInfo)] +enum MyError {} + +fn main() { +} diff --git a/frame/support/test/tests/pallet_ui/error_not_compact.stderr b/frame/support/test/tests/pallet_ui/error_not_compact.stderr new file mode 100644 index 0000000000000..02c1ca8f0863a --- /dev/null +++ b/frame/support/test/tests/pallet_ui/error_not_compact.stderr @@ -0,0 +1,12 @@ +error[E0277]: the trait bound `MyError: CompactPalletError` is not satisfied + --> $DIR/error_not_compact.rs:1:1 + | +1 | #[frame_support::pallet] + | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `CompactPalletError` is not implemented for `MyError` + | +note: required by `check_compactness` + --> $DIR/error.rs:30:2 + | +30 | fn check_compactness() -> bool { false } + | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ + = note: this error originates in the attribute macro `frame_support::pallet` (in Nightly builds, run with -Z macro-backtrace for more info) From e8f671419eb8a7654666f8cbc68b3a43a84262ae Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 18 Nov 2021 14:02:22 -0800 Subject: [PATCH 06/75] cargo fmt --- client/consensus/babe/src/verification.rs | 4 ++-- client/network/src/protocol/notifications/behaviour.rs | 4 ++-- client/network/src/protocol/sync/blocks.rs | 2 +- client/network/src/service/tests.rs | 2 +- client/network/src/transactions.rs | 4 ++-- frame/election-provider-multi-phase/src/lib.rs | 2 +- frame/support/src/traits/error.rs | 4 +++- 7 files changed, 12 insertions(+), 10 deletions(-) diff --git a/client/consensus/babe/src/verification.rs b/client/consensus/babe/src/verification.rs index af118312dd07c..1554fa6de31be 100644 --- a/client/consensus/babe/src/verification.rs +++ b/client/consensus/babe/src/verification.rs @@ -114,7 +114,7 @@ where ); check_secondary_plain_header::(pre_hash, secondary, sig, &epoch)?; - } + }, PreDigest::SecondaryVRF(secondary) if epoch.config.allowed_slots.is_secondary_vrf_slots_allowed() => { @@ -125,7 +125,7 @@ where ); check_secondary_vrf_header::(pre_hash, secondary, sig, &epoch)?; - } + }, _ => return Err(babe_err(Error::SecondarySlotAssignmentsDisabled)), } diff --git a/client/network/src/protocol/notifications/behaviour.rs b/client/network/src/protocol/notifications/behaviour.rs index 01138e3207570..f66f1fbe9e95a 100644 --- a/client/network/src/protocol/notifications/behaviour.rs +++ b/client/network/src/protocol/notifications/behaviour.rs @@ -712,7 +712,7 @@ impl Notifications { timer: delay_id, timer_deadline: *backoff, }; - } + }, // Disabled => Enabled PeerState::Disabled { mut connections, backoff_until } => { @@ -2085,7 +2085,7 @@ impl NetworkBehaviour for Notifications { .boxed(), ); } - } + }, // We intentionally never remove elements from `delays`, and it may // thus contain obsolete entries. This is a normal situation. diff --git a/client/network/src/protocol/sync/blocks.rs b/client/network/src/protocol/sync/blocks.rs index 30ba7ffafeffc..ce4535dc0b45f 100644 --- a/client/network/src/protocol/sync/blocks.rs +++ b/client/network/src/protocol/sync/blocks.rs @@ -203,7 +203,7 @@ impl BlockCollection { { *downloading -= 1; false - } + }, Some(&mut BlockRangeState::Downloading { .. }) => true, _ => false, }; diff --git a/client/network/src/service/tests.rs b/client/network/src/service/tests.rs index 69b172d07edfe..87e481dc87f2d 100644 --- a/client/network/src/service/tests.rs +++ b/client/network/src/service/tests.rs @@ -530,7 +530,7 @@ fn fallback_name_working() { { assert_eq!(negotiated_fallback, Some(PROTOCOL_NAME)); break - } + }, _ => {}, }; } diff --git a/client/network/src/transactions.rs b/client/network/src/transactions.rs index 99350f603a375..6d190651160f0 100644 --- a/client/network/src/transactions.rs +++ b/client/network/src/transactions.rs @@ -336,13 +336,13 @@ impl TransactionsHandler { }, ); debug_assert!(_was_in.is_none()); - } + }, Event::NotificationStreamClosed { remote, protocol } if protocol == self.protocol_name => { let _peer = self.peers.remove(&remote); debug_assert!(_peer.is_some()); - } + }, Event::NotificationsReceived { remote, messages } => { for (protocol, message) in messages { diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index a7863fafa7747..ab8b1523f0509 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -772,7 +772,7 @@ pub mod pallet { Self::on_initialize_open_unsigned(enabled, now); T::WeightInfo::on_initialize_open_unsigned() } - } + }, _ => T::WeightInfo::on_initialize_nothing(), } } diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index 6541d58ceab37..4ac36af6d3b4c 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -27,7 +27,9 @@ pub trait CompactPalletError: TypeInfo { /// /// It is up to the implementing type to prove that it is maximally compact, thus this /// function defaults to false. - fn check_compactness() -> bool { false } + fn check_compactness() -> bool { + false + } } macro_rules! impl_for_types { From 815a61661e9addd6468326b50372bc49355c2d0b Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 18 Nov 2021 17:36:56 -0800 Subject: [PATCH 07/75] Do not assume tuple variants --- .../procedural/src/pallet/expand/error.rs | 43 +++++++++++++------ .../procedural/src/pallet/parse/error.rs | 22 +++++++--- .../tests/pallet_ui/error_not_compact.stderr | 2 +- 3 files changed, 48 insertions(+), 19 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index d8cd5f366c243..6632410f9a6c1 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -15,7 +15,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::pallet::Def; +use crate::pallet::{parse::error::VariantField, Def}; use frame_support_procedural_tools::get_doc_literals; /// @@ -39,20 +39,35 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { ) ); - let as_u8_matches = error.variants.iter().enumerate().map(|(i, (variant, field_ty, _))| { - if field_ty.is_some() { - quote::quote_spanned!(error.attr_span => Self::#variant(..) => #i as u8,) - } else { - quote::quote_spanned!(error.attr_span => Self::#variant => #i as u8,) - } - }); + let as_u8_matches = + error + .variants + .iter() + .enumerate() + .map(|(i, (variant, field_ty, _))| match field_ty { + Some(VariantField { is_named: true, .. }) => { + quote::quote_spanned!(error.attr_span => Self::#variant { .. } => #i as u8,) + }, + Some(VariantField { is_named: false, .. }) => { + quote::quote_spanned!(error.attr_span => Self::#variant(..) => #i as u8,) + }, + None => { + quote::quote_spanned!(error.attr_span => Self::#variant => #i as u8,) + }, + }); let as_str_matches = error.variants.iter().map(|(variant, field_ty, _)| { let variant_str = format!("{}", variant); - if field_ty.is_some() { - quote::quote_spanned!(error.attr_span => Self::#variant(..) => #variant_str,) - } else { - quote::quote_spanned!(error.attr_span => Self::#variant => #variant_str,) + match field_ty { + Some(VariantField { is_named: true, .. }) => { + quote::quote_spanned!(error.attr_span => Self::#variant { .. } => #variant_str,) + }, + Some(VariantField { is_named: false, .. }) => { + quote::quote_spanned!(error.attr_span => Self::#variant(..) => #variant_str,) + }, + None => { + quote::quote_spanned!(error.attr_span => Self::#variant => #variant_str,) + }, } }); @@ -86,7 +101,9 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { let field_tys = error .variants .iter() - .filter_map(|(_, field_ty, _)| field_ty.as_ref()) + .filter_map(|(_, variant_field, _)| { + variant_field.as_ref().map(|VariantField { ty, .. }| ty) + }) .collect::>(); let compactness_check = if field_tys.is_empty() { diff --git a/frame/support/procedural/src/pallet/parse/error.rs b/frame/support/procedural/src/pallet/parse/error.rs index 14783c9001ae3..deafac6d30f6a 100644 --- a/frame/support/procedural/src/pallet/parse/error.rs +++ b/frame/support/procedural/src/pallet/parse/error.rs @@ -25,13 +25,21 @@ mod keyword { syn::custom_keyword!(Error); } +/// Records information about the error enum variants. +pub struct VariantField { + /// The type of the field in the variant. + pub ty: syn::Type, + /// Whether or not the field is named, i.e. whether it is a tuple variant or struct variant. + pub is_named: bool, +} + /// This checks error declaration as a enum declaration with only variants without fields nor /// discriminant. pub struct ErrorDef { /// The index of error item in pallet module. pub index: usize, /// Variants ident, optional field and doc literals (ordered as declaration order) - pub variants: Vec<(syn::Ident, Option, Vec)>, + pub variants: Vec<(syn::Ident, Option, Vec)>, /// A set of usage of instance, must be check for consistency with trait. pub instances: Vec, /// The keyword error used (contains span). @@ -72,10 +80,14 @@ impl ErrorDef { .map(|variant| { let field_ty = match &variant.fields { Fields::Unit => None, - Fields::Named(f) if f.named.len() == 1 => - Some(f.named.first().unwrap().ty.clone()), - Fields::Unnamed(u) if u.unnamed.len() == 1 => - Some(u.unnamed.first().unwrap().ty.clone()), + Fields::Named(f) if f.named.len() == 1 => Some(VariantField { + ty: f.named.first().unwrap().ty.clone(), + is_named: true, + }), + Fields::Unnamed(u) if u.unnamed.len() == 1 => Some(VariantField { + ty: u.unnamed.first().unwrap().ty.clone(), + is_named: false, + }), _ => { let msg = "Invalid pallet::error, unexpected fields, must be `Unit` or \ contain only 1 field"; diff --git a/frame/support/test/tests/pallet_ui/error_not_compact.stderr b/frame/support/test/tests/pallet_ui/error_not_compact.stderr index 02c1ca8f0863a..18b056f26310a 100644 --- a/frame/support/test/tests/pallet_ui/error_not_compact.stderr +++ b/frame/support/test/tests/pallet_ui/error_not_compact.stderr @@ -7,6 +7,6 @@ error[E0277]: the trait bound `MyError: CompactPalletError` is not satisfied note: required by `check_compactness` --> $DIR/error.rs:30:2 | -30 | fn check_compactness() -> bool { false } +30 | fn check_compactness() -> bool { | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ = note: this error originates in the attribute macro `frame_support::pallet` (in Nightly builds, run with -Z macro-backtrace for more info) From 78ff9afd21e1cec9ace154b856def7ae9acaef8a Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Fri, 19 Nov 2021 14:47:54 -0800 Subject: [PATCH 08/75] Add CompactPalletError derive macro --- .../procedural/src/compact_pallet_error.rs | 115 ++++++++++++++++++ frame/support/procedural/src/lib.rs | 6 + frame/support/src/lib.rs | 3 +- .../pallet_ui/pass/error_nested_types.rs | 38 ++++++ 4 files changed, 161 insertions(+), 1 deletion(-) create mode 100644 frame/support/procedural/src/compact_pallet_error.rs create mode 100644 frame/support/test/tests/pallet_ui/pass/error_nested_types.rs diff --git a/frame/support/procedural/src/compact_pallet_error.rs b/frame/support/procedural/src/compact_pallet_error.rs new file mode 100644 index 0000000000000..62d0b73f1d033 --- /dev/null +++ b/frame/support/procedural/src/compact_pallet_error.rs @@ -0,0 +1,115 @@ +// This file is part of Substrate. + +// Copyright (C) 2021 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. + +use frame_support_procedural_tools::generate_crate_access_2018; +use std::convert::identity; + +// Derive `CompactPalletError` +pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenStream { + let syn::DeriveInput { ident: name, generics, data, .. } = match syn::parse(input) { + Ok(input) => input, + Err(e) => return e.to_compile_error().into(), + }; + + let frame_support = match generate_crate_access_2018("frame-support") { + Ok(c) => c, + Err(e) => return e.into_compile_error().into(), + }; + let frame_support = &frame_support; + let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); + + let compactness_check = match data { + syn::Data::Struct(syn::DataStruct { struct_token, fields, .. }) => { + if fields.len() > 1 { + let msg = "Cannot derive `CompactPalletError` for structs with more than 1 field"; + return syn::Error::new(struct_token.span, msg).into_compile_error().into() + } + + match fields { + syn::Fields::Named(mut f) if f.named.len() == 1 => { + let field_ty = f.named.pop().unwrap().into_value().ty; + quote::quote! { + < + #field_ty as #frame_support::traits::CompactPalletError + >::check_compactness() + } + }, + syn::Fields::Unnamed(mut f) if f.unnamed.len() == 1 => { + let field_ty = f.unnamed.pop().unwrap().into_value().ty; + quote::quote! { + < + #field_ty as #frame_support::traits::CompactPalletError + >::check_compactness() + } + }, + _ => quote::quote!(true), + } + }, + syn::Data::Enum(syn::DataEnum { variants, .. }) => { + let field_tys = variants + .into_iter() + .map(|variant| match variant.fields { + syn::Fields::Named(mut f) if f.named.len() == 1 => + Ok(Some(f.named.pop().unwrap().into_value().ty)), + syn::Fields::Unnamed(mut f) if f.unnamed.len() == 1 => + Ok(Some(f.unnamed.pop().unwrap().into_value().ty)), + syn::Fields::Unit => Ok(None), + _ => { + let msg = "Cannot derive `CompactPalletError` for enum with variants \ + containing more than 1 field"; + let err = syn::Error::new(variant.ident.span(), msg); + Err(err) + }, + }) + .collect::>, syn::Error>>(); + + let field_tys = match field_tys { + Ok(tys) => tys.into_iter().filter_map(identity).collect::>(), + Err(e) => return e.to_compile_error().into(), + }; + + if field_tys.is_empty() { + quote::quote!(true) + } else { + quote::quote! { + #( + < + #field_tys as #frame_support::traits::CompactPalletError + >::check_compactness() + )&&* + } + } + }, + syn::Data::Union(syn::DataUnion { union_token, .. }) => { + let msg = "Cannot derive `CompactPalletError` for union; please implement it directly"; + return syn::Error::new(union_token.span, msg).into_compile_error().into() + }, + }; + + quote::quote!( + const _: () = { + impl #impl_generics #frame_support::traits::CompactPalletError + for #name #ty_generics #where_clause + { + fn check_compactness() -> bool { + #compactness_check + } + } + }; + ) + .into() +} diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index d01bbf6ace526..f283f50ffc5f8 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -20,6 +20,7 @@ #![recursion_limit = "512"] mod clone_no_bound; +mod compact_pallet_error; mod construct_runtime; mod crate_version; mod debug_no_bound; @@ -562,3 +563,8 @@ pub fn __generate_dummy_part_checker(input: TokenStream) -> TokenStream { pub fn match_and_insert(input: TokenStream) -> TokenStream { match_and_insert::match_and_insert(input) } + +#[proc_macro_derive(CompactPalletError)] +pub fn derive_compact_pallet_error(input: TokenStream) -> TokenStream { + compact_pallet_error::derive_compact_pallet_error(input) +} diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 9358e21222a3c..7649ec80c7830 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -575,7 +575,8 @@ pub fn debug(data: &impl sp_std::fmt::Debug) { #[doc(inline)] pub use frame_support_procedural::{ - construct_runtime, decl_storage, match_and_insert, transactional, RuntimeDebugNoBound, + construct_runtime, decl_storage, match_and_insert, transactional, CompactPalletError, + RuntimeDebugNoBound, }; #[doc(hidden)] diff --git a/frame/support/test/tests/pallet_ui/pass/error_nested_types.rs b/frame/support/test/tests/pallet_ui/pass/error_nested_types.rs new file mode 100644 index 0000000000000..4c128cf4969e8 --- /dev/null +++ b/frame/support/test/tests/pallet_ui/pass/error_nested_types.rs @@ -0,0 +1,38 @@ +#[frame_support::pallet] +mod pallet { + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::pallet] + pub struct Pallet(core::marker::PhantomData); + + #[pallet::error] + pub enum Error { + CustomError(crate::MyError), + } +} + +#[derive(frame_support::CompactPalletError, scale_info::TypeInfo)] +pub enum MyError { + Foo, + Bar, + Baz(NestedError), + Struct(MyStruct), + Wrapper(Wrapper), +} + +#[derive(frame_support::CompactPalletError, scale_info::TypeInfo)] +pub enum NestedError { + Quux +} + +#[derive(frame_support::CompactPalletError, scale_info::TypeInfo)] +pub struct MyStruct { + field: u8, +} + +#[derive(frame_support::CompactPalletError, scale_info::TypeInfo)] +pub struct Wrapper(Option); + +fn main() { +} From e86c2d99a126c0d7bd4544ce0158cf85c43eddd2 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Sat, 20 Nov 2021 16:19:37 -0800 Subject: [PATCH 09/75] Check for error type compactness in construct_runtime --- .../procedural/src/compact_pallet_error.rs | 1 + .../procedural/src/construct_runtime/mod.rs | 5 ++++ .../procedural/src/pallet/expand/error.rs | 30 ++++++++++++++++--- frame/support/src/dispatch.rs | 4 +++ frame/support/src/error.rs | 2 +- frame/support/src/traits.rs | 2 +- frame/support/src/traits/error.rs | 21 +++++++++++-- frame/support/test/tests/construct_runtime.rs | 1 + frame/support/test/tests/pallet.rs | 5 ++++ frame/support/test/tests/pallet_instance.rs | 5 ++++ 10 files changed, 67 insertions(+), 9 deletions(-) diff --git a/frame/support/procedural/src/compact_pallet_error.rs b/frame/support/procedural/src/compact_pallet_error.rs index 62d0b73f1d033..2a6135b2c97ca 100644 --- a/frame/support/procedural/src/compact_pallet_error.rs +++ b/frame/support/procedural/src/compact_pallet_error.rs @@ -68,6 +68,7 @@ pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro syn::Fields::Unnamed(mut f) if f.unnamed.len() == 1 => Ok(Some(f.unnamed.pop().unwrap().into_value().ty)), syn::Fields::Unit => Ok(None), + _ if variant.ident == "__Ignore" => Ok(None), _ => { let msg = "Cannot derive `CompactPalletError` for enum with variants \ containing more than 1 field"; diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 4315d4278183a..399dbffcc00b9 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -411,6 +411,11 @@ fn decl_integrity_test(scrate: &TokenStream2) -> TokenStream2 { pub fn runtime_integrity_tests() { ::integrity_test(); } + + #[test] + pub fn error_compactness_tests() { + ::error_compactness_test(); + } } ) } diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index 6632410f9a6c1..55cc193a61482 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -21,14 +21,23 @@ use frame_support_procedural_tools::get_doc_literals; /// /// * impl various trait on Error pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { - let error = if let Some(error) = &def.error { error } else { return Default::default() }; - - let error_ident = &error.error; let frame_support = &def.frame_support; let frame_system = &def.frame_system; + let pallet_ident = &def.pallet_struct.pallet; + let pallet_type_impl_gen = &def.type_impl_generics(def.pallet_struct.attr_span); + let pallet_type_use_gen = &def.type_use_generics(def.pallet_struct.attr_span); + let config_where_clause = &def.config.where_clause; + + let error = if let Some(error) = &def.error { error } else { + return quote::quote! { + impl<#pallet_type_impl_gen> #frame_support::traits::ErrorCompactnessTest + for #pallet_ident<#pallet_type_use_gen> #config_where_clause {} + } + }; + + let error_ident = &error.error; let type_impl_gen = &def.type_impl_generics(error.attr_span); let type_use_gen = &def.type_use_generics(error.attr_span); - let config_where_clause = &def.config.where_clause; let phantom_variant: syn::Variant = syn::parse_quote!( #[doc(hidden)] @@ -115,6 +124,19 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { }; quote::quote_spanned!(error.attr_span => + impl<#pallet_type_impl_gen> #frame_support::traits::ErrorCompactnessTest + for #pallet_ident<#pallet_type_use_gen> #config_where_clause + { + fn error_compactness_test() { + assert!( + < + #error_ident<#type_use_gen> as #frame_support::traits::CompactPalletError + >::check_compactness(), + "Pallet error type is not the most compact possible" + ); + } + } + impl<#type_impl_gen> #frame_support::sp_std::fmt::Debug for #error_ident<#type_use_gen> #config_where_clause { diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index a492bc12f6a38..de59a261bfb42 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -2035,6 +2035,10 @@ macro_rules! decl_module { $( $integrity_test )* } + /// Error compactness test is unsupported in declarative macros. + impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::traits::ErrorCompactnessTest + for $mod_type<$trait_instance $(, $instance)?> where $( $other_where_bounds )* {} + /// Can also be called using [`Call`]. /// /// [`Call`]: enum.Call.html diff --git a/frame/support/src/error.rs b/frame/support/src/error.rs index 836428c6bc7db..026406a160d7b 100644 --- a/frame/support/src/error.rs +++ b/frame/support/src/error.rs @@ -85,7 +85,7 @@ macro_rules! decl_error { } ) => { $(#[$attr])* - #[derive($crate::scale_info::TypeInfo)] + #[derive($crate::scale_info::TypeInfo, $crate::CompactPalletError)] #[scale_info(skip_type_params($generic $(, $inst_generic)?), capture_docs = "always")] pub enum $error<$generic: $trait $(, $inst_generic: $instance)?> $( where $( $where_ty: $where_bound ),* )? diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 1d70f9e1ec9cf..4a87f4e9a3633 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -46,7 +46,7 @@ pub use validation::{ }; mod error; -pub use error::CompactPalletError; +pub use error::{CompactPalletError, ErrorCompactnessTest}; mod filter; pub use filter::{ClearFilterGuard, FilterStack, FilterStackGuard, InstanceFilter, IntegrityTest}; diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index 4ac36af6d3b4c..595c0f335d2e5 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -17,11 +17,9 @@ //! Traits for describing and constraining pallet error types. -use scale_info::TypeInfo; - /// Trait denoting that the implementing type has the most compact encoded size that is fit to be /// included as a field in a variant of the `#[pallet::error]` enum type. -pub trait CompactPalletError: TypeInfo { +pub trait CompactPalletError { /// Function that checks whether implementing types are either 1 bytes in size, or that its /// nested types are 1 bytes in size, i.e. whether they are as memory efficient as possible. /// @@ -43,3 +41,20 @@ macro_rules! impl_for_types { } impl_for_types!(u8, i8, bool, char, Option); + +pub trait ErrorCompactnessTest { + fn error_compactness_test() {} +} + +impl ErrorCompactnessTest for (A,) { + fn error_compactness_test() { + A::error_compactness_test(); + } +} + +impl ErrorCompactnessTest for (A, B) { + fn error_compactness_test() { + A::error_compactness_test(); + B::error_compactness_test(); + } +} diff --git a/frame/support/test/tests/construct_runtime.rs b/frame/support/test/tests/construct_runtime.rs index 2d14da04f64b7..2cd90833268af 100644 --- a/frame/support/test/tests/construct_runtime.rs +++ b/frame/support/test/tests/construct_runtime.rs @@ -406,6 +406,7 @@ fn check_modules_error_type() { #[test] fn integrity_test_works() { __construct_runtime_integrity_test::runtime_integrity_tests(); + __construct_runtime_integrity_test::error_compactness_tests(); assert_eq!(INTEGRITY_TEST_EXEC.with(|i| *i.borrow()), 2); } diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index a314f576187dc..a256c9d3c0001 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -592,6 +592,11 @@ fn _ensure_call_is_correctly_excluded_and_included(call: Call) { } } +#[test] +fn error_compactness_test() { + __construct_runtime_integrity_test::error_compactness_tests(); +} + #[test] fn transactional_works() { TestExternalities::default().execute_with(|| { diff --git a/frame/support/test/tests/pallet_instance.rs b/frame/support/test/tests/pallet_instance.rs index c031ac9fe1bf5..21ffff2e16978 100644 --- a/frame/support/test/tests/pallet_instance.rs +++ b/frame/support/test/tests/pallet_instance.rs @@ -311,6 +311,11 @@ frame_support::construct_runtime!( } ); +#[test] +fn error_compactness_test() { + __construct_runtime_integrity_test::error_compactness_tests(); +} + #[test] fn call_expand() { let call_foo = pallet::Call::::foo { foo: 3 }; From 2ba5c5bf8d1b4dd65f7078cd8fb14245478cb41d Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Sat, 20 Nov 2021 16:24:58 -0800 Subject: [PATCH 10/75] cargo fmt --- frame/support/procedural/src/pallet/expand/error.rs | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index 55cc193a61482..c8d936d4cb4d0 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -28,7 +28,9 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { let pallet_type_use_gen = &def.type_use_generics(def.pallet_struct.attr_span); let config_where_clause = &def.config.where_clause; - let error = if let Some(error) = &def.error { error } else { + let error = if let Some(error) = &def.error { + error + } else { return quote::quote! { impl<#pallet_type_impl_gen> #frame_support::traits::ErrorCompactnessTest for #pallet_ident<#pallet_type_use_gen> #config_where_clause {} From 217e1a441dfa82fb3ec36f1ba12143911b7e00e2 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Mon, 22 Nov 2021 18:03:12 -0800 Subject: [PATCH 11/75] Derive CompactPalletError instead of implementing it directly during macro expansion --- .../procedural/src/pallet/expand/error.rs | 32 +++++-------------- frame/support/src/traits/error.rs | 4 +-- .../pallet_ui/pass/error_nested_types.rs | 11 ++++--- 3 files changed, 17 insertions(+), 30 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index c8d936d4cb4d0..4b7341e67e679 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -95,7 +95,14 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { // derive TypeInfo for error metadata error_item .attrs - .push(syn::parse_quote!( #[derive(#frame_support::scale_info::TypeInfo)] )); + .push(syn::parse_quote! { + #[derive( + #frame_support::codec::Encode, + #frame_support::codec::Decode, + #frame_support::scale_info::TypeInfo, + #frame_support::CompactPalletError, + )] + }); error_item.attrs.push(syn::parse_quote!( #[scale_info(skip_type_params(#type_use_gen), capture_docs = "always")] )); @@ -109,22 +116,6 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { )); } - let field_tys = error - .variants - .iter() - .filter_map(|(_, variant_field, _)| { - variant_field.as_ref().map(|VariantField { ty, .. }| ty) - }) - .collect::>(); - - let compactness_check = if field_tys.is_empty() { - quote::quote!(true) - } else { - quote::quote! { - #( <#field_tys as #frame_support::traits::CompactPalletError>::check_compactness() )&&* - } - }; - quote::quote_spanned!(error.attr_span => impl<#pallet_type_impl_gen> #frame_support::traits::ErrorCompactnessTest for #pallet_ident<#pallet_type_use_gen> #config_where_clause @@ -191,12 +182,5 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { } } } - - impl<#type_impl_gen> #frame_support::traits::CompactPalletError - for #error_ident<#type_use_gen> - #config_where_clause - { - fn check_compactness() -> bool { #compactness_check } - } ) } diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index 595c0f335d2e5..743c2d277f0d2 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -19,7 +19,7 @@ /// Trait denoting that the implementing type has the most compact encoded size that is fit to be /// included as a field in a variant of the `#[pallet::error]` enum type. -pub trait CompactPalletError { +pub trait CompactPalletError: codec::Encode + codec::Decode { /// Function that checks whether implementing types are either 1 bytes in size, or that its /// nested types are 1 bytes in size, i.e. whether they are as memory efficient as possible. /// @@ -40,7 +40,7 @@ macro_rules! impl_for_types { }; } -impl_for_types!(u8, i8, bool, char, Option); +impl_for_types!(u8, i8, bool, Option); pub trait ErrorCompactnessTest { fn error_compactness_test() {} diff --git a/frame/support/test/tests/pallet_ui/pass/error_nested_types.rs b/frame/support/test/tests/pallet_ui/pass/error_nested_types.rs index 4c128cf4969e8..63c39893195be 100644 --- a/frame/support/test/tests/pallet_ui/pass/error_nested_types.rs +++ b/frame/support/test/tests/pallet_ui/pass/error_nested_types.rs @@ -1,3 +1,6 @@ +use codec::{Decode, Encode}; +use frame_support::CompactPalletError; + #[frame_support::pallet] mod pallet { #[pallet::config] @@ -12,7 +15,7 @@ mod pallet { } } -#[derive(frame_support::CompactPalletError, scale_info::TypeInfo)] +#[derive(Encode, Decode, CompactPalletError, scale_info::TypeInfo)] pub enum MyError { Foo, Bar, @@ -21,17 +24,17 @@ pub enum MyError { Wrapper(Wrapper), } -#[derive(frame_support::CompactPalletError, scale_info::TypeInfo)] +#[derive(Encode, Decode, CompactPalletError, scale_info::TypeInfo)] pub enum NestedError { Quux } -#[derive(frame_support::CompactPalletError, scale_info::TypeInfo)] +#[derive(Encode, Decode, CompactPalletError, scale_info::TypeInfo)] pub struct MyStruct { field: u8, } -#[derive(frame_support::CompactPalletError, scale_info::TypeInfo)] +#[derive(Encode, Decode, CompactPalletError, scale_info::TypeInfo)] pub struct Wrapper(Option); fn main() { From 3a399b6bc863e53f91bbff9c7b1fe8a648448461 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Mon, 22 Nov 2021 23:13:03 -0800 Subject: [PATCH 12/75] Implement CompactPalletError on OptionBool instead of Option --- frame/support/src/traits/error.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index 743c2d277f0d2..bd7535828fac2 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -16,10 +16,11 @@ // limitations under the License. //! Traits for describing and constraining pallet error types. +use codec::{Decode, Encode, OptionBool}; /// Trait denoting that the implementing type has the most compact encoded size that is fit to be /// included as a field in a variant of the `#[pallet::error]` enum type. -pub trait CompactPalletError: codec::Encode + codec::Decode { +pub trait CompactPalletError: Encode + Decode { /// Function that checks whether implementing types are either 1 bytes in size, or that its /// nested types are 1 bytes in size, i.e. whether they are as memory efficient as possible. /// @@ -40,7 +41,7 @@ macro_rules! impl_for_types { }; } -impl_for_types!(u8, i8, bool, Option); +impl_for_types!(u8, i8, bool, OptionBool); pub trait ErrorCompactnessTest { fn error_compactness_test() {} From a5830254d60c61a9023f80e8cbe9358177b094e6 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 23 Nov 2021 18:21:37 -0800 Subject: [PATCH 13/75] Check for type idents instead of variant ident --- .../procedural/src/compact_pallet_error.rs | 47 ++++++++++++++----- .../procedural/src/pallet/expand/error.rs | 18 ++++--- frame/support/src/error.rs | 7 ++- frame/support/src/traits/error.rs | 7 +++ 4 files changed, 57 insertions(+), 22 deletions(-) diff --git a/frame/support/procedural/src/compact_pallet_error.rs b/frame/support/procedural/src/compact_pallet_error.rs index 2a6135b2c97ca..fdd3026fb6d0e 100644 --- a/frame/support/procedural/src/compact_pallet_error.rs +++ b/frame/support/procedural/src/compact_pallet_error.rs @@ -62,19 +62,44 @@ pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro syn::Data::Enum(syn::DataEnum { variants, .. }) => { let field_tys = variants .into_iter() - .map(|variant| match variant.fields { - syn::Fields::Named(mut f) if f.named.len() == 1 => - Ok(Some(f.named.pop().unwrap().into_value().ty)), - syn::Fields::Unnamed(mut f) if f.unnamed.len() == 1 => - Ok(Some(f.unnamed.pop().unwrap().into_value().ty)), - syn::Fields::Unit => Ok(None), - _ if variant.ident == "__Ignore" => Ok(None), - _ => { + .map(|variant| { + let span = variant.ident.span(); + let make_err = || { let msg = "Cannot derive `CompactPalletError` for enum with variants \ - containing more than 1 field"; - let err = syn::Error::new(variant.ident.span(), msg); + containing more than 1 field"; + let err = syn::Error::new(span, msg); Err(err) - }, + }; + + match variant.fields { + syn::Fields::Named(mut f) if f.named.len() == 1 => + Ok(Some(f.named.pop().unwrap().into_value().ty)), + syn::Fields::Unnamed(mut f) if f.unnamed.len() == 1 => + Ok(Some(f.unnamed.pop().unwrap().into_value().ty)), + syn::Fields::Unnamed(mut f) if f.unnamed.len() == 2 => { + let second = f.unnamed.pop().unwrap().into_value().ty; + let first = f.unnamed.pop().unwrap().into_value().ty; + + match (first, second) { + // Check whether we have (PhantomData, Never), if so we skip it. + (syn::Type::Path(p1), syn::Type::Path(p2)) + if p1 + .path + .segments + .last() + .map_or(false, |seg| seg.ident == "PhantomData") && + p2.path + .segments + .last() + .map_or(false, |seg| seg.ident == "Never") => + Ok(None), + // Otherwise, it's an error. + _ => make_err(), + } + }, + syn::Fields::Unit => Ok(None), + _ => make_err(), + } }) .collect::>, syn::Error>>(); diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index 4b7341e67e679..153cee4c832ae 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -93,16 +93,14 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { error_item.variants.insert(0, phantom_variant); // derive TypeInfo for error metadata - error_item - .attrs - .push(syn::parse_quote! { - #[derive( - #frame_support::codec::Encode, - #frame_support::codec::Decode, - #frame_support::scale_info::TypeInfo, - #frame_support::CompactPalletError, - )] - }); + error_item.attrs.push(syn::parse_quote! { + #[derive( + #frame_support::codec::Encode, + #frame_support::codec::Decode, + #frame_support::scale_info::TypeInfo, + #frame_support::CompactPalletError, + )] + }); error_item.attrs.push(syn::parse_quote!( #[scale_info(skip_type_params(#type_use_gen), capture_docs = "always")] )); diff --git a/frame/support/src/error.rs b/frame/support/src/error.rs index 026406a160d7b..9453e4065b1a9 100644 --- a/frame/support/src/error.rs +++ b/frame/support/src/error.rs @@ -85,7 +85,12 @@ macro_rules! decl_error { } ) => { $(#[$attr])* - #[derive($crate::scale_info::TypeInfo, $crate::CompactPalletError)] + #[derive( + $crate::codec::Encode, + $crate::codec::Decode, + $crate::scale_info::TypeInfo, + $crate::CompactPalletError, + )] #[scale_info(skip_type_params($generic $(, $inst_generic)?), capture_docs = "always")] pub enum $error<$generic: $trait $(, $inst_generic: $instance)?> $( where $( $where_ty: $where_bound ),* )? diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index bd7535828fac2..3278e980afd02 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -17,6 +17,7 @@ //! Traits for describing and constraining pallet error types. use codec::{Decode, Encode, OptionBool}; +use sp_std::marker::PhantomData; /// Trait denoting that the implementing type has the most compact encoded size that is fit to be /// included as a field in a variant of the `#[pallet::error]` enum type. @@ -43,6 +44,12 @@ macro_rules! impl_for_types { impl_for_types!(u8, i8, bool, OptionBool); +impl CompactPalletError for PhantomData { + fn check_compactness() -> bool { + true + } +} + pub trait ErrorCompactnessTest { fn error_compactness_test() {} } From e68ba71076aa0f557007ba00f6fc6b267b779def Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 23 Nov 2021 23:25:34 -0800 Subject: [PATCH 14/75] Add doc comments for ErrorCompactnessTest --- frame/support/src/traits/error.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index 3278e980afd02..e1ade77794929 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -50,7 +50,10 @@ impl CompactPalletError for PhantomData { } } +/// Trait for testing the pallet's error enum compactness. pub trait ErrorCompactnessTest { + /// The function that gets called during integrity testing to check for the compactness + /// of the pallet's error enum type. fn error_compactness_test() {} } From ed643ff88f3e04398ff7ac31ad28eef77918be4f Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 30 Nov 2021 19:53:25 -0800 Subject: [PATCH 15/75] Add an trait implementation of ErrorCompactnessTest for () --- frame/support/src/traits/error.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index e1ade77794929..eedb0fc7d8948 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -57,6 +57,10 @@ pub trait ErrorCompactnessTest { fn error_compactness_test() {} } +// This can happen in tests where no additional pallets aside from the System pallet is included +// in the runtime +impl ErrorCompactnessTest for () {} + impl ErrorCompactnessTest for (A,) { fn error_compactness_test() { A::error_compactness_test(); From 3913e859e5c48d569bbd262c1a7d42fa2b99adda Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Sat, 4 Dec 2021 02:21:10 -0800 Subject: [PATCH 16/75] Convert the error field of DispatchError to a 4-element byte array --- .../procedural/src/pallet/expand/error.rs | 30 +++---------------- frame/support/src/lib.rs | 5 +++- frame/support/src/traits/error.rs | 6 ++++ frame/support/test/tests/pallet.rs | 13 +++++++- .../pallet_ui/pass/error_nested_types.rs | 2 +- primitives/runtime/src/lib.rs | 5 +++- primitives/runtime/src/traits.rs | 6 ++++ 7 files changed, 37 insertions(+), 30 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index 153cee4c832ae..9442dd62cac66 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -50,23 +50,6 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { ) ); - let as_u8_matches = - error - .variants - .iter() - .enumerate() - .map(|(i, (variant, field_ty, _))| match field_ty { - Some(VariantField { is_named: true, .. }) => { - quote::quote_spanned!(error.attr_span => Self::#variant { .. } => #i as u8,) - }, - Some(VariantField { is_named: false, .. }) => { - quote::quote_spanned!(error.attr_span => Self::#variant(..) => #i as u8,) - }, - None => { - quote::quote_spanned!(error.attr_span => Self::#variant => #i as u8,) - }, - }); - let as_str_matches = error.variants.iter().map(|(variant, field_ty, _)| { let variant_str = format!("{}", variant); match field_ty { @@ -123,7 +106,7 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { < #error_ident<#type_use_gen> as #frame_support::traits::CompactPalletError >::check_compactness(), - "Pallet error type is not the most compact possible" + "Pallet error enum is not the most compact possible" ); } } @@ -139,13 +122,6 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { } impl<#type_impl_gen> #error_ident<#type_use_gen> #config_where_clause { - pub fn as_u8(&self) -> u8 { - match &self { - Self::__Ignore(_, _) => unreachable!("`__Ignore` can never be constructed"), - #( #as_u8_matches )* - } - } - pub fn as_str(&self) -> &'static str { match &self { Self::__Ignore(_, _) => unreachable!("`__Ignore` can never be constructed"), @@ -172,10 +148,12 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { as #frame_support::traits::PalletInfo >::index::>() .expect("Every active module has an index in the runtime; qed") as u8; + let mut encoded = err.encode(); + encoded.resize(#frame_support::MAX_NESTED_PALLET_ERROR_DEPTH, 0); #frame_support::sp_runtime::DispatchError::Module { index, - error: err.as_u8(), + error: encoded.try_into().expect("encoded error is resized to be equal to 4 bytes; qed"), message: Some(err.as_str()), } } diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index ae7cea45ecd77..0ffac837fa4c2 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -93,7 +93,9 @@ pub use self::{ StorageMap, StorageNMap, StoragePrefixedMap, StorageValue, }, }; -pub use sp_runtime::{self, print, traits::Printable, ConsensusEngineId}; +pub use sp_runtime::{ + self, print, traits::Printable, ConsensusEngineId, MAX_NESTED_PALLET_ERROR_DEPTH, +}; use codec::{Decode, Encode}; use scale_info::TypeInfo; @@ -1339,6 +1341,7 @@ pub mod pallet_prelude { TransactionTag, TransactionValidity, TransactionValidityError, UnknownTransaction, ValidTransaction, }, + MAX_NESTED_PALLET_ERROR_DEPTH, }; pub use sp_std::marker::PhantomData; } diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index eedb0fc7d8948..544d3e424c029 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -21,6 +21,12 @@ use sp_std::marker::PhantomData; /// Trait denoting that the implementing type has the most compact encoded size that is fit to be /// included as a field in a variant of the `#[pallet::error]` enum type. +/// +/// ## Notes +/// The pallet error enum has a maximum nested depth as defined by +/// [`frame_support::MAX_NESTED_PALLET_ERROR_DEPTH`]. If the pallet error type exceeds this size +/// limit, the encoded representation of it will truncate any excess bytes when setting the error +/// field during the creation of the [`DispatchError`] type. pub trait CompactPalletError: Encode + Decode { /// Function that checks whether implementing types are either 1 bytes in size, or that its /// nested types are 1 bytes in size, i.e. whether they are as memory efficient as possible. diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index 9e6d29fbd0b93..c5784f29f69d6 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -230,9 +230,11 @@ pub mod pallet { } #[pallet::error] + #[derive(PartialEq, Eq)] pub enum Error { /// doc comment put into metadata InsufficientProposersBalance, + Code(u8), } #[pallet::event] @@ -651,6 +653,7 @@ fn call_expand() { #[test] fn error_expand() { + use codec::Decode; assert_eq!( format!("{:?}", pallet::Error::::InsufficientProposersBalance), String::from("InsufficientProposersBalance"), @@ -661,7 +664,15 @@ fn error_expand() { ); assert_eq!( DispatchError::from(pallet::Error::::InsufficientProposersBalance), - DispatchError::Module { index: 1, error: 0, message: Some("InsufficientProposersBalance") }, + DispatchError::Module { + index: 1, + error: [0, 0, 0, 0], + message: Some("InsufficientProposersBalance") + }, + ); + assert_eq!( + pallet::Error::::decode(&mut &[1, 4, 0, 0][..]), + Ok(pallet::Error::::Code(4)), ); } diff --git a/frame/support/test/tests/pallet_ui/pass/error_nested_types.rs b/frame/support/test/tests/pallet_ui/pass/error_nested_types.rs index 63c39893195be..cf211a55db137 100644 --- a/frame/support/test/tests/pallet_ui/pass/error_nested_types.rs +++ b/frame/support/test/tests/pallet_ui/pass/error_nested_types.rs @@ -35,7 +35,7 @@ pub struct MyStruct { } #[derive(Encode, Decode, CompactPalletError, scale_info::TypeInfo)] -pub struct Wrapper(Option); +pub struct Wrapper(bool); fn main() { } diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 80293fe734844..cb178b31d4825 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -96,6 +96,9 @@ pub use sp_arithmetic::{ pub use either::Either; +/// The maximum depth for a nested pallet error enum. +pub const MAX_NESTED_PALLET_ERROR_DEPTH: usize = 4; + /// An abstraction over justification for a block's validity under a consensus algorithm. /// /// Essentially a finality proof. The exact formulation will vary between consensus @@ -484,7 +487,7 @@ pub enum DispatchError { /// Module index, matching the metadata module index. index: u8, /// Module specific error value. - error: u8, + error: [u8; MAX_NESTED_PALLET_ERROR_DEPTH], /// Optional error message. #[codec(skip)] #[cfg_attr(feature = "std", serde(skip_deserializing))] diff --git a/primitives/runtime/src/traits.rs b/primitives/runtime/src/traits.rs index f61de70e35197..50a5787469fae 100644 --- a/primitives/runtime/src/traits.rs +++ b/primitives/runtime/src/traits.rs @@ -1533,6 +1533,12 @@ impl Printable for &[u8] { } } +impl Printable for [u8; N] { + fn print(&self) { + sp_io::misc::print_hex(&self[..]); + } +} + impl Printable for &str { fn print(&self) { sp_io::misc::print_utf8(self.as_bytes()); From 26b0adf8408814e0f2ae23678d04c6164ce2fbb8 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 7 Dec 2021 04:33:30 -0800 Subject: [PATCH 17/75] Add static check for pallet error size --- Cargo.lock | 1 + frame/support/Cargo.toml | 1 + .../procedural/src/compact_pallet_error.rs | 68 ++++++++++++----- .../procedural/src/construct_runtime/mod.rs | 44 ++++++++++- .../procedural/src/pallet/expand/error.rs | 43 ++++++++++- frame/support/src/lib.rs | 2 + frame/support/src/traits/error.rs | 22 ++++-- .../pallet_error_too_large.rs | 58 +++++++++++++++ .../pallet_error_too_large.stderr | 73 +++++++++++++++++++ 9 files changed, 283 insertions(+), 29 deletions(-) create mode 100644 frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.rs create mode 100644 frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.stderr diff --git a/Cargo.lock b/Cargo.lock index fbdf0297d4531..edb31b8fe3920 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2067,6 +2067,7 @@ dependencies = [ "sp-state-machine", "sp-std", "sp-tracing", + "static_assertions", "tt-call", ] diff --git a/frame/support/Cargo.toml b/frame/support/Cargo.toml index 1f48dadc2987d..a7af957799f48 100644 --- a/frame/support/Cargo.toml +++ b/frame/support/Cargo.toml @@ -25,6 +25,7 @@ sp-core = { version = "4.0.0-dev", default-features = false, path = "../../primi sp-arithmetic = { version = "4.0.0-dev", default-features = false, path = "../../primitives/arithmetic" } sp-inherents = { version = "4.0.0-dev", default-features = false, path = "../../primitives/inherents" } sp-staking = { version = "4.0.0-dev", default-features = false, path = "../../primitives/staking" } +static_assertions = "1.1.0" tt-call = "1.0.8" frame-support-procedural = { version = "4.0.0-dev", default-features = false, path = "./procedural" } paste = "1.0" diff --git a/frame/support/procedural/src/compact_pallet_error.rs b/frame/support/procedural/src/compact_pallet_error.rs index fdd3026fb6d0e..59de0f6b6afe8 100644 --- a/frame/support/procedural/src/compact_pallet_error.rs +++ b/frame/support/procedural/src/compact_pallet_error.rs @@ -32,7 +32,7 @@ pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro let frame_support = &frame_support; let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); - let compactness_check = match data { + let (max_encoded_size, compactness_check) = match data { syn::Data::Struct(syn::DataStruct { struct_token, fields, .. }) => { if fields.len() > 1 { let msg = "Cannot derive `CompactPalletError` for structs with more than 1 field"; @@ -42,21 +42,35 @@ pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro match fields { syn::Fields::Named(mut f) if f.named.len() == 1 => { let field_ty = f.named.pop().unwrap().into_value().ty; - quote::quote! { - < - #field_ty as #frame_support::traits::CompactPalletError - >::check_compactness() - } + ( + quote::quote! { + < + #field_ty as #frame_support::traits::CompactPalletError + >::MAX_ENCODED_SIZE + }, + quote::quote! { + < + #field_ty as #frame_support::traits::CompactPalletError + >::check_compactness() + }, + ) }, syn::Fields::Unnamed(mut f) if f.unnamed.len() == 1 => { let field_ty = f.unnamed.pop().unwrap().into_value().ty; - quote::quote! { - < - #field_ty as #frame_support::traits::CompactPalletError - >::check_compactness() - } + ( + quote::quote! { + < + #field_ty as #frame_support::traits::CompactPalletError + >::MAX_ENCODED_SIZE + }, + quote::quote! { + < + #field_ty as #frame_support::traits::CompactPalletError + >::check_compactness() + }, + ) }, - _ => quote::quote!(true), + _ => (quote::quote!(1), quote::quote!(true)), } }, syn::Data::Enum(syn::DataEnum { variants, .. }) => { @@ -109,15 +123,28 @@ pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro }; if field_tys.is_empty() { - quote::quote!(true) + (quote::quote!(1), quote::quote!(true)) } else { - quote::quote! { - #( - < - #field_tys as #frame_support::traits::CompactPalletError - >::check_compactness() - )&&* - } + ( + quote::quote! {{ + let mut size = 1; + let mut tmp: usize; + #( + tmp = 1 + < + #field_tys as #frame_support::traits::CompactPalletError + >::MAX_ENCODED_SIZE; + size = if tmp > size { tmp } else { size }; + )* + size + }}, + quote::quote! { + #( + < + #field_tys as #frame_support::traits::CompactPalletError + >::check_compactness() + )&&* + }, + ) } }, syn::Data::Union(syn::DataUnion { union_token, .. }) => { @@ -131,6 +158,7 @@ pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro impl #impl_generics #frame_support::traits::CompactPalletError for #name #ty_generics #where_clause { + const MAX_ENCODED_SIZE: usize = #max_encoded_size; fn check_compactness() -> bool { #compactness_check } diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 3073f7c551c65..389b4e0d0cfe6 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -153,7 +153,7 @@ use parse::{ }; use proc_macro::TokenStream; use proc_macro2::TokenStream as TokenStream2; -use quote::quote; +use quote::{format_ident, quote}; use syn::{Ident, Result}; /// The fixed name of the system pallet. @@ -241,6 +241,7 @@ fn construct_runtime_final_expansion( expand::expand_outer_inherent(&name, &block, &unchecked_extrinsic, &pallets, &scrate); let validate_unsigned = expand::expand_outer_validate_unsigned(&name, &pallets, &scrate); let integrity_test = decl_integrity_test(&scrate); + let static_assertions = decl_static_assertions(&name, &pallets, &scrate); let res = quote!( #scrate_decl @@ -282,6 +283,8 @@ fn construct_runtime_final_expansion( #validate_unsigned #integrity_test + + #static_assertions ); Ok(res) @@ -476,3 +479,42 @@ fn decl_integrity_test(scrate: &TokenStream2) -> TokenStream2 { } ) } + +fn decl_static_assertions( + runtime: &Ident, + pallet_decls: &[Pallet], + scrate: &TokenStream2, +) -> TokenStream2 { + let error_encoded_size_check = pallet_decls.iter().map(|decl| { + let name = &decl.name; + let path = &decl.path; + let assert_macro_name = format_ident!("assert_error_encoded_size_for_{}", name); + + quote! { + #scrate::tt_call! { + macro = [{ #path::tt_error_token }] + frame_support = [{ #scrate }] + ~~> #assert_macro_name + } + + #[macro_export] + #[doc(hidden)] + macro_rules! #assert_macro_name { + { + error = [{ $error:ident }] + } => { + #scrate::const_assert! { + < + #path::$error<#runtime> as #scrate::traits::CompactPalletError + >::MAX_ENCODED_SIZE <= #scrate::MAX_NESTED_PALLET_ERROR_DEPTH + } + }; + {} => {}; + } + } + }); + + quote! { + #(#error_encoded_size_check)* + } +} diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index 9442dd62cac66..838d9092606ef 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -15,12 +15,20 @@ // See the License for the specific language governing permissions and // limitations under the License. -use crate::pallet::{parse::error::VariantField, Def}; +use crate::{ + pallet::{parse::error::VariantField, Def}, + COUNTER, +}; use frame_support_procedural_tools::get_doc_literals; +use syn::spanned::Spanned; /// /// * impl various trait on Error pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { + let count = COUNTER.with(|counter| counter.borrow_mut().inc()); + let error_token_unique_id = + syn::Ident::new(&format!("__tt_error_token_{}", count), def.item.span()); + let frame_support = &def.frame_support; let frame_system = &def.frame_system; let pallet_ident = &def.pallet_struct.pallet; @@ -32,6 +40,22 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { error } else { return quote::quote! { + + #[macro_export] + #[doc(hidden)] + macro_rules! #error_token_unique_id { + { + $caller:tt + frame_support = [{ $($frame_support:ident)::* }] + } => { + $($frame_support::)*tt_return! { + $caller + } + }; + } + + pub use #error_token_unique_id as tt_error_token; + impl<#pallet_type_impl_gen> #frame_support::traits::ErrorCompactnessTest for #pallet_ident<#pallet_type_use_gen> #config_where_clause {} } @@ -143,6 +167,7 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { #config_where_clause { fn from(err: #error_ident<#type_use_gen>) -> Self { + use #frame_support::codec::Encode; let index = < ::PalletInfo as #frame_support::traits::PalletInfo @@ -158,5 +183,21 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { } } } + + #[macro_export] + #[doc(hidden)] + macro_rules! #error_token_unique_id { + { + $caller:tt + frame_support = [{ $($frame_support:ident)::* }] + } => { + $($frame_support::)*tt_return! { + $caller + error = [{ #error_ident }] + } + }; + } + + pub use #error_token_unique_id as tt_error_token; ) } diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 0ffac837fa4c2..9d4a28fb0c5b5 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -53,6 +53,8 @@ pub use sp_state_machine::BasicExternalities; #[doc(hidden)] pub use sp_std; #[doc(hidden)] +pub use static_assertions::*; +#[doc(hidden)] pub use tt_call::*; #[macro_use] diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index 544d3e424c029..542d9cb1cf005 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -28,6 +28,12 @@ use sp_std::marker::PhantomData; /// limit, the encoded representation of it will truncate any excess bytes when setting the error /// field during the creation of the [`DispatchError`] type. pub trait CompactPalletError: Encode + Decode { + /// The maximum encoded size for the implementing type. + /// + /// This will be used to check whether the pallet error type is less than or equal to + /// [`frame_support::MAX_NESTED_PALLET_ERROR_DEPTH`], and if it is, a compile error will be + /// thrown. + const MAX_ENCODED_SIZE: usize; /// Function that checks whether implementing types are either 1 bytes in size, or that its /// nested types are 1 bytes in size, i.e. whether they are as memory efficient as possible. /// @@ -39,18 +45,20 @@ pub trait CompactPalletError: Encode + Decode { } macro_rules! impl_for_types { - ($($typ:ty),+) => { - $( - impl CompactPalletError for $typ { - fn check_compactness() -> bool { true } - } - )+ - }; + ($($typ:ty),+) => { + $( + impl CompactPalletError for $typ { + const MAX_ENCODED_SIZE: usize = 1; + fn check_compactness() -> bool { true } + } + )+ + }; } impl_for_types!(u8, i8, bool, OptionBool); impl CompactPalletError for PhantomData { + const MAX_ENCODED_SIZE: usize = 0; fn check_compactness() -> bool { true } diff --git a/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.rs b/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.rs new file mode 100644 index 0000000000000..c4b12bba7e87e --- /dev/null +++ b/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.rs @@ -0,0 +1,58 @@ +use frame_support::construct_runtime; +use sp_runtime::{generic, traits::BlakeTwo256}; +use sp_core::sr25519; + +#[frame_support::pallet] +mod pallet { + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::pallet] + pub struct Pallet(core::marker::PhantomData); + + #[pallet::error] + pub enum Error { + MyError(crate::Nested1), + } +} + +#[derive(scale_info::TypeInfo, frame_support::CompactPalletError, codec::Encode, codec::Decode)] +pub enum Nested1 { + Nested2(Nested2) +} + +#[derive(scale_info::TypeInfo, frame_support::CompactPalletError, codec::Encode, codec::Decode)] +pub enum Nested2 { + Nested3(Nested3) +} + +#[derive(scale_info::TypeInfo, frame_support::CompactPalletError, codec::Encode, codec::Decode)] +pub enum Nested3 { + Nested4(Nested4) +} + +#[derive(scale_info::TypeInfo, frame_support::CompactPalletError, codec::Encode, codec::Decode)] +pub enum Nested4 { + Num(u8) +} + +pub type Signature = sr25519::Signature; +pub type BlockNumber = u64; +pub type Header = generic::Header; +pub type Block = generic::Block; +pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; + +impl pallet::Config for Runtime {} + +construct_runtime! { + pub enum Runtime where + Block = Block, + NodeBlock = Block, + UncheckedExtrinsic = UncheckedExtrinsic + { + System: system::{Pallet, Call, Storage, Config, Event}, + Pallet: pallet::{Pallet}, + } +} + +fn main() {} diff --git a/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.stderr b/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.stderr new file mode 100644 index 0000000000000..c97764dfbe97d --- /dev/null +++ b/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.stderr @@ -0,0 +1,73 @@ +error[E0433]: failed to resolve: use of undeclared crate or module `system` + --> tests/construct_runtime_ui/pallet_error_too_large.rs:53:11 + | +53 | System: system::{Pallet, Call, Storage, Config, Event}, + | ^^^^^^ use of undeclared crate or module `system` + +error[E0433]: failed to resolve: use of undeclared crate or module `system` + --> tests/construct_runtime_ui/pallet_error_too_large.rs:47:1 + | +47 | / construct_runtime! { +48 | | pub enum Runtime where +49 | | Block = Block, +50 | | NodeBlock = Block, +... | +55 | | } +56 | | } + | |_^ not found in `system` + | + = note: this error originates in the macro `construct_runtime` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider importing this enum + | +1 | use frame_system::RawOrigin; + | + +error[E0433]: failed to resolve: use of undeclared crate or module `system` + --> tests/construct_runtime_ui/pallet_error_too_large.rs:47:1 + | +47 | / construct_runtime! { +48 | | pub enum Runtime where +49 | | Block = Block, +50 | | NodeBlock = Block, +... | +55 | | } +56 | | } + | |_^ not found in `system` + | + = note: this error originates in the macro `construct_runtime` (in Nightly builds, run with -Z macro-backtrace for more info) +help: consider importing one of these items + | +1 | use crate::pallet::Pallet; + | +1 | use frame_support_test::Pallet; + | +1 | use frame_system::Pallet; + | +1 | use test_pallet::Pallet; + | + +error[E0277]: the trait bound `Runtime: frame_system::Config` is not satisfied + --> tests/construct_runtime_ui/pallet_error_too_large.rs:45:6 + | +45 | impl pallet::Config for Runtime {} + | ^^^^^^^^^^^^^^ the trait `frame_system::Config` is not implemented for `Runtime` + | +note: required by a bound in `pallet::Config` + --> tests/construct_runtime_ui/pallet_error_too_large.rs:8:20 + | +8 | pub trait Config: frame_system::Config {} + | ^^^^^^^^^^^^^^^^^^^^ required by this bound in `pallet::Config` + +error[E0080]: evaluation of constant value failed + --> tests/construct_runtime_ui/pallet_error_too_large.rs:47:1 + | +47 | / construct_runtime! { +48 | | pub enum Runtime where +49 | | Block = Block, +50 | | NodeBlock = Block, +... | +55 | | } +56 | | } + | |_^ attempt to compute `0_usize - 1_usize`, which would overflow + | + = note: this error originates in the macro `self::sp_api_hidden_includes_construct_runtime::hidden_include::const_assert` (in Nightly builds, run with -Z macro-backtrace for more info) From 82c29a917533099206b13b76feada6c22220ee73 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 7 Dec 2021 16:20:24 -0800 Subject: [PATCH 18/75] Rename to MAX_PALLET_ERROR_ENCODED_SIZE --- frame/support/procedural/src/construct_runtime/mod.rs | 2 +- frame/support/procedural/src/pallet/expand/error.rs | 2 +- frame/support/src/lib.rs | 4 ++-- frame/support/src/traits/error.rs | 6 +++--- primitives/runtime/src/lib.rs | 4 ++-- 5 files changed, 9 insertions(+), 9 deletions(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 389b4e0d0cfe6..3609ecdf140b5 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -506,7 +506,7 @@ fn decl_static_assertions( #scrate::const_assert! { < #path::$error<#runtime> as #scrate::traits::CompactPalletError - >::MAX_ENCODED_SIZE <= #scrate::MAX_NESTED_PALLET_ERROR_DEPTH + >::MAX_ENCODED_SIZE <= #scrate::MAX_PALLET_ERROR_ENCODED_SIZE } }; {} => {}; diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index 838d9092606ef..98bf20c5d7386 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -174,7 +174,7 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { >::index::>() .expect("Every active module has an index in the runtime; qed") as u8; let mut encoded = err.encode(); - encoded.resize(#frame_support::MAX_NESTED_PALLET_ERROR_DEPTH, 0); + encoded.resize(#frame_support::MAX_PALLET_ERROR_ENCODED_SIZE, 0); #frame_support::sp_runtime::DispatchError::Module { index, diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 9d4a28fb0c5b5..df277d4899482 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -96,7 +96,7 @@ pub use self::{ }, }; pub use sp_runtime::{ - self, print, traits::Printable, ConsensusEngineId, MAX_NESTED_PALLET_ERROR_DEPTH, + self, print, traits::Printable, ConsensusEngineId, MAX_PALLET_ERROR_ENCODED_SIZE, }; use codec::{Decode, Encode}; @@ -1343,7 +1343,7 @@ pub mod pallet_prelude { TransactionTag, TransactionValidity, TransactionValidityError, UnknownTransaction, ValidTransaction, }, - MAX_NESTED_PALLET_ERROR_DEPTH, + MAX_PALLET_ERROR_ENCODED_SIZE, }; pub use sp_std::marker::PhantomData; } diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index 542d9cb1cf005..b19a0c8bb4d16 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -23,15 +23,15 @@ use sp_std::marker::PhantomData; /// included as a field in a variant of the `#[pallet::error]` enum type. /// /// ## Notes -/// The pallet error enum has a maximum nested depth as defined by -/// [`frame_support::MAX_NESTED_PALLET_ERROR_DEPTH`]. If the pallet error type exceeds this size +/// The pallet error enum has a maximum encoded size as defined by +/// [`frame_support::MAX_PALLET_ERROR_ENCODED_SIZE`]. If the pallet error type exceeds this size /// limit, the encoded representation of it will truncate any excess bytes when setting the error /// field during the creation of the [`DispatchError`] type. pub trait CompactPalletError: Encode + Decode { /// The maximum encoded size for the implementing type. /// /// This will be used to check whether the pallet error type is less than or equal to - /// [`frame_support::MAX_NESTED_PALLET_ERROR_DEPTH`], and if it is, a compile error will be + /// [`frame_support::MAX_PALLET_ERROR_ENCODED_SIZE`], and if it is, a compile error will be /// thrown. const MAX_ENCODED_SIZE: usize; /// Function that checks whether implementing types are either 1 bytes in size, or that its diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index cb178b31d4825..11a49392b8c8c 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -97,7 +97,7 @@ pub use sp_arithmetic::{ pub use either::Either; /// The maximum depth for a nested pallet error enum. -pub const MAX_NESTED_PALLET_ERROR_DEPTH: usize = 4; +pub const MAX_PALLET_ERROR_ENCODED_SIZE: usize = 4; /// An abstraction over justification for a block's validity under a consensus algorithm. /// @@ -487,7 +487,7 @@ pub enum DispatchError { /// Module index, matching the metadata module index. index: u8, /// Module specific error value. - error: [u8; MAX_NESTED_PALLET_ERROR_DEPTH], + error: [u8; MAX_PALLET_ERROR_ENCODED_SIZE], /// Optional error message. #[codec(skip)] #[cfg_attr(feature = "std", serde(skip_deserializing))] From a1a0c32f0f00b3f3697501250acaa3186b2639fa Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 7 Dec 2021 17:22:16 -0800 Subject: [PATCH 19/75] Remove ErrorCompactnessTest trait --- .../procedural/src/construct_runtime/mod.rs | 5 ---- .../procedural/src/pallet/expand/error.rs | 19 ------------ frame/support/src/dispatch.rs | 4 --- frame/support/src/traits.rs | 2 +- frame/support/src/traits/error.rs | 29 ++----------------- frame/support/test/tests/construct_runtime.rs | 1 - frame/support/test/tests/pallet.rs | 5 ---- frame/support/test/tests/pallet_instance.rs | 5 ---- 8 files changed, 4 insertions(+), 66 deletions(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 3609ecdf140b5..cdc72fcbcfcc5 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -471,11 +471,6 @@ fn decl_integrity_test(scrate: &TokenStream2) -> TokenStream2 { pub fn runtime_integrity_tests() { ::integrity_test(); } - - #[test] - pub fn error_compactness_tests() { - ::error_compactness_test(); - } } ) } diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index 98bf20c5d7386..dc37d49e13e80 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -31,9 +31,6 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { let frame_support = &def.frame_support; let frame_system = &def.frame_system; - let pallet_ident = &def.pallet_struct.pallet; - let pallet_type_impl_gen = &def.type_impl_generics(def.pallet_struct.attr_span); - let pallet_type_use_gen = &def.type_use_generics(def.pallet_struct.attr_span); let config_where_clause = &def.config.where_clause; let error = if let Some(error) = &def.error { @@ -55,9 +52,6 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { } pub use #error_token_unique_id as tt_error_token; - - impl<#pallet_type_impl_gen> #frame_support::traits::ErrorCompactnessTest - for #pallet_ident<#pallet_type_use_gen> #config_where_clause {} } }; @@ -122,19 +116,6 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { } quote::quote_spanned!(error.attr_span => - impl<#pallet_type_impl_gen> #frame_support::traits::ErrorCompactnessTest - for #pallet_ident<#pallet_type_use_gen> #config_where_clause - { - fn error_compactness_test() { - assert!( - < - #error_ident<#type_use_gen> as #frame_support::traits::CompactPalletError - >::check_compactness(), - "Pallet error enum is not the most compact possible" - ); - } - } - impl<#type_impl_gen> #frame_support::sp_std::fmt::Debug for #error_ident<#type_use_gen> #config_where_clause { diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index de59a261bfb42..a492bc12f6a38 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -2035,10 +2035,6 @@ macro_rules! decl_module { $( $integrity_test )* } - /// Error compactness test is unsupported in declarative macros. - impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::traits::ErrorCompactnessTest - for $mod_type<$trait_instance $(, $instance)?> where $( $other_where_bounds )* {} - /// Can also be called using [`Call`]. /// /// [`Call`]: enum.Call.html diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index c160b8e88e351..34458d2a14cf6 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -46,7 +46,7 @@ pub use validation::{ }; mod error; -pub use error::{CompactPalletError, ErrorCompactnessTest}; +pub use error::CompactPalletError; mod filter; pub use filter::{ClearFilterGuard, FilterStack, FilterStackGuard, InstanceFilter, IntegrityTest}; diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index b19a0c8bb4d16..53ae3790ee8bd 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -25,8 +25,9 @@ use sp_std::marker::PhantomData; /// ## Notes /// The pallet error enum has a maximum encoded size as defined by /// [`frame_support::MAX_PALLET_ERROR_ENCODED_SIZE`]. If the pallet error type exceeds this size -/// limit, the encoded representation of it will truncate any excess bytes when setting the error -/// field during the creation of the [`DispatchError`] type. +/// limit, a static assertion during compilation will fail. The compilation error will be in the +/// format of `error[E0080]: evaluation of constant value failed` due to the usage of +/// [`static_assertions::const_assert`]. pub trait CompactPalletError: Encode + Decode { /// The maximum encoded size for the implementing type. /// @@ -63,27 +64,3 @@ impl CompactPalletError for PhantomData { true } } - -/// Trait for testing the pallet's error enum compactness. -pub trait ErrorCompactnessTest { - /// The function that gets called during integrity testing to check for the compactness - /// of the pallet's error enum type. - fn error_compactness_test() {} -} - -// This can happen in tests where no additional pallets aside from the System pallet is included -// in the runtime -impl ErrorCompactnessTest for () {} - -impl ErrorCompactnessTest for (A,) { - fn error_compactness_test() { - A::error_compactness_test(); - } -} - -impl ErrorCompactnessTest for (A, B) { - fn error_compactness_test() { - A::error_compactness_test(); - B::error_compactness_test(); - } -} diff --git a/frame/support/test/tests/construct_runtime.rs b/frame/support/test/tests/construct_runtime.rs index 2cd90833268af..2d14da04f64b7 100644 --- a/frame/support/test/tests/construct_runtime.rs +++ b/frame/support/test/tests/construct_runtime.rs @@ -406,7 +406,6 @@ fn check_modules_error_type() { #[test] fn integrity_test_works() { __construct_runtime_integrity_test::runtime_integrity_tests(); - __construct_runtime_integrity_test::error_compactness_tests(); assert_eq!(INTEGRITY_TEST_EXEC.with(|i| *i.borrow()), 2); } diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index c5784f29f69d6..f08e70f17742f 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -608,11 +608,6 @@ fn _ensure_call_is_correctly_excluded_and_included(call: Call) { } } -#[test] -fn error_compactness_test() { - __construct_runtime_integrity_test::error_compactness_tests(); -} - #[test] fn transactional_works() { TestExternalities::default().execute_with(|| { diff --git a/frame/support/test/tests/pallet_instance.rs b/frame/support/test/tests/pallet_instance.rs index afb5aef4e3084..de70b0e7e404e 100644 --- a/frame/support/test/tests/pallet_instance.rs +++ b/frame/support/test/tests/pallet_instance.rs @@ -311,11 +311,6 @@ frame_support::construct_runtime!( } ); -#[test] -fn error_compactness_test() { - __construct_runtime_integrity_test::error_compactness_tests(); -} - #[test] fn call_expand() { let call_foo = pallet::Call::::foo { foo: 3 }; From 0fb5860e038e918fc175578a764c338c8f6af5e1 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 7 Dec 2021 17:51:58 -0800 Subject: [PATCH 20/75] Remove check_compactness --- .../procedural/src/compact_pallet_error.rs | 74 ++++++------------- frame/support/src/traits/error.rs | 12 --- .../test/tests/pallet_ui/error_not_compact.rs | 2 +- .../tests/pallet_ui/error_not_compact.stderr | 10 +-- 4 files changed, 30 insertions(+), 68 deletions(-) diff --git a/frame/support/procedural/src/compact_pallet_error.rs b/frame/support/procedural/src/compact_pallet_error.rs index 59de0f6b6afe8..d34744c7ee6e0 100644 --- a/frame/support/procedural/src/compact_pallet_error.rs +++ b/frame/support/procedural/src/compact_pallet_error.rs @@ -32,7 +32,7 @@ pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro let frame_support = &frame_support; let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); - let (max_encoded_size, compactness_check) = match data { + let max_encoded_size = match data { syn::Data::Struct(syn::DataStruct { struct_token, fields, .. }) => { if fields.len() > 1 { let msg = "Cannot derive `CompactPalletError` for structs with more than 1 field"; @@ -42,35 +42,21 @@ pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro match fields { syn::Fields::Named(mut f) if f.named.len() == 1 => { let field_ty = f.named.pop().unwrap().into_value().ty; - ( - quote::quote! { - < - #field_ty as #frame_support::traits::CompactPalletError - >::MAX_ENCODED_SIZE - }, - quote::quote! { - < - #field_ty as #frame_support::traits::CompactPalletError - >::check_compactness() - }, - ) + quote::quote! { + < + #field_ty as #frame_support::traits::CompactPalletError + >::MAX_ENCODED_SIZE + } }, syn::Fields::Unnamed(mut f) if f.unnamed.len() == 1 => { let field_ty = f.unnamed.pop().unwrap().into_value().ty; - ( - quote::quote! { - < - #field_ty as #frame_support::traits::CompactPalletError - >::MAX_ENCODED_SIZE - }, - quote::quote! { - < - #field_ty as #frame_support::traits::CompactPalletError - >::check_compactness() - }, - ) + quote::quote! { + < + #field_ty as #frame_support::traits::CompactPalletError + >::MAX_ENCODED_SIZE + } }, - _ => (quote::quote!(1), quote::quote!(true)), + _ => quote::quote!(1), } }, syn::Data::Enum(syn::DataEnum { variants, .. }) => { @@ -123,28 +109,19 @@ pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro }; if field_tys.is_empty() { - (quote::quote!(1), quote::quote!(true)) + quote::quote!(1) } else { - ( - quote::quote! {{ - let mut size = 1; - let mut tmp: usize; - #( - tmp = 1 + < - #field_tys as #frame_support::traits::CompactPalletError - >::MAX_ENCODED_SIZE; - size = if tmp > size { tmp } else { size }; - )* - size - }}, - quote::quote! { - #( - < - #field_tys as #frame_support::traits::CompactPalletError - >::check_compactness() - )&&* - }, - ) + quote::quote! {{ + let mut size = 1; + let mut tmp: usize; + #( + tmp = 1 + < + #field_tys as #frame_support::traits::CompactPalletError + >::MAX_ENCODED_SIZE; + size = if tmp > size { tmp } else { size }; + )* + size + }} } }, syn::Data::Union(syn::DataUnion { union_token, .. }) => { @@ -159,9 +136,6 @@ pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro for #name #ty_generics #where_clause { const MAX_ENCODED_SIZE: usize = #max_encoded_size; - fn check_compactness() -> bool { - #compactness_check - } } }; ) diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index 53ae3790ee8bd..1c5b02b4344d7 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -35,14 +35,6 @@ pub trait CompactPalletError: Encode + Decode { /// [`frame_support::MAX_PALLET_ERROR_ENCODED_SIZE`], and if it is, a compile error will be /// thrown. const MAX_ENCODED_SIZE: usize; - /// Function that checks whether implementing types are either 1 bytes in size, or that its - /// nested types are 1 bytes in size, i.e. whether they are as memory efficient as possible. - /// - /// It is up to the implementing type to prove that it is maximally compact, thus this - /// function defaults to false. - fn check_compactness() -> bool { - false - } } macro_rules! impl_for_types { @@ -50,7 +42,6 @@ macro_rules! impl_for_types { $( impl CompactPalletError for $typ { const MAX_ENCODED_SIZE: usize = 1; - fn check_compactness() -> bool { true } } )+ }; @@ -60,7 +51,4 @@ impl_for_types!(u8, i8, bool, OptionBool); impl CompactPalletError for PhantomData { const MAX_ENCODED_SIZE: usize = 0; - fn check_compactness() -> bool { - true - } } diff --git a/frame/support/test/tests/pallet_ui/error_not_compact.rs b/frame/support/test/tests/pallet_ui/error_not_compact.rs index d85e6c32ebb8a..254d65866774f 100644 --- a/frame/support/test/tests/pallet_ui/error_not_compact.rs +++ b/frame/support/test/tests/pallet_ui/error_not_compact.rs @@ -12,7 +12,7 @@ mod pallet { } } -#[derive(scale_info::TypeInfo)] +#[derive(scale_info::TypeInfo, codec::Encode, codec::Decode)] enum MyError {} fn main() { diff --git a/frame/support/test/tests/pallet_ui/error_not_compact.stderr b/frame/support/test/tests/pallet_ui/error_not_compact.stderr index 18b056f26310a..007faaabfe4f0 100644 --- a/frame/support/test/tests/pallet_ui/error_not_compact.stderr +++ b/frame/support/test/tests/pallet_ui/error_not_compact.stderr @@ -1,12 +1,12 @@ error[E0277]: the trait bound `MyError: CompactPalletError` is not satisfied - --> $DIR/error_not_compact.rs:1:1 + --> tests/pallet_ui/error_not_compact.rs:1:1 | 1 | #[frame_support::pallet] | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `CompactPalletError` is not implemented for `MyError` | -note: required by `check_compactness` - --> $DIR/error.rs:30:2 +note: required by `MAX_ENCODED_SIZE` + --> $WORKSPACE/frame/support/src/traits/error.rs | -30 | fn check_compactness() -> bool { + | const MAX_ENCODED_SIZE: usize; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in the attribute macro `frame_support::pallet` (in Nightly builds, run with -Z macro-backtrace for more info) + = note: this error originates in the derive macro `frame_support::CompactPalletError` (in Nightly builds, run with -Z macro-backtrace for more info) From 69753e9aed449f26ec704d3ca108cd301cba023d Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 7 Dec 2021 17:52:52 -0800 Subject: [PATCH 21/75] Return only the most significant byte when constructing a custom InvalidTransaction --- frame/election-provider-multi-phase/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/election-provider-multi-phase/src/lib.rs b/frame/election-provider-multi-phase/src/lib.rs index 70bbed95fe973..b53eb5d84ee6b 100644 --- a/frame/election-provider-multi-phase/src/lib.rs +++ b/frame/election-provider-multi-phase/src/lib.rs @@ -1517,7 +1517,7 @@ impl ElectionProvider for Pallet { /// number. pub fn dispatch_error_to_invalid(error: DispatchError) -> InvalidTransaction { let error_number = match error { - DispatchError::Module { error, .. } => error, + DispatchError::Module { error, .. } => error[0], _ => 0, }; InvalidTransaction::Custom(error_number) From c57ccb6ea69c64211cf4fd8b9072d026104dc3e1 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 7 Dec 2021 19:05:48 -0800 Subject: [PATCH 22/75] Rename CompactPalletError to PalletError --- .../procedural/src/construct_runtime/mod.rs | 2 +- frame/support/procedural/src/lib.rs | 8 ++++---- .../procedural/src/pallet/expand/error.rs | 2 +- ...compact_pallet_error.rs => pallet_error.rs} | 18 +++++++++--------- frame/support/src/error.rs | 2 +- frame/support/src/lib.rs | 2 +- frame/support/src/traits.rs | 2 +- frame/support/src/traits/error.rs | 12 ++++++------ .../pallet_error_too_large.rs | 8 ++++---- .../tests/pallet_ui/error_not_compact.stderr | 6 +++--- .../tests/pallet_ui/pass/error_nested_types.rs | 10 +++++----- 11 files changed, 36 insertions(+), 36 deletions(-) rename frame/support/procedural/src/{compact_pallet_error.rs => pallet_error.rs} (86%) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index cdc72fcbcfcc5..21190e84794da 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -500,7 +500,7 @@ fn decl_static_assertions( } => { #scrate::const_assert! { < - #path::$error<#runtime> as #scrate::traits::CompactPalletError + #path::$error<#runtime> as #scrate::traits::PalletError >::MAX_ENCODED_SIZE <= #scrate::MAX_PALLET_ERROR_ENCODED_SIZE } }; diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index f283f50ffc5f8..bba80f64af033 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -20,7 +20,7 @@ #![recursion_limit = "512"] mod clone_no_bound; -mod compact_pallet_error; +mod pallet_error; mod construct_runtime; mod crate_version; mod debug_no_bound; @@ -564,7 +564,7 @@ pub fn match_and_insert(input: TokenStream) -> TokenStream { match_and_insert::match_and_insert(input) } -#[proc_macro_derive(CompactPalletError)] -pub fn derive_compact_pallet_error(input: TokenStream) -> TokenStream { - compact_pallet_error::derive_compact_pallet_error(input) +#[proc_macro_derive(PalletError)] +pub fn derive_pallet_error(input: TokenStream) -> TokenStream { + pallet_error::derive_pallet_error(input) } diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index dc37d49e13e80..2ac2651507fd0 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -99,7 +99,7 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { #frame_support::codec::Encode, #frame_support::codec::Decode, #frame_support::scale_info::TypeInfo, - #frame_support::CompactPalletError, + #frame_support::PalletError, )] }); error_item.attrs.push(syn::parse_quote!( diff --git a/frame/support/procedural/src/compact_pallet_error.rs b/frame/support/procedural/src/pallet_error.rs similarity index 86% rename from frame/support/procedural/src/compact_pallet_error.rs rename to frame/support/procedural/src/pallet_error.rs index d34744c7ee6e0..86359077067cc 100644 --- a/frame/support/procedural/src/compact_pallet_error.rs +++ b/frame/support/procedural/src/pallet_error.rs @@ -18,8 +18,8 @@ use frame_support_procedural_tools::generate_crate_access_2018; use std::convert::identity; -// Derive `CompactPalletError` -pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenStream { +// Derive `PalletError` +pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let syn::DeriveInput { ident: name, generics, data, .. } = match syn::parse(input) { Ok(input) => input, Err(e) => return e.to_compile_error().into(), @@ -35,7 +35,7 @@ pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro let max_encoded_size = match data { syn::Data::Struct(syn::DataStruct { struct_token, fields, .. }) => { if fields.len() > 1 { - let msg = "Cannot derive `CompactPalletError` for structs with more than 1 field"; + let msg = "Cannot derive `PalletError` for structs with more than 1 field"; return syn::Error::new(struct_token.span, msg).into_compile_error().into() } @@ -44,7 +44,7 @@ pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro let field_ty = f.named.pop().unwrap().into_value().ty; quote::quote! { < - #field_ty as #frame_support::traits::CompactPalletError + #field_ty as #frame_support::traits::PalletError >::MAX_ENCODED_SIZE } }, @@ -52,7 +52,7 @@ pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro let field_ty = f.unnamed.pop().unwrap().into_value().ty; quote::quote! { < - #field_ty as #frame_support::traits::CompactPalletError + #field_ty as #frame_support::traits::PalletError >::MAX_ENCODED_SIZE } }, @@ -65,7 +65,7 @@ pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro .map(|variant| { let span = variant.ident.span(); let make_err = || { - let msg = "Cannot derive `CompactPalletError` for enum with variants \ + let msg = "Cannot derive `PalletError` for enum with variants \ containing more than 1 field"; let err = syn::Error::new(span, msg); Err(err) @@ -116,7 +116,7 @@ pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro let mut tmp: usize; #( tmp = 1 + < - #field_tys as #frame_support::traits::CompactPalletError + #field_tys as #frame_support::traits::PalletError >::MAX_ENCODED_SIZE; size = if tmp > size { tmp } else { size }; )* @@ -125,14 +125,14 @@ pub fn derive_compact_pallet_error(input: proc_macro::TokenStream) -> proc_macro } }, syn::Data::Union(syn::DataUnion { union_token, .. }) => { - let msg = "Cannot derive `CompactPalletError` for union; please implement it directly"; + let msg = "Cannot derive `PalletError` for union; please implement it directly"; return syn::Error::new(union_token.span, msg).into_compile_error().into() }, }; quote::quote!( const _: () = { - impl #impl_generics #frame_support::traits::CompactPalletError + impl #impl_generics #frame_support::traits::PalletError for #name #ty_generics #where_clause { const MAX_ENCODED_SIZE: usize = #max_encoded_size; diff --git a/frame/support/src/error.rs b/frame/support/src/error.rs index 9453e4065b1a9..e3a5851fd5bd0 100644 --- a/frame/support/src/error.rs +++ b/frame/support/src/error.rs @@ -89,7 +89,7 @@ macro_rules! decl_error { $crate::codec::Encode, $crate::codec::Decode, $crate::scale_info::TypeInfo, - $crate::CompactPalletError, + $crate::PalletError, )] #[scale_info(skip_type_params($generic $(, $inst_generic)?), capture_docs = "always")] pub enum $error<$generic: $trait $(, $inst_generic: $instance)?> diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index df277d4899482..f0ce697c0a6c5 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -579,7 +579,7 @@ pub fn debug(data: &impl sp_std::fmt::Debug) { #[doc(inline)] pub use frame_support_procedural::{ - construct_runtime, decl_storage, match_and_insert, transactional, CompactPalletError, + construct_runtime, decl_storage, match_and_insert, transactional, PalletError, RuntimeDebugNoBound, }; diff --git a/frame/support/src/traits.rs b/frame/support/src/traits.rs index 34458d2a14cf6..98922e762f9e2 100644 --- a/frame/support/src/traits.rs +++ b/frame/support/src/traits.rs @@ -46,7 +46,7 @@ pub use validation::{ }; mod error; -pub use error::CompactPalletError; +pub use error::PalletError; mod filter; pub use filter::{ClearFilterGuard, FilterStack, FilterStackGuard, InstanceFilter, IntegrityTest}; diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index 1c5b02b4344d7..ca490cc353abf 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -19,8 +19,8 @@ use codec::{Decode, Encode, OptionBool}; use sp_std::marker::PhantomData; -/// Trait denoting that the implementing type has the most compact encoded size that is fit to be -/// included as a field in a variant of the `#[pallet::error]` enum type. +/// Trait indicating that the implementing type is going to be included as a field in a variant of +/// the `#[pallet::error]` enum type. /// /// ## Notes /// The pallet error enum has a maximum encoded size as defined by @@ -28,11 +28,11 @@ use sp_std::marker::PhantomData; /// limit, a static assertion during compilation will fail. The compilation error will be in the /// format of `error[E0080]: evaluation of constant value failed` due to the usage of /// [`static_assertions::const_assert`]. -pub trait CompactPalletError: Encode + Decode { +pub trait PalletError: Encode + Decode { /// The maximum encoded size for the implementing type. /// /// This will be used to check whether the pallet error type is less than or equal to - /// [`frame_support::MAX_PALLET_ERROR_ENCODED_SIZE`], and if it is, a compile error will be + /// [`frame_support::MAX_PALLET_ERROR_ENCODED_SIZE`], and if it is, a compilation error will be /// thrown. const MAX_ENCODED_SIZE: usize; } @@ -40,7 +40,7 @@ pub trait CompactPalletError: Encode + Decode { macro_rules! impl_for_types { ($($typ:ty),+) => { $( - impl CompactPalletError for $typ { + impl PalletError for $typ { const MAX_ENCODED_SIZE: usize = 1; } )+ @@ -49,6 +49,6 @@ macro_rules! impl_for_types { impl_for_types!(u8, i8, bool, OptionBool); -impl CompactPalletError for PhantomData { +impl PalletError for PhantomData { const MAX_ENCODED_SIZE: usize = 0; } diff --git a/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.rs b/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.rs index c4b12bba7e87e..ec661fe8e50f3 100644 --- a/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.rs +++ b/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.rs @@ -16,22 +16,22 @@ mod pallet { } } -#[derive(scale_info::TypeInfo, frame_support::CompactPalletError, codec::Encode, codec::Decode)] +#[derive(scale_info::TypeInfo, frame_support::PalletError, codec::Encode, codec::Decode)] pub enum Nested1 { Nested2(Nested2) } -#[derive(scale_info::TypeInfo, frame_support::CompactPalletError, codec::Encode, codec::Decode)] +#[derive(scale_info::TypeInfo, frame_support::PalletError, codec::Encode, codec::Decode)] pub enum Nested2 { Nested3(Nested3) } -#[derive(scale_info::TypeInfo, frame_support::CompactPalletError, codec::Encode, codec::Decode)] +#[derive(scale_info::TypeInfo, frame_support::PalletError, codec::Encode, codec::Decode)] pub enum Nested3 { Nested4(Nested4) } -#[derive(scale_info::TypeInfo, frame_support::CompactPalletError, codec::Encode, codec::Decode)] +#[derive(scale_info::TypeInfo, frame_support::PalletError, codec::Encode, codec::Decode)] pub enum Nested4 { Num(u8) } diff --git a/frame/support/test/tests/pallet_ui/error_not_compact.stderr b/frame/support/test/tests/pallet_ui/error_not_compact.stderr index 007faaabfe4f0..63ddc2d38d53d 100644 --- a/frame/support/test/tests/pallet_ui/error_not_compact.stderr +++ b/frame/support/test/tests/pallet_ui/error_not_compact.stderr @@ -1,12 +1,12 @@ -error[E0277]: the trait bound `MyError: CompactPalletError` is not satisfied +error[E0277]: the trait bound `MyError: PalletError` is not satisfied --> tests/pallet_ui/error_not_compact.rs:1:1 | 1 | #[frame_support::pallet] - | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `CompactPalletError` is not implemented for `MyError` + | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `PalletError` is not implemented for `MyError` | note: required by `MAX_ENCODED_SIZE` --> $WORKSPACE/frame/support/src/traits/error.rs | | const MAX_ENCODED_SIZE: usize; | ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^ - = note: this error originates in the derive macro `frame_support::CompactPalletError` (in Nightly builds, run with -Z macro-backtrace for more info) + = note: this error originates in the derive macro `frame_support::PalletError` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/frame/support/test/tests/pallet_ui/pass/error_nested_types.rs b/frame/support/test/tests/pallet_ui/pass/error_nested_types.rs index cf211a55db137..1b6f584af23b9 100644 --- a/frame/support/test/tests/pallet_ui/pass/error_nested_types.rs +++ b/frame/support/test/tests/pallet_ui/pass/error_nested_types.rs @@ -1,5 +1,5 @@ use codec::{Decode, Encode}; -use frame_support::CompactPalletError; +use frame_support::PalletError; #[frame_support::pallet] mod pallet { @@ -15,7 +15,7 @@ mod pallet { } } -#[derive(Encode, Decode, CompactPalletError, scale_info::TypeInfo)] +#[derive(Encode, Decode, PalletError, scale_info::TypeInfo)] pub enum MyError { Foo, Bar, @@ -24,17 +24,17 @@ pub enum MyError { Wrapper(Wrapper), } -#[derive(Encode, Decode, CompactPalletError, scale_info::TypeInfo)] +#[derive(Encode, Decode, PalletError, scale_info::TypeInfo)] pub enum NestedError { Quux } -#[derive(Encode, Decode, CompactPalletError, scale_info::TypeInfo)] +#[derive(Encode, Decode, PalletError, scale_info::TypeInfo)] pub struct MyStruct { field: u8, } -#[derive(Encode, Decode, CompactPalletError, scale_info::TypeInfo)] +#[derive(Encode, Decode, PalletError, scale_info::TypeInfo)] pub struct Wrapper(bool); fn main() { From f6f8be5b559241dff4e567941fef32a383ebd5c4 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 7 Dec 2021 19:10:13 -0800 Subject: [PATCH 23/75] Use counter to generate unique idents for assert macros --- frame/support/procedural/src/construct_runtime/mod.rs | 5 +++-- frame/support/procedural/src/lib.rs | 6 +++--- 2 files changed, 6 insertions(+), 5 deletions(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 21190e84794da..fb68666f28cff 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -144,6 +144,7 @@ mod expand; mod parse; +use crate::COUNTER; use frame_support_procedural_tools::{ generate_crate_access, generate_crate_access_2018, generate_hidden_includes, }; @@ -481,9 +482,9 @@ fn decl_static_assertions( scrate: &TokenStream2, ) -> TokenStream2 { let error_encoded_size_check = pallet_decls.iter().map(|decl| { - let name = &decl.name; + let count = COUNTER.with(|counter| counter.borrow_mut().inc()); let path = &decl.path; - let assert_macro_name = format_ident!("assert_error_encoded_size_for_{}", name); + let assert_macro_name = format_ident!("assert_error_encoded_size_{}", count); quote! { #scrate::tt_call! { diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index bba80f64af033..ca58e2d8e52c9 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -42,9 +42,9 @@ thread_local! { static COUNTER: RefCell = RefCell::new(Counter(0)); } -/// Counter to generate a relatively unique identifier for macros querying for the existence of -/// pallet parts. This is necessary because declarative macros gets hoisted to the crate root, -/// which shares the namespace with other pallets containing the very same query macros. +/// Counter to generate a relatively unique identifier for macros. This is necessary because +/// declarative macros gets hoisted to the crate root, which shares the namespace with other pallets +/// containing the very same macros. struct Counter(u64); impl Counter { From 9b923792baf317553bf332677621db713deac1db Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Mon, 13 Dec 2021 17:50:12 -0800 Subject: [PATCH 24/75] Make declarative pallet macros compile with pallet error size checks --- frame/support/procedural/src/lib.rs | 9 +- frame/support/procedural/src/tt_macro.rs | 83 +++++++ frame/support/src/dispatch.rs | 5 + frame/support/src/error.rs | 47 +--- frame/support/src/lib.rs | 2 +- frame/support/test/tests/construct_runtime.rs | 115 ++-------- frame/support/test/tests/origin.rs | 217 ++++++++++++++++++ 7 files changed, 334 insertions(+), 144 deletions(-) create mode 100644 frame/support/procedural/src/tt_macro.rs create mode 100644 frame/support/test/tests/origin.rs diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index ca58e2d8e52c9..06eb7c66c45fd 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -20,7 +20,6 @@ #![recursion_limit = "512"] mod clone_no_bound; -mod pallet_error; mod construct_runtime; mod crate_version; mod debug_no_bound; @@ -29,9 +28,11 @@ mod dummy_part_checker; mod key_prefix; mod match_and_insert; mod pallet; +mod pallet_error; mod partial_eq_no_bound; mod storage; mod transactional; +mod tt_macro; use proc_macro::TokenStream; use std::{cell::RefCell, str::FromStr}; @@ -568,3 +569,9 @@ pub fn match_and_insert(input: TokenStream) -> TokenStream { pub fn derive_pallet_error(input: TokenStream) -> TokenStream { pallet_error::derive_pallet_error(input) } + +/// Internal macro used by `frame_support` to create tt-call-compliant macros +#[proc_macro] +pub fn __create_tt_macro(input: TokenStream) -> TokenStream { + tt_macro::create_tt_return_macro(input) +} diff --git a/frame/support/procedural/src/tt_macro.rs b/frame/support/procedural/src/tt_macro.rs new file mode 100644 index 0000000000000..aac869e5db6aa --- /dev/null +++ b/frame/support/procedural/src/tt_macro.rs @@ -0,0 +1,83 @@ +// This file is part of Substrate. + +// Copyright (C) 2021 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. + +//! Implementation of the `create_tt_return_macro` macro +use crate::COUNTER; +use frame_support_procedural_tools::generate_crate_access_2018; +use proc_macro2::{Ident, TokenStream}; +use quote::format_ident; + +struct CreateTtReturnMacroDef { + name: Ident, + args: Vec<(Ident, TokenStream)>, +} + +impl syn::parse::Parse for CreateTtReturnMacroDef { + fn parse(input: syn::parse::ParseStream) -> syn::Result { + let name = input.parse()?; + let _ = input.parse::()?; + + let mut args = Vec::new(); + while !input.is_empty() { + let mut value; + let key: Ident = input.parse()?; + let _ = input.parse::()?; + let _: syn::token::Bracket = syn::bracketed!(value in input); + let _: syn::token::Brace = syn::braced!(value in value); + let value: TokenStream = value.parse()?; + + args.push((key, value)) + } + + Ok(Self { name, args }) + } +} + +pub fn create_tt_return_macro(input: proc_macro::TokenStream) -> proc_macro::TokenStream { + let CreateTtReturnMacroDef { name, args } = + syn::parse_macro_input!(input as CreateTtReturnMacroDef); + + let frame_support = match generate_crate_access_2018("frame-support") { + Ok(i) => i, + Err(e) => return e.into_compile_error().into(), + }; + let (keys, values): (Vec<_>, Vec<_>) = args.into_iter().unzip(); + let count = COUNTER.with(|counter| counter.borrow_mut().inc()); + let unique_name = format_ident!("{}_{}", name, count); + + let decl_macro = quote::quote! { + #[macro_export] + #[doc(hidden)] + macro_rules! #unique_name { + { + $caller:tt + $(frame_support = [{ $($frame_support:ident)::* }])? + } => { + #frame_support::tt_return! { + $caller + #( + #keys = [{ #values }] + )* + } + } + } + + pub use #unique_name as #name; + }; + + decl_macro.into() +} diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index a492bc12f6a38..99367802440db 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -1980,6 +1980,11 @@ macro_rules! decl_module { pub type Pallet<$trait_instance $(, $instance $( = $module_default_instance)?)?> = $mod_type<$trait_instance $(, $instance)?>; + /// Declarative macros do not support const assertions for error sizes + $crate::__create_tt_macro! { + tt_error_token, + } + $crate::decl_module! { @impl_on_initialize { $system } diff --git a/frame/support/src/error.rs b/frame/support/src/error.rs index e3a5851fd5bd0..a71e68deb8028 100644 --- a/frame/support/src/error.rs +++ b/frame/support/src/error.rs @@ -119,17 +119,6 @@ macro_rules! decl_error { impl<$generic: $trait $(, $inst_generic: $instance)?> $error<$generic $(, $inst_generic)?> $( where $( $where_ty: $where_bound ),* )? { - fn as_u8(&self) -> u8 { - $crate::decl_error! { - @GENERATE_AS_U8 - self - $error - {} - 0, - $( $name ),* - } - } - fn as_str(&self) -> &'static str { match self { Self::__Ignore(_, _) => unreachable!("`__Ignore` can never be constructed"), @@ -154,47 +143,19 @@ macro_rules! decl_error { $( where $( $where_ty: $where_bound ),* )? { fn from(err: $error<$generic $(, $inst_generic)?>) -> Self { + use $crate::codec::Encode; let index = <$generic::PalletInfo as $crate::traits::PalletInfo> ::index::<$module<$generic $(, $inst_generic)?>>() .expect("Every active module has an index in the runtime; qed") as u8; + let mut error = err.encode(); + error.resize($crate::MAX_PALLET_ERROR_ENCODED_SIZE, 0); $crate::sp_runtime::DispatchError::Module { index, - error: err.as_u8(), + error: error.try_into().expect("error has been resized to be 4 bytes; qed"), message: Some(err.as_str()), } } } }; - (@GENERATE_AS_U8 - $self:ident - $error:ident - { $( $generated:tt )* } - $index:expr, - $name:ident - $( , $rest:ident )* - ) => { - $crate::decl_error! { - @GENERATE_AS_U8 - $self - $error - { - $( $generated )* - $error::$name => $index, - } - $index + 1, - $( $rest ),* - } - }; - (@GENERATE_AS_U8 - $self:ident - $error:ident - { $( $generated:tt )* } - $index:expr, - ) => { - match $self { - $error::__Ignore(_, _) => unreachable!("`__Ignore` can never be constructed"), - $( $generated )* - } - } } diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index f0ce697c0a6c5..d10ec5aef563a 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -584,7 +584,7 @@ pub use frame_support_procedural::{ }; #[doc(hidden)] -pub use frame_support_procedural::__generate_dummy_part_checker; +pub use frame_support_procedural::{__create_tt_macro, __generate_dummy_part_checker}; /// Derive [`Clone`] but do not bound any generic. /// diff --git a/frame/support/test/tests/construct_runtime.rs b/frame/support/test/tests/construct_runtime.rs index 2d14da04f64b7..ec3cc65b7fa04 100644 --- a/frame/support/test/tests/construct_runtime.rs +++ b/frame/support/test/tests/construct_runtime.rs @@ -39,6 +39,7 @@ thread_local! { pub static INTEGRITY_TEST_EXEC: RefCell = RefCell::new(0); } +#[macro_use] mod module1 { use super::*; @@ -77,6 +78,7 @@ mod module1 { } } +#[macro_use] mod module2 { use super::*; @@ -117,9 +119,11 @@ mod module2 { } } +#[macro_use] mod nested { use super::*; + #[macro_use] pub mod module3 { use super::*; @@ -164,6 +168,7 @@ mod nested { } } +#[macro_use] pub mod module3 { use super::*; @@ -267,139 +272,51 @@ pub type Header = generic::Header; pub type Block = generic::Block; pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; -mod origin_test { - use super::{module3, nested, system, Block, UncheckedExtrinsic}; - use frame_support::traits::{Contains, OriginTrait}; - - impl nested::module3::Config for RuntimeOriginTest {} - impl module3::Config for RuntimeOriginTest {} - - pub struct BaseCallFilter; - impl Contains for BaseCallFilter { - fn contains(c: &Call) -> bool { - match c { - Call::NestedModule3(_) => true, - _ => false, - } - } - } - - impl system::Config for RuntimeOriginTest { - type BaseCallFilter = BaseCallFilter; - type Hash = super::H256; - type Origin = Origin; - type BlockNumber = super::BlockNumber; - type AccountId = u32; - type Event = Event; - type PalletInfo = PalletInfo; - type Call = Call; - type DbWeight = (); - } - - frame_support::construct_runtime!( - pub enum RuntimeOriginTest where - Block = Block, - NodeBlock = Block, - UncheckedExtrinsic = UncheckedExtrinsic - { - System: system::{Pallet, Event, Origin}, - NestedModule3: nested::module3::{Pallet, Origin, Call}, - Module3: module3::{Pallet, Origin, Call}, - } - ); - - #[test] - fn origin_default_filter() { - let accepted_call = nested::module3::Call::fail {}.into(); - let rejected_call = module3::Call::fail {}.into(); - - assert_eq!(Origin::root().filter_call(&accepted_call), true); - assert_eq!(Origin::root().filter_call(&rejected_call), true); - assert_eq!(Origin::none().filter_call(&accepted_call), true); - assert_eq!(Origin::none().filter_call(&rejected_call), false); - assert_eq!(Origin::signed(0).filter_call(&accepted_call), true); - assert_eq!(Origin::signed(0).filter_call(&rejected_call), false); - assert_eq!(Origin::from(Some(0)).filter_call(&accepted_call), true); - assert_eq!(Origin::from(Some(0)).filter_call(&rejected_call), false); - assert_eq!(Origin::from(None).filter_call(&accepted_call), true); - assert_eq!(Origin::from(None).filter_call(&rejected_call), false); - assert_eq!(Origin::from(super::nested::module3::Origin).filter_call(&accepted_call), true); - assert_eq!(Origin::from(super::nested::module3::Origin).filter_call(&rejected_call), false); - - let mut origin = Origin::from(Some(0)); - origin.add_filter(|c| matches!(c, Call::Module3(_))); - assert_eq!(origin.filter_call(&accepted_call), false); - assert_eq!(origin.filter_call(&rejected_call), false); - - // Now test for root origin and filters: - let mut origin = Origin::from(Some(0)); - origin.set_caller_from(Origin::root()); - assert!(matches!(origin.caller, OriginCaller::system(super::system::RawOrigin::Root))); - - // Root origin bypass all filter. - assert_eq!(origin.filter_call(&accepted_call), true); - assert_eq!(origin.filter_call(&rejected_call), true); - - origin.set_caller_from(Origin::from(Some(0))); - - // Back to another signed origin, the filtered are now effective again - assert_eq!(origin.filter_call(&accepted_call), true); - assert_eq!(origin.filter_call(&rejected_call), false); - - origin.set_caller_from(Origin::root()); - origin.reset_filter(); - - // Root origin bypass all filter, even when they are reset. - assert_eq!(origin.filter_call(&accepted_call), true); - assert_eq!(origin.filter_call(&rejected_call), true); - } -} - #[test] fn check_modules_error_type() { assert_eq!( Module1_1::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 31, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 31, error: [0; 4], message: Some("Something") }), ); assert_eq!( Module2::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 32, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 32, error: [0; 4], message: Some("Something") }), ); assert_eq!( Module1_2::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 33, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 33, error: [0; 4], message: Some("Something") }), ); assert_eq!( NestedModule3::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 34, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 34, error: [0; 4], message: Some("Something") }), ); assert_eq!( Module1_3::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 6, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 6, error: [0; 4], message: Some("Something") }), ); assert_eq!( Module1_4::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 3, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 3, error: [0; 4], message: Some("Something") }), ); assert_eq!( Module1_5::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 4, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 4, error: [0; 4], message: Some("Something") }), ); assert_eq!( Module1_6::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 1, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 1, error: [0; 4], message: Some("Something") }), ); assert_eq!( Module1_7::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 2, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 2, error: [0; 4], message: Some("Something") }), ); assert_eq!( Module1_8::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 12, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 12, error: [0; 4], message: Some("Something") }), ); assert_eq!( Module1_9::fail(system::Origin::::Root.into()), - Err(DispatchError::Module { index: 13, error: 0, message: Some("Something") }), + Err(DispatchError::Module { index: 13, error: [0; 4], message: Some("Something") }), ); } diff --git a/frame/support/test/tests/origin.rs b/frame/support/test/tests/origin.rs new file mode 100644 index 0000000000000..bb97c828e005d --- /dev/null +++ b/frame/support/test/tests/origin.rs @@ -0,0 +1,217 @@ +// This file is part of Substrate. + +// Copyright (C) 2021 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. + +//! Origin tests for construct_runtime macro + +#![recursion_limit = "128"] + +use frame_support::traits::{Contains, OriginTrait}; +use scale_info::TypeInfo; +use sp_core::{sr25519, H256}; +use sp_runtime::{generic, traits::BlakeTwo256}; + +mod system; + +#[macro_use] +mod nested { + use super::*; + + #[macro_use] + pub mod module { + use super::*; + + pub trait Config: system::Config {} + + frame_support::decl_module! { + pub struct Module for enum Call + where origin: ::Origin, system=system + { + #[weight = 0] + pub fn fail(_origin) -> frame_support::dispatch::DispatchResult { + Err(Error::::Something.into()) + } + } + } + + #[derive(Clone, PartialEq, Eq, Debug, codec::Encode, codec::Decode, TypeInfo)] + pub struct Origin; + + frame_support::decl_event! { + pub enum Event { + A, + } + } + + frame_support::decl_error! { + pub enum Error for Module { + Something + } + } + + frame_support::decl_storage! { + trait Store for Module as Module {} + add_extra_genesis { + build(|_config| {}) + } + } + } +} + +#[macro_use] +pub mod module { + use super::*; + + pub trait Config: system::Config {} + + frame_support::decl_module! { + pub struct Module for enum Call + where origin: ::Origin, system=system + { + #[weight = 0] + pub fn fail(_origin) -> frame_support::dispatch::DispatchResult { + Err(Error::::Something.into()) + } + #[weight = 0] + pub fn aux_1(_origin, #[compact] _data: u32) -> frame_support::dispatch::DispatchResult { + unreachable!() + } + #[weight = 0] + pub fn aux_2(_origin, _data: i32, #[compact] _data2: u32) -> frame_support::dispatch::DispatchResult { + unreachable!() + } + #[weight = 0] + fn aux_3(_origin, _data: i32, _data2: String) -> frame_support::dispatch::DispatchResult { + unreachable!() + } + #[weight = 3] + fn aux_4(_origin) -> frame_support::dispatch::DispatchResult { unreachable!() } + #[weight = (5, frame_support::weights::DispatchClass::Operational)] + fn operational(_origin) { unreachable!() } + } + } + + #[derive(Clone, PartialEq, Eq, Debug, codec::Encode, codec::Decode, TypeInfo)] + pub struct Origin(pub core::marker::PhantomData); + + frame_support::decl_event! { + pub enum Event { + A, + } + } + + frame_support::decl_error! { + pub enum Error for Module { + Something + } + } + + frame_support::decl_storage! { + trait Store for Module as Module {} + add_extra_genesis { + build(|_config| {}) + } + } +} + +impl nested::module::Config for RuntimeOriginTest {} +impl module::Config for RuntimeOriginTest {} + +pub struct BaseCallFilter; +impl Contains for BaseCallFilter { + fn contains(c: &Call) -> bool { + match c { + Call::NestedModule(_) => true, + _ => false, + } + } +} + +impl system::Config for RuntimeOriginTest { + type BaseCallFilter = BaseCallFilter; + type Hash = H256; + type Origin = Origin; + type BlockNumber = BlockNumber; + type AccountId = u32; + type Event = Event; + type PalletInfo = PalletInfo; + type Call = Call; + type DbWeight = (); +} + +frame_support::construct_runtime!( + pub enum RuntimeOriginTest where + Block = Block, + NodeBlock = Block, + UncheckedExtrinsic = UncheckedExtrinsic + { + System: system::{Pallet, Event, Origin}, + NestedModule: nested::module::{Pallet, Origin, Call}, + Module: module::{Pallet, Origin, Call}, + } +); + +pub type Signature = sr25519::Signature; +pub type BlockNumber = u64; +pub type Header = generic::Header; +pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; +pub type Block = generic::Block; + +#[test] +fn origin_default_filter() { + let accepted_call = nested::module::Call::fail {}.into(); + let rejected_call = module::Call::fail {}.into(); + + assert_eq!(Origin::root().filter_call(&accepted_call), true); + assert_eq!(Origin::root().filter_call(&rejected_call), true); + assert_eq!(Origin::none().filter_call(&accepted_call), true); + assert_eq!(Origin::none().filter_call(&rejected_call), false); + assert_eq!(Origin::signed(0).filter_call(&accepted_call), true); + assert_eq!(Origin::signed(0).filter_call(&rejected_call), false); + assert_eq!(Origin::from(Some(0)).filter_call(&accepted_call), true); + assert_eq!(Origin::from(Some(0)).filter_call(&rejected_call), false); + assert_eq!(Origin::from(None).filter_call(&accepted_call), true); + assert_eq!(Origin::from(None).filter_call(&rejected_call), false); + assert_eq!(Origin::from(nested::module::Origin).filter_call(&accepted_call), true); + assert_eq!(Origin::from(nested::module::Origin).filter_call(&rejected_call), false); + + let mut origin = Origin::from(Some(0)); + origin.add_filter(|c| matches!(c, Call::Module(_))); + assert_eq!(origin.filter_call(&accepted_call), false); + assert_eq!(origin.filter_call(&rejected_call), false); + + // Now test for root origin and filters: + let mut origin = Origin::from(Some(0)); + origin.set_caller_from(Origin::root()); + assert!(matches!(origin.caller, OriginCaller::system(system::RawOrigin::Root))); + + // Root origin bypass all filter. + assert_eq!(origin.filter_call(&accepted_call), true); + assert_eq!(origin.filter_call(&rejected_call), true); + + origin.set_caller_from(Origin::from(Some(0))); + + // Back to another signed origin, the filtered are now effective again + assert_eq!(origin.filter_call(&accepted_call), true); + assert_eq!(origin.filter_call(&rejected_call), false); + + origin.set_caller_from(Origin::root()); + origin.reset_filter(); + + // Root origin bypass all filter, even when they are reset. + assert_eq!(origin.filter_call(&accepted_call), true); + assert_eq!(origin.filter_call(&rejected_call), true); +} From b984433330a61602977901db058647de45c89068 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Mon, 13 Dec 2021 22:20:31 -0800 Subject: [PATCH 25/75] Remove unused doc comment --- frame/support/src/dispatch.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/frame/support/src/dispatch.rs b/frame/support/src/dispatch.rs index 99367802440db..5f719e65de7a4 100644 --- a/frame/support/src/dispatch.rs +++ b/frame/support/src/dispatch.rs @@ -1980,7 +1980,6 @@ macro_rules! decl_module { pub type Pallet<$trait_instance $(, $instance $( = $module_default_instance)?)?> = $mod_type<$trait_instance $(, $instance)?>; - /// Declarative macros do not support const assertions for error sizes $crate::__create_tt_macro! { tt_error_token, } From 2fb09fbe1db57856f28857ef807046e72fec2709 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 14 Dec 2021 01:59:02 -0800 Subject: [PATCH 26/75] Try and fix build errors --- frame/benchmarking/src/baseline.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/benchmarking/src/baseline.rs b/frame/benchmarking/src/baseline.rs index 2b924a692129a..ac15e042de58c 100644 --- a/frame/benchmarking/src/baseline.rs +++ b/frame/benchmarking/src/baseline.rs @@ -120,6 +120,7 @@ benchmarks! { } #[cfg(test)] +#[cfg_attr(test, macro_use)] pub mod mock { use sp_runtime::{testing::H256, traits::IdentityLookup}; From adceb34ff32efb165fb7c9657de111a56672b9b7 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Mon, 20 Dec 2021 23:46:41 -0800 Subject: [PATCH 27/75] Fix build errors --- frame/benchmarking/src/baseline.rs | 1 - frame/sudo/src/mock.rs | 1 - frame/support/procedural/src/pallet/expand/error.rs | 1 - 3 files changed, 3 deletions(-) diff --git a/frame/benchmarking/src/baseline.rs b/frame/benchmarking/src/baseline.rs index ac15e042de58c..2b924a692129a 100644 --- a/frame/benchmarking/src/baseline.rs +++ b/frame/benchmarking/src/baseline.rs @@ -120,7 +120,6 @@ benchmarks! { } #[cfg(test)] -#[cfg_attr(test, macro_use)] pub mod mock { use sp_runtime::{testing::H256, traits::IdentityLookup}; diff --git a/frame/sudo/src/mock.rs b/frame/sudo/src/mock.rs index bfbed0d38ab34..2373a7530b06a 100644 --- a/frame/sudo/src/mock.rs +++ b/frame/sudo/src/mock.rs @@ -34,7 +34,6 @@ use sp_runtime::{ // Logger module to track execution. #[frame_support::pallet] pub mod logger { - use super::*; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index 2ac2651507fd0..8a8a4c194cbcd 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -37,7 +37,6 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { error } else { return quote::quote! { - #[macro_export] #[doc(hidden)] macro_rules! #error_token_unique_id { From b018d42f4d914de4fd215d9938f58a18886d8c00 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 00:17:21 -0800 Subject: [PATCH 28/75] Add macro_use for some test modules --- frame/benchmarking/src/baseline.rs | 1 + frame/election-provider-support/src/onchain.rs | 1 + 2 files changed, 2 insertions(+) diff --git a/frame/benchmarking/src/baseline.rs b/frame/benchmarking/src/baseline.rs index 2b924a692129a..ac15e042de58c 100644 --- a/frame/benchmarking/src/baseline.rs +++ b/frame/benchmarking/src/baseline.rs @@ -120,6 +120,7 @@ benchmarks! { } #[cfg(test)] +#[cfg_attr(test, macro_use)] pub mod mock { use sp_runtime::{testing::H256, traits::IdentityLookup}; diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index 6379adae4206b..320a512e252a8 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -99,6 +99,7 @@ impl ElectionProvider for OnChainSequen } #[cfg(test)] +#[cfg_attr(test, macro_use)] mod tests { use super::*; use sp_npos_elections::Support; From 56ff09dfb52c857c7efcc6bada38ce79345ec6ed Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 00:18:39 -0800 Subject: [PATCH 29/75] Test fix --- frame/benchmarking/src/baseline.rs | 1 - frame/election-provider-support/src/onchain.rs | 1 - frame/support/procedural/src/construct_runtime/mod.rs | 3 +++ 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/frame/benchmarking/src/baseline.rs b/frame/benchmarking/src/baseline.rs index ac15e042de58c..2b924a692129a 100644 --- a/frame/benchmarking/src/baseline.rs +++ b/frame/benchmarking/src/baseline.rs @@ -120,7 +120,6 @@ benchmarks! { } #[cfg(test)] -#[cfg_attr(test, macro_use)] pub mod mock { use sp_runtime::{testing::H256, traits::IdentityLookup}; diff --git a/frame/election-provider-support/src/onchain.rs b/frame/election-provider-support/src/onchain.rs index 320a512e252a8..6379adae4206b 100644 --- a/frame/election-provider-support/src/onchain.rs +++ b/frame/election-provider-support/src/onchain.rs @@ -99,7 +99,6 @@ impl ElectionProvider for OnChainSequen } #[cfg(test)] -#[cfg_attr(test, macro_use)] mod tests { use super::*; use sp_npos_elections::Support; diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index fb68666f28cff..355f8f38dffc9 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -507,6 +507,9 @@ fn decl_static_assertions( }; {} => {}; } + + #[doc(hidden)] + pub use #assert_macro_name; } }); From 29841d94d5e7ec1f82bb6be1c2e9d64d9a64f72c Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 01:12:40 -0800 Subject: [PATCH 30/75] Fix compilation errors --- frame/support/procedural/src/construct_runtime/mod.rs | 7 ++++--- frame/support/test/tests/construct_runtime.rs | 5 ----- 2 files changed, 4 insertions(+), 8 deletions(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 355f8f38dffc9..773418567effc 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -484,13 +484,14 @@ fn decl_static_assertions( let error_encoded_size_check = pallet_decls.iter().map(|decl| { let count = COUNTER.with(|counter| counter.borrow_mut().inc()); let path = &decl.path; - let assert_macro_name = format_ident!("assert_error_encoded_size_{}", count); + let assert_macro_name = format_ident!("__assert_error_encoded_size_{}", count); + let macro_alias = format_ident!("assert_error_encoded_size_{}", count); quote! { #scrate::tt_call! { macro = [{ #path::tt_error_token }] frame_support = [{ #scrate }] - ~~> #assert_macro_name + ~~> #macro_alias } #[macro_export] @@ -509,7 +510,7 @@ fn decl_static_assertions( } #[doc(hidden)] - pub use #assert_macro_name; + pub use #assert_macro_name as #macro_alias; } }); diff --git a/frame/support/test/tests/construct_runtime.rs b/frame/support/test/tests/construct_runtime.rs index ec3cc65b7fa04..21ae370ed4f66 100644 --- a/frame/support/test/tests/construct_runtime.rs +++ b/frame/support/test/tests/construct_runtime.rs @@ -39,7 +39,6 @@ thread_local! { pub static INTEGRITY_TEST_EXEC: RefCell = RefCell::new(0); } -#[macro_use] mod module1 { use super::*; @@ -78,7 +77,6 @@ mod module1 { } } -#[macro_use] mod module2 { use super::*; @@ -119,11 +117,9 @@ mod module2 { } } -#[macro_use] mod nested { use super::*; - #[macro_use] pub mod module3 { use super::*; @@ -168,7 +164,6 @@ mod nested { } } -#[macro_use] pub mod module3 { use super::*; From 1ca9680d66aeb7b6b32fa5d3ec5856f9c87b6735 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 01:13:39 -0800 Subject: [PATCH 31/75] Remove unneeded #[macro_use] --- frame/support/test/tests/origin.rs | 3 --- 1 file changed, 3 deletions(-) diff --git a/frame/support/test/tests/origin.rs b/frame/support/test/tests/origin.rs index bb97c828e005d..1def44c15b48f 100644 --- a/frame/support/test/tests/origin.rs +++ b/frame/support/test/tests/origin.rs @@ -26,11 +26,9 @@ use sp_runtime::{generic, traits::BlakeTwo256}; mod system; -#[macro_use] mod nested { use super::*; - #[macro_use] pub mod module { use super::*; @@ -71,7 +69,6 @@ mod nested { } } -#[macro_use] pub mod module { use super::*; From b7c2b9fb39bbdd1a1404d1066e9967ee622c4dad Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 01:26:30 -0800 Subject: [PATCH 32/75] Resolve import ambiguity --- frame/scheduler/src/mock.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/scheduler/src/mock.rs b/frame/scheduler/src/mock.rs index 014f473302ab7..2bfc1a105d3f9 100644 --- a/frame/scheduler/src/mock.rs +++ b/frame/scheduler/src/mock.rs @@ -38,7 +38,7 @@ use sp_runtime::{ // Logger module to track execution. #[frame_support::pallet] pub mod logger { - use super::*; + use super::{OriginCaller, OriginTrait}; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; use std::cell::RefCell; @@ -71,7 +71,7 @@ pub mod logger { #[pallet::call] impl Pallet where - ::Origin: OriginTrait, + ::Origin: OriginTrait, { #[pallet::weight(*weight)] pub fn log(origin: OriginFor, i: u32, weight: Weight) -> DispatchResult { From 8783da542385b67dad7b951764d22a6db6c975a4 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 02:52:23 -0800 Subject: [PATCH 33/75] Make path to pallet Error enum more specific --- frame/support/procedural/src/construct_runtime/mod.rs | 3 ++- frame/support/procedural/src/pallet/expand/error.rs | 2 ++ 2 files changed, 4 insertions(+), 1 deletion(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 773418567effc..a78a226d33a6b 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -498,11 +498,12 @@ fn decl_static_assertions( #[doc(hidden)] macro_rules! #assert_macro_name { { + pallet_module_name = [{ $mod_name:ident }] error = [{ $error:ident }] } => { #scrate::const_assert! { < - #path::$error<#runtime> as #scrate::traits::PalletError + #path::$mod_name::$error<#runtime> as #scrate::traits::PalletError >::MAX_ENCODED_SIZE <= #scrate::MAX_PALLET_ERROR_ENCODED_SIZE } }; diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index 8a8a4c194cbcd..84b6ae6233b55 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -54,6 +54,7 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { } }; + let pallet_module_name = &def.item.ident; let error_ident = &error.error; let type_impl_gen = &def.type_impl_generics(error.attr_span); let type_use_gen = &def.type_use_generics(error.attr_span); @@ -173,6 +174,7 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { } => { $($frame_support::)*tt_return! { $caller + pallet_module_name = [{ #pallet_module_name }] error = [{ #error_ident }] } }; From 1f991ff8e1269ca5752c58616b5ec764b4ba9629 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 02:54:52 -0800 Subject: [PATCH 34/75] Fix test expectation --- frame/election-provider-multi-phase/src/unsigned.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index 1770f4343a0a4..9e8bd9347c00e 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -1037,7 +1037,7 @@ mod tests { MultiPhase::mine_check_save_submit().unwrap_err(), MinerError::PreDispatchChecksFailed(DispatchError::Module { index: 2, - error: 1, + error: [1, 0, 0, 0], message: Some("PreDispatchWrongWinnerCount"), }), ); From d7e3597f226551f37b73719ebde6ab54c723e89c Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 03:09:25 -0800 Subject: [PATCH 35/75] Disambiguate imports --- frame/utility/src/tests.rs | 1 - 1 file changed, 1 deletion(-) diff --git a/frame/utility/src/tests.rs b/frame/utility/src/tests.rs index 11b63254eb40b..0dad50f2487e7 100644 --- a/frame/utility/src/tests.rs +++ b/frame/utility/src/tests.rs @@ -38,7 +38,6 @@ use sp_runtime::{ // example module to test behaviors. #[frame_support::pallet] pub mod example { - use super::*; use frame_support::{dispatch::WithPostDispatchInfo, pallet_prelude::*}; use frame_system::pallet_prelude::*; From aea23e073a5dabc369eed8b8bcf154e6fbeb848b Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 03:34:49 -0800 Subject: [PATCH 36/75] Fix test expectations --- frame/support/test/tests/pallet_instance.rs | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/frame/support/test/tests/pallet_instance.rs b/frame/support/test/tests/pallet_instance.rs index 740bfe51d439d..17652f5dacff4 100644 --- a/frame/support/test/tests/pallet_instance.rs +++ b/frame/support/test/tests/pallet_instance.rs @@ -341,7 +341,11 @@ fn error_expand() { ); assert_eq!( DispatchError::from(pallet::Error::::InsufficientProposersBalance), - DispatchError::Module { index: 1, error: 0, message: Some("InsufficientProposersBalance") }, + DispatchError::Module { + index: 1, + error: [0; 4], + message: Some("InsufficientProposersBalance"), + }, ); assert_eq!( @@ -358,7 +362,11 @@ fn error_expand() { DispatchError::from( pallet::Error::::InsufficientProposersBalance ), - DispatchError::Module { index: 2, error: 0, message: Some("InsufficientProposersBalance") }, + DispatchError::Module { + index: 2, + error: [0; 4], + message: Some("InsufficientProposersBalance"), + }, ); } From 1a4bcdf3daef5aa643eacf719be6a7e2f019a3a6 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 03:35:17 -0800 Subject: [PATCH 37/75] Revert appending pallet module name to path --- frame/support/procedural/src/construct_runtime/mod.rs | 3 +-- frame/support/procedural/src/pallet/expand/error.rs | 2 -- 2 files changed, 1 insertion(+), 4 deletions(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index a78a226d33a6b..773418567effc 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -498,12 +498,11 @@ fn decl_static_assertions( #[doc(hidden)] macro_rules! #assert_macro_name { { - pallet_module_name = [{ $mod_name:ident }] error = [{ $error:ident }] } => { #scrate::const_assert! { < - #path::$mod_name::$error<#runtime> as #scrate::traits::PalletError + #path::$error<#runtime> as #scrate::traits::PalletError >::MAX_ENCODED_SIZE <= #scrate::MAX_PALLET_ERROR_ENCODED_SIZE } }; diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index 84b6ae6233b55..8a8a4c194cbcd 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -54,7 +54,6 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { } }; - let pallet_module_name = &def.item.ident; let error_ident = &error.error; let type_impl_gen = &def.type_impl_generics(error.attr_span); let type_use_gen = &def.type_use_generics(error.attr_span); @@ -174,7 +173,6 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { } => { $($frame_support::)*tt_return! { $caller - pallet_module_name = [{ #pallet_module_name }] error = [{ #error_ident }] } }; From d2e42ef8468eb4f5d80626d9db542b2ee711394d Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 03:36:17 -0800 Subject: [PATCH 38/75] Rename bags_list::list::Error to BagError --- frame/bags-list/src/list/mod.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index 4524101d793cf..11fcffa67cb89 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -38,7 +38,7 @@ use sp_std::{ }; #[derive(Debug, PartialEq, Eq)] -pub enum Error { +pub enum BagError { /// A duplicate id has been detected. Duplicate, } @@ -261,9 +261,9 @@ impl List { /// Insert a new id into the appropriate bag in the list. /// /// Returns an error if the list already contains `id`. - pub(crate) fn insert(id: T::AccountId, weight: VoteWeight) -> Result<(), Error> { + pub(crate) fn insert(id: T::AccountId, weight: VoteWeight) -> Result<(), BagError> { if Self::contains(&id) { - return Err(Error::Duplicate) + return Err(BagError::Duplicate) } let bag_weight = notional_bag_for::(weight); From 9ab1b560106acdfb98447bc6aeffb8b35c6038e1 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 03:37:03 -0800 Subject: [PATCH 39/75] Fixes --- frame/bags-list/src/list/tests.rs | 2 +- frame/bags-list/src/tests.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/bags-list/src/list/tests.rs b/frame/bags-list/src/list/tests.rs index f3043589681ec..4a5cad08f11ee 100644 --- a/frame/bags-list/src/list/tests.rs +++ b/frame/bags-list/src/list/tests.rs @@ -242,7 +242,7 @@ mod list { // then assert_storage_noop!(assert_eq!( List::::insert(3, 20).unwrap_err(), - Error::Duplicate + BagError::Duplicate )); }); } diff --git a/frame/bags-list/src/tests.rs b/frame/bags-list/src/tests.rs index 8f1ccacaf1171..ee4e2e12a8fcb 100644 --- a/frame/bags-list/src/tests.rs +++ b/frame/bags-list/src/tests.rs @@ -518,7 +518,7 @@ mod sorted_list_provider { // then assert_storage_noop!(assert_eq!( BagsList::on_insert(3, 20).unwrap_err(), - Error::Duplicate + BagError::Duplicate )); }); } From 1be322706d3aaf7080426340efb6210983e80335 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 03:41:23 -0800 Subject: [PATCH 40/75] Fixes --- frame/bags-list/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index 193a334cf08f6..bb57c53a69d89 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -67,7 +67,7 @@ pub mod mock; mod tests; pub mod weights; -pub use list::{notional_bag_for, Bag, Error, List, Node}; +pub use list::{notional_bag_for, Bag, BagError, List, Node}; pub use pallet::*; pub use weights::WeightInfo; From 034285952b35394957f4c1a95c6d8a80bbfad491 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 03:43:11 -0800 Subject: [PATCH 41/75] Fixes --- frame/bags-list/src/lib.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index bb57c53a69d89..2452b942dcd05 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -255,7 +255,7 @@ impl Pallet { } impl SortedListProvider for Pallet { - type Error = Error; + type Error = BagError; fn iter() -> Box> { Box::new(List::::iter().map(|n| n.id().clone())) @@ -269,7 +269,7 @@ impl SortedListProvider for Pallet { List::::contains(id) } - fn on_insert(id: T::AccountId, weight: VoteWeight) -> Result<(), Error> { + fn on_insert(id: T::AccountId, weight: VoteWeight) -> Result<(), BagError> { List::::insert(id, weight) } From 066ec5fd185e814618758d3312307db742bea9ee Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 04:04:29 -0800 Subject: [PATCH 42/75] Fix test expectations --- primitives/runtime/src/lib.rs | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index db4a2e8c1938a..262c2de8735de 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -934,11 +934,12 @@ mod tests { #[test] fn dispatch_error_encoding() { - let error = DispatchError::Module { index: 1, error: 2, message: Some("error message") }; + let error = + DispatchError::Module { index: 1, error: [2, 0, 0, 0], message: Some("error message") }; let encoded = error.encode(); let decoded = DispatchError::decode(&mut &encoded[..]).unwrap(); assert_eq!(encoded, vec![3, 1, 2]); - assert_eq!(decoded, DispatchError::Module { index: 1, error: 2, message: None }); + assert_eq!(decoded, DispatchError::Module { index: 1, error: [2, 0, 0, 0], message: None }); } #[test] @@ -950,9 +951,9 @@ mod tests { Other("bar"), CannotLookup, BadOrigin, - Module { index: 1, error: 1, message: None }, - Module { index: 1, error: 2, message: None }, - Module { index: 2, error: 1, message: None }, + Module { index: 1, error: [1, 0, 0, 0], message: None }, + Module { index: 1, error: [2, 0, 0, 0], message: None }, + Module { index: 2, error: [1, 0, 0, 0], message: None }, ConsumerRemaining, NoProviders, Token(TokenError::NoFunds), @@ -977,8 +978,8 @@ mod tests { // Ignores `message` field in `Module` variant. assert_eq!( - Module { index: 1, error: 1, message: Some("foo") }, - Module { index: 1, error: 1, message: None }, + Module { index: 1, error: [1, 0, 0, 0], message: Some("foo") }, + Module { index: 1, error: [1, 0, 0, 0], message: None }, ); } From 1f3ab67602d0b7b8d84491e08896f80d2d78f76f Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 04:46:08 -0800 Subject: [PATCH 43/75] Fix test expectation --- frame/election-provider-multi-phase/src/unsigned.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/election-provider-multi-phase/src/unsigned.rs b/frame/election-provider-multi-phase/src/unsigned.rs index 9e8bd9347c00e..d815723bddb6b 100644 --- a/frame/election-provider-multi-phase/src/unsigned.rs +++ b/frame/election-provider-multi-phase/src/unsigned.rs @@ -924,7 +924,7 @@ mod tests { #[test] #[should_panic(expected = "Invalid unsigned submission must produce invalid block and \ deprive validator from their authoring reward.: \ - Module { index: 2, error: 1, message: \ + Module { index: 2, error: [1, 0, 0, 0], message: \ Some(\"PreDispatchWrongWinnerCount\") }")] fn unfeasible_solution_panics() { ExtBuilder::default().build_and_execute(|| { From b45ca964f0e989a38dad62e0ba4e1108d602d0bb Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 05:00:14 -0800 Subject: [PATCH 44/75] Add more implementations for PalletError --- frame/support/src/traits/error.rs | 38 ++++++++++++++++++++++++++++--- 1 file changed, 35 insertions(+), 3 deletions(-) diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index ca490cc353abf..b2f3dc25cb893 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -38,17 +38,49 @@ pub trait PalletError: Encode + Decode { } macro_rules! impl_for_types { - ($($typ:ty),+) => { + (size: $size:expr, $($typ:ty),+) => { $( impl PalletError for $typ { - const MAX_ENCODED_SIZE: usize = 1; + const MAX_ENCODED_SIZE: usize = $size; } )+ }; } -impl_for_types!(u8, i8, bool, OptionBool); +impl_for_types!(size: 0, ()); +impl_for_types!(size: 1, u8, i8, bool, OptionBool); +impl_for_types!(size: 2, u16, i16); +impl_for_types!(size: 4, u32, i32); +impl_for_types!(size: 8, u64, i64); +// Contains a u64 for secs and u32 for nanos, hence 12 bytes +impl_for_types!(size: 12, core::time::Duration); +impl_for_types!(size: 16, u128, i128); impl PalletError for PhantomData { const MAX_ENCODED_SIZE: usize = 0; } + +impl PalletError for core::ops::Range { + const MAX_ENCODED_SIZE: usize = 2 * T::MAX_ENCODED_SIZE; +} + +impl PalletError for [T; N] { + const MAX_ENCODED_SIZE: usize = T::MAX_ENCODED_SIZE * N; +} + +impl PalletError for Option { + const MAX_ENCODED_SIZE: usize = 1 + T::MAX_ENCODED_SIZE; +} + +impl PalletError for Result { + const MAX_ENCODED_SIZE: usize = 1 + if T::MAX_ENCODED_SIZE > E::MAX_ENCODED_SIZE { + T::MAX_ENCODED_SIZE + } else { + E::MAX_ENCODED_SIZE + }; +} + +#[impl_trait_for_tuples::impl_for_tuples(1, 18)] +impl PalletError for Tuple { + for_tuples!( const MAX_ENCODED_SIZE: usize = #(Tuple::MAX_ENCODED_SIZE)+*; ); +} From 981a29f95e314a2bdc380b40aa31484ab7f68551 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 19:19:44 -0800 Subject: [PATCH 45/75] Lift the 1-field requirement for nested pallet errors --- .../procedural/src/construct_runtime/mod.rs | 3 + .../procedural/src/pallet/expand/error.rs | 4 +- .../procedural/src/pallet/parse/error.rs | 17 +--- frame/support/procedural/src/pallet_error.rs | 89 ++++++++----------- ... => error_does_not_derive_pallet_error.rs} | 0 ...error_does_not_derive_pallet_error.stderr} | 0 6 files changed, 46 insertions(+), 67 deletions(-) rename frame/support/test/tests/pallet_ui/{error_not_compact.rs => error_does_not_derive_pallet_error.rs} (100%) rename frame/support/test/tests/pallet_ui/{error_not_compact.stderr => error_does_not_derive_pallet_error.stderr} (100%) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 773418567effc..16e1d92dd59b6 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -484,6 +484,9 @@ fn decl_static_assertions( let error_encoded_size_check = pallet_decls.iter().map(|decl| { let count = COUNTER.with(|counter| counter.borrow_mut().inc()); let path = &decl.path; + // This weirdness is required because declarative macros gets hoisted up to the crate root, + // and thus doesn't appear in the same module namespace as the tt_call macro. We use a + // re-export hack to make the macro appear in the same module namespace. let assert_macro_name = format_ident!("__assert_error_encoded_size_{}", count); let macro_alias = format_ident!("assert_error_encoded_size_{}", count); diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index 8a8a4c194cbcd..6f2c2e18a5fc0 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -70,10 +70,10 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { let as_str_matches = error.variants.iter().map(|(variant, field_ty, _)| { let variant_str = format!("{}", variant); match field_ty { - Some(VariantField { is_named: true, .. }) => { + Some(VariantField { is_named: true }) => { quote::quote_spanned!(error.attr_span => Self::#variant { .. } => #variant_str,) }, - Some(VariantField { is_named: false, .. }) => { + Some(VariantField { is_named: false }) => { quote::quote_spanned!(error.attr_span => Self::#variant(..) => #variant_str,) }, None => { diff --git a/frame/support/procedural/src/pallet/parse/error.rs b/frame/support/procedural/src/pallet/parse/error.rs index deafac6d30f6a..f79a4a29beea9 100644 --- a/frame/support/procedural/src/pallet/parse/error.rs +++ b/frame/support/procedural/src/pallet/parse/error.rs @@ -27,8 +27,6 @@ mod keyword { /// Records information about the error enum variants. pub struct VariantField { - /// The type of the field in the variant. - pub ty: syn::Type, /// Whether or not the field is named, i.e. whether it is a tuple variant or struct variant. pub is_named: bool, } @@ -80,19 +78,8 @@ impl ErrorDef { .map(|variant| { let field_ty = match &variant.fields { Fields::Unit => None, - Fields::Named(f) if f.named.len() == 1 => Some(VariantField { - ty: f.named.first().unwrap().ty.clone(), - is_named: true, - }), - Fields::Unnamed(u) if u.unnamed.len() == 1 => Some(VariantField { - ty: u.unnamed.first().unwrap().ty.clone(), - is_named: false, - }), - _ => { - let msg = "Invalid pallet::error, unexpected fields, must be `Unit` or \ - contain only 1 field"; - return Err(syn::Error::new(variant.fields.span(), msg)) - }, + Fields::Named(_) => Some(VariantField { is_named: true }), + Fields::Unnamed(_) => Some(VariantField { is_named: false }), }; if variant.discriminant.is_some() { let msg = "Invalid pallet::error, unexpected discriminant, discriminants \ diff --git a/frame/support/procedural/src/pallet_error.rs b/frame/support/procedural/src/pallet_error.rs index 86359077067cc..a4ff2d4818ba9 100644 --- a/frame/support/procedural/src/pallet_error.rs +++ b/frame/support/procedural/src/pallet_error.rs @@ -33,52 +33,33 @@ pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenS let (impl_generics, ty_generics, where_clause) = generics.split_for_impl(); let max_encoded_size = match data { - syn::Data::Struct(syn::DataStruct { struct_token, fields, .. }) => { - if fields.len() > 1 { - let msg = "Cannot derive `PalletError` for structs with more than 1 field"; - return syn::Error::new(struct_token.span, msg).into_compile_error().into() - } - - match fields { - syn::Fields::Named(mut f) if f.named.len() == 1 => { - let field_ty = f.named.pop().unwrap().into_value().ty; - quote::quote! { - < - #field_ty as #frame_support::traits::PalletError - >::MAX_ENCODED_SIZE - } - }, - syn::Fields::Unnamed(mut f) if f.unnamed.len() == 1 => { - let field_ty = f.unnamed.pop().unwrap().into_value().ty; - quote::quote! { - < - #field_ty as #frame_support::traits::PalletError - >::MAX_ENCODED_SIZE - } - }, - _ => quote::quote!(1), - } + syn::Data::Struct(syn::DataStruct { fields, .. }) => match fields { + syn::Fields::Named(f) => { + let field_tys = f.named.iter().map(|field| &field.ty); + quote::quote! { + #(< + #field_tys as #frame_support::traits::PalletError + >::MAX_ENCODED_SIZE)+* + } + }, + syn::Fields::Unnamed(f) => { + let field_tys = f.unnamed.iter().map(|field| &field.ty); + quote::quote! { + #(< + #field_tys as #frame_support::traits::PalletError + >::MAX_ENCODED_SIZE)+* + } + }, + syn::Fields::Unit => quote::quote!(0), }, syn::Data::Enum(syn::DataEnum { variants, .. }) => { let field_tys = variants - .into_iter() + .iter() .map(|variant| { - let span = variant.ident.span(); - let make_err = || { - let msg = "Cannot derive `PalletError` for enum with variants \ - containing more than 1 field"; - let err = syn::Error::new(span, msg); - Err(err) - }; - - match variant.fields { - syn::Fields::Named(mut f) if f.named.len() == 1 => - Ok(Some(f.named.pop().unwrap().into_value().ty)), - syn::Fields::Unnamed(mut f) if f.unnamed.len() == 1 => - Ok(Some(f.unnamed.pop().unwrap().into_value().ty)), - syn::Fields::Unnamed(mut f) if f.unnamed.len() == 2 => { - let second = f.unnamed.pop().unwrap().into_value().ty; - let first = f.unnamed.pop().unwrap().into_value().ty; + match &variant.fields { + syn::Fields::Unnamed(f) if f.unnamed.len() == 2 => { + let first = &f.unnamed.first().unwrap().ty; + let second = &f.unnamed.last().unwrap().ty; match (first, second) { // Check whether we have (PhantomData, Never), if so we skip it. @@ -93,15 +74,17 @@ pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenS .last() .map_or(false, |seg| seg.ident == "Never") => Ok(None), - // Otherwise, it's an error. - _ => make_err(), + _ => Ok(Some(vec![first, second])), } }, + syn::Fields::Named(f) => + Ok(Some(f.named.iter().map(|field| &field.ty).collect::>())), + syn::Fields::Unnamed(f) => + Ok(Some(f.unnamed.iter().map(|field| &field.ty).collect::>())), syn::Fields::Unit => Ok(None), - _ => make_err(), } }) - .collect::>, syn::Error>>(); + .collect::>>, syn::Error>>(); let field_tys = match field_tys { Ok(tys) => tys.into_iter().filter_map(identity).collect::>(), @@ -111,13 +94,19 @@ pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenS if field_tys.is_empty() { quote::quote!(1) } else { + let variant_sizes = field_tys.into_iter().map(|variant_field_tys| { + quote::quote! { + #(< + #variant_field_tys as #frame_support::traits::PalletError + >::MAX_ENCODED_SIZE)+* + } + }); + quote::quote! {{ let mut size = 1; - let mut tmp: usize; + let mut tmp = 1; #( - tmp = 1 + < - #field_tys as #frame_support::traits::PalletError - >::MAX_ENCODED_SIZE; + tmp += #variant_sizes; size = if tmp > size { tmp } else { size }; )* size diff --git a/frame/support/test/tests/pallet_ui/error_not_compact.rs b/frame/support/test/tests/pallet_ui/error_does_not_derive_pallet_error.rs similarity index 100% rename from frame/support/test/tests/pallet_ui/error_not_compact.rs rename to frame/support/test/tests/pallet_ui/error_does_not_derive_pallet_error.rs diff --git a/frame/support/test/tests/pallet_ui/error_not_compact.stderr b/frame/support/test/tests/pallet_ui/error_does_not_derive_pallet_error.stderr similarity index 100% rename from frame/support/test/tests/pallet_ui/error_not_compact.stderr rename to frame/support/test/tests/pallet_ui/error_does_not_derive_pallet_error.stderr From 0ef28121a0c6c08179c26b130433476e5238083a Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 19:20:51 -0800 Subject: [PATCH 46/75] Fix UI test expectation --- .../tests/pallet_ui/error_does_not_derive_pallet_error.stderr | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/test/tests/pallet_ui/error_does_not_derive_pallet_error.stderr b/frame/support/test/tests/pallet_ui/error_does_not_derive_pallet_error.stderr index 63ddc2d38d53d..2a8149e309ac1 100644 --- a/frame/support/test/tests/pallet_ui/error_does_not_derive_pallet_error.stderr +++ b/frame/support/test/tests/pallet_ui/error_does_not_derive_pallet_error.stderr @@ -1,5 +1,5 @@ error[E0277]: the trait bound `MyError: PalletError` is not satisfied - --> tests/pallet_ui/error_not_compact.rs:1:1 + --> tests/pallet_ui/error_does_not_derive_pallet_error.rs:1:1 | 1 | #[frame_support::pallet] | ^^^^^^^^^^^^^^^^^^^^^^^^ the trait `PalletError` is not implemented for `MyError` From fb1b32b2f473909664caeb37286e493fa3cf03a8 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 21 Dec 2021 19:36:30 -0800 Subject: [PATCH 47/75] Remove PalletError impl for OptionBool --- frame/support/src/traits/error.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index b2f3dc25cb893..09929ca7b990a 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -16,7 +16,7 @@ // limitations under the License. //! Traits for describing and constraining pallet error types. -use codec::{Decode, Encode, OptionBool}; +use codec::{Decode, Encode}; use sp_std::marker::PhantomData; /// Trait indicating that the implementing type is going to be included as a field in a variant of @@ -48,7 +48,7 @@ macro_rules! impl_for_types { } impl_for_types!(size: 0, ()); -impl_for_types!(size: 1, u8, i8, bool, OptionBool); +impl_for_types!(size: 1, u8, i8, bool); impl_for_types!(size: 2, u16, i16); impl_for_types!(size: 4, u32, i32); impl_for_types!(size: 8, u64, i64); From fb73b3369c11e46fd2467ad6e296f18255526fd3 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 23 Dec 2021 00:48:12 -0800 Subject: [PATCH 48/75] Use saturating operations --- frame/support/procedural/src/pallet_error.rs | 22 ++++++++++++-------- frame/support/src/traits/error.rs | 16 ++++++++------ 2 files changed, 23 insertions(+), 15 deletions(-) diff --git a/frame/support/procedural/src/pallet_error.rs b/frame/support/procedural/src/pallet_error.rs index a4ff2d4818ba9..fc22aafc74a7d 100644 --- a/frame/support/procedural/src/pallet_error.rs +++ b/frame/support/procedural/src/pallet_error.rs @@ -37,17 +37,19 @@ pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenS syn::Fields::Named(f) => { let field_tys = f.named.iter().map(|field| &field.ty); quote::quote! { - #(< + 0_usize + #(.saturating_add(< #field_tys as #frame_support::traits::PalletError - >::MAX_ENCODED_SIZE)+* + >::MAX_ENCODED_SIZE))* } }, syn::Fields::Unnamed(f) => { let field_tys = f.unnamed.iter().map(|field| &field.ty); quote::quote! { - #(< + 0_usize + #(.saturating_add(< #field_tys as #frame_support::traits::PalletError - >::MAX_ENCODED_SIZE)+* + >::MAX_ENCODED_SIZE))* } }, syn::Fields::Unit => quote::quote!(0), @@ -96,18 +98,20 @@ pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenS } else { let variant_sizes = field_tys.into_iter().map(|variant_field_tys| { quote::quote! { - #(< + 1_usize + #(.saturating_add(< #variant_field_tys as #frame_support::traits::PalletError - >::MAX_ENCODED_SIZE)+* + >::MAX_ENCODED_SIZE))* } }); quote::quote! {{ - let mut size = 1; - let mut tmp = 1; + let mut size = 1_usize; + let mut tmp = 0_usize; #( - tmp += #variant_sizes; + tmp = #variant_sizes; size = if tmp > size { tmp } else { size }; + tmp = 0_usize; )* size }} diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index 09929ca7b990a..e8b6168c01603 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -61,26 +61,30 @@ impl PalletError for PhantomData { } impl PalletError for core::ops::Range { - const MAX_ENCODED_SIZE: usize = 2 * T::MAX_ENCODED_SIZE; + const MAX_ENCODED_SIZE: usize = T::MAX_ENCODED_SIZE.saturating_mul(2); } impl PalletError for [T; N] { - const MAX_ENCODED_SIZE: usize = T::MAX_ENCODED_SIZE * N; + const MAX_ENCODED_SIZE: usize = T::MAX_ENCODED_SIZE.saturating_mul(N); } impl PalletError for Option { - const MAX_ENCODED_SIZE: usize = 1 + T::MAX_ENCODED_SIZE; + const MAX_ENCODED_SIZE: usize = T::MAX_ENCODED_SIZE.saturating_add(1); } impl PalletError for Result { - const MAX_ENCODED_SIZE: usize = 1 + if T::MAX_ENCODED_SIZE > E::MAX_ENCODED_SIZE { + const MAX_ENCODED_SIZE: usize = if T::MAX_ENCODED_SIZE > E::MAX_ENCODED_SIZE { T::MAX_ENCODED_SIZE } else { E::MAX_ENCODED_SIZE - }; + }.saturating_add(1); } #[impl_trait_for_tuples::impl_for_tuples(1, 18)] impl PalletError for Tuple { - for_tuples!( const MAX_ENCODED_SIZE: usize = #(Tuple::MAX_ENCODED_SIZE)+*; ); + const MAX_ENCODED_SIZE: usize = { + let mut size = 0_usize; + for_tuples!( #(size = size.saturating_add(Tuple::MAX_ENCODED_SIZE);)* ); + size + }; } From 8e8a27afec4b55a858e659eea260d1fc6d6b2a43 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 23 Dec 2021 01:14:35 -0800 Subject: [PATCH 49/75] cargo fmt --- frame/support/src/traits/error.rs | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index e8b6168c01603..600dd8b0c21ad 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -77,7 +77,8 @@ impl PalletError for Result { T::MAX_ENCODED_SIZE } else { E::MAX_ENCODED_SIZE - }.saturating_add(1); + } + .saturating_add(1); } #[impl_trait_for_tuples::impl_for_tuples(1, 18)] From 2a547366c8fb342f3bd2da78787cf58304de28c8 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 23 Dec 2021 01:15:35 -0800 Subject: [PATCH 50/75] Delete obsolete test --- .../pallet_ui/error_more_than_1_field.rs | 26 ------------------- .../pallet_ui/error_more_than_1_field.stderr | 5 ---- 2 files changed, 31 deletions(-) delete mode 100644 frame/support/test/tests/pallet_ui/error_more_than_1_field.rs delete mode 100644 frame/support/test/tests/pallet_ui/error_more_than_1_field.stderr diff --git a/frame/support/test/tests/pallet_ui/error_more_than_1_field.rs b/frame/support/test/tests/pallet_ui/error_more_than_1_field.rs deleted file mode 100644 index e972d9c205af3..0000000000000 --- a/frame/support/test/tests/pallet_ui/error_more_than_1_field.rs +++ /dev/null @@ -1,26 +0,0 @@ -#[frame_support::pallet] -mod pallet { - use frame_support::pallet_prelude::Hooks; - use frame_system::pallet_prelude::BlockNumberFor; - - #[pallet::config] - pub trait Config: frame_system::Config {} - - #[pallet::pallet] - pub struct Pallet(core::marker::PhantomData); - - #[pallet::hooks] - impl Hooks> for Pallet {} - - #[pallet::call] - impl Pallet {} - - #[pallet::error] - pub enum Error { - Tuple(u8, u8), - U8(u8), - } -} - -fn main() { -} diff --git a/frame/support/test/tests/pallet_ui/error_more_than_1_field.stderr b/frame/support/test/tests/pallet_ui/error_more_than_1_field.stderr deleted file mode 100644 index f764db17a29b0..0000000000000 --- a/frame/support/test/tests/pallet_ui/error_more_than_1_field.stderr +++ /dev/null @@ -1,5 +0,0 @@ -error: Invalid pallet::error, unexpected fields, must be `Unit` or contain only 1 field - --> $DIR/error_more_than_1_field.rs:20:8 - | -20 | Tuple(u8, u8), - | ^^^^^^^^ From a250788b6c747336e4554e31f456ad17adb71a0c Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 23 Dec 2021 20:13:44 -0800 Subject: [PATCH 51/75] Fix test expectation --- primitives/runtime/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 262c2de8735de..020311ba03570 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -938,7 +938,7 @@ mod tests { DispatchError::Module { index: 1, error: [2, 0, 0, 0], message: Some("error message") }; let encoded = error.encode(); let decoded = DispatchError::decode(&mut &encoded[..]).unwrap(); - assert_eq!(encoded, vec![3, 1, 2]); + assert_eq!(encoded, vec![3, 1, 2, 0, 0, 0]); assert_eq!(decoded, DispatchError::Module { index: 1, error: [2, 0, 0, 0], message: None }); } From c44e6e46b4d3ca1c37559297de65655e7e949117 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Wed, 19 Jan 2022 01:37:49 -0800 Subject: [PATCH 52/75] Try and use assert macro in const context --- Cargo.lock | 1 - frame/support/Cargo.toml | 1 - .../procedural/src/construct_runtime/mod.rs | 12 +++++++++--- frame/support/src/lib.rs | 2 -- .../test/tests/pallet_ui/error_size_too_large.rs | 16 ++++++++++++++++ 5 files changed, 25 insertions(+), 7 deletions(-) create mode 100644 frame/support/test/tests/pallet_ui/error_size_too_large.rs diff --git a/Cargo.lock b/Cargo.lock index 1f3f5228cf6f6..b65795a373b35 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -2126,7 +2126,6 @@ dependencies = [ "sp-state-machine", "sp-std", "sp-tracing", - "static_assertions", "tt-call", ] diff --git a/frame/support/Cargo.toml b/frame/support/Cargo.toml index 206938de73eab..efc7caab1bd4e 100644 --- a/frame/support/Cargo.toml +++ b/frame/support/Cargo.toml @@ -25,7 +25,6 @@ sp-core = { version = "4.1.0-dev", default-features = false, path = "../../primi sp-arithmetic = { version = "4.0.0", default-features = false, path = "../../primitives/arithmetic" } sp-inherents = { version = "4.0.0-dev", default-features = false, path = "../../primitives/inherents" } sp-staking = { version = "4.0.0-dev", default-features = false, path = "../../primitives/staking" } -static_assertions = "1.1.0" tt-call = "1.0.8" frame-support-procedural = { version = "4.0.0-dev", default-features = false, path = "./procedural" } paste = "1.0" diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 1fba877d2499e..a16834c3984dd 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -488,6 +488,11 @@ fn decl_static_assertions( // and thus doesn't appear in the same module namespace as the tt_call macro. We use a // re-export hack to make the macro appear in the same module namespace. let assert_macro_name = format_ident!("__assert_error_encoded_size_{}", count); + let assert_message = format!( + "The maximum encoded size of the error type in the `{}` pallet exceeds \ + `MAX_PALLET_ERROR_ENCODED_SIZE`", + decl.name, + ); let macro_alias = format_ident!("assert_error_encoded_size_{}", count); quote! { @@ -503,11 +508,12 @@ fn decl_static_assertions( { error = [{ $error:ident }] } => { - #scrate::const_assert! { + const _: () = assert!( < #path::$error<#runtime> as #scrate::traits::PalletError - >::MAX_ENCODED_SIZE <= #scrate::MAX_PALLET_ERROR_ENCODED_SIZE - } + >::MAX_ENCODED_SIZE <= #scrate::MAX_PALLET_ERROR_ENCODED_SIZE, + #assert_message + ); }; {} => {}; } diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 0e892dc024261..f193255371f1c 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -53,8 +53,6 @@ pub use sp_state_machine::BasicExternalities; #[doc(hidden)] pub use sp_std; #[doc(hidden)] -pub use static_assertions::*; -#[doc(hidden)] pub use tt_call::*; #[macro_use] diff --git a/frame/support/test/tests/pallet_ui/error_size_too_large.rs b/frame/support/test/tests/pallet_ui/error_size_too_large.rs new file mode 100644 index 0000000000000..a8b9cdb672384 --- /dev/null +++ b/frame/support/test/tests/pallet_ui/error_size_too_large.rs @@ -0,0 +1,16 @@ +#[frame_support::pallet] +mod pallet { + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::pallet] + pub struct Pallet(core::marker::PhantomData); + + #[pallet::error] + pub enum Error { + CustomError(u128), + } +} + +fn main() { +} From cc062061fe192ab8d3aa2ebdaadb714d6fb65173 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 3 Feb 2022 16:33:16 -0800 Subject: [PATCH 53/75] Pull out the pallet error size check macro --- .../procedural/src/construct_runtime/mod.rs | 34 ++------- frame/support/src/lib.rs | 21 ++++++ .../pallet_error_too_large.stderr | 73 ------------------- .../tests/pallet_ui/error_size_too_large.rs | 16 ---- 4 files changed, 27 insertions(+), 117 deletions(-) delete mode 100644 frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.stderr delete mode 100644 frame/support/test/tests/pallet_ui/error_size_too_large.rs diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index a16834c3984dd..0726e40e1e14d 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -144,7 +144,6 @@ mod expand; mod parse; -use crate::COUNTER; use frame_support_procedural_tools::{ generate_crate_access, generate_crate_access_2018, generate_hidden_includes, }; @@ -154,7 +153,7 @@ use parse::{ }; use proc_macro::TokenStream; use proc_macro2::TokenStream as TokenStream2; -use quote::{format_ident, quote}; +use quote::quote; use syn::{Ident, Result}; /// The fixed name of the system pallet. @@ -482,44 +481,23 @@ fn decl_static_assertions( scrate: &TokenStream2, ) -> TokenStream2 { let error_encoded_size_check = pallet_decls.iter().map(|decl| { - let count = COUNTER.with(|counter| counter.borrow_mut().inc()); let path = &decl.path; - // This weirdness is required because declarative macros gets hoisted up to the crate root, - // and thus doesn't appear in the same module namespace as the tt_call macro. We use a - // re-export hack to make the macro appear in the same module namespace. - let assert_macro_name = format_ident!("__assert_error_encoded_size_{}", count); let assert_message = format!( "The maximum encoded size of the error type in the `{}` pallet exceeds \ `MAX_PALLET_ERROR_ENCODED_SIZE`", decl.name, ); - let macro_alias = format_ident!("assert_error_encoded_size_{}", count); quote! { #scrate::tt_call! { macro = [{ #path::tt_error_token }] frame_support = [{ #scrate }] - ~~> #macro_alias - } - - #[macro_export] - #[doc(hidden)] - macro_rules! #assert_macro_name { - { - error = [{ $error:ident }] - } => { - const _: () = assert!( - < - #path::$error<#runtime> as #scrate::traits::PalletError - >::MAX_ENCODED_SIZE <= #scrate::MAX_PALLET_ERROR_ENCODED_SIZE, - #assert_message - ); - }; - {} => {}; + ~~> #scrate::assert_error_encoded_size! { + path = [{ #path }] + runtime = [{ #runtime }] + assert_message = [{ #assert_message }] + } } - - #[doc(hidden)] - pub use #assert_macro_name as #macro_alias; } }); diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index f193255371f1c..b83fab306982c 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -827,6 +827,27 @@ macro_rules! assert_ok { }; } +/// Assert that the maximum encoding size does not exceed the value defined in +/// [`MAX_PALLET_ERROR_ENCODED_SIZE`] during compilation. +/// +/// This macro is intended to be used in conjuction with `tt_call!`. +#[macro_export] +macro_rules! assert_error_encoded_size { + { + path = [{ $path:path }] + runtime = [{ $runtime:ident }] + assert_message = [{ $assert_message:literal }] + error = [{ $error:ident }] + } => { + const _: () = assert!( + < + <$path>::$error<$runtime> as $crate::traits::PalletError + >::MAX_ENCODED_SIZE <= $crate::MAX_PALLET_ERROR_ENCODED_SIZE, + $assert_message + ); + } +} + #[cfg(feature = "std")] #[doc(hidden)] pub use serde::{Deserialize, Serialize}; diff --git a/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.stderr b/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.stderr deleted file mode 100644 index c97764dfbe97d..0000000000000 --- a/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.stderr +++ /dev/null @@ -1,73 +0,0 @@ -error[E0433]: failed to resolve: use of undeclared crate or module `system` - --> tests/construct_runtime_ui/pallet_error_too_large.rs:53:11 - | -53 | System: system::{Pallet, Call, Storage, Config, Event}, - | ^^^^^^ use of undeclared crate or module `system` - -error[E0433]: failed to resolve: use of undeclared crate or module `system` - --> tests/construct_runtime_ui/pallet_error_too_large.rs:47:1 - | -47 | / construct_runtime! { -48 | | pub enum Runtime where -49 | | Block = Block, -50 | | NodeBlock = Block, -... | -55 | | } -56 | | } - | |_^ not found in `system` - | - = note: this error originates in the macro `construct_runtime` (in Nightly builds, run with -Z macro-backtrace for more info) -help: consider importing this enum - | -1 | use frame_system::RawOrigin; - | - -error[E0433]: failed to resolve: use of undeclared crate or module `system` - --> tests/construct_runtime_ui/pallet_error_too_large.rs:47:1 - | -47 | / construct_runtime! { -48 | | pub enum Runtime where -49 | | Block = Block, -50 | | NodeBlock = Block, -... | -55 | | } -56 | | } - | |_^ not found in `system` - | - = note: this error originates in the macro `construct_runtime` (in Nightly builds, run with -Z macro-backtrace for more info) -help: consider importing one of these items - | -1 | use crate::pallet::Pallet; - | -1 | use frame_support_test::Pallet; - | -1 | use frame_system::Pallet; - | -1 | use test_pallet::Pallet; - | - -error[E0277]: the trait bound `Runtime: frame_system::Config` is not satisfied - --> tests/construct_runtime_ui/pallet_error_too_large.rs:45:6 - | -45 | impl pallet::Config for Runtime {} - | ^^^^^^^^^^^^^^ the trait `frame_system::Config` is not implemented for `Runtime` - | -note: required by a bound in `pallet::Config` - --> tests/construct_runtime_ui/pallet_error_too_large.rs:8:20 - | -8 | pub trait Config: frame_system::Config {} - | ^^^^^^^^^^^^^^^^^^^^ required by this bound in `pallet::Config` - -error[E0080]: evaluation of constant value failed - --> tests/construct_runtime_ui/pallet_error_too_large.rs:47:1 - | -47 | / construct_runtime! { -48 | | pub enum Runtime where -49 | | Block = Block, -50 | | NodeBlock = Block, -... | -55 | | } -56 | | } - | |_^ attempt to compute `0_usize - 1_usize`, which would overflow - | - = note: this error originates in the macro `self::sp_api_hidden_includes_construct_runtime::hidden_include::const_assert` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/frame/support/test/tests/pallet_ui/error_size_too_large.rs b/frame/support/test/tests/pallet_ui/error_size_too_large.rs deleted file mode 100644 index a8b9cdb672384..0000000000000 --- a/frame/support/test/tests/pallet_ui/error_size_too_large.rs +++ /dev/null @@ -1,16 +0,0 @@ -#[frame_support::pallet] -mod pallet { - #[pallet::config] - pub trait Config: frame_system::Config {} - - #[pallet::pallet] - pub struct Pallet(core::marker::PhantomData); - - #[pallet::error] - pub enum Error { - CustomError(u128), - } -} - -fn main() { -} From e4c3f474611837fd86d7b88416adcd97d41da39b Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 3 Feb 2022 17:32:24 -0800 Subject: [PATCH 54/75] Fix UI test for const assertion --- frame/support/src/lib.rs | 13 +++++--- .../pallet_error_too_large.rs | 31 +++++++++++++++++-- .../pallet_error_too_large.stderr | 13 ++++++++ frame/support/test/tests/pallet.rs | 1 - 4 files changed, 51 insertions(+), 7 deletions(-) create mode 100644 frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.stderr diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index e6be408abd0a1..57d2c607dc2b4 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -853,22 +853,27 @@ macro_rules! assert_ok { /// Assert that the maximum encoding size does not exceed the value defined in /// [`MAX_PALLET_ERROR_ENCODED_SIZE`] during compilation. /// -/// This macro is intended to be used in conjuction with `tt_call!`. +/// This macro is intended to be used in conjunction with `tt_call!`. #[macro_export] macro_rules! assert_error_encoded_size { { - path = [{ $path:path }] + path = [{ $($path:ident)::+ }] runtime = [{ $runtime:ident }] assert_message = [{ $assert_message:literal }] error = [{ $error:ident }] } => { const _: () = assert!( < - <$path>::$error<$runtime> as $crate::traits::PalletError + $($path::)+$error<$runtime> as $crate::traits::PalletError >::MAX_ENCODED_SIZE <= $crate::MAX_PALLET_ERROR_ENCODED_SIZE, $assert_message ); - } + }; + { + path = [{ $($path:ident)::+ }] + runtime = [{ $runtime:ident }] + assert_message = [{ $assert_message:literal }] + } => {}; } #[cfg(feature = "std")] diff --git a/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.rs b/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.rs index ec661fe8e50f3..827d8a58af733 100644 --- a/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.rs +++ b/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.rs @@ -37,20 +37,47 @@ pub enum Nested4 { } pub type Signature = sr25519::Signature; -pub type BlockNumber = u64; +pub type BlockNumber = u32; pub type Header = generic::Header; pub type Block = generic::Block; pub type UncheckedExtrinsic = generic::UncheckedExtrinsic; impl pallet::Config for Runtime {} +impl frame_system::Config for Runtime { + type BaseCallFilter = frame_support::traits::Everything; + type Origin = Origin; + type Index = u64; + type BlockNumber = u32; + type Call = Call; + type Hash = sp_runtime::testing::H256; + type Hashing = sp_runtime::traits::BlakeTwo256; + type AccountId = u64; + type Lookup = sp_runtime::traits::IdentityLookup; + type Header = Header; + type Event = Event; + type BlockHashCount = frame_support::traits::ConstU32<250>; + type BlockWeights = (); + type BlockLength = (); + type DbWeight = (); + type Version = (); + type PalletInfo = PalletInfo; + type AccountData = (); + type OnNewAccount = (); + type OnKilledAccount = (); + type SystemWeightInfo = (); + type SS58Prefix = (); + type OnSetCode = (); + type MaxConsumers = frame_support::traits::ConstU32<16>; +} + construct_runtime! { pub enum Runtime where Block = Block, NodeBlock = Block, UncheckedExtrinsic = UncheckedExtrinsic { - System: system::{Pallet, Call, Storage, Config, Event}, + System: frame_system::{Pallet, Call, Storage, Config, Event}, Pallet: pallet::{Pallet}, } } diff --git a/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.stderr b/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.stderr new file mode 100644 index 0000000000000..d95097b586d00 --- /dev/null +++ b/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.stderr @@ -0,0 +1,13 @@ +error[E0080]: evaluation of constant value failed + --> tests/construct_runtime_ui/pallet_error_too_large.rs:74:1 + | +74 | / construct_runtime! { +75 | | pub enum Runtime where +76 | | Block = Block, +77 | | NodeBlock = Block, +... | +82 | | } +83 | | } + | |_^ the evaluated program panicked at 'The maximum encoded size of the error type in the `Pallet` pallet exceeds `MAX_PALLET_ERROR_ENCODED_SIZE`', $DIR/tests/construct_runtime_ui/pallet_error_too_large.rs:74:1 + | + = note: this error originates in the macro `$crate::panic::panic_2021` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index 681fc172f937b..a63602c30e6cf 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -646,7 +646,6 @@ fn call_expand() { #[test] fn error_expand() { - use codec::Decode; assert_eq!( format!("{:?}", pallet::Error::::InsufficientProposersBalance), String::from("InsufficientProposersBalance"), From fa0e1554af714e97de8a4c4447f4fc2fc13337de Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 3 Feb 2022 17:46:03 -0800 Subject: [PATCH 55/75] cargo fmt --- frame/support/test/tests/construct_runtime.rs | 66 +++++++++++++++---- frame/support/test/tests/pallet.rs | 1 - 2 files changed, 55 insertions(+), 12 deletions(-) diff --git a/frame/support/test/tests/construct_runtime.rs b/frame/support/test/tests/construct_runtime.rs index 2a0948df6f67d..804deb08919a4 100644 --- a/frame/support/test/tests/construct_runtime.rs +++ b/frame/support/test/tests/construct_runtime.rs @@ -275,47 +275,91 @@ pub type UncheckedExtrinsic = generic::UncheckedExtrinsic::Root.into()), - Err(DispatchError::Module(ModuleError { index: 31, error: [0; 4], message: Some("Something") })), + Err(DispatchError::Module(ModuleError { + index: 31, + error: [0; 4], + message: Some("Something") + })), ); assert_eq!( Module2::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { index: 32, error: [0; 4], message: Some("Something") })), + Err(DispatchError::Module(ModuleError { + index: 32, + error: [0; 4], + message: Some("Something") + })), ); assert_eq!( Module1_2::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { index: 33, error: [0; 4], message: Some("Something") })), + Err(DispatchError::Module(ModuleError { + index: 33, + error: [0; 4], + message: Some("Something") + })), ); assert_eq!( NestedModule3::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { index: 34, error: [0; 4], message: Some("Something") })), + Err(DispatchError::Module(ModuleError { + index: 34, + error: [0; 4], + message: Some("Something") + })), ); assert_eq!( Module1_3::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { index: 6, error: [0; 4], message: Some("Something") })), + Err(DispatchError::Module(ModuleError { + index: 6, + error: [0; 4], + message: Some("Something") + })), ); assert_eq!( Module1_4::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { index: 3, error: [0; 4], message: Some("Something") })), + Err(DispatchError::Module(ModuleError { + index: 3, + error: [0; 4], + message: Some("Something") + })), ); assert_eq!( Module1_5::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { index: 4, error: [0; 4], message: Some("Something") })), + Err(DispatchError::Module(ModuleError { + index: 4, + error: [0; 4], + message: Some("Something") + })), ); assert_eq!( Module1_6::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { index: 1, error: [0; 4], message: Some("Something") })), + Err(DispatchError::Module(ModuleError { + index: 1, + error: [0; 4], + message: Some("Something") + })), ); assert_eq!( Module1_7::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { index: 2, error: [0; 4], message: Some("Something") })), + Err(DispatchError::Module(ModuleError { + index: 2, + error: [0; 4], + message: Some("Something") + })), ); assert_eq!( Module1_8::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { index: 12, error: [0; 4], message: Some("Something") })), + Err(DispatchError::Module(ModuleError { + index: 12, + error: [0; 4], + message: Some("Something") + })), ); assert_eq!( Module1_9::fail(system::Origin::::Root.into()), - Err(DispatchError::Module(ModuleError { index: 13, error: [0; 4], message: Some("Something") })), + Err(DispatchError::Module(ModuleError { + index: 13, + error: [0; 4], + message: Some("Something") + })), ); } diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index a63602c30e6cf..429c4f9bbf5d6 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -656,7 +656,6 @@ fn error_expand() { ); assert_eq!( DispatchError::from(pallet::Error::::InsufficientProposersBalance), - DispatchError::Module(ModuleError { index: 1, error: [0, 0, 0, 0], From 29e3df1da8a1975871a9c3dd5eb9f411bcee1b63 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 3 Feb 2022 17:47:09 -0800 Subject: [PATCH 56/75] Apply clippy suggestion --- frame/support/procedural/src/pallet_error.rs | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/frame/support/procedural/src/pallet_error.rs b/frame/support/procedural/src/pallet_error.rs index fc22aafc74a7d..9c8c93754473a 100644 --- a/frame/support/procedural/src/pallet_error.rs +++ b/frame/support/procedural/src/pallet_error.rs @@ -16,7 +16,6 @@ // limitations under the License. use frame_support_procedural_tools::generate_crate_access_2018; -use std::convert::identity; // Derive `PalletError` pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenStream { @@ -89,7 +88,7 @@ pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenS .collect::>>, syn::Error>>(); let field_tys = match field_tys { - Ok(tys) => tys.into_iter().filter_map(identity).collect::>(), + Ok(tys) => tys.into_iter().flatten().collect::>(), Err(e) => return e.to_compile_error().into(), }; From a8b621479c32480405c4f25b5f625ceed7531c19 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 3 Feb 2022 17:48:14 -0800 Subject: [PATCH 57/75] Fix doc comment --- frame/support/src/traits/error.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index 600dd8b0c21ad..f059d47085d1f 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -27,7 +27,7 @@ use sp_std::marker::PhantomData; /// [`frame_support::MAX_PALLET_ERROR_ENCODED_SIZE`]. If the pallet error type exceeds this size /// limit, a static assertion during compilation will fail. The compilation error will be in the /// format of `error[E0080]: evaluation of constant value failed` due to the usage of -/// [`static_assertions::const_assert`]. +/// const assertions. pub trait PalletError: Encode + Decode { /// The maximum encoded size for the implementing type. /// From cca5ed66846b11bac3a2d488ae46c7198821a8a6 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Fri, 4 Feb 2022 05:28:01 -0800 Subject: [PATCH 58/75] Docs for create_tt_return_macro --- frame/support/procedural/src/tt_macro.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/frame/support/procedural/src/tt_macro.rs b/frame/support/procedural/src/tt_macro.rs index aac869e5db6aa..7780aafbcabbb 100644 --- a/frame/support/procedural/src/tt_macro.rs +++ b/frame/support/procedural/src/tt_macro.rs @@ -47,6 +47,10 @@ impl syn::parse::Parse for CreateTtReturnMacroDef { } } +/// A proc macro that accepts a name and any number of key-value pairs, to be used to create a +/// declarative macro that follows tt-call conventions and simply calls [`tt_call::tt_return`], +/// accepting an optional `frame-support` argument and returning the key-value pairs that were +/// supplied to the proc macro. pub fn create_tt_return_macro(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let CreateTtReturnMacroDef { name, args } = syn::parse_macro_input!(input as CreateTtReturnMacroDef); From 52a0a7358356167dbc85524b555b6f7e5c6ca1f3 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Mon, 7 Feb 2022 14:18:43 -0800 Subject: [PATCH 59/75] Ensure TryInto is imported in earlier Rust editions --- frame/support/procedural/src/pallet/expand/error.rs | 3 +++ 1 file changed, 3 insertions(+) diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index 368b42b1b5936..4c75f3d4a66a0 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -150,6 +150,9 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { #config_where_clause { fn from(err: #error_ident<#type_use_gen>) -> Self { + // Ensure that we can still use `try_into` in earlier editions + #[cfg(not(feature = "prelude_2021"))] + use core::convert::TryInto; use #frame_support::codec::Encode; let index = < ::PalletInfo From ece305eeb55c8181c2bd0b3cc945064e430503ec Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 10 Feb 2022 22:12:09 -0800 Subject: [PATCH 60/75] Apply suggestions from code review MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- frame/support/procedural/src/pallet/expand/error.rs | 7 ++----- frame/support/procedural/src/pallet_error.rs | 1 + frame/support/procedural/src/tt_macro.rs | 3 ++- frame/support/src/error.rs | 2 +- frame/support/src/traits/error.rs | 3 ++- 5 files changed, 8 insertions(+), 8 deletions(-) diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index 4c75f3d4a66a0..083fdd1bf4173 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -68,7 +68,7 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { ); let as_str_matches = error.variants.iter().map(|(variant, field_ty, _)| { - let variant_str = format!("{}", variant); + let variant_str = variant.to_string(); match field_ty { Some(VariantField { is_named: true }) => { quote::quote_spanned!(error.attr_span => Self::#variant { .. } => #variant_str,) @@ -150,9 +150,6 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { #config_where_clause { fn from(err: #error_ident<#type_use_gen>) -> Self { - // Ensure that we can still use `try_into` in earlier editions - #[cfg(not(feature = "prelude_2021"))] - use core::convert::TryInto; use #frame_support::codec::Encode; let index = < ::PalletInfo @@ -164,7 +161,7 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { #frame_support::sp_runtime::DispatchError::Module(#frame_support::sp_runtime::ModuleError { index, - error: encoded.try_into().expect("encoded error is resized to be equal to 4 bytes; qed"), + error: core::convert::TryInto::try_into(encoded).expect("encoded error is resized to be equal to the maximum encoded error size; qed"), message: Some(err.as_str()), }) } diff --git a/frame/support/procedural/src/pallet_error.rs b/frame/support/procedural/src/pallet_error.rs index 9c8c93754473a..d12a7c5a65da7 100644 --- a/frame/support/procedural/src/pallet_error.rs +++ b/frame/support/procedural/src/pallet_error.rs @@ -92,6 +92,7 @@ pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenS Err(e) => return e.to_compile_error().into(), }; + // We start with `1`, because the discriminant of an enum is stored as u8 if field_tys.is_empty() { quote::quote!(1) } else { diff --git a/frame/support/procedural/src/tt_macro.rs b/frame/support/procedural/src/tt_macro.rs index 7780aafbcabbb..96bf8537ca1d2 100644 --- a/frame/support/procedural/src/tt_macro.rs +++ b/frame/support/procedural/src/tt_macro.rs @@ -1,6 +1,6 @@ // This file is part of Substrate. -// Copyright (C) 2021 Parity Technologies (UK) Ltd. +// Copyright (C) 2022 Parity Technologies (UK) Ltd. // SPDX-License-Identifier: Apache-2.0 // Licensed under the Apache License, Version 2.0 (the "License"); @@ -16,6 +16,7 @@ // limitations under the License. //! Implementation of the `create_tt_return_macro` macro + use crate::COUNTER; use frame_support_procedural_tools::generate_crate_access_2018; use proc_macro2::{Ident, TokenStream}; diff --git a/frame/support/src/error.rs b/frame/support/src/error.rs index 30341b1e7dcae..11a10e4ec5b32 100644 --- a/frame/support/src/error.rs +++ b/frame/support/src/error.rs @@ -152,7 +152,7 @@ macro_rules! decl_error { $crate::sp_runtime::DispatchError::Module($crate::sp_runtime::ModuleError { index, - error: error.try_into().expect("error has been resized to be 4 bytes; qed"), + error: core::convert::TryInto::try_into(error).expect("encoded error is resized to be equal to the maximum encoded error size; qed"), message: Some(err.as_str()), }) } diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index f059d47085d1f..75c0a026c3979 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -1,6 +1,6 @@ // This file is part of Substrate. -// Copyright (C) 2021 Parity Technologies (UK) Ltd. +// Copyright (C) 2022 Parity Technologies (UK) Ltd. // SPDX-License-Identifier: Apache-2.0 // Licensed under the Apache License, Version 2.0 (the "License"); @@ -23,6 +23,7 @@ use sp_std::marker::PhantomData; /// the `#[pallet::error]` enum type. /// /// ## Notes +/// /// The pallet error enum has a maximum encoded size as defined by /// [`frame_support::MAX_PALLET_ERROR_ENCODED_SIZE`]. If the pallet error type exceeds this size /// limit, a static assertion during compilation will fail. The compilation error will be in the From 29a131ac1499aa3a4d9f9610985ed3ac3499e192 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Fri, 11 Feb 2022 17:06:46 -0800 Subject: [PATCH 61/75] Fix up comments and names --- .../procedural/src/construct_runtime/mod.rs | 2 +- .../procedural/src/pallet/expand/error.rs | 2 +- frame/support/procedural/src/tt_macro.rs | 22 +++++++++++++++++ frame/support/src/error.rs | 2 +- frame/support/src/lib.rs | 24 ++++++++++++------- frame/support/src/traits/error.rs | 4 ++-- .../pallet_error_too_large.stderr | 2 +- primitives/runtime/src/lib.rs | 7 +++--- 8 files changed, 48 insertions(+), 17 deletions(-) diff --git a/frame/support/procedural/src/construct_runtime/mod.rs b/frame/support/procedural/src/construct_runtime/mod.rs index 0726e40e1e14d..2a86869382c93 100644 --- a/frame/support/procedural/src/construct_runtime/mod.rs +++ b/frame/support/procedural/src/construct_runtime/mod.rs @@ -484,7 +484,7 @@ fn decl_static_assertions( let path = &decl.path; let assert_message = format!( "The maximum encoded size of the error type in the `{}` pallet exceeds \ - `MAX_PALLET_ERROR_ENCODED_SIZE`", + `MAX_MODULE_ERROR_ENCODED_SIZE`", decl.name, ); diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index 083fdd1bf4173..80e0100ce0688 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -157,7 +157,7 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { >::index::>() .expect("Every active module has an index in the runtime; qed") as u8; let mut encoded = err.encode(); - encoded.resize(#frame_support::MAX_PALLET_ERROR_ENCODED_SIZE, 0); + encoded.resize(#frame_support::MAX_MODULE_ERROR_ENCODED_SIZE, 0); #frame_support::sp_runtime::DispatchError::Module(#frame_support::sp_runtime::ModuleError { index, diff --git a/frame/support/procedural/src/tt_macro.rs b/frame/support/procedural/src/tt_macro.rs index 96bf8537ca1d2..c9c8ae6457924 100644 --- a/frame/support/procedural/src/tt_macro.rs +++ b/frame/support/procedural/src/tt_macro.rs @@ -52,6 +52,28 @@ impl syn::parse::Parse for CreateTtReturnMacroDef { /// declarative macro that follows tt-call conventions and simply calls [`tt_call::tt_return`], /// accepting an optional `frame-support` argument and returning the key-value pairs that were /// supplied to the proc macro. +/// +/// # Example +/// ```rust,nocompile +/// __create_tt_macro! { +/// my_tt_macro, +/// foo = [{ bar }] +/// } +/// +/// // Creates the following declarative macro: +/// +/// macro_rules! my_tt_macro { +/// { +/// $caller:tt +/// $(frame_support = [{ $($frame_support:ident)::* }])? +/// } => { +/// frame_support::tt_return! { +/// $caller +/// foo = [{ bar }] +/// } +/// } +/// } +/// ``` pub fn create_tt_return_macro(input: proc_macro::TokenStream) -> proc_macro::TokenStream { let CreateTtReturnMacroDef { name, args } = syn::parse_macro_input!(input as CreateTtReturnMacroDef); diff --git a/frame/support/src/error.rs b/frame/support/src/error.rs index 11a10e4ec5b32..764376a4e1dc2 100644 --- a/frame/support/src/error.rs +++ b/frame/support/src/error.rs @@ -148,7 +148,7 @@ macro_rules! decl_error { ::index::<$module<$generic $(, $inst_generic)?>>() .expect("Every active module has an index in the runtime; qed") as u8; let mut error = err.encode(); - error.resize($crate::MAX_PALLET_ERROR_ENCODED_SIZE, 0); + error.resize($crate::MAX_MODULE_ERROR_ENCODED_SIZE, 0); $crate::sp_runtime::DispatchError::Module($crate::sp_runtime::ModuleError { index, diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 57d2c607dc2b4..374e26cd2a673 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -94,7 +94,7 @@ pub use self::{ }, }; pub use sp_runtime::{ - self, print, traits::Printable, ConsensusEngineId, MAX_PALLET_ERROR_ENCODED_SIZE, + self, print, traits::Printable, ConsensusEngineId, MAX_MODULE_ERROR_ENCODED_SIZE, }; use codec::{Decode, Encode}; @@ -851,7 +851,7 @@ macro_rules! assert_ok { } /// Assert that the maximum encoding size does not exceed the value defined in -/// [`MAX_PALLET_ERROR_ENCODED_SIZE`] during compilation. +/// [`MAX_MODULE_ERROR_ENCODED_SIZE`] during compilation. /// /// This macro is intended to be used in conjunction with `tt_call!`. #[macro_export] @@ -865,7 +865,7 @@ macro_rules! assert_error_encoded_size { const _: () = assert!( < $($path::)+$error<$runtime> as $crate::traits::PalletError - >::MAX_ENCODED_SIZE <= $crate::MAX_PALLET_ERROR_ENCODED_SIZE, + >::MAX_ENCODED_SIZE <= $crate::MAX_MODULE_ERROR_ENCODED_SIZE, $assert_message ); }; @@ -1390,7 +1390,7 @@ pub mod pallet_prelude { TransactionTag, TransactionValidity, TransactionValidityError, UnknownTransaction, ValidTransaction, }, - MAX_PALLET_ERROR_ENCODED_SIZE, + MAX_MODULE_ERROR_ENCODED_SIZE, }; pub use sp_std::marker::PhantomData; } @@ -1669,15 +1669,23 @@ pub mod pallet_prelude { /// /// $some_optional_doc /// $SomeFieldLessVariant, /// /// $some_more_optional_doc -/// $SomeSingleFieldVariant(FieldType), +/// $SomeVariantWithOneField(FieldType), /// ... /// } /// ``` -/// I.e. a regular rust enum named `Error`, with generic `T` and fieldless or single-field +/// I.e. a regular rust enum named `Error`, with generic `T` and fieldless or multiple-field /// variants. -/// Any field in the enum variants must implement `scale_info::TypeInfo` in order to be +/// +/// Any field type in the enum variants must implement `scale_info::TypeInfo` in order to be /// properly used in the metadata, and its encoded size should be as small as possible, -/// preferably 1 byte in size. +/// preferably 1 byte in size in order to reduce storage size. The error enum itself has an +/// absolute maximum encoded size specified by [`MAX_MODULE_ERROR_ENCODED_SIZE`]. +/// +/// Field types in enum variants must also implement `PalletError`, otherwise the pallet will fail +/// to compile. Rust primitive types have already implemented the `PalletError` trait along with +/// some commonly used stdlib types such as `Option` and `PhantomData`, and hence in most use cases, +/// a manual implementation is not necessary and is discouraged. +/// /// The generic `T` mustn't bound anything and where clause is not allowed. But bounds and /// where clause shouldn't be needed for any usecase. /// diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index 75c0a026c3979..15a07be48eca9 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -25,7 +25,7 @@ use sp_std::marker::PhantomData; /// ## Notes /// /// The pallet error enum has a maximum encoded size as defined by -/// [`frame_support::MAX_PALLET_ERROR_ENCODED_SIZE`]. If the pallet error type exceeds this size +/// [`frame_support::MAX_MODULE_ERROR_ENCODED_SIZE`]. If the pallet error type exceeds this size /// limit, a static assertion during compilation will fail. The compilation error will be in the /// format of `error[E0080]: evaluation of constant value failed` due to the usage of /// const assertions. @@ -33,7 +33,7 @@ pub trait PalletError: Encode + Decode { /// The maximum encoded size for the implementing type. /// /// This will be used to check whether the pallet error type is less than or equal to - /// [`frame_support::MAX_PALLET_ERROR_ENCODED_SIZE`], and if it is, a compilation error will be + /// [`frame_support::MAX_MODULE_ERROR_ENCODED_SIZE`], and if it is, a compilation error will be /// thrown. const MAX_ENCODED_SIZE: usize; } diff --git a/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.stderr b/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.stderr index d95097b586d00..161873866b6f3 100644 --- a/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.stderr +++ b/frame/support/test/tests/construct_runtime_ui/pallet_error_too_large.stderr @@ -8,6 +8,6 @@ error[E0080]: evaluation of constant value failed ... | 82 | | } 83 | | } - | |_^ the evaluated program panicked at 'The maximum encoded size of the error type in the `Pallet` pallet exceeds `MAX_PALLET_ERROR_ENCODED_SIZE`', $DIR/tests/construct_runtime_ui/pallet_error_too_large.rs:74:1 + | |_^ the evaluated program panicked at 'The maximum encoded size of the error type in the `Pallet` pallet exceeds `MAX_MODULE_ERROR_ENCODED_SIZE`', $DIR/tests/construct_runtime_ui/pallet_error_too_large.rs:74:1 | = note: this error originates in the macro `$crate::panic::panic_2021` (in Nightly builds, run with -Z macro-backtrace for more info) diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 7d2673c18342e..5e242b7bfe10f 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -97,8 +97,9 @@ pub use sp_arithmetic::{ pub use either::Either; -/// The maximum depth for a nested pallet error enum. -pub const MAX_PALLET_ERROR_ENCODED_SIZE: usize = 4; +/// The number of bytes of the module-specific `error` field defined in `ModuleError`. +/// In FRAME, this is the maximum encoded size of a pallet error type. +pub const MAX_MODULE_ERROR_ENCODED_SIZE: usize = 4; /// An abstraction over justification for a block's validity under a consensus algorithm. /// @@ -471,7 +472,7 @@ pub struct ModuleError { /// Module index, matching the metadata module index. pub index: u8, /// Module specific error value. - pub error: [u8; MAX_PALLET_ERROR_ENCODED_SIZE], + pub error: [u8; MAX_MODULE_ERROR_ENCODED_SIZE], /// Optional error message. #[codec(skip)] #[cfg_attr(feature = "std", serde(skip_deserializing))] From 9cdf1aa24d424e8be57a19d7e4fb8681b2664f11 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Fri, 11 Feb 2022 20:59:16 -0800 Subject: [PATCH 62/75] Implement PalletError for Never --- frame/support/procedural/src/pallet_error.rs | 20 -------------------- frame/support/src/lib.rs | 2 +- frame/support/src/traits/error.rs | 2 +- 3 files changed, 2 insertions(+), 22 deletions(-) diff --git a/frame/support/procedural/src/pallet_error.rs b/frame/support/procedural/src/pallet_error.rs index d12a7c5a65da7..d5e8aa7b0d207 100644 --- a/frame/support/procedural/src/pallet_error.rs +++ b/frame/support/procedural/src/pallet_error.rs @@ -58,26 +58,6 @@ pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenS .iter() .map(|variant| { match &variant.fields { - syn::Fields::Unnamed(f) if f.unnamed.len() == 2 => { - let first = &f.unnamed.first().unwrap().ty; - let second = &f.unnamed.last().unwrap().ty; - - match (first, second) { - // Check whether we have (PhantomData, Never), if so we skip it. - (syn::Type::Path(p1), syn::Type::Path(p2)) - if p1 - .path - .segments - .last() - .map_or(false, |seg| seg.ident == "PhantomData") && - p2.path - .segments - .last() - .map_or(false, |seg| seg.ident == "Never") => - Ok(None), - _ => Ok(Some(vec![first, second])), - } - }, syn::Fields::Named(f) => Ok(Some(f.named.iter().map(|field| &field.ty).collect::>())), syn::Fields::Unnamed(f) => diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 374e26cd2a673..4bea12d3cb103 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -105,7 +105,7 @@ use sp_runtime::TypeId; pub const LOG_TARGET: &'static str = "runtime::frame-support"; /// A type that cannot be instantiated. -#[derive(Debug, PartialEq, Eq, Clone, TypeInfo)] +#[derive(Encode, Decode, Debug, PartialEq, Eq, Clone, TypeInfo)] pub enum Never {} /// A pallet identifier. These are per pallet and should be stored in a registry somewhere. diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index 15a07be48eca9..9cfbedc4598d5 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -48,7 +48,7 @@ macro_rules! impl_for_types { }; } -impl_for_types!(size: 0, ()); +impl_for_types!(size: 0, (), crate::Never); impl_for_types!(size: 1, u8, i8, bool); impl_for_types!(size: 2, u16, i16); impl_for_types!(size: 4, u32, i32); From 5070335b7a9f7cec807257ec06905f7d1bf0448a Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Fri, 11 Feb 2022 20:59:51 -0800 Subject: [PATCH 63/75] cargo fmt --- frame/support/procedural/src/pallet_error.rs | 14 ++++++-------- frame/support/src/lib.rs | 8 ++++---- 2 files changed, 10 insertions(+), 12 deletions(-) diff --git a/frame/support/procedural/src/pallet_error.rs b/frame/support/procedural/src/pallet_error.rs index d5e8aa7b0d207..338e9f8c7ff8d 100644 --- a/frame/support/procedural/src/pallet_error.rs +++ b/frame/support/procedural/src/pallet_error.rs @@ -56,14 +56,12 @@ pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenS syn::Data::Enum(syn::DataEnum { variants, .. }) => { let field_tys = variants .iter() - .map(|variant| { - match &variant.fields { - syn::Fields::Named(f) => - Ok(Some(f.named.iter().map(|field| &field.ty).collect::>())), - syn::Fields::Unnamed(f) => - Ok(Some(f.unnamed.iter().map(|field| &field.ty).collect::>())), - syn::Fields::Unit => Ok(None), - } + .map(|variant| match &variant.fields { + syn::Fields::Named(f) => + Ok(Some(f.named.iter().map(|field| &field.ty).collect::>())), + syn::Fields::Unnamed(f) => + Ok(Some(f.unnamed.iter().map(|field| &field.ty).collect::>())), + syn::Fields::Unit => Ok(None), }) .collect::>>, syn::Error>>(); diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 4bea12d3cb103..41bab8d9a3cbc 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1681,10 +1681,10 @@ pub mod pallet_prelude { /// preferably 1 byte in size in order to reduce storage size. The error enum itself has an /// absolute maximum encoded size specified by [`MAX_MODULE_ERROR_ENCODED_SIZE`]. /// -/// Field types in enum variants must also implement `PalletError`, otherwise the pallet will fail -/// to compile. Rust primitive types have already implemented the `PalletError` trait along with -/// some commonly used stdlib types such as `Option` and `PhantomData`, and hence in most use cases, -/// a manual implementation is not necessary and is discouraged. +/// Field types in enum variants must also implement `PalletError`, otherwise the pallet will +/// fail to compile. Rust primitive types have already implemented the `PalletError` trait +/// along with some commonly used stdlib types such as `Option` and `PhantomData`, and hence in +/// most use cases, a manual implementation is not necessary and is discouraged. /// /// The generic `T` mustn't bound anything and where clause is not allowed. But bounds and /// where clause shouldn't be needed for any usecase. From ac1f19892b948de6acb97b50fcb48873dea34e3a Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Fri, 11 Feb 2022 23:28:06 -0800 Subject: [PATCH 64/75] Don't compile example code --- frame/support/procedural/src/tt_macro.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/procedural/src/tt_macro.rs b/frame/support/procedural/src/tt_macro.rs index c9c8ae6457924..0a270a7173cfc 100644 --- a/frame/support/procedural/src/tt_macro.rs +++ b/frame/support/procedural/src/tt_macro.rs @@ -54,7 +54,7 @@ impl syn::parse::Parse for CreateTtReturnMacroDef { /// supplied to the proc macro. /// /// # Example -/// ```rust,nocompile +/// ```ignore /// __create_tt_macro! { /// my_tt_macro, /// foo = [{ bar }] From 811e2f2edd11167d3fa93b7d05c84454046ea448 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Tue, 15 Feb 2022 23:50:08 -0800 Subject: [PATCH 65/75] Bump API version for block builder --- primitives/block-builder/src/lib.rs | 10 ++- primitives/runtime/src/legacy.rs | 20 ++++++ primitives/runtime/src/legacy/before_v6.rs | 79 ++++++++++++++++++++++ primitives/runtime/src/lib.rs | 1 + 4 files changed, 108 insertions(+), 2 deletions(-) create mode 100644 primitives/runtime/src/legacy.rs create mode 100644 primitives/runtime/src/legacy/before_v6.rs diff --git a/primitives/block-builder/src/lib.rs b/primitives/block-builder/src/lib.rs index 229f115c6667f..e1557b910a41f 100644 --- a/primitives/block-builder/src/lib.rs +++ b/primitives/block-builder/src/lib.rs @@ -20,11 +20,14 @@ #![cfg_attr(not(feature = "std"), no_std)] use sp_inherents::{CheckInherentsResult, InherentData}; -use sp_runtime::{traits::Block as BlockT, ApplyExtrinsicResult}; +use sp_runtime::{ + legacy::before_v6::ApplyExtrinsicResult as ApplyExtrinsicResultBeforeV6, + traits::Block as BlockT, ApplyExtrinsicResult, +}; sp_api::decl_runtime_apis! { /// The `BlockBuilder` api trait that provides the required functionality for building a block. - #[api_version(5)] + #[api_version(6)] pub trait BlockBuilder { /// Apply the given extrinsic. /// @@ -32,6 +35,9 @@ sp_api::decl_runtime_apis! { /// this block or not. fn apply_extrinsic(extrinsic: ::Extrinsic) -> ApplyExtrinsicResult; + #[changed_in(6)] + fn apply_extrinsic(extrinsic: ::Extrinsic) -> ApplyExtrinsicResultBeforeV6; + /// Finish the current block. #[renamed("finalise_block", 3)] fn finalize_block() -> ::Header; diff --git a/primitives/runtime/src/legacy.rs b/primitives/runtime/src/legacy.rs new file mode 100644 index 0000000000000..d8305136af442 --- /dev/null +++ b/primitives/runtime/src/legacy.rs @@ -0,0 +1,20 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 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. + +//! Runtime types that existed in old API versions. + +pub mod before_v6; diff --git a/primitives/runtime/src/legacy/before_v6.rs b/primitives/runtime/src/legacy/before_v6.rs new file mode 100644 index 0000000000000..24f45c253784c --- /dev/null +++ b/primitives/runtime/src/legacy/before_v6.rs @@ -0,0 +1,79 @@ +// This file is part of Substrate. + +// Copyright (C) 2022 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. + +//! Runtime types that existed prior to BlockBuilder API version 6. + +use crate::{ArithmeticError, TokenError}; +use codec::{Decode, Encode}; +use scale_info::TypeInfo; +#[cfg(feature = "std")] +use serde::{Deserialize, Serialize}; + +/// [`ModuleError`] type definition before BlockBuilder API version 6. +#[derive(Eq, Clone, Copy, Encode, Decode, Debug, TypeInfo)] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] +pub struct ModuleError { + /// Module index, matching the metadata module index. + pub index: u8, + /// Module specific error value. + pub error: u8, + /// Optional error message. + #[codec(skip)] + #[cfg_attr(feature = "std", serde(skip_deserializing))] + pub message: Option<&'static str>, +} + +impl PartialEq for ModuleError { + fn eq(&self, other: &Self) -> bool { + (self.index == other.index) && (self.error == other.error) + } +} + +/// [`DispatchError`] type definition before BlockBuilder API version 6. +#[derive(Eq, Clone, Copy, Encode, Decode, Debug, TypeInfo, PartialEq)] +#[cfg_attr(feature = "std", derive(Serialize, Deserialize))] +pub enum DispatchError { + /// Some error occurred. + Other( + #[codec(skip)] + #[cfg_attr(feature = "std", serde(skip_deserializing))] + &'static str, + ), + /// Failed to lookup some data. + CannotLookup, + /// A bad origin. + BadOrigin, + /// A custom error in a module. + Module(ModuleError), + /// At least one consumer is remaining so the account cannot be destroyed. + ConsumerRemaining, + /// There are no providers so the account cannot be created. + NoProviders, + /// There are too many consumers so the account cannot be created. + TooManyConsumers, + /// An error to do with tokens. + Token(TokenError), + /// An arithmetic error. + Arithmetic(ArithmeticError), +} + +/// [`DispatchOutcome`] type definition before BlockBuilder API version 6. +pub type DispatchOutcome = Result<(), DispatchError>; + +/// [`ApplyExtrinsicResult`] type definition before BlockBuilder API version 6. +pub type ApplyExtrinsicResult = + Result; diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 5e242b7bfe10f..fc5a38da87e30 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -57,6 +57,7 @@ use scale_info::TypeInfo; pub mod curve; pub mod generic; +pub mod legacy; mod multiaddress; pub mod offchain; pub mod runtime_logger; From 7595123380f8f2a3c43d1ef91dda15a87b82d85b Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Wed, 16 Feb 2022 21:25:36 -0800 Subject: [PATCH 66/75] Factor in codec attributes while derving PalletError --- frame/support/procedural/src/pallet_error.rs | 131 +++++++++++++++---- frame/support/src/traits/error.rs | 9 +- frame/support/test/tests/pallet.rs | 6 +- 3 files changed, 120 insertions(+), 26 deletions(-) diff --git a/frame/support/procedural/src/pallet_error.rs b/frame/support/procedural/src/pallet_error.rs index 338e9f8c7ff8d..3a4b16882cd23 100644 --- a/frame/support/procedural/src/pallet_error.rs +++ b/frame/support/procedural/src/pallet_error.rs @@ -16,6 +16,8 @@ // limitations under the License. use frame_support_procedural_tools::generate_crate_access_2018; +use quote::ToTokens; +use std::str::FromStr; // Derive `PalletError` pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenStream { @@ -24,6 +26,10 @@ pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenS Err(e) => return e.to_compile_error().into(), }; + let codec = match generate_crate_access_2018("parity-scale-codec") { + Ok(c) => c, + Err(e) => return e.into_compile_error().into(), + }; let frame_support = match generate_crate_access_2018("frame-support") { Ok(c) => c, Err(e) => return e.into_compile_error().into(), @@ -33,22 +39,23 @@ pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenS let max_encoded_size = match data { syn::Data::Struct(syn::DataStruct { fields, .. }) => match fields { - syn::Fields::Named(f) => { - let field_tys = f.named.iter().map(|field| &field.ty); - quote::quote! { - 0_usize - #(.saturating_add(< - #field_tys as #frame_support::traits::PalletError - >::MAX_ENCODED_SIZE))* - } - }, - syn::Fields::Unnamed(f) => { - let field_tys = f.unnamed.iter().map(|field| &field.ty); + syn::Fields::Named(syn::FieldsNamed { named: fields, .. }) | + syn::Fields::Unnamed(syn::FieldsUnnamed { unnamed: fields, .. }) => { + let maybe_field_tys = fields + .iter() + .map(|f| generate_field_types(f, &codec)) + .collect::>>(); + let field_tys = match maybe_field_tys { + Ok(tys) => tys.into_iter().flatten(), + Err(e) => return e.into_compile_error().into(), + }; quote::quote! { 0_usize - #(.saturating_add(< - #field_tys as #frame_support::traits::PalletError - >::MAX_ENCODED_SIZE))* + #( + .saturating_add(< + #field_tys as #frame_support::traits::PalletError + >::MAX_ENCODED_SIZE) + )* } }, syn::Fields::Unit => quote::quote!(0), @@ -56,14 +63,8 @@ pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenS syn::Data::Enum(syn::DataEnum { variants, .. }) => { let field_tys = variants .iter() - .map(|variant| match &variant.fields { - syn::Fields::Named(f) => - Ok(Some(f.named.iter().map(|field| &field.ty).collect::>())), - syn::Fields::Unnamed(f) => - Ok(Some(f.unnamed.iter().map(|field| &field.ty).collect::>())), - syn::Fields::Unit => Ok(None), - }) - .collect::>>, syn::Error>>(); + .map(|variant| generate_variant_field_types(variant, &codec)) + .collect::>>, syn::Error>>(); let field_tys = match field_tys { Ok(tys) => tys.into_iter().flatten().collect::>(), @@ -112,3 +113,89 @@ pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenS ) .into() } + +fn generate_field_types( + field: &syn::Field, + ccrate: &syn::Ident, +) -> syn::Result> { + let attrs = &field.attrs; + + for attr in attrs { + if attr.path.is_ident("codec") { + match attr.parse_meta()? { + syn::Meta::List(ref meta_list) if meta_list.nested.len() == 1 => { + match meta_list + .nested + .first() + .expect("Just checked that there is one item; qed") + { + syn::NestedMeta::Meta(syn::Meta::Path(path)) + if path.get_ident().map_or(false, |i| i == "skip") => + return Ok(None), + + syn::NestedMeta::Meta(syn::Meta::Path(path)) + if path.get_ident().map_or(false, |i| i == "compact") => + { + let field_ty = &field.ty; + return Ok(Some(quote::quote!(#ccrate::Compact<#field_ty>))) + }, + + syn::NestedMeta::Meta(syn::Meta::NameValue(syn::MetaNameValue { + path, + lit: syn::Lit::Str(lit_str), + .. + })) if path.get_ident().map_or(false, |i| i == "encoded_as") => { + let ty = proc_macro2::TokenStream::from_str(&lit_str.value())?; + return Ok(Some(ty)) + }, + + _ => (), + } + }, + _ => (), + } + } + } + + Ok(Some(field.ty.to_token_stream())) +} + +fn generate_variant_field_types( + variant: &syn::Variant, + ccrate: &syn::Ident, +) -> syn::Result>> { + let attrs = &variant.attrs; + + for attr in attrs { + if attr.path.is_ident("codec") { + match attr.parse_meta()? { + syn::Meta::List(ref meta_list) if meta_list.nested.len() == 1 => { + match meta_list + .nested + .first() + .expect("Just checked that there is one item; qed") + { + syn::NestedMeta::Meta(syn::Meta::Path(path)) + if path.get_ident().map_or(false, |i| i == "skip") => + return Ok(None), + + _ => (), + } + }, + _ => (), + } + } + } + + match &variant.fields { + syn::Fields::Named(syn::FieldsNamed { named: fields, .. }) | + syn::Fields::Unnamed(syn::FieldsUnnamed { unnamed: fields, .. }) => { + let field_tys = fields + .iter() + .map(|field| generate_field_types(field, ccrate)) + .collect::>>()?; + Ok(Some(field_tys.into_iter().flatten().collect())) + }, + syn::Fields::Unit => Ok(None), + } +} diff --git a/frame/support/src/traits/error.rs b/frame/support/src/traits/error.rs index 9cfbedc4598d5..8e26891669e65 100644 --- a/frame/support/src/traits/error.rs +++ b/frame/support/src/traits/error.rs @@ -16,7 +16,7 @@ // limitations under the License. //! Traits for describing and constraining pallet error types. -use codec::{Decode, Encode}; +use codec::{Compact, Decode, Encode}; use sp_std::marker::PhantomData; /// Trait indicating that the implementing type is going to be included as a field in a variant of @@ -50,12 +50,15 @@ macro_rules! impl_for_types { impl_for_types!(size: 0, (), crate::Never); impl_for_types!(size: 1, u8, i8, bool); -impl_for_types!(size: 2, u16, i16); -impl_for_types!(size: 4, u32, i32); +impl_for_types!(size: 2, u16, i16, Compact); +impl_for_types!(size: 4, u32, i32, Compact); +impl_for_types!(size: 5, Compact); impl_for_types!(size: 8, u64, i64); +impl_for_types!(size: 9, Compact); // Contains a u64 for secs and u32 for nanos, hence 12 bytes impl_for_types!(size: 12, core::time::Duration); impl_for_types!(size: 16, u128, i128); +impl_for_types!(size: 17, Compact); impl PalletError for PhantomData { const MAX_ENCODED_SIZE: usize = 0; diff --git a/frame/support/test/tests/pallet.rs b/frame/support/test/tests/pallet.rs index 429c4f9bbf5d6..83f6a722f93aa 100644 --- a/frame/support/test/tests/pallet.rs +++ b/frame/support/test/tests/pallet.rs @@ -20,7 +20,7 @@ use frame_support::{ storage::unhashed, traits::{ ConstU32, GetCallName, GetStorageVersion, OnFinalize, OnGenesis, OnInitialize, - OnRuntimeUpgrade, PalletInfoAccess, StorageVersion, + OnRuntimeUpgrade, PalletError, PalletInfoAccess, StorageVersion, }, weights::{DispatchClass, DispatchInfo, GetDispatchInfo, Pays, RuntimeDbWeight}, }; @@ -234,6 +234,9 @@ pub mod pallet { /// doc comment put into metadata InsufficientProposersBalance, Code(u8), + #[codec(skip)] + Skipped(u128), + CompactU8(#[codec(compact)] u8), } #[pallet::event] @@ -662,6 +665,7 @@ fn error_expand() { message: Some("InsufficientProposersBalance") }), ); + assert_eq!( as PalletError>::MAX_ENCODED_SIZE, 3); } #[test] From 89abef7381e88a937ea9727ed8b4fef648beb9d7 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Wed, 16 Feb 2022 22:44:12 -0800 Subject: [PATCH 67/75] Rename module and fix unit test --- client/rpc/src/state/tests.rs | 2 +- primitives/block-builder/src/lib.rs | 2 +- primitives/runtime/src/legacy.rs | 2 +- .../runtime/src/legacy/{before_v6.rs => byte_sized_error.rs} | 0 4 files changed, 3 insertions(+), 3 deletions(-) rename primitives/runtime/src/legacy/{before_v6.rs => byte_sized_error.rs} (100%) diff --git a/client/rpc/src/state/tests.rs b/client/rpc/src/state/tests.rs index 9dbe02cdb7d64..287dfac8c6ba0 100644 --- a/client/rpc/src/state/tests.rs +++ b/client/rpc/src/state/tests.rs @@ -527,7 +527,7 @@ fn should_return_runtime_version() { let result = "{\"specName\":\"test\",\"implName\":\"parity-test\",\"authoringVersion\":1,\ \"specVersion\":2,\"implVersion\":2,\"apis\":[[\"0xdf6acb689907609b\",4],\ - [\"0x37e397fc7c91f5e4\",1],[\"0xd2bc9897eed08f15\",3],[\"0x40fe3ad401f8959a\",5],\ + [\"0x37e397fc7c91f5e4\",1],[\"0xd2bc9897eed08f15\",3],[\"0x40fe3ad401f8959a\",6],\ [\"0xc6e9a76309f39b09\",1],[\"0xdd718d5cc53262d4\",1],[\"0xcbca25e39f142387\",2],\ [\"0xf78b278be53f454c\",2],[\"0xab3c0572291feb8b\",1],[\"0xbc9d89904f5b923f\",1]],\ \"transactionVersion\":1,\"stateVersion\":1}"; diff --git a/primitives/block-builder/src/lib.rs b/primitives/block-builder/src/lib.rs index e1557b910a41f..1b74c27b7ae43 100644 --- a/primitives/block-builder/src/lib.rs +++ b/primitives/block-builder/src/lib.rs @@ -21,7 +21,7 @@ use sp_inherents::{CheckInherentsResult, InherentData}; use sp_runtime::{ - legacy::before_v6::ApplyExtrinsicResult as ApplyExtrinsicResultBeforeV6, + legacy::byte_sized_error::ApplyExtrinsicResult as ApplyExtrinsicResultBeforeV6, traits::Block as BlockT, ApplyExtrinsicResult, }; diff --git a/primitives/runtime/src/legacy.rs b/primitives/runtime/src/legacy.rs index d8305136af442..7bc7c88a7e10d 100644 --- a/primitives/runtime/src/legacy.rs +++ b/primitives/runtime/src/legacy.rs @@ -17,4 +17,4 @@ //! Runtime types that existed in old API versions. -pub mod before_v6; +pub mod byte_sized_error; diff --git a/primitives/runtime/src/legacy/before_v6.rs b/primitives/runtime/src/legacy/byte_sized_error.rs similarity index 100% rename from primitives/runtime/src/legacy/before_v6.rs rename to primitives/runtime/src/legacy/byte_sized_error.rs From 161fb88914bd112b603ec801c306ebfe1e982191 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Fri, 18 Feb 2022 01:43:38 -0800 Subject: [PATCH 68/75] Add missing attribute --- frame/support/procedural/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/support/procedural/src/lib.rs b/frame/support/procedural/src/lib.rs index eb10faf7ec436..92564e94493c1 100644 --- a/frame/support/procedural/src/lib.rs +++ b/frame/support/procedural/src/lib.rs @@ -565,7 +565,7 @@ pub fn match_and_insert(input: TokenStream) -> TokenStream { match_and_insert::match_and_insert(input) } -#[proc_macro_derive(PalletError)] +#[proc_macro_derive(PalletError, attributes(codec))] pub fn derive_pallet_error(input: TokenStream) -> TokenStream { pallet_error::derive_pallet_error(input) } From babebfc86de9d4dd9906ea9f87af178647c82d8c Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Fri, 18 Feb 2022 02:27:46 -0800 Subject: [PATCH 69/75] Check API version and convert ApplyExtrinsicResult accordingly --- client/block-builder/src/lib.rs | 30 +++++++++++--- .../runtime/src/legacy/byte_sized_error.rs | 21 ++++++++++ utils/frame/rpc/system/src/lib.rs | 39 ++++++++++++++++--- 3 files changed, 79 insertions(+), 11 deletions(-) diff --git a/client/block-builder/src/lib.rs b/client/block-builder/src/lib.rs index a4c6f5aad2aeb..3f9fecbccbb9e 100644 --- a/client/block-builder/src/lib.rs +++ b/client/block-builder/src/lib.rs @@ -35,6 +35,7 @@ use sp_blockchain::{ApplyExtrinsicFailed, Error}; use sp_core::ExecutionContext; use sp_runtime::{ generic::BlockId, + legacy, traits::{Block as BlockT, Hash, HashFor, Header as HeaderT, NumberFor, One}, Digest, }; @@ -135,6 +136,7 @@ where pub struct BlockBuilder<'a, Block: BlockT, A: ProvideRuntimeApi, B> { extrinsics: Vec, api: ApiRef<'a, A::Api>, + version: u32, block_id: BlockId, parent_hash: Block::Hash, backend: &'a B, @@ -183,10 +185,15 @@ where api.initialize_block_with_context(&block_id, ExecutionContext::BlockConstruction, &header)?; + let version = api + .api_version::>(&block_id)? + .ok_or_else(|| Error::VersionInvalid("BlockBuilderApi".to_string()))?; + Ok(Self { parent_hash, extrinsics: Vec::new(), api, + version, block_id, backend, estimated_header_size, @@ -199,13 +206,26 @@ where pub fn push(&mut self, xt: ::Extrinsic) -> Result<(), Error> { let block_id = &self.block_id; let extrinsics = &mut self.extrinsics; + let version = self.version; self.api.execute_in_transaction(|api| { - match api.apply_extrinsic_with_context( - block_id, - ExecutionContext::BlockConstruction, - xt.clone(), - ) { + let res = if version < 6 { + #[allow(deprecated)] + api.apply_extrinsic_before_version_6_with_context( + block_id, + ExecutionContext::BlockConstruction, + xt.clone(), + ) + .map(legacy::byte_sized_error::convert_to_latest) + } else { + api.apply_extrinsic_with_context( + block_id, + ExecutionContext::BlockConstruction, + xt.clone(), + ) + }; + + match res { Ok(Ok(_)) => { extrinsics.push(xt); TransactionOutcome::Commit(Ok(())) diff --git a/primitives/runtime/src/legacy/byte_sized_error.rs b/primitives/runtime/src/legacy/byte_sized_error.rs index 24f45c253784c..049abff69ff1a 100644 --- a/primitives/runtime/src/legacy/byte_sized_error.rs +++ b/primitives/runtime/src/legacy/byte_sized_error.rs @@ -77,3 +77,24 @@ pub type DispatchOutcome = Result<(), DispatchError>; /// [`ApplyExtrinsicResult`] type definition before BlockBuilder API version 6. pub type ApplyExtrinsicResult = Result; + +/// Convert the legacy `ApplyExtrinsicResult` type to the latest version. +pub fn convert_to_latest(old: ApplyExtrinsicResult) -> crate::ApplyExtrinsicResult { + old.map(|outcome| { + outcome.map_err(|e| match e { + DispatchError::Other(s) => crate::DispatchError::Other(s), + DispatchError::CannotLookup => crate::DispatchError::CannotLookup, + DispatchError::BadOrigin => crate::DispatchError::BadOrigin, + DispatchError::Module(err) => crate::DispatchError::Module(crate::ModuleError { + index: err.index, + error: [err.error, 0, 0, 0], + message: err.message, + }), + DispatchError::ConsumerRemaining => crate::DispatchError::ConsumerRemaining, + DispatchError::NoProviders => crate::DispatchError::NoProviders, + DispatchError::TooManyConsumers => crate::DispatchError::TooManyConsumers, + DispatchError::Token(err) => crate::DispatchError::Token(err), + DispatchError::Arithmetic(err) => crate::DispatchError::Arithmetic(err), + }) + }) +} diff --git a/utils/frame/rpc/system/src/lib.rs b/utils/frame/rpc/system/src/lib.rs index df24e208b51a4..4b59429e07ba4 100644 --- a/utils/frame/rpc/system/src/lib.rs +++ b/utils/frame/rpc/system/src/lib.rs @@ -25,10 +25,11 @@ use jsonrpc_core::{Error as RpcError, ErrorCode}; use jsonrpc_derive::rpc; use sc_rpc_api::DenyUnsafe; use sc_transaction_pool_api::{InPoolTransaction, TransactionPool}; +use sp_api::ApiExt; use sp_block_builder::BlockBuilder; use sp_blockchain::HeaderBackend; use sp_core::{hexdisplay::HexDisplay, Bytes}; -use sp_runtime::{generic::BlockId, traits}; +use sp_runtime::{generic::BlockId, legacy, traits}; pub use self::gen_client::Client as SystemClient; pub use frame_system_rpc_runtime_api::AccountNonceApi; @@ -138,11 +139,37 @@ where data: Some(format!("{:?}", e).into()), })?; - let result = api.apply_extrinsic(&at, uxt).map_err(|e| RpcError { - code: ErrorCode::ServerError(Error::RuntimeError.into()), - message: "Unable to dry run extrinsic.".into(), - data: Some(format!("{:?}", e).into()), - })?; + let api_version = api + .api_version::>(&at) + .map_err(|e| RpcError { + code: ErrorCode::ServerError(Error::RuntimeError.into()), + message: "Unable to dry run extrinsic.".into(), + data: Some(format!("{:?}", e).into()), + })? + .ok_or_else(|| RpcError { + code: ErrorCode::ServerError(Error::RuntimeError.into()), + message: "Unable to dry run extrinsic.".into(), + data: Some( + format!("Could not find `BlockBuilder` api for block `{:?}`.", at).into(), + ), + })?; + + let result = if api_version < 6 { + #[allow(deprecated)] + api.apply_extrinsic_before_version_6(&at, uxt) + .map(legacy::byte_sized_error::convert_to_latest) + .map_err(|e| RpcError { + code: ErrorCode::ServerError(Error::RuntimeError.into()), + message: "Unable to dry run extrinsic.".into(), + data: Some(format!("{:?}", e).into()), + })? + } else { + api.apply_extrinsic(&at, uxt).map_err(|e| RpcError { + code: ErrorCode::ServerError(Error::RuntimeError.into()), + message: "Unable to dry run extrinsic.".into(), + data: Some(format!("{:?}", e).into()), + })? + }; Ok(Encode::encode(&result).into()) }; From 7fed725dcffc382fde356942e3522e0833ee8244 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Thu, 17 Mar 2022 07:17:18 -0700 Subject: [PATCH 70/75] Rename BagError to ListError Co-authored-by: Kian Paimani <5588131+kianenigma@users.noreply.github.com> --- frame/bags-list/src/lib.rs | 6 +++--- frame/bags-list/src/list/mod.rs | 6 +++--- frame/bags-list/src/list/tests.rs | 2 +- frame/bags-list/src/tests.rs | 2 +- 4 files changed, 8 insertions(+), 8 deletions(-) diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index 818894286efea..18c86b0a9dfd2 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -67,7 +67,7 @@ pub mod mock; mod tests; pub mod weights; -pub use list::{notional_bag_for, Bag, BagError, List, Node}; +pub use list::{notional_bag_for, Bag, ListError, List, Node}; pub use pallet::*; pub use weights::WeightInfo; @@ -254,7 +254,7 @@ impl Pallet { } impl SortedListProvider for Pallet { - type Error = BagError; + type Error = ListError; fn iter() -> Box> { Box::new(List::::iter().map(|n| n.id().clone())) @@ -268,7 +268,7 @@ impl SortedListProvider for Pallet { List::::contains(id) } - fn on_insert(id: T::AccountId, weight: VoteWeight) -> Result<(), BagError> { + fn on_insert(id: T::AccountId, weight: VoteWeight) -> Result<(), ListError> { List::::insert(id, weight) } diff --git a/frame/bags-list/src/list/mod.rs b/frame/bags-list/src/list/mod.rs index c3a089aaefe28..b3f8709453c69 100644 --- a/frame/bags-list/src/list/mod.rs +++ b/frame/bags-list/src/list/mod.rs @@ -38,7 +38,7 @@ use sp_std::{ }; #[derive(Debug, PartialEq, Eq)] -pub enum BagError { +pub enum ListError { /// A duplicate id has been detected. Duplicate, } @@ -261,9 +261,9 @@ impl List { /// Insert a new id into the appropriate bag in the list. /// /// Returns an error if the list already contains `id`. - pub(crate) fn insert(id: T::AccountId, weight: VoteWeight) -> Result<(), BagError> { + pub(crate) fn insert(id: T::AccountId, weight: VoteWeight) -> Result<(), ListError> { if Self::contains(&id) { - return Err(BagError::Duplicate) + return Err(ListError::Duplicate) } let bag_weight = notional_bag_for::(weight); diff --git a/frame/bags-list/src/list/tests.rs b/frame/bags-list/src/list/tests.rs index e951b3b1a5701..1a373539c0ad7 100644 --- a/frame/bags-list/src/list/tests.rs +++ b/frame/bags-list/src/list/tests.rs @@ -242,7 +242,7 @@ mod list { // then assert_storage_noop!(assert_eq!( List::::insert(3, 20).unwrap_err(), - BagError::Duplicate + ListError::Duplicate )); }); } diff --git a/frame/bags-list/src/tests.rs b/frame/bags-list/src/tests.rs index 84fec1bb142fd..76e8eda673c27 100644 --- a/frame/bags-list/src/tests.rs +++ b/frame/bags-list/src/tests.rs @@ -518,7 +518,7 @@ mod sorted_list_provider { // then assert_storage_noop!(assert_eq!( BagsList::on_insert(3, 20).unwrap_err(), - BagError::Duplicate + ListError::Duplicate )); }); } From 9fef2e1703788b554ca5be322d3d6b0140c3351d Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Sun, 20 Mar 2022 05:02:49 -0700 Subject: [PATCH 71/75] Use codec crate re-exported from frame support --- frame/support/procedural/src/pallet_error.rs | 16 ++++++---------- 1 file changed, 6 insertions(+), 10 deletions(-) diff --git a/frame/support/procedural/src/pallet_error.rs b/frame/support/procedural/src/pallet_error.rs index 3a4b16882cd23..216168131e43d 100644 --- a/frame/support/procedural/src/pallet_error.rs +++ b/frame/support/procedural/src/pallet_error.rs @@ -26,10 +26,6 @@ pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenS Err(e) => return e.to_compile_error().into(), }; - let codec = match generate_crate_access_2018("parity-scale-codec") { - Ok(c) => c, - Err(e) => return e.into_compile_error().into(), - }; let frame_support = match generate_crate_access_2018("frame-support") { Ok(c) => c, Err(e) => return e.into_compile_error().into(), @@ -43,7 +39,7 @@ pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenS syn::Fields::Unnamed(syn::FieldsUnnamed { unnamed: fields, .. }) => { let maybe_field_tys = fields .iter() - .map(|f| generate_field_types(f, &codec)) + .map(|f| generate_field_types(f, &frame_support)) .collect::>>(); let field_tys = match maybe_field_tys { Ok(tys) => tys.into_iter().flatten(), @@ -63,7 +59,7 @@ pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenS syn::Data::Enum(syn::DataEnum { variants, .. }) => { let field_tys = variants .iter() - .map(|variant| generate_variant_field_types(variant, &codec)) + .map(|variant| generate_variant_field_types(variant, &frame_support)) .collect::>>, syn::Error>>(); let field_tys = match field_tys { @@ -116,7 +112,7 @@ pub fn derive_pallet_error(input: proc_macro::TokenStream) -> proc_macro::TokenS fn generate_field_types( field: &syn::Field, - ccrate: &syn::Ident, + scrate: &syn::Ident, ) -> syn::Result> { let attrs = &field.attrs; @@ -137,7 +133,7 @@ fn generate_field_types( if path.get_ident().map_or(false, |i| i == "compact") => { let field_ty = &field.ty; - return Ok(Some(quote::quote!(#ccrate::Compact<#field_ty>))) + return Ok(Some(quote::quote!(#scrate::codec::Compact<#field_ty>))) }, syn::NestedMeta::Meta(syn::Meta::NameValue(syn::MetaNameValue { @@ -162,7 +158,7 @@ fn generate_field_types( fn generate_variant_field_types( variant: &syn::Variant, - ccrate: &syn::Ident, + scrate: &syn::Ident, ) -> syn::Result>> { let attrs = &variant.attrs; @@ -192,7 +188,7 @@ fn generate_variant_field_types( syn::Fields::Unnamed(syn::FieldsUnnamed { unnamed: fields, .. }) => { let field_tys = fields .iter() - .map(|field| generate_field_types(field, ccrate)) + .map(|field| generate_field_types(field, scrate)) .collect::>>()?; Ok(Some(field_tys.into_iter().flatten().collect())) }, From ddc2cce43aa0c9c8eaf490db01301ac2226f2e58 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Sun, 20 Mar 2022 05:05:17 -0700 Subject: [PATCH 72/75] Add links to types mentioned in doc comments MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- frame/support/src/lib.rs | 6 +++--- primitives/runtime/src/lib.rs | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index e4b7773bb726c..7143a6289606a 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1676,13 +1676,13 @@ pub mod pallet_prelude { /// I.e. a regular rust enum named `Error`, with generic `T` and fieldless or multiple-field /// variants. /// -/// Any field type in the enum variants must implement `scale_info::TypeInfo` in order to be +/// Any field type in the enum variants must implement [`scale_info::TypeInfo`] in order to be /// properly used in the metadata, and its encoded size should be as small as possible, /// preferably 1 byte in size in order to reduce storage size. The error enum itself has an /// absolute maximum encoded size specified by [`MAX_MODULE_ERROR_ENCODED_SIZE`]. /// -/// Field types in enum variants must also implement `PalletError`, otherwise the pallet will -/// fail to compile. Rust primitive types have already implemented the `PalletError` trait +/// Field types in enum variants must also implement [`PalletError`](traits::PalletError), otherwise the pallet will +/// fail to compile. Rust primitive types have already implemented the [`PalletError`](traits::PalletError) trait /// along with some commonly used stdlib types such as `Option` and `PhantomData`, and hence in /// most use cases, a manual implementation is not necessary and is discouraged. /// diff --git a/primitives/runtime/src/lib.rs b/primitives/runtime/src/lib.rs index 0a89e70d07608..337fac5812aed 100644 --- a/primitives/runtime/src/lib.rs +++ b/primitives/runtime/src/lib.rs @@ -98,7 +98,7 @@ pub use sp_arithmetic::{ pub use either::Either; -/// The number of bytes of the module-specific `error` field defined in `ModuleError`. +/// The number of bytes of the module-specific `error` field defined in [`ModuleError`]. /// In FRAME, this is the maximum encoded size of a pallet error type. pub const MAX_MODULE_ERROR_ENCODED_SIZE: usize = 4; From 23608e7643dd65769f497dac06db4b9d04fdfcbe Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Sun, 20 Mar 2022 05:05:58 -0700 Subject: [PATCH 73/75] cargo fmt --- frame/support/src/lib.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/frame/support/src/lib.rs b/frame/support/src/lib.rs index 7143a6289606a..f290b3ed9d533 100644 --- a/frame/support/src/lib.rs +++ b/frame/support/src/lib.rs @@ -1681,10 +1681,11 @@ pub mod pallet_prelude { /// preferably 1 byte in size in order to reduce storage size. The error enum itself has an /// absolute maximum encoded size specified by [`MAX_MODULE_ERROR_ENCODED_SIZE`]. /// -/// Field types in enum variants must also implement [`PalletError`](traits::PalletError), otherwise the pallet will -/// fail to compile. Rust primitive types have already implemented the [`PalletError`](traits::PalletError) trait -/// along with some commonly used stdlib types such as `Option` and `PhantomData`, and hence in -/// most use cases, a manual implementation is not necessary and is discouraged. +/// Field types in enum variants must also implement [`PalletError`](traits::PalletError), +/// otherwise the pallet will fail to compile. Rust primitive types have already implemented +/// the [`PalletError`](traits::PalletError) trait along with some commonly used stdlib types +/// such as `Option` and `PhantomData`, and hence in most use cases, a manual implementation is +/// not necessary and is discouraged. /// /// The generic `T` mustn't bound anything and where clause is not allowed. But bounds and /// where clause shouldn't be needed for any usecase. From 9d0dfa71c31a313bec16efae05a77badbfc749c9 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Sun, 20 Mar 2022 05:14:39 -0700 Subject: [PATCH 74/75] cargo fmt --- frame/bags-list/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/frame/bags-list/src/lib.rs b/frame/bags-list/src/lib.rs index e7c23ee33fa2e..aa9f1c80dfdbc 100644 --- a/frame/bags-list/src/lib.rs +++ b/frame/bags-list/src/lib.rs @@ -70,7 +70,7 @@ pub mod mock; mod tests; pub mod weights; -pub use list::{notional_bag_for, Bag, ListError, List, Node}; +pub use list::{notional_bag_for, Bag, List, ListError, Node}; pub use pallet::*; pub use weights::WeightInfo; From 2b3b73097b9b97d3d445c39d44691e5882579378 Mon Sep 17 00:00:00 2001 From: Keith Yeung Date: Mon, 21 Mar 2022 05:11:24 -0700 Subject: [PATCH 75/75] Re-add attribute for hidden docs --- frame/support/procedural/src/pallet/expand/error.rs | 1 + 1 file changed, 1 insertion(+) diff --git a/frame/support/procedural/src/pallet/expand/error.rs b/frame/support/procedural/src/pallet/expand/error.rs index 80e0100ce0688..86b06d737decf 100644 --- a/frame/support/procedural/src/pallet/expand/error.rs +++ b/frame/support/procedural/src/pallet/expand/error.rs @@ -129,6 +129,7 @@ pub fn expand_error(def: &mut Def) -> proc_macro2::TokenStream { } impl<#type_impl_gen> #error_ident<#type_use_gen> #config_where_clause { + #[doc(hidden)] pub fn as_str(&self) -> &'static str { match &self { Self::__Ignore(_, _) => unreachable!("`__Ignore` can never be constructed"),