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 <[email protected]>

* 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 <[email protected]>
Co-authored-by: Javier Viola <[email protected]>

cumulus/client/relay-chain-minimal-node/src/blockchain_rpc_client.rs

@@ -338,6 +338,14 @@ impl RuntimeApiSubsystemClient for BlockChainRpcClient {
 			.await?)
 	}
 
+	async fn minimum_backing_votes(
+		&self,
+		at: Hash,
+		session_index: polkadot_primitives::SessionIndex,
+	) -> Result<u32, ApiError> {
+		Ok(self.rpc_client.parachain_host_minimum_backing_votes(at, session_index).await?)
+	}
+
 	async fn staging_async_backing_params(&self, at: Hash) -> Result<AsyncBackingParams, ApiError> {
 		Ok(self.rpc_client.parachain_host_staging_async_backing_params(at).await?)
 	}

cumulus/client/relay-chain-rpc-interface/src/rpc_client.rs

@@ -588,6 +588,16 @@ impl RelayChainRpcClient {
 			.await
 	}
 
+	/// Get the minimum number of backing votes for a candidate.
+	pub async fn parachain_host_minimum_backing_votes(
+		&self,
+		at: RelayHash,
+		_session_index: SessionIndex,
+	) -> Result<u32, RelayChainError> {
+		self.call_remote_runtime_function("ParachainHost_minimum_backing_votes", at, None::<()>)
+			.await
+	}
+
 	#[allow(missing_docs)]
 	pub async fn parachain_host_staging_async_backing_params(
 		&self,

cumulus/parachains/integration-tests/emulated/common/src/lib.rs

@@ -35,6 +35,7 @@ pub use impls::{RococoWococoMessageHandler, WococoRococoMessageHandler};
 pub use parachains_common::{AccountId, Balance};
 pub use paste;
 use polkadot_parachain::primitives::HrmpChannelId;
+use polkadot_primitives::runtime_api::runtime_decl_for_parachain_host::ParachainHostV6;
 pub use polkadot_runtime_parachains::inclusion::{AggregateMessageOrigin, UmpQueueId};
 pub use sp_core::{sr25519, storage::Storage, Get};
 use sp_tracing;

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,39 +969,38 @@ 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)
 		},)
 		.await,
 	)
 	.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,23 +282,24 @@ async fn test_startup(virtual_overseer: &mut VirtualOverseer, test_state: &TestS
 		}
 	);
 
-	// Check that subsystem job issues a request for the session index for child.
+	// Check that subsystem job issues a request for the availability cores.
 	assert_matches!(
 		virtual_overseer.recv().await,
 		AllMessages::RuntimeApi(
-			RuntimeApiMessage::Request(parent, RuntimeApiRequest::SessionIndexForChild(tx))
+			RuntimeApiMessage::Request(parent, RuntimeApiRequest::AvailabilityCores(tx))
 		) if parent == test_state.relay_parent => {
-			tx.send(Ok(test_state.signing_context.session_index)).unwrap();
+			tx.send(Ok(test_state.availability_cores.clone())).unwrap();
 		}
 	);
 
-	// Check that subsystem job issues a request for the availability cores.
+	// Check if subsystem job issues a request for the minimum backing votes.
 	assert_matches!(
 		virtual_overseer.recv().await,
-		AllMessages::RuntimeApi(
-			RuntimeApiMessage::Request(parent, RuntimeApiRequest::AvailabilityCores(tx))
-		) if parent == test_state.relay_parent => {
-			tx.send(Ok(test_state.availability_cores.clone())).unwrap();
+		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();
 		}
 	);
 }

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,23 +175,24 @@ async fn activate_leaf(
 			}
 		);
 
-		// Check that subsystem job issues a request for the session index for child.
+		// Check that subsystem job issues a request for the availability cores.
 		assert_matches!(
 			virtual_overseer.recv().await,
 			AllMessages::RuntimeApi(
-				RuntimeApiMessage::Request(parent, RuntimeApiRequest::SessionIndexForChild(tx))
+				RuntimeApiMessage::Request(parent, RuntimeApiRequest::AvailabilityCores(tx))
 			) if parent == hash => {
-				tx.send(Ok(test_state.signing_context.session_index)).unwrap();
+				tx.send(Ok(test_state.availability_cores.clone())).unwrap();
 			}
 		);
 
-		// Check that subsystem job issues a request for the availability cores.
+		// Check if subsystem job issues a request for the minimum backing votes.
 		assert_matches!(
 			virtual_overseer.recv().await,
-			AllMessages::RuntimeApi(
-				RuntimeApiMessage::Request(parent, RuntimeApiRequest::AvailabilityCores(tx))
-			) if parent == hash => {
-				tx.send(Ok(test_state.availability_cores.clone())).unwrap();
+			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();
 			}
 		);
 	}

polkadot/node/core/runtime-api/src/cache.rs

@@ -65,6 +65,7 @@ pub(crate) struct RequestResultCache {
 		LruMap<Hash, Vec<(SessionIndex, CandidateHash, vstaging::slashing::PendingSlashes)>>,
 	key_ownership_proof:
 		LruMap<(Hash, ValidatorId), Option<vstaging::slashing::OpaqueKeyOwnershipProof>>,
+	minimum_backing_votes: LruMap<SessionIndex, u32>,
 
 	staging_para_backing_state: LruMap<(Hash, ParaId), Option<vstaging::BackingState>>,
 	staging_async_backing_params: LruMap<Hash, vstaging::AsyncBackingParams>,
@@ -97,6 +98,7 @@ impl Default for RequestResultCache {
 			disputes: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)),
 			unapplied_slashes: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)),
 			key_ownership_proof: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)),
+			minimum_backing_votes: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)),
 
 			staging_para_backing_state: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)),
 			staging_async_backing_params: LruMap::new(ByLength::new(DEFAULT_CACHE_CAP)),
@@ -434,6 +436,18 @@ impl RequestResultCache {
 		None
 	}
 
+	pub(crate) fn minimum_backing_votes(&mut self, session_index: SessionIndex) -> Option<u32> {
+		self.minimum_backing_votes.get(&session_index).copied()
+	}
+
+	pub(crate) fn cache_minimum_backing_votes(
+		&mut self,
+		session_index: SessionIndex,
+		minimum_backing_votes: u32,
+	) {
+		self.minimum_backing_votes.insert(session_index, minimum_backing_votes);
+	}
+
 	pub(crate) fn staging_para_backing_state(
 		&mut self,
 		key: (Hash, ParaId),
@@ -469,6 +483,7 @@ pub(crate) enum RequestResult {
 	// The structure of each variant is (relay_parent, [params,]*, result)
 	Authorities(Hash, Vec<AuthorityDiscoveryId>),
 	Validators(Hash, Vec<ValidatorId>),
+	MinimumBackingVotes(Hash, SessionIndex, u32),
 	ValidatorGroups(Hash, (Vec<Vec<ValidatorIndex>>, GroupRotationInfo)),
 	AvailabilityCores(Hash, Vec<CoreState>),
 	PersistedValidationData(Hash, ParaId, OccupiedCoreAssumption, Option<PersistedValidationData>),

polkadot/node/core/runtime-api/src/lib.rs

@@ -101,6 +101,9 @@ where
 				self.requests_cache.cache_authorities(relay_parent, authorities),
 			Validators(relay_parent, validators) =>
 				self.requests_cache.cache_validators(relay_parent, validators),
+			MinimumBackingVotes(_, session_index, minimum_backing_votes) => self
+				.requests_cache
+				.cache_minimum_backing_votes(session_index, minimum_backing_votes),
 			ValidatorGroups(relay_parent, groups) =>
 				self.requests_cache.cache_validator_groups(relay_parent, groups),
 			AvailabilityCores(relay_parent, cores) =>
@@ -301,6 +304,15 @@ where
 			Request::StagingAsyncBackingParams(sender) =>
 				query!(staging_async_backing_params(), sender)
 					.map(|sender| Request::StagingAsyncBackingParams(sender)),
+			Request::MinimumBackingVotes(index, sender) => {
+				if let Some(value) = self.requests_cache.minimum_backing_votes(index) {
+					self.metrics.on_cached_request();
+					let _ = sender.send(Ok(value));
+					None
+				} else {
+					Some(Request::MinimumBackingVotes(index, sender))
+				}
+			},
 		}
 	}
 
@@ -551,6 +563,12 @@ where
 			ver = Request::SUBMIT_REPORT_DISPUTE_LOST_RUNTIME_REQUIREMENT,
 			sender
 		),
+		Request::MinimumBackingVotes(index, sender) => query!(
+			MinimumBackingVotes,
+			minimum_backing_votes(index),
+			ver = Request::MINIMUM_BACKING_VOTES_RUNTIME_REQUIREMENT,
+			sender
+		),
 
 		Request::StagingParaBackingState(para, sender) => {
 			query!(