Skip to content

sp-api: Propagate deprecation information to metadata from impl_runtime_apis! blocks #6394

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

Open
wants to merge 5 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
16 changes: 16 additions & 0 deletions prdoc/pr_6394.prdoc
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# 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: Propagate deprecation information from `impl_runtime_apis!` blocks

doc:
- audience: Runtime User, Runtime Dev
description: |
Prior to this PR, the metadata for runtime APIs would not propagate deprecation information
from `#[deprecated]` attributes inside `impl_runtime_apis!` blocks. Now such information will be
propagated into the metadata and will override deprecation information from `decl_runtime_apis!` if
it was present.

crates:
- name: sp-api-proc-macro
bump: none
41 changes: 34 additions & 7 deletions substrate/primitives/api/proc-macro/src/runtime_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@

use proc_macro2::TokenStream as TokenStream2;
use quote::quote;
use syn::{parse_quote, spanned::Spanned, ItemImpl, ItemTrait, Result};
use syn::{parse_quote, spanned::Spanned, ImplItem, ItemImpl, ItemTrait, Result};

use crate::utils::{
extract_api_version, extract_impl_trait, filter_cfg_attributes, generate_crate_access,
Expand Down Expand Up @@ -131,12 +131,13 @@ pub fn generate_decl_runtime_metadata<'a>(
methods.push(quote!(
#( #attrs )*
if #version <= impl_version {
let deprecation = if let Some(deprecation) = impl_deprecations.remove(&#method_name) { deprecation } else { #deprecation };
Some(#crate_::metadata_ir::RuntimeApiMethodMetadataIR {
name: #method_name,
inputs: #crate_::vec![ #( #inputs, )* ],
output: #output,
docs: #docs,
deprecation_info: #deprecation,
deprecation_info: deprecation,
})
} else {
None
Expand Down Expand Up @@ -173,17 +174,21 @@ pub fn generate_decl_runtime_metadata<'a>(
#crate_::frame_metadata_enabled! {
#( #attrs )*
#[inline(always)]
pub fn runtime_metadata #impl_generics (impl_version: u32) -> #crate_::metadata_ir::RuntimeApiMetadataIR
pub fn runtime_metadata #impl_generics (
impl_version: u32,
mut impl_deprecations: #crate_::scale_info::prelude::collections::BTreeMap<&str, #crate_::metadata_ir::DeprecationStatusIR>
) -> #crate_::metadata_ir::RuntimeApiMetadataIR
#where_clause
{
let deprecation = if let Some(deprecation) = impl_deprecations.remove(&#trait_name) { deprecation } else { #deprecation };
#crate_::metadata_ir::RuntimeApiMetadataIR {
name: #trait_name,
methods: [ #( #methods, )* ]
.into_iter()
.filter_map(|maybe_m| maybe_m)
.collect(),
docs: #docs,
deprecation_info: #deprecation,
deprecation_info: deprecation,
}
}
}
Expand Down Expand Up @@ -222,6 +227,24 @@ pub fn generate_impl_runtime_metadata(impls: &[ItemImpl]) -> Result<TokenStream2
.expect("Trait path should always contain at least one item; qed")
.ident;

let mut deprecations = vec![];

if impl_.attrs.iter().any(|x| x.path().is_ident("deprecated")) {
let deprecation = crate::utils::get_deprecation(&crate_, &impl_.attrs)?;
deprecations.push(quote! {(stringify!(#trait_name_ident), #deprecation)});
};

for method in &impl_.items {
if let ImplItem::Fn(method) = method {
if method.attrs.iter().any(|x| x.path().is_ident("deprecated")) {
let name = &method.sig.ident;
dbg!(name);
let deprecation = crate::utils::get_deprecation(&crate_, &method.attrs)?;
deprecations.push(quote! {(stringify!(#name), #deprecation)});
Copy link
Contributor

Choose a reason for hiding this comment

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

I forgot, method names are unique or they are prefixed with the trait name in the runtime api?
If they are not unique and prefixed by the trait name we should do same here. Otherwise if 2 runtime API have a method with the same name then deprecation information will propagate to both.

You could even strongly type it to differentiate between trait deprecation and method deprecation, but it can be ok as it is.

};
}
}

// Extract the generics from the trait to pass to the `runtime_metadata`
// function on the hidden module.
let generics = trait_
Expand Down Expand Up @@ -255,6 +278,10 @@ pub fn generate_impl_runtime_metadata(impls: &[ItemImpl]) -> Result<TokenStream2
quote! { #trait_::VERSION }
};

let map = quote! {
#crate_::scale_info::prelude::collections::BTreeMap::from([ #( #deprecations, )*])
};

// Handle the case where eg `#[cfg_attr(feature = "foo", api_version(4))]` is
// present by using that version only when the feature is enabled and falling
// back to the above version if not.
Expand All @@ -263,14 +290,14 @@ pub fn generate_impl_runtime_metadata(impls: &[ItemImpl]) -> Result<TokenStream2
let cfg_version = cfg_version.1;
quote! {{
if cfg!(feature = #cfg_feature) {
#trait_::runtime_metadata::#generics(#cfg_version)
#trait_::runtime_metadata::#generics(#cfg_version, #map)
} else {
#trait_::runtime_metadata::#generics(#base_version)
#trait_::runtime_metadata::#generics(#base_version, #map)
}
}}
} else {
quote! {
#trait_::runtime_metadata::#generics(#base_version)
#trait_::runtime_metadata::#generics(#base_version, #map)
}
}
};
Expand Down
1 change: 1 addition & 0 deletions substrate/primitives/api/test/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ sp-state-machine = { workspace = true, default-features = true }
trybuild = { workspace = true }
rustversion = { workspace = true }
scale-info = { features = ["derive"], workspace = true }
sp-metadata-ir = { workspace = true }

[dev-dependencies]
criterion = { workspace = true, default-features = true }
Expand Down
18 changes: 18 additions & 0 deletions substrate/primitives/api/test/tests/decl_and_impl.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
#![allow(useless_deprecated)]

use sp_api::{
decl_runtime_apis, impl_runtime_apis, mock_impl_runtime_apis, ApiError, ApiExt, RuntimeApiInfo,
Expand Down Expand Up @@ -95,6 +96,7 @@ impl_runtime_apis! {
}

impl self::ApiWithCustomVersion<Block> for Runtime {
#[deprecated = "example"]
fn same_name() {}
}

Expand Down Expand Up @@ -367,3 +369,19 @@ fn runtime_api_metadata_matches_version_implemented() {
],
);
}

#[test]
fn allow_deprecation_in_impl() {
let rt = Runtime {};
let runtime_metadata = rt.runtime_metadata();

// Check that the metadata for some runtime API matches expectation.
let Some(api) = runtime_metadata.iter().find(|api| api.name == "ApiWithCustomVersion") else {
panic!("Can't find runtime API 'ApiWithCustomVersion'");
};
let Some(method) = api.methods.iter().find(|method| method.name == "same_name") else {
panic!("Can't find runtime API method 'some_name'");
};
let expected = sp_metadata_ir::DeprecationStatusIR::Deprecated { note: "example", since: None };
assert_eq!(method.deprecation_info, expected);
}
Loading