collation-generation: guide and tidying (#1753)

* Guide: Change for {Global,Local}ValidationData to ValidationData

Seems like that was missed during the transition from notions of global and local validation data to persistent and transient

* collation-generation: a small drive-by fix of line length

* collation-generation: extract an error module

this allows us to avoid piling Error and Result types from different contexts, specifically std vs. module-local

* collation-generation: extract LOG_TARGET from log statements

node/collation-generation/src/error.rs

+// Copyright 2020 Parity Technologies (UK) Ltd.
+// This file is part of Polkadot.
+
+// Polkadot is free software: you can redistribute it and/or modify
+// it under the terms of the GNU General Public License as published by
+// the Free Software Foundation, either version 3 of the License, or
+// (at your option) any later version.
+
+// Polkadot is distributed in the hope that it will be useful,
+// but WITHOUT ANY WARRANTY; without even the implied warranty of
+// MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+// GNU General Public License for more details.
+
+// You should have received a copy of the GNU General Public License
+// along with Polkadot.  If not, see <http://www.gnu.org/licenses/>.
+
+
+#[derive(Debug, derive_more::From)]
+pub enum Error {
+	#[from]
+	Subsystem(polkadot_node_subsystem::SubsystemError),
+	#[from]
+	OneshotRecv(futures::channel::oneshot::Canceled),
+	#[from]
+	Runtime(polkadot_node_subsystem::errors::RuntimeApiError),
+	#[from]
+	Util(polkadot_node_subsystem_util::Error),
+	#[from]
+	Erasure(polkadot_erasure_coding::Error),
+}
+
+pub type Result<T> = std::result::Result<T, Error>;

node/collation-generation/src/lib.rs

@@ -19,7 +19,7 @@
 #![deny(missing_docs)]
 
 use futures::{
-	channel::{mpsc, oneshot},
+	channel::mpsc,
 	future::FutureExt,
 	join,
 	select,
@@ -28,13 +28,12 @@ use futures::{
 };
 use polkadot_node_primitives::CollationGenerationConfig;
 use polkadot_node_subsystem::{
-	errors::RuntimeApiError,
 	messages::{AllMessages, CollationGenerationMessage, CollatorProtocolMessage},
-	FromOverseer, SpawnedSubsystem, Subsystem, SubsystemContext, SubsystemError, SubsystemResult,
+	FromOverseer, SpawnedSubsystem, Subsystem, SubsystemContext, SubsystemResult,
 	metrics::{self, prometheus},
 };
 use polkadot_node_subsystem_util::{
-	self as util, request_availability_cores_ctx, request_full_validation_data_ctx,
+	request_availability_cores_ctx, request_full_validation_data_ctx,
 	request_validators_ctx,
 };
 use polkadot_primitives::v1::{
@@ -45,6 +44,10 @@ use polkadot_primitives::v1::{
 use sp_core::crypto::Pair;
 use std::sync::Arc;
 
+mod error;
+
+const LOG_TARGET: &'static str = "collation_generation";
+
 /// Collation Generation Subsystem
 pub struct CollationGenerationSubsystem {
 	config: Option<Arc<CollationGenerationConfig>>,
@@ -92,7 +95,7 @@ impl CollationGenerationSubsystem {
 				msg = receiver.next().fuse() => {
 					if let Some(msg) = msg {
 						if let Err(err) = ctx.send_message(msg).await {
-							log::warn!(target: "collation_generation", "failed to forward message to overseer: {:?}", err);
+							log::warn!(target: LOG_TARGET, "failed to forward message to overseer: {:?}", err);
 							break;
 						}
 					}
@@ -126,7 +129,7 @@ impl CollationGenerationSubsystem {
 					if let Err(err) =
 						handle_new_activations(config.clone(), &activated, ctx, metrics, sender).await
 					{
-						log::warn!(target: "collation_generation", "failed to handle new activations: {:?}", err);
+						log::warn!(target: LOG_TARGET, "failed to handle new activations: {:?}", err);
 						return true;
 					};
 				}
@@ -137,7 +140,7 @@ impl CollationGenerationSubsystem {
 				msg: CollationGenerationMessage::Initialize(config),
 			}) => {
 				if self.config.is_some() {
-					log::warn!(target: "collation_generation", "double initialization");
+					log::warn!(target: LOG_TARGET, "double initialization");
 					true
 				} else {
 					self.config = Some(Arc::new(config));
@@ -146,7 +149,11 @@ impl CollationGenerationSubsystem {
 			}
 			Ok(Signal(BlockFinalized(_))) => false,
 			Err(err) => {
-				log::error!(target: "collation_generation", "error receiving message from subsystem context: {:?}", err);
+				log::error!(
+					target: LOG_TARGET,
+					"error receiving message from subsystem context: {:?}",
+					err
+				);
 				true
 			}
 		}
@@ -169,29 +176,13 @@ where
 	}
 }
 
-#[derive(Debug, derive_more::From)]
-enum Error {
-	#[from]
-	Subsystem(SubsystemError),
-	#[from]
-	OneshotRecv(oneshot::Canceled),
-	#[from]
-	Runtime(RuntimeApiError),
-	#[from]
-	Util(util::Error),
-	#[from]
-	Erasure(polkadot_erasure_coding::Error),
-}
-
-type Result<T> = std::result::Result<T, Error>;
-
 async fn handle_new_activations<Context: SubsystemContext>(
 	config: Arc<CollationGenerationConfig>,
 	activated: &[Hash],
 	ctx: &mut Context,
 	metrics: Metrics,
 	sender: &mpsc::Sender<AllMessages>,
-) -> Result<()> {
+) -> crate::error::Result<()> {
 	// follow the procedure from the guide:
 	// https://w3f.github.io/parachain-implementers-guide/node/collators/collation-generation.html
 
@@ -262,7 +253,12 @@ async fn handle_new_activations<Context: SubsystemContext>(
 				) {
 					Ok(erasure_root) => erasure_root,
 					Err(err) => {
-						log::error!(target: "collation_generation", "failed to calculate erasure root for para_id {}: {:?}", scheduled_core.para_id, err);
+						log::error!(
+							target: LOG_TARGET,
+							"failed to calculate erasure root for para_id {}: {:?}",
+							scheduled_core.para_id,
+							err
+						);
 						return
 					}
 				};
@@ -292,7 +288,12 @@ async fn handle_new_activations<Context: SubsystemContext>(
 				if let Err(err) = task_sender.send(AllMessages::CollatorProtocol(
 					CollatorProtocolMessage::DistributeCollation(ccr, collation.proof_of_validity)
 				)).await {
-					log::warn!(target: "collation_generation", "failed to send collation result for para_id {}: {:?}", scheduled_core.para_id, err);
+					log::warn!(
+						target: LOG_TARGET,
+						"failed to send collation result for para_id {}: {:?}",
+						scheduled_core.para_id,
+						err
+					);
 				}
 			})).await?;
 		}
@@ -305,7 +306,7 @@ fn erasure_root(
 	n_validators: usize,
 	persisted_validation: PersistedValidationData,
 	pov: PoV,
-) -> Result<Hash> {
+) -> crate::error::Result<Hash> {
 	let available_data = AvailableData {
 		validation_data: persisted_validation,
 		pov,
@@ -333,7 +334,7 @@ impl Metrics {
 }
 
 impl metrics::Metrics for Metrics {
-	fn try_register(registry: &prometheus::Registry) -> std::result::Result<Self, prometheus::PrometheusError> {
+	fn try_register(registry: &prometheus::Registry) -> Result<Self, prometheus::PrometheusError> {
 		let metrics = MetricsInner {
 			collations_generated_total: prometheus::register(
 				prometheus::Counter::new(

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

@@ -29,7 +29,7 @@ pub struct Collation {
 
 struct CollationGenerationConfig {
   key: CollatorPair,
-  collator: Box<dyn Fn(&GlobalValidationData, &LocalValidationData) -> Box<dyn Future<Output = Collation>>>
+  collator: Box<dyn Fn(&ValidationData) -> Box<dyn Future<Output = Collation>>>
   para_id: ParaId,
 }
 ```