Use a more typesafe approach for managing indexed data

node/core/approval-voting/src/criteria.rs

@@ -21,7 +21,8 @@ use polkadot_node_primitives::approval::{
 	self as approval_types, AssignmentCert, AssignmentCertKind, DelayTranche, RelayVRFStory,
 };
 use polkadot_primitives::v2::{
-	AssignmentId, AssignmentPair, CandidateHash, CoreIndex, GroupIndex, SessionInfo, ValidatorIndex,
+	AssignmentId, AssignmentPair, CandidateHash, CoreIndex, GroupIndex, SessionInfo,
+	ValidatorGroups, ValidatorIndex,
 };
 use sc_keystore::LocalKeystore;
 use sp_application_crypto::ByteArray;
@@ -138,7 +139,7 @@ pub(crate) struct Config {
 	/// The assignment public keys for validators.
 	assignment_keys: Vec<AssignmentId>,
 	/// The groups of validators assigned to each core.
-	validator_groups: Vec<Vec<ValidatorIndex>>,
+	validator_groups: ValidatorGroups,
 	/// The number of availability cores used by the protocol during this session.
 	n_cores: u32,
 	/// The zeroth delay tranche width.
@@ -541,11 +542,11 @@ pub(crate) fn check_assignment_cert(
 }
 
 fn is_in_backing_group(
-	validator_groups: &[Vec<ValidatorIndex>],
+	validator_groups: &ValidatorGroups,
 	validator: ValidatorIndex,
 	group: GroupIndex,
 ) -> bool {
-	validator_groups.get(group.0 as usize).map_or(false, |g| g.contains(&validator))
+	validator_groups.get(group).map_or(false, |g| g.contains(&validator))
 }
 
 #[cfg(test)]
@@ -590,7 +591,7 @@ mod tests {
 			.collect()
 	}
 
-	fn basic_groups(n_validators: usize, n_groups: usize) -> Vec<Vec<ValidatorIndex>> {
+	fn basic_groups(n_validators: usize, n_groups: usize) -> ValidatorGroups {
 		let size = n_validators / n_groups;
 		let big_groups = n_validators % n_groups;
 		let scraps = n_groups * size;
@@ -631,10 +632,10 @@ mod tests {
 					Sr25519Keyring::Bob,
 					Sr25519Keyring::Charlie,
 				]),
-				validator_groups: vec![
+				validator_groups: ValidatorGroups::from(vec![
 					vec![ValidatorIndex(0)],
 					vec![ValidatorIndex(1), ValidatorIndex(2)],
-				],
+				]),
 				n_cores: 2,
 				zeroth_delay_tranche_width: 10,
 				relay_vrf_modulo_samples: 3,
@@ -666,10 +667,10 @@ mod tests {
 					Sr25519Keyring::Bob,
 					Sr25519Keyring::Charlie,
 				]),
-				validator_groups: vec![
+				validator_groups: ValidatorGroups::from(vec![
 					vec![ValidatorIndex(0)],
 					vec![ValidatorIndex(1), ValidatorIndex(2)],
-				],
+				]),
 				n_cores: 2,
 				zeroth_delay_tranche_width: 10,
 				relay_vrf_modulo_samples: 3,
@@ -696,7 +697,7 @@ mod tests {
 					Sr25519Keyring::Bob,
 					Sr25519Keyring::Charlie,
 				]),
-				validator_groups: vec![],
+				validator_groups: Default::default(),
 				n_cores: 0,
 				zeroth_delay_tranche_width: 10,
 				relay_vrf_modulo_samples: 3,

node/core/approval-voting/src/import.rs

