Make all cross pallet extrinsics transactional

[bedeho]

Background

Substrate now supports transactional macro, which means any side-effect in the extrinsic is rolled back if it results in an Err. Read more here:

https://substrate.stackexchange.com/questions/1184/whats-the-overhead-associated-with-the-transactional-macro

The computational overhead is no 0, but its quite low. This is a very substantial change, and deeply impacts how one can write extrinsics: in a more care free way without all the validation up front. I don't think we should abandon our prior methodology of having all verifications up front though, because it really encourages clarity of thinking. However, we have some very sensitive cases in our runtime where I think adding a tranactional protection can really be a good long term protection against errors: cross pallet calls.

Proposal

Identify all cross pallet calls, and make all such extrinsics transactional. This may require benchmarking, as transactionality has some performance implication.

┆Issue is synchronized with this Asana task by Unito

[ignazio-bovo]

Is the default behavior in Substrate v3 now: paritytech/substrate#11431
What we can do instead is:

• remove all storage related checks from content made to ensure that content method won't fail
• add some tests (unit or integration) to check that all cross pallet call scenarios behave as they should

[bedeho]

Ok, that is great, but let me ask, if we remove the storage checks, what do we do about the mutation safety barrier? it will be quite misleading I think. Either way, can you make a new issue with this idea, which includes an explicit list of extrinsics, and which also explicitly states that the work is done after benchmark is done.

[bedeho]

Is already the case by default in the new version of Substrate.

[ignazio-bovo]

Is already the case by default in the new version of Substrate.

yes