From 848aa04ff4597f4b486f23daf8bf3699d3fbaf8e Mon Sep 17 00:00:00 2001
From: Sergey Shulepov <s.pepyakin@gmail.com>
Date: Fri, 17 Dec 2021 14:12:18 +0000
Subject: [PATCH] configuration: refactor configuration initialization

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.
---
 runtime/parachains/src/configuration.rs | 54 +++++++++++++++++++++----
 runtime/parachains/src/initializer.rs   |  9 +----
 2 files changed, 48 insertions(+), 15 deletions(-)

diff --git a/runtime/parachains/src/configuration.rs b/runtime/parachains/src/configuration.rs
index 8b18c7d59559..7e2073e1fbc8 100644
--- a/runtime/parachains/src/configuration.rs
+++ b/runtime/parachains/src/configuration.rs
@@ -1064,13 +1064,23 @@ 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,
+	) -> (HostConfiguration<T::BlockNumber>, HostConfiguration<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
+			let new_config = prev_config.clone();
+			return (prev_config, new_config)
 		}
 
-		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 +1092,18 @@ 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 = match past_and_present.pop() {
+			Some((_, pending)) => {
+				<Self as Store>::ActiveConfig::put(&pending);
+				pending
+			},
+			None => prev_config.clone(),
+		};
 
 		<PendingConfigs<T>>::put(future);
+
+		(prev_config, new_config)
 	}
 
 	/// Return the session index that should be used for any future scheduled changes.
@@ -1167,9 +1184,11 @@ 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);
+		Configuration::initializer_on_new_session(&session_index)
 	}
 
 	#[test]
@@ -1182,6 +1201,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(|| {
diff --git a/runtime/parachains/src/initializer.rs b/runtime/parachains/src/initializer.rs
index a7cb23799e08..463a8dc2574c 100644
--- a/runtime/parachains/src/initializer.rs
+++ b/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,8 @@ 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 (prev_config, new_config) =
+			configuration::Pallet::<T>::initializer_on_new_session(&session_index);
 
 		let validators = shared::Pallet::<T>::initializer_on_new_session(
 			session_index,