avoid reshuffling active validators (#2605)

runtime/parachains/src/inclusion.rs

@@ -1213,7 +1213,7 @@ mod tests {
 		let validator_public = validator_pubkeys(&validators);
 
 		new_test_ext(genesis_config(paras)).execute_with(|| {
-			shared::Module::<Test>::set_active_validators(validator_public.clone());
+			shared::Module::<Test>::set_active_validators_ascending(validator_public.clone());
 			shared::Module::<Test>::set_session_index(5);
 
 			let signing_context = SigningContext {
@@ -1446,7 +1446,7 @@ mod tests {
 		let validator_public = validator_pubkeys(&validators);
 
 		new_test_ext(genesis_config(paras)).execute_with(|| {
-			shared::Module::<Test>::set_active_validators(validator_public.clone());
+			shared::Module::<Test>::set_active_validators_ascending(validator_public.clone());
 			shared::Module::<Test>::set_session_index(5);
 
 			let signing_context = SigningContext {
@@ -1611,7 +1611,7 @@ mod tests {
 		let validator_public = validator_pubkeys(&validators);
 
 		new_test_ext(genesis_config(paras)).execute_with(|| {
-			shared::Module::<Test>::set_active_validators(validator_public.clone());
+			shared::Module::<Test>::set_active_validators_ascending(validator_public.clone());
 			shared::Module::<Test>::set_session_index(5);
 
 			run_to_block(5, |_| None);
@@ -2098,7 +2098,7 @@ mod tests {
 		let validator_public = validator_pubkeys(&validators);
 
 		new_test_ext(genesis_config(paras)).execute_with(|| {
-			shared::Module::<Test>::set_active_validators(validator_public.clone());
+			shared::Module::<Test>::set_active_validators_ascending(validator_public.clone());
 			shared::Module::<Test>::set_session_index(5);
 
 			run_to_block(5, |_| None);
@@ -2295,7 +2295,7 @@ mod tests {
 		let validator_public = validator_pubkeys(&validators);
 
 		new_test_ext(genesis_config(paras)).execute_with(|| {
-			shared::Module::<Test>::set_active_validators(validator_public.clone());
+			shared::Module::<Test>::set_active_validators_ascending(validator_public.clone());
 			shared::Module::<Test>::set_session_index(5);
 
 			run_to_block(5, |_| None);
@@ -2392,7 +2392,7 @@ mod tests {
 		let validator_public = validator_pubkeys(&validators);
 
 		new_test_ext(genesis_config(paras)).execute_with(|| {
-			shared::Module::<Test>::set_active_validators(validator_public.clone());
+			shared::Module::<Test>::set_active_validators_ascending(validator_public.clone());
 			shared::Module::<Test>::set_session_index(5);
 
 			let validators_new = vec![

runtime/parachains/src/mock.rs

@@ -135,9 +135,21 @@ impl crate::inclusion_inherent::Config for Test { }
 
 impl crate::session_info::Config for Test { }
 
+thread_local! {
+	pub static DISCOVERY_AUTHORITIES: RefCell<Vec<AuthorityDiscoveryId>> = RefCell::new(Vec::new());
+}
+
+pub fn discovery_authorities() -> Vec<AuthorityDiscoveryId> {
+	DISCOVERY_AUTHORITIES.with(|r| r.borrow().clone())
+}
+
+pub fn set_discovery_authorities(new: Vec<AuthorityDiscoveryId>) {
+	DISCOVERY_AUTHORITIES.with(|r| *r.borrow_mut() = new);
+}
+
 impl crate::session_info::AuthorityDiscoveryConfig for Test {
 	fn authorities() -> Vec<AuthorityDiscoveryId> {
-		Vec::new()
+		discovery_authorities()
 	}
 }
 

runtime/parachains/src/session_info.rs

@@ -90,8 +90,8 @@ impl<T: Config> Module<T> {
 		let validators = notification.validators.clone();
 		let discovery_keys = <T as AuthorityDiscoveryConfig>::authorities();
 		let assignment_keys = AssignmentKeysUnsafe::get();
-
 		let active_set = <shared::Module<T>>::active_validator_indices();
+
 		let validator_groups = <scheduler::Module<T>>::validator_groups();
 		let n_cores = n_parachains + config.parathread_cores;
 		let zeroth_delay_tranche_width = config.zeroth_delay_tranche_width;
@@ -118,7 +118,7 @@ impl<T: Config> Module<T> {
 		}
 		// create a new entry in `Sessions` with information about the current session
 		let new_session_info = SessionInfo {
-			validators: take_active_subset(&active_set, &validators),
+			validators, // these are from the notification and are thus already correct.
 			discovery_keys: take_active_subset(&active_set, &discovery_keys),
 			assignment_keys: take_active_subset(&active_set, &assignment_keys),
 			validator_groups,
@@ -175,7 +175,8 @@ mod tests {
 	use crate::initializer::SessionChangeNotification;
 	use crate::configuration::HostConfiguration;
 	use frame_support::traits::{OnFinalize, OnInitialize};
-	use primitives::v1::BlockNumber;
+	use primitives::v1::{BlockNumber, ValidatorId, ValidatorIndex};
+	use keyring::Sr25519Keyring;
 
 	fn run_to_block(
 		to: BlockNumber,
@@ -319,4 +320,58 @@ mod tests {
 			assert_eq!(session.needed_approvals, 42);
 		})
 	}
+
+	#[test]
+	fn session_info_active_subsets() {
+		let unscrambled = vec![
+			Sr25519Keyring::Alice,
+			Sr25519Keyring::Bob,
+			Sr25519Keyring::Charlie,
+			Sr25519Keyring::Dave,
+			Sr25519Keyring::Eve,
+		];
+
+		let active_set = vec![ValidatorIndex(4), ValidatorIndex(0), ValidatorIndex(2)];
+
+		let unscrambled_validators: Vec<ValidatorId>
+			= unscrambled.iter().map(|v| v.public().into()).collect();
+		let unscrambled_discovery: Vec<AuthorityDiscoveryId>
+			= unscrambled.iter().map(|v| v.public().into()).collect();
+		let unscrambled_assignment: Vec<AssignmentId>
+			= unscrambled.iter().map(|v| v.public().into()).collect();
+
+		let validators = take_active_subset(&active_set, &unscrambled_validators);
+
+		new_test_ext(genesis_config()).execute_with(|| {
+			Shared::set_active_validators_with_indices(
+				active_set.clone(),
+				validators.clone(),
+			);
+
+			assert_eq!(Shared::active_validator_indices(), active_set);
+
+			AssignmentKeysUnsafe::set(unscrambled_assignment.clone());
+			crate::mock::set_discovery_authorities(unscrambled_discovery.clone());
+			assert_eq!(<crate::mock::Test>::authorities(), unscrambled_discovery);
+
+			// invoke directly, because `run_to_block` will invoke `Shared`	and clobber our
+			// values.
+			SessionInfo::initializer_on_new_session(&SessionChangeNotification {
+				session_index: 1,
+				validators: validators.clone(),
+				..Default::default()
+			});
+			let session = Sessions::get(&1).unwrap();
+
+			assert_eq!(session.validators, validators);
+			assert_eq!(
+				session.discovery_keys,
+				take_active_subset(&active_set, &unscrambled_discovery),
+			);
+			assert_eq!(
+				session.assignment_keys,
+				take_active_subset(&active_set, &unscrambled_assignment),
+			);
+		})
+	}
 }

runtime/parachains/src/shared.rs

@@ -116,12 +116,22 @@ impl<T: Config> Module<T> {
 	}
 
 	#[cfg(test)]
-	pub(crate) fn set_active_validators(active: Vec<ValidatorId>) {
+	pub(crate) fn set_active_validators_ascending(active: Vec<ValidatorId>) {
 		ActiveValidatorIndices::set(
 			(0..active.len()).map(|i| ValidatorIndex(i as _)).collect()
 		);
 		ActiveValidatorKeys::set(active);
 	}
+
+	#[cfg(test)]
+	pub(crate) fn set_active_validators_with_indices(
+		indices: Vec<ValidatorIndex>,
+		keys: Vec<ValidatorId>,
+	) {
+		assert_eq!(indices.len(), keys.len());
+		ActiveValidatorIndices::set(indices);
+		ActiveValidatorKeys::set(keys);
+	}
 }
 
 #[cfg(test)]