Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Merged
merged 23 commits into from
Nov 6, 2024
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
3c2cb70
Filter runtime API metadata by version implemented
jsdw Nov 1, 2024
bc0b17a
Add test and fix a coupole of issues
jsdw Nov 1, 2024
c695171
fmt
jsdw Nov 1, 2024
7423f35
Merge branch 'master' into jsdw-versioned-runtime-api-metadata
jsdw Nov 1, 2024
03da4bf
Add comment
jsdw Nov 1, 2024
9738eb7
Add prdoc
jsdw Nov 4, 2024
8bac882
Merge branch 'master' into jsdw-versioned-runtime-api-metadata
jsdw Nov 4, 2024
f99faec
Delete polkadot/Cargo.lock
bkchr Nov 5, 2024
a2defb2
Updateframe-support tests
jsdw Nov 5, 2024
36d8ae0
Merge branch 'jsdw-versioned-runtime-api-metadata' of github.com:pari…
jsdw Nov 5, 2024
9cbb5c5
u32 api_version to align with VERSION, and use VERSION instead of add…
jsdw Nov 5, 2024
5d75c2c
Merge branch 'master' into jsdw-versioned-runtime-api-metadata
jsdw Nov 5, 2024
0c48f03
Fix prdoc
jsdw Nov 5, 2024
f0073db
Merge branch 'jsdw-versioned-runtime-api-metadata' of github.com:pari…
jsdw Nov 5, 2024
b9e7818
Update from jsdw running command 'fmt'
actions-user Nov 5, 2024
7719861
Merge branch 'master' into jsdw-versioned-runtime-api-metadata
jsdw Nov 5, 2024
65bb934
runtime_metadata(impl_version) to filter there
jsdw Nov 5, 2024
cabb755
Merge branch 'jsdw-versioned-runtime-api-metadata' of github.com:pari…
jsdw Nov 5, 2024
048103a
Merge branch 'master' into jsdw-versioned-runtime-api-metadata
jsdw Nov 5, 2024
e710771
Undo frame-support tests change
jsdw Nov 5, 2024
1275f0d
Merge branch 'jsdw-versioned-runtime-api-metadata' of github.com:pari…
jsdw Nov 5, 2024
5cf3857
Update from jsdw running command 'fmt'
actions-user Nov 6, 2024
78c1716
Merge branch 'master' into jsdw-versioned-runtime-api-metadata
bkchr Nov 6, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14,821 changes: 14,821 additions & 0 deletions polkadot/Cargo.lock
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Committed by accident?

Large diffs are not rendered by default.

8 changes: 5 additions & 3 deletions substrate/primitives/api/proc-macro/src/decl_runtime_apis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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();
Expand Down Expand Up @@ -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();
Expand Down
98 changes: 6 additions & 92 deletions substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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};
Expand All @@ -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;
Expand Down Expand Up @@ -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::*;
Expand Down
69 changes: 48 additions & 21 deletions substrate/primitives/api/proc-macro/src/runtime_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();

Expand All @@ -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 {
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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 })
}
));
}

Expand Down
88 changes: 85 additions & 3 deletions substrate/primitives/api/proc-macro/src/utils.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't change any of this code; just moved it to the utils module so that I could use it in runtime_metadata too.

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;
Expand Down
Loading
Loading