clarify candidate selection algorithm

Cargo.lock

@@ -4701,6 +4701,7 @@ dependencies = [
  "log 0.4.8",
  "polkadot-node-subsystem",
  "polkadot-primitives",
+ "sp-core",
 ]
 
 [[package]]

node/core/provisioner/Cargo.toml

@@ -11,3 +11,6 @@ futures = "0.3.5"
 log = "0.4.8"
 polkadot-primitives = { path = "../../../primitives" }
 polkadot-node-subsystem = { path = "../../subsystem" }
+
+[dev-dependencies]
+sp-core = { git = "https://github.com/paritytech/substrate", branch = "master" }

node/core/provisioner/src/lib.rs

@@ -31,10 +31,10 @@ use polkadot_node_subsystem::{
 		AllMessages, ChainApiMessage, ProvisionableData, ProvisionerInherentData,
 		ProvisionerMessage, RuntimeApiMessage,
 	},
-	util::{self, request_availability_cores, JobTrait, ToJobTrait},
+	util::{self, request_availability_cores, request_global_validation_data, request_local_validation_data, JobTrait, ToJobTrait},
 };
 use polkadot_primitives::v1::{
-	BackedCandidate, BlockNumber, CoreState, Hash, OccupiedCore, OccupiedCoreAssumption,
+	BackedCandidate, BlockNumber, CoreState, GlobalValidationData, LocalValidationData,Hash, OccupiedCore, OccupiedCoreAssumption,
 	ScheduledCore, SignedAvailabilityBitfield,
 };
 use std::{collections::HashMap, convert::TryFrom, pin::Pin};
