Better handling of error in inherents logic.

[gui1117]

Currently the code generated by construct_runtime for the check of inherent have a expect with a quite light proof.

In case multiple non fatal error are reported for an inherent the proof is not accurate.
This code makes a more explicit handling of the error and handle this case more appropriately.

IMO we should modify more deeply the inherents logic so that CheckInherentResult could handle multiple non fatal error for an inherent. Or only allow one error by inherent. And maybe remove non fatal error if it is considered superfluous.

But prior to a more consequent refactor to be discussed I think it is better to have an accurate handling of the result from put_error.

[kianenigma]

I would suggest that we move the if fata_error { return } part also to a helper function as this is the part that is still important and ensures we bail after seeing one error.

[gui1117]

I'm not exactly sure how to factorise it out. something like this?

@@ -138,10 +138,12 @@ pub fn expand_outer_inherent(
                                                                if #pallet_names::is_inherent(call) {
                                                                        is_inherent = true;
                                                                        if let Err(e) = #pallet_names::check_inherent(call, self) {
-                                                                               handle_put_error_result(result.put_error(
-                                                                                       #pallet_names::INHERENT_IDENTIFIER, &e
+                                                                               let is_fatal = put_error_and_is_error_fatal(
+                                                                                       result,
+                                                                                       #pallet_names::INHERENT_IDENTIFIER,
+                                                                                       &e
                                                                                ));
-                                                                               if e.is_fatal_error() {
+                                                                               if is_fatal {
                                                                                        return result;
                                                                                }
                                                                        }

[kianenigma]

Could use defensive macro here instead, as we still know these should (almost) never happen.

[gui1117]

but defensive doesn't support renaming of frame_support contrary to construct_runtime.

(by renaming mean if somebody rename in its Cargo.toml like this:

s = { version = "*", package = "frame-support" }

)

[gui1117]

error variant are different when sp-inherents is compiled with std or not.
but some test crate use construct_runtime without having a std feature. So there is no way to know what is the actual error type.

EDIT: Anyway I will do a follow up issue with proposals for improving inherent code