-
Notifications
You must be signed in to change notification settings - Fork 2.7k
Better identifier and logging for runtime upgrades #8123
Changes from 38 commits
4c0abc5
7f9b5b8
4883812
be549b4
8e97733
d84dad4
aeb7a0e
d968f58
ce4128b
ee8ae08
87baef8
01298eb
14bf87f
62be119
712c240
9a23940
93f299a
3cea840
ab9d4a3
b13ea31
d3a2368
c8ba546
726f0b6
ce123d1
67eead2
a444530
511db92
96d8ecf
1e41e0c
2655290
7ed0ecd
be547cb
415bca9
bdad8a5
2d6df71
05a5daf
d6e7b8b
6714bfc
d06d069
4e154f0
280666f
6ff2845
File filter
Filter by extension
Conversations
Jump to
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.
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, | ||
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. Shouldn't this be under the target of the pallet? 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. Then we would need to assume that each pallet is exposing a 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. Ahh the link there is now broken, in this commit
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. I think we should provide some macro 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. 👍 woudl also be good |
||
"⚠️ 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) | ||
|
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.
yes this is needed because otherwise we can't compile node-runtime with try-runtime features and std :-/
the unused dependency on std should harm to much, and this is what we do for frame-benchmarking.