add notes and warnings to ProvideInherent docs (paritytech#9730)

* add notes and warnings to ProvideInherent docs

* rephrase ProvideInherent doc comments

* more comment refinement

* remove multiple inherents note

* remove repetition

Co-authored-by: Bastian Köcher <[email protected]>

* replace inherent example in docs

* add note about who checks is_inherent_required

* Apply suggestions from code review

Co-authored-by: Bastian Köcher <[email protected]>

Co-authored-by: Bastian Köcher <[email protected]>

Author: 2 people

frame/support/src/inherent.rs

@@ -24,9 +24,12 @@ pub use sp_inherents::{
 	CheckInherentsResult, InherentData, InherentIdentifier, IsFatalError, MakeFatalError,
 };
 
-/// A pallet that provides or verifies an inherent extrinsic.
+/// A pallet that provides or verifies an inherent extrinsic will implement this trait.
 ///
-/// The pallet may provide the inherent, verify an inherent, or both provide and verify.
+/// The pallet may provide an inherent, verify an inherent, or both provide and verify.
+///
+/// Briefly, inherent extrinsics ("inherents") are extrinsics that are added to a block by the block
+/// producer. See [`sp_inherents`] for more documentation on inherents.
 pub trait ProvideInherent {
 	/// The call type of the pallet.
 	type Call;
@@ -36,6 +39,12 @@ pub trait ProvideInherent {
 	const INHERENT_IDENTIFIER: self::InherentIdentifier;
 
 	/// Create an inherent out of the given `InherentData`.
+	///
+	/// NOTE: All checks necessary to ensure that the inherent is correct and that can be done in
+	/// the runtime should happen in the returned `Call`.
+	/// E.g. if this provides the timestamp, the call will check that the given timestamp is
+	/// increasing the old timestamp by more than a minimum and it will also check that the
+	/// timestamp hasn't already been set in the current block.
 	fn create_inherent(data: &InherentData) -> Option<Self::Call>;
 
 	/// Determines whether this inherent is required in this block.
@@ -44,15 +53,17 @@ pub trait ProvideInherent {
 	/// implementation returns this.
 	///
 	/// - `Ok(Some(e))` indicates that this inherent is required in this block. `construct_runtime!`
-	/// will call this function from in its implementation of `fn check_extrinsics`.
+	/// will call this function in its implementation of `fn check_extrinsics`.
 	/// If the inherent is not present, it will return `e`.
 	///
 	/// - `Err(_)` indicates that this function failed and further operations should be aborted.
 	///
-	/// NOTE: If inherent is required then the runtime asserts that the block contains at least
+	/// NOTE: If the inherent is required then the runtime asserts that the block contains at least
 	/// one inherent for which:
 	/// * type is [`Self::Call`],
 	/// * [`Self::is_inherent`] returns true.
+	///
+	/// NOTE: This is currently only checked by block producers, not all full nodes.
 	fn is_inherent_required(_: &InherentData) -> Result<Option<Self::Error>, Self::Error> {
 		Ok(None)
 	}
@@ -64,21 +75,27 @@ pub trait ProvideInherent {
 	/// included in the block by its author. Whereas the second parameter represents the inherent
 	/// data that the verifying node calculates.
 	///
-	/// NOTE: A block can contains multiple inherent.
+	/// This is intended to allow for checks that cannot be done within the runtime such as, e.g.,
+	/// the timestamp.
+	///
+	/// # Warning
+	///
+	/// This check is not guaranteed to be run by all full nodes and cannot be relied upon for
+	/// ensuring that the block is correct.
 	fn check_inherent(_: &Self::Call, _: &InherentData) -> Result<(), Self::Error> {
 		Ok(())
 	}
 
 	/// Return whether the call is an inherent call.
 	///
-	/// NOTE: Signed extrinsics are not inherent, but signed extrinsic with the given call variant
-	/// can be dispatched.
+	/// NOTE: Signed extrinsics are not inherents, but a signed extrinsic with the given call
+	/// variant can be dispatched.
 	///
 	/// # Warning
 	///
-	/// In FRAME, inherent are enforced to be before other extrinsics, for this reason,
+	/// In FRAME, inherents are enforced to be executed before other extrinsics. For this reason,
 	/// pallets with unsigned transactions **must ensure** that no unsigned transaction call
 	/// is an inherent call, when implementing `ValidateUnsigned::validate_unsigned`.
-	/// Otherwise block producer can produce invalid blocks by including them after non inherent.
+	/// Otherwise block producers can produce invalid blocks by including them after non inherents.
 	fn is_inherent(call: &Self::Call) -> bool;
 }

primitives/inherents/src/lib.rs

@@ -15,14 +15,14 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-//! Substrate inherent extrinsics
+//! Substrate Inherent Extrinsics
 //!
 //! Inherent extrinsics are extrinsics that are inherently added to each block. However, it is up to
-//! runtime implementation to require an inherent for each block or to make it optional. Inherents
-//! are mainly used to pass data from the block producer to the runtime. So, inherents require some
-//! part that is running on the client side and some part that is running on the runtime side. Any
-//! data that is required by an inherent is passed as [`InherentData`] from the client to the
-//! runtime when the inherents are constructed.
+//! the runtime implementation to require an inherent for each block or to make it optional.
+//! Inherents are mainly used to pass data from the block producer to the runtime. So, inherents
+//! require some part that is running on the client side and some part that is running on the
+//! runtime side. Any data that is required by an inherent is passed as [`InherentData`] from the
+//! client to the runtime when the inherents are constructed.
 //!
 //! The process of constructing and applying inherents is the following:
 //!