Make collation an optional return (#1787)

This pr changes the collator interface function to return an optional
collation instead of a collation. This is required as the parachain
itself can fail to generate a valid collation for various reason. Now if
the collation fails it will return `None`.

Besides that the pr adds some `RuntimeDebug` derive for `ValidationData`
and removes some whitespaces.

polkadot/node/collation-generation/src/lib.rs

@@ -233,7 +233,17 @@ async fn handle_new_activations<Context: SubsystemContext>(
 			ctx.spawn("collation generation collation builder", Box::pin(async move {
 				let persisted_validation_data_hash = validation_data.persisted.hash();
 
-				let collation = (task_config.collator)(&validation_data).await;
+				let collation = match (task_config.collator)(&validation_data).await {
+					Some(collation) => collation,
+					None => {
+						log::debug!(
+							target: LOG_TARGET,
+							"collator returned no collation on collate for para_id {}.",
+							scheduled_core.para_id,
+						);
+						return
+					}
+				};
 
 				let pov_hash = collation.proof_of_validity.hash();
 
@@ -384,10 +394,10 @@ mod tests {
 		struct TestCollator;
 
 		impl Future for TestCollator {
-			type Output = Collation;
+			type Output = Option<Collation>;
 
 			fn poll(self: Pin<&mut Self>, _cx: &mut FuturesContext) -> Poll<Self::Output> {
-				Poll::Ready(test_collation())
+				Poll::Ready(Some(test_collation()))
 			}
 		}
 

polkadot/node/network/collator-protocol/src/collator_side.rs

@@ -108,11 +108,11 @@ struct State {
 	/// We will keep up to one local collation per relay-parent.
 	collations: HashMap<Hash, (CandidateReceipt, PoV)>,
 
-	/// Our validator groups active leafs. 
+	/// Our validator groups active leafs.
 	our_validators_groups: HashMap<Hash, Vec<ValidatorId>>,
 
 	/// Validators we know about via `ConnectToValidators` message.
-	/// 
+	///
 	/// These are the only validators we are interested in talking to and as such
 	/// all actions from peers not in this map will be ignored.
 	/// Entries in this map will be cleared as validator groups in `our_validator_groups`
@@ -124,7 +124,7 @@ struct State {
 }
 
 /// Distribute a collation.
-/// 
+///
 /// Figure out the core our para is assigned to and the relevant validators.
 /// Issue a connection request to these validators.
 /// If the para is not scheduled or next up on any core, at the relay-parent,
@@ -162,8 +162,8 @@ where
 	// Determine which core the para collated-on is assigned to.
 	// If it is not scheduled then ignore the message.
 	let (our_core, num_cores) = match determine_core(ctx, id, relay_parent).await? {
-	    Some(core) => core,
-	    None => {
+		Some(core) => core,
+		None => {
 			warn!(
 				target: TARGET,
 				"Looks like no core is assigned to {:?} at {:?}", id, relay_parent,
@@ -174,8 +174,8 @@ where
 
 	// Determine the group on that core and the next group on that core.
 	let our_validators = match determine_our_validators(ctx, our_core, num_cores, relay_parent).await? {
-	    Some(validators) => validators,
-	    None => {
+		Some(validators) => validators,
+		None => {
 			warn!(
 				target: TARGET,
 				"There are no validators assigned to {:?} core", our_core,
@@ -228,7 +228,7 @@ where
 }
 
 /// Figure out a group of validators assigned to the para being collated on.
-/// 
+///
 /// This returns validators for the current group and the next group.
 async fn determine_our_validators<Context>(
 	ctx: &mut Context,
@@ -260,7 +260,7 @@ where
 
 	let validators = validators.await??;
 
-	let validators = connect_to_validators 
+	let validators = connect_to_validators
 		.into_iter()
 		.map(|idx| validators[idx as usize].clone())
 		.collect();
@@ -324,8 +324,8 @@ where
 	Context: SubsystemContext<Message = CollatorProtocolMessage>
 {
 	let collating_on = match state.collating_on {
-	    Some(collating_on) => collating_on,
-	    None => {
+		Some(collating_on) => collating_on,
+		None => {
 			return Ok(());
 		}
 	};
@@ -461,19 +461,19 @@ where
 	use protocol_v1::CollatorProtocolMessage::*;
 
 	match msg {
-	    Declare(_) => {
+		Declare(_) => {
 			warn!(
 				target: TARGET,
 				"Declare message is not expected on the collator side of the protocol",
 			);
 		}
-	    AdvertiseCollation(_, _) => {
+		AdvertiseCollation(_, _) => {
 			warn!(
 				target: TARGET,
 				"AdvertiseCollation message is not expected on the collator side of the protocol",
 			);
 		}
-	    RequestCollation(request_id, relay_parent, para_id) => {
+		RequestCollation(request_id, relay_parent, para_id) => {
 			match state.collating_on {
 				Some(our_para_id) => {
 					if our_para_id == para_id {
@@ -497,7 +497,7 @@ where
 				}
 			}
 		}
-	    Collation(_, _, _) => {
+		Collation(_, _, _) => {
 			warn!(
 				target: TARGET,
 				"Collation message is not expected on the collator side of the protocol",
@@ -570,7 +570,7 @@ where
 	use NetworkBridgeEvent::*;
 
 	match bridge_message {
-	    PeerConnected(peer_id, _observed_role) => {
+		PeerConnected(peer_id, _observed_role) => {
 			handle_peer_connected(ctx, state, peer_id).await?;
 		}
 		PeerViewChange(peer_id, view) => {
@@ -582,7 +582,7 @@ where
 		OurViewChange(view) => {
 			handle_our_view_change(state, view).await?;
 		}
-	    PeerMessage(remote, msg) => {
+		PeerMessage(remote, msg) => {
 			handle_incoming_peer_message(ctx, state, remote, msg).await?;
 		}
 	}

polkadot/node/network/collator-protocol/src/validator_side.rs

@@ -101,7 +101,7 @@ enum CollationRequestResult {
 /// It may timeout or end in a graceful fashion if a requested
 /// collation has been received sucessfully or chain has moved on.
 struct CollationRequest {
-	// The response for this request has been received successfully or 
+	// The response for this request has been received successfully or
 	// chain has moved forward and this request is no longer relevant.
 	received: oneshot::Receiver<()>,
 
@@ -173,7 +173,7 @@ struct State {
 	requests_in_progress: FuturesUnordered<BoxFuture<'static, CollationRequestResult>>,
 
 	/// Delay after which a collation request would time out.
-	request_timeout: Duration, 
+	request_timeout: Duration,
 
 	/// Possessed collations.
 	collations: HashMap<(Hash, ParaId), Vec<(CollatorId, CandidateReceipt, PoV)>>,
@@ -471,22 +471,22 @@ where
 	use protocol_v1::CollatorProtocolMessage::*;
 
 	match msg {
-	    Declare(id) => {
+		Declare(id) => {
 			state.known_collators.insert(origin.clone(), id);
 			state.peer_views.entry(origin).or_default();
 		}
-	    AdvertiseCollation(relay_parent, para_id) => {
+		AdvertiseCollation(relay_parent, para_id) => {
 			state.advertisments.entry(origin.clone()).or_default().insert((para_id, relay_parent));
 
 			if let Some(collator) = state.known_collators.get(&origin) {
 				notify_candidate_selection(ctx, collator.clone(), relay_parent, para_id).await?;
 			}
 		}
-	    RequestCollation(_, _, _) => {
+		RequestCollation(_, _, _) => {
 			// This is a validator side of the protocol, collation requests are not expected here.
 			return modify_reputation(ctx, origin, COST_UNEXPECTED_MESSAGE).await;
 		}
-	    Collation(request_id, receipt, pov) => {
+		Collation(request_id, receipt, pov) => {
 			received_collation(ctx, state, origin, request_id, receipt, pov).await?;
 		}
 	}

polkadot/node/overseer/src/lib.rs

@@ -1883,7 +1883,7 @@ mod tests {
 	struct TestCollator;
 
 	impl Future for TestCollator {
-		type Output = Collation;
+		type Output = Option<Collation>;
 
 		fn poll(self: Pin<&mut Self>, _cx: &mut futures::task::Context) -> Poll<Self::Output> {
 			panic!("at the Disco")

polkadot/node/primitives/src/lib.rs

@@ -276,7 +276,7 @@ pub struct Collation {
 	pub new_validation_code: Option<ValidationCode>,
 	/// The head-data produced as a result of execution.
 	pub head_data: HeadData,
-	/// Proof that this block is valid.
+	/// Proof to verify the state transition of the parachain.
 	pub proof_of_validity: PoV,
 }
 
@@ -285,7 +285,7 @@ pub struct CollationGenerationConfig {
 	/// Collator's authentication key, so it can sign things.
 	pub key: CollatorPair,
 	/// Collation function.
-	pub collator: Box<dyn Fn(&ValidationData) -> Box<dyn Future<Output = Collation> + Unpin + Send> + Send + Sync>,
+	pub collator: Box<dyn Fn(&ValidationData) -> Box<dyn Future<Output = Option<Collation>> + Unpin + Send> + Send + Sync>,
 	/// The parachain that this collator collates for
 	pub para_id: ParaId,
 }

polkadot/primitives/src/v1.rs

@@ -244,8 +244,8 @@ impl Ord for CommittedCandidateReceipt {
 /// Nevertheless, we expose it so the backing validators can validate the outputs of a
 /// candidate before voting to submit it to the relay-chain and so collators can
 /// collate candidates that satisfy the criteria implied these transient validation data.
-#[derive(PartialEq, Eq, Clone, Encode, Decode)]
-#[cfg_attr(feature = "std", derive(Debug, Default))]
+#[derive(PartialEq, Eq, Clone, Encode, Decode, RuntimeDebug)]
+#[cfg_attr(feature = "std", derive(Default))]
 pub struct ValidationData<N = BlockNumber> {
 	/// The persisted validation data.
 	pub persisted: PersistedValidationData<N>,

polkadot/roadmap/implementers-guide/src/node/collators/collation-generation.md

@@ -22,14 +22,21 @@ The process of generating a collation for a parachain is very parachain-specific
 
 ```rust
 pub struct Collation {
-  /// Hash of `CandidateCommitments` as understood by the collator.
-  pub commitments_hash: Hash,
+  /// Fees paid from the chain to the relay chain validators.
+  pub fees: Balance,
+  /// Messages destined to be interpreted by the Relay chain itself.
+  pub upward_messages: Vec<UpwardMessage>,
+  /// New validation code.
+  pub new_validation_code: Option<ValidationCode>,
+  /// The head-data produced as a result of execution.
+  pub head_data: HeadData,
+  /// Proof to verify the state transition of the parachain.
   pub proof_of_validity: PoV,
 }
 
 struct CollationGenerationConfig {
   key: CollatorPair,
-  collator: Box<dyn Fn(&ValidationData) -> Box<dyn Future<Output = Collation>>>
+  collator: Box<dyn Fn(&ValidationData) -> Box<dyn Future<Output = Option<Collation>>>>
   para_id: ParaId,
 }
 ```