[3 / 5] Move crypto checks in the approval-distribution (#4928)

# Prerequisite 
This is part of the work to further optimize the approval subsystems, if
you want to understand the full context start with reading
#4849 (comment),

# Description
This PR contain changes, so that the crypto checks are performed by the
approval-distribution subsystem instead of the approval-voting one. The
benefit for these, is twofold:
1. Approval-distribution won't have to wait every single time for the
approval-voting to finish its job, so the work gets to be pipelined
between approval-distribution and approval-voting.

2. By running in parallel multiple instances of approval-distribution as
described here
#4849 (comment),
this significant body of work gets to run in parallel.

## Changes:
1. When approval-voting send `ApprovalDistributionMessage::NewBlocks` it
needs to pass the core_index and candidate_hash of the candidates.
2. ApprovalDistribution needs to use `RuntimeInfo` to be able to fetch
the SessionInfo from the runtime.
3. Move `approval-voting` logic that checks VRF assignment into
`approval-distribution`
4. Move `approval-voting` logic that checks vote is correctly signed
into `approval-distribution`
5. Plumb `approval-distribution` and `approval-voting` tests to support
the new logic.

## Benefits
Even without parallelisation the gains are significant, for example on
my machine if we run approval subsystem bench for 500 validators and 100
cores and trigger all 89 tranches of assignments and approvals, the
system won't fall behind anymore because of late processing of messages.
```
Before change
Chain selection approved  after 11500 ms hash=0x0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a

After change

Chain selection approved  after 5500 ms hash=0x0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a0a
```

## TODO:
- [x] Run on versi.
- [x] Update parachain host documentation.

---------

Signed-off-by: Alexandru Gheorghe <[email protected]>

Author: alexggh

Cargo.lock

@@ -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/benches/approval-voting-regression-bench.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/approval_checking.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/criteria.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
 		}