Make sure that on_before_session_ending is called (#3487)

* Make sure that `on_before_session_ending` is called

* Move the call above the validtor set being set

* Bump spec_version

substrate/node/runtime/src/lib.rs

@@ -79,8 +79,8 @@ pub const VERSION: RuntimeVersion = RuntimeVersion {
 	// and set impl_version to equal spec_version. If only runtime
 	// implementation changes and behavior does not, then leave spec_version as
 	// is and increment impl_version.
-	spec_version: 151,
+	spec_version: 152,
-	impl_version: 153,
+	impl_version: 152,
 	apis: RUNTIME_API_VERSIONS,
 };
 

substrate/srml/session/src/lib.rs

@@ -481,6 +481,9 @@ impl<T: Trait> Module<T> {
 
 		let changed = QueuedChanged::get();
 
+		// Inform the session handlers that a session is going to end.
+		T::SessionHandler::on_before_session_ending();
+
 		// Get queued session keys and validators.
 		let session_keys = <QueuedKeys<T>>::get();
 		let validators = session_keys.iter()
@@ -663,6 +666,7 @@ mod tests {
 	use mock::{
 		NEXT_VALIDATORS, SESSION_CHANGED, TEST_SESSION_CHANGED, authorities, force_new_session,
 		set_next_validators, set_session_length, session_changed, Test, Origin, System, Session,
+		reset_before_session_end_called, before_session_end_called,
 	};
 
 	fn new_test_ext() -> runtime_io::TestExternalities<Blake2Hasher> {
@@ -715,6 +719,8 @@ mod tests {
 
 	#[test]
 	fn authorities_should_track_validators() {
+		reset_before_session_end_called();
+
 		with_externalities(&mut new_test_ext(), || {
 			set_next_validators(vec![1, 2]);
 			force_new_session();
@@ -725,6 +731,8 @@ mod tests {
 			]);
 			assert_eq!(Session::validators(), vec![1, 2, 3]);
 			assert_eq!(authorities(), vec![UintAuthorityId(1), UintAuthorityId(2), UintAuthorityId(3)]);
+			assert!(before_session_end_called());
+			reset_before_session_end_called();
 
 			force_new_session();
 			initialize_block(2);
@@ -734,6 +742,8 @@ mod tests {
 			]);
 			assert_eq!(Session::validators(), vec![1, 2]);
 			assert_eq!(authorities(), vec![UintAuthorityId(1), UintAuthorityId(2)]);
+			assert!(before_session_end_called());
+			reset_before_session_end_called();
 
 			set_next_validators(vec![1, 2, 4]);
 			assert_ok!(Session::set_keys(Origin::signed(4), UintAuthorityId(4).into(), vec![]));
@@ -746,6 +756,7 @@ mod tests {
 			]);
 			assert_eq!(Session::validators(), vec![1, 2]);
 			assert_eq!(authorities(), vec![UintAuthorityId(1), UintAuthorityId(2)]);
+			assert!(before_session_end_called());
 
 			force_new_session();
 			initialize_block(4);
@@ -827,45 +838,63 @@ mod tests {
 
 	#[test]
 	fn session_changed_flag_works() {
+		reset_before_session_end_called();
+
 		with_externalities(&mut new_test_ext(), || {
 			TEST_SESSION_CHANGED.with(|l| *l.borrow_mut() = true);
 
 			force_new_session();
 			initialize_block(1);
 			assert!(!session_changed());
+			assert!(before_session_end_called());
+			reset_before_session_end_called();
 
 			force_new_session();
 			initialize_block(2);
 			assert!(!session_changed());
+			assert!(before_session_end_called());
+			reset_before_session_end_called();
 
 			Session::disable_index(0);
 			force_new_session();
 			initialize_block(3);
 			assert!(!session_changed());
+			assert!(before_session_end_called());
+			reset_before_session_end_called();
 
 			force_new_session();
 			initialize_block(4);
 			assert!(session_changed());
+			assert!(before_session_end_called());
+			reset_before_session_end_called();
 
 			force_new_session();
 			initialize_block(5);
 			assert!(!session_changed());
+			assert!(before_session_end_called());
+			reset_before_session_end_called();
 
 			assert_ok!(Session::set_keys(Origin::signed(2), UintAuthorityId(5).into(), vec![]));
 			force_new_session();
 			initialize_block(6);
 			assert!(!session_changed());
+			assert!(before_session_end_called());
+			reset_before_session_end_called();
 
 			// changing the keys of a validator leads to change.
 			assert_ok!(Session::set_keys(Origin::signed(69), UintAuthorityId(69).into(), vec![]));
 			force_new_session();
 			initialize_block(7);
 			assert!(session_changed());
+			assert!(before_session_end_called());
+			reset_before_session_end_called();
 
 			// while changing the keys of a non-validator does not.
 			force_new_session();
 			initialize_block(7);
 			assert!(!session_changed());
+			assert!(before_session_end_called());
+			reset_before_session_end_called();
 		});
 	}
 

substrate/srml/session/src/mock.rs

@@ -53,6 +53,8 @@ thread_local! {
 	pub static SESSION_CHANGED: RefCell<bool> = RefCell::new(false);
 	pub static TEST_SESSION_CHANGED: RefCell<bool> = RefCell::new(false);
 	pub static DISABLED: RefCell<bool> = RefCell::new(false);
+	// Stores if `on_before_session_end` was called
+	pub static BEFORE_SESSION_END_CALLED: RefCell<bool> = RefCell::new(false);
 }
 
 pub struct TestShouldEndSession;
@@ -81,6 +83,9 @@ impl SessionHandler<u64> for TestSessionHandler {
 	fn on_disabled(_validator_index: usize) {
 		DISABLED.with(|l| *l.borrow_mut() = true)
 	}
+	fn on_before_session_ending() {
+		BEFORE_SESSION_END_CALLED.with(|b| *b.borrow_mut() = true);
+	}
 }
 
 pub struct TestOnSessionEnding;
@@ -134,8 +139,17 @@ pub fn set_next_validators(next: Vec<u64>) {
 	NEXT_VALIDATORS.with(|v| *v.borrow_mut() = next);
 }
 
+pub fn before_session_end_called() -> bool {
+	BEFORE_SESSION_END_CALLED.with(|b| *b.borrow())
+}
+
+pub fn reset_before_session_end_called() {
+	BEFORE_SESSION_END_CALLED.with(|b| *b.borrow_mut() = false);
+}
+
 #[derive(Clone, Eq, PartialEq)]
 pub struct Test;
+
 parameter_types! {
 	pub const BlockHashCount: u64 = 250;
 	pub const MaximumBlockWeight: u32 = 1024;