From d2240b4df9128b0f60316f2c529d7173145acd8b Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Tue, 26 Sep 2023 19:40:10 +0200 Subject: [PATCH 1/4] Use `Extensions` to register offchain worker extensions --- Cargo.lock | 1 + cumulus/parachain-template/node/src/service.rs | 2 +- polkadot/node/service/src/lib.rs | 2 +- substrate/bin/node-template/node/src/service.rs | 2 +- substrate/bin/node/cli/Cargo.toml | 1 + substrate/bin/node/cli/src/service.rs | 5 ++++- substrate/client/offchain/src/lib.rs | 14 ++++++++------ .../api/proc-macro/src/impl_runtime_apis.rs | 4 ++++ .../api/proc-macro/src/mock_impl_runtime_apis.rs | 4 ++++ substrate/primitives/api/src/lib.rs | 3 +++ 10 files changed, 28 insertions(+), 10 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index dade37f9a0258..d4ed7047c7dfa 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8243,6 +8243,7 @@ dependencies = [ "sp-consensus-babe", "sp-consensus-grandpa", "sp-core", + "sp-externalities", "sp-inherents", "sp-io", "sp-keyring", diff --git a/cumulus/parachain-template/node/src/service.rs b/cumulus/parachain-template/node/src/service.rs index 9fa6d60c2e74a..b69f2f133957f 100644 --- a/cumulus/parachain-template/node/src/service.rs +++ b/cumulus/parachain-template/node/src/service.rs @@ -213,7 +213,7 @@ async fn start_node_impl( network_provider: network.clone(), is_validator: parachain_config.role.is_authority(), enable_http_requests: false, - custom_extensions: move |_| vec![], + custom_extensions: move |_| Default::default(), }) .run(client.clone(), task_manager.spawn_handle()) .boxed(), diff --git a/polkadot/node/service/src/lib.rs b/polkadot/node/service/src/lib.rs index 5286631fbbb5c..b545ba979c761 100644 --- a/polkadot/node/service/src/lib.rs +++ b/polkadot/node/service/src/lib.rs @@ -909,7 +909,7 @@ pub fn new_full( network_provider: network.clone(), is_validator: role.is_authority(), enable_http_requests: false, - custom_extensions: move |_| vec![], + custom_extensions: move |_| Default::default(), }) .run(client.clone(), task_manager.spawn_handle()) .boxed(), diff --git a/substrate/bin/node-template/node/src/service.rs b/substrate/bin/node-template/node/src/service.rs index 403202829241e..5875bcd3f9501 100644 --- a/substrate/bin/node-template/node/src/service.rs +++ b/substrate/bin/node-template/node/src/service.rs @@ -200,7 +200,7 @@ pub fn new_full(config: Configuration) -> Result { )), network_provider: network.clone(), enable_http_requests: true, - custom_extensions: |_| vec![], + custom_extensions: |_| Default::default(), }) .run(client.clone(), task_manager.spawn_handle()) .boxed(), diff --git a/substrate/bin/node/cli/Cargo.toml b/substrate/bin/node/cli/Cargo.toml index c47f8a5c3e52f..f47be5a2fab3d 100644 --- a/substrate/bin/node/cli/Cargo.toml +++ b/substrate/bin/node/cli/Cargo.toml @@ -57,6 +57,7 @@ sp-timestamp = { path = "../../../primitives/timestamp" } sp-inherents = { path = "../../../primitives/inherents" } sp-keyring = { path = "../../../primitives/keyring" } sp-keystore = { path = "../../../primitives/keystore" } +sp-externalities = { path = "../../../primitives/externalities" } sp-consensus = { path = "../../../primitives/consensus/common" } sp-transaction-storage-proof = { path = "../../../primitives/transaction-storage-proof" } sp-io = { path = "../../../primitives/io" } diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs index 977c90e73e9ff..3cb5d3c4d82f9 100644 --- a/substrate/bin/node/cli/src/service.rs +++ b/substrate/bin/node/cli/src/service.rs @@ -39,6 +39,7 @@ use sc_telemetry::{Telemetry, TelemetryWorker}; use sc_transaction_pool_api::OffchainTransactionPoolFactory; use sp_api::ProvideRuntimeApi; use sp_core::crypto::Pair; +use sp_externalities::Extensions; use sp_runtime::{generic, traits::Block as BlockT, SaturatedConversion}; use std::sync::Arc; @@ -602,7 +603,9 @@ pub fn new_full_base( is_validator: role.is_authority(), enable_http_requests: true, custom_extensions: move |_| { - vec![Box::new(statement_store.clone().as_statement_store_ext()) as Box<_>] + let mut extensions = Extensions::new(); + extensions.register(statement_store.clone().as_statement_store_ext()); + extensions }, }) .run(client.clone(), task_manager.spawn_handle()) diff --git a/substrate/client/offchain/src/lib.rs b/substrate/client/offchain/src/lib.rs index a11ac7d86ecb8..130652d193cb1 100644 --- a/substrate/client/offchain/src/lib.rs +++ b/substrate/client/offchain/src/lib.rs @@ -47,7 +47,7 @@ use sc_network::{NetworkPeers, NetworkStateInfo}; use sc_transaction_pool_api::OffchainTransactionPoolFactory; use sp_api::{ApiExt, ProvideRuntimeApi}; use sp_core::{offchain, traits::SpawnNamed}; -use sp_externalities::Extension; +use sp_externalities::Extensions; use sp_keystore::{KeystoreExt, KeystorePtr}; use sp_runtime::traits::{self, Header}; use threadpool::ThreadPool; @@ -120,7 +120,9 @@ pub struct OffchainWorkerOptions { /// /// ```nocompile /// custom_extensions: |block_hash| { - /// vec![MyCustomExtension::new()] + /// let extensions = Extensions::new(); + /// extensions.register(MyCustomExtension::new()); + /// extensions /// } /// ``` pub custom_extensions: CE, @@ -137,12 +139,12 @@ pub struct OffchainWorkers { transaction_pool: Option>, network_provider: Arc, is_validator: bool, - custom_extensions: Box Vec> + Send>, + custom_extensions: Box Extensions + Send>, } impl OffchainWorkers { /// Creates new [`OffchainWorkers`]. - pub fn new Vec> + Send + 'static>( + pub fn new Extensions + Send + 'static>( OffchainWorkerOptions { runtime_api_provider, keystore, @@ -283,7 +285,7 @@ where offchain::LimitedExternalities::new(capabilities, api), )); - custom_extensions.into_iter().for_each(|ext| runtime.register_extension(ext)); + runtime.register_extensions(custom_extensions); let run = if version == 2 { runtime.offchain_worker(hash, &header) @@ -444,7 +446,7 @@ mod tests { network_provider: network, is_validator: false, enable_http_requests: false, - custom_extensions: |_| Vec::new(), + custom_extensions: |_| Default::default(), }); futures::executor::block_on(offchain.on_block_imported(&header)); 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 74cfa0980623b..bf696986e3692 100644 --- a/substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs +++ b/substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs @@ -365,6 +365,10 @@ fn generate_runtime_api_base_structures() -> Result { fn register_extension(&mut self, extension: E) { std::cell::RefCell::borrow_mut(&self.extensions).register(extension); } + + fn register_extensions(&mut self, extension: #crate_::Extensions) { + std::cell::RefCell::borrow_mut(&self.extensions).merge(extension); + } } impl #crate_::ConstructRuntimeApi diff --git a/substrate/primitives/api/proc-macro/src/mock_impl_runtime_apis.rs b/substrate/primitives/api/proc-macro/src/mock_impl_runtime_apis.rs index c1339ff6621b3..185f545f6bc34 100644 --- a/substrate/primitives/api/proc-macro/src/mock_impl_runtime_apis.rs +++ b/substrate/primitives/api/proc-macro/src/mock_impl_runtime_apis.rs @@ -127,6 +127,10 @@ fn implement_common_api_traits(block_type: TypePath, self_ty: Type) -> Result(&mut self, _: E) { unimplemented!("`register_extension` not implemented for runtime api mocks") } + + fn register_extensions(&mut self, _: #crate_::Extensions) { + unimplemented!("`register_extensions` not implemented for runtime api mocks") + } } impl #crate_::Core<#block_type> for #self_ty { diff --git a/substrate/primitives/api/src/lib.rs b/substrate/primitives/api/src/lib.rs index c3f80acf09ae5..225b41ff1eaa0 100644 --- a/substrate/primitives/api/src/lib.rs +++ b/substrate/primitives/api/src/lib.rs @@ -620,6 +620,9 @@ pub trait ApiExt { /// Register an [`Extension`] that will be accessible while executing a runtime api call. fn register_extension(&mut self, extension: E); + + /// Register a set of [`Extensions`] that will be accessible while executing a runtime api call. + fn register_extensions(&mut self, extensions: Extensions); } /// Parameters for [`CallApiAt::call_api_at`]. From a48ea31c0f00c6802500d35512a97b510843a6a2 Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Wed, 27 Sep 2023 14:35:15 +0200 Subject: [PATCH 2/4] Revert "Use `Extensions` to register offchain worker extensions" This reverts commit d2240b4df9128b0f60316f2c529d7173145acd8b. --- Cargo.lock | 1 - cumulus/parachain-template/node/src/service.rs | 2 +- polkadot/node/service/src/lib.rs | 2 +- substrate/bin/node-template/node/src/service.rs | 2 +- substrate/bin/node/cli/Cargo.toml | 1 - substrate/bin/node/cli/src/service.rs | 5 +---- substrate/client/offchain/src/lib.rs | 14 ++++++-------- .../api/proc-macro/src/impl_runtime_apis.rs | 4 ---- .../api/proc-macro/src/mock_impl_runtime_apis.rs | 4 ---- substrate/primitives/api/src/lib.rs | 3 --- 10 files changed, 10 insertions(+), 28 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d4ed7047c7dfa..dade37f9a0258 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -8243,7 +8243,6 @@ dependencies = [ "sp-consensus-babe", "sp-consensus-grandpa", "sp-core", - "sp-externalities", "sp-inherents", "sp-io", "sp-keyring", diff --git a/cumulus/parachain-template/node/src/service.rs b/cumulus/parachain-template/node/src/service.rs index b69f2f133957f..9fa6d60c2e74a 100644 --- a/cumulus/parachain-template/node/src/service.rs +++ b/cumulus/parachain-template/node/src/service.rs @@ -213,7 +213,7 @@ async fn start_node_impl( network_provider: network.clone(), is_validator: parachain_config.role.is_authority(), enable_http_requests: false, - custom_extensions: move |_| Default::default(), + custom_extensions: move |_| vec![], }) .run(client.clone(), task_manager.spawn_handle()) .boxed(), diff --git a/polkadot/node/service/src/lib.rs b/polkadot/node/service/src/lib.rs index b545ba979c761..5286631fbbb5c 100644 --- a/polkadot/node/service/src/lib.rs +++ b/polkadot/node/service/src/lib.rs @@ -909,7 +909,7 @@ pub fn new_full( network_provider: network.clone(), is_validator: role.is_authority(), enable_http_requests: false, - custom_extensions: move |_| Default::default(), + custom_extensions: move |_| vec![], }) .run(client.clone(), task_manager.spawn_handle()) .boxed(), diff --git a/substrate/bin/node-template/node/src/service.rs b/substrate/bin/node-template/node/src/service.rs index 5875bcd3f9501..403202829241e 100644 --- a/substrate/bin/node-template/node/src/service.rs +++ b/substrate/bin/node-template/node/src/service.rs @@ -200,7 +200,7 @@ pub fn new_full(config: Configuration) -> Result { )), network_provider: network.clone(), enable_http_requests: true, - custom_extensions: |_| Default::default(), + custom_extensions: |_| vec![], }) .run(client.clone(), task_manager.spawn_handle()) .boxed(), diff --git a/substrate/bin/node/cli/Cargo.toml b/substrate/bin/node/cli/Cargo.toml index f47be5a2fab3d..c47f8a5c3e52f 100644 --- a/substrate/bin/node/cli/Cargo.toml +++ b/substrate/bin/node/cli/Cargo.toml @@ -57,7 +57,6 @@ sp-timestamp = { path = "../../../primitives/timestamp" } sp-inherents = { path = "../../../primitives/inherents" } sp-keyring = { path = "../../../primitives/keyring" } sp-keystore = { path = "../../../primitives/keystore" } -sp-externalities = { path = "../../../primitives/externalities" } sp-consensus = { path = "../../../primitives/consensus/common" } sp-transaction-storage-proof = { path = "../../../primitives/transaction-storage-proof" } sp-io = { path = "../../../primitives/io" } diff --git a/substrate/bin/node/cli/src/service.rs b/substrate/bin/node/cli/src/service.rs index 3cb5d3c4d82f9..977c90e73e9ff 100644 --- a/substrate/bin/node/cli/src/service.rs +++ b/substrate/bin/node/cli/src/service.rs @@ -39,7 +39,6 @@ use sc_telemetry::{Telemetry, TelemetryWorker}; use sc_transaction_pool_api::OffchainTransactionPoolFactory; use sp_api::ProvideRuntimeApi; use sp_core::crypto::Pair; -use sp_externalities::Extensions; use sp_runtime::{generic, traits::Block as BlockT, SaturatedConversion}; use std::sync::Arc; @@ -603,9 +602,7 @@ pub fn new_full_base( is_validator: role.is_authority(), enable_http_requests: true, custom_extensions: move |_| { - let mut extensions = Extensions::new(); - extensions.register(statement_store.clone().as_statement_store_ext()); - extensions + vec![Box::new(statement_store.clone().as_statement_store_ext()) as Box<_>] }, }) .run(client.clone(), task_manager.spawn_handle()) diff --git a/substrate/client/offchain/src/lib.rs b/substrate/client/offchain/src/lib.rs index 130652d193cb1..a11ac7d86ecb8 100644 --- a/substrate/client/offchain/src/lib.rs +++ b/substrate/client/offchain/src/lib.rs @@ -47,7 +47,7 @@ use sc_network::{NetworkPeers, NetworkStateInfo}; use sc_transaction_pool_api::OffchainTransactionPoolFactory; use sp_api::{ApiExt, ProvideRuntimeApi}; use sp_core::{offchain, traits::SpawnNamed}; -use sp_externalities::Extensions; +use sp_externalities::Extension; use sp_keystore::{KeystoreExt, KeystorePtr}; use sp_runtime::traits::{self, Header}; use threadpool::ThreadPool; @@ -120,9 +120,7 @@ pub struct OffchainWorkerOptions { /// /// ```nocompile /// custom_extensions: |block_hash| { - /// let extensions = Extensions::new(); - /// extensions.register(MyCustomExtension::new()); - /// extensions + /// vec![MyCustomExtension::new()] /// } /// ``` pub custom_extensions: CE, @@ -139,12 +137,12 @@ pub struct OffchainWorkers { transaction_pool: Option>, network_provider: Arc, is_validator: bool, - custom_extensions: Box Extensions + Send>, + custom_extensions: Box Vec> + Send>, } impl OffchainWorkers { /// Creates new [`OffchainWorkers`]. - pub fn new Extensions + Send + 'static>( + pub fn new Vec> + Send + 'static>( OffchainWorkerOptions { runtime_api_provider, keystore, @@ -285,7 +283,7 @@ where offchain::LimitedExternalities::new(capabilities, api), )); - runtime.register_extensions(custom_extensions); + custom_extensions.into_iter().for_each(|ext| runtime.register_extension(ext)); let run = if version == 2 { runtime.offchain_worker(hash, &header) @@ -446,7 +444,7 @@ mod tests { network_provider: network, is_validator: false, enable_http_requests: false, - custom_extensions: |_| Default::default(), + custom_extensions: |_| Vec::new(), }); futures::executor::block_on(offchain.on_block_imported(&header)); 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 bf696986e3692..74cfa0980623b 100644 --- a/substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs +++ b/substrate/primitives/api/proc-macro/src/impl_runtime_apis.rs @@ -365,10 +365,6 @@ fn generate_runtime_api_base_structures() -> Result { fn register_extension(&mut self, extension: E) { std::cell::RefCell::borrow_mut(&self.extensions).register(extension); } - - fn register_extensions(&mut self, extension: #crate_::Extensions) { - std::cell::RefCell::borrow_mut(&self.extensions).merge(extension); - } } impl #crate_::ConstructRuntimeApi diff --git a/substrate/primitives/api/proc-macro/src/mock_impl_runtime_apis.rs b/substrate/primitives/api/proc-macro/src/mock_impl_runtime_apis.rs index 185f545f6bc34..c1339ff6621b3 100644 --- a/substrate/primitives/api/proc-macro/src/mock_impl_runtime_apis.rs +++ b/substrate/primitives/api/proc-macro/src/mock_impl_runtime_apis.rs @@ -127,10 +127,6 @@ fn implement_common_api_traits(block_type: TypePath, self_ty: Type) -> Result(&mut self, _: E) { unimplemented!("`register_extension` not implemented for runtime api mocks") } - - fn register_extensions(&mut self, _: #crate_::Extensions) { - unimplemented!("`register_extensions` not implemented for runtime api mocks") - } } impl #crate_::Core<#block_type> for #self_ty { diff --git a/substrate/primitives/api/src/lib.rs b/substrate/primitives/api/src/lib.rs index 225b41ff1eaa0..c3f80acf09ae5 100644 --- a/substrate/primitives/api/src/lib.rs +++ b/substrate/primitives/api/src/lib.rs @@ -620,9 +620,6 @@ pub trait ApiExt { /// Register an [`Extension`] that will be accessible while executing a runtime api call. fn register_extension(&mut self, extension: E); - - /// Register a set of [`Extensions`] that will be accessible while executing a runtime api call. - fn register_extensions(&mut self, extensions: Extensions); } /// Parameters for [`CallApiAt::call_api_at`]. From c36c1bb9ee53bde48b91d5655fbcbe7bf836e62f Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Wed, 27 Sep 2023 16:26:32 +0200 Subject: [PATCH 3/4] Remove `Any` supertrait and add `type_id` --- .../externalities/src/extensions.rs | 38 ++++++++++++++++++- 1 file changed, 37 insertions(+), 1 deletion(-) diff --git a/substrate/primitives/externalities/src/extensions.rs b/substrate/primitives/externalities/src/extensions.rs index 8b0bbd2c5921b..be354b4a05cf7 100644 --- a/substrate/primitives/externalities/src/extensions.rs +++ b/substrate/primitives/externalities/src/extensions.rs @@ -35,17 +35,24 @@ use sp_std::{ /// /// As extensions are stored as `Box`, this trait should give more confidence that the correct /// type is registered and requested. -pub trait Extension: Send + Any { +pub trait Extension: Send + 'static { /// Return the extension as `&mut dyn Any`. /// /// This is a trick to make the trait type castable into an `Any`. fn as_mut_any(&mut self) -> &mut dyn Any; + + /// Get the [`std::any::TypeId`] of this `Extension`. + fn type_id(&self) -> TypeId; } impl Extension for Box { fn as_mut_any(&mut self) -> &mut dyn Any { (**self).as_mut_any() } + + fn type_id(&self) -> TypeId { + (**self).type_id() + } } /// Macro for declaring an extension that usable with [`Extensions`]. @@ -74,6 +81,10 @@ macro_rules! decl_extension { fn as_mut_any(&mut self) -> &mut dyn std::any::Any { self } + + fn type_id(&self) -> std::any::TypeId { + std::any::Any::type_id(self) + } } impl std::ops::Deref for $ext_name { @@ -107,6 +118,10 @@ macro_rules! decl_extension { fn as_mut_any(&mut self) -> &mut dyn std::any::Any { self } + + fn type_id(&self) -> std::any::TypeId { + std::any::Any::type_id(self) + } } } } @@ -235,4 +250,25 @@ mod tests { assert_eq!(ext_ty.0, 1); } + + #[test] + fn register_box_extension() { + let mut exts = Extensions::new(); + let box1: Box = Box::new(DummyExt(1)); + let box2: Box = Box::new(DummyExt2(2)); + exts.register(box1); + exts.register(box2); + + { + let ext = exts.get_mut(TypeId::of::()).expect("Extension 1 is registered"); + let ext_ty = ext.downcast_mut::().expect("Downcasting works for Extension 1"); + assert_eq!(ext_ty.0, 1); + } + { + let ext2 = exts.get_mut(TypeId::of::()).expect("Extension 2 is registered"); + let ext_ty2 = + ext2.downcast_mut::().expect("Downcasting works for Extension 2"); + assert_eq!(ext_ty2.0, 2); + } + } } From 49584deddef1fce682b8e266109bfd9b3479e37f Mon Sep 17 00:00:00 2001 From: Sebastian Kunert Date: Wed, 27 Sep 2023 23:30:54 +0200 Subject: [PATCH 4/4] Update extensions.rs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Bastian Köcher --- substrate/primitives/externalities/src/extensions.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/substrate/primitives/externalities/src/extensions.rs b/substrate/primitives/externalities/src/extensions.rs index be354b4a05cf7..282e6ea914a84 100644 --- a/substrate/primitives/externalities/src/extensions.rs +++ b/substrate/primitives/externalities/src/extensions.rs @@ -41,7 +41,7 @@ pub trait Extension: Send + 'static { /// This is a trick to make the trait type castable into an `Any`. fn as_mut_any(&mut self) -> &mut dyn Any; - /// Get the [`std::any::TypeId`] of this `Extension`. + /// Get the [`TypeId`] of this `Extension`. fn type_id(&self) -> TypeId; }