Use a more typesafe approach for managing indexed data (#6150)

* Fix for issue #2403 * Nightly fmt * Quick documentation fixes * Default Implementation * iter() function integrated * Implemented iter functionalities * Fmt * small change * updates node-network * updates in dispute-coordinator * Updates * benchmarking fix * minor fix * test fixes in runtime api * Update primitives/src/v2/mod.rs Co-authored-by: Andronik <[email protected]> * Update primitives/src/v2/mod.rs Co-authored-by: Andronik <[email protected]> * Update primitives/src/v2/mod.rs Co-authored-by: Andronik <[email protected]> * Update primitives/src/v2/mod.rs Co-authored-by: Andronik <[email protected]> * Update primitives/src/v2/mod.rs Co-authored-by: Andronik <[email protected]> * Removal of [index], shorting of FromIterator, Renaming of GroupValidators to ValidatorGroups * Removal of ops import * documentation fixes for spell check * implementation of generic type * Refactoring * Test and documentation fixes * minor test fix * minor test fix * minor test fix * Update node/network/statement-distribution/src/lib.rs Co-authored-by: Andronik <[email protected]> * Update primitives/src/v2/mod.rs Co-authored-by: Andronik <[email protected]> * Update primitives/src/v2/mod.rs Co-authored-by: Andronik <[email protected]> * removed IterMut * Update node/core/dispute-coordinator/src/import.rs Co-authored-by: Andronik <[email protected]> * Update node/core/dispute-coordinator/src/initialized.rs Co-authored-by: Andronik <[email protected]> * Update primitives/src/v2/mod.rs Co-authored-by: Andronik <[email protected]> * fmt * IterMut * documentation update Co-authored-by: Andronik <[email protected]> * minor adjustments and new TypeIndex trait * spelling fix * TypeIndex fix Co-authored-by: Andronik <[email protected]>
paritytech · Oct 22, 2022 · a060975 · a060975
1 parent 0fd106c
commit a060975
Show file tree

Hide file tree

Showing 28 changed files with 261 additions and 138 deletions.
diff --git a/node/core/approval-voting/src/criteria.rs b/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, IndexedVec, SessionInfo,
+	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: IndexedVec<GroupIndex, Vec<ValidatorIndex>>,
 	/// 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: &IndexedVec<GroupIndex, Vec<ValidatorIndex>>,
 	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,10 @@ mod tests {
 			.collect()
 	}
 
-	fn basic_groups(n_validators: usize, n_groups: usize) -> Vec<Vec<ValidatorIndex>> {
+	fn basic_groups(
+		n_validators: usize,
+		n_groups: usize,
+	) -> IndexedVec<GroupIndex, Vec<ValidatorIndex>> {
 		let size = n_validators / n_groups;
 		let big_groups = n_validators % n_groups;
 		let scraps = n_groups * size;
@@ -631,10 +635,10 @@ mod tests {
 					Sr25519Keyring::Bob,
 					Sr25519Keyring::Charlie,
 				]),
-				validator_groups: vec![
+				validator_groups: IndexedVec::<GroupIndex, Vec<ValidatorIndex>>::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 +670,10 @@ mod tests {
 					Sr25519Keyring::Bob,
 					Sr25519Keyring::Charlie,
 				]),
-				validator_groups: vec![
+				validator_groups: IndexedVec::<GroupIndex, Vec<ValidatorIndex>>::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 +700,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,

diff --git a/node/core/approval-voting/src/import.rs b/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, IndexedVec, SessionInfo, ValidatorId, ValidatorIndex,
+	};
 	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 _,
@@ -1174,21 +1176,27 @@ pub(crate) mod tests {
 
 		let session = 5;
 		let irrelevant = 666;
-		let session_info = SessionInfo {
-			validators: 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]],
-			n_cores: 6,
-			needed_approvals: 2,
-			zeroth_delay_tranche_width: irrelevant,
-			relay_vrf_modulo_samples: irrelevant,
-			n_delay_tranches: irrelevant,
-			no_show_slots: irrelevant,
-			active_validator_indices: Vec::new(),
-			dispute_period: 6,
-			random_seed: [0u8; 32],
-		};
+		let session_info =
+			SessionInfo {
+				validators: IndexedVec::<ValidatorIndex, ValidatorId>::from(
+					vec![Sr25519Keyring::Alice.public().into(); 6],
+				),
+				discovery_keys: Vec::new(),
+				assignment_keys: Vec::new(),
+				validator_groups: IndexedVec::<GroupIndex, Vec<ValidatorIndex>>::from(vec![
+					vec![ValidatorIndex(0); 5],
+					vec![ValidatorIndex(0); 2],
+				]),
+				n_cores: 6,
+				needed_approvals: 2,
+				zeroth_delay_tranche_width: irrelevant,
+				relay_vrf_modulo_samples: irrelevant,
+				n_delay_tranches: irrelevant,
+				no_show_slots: irrelevant,
+				active_validator_indices: Vec::new(),
+				dispute_period: 6,
+				random_seed: [0u8; 32],
+			};
 
 		let slot = Slot::from(10);
 

diff --git a/node/core/approval-voting/src/lib.rs b/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!(

diff --git a/node/core/approval-voting/src/tests.rs b/node/core/approval-voting/src/tests.rs
@@ -32,7 +32,7 @@ use polkadot_node_subsystem_test_helpers as test_helpers;
 use polkadot_node_subsystem_util::TimeoutExt;
 use polkadot_overseer::HeadSupportsParachains;
 use polkadot_primitives::v2::{
-	CandidateCommitments, CandidateEvent, CoreIndex, GroupIndex, Header, Id as ParaId,
+	CandidateCommitments, CandidateEvent, CoreIndex, GroupIndex, Header, Id as ParaId, IndexedVec,
 	ValidationCode, 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: IndexedVec::<GroupIndex, Vec<ValidatorIndex>>::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: IndexedVec::<GroupIndex, Vec<ValidatorIndex>>::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: IndexedVec::<GroupIndex, Vec<ValidatorIndex>>::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: IndexedVec::<GroupIndex, Vec<ValidatorIndex>>::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: IndexedVec::<GroupIndex, Vec<ValidatorIndex>>::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: IndexedVec::<GroupIndex, Vec<ValidatorIndex>>::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: IndexedVec::<GroupIndex, Vec<ValidatorIndex>>::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: IndexedVec::<GroupIndex, Vec<ValidatorIndex>>::from(vec![
 				vec![ValidatorIndex(0), ValidatorIndex(1)],
 				vec![ValidatorIndex(2), ValidatorIndex(5)],
 				vec![ValidatorIndex(3), ValidatorIndex(4)],
-			],
+			]),
 			..session_info(&validators)
 		};
 

diff --git a/node/core/dispute-coordinator/src/import.rs b/node/core/dispute-coordinator/src/import.rs
@@ -31,8 +31,8 @@ use std::collections::{BTreeMap, HashMap, HashSet};
 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,
+	CandidateReceipt, DisputeStatement, IndexedVec, SessionIndex, SessionInfo,
+	ValidDisputeStatementKind, ValidatorId, ValidatorIndex, ValidatorPair, ValidatorSignature,
 };
 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) -> &IndexedVec<ValidatorIndex, ValidatorId> {
 		&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("indices are validated by approval-voting subsystem; 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: &IndexedVec<ValidatorIndex, ValidatorId>,
 ) -> HashSet<ValidatorIndex> {
 	let mut controlled = HashSet::new();
 	for (index, validator) in validators.iter().enumerate() {

diff --git a/node/core/dispute-coordinator/src/initialized.rs b/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,10 @@ impl Initialized {
 				valid,
 				candidate_hash,
 				session,
-				env.validators()[index.0 as usize].clone(),
+				env.validators()
+					.get(*index)
+					.expect("`controlled_indices` are derived from `validators`; qed")
+					.clone(),
 			)
 			.await;
 
@@ -1238,7 +1241,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 +1256,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(),