From 372708d7a2fa63c175b6c3351d57e8eb0bb84966 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Bastian=20K=C3=B6cher?= <git@kchr.de> Date: Thu, 1 Jun 2023 15:29:32 +0100 Subject: [PATCH] sp-api: Make the generated code act based on `std` in `sp-api` (#14267) * sp-api: Make the generated code act based on `std` in `sp-api` Instead of letting the macro generate code that checks if the `std` feature is enabled, it will now generate code that checks if the `std` feature is enabled for the `sp-api` crate. The old implementation basically required that the crate in which the macro was used, had a `std` feature. Now we don't have this requirement anymore and act accordingly the feature in `sp-api` directly. * Missing feature! --------- Co-authored-by: parity-processbot <> --- substrate/frame/executive/Cargo.toml | 1 + .../api/proc-macro/src/decl_runtime_apis.rs | 21 +- .../api/proc-macro/src/impl_runtime_apis.rs | 344 +++++++++--------- substrate/primitives/api/src/lib.rs | 3 + substrate/primitives/api/test/Cargo.toml | 5 - substrate/primitives/core/src/lib.rs | 12 + 6 files changed, 200 insertions(+), 186 deletions(-) diff --git a/substrate/frame/executive/Cargo.toml b/substrate/frame/executive/Cargo.toml index dfc8e85afd8..2532df31682 100644 --- a/substrate/frame/executive/Cargo.toml +++ b/substrate/frame/executive/Cargo.toml @@ -42,6 +42,7 @@ std = [ "codec/std", "frame-support/std", "frame-system/std", + "frame-try-runtime/std", "scale-info/std", "sp-core/std", "sp-io/std", diff --git a/substrate/primitives/api/proc-macro/src/decl_runtime_apis.rs b/substrate/primitives/api/proc-macro/src/decl_runtime_apis.rs index cde33c19016..ca2bd6b070a 100644 --- a/substrate/primitives/api/proc-macro/src/decl_runtime_apis.rs +++ b/substrate/primitives/api/proc-macro/src/decl_runtime_apis.rs @@ -539,8 +539,6 @@ impl<'a> Fold for ToClientSideDecl<'a> { input.supertraits.push(parse_quote!( #crate_::Core<#block_ident> )); } - // The client side trait is only required when compiling with the feature `std` or `test`. - input.attrs.push(parse_quote!( #[cfg(any(feature = "std", test))] )); input.items = self.fold_item_trait_items(input.items, input.generics.params.len()); fold::fold_item_trait(self, input) @@ -584,12 +582,13 @@ fn generate_runtime_info_impl(trait_: &ItemTrait, version: u64) -> TokenStream { }); quote!( - #[cfg(any(feature = "std", test))] - impl < #( #impl_generics, )* > #crate_::RuntimeApiInfo - for dyn #trait_name < #( #ty_generics, )* > - { - #id - #version + #crate_::std_enabled! { + impl < #( #impl_generics, )* > #crate_::RuntimeApiInfo + for dyn #trait_name < #( #ty_generics, )* > + { + #id + #version + } } ) } @@ -636,7 +635,11 @@ fn generate_client_side_decls(decls: &[ItemTrait]) -> Result<TokenStream> { let runtime_info = api_version.map(|v| generate_runtime_info_impl(&decl, v))?; - result.push(quote!( #decl #runtime_info #( #errors )* )); + result.push(quote!( + #crate_::std_enabled! { #decl } + #runtime_info + #( #errors )* + )); } Ok(quote!( #( #result )* )) diff --git a/substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs b/substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs index b8dcf625df4..fa933ceb917 100644 --- a/substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs +++ b/substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs @@ -172,11 +172,12 @@ fn generate_dispatch_function(impls: &[ItemImpl]) -> Result<TokenStream> { }); Ok(quote!( - #[cfg(feature = "std")] - pub fn dispatch(method: &str, mut #data: &[u8]) -> Option<Vec<u8>> { - match method { - #( #impl_calls )* - _ => None, + #c::std_enabled! { + pub fn dispatch(method: &str, mut #data: &[u8]) -> Option<Vec<u8>> { + match method { + #( #impl_calls )* + _ => None, + } } } )) @@ -195,22 +196,23 @@ fn generate_wasm_interface(impls: &[ItemImpl]) -> Result<TokenStream> { Ident::new(&prefix_function_with_trait(&trait_, &fn_name), Span::call_site()); quote!( - #( #attrs )* - #[cfg(not(feature = "std"))] - #[no_mangle] - pub unsafe fn #fn_name(input_data: *mut u8, input_len: usize) -> u64 { - let mut #input = if input_len == 0 { - &[0u8; 0] - } else { - unsafe { - #c::slice::from_raw_parts(input_data, input_len) - } - }; - - #c::init_runtime_logger(); - - let output = (move || { #impl_ })(); - #c::to_substrate_wasm_fn_return_value(&output) + #c::std_disabled! { + #( #attrs )* + #[no_mangle] + pub unsafe fn #fn_name(input_data: *mut u8, input_len: usize) -> u64 { + let mut #input = if input_len == 0 { + &[0u8; 0] + } else { + unsafe { + #c::slice::from_raw_parts(input_data, input_len) + } + }; + + #c::init_runtime_logger(); + + let output = (move || { #impl_ })(); + #c::to_substrate_wasm_fn_return_value(&output) + } } ) }); @@ -224,175 +226,173 @@ fn generate_runtime_api_base_structures() -> Result<TokenStream> { Ok(quote!( pub struct RuntimeApi {} /// Implements all runtime apis for the client side. - #[cfg(any(feature = "std", test))] - pub struct RuntimeApiImpl<Block: #crate_::BlockT, C: #crate_::CallApiAt<Block> + 'static> { - call: &'static C, - commit_on_success: std::cell::RefCell<bool>, - changes: std::cell::RefCell<#crate_::OverlayedChanges>, - storage_transaction_cache: std::cell::RefCell< - #crate_::StorageTransactionCache<Block, C::StateBackend> - >, - recorder: std::option::Option<#crate_::ProofRecorder<Block>>, - } + #crate_::std_enabled! { + pub struct RuntimeApiImpl<Block: #crate_::BlockT, C: #crate_::CallApiAt<Block> + 'static> { + call: &'static C, + commit_on_success: std::cell::RefCell<bool>, + changes: std::cell::RefCell<#crate_::OverlayedChanges>, + storage_transaction_cache: std::cell::RefCell< + #crate_::StorageTransactionCache<Block, C::StateBackend> + >, + recorder: std::option::Option<#crate_::ProofRecorder<Block>>, + } - #[cfg(any(feature = "std", test))] - impl<Block: #crate_::BlockT, C: #crate_::CallApiAt<Block>> #crate_::ApiExt<Block> for - RuntimeApiImpl<Block, C> - { - type StateBackend = C::StateBackend; + impl<Block: #crate_::BlockT, C: #crate_::CallApiAt<Block>> #crate_::ApiExt<Block> for + RuntimeApiImpl<Block, C> + { + type StateBackend = C::StateBackend; - fn execute_in_transaction<F: FnOnce(&Self) -> #crate_::TransactionOutcome<R>, R>( - &self, - call: F, - ) -> R where Self: Sized { - self.start_transaction(); + fn execute_in_transaction<F: FnOnce(&Self) -> #crate_::TransactionOutcome<R>, R>( + &self, + call: F, + ) -> R where Self: Sized { + self.start_transaction(); - *std::cell::RefCell::borrow_mut(&self.commit_on_success) = false; - let res = call(self); - *std::cell::RefCell::borrow_mut(&self.commit_on_success) = true; + *std::cell::RefCell::borrow_mut(&self.commit_on_success) = false; + let res = call(self); + *std::cell::RefCell::borrow_mut(&self.commit_on_success) = true; - self.commit_or_rollback(std::matches!(res, #crate_::TransactionOutcome::Commit(_))); + self.commit_or_rollback(std::matches!(res, #crate_::TransactionOutcome::Commit(_))); - res.into_inner() - } + res.into_inner() + } - fn has_api<A: #crate_::RuntimeApiInfo + ?Sized>( - &self, - at: <Block as #crate_::BlockT>::Hash, - ) -> std::result::Result<bool, #crate_::ApiError> where Self: Sized { - #crate_::CallApiAt::<Block>::runtime_version_at(self.call, at) + fn has_api<A: #crate_::RuntimeApiInfo + ?Sized>( + &self, + at: <Block as #crate_::BlockT>::Hash, + ) -> std::result::Result<bool, #crate_::ApiError> where Self: Sized { + #crate_::CallApiAt::<Block>::runtime_version_at(self.call, at) .map(|v| #crate_::RuntimeVersion::has_api_with(&v, &A::ID, |v| v == A::VERSION)) - } + } - fn has_api_with<A: #crate_::RuntimeApiInfo + ?Sized, P: Fn(u32) -> bool>( - &self, - at: <Block as #crate_::BlockT>::Hash, - pred: P, - ) -> std::result::Result<bool, #crate_::ApiError> where Self: Sized { - #crate_::CallApiAt::<Block>::runtime_version_at(self.call, at) + fn has_api_with<A: #crate_::RuntimeApiInfo + ?Sized, P: Fn(u32) -> bool>( + &self, + at: <Block as #crate_::BlockT>::Hash, + pred: P, + ) -> std::result::Result<bool, #crate_::ApiError> where Self: Sized { + #crate_::CallApiAt::<Block>::runtime_version_at(self.call, at) .map(|v| #crate_::RuntimeVersion::has_api_with(&v, &A::ID, pred)) - } + } - fn api_version<A: #crate_::RuntimeApiInfo + ?Sized>( - &self, - at: <Block as #crate_::BlockT>::Hash, - ) -> std::result::Result<Option<u32>, #crate_::ApiError> where Self: Sized { - #crate_::CallApiAt::<Block>::runtime_version_at(self.call, at) + fn api_version<A: #crate_::RuntimeApiInfo + ?Sized>( + &self, + at: <Block as #crate_::BlockT>::Hash, + ) -> std::result::Result<Option<u32>, #crate_::ApiError> where Self: Sized { + #crate_::CallApiAt::<Block>::runtime_version_at(self.call, at) .map(|v| #crate_::RuntimeVersion::api_version(&v, &A::ID)) - } + } - fn record_proof(&mut self) { - self.recorder = std::option::Option::Some(std::default::Default::default()); - } + fn record_proof(&mut self) { + self.recorder = std::option::Option::Some(std::default::Default::default()); + } - fn proof_recorder(&self) -> std::option::Option<#crate_::ProofRecorder<Block>> { - std::clone::Clone::clone(&self.recorder) - } + fn proof_recorder(&self) -> std::option::Option<#crate_::ProofRecorder<Block>> { + std::clone::Clone::clone(&self.recorder) + } - fn extract_proof( - &mut self, - ) -> std::option::Option<#crate_::StorageProof> { - let recorder = std::option::Option::take(&mut self.recorder); - std::option::Option::map(recorder, |recorder| { - #crate_::ProofRecorder::<Block>::drain_storage_proof(recorder) - }) - } + fn extract_proof( + &mut self, + ) -> std::option::Option<#crate_::StorageProof> { + let recorder = std::option::Option::take(&mut self.recorder); + std::option::Option::map(recorder, |recorder| { + #crate_::ProofRecorder::<Block>::drain_storage_proof(recorder) + }) + } - fn into_storage_changes( - &self, - backend: &Self::StateBackend, - parent_hash: Block::Hash, - ) -> core::result::Result< - #crate_::StorageChanges<C::StateBackend, Block>, + fn into_storage_changes( + &self, + backend: &Self::StateBackend, + parent_hash: Block::Hash, + ) -> core::result::Result< + #crate_::StorageChanges<C::StateBackend, Block>, String - > where Self: Sized { - let state_version = #crate_::CallApiAt::<Block>::runtime_version_at(self.call, std::clone::Clone::clone(&parent_hash)) - .map(|v| #crate_::RuntimeVersion::state_version(&v)) - .map_err(|e| format!("Failed to get state version: {}", e))?; - - #crate_::OverlayedChanges::into_storage_changes( - std::cell::RefCell::take(&self.changes), - backend, - core::cell::RefCell::take(&self.storage_transaction_cache), - state_version, - ) + > where Self: Sized { + let state_version = #crate_::CallApiAt::<Block>::runtime_version_at(self.call, std::clone::Clone::clone(&parent_hash)) + .map(|v| #crate_::RuntimeVersion::state_version(&v)) + .map_err(|e| format!("Failed to get state version: {}", e))?; + + #crate_::OverlayedChanges::into_storage_changes( + std::cell::RefCell::take(&self.changes), + backend, + core::cell::RefCell::take(&self.storage_transaction_cache), + state_version, + ) + } } - } - #[cfg(any(feature = "std", test))] - impl<Block: #crate_::BlockT, C> #crate_::ConstructRuntimeApi<Block, C> - for RuntimeApi - where - C: #crate_::CallApiAt<Block> + 'static, - { - type RuntimeApi = RuntimeApiImpl<Block, C>; - - fn construct_runtime_api<'a>( - call: &'a C, - ) -> #crate_::ApiRef<'a, Self::RuntimeApi> { - RuntimeApiImpl { - call: unsafe { std::mem::transmute(call) }, - commit_on_success: true.into(), - changes: std::default::Default::default(), - recorder: std::default::Default::default(), - storage_transaction_cache: std::default::Default::default(), - }.into() + impl<Block: #crate_::BlockT, C> #crate_::ConstructRuntimeApi<Block, C> + for RuntimeApi + where + C: #crate_::CallApiAt<Block> + 'static, + { + type RuntimeApi = RuntimeApiImpl<Block, C>; + + fn construct_runtime_api<'a>( + call: &'a C, + ) -> #crate_::ApiRef<'a, Self::RuntimeApi> { + RuntimeApiImpl { + call: unsafe { std::mem::transmute(call) }, + commit_on_success: true.into(), + changes: std::default::Default::default(), + recorder: std::default::Default::default(), + storage_transaction_cache: std::default::Default::default(), + }.into() + } } - } - #[cfg(any(feature = "std", test))] - impl<Block: #crate_::BlockT, C: #crate_::CallApiAt<Block>> RuntimeApiImpl<Block, C> { - fn commit_or_rollback(&self, commit: bool) { - let proof = "\ + impl<Block: #crate_::BlockT, C: #crate_::CallApiAt<Block>> RuntimeApiImpl<Block, C> { + fn commit_or_rollback(&self, commit: bool) { + let proof = "\ We only close a transaction when we opened one ourself. Other parts of the runtime that make use of transactions (state-machine) also balance their transactions. The runtime cannot close client initiated transactions; qed"; - if *std::cell::RefCell::borrow(&self.commit_on_success) { - let res = if commit { - let res = if let Some(recorder) = &self.recorder { - #crate_::ProofRecorder::<Block>::commit_transaction(&recorder) + if *std::cell::RefCell::borrow(&self.commit_on_success) { + let res = if commit { + let res = if let Some(recorder) = &self.recorder { + #crate_::ProofRecorder::<Block>::commit_transaction(&recorder) + } else { + Ok(()) + }; + + let res2 = #crate_::OverlayedChanges::commit_transaction( + &mut std::cell::RefCell::borrow_mut(&self.changes) + ); + + // Will panic on an `Err` below, however we should call commit + // on the recorder and the changes together. + std::result::Result::and(res, std::result::Result::map_err(res2, drop)) } else { - Ok(()) + let res = if let Some(recorder) = &self.recorder { + #crate_::ProofRecorder::<Block>::rollback_transaction(&recorder) + } else { + Ok(()) + }; + + let res2 = #crate_::OverlayedChanges::rollback_transaction( + &mut std::cell::RefCell::borrow_mut(&self.changes) + ); + + // Will panic on an `Err` below, however we should call commit + // on the recorder and the changes together. + std::result::Result::and(res, std::result::Result::map_err(res2, drop)) }; - let res2 = #crate_::OverlayedChanges::commit_transaction( - &mut std::cell::RefCell::borrow_mut(&self.changes) - ); - - // Will panic on an `Err` below, however we should call commit - // on the recorder and the changes together. - std::result::Result::and(res, std::result::Result::map_err(res2, drop)) - } else { - let res = if let Some(recorder) = &self.recorder { - #crate_::ProofRecorder::<Block>::rollback_transaction(&recorder) - } else { - Ok(()) - }; - - let res2 = #crate_::OverlayedChanges::rollback_transaction( - &mut std::cell::RefCell::borrow_mut(&self.changes) - ); - - // Will panic on an `Err` below, however we should call commit - // on the recorder and the changes together. - std::result::Result::and(res, std::result::Result::map_err(res2, drop)) - }; - - std::result::Result::expect(res, proof); + std::result::Result::expect(res, proof); + } } - } - fn start_transaction(&self) { - if !*std::cell::RefCell::borrow(&self.commit_on_success) { - return - } + fn start_transaction(&self) { + if !*std::cell::RefCell::borrow(&self.commit_on_success) { + return + } - #crate_::OverlayedChanges::start_transaction( - &mut std::cell::RefCell::borrow_mut(&self.changes) - ); - if let Some(recorder) = &self.recorder { - #crate_::ProofRecorder::<Block>::start_transaction(&recorder); + #crate_::OverlayedChanges::start_transaction( + &mut std::cell::RefCell::borrow_mut(&self.changes) + ); + if let Some(recorder) = &self.recorder { + #crate_::ProofRecorder::<Block>::start_transaction(&recorder); + } } } } @@ -571,10 +571,6 @@ impl<'a> Fold for ApiRuntimeImplToApiRuntimeApiImpl<'a> { input.attrs = filter_cfg_attrs(&input.attrs); - // The implementation for the `RuntimeApiImpl` is only required when compiling with - // the feature `std` or `test`. - input.attrs.push(parse_quote!( #[cfg(any(feature = "std", test))] )); - fold::fold_item_impl(self, input) } } @@ -595,7 +591,10 @@ fn generate_api_impl_for_runtime_api(impls: &[ItemImpl]) -> Result<TokenStream> result.push(processed_impl); } - Ok(quote!( #( #result )* )) + + let crate_ = generate_crate_access(); + + Ok(quote!( #crate_::std_enabled! { #( #result )* } )) } fn populate_runtime_api_versions( @@ -612,13 +611,14 @@ fn populate_runtime_api_versions( )); sections.push(quote!( - #( #attrs )* - const _: () = { - // All sections with the same name are going to be merged by concatenation. - #[cfg(not(feature = "std"))] - #[link_section = "runtime_apis"] - static SECTION_CONTENTS: [u8; 12] = #crate_access::serialize_runtime_api_info(#id, #version); - }; + #crate_access::std_disabled! { + #( #attrs )* + const _: () = { + // All sections with the same name are going to be merged by concatenation. + #[link_section = "runtime_apis"] + static SECTION_CONTENTS: [u8; 12] = #crate_access::serialize_runtime_api_info(#id, #version); + }; + } )); } diff --git a/substrate/primitives/api/src/lib.rs b/substrate/primitives/api/src/lib.rs index 02770280f7b..4e474e02dea 100644 --- a/substrate/primitives/api/src/lib.rs +++ b/substrate/primitives/api/src/lib.rs @@ -750,3 +750,6 @@ decl_runtime_apis! { fn metadata_versions() -> sp_std::vec::Vec<u32>; } } + +sp_core::generate_feature_enabled_macro!(std_enabled, feature = "std", $); +sp_core::generate_feature_enabled_macro!(std_disabled, not(feature = "std"), $); diff --git a/substrate/primitives/api/test/Cargo.toml b/substrate/primitives/api/test/Cargo.toml index 7f5c527b1ba..376375bcc11 100644 --- a/substrate/primitives/api/test/Cargo.toml +++ b/substrate/primitives/api/test/Cargo.toml @@ -34,8 +34,3 @@ sp-core = { version = "21.0.0", path = "../../core" } [[bench]] name = "bench" harness = false - -# We only need this to generate the correct code. -[features] -default = [ "std" ] -std = [] diff --git a/substrate/primitives/core/src/lib.rs b/substrate/primitives/core/src/lib.rs index b61009bc640..951b481253b 100644 --- a/substrate/primitives/core/src/lib.rs +++ b/substrate/primitives/core/src/lib.rs @@ -442,6 +442,18 @@ pub const MAX_POSSIBLE_ALLOCATION: u32 = 33554432; // 2^25 bytes, 32 MiB /// /// These feature checking macros can be used to conditionally enable/disable code in a dependent /// crate based on a feature in the crate where the macro is called. +/// +/// # Example +///``` +/// sp_core::generate_feature_enabled_macro!(check_std_is_enabled, feature = "std", $); +/// sp_core::generate_feature_enabled_macro!(check_std_or_serde_is_enabled, any(feature = "std", feature = "serde"), $); +/// +/// // All the code passed to the macro will then conditionally compiled based on the features +/// // activated for the crate where the macro was generated. +/// check_std_is_enabled! { +/// struct StdEnabled; +/// } +///``` #[macro_export] // We need to skip formatting this macro because of this bug: // https://github.com/rust-lang/rustfmt/issues/5283 -- GitLab