configuration: refactor configuration initialization (#4569)

Refactor the configuration module's initializer_on_new_session in such a
way that it returns the configuration. This would make it inline with
other special initialization routines like `shared`'s or `paras`.

This will be useful in a following PR that will check consistency of the
configuration before setting it.

Author: pepyakin

runtime/parachains/src/configuration.rs

@@ -1054,6 +1054,15 @@ pub mod pallet {
 	}
 }
 
+/// A struct that holds the configuration that was active before the session change and optionally
+/// a configuration that became active after the session change.
+pub struct SessionChangeOutcome<BlockNumber> {
+	/// Previously active configuration.
+	pub prev_config: HostConfiguration<BlockNumber>,
+	/// If new configuration was applied during the session change, this is the new configuration.
+	pub new_config: Option<HostConfiguration<BlockNumber>>,
+}
+
 impl<T: Config> Pallet<T> {
 	/// Called by the initializer to initialize the configuration module.
 	pub(crate) fn initializer_initialize(_now: T::BlockNumber) -> Weight {
@@ -1064,13 +1073,22 @@ impl<T: Config> Pallet<T> {
 	pub(crate) fn initializer_finalize() {}
 
 	/// Called by the initializer to note that a new session has started.
-	pub(crate) fn initializer_on_new_session(session_index: &SessionIndex) {
+	///
+	/// Returns the configuration that was actual before the session change and the configuration
+	/// that became active after the session change. If there were no scheduled changes, both will
+	/// be the same.
+	pub(crate) fn initializer_on_new_session(
+		session_index: &SessionIndex,
+	) -> SessionChangeOutcome<T::BlockNumber> {
 		let pending_configs = <PendingConfigs<T>>::get();
+		let prev_config = <Self as Store>::ActiveConfig::get();
+
+		// No pending configuration changes, so we're done.
 		if pending_configs.is_empty() {
-			return
+			return SessionChangeOutcome { prev_config, new_config: None }
 		}
 
-		let (past_and_present, future) = pending_configs
+		let (mut past_and_present, future) = pending_configs
 			.into_iter()
 			.partition::<Vec<_>, _>(|&(apply_at_session, _)| apply_at_session <= *session_index);
 
@@ -1082,11 +1100,16 @@ impl<T: Config> Pallet<T> {
 				"Skipping applying configuration changes scheduled sessions in the past",
 			);
 		}
-		if let Some((_, pending)) = past_and_present.last() {
-			<Self as Store>::ActiveConfig::put(pending);
+
+		let new_config = past_and_present.pop().map(|(_, config)| config);
+		if let Some(ref new_config) = new_config {
+			// Apply the new configuration.
+			<Self as Store>::ActiveConfig::put(new_config);
 		}
 
 		<PendingConfigs<T>>::put(future);
+
+		SessionChangeOutcome { prev_config, new_config }
 	}
 
 	/// Return the session index that should be used for any future scheduled changes.
@@ -1167,9 +1190,14 @@ mod tests {
 
 	use frame_support::assert_ok;
 
-	fn on_new_session(session_index: SessionIndex) {
+	fn on_new_session(
+		session_index: SessionIndex,
+	) -> (HostConfiguration<u32>, HostConfiguration<u32>) {
 		ParasShared::set_session_index(session_index);
-		Configuration::initializer_on_new_session(&session_index);
+		let SessionChangeOutcome { prev_config, new_config } =
+			Configuration::initializer_on_new_session(&session_index);
+		let new_config = new_config.unwrap_or_else(|| prev_config.clone());
+		(prev_config, new_config)
 	}
 
 	#[test]
@@ -1182,6 +1210,25 @@ mod tests {
 		});
 	}
 
+	#[test]
+	fn initializer_on_new_session() {
+		new_test_ext(Default::default()).execute_with(|| {
+			let (prev_config, new_config) = on_new_session(1);
+			assert_eq!(prev_config, new_config);
+			assert_ok!(Configuration::set_validation_upgrade_delay(Origin::root(), 100));
+
+			let (prev_config, new_config) = on_new_session(2);
+			assert_eq!(prev_config, new_config);
+
+			let (prev_config, new_config) = on_new_session(3);
+			assert_eq!(prev_config, HostConfiguration::default());
+			assert_eq!(
+				new_config,
+				HostConfiguration { validation_upgrade_delay: 100, ..prev_config }
+			);
+		});
+	}
+
 	#[test]
 	fn config_changes_after_2_session_boundary() {
 		new_test_ext(Default::default()).execute_with(|| {

runtime/parachains/src/initializer.rs

@@ -221,8 +221,6 @@ impl<T: Config> Pallet<T> {
 		all_validators: Vec<ValidatorId>,
 		queued: Vec<ValidatorId>,
 	) {
-		let prev_config = <configuration::Pallet<T>>::config();
-
 		let random_seed = {
 			let mut buf = [0u8; 32];
 			// TODO: audit usage of randomness API
@@ -233,11 +231,9 @@ impl<T: Config> Pallet<T> {
 			buf
 		};
 
-		// We can't pass the new config into the thing that determines the new config,
-		// so we don't pass the `SessionChangeNotification` into this module.
-		configuration::Pallet::<T>::initializer_on_new_session(&session_index);
-
-		let new_config = <configuration::Pallet<T>>::config();
+		let configuration::SessionChangeOutcome { prev_config, new_config } =
+			configuration::Pallet::<T>::initializer_on_new_session(&session_index);
+		let new_config = new_config.unwrap_or_else(|| prev_config.clone());
 
 		let validators = shared::Pallet::<T>::initializer_on_new_session(
 			session_index,