Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

pallet macro: do not generate try-runtime related code when frame-support doesn't have try-runtime. #5099

Merged
merged 7 commits into from
Jul 24, 2024
Merged

pallet macro: do not generate try-runtime related code when frame-support doesn't have try-runtime. #5099

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
c71e315
try-runtime
gui1117 Jul 22, 2024
acef975
add test, though on stable no warning appear anyway, but for future
gui1117 Jul 22, 2024
514e714
redo pass test
gui1117 Jul 23, 2024
61fa26d
do not use new feature, use macro in frame-support instead
gui1117 Jul 23, 2024
c4eea73
Merge remote-tracking branch 'origin/master' into gui-pallet-try-runtime
gui1117 Jul 23, 2024
3422fbb
Merge branch 'master' into gui-pallet-try-runtime
gui1117 Jul 24, 2024
caa1a81
Merge branch 'master' into gui-pallet-try-runtime
bkchr Jul 24, 2024
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
1 change: 1 addition & 0 deletions substrate/frame/support/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ runtime-benchmarks = [
"sp-staking/runtime-benchmarks",
]
try-runtime = [
"frame-support-procedural/try-runtime",
"frame-system/try-runtime",
"sp-debug-derive/force-debug",
"sp-runtime/try-runtime",
Expand Down
3 changes: 3 additions & 0 deletions substrate/frame/support/procedural/Cargo.toml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,9 @@ regex = { workspace = true }
[features]
default = ["std"]
std = ["sp-crypto-hashing/std"]
# Generates the implementation for traits and methods available when frame-support is compiled with
# `try-runtime` feature.
try-runtime = []
no-metadata-docs = []
experimental = []
# Generate impl-trait for tuples with the given number of tuples. Will be needed as the number of
Expand Down
111 changes: 62 additions & 49 deletions substrate/frame/support/procedural/src/pallet/expand/hooks.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,66 @@ pub fn expand_hooks(def: &mut Def) -> proc_macro2::TokenStream {
}
};

let pre_upgrade_post_upgrade = if cfg!(feature = "try-runtime") {
quote::quote_spanned!(span =>
fn pre_upgrade() -> Result<#frame_support::__private::Vec<u8>, #frame_support::sp_runtime::TryRuntimeError> {
<
Self
as
#frame_support::traits::Hooks<#frame_system::pallet_prelude::BlockNumberFor::<T>>
>::pre_upgrade()
}

fn post_upgrade(state: #frame_support::__private::Vec<u8>) -> Result<(), #frame_support::sp_runtime::TryRuntimeError> {
#post_storage_version_check

<
Self
as
#frame_support::traits::Hooks<#frame_system::pallet_prelude::BlockNumberFor::<T>>
>::post_upgrade(state)
}
)
} else {
proc_macro2::TokenStream::new()
};

let try_state = if cfg!(feature = "try-runtime") {
Copy link
Member

Choose a reason for hiding this comment

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

We should not do this. This is partly "cursed". Sometimes this is not really working well.

You can create a macro in frame_support using this macro and then you can use this to guard the code. Here is an example of what I mean.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ah yes, I implemented as you suggested here 61fa26d

quote::quote_spanned!(span =>
impl<#type_impl_gen>
#frame_support::traits::TryState<#frame_system::pallet_prelude::BlockNumberFor::<T>>
for #pallet_ident<#type_use_gen> #where_clause
{
fn try_state(
n: #frame_system::pallet_prelude::BlockNumberFor::<T>,
_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::<T>
>
>::try_state(n).map_err(|err| {
#frame_support::__private::log::error!(
target: #frame_support::LOG_TARGET,
"❌ {:?} try_state checks failed: {:?}",
#pallet_name,
err
);

err
})
}
}
)
} else {
proc_macro2::TokenStream::new()
};

quote::quote_spanned!(span =>
#hooks_impl

Expand Down Expand Up @@ -254,25 +314,7 @@ 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<u8>, #frame_support::sp_runtime::TryRuntimeError> {
<
Self
as
#frame_support::traits::Hooks<#frame_system::pallet_prelude::BlockNumberFor::<T>>
>::pre_upgrade()
}

#[cfg(feature = "try-runtime")]
fn post_upgrade(state: #frame_support::__private::Vec<u8>) -> Result<(), #frame_support::sp_runtime::TryRuntimeError> {
#post_storage_version_check

<
Self
as
#frame_support::traits::Hooks<#frame_system::pallet_prelude::BlockNumberFor::<T>>
>::post_upgrade(state)
}
#pre_upgrade_post_upgrade
}

impl<#type_impl_gen>
Expand Down Expand Up @@ -306,35 +348,6 @@ 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::<T>>
for #pallet_ident<#type_use_gen> #where_clause
{
fn try_state(
n: #frame_system::pallet_prelude::BlockNumberFor::<T>,
_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::<T>
>
>::try_state(n).map_err(|err| {
#frame_support::__private::log::error!(
target: #frame_support::LOG_TARGET,
"❌ {:?} try_state checks failed: {:?}",
#pallet_name,
err
);

err
})
}
}
#try_state
)
}
5 changes: 3 additions & 2 deletions substrate/frame/support/procedural/src/pallet/expand/storage.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -829,7 +829,7 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream {
let type_impl_gen = &def.type_impl_generics(proc_macro2::Span::call_site());
let type_use_gen = &def.type_use_generics(proc_macro2::Span::call_site());

let try_decode_entire_state = {
let try_decode_entire_state = if cfg!(feature = "try-runtime") {
let mut storage_names = def
.storages
.iter()
Expand All @@ -849,7 +849,6 @@ 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
{
Expand Down Expand Up @@ -886,6 +885,8 @@ pub fn expand_storages(def: &mut Def) -> proc_macro2::TokenStream {
}
}
)
} else {
proc_macro2::TokenStream::new()
};

quote::quote!(
Expand Down
Loading