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.

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

[FRAME] Execution phases ApplyInherent and AfterInherent #3666

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

ggwpez
Copy link
Member

@ggwpez ggwpez commented Mar 12, 2024

Follow up on #1781

Changes:

  • Add two new execution phases: ApplyInherent(u32) and AfterInherent.
  • Function System::note_after_inherents_done to be called by Executive after running all after_inherents logic.
  • Tests for the on_poll hook.
  • Changing the phase of things that are done in pre_inherents into the Initialization phase instead of ApplyExtrinsic.

@ggwpez ggwpez added the T1-FRAME This PR/Issue is related to core FRAME, the framework. label Mar 12, 2024
@ggwpez ggwpez self-assigned this Mar 12, 2024
@ggwpez ggwpez force-pushed the oty-inherent-execution-phase branch from 998d9f8 to 8e0bca6 Compare March 12, 2024 15:31
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@ggwpez ggwpez marked this pull request as ready for review April 9, 2024 15:10
@ggwpez ggwpez requested a review from a team as a code owner April 9, 2024 15:10
ggwpez added 2 commits April 9, 2024 17:15
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
Signed-off-by: Oliver Tale-Yazdi <[email protected]>
@@ -601,8 +601,8 @@ where
DispatchClass::Mandatory,
);

frame_system::Pallet::<System>::note_finished_initialize();
<System as frame_system::Config>::PreInherents::pre_inherents();
Copy link
Contributor

Choose a reason for hiding this comment

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

here you wanna have it within Initialization phase?

Copy link
Member

Choose a reason for hiding this comment

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

Yeah maybe some extra PreInherent phase? Similar to the AfterInherent one.

Copy link
Member

@bkchr bkchr left a comment

Choose a reason for hiding this comment

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

Maybe adding one more phase PreInherent would be a good idea, otherwise it looks good to me.

@@ -601,8 +601,8 @@ where
DispatchClass::Mandatory,
);

frame_system::Pallet::<System>::note_finished_initialize();
<System as frame_system::Config>::PreInherents::pre_inherents();
Copy link
Member

Choose a reason for hiding this comment

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

Yeah maybe some extra PreInherent phase? Similar to the AfterInherent one.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T1-FRAME This PR/Issue is related to core FRAME, the framework.
Projects
Status: In Progress
Development

Successfully merging this pull request may close these issues.

4 participants