Skip to content
Merged
Show file tree
Hide file tree
Changes from 25 commits
Commits
Show all changes
34 commits
Select commit Hold shift + click to select a range
ca1db8c
initialise on-chain storage versions
liamaharon Aug 30, 2023
1c44c96
Merge branch 'master' into liam/initialise-storage-version
liamaharon Aug 30, 2023
1ab2eca
Merge branch 'master' into liam/initialise-storage-version
liamaharon Sep 2, 2023
1d84c1a
Merge branch 'master' into liam/initialise-storage-version
liamaharon Sep 4, 2023
a2a6078
Merge branch 'master' of github.com:paritytech/polkadot-sdk into liam…
liamaharon Sep 26, 2023
011ec33
move to standalone trait
liamaharon Sep 26, 2023
f12dcb1
remove redundant spacing
liamaharon Sep 26, 2023
9e0980a
Merge branch 'master' into liam/initialise-storage-version
liamaharon Sep 27, 2023
a0ff4b7
fix for Tuple implementation
liamaharon Sep 27, 2023
06e0b10
Merge branch 'master' into liam/initialise-storage-version
liamaharon Sep 28, 2023
62c4c3d
update old test
liamaharon Sep 28, 2023
6a66617
fix log
liamaharon Sep 30, 2023
68183e9
Merge branch 'master' of github.com:paritytech/polkadot-sdk into liam…
liamaharon Oct 10, 2023
f30da51
Merge branch 'master' of github.com:paritytech/polkadot-sdk into liam…
liamaharon Oct 12, 2023
bb0915e
init based on contains prefix rather than version
liamaharon Oct 13, 2023
c4d7f40
Merge branch 'master' into liam/initialise-storage-version
liamaharon Oct 13, 2023
d7ed843
improve log
liamaharon Oct 13, 2023
2319a51
Merge branch 'liam/initialise-storage-version' of github.com:parityte…
liamaharon Oct 13, 2023
41cb160
Update substrate/frame/support/src/traits/metadata.rs
liamaharon Oct 31, 2023
d30765f
Merge branch 'master' of github.com:paritytech/polkadot-sdk into liam…
liamaharon Oct 31, 2023
18a1f82
remove regression
liamaharon Oct 31, 2023
bc50779
reuse pallet name
liamaharon Oct 31, 2023
11b33aa
fix pallet_name
liamaharon Nov 1, 2023
7dccbf6
Merge branch 'master' of github.com:paritytech/polkadot-sdk into liam…
liamaharon Nov 1, 2023
6919547
Merge branch 'master' into liam/initialise-storage-version
liamaharon Nov 2, 2023
5b196f1
Update substrate/frame/support/procedural/src/pallet/expand/hooks.rs
liamaharon Nov 3, 2023
5f00efa
Update substrate/frame/support/src/traits/hooks.rs
liamaharon Nov 3, 2023
031da73
Merge branch 'master' of github.com:paritytech/polkadot-sdk into liam…
liamaharon Nov 6, 2023
a552b6b
address comments
liamaharon Nov 6, 2023
43b587e
fix
liamaharon Nov 6, 2023
dfe7347
restore old tokenstream
liamaharon Nov 7, 2023
dcd7595
Merge branch 'master' of github.com:paritytech/polkadot-sdk into liam…
liamaharon Nov 7, 2023
40e6035
fix lint
liamaharon Nov 7, 2023
fc09aff
fix typo
liamaharon Nov 7, 2023
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
16 changes: 12 additions & 4 deletions substrate/frame/executive/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,8 +121,8 @@ use frame_support::{
dispatch::{DispatchClass, DispatchInfo, GetDispatchInfo, PostDispatchInfo},
pallet_prelude::InvalidTransaction,
traits::{
EnsureInherentsAreFirst, ExecuteBlock, OffchainWorker, OnFinalize, OnIdle, OnInitialize,
OnRuntimeUpgrade,
BeforeAllRuntimeMigrations, EnsureInherentsAreFirst, ExecuteBlock, OffchainWorker,
OnFinalize, OnIdle, OnInitialize, OnRuntimeUpgrade,
},
weights::Weight,
};
Expand Down Expand Up @@ -188,6 +188,7 @@ impl<
Context: Default,
UnsignedValidator,
AllPalletsWithSystem: OnRuntimeUpgrade
+ BeforeAllRuntimeMigrations
+ OnInitialize<BlockNumberFor<System>>
+ OnIdle<BlockNumberFor<System>>
+ OnFinalize<BlockNumberFor<System>>
Expand Down Expand Up @@ -225,6 +226,7 @@ impl<
Context: Default,
UnsignedValidator,
AllPalletsWithSystem: OnRuntimeUpgrade
+ BeforeAllRuntimeMigrations
+ OnInitialize<BlockNumberFor<System>>
+ OnIdle<BlockNumberFor<System>>
+ OnFinalize<BlockNumberFor<System>>
Expand Down Expand Up @@ -355,7 +357,9 @@ where
pub fn try_runtime_upgrade(
checks: frame_try_runtime::UpgradeCheckSelect,
) -> Result<Weight, TryRuntimeError> {
let weight =
let before_all_weight =
<AllPalletsWithSystem as BeforeAllRuntimeMigrations>::before_all_runtime_migrations();
let try_on_runtime_upgrade_weight =
<(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::try_on_runtime_upgrade(
checks.pre_and_post(),
)?;
Expand All @@ -370,7 +374,7 @@ where
)?;
}