@@ -620,7 +620,9 @@ pub(crate) mod tests {
 	use polkadot_node_subsystem::messages::{AllMessages, ApprovalVotingMessage};
 	use polkadot_node_subsystem_test_helpers::make_subsystem_context;
 	use polkadot_node_subsystem_util::database::Database;
-	use polkadot_primitives::v2::{Id as ParaId, SessionInfo, ValidatorIndex};
+	use polkadot_primitives::v2::{
+		Id as ParaId, SessionInfo, ValidatorGroups, ValidatorIndex, Validators,
+	};
 	pub(crate) use sp_consensus_babe::{
 		digests::{CompatibleDigestItem, PreDigest, SecondaryVRFPreDigest},
 		AllowedSlots, BabeEpochConfiguration, Epoch as BabeEpoch,
@@ -713,10 +715,10 @@ pub(crate) mod tests {
 
 	fn dummy_session_info(index: SessionIndex) -> SessionInfo {
 		SessionInfo {
-			validators: Vec::new(),
+			validators: Default::default(),
 			discovery_keys: Vec::new(),
 			assignment_keys: Vec::new(),
-			validator_groups: Vec::new(),
+			validator_groups: Default::default(),
 			n_cores: index as _,
 			zeroth_delay_tranche_width: index as _,
 			relay_vrf_modulo_samples: index as _,
@@ -1175,10 +1177,13 @@ pub(crate) mod tests {
 		let session = 5;
 		let irrelevant = 666;
 		let session_info = SessionInfo {
-			validators: vec![Sr25519Keyring::Alice.public().into(); 6],
+			validators: Validators::from(vec![Sr25519Keyring::Alice.public().into(); 6]),
 			discovery_keys: Vec::new(),
 			assignment_keys: Vec::new(),
-			validator_groups: vec![vec![ValidatorIndex(0); 5], vec![ValidatorIndex(0); 2]],
+			validator_groups: ValidatorGroups::from(vec![
+				vec![ValidatorIndex(0); 5],
+				vec![ValidatorIndex(0); 2],
+			]),
 			n_cores: 6,
 			needed_approvals: 2,
 			zeroth_delay_tranche_width: irrelevant,

node/core/approval-voting/src/lib.rs

@@ -1803,7 +1803,7 @@ fn check_and_import_approval<T>(
 		)),
 	};
 
-	let pubkey = match session_info.validators.get(approval.validator.0 as usize) {
+	let pubkey = match session_info.validators.get(approval.validator) {
 		Some(k) => k,
 		None => respond_early!(ApprovalCheckResult::Bad(
 			ApprovalCheckError::InvalidValidatorIndex(approval.validator),
@@ -2503,7 +2503,7 @@ async fn issue_approval<Context>(
 		},
 	};
 
-	let validator_pubkey = match session_info.validators.get(validator_index.0 as usize) {
+	let validator_pubkey = match session_info.validators.get(validator_index) {
 		Some(p) => p,
 		None => {
 			gum::warn!(

node/core/approval-voting/src/tests.rs

@@ -33,7 +33,7 @@ use polkadot_node_subsystem_util::TimeoutExt;
 use polkadot_overseer::HeadSupportsParachains;
 use polkadot_primitives::v2::{
 	CandidateCommitments, CandidateEvent, CoreIndex, GroupIndex, Header, Id as ParaId,
-	ValidationCode, ValidatorSignature,
+	ValidationCode, ValidatorGroups, ValidatorSignature,
 };
 use std::time::Duration;
 
@@ -739,7 +739,10 @@ fn session_info(keys: &[Sr25519Keyring]) -> SessionInfo {
 		validators: keys.iter().map(|v| v.public().into()).collect(),
 		discovery_keys: keys.iter().map(|v| v.public().into()).collect(),
 		assignment_keys: keys.iter().map(|v| v.public().into()).collect(),
-		validator_groups: vec![vec![ValidatorIndex(0)], vec![ValidatorIndex(1)]],
+		validator_groups: ValidatorGroups::from(vec![
+			vec![ValidatorIndex(0)],
+			vec![ValidatorIndex(1)],
+		]),
 		n_cores: keys.len() as _,
 		needed_approvals: 2,
 		zeroth_delay_tranche_width: 5,
@@ -1552,11 +1555,11 @@ fn subsystem_second_approval_import_only_schedules_wakeups() {
 			Sr25519Keyring::Eve,
 		];
 		let session_info = SessionInfo {
-			validator_groups: vec![
+			validator_groups: ValidatorGroups::from(vec![
 				vec![ValidatorIndex(0), ValidatorIndex(1)],
 				vec![ValidatorIndex(2)],
 				vec![ValidatorIndex(3), ValidatorIndex(4)],
-			],
+			]),
 			needed_approvals: 1,
 			..session_info(&validators)
 		};
@@ -1889,11 +1892,11 @@ fn import_checked_approval_updates_entries_and_schedules() {
 			Sr25519Keyring::Eve,
 		];
 		let session_info = SessionInfo {
-			validator_groups: vec![
+			validator_groups: ValidatorGroups::from(vec![
 				vec![ValidatorIndex(0), ValidatorIndex(1)],
 				vec![ValidatorIndex(2)],
 				vec![ValidatorIndex(3), ValidatorIndex(4)],
-			],
+			]),
 			..session_info(&validators)
 		};
 
@@ -2046,11 +2049,11 @@ fn subsystem_import_checked_approval_sets_one_block_bit_at_a_time() {
 			Sr25519Keyring::Eve,
 		];
 		let session_info = SessionInfo {
-			validator_groups: vec![
+			validator_groups: ValidatorGroups::from(vec![
 				vec![ValidatorIndex(0), ValidatorIndex(1)],
 				vec![ValidatorIndex(2)],
 				vec![ValidatorIndex(3), ValidatorIndex(4)],
-			],
+			]),
 			..session_info(&validators)
 		};
 
@@ -2336,11 +2339,11 @@ fn subsystem_validate_approvals_cache() {
 			Sr25519Keyring::Eve,
 		];
 		let session_info = SessionInfo {
-			validator_groups: vec![
+			validator_groups: ValidatorGroups::from(vec![
 				vec![ValidatorIndex(0), ValidatorIndex(1)],
 				vec![ValidatorIndex(2)],
 				vec![ValidatorIndex(3), ValidatorIndex(4)],
-			],
+			]),
 			..session_info(&validators)
 		};
 
@@ -2548,11 +2551,11 @@ where
 			Sr25519Keyring::Ferdie,
 		];
 		let session_info = SessionInfo {
-			validator_groups: vec![
+			validator_groups: ValidatorGroups::from(vec![
 				vec![ValidatorIndex(0), ValidatorIndex(1)],
 				vec![ValidatorIndex(2), ValidatorIndex(3)],
 				vec![ValidatorIndex(4), ValidatorIndex(5)],
-			],
+			]),
 			relay_vrf_modulo_samples: 2,
 			no_show_slots,
 			..session_info(&validators)
@@ -2868,11 +2871,11 @@ fn pre_covers_dont_stall_approval() {
 			Sr25519Keyring::One,
 		];
 		let session_info = SessionInfo {
-			validator_groups: vec![
+			validator_groups: ValidatorGroups::from(vec![
 				vec![ValidatorIndex(0), ValidatorIndex(1)],
 				vec![ValidatorIndex(2), ValidatorIndex(5)],
 				vec![ValidatorIndex(3), ValidatorIndex(4)],
-			],
+			]),
 			..session_info(&validators)
 		};
 
@@ -3045,11 +3048,11 @@ fn waits_until_approving_assignments_are_old_enough() {
 			Sr25519Keyring::One,
 		];
 		let session_info = SessionInfo {
-			validator_groups: vec![
+			validator_groups: ValidatorGroups::from(vec![
 				vec![ValidatorIndex(0), ValidatorIndex(1)],
 				vec![ValidatorIndex(2), ValidatorIndex(5)],
 				vec![ValidatorIndex(3), ValidatorIndex(4)],
-			],
+			]),
 			..session_info(&validators)
 		};
 

node/core/dispute-coordinator/src/import.rs

@@ -32,7 +32,7 @@ use polkadot_node_primitives::{CandidateVotes, SignedDisputeStatement};
 use polkadot_node_subsystem_util::rolling_session_window::RollingSessionWindow;
 use polkadot_primitives::v2::{
 	CandidateReceipt, DisputeStatement, SessionIndex, SessionInfo, ValidDisputeStatementKind,
-	ValidatorId, ValidatorIndex, ValidatorPair, ValidatorSignature,
+	ValidatorIndex, ValidatorPair, ValidatorSignature, Validators,
 };
 use sc_keystore::LocalKeystore;
 
@@ -63,7 +63,7 @@ impl<'a> CandidateEnvironment<'a> {
 	}
 
 	/// Validators in the candidate's session.
-	pub fn validators(&self) -> &Vec<ValidatorId> {
+	pub fn validators(&self) -> &Validators {
 		&self.session.validators
 	}
 
@@ -229,7 +229,7 @@ impl CandidateVoteState<CandidateVotes> {
 		for (statement, val_index) in statements {
 			if env
 				.validators()
-				.get(val_index.0 as usize)
+				.get(val_index)
 				.map_or(true, |v| v != statement.validator_public())
 			{
 				gum::error!(
@@ -488,7 +488,7 @@ impl ImportResult {
 		for (index, sig) in approval_votes.into_iter() {
 			debug_assert!(
 				{
-					let pub_key = &env.session_info().validators[index.0 as usize];
+					let pub_key = &env.session_info().validators.get(index).expect("<write an actual proof here; qed>");
 					let candidate_hash = votes.candidate_receipt.hash();
 					let session_index = env.session_index();
 					DisputeStatement::Valid(ValidDisputeStatementKind::ApprovalChecking)
@@ -538,7 +538,7 @@ impl ImportResult {
 /// That is all `ValidatorIndex`es we have private keys for. Usually this will only be one.
 fn find_controlled_validator_indices(
 	keystore: &LocalKeystore,
-	validators: &[ValidatorId],
+	validators: &Validators,
 ) -> HashSet<ValidatorIndex> {
 	let mut controlled = HashSet::new();
 	for (index, validator) in validators.iter().enumerate() {

node/core/dispute-coordinator/src/initialized.rs

@@ -372,7 +372,7 @@ impl Initialized {
 				.filter_map(|(validator_index, attestation)| {
 					let validator_public: ValidatorId = session_info
 						.validators
-						.get(validator_index.0 as usize)
+						.get(validator_index)
 						.or_else(|| {
 							gum::error!(
 								target: LOG_TARGET,
@@ -473,7 +473,7 @@ impl Initialized {
 
 					let validator_public: ValidatorId = session_info
 						.validators
-						.get(validator_index.0 as usize)
+						.get(validator_index)
 						.or_else(|| {
 							gum::error!(
 								target: LOG_TARGET,
@@ -903,7 +903,7 @@ impl Initialized {
 			let no_votes = Vec::new();
 			let our_approval_votes = new_state.own_approval_votes().unwrap_or(&no_votes);
 			for (validator_index, sig) in our_approval_votes {
-				let pub_key = match env.validators().get(validator_index.0 as usize) {
+				let pub_key = match env.validators().get(*validator_index) {
 					None => {
 						gum::error!(
 							target: LOG_TARGET,
@@ -1097,7 +1097,7 @@ impl Initialized {
 				valid,
 				candidate_hash,
 				session,
-				env.validators()[index.0 as usize].clone(),
+				env.validators().get(*index).expect("<write an actual proof here; qed>").clone(),
 			)
 			.await;
 
@@ -1238,7 +1238,7 @@ fn make_dispute_message(
 				our_vote.candidate_hash().clone(),
 				our_vote.session_index(),
 				validators
-					.get(validator_index.0 as usize)
+					.get(*validator_index)
 					.ok_or(DisputeMessageCreationError::InvalidValidatorIndex)?
 					.clone(),
 				validator_signature.clone(),
@@ -1253,7 +1253,7 @@ fn make_dispute_message(
 				our_vote.candidate_hash().clone(),
 				our_vote.session_index(),
 				validators
-					.get(validator_index.0 as usize)
+					.get(*validator_index)
 					.ok_or(DisputeMessageCreationError::InvalidValidatorIndex)?
 					.clone(),
 				validator_signature.clone(),

node/core/dispute-coordinator/src/tests.rs

@@ -59,8 +59,8 @@ use polkadot_node_subsystem_test_helpers::{make_subsystem_context, TestSubsystem
 use polkadot_primitives::v2::{
 	ApprovalVote, BlockNumber, CandidateCommitments, CandidateHash, CandidateReceipt,
 	DisputeStatement, Hash, Header, MultiDisputeStatementSet, ScrapedOnChainVotes, SessionIndex,
-	SessionInfo, SigningContext, ValidDisputeStatementKind, ValidatorId, ValidatorIndex,
-	ValidatorSignature,
+	SessionInfo, SigningContext, ValidDisputeStatementKind, ValidatorGroups, ValidatorId,
+	ValidatorIndex, ValidatorSignature, Validators,
 };
 
 use crate::{
@@ -125,8 +125,8 @@ impl MockClock {
 
 struct TestState {
 	validators: Vec<Pair>,
-	validator_public: Vec<ValidatorId>,
-	validator_groups: Vec<Vec<ValidatorIndex>>,
+	validator_public: Validators,
+	validator_groups: ValidatorGroups,
 	master_keystore: Arc<sc_keystore::LocalKeystore>,
 	subsystem_keystore: Arc<sc_keystore::LocalKeystore>,
 	db: Arc<dyn Database>,
@@ -163,11 +163,11 @@ impl Default for TestState {
 			.map(|k| ValidatorId::from(k.0.public()))
 			.collect();
 
-		let validator_groups = vec![
+		let validator_groups = ValidatorGroups::from(vec![
 			vec![ValidatorIndex(0), ValidatorIndex(1)],
 			vec![ValidatorIndex(2), ValidatorIndex(3)],
 			vec![ValidatorIndex(4), ValidatorIndex(5), ValidatorIndex(6)],
-		];
+		]);
 
 		let master_keystore = make_keystore(validators.iter().map(|v| v.1.clone())).into();
 		let subsystem_keystore =
@@ -431,7 +431,7 @@ impl TestState {
 		session: SessionIndex,
 		valid: bool,
 	) -> SignedDisputeStatement {
-		let public = self.validator_public[index.0 as usize].clone();
+		let public = self.validator_public.get(index).unwrap().clone();
 
 		let keystore = self.master_keystore.clone() as SyncCryptoStorePtr;