From 028d05f7e149ebe74647b63cba8301adbb554688 Mon Sep 17 00:00:00 2001 From: Guillaume Thiolliere Date: Wed, 24 Jul 2024 19:02:23 +0900 Subject: [PATCH] pallet macro: do not generate try-runtime related code when frame-support doesn't have try-runtime. (#5099) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Status: Ready for review Fix https://github.com/paritytech/polkadot-sdk/issues/5092 Introduce a new macro in frame-support which discard content if `try-runtime` is not enabled. Use this macro inside `frame-support-procedural` to generate code only when `frame-support` is compiled with `try-runtime`. --------- Co-authored-by: Bastian Köcher --- .../procedural/src/pallet/expand/hooks.rs | 81 ++++++++++--------- .../procedural/src/pallet/expand/storage.rs | 63 ++++++++------- substrate/frame/support/src/lib.rs | 2 + .../tests/pallet_ui/pass/simple_storage.rs | 32 ++++++++ 4 files changed, 107 insertions(+), 71 deletions(-) create mode 100644 substrate/frame/support/test/tests/pallet_ui/pass/simple_storage.rs diff --git a/substrate/frame/support/procedural/src/pallet/expand/hooks.rs b/substrate/frame/support/procedural/src/pallet/expand/hooks.rs index d7b1ca14f574b..1b0c09c4e3657 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/hooks.rs @@ -254,24 +254,24 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { >::on_runtime_upgrade() } - #[cfg(feature = "try-runtime")] - fn pre_upgrade() -> Result<#frame_support::__private::Vec, #frame_support::sp_runtime::TryRuntimeError> { - < - Self - as - #frame_support::traits::Hooks<#frame_system::pallet_prelude::BlockNumberFor::> - >::pre_upgrade() - } + #frame_support::try_runtime_enabled! { + fn pre_upgrade() -> Result<#frame_support::__private::Vec, #frame_support::sp_runtime::TryRuntimeError> { + < + Self + as + #frame_support::traits::Hooks<#frame_system::pallet_prelude::BlockNumberFor::> + >::pre_upgrade() + } - #[cfg(feature = "try-runtime")] - fn post_upgrade(state: #frame_support::__private::Vec) -> Result<(), #frame_support::sp_runtime::TryRuntimeError> { - #post_storage_version_check + fn post_upgrade(state: #frame_support::__private::Vec) -> Result<(), #frame_support::sp_runtime::TryRuntimeError> { + #post_storage_version_check - < - Self - as - #frame_support::traits::Hooks<#frame_system::pallet_prelude::BlockNumberFor::> - >::post_upgrade(state) + < + Self + as + #frame_support::traits::Hooks<#frame_system::pallet_prelude::BlockNumberFor::> + >::post_upgrade(state) + } } } @@ -306,34 +306,35 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { } } - #[cfg(feature = "try-runtime")] - impl<#type_impl_gen> - #frame_support::traits::TryState<#frame_system::pallet_prelude::BlockNumberFor::> - for #pallet_ident<#type_use_gen> #where_clause - { - fn try_state( - n: #frame_system::pallet_prelude::BlockNumberFor::, - _s: #frame_support::traits::TryStateSelect - ) -> Result<(), #frame_support::sp_runtime::TryRuntimeError> { - #frame_support::__private::log::info!( - target: #frame_support::LOG_TARGET, - "🩺 Running {:?} try-state checks", - #pallet_name, - ); - < - Self as #frame_support::traits::Hooks< - #frame_system::pallet_prelude::BlockNumberFor:: - > - >::try_state(n).map_err(|err| { - #frame_support::__private::log::error!( + #frame_support::try_runtime_enabled! { + impl<#type_impl_gen> + #frame_support::traits::TryState<#frame_system::pallet_prelude::BlockNumberFor::> + for #pallet_ident<#type_use_gen> #where_clause + { + fn try_state( + n: #frame_system::pallet_prelude::BlockNumberFor::, + _s: #frame_support::traits::TryStateSelect + ) -> Result<(), #frame_support::sp_runtime::TryRuntimeError> { + #frame_support::__private::log::info!( target: #frame_support::LOG_TARGET, - "❌ {:?} try_state checks failed: {:?}", + "🩺 Running {:?} try-state checks", #pallet_name, - err ); + < + Self as #frame_support::traits::Hooks< + #frame_system::pallet_prelude::BlockNumberFor:: + > + >::try_state(n).map_err(|err| { + #frame_support::__private::log::error!( + target: #frame_support::LOG_TARGET, + "❌ {:?} try_state checks failed: {:?}", + #pallet_name, + err + ); - err - }) + err + }) + } } } ) diff --git a/substrate/frame/support/procedural/src/pallet/expand/storage.rs b/substrate/frame/support/procedural/src/pallet/expand/storage.rs index 267b0f2dd3ba4..9abb3029bbd94 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/storage.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/storage.rs @@ -849,39 +849,40 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream { storage_names.sort_by_cached_key(|ident| ident.to_string()); quote::quote!( - #[cfg(feature = "try-runtime")] - impl<#type_impl_gen> #frame_support::traits::TryDecodeEntireStorage - for #pallet_ident<#type_use_gen> #completed_where_clause - { - fn try_decode_entire_state() -> Result> { - let pallet_name = <::PalletInfo as #frame_support::traits::PalletInfo> - ::name::<#pallet_ident<#type_use_gen>>() - .expect("Every active pallet has a name in the runtime; qed"); - - #frame_support::__private::log::debug!(target: "runtime::try-decode-state", "trying to decode pallet: {pallet_name}"); - - // NOTE: for now, we have to exclude storage items that are feature gated. - let mut errors = #frame_support::__private::Vec::new(); - let mut decoded = 0usize; - - #( - #frame_support::__private::log::debug!(target: "runtime::try-decode-state", "trying to decode storage: \ - {pallet_name}::{}", stringify!(#storage_names)); + #frame_support::try_runtime_enabled! { + impl<#type_impl_gen> #frame_support::traits::TryDecodeEntireStorage + for #pallet_ident<#type_use_gen> #completed_where_clause + { + fn try_decode_entire_state() -> Result> { + let pallet_name = <::PalletInfo as #frame_support::traits::PalletInfo> + ::name::<#pallet_ident<#type_use_gen>>() + .expect("Every active pallet has a name in the runtime; qed"); + + #frame_support::__private::log::debug!(target: "runtime::try-decode-state", "trying to decode pallet: {pallet_name}"); + + // NOTE: for now, we have to exclude storage items that are feature gated. + let mut errors = #frame_support::__private::Vec::new(); + let mut decoded = 0usize; + + #( + #frame_support::__private::log::debug!(target: "runtime::try-decode-state", "trying to decode storage: \ + {pallet_name}::{}", stringify!(#storage_names)); + + match <#storage_names as #frame_support::traits::TryDecodeEntireStorage>::try_decode_entire_state() { + Ok(count) => { + decoded += count; + }, + Err(err) => { + errors.extend(err); + }, + } + )* - match <#storage_names as #frame_support::traits::TryDecodeEntireStorage>::try_decode_entire_state() { - Ok(count) => { - decoded += count; - }, - Err(err) => { - errors.extend(err); - }, + if errors.is_empty() { + Ok(decoded) + } else { + Err(errors) } - )* - - if errors.is_empty() { - Ok(decoded) - } else { - Err(errors) } } } diff --git a/substrate/frame/support/src/lib.rs b/substrate/frame/support/src/lib.rs index 06696488e2988..4864059167a97 100644 --- a/substrate/frame/support/src/lib.rs +++ b/substrate/frame/support/src/lib.rs @@ -2480,6 +2480,8 @@ pub use frame_support_procedural::register_default_impl; // Generate a macro that will enable/disable code based on `std` feature being active. sp_core::generate_feature_enabled_macro!(std_enabled, feature = "std", $); +// Generate a macro that will enable/disable code based on `try-runtime` feature being active. +sp_core::generate_feature_enabled_macro!(try_runtime_enabled, feature = "try-runtime", $); // Helper for implementing GenesisBuilder runtime API pub mod genesis_builder_helper; diff --git a/substrate/frame/support/test/tests/pallet_ui/pass/simple_storage.rs b/substrate/frame/support/test/tests/pallet_ui/pass/simple_storage.rs new file mode 100644 index 0000000000000..13ed3b2306fa0 --- /dev/null +++ b/substrate/frame/support/test/tests/pallet_ui/pass/simple_storage.rs @@ -0,0 +1,32 @@ +// This file is part of Substrate. + +// Copyright (C) Parity Technologies (UK) Ltd. +// SPDX-License-Identifier: Apache-2.0 + +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +#[frame_support::pallet] +mod pallet { + use frame_support::pallet_prelude::*; + + #[pallet::config(with_default)] + pub trait Config: frame_system::Config {} + + #[pallet::pallet] + pub struct Pallet(_); + + #[pallet::storage] + pub type MyStorage = StorageValue<_, u32>; +} + +fn main() {}