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/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.
@@ -193,8 +192,6 @@ fn generate_runtime_decls(decls: &[ItemTrait]) -> Result<TokenStream> {
 			get_api_version(&found_attributes).map(|v| generate_runtime_api_version(v as u32))?;
 		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();
@@ -255,6 +252,11 @@ fn generate_runtime_decls(decls: &[ItemTrait]) -> Result<TokenStream> {
 			_ => (),
 		});
 
+		let versioned_methods: Vec<(&syn::TraitItemFn, u64)> = methods_by_version.iter().flat_map(|(&version, methods)| {
+			methods.iter().map(move |method| (method, version))
+		}).collect();
+		let metadata = crate::runtime_metadata::generate_decl_runtime_metadata(&decl, trait_api_version, &versioned_methods);
+
 		let versioned_api_traits = generate_versioned_api_traits(decl.clone(), methods_by_version);
 
 		let main_api_ident = decl.ident.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;
@@ -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(decl: &ItemTrait, base_version: u64, versioned_methods: &[(&syn::TraitItemFn, u64)]) -> 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 {
 		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,
@@ -181,6 +170,7 @@ pub fn generate_decl_runtime_metadata(decl: &ItemTrait) -> TokenStream2 {
 			{
 				#crate_::metadata_ir::RuntimeApiMetadataIR {
 					name: #trait_name,
+					base_version: #base_version,
 					methods: #crate_::vec![ #( #methods, )* ],
 					docs: #docs,
 					deprecation_info: #deprecation,
@@ -242,10 +232,47 @@ 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!{ base_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()
+			{
+				let md = #trait_::runtime_metadata::#generics();
+				let base_version = md.base_version;
+				md.keeping_methods(|method| { #api_version_comparison })
+			}
 		));
 	}
 

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

@@ -21,9 +21,9 @@ use proc_macro2::{Span, TokenStream};
 use proc_macro_crate::{crate_name, FoundCrate};
 use quote::{format_ident, quote};
 use syn::{
-	parse_quote, punctuated::Punctuated, spanned::Spanned, token::And, Attribute, Error, Expr,
-	ExprLit, FnArg, GenericArgument, Ident, ItemImpl, Lit, Meta, MetaNameValue, Pat, Path,
-	PathArguments, Result, ReturnType, Signature, Token, Type, TypePath,
+	parse_quote, parenthesized, punctuated::Punctuated, spanned::Spanned, token::And, Attribute, Error, Expr,
+	ExprLit, FnArg, GenericArgument, Ident, ItemImpl, Lit, LitStr, LitInt, Meta, MetaNameValue, 
+	Pat, Path, PathArguments, Result, ReturnType, Signature, Token, Type, TypePath,
 };
 
 /// Generates the access to the `sc_client` crate.
@@ -334,6 +334,88 @@ pub fn get_deprecation(crate_: &TokenStream, attrs: &[syn::Attribute]) -> Result
 		.unwrap_or_else(|| Ok(quote! {#crate_::metadata_ir::DeprecationStatusIR::NotDeprecated}))
 }
 
+/// Represents an API version.
+pub 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`
+pub fn extract_api_version(attrs: &[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)?,
+	})
+}
+
+/// Parse feature flagged api_version.
+/// E.g. `#[cfg_attr(feature = "enable-staging-api", api_version(99))]`
+fn extract_cfg_api_version(attrs: &[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)))
+}
+
 #[cfg(test)]
 mod tests {
 	use assert_matches::assert_matches;