There is no module hook after inherents and before transactions

[coriolinus]

The concrete problem which triggered this issue is discussed here. The general form is that it is very common for a module to use an inherent to set some data [citation needed], but that data isn't yet available at on_initialize(); because it is executed first, that function will refer to the previous block's inherent data.

In other words, we can model the sequence of calls as they currently exist like this:

• system_initialize
• on_initialize
• process all inherents
• process all transactions

There are a number of ways we could improve this situation; this issue is designed to gather opinions about which approach would be best, and whether any of them have subtle pitfalls.

Just reorder the sequence

In this approach, we just organize the sequence differently:

• system_initialize
• process all inherents
• on_initialize
• process all transactions

We'd then want to add documentation to the effect that while inherents are good for injecting data into the state, one should avoid calling into different pallets within the inherent.

I do not know if this change in behavior would break any existing pallets, but I believe it has the potential to.

Add a new hook after the inherents are processed

In this approach, we add a new special hook to decl_module: on_inherents_complete, or an appropriately bikeshedded variation thereof. The sequence of calls would look like this:

• system_initialize
• on_initialize
• process all inherents
• on_inherents_complete
• process all transactions

As this adds the potential for new behavior, but does not change existing behavior, no current pallets should break.

Both

In this approach, we reorder the sequence and add a new hook where on_inherents used to be, just in case any pallet really does need to do something before any inherents are processed. This involves adding a new special hook to decl_module: pre_initialize, or some appropriately bikeshedded variation thereof. The sequence of calls would look like this:

• system_initialize
• pre_initialize
• process all inherents
• on_initialize
• process all transactions

This breaks all the same pallets that a simple reordering does, but also gives broken pallets the opportunity to fix things relatively simply. pre_initialize seems a less cumbersome name than on_inherents_complete.

Neither

The best, fastest, must bug-free code is the code that does not exist. Maybe there already exists some way to work around this, so no changes are necessary.

Something Else

I don't know what other options are out there which have just not yet appeared in my vision of the possibility space; new ideas are welcome.

[bkchr]

I talked with @gavofyork and he proposed that we just move the logic from on_initialize to the inherent call. That will solve our problem as well and doesn't require any changes in Substrate ;)

[coriolinus]

It does seem plausible that that approach will work to solve the immediate problem. Two tests broke after making the change, and I am still investigating, but I suspect that the problem is mostly that the test suite is not yet constructing any inherents.

However, what that change does is move initialization logic from on_initialize to a function much less obvious. It reduces maintainability. In my opinion, a solution to the immediate problem does not negate the value of applying a fix to the general problem.

[bkchr]

Why is it less maintainable?

[coriolinus]

Discoverability. You know, and I know, that we've moved block initialization logic from on_initialize to create_inherent. Within the code, there's a comment to that effect.

However, we aren't the only people who will ever look at this code. If someone new tries to read the module, they'll read the decl_module block and note that there is no on_initialize function anymore. It would be easy for them to assume that there is no per-block initialization, and just miss that all of that happens within ProvideInherent.

[ggwpez]

after_inherents is being added here #14414, although currently not exposed to pallets or runtime.

[bkchr]

Duplicate of: paritytech/polkadot-sdk#312