Skip to content
This repository has been archived by the owner on Nov 15, 2023. It is now read-only.

Put all authorities of a session into SessionInfo. #3813

Merged
merged 9 commits into from
Sep 14, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion node/network/gossip-support/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
2 changes: 1 addition & 1 deletion node/subsystem-util/src/runtime/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down
20 changes: 20 additions & 0 deletions primitives/src/v1/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// `SessionInfo::validators` will be limited to to `max_validators` when set.
/// `SessionInfo::validators` will be limited 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
Expand Down
48 changes: 34 additions & 14 deletions roadmap/implementers-guide/src/runtime/session_info.md
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 7 additions & 3 deletions runtime/parachains/src/session_info.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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),
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only seems to alter the discovery keys

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes, that's on purpose. We do have more authorities than parachain validators, but only parachain validators will be approval checkers, thus limiting assignment keys seems to be desired.

assignment_keys: take_active_subset(&active_set, &assignment_keys),
validator_groups,
n_cores,
Expand Down Expand Up @@ -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};
Expand Down Expand Up @@ -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,
Expand Down
66 changes: 60 additions & 6 deletions runtime/parachains/src/util.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};

Expand All @@ -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
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
/// First item in pair will be all items in set have indeces found in the `active` indices set (in
/// First item in pair will be all items in set have indices 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
Comment on lines +77 to +79
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: iiuc is only done, to assure that the acitve validators occupy the indices 0..n, correct? Would be nice to state this in the comment and/or the fn-name

}

/// 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",
Expand All @@ -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]);
}
}