Ok(weight)
Ok(before_all_weight.saturating_add(try_on_runtime_upgrade_weight))
}
}

Expand All @@ -383,6 +387,7 @@ impl<
Context: Default,
UnsignedValidator,
AllPalletsWithSystem: OnRuntimeUpgrade
+ BeforeAllRuntimeMigrations
+ OnInitialize<BlockNumberFor<System>>
+ OnIdle<BlockNumberFor<System>>
+ OnFinalize<BlockNumberFor<System>>
Expand All @@ -399,7 +404,10 @@ where
{
/// Execute all `OnRuntimeUpgrade` of this runtime, and return the aggregate weight.
pub fn execute_on_runtime_upgrade() -> Weight {
let before_all_weight =
<AllPalletsWithSystem as BeforeAllRuntimeMigrations>::before_all_runtime_migrations();
<(COnRuntimeUpgrade, AllPalletsWithSystem) as OnRuntimeUpgrade>::on_runtime_upgrade()
.saturating_add(before_all_weight)
}

/// Start the execution of a particular block.
Expand Down
93 changes: 64 additions & 29 deletions substrate/frame/support/procedural/src/pallet/expand/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,38 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream {
let type_use_gen = &def.type_use_generics(span);
let pallet_ident = &def.pallet_struct.pallet;
let frame_system = &def.frame_system;
let pallet_name = quote::quote! {
<
<T as #frame_system::Config>::PalletInfo
as
#frame_support::traits::PalletInfo
>::name::<Self>().unwrap_or("<unknown pallet name>")
};

let initialize_on_chain_storage_version = if let Some(current_version) =
&def.pallet_struct.storage_version
{
quote::quote! {
#frame_support::__private::log::info!(
target: #frame_support::LOG_TARGET,
"🐥 New pallet {:?} detected in the runtime. Initializing the on-chain storage version to match the storage version defined in the pallet: {:?}",
#pallet_name,
#current_version
);
#current_version.put::<Self>();
}
} else {
quote::quote! {
let default_version = #frame_support::traits::StorageVersion::new(0);
#frame_support::__private::log::info!(
target: #frame_support::LOG_TARGET,
"🐥 New pallet {:?} detected in the runtime. The pallet has not defined storage version, so the on-chain version is being initialized to {:?}.",
#pallet_name,
default_version
);
default_version.put::<Self>();
}
};

let log_runtime_upgrade = if has_runtime_upgrade {
// a migration is defined here.
Expand All @@ -42,7 +74,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream {
target: #frame_support::LOG_TARGET,
"⚠️ {} declares internal migrations (which *might* execute). \
On-chain `{:?}` vs current storage version `{:?}`",
pallet_name,
#pallet_name,
<Self as #frame_support::traits::GetStorageVersion>::on_chain_storage_version(),
<Self as #frame_support::traits::GetStorageVersion>::current_storage_version(),
);
Expand All @@ -53,7 +85,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream {
#frame_support::__private::log::debug!(
target: #frame_support::LOG_TARGET,
"✅ no migration for {}",
pallet_name,
#pallet_name,
);
}
};
Expand All @@ -78,16 +110,10 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream {
let current_version = <Self as #frame_support::traits::GetStorageVersion>::current_storage_version();

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

#frame_support::__private::log::error!(
target: #frame_support::LOG_TARGET,
"{}: On chain storage version {:?} doesn't match current storage version {:?}.",
pallet_name,
#pallet_name,
on_chain_version,
current_version,
);
Expand All @@ -100,17 +126,11 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream {
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::__private::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,
#pallet_name,
on_chain_version,
);

