[3 / 5] Move crypto checks in the approval-distribution

polkadot/node/core/approval-voting/benches/approval-voting-regression-bench.rs

@@ -82,8 +82,8 @@ fn main() -> Result<(), String> {
 		("Sent to peers", 63995.2200, 0.01),
 	]));
 	messages.extend(average_usage.check_cpu_usage(&[
-		("approval-distribution", 6.3912, 0.1),
-		("approval-voting", 10.0578, 0.1),
+		("approval-distribution", 12.2736, 0.1),
+		("approval-voting", 2.7174, 0.1),
 	]));
 
 	if messages.is_empty() {

polkadot/node/core/approval-voting/src/approval_checking.rs

@@ -22,9 +22,9 @@ use polkadot_primitives::ValidatorIndex;
 
 use crate::{
 	persisted_entries::{ApprovalEntry, CandidateEntry, TrancheEntry},
-	time::Tick,
 	MAX_RECORDED_NO_SHOW_VALIDATORS_PER_CANDIDATE,
 };
+use polkadot_node_primitives::approval::time::Tick;
 
 /// Result of counting the necessary tranches needed for approving a block.
 #[derive(Debug, PartialEq, Clone)]
@@ -1195,9 +1195,9 @@ mod tests {
 	struct NoShowTest {
 		assignments: Vec<(ValidatorIndex, Tick)>,
 		approvals: Vec<usize>,
-		clock_drift: crate::time::Tick,
-		no_show_duration: crate::time::Tick,
-		drifted_tick_now: crate::time::Tick,
+		clock_drift: Tick,
+		no_show_duration: Tick,
+		drifted_tick_now: Tick,
 		exp_no_shows: usize,
 		exp_next_no_show: Option<u64>,
 	}

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

@@ -16,18 +16,21 @@
 
 //! Assignment criteria VRF generation and checking.
 
-use codec::{Decode, Encode};
+use codec::Encode;
 use itertools::Itertools;
+pub use polkadot_node_primitives::approval::criteria::{
+	AssignmentCriteria, Config, InvalidAssignment, InvalidAssignmentReason, OurAssignment,
+};
 use polkadot_node_primitives::approval::{
 	self as approval_types,
 	v1::{AssignmentCert, AssignmentCertKind, DelayTranche, RelayVRFStory},
 	v2::{
 		AssignmentCertKindV2, AssignmentCertV2, CoreBitfield, VrfPreOutput, VrfProof, VrfSignature,
 	},
 };
+
 use polkadot_primitives::{
-	AssignmentId, AssignmentPair, CandidateHash, CoreIndex, GroupIndex, IndexedVec, SessionInfo,
-	ValidatorIndex,
+	AssignmentPair, CandidateHash, CoreIndex, GroupIndex, IndexedVec, ValidatorIndex,
 };
 use rand::{seq::SliceRandom, SeedableRng};
 use rand_chacha::ChaCha20Rng;
@@ -44,56 +47,19 @@ use std::{
 
 use super::LOG_TARGET;
 
-/// Details pertaining to our assignment on a block.
-#[derive(Debug, Clone, Encode, Decode, PartialEq)]
-pub struct OurAssignment {
-	cert: AssignmentCertV2,
-	tranche: DelayTranche,
-	validator_index: ValidatorIndex,
-	// Whether the assignment has been triggered already.
-	triggered: bool,
-}
-
-impl OurAssignment {
-	pub fn cert(&self) -> &AssignmentCertV2 {
-		&self.cert
-	}
-
-	pub fn tranche(&self) -> DelayTranche {
-		self.tranche
-	}
-
-	pub(crate) fn validator_index(&self) -> ValidatorIndex {
-		self.validator_index
-	}
-
-	pub(crate) fn triggered(&self) -> bool {
-		self.triggered
-	}
-
-	pub(crate) fn mark_triggered(&mut self) {
-		self.triggered = true;
-	}
-}
-
 impl From<crate::approval_db::v2::OurAssignment> for OurAssignment {
 	fn from(entry: crate::approval_db::v2::OurAssignment) -> Self {
-		OurAssignment {
-			cert: entry.cert,
-			tranche: entry.tranche,
-			validator_index: entry.validator_index,
-			triggered: entry.triggered,
-		}
+		OurAssignment::new(entry.cert, entry.tranche, entry.validator_index, entry.triggered)
 	}
 }
 
 impl From<OurAssignment> for crate::approval_db::v2::OurAssignment {
 	fn from(entry: OurAssignment) -> Self {
 		Self {
-			cert: entry.cert,
-			tranche: entry.tranche,
-			validator_index: entry.validator_index,
-			triggered: entry.triggered,
+			tranche: entry.tranche(),
+			validator_index: entry.validator_index(),
+			triggered: entry.triggered(),
+			cert: entry.into_cert(),
 		}
 	}
 }
@@ -223,60 +189,7 @@ fn assigned_core_transcript(core_index: CoreIndex) -> Transcript {
 	t
 }
 
-/// Information about the world assignments are being produced in.
-#[derive(Clone, Debug)]
-pub struct Config {
-	/// The assignment public keys for validators.
-	assignment_keys: Vec<AssignmentId>,
-	/// The groups of validators assigned to each core.
-	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.
-	zeroth_delay_tranche_width: u32,
-	/// The number of samples we do of `relay_vrf_modulo`.
-	relay_vrf_modulo_samples: u32,
-	/// The number of delay tranches in total.
-	n_delay_tranches: u32,
-}
-
-impl<'a> From<&'a SessionInfo> for Config {
-	fn from(s: &'a SessionInfo) -> Self {
-		Config {
-			assignment_keys: s.assignment_keys.clone(),
-			validator_groups: s.validator_groups.clone(),
-			n_cores: s.n_cores,
-			zeroth_delay_tranche_width: s.zeroth_delay_tranche_width,
-			relay_vrf_modulo_samples: s.relay_vrf_modulo_samples,
-			n_delay_tranches: s.n_delay_tranches,
-		}
-	}
-}
-
-/// A trait for producing and checking assignments. Used to mock.
-pub(crate) trait AssignmentCriteria {
-	fn compute_assignments(
-		&self,
-		keystore: &LocalKeystore,
-		relay_vrf_story: RelayVRFStory,
-		config: &Config,
-		leaving_cores: Vec<(CandidateHash, CoreIndex, GroupIndex)>,
-		enable_v2_assignments: bool,
-	) -> HashMap<CoreIndex, OurAssignment>;
-
-	fn check_assignment_cert(
-		&self,
-		claimed_core_bitfield: CoreBitfield,
-		validator_index: ValidatorIndex,
-		config: &Config,
-		relay_vrf_story: RelayVRFStory,
-		assignment: &AssignmentCertV2,
-		// Backing groups for each "leaving core".
-		backing_groups: Vec<GroupIndex>,
-	) -> Result<DelayTranche, InvalidAssignment>;
-}
-
-pub(crate) struct RealAssignmentCriteria;
+pub struct RealAssignmentCriteria;
 
 impl AssignmentCriteria for RealAssignmentCriteria {
 	fn compute_assignments(
@@ -469,12 +382,12 @@ fn compute_relay_vrf_modulo_assignments_v1(
 			};
 
 			// All assignments of type RelayVRFModulo have tranche 0.
-			assignments.entry(core).or_insert(OurAssignment {
-				cert: cert.into(),
-				tranche: 0,
+			assignments.entry(core).or_insert(OurAssignment::new(
+				cert.into(),
+				0,
 				validator_index,
-				triggered: false,
-			});
+				false,
+			));
 		}
 	}
 }
@@ -549,7 +462,7 @@ fn compute_relay_vrf_modulo_assignments_v2(
 		};
 
 		// All assignments of type RelayVRFModulo have tranche 0.
-		OurAssignment { cert, tranche: 0, validator_index, triggered: false }
+		OurAssignment::new(cert, 0, validator_index, false)
 	}) {
 		for core_index in assigned_cores {
 			assignments.insert(core_index, assignment.clone());
@@ -583,15 +496,15 @@ fn compute_relay_vrf_delay_assignments(
 			},
 		};
 
-		let our_assignment = OurAssignment { cert, tranche, validator_index, triggered: false };
+		let our_assignment = OurAssignment::new(cert, tranche, validator_index, false);
 
 		let used = match assignments.entry(core) {
 			Entry::Vacant(e) => {
 				let _ = e.insert(our_assignment);
 				true
 			},
 			Entry::Occupied(mut e) =>
-				if e.get().tranche > our_assignment.tranche {
+				if e.get().tranche() > our_assignment.tranche() {
 					e.insert(our_assignment);
 					true
 				} else {
@@ -612,35 +525,6 @@ fn compute_relay_vrf_delay_assignments(
 	}
 }
 
-/// Assignment invalid.
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub struct InvalidAssignment(pub(crate) InvalidAssignmentReason);
-
-impl std::fmt::Display for InvalidAssignment {
-	fn fmt(&self, f: &mut std::fmt::Formatter) -> std::fmt::Result {
-		write!(f, "Invalid Assignment: {:?}", self.0)
-	}
-}
-
-impl std::error::Error for InvalidAssignment {}
-
-/// Failure conditions when checking an assignment cert.
-#[derive(Debug, Clone, Copy, PartialEq, Eq)]
-pub(crate) enum InvalidAssignmentReason {
-	ValidatorIndexOutOfBounds,
-	SampleOutOfBounds,
-	CoreIndexOutOfBounds,
-	InvalidAssignmentKey,
-	IsInBackingGroup,
-	VRFModuloCoreIndexMismatch,
-	VRFModuloOutputMismatch,
-	VRFDelayCoreIndexMismatch,
-	VRFDelayOutputMismatch,
-	InvalidArguments,
-	/// Assignment vrf check resulted in 0 assigned cores.
-	NullAssignment,
-}
-
 /// Checks the crypto of an assignment cert. Failure conditions:
 ///   * Validator index out of bounds
 ///   * VRF signature check fails
@@ -820,21 +704,21 @@ fn is_in_backing_group(
 /// Migration helpers.
 impl From<crate::approval_db::v1::OurAssignment> for OurAssignment {
 	fn from(value: crate::approval_db::v1::OurAssignment) -> Self {
-		Self {
-			cert: value.cert.into(),
-			tranche: value.tranche,
-			validator_index: value.validator_index,
+		Self::new(
+			value.cert.into(),
+			value.tranche,
+			value.validator_index,
 			// Whether the assignment has been triggered already.
-			triggered: value.triggered,
-		}
+			value.triggered,
+		)
 	}
 }
 
 #[cfg(test)]
 mod tests {
 	use super::*;
 	use crate::import::tests::garbage_vrf_signature;
-	use polkadot_primitives::{Hash, ASSIGNMENT_KEY_TYPE_ID};
+	use polkadot_primitives::{AssignmentId, Hash, ASSIGNMENT_KEY_TYPE_ID};
 	use sp_application_crypto::sr25519;
 	use sp_core::crypto::Pair as PairT;
 	use sp_keyring::sr25519::Keyring as Sr25519Keyring;
@@ -1053,7 +937,7 @@ mod tests {
 
 		let mut counted = 0;
 		for (core, assignment) in assignments {
-			let cores = match assignment.cert.kind.clone() {
+			let cores = match assignment.cert().kind.clone() {
 				AssignmentCertKindV2::RelayVRFModuloCompact { core_bitfield } => core_bitfield,
 				AssignmentCertKindV2::RelayVRFModulo { sample: _ } => core.into(),
 				AssignmentCertKindV2::RelayVRFDelay { core_index } => core_index.into(),
@@ -1062,7 +946,7 @@ mod tests {
 			let mut mutated = MutatedAssignment {
 				cores: cores.clone(),
 				groups: cores.iter_ones().map(|core| group_for_core(core)).collect(),
-				cert: assignment.cert,
+				cert: assignment.into_cert(),
 				own_group: GroupIndex(0),
 				val_index: ValidatorIndex(0),
 				config: config.clone(),

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

@@ -62,9 +62,10 @@ use crate::{
 	criteria::{AssignmentCriteria, OurAssignment},
 	get_extended_session_info, get_session_info,
 	persisted_entries::CandidateEntry,
-	time::{slot_number_to_tick, Tick},
 };
 
+use polkadot_node_primitives::approval::time::{slot_number_to_tick, Tick};
+
 use super::{State, LOG_TARGET};
 
 #[derive(Debug)]
@@ -574,9 +575,13 @@ pub(crate) async fn handle_new_head<Context, B: Backend>(
 			hash: block_hash,
 			number: block_header.number,
 			parent_hash: block_header.parent_hash,
-			candidates: included_candidates.iter().map(|(hash, _, _, _)| *hash).collect(),
+			candidates: included_candidates
+				.iter()
+				.map(|(hash, _, core_index, group_index)| (*hash, *core_index, *group_index))
+				.collect(),
 			slot,
 			session: session_index,
+			vrf_story: relay_vrf_story,
 		});
 
 		imported_candidates.push(BlockImportedCandidates {
@@ -609,6 +614,7 @@ pub(crate) mod tests {
 		approval_db::common::{load_block_entry, DbBackend},
 		RuntimeInfo, RuntimeInfoConfig, MAX_BLOCKS_WITH_ASSIGNMENT_TIMESTAMPS,
 	};
+	use approval_types::time::Clock;
 	use assert_matches::assert_matches;
 	use polkadot_node_primitives::{
 		approval::v1::{VrfSignature, VrfTranscript},
@@ -642,7 +648,7 @@ pub(crate) mod tests {
 	#[derive(Default)]
 	struct MockClock;
 
-	impl crate::time::Clock for MockClock {
+	impl Clock for MockClock {
 		fn tick_now(&self) -> Tick {
 			42 // chosen by fair dice roll
 		}