diff --git a/substrate/Cargo.lock b/substrate/Cargo.lock index 41e0593662..edde0fed26 100644 --- a/substrate/Cargo.lock +++ b/substrate/Cargo.lock @@ -2596,6 +2596,7 @@ name = "frame-support-test" version = "3.0.0" dependencies = [ "frame-benchmarking", + "frame-executive", "frame-support", "frame-support-test-pallet", "frame-system", diff --git a/substrate/frame/support/procedural/src/pallet/expand/hooks.rs b/substrate/frame/support/procedural/src/pallet/expand/hooks.rs index 711b78e642..df07040b82 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/hooks.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/hooks.rs @@ -82,6 +82,57 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { proc_macro2::TokenStream::new() }; + // If a storage version is set, we should ensure that the storage version on chain matches the + // current storage version. This assumes that `Executive` is running custom migrations before + // the pallets are called. + let post_storage_version_check = if def.pallet_struct.storage_version.is_some() { + quote::quote! { + let on_chain_version = ::on_chain_storage_version(); + let current_version = ::current_storage_version(); + + if on_chain_version != current_version { + let pallet_name = < + ::PalletInfo + as + #frame_support::traits::PalletInfo + >::name::().unwrap_or(""); + + #frame_support::log::error!( + target: #frame_support::LOG_TARGET, + "{}: On chain storage version {:?} doesn't match current storage version {:?}.", + pallet_name, + on_chain_version, + current_version, + ); + + return Err("On chain and current storage version do not match. Missing runtime upgrade?"); + } + } + } else { + quote::quote! { + let on_chain_version = ::on_chain_storage_version(); + + if on_chain_version != #frame_support::traits::StorageVersion::new(0) { + let pallet_name = < + ::PalletInfo + as + #frame_support::traits::PalletInfo + >::name::().unwrap_or(""); + + #frame_support::log::error!( + target: #frame_support::LOG_TARGET, + "{}: On chain storage version {:?} is set to non zero,\ + while the pallet is missing the `#[pallet::storage_version(VERSION)]` attribute.", + pallet_name, + on_chain_version, + ); + + return Err("On chain storage version set, while the pallet doesn't \ + have the `#[pallet::storage_version(VERSION)]` attribute."); + } + } + }; + quote::quote_spanned!(span => #hooks_impl @@ -170,6 +221,8 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream { #[cfg(feature = "try-runtime")] fn post_upgrade(state: #frame_support::sp_std::vec::Vec) -> Result<(), &'static str> { + #post_storage_version_check + < Self as diff --git a/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs b/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs index 7acfb90906..99d2d79f23 100644 --- a/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs +++ b/substrate/frame/support/procedural/src/pallet/expand/pallet_struct.rs @@ -160,11 +160,15 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { } ); - let storage_version = if let Some(v) = def.pallet_struct.storage_version.as_ref() { - quote::quote! { #v } - } else { - quote::quote! { #frame_support::traits::StorageVersion::default() } - }; + let (storage_version, current_storage_version_ty) = + if let Some(v) = def.pallet_struct.storage_version.as_ref() { + (quote::quote! { #v }, quote::quote! { #frame_support::traits::StorageVersion }) + } else { + ( + quote::quote! { core::default::Default::default() }, + quote::quote! { #frame_support::traits::NoStorageVersionSet }, + ) + }; let whitelisted_storage_idents: Vec = def .storages @@ -199,7 +203,9 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { for #pallet_ident<#type_use_gen> #config_where_clause { - fn current_storage_version() -> #frame_support::traits::StorageVersion { + type CurrentStorageVersion = #current_storage_version_ty; + + fn current_storage_version() -> Self::CurrentStorageVersion { #storage_version } @@ -214,7 +220,7 @@ pub fn expand_pallet_struct(def: &mut Def) -> proc_macro2::TokenStream { #config_where_clause { fn on_genesis() { - let storage_version = #storage_version; + let storage_version: #frame_support::traits::StorageVersion = #storage_version; storage_version.put::(); } } diff --git a/substrate/frame/support/src/dispatch.rs b/substrate/frame/support/src/dispatch.rs index c75d75b41a..db9405245f 100644 --- a/substrate/frame/support/src/dispatch.rs +++ b/substrate/frame/support/src/dispatch.rs @@ -2500,7 +2500,9 @@ macro_rules! decl_module { impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::traits::GetStorageVersion for $module<$trait_instance $(, $instance)?> where $( $other_where_bounds )* { - fn current_storage_version() -> $crate::traits::StorageVersion { + type CurrentStorageVersion = $crate::traits::StorageVersion; + + fn current_storage_version() -> Self::CurrentStorageVersion { $( $storage_version )* } @@ -2508,6 +2510,16 @@ macro_rules! decl_module { $crate::traits::StorageVersion::get::() } } + + // Implement `OnGenesis` for `Module` + impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::traits::OnGenesis + for $module<$trait_instance $(, $instance)?> where $( $other_where_bounds )* + { + fn on_genesis() { + let storage_version = ::current_storage_version(); + storage_version.put::(); + } + } }; // Implementation for `GetStorageVersion` when no storage version is passed. @@ -2519,7 +2531,9 @@ macro_rules! decl_module { impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::traits::GetStorageVersion for $module<$trait_instance $(, $instance)?> where $( $other_where_bounds )* { - fn current_storage_version() -> $crate::traits::StorageVersion { + type CurrentStorageVersion = $crate::traits::NoStorageVersionSet; + + fn current_storage_version() -> Self::CurrentStorageVersion { Default::default() } @@ -2527,6 +2541,16 @@ macro_rules! decl_module { $crate::traits::StorageVersion::get::() } } + + // Implement `OnGenesis` for `Module` + impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::traits::OnGenesis + for $module<$trait_instance $(, $instance)?> where $( $other_where_bounds )* + { + fn on_genesis() { + let storage_version = $crate::traits::StorageVersion::default(); + storage_version.put::(); + } + } }; // The main macro expansion that actually renders the module code. @@ -2814,16 +2838,6 @@ macro_rules! decl_module { } } - // Implement `OnGenesis` for `Module` - impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::traits::OnGenesis - for $mod_type<$trait_instance $(, $instance)?> where $( $other_where_bounds )* - { - fn on_genesis() { - let storage_version = ::current_storage_version(); - storage_version.put::(); - } - } - // manual implementation of clone/eq/partialeq because using derive erroneously requires // clone/eq/partialeq from T. impl<$trait_instance: $trait_name $(, $instance: $instantiable)?> $crate::dispatch::Clone diff --git a/substrate/frame/support/src/migrations.rs b/substrate/frame/support/src/migrations.rs index f54b73aad8..381f1feda9 100644 --- a/substrate/frame/support/src/migrations.rs +++ b/substrate/frame/support/src/migrations.rs @@ -18,7 +18,7 @@ #[cfg(feature = "try-runtime")] use crate::storage::unhashed::contains_prefixed_key; use crate::{ - traits::{GetStorageVersion, PalletInfoAccess}, + traits::{GetStorageVersion, NoStorageVersionSet, PalletInfoAccess, StorageVersion}, weights::{RuntimeDbWeight, Weight}, }; use impl_trait_for_tuples::impl_for_tuples; @@ -28,12 +28,38 @@ use sp_std::marker::PhantomData; #[cfg(feature = "try-runtime")] use sp_std::vec::Vec; +/// Can store the current pallet version in storage. +pub trait StoreCurrentStorageVersion { + /// Write the current storage version to the storage. + fn store_current_storage_version(); +} + +impl + PalletInfoAccess> + StoreCurrentStorageVersion for StorageVersion +{ + fn store_current_storage_version() { + let version = ::current_storage_version(); + version.put::(); + } +} + +impl + PalletInfoAccess> + StoreCurrentStorageVersion for NoStorageVersionSet +{ + fn store_current_storage_version() { + StorageVersion::default().put::(); + } +} + /// Trait used by [`migrate_from_pallet_version_to_storage_version`] to do the actual migration. pub trait PalletVersionToStorageVersionHelper { fn migrate(db_weight: &RuntimeDbWeight) -> Weight; } -impl PalletVersionToStorageVersionHelper for T { +impl PalletVersionToStorageVersionHelper for T +where + T::CurrentStorageVersion: StoreCurrentStorageVersion, +{ fn migrate(db_weight: &RuntimeDbWeight) -> Weight { const PALLET_VERSION_STORAGE_KEY_POSTFIX: &[u8] = b":__PALLET_VERSION__:"; @@ -43,8 +69,8 @@ impl PalletVersionToStorageVersionHelpe sp_io::storage::clear(&pallet_version_key(::name())); - let version = ::current_storage_version(); - version.put::(); + >::store_current_storage_version( + ); db_weight.writes(2) } diff --git a/substrate/frame/support/src/traits.rs b/substrate/frame/support/src/traits.rs index 17d2fb06c8..b2e6a6138c 100644 --- a/substrate/frame/support/src/traits.rs +++ b/substrate/frame/support/src/traits.rs @@ -75,8 +75,8 @@ pub use randomness::Randomness; mod metadata; pub use metadata::{ CallMetadata, CrateVersion, GetCallIndex, GetCallMetadata, GetCallName, GetStorageVersion, - PalletInfo, PalletInfoAccess, PalletInfoData, PalletsInfoAccess, StorageVersion, - STORAGE_VERSION_STORAGE_KEY_POSTFIX, + NoStorageVersionSet, PalletInfo, PalletInfoAccess, PalletInfoData, PalletsInfoAccess, + StorageVersion, STORAGE_VERSION_STORAGE_KEY_POSTFIX, }; mod hooks; diff --git a/substrate/frame/support/src/traits/metadata.rs b/substrate/frame/support/src/traits/metadata.rs index 20ddb1d34c..54d264ec65 100644 --- a/substrate/frame/support/src/traits/metadata.rs +++ b/substrate/frame/support/src/traits/metadata.rs @@ -232,6 +232,16 @@ impl PartialOrd for StorageVersion { } } +/// Special marker struct if no storage version is set for a pallet. +/// +/// If you (the reader) end up here, it probably means that you tried to compare +/// [`GetStorageVersion::on_chain_storage_version`] against +/// [`GetStorageVersion::current_storage_version`]. This basically means that the +/// [`storage_version`](crate::pallet_macros::storage_version) is missing in the pallet where the +/// mentioned functions are being called. +#[derive(Debug, Default)] +pub struct NoStorageVersionSet; + /// Provides information about the storage version of a pallet. /// /// It differentiates between current and on-chain storage version. Both should be only out of sync @@ -244,8 +254,18 @@ impl PartialOrd for StorageVersion { /// /// It is required to update the on-chain storage version manually when a migration was applied. pub trait GetStorageVersion { + /// This will be filled out by the [`pallet`](crate::pallet) macro. + /// + /// If the [`storage_version`](crate::pallet_macros::storage_version) attribute isn't given + /// this is set to [`NoStorageVersionSet`] to inform the user that the attribute is missing. + /// This should prevent that the user forgets to set a storage version when required. However, + /// this will only work when the user actually tries to call [`Self::current_storage_version`] + /// to compare it against the [`Self::on_chain_storage_version`]. If the attribute is given, + /// this will be set to [`StorageVersion`]. + type CurrentStorageVersion; + /// Returns the current storage version as supported by the pallet. - fn current_storage_version() -> StorageVersion; + fn current_storage_version() -> Self::CurrentStorageVersion; /// Returns the on-chain storage version of the pallet as stored in the storage. fn on_chain_storage_version() -> StorageVersion; } diff --git a/substrate/frame/support/test/Cargo.toml b/substrate/frame/support/test/Cargo.toml index 9564b6a04d..68411210f9 100644 --- a/substrate/frame/support/test/Cargo.toml +++ b/substrate/frame/support/test/Cargo.toml @@ -26,9 +26,10 @@ sp-core = { version = "7.0.0", default-features = false, path = "../../../primit sp-std = { version = "5.0.0", default-features = false, path = "../../../primitives/std" } sp-version = { version = "5.0.0", default-features = false, path = "../../../primitives/version" } trybuild = { version = "1.0.74", features = [ "diff" ] } -pretty_assertions = "1.2.1" +pretty_assertions = "1.3.0" rustversion = "1.0.6" frame-system = { version = "4.0.0-dev", default-features = false, path = "../../system" } +frame-executive = { version = "4.0.0-dev", default-features = false, path = "../../executive" } # The "std" feature for this pallet is never activated on purpose, in order to test construct_runtime error message test-pallet = { package = "frame-support-test-pallet", default-features = false, path = "pallet" } @@ -39,6 +40,7 @@ std = [ "codec/std", "scale-info/std", "frame-benchmarking/std", + "frame-executive/std", "frame-support/std", "frame-system/std", "sp-core/std", @@ -50,7 +52,11 @@ std = [ "sp-version/std", "sp-api/std", ] -try-runtime = ["frame-support/try-runtime"] +try-runtime = [ + "frame-support/try-runtime", + "frame-system/try-runtime", + "frame-executive/try-runtime", +] # WARNING: # Only CI runs with this feature enabled. This feature is for testing stuff related to the FRAME macros # in conjunction with rust features. diff --git a/substrate/frame/support/test/tests/derive_no_bound.rs b/substrate/frame/support/test/tests/derive_no_bound.rs index 8b1394fcd5..dc78027f22 100644 --- a/substrate/frame/support/test/tests/derive_no_bound.rs +++ b/substrate/frame/support/test/tests/derive_no_bound.rs @@ -50,6 +50,23 @@ struct StructNamed { phantom: core::marker::PhantomData<(U, V)>, } +#[rustversion::attr(not(stable), ignore)] +#[cfg(not(feature = "disable-ui-tests"))] +#[test] +fn test_struct_named_debug_print() { + let a_1 = StructNamed:: { + a: 1, + b: 2, + c: 3, + phantom: Default::default(), + }; + + assert_eq!( + format!("{:?}", a_1), + String::from("StructNamed { a: 1, b: 2, c: 3, phantom: PhantomData<(derive_no_bound::ImplNone, derive_no_bound::ImplNone)> }") + ); +} + #[test] fn test_struct_named() { let a_1 = StructNamed:: { @@ -70,10 +87,6 @@ fn test_struct_named() { assert_eq!(a_2.b, 2); assert_eq!(a_2.c, 3); assert_eq!(a_2, a_1); - assert_eq!( - format!("{:?}", a_1), - String::from("StructNamed { a: 1, b: 2, c: 3, phantom: PhantomData<(derive_no_bound::ImplNone, derive_no_bound::ImplNone)> }") - ); let b = StructNamed:: { a: 1, @@ -88,6 +101,14 @@ fn test_struct_named() { #[derive(DebugNoBound, CloneNoBound, EqNoBound, PartialEqNoBound, DefaultNoBound)] struct StructUnnamed(u32, u64, T::C, core::marker::PhantomData<(U, V)>); +#[rustversion::attr(not(stable), ignore)] +#[cfg(not(feature = "disable-ui-tests"))] +#[test] +fn test_struct_unnamed_debug_print() { + let a_1 = StructUnnamed::(1, 2, 3, Default::default()); + assert_eq!(format!("{:?}", a_1), String::from("StructUnnamed(1, 2, 3, PhantomData<(derive_no_bound::ImplNone, derive_no_bound::ImplNone)>)")); +} + #[test] fn test_struct_unnamed() { let a_1 = StructUnnamed::(1, 2, 3, Default::default()); @@ -103,7 +124,6 @@ fn test_struct_unnamed() { assert_eq!(a_2.1, 2); assert_eq!(a_2.2, 3); assert_eq!(a_2, a_1); - assert_eq!(format!("{:?}", a_1), String::from("StructUnnamed(1, 2, 3, PhantomData<(derive_no_bound::ImplNone, derive_no_bound::ImplNone)>)")); let b = StructUnnamed::(1, 2, 4, Default::default()); @@ -169,6 +189,28 @@ enum Enum3 { VariantUnit2, } +#[rustversion::attr(not(stable), ignore)] +#[cfg(not(feature = "disable-ui-tests"))] +#[test] +fn test_enum_debug_print() { + type TestEnum = Enum; + let variant_0 = TestEnum::VariantUnnamed(1, 2, 3, Default::default()); + let variant_1 = TestEnum::VariantNamed { a: 1, b: 2, c: 3, phantom: Default::default() }; + let variant_2 = TestEnum::VariantUnit; + let variant_3 = TestEnum::VariantUnit2; + + assert_eq!( + format!("{:?}", variant_0), + String::from("Enum::VariantUnnamed(1, 2, 3, PhantomData<(derive_no_bound::ImplNone, derive_no_bound::ImplNone)>)"), + ); + assert_eq!( + format!("{:?}", variant_1), + String::from("Enum::VariantNamed { a: 1, b: 2, c: 3, phantom: PhantomData<(derive_no_bound::ImplNone, derive_no_bound::ImplNone)> }"), + ); + assert_eq!(format!("{:?}", variant_2), String::from("Enum::VariantUnit")); + assert_eq!(format!("{:?}", variant_3), String::from("Enum::VariantUnit2")); +} + #[test] fn test_enum() { type TestEnum = Enum; @@ -208,15 +250,4 @@ fn test_enum() { assert!(variant_1.clone() == variant_1); assert!(variant_2.clone() == variant_2); assert!(variant_3.clone() == variant_3); - - assert_eq!( - format!("{:?}", variant_0), - String::from("Enum::VariantUnnamed(1, 2, 3, PhantomData<(derive_no_bound::ImplNone, derive_no_bound::ImplNone)>)"), - ); - assert_eq!( - format!("{:?}", variant_1), - String::from("Enum::VariantNamed { a: 1, b: 2, c: 3, phantom: PhantomData<(derive_no_bound::ImplNone, derive_no_bound::ImplNone)> }"), - ); - assert_eq!(format!("{:?}", variant_2), String::from("Enum::VariantUnit")); - assert_eq!(format!("{:?}", variant_3), String::from("Enum::VariantUnit2")); } diff --git a/substrate/frame/support/test/tests/pallet.rs b/substrate/frame/support/test/tests/pallet.rs index f3c4e07403..7f15ad1f92 100644 --- a/substrate/frame/support/test/tests/pallet.rs +++ b/substrate/frame/support/test/tests/pallet.rs @@ -23,6 +23,7 @@ use frame_support::{ }, dispatch_context::with_context, pallet_prelude::{StorageInfoTrait, ValueQuery}, + parameter_types, storage::unhashed, traits::{ ConstU32, GetCallIndex, GetCallName, GetStorageVersion, OnFinalize, OnGenesis, @@ -37,6 +38,11 @@ use sp_io::{ }; use sp_runtime::{DispatchError, ModuleError}; +parameter_types! { + /// Used to control if the storage version should be updated. + storage UpdateStorageVersion: bool = false; +} + /// Latest stable metadata version used for testing. const LATEST_METADATA_VERSION: u32 = 14; @@ -500,10 +506,12 @@ pub mod pallet { // and that a pallet with the attribute without_storage_info is correctly handled. #[frame_support::pallet] pub mod pallet2 { - use super::{SomeAssociation1, SomeType1}; + use super::{SomeAssociation1, SomeType1, UpdateStorageVersion}; use frame_support::pallet_prelude::*; use frame_system::pallet_prelude::*; + pub(crate) const STORAGE_VERSION: StorageVersion = StorageVersion::new(2); + #[pallet::config] pub trait Config: frame_system::Config where @@ -513,6 +521,7 @@ pub mod pallet2 { } #[pallet::pallet] + #[pallet::storage_version(STORAGE_VERSION)] #[pallet::without_storage_info] pub struct Pallet(_); @@ -530,6 +539,11 @@ pub mod pallet2 { } fn on_runtime_upgrade() -> Weight { Self::deposit_event(Event::Something(31)); + + if UpdateStorageVersion::get() { + Self::current_storage_version().put::(); + } + Weight::zero() } } @@ -683,7 +697,8 @@ impl pallet5::Config for Runtime { pub type Header = sp_runtime::generic::Header; pub type Block = sp_runtime::generic::Block; -pub type UncheckedExtrinsic = sp_runtime::generic::UncheckedExtrinsic; +pub type UncheckedExtrinsic = + sp_runtime::testing::TestXt>; frame_support::construct_runtime!( pub struct Runtime where @@ -812,7 +827,7 @@ fn inherent_expand() { let inherents = InherentData::new().create_extrinsics(); let expected = vec![UncheckedExtrinsic { - function: RuntimeCall::Example(pallet::Call::foo_no_post_info {}), + call: RuntimeCall::Example(pallet::Call::foo_no_post_info {}), signature: None, }]; assert_eq!(expected, inherents); @@ -827,11 +842,11 @@ fn inherent_expand() { ), vec![ UncheckedExtrinsic { - function: RuntimeCall::Example(pallet::Call::foo_no_post_info {}), + call: RuntimeCall::Example(pallet::Call::foo_no_post_info {}), signature: None, }, UncheckedExtrinsic { - function: RuntimeCall::Example(pallet::Call::foo { foo: 1, bar: 0 }), + call: RuntimeCall::Example(pallet::Call::foo { foo: 1, bar: 0 }), signature: None, }, ], @@ -849,11 +864,11 @@ fn inherent_expand() { ), vec![ UncheckedExtrinsic { - function: RuntimeCall::Example(pallet::Call::foo_no_post_info {}), + call: RuntimeCall::Example(pallet::Call::foo_no_post_info {}), signature: None, }, UncheckedExtrinsic { - function: RuntimeCall::Example(pallet::Call::foo { foo: 0, bar: 0 }), + call: RuntimeCall::Example(pallet::Call::foo { foo: 0, bar: 0 }), signature: None, }, ], @@ -870,7 +885,7 @@ fn inherent_expand() { Digest::default(), ), vec![UncheckedExtrinsic { - function: RuntimeCall::Example(pallet::Call::foo_storage_layer { foo: 0 }), + call: RuntimeCall::Example(pallet::Call::foo_storage_layer { foo: 0 }), signature: None, }], ); @@ -888,8 +903,8 @@ fn inherent_expand() { Digest::default(), ), vec![UncheckedExtrinsic { - function: RuntimeCall::Example(pallet::Call::foo_no_post_info {}), - signature: Some((1, (), ())), + call: RuntimeCall::Example(pallet::Call::foo_no_post_info {}), + signature: Some((1, Default::default())), }], ); @@ -907,11 +922,11 @@ fn inherent_expand() { ), vec![ UncheckedExtrinsic { - function: RuntimeCall::Example(pallet::Call::foo { foo: 1, bar: 1 }), + call: RuntimeCall::Example(pallet::Call::foo { foo: 1, bar: 1 }), signature: None, }, UncheckedExtrinsic { - function: RuntimeCall::Example(pallet::Call::foo_storage_layer { foo: 0 }), + call: RuntimeCall::Example(pallet::Call::foo_storage_layer { foo: 0 }), signature: None, }, ], @@ -929,15 +944,15 @@ fn inherent_expand() { ), vec![ UncheckedExtrinsic { - function: RuntimeCall::Example(pallet::Call::foo { foo: 1, bar: 1 }), + call: RuntimeCall::Example(pallet::Call::foo { foo: 1, bar: 1 }), signature: None, }, UncheckedExtrinsic { - function: RuntimeCall::Example(pallet::Call::foo_storage_layer { foo: 0 }), + call: RuntimeCall::Example(pallet::Call::foo_storage_layer { foo: 0 }), signature: None, }, UncheckedExtrinsic { - function: RuntimeCall::Example(pallet::Call::foo_no_post_info {}), + call: RuntimeCall::Example(pallet::Call::foo_no_post_info {}), signature: None, }, ], @@ -955,15 +970,15 @@ fn inherent_expand() { ), vec![ UncheckedExtrinsic { - function: RuntimeCall::Example(pallet::Call::foo { foo: 1, bar: 1 }), + call: RuntimeCall::Example(pallet::Call::foo { foo: 1, bar: 1 }), signature: None, }, UncheckedExtrinsic { - function: RuntimeCall::Example(pallet::Call::foo { foo: 1, bar: 0 }), - signature: Some((1, (), ())), + call: RuntimeCall::Example(pallet::Call::foo { foo: 1, bar: 0 }), + signature: Some((1, Default::default())), }, UncheckedExtrinsic { - function: RuntimeCall::Example(pallet::Call::foo_no_post_info {}), + call: RuntimeCall::Example(pallet::Call::foo_no_post_info {}), signature: None, }, ], @@ -1278,7 +1293,7 @@ fn migrate_from_pallet_version_to_storage_version() { assert!(sp_io::storage::get(&pallet_version_key(System::name())).is_none()); assert_eq!(Example::on_chain_storage_version(), pallet::STORAGE_VERSION); - assert_eq!(Example2::on_chain_storage_version(), StorageVersion::new(0)); + assert_eq!(Example2::on_chain_storage_version(), pallet2::STORAGE_VERSION); assert_eq!(System::on_chain_storage_version(), StorageVersion::new(0)); }); } @@ -2040,6 +2055,95 @@ fn test_storage_alias() { }) } +#[cfg(feature = "try-runtime")] +#[test] +fn post_runtime_upgrade_detects_storage_version_issues() { + use frame_support::traits::UpgradeCheckSelect; + + struct CustomUpgrade; + + impl OnRuntimeUpgrade for CustomUpgrade { + fn on_runtime_upgrade() -> Weight { + Example2::current_storage_version().put::(); + + Default::default() + } + } + + struct CustomUpgradePallet4; + + impl OnRuntimeUpgrade for CustomUpgradePallet4 { + fn on_runtime_upgrade() -> Weight { + StorageVersion::new(100).put::(); + + Default::default() + } + } + + type Executive = frame_executive::Executive< + Runtime, + Block, + frame_system::ChainContext, + Runtime, + AllPalletsWithSystem, + >; + + type ExecutiveWithUpgrade = frame_executive::Executive< + Runtime, + Block, + frame_system::ChainContext, + Runtime, + AllPalletsWithSystem, + CustomUpgrade, + >; + + type ExecutiveWithUpgradePallet4 = frame_executive::Executive< + Runtime, + Block, + frame_system::ChainContext, + Runtime, + AllPalletsWithSystem, + CustomUpgradePallet4, + >; + + TestExternalities::default().execute_with(|| { + // Call `on_genesis` to put the storage version of `Example` into the storage. + Example::on_genesis(); + // The version isn't changed, we should detect it. + assert!(Executive::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost) + .unwrap_err() + .contains("On chain and current storage version do not match")); + }); + + TestExternalities::default().execute_with(|| { + // Call `on_genesis` to put the storage version of `Example` into the storage. + Example::on_genesis(); + // We set the new storage version in the pallet and that should be detected. + UpdateStorageVersion::set(&true); + Executive::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost).unwrap(); + }); + + TestExternalities::default().execute_with(|| { + // Call `on_genesis` to put the storage version of `Example` into the storage. + Example::on_genesis(); + // We set the new storage version in the custom upgrade and that should be detected. + ExecutiveWithUpgrade::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost).unwrap(); + }); + + TestExternalities::default().execute_with(|| { + // Call `on_genesis` to put the storage version of `Example` into the storage. + Example::on_genesis(); + // We need to set the correct storage version for `Example2` + UpdateStorageVersion::set(&true); + + // `CustomUpgradePallet4` will set a storage version for `Example4` while this doesn't has + // any storage version "enabled". + assert!(ExecutiveWithUpgradePallet4::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost) + .unwrap_err() + .contains("On chain storage version set, while the pallet doesn't")); + }); +} + #[test] fn test_dispatch_context() { TestExternalities::default().execute_with(|| { diff --git a/substrate/frame/support/test/tests/pallet_ui/compare_unset_storage_version.rs b/substrate/frame/support/test/tests/pallet_ui/compare_unset_storage_version.rs new file mode 100644 index 0000000000..e417c619fc --- /dev/null +++ b/substrate/frame/support/test/tests/pallet_ui/compare_unset_storage_version.rs @@ -0,0 +1,27 @@ +#[frame_support::pallet] +mod pallet { + use frame_support::pallet_prelude::*; + use frame_system::pallet_prelude::*; + + #[pallet::config] + pub trait Config: frame_system::Config {} + + #[pallet::pallet] + pub struct Pallet(core::marker::PhantomData); + + #[pallet::hooks] + impl Hooks> for Pallet { + fn on_runtime_upgrade() -> Weight { + if Self::current_storage_version() != Self::on_chain_storage_version() { + + } + + Default::default() + } + } + + #[pallet::call] + impl Pallet {} +} + +fn main() {} diff --git a/substrate/frame/support/test/tests/pallet_ui/compare_unset_storage_version.stderr b/substrate/frame/support/test/tests/pallet_ui/compare_unset_storage_version.stderr new file mode 100644 index 0000000000..e75aa52261 --- /dev/null +++ b/substrate/frame/support/test/tests/pallet_ui/compare_unset_storage_version.stderr @@ -0,0 +1,7 @@ +error[E0369]: binary operation `!=` cannot be applied to type `NoStorageVersionSet` + --> tests/pallet_ui/compare_unset_storage_version.rs:15:39 + | +15 | if Self::current_storage_version() != Self::on_chain_storage_version() { + | ------------------------------- ^^ -------------------------------- StorageVersion + | | + | NoStorageVersionSet diff --git a/substrate/frame/support/test/tests/runtime_metadata.rs b/substrate/frame/support/test/tests/runtime_metadata.rs index 8c04c785a3..70ca307d44 100644 --- a/substrate/frame/support/test/tests/runtime_metadata.rs +++ b/substrate/frame/support/test/tests/runtime_metadata.rs @@ -218,5 +218,5 @@ fn runtime_metadata() { let rt = Runtime; let runtime_metadata = (&rt).runtime_metadata(); - assert_eq!(runtime_metadata, expected_runtime_metadata); + pretty_assertions::assert_eq!(runtime_metadata, expected_runtime_metadata); } diff --git a/substrate/frame/system/src/extensions/check_non_zero_sender.rs b/substrate/frame/system/src/extensions/check_non_zero_sender.rs index b0b6704fe7..92eed60fc6 100644 --- a/substrate/frame/system/src/extensions/check_non_zero_sender.rs +++ b/substrate/frame/system/src/extensions/check_non_zero_sender.rs @@ -17,7 +17,7 @@ use crate::Config; use codec::{Decode, Encode}; -use frame_support::dispatch::DispatchInfo; +use frame_support::{dispatch::DispatchInfo, DefaultNoBound}; use scale_info::TypeInfo; use sp_runtime::{ traits::{DispatchInfoOf, Dispatchable, SignedExtension}, @@ -28,9 +28,9 @@ use sp_runtime::{ use sp_std::{marker::PhantomData, prelude::*}; /// Check to ensure that the sender is not the zero address. -#[derive(Encode, Decode, Clone, Eq, PartialEq, TypeInfo)] +#[derive(Encode, Decode, DefaultNoBound, Clone, Eq, PartialEq, TypeInfo)] #[scale_info(skip_type_params(T))] -pub struct CheckNonZeroSender(PhantomData); +pub struct CheckNonZeroSender(PhantomData); impl sp_std::fmt::Debug for CheckNonZeroSender { #[cfg(feature = "std")] diff --git a/substrate/primitives/api/Cargo.toml b/substrate/primitives/api/Cargo.toml index 55e3993466..1d140854f6 100644 --- a/substrate/primitives/api/Cargo.toml +++ b/substrate/primitives/api/Cargo.toml @@ -55,4 +55,4 @@ std = [ # This sets the max logging level to `off` for `log`. disable-logging = ["log/max_level_off"] # Do not report the documentation in the metadata. -no-metadata-docs = [] +no-metadata-docs = ["sp-api-proc-macro/no-metadata-docs"] diff --git a/substrate/primitives/api/proc-macro/Cargo.toml b/substrate/primitives/api/proc-macro/Cargo.toml index 594c20e82c..9d721950f9 100644 --- a/substrate/primitives/api/proc-macro/Cargo.toml +++ b/substrate/primitives/api/proc-macro/Cargo.toml @@ -27,7 +27,8 @@ Inflector = "0.11.4" [dev-dependencies] assert_matches = "1.3.0" -# Required for the doc tests [features] +# Required for the doc tests default = ["std"] std = [] +no-metadata-docs = [] diff --git a/substrate/scripts/ci/gitlab/pipeline/test.yml b/substrate/scripts/ci/gitlab/pipeline/test.yml index ecfcf9ff33..52130f5b7b 100644 --- a/substrate/scripts/ci/gitlab/pipeline/test.yml +++ b/substrate/scripts/ci/gitlab/pipeline/test.yml @@ -246,8 +246,8 @@ test-frame-support: script: - rusty-cachier snapshot create - cat /cargo_target_dir/debug/.fingerprint/memory_units-759eddf317490d2b/lib-memory_units.json || true - - time cargo test --verbose --locked -p frame-support-test --features=frame-feature-testing,no-metadata-docs --manifest-path ./frame/support/test/Cargo.toml --test pallet - - time cargo test --verbose --locked -p frame-support-test --features=frame-feature-testing,frame-feature-testing-2,no-metadata-docs --manifest-path ./frame/support/test/Cargo.toml --test pallet + - time cargo test --verbose --locked -p frame-support-test --features=frame-feature-testing,no-metadata-docs,try-runtime --manifest-path ./frame/support/test/Cargo.toml + - time cargo test --verbose --locked -p frame-support-test --features=frame-feature-testing,frame-feature-testing-2,no-metadata-docs,try-runtime --manifest-path ./frame/support/test/Cargo.toml - SUBSTRATE_TEST_TIMEOUT=1 time cargo test -p substrate-test-utils --release --verbose --locked -- --ignored timeout - cat /cargo_target_dir/debug/.fingerprint/memory_units-759eddf317490d2b/lib-memory_units.json || true - rusty-cachier cache upload