@@ -244,6 +244,7 @@ type CoreAvailability = BitVec<bitvec::order::Lsb0, u8>;
 
 // preprocessing the cores involves a bit more data than is comfortable in a tuple, so let's make a struct of it
 struct PreprocessedCore {
+	relay_parent: Hash,
 	assumption: OccupiedCoreAssumption,
 	scheduled_core: ScheduledCore,
 	availability: Option<CoreAvailability>,
@@ -252,13 +253,14 @@ struct PreprocessedCore {
 }
 
 impl PreprocessedCore {
-	fn new(idx: usize, core: CoreState) -> Option<Self> {
+	fn new(relay_parent: Hash, idx: usize, core: CoreState) -> Option<Self> {
 		match core {
 			CoreState::Occupied(OccupiedCore {
 				availability,
 				next_up_on_available: Some(scheduled_core),
 				..
 			}) => Some(Self {
+				relay_parent,
 				assumption: OccupiedCoreAssumption::Included,
 				scheduled_core,
 				availability: Some(availability),
@@ -271,6 +273,7 @@ impl PreprocessedCore {
 				time_out_at,
 				..
 			}) => Some(Self {
+				relay_parent,
 				assumption: OccupiedCoreAssumption::TimedOut,
 				scheduled_core,
 				availability: Some(availability),
@@ -278,6 +281,7 @@ impl PreprocessedCore {
 				idx,
 			}),
 			CoreState::Scheduled(scheduled_core) => Some(Self {
+				relay_parent,
 				assumption: OccupiedCoreAssumption::Free,
 				scheduled_core,
 				availability: None,
@@ -304,11 +308,20 @@ impl PreprocessedCore {
 		bitfields: &[SignedAvailabilityBitfield],
 		candidates: &[BackedCandidate],
 		block_number: BlockNumber,
+		sender: &mut mpsc::Sender<FromJob>,
 	) -> Option<BackedCandidate> {
+		// the validation data hash must match under the appropriate occupied core assumption.
+		// to compute the validation data hash, we need both global and local validation data.
+		let global_validation_data = request_global_validation_data(self.relay_parent, sender).await.ok()?.await.ok()?.ok()?;
+
 		// choose only one per parachain
 		candidates
 			.iter()
 			.find(|candidate| candidate.candidate.descriptor.para_id == self.scheduled_core.para_id)
+			.filter(|candidate| {
+				let local_validation_data = request_local_validation_data(self.relay_parent, self.scheduled_core.para_id, self.assumption).await.ok()?.await.ok()?.ok()?;
+
+			})
 			.map(|candidate| {
 				match (self.assumption, self.availability.as_ref(), self.timeout) {
 					(OccupiedCoreAssumption::Free, _, _) => {
@@ -511,3 +524,44 @@ fn merged_bitfields_are_gte_two_thirds(
 }
 
 delegated_subsystem!(ProvisioningJob(()) <- ToJob as ProvisioningSubsystem);
+
+#[cfg(test)]
+mod tests {
+	mod select_availability_bitfields {
+		use bitvec::bitvec;
+		use polkadot_primitives::v1::{
+			GroupIndex, Id as ParaId, ValidatorPair
+		};
+		use sp_core::crypto::Pair;
+		use super::super::*;
+
+		fn occupied_core(para_id: ParaId, group_responsible: GroupIndex) -> CoreState {
+			CoreState::Occupied(OccupiedCore {
+				para_id,
+				group_responsible,
+				next_up_on_available: None,
+				occupied_since: 100_u32,
+				time_out_at: 200_u32,
+				next_up_on_time_out: None,
+				availability: bitvec![bitvec::order::Lsb0, u8; 0; 32],
+			})
+		}
+
+		fn signed_bitfield(field: CoreAvailability, validator: &ValidatorPair, ) -> SignedAvailabilityBitfield {
+			unimplemented!()
+		}
+
+		#[test]
+		fn not_more_than_one_per_validator() {
+			let validator = ValidatorPair::generate().0;
+			let cores = vec![occupied_core(0.into(), 0.into()), occupied_core(1.into(), 1.into())];
+			let bitfields = vec![];
+			unimplemented!()
+		}
+
+		#[test]
+		fn each_corresponds_to_an_occupied_core() {
+			unimplemented!()
+		}
+	}
+}

node/subsystem/src/util.rs

@@ -173,6 +173,36 @@ where
 	}).await
 }
 
+/// Request global validation data.
+pub async fn request_global_validation_data<FromJob>(
+	parent: Hash,
+	s: &mut mpsc::Sender<FromJob>,
+) -> Result<RuntimeApiReceiver<GlobalValidationData>, Error>
+where
+	FromJob: TryFrom<AllMessages>,
+	<FromJob as TryFrom<AllMessages>>::Error: std::fmt::Debug,
+{
+	request_from_runtime(parent, s, |tx| {
+		RuntimeApiRequest::GlobalValidationData(tx)
+	}).await
+}
+
+/// Request local validation data.
+pub async fn request_local_validation_Data<FromJob>(
+	parent: Hash,
+	para_id: ParaId,
+	assumption: OccupiedCoreAssumption,
+	s: &mut mpsc::Sender<FromJob>,
+) -> Result<RuntimeApiReceiver<LocalValidationData>, Error>
+where
+	FromJob: TryFrom<AllMessages>,
+	<FromJob as TryFrom<AllMessages>>::Error: std::fmt::Debug,
+{
+	request_from_runtime(parent, s, |tx| {
+		RuntimeApiRequest::LocalValidationData(para_id, assumption, tx)
+	}).await
+}
+
 /// From the given set of validators, find the first key we can sign with, if any.
 pub fn signing_key(validators: &[ValidatorId], keystore: &KeyStorePtr) -> Option<ValidatorPair> {
 	let keystore = keystore.read();

roadmap/implementers-guide/src/node/utility/provisioner.md

@@ -53,16 +53,17 @@ Beyond that, a semi-arbitrary selection policy is fine. In order to meet the goa
 
 ### Candidate Selection
 
-The goal of candidate selection is to determine which cores are available, and then to the degree possible, pick a candidate appropriate to each available core.
+The goal of candidate selection is to determine which cores are free, and then to the degree possible, pick a candidate appropriate to each available core.
 
 To determine availability:
 
 - Get the list of core states from the runtime API
 - For each core state:
-  - If the core is scheduled, then it usable; we can make an `OccupiedCoreAssumption::Free`; it is available.
-  - If the core is currently occupied, then we can make some assumptions:
-    - If there is a scheduled `next_up_on_available`, then we can make an `OccupiedCoreAssumption::Included`. This only works if the bitfields indicate availability; more on that later.
-    - If there is a scheduled `next_up_on_time_out`, and `occupied_core.time_out_at == block_number_under_production`, then we can make an `OccupiedCoreAssumption::TimedOut`. This only works if the bitfields do not indicate availability.
+  - On `CoreState::Scheduled`, then we can make an `OccupiedCoreAssumption::Free`.
+  - On `CoreState::Occupied`, then we may be able to make an assumption:
+    - If the bitfields indicate availability and there is a scheduled `next_up_on_available`, then we can make an `OccupiedCoreAssumption::Included`.
+    - If the bitfields do not indicate availability, and there is a scheduled `next_up_on_time_out`, and `occupied_core.time_out_at == block_number_under_production`, then we can make an `OccupiedCoreAssumption::TimedOut`.
+  - If we did not make an `OccupiedCoreAssumption`, then continue on to the next core.
   - Now compute the core's `validation_data_hash`: get the `LocalValidationData` from the runtime, given the known `ParaId` and `OccupiedCoreAssumption`; this can be combined with a cached `GlobalValidationData` to compute the hash.
   - Find an appropriate candidate for the core.
     - There are two constraints: `backed_candidate.candidate.descriptor.para_id == scheduled_core.para_id && candidate.candidate.descriptor.validation_data_hash == computed_validation_data_hash`.