Update Scheduler Pallet Documentation (#14740)

* Update pallet scheduler documentation, warning section, guidelines update

* Update call filter note

Co-authored-by: Kelvin Bonilla <bonilla_kelvin@hotmail.com>

* revert format cargo

* Doc wording

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

* Doc wording

Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

* Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>

Ammend comments related to documentation

* Include additional warning section in `on_initialize` hook

* Amend doc

Co-authored-by: Sam Johnson <sam@durosoft.com>

* Amend doc

Co-authored-by: Sam Johnson <sam@durosoft.com>

* Move no_std to appropriate place

* Amend doc

Co-authored-by: Nate Armstrong <naterarmstrong@gmail.com>

* Amend comment

Co-authored-by: Nate Armstrong <naterarmstrong@gmail.com>

---------

Co-authored-by: Kelvin Bonilla <bonilla_kelvin@hotmail.com>
Co-authored-by: Keith Yeung <kungfukeith11@gmail.com>
Co-authored-by: Sam Johnson <sam@durosoft.com>
Co-authored-by: Nate Armstrong <naterarmstrong@gmail.com>

substrate/Cargo.lock

@@ -6996,6 +6996,7 @@ dependencies = [
 name = "pallet-scheduler"
 version = "4.0.0-dev"
 dependencies = [
+ "docify",
  "frame-benchmarking",
  "frame-support",
  "frame-system",

substrate/frame/scheduler/Cargo.toml

@@ -20,6 +20,7 @@ sp-io = { version = "23.0.0", default-features = false, path = "../../primitives
 sp-runtime = { version = "24.0.0", default-features = false, path = "../../primitives/runtime" }
 sp-std = { version = "8.0.0", default-features = false, path = "../../primitives/std" }
 sp-weights = { version = "20.0.0", default-features = false, path = "../../primitives/weights" }
+docify = "0.2.1"
 
 [dev-dependencies]
 pallet-preimage = { version = "4.0.0-dev", path = "../preimage" }

substrate/frame/scheduler/src/lib.rs

@@ -15,37 +15,63 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-//! # Scheduler
+//! > Made with *Substrate*, for *Polkadot*.
-//! A Pallet for scheduling dispatches.
 //!
-//! - [`Config`]
+//! [![github]](https://github.com/paritytech/substrate/frame/fast-unstake) -
-//! - [`Call`]
+//! [![polkadot]](https://polkadot.network)
-//! - [`Pallet`]
+//!
+//! [polkadot]: https://img.shields.io/badge/polkadot-E6007A?style=for-the-badge&logo=polkadot&logoColor=white
+//! [github]: https://img.shields.io/badge/github-8da0cb?style=for-the-badge&labelColor=555555&logo=github
+//!
+//! # Scheduler Pallet
+//!
+//! A Pallet for scheduling runtime calls.
 //!
 //! ## Overview
 //!
-//! This Pallet exposes capabilities for scheduling dispatches to occur at a
+//! This Pallet exposes capabilities for scheduling runtime calls to occur at a specified block
-//! specified block number or at a specified period. These scheduled dispatches
+//! number or at a specified period. These scheduled runtime calls may be named or anonymous and may
-//! may be named or anonymous and may be canceled.
+//! be canceled.
 //!
-//! **NOTE:** The scheduled calls will be dispatched with the default filter
+//! __NOTE:__ Instead of using the filter contained in the origin to call `fn schedule`, scheduled
-//! for the origin: namely `frame_system::Config::BaseCallFilter` for all origin
+//! runtime calls will be dispatched with the default filter for the origin: namely
-//! except root which will get no filter. And not the filter contained in origin
+//! `frame_system::Config::BaseCallFilter` for all origin types (except root which will get no
-//! use to call `fn schedule`.
+//! filter).
 //!
-//! If a call is scheduled using proxy or whatever mecanism which adds filter,
+//! If a call is scheduled using proxy or whatever mechanism which adds filter, then those filter
-//! then those filter will not be used when dispatching the schedule call.
+//! will not be used when dispatching the schedule runtime call.
 //!
-//! ## Interface
+//! ### Examples
 //!
-//! ### Dispatchable Functions
+//! 1. Scheduling a runtime call at a specific block.
+#![doc = docify::embed!("src/tests.rs", basic_scheduling_works)]
 //!
-//! * `schedule` - schedule a dispatch, which may be periodic, to occur at a specified block and
+//! 2. Scheduling a preimage hash of a runtime call at a specifc block
-//!   with a specified priority.
+#![doc = docify::embed!("src/tests.rs", scheduling_with_preimages_works)]
-//! * `cancel` - cancel a scheduled dispatch, specified by block number and index.
+
-//! * `schedule_named` - augments the `schedule` interface with an additional `Vec<u8>` parameter
+//!
-//!   that can be used for identification.
+//! ## Pallet API
-//! * `cancel_named` - the named complement to the cancel function.
+//!
+//! See the [`pallet`] module for more information about the interfaces this pallet exposes,
+//! including its configuration trait, dispatchables, storage items, events and errors.
+//!
+//! ## Warning
+//!
+//! This Pallet executes all scheduled runtime calls in the [`on_initialize`] hook. Do not execute
+//! any runtime calls which should not be considered mandatory.
+//!
+//! Please be aware that any scheduled runtime calls executed in a future block may __fail__ or may
+//! result in __undefined behavior__ since the runtime could have upgraded between the time of
+//! scheduling and execution. For example, the runtime upgrade could have:
+//!
+//! * Modified the implementation of the runtime call (runtime specification upgrade).
+//!     * Could lead to undefined behavior.
+//! * Removed or changed the ordering/index of the runtime call.
+//!     * Could fail due to the runtime call index not being part of the `Call`.
+//!     * Could lead to undefined behavior, such as executing another runtime call with the same
+//!       index.
+//!
+//! [`on_initialize`]: frame_support::traits::Hooks::on_initialize
 
 // Ensure we're `no_std` when compiling for Wasm.
 #![cfg_attr(not(feature = "std"), no_std)]

substrate/frame/scheduler/src/tests.rs

@@ -30,11 +30,18 @@ use sp_runtime::traits::Hash;
 use substrate_test_utils::assert_eq_uvec;
 
 #[test]
+#[docify::export]
 fn basic_scheduling_works() {
 	new_test_ext().execute_with(|| {
+		// Call to schedule
 		let call =
 			RuntimeCall::Logger(LoggerCall::log { i: 42, weight: Weight::from_parts(10, 0) });
+
+		// BaseCallFilter should be implemented to accept `Logger::log` runtime call which is
+		// implemented for `BaseFilter` in the mock runtime
 		assert!(!<Test as frame_system::Config>::BaseCallFilter::contains(&call));
+
+		// Schedule call to be executed at the 4th block
 		assert_ok!(Scheduler::do_schedule(
 			DispatchTime::At(4),
 			None,
@@ -42,33 +49,53 @@ fn basic_scheduling_works() {
 			root(),
 			Preimage::bound(call).unwrap()
 		));
+
+		// `log` runtime call should not have executed yet
 		run_to_block(3);
 		assert!(logger::log().is_empty());
+
 		run_to_block(4);
+		// `log` runtime call should have executed at block 4
 		assert_eq!(logger::log(), vec![(root(), 42u32)]);
+
 		run_to_block(100);
 		assert_eq!(logger::log(), vec![(root(), 42u32)]);
 	});
 }
 
 #[test]
+#[docify::export]
 fn scheduling_with_preimages_works() {
 	new_test_ext().execute_with(|| {
+		// Call to schedule
 		let call =
 			RuntimeCall::Logger(LoggerCall::log { i: 42, weight: Weight::from_parts(10, 0) });
+
 		let hash = <Test as frame_system::Config>::Hashing::hash_of(&call);
 		let len = call.using_encoded(|x| x.len()) as u32;
-		// Important to use here `Bounded::Lookup` to ensure that we request the hash.
+
+		// Important to use here `Bounded::Lookup` to ensure that that the Scheduler can request the
+		// hash from PreImage to dispatch the call
 		let hashed = Bounded::Lookup { hash, len };
+
+		// Schedule call to be executed at block 4 with the PreImage hash
 		assert_ok!(Scheduler::do_schedule(DispatchTime::At(4), None, 127, root(), hashed));
+
+		// Register preimage on chain
 		assert_ok!(Preimage::note_preimage(RuntimeOrigin::signed(0), call.encode()));
 		assert!(Preimage::is_requested(&hash));
+
+		// `log` runtime call should not have executed yet
 		run_to_block(3);
 		assert!(logger::log().is_empty());
+
 		run_to_block(4);
+		// preimage should not have been removed when executed by the scheduler
 		assert!(!Preimage::len(&hash).is_some());
 		assert!(!Preimage::is_requested(&hash));
+		// `log` runtime call should have executed at block 4
 		assert_eq!(logger::log(), vec![(root(), 42u32)]);
+
 		run_to_block(100);
 		assert_eq!(logger::log(), vec![(root(), 42u32)]);
 	});

substrate/frame/support/src/traits/hooks.rs

@@ -270,9 +270,14 @@ pub trait Hooks<BlockNumber> {
 	/// Must return the non-negotiable weight of both itself and whatever [`Hooks::on_finalize`]
 	/// wishes to consume.
 	///
+	/// ## Warning
+	///
 	/// The weight returned by this is treated as `DispatchClass::Mandatory`, meaning that
 	/// it MUST BE EXECUTED. If this is not the case, consider using [`Hooks::on_idle`] instead.
 	///
+	/// Try to keep any arbitrary execution __deterministic__ and within __minimal__ time
+	/// complexity. For example, do not execute any unbounded iterations.
+	///
 	/// NOTE: This function is called BEFORE ANY extrinsic in a block is applied, including inherent
 	/// extrinsics. Hence for instance, if you runtime includes `pallet-timestamp`, the `timestamp`
 	/// is not yet up to date at this point.