Deprecate `ValidateUnsigned` and `#[validate_unsigned]`

[ggwpez]

Part of #2415

ValidateUnsigned can be deprecated with a rust attribute and #[validate_unsigned] by emitting a proc-macro-warning.
The logic should be moved to TransactionExtension.

[philoniare]

@ggwpez can you assign this to me?

[kianenigma]

@philoniare you are free to start, would be good if you open a draft PR sooner to signal you are working on this.

[kianenigma]

FYI, this should ideally wait until #2160 is done.

[philoniare]

Thanks for the reminder, I'll just wait until #2160 is done. Once it's in, this ticket looks pretty straightforward.

[kianenigma]

Note that the above PR is merged, but reverted and is now tracked in #3685

[RomarQ]

Working on it here: #10150

[RomarQ]

From a discussion with @gui1117 in #10150, there is a question about if we really want to deprecate ValidateUnsigned.

What do you guys think?

@bkchr, maybe you also have an opinion about this?

[gui1117]

I didn't see a strong reason to remove validate unsigned, there was one reason in the non-fork aware transaction pool, but the fork-aware one don't have the issue (it was about forwarding extrinsics for uncle blocks, while avoiding forwarding inherents).

But also I don't like that by default unsigned transactions are accepted in pre-dispatch, this is made to allow inherents, it would have been better to just explicitly accept inherents in the first place, this would have prevented the issue with utility::batch accepting any origin that could be used to dispatch any transaction with the origin None and thus bypass the checks.
We should not allow any transaction to dispatch with None in pre-dispatch by default, but it is hard to fix it now.

So deprecating validate unsigned could help us fix the difficult relation between unsigned and inherent in FRAME, maybe it is still worth pushing it but I am not so sure.

As a final note I think the concept of inherent doesn't make sense for parachain, because the consensus is outsourced to the relay chain. We use it for parachain as a way to submit transaction just before producing a block, it should be renamed to something like pre_block_offchain_worker.

[bkchr]

I'm supporting to remove ValidateUnsigned. It was a mistake from the beginning, that we never have reverted. Having two ways for checking unsigned transactions is just causing more confusion than what it brings.

But also I don't like that by default unsigned transactions are accepted in pre-dispatch, this is made to allow inherents, it would have been better to just explicitly accept inherents in the first place, this would have prevented the issue with

I need to check again the extrinsic version 5, but there we have a clearer separation? We could also think to pass in some RawOrigin::Inherent as start origin when verifying an inherent. This way it is more clear that we are talking about an inherent.

[gui1117]

Ok, let’s deprecate and remove it

I need to check again the extrinsic version 5, but there we have a clearer separation? We could also think to pass in some RawOrigin::Inherent as start origin when verifying an inherent. This way it is more clear that we are talking about an inherent.

The clear separation is just we decided we will use Bare extrinsics exclusively for inherents.