fix up and use shared

runtime/parachains/src/configuration.rs

@@ -29,6 +29,7 @@ use frame_support::{
 use parity_scale_codec::{Encode, Decode};
 use frame_system::ensure_root;
 use sp_runtime::traits::Zero;
+use crate::shared;
 
 /// All configuration of the runtime with respect to parachains and parathreads.
 #[derive(Clone, Encode, Decode, PartialEq, sp_core::RuntimeDebug)]
@@ -231,14 +232,14 @@ impl<BlockNumber: Zero> HostConfiguration<BlockNumber> {
 	}
 }
 
-pub trait Config: frame_system::Config { }
+pub trait Config: frame_system::Config + shared::Config { }
 
 decl_storage! {
 	trait Store for Module<T: Config> as Configuration {
 		/// The active configuration for the current session.
 		ActiveConfig get(fn config) config(): HostConfiguration<T::BlockNumber>;
 		/// Pending configuration (if any) for the next session.
-		PendingConfig: Option<HostConfiguration<T::BlockNumber>>;
+		PendingConfig: map hasher(twox_64_concat) SessionIndex => Option<HostConfiguration<T::BlockNumber>>;
 	}
 	add_extra_genesis {
 		build(|config: &Self| {
@@ -646,12 +647,21 @@ impl<T: Config> Module<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(_validators: &[ValidatorId], _queued: &[ValidatorId]) {
-		if let Some(pending) = <Self as Store>::PendingConfig::take() {
+	pub(crate) fn initializer_on_new_session(
+		_validators: &[ValidatorId],
+		_queued: &[ValidatorId],
+		session_index: &SessionIndex,
+	) {
+		if let Some(pending) = <Self as Store>::PendingConfig::take(session_index) {
 			<Self as Store>::ActiveConfig::set(pending);
 		}
 	}
 
+	/// Return the session index that should be used for any future scheduled changes.
+	fn scheduled_session() -> SessionIndex {
+		shared::Module::<T>::scheduled_session()
+	}
+
 	// NOTE: Explicitly tell rustc not to inline this because otherwise heuristics note the incoming
 	// closure making it's attractive to inline. However, in this case, we will end up with lots of
 	// duplicated code (making this function to show up in the top of heaviest functions) only for
@@ -660,44 +670,45 @@ impl<T: Config> Module<T> {
 	fn update_config_member(
 		updater: impl FnOnce(&mut HostConfiguration<T::BlockNumber>) -> bool,
 	) {
-		let pending = <Self as Store>::PendingConfig::get();
+		let scheduled_session = Self::scheduled_session();
+		let pending = <Self as Store>::PendingConfig::get(scheduled_session);
 		let mut prev = pending.unwrap_or_else(Self::config);
 
 		if updater(&mut prev) {
-			<Self as Store>::PendingConfig::set(Some(prev));
+			<Self as Store>::PendingConfig::insert(scheduled_session, prev);
 		}
 	}
 }
 
 #[cfg(test)]
 mod tests {
 	use super::*;
-	use crate::mock::{new_test_ext, Initializer, Configuration, Origin};
+	use crate::mock::{new_test_ext, Configuration, Origin};
 
-	use frame_support::traits::{OnFinalize, OnInitialize};
+	use frame_support::assert_ok;
 
 	#[test]
-	fn config_changes_on_session_boundary() {
+	fn config_changes_after_2_session_boundary() {
 		new_test_ext(Default::default()).execute_with(|| {
 			let old_config = Configuration::config();
 			let mut config = old_config.clone();
 			config.validation_upgrade_delay = 100;
-
 			assert!(old_config != config);
 
-			<Configuration as Store>::PendingConfig::set(Some(config.clone()));
-
-			Initializer::on_initialize(1);
+			assert_ok!(Configuration::set_validation_upgrade_delay(Origin::root(), 100));
 
 			assert_eq!(Configuration::config(), old_config);
-			assert_eq!(<Configuration as Store>::PendingConfig::get(), Some(config.clone()));
+			assert_eq!(<Configuration as Store>::PendingConfig::get(1), None);
+
+			Configuration::initializer_on_new_session(&[], &[], &1);
 
-			Initializer::on_finalize(1);
+			assert_eq!(Configuration::config(), old_config);
+			assert_eq!(<Configuration as Store>::PendingConfig::get(2), Some(config.clone()));
 
-			Configuration::initializer_on_new_session(&[], &[]);
+			Configuration::initializer_on_new_session(&[], &[], &2);
 
 			assert_eq!(Configuration::config(), config);
-			assert!(<Configuration as Store>::PendingConfig::get().is_none());
+			assert_eq!(<Configuration as Store>::PendingConfig::get(3), None);
 		})
 	}
 
@@ -743,7 +754,7 @@ mod tests {
 				hrmp_max_message_num_per_candidate: 20,
 			};
 
-			assert!(<Configuration as Store>::PendingConfig::get().is_none());
+			assert!(<Configuration as Store>::PendingConfig::get(shared::SESSION_DELAY).is_none());
 
 			Configuration::set_validation_upgrade_frequency(
 				Origin::root(), new_config.validation_upgrade_frequency,
@@ -865,7 +876,7 @@ mod tests {
 				new_config.hrmp_max_message_num_per_candidate,
 			).unwrap();
 
-			assert_eq!(<Configuration as Store>::PendingConfig::get(), Some(new_config));
+			assert_eq!(<Configuration as Store>::PendingConfig::get(shared::SESSION_DELAY), Some(new_config));
 		})
 	}
 
@@ -880,7 +891,7 @@ mod tests {
 	fn setting_config_to_same_as_current_is_noop() {
 		new_test_ext(Default::default()).execute_with(|| {
 			Configuration::set_validation_upgrade_delay(Origin::root(), Default::default()).unwrap();
-			assert!(<Configuration as Store>::PendingConfig::get().is_none())
+			assert!(<Configuration as Store>::PendingConfig::get(shared::SESSION_DELAY).is_none())
 		});
 	}
 

runtime/parachains/src/initializer.rs

@@ -21,7 +21,7 @@
 
 use sp_std::prelude::*;
 use frame_support::weights::Weight;
-use primitives::v1::ValidatorId;
+use primitives::v1::{ValidatorId, SessionIndex};
 use frame_support::{
 	decl_storage, decl_module, decl_error, traits::{OneSessionHandler, Randomness},
 };
@@ -45,7 +45,7 @@ pub struct SessionChangeNotification<BlockNumber> {
 	/// A secure random seed for the session, gathered from BABE.
 	pub random_seed: [u8; 32],
 	/// New session index.
-	pub session_index: sp_staking::SessionIndex,
+	pub session_index: SessionIndex,
 }
 
 impl<BlockNumber: Default + From<u32>> Default for SessionChangeNotification<BlockNumber> {
@@ -65,7 +65,7 @@ impl<BlockNumber: Default + From<u32>> Default for SessionChangeNotification<Blo
 struct BufferedSessionChange {
 	validators: Vec<ValidatorId>,
 	queued: Vec<ValidatorId>,
-	session_index: sp_staking::SessionIndex,
+	session_index: SessionIndex,
 }
 
 pub trait Config:
@@ -173,7 +173,7 @@ decl_module! {
 
 impl<T: Config> Module<T> {
 	fn apply_new_session(
-		session_index: sp_staking::SessionIndex,
+		session_index: SessionIndex,
 		validators: Vec<ValidatorId>,
 		queued: Vec<ValidatorId>,
 	) {
@@ -189,7 +189,7 @@ impl<T: Config> Module<T> {
 
 		// 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::Module::<T>::initializer_on_new_session(&validators, &queued);
+		configuration::Module::<T>::initializer_on_new_session(&validators, &queued, &session_index);
 
 		let new_config = <configuration::Module<T>>::config();
 
@@ -216,7 +216,7 @@ impl<T: Config> Module<T> {
 	/// at the end of the block. If `queued` is `None`, the `validators` are considered queued.
 	fn on_new_session<'a, I: 'a>(
 		_changed: bool,
-		session_index: sp_staking::SessionIndex,
+		session_index: SessionIndex,
 		validators: I,
 		queued: Option<I>,
 	)

runtime/parachains/src/paras.rs

@@ -119,9 +119,6 @@ pub enum ParaLifecycle {
 	OffboardingParachain,
 }
 
-// Wait until the session index is 2 larger then the current index to apply any changes.
-pub(crate) const SESSION_DELAY: SessionIndex = 2;
-
 impl ParaLifecycle {
 	/// Returns true if parachain is currently onboarding. To learn if the
 	/// parachain is onboarding as a parachain or parathread, look at the
@@ -523,7 +520,7 @@ impl<T: Config> Module<T> {
 	///
 	/// Will return error if para is already registered in the system.
 	pub(crate) fn schedule_para_initialize(id: ParaId, genesis: ParaGenesisArgs) -> DispatchResult {
-		let scheduled_session = shared::Module::<T>::session_index() + SESSION_DELAY;
+		let scheduled_session = Self::scheduled_session();
 
 		// Make sure parachain isn't already in our system.
 		ensure!(Self::can_schedule_para_initialize(&id, &genesis), Error::<T>::CannotOnboard);
@@ -543,7 +540,7 @@ impl<T: Config> Module<T> {
 	///
 	/// Will return error if para is not a stable parachain or parathread.
 	pub(crate) fn schedule_para_cleanup(id: ParaId) -> DispatchResult {
-		let scheduled_session = shared::Module::<T>::session_index() + SESSION_DELAY;
+		let scheduled_session = Self::scheduled_session();
 		let lifecycle = ParaLifecycles::get(&id).ok_or(Error::<T>::NotRegistered)?;
 
 		match lifecycle {
@@ -570,7 +567,7 @@ impl<T: Config> Module<T> {
 	/// Will return error if `ParaLifecycle` is not `Parathread`.
 	#[allow(unused)]
 	pub(crate) fn schedule_parathread_upgrade(id: ParaId) -> DispatchResult {
-		let scheduled_session = shared::Module::<T>::session_index() + SESSION_DELAY;
+		let scheduled_session = Self::scheduled_session();
 		let lifecycle = ParaLifecycles::get(&id).ok_or(Error::<T>::NotRegistered)?;
 
 		ensure!(lifecycle == ParaLifecycle::Parathread, Error::<T>::CannotUpgrade);
@@ -590,7 +587,7 @@ impl<T: Config> Module<T> {
 	/// Noop if `ParaLifecycle` is not `Parachain`.
 	#[allow(unused)]
 	pub(crate) fn schedule_parachain_downgrade(id: ParaId) -> DispatchResult {
-		let scheduled_session = shared::Module::<T>::session_index() + SESSION_DELAY;
+		let scheduled_session = Self::scheduled_session();
 		let lifecycle = ParaLifecycles::get(&id).ok_or(Error::<T>::NotRegistered)?;
 
 		ensure!(lifecycle == ParaLifecycle::Parachain, Error::<T>::CannotDowngrade);
@@ -751,6 +748,11 @@ impl<T: Config> Module<T> {
 
 		Self::past_code_meta(&id).most_recent_change()
 	}
+
+	/// Return the session index that should be used for any future scheduled changes.
+	fn scheduled_session() -> SessionIndex {
+		shared::Module::<T>::scheduled_session()
+	}
 }
 
 #[cfg(test)]
@@ -1245,7 +1247,7 @@ mod tests {
 			// Just scheduling cleanup shouldn't change anything.
 			{
 				assert_eq!(
-					<Paras as Store>::ActionsQueue::get(Shared::session_index() + SESSION_DELAY),
+					<Paras as Store>::ActionsQueue::get(Paras::scheduled_session()),
 					vec![para_id],
 				);
 				assert_eq!(Paras::parachains(), vec![para_id]);
@@ -1321,7 +1323,7 @@ mod tests {
 			));
 
 			assert_eq!(
-				<Paras as Store>::ActionsQueue::get(Shared::session_index() + SESSION_DELAY),
+				<Paras as Store>::ActionsQueue::get(Paras::scheduled_session()),
 				vec![c, b, a],
 			);
 
@@ -1335,7 +1337,7 @@ mod tests {
 
 			assert_eq!(Paras::parachains(), Vec::new());
 			assert_eq!(
-				<Paras as Store>::ActionsQueue::get(Shared::session_index() + SESSION_DELAY),
+				<Paras as Store>::ActionsQueue::get(Paras::scheduled_session()),
 				vec![c, b, a],
 			);
 
@@ -1349,7 +1351,7 @@ mod tests {
 			run_to_block(4, Some(vec![3,4]));
 
 			assert_eq!(Paras::parachains(), vec![c, b]);
-			assert_eq!(<Paras as Store>::ActionsQueue::get(Shared::session_index() + SESSION_DELAY), Vec::new());
+			assert_eq!(<Paras as Store>::ActionsQueue::get(Paras::scheduled_session()), Vec::new());
 
 			// Lifecycle is tracked correctly
 			assert_eq!(ParaLifecycles::get(&a), Some(ParaLifecycle::Parathread));

runtime/parachains/src/scheduler.rs

@@ -770,7 +770,7 @@ mod tests {
 				let mut notification_with_session_index = notification;
 				// We will make every session change trigger an action queue. Normally this may require 2 or more session changes.
 				if notification_with_session_index.session_index == SessionIndex::default() {
-					notification_with_session_index.session_index = Shared::session_index() + crate::paras::SESSION_DELAY;
+					notification_with_session_index.session_index = Shared::scheduled_session();
 				}
 				Paras::initializer_on_new_session(&notification_with_session_index);
 				Scheduler::initializer_on_new_session(&notification_with_session_index);