Allow Staking tests to run with session length other than 1 (#7719)

* fix periodic session

* Allow staking tests to run with session lengths other than 1.

* Update frame/staking/src/mock.rs

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* Fix all tests with session length 5.

* Test for active != current

* Better doc

* Update frame/staking/src/lib.rs

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

* also set the timestamp properly.

* trigger CI

* Revert "trigger CI"

This reverts commit 0f254944cdad848aa6e63bd8a618db95447a8e68.

* Update frame/staking/src/lib.rs

Co-authored-by: Guillaume Thiolliere <gui.thiolliere@gmail.com>

Author: kianenigma

src/lib.rs

@@ -17,8 +17,8 @@
 
 //! # Session Module
 //!
-//! The Session module allows validators to manage their session keys, provides a function for changing
-//! the session length, and handles session rotation.
+//! The Session module allows validators to manage their session keys, provides a function for
+//! changing the session length, and handles session rotation.
 //!
 //! - [`session::Config`](./trait.Config.html)
 //! - [`Call`](./enum.Call.html)
@@ -29,34 +29,39 @@
 //! ### Terminology
 //! <!-- Original author of paragraph: @gavofyork -->
 //!
-//! - **Session:** A session is a period of time that has a constant set of validators. Validators can only join
-//! or exit the validator set at a session change. It is measured in block numbers. The block where a session is
-//! ended is determined by the `ShouldEndSession` trait. When the session is ending, a new validator set
-//! can be chosen by `OnSessionEnding` implementations.
-//! - **Session key:** A session key is actually several keys kept together that provide the various signing
-//! functions required by network authorities/validators in pursuit of their duties.
-//! - **Validator ID:** Every account has an associated validator ID. For some simple staking systems, this
-//! may just be the same as the account ID. For staking systems using a stash/controller model,
-//! the validator ID would be the stash account ID of the controller.
+//! - **Session:** A session is a period of time that has a constant set of validators. Validators
+//!   can only join or exit the validator set at a session change. It is measured in block numbers.
+//!   The block where a session is ended is determined by the `ShouldEndSession` trait. When the
+//!   session is ending, a new validator set can be chosen by `OnSessionEnding` implementations.
+//!
+//! - **Session key:** A session key is actually several keys kept together that provide the various
+//!   signing functions required by network authorities/validators in pursuit of their duties.
+//! - **Validator ID:** Every account has an associated validator ID. For some simple staking
+//!   systems, this may just be the same as the account ID. For staking systems using a
+//!   stash/controller model, the validator ID would be the stash account ID of the controller.
+//!
 //! - **Session key configuration process:** Session keys are set using `set_keys` for use not in
-//! the next session, but the session after next. They are stored in `NextKeys`, a mapping between
-//! the caller's `ValidatorId` and the session keys provided. `set_keys` allows users to set their
-//! session key prior to being selected as validator.
-//! It is a public call since it uses `ensure_signed`, which checks that the origin is a signed account.
-//! As such, the account ID of the origin stored in `NextKeys` may not necessarily be associated with
-//! a block author or a validator. The session keys of accounts are removed once their account balance is zero.
+//!   the next session, but the session after next. They are stored in `NextKeys`, a mapping between
+//!   the caller's `ValidatorId` and the session keys provided. `set_keys` allows users to set their
+//!   session key prior to being selected as validator. It is a public call since it uses
+//!   `ensure_signed`, which checks that the origin is a signed account. As such, the account ID of
+//!   the origin stored in `NextKeys` may not necessarily be associated with a block author or a
+//!   validator. The session keys of accounts are removed once their account balance is zero.
+//!
 //! - **Session length:** This pallet does not assume anything about the length of each session.
-//! Rather, it relies on an implementation of `ShouldEndSession` to dictate a new session's start.
-//! This pallet provides the `PeriodicSessions` struct for simple periodic sessions.
-//! - **Session rotation configuration:** Configure as either a 'normal' (rewardable session where rewards are
-//! applied) or 'exceptional' (slashable) session rotation.
+//!   Rather, it relies on an implementation of `ShouldEndSession` to dictate a new session's start.
+//!   This pallet provides the `PeriodicSessions` struct for simple periodic sessions.
+//!
+//! - **Session rotation configuration:** Configure as either a 'normal' (rewardable session where
+//!   rewards are applied) or 'exceptional' (slashable) session rotation.
+//!
 //! - **Session rotation process:** At the beginning of each block, the `on_initialize` function
-//! queries the provided implementation of `ShouldEndSession`. If the session is to end the newly
-//! activated validator IDs and session keys are taken from storage and passed to the
-//! `SessionHandler`. The validator set supplied by `SessionManager::new_session` and the corresponding session
-//! keys, which may have been registered via `set_keys` during the previous session, are written
-//! to storage where they will wait one session before being passed to the `SessionHandler`
-//! themselves.
+//!   queries the provided implementation of `ShouldEndSession`. If the session is to end the newly
+//!   activated validator IDs and session keys are taken from storage and passed to the
+//!   `SessionHandler`. The validator set supplied by `SessionManager::new_session` and the
+//!   corresponding session keys, which may have been registered via `set_keys` during the previous
+//!   session, are written to storage where they will wait one session before being passed to the
+//!   `SessionHandler` themselves.
 //!
 //! ### Goals
 //!
@@ -75,21 +80,22 @@
 //! ### Public Functions
 //!
 //! - `rotate_session` - Change to the next session. Register the new authority set. Queue changes
-//! for next session rotation.
+//!   for next session rotation.
 //! - `disable_index` - Disable a validator by index.
 //! - `disable` - Disable a validator by Validator ID
 //!
 //! ## Usage
 //!
 //! ### Example from the FRAME
 //!
-//! The [Staking pallet](../pallet_staking/index.html) uses the Session pallet to get the validator set.
+//! The [Staking pallet](../pallet_staking/index.html) uses the Session pallet to get the validator
+//! set.
 //!
 //! ```
 //! use pallet_session as session;
 //!
 //! fn validators<T: pallet_session::Config>() -> Vec<<T as pallet_session::Config>::ValidatorId> {
-//!	<pallet_session::Module<T>>::validators()
+//! <pallet_session::Module<T>>::validators()
 //! }
 //! # fn main(){}
 //! ```
@@ -166,18 +172,18 @@ impl<
 					period.saturating_sub(block_after_last_session)
 				)
 			} else {
-				Zero::zero()
+				now
 			}
 		} else {
 			offset
 		})
 	}
 
 	fn weight(_now: BlockNumber) -> Weight {
-		// Weight note: `estimate_next_session_rotation` has no storage reads and trivial computational overhead.
-		// There should be no risk to the chain having this weight value be zero for now.
-		// However, this value of zero was not properly calculated, and so it would be reasonable
-		// to come back here and properly calculate the weight of this function.
+		// Weight note: `estimate_next_session_rotation` has no storage reads and trivial
+		// computational overhead. There should be no risk to the chain having this weight value be
+		// zero for now. However, this value of zero was not properly calculated, and so it would be
+		// reasonable to come back here and properly calculate the weight of this function.
 		0
 	}
 }
