Don't expose metadata for Runtime APIs that haven't been implemented

prdoc/pr_6337.prdoc

@@ -0,0 +1,17 @@
+# Schema: Polkadot SDK PRDoc Schema (prdoc) v1.0.0
+# See doc at https://raw.githubusercontent.com/paritytech/polkadot-sdk/master/prdoc/schema_user.json
+
+title: Don't expose metadata for Runtime APIs that haven't been implemented
+
+doc:
+  - audience: Runtime User
+    description: |
+      Prior to this PR, the metadata for runtime APIs would contain all methods for the 
+      latest version of each API, regardless of which version a runtime implements. This 
+      PR fixes that, so that the runtime API metadata reflects what is actually implemented.
+
+crates:
+  - name: sp-api
+    bump: patch
+  - name: sp-metadata-ir
+    bump: major

substrate/frame/support/test/tests/runtime_metadata.rs

@@ -128,6 +128,7 @@ fn runtime_metadata() {
 			methods: vec![
 				RuntimeApiMethodMetadataIR {
 					name: "test",
+					version: 1,
 					inputs: vec![RuntimeApiMethodParamMetadataIR::<MetaForm> {
 						name: "data",
 						ty: meta_type::<u64>(),
@@ -138,6 +139,7 @@ fn runtime_metadata() {
 				},
 				RuntimeApiMethodMetadataIR {
 					name: "something_with_block",
+					version: 1,
 					inputs: vec![RuntimeApiMethodParamMetadataIR::<MetaForm> {
 						name: "block",
 						ty: meta_type::<Block>(),
@@ -148,6 +150,7 @@ fn runtime_metadata() {
 				},
 				RuntimeApiMethodMetadataIR {
 					name: "function_with_two_args",
+					version: 1,
 					inputs: vec![
 						RuntimeApiMethodParamMetadataIR::<MetaForm> {
 							name: "data",
@@ -167,6 +170,7 @@ fn runtime_metadata() {
 				},
 				RuntimeApiMethodMetadataIR {
 					name: "same_name",
+					version: 1,
 					inputs: vec![],
 					output: meta_type::<()>(),
 					docs: vec![],
@@ -177,8 +181,9 @@ fn runtime_metadata() {
 			},
 				RuntimeApiMethodMetadataIR {
 					name: "wild_card",
+					version: 1,
 					inputs: vec![RuntimeApiMethodParamMetadataIR::<MetaForm> {
-						name: "_",
+						name: "__runtime_api_generated_name_0__",
 						ty: meta_type::<u32>(),
 					}],
 					output: meta_type::<()>(),
@@ -202,13 +207,15 @@ fn runtime_metadata() {
 			methods: vec![
 				RuntimeApiMethodMetadataIR {
 					name: "version",
+					version: 5,
 					inputs: vec![],
 					output: meta_type::<sp_version::RuntimeVersion>(),
 					docs: maybe_docs(vec![" Returns the version of the runtime."]),
 					deprecation_info: DeprecationStatusIR::NotDeprecated,
 				},
 				RuntimeApiMethodMetadataIR {
 					name: "execute_block",
+					version: 5,
 					inputs: vec![RuntimeApiMethodParamMetadataIR::<MetaForm> {
 						name: "block",
 						ty: meta_type::<Block>(),
@@ -220,6 +227,7 @@ fn runtime_metadata() {
 				},
 				RuntimeApiMethodMetadataIR {
 					name: "initialize_block",
+					version: 5,
 					inputs: vec![RuntimeApiMethodParamMetadataIR::<MetaForm> {
 						name: "header",
 						ty: meta_type::<&<Block as BlockT>::Header>(),

substrate/primitives/api/proc-macro/src/decl_runtime_apis.rs

@@ -42,7 +42,6 @@ use syn::{
 	Attribute, FnArg, GenericParam, Generics, Ident, ItemTrait, LitInt, LitStr, TraitBound,
 	TraitItem, TraitItemFn,
 };
-
 use std::collections::{BTreeMap, HashMap};
 
 /// The structure used for parsing the runtime api declarations.
@@ -133,7 +132,7 @@ fn remove_supported_attributes(attrs: &mut Vec<Attribute>) -> HashMap<&'static s
 /// ```
 fn generate_versioned_api_traits(
 	api: ItemTrait,
-	methods: BTreeMap<u64, Vec<TraitItemFn>>,
+	methods: BTreeMap<u32, Vec<TraitItemFn>>,
 ) -> Vec<ItemTrait> {
 	let mut result = Vec::<ItemTrait>::new();
 	for (version, _) in &methods {
@@ -190,14 +189,12 @@ fn generate_runtime_decls(decls: &[ItemTrait]) -> Result<TokenStream> {
 		let mod_name = generate_runtime_mod_name_for_trait(&decl.ident);
 		let found_attributes = remove_supported_attributes(&mut decl.attrs);
 		let api_version =
-			get_api_version(&found_attributes).map(|v| generate_runtime_api_version(v as u32))?;
+			get_api_version(&found_attributes).map(generate_runtime_api_version)?;
 		let id = generate_runtime_api_id(&decl.ident.to_string());
 
-		let metadata = crate::runtime_metadata::generate_decl_runtime_metadata(&decl);
-
 		let trait_api_version = get_api_version(&found_attributes)?;
 
-		let mut methods_by_version: BTreeMap<u64, Vec<TraitItemFn>> = BTreeMap::new();
+		let mut methods_by_version: BTreeMap<u32, Vec<TraitItemFn>> = BTreeMap::new();
 
 		// Process the items in the declaration. The filter_map function below does a lot of stuff
 		// because the method attributes are stripped at this point
@@ -255,6 +252,11 @@ fn generate_runtime_decls(decls: &[ItemTrait]) -> Result<TokenStream> {
 			_ => (),
 		});
 
+		let versioned_methods_iter = methods_by_version.iter().flat_map(|(&version, methods)| {
+			methods.iter().map(move |method| (method, version))
+		});
+		let metadata = crate::runtime_metadata::generate_decl_runtime_metadata(&decl, versioned_methods_iter);
+
 		let versioned_api_traits = generate_versioned_api_traits(decl.clone(), methods_by_version);
 
 		let main_api_ident = decl.ident.clone();
@@ -505,7 +507,7 @@ fn generate_runtime_api_version(version: u32) -> TokenStream {
 }
 
 /// Generates the implementation of `RuntimeApiInfo` for the given trait.
-fn generate_runtime_info_impl(trait_: &ItemTrait, version: u64) -> TokenStream {
+fn generate_runtime_info_impl(trait_: &ItemTrait, version: u32) -> TokenStream {
 	let trait_name = &trait_.ident;
 	let crate_ = generate_crate_access();
 	let id = generate_runtime_api_id(&trait_name.to_string());
@@ -537,15 +539,15 @@ fn generate_runtime_info_impl(trait_: &ItemTrait, version: u64) -> TokenStream {
 }
 
 /// Get changed in version from the user given attribute or `Ok(None)`, if no attribute was given.
-fn get_changed_in(found_attributes: &HashMap<&'static str, Attribute>) -> Result<Option<u64>> {
+fn get_changed_in(found_attributes: &HashMap<&'static str, Attribute>) -> Result<Option<u32>> {
 	found_attributes
 		.get(&CHANGED_IN_ATTRIBUTE)
 		.map(|v| parse_runtime_api_version(v).map(Some))
 		.unwrap_or(Ok(None))
 }
 
 /// Get the api version from the user given attribute or `Ok(1)`, if no attribute was given.
-fn get_api_version(found_attributes: &HashMap<&'static str, Attribute>) -> Result<u64> {
+fn get_api_version(found_attributes: &HashMap<&'static str, Attribute>) -> Result<u32> {
 	found_attributes
 		.get(&API_VERSION_ATTRIBUTE)
 		.map(parse_runtime_api_version)
@@ -610,7 +612,7 @@ impl CheckTraitDecl {
 	///
 	/// Any error is stored in `self.errors`.
 	fn check_method_declarations<'a>(&mut self, methods: impl Iterator<Item = &'a TraitItemFn>) {
-		let mut method_to_signature_changed = HashMap::<Ident, Vec<Option<u64>>>::new();
+		let mut method_to_signature_changed = HashMap::<Ident, Vec<Option<u32>>>::new();
 
 		methods.into_iter().for_each(|method| {
 			let attributes = remove_supported_attributes(&mut method.attrs.clone());

substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs

@@ -15,14 +15,11 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-use crate::{
-	common::API_VERSION_ATTRIBUTE,
-	utils::{
-		extract_block_type_from_trait_path, extract_impl_trait,
-		extract_parameter_names_types_and_borrows, generate_crate_access,
-		generate_runtime_mod_name_for_trait, parse_runtime_api_version, prefix_function_with_trait,
-		versioned_trait_name, AllowSelfRefInParameters, RequireQualifiedTraitPath,
-	},
+use crate::utils::{
+	extract_api_version, extract_block_type_from_trait_path, extract_impl_trait,
+	extract_parameter_names_types_and_borrows, generate_crate_access,
+	generate_runtime_mod_name_for_trait, prefix_function_with_trait,
+	versioned_trait_name, ApiVersion, AllowSelfRefInParameters, RequireQualifiedTraitPath,
 };
 
 use proc_macro2::{Span, TokenStream};
@@ -31,11 +28,10 @@ use quote::quote;
 
 use syn::{
 	fold::{self, Fold},
-	parenthesized,
 	parse::{Error, Parse, ParseStream, Result},
 	parse_macro_input, parse_quote,
 	spanned::Spanned,
-	Attribute, Ident, ImplItem, ItemImpl, LitInt, LitStr, Path, Signature, Type, TypePath,
+	Attribute, Ident, ImplItem, ItemImpl, Path, Signature, Type, TypePath,
 };
 
 use std::collections::HashMap;
@@ -466,7 +462,7 @@ fn extend_with_runtime_decl_path(mut trait_: Path) -> Path {
 	trait_
 }
 
-fn extend_with_api_version(mut trait_: Path, version: Option<u64>) -> Path {
+fn extend_with_api_version(mut trait_: Path, version: Option<u32>) -> Path {
 	let version = if let Some(v) = version {
 		v
 	} else {
@@ -841,88 +837,6 @@ fn filter_cfg_attrs(attrs: &[Attribute]) -> Vec<Attribute> {
 	attrs.iter().filter(|a| a.path().is_ident("cfg")).cloned().collect()
 }
 
-/// Parse feature flagged api_version.
-/// E.g. `#[cfg_attr(feature = "enable-staging-api", api_version(99))]`
-fn extract_cfg_api_version(attrs: &Vec<Attribute>, span: Span) -> Result<Option<(String, u64)>> {
-	let cfg_attrs = attrs.iter().filter(|a| a.path().is_ident("cfg_attr")).collect::<Vec<_>>();
-
-	let mut cfg_api_version_attr = Vec::new();
-	for cfg_attr in cfg_attrs {
-		let mut feature_name = None;
-		let mut api_version = None;
-		cfg_attr.parse_nested_meta(|m| {
-			if m.path.is_ident("feature") {
-				let a = m.value()?;
-				let b: LitStr = a.parse()?;
-				feature_name = Some(b.value());
-			} else if m.path.is_ident(API_VERSION_ATTRIBUTE) {
-				let content;
-				parenthesized!(content in m.input);
-				let ver: LitInt = content.parse()?;
-				api_version = Some(ver.base10_parse::<u64>()?);
-			}
-			Ok(())
-		})?;
-
-		// If there is a cfg attribute containing api_version - save if for processing
-		if let (Some(feature_name), Some(api_version)) = (feature_name, api_version) {
-			cfg_api_version_attr.push((feature_name, api_version, cfg_attr.span()));
-		}
-	}
-
-	if cfg_api_version_attr.len() > 1 {
-		let mut err = Error::new(span, format!("Found multiple feature gated api versions (cfg attribute with nested `{}` attribute). This is not supported.", API_VERSION_ATTRIBUTE));
-		for (_, _, attr_span) in cfg_api_version_attr {
-			err.combine(Error::new(attr_span, format!("`{}` found here", API_VERSION_ATTRIBUTE)));
-		}
-
-		return Err(err);
-	}
-
-	Ok(cfg_api_version_attr
-		.into_iter()
-		.next()
-		.map(|(feature, name, _)| (feature, name)))
-}
-
-/// Represents an API version.
-struct ApiVersion {
-	/// Corresponds to `#[api_version(X)]` attribute.
-	pub custom: Option<u64>,
-	/// Corresponds to `#[cfg_attr(feature = "enable-staging-api", api_version(99))]`
-	/// attribute. `String` is the feature name, `u64` the staging api version.
-	pub feature_gated: Option<(String, u64)>,
-}
-
-// Extracts the value of `API_VERSION_ATTRIBUTE` and handles errors.
-// Returns:
-// - Err if the version is malformed
-// - `ApiVersion` on success. If a version is set or not is determined by the fields of `ApiVersion`
-fn extract_api_version(attrs: &Vec<Attribute>, span: Span) -> Result<ApiVersion> {
-	// First fetch all `API_VERSION_ATTRIBUTE` values (should be only one)
-	let api_ver = attrs
-		.iter()
-		.filter(|a| a.path().is_ident(API_VERSION_ATTRIBUTE))
-		.collect::<Vec<_>>();
-
-	if api_ver.len() > 1 {
-		return Err(Error::new(
-			span,
-			format!(
-				"Found multiple #[{}] attributes for an API implementation. \
-				Each runtime API can have only one version.",
-				API_VERSION_ATTRIBUTE
-			),
-		));
-	}
-
-	// Parse the runtime version if there exists one.
-	Ok(ApiVersion {
-		custom: api_ver.first().map(|v| parse_runtime_api_version(v)).transpose()?,
-		feature_gated: extract_cfg_api_version(attrs, span)?,
-	})
-}
-
 #[cfg(test)]
 mod tests {
 	use super::*;

substrate/primitives/api/proc-macro/src/runtime_metadata.rs

@@ -17,14 +17,12 @@
 
 use proc_macro2::TokenStream as TokenStream2;
 use quote::quote;
-use syn::{parse_quote, ItemImpl, ItemTrait, Result};
-
-use crate::{
-	common::CHANGED_IN_ATTRIBUTE,
-	utils::{
-		extract_impl_trait, filter_cfg_attributes, generate_crate_access,
-		generate_runtime_mod_name_for_trait, get_doc_literals, RequireQualifiedTraitPath,
-	},
+use syn::{parse_quote, spanned::Spanned, ItemImpl, ItemTrait, Result};
+
+use crate::utils::{
+	extract_api_version,
+	extract_impl_trait, filter_cfg_attributes, generate_crate_access,
+	generate_runtime_mod_name_for_trait, get_doc_literals, RequireQualifiedTraitPath,
 };
 
 /// Get the type parameter argument without lifetime or mutability
@@ -72,7 +70,7 @@ fn collect_docs(attrs: &[syn::Attribute], crate_: &TokenStream2) -> TokenStream2
 ///
 /// The metadata is exposed as a generic function on the hidden module
 /// of the trait generated by the `decl_runtime_apis`.
-pub fn generate_decl_runtime_metadata(decl: &ItemTrait) -> TokenStream2 {
+pub fn generate_decl_runtime_metadata<'a>(decl: &ItemTrait, versioned_methods_iter: impl Iterator<Item=(&'a syn::TraitItemFn, u32)>) -> TokenStream2 {
 	let crate_ = generate_crate_access();
 	let mut methods = Vec::new();
 
@@ -86,17 +84,7 @@ pub fn generate_decl_runtime_metadata(decl: &ItemTrait) -> TokenStream2 {
 	// This restricts the bounds at the metadata level, without needing to modify the `BlockT`
 	// itself, since the concrete implementations are already satisfying `TypeInfo`.
 	let mut where_clause = Vec::new();
-	for item in &decl.items {
-		// Collect metadata for methods only.
-		let syn::TraitItem::Fn(method) = item else { continue };
-
-		// Collect metadata only for the latest methods.
-		let is_changed_in =
-			method.attrs.iter().any(|attr| attr.path().is_ident(CHANGED_IN_ATTRIBUTE));
-		if is_changed_in {
-			continue;
-		}
-
+	for (method, version) in versioned_methods_iter {
 		let mut inputs = Vec::new();
 		let signature = &method.sig;
 		for input in &signature.inputs {
@@ -139,6 +127,7 @@ pub fn generate_decl_runtime_metadata(decl: &ItemTrait) -> TokenStream2 {
 			#( #attrs )*
 			#crate_::metadata_ir::RuntimeApiMethodMetadataIR {
 				name: #method_name,
+				version: #version,
 				inputs: #crate_::vec![ #( #inputs, )* ],
 				output: #output,
 				docs: #docs,
@@ -242,10 +231,43 @@ pub fn generate_impl_runtime_metadata(impls: &[ItemImpl]) -> Result<TokenStream2
 			*segment = parse_quote!(#mod_name);
 		}
 
+		// We apply this filter to remove any API methods that we are not actually implementing
+		// owing to the runtime API impl being for a lower version than what is declared.
+		let api_version_comparison = {
+			let api_version = extract_api_version(&impl_.attrs, impl_.span())?;
+
+			// If we've annotated an api_version, that defines the methods we need to impl.
+			// If we haven't, then by default we are implementing methods for whatever the
+			// base version of the declared runtime API is.
+			let base_version = if let Some(version) = api_version.custom {
+				quote!{ #version }
+			} else {
+				quote!{ #trait_::VERSION }
+			};
+
+			// If we have used a cfg_attr to determine which version we're implementing, then
+			// translate that to cfg attrs to conditionally check for that version. Else, we are
+			// looking for any API whose version is greater than the base version defined above.
+			if let Some(cfg_version) = api_version.feature_gated {
+				let cfg_feature = cfg_version.0;
+				let cfg_version = cfg_version.1;
+				quote! {
+					#[cfg(feature = #cfg_feature)]
+					{ method.version <= #cfg_version }
+					#[cfg(not(feature = #cfg_feature))]
+					{ method.version <= #base_version }
+				}
+			} else {
+				quote! {
+					method.version <= #base_version
+				}
+			}
+		};
+
 		let attrs = filter_cfg_attributes(&impl_.attrs);
 		metadata.push(quote!(
 			#( #attrs )*
-			#trait_::runtime_metadata::#generics()
+			#trait_::runtime_metadata::#generics().keeping_methods(|method| { #api_version_comparison })
 		));
 	}