rewrite network code to use notifications_protocol APIs from Substrate (#788)

* extract all network code to legacy submodule

* update references to legacy proto

* skeleton of futures-based protocol

* refactor skeleton to use background task

* rename communication_for to build_table_router

* implement internal message types for validation network

* basic ParachainNetwork and TableRouter implementations

* add some module docs

* remove exit-future from validation

* hack: adapt legacy protocol to lack of exit-future

* generalize RegisteredMessageValidator somewhat

* instantiate and teardown table routers

* clean up RouterInner drop logic

* implement most of the statement import loop

* implement statement loop in async/await

* remove unneeded TODO

* most of the collation skeleton

* send session keys and validator roles

* also send role after status

* use config in startup

* point TODO to issue

* fix test compilation

polkadot/validation/src/block_production.rs

@@ -132,7 +132,7 @@ pub struct Proposer<Client, TxPool, Backend> {
 	parent_hash: Hash,
 	parent_id: BlockId,
 	parent_number: BlockNumber,
-	tracker: Arc<crate::validation_service::ValidationInstanceHandle>,
+	tracker: crate::validation_service::ValidationInstanceHandle,
 	transaction_pool: Arc<TxPool>,
 	slot_duration: u64,
 	backend: Arc<Backend>,

polkadot/validation/src/collation.rs

@@ -55,6 +55,9 @@ pub trait Collators: Clone {
 	///
 	/// This does not have to guarantee local availability, as a valid collation
 	/// will be passed to the `TableRouter` instance.
+	///
+	/// The returned future may be prematurely concluded if the `relay_parent` goes
+	/// out of date.
 	fn collate(&self, parachain: ParaId, relay_parent: Hash) -> Self::Collation;
 
 	/// Note a bad collator. TODO: take proof (https://github.com/paritytech/polkadot/issues/217)

polkadot/validation/src/lib.rs

@@ -76,9 +76,13 @@ pub type Incoming = Vec<(ParaId, Vec<Message>)>;
 /// A handle to a statement table router.
 ///
 /// This is expected to be a lightweight, shared type like an `Arc`.
+/// Once all instances are dropped, consensus networking for this router
+/// should be cleaned up.
 pub trait TableRouter: Clone {
 	/// Errors when fetching data from the network.
 	type Error: std::fmt::Debug;
+	/// Future that drives sending of the local collation to the network.
+	type SendLocalCollation: Future<Output=Result<(), Self::Error>>;
 	/// Future that resolves when candidate data is fetched.
 	type FetchValidationProof: Future<Output=Result<PoVBlock, Self::Error>>;
 
@@ -90,9 +94,12 @@ pub trait TableRouter: Clone {
 		receipt: CandidateReceipt,
 		outgoing: OutgoingMessages,
 		chunks: (ValidatorIndex, &[ErasureChunk]),
-	);
+	) -> Self::SendLocalCollation;
 
 	/// Fetch validation proof for a specific candidate.
+	///
+	/// This future must conclude once all `Clone`s of this `TableRouter` have
+	/// been cleaned up.
 	fn fetch_pov_block(&self, candidate: &CandidateReceipt) -> Self::FetchValidationProof;
 }
 
@@ -111,11 +118,10 @@ pub trait Network {
 
 	/// Instantiate a table router using the given shared table.
 	/// Also pass through any outgoing messages to be broadcast to peers.
-	fn communication_for(
+	fn build_table_router(
 		&self,
 		table: Arc<SharedTable>,
 		authorities: &[ValidatorId],
-		exit: exit_future::Exit,
 	) -> Self::BuildTableRouter;
 }
 

polkadot/validation/src/shared_table/mod.rs

@@ -620,6 +620,7 @@ mod tests {
 	struct DummyRouter;
 	impl TableRouter for DummyRouter {
 		type Error = ::std::io::Error;
+		type SendLocalCollation = future::Ready<Result<(),Self::Error>>;
 		type FetchValidationProof = future::Ready<Result<PoVBlock,Self::Error>>;
 
 		fn local_collation(
@@ -628,7 +629,7 @@ mod tests {
 			_candidate: CandidateReceipt,
 			_outgoing: OutgoingMessages,
 			_chunks: (ValidatorIndex, &[ErasureChunk])
-		) {}
+		) -> Self::SendLocalCollation { future::ready(Ok(())) }
 
 		fn fetch_pov_block(&self, _candidate: &CandidateReceipt) -> Self::FetchValidationProof {
 			future::ok(pov_block_with_data(vec![1, 2, 3, 4, 5]))

polkadot/validation/src/validation_service/mod.rs

@@ -34,7 +34,7 @@ use sp_blockchain::HeaderBackend;
 use block_builder::BlockBuilderApi;
 use consensus::SelectChain;
 use futures::prelude::*;
-use futures::{future::select, task::{Spawn, SpawnExt}};
+use futures::task::{Spawn, SpawnExt};
 use polkadot_primitives::{Block, Hash, BlockId};
 use polkadot_primitives::parachain::{
 	Chain, ParachainHost, Id as ParaId, ValidatorIndex, ValidatorId, ValidatorPair,
@@ -57,14 +57,14 @@ pub type TaskExecutor = Arc<dyn Spawn + Send + Sync>;
 // They send a oneshot channel.
 type ValidationInstanceRequest = (
 	Hash,
-	futures::channel::oneshot::Sender<Result<Arc<ValidationInstanceHandle>, Error>>,
+	futures::channel::oneshot::Sender<Result<ValidationInstanceHandle, Error>>,
 );
 
 /// A handle to a single instance of parachain validation, which is pinned to
 /// a specific relay-chain block. This is the instance that should be used when
 /// constructing any
+#[derive(Clone)]
 pub(crate) struct ValidationInstanceHandle {
-	_drop_signal: exit_future::Signal,
 	table: Arc<SharedTable>,
 	started: Instant,
 }
@@ -92,7 +92,7 @@ impl ServiceHandle {
 	///
 	/// This can fail if the service task has shut down for some reason.
 	pub(crate) async fn get_validation_instance(self, relay_parent: Hash)
-		-> Result<Arc<ValidationInstanceHandle>, Error>
+		-> Result<ValidationInstanceHandle, Error>
 	{
 		let mut sender = self.sender;
 		let instance_rx = loop {
@@ -261,7 +261,7 @@ pub(crate) struct ParachainValidationInstances<C, N, P, SP> {
 	availability_store: AvailabilityStore,
 	/// Live agreements. Maps relay chain parent hashes to attestation
 	/// instances.
-	live_instances: HashMap<Hash, Arc<ValidationInstanceHandle>>,
+	live_instances: HashMap<Hash, ValidationInstanceHandle>,
 }
 
 impl<C, N, P, SP> ParachainValidationInstances<C, N, P, SP> where
@@ -289,7 +289,7 @@ impl<C, N, P, SP> ParachainValidationInstances<C, N, P, SP> where
 		keystore: &KeyStorePtr,
 		max_block_data_size: Option<u64>,
 	)
-		-> Result<Arc<ValidationInstanceHandle>, Error>
+		-> Result<ValidationInstanceHandle, Error>
 	{
 		use primitives::Pair;
 
@@ -344,23 +344,19 @@ impl<C, N, P, SP> ParachainValidationInstances<C, N, P, SP> where
 			max_block_data_size,
 		));
 
-		let (_drop_signal, exit) = exit_future::signal();
-
-		let router = self.network.communication_for(
+		let router = self.network.build_table_router(
 			table.clone(),
 			&validators,
-			exit.clone(),
 		);
 
 		if let Some((Chain::Parachain(id), index)) = local_duty.as_ref().map(|d| (d.validation, d.index)) {
-			self.launch_work(parent_hash, id, router, max_block_data_size, validators.len(), index, exit);
+			self.launch_work(parent_hash, id, router, max_block_data_size, validators.len(), index);
 		}
 
-		let tracker = Arc::new(ValidationInstanceHandle {
+		let tracker = ValidationInstanceHandle {
 			table,
 			started: Instant::now(),
-			_drop_signal,
-		});
+		};
 
 		self.live_instances.insert(parent_hash, tracker.clone());
 
@@ -381,7 +377,6 @@ impl<C, N, P, SP> ParachainValidationInstances<C, N, P, SP> where
 		max_block_data_size: Option<u64>,
 		authorities_num: usize,
 		local_id: ValidatorIndex,
-		exit: exit_future::Exit,
 	) {
 		let (collators, client) = (self.collators.clone(), self.client.clone());
 		let availability_store = self.availability_store.clone();
@@ -423,13 +418,20 @@ impl<C, N, P, SP> ParachainValidationInstances<C, N, P, SP> where
 									"Failed to add erasure chunks: {}", e
 								);
 							} else {
-								router.local_collation(
+								let res = router.local_collation(
 									collation,
 									receipt,
 									outgoing_targeted,
 									(local_id, &chunks),
-								);
-							}
+								).await;
+
+								if let Err(e) = res {
+									warn!(
+										target: "validation",
+										"Failed to notify network of local collation: {:?}", e
+									);
+								}
+							};
 						}
 						Err(e) => {
 							warn!(target: "validation", "Failed to produce a receipt: {:?}", e);
@@ -442,17 +444,17 @@ impl<C, N, P, SP> ParachainValidationInstances<C, N, P, SP> where
 			}
 		};
 
-		let router = build_router
+		let router_work = build_router
 			.map_ok(with_router)
 			.map_err(|e| {
 				warn!(target: "validation" , "Failed to build table router: {:?}", e);
-			});
+			})
+			.map(|_| ());
 
-		let cancellable_work = select(exit, router).map(drop);
 
 		// spawn onto thread pool.
-		if self.spawner.spawn(cancellable_work).is_err() {
-			error!("Failed to spawn cancellable work task");
+		if self.spawner.spawn(router_work).is_err() {
+			error!("Failed to spawn router work task");
 		}
 	}
 }