Collation protocol: stricter validators (#2810)

* guide: declare one para as a collator * add ParaId to Declare messages and clean up * fix build * fix the testerinos * begin adding keystore to collator-protocol * remove request_x_ctx * add core_for_group * add bump_rotation * add some more helpers to subsystem-util * change signing_key API to take ref * determine current and next para assignments * disconnect collators who are not on current or next para * add collator peer count metric * notes for later * some fixes * add data & keystore to test state * add a test utility for answering runtime API requests * fix existing collator tests * add new tests * remove sc_keystore * update cargo lock Co-authored-by: Andronik Ordian <[email protected]>
paritytech · Apr 3, 2021 · a00e5ab · a00e5ab
1 parent d23becf
commit a00e5ab
Show file tree

Hide file tree

Showing 22 changed files with 1,057 additions and 325 deletions.
diff --git a/Cargo.lock b/Cargo.lock
diff --git a/node/collation-generation/src/lib.rs b/node/collation-generation/src/lib.rs
@@ -32,8 +32,8 @@ use polkadot_node_subsystem::{
 	FromOverseer, SpawnedSubsystem, Subsystem, SubsystemContext, SubsystemResult,
 };
 use polkadot_node_subsystem_util::{
-	request_availability_cores_ctx, request_persisted_validation_data_ctx,
-	request_validators_ctx,
+	request_availability_cores, request_persisted_validation_data,
+	request_validators,
 	metrics::{self, prometheus},
 };
 use polkadot_primitives::v1::{
@@ -198,8 +198,8 @@ async fn handle_new_activations<Context: SubsystemContext>(
 		let _relay_parent_timer = metrics.time_new_activations_relay_parent();
 
 		let (availability_cores, validators) = join!(
-			request_availability_cores_ctx(relay_parent, ctx).await?,
-			request_validators_ctx(relay_parent, ctx).await?,
+			request_availability_cores(relay_parent, ctx.sender()).await,
+			request_validators(relay_parent, ctx.sender()).await,
 		);
 
 		let availability_cores = availability_cores??;
@@ -247,13 +247,14 @@ async fn handle_new_activations<Context: SubsystemContext>(
 			// we get validation data synchronously for each core instead of
 			// within the subtask loop, because we have only a single mutable handle to the
 			// context, so the work can't really be distributed
-			let validation_data = match request_persisted_validation_data_ctx(
+
+			let validation_data = match request_persisted_validation_data(
 				relay_parent,
 				scheduled_core.para_id,
 				assumption,
-				ctx,
+				ctx.sender(),
 			)
-			.await?
+			.await
 			.await??
 			{
 				Some(v) => v,

diff --git a/node/core/candidate-selection/src/lib.rs b/node/core/candidate-selection/src/lib.rs
@@ -294,7 +294,7 @@ impl CandidateSelectionJob {
 				).await {
 					Ok(response) => response,
 					Err(err) => {
-						tracing::warn!(
+						tracing::debug!(
 							target: LOG_TARGET,
 							err = ?err,
 							"failed to get collation from collator protocol subsystem",

diff --git a/node/network/availability-distribution/src/error.rs b/node/network/availability-distribution/src/error.rs
@@ -109,13 +109,9 @@ impl From<SubsystemError> for Error {
 
 /// Receive a response from a runtime request and convert errors.
 pub(crate) async fn recv_runtime<V>(
-	r: std::result::Result<
-		oneshot::Receiver<std::result::Result<V, RuntimeApiError>>,
-		UtilError,
-	>,
+	r: oneshot::Receiver<std::result::Result<V, RuntimeApiError>>,
 ) -> std::result::Result<V, Error> {
-	r.map_err(Error::UtilRequest)?
-		.await
+	r.await
 		.map_err(Error::RuntimeRequestCanceled)?
 		.map_err(Error::RuntimeRequest)
 }

diff --git a/node/network/availability-distribution/src/requester/mod.rs b/node/network/availability-distribution/src/requester/mod.rs
@@ -32,7 +32,7 @@ use futures::{
 
 use sp_keystore::SyncCryptoStorePtr;
 
-use polkadot_node_subsystem_util::request_availability_cores_ctx;
+use polkadot_node_subsystem_util::request_availability_cores;
 use polkadot_primitives::v1::{CandidateHash, CoreState, Hash, OccupiedCore};
 use polkadot_subsystem::{
 	messages::AllMessages, ActiveLeavesUpdate, SubsystemContext, ActivatedLeaf,
@@ -235,7 +235,7 @@ async fn query_occupied_cores<Context>(
 where
 	Context: SubsystemContext,
 {
-	let cores = recv_runtime(request_availability_cores_ctx(relay_parent, ctx).await).await?;
+	let cores = recv_runtime(request_availability_cores(relay_parent, ctx.sender()).await).await?;
 
 	Ok(cores
 		.into_iter()

diff --git a/node/network/availability-distribution/src/runtime.rs b/node/network/availability-distribution/src/runtime.rs
@@ -23,7 +23,7 @@ use sp_core::crypto::Public;
 use sp_keystore::{CryptoStore, SyncCryptoStorePtr};
 
 use polkadot_node_subsystem_util::{
-	request_session_index_for_child_ctx, request_session_info_ctx,
+	request_session_index_for_child, request_session_info,
 };
 use polkadot_primitives::v1::{GroupIndex, Hash, SessionIndex, SessionInfo, ValidatorId, ValidatorIndex};
 use polkadot_subsystem::SubsystemContext;
@@ -93,7 +93,7 @@ impl Runtime {
 			Some(index) => Ok(*index),
 			None => {
 				let index =
-					recv_runtime(request_session_index_for_child_ctx(parent, ctx).await)
+					recv_runtime(request_session_index_for_child(parent, ctx.sender()).await)
 						.await?;
 				self.session_index_cache.put(parent, index);
 				Ok(index)
@@ -117,7 +117,7 @@ impl Runtime {
 
 	/// Get `ExtendedSessionInfo` by session index.
 	///
-	/// `request_session_info_ctx` still requires the parent to be passed in, so we take the parent
+	/// `request_session_info` still requires the parent to be passed in, so we take the parent
 	/// in addition to the `SessionIndex`.
 	pub async fn get_session_info_by_index<'a, Context>(
 		&'a mut self,
@@ -130,7 +130,7 @@ impl Runtime {
 	{
 		if !self.session_info_cache.contains(&session_index) {
 			let session_info =
-				recv_runtime(request_session_info_ctx(parent, session_index, ctx).await)
+				recv_runtime(request_session_info(parent, session_index, ctx.sender()).await)
 					.await?
 					.ok_or(Error::NoSuchSession(session_index))?;
 			let validator_info = self.get_validator_info(&session_info).await?;

diff --git a/node/network/availability-distribution/src/session_cache.rs b/node/network/availability-distribution/src/session_cache.rs
@@ -24,7 +24,7 @@ use sp_core::crypto::Public;
 use sp_keystore::{CryptoStore, SyncCryptoStorePtr};
 
 use polkadot_node_subsystem_util::{
-	request_session_index_for_child_ctx, request_session_info_ctx,
+	request_session_index_for_child, request_session_info,
 };
 use polkadot_primitives::v1::SessionInfo as GlobalSessionInfo;
 use polkadot_primitives::v1::{
@@ -132,7 +132,7 @@ impl SessionCache {
 			Some(index) => *index,
 			None => {
 				let index =
-					recv_runtime(request_session_index_for_child_ctx(parent, ctx).await)
+					recv_runtime(request_session_index_for_child(parent, ctx.sender()).await)
 						.await?;
 				self.session_index_cache.put(parent, index);
 				index
@@ -210,7 +210,7 @@ impl SessionCache {
 
 	/// Query needed information from runtime.
 	///
-	/// We need to pass in the relay parent for our call to `request_session_info_ctx`. We should
+	/// We need to pass in the relay parent for our call to `request_session_info`. We should
 	/// actually don't need that: I suppose it is used for internal caching based on relay parents,
 	/// which we don't use here. It should not do any harm though.
 	///
@@ -229,7 +229,7 @@ impl SessionCache {
 			discovery_keys,
 			mut validator_groups,
 			..
-		} = recv_runtime(request_session_info_ctx(parent, session_index, ctx).await)
+		} = recv_runtime(request_session_info(parent, session_index, ctx.sender()).await)
 			.await?
 			.ok_or(Error::NoSuchSession(session_index))?;
 

diff --git a/node/network/availability-distribution/src/tests/mod.rs b/node/network/availability-distribution/src/tests/mod.rs
@@ -111,7 +111,7 @@ fn check_fetch_retry() {
 		})
 		.collect();
 	state.valid_chunks.retain(|(ch, _)| valid_candidate_hashes.contains(ch));
-			
+
 
 	for (_, v) in state.chunks.iter_mut() {
 		// This should still succeed as cores are still pending availability on next block.

diff --git a/node/network/availability-recovery/src/lib.rs b/node/network/availability-recovery/src/lib.rs
@@ -49,7 +49,7 @@ use polkadot_node_network_protocol::{
 		request::RequestError,
 	},
 };
-use polkadot_node_subsystem_util::request_session_info_ctx;
+use polkadot_node_subsystem_util::request_session_info;
 use polkadot_erasure_coding::{branches, branch_hash, recovery_threshold, obtain_chunks_v1};
 mod error;
 
@@ -697,11 +697,11 @@ async fn handle_recover(
 	}
 
 	let _span = span.child("not-cached");
-	let session_info = request_session_info_ctx(
+	let session_info = request_session_info(
 		state.live_block.1,
 		session_index,
-		ctx,
-	).await?.await.map_err(error::Error::CanceledSessionInfo)??;
+		ctx.sender(),
+	).await.await.map_err(error::Error::CanceledSessionInfo)??;
 
 	let _span = span.child("session-info-ctx-received");
 	match session_info {

diff --git a/node/network/bridge/src/lib.rs b/node/network/bridge/src/lib.rs
@@ -1794,6 +1794,7 @@ mod tests {
 			let collator_protocol_message = protocol_v1::CollatorProtocolMessage::Declare(
 				Sr25519Keyring::Alice.public().into(),
 				Default::default(),
+				Default::default(),
 			);
 
 			let message = protocol_v1::CollationProtocol::CollatorProtocol(
@@ -2064,6 +2065,7 @@ mod tests {
 				let collator_protocol_message = protocol_v1::CollatorProtocolMessage::Declare(
 					Sr25519Keyring::Alice.public().into(),
 					Default::default(),
+					Default::default(),
 				);
 
 				let message = protocol_v1::CollationProtocol::CollatorProtocol(

diff --git a/node/network/collator-protocol/Cargo.toml b/node/network/collator-protocol/Cargo.toml
@@ -11,8 +11,9 @@ futures-timer = "3"
 thiserror = "1.0.23"
 tracing = "0.1.25"
 
-sp-core = { package = "sp-core", git = "https://github.com/paritytech/substrate", branch = "rococo-v1" }
-sp-runtime = { package = "sp-runtime", git = "https://github.com/paritytech/substrate", branch = "rococo-v1" }
+sp-core = { git = "https://github.com/paritytech/substrate", branch = "rococo-v1" }
+sp-runtime = { git = "https://github.com/paritytech/substrate", branch = "rococo-v1" }
+sp-keystore = { git = "https://github.com/paritytech/substrate", branch = "rococo-v1" }
 
 polkadot-primitives = { path = "../../../primitives" }
 polkadot-node-network-protocol = { path = "../../network/protocol" }