backing: move the min votes threshold to the runtime (#1200)

* move min backing votes const to runtime

also cache it per-session in the backing subsystem

Signed-off-by: alindima <alin@parity.io>

* add runtime migration

* introduce api versioning for min_backing votes

also enable it for rococo/versi for testing

* also add min_backing_votes runtime calls to statement-distribution

this dependency has been recently introduced by async backing

* remove explicit version runtime API call

this is not needed, as the RuntimeAPISubsystem already takes care
of versioning and will return NotSupported if the version is not
right.

* address review comments

- parametrise backing votes runtime API with session index
- remove RuntimeInfo usage in backing subsystem, as runtime API
caches the min backing votes by session index anyway.
- move the logic for adjusting the configured needed backing votes with the size of the backing group
to a primitives helper.
- move the legacy min backing votes value to a primitives helper.
- mark JoinMultiple error as fatal, since the Canceled (non-multiple) counterpart is also fatal.
- make backing subsystem handle fatal errors for new leaves update.
- add HostConfiguration consistency check for zeroed backing votes threshold
- add cumulus accompanying change

* fix cumulus test compilation

* fix tests

* more small fixes

* fix merge

* bump runtime api version for westend and rollback version for rococo

---------

Signed-off-by: alindima <alin@parity.io>
Co-authored-by: Javier Viola <javier@parity.io>

polkadot/node/core/backing/src/error.rs

@@ -79,6 +79,7 @@ pub enum Error {
 	RuntimeApiUnavailable(#[source] oneshot::Canceled),
 
 	#[error("a channel was closed before receipt in try_join!")]
+	#[fatal]
 	JoinMultiple(#[source] oneshot::Canceled),
 
 	#[error("Obtaining erasure chunks failed")]

polkadot/node/core/backing/src/lib.rs

@@ -80,8 +80,8 @@ use futures::{
 
 use error::{Error, FatalResult};
 use polkadot_node_primitives::{
-	minimum_votes, AvailableData, InvalidCandidate, PoV, SignedFullStatementWithPVD,
-	StatementWithPVD, ValidationResult,
+	AvailableData, InvalidCandidate, PoV, SignedFullStatementWithPVD, StatementWithPVD,
+	ValidationResult,
 };
 use polkadot_node_subsystem::{
 	messages::{
@@ -98,7 +98,9 @@ use polkadot_node_subsystem_util::{
 	backing_implicit_view::{FetchError as ImplicitViewFetchError, View as ImplicitView},
 	request_from_runtime, request_session_index_for_child, request_validator_groups,
 	request_validators,
-	runtime::{prospective_parachains_mode, ProspectiveParachainsMode},
+	runtime::{
+		self, prospective_parachains_mode, request_min_backing_votes, ProspectiveParachainsMode,
+	},
 	Validator,
 };
 use polkadot_primitives::{
@@ -219,6 +221,8 @@ struct PerRelayParentState {
 	awaiting_validation: HashSet<CandidateHash>,
 	/// Data needed for retrying in case of `ValidatedCandidateCommand::AttestNoPoV`.
 	fallbacks: HashMap<CandidateHash, AttestingData>,
+	/// The minimum backing votes threshold.
+	minimum_backing_votes: u32,
 }
 
 struct PerCandidateState {
@@ -400,8 +404,8 @@ impl TableContextTrait for TableContext {
 		self.groups.get(group).map_or(false, |g| g.iter().any(|a| a == authority))
 	}
 
-	fn requisite_votes(&self, group: &ParaId) -> usize {
-		self.groups.get(group).map_or(usize::MAX, |g| minimum_votes(g.len()))
+	fn get_group_size(&self, group: &ParaId) -> Option<usize> {
+		self.groups.get(group).map(|g| g.len())
 	}
 }
 
@@ -965,28 +969,25 @@ async fn construct_per_relay_parent_state<Context>(
 		($x: expr) => {
 			match $x {
 				Ok(x) => x,
-				Err(e) => {
-					gum::warn!(
-						target: LOG_TARGET,
-						err = ?e,
-						"Failed to fetch runtime API data for job",
-					);
+				Err(err) => {
+					// Only bubble up fatal errors.
+					error::log_error(Err(Into::<runtime::Error>::into(err).into()))?;
 
 					// We can't do candidate validation work if we don't have the
 					// requisite runtime API data. But these errors should not take
 					// down the node.
-					return Ok(None);
-				}
+					return Ok(None)
+				},
 			}
-		}
+		};
 	}
 
 	let parent = relay_parent;
 
-	let (validators, groups, session_index, cores) = futures::try_join!(
+	let (session_index, validators, groups, cores) = futures::try_join!(
+		request_session_index_for_child(parent, ctx.sender()).await,
 		request_validators(parent, ctx.sender()).await,
 		request_validator_groups(parent, ctx.sender()).await,
-		request_session_index_for_child(parent, ctx.sender()).await,
 		request_from_runtime(parent, ctx.sender(), |tx| {
 			RuntimeApiRequest::AvailabilityCores(tx)
 		},)
@@ -994,10 +995,12 @@ async fn construct_per_relay_parent_state<Context>(
 	)
 	.map_err(Error::JoinMultiple)?;
 
+	let session_index = try_runtime_api!(session_index);
 	let validators: Vec<_> = try_runtime_api!(validators);
 	let (validator_groups, group_rotation_info) = try_runtime_api!(groups);
-	let session_index = try_runtime_api!(session_index);
 	let cores = try_runtime_api!(cores);
+	let minimum_backing_votes =
+		try_runtime_api!(request_min_backing_votes(parent, session_index, ctx.sender()).await);
 
 	let signing_context = SigningContext { parent_hash: parent, session_index };
 	let validator =
@@ -1061,6 +1064,7 @@ async fn construct_per_relay_parent_state<Context>(
 		issued_statements: HashSet::new(),
 		awaiting_validation: HashSet::new(),
 		fallbacks: HashMap::new(),
+		minimum_backing_votes,
 	}))
 }
 
@@ -1563,10 +1567,13 @@ async fn post_import_statement_actions<Context>(
 	rp_state: &mut PerRelayParentState,
 	summary: Option<&TableSummary>,
 ) -> Result<(), Error> {
-	if let Some(attested) = summary
-		.as_ref()
-		.and_then(|s| rp_state.table.attested_candidate(&s.candidate, &rp_state.table_context))
-	{
+	if let Some(attested) = summary.as_ref().and_then(|s| {
+		rp_state.table.attested_candidate(
+			&s.candidate,
+			&rp_state.table_context,
+			rp_state.minimum_backing_votes,
+		)
+	}) {
 		let candidate_hash = attested.candidate.hash();
 
 		// `HashSet::insert` returns true if the thing wasn't in there already.
@@ -2009,7 +2016,11 @@ fn handle_get_backed_candidates_message(
 			};
 			rp_state
 				.table
-				.attested_candidate(&candidate_hash, &rp_state.table_context)
+				.attested_candidate(
+					&candidate_hash,
+					&rp_state.table_context,
+					rp_state.minimum_backing_votes,
+				)
 				.and_then(|attested| table_attested_to_backed(attested, &rp_state.table_context))
 		})
 		.collect();

polkadot/node/core/backing/src/tests/mod.rs

@@ -34,7 +34,7 @@ use polkadot_node_subsystem::{
 use polkadot_node_subsystem_test_helpers as test_helpers;
 use polkadot_primitives::{
 	CandidateDescriptor, GroupRotationInfo, HeadData, PersistedValidationData, PvfExecTimeoutKind,
-	ScheduledCore, SessionIndex,
+	ScheduledCore, SessionIndex, LEGACY_MIN_BACKING_VOTES,
 };
 use sp_application_crypto::AppCrypto;
 use sp_keyring::Sr25519Keyring;
@@ -80,6 +80,7 @@ struct TestState {
 	head_data: HashMap<ParaId, HeadData>,
 	signing_context: SigningContext,
 	relay_parent: Hash,
+	minimum_backing_votes: u32,
 }
 
 impl TestState {
@@ -150,6 +151,7 @@ impl Default for TestState {
 			validation_data,
 			signing_context,
 			relay_parent,
+			minimum_backing_votes: LEGACY_MIN_BACKING_VOTES,
 		}
 	}
 }
@@ -250,6 +252,16 @@ async fn test_startup(virtual_overseer: &mut VirtualOverseer, test_state: &TestS
 		}
 	);
 
+	// Check that subsystem job issues a request for the session index for child.
+	assert_matches!(
+		virtual_overseer.recv().await,
+		AllMessages::RuntimeApi(
+			RuntimeApiMessage::Request(parent, RuntimeApiRequest::SessionIndexForChild(tx))
+		) if parent == test_state.relay_parent => {
+			tx.send(Ok(test_state.signing_context.session_index)).unwrap();
+		}
+	);
+
 	// Check that subsystem job issues a request for a validator set.
 	assert_matches!(
 		virtual_overseer.recv().await,
@@ -270,16 +282,6 @@ async fn test_startup(virtual_overseer: &mut VirtualOverseer, test_state: &TestS
 		}
 	);
 
-	// Check that subsystem job issues a request for the session index for child.
-	assert_matches!(
-		virtual_overseer.recv().await,
-		AllMessages::RuntimeApi(
-			RuntimeApiMessage::Request(parent, RuntimeApiRequest::SessionIndexForChild(tx))
-		) if parent == test_state.relay_parent => {
-			tx.send(Ok(test_state.signing_context.session_index)).unwrap();
-		}
-	);
-
 	// Check that subsystem job issues a request for the availability cores.
 	assert_matches!(
 		virtual_overseer.recv().await,
@@ -289,6 +291,17 @@ async fn test_startup(virtual_overseer: &mut VirtualOverseer, test_state: &TestS
 			tx.send(Ok(test_state.availability_cores.clone())).unwrap();
 		}
 	);
+
+	// Check if subsystem job issues a request for the minimum backing votes.
+	assert_matches!(
+		virtual_overseer.recv().await,
+		AllMessages::RuntimeApi(RuntimeApiMessage::Request(
+			parent,
+			RuntimeApiRequest::MinimumBackingVotes(session_index, tx),
+		)) if parent == test_state.relay_parent && session_index == test_state.signing_context.session_index => {
+			tx.send(Ok(test_state.minimum_backing_votes)).unwrap();
+		}
+	);
 }
 
 // Test that a `CandidateBackingMessage::Second` issues validation work

polkadot/node/core/backing/src/tests/prospective_parachains.rs

@@ -138,13 +138,24 @@ async fn activate_leaf(
 	}
 
 	for (hash, number) in ancestry_iter.take(requested_len) {
-		// Check that subsystem job issues a request for a validator set.
 		let msg = match next_overseer_message.take() {
 			Some(msg) => msg,
 			None => virtual_overseer.recv().await,
 		};
+
+		// Check that subsystem job issues a request for the session index for child.
 		assert_matches!(
 			msg,
+			AllMessages::RuntimeApi(
+				RuntimeApiMessage::Request(parent, RuntimeApiRequest::SessionIndexForChild(tx))
+			) if parent == hash => {
+				tx.send(Ok(test_state.signing_context.session_index)).unwrap();
+			}
+		);
+
+		// Check that subsystem job issues a request for a validator set.
+		assert_matches!(
+			virtual_overseer.recv().await,
 			AllMessages::RuntimeApi(
 				RuntimeApiMessage::Request(parent, RuntimeApiRequest::Validators(tx))
 			) if parent == hash => {
@@ -164,16 +175,6 @@ async fn activate_leaf(
 			}
 		);
 
-		// Check that subsystem job issues a request for the session index for child.
-		assert_matches!(
-			virtual_overseer.recv().await,
-			AllMessages::RuntimeApi(
-				RuntimeApiMessage::Request(parent, RuntimeApiRequest::SessionIndexForChild(tx))
-			) if parent == hash => {
-				tx.send(Ok(test_state.signing_context.session_index)).unwrap();
-			}
-		);
-
 		// Check that subsystem job issues a request for the availability cores.
 		assert_matches!(
 			virtual_overseer.recv().await,
@@ -183,6 +184,17 @@ async fn activate_leaf(
 				tx.send(Ok(test_state.availability_cores.clone())).unwrap();
 			}
 		);
+
+		// Check if subsystem job issues a request for the minimum backing votes.
+		assert_matches!(
+			virtual_overseer.recv().await,
+			AllMessages::RuntimeApi(RuntimeApiMessage::Request(
+				parent,
+				RuntimeApiRequest::MinimumBackingVotes(session_index, tx),
+			)) if parent == hash && session_index == test_state.signing_context.session_index => {
+				tx.send(Ok(test_state.minimum_backing_votes)).unwrap();
+			}
+		);
 	}
 }