@@ -186,17 +192,17 @@ impl<
 pub trait SessionManager<ValidatorId> {
 	/// Plan a new session, and optionally provide the new validator set.
 	///
-	/// Even if the validator-set is the same as before, if any underlying economic
-	/// conditions have changed (i.e. stake-weights), the new validator set must be returned.
-	/// This is necessary for consensus engines making use of the session module to
-	/// issue a validator-set change so misbehavior can be provably associated with the new
-	/// economic conditions as opposed to the old.
-	/// The returned validator set, if any, will not be applied until `new_index`.
-	/// `new_index` is strictly greater than from previous call.
+	/// Even if the validator-set is the same as before, if any underlying economic conditions have
+	/// changed (i.e. stake-weights), the new validator set must be returned. This is necessary for
+	/// consensus engines making use of the session module to issue a validator-set change so
+	/// misbehavior can be provably associated with the new economic conditions as opposed to the
+	/// old. The returned validator set, if any, will not be applied until `new_index`. `new_index`
+	/// is strictly greater than from previous call.
 	///
 	/// The first session start at index 0.
 	///
-	/// `new_session(session)` is guaranteed to be called before `end_session(session-1)`.
+	/// `new_session(session)` is guaranteed to be called before `end_session(session-1)`. In other
+	/// words, a new session must always be planned before an ongoing one can be finished.
 	fn new_session(new_index: SessionIndex) -> Option<Vec<ValidatorId>>;
 	/// End the session.
 	///
@@ -205,7 +211,7 @@ pub trait SessionManager<ValidatorId> {
 	fn end_session(end_index: SessionIndex);
 	/// Start the session.
 	///
-	/// The session start to be used for validation
+	/// The session start to be used for validation.
 	fn start_session(start_index: SessionIndex);
 }
 
@@ -242,7 +248,7 @@ pub trait SessionHandler<ValidatorId> {
 
 	/// A notification for end of the session.
 	///
-	/// Note it is triggered before any `SessionManager::end_session` handlers,
+	/// Note it is triggered before any [`SessionManager::end_session`] handlers,
 	/// so we can still affect the validator set.
 	fn on_before_session_ending() {}
 

src/tests.rs

@@ -252,31 +252,32 @@ fn session_changed_flag_works() {
 
 #[test]
 fn periodic_session_works() {
-	struct Period;
-	struct Offset;
 
-	impl Get<u64> for Period {
-		fn get() -> u64 { 10 }
+	frame_support::parameter_types! {
+		const Period: u64 = 10;
+		const Offset: u64 = 3;
 	}
 
-	impl Get<u64> for Offset {
-		fn get() -> u64 { 3 }
-	}
-
-
 	type P = PeriodicSessions<Period, Offset>;
 
-	for i in 0..3 {
+	for i in 0u64..3 {
 		assert!(!P::should_end_session(i));
+		assert_eq!(P::estimate_next_session_rotation(i).unwrap(), 3);
 	}
 
-	assert!(P::should_end_session(3));
+	assert!(P::should_end_session(3u64));
+	assert_eq!(P::estimate_next_session_rotation(3u64).unwrap(), 3);
 
-	for i in (1..10).map(|i| 3 + i) {
+	for i in (1u64..10).map(|i| 3 + i) {
 		assert!(!P::should_end_session(i));
+		assert_eq!(P::estimate_next_session_rotation(i).unwrap(), 13);
 	}
 
-	assert!(P::should_end_session(13));
+	assert!(P::should_end_session(13u64));
+	assert_eq!(P::estimate_next_session_rotation(13u64).unwrap(), 13);
+
+	assert!(!P::should_end_session(14u64));
+	assert_eq!(P::estimate_next_session_rotation(14u64).unwrap(), 23);
 }
 
 #[test]