Make produce_candidate return an Option (#1309)

* Make `produce_candidate` return an `Option`

Instead of `produce_candidate` returning a `Result`, it should return an
`Option`. The only supported error was `InvalidHead` anyway and Cumulus
will take care to print appropriate information on what failed and
Polkadot can just ignore it.

* Fix warning

collator/src/lib.rs

@@ -50,7 +50,7 @@ use std::sync::Arc;
 use std::time::Duration;
 use std::pin::Pin;
 
-use futures::{future, Future, Stream, FutureExt, TryFutureExt, StreamExt, task::Spawn};
+use futures::{future, Future, Stream, FutureExt, StreamExt, task::Spawn};
 use log::warn;
 use sc_client_api::{StateBackend, BlockchainEvents};
 use sp_blockchain::HeaderBackend;
@@ -100,24 +100,17 @@ impl Network for polkadot_network::protocol::Service {
 	}
 }
 
-/// Error to return when the head data was invalid.
-#[derive(Clone, Copy, Debug)]
-pub struct InvalidHead;
-
 /// Collation errors.
 #[derive(Debug)]
 pub enum Error {
 	/// Error on the relay-chain side of things.
 	Polkadot(String),
-	/// Error on the collator side of things.
-	Collator(InvalidHead),
 }
 
 impl fmt::Display for Error {
 	fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
 		match *self {
 			Error::Polkadot(ref err) => write!(f, "Polkadot node error: {}", err),
-			Error::Collator(_) => write!(f, "Collator node error: Invalid head data"),
 		}
 	}
 }
@@ -147,7 +140,7 @@ pub trait BuildParachainContext {
 /// This can be implemented through an externally attached service or a stub.
 /// This is expected to be a lightweight, shared type like an Arc.
 pub trait ParachainContext: Clone {
-	type ProduceCandidate: Future<Output = Result<(BlockData, HeadData), InvalidHead>>;
+	type ProduceCandidate: Future<Output = Option<(BlockData, HeadData)>>;
 
 	/// Produce a candidate, given the relay parent hash, the latest ingress queue information
 	/// and the last parachain head.
@@ -167,8 +160,7 @@ pub async fn collate<P>(
 	local_validation_data: LocalValidationData,
 	mut para_context: P,
 	key: Arc<CollatorPair>,
-)
-	-> Result<parachain::Collation, Error>
+) -> Option<parachain::Collation>
 	where
 		P: ParachainContext,
 		P::ProduceCandidate: Send,
@@ -177,7 +169,7 @@ pub async fn collate<P>(
 		relay_parent,
 		global_validation,
 		local_validation_data,
-	).map_err(Error::Collator).await?;
+	).await?;
 
 	let pov_block = PoVBlock {
 		block_data,
@@ -204,7 +196,7 @@ pub async fn collate<P>(
 		pov: pov_block,
 	};
 
-	Ok(collation)
+	Some(collation)
 }
 
 #[cfg(feature = "service-rewr")]
@@ -341,8 +333,13 @@ fn build_collator_service<SP, P, C, R, Extrinsic>(
 					local_validation,
 					parachain_context,
 					key,
-				).map_ok(move |collation| {
-					network.distribute_collation(targets, collation)
+				).map(move |collation| {
+					match collation {
+						Some(collation) => network.distribute_collation(targets, collation),
+						None => log::trace!("Skipping collation as `collate` returned `None`"),
+					}
+
+					Ok(())
 				});
 
 				future::Either::Right(collation_work)
@@ -470,7 +467,7 @@ mod tests {
 	struct DummyParachainContext;
 
 	impl ParachainContext for DummyParachainContext {
-		type ProduceCandidate = future::Ready<Result<(BlockData, HeadData), InvalidHead>>;
+		type ProduceCandidate = future::Ready<Option<(BlockData, HeadData)>>;
 
 		fn produce_candidate(
 			&mut self,
@@ -479,10 +476,10 @@ mod tests {
 			_local_validation: LocalValidationData,
 		) -> Self::ProduceCandidate {
 			// send messages right back.
-			future::ok((
+			future::ready(Some((
 				BlockData(vec![1, 2, 3, 4, 5,]),
 				HeadData(vec![9, 9, 9]),
-			))
+			)))
 		}
 	}
 

parachain/test-parachains/adder/collator/src/main.rs

@@ -26,11 +26,9 @@ use primitives::{
 	Hash,
 	parachain::{HeadData, BlockData, Id as ParaId, LocalValidationData, GlobalValidationSchedule},
 };
-use collator::{
-	InvalidHead, ParachainContext, Network, BuildParachainContext, Cli, SubstrateCli,
-};
+use collator::{ParachainContext, Network, BuildParachainContext, Cli, SubstrateCli};
 use parking_lot::Mutex;
-use futures::future::{Ready, ok, err, TryFutureExt};
+use futures::future::{Ready, ready, TryFutureExt};
 
 const GENESIS: AdderHead = AdderHead {
 	number: 0,
@@ -55,7 +53,7 @@ struct AdderContext {
 
 /// The parachain context.
 impl ParachainContext for AdderContext {
-	type ProduceCandidate = Ready<Result<(BlockData, HeadData), InvalidHead>>;
+	type ProduceCandidate = Ready<Option<(BlockData, HeadData)>>;
 
 	fn produce_candidate(
 		&mut self,
@@ -64,9 +62,9 @@ impl ParachainContext for AdderContext {
 		local_validation: LocalValidationData,
 	) -> Self::ProduceCandidate
 	{
-		let adder_head = match AdderHead::decode(&mut &local_validation.parent_head.0[..]) {
-			Ok(adder_head) => adder_head,
-			Err(_) => return err(InvalidHead)
+		let adder_head = match AdderHead::decode(&mut &local_validation.parent_head.0[..]).ok() {
+			Some(res) => res,
+			None => return ready(None),
 		};
 
 		let mut db = self.db.lock();
@@ -94,7 +92,7 @@ impl ParachainContext for AdderContext {
 			next_head.number, next_body.state.overflowing_add(next_body.add).0);
 
 		db.insert(next_head.clone(), next_body);
-		ok((encoded_body, encoded_head))
+		ready(Some((encoded_body, encoded_head)))
 	}
 }