Ensure inherent are first

[gui1117]

fix #6243
fix #8144

polkadot companion: paritytech/polkadot#2560

Breaking change:

• execute_block now checks that inherents are first.
• inherent is any extrinsics, not signed and accepted by one module as inherent (ProvideInherent::is_inherent(call) == true)
• module only check inherent for which <$module as ProvideInherent>::is_inherent(call) == true
• any validate unsigned implementation which accept an inherent call is invalid as block producer could include them after some non inherents.

Description:

We basically introduce ProvideInherent::is_inherent(call) function and make use of it in impl_outer_inherent.

The implementation consider a signed extrinsic to be not an inherent.
So we can accept calls where ProvideInherent::is_inherent(call) == true and are signed. Those are not inherent and can be valid calls.
(note: we could refuse those, by adding some checker in extra_signed)

We also don't ensure that inherent call are successful.
And we don't ensure that inherent call are unique.

All we do is:

• we consider unsigned extrinsic with call where ProvideInherent::is_inherent(call) == true to be inherent
• inherent are first
• module which requires inherent have at least one inherent (thus unsigned) for which <$module as ProvideInherent>::is_inherent(call) is true.

[gui1117]

I think this cannot be added inside Extrinsic easily (because OpaqueExtrinsic for instance implements it), thus I extracted it.

[kianenigma]

Looks sensible to me. Maybe I would call it ExtractCall or CallOfExtrinsic, but looks okay.

[shawntabrizi]

My general comment is that the inherent / unsigned story I think is not super clean.

It seems that the difference between an inherent tx and an unsigned tx now is whether the module remembers to write the function is_inherent.

Maybe this is as good as it gets, but i would actually expect something a little more clean, and without chance of forgetting to implement such a fine detail.

[bkchr]

My general comment is that the inherent / unsigned story I think is not super clean.

It seems that the difference between an inherent tx and an unsigned tx now is whether the module remembers to write the function is_inherent.

Maybe this is as good as it gets, but i would actually expect something a little more clean, and without chance of forgetting to implement such a fine detail.

The compiler forces you to implement is_inherent, as the trait ProvideInherent requires this :P

[gui1117]

My general comment is that the inherent / unsigned story I think is not super clean.

It seems that the difference between an inherent tx and an unsigned tx now is whether the module remembers to write the function is_inherent.

Maybe this is as good as it gets, but i would actually expect something a little more clean, and without chance of forgetting to implement such a fine detail.

Yes, I also think it needs some documentation somewhere but this is the summary:

• an inherent is an unsigned extrinsic, whose call is marked as inherent by one pallet.
• non-inherent unsigned extrinsics are unsigned transactions.
• signed extrinsic are signed transactions and not inherent.

No inherent should be accepted in ValidateUnsigned::validate_unsigned, offchain worker must not send inherent too. Otherwise the block producer can include these extrinsics after some non-inherent and the block is invalid.
Though inherent must be accepted in ValidateUnsigned::pre_dispatch.

Maybe we can improve on this with some macro attributes on calls themself so that ppl can't mess themself.

[kianenigma]

audit? burnin? anything :P

[gui1117]

there is no client change, so no burnin.
I guess audit is required considering it is not trivial.

[shawntabrizi]

@thiolliere Can you get merge-able, and then it seems we are good to go

[gui1117]

bot merge

[ghost]

Trying merge.

[tdelabro]

Hey,
I would like to have internals executed after transactions. Around block finalization. How can I achieve this?