Expand Down Expand Up @@ -173,6 +193,32 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream {
}
}

impl<#type_impl_gen>
#frame_support::traits::BeforeAllRuntimeMigrations
for #pallet_ident<#type_use_gen> #where_clause
{
fn before_all_runtime_migrations() -> #frame_support::weights::Weight {
use #frame_support::traits::Get;
use #frame_support::__private::hashing::twox_128;
use #frame_support::storage::unhashed::contains_prefixed_key;
#frame_support::__private::sp_tracing::enter_span!(
#frame_support::__private::sp_tracing::trace_span!("before_all")
);

// Check if the pallet has any keys set, including the storage version. If there are
// no keys set, the pallet was just added to the runtime and needs to have its
// version initialized.
let pallet_hashed_prefix = twox_128(#pallet_name.as_bytes());
let exists = contains_prefixed_key(&pallet_hashed_prefix);
if !exists {
#initialize_on_chain_storage_version
<T as #frame_system::Config>::DbWeight::get().reads_writes(1, 1)
} else {
<T as #frame_system::Config>::DbWeight::get().reads(1)
}
}
}

impl<#type_impl_gen>
#frame_support::traits::OnRuntimeUpgrade
for #pallet_ident<#type_use_gen> #where_clause
Expand All @@ -183,11 +229,6 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream {
);

// log info about the upgrade.
let pallet_name = <
<T as #frame_system::Config>::PalletInfo
as
#frame_support::traits::PalletInfo
>::name::<Self>().unwrap_or("<unknown pallet name>");
#log_runtime_upgrade

<
Expand Down Expand Up @@ -258,16 +299,10 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream {
n: #frame_system::pallet_prelude::BlockNumberFor::<T>,
_s: #frame_support::traits::TryStateSelect
) -> Result<(), #frame_support::sp_runtime::TryRuntimeError> {
let pallet_name = <
<T as #frame_system::Config>::PalletInfo
as
#frame_support::traits::PalletInfo
>::name::<Self>().expect("No name found for the pallet! This usually means that the pallet wasn't added to `construct_runtime!`.");

#frame_support::__private::log::info!(
target: #frame_support::LOG_TARGET,
"🩺 Running {:?} try-state checks",
pallet_name,
#pallet_name,
);
<
Self as #frame_support::traits::Hooks<
Expand All @@ -277,7 +312,7 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream {
#frame_support::__private::log::error!(
target: #frame_support::LOG_TARGET,
"❌ {:?} try_state checks failed: {:?}",
pallet_name,
#pallet_name,
err
);

Expand Down
2 changes: 1 addition & 1 deletion substrate/frame/support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@ pub mod __private {
pub use sp_core::{OpaqueMetadata, Void};
pub use sp_core_hashing_proc_macro;
pub use sp_inherents;
pub use sp_io::{self, storage::root as storage_root};
pub use sp_io::{self, hashing, storage::root as storage_root};
pub use sp_metadata_ir as metadata_ir;
#[cfg(feature = "std")]
pub use sp_runtime::{bounded_btree_map, bounded_vec};
Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/support/src/migrations.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ impl<
let on_chain_version = Pallet::on_chain_storage_version();
if on_chain_version == FROM {
log::info!(
"🚚 Pallet {:?} migrating storage version from {:?} to {:?}.",
"🚚 Pallet {:?} VersionedMigration migrating storage version from {:?} to {:?}.",
Pallet::name(),
FROM,
TO
Expand All @@ -134,7 +134,7 @@ impl<
weight.saturating_add(DbWeight::get().reads_writes(1, 1))
} else {
log::warn!(
"🚚 Pallet {:?} migration {}->{} can be removed; on-chain is already at {:?}.",
"🚚 Pallet {:?} VersionedMigration migration {}->{} can be removed; on-chain is already at {:?}.",
Pallet::name(),
FROM,
TO,
Expand Down
4 changes: 2 additions & 2 deletions substrate/frame/support/src/traits.rs
Original file line number Diff line number Diff line change
Expand Up @@ -84,8 +84,8 @@ mod hooks;
#[allow(deprecated)]
pub use hooks::GenesisBuild;
pub use hooks::{
BuildGenesisConfig, Hooks, IntegrityTest, OnFinalize, OnGenesis, OnIdle, OnInitialize,
OnRuntimeUpgrade, OnTimestampSet,
BeforeAllRuntimeMigrations, BuildGenesisConfig, Hooks, IntegrityTest, OnFinalize, OnGenesis,
OnIdle, OnInitialize, OnRuntimeUpgrade, OnTimestampSet,
};

pub mod schedule;
Expand Down
30 changes: 30 additions & 0 deletions substrate/frame/support/src/traits/hooks.rs
Original file line number Diff line number Diff line change
Expand Up @@ -99,6 +99,22 @@ pub trait OnGenesis {
fn on_genesis() {}
}

/// Implemented by pallets, allows defining logic to run prior to any [`OnRuntimeUpgrade`] logic.
///
/// This was introduced to allow pallets to initialize their on-chain storage version in the case
/// that they are added to the runtime after genesis.
///
/// This hook is intended to be used internally in FRAME and not be exposed to FRAME developers.
///
/// It is defined as a seperate trait from [`OnRuntimeUpgrade`] precisely to not pollute the public
/// API.
pub trait BeforeAllRuntimeMigrations {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

in principle we can shield this to prevent anyone other than FRAME macros to implement it.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That sounds good, are you able to point me in the direction for how that could be achieved?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can not do this. There is no way to only allow our macros to implement a trait.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was hinting at the trait sealing technique but indeed it doesn't properly work here, because the trait that I am intending to seal is implemented at the end of the day by Pallet which is in a different crate. So, by definition, that other crate must have access to the sealed trait as well.

We could still semi-seal it by putting the trait Sealed {} in mod __private and give it some name like trait DoNotImplement {}, but yeah it won't be proper sealing.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

trait ShittySealingPleaseSerDoNotImplementPlease {} lol :P

/// Something that should happen before runtime migrations are executed.
fn before_all_runtime_migrations() -> Weight {
Weight::zero()
}
}

/// See [`Hooks::on_runtime_upgrade`].
pub trait OnRuntimeUpgrade {
/// See [`Hooks::on_runtime_upgrade`].
Expand Down Expand Up @@ -150,10 +166,24 @@ pub trait OnRuntimeUpgrade {
}
}

#[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))]
#[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))]
#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))]
impl BeforeAllRuntimeMigrations for Tuple {
/// Implements the default behavior of
/// [`BeforeAllRuntimeMigrations::before_all_runtime_migrations`] for tuples.
fn before_all_runtime_migrations() -> Weight {
let mut weight = Weight::zero();
for_tuples!( #( weight = weight.saturating_add(Tuple::before_all_runtime_migrations()); )* );
weight
}
}

#[cfg_attr(all(not(feature = "tuples-96"), not(feature = "tuples-128")), impl_for_tuples(64))]
#[cfg_attr(all(feature = "tuples-96", not(feature = "tuples-128")), impl_for_tuples(96))]
#[cfg_attr(feature = "tuples-128", impl_for_tuples(128))]
impl OnRuntimeUpgrade for Tuple {
/// Implements the default behavior of [`OnRuntimeUpgrade::on_runtime_upgrade`] for tuples.
fn on_runtime_upgrade() -> Weight {
let mut weight = Weight::zero();
for_tuples!( #( weight = weight.saturating_add(Tuple::on_runtime_upgrade()); )* );
Expand Down
17 changes: 17 additions & 0 deletions substrate/frame/support/src/traits/metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -222,6 +222,23 @@ impl StorageVersion {

crate::storage::unhashed::get_or_default(&key)
}

/// Returns if the storage version key for the given pallet exists in storage.
///
/// See [`STORAGE_VERSION_STORAGE_KEY_POSTFIX`] on how this key is built.
///
/// # Panics
///
/// This function will panic iff `Pallet` can not be found by `PalletInfo`.
/// In a runtime that is put together using
/// [`construct_runtime!`](crate::construct_runtime) this should never happen.
///
/// It will also panic if this function isn't executed in an externalities
/// provided environment.
pub fn exists<P: PalletInfoAccess>() -> bool {
let key = Self::storage_key::<P>();
crate::storage::unhashed::exists(&key)
}
}

impl PartialEq<u16> for StorageVersion {
Expand Down
Loading