This repository has been archived by the owner on Nov 15, 2023. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Better identifier and logging for runtime upgrades #8123
Merged
Merged
Changes from 37 commits
Commits
Show all changes
42 commits
Select commit
Hold shift + click to select a range
4c0abc5
A clean new attempt
kianenigma 7f9b5b8
Checkpoint to move remote.
kianenigma 4883812
A lot of dependency wiring to make it feature gated.
kianenigma be549b4
bad macro, bad macro.
kianenigma 8e97733
Master.into()
kianenigma d84dad4
Undo the DB mess.
kianenigma aeb7a0e
Update frame/support/src/traits.rs
kianenigma d968f58
Apply suggestions from code review
kianenigma ce4128b
unbreak the build
kianenigma ee8ae08
Merge branch 'kiz-finally-finally-finally-finally-migration-testing-2…
kianenigma 87baef8
Better logging and ids for migrations
kianenigma 01298eb
Fix doc.
kianenigma 14bf87f
Test
kianenigma 62be119
Master.into()
kianenigma 712c240
Update frame/try-runtime/src/lib.rs
kianenigma 9a23940
Update utils/frame/try-runtime/cli/Cargo.toml
kianenigma 93f299a
Update frame/try-runtime/Cargo.toml
kianenigma 3cea840
Address most review grumbles.
kianenigma ab9d4a3
Upstream.into()
kianenigma b13ea31
Fix build
kianenigma d3a2368
Add some comments
kianenigma c8ba546
Remove allowing one pallet at a time.
kianenigma 726f0b6
Merge branch 'kiz-finally-finally-finally-finally-migration-testing-2…
kianenigma ce123d1
Rework the PR
kianenigma 67eead2
nit
kianenigma a444530
Master.into()
kianenigma 511db92
Slightly better error handling.
kianenigma 96d8ecf
Remove files
kianenigma 1e41e0c
Update utils/frame/remote-externalities/src/lib.rs
kianenigma 2655290
Merge branch 'master' of github.com:paritytech/substrate into kiz-mig…
kianenigma 7ed0ecd
Update frame/support/src/dispatch.rs
kianenigma be547cb
Master.into
kianenigma 415bca9
Update frame/support/src/dispatch.rs
kianenigma bdad8a5
Fix test
kianenigma 2d6df71
Merge branch 'kiz-migration-testin-2' of github.com:paritytech/substr…
kianenigma 05a5daf
Merge branch 'master' of github.com:paritytech/substrate into kiz-mig…
kianenigma d6e7b8b
Make extension trait.
kianenigma 6714bfc
Bring back try-runtime/std
kianenigma d06d069
Merge branch 'master' of github.com:paritytech/substrate into kiz-mig…
kianenigma 4e154f0
remove bincode
kianenigma 280666f
Remove warning
kianenigma 6ff2845
Change test features
kianenigma File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -80,27 +80,29 @@ impl<T> Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {} | |
/// // FRAME pallets. | ||
/// #[weight = 0] | ||
/// fn my_function(origin, var: u64) -> dispatch::DispatchResult { | ||
/// // Your implementation | ||
/// Ok(()) | ||
/// // Your implementation | ||
/// Ok(()) | ||
/// } | ||
/// | ||
/// // Public functions are both dispatchable and available to other | ||
/// // Public functions are both dispatchable and available to other | ||
/// // FRAME pallets. | ||
/// #[weight = 0] | ||
/// pub fn my_public_function(origin) -> dispatch::DispatchResult { | ||
/// pub fn my_public_function(origin) -> dispatch::DispatchResult { | ||
/// // Your implementation | ||
/// Ok(()) | ||
/// Ok(()) | ||
/// } | ||
/// } | ||
/// } | ||
/// } | ||
/// # fn main() {} | ||
/// ``` | ||
/// | ||
/// The declaration is set with the header where: | ||
/// | ||
/// * `Module`: The struct generated by the macro, with type `Config`. | ||
/// * `Call`: The enum generated for every pallet, which implements [`Callable`](./dispatch/trait.Callable.html). | ||
/// * `origin`: Alias of `T::Origin`, declared by the [`impl_outer_origin!`](./macro.impl_outer_origin.html) macro. | ||
/// * `Call`: The enum generated for every pallet, which implements | ||
/// [`Callable`](./dispatch/trait.Callable.html). | ||
/// * `origin`: Alias of `T::Origin`, declared by the | ||
/// [`impl_outer_origin!`](./macro.impl_outer_origin.html) macro. | ||
/// * `Result`: The expected return type from pallet functions. | ||
/// | ||
/// The first parameter of dispatchable functions must always be `origin`. | ||
|
@@ -119,15 +121,15 @@ impl<T> Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {} | |
/// pub struct Module<T: Config> for enum Call where origin: T::Origin { | ||
/// #[weight = 0] | ||
/// fn my_long_function(origin) -> dispatch::DispatchResult { | ||
/// // Your implementation | ||
/// // Your implementation | ||
/// Ok(()) | ||
/// } | ||
/// | ||
/// #[weight = 0] | ||
/// fn my_short_function(origin) { | ||
/// // Your implementation | ||
/// // Your implementation | ||
/// } | ||
/// } | ||
/// } | ||
/// } | ||
/// # fn main() {} | ||
/// ``` | ||
|
@@ -184,7 +186,7 @@ impl<T> Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {} | |
/// #[weight = 0] | ||
/// #[transactional] | ||
/// fn my_short_function(origin) { | ||
/// // Your implementation | ||
/// // Your implementation | ||
/// } | ||
/// } | ||
/// } | ||
|
@@ -203,12 +205,12 @@ impl<T> Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {} | |
/// decl_module! { | ||
/// pub struct Module<T: Config> for enum Call where origin: T::Origin { | ||
/// #[weight = 0] | ||
/// fn my_privileged_function(origin) -> dispatch::DispatchResult { | ||
/// fn my_privileged_function(origin) -> dispatch::DispatchResult { | ||
/// ensure_root(origin)?; | ||
/// // Your implementation | ||
/// // Your implementation | ||
/// Ok(()) | ||
/// } | ||
/// } | ||
/// } | ||
/// } | ||
/// # fn main() {} | ||
/// ``` | ||
|
@@ -218,15 +220,17 @@ impl<T> Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {} | |
/// Attributes on functions are supported, but must be in the order of: | ||
/// 1. Optional #\[doc\] attribute. | ||
/// 2. #\[weight\] attribute. | ||
/// 3. Optional function attributes, for instance #\[transactional\]. Those function attributes will be written | ||
/// only on the dispatchable functions implemented on `Module`, not on the `Call` enum variant. | ||
/// 3. Optional function attributes, for instance #\[transactional\]. Those function attributes will | ||
/// be written only on the dispatchable functions implemented on `Module`, not on the `Call` enum | ||
/// variant. | ||
/// | ||
/// ## Multiple Module Instances Example | ||
/// | ||
/// A Substrate module can be built such that multiple instances of the same module can be used within a single | ||
/// runtime. For example, the [Balances module](../pallet_balances/index.html) can be added multiple times to your | ||
/// runtime in order to support multiple, independent currencies for your blockchain. Here is an example of how | ||
/// you would declare such a module using the `decl_module!` macro: | ||
/// A Substrate module can be built such that multiple instances of the same module can be used | ||
/// within a single runtime. For example, the [Balances module](../pallet_balances/index.html) can | ||
/// be added multiple times to your runtime in order to support multiple, independent currencies for | ||
/// your blockchain. Here is an example of how you would declare such a module using the | ||
/// `decl_module!` macro: | ||
/// | ||
/// ``` | ||
/// # #[macro_use] | ||
|
@@ -251,10 +255,10 @@ impl<T> Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {} | |
/// | ||
/// ## Where clause | ||
/// | ||
/// Besides the default `origin: T::Origin`, you can also pass other bounds to the module declaration. | ||
/// This where bound will be replicated to all types generated by this macro. The chaining of multiple | ||
/// trait bounds with `+` is not supported. If multiple bounds for one type are required, it needs to | ||
/// be split up into multiple bounds. | ||
/// Besides the default `origin: T::Origin`, you can also pass other bounds to the module | ||
/// declaration. This where bound will be replicated to all types generated by this macro. The | ||
/// chaining of multiple trait bounds with `+` is not supported. If multiple bounds for one type are | ||
/// required, it needs to be split up into multiple bounds. | ||
/// | ||
/// ``` | ||
/// # #[macro_use] | ||
|
@@ -276,16 +280,18 @@ impl<T> Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {} | |
/// The following are reserved function signatures: | ||
/// | ||
/// * `deposit_event`: Helper function for depositing an [event](https://docs.substrate.dev/docs/event-enum). | ||
/// The default behavior is to call `deposit_event` from the [System module](../frame_system/index.html). | ||
/// However, you can write your own implementation for events in your runtime. To use the default behavior, | ||
/// add `fn deposit_event() = default;` to your `Module`. | ||
/// The default behavior is to call `deposit_event` from the [System | ||
/// module](../frame_system/index.html). However, you can write your own implementation for events | ||
/// in your runtime. To use the default behavior, add `fn deposit_event() = default;` to your | ||
/// `Module`. | ||
/// | ||
/// The following reserved functions also take the block number (with type `T::BlockNumber`) as an optional input: | ||
/// The following reserved functions also take the block number (with type `T::BlockNumber`) as an | ||
/// optional input: | ||
/// | ||
/// * `on_runtime_upgrade`: Executes at the beginning of a block prior to on_initialize when there | ||
/// is a runtime upgrade. This allows each module to upgrade its storage before the storage items are used. | ||
/// As such, **calling other modules must be avoided**!! Using this function will implement the | ||
/// [`OnRuntimeUpgrade`](../sp_runtime/traits/trait.OnRuntimeUpgrade.html) trait. | ||
/// is a runtime upgrade. This allows each module to upgrade its storage before the storage items | ||
/// are used. As such, **calling other modules must be avoided**!! Using this function will | ||
/// implement the [`OnRuntimeUpgrade`](../sp_runtime/traits/trait.OnRuntimeUpgrade.html) trait. | ||
/// Function signature must be `fn on_runtime_upgrade() -> frame_support::weights::Weight`. | ||
/// | ||
/// * `on_initialize`: Executes at the beginning of a block. Using this function will | ||
|
@@ -300,11 +306,11 @@ impl<T> Parameter for T where T: Codec + EncodeLike + Clone + Eq + fmt::Debug {} | |
/// * `fn on_finalize(n: BlockNumber) -> frame_support::weights::Weight` or | ||
/// * `fn on_finalize() -> frame_support::weights::Weight` | ||
/// | ||
/// * `offchain_worker`: Executes at the beginning of a block and produces extrinsics for a future block | ||
/// upon completion. Using this function will implement the | ||
/// * `offchain_worker`: Executes at the beginning of a block and produces extrinsics for a future | ||
/// block upon completion. Using this function will implement the | ||
/// [`OffchainWorker`](./traits/trait.OffchainWorker.html) trait. | ||
/// * `integrity_test`: Executes in a test generated by `construct_runtime`, note it doesn't | ||
/// execute in an externalities-provided environment. Implement | ||
/// * `integrity_test`: Executes in a test generated by `construct_runtime`, note it doesn't execute | ||
/// in an externalities-provided environment. Implement | ||
/// [`IntegrityTest`](./trait.IntegrityTest.html) trait. | ||
#[macro_export] | ||
macro_rules! decl_module { | ||
|
@@ -1325,13 +1331,27 @@ macro_rules! decl_module { | |
$crate::sp_tracing::enter_span!($crate::sp_tracing::trace_span!("on_runtime_upgrade")); | ||
let result: $return = (|| { $( $impl )* })(); | ||
|
||
$crate::crate_to_pallet_version!() | ||
let new_storage_version = $crate::crate_to_pallet_version!(); | ||
new_storage_version | ||
.put_into_storage::<<$trait_instance as $system::Config>::PalletInfo, Self>(); | ||
|
||
let additional_write = < | ||
<$trait_instance as $system::Config>::DbWeight as $crate::traits::Get<_> | ||
>::get().writes(1); | ||
|
||
let pallet_name = << | ||
$trait_instance | ||
as | ||
$system::Config | ||
>::PalletInfo as $crate::traits::PalletInfo>::name::<Self>().expect("pallet will have name in the runtime; qed"); | ||
|
||
$crate::debug::info!( | ||
target: $crate::LOG_TARGET, | ||
"⚠️ running migration for {} and setting new storage version to {:?}", | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And you want to print this for each runtime upgrade? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. exactly, see example in the description, I think it is very important to have an overview of wtf is doing what. |
||
pallet_name, | ||
new_storage_version, | ||
); | ||
|
||
result.saturating_add(additional_write) | ||
} | ||
} | ||
|
@@ -1349,9 +1369,23 @@ macro_rules! decl_module { | |
fn on_runtime_upgrade() -> $crate::dispatch::Weight { | ||
$crate::sp_tracing::enter_span!($crate::sp_tracing::trace_span!("on_runtime_upgrade")); | ||
|
||
$crate::crate_to_pallet_version!() | ||
let new_storage_version = $crate::crate_to_pallet_version!(); | ||
new_storage_version | ||
.put_into_storage::<<$trait_instance as $system::Config>::PalletInfo, Self>(); | ||
|
||
let pallet_name = << | ||
$trait_instance | ||
as | ||
$system::Config | ||
>::PalletInfo as $crate::traits::PalletInfo>::name::<Self>().expect("pallet will have name in the runtime; qed"); | ||
|
||
$crate::debug::info!( | ||
target: $crate::LOG_TARGET, | ||
"✅ no migration for '{}' and setting new storage version to {:?}", | ||
pallet_name, | ||
new_storage_version, | ||
); | ||
|
||
< | ||
<$trait_instance as $system::Config>::DbWeight as $crate::traits::Get<_> | ||
>::get().writes(1) | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't this be under the target of the pallet?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Then we would need to assume that each pallet is exposing a
LOG_TARGET
or some similar identifier. I actually propsed it earlier in this PR, but it didn't gain any attention :( #8128 (comment)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ahh the link there is now broken, in this commit
if you look for TODO you will find my note that: 87baef8#diff-6faadaafc4a2f7556e8ba9cf9bd8544e5c039e97441e1d470a88c296d39819edR1378-R1383
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think we should provide some macro
log_target!(Self)
that is concatenatingruntime
with the name ofSelf
in the runtime (by usingPalletInfo
)There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 woudl also be good