-
Notifications
You must be signed in to change notification settings - Fork 2.6k
There is no module hook after inherents and before transactions #5757
Comments
I talked with @gavofyork and he proposed that we just move the logic from |
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 |
Why is it less maintainable? |
Discoverability. You know, and I know, that we've moved block initialization logic from 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 |
|
Duplicate of: paritytech/polkadot-sdk#312 |
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
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
on_initialize
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
on_inherents_complete
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 todecl_module
:pre_initialize
, or some appropriately bikeshedded variation thereof. The sequence of calls would look like this:system_initialize
pre_initialize
on_initialize
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 thanon_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.
The text was updated successfully, but these errors were encountered: