From 2ccbf38b85e4569bb9090bc5ec84f2fc5d7bb3f8 Mon Sep 17 00:00:00 2001
From: Sergei Shulepov <sergei@parity.io>
Date: Tue, 14 Dec 2021 13:06:58 +0100
Subject: [PATCH] pvf-precheck: teach runtime-api to work with versions (#4510)

As part of #3211 we will need to add a couple of runtime APIs. That
change is coming in a following PR.

The runtime API for pre-checking will be introduced with a version bump
for `ParachainHost`. This commit prepares the ground for that change,
by introducing a error condition that signals that the given API is not
supported.
---
 polkadot/node/core/av-store/src/tests.rs    |  13 ++-
 polkadot/node/core/runtime-api/src/lib.rs   | 108 +++++++++++++++-----
 polkadot/node/subsystem-types/src/errors.rs |  32 +++---
 3 files changed, 115 insertions(+), 38 deletions(-)

diff --git a/polkadot/node/core/av-store/src/tests.rs b/polkadot/node/core/av-store/src/tests.rs
index bb2ab22bf47..da34fca5fa5 100644
--- a/polkadot/node/core/av-store/src/tests.rs
+++ b/polkadot/node/core/av-store/src/tests.rs
@@ -240,7 +240,18 @@ fn runtime_api_error_does_not_stop_the_subsystem() {
 				RuntimeApiRequest::CandidateEvents(tx),
 			)) => {
 				assert_eq!(relay_parent, new_leaf);
-				tx.send(Err(RuntimeApiError::from("oh no".to_string()))).unwrap();
+				#[derive(Debug)]
+				struct FauxError;
+				impl std::error::Error for FauxError {}
+				impl std::fmt::Display for FauxError {
+					fn fmt(&self, _f: &mut std::fmt::Formatter) -> std::fmt::Result {
+						Ok(())
+					}
+				}
+				tx.send(Err(RuntimeApiError::Execution {
+					runtime_api_name: "faux",
+					source: Arc::new(FauxError),
+				})).unwrap();
 			}
 		);
 
