Skip to content
This repository was archived by the owner on Nov 15, 2023. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
Ensure we don't set a storage version when the pallet is missing the …
…attribute
  • Loading branch information
bkchr committed Apr 27, 2023
commit 399af7be24eea192c88c0333035bcd46a4ef6cc8
21 changes: 20 additions & 1 deletion frame/support/procedural/src/pallet/expand/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -109,7 +109,26 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream {
}
}
} else {
quote::quote! {}
quote::quote! {
let on_chain_version = <Self as #frame_support::traits::GetStorageVersion>::on_chain_storage_version();

if on_chain_version != #frame_support::traits::StorageVersion::new(0) {
let pallet_name = <
<T as #frame_system::Config>::PalletInfo
as
#frame_support::traits::PalletInfo
>::name::<Self>().unwrap_or("<unknown pallet name>");

#frame_support::log::error!(
target: #frame_support::LOG_TARGET,
"{pallet_name}: On chain storage version {on_chain_version:?} is set to non zero,\
while the pallet is missing the `#[pallet::storage_version(VERSION)]` attribute.",
);

return Err("On chain storage version set, while pallet doesn't \
has the `#[pallet::storage_version(VERSION)]` attribute.");
}
}
};

quote::quote_spanned!(span =>
Expand Down
39 changes: 35 additions & 4 deletions frame/support/test/tests/pallet.rs
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,7 @@ use sp_runtime::{DispatchError, ModuleError};

parameter_types! {
/// Used to control if the storage version should be updated.
static UpdateStorageVersion: bool = false;
storage UpdateStorageVersion: bool = false;
}

/// Latest stable metadata version used for testing.
Expand Down Expand Up @@ -2057,7 +2057,7 @@ fn test_storage_alias() {

#[cfg(feature = "try-runtime")]
#[test]
fn post_runtime_upgrade_detects_missing_storage_version_change() {
fn post_runtime_upgrade_detects_storage_version_issues() {
use frame_support::traits::UpgradeCheckSelect;

struct CustomUpgrade;
Expand All @@ -2070,6 +2070,16 @@ fn post_runtime_upgrade_detects_missing_storage_version_change() {
}
}

struct CustomUpgradePallet4;

impl OnRuntimeUpgrade for CustomUpgradePallet4 {
fn on_runtime_upgrade() -> Weight {
StorageVersion::new(100).put::<Example4>();

Default::default()
}
}

type Executive = frame_executive::Executive<
Runtime,
Block,
Expand All @@ -2087,6 +2097,15 @@ fn post_runtime_upgrade_detects_missing_storage_version_change() {
CustomUpgrade,
>;

type ExecutiveWithUpgradePallet4 = frame_executive::Executive<
Runtime,
Block,
frame_system::ChainContext<Runtime>,
Runtime,
AllPalletsWithSystem,
CustomUpgradePallet4,
>;

TestExternalities::default().execute_with(|| {
// Call `on_genesis` to put the storage version of `Example` into the storage.
Example::on_genesis();
Expand All @@ -2100,9 +2119,8 @@ fn post_runtime_upgrade_detects_missing_storage_version_change() {
// 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);
UpdateStorageVersion::set(&true);
Executive::try_runtime_upgrade(UpgradeCheckSelect::PreAndPost).unwrap();
UpdateStorageVersion::set(false);
});

TestExternalities::default().execute_with(|| {
Expand All @@ -2111,6 +2129,19 @@ fn post_runtime_upgrade_detects_missing_storage_version_change() {
// 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 pallet doesn't"));
});
}

#[test]
Expand Down