Put all authorities of a session into SessionInfo. (#3813)

* Make SessionInfo include all authorities

of the current session.

* Add missing import.

* Only take subset for assignment keys.

* Fix typo.

* Make spellcheck happy.

* Really.

* Fix test.

* More clear documentation.

* Update comments in `SessionInfo`.

node/network/gossip-support/src/lib.rs

@@ -54,7 +54,6 @@ const BACKOFF_DURATION: Duration = Duration::from_secs(5);
 /// should be fine:
 ///
 /// https://github.com/paritytech/substrate/blob/fc49802f263529160635471c8a17888846035f5d/client/authority-discovery/src/lib.rs#L88
-///
 const LOW_CONNECTIVITY_WARN_DELAY: Duration = Duration::from_secs(600);
 
 /// The Gossip Support subsystem.

node/subsystem-util/src/runtime/mod.rs

@@ -199,7 +199,7 @@ impl RuntimeInfo {
 	/// Build `ValidatorInfo` for the current session.
 	///
 	///
-	/// Returns: `None` if not a validator.
+	/// Returns: `None` if not a parachain validator.
 	async fn get_validator_info(&self, session_info: &SessionInfo) -> Result<ValidatorInfo> {
 		if let Some(our_index) = self.get_our_index(&session_info.validators).await {
 			// Get our group index:

primitives/src/v1/mod.rs

@@ -892,11 +892,31 @@ pub enum CandidateEvent<H = Hash> {
 #[cfg_attr(feature = "std", derive(PartialEq, Default, MallocSizeOf))]
 pub struct SessionInfo {
 	/// Validators in canonical ordering.
+	///
+	/// NOTE: There might be more authorities in the current session, than `validators` participating
+	/// in parachain consensus. See
+	/// [`max_validators`](https://github.com/paritytech/polkadot/blob/a52dca2be7840b23c19c153cf7e110b1e3e475f8/runtime/parachains/src/configuration.rs#L148).
+	///
+	/// `SessionInfo::validators` will be limited to to `max_validators` when set.
 	pub validators: Vec<ValidatorId>,
 	/// Validators' authority discovery keys for the session in canonical ordering.
+	///
+	/// NOTE: The first `validators.len()` entries will match the corresponding validators in
+	/// `validators`, afterwards any remaining authorities can be found. This is any authorities not
+	/// participating in parachain consensus - see
+	/// [`max_validators`](https://github.com/paritytech/polkadot/blob/a52dca2be7840b23c19c153cf7e110b1e3e475f8/runtime/parachains/src/configuration.rs#L148)
 	#[cfg_attr(feature = "std", ignore_malloc_size_of = "outside type")]
 	pub discovery_keys: Vec<AuthorityDiscoveryId>,
 	/// The assignment keys for validators.
+	///
+	/// NOTE: There might be more authorities in the current session, than validators participating
+	/// in parachain consensus. See
+	/// [`max_validators`](https://github.com/paritytech/polkadot/blob/a52dca2be7840b23c19c153cf7e110b1e3e475f8/runtime/parachains/src/configuration.rs#L148).
+	///
+	/// Therefore:
+	/// ```ignore
+	///		assignment_keys.len() == validators.len() && validators.len() <= discovery_keys.len()
+	///	```
 	pub assignment_keys: Vec<AssignmentId>,
 	/// Validators in shuffled ordering - these are the validator groups as produced
 	/// by the `Scheduler` module for the session and are typically referred to by

roadmap/implementers-guide/src/runtime/session_info.md

@@ -8,27 +8,47 @@ Helper structs:
 
 ```rust
 struct SessionInfo {
-    // validators in canonical ordering. These are the public keys used for backing,
-    // dispute participation, and approvals.
+    /// Validators in canonical ordering.
+    ///
+    /// NOTE: There might be more authorities in the current session, than `validators` participating
+    /// in parachain consensus. See
+    /// [`max_validators`](https://github.com/paritytech/polkadot/blob/a52dca2be7840b23c19c153cf7e110b1e3e475f8/runtime/parachains/src/configuration.rs#L148).
+    ///
+    /// `SessionInfo::validators` will be limited to to `max_validators` when set.
     validators: Vec<ValidatorId>,
-    // validators' authority discovery keys for the session in canonical ordering.
-    discovery_keys: Vec<DiscoveryId>,
-    // The assignment keys for validators.
+    /// Validators' authority discovery keys for the session in canonical ordering.
+    ///
+    /// NOTE: The first `validators.len()` entries will match the corresponding validators in
+    /// `validators`, afterwards any remaining authorities can be found. This is any authorities not
+    /// participating in parachain consensus - see
+    /// [`max_validators`](https://github.com/paritytech/polkadot/blob/a52dca2be7840b23c19c153cf7e110b1e3e475f8/runtime/parachains/src/configuration.rs#L148)
+    #[cfg_attr(feature = "std", ignore_malloc_size_of = "outside type")]
+    discovery_keys: Vec<AuthorityDiscoveryId>,
+    /// The assignment keys for validators.
+    ///
+    /// NOTE: There might be more authorities in the current session, than validators participating
+    /// in parachain consensus. See
+    /// [`max_validators`](https://github.com/paritytech/polkadot/blob/a52dca2be7840b23c19c153cf7e110b1e3e475f8/runtime/parachains/src/configuration.rs#L148).
+    ///
+    /// Therefore:
+    /// ```ignore
+    ///		assignment_keys.len() == validators.len() && validators.len() <= discovery_keys.len()
+    ///	```
     assignment_keys: Vec<AssignmentId>,
-    // validators in shuffled ordering - these are the validator groups as produced
-    // by the `Scheduler` module for the session and are typically referred to by
-    // `GroupIndex`.
+    /// Validators in shuffled ordering - these are the validator groups as produced
+    /// by the `Scheduler` module for the session and are typically referred to by
+    /// `GroupIndex`.
     validator_groups: Vec<Vec<ValidatorIndex>>,
-    // The number of availability cores used by the protocol during this session.
+    /// The number of availability cores used by the protocol during this session.
     n_cores: u32,
-    // the zeroth delay tranche width.
+    /// The zeroth delay tranche width.
     zeroth_delay_tranche_width: u32,
-    // The number of samples we do of relay_vrf_modulo.
+    /// The number of samples we do of `relay_vrf_modulo`.
     relay_vrf_modulo_samples: u32,
-    // The number of delay tranches in total.
+    /// The number of delay tranches in total.
     n_delay_tranches: u32,
-    // How many slots (BABE / SASSAFRAS) must pass before an assignment is considered a
-    // no-show.
+    /// How many slots (BABE / SASSAFRAS) must pass before an assignment is considered a
+    /// no-show.
     no_show_slots: u32,
     /// The number of validators needed to approve a block.
     needed_approvals: u32,

runtime/parachains/src/session_info.rs

@@ -19,7 +19,10 @@
 //!
 //! See https://w3f.github.io/parachain-implementers-guide/runtime/session_info.html.
 
-use crate::{configuration, paras, scheduler, shared, util::take_active_subset};
+use crate::{
+	configuration, paras, scheduler, shared,
+	util::{take_active_subset, take_active_subset_and_inactive},
+};
 use frame_support::{pallet_prelude::*, traits::OneSessionHandler};
 use primitives::v1::{AssignmentId, AuthorityDiscoveryId, SessionIndex, SessionInfo};
 use sp_std::vec::Vec;
@@ -120,7 +123,7 @@ impl<T: Config> Pallet<T> {
 		// create a new entry in `Sessions` with information about the current session
 		let new_session_info = SessionInfo {
 			validators, // these are from the notification and are thus already correct.
-			discovery_keys: take_active_subset(&active_set, &discovery_keys),
+			discovery_keys: take_active_subset_and_inactive(&active_set, &discovery_keys),
 			assignment_keys: take_active_subset(&active_set, &assignment_keys),
 			validator_groups,
 			n_cores,
@@ -176,6 +179,7 @@ mod tests {
 			new_test_ext, Configuration, MockGenesisConfig, Origin, ParasShared, SessionInfo,
 			System, Test,
 		},
+		util::take_active_subset,
 	};
 	use keyring::Sr25519Keyring;
 	use primitives::v1::{BlockNumber, ValidatorId, ValidatorIndex};
@@ -357,7 +361,7 @@ mod tests {
 			assert_eq!(session.validators, validators);
 			assert_eq!(
 				session.discovery_keys,
-				take_active_subset(&active_set, &unscrambled_discovery),
+				take_active_subset_and_inactive(&active_set, &unscrambled_discovery),
 			);
 			assert_eq!(
 				session.assignment_keys,

runtime/parachains/src/util.rs

@@ -18,7 +18,7 @@
 //! on all modules.
 
 use primitives::v1::{Id as ParaId, PersistedValidationData, ValidatorIndex};
-use sp_std::vec::Vec;
+use sp_std::{collections::btree_set::BTreeSet, vec::Vec};
 
 use crate::{configuration, hrmp, paras};
 
@@ -41,15 +41,49 @@ pub fn make_persisted_validation_data<T: paras::Config + hrmp::Config>(
 	})
 }
 
-/// Take the active subset of a set containing all validators.
-pub fn take_active_subset<T: Clone>(active_validators: &[ValidatorIndex], set: &[T]) -> Vec<T> {
-	let subset: Vec<_> = active_validators
+/// Take an active subset of a set containing all validators.
+///
+/// First item in pair will be all items in set have indeces found in the `active` indices set (in
+/// the order of the `active` vec, the second item will contain the rest, in the original order.
+///
+/// ```ignore
+///		split_active_subset(active.into_iter().collect(), all).0 == take_active_subset(active, all)
+/// ```
+pub fn split_active_subset<T: Clone>(active: &[ValidatorIndex], all: &[T]) -> (Vec<T>, Vec<T>) {
+	let active_set: BTreeSet<_> = active.iter().cloned().collect();
+	// active result has ordering of active set.
+	let active_result = take_active_subset(active, all);
+	// inactive result preserves original ordering of `all`.
+	let inactive_result = all
 		.iter()
-		.filter_map(|i| set.get(i.0 as usize))
+		.enumerate()
+		.filter(|(i, _)| !active_set.contains(&ValidatorIndex(*i as _)))
+		.map(|(_, v)| v)
 		.cloned()
 		.collect();
 
-	if subset.len() != active_validators.len() {
+	if active_result.len() != active.len() {
+		log::warn!(
+			target: "runtime::parachains",
+			"Took active validators from set with wrong size.",
+		);
+	}
+
+	(active_result, inactive_result)
+}
+
+/// Uses `split_active_subset` and concatenates the inactive to the active vec.
+pub fn take_active_subset_and_inactive<T: Clone>(active: &[ValidatorIndex], all: &[T]) -> Vec<T> {
+	let (mut a, mut i) = split_active_subset(active, all);
+	a.append(&mut i);
+	a
+}
+
+/// Take the active subset of a set containing all validators.
+pub fn take_active_subset<T: Clone>(active: &[ValidatorIndex], set: &[T]) -> Vec<T> {
+	let subset: Vec<_> = active.iter().filter_map(|i| set.get(i.0 as usize)).cloned().collect();
+
+	if subset.len() != active.len() {
 		log::warn!(
 			target: "runtime::parachains",
 			"Took active validators from set with wrong size",
@@ -58,3 +92,23 @@ pub fn take_active_subset<T: Clone>(active_validators: &[ValidatorIndex], set: &
 
 	subset
 }
+
+#[cfg(test)]
+mod tests {
+
+	use sp_std::vec::Vec;
+
+	use crate::util::{split_active_subset, take_active_subset};
+	use primitives::v1::ValidatorIndex;
+
+	#[test]
+	fn take_active_subset_is_compatible_with_split_active_subset() {
+		let active: Vec<_> = vec![ValidatorIndex(1), ValidatorIndex(7), ValidatorIndex(3)];
+		let validators = vec![9, 1, 6, 7, 4, 5, 2, 3, 0, 8];
+		let (selected, unselected) = split_active_subset(&active, &validators);
+		let selected2 = take_active_subset(&active, &validators);
+		assert_eq!(selected, selected2);
+		assert_eq!(unselected, vec![9, 6, 4, 5, 2, 0, 8]);
+		assert_eq!(selected, vec![1, 3, 7]);
+	}
+}