diff --git a/polkadot/node/core/runtime-api/src/lib.rs b/polkadot/node/core/runtime-api/src/lib.rs
index 407046c0487..b4d625a727d 100644
--- a/polkadot/node/core/runtime-api/src/lib.rs
+++ b/polkadot/node/core/runtime-api/src/lib.rs
@@ -335,12 +335,52 @@ where
 {
 	let _timer = metrics.time_make_runtime_api_request();
 
+	// The version of the `ParachainHost` runtime API that we are absolutely sure is deployed widely
+	// on all supported networks: Rococo, Kusama, Polkadot and perhaps others like Versi.
+	//
+	// This version is used for eliding unnecessary runtime API calls. It is safer to keep the lower
+	// version.
+	const WIDELY_DEPLOYED_API_VERSION: u32 = 1;
+
 	macro_rules! query {
-		($req_variant:ident, $api_name:ident ($($param:expr),*), $sender:expr) => {{
+		($req_variant:ident, $api_name:ident ($($param:expr),*), ver = $version:literal, $sender:expr) => {{
 			let sender = $sender;
 			let api = client.runtime_api();
-			let res = api.$api_name(&BlockId::Hash(relay_parent) $(, $param.clone() )*)
-				.map_err(|e| RuntimeApiError::from(format!("{:?}", e)));
+
+			let runtime_version = if $version > WIDELY_DEPLOYED_API_VERSION {
+				use sp_api::ApiExt;
+				api.api_version::<dyn ParachainHost<Block>>(&BlockId::Hash(relay_parent))
+					.unwrap_or_else(|e| {
+						tracing::warn!(
+							target: LOG_TARGET,
+							"cannot query the runtime API version: {}",
+							e,
+						);
+						Some(WIDELY_DEPLOYED_API_VERSION)
+					})
+					.unwrap_or_else(|| {
+						tracing::warn!(
+							target: LOG_TARGET,
+							"no runtime version is reported"
+						);
+						WIDELY_DEPLOYED_API_VERSION
+					})
+			} else {
+				// The required version is less or equal to the widely deployed runtime API version.
+				WIDELY_DEPLOYED_API_VERSION
+			};
+
+			let res = if runtime_version <= $version {
+				api.$api_name(&BlockId::Hash(relay_parent) $(, $param.clone() )*)
+					.map_err(|e| RuntimeApiError::Execution {
+						runtime_api_name: stringify!($api_name),
+						source: std::sync::Arc::new(e),
+					})
+			} else {
+				Err(RuntimeApiError::NotSupported {
+					runtime_api_name: stringify!($api_name),
+				})
+			};
 			metrics.on_request(res.is_ok());
 			let _ = sender.send(res.clone());
 
@@ -349,36 +389,58 @@ where
 	}
 
 	match request {
-		Request::Authorities(sender) => query!(Authorities, authorities(), sender),
-		Request::Validators(sender) => query!(Validators, validators(), sender),
-		Request::ValidatorGroups(sender) => query!(ValidatorGroups, validator_groups(), sender),
+		Request::Authorities(sender) => query!(Authorities, authorities(), ver = 1, sender),
+		Request::Validators(sender) => query!(Validators, validators(), ver = 1, sender),
+		Request::ValidatorGroups(sender) =>
+			query!(ValidatorGroups, validator_groups(), ver = 1, sender),
 		Request::AvailabilityCores(sender) =>
-			query!(AvailabilityCores, availability_cores(), sender),
-		Request::PersistedValidationData(para, assumption, sender) =>
-			query!(PersistedValidationData, persisted_validation_data(para, assumption), sender),
+			query!(AvailabilityCores, availability_cores(), ver = 1, sender),
+		Request::PersistedValidationData(para, assumption, sender) => query!(
+			PersistedValidationData,
+			persisted_validation_data(para, assumption),
+			ver = 1,
+			sender
+		),
 		Request::AssumedValidationData(para, expected_persisted_validation_data_hash, sender) =>
 			query!(
 				AssumedValidationData,
 				assumed_validation_data(para, expected_persisted_validation_data_hash),
+				ver = 1,
 				sender
 			),
-		Request::CheckValidationOutputs(para, commitments, sender) =>
-			query!(CheckValidationOutputs, check_validation_outputs(para, commitments), sender),
+		Request::CheckValidationOutputs(para, commitments, sender) => query!(
+			CheckValidationOutputs,
+			check_validation_outputs(para, commitments),
+			ver = 1,
+			sender
+		),
 		Request::SessionIndexForChild(sender) =>
-			query!(SessionIndexForChild, session_index_for_child(), sender),
+			query!(SessionIndexForChild, session_index_for_child(), ver = 1, sender),
 		Request::ValidationCode(para, assumption, sender) =>
-			query!(ValidationCode, validation_code(para, assumption), sender),
-		Request::ValidationCodeByHash(validation_code_hash, sender) =>
-			query!(ValidationCodeByHash, validation_code_by_hash(validation_code_hash), sender),
-		Request::CandidatePendingAvailability(para, sender) =>
-			query!(CandidatePendingAvailability, candidate_pending_availability(para), sender),
-		Request::CandidateEvents(sender) => query!(CandidateEvents, candidate_events(), sender),
-		Request::SessionInfo(index, sender) => query!(SessionInfo, session_info(index), sender),
-		Request::DmqContents(id, sender) => query!(DmqContents, dmq_contents(id), sender),
+			query!(ValidationCode, validation_code(para, assumption), ver = 1, sender),
+		Request::ValidationCodeByHash(validation_code_hash, sender) => query!(
+			ValidationCodeByHash,
+			validation_code_by_hash(validation_code_hash),
+			ver = 1,
+			sender
+		),
+		Request::CandidatePendingAvailability(para, sender) => query!(
+			CandidatePendingAvailability,
+			candidate_pending_availability(para),
+			ver = 1,
+			sender
+		),
+		Request::CandidateEvents(sender) =>
+			query!(CandidateEvents, candidate_events(), ver = 1, sender),
+		Request::SessionInfo(index, sender) =>
+			query!(SessionInfo, session_info(index), ver = 1, sender),
+		Request::DmqContents(id, sender) => query!(DmqContents, dmq_contents(id), ver = 1, sender),
 		Request::InboundHrmpChannelsContents(id, sender) =>
-			query!(InboundHrmpChannelsContents, inbound_hrmp_channels_contents(id), sender),
-		Request::CurrentBabeEpoch(sender) => query!(CurrentBabeEpoch, current_epoch(), sender),
-		Request::FetchOnChainVotes(sender) => query!(FetchOnChainVotes, on_chain_votes(), sender),
+			query!(InboundHrmpChannelsContents, inbound_hrmp_channels_contents(id), ver = 1, sender),
+		Request::CurrentBabeEpoch(sender) =>
+			query!(CurrentBabeEpoch, current_epoch(), ver = 1, sender),
+		Request::FetchOnChainVotes(sender) =>
+			query!(FetchOnChainVotes, on_chain_votes(), ver = 1, sender),
 	}
 }
 
diff --git a/polkadot/node/subsystem-types/src/errors.rs b/polkadot/node/subsystem-types/src/errors.rs
index 4de85c6b5f9..14febb2d7db 100644
--- a/polkadot/node/subsystem-types/src/errors.rs
+++ b/polkadot/node/subsystem-types/src/errors.rs
@@ -19,23 +19,27 @@
 use crate::JaegerError;
 
 /// A description of an error causing the runtime API request to be unservable.
-#[derive(Debug, Clone)]
-pub struct RuntimeApiError(String);
-
-impl From<String> for RuntimeApiError {
-	fn from(s: String) -> Self {
-		RuntimeApiError(s)
-	}
-}
+#[derive(thiserror::Error, Debug, Clone)]
+pub enum RuntimeApiError {
+	/// The runtime API cannot be executed due to a
+	#[error("The runtime API '{runtime_api_name}' cannot be executed")]
+	Execution {
+		/// The runtime API being called
+		runtime_api_name: &'static str,
+		/// The wrapped error. Marked as source for tracking the error chain.
+		#[source]
+		source: std::sync::Arc<dyn 'static + std::error::Error + Send + Sync>,
+	},
 
-impl core::fmt::Display for RuntimeApiError {
-	fn fmt(&self, f: &mut core::fmt::Formatter) -> Result<(), core::fmt::Error> {
-		write!(f, "{}", self.0)
-	}
+	/// The runtime API request in question cannot be executed because the runtime at the requested
+	/// relay-parent is an old version.
+	#[error("The API is not supported by the runtime at the relay-parent")]
+	NotSupported {
+		/// The runtime API being called
+		runtime_api_name: &'static str,
+	},
 }
 
-impl std::error::Error for RuntimeApiError {}
-
 /// A description of an error causing the chain API request to be unservable.
 #[derive(Debug, Clone)]
 pub struct ChainApiError {
-- 
GitLab