This repository was 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
Add on_idle hook #8209
Merged
Merged
Add on_idle hook #8209
Changes from all commits
Commits
Show all changes
48 commits
Select commit
Hold shift + click to select a range
d49a114
add in idle hook
JesseAbram 7bb95a3
remaining weight passed through to on_idle
JesseAbram 3bf72c5
added weight return
JesseAbram c4bebca
remove TODO
JesseAbram ad2adb9
weight adjustment fix
JesseAbram 99a8bce
added adjusted weight into tuple
JesseAbram 848f147
Update frame/support/src/dispatch.rs
JesseAbram cf010e4
Update frame/support/src/dispatch.rs
JesseAbram ff4826e
Update frame/support/src/dispatch.rs
JesseAbram 1a41fd1
Update frame/support/src/dispatch.rs
JesseAbram 4879b79
Update frame/support/src/dispatch.rs
JesseAbram 84820f7
Update frame/support/src/dispatch.rs
JesseAbram 4933986
Update frame/support/src/dispatch.rs
JesseAbram 45fb696
compile errors for on_idle in dispatch
JesseAbram c62f496
Update frame/support/src/dispatch.rs
JesseAbram 5f54ce2
Update frame/support/src/dispatch.rs
JesseAbram 39ba7a9
Update frame/support/src/dispatch.rs
JesseAbram 5e0e9d2
on idle tuple clean up
JesseAbram 423d5bf
register reduced weight
JesseAbram faeb2ce
collect and add reduced wait from on idle call
JesseAbram fbc2947
better demo example
JesseAbram 4c49899
Update frame/support/procedural/src/pallet/expand/hooks.rs
JesseAbram d36b806
added tests to dispatch.rs
JesseAbram 188bbdf
idle test on executive
JesseAbram a05b383
skip on idle if remaining weight is 0
JesseAbram fc8ba2b
Update frame/executive/src/lib.rs
JesseAbram ebb2d98
Update frame/support/src/dispatch.rs
JesseAbram 3501d4d
abstract common logic out to functions
JesseAbram 9e03788
docs
JesseAbram 7f404ca
remove demo example
JesseAbram 4e03e2c
remove debug
JesseAbram 3c30ff3
spacing
JesseAbram 3c76f0b
docs
JesseAbram b9f7bdf
revert template pallet to master
JesseAbram ae9a00d
Merge branch 'master' of github.com:paritytech/substrate into jesse-o…
JesseAbram 84ead6f
change reduced weight to used weight
JesseAbram 622fe37
remove empty line
apopiak 7ae5ad6
lint
JesseAbram d3660cd
spacing
JesseAbram 5d52fba
Update frame/support/src/traits.rs
JesseAbram ffbc58a
documentation
JesseAbram 17393fc
Update frame/support/procedural/src/pallet/expand/hooks.rs
JesseAbram 4623537
docs
JesseAbram 84c7180
Update frame/support/src/traits.rs
JesseAbram 9386c71
docs
JesseAbram 02a42e3
Update frame/support/src/traits.rs
JesseAbram b4061e4
Update frame/support/src/traits.rs
JesseAbram 855413b
Update frame/support/src/traits.rs
JesseAbram 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
This file contains hidden or 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 |
|---|---|---|
|
|
@@ -119,7 +119,7 @@ | |
| use sp_std::{prelude::*, marker::PhantomData}; | ||
| use frame_support::{ | ||
| weights::{GetDispatchInfo, DispatchInfo, DispatchClass}, | ||
| traits::{OnInitialize, OnFinalize, OnRuntimeUpgrade, OffchainWorker, ExecuteBlock}, | ||
| traits::{OnInitialize, OnIdle, OnFinalize, OnRuntimeUpgrade, OffchainWorker, ExecuteBlock}, | ||
| dispatch::PostDispatchInfo, | ||
| }; | ||
| use sp_runtime::{ | ||
|
|
@@ -160,6 +160,7 @@ impl< | |
| AllModules: | ||
| OnRuntimeUpgrade + | ||
| OnInitialize<System::BlockNumber> + | ||
| OnIdle<System::BlockNumber> + | ||
| OnFinalize<System::BlockNumber> + | ||
| OffchainWorker<System::BlockNumber>, | ||
| COnRuntimeUpgrade: OnRuntimeUpgrade, | ||
|
|
@@ -186,6 +187,7 @@ impl< | |
| UnsignedValidator, | ||
| AllModules: OnRuntimeUpgrade | ||
| + OnInitialize<System::BlockNumber> | ||
| + OnIdle<System::BlockNumber> | ||
| + OnFinalize<System::BlockNumber> | ||
| + OffchainWorker<System::BlockNumber>, | ||
| COnRuntimeUpgrade: OnRuntimeUpgrade, | ||
|
|
@@ -349,8 +351,8 @@ where | |
|
|
||
| // post-extrinsics book-keeping | ||
| <frame_system::Module<System>>::note_finished_extrinsics(); | ||
| <frame_system::Module<System> as OnFinalize<System::BlockNumber>>::on_finalize(block_number); | ||
| <AllModules as OnFinalize<System::BlockNumber>>::on_finalize(block_number); | ||
|
|
||
| Self::idle_and_finalize_hook(block_number); | ||
| } | ||
|
|
||
| /// Finalize the block - it is up the caller to ensure that all header fields are valid | ||
|
|
@@ -360,12 +362,36 @@ where | |
| sp_tracing::enter_span!( sp_tracing::Level::TRACE, "finalize_block" ); | ||
| <frame_system::Module<System>>::note_finished_extrinsics(); | ||
| let block_number = <frame_system::Module<System>>::block_number(); | ||
| <frame_system::Module<System> as OnFinalize<System::BlockNumber>>::on_finalize(block_number); | ||
| <AllModules as OnFinalize<System::BlockNumber>>::on_finalize(block_number); | ||
|
|
||
| Self::idle_and_finalize_hook(block_number); | ||
|
|
||
| <frame_system::Module<System>>::finalize() | ||
| } | ||
|
|
||
| fn idle_and_finalize_hook(block_number: NumberFor<Block>) { | ||
| let weight = <frame_system::Module<System>>::block_weight(); | ||
| let max_weight = <System::BlockWeights as frame_support::traits::Get<_>>::get().max_block; | ||
| let mut remaining_weight = max_weight.saturating_sub(weight.total()); | ||
|
|
||
| if remaining_weight > 0 { | ||
| let mut used_weight = | ||
| <frame_system::Module<System> as OnIdle<System::BlockNumber>>::on_idle( | ||
| block_number, | ||
| remaining_weight | ||
| ); | ||
| remaining_weight = remaining_weight.saturating_sub(used_weight); | ||
| used_weight = <AllModules as OnIdle<System::BlockNumber>>::on_idle( | ||
| block_number, | ||
| remaining_weight | ||
| ) | ||
| .saturating_add(used_weight); | ||
| <frame_system::Module::<System>>::register_extra_weight_unchecked(used_weight, DispatchClass::Mandatory); | ||
| } | ||
|
|
||
JesseAbram marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| <frame_system::Module<System> as OnFinalize<System::BlockNumber>>::on_finalize(block_number); | ||
| <AllModules as OnFinalize<System::BlockNumber>>::on_finalize(block_number); | ||
| } | ||
|
|
||
| /// Apply extrinsic outside of the block execution function. | ||
| /// | ||
| /// This doesn't attempt to validate anything regarding the block, but it builds a list of uxt | ||
|
|
@@ -555,6 +581,11 @@ mod tests { | |
| 175 | ||
| } | ||
|
|
||
| fn on_idle(n: T::BlockNumber, remaining_weight: Weight) -> Weight { | ||
| println!("on_idle{}, {})", n, remaining_weight); | ||
| 175 | ||
| } | ||
|
|
||
| fn on_finalize() { | ||
| println!("on_finalize(?)"); | ||
| } | ||
|
|
@@ -769,7 +800,7 @@ mod tests { | |
| header: Header { | ||
| parent_hash: [69u8; 32].into(), | ||
| number: 1, | ||
| state_root: hex!("2c01e6f33d595793119823478b45b36978a8f65a731b5ae3fdfb6330b4cd4b11").into(), | ||
| state_root: hex!("6e70de4fa07bac443dc7f8a812c8a0c941aacfa892bb373c5899f7d511d4c25b").into(), | ||
apopiak marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| extrinsics_root: hex!("03170a2e7597b7b7e3d84c05391d139a62b157e78786d8c082f29dcf4c111314").into(), | ||
| digest: Digest { logs: vec![], }, | ||
| }, | ||
|
|
@@ -1006,10 +1037,11 @@ mod tests { | |
| new_test_ext(1).execute_with(|| { | ||
|
|
||
| Executive::initialize_block(&Header::new_from_number(1)); | ||
| Executive::finalize_block(); | ||
| // NOTE: might need updates over time if new weights are introduced. | ||
| // For now it only accounts for the base block execution weight and | ||
| // the `on_initialize` weight defined in the custom test module. | ||
| assert_eq!(<frame_system::Module<Runtime>>::block_weight().total(), 175 + 10); | ||
| assert_eq!(<frame_system::Module<Runtime>>::block_weight().total(), 175 + 175 + 10); | ||
|
Member
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. seems like you should create some new tests for this new functionality
Contributor
Author
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. hmmm so the test is named block_hooks_weight_is_stored and the note indicates that if new weights are introduced it should be added here (that is how I read it, but it would seem as if you wrote the note so if I mis-interpreted it no worries) . To me that made sense to put it in there, but if you think it shouldn't be I am happy to make a new test for it |
||
| }) | ||
| } | ||
|
|
||
|
|
||
This file contains hidden or 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.
it is a little strange to me you have coupled these two together
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.
ya, at first there were just two lines of duplicate code, then after adding the hook, there was 8, so I thought it was prudent to handle all of them. I was thinking ok both hooks, they can go in the same function block. If you feel strongly that I should decouple them, I can make an on idle function and on